feat(serve): scope extensions to workspace runtimes - #11086
Conversation
Verification reportBrowser-driven E2E was not run, per request not to control the browser.
|
|
Thanks for the PR! Template looks good ✓ — every required heading is filled in, and the 中文说明 mirrors the English body. Problem: Real, and verifiable in the tree rather than theoretical. This is the second layer of the contract landed in #10593, which explicitly deferred "capability convergence and catalog projections" as follow-up work. I checked the base: Direction: Aligned. It finishes an architecture this repo already merged, and it does so by extending existing shapes rather than adding parallel ones — the new routes mirror the skills runtime routes, the capability goes on the coordinator that #10593 established, and the SDK methods it calls ( Size: Core paths are touched — this spans four packages (
Title is Approach: Scope feels right for the stated goal, and the backward-compat handling is careful —
Risk: Stage 1e matched — Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需小标题都填写了，中文说明与英文正文对应。 问题： 真实存在，而且可以在代码树里验证，不是理论性加固。这是 #10593 所确立契约的第二层，那个 PR 明确把「capability 收敛与 catalog 投射」列为后续工作。我核对了基线： 方向： 对齐。它完成的是本仓库已经合并的架构，而且是扩展既有形态而不是另起一套 —— 新路由与 skills runtime 路由对称，capability 挂在 #10593 建立的协调器上，调用的 SDK 方法（ 规模： 触及核心路径 —— 跨四个包（
标题是 方案： 就目标而言范围合理，向后兼容处理也很谨慎 —— 当运行时快照未被认证为最新时，
风险： Stage 1e 命中 —— 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI wrote my own proposal before opening the diff: put an extensions capability on the coordinator that #10593 landed, carry a desired/applied generation plus the runtime epoch, reconcile through an ACP control method, add workspace-qualified routes mirroring the skills runtime routes, and have the Web Shell talk to the selected workspace client. That is essentially what this PR does, and it does it by extending existing shapes — Two things worth fixing, neither blocking on its own: 1. 2. A config-only refresh failure reports Things I checked and am not raising: the The three hand-rolled feature-flag ladders from my Stage 1 note stand as a question, not a finding — sequenceDiagram
participant P1 as Web Shell Extensions page
participant P2 as WorkspaceDaemonClient
participant P3 as serve extensions route
participant P4 as WorkspaceRuntimeCoordinator
participant P5 as AcpSessionBridge
participant P6 as ACP agent runtime
P1->>P2: extensionCatalog and workspaceExtensions (no ACP start)
P2-->>P1: catalog generation plus activation projection
P1->>P2: ensureRuntime
P2->>P3: POST workspace runtime ensure
P3->>P4: ensure
P4->>P5: preheat when cold
P4->>P5: invoke workspaceExtensionsReconcile
P5->>P6: ext method control call
P6-->>P5: config and session refresh counts
P4->>P5: getWorkspaceExtensionsStatus
P5-->>P4: live catalog stamped with runtimeEpoch
P4->>P4: certify epoch and generation then mark ready
P4-->>P1: runtime status carrying extensions capability
P1->>P2: workspaceRuntimeExtensions
P2-->>P1: live details merged only when certified current
Files changed (20 of 28 shown)
TestingThis is an unattended CI run, so I did not build or execute anything from this PR — the evidence below is the PR's own CI, read through the API for Not verified: the runtime behaviour. Nothing in the diff or the CI state so far demonstrates a live two-workspace daemon actually serving extension skills from the secondary runtime. The author's own report says browser-driven E2E was not run and Windows/Linux were not exercised locally — that is the author's claim about what they did, not evidence about what the code does, and I am not treating it as either. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification would settle this: 中文说明代码审查我在看 diff 之前先写了自己的方案：把 extensions capability 挂在 #10593 落地的协调器上，携带 desired/applied generation 与 runtime epoch，通过 ACP 控制方法协调，新增与 skills runtime 路由对称的工作区限定路由，Web Shell 改为与选中的工作区客户端通信。这个 PR 基本就是这么做的，而且是扩展既有形态 —— 复用了 有两处值得修，单独看都不构成阻断： 1. 2. 仅有 config 刷新失败时会上报 以下是我核查过但不提为问题的： Stage 1 提到的三处手写 feature-flag 阶梯仍然是疑问而非结论 —— 我会优先收敛 测试这是无人值守的 CI 运行，因此我没有构建或执行本 PR 的任何代码 —— 下面的证据是 PR 自身的 CI，通过 API 针对 未验证：运行时行为。diff 和目前的 CI 状态都没有证明一个真实的双工作区 daemon 确实能从次级运行时提供扩展技能。作者自己的报告说明未执行浏览器驱动 E2E，也未在 Windows/Linux 本地运行 —— 那是作者关于自己做了什么的陈述，不是关于代码实际行为的证据，我两者都不当作证据。 沙箱验证可以定论： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterℹ️ No screenshot changed against the PR base — but this PR edits 5 render-shaping files:
Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
|
Confidence: 3/5 — the review itself came back clean apart from two small fixes, but a 1335-production-line change spanning four packages is over the size threshold where this gate hands the call to a maintainer rather than approving it, and the unit suite has not finished. Going back to the proposal I wrote before opening the diff: this PR matches it, and beats it on the part I would most likely have got wrong. I said "carry a generation and an epoch and reject stale results" — the implementation actually does that in two independent places, stamping the epoch at bridge request time and then re-reading the lifecycle snapshot after the catalog comes back, so a runtime replaced mid-reconcile cannot advance readiness either way. It also folds the new async work into the lifecycle accounting that #10593 built instead of running around it. That is the difference between a change that looks right and one that fails closed. Does it solve something users care about? Yes, and it is concrete rather than architectural self-indulgence: today a selected secondary workspace can miss extension-provided skills entirely, because there is no So why not approve? Three reasons, in order of weight:
The two findings from the review are real but small, and I would not hold the PR on either alone: the injected-manager path repopulating the controller's own 2-second cache (one-line fix), and a config-only refresh failure reporting One process note: I could not resolve an owner to hand this to. The deterministic resolver found no matching area because this PR carries no labels, and there is no human review to fall back on, so I am not going to guess a login or assign it to someone who did not opt in. A maintainer picking this up — or adding the relevant area label and re-running ⏸️ Deferring to a maintainer. Nothing here looks wrong to me; it is too broad for this gate to sign off on its own, and CI has not landed. 中文说明Confidence: 3/5 —— 审查本身除了两处小修改之外是干净的，但一个跨四个包、1335 行生产逻辑的改动超过了本关卡应交回维护者判断的规模阈值，而且单元测试套件尚未跑完。 回到我在看 diff 之前写的方案：这个 PR 与之一致，并且在我最可能做错的那部分做得比我更好。我当时说的是「携带 generation 与 epoch，并拒绝陈旧结果」—— 实现实际上在两个相互独立的位置做到了这一点：在 bridge 请求时打上 epoch，并在 catalog 返回之后重新读取生命周期快照，因此协调过程中被替换的运行时无论走哪条路径都无法推进就绪态。它还把新的异步工作纳入了 #10593 建立的生命周期计账，而不是绕开它。这正是「看起来对」与「失败即封闭」之间的区别。 它是否解决了用户在意的问题？是，而且很具体，不是架构上的自我满足：今天一个被选中的次级工作区可能完全拿不到扩展提供的技能，因为 那为什么不批准？三个理由，按权重排列：
审查中的两处发现是真实的但都很小，任何一处单独都不足以压住这个 PR：注入 manager 的路径会重填控制器自己的 2 秒缓存（一行即可修复），以及仅有 config 刷新失败时在广播事件里上报 一个流程说明：我无法解析出接手人。确定性解析器没有找到匹配的区域，因为本 PR 没有任何标签，也没有可回退的人工 review，所以我不会去猜一个登录名、也不会把 PR 指派给没有主动参与的人。需要一位维护者接手 —— 或者补上相应区域标签后重跑 ⏸️ 转交维护者处理。在我看来这里没有错的东西；只是范围太广，本关卡不宜独自签署，而且 CI 尚未落地。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
…e-runtime-extensions-main # Conflicts: # docs/developers/qwen-serve-protocol.md
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "workspace_extensions_config_runtime" |
features[] |
— | "workspace_extension_mentions" |
— Qwen Code · serve A/B
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- the shared extensions status cache is written from a trust-forced manager, so GET /workspace/extensions can serve workspace-localized entries for an untrusted primary — already reported (issue comment 5550758390, item 1; still stands at 949…
- the reconcile
failedcount omitsconfigsFailed, reporting failed: 0 on a config-only refresh failure — already reported (issue comment 5550758390, item 2; still stands at 9493fa6, probe-confirmed)
Not reviewed: issue-fidelity — the closing-issue reference set could not be fetched (gh predates 2.72.0, so closingIssuesReferences is unavailable); the set is UNKNOWN, not empty. Agent 0 fetched the PR-named #10593 and replayed the narrated incident instead, and root-cause ownership was ruled from the PR own narrative..
Not explored to full depth (tool budget reached): chunk 2: design-doc claim daemon-workspace-runtime-extensions.md item 4 ("shows the workspace selector on the list page and the disabled selector in detail view") — I …; "agent reverse-audit (round 2)": what buildLocalExtensionsStatus() actually returns for an untrusted primary (i.e. whether the pre-diff trust-free GET /workspace/extensions listed any isAc…; "agent reverse-audit (round 2)": whether the ACP child can answer SERVE_STATUS_EXT_METHODS.workspaceExtensions with initialized: false while its channel is live (which would widen Finding 2…; "agent reverse-audit (round 1)": did not read diff lines 387-429 line by line — the re-flowed conditional-serve-features table tail of qwen-serve-protocol.md ; I verified the two added rows …; "agent reverse-audit (round 1)": did not independently count SERVE_CAPABILITY_REGISTRY keys / CONDITIONAL_SERVE_FEATURES entries to confirm the literal numbers 159 and 47; I verified only t…, and 16 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：issue-fidelity — the closing-issue reference set could not be fetched (gh predates 2.72.0, so closingIssuesReferences is unavailable); the set is UNKNOWN, not empty. Agent 0 fetched the PR-named #10593 and replayed the narrated incident instead, and root-cause ownership was ruled from the PR own narrative..
未探索到全部深度（达到工具调用预算）：chunk 2：design-doc claim daemon-workspace-runtime-extensions.md item 4 ("shows the workspace selector on the list page and the disabled selector in detail view") — I …；"agent reverse-audit (round 2)"：what buildLocalExtensionsStatus() actually returns for an untrusted primary (i.e. whether the pre-diff trust-free GET /workspace/extensions listed any isAc…；"agent reverse-audit (round 2)"：whether the ACP child can answer SERVE_STATUS_EXT_METHODS.workspaceExtensions with initialized: false while its channel is live (which would widen Finding 2…；"agent reverse-audit (round 1)"：did not read diff lines 387-429 line by line — the re-flowed conditional-serve-features table tail of qwen-serve-protocol.md ; I verified the two added rows …；"agent reverse-audit (round 1)"：did not independently count SERVE_CAPABILITY_REGISTRY keys / CONDITIONAL_SERVE_FEATURES entries to confirm the literal numbers 159 and 47; I verified only t…，另有 16 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| ...configuredEntry, | ||
| ...live, | ||
| updateState: configuredEntry.updateState, | ||
| isActive: |
There was a problem hiding this comment.
[Suggestion] The merge computes the selected workspace's effective activation into isActive, but the only reader short-circuits on defaultActivation — which in split mode is always defined — so the newly fetched authoritative per-workspace value is dead data on new surface.
extensionIsActive (ExtensionsManagerPage.tsx:156-166) reads workspaceActivation (when present and not 'inherit'), then defaultActivation, then isActive. ExtensionCatalogEntry.defaultActivation is a required field and GET /extensions always sets it (policy?.defaultActivation ?? 'enabled'), so in split mode the third branch is unreachable and the merged isActive is never consulted; every badge goes through extensionIsActive. For an extension disabled by a legacy path rule the projection reports workspaceActivation: null with effectiveActivation: 'disabled', so the badge reads the user-scope default. Two arms genuinely regress against the merge base: when the projection was unavailable pre-diff (.catch(() => null)) or had no matching entry, DaemonExtensionEntry carried no defaultActivation and the badge fell through to the real isActive — correct — whereas the catalog now always supplies one; and when activationCurrent is false the projection fields are dropped entirely, so a real workspaceActivation override disappears from the badge too. For a secondary workspace the page has no pre-diff comparison at all, so the wrong badge there is new surface.
Witness:
probe with the real exported mergeExtensionCatalog and a transcription of the nine-line display rule evaluated beside the merge-base transcription (git show 82612e3584), using the finding's own fixture (user default enabled, effectiveActivation: 'disabled', activationSource: 'legacy_path_rule', live runtime isActive: false):
groundTruth_effectiveActivation: "disabled"
converged: {"isActive":false,"defaultActivation":"enabled","workspaceActivation":"inherit","badge":"Enabled"}
baseWithProjectionEntry: {"isActive":false,"defaultActivation":"enabled","badgeAtBase":"Enabled"}
baseWithoutProjectionEntry:{"isActive":false,"defaultActivation":"(absent)","badgeAtBase":"Disabled"}
The third line is why the filed Critical/regression framing was downgraded: in the normal pre-diff case the badge already read "Enabled" over a disabled ground truth, so that arm is pre-existing behaviour in unchanged code.
Make the third branch reachable in split mode so the new runtime read earns its keep: prefer the merged isActive when the runtime leg converged — reorder to workspaceActivation -> (converged) isActive -> defaultActivation — or have mergeExtensionCatalog omit defaultActivation from a merged row whose runtime leg was certified.
The fix must not violate an existing fact: isActive: boolean; is required on DaemonExtensionEntry (packages/sdk-typescript/src/daemon/types.ts:4750) while ManagedExtensionEntry widened it to optional, so the ?? false arm at ExtensionsManagerPage.tsx:165 must stay for catalog-only rows; and the merge must not start preferring a stale runtime isActive, which is what runtimeCurrent certification exists to prevent.
Acceptance criterion: A case in extensions-manager-logic.test.ts plus a page-level assertion that a converged merge with effectiveActivation: 'disabled' and a user defaultActivation: 'enabled' renders Disabled; it must go red when the precedence is restored to defaultActivation first. Please apply the fix, then remove it again and confirm that test goes red.
中文说明
合并逻辑把所选工作区的有效激活计算进 isActive，但唯一的读取方会在 defaultActivation 上短路——而 split 模式下它总是有值——于是新获取的、权威的单工作区值成了新表面上的死数据。extensionIsActive 依次读取 workspaceActivation（存在且不为 inherit 时）、defaultActivation、最后才是 isActive；ExtensionCatalogEntry.defaultActivation 是必填字段，GET /extensions 也总会设置它，所以 split 模式下第三个分支不可达，合并出的 isActive 永不被查阅，而所有徽标都经过 extensionIsActive。对一个被 legacy path rule 禁用的扩展，投影上报 workspaceActivation: null 且 effectiveActivation: 'disabled'，徽标于是读用户级默认值。相较合并基线确实退化的有两处：改动前当投影不可用（.catch(() => null)）或没有匹配条目时，DaemonExtensionEntry 不带 defaultActivation，徽标会落到真实的 isActive——那是正确的——而现在目录总会提供它；以及当 activationCurrent 为假时投影字段被整体丢弃，一个真实的 workspaceActivation 覆盖也会从徽标上消失。对次级工作区而言页面根本没有改动前的可比状态，所以那里的错误徽标属于新表面。建议让第三个分支在 split 模式下可达，使新的运行时读取真正发挥作用：把顺序改为 workspaceActivation →（已收敛时的）isActive → defaultActivation，或让 mergeExtensionCatalog 在运行时那一腿已认证时从合并行中省略 defaultActivation。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Valid — the isActive computation is dead data (no component reads it; the page recomputes from defaultActivation/workspaceActivation). Deferred to the next round; the behavior will be pinned by tests.
中文说明
成立——isActive 的计算是死数据（没有组件读取；页面用 defaultActivation/workspaceActivation 重新计算）。顺延至下一轮；届时将用测试固定该行为。
There was a problem hiding this comment.
Not addressed in 0e3dbea201. This remains Suggestion-level and is deferred to a follow-up: this PR has passed the review-round convergence limit, so the latest pass was limited to correctness and regression fixes. Leaving this thread open.
There was a problem hiding this comment.
Rechecked against pushed head c80017edd0 (repairs in c031c91474).
Partially fixed: when the projection is missing/non-current, extensionIsActive now reads live isActive before the global default. The inherited/legacy-path-rule badge precedence described in the review remains pre-existing behavior and has not been redesigned. Keeping this thread open for that remaining presentation issue rather than treating the whole suggestion as fixed.
| expect( | ||
| container | ||
| .querySelector('[data-testid="extensions-page"]') | ||
| ?.getAttribute('data-workspace'), |
There was a problem hiding this comment.
[Suggestion] The new Extensions test's fixture makes the default selection equal the primary workspace, so it cannot distinguish selectedWorkspaceCwd from workspace.workspaceCwd and the retargeting plus the cwd-keyed remount this PR adds for that tab are unpinned.
The fixture sets workspaceState.workspaceCwd = '/work/a' with a trusted, so defaultWorkspaceCwd (PluginManagerPage.tsx:49-55) resolves selectedWorkspaceCwd to /work/a — the same value workspace.workspaceCwd already has. Replacing the wiring workspaceCwd={splitExtensionsRuntimeAvailable ? selectedWorkspaceCwd : workspace.workspaceCwd} with plain workspace.workspaceCwd, or dropping selectedWorkspaceCwd from the key, keeps the test green: a regression that leaves the extensions page on the primary workspace while the selector shows B ships undetected. The Skills suite avoids this by marking a untrusted so the default resolves to /work/b (:139-141, asserting /work/b at :180-183), and the MCP suite switches the selector to Secondary and re-asserts (:302-323); the extensions tab — the tab this PR is about — does neither.
Witness:
mutation with a positive control in the same tree:
INTACT: Test Files 1 passed | Tests 5 passed (5)
MUTANT (both selectedWorkspaceCwd uses removed): Test Files 1 passed | Tests 5 passed (5) <- still green
CONTROL (workspaceCwd={'/MUTATED-PRIMARY'}): x expected '/MUTATED-PRIMARY' to be '/work/a'
Either mark /work/a untrusted in this describe's fixture (so the default becomes /work/b and the assertion differs from workspace.workspaceCwd), or add a step that opens the combobox, selects /work/b, and asserts data-workspace becomes /work/b — mirroring the MCP test at :302-323.
The fix must not violate an existing fact: defaultWorkspaceCwd prefers the entry matching workspace.workspaceCwd when trusted — packages/web-shell/client/components/plugins/PluginManagerPage.tsx:49-55 — so a discriminating fixture must either mark that entry untrusted or drive the selector explicitly.
Acceptance criterion: The strengthened assertion must go red when workspaceCwd={splitExtensionsRuntimeAvailable ? selectedWorkspaceCwd : workspace.workspaceCwd} collapses to workspace.workspaceCwd, and when the key drops -${selectedWorkspaceCwd ?? ''}. Please apply the fix, then remove it again and confirm that test goes red.
中文说明
新的扩展标签页测试的 fixture 让默认选中项等于主工作区，因此它无法区分 selectedWorkspaceCwd 与 workspace.workspaceCwd，本 PR 为该标签页加入的重定向与按 cwd 键的重挂载都未被固定。fixture 设置 workspaceState.workspaceCwd = '/work/a' 且 a 受信任，于是 defaultWorkspaceCwd 把 selectedWorkspaceCwd 解析为 /work/a——与 workspace.workspaceCwd 同值。把接线 workspaceCwd={splitExtensionsRuntimeAvailable ? selectedWorkspaceCwd : workspace.workspaceCwd} 换成裸的 workspace.workspaceCwd，或从 key 中去掉 selectedWorkspaceCwd，测试仍然全绿：一个让扩展页停留在主工作区、而选择器显示 B 的回归会不被察觉地进入仓库。Skills 套件通过把 a 标为未受信任来避免这一点（默认解析为 /work/b），MCP 套件通过切换选择器再断言来避免；而本 PR 真正涉及的扩展标签页两者都没有做。建议要么在该 describe 的 fixture 中把 /work/a 标为未受信任（使默认值变成 /work/b，断言因此不同于 workspace.workspaceCwd），要么补一步打开下拉、选择 /work/b 并断言 data-workspace 变为 /work/b，照 MCP 测试的写法。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Valid — the PluginManagerPage fixture equals the primary workspace, so the test cannot catch a regression that ignores workspaceCwd. Deferred to the next round; the fixture will move off the primary.
中文说明
成立——PluginManagerPage 的 fixture 与主工作区相同，无法捕获忽略 workspaceCwd 的回归。顺延至下一轮；fixture 将改为非主工作区。
There was a problem hiding this comment.
Not addressed in 0e3dbea201. This remains Suggestion-level and is deferred to a follow-up: this PR has passed the review-round convergence limit, so the latest pass was limited to correctness and regression fixes. Leaving this thread open.
| const workspaceRuntimeExtensions = vi.fn(async () => ({ | ||
| extensions: [] as never[], | ||
| })); | ||
| const workspaceByCwd = vi.fn(() => ({ |
There was a problem hiding this comment.
[Suggestion] The only tests for the new composer Extension loader cannot fail for the two properties they exist to pin — that the catalog comes from the selected workspace's client, and that the runtime is ensured before the catalog is read.
workspaceByCwd returns the same stub for every cwd, and useComposerCore.ts:1577 already calls workspace.client.workspaceByCwd(atWorkspaceCwd) in the render body for globWorkspace/listDirectory, so expect(workspaceByCwd).toHaveBeenCalledWith('/secondary') is satisfied by pre-existing code and proves nothing about the new loader. Nothing asserts ordering or the returned value (both stubs resolve { extensions: [] }). Rewriting the override to resolve the client from workspace.workspaceCwd ?? atWorkspaceCwd and to drop the await before the read keeps both new tests green, while in production the composer would read the live catalog before the runtime is prepared (an empty list, cached for the whole menu session) and resolve the client from the wrong workspace — exactly the primary/selected confusion this change exists to remove.
Witness:
mutation with a discriminating probe (per-cwd clients, workspaceCwd: '/primary' on the workspace object, ensure held behind a gate):
BASELINE: 66/66 green; under the mutant 66/66 STILL green (the 1 failure is the probe)
INTACT probe: order while ensure pending ["ensure:/secondary"] -> after release ["ensure:/secondary","catalog:/secondary"]
MUTANT probe: order while ensure pending ["ensure:/primary","catalog:/primary"]
-> expected [ 'ensure:/primary', ...(1) ] to deeply equal [ 'ensure:/secondary' ]
Key the stub by cwd (vi.fn((cwd) => ({ cwd, ensureRuntime, workspaceRuntimeExtensions }))), assert the resolved catalog is the one tagged /secondary, assert sequencing with expect(ensureRuntime.mock.invocationCallOrder[0]).toBeLessThan(workspaceRuntimeExtensions.mock.invocationCallOrder[0]), and give the two stubs distinguishable payloads so the pass-through is pinned.
The fix must not violate an existing fact: useComposerCore.ts:1577 calls workspace.client.workspaceByCwd(atWorkspaceCwd) in the render body for the glob/dirList actions, so a toHaveBeenCalledWith assertion on that mock is satisfied by pre-existing code — the new assertion must key on the returned object's identity, not the call argument.
Acceptance criterion: The strengthened test must go red under the mutation named above (wrong-cwd client, or the read fired without awaiting ensure); today it does not. Please apply the fix, then remove it again and confirm that test goes red.
中文说明
新的输入区扩展加载器只有这两个测试，而它们对各自要固定的两个性质都无法失败——目录来自所选工作区的客户端，以及运行时在读取目录之前被 ensure。workspaceByCwd 对每个 cwd 返回同一个 stub，而 useComposerCore.ts:1577 在渲染体中就已经为 globWorkspace/listDirectory 调用了 workspace.client.workspaceByCwd(atWorkspaceCwd)，所以 expect(workspaceByCwd).toHaveBeenCalledWith('/secondary') 由既有代码就能满足，对新加载器毫无证明力。也没有任何断言检查顺序或返回值（两个 stub 都解析为 { extensions: [] }）。具体地：把该覆写改成从 workspace.workspaceCwd ?? atWorkspaceCwd 解析客户端、并在读取前不 await ensure，两个新测试仍然全绿，而生产中输入区会在运行时准备好之前读取 live 目录（得到空列表，并在整个菜单会话中被缓存）、并从错误的工作区解析客户端——正是本次改动要消除的主/选中混淆。建议按 cwd 键化 stub（vi.fn((cwd) => ({ cwd, ensureRuntime, workspaceRuntimeExtensions }))），断言解析到的目录是标记为 /secondary 的那一个，用 invocationCallOrder 断言先后顺序，并让两个 stub 返回可区分的内容以固定透传。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Valid — the composer extension loader tests register capabilities that short-circuit before the code under test. Deferred to the next round; positive witnesses will be added.
中文说明
成立——composer 扩展加载器测试注册的能力会在被测代码之前短路。顺延至下一轮；届时将补充正向见证。
There was a problem hiding this comment.
Not addressed in 0e3dbea201. This remains Suggestion-level and is deferred to a follow-up: this PR has passed the review-round convergence limit, so the latest pass was limited to correctness and regression fixes. Leaving this thread open.
There was a problem hiding this comment.
Rechecked against pushed head c80017edd0 (repairs in c031c91474).
Additional readiness/error/trust/retry cases exist, but the original happy-path fixture still does not prove every requested per-client identity and pending-ensure ordering property. No claim that the mutation witness is fully covered. Leaving this test-strengthening suggestion open.
| ) === true | ||
| ? { | ||
| async loadExtensionsStatus() { | ||
| await client.ensureRuntime(); |
There was a problem hiding this comment.
[Suggestion] The composer's @ext: / + Extension loader awaits a mutating POST .../runtime/ensure on a keystroke-driven read path, drops the withActionTimeout bound every sibling workspace action uses, propagates no abort, is re-issued on every menu open, and turns a rejection into a silently empty list.
Typing @ resets the provider cache (AddMenu.tsx:321-324; useAtMentionMenu.close() at :503; the workspaceKey effect at :476), so each open re-issues the ensure. On a workspace whose runtime is not live, the Extensions pane sits at loading: true with zero items for the ensure round trip — the client budget is 62 s (WORKSPACE_RUNTIME_ENSURE_TIMEOUT_MS = 60_000 + 2_000, DaemonClient.ts:419-423) against the legacy path's bounded 30 s withActionTimeout(client.workspaceExtensions(), ...) (client/daemon/workspace/actions.ts:538-544) — and the menu's abort.signal cannot cancel it because neither SDK method takes a signal (useAtMentionSources.ts:496 checks signal.aborted only after the await). When ensure rejects, createExtensionProvider.search rethrows and loadItems's catch sets items: [] with only a console.warn (useAtMentionMenu.ts:680-698); because the .catch at useAtMentionSources.ts:492-495 clears the cached promise on rejection, every subsequent debounced keystroke fires another ensure POST. The Skills equivalent in this same composer does the opposite: it reads the config catalog first so the menu is never empty and void-prefixes the ensure as fire-and-forget (App.tsx:7411-7448). The harm is bounded to the cold or non-converged runtime — on a live converged runtime ensure() short-circuits.
Witness:
probe, blocking and re-issue arms with a positive control:
order while ensure pending: ["ensure:/secondary"] <- nothing reaches the menu until ensure resolves
loader call count after 4 keystrokes = 4
outcomes = ["query=\"\" threw=workspace_draining","query=\"d\" threw=...","query=\"de\" threw=...","query=\"dem\" threw=..."]
loader call count on success path = 1 (three searches, one call) <- the instrument reports 1 when caching works
Declared stub: the rejection was an injected new Error('workspace_draining'); the probe settles the provider's retry/caching behaviour given any rejection.
Do not bootstrap a runtime from autocomplete: read the runtime status (a GET) first and call ensureRuntime() only when the runtime is not live, or void-prefix the ensure and return the config-side catalog immediately as the Skills loader does. Give the action an options?: { signal?: AbortSignal } so the menu's abort reaches both legs as it does for globWorkspace and listDirectory, and on ensure failure still attempt the read for the same workspace rather than rejecting the whole provider.
The fix must not violate an existing fact: const WORKSPACE_RUNTIME_ENSURE_TIMEOUT_MS = WORKSPACE_RUNTIME_ENSURE_SERVER_DEADLINE_MS + WORKSPACE_RUNTIME_ENSURE_CLIENT_HEADROOM_MS; = 60 000 + 2 000 (packages/sdk-typescript/src/daemon/DaemonClient.ts:419-424, commented "Keep in sync with DEFAULT_ENSURE_TIMEOUT_MS"), so a withActionTimeout wrapper at its DEFAULT_ACTION_TIMEOUT_MS = 30_000 default (packages/web-shell/client/daemon/timing.ts:9) would abort a legitimate in-progress ensure — any bound must exceed 62 s, or the fix must avoid calling ensure at all. docs/developers/qwen-serve-protocol.md:286 requires the selected workspace's live runtime as the source, so no primary fallback.
Acceptance criterion: Add cases to packages/web-shell/client/hooks/useComposerCore.dom.test.tsx where ensureRuntime rejects and where it never settles, asserting loadExtensionsStatus() still resolves with the workspace catalog or settles within a bound, plus a case passing an already-aborted signal asserting neither call is made; removing any bound must turn them red. Please apply the fix, then remove it again and confirm that test goes red.
中文说明
输入区的 @ext: / + 扩展加载器现在会在一次由按键驱动的读取路径上等待一个会产生副作用的 POST .../runtime/ensure：它丢掉了所有同类工作区动作都在用的 withActionTimeout 约束、不传播 abort、每次菜单打开都重新发起，并且把一次拒绝变成一个静默的空列表。输入 @ 会重置 provider 缓存（打开时、close() 时、以及 workspaceKey 变化时），所以每次打开都重新发起 ensure。在运行时不 live 的工作区上，扩展面板会以零条目停留在 loading: true 直到 ensure 往返结束——客户端预算是 62 秒，而被替换掉的旧路径是有 30 秒约束的 withActionTimeout(client.workspaceExtensions(), ...)——并且菜单的 abort.signal 无法取消它，因为两个 SDK 方法都不接受 signal（provider 只在 await 之后检查 signal.aborted）。ensure 被拒绝时，createExtensionProvider.search 重抛，loadItems 的 catch 只设置 items: [] 并 console.warn；由于 .catch 会在拒绝时清掉缓存的 promise，之后每一次去抖按键都会再发一次 ensure POST。同一输入区里的 Skills 等价实现做法相反：先读 config 目录以保证菜单永不为空，并把 ensure 以 void 前缀作为 fire-and-forget。危害仅限于冷启动或未收敛的运行时——在 live 且已收敛的运行时上 ensure() 会短路。建议不要从自动补全里引导运行时：先读运行时状态（GET），只在运行时不 live 时才调用 ensureRuntime()；或像 Skills 加载器那样 void 前缀 ensure 并立即返回 config 侧目录。同时给该动作加上 options?: { signal?: AbortSignal }，让菜单的 abort 像对 globWorkspace 与 listDirectory 那样传到两条腿，并在 ensure 失败时仍尝试对同一工作区读取，而不是让整个 provider 拒绝。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Valid — the composer loader awaits a mutating POST on the render path, so keystrokes can serialize behind a mutation. Deferred to the next round; the call will be serialized or short-circuited.
中文说明
成立——composer 加载器在渲染路径上等待一个会修改状态的 POST，按键可能被串行在变更之后。顺延至下一轮；该调用将被串行化或短路。
There was a problem hiding this comment.
Not addressed in 0e3dbea201. This remains Suggestion-level and is deferred to a follow-up: this PR has passed the review-round convergence limit, so the latest pass was limited to correctness and regression fixes. Leaving this thread open.
There was a problem hiding this comment.
Rechecked against pushed head c80017edd0 (repairs in c031c91474).
The silent-empty failure is fixed by the mention menu's error state/alert, and startup retry is bounded to three attempts. Awaiting selected-runtime ensure remains intentional: the durable global Extension inventory is not evidence of the selected runtime's available contributions, and no primary fallback is acceptable. SDK ensure already has its 62-second budget. Cancellation and a different cold-menu UX remain follow-up work; not adopting the proposed unconditional config fallback.
|
@qwen-code /takeover |
qwen-code-dev-bot
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
|
🤖 AutoFix ran out of time before finishing (timeout (7200000ms)) (attempt 1/100) — it will retry on the next scan. What I found before stopping: See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (7200000ms)）（第 1/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/33999036192 🧠 Handled by Qwen Code · model/模型 |
…iness (#11086) - Surface per-session command-update failures from workspaceExtensionsReconcile instead of swallowing them, so a stale command set can no longer be certified as reconciled. - Sanitize Extension refresh errors once at the coordinator (URL credentials, ANSI/control sequences, 500-char bound) covering both the persisted capability status and the extensions_changed broadcast. - Give the Extensions capability the Skills failed-revision guard so a failed prepare is terminal for the ensure path until an observed generation move or an explicit reconcile retries it. - Keep read-side generation observation pure: derived Skills/MCP capabilities are invalidated only on the reconcile path, which owns rescheduling them, so a queued reload can no longer be silently discarded by a GET. - Advertise the Extension runtime capability tags only when the primary workspace is trusted, since the routes they gate onto require trust. - Close the stale Extension reference picker when the scheduled-task form's target workspace changes. - Treat the split-mode uninstall 204 as a completed no-op instead of a queued operation that will never be polled.
|
🤖 Addressed the latest review feedback (round 2/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 2/100 轮）。改动内容与我反驳保留之处如下： Round summary — review feedback addressedCommit: This round implements the 9 Critical findings (10 inline threads, two pairs Resolved in code (10 threads)
Deferred to the next round (32 threads)All [Suggestion] threads: Reason: round-size bound after the budget warning — Critical findings first; Notes
VerificationCommands actually run, with results:
Mutation probes (guard removed/negated → focused test red → restored → green):
中文说明本轮总结 — 评审反馈处理提交： 本轮实现了全部 9 项 Critical 发现（10 条内联讨论，其中两对共用一个修复）。 已在代码中解决（10 条讨论）
顺延至下一轮（32 条讨论）全部 [Suggestion] 讨论： 原因：时间预算警告后的单轮规模约束——Critical 优先；每条讨论均已单独回复处理 备注
验证实际执行过的命令及结果：
变异探针（删除/取反守卫 → 聚焦测试转红 → 还原 → 转绿）：
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧵 Resolved all 10 selected review thread(s). · 已关闭全部选中的 10 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Integration Tests (no-AK, No Sandbox)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Integration Tests (no-AK, No Sandbox)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
…velope test (#11086) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 3/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 3/100 轮）。改动内容与我反驳保留之处如下： Autofix round summary — PR #11086Feedback addressed1. Failed check:
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
22 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-12 SDK recipe names ensureRuntime() with no receiver — already reported (round-1 inline comment on docs/developers/daemon/13-sdk-daemon-client.md:188)
- R1-13 documented merge condition weaker than the shipped client — already reported (round-1 inline comment on docs/developers/qwen-serve-protocol.md:284)
- R1-15 runtimeEpoch stamp on the Extension catalog untested — already reported (round-1 inline comment on packages/acp-bridge/src/bridge.ts:6202)
- R1-16 reconcile test pins only the returned counters — already reported (round-1 inline comment on packages/cli/src/acp-integration/acpAgent.test.ts:29762)
- R1-17 added refreshCache duplicates the refresh inside refreshTools — already reported (round-1 inline comment on packages/cli/src/acp-integration/acpAgent.ts:13543)
- R1-19 coordinator branch unreachable from the route tests — already reported (round-1 inline comment on packages/cli/src/serve/routes/workspace-extensions-controller.ts:716)
- R1-20 coordinator branch unreachable from the route tests — already reported (round-1 inline comment on packages/cli/src/serve/routes/workspace-extensions.ts:2455)
- R1-21 coordinator branch never reads options.skillsOnly — already reported (round-1 inline comment on packages/cli/src/serve/routes/workspace-extensions-controller.ts:718)
- R1-22 runtime-catalog route trust gate untested — already reported (round-1 inline comment on packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts:435)
- R1-23 test pins a lower generation moving desired and applied — already reported (round-1 inline comment on packages/cli/src/serve/workspace-runtime-coordinator.test.ts:186)
- R1-25 drain-rollback replay test counts only the catalog read — already reported (round-1 inline comment on packages/cli/src/serve/workspace-runtime-coordinator.test.ts:302)
- R1-26 cancelDrain replay double-schedules the derived reconciliations — already reported (round-1 inline comment on packages/cli/src/serve/workspace-runtime-coordinator.ts:157)
- R1-29 only the secondary branch of the dialog loader is tested — already reported (round-1 inline comment on packages/web-shell/client/components/dialogs/ScheduledTasksDialog.test.tsx:1427)
- R1-30 failed workspace activation projection swallowed to null — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:569)
- R1-31 split path's first apply() replaces detailed rows — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:588)
- R1-32 refetch when the certification gate is false — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:592)
- R1-34 two new user-facing strings bypass the i18n table — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:620)
- R1-35 no test executes the split-runtime arm of ExtensionsManagerPage — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:626)
- R1-36 fixture uses an activationSource outside the SDK union — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/extensions-manager-logic.test.ts:186)
- R1-38 merge overwrites the live runtime entry's updateState — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/extensions-manager-logic.ts:75)
- …and 2 more (see the run report)
Not reviewed: build-and-test — the test phase never started (the build set exhausted build-test's 600s budget), so no suite ran and no test behaviour is certified; the test-efficacy harness was never validated (harnessValidated null) and every probe was inconclusive on a missing generated prerequisite.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: issue-fidelity — the closing-issue reference set could not be fetched (gh 2.45.0 predates 2.72.0, so closingIssuesReferences is unavailable); the set is UNKNOWN, not empty.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": did not trace that a prompt session created from a composer mention is bound to the same atWorkspaceCwd whose runtime supplied the catalog — the "resolve in t…; "agent reverse-audit (round 1)": did not open ExtensionsManagerPage 's render, so the design doc's step 4 ("shows the workspace selector on the list page and the disabled selector in detail vi…; "agent reverse-audit (round 1)": the documented-behaviour-nothing-tests layer was traced by grep reference for the mentions gate ( useComposerCore.dom.test.tsx:267–298 ) instead of by reading i…; "agent reverse-audit (round 1)": whether the Web Shell refreshes workspace.capabilities after the runtime app replaces the bootstrap app — that settles whether the under-report above is a bri…; "agent reverse-audit (round 1)": whether refreshCacheWithSnapshot() can return a different generation for a manager built as (runtime.workspaceCwd, runtime.trusted) than for the poller's …, and 6 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/workspace-extensions-controller.ts:728 — [probe] coordinator branch broadcasts extensions_changed unconditionally, dropping the bridge's no-op suppression gatepackages/web-shell/client/hooks/useComposerCore.ts:1583 (+1 locations) — [probe] the new runtime Extension loaders discard the ensure result and never check catalog currencypackages/cli/src/serve/workspace-runtime-coordinator.ts:187 — [probe] the new extensionsQueuedWork term feeding the removal busy gate is pinned by no testpackages/web-shell/client/components/dialogs/ScheduledTasksDialog.tsx:878 — [probe] the dialog's workspace_extension_mentions gate has no negative-direction witnesspackages/cli/src/serve/routes/workspace-extensions.ts:768 — [probe] the poller permanently disables ensure()'s new boot fast path, so every cold start pays an inline reconcilepackages/cli/src/serve/workspace-runtime-coordinator.ts:1015 — [review] both deferral assignments in invalidateDerivedCapabilities are unreachable-true no-opspackages/cli/src/serve/workspace-runtime-coordinator.ts:994 — [review] the mutation-side half of the invalidation asymmetry has no testpackages/cli/src/serve/routes/workspace-extensions.ts:2403 — [probe] the primary runtime-catalog alias has no server-side testintegration-tests/cli/qwen-serve-routes.test.ts:389 — [test] the new end-to-end capability coverage sits outside every npm workspace, so the standard gate never collects itpackages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:604 — [review] the one caller that awaits load() clobbers the runtime error banner with a success message
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 22 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — the test phase never started (the build set exhausted build-test's 600s budget), so no suite ran and no test behaviour is certified; the test-efficacy harness was never validated (harnessValidated null) and every probe was inconclusive on a missing generated prerequisite.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查（原文为英文）：issue-fidelity — the closing-issue reference set could not be fetched (gh 2.45.0 predates 2.72.0, so closingIssuesReferences is unavailable); the set is UNKNOWN, not empty.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：did not trace that a prompt session created from a composer mention is bound to the same atWorkspaceCwd whose runtime supplied the catalog — the "resolve in t…；"agent reverse-audit (round 1)"：did not open ExtensionsManagerPage 's render, so the design doc's step 4 ("shows the workspace selector on the list page and the disabled selector in detail vi…；"agent reverse-audit (round 1)"：the documented-behaviour-nothing-tests layer was traced by grep reference for the mentions gate ( useComposerCore.dom.test.tsx:267–298 ) instead of by reading i…；"agent reverse-audit (round 1)"：whether the Web Shell refreshes workspace.capabilities after the runtime app replaces the bootstrap app — that settles whether the under-report above is a bri…；"agent reverse-audit (round 1)"：whether refreshCacheWithSnapshot() can return a different generation for a manager built as (runtime.workspaceCwd, runtime.trusted) than for the poller's …，另有 6 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 10 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.0)
| // error can carry git credentials, ANSI/control sequences, or unbounded | ||
| // output. Sanitize once at the producer so both sinks stay safe. | ||
| const sanitizeExtensionsErrorMessage = (message: string): string => | ||
| redactUrlCredentials(stripAnsiAndControl(message)).slice(0, 500); |
There was a problem hiding this comment.
[Suggestion] R2-7: This adds a second copy of the pre-existing sanitizeDaemonMessage in the same subsystem, so the daemon's credential/ANSI scrubbing rule now lives in two places that already disagree on where the 500-char cap is applied.
The two copies are applied to the same string on the failure path: the coordinator caps inside the helper, while the controller re-sanitizes the already-sanitized reconciliationError and caps at each call site (workspace-extensions-controller.ts:785, 800, 807, 812, all sanitizeDaemonMessage(...).slice(0, 500)); a third composition exists at packages/core/src/telemetry/session-tracing.ts:473. When the next secret shape is added — an Authorization: Bearer header or a ?token= query parameter — a fix landed in one copy leaves the other a stale near-duplicate, which is exactly the drift stripAnsiAndControl's own docstring warns against ("Centralised here so the rule can't drift between call sites … instead of leaving a stale near-duplicate vulnerable"). The extensions_changed broadcast and the persisted capabilities status are the sinks that would keep leaking.
Witness:
witness: not run — the nearest capability was a probe on both helpers with a new secret shape, which would only
demonstrate the drift after a hypothetical future edit. The duplication and the divergent cap placement are read
from source: sanitizeDaemonMessage at routes/workspace-extensions-controller.ts:41-43 (no cap inside) against
sanitizeExtensionsErrorMessage at workspace-runtime-coordinator.ts:47-48 (cap inside), with four call sites
re-capping the controller's output.
Hoist one helper — for example serve/sanitize-daemon-message.ts exporting sanitizeDaemonMessage(message: string, maxLength = 500) — and have both workspace-runtime-coordinator.ts and workspace-extensions-controller.ts import it, deleting the local copies and the per-call-site .slice(0, 500) calls.
The fix must not violate an existing fact: the coordinator cannot import the existing helper from packages/cli/src/serve/routes/workspace-extensions-controller.ts — that module already imports the coordinator at :34, so the hoist must land in a third module to avoid a cycle.
Acceptance criterion: packages/cli/src/serve/workspace-runtime-coordinator.test.ts → sanitizes Extension reconcile failures before broadcasting and persisting them, which pins both sinks (reconciliation.error and status().capabilities.extensions.error.message) for tok3n, ANSI escapes and length <= 500. It must stay green through the hoist, and go red if the shared helper drops either the cap or the redaction.
中文说明
这在同一子系统内新增了既有 sanitizeDaemonMessage 的第二份副本，于是 daemon 的凭据/ANSI 清理规则现在存在于两处，而且它们对 500 字符截断的应用位置已经不一致。
两份副本在失败路径上被应用于同一个字符串：coordinator 在辅助函数内部截断，而 controller 会对已经清理过的 reconciliationError 再清理一次，并在每个调用点各自截断（workspace-extensions-controller.ts:785、800、807、812，都是 sanitizeDaemonMessage(...).slice(0, 500)）；第三处组合存在于 packages/core/src/telemetry/session-tracing.ts:473。当下一种密钥形态被加入时——例如 Authorization: Bearer 头或 ?token= 查询参数——落在其中一份副本里的修复会让另一份成为过期的近似重复，而这正是 stripAnsiAndControl 自身文档注释所警告的漂移（“集中于此，规则就不会在调用点之间漂移……而不是留下一个易受攻击的过期近似副本”）。extensions_changed 广播与持久化的能力状态就是会继续泄漏的两个出口。
建议提取一个辅助函数——例如 serve/sanitize-daemon-message.ts，导出 sanitizeDaemonMessage(message: string, maxLength = 500)——让 workspace-runtime-coordinator.ts 与 workspace-extensions-controller.ts 都从它导入，删除各自的本地副本以及调用点上的 .slice(0, 500)。
修复不得违反的既有事实：coordinator 不能从 packages/cli/src/serve/routes/workspace-extensions-controller.ts 导入既有辅助函数——该模块在 :34 已经导入了 coordinator，因此提取必须落在第三个模块中以避免循环依赖。
验收标准：packages/cli/src/serve/workspace-runtime-coordinator.test.ts 中的 sanitizes Extension reconcile failures before broadcasting and persisting them，它为两个出口（reconciliation.error 与 status().capabilities.extensions.error.message）固定了 tok3n、ANSI 转义和 length <= 500。它在提取过程中必须保持绿色，而在共享辅助函数丢掉截断或脱敏任一项时必须变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Not addressed in 0e3dbea201. This remains Suggestion-level and is deferred to a follow-up: this PR has passed the review-round convergence limit, so the latest pass was limited to correctness and regression fixes. Leaving this thread open.
There was a problem hiding this comment.
Still deferred as of this round. The duplicate sanitizeDaemonMessage hoist remains Suggestion-level, and the thread already carries the author's deferral under the review-round convergence posture — this round was limited to the correctness regression (untrusted-primary 403s after the predicate revert, R2-10) and the unanswered coverage gap (R2-8). Leaving the thread open for the follow-up.
中文说明
本轮仍然暂缓。重复的 sanitizeDaemonMessage 抽取仍是 Suggestion 级，且该讨论串已有作者按评审轮次收敛策略做出的推迟回复——本轮只处理正确性回归（谓词回退后未信任主工作区的 403，R2-10）和唯一无人回复的覆盖缺口（R2-8）。讨论串保持开放，留待后续处理。
There was a problem hiding this comment.
Rechecked against pushed head c80017edd0 (repairs in c031c91474).
Not extracting another shared helper in this PR. Both current daemon sinks apply the existing core credential/control-character sanitizers and a 500-character cap; the suggested drift is a future-maintenance concern rather than a reproduced current leak. Consolidation remains open for follow-up, without introducing the controller/coordinator import cycle.
|
🔀 Base updated: red check(s) [Lint & Static (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Lint & Static (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
21 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R2-6 new toggle wired into only one of the two producers of the advertised feature set — still stands, already reported (round-2 inline comment on packages/cli/src/serve/server/serve-features.ts:164)
- R2-7 second copy of sanitizeDaemonMessage in the same subsystem — still stands, already reported (round-2 inline comment on packages/cli/src/serve/workspace-runtime-coordinator.ts:48)
- R2-8 the only ExtensionRuntimeRefreshError test asserts nothing about refreshed/failed — still stands, already reported (round-2 inline comment on packages/cli/src/serve/workspace-runtime-coordinator.test.ts:288)
- R2-9 resetReferenceState clears only the picker's candidate list — still stands, already reported (round-2 inline comment on packages/web-shell/client/components/dialogs/ScheduledTasksDialog.tsx:1348)
- R2-10 primaryWorkspaceTrusted gates both tags on the primary while the routes are trust-gated per target — still stands, already reported (round-2 inline comment on packages/cli/src/serve/capabilities.ts:719)
- shared extensions status cache written from a trust-forced manager — already reported (issue comment 5550758390 item 1, packages/cli/src/serve/routes/workspace-extensions-controller.ts:1063)
- reconcile failed count omits configsFailed — already reported (issue comment 5550758390 item 2, packages/cli/src/serve/workspace-runtime-coordinator.ts:407)
- both new coordinator branches unreachable from the unit suite — already reported (round-1 inline comments on packages/cli/src/serve/routes/workspace-extensions-controller.ts:716 and packages/cli/src/serve/routes/workspace-extensions.ts:2455…
- cancelDrain replay double-schedules the derived reconciliations — already reported (round-1 inline comment on packages/cli/src/serve/workspace-runtime-coordinator.ts:170)
- unreachable drain-deferral assignments in invalidateDerivedCapabilities — already reported (round-2 deferral on packages/cli/src/serve/workspace-runtime-coordinator.ts:1015)
- two new user-facing strings bypass the i18n table — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:621)
- merge overwrites the live runtime entry's updateState — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/extensions-manager-logic.ts:75)
- PluginManagerPage Extensions fixture equals the primary — already reported (round-1 inline comment on packages/web-shell/client/components/plugins/PluginManagerPage.test.tsx:239)
- runtimeEpoch stamp on the Extension catalog untested — already reported (round-1 inline comment on packages/acp-bridge/src/bridge.ts:6306)
- extensionsQueuedWork term in hasActiveWork() pinned by no test — already reported (round-2 deferral on packages/cli/src/serve/workspace-runtime-coordinator.ts:187)
- runtime-catalog route trust gate untested — already reported (round-1 inline comment on packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts:435)
- primary runtime-catalog alias has no server-side test — already reported (round-2 deferral on packages/cli/src/serve/routes/workspace-extensions.ts:2403)
- no test executes the split-runtime arm of ExtensionsManagerPage — already reported (round-1 inline comment on packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:627)
- new end-to-end capability coverage sits outside every npm workspace — already reported (round-2 deferral on integration-tests/cli/qwen-serve-routes.test.ts:389)
- poller reconciles an already-applied generation a second time — folded into R1-4 as its measured trigger (packages/cli/src/serve/routes/workspace-extensions.ts:799)
- …and 1 more (see the run report)
Not reviewed: issue-fidelity — the closing-issue reference set could not be fetched (gh 2.45.0 predates 2.72.0), so the set is UNKNOWN rather than empty; the motivating-incident replay ran against the PR's own narrative instead.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally, so the assertions this PR adds in integration-tests/cli/qwen-serve-routes.test.ts were never executed anywhere.
Not reviewed: build-and-test — the test-efficacy probe could not validate its harness (harnessValidated null: the positive control never ran, no probe file was green in the unmutated baseline) and ran no mutants or hunks (24+3 mutants and 70+6 hunks skipped for cap/baseline), so no coverage claim rests on it.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": did not compare the scope of the legacy DELETE /workspace/extensions/:name (used by actions.uninstallExtension ) against the new user-store DELETE /extensio…; "agent reverse-audit (round 2)": did not confirm that Extension.id always matches /^[a-f0-9]{64}$/ ( parseExtensionId , workspace-extensions.ts:1785-1795 ), so I could not rule out a 400 …; "agent 1c": did not execute the new/changed unit suites ( packages/cli/src/serve/workspace-runtime-coordinator.test.ts , packages/cli/src/serve/routes/workspace-qualified-…; "agent reverse-audit (round 1)": design-doc claim "Source installs use the V2 global route. Archive uploads remain on the legacy workspace route until a V2 archive endpoint exists, so they reta…; "agent reverse-audit (round 1)": design-doc fan-out claim "Global mutations invalidate every managed runtime. Workspace activation and resource-state mutations invalidate only the selected runt…, and 8 more.
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
docs/developers/qwen-serve-protocol.md:375 — [probe] the new wire-contract row carries no owning-capability annotationpackages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:623 — [probe] the retry condition enumerates two of the five capability statespackages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:589 — [probe] the split branch never reads runtime.errorspackages/web-shell/client/components/extensions/extensions-manager-logic.test.ts:170 — [probe] the updateState precedence assertion cannot failpackages/cli/src/serve/workspace-runtime-coordinator.ts:613 — [probe] no drain re-check after the preheat await, so a drain mid-flight still fires the mutating reconcilepackages/cli/src/serve/workspace-runtime-coordinator.ts:619 — [probe] any configsFailed makes the 30s poller re-drive the full refresh forever with no boundpackages/cli/src/serve/workspace-runtime-coordinator.test.ts:168 — [probe] no test drives a non-zero sessionsRefreshed, so the broadcast refreshed count is unpinnedpackages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:627 — [probe] the 2s retry has no cap or backoff and re-fetches twice per cycle plus a mutating preheatpackages/web-shell/client/components/plugins/PluginManagerPage.test.tsx:212 — [probe] no test pins the feature-absent Extensions branchdocs/design/workspace-runtime-architecture.md:983 (+3 locations) — [probe] the docs claim an Extensions config/runtime Catalog pair; only the runtime leg existspackages/cli/src/serve/routes/workspace-extensions.ts:781 — [probe] the poller's state !== 'stopping' conjunct is dead and untestablepackages/cli/src/serve/server/serve-features.ts:161 — [probe] both tags are advertised when trust is not authoritative while the primary runtime is trusted: falsepackages/cli/src/serve/workspace-runtime-coordinator.test.ts:181 — [probe] nothing pins the success-gate derived Skills/MCP reschedulepackages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:1026 — [review] the check-for-updates poll has no abort signal and no unmount teardown
Convergence: round 3 posted 14 inline comment(s), 6 of them reported for the first time; the previous round posted 13 (11 new). Findings keep coming back to the same files: packages/cli/src/serve/workspace-runtime-coordinator.ts (findings in rounds 1, 2; 2 more now); packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx (findings in round 2; 2 more now); packages/cli/src/serve/routes/workspace-extensions.ts (findings in round 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 21 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：issue-fidelity — the closing-issue reference set could not be fetched (gh 2.45.0 predates 2.72.0), so the set is UNKNOWN rather than empty; the motivating-incident replay ran against the PR's own narrative instead.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally, so the assertions this PR adds in integration-tests/cli/qwen-serve-routes.test.ts were never executed anywhere.
未审查（原文为英文）：build-and-test — the test-efficacy probe could not validate its harness (harnessValidated null: the positive control never ran, no probe file was green in the unmutated baseline) and ran no mutants or hunks (24+3 mutants and 70+6 hunks skipped for cap/baseline), so no coverage claim rests on it.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：did not compare the scope of the legacy DELETE /workspace/extensions/:name (used by actions.uninstallExtension ) against the new user-store DELETE /extensio…；"agent reverse-audit (round 2)"：did not confirm that Extension.id always matches /^[a-f0-9]{64}$/ ( parseExtensionId , workspace-extensions.ts:1785-1795 ), so I could not rule out a 400 …；"agent 1c"：did not execute the new/changed unit suites ( packages/cli/src/serve/workspace-runtime-coordinator.test.ts , packages/cli/src/serve/routes/workspace-qualified-…；"agent reverse-audit (round 1)"：design-doc claim "Source installs use the V2 global route. Archive uploads remain on the legacy workspace route until a V2 archive endpoint exists, so they reta…；"agent reverse-audit (round 1)"：design-doc fan-out claim "Global mutations invalidate every managed runtime. Workspace activation and resource-state mutations invalidate only the selected runt…，另有 8 条。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 14 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 14 条行内评论，其中 6 条是首次提出；上一轮发布了 13 条（其中 11 条首次提出）。发现反复回到同一批文件：packages/cli/src/serve/workspace-runtime-coordinator.ts（第 1、2 轮已出过发现，本轮又有 2 条）；packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx（第 2 轮已出过发现，本轮又有 2 条）；packages/cli/src/serve/routes/workspace-extensions.ts（第 2 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.0)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] R9-9 (packages/cli/src/serve/workspace-runtime-coordinator.ts:1188) — could not determine whether the failure cooldown still admits two re-drives per window: this round's agents split, one tracing steady state to a single failing attempt pe…
- [Critical] R10-2 (packages/cli/src/serve/routes/workspace-extensions.ts:2296) — could not determine: the ledger's claim text was not recoverable in full within this round's time budget, so the mechanism could not be re-traced.
- [Critical] 3 entries — could not determine: claim text not recoverable in full within the time budget.:
- R10-3 (packages/cli/src/serve/routes/workspace-extensions.ts:2239)
- R10-6 (packages/cli/src/serve/workspace-runtime-coordinator.ts:731)
- R10-7 (packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:1672)
- [Critical] R10-8 (whole-PR) — could not determine: the claim that narrowing the 30-second reconciler to live coordinator runtimes leaves cold workspaces unconverged; this round confirmed the narrowing is implemented and documented as intended, but c…
- [Critical] R11-2 (packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx) — could not determine whether the overlay-less first-paint merge still states "No description" as fact:
applyFirstPaintnow keeps the previous rows when non…
Not reviewed: issue-fidelity — the closing-issue reference set could not be fetched (gh 2.45.0 is older than the 2.72.0 the query needs), so no linked-issue evidence was available; the root-cause-ownership and motivating-incident duties still ran against the PR's own narrative.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and only the Linux suites ran locally, so no cross-platform verification of this change exists.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": the design doc's "Their preparation deadline cancels pending input" — I confirmed a preparation deadline exists ( workspace-extensions-controller.ts:569 , opti…; "agent reverse-audit (round 2)": "Prepared resources remain owned by the route until its finally disposal, including when the deadline prevents commit" — not traced to the route's finally b…; "agent reverse-audit (round 2)": the new design doc's Web Shell steps 3-4 and its "badges show unknown, not the global default" and notice-attribution/in-flight-lock sentences — not walked agai…; "agent reverse-audit (round 1)": did not trace @ext: resolution from the ACP prompt handler into the owning runtime's session config ( packages/cli/src/ui/hooks/atCommandProcessor.ts:268,295 …; "agent reverse-audit (round 1)": did not confirm from the workspace registry state machine that a registered secondary can be listed in /capabilities workspaces[] while state !== 'active' a…, and 26 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Not reviewed: "agent verify" — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
Deferred under the convergence posture (round 12, not a blocker) — recorded, not requested in this round:
docs/design/workspace-runtime-architecture.md:551 — [review] This diff declares the Extension generation/epoch contract…docs/developers/qwen-serve-protocol.md:526 — [review] The changed transition sentence describes a single forward…docs/developers/qwen-serve-protocol.md:602 — [review] The documented trusted field on the workspace activation…packages/acp-bridge/src/bridge.ts:6424 — [review] The only place that stamps runtimeEpoch onto a live…packages/cli/src/serve/routes/workspace-extensions.ts:2474 — [review] sendRuntimeCatalog awaits a cross-process runtime read…packages/cli/src/serve/routes/workspace-extensions.ts:2493 — [review] The new legacy-primary route has no daemon-side test:…packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts:1635 — [review] All six new tests hand-inline the same coordinator-runtime…packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts:3229 — [review] The route-conditional expectation is inert for the…packages/cli/src/serve/workspace-runtime-coordinator.test.ts:697 — [review] This PR adds this.extensionsQueuedWork > 0 to…packages/cli/src/serve/workspace-runtime-coordinator.ts:493 — [review] The reconciliation counters are only recoverable from an…packages/cli/src/serve/workspace-runtime-coordinator.ts:538 — [review] The coalescing of two *overlapping* deferred Extension…packages/cli/src/serve/workspace-runtime-coordinator.ts:823 — [review] The recoveringFromError term makes…packages/cli/src/serve/workspace-runtime-coordinator.ts:1193 — [review] Everything invalidateDerivedCapabilities writes except…packages/cli/src/serve/workspace-runtime-coordinator.ts:1259 — [review] The "one retry before latching" marker is revision-scoped…packages/web-shell/client/components/dialogs/ScheduledTasksDialog.tsx:900 — [review] The runtime-catalog freshness guard (ensure → capability…packages/web-shell/client/components/extensions/ExtensionsManagerPage.test.tsx:218 — [review] The new tests rebind the shared hoisted fixture…packages/web-shell/client/components/extensions/ExtensionsManagerPage.test.tsx:391 — [review] renders an unowned runtime Extension error in the detail…packages/web-shell/client/components/extensions/ExtensionsManagerPage.test.tsx:967 — [review] The second half of clears a stale refresh failure when a…packages/web-shell/client/components/extensions/ExtensionsManagerPage.test.tsx:2056 — [review] The last test never sets state.workspace.workspaceCwd ,…packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:706 — [review] The split-runtime load only re-arms its 2 s retry for…- …and 10 more (see the run report)
Convergence: round 12 posted 2 inline comment(s), 1 of them reported for the first time; the previous round posted 10 (4 new). Findings keep coming back to the same files: packages/cli/src/serve/routes/workspace-extensions.ts (findings in round 10; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R9-3 (packages/cli/src/serve/routes/workspace-extensions-controller.ts:752) — still stands: the deferral warning is chosen from a liveness sample taken before the reconcile, so a runtime that is live but draining is told to "Retry the runtime refresh" even though the coordinator has already queued that reconciliation and will replay it after the drain; the message asks the user for an action the daemon has already taken. Re-confirmed at the moved line this round (the finding is recorded at low confidence because the drain window could not be executed, but the sampling order is unchanged code).
[Critical] R10-4 (packages/cli/src/serve/routes/workspace-extensions.ts:2520) — still stands: the projection read re-sources appliedGeneration from the coordinator capability but observes the store generation with no storeReadRevision, so a fresh client read can never adopt an automatic backup recovery. After a rollback from generation 7 to 6 the same response body reports desiredGeneration: 6 beside appliedGeneration: 7 — a pairing the documented readiness rule treats as impossible — and holds it until the daemon-internal 30 s poller (which does sample the revision) repairs it.
[Critical] R11-1 (packages/cli/src/serve/workspace-runtime-coordinator.ts:49) — still stands: the sanitization invariant this diff declares in its own comment ("Sanitize once at the producer so both sinks stay safe") is enforced for the Extensions producer only. recordSkillsError and recordMcpError store the child runtime's raw error text verbatim into the same capabilities envelope and the same drain-cause log field, so the sibling capabilities can still carry unredacted credentials and ANSI control characters.
[Critical] R11-3 (packages/cli/src/serve/workspace-runtime-coordinator.ts:407) — still stands: observeExtensionGeneration is monotonic while the durable store can reuse a generation number for different content. Every mutation-driven reconcile observes without a hash and clears observedExtensionStoreHash, so the next hash-carrying poll records the new identity instead of diffing it — the reused-generation detection is disarmed for the whole window after any install/update/uninstall, which is the common case rather than the exception.
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 7 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：issue-fidelity — the closing-issue reference set could not be fetched (gh 2.45.0 is older than the 2.72.0 the query needs), so no linked-issue evidence was available; the root-cause-ownership and motivating-incident duties still ran against the PR's own narrative.
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and only the Linux suites ran locally, so no cross-platform verification of this change exists.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：the design doc's "Their preparation deadline cancels pending input" — I confirmed a preparation deadline exists ( workspace-extensions-controller.ts:569 , opti…；"agent reverse-audit (round 2)"："Prepared resources remain owned by the route until its finally disposal, including when the deadline prevents commit" — not traced to the route's finally b…；"agent reverse-audit (round 2)"：the new design doc's Web Shell steps 3-4 and its "badges show unknown, not the global default" and notice-attribution/in-flight-lock sentences — not walked agai…；"agent reverse-audit (round 1)"：did not trace @ext: resolution from the ACP prompt handler into the owning runtime's session config ( packages/cli/src/ui/hooks/atCommandProcessor.ts:268,295 …；"agent reverse-audit (round 1)"：did not confirm from the workspace registry state machine that a registered secondary can be listed in /capabilities workspaces[] while state !== 'active' a…，另有 26 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
未审查："agent verify"——启动 prompt 为它指定了 diff 中的行，但它从未打开：有工具调用，却没有一次读取 diff。
收敛姿态下延后（第 12 轮，非阻断）——已记录，本轮不要求修改：共 30 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 12 轮发布了 2 条行内评论，其中 1 条是首次提出；上一轮发布了 10 条（其中 4 条首次提出）。发现反复回到同一批文件：packages/cli/src/serve/routes/workspace-extensions.ts（第 10 轮已出过发现，本轮又有 1 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R9-3 (packages/cli/src/serve/routes/workspace-extensions-controller.ts:752) — still stands: the deferral warning is chosen from a liveness sample taken before the reconcile, so a runtime that is live but draining is told to "Retry the runtime refresh" even though the coordinator has already queued that reconciliation and will replay it after the drain; the message asks the user for an action the daemon has already taken. Re-confirmed at the moved line this round (the finding is recorded at low confidence because the drain window could not be executed, but the sampling order is unchanged code).
[Critical] R10-4 (packages/cli/src/serve/routes/workspace-extensions.ts:2520) — still stands: the projection read re-sources appliedGeneration from the coordinator capability but observes the store generation with no storeReadRevision, so a fresh client read can never adopt an automatic backup recovery. After a rollback from generation 7 to 6 the same response body reports desiredGeneration: 6 beside appliedGeneration: 7 — a pairing the documented readiness rule treats as impossible — and holds it until the daemon-internal 30 s poller (which does sample the revision) repairs it.
[Critical] R11-1 (packages/cli/src/serve/workspace-runtime-coordinator.ts:49) — still stands: the sanitization invariant this diff declares in its own comment ("Sanitize once at the producer so both sinks stay safe") is enforced for the Extensions producer only. recordSkillsError and recordMcpError store the child runtime's raw error text verbatim into the same capabilities envelope and the same drain-cause log field, so the sibling capabilities can still carry unredacted credentials and ANSI control characters.
[Critical] R11-3 (packages/cli/src/serve/workspace-runtime-coordinator.ts:407) — still stands: observeExtensionGeneration is monotonic while the durable store can reuse a generation number for different content. Every mutation-driven reconcile observes without a hash and clears observedExtensionStoreHash, so the next hash-carrying poll records the new identity instead of diffing it — the reused-generation detection is disarmed for the whole window after any install/update/uninstall, which is the common case rather than the exception.
— qwen3.8-max via Qwen Code /review (v0.23.2)
|
🔀 Base updated: red check(s) [Lint & Static (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Lint & Static (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
4 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- documented trusted field on the projection unasserted — docs/developers/qwen-serve-protocol.md:602 — already deferred in round 12
- only site stamping runtimeEpoch onto a live extensions status — packages/acp-bridge/src/bridge.ts:6423 — already deferred in round 12
- six tests hand-inline the same coordinator-arming fixture — packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts:1639 — already deferred in round 12
- runtime-catalog freshness guard duplicated — packages/web-shell/client/components/dialogs/ScheduledTasksDialog.tsx:900 — already deferred in round 12
Unresolved, please confirm:
- [Critical] R9-9 (packages/cli/src/serve/workspace-runtime-coordinator.ts:1188) — round 12 could not determine whether the failure cooldown still admits two re-drives per window; not re-ruled this round
- [Critical] R10-2 (packages/cli/src/serve/routes/workspace-extensions.ts:2296) — round 12 could not recover the claim text in full, so the mechanism could not be re-traced; not re-ruled this round
- [Critical] 3 entries — round 12 could not recover the claim text in full; not re-ruled this round:
- R10-3 (packages/cli/src/serve/routes/workspace-extensions.ts:2239)
- R10-6 (packages/cli/src/serve/workspace-runtime-coordinator.ts:731)
- R10-7 (packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:1672)
- [Critical] R10-8 (whole-PR) — round 12 could not determine whether narrowing the 30-second reconciler to live coordinator runtimes leaves cold workspaces unconverged; this round confirmed the narrowing and filed the untouched sibling doc sentence (D13…
- [Critical] R11-2 (packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx) — round 12 could not determine whether the overlay-less first-paint merge still states the absent-description text as fact; not re-ruled this round
- [Critical] 90 inline blocker threads from the 12 prior rounds were not individually re-ruled this round — the recovered incremental anchor was refused (behind-merge-base), so the round re-read the full 10348-line diff and the remaining budget went to …
Not reviewed: issue-fidelity — the closing-issue reference set could not be fetched (gh 2.45.0 is older than the 2.72.0 the query needs), so no linked-issue evidence was available; the root-cause-ownership and motivating-incident duties ran against the own narrative of the PR.
Not reviewed: reverse audit — stopped after round 1 by the review time budget; the loop did not converge (round 1 reported roughly 40 new findings, so two consecutive dry rounds were never reached).
Not reviewed: reverse-audit round 1 Suggestions (about 30 findings) — the verifier never ruled on them; reported terminal-only as unverified.
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and only the Linux suites ran locally, so no cross-platform verification of this change exists.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI; only integration-tests/cli/qwen-serve-routes.test.ts ran locally (38/38 passed), the rest of the integration suite did not run.
Not reviewed: test-efficacy probe — harnessValidated was null and all 16 probes came back inconclusive (the vitest globalSetup prerequisite guard stopped the runner before collection), so no mutation-coverage measurement exists for this diff.
Not explored to full depth (tool budget reached): chunk 26: did not run npx vitest run src/components/extensions/extensions-manager-logic.test.ts in packages/web-shell (needs built workspace dist/ per AGENTS.md, be…; chunk 3: whether a generic conformance test outside workspace-qualified-extensions.test.ts (e.g. integration-tests/cli/qwen-serve-routes.test.ts or multi-workspace-…; chunk 3: whether bridge.refreshExtensionsForAllSessions() can preheat a *legacy* (non-coordinator) runtime's channel, i.e. the legacy arm of the poller's pending filte…; "agent reverse-audit (round 1)": did not verify whether any daemon-spawned workspace runtime is actually constructed with enabledExtensionOverrides (the trigger link for the second finding — …; "agent reverse-audit (round 1)": did not run packages/web-shell vitest or typecheck against the new extensions-manager-logic.ts ; both findings are from direct code reads, not from an observ…, and 6 more.
Deferred under the convergence posture (round 13, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/serve/routes/workspace-extensions.ts:995 — [review] Critical [fails-closed] [new-surface] Interaction answer endpoint validates the client id…docs/design/workspace-runtime-architecture.md:9 — [review] Updated design exists only in Chinese under the English pathdocs/design/workspace-runtime-architecture.md:983 — [review] Status markers flipped to landed contradict the untouched…docs/developers/daemon/13-sdk-daemon-client.md:190 — [review] Untouched sibling doc still promises reconciler…docs/developers/qwen-serve-protocol.md:380 — [review] Documented 400/503/403 gate ordering is unpinned by any testdocs/developers/qwen-serve-protocol.md:526 — [review] Documented status chain is one-way but the wire moves…packages/cli/src/acp-integration/acpAgent.ts:13977 — [review] skillsOnly parameter-validation branch has no testpackages/cli/src/serve/routes/workspace-extensions.ts:791 — [review] Content-hash identity covers activation only, not artifactspackages/cli/src/serve/routes/workspace-extensions.ts:1941 — [review] GET /extensions pays a second discarded loadSettings() per…packages/cli/src/serve/routes/workspace-extensions.ts:2260 — [review] Adds the 4th and 5th copies of the interactive-operation…packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts:2083 — [review] No-op broadcast-suppression test has no positive controlpackages/cli/src/serve/workspace-runtime-coordinator.test.ts:715 — [review] Deferral coalescing of a full and a skills-only reconcile…packages/cli/src/serve/workspace-runtime-coordinator.ts:703 — [review] Overlapping reconciles both invoke a full runtime refreshpackages/web-shell/client/components/MessageList.dom.test.tsx:5160 — [review] Third copy of an unmount-timer test for a file this PR…packages/web-shell/client/components/dialogs/ScheduledTasksDialog.tsx:876 — [review] Verbatim second copy of the composer loader drops the…packages/web-shell/client/components/extensions/ExtensionsManagerPage.test.tsx:873 — [review] Both it.each owner cases take the identical global-notice…packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:460 — [review] Standalone extensions panel stays bound to the primary…packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:491 — [review] checkUpdates does not clear loadNoticeRef like its three…packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:700 — [review] Sticky loadNoticeRef lets a later load blank a mutation's…packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:766 — [review] Fixed 2s retry with no backoff, no cap, ignores Retry-After- …and 6 more (see the run report)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
[Critical] R13-1 [certifies-falsely] [new-surface] (packages/cli/src/serve/workspace-runtime-coordinator.ts:301) — when the initial Extension reconcile outlives the ensure() observation budget the wait is abandoned but the queued work is not, and ensure() then starts Skills/MCP on their independent tails; on the initial apply refreshesDerivedCapabilities is false, so afterExtensionApply never runs and both publish ready from a runtime read that predates the applied Extension catalog. Relocated from inline: the resolved anchor at this line collides with a different, already-posted claim (comment 3941458978, about a prepareExtensionsRevision readiness re-check), so this distinct confirmed Critical is carried in the body rather than dropped. Fix constraint: packages/cli/src/serve/workspace-runtime-coordinator.ts:1168-1171 — the comment states the invariant that a ready Skills/MCP status never certifies revisions predating the applied generation; and EXTENSIONS_RECONCILE_TIMEOUT_MS = 5 * 60_000 (:30) deliberately exceeds the ensure observation deadline, so a fix must not shorten the reconcile budget to make the race unreachable. Fix witness: a case in packages/cli/src/serve/workspace-runtime-coordinator.test.ts that holds the Extension reconcile in flight past the ensure budget on a generation-0 store, lets it settle, and asserts invalidateWorkspaceSkillsStatus ran and the Skills/MCP revision bumped; it must go red with the revision > 0 gate restored.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 4 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未决，请确认：共 8 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：issue-fidelity — the closing-issue reference set could not be fetched (gh 2.45.0 is older than the 2.72.0 the query needs), so no linked-issue evidence was available; the root-cause-ownership and motivating-incident duties ran against the own narrative of the PR.
未审查（原文为英文）：reverse audit — stopped after round 1 by the review time budget; the loop did not converge (round 1 reported roughly 40 new findings, so two consecutive dry rounds were never reached).
未审查（原文为英文）：reverse-audit round 1 Suggestions (about 30 findings) — the verifier never ruled on them; reported terminal-only as unverified.
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and only the Linux suites ran locally, so no cross-platform verification of this change exists.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI; only integration-tests/cli/qwen-serve-routes.test.ts ran locally (38/38 passed), the rest of the integration suite did not run.
未审查（原文为英文）：test-efficacy probe — harnessValidated was null and all 16 probes came back inconclusive (the vitest globalSetup prerequisite guard stopped the runner before collection), so no mutation-coverage measurement exists for this diff.
未探索到全部深度（达到工具调用预算）：chunk 26：did not run npx vitest run src/components/extensions/extensions-manager-logic.test.ts in packages/web-shell (needs built workspace dist/ per AGENTS.md, be…；chunk 3：whether a generic conformance test outside workspace-qualified-extensions.test.ts (e.g. integration-tests/cli/qwen-serve-routes.test.ts or multi-workspace-…；chunk 3：whether bridge.refreshExtensionsForAllSessions() can preheat a *legacy* (non-coordinator) runtime's channel, i.e. the legacy arm of the poller's pending filte…；"agent reverse-audit (round 1)"：did not verify whether any daemon-spawned workspace runtime is actually constructed with enabledExtensionOverrides (the trigger link for the second finding — …；"agent reverse-audit (round 1)"：did not run packages/web-shell vitest or typecheck against the new extensions-manager-logic.ts ; both findings are from direct code reads, not from an observ…，另有 6 条。
收敛姿态下延后（第 13 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 26 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
[Critical] R13-1 [certifies-falsely] [new-surface] (packages/cli/src/serve/workspace-runtime-coordinator.ts:301) — when the initial Extension reconcile outlives the ensure() observation budget the wait is abandoned but the queued work is not, and ensure() then starts Skills/MCP on their independent tails; on the initial apply refreshesDerivedCapabilities is false, so afterExtensionApply never runs and both publish ready from a runtime read that predates the applied Extension catalog. Relocated from inline: the resolved anchor at this line collides with a different, already-posted claim (comment 3941458978, about a prepareExtensionsRevision readiness re-check), so this distinct confirmed Critical is carried in the body rather than dropped. Fix constraint: packages/cli/src/serve/workspace-runtime-coordinator.ts:1168-1171 — the comment states the invariant that a ready Skills/MCP status never certifies revisions predating the applied generation; and EXTENSIONS_RECONCILE_TIMEOUT_MS = 5 * 60_000 (:30) deliberately exceeds the ensure observation deadline, so a fix must not shorten the reconcile budget to make the race unreachable. Fix witness: a case in packages/cli/src/serve/workspace-runtime-coordinator.test.ts that holds the Extension reconcile in flight past the ensure budget on a generation-0 store, lets it settle, and asserts invalidateWorkspaceSkillsStatus ran and the Skills/MCP revision bumped; it must go red with the revision > 0 gate restored.
— qwen3.8-max via Qwen Code /review (v0.23.2)
|
@qwen-code /resolve |
|
Qwen Code attempted to resolve merge conflicts but the run did not complete successfully. Check the workflow run for full logs. |
…e-runtime-extensions-main # Conflicts: # packages/cli/src/serve/workspace-runtime-coordinator.ts
… deferrals - the mutation receipt now pairs the committed generation with the committed store's content hash, so a recovery plus recommit that reuses the generation number is detected instead of silently re-recording a baseline (R10-1/R11-3) - an update supersedes only the same extension's parked operations with its own cancel wording; installs keep the name-blind install-replaces-install deadlock breaker (R12-1) - reconcile results flag drain-queued deferrals, and the controller samples runtime liveness after the await, so a drain deferral no longer tells the user to retry (R9-3) - an initial Extension apply that settles after ensure() abandoned its observation budget invalidates and re-drives the Skills/MCP capabilities that certified from a pre-catalog read (R13-1) - the projection GET passes the pre-read revision so a fresh read adopts an automatic backup recovery (R10-4) - Skills/MCP preparation errors are sanitized at the producer like Extension errors (R11-1) Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 5/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 5/100 轮）。改动内容与我反驳保留之处如下： Autofix round summaryThis round merged Feedback dispositionsrc:3978577182 (R12-1, cross-extension supersession) — fixed. rc:3978577192 (R10-1) + rc:3982030371 (R11-3, receipt path drops content identity) — fixed (one root cause). The controller now reads the post-commit store snapshot and passes its rc:3982030380 (R9-3, drain-deferred shares the retry message) — fixed. R13-1 (body; late-settling initial apply leaves pre-catalog Skills/MCP certified) — fixed. R10-4 (round-12 body, still open: projection read can never adopt a recovery rollback) — fixed. R11-1 (round-12 body, still open: Skills/MCP errors persisted raw) — fixed. "Could not determine" items — verified, no defect found
Verification
中文说明Autofix 本轮摘要本轮合并了 反馈处置rc:3978577182（R12-1，跨扩展取代）——已修复。 rc:3978577192（R10-1）+ rc:3982030371（R11-3，回执路径丢失内容标识）——已修复（同一根因）。 controller 现在读取提交后的 store 快照，将其 rc:3982030380（R9-3，drain 延期与「未应用」共用文案）——已修复。 R13-1（review 正文；初始 apply 迟于预算落账，Skills/MCP 以旧读数发布 ready）——已修复。 R10-4（第 12 轮正文仍未解决：projection 读永远无法采纳恢复回滚）——已修复。 R11-1（第 12 轮正文仍未解决：Skills/MCP 错误原文持久化）——已修复。 「无法确定」条目——已逐一核实，未发现缺陷
验证
Base-conflict check · 基分支冲突检查: conflicted with main — resolved in this push. · 与 main 有冲突——已在本次推送中解决。 🧵 Resolved all 4 selected review thread(s). · 已关闭全部选中的 4 条评审线程。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
…e-runtime-extensions-main # Conflicts: # packages/web-shell/client/components/dialogs/ScheduledTasksDialog.test.tsx # packages/web-shell/client/components/dialogs/ScheduledTasksDialog.tsx
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Unresolved, please confirm:
- [Critical] 89 inline blocker threads from the 12 prior rounds (of the 92 the comment-status index reports) were not individually re-ruled this round — the recovered incremental anchor was refused (behind-merge-base), so the round re-read the full 1174…
Not reviewed: reverse-audit rounds 1-2 Suggestions (about 54 findings) — the verifier never ruled on them; reported terminal-only as unverified.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (integration-tests/ is not an npm workspace, so the scoped test command never collects it).
Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and only the Linux suites ran locally, so no cross-platform verification of this change exists.
Not reviewed: test-efficacy probe — harnessValidated was null and all 17 revert probes came back inconclusive (the vitest globalSetup prerequisite guard aborts the disposable worktree before collection), so no mutation-coverage measurement exists for this diff.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": I did not grep packages/cli/src/serve/server.test.ts or run-qwen-serve.test.ts for an end-to-end assertion on the projection body's appliedGeneration — fi…; "agent reverse-audit (round 1)": did not confirm which structure fixes the advertised order that expect(features).toEqual([...EXPECTED_REGISTERED_FEATURES]) (server.test.ts:3166) compares aga…; "agent reverse-audit (round 1)": did not confirm entry.state === 'active' for the createSingleWorkspaceRegistry primary entry (server.ts:1356-1378), which the suggested trusted: true cont…; "agent reverse-audit (round 2)": did not execute the extensionSnapshotsCurrent → return false mutation against npx vitest run src/components/extensions/extensions-manager-logic.test.ts src…; "agent reverse-audit (round 2)": did not locate the production definition of workspace.actions.loadExtensionsStatus to confirm the legacy loader's literal route is /workspace/extensions as …, and 15 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Deferred under the convergence posture (round 14, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/serve/routes/workspace-extensions.ts:2277 — [review] Critical [fails-closed] [new-surface] Interaction answer route validates the client id against…packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx:1331 — [review] loadNoticeRef not reset by action paths blanks their notice
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
中文说明
仅完成部分审查，审查缺口已披露。
未决，请确认：共 1 条（原文未翻译，列表见上方英文部分）。
未审查（原文为英文）：reverse-audit rounds 1-2 Suggestions (about 54 findings) — the verifier never ruled on them; reported terminal-only as unverified.
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally (integration-tests/ is not an npm workspace, so the scoped test command never collects it).
未审查（原文为英文）：build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) were skipped in CI and only the Linux suites ran locally, so no cross-platform verification of this change exists.
未审查（原文为英文）：test-efficacy probe — harnessValidated was null and all 17 revert probes came back inconclusive (the vitest globalSetup prerequisite guard aborts the disposable worktree before collection), so no mutation-coverage measurement exists for this diff.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：I did not grep packages/cli/src/serve/server.test.ts or run-qwen-serve.test.ts for an end-to-end assertion on the projection body's appliedGeneration — fi…；"agent reverse-audit (round 1)"：did not confirm which structure fixes the advertised order that expect(features).toEqual([...EXPECTED_REGISTERED_FEATURES]) (server.test.ts:3166) compares aga…；"agent reverse-audit (round 1)"：did not confirm entry.state === 'active' for the createSingleWorkspaceRegistry primary entry (server.ts:1356-1378), which the suggested trusted: true cont…；"agent reverse-audit (round 2)"：did not execute the extensionSnapshotsCurrent → return false mutation against npx vitest run src/components/extensions/extensions-manager-logic.test.ts src…；"agent reverse-audit (round 2)"：did not locate the production definition of workspace.actions.loadExtensionsStatus to confirm the legacy loader's literal route is /workspace/extensions as …，另有 15 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
收敛姿态下延后（第 14 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 2 条（原文未翻译，列表见上方英文部分）。
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.3)
| @@ -515,7 +523,7 @@ An operation snapshot has this shape: | |||
| } | |||
| ``` | |||
|
|
|||
| `status` transitions from `queued` to `running`, then to `succeeded`, `succeeded_with_warnings`, or `failed`. While running, `phase` is `preparing`, `committing`, or `reconciling`. Terminal success may include `result` with `status` equal to `installed`, `enabled`, `disabled`, `updated`, `uninstalled`, `checked`, or `refreshed`; reconciliation results can additionally contain `refreshed`, `failed`, and `error`, while batch activation results contain ordered `results`. Update checks return `result.states`, keyed by extension name, with values such as `checking for updates`, `update available`, `up to date`, `not updatable`, or `error`. Credentials and authorization headers are never operation fields. | |||
| `status` transitions from `queued` to `running`, optionally `waiting_for_input`, then to `succeeded`, `succeeded_with_warnings`, or `failed`. While running, `phase` is `preparing`, `committing`, or `reconciling`. Terminal success may include `result` with `status` equal to `installed`, `enabled`, `disabled`, `updated`, `uninstalled`, `checked`, or `refreshed`; reconciliation results can additionally contain `refreshed`, `failed`, and `error`, while batch activation results contain ordered `results`. Update checks return `result.states`, keyed by extension name, with values such as `checking for updates`, `update available`, `up to date`, `not updatable`, or `error`. Credentials and authorization headers are never operation fields. | |||
There was a problem hiding this comment.
[Critical] R14-2: [certifies-falsely] [new-surface] The operation-status transition chain this diff rewrites promises one optional waiting_for_input before a terminal status, but the code re-parks with a fresh interaction.id per promptable setting.
The rewritten sentence documents queued → running → [waiting_for_input] → terminal. In fact every accepted answer resets the operation to status: 'running', phase: 'preparing', interaction: undefined, and the preparation loop prompts sequentially, so the real chain is running → waiting_for_input → running → waiting_for_input → … → terminal. The adjacent new paragraph at :508-509 states neither the repeat nor the id freshness.
Failure scenario: an extension declares two settings that need prompting. POST /extensions/install returns 202; packages/core/src/extension/extensionSettings.ts:250-253 iterates promptForSensitive.concat(promptForEnv) awaiting each answer, and each park mints id: crypto.randomUUID(). A client written against this documented chain treats the first waiting_for_input as the only one, so it stops polling after answering once and hangs; a client that caches the first interaction.id and replays it after the second park gets 404 Extension interaction not found.
Witness — probe on the PR commit (unmodified), one install whose prepare prompts twice:
PROBE-C5 first park answer status: 200
PROBE-C5 replaying the FIRST interaction id now returns: 404
PROBE-C5 answering the second park: 200
PROBE-C5 observed status sequence: waiting_for_input#fa1b4ca2-581c-4999-ad53-f82c3aab30ed
-> waiting_for_input#3ba52d2b-b1c5-4da3-859b-7dfc7776213c
| terminal: {"operation":"install","status":"succeeded","result":{"status":"installed",...}}
baseline: new-surface — git show 2f426a64f4:docs/developers/qwen-serve-protocol.md line 518 reads "status transitions from queued to running, then to succeeded, …" with no waiting_for_input clause, so the inaccurate clause is a + line of this diff.
Suggested fix: state the cycle and the per-park id, e.g. replace the first clause with: "status transitions from queued to running, then to succeeded, succeeded_with_warnings, or failed. An operation that needs user input parks in waiting_for_input and returns to running when its answer is accepted; it may park repeatedly (once per required setting), and each park carries a fresh interaction.id — an id from an earlier park answers 404." The same sentence is a good place to record the real running → queued backward edge (workspace-extensions-controller.ts:579-591), which is pre-existing.
Please add a case to packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts that drives an install prompting for two settings and asserts the observed sequence parks twice with two distinct interaction ids, and that replaying the first id returns 404; it must go red if the documented single-park chain is what the code implements.
中文说明
本次 diff 改写的操作状态流转链承诺「终端状态之前至多一次可选的 waiting_for_input」，但代码会为每一个需要提示的设置重新挂起，并且每次都生成全新的 interaction.id。
改写后的句子描述的是 queued → running → [waiting_for_input] → terminal。实际上每次应答被接受后，操作都会重置为 status: 'running', phase: 'preparing', interaction: undefined，而准备循环是逐个顺序提示的，因此真实链路是 running → waiting_for_input → running → waiting_for_input → … → terminal。相邻的新段落（:508-509）既未说明会重复挂起，也未说明 id 每次都会更新。
触发场景： 某扩展声明了两个需要提示的设置。POST /extensions/install 返回 202；packages/core/src/extension/extensionSettings.ts:250-253 会遍历 promptForSensitive.concat(promptForEnv) 并逐个等待应答，每次挂起都通过 id: crypto.randomUUID() 生成新 id。按本文档链路实现的客户端会把第一次 waiting_for_input 当作唯一一次，于是应答一次后便停止轮询而挂死；而缓存了首个 interaction.id 并在第二次挂起后重放它的客户端会得到 404 Extension interaction not found。
证据（在未修改的 PR 提交上探测，一次 prepare 提示两次的 install）：第一次挂起应答返回 200；重放第一个 interaction id 返回 404；应答第二次挂起返回 200；观测到的状态序列为两个不同 id 的 waiting_for_input，终态为 {"operation":"install","status":"succeeded"}。baseline: new-surface——git show 2f426a64f4:docs/developers/qwen-serve-protocol.md 第 518 行原文为「status 由 queued 转为 running，随后转为 succeeded……」，不含 waiting_for_input 从句，因此不准确的这句正是本 diff 的 + 行。
建议修复： 明确写出该循环与「每次挂起都有新 id」，例如将首个从句替换为：「status 由 queued 转为 running，随后转为 succeeded、succeeded_with_warnings 或 failed。需要用户输入的操作会停在 waiting_for_input，其应答被接受后回到 running；它可以反复挂起（每个必需设置一次），且每次挂起都携带全新的 interaction.id——使用早前挂起的 id 应答会返回 404。」同一句也适合记录真实存在的 running → queued 反向边（workspace-extensions-controller.ts:579-591，属既有行为）。
请在 packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts 中补充用例：驱动一个需要提示两个设置的 install，断言观测到的序列挂起两次且两次的 interaction id 不同，并重放第一个 id 返回 404；若代码实现的仍是文档所述的单次挂起链路，该用例必须为红。
— qwen3.8-max via Qwen Code /review (v0.23.3)
There was a problem hiding this comment.
Reviewed against 4e35093. The documentation omission is valid: an operation can repeatedly enter waiting_for_input, each prompt has a fresh interaction.id, and replaying an earlier ID returns 404. This is not a runtime Critical: ExtensionsManagerPage restarts polling after each answer and showInteraction handles changed IDs, so the existing client supports sequential prompts. Per the author’s decision, defer the wording clarification (including the running → queued preparation edge) as a documentation follow-up rather than expanding this already heavily reviewed PR. No runtime change is proposed for R14-2.
There was a problem hiding this comment.
[Critical] R14-2: The newly added lifecycle sentence describes a single optional excursion into waiting_for_input before a terminal status, but an answered interaction returns the operation to running and the same operation can park again — once per prompt.
Answering flips the operation back with status: 'running', phase: 'preparing', interaction: undefined (workspace-extensions.ts:1066-1071), and waitForExtensionInteraction explicitly re-admits an operation already in waiting_for_input (:630-637), so the cycle repeats. It repeats in practice whenever an extension declares more than one setting to prompt for: maybePromptForSettings awaits requestSetting in a loop over settingsChanges.promptForSensitive.concat(settingsChanges.promptForEnv) (packages/core/src/extension/extensionSettings.ts:248-253), and a marketplace install prompts for the plugin choice first and then for settings (extensionManager.ts:2135). An integrator who implements the documented sequence literally — park on waiting_for_input, answer it, then wait for one of the three terminal statuses — never answers the second prompt; the operation sits until the 10-minute interaction deadline and fails with Extension interaction timed out, so the install is lost and the first answer was wasted. Cost is a client whose state machine rejects or ignores a legal, recurring state on the wire.
Witness
witness: not run — a probe would park one install operation twice and quote the two `waiting_for_input` snapshots either side of an answer; it needs a scratch tree, which `qwen review scratch-tree` refused (`available: false`). The recurrence is quoted from code above, and no existing test observes it: every `waiting_for_input` assertion in `packages/cli/src/serve/server.test.ts` (7519, 7551, 7689, 7835, 7913, 7980, 7992, 8136, 8157, 8592, 8605) belongs to a single-interaction scenario, and `answerSettingInteraction` in `workspace-qualified-extensions.test.ts:333-355` answers once.
Suggested fix
State the cycle in the same sentence, e.g. "status transitions from queued to running; an operation that needs input moves to waiting_for_input and returns to running when the interaction is answered, and may do so once per prompt; it ends at succeeded, succeeded_with_warnings, or failed."
Fix witness — N/A (docs-line change; the recurring-prompt behaviour is already pinned by the answer→running→waiting_for_input cycles in packages/cli/src/serve/server.test.ts, e.g. the interaction-timeout and supersede cases around :7861-8140).
中文说明
[Critical] R14-2 本 diff 重写的操作状态迁移链只承诺一次可选的 waiting_for_input，但代码允许同一个操作反复进入该状态——每个需要提示的设置一次。
回答交互后操作会以 status: 'running', phase: 'preparing', interaction: undefined 回到运行态（workspace-extensions.ts:1066-1071），而 waitForExtensionInteraction 明确允许已处于 waiting_for_input 的操作再次进入（:630-637），因此该循环会重复。当一个扩展声明了多个需要提示的设置时，这在实践中必然发生：maybePromptForSettings 会在 promptForSensitive.concat(promptForEnv) 上循环 await requestSetting（packages/core/src/extension/extensionSettings.ts:248-253），而市场安装会先提示选择插件、再提示设置（extensionManager.ts:2135）。严格按文档实现状态机的集成方在回答第一个提示后只会等待三个终态之一，于是永远不会回答第二个提示；操作会一直挂到 10 分钟的交互超时并以 Extension interaction timed out 失败，安装丢失且用户第一次输入被丢弃。
建议的修复
在同一句话里说明这个循环，例如：「status 由 queued 迁移到 running；需要输入的操作会进入 waiting_for_input，交互被回答后回到 running，且每个提示可能发生一次；最终以 succeeded、succeeded_with_warnings 或 failed 结束。」
— qwen3.8-max via Qwen Code /review (v0.23.3)
There was a problem hiding this comment.
Verified the claim against the code: an accepted answer returns the operation to running and the preparation loop parks again once per promptable setting, each park minting a fresh interaction.id, so replaying an earlier id answers 404 — the rewritten sentence does read as a single optional park. Per the maintainer's ruling on this thread (rc:3987761094), the wording clarification — including the pre-existing running → queued edge — is deferred to a documentation follow-up rather than expanding this PR, so no change is landing here this round. Recorded in the deferred-findings queue so it survives the merge; leaving the thread open for that follow-up.
中文说明
已对代码核实该论断：应答被接受后操作回到 running，准备循环会按每个需提示的设置再次挂起，且每次挂起都生成全新的 interaction.id，重放旧 id 会得到 404 —— 改写后的句子确实读起来像只有一次可选挂起。按照维护者在本线程的裁定（rc:3987761094），该措辞澄清（包括既有的 running → queued 反向边）延后到文档后续跟进，不再扩大本 PR，因此本轮不在此落地改动。已记录进延后队列，合并后仍会保留；线程保持开放，等待后续处理。
There was a problem hiding this comment.
Same disposition as the earlier raise of this finding (rc:3987175814): the re-park-per-prompt cycle with a fresh interaction.id per park is confirmed real, but the maintainer has already ruled that the wording clarification is a documentation follow-up outside this PR, with no runtime change. Recorded in the deferred-findings queue; leaving the thread open for that follow-up.
中文说明
与本发现此前提出时（rc:3987175814）的处置相同：每个提示重新挂起、每次挂起生成全新 interaction.id 的循环已确认为真，但维护者已裁定该措辞澄清作为文档后续跟进、不在本 PR 内处理，且不做运行时改动。已记录进延后队列；线程保持开放，等待后续处理。
There was a problem hiding this comment.
[Critical] R14-2: [certifies-falsely] [new-surface] R14-2 (still stands): the lifecycle sentence this diff adds describes a single optional excursion into waiting_for_input before a terminal status, but an answered interaction returns the operation to running and the same operation can park again — once per promptable setting, each with a fresh interaction.id.
Failure scenario: A client written against this page models one optional prompt per operation, so it allocates a single interaction slot and treats a second waiting_for_input snapshot (with a new interaction.id) as an unexpected state; the operation can park once per promptable setting, and answering flips it back to status: running, phase: preparing, interaction: undefined. The sentence is unchanged since the previous round posted, and so is the code it describes.
Witness:
not run — a prose-vs-code mismatch has no run capability here. Age proof instead: `git --literal-pathspecs diff 44983f74c6199feaa044717992cfc65d2cf11b1a..HEAD -- docs/developers/qwen-serve-protocol.md` is empty (the file is not among the four the fix round touched), and the re-park path it describes (`workspace-extensions.ts:638-657,1067-1075`, `extensionSettings.ts:249-253`) is likewise outside every hunk of that diff, so both sides of the mismatch stand as filed.
Suggested fix: State the repetition, e.g. "status transitions from queued to running, then through zero or more waiting_for_input excursions — one per promptable setting, each carrying a fresh interaction.id — before reaching succeeded, succeeded_with_warnings, or failed."
中文说明
[Critical] 本 diff 新增的生命周期语句把 waiting_for_input 描述为终态前“可选的一次”停留，但一次回答会把 operation 退回 running，同一 operation 可再次进入等待——每个可提示的 setting 一次，且每次都是全新的 interaction.id。按此页面实现的客户端只会预留一个交互槽位。该句与它描述的代码自上一轮以来均未改动。
— qwen3.8-max via Qwen Code /review (v0.23.3)
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
Agent-assisted review at 5fc7d91a7a926264f375c89cb1d7e5997c0203a5 — Partial review — coverage gaps; historical Criticals are not comprehensively cleared.
Base: 2f426a64f407375013ea8de65b1d84579e800ba1. Comment only — no approval implied. No independently confirmed new Critical in this pass; this is not a clean/C=0 disposition.
Reassessment of recent Critical claims
- R14-1: canonical update identity fixed; broader claim not verified as stated. Preflight detected the move from
4e3509398660c37892fc634495e867fe47877c38; I inspected the complete one-commit delta (one production line and one test row) and reanchored.packages/cli/src/serve/routes/workspace-extensions.ts:1698now records the resolvedextension.namefor legacy source-URL updates, matching V2 at 2354–2356. The added source-URL test row is atworkspace-qualified-extensions.test.ts:3180; it was inspected, not executed. The sweep still comparesoperation.nameat 605–611 while install creation at 1369–1374 supplies only source. Preparation concurrency is two (workspace-extensions-controller.ts:60,293–295) and the sweep runs inside preparation. However, “queued indefinitely” is not established by a 1.5-second sample: interactions expire after 10 minutes (workspace-extensions.ts:70–73,648–657) and preparation has its own deadline. More importantly, the claimed install-over-update downgrade needs reconciliation with the real store:packages/core/src/extension/extension-store.ts:681–685rejects an install when its destination already exists; updates also check artifact generation at 747–755, wired fromextensionManager.ts:2772–2782. I did not reproduce a bypass of these guards. Naming/supersession behavior remains unresolved, but I am not treating a mocked ordering witness as proof of silent artifact downgrade. Anchor: #11086 (comment) - R14-2 is documentation clarification, not independently demonstrated code Critical. The loop and fresh IDs are real (
extensionSettings.ts:249–253;workspace-extensions.ts:638–657,1067–1075). The shorthand atdocs/developers/qwen-serve-protocol.md:526should not imply one prompt only. Current WebShell polling handles repeated interaction snapshots (ExtensionsManagerPage.tsx:875–902, with ID changes at 569–579). No actual client failure was established here; keep this nonblocking/deferred. Anchor: #11086 (comment) - The old pre-await liveness warning mechanism is addressed by the post-await sample and drain-specific message (
workspace-extensions-controller.ts:774–789). The generation-zero/error recovery path now includesrecoveringFromErrorin derived-capability invalidation (workspace-runtime-coordinator.ts:801–807,905–923). The receipt now passes content hash or explicit null and recovery ID;observeExtensionGenerationinvalidates certification for null/reused generation/recovery (425–485). These are specific source checks, not clearance of all older recovery/trust/failure-isolation Criticals.
Ownership and coverage. /extensions and operation history/queues are process-global; /workspace/extensions is the explicit legacy-primary adapter. Qualified runtime catalogs use the selected runtime's workspace service; qualified activation/skill writes are persisted-workspace policy mutations with selected-runtime reconciliation, not primary-runtime execution. The qualified resolver requires an active registry generation and returns mismatch/unavailable rather than primary fallback; trust gates protect runtime execution/mutations, while untrusted projection reads pass runtime.trusted into their manager. The coordinator dispatches through runtime.bridge with its cwd, checks epoch/revision after awaits, defers cold/draining work, and per-target reconciliation catches isolate failures. These checked paths are not a complete trust/TOCTOU audit of every route, bootstrap/removal transition or filesystem/env consumer.
Reviewed selected route/controller/coordinator methods, ACP reconciliation, real store commit guards, protocol and interaction UI. Full 42-file production/test coverage, SDK/UI field propagation, legacy runtime compatibility and the entire historical backlog remain gaps. About 2,462 non-test/non-schema production changed lines is an upper bound, not normalized executable logic; this feature merits advisory maintainer review, not a size-only hard gate. Write access does not establish a maintainer exemption. No PR code/tests/build executed; pinned archive integrity was independently verified. No new Suggestions. Posted partial coverage is not comprehensive review completion.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit of 49 round-1/2 auditor findings (A24–A72) — the budget stop left them unverified; they are reported terminal-only, never as blockers.
Not reviewed: issue-fidelity (closing-issue set) — gh 2.45.0 cannot query closingIssuesReferences (needs >= 2.72.0), so the linked-issue set is UNKNOWN, not empty; the motivating-incident replay and root-cause ownership were still performed.
Not reviewed: build-and-test — the CI check "Integration Tests (CLI, No Sandbox)" was skipped at this commit; only the diff-changed integration file was run locally (38/38 pass), the rest of that suite did not run.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": whether the deps-injected queryWorkspaceStatus ( packages/cli/src/serve/workspace-service/index.ts:256 , definition not reached) can start an ACP child for an…; "agent reverse-audit (round 1)": whether ServeStatusCell messages inside the ...status spread of sendRuntimeCatalog can carry an unredacted install source — I confirmed extensions[].sour…; "agent reverse-audit (round 1)": did not confirm from the daemon side whether a single queue already serializes the coordinator's workspaceExtensionsReconcile against the legacy refreshExten…; "agent reverse-audit (round 1)": did not read createBootstrapCapabilities to the end, so the integration test's poll predicate ( caps.workspaces !== undefined as the runtime-envelope signal)…; "agent reverse-audit (round 2)": did not grep the rest of docs/developers/qwen-serve-protocol.md for a definition of the projection's appliedGeneration field that would settle whether epoch…, and 25 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
Not reviewed: "agent verify (round 2)" — pointed at diff lines it never opened: it made tool calls, but none of them read the diff.
— [unverified] tag when the loop ended — the verifier never ruled on them, and they are not confirmed.
Deferred under the convergence posture (round 15, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/serve/workspace-runtime-coordinator.ts:547 — [review] Critical [fails-closed] [new-surface] D15-1: reconcileExtensionGeneration evaluates its "already appliedocs/design/workspace-runtime-architecture.md:9 — [review] This PR updates the single-language (Chinese-only) design ddocs/developers/qwen-serve-protocol.md:602 — [review] The trust-reporting half of this newly documented contract ipackages/acp-bridge/src/bridge.ts:6460 — [review] The only producer of runtimeEpoch on the live Extension capackages/cli/src/acp-integration/acpAgent.test.ts:31523 — [review] The ~126-line workspaceExtensionsReconcile scenario is apppackages/cli/src/acp-integration/acpAgent.ts:14066 — [review] configErrors records the message but not which config prodpackages/cli/src/acp-integration/acpAgent.ts:14075 — [review] A failure in *any* leg of a config's reconcile body now supppackages/cli/src/serve/routes/workspace-extensions-controller.test.ts:539 — [review] The three new tests each rebuild the same ~40-line scaffoldpackages/cli/src/serve/routes/workspace-extensions-controller.ts:650 — [review] Of the three deadlineController.signal.throwIfAborted() gupackages/cli/src/serve/routes/workspace-extensions-controller.ts:729 — [review] Nothing in the CLI package asserts the store-identity receippackages/cli/src/serve/routes/workspace-extensions-controller.ts:791 — [review] The new coordinator branch broadcasts unconditionally, dropppackages/cli/src/serve/routes/workspace-extensions.ts:799 — [review] The store-identity hash is loop-invariant but recomputed inspackages/cli/src/serve/routes/workspace-extensions.ts:1959 — [review] GET /extensions now enters buildLocalExtensionsStatus inpackages/cli/src/serve/routes/workspace-extensions.ts:2515 — [review] The new legacy-primary runtime-catalog route has no test anypackages/cli/src/serve/routes/workspace-qualified-extensions.test.ts:1633 — [review] The ~45-line coordinator-arming block ( Object.assign(h.secopackages/cli/src/serve/workspace-runtime-coordinator.ts:228 — [review] The new extensionsQueuedWork term in hasActiveWork() ispackages/cli/src/serve/workspace-runtime-coordinator.ts:1350 — [review] The Extensions retry budget ( extensionsRefreshRetryRevisionpackages/web-shell/client/components/dialogs/ScheduledTasksDialog.tsx:996 — [review] The dialog re-implements the composer's new runtime-catalogpackages/web-shell/client/components/dialogs/ScheduledTasksDialog.tsx:1545 — [review] Candidates are now workspace-scoped, but the open picker ispackages/web-shell/client/components/extensions/ExtensionsManagerPage.test.tsx:335 — [review] The test named for the catalog leg rejecting 403 never rejec- …and 7 more (see the run report)
Convergence: round 15 posted 6 inline comment(s), 4 of them reported for the first time; the previous round posted 2 (2 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (6 Critical(s)), the rate of first-time findings is not falling (this round 4, previous 2), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：reverse audit of 49 round-1/2 auditor findings (A24–A72) — the budget stop left them unverified; they are reported terminal-only, never as blockers.
未审查（原文为英文）：issue-fidelity (closing-issue set) — gh 2.45.0 cannot query closingIssuesReferences (needs >= 2.72.0), so the linked-issue set is UNKNOWN, not empty; the motivating-incident replay and root-cause ownership were still performed.
未审查（原文为英文）：build-and-test — the CI check "Integration Tests (CLI, No Sandbox)" was skipped at this commit; only the diff-changed integration file was run locally (38/38 pass), the rest of that suite did not run.
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：whether the deps-injected queryWorkspaceStatus ( packages/cli/src/serve/workspace-service/index.ts:256 , definition not reached) can start an ACP child for an…；"agent reverse-audit (round 1)"：whether ServeStatusCell messages inside the ...status spread of sendRuntimeCatalog can carry an unredacted install source — I confirmed extensions[].sour…；"agent reverse-audit (round 1)"：did not confirm from the daemon side whether a single queue already serializes the coordinator's workspaceExtensionsReconcile against the legacy refreshExten…；"agent reverse-audit (round 1)"：did not read createBootstrapCapabilities to the end, so the integration test's poll predicate ( caps.workspaces !== undefined as the runtime-envelope signal)…；"agent reverse-audit (round 2)"：did not grep the rest of docs/developers/qwen-serve-protocol.md for a definition of the projection's appliedGeneration field that would settle whether epoch…，另有 25 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
未审查："agent verify (round 2)"——启动 prompt 为它指定了 diff 中的行，但它从未打开：有工具调用，却没有一次读取 diff。
— [unverified] 标记——验证者从未对它们作出裁决，它们不算已确认。
收敛姿态下延后（第 15 轮，非阻断）——已记录，本轮不要求修改；其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface：未认证任何错误结果，且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中：共 27 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 15 轮发布了 6 条行内评论，其中 4 条是首次提出；上一轮发布了 2 条（其中 2 条首次提出）。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 6 条 Critical），首次发现的速率没有下降（本轮 4，上一轮 2），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.23.3)
…11086) - web-shell: reset the load-notice latch when a mutation takes over the notice so a later successful load cannot erase the mutation's result - web-shell: keep recovered operation notices global so they render on the list view the user lands on - web-shell: tolerate runtime-catalog read failures in the split loader so a trust-gated 403 no longer discards the fetched catalog/projection - serve: drop the unreachable install disjunct from the update-side supersede sweep and correct its comment (installs carry no name) - docs: align capabilities-versioning with the live-only generation reconciler rule Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🤖 Addressed the latest review feedback (round 6/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 6/100 轮）。改动内容与我反驳保留之处如下： Round summary — PR #11086 review feedbackCommit: Growth audit (required this window)Window growth: source 148 / test 774 net lines vs budgets 400/400. Verdict: sound (recorded in Feedback dispositionsrc:3990785946 — R15-1 [Critical]: successful load erases a mutation's just-set notice — RESOLVEDReproduced before fixing: with the load-notice latch set by a capability error and a mutation failing while polled, the next signal-driven rc:3990785962 — R15-3 [Critical]: recovery-effect owner attribution hides the notice on the list view — RESOLVEDVerified against the code: rc:3990785968 — R15-4 [Critical]: un-tolerated runtime-catalog leg discards catalog + projection, caller reports success — RESOLVEDVerified: the projection leg already tolerates failure ( rc:3990786210 + rc:3987175805 — R14-1 [Critical]: update-side supersede sweep can never match a parked install — RESOLVED (option (a))Verified: installs never carry rc:3987175814 + rc:3990786479 — R14-2 [Critical]: lifecycle sentence implies a single
|
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: issue-fidelity — the closing-issue reference set could not be fetched (installed gh 2.45.0 predates closingIssuesReferences); the set is UNKNOWN, not empty. Agent 0 fetched the PR-named #10593 and replayed the narrated incident instead..
Not reviewed: build-and-test — "Integration Tests (CLI, No Sandbox)" was skipped in CI and its suite did not run locally (Agent 7 measured integration-tests/cli/qwen-serve-routes.test.ts as unreachable from every npm workspace).
Not explored to full depth (tool budget reached): "agent reverse-audit (round 2)": whether the runtime-catalog response's pass-through status.errors[].error can carry an unredacted credentialed source URL — sendRuntimeCatalog spreads ...s…; "agent reverse-audit (round 2)": whether workspaceRegistry can be undefined in a daemon that still advertises workspace_extensions_config_runtime unconditionally ( capabilities.ts :91), whi…; "agent reverse-audit (round 2)": whether the legacy-primary GET /workspace/runtime/extensions needs the same selector/activeness semantics the :380 row documents for the qualified route — it …; "agent reverse-audit (round 1)": I did not execute packages/web-shell/client/components/plugins/PluginManagerPage.test.tsx or useAtMentionMenu.test.tsx to confirm the new/changed cases pass…; "agent reverse-audit (round 1)": whether workspace-qualified-extensions.test.ts (the only file posting /extensions/install , :2632-3294) already drives a V2-created operation through the leg…, and 22 more.
Not reviewed: reverse audit — stopped before round 3 by the review time budget.
1 Suggestion(s) were drafted inline past the resolved critical posting floor; the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 16, not a blocker) — recorded, not requested in this round:
integration-tests/cli/qwen-serve-routes.test.ts:391 — [review] R16-6: This changed integration test is outside every npm workspace, so neither this review's build-test scoped run nor the efficacy probe collected it (efficacy kind: "unrea…docs/design/daemon-workspace-runtime-extensions.md:70 — [review] The added sentence states the derived-capability…docs/design/daemon-workspace-runtime-extensions.md:116 — [review] The new design doc states the composer's Extension loader…docs/design/workspace-runtime-architecture.md:9 — [review] This PR updates the Foundation/Target status of a…docs/design/workspace-runtime-architecture.md:32 — [review] This diff flips Extensions to "已落地/已完成" in five places…docs/developers/daemon/11-capabilities-versioning.md:128 — [review] The added convergence guarantee is wider than the code: a…docs/developers/qwen-serve-protocol.md:363 — [review] The rewritten extension_activation_explicit_refresh …docs/developers/qwen-serve-protocol.md:380 — [review] Two of the four documented outcomes for the new GET…docs/developers/qwen-serve-protocol.md:602 — [review] The new workspace_extensions_config_runtime row names…packages/acp-bridge/src/bridge.ts:6460 — [review] The only production code that stamps runtimeEpoch onto…packages/cli/src/acp-integration/acpAgent.ts:13998 — [review] The new skillsOnly type guard is untested —…packages/cli/src/serve/routes/workspace-extensions-controller.test.ts:463 — [review] The it.each title template omits $state , so two rows…packages/cli/src/serve/routes/workspace-extensions-controller.ts:650 — [review] The diff adds two deadlineController.signal.throwIfAborted…packages/cli/src/serve/routes/workspace-extensions-controller.ts:791 — [review] The coordinator branch broadcasts extensions_changed …packages/cli/src/serve/routes/workspace-extensions-controller.ts:1206 — [review] The new currentManager path bypasses both the cache read…packages/cli/src/serve/routes/workspace-extensions.ts:607 — [review] The new same-extension update supersede fires only when…packages/cli/src/serve/routes/workspace-extensions.ts:642 — [review] The new deadline-abort cancel deletes the pending…packages/cli/src/serve/routes/workspace-extensions.ts:821 — [review] This diff narrows the 30-second generation reconciler to…packages/cli/src/serve/routes/workspace-extensions.ts:822 — [review] The state === 'stopping' half of the new skip is…packages/cli/src/serve/routes/workspace-extensions.ts:844 — [review] The new coordinator branch is nested inside the poller's…- …and 45 more (see the run report)
Convergence: round 16 posted 5 inline comment(s), 4 of them reported for the first time; the previous round posted 6 (4 new). Findings keep coming back to the same files: packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx (findings in round 15; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had no anchor this round could use either — none at all, one with no certifier, one certified by an identity other than the one this round runs under, or one this round's fetch refused or resolved to the head — so the next review re-reads the whole diff unless recovery grafts an earlier own anchor that the round running it can use onto the complete work list this round leaves behind, and keeps doing so until a round's marker carries an anchor again or a graft lands that the round running it can use. (Stated, not acted on — this changes nothing about what the round posts.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (6 Critical(s)), the rate of first-time findings is not falling (this round 4, previous 4), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
[Critical] R16-2 This diff makes the process-global POST /extensions/install and POST /extensions/:extensionId/update interactive, but the route that starts the operation validates X-Qwen-Client-Id against every registered workspace runtime's bridge while the only route that can answer the parked interaction validates it against the primary bridge alone — so a client whose id is known only to a secondary runtime can start an install it is then refused the ability to answer. (absorbs the convergence-pair duplicate r064) Failure scenario: sendOperation gates the mutation with primaryController.validateExtensionMutationClient(req, res, { requireClientId: false, bridges: mutationClientBridges(options.refreshRuntimes) }) (:1919-1924), and both changed routes pass refreshRuntimes: () => workspaceRegistry.listAll() (:2296-2299, :2421-2424), so mutationClientBridges returns every runtime's bridge (AcpSessionBridge.knownClientIds() is the union of that bridge's own live sessions — packages/acp-bridge/src/bridge.ts:12498-12508). Take a client id c-S registered only on secondary workspace S's bridge (an SDK/Web Shell client whose sessions all live in S — ExtensionsManagerPage.tsx:1240 passes that same clientId to installUserExtension/installExtension). POST /extensions/install with X-Qwen-Client-Id: c-S is accepted → 202 → the newly attached extensionInteractionHandlers park the operation in waiting_for_input (e.g. a marketplace plugin choice). The only answer endpoint is POST /workspace/extensions/operations/:operationId/interactions/:interactionId — registerFor is called exactly once, with /workspace/extensions (:1823), and packages/sdk-typescript/src/daemon/DaemonClient.ts:1753 Witness: probe appended to packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts in the isolated tree, reusing the file's own harness (makeHarness, mockExtensionManager, requestApiKey) and the same knownClientIds arming as the diff's new test validates mutation clients against the targeted runtime set (primary → {primary-client}, secondary → {secondary-client}), on unmodified Suggested fix: Validate the interaction answer against the same bridge set the operation was accepted under — e.g. in the registerFor interactions handler pass bridges: mutationClientBridges() (which already falls back to [bridge] when there is no registry, workspace-extensions.ts:771-780) instead of relying on the controller's default single bridge. The fix must not violate an existing fact: The answer endpoint is registered once — registerFor('/workspace/extensions', ...) (packages/cli/src/serve/routes/workspace-extensions.ts:1823) — and docs/developers/qwen-serve-protocol.md:508 states "there is no equivalent global-base alias", so the fix must widen validation on the existing route, not add an /extensions-base alias. Acceptance criterion: packages/cli/src/serve/server.test.ts (the file that already drives /workspace/extensions/operations/:operationId/interactions/:interactionId, e.g. :7841, :8161, :8609): a test that registers a client id on a secondary runtime's bridge only, starts POST /extensions/install with that header, waits for waiting_for_input, then POSTs the answer with the same header and asserts 200 { accepted: true }. Without the widened bridges argument it must fail with 400 invalid_client_id. (Could not be anchored to a diff line — packages/cli/src/serve/routes/workspace-extensions.ts:2279-2288: the quoted snippet matched more than one place once indentation was normalised.) Axes: [fails-closed] [new-surface]
中文说明
仅完成部分审查，审查缺口已披露。
未审查（原文为英文）：issue-fidelity — the closing-issue reference set could not be fetched (installed gh 2.45.0 predates closingIssuesReferences); the set is UNKNOWN, not empty. Agent 0 fetched the PR-named #10593 and replayed the narrated incident instead..
未审查（原文为英文）：build-and-test — "Integration Tests (CLI, No Sandbox)" was skipped in CI and its suite did not run locally (Agent 7 measured integration-tests/cli/qwen-serve-routes.test.ts as unreachable from every npm workspace).
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 2)"：whether the runtime-catalog response's pass-through status.errors[].error can carry an unredacted credentialed source URL — sendRuntimeCatalog spreads ...s…；"agent reverse-audit (round 2)"：whether workspaceRegistry can be undefined in a daemon that still advertises workspace_extensions_config_runtime unconditionally ( capabilities.ts :91), whi…；"agent reverse-audit (round 2)"：whether the legacy-primary GET /workspace/runtime/extensions needs the same selector/activeness semantics the :380 row documents for the qualified route — it …；"agent reverse-audit (round 1)"：I did not execute packages/web-shell/client/components/plugins/PluginManagerPage.test.tsx or useAtMentionMenu.test.tsx to confirm the new/changed cases pass…；"agent reverse-audit (round 1)"：whether workspace-qualified-extensions.test.ts (the only file posting /extensions/install , :2632-3294) already drives a V2-created operation through the leg…，另有 22 条。
未审查：反向审计——评审时间预算不足，未能开始第 3 轮。
1 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论；CLI 已将其移入下方延后清单（下限强制执行）。
收敛姿态下延后（第 16 轮，非阻断）——已记录，本轮不要求修改：共 65 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 16 轮发布了 5 条行内评论，其中 4 条是首次提出；上一轮发布了 6 条（其中 4 条首次提出）。发现反复回到同一批文件：packages/web-shell/client/components/extensions/ExtensionsManagerPage.tsx（第 15 轮已出过发现，本轮又有 1 条）。新发现的产出速度没有下降。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送，可以避免循环反复推导同一组发现；本 PR 的评审已解析为 critical 发布下限。（仅为观察——本轮评审未因此扣留任何内容。）
机制健康：本轮未能干净收尾，因而扣留了增量锚点，而它恢复到的那一轮也没有留下本轮可用的锚点——要么完全没有、要么没有认证者、要么由本轮运行身份之外的身份认证、要么被本轮的获取拒绝或解析为头提交——因此下一次评审将重读整个 diff，除非恢复流程把本轮能使用的更早自有锚点嫁接到本轮留下的完整工作清单上；并会一直如此，直到某一轮的标记重新带上锚点，或落地的嫁接能被运行该轮的评审使用。（仅陈述，不据此行动——这不改变本轮发布的任何内容。）
残余风险：本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在（本轮 6 条 Critical），首次发现的速率没有下降（本轮 4，上一轮 4），且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议：land-with-residual-risk——出口是 maintainer 的风险接受决定（合入并承担残余风险），而非再开一轮评审。供该决定使用的残余风险清单（maintainer 填写）：按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
[Critical] R16-2 This diff makes the process-global POST /extensions/install and POST /extensions/:extensionId/update interactive, but the route that starts the operation validates X-Qwen-Client-Id against every registered workspace runtime's bridge while the only route that can answer the parked interaction validates it against the primary bridge alone — so a client whose id is known only to a secondary runtime can start an install it is then refused the ability to answer. (absorbs the convergence-pair duplicate r064) Failure scenario: sendOperation gates the mutation with primaryController.validateExtensionMutationClient(req, res, { requireClientId: false, bridges: mutationClientBridges(options.refreshRuntimes) }) (:1919-1924), and both changed routes pass refreshRuntimes: () => workspaceRegistry.listAll() (:2296-2299, :2421-2424), so mutationClientBridges returns every runtime's bridge (AcpSessionBridge.knownClientIds() is the union of that bridge's own live sessions — packages/acp-bridge/src/bridge.ts:12498-12508). Take a client id c-S registered only on secondary workspace S's bridge (an SDK/Web Shell client whose sessions all live in S — ExtensionsManagerPage.tsx:1240 passes that same clientId to installUserExtension/installExtension). POST /extensions/install with X-Qwen-Client-Id: c-S is accepted → 202 → the newly attached extensionInteractionHandlers park the operation in waiting_for_input (e.g. a marketplace plugin choice). The only answer endpoint is POST /workspace/extensions/operations/:operationId/interactions/:interactionId — registerFor is called exactly once, with /workspace/extensions (:1823), and packages/sdk-typescript/src/daemon/DaemonClient.ts:1753 Witness: probe appended to packages/cli/src/serve/routes/workspace-qualified-extensions.test.ts in the isolated tree, reusing the file's own harness (makeHarness, mockExtensionManager, requestApiKey) and the same knownClientIds arming as the diff's new test validates mutation clients against the targeted runtime set (primary → {primary-client}, secondary → {secondary-client}), on unmodified Suggested fix: Validate the interaction answer against the same bridge set the operation was accepted under — e.g. in the registerFor interactions handler pass bridges: mutationClientBridges() (which already falls back to [bridge] when there is no registry, workspace-extensions.ts:771-780) instead of relying on the controller's default single bridge. The fix must not violate an existing fact: The answer endpoint is registered once — registerFor('/workspace/extensions', ...) (packages/cli/src/serve/routes/workspace-extensions.ts:1823) — and docs/developers/qwen-serve-protocol.md:508 states "there is no equivalent global-base alias", so the fix must widen validation on the existing route, not add an /extensions-base alias. Acceptance criterion: packages/cli/src/serve/server.test.ts (the file that already drives /workspace/extensions/operations/:operationId/interactions/:interactionId, e.g. :7841, :8161, :8609): a test that registers a client id on a secondary runtime's bridge only, starts POST /extensions/install with that header, waits for waiting_for_input, then POSTs the answer with the same header and asserts 200 { accepted: true }. Without the widened bridges argument it must fail with 400 invalid_client_id. (Could not be anchored to a diff line — packages/cli/src/serve/routes/workspace-extensions.ts:2279-2288: the quoted snippet matched more than one place once indentation was normalised.) Axes: [fails-closed] [new-surface]
— qwen3.8-max via Qwen Code /review (v0.23.3)
| storeContentHash: | ||
| committedStoreContentHash ?? null, |
There was a problem hiding this comment.
[Critical] R16-1: [certifies-falsely] [new-surface] When the store has already moved past the committed generation, the receipt coerces "identity not taken" into null, which observeExtensionGeneration defines as "identity could not be read" and answers by dropping the runtime's applied-generation certification — the opposite of what this hunk's own comment says it intends.
Failure scenario: Two installs overlap on one daemon (MAX_UNFINISHED_EXTENSION_OPERATIONS = 10, EXTENSION_PREPARATION_CONCURRENCY = 2, commit queue serialized at 1 but released early by onCommitted at :663–666). Operation A commits generation 5; operation B then acquires the commit queue and commits 6 before A's post-commit tail (disposePreparedExtension, updateExtensionOperation) reaches the receipt read at :727. A's snapshot shows generation: 6 !== 5, so the if at :730 is false and both identity variables stay undefined → storeContentHash: null is passed to reconcileExtensionGeneration(5, …) for every runtime in refreshTargets (workspaceRegistry.listAll()). In observeExtensionGeneration, null fails the dedup (null === undefined/null === observedHash are both false, coordinator :449–452) and hits storeContentHash === null in the reset at :456–463: appliedExtensionGeneration = 0, appliedExtensionRuntimeEpoch = undefined, observedExtensionStoreHash = undefined, revision bumped, capability status rewritten to {state:'stale', appliedGeneration: 0} — a status that is factually wrong, since the runtime has generation 5 applied. The short-circuit at :510–517 c
Witness:
two probe arms on the **real** `WorkspaceRuntimeCoordinator` (dist), both starting from an identical certified state `{state:'ready', revision:1, runtimeEpoch:1, desiredGeneration:5, appliedGeneration:5}` and then calling `reconcileExtensionGeneration(5, {storeContentHash: X})`:
Suggested fix: Distinguish "read failed" from "not this receipt's generation". Track the failure explicitly and only send null for it, e.g. add let storeIdentityUnreadable = false;, set it in the catch at :734–736, and pass storeContentHash: storeIdentityUnreadable ? null : committedStoreContentHash so the moved-past branch sends undefined. With undefined, observeExtensionGeneration dedups when desired === generation (coordinator :449–452) and otherwise advances desired without the :456–463 reset, leaving the single legitimate prepare to B's receipt / the 30 s poller.
The fix must not violate an existing fact: null must keep meaning "identity unknown → drop the certification": workspace-runtime-coordinator.ts:455–457 — "Recovery can reuse a generation for different artifacts. null denotes a committed mutation whose identity could not be read; neither it nor an unknown prior baseline may reuse the previous certification." — and the reset it guards at :458–463. Narrowing the call site must not narrow that contract for the genuine read-failure path.
Acceptance criterion: No test reaches this branch today — workspace-extensions-controller.test.ts mocks getExtensionStoreSnapshot only as vi.fn(async () => ({ generation: 2 })) (:498, :566, :636) and never asserts the storeContentHash argument. Add a case in that file: a handler that commits generation 5 (context.commit(async (onCommitted) => { onCommitted(5); return { generation: 5 }; })) with getExtensionStoreSnapshot resolving { generation: 6 }, a spied coordinator.reconcileExtensionGeneration, and expect(spy).toHaveBeenCalledWith(5, expect.objectContaining({ storeContentHash: undefined })). It goes red against the current ?? null, and a companion row with getExtensionStoreSnapshot reject Please prove it by mutation — remove the fix, run that test, confirm it goes red.
中文说明
[Critical] 当 store 已越过本次提交的 generation 时，新的 store 身份回执把“未取到身份”强转为 null，而 observeExtensionGeneration 把 null 定义为“身份读不出来”，其响应是丢弃该 runtime 的 applied-generation 认证——与本 hunk 注释声明的意图相反。
— qwen3.8-max via Qwen Code /review (v0.23.3)
| await expect( | ||
| coordinator.reconcileExtensionGeneration(10, { skillsOnly: true }), | ||
| ).resolves.toMatchObject({ state: 'deferred' }); |
There was a problem hiding this comment.
[Critical] R16-3: [certifies-falsely] [new-surface] A skills-only reconcile at the same generation whose full reconcile just failed certifies that generation — appliedExtensionGeneration === generation - 1 cannot distinguish "N-1 applied, N is a fresh skill delta" from "N's own full apply failed, leaving applied at N-1" — and the test named does not let a skills-only reconcile certify an unapplied generation only exercises the case one generation above the failure, so the reachable hole ships unpinned.
Failure scenario: Runtime at epoch 3, generation 8 fully applied (appliedExtensionGeneration: 8). A mutation commits generation 9 (install, or a skill-state toggle). Two independent production issuers target generation 9: the 30 s poller / ensure() full reconcile (workspace-extensions.ts:846-848 selects the runtime whenever appliedGeneration !== generation || state !== 'ready'; prepareExtensions() reconciles desiredExtensionGeneration with no skillsOnly), and the mutation's own skills-only receipt (workspace-extensions-controller.ts:762-767), also re-issued by cancelDrain() at this.desiredExtensionGeneration with the stored {skillsOnly:true} (workspace-runtime-coordinator.ts:203-213). They serialize FIFO on extensionsTail, so take the order full-then-skills-only. The full reconcile at 9 fails (configsFailed: 1 — e.g. the git-auth failure this file's own sanitizer test uses at :852): recordExtensionsError (:1314) leaves appliedGeneration 8, sets error, and arms one retry (:1355). The skills-only reconcile at 9 then runs: observeExtensionGeneration(9, undefined, undefined, undefined) early-returns (:447-453, same generation, no hash), so the revision stays put and t
Witness:
two probes, each run on the unmodified PR and again with the finding's own fix, plus the 86-test suite under the fix.
Suggested fix: Refuse skills-only certification while this revision's own apply is in error — recoveringFromError is already computed for exactly that state three lines earlier (:805-807), before extensionsStatus is overwritten with 'starting':
The fix must not violate an existing fact: The narrow window must still certify a fresh skill-state generation — preserves the narrow refresh for Extension Skill-state changes (workspace-runtime-coordinator.test.ts:591-623) asserts reconcileExtensionGeneration(7, { skillsOnly: true }) resolves { state: 'reconciled' } after generation 6 applied, and it.each([false, true])('replays interrupted Extension reconciliation with skillsOnly=%s') (:917-969) asserts the drain replay reaches { state: 'ready', desiredGeneration: 7, appliedGeneration: 7 }; in both, the pre-apply status is ready/stale, not error, so !recoveringFromError keeps them green. The fix must also not add a second cooldown: `EXTENSIONS_ERROR_RETRY_COOL
Acceptance criterion: A new case beside workspace-runtime-coordinator.test.ts:625 — full reconcile at generation N fails (configsFailed: 1), then reconcileExtensionGeneration(N, {skillsOnly: true}) must leave capabilities.extensions.state !== 'ready' and appliedGeneration === N - 1, and the following ensure() must re-issue the full reconcile (assert the qwen/control/workspace/extensions/reconcile call count rises, i.e. the retry marker at :891-893 survived). I ran that probe: it is red today, and it goes red again if !recoveringFromError is removed from the fix. Please prove it by mutation — remove the fix, run that test, confirm it goes red.
中文说明
[Critical] 在同一 generation 上，若完整 reconcile 刚失败，随后的 skills-only reconcile 仍会认证该 generation：appliedExtensionGeneration === generation - 1 无法区分“N-1 已应用、N 是新 skill 增量”与“N 自身完整应用失败、applied 仍停在 N-1”。
— qwen3.8-max via Qwen Code /review (v0.23.3)
| // The Extensions work outlived the observation budget but stays | ||
| // queued; the Skills/MCP preparation below then certifies from a | ||
| // runtime read that predates the applied catalog, so the | ||
| // late-settling apply must invalidate and re-drive them. | ||
| this.extensionsEnsureAbandonedAtEpoch = status.runtimeEpoch; |
There was a problem hiding this comment.
[Critical] R16-4: [certifies-falsely] [regression] When the Extensions prepare is abandoned at the ensure deadline, ensure() returns a status whose capabilities.skills.state / capabilities.mcp.state are still their pre-ensure values (not_started on a cold runtime), not starting — so loadReadyWorkspaceSkills, which polls only on 'starting', returns undefined immediately and the composer never replaces its config-derived skill list with the matching-epoch runtime catalog.
Failure scenario: Trace of the two moments. Produced: ensure() (line 337-402) awaits withTimeout(this.prepareExtensions(), remainingMs); the new Extensions reconcile has a 5-minute physical budget (EXTENSIONS_RECONCILE_TIMEOUT_MS = 5 * 60_000, line 30) against a 60-second observation budget (DEFAULT_ENSURE_TIMEOUT_MS = 60_000, line 27), so on a workspace whose refreshTools() re-initialises MCP servers, LSP, subagents, hooks and memory it routinely outlives it. On timeout, Date.now() >= deadline, so the second const remainingMs = deadline - Date.now(); if (remainingMs > 0) at line 394-395 is false and the await withTimeout(Promise.all([skillsPrep, mcpPrep]), …) is skipped. prepareSkills() was called at line 385, but queueSkillsWork (line 1050-1066) only chains .catch().then() onto this.skillsTail, so its body — and the this.skillsStatus = { state: 'starting', … } assignment inside prepareSkillsRevision (line 1115) — runs on a later microtask. const finalStatus = this.status(); at line 404 executes in the same synchronous run, so the returned envelope carries skills: { state: 'not_started', revision: 0 } (and mcp likewise). Needed: App.tsx:7830-7838 feed
Witness:
` [probe] isolated copy at the reviewed commit, replaying the PR's own 61 s scenario (extensions reconcile settles at 61 s against the 60 s ensure budget): ``` PR: AB1 envelope = {"extensions":{"state":"starting",…},"mcp":{"state":"not_started","revision":0},"skills":{"state":"not_started","revision":0}} extReconcileCalls = 1 BASE: AB1 envelope = {"mcp":{"state":"ready",…,"runtimeEpoch":3},"skills":{"state":"ready",…,"runtimeEpoch":3}} extReconcileCalls = 0 (arm proved: grep -c prepareExtensions → PR 5 / BASE 0; 1424 / 742 lines; same probe file, only the coordinator swapped) B003 status() one
Suggested fix: Make the derived capabilities report 'starting' at the moment their preparation is queued rather than when the queued body first runs — assign this.skillsStatus = { state: 'starting', revision, runtimeEpoch: snapshot.runtimeEpoch } in prepareSkills() before queueSkillsWork(...) (and the mirror in prepareMcp()), the way scheduleSkillsReconciliation() already does at lines 659-663. Then finalStatus is accurate whether or not the remainingMs > 0 await ran. (Alternatively, widen the loadReadyWorkspaceSkills loop guard to 'not_started', but that leaves the wire status lying to every other client.)
The fix must not violate an existing fact: packages/cli/src/serve/workspace-runtime-coordinator.ts:27,30 — const DEFAULT_ENSURE_TIMEOUT_MS = 60_000; and const EXTENSIONS_RECONCILE_TIMEOUT_MS = 5 * 60_000;. The physical refresh budget deliberately exceeds the observation budget (design doc: "The shorter ensure observation deadline does not cancel that refresh; a later successful result can still certify readiness"), so the fix must not shrink the reconcile timeout to fit inside ensure().
Acceptance criterion: packages/cli/src/serve/workspace-runtime-coordinator.test.ts — the existing test 're-certifies Skills/MCP when the initial Extension apply settles past the ensure budget' asserts only await expect(ensured).resolves.toMatchObject({ runtimeLive: true }) and then reads coordinator.status() after the fact; extend it to assert capabilities.skills.state === 'starting' and capabilities.mcp.state === 'starting' on the resolved ensured value. That assertion is red today. A web-shell-side witness belongs in packages/web-shell/client/daemon/workspace/load-ready-skills tests: given an initial runtime with skills.state === 'not_started' that later reports 'starting' then `'ready Please prove it by mutation — remove the fix, run that test, confirm it goes red.
中文说明
[Critical] 当 Extensions 准备在 ensure 观察超时后被放弃时，ensure() 返回状态里的 capabilities.skills.state / capabilities.mcp.state 仍是 ensure 之前的值（冷启动为 not_started）而非 starting；只按 starting 轮询的 loadReadyWorkspaceSkills 立刻返回 undefined，composer 因此不会用匹配 epoch 的 runtime 目录替换按配置推导的 skill 列表。
— qwen3.8-max via Qwen Code /review (v0.23.3)
| if (requestId !== loadRequestRef.current) return observedTrusted; | ||
| // Resolve the trust as soon as the projection answers: the runtime | ||
| // leg below is trust-gated, so its 403 must not discard a trust |
There was a problem hiding this comment.
[Critical] R16-5: [certifies-falsely] [new-surface] The new supersede guard returns observedTrusted before it is assigned from the projection the load already holds, so a superseded activation reload reports "trust not observed" and the refresh decision at :1414 (const observedTrust = (await load(true)) ?? workspaceTrusted;) silently falls back to the render-time snapshot — the exact thing the comment this diff adds at :625-627 ("resolved per branch so awaiting callers decide on the fresh value, not the render-time state snapshot") forbids. This is a regression: at the merge base the loader had one exit, `return activation ? activatio
Failure scenario: observedTrusted is declared = null at :628 and first assigned at :640 — one line after the guard at :636 — so on that path the return value is provably null even though activation is in hand. Concurrent loads are not gated by busyName: the signals?.extensionsVersion effect calls void load(true) unconditionally (:871-879), as do the Refresh button (:1144), the install/update/checkUpdates reloads (:1025, :1099, :1341) and the 2 s retry timers (:716, :773). Trigger: the page is mounted on a workspace whose projection reported trusted: true; trust is revoked out of band; the user toggles an activation, and the daemon's own generation broadcast for that mutation lands signals.extensionsVersion while the activation's load(true) is inside its Promise.all([extensionCatalog(), workspaceExtensions()]) round-trip. The signal-driven load bumps loadRequestRef.current, the activation's load early-returns null, and observedTrust resolves to the closure's stale workspaceTrusted === true. activationRequiresExplicitRefresh && observedTrust is then true, so refreshExtensionRuntime() is fired at an untrusted workspace, the daemon 403s it, and `se
Witness:
probe arm 1 (trust revoked out of band; the activation's reload gated inside its catalog round trip via `extensionCatalog.mockImplementationOnce`; a daemon-pushed `signals.extensionsVersion` load supersedes it; gate released) — PR code: `AssertionError: expected "spy" to not be called at all, but actually been called 1 times` → the refresh fires at a workspace the same load had just read as `trust
Suggested fix: Resolve the trust from the projection before the supersede check, keeping only the state write behind the guard:
The fix must not violate an existing fact: The setWorkspaceTrusted(observedTrusted) write must stay inside the requestId guard — ExtensionsManagerPage.tsx:641-643 (if (observedTrusted !== null) { setWorkspaceTrusted(observedTrusted); }), matching apply's own guard at :593 (if (requestId !== loadRequestRef.current) return;); only the pure local assignment may move above it.
Acceptance criterion: A case in packages/web-shell/client/components/extensions/ExtensionsManagerPage.test.tsx beside decides the refresh on the trust the activation reload observes, even when the runtime leg fails (:2105): gate the reload's projection leg with a deferred workspaceExtensions, flip trusted = false, bump state.signals = { extensionsVersion: 1 } inside act (fires the :876 reload that supersedes the activation's load), release the gate, then assert expect(refreshExtensionRuntime).not.toHaveBeenCalled() and not.toContain('session refresh failed'). Revert the reordering (guard before assignment) and both assertions go red, because load(true) returns null and ?? workspaceTrusted Please prove it by mutation — remove the fix, run that test, confirm it goes red.
中文说明
[Critical] 新增的 supersede 保护在 observedTrusted 尚未从本次 load 已持有的 projection 赋值之前就返回它，被取代的 activation reload 因此报告“未观察到信任”，影响 :1414 处的刷新判定。
— qwen3.8-max via Qwen Code /review (v0.23.3)
Maintainer verification — real two-workspace daemon + real Web Shell (Linux)Verified on a real Verdict: works as described — recommend merge. One non-blocking observation and one pre-existing gap (reproduced identically on base, so not caused by this PR) are recorded at the end. Head SetupTwo workspaces Two probe extensions installed globally at user scope, each shipping one skill and one slash command:
The whole A/B rests on that single asymmetry: with the composer/manager pointed at 1. The defect this PR fixes, reproduced on baseWith the composer workspace set to
Observed network reads (captured from the browser, same store state on both arms):
Selecting 2. Extension management follows the selected workspaceBase has no workspace selector on Plugins ▸ Extensions and reads the primary only, so
The detail pane separates the two scopes correctly, and the live capability counts (
3. Enablement is reconciled into the live secondary runtimeThis is the core claim, so it was driven through the real UI and read back from the daemon. Toggling Workspace setting → Enabled for
Installing a new extension while both runtimes are live behaves the same way (measured on A live session in 4. Documented contracts spot-checked
5. TestsAll PR-touched suites pass locally at
Non-vacuity — four mutations, all killed. Each reverts one specific guard this PR adds:
The first mutation is the important one: it proves the E2E oracle above is sensitive to the fix itself and not to incidental setup. Three of the four mutated files are byte-identical between Findings (neither blocks merge)1 · Pre-existing, not caused by this PR — re-enabling an extension does not restore its slash command in an already-live session. Same session, same runtime, At that last point the daemon is correct — 2 · Observation — the new composer loader is not wrapped in the client action timeout.
This is bounded and narrow, and I did not reproduce it — on a genuinely cold 3 · Note, no action needed — the untrusted-secondary client branches are defence-in-depth.
Screenshots, plus the full report in both languages, are on 中文版报告（完整版含截图：report.zh-CN.md）维护者验证 — 真实双工作区 daemon + 真实 Web Shell（Linux）在一台注册了两个工作区运行时的真实 结论：行为与描述一致，建议合并。 末尾记录了一个不阻塞的观察项，以及一个在 base 上同样复现的既有问题（因此不是本 PR 引入的）。 Head 环境两个工作区 两个探针扩展以 user scope 全局安装，各带一个 skill 和一个斜杠命令：
整个 A/B 就建立在这一处不对称上：当输入区/管理页指向 1. 本 PR 修复的缺陷，已在 base 上复现当输入区工作区选为 浏览器实际抓到的请求（两侧扩展存储状态完全一致）：
切回 2. 扩展管理跟随所选工作区base 的 Plugins ▸ Extensions 没有工作区选择器、只读 primary，所以无论你关心哪个工作区， 3. 启用状态被协调进存活的次级运行时这是核心论断，所以通过真实界面操作、再从 daemon 读回验证。在运行时已经存活的情况下，于管理页把
在两个次级运行时都存活时新装一个扩展，表现一致：store generation 推进，两个运行时的
4. 文档契约抽查
5. 测试在
非空洞性 —— 四个变异，全部被杀掉。 每个变异都只回退本 PR 新增的某一处保护：
第一个变异最关键：它证明上面那条 E2E 判据确实对这个修复本身敏感，而不是被环境凑出来的。四个被变异文件中有三个在 发现（均不阻塞合并）1 · 既有问题，非本 PR 引入 —— 重新启用扩展后，已存活会话不会恢复它的斜杠命令。 同一会话、同一运行时、 此时 daemon 侧是正确的 —— 2 · 观察项 —— 新的输入区加载器没有套客户端 action 超时。 被替换掉的 范围有限，而且我没有复现出来 —— 在真正冷启动的 3 · 说明，无需处理 —— 未受信任次级工作区的客户端分支属于纵深防御。
|
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind Round summaryThis round is a same-run verification repair: the round-16 commit ( The rejection and its repairRejection (deterministic gate): Root cause: the round-16 witness test Fix: index the call tuple through the file's own established pattern — `(call) => Why it was not pushed: Note: the base has since been auto-updated; the verdict below predates that update, and the next round's re-measurement may charge the round. tests failed in packages/core 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 验证门的拒绝原因与日志证据见上方英文部分（gate-rejection 不翻译）。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/34654953653 🧠 Handled by Qwen Code · model/模型 |
|
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 |
|
🤖 AutoFix updated a stale base — the fix did not pass verification, but this PR was behind Autofix round — PR #11086 (extension workspace runtimes)Same-run verification repairThe previous commit ( Repair: the two filters now use the file's own existing convention — Feedback dispositionsFixed in code this round
Why it was not pushed: Note: the base has since been auto-updated; the verdict below predates that update, and the next round's re-measurement may charge the round. tests failed in packages/core 中文说明🤖 AutoFix 更新了一个过期的 base —— 修复未通过验证，但本 PR 落后于 验证门的拒绝原因与日志证据见上方英文部分（gate-rejection 不翻译）。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/34687215427 🧠 Handled by Qwen Code · model/模型 |
7 participants
What this PR does
This PR makes the global extension catalog available through the runtime selected for each workspace. It reconciles extension state into live workspace runtimes, exposes workspace-qualified daemon and SDK access, and updates extension management, the composer add menu, and
@mentions to use the selected workspace runtime. It also advertises the complete capability surface and documents the protocol behavior.Why it's needed
Extensions are globally installed, but their skills and commands must be projected into the runtime that owns the active workspace. Without workspace-qualified routing, extension management and task composition can read or update the primary runtime instead of the selected runtime, so secondary workspaces may miss extension-provided skills or show stale state.
Reviewer Test Plan
How to verify
Start the daemon with at least two workspace runtimes, select a non-primary workspace, and confirm that the extension management page lists the global catalog while enablement changes are reconciled into that workspace's live runtime. In a new task for the same workspace, confirm that the add menu and
@menu expose extension-provided entries. Switch workspaces and confirm each UI surface follows the selected runtime without falling back to the primary runtime. Automated coverage verifies coordinator reconciliation, workspace-qualified routes, ACP propagation, SDK behavior, capability documentation, and Web Shell consumers.Evidence (Before & After)
Before: extension UI and composer discovery could resolve through the primary runtime, so a selected secondary workspace could not reliably access extension-provided skills.
After: extension management and composer discovery resolve through the selected workspace runtime, with global extension metadata reconciled into each live runtime.
Tested on
Environment (optional)
Node.js 22 workspace build and package-scoped Vitest runs against the daemon, SDK, and Web Shell.
Risk & Scope
Linked Issues
Related to #10593.
中文说明
本 PR 做了什么
本 PR 让全局扩展目录能够通过每个工作区所选的运行时使用。它会把扩展状态协调到存活的工作区运行时中，提供工作区限定的 daemon 和 SDK 访问，并让扩展管理、输入区加号菜单和
@提及都使用当前选中的工作区运行时。同时补全 capability 声明并记录协议行为。为什么需要
扩展是全局安装的，但扩展提供的技能和命令必须投射到当前工作区所属的运行时中。如果没有工作区限定路由，扩展管理和新建任务输入区可能读取或更新主运行时，而不是选中的运行时，导致次级工作区缺少扩展技能或显示过期状态。
Reviewer 测试计划
如何验证
启动至少包含两个工作区运行时的 daemon，选择一个非主工作区，确认扩展管理页展示全局扩展目录，并且启用状态变更会协调到该工作区的存活运行时。在同一工作区新建任务，确认加号菜单和
@菜单能够展示扩展提供的条目。切换工作区，确认所有界面都跟随选中的运行时，并且不会回退到主运行时。自动化测试覆盖协调器同步、工作区限定路由、ACP 传播、SDK 行为、capability 文档和 Web Shell 消费端。前后对比证据
改造前：扩展界面和输入区发现逻辑可能通过主运行时解析，因此选中的次级工作区无法稳定获取扩展提供的技能。
改造后：扩展管理和输入区发现逻辑通过选中的工作区运行时解析，并将全局扩展元数据协调到每个存活运行时。
测试平台
环境（可选）
Node.js 22 工作区构建，以及针对 daemon、SDK 和 Web Shell 的包级 Vitest 测试。
风险与范围
关联事项
关联 #10593。