fix(serve): allow preinstall skill batch toggles (#9139)

Treat names absent from the installed Skill snapshot as valid batch targets so their workspace state can be declared before installation. Preserve the existing persistence semantics: an undeclared enable is a no-op, an existing workspace disable is removed, and disable writes the declaration.

docs/design/daemon-skill-batch-toggle.md

@@ -24,11 +24,16 @@ The request body is:
 
 `skillNames` is a non-empty string array with at most 100 entries. Names are
 trimmed and deduplicated case-insensitively while preserving first-seen order.
-The response is best-effort for expected target errors: valid targets are
-validated against one status snapshot, persisted in one locked write, and
-applied with one live-session refresh. Unknown, hidden, inactive-extension,
-and locked targets are returned without blocking the valid targets. Unexpected
-persistence and runtime-generation failures fail the whole request.
+The response is best-effort for expected target errors: installed targets are
+validated against one status snapshot, all valid names are persisted in one
+locked write, and changes are applied with one live-session refresh. Names
+that are not installed remain valid so callers can declare their state before
+installation. Enabling one removes a matching workspace `skills.disabled`
+entry and is otherwise a no-op, except for the existing `defaultDisabled`
+override behavior; disabling one writes `skills.disabled`. Hidden,
+inactive-extension, and locked targets are returned without blocking valid
+targets. Unexpected persistence and runtime-generation failures fail the whole
+request.
 
 ```json
 {
@@ -46,15 +51,14 @@ persistence and runtime-generation failures fail the whole request.
       "skillName": "deploy",
       "enabled": false,
       "changed": true
-    }
-  ],
-  "errors": [
+    },
     {
       "skillName": "missing",
-      "code": "skill_not_found",
-      "error": "Skill not found: missing"
+      "enabled": false,
+      "changed": true
     }
-  ]
+  ],
+  "errors": []
 }
 ```
 

packages/cli/src/serve/routes/workspace-skills.test.ts

@@ -173,20 +173,18 @@ describe('workspace Skill management routes', () => {
     expect(harness.deleteWorkspaceSkill).not.toHaveBeenCalled();
   });
 
-  it('toggles a deduplicated Skill batch and returns per-target errors', async () => {
+  it('toggles a deduplicated Skill batch and returns per-target outcomes', async () => {
     const harness = createHarness();
     harness.setWorkspaceSkillsEnabled.mockResolvedValueOnce({
       enabled: false,
       activation: 'applied',
       sessionsRefreshed: 1,
       sessionsFailed: 0,
-      results: [{ skillName: 'review', enabled: false, changed: true }],
+      results: [
+        { skillName: 'review', enabled: false, changed: true },
+        { skillName: 'missing', enabled: false, changed: true },
+      ],
       errors: [
-        {
-          skillName: 'missing',
-          code: 'skill_not_found',
-          error: 'Skill not found: missing',
-        },
         {
           skillName: 'locked',
           code: 'skill_not_toggleable',
@@ -216,13 +214,13 @@ describe('workspace Skill management routes', () => {
           enabled: false,
           changed: true,
         },
-      ],
-      errors: [
         {
           skillName: 'missing',
-          code: 'skill_not_found',
-          error: 'Skill not found: missing',
+          enabled: false,
+          changed: true,
         },
+      ],
+      errors: [
         {
           skillName: 'locked',
           code: 'skill_not_toggleable',

packages/cli/src/serve/run-qwen-serve.test.ts

@@ -914,6 +914,30 @@ describe('workspace skill settings persistence', () => {
     expect(savedUser.skills.disabled).toEqual(['locked-skill']);
     expect(savedUser.skills.enabled).toBeUndefined();
 
+    const preinstallNoop = await persistDisabledSkillsBatch!(
+      workspace,
+      ['future-skill'],
+      true,
+    );
+    expect(preinstallNoop.outcomes).toEqual([
+      { skillName: 'future-skill', changed: false },
+    ]);
+    expect(preinstallNoop.settingsChanges).toEqual([]);
+    expect(setValues).toHaveBeenCalledOnce();
+
+    const preinstallEnable = await persistDisabledSkillsBatch!(
+      workspace,
+      ['orphan'],
+      true,
+    );
+    expect(preinstallEnable.outcomes).toEqual([
+      { skillName: 'orphan', changed: true },
+    ]);
+    expect(preinstallEnable.settingsChanges).toEqual([
+      { key: 'skills.disabled', value: ['review', 'alpha'] },
+    ]);
+    expect(setValues).toHaveBeenCalledTimes(2);
+
     const enableResult = await persistDisabledSkillsBatch!(
       workspace,
       ['opt-in'],
@@ -929,16 +953,12 @@ describe('workspace skill settings persistence', () => {
         value: ['opt-in'],
       },
     ]);
-    expect(setValues).toHaveBeenCalledTimes(2);
+    expect(setValues).toHaveBeenCalledTimes(3);
 
     const savedAfterEnable = JSON.parse(
       fs.readFileSync(path.join(workspace, '.qwen', 'settings.json'), 'utf8'),
     ) as { skills: { disabled: string[]; enabled: string[] } };
-    expect(savedAfterEnable.skills.disabled).toEqual([
-      'orphan',
-      'review',
-      'alpha',
-    ]);
+    expect(savedAfterEnable.skills.disabled).toEqual(['review', 'alpha']);
     expect(savedAfterEnable.skills.enabled).toEqual(['opt-in']);
 
     const guard = vi.fn();

packages/cli/src/serve/workspace-service/__tests__/facade.test.ts

@@ -2098,6 +2098,38 @@ describe('createDaemonWorkspaceService', () => {
       },
     ];
 
+    it('accepts enabling a Skill before installation as an idempotent result', async () => {
+      const persistDisabledSkillsBatch = vi.fn().mockResolvedValue({
+        outcomes: [{ skillName: 'future-skill', changed: false }],
+        settingsChanges: [],
+      });
+      const svc = createDaemonWorkspaceService(
+        makeDeps({
+          queryWorkspaceStatus: vi.fn().mockResolvedValue({
+            v: 1,
+            workspaceCwd: '/workspace',
+            initialized: true,
+            skills,
+          }),
+          persistDisabledSkillsBatch,
+          isChannelLive: () => false,
+        }),
+      );
+
+      await expect(
+        svc.setWorkspaceSkillsEnabled(makeCtx(), ['future-skill'], true),
+      ).resolves.toMatchObject({
+        results: [{ skillName: 'future-skill', enabled: true, changed: false }],
+        errors: [],
+      });
+      expect(persistDisabledSkillsBatch).toHaveBeenCalledWith(
+        '/workspace',
+        ['future-skill'],
+        true,
+        undefined,
+      );
+    });
+
     it('persists and refreshes once while preserving ordered target outcomes', async () => {
       const queryWorkspaceStatus = vi.fn().mockResolvedValue({
         v: 1,
@@ -2108,6 +2140,7 @@ describe('createDaemonWorkspaceService', () => {
       const persistDisabledSkillsBatch = vi.fn().mockResolvedValue({
         outcomes: [
           { skillName: 'review', changed: true },
+          { skillName: 'missing', changed: true },
           {
             skillName: 'locked',
             error: new WorkspaceSkillNotToggleableError(
@@ -2119,7 +2152,10 @@ describe('createDaemonWorkspaceService', () => {
           { skillName: 'deploy', changed: true },
         ],
         settingsChanges: [
-          { key: 'skills.disabled', value: ['review', 'deploy'] },
+          {
+            key: 'skills.disabled',
+            value: ['review', 'missing', 'deploy'],
+          },
         ],
       });
       const invokeWorkspaceCommand = vi.fn().mockResolvedValue({
@@ -2147,7 +2183,7 @@ describe('createDaemonWorkspaceService', () => {
       expect(persistDisabledSkillsBatch).toHaveBeenCalledOnce();
       expect(persistDisabledSkillsBatch).toHaveBeenCalledWith(
         '/workspace',
-        ['review', 'locked', 'deploy'],
+        ['review', 'missing', 'locked', 'deploy'],
         false,
         undefined,
       );
@@ -2163,14 +2199,10 @@ describe('createDaemonWorkspaceService', () => {
         sessionsFailed: 0,
         results: [
           { skillName: 'review', enabled: false, changed: true },
+          { skillName: 'missing', enabled: false, changed: true },
           { skillName: 'deploy', enabled: false, changed: true },
         ],
         errors: [
-          {
-            skillName: 'missing',
-            code: 'skill_not_found',
-            error: 'Skill not found: missing',
-          },
           {
             skillName: 'hidden',
             code: 'skill_not_toggleable',
@@ -2197,7 +2229,7 @@ describe('createDaemonWorkspaceService', () => {
         type: 'settings_changed',
         data: {
           key: 'skills.disabled',
-          value: ['review', 'deploy'],
+          value: ['review', 'missing', 'deploy'],
           scope: 'workspace',
         },
         originatorClientId: 'client-1',
@@ -2225,6 +2257,7 @@ describe('createDaemonWorkspaceService', () => {
                 ),
               },
               { skillName: 'review', changed: true },
+              { skillName: 'missing', changed: true },
             ],
             settingsChanges: [],
           }),
@@ -2240,6 +2273,7 @@ describe('createDaemonWorkspaceService', () => {
 
       expect(result.results).toEqual([
         { skillName: 'review', enabled: false, changed: true },
+        { skillName: 'missing', enabled: false, changed: true },
         { skillName: 'deploy', enabled: false, changed: true },
       ]);
       expect(result.errors).toEqual([
@@ -2250,11 +2284,6 @@ describe('createDaemonWorkspaceService', () => {
           reason: 'locked',
           lockedScope: 'user',
         },
-        {
-          skillName: 'missing',
-          code: 'skill_not_found',
-          error: 'Skill not found: missing',
-        },
       ]);
     });
 
@@ -2329,18 +2358,13 @@ describe('createDaemonWorkspaceService', () => {
       );
 
       await expect(
-        svc.setWorkspaceSkillsEnabled(
-          makeCtx(),
-          ['missing', 'hidden', 'inactive'],
-          false,
-        ),
+        svc.setWorkspaceSkillsEnabled(makeCtx(), ['hidden', 'inactive'], false),
       ).resolves.toMatchObject({
         activation: 'applied',
         sessionsRefreshed: 0,
         sessionsFailed: 0,
         results: [],
         errors: [
-          { skillName: 'missing', code: 'skill_not_found' },
           { skillName: 'hidden', code: 'skill_not_toggleable' },
           { skillName: 'inactive', code: 'skill_inactive_extension' },
         ],

packages/cli/src/serve/workspace-service/index.ts

@@ -959,10 +959,12 @@ export function createDaemonWorkspaceService(
       for (const requestedName of requestedSkillNames) {
         const normalizedName = requestedName.trim().toLowerCase();
         const skill = skillsByName.get(normalizedName);
-        let domainError: unknown;
         if (!skill) {
-          domainError = new WorkspaceSkillNotFoundError(requestedName);
-        } else if (skill.userInvocable === false) {
+          targets.push({ requestedName, skillName: requestedName });
+          continue;
+        }
+        let domainError: unknown;
+        if (skill.userInvocable === false) {
           domainError = new WorkspaceSkillNotToggleableError(
             skill.name,
             'not_user_invocable',
@@ -990,7 +992,7 @@ export function createDaemonWorkspaceService(
           if (!error) throw domainError;
           targets.push({ requestedName, error });
         } else {
-          targets.push({ requestedName, skillName: skill!.name });
+          targets.push({ requestedName, skillName: skill.name });
         }
       }