From 5a3d0008f69ebef8c9dc829bbf415d5ac7b64c6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Wed, 26 Aug 2026 11:57:41 +0800 Subject: [PATCH 01/10] fix(serve): persist skill toggles without catalog validation --- .../2026-07-20-skills-default-disabled.md | 10 +- docs/design/daemon-skill-batch-toggle.md | 37 +- docs/design/daemon-skill-toggle.md | 24 +- .../developers/daemon/13-sdk-daemon-client.md | 4 +- docs/developers/qwen-serve-protocol.md | 21 +- docs/users/qwen-serve.md | 2 +- .../src/serve/routes/workspace-skills.test.ts | 2 +- packages/cli/src/serve/run-qwen-serve.test.ts | 49 ++- packages/cli/src/serve/run-qwen-serve.ts | 21 -- packages/cli/src/serve/server.test.ts | 75 ++-- .../__tests__/facade.test.ts | 321 +++++++++++------- .../cli/src/serve/workspace-service/index.ts | 123 +------ .../sdk-typescript/src/daemon/DaemonClient.ts | 4 +- packages/sdk-typescript/src/daemon/types.ts | 1 + 14 files changed, 353 insertions(+), 341 deletions(-) diff --git a/docs/design/2026-07-20-skills-default-disabled.md b/docs/design/2026-07-20-skills-default-disabled.md index 353ce5bd4b9..89c5a21e098 100644 --- a/docs/design/2026-07-20-skills-default-disabled.md +++ b/docs/design/2026-07-20-skills-default-disabled.md @@ -20,11 +20,13 @@ Effective disables are `disabled + (defaultDisabled - enabled)`. An explicit `en One CLI-local resolver computes the effective disabled names and whether each disabled skill is `hard` or `default`. Existing runtime consumers continue reading the effective set through `Config.getDisabledSkillNames()`; core skill discovery and execution APIs do not change. -The `/skills` picker and daemon toggle apply the same rules: +The `/skills` picker continues to operate on discovered Skills. Daemon toggle +routes instead persist settings by requested name without consulting that +catalog: -- enabling removes a workspace hard disable and adds the canonical name to workspace `skills.enabled` only when needed; -- disabling removes the workspace opt-in and adds the canonical name to workspace `skills.disabled`; -- higher-scope `skills.disabled` entries remain locked; +- enabling removes a workspace hard disable and adds the requested name to workspace `skills.enabled` only when needed; +- disabling removes the workspace opt-in and adds the requested name to workspace `skills.disabled`; +- higher-scope `skills.disabled` entries remain authoritative for effective availability but do not block workspace scope from recording or removing its own declaration; - unrelated and unavailable skill entries are preserved. Workspace skill status adds a disable reason and optional lock scope so clients can distinguish a hard lock from an overridable default. The daemon-local and ACP status paths both read the same CLI-local resolver. diff --git a/docs/design/daemon-skill-batch-toggle.md b/docs/design/daemon-skill-batch-toggle.md index ab2c3151865..7f86405e14c 100644 --- a/docs/design/daemon-skill-batch-toggle.md +++ b/docs/design/daemon-skill-batch-toggle.md @@ -2,9 +2,10 @@ ## Problem -Remote Skill managers can toggle only one Skill per request. Closing several -Skills therefore requires client-side request orchestration and provides no -single response that records all target outcomes. +Remote Skill managers need both single and batch mutations to behave like +workspace settings writes. A runtime Skill snapshot is not an ownership source +for `skills.disabled` or `skills.enabled`: entries may be declared before +installation and may intentionally outlive the currently loaded catalog. ## API @@ -24,16 +25,15 @@ 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: 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. +The daemon does not read or validate against runtime Skill status. Every name +is persisted in one locked write, and changes are applied with one live-session +refresh. 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`. Unknown, non-user-invocable, +inactive-Extension, and higher-scope-disabled names use the same settings path. +Higher scopes still determine effective availability after settings merge, but +do not prevent the workspace scope from recording its own declaration. +Unexpected persistence and runtime-generation failures fail the whole request. ```json { @@ -62,9 +62,8 @@ request. } ``` -`results` and `errors` each preserve request order within their own array; -the response does not reconstruct the original mixed ordering, so clients -re-match targets by `skillName`. +`results` preserves request order. `errors` remains present for wire +compatibility and is empty for structurally valid names. Malformed requests still fail as a whole with HTTP 400. Workspace trust, authentication, client identity, and generation ownership use the same gates @@ -74,7 +73,9 @@ as the single-Skill route. Advertise `workspace_skill_batch_toggle` separately from `workspace_skill_toggle`. Clients must pre-flight the new capability before -calling the collection route. The existing single-Skill route and response -remain unchanged. The collection routes are HTTP-only: the ACP +calling the collection route. The single-Skill route now follows the same +settings-only contract and returns the trimmed request name because there is no +catalog lookup from which to obtain a canonical spelling. The collection +routes are HTTP-only: the ACP `_qwen/workspace/skills` dispatch surface stays read-only, matching the single-Skill toggle. diff --git a/docs/design/daemon-skill-toggle.md b/docs/design/daemon-skill-toggle.md index a22c2531bb4..4ebcff33e16 100644 --- a/docs/design/daemon-skill-toggle.md +++ b/docs/design/daemon-skill-toggle.md @@ -2,7 +2,7 @@ ## Goal -Expose the CLI `/skills` panel's workspace enable/disable behavior through daemon REST and the TypeScript SDK, including immediate refresh of active ACP sessions. +Expose workspace Skill settings writes through daemon REST and the TypeScript SDK, including immediate refresh of active ACP sessions without making the runtime Skill catalog an ownership source. ## Public contract @@ -12,21 +12,15 @@ Expose the CLI `/skills` panel's workspace enable/disable behavior through daemo - SDK: `DaemonClient.setWorkspaceSkillEnabled` and `WorkspaceDaemonClient.setWorkspaceSkillEnabled` - Capability: `workspace_skill_toggle` -The response contains the canonical skill name, requested state, whether persistence changed, activation state, and session refresh counts. `applied` means every active session refreshed, `deferred` means no ACP child was running, and `partial` means at least one session failed to refresh after persistence committed. +The response contains the trimmed requested name, requested state, whether persistence changed, activation state, and session refresh counts. `applied` means every active session refreshed, `deferred` means no ACP child was running, and `partial` means at least one session failed to refresh after persistence committed. ## Semantics -The API changes workspace `skills.disabled` and `skills.enabled` as needed. Skill lookup is case-insensitive, but the canonical discovered name is persisted. Enabling a default-disabled skill writes an explicit opt-in; disabling it removes the opt-in and writes a hard workspace disable. Updating one target removes target duplicates and case variants without deleting orphan entries for unavailable skills. A second identical request is a no-op. +The API changes workspace `skills.disabled` and `skills.enabled` by name without consulting the runtime Skill catalog. Enabling a default-disabled Skill writes an explicit opt-in; disabling it removes the opt-in and writes a hard workspace disable. Updating one target removes target duplicates and case variants without deleting orphan entries for unavailable Skills. Names may be configured before installation, while hidden from user invocation, or while their Extension is inactive. A second identical request is a no-op. -The route rejects states the CLI panel cannot toggle: +Higher-scope settings still determine effective availability after settings merge, but do not prevent workspace scope from recording or removing its own declaration. The route retains request-shape, authentication, client identity, workspace trust, and runtime-generation gates; none of those require a Skill catalog lookup. -- unknown skill: `404 skill_not_found`; -- `userInvocable === false`: `409 skill_not_toggleable`; -- skill from an inactive extension: `409 skill_not_toggleable`; -- disabled in system defaults, user, or system scope: `409 skill_not_toggleable` with the locking scope; -- untrusted workspace: `403 untrusted_workspace`. - -The scope lock check and workspace read-modify-write happen inside the daemon's per-workspace settings lock. A failed write stops before refresh and event publication. +The workspace read-modify-write happens inside the daemon's per-workspace settings lock. A failed write stops before refresh and event publication. ## Skill availability versus `disable-model-invocation` @@ -36,8 +30,8 @@ The scope lock check and workspace read-modify-write happen inside the daemon's ## Activation flow -1. Resolve the canonical, toggleable skill from the workspace status snapshot. -2. Under the workspace settings lock, re-read every scope, reject higher-scope locks, and commit the canonical workspace list. +1. Validate the request name, authorization, workspace trust, client identity, and runtime generation. +2. Under the workspace settings lock, re-read every scope and commit the requested name to the workspace list. 3. Invalidate the daemon's cached skill status. 4. If an ACP child is live, invoke `qwen/control/workspace/skills/refresh`. 5. The child reloads workspace-scope settings and refreshes every active session, including busy sessions. @@ -53,9 +47,9 @@ An in-flight model request cannot be rewritten. Subsequent skill execution check - Slash commands: available-command construction removes disabled skills and sends updated command metadata to daemon clients. - Model context: SkillManager change listeners refresh the Skill tool description and available-skill context. - Execution validation: the Skill tool re-reads the disabled-name provider before invocation, so later calls are rejected immediately. -- Extension state: inactive extension skills remain non-toggleable even when they are not disabled by settings. +- Extension state: inactive Extensions still keep their Skills unavailable at runtime, independently of whether workspace settings record those names. - Daemon cache: the cached live-child skill snapshot is invalidated after persistence so later GET requests cannot replay stale state. -- SDK consumers: both primary-workspace and workspace-qualified clients share the response and error contract. +- SDK consumers: both primary-workspace and workspace-qualified clients share the settings-only response contract. - Events: existing `settings_changed` consumers observe each committed `skills.disabled` or `skills.enabled` value; there is no new event type. ## Failure behavior diff --git a/docs/developers/daemon/13-sdk-daemon-client.md b/docs/developers/daemon/13-sdk-daemon-client.md index e7b47726c0c..5bc1c9db0b6 100644 --- a/docs/developers/daemon/13-sdk-daemon-client.md +++ b/docs/developers/daemon/13-sdk-daemon-client.md @@ -152,7 +152,7 @@ await client .setWorkspaceSkillEnabled('review', true, { clientId: 'dashboard-1' }); ``` -Pre-flight `capabilities.features.includes('workspace_skill_toggle')`. The typed `DaemonSkillToggleResult` reports the canonical `skillName`, whether disk state `changed`, activation state (`applied`, `deferred`, or `partial`), and refreshed/failed session counts. `DaemonWorkspaceSkillStatus.userInvocable` is an optional false-only field; absence means the skill is user-invocable. +Pre-flight `capabilities.features.includes('workspace_skill_toggle')`. The typed `DaemonSkillToggleResult` reports the trimmed requested `skillName`, whether disk state `changed`, activation state (`applied`, `deferred`, or `partial`), and refreshed/failed session counts. The write is settings-only and does not require the name to appear in `DaemonWorkspaceSkillStatus`; that status type's optional false-only `userInvocable` field remains useful for rendering the live catalog but does not gate persistence. For batch changes, pre-flight `workspace_skill_batch_toggle` and call either client shape with the same contract: @@ -165,7 +165,7 @@ await client .setWorkspaceSkillsEnabled(['review', 'deploy'], true); ``` -`DaemonSkillBatchToggleResult` contains ordered successful `results`, per-target `errors`, and batch-level activation/session-refresh counts. The daemon persists valid targets together and refreshes active sessions once; one expected target error does not block other valid targets. The method throws only on a non-200 response; a 200 does not mean every target was applied, so always inspect `errors` before treating the batch as successful. +`DaemonSkillBatchToggleResult` contains ordered `results`, a compatibility `errors` array, and batch-level activation/session-refresh counts. Current daemons persist every structurally valid name together, refresh active sessions once, and return an empty `errors` array without consulting the loaded Skill catalog. The error item types remain available so the SDK can still decode responses from older daemons. The method throws on a non-200 response. V2 Extension batch activation retains the asynchronous Extension operation model. Pre-flight `extension_batch_activation_v2`, submit a global default batch or a selected-workspace override batch, then poll it with the existing operation helper: diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index d786d3ae7b8..0fbab4a1ec3 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -2817,7 +2817,7 @@ SSE event (workspace-scoped): `tool_toggled` with `{toolName, enabled, originato Capability tag: `workspace_skill_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/:name/enable`. -Toggle a loaded, user-invocable skill through the workspace skill settings, matching the CLI `/skills` panel's Space-key behavior. Lookup is case-insensitive, while persistence and the response use the skill's canonical name. Enabling a `skills.defaultDisabled` skill adds a workspace `skills.enabled` opt-in; disabling removes that opt-in and adds a workspace `skills.disabled` entry. Existing entries for skills that are no longer loaded are preserved, and duplicate/case-variant entries for the target are collapsed. A hard-disable entry inherited from system defaults, user, or system scope locks the skill: workspace scope cannot override it. +Update the workspace Skill settings for a name without consulting the loaded Skill catalog. The trimmed request name is passed to persistence and returned in the response. Enabling a `skills.defaultDisabled` Skill adds a workspace `skills.enabled` opt-in; disabling removes that opt-in and adds a workspace `skills.disabled` entry. Existing entries for Skills that are no longer loaded are preserved, and duplicate/case-variant entries for the target are collapsed. Higher-scope settings remain authoritative for effective availability, but they do not prevent the workspace scope from recording or removing its own declaration. This is different from the ACP `qwen/skills/setEnabled` managed-skill operation and the `disable-model-invocation` frontmatter field. Effective skill availability follows `skills.disabled` > `skills.enabled` > `skills.defaultDisabled`. Both hard and default disables remove the skill from slash-command/model availability and reject later skill execution. `disable-model-invocation: true` keeps direct user invocation available and only hides the skill from model invocation. @@ -2847,16 +2847,14 @@ Errors: - `400 {code: 'invalid_skill_name'}` — empty path parameter, or more than 256 characters. - `400 {code: 'invalid_enabled_flag'}` — `enabled` missing or non-boolean. - `403 {code: 'untrusted_workspace'}` — the selected workspace is not trusted. -- `404 {code: 'skill_not_found'}` — no loaded skill matches the name. -- `409 {code: 'skill_not_toggleable', reason: 'not_user_invocable' | 'inactive_extension' | 'locked', lockedScope?: 'system' | 'user' | 'systemDefaults'}` — the CLI panel would not allow the target to be toggled. `lockedScope` is present only when `reason` is `locked`. -The mutation reuses the workspace-scoped `settings_changed` event for each changed key (`skills.disabled` and/or `skills.enabled`); it does not add a new event type. Each of those events includes the same `mutation` object: `{ id, kind: 'skill_toggle', skills: [{ name, enabled }], activation, sessionsRefreshed, sessionsFailed }`. `id` correlates every settings event produced by one toggle request. `skills` lists the canonical names and resulting enabled states of Skills that actually changed. Workspace skill status cells include optional `disabledReason: 'hard' | 'default' | 'inactive_extension'` and `lockedScope: 'system' | 'user' | 'systemDefaults'` fields. +The mutation reuses the workspace-scoped `settings_changed` event for each changed key (`skills.disabled` and/or `skills.enabled`); it does not add a new event type. Each of those events includes the same `mutation` object: `{ id, kind: 'skill_toggle', skills: [{ name, enabled }], activation, sessionsRefreshed, sessionsFailed }`. `id` correlates every settings event produced by one toggle request. `skills` lists the requested names and resulting enabled states of Skills that actually changed. Workspace skill status cells include optional `disabledReason: 'hard' | 'default' | 'inactive_extension'` and `lockedScope: 'system' | 'user' | 'systemDefaults'` fields. #### `POST /workspace/skills/enable` Capability tag: `workspace_skill_batch_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/enable`. -Toggle up to 100 loaded Skills in one request; the cap counts the raw `skillNames` entries before deduplication. Names are trimmed and deduplicated case-insensitively while preserving first-seen order. The daemon validates against one Skill status snapshot, persists all valid changes in one locked settings write, and refreshes active sessions once. Processing is best-effort for expected target errors: an unknown, hidden, inactive-extension, or locked target is recorded in `errors` without preventing other valid targets from being applied. Unexpected persistence or runtime-generation failures still fail the whole request. +Update workspace Skill settings for up to 100 names in one request; the cap counts the raw `skillNames` entries before deduplication. Names are trimmed and deduplicated case-insensitively while preserving first-seen order and casing. The daemon does not consult the loaded Skill catalog. It persists all names in one locked settings write and refreshes active sessions once. Unexpected persistence or runtime-generation failures fail the whole request. Request: @@ -2885,19 +2883,18 @@ Response (200): "skillName": "deploy", "enabled": false, "changed": true - } - ], - "errors": [ + }, { "skillName": "missing", - "code": "skill_not_found", - "error": "Skill not found: missing" + "enabled": false, + "changed": true } - ] + ], + "errors": [] } ``` -Target errors use `skill_not_found`, `skill_not_toggleable`, or `skill_inactive_extension`. Malformed requests return HTTP 400 with `invalid_skill_names`, `invalid_skill_name`, or `invalid_enabled_flag`. Authentication, workspace trust, client identity, unexpected persistence failures, and runtime-generation failures fail the whole request through the standard route gates. Batch-level `activation`, `sessionsRefreshed`, and `sessionsFailed` describe the single live-session refresh shared by all changed results. `activation` reports the refresh attempt rather than the outcome: a batch in which no target changed (for example, every target errored) still answers `applied` when a session is live, matching the single-Skill no-op response, so derive what actually changed from each result's `changed` flag and the `errors` array. When at least one target changes, the daemon emits the same `settings_changed` mutation metadata as the single-Skill route; every `skills.disabled` / `skills.enabled` event from that request shares one `mutation.id`. +Malformed requests return HTTP 400 with `invalid_skill_names`, `invalid_skill_name`, or `invalid_enabled_flag`. Authentication, workspace trust, client identity, unexpected persistence failures, and runtime-generation failures fail the whole request through the standard route gates. `errors` remains in the response for wire compatibility and is empty for structurally valid names. Batch-level `activation`, `sessionsRefreshed`, and `sessionsFailed` describe the single live-session refresh shared by all changed results. `activation` reports the refresh attempt rather than the outcome: a batch in which no target changed still answers `applied` when a session is live, matching the single-Skill no-op response, so derive what actually changed from each result's `changed` flag. When at least one target changes, the daemon emits the same `settings_changed` mutation metadata as the single-Skill route; every `skills.disabled` / `skills.enabled` event from that request shares one `mutation.id`. #### `POST /workspace/init` diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 7a492021f39..4face952f88 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -209,7 +209,7 @@ idle daemon returns `initialized: false` with an empty snapshot. Once a session is alive they switch to `initialized: true` and surface the real state. -To mirror the CLI `/skills` panel remotely, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_toggle` capability. To change several Skills, check `workspace_skill_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; its response separates successful `results` from per-target `errors`, persists valid targets together, and refreshes active ACP sessions once. The routes update workspace `skills.disabled` and `skills.enabled` as needed and reject unknown, hidden, inactive-extension, higher-scope-locked, and untrusted targets. Enabling a `skills.defaultDisabled` skill writes a canonical opt-in to `skills.enabled`; a hard `skills.disabled` entry inherited from a higher scope still cannot be overridden. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. A `deferred` response means the setting was saved while no ACP child was running; it will apply when the child starts. `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. +To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_toggle` capability. To change several names, check `workspace_skill_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it persists every structurally valid name together and refreshes active ACP sessions once. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be configured before installation or while their Extension is inactive. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. Enabling a `skills.defaultDisabled` Skill writes an explicit opt-in to `skills.enabled`. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means the setting was saved while no ACP child was running; it will apply when the child starts. `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. `GET /workspace/env` and `GET /workspace/preflight` always answer with `initialized: true` regardless of ACP state. `env` never consults ACP diff --git a/packages/cli/src/serve/routes/workspace-skills.test.ts b/packages/cli/src/serve/routes/workspace-skills.test.ts index f00d7af910d..ccfc0d3f604 100644 --- a/packages/cli/src/serve/routes/workspace-skills.test.ts +++ b/packages/cli/src/serve/routes/workspace-skills.test.ts @@ -173,7 +173,7 @@ describe('workspace Skill management routes', () => { expect(harness.deleteWorkspaceSkill).not.toHaveBeenCalled(); }); - it('toggles a deduplicated Skill batch and returns per-target outcomes', async () => { + it('forwards a deduplicated Skill batch response with legacy errors', async () => { const harness = createHarness(); harness.setWorkspaceSkillsEnabled.mockResolvedValueOnce({ enabled: false, diff --git a/packages/cli/src/serve/run-qwen-serve.test.ts b/packages/cli/src/serve/run-qwen-serve.test.ts index aa6c1452075..37b087d2269 100644 --- a/packages/cli/src/serve/run-qwen-serve.test.ts +++ b/packages/cli/src/serve/run-qwen-serve.test.ts @@ -722,7 +722,7 @@ describe('workspace skill settings persistence', () => { vi.restoreAllMocks(); }); - it('canonicalizes, deduplicates, preserves orphans, serializes updates, and enforces user locks', async () => { + it('canonicalizes, deduplicates, preserves orphans, and serializes updates across settings scopes', async () => { workspace = fs.realpathSync( fs.mkdtempSync(path.join(os.tmpdir(), 'qws-skill-settings-')), ); @@ -815,9 +815,34 @@ describe('workspace skill settings persistence', () => { ) as { skills: { disabled: string[]; enabled: string[] } }; expect(saved.skills.disabled).toEqual(['orphan', 'alpha', 'beta']); expect(saved.skills.enabled).toEqual(['opt-in-skill']); + await expect( + persistDisabledSkills!(workspace, 'locked-skill', false), + ).resolves.toEqual({ + changed: true, + disabled: ['orphan', 'alpha', 'beta', 'locked-skill'], + settingsChanges: [ + { + key: 'skills.disabled', + value: ['orphan', 'alpha', 'beta', 'locked-skill'], + }, + ], + }); await expect( persistDisabledSkills!(workspace, 'locked-skill', true), - ).rejects.toMatchObject({ reason: 'locked', lockedScope: 'user' }); + ).resolves.toEqual({ + changed: true, + disabled: ['orphan', 'alpha', 'beta'], + settingsChanges: [ + { + key: 'skills.disabled', + value: ['orphan', 'alpha', 'beta'], + }, + ], + }); + const savedUser = JSON.parse( + fs.readFileSync(path.join(qwenHome, 'settings.json'), 'utf8'), + ) as { skills: { disabled: string[] } }; + expect(savedUser.skills.disabled).toEqual(['locked-skill']); }); it('produces both skills.disabled and skills.enabled changes when enabling a workspace-hard-disabled default-disabled skill', async () => { @@ -911,7 +936,7 @@ describe('workspace skill settings persistence', () => { expect(setValue.mock.calls[0]?.[3]).toBe(toolGuard); }); - it('persists a Skill batch with one settings write and per-target lock outcomes', async () => { + it('persists a Skill batch with one settings write across settings scopes', async () => { workspace = fs.realpathSync( fs.mkdtempSync(path.join(os.tmpdir(), 'qws-skill-batch-')), ); @@ -978,14 +1003,14 @@ describe('workspace skill settings persistence', () => { skillName: 'alpha', changed: true, }); - expect(result.outcomes[2]).toMatchObject({ + expect(result.outcomes[2]).toEqual({ skillName: 'locked-skill', - error: { reason: 'locked', lockedScope: 'user' }, + changed: true, }); expect(result.settingsChanges).toEqual([ { key: 'skills.disabled', - value: ['orphan', 'review', 'alpha'], + value: ['orphan', 'review', 'alpha', 'locked-skill'], }, ]); expect(setValues).toHaveBeenCalledOnce(); @@ -1008,6 +1033,7 @@ describe('workspace skill settings persistence', () => { 'orphan', 'review', 'alpha', + 'locked-skill', ]); expect(savedAfterDisable.skills.enabled).toBeUndefined(); const savedUser = JSON.parse( @@ -1036,7 +1062,10 @@ describe('workspace skill settings persistence', () => { { skillName: 'orphan', changed: true }, ]); expect(preinstallEnable.settingsChanges).toEqual([ - { key: 'skills.disabled', value: ['review', 'alpha'] }, + { + key: 'skills.disabled', + value: ['review', 'alpha', 'locked-skill'], + }, ]); expect(setValues).toHaveBeenCalledTimes(2); @@ -1060,7 +1089,11 @@ describe('workspace skill settings persistence', () => { const savedAfterEnable = JSON.parse( fs.readFileSync(path.join(workspace, '.qwen', 'settings.json'), 'utf8'), ) as { skills: { disabled: string[]; enabled: string[] } }; - expect(savedAfterEnable.skills.disabled).toEqual(['review', 'alpha']); + expect(savedAfterEnable.skills.disabled).toEqual([ + 'review', + 'alpha', + 'locked-skill', + ]); expect(savedAfterEnable.skills.enabled).toEqual(['opt-in']); const guard = vi.fn(); diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index 891c6b6006b..bd17a814a00 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -4978,15 +4978,6 @@ async function runQwenServeImpl( const fresh = loadSettingsForPersistence(workspace); const normalizedName = skillName.trim().toLowerCase(); const resolved = resolveSkillSettings(fresh); - const disablement = resolved.disablements.get(normalizedName); - if (disablement?.reason === 'hard' && disablement.lockedScope) { - throw new runtime.WorkspaceSkillNotToggleableError( - skillName, - 'locked', - disablement.lockedScope, - ); - } - const workspaceDisabled = skillSettingStrings( fresh, WORKSPACE_SETTING_SCOPE, @@ -5071,18 +5062,6 @@ async function runQwenServeImpl( for (const skillName of skillNames) { const normalizedName = skillName.trim().toLowerCase(); - const disablement = resolved.disablements.get(normalizedName); - if (disablement?.reason === 'hard' && disablement.lockedScope) { - outcomes.push({ - skillName, - error: new runtime.WorkspaceSkillNotToggleableError( - skillName, - 'locked', - disablement.lockedScope, - ), - }); - continue; - } const updated = updateWorkspaceSkillSettingLists( next, skillName, diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index ed934cbef4d..4c695d5aefe 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -22949,7 +22949,7 @@ describe('createServeApp', () => { expect(badBody.body.code).toBe('invalid_enabled_flag'); }); - it('returns the canonical name and deferred activation without a child', async () => { + it('returns the requested name and deferred activation without a child', async () => { const bridge = fakeBridge({ workspaceSkillsImpl: async () => ({ v: 1, @@ -22974,7 +22974,7 @@ describe('createServeApp', () => { expect(res.status).toBe(200); expect(res.body).toEqual({ - skillName: 'review', + skillName: 'ReViEw', enabled: false, changed: true, activation: 'deferred', @@ -22983,14 +22983,17 @@ describe('createServeApp', () => { }); expect(persistDisabledSkills).toHaveBeenCalledWith( WS_BOUND, - 'review', + 'ReViEw', false, undefined, ); }); - it('returns 404 for an unknown skill', async () => { - const persistDisabledSkills = vi.fn(); + it('persists an unknown Skill name without catalog validation', async () => { + const persistDisabledSkills = vi.fn().mockResolvedValue({ + changed: true, + disabled: ['missing'], + }); const app = createServeApp(tokenOpts, undefined, { bridge: fakeBridge({ workspaceSkillsImpl: async () => ({ @@ -23000,15 +23003,25 @@ describe('createServeApp', () => { skills: [reviewSkill], }), }), + boundWorkspace: WS_BOUND, persistDisabledSkills, primaryWorkspaceTrusted: true, }); const res = await auth( request(app).post('/workspace/skills/missing/enable'), ).send({ enabled: false }); - expect(res.status).toBe(404); - expect(res.body.code).toBe('skill_not_found'); - expect(persistDisabledSkills).not.toHaveBeenCalled(); + expect(res.status).toBe(200); + expect(res.body).toMatchObject({ + skillName: 'missing', + enabled: false, + changed: true, + }); + expect(persistDisabledSkills).toHaveBeenCalledWith( + WS_BOUND, + 'missing', + false, + undefined, + ); }); it('rejects an unknown workspace client id before persistence', async () => { @@ -23028,8 +23041,11 @@ describe('createServeApp', () => { expect(persistDisabledSkills).not.toHaveBeenCalled(); }); - it('returns 409 without persisting a non-user-invocable skill', async () => { - const persistDisabledSkills = vi.fn(); + it('persists a non-user-invocable Skill name without catalog validation', async () => { + const persistDisabledSkills = vi.fn().mockResolvedValue({ + changed: true, + disabled: ['review'], + }); const app = createServeApp(tokenOpts, undefined, { bridge: fakeBridge({ workspaceSkillsImpl: async () => ({ @@ -23039,22 +23055,32 @@ describe('createServeApp', () => { skills: [{ ...reviewSkill, userInvocable: false }], }), }), + boundWorkspace: WS_BOUND, persistDisabledSkills, primaryWorkspaceTrusted: true, }); const res = await auth( request(app).post('/workspace/skills/review/enable'), ).send({ enabled: false }); - expect(res.status).toBe(409); + expect(res.status).toBe(200); expect(res.body).toMatchObject({ - code: 'skill_not_toggleable', - reason: 'not_user_invocable', + skillName: 'review', + enabled: false, + changed: true, }); - expect(persistDisabledSkills).not.toHaveBeenCalled(); + expect(persistDisabledSkills).toHaveBeenCalledWith( + WS_BOUND, + 'review', + false, + undefined, + ); }); - it('returns a dedicated code for an inactive extension skill', async () => { - const persistDisabledSkills = vi.fn(); + it('persists an inactive Extension Skill name without catalog validation', async () => { + const persistDisabledSkills = vi.fn().mockResolvedValue({ + changed: false, + disabled: [], + }); const app = createServeApp(tokenOpts, undefined, { bridge: fakeBridge({ workspaceSkillsImpl: async () => ({ @@ -23066,6 +23092,7 @@ describe('createServeApp', () => { ], }), }), + boundWorkspace: WS_BOUND, persistDisabledSkills, primaryWorkspaceTrusted: true, }); @@ -23073,15 +23100,21 @@ describe('createServeApp', () => { request(app).post('/workspace/skills/review/enable'), ).send({ enabled: true }); - expect(res.status).toBe(409); + expect(res.status).toBe(200); expect(res.body).toMatchObject({ - code: 'skill_inactive_extension', - reason: 'inactive_extension', + skillName: 'review', + enabled: true, + changed: false, }); - expect(persistDisabledSkills).not.toHaveBeenCalled(); + expect(persistDisabledSkills).toHaveBeenCalledWith( + WS_BOUND, + 'review', + true, + undefined, + ); }); - it('returns the locked scope from persistence validation', async () => { + it('passes through a legacy persistence lock error', async () => { const app = createServeApp(tokenOpts, undefined, { bridge: fakeBridge({ workspaceSkillsImpl: async () => ({ diff --git a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts index 15b30add3e6..4a2e8974eec 100644 --- a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts +++ b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts @@ -115,7 +115,6 @@ import { import { WorkspaceVoiceError } from '../../../services/voice-service.js'; import { WorkspacePermissionRulesSessionRequiredError, - WorkspaceSkillNotFoundError, WorkspaceSkillNotToggleableError, WorkspaceSettingsPartialPersistError, } from '../types.js'; @@ -1631,7 +1630,7 @@ describe('createDaemonWorkspaceService', () => { skills: [skill], }); - it('uses the canonical skill name and refreshes every active session', async () => { + it('uses the requested skill name and refreshes every active session', async () => { const invalidate = vi.fn(); const workspaceSkillsStatusProvider = Object.assign(vi.fn(), { invalidate, @@ -1664,7 +1663,7 @@ describe('createDaemonWorkspaceService', () => { expect(persistDisabledSkills).toHaveBeenCalledWith( '/workspace', - 'review', + 'ReViEw', false, undefined, ); @@ -1674,7 +1673,7 @@ describe('createDaemonWorkspaceService', () => { { cwd: '/workspace', reason: 'settings' }, ); expect(result).toEqual({ - skillName: 'review', + skillName: 'ReViEw', enabled: false, changed: true, activation: 'applied', @@ -1685,7 +1684,7 @@ describe('createDaemonWorkspaceService', () => { skillToggleSettingsChanged({ key: 'skills.disabled', value: ['review'], - skills: [{ name: 'review', enabled: false }], + skills: [{ name: 'ReViEw', enabled: false }], activation: 'applied', sessionsRefreshed: 2, sessionsFailed: 0, @@ -1755,7 +1754,6 @@ describe('createDaemonWorkspaceService', () => { const queryWorkspaceStatus = vi .fn() .mockResolvedValueOnce(oldStatus) - .mockResolvedValueOnce(oldStatus) .mockResolvedValueOnce(newStatus); const invokeWorkspaceCommand = vi.fn( () => refresh.promise, @@ -1786,7 +1784,7 @@ describe('createDaemonWorkspaceService', () => { await expect(svc.getWorkspaceSkillsStatus(makeCtx())).resolves.toEqual( newStatus, ); - expect(queryWorkspaceStatus).toHaveBeenCalledTimes(3); + expect(queryWorkspaceStatus).toHaveBeenCalledTimes(2); }); it('publishes an explicit enabled override for a default-disabled skill', async () => { @@ -1981,76 +1979,97 @@ describe('createDaemonWorkspaceService', () => { expect(publishWorkspaceEvent).not.toHaveBeenCalled(); }); - it('rejects unknown, hidden, and inactive extension skills before persisting', async () => { - const persistDisabledSkills = vi.fn(); - const unknown = createDaemonWorkspaceService( + it('persists by requested name without reading the runtime Skill catalog', async () => { + const queryWorkspaceStatus = vi + .fn() + .mockRejectedValue(new Error('runtime unavailable')); + const persistDisabledSkills = vi.fn().mockResolvedValue({ + changed: true, + disabled: ['future-skill'], + }); + const svc = createDaemonWorkspaceService( makeDeps({ - queryWorkspaceStatus: statusQuery(), + queryWorkspaceStatus, persistDisabledSkills, + isChannelLive: () => false, }), ); - await expect( - unknown.setWorkspaceSkillEnabled(makeCtx(), 'missing', false), - ).rejects.toBeInstanceOf(WorkspaceSkillNotFoundError); - const hidden = createDaemonWorkspaceService( - makeDeps({ - queryWorkspaceStatus: statusQuery( - skillStatus({ userInvocable: false }), - ), - persistDisabledSkills, - }), - ); await expect( - hidden.setWorkspaceSkillEnabled(makeCtx(), 'review', false), - ).rejects.toMatchObject({ - reason: 'not_user_invocable', + svc.setWorkspaceSkillEnabled(makeCtx(), 'future-skill', false), + ).resolves.toMatchObject({ + skillName: 'future-skill', + enabled: false, + changed: true, }); + expect(queryWorkspaceStatus).not.toHaveBeenCalled(); + expect(persistDisabledSkills).toHaveBeenCalledWith( + '/workspace', + 'future-skill', + false, + undefined, + ); + }); - const inactive = createDaemonWorkspaceService( - makeDeps({ - queryWorkspaceStatus: statusQuery( - skillStatus({ - status: 'disabled', - disabledReason: 'inactive_extension', - level: 'extension', - extensionName: 'review-ext', - }), - ), - persistDisabledSkills, - }), + it('does not validate unknown, hidden, or inactive Extension names', async () => { + const queryWorkspaceStatus = vi.fn(); + const persistDisabledSkills = vi.fn().mockResolvedValue({ + changed: false, + disabled: [], + }); + const svc = createDaemonWorkspaceService( + makeDeps({ queryWorkspaceStatus, persistDisabledSkills }), ); + await expect( - inactive.setWorkspaceSkillEnabled(makeCtx(), 'review', true), - ).rejects.toMatchObject({ - reason: 'inactive_extension', - }); - expect(persistDisabledSkills).not.toHaveBeenCalled(); + svc.setWorkspaceSkillEnabled(makeCtx(), 'missing', false), + ).resolves.toMatchObject({ skillName: 'missing', enabled: false }); + await expect( + svc.setWorkspaceSkillEnabled(makeCtx(), 'hidden', false), + ).resolves.toMatchObject({ skillName: 'hidden', enabled: false }); + await expect( + svc.setWorkspaceSkillEnabled(makeCtx(), 'inactive', true), + ).resolves.toMatchObject({ skillName: 'inactive', enabled: true }); + + expect(queryWorkspaceStatus).not.toHaveBeenCalled(); + expect(persistDisabledSkills.mock.calls).toEqual([ + ['/workspace', 'missing', false, undefined], + ['/workspace', 'hidden', false, undefined], + ['/workspace', 'inactive', true, undefined], + ]); }); - it('rejects a legacy inactive extension skill with no disabledReason and not disabled by settings', async () => { + it('does not infer a legacy inactive Extension from runtime status', async () => { await withIsolatedWorkspace(async ({ workspace }) => { - const persistDisabledSkills = vi.fn(); + const queryWorkspaceStatus = statusQuery( + skillStatus({ + status: 'disabled', + disabledReason: undefined, + level: 'extension', + extensionName: 'review-ext', + }), + ); + const persistDisabledSkills = vi.fn().mockResolvedValue({ + changed: false, + disabled: [], + }); const svc = createDaemonWorkspaceService( makeDeps({ boundWorkspace: workspace, - queryWorkspaceStatus: statusQuery( - skillStatus({ - status: 'disabled', - disabledReason: undefined, - level: 'extension', - extensionName: 'review-ext', - }), - ), + queryWorkspaceStatus, persistDisabledSkills, }), ); await expect( svc.setWorkspaceSkillEnabled(makeCtx(), 'review', true), - ).rejects.toMatchObject({ - reason: 'inactive_extension', - }); - expect(persistDisabledSkills).not.toHaveBeenCalled(); + ).resolves.toMatchObject({ skillName: 'review', enabled: true }); + expect(queryWorkspaceStatus).not.toHaveBeenCalled(); + expect(persistDisabledSkills).toHaveBeenCalledWith( + workspace, + 'review', + true, + undefined, + ); }); }); @@ -2208,6 +2227,45 @@ describe('createDaemonWorkspaceService', () => { ); }); + it('persists every requested name without reading the runtime Skill catalog', async () => { + const queryWorkspaceStatus = vi + .fn() + .mockRejectedValue(new Error('runtime unavailable')); + const persistDisabledSkillsBatch = vi.fn().mockResolvedValue({ + outcomes: [ + { skillName: 'hidden', changed: true }, + { skillName: 'inactive', changed: true }, + ], + settingsChanges: [ + { key: 'skills.disabled', value: ['hidden', 'inactive'] }, + ], + }); + const svc = createDaemonWorkspaceService( + makeDeps({ + queryWorkspaceStatus, + persistDisabledSkillsBatch, + isChannelLive: () => false, + }), + ); + + await expect( + svc.setWorkspaceSkillsEnabled(makeCtx(), ['hidden', 'inactive'], false), + ).resolves.toMatchObject({ + results: [ + { skillName: 'hidden', enabled: false, changed: true }, + { skillName: 'inactive', enabled: false, changed: true }, + ], + errors: [], + }); + expect(queryWorkspaceStatus).not.toHaveBeenCalled(); + expect(persistDisabledSkillsBatch).toHaveBeenCalledWith( + '/workspace', + ['hidden', 'inactive'], + false, + undefined, + ); + }); + it('persists and refreshes once while preserving ordered target outcomes', async () => { const queryWorkspaceStatus = vi.fn().mockResolvedValue({ v: 1, @@ -2217,22 +2275,24 @@ describe('createDaemonWorkspaceService', () => { }); const persistDisabledSkillsBatch = vi.fn().mockResolvedValue({ outcomes: [ - { skillName: 'review', changed: true }, + { skillName: 'Review', changed: true }, { skillName: 'missing', changed: true }, - { - skillName: 'locked', - error: new WorkspaceSkillNotToggleableError( - 'locked', - 'locked', - 'user', - ), - }, + { skillName: 'hidden', changed: true }, + { skillName: 'inactive', changed: true }, + { skillName: 'locked', changed: true }, { skillName: 'deploy', changed: true }, ], settingsChanges: [ { key: 'skills.disabled', - value: ['review', 'missing', 'deploy'], + value: [ + 'Review', + 'missing', + 'hidden', + 'inactive', + 'locked', + 'deploy', + ], }, ], }); @@ -2257,11 +2317,11 @@ describe('createDaemonWorkspaceService', () => { false, ); - expect(queryWorkspaceStatus).toHaveBeenCalledOnce(); + expect(queryWorkspaceStatus).not.toHaveBeenCalled(); expect(persistDisabledSkillsBatch).toHaveBeenCalledOnce(); expect(persistDisabledSkillsBatch).toHaveBeenCalledWith( '/workspace', - ['review', 'missing', 'locked', 'deploy'], + ['Review', 'missing', 'hidden', 'inactive', 'locked', 'deploy'], false, undefined, ); @@ -2276,40 +2336,33 @@ describe('createDaemonWorkspaceService', () => { sessionsRefreshed: 2, sessionsFailed: 0, results: [ - { skillName: 'review', enabled: false, changed: true }, + { skillName: 'Review', enabled: false, changed: true }, { skillName: 'missing', enabled: false, changed: true }, + { skillName: 'hidden', enabled: false, changed: true }, + { skillName: 'inactive', enabled: false, changed: true }, + { skillName: 'locked', enabled: false, changed: true }, { skillName: 'deploy', enabled: false, changed: true }, ], - errors: [ - { - skillName: 'hidden', - code: 'skill_not_toggleable', - error: 'Skill hidden is not toggleable: not_user_invocable', - reason: 'not_user_invocable', - }, - { - skillName: 'inactive', - code: 'skill_inactive_extension', - error: 'Skill inactive is not toggleable: inactive_extension', - reason: 'inactive_extension', - }, - { - skillName: 'locked', - code: 'skill_not_toggleable', - error: 'Skill locked is locked by user settings', - reason: 'locked', - lockedScope: 'user', - }, - ], + errors: [], }); expect(publishWorkspaceEvent).toHaveBeenCalledOnce(); expect(publishWorkspaceEvent).toHaveBeenCalledWith( skillToggleSettingsChanged({ key: 'skills.disabled', - value: ['review', 'missing', 'deploy'], + value: [ + 'Review', + 'missing', + 'hidden', + 'inactive', + 'locked', + 'deploy', + ], skills: [ - { name: 'review', enabled: false }, + { name: 'Review', enabled: false }, { name: 'missing', enabled: false }, + { name: 'hidden', enabled: false }, + { name: 'inactive', enabled: false }, + { name: 'locked', enabled: false }, { name: 'deploy', enabled: false }, ], activation: 'applied', @@ -2319,7 +2372,7 @@ describe('createDaemonWorkspaceService', () => { ); }); - it('orders results and errors by request targets, not persist outcomes', async () => { + it('orders results and legacy errors by request targets', async () => { const svc = createDaemonWorkspaceService( makeDeps({ queryWorkspaceStatus: vi.fn().mockResolvedValue({ @@ -2425,48 +2478,65 @@ describe('createDaemonWorkspaceService', () => { expect(publishWorkspaceEvent).not.toHaveBeenCalled(); }); - it('returns validation errors without persisting when no target is valid', async () => { - const persistDisabledSkillsBatch = vi.fn(); + it('persists hidden and inactive Extension names without validation errors', async () => { + const queryWorkspaceStatus = vi.fn().mockResolvedValue({ + v: 1, + workspaceCwd: '/workspace', + initialized: true, + skills, + }); + const persistDisabledSkillsBatch = vi.fn().mockResolvedValue({ + outcomes: [ + { skillName: 'hidden', changed: true }, + { skillName: 'inactive', changed: true }, + ], + settingsChanges: [], + }); const svc = createDaemonWorkspaceService( makeDeps({ - queryWorkspaceStatus: vi.fn().mockResolvedValue({ - v: 1, - workspaceCwd: '/workspace', - initialized: true, - skills, - }), + queryWorkspaceStatus, persistDisabledSkillsBatch, - isChannelLive: () => true, + isChannelLive: () => false, }), ); await expect( svc.setWorkspaceSkillsEnabled(makeCtx(), ['hidden', 'inactive'], false), ).resolves.toMatchObject({ - activation: 'applied', + activation: 'deferred', sessionsRefreshed: 0, sessionsFailed: 0, - results: [], - errors: [ - { skillName: 'hidden', code: 'skill_not_toggleable' }, - { skillName: 'inactive', code: 'skill_inactive_extension' }, + results: [ + { skillName: 'hidden', enabled: false, changed: true }, + { skillName: 'inactive', enabled: false, changed: true }, ], + errors: [], }); - expect(persistDisabledSkillsBatch).not.toHaveBeenCalled(); + expect(queryWorkspaceStatus).not.toHaveBeenCalled(); + expect(persistDisabledSkillsBatch).toHaveBeenCalledWith( + '/workspace', + ['hidden', 'inactive'], + false, + undefined, + ); }); - it('rejects a legacy inactive extension skill like the single-toggle path', async () => { + it('persists a legacy inactive Extension name like the single-toggle path', async () => { await withIsolatedWorkspace(async ({ workspace }) => { - const persistDisabledSkillsBatch = vi.fn(); + const queryWorkspaceStatus = vi.fn().mockResolvedValue({ + v: 1, + workspaceCwd: workspace, + initialized: true, + skills, + }); + const persistDisabledSkillsBatch = vi.fn().mockResolvedValue({ + outcomes: [{ skillName: 'legacy-inactive', changed: true }], + settingsChanges: [], + }); const svc = createDaemonWorkspaceService( makeDeps({ boundWorkspace: workspace, - queryWorkspaceStatus: vi.fn().mockResolvedValue({ - v: 1, - workspaceCwd: workspace, - initialized: true, - skills, - }), + queryWorkspaceStatus, persistDisabledSkillsBatch, }), ); @@ -2474,16 +2544,22 @@ describe('createDaemonWorkspaceService', () => { await expect( svc.setWorkspaceSkillsEnabled(makeCtx(), ['legacy-inactive'], false), ).resolves.toMatchObject({ - results: [], - errors: [ + results: [ { skillName: 'legacy-inactive', - code: 'skill_inactive_extension', - reason: 'inactive_extension', + enabled: false, + changed: true, }, ], + errors: [], }); - expect(persistDisabledSkillsBatch).not.toHaveBeenCalled(); + expect(queryWorkspaceStatus).not.toHaveBeenCalled(); + expect(persistDisabledSkillsBatch).toHaveBeenCalledWith( + workspace, + ['legacy-inactive'], + false, + undefined, + ); }); }); @@ -2944,7 +3020,6 @@ describe('createDaemonWorkspaceService', () => { const queryWorkspaceStatus = vi .fn() .mockResolvedValueOnce(beforeStatus) - .mockResolvedValueOnce(beforeStatus) .mockResolvedValueOnce(afterStatus); const invokeWorkspaceCommand = vi.fn( () => refresh.promise, @@ -2979,7 +3054,7 @@ describe('createDaemonWorkspaceService', () => { await expect(svc.getWorkspaceSkillsStatus(makeCtx())).resolves.toEqual( afterStatus, ); - expect(queryWorkspaceStatus).toHaveBeenCalledTimes(3); + expect(queryWorkspaceStatus).toHaveBeenCalledTimes(2); }); }); diff --git a/packages/cli/src/serve/workspace-service/index.ts b/packages/cli/src/serve/workspace-service/index.ts index 40c0728e070..68521a684bd 100644 --- a/packages/cli/src/serve/workspace-service/index.ts +++ b/packages/cli/src/serve/workspace-service/index.ts @@ -48,7 +48,6 @@ import { import { MCP_RESTART_SERVER_DEADLINE_MS } from '@qwen-code/acp-bridge/mcpTimeouts'; import { loadSettings } from '../../config/settings.js'; -import { resolveSkillSettings } from '../../config/skill-settings.js'; import { getWorkspaceTrustStatus } from '../../config/trustedFolders.js'; import { buildPermissionSettings } from '../../config/permission-settings.js'; import { @@ -73,7 +72,6 @@ import { mapWorkspaceSkillToggleError, WorkspacePermissionRulesSessionRequiredError, WorkspaceSkillNotFoundError, - WorkspaceSkillNotToggleableError, WorkspaceSettingsPartialPersistError, } from './types.js'; import type { @@ -87,7 +85,6 @@ import type { WorkspaceAcpPreheatResult, WorkspaceAcpStatusResult, WorkspaceSkillBatchToggleResult, - WorkspaceSkillToggleError, WorkspaceSkillToggleResult, WorkspaceSkillToggleActivation, PersistDisabledSkillsBatchResult, @@ -840,43 +837,10 @@ export function createDaemonWorkspaceService( enabled: boolean, ): Promise { assertActiveGeneration(); - const normalizedName = requestedSkillName.trim().toLowerCase(); - const status = await getWorkspaceSkillsStatus(); - const skill = status.skills.find( - (candidate) => candidate.name.trim().toLowerCase() === normalizedName, - ); - if (!skill) throw new WorkspaceSkillNotFoundError(requestedSkillName); - if (skill.userInvocable === false) { - throw new WorkspaceSkillNotToggleableError( - skill.name, - 'not_user_invocable', - ); - } - - const needsLegacyInactiveCheck = - skill.level === 'extension' && - skill.status === 'disabled' && - skill.disabledReason === undefined; - const disabledBySettings = - needsLegacyInactiveCheck && - resolveSkillSettings(loadBoundSettings(true)).disabledNames.has( - normalizedName, - ); - if ( - skill.level === 'extension' && - skill.status === 'disabled' && - (skill.disabledReason === 'inactive_extension' || - (skill.disabledReason === undefined && !disabledBySettings)) - ) { - throw new WorkspaceSkillNotToggleableError( - skill.name, - 'inactive_extension', - ); - } - + const skillName = requestedSkillName.trim(); const persisted = await persistDisabledSkills( boundWorkspace, - skill.name, + skillName, enabled, assertGenerationOpen, ); @@ -930,7 +894,7 @@ export function createDaemonWorkspaceService( }, ]; const mutation = createSkillToggleMutation({ - skills: [{ name: skill.name, enabled }], + skills: [{ name: skillName, enabled }], activation, sessionsRefreshed, sessionsFailed, @@ -950,7 +914,7 @@ export function createDaemonWorkspaceService( } return { - skillName: skill.name, + skillName, enabled, changed: persisted.changed, activation, @@ -965,73 +929,12 @@ export function createDaemonWorkspaceService( enabled: boolean, ): Promise { assertActiveGeneration(); - const status = await getWorkspaceSkillsStatus(); - const skillsByName = new Map< - string, - ServeWorkspaceSkillsStatus['skills'][number] - >(); - for (const skill of status.skills) { - const normalizedName = skill.name.trim().toLowerCase(); - if (!skillsByName.has(normalizedName)) { - skillsByName.set(normalizedName, skill); - } - } - const disabledNames = resolveSkillSettings( - loadBoundSettings(true), - ).disabledNames; - const targets: Array< - | { requestedName: string; skillName: string } - | { requestedName: string; error: WorkspaceSkillToggleError } - > = []; - - for (const requestedName of requestedSkillNames) { - const normalizedName = requestedName.trim().toLowerCase(); - const skill = skillsByName.get(normalizedName); - if (!skill) { - targets.push({ requestedName, skillName: requestedName }); - continue; - } - let domainError: unknown; - if (skill.userInvocable === false) { - domainError = new WorkspaceSkillNotToggleableError( - skill.name, - 'not_user_invocable', - ); - } else { - const legacyInactive = - skill.level === 'extension' && - skill.status === 'disabled' && - skill.disabledReason === undefined && - !disabledNames.has(normalizedName); - if ( - skill.level === 'extension' && - skill.status === 'disabled' && - (skill.disabledReason === 'inactive_extension' || legacyInactive) - ) { - domainError = new WorkspaceSkillNotToggleableError( - skill.name, - 'inactive_extension', - ); - } - } - - if (domainError) { - const error = mapWorkspaceSkillToggleError(domainError); - if (!error) throw domainError; - targets.push({ requestedName, error }); - } else { - targets.push({ requestedName, skillName: skill.name }); - } - } - - const validSkillNames = targets.flatMap((target) => - 'skillName' in target ? [target.skillName] : [], - ); + const skillNames = requestedSkillNames.map((name) => name.trim()); const persisted: PersistDisabledSkillsBatchResult = - validSkillNames.length > 0 + skillNames.length > 0 ? await persistDisabledSkillsBatch( boundWorkspace, - validSkillNames, + skillNames, enabled, assertGenerationOpen, ) @@ -1045,17 +948,11 @@ export function createDaemonWorkspaceService( ); const results: WorkspaceSkillBatchToggleResult['results'] = []; const errors: WorkspaceSkillBatchToggleResult['errors'] = []; - for (const target of targets) { - if ('error' in target) { - errors.push(target.error); - continue; - } - const outcome = persistedByName.get( - target.skillName.trim().toLowerCase(), - ); + for (const skillName of skillNames) { + const outcome = persistedByName.get(skillName.toLowerCase()); if (!outcome) { throw new Error( - `Missing persisted Skill batch outcome: ${target.skillName}`, + `Missing persisted Skill batch outcome: ${skillName}`, ); } if ('error' in outcome) { diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 8a4d7777a32..7a347a17f25 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -3628,7 +3628,7 @@ export class DaemonClient { } /** - * Toggle a user-invocable skill in workspace `skills.disabled` settings. + * Update workspace Skill settings by name without requiring a loaded Skill. * Active ACP sessions refresh their skill validation and command lists before * the response returns; `activation` reports deferred or partial refreshes. * @@ -3662,7 +3662,7 @@ export class DaemonClient { } /** - * Toggle up to 100 user-invocable skills and return every target outcome. + * Update workspace Skill settings for up to 100 names in one write. * * Pre-flight * `caps.features.includes('workspace_skill_batch_toggle')` before calling. diff --git a/packages/sdk-typescript/src/daemon/types.ts b/packages/sdk-typescript/src/daemon/types.ts index 22b02452f64..c98250fbb32 100644 --- a/packages/sdk-typescript/src/daemon/types.ts +++ b/packages/sdk-typescript/src/daemon/types.ts @@ -2847,6 +2847,7 @@ export interface DaemonSkillToggleResult { sessionsFailed: number; } +/** Per-target error codes returned by older daemon versions. */ export type DaemonSkillBatchToggleErrorCode = | 'skill_not_found' | 'skill_not_toggleable' From 6befd5f3b09cc1a9fdee6e74ec3c01e6694e3580 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Wed, 26 Aug 2026 15:31:55 +0800 Subject: [PATCH 02/10] fix(web-shell): reconcile skill toggle status --- .../skills/SkillsManagerPage.test.tsx | 223 ++++++++++++++++++ .../components/skills/SkillsManagerPage.tsx | 42 +--- packages/web-shell/client/i18n.tsx | 6 + 3 files changed, 240 insertions(+), 31 deletions(-) create mode 100644 packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx diff --git a/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx b/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx new file mode 100644 index 00000000000..d633c363cfc --- /dev/null +++ b/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx @@ -0,0 +1,223 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +// @vitest-environment jsdom + +import { act } from 'react'; +import { createRoot, type Root } from 'react-dom/client'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import type { DaemonWorkspaceSkillStatus } from '@qwen-code/webui/daemon-react-sdk'; + +Object.assign(globalThis, { IS_REACT_ACT_ENVIRONMENT: true }); + +const { skillsState, workspaceState } = vi.hoisted(() => ({ + skillsState: { + current: { + status: undefined, + skills: [] as DaemonWorkspaceSkillStatus[], + loading: false, + error: undefined, + reload: vi.fn(), + setEnabled: vi.fn(), + install: vi.fn(), + remove: vi.fn(), + }, + }, + workspaceState: { + current: { + capabilities: { + features: ['workspace_skill_toggle'], + }, + }, + }, +})); + +vi.mock('@qwen-code/webui/daemon-react-sdk', () => ({ + useSkills: () => skillsState.current, + useWorkspace: () => workspaceState.current, +})); + +const { SkillsManagerPage } = await import('./SkillsManagerPage'); +const { I18nProvider } = await import('../../i18n'); + +let container: HTMLDivElement; +let root: Root; + +async function renderPage(): Promise { + await act(async () => { + root.render( + + + , + ); + }); +} + +async function openDisabledSkill(name: string): Promise { + const statusFilter = container.querySelector( + '[aria-label="Filter skills by status"]', + ); + expect(statusFilter).not.toBeNull(); + await act(async () => { + statusFilter!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + const disabledOption = Array.from( + document.body.querySelectorAll('[role="option"]'), + ).find((item) => item.textContent?.trim() === 'Disabled'); + expect(disabledOption).toBeDefined(); + await act(async () => { + disabledOption!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + const skill = container.querySelector(`[aria-label="${name}"]`); + expect(skill).not.toBeNull(); + await act(async () => { + skill!.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); +} + +async function enableSelectedSkill(): Promise { + const actions = container.querySelector( + '[data-testid="skill-actions"]', + ); + expect(actions).not.toBeNull(); + await act(async () => { + actions!.dispatchEvent( + new MouseEvent('pointerdown', { bubbles: true, button: 0 }), + ); + }); + const enable = Array.from( + document.body.querySelectorAll('[role="menuitem"]'), + ).find((item) => item.textContent?.trim() === 'Enable'); + expect(enable).toBeDefined(); + await act(async () => { + enable!.click(); + await Promise.resolve(); + await Promise.resolve(); + }); +} + +function runButton(): HTMLButtonElement | undefined { + return Array.from(container.querySelectorAll('button')).find( + (button) => button.textContent?.trim() === 'Reference skill', + ); +} + +beforeEach(() => { + container = document.createElement('div'); + document.body.appendChild(container); + root = createRoot(container); + skillsState.current.status = undefined; + skillsState.current.skills = []; + skillsState.current.loading = false; + skillsState.current.error = undefined; + skillsState.current.reload.mockReset(); + skillsState.current.setEnabled.mockReset().mockResolvedValue(undefined); + skillsState.current.install.mockReset(); + skillsState.current.remove.mockReset(); +}); + +afterEach(() => { + act(() => root.unmount()); + container.remove(); +}); + +describe('SkillsManagerPage', () => { + it('shows the authoritative enabled state after a normal toggle', async () => { + const disabledSkill: DaemonWorkspaceSkillStatus = { + kind: 'skill', + status: 'disabled', + name: 'review', + description: 'Review code', + level: 'user', + modelInvocable: true, + disabledReason: 'default', + }; + const enabledSkill: DaemonWorkspaceSkillStatus = { + ...disabledSkill, + status: 'ok', + disabledReason: undefined, + }; + skillsState.current.skills = [disabledSkill]; + skillsState.current.reload.mockResolvedValue({ + v: 1, + workspaceCwd: '/workspace/demo', + initialized: true, + skills: [enabledSkill], + errors: [], + }); + + await renderPage(); + await openDisabledSkill(disabledSkill.name); + await enableSelectedSkill(); + skillsState.current.skills = [enabledSkill]; + await renderPage(); + + expect(container.textContent).toContain('Skill enabled.'); + expect(container.textContent).toContain('enabled'); + expect(runButton()?.disabled).toBe(false); + }); + + it.each([ + { + label: 'higher-scope locked', + skill: { + kind: 'skill' as const, + status: 'disabled' as const, + name: 'locked', + description: 'Locked by user settings', + level: 'bundled' as const, + modelInvocable: true, + disabledReason: 'hard' as const, + lockedScope: 'user' as const, + }, + }, + { + label: 'inactive Extension', + skill: { + kind: 'skill' as const, + status: 'disabled' as const, + name: 'inactive', + description: 'Inactive extension skill', + level: 'extension' as const, + modelInvocable: true, + extensionName: 'demo', + disabledReason: 'inactive_extension' as const, + }, + }, + ])( + 'keeps a $label Skill disabled after its workspace setting is enabled', + async ({ skill }) => { + skillsState.current.skills = [skill]; + skillsState.current.reload.mockResolvedValue({ + v: 1, + workspaceCwd: '/workspace/demo', + initialized: true, + skills: [skill], + errors: [], + }); + + await renderPage(); + await openDisabledSkill(skill.name); + expect(runButton()?.disabled).toBe(true); + + await enableSelectedSkill(); + + expect(skillsState.current.setEnabled).toHaveBeenCalledWith( + skill.name, + true, + ); + expect(skillsState.current.reload).toHaveBeenCalledTimes(1); + skillsState.current.skills = [{ ...skill }]; + await renderPage(); + expect(container.textContent).toContain( + 'Workspace setting updated. Effective Skill availability did not change.', + ); + expect(container.textContent).toContain('disabled'); + expect(runButton()?.disabled).toBe(true); + }, + ); +}); diff --git a/packages/web-shell/client/components/skills/SkillsManagerPage.tsx b/packages/web-shell/client/components/skills/SkillsManagerPage.tsx index 644381b442f..f648bce30ba 100644 --- a/packages/web-shell/client/components/skills/SkillsManagerPage.tsx +++ b/packages/web-shell/client/components/skills/SkillsManagerPage.tsx @@ -182,9 +182,6 @@ export function SkillsManagerPage({ const [levelFilter, setLevelFilter] = useState('all'); const [statusFilter, setStatusFilter] = useState('enabled'); - const [statusOverrides, setStatusOverrides] = useState< - Record - >({}); const [selectedName, setSelectedName] = useState(null); const [busySkill, setBusySkill] = useState(null); const [installOpen, setInstallOpen] = useState(false); @@ -195,14 +192,7 @@ export function SkillsManagerPage({ text: string; error: boolean; } | null>(null); - const displayedSkills = useMemo( - () => - skills.map((skill) => ({ - ...skill, - status: statusOverrides[skill.name] ?? skill.status, - })), - [skills, statusOverrides], - ); + const displayedSkills = skills; const selectedSkill = useMemo( () => displayedSkills.find((skill) => skill.name === selectedName), [displayedSkills, selectedName], @@ -230,20 +220,6 @@ export function SkillsManagerPage({ setSelectedName((name) => preserveSkillSelection(name, displayedSkills)); }, [displayedSkills]); - useEffect(() => { - setStatusOverrides((current) => { - const next = { ...current }; - let changed = false; - for (const skill of skills) { - if (next[skill.name] === skill.status) { - delete next[skill.name]; - changed = true; - } - } - return changed ? next : current; - }); - }, [skills]); - useEffect(() => { embedded?.onDetailChange(Boolean(selectedSkill)); }, [embedded, selectedSkill]); @@ -254,14 +230,18 @@ export function SkillsManagerPage({ setNotice(null); try { await setEnabled(skill.name, enabled); - setStatusOverrides((current) => ({ - ...current, - [skill.name]: enabled ? 'ok' : 'disabled', - })); - await reload(); + const refreshed = await reload(); + const refreshedSkill = refreshed?.skills.find( + (item) => item.name.toLowerCase() === skill.name.toLowerCase(), + ); + const expectedStatus = enabled ? 'ok' : 'disabled'; setNotice({ skillName: skill.name, - text: t(enabled ? 'skills.enabled' : 'skills.disabled'), + text: !refreshedSkill + ? t('skills.settingUpdated') + : refreshedSkill.status === expectedStatus + ? t(enabled ? 'skills.enabled' : 'skills.disabled') + : t('skills.settingUpdatedAvailabilityUnchanged'), error: false, }); } catch (toggleError) { diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index 0feeba4f258..a4617755faa 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -2364,6 +2364,9 @@ const EN: Messages = { 'skills.notToggleable': 'This skill cannot be enabled or disabled.', 'skills.run': 'Reference skill', 'skills.search': 'Search skills…', + 'skills.settingUpdated': 'Workspace setting updated.', + 'skills.settingUpdatedAvailabilityUnchanged': + 'Workspace setting updated. Effective Skill availability did not change.', 'skills.status': 'Status', 'skills.status.disabled': 'disabled', 'skills.status.enabled': 'enabled', @@ -5291,6 +5294,9 @@ const ZH: Messages = { 'skills.notToggleable': '此 Skill 不支持启用或禁用。', 'skills.run': '引用 skill', 'skills.search': '搜索 Skills…', + 'skills.settingUpdated': 'Workspace 设置已更新。', + 'skills.settingUpdatedAvailabilityUnchanged': + 'Workspace 设置已更新，Skill 的实际可用状态未改变。', 'skills.status': '状态', 'skills.status.disabled': '已禁用', 'skills.status.enabled': '已启用', From 18a9b9651ecdfe80996856361ca7a8f74a849399 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Wed, 26 Aug 2026 15:47:56 +0800 Subject: [PATCH 03/10] fix(skills): reconcile settings-only toggles --- docs/developers/qwen-serve-protocol.md | 7 +- .../cli/src/config/skill-settings.test.ts | 11 +-- packages/cli/src/config/skill-settings.ts | 16 +--- packages/cli/src/serve/server.test.ts | 4 +- .../components/skills/SkillsManagerDialog.tsx | 3 - packages/web-shell/client/App.test.tsx | 74 +++++++++++++++++++ packages/web-shell/client/App.tsx | 27 ++++++- 7 files changed, 109 insertions(+), 33 deletions(-) diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 0fbab4a1ec3..f98607fa90d 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -1387,8 +1387,9 @@ Recommended poll cadence: aligned with whatever already polls `/workspace/mcp`; `level` is one of `project`, `user`, `extension`, or `bundled`. `userInvocable` (boolean, optional) is omitted for normal skills (meaning -`true`) and is present only as `false` when the skill cannot be invoked manually -or toggled through the skill API. `modelInvocable` is independent: `false` +`true`) and is present only as `false` when the skill cannot be invoked +manually. It does not gate the settings-only Skill toggle routes described +below. `modelInvocable` is independent: `false` means the skill remains manually available but is hidden from model invocation. `installedPath` is the existing absolute path to the skill's `SKILL.md`; the daemon returns it as stored without separately resolving symlinks or @@ -2848,7 +2849,7 @@ Errors: - `400 {code: 'invalid_enabled_flag'}` — `enabled` missing or non-boolean. - `403 {code: 'untrusted_workspace'}` — the selected workspace is not trusted. -The mutation reuses the workspace-scoped `settings_changed` event for each changed key (`skills.disabled` and/or `skills.enabled`); it does not add a new event type. Each of those events includes the same `mutation` object: `{ id, kind: 'skill_toggle', skills: [{ name, enabled }], activation, sessionsRefreshed, sessionsFailed }`. `id` correlates every settings event produced by one toggle request. `skills` lists the requested names and resulting enabled states of Skills that actually changed. Workspace skill status cells include optional `disabledReason: 'hard' | 'default' | 'inactive_extension'` and `lockedScope: 'system' | 'user' | 'systemDefaults'` fields. +The mutation reuses the workspace-scoped `settings_changed` event for each changed key (`skills.disabled` and/or `skills.enabled`); it does not add a new event type. Each of those events includes the same `mutation` object: `{ id, kind: 'skill_toggle', skills: [{ name, enabled }], activation, sessionsRefreshed, sessionsFailed }`. `id` correlates every settings event produced by one toggle request. `skills` lists the requested names and requested enabled values whose workspace settings declarations actually changed; a higher-scope setting can leave effective availability unchanged. Workspace skill status cells include optional `disabledReason: 'hard' | 'default' | 'inactive_extension'` and `lockedScope: 'system' | 'user' | 'systemDefaults'` fields. #### `POST /workspace/skills/enable` diff --git a/packages/cli/src/config/skill-settings.test.ts b/packages/cli/src/config/skill-settings.test.ts index 81e482492ec..dbca0d601d9 100644 --- a/packages/cli/src/config/skill-settings.test.ts +++ b/packages/cli/src/config/skill-settings.test.ts @@ -148,7 +148,6 @@ describe('computeWorkspaceSkillListUpdates', () => { // 'review' is toggled. const result = computeWorkspaceSkillListUpdates( ['orphan', 'review'], - new Set(), [], [ { @@ -165,13 +164,9 @@ describe('computeWorkspaceSkillListUpdates', () => { expect(result.enabledChanged).toBe(false); }); - it('drops locked higher-scope entries so they are not re-emitted', () => { - // 'locked' is disabled at a higher scope; the picker must not re-emit it at - // workspace scope. Toggling 'review' on provides a genuine change so the - // write path is exercised while 'orphan' is still preserved. + it('preserves workspace declarations that duplicate higher-scope entries', () => { const result = computeWorkspaceSkillListUpdates( ['locked', 'orphan', 'review'], - new Set(['locked']), [], [ { @@ -183,14 +178,13 @@ describe('computeWorkspaceSkillListUpdates', () => { ], ); - expect(result.disabled).toEqual(['orphan']); + expect(result.disabled).toEqual(['locked', 'orphan']); expect(result.disabledChanged).toBe(true); }); it('reports no change when nothing toggled and lists already match', () => { const result = computeWorkspaceSkillListUpdates( ['orphan'], - new Set(), [], [ { @@ -210,7 +204,6 @@ describe('computeWorkspaceSkillListUpdates', () => { it('records an explicit opt-in when enabling a default-disabled skill', () => { const result = computeWorkspaceSkillListUpdates( [], - new Set(), [], [ { diff --git a/packages/cli/src/config/skill-settings.ts b/packages/cli/src/config/skill-settings.ts index aae32d82487..158fd1e3aa0 100644 --- a/packages/cli/src/config/skill-settings.ts +++ b/packages/cli/src/config/skill-settings.ts @@ -159,24 +159,16 @@ export interface WorkspaceSkillListUpdates { * Computes the workspace `skills.disabled` / `skills.enabled` lists the skills * picker should persist after a set of toggle changes. * - * The seed lists are the workspace's current entries. Locked skills (disabled - * at a higher scope) are dropped from the seed so we never re-emit redundant - * entries the higher scope already enforces. Orphaned entries — workspace - * disables for skills not currently loaded (a different git branch, an - * uninstalled extension, a deleted skills dir) — are preserved verbatim: only - * the toggled, currently-loaded skills passed in `toggles` mutate the lists. - * That preservation is load-bearing; the orphan case is pinned by a test in - * `skill-settings.test.ts`. + * The seed lists are the workspace's current entries. Orphaned entries and + * declarations duplicated at a higher scope are preserved verbatim: only the + * toggled, currently-loaded skills passed in `toggles` mutate the lists. */ export function computeWorkspaceSkillListUpdates( workspaceDisabled: readonly string[], - lockedNames: ReadonlySet, workspaceEnabled: readonly string[], toggles: readonly WorkspaceSkillListToggle[], ): WorkspaceSkillListUpdates { - const previousDisabled = workspaceDisabled.filter( - (name) => !lockedNames.has(name.trim().toLowerCase()), - ); + const previousDisabled = [...workspaceDisabled]; const previousEnabled = [...workspaceEnabled]; let next: WorkspaceSkillSettingLists = { disabled: previousDisabled, diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 4c695d5aefe..49673a39b81 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -22949,7 +22949,7 @@ describe('createServeApp', () => { expect(badBody.body.code).toBe('invalid_enabled_flag'); }); - it('returns the requested name and deferred activation without a child', async () => { + it('trims and returns the requested name with deferred activation without a child', async () => { const bridge = fakeBridge({ workspaceSkillsImpl: async () => ({ v: 1, @@ -22969,7 +22969,7 @@ describe('createServeApp', () => { primaryWorkspaceTrusted: true, }); const res = await auth( - request(app).post('/workspace/skills/ReViEw/enable'), + request(app).post('/workspace/skills/%20ReViEw%20/enable'), ).send({ enabled: false }); expect(res.status).toBe(200); diff --git a/packages/cli/src/ui/components/skills/SkillsManagerDialog.tsx b/packages/cli/src/ui/components/skills/SkillsManagerDialog.tsx index f8b82f670af..f50b37229b4 100644 --- a/packages/cli/src/ui/components/skills/SkillsManagerDialog.tsx +++ b/packages/cli/src/ui/components/skills/SkillsManagerDialog.tsx @@ -309,11 +309,9 @@ export function SkillsManagerDialog({ settings, SettingScope.Workspace, ).filter((name): name is string => typeof name === 'string'); - const lockedNames = new Set(lockedSkills.map((skill) => lower(skill.name))); const { disabled, enabled, disabledChanged, enabledChanged } = computeWorkspaceSkillListUpdates( workspaceDisabled, - lockedNames, skillSettingStrings(settings, SettingScope.Workspace, 'enabled'), unlockedSkills.map((skill) => ({ name: skill.name, @@ -397,7 +395,6 @@ export function SkillsManagerDialog({ addItem, initialResolved, initialSelectedKeys, - lockedSkills, reloadCommands, selectedKeys, settings, diff --git a/packages/web-shell/client/App.test.tsx b/packages/web-shell/client/App.test.tsx index 8fdcb24180a..7b7a90267f8 100644 --- a/packages/web-shell/client/App.test.tsx +++ b/packages/web-shell/client/App.test.tsx @@ -10985,6 +10985,80 @@ describe('App session callbacks', () => { ]); }); + it('removes declaration-only enables from a mixed pending mutation', async () => { + const lockedStatus = { + skills: [ + { + name: 'locked', + description: 'Locked by user settings', + status: 'disabled' as const, + disabledReason: 'hard' as const, + lockedScope: 'user' as const, + }, + { + name: 'other', + description: 'Other skill', + status: 'ok' as const, + }, + { + name: 'review', + description: 'Review code', + status: 'ok' as const, + }, + { + name: 'review', + description: 'Inactive Extension copy', + status: 'disabled' as const, + disabledReason: 'inactive_extension' as const, + }, + ], + }; + mockWorkspaceActions.loadSkillsStatus.mockResolvedValue(lockedStatus); + mockConnection.commands = [skillCommandFixture('other', 'Other skill')]; + mockConnection.skills = ['other']; + const { rerender } = renderApp(); + await flush(); + + emitPartialSkillMutation('enable-mixed-declarations', [ + { name: 'locked', enabled: true }, + { name: 'review', enabled: true }, + ]); + rerender(); + await vi.waitFor(() => { + expect(mockWorkspaceActions.loadSkillsStatus).toHaveBeenCalledTimes(2); + }); + await vi.waitFor(() => { + expect(testState.latestChatEditorProps?.skills).toEqual([ + { name: 'other', description: 'Other skill' }, + { name: 'review', description: 'Review code' }, + ]); + }); + + mockConnection.commands = [ + skillCommandFixture('other', 'Other skill'), + skillCommandFixture('review', 'Review code'), + skillCommandFixture('late', 'Late session skill'), + ]; + mockConnection.skills = ['other', 'review', 'late']; + rerender(); + await flush(); + + expect(testState.latestChatEditorProps?.skills).toEqual([ + { name: 'late', description: 'Late session skill' }, + { name: 'other', description: 'Other skill' }, + { name: 'review', description: 'Review code' }, + ]); + + emitSkillMutation( + 'applied-after-mixed', + [{ name: 'other', enabled: true }], + 'applied', + ); + rerender(); + await flush(); + expect(mockWorkspaceActions.loadSkillsStatus).toHaveBeenCalledTimes(2); + }); + it('revalidates a partial Skill mutation only within its workspace', async () => { const enabledStatus = { skills: [ diff --git a/packages/web-shell/client/App.tsx b/packages/web-shell/client/App.tsx index 6e9d8a9c8d7..e7ccdb8afaf 100644 --- a/packages/web-shell/client/App.tsx +++ b/packages/web-shell/client/App.tsx @@ -4775,7 +4775,7 @@ export function App({ if (request !== loadedSkillsRequestRef.current) return; setLoadedSkills(availableSkillInfos(status)); setLoadedSkillsReady(true); - return true; + return status; } catch (error) { if (notifyOnError) { pushToast( @@ -4846,17 +4846,36 @@ export function App({ } pendingSkillTogglesByContextRef.current.set(contextKey, pendingToggles); let cancelled = false; - void reloadLoadedSkills(workspaceCwd, true).then((loaded) => { - if (cancelled || !loaded) return; + void reloadLoadedSkills(workspaceCwd, true).then((status) => { + if (cancelled || !status) return; markHandled(); if (!sessionId) { pendingSkillTogglesByContextRef.current.delete(contextKey); return; } + const availableWorkspaceSkillNames = new Set( + status.skills + .filter((skill) => skill.status === 'ok') + .map((skill) => skill.name.toLowerCase()), + ); + const pendingForSession = pendingToggles.filter( + (toggle) => + !toggle.enabled || + availableWorkspaceSkillNames.has(toggle.name.toLowerCase()), + ); + if (pendingForSession.length === 0) { + pendingSkillTogglesByContextRef.current.delete(contextKey); + setLoadedSkillsFallback(undefined); + return; + } + pendingSkillTogglesByContextRef.current.set( + contextKey, + pendingForSession, + ); const currentSnapshot = connectionSkillSnapshotRef.current; if ( currentSnapshot.sessionId === sessionId && - sessionSkillsReflectToggle(currentSnapshot.skills, pendingToggles) + sessionSkillsReflectToggle(currentSnapshot.skills, pendingForSession) ) { pendingSkillTogglesByContextRef.current.delete(contextKey); setLoadedSkillsFallback(undefined); From 808d41aac0335d343c596fc989ae52f5cba6c07c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Wed, 26 Aug 2026 17:13:55 +0800 Subject: [PATCH 04/10] fix(serve): version skill settings capabilities --- docs/design/daemon-skill-batch-toggle.md | 14 +++-- docs/design/daemon-skill-toggle.md | 2 +- .../daemon/11-capabilities-versioning.md | 2 +- .../developers/daemon/13-sdk-daemon-client.md | 4 +- docs/developers/qwen-serve-protocol.md | 10 +-- docs/users/qwen-serve.md | 2 +- .../cli/qwen-serve-routes.test.ts | 4 +- packages/cli/src/serve/capabilities.ts | 4 +- .../src/serve/routes/workspace-skills.test.ts | 22 ++----- packages/cli/src/serve/run-qwen-serve.ts | 2 - packages/cli/src/serve/server.test.ts | 37 +---------- .../cli/src/serve/server/error-response.ts | 4 +- .../__tests__/facade.test.ts | 62 ++----------------- .../cli/src/serve/workspace-service/index.ts | 21 ++----- .../cli/src/serve/workspace-service/types.ts | 46 ++------------ .../sdk-typescript/src/daemon/DaemonClient.ts | 6 +- .../test/unit/DaemonClient.test.ts | 4 +- .../skills/SkillsManagerPage.test.tsx | 38 +++++++++++- .../components/skills/SkillsManagerPage.tsx | 5 +- 19 files changed, 94 insertions(+), 195 deletions(-) diff --git a/docs/design/daemon-skill-batch-toggle.md b/docs/design/daemon-skill-batch-toggle.md index 7f86405e14c..fd0b1356abe 100644 --- a/docs/design/daemon-skill-batch-toggle.md +++ b/docs/design/daemon-skill-batch-toggle.md @@ -71,11 +71,13 @@ as the single-Skill route. ## Compatibility -Advertise `workspace_skill_batch_toggle` separately from -`workspace_skill_toggle`. Clients must pre-flight the new capability before -calling the collection route. The single-Skill route now follows the same -settings-only contract and returns the trimmed request name because there is no -catalog lookup from which to obtain a canonical spelling. The collection -routes are HTTP-only: the ACP +Advertise `workspace_skill_settings_batch_toggle` separately from +`workspace_skill_settings_toggle`. These tags replace the retired +`workspace_skill_batch_toggle` and `workspace_skill_toggle` tags, whose +catalog-validated contract is incompatible with settings-only writes. Clients +must pre-flight the settings capability before calling the unchanged route. +The single-Skill route returns the trimmed request name because there is no +catalog lookup from which to obtain a canonical spelling. The collection routes +are HTTP-only: the ACP `_qwen/workspace/skills` dispatch surface stays read-only, matching the single-Skill toggle. diff --git a/docs/design/daemon-skill-toggle.md b/docs/design/daemon-skill-toggle.md index 4ebcff33e16..88c77c7ec8a 100644 --- a/docs/design/daemon-skill-toggle.md +++ b/docs/design/daemon-skill-toggle.md @@ -10,7 +10,7 @@ Expose workspace Skill settings writes through daemon REST and the TypeScript SD - `POST /workspaces/:workspace/skills/:name/enable` - Request body: `{ "enabled": boolean }` - SDK: `DaemonClient.setWorkspaceSkillEnabled` and `WorkspaceDaemonClient.setWorkspaceSkillEnabled` -- Capability: `workspace_skill_toggle` +- Capability: `workspace_skill_settings_toggle` The response contains the trimmed requested name, requested state, whether persistence changed, activation state, and session refresh counts. `applied` means every active session refreshed, `deferred` means no ACP child was running, and `partial` means at least one session failed to refresh after persistence committed. diff --git a/docs/developers/daemon/11-capabilities-versioning.md b/docs/developers/daemon/11-capabilities-versioning.md index 1f792404d1e..d48872fc3f0 100644 --- a/docs/developers/daemon/11-capabilities-versioning.md +++ b/docs/developers/daemon/11-capabilities-versioning.md @@ -124,7 +124,7 @@ V2 Extension batch activation: `extension_batch_activation_v2` adds queued globa Workspace-qualified session reads: `workspace_persisted_transcript`, `workspace_session_export`, `workspace_archived_session_export`, `workspace_session_live_state`. The active and archived export tags are independent from each other and from `session_export` and `workspace_qualified_rest_core`, so clients must pre-flight the exact storage state they intend to export. Persisted transcript paging permits an untrusted secondary under its bounded read policy; both full export paths remain trusted-only. `workspace_session_live_state` is likewise independent from `workspace_qualified_rest_core` and is trusted-only: it serves the selected runtime's memory-only live-session snapshot and catalog version and does not extend the untrusted-secondary persisted read policy to live bridge state. -Workspace mutation (Wave 4+): `workspace_memory`, `workspace_agents`, `workspace_agent_generate`, `workspace_acp_preheat`, `workspace_tool_toggle`, **`workspace_settings`** (conditional), `workspace_permissions`, `workspace_init`, `workspace_github_setup`, `workspace_trust`, `workspace_mcp_restart`, `workspace_mcp_manage`, `workspace_file_read`, `workspace_file_bytes`, `workspace_file_read_cursor`, `workspace_file_write`, `workspace_file_upload`, **`workspace_reload`** (conditional). +Workspace mutation (Wave 4+): `workspace_memory`, `workspace_agents`, `workspace_agent_generate`, `workspace_acp_preheat`, `workspace_tool_toggle`, `workspace_skill_settings_toggle`, `workspace_skill_settings_batch_toggle`, **`workspace_settings`** (conditional), `workspace_permissions`, `workspace_init`, `workspace_github_setup`, `workspace_trust`, `workspace_mcp_restart`, `workspace_mcp_manage`, `workspace_file_read`, `workspace_file_bytes`, `workspace_file_read_cursor`, `workspace_file_write`, `workspace_file_upload`, **`workspace_reload`** (conditional). MCP guardrails: **`mcp_guardrails`** (`modes: ['warn', 'enforce']`), `mcp_guardrail_events`, `mcp_server_runtime_mutation`, **`mcp_workspace_pool`** (conditional), **`mcp_pool_restart`** (conditional). diff --git a/docs/developers/daemon/13-sdk-daemon-client.md b/docs/developers/daemon/13-sdk-daemon-client.md index 5bc1c9db0b6..a7fa6bb4b3d 100644 --- a/docs/developers/daemon/13-sdk-daemon-client.md +++ b/docs/developers/daemon/13-sdk-daemon-client.md @@ -152,9 +152,9 @@ await client .setWorkspaceSkillEnabled('review', true, { clientId: 'dashboard-1' }); ``` -Pre-flight `capabilities.features.includes('workspace_skill_toggle')`. The typed `DaemonSkillToggleResult` reports the trimmed requested `skillName`, whether disk state `changed`, activation state (`applied`, `deferred`, or `partial`), and refreshed/failed session counts. The write is settings-only and does not require the name to appear in `DaemonWorkspaceSkillStatus`; that status type's optional false-only `userInvocable` field remains useful for rendering the live catalog but does not gate persistence. +Pre-flight `capabilities.features.includes('workspace_skill_settings_toggle')`. The typed `DaemonSkillToggleResult` reports the trimmed requested `skillName`, whether disk state `changed`, activation state (`applied`, `deferred`, or `partial`), and refreshed/failed session counts. The write is settings-only and does not require the name to appear in `DaemonWorkspaceSkillStatus`; that status type's optional false-only `userInvocable` field remains useful for rendering the live catalog but does not gate persistence. The retired `workspace_skill_toggle` tag described the earlier catalog-validated behavior and is not advertised for this contract. -For batch changes, pre-flight `workspace_skill_batch_toggle` and call either client shape with the same contract: +For batch changes, pre-flight `workspace_skill_settings_batch_toggle` and call either client shape with the same contract. The routes and request bodies are unchanged: ```ts await client.setWorkspaceSkillsEnabled(['review', 'deploy'], false, { diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index f98607fa90d..f57bbf67d32 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -198,8 +198,8 @@ registry. Clients **must** gate UI off `features`, not off `mode` (per design 'mcp_server_runtime_mutation', 'workspace_file_read', 'workspace_file_bytes', 'workspace_file_write', 'workspace_file_upload', - 'session_approval_mode_control', 'workspace_tool_toggle', 'workspace_skill_toggle', - 'workspace_skill_batch_toggle', + 'session_approval_mode_control', 'workspace_tool_toggle', + 'workspace_skill_settings_toggle', 'workspace_skill_settings_batch_toggle', 'extension_batch_activation_v2', 'workspace_settings', 'workspace_init', 'workspace_mcp_restart', 'session_recap', 'session_generation', 'session_btw', 'session_shell_command', @@ -270,7 +270,7 @@ registry. Clients **must** gate UI off `features`, not off `mode` (per design `session_info` advertises `GET /workspace/:id/session-info` and its `/workspaces/:workspace/session-info` twin. The response aggregates persisted active and archived session counts without hydrating list metadata. It is an explicit O(n) disk scan and must not be polled; clients should treat `truncated: true` as a lower-bound result. -`session_approval_mode_control`, `workspace_tool_toggle`, `workspace_skill_toggle`, `workspace_skill_batch_toggle`, `extension_batch_activation_v2`, `workspace_init`, and `workspace_mcp_restart` advertise the mutation control routes documented below. They are strict-gated by the mutation gate (a daemon configured without a bearer token rejects them with 401 `token_required`). Older daemons return `404`; pre-flight each tag before exposing the corresponding affordance. +`session_approval_mode_control`, `workspace_tool_toggle`, `workspace_skill_settings_toggle`, `workspace_skill_settings_batch_toggle`, `extension_batch_activation_v2`, `workspace_init`, and `workspace_mcp_restart` advertise the mutation control routes documented below. They are strict-gated by the mutation gate (a daemon configured without a bearer token rejects them with 401 `token_required`). Older daemons return `404`; pre-flight each tag before exposing the corresponding affordance. The settings-specific Skill tags replace the retired `workspace_skill_toggle` and `workspace_skill_batch_toggle` tags, whose catalog-validated behavior is a different contract; the route paths and request bodies did not change. `mcp_guardrails` (issue [#4175](https://github.com/QwenLM/qwen-code/issues/4175) PR 14) covers the MCP budget surface: the `clientCount` / `clientBudget` / `budgetMode` / `budgets[]` fields on `GET /workspace/mcp`, the `disabledReason` field on per-server cells, and the `--mcp-client-budget` / `--mcp-budget-mode` CLI flags. Older daemons omit the new fields entirely; SDK clients pre-flight this tag before relying on `budgets[]` semantics. The registry descriptor also carries `modes: ['warn', 'enforce']` for future feature-modes exposure — for now, clients infer mode from the snapshot's `budgetMode` field. Server refusal under `enforce` mode is deterministic by `Object.entries(mcpServers)` declaration order; a future scope-precedence layer (if qwen-code adopts one) would shift this to "lowest-precedence first" to mirror claude-code's `plugin < user < project < local` convention. @@ -2816,7 +2816,7 @@ SSE event (workspace-scoped): `tool_toggled` with `{toolName, enabled, originato #### `POST /workspace/skills/:name/enable` -Capability tag: `workspace_skill_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/:name/enable`. +Capability tag: `workspace_skill_settings_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/:name/enable`. Update the workspace Skill settings for a name without consulting the loaded Skill catalog. The trimmed request name is passed to persistence and returned in the response. Enabling a `skills.defaultDisabled` Skill adds a workspace `skills.enabled` opt-in; disabling removes that opt-in and adds a workspace `skills.disabled` entry. Existing entries for Skills that are no longer loaded are preserved, and duplicate/case-variant entries for the target are collapsed. Higher-scope settings remain authoritative for effective availability, but they do not prevent the workspace scope from recording or removing its own declaration. @@ -2853,7 +2853,7 @@ The mutation reuses the workspace-scoped `settings_changed` event for each chang #### `POST /workspace/skills/enable` -Capability tag: `workspace_skill_batch_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/enable`. +Capability tag: `workspace_skill_settings_batch_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/enable`. Update workspace Skill settings for up to 100 names in one request; the cap counts the raw `skillNames` entries before deduplication. Names are trimmed and deduplicated case-insensitively while preserving first-seen order and casing. The daemon does not consult the loaded Skill catalog. It persists all names in one locked settings write and refreshes active sessions once. Unexpected persistence or runtime-generation failures fail the whole request. diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 4face952f88..8c643fc8874 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -209,7 +209,7 @@ idle daemon returns `initialized: false` with an empty snapshot. Once a session is alive they switch to `initialized: true` and surface the real state. -To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_toggle` capability. To change several names, check `workspace_skill_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it persists every structurally valid name together and refreshes active ACP sessions once. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be configured before installation or while their Extension is inactive. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. Enabling a `skills.defaultDisabled` Skill writes an explicit opt-in to `skills.enabled`. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means the setting was saved while no ACP child was running; it will apply when the child starts. `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. +To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_settings_toggle` capability. To change several names, check `workspace_skill_settings_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it persists every structurally valid name together and refreshes active ACP sessions once. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be configured before installation or while their Extension is inactive. Their paths and request bodies are unchanged; the settings-specific capabilities replace the retired catalog-validated Skill toggle tags. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. Enabling a `skills.defaultDisabled` Skill writes an explicit opt-in to `skills.enabled`. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means the setting was saved while no ACP child was running; it will apply when the child starts. `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. `GET /workspace/env` and `GET /workspace/preflight` always answer with `initialized: true` regardless of ACP state. `env` never consults ACP diff --git a/integration-tests/cli/qwen-serve-routes.test.ts b/integration-tests/cli/qwen-serve-routes.test.ts index 084c1fe1a48..27ec6c5726b 100644 --- a/integration-tests/cli/qwen-serve-routes.test.ts +++ b/integration-tests/cli/qwen-serve-routes.test.ts @@ -370,8 +370,8 @@ describe('qwen serve — capabilities envelope', () => { 'workspace_file_upload', 'session_approval_mode_control', 'workspace_tool_toggle', - 'workspace_skill_toggle', - 'workspace_skill_batch_toggle', + 'workspace_skill_settings_toggle', + 'workspace_skill_settings_batch_toggle', 'extension_batch_activation_v2', 'workspace_skill_manage', 'workspace_settings', diff --git a/packages/cli/src/serve/capabilities.ts b/packages/cli/src/serve/capabilities.ts index d4b3460a3b6..fd8704108b1 100644 --- a/packages/cli/src/serve/capabilities.ts +++ b/packages/cli/src/serve/capabilities.ts @@ -188,8 +188,8 @@ export const SERVE_CAPABILITY_REGISTRY = { // unregistered — the toggle takes effect on the next ACP child spawn // (`tools.disabled` is consulted at `Config` construction time). workspace_tool_toggle: { since: 'v1' }, - workspace_skill_toggle: { since: 'v1' }, - workspace_skill_batch_toggle: { since: 'v1' }, + workspace_skill_settings_toggle: { since: 'v1' }, + workspace_skill_settings_batch_toggle: { since: 'v1' }, extension_batch_activation_v2: { since: 'v1' }, workspace_skill_manage: { since: 'v1' }, workspace_settings: { since: 'v1' }, diff --git a/packages/cli/src/serve/routes/workspace-skills.test.ts b/packages/cli/src/serve/routes/workspace-skills.test.ts index ccfc0d3f604..1d99309093e 100644 --- a/packages/cli/src/serve/routes/workspace-skills.test.ts +++ b/packages/cli/src/serve/routes/workspace-skills.test.ts @@ -173,7 +173,7 @@ describe('workspace Skill management routes', () => { expect(harness.deleteWorkspaceSkill).not.toHaveBeenCalled(); }); - it('forwards a deduplicated Skill batch response with legacy errors', async () => { + it('forwards a deduplicated Skill batch response', async () => { const harness = createHarness(); harness.setWorkspaceSkillsEnabled.mockResolvedValueOnce({ enabled: false, @@ -183,16 +183,9 @@ describe('workspace Skill management routes', () => { results: [ { skillName: 'review', enabled: false, changed: true }, { skillName: 'missing', enabled: false, changed: true }, + { skillName: 'locked', enabled: false, changed: true }, ], - errors: [ - { - skillName: 'locked', - code: 'skill_not_toggleable', - error: 'Skill locked is locked by user settings', - reason: 'locked', - lockedScope: 'user', - }, - ], + errors: [], }); const response = await request(harness.app) @@ -219,16 +212,13 @@ describe('workspace Skill management routes', () => { enabled: false, changed: true, }, - ], - errors: [ { skillName: 'locked', - code: 'skill_not_toggleable', - error: 'Skill locked is locked by user settings', - reason: 'locked', - lockedScope: 'user', + enabled: false, + changed: true, }, ], + errors: [], }); expect(harness.setWorkspaceSkillsEnabled).toHaveBeenCalledWith( expect.objectContaining({ diff --git a/packages/cli/src/serve/run-qwen-serve.ts b/packages/cli/src/serve/run-qwen-serve.ts index bd17a814a00..b06322d969c 100644 --- a/packages/cli/src/serve/run-qwen-serve.ts +++ b/packages/cli/src/serve/run-qwen-serve.ts @@ -2113,8 +2113,6 @@ async function loadServeRuntimeModules() { createDaemonWorkspaceService: workspaceModule.createDaemonWorkspaceService, WorkspaceSettingsPartialPersistError: workspaceTypesModule.WorkspaceSettingsPartialPersistError, - WorkspaceSkillNotToggleableError: - workspaceTypesModule.WorkspaceSkillNotToggleableError, createDaemonStatusProvider: daemonStatusProviderModule.createDaemonStatusProvider, createWorkspaceProvidersStatusProvider: diff --git a/packages/cli/src/serve/server.test.ts b/packages/cli/src/serve/server.test.ts index 49673a39b81..a595a1bef73 100644 --- a/packages/cli/src/serve/server.test.ts +++ b/packages/cli/src/serve/server.test.ts @@ -161,10 +161,7 @@ import { isValidSessionId } from '../config/config.js'; import type { DaemonLogger } from './daemon-logger.js'; import { FsError, type WorkspaceFileSystemFactory } from './fs/index.js'; import { getRateLimiter } from './rate-limit.js'; -import { - WorkspaceSkillNotToggleableError, - type DaemonWorkspaceService, -} from './workspace-service/types.js'; +import type { DaemonWorkspaceService } from './workspace-service/types.js'; import type { WorkspaceRegistrationStore } from './workspace-registration-store.js'; import { createWorkspaceGenerationGuard, @@ -613,8 +610,8 @@ const EXPECTED_STAGE1_FEATURES = [ // init scaffold, and MCP server restart). 'session_approval_mode_control', 'workspace_tool_toggle', - 'workspace_skill_toggle', - 'workspace_skill_batch_toggle', + 'workspace_skill_settings_toggle', + 'workspace_skill_settings_batch_toggle', 'extension_batch_activation_v2', 'workspace_skill_manage', 'workspace_permissions', @@ -23114,34 +23111,6 @@ describe('createServeApp', () => { ); }); - it('passes through a legacy persistence lock error', async () => { - const app = createServeApp(tokenOpts, undefined, { - bridge: fakeBridge({ - workspaceSkillsImpl: async () => ({ - v: 1, - workspaceCwd: WS_BOUND, - initialized: true, - skills: [reviewSkill], - }), - }), - persistDisabledSkills: vi - .fn() - .mockRejectedValue( - new WorkspaceSkillNotToggleableError('review', 'locked', 'user'), - ), - primaryWorkspaceTrusted: true, - }); - const res = await auth( - request(app).post('/workspace/skills/review/enable'), - ).send({ enabled: true }); - expect(res.status).toBe(409); - expect(res.body).toMatchObject({ - code: 'skill_not_toggleable', - reason: 'locked', - lockedScope: 'user', - }); - }); - it('rejects writes to an untrusted primary workspace', async () => { const app = createServeApp(tokenOpts, undefined, { bridge: fakeBridge(), diff --git a/packages/cli/src/serve/server/error-response.ts b/packages/cli/src/serve/server/error-response.ts index 523faf7f20e..ef157f107aa 100644 --- a/packages/cli/src/serve/server/error-response.ts +++ b/packages/cli/src/serve/server/error-response.ts @@ -257,9 +257,7 @@ export function sendBridgeError( } const skillError = mapWorkspaceSkillToggleError(err); if (skillError) { - res - .status(skillError.code === 'skill_not_found' ? 404 : 409) - .json(skillError); + res.status(404).json(skillError); return; } if (err instanceof InvalidSessionTranscriptCursorError) { diff --git a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts index 4a2e8974eec..7b70a0ffbd8 100644 --- a/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts +++ b/packages/cli/src/serve/workspace-service/__tests__/facade.test.ts @@ -115,7 +115,6 @@ import { import { WorkspaceVoiceError } from '../../../services/voice-service.js'; import { WorkspacePermissionRulesSessionRequiredError, - WorkspaceSkillNotToggleableError, WorkspaceSettingsPartialPersistError, } from '../types.js'; import type { @@ -2275,12 +2274,12 @@ describe('createDaemonWorkspaceService', () => { }); const persistDisabledSkillsBatch = vi.fn().mockResolvedValue({ outcomes: [ - { skillName: 'Review', changed: true }, - { skillName: 'missing', changed: true }, - { skillName: 'hidden', changed: true }, - { skillName: 'inactive', changed: true }, - { skillName: 'locked', changed: true }, { skillName: 'deploy', changed: true }, + { skillName: 'locked', changed: true }, + { skillName: 'inactive', changed: true }, + { skillName: 'hidden', changed: true }, + { skillName: 'missing', changed: true }, + { skillName: 'Review', changed: true }, ], settingsChanges: [ { @@ -2372,57 +2371,6 @@ describe('createDaemonWorkspaceService', () => { ); }); - it('orders results and legacy errors by request targets', async () => { - const svc = createDaemonWorkspaceService( - makeDeps({ - queryWorkspaceStatus: vi.fn().mockResolvedValue({ - v: 1, - workspaceCwd: '/workspace', - initialized: true, - skills, - }), - persistDisabledSkillsBatch: vi.fn().mockResolvedValue({ - outcomes: [ - { skillName: 'deploy', changed: true }, - { - skillName: 'locked', - error: new WorkspaceSkillNotToggleableError( - 'locked', - 'locked', - 'user', - ), - }, - { skillName: 'review', changed: true }, - { skillName: 'missing', changed: true }, - ], - settingsChanges: [], - }), - isChannelLive: () => false, - }), - ); - - const result = await svc.setWorkspaceSkillsEnabled( - makeCtx(), - ['review', 'locked', 'missing', 'deploy'], - false, - ); - - 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([ - { - skillName: 'locked', - code: 'skill_not_toggleable', - error: 'Skill locked is locked by user settings', - reason: 'locked', - lockedScope: 'user', - }, - ]); - }); - it('fails the whole batch when persistence fails unexpectedly', async () => { const invokeWorkspaceCommand = vi.fn(); const publishWorkspaceEvent = vi.fn(); diff --git a/packages/cli/src/serve/workspace-service/index.ts b/packages/cli/src/serve/workspace-service/index.ts index 68521a684bd..fb3939a2104 100644 --- a/packages/cli/src/serve/workspace-service/index.ts +++ b/packages/cli/src/serve/workspace-service/index.ts @@ -69,7 +69,6 @@ import { } from '../workspace-skill-management.js'; import { - mapWorkspaceSkillToggleError, WorkspacePermissionRulesSessionRequiredError, WorkspaceSkillNotFoundError, WorkspaceSettingsPartialPersistError, @@ -119,7 +118,6 @@ export type { export { WorkspacePermissionRulesSessionRequiredError, WorkspaceSkillNotFoundError, - WorkspaceSkillNotToggleableError, mapWorkspaceSkillToggleError, } from './types.js'; @@ -947,7 +945,6 @@ export function createDaemonWorkspaceService( ]), ); const results: WorkspaceSkillBatchToggleResult['results'] = []; - const errors: WorkspaceSkillBatchToggleResult['errors'] = []; for (const skillName of skillNames) { const outcome = persistedByName.get(skillName.toLowerCase()); if (!outcome) { @@ -955,17 +952,11 @@ export function createDaemonWorkspaceService( `Missing persisted Skill batch outcome: ${skillName}`, ); } - if ('error' in outcome) { - const error = mapWorkspaceSkillToggleError(outcome.error); - if (!error) throw outcome.error; - errors.push(error); - } else { - results.push({ - skillName: outcome.skillName, - enabled, - changed: outcome.changed, - }); - } + results.push({ + skillName: outcome.skillName, + enabled, + changed: outcome.changed, + }); } const changed = results.some((result) => result.changed); @@ -1033,7 +1024,7 @@ export function createDaemonWorkspaceService( sessionsRefreshed, sessionsFailed, results, - errors, + errors: [], }; }, diff --git a/packages/cli/src/serve/workspace-service/types.ts b/packages/cli/src/serve/workspace-service/types.ts index f17c6afcd43..1d361ed2f41 100644 --- a/packages/cli/src/serve/workspace-service/types.ts +++ b/packages/cli/src/serve/workspace-service/types.ts @@ -349,17 +349,12 @@ export interface WorkspaceSkillToggleResult { sessionsFailed: number; } -export type WorkspaceSkillToggleErrorCode = - | 'skill_not_found' - | 'skill_not_toggleable' - | 'skill_inactive_extension'; +export type WorkspaceSkillToggleErrorCode = 'skill_not_found'; export interface WorkspaceSkillToggleError { skillName: string; code: WorkspaceSkillToggleErrorCode; error: string; - reason?: WorkspaceSkillNotToggleableReason; - lockedScope?: 'system' | 'user' | 'systemDefaults'; } export interface WorkspaceSkillBatchToggleItem { @@ -386,9 +381,10 @@ export interface PersistDisabledSkillResult { }>; } -export type PersistDisabledSkillsBatchOutcome = - | { skillName: string; changed: boolean } - | { skillName: string; error: WorkspaceSkillNotToggleableError }; +export interface PersistDisabledSkillsBatchOutcome { + skillName: string; + changed: boolean; +} export interface PersistDisabledSkillsBatchResult { outcomes: PersistDisabledSkillsBatchOutcome[]; @@ -398,11 +394,6 @@ export interface PersistDisabledSkillsBatchResult { }>; } -export type WorkspaceSkillNotToggleableReason = - | 'not_user_invocable' - | 'inactive_extension' - | 'locked'; - export class WorkspaceSkillNotFoundError extends Error { constructor(readonly skillName: string) { super(`Skill not found: ${skillName}`); @@ -410,21 +401,6 @@ export class WorkspaceSkillNotFoundError extends Error { } } -export class WorkspaceSkillNotToggleableError extends Error { - constructor( - readonly skillName: string, - readonly reason: WorkspaceSkillNotToggleableReason, - readonly lockedScope?: 'system' | 'user' | 'systemDefaults', - ) { - super( - lockedScope - ? `Skill ${skillName} is locked by ${lockedScope} settings` - : `Skill ${skillName} is not toggleable: ${reason}`, - ); - this.name = 'WorkspaceSkillNotToggleableError'; - } -} - export function mapWorkspaceSkillToggleError( error: unknown, ): WorkspaceSkillToggleError | undefined { @@ -435,18 +411,6 @@ export function mapWorkspaceSkillToggleError( error: error.message, }; } - if (error instanceof WorkspaceSkillNotToggleableError) { - return { - skillName: error.skillName, - code: - error.reason === 'inactive_extension' - ? 'skill_inactive_extension' - : 'skill_not_toggleable', - error: error.message, - reason: error.reason, - ...(error.lockedScope ? { lockedScope: error.lockedScope } : {}), - }; - } return undefined; } diff --git a/packages/sdk-typescript/src/daemon/DaemonClient.ts b/packages/sdk-typescript/src/daemon/DaemonClient.ts index 7a347a17f25..79e3f8bc36e 100644 --- a/packages/sdk-typescript/src/daemon/DaemonClient.ts +++ b/packages/sdk-typescript/src/daemon/DaemonClient.ts @@ -3632,7 +3632,8 @@ export class DaemonClient { * Active ACP sessions refresh their skill validation and command lists before * the response returns; `activation` reports deferred or partial refreshes. * - * Pre-flight `caps.features.includes('workspace_skill_toggle')` before calling. + * Pre-flight + * `caps.features.includes('workspace_skill_settings_toggle')` before calling. */ async setWorkspaceSkillEnabled( skillName: string, @@ -3665,7 +3666,8 @@ export class DaemonClient { * Update workspace Skill settings for up to 100 names in one write. * * Pre-flight - * `caps.features.includes('workspace_skill_batch_toggle')` before calling. + * `caps.features.includes('workspace_skill_settings_batch_toggle')` before + * calling. */ async setWorkspaceSkillsEnabled( skillNames: readonly string[], diff --git a/packages/sdk-typescript/test/unit/DaemonClient.test.ts b/packages/sdk-typescript/test/unit/DaemonClient.test.ts index 6fdadbf6839..4f47dd3b34b 100644 --- a/packages/sdk-typescript/test/unit/DaemonClient.test.ts +++ b/packages/sdk-typescript/test/unit/DaemonClient.test.ts @@ -346,7 +346,7 @@ describe('DaemonClient', () => { supported: ['v1'], }, mode: 'http-bridge' as const, - features: ['health', 'capabilities', 'workspace_skill_toggle'], + features: ['health', 'capabilities', 'workspace_skill_settings_toggle'], modelServices: [], workspaceCwd: '/work/bound', }; @@ -354,7 +354,7 @@ describe('DaemonClient', () => { const client = new DaemonClient({ baseUrl: 'http://daemon', fetch }); const caps = await client.capabilities(); expect(caps).toEqual(envelope); - expect(caps.features).toContain('workspace_skill_toggle'); + expect(caps.features).toContain('workspace_skill_settings_toggle'); // #3803 §02: clients use `workspaceCwd` to pre-flight check + // omit `cwd` from `POST /session` (route falls back). expect(caps.workspaceCwd).toBe('/work/bound'); diff --git a/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx b/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx index d633c363cfc..22e648585f1 100644 --- a/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx +++ b/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx @@ -29,7 +29,7 @@ const { skillsState, workspaceState } = vi.hoisted(() => ({ workspaceState: { current: { capabilities: { - features: ['workspace_skill_toggle'], + features: ['workspace_skill_settings_toggle'], }, }, }, @@ -118,6 +118,9 @@ beforeEach(() => { skillsState.current.setEnabled.mockReset().mockResolvedValue(undefined); skillsState.current.install.mockReset(); skillsState.current.remove.mockReset(); + workspaceState.current.capabilities.features = [ + 'workspace_skill_settings_toggle', + ]; }); afterEach(() => { @@ -126,6 +129,39 @@ afterEach(() => { }); describe('SkillsManagerPage', () => { + it('does not treat the retired Skill toggle capability as settings support', async () => { + workspaceState.current.capabilities.features = ['workspace_skill_toggle']; + skillsState.current.skills = [ + { + kind: 'skill', + status: 'disabled', + name: 'review', + description: 'Review code', + level: 'user', + modelInvocable: true, + disabledReason: 'default', + }, + ]; + + await renderPage(); + await openDisabledSkill('review'); + + const actions = container.querySelector( + '[data-testid="skill-actions"]', + ); + expect(actions).not.toBeNull(); + await act(async () => { + actions!.dispatchEvent( + new MouseEvent('pointerdown', { bubbles: true, button: 0 }), + ); + }); + + const enable = Array.from( + document.body.querySelectorAll('[role="menuitem"]'), + ).find((item) => item.textContent?.trim() === 'Enable'); + expect(enable?.hasAttribute('data-disabled')).toBe(true); + }); + it('shows the authoritative enabled state after a normal toggle', async () => { const disabledSkill: DaemonWorkspaceSkillStatus = { kind: 'skill', diff --git a/packages/web-shell/client/components/skills/SkillsManagerPage.tsx b/packages/web-shell/client/components/skills/SkillsManagerPage.tsx index f648bce30ba..5685f935437 100644 --- a/packages/web-shell/client/components/skills/SkillsManagerPage.tsx +++ b/packages/web-shell/client/components/skills/SkillsManagerPage.tsx @@ -173,8 +173,9 @@ export function SkillsManagerPage({ remove, } = useSkills({ autoLoad: true }); const canToggleSkills = - workspace.capabilities?.features.includes('workspace_skill_toggle') === - true; + workspace.capabilities?.features.includes( + 'workspace_skill_settings_toggle', + ) === true; const canManageSkills = workspace.capabilities?.features.includes('workspace_skill_manage') === true; From 0fb288b22ee15af6eaf5049e4f7955f85ad67e23 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Wed, 26 Aug 2026 20:23:28 +0800 Subject: [PATCH 05/10] fix(serve): clarify skill settings outcomes --- docs/design/daemon-skill-toggle.md | 2 +- .../daemon/11-capabilities-versioning.md | 4 +-- docs/developers/qwen-serve-protocol.md | 2 +- .../skills/SkillsManagerPage.test.tsx | 35 +++++++++++++++---- .../components/skills/SkillsManagerPage.tsx | 14 ++++---- packages/web-shell/client/i18n.tsx | 3 ++ 6 files changed, 44 insertions(+), 16 deletions(-) diff --git a/docs/design/daemon-skill-toggle.md b/docs/design/daemon-skill-toggle.md index 88c77c7ec8a..640646e67be 100644 --- a/docs/design/daemon-skill-toggle.md +++ b/docs/design/daemon-skill-toggle.md @@ -18,7 +18,7 @@ The response contains the trimmed requested name, requested state, whether persi The API changes workspace `skills.disabled` and `skills.enabled` by name without consulting the runtime Skill catalog. Enabling a default-disabled Skill writes an explicit opt-in; disabling it removes the opt-in and writes a hard workspace disable. Updating one target removes target duplicates and case variants without deleting orphan entries for unavailable Skills. Names may be configured before installation, while hidden from user invocation, or while their Extension is inactive. A second identical request is a no-op. -Higher-scope settings still determine effective availability after settings merge, but do not prevent workspace scope from recording or removing its own declaration. The route retains request-shape, authentication, client identity, workspace trust, and runtime-generation gates; none of those require a Skill catalog lookup. +A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not prevent workspace scope from recording or removing its own declaration. Workspace declarations otherwise participate in the usual `skills.disabled > skills.enabled > skills.defaultDisabled` resolution and can override higher-scope `skills.defaultDisabled` or `skills.enabled` entries. The route retains request-shape, authentication, client identity, workspace trust, and runtime-generation gates; none of those require a Skill catalog lookup. The workspace read-modify-write happens inside the daemon's per-workspace settings lock. A failed write stops before refresh and event publication. diff --git a/docs/developers/daemon/11-capabilities-versioning.md b/docs/developers/daemon/11-capabilities-versioning.md index d48872fc3f0..48dcbdb94d5 100644 --- a/docs/developers/daemon/11-capabilities-versioning.md +++ b/docs/developers/daemon/11-capabilities-versioning.md @@ -124,7 +124,7 @@ V2 Extension batch activation: `extension_batch_activation_v2` adds queued globa Workspace-qualified session reads: `workspace_persisted_transcript`, `workspace_session_export`, `workspace_archived_session_export`, `workspace_session_live_state`. The active and archived export tags are independent from each other and from `session_export` and `workspace_qualified_rest_core`, so clients must pre-flight the exact storage state they intend to export. Persisted transcript paging permits an untrusted secondary under its bounded read policy; both full export paths remain trusted-only. `workspace_session_live_state` is likewise independent from `workspace_qualified_rest_core` and is trusted-only: it serves the selected runtime's memory-only live-session snapshot and catalog version and does not extend the untrusted-secondary persisted read policy to live bridge state. -Workspace mutation (Wave 4+): `workspace_memory`, `workspace_agents`, `workspace_agent_generate`, `workspace_acp_preheat`, `workspace_tool_toggle`, `workspace_skill_settings_toggle`, `workspace_skill_settings_batch_toggle`, **`workspace_settings`** (conditional), `workspace_permissions`, `workspace_init`, `workspace_github_setup`, `workspace_trust`, `workspace_mcp_restart`, `workspace_mcp_manage`, `workspace_file_read`, `workspace_file_bytes`, `workspace_file_read_cursor`, `workspace_file_write`, `workspace_file_upload`, **`workspace_reload`** (conditional). +Workspace mutation (Wave 4+): `workspace_memory`, `workspace_agents`, `workspace_agent_generate`, `workspace_acp_preheat`, `workspace_tool_toggle`, `workspace_skill_settings_toggle`, `workspace_skill_settings_batch_toggle`, **`workspace_settings`** (conditional), `workspace_permissions`, `workspace_init`, `workspace_github_setup`, `workspace_trust`, `workspace_mcp_restart`, `workspace_mcp_manage`, `workspace_file_read`, `workspace_file_bytes`, `workspace_file_read_cursor`, `workspace_file_write`, `workspace_file_upload`, **`workspace_reload`** (conditional). The two Skill settings tags replace the retired catalog-validated `workspace_skill_toggle` and `workspace_skill_batch_toggle` tags. MCP guardrails: **`mcp_guardrails`** (`modes: ['warn', 'enforce']`), `mcp_guardrail_events`, `mcp_server_runtime_mutation`, **`mcp_workspace_pool`** (conditional), **`mcp_pool_restart`** (conditional). @@ -185,7 +185,7 @@ sequenceDiagram ## State and lifecycle - `CAPABILITIES_SCHEMA_VERSION` is the wire envelope shape version, currently `1`. Bump it only for an envelope break. -- `SERVE_PROTOCOL_VERSION = 'v1'` is the protocol-feature version. Adding features inside v1 is additive; old clients do not see new behavior unless they preflight the new tag. Removing a feature is a v2 break. +- `SERVE_PROTOCOL_VERSION = 'v1'` is the protocol-feature version. Adding features inside v1 is additive; old clients do not see new behavior unless they preflight the new tag. Corrected behavior may replace a capability inside v1: the replacement tag supersedes the old tag, the old tag stops being advertised, and clients must preflight the replacement. Removing a feature without a replacement is a v2 break. - `EVENT_SCHEMA_VERSION = 1` is the SSE frame `v` field (see [`09-event-schema.md`](./09-event-schema.md)). It is an independent version axis; bumping event schema does not imply bumping protocol version, and vice versa. - `session_resume` is the stable daemon capability for `POST /session/:id/resume`. `unstable_session_resume` remains advertised as a deprecated alias because the underlying ACP method is still named `connection.unstable_resumeSession`; new clients should feature-detect `session_resume`. diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index f57bbf67d32..14b053fe179 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -2818,7 +2818,7 @@ SSE event (workspace-scoped): `tool_toggled` with `{toolName, enabled, originato Capability tag: `workspace_skill_settings_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/:name/enable`. -Update the workspace Skill settings for a name without consulting the loaded Skill catalog. The trimmed request name is passed to persistence and returned in the response. Enabling a `skills.defaultDisabled` Skill adds a workspace `skills.enabled` opt-in; disabling removes that opt-in and adds a workspace `skills.disabled` entry. Existing entries for Skills that are no longer loaded are preserved, and duplicate/case-variant entries for the target are collapsed. Higher-scope settings remain authoritative for effective availability, but they do not prevent the workspace scope from recording or removing its own declaration. +Update the workspace Skill settings for a name without consulting the loaded Skill catalog. The trimmed request name is passed to persistence and returned in the response. Enabling a `skills.defaultDisabled` Skill adds a workspace `skills.enabled` opt-in; disabling removes that opt-in and adds a workspace `skills.disabled` entry. Existing entries for Skills that are no longer loaded are preserved, and duplicate/case-variant entries for the target are collapsed. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not prevent the workspace scope from recording or removing its own declaration. Workspace declarations otherwise participate in the usual `skills.disabled > skills.enabled > skills.defaultDisabled` resolution and can override higher-scope `skills.defaultDisabled` or `skills.enabled` entries. This is different from the ACP `qwen/skills/setEnabled` managed-skill operation and the `disable-model-invocation` frontmatter field. Effective skill availability follows `skills.disabled` > `skills.enabled` > `skills.defaultDisabled`. Both hard and default disables remove the skill from slash-command/model availability and reject later skill execution. `disable-model-invocation: true` keeps direct user invocation available and only hides the skill from model invocation. diff --git a/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx b/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx index 22e648585f1..1a8e558ba28 100644 --- a/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx +++ b/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx @@ -115,7 +115,9 @@ beforeEach(() => { skillsState.current.loading = false; skillsState.current.error = undefined; skillsState.current.reload.mockReset(); - skillsState.current.setEnabled.mockReset().mockResolvedValue(undefined); + skillsState.current.setEnabled.mockReset().mockResolvedValue({ + changed: true, + }); skillsState.current.install.mockReset(); skillsState.current.remove.mockReset(); workspaceState.current.capabilities.features = [ @@ -200,6 +202,9 @@ describe('SkillsManagerPage', () => { it.each([ { label: 'higher-scope locked', + changed: false, + notice: + 'Skill already has the requested workspace setting; no setting was changed.', skill: { kind: 'skill' as const, status: 'disabled' as const, @@ -213,6 +218,9 @@ describe('SkillsManagerPage', () => { }, { label: 'inactive Extension', + changed: false, + notice: + 'Skill already has the requested workspace setting; no setting was changed.', skill: { kind: 'skill' as const, status: 'disabled' as const, @@ -224,10 +232,27 @@ describe('SkillsManagerPage', () => { disabledReason: 'inactive_extension' as const, }, }, + { + label: 'changed default-disabled and higher-scope locked', + changed: true, + notice: + 'Workspace setting updated. Effective Skill availability did not change.', + skill: { + kind: 'skill' as const, + status: 'disabled' as const, + name: 'default-locked', + description: 'Default-disabled and locked by user settings', + level: 'bundled' as const, + modelInvocable: true, + disabledReason: 'hard' as const, + lockedScope: 'user' as const, + }, + }, ])( - 'keeps a $label Skill disabled after its workspace setting is enabled', - async ({ skill }) => { + 'reports the workspace result for a $label Skill that stays disabled', + async ({ skill, changed, notice }) => { skillsState.current.skills = [skill]; + skillsState.current.setEnabled.mockResolvedValueOnce({ changed }); skillsState.current.reload.mockResolvedValue({ v: 1, workspaceCwd: '/workspace/demo', @@ -249,9 +274,7 @@ describe('SkillsManagerPage', () => { expect(skillsState.current.reload).toHaveBeenCalledTimes(1); skillsState.current.skills = [{ ...skill }]; await renderPage(); - expect(container.textContent).toContain( - 'Workspace setting updated. Effective Skill availability did not change.', - ); + expect(container.textContent).toContain(notice); expect(container.textContent).toContain('disabled'); expect(runButton()?.disabled).toBe(true); }, diff --git a/packages/web-shell/client/components/skills/SkillsManagerPage.tsx b/packages/web-shell/client/components/skills/SkillsManagerPage.tsx index 5685f935437..50c341e9655 100644 --- a/packages/web-shell/client/components/skills/SkillsManagerPage.tsx +++ b/packages/web-shell/client/components/skills/SkillsManagerPage.tsx @@ -230,7 +230,7 @@ export function SkillsManagerPage({ setBusySkill(skill.name); setNotice(null); try { - await setEnabled(skill.name, enabled); + const result = await setEnabled(skill.name, enabled); const refreshed = await reload(); const refreshedSkill = refreshed?.skills.find( (item) => item.name.toLowerCase() === skill.name.toLowerCase(), @@ -238,11 +238,13 @@ export function SkillsManagerPage({ const expectedStatus = enabled ? 'ok' : 'disabled'; setNotice({ skillName: skill.name, - text: !refreshedSkill - ? t('skills.settingUpdated') - : refreshedSkill.status === expectedStatus - ? t(enabled ? 'skills.enabled' : 'skills.disabled') - : t('skills.settingUpdatedAvailabilityUnchanged'), + text: !result.changed + ? t('skills.settingUnchanged') + : !refreshedSkill + ? t('skills.settingUpdated') + : refreshedSkill.status === expectedStatus + ? t(enabled ? 'skills.enabled' : 'skills.disabled') + : t('skills.settingUpdatedAvailabilityUnchanged'), error: false, }); } catch (toggleError) { diff --git a/packages/web-shell/client/i18n.tsx b/packages/web-shell/client/i18n.tsx index a4617755faa..da3c74bd7b7 100644 --- a/packages/web-shell/client/i18n.tsx +++ b/packages/web-shell/client/i18n.tsx @@ -2367,6 +2367,8 @@ const EN: Messages = { 'skills.settingUpdated': 'Workspace setting updated.', 'skills.settingUpdatedAvailabilityUnchanged': 'Workspace setting updated. Effective Skill availability did not change.', + 'skills.settingUnchanged': + 'Skill already has the requested workspace setting; no setting was changed.', 'skills.status': 'Status', 'skills.status.disabled': 'disabled', 'skills.status.enabled': 'enabled', @@ -5297,6 +5299,7 @@ const ZH: Messages = { 'skills.settingUpdated': 'Workspace 设置已更新。', 'skills.settingUpdatedAvailabilityUnchanged': 'Workspace 设置已更新，Skill 的实际可用状态未改变。', + 'skills.settingUnchanged': 'Skill 已处于请求的 Workspace 设置，无需更改。', 'skills.status': '状态', 'skills.status.disabled': '已禁用', 'skills.status.enabled': '已启用', From 41752966fd4481693f24ef57c1d00733d4675838 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Wed, 26 Aug 2026 23:44:27 +0800 Subject: [PATCH 06/10] docs(serve): clarify skill settings contracts --- docs/developers/daemon/13-sdk-daemon-client.md | 2 +- docs/developers/qwen-serve-protocol.md | 6 +++--- docs/users/qwen-serve.md | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/developers/daemon/13-sdk-daemon-client.md b/docs/developers/daemon/13-sdk-daemon-client.md index a7fa6bb4b3d..e8cc5bd8195 100644 --- a/docs/developers/daemon/13-sdk-daemon-client.md +++ b/docs/developers/daemon/13-sdk-daemon-client.md @@ -165,7 +165,7 @@ await client .setWorkspaceSkillsEnabled(['review', 'deploy'], true); ``` -`DaemonSkillBatchToggleResult` contains ordered `results`, a compatibility `errors` array, and batch-level activation/session-refresh counts. Current daemons persist every structurally valid name together, refresh active sessions once, and return an empty `errors` array without consulting the loaded Skill catalog. The error item types remain available so the SDK can still decode responses from older daemons. The method throws on a non-200 response. +`DaemonSkillBatchToggleResult` contains ordered `results`, a compatibility `errors` array, and batch-level activation/session-refresh counts. Current daemons process every structurally valid name in request order, persist all resulting declaration changes together in at most one locked settings write, refresh active sessions once, and return an empty `errors` array without consulting the loaded Skill catalog. Enabling a name with no existing workspace declaration and no effective `skills.defaultDisabled` entry returns `changed: false` and performs no write. The error item types remain available so the SDK can still decode responses from older daemons. The method throws on a non-200 response. V2 Extension batch activation retains the asynchronous Extension operation model. Pre-flight `extension_batch_activation_v2`, submit a global default batch or a selected-workspace override batch, then poll it with the existing operation helper: diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 14b053fe179..092010fc07a 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -270,7 +270,7 @@ registry. Clients **must** gate UI off `features`, not off `mode` (per design `session_info` advertises `GET /workspace/:id/session-info` and its `/workspaces/:workspace/session-info` twin. The response aggregates persisted active and archived session counts without hydrating list metadata. It is an explicit O(n) disk scan and must not be polled; clients should treat `truncated: true` as a lower-bound result. -`session_approval_mode_control`, `workspace_tool_toggle`, `workspace_skill_settings_toggle`, `workspace_skill_settings_batch_toggle`, `extension_batch_activation_v2`, `workspace_init`, and `workspace_mcp_restart` advertise the mutation control routes documented below. They are strict-gated by the mutation gate (a daemon configured without a bearer token rejects them with 401 `token_required`). Older daemons return `404`; pre-flight each tag before exposing the corresponding affordance. The settings-specific Skill tags replace the retired `workspace_skill_toggle` and `workspace_skill_batch_toggle` tags, whose catalog-validated behavior is a different contract; the route paths and request bodies did not change. +`session_approval_mode_control`, `workspace_tool_toggle`, `workspace_skill_settings_toggle`, `workspace_skill_settings_batch_toggle`, `extension_batch_activation_v2`, `workspace_init`, and `workspace_mcp_restart` advertise the mutation control routes documented below. They are strict-gated by the mutation gate (a daemon configured without a bearer token rejects them with 401 `token_required`). Daemons that lack one of these routes return `404`. The settings-specific Skill tags are different: older daemons advertise the retired `workspace_skill_toggle` and `workspace_skill_batch_toggle` tags and serve their catalog-validated contract at the same paths, where a target can return `404 skill_not_found` or `409 skill_not_toggleable`. Pre-flight each tag before exposing its affordance, and do not infer the settings-specific Skill contract by probing route reachability. The route paths and request bodies did not change. `mcp_guardrails` (issue [#4175](https://github.com/QwenLM/qwen-code/issues/4175) PR 14) covers the MCP budget surface: the `clientCount` / `clientBudget` / `budgetMode` / `budgets[]` fields on `GET /workspace/mcp`, the `disabledReason` field on per-server cells, and the `--mcp-client-budget` / `--mcp-budget-mode` CLI flags. Older daemons omit the new fields entirely; SDK clients pre-flight this tag before relying on `budgets[]` semantics. The registry descriptor also carries `modes: ['warn', 'enforce']` for future feature-modes exposure — for now, clients infer mode from the snapshot's `budgetMode` field. Server refusal under `enforce` mode is deterministic by `Object.entries(mcpServers)` declaration order; a future scope-precedence layer (if qwen-code adopts one) would shift this to "lowest-precedence first" to mirror claude-code's `plugin < user < project < local` convention. @@ -2745,7 +2745,7 @@ The daemon exposes five mutation control routes that let remote clients change r - Are gated by the **strict** mutation gate from PR 15. A daemon configured without a bearer token rejects them with `401 {code: 'token_required'}`. Configure `--token` (or `QWEN_SERVER_TOKEN`) before opting in. - Accept and stamp the `X-Qwen-Client-Id` header (PR 7 audit chain). When the header carries a trusted id, the daemon emits `originatorClientId` on the corresponding SSE event so cross-client UIs can suppress echoes of their own mutations. -- Pre-flight each per-tag capability before exposing the affordance. Older daemons return `404` for the route. +- Pre-flight each per-tag capability before exposing the affordance. A daemon that lacks a route returns `404`. For the Skill settings routes, tag absence can instead mean that the same path serves the retired catalog-validated contract, including per-target `404 skill_not_found` and `409 skill_not_toggleable` responses; do not use route probing as the version check. The tool toggle, skill toggle, init, and MCP restart routes emit **workspace-scoped** events: every active session SSE bus receives the event, regardless of which session was attached when the mutation was triggered. `approval-mode` emits a **session-scoped** event because the change is local to one session's `Config`. @@ -2855,7 +2855,7 @@ The mutation reuses the workspace-scoped `settings_changed` event for each chang Capability tag: `workspace_skill_settings_batch_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/enable`. -Update workspace Skill settings for up to 100 names in one request; the cap counts the raw `skillNames` entries before deduplication. Names are trimmed and deduplicated case-insensitively while preserving first-seen order and casing. The daemon does not consult the loaded Skill catalog. It persists all names in one locked settings write and refreshes active sessions once. Unexpected persistence or runtime-generation failures fail the whole request. +Update workspace Skill settings for up to 100 names in one request; the cap counts the raw `skillNames` entries before deduplication. Names are trimmed and deduplicated case-insensitively while preserving first-seen order and casing. The daemon does not consult the loaded Skill catalog. It applies all resulting declaration changes in at most one locked settings write and refreshes active sessions once. Enabling a name with no existing workspace declaration and no effective `skills.defaultDisabled` entry is a no-op (`changed: false`) and performs no write. Unexpected persistence or runtime-generation failures fail the whole request. Request: diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 8c643fc8874..34d564ee42f 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -209,7 +209,7 @@ idle daemon returns `initialized: false` with an empty snapshot. Once a session is alive they switch to `initialized: true` and surface the real state. -To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_settings_toggle` capability. To change several names, check `workspace_skill_settings_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it persists every structurally valid name together and refreshes active ACP sessions once. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be configured before installation or while their Extension is inactive. Their paths and request bodies are unchanged; the settings-specific capabilities replace the retired catalog-validated Skill toggle tags. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. Enabling a `skills.defaultDisabled` Skill writes an explicit opt-in to `skills.enabled`. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means the setting was saved while no ACP child was running; it will apply when the child starts. `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. +To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_settings_toggle` capability. To change several names, check `workspace_skill_settings_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it processes every structurally valid name together, persists all resulting declaration changes in at most one locked settings write, and refreshes active ACP sessions once. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be disabled before installation or while their Extension is inactive. Enabling a name before installation writes an explicit `skills.enabled` opt-in only when an effective `skills.defaultDisabled` entry applies; with no existing workspace declaration and no such entry, enable is a no-op (`changed: false`). Their paths and request bodies are unchanged; the settings-specific capabilities replace the retired catalog-validated Skill toggle tags. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means a declaration changed while no ACP child was running; it will apply when the child starts. `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. `GET /workspace/env` and `GET /workspace/preflight` always answer with `initialized: true` regardless of ACP state. `env` never consults ACP From aa9a4d960a670cd07fca8b19f7b9b7f4895edaad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Wed, 26 Aug 2026 23:51:01 +0800 Subject: [PATCH 07/10] docs(serve): align skill toggle design semantics --- docs/design/daemon-skill-batch-toggle.md | 24 +++++++++++++----------- docs/design/daemon-skill-toggle.md | 18 ++++++++++-------- docs/developers/qwen-serve-protocol.md | 4 ++-- 3 files changed, 25 insertions(+), 21 deletions(-) diff --git a/docs/design/daemon-skill-batch-toggle.md b/docs/design/daemon-skill-batch-toggle.md index fd0b1356abe..4ffdf6feced 100644 --- a/docs/design/daemon-skill-batch-toggle.md +++ b/docs/design/daemon-skill-batch-toggle.md @@ -4,8 +4,9 @@ Remote Skill managers need both single and batch mutations to behave like workspace settings writes. A runtime Skill snapshot is not an ownership source -for `skills.disabled` or `skills.enabled`: entries may be declared before -installation and may intentionally outlive the currently loaded catalog. +for `skills.disabled` or `skills.enabled`: disabled entries and applicable +default-disabled opt-ins may be declared before installation and may +intentionally outlive the currently loaded catalog. ## API @@ -25,15 +26,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 daemon does not read or validate against runtime Skill status. Every name -is persisted in one locked write, and changes are applied with one live-session -refresh. 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`. Unknown, non-user-invocable, -inactive-Extension, and higher-scope-disabled names use the same settings path. -Higher scopes still determine effective availability after settings merge, but -do not prevent the workspace scope from recording its own declaration. -Unexpected persistence and runtime-generation failures fail the whole request. +The daemon does not read or validate against runtime Skill status. It applies +all resulting declaration changes in at most one locked settings write and, +when anything changed, performs one live-session refresh. 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`. Unknown, non-user-invocable, inactive-Extension, and +higher-scope-disabled names use the same settings path. Higher scopes still +determine effective availability after settings merge, but do not prevent the +workspace scope from recording its own declaration. Unexpected persistence and +runtime-generation failures fail the whole request. ```json { diff --git a/docs/design/daemon-skill-toggle.md b/docs/design/daemon-skill-toggle.md index 640646e67be..bc747bfdb8f 100644 --- a/docs/design/daemon-skill-toggle.md +++ b/docs/design/daemon-skill-toggle.md @@ -16,7 +16,7 @@ The response contains the trimmed requested name, requested state, whether persi ## Semantics -The API changes workspace `skills.disabled` and `skills.enabled` by name without consulting the runtime Skill catalog. Enabling a default-disabled Skill writes an explicit opt-in; disabling it removes the opt-in and writes a hard workspace disable. Updating one target removes target duplicates and case variants without deleting orphan entries for unavailable Skills. Names may be configured before installation, while hidden from user invocation, or while their Extension is inactive. A second identical request is a no-op. +The API changes workspace `skills.disabled` and `skills.enabled` by name without consulting the runtime Skill catalog. Enabling a default-disabled Skill writes an explicit opt-in; disabling it removes the opt-in and writes a hard workspace disable. Updating one target removes target duplicates and case variants without deleting orphan entries for unavailable Skills. A name may be disabled before installation, while hidden from user invocation, or while its Extension is inactive. Enabling removes an existing workspace disable or records an opt-in for an effective `skills.defaultDisabled` entry; with neither condition, it is a no-op. A second identical request is also a no-op. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not prevent workspace scope from recording or removing its own declaration. Workspace declarations otherwise participate in the usual `skills.disabled > skills.enabled > skills.defaultDisabled` resolution and can override higher-scope `skills.defaultDisabled` or `skills.enabled` entries. The route retains request-shape, authentication, client identity, workspace trust, and runtime-generation gates; none of those require a Skill catalog lookup. @@ -31,12 +31,13 @@ The workspace read-modify-write happens inside the daemon's per-workspace settin ## Activation flow 1. Validate the request name, authorization, workspace trust, client identity, and runtime generation. -2. Under the workspace settings lock, re-read every scope and commit the requested name to the workspace list. -3. Invalidate the daemon's cached skill status. -4. If an ACP child is live, invoke `qwen/control/workspace/skills/refresh`. -5. The child reloads workspace-scope settings and refreshes every active session, including busy sessions. -6. Each session reloads its own workspace settings, rebuilds and pushes `available_commands_update`, and notifies SkillManager consumers. -7. Publish the existing workspace `settings_changed` event for each changed skill-settings key. +2. Under the workspace settings lock, re-read every scope, compute the resulting workspace declaration changes, and commit them in at most one write. +3. If no declaration changed, return `changed: false` without cache invalidation, refresh, or event publication. +4. Otherwise, invalidate the daemon's cached skill status. +5. If an ACP child is live, invoke `qwen/control/workspace/skills/refresh`. +6. The child reloads workspace-scope settings and refreshes every active session, including busy sessions. +7. Each session reloads its own workspace settings, rebuilds and pushes `available_commands_update`, and notifies SkillManager consumers. +8. Publish the existing workspace `settings_changed` event for each changed skill-settings key. An in-flight model request cannot be rewritten. Subsequent skill execution checks, command snapshots, and model contexts read the new state. @@ -55,6 +56,7 @@ An in-flight model request cannot be rewritten. Subsequent skill execution check ## Failure behavior - Persistence failure: the HTTP request fails; no ACP refresh and no event. -- No child: persistence succeeds with `deferred`; the next child loads the setting at startup. +- No child after a declaration changed: persistence succeeds with `deferred`; the next child loads the setting at startup. +- No declaration change: the response reports `changed: false`; no refresh or event occurs. `activation` still reflects whether a child was live, but no activation work is needed. - Per-session refresh failure: persistence remains committed; successful sessions stay refreshed and the response is `partial`. - Child transport race: if the child disappears after the liveness check, the response is `deferred`; other refresh failures are reported as `partial`. diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 092010fc07a..5902c880fd5 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -270,7 +270,7 @@ registry. Clients **must** gate UI off `features`, not off `mode` (per design `session_info` advertises `GET /workspace/:id/session-info` and its `/workspaces/:workspace/session-info` twin. The response aggregates persisted active and archived session counts without hydrating list metadata. It is an explicit O(n) disk scan and must not be polled; clients should treat `truncated: true` as a lower-bound result. -`session_approval_mode_control`, `workspace_tool_toggle`, `workspace_skill_settings_toggle`, `workspace_skill_settings_batch_toggle`, `extension_batch_activation_v2`, `workspace_init`, and `workspace_mcp_restart` advertise the mutation control routes documented below. They are strict-gated by the mutation gate (a daemon configured without a bearer token rejects them with 401 `token_required`). Daemons that lack one of these routes return `404`. The settings-specific Skill tags are different: older daemons advertise the retired `workspace_skill_toggle` and `workspace_skill_batch_toggle` tags and serve their catalog-validated contract at the same paths, where a target can return `404 skill_not_found` or `409 skill_not_toggleable`. Pre-flight each tag before exposing its affordance, and do not infer the settings-specific Skill contract by probing route reachability. The route paths and request bodies did not change. +`session_approval_mode_control`, `workspace_tool_toggle`, `workspace_skill_settings_toggle`, `workspace_skill_settings_batch_toggle`, `extension_batch_activation_v2`, `workspace_init`, and `workspace_mcp_restart` advertise the mutation control routes documented below. They are strict-gated by the mutation gate (a daemon configured without a bearer token rejects them with 401 `token_required`). Daemons that lack one of these routes return `404`. The settings-specific Skill tags are different: daemons from the retired-tag generation advertise `workspace_skill_toggle` and `workspace_skill_batch_toggle` and serve their catalog-validated contract at the same paths. The retired single-target route can return HTTP `404 skill_not_found` or `409 skill_not_toggleable`; the retired batch route returns HTTP 200 and places catalog-derived failures in `errors[]`. Pre-flight each tag before exposing its affordance, and do not infer the settings-specific Skill contract by probing route reachability. The route paths and request bodies did not change. `mcp_guardrails` (issue [#4175](https://github.com/QwenLM/qwen-code/issues/4175) PR 14) covers the MCP budget surface: the `clientCount` / `clientBudget` / `budgetMode` / `budgets[]` fields on `GET /workspace/mcp`, the `disabledReason` field on per-server cells, and the `--mcp-client-budget` / `--mcp-budget-mode` CLI flags. Older daemons omit the new fields entirely; SDK clients pre-flight this tag before relying on `budgets[]` semantics. The registry descriptor also carries `modes: ['warn', 'enforce']` for future feature-modes exposure — for now, clients infer mode from the snapshot's `budgetMode` field. Server refusal under `enforce` mode is deterministic by `Object.entries(mcpServers)` declaration order; a future scope-precedence layer (if qwen-code adopts one) would shift this to "lowest-precedence first" to mirror claude-code's `plugin < user < project < local` convention. @@ -2745,7 +2745,7 @@ The daemon exposes five mutation control routes that let remote clients change r - Are gated by the **strict** mutation gate from PR 15. A daemon configured without a bearer token rejects them with `401 {code: 'token_required'}`. Configure `--token` (or `QWEN_SERVER_TOKEN`) before opting in. - Accept and stamp the `X-Qwen-Client-Id` header (PR 7 audit chain). When the header carries a trusted id, the daemon emits `originatorClientId` on the corresponding SSE event so cross-client UIs can suppress echoes of their own mutations. -- Pre-flight each per-tag capability before exposing the affordance. A daemon that lacks a route returns `404`. For the Skill settings routes, tag absence can instead mean that the same path serves the retired catalog-validated contract, including per-target `404 skill_not_found` and `409 skill_not_toggleable` responses; do not use route probing as the version check. +- Pre-flight each per-tag capability before exposing the affordance. A daemon that lacks a route returns `404`. For the Skill settings routes, tag absence can instead mean that the same paths serve the retired catalog-validated contract: the single-target route can return HTTP `404 skill_not_found` or `409 skill_not_toggleable`, while the batch route returns HTTP 200 with catalog-derived failures in `errors[]`. Do not use route probing as the version check. The tool toggle, skill toggle, init, and MCP restart routes emit **workspace-scoped** events: every active session SSE bus receives the event, regardless of which session was attached when the mutation was triggered. `approval-mode` emits a **session-scoped** event because the change is local to one session's `Config`. From 86b4069eebf78a027ae29b572112d36ecad77e24 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Thu, 27 Aug 2026 01:30:45 +0800 Subject: [PATCH 08/10] docs(serve): distinguish skill changes from activation --- docs/design/daemon-skill-batch-toggle.md | 22 ++++++++++++------- docs/design/daemon-skill-toggle.md | 4 ++-- .../developers/daemon/13-sdk-daemon-client.md | 2 +- docs/developers/qwen-serve-protocol.md | 6 ++--- docs/users/qwen-serve.md | 2 +- 5 files changed, 21 insertions(+), 15 deletions(-) diff --git a/docs/design/daemon-skill-batch-toggle.md b/docs/design/daemon-skill-batch-toggle.md index 4ffdf6feced..ced121a3373 100644 --- a/docs/design/daemon-skill-batch-toggle.md +++ b/docs/design/daemon-skill-batch-toggle.md @@ -29,13 +29,16 @@ trimmed and deduplicated case-insensitively while preserving first-seen order. The daemon does not read or validate against runtime Skill status. It applies all resulting declaration changes in at most one locked settings write and, when anything changed, performs one live-session refresh. 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`. Unknown, non-user-invocable, inactive-Extension, and -higher-scope-disabled names use the same settings path. Higher scopes still -determine effective availability after settings merge, but do not prevent the -workspace scope from recording its own declaration. Unexpected persistence and -runtime-generation failures fail the whole request. +a matching workspace `skills.disabled` entry, preserves and normalizes an +existing workspace `skills.enabled` declaration, or records an opt-in for an +effective `skills.defaultDisabled` entry. With no existing workspace +declaration and no effective `skills.defaultDisabled` entry, enable is a no-op +(`changed: false`). Disabling writes `skills.disabled`. Unknown, +non-user-invocable, inactive-Extension, and higher-scope-disabled names use the +same settings path. Higher scopes still determine effective availability after +settings merge, but do not prevent the workspace scope from recording its own +declaration. Unexpected persistence and runtime-generation failures fail the +whole request. ```json { @@ -65,7 +68,10 @@ runtime-generation failures fail the whole request. ``` `results` preserves request order. `errors` remains present for wire -compatibility and is empty for structurally valid names. +compatibility and is empty for structurally valid names. Batch `activation` +reflects child liveness and any required shared refresh independently from each +result's `changed` flag; an all-no-op batch can therefore be `applied` or +`deferred`, and no refresh occurs. Malformed requests still fail as a whole with HTTP 400. Workspace trust, authentication, client identity, and generation ownership use the same gates diff --git a/docs/design/daemon-skill-toggle.md b/docs/design/daemon-skill-toggle.md index bc747bfdb8f..34fe36becb1 100644 --- a/docs/design/daemon-skill-toggle.md +++ b/docs/design/daemon-skill-toggle.md @@ -12,11 +12,11 @@ Expose workspace Skill settings writes through daemon REST and the TypeScript SD - SDK: `DaemonClient.setWorkspaceSkillEnabled` and `WorkspaceDaemonClient.setWorkspaceSkillEnabled` - Capability: `workspace_skill_settings_toggle` -The response contains the trimmed requested name, requested state, whether persistence changed, activation state, and session refresh counts. `applied` means every active session refreshed, `deferred` means no ACP child was running, and `partial` means at least one session failed to refresh after persistence committed. +The response contains the trimmed requested name, requested state, whether persistence changed, activation state, and session refresh counts. `activation` reflects child liveness and any required refresh independently from `changed`: `applied` means a child was live and any required refresh succeeded, `deferred` means no ACP child was running, and `partial` means at least one required refresh failed after persistence committed. A no-op can therefore be `applied` or `deferred` while `changed` remains false. ## Semantics -The API changes workspace `skills.disabled` and `skills.enabled` by name without consulting the runtime Skill catalog. Enabling a default-disabled Skill writes an explicit opt-in; disabling it removes the opt-in and writes a hard workspace disable. Updating one target removes target duplicates and case variants without deleting orphan entries for unavailable Skills. A name may be disabled before installation, while hidden from user invocation, or while its Extension is inactive. Enabling removes an existing workspace disable or records an opt-in for an effective `skills.defaultDisabled` entry; with neither condition, it is a no-op. A second identical request is also a no-op. +The API changes workspace `skills.disabled` and `skills.enabled` by name without consulting the runtime Skill catalog. Enabling a default-disabled Skill writes an explicit opt-in; disabling it removes the opt-in and writes a hard workspace disable. Updating one target removes target duplicates and case variants without deleting orphan entries for unavailable Skills. A name may be disabled before installation, while hidden from user invocation, or while its Extension is inactive. Enabling removes an existing workspace disable or records an opt-in for an effective `skills.defaultDisabled` entry. An existing workspace `skills.enabled` declaration is preserved and normalized to the requested casing. With no existing workspace declaration and no effective `skills.defaultDisabled` entry, enable is a no-op (`changed: false`). A second identical request is also a no-op. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not prevent workspace scope from recording or removing its own declaration. Workspace declarations otherwise participate in the usual `skills.disabled > skills.enabled > skills.defaultDisabled` resolution and can override higher-scope `skills.defaultDisabled` or `skills.enabled` entries. The route retains request-shape, authentication, client identity, workspace trust, and runtime-generation gates; none of those require a Skill catalog lookup. diff --git a/docs/developers/daemon/13-sdk-daemon-client.md b/docs/developers/daemon/13-sdk-daemon-client.md index e8cc5bd8195..1eda8c7985d 100644 --- a/docs/developers/daemon/13-sdk-daemon-client.md +++ b/docs/developers/daemon/13-sdk-daemon-client.md @@ -165,7 +165,7 @@ await client .setWorkspaceSkillsEnabled(['review', 'deploy'], true); ``` -`DaemonSkillBatchToggleResult` contains ordered `results`, a compatibility `errors` array, and batch-level activation/session-refresh counts. Current daemons process every structurally valid name in request order, persist all resulting declaration changes together in at most one locked settings write, refresh active sessions once, and return an empty `errors` array without consulting the loaded Skill catalog. Enabling a name with no existing workspace declaration and no effective `skills.defaultDisabled` entry returns `changed: false` and performs no write. The error item types remain available so the SDK can still decode responses from older daemons. The method throws on a non-200 response. +`DaemonSkillBatchToggleResult` contains ordered `results`, a compatibility `errors` array, and batch-level activation/session-refresh counts. Current daemons process every structurally valid name in request order, persist all resulting declaration changes together in at most one locked settings write, refresh active sessions once when anything changed, and return an empty `errors` array without consulting the loaded Skill catalog. Enabling a name with no existing workspace declaration and no effective `skills.defaultDisabled` entry returns `changed: false` and performs no write. The error item types remain available so the SDK can still decode responses from older daemons. The method throws on a non-200 response. V2 Extension batch activation retains the asynchronous Extension operation model. Pre-flight `extension_batch_activation_v2`, submit a global default batch or a selected-workspace override batch, then poll it with the existing operation helper: diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 5902c880fd5..5706d62579c 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -2841,7 +2841,7 @@ Response (200): } ``` -`activation` is `applied` when every active session refreshed, `deferred` when no ACP child exists (the persisted setting is used when one starts), and `partial` when at least one active session failed to refresh. Busy sessions are included. The daemon reloads workspace settings for the ACP child and every active session, notifies SkillManager consumers, and pushes `available_commands_update`. A request already sent to the model is not rewritten; subsequent validation, command snapshots, and model contexts use the new state. If persistence fails, no refresh or event is emitted. If a session refresh fails, the committed setting is retained. When the child returns per-session results, the session counts are exact. If the refresh control itself fails before returning those results, `sessionsFailed: 1` is a conservative lower bound indicating that the refresh request failed. +`activation` reflects child liveness and any required refresh independently from `changed`. It is `applied` when an ACP child is live and any required refresh succeeds, `deferred` when no child exists, and `partial` when at least one required refresh fails. A no-op can therefore be `applied` or `deferred` with `changed: false`; when `changed` is true and activation is `deferred`, the persisted declaration is used when a child starts. Busy sessions are included in a required refresh. The daemon reloads workspace settings for the ACP child and every active session, notifies SkillManager consumers, and pushes `available_commands_update`. A request already sent to the model is not rewritten; subsequent validation, command snapshots, and model contexts use the new state. If persistence fails, no refresh or event is emitted. If a session refresh fails, the committed setting is retained. When the child returns per-session results, the session counts are exact. If the refresh control itself fails before returning those results, `sessionsFailed: 1` is a conservative lower bound indicating that the refresh request failed. Errors: @@ -2855,7 +2855,7 @@ The mutation reuses the workspace-scoped `settings_changed` event for each chang Capability tag: `workspace_skill_settings_batch_toggle`. The workspace-qualified form is `POST /workspaces/:workspace/skills/enable`. -Update workspace Skill settings for up to 100 names in one request; the cap counts the raw `skillNames` entries before deduplication. Names are trimmed and deduplicated case-insensitively while preserving first-seen order and casing. The daemon does not consult the loaded Skill catalog. It applies all resulting declaration changes in at most one locked settings write and refreshes active sessions once. Enabling a name with no existing workspace declaration and no effective `skills.defaultDisabled` entry is a no-op (`changed: false`) and performs no write. Unexpected persistence or runtime-generation failures fail the whole request. +Update workspace Skill settings for up to 100 names in one request; the cap counts the raw `skillNames` entries before deduplication. Names are trimmed and deduplicated case-insensitively while preserving first-seen order and casing. The daemon does not consult the loaded Skill catalog. It applies all resulting declaration changes in at most one locked settings write and, when anything changed, refreshes active sessions once. Enabling a name with no existing workspace declaration and no effective `skills.defaultDisabled` entry is a no-op (`changed: false`) and performs no write. Unexpected persistence or runtime-generation failures fail the whole request. Request: @@ -2895,7 +2895,7 @@ Response (200): } ``` -Malformed requests return HTTP 400 with `invalid_skill_names`, `invalid_skill_name`, or `invalid_enabled_flag`. Authentication, workspace trust, client identity, unexpected persistence failures, and runtime-generation failures fail the whole request through the standard route gates. `errors` remains in the response for wire compatibility and is empty for structurally valid names. Batch-level `activation`, `sessionsRefreshed`, and `sessionsFailed` describe the single live-session refresh shared by all changed results. `activation` reports the refresh attempt rather than the outcome: a batch in which no target changed still answers `applied` when a session is live, matching the single-Skill no-op response, so derive what actually changed from each result's `changed` flag. When at least one target changes, the daemon emits the same `settings_changed` mutation metadata as the single-Skill route; every `skills.disabled` / `skills.enabled` event from that request shares one `mutation.id`. +Malformed requests return HTTP 400 with `invalid_skill_names`, `invalid_skill_name`, or `invalid_enabled_flag`. Authentication, workspace trust, client identity, unexpected persistence failures, and runtime-generation failures fail the whole request through the standard route gates. `errors` remains in the response for wire compatibility and is empty for structurally valid names. Batch-level `activation`, `sessionsRefreshed`, and `sessionsFailed` describe child liveness and the single live-session refresh shared by all changed results. A batch in which no target changed can still answer `applied` when a child is live or `deferred` when none exists, matching the single-Skill no-op response, so derive what actually changed from each result's `changed` flag. When at least one target changes, the daemon emits the same `settings_changed` mutation metadata as the single-Skill route; every `skills.disabled` / `skills.enabled` event from that request shares one `mutation.id`. #### `POST /workspace/init` diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index 34d564ee42f..e7d3695f702 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -209,7 +209,7 @@ idle daemon returns `initialized: false` with an empty snapshot. Once a session is alive they switch to `initialized: true` and surface the real state. -To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_settings_toggle` capability. To change several names, check `workspace_skill_settings_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it processes every structurally valid name together, persists all resulting declaration changes in at most one locked settings write, and refreshes active ACP sessions once. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be disabled before installation or while their Extension is inactive. Enabling a name before installation writes an explicit `skills.enabled` opt-in only when an effective `skills.defaultDisabled` entry applies; with no existing workspace declaration and no such entry, enable is a no-op (`changed: false`). Their paths and request bodies are unchanged; the settings-specific capabilities replace the retired catalog-validated Skill toggle tags. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means a declaration changed while no ACP child was running; it will apply when the child starts. `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. +To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_settings_toggle` capability. To change several names, check `workspace_skill_settings_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it processes every structurally valid name together, persists all resulting declaration changes in at most one locked settings write, and refreshes active ACP sessions once when anything changed. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be disabled before installation or while their Extension is inactive. Enabling a name before installation can remove a matching workspace disable, normalize an existing workspace `skills.enabled` declaration to the requested casing, or write an explicit opt-in when an effective `skills.defaultDisabled` entry applies; with no existing workspace declaration and no such entry, enable is a no-op (`changed: false`). Their paths and request bodies are unchanged; the settings-specific capabilities replace the retired catalog-validated Skill toggle tags. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means no ACP child was running to refresh; when `changed` is true, the new declaration applies when a child starts. Whether a declaration actually changed is reported by `changed` (on each result for a batch request). `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. `GET /workspace/env` and `GET /workspace/preflight` always answer with `initialized: true` regardless of ACP state. `env` never consults ACP From 7c8ec940704775facb3defb8ca743805c32b1548 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Thu, 27 Aug 2026 02:04:44 +0800 Subject: [PATCH 09/10] docs(serve): clarify deferred skill refreshes --- docs/design/daemon-skill-toggle.md | 2 +- docs/developers/qwen-serve-protocol.md | 2 +- docs/users/qwen-serve.md | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/design/daemon-skill-toggle.md b/docs/design/daemon-skill-toggle.md index 34fe36becb1..a4bccab9930 100644 --- a/docs/design/daemon-skill-toggle.md +++ b/docs/design/daemon-skill-toggle.md @@ -12,7 +12,7 @@ Expose workspace Skill settings writes through daemon REST and the TypeScript SD - SDK: `DaemonClient.setWorkspaceSkillEnabled` and `WorkspaceDaemonClient.setWorkspaceSkillEnabled` - Capability: `workspace_skill_settings_toggle` -The response contains the trimmed requested name, requested state, whether persistence changed, activation state, and session refresh counts. `activation` reflects child liveness and any required refresh independently from `changed`: `applied` means a child was live and any required refresh succeeded, `deferred` means no ACP child was running, and `partial` means at least one required refresh failed after persistence committed. A no-op can therefore be `applied` or `deferred` while `changed` remains false. +The response contains the trimmed requested name, requested state, whether persistence changed, activation state, and session refresh counts. `activation` reflects child liveness and any required refresh independently from `changed`: `applied` means a child was live and any required refresh succeeded, `deferred` means no child was live at the liveness check or a changed request lost its child/session during the required refresh, and `partial` means at least one other required refresh failed after persistence committed. A no-op can therefore be `applied` or `deferred` while `changed` remains false. ## Semantics diff --git a/docs/developers/qwen-serve-protocol.md b/docs/developers/qwen-serve-protocol.md index 5706d62579c..f5dadccc29e 100644 --- a/docs/developers/qwen-serve-protocol.md +++ b/docs/developers/qwen-serve-protocol.md @@ -2841,7 +2841,7 @@ Response (200): } ``` -`activation` reflects child liveness and any required refresh independently from `changed`. It is `applied` when an ACP child is live and any required refresh succeeds, `deferred` when no child exists, and `partial` when at least one required refresh fails. A no-op can therefore be `applied` or `deferred` with `changed: false`; when `changed` is true and activation is `deferred`, the persisted declaration is used when a child starts. Busy sessions are included in a required refresh. The daemon reloads workspace settings for the ACP child and every active session, notifies SkillManager consumers, and pushes `available_commands_update`. A request already sent to the model is not rewritten; subsequent validation, command snapshots, and model contexts use the new state. If persistence fails, no refresh or event is emitted. If a session refresh fails, the committed setting is retained. When the child returns per-session results, the session counts are exact. If the refresh control itself fails before returning those results, `sessionsFailed: 1` is a conservative lower bound indicating that the refresh request failed. +`activation` reflects child liveness and any required refresh independently from `changed`. It is `applied` when an ACP child is live and any required refresh succeeds, `deferred` when no child was live at the liveness check or a changed request loses its child/session during the required refresh, and `partial` when at least one other required refresh fails. A no-op can therefore be `applied` or `deferred` with `changed: false`; when `changed` is true and activation is `deferred`, the persisted declaration is used when a child starts. Busy sessions are included in a required refresh. The daemon reloads workspace settings for the ACP child and every active session, notifies SkillManager consumers, and pushes `available_commands_update`. A request already sent to the model is not rewritten; subsequent validation, command snapshots, and model contexts use the new state. If persistence fails, no refresh or event is emitted. If a session refresh fails, the committed setting is retained. When the child returns per-session results, the session counts are exact. If the refresh control itself fails before returning those results, `sessionsFailed: 1` is a conservative lower bound indicating that the refresh request failed. Errors: diff --git a/docs/users/qwen-serve.md b/docs/users/qwen-serve.md index e7d3695f702..2e8483bb40c 100644 --- a/docs/users/qwen-serve.md +++ b/docs/users/qwen-serve.md @@ -209,7 +209,7 @@ idle daemon returns `initialized: false` with an empty snapshot. Once a session is alive they switch to `initialized: true` and surface the real state. -To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_settings_toggle` capability. To change several names, check `workspace_skill_settings_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it processes every structurally valid name together, persists all resulting declaration changes in at most one locked settings write, and refreshes active ACP sessions once when anything changed. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be disabled before installation or while their Extension is inactive. Enabling a name before installation can remove a matching workspace disable, normalize an existing workspace `skills.enabled` declaration to the requested casing, or write an explicit opt-in when an effective `skills.defaultDisabled` entry applies; with no existing workspace declaration and no such entry, enable is a no-op (`changed: false`). Their paths and request bodies are unchanged; the settings-specific capabilities replace the retired catalog-validated Skill toggle tags. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means no ACP child was running to refresh; when `changed` is true, the new declaration applies when a child starts. Whether a declaration actually changed is reported by `changed` (on each result for a batch request). `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. +To update workspace Skill settings by name, call `POST /workspace/skills/:name/enable` with `{ "enabled": true | false }` after checking the `workspace_skill_settings_toggle` capability. To change several names, check `workspace_skill_settings_batch_toggle` and call `POST /workspace/skills/enable` with `{ "skillNames": ["review", "deploy"], "enabled": false }`; it processes every structurally valid name together, persists all resulting declaration changes in at most one locked settings write, and refreshes active ACP sessions once when anything changed. These routes write workspace `skills.disabled` and `skills.enabled` without consulting the loaded Skill catalog, so names may be disabled before installation or while their Extension is inactive. Enabling a name before installation can remove a matching workspace disable, normalize an existing workspace `skills.enabled` declaration to the requested casing, or write an explicit opt-in when an effective `skills.defaultDisabled` entry applies; with no existing workspace declaration and no such entry, enable is a no-op (`changed: false`). Their paths and request bodies are unchanged; the settings-specific capabilities replace the retired catalog-validated Skill toggle tags. The batch `errors` array remains for wire compatibility and is empty for structurally valid names. A hard `skills.disabled` entry inherited from a higher scope remains authoritative for effective availability, but does not block the workspace from recording or removing its own declaration. Skill status cells expose `disabledReason` (`hard`, `default`, or `inactive_extension`) and an optional `lockedScope`. Untrusted workspace writes are still rejected. A `deferred` response means no child was live at the liveness check or a changed request lost its child/session during the required refresh; when `changed` is true, the persisted declaration applies when a child starts. Whether a declaration actually changed is reported by `changed` (on each result for a batch request). `skills.disabled` disables both manual and model use, unlike `disable-model-invocation: true`, which keeps direct `/skill-name` invocation available. For V2 Extension batches, check `extension_batch_activation_v2`: `PUT /extensions/activation` changes global defaults, while `PUT /workspaces/:workspace/extensions/activation` changes exact overrides for the selected workspace and accepts `"inherit"` to clear them. Both accept names in `extensionNames`; `enabled` and `disabled` may be declared before installation, while `inherit` for an unknown name is a no-op. Each request returns one operation to poll. `GET /workspace/env` and `GET /workspace/preflight` always answer with `initialized: true` regardless of ACP state. `env` never consults ACP From 6af0f6cc999d0b6feab1145bde863edcaab5925e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E5=8F=B6=E5=85=AC?= Date: Thu, 27 Aug 2026 21:43:20 +0800 Subject: [PATCH 10/10] fix(web-shell): allow model-only skill settings writes --- .../web-shell/web-shell-skill-manager-page.md | 7 ++-- .../skills/SkillsManagerPage.test.tsx | 35 +++++++++++++++++++ .../components/skills/SkillsManagerPage.tsx | 10 ++---- 3 files changed, 41 insertions(+), 11 deletions(-) diff --git a/docs/design/web-shell/web-shell-skill-manager-page.md b/docs/design/web-shell/web-shell-skill-manager-page.md index 6ab765a8431..f3033987e4a 100644 --- a/docs/design/web-shell/web-shell-skill-manager-page.md +++ b/docs/design/web-shell/web-shell-skill-manager-page.md @@ -17,9 +17,10 @@ active chat session. MCP management page. - Selecting a skill opens its details in the same page. - Returning from details preserves the active scope filter and search query. -- The details page exposes the daemon's per-skill enable/disable action. Skills - that are not user-invocable cannot be toggled; extension skills can be - toggled unless their parent extension is inactive. +- The details page exposes the daemon's per-skill settings action. Runtime + catalog metadata such as user invocability and parent Extension activation + does not gate workspace declaration writes; authoritative availability may + remain unchanged after a successful write. - “Reference skill” returns to chat and places `/` in the composer without submitting it. - The list header exposes an Upload action for GitHub, daemon-local folder, and diff --git a/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx b/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx index 1a8e558ba28..ff82e070573 100644 --- a/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx +++ b/packages/web-shell/client/components/skills/SkillsManagerPage.test.tsx @@ -164,6 +164,41 @@ describe('SkillsManagerPage', () => { expect(enable?.hasAttribute('data-disabled')).toBe(true); }); + it('allows settings writes for non-user-invocable Skills', async () => { + const disabledSkill: DaemonWorkspaceSkillStatus = { + kind: 'skill', + status: 'disabled', + name: 'model-only-helper', + description: 'Model-only helper', + level: 'user', + modelInvocable: true, + userInvocable: false, + disabledReason: 'default', + }; + const enabledSkill: DaemonWorkspaceSkillStatus = { + ...disabledSkill, + status: 'ok', + disabledReason: undefined, + }; + skillsState.current.skills = [disabledSkill]; + skillsState.current.reload.mockResolvedValue({ + v: 1, + workspaceCwd: '/workspace/demo', + initialized: true, + skills: [enabledSkill], + errors: [], + }); + + await renderPage(); + await openDisabledSkill(disabledSkill.name); + await enableSelectedSkill(); + + expect(skillsState.current.setEnabled).toHaveBeenCalledWith( + disabledSkill.name, + true, + ); + }); + it('shows the authoritative enabled state after a normal toggle', async () => { const disabledSkill: DaemonWorkspaceSkillStatus = { kind: 'skill', diff --git a/packages/web-shell/client/components/skills/SkillsManagerPage.tsx b/packages/web-shell/client/components/skills/SkillsManagerPage.tsx index 50c341e9655..7b3644a7600 100644 --- a/packages/web-shell/client/components/skills/SkillsManagerPage.tsx +++ b/packages/web-shell/client/components/skills/SkillsManagerPage.tsx @@ -417,17 +417,11 @@ export function SkillsManagerPage({ > void toggleSkill(selectedSkill)} >