feat(web-shell): refresh composer skills incrementally after toggles

[samuelhsin]

What this PR does

Adds a dedicated, de-duplicated workspace signal for Skill-toggle mutations and makes Web Shell choose the correct composer source for each runtime state. Active sessions use the latest authoritative command snapshot, while session-less, deferred, and partial activation paths refresh the workspace Skill snapshot. Failed fallback refreshes are surfaced instead of leaving stale suggestions silently.

Why it's needed

Web Shell previously loaded a workspace Skill snapshot and always merged it back into composer commands. That could reintroduce a disabled Skill after the active session had already published an authoritative empty command list. The generic settings signal also discarded Skill mutation metadata, so the two settings events from one toggle could not be de-duplicated or handled according to activation outcome.

Reviewer Test Plan

How to verify

1. In an active session, enable a Skill and confirm its slash command appears through the live command update without reloading the task or transcript.
2. Disable the last visible Skill and confirm composer autocomplete replaces the Skill list with an empty list.
3. Before the first prompt, toggle a Skill and confirm the session-less workspace snapshot refreshes once; repeat the same mutation id and confirm no second refresh occurs.
4. Simulate partial activation and confirm the workspace snapshot temporarily supplies composer Skills, then confirm a later live command update becomes authoritative again.
5. Simulate a failed fallback refresh and confirm Web Shell displays an error; send an unrelated legacy settings event and confirm the normal settings reload path still runs.

Evidence (Before & After)

Before: after an authoritative empty command update, the stale workspace snapshot reintroduced /web-search and it remained visible in autocomplete after 10 seconds.

After: the same Chromium flow removes /web-search immediately without a task/transcript reload. The independent Chromium matrix passed 5/5 scenarios.

Tested on

OS	Status
🍏 macOS	✅ Chromium E2E, unit tests, typecheck, package builds
🪟 Windows	N/A
🐧 Linux	N/A

Environment (optional)

Node.js 22.14.0, Playwright Chromium, mock daemon event stream.

Risk & Scope

• Main risk or tradeoff: partial activation temporarily prefers the refreshed workspace snapshot, but any later live command snapshot immediately regains authority.
• Not validated / out of scope: native Windows/Linux browser runs; daemon-side mutation production and normalization are supplied by the dependent PR.
• Breaking changes / migration notes: none. Older or malformed events without valid mutation metadata keep the existing generic settings behavior.

Linked Issues

Closes #9123

Depends on #9051

中文说明

这个 PR 做了什么

为 Skill 切换 mutation 增加独立且按 id 去重的 workspace signal，并让 Web Shell 根据 runtime 状态选择正确的 composer 数据源。活跃 session 使用最新的权威命令快照；无 session、deferred 和 partial activation 路径会刷新 workspace Skill 快照。fallback 刷新失败会直接提示，不再静默保留过期建议。

为什么需要

Web Shell 之前会读取 workspace Skill 快照，并始终把它重新合并进 composer commands。即使活跃 session 已经发布权威的空命令列表，旧快照仍可能把已禁用 Skill 加回来。通用 settings signal 还会丢弃 Skill mutation 元数据，因此无法对一次切换产生的两条 settings 事件去重，也无法根据 activation 结果选择处理方式。

评审测试计划

如何验证

1. 在活跃 session 中启用一个 Skill，确认 slash command 通过 live command update 出现，且不会重载 task 或 transcript。
2. 禁用最后一个可见 Skill，确认 composer autocomplete 把 Skill 列表替换为空。
3. 在第一次 prompt 之前切换 Skill，确认 session-less workspace 快照只刷新一次；重复发送相同 mutation id，确认不会再次刷新。
4. 模拟 partial activation，确认 composer 会暂时使用 workspace 快照；随后发送新的 live command update，确认它重新成为权威数据源。
5. 模拟 fallback 刷新失败，确认 Web Shell 显示错误；再发送无关的旧格式 settings 事件，确认普通 settings reload 路径仍正常执行。

证据（修复前后）

修复前：收到权威空命令更新后，旧 workspace 快照仍会重新加入 /web-search，10 秒后 autocomplete 中依然可见。

修复后：同一 Chromium 流程会立即移除 /web-search，且不重载 task/transcript。独立 Chromium 验证矩阵 5/5 通过。

测试环境

OS	Status
🍏 macOS	✅ Chromium E2E、单元测试、typecheck、包构建
🪟 Windows	N/A
🐧 Linux	N/A

运行环境（可选）

Node.js 22.14.0、Playwright Chromium、mock daemon event stream。

风险与范围

• 主要风险或取舍：partial activation 会暂时优先使用刷新后的 workspace 快照，但后续任何 live command snapshot 都会立即恢复为权威数据源。
• 未验证 / 范围外：Windows/Linux 原生浏览器运行；daemon 端 mutation 生成与 normalization 由依赖 PR 提供。
• 破坏性变更 / 迁移说明：无。缺少有效 mutation 元数据的旧事件或格式错误事件继续沿用现有通用 settings 行为。

关联 Issue

Closes #9123

Depends on #9051

[samuelhsin]

E2E Test Report

• Chromium mock-daemon matrix: 5/5 passed, covering active enable, disable-to-empty, duplicate partial mutation deduplication, refresh-failure toast, and unrelated legacy settings reload.
• Full Web Shell App tests: 438/438 passed.
• Full WebUI daemon-session provider tests: 268/268 passed.
• Web Shell and WebUI package typechecks passed.
• Web Shell and WebUI production builds passed.
• Environment: macOS, Node.js 22.14.0, Playwright Chromium.

Repository-wide build and typecheck remain blocked by pre-existing local dependency artifacts outside this diff: Core cannot find Sharp's SharpConstructor, and CLI sees stale workspace export declarations. No touched package failed.

[qwen-code-ci-bot]

Reviewed.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

• round-5 reverse-audit catch-path staleness-guard finding — already reported as R1-1 (comment 3796222512, [comment 3797837422](https://github.com/QwenLM/qwen-code/pull/9…

Unresolved, please confirm:

• [Critical] R9-1 secondary clause (comment 3815862715, packages/web-shell/client/App.tsx:4744) — the daemon can emit activation: 'applied' with sessionsRefreshed: 0 when the br…

Deferred under the convergence posture (round 16, not a blocker) — recorded, not requested in this round:

• packages/webui/src/daemon/session/DaemonSessionProvider.tsx:4492 — [probe] within-batch same-id dedupe (seenSkillMutationIds) untested on the replay path — guard-deletion mutant doubles a batch mutation, all provider tests stay green
• packages/webui/src/daemon/session/types.ts:557 — [review] skillsVersion has zero production read sites — provider tests certify a signal nobody consumes
• packages/web-shell/client/App.tsx:534 — [probe] mergeSkillToggles same-skill double-toggle in one signal update untested — concat mutant pins the fallback, all tests green
• packages/web-shell/client/App.tsx:4671 — [probe] silent initial-load half of the notifyOnError split unpinned — toast-on-connect mutant survives all 504 tests
• packages/web-shell/client/App.test.tsx:10895 — [probe] late-resolving reload tests resolve with the identical payload — stale-response guard + cancelled cleanup unpinned
• packages/web-shell/client/App.test.tsx:11078 — [probe] the || !loaded reload-failure branch unpinned — mutant marks a failed session-bound refresh handled and pins the stale fallback, ships green
• packages/web-shell/client/App.test.tsx:11036 — [probe] duplicate concurrent snapshot fetches pinned by exact call counts — the first (successful) response is discarded when the racing reconcile fetch fails

中文说明

已审查。

本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。

未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。

收敛姿态下延后（第 16 轮，非阻断）——已记录，本轮不要求修改：共 7 条（原文未翻译，列表见上方英文部分）。

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

[chiga0]

Web Shell Skill Toggle Refresh — first review.

What I checked

availableSessionSkillInfos — correctly derives SkillInfo from the active session's skills name list + commands, falling back to empty description when no matching command exists. Sort is stable and case-normalised via localeCompare. ✅

sessionSkillsReflectToggle — returns false when sessionSkills === undefined (session not yet reporting), preventing premature removal of the fallback before the session catches up. The every predicate correctly handles the case where a disabled skill is absent from the enabled set. ✅

mergeSkillToggles — last-write-wins semantics via Map keyed on lowercase name; correct for consecutive toggle events where later events supersede earlier ones. ✅

mappers.ts — _meta.availableSkills fallback: backward-compatible; the flat availableSkills field still takes priority over _meta (two tests at lines 845 and 863 pin both directions). ✅

bumpWorkspaceEventSignals dedup: skill-toggle mutations are deduped within a batch via seenSkillMutationIds and across renders via existingIds derived from prior signal state — prevents the double-increment from a server emitting two workspace.settings.changed events per toggle. ✅

handledSkillMutationKeysRef cleanup on context switch: iterates the Set and deletes matching entries during the loop. JS Set iteration with deletion is well-defined — elements already visited are gone, unvisited elements that haven't been deleted are visited once. ✅

connectionSkillSnapshotRef: updated on every render (not in an effect) so the ref is current when the async reloadLoadedSkills callback reads it. Correct pattern for avoiding stale closure reads inside async callbacks. ✅

CI scope

Test (ubuntu-latest, Node 22.x) ✅ · web-shell E2E Smoke ✅ · Dependency CVE audit ✅.
Test (macos-latest/windows-latest) SKIPPED — platform behaviour unverified.

No blocking findings. doudouOUC already approved (round 1).

Reviewed with AI assistance.

[wenshao]

Local real-stack verification of #9131 — ✅ core fix confirmed, 2 non-blocking notes

I built this PR locally and drove it against a real qwen serve daemon and a real browser (no mocked daemon), side by side with a build of the merge base, to check the claims in the test plan.

Verdict: the reported bug is real, this PR fixes it, and I found no regression. Two small notes below are worth a look but nothing I'd block on.

Harness

PR build	7992505dda (this PR head)
Base build	7bc0d80998 (merge base with main)
Daemon	one qwen serve from the PR tree (node scripts/dev.js serve --port 4270), bound to a fixture workspace
Arms	two Vite dev servers against the same daemon — :5273 = PR, :5274 = base — so both tabs receive the identical daemon event stream
Workspace	3 project skills (web-search, pdf-export, chart-lab) + 10 bundled skills, isolated QWEN_HOME
Model	local OpenAI-compatible mock, so sessions are real but deterministic
Driver	Playwright Chromium 1.61.1, macOS 26.6.2, Node 24.18.1
Toggling	a third tab driving the real Skills manager UI (Plugins → Skills → ⋮ → Disable/Enable), so both measured arms are pure observers

Instrumentation: the SSE body of GET /session/:id/events was teed in-page to read the raw settings_changed frames, and every request whose path contains skills was counted per arm.

Results

#	Scenario	PR build (:5273)	Base build (:5274)
1	Active session, disable web-search	/web-search gone from autocomplete, 0 workspace-skill refreshes	still offered — stale
2	Active session, enable a pre-disabled pdf-export	appears	appears (no difference; base was already fine here)
3	Active session, activation: "partial" (fault-injected)	1 workspace-skill refresh, entry removed	0 refreshes, stale
4	Next applied toggle after that partial	0 refreshes, live snapshot authoritative again, earlier removal sticks	stale
5	One toggle emitting two settings_changed with one mutation id	1 refresh (de-duplicated)	0
6	Fallback refresh made to fail	error toast shown	nothing shown
7	Freshly-opened tab, no prompt sent yet	no refresh	no refresh (see note 3)

1 — the headline bug and its fix

Baseline: both arms have a live session and both offer /web-search.

web-search is then disabled from the Skills manager…

…and 10 s later the PR build has dropped it while the base build still offers the disabled skill:

The single settings_changed frame both arms received:

{"type":"settings_changed","data":{"key":"skills.disabled","value":["web-search"],"scope":"workspace",
 "mutation":{"id":"eb406619-…","kind":"skill_toggle","skills":[{"name":"web-search","enabled":false}],
 "activation":"applied","sessionsRefreshed":2,"sessionsFailed":0}}}

Note the PR build made zero extra GET …/skills calls here — on the applied path it really does trust available_commands_update instead of re-fetching, as designed.

2 — why base fails only in the disable direction

Two wire facts from the live daemon explain it and are worth recording:

• available_commands_update carries the skill names only under _meta.availableSkills; the top-level availableSkills field is undefined. So on main connection.skills is permanently [] — the mappers.ts hunk in this PR is what makes it non-empty.
• The skills' own slash commands arrive inside availableCommands with _meta.source = "skill-dir-command", not "skill". That is why main's command.source !== 'skill' filter never drops them, and why the stale /workspace/skills snapshot only ever adds entries back — a newly enabled skill shows up fine, a newly disabled one lingers forever.

3 — both hunks are load-bearing (mutation A/B on the PR tree)

Mutation applied to the PR build	Result
commands memo gate reverted (useWorkspaceSkillSnapshot → loadedSkillsReady, 3 tokens in App.tsx)	bug returns: disabled /web-search is offered again
mappers.ts _meta.availableSkills hunk reverted	/skills <TAB> produces no completion popup at all (connection.skills is [] ⇒ composerSkills is empty)

So the App.tsx gate is the fix, and the mappers.ts change is the prerequisite that keeps the new session-sourced Skill list from being empty.

4 — partial activation, dedup, and failed refresh

To reach the partial / deferred branches I patched the daemon (not this PR's client code) to report activation: "partial", sessionsFailed: 1 for one skill; the PR's own files were byte-identical to the PR head in every run except the two deliberate mutation runs above.

• Partial → the PR arm issued exactly 1 GET /workspaces/…/skills and removed the entry; the base arm issued 0 and stayed stale.
• Authority returns → the next applied toggle (chart-lab) updated the PR arm with 0 refreshes, and the earlier web-search removal stuck — the fallback did not latch.
• Dedup → seeding skills.enabled makes one toggle write both skills.disabled and skills.enabled, so the daemon emitted two settings_changed events (id=9, id=10) sharing mutation.id=c123edc7-…. The PR arm still performed exactly one refresh. This is the only configuration in which I could observe two events for one toggle.
• Failed refresh → aborting the fallback request surfaces an error toast on the PR arm and nothing on the base arm:

Notes (non-blocking)

N1 — the new error message never reaches the user. pushToast('error', formatError(error, 'Failed to refresh composer skills')), and formatError(error, fallback) returns error.message whenever error instanceof Error. Every realistic failure here (network error, non-2xx) is an Error, so the fallback string is unreachable and the user sees a bare "Failed to fetch" with no hint that composer skills are now stale — see the screenshot above. Something like `Failed to refresh composer skills: ${formatError(error, '')}` would keep the context. (Pre-existing formatError convention, so purely a suggestion.)

N2 — the mutation bookkeeping grows without bound. skillMutationsByCwd[workspaceCwd] appends every mutation and is never pruned, and handledSkillMutationKeysRef only ever grows for the current context; the effect then re-scans the whole accumulated array on each new signal. For a long-lived tab where someone toggles skills repeatedly this is O(n) memory and O(n) work per event. A small cap (keep the last K mutations, drop ids once handled) would bound it. This is a code reading, not something I measured at runtime — the observed behaviour was correct.

N3 — reviewer test-plan step 3 is not reproducible through the UI. For a tab that has been opened but has not yet sent a prompt, I measured no GET /session/:id/events stream at all, therefore zero settings_changed frames and zero refreshes on both arms. bumpWorkspaceEventSignals is only called from the session event-stream handlers, and the daemon has no workspace-level event stream, so a session-less client cannot observe a skill mutation live. The session-less / deferred branch is therefore covered by the unit tests only. Not a defect introduced here, but the test plan promises something a reviewer cannot see.

Regression checks

Check	Result
packages/web-shell client/App.test.tsx	504 passed
packages/webui DaemonSessionProvider.test.tsx + mappers.test.ts	271 passed
npm run typecheck (web-shell, webui)	clean
eslint --max-warnings 0 on the 4 changed source files	clean

中文版本

#9131 本地真实环境验证 — ✅ 核心修复成立，2 条非阻塞建议

我在本地构建了这个 PR，并用真实的 qwen serve daemon + 真实浏览器（不是 mock daemon）驱动，与 merge base 构建并排对照，逐条核对测试计划里的声明。

结论：所描述的 bug 真实存在，本 PR 确实修好了它，未发现回归。 下面两条建议值得看一眼，但都不足以阻塞合并。

验证环境

PR 构建	7992505dda（本 PR head）
Base 构建	7bc0d80998（与 main 的 merge base）
Daemon	从 PR 源码起的一个 qwen serve（node scripts/dev.js serve --port 4270），绑定 fixture workspace
两条腿	两个 Vite dev server 指向同一个 daemon —— :5273 = PR，:5274 = base，因此两个标签页收到完全相同的 daemon 事件流
Workspace	3 个 project 级 skill（web-search、pdf-export、chart-lab）+ 10 个内置 skill，隔离的 QWEN_HOME
模型	本地 OpenAI 兼容 mock，session 是真的但输出确定
驱动	Playwright Chromium 1.61.1、macOS 26.6.2、Node 24.18.1
切换方式	第三个标签页走真实的 Skills 管理界面（Plugins → Skills → ⋮ → Disable/Enable），保证两条被测腿都是纯观察者

埋点：在页面内对 GET /session/:id/events 的 SSE 响应体做 tee，读取原始 settings_changed 帧；同时按腿统计路径中含 skills 的所有请求。

结果

#	场景	PR 构建（:5273）	Base 构建（:5274）
1	活跃 session 下禁用 web-search	自动补全中立即消失，0 次 workspace skill 刷新	仍然出现 —— 过期
2	活跃 session 下启用预先禁用的 pdf-export	出现	出现（无差异；base 本来就正常）
3	活跃 session，activation: "partial"（故障注入）	1 次 workspace skill 刷新，条目移除	0 次刷新，过期
4	该 partial 之后的下一次 applied 切换	0 次刷新，live 快照重新成为权威，之前的移除保持	过期
5	一次切换发出两条同 mutation id 的 settings_changed	1 次刷新（已去重）	0
6	让 fallback 刷新失败	弹出错误提示	无任何提示
7	新开标签页、尚未发过 prompt	不刷新	不刷新（见 N3）

1 —— 核心 bug 与修复

基线：两条腿都有活跃 session，都能补全出 /web-search（图 01）。随后在 Skills 管理页禁用 web-search（图 04）。10 秒后，PR 构建已经把它去掉，base 构建仍在推荐这个已禁用的 skill（图 02）。

两条腿收到的那条 settings_changed 帧为 activation: "applied"、sessionsRefreshed: 2、sessionsFailed: 0。注意 PR 构建在这里没有发出任何额外的 GET …/skills —— 在 applied 路径上它确实按设计信任 available_commands_update 而不再回源。

2 —— 为什么 base 只在「禁用」方向出错

来自实跑 daemon 的两个线上事实解释了这一点，值得记录：

• available_commands_update 的 skill 名单只放在 _meta.availableSkills 里，顶层 availableSkills 是 undefined。所以在 main 上 connection.skills 永远是 [] —— 本 PR 的 mappers.ts 改动才让它非空。
• skill 自己的 slash command 是通过 availableCommands 下发的，_meta.source = "skill-dir-command"，不是 "skill"。因此 main 的 command.source !== 'skill' 过滤永远删不掉它们，而过期的 /workspace/skills 快照只会往回加条目 —— 新启用的 skill 正常出现，新禁用的则永远残留。

3 —— 两处改动都是必需的（在 PR 树上做变异 A/B）

对 PR 构建施加的变异	结果
还原 commands memo 的判据（App.tsx 里 3 个 token：useWorkspaceSkillSnapshot → loadedSkillsReady）	bug 复现：已禁用的 /web-search 又被推荐
还原 mappers.ts 的 _meta.availableSkills 改动	/skills <TAB> 完全不弹补全（connection.skills 为 [] ⇒ composerSkills 为空）

即：App.tsx 的判据是修复本体，mappers.ts 是让新的「以 session 为准的 skill 列表」不为空的前置条件。

4 —— partial 激活、去重与刷新失败

为了走到 partial / deferred 分支，我给 daemon（不是本 PR 的客户端代码）打了补丁，让某个 skill 的切换上报 activation: "partial", sessionsFailed: 1；除上面两次刻意变异外，其余每一轮 PR 自身的文件都与 PR head 逐字节一致。

• Partial → PR 腿恰好发出 1 次 GET /workspaces/…/skills 并移除条目；base 腿 0 次、保持过期。
• 权威回归 → 之后一次 applied 切换（chart-lab）在 PR 腿上以 0 次刷新完成更新，且之前 web-search 的移除依然保持 —— fallback 没有粘住。
• 去重 → 预置 skills.enabled 后，一次切换会同时写 skills.disabled 与 skills.enabled，daemon 发出两条 settings_changed（id=9、id=10），共享同一个 mutation.id=c123edc7-…。PR 腿仍然只刷新了 1 次。这是我能构造出「一次切换两条事件」的唯一配置。
• 刷新失败 → 中断 fallback 请求后，PR 腿弹出错误提示，base 腿没有任何提示（图 03）。

建议（非阻塞）

N1 —— 新加的错误文案实际上永远不会出现。 pushToast('error', formatError(error, 'Failed to refresh composer skills'))，而 formatError(error, fallback) 在 error instanceof Error 时返回 error.message。这里现实中的失败（网络错误、非 2xx）都是 Error，所以 fallback 文案不可达，用户只会看到一句 "Failed to fetch"，完全看不出是 composer skill 过期了（见上图）。写成 `Failed to refresh composer skills: ${formatError(error, '')}` 之类可以保住上下文。（formatError 是既有约定，因此只是建议。）

N2 —— mutation 记账无上界增长。 skillMutationsByCwd[workspaceCwd] 会追加每一条 mutation 且从不裁剪，handledSkillMutationKeysRef 在当前上下文内也只增不减；每来一个新信号，effect 还要重新扫一遍整个累积数组。对于长期开着、反复切换 skill 的标签页，这是 O(n) 内存与每事件 O(n) 的开销。加个小上限（只保留最近 K 条、处理过的 id 及时丢弃）即可收敛。这一条是读代码得出的，不是实测 —— 实测行为是正确的。

N3 —— 评审测试计划第 3 步无法通过 UI 复现。 对于「已打开但尚未发过 prompt」的标签页，我实测到根本没有 GET /session/:id/events 流，因此两条腿都是 0 条 settings_changed、0 次刷新。bumpWorkspaceEventSignals 只在 session 事件流处理路径上被调用，而 daemon 没有 workspace 级事件流，所以无 session 的客户端无法实时观察到 skill mutation。session-less / deferred 分支因此只有单测覆盖。这不是本 PR 引入的缺陷，但测试计划承诺了评审者看不到的东西。

回归检查

检查	结果
packages/web-shell client/App.test.tsx	504 通过
packages/webui DaemonSessionProvider.test.tsx + mappers.test.ts	271 通过
npm run typecheck（web-shell、webui）	干净
对 4 个改动源文件跑 eslint --max-warnings 0	干净

[qwen-code-ci-bot]

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

fxied

[wenshao]

Partially reviewed — gaps disclosed.

2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

• R17-6 per-context handled-key history replay forces a redundant /workspace/skills fetch per session switch — core mechanism already reported in the open thread at App.tsx (comment 3798641362)
• R17-10 failed mutation-triggered reload parks the composer on the pre-toggle snapshot with no retry — same root cause as the open failed-reload thread (comment 3798641366)

Unresolved, please confirm:

• [Critical] R9-1 secondary clause (comment 3815862715, packages/web-shell/client/App.tsx:4744): the applied fast path ignores sessionsRefreshed — the client-side hole is probe-confirmed at this head (an applied/sessionsFailed:0/sessionsRefreshed:0 muta…

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

Deferred under the convergence posture (round 17, not a blocker) — recorded, not requested in this round:

• packages/webui/src/daemon/session/DaemonSessionProvider.tsx:4581 — [probe] new skillsVersion signal has no production read site; useDaemonSkills wires no reload
• packages/web-shell/client/App.tsx:499 — [probe] session-path argumentHint propagation in availableSessionSkillInfos is untested (spread-deletion mutant survives all 504 tests)
• packages/web-shell/client/App.test.tsx:4669 — [review] emitSkillMutation helper bumps skillsVersion and replaces lastSkillMutation for duplicate ids, unlike the provider's no-change semantics
• packages/web-shell/client/App.test.tsx:10805 — [review] no test asserts the mutation-driven reload fetches the connection's workspace (workspaceByCwd mock ignores cwd)
• packages/web-shell/client/App.test.tsx:10822 — [review] workspace re-entry/reconnect fires two identical /workspace/skills fetches per commit; the duplication is pinned by the call-count tests
• packages/web-shell/client/App.test.tsx:10898 — [probe] race tests resolve stale in-flight fetches with identical values — the staleness guard is unpinned (guard-deletion mutant ships 504/504 green)
• packages/web-shell/client/App.test.tsx:10881 — [probe] fast-path priorPending.length === 0 conjunct has no discriminating test (conjunct-deletion mutant ships 504/504 green)
• packages/webui/src/daemon/session/types.ts:558 — [review] resolved mutations resurrect on every reconnect — the ledger persists while handled keys are cleared on disconnect

中文说明

仅完成部分审查，审查缺口已披露。

本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。

未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。

未审查：反向审计——在 5 轮的反审轮数上限内未收敛。

收敛姿态下延后（第 17 轮，非阻断）——已记录，本轮不要求修改：共 8 条（原文未翻译，列表见上方英文部分）。

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

[qwen-code-ci-bot]

3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

• skillsVersion has zero production read sites (packages/webui/src/daemon/session/types.ts:557, DaemonSessionProvider.tsx:4579) — already recorded in the round-14/15/16 deferred lists
• skillMutationsByCwd append-only retention × handled-keys cleared on disconnect → reconnect/session-switch replay (packages/webui/src/daemon/session/types.ts:559) — already recorded in the round-8/9/10/15/16 deferred lists and open…
• within-batch seenSkillMutationIds dedup untested on the replay path (packages/webui/src/daemon/session/DaemonSessionProvider.tsx:4492) — already recorded in the round-8/9/10/11/14/16 deferred lists

中文说明

本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。

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

[qwen-code-ci-bot]

Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with @qwen-code /review. See workflow logs.

[qwen-code-ci-bot]

Partially reviewed — gaps disclosed.

6 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

• skillsVersion has zero production read sites (packages/webui/src/daemon/session/types.ts:557) — already recorded in the round-14/15/16/17 deferred lists
• emitSkillMutation helper bumps skillsVersion for duplicate ids (packages/web-shell/client/App.test.tsx:4668) — already recorded in the round-9 and round-17 deferred lists
• priorPending.length === 0 fast-path conjunct has no isolating test (packages/web-shell/client/App.tsx:4743) — already recorded in the round-14 and round-16 deferred lists
• skillMutationsByCwd append-only retention x handled-keys cleared on disconnect (packages/webui/src/daemon/session/DaemonSessionProvider.tsx:4590) — already recorded in the round-8/9/10/15/16/17 deferred lists and open threads
• in-batch seenSkillMutationIds dedupe untested on the replay path (packages/webui/src/daemon/session/DaemonSessionProvider.tsx:4492) — already recorded in the round-8/9/10/11/14/16/17 deferred lists
• catch-path staleness toast in reloadLoadedSkills (packages/web-shell/client/App.tsx:4687) — already reported as R1-1 (comments 3796222512, 3797837422, 3798641333), confirmed still open at this head

Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.

Deferred under the convergence posture (round 18, not a blocker) — recorded, not requested in this round:

• packages/webui/src/daemon/session/DaemonSessionProvider.tsx:4492 — [review] legacy mutation-less skills settings event untested (design-doc promise unverified)
• packages/web-shell/client/App.tsx:4746 — [probe] fast-path guard conjuncts (sessionId, activation, sessionsFailed) have no isolating tests — three deletion mutants ship 505/505 green
• packages/web-shell/client/App.test.tsx:11155 — [probe] unknown-skills reload test pins refresh count with a loose inequality — double-fire mutant ships green

中文说明

仅完成部分审查，审查缺口已披露。

本轮确认的 6 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。

未审查：反向审计——在 5 轮的反审轮数上限内未收敛。

收敛姿态下延后（第 18 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。

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

[wenshao]

@qwen-code /triage

[qwen-code-ci-bot]

Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 30 passed · 0 failed · 30 total

Flakiness gate: ✅ 3 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定：✅ 通过 · 可合入（agent 判定）

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。

脚本断言：30 通过 · 0 失败 · 30 总计

抖动门：✅ 3 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR #9131 verification — feat(web-shell): refresh composer skills incrementally after toggles

Verdict: merge-ready — 30/30 scripted assertions passed, 0 unexpected failures. Verified head: e51199b06f4956169f7021a71bcb00165e81d9cf (base tip cef54b8340). First round (no previous report).

中文摘要

• 结论: merge-ready。30/30 脚本化断言通过,0 个意外失败。
• A/B 结论: 核心行为被证明 load-bearing。wire-oracle(真实 daemon 帧形状)在 head 编译产物上 16/16 通过,base 编译产物在嵌套 _meta.availableSkills 形状上全部丢失 skills(控制组按预期变红);测试级 A/B 中,同一批 HEAD 测试文件在 HEAD 源码上全绿(App 520/520、webui 271/271),在 base 源码上 14 个新测试按预期失败,其中首个失败断言 expected [ { name: 'stale-skill' } ] to deeply equal [] 正是 issue feat(web-shell): refresh composer skills incrementally after Skill toggles #9123 描述的"旧快照重新加入已禁用 Skill"。
• Mutation 矩阵: 5 个单 guard 回退(M1 mapper 嵌套读取、M2 provider 去重、M3 composer 数据源选择、M4 调和 effect、M5 skills === undefined 子句)各自只红预期测试,无幸存者;M4 显示 applied 快速路径对该测试是冗余防御(删掉仍绿),分类为"redundant defence, correct as-is"。
• Findings: 无阻塞性问题。非阻塞观察:settingsVersion 不再为 skill_toggle 自增是安全的(skills.* 在设置对话框 showInDialog: false,四个消费方均不展示 skill 键);mapper 只接受包裹式 data.update 信封,与 base 行为一致,无回归;中间提交的 SDK bundle 预算上调未进入最终 diff(base 已是 208KB)。
• 未覆盖: 作者外部的 Chromium 5/5 E2E 矩阵(仓库内无对应已提交 spec,未复跑);逐 commit 归因(depth-2 checkout,84 个 commit 不可达);用真实 daemon 端到端驱动 feat(daemon): attach skill-toggle mutation metadata to settings_changed #9051 的 mutation 生产(已静态验证其在 base 存在,provider 测试以真实事件形状回放)。

Central claim and A/B proof

Central claim: after a Skill toggle, the Web Shell composer reflects the correct Skill list incrementally — an active session trusts the authoritative session command snapshot (a stale workspace snapshot must not reintroduce a disabled Skill); session-less / deferred / partial-activation paths refresh the workspace snapshot once per mutation id; a failed refresh is surfaced instead of swallowed.
Secondary claims: (a) the provider de-duplicates the two settings_changed events of one toggle into a dedicated skill signal without touching the generic settings signal; (b) the mapper reads the nested _meta.availableSkills wire shape the daemon actually emits.

Wire-oracle A/B on the compiled mapper (01-wire-oracle-mapper-ab.png)

mappers.ts was compiled standalone with esbuild from head and base; the two compiled files differ only by the hunk under test (verified by diff). Fixtures are lifted verbatim from the real producers: sendAvailableCommandsUpdateOrThrow in packages/cli/src/acp-integration/session/Session.ts (nested _meta.availableSkills + availableSkillDetails) and the SDK/browser egress redactor packages/cli/src/serve/skill-details-redaction.ts (details stripped, name list kept — the shape the Web Shell actually receives over SSE). 16/16 scripted assertions passed:

fixture (real daemon shape)	head skills	base skills	cell
F1 nested, pre-redaction	['web-search']	[]	flip
F2 nested, redacted SSE shape	['web-search']	[]	flip
F6 nested with non-strings	['ok'] (filtered)	[]	flip
F3 authoritative disable-all (empty nested)	[]	[]	parity
F4 flat legacy	['legacy-skill']	identical	parity
F5 both shapes present	['flat-skill'] (flat wins)	identical	parity
F7 neither	[]	identical	parity

This also proves the premise of the whole PR: the daemon emits availableSkills nested under _meta (Session.ts), the SDK does not flatten it, and the base mapper read only the flat key — so on base, connection.skills from live command updates is always [], which is precisely why the stale workspace snapshot always won.

Test-level A/B (02-test-ab-head-vs-base.png)

Identical HEAD test files run against head source vs base source (base worktree at HEAD^1, node_modules wired; realpath-asserted that @qwen-code/sdk resolves into the head tree — the PR leaves packages/sdk-typescript, package.json, package-lock.json untouched, and @qwen-code/webui is fully vi.mock-ed in the web-shell tests, so the control is clean; webui tests compile base source directly).

suite	@​ HEAD	@​ BASE	expectation on base
App.test.tsx	520/520	11 failed / 509 passed	the 11 new skill tests fail
DaemonSessionProvider.test.tsx	239/239	2 failed / 237 passed	the 2 new signal tests fail
mappers.test.ts	32/32	1 failed / 31 passed	the nested-read test fails

All 14 base-side failures are the intended behavioral assertions, not import/compile errors. The first App failure prints the exact bug from issue #9123: expected [ { name: 'stale-skill', … } ] to deeply equal [] — the stale workspace snapshot reintroducing a disabled Skill after an authoritative empty update. The one new App test that passes on base ("uses the workspace Skill snapshot while the session Skill list is unknown") asserts behavior base already had (always-workspace-snapshot), so it is not evidence either way on base and is pinned at HEAD by mutation M5 instead.

Mutation matrix (03-m1-live-mutation-red.png; full table in capture 02)

Single-guard reverts in a scratch worktree at HEAD, same test files, each row a real vitest run:

mutation	red set	classification
M1 mapper nested-read reverted	1 (nested wire-shape test); flat-preference control stays green	guard pinned
M2 provider dedupe reverted	2 (dedupe + replay-batch signal tests)	guard pinned
M3 composer source-selection reverted to loadedSkillsReady	4 (active-session snapshot, applied-no-refresh, partial-activation, workspace-scoped revalidation)	guard pinned
M4 reconciliation effect disabled	10 (every mutation-driven refresh path); the applied-path test stays green	guard pinned
M5 connection.skills === undefined clause removed	1 (unknown-session-list fallback test)	guard pinned

Union of M1–M5 red sets is 15; base red is 14 (base's always-workspace behavior coincidentally satisfies M5's test). Every guard the PR introduces is pinned by at least one mutation row, and every new test is pinned by at least one row or the base cell — no survivors, no vacuous tests. Positive controls: the flat-preference test inside M1's file and the 509/237/31 green tests in the base cells prove the harnesses collect and execute the mutated files.

Reviewer Test Plan walkthrough

1. Active session enable appears via live update, no reload — "uses the active session command snapshot for Skill enable and empty disable updates" asserts reloadSession not called and the command appearing; green @​ head, red @​ base. ✔
2. Disable last Skill → empty autocomplete — same test's second phase asserts skills === [] and the command gone. ✔
3. Pre-session toggle refreshes once; same mutation id not re-refreshed — "refreshes session-less composer Skills once for a deferred mutation" (loadSkillsStatus call count pinned at 2 across re-renders) plus provider dedupe test (two events, one id → skillsVersion +1). ✔
4. Partial activation: workspace snapshot temporarily authoritative, later live update regains authority — "uses a refreshed workspace Skill snapshot after partial activation". ✔
5. Failed fallback refresh surfaces; unrelated legacy settings event still reloads — "surfaces a failed deferred Skill snapshot refresh" (toast) + provider test's ui.theme event still bumping settingsVersion. ✔

All five steps are executable and pinned; no unreachable step.

Findings (non-blocking observations)

No blocking findings. The following are completeness notes, each verified, none changing the verdict:

1. Redundant defence, correct as-is: M4 shows the reconciliation effect's applied-fast-path is unobservable by the applied-path test (source selection alone passes it). It is still needed to clear a stale loadedSkillsFallback in other flows (its absence flips 10 other tests). Classification per the matrix taxonomy: redundant defence, not dead code — keep.
2. settingsVersion de-bump blast radius is bounded: skill_toggle events no longer bump the generic signal. Consumers enumerated: voice settings (unrelated keys), useDaemonProviders (unrelated), useDaemonSettings/settings dialog — and skills.disabled/skills.enabled are showInDialog: false in settingsSchema.ts, so no dialog consumer displays them. The skill-manager UI is driven by the new skill signal. No consumer left stale.
3. Envelope unchanged, no sibling regression: updateConnectionFromDaemonEvent accepts only the wrapped data.update envelope (identical at base); the flat persisted-transcript shape handled by the redactor serves other consumers and never reached this mapper at base either.
4. Intermediate bundle-budget commits are historical: the PR's commits mention raising the SDK browser bundle cap (186→190KB), but the aggregate diff touches no build config; base MAX_DAEMON_BROWSER_BUNDLE_BYTES is already 208KB. Nothing to gate.

Not covered

• The author's external Chromium 5/5 E2E matrix was not re-run; no committed Playwright spec covers this surface (checked packages/web-shell/client/e2e/), so there is nothing in-repo to execute. Unit + wire-oracle evidence stands in its place.
• Per-commit attribution: depth-2 checkout, git rev-list HEAD^1..HEAD^2 returns 1 vs 84 commits in the metadata; only the aggregate HEAD^1..HEAD diff was verified.
• Daemon-side mutation production (feat(daemon): attach skill-toggle mutation metadata to settings_changed #9051) verified present at base by code reading (workspace-service/index.ts emits mutation on settings_changed; SDK types at base) and by provider tests replaying the real event shapes — not by driving a live daemon end-to-end.
• Windows/Linux native browser runs (author-declared N/A).

Methodology

Environment: node:22-bookworm container, merge-ref checkout at depth 2 (HEAD merge, HEAD^1 base, HEAD^2 head). Base and mutation worktrees under tmp/ with the root node_modules symlinked; realpath of @qwen-code/sdk asserted to point into the head tree and shown harmless (SDK untouched by the PR; webui vi-mocked in web-shell tests). Wire-oracle harness (harness/mapper-wire-oracle.mjs) compiles head/base mappers.ts standalone (esbuild) and drives identical fixtures through the exported updateConnectionFromDaemonEvent — no mocks of the unit under test. Test-level cells and mutations ran the repo's own vitest suites; every mutation was applied in a scratch worktree and restored (sha-verified). Raw logs in logs/, harnesses in harness/, captures in evidence/ (01: wire-oracle run; 02: A/B + matrix parsed from the real logs; 03: live M1 mutation run). Gates: full packages/webui suite 599/599, full packages/web-shell suite 4113/4113, tsc --noEmit clean in both.

Flakiness gate log

rounds=5 files=3 skipped=0
file packages/web-shell/client/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/DaemonSessionProvider.test.tsx
file packages/webui/src/daemon/session/mappers.test.ts: (cd packages/webui) npx --no-install vitest run ./src/daemon/session/mappers.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/web-shell/client/App.test.tsx: PPPPP
  packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: PPPPP
  packages/webui/src/daemon/session/mappers.test.ts: PPPPP

verdict: pass
summary: 3 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 1 · packages/webui/src/daemon/session/mappers.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 2 · packages/webui/src/daemon/session/mappers.test.ts: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 3 · packages/webui/src/daemon/session/mappers.test.ts: P (exit 0)
round 4 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 4 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 4 · packages/webui/src/daemon/session/mappers.test.ts: P (exit 0)
round 5 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 5 · packages/webui/src/daemon/session/DaemonSessionProvider.test.tsx: P (exit 0)
round 5 · packages/webui/src/daemon/session/mappers.test.ts: P (exit 0)

Evidence images

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

— Qwen Code · sandboxed verification

[qwen-code-ci-bot]

LGTM, looks ready to ship. ✅

fixed

[qwen-code-ci-bot]

Released in v0.22.2.