Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
6291b53
feat(daemon): attach skill-toggle mutation metadata to settings_changed
samuelhsin Aug 13, 2026
c127037
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 13, 2026
123f747
fix(review): fit skill-toggle mutation metadata in the SDK bundle budget
samuelhsin Aug 13, 2026
d69bce1
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 13, 2026
7c1c0f6
test(daemon): pin skill-toggle mutation event count and parser edges
samuelhsin Aug 13, 2026
9da7603
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 13, 2026
98ea100
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 14, 2026
5df1746
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 14, 2026
f2063a5
Merge remote-tracking branch 'upstream/main' into codex/fix-9051-cr
samuelhsin Aug 14, 2026
b698a87
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 14, 2026
00924b8
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 15, 2026
42a825b
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 15, 2026
07ce8eb
fix(sdk): raise daemon browser bundle budget for skill-toggle metadata
samuelhsin Aug 15, 2026
b1f752b
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 15, 2026
503e1ff
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
wenshao Aug 16, 2026
806d758
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 16, 2026
3c7bdb7
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 16, 2026
fe0b74f
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 16, 2026
fc147d3
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 16, 2026
3579db6
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 16, 2026
3c46058
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 17, 2026
c131b3f
Merge branch 'main' into feat/9024-skill-toggle-mutation-metadata
samuelhsin Aug 17, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 4 additions & 2 deletions docs/developers/daemon/09-event-schema.md
Original file line number Diff line number Diff line change
Expand Up @@ -77,12 +77,14 @@ Grouped by domain.
| `agent_changed` | S->C | `change: 'created' \| 'updated' \| 'deleted', name, level: 'project' \| 'user'` |
| `approval_mode_changed` | S->C | `sessionId, previous, next, persisted: boolean` |
| `tool_toggled` | S->C | `toolName, enabled`; affects the next ACP child spawn and does not mutate already-running sessions. |
| `settings_changed` | S->C | Workspace settings write completed. Payload is open; consumers should refresh with read-after-write. |
| `settings_changed` | S->C | Workspace settings write completed. Payload includes `key`; `value`, `scope`, and Skill-toggle `mutation` are optional. |
| `settings_reloaded` | S->C | Daemon workspace service reread settings. Payload is open. |
| `trust_change_requested` | S->C | `workspaceCwd, desiredState: 'trusted' \| 'untrusted', reason?` |
| `workspace_initialized` | S->C | `path, action: 'created' \| 'overwrote' \| 'noop', originatorClientId?` |
| `github_setup_completed` | S->C | `releaseTag, readmeUrl, secretsUrl?, workflows: [{path, status, sizeBytes?, error?}], gitignore: {path, status, added?, error?}` |

Skill toggle APIs attach optional `mutation: { id, kind: 'skill_toggle', skills: [{ name, enabled }], activation, sessionsRefreshed, sessionsFailed }`. Every `skills.disabled` / `skills.enabled` event from the same request shares one mutation id. Other settings writes omit `mutation`. Workspace-service writes include `scope`; some other emitters (for example session model switches) omit it. The SDK normalizer defaults missing `scope` to `'workspace'`.

`memory_changed` also covers sessionless managed-memory tasks. For those
payloads, `scope` is `"managed"`, `source` is one of
`"workspace_memory_remember"`, `"workspace_memory_forget"`, or
Expand Down Expand Up @@ -174,7 +176,7 @@ These events are workspace-keyed, not session-keyed. The session reducer treats
- `workspaceInitCount`, `lastWorkspaceInit?` - from `workspace_initialized`.
- `mcpRestartCount`, `lastMcpRestart?` - from `mcp_server_restarted`.
- `mcpRestartRefusedCount`, `lastMcpRestartRefused?` - from `mcp_server_restart_refused`.
- `settings_changed` / `settings_reloaded` - recognized by `asKnownDaemonEvent`; the session reducer does not maintain dedicated view-state fields, and UIs usually treat them as refresh signals.
- `settings_changed` / `settings_reloaded` - recognized by `asKnownDaemonEvent`; the session reducer does not maintain dedicated view-state fields. Skill-toggle `settings_changed` events carry optional `mutation` metadata so hosts can apply Skill-only changes incrementally instead of reloading the task. Other UIs may still treat the event as a refresh signal.
- `permissionVoteProgress: Record<string, DaemonPermissionPartialVoteData>` - consensus voting progress.
- `forbiddenVotes: DaemonPermissionForbiddenData[]`, `forbiddenVoteCount` - policy-rejected vote records, capped at 32.
- `awaitingResync: boolean` - set by `state_resync_required`; cleared when consumer resets view state.
Expand Down
4 changes: 2 additions & 2 deletions docs/developers/qwen-serve-protocol.md
Original file line number Diff line number Diff line change
Expand Up @@ -2768,7 +2768,7 @@ Errors:
- `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. 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 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.

#### `POST /workspace/skills/enable`

Expand Down Expand Up @@ -2815,7 +2815,7 @@ Response (200):
}
```

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.
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`.

#### `POST /workspace/init`

Expand Down
195 changes: 153 additions & 42 deletions packages/cli/src/serve/workspace-service/__tests__/facade.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ import type {
InvokeWorkspaceCommandFn,
QueryWorkspaceStatusFn,
WorkspaceRequestContext,
WorkspaceSkillToggleActivation,
} from '../types.js';

// ---------------------------------------------------------------------------
Expand Down Expand Up @@ -167,6 +168,34 @@ function makeCtx(
};
}

function skillToggleSettingsChanged(args: {
key: 'skills.disabled' | 'skills.enabled';
value: unknown;
skills: Array<{ name: string; enabled: boolean }>;
activation: WorkspaceSkillToggleActivation;
sessionsRefreshed: number;
sessionsFailed: number;
originatorClientId?: string;
}) {
return {
type: 'settings_changed',
data: {
key: args.key,
value: args.value,
scope: 'workspace',
mutation: {
id: expect.any(String),
kind: 'skill_toggle',
skills: args.skills,
activation: args.activation,
sessionsRefreshed: args.sessionsRefreshed,
sessionsFailed: args.sessionsFailed,
},
},
originatorClientId: args.originatorClientId ?? 'client-1',
};
}

async function withIsolatedQwenHome<T>(fn: () => Promise<T>): Promise<T> {
return withIsolatedWorkspace(() => fn());
}
Expand Down Expand Up @@ -1647,15 +1676,46 @@ describe('createDaemonWorkspaceService', () => {
sessionsRefreshed: 2,
sessionsFailed: 0,
});
expect(publishWorkspaceEvent).toHaveBeenCalledWith({
type: 'settings_changed',
data: {
expect(publishWorkspaceEvent).toHaveBeenCalledWith(
skillToggleSettingsChanged({
key: 'skills.disabled',
value: ['review'],
scope: 'workspace',
},
originatorClientId: 'client-1',
});
skills: [{ name: 'review', enabled: false }],
activation: 'applied',
sessionsRefreshed: 2,
sessionsFailed: 0,
}),
);
});

it('shares mutation ids within a request and renews them across requests', async () => {
const publishWorkspaceEvent = vi.fn();
const svc = createDaemonWorkspaceService(
makeDeps({
queryWorkspaceStatus: statusQuery(),
persistDisabledSkills: vi.fn().mockResolvedValue({
changed: true,
disabled: [],
settingsChanges: [
{ key: 'skills.disabled', value: undefined },
{ key: 'skills.enabled', value: ['review'] },
],
}),
publishWorkspaceEvent,
isChannelLive: () => false,
}),
);

await svc.setWorkspaceSkillEnabled(makeCtx(), 'review', true);
await svc.setWorkspaceSkillEnabled(makeCtx(), 'review', true);

const mutationIds = publishWorkspaceEvent.mock.calls.map(
([event]) => event.data.mutation.id,
);
expect(mutationIds).toHaveLength(4);
expect(mutationIds[0]).toBe(mutationIds[1]);
expect(mutationIds[2]).toBe(mutationIds[3]);
expect(mutationIds[0]).not.toBe(mutationIds[2]);
});

it('does not retain a status snapshot read while a settings refresh is in flight', async () => {
Expand Down Expand Up @@ -1750,18 +1810,20 @@ describe('createDaemonWorkspaceService', () => {
enabled: true,
activation: 'applied',
});
expect(publishWorkspaceEvent).toHaveBeenCalledWith({
type: 'settings_changed',
data: {
expect(publishWorkspaceEvent).toHaveBeenCalledWith(
skillToggleSettingsChanged({
key: 'skills.enabled',
value: ['review'],
scope: 'workspace',
},
originatorClientId: 'client-1',
});
skills: [{ name: 'review', enabled: true }],
activation: 'applied',
sessionsRefreshed: 1,
sessionsFailed: 0,
}),
);
});

it('reports partial activation when a session refresh fails', async () => {
const publishWorkspaceEvent = vi.fn();
const svc = createDaemonWorkspaceService(
makeDeps({
queryWorkspaceStatus: statusQuery(),
Expand All @@ -1773,6 +1835,7 @@ describe('createDaemonWorkspaceService', () => {
sessionsRefreshed: 1,
sessionsFailed: 1,
}),
publishWorkspaceEvent,
isChannelLive: () => true,
}),
);
Expand All @@ -1784,6 +1847,16 @@ describe('createDaemonWorkspaceService', () => {
sessionsRefreshed: 1,
sessionsFailed: 1,
});
expect(publishWorkspaceEvent).toHaveBeenCalledWith(
skillToggleSettingsChanged({
key: 'skills.disabled',
value: ['review'],
skills: [{ name: 'review', enabled: false }],
activation: 'partial',
sessionsRefreshed: 1,
sessionsFailed: 1,
}),
);
});

it('defers refresh when no child exists or the child closes mid-refresh', async () => {
Expand Down Expand Up @@ -2225,15 +2298,20 @@ describe('createDaemonWorkspaceService', () => {
],
});
expect(publishWorkspaceEvent).toHaveBeenCalledOnce();
expect(publishWorkspaceEvent).toHaveBeenCalledWith({
type: 'settings_changed',
data: {
expect(publishWorkspaceEvent).toHaveBeenCalledWith(
skillToggleSettingsChanged({
key: 'skills.disabled',
value: ['review', 'missing', 'deploy'],
scope: 'workspace',
},
originatorClientId: 'client-1',
});
skills: [
{ name: 'review', enabled: false },
{ name: 'missing', enabled: false },
{ name: 'deploy', enabled: false },
],
activation: 'applied',
sessionsRefreshed: 2,
sessionsFailed: 0,
}),
);
});

it('orders results and errors by request targets, not persist outcomes', async () => {
Expand Down Expand Up @@ -2483,15 +2561,16 @@ describe('createDaemonWorkspaceService', () => {
results: [{ skillName: 'review', enabled: true, changed: true }],
});
expect(publishWorkspaceEvent).toHaveBeenCalledOnce();
expect(publishWorkspaceEvent).toHaveBeenCalledWith({
type: 'settings_changed',
data: {
expect(publishWorkspaceEvent).toHaveBeenCalledWith(
skillToggleSettingsChanged({
key: 'skills.disabled',
value: undefined,
scope: 'workspace',
},
originatorClientId: 'client-1',
});
skills: [{ name: 'review', enabled: true }],
activation: 'deferred',
sessionsRefreshed: 0,
sessionsFailed: 0,
}),
);
});

it('publishes one settings_changed event per settingsChanges entry in order', async () => {
Expand Down Expand Up @@ -2523,27 +2602,38 @@ describe('createDaemonWorkspaceService', () => {
);

expect(publishWorkspaceEvent).toHaveBeenCalledTimes(2);
expect(publishWorkspaceEvent).toHaveBeenNthCalledWith(1, {
type: 'settings_changed',
data: {
expect(publishWorkspaceEvent).toHaveBeenNthCalledWith(
1,
skillToggleSettingsChanged({
key: 'skills.disabled',
value: undefined,
scope: 'workspace',
},
originatorClientId: 'client-1',
});
expect(publishWorkspaceEvent).toHaveBeenNthCalledWith(2, {
type: 'settings_changed',
data: {
skills: [{ name: 'review', enabled: true }],
activation: 'deferred',
sessionsRefreshed: 0,
sessionsFailed: 0,
}),
);
expect(publishWorkspaceEvent).toHaveBeenNthCalledWith(
2,
skillToggleSettingsChanged({
key: 'skills.enabled',
value: ['review'],
scope: 'workspace',
},
originatorClientId: 'client-1',
});
skills: [{ name: 'review', enabled: true }],
activation: 'deferred',
sessionsRefreshed: 0,
sessionsFailed: 0,
}),
);
const firstMutation = publishWorkspaceEvent.mock.calls[0]?.[0]?.data
?.mutation as { id?: string } | undefined;
const secondMutation = publishWorkspaceEvent.mock.calls[1]?.[0]?.data
?.mutation as { id?: string } | undefined;
expect(firstMutation?.id).toEqual(expect.any(String));
expect(firstMutation?.id).toBe(secondMutation?.id);
Comment on lines +2631 to +2632

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] The mutation-id correlation contract is only half-tested: same-id within one request is pinned here, but distinct ids across separate toggle requests are not — a one-line mutation id: randomUUID()id: 'constant' in createSkillToggleMutation survives the entire suite. Probe-verified: the mutant passes 112/112, and a distinctness assertion catches it while staying green against the real code. — Failure scenario: the docs this PR adds state id correlates every settings event produced by one toggle request, which only has meaning if different requests get different ids. If a future refactor replaces randomUUID() with a deterministic id (constant, counter, workspace hash), every test stays green, and a host that groups buffered settings_changed events by mutation.id — the exact consumer this metadata was built for (#9024) — merges events from two unrelated toggles into one mutation and applies them as a single incremental change.

Suggested fix — perform a second, separate toggle call in a facade test and assert its mutation id differs from the first request's:

expect(secondRequestMutation?.id).not.toBe(firstMutation?.id);
中文说明

[建议] mutation id 关联契约只被测试了一半:同一请求内共享 id 在此处被钉住,但不同请求之间 id 必须不同这一点没有任何测试覆盖——把 createSkillToggleMutation 中的 id: randomUUID() 一行突变为 id: 'constant' 后整个套件仍然全部通过。已探针验证:该突变体以 112/112 通过,而补上差异性断言即可捕获它,且该断言对真实代码保持绿色。——失败场景:本 PR 新增的文档写明 id 用于关联一次切换请求产生的所有 settings 事件,这只有在不同请求获得不同 id 时才有意义。如果未来某次重构把 randomUUID() 换成确定性 id(常量、计数器、workspace 哈希),所有测试仍为绿色,而按 mutation.id 归组缓冲的 settings_changed 事件的宿主——正是这份元数据为之而生的消费者(#9024)——会把两次互不相关的切换的事件合并进同一个 mutation,并作为一次增量变更应用。

建议修复——在 facade 测试中再发起一次独立的切换调用,并断言其 mutation id 与第一次请求的不同:

expect(secondRequestMutation?.id).not.toBe(firstMutation?.id);

— qwen3.8-max via Qwen Code /review (v0.21.11)

});

it('reports partial activation when the shared batch refresh fails', async () => {
const publishWorkspaceEvent = vi.fn();
const failedSessions = createDaemonWorkspaceService(
makeDeps({
queryWorkspaceStatus: vi.fn().mockResolvedValue({
Expand All @@ -2560,6 +2650,7 @@ describe('createDaemonWorkspaceService', () => {
sessionsRefreshed: 1,
sessionsFailed: 1,
}),
publishWorkspaceEvent,
isChannelLive: () => true,
}),
);
Expand All @@ -2570,6 +2661,16 @@ describe('createDaemonWorkspaceService', () => {
sessionsRefreshed: 1,
sessionsFailed: 1,
});
expect(publishWorkspaceEvent).toHaveBeenCalledWith(
skillToggleSettingsChanged({
key: 'skills.disabled',
value: ['review'],
skills: [{ name: 'review', enabled: false }],
activation: 'partial',
sessionsRefreshed: 1,
sessionsFailed: 1,
}),
);

const unexpectedError = createDaemonWorkspaceService(
makeDeps({
Expand Down Expand Up @@ -2752,6 +2853,16 @@ describe('createDaemonWorkspaceService', () => {
});
expect(invokeWorkspaceCommand).toHaveBeenCalledOnce();
expect(publishWorkspaceEvent).toHaveBeenCalledOnce();
expect(publishWorkspaceEvent).toHaveBeenCalledWith(
skillToggleSettingsChanged({
key: 'skills.disabled',
value: ['deploy'],
Comment thread
samuelhsin marked this conversation as resolved.
skills: [{ name: 'deploy', enabled: false }],
activation: 'applied',
sessionsRefreshed: 1,
sessionsFailed: 0,
}),
);
});

it('drops the cached skill snapshot after a changed batch like the single-toggle path', async () => {
Expand Down
Loading
Loading