feat(web-shell): Consume live-state session activity timestamps - #9476
Conversation
E2E Test ReportExecuted 2026-08-19 against a real daemon ( Scenario 1 — single turn completion (PASS)Completed a turn in the bottom-most session of a 5-session active list. The row moved to the top within one poll cycle, its stamp refreshed, live-state polling kept flowing, and the daemon Scenario 2 — two back-to-back turns (PASS)Both completions were reflected and the target session moved to the top; the second completion, recorded while the first response could still be in flight, settled on the following poll (in-flight fencing, also pinned by unit tests). Fallback scenarios (unit-pinned)An unusable watermark (old server / no running terminal yet in the daemon generation) and a completed session missing from the loaded active page both fall back to exactly one rate-limited full catalog refresh per 10s window — pinned by the Attribution noteThe run still observed catalog list requests per turn from a pre-existing display-name fallback fetch (URL signature Untouched surfaces (observed)Session creation, auto-naming, first catalog load, archived section, and the pinned block behaved as on |
|
Thanks for the PR! Re-run at Template looks good ✓ Problem: this is not a theoretical concern — it is the planned consumer half of a merged design contract ( Direction: aligned. This completes a protocol the repo itself committed to — the server side shipped in #9396 specifically so this consumer could retire steady-state catalog scans from the turn-completion path. No direct CHANGELOG reference, but the area is squarely within the Web Shell work tracked by those PRs. Size: core module gate not applicable — all 10 changed files are under Approach: the scope matches the contract one-for-one (per-session completion sequences, request-start snapshotting, strictly-fresher watermark application, server-mirroring comparator, preserved version-fenced fallback for every degraded case). The new commit stays strictly within the same scope — it only reworks the settle derivation and adds the missing test pins requested in review, net −22 production lines. The PR also correctly records the pre-existing display-name fallback fetch as out of scope rather than sneaking it in. Risk: no elevated risk signals — none of the changed files match the high-risk path patterns from the revert-history analysis. Moving on to code review. 🔍 中文说明感谢贡献！ 复跑于 模板完整 ✓ 问题：这不是理论性问题——它是已合入设计契约（ 方向：对齐。这是在完成仓库自身已承诺的协议——服务端已随 #9396 合入，正是为了让本消费端把稳态目录扫描从 turn 完成路径中移除。CHANGELOG 无直接条目，但该领域正是上述 PR 所追踪的 Web Shell 工作。 规模：核心模块门禁不适用——全部 10 个改动文件均位于 方案：范围与契约逐条对应（会话级完成序号、请求发起时快照、仅在严格更新时应用水位、镜像服务端比较器、所有降级场景保留版本围栏回退）。新提交严格保持在同一范围内——只重构了 settle 判定推导并补齐评审要求的测试钉住，生产代码净减 22 行。PR 也正确地将既有的显示名兜底请求记录为超出范围，而没有顺手夹带进来。 风险：无升级风险信号——改动文件均未命中 revert 历史分析中的高风险路径模式。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewI reviewed this against the consumption contract in the design doc and the server code it mirrors. No blockers found — the implementation is faithful to the contract and the diff stays strictly inside it. What I verified in detail:
No unrelated changes in the diff; the display-name fallback fetch is untouched as promised. The added comments explain the non-obvious why (sequence fencing, cursor/archived exclusion) in house style. Re-run at
|
| File | What changed |
|---|---|
packages/web-shell/client/App.tsx |
forwards the completed session id to turnCompleted |
packages/web-shell/client/App.test.tsx |
asserts the session id is forwarded |
packages/web-shell/client/components/ChatPane.tsx |
same forwarding for pane-owned streaming completions |
packages/web-shell/client/components/ChatPane.test.tsx |
asserts pane completion and outer-session dedupe carry the id |
packages/web-shell/client/session-catalog/session-catalog-hooks.ts |
turn completions now record session activity when live-state owns the workspace, keeping the legacy rescan otherwise |
packages/web-shell/client/session-catalog/session-catalog-hooks.test.tsx |
pins legacy vs live-state routing of completions |
packages/web-shell/client/session-catalog/session-catalog-store.ts |
watermark application, server-mirroring comparator, pending-sequence lifecycle; applyLiveState now returns the absorbed-watermark set and the separate loaded-session probe is gone |
packages/web-shell/client/session-catalog/session-catalog-store.test.ts |
stamp/reject cases, pinned-block ordering, sequence lifecycle, per-gate absorption coverage, createdAt lower-bound ordering |
packages/web-shell/client/session-catalog/workspace-session-live-state.ts |
snapshots pending completions per request, settles against the absorbed set or flags the fallback |
packages/web-shell/client/session-catalog/workspace-session-live-state.test.tsx |
settle-in-place, in-flight fencing, unloaded-row and ghost-session fallbacks with resolution pins, reconcile and failure recovery paths |
Testing evidence
Unattended CI re-run — nothing was built or executed here; the evidence is the PR's own CI signal for the reviewed head, fetched through the API. CI is fully green on 8bf5769: the main unit suite (Test (ubuntu-latest, Node 22.x)), the web-shell E2E smoke, the visual capture, and both Desktop Shell jobs all pass; macOS/Windows tests and CLI integration tests are skipped by design (merge queue / on demand only).
| Check | Conclusion |
|---|---|
Capture web-shell visuals (ubuntu-latest, Node 22.x) |
✅ success |
Classify PR |
✅ success |
Dependency CVE audit |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Remind on force-push |
✅ success |
Secret scan (TruffleHog) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。
Not independently verified: the central behavioural claim — that a completion causes no additional full-catalog request when the watermark is usable and the row re-sorts within one poll — is pinned by mock-based unit tests here; against a real daemon it rests on the author's Playwright E2E report in this thread (their claim, not re-run evidence, and run against the pre-feedback head — the incremental commit is a semantics-preserving refactor plus tests, so the report still applies, but it was not re-run either). Sandboxed verification would settle this: @qwen-code /verify — an A/B run could prove per-turn /sessions request volume drops to zero on the watermark-usable path versus the base build, which neither static review nor the unit suite can show. (Sponsored run: the author is a fork contributor, so a maintainer's @qwen-code /verify comment approves the head it was written against; that run carries a pre-execution risk screen and a full workspace wipe — read its report with the same skepticism as the fork's own CI logs.)
中文说明
代码审查
我对照设计文档中的消费契约及其镜像的服务端代码做了审查，未发现阻塞问题——实现忠实于契约，diff 严格限定在契约范围内。
重点核验：
- 比较器一致性（PR 自己标注的主要风险）：
compareSessionsByActivity与服务端逐位一致——organized 变体与session-list.ts中的compareOrganizedCursorKeys完全相同（pinned 降序 → 活动时间降序 →sessionId.localeCompare），getSessionActivityTime与getSummaryActivityTime一致（含不可解析时间戳记 0 的规则）。来源/分组过滤页同样覆盖。一个良性差异：无过滤的 legacy 列表端点仅按时间排序、相等时保持归并插入顺序，而客户端使用 id 决胜——只影响毫秒级完全相等的时间戳，且是确定性的，不值得改。 - 契约规则 2（仅运行态）：两处
turnCompleted调用点都只在实际正在 streaming 的会话由非 idle 转为 idle 时触发。 - 序号围栏：待处理集合在请求发出前快照，
resolveSessionActivity按序号守卫，飞行中的旧响应无法确认更新的完成。reconcile 路径刻意不允许确认。两个竞态均有测试钉住。 - 回退保留：水位不可用或行不在已加载页面时置
invalidationRequested，进入既有的版本围栏、10 秒合并的 reconcile——与之前相同的路径与冷却。 - 防护边界：水位仅在严格比行的有效时间（
updatedAt ?? createdAt）更新时应用，且只作用于已加载、无游标、非归档页面上已存在的行；绝不触碰归档行/页，绝不插入未知会话；只有时间戳实际前进才重排。
复跑于 8bf5769 —— 评审反馈核验
新提交处理了全部八条 /review 线程。我逐条对照 diff 与最终代码核验（而非只看回复）：
- R1-4 settle/applyLiveState 漂移（实质性的一条）：从构造上消除。
applyLiveState现在返回在已加载、拥有重排权的页面上吸收了可用水位的会话 id 集合，settle 循环直接消费该集合，不再重新推导索引、解析规则与加载页探测。我读了最终代码：absorbed.add与盖章处于完全相同的门槛之下，且会话在其行被盖章或已至少同样新时原地 settle——语义正确（没有需要补扫的内容）。独立探测函数hasLoadedActiveSession已删除且无任何残留引用，生产代码净减 22 行。两处不再可能漂移。 - R1-1 门槛覆盖：重写后的测试对返回集合逐门钉住——带游标页、跨工作区行、归档行与归档页、未知会话、缺失水位——断言只有活动页的行被吸收。
- R1-2 / createdAt 下限：新测试以判别性探针形态钉住此前无测试的
?? createdAt回退，并覆盖水位下限的两侧。 - R1-3 回退后 resolve 泄漏：两个回退测试均断言待处理条目已被 resolve，未加载用例额外断言一个空闲冷却窗口保持安静——泄漏条目导致的永久重扫变异体现在会失败。
- 幽灵会话分支：新增测试覆盖此前无测试的"完成会话不在 live 响应中"路径；重构还移除了脆弱的可选链本身。
- 延期项（认同）：把比较器上提到
sdk-typescript共享模块作为跨包公共 API 决策延期——合理的范围判断，已与服务端内联漂移一并记为后续候选。
测试证据
无人值守 CI 复跑——未构建或执行任何代码，以上证据为通过 API 获取的该 head 的 PR 自身 CI 信号。8bf5769 的 CI 全绿：主单元测试、web-shell E2E 冒烟、视觉捕获与两个 Desktop Shell 任务均通过；macOS/Windows 测试与 CLI 集成测试按设计跳过。
未独立验证：核心行为性结论——水位可用时完成不产生额外全量目录请求、行在一个轮询周期内重排——目前由基于 mock 的单元测试钉住；对真实守护进程的结论来自作者在本帖中的 Playwright E2E 报告（作者声明，非复跑证据，且针对反馈前的 head——增量提交是语义保持的重构加测试，报告仍然适用，但也未被复跑）。沙箱验证可以定论：@qwen-code /verify —— A/B 运行可以证明水位可用路径上每 turn 的 /sessions 请求量相对基线构建降为零。（赞助运行：作者为 fork 贡献者，需维护者发出 @qwen-code /verify；该运行带执行前风险筛查与完整工作区清空——请以审视 fork CI 日志的同等怀疑态度阅读其报告。）
— Qwen Code · qwen3.8-max
Reviewed at 8bf57693a1f41a9ddbfedfbbee54661fc2f93545 · re-run with @qwen-code /triage
🖼️ 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 |
|
Confidence: 4/5 — contract-faithful and race-tested at the first head, and the follow-up commit did exactly what it should: made the one structural concern (settle/stamp drift) impossible by construction and pinned every previously untested gate. Remaining nits are non-blocking and named below. Stepping back: my first-pass independent proposal for this consumer landed on the same mechanism the PR implements, and my independent proposal for the R1-4 feedback — have the store own the settle answer instead of two co-varying copies — is exactly what What changed my reading since the first pass, concretely: I re-verified the absorbed-set semantics line by line — it fires under exactly the stamp gates, and settling a session whose row is already at least as fresh (no stamp applied) is correct because there is nothing left to fetch. The deferred comparator-sharing item is the right scope call: lifting it into Reservations, named so they don't get lost: (1) the "zero extra catalog requests per turn" claim is proven by mocks in CI and by the author's Playwright E2E (their claim, run against the pre-feedback head); CI is fully green on the reviewed head and a maintainer has already approved it. Verdict: approve, pinned to the reviewed commit. 中文说明置信度：4/5 —— 首版 head 即忠实于契约、竞态均有测试钉住；后续提交做了正该做的事：把唯一的结构性隐患（settle/盖章漂移）从构造上变为不可能，并把此前每个无测试的门槛都钉住。剩余的保留意见均为非阻塞，列在下方。 退一步看：我第一次独立构想的消费端方案与 PR 实现的机制相同；我对 R1-4 反馈的独立方案——让 store 成为 settle 判定的唯一权威，而不是两份必须手工同步的拷贝——正是 相较首轮我判断的变化依据：逐行复核了 absorbed 集合的语义——它与盖章处于完全相同的门槛之下；对行已至少同样新（未盖章）的会话原地 settle 是正确的，因为没有需要补扫的内容。比较器共享的延期项是正确的范围判断：上提到 明确列出不被忽略的保留意见：(1) "每 turn 零额外目录请求"由 CI 中的 mock 与作者的 Playwright E2E 证明（作者声明，针对反馈前的 head）；评审评论已点名 该 head 的 CI 全绿，且已有维护者批准。结论：批准，并与所评审的提交绑定。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
中文说明
已审查。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
中文说明
已审查——无阻断问题。 建议见行内评论。
— qwen3.8-max via Qwen Code /review (v0.21.14)
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewed the core (workspace-session-live-state.ts, session-catalog hooks/store, App.tsx).
Approve. Correct WebShell consumption of the #9396 live-state activity watermark: pending activity is snapshotted before the live-state read, and any session whose activity was not absorbed by the applied live state triggers invalidation + resolve. No obvious blockers.
0 unresolved threads.
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (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: 647 passed · 0 failed · 647 total Flakiness gate: ✅ 5 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：647 通过 · 0 失败 · 647 总计 抖动门：✅ 5 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9476 verification — feat(web-shell): Consume live-state session activity timestampsVerdict: 中文摘要
Central claim + A/BCentral claim: after a turn completes in a live-state-enabled workspace, the completion settles from the next live-state poll's Both arms run the identical harness (
Witnesses:
Witnesses: Reviewer Test Plan walk: step 1 ✔ (cells 2 + wire); step 2 ✔ (cell 3 + PR test + M1); step 3 ✔ (cells 4/5 + M2); step 4 ⚠ see Correction C1 (archived pages never stamped ✔, filtered pages do reorder, never gain rows); step 5 ✔ 621/621. CorrectionsC1 — "group/source-filtered pages never reorder" is inaccurate as written. The Reviewer Test Plan bullet says archived sections and group/source-filtered pages "never gain rows or reorder from live activity". Measured ( FindingsF1 (low) — default-view in-place re-sort diverges from the default list route on activity ties. The client comparator ( F2 (low) — watermark acceptance is Vacuity / mutation matrixSix one-point mutants, each killed by the PR suites and/or the A/B harness — 6/6 KILLED ( Not covered
MethodologyEnvironment: Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/session-catalog/session-catalog-store.ts:150 — [probe] sessionId tie-break direction of the activity comparator is unpinned by tests
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后（第 2 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.14)
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Local end-to-end verification — real daemon, real browser, two-arm A/BI built a real verification environment for this PR and ran it against a live How it was verified
Daemon ground truth first, by direct REST probe across one turn: The catalog revision does not advance across a turn while the watermark does — the premise this PR is built on holds against the real daemon. Results
Ordering agreement was asserted on every scenario by refetching
Across all six configurations I measured, the PR arm never issued more catalog scans than main. It does issue a few more live-state polls (28 vs 24 in scenario 2) — memory-only, no disk touch. Unit tests, RED/GREEN, static checks
Caveat 1 — the win is currently masked in the flows a user actually hitsScenarios 1 and 2 above only isolate this PR's effect because I first populated
That is 6 catalog requests per turn on both arms, and main's own invalidation reconcile gets fully absorbed into the same 10 s cooldown window — so end to end, the two arms are indistinguishable in these flows today. Note the session was auto-named by the daemon ( This is pre-existing behaviour, byte-identical on both arms, and the PR description already records it as out of scope — I am not asking for it here. I am flagging that the follow-up is what actually unlocks the benefit this PR builds, so it is worth landing soon after, and that the "retires steady-state catalog scans from the turn-completion path" framing is true of the mechanism but not yet of the observed request volume. One genuine user-visible improvement is not masked: scenario 6. Another client's activity now reorders the list within one poll with no catalog fetch, where main's list stayed stale (main would still catch up on the observer's own next turn, via the fallback refresh). Caveat 2 — description nit
Cost noteOn this box (20 sessions, fast local disk) a cold catalog scan measured ~1.1 ms p50 against ~0.84 ms for a live-state read — the per-request latency saving is small here. The saving that matters is scan volume, which scales with session count and disk latency, not the numbers from this machine. VerdictCorrect, well-guarded, and strictly non-worse on every axis I measured — no blocking findings. The two caveats above are context for the merge decision, not objections. 中文说明本地端到端验证 —— 真实 daemon + 真实浏览器 + 双臂 A/B我为这个 PR 搭建了真实验证环境，对着运行中的 验证方式
先给 daemon 侧的基准事实（直接 REST 探针，跨越一次完整 turn）： 一次 turn 前后目录 revision 没有推进，而水位推进了 —— 本 PR 依赖的前提在真实 daemon 上成立。 测量结果
每个场景都会重新拉取 （图见英文部分。） 在我测量的全部 6 种配置中，PR 臂的目录扫描次数从未多于 main；代价是多了几次 live-state 轮询（场景 2 中 28 对 24），这些都是纯内存读取，不触碰磁盘。 单测、RED/GREEN 与静态检查
注意事项 1 —— 在用户实际会走的路径上，收益目前被掩盖上面场景 1、2 之所以能隔离出本 PR 的效果，是因为我先通过（附着状态下的）真实重命名把
即两臂都是每 turn 6 个目录请求，而 main 自身的失效 reconcile 被完全吸收进同一个 10 s 冷却窗口 —— 因此在这些路径上，两臂端到端不可区分。注意会话其实已被 daemon 自动命名（目录里能看到 这属于既有行为，两臂逐字节相同，PR 描述也已把它列为范围外 —— 我并不要求在本 PR 处理。我要指出的是：真正解锁本 PR 收益的是那个后续项，值得紧接着落地；同时「把稳态目录扫描从 turn 完成路径中移除」这一表述对机制成立，但对当前可观测的请求量还不成立。 有一处用户可见的改进没有被掩盖：场景 6。其他客户端的活动现在能在一个轮询周期内重排列表且不发起任何目录请求，而 main 的列表保持陈旧（main 仍会在观察者自己下一次 turn 时通过兜底刷新追上）。 注意事项 2 —— 描述小瑕疵
成本补充在这台机器上（20 个会话、本地快速磁盘），冷目录扫描实测 p50 约 1.1 ms，live-state 读取约 0.84 ms —— 单请求延迟收益在此并不显著。真正有意义的是扫描次数的削减，它随会话数量与磁盘延迟增长，而不是这台机器上的绝对数字。 结论在我测量的所有维度上都正确、防护到位、且严格不劣于 main —— 无阻塞性问题。上面两条注意事项是给合并决策的上下文，不构成反对意见。 🤖 Generated with Claude Code — Claude Opus 5 (1M context) |
| : undefined; | ||
| const liveActivityTime = | ||
| liveUpdatedAt !== undefined ? Date.parse(liveUpdatedAt) : Number.NaN; | ||
| if (Number.isFinite(liveActivityTime)) absorbed.add(session.sessionId); |
There was a problem hiding this comment.
absorbed.add() fires when the live watermark merely parses, not when it actually advances the row, so a completion whose watermark did not move settles with no stamp change and no fallback rescan.
The daemon advances the watermark only for a terminal of a prompt that reached running (packages/acp-bridge/src/bridge.ts:2038 const mutateTurnState = pendingEntry.state === 'running';). Queue a prompt behind a running turn and cancel it, or drop the stream mid-turn: App.tsx:7247 fires turnCompleted on any non-idle to idle transition, the live row still carries the PREVIOUS turn's stamp, Number.isFinite(liveActivityTime) is true so the id enters absorbed, line 542 then rejects it as not fresher, and the settle loop therefore never sets invalidationRequested and deletes the pending record — while the transcript mtime the server would have returned did move. Measured A/B with a fake daemon whose live updatedAt is older than the row's: base issues 1 extra catalog read for that completion, PR head issues 0. Note the fix is not simply moving the add inside the > branch — a mutation test shows that passes all 77 tests but would fire a spurious reconcile whenever the daemon reports an equal-or-older watermark for a row the page already holds fresher data for; the two cases ("already at least as fresh" vs "no running terminal happened") need to be distinguished.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| // The catalog revision doesn't advance on turn completion; record | ||
| // the completion so the live-state loop can settle it from the | ||
| // response's updatedAt watermark instead of a full catalog scan. | ||
| store.recordSessionActivity(workspaceCwd, sessionId); |
There was a problem hiding this comment.
Turn completions were the only steady-state catalog reader once live-state is on, so retiring the rescan freezes every row whose recency the live overlay does not carry.
With live-state enabled the store refuses all background/poll catalog work (session-catalog-store.ts:822 if (this.isWorkspaceLiveStateEnabled(entry.query.workspaceCwd)) return;) and the sidebar disables its own autoLoad/pollIntervalMs (WebShellSidebar.tsx:995, :1304); every remaining invalidateWorkspace producer is a user action (create/rename/archive/delete), and no persistent subscriber sets maxAgeMs. A persisted-only row, or a row advanced by a second qwen process on the same workspace, has no live entry so applyLiveState never touches its updatedAt, and the bridge revision does not move (bridge.ts:2512 is reached only from membership/metadata changes). Measured A/B: 30 completions over a minute produce 6 catalog rescans on base (the other row's timestamp converges to its new value) and 0 on head (it stays frozen at the pre-advance value) for as long as the sidebar is open.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| // reorder-owning page (stamped or already at least as fresh). The settle | ||
| // loop consumes this instead of re-deriving the index and acceptance | ||
| // rules, so the two sites cannot drift apart. | ||
| const absorbed = new Set<string>(); |
There was a problem hiding this comment.
absorbed is a workspace-wide union across every catalog entry, so one page absorbing a watermark suppresses the fallback rescan a different page still needed.
The loop at 509-512 filters only on workspaceCwd and snapshot.page — it includes zero-subscriber entries still inside the 30s SESSION_CATALOG_RETENTION_MS window, and refreshActiveSessionDisplayName mints exactly such an entry on the same turn completion (App.tsx:5251-5259, options: { pageSize: 200 }, no cursor, no archiveState, so ownsActivityReorder is true). Probe: two loaded reorder-owning entries, session s3 present on entry B (wider filter) but not on entry A (the server clamps a page to MAX_SESSION_PAGE_SIZE = 100, session-list.ts:29); applyLiveState returns absorbed = ['s3'] from B while A's page is untouched, so workspace-session-live-state.ts:303 never sets invalidationRequested and A keeps a page 1 that omits the workspace's most recently active session. The verdict should be per-entry (or an intersection over entries that actually hold the row), not a union.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| return Number.isFinite(time) ? time : 0; | ||
| } | ||
|
|
||
| function compareSessionsByActivity( |
There was a problem hiding this comment.
The mirrored comparator adds a sessionId tie-break and a 0 floor that the server's default (non-organized, non-metadata) list path does not have, so rows reorder client-side and snap back on the next reconcile.
The server's default branch is session-list.ts:1227-1231 — return bTime - aTime; with no id tie-break (relying on sort stability) and no Number.isFinite guard (a NaN comparator result is treated as 0, leaving the row in place). That branch is reached whenever view and sourceType are both absent: WorkspaceSection.tsx:215-229 and WebShellSidebar.tsx:994-1004 build exactly that query when organizationEnabled is false and no source filter is selected, and App.tsx:5256 always does. Probe on the head store: a page the server returned as [zzz, aaa, trigger] (zzz and aaa share an activity millisecond) becomes [aaa, zzz, trigger] after any activity advance, and WorkspaceSection.tsx:628 renders sessionsResult.sessions directly, so the two rows visibly swap and swap back on the next reconcile. Same for an unparsable stamp, which the client sinks to the bottom and the server leaves in place.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| } | ||
| const byTime = getSessionActivityTime(b) - getSessionActivityTime(a); | ||
| if (byTime !== 0) return byTime; | ||
| return a.sessionId.localeCompare(b.sessionId); |
There was a problem hiding this comment.
localeCompare() with no explicit locale makes the tie-break depend on the viewer's browser collation, so the client can order equal-activity rows opposite to the daemon.
Measured: 'aa1'.localeCompare('ab1','en') === -1 but 'aa1'.localeCompare('ab1','da') === 1 (Danish/Norwegian collate aa after z); case order flips too ('0a'.localeCompare('0A') is -1 in en, +1 in da). Session ids are lowercase hex UUIDs, so aa…/ab… prefixes occur. On a Danish- or Norwegian-locale browser two equal-activity rows render in the opposite order from the page the daemon returned (the daemon runs the same call under Node's own default locale) and they swap on every activity advance / reconcile cycle. Passing an explicit locale — or comparing with </> — on both sides removes the dependence.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| const updatedAt = | ||
| liveUpdatedAt !== undefined && | ||
| Number.isFinite(liveActivityTime) && | ||
| liveActivityTime > getSessionActivityTime(session) |
There was a problem hiding this comment.
The client's live-vs-persisted freshness rule is hand-written and differs from laterActivityTimestamp, the module whose docstring claims every recency surface shares it.
packages/cli/src/serve/server/activity-timestamp.ts:21 compares the live watermark against the persisted updatedAt only and prefers live on a tie (liveTime >= persistedTime ? live : persisted); the client compares against updatedAt ?? createdAt with a strict >. For a row whose persisted updatedAt is absent and whose createdAt parses later than the bridge watermark (daemon/recorder clock skew, or a summary written by an older daemon), mergeLiveSessionSummary (session-list.ts:453) gives that row the live watermark while applyLiveState rejects it and keeps createdAt — and line 538 still counts it as absorbed, so no fallback rescan runs either. The same session then reports two different recencies depending on whether you read the REST list or the Web Shell sidebar, which is exactly what activity-timestamp.ts:17-19 says the shared helper exists to prevent.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| this.liveStatePendingActivity.set(workspaceCwd, pending); | ||
| } | ||
| pending.set(sessionId, ++this.liveStateActivitySequence); | ||
| for (const handler of this.liveStateWakeHandlers) handler(workspaceCwd); |
There was a problem hiding this comment.
recordSessionActivity wakes every poll loop on every turn completion with no dedupe and no minimum interval; the invalidation path it replaced deliberately did neither.
requestLiveStateRefresh both self-dedupes (if (this.liveStateWorkspaceRefreshRequests.get(workspaceCwd) === 'interactive') return;) and wakes only for 'interactive', so the old 'invalidated' turn-completion path added zero off-cadence requests. Measured A/B: one turnCompleted produces 0 immediate getWorkspaceSessionLiveState calls on base and 1 on head. The only brakes in poll() are state.inFlight, liveRetryAt (set on error only) and document.hidden, so in a workspace where several sessions settle turns in a burst the 2s cadence degrades to roughly one request per completion. The wake buys at most 2s of reorder latency; a lastPollStartedAt + SESSION_LIVE_STATE_POLL_MS floor would coalesce the burst.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| live.sessions, | ||
| ); | ||
| state.liveRetryAt = 0; | ||
| if (pendingActivity) { |
There was a problem hiding this comment.
reconcile() discards applyLiveState's new return at both of its call sites and never settles pending activity, so a completion recorded during a reconcile can trigger a duplicate full reconcile on the very next tick.
A completion recorded while a reconcile is running has its wake dropped by the state.inFlight guard (lines 248-255), so it survives the reconcile — even though readLiveState at line 216 was issued after the record and line 233 already stamped the row post-commit. Probe: the two absorbed sets returned inside the reconcile both contain the completed session, both are thrown away, and snapshotSessionActivity still holds the record afterwards; when that session is off-page the next tick runs a second full stage+commit (the first reconcile was interactive so invalidationReconcileAt is still -Infinity and the cooldown at 326-336 does not stop it). The correct fix mirrors poll(): snapshot the pending set BEFORE line 216 and settle only that snapshot, since a completion recorded after that request went out is not in liveB.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| reportCatalogTurnCompletion | ||
| ) { | ||
| sessionCatalogController.turnCompleted(catalogOwnerCwd); | ||
| sessionCatalogController.turnCompleted( |
There was a problem hiding this comment.
In split view ChatPane and App can both report the same completion, and the second record bumps the sequence so the in-flight settle no-ops, costing a full extra poll cycle.
SplitView.tsx:488-491 sets reportCatalogTurnCompletion={sessionId !== currentSessionId || paneWorkspaceCwd !== connection.workspaceCwd}, and paneWorkspaceCwd = workspaceCwdById.get(sessionId) (SplitView.tsx:420) is undefined until the session list resolves, so the second disjunct is true even for a pane showing the current session. React runs the child effect first: ChatPane records seq 1, the wake starts a poll that snapshots {S:1}, then App's effect records seq 2, so resolveSessionActivity(cwd, S, 1) hits if (pending?.get(sessionId) !== sequence) return; and no-ops. Probe: a single record leaves snapshotSessionActivity('/work') === undefined after the wake poll, a double record leaves Map(1) { 'session-a' => 2 } — the completion waits another SESSION_LIVE_STATE_POLL_MS, and if the row is off-page it raises invalidationRequested twice, scheduling a duplicate full reconcile.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| ).toBeUndefined(); | ||
| }); | ||
|
|
||
| it('settles a turn completion from the live watermark without a catalog scan', async () => { |
There was a problem hiding this comment.
No end-to-end test exercises the new watermark path at all, and the request-count E2E the design doc assigns to this PR was not written.
packages/web-shell/client/e2e/utils/mockDaemon.ts:941-947 returns live rows with only sessionId/clientCount/hasActivePrompt/isWaitingForPermission/isWaitingForUserQuestion and never updatedAt, and this PR does not touch it — so in e2e/web-shell.session-live-state.spec.ts absorbed is always empty and every completion takes the full-rescan fallback; that spec also never drives a turn, so reverting this entire PR leaves it green. (The handler's upstream filter (session.clientCount ?? 0) > 0 || session.hasActivePrompt === true || … also excludes idle rows, so adding updatedAt alone is not enough.) docs/design/2026-08-18-workspace-session-live-state-updated-at.md:604 assigns the coverage explicitly: "The later Web Shell PR adds request-count E2E coverage proving repeated turns on an already-loaded row issue no additional full catalog requests."
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| return session; | ||
| } | ||
| changed = true; | ||
| if (session.updatedAt !== updatedAt) activityAdvanced = true; |
There was a problem hiding this comment.
The "only reorder when the effective value changes" gate and the absorbed-set rule are both unpinned — mutating either keeps the whole suite green.
Mutation testing against the 77 passing tests: replacing if (session.updatedAt !== updatedAt) activityAdvanced = true; with an unconditional activityAdvanced = true; — 77/77 still pass; replacing ...(updatedAt !== undefined ? { updatedAt } : {}) (line 562) with a bare updatedAt, — 77/77 still pass; and narrowing absorbed.add (line 538) to the accepted-stamp branch — 77/77 still pass. Control mutations do fail (deleting sessions.sort fails 3, dropping session.updatedAt === updatedAt from the equality check fails 3), so the suite is otherwise live. Line 555 encodes design-doc rule "should only reorder when the effective value changes" (2026-08-18-workspace-session-live-state-updated-at.md:474): if a refactor breaks it, every 2s poll where clientCount/hasActivePrompt jitters re-sorts the whole page with the client comparator, the sidebar list churns continuously, and nothing goes red.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| const schedule = vi.spyOn(store, 'scheduleWorkspaceRefresh'); | ||
|
|
||
| // Without live-state ownership the legacy rescan path stays. | ||
| act(() => controller!.turnCompleted('/w', 'sess-1')); |
There was a problem hiding this comment.
The new hooks test leaks a real 2-second timer and three unrestored spies, breaking the teardown convention its sibling suites follow.
This file has no vi.useFakeTimers(), no vi.restoreAllMocks() and no store.dispose(); afterEach (lines 80-83) only unmounts the root and removes the container. The new case's first half takes the non-live-state branch and really calls store.scheduleWorkspaceRefresh('/w'), whose internal setTimeout(…, SESSION_CATALOG_TRAILING_REFRESH_MS = 2000) is never cleared, so it lands during later cases and calls invalidateWorkspace('/w') on a store nobody disposed; the three vi.spyOn(store, …) handles are never restored. The sibling workspace-session-live-state.test.tsx afterEach does both vi.useRealTimers() and vi.restoreAllMocks(). Isolation currently survives only because each test builds a fresh client (the store is cached per client) — coincidence, not containment.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| // stamp (e.g. a staged page committed fresher data mid-flight) must | ||
| // not regress the row. | ||
| const liveUpdatedAt = | ||
| acceptsActivity && session.isArchived !== true |
There was a problem hiding this comment.
Two guards in the new code are unreachable, and the tests that "cover" one of them fabricate a response the daemon cannot produce.
On an entry where acceptsActivity is true (archiveState !== 'archived') session.isArchived can never be true: the server filters the active list by archiveState and mergeLiveSessionSummary forces isArchived: false, and nothing in packages/web-shell/client ever writes the field. session-catalog-store.test.ts:601-606 and :788 cover it by placing { isArchived: true } rows inside an archiveState: 'active' page, so the guard's entire safety signal rests on invented server behavior. Line 540's liveUpdatedAt !== undefined && is dead the same way — when liveUpdatedAt is undefined liveActivityTime is Number.NaN (536-537) so Number.isFinite is already false; deleting it leaves tsc --noEmit clean and 77/77 tests green, and Number.isFinite(liveActivityTime) is then evaluated twice (538 and 541).
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| // the live-state overlay carries no updatedAt — flag a | ||
| // rate-limited reconcile so activity stamps keep refreshing. | ||
| store.requestWorkspaceLiveStateInvalidation(workspaceCwd); | ||
| // The catalog revision doesn't advance on turn completion; record |
There was a problem hiding this comment.
This PR is the "follow-up consumer" the design doc describes, but the doc still states the consumer is unimplemented.
docs/design/2026-08-18-workspace-session-live-state-updated-at.md still reads "This document defines the server and TypeScript SDK contract only. Web Shell consumption is a separate implementation change." (lines 5-7), heads section 447 as "Follow-up Web Shell Consumption Contract" in the future tense, says at 519 "It intentionally excludes all Web Shell changes.", and at 640 "That metric changes only after the follow-up consumer ships." A reader cannot tell which of the seven numbered rules landed and which were relaxed — rule 2 ("records … only for a turn known to have reached the running state", which this client does not distinguish) and rule 5 ("using the server comparator", which diverges on the default path) both shipped with caveats.
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
| ); | ||
| } | ||
|
|
||
| // Mirrors the server's getSummaryActivityTime: activity is updatedAt with a |
There was a problem hiding this comment.
The server's activity-ordering rule is hand-copied into the client with only a comment binding the two, and no test compares the two implementations.
getSessionActivityTime duplicates getSummaryActivityTime (session-list.ts:582) and compareSessionsByActivity duplicates compareOrganizedCursorKeys (:627) and compareLiveSessionCursorKeys (:596) — the same functions that decide the server's pagination cursor boundaries — with no shared module and no mirror test, so any future server ordering change silently desyncs the client and surfaces as rows that reorder on poll and reorder back on reconcile, months later, with no link to the server commit that caused it. Both packages already depend on @qwen-code/sdk, where DaemonSessionLiveState.updatedAt is already declared, so one exported comparator plus one shared activity-stamp accessor would remove the drift surface (the repo already has both a types-sync.ts and a *-sdk-mirror.test.ts precedent for this).
Reviewed with Claude Code (claude-opus-5, 1M context) — /code-review 9476 --comment.
|
Released in v0.21.15. |
4 participants
What this PR does
After a normal turn completes, the Web Shell now refreshes that session's recency and re-sorts the loaded active list directly from the activity watermark already carried by the existing two-second live-state poll, instead of flagging a rate-limited full catalog rescan. Each completion is recorded with a per-session sequence, and only a live-state response whose request started after the completion was recorded may settle it, so a stale in-flight response can never mask a newer completion. A watermark is applied only when strictly fresher, only to rows already present on loaded, cursor-less, non-archived pages (unknown sessions are never inserted and archived pages are never touched), and the page is re-sorted with the same ordering the server uses (pinned block first in organized views, then activity, with a stable id tie-break). Whenever the watermark is unusable (old server, or no prompt has reached the running state in the current daemon generation) or the completed session is not on a loaded active page, the previous version-fenced, rate-limited full-catalog fallback still runs with its 10-second coalescing. The two-second polling cadence, the live→catalog→live version handshake, initial load, and the create/archive/delete/rename paths are unchanged, and workspaces without live-state keep the legacy invalidate-and-refresh behavior.
Why it's needed
The server side of the live-state activity protocol shipped in #9396: every live session row can now carry a daemon-observed activity timestamp. The Web Shell, however, still ran the interim behavior from #9366, where every turn completion scheduled a rate-limited full session-catalog rescan just to refresh ordering timestamps — a disk-backed directory scan per cooldown window for data the client already polls every two seconds. This PR implements the consumer side exactly as specified by the follow-up consumption contract in docs/design/2026-08-18-workspace-session-live-state-updated-at.md, retiring steady-state catalog scans from the turn-completion path while keeping every degraded scenario on the previous conservative behavior.
Reviewer Test Plan
How to verify
cd packages/web-shell && npx vitest run client/session-catalog client/components/ChatPane.test.tsx client/App.test.tsxEvidence (Before & After)
Before: every turn completion scheduled a rate-limited full
/sessionsrescan to refresh ordering. After: completions settle from the live-state response in place (E2E against a real daemon: completed bottom-most session moved to the top within one poll, daemon catalog version unchanged throughout, confirming recency no longer rides the version-fenced rescan). Screenshots and the full E2E report follow in a separate comment.Tested on
Environment (optional)
Unit tests via vitest (Node 22). E2E via a locally built daemon (
npm run build,node packages/cli/dist/index.js serve) driving the built Web Shell with Playwright.Risk & Scope
Linked Issues
Consumer follow-up to #9396 (server-side activity watermark) and #9366 (interim Web Shell live-state consumption).
中文说明
本 PR 做了什么
普通 turn 完成后，Web Shell 现在直接使用既有 2 秒 live-state 轮询响应中携带的活动水位来刷新该会话的时间戳并原地重排已加载的活动列表，而不再触发限频的全量目录重扫。每次完成会以会话级序号记录，只有在记录之后才发起的 live-state 请求的响应才可以确认该次完成，因此飞行中的旧响应永远不会掩盖更新的完成。水位仅在严格更新时应用，且只应用于已加载、无分页游标、非归档页面中已存在的行（绝不插入未知会话、绝不触碰归档页面），随后按服务端一致的排序规则（organized 视图 pinned 块优先，其后按活动时间，最后按稳定 id 决胜）重排该页。当水位不可用（旧服务端，或当前守护进程代际内尚无 prompt 进入运行态）或完成的会话不在已加载活动页时，仍走此前的版本围栏、10 秒限频全量目录回退。2 秒轮询节奏、live→catalog→live 版本握手、首次加载以及创建/归档/删除/重命名路径均保持不变；未启用 live-state 的工作区继续沿用旧的失效加刷新路径。
为什么需要
live-state 活动协议的服务端已随 #9396 合入：每个 live 会话行可携带守护进程观测到的活动时间戳。但 Web Shell 仍运行 #9366 的过渡行为：每次 turn 完成都会安排一次限频的全量会话目录重扫，仅为刷新排序时间戳——即每个冷却窗口一次磁盘目录扫描，而这些数据客户端本来每两秒就在轮询。本 PR 严格按照 docs/design/2026-08-18-workspace-session-live-state-updated-at.md 中的后续消费契约实现消费端，把稳态目录扫描从 turn 完成路径中移除，同时让所有降级场景保持此前的保守行为。
评审验证计划
如何验证
cd packages/web-shell && npx vitest run client/session-catalog client/components/ChatPane.test.tsx client/App.test.tsx证据（前后对比）
之前：每次 turn 完成都会安排一次限频全量
/sessions重扫来刷新排序。之后：完成从 live-state 响应原地确认（对真实守护进程的 E2E：位于底部的会话完成后在一个轮询周期内升至顶部，全程守护进程目录版本号不变，证明时间戳刷新不再依赖版本围栏重扫）。截图与完整 E2E 报告见后续评论。已测试平台
macOS ✅；Windows⚠️ ；Linux ⚠️ 。
环境（可选）
单元测试使用 vitest（Node 22）。E2E 使用本地构建的守护进程（
npm run build后node packages/cli/dist/index.js serve）配合 Playwright 驱动构建产物中的 Web Shell。风险与范围
关联 Issue
作为 #9396（服务端活动水位）与 #9366（Web Shell 过渡消费）的消费端后续。