Browse Source

fix(skill-sync): preserve sync API and result counts

Alvin Unreal 1 month ago
parent
commit
bafa56eaac

+ 2 - 4
src/cli/custom-skills.ts

@@ -1,5 +1,6 @@
 import { join } from 'node:path';
 import { fileURLToPath } from 'node:url';
+import { syncBundledSkillsFromPackage } from '../hooks/auto-update-checker/skill-sync';
 import { CUSTOM_SKILLS, type CustomSkill } from './custom-skills-registry';
 import { getConfigDir } from './paths';
 
@@ -18,14 +19,11 @@ export function getCustomSkillsDir(): string {
  * @returns True if installation succeeded, false otherwise
  * @deprecated Use syncBundledSkillsFromPackage instead.
  */
-export async function installCustomSkill(skill: CustomSkill): Promise<boolean> {
+export function installCustomSkill(skill: CustomSkill): boolean {
   console.warn(
     `[DEPRECATED] installCustomSkill is deprecated and will be removed. Use syncBundledSkillsFromPackage instead.`,
   );
   try {
-    const { syncBundledSkillsFromPackage } = await import(
-      '../hooks/auto-update-checker/skill-sync'
-    );
     const packageRoot = fileURLToPath(new URL('../..', import.meta.url));
     const result = syncBundledSkillsFromPackage(packageRoot, {
       skills: [skill],

+ 3 - 1
src/cli/install.test.ts

@@ -57,7 +57,6 @@ mock.module('../hooks/auto-update-checker/skill-sync', () => {
             installed: [],
             skippedExisting: [],
             failed: mockFailedResult,
-            updated: [],
             staged: mockStagedResult,
             adopted: mockAdoptedResult,
             customized: [],
@@ -230,6 +229,7 @@ describe('install skill synchronization error mapping', () => {
       msg?.includes('Skill synchronization complete.'),
     );
     expect(summaryMsg).toBeDefined();
+    expect(summaryMsg).toContain('Processed 0 skills:');
     expect(summaryMsg).toContain('0 failed.');
   });
 
@@ -259,6 +259,7 @@ describe('install skill synchronization error mapping', () => {
       msg?.includes('Skill synchronization complete.'),
     );
     expect(summaryMsg).toBeDefined();
+    expect(summaryMsg).toContain('Processed 0 skills:');
     expect(summaryMsg).toContain('0 failed.');
   });
 
@@ -283,6 +284,7 @@ describe('install skill synchronization error mapping', () => {
       msg?.includes('Skill synchronization complete.'),
     );
     expect(summaryMsg).toBeDefined();
+    expect(summaryMsg).toContain('Processed 1 skills:');
     expect(summaryMsg).toContain('1 failed.');
   });
 

+ 5 - 2
src/cli/install.ts

@@ -458,9 +458,12 @@ async function runInstall(config: InstallConfig): Promise<number> {
         const realFailed = result.failed.filter(
           (skill) => skill !== '__lock__' && skill !== '__manifest__',
         );
-        const totalCustom = CUSTOM_SKILLS.length;
+        const totalProcessed =
+          result.installed.length +
+          result.skippedExisting.length +
+          realFailed.length;
         printSuccess(
-          `Skill synchronization complete. Processed ${totalCustom} skills: ` +
+          `Skill synchronization complete. Processed ${totalProcessed} skills: ` +
             `${result.installed.length} installed/updated, ` +
             `${result.skippedExisting.length} skipped/preserved, ` +
             `${realFailed.length} failed.`,

+ 9 - 0
src/hooks/auto-update-checker/index.test.ts

@@ -144,6 +144,9 @@ describe('auto-update-checker/index', () => {
       installed: [],
       skippedExisting: [],
       failed: [],
+      staged: [],
+      adopted: [],
+      customized: [],
     }));
 
     companionUpdaterMocks.ensureCompanionVersion.mockReset();
@@ -255,6 +258,9 @@ describe('auto-update-checker/index', () => {
       installed: ['reflect', 'worktrees'],
       skippedExisting: ['codemap'],
       failed: [],
+      staged: [],
+      adopted: [],
+      customized: [],
     }));
 
     const { createAutoUpdateCheckerHook } = await import(
@@ -431,6 +437,9 @@ describe('auto-update-checker/index', () => {
       installed: [],
       skippedExisting: [],
       failed: ['reflect'],
+      staged: [],
+      adopted: [],
+      customized: [],
     }));
 
     const { createAutoUpdateCheckerHook } = await import(

+ 2 - 2
src/hooks/auto-update-checker/index.ts

@@ -221,8 +221,8 @@ async function runBackgroundUpdateCheck(
     try {
       const syncResult = syncBundledSkillsFromPackage(packageRoot);
       installedSkills = syncResult.installed;
-      stagedSkills = syncResult.staged ?? [];
-      customizedSkills = syncResult.customized ?? [];
+      stagedSkills = syncResult.staged;
+      customizedSkills = syncResult.customized;
       if (syncResult.failed.length > 0) {
         log(
           `[auto-update-checker] Skill sync warnings/failures: ${syncResult.failed.join(', ')}`,

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

@@ -1446,7 +1446,6 @@ describe('syncBundledSkillsFromPackage', () => {
     const result = await syncBundledSkillsFromPackage(fakePackageRoot);
 
     expect(result.installed).toContain(skillName);
-    expect(result.updated).toContain(skillName);
     expect(fs.existsSync(path.join(destSkillDir, 'user-link'))).toBe(false);
     expect(fs.readFileSync(path.join(destSkillDir, 'SKILL.md'), 'utf-8')).toBe(
       '# Updated',

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

@@ -35,7 +35,6 @@ export interface SkillSyncResult {
   installed: string[];
   skippedExisting: string[];
   failed: string[];
-  updated: string[];
   staged: string[];
   adopted: string[];
   customized: string[];
@@ -544,7 +543,6 @@ export function syncBundledSkillsFromPackage(
   const installed: string[] = [];
   const skippedExisting: string[] = [];
   const failed: string[] = [];
-  const updated: string[] = [];
   const staged: string[] = [];
   const adopted: string[] = [];
   const customized: string[] = [];
@@ -561,7 +559,6 @@ export function syncBundledSkillsFromPackage(
         installed,
         skippedExisting,
         failed,
-        updated,
         staged,
         adopted,
         customized,
@@ -575,7 +572,6 @@ export function syncBundledSkillsFromPackage(
       installed,
       skippedExisting,
       failed,
-      updated,
       staged,
       adopted,
       customized,
@@ -619,7 +615,6 @@ export function syncBundledSkillsFromPackage(
       installed,
       skippedExisting,
       failed: ['__lock__'],
-      updated,
       staged,
       adopted,
       customized,
@@ -898,7 +893,6 @@ export function syncBundledSkillsFromPackage(
                 try {
                   atomicReplaceDir(sourcePath, destPath);
                   installed.push(skill.name);
-                  updated.push(skill.name);
                   manifest.skills[skill.name] = {
                     status: 'managed',
                     packageVersion,
@@ -1168,7 +1162,6 @@ export function syncBundledSkillsFromPackage(
             try {
               atomicReplaceDir(sourcePath, destPath);
               installed.push(skill.name);
-              updated.push(skill.name);
               manifest.skills[skill.name] = {
                 status: 'managed',
                 packageVersion,
@@ -1262,7 +1255,6 @@ export function syncBundledSkillsFromPackage(
     installed,
     skippedExisting,
     failed,
-    updated,
     staged,
     adopted,
     customized,