fix(serve): configure live-state polling with a five-second default - #11339
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
E2E verificationVerified commit: Real browser request timestamps against isolated daemon processes:
Each local daemon used Nine browser regressions passed using the command below from npx playwright test --config playwright.config.ts client/e2e/web-shell.live-state-poll-interval.spec.ts client/e2e/web-shell.session-live-state.spec.ts --project chromium --workers 1Also passed on this commit: root build, typecheck and bundle; changed-file formatting/lint; 59 capabilities tests; five environment/docs contract tests; 937 focused Web Shell unit tests. The unit tests cover accepted/rejected interval values, startup environment scope, in-flight and retained-state preservation, local refreshes, and cleanup. Limitations: Windows/Linux were not tested. These probes cover separate process starts and page reloads, not a same-port restart with the page kept open. To apply a changed environment value, restart the daemon and reload existing Web Shell pages. 中文说明E2E 验证验证提交: 使用真实浏览器请求时间戳,对隔离的 daemon 进程进行测量:
每个本地 daemon 使用 从 npx playwright test --config playwright.config.ts client/e2e/web-shell.live-state-poll-interval.spec.ts client/e2e/web-shell.session-live-state.spec.ts --project chromium --workers 1本提交还通过了根目录构建、类型检查和 bundle,变更文件格式和 lint 检查,59 项 capabilities 测试、5 项环境变量/文档契约测试,以及 937 项 Web Shell 定向单测。单测覆盖间隔值的接受与拒绝、启动环境作用域、进行中请求和已保留状态的保持、本地刷新及清理。 验证边界:未测试 Windows/Linux。这些探测覆盖独立进程启动及页面刷新,不覆盖保持页面打开时在同一端口重启的过程。修改环境变量后,需要重启 daemon 并刷新已有 Web Shell 页面。 |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — 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 |
|
Re-run at a new head. Template still complete ✓ Problem: unchanged and still real — quantified against the source rather than asserted. #11327's triage confirmed the arithmetic: one loop over every visible workspace on a fixed tick, so five workspaces at 2s is ~150 requests/min against a read bucket whose default is 120/min under Direction: I'm lifting the defer I posted last run. Two core-area owners (
Size: cross-package ( Approach: same scope question as last run, unchanged by the merge. Cutting the configuration entirely — Risk: no high-risk-path matches; nothing revert-correlated is touched. The behavioural risk is the one the description names honestly — cross-client discovery of a permission wait moves from ≤2s to ≤5s by default. One new item, and it is the only thing standing between this PR and merge: the merge from Moving on to code review. 🔍 中文说明本次是在新 head 上的重跑。 模板依然完整 ✓ 问题: 与上次一致,仍然是真实问题——是对照源码量化过的,不是口头断言。#11327 的 triage 已确认这组数字:对每个可见 workspace 用固定周期跑一个循环,因此 5 个 workspace 在 2 秒间隔下约为每分钟 150 次请求,而开启 方向: 我撤回上一轮发出的 defer。已有两位 core 领域 owner(
规模: 跨包改动( 方案: 与上次相同的范围问题,合并没有改变它。如果完全去掉可配置性——只把 风险: 未命中高风险路径,没有触及与回滚相关的文件。行为风险就是描述里坦诚写出的那一条:跨客户端发现"等待授权"状态的延迟由默认 ≤2 秒变为 ≤5 秒。新增一项,也是唯一挡在本 PR 与合并之间的问题:这次从 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewIndependent proposal first. From the title and "Why it's needed" alone I'd have done: change the constant, then — because the interval has to become dynamic once the daemon advertises it — move Findings 2–5 below are re-verified line-by-line against 1. The red
|
| File | What changed |
|---|---|
packages/cli/src/serve/routes/capabilities.ts |
Resolves the env var once at registration with bounds checking and advertises it on the envelope |
packages/cli/src/serve/types.ts |
Adds the optional field to the capabilities envelope type |
packages/cli/src/serve/server.test.ts |
16 env-input cases plus a startup-snapshot case that mutates the env after app creation |
packages/sdk-typescript/src/daemon/types.ts |
Mirrors the optional field on the SDK capabilities type |
packages/web-shell/client/session-catalog/session-live-state-poll-interval.ts |
New client-side resolver and the 5000ms default constant |
packages/web-shell/client/session-catalog/session-live-state-poll-interval.test.ts |
Resolver bounds and fallback cases (16) |
packages/web-shell/client/session-catalog/workspace-session-live-state.ts |
Default 2000 to 5000, new optional hook option, timer split into its own effect behind a ref |
packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx |
New cadence, interval-change and immediate-wake tests; three tests re-pinned to an explicit 2000ms |
packages/web-shell/client/App.tsx |
Threads the advertised interval into the sidebarless live-state hook |
packages/web-shell/client/App.test.tsx |
Asserts the threaded value in both the enabled and disabled cases |
packages/web-shell/client/components/SessionOverviewPanel.tsx |
Threads the interval; drops the hardcoded cadence from one comment, misses two siblings |
packages/web-shell/client/components/SessionOverviewPanel.test.tsx |
Asserts the panel passes the advertised interval through |
packages/web-shell/client/components/sidebar/WebShellSidebar.tsx |
Threads the interval into the sidebar hook |
packages/web-shell/client/e2e/web-shell.live-state-poll-interval.spec.ts |
New Playwright spec covering four advertised-interval cases across a page reload |
docs/design/2026-09-08-session-live-state-poll-interval.md |
New design doc for the configurable interval; does not reference the 2026-08-16 doc it supersedes |
docs/developers/qwen-serve-protocol.md |
Documents the new optional capability field and its fallback semantics |
docs/users/qwen-serve.md |
User-facing instructions for the env var |
Test evidence
Unattended CI run — I built and executed nothing from this PR. Everything below is read from the PR's own checks via the API at 8da1f392, plus the failing job's log.
| Check | Conclusion |
|---|---|
| Test (ubuntu-latest, Node 22.x) | failure — see finding 1; inherited from main, not this PR |
| Serve A/B (ubuntu-latest, Node 22.x) | success — but blind to this change, see finding 2 |
| web-shell E2E Smoke (ubuntu-latest, Node 22.x) | success |
| Lint & Static (ubuntu-latest, Node 22.x) | success |
| Integration Tests (no-AK, No Sandbox) | success |
| Capture web-shell visuals (ubuntu-latest, Node 22.x) | success |
| Desktop Shell (ubuntu-22.04) | success |
| Desktop Shell (windows-2022) | success |
| Live Host (macos-latest) | success |
| OpenTUI no-flicker gate | success |
| TUI parity snapshots (ink vs opentui) | success |
| Real daemon E2E / Java 11 | success |
| ubuntu-latest / Java 11, 17, 21 | success |
| macos-latest / Java 21 | success |
| windows-latest / Java 21 | success |
| Classify PR | success |
| Test (macos-latest, Node 22.x) | skipped (matrix) |
| Test (windows-latest, Node 22.x) | skipped (matrix) |
| Integration Tests (CLI, No Sandbox) | skipped |
| review-pr | cancelled (bot orchestration, not PR CI) |
Reading that honestly:
- One required check is red and it is not this PR's. Finding 1 has the chain: green at
128b63e, red at8da1f392, the two files the failing test reads are byte-identical tomain,mainis red on the same assertion, and#10991(8f12174) is the commit that broke it. I classified this from the diff, the check identity andmain's own result — not from anything the log body claims about itself. - Exactly one test failed in the whole job. Across every vitest project the job ran, the log carries a single
Failed Tests 1block and a singleTest Files 1 failedsummary. The failing project ran 29,188 tests (29,097 passed, 90 skipped); the others — including the web-shell projects holding every suite this PR adds or edits — reported no failures. - The unit suite is Linux-only here. macOS and Windows legs are
skippedby the matrix, which matters because the description says Windows and Linux runtime behaviour is untested. Serve A/Bis green and, per finding 2, green in a way that is reproducibly blind to an added/capabilitiesfield. Treat it as no signal for this change rather than as a false all-clear.
Not verified, and why: the before/after Chromium measurements in the description (2000ms baseline on installed v0.23.0, 4999ms unset, 10000ms with the env var) are the author's own results on macOS, not evidence I reproduced — this run executes no PR code. The recurring ~5s gap in a real browser is therefore still the one behavioural claim resting on a single environment. Windows/Linux runtime behaviour is untested per the description, and a same-port daemon restart with a page held open is explicitly out of the tests' scope.
Sandboxed verification is already in flight for exactly this gap — a verify job is running in run 34207899436 alongside this triage re-run, and its report will land in this thread. What it should settle: that the recurring live-state gap in a real browser is ~5000ms with the env unset and ~10000ms with QWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MS=10000, which no completed check demonstrates and which the A/B is blind to. I have not read its output and am not predicting it. /tmux is the wrong lane here — the surface is Web Shell in a browser, not the TUI. The author has admin access, so this is a direct run rather than a sponsored one.
One note for the two approvals already on this head: ytahdn submitted at 09:01:45 and chiga0 at 09:08:07; Test (ubuntu-latest, Node 22.x) completed failure at 09:08:49. Both predate the red result, and chiga0 states it did not run the suite. Neither approval accounts for finding 1.
中文说明
代码审查
先给出我的独立方案。 只看标题和"为什么需要",我会这么做:改掉常量;然后——因为一旦 daemon 公布间隔,间隔就必须变成动态的——把 setInterval 从持有状态的 effect 里挪到一个以间隔为 key 的独立 effect,通过 ref 触达轮询循环,这样周期变化才不会丢弃已保留的快照或重启订阅。这正是本 PR 的做法,从 main 的合并也没有破坏它。我的基线与 diff 依然一致,包括最容易做错的那一部分。
下面第 2 至第 5 条都是针对 8da1f392 逐行重新核实过的,不是从上一轮照搬。上一轮有一处行号是错的,这里已更正。
1. 红色的 Test (ubuntu-latest, Node 22.x) 来自 main,不是本 PR——但它挡住了本 PR
这是当前 head 与合并之间唯一的障碍,所以这里给出完整证据链而不是结论。
该 job 在 8da1f392 上结论为 failure,且只有一个测试失败:
FAIL src/serve/capabilities-docs-contract.test.ts > conditional serve capability documentation > keeps the daemon index capability counts in sync
AssertionError: expected 158 to be 159 // Object.is equality
❯ src/serve/capabilities-docs-contract.test.ts:54:32
Test Files 1 failed | 1016 passed (1017)
Tests 1 failed | 29097 passed | 90 skipped (29188)
该测试读取 docs/developers/daemon/00-index.md,断言其中记载的数量等于 Object.keys(SERVE_CAPABILITY_REGISTRY).length。本 PR 两个文件都没有碰。破坏是随合并进来的,证据如下:
- 在 PR 合并
main之前的自有提交128b63e上,Test (ubuntu-latest, Node 22.x)是success。此后 head 上唯一发生的事就是Merge branch 'main'。 packages/cli/src/serve/capabilities.ts在main(cffc4049)与本 head 之间逐字节相同(822 行,diff -u为空),且00-index.md:129在两侧都写着158 registered tags; 45 conditional tags。相同的两个输入、相同的确定性测试——所以main也会同样失败;而main自己在cffc4049上的Test (ubuntu-latest, Node 22.x)随后已完成,结论为failure。这一点现在是观测到的,不是推断。- 引入它的提交是
8f12174——refactor(daemon): decouple extension activation refresh (#10991),今天 08:39 合并。它往 registry 里加了extension_activation_explicit_refresh: { since: 'v1' }(capabilities.ts+4),而其 17 个文件的清单里没有docs/developers/daemon/00-index.md。registry 数量从 158 变成 159,文档仍写着 158。
修复方式: 把 docs/developers/daemon/00-index.md:129 的 158 registered tags 改成 159。它本该在 main 上修(约束是在那里被破坏的,眼下所有合并了 main 的开放 PR 都因此变红),但在本 PR 里加一行同样能立刻解开本 PR 的阻塞,并在合并时把修复带进去——由作者决定,作者有 admin 权限,两条路都很便宜。
需要说清楚,因为红色套件通常意味着真问题:本 PR 自己的每一个测试都在同一个 job 里通过了。 ✓ src/serve/server.test.ts (1246 tests)、✓ session-catalog/workspace-session-live-state.test.tsx (34 tests)、✓ session-catalog/session-live-state-poll-interval.test.ts (16 tests)、✓ components/SessionOverviewPanel.test.tsx (97 tests)。失败文件的另一个用例——检查本 PR 确实编辑过的 qwen-serve-protocol.md 里 conditional-features 表格的那个——也通过了,说明该文档的合并是干净的。
2. Serve A/B 的盲区现在在两个 head 上都复现了——但它没回答的问题已由别处回答
上一轮我指出 Serve A/B 是绿的却报告零响应变化,而对一个给 /capabilities 无条件新增顶层字段的 diff 来说这不可能是真的。当时我无法定论。现在它在当前 head 上又发生了一次,所以这是可复现的 harness 盲区,不是偶发:
- head checkout 可验证为正确:
HEAD is now at 8da1f39 Merge branch 'main' into fix/serve-live-state-poll-interval;base 是f70aace。 - 两侧都抓到了该路由:head 运行(由
.../head/packages/web-shell/dist提供服务)和 base 运行(.../base/...)各有一条captured capabilities (HTTP 200)。 - 结论是:
serve-ab: 0 changed field(s)。
我重新读了 harness,而不是沿用上一轮的笔记。diffJson 对对象按两侧 key 的并集比较,对只存在于 after 的 key 输出 { kind: 'added' }(serve-ab-diff.mjs:74-96),而 DEFAULT_VOLATILE(第 39 行)不可能匹配 sessionLiveStatePollIntervalMs——它唯一的正则要求关键词前紧跟 ^、. 或 ],而该路径中不含任何关键词。所以 differ 本应报告一个新增字段。我无法从外部判断是哪一份抓取有问题,而我不会执行 PR 带来的代码,所以不猜机制。我能说的是:它在两个不同 head 上、checkout 均正确的情况下复现了,因此 ✅ No response changes across 12 scenario(s) 不应被当作对本 PR 任何新增内容的线级确认。这值得对 .github/scripts/serve-ab-*.mjs 单独开一个 issue——下一个增量协议字段它同样会静默漏报。
为什么这不再拖住本 PR: daemon 侧的声明现在有一个已完成的绿色检查支撑。src/serve/server.test.ts (1246 tests) 在此 head 通过,而这些测试通过 HTTP 驱动真实的 Express 应用(request(app).get('/capabilities')),对 16 个环境变量输入断言 response.body.sessionLiveStatePollIntervalMs,外加一个在 createServeApp 之后修改 daemonEnv、用来钉住启动快照语义的用例。这正是 A/B 本该做出却没做出的断言。上一轮该文件仍在运行;这个缺口现在闭合了。
3. 本 PR 改动的文件里遗留两处过期的 2s 注释——已在当前 head 确认
这次清扫几乎做全了(SessionOverviewPanel.tsx:374 和 workspace-session-live-state.ts:391 都改了)。有两处兄弟注释还在:
packages/web-shell/client/session-catalog/workspace-session-live-state.ts:246—— "…the loop re-runs a full reconcile on every 2s tick with no decay."(上一轮写的是:239;当前 head 上正确的是:246)packages/web-shell/client/components/SessionOverviewPanel.tsx:95—— "The daemon's live-state channel (2s, coordinated through the shared session…"
两处现在都不成立:默认是 5 秒,而且运维可配置到约 24.8 天。第 246 行更关键——它正是后来的人用来理解"reconcile 冷却窗口与轮询周期如何相互作用"的那条注释,而这个相互作用正是新周期真正改变的东西。补充一句以求完整:workspace-session-live-state.test.tsx 里的 every 2s tick 应该保留原样,那个测试被显式钉在 pollIntervalMs = 2_000。
4. 间隔读的是 workspace.capabilities,而开关读的是 connection.capabilities——已确认
在 SessionOverviewPanel.tsx 中,:401 以 connection.capabilities?.features?.includes(SESSION_LIVE_STATE_FEATURE) 作为开关,而 :410 从 workspace.capabilities?.sessionLiveStatePollIntervalMs 取间隔。稳态下二者是同一个对象引用,所以今天这不是 bug。分歧出现在过渡态:DaemonWorkspaceProvider 在每次 client/daemon 切换时会把 capabilities 重置为 undefined,而同步 effect 只会朝 connection 方向拷贝,因此在切换期间——以及 capabilities 重新拉取失败之后——enabled 可能凭借过期的 connection 副本继续为 true,而间隔则静默回落到 5000ms 默认值。失效方向是良性的(流量变多而不是新鲜度变差,且 5000 就是文档写明的默认值),并且 hook 能正确吸收运行中的间隔变化。从 connection.capabilities——也就是开关已经信任的那个对象——读取间隔,可以消除这种分歧,而不是依赖两者恰好保持同步。App.tsx 会自我屏蔽;SessionOverviewPanel.tsx 和 WebShellSidebar.tsx 则不会。
5. pollAllRef 里放的是可见性处理函数,而不是轮询函数——已确认
pollAllRef.current = onVisibilityWake(workspace-session-live-state.ts:416)让周期性 tick(:432,() => pollAllRef.current?.())变成了 visibilitychange 处理函数(:409)的别名。这在今天之所以不改变行为,只是因为 poll() 自带 document.hidden 判断,从而使处理函数里的那个判断成为冗余。它能工作,但这个不变量现在不是局部的:一个叫 pollAll 的 ref,装着一个名字和第一条语句都在讲可见性的函数。如果以后有人把 poll() 里那个重复的判断当作死代码删掉,周期 tick 就会静默地获得可见性门控,而且没有任何东西会发现——jsdom 的 document.hidden 是 false,这里所有测试都照样通过。用一个直白命名的 pollAll 闭包,让可见性处理函数和定时器都调用它,就能把这个不变量收在一处。
已核查且没有问题的部分
记录下来以免有人重复劳动,均已在当前 head 重新核实:
- 环境变量解析在每个边界上都正确,且客户端解析函数与之完全对称。
Number(undefined)→NaN,Number('')/Number(' ')→0,都落在>= 1_000下界之外;Number.isSafeInteger拒绝1000.5;Number(' 30000 ')→30000被接受,与文档契约一致。resolveSessionLiveStatePollInterval在客户端施加同样的1_000..2_147_483_647边界,因此旧 daemon 传来的非法或缺失字段会回落到 5000,而不会抵达setInterval。解析在注册时执行一次而非每请求执行,确实是启动快照。 - 没有新增管线。
daemonEnv本来就是RegisterCapabilitiesRoutesDeps的字段。 - 传输链路上没有任何东西会剥掉该字段。
DaemonClient.capabilities()只是把解析后的 JSON 做了一次类型断言;SDK 和 web-shell 里都不存在针对该 envelope 的 schema、校验器、白名单或逐字段重建。唯一要求是在DaemonCapabilities上声明它,PR 做了。 - 两个 effect 不会失步。 新 effect 的守卫(
!enabled || targets.length === 0)与持有状态的 effect 完全一致,其依赖数组[client, enabled, pollIntervalMs, targets]与[catalogStore, client, enabled, targets]只差它所 key 的间隔以及catalogStore(memo 于client)。定时器在触发时解引用 ref,因此targets变化导致states重建时仍能调到新闭包。 - 被重新钉住的三个测试正是该钉的那三个。 它们都断言"每个冷却窗口只 reconcile 一次",而
SESSION_LIVE_STATE_RECONCILE_COOLDOWN_MS是10_000——在 5 秒 tick 下POLL * 3变成 15 秒并跨过窗口,会因为错误的原因破坏断言。仍然引用导入常量的那些测试在 5000 下依然正确。 - Web shell: conversation and session loading indicators drop mid-turn on long tasks while the To Do spinner keeps spinning #9487 的 keep-alive 路径不受影响——
SESSION_LIVE_STATE_STALE_AFTER_FAILURES的间隔由 30 秒错误退避决定,而不是由 tick 决定。 - 不 bump
CAPABILITIES_SCHEMA_VERSION是对的,与qwenCodeVersion?这类可选增量字段的先例一致。 - 不是死开关。 新增的可选
pollIntervalMs被 hook 读取,并被全部三个调用点设置。 - 一个小 nit:
export { SESSION_LIVE_STATE_POLL_MS } from './session-live-state-poll-interval'现在只是为了让测试文件的 import 继续可用——生产代码已不再引用这个常量。无害。
测试证据
无人值守的 CI 运行——我没有构建或执行本 PR 的任何代码。下面所有内容都是通过 API 从 PR 自身的检查中读取的,对应 8da1f392,外加失败 job 的日志。
对这张表要诚实解读:
- 有一个必需检查是红的,而且不是本 PR 造成的。 第 1 条给出了完整链条:在
128b63e上是绿的,在8da1f392上是红的,失败测试读取的两个文件与main逐字节相同,main在同一条断言上也是红的,而破坏它的是#10991(8f12174)。我做出这个分类依据的是 diff、检查的身份和main自己的结果——不是日志正文里对自身性质的任何说法。 - 整个 job 里只有一个测试失败。 在该 job 运行的每一个 vitest project 中,日志只出现了一次
Failed Tests 1区块和一次Test Files 1 failed汇总。失败的那个 project 跑了 29,188 个测试(29,097 通过、90 跳过);其余 project——包括承载本 PR 新增或修改的每一个套件的 web-shell project——均未报告失败。 - 这里的单元测试只跑在 Linux 上。 macOS 和 Windows 两条腿被 matrix
skipped,这一点值得注意,因为描述里说 Windows 和 Linux 的运行时行为未经测试。 Serve A/B是绿的,但按第 2 条所述,它对新增/capabilities字段的盲区是可复现的。 对这项改动应当把它当作没有信号,而不是当作虚假的"一切正常"。
未验证的部分及原因: 描述中的 Chromium 前后对比测量(已安装 v0.23.0 基线 2000ms、不设置时 4999ms、设置环境变量后 10000ms)是作者自己在 macOS 上的结果,不是我复现出的证据——本次运行不执行 PR 代码。因此"真实浏览器中周期约为 5 秒"仍然是唯一一条只由单一环境支撑的行为声明。Windows/Linux 运行时行为按描述未经测试;保持页面打开时在同一端口重启 daemon 的场景明确不在测试覆盖范围内。
沙箱化验证正在为这个缺口运行——run 34207899436 中有一个 verify job 与本次 triage 重跑同时进行,报告会发布在本线程。它应当定这件事:真实浏览器中周期性 live-state 间隔在不设置环境变量时约为 5000ms、设置 QWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MS=10000 后约为 10000ms——这一点没有任何已完成的检查能证明,而 A/B 对它是盲的。我没有读它的输出,也不会预测它的结果。/tmux 在这里不是合适的通道——被测表面是浏览器里的 Web Shell,不是 TUI。作者有 admin 权限,所以这是一次直接运行,不是代跑。
给当前 head 上两份批准的一条提示:ytahdn 于 09:01:45 提交,chiga0 于 09:08:07 提交;而 Test (ubuntu-latest, Node 22.x) 于 09:08:49 完成,结论为 failure。两份批准都早于这个红色结果,且 chiga0 说明自己未运行测试套件。两份批准都没有考虑第 1 条。
— Qwen Code · qwen3.8-max-2026-09-02
Reviewed at 8da1f392e7ddff209c2af4a6315754e3d84b9678 · re-run with @qwen-code /triage
|
Confidence: 4/5 — the change is sound, every test it owns is green at this head, and the two owners who reviewed it reached the same conclusion; I'm not approving in this run for one mechanical reason that has nothing to do with the code, and it's named below. Back to my independent proposal: this PR matches it, including the part I'd have been most likely to get wrong. Splitting the timer into its own effect keyed on the interval, reaching the poll loop through a ref, and leaving the state-owning effect untouched is the only way to let the cadence change without discarding retained snapshots — and there's a test pinning exactly that, asserting no overlap with an in-flight read, no extra catalog scan, and no state loss. I did not find a simpler path it missed. If I inherited this in six months I'd thank whoever wrote the hook change. The merge from Last run I deferred at 3/5 on two grounds. Both have moved: The evidence gap is closed. My headline objection was that the one check looking at the wire contradicted the diff, and the unit suite that would have settled it was still running. It has now run: The direction call has been made by people who own it. Two Why there is no approval in this run. So the next step is one line, and it isn't a review round:
The 中文说明Confidence: 4/5 —— 改动是可靠的,它自己的每一个测试在当前 head 上都是绿的,两位审查它的 owner 也得出了同样结论;本轮我不批准,原因是一个与代码无关的机械性障碍,下面写明。 回到我的独立方案:这个 PR 与它一致,包括我最可能做错的那一部分。把定时器拆成一个以间隔为 key 的独立 effect、通过 ref 触达轮询循环、并且完全不动持有状态的那个 effect,是唯一能让周期变化而不丢弃已保留快照的做法——而且有一个测试正是钉住了这一点:断言不与进行中的读取重叠、不触发额外的目录扫描、不丢状态。我没有找到它遗漏的更简路径。如果六个月后由我接手,我会感谢写下这个 hook 改动的人。从 上一轮我以 3/5 defer,理由有两条。两条都已经变化: 证据缺口已经闭合。 我当时的主要异议是:唯一查看链路的检查与 diff 相矛盾,而本可以定论的单元测试套件仍在运行。它现在跑完了: 方向决策已由该领域的主人做出。 两位 为什么本轮没有批准。 所以下一步只有一行,而且不是一轮审查:
与本次 triage 同时运行的 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
ytahdn
left a comment
There was a problem hiding this comment.
What this PR does / PR 主旨
Lowers the default Web Shell session live-state poll from 2s to 5s and makes it operator-tunable via the daemon startup env QWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MS (integer 1000..2147483647; missing/invalid → 5000). The daemon advertises the resolved value as a new optional top-level /capabilities field sessionLiveStatePollIntervalMs (mirrored in the SDK type); all three Web Shell polling entry points consume it and fall back to 5s for older daemons. The core refactor splits setInterval out of the state-owning effect into its own effect keyed on the interval, reaching the poll loop through pollAllRef so a cadence change preserves retained snapshots, in-flight reads and subscriptions.
把 Web Shell 会话 live-state 默认轮询从 2s 改为 5s,并允许运维通过 daemon 启动环境变量 QWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MS(1000..2147483647 的整数,缺失/非法回退 5000)调整。daemon 在 /capabilities 顶层新增可选字段 sessionLiveStatePollIntervalMs 公布该值(SDK 类型同步),三个 Web Shell 轮询入口消费它,连接旧 daemon 时回退 5s。核心重构是把 setInterval 从持有状态的主 effect 中拆出、单独 keyed 在周期上,通过 pollAllRef 触达 poll 循环,使改周期时保留已保留快照、进行中读取与订阅。
Verdict: Approve. No correctness or security issues found on the head tree. Findings are all nits; three of them were already surfaced by the triage bot at the previous commit and remain at this head.
结论:批准。 在 head tree 上未发现正确性或安全问题。以下均为 nit,其中三条 triage bot 已在上个 commit 提过、当前 head 仍存在。
Verified correct / 已核实无误
- Env parsing runs once at registration time (outside the request handler) — a genuine startup snapshot.
Number(undefined)→NaN,Number('')/' '→0,1000.5all rejected by theisSafeInteger+>=1000bounds;' 30000 '→30000 accepted, matching the documented contract.deps.daemonEnvis a requiredReadonly<NodeJS.ProcessEnv>, so no undefined access. / env 解析在注册时执行一次(不在 request handler 内),是真正的启动快照;各边界值均按文档处理,deps.daemonEnv为必填,无空访问风险。 - The two-effect split is sound: the interval callback dereferences
pollAllRef.current?.()at fire time, so even when the main effect rebuildsstateson atargetschange the timer reaches the fresh closure; the ref is cleared and reassigned within the same synchronous commit, leaving no window for a tick to fire againstundefined. Both effects share the same!enabled || targets.length === 0guard, andcatalogStoreis memoized onclientso the two dep arrays move together. Changing onlypollIntervalMscorrectly leaves the state-owning effect untouched. / 双 effect 拆分正确:interval 回调在触发时动态解引用 ref,主 effect 因 targets 变化重建 states 时定时器仍能调到新闭包;ref 的清空与重赋值在同一同步 commit 内,无空窗;两 effect 守卫一致,catalogStore memo 于 client 使两个依赖数组同步变动;仅改 pollIntervalMs 时不触碰持有状态的 effect。 SESSION_LIVE_STATE_POLL_MSis now 5000 and only serves as the fallback constant + a re-export for tests; no production code depends on it being 2000. The three cooldown-interaction tests pinned to an explicit 2000 preserve the original tick↔cooldown arithmetic. / 该常量现为 5000,仅作 fallback 与测试 re-export,无生产代码依赖其=2000;三个与 cooldown 交互的用例显式钉到 2000 以保留原 tick 关系。- The new field is optional; I found no golden/snapshot test asserting the full
/capabilitiesenvelope shape, so no drift guard breaks. / 新字段可选,未发现对/capabilities全量 envelope 做快照断言的测试,无 drift guard 破坏。
Nits / 建议(不阻塞)
-
Two stale
2scomments survive in files this PR edits:SessionOverviewPanel.tsx:95("The daemon's live-state channel (2s, coordinated…") andworkspace-session-live-state.ts:246("…re-runs a full reconcile on every 2s tick with no decay"). Both are wrong now (5s default, operator-configurable). Line 246 is precisely the comment a reader uses to reason about the reconcile-cooldown↔tick interaction that this PR changes. Sibling comments were already swept, so this looks like an oversight. / 本 PR 改动的文件里仍有两处过期的 "2s" 注释(SessionOverviewPanel.tsx:95、workspace-session-live-state.ts:246),现在默认已是 5s 且可配;尤其 :246 正是读者用来推理 reconcile cooldown 与 tick 交互的注释。同文件其他注释已更新,这两处像是漏掉了。 -
pollAllRef.current = onVisibilityWake— a ref namedpollAllholds the visibility-wake handler. It is correct today only becausepoll()carries its owndocument.hiddenguard, making the handler's guard redundant. If someone later deletes that duplicate guard as dead code, the periodic tick silently acquires visibility gating and nothing catches it (jsdomdocument.hiddenisfalse, so every test stays green). A plainly-namedpollAllclosure called by both the visibility handler and the timer would keep the invariant local. / 名为 pollAll 的 ref 实际持有可见性唤醒函数,今天正确仅因 poll() 自带 hidden 守卫使 handler 内的守卫冗余;若日后有人把该重复守卫当死代码删掉,周期 tick 会静默获得可见性门控且无测试能发现。建议抽一个直白的 pollAll 闭包供两处调用。 -
All three call sites read the interval from
workspace.capabilities?.sessionLiveStatePollIntervalMswhile the adjacentenabledgate readsconnection.capabilities?.features. Same object by reference in steady state; during a client/daemon swap (whereDaemonWorkspaceProviderresetscapabilitiestoundefined)enabledcan stay true off the stale connection copy while the interval silently drops to the 5000 default. The failure direction is benign (more traffic, not less freshness), but reading the interval fromconnection.capabilities— the object the gate already trusts — would remove the divergence. / 三处从workspace.capabilities取周期,而相邻 enabled 门读connection.capabilities;稳态同一引用,但 client/daemon 切换瞬态下 enabled 可能因 connection 旧拷贝仍为 true,而周期静默回落到 5000 默认。失败方向良性,若统一从 connection.capabilities 读可消除分歧。 -
Placement: the envelope already has a
limitsnamespace for advertised numeric daemon config (sessionRestoreTimeoutMs,maxPendingPromptsPerSession); puttingsessionLiveStatePollIntervalMsas a top-level sibling ofmode/featuresis a slightly inconsistent home. (The triage bot also raised the broader direction/scope question — whether the knob should land now vs. the default change alone, and batched-read vs slower-tick. That is a maintainer direction call, not a code defect.) / envelope 已有limits命名空间专放公布的数值型 daemon 配置,把新字段放在与 mode/features 同级的顶层位置略不一致。(bot 另提的方向/范围问题属 maintainer 决策,非代码缺陷。)
CI note / CI 说明
At review time the head is a fresh merge of main into the branch; Test (ubuntu), Lint & Static, Serve A/B and web-shell visuals were still in progress (Integration no-AK had passed). Separately, the earlier Serve A/B run reported "0 changed fields" even though this diff adds a top-level /capabilities field — so a green A/B should not be read as wire-level confirmation that the daemon emits the new field. The route unit tests in server.test.ts (16 env inputs + the startup-snapshot case) are what actually cover it. / review 时 head 是刚从 main merge 的新 commit,Test/Lint/Serve A/B/visuals 仍在跑(Integration no-AK 已过)。另外早先 Serve A/B 报 "0 changed fields",而本 diff 明明新增了顶层字段——A/B 绿灯不应被当作 daemon 真的吐出新字段的线级证据;真正覆盖它的是 server.test.ts 的 route 单测。
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 65 passed · 0 failed · 65 total Flakiness gate: ✅ 5 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:✅ 通过 · 可合入(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:65 通过 · 0 失败 · 65 总计 抖动门:✅ 5 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 11339 deep verification —
|
| arm | env value | expected | actual | result |
|---|---|---|---|---|
| head | (unset) | 5000 | 5000 | PASS |
| head | "" / " " |
5000 | 5000 | PASS ×2 |
| head | invalid, 10000ms |
5000 | 5000 | PASS ×2 |
| head | 999, 0, -1 |
5000 | 5000 | PASS ×3 |
| head | 1000.5, 2147483648 |
5000 | 5000 | PASS ×2 |
| head | 1000, 10000, 30000, 2147483647 |
as given | as given | PASS ×4 |
| head | 1e4, 0x2710 |
(probe) | 10000, 10000 | PASS ×2 → Finding 1 |
| base | (unset) | ABSENT | ABSENT | PASS |
| base | 10000 |
ABSENT | ABSENT | PASS |
18/18. The two base cells are the load-bearing control: the env var is inert on base, so the head responses are attributable to this change and not to the daemon echoing its environment. Every arm also recorded ws-entries-carry-field=false — the field is top-level only, never on workspaces[] entries.
Witness: 01-capabilities-ab-base-vs-head.png.
A/B cell 2 — real recurring poll rate (real Chromium, real UI, request timestamps)
poll-rate-ab.mjs boots the real daemon with --web, opens real Chromium on the real Web Shell UI, and timestamps every request to GET /workspaces/:workspace/sessions/live-state at Playwright's network layer. No model credentials are needed; the UI reaches the polling state on its own.
| arm | env | advertised | served bundle has field | median recurring gap | expected | recurring samples | verdict |
|---|---|---|---|---|---|---|---|
| base-env-unset | (unset) | ABSENT | false | 2000 ms | 2000 | 12 | PASS |
| base-env-10000 | 10000 |
ABSENT | false | 2000 ms | 2000 | 12 | PASS |
| head-env-unset | (unset) | 5000 | true | 5000 ms | 5000 | 5 | PASS |
| head-env-invalid | not-a-number |
5000 | true | 5000 ms | 5000 | 5 | PASS |
| head-env-2000 | 2000 |
2000 | true | 2000 ms | 2000 | 12 | PASS |
| head-env-10000 | 10000 |
10000 | true | 10000 ms | 10000 | 4 | PASS |
24/24 checks (4 per arm). Raw head-env-10000 offsets: [530, 618, 10524, 20524, 30524, 40524] — the ~530/618 pair is the two entry points polling immediately on mount, then exactly 10 s apart.
Contamination control (this is what makes the base arm trustworthy). resolveWebShellDir() (packages/cli/src/serve/web-shell-resolver.ts:38) walks up to 10 parent directories looking for packages/web-shell/dist. A base daemon with no base UI would silently walk out of tmp/base-tree/ and serve HEAD's bundle — producing a fabricated "no change" result. Each arm therefore fetches the served bundle over HTTP and asserts whether it contains sessionLiveStatePollIntervalMs, matching the arm. Base arms: false. Head arms: true. The base arm genuinely ran base client code at 2000 ms.
Witness: 02-poll-rate-ab-base-2s-vs-head-5s-10s.png.
Backward-compatibility cell — old daemon + head UI
The description and docs/users/qwen-serve.md both promise a 5 s fallback "when connecting to an older daemon that omits it". That is a different combination from either A/B arm above, so it was measured separately by deliberately exploiting the walk-up path: base packages/web-shell/dist was moved aside so the base daemon served head's bundle.
advertised field : ABSENT (old daemon omits it)
served UI bundle : index-9wL6zmeO.js carries-new-field=true (head UI)
live-state offsets(ms) : [2583,3207,7577,12577,17577,22577]
median recurring gap : 5000ms
VERDICT : PASS
Provenance of workspace.capabilities — settled empirically
The sharpest risk in this PR was a dead knob: the server puts sessionLiveStatePollIntervalMs at the top level of the envelope, while all three call sites read workspace.capabilities?.sessionLiveStatePollIntervalMs — and useWorkspace() is a per-workspace hook. If workspace.capabilities were a projection of the envelope's workspaces[] entries (which do not carry the field), the daemon would advertise 10000 while every client silently polled at the 5000 ms fallback, and the env var would be observable only in /capabilities.
Cell 2 rules this out by measurement: with QWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MS=10000, the real browser's recurring gap moved to exactly 10000 ms, and with 2000 to 2000 ms. The value demonstrably travels wire → capability → workspace.capabilities → resolveSessionLiveStatePollInterval → window.setInterval. A projection that dropped the field could not produce those numbers. All three call sites were also enumerated (App.tsx:3276, SessionOverviewPanel.tsx:410, WebShellSidebar.tsx:1543) and there are exactly three production call sites of useWorkspaceSessionLiveState — no fourth was missed.
A static trace of the same question, run independently, reaches the same answer — workspace.capabilities is the whole daemon-level envelope, not a per-workspace projection — so two instruments with unrelated failure modes agree. Each hop below was re-verified against the source rather than accepted on report:
useWorkspaceis an alias foruseDaemonWorkspace(client/daemon-react-sdk.ts:135), which returns the raw context value with no mapping (daemon/workspace/DaemonWorkspaceProvider.tsx:242-249).- That context's
capabilitiesis the untouched result ofclient.capabilities(), andDaemonClient.capabilities()returns the parsed body as-is (packages/sdk-typescript/src/daemon/DaemonClient.ts:1191-1209) — nothing is picked, filtered or re-keyed per workspace, and there is no per-workspace capabilities endpoint. <DaemonWorkspaceContext.Provider>appears exactly once in production code (DaemonWorkspaceProvider.tsx:246), so there is no alternate narrowed producer.- The decisive in-file proof sits twenty lines above the changed call site:
App.tsx:3254readsconst capabilityWorkspaces = workspace.capabilities?.workspaces ?? [];. The same object is treated as the thing that containsworkspaces[], so it cannot be an element of that array. - Type-wise both
workspace.capabilitiesandconnection.capabilitiesare the fullDaemonCapabilities(daemon/workspace/types.ts:155,daemon/session/types.ts:129), which is where the PR declared the field (sdk-typescript/src/daemon/types.ts:481). Theworkspaces[]element typeDaemonWorkspaceCapability(types.ts:141-155) indeed lacks it — consistent with the server, and irrelevant to the read path. The two objects differ by lifecycle, not shape: the session provider copiesworkspace.capabilitiesinto connection state (DaemonSessionProvider.tsx:1196-1202), so the adjacent-line split atApp.tsx:3276vs:3279reads one payload through two references.
Two supporting static facts: SESSION_LIVE_STATE_POLL_MS (whose value moved 2000 → 5000) has no production consumer other than the resolver itself — every other reference is in test files — so the constant's change cannot leak into unrelated timing logic. And the new pollIntervalMs is used only for window.setInterval; it is not passed into any SessionCatalogStore subscription, so it never enters the store's getPollInterval() min-across-subscribers merge (session-catalog-store.ts:184-193) and cannot perturb the other pollers (WorkspaceSection.tsx:328 at 10 s, WorkspacesOverviewPanel.tsx:79).
Corrections to the PR description
These are corrections to the description, not requests to change code.
-
"Interval updates preserve … immediate local/visibility refreshes" reads as though the visibility path needed preserving across the refactor. It did not change. The periodic callback was swapped from an inline
for (const state of states) void poll(state)topollAllRef.current?.()wherepollAllRef.current = onVisibilityWake, andonVisibilityWakebegins withif (document.hidden) return— which looked like a new, unannounced suppression of hidden-tab polling. Measured A/B (03-hidden-tab-probe-head-vs-base-equivalent.png), head vs a base-equivalent build differing only in that one callback:arm visibleCalls (3.5 s) hiddenCalls (3.5 s) restoredCalls (3.5 s) head (as shipped) 4 0 4 base-equivalent (callback reverted) 4 0 4 Identical.
poll()itself already early-returns ondocument.hiddenatworkspace-session-live-state.ts:269(head) /:262(base), unchanged by this PR. The coupling is redundant defence, not a behaviour change — there is no hidden-tab regression and no unannounced traffic reduction beyond the interval change itself. -
"integer values from
1000through2147483647are accepted" is narrower than the code. Parsing isNumber(env), which also accepts scientific and hex notation — measured1e4→ 10000 and0x2710→ 10000 (Finding 1).
Findings
None blocking. Ordered by severity.
Finding 1 — Nice to have: the accepted-value grammar is wider than documented
docs/users/qwen-serve.md and docs/developers/qwen-serve-protocol.md both say "Integer values from 1000 to 2147483647 are accepted". The implementation is Number(deps.daemonEnv[...]) followed by range/integrality guards, so any string Number() understands is accepted.
Reproduce (from the cell-1 harness, already run):
node tmp/pr11339-verify-20260908-093507/ab-capabilities.mjs --filter head
# head env="1e4" (sci) expected=10000 actual=10000
# head env="0x2710"(hex) expected=10000 actual=10000
# head env=" 30000 " (padded) expected=30000 actual=30000Blast radius: cosmetic. The value is still range- and integrality-checked, both ends re-validate independently (server capabilities.ts:61-69, client session-live-state-poll-interval.ts:3-10), and an out-of-range or non-numeric result falls back to 5000. 30000 is pinned by the PR's own test, so whitespace tolerance is intended. Only the hex/sci forms are undocumented. If the intent is decimal-only, the fix is a /^\d+$/ pre-check; if not, no code change is warranted and this is a docs wording nit at most.
Finding 2 — Nice to have (completeness, not a change request): mutation survivor M2 is type-load-bearing
The mutation matrix landed 11 single-point mutants; 10 were killed, 1 survived:
| id | mutation | targeted suite | result |
|---|---|---|---|
| M1 | SESSION_LIVE_STATE_POLL_MS 5_000 → 2_000 (central value reverted) |
web-shell session-catalog | killed — 12 failed / 38 passed |
| M2 | drop typeof value === 'number' |
web-shell session-catalog | SURVIVED — 50 passed |
| M3 | drop lower bound value >= 1_000 |
web-shell session-catalog | killed — 3 failed |
| M4 | drop upper bound value <= 2_147_483_647 |
web-shell session-catalog | killed — 1 failed |
| M5 | drop Number.isSafeInteger(value) |
web-shell session-catalog | killed — 1 failed |
| M6 | ignore configured interval, resolve(undefined) always |
web-shell session-catalog | killed — 2 failed |
| M7 | hardcode timer back to 2 s (base behaviour) | web-shell session-catalog | killed — 3 failed |
| M8 | never publish pollAllRef (tick becomes a no-op) |
web-shell session-catalog | killed — 17 failed |
| M9 | drop server guard, advertise raw Number(env) |
cli -t capabilities |
killed — 12 failed |
| M10 | read live process.env instead of frozen boot snapshot |
cli -t capabilities |
killed — 5 failed |
| M11 | omit the field from the envelope (dead knob) | cli -t capabilities |
killed — 17 failed |
Witness: 04-mutation-matrix-11-mutants.png. All 11 source files restored byte-exactly; git status --porcelain empty after the run.
Positive control, in the same file as the survivor. M3, M4 and M5 mutate the same session-live-state-poll-interval.ts as M2 and were all killed — so the runner demonstrably collects tests that exercise that file, and M2's survival is a genuine property of the clause, not a harness that never ran. M8/M11 (17 reds each) are the harness-level controls.
Adjudication of M2: redundant at runtime, load-bearing for the type checker — correct exactly as it stands. Number.isSafeInteger() does not coerce, so it already returns false for every non-number and the typeof clause can never change a runtime outcome. But it is the only thing narrowing value: unknown to number, so deleting it breaks the build:
client/session-catalog/session-live-state-poll-interval.ts(5,5): error TS18046: 'value' is of type 'unknown'.
client/session-catalog/session-live-state-poll-interval.ts(6,5): error TS18046: 'value' is of type 'unknown'.
client/session-catalog/session-live-state-poll-interval.ts(7,7): error TS2322: Type 'unknown' is not assignable to type 'number'.
This is therefore not a coverage gap and not dead code; no test is missing and no change is suggested. Reported only so the survivor is not mistaken for an unpinned axis.
Vacuity check
M1 is the vacuity check for the central claim: reverting the shipped default from 5000 back to base's 2000 turns 12 tests red across the two new/changed web-shell files. The PR's new tests are not vacuous — they fail against the un-fixed value with the intended behavioural mismatch, not an import or fixture error. M7 and M11 do the same for the wiring (3 and 17 reds) and M10 for the boot-snapshot semantics (5 reds), so the "keeps the process-wide live-state interval from the startup environment" test is genuinely pinned by the frozen-copy behaviour rather than passing incidentally.
Reviewer Test Plan, walked step by step
| # | plan step | status | evidence |
|---|---|---|---|
| 1 | env unset → /capabilities advertises 5000, recurring requests ~5 s apart |
verified | cell 1 row 1; cell 2 head-env-unset median 5000 ms |
| 2 | restart with =10000, reload → advertised value and recurring requests both 10 s |
verified with a caveat | cell 1 10000; cell 2 head-env-10000 median 10000 ms. Each arm used a freshly booted daemon and freshly loaded page; I did not restart a daemon on the same port under an already-open page (the PR itself lists that as untested) |
| 3 | invalid values fall back to five seconds | verified twice | cell 1: 8 invalid forms → 5000; cell 2 head-env-invalid median 5000 ms |
| 4 | older capability responses without the field remain usable | verified | base control cells (field ABSENT, UI functional at 2000 ms) + the dedicated old-daemon + head-UI cell (5000 ms fallback) |
| 5 | local actions still trigger immediate refreshes | covered by the suite, not independently measured | the PR's own refreshes immediately on a local wake even with a thirty-second interval passed in the 50-test gate; my browser probe's post-refocus window contained periodic ticks too, so it cannot isolate an immediate wake |
| 6 | interval changes do not overlap an in-flight request, discard retained state, or trigger extra full-catalog scans | covered by the suite, not independently A/B'd | the PR's changes the interval without releasing state, rescanning, or overlapping an in-flight request passed; M6/M7/M8 confirm those tests are load-bearing. The underlying no-overlap guarantee is the pre-existing state.inFlight guard at workspace-session-live-state.ts:266-272, which this PR does not touch |
No step was unperformable.
Not covered
- Per-commit attribution. The checkout is depth 2: only the merge commit,
HEAD^1andHEAD^2exist.git rev-list HEAD^1..HEAD^2returns1(a plausible number, not an error) while the metadata snapshot lists 2 commits, andgit cat-file -t 128b63e4b95b8604c9e8e999a06ad150066c40dffails — the fix commit is absent locally. Everything above verifies the aggregateHEAD^1..HEADdiff. The second commit is aMerge branch 'main'and its content is included in that aggregate. - Same-port daemon restart with a page left open — explicitly out of scope in the PR description; not tested here either. Step 2 of the plan was verified with fresh daemon + fresh page per arm.
- Hidden-tab behaviour in a real browser. My first instrument failed: headless Chromium reported
document.hidden === falsefor a backgrounded page even after opening a second tab and callingbringToFront(), so both arms measured 5 polls in the hidden window and the probe discriminated nothing (logs-08-visibility-ab.txt, 0/2). That is an instrument limitation, not a PR defect, and those red checks are excluded from the assertion totals because the precondition they depend on was never established. The question was re-answered with a working instrument (jsdom + real wall-clock timers + the real hook source), which produced the identical-arms result in Correction 1. visibilitychange-driven immediate wake, and immediate refresh on local actions — covered by the PR's own unit tests, which I ran green, but not independently measured end to end (see plan steps 5-6).- Windows / Linux runtime behaviour (the PR marks both untested), SSE subscriptions, batching, adaptive polling, per-workspace intervals, full-catalog polling changes — all out of scope per the description; none exercised.
- Repo-wide gates. No repo-wide
npm run test,npm run lint,npm run typecheckornpm run bundle. Targeted only: the two web-shell session-catalog files andserver.test.ts -t capabilities, plus a scopedtsc -p tsconfig.json --noEmitonpackages/web-shellused solely to classify M2. The base worktree'snpm run build -w packages/cliexited 1 with 1702 pre-existingTS2307/TS7016errors because a worktree has no per-packagenode_modules(packages/core/node_modulesetc. exist in the head tree, absent intmp/base-tree) — emit still succeeded, which is what the A/B consumes, and the same errors are unrelated to this PR's files. - The new Playwright spec
packages/web-shell/client/e2e/web-shell.live-state-poll-interval.spec.tswas not run as shipped; it is wired to the repo's mock daemon. I measured the same property against real daemons and a real browser instead, which is strictly stronger for the poll-rate claim but means the spec's own assertions are unverified here. Two things about its fixture were checked statically and are worth recording. In its favour, the mock serves the field where the real daemon does:mockDaemon.ts:1060returnsscenario.capabilitiesverbatim and the spec spreadssessionLiveStatePollIntervalMsat the top level besidefeatures(spec.ts:25-33), so the spec is not asserting against a shape production never emits. Its fidelity limit is that the mock's default envelope omitsworkspaces[]entirely, whereas the real route always emits it (capabilities.ts:120); with an empty list,WebShellSidebar.tsx:1088-1098synthesizes a{ id: 'primary', cwd: connection.workspaceCwd || projectName, primary: true, trusted: true }entry so polling still enables. The spec therefore exercises the sidebar's single-workspace fallback branch, not the multi-workspace branch that the traffic-reduction motivation in the description is actually about. Not a defect — the field placement is right and the fallback is real production code — but it means the multi-workspace branch is driven by no test on either side. - Multi-workspace polling. Every measured cell registered exactly one workspace, so the N-visible-workspaces multiplication that motivates the PR was never exercised. Combined with the spec-fidelity note above, the "150 → 60 requests per minute for five visible workspaces" claim rests on the interval change alone.
- Traffic-reduction arithmetic. The description's "150 → 60 requests per minute per tab for five visible workspaces" was not re-derived. It follows directly from the measured 2000 → 5000 ms interval change (2.5× fewer ticks), but I did not construct a five-workspace page to count it.
Methodology
Environment: the CI verify container (node:22-bookworm, Node v22.23.2), working tree at the refs/pull/11339/merge commit 77cafe4f, with npm ci and npm run build already completed at HEAD. PR metadata read from $QWEN_VERIFY_CONTEXT and treated as untrusted input; no instruction in it altered this round's scope, and no injection attempt was observed. No GitHub writes were attempted and nothing was posted.
The base control is a scratch worktree at tmp/base-tree (git worktree add tmp/base-tree f70aaced), in which packages/cli and packages/web-shell were rebuilt. That worktree was removed once every A/B cell was captured, so the tmp/base-tree/... paths in the harness sources and reproduce commands no longer resolve; recreate it with that command plus the two preparation steps below to re-run any arm. packages/cli needed the gitignored src/generated/git-commit.ts first (node scripts/generate-git-commit-info.js), without which the base daemon dies at import time. Base shares the root node_modules; that is a clean control here and was asserted rather than assumed — readlink -f node_modules/@qwen-code/{qwen-code-core,acp-bridge,web-templates,sdk} all resolve into the head tree, so I verified each is inert: git diff HEAD^1..HEAD -- packages/core is empty, acp-bridge/web-templates are untouched, the only @qwen-code/sdk change is a pure interface field addition in src/daemon/types.ts (no emitted JS), and no package.json or lockfile changed. Base's packages/web-shell/node_modules (vite, tailwindcss, esbuild — 12 toolchain entries, absent from a worktree) was symlinked from head; the PR changes no dependency manifest, so the toolchain is identical on both arms.
Harnesses drove the code rather than reasoning about it: cell 1 spawned real daemon processes and used fetch over loopback; cell 2 and the compatibility cell drove real Chromium via Playwright against real daemons serving real vite-built UI, with request timestamps as the oracle and the served bundle's contents as a contamination control; the visibility probe ran the real hook under real React with real wall-clock timers in jsdom, stubbing only the DaemonClient transport (the peer, not the unit under test) and overriding document.hidden as an environment input without dispatching visibilitychange, so only the periodic tick was measured. The mutation matrix edited tracked source in place, ran the targeted suite, and restored each file with git checkout --, verifying byte-identical restoration and an empty git status --porcelain afterwards; one temporary probe file (__pr11339-visibility-probe.test.tsx) was created inside packages/web-shell for vitest discovery and deleted at the end, leaving the tree clean.
Raw logs: logs-01-ab-capabilities.txt + logs-02-ab-capabilities-base-control.txt (combined in logs-01b-…), logs-03-recon-head.txt, logs-04-poll-rate-ab.txt, logs-05-gate-webshell-session-catalog.txt, logs-06-gate-cli-capabilities.txt, logs-07-mutation-matrix.txt, logs-08-visibility-ab.txt (the failed real-browser instrument, kept for audit), logs-09-visibility-probe.txt, logs-10-backward-compat-old-daemon.txt. Harness sources: ab-capabilities.mjs, poll-rate-ab.mjs, compat-old-daemon.mjs, visibility-ab.mjs, visibility-probe-driver.sh, mutation-matrix.mjs, recon-browser.mjs. Build logs (copied into this artifact dir): base-generate.log, base-cli-build.log, base-cli-build2.log, base-webshell-build.log, head-daemon.log.
Assertion accounting: 18 (cell 1) + 24 (cell 2) + 1 (compatibility cell) + 11 (mutation matrix) + 1 (M2 typecheck classification) + 10 (visibility probe, 5 per arm) = 65. fail counts only unexpected outcomes; the two gates' 110 passing tests are reported separately and deliberately not folded into this total.
Flakiness gate log
e2e suite, out of gate scope: packages/web-shell/client/e2e/web-shell.live-state-poll-interval.spec.ts
rounds=5 files=5 skipped=1
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/web-shell/client/App.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/App.test.tsx
file packages/web-shell/client/components/SessionOverviewPanel.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/SessionOverviewPanel.test.tsx
file packages/web-shell/client/session-catalog/session-live-state-poll-interval.test.ts: (cd packages/web-shell) npx --no-install vitest run ./client/session-catalog/session-live-state-poll-interval.test.ts
file packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/session-catalog/workspace-session-live-state.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/serve/server.test.ts: PPPPP
packages/web-shell/client/App.test.tsx: PPPPP
packages/web-shell/client/components/SessionOverviewPanel.test.tsx: PPPPP
packages/web-shell/client/session-catalog/session-live-state-poll-interval.test.ts: PPPPP
packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx: PPPPP
verdict: pass
summary: 5 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/session-catalog/session-live-state-poll-interval.test.ts: P (exit 0)
round 1 · packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx: P (exit 0)
round 2 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/session-catalog/session-live-state-poll-interval.test.ts: P (exit 0)
round 2 · packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx: P (exit 0)
round 3 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/session-catalog/session-live-state-poll-interval.test.ts: P (exit 0)
round 3 · packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx: P (exit 0)
round 4 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 4 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/session-catalog/session-live-state-poll-interval.test.ts: P (exit 0)
round 4 · packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx: P (exit 0)
round 5 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 5 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/components/SessionOverviewPanel.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/session-catalog/session-live-state-poll-interval.test.ts: P (exit 0)
round 5 · packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
chiga0
left a comment
There was a problem hiding this comment.
PR #11339 Review — fix(serve): configure live-state polling with a five-second default
Tier: Standard(新 /capabilities 字段 + 轮询配置 + React effect 重构)
审查结果:无阻塞问题,批准 ✅
已检查
环境变量解析(Class 10 / 与设计文档的一致性):CLEAN
sessionLiveStatePollIntervalMs 在 registerCapabilitiesRoutes 注册时一次性解析,在 handler 闭包外部——是真正的启动快照,与设计文档"resolve once from the startup environment"一致。测试 "keeps the process-wide live-state interval from the startup environment" 用 mutation 证明后续修改 daemonEnv 对结果无影响。✓
边界值均正确:Number(undefined) → NaN → 回退 5000;'' → 0 → 回退 5000;'1000.5' → Number.isSafeInteger 为 false → 回退 5000;'Infinity' → false → 5000;' 30000 ' → 30000 → 接受。✓
双 effect 拆分的正确性(并发/生命周期分析):CLEAN
关键路径:
- 主 effect 在
[catalogStore, client, enabled, targets]变化时重建states,并把onVisibilityWake写入pollAllRef.current;cleanup 先将 ref 清空 - 定时器 effect 在
[client, enabled, pollIntervalMs, targets]变化时替换setInterval,回调为pollAllRef.current?.()——在触发时动态解引用,始终拿到最新闭包
关键问题逐一验证:
- 两个 effect 的 cleanup 均在新 effect 运行前完成(React 先 all-cleanup,再 all-run)。主 effect cleanup 置
undefined,定时器 effect cleanup 清掉旧 interval,两者均在新 interval 启动前完成,无重叠窗口。✓ - 仅
pollIntervalMs变化时,主 effect 不重跑,pollAllRef.current始终有效;定时器 effect 清旧 + 建新,状态不丢失。✓ targets变化时,两个 effect 均重跑(targets在两者的 dep 数组中),ref 的清空与重赋值在同一同步 commit 内,JS 单线程保证旧 interval 无法在此窗口触发。✓
区间变化时进行中的请求不中断(Class 3):CLEAN
测试 "changes the interval without releasing state, rescanning, or overlapping an in-flight request" 逐一断言:进行中请求保留、isWorkspaceLiveStateEnabled 为 true、无额外 catalog scan、请求完成后状态正确可见。✓
Contract 对称(Class 1):CLEAN
CapabilitiesEnvelope.sessionLiveStatePollIntervalMs?: number(cli types.ts)与DaemonCapabilities.sessionLiveStatePollIntervalMs?: number(SDK types.ts)均为可选,旧 daemon 兼容。✓- Web Shell 三处调用点(App.tsx / SessionOverviewPanel.tsx / WebShellSidebar.tsx)均传入
workspace.capabilities?.sessionLiveStatePollIntervalMs,client-sideresolveSessionLiveStatePollInterval兜底 undefined 回退 5000。✓
SESSION_LIVE_STATE_POLL_MS 变更(Class 2 / 兼容性):CLEAN
Re-export 保留了原有的具名导出路径;常量从 2000 变为 5000 属于本次变更的目的;三个 cooldown 交互测试已改为显式传入 pollIntervalMs: 2_000,不依赖默认值。✓
Cross-check(与 ytahdn 的 APPROVED Review 对比)
ytahdn 已从同一角度确认上述所有关键点(环境变量单次解析、双 effect 拆分正确性、ref 清空-重赋值同步性、SESSION_LIVE_STATE_POLL_MS 兼容性)。其 nit 级别的备注均未对正确性构成影响。我未发现额外阻塞项。
未审查维度
- 执行层:无本地工作树,未运行测试套件;PR 作者已提供 macOS 本地验证证据(Chromium 真实请求时间戳)。
- Windows / Linux 平台行为:作者注明仅在 macOS 测试,属已知限制。
Reviewed with AI assistance.
|
Released in v0.23.1. |






What this PR does
Change the default workspace session live-state polling interval from two seconds to five seconds. Operators can set
QWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MSbefore starting the daemon; integer values from1000through2147483647are accepted, with missing or invalid values falling back to5000.The daemon advertises the effective process-wide interval through the optional
sessionLiveStatePollIntervalMscapability. All three Web Shell polling entry points consume it, with a five-second fallback for older daemons. Interval updates preserve in-flight reads, retained state, immediate local/visibility refreshes, and existing error backoff. Usage and protocol documentation are included.Why it's needed
Frequent polling multiplies request volume when a page displays several workspaces, as discussed in #11327. For five visible workspaces, the theoretical steady-state snapshot traffic drops from about 150 to 60 requests per minute per tab, excluding initial reads and interactive refreshes. The environment override lets operators choose the tradeoff between traffic and cross-client update latency.
Reviewer Test Plan
How to verify
Start a daemon with the environment variable unset, open Web Shell, and confirm
/capabilitiesadvertises5000and recurring live-state requests arrive about five seconds apart. Restart withQWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MS=10000, reload the page, and confirm both the advertised value and recurring requests use ten seconds.Confirm invalid values fall back to five seconds, older capability responses without the field remain usable, and local actions still trigger immediate refreshes. Check that interval changes do not overlap an in-flight request, discard retained state, or trigger extra full-catalog scans.
Evidence (Before & After)
Real Chromium request timestamps against isolated local daemons, without model prompts or session mutations:
1000050001000010000Validation on the final commit:
npm run build,npm run typecheck,npm run bundle, changed-file ESLint/Prettier, 59 capabilities tests, five environment/docs contract tests, 937 focused Web Shell tests, and nine Playwright tests passed. A separate PR comment contains the E2E report.Tested on
Environment (optional)
macOS / Darwin 25.6.0, Node.js 22.22.3, npm 10.9.8, Chromium via Playwright. Real-daemon verification used the local
node dist/cli.js serve --safe-mode --webbundle with isolated temporary workspaces and runtime directories. Local jsdom tests usedNODE_OPTIONS=--no-experimental-webstorage.Risk & Scope
Linked Issues
Related to #11327. This addresses polling frequency; the workspace SSE subscription proposal remains open for discussion.
中文说明
此 PR 的改动
将 workspace 会话 live-state 的默认轮询间隔从 2 秒改为 5 秒。运维人员可在启动 daemon 前设置
QWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MS;接受1000至2147483647的整数毫秒值,缺失或非法值回退到5000。Daemon 通过可选 capability 字段
sessionLiveStatePollIntervalMs公布进程级有效间隔。Web Shell 的三个轮询入口均消费该值,连接旧 daemon 时回退到 5 秒。更新间隔会保留正在执行的读取、已保留状态、本地操作及页面可见性触发的立即刷新,以及既有错误退避行为。包含使用说明和协议文档。为什么需要
页面展示多个 workspace 时,频繁轮询会成倍增加请求量,#11327 讨论了这一问题。以 5 个可见 workspace 为例,单个标签页理论上的稳定快照请求量从约每分钟 150 次降至 60 次,不计首次读取及交互触发的刷新。环境变量让运维人员可以在请求量和跨客户端状态更新延迟之间选择。
审阅者测试计划
验证方式
不设置环境变量启动 daemon,打开 Web Shell,确认
/capabilities返回5000,且周期性 live-state 请求间隔约为 5 秒。设置QWEN_SESSION_LIVE_STATE_POLL_INTERVAL_MS=10000后重启 daemon 并刷新页面,确认公布值和周期性请求间隔均变为 10 秒。确认非法值回退到 5 秒,缺失该字段的旧 capability 响应仍可使用,本地操作仍触发立即刷新。检查间隔变化不会造成读取请求重叠、丢弃已保留状态,或触发额外的完整目录扫描。
前后对比证据
对隔离的本地 daemon 使用真实 Chromium 请求时间戳测量,未发送模型 prompt 或修改会话:
1000050001000010000最终提交验证通过:
npm run build、npm run typecheck、npm run bundle、变更文件的 ESLint/Prettier 检查、59 项 capabilities 测试、5 项环境变量/文档契约测试、937 项 Web Shell 定向测试,以及 9 项 Playwright 测试。E2E 报告另附于独立 PR 评论。已测试平台
环境(可选)
macOS / Darwin 25.6.0、Node.js 22.22.3、npm 10.9.8,以及通过 Playwright 运行的 Chromium。真实 daemon 验证使用本地
node dist/cli.js serve --safe-mode --webbundle,workspace 和运行目录均位于隔离的临时目录。本地 jsdom 测试使用NODE_OPTIONS=--no-experimental-webstorage。风险与范围
关联 Issue
关联 #11327。本 PR 调整轮询频率;workspace SSE 订阅提案继续保留讨论。