Просмотр исходного кода

fix(skill-sync): tighten conflict reconciliation

Alvin Unreal 1 месяц назад
Родитель
Сommit
eda11dfca1

+ 76 - 0
src/hooks/auto-update-checker/skill-sync.test.ts

@@ -1593,4 +1593,80 @@ describe('syncBundledSkillsFromPackage', () => {
       '# Backup Content',
     );
   });
+
+  test('LEGACY_MANAGED_SKILL_HASHES: is exported as a record mapping string keys to string arrays', async () => {
+    const { LEGACY_MANAGED_SKILL_HASHES } = await import(
+      `./skill-sync?test=${importCounter++}`
+    );
+    expect(typeof LEGACY_MANAGED_SKILL_HASHES).toBe('object');
+    expect(LEGACY_MANAGED_SKILL_HASHES).not.toBeNull();
+    for (const key of Object.keys(LEGACY_MANAGED_SKILL_HASHES)) {
+      expect(typeof key).toBe('string');
+      expect(Array.isArray(LEGACY_MANAGED_SKILL_HASHES[key])).toBe(true);
+    }
+  });
+
+  test('conflict staging failure: failed skills are not double-counted as skippedExisting', async () => {
+    const skillName = 'test-double-counting-skill';
+    const skillSrcDir = path.join(fakePackageRoot, 'src', 'skills', skillName);
+    fs.mkdirSync(skillSrcDir, { recursive: true });
+    fs.writeFileSync(path.join(skillSrcDir, 'SKILL.md'), '# Bundle Content');
+
+    const manifestDir = path.join(fakeDestConfigDir, '.oh-my-opencode-slim');
+    fs.mkdirSync(manifestDir, { recursive: true });
+    const manifestPath = path.join(manifestDir, 'skills-manifest.json');
+
+    const initialManifest = {
+      schemaVersion: 1,
+      updatedAt: new Date().toISOString(),
+      skills: {
+        [skillName]: {
+          status: 'conflict',
+          packageVersion: '1.0.0',
+          sourceHash: 'old-source-hash',
+          lastManagedHash: 'old-managed-hash',
+          lastSeenHash: 'old-seen-hash',
+          updatedAt: new Date().toISOString(),
+        },
+      },
+    };
+    fs.writeFileSync(manifestPath, JSON.stringify(initialManifest, null, 2));
+
+    const destSkillsDir = path.join(fakeDestConfigDir, 'skills');
+    fs.mkdirSync(destSkillsDir, { recursive: true });
+    // Destination is different (so we fall to destHash !== sourceHash, which attempts staging)
+    const destSkillDir = path.join(destSkillsDir, skillName);
+    fs.mkdirSync(destSkillDir, { recursive: true });
+    fs.writeFileSync(
+      path.join(destSkillDir, 'SKILL.md'),
+      '# Different Content',
+    );
+
+    // Force staging copy recursive to fail by making dest directory nested path locked
+    const stagedSkillDir = path.join(
+      manifestDir,
+      'skill-updates',
+      '1.2.3',
+      skillName,
+    );
+    // Write package.json mock version
+    fs.writeFileSync(
+      path.join(fakePackageRoot, 'package.json'),
+      JSON.stringify({ version: '1.2.3' }),
+    );
+
+    // Make fs mkdirSync throw on stagedSkillDir or hijack mkdirSync via mock module if possible and cleaner, OR lock it!
+    // Locking stagedParent updates directory
+    const stagedParent = path.dirname(stagedSkillDir);
+    fs.mkdirSync(stagedParent, { recursive: true });
+    fs.chmodSync(stagedParent, 0o000);
+
+    const result = await syncBundledSkillsFromPackage(fakePackageRoot);
+
+    expect(result.failed).toContain(skillName);
+    expect(result.skippedExisting).not.toContain(skillName);
+
+    // Reset permissions so afterEach cleanup succeeds
+    fs.chmodSync(stagedParent, 0o777);
+  });
 });

+ 15 - 3
src/hooks/auto-update-checker/skill-sync.ts

@@ -8,7 +8,6 @@ import {
   readFileSync,
   renameSync,
   rmSync,
-  statSync,
   unlinkSync,
   writeFileSync,
 } from 'node:fs';
@@ -69,6 +68,19 @@ interface SkillSyncOptions {
  * Hashes of historically managed versions of skills.
  * When a release changes skill content, this table must be populated
  * from the published npm package tarballs to allow upgrading existing users.
+ *
+ * How to populate:
+ * 1. Download previous releases of the npm package: `npm pack oh-my-opencode-slim@<version>`
+ * 2. Compute directory hash for each legacy skill directory inside the unpacked tarball:
+ *    `import { computeDirectoryHash } from './skill-sync';`
+ *    `const hash = computeDirectoryHash('path/to/extracted/package/src/skills/<skill-name>');`
+ * 3. Append the hash to the skill's string array below:
+ *    ```typescript
+ *    export const LEGACY_MANAGED_SKILL_HASHES: Record<string, string[]> = {
+ *      'simplify': ['hash1', 'hash2'],
+ *      'codemap': ['hash3']
+ *    };
+ *    ```
  */
 export const LEGACY_MANAGED_SKILL_HASHES: Record<string, string[]> = {};
 
@@ -274,7 +286,7 @@ export function acquireLock(lockDir: string): boolean {
         shouldSteal = true;
       }
     } else {
-      const stat = statSync(lockDir);
+      const stat = lstatSync(lockDir);
       ageMs = Date.now() - stat.mtimeMs;
       if (ageMs > 30000) {
         shouldSteal = true;
@@ -1130,6 +1142,7 @@ export function syncBundledSkillsFromPackage(
 
                 staged.push(skill.name);
                 customized.push(skill.name);
+                skippedExisting.push(skill.name);
                 log(
                   `[skill-sync] Conflicted skill ${skill.name} recovered as customized and staged at ${stagedSkillDir}`,
                 );
@@ -1141,7 +1154,6 @@ export function syncBundledSkillsFromPackage(
                 failed.push(skill.name);
               }
             }
-            skippedExisting.push(skill.name);
           }
         } else {
           if (destHash === sourceHash) {