fix(web-shell): show session active work - #11267
Conversation
|
Thanks for the PR! Template looks good ✓ — every required heading is present, including the Chinese translation and a genuinely filled-in Risk & Scope rather than a stub. Problem: observed, not theoretical. #11119 is an open Direction: aligned, and the reference signal is unusually direct. The Claude Code CHANGELOG carries "Fixed the terminal progress indicator (iTerm2, Ghostty, ConEmu) showing the session as finished while a background workflow or agent was still running" — the same failure class — plus "[VSCode] Added a hollow ring in the session list for sessions open in a terminal, another VS Code window, or Claude Desktop, so they no longer look closed", which is the same move of giving a session row a distinct visual state for a liveness condition that would otherwise read as closed. Size: Stage 0 applies via the cross-package clause — the diff spans four packages ( Approach: before reading the diff I'd proposed something smaller — one Two things I'd still push on:
Risk: the Stage 1e high-risk path scan matched nothing, so no elevated-risk signal from revert history. There is a different dependency worth naming though: this PR's Flagging these for discussion before diving deeper. 中文说明感谢贡献！ 模板完整 ✓ —— 所有必需小标题都在，包含中文说明，Risk & Scope 也是认真填写的，不是占位。 问题： 是已观测到的问题，不是理论性加固。#11119 是一个仍处于 open 的 方向： 对齐，而且参考信号非常直接。Claude Code 的 CHANGELOG 里有 "Fixed the terminal progress indicator (iTerm2, Ghostty, ConEmu) showing the session as finished while a background workflow or agent was still running"——同一类故障；还有 "[VSCode] Added a hollow ring in the session list for sessions open in a terminal, another VS Code window, or Claude Desktop, so they no longer look closed"——同样是给 session 行加一个独立视觉状态，用来表达否则会被读成"已关闭"的 liveness 情况。 规模： Stage 0 因跨包改动而适用——diff 横跨四个包（ 方案： 在读 diff 之前我提出的是更小的方案——在 仍有两点我想提出来：
风险： Stage 1e 高风险路径扫描没有命中，因此从 revert 历史角度看没有升级风险信号。但有另一个依赖值得点名：本 PR 的 先提出来讨论，再深入看代码。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewI traced the new field hop by hop before judging the logic, because an optional field added to a wire contract fails silently — a missed projection just means the UI never sees it, with no error anywhere. The plumbing is complete:
No correctness blocker, no security issue, and existing tests are unaffected (with the field absent every new branch is inert). Everything below is a Suggestion. 1. The merge order is load-bearing, not incidental. 2. No tests for the derivation. Zero test lines for a four-state function whose whole value is the 3. 4. Third copy of the coverage predicate. 5. Small UI and a11y nits. The archive-disabled tooltip still reads sequenceDiagram
participant P1 as Child runtime Session
participant P2 as ACP bridge
participant P3 as serve routes
participant P4 as Web Shell catalog store
participant P5 as Sidebar row
P1->>P2: hold snapshot (categories, timestamp)
P2->>P2: entryActiveWorkState - local work, freshness, category coverage
P3->>P2: listWorkspaceSessions and live-state
P2-->>P3: BridgeSessionSummary with activeWorkState
P3-->>P4: wire row, field optional
P4->>P4: applyLiveState merge plus snapshot equality check
P4-->>P5: catalog row
P5->>P5: green when hasActivePrompt, blue dot plus spinner when active, marker when unknown
Files changed (13)
Test evidenceThis is an unattended CI run, so per the gate's rules I did not build, run or execute anything from this PR. The evidence below is the PR's own CI, read through the API for commit The important thing is not what is red — nothing is — it is what is absent. The author's own statement that no local test, build or typecheck was run is quoted from the description; it is the author's claim, not evidence I reproduced.
Not verified, and why: typecheck, lint and unit tests — no lane covers this base and none was run locally, so a compile error in the four-package contract change would currently be invisible. The four-state derivation — no test asserts any branch. The rendered sidebar indicators — the visual diff lane is filtered out, and no before/after capture was supplied ("No local screenshot was captured"). Sandboxed verification would settle this: 中文说明代码审查在评判逻辑之前，我先逐跳追踪了这个新字段，因为加在 wire 契约上的可选字段是静默失败的——漏掉一处投影，只会让 UI 永远看不到它，任何地方都不会报错。整条链路是完整的：
没有正确性阻塞项，没有安全问题，现有测试也不受影响（字段缺失时所有新分支都是惰性的）。以下都是 Suggestion。 1. 合并顺序是关键性的，不是附带细节。 2. 派生逻辑没有测试。 一个四态函数，其全部价值就在 3. 4. 覆盖度判断出现了第三份拷贝。 5. UI 与无障碍小问题。 行内只有后台 work 时，archive 禁用的 tooltip 仍然显示 测试证据这是一次无人值守的 CI 运行，因此按 gate 规则我没有对本 PR 做任何 build、运行或执行。以下证据来自 PR 自己的 CI，通过 API 读取 commit 关键不在于哪些是红的——没有红的——而在于哪些根本不存在。 作者关于未运行本地测试、build 或 typecheck 的说明，是引自 PR 描述；那是作者的陈述，不是我复现出的证据。 未验证的部分及原因：typecheck、lint 与单元测试——没有 lane 覆盖这个 base，本地也没跑，所以四包契约改动里若存在编译错误，目前是不可见的。四态派生——没有任何测试断言其中任一分支。侧栏指示器的实际渲染——视觉 diff lane 被过滤掉，也没有提供前后对比截图（"未采集本地截图"）。 沙箱化验证可以定论： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — the code is coherent and I could trace every hop, but nothing has compiled or tested it, and I am not willing to attest to a green result that does not exist. Stepping back: my independent proposal was a single boolean, and the PR's four-state contract is better than mine on this base. Once capability negotiation, hold categories and a freshness window already exist, "no fresh report" and "fresh empty report" really are different facts, and flattening them would reproduce the silent-idle failure that #11119 is about. The What keeps this at 3 rather than 4 is verification, not design. Two facts sit next to each other and neither is the PR's fault individually:
So a change that touches a synchronized wire contract across four packages, adds a state machine to a 14,867-line bridge and adds render branches to a 6,384-line sidebar component, currently has zero compile signal and zero test signal behind it. Second reservation, narrower: the derivation is entirely untested, and its only interesting behaviour is the Third, the merge order is load-bearing. The goal/cron path resolves to None of this is a reason to rework the change, which is why this is a defer and not a request for changes. Two ways to unblock, either sufficient:
I tried to resolve an accountable owner to hand this to and could not: 中文说明Confidence: 3/5 —— 代码是自洽的，每一跳我都能追通，但没有任何东西编译过或测试过它，而我不愿意为一个并不存在的绿色结果背书。 退一步看：我独立提出的方案是单个布尔值，而本 PR 的四态契约在这个 base 上优于我的方案。一旦能力协商、hold 类别和 freshness window 已经存在，"没有新鲜上报"与"新鲜且为空的上报"就确实是两个不同的事实，把它们压平会重现 #11119 所说的那种静默 idle 故障。"只有在新鲜、全覆盖且为空的 snapshot 下才发 让它停在 3 分而不是 4 分的是验证，不是设计。两个事实并排摆着，单独看都不是本 PR 的错：
于是一个横跨四个包、修改了需保持同步的 wire 契约、给 14867 行的 bridge 加了状态机、又给 6384 行的 sidebar 组件加了渲染分支的改动，背后目前既没有编译信号也没有测试信号。这里 第二点保留意见更窄：派生逻辑完全没有测试，而它唯一有意思的行为就是 第三点，合并顺序是关键性的。goal/cron 路径之所以能解析成 这些都不构成重做这份改动的理由，所以这是 defer，而不是 request changes。两种解法，任一即可：
我尝试解析出一个可交接的责任人但没能成功： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
…e-state # Conflicts: # packages/web-shell/client/components/sidebar/WebShellSidebar.tsx
🖼️ 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 |
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
activeWork |
true |
false |
activeWorkReporting |
"partial" |
"full" |
activeWorkStaleMs |
0 |
9 |
— Qwen Code · serve A/B
Local verification against a real daemon — PR #11267 @
|
Resolve packages/acp-bridge/src/bridge.ts: main extracted hasInFlightPromptActivity() (promptActive || goalTurnActive || a parked deferred restore prompt) where this branch still had the inline two-term expression, so take main's helper and keep this branch's adjacent activeWorkState field. The helper is a strict superset of the expression it replaces. Also fix the Test job's typecheck failure: DaemonSessionLiveState gained `activeWorkState`, so the public-surface pin in packages/sdk-typescript/test/unit/daemon-public-surface.test.ts failed with TS2344. Add the optional union to the expected literal so it matches the interface field for field. Verified locally: no conflict markers, git diff --check clean, prettier --check clean on both edited files, and the PR's per-file footprint is byte-for-byte the same before and after the merge (bridge.ts +20/-0, and identical numstat for the seven other files both sides touched). Tests not run: no dependency tree on this machine matches this lockfile. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtrcr08gpe
|
Thanks for the real-daemon verification. I rechecked each item against the current head.
|
|
Resolved the review against the current head
Verification for this follow-up: |
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
门禁 Review — head de6b2ab89a
决定：REQUEST_CHANGES，一条。 daemon 侧我全看了、也认可；卡住的是客户端这一半整块没有任何断言覆盖，而它改的是用户看得见的行为。
已经核实没问题的部分（写清楚，免得下一轮重看）
- 状态推导是 fail-closed 的：
entryActiveWorkState（bridge.ts:3167）顺序是 本地 work 或新鲜 hold →active；没协商能力 →unsupported；childWorkIsUnknown或类别不全 →unknown；只剩全类别 + 新鲜快照才落到idle。childHoldsAreFresh（:3114）把childHoldsAt === null直接判为不新鲜，注释也写清了"旧快照不是'空闲'的证据，它是'没有报告'"——这正是这类 live-state 最容易做错的地方。entryHasActiveWork（:3162）把 unknown 读成 busy，健康面不会因为沉默而放行回收，理由注释也在。 - 注册竞态是对的：
:7053用 channel 上保留的 snapshot 给"报告先于 newSession 返回"的会话补childHolds/childHoldsAt，且reportedChildHolds === undefined时留 null/null（→ unknown，而不是 idle）。applies a snapshot received before session registration把这条路跑通了 ✔。 - 四个状态在
bridge.test.ts都有断言（unknown/idle/active/unsupported）✔；wire 两处投影都是!== undefined才带字段 ✔ 旧 daemon 的响应形状不变；liveSessionSnapshotsEqual把新字段纳进比较（否则 UI 根本不会重渲染，这一步很容易漏）✔；useDaemonActivePromptBridge保留成 wrapper、新useDaemonSessionActivityBridge另出口，hasActivePrompt语义完全没动 ✔。 - 归档提示的措辞（en/zh 一起改）跟上了
running的新含义，hasLiveActivity = true的默认也保证了未接线调用方行为不变 ✔。
要补的：客户端新行为零覆盖
git grep 过 sessionWorkActive、activityUnknown、hasLiveActivity、data-web-shell-session-active-work 这四个符号，在 packages/web-shell 下只命中生产代码，没有任何测试引用。同时 App.test.tsx 这次只是把 mock 从 useDaemonActivePromptBridge 换成 useDaemonSessionActivityBridge，并且固定返回 activeWorkState: undefined——也就是说现有用例一条都不会经过新分支。于是这几件事全部处于"改了也没人会红"的状态：
- 三态指示器：
activeWorkState === 'active'时那个蓝点、'unknown'时带aria-label的?、'unsupported'/缺字段时什么都不显示，以及绿点仍只代表前台 prompt。 running = hasActivePrompt || sessionWorkActive（WebShellSidebar.tsx:4377，并被:4709的disabled={busy || isCurrent || running}消费）：这一行让有后台工作的会话把那个动作变成"可见但不可点"，也就是改变了归档/删除等操作的可用性。这是本 PR 里唯一一处行为收紧，最需要一条用例钉住它的边界（active让动作 inert；unknown不让它 inert）。todo.status === 'in_progress' && hasLiveActivity：TodoPanel的转圈从此依赖 live state，需要一条"会话工作结算后不再转圈"的用例，否则这个修复本身可以随时被改回去。
三条都是现有渲染套件里的 render 断言，加起来不到一两个小时；activeWorkState 只要让现有 mock 返回不同值就能进分支。补上我就签——daemon 侧不需要再动。
CI 事实。 head 上 Integration Tests (no-AK) 11m17s 绿，Test (ubuntu-latest)、Lint & Static、Serve A/B、Capture web-shell visuals、review-pr 提交时仍在跑，没有任何红；页面 REVIEW_REQUIRED，还没有任何 review。顺带一句：0 条 thread 说明这是第一次正式被 review，不是我推翻谁的结论。
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Review — fix(web-shell): show session active work
No blocking findings. Approved.
Scope
Reviewed source only. 21 changed files, +252/-41 lines.
NOT reviewed: docs/design markdown (design doc, informational only) · local build/typecheck/runtime (working tree unavailable).
What was checked
Contract completeness (Class 1 — writer/reader symmetry)
New activeWorkState field traced end-to-end:
- Declared in
BridgeSessionSummary(bridgeTypes.ts:817),DaemonSessionSummary(sdk-types:1374),DaemonSessionLiveState(sdk-types:1601). - Written unconditionally by
entryActiveWorkState()in bridge.ts; the return type isNonNullable<…>so every live session summary carries the field. - Forwarded in all three server projection paths:
routes/session.ts,standalone-session-service.tsmergeLiveStandaloneSummary,dispatch.tsACP list. - Read by
session-catalog-store.tsequality check and merge, surfaced throughuseSessionActivePromptState/useDaemonSessionActivityBridgein hooks. - Consumed in WebShellSidebar (archive guard, running indicator, spinner, poll interval), TodoPanel (
hasLiveActivity), App.tsx (hasLiveActivityprop computation). - No missing end found.
Race condition fix (Class 7/8 — snapshot arriving before session registration)
The hasSessionSpawnInFlight() guard in BridgeClient is correctly scoped:
() =>
channelInfo?.sessionIds === sessionIds &&
channelInfo.sessionSpawnsInFlight > 0- Channel-scoped: checks the closure-captured
sessionIdsset against the livechannelInfo.sessionIdsreference and the per-channelsessionSpawnsInFlightcounter. - Counter lifecycle: incremented at bridge.ts:5449 (before
newSession), decremented withMath.max(0, …-1)at bridge.ts:5826 (unconditionally after spawn attempt). No leak path visible. - Snapshot storage:
applyActiveWorkSnapshotstores the full reported map ininfo.activeWork.snapshot.createSessionEntryreadschildSnapshot?.sessions.get(sessionId)— a keyed lookup — so concurrent spawns pick up only their own holds by ID. - Stale-snapshot safety:
childHoldsAtis set tochildSnapshot.receivedAt, so the existing freshness gate (childHoldsAreFresh) applies. A snapshot older thanintervalMs * ACTIVE_WORK_STALE_INTERVALSat registration time still produces'unknown', not'active'. - New test at bridge.test.ts:+486 specifically exercises the race and verifies the correct
'active'→'idle'transition.
State machine correctness (Class 10 — stated intent vs. code)
entryActiveWorkState() four-state mapping verified against PR description:
active: local work (pendingPromptCount > 0orpendingAgentNotificationCount > 0) OR child-reported held work in a fresh snapshot.unsupported: channel has noactiveWorkcapability (non-negotiating child/old daemon).unknown: capability present but snapshot is stale, OR negotiated categories don't coverACTIVE_WORK_HOLD_CATEGORIES.idle: all guards passed; only emitted from a fresh, fully-covered, empty snapshot.
Consistent with the PR's stated invariant: "prove idle only from a fresh fully covered empty snapshot."
Backward compatibility (Class 2 — API contract)
- All three type additions use
?(optional). Older daemons that omit the field remain readable. useDaemonActivePromptBridgepreserved as a thin wrapper overuseDaemonSessionActivityBridge. Callers that weren't updated continue to work.TodoPanelhasLiveActivitydefaults totrue, preserving spinner behavior for all existing callers.- SDK public-surface test (daemon-public-surface.test.ts) updated to include the new field.
Archive guard behavior (Class 10)
session.activeWorkState !== 'active' — the guard intentionally does NOT block archival for 'unknown' or 'unsupported'. This matches the design: active is positive knowledge; unknown is degraded coverage, not confirmed activity. Correct by design.
Polling interval
hasRunningSession now includes activeWorkState === 'active' alongside hasActivePrompt. This keeps the 2-second poll cadence while background work is known active. Correct.
Unreviewed dimensions
- Working tree unavailable → no local build, typecheck, or runtime execution. Execution-tier findings (test validity by mutation, rung 3 runtime behavior) not performed. No open finding requires runtime evidence to settle.
- docs/design/web-shell/session-active-work-live-state.md not reviewed (design doc; no code).
Reviewed with AI assistance.
|
@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: 118 passed · 0 failed · 118 total Flakiness gate: ✅ 4 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：118 通过 · 0 失败 · 118 总计 抖动门：✅ 4 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #11267 deep verification —
|
| arm | bridge.js |
bridgeClient.js |
represents |
|---|---|---|---|
| V0 | base f68d65e0… |
base 692bf5fb… |
HEAD^1 — neither hunk |
| V1 | head 485e73b9… |
base 692bf5fb… |
filter relaxation reverted |
| V2 | seed-reverted a679208e… |
head 2307b630… |
registration seeding reverted |
| V3 | head 485e73b9… |
head 2307b630… |
PR head |
V3's bridge.js and bridgeClient.js were asserted byte-identical to the CI's own head build, so V3 is the shipped artifact and not a re-build that could have drifted. Exactly three .js files differ base↔head (bridge.js, bridge.test.js, bridgeClient.js) and the .js file set is identical; bridgeTypes.js is byte-identical, which confirms the two type-only changes (BridgeSessionSummary, DaemonSessionSummary/DaemonSessionLiveState) emit nothing at runtime.
Every cell was produced by a real child process speaking NDJSON JSON-RPC over real stdio pipes, spawned by the production createSpawnChannelFactory via QWEN_CLI_ENTRY. The peer is a real ACP agent built on the shipped @agentclientprotocol/sdk AgentSideConnection; nothing in the bridge or BridgeClient is stubbed. Snapshots after registration are pushed over a real loopback TCP control socket.
Table 1 — activeWorkState by arm
Witness: 01-ab-matrix-four-arms.png. ABSENT = the optional wire field is not present.
| scenario / observation | V0 base | V1 client-reverted | V2 seed-reverted | V3 head |
|---|---|---|---|---|
| S0 held row (child hold, no prompt) | ABSENT | active |
active |
active |
| S0 free row (no hold) | ABSENT | idle |
idle |
idle |
S0 held hasActivePrompt |
false |
false |
false |
false |
| S0 rows distinguishable per session | false |
true |
true |
true |
S0 bridge-wide activeWork (unattributed) |
true |
true |
true |
true |
| S2 fresh empty snapshot, full coverage | ABSENT | idle |
idle |
idle |
| S3 negotiated but never reported | ABSENT | unknown |
unknown |
unknown |
| S4 legacy categories only | ABSENT | unknown |
unknown |
unknown |
| S5 categories absent | ABSENT | unknown |
unknown |
unknown |
| S6 no negotiation at all | ABSENT | unsupported |
unsupported |
unsupported |
| S7 daemon-owned work, during prompt | ABSENT | active |
active |
active |
| S7 daemon-owned work, after settle | ABSENT | unsupported |
unsupported |
unsupported |
| S8 RACE at registration (0.7 s spawn) | ABSENT | unknown |
unknown |
active |
| S9 RACE with an 8 s spawn | ABSENT | unknown |
unknown |
active |
| S9 RACE with a 13 s spawn | TIMEOUT-10s | — | — | TIMEOUT-10s |
| S10 1st session (no row named it) | ABSENT | unknown |
unknown |
unknown |
| S10 2nd session reusing that id | ABSENT | unknown |
unknown |
active |
| S11 proven idle → prompt → settle | ABSENT | idle→active→idle |
same | idle→active→idle |
Three readings of this table carry the verdict:
- The headline cell is S0. On base, two sessions on one channel — one held by a background agent, one genuinely free — both report
hasActivePrompt: false. Base does know something is busy (bridge.activeWork: true), but that flag is bridge-wide and cannot say which session;distinguishableisfalse. Head reportsactiveandidleon the two rows. That is exactly the gap the design doc names, and it is the only cell in the table where base and head differ on a fact base already possessed. hasActivePromptis unchanged, as claimed:falsein S0/S1/S2/S8/S10 on every arm, andtrueonly in S7/S11 where a real foreground prompt is in flight. No cell turned background work into a foreground turn.- The race fix needs both hunks. Reverting the
BridgeClientfilter alone (V1) or thecreateSessionEntryseeding alone (V2) each leaves S8/S9 atunknown; only V3 reachesactive. This holds at a 0.7 s spawn and at an 8 s spawn.
The freshness threshold, bisected through the real code path
The seeding writes childHoldsAt = childSnapshot.receivedAt, so a pre-registration report could in principle go stale before the session registers. I bisected that threshold rather than sampling it:
- The freshness window is
intervalMs × ACTIVE_WORK_STALE_INTERVALS=intervalMs × 3. The daemon clamps a child's advertised cadence into[5 s, 60 s](clampActiveWorkIntervalMs), so the window's floor is 15 s and its production default is 45 s (15 s × 3). - Driving a 13 s / 15 s / 17 s
session/newproducedBridgeTimeoutError: newSession timed out after 10000ms— identically on V0 and V3. That is an A/A control: the failure is a pre-existing property of the bridge (DEFAULT_INIT_TIMEOUT_MS, present at base), not a regression. - Because 10 s < 15 s ≤ window, a slow spawn cannot stale the retained snapshot under default configuration. The 8 s cell (the slowest spawn the default timeout permits, with margin) still reports
activeon V3.
This consequence does not hold if an operator raises initializeTimeoutMs above 15 s; that configuration was not exercised.
Corrections to the PR description
These are corrections to the description, not requests to change the code.
- Reviewer Test Plan step 5 is inaccurate. It asks the reviewer to "use an older or non-negotiating child and confirm the state is
unsupported". Those are two different outcomes. A child that never negotiates reportsunsupported(S6, measured). But a child that negotiates active-work v1 while covering only the legacy categories reportsunknown(S4, measured), and one that negotiates v1 withcategoriesabsent also reportsunknown(S5, measured). The realacpAgentproduces exactly that second shape: it answers a daemon that sends nocategoriesarray withACTIVE_WORK_LEGACY_HOLD_CATEGORIES(['agent','notification']), and the daemon intersects what comes back with its own five-category set. Only a non-negotiating child yieldsunsupported. - "a blue indicator and spinner" — the spinner is not blue. The new active-work dot is blue as described (
--agent-blue-500,.sessionStatusDot), and the foreground-turn dot stays green (.sessionStatusDotRunning→--success-color). But the spinner it also renders is.sessionLoading, whose border is--muted-foreground(grey). That is pre-existing styling reused correctly; only the description overstates the colour.
Findings
Ordered by severity. None is a Critical; none blocks the central claim.
F1 — Widening hasRunningSession silently raises catalog polling 15× for long-running background work (Suggestion)
hasRunningSession now counts session.activeWorkState === 'active'. Its consumer is not cosmetic — it selects the session-catalog poll cadence:
projectExpanded || hasRunningSession || selectedSessionSource === 'channel'
? (hasRunningSession || selectedSessionSource === 'channel') && !error
? ACTIVE_SESSION_POLL_INTERVAL_MS // 2000
: IDLE_SESSION_POLL_INTERVAL_MS // 30_000
A workspace that previously polled every 30 s now polls every 2 s — 15× — for as long as any session reports background work. That is precisely the case this feature exists to surface (a background Agent, shell, workflow, or cron turn can run for many minutes), whereas hasActivePrompt was true only for the short duration of a foreground turn. Each browser tab with the sidebar open multiplies it.
Reproduce: grep -n "hasRunningSession" packages/web-shell/client/components/sidebar/WebShellSidebar.tsx (definition ~2325, consumer ~2366) and read the two constants at lines 159–160.
Bound — where it does not apply. The poll path is gated on sessionCatalogRequestsEnabled && !primaryWorkspaceSessionLiveStateEnabled, so a workspace using the live-state subscription is unaffected. It is reachable on the capability-absent / older-daemon path, where the catalog list itself now carries activeWorkState through the ACP projection. Suggested handling: name the cadence change in Risk & Scope, or gate the poll interval on hasActivePrompt alone while letting the indicator use the wider predicate.
F2 — Widening running suppresses the row's hover action reveal (Suggestion)
running is now Boolean(session.hasActivePrompt || sessionWorkActive), and running && styles.runningSession is applied to the row. .runningSession has no standalone rule; it appears only inside :not() selectors:
.sessionRow:hover:not(.runningSession) .sessionActions,
.sessionRow:focus-within:not(.runningSession) .sessionActions { opacity: 1 }
with .sessionActions { opacity: 0 } by default. So a session with background work no longer reveals its row actions on mouse hover or on focus-within, where before this PR it did.
Bound — the a11y reading is worse than the reality. .sessionActions:has(.sessionActionButton:focus-visible) still sets opacity: 1, so a keyboard user tabbing onto a button does reveal the actions; and opacity: 0 keeps the buttons focusable and clickable rather than removing them. The regression is a mouse-hover affordance, not a lost capability. It also sits oddly beside canArchiveNonCurrent, which deliberately keeps the archive action visible-but-inert for active work — the row now hides the button that the PR elsewhere took care to keep visible.
Reproduce: mutate running back to Boolean(session.hasActivePrompt) and compare the row's class list for a session with activeWorkState: 'active', hasActivePrompt: false.
F3 — The retained snapshot is keyed by session id, not by the spawn that caused it (Suggestion)
The BridgeClient filter now retains every row of a snapshot while any spawn is in flight (ownsSession(id) || hasSessionSpawnInFlight()), and applyActiveWorkSnapshot stores the whole map in info.activeWork.snapshot, which createSessionEntry later reads by id. The retention therefore outlives the spawn that justified it.
Measured (S10): a child emits exactly one snapshot — corroborated at the destination by its own stderr, emit seq=1 [{"sessionId":"FOREIGN",…}] — naming an id it does not own, while spawn #1 is in flight; spawn #1 yields id first, and spawn #2 then yields FOREIGN and sends nothing. On V3 the second session reports active at registration and activeWorkCoverage.covered goes 0→1, inheriting a hold reported before that session existed. V0/V1/V2 all report unknown.
This is not academic: doSpawn documents that "a caller-supplied id can legitimately reuse an id after an abandoned restore settles", so id reuse is a supported scenario.
Bound — what this is not.
- Cross-channel contamination is disproved by the static chain, not merely unevidenced.
info.activeWork.snapshotlives onChannelInfo;applyActiveWorkSnapshotskips entries whereentry.channel !== info.channel;createSessionEntryreads only its ownci.activeWork.snapshot; and a bridge holds a single live channel (ensureChannelreturns the existingchannelInfounless it is dying). The invariant the deleted comment named — "a channel must never influence another channel's state" — still holds. It was not exercised at runtime because two simultaneously live channels are not constructible through the public API; see Not covered. - No new authority is granted. The child is already the authority on its own holds and can make any session it owns read
activesimply by reporting a hold after registration. - The failure direction is safe. A stale retained row yields
activeorunknown, never a falseidle.
Suggested handling if the author wants it narrower: drop the retained row once the spawn that caused it settles, or stamp it with the in-flight spawn and ignore it otherwise.
F4 — Coverage census (completeness reporting, not a merge condition)
Witness: 02-mutation-matrix-vacuity.png. 12 mutations, each an exact string replacement that hard-fails if its target text is absent, so a mutation that silently did not apply can never be misreported as a survivor. The tree was asserted git status --porcelain-clean after every restore (12/12).
| mutation | what it removes | suite | result | classification |
|---|---|---|---|---|
| M3 | unsupported → idle (fails open) |
bridge.test | red, 1 failed / 913 passed | positive control |
| M1 | the PR's BridgeClient hunk |
bridge.test | red, 1 failed / 913 passed | pinned |
| M2 | the PR's createSessionEntry hunk |
bridge.test | red, 1 failed / 913 passed | pinned |
| M5 | the entryHasLocalWork term |
bridge.test | red | pinned |
| M6 | the info.activeWork.snapshot store |
bridge.test | red | pinned |
| WC | revert the new i18n archive string | sidebar-removal | red | positive control, same suite as W1/W2/W5 |
| M4 | the category-coverage unknown clause |
bridge.test | green, 914 passed | coverage gap |
| W1 | the blue active-work dot branch | sidebar-removal | green, 132 passed | coverage gap |
| W2 | the activeWorkState !== 'active' archive guard |
sidebar-removal | green | coverage gap |
| W5 | both tooltip branches | sidebar-removal | green | coverage gap |
| W3 | the TodoPanel hasLiveActivity gate |
TodoPanel | green, 3 passed | coverage gap |
| W4 | live-state → catalog-row propagation | hooks + sidebar | green | coverage gap |
The new test is not vacuous, and the reds fail the intended assertion. M1 and M2 each turn exactly one test red — createAcpSessionBridge > active work > applies a snapshot received before session registration — with AssertionError: expected 'unknown' to be 'active' // Object.is equality. That names expected versus actual, so the revert reached the assertion rather than breaking an import or a fixture. The positive control M3 goes red on a different test (grades a child that never acknowledges the capability as none, expected 'idle' to be 'unsupported'), which proves the harness can make this suite fail and that each mutant is caught by its own test rather than by collateral damage. Every green survivor still collected tests (914 / 132 / 3), so none is an empty run.
Adjudication of the survivors. All six are ordinary coverage gaps — the behaviour is right, nothing asserts it. None is dead code and none is redundant defence:
- M4 is the sharpest. Deleting the category-coverage clause leaves all 914 bridge tests green, yet that clause is the entire mechanism behind S4/S5 — it is what makes a legacy or pre-category child report
unknowninstead of a confidentidle. The fix's own new tests pin the four reported shapes; the version-skew sibling is unpinned. Given F1's correction shows this shape is reachable in production, a fixture advertisingcategories: ['agent','notification']with a fresh empty snapshot and assertingunknownwould close it. - W1/W2/W5/W4: the whole Web Shell rendering half of this PR is unpinned. A census makes this concrete rather than inferred — repo-wide, only three test files mention
activeWorkStateat all (daemon-public-surface.test.ts, a type-level pin;bridge.test.ts, the real behavioural tests; andApp.test.tsx), andApp.test.tsxmentions it exactly once, asactiveWorkState: undefinedinside themockUseDaemonSessionActivityBridgemock. Its 790 tests therefore cannot observe any active-work behaviour by construction. The positive control WC goes red in the same suite the sidebar mutants survive in, so the suite is live and collecting — it simply asserts nothing here. - The three CLI wire projections (
acp-http/dispatch.ts,routes/session.ts,conversations/standalone-session-service.ts) are unpinned by census: zero test files underpackages/clireferenceactiveWorkState; the only three files in that package which mention it are the three production projections the PR added.
A missing test for changed behaviour is a Suggestion, not a Critical, so none of the above blocks. But the unpinned axis is exactly where the next regression will land: mutating any of the six guards today is indistinguishable from not mutating it.
Not covered
- The real HTTP wire. The three CLI projections are pass-through spreads verified by census and by reading, not by a live daemon request. I did not boot
qwen serveandGET /workspaces/:ws/sessions/live-state; that needs a running runtime and auth this job does not have. The claim "the field reaches the wire" is therefore argued from the projection sites plus the SDK type pin, not observed on a socket. This reproduces the derivation end to end, not the transport. - Cross-channel contamination was not exercised at runtime. Disproved by static chain only (per-
ChannelInfomap, theentry.channel !== info.channelguard, one live channel per bridge). A bridge holds a single livechannelInfo, so two simultaneously live channels are not constructible through the public API; forcing one would have meant driving a dying channel, which is a different question. - No browser rendering. The blue dot, the
?marker, the spinner and the hover-affordance change (F2) were verified from the CSS module and the class logic, not from a painted frame.verify-capture.mjsgives the captured command no TTY, so it cannot rasterise a React UI; theterminal-capturepipeline needs a browser this container does not have. - Per-commit attribution. The checkout is depth 2, so
git rev-list HEAD^1..HEAD^2returns 1 commit while the metadata snapshot lists 11.git rev-parse --is-shallow-repositoryistrue, so per-commit claims were treated as unreachable and the aggregateHEAD^1..HEADdiff was verified instead. The 11 commits include two merge-from-main resolutions; I verified the merge added nothing topackages/acp-bridgeover the PR head (git diff --name-only HEAD^2..HEAD -- packages/acp-bridgeis empty) but did not separately exercise the conflict-resolution commit's claims aboutbridge.tsfootprint. initializeTimeoutMsabove 15 s. The staleness bound in the threshold section depends on the 10 s default being below the 15 s window floor. An operator raising that timeout reopens the question; not exercised.unstable_resumeSessionand the restore path. I confirmed the spawn-in-flight counter is incremented indoSpawn, the single spawn path shared by new sessions and caller-supplied-id restores, so the sibling is covered structurally. I did not drive an actualsession/loadrace.- Repo-wide gates.
npm run typecheckwas run in full and passes (exit 0). Lint was not run. Only the seven affected test files were run, not the full per-package suites; the PR's own CI covers the rest.
Methodology
Environment: the CI verify container (node:22-bookworm), Node v22.23.2, working tree at the merge commit 8936e40 with npm ci and npm run build already completed at head. The base control is a scratch worktree at HEAD^1 under tmp/base-tree; running npm run build inside its packages/acp-bridge exits 1 on packages/core type errors (unresolved @opentelemetry/*, fdir, mime/lite, ignore and ajv typings as seen from the nested worktree) but still emits acp-bridge's own JS. Every one of those errors is in packages/core, a package this PR does not touch, and the head build of the same tree had already succeeded in CI — so I verified the emit rather than assuming it: the emitted .js file set is identical to head's, exactly three files differ, and bridge.js contains zero occurrences of entryActiveWorkState. The two revert arms were compiled in a second scratch worktree at HEAD. Internal workspace links were checked with readlink -f rather than require.resolve (these packages are ESM-only with import-only exports, so require.resolve throws ERR_PACKAGE_PATH_NOT_EXPORTED): node_modules/@qwen-code/qwen-code-core resolves into the head tree, which is a clean control because packages/core is untouched by this PR and sdk-typescript's change is type-only — confirmed by bridgeTypes.js being byte-identical base↔head.
Each cell was driven by a real child process over real stdio pipes through the production spawn path, with post-registration snapshots pushed over a real loopback TCP socket; the child's own stderr was captured as a destination-side corroborator of how many snapshots it actually emitted (used to attribute S10). Harnesses are .mjs files in this directory so a maintainer can rerun them: fake-acp-child.mjs (the peer), harness.mjs (one arm, ARM_DIR=…), evaluate.mjs (the matrix and its 75 assertions), mutate.mjs (the 12 mutations), matrix.mjs, aggregate.mjs (writes assertions.json). Raw per-arm output is raw-V{0,1,2,3}.jsonl, raw2-*.jsonl, s9-*.jsonl; per-cell stderr is raw-*.stderr.log; suite logs are gates.log and mutout-*.log. Targeted gates: bridge.test.ts 914 passed, bridgeClient.test.ts 130, daemon-public-surface.test.ts 21, TodoPanel.test.tsx 3, session-catalog-hooks.test.tsx 15, WebShellSidebar.workspace-removal.test.tsx 132, App.test.tsx 790 — 2005 tests, all exit 0 — plus npm run typecheck exit 0.
Flakiness gate log
rounds=5 files=4 skipped=0
file packages/acp-bridge/src/bridge.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridge.test.ts
file packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/daemon-public-surface.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/sidebar/WebShellSidebar.workspace-removal.test.tsx: (cd packages/web-shell) npx --no-install vitest run ./client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/acp-bridge/src/bridge.test.ts: PPPPP
packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: PPPPP
packages/web-shell/client/App.test.tsx: PPPPP
packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: PPPPP
verdict: pass
summary: 4 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 1 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 1 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 2 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 2 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 2 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 2 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 3 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 3 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 3 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 3 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 4 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 4 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 4 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 4 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.test.tsx: P (exit 0)
round 5 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 5 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 5 · packages/web-shell/client/App.test.tsx: P (exit 0)
round 5 · packages/web-shell/client/components/sidebar/WebShellSidebar.workspace-removal.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
About the error in this PR's visual previewThe red error row in the preview above is scripted mock data, not a failure on this branch. Two things worth knowing, because they meant this preview told you nothing about your own change: 1. The view was flagged purely by clock noise. The compose step for run 34207473931 reported exactly one CHANGED view: It landed exactly on the threshold, and the whole diff was a 2. The garbled word is a real bug, and it is not in your diff. Both are fixed in #11380: the chip moves to the bottom-right, the visuals harness renders at a fixed clock, and the turn-error captures are dropped (their assertions stay). Once that lands, previews here should stop being dominated by that scenario, and a flagged view should mean a real change. One thing that may matter for this PR: no visual scenario currently exercises 中文上面 preview 里的红色 error 行是写死的 mock 数据，不是这个分支出了问题。 有两点值得说明，因为它们意味着这次 preview 没能反映你的实际改动： 1. 这张图被标记纯粹是时间戳噪声。 run 34207473931 的 compose 步骤里只有一个 CHANGED，且正好卡在 0.02% 阈值上，全部差异就是 2. 那个被糊掉的词是真 bug，而且不在你的改动里。 fixture 里写的是 两者都在 #11380 修了：chip 移到右下角，visuals harness 用固定时钟渲染，turn-error 的截图删掉（断言全部保留）。合并后这类 preview 就不会再被这个场景占满，被标记的视图也才真正代表有改动。 对这个 PR 可能有影响的一点：目前没有任何 visual scenario 覆盖 |
|
Released in v0.23.1. |
…#11380) * fix(web-shell): stop the hover timestamp from garbling message text `MessageTimestamp`'s hover chip is absolutely positioned at the row's top-right with a 92%-opaque background, so it paints over the first line of the message underneath and lets the glyphs bleed through it. The first line of a wrapped message runs the full column width, so it is almost always the one that gets hit: in the web-shell visual preview for QwenLM#11267 the turn-error copy "...before the answer finished." renders as "finisheu." beneath an 09:18:16 tip, which reads as a corrupted error message rather than as an overlay. Anchor the chip at the bottom-right instead and make its background opaque. The last line of a wrapped block is a ragged remainder, and rows that carry an action bar - the turn-error Copy button is one - leave that corner empty outright, so the common case no longer overlaps text at all; where it still does, an opaque chip with a border reads as an overlay instead of as broken characters. `.chatTip` and `.copyButton` share the same translucent background over content that `.chatActions` pulls up by -24px, so they get the same treatment. This makes overlap rare, not impossible - a message ending in a full-width code block or table still gets the chip in its corner. Reserving space in flow was rejected on purpose: adding padding on hover reflows the transcript, and a permanent float notch shrinks every code block that establishes its own formatting context. * test(web-shell): freeze the wall clock for visual captures The visuals job renders the PR head, then reinstalls dependencies and renders the merge-base, so the two passes photograph the same view minutes apart. Any view showing a wall-clock time therefore differed on every run for no reason other than when it was taken. On QwenLM#11267 that was the entire preview: the only view the compose step reported as CHANGED was `terminal-turn-error-copy-narrow-dark` at exactly the 0.02% threshold, and the whole diff was a 09:09:28 timestamp against a 09:18:16 one. The narrow viewport is half the pixels of the wide one, which is why the same ~100 timestamp pixels cleared the threshold there and were skipped at 0.01% everywhere else - so the noise also crowded out any real change the PR made. Pin `Date.now()` / `new Date()` with `page.clock.setFixedTime` from `gotoSession` and `gotoNewSession`, which every visuals spec navigates through, including the Pixel 7 touch page. `setFixedTime` fakes only clock readings and keeps timers running, so replay, streaming and `freezeLoopingAnimations` are unaffected; elapsed-duration UI settles on a constant instead of drifting, which is the point. This is the same class of fix as `freezeLoopingAnimations`, and `harness.spec.ts` pins its contract the same way. `formatTimestamp` renders local time, so pin `timezoneId: 'UTC'` alongside it. CI runners are already UTC, so this changes no baseline there; it is what makes a local capture match them. Contexts created directly from the browser do not inherit `use` from the config, so the flow-recording context and the touch context repeat it. * fix(web-shell): future-date the frozen capture clock The first pass froze captures at 2026-01-01, which is BEFORE dates the fixtures hardcode. `formatRelativeTime` measures `Date.now() - value`, so the channel editor's pairing requests, dated 2026-07-28, came out negative and collapsed to "just now" where the real clock rendered "7/28/2026" -- visible as the only unexplained CHANGED pair in this PR's own preview (`channel-editor-existing-{dark,light}`). Move the constant to 2027-01-01. Future-dating is the safe direction: every fixture then reads as older than now, which is what all of them mean, and the relative formatter lands in the same branch the live clock did. `git-branch-picker` seeded `computedAt` from `Date.now()`, which in a spec is Node's real clock rather than the page's frozen one. The status therefore looked newer than the branch listing and armed a reconcile re-fetch that no real session performs; it now derives from the same constant. Both rules are written into the constant's docstring for the next fixture. * test(web-shell): make the turn-error fixture identify itself The visual harness injects a fake `turn_error` so the error row's Copy button can be photographed, but its copy was written to read like a real incident -- "The model provider closed the response stream before the answer finished. Retry the request or copy these details when reporting the failure." One test produces four screenshots (two themes x two viewports), so every web-shell preview shows that sentence four times, and readers keep taking the preview for a live failure rather than for mock data. It is the top question this preview generates. Nothing about the scenario needs realistic copy; it needs an error row. Say what the row is instead, and follow the string in the two locators that filter on it. * revert(web-shell): drop the fixture recopy, the timezone pin and the tip move Three of the changes on this branch were not root-caused before being made, and this reverts them to what the reported symptom actually justifies. The tip no longer moves to the bottom-right. The symptom -- "finished." reading as "finisheu." -- comes entirely from the chip's 92%-opaque background letting the glyph beneath bleed through it, so the fix is the one word that makes it opaque. Repositioning was a second, bundled change: more correct in that it stops covering a word at all, but not what was reported, and it is what churned six screenshots. `.chatTip` and `.copyButton` carry the identical translucent background over content, so they change with it. The turn-error fixture keeps its original copy. Rewording it fixed no bug and measurably backfired: the touch pair started differing BECAUSE of the edit, so the preview went from four red screenshots to six, and the replacement text was a line longer. The red rows are the subject of the only visual test that hovers a message row -- the Copy affordance shipped in QwenLM#10001 -- not evidence of a failure, and no wording makes them look otherwise. The `timezoneId: 'UTC'` pins go too. CI runners are already UTC, so they changed no baseline and only served a local reproduction nobody had asked for. The `harness.spec.ts` clock case goes with them: `freezeWallClock` is a one-line wrapper, so the case asserted that Playwright's own `setFixedTime` works. `freezeLoopingAnimations` keeps its case because that function has real logic to pin. What survives is what measurement supports: the opaque background, the frozen clock with its future-dated constant, and the `git-branch-picker` fixture that the freeze would otherwise desynchronise. * test(web-shell): drop the touch turn-error captures, keep their assertions The touch variant renders under `hover: none`, so `.row:hover > .tip` never fires and the hover timestamp is absent from those two screenshots entirely. A run that moved the chip and repainted its background scored both of them at 0% diff, which is direct evidence they carry no coverage of it -- they only contributed two full-height red error images to every web-shell preview. The behaviour they were meant to protect is the one QwenLM#10001 shipped: on a touch device the row actions stay visible with no hover to reveal them. That is asserted directly (`toHaveCSS('opacity', '1')` plus a visible Copy button) and those assertions stay. Only the captures go. The four remaining turn-error captures stay for now: they are the only views in the suite where the hover timestamp lands on real text, so they are the sole visual evidence for the opacity fix on this branch. * fix(web-shell): move the hover timestamp off the first line of the message Reverting the chip to the top-right was wrong, and zooming into the captured pixels is what showed it. The chip's lower edge cuts horizontally through the first line of the message; it does not cover the last word so much as decapitate it, removing a letter's ascender while the bowl stays visible underneath. That is the whole of the reported symptom: "finished." reads as "finisheu." because only the bowl of the d survives. An opaque background therefore cannot fix it. The faint remnant of the ascender currently showing through the 92% chip is the only hint the letter is a d at all -- painting the chip solid erases that hint and leaves a cleaner "u". The previous commit would have made the reported symptom slightly worse. Anchoring at the bottom-right is what removes the overlap: the last line of a wrapped block is a ragged remainder, and rows carrying an action bar leave that corner empty outright. Opacity is kept for its own reason -- a glyph bleeding through any overlay reads as broken text -- and the border makes the chip legible as an overlay where it does still land on content. * test(web-shell): stop capturing the turn-error scenario This scenario is the only visual test that hovers a message row, which it must do to reveal the Copy affordance shipped in QwenLM#10001. The hover timestamp is `opacity: 0` until `.row:hover`, so those captures were the only place in the whole suite where a hover-timestamp change could appear -- and the six screenshots they produced meant every web-shell preview led with full-height red error images, whatever the PR had actually touched. Readers repeatedly took the preview for a live failure. They have now done the one job that justified keeping them: run 34231051803 proved the overlap fix at pixel level, with the d in "finished." regaining its ascender at full h-height while the base panel still shows "finisheu.". That evidence is recorded in the PR description, where it does not have to be re-rendered on every unrelated push. The behaviour stays pinned by assertion, not by image: seven opacity checks across the default viewport, the 720px viewport and the Pixel 7 touch context cover reveal on hover, reveal on focus, hide on blur, and always-visible actions under `hover: none`. A capture never verified any of that; Playwright does not diff these images, the preview bot does, and the preview is a review aid rather than a test. The scenario, the fixture and every assertion remain. Only the captures go. * test(web-shell): fail when a fixture date outlives the frozen capture clock Captures render at a fixed instant, so a fixture dated after it silently reads as "just now": `formatRelativeTime` measures `Date.now() - value`, a future value yields a negative age, and that lands in the `mins < 1` branch. This already happened on this branch. The first frozen constant sat at 2026-01-01, before the channel editor's 2026-07-28 pairing requests, and turned "7/28/2026" into "just now" across four views. Nothing failed -- no assertion covers that label -- and it was caught only by reading a preview diff by eye. Scan the visuals specs for ISO date literals and require each to precede FIXED_CAPTURE_TIME. The docstring already asked for this; a comment does not fail a build. `harness.ts` is skipped because it is where the constant itself is written, so its literal is the boundary rather than a fixture. The scan reaches further than the fixture that caused the incident: it covers eleven literals across the suite, eight of them a 2026-07-03 pair I had not been aware of. Verified in both directions -- silent against the tree as it stands, and flagging a planted 2027-06-01 literal. Not a general date linter. It catches ISO strings in fixtures, which is the shape the regression actually took; an epoch number is already opaque enough that nobody reads it as a date. * test(web-shell): fail when a fixture date outlives the frozen capture clock Captures render at a fixed instant, so a fixture dated after it silently reads as "just now": `formatRelativeTime` measures `Date.now() - value`, a future value yields a negative age, and that lands in the `mins < 1` branch. This already happened here. The first frozen constant sat at 2026-01-01, before the channel editor's 2026-07-28 pairing requests, and turned "7/28/2026" into "just now" across four views. Nothing failed -- no assertion covers that label -- and it surfaced only by reading a preview diff by eye. Scan the visuals specs for ISO date literals and require each to precede FIXED_CAPTURE_TIME. The constant's docstring already asked for this, and a docstring does not fail a build. `harness.ts` is skipped because it is where the constant itself is written, so its literal is the boundary, not a fixture. The scan reaches further than the fixture that caused the incident: eleven literals across the suite, eight of them a 2026-07-03 group that was not on my radar. Checked in both directions before committing -- silent against the tree as it stands, and flagging a planted 2027-06-01 literal. Not a general date linter. It catches ISO strings in fixtures, which is the shape this regression actually took; an epoch number is already opaque enough that nobody reads it as a date. b162c19 was meant to carry this and landed empty: the scratch copy of the file was overwritten by a later extraction step before it was hashed, so the commit recorded no change while its message described one. History is append-only on an open PR here, so it stays in the log; this commit is the content. * fix(web-shell): correct three wrong comments and give the contracts a gating home Review found eight issues on this branch. Three were comments asserting things the code does not do, and comments that confidently state the opposite of the behaviour are worse than no comment. The CSS block kept its original opening sentence, "Anchored inside the message's top-right corner", above a paragraph declaring the BOTTOM-right anchor and a `bottom: 2px` rule. The retained sentence also carries the overflow rationale that is still true, so it reads as the block's thesis -- someone "restoring" `top: 2px` to match it puts the chip back over the first line and reintroduces the exact decapitation this branch exists to remove. Only the corner is dropped; the overflow reason stays. The `git-branch-picker` fixture comment claimed a Node-stamped `computedAt` would look NEWER than the branch listing and arm a reconcile re-fetch. The constant is future-dated, so a real-clock value sits months BELOW `listingFetchedAt`, and the gate is `at <= listingFetchedAt` -- the branch was already unreachable and the edit is inert today. The comment now says what the edit is for (the harness rule about "now" values) and what would actually exercise the reconcile path. The `freezeWallClock` docstring said `setFixedTime` fakes clock readings and not timers. It installs Playwright's full fake clock: timers, rAF, rIC, `performance` and `Intl` are all replaced. Outcomes are unaffected today, but `performance.mark`/`measure` are stubbed and every `Date.now()`-delta window in the app is pinned permanently shut. Nothing seeds those states, so nothing fails -- until a scenario needs one to elapse and hangs as a bare expect timeout while the docstring insists timers behave normally. `.toolTimeTip` in SubAgentPanel mirrors this chip over a sub-tool line, was missed by a paragraph claiming to sweep the overlays, and was strictly worse: same `top: 2px` geometry, no `background` at all, so two texts composited directly. It gets the same treatment, and the paragraph now names it. The remaining three findings share a root: nothing observes any of this. `.tip` is `opacity: 0` until hover and no capture paints it any more, so reverting the anchor or the background keeps every suite green. `freezeWallClock` runs only implicitly and no assertion reads the page clock, so dropping the call resumes the drift silently. And the date guard sat in `harness.spec.ts`, which no gating config collects -- `vitest.config.ts` excludes `e2e/**`, `playwright.config.ts` ignores `visuals/**`, and the one workflow step that does run it is deliberately `continue-on-error: true` so a failing scenario cannot discard the preview. `visual-capture-contracts.test.ts` is a plain vitest file outside `e2e/`, collected by `test:ci:workspaces`, pinning all three: fixture dates precede the frozen clock, both navigation helpers freeze before they navigate, and every hover chip stays bottom-anchored and opaque. The date scan also now reads `e2e/utils`, where `mockDaemon.ts` holds the date the default scenario's session rows actually render -- scanning only `visuals/` missed it, which is what the review caught. Its reach is still ISO string literals; the comment says so rather than implying the name's full promise. `FIXED_CAPTURE_TIME` moves to `constants.ts` so the guard can import it without pulling in `@playwright/test`. Each guard was checked in both directions before committing: silent against the tree as it stands, and firing on a planted 2027-06-01 date in `mockDaemon.ts`, a `freezeWallClock` moved after `page.goto`, and a `.tip` reverted to `top`. * fix(web-shell): stop claiming the sub-tool chip moved off its row Moving `.toolTimeTip` to the bottom of its row was inert, and the comment saying it 'inherits its fix for the same reason' overstated it the same way three comments this branch already had to correct. The row has no vertical slack to move into. `.line` is `padding: 2px 0` around 13px text, so roughly 17-24px tall, and `.lineArg` is `white-space: nowrap` so it never wraps to a second line. The chip is about 19px once padding and a border are added -- as tall as the row it sits in. `bottom: 2px` therefore lands where `top: 2px` already was, and the padding and border I added only made the chip taller than the row on the narrow end of that range. What was actually wrong there is that the chip declared no `background` at all, so its digits overprinted the row's text rather than covering it. That is the whole fix: one declaration, no geometry change. The text under the chip is still covered -- cleanly now instead of overprinted -- and the comment says so rather than implying the overlap is gone. `.tip` keeps its bottom anchor, which does work: a message wraps, so its last line is a ragged remainder and rows carrying an action bar leave that corner empty outright. The contract test splits along the same line: every chip must be opaque, only `.tip` must be bottom-anchored. Asserting a bottom anchor for a row that has nowhere to put one would have pinned a cosmetic no-op as if it were the fix. * docs(web-shell): correct the claim that only turn-error paints the chip The comment left where the captures were removed said this scenario's screenshots were "the only place a hover-timestamp change could show up". That is false, and this PR's own preview disproves it: the parallel agents test leaves the cursor on the group header after `summary.click()`, so `parallel-agents-expanded` paints the chip through residual hover -- and it is the only view still reported as CHANGED here, at 0.08%, precisely because the chip moved. The distinction that survives is deliberate versus incidental. This test is the only one that hovers on purpose; the parallel agents coverage is a side effect of a click and would vanish the moment that test stopped clicking. Neither is what pins the chip now -- visual-capture-contracts.test.ts is.
6 participants
What this PR does
Adds a wire-additive per-session active-work state derived from the daemon's existing local work counters, negotiated child hold snapshots, category coverage, and freshness window. The workspace live-state response, TypeScript SDK, trusted session catalogs, and ACP list projection carry the state without changing
hasActivePrompt. Web Shell session rows keep the existing green foreground-turn indicator, show a blue indicator and spinner for other known active work, and show an explicit unknown marker when negotiated reporting is stale or incomplete. The Todo panel spinner uses the same live state so persisted todos do not look active after session work settles.Why it's needed
The session list currently becomes indistinguishable from idle as soon as a foreground prompt settles, even when the Session still owns a background Agent, shell, workflow, cron/goal turn, history mutation, queued prompt, or pending terminal continuation. Reusing
hasActivePromptwould incorrectly turn those states into foreground turns. The new four-state value preserves positive activity, proves idle only from a fresh fully covered empty snapshot, and exposes degraded knowledge instead of silently treating it as idle.This is stacked on #11265 so the active-work category set already includes child-owned Session turns.
Reviewer Test Plan
How to verify
activeWorkState: "active"; the sidebar keeps a distinct blue activity indicator and spinner.idleand removes the activity indicator.unknownrather thanidle.unsupported; older daemons that omit the field remain readable.Evidence (Before & After)
Before:
hasActivePrompt: falsemakes a Session with background work look idle. After: the same row carriesactiveWorkState: "active"and renders an independent activity indicator. No local screenshot was captured.Tested on
Environment (optional)
No local test, build, typecheck, or CI command was run. Verification was limited to two clean static production-flow reviews and
git diff --check.Risk & Scope
activeis positive knowledge;idleis only as fresh as the negotiated heartbeat window;unknowndeliberately surfaces degraded coverage.hasActivePromptsemantics are unchanged.Linked Issues
Part of #11119
Depends on #11265
中文说明
本 PR 做了什么
新增一个向后兼容的 per-session active-work 状态，直接从 daemon 现有的本地 work 计数、协商后的 child hold snapshot、类别覆盖度和 freshness window 派生。workspace live-state 响应、TypeScript SDK、可信 session catalog 和 ACP list 投影都会携带该状态，但不改变
hasActivePrompt。Web Shell session 行继续用绿色表示正在运行的前台 turn；其他已知活动使用独立的蓝色指示点和 spinner；协商式上报过期或类别不完整时显示明确的未知状态。Todo 面板的 spinner 也使用同一份 live state，避免 session work 结束后 persisted todos 仍显示为活动。为什么需要
当前前台 prompt 一结束，session list 就与真正 idle 无法区分，即使 Session 仍拥有后台 Agent、shell、workflow、cron/goal turn、history mutation、排队 prompt 或待处理终态 continuation。复用
hasActivePrompt会错误地把这些状态伪装成前台 turn。新的四态值会保留正向活动证据，只在 fresh、全覆盖且为空的 snapshot 下声明 idle，并把退化状态明确暴露出来，而不是静默当作 idle。本 PR 叠加在 #11265 之上，因此 active-work 类别集合已经包含 child-owned Session turn。
Reviewer Test Plan
如何验证
activeWorkState: "active"，侧栏继续显示独立的蓝色活动指示和 spinner。idle，活动指示消失。unknown，而不是idle。unsupported；省略该字段的旧 daemon 仍可正常读取。证据（Before & After）
Before：
hasActivePrompt: false会让仍有后台 work 的 Session 看起来 idle。After：同一行携带activeWorkState: "active"并渲染独立活动指示。未采集本地截图。已测试平台
环境（可选）
未运行本地测试、build、typecheck 或 CI 命令。验证仅包含两轮干净的生产链路静态审核与
git diff --check。风险与范围
active是正向证据；idle只在协商 heartbeat freshness window 内成立；unknown会有意暴露覆盖退化。hasActivePrompt语义不变。关联 Issue
Part of #11119
Depends on #11265