Skip to content

fix(web-shell): gate todo spinner on live work - #11269

Merged
yiliang114 merged 2 commits into
codex/issue-11119-live-statefrom
codex/issue-11119-todo-liveness
Sep 7, 2026
Merged

fix(web-shell): gate todo spinner on live work#11269
yiliang114 merged 2 commits into
codex/issue-11119-live-statefrom
codex/issue-11119-todo-liveness

Conversation

@yiliang114

Copy link
Copy Markdown
Collaborator

What this PR does

Makes the floating Todo panel animate an in_progress item only while the current Session has observable live execution: a locally streaming turn, a daemon-reported foreground prompt, or per-session activeWorkState: "active". When the persisted Todo still says in_progress but no live source confirms execution, the panel keeps the existing static status glyph instead of showing an endless spinner.

The existing prompt bridge now returns the active-work state from the same live-session snapshot it already consumes. Its original boolean wrapper remains unchanged for prompt-only consumers, and the App test harness is only updated to preserve that existing mock boundary.

Why it's needed

A Todo item is persisted plan state, not a liveness signal. Today in_progress unconditionally renders a spinner, so a settled or disconnected Session can look as if it is still executing forever. The live-state contract introduced by #11267 supplies the missing execution fact; this PR consumes it without inventing another timer or task tracker.

Reviewer Test Plan

How to verify

  • While a prompt is streaming, confirm an in_progress Todo keeps spinning.
  • During a silent foreground turn with daemon hasActivePrompt: true, confirm the spinner remains active.
  • After the foreground turn settles, keep a Session-managed background hold active and confirm the spinner remains active through activeWorkState: "active".
  • Settle all live work while leaving the persisted Todo item in_progress; confirm its icon becomes static.
  • Confirm prompt-only consumers still receive the same boolean from the existing active-prompt bridge.

Evidence (Before & After)

Before: persisted in_progress always animated, even with no live execution. After: animation is gated by the existing local/daemon liveness facts. No local screenshot was captured.

Tested on

OS Status
🍏 macOS ⚠️ not run
🪟 Windows ⚠️ not run
🐧 Linux ⚠️ not run

Environment (optional)

No local test, build, typecheck, or CI command was run. Verification was limited to two clean static flow reviews and git diff --check.

Risk & Scope

  • Main risk or tradeoff: on an older daemon that cannot expose per-session active work, a background-only Todo is static after the foreground turn settles rather than pretending that execution is known.
  • Not validated / out of scope: durable task ledgers, cross-runtime route rebinding, and runtime-generation recovery.
  • Breaking changes / migration notes: none. The existing TodoPanel default and prompt-only hook API remain compatible.

Linked Issues

Part of #11119

Depends on #11267

中文说明

本 PR 做了什么

让浮动 Todo 面板只在当前 Session 存在可观测的 live execution 时,为 in_progress 项目显示旋转动画:本地正在流式输出、daemon 报告前台 prompt 正在运行,或者 per-session activeWorkState: "active"。如果持久化 Todo 仍为 in_progress,但没有任何 live 来源确认仍在执行,则保留现有静态状态图标,不再无限旋转。

现有 prompt bridge 现在会从它已经消费的同一份 live-session snapshot 返回 active-work 状态。给只关心 prompt 的调用方保留原有 boolean wrapper;App 测试夹具仅做必要的 mock 边界适配。

为什么需要

Todo 项目是持久化计划状态,不是 liveness 信号。目前 in_progress 无条件渲染 spinner,所以已经 settle 或断开的 Session 也可能永久显示成仍在执行。#11267 引入的 live-state contract 提供了缺失的执行事实;本 PR 直接消费它,不新增 timer 或 task tracker。

Reviewer Test Plan

如何验证

  • prompt 正在流式输出时,确认 in_progress Todo 继续旋转。
  • daemon 返回 hasActivePrompt: true 的静默前台 turn 中,确认 spinner 保持活动。
  • 前台 turn 结束后保留一个 Session-managed 后台 hold,确认 activeWorkState: "active" 会维持 spinner。
  • 所有 live work 结束但持久化 Todo 仍为 in_progress 时,确认图标变为静态。
  • 确认只关心 prompt 的调用方仍从原有 active-prompt bridge 得到相同 boolean。

证据(Before & After)

Before:只要持久化状态是 in_progress 就一定旋转,即使没有 live execution。After:动画由现有本地/daemon liveness 事实控制。未采集本地截图。

已测试平台

OS 状态
🍏 macOS ⚠️ 未运行
🪟 Windows ⚠️ 未运行
🐧 Linux ⚠️ 未运行

环境(可选)

未运行本地测试、build、typecheck 或 CI 命令。验证仅包含两轮干净的静态链路审核与 git diff --check

风险与范围

  • 主要风险或取舍:旧 daemon 无法暴露 per-session active work 时,前台 turn 结束后的后台-only Todo 会显示为静态,而不是假装已知它仍在执行。
  • 未验证 / 范围外:持久化任务 ledger、跨 runtime 路由重绑和 runtime-generation 恢复。
  • 破坏性变更 / 迁移说明:无。TodoPanel 现有默认值与 prompt-only hook API 保持兼容。

关联 Issue

Part of #11119

Depends on #11267

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required heading is present, and the Tested-on table is honestly marked "not run" instead of being left blank or optimistically ticked.

Problem: real, and reachable from the code rather than theoretical. TodoPanel rendered styles.loadingIcon for any in_progress item unconditionally, and a Todo is persisted plan state — so a session that settles, disconnects, or recycles while an item is still in_progress spins forever. That is the same wedge shape #11119 (open, P1) describes. The honest gap: no before/after capture is attached, and the body says so plainly. I'm treating the problem as observed because the state is reachable by construction and the linked issue documents sessions that stop executing while persisted state survives — not because there's a recording.

Direction: aligned. The reference product has fixed this exact class repeatedly — claude-code 2.1.261: "Fixed Remote Control sessions showing as still working (stuck spinner and Stop button) after stopping a turn from a connected phone or browser", and the same changelog carries a stuck-spinner fix for remote clients and an idle session wrongly reported as "a turn is in progress" while a background task ran. A browser-side panel animating work nobody is doing is the same bug in a different hat.

Size: Stage 0 not triggered — no core paths (packages/web-shell/client/** plus one design doc), single package, no cross-package span. 58 production lines (53 code + 5 docs) and 17 test lines, 80 total.

Approach: close to what I'd have written independently — one optional prop, gated at the single call site, reusing the live-state contract #11267 already establishes instead of adding a timer or a task tracker. I checked the scope is complete rather than partial: App.tsx:17659 is the only production TodoPanel render, and the in-message TodoView has no spinner at all, so the floating panel genuinely is the only surface with this bug.

One question, not a blocker: the hooks module now exports two functions where it had one — useDaemonSessionActivityBridge plus the retained boolean useDaemonActivePromptBridge. I think keeping the wrapper is the right call, since it leaves ChatPane and two test files untouched, but it does mean carrying a shim for exactly one internal caller. Worth a moment's thought on whether you'd rather have ChatPane destructure.

Risk: no Stage 1e high-risk path match. Two structural risks worth stating up front, neither about the code itself:

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 所有必需标题都在,Tested-on 表格也如实标注"未运行",没有留空或乐观打勾。

问题: 真实存在,而且是代码上可推导出来的,不是理论性加固。TodoPanel 对任何 in_progress 项目都无条件渲染 styles.loadingIcon,而 Todo 是持久化的计划状态 —— 所以一个已经 settle、断开或被回收的 Session,只要还有项目停在 in_progress,就会永远旋转。这正是 #11119(open,P1)描述的 wedge 形态。坦白说缺口在于:没有附 before/after 截图,PR 正文也明确写了。我之所以判定问题已观测,是因为该状态在结构上必然可达,且关联 issue 记录了"执行已停止但持久化状态仍在"的 Session —— 而不是因为有录屏。

方向: 对齐。参考产品反复修过同一类问题 —— claude-code 2.1.261:"Fixed Remote Control sessions showing as still working (stuck spinner and Stop button) after stopping a turn from a connected phone or browser";同一份 changelog 里还有 remote client 卡住 spinner 的修复,以及"空闲 Session 因后台任务被误报 a turn is in progress"的修复。浏览器侧面板给没人执行的工作播放动画,是同一个 bug 换了件外套。

规模: 未触发 Stage 0 —— 没有核心路径(packages/web-shell/client/** 加一份设计文档),单包,不跨包。生产代码 58 行(53 行代码 + 5 行文档),测试 17 行,合计 80 行。

方案: 和我独立想到的做法基本一致 —— 一个可选 prop,在唯一调用点收口,复用 #11267 已经建立的 live-state 契约,而不是新增 timer 或 task tracker。我确认了范围是完整的而非局部:App.tsx:17659 是生产代码里唯一的 TodoPanel 渲染点,而消息流里的 TodoView 根本没有 spinner,所以浮动面板确实是目前唯一有这个 bug 的界面。

一个疑问(非阻塞):hooks 模块现在从一个导出变成两个 —— useDaemonSessionActivityBridge 加上保留的 boolean useDaemonActivePromptBridge。我认为保留 wrapper 是对的选择,因为它让 ChatPane 和两个测试文件都不用动,但这确实意味着为一个内部调用方维护了一层兼容 shim。可以再想一下是否更愿意让 ChatPane 直接解构。

风险: Stage 1e 高风险路径未命中。有两个结构性风险需要先讲清楚,都与代码本身无关:

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at c926de9df9f2118556a785ca0b09067d256d99c7 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Code review

No Critical findings and no AGENTS.md violations. The change is small enough that I traced every read site rather than sampling, which matters here because nothing has compiled this commit (see Testing below). What I verified:

The field is live, not a dead switch. activeWorkState doesn't exist anywhere on main, so I checked the stacked base instead: DaemonSessionSummary.activeWorkState (packages/sdk-typescript/src/daemon/types.ts:1346) and DaemonSessionLiveState.activeWorkState (:1572) are both declared there as optional 'active' | 'idle' | 'unknown' | 'unsupported', so sessionActiveWorkState === 'active' narrows correctly. The producer path is real end to end — #11267 computes it in the bridge (entryActiveWorkState), forwards it through serve/acp-http/dispatch.ts, routes/session.ts and standalone-session-service.ts, and session-catalog-store.ts carries it into the live map (:732, :768), which is exactly what store.getLiveSession(...)?.activeWorkState reads. The page-row fallback row?.activeWorkState is populated by the same field on the summary. Deepest producer is #11265.

Every consumer of the changed hook is named. At the base ref only App.tsx and ChatPane.tsx touch these hooks; the sidebar components #11267 adds read catalog rows directly, not the hook. So the signature change has exactly two consumers — App, updated here, and ChatPane, still on the retained boolean wrapper, whose publish side effect is unchanged because the effect lives inside the renamed function.

Scope is valid. export function App({ opens at line 2851 and its body closes at 18528; streamingState (7064), sessionActiveWorkState (3269) and the <TodoPanel> JSX (17659) are all inside that one function. Worth stating explicitly only because no typechecker ran.

The hook refactor preserves semantics. I walked all four combinations of liveSessionRevision / sessionId defined-or-not: the new liveSession intermediate yields the same liveActivePrompt value as the old inline expression in every case, so hasLiveSessions = liveActivePrompt !== undefined is unchanged.

Existing tests survive the rename. App.test.tsx renames all five sites, and it never renders ChatPaneApp.tsx only holds an import type from it, which is erased — so dropping the old key from the session-catalog-hooks mock is safe. session-catalog-hooks.test.tsx asserts through rendered text from destructured fields rather than toEqual on the whole hook result, and its applyLiveState literals still typecheck because the new field is optional. TodoPanel.test.tsx passes untouched via the hasLiveActivity = true default.

The fallback glyph is meaningful. getTodoStatusIcon('in_progress') returns , distinct from (pending) and (completed), so a gated item still reads as in-progress rather than going blank or looking done. Good outcome for the "no live work" case.

Two things I'd note without blocking:

  • activeWorkState comes back from the live branch without the freshness qualification that feeds authoritative. That is consistent with how hasActivePrompt is already returned raw in the same branch — freshness there gates the provider publish, not the returned value — and the store drops the snapshot after a failure streak, so staleness is bounded. Flagging only so it reads as a deliberate choice rather than an oversight.
  • No test pins the new gate. TodoPanel.test.tsx never passes hasLiveActivity={false}, and no App-level test drives the settled-but-still-in_progress case, so the suite would pass identically with the prop hardcoded to true — i.e. with the fix removed. Per AGENTS.md a missing test for changed behavior is a Suggestion rather than a Critical, so I'm not gating on it, but it's the one gap I'd close before this stack lands.

Testing

Being plain about what this section carries: static review only. No build, no typecheck, no unit test ran on this commit — not by me, not by CI, not by the author.

That is structural, not a timing artifact. ci.yml filters its pull_request trigger to main and release/**, and this PR's base is codex/issue-11119-live-state, so the repo's test / lint_and_static lanes are never scheduled here. All 17 check-runs on the head commit are the bot's own pull_request_target orchestration:

Check Conclusion
assign success
authorize (×2) success
label success
delay-automatic-review success
review-pr in progress
triage in progress
verify skipped
tmux-testing skipped
publish-verify skipped
publish-tmux skipped
publish-resolution skipped
review-config skipped
resolve-pr skipped
ack-review-request skipped
precheck-pr (×2) skipped

There is no failing check to excerpt, and no green suite to lean on either — the absence of a red check here is not evidence of anything. I did not execute any PR-derived code; per the triage rules the review is static, and I read the base-ref files through the API rather than checking the branch out.

Not verified: that the stack compiles as a whole, that the web-shell suites still pass after the mock rename, and that the spinner actually stops when live work settles. The author's own report ("no local test, build, typecheck, or CI command was run") is their claim, and I'm not presenting it as evidence.

Sandboxed verification would settle the behavioural half of this: @qwen-code /verify — that the panel keeps animating through a background hold on activeWorkState: "active" and drops to the static once every live source says the session is idle, is not observable from the diff, and this PR's suite would pass with the hasLiveActivity gate removed entirely. It needs a DOM-level render assertion on TodoPanel, so /verify is the right lane rather than /tmux, which drives the TUI and never reaches a browser surface.

中文说明

代码审查

没有 Critical,也没有违反 AGENTS.md。改动足够小,所以我把每个读取点都追了一遍而不是抽样 —— 在这个 PR 上这点尤其重要,因为没有任何机器编译过这个 commit(见下方"测试")。已核实的内容:

这个字段是活的,不是死开关。 activeWorkStatemain 上完全不存在,所以我改查堆叠的 base:DaemonSessionSummary.activeWorkStatepackages/sdk-typescript/src/daemon/types.ts:1346)和 DaemonSessionLiveState.activeWorkState:1572)都在那里声明为可选的 'active' | 'idle' | 'unknown' | 'unsupported',因此 sessionActiveWorkState === 'active' 能正确收窄。生产链路端到端是通的 —— #11267 在 bridge 里计算它(entryActiveWorkState),经 serve/acp-http/dispatch.tsroutes/session.tsstandalone-session-service.ts 转发,session-catalog-store.ts 再把它带进 live map(:732:768),而这正是 store.getLiveSession(...)?.activeWorkState 读取的东西。分页 row 回退分支的 row?.activeWorkState 由 summary 上的同名字段填充。最底层的生产者是 #11265

被改动 hook 的每个消费方都已点名。 在 base ref 上只有 App.tsxChatPane.tsx 用到这两个 hook;#11267 新增的 sidebar 组件直接读 catalog row,不走 hook。所以签名变化恰好只有两个消费方 —— App(本 PR 已改),以及 ChatPane(继续用保留的 boolean wrapper)。因为发布副作用在改名后的函数内部,ChatPane 的行为完全没变。

作用域有效。 export function App({ 从 2851 行开始,函数体到 18528 行结束;streamingState(7064)、sessionActiveWorkState(3269)和 <TodoPanel> JSX(17659)都在这一个函数内。之所以要专门讲,是因为没有类型检查器跑过。

hook 重构保持了语义。 我把 liveSessionRevision / sessionId 四种"有/无"组合都走了一遍:新的 liveSession 中间变量在每种情况下得到的 liveActivePrompt 都与原先的内联表达式一致,所以 hasLiveSessions = liveActivePrompt !== undefined 没有变化。

现有测试能扛过这次改名。 App.test.tsx 五处全部改了名,而且它从不渲染 ChatPane —— App.tsx 只有一个会被擦除的 import type —— 所以从 session-catalog-hooks 的 mock 里去掉旧 key 是安全的。session-catalog-hooks.test.tsx 是通过解构字段后的渲染文本断言,而不是对整个 hook 返回值做 toEqual,其 applyLiveState 字面量也仍能通过类型检查,因为新字段是可选的。TodoPanel.test.tsxhasLiveActivity = true 的默认值原样通过。

回退图标是有意义的。 getTodoStatusIcon('in_progress') 返回 ,与 (pending)、(completed)都不同,所以被收口的项目仍然读作"进行中",而不是变空白或看起来已完成。"无 live work"这一分支的结果是好的。

两点提醒,都不阻塞:

  • live 分支返回的 activeWorkState 没有带上决定 authoritative 的新鲜度限定。这与同一分支里 hasActivePrompt 本来就是裸返回的做法一致 —— 那里的新鲜度只用于把关 provider 发布,不影响返回值 —— 而且 store 在连续失败后会丢弃快照,所以陈旧是有上界的。提出来只是希望它看起来是有意选择,而不是疏漏。
  • 没有测试钉住这个新闸门。 TodoPanel.test.tsx 从未传 hasLiveActivity={false},App 层也没有测试覆盖"已 settle 但仍为 in_progress"这一情形,所以把该 prop 硬编码成 true(也就是把修复去掉)测试照样全绿。按 AGENTS.md,行为变更缺测试属于 Suggestion 而非 Critical,因此我不以此设卡,但在这个堆叠合并前,这是我最想补上的一个缺口。

测试

这一节到底带了什么证据,说清楚:只有静态审查。这个 commit 上没有跑过任何 build、typecheck 或 unit test —— 我没跑,CI 没跑,作者也没跑。

这是结构性的,不是时序问题。ci.ymlpull_request 触发限定在 mainrelease/**,而本 PR 的 base 是 codex/issue-11119-live-state,所以仓库的 test / lint_and_static 这两条 lane 在这里根本不会被调度。head commit 上全部 17 个 check-run 都是 bot 自己的 pull_request_target 编排任务(见上表)。

没有失败的检查可以摘录,但也没有绿色的套件可以依靠 —— 这里"没有红叉"并不能证明任何事。我没有执行任何来自 PR 的代码;按 triage 规则审查是静态的,base ref 的文件我通过 API 读取,而不是把分支 checkout 出来。

未验证:整个堆叠能否编译通过、mock 改名后 web-shell 套件是否仍然全绿、以及 live work 结束后 spinner 是否真的停下。作者自己的说明("未运行本地测试、build、typecheck 或 CI 命令")属于作者的声明,我不把它当作证据呈现。

沙箱验证可以解决其中行为层面的部分:@qwen-code /verify —— "后台 hold 期间靠 activeWorkState: "active" 维持动画、所有 live 来源都表明 Session 空闲后落回静态 "这件事从 diff 上看不出来,而且把 hasLiveActivity 闸门整个去掉本 PR 的测试也照样通过。它需要对 TodoPanel 做 DOM 级渲染断言,所以合适的 lane 是 /verify 而不是 /tmux —— 后者驱动的是 TUI,够不到浏览器界面。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at c926de9df9f2118556a785ca0b09067d256d99c7 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — the diff is right and I could trace every read site; the missing fifth is that no machine has compiled or tested it, so this approval attests to a static read and nothing more.

Stepping back: my independent proposal before opening the diff was "gate the animation on a liveness fact App already holds, pass one prop down, default it so nothing else has to move" — which is what this does, so I have no simpler path to offer. The restraint I look for is there: no new timer, no task tracker, no second source of truth, and the field it consumes is one #11267 already plumbs end to end. "Cut 80% of the scope" doesn't apply, because there isn't 80% — 53 production lines, one of which is the actual fix.

Am I approving because I ran out of reasons to say no? I don't think so. I went in expecting to find the classic version of this bug — a gate wired to a signal that never arrives — and checked for it specifically: activeWorkState doesn't exist on main at all, so it would have been easy for this to be a dead switch. It isn't. The base declares it on both types, the bridge computes it, serve forwards it, and the store carries it into the very snapshot this hook reads. Likewise the retained boolean wrapper looked like leftover API surface until I found ChatPane.tsx:276 still using it, and the dropped mock key in App.test.tsx looked like a guaranteed crash until I confirmed App never renders ChatPane. Each of those survived the check rather than being waved through.

What gives me pause is the evidence, not the code. Nothing has compiled this commit: the base isn't main or release/**, so ci.yml never schedules test or lint_and_static, and the author reports running nothing locally. I substituted static tracing for that — identifier scope, type existence at the base ref, import/export consistency, every consumer of the changed signature — and it all held, but a static read is weaker than a green lane and I'm not going to pretend otherwise. Two concrete asks, neither blocking this approval:

One honest observation on cadence, not a criticism of the diff: this is the third PR in one chain, and one of forty you have open, several created within the last hour. I reviewed it on its own contents and not on volume — but a chain this deep is worth a moment's thought about whether #11265 and #11267 could land first and let this one be reviewed against main, where CI would actually cover it.

Mechanically, for the record: the fork-refactor guardrail doesn't apply (same-repo branch, fix type), Stage 0 raised no escalation (no core paths), and there are zero pending pull_request workflow runs on this commit — there never will be any on a non-main base. So deferring via an approve-on-green marker would strand the approval permanently, which is why I'm approving now, pinned to the exact commit I reviewed.

中文说明

Confidence: 4/5 —— diff 是对的,每个读取点我都追到了;差的那一分在于没有任何机器编译或测试过它,所以这次 approve 只为一次静态阅读背书,仅此而已。

退一步看:我在打开 diff 之前的独立设想是"用 App 已经持有的 liveness 事实来给动画收口,往下传一个 prop,并给它默认值以免别处跟着动" —— 这正是本 PR 做的事,所以我拿不出更简单的路径。我想看到的克制也在:没有新增 timer,没有 task tracker,没有第二个真相来源,而它消费的字段是 #11267 已经端到端打通的。"砍掉 80% 范围"在这里不适用,因为根本没有 80% —— 生产代码 53 行,其中一行才是真正的修复。

我是不是因为找不到理由拒绝才 approve?我认为不是。我进来时就预期会看到这类 bug 的经典版本 —— 闸门接到一个永远不会到达的信号 —— 并专门查了这一点:activeWorkStatemain 上完全不存在,所以它很容易就会是个死开关。结果不是。base 在两个类型上都声明了它,bridge 计算它,serve 转发它,store 把它带进这个 hook 读取的那份快照。同样,保留的 boolean wrapper 看起来像多余的 API 表面,直到我发现 ChatPane.tsx:276 仍在用它;App.test.tsx 里被去掉的 mock key 看起来必然导致崩溃,直到我确认 App 从不渲染 ChatPane。这些是逐条通过了检查,而不是被挥手放过。

让我犹豫的是证据,不是代码。没有任何东西编译过这个 commit:base 不是 main 也不是 release/**,所以 ci.yml 从不调度 testlint_and_static,而作者说本地什么都没跑。我用静态追踪替代了它 —— 标识符作用域、base ref 上的类型存在性、import/export 一致性、被改签名的每个消费方 —— 结论都成立,但静态阅读弱于一条绿色 lane,我不会假装它等价。两个具体请求,都不阻塞这次 approve:

关于节奏的一点坦白观察,不是对 diff 的批评:这是同一条链上的第三个 PR,也是你 40 个 open PR 之一,其中好几个是最近一小时内创建的。我是按它自身的内容审查的,不是按数量 —— 但这么深的链值得想一下:能否先让 #11265#11267 落地,使这一个可以对着 main 被审查,那样 CI 才真正覆盖得到它。

流程上留个记录:fork-refactor 护栏不适用(同仓库分支,fix 类型),Stage 0 没有升级(无核心路径),并且该 commit 上待完成的 pull_request workflow run 数为 0 —— 在非 main 的 base 上永远不会有。所以用 approve-on-green 标记来延后会把这次 approve 永久搁置,这也是我现在就 approve、并钉在我实际审查的那个 commit 上的原因。

Qwen Code · qwen3.8-max-2026-09-02

Reviewed at c926de9df9f2118556a785ca0b09067d256d99c7 · re-run with @qwen-code /triage

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@yiliang114
yiliang114 merged commit 6ff45b4 into codex/issue-11119-live-state Sep 7, 2026
10 of 11 checks passed
@wenshao

wenshao commented Sep 7, 2026

Copy link
Copy Markdown
Collaborator

Local verification — real browser + real daemon

I built a real verification environment for this PR locally and ran it as a two-arm A/B. Everything below is measured, not read.

Armsbase = 3dd860aafb (this PR's merge base, #11267 head) · head = c926de9df9 (this PR). The two arms differ only by this PR's 5 files.

Worth knowing before reading the rest: Qwen Code CI only triggers on pull_request against main / release/** (.github/workflows/ci.yml). This PR targets codex/issue-11119-live-state, so no CI job has ever compiled, linted or tested this commit — the checks on the PR are triage/routing only. The runs below are the first.

before and after

1. Behaviour matrix — real Chromium, real Web Shell client

I drove the real client (real App, real session-catalog-store, real hooks, real TodoPanel.module.css) in headless Chromium 149 against a wire-level daemon. The verdict per scenario is read from the browser itself — whether TodoPanel rendered the spinner element, plus Element.getAnimations() to confirm the todoPanelSpin keyframes are actually running, so a spinner that renders but never animates cannot pass as one.

Every scenario starts from the same persisted state: a settled transcript whose last todo_write left Apply the fix at in_progress.

# Scenario live-state row base head
A local turn streaming hasActivePrompt:false, activeWorkState:idle spinning spinning
B silent foreground turn hasActivePrompt:true spinning spinning
C foreground settled, background hold activeWorkState:"active" spinning spinning
D everything settled activeWorkState:"idle" spinning static
E older daemon, field absent activeWorkState omitted spinning static
F bridge cannot answer activeWorkState:"unknown" spinning static
G no live-state capability, catalog row activeWorkState:"idle" spinning static
H no live-state capability, catalog row activeWorkState:"active" spinning spinning

8/8 as intended on head; 8/8 spinning on base, which is the bug — including D/E/F/G where nothing is live.

scenario matrix

Two details that mattered while building this:

  • Scenario A cannot be faked with a stale running tool block. selectDaemonStreamingState returns idle whenever the daemon's promptStatus is idle, so the arm submits a real prompt through the composer and the plan arrives mid-turn over SSE.
  • The E2E mock daemon's live-state route drops activeWorkState, unlike the real route in packages/cli/src/serve/routes/session.ts. I patched the mock to forward it (identically in both arms) so the client sees the real payload shape. That 4-line passthrough is worth upstreaming — see N4.

2. Real qwen serve daemon, real turn

The matrix above scripts the daemon's answers. To rule out a dead switch at system level I also ran the whole stack for real: a real qwen serve daemon (built from the merge base — this PR does not touch packages/cli, so the daemon is byte-identical between arms), a real session, and a scripted OpenAI-compatible model that writes a Todo plan with an in_progress item, works for 30s, then answers. Both arms ran against that same daemon, one after the other, each with its own fresh session.

The daemon's own live-state payload, sampled once a second:

 before the turn   hasActivePrompt:false  activeWorkState:"unknown"
 t=0..29s          hasActivePrompt:true   activeWorkState:"active"
 t=30s..           hasActivePrompt:false  activeWorkState:"idle"

So the field is genuinely produced end to end — entryActiveWorkStateserve → store → hook → prop. Not a dead switch.

I sampled the icon every 300ms across the whole turn (≈125 samples per arm) so a flicker could not hide between two stills:

arm icon timeline
base 0ms = spinner — one state, never changes; still spinning 8s after the daemon reported the turn settled
head 0ms = spinner → 29948ms = staticexactly one transition, landing with the settle

real daemon

No flicker, no intermediate drop-out. That also answers the risk I was most worried about going in: the icon does not blink off and back on while the client's streamingState and the daemon's answer disagree during a turn.

One thing worth knowing for sizing the impact: todo_write is opt-in (tools.todoWrite.enabled, default false), so both the bug and this fix only reach users who turned it on. My first real-daemon run silently produced no plan at all until I enabled it.

3. Mutation controls

To show the matrix is not a tautology, I mutated the fix on head and re-ran:

Mutation Expected to break Actually broke
Drop && hasLiveActivity from TodoPanel D, E, F, G exactly D, E, F, G
Make the bridge return activeWorkState: undefined C, H exactly C, H

Both new lines in session-catalog-hooks.ts are load-bearing — the live path (C) and the catalog-fallback path (H) each have exactly one scenario that dies without them.

4. Gates run locally

Gate Result
vitest run (web-shell, head) 853 passed / 853
vitest run (web-shell, base) 853 passed / 853
tsc -p tsconfig.json --noEmit clean
eslint packages/web-shell --max-warnings 0 clean
prettier --experimental-cli --check packages/web-shell 1 file — pre-existing on the base, see N3

Findings — none blocking

N1 · The fix has no test coverage. Both arms run 853 tests: this PR adds none. App.test.tsx hard-codes activeWorkState: undefined in the mock, so sessionActiveWorkState === 'active' is never exercised, and TodoPanel.test.tsx never passes hasLiveActivity. A repo-wide grep finds activeWorkState in exactly one test file — that one mock line — so the field has no behavioural coverage anywhere in the stack (#11265#11267 → this).

Three cases in TodoPanel.test.tsx would close it for this PR. I ran them: they pass on the PR and the middle one fails when && hasLiveActivity is removed.

const todos: TodoItem[] = [
  { id: '1', status: 'in_progress', content: 'Apply the fix' },
];
const iconOf = (c: HTMLElement) =>
  c.querySelector('[role="tooltip"] > div')?.querySelector('span');

it('animates an in_progress item while work is live', () => {
  const icon = iconOf(render(<TodoPanel todos={todos} hasLiveActivity />));
  expect(icon?.firstElementChild).not.toBeNull();
});

it('keeps the static glyph when no live work is reported', () => {
  const icon = iconOf(render(<TodoPanel todos={todos} hasLiveActivity={false} />));
  expect(icon?.firstElementChild).toBeNull();
  expect(icon?.textContent).toBe('◐');
});

it('defaults to animating so existing callers are unaffected', () => {
  const icon = iconOf(render(<TodoPanel todos={todos} />));
  expect(icon?.firstElementChild).not.toBeNull();
});

N2 · unknown renders identically to idle. WebShellSidebar gives activeWorkState: 'unknown' its own affordance (sessionStatusUnknown, sidebar.activityUnknown), but the Todo panel collapses unknown into the static glyph, so a plan on a daemon that cannot answer looks settled. That is consistent with the PR's own principle — better than pretending execution is known — but the design doc paragraph added here says "confirms that execution is live" without saying what unknown does. One sentence there would close the gap.

N3 · The base carries a prettier violation that will fail the stack. packages/web-shell/client/components/sidebar/WebShellSidebar.tsx fails format:check. It is clean at origin/main and fails at 3dd860aafb, so it came in with #11267, not here — but format:check will go red as soon as the stack reaches a main-targeted PR. Worth fixing on #11267 while it is still open.

N4 · The E2E mock daemon is unfaithful on this field. client/e2e/utils/mockDaemon.ts builds its live-state response by hand and omits activeWorkState, which the real route forwards. Any future E2E written against that mock silently cannot see the field. Four lines, matching the real route's conditional spread.

N5 · The gate inherits the bridge's workspace-scoping blind spot. activePromptWorkspaceCwd is undefined when connection.sessionContext?.kind === 'live' and the number of trusted live workspaces is not exactly 1 (App.tsx:3261). In that configuration the bridge returns hasActivePrompt: false, activeWorkState: undefined, so the spinner falls back to streamingState alone — and the comment right above that hook says streamingState is exactly what drops to idle through silent tool gaps. This is pre-existing scoping that already governs the conversation indicator, so the Todo spinner is now merely consistent with it. My real-daemon run measured zero spurious transitions in the ordinary workspace configuration, and I did not build the multi-live-workspace configuration, so this is a code-path observation rather than a reproduction — not a reason to hold this PR. Flagging it because this PR is what makes it visible in a second surface.

Verdict

The change does exactly what it says, on every combination of the three signals, and each line of it is covered by a mutation that kills only the scenarios it should. From my side this is merge-ready; N1 is the one I would like to see land with it, since nothing in CI will notice if this regresses.

Screenshots, the raw per-scenario captures, the JSON result matrices and the whole harness (Playwright spec, the mock-daemon patch, the suggested test, and the real-daemon rig) are on assets-pr11269.

中文版

本地验证 —— 真实浏览器 + 真实 daemon

我在本地为这个 PR 搭了一套真实验证环境,按两臂 A/B 跑了一遍。下面每一条都是实测结果,不是静态阅读。

两臂 —— base = 3dd860aafb(本 PR 的 merge base,即 #11267 head)· head = c926de9df9(本 PR)。两臂之间只差本 PR 的 5 个文件。

先说一个前提:Qwen Code CI 只在 base 分支是 main / release/**pull_request 上触发(.github/workflows/ci.yml)。本 PR 的 base 是 codex/issue-11119-live-state,所以没有任何 CI job 编译、lint 或测试过这个 commit —— PR 上那些 check 全是 triage/路由类。下面这些是第一次真跑。

before and after

1. 行为矩阵 —— 真 Chromium,真 Web Shell 客户端

我在无头 Chromium 149 里驱动真实客户端(真 App、真 session-catalog-store、真 hooks、真 TodoPanel.module.css),对接一个线级 daemon。每个场景的判据直接读自浏览器:TodoPanel 是否渲染了 spinner 元素,再加 Element.getAnimations() 确认 todoPanelSpin 关键帧确实在 running —— 这样"渲染了但不动"的 spinner 不会被当成在转。

所有场景的初始状态相同:一段已经 settle 的 transcript,最后一次 todo_writeApply the fix 留在 in_progress

# 场景 live-state row base head
A 本地 turn 正在流式输出 hasActivePrompt:false, activeWorkState:idle 旋转 旋转
B 静默前台 turn hasActivePrompt:true 旋转 旋转
C 前台已结束,后台 hold activeWorkState:"active" 旋转 旋转
D 全部已结束 activeWorkState:"idle" 旋转 静态
E 旧 daemon,字段缺失 不带 activeWorkState 旋转 静态
F bridge 无法回答 activeWorkState:"unknown" 旋转 静态
G 无 live-state 能力,走 catalog row activeWorkState:"idle" 旋转 静态
H 无 live-state 能力,走 catalog row activeWorkState:"active" 旋转 旋转

head 8/8 符合预期;base 8/8 全部旋转 —— 这就是那个 bug,包括 D/E/F/G 这四个根本没有 live work 的情况。

scenario matrix

搭这套东西时有两个细节值得记一笔:

  • 场景 A 不能用一个残留的 running tool block 伪造。只要 daemon 的 promptStatus 是 idle,selectDaemonStreamingState 就返回 idle,所以这一臂是从 composer 真提交一个 prompt,plan 在 turn 中途通过 SSE 到达。
  • E2E mock daemon 的 live-state 路由会丢掉 activeWorkState,而真实路由(packages/cli/src/serve/routes/session.ts)是转发的。我给 mock 打了补丁把它透传出来(两臂完全一致),让客户端看到真实的 payload 形状。这 4 行值得合回仓库 —— 见 N4。

2. 真实 qwen serve daemon,真实 turn

上面那个矩阵是脚本化 daemon 的回答。为了排除"系统级死开关",我又把整条栈真跑了一遍:真的 qwen serve daemon(从 merge base 构建 —— 本 PR 不碰 packages/cli,所以两臂的 daemon 逐字节相同)、真的 session,加一个脚本化的 OpenAI 兼容模型:先写一份带 in_progress 项的 Todo 计划,工作 30 秒,然后作答。两臂先后跑在同一个 daemon 上,各自用全新的 session。

daemon 自己的 live-state 响应,每秒采样一次:

 turn 之前     hasActivePrompt:false  activeWorkState:"unknown"
 t=0..29s      hasActivePrompt:true   activeWorkState:"active"
 t=30s..       hasActivePrompt:false  activeWorkState:"idle"

也就是说这个字段确实是端到端产出的 —— entryActiveWorkStateserve → store → hook → prop,不是死开关。

我在整个 turn 期间每 300ms 采一次图标状态(每臂约 125 个采样),这样"闪烁"就藏不到两张静态截图之间:

图标时间线
base 0ms = spinner —— 只有一个状态,从不改变;daemon 报告 turn 结束 8 秒后仍在转
head 0ms = spinner → 29948ms = static —— 恰好一次跳变,与 settle 同步落下

real daemon

没有闪烁,也没有中途掉出。这同时回答了我事先最担心的风险:在 turn 期间客户端 streamingState 与 daemon 的回答不一致时,图标不会先灭再亮。

还有一点对判断影响面有用:todo_write 是 opt-in 的(tools.todoWrite.enabled,默认 false),所以这个 bug 和这个修复都只影响开了它的用户。我第一次真 daemon 跑的时候,没开这个开关,计划面板根本不出现,而且没有任何提示。

3. 变异对照

为了证明上面的矩阵不是自证,我在 head 上把修复变异掉再跑:

变异 预期挂掉 实际挂掉
TodoPanel 删掉 && hasLiveActivity D、E、F、G 恰好 D、E、F、G
让 bridge 返回 activeWorkState: undefined C、H 恰好 C、H

session-catalog-hooks.ts 里新增的两条线都是承重的 —— live 路径(C)和 catalog-fallback 路径(H)各自都有且只有一个场景在缺了它之后会死。

4. 本地跑过的门禁

门禁 结果
vitest run(web-shell,head) 853 通过 / 853
vitest run(web-shell,base) 853 通过 / 853
tsc -p tsconfig.json --noEmit 干净
eslint packages/web-shell --max-warnings 0 干净
prettier --experimental-cli --check packages/web-shell 1 个文件 —— base 上就有,见 N3

结论性意见 —— 没有阻塞项

N1 · 这个修复没有任何测试覆盖。 两臂都跑 853 个测试:本 PR 一个都没加。App.test.tsx 在 mock 里把 activeWorkState 写死成 undefined,所以 sessionActiveWorkState === 'active' 这条分支从未被执行;TodoPanel.test.tsx 也从没传过 hasLiveActivity。全仓 grep 下来,activeWorkState 只出现在一个测试文件里,就是那行 mock —— 也就是说整条栈(#11265#11267 → 本 PR)里这个字段没有任何行为级覆盖。

TodoPanel.test.tsx 里加三个 case 就能补上本 PR 这一段。我跑过:在本 PR 上三个全过,把 && hasLiveActivity 删掉之后中间那个会挂。

const todos: TodoItem[] = [
  { id: '1', status: 'in_progress', content: 'Apply the fix' },
];
const iconOf = (c: HTMLElement) =>
  c.querySelector('[role="tooltip"] > div')?.querySelector('span');

it('animates an in_progress item while work is live', () => {
  const icon = iconOf(render(<TodoPanel todos={todos} hasLiveActivity />));
  expect(icon?.firstElementChild).not.toBeNull();
});

it('keeps the static glyph when no live work is reported', () => {
  const icon = iconOf(render(<TodoPanel todos={todos} hasLiveActivity={false} />));
  expect(icon?.firstElementChild).toBeNull();
  expect(icon?.textContent).toBe('◐');
});

it('defaults to animating so existing callers are unaffected', () => {
  const icon = iconOf(render(<TodoPanel todos={todos} />));
  expect(icon?.firstElementChild).not.toBeNull();
});

N2 · unknownidle 渲染完全一样。 WebShellSidebaractiveWorkState: 'unknown' 单独留了表达(sessionStatusUnknownsidebar.activityUnknown),但 Todo 面板把 unknown 折叠成了静态图标 —— 于是在一个答不上来的 daemon 上,计划看起来像已经结束了。这跟本 PR 自己的原则是一致的(总好过假装"已知在执行"),但这里新增的设计文档段落只写了 "confirms that execution is live",没说 unknown 会怎样。补一句就够了。

N3 · base 上带着一个会让整条栈变红的 prettier 违规。 packages/web-shell/client/components/sidebar/WebShellSidebar.tsx 过不了 format:check。它在 origin/main 上是干净的,在 3dd860aafb 上挂 —— 所以是 #11267 带进来的,不是这里。但只要这条栈走到一个以 main 为 base 的 PR,format:check 就会红。趁 #11267 还开着,建议在那边修掉。

N4 · E2E mock daemon 在这个字段上不忠实。 client/e2e/utils/mockDaemon.ts 手工拼 live-state 响应时漏了 activeWorkState,而真实路由是转发的。今后任何基于这个 mock 写的 E2E 都会静默地看不见这个字段。补 4 行,跟真实路由的条件展开保持一致即可。

N5 · 这个门禁继承了 bridge 的 workspace 作用域盲区。connection.sessionContext?.kind === 'live' 且 trusted live workspace 的数量不是恰好 1 个时,activePromptWorkspaceCwdundefinedApp.tsx:3261)。这种配置下 bridge 返回 hasActivePrompt: false, activeWorkState: undefined,spinner 就只能靠 streamingState —— 而那个 hook 上面的注释恰好写着:streamingState 正是会在静默工具间隙掉到 idle 的那个量。这是既有的作用域问题,本来就管着 conversation indicator,所以 Todo spinner 现在只是跟它保持了一致。真 daemon 那一轮在普通 workspace 配置下测到的多余跳变是 0 次,而多 live workspace 的配置我没有搭,所以这是一条代码路径上的观察、不是复现 —— 不构成拦本 PR 的理由。之所以提,是因为本 PR 让这个盲区多了一个可见的表面。

结论

这个改动在三路信号的每一种组合上都做到了它声称的事情,而且它的每一行都有一个变异能且只能打死它该负责的场景。就我这边看,可以合。N1 我希望能跟着一起进来 —— 因为一旦回归,CI 里没有任何东西会发现。

截图、每个场景的原始截屏、JSON 结果矩阵,以及整套 harness(Playwright spec、mock-daemon 补丁、建议的测试、真 daemon 装置)都在 assets-pr11269


Verified locally with Claude Code (Claude Opus 5, 1M context) — Node v22.22.2, vitest 3.2.4, Playwright 1.61.1 / Chrome for Testing 149.0.7827.55, Linux x86_64.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants