refactor(daemon): decouple extension activation refresh - #10991
Conversation
E2E test reportValidated locally on macOS with Node.js 22.
Commands/results: Extension route/controller/capability tests 67 passed; Web Shell behavior tests 4 passed; full daemon server file 1,173 passed with 3 unrelated concurrent socket/timeout flakes, all 3 passing on serial rerun; Not run against a production instance; no production Extension state was changed. |
|
Thanks for the PR! Template looks good ✓ — all required headings present, Tested-on filled in, full Chinese translation. Problem: partly substantiated, and one half of the stated motivation doesn't survive reading the code. The latency coupling is real. Activation currently runs The second claim, that coupling "lets refresh failures downgrade an otherwise successful policy commit", isn't what the code does. All three refresh-failure paths already land on Direction: aligned. Capability-gating a contract change is the established pattern on this surface, and the daemon-side mechanism isn't new: Size: cross-package ( Approach: scope is tight and I couldn't find a smaller version of it. All seven One question I couldn't settle from the diff, and it's the main thing I'd want answered before merging: the client-side remedy is narrower than the behaviour it replaces. I did confirm the 30-second poller closes the gap — it filters Risk: no Stage 1e high-risk path match. The elevated item is the multi-workspace consistency window above, which is a product decision rather than a code defect. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ —— 所有必填小标题齐全,测试平台已填写,中文翻译完整。 问题: 部分成立,而且动机里有一半经不起读代码。 延迟耦合是真实存在的。activation 目前会在 operation 内部对所有已注册 runtime 执行 第二个说法,即耦合"会让 refresh 失败把一个本来成功的策略提交降级",与代码实际行为不符。三条 refresh 失败路径全都落在 方向: 对齐。用 capability 来门控契约变更是这个接口面既有的做法;而且 daemon 侧的机制并不是新东西: 规模: 跨包( 方案: 范围收得很紧,我找不到更小的实现版本。七处 有一个我无法从 diff 判断的问题,也是合并前我最希望得到答复的一点:客户端的补救措施比它所替代的行为范围更窄。 我确实确认了 30 秒轮询器能补上这个缺口——它按 风险: Stage 1e 没有命中高风险路径。需要留意的点是上面这个多 workspace 一致性窗口,它属于产品决策,而不是代码缺陷。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "extension_activation_explicit_refresh" |
— Qwen Code · serve A/B
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code reviewRead against One blocker, one decision that needs a human, and some accuracy points. Blocking — the new capability tag was not added to the integration test's baseline list
I enumerated every hardcoded copy of that list to be sure this is the only miss, and it is: the registry itself, Needs a product decision — a global activation change now refreshes only the current workspaceCovered in the gate comment, restating the code path concretely because it's the substance of the change. I traced the convergence path and it does hold: the poller filters What changes is the guarantee. Workspace-scoped activation is unaffected — it refreshed only Accuracy points
Non-blocking
Everything else I checked held up: the fire-and-forget refresh is dispatched after Flow after this changesequenceDiagram
participant P1 as Web Shell ExtensionsManagerPage
participant P2 as activation route
participant P3 as extensions controller
participant P4 as Extension Store generation
participant P5 as runtime bridge sessions
participant P6 as 30s generation poller
P1->>P2: activation change (global or workspace scope)
P2->>P3: sendOperation with skipRefresh true
P3->>P4: durable policy commit, generation bumps
P3-->>P1: operation succeeded, no reconciling phase
P1->>P5: refreshExtensionRuntime, current workspace only
Note over P5: sessions in other workspaces are untouched here
P6->>P4: read store generation
P6->>P5: refresh every runtime whose applied generation lags
TestingThis is an unattended CI run, so nothing was built or executed here — the evidence below is this PR's own CI on the reviewed commit, read through the API. No tmux or real-scenario testing was driven.
The unit suite, lint, typecheck and the serve A/B job had not finished when this was fetched — Not verified: the author's local results quoted in the PR body (51 route tests, 14 controller tests, 2 capability doc contract tests, 4 Web Shell activation-refresh tests, the 1,176-test daemon server file, and three socket/timeout flakes passing on serial rerun). That is the author's claim, tested on macOS only, not evidence this review re-ran. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 Bot orchestration jobs ( Sandboxed verification would settle what CI cannot: 中文说明代码审查基于 一个阻断项、一个需要人来拍板的决策,以及几处表述准确性问题。 阻断——新 capability tag 没有加进集成测试的基线列表
为了确认这是唯一的遗漏,我把该列表的所有硬编码副本都列了一遍,确实只有这一处漏了:registry 本身、 需要产品决策——全局 activation 变更现在只刷新当前 workspace闸门评论里已经讲过,这里把代码路径具体重述一遍,因为它是本次改动的实质。 我跟了一遍收敛路径,它确实成立:轮询器按 变化的是保证强度。workspace 范围的 activation 不受影响——它之前也只刷新 表述准确性
非阻断
其余我检查过的地方都成立:fire-and-forget 的 refresh 是在 测试这是一次无人值守的 CI 运行,因此本地没有构建或执行任何代码——下面的证据来自本 PR 自己在被审查 commit 上的 CI,通过 API 读取。没有驱动 tmux 或真实场景测试。
抓取时单元测试套件、lint、typecheck 与 serve A/B job 尚未结束—— 未验证:PR 正文引用的作者本地结果(51 个路由测试、14 个 controller 测试、2 个 capability 文档契约测试、4 个 Web Shell activation-refresh 测试、1,176 个用例的 daemon server 文件,以及 3 个 socket/timeout 抖动串行复跑通过)。那是作者的自述,且仅在 macOS 上测过,不是本次审查重跑的证据。 CI 表格见上方英文部分(由 finalize 工作流在 CI 结束后就地更新)。机器人编排 job( 沙箱验证可以补上 CI 补不了的部分: — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 2/5 — the shape of this change is right and the diff is disciplined, but CI is red on a tag list the PR forgot to update, and one behaviour change is a product decision I shouldn't make from a diff. My independent proposal before reading the code was almost exactly what this PR does: keep the commit as the durability point, reuse the controller's existing What I'd say honestly: this is a well-built PR. Fifty-six production lines accomplish the decoupling, the seven I'm still not approving, for three reasons in order of weight. The integration check is red and this PR caused it. The multi-workspace question is not mine to settle. Global default activation goes from refreshing every runtime immediately to refreshing the current workspace immediately and the rest within ~30 seconds via the poller. I traced the poller and convergence does hold, so nothing is lost permanently — but the guarantee genuinely weakens, there's no client-callable way to refresh all runtimes, and the sharpest case is a global disable where the UI reports success while the extension stays live in other workspaces' sessions for up to half a minute. That may well be the right tradeoff for the latency win. "May well be" is my judgment, not a verification, and this is a cross-package Half the stated motivation is wrong, and that changes what this PR is. Refresh failures already produce What would move me to approve: the integration-test line; either a per-affected-runtime refresh on the global path or an explicit statement in Note on evidence: the unit suite, lint, typecheck and serve A/B job were still running when I fetched CI, and I did not poll them — so those results are unknown to this review, not assumed green. The finalize workflow updates the CI table in the review comment once they settle. Nothing was built or executed here; the review is static plus this PR's own CI read through the API. Requesting changes on the blocking item and deferring the design question to a maintainer. 🙏 中文说明Confidence: 2/5 —— 改动的形态是对的,diff 也很克制,但 CI 因为本 PR 漏改的一处 tag 列表而变红,而且有一个行为变更属于产品决策,不该由我从 diff 里拍板。 在读代码之前,我自己的方案与这个 PR 几乎完全一致:把 commit 作为持久化点,仅在 activation 路由上复用 controller 已有的 老实说:这是一个做得很扎实的 PR。56 行生产代码完成了这次解耦;七处 我仍然不批准,按权重排列有三点理由。 集成检查是红的,而且由本 PR 引起。 多 workspace 的问题不该由我来定。 全局默认 activation 从"立即刷新所有 runtime"变成"立即刷新当前 workspace、其余在约 30 秒内由轮询器刷新"。我跟了轮询器的实现,收敛确实成立,所以不会有永久丢失——但保证强度确实变弱了,客户端没有任何可调用方式能刷新全部 runtime,而最尖锐的场景是全局禁用:UI 已报成功,而该 Extension 在其他 workspace 的会话里还会存活最长半分钟。为了延迟收益,这个取舍很可能值得。但"很可能值得"是我的判断,不是验证结果;而这是一个跨包、触及核心路径的 动机里有一半是错的,而这改变了这个 PR 的性质。 三条 refresh 失败路径都已经产出 能让我转为批准的条件:补上集成测试那一行;在全局路径上按受影响的 runtime 逐个刷新,或在 关于证据:抓取 CI 时单元测试套件、lint、typecheck 与 serve A/B job 仍在运行,我没有轮询等待——因此这些结果对本次审查是未知的,而不是被假定为绿。它们结束后,finalize 工作流会更新审查评论里的 CI 表格。本次没有构建或执行任何代码;审查方式为静态审查加上通过 API 读取本 PR 自己的 CI。 就阻断项提出修改请求,并把设计问题转交维护者。🙏 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Needs one fix and one human decision before this can merge — details in my review comments above. 🙏
Blocking: integration-tests/cli/qwen-serve-routes.test.ts asserts the full baseline capability array with toEqual, and extension_activation_explicit_refresh was never added to that hardcoded list. That is the red Integration Tests (no-AK, No Sandbox) check — the assertion diff names this PR's own new tag as the extra received element. One-line insertion, in the same position the tag occupies in the registry. I checked every other hardcoded copy of that list (the registry, server.test.ts, qwen-serve-protocol.md, workspace-qualified-extensions.test.ts) and they were all updated, so this is the only miss.
Needs a maintainer's call: global default activation previously refreshed every runtime immediately; it now refreshes the current workspace immediately and the rest within ~30s via the generation poller. I traced the poller and convergence holds, so nothing is lost permanently, and the Web Shell's workspace-scoped activation path is unaffected. But the guarantee weakens, there is no client-callable operation that refreshes all runtimes, and the sharpest case is a global disable: the UI reports success while the extension stays live in other workspaces' sessions for up to 30 seconds. Either submit the refresh per affected runtime, or state the window explicitly in docs/users/qwen-serve.md. This is a cross-package refactor over core paths, so I'm escalating the tradeoff rather than signing off on it — no maintainer resolved deterministically (no area label, no human reviewer yet), so I'm not guessing a login.
Also worth correcting: the description says coupling "lets refresh failures downgrade an otherwise successful policy commit". All three refresh-failure paths already produce succeeded_with_warnings, and the code carries an explicit comment that a failed notification must not turn a committed mutation into a failed operation; status: 'failed' is only reachable when the commit never happened. The latency half of the motivation is real, this half isn't.
To be clear about what I'm not saying: the design itself is sound. Fifty-six production lines, all seven skipRefresh additions correctly confined to activation routes, install/update/uninstall untouched, the deleted helper's two call sites both rewritten here, an existing i18n key reused in both locales, and unusually thorough documentation. Non-blocking: the legacy POST /workspace/extensions/refresh is not an operation handle despite the protocol doc naming both routes in one sentence, and a doc reflow left a sentence broken mid-clause.
Note: the unit suite, lint, typecheck and serve A/B job were still running when I fetched CI and I did not poll them, so those results are unknown to this review rather than assumed green. Nothing was built or executed here — static review plus this PR's own CI read through the API.
中文说明
合并前需要一个修复和一个人工决策——细节见上方的审查评论。🙏
阻断项: integration-tests/cli/qwen-serve-routes.test.ts 用 toEqual 断言完整的基线 capability 数组,而 extension_activation_explicit_refresh 从未加入那个硬编码列表。这就是 Integration Tests (no-AK, No Sandbox) 变红的原因——断言 diff 把本 PR 自己新增的 tag 列为多出来的接收项。一行插入即可,位置与该 tag 在 registry 中的位置一致。我检查了该列表的其他所有硬编码副本(registry、server.test.ts、qwen-serve-protocol.md、workspace-qualified-extensions.test.ts),它们都更新了,所以这是唯一的遗漏。
需要维护者拍板: 全局默认 activation 之前会立即刷新所有 runtime;现在只立即刷新当前 workspace,其余在约 30 秒内由 generation 轮询器刷新。我跟了轮询器实现,收敛确实成立,不会有永久丢失,而且 Web Shell 的 workspace 范围 activation 路径不受影响。但保证强度变弱了,客户端没有任何可调用操作能刷新全部 runtime,最尖锐的场景是全局禁用:UI 已报成功,而该 Extension 在其他 workspace 的会话里还会存活最长 30 秒。要么按受影响的 runtime 逐个提交 refresh,要么在 docs/users/qwen-serve.md 中明确写出这个窗口。这是一个跨包、触及核心路径的 refactor,所以我选择上报这个取舍而不是替它签字——此处没有确定性地解析出维护者(无领域标签、尚无人类 reviewer),因此我不会去猜登录名。
另外建议修正: 描述中说耦合"会让 refresh 失败把一个本来成功的策略提交降级"。三条 refresh 失败路径都已经产出 succeeded_with_warnings,代码里还有一句明确注释:通知失败不能把一个已提交的变更变成失败的 operation;只有在提交根本没发生时才可能走到 status: 'failed'。动机里关于延迟的那一半是真实的,这一半不是。
需要说清我没有在说什么:设计本身是可靠的。56 行生产代码,七处 skipRefresh 全部正确地限定在 activation 路由,install/update/uninstall 未被触及,被删辅助函数的两个调用点都在此改写,两种语言都复用了已有的 i18n 键,文档也相当充分。非阻断项:尽管协议文档在同一句里点名了两条 refresh 路由,legacy 的 POST /workspace/extensions/refresh 并不是 operation handle;另外一处文档换行错乱让句子在从句中间断开。
注意:抓取 CI 时单元测试套件、lint、typecheck 与 serve A/B job 仍在运行,我没有轮询等待,因此这些结果对本次审查是未知的,而不是被假定为绿。本次没有构建或执行任何代码——静态审查加上通过 API 读取本 PR 自己的 CI。
— Qwen Code · qwen3.8-max-2026-09-02
|
Addressed every item from the requested-changes review on the current head:
There are no review threads. The requested-changes review is anchored to |
Dismissed as stale after the current head fixed the missing capability expectation, documented the deliberate 30-second cross-workspace convergence window, corrected the refresh-warning motivation, and clarified both refresh response shapes. No review threads remain; the review targets 8b0e0e7 while current head is 1f06d58.








What this PR does
Extension activation operations now complete after their activation policy is durably committed instead of directly refreshing every active session. A new
extension_activation_explicit_refreshcapability lets clients distinguish this contract from older daemons. The Web Shell uses that capability to submit a current-workspace refresh after activation succeeds without polling the refresh operation or keeping the Extension page busy. The independent 30-second generation reconciler remains unchanged.Why it's needed
Refreshing every active session can rebuild and retransmit a large command and Skill snapshot, so coupling that work to a simple activation file update makes the activation operation slow and mixes runtime-refresh warnings into an otherwise successful policy operation. Separating the operations gives callers control over immediate application while preserving eventual generation reconciliation.
Reviewer Test Plan
How to verify
Start a daemon with an installed Extension and an active session, change the Extension's global or workspace activation, and poll the returned operation. Confirm that activation reaches
succeededafter the policy commit without enteringreconciling, withoutrefreshedorfailedresult fields, and without directly refreshing the session. Submit the independent workspace refresh and confirm that it owns runtime refresh separately. In Web Shell, confirm that the activation control unlocks before the submitted refresh finishes; an older daemon without the capability must not receive a duplicate refresh.Local verification:
npm run build;npm run typecheck; focused ESLint; 51 Extension management route tests; 14 Extension controller tests; 2 capability documentation contract tests; 4 Web Shell activation-refresh tests; and the 1,176-test daemon server file. The full server run's three unrelated socket/timeout flakes passed when rerun serially.Evidence (Before & After)
N/A — behavior and operation-contract change with automated coverage; no visual layout change.
Tested on
Environment (optional)
Local Node.js 22 workspace with daemon HTTP route tests and jsdom Web Shell tests.
Risk & Scope
extension_activation_explicit_refreshand submit the existing refresh operation. The Web Shell refreshes only its current workspace; other workspaces can retain the previous activation until the next 30-second generation-reconciler pass, with failures retried later.Linked Issues
N/A
中文说明
本 PR 做了什么
Extension activation operation 现在会在 activation 策略持久化提交后完成,不再直接刷新所有活动 session。新增
extension_activation_explicit_refreshcapability,供调用方区分该契约与旧 daemon。Web Shell 在 activation 成功后根据该 capability 向当前 workspace 提交 refresh,但不轮询 refresh operation,也不会继续锁定 Extension 页面。独立的 30 秒 generation reconciler 保持不变。为什么需要
刷新所有活动 session 会重建并传输较大的命令与 Skill 快照。把这项工作耦合到简单的 activation 文件写入,会拖慢 activation operation,并把 runtime refresh warning 混入已经成功的策略 operation。拆分两个 operation 后,调用方可以决定是否立即生效,同时保留 generation 的最终收敛。
Reviewer Test Plan
如何验证
启动一个安装了 Extension 且存在活动 session 的 daemon,修改全局或 workspace activation,并轮询返回的 operation。确认 activation 在策略提交后直接进入
succeeded,不进入reconciling,结果不包含refreshed或failed,也不直接刷新 session。随后提交独立的 workspace refresh,确认 runtime refresh 由它单独负责。在 Web Shell 中确认 activation 控件在 refresh 完成前已经解锁;连接不包含该 capability 的旧 daemon 时不得重复提交 refresh。本地验证:
npm run build;npm run typecheck;定向 ESLint;51 个 Extension management 路由测试;14 个 Extension controller 测试;2 个 capability 文档契约测试;4 个 Web Shell activation-refresh 测试;以及包含 1,176 个用例的 daemon server 测试文件。完整 server 测试中 3 个无关 socket/timeout 抖动用例在串行复跑后通过。前后证据
N/A——这是行为与 operation 契约变更,没有视觉布局变化,已有自动化覆盖。
测试平台
环境(可选)
本地 Node.js 22 workspace,运行 daemon HTTP 路由测试与 jsdom Web Shell 测试。
风险与范围
extension_activation_explicit_refresh并提交现有 refresh operation。Web Shell 只刷新当前 workspace;其他 workspace 最长可能到下一轮 30 秒 generation reconciler 才应用新 activation,失败时由后续轮次重试。关联 Issue
N/A