feat(web-shell): keep a turn expanded while its background shell runs - #9632
Conversation
A turn that launches a background shell stays expanded while the shell is still running and collapses once it completes. Completion is detected from the background shell notification or a terminal snapshot of shell tasks, so a lost notification cannot pin the turn open forever. Promoted shells are also classified as background tasks so polling stays armed for the snapshot fallback. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Thanks for the PR! Template looks good ✓ Problem: real, not theoretical. The current collapse logic ( Direction: aligned. Web Shell already keeps turns open for background subagents; extending the same treatment to background shells (same keep-open semantics, same "only the active turn shows a live timer" rule) makes the two background mechanisms behave consistently. The claude-code CHANGELOG has no direct reference, but background-task UX is an actively improved area there (expanded task list fixes, background notification handling). Size: not applicable — no core paths touched. 102 production lines (App.tsx 27, toolClassification.ts 21, MessageList.tsx 54) plus 225 test lines. Approach: scope feels right. It mirrors the existing Risk: no elevated risk signals — no high-risk paths matched. Moving on to code review. 🔍 中文说明感谢贡献! 模板完整 ✓ 问题: 真实存在,不是理论问题。当前折叠逻辑( 方向: 对齐。Web Shell 已经会为后台 subagent 保持 turn 展开;把同样处理扩展到后台 shell(相同的保持展开语义、相同的"只有活动 turn 显示计时器"规则)让两种后台机制行为一致。claude-code CHANGELOG 没有直接对应条目,但后台任务 UX 是那边持续改进的领域。 规模: 不适用——未触及核心路径。102 行生产代码 + 225 行测试。 方案: 范围合理。复用既有 风险: 无升级风险信号——未命中高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
|
@qwen-code /takeover |
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管:autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突,直到移除标签或达到轮次上限。移除 |
Code reviewNo blockers found. Before reading the diff I sketched what this would need — a shell analogue of the existing background-subagent keep-open path, keyed by task id from the launch output, released by completion notification with a fallback for lost notifications — and the PR is exactly that shape, integrated the same way (
Nothing to flag under the "critical blocker or clear convention violation" bar — the Set-identity-preserving join/split memo in App.tsx even carries a comment explaining why. Testing evidence (PR's own CI, via API)CI on the reviewed commit was still running at review time — the ubuntu unit-test job and the web-shell visuals job are pending; everything completed so far is green or intentionally skipped. Per the triage rules I do not run PR code myself; the table below is the PR's own CI, and the finalize job will update it once CI settles. Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。 The author verified via unit tests on macOS only (author's claim — no E2E run, no screenshots). The new stay-open/snapshot/release tests exercise the changed function directly and would fail on base, so the logic itself is pinned; what remains substantiated only by mocked wiring is the live loop through a real daemon session. Sandboxed verification would settle that: 中文说明代码审查未发现阻塞问题。读 diff 之前我先独立构思了方案——给现有"后台 subagent 保持展开"路径加一个 shell 对应实现:从启动输出提取任务 id,用完成通知释放,并为丢失通知加兜底——PR 正是这个形态,且集成方式一致(
没有达到"阻塞或明确违反约定"标准的问题。 测试证据(PR 自身 CI,经 API 获取)审查时该 commit 的 CI 仍在运行——ubuntu 单测与 web-shell 视觉任务未完成;已完成的全绿或为正常跳过。按 triage 规则我不执行 PR 代码;下表为 PR 自身 CI,finalize 任务会在 CI 结束后更新。单测对改动函数的覆盖是有效的(新用例在 base 上会失败),但经真实 daemon 会话的端到端闭环只有 mock 接线覆盖。作者仅在 macOS 上跑单测验证(作者自述,未做 E2E、无截图)。可用 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean, minimal change that does exactly what it sets out to; the only reservations are that CI had not settled at review time and the live end-to-end loop is pinned only by mocked unit tests. Stepping back: this is the change I would have proposed. A turn that launches a background shell used to collapse the moment the next turn started, which is plainly wrong once you notice it — the subagent path already gets this right, and this PR ports that behavior to shells without inventing anything new. The diff is 102 production lines mirroring an existing pattern, the tests pin the new behavior (they would fail on base), and every assumption the code rests on — launch-text formats, notification payload shape, task-id format, polling lifecycle — checks out against the producing code, not just against the tests. The two reservations, named plainly: nobody has watched this work against a live daemon session (the author ran unit tests on macOS only), and the unit suite's Approval deferred until CI lands green on 中文说明置信度:4/5 —— 干净、最小的改动,完全达成目标;唯一的保留意见是审查时 CI 尚未结束,且端到端的真实闭环只有 mock 单测钉住。 退一步看:这正是我会提出的方案。启动后台 shell 的 turn 过去会在新 turn 开始时就折叠,一旦注意到就明显不合理——subagent 路径早已正确处理,本 PR 把该行为移植到 shell,没有新造任何东西。102 行生产代码复用既有模式,测试钉住了新行为(在 base 上会失败),代码依赖的每个假设——启动文案格式、通知负载结构、任务 id 格式、轮询生命周期——都与产出端代码核对过,而不只是与测试核对。 两点保留意见直说:没有人看着它在真实 daemon 会话里跑过(作者仅在 macOS 上跑了单测);审查完成时 ubuntu 单测与 web-shell 视觉任务仍在运行。前者可用上面的 批准延迟到 CI 在 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
chiga0
left a comment
There was a problem hiding this comment.
No blocking findings.
Checked:
- Three-place consistency:
terminalBackgroundShellTaskIdscorrectly threaded into the manualdependenciescache key, theapplyTurnCollapseoptions object, and theuseMemodependency array inMessageList. - Notification path (
completedBackgroundShellTaskIds): guards against malformeddataobjects and wrongkindvalues — matches the test matrix. - Snapshot fallback in
App.tsx: captures all non-runningshell tasks from the daemon;new Set(undefined)on first render is a safe empty set. BACKGROUND_SHELL_ID_PATTERN:^anchor is correct given these are server-generated strings with no prefix.isBackgroundTaskToolCallextension: correctly short-circuits for promoted shells (is_background: false+ promotion rawOutput).- CI: E2E Smoke ✅, Coverage ✅. SKIPPED checks are all review-automation, not build/test.
- Cross-check: existing review (qwen-code-ci-bot APPROVED) — no findings carried; mine align.
Minor (not must-fix): terminalBackgroundShellTaskIdsKey in App.tsx joins IDs without sorting. If the daemon returns the same task set in a different order across polls, the key changes → new Set with identical content → unnecessary re-render in MessageList. Collapse logic stays correct; only a performance edge case on unstable poll ordering.
Reviewed with AI assistance.
|
Released in v0.22.0. |
What this PR does
Web Shell keeps a conversation turn expanded while the background shell it launched is still running, and collapses it once that shell completes. This covers both shells started directly in the background (
is_background: true) and foreground commands promoted to the background, and matches the existing background-subagent handling: only the active turn shows a live timer, and older turns with pending background work stay open without one.Why it's needed
A turn that launches a long-running background shell used to collapse as soon as a newer turn started, hiding the shell's progress and the launch context. With this change the launch turn stays visible until the shell finishes, so the user can follow the background task without losing its place in the conversation. Completion is detected from the shell completion notification or, when that notification is lost, from a terminal snapshot of shell tasks, so a turn can never be pinned open forever.
Reviewer Test Plan
How to verify
npm testwithis_background: true), then start a second turn. The first turn must stay expanded with the background shell still running.toolClassification.test.tspins task-id extraction (both launch texts, case-insensitive), failed-call and non-shell rejection;MessageList.test.tspins stay-open, snapshot fallback, release-on-notification and malformed-notification guards;App.test.tsxpins the sessionTasks → MessageList wiring.Evidence (Before & After)
N/A — behavior verified by 651 unit tests in
packages/web-shell(MessageList, App, toolClassification); no screenshots captured.Tested on
Environment (optional)
npm run dev/ unit tests only (npx vitest run components/MessageList.test.ts App.test.tsx adapters/toolClassification.test.ts). No E2E run performed.Risk & Scope
liveStartedAtbehavior for non-active turns is unchanged from before this PR.terminalBackgroundShellTaskIdsprop onMessageListis optional and defaults toundefined, so existing consumers are unaffected.Linked Issues
N/A
中文说明
本 PR 做了什么
Web Shell 会在某个 turn 启动的后台 shell 仍在运行时保持该 turn 展开,直到该 shell 完成后再折叠。覆盖直接以
is_background: true启动的 shell 和前台命令提升为后台(promoted)两种情况,并与既有后台 subagent 行为保持一致:只有活动 turn 显示实时计时器,旧的、有待处理后台工作的 turn 保持展开但不显示计时器。为什么需要
启动长耗时后台 shell 的 turn 过去会在新 turn 开始后立即折叠,隐藏了 shell 的进度和启动上下文。本改动让启动 turn 在 shell 结束前保持可见,用户能跟随后台任务进度而不丢失对话中的位置。完成状态来自 shell 完成通知,通知丢失时则回退到 shell 任务的终态快照,因此 turn 不会被永久钉在展开状态。
Reviewer 测试计划
如何验证
is_background: true的npm test)的 turn,再开始第二个 turn——第一个 turn 在后台 shell 运行期间必须保持展开。toolClassification.test.ts钉住任务 id 提取(两种启动文案、大小写不敏感)、失败与非 shell 调用拒绝;MessageList.test.ts钉住保持展开、快照兜底、通知释放与畸形通知守卫;App.test.tsx钉住 sessionTasks → MessageList 的接线。前后对比证据
N/A——行为由
packages/web-shell的 651 个单元测试验证(MessageList、App、toolClassification);未截图。测试环境
macOS ✅(仅单元测试);Windows/Linux 未本地测试(CI 覆盖)。
风险与范围
liveStartedAt行为与 PR 之前完全一致。MessageList新增的terminalBackgroundShellTaskIdsprop 可选、默认undefined,现有接入方不受影响。关联 Issue
N/A