feat(web-shell): take back a cancelled prompt that produced nothing - #13488

评审基于当前提交 ff617efcd960885c6c151c3ef8ad98a4cad72114。发现 2 个需要修复的问题，建议处理后再合并。

1. [P1] 自动回退前，需要确认该轮输出已经同步。 App.tsx:17809–17817 仅根据浏览器当前的 transcript 判断“没有产出”，但取消和快照读取的 HTTP 响应不保证此前的 SSE 已被浏览器处理。如果 daemon 已产生回答或执行了工具，而对应事件延迟到达，这里仍会把该轮判为空并自动回退。测试中把 assistant 事件延迟到取消及快照读取之后，实际仍调用了 rewindSession，随后真实 transcript reducer 将回答随该轮删除。服务端的 rewindToTurn 只要求轮次已空闲，并不会再验证“没有产出”，因此本地判断失真时无法保护模型历史；工具副作用也不会被 rewindFiles: false 撤销。需要先同步到该取消轮次的确定事件边界，或由服务端原子验证撤回条件，不能将“尚未收到输出”当作“未产生输出”。

2. [P2] 自动撤回完成并应用回退事件前，阻止新 prompt 提交。 App.tsx:17821–17844 先恢复输入框，再异步读取快照和回退，全程没有阻塞提交。用 deferred promise 暂停快照读取后立即发送修正 prompt，实际发送次数已经从 1 变为 2，之后最新消息检查使回退被跳过，错误 prompt 继续留在历史里。另一个测试让回退 HTTP 先成功、延迟 session_rewound：修正 prompt 已添加到本地 transcript 后，真实 SDK reducer 收到旧轮次的回退事件，会把修正 prompt 一并删掉，用户消息从 ["first", "oops typo", "corrected prompt"] 变成 ["first"]。这里需要覆盖整个自动撤回过程的提交阻塞，并等待回退事件应用；现有内联编辑路径的 waitForRewindApplied / pendingEditRewind 已处理同类同步问题。

验证范围：npm run build、npm run typecheck 均通过；针对改动文件运行的原有测试共 5 个文件、1852 个用例通过。额外的竞态复现使用当前提交的真实 App 和 SDK transcript reducer，mock session I/O 以确定性控制 HTTP/SSE 的先后顺序；上述 3 个安全性断言均失败。它们是测试脚本复现，不是对真实模型/daemon 的网络延迟 E2E 实测；服务端回退行为另经源码核对。

Verdict: merge-ready — 105/105 scripted assertions passed at head ff617efcd960885c6c151c3ef8ad98a4cad72114

Independent maintainer verification of #13488 on a fresh machine (Orange Pi 5, aarch64 Linux, Node v24.13.0, pnpm 11.24.0, Chromium via Playwright 1.61.1). Both arms built from scratch: head ff617ef and base 69d5db2 (the PR's baseRefOid), each served by its own real qwen serve bundle with an isolated QWEN_HOME, driven by real Chromium through the real Web Shell against a scripted OpenAI-compatible model that logs every request it receives.

Central claim and A/B proof

Claim: cancelling a composer prompt before the turn produced anything (double-Esc / stop button) returns the prompt to the composer and rewinds the turn out of the session; everything that already produced output, has a draft, a queue, or a foreign owner keeps today's plain stop.

The harness (harness/e2e.cjs, 14 scenarios) ran identically against both arms; harness/check-results.mjs encodes per-arm expectations (base cells assert the broken behaviour — a base red cell is a pass). Witness: 04-assertions-head.png (50/50), 05-assertions-base.png (43/43).

Test efficacy (vacuity check)

The PR adds 306 lines to App.test.tsx plus three smaller suites. The load-bearing guard cancelledTurnProducedNothing was mutated both ways (06-mutation-matrix.png):

Both mutations die against the new tests; the restored source is green. The tests pin the guard.

Targeted gates (head ff617ef)

Findings

None blocking. One observation, identical on both arms and matching the author's published results: a pasted image rides the wire as an inline [image: image/png] text marker rather than an image_url part, so the "resend carries the image once" claim was verified in that form. Not introduced by this PR.

Not covered

• SSH workspaces (rewind is rejected there; the composer-restore fallback is unit-tested but not run against a real remote — same gap the author declared).
• Split-view ChatPane keeps the plain stop by design (declared out of scope by the PR).
• Daemon-API timing probe (the PR's "rewind lands 27/27 immediately after cancel" claim) — not re-run as raw HTTP; the e2e S11 scenario (Enter, Esc, Esc with no pause) covers the same race through the real client and passed on head.
• @smoke Playwright mock-daemon suite — CI ran it green; my real-daemon A/B is the stronger evidence for this feature.
• macOS/Windows — browser-side only change, aarch64 Linux verified; other desktops left to CI per the PR.

Methodology

Two git worktrees at ff617ef (head) and 69d5db2 (base). Head: corepack pnpm install --frozen-lockfile (13m22s, cold store) + npm run build + npm run bundle. Base: node_modules hardlinked from head (cp -al; the PR touches no package.json/lockfile), then full build + bundle. Control cleanliness was asserted, not assumed: readlink -f base-tree/node_modules/@qwen-code/{qwen-code-core,web-shell} resolves into the base tree. Each arm ran node dist/cli.js serve --port 18932 --token rigtoken with an isolated QWEN_HOME (auth openai + modelProviders.openai registering fake-model — without the registry entry the daemon rejects the client's POST /session/:id/model and a "Set model failed" toast pollutes the no-toast assertions; environmental, unrelated to the PR). The scripted model (harness/fake-model.cjs) keys behaviour off the last user text (HOLD stalls, THINK streams only reasoning, SLOWTEXT streams partial text, TOOL calls a tool) and logs every request's full user-message list — the wire oracle for the merged-prompt claim. Chromium 1.61.1 headless on aarch64. Harness scripts, raw per-scenario JSON (out-head/, out-base/), console logs and screenshots are in the artifact directory. Raw results diff cleanly against the author's published results-main.json / results-this-pr.json where scenarios overlap.

Machine: Orange Pi 5 (RK3588S, 8 cores), Armbian Linux aarch64, Node v24.13.0, pnpm 11.24.0.

Both points are right, and both are fixed in d65f8b171f (pushed; PR body updated to match). Thanks for constructing the races with the real App and reducer — the two new scenarios below are the real-stack versions of them.

P1 — decide on the turn's terminal event, not when the cancel returns. The take-back no longer runs from cancel()'s promise. handleCancel only records the prompt it is cancelling; the decision runs from the prompt settlement bus (useDaemonPromptSettled), i.e. on the daemon's turn_complete / turn_error for that prompt. The provider flushes every buffered transcript block before it publishes a settlement, and the bridge publishes the terminal frame only after the agent has wound down, so by the time the "produced nothing" check runs the transcript holds everything the turn produced. A turn that completed or failed on its own keeps its result (outcome !== 'cancelled'), and a settlement that takes longer than 10 s lapses.

The settled turn is matched by the admitted prompt id, which onAdmitted now carries. One case needed more: Enter → Esc → Esc with no pause cancels before the admission response reaches the tab, yet the daemon has usually taken the prompt (your P1 scenario in reverse — the tab knows nothing about a turn that exists). The terminal frame's originatorClientId identifies the tab's own turn there; DaemonPromptSettledEvent now carries it (internal field; the host projection in assistant-turn-settlement.ts is unchanged). An intermediate build that matched by prompt id only regressed exactly this scenario on the real stack (turn kept, resend merged) — kept as results-round2-promptid-only-build.json.

P2 — hold prompts until the rewind has landed. The take-back sets the pending-rewind state before it reads the snapshots and leaves it set until the transcript shows the rewind (the same sessionWriteBlocked the inline edit path drives; pendingEditRewind is now pendingRewind, shared by both). A daemon refusal (DaemonHttpError, e.g. an SSH workspace) releases it immediately; no rewind issued releases it; an unknown failure gives the event up to 2 s and then releases. Your two shapes — correction while the snapshot read is deferred, and correction between the rewind's HTTP response and session_rewound — are now unit tests (holding prompts back while the turn is taken back): the correction is refused (returns false, draft stays) and writeBlocked is observed true, then false once the turn is gone. Enter during the hold is refused rather than queued; locally the hold is tens of milliseconds.

Verification. Real daemon + real Chromium, same harness as before, client built from d65f8b171f (round2):

• Answer lands as the cancel does — scripted model answers at 900 ms, Esc Esc at 700–1200 ms, 10 runs: 0 violations (7 runs where the model had delivered → answer and prompt kept; 3 → taken back).
• Correction sent ~130 ms after the prompt is back — sent once, not merged, not lost; model receives ["hello first", "fixed prompt S16"], same after reload.
• Enter, Esc, Esc with no pause — taken back via the originator stamp; wire clean.
• The 14 earlier scenarios unchanged (12/12 repeated take-backs, observer tab, attachments, all plain-stop cases).
• Unit: the two race shapes plus settlement outcome/timeout/other-prompt/other-client/write-blocked/refusal/unknown-failure cases; packages/web-shell 412 files / 10818 tests green; 28 guard mutations each killed by a new test, dropping the originator stamp fails the provider's new test.
• One caveat, stated in the PR: a single stop-button run in the first full pass stopped without taking back (no snapshot read after the terminal in the daemon log) and did not recur in a second full pass, 20 isolated runs, or 4 back-to-back runs; raw result kept.

重新验证当前 head d65f8b171f0a2e7cd299d8ab987ed32c75186ccd 后：P1 已修复；P2 的正常路径已修复，但未知失败路径仍可复现，尚不能标记为完全解决。

• P1：已修复。 判定现在等待对应轮次的 settlement，provider 在发布前先 flush transcript。独立测试分别让 assistant 和 tool 输出晚于 cancel HTTP、早于 terminal 到达，均保留输出，快照读取和回退调用均为 0。
• P2：正常路径已修复。 快照读取挂起、回退 HTTP 已成功但 SSE 未到时，修正 prompt 均被阻止提交；即使成功响应后等待超过 2 秒，阻塞仍保持。应用真实 SDK 的回退事件后再提交，历史正确保留 ["first", "corrected prompt"]。

仍需修复 [P2]：未知回退失败后，不能仅因等待 2 秒就解除提交阻塞。 App.tsx:17909–17917 忽略 waitForRewindApplied() 返回的 false，随后 finally 无条件释放 pending 状态。网络错误或客户端超时并不能证明 daemon 没有执行回退：客户端 timeout 只竞争 promise，bridge 也明确说明已发出的回退无法取消。因此原来的迟到事件删掉新消息的问题只是延后了 2 秒。

独立复现：令回退请求以 TypeError('fetch failed') 拒绝，推进 fake timer 2100 ms，发送修正 prompt，再应用迟到的真实 SDK session.rewound 事件。实际结果：

blocked: true -> false
sendPrompt calls before rewind event: 2
user texts before event: ["first", "oops typo", "corrected prompt"]
user texts after event:  ["first"]

需要在结果未知时继续阻止新 prompt，直到回退事件应用或通过会话重载/权威状态确认结果；等待超时本身不应当作为安全解锁的依据。现有未知失败测试只覆盖事件在等待窗口内到达，建议补上窗口过期后才到达的用例。

本次验证：npm run build、npm run typecheck 通过；改动文件相关原有测试 5 个文件 / 1865 个用例全部通过；额外独立时序测试 6 通过、1 失败，失败即上述残留问题。时序复现使用真实 App 和 SDK reducer、mock session I/O，未把它宣称为真实 daemon 网络故障 E2E。CI 产品测试与 E2E 均绿；机器人评审及 fallback 任务失败的日志为其 GitHub 账号 HTTP 403（account suspended），与本次代码测试失败无关。

🖼️ web-shell visual preview

Rendered against a mock daemon (no real backend): the PR base vs this PR head 16bd737. Only screenshots that changed are shown (flows below, if any, are head-only) — refreshes on every push.

Screenshots · before / after

ℹ️ No screenshot changed against the PR base — but this PR edits 2 render-shaping files:

• packages/web-shell/client/App.tsx
• packages/web-shell/client/daemon/session/DaemonSessionProvider.tsx

Either the change has no visual effect (logic, plumbing, a state the scenarios never reach), or no scenario renders this UI — in which case the preview cannot see it, and an empty result is a coverage gap rather than a clean bill of health. To make it visible, add a scenario to packages/web-shell/client/e2e/visuals/screenshots.spec.ts that seeds whatever state the UI is gated on; it then appears here as a head-only (NEW) capture.

_{Full-resolution recordings (.webm) are attached to the workflow run.}

— Qwen Code · web-shell visuals

Agreed — the 2 s wait was a timer standing in for something the tab could not know. Fixed in ccdac0bdf5 (pushed; PR body updated).

What changes. After a rewind call fails for any reason other than a daemon refusal, the hold now stays until the daemon has answered, never until a timer runs out. The daemon lists a turn's snapshot in GET /rewind/snapshots only while that turn is in its API history (getRewindableUserTurnCount() is computed from the live history), and a snapshot id is never reused (Session.turn is monotonic across rewinds), so re-reading the listing is the authority:

• target still listed → no rewind happened → prompts are released;
• target gone → the rewind landed → the hold lasts until the transcript shows it (session_rewound, or the reconnect replay);
• listing unreachable → ask again (0.5 s, 1 s, 2 s, …) for as long as the hold stands.

"Still listed" has to be seen twice: one reading can be answered from the same read of the agent's pipe as a rewind still queued there, before that rewind truncates — the second reading is only sent after the first was answered. Two smaller things fell out of it: a failed snapshot read before any rewind was issued releases at once (nothing can land), and the loop stops as soon as the hold has lifted.

Your scenario is a unit test now — rewind rejects with TypeError('fetch failed'), the daemon's listing no longer has the turn, the clock is moved 2.1 s, the correction is refused, then the late session_rewound lifts the hold and the correction goes out once (holds prompts until the event arrives when the daemon no longer lists the turn). Next to it: released once the daemon has twice listed the turn as still there; keeps asking while the daemon cannot be reached; stops asking once the transcript shows the rewind; releases when the snapshots cannot be read because no rewind was issued. 11 mutations of the outcome check: 10 fail a test, the one survivor only flattens the retry backoff.

One note for the harness: a daemon that goes on listing the turn and later emits session_rewound for it cannot exist — the listing is derived from the history the rewind truncates. If getRewindSnapshots is mocked to return the pre-rewind listing after the failed call, the fix will (correctly) release; with the listing truncated the hold stays.

Real stack (Playwright route interception in front of the real daemon, the composer's contenteditable polled to time the hold; raw results in pr-13488/round3):

• request lost before the daemon: asked at +41 ms and +546 ms after the restore, listed both times, released at +565 ms; text typed and Enter pressed during the hold changed and sent nothing;
• daemon applied the rewind, browser lost the response: 200 at +34 ms, the one reading at +41 ms no longer listed the turn, so the daemon's answer released nothing; the hold lifted with the event at +91 ms; model receives ["hello first", "fixed prompt S17b"];
• request lost and the daemon unreachable for the next two readings: failed at +32 ms and +535 ms, answered at +1540 ms and +2046 ms, released at +2064 ms — past the old timer, and only after two answers.

Two things the PR body now states plainly. During the hold the composer is read-only (the shared sessionWriteBlocked drives EditorView.editable), so typing is dropped rather than refused at Enter — tens of ms on the normal path, about half a second after a lost request. And the residual: a rewind the daemon still has queued when both readings are answered would land after the release; that takes the daemon sitting on an admitted rewind across that exchange while answering status reads. Serving the listing through the same history-mutation gate on the daemon would close it; I'd do that as a follow-up rather than widen this PR, if you think it is worth closing.

The 16 earlier scenarios were re-run twice on the new build. In the first pass one of the twelve take-backs in a row (a reasoning-only turn) stopped without being taken back — a plain stop; the later rounds were taken back and the final prompt reached the model merged with that kept turn, as on main. It did not recur in the second full pass or in 9 isolated runs (108/108 rounds); kept as results-this-pr-round3-first-pass.json, and noted in the PR body next to the round-2 stop-button miss.

重新验证当前 head ccdac0bdf5668882b9790b9b983768dde58f8104：P1 仍已修复；P2 在回退已执行、事件迟到的路径已修复，但回退仍排队时的问题可以复现，尚不能标记为完全解决。

[P2] 两次快照中仍有目标，不能证明已接纳的回退不会再执行。 App.tsx:996–998 在连续两次看到目标时返回 true，随后 17971–17979 解除提交阻塞。实际 bridge 中，回退排在 entry.promptQueue 上，而 快照查询直接发送 ACP status 请求，不等待该队列。因此两次回答只能证明“目前尚未截断”，不能证明“这次回退已确定不会执行”。

这次先用生产 bridge 和内存 ACP 测试通道验证后台时序，再用真实 App 与 SDK reducer 复现界面影响：

1. 挂起一个合法的并发 branch 操作，接纳其后面的 rewind；branch 不修改源会话快照。
2. 两次快照读取相隔 550 ms，均返回目标；断言此时 rewind 尚未派发。
3. 前端的 rewind 请求以未知网络错误结束后，按上述两次回答解除阻塞，接受修正消息。
4. 放行 branch，生产 bridge 随后执行 rewind、发布 session_rewound，最后执行修正 prompt。App 应用迟到的真实 SDK 回退事件时，修正消息被一并从界面历史中删除。

heldInitially: true
heldAfterBothReads: false
sendPrompt calls: 2
user texts before event: ["first", "oops typo", "corrected prompt"]
user texts after event:  ["first"]
AssertionError: expected [ 'first' ] to include 'corrected prompt'

两次“仍有目标”的回答都发生在回退执行之前，不存在“已经截断却继续返回旧快照”的假设。后台可继续执行修正 prompt；这里验证的缺陷是其对应用户消息在当前界面消失。这个残留属于原 P2 的正确性问题，建议在本 PR 中关闭：解除阻塞须有已接纳回退完成或确定不再执行的确认，状态查询须与该操作正确排序。确认机制需要覆盖尚在 bridge 队列中、还未派发给 agent 的回退。

已通过的独立复测包括：延迟 assistant/tool 输出仍保留；正常回退期间阻止提交；目标已消失时等待 10 秒仍不放行，直到事件应用；请求确实未送达时两次查询后放行；查询失败重试；发起回退前读取失败立即放行。

验证结果：npm run build、npm run typecheck 通过；相关原有测试 5 文件 / 1871 用例全部通过；额外验证 9 个案例，8 通过、1 失败，失败即上面的界面历史断言。后台验证使用真实 bridge 队列与内存 ACP 通道，前端验证使用真实 App/reducer 与 mock session I/O；本次没有把它宣称为完整 daemon + 浏览器网络故障 E2E。

Right — two readings could only ever show "not truncated yet", not "will not run"; the ordering has to live where the queue is. Closed in this PR at the bridge, in 16bd737bd3 (pushed; PR body updated).

What changes. packages/acp-bridge/src/session-control-plane.ts: the session entry now keeps rewindTail, the settled state of the last admitted rewind (set synchronously at admission, next to the promptQueue chain, always resolving). getRewindSnapshots awaits it before sending the status request to the agent. So a listing requested while an admitted rewind is still queued behind a branch is answered only after the branch has released and the rewind has run — and since the agent processes the listing after it has responded to the rewind, the agent side needs nothing. Nothing else waits on it: prompts, branches and other status reads are unchanged, and a rewind that fails does not block later listings.

Tests. Two bridge tests on the in-memory ACP channel, the shape you used: a branch gated open, a rewind admitted behind it, getRewindSnapshots called meanwhile — not answered for as long as the branch holds (50 ms probe), then answered after both ran, with the agent seeing session/branch, session/rewind, rewind_snapshots in that order; and a failed rewind followed by a listing that is still answered. Removing the await fails the first test. packages/acp-bridge: 54 files / 2597 tests green; packages/web-shell: 412 files / 10822 green; the Web Shell side is unchanged apart from the comment on rewindMissed.

Real stack, bundle rebuilt from 16bd737bd3: all 19 scenarios as before (twelve take-backs in a row 12/12, answer-lands-as-the-cancel-does 0 violations, request lost released at +571 ms after two listings, response lost held until the event, daemon unreachable released at +2063 ms after two answers). Raw results in pr-13488/round4.

The two readings stay as the client-side half: the first can be answered while the daemon is still reading the rewind's own request (same event-loop wake-up, body not yet parsed), the second is sent only after the first came back. What that leaves is a rewind request reaching the daemon more than half a second after the browser reported it failed — a timed-out request has been at the daemon for 30 s by then, and one the browser aborted is either already there or never arrives. Stated as such in the PR body.

重新验证当前 head 16bd737bd3be827e4d4c312d71e30b0d0829a675：此前报告的 P1、P2 均已通过复测，本轮未发现新的阻塞问题。

• P1 保持修复。 独立测试再次让 assistant/tool 输出晚于 cancel HTTP、早于 settlement 到达，输出均被保留，未触发回退。
• P2 的排队路径已修复。 rewindTail 在回退接纳时与 promptQueue 同步设置，快照查询先等待该回退完成，因此上轮“已接纳回退仍排队，两次读取却提前返回旧目标”的时序已被阻断。

本次将生产 App 的快照查询、回退和更正提交直接接入新构建的生产 bridge，以内存 ACP 通道控制 branch 完成时机；模拟调用方丢失回退 HTTP 结果，并延迟向 App 投递 bridge 实际发布的回退事件。结果：

1. branch 挂起、rewind 已接纳时，等待 550 ms 后，结果确认查询仍挂起，修正消息未提交。
2. branch 放行、rewind 完成后，查询返回目标已消失；App 仍保持阻塞，直到真实 SDK reducer 应用 session_rewound。
3. 此后修正消息只提交一次，并通过 bridge 执行；最终用户消息为 ["first", "corrected prompt"]。

额外覆盖了已派发但未完成的回退、失败回退后的查询、多个已接纳回退、其他状态查询与另一会话隔离，以及前几轮的迟到输出、快照/事件等待、目标消失后等待 10 秒、查询失败重试、未发起回退时读取失败等路径。独立验证 13/13 通过（bridge 5、App 8）。

常规验证也通过：根目录 npm run build、npm run typecheck；原有 bridge 测试 994/994；Web Shell 相关原有测试 5 文件 / 1871 用例。已检查新增字段的全部读写及回退弹窗、内联编辑、自动撤回、VS Code 编辑、SDK 和会话所属 runtime 路由的调用链。

验证范围：上述联合测试使用真实 App、bridge、SDK reducer 和内存 ACP 通道，agent 操作与 HTTP 结果丢失由测试夹具控制；本次未额外运行完整浏览器网络故障 E2E。

发评时 CI 尚有 Web Shell E2E、视觉预览等任务排队或运行中；以上通过数均为本地实际执行结果。

@qwen-code /triage

Verdict: merge-ready — 128/128 scripted assertions passed at head ccdac0bdf5668882b9790b9b983768dde58f8104

Follow-up round (4th) of maintainer verification, run on macOS/Intel, Node v25.8.0, Chromium via Playwright 1.61.1. Three arms built from scratch here: base 9cdb0f38 (the PR's base, also the merge-base with current main), head ccdac0bdf5, and intermediate = the head tree with only this round's commit's App.tsx reverted to d65f8b171f (sha256-verified overlay and byte-for-byte restore), so any head↔intermediate difference is attributable to ccdac0bdf5 alone. Every arm is a real qwen serve daemon serving its own built Web Shell bundle to real headless Chromium against a scripted OpenAI-compatible model that logs every request's user-message parts. Nothing about the daemon, client, or browser is mocked.

Previous findings — status at the new head

Central claim, A/B against base

P0 matters because the new client logic resolves an unknown rewind outcome by re-reading GET /session/:id/rewind/snapshots, which is only sound if that listing follows live history and never reuses an id — the two facts the unit tests mock. Measured against the real daemon on both arms: they hold.

The delta commit, isolated: hold A/B head vs intermediate

F2 (daemon applied the rewind, browser lost the response) is a safety cell, not a discriminator: the live event stream delivers session_rewound within ~100 ms, so both arms end with ["hello first","fixed prompt F2"], unmerged, surviving a reload.

Test efficacy

Control green (30 + 8). Six of seven guard mutations killed, each red test named: M0 positive control (2 red), M2/M3 (releases prompts once the daemon has twice listed the turn as still there), M4 (stops asking once the transcript shows the rewind), M5 (releases prompts when the snapshots cannot be read), M7 (2 red). Survivors: M6 (flatten the retry backoff — the author-disclosed one) and M1, discussed below.

Targeted gates

cancelledTurn.test.ts 8/8 · App.test.tsx -t "cancelling a prompt before it produced anything" 30 passed · DaemonSessionProvider.test.tsx (recovery/originator) 4 passed · actions.test.ts -t "failed rewind calls" 2 passed · web-shell tsc --noEmit clean. Full suite/lint left to CI: Test (ubuntu-latest, Node 22.x) pass; note web-shell E2E Smoke was still pending when this was posted.

Findings (non-blocking)

1. Survivor M1 is behaviour-neutral but untested. Deleting if (listed === false) return false; from rewindMissed left all 30 take-back tests green. Tracing every reachable path shows both variants end in rewound = true waiting for the transcript; the clause only stops the polling loop early, so it bounds polling traffic rather than deciding an outcome. Coverage gap on an optimisation, not a defect — a test counting listing reads after the transcript lifts would pin it. Suggestion only.
2. The hold has no in-UI explanation or escape when the daemon never answers. D1: composer read-only 12+ s, correction refused, recovery only via switching session (by construction, rewindSyncBlocked is session-scoped) or reload (measured). This is the stated design and the safe side of the trade, but a user staring at a frozen composer gets no hint why. Suggestion: surface the pending-rewind state in the placeholder or a toast.
3. The disclosed residual is real but narrow; its three doors, from source. requestSessionStatus (the listing) calls extMethod directly (session-control-plane.ts:6874-6882) and bypasses entry.promptQueue, while rewindSession chains onto it (:15432). Queue occupants that can hold a rewind >500 ms without tripping the admission guard (pendingPromptCount/promptActive/backgroundTurn) are branchSession (:12402), the cwd-change/transfer chain (:12459), and the quarantine-gated operation at :15202. Not constructed end-to-end (needs daemon-side fault injection); the proposed follow-up of serving the listing through the same history-mutation gate would close it.

Not covered

SSH workspaces and split-view ChatPane (same gaps declared by the author); a real-stack reasoning-only turn (the scripted model's reasoning_content produced no transcript blocks in this stack, so S2 measured "nothing recorded" — the thoughts rule is covered by cancelledTurn.test.ts and pinned by M0); the delayed-session_rewound window on the real stack (unit test + P0 premise instead); the residual race end-to-end; round-1 scenarios not re-measured at this head (12-in-a-row, pasted image, observer tab, other-tab cancel, reload-mid-turn — the delta touches only the unknown-failure path); the full web-shell suite, lint and @smoke locally; non-macOS browsers.

Methodology

Detached worktrees at base/head plus an overlay arm; head installed with corepack pnpm install --frozen-lockfile, base reusing it via cp -al (no lockfile change) with readlink -f asserting workspace links resolve into the base tree; bundles proven to carry pre-fix source (cancelledTurn.ts absent from base, rewindMissed absent from the intermediate bundle). Oracles: the daemon's /session/:id/transcript, its lifecycle log markers (prompt enqueued, prompt turn completed, cancel sent, session rewind completed, rewind snapshots loaded), the scripted model's per-request user-part log, and the composer's contenteditable/text. Faults via Playwright route interception on the two rewind routes (separate globs — one pattern silently misses the listing). Assertions in harness/check.cjs, written from the PR's claims before the runs were inspected; base/intermediate cells asserting the broken behaviour count as passes. Harness bugs found and fixed on my side, none in the PR: Node 25 marks a fully-read request destroyed (truncated every scripted SSE response after its first frame); the CodeMirror placeholder reads as composer text when empty; the hold timer first mis-read "not yet engaged" as "released". Raw logs, wire logs, per-scenario JSON, gate logs and the mutation matrix are in the run artifacts.

✅ Qwen Triage finished — view run. See the stage comments in this thread for the result.

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

Thanks for the PR — one thing up front: this merged at 15:35 UTC on 2026-10-06 (merge commit 28e0f3a8, two human approvals already standing) while this triage run was still in flight. The /triage request went in at 14:58, 37 minutes before the merge. So this is a retrospective gate pass — nothing below gated anything, and no approve/request-changes action is possible or meaningful on a merged PR.

Template looks good ✓ — every heading is present and actually filled in, including a real reviewer test plan, a per-scenario before/after table, and a complete Chinese translation.

Problem: observed, not theoretical. The PR measures main against a real daemon with a scripted model and shows the concrete consequence rather than asserting one: the corrected prompt reaches the model merged with the mistaken one — the author reports the model receiving "HOLD oops typo S1\nfixed prompt S1" as a single user message — plus an "interrupted · Continue execution" banner offering to resume a turn the user just deliberately abandoned. There's no linked issue, but there is a reproduction with before/after artifacts and raw harness output, and a parity argument that doesn't depend on them: the TUI already restores a cancelled prompt (AppContainer.tsx), so the two surfaces disagreed about what Esc Esc means. That clears the problem-existence bar comfortably.

Direction: aligned. Esc Esc is a core gesture, and "cancelling a prompt you just mistyped should not leave it in the model's history" is a user-facing correctness gap, not a solution looking for a problem. claude-code's CHANGELOG has no entry for restoring a cancelled prompt to the composer, but Esc Esc / rewind / interrupted-turn handling is an actively fixed area there (rewind-menu responsiveness, the Interrupted-row hint, stop semantics for queued messages), so the direction matches where the reference product is going.

Size: the change spans two packages (acp-bridge, web-shell), which brings it under the cross-package clause of the core-module gate. It's clear on both counts regardless — the author has admin on this repo, so the two-tier gate doesn't apply, and at 416 production lines against 1032 test lines it's under the 500-line escalation threshold and well under the 1000-line advisory. Breakdown: 22 production lines in acp-bridge (bridge types + session control plane), 394 in web-shell, of which 325 sit in App.tsx. The test-to-production ratio (~2.5:1) is worth naming on its own — this is not a PR that shipped a concurrency change untested.

Approach: the scope feels right, and it reuses rather than invents. The prompt hold is the same mechanism the inline edit-and-resend path already used, generalized and renamed (pendingEditRewind → pendingRewind) instead of duplicated, and the composer-restore sequence mirrors that existing path. Deciding at the turn's terminal event rather than at cancel time is the correct call and the harder one — the round-2 evidence in the PR body is exactly the case that proves it, where output still in flight when the cancel landed would otherwise be dropped.

One honest architectural question, offered as a question and not a blocker: this places "a cancelled turn that produced nothing gets dropped" in the client, which is why correctness needs four cooperating pieces — snapshot listing, a prompt hold, an originator stamp on the settlement, and a bridge-side ordering fix so a listing can't be answered ahead of a rewind it has already admitted. The daemon owns the history and already stamps terminal frames with the submitting client, so it could own the decision and every client would get it by construction. The counter-argument is equally real: the TUI already does its own restore, so moving this daemon-side would change behaviour for a second client that didn't ask for it. Worth revisiting if a third surface ever needs the same semantics — three client-side copies of a race-prone state machine is where this gets expensive.

Risk: no elevated risk signals. None of the changed files match the high-risk paths from the repo's revert-history analysis (shell.ts, mcp-client, geminiChat, acpConnection, sandbox/relaunch, and friends).

Merged, so there's nothing to gate. Code review findings and the CI evidence are in the next comment. 🔍

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

_{Reviewed at 16bd737bd3be827e4d4c312d71e30b0d0829a675 · re-run with @qwen-code /triage}

Retrospective review — the PR merged before this run posted (see the Stage 1 comment). Static review only: per this gate's rules I did not build, run, or execute anything from the PR's tree. The testing evidence below is the PR's own CI, read through the API. Nothing here was verified by driving the product.

Code review

The interesting risk in this change is not the feature, it's the concurrency — a client-side rewind that can drop a turn it shouldn't, or a hold that never lifts. I went after those specifically, against the pre-PR tree rather than the PR's own description. Three things I expected to be problems are not:

The new SessionNotFoundError throw in getRewindSnapshots is not a new error path. The added early guard looks like it changes what an unknown session returns, but requestSessionStatus — the function the call previously fell straight through to — already throws exactly new SessionNotFoundError(sessionId) as its first statement (session-control-plane.ts:6870). The guard exists because the code now has to read entry.rewindTail before delegating. Error semantics for the HTTP route (serve/routes/session.ts:10212) and the VS Code embedded caller are unchanged.

await entry.rewindTail cannot park a snapshot listing behind a long turn. This was my main deadlock/latency worry: rewinds queue on entry.promptQueue, so if a listing had to wait out a queued rewind, and a rewind could sit behind a running prompt, opening the rewind picker mid-turn would hang. It can't — rewindSession rejects synchronously with SessionBusyError when pendingPromptCount > 0 || promptActive || backgroundTurn (session-control-plane.ts:15421-15430), so a rewind is only ever admitted into an idle queue. rewindTail is also rewindResult.then(() => undefined, () => undefined) — it swallows both outcomes, so a failed rewind can't wedge every later listing, which is what its own doc comment claims. And the client bounds the wait anyway via withActionTimeout. The ordering fix does what it says without introducing a stall.

No dead switches. Both new option/field additions have live producers and consumers, which is the failure mode I check for first: originatorClientId is genuinely stamped upstream on the bridge envelope (eventBus.ts:96, set at session-control-plane.ts:1378 and :1416), flows through promptSettledFromTurnEvent into the settlement event, and is read by settlesCancelledPrompt — so the "admission response never reached the tab" fallback is real, not decorative. The new silent flags follow an existing convention rather than inventing one (types.ts:436 and :606, read at actions.ts:2598, :2998, :3031), are set to true by the take-back path, and are read at both throw sites.

Reuse is good throughout: the composer-restore sequence mirrors the existing inline edit-and-resend path (App.tsx:16578-16583) call for call instead of growing a parallel helper, cancelledTurnProducedNothing is a 30-line pure predicate in its own file with its own test, and the identity matching reuses matchesUserMessageIdentity including its allowLocalId third parameter. The guard ordering in takeBackCancelledPrompt is right — bareTurnIndex() is re-evaluated after the await on the snapshot listing, so a transcript that moved during the round trip is caught rather than trusted.

Non-blocking, named so they're on the record rather than lost:

• recover? was made optional but every call site still passes it. The base type required it and the one existing call site supplied it (App.tsx:16131-16136); the new take-back site supplies it too. So the ? and the pendingRewind.recover?.() guard defend a case that cannot occur. Trivial, but it's exactly the kind of unrequested flexibility AGENTS.md asks not to add.
• rewindMissed is an unbounded for (;;) poll. Its liveness depends entirely on stillHeld() — hold lifted, unmount, or session change — not on any attempt cap, pacing at 0.5 s → 1 s → 2 s forever while the daemon is unreachable. That's a deliberate, documented decision (a timer can't safely release a hold whose rewind outcome is unknown, and the author is right that it can't), and the failure mode is correct: it keeps asking rather than guessing. Flagging it because it's the one loop in the change with no bound, and a future reader could mistake that for an oversight.
• The stuck-composer tradeoff is now reachable from a much more common gesture. If the daemon applies the rewind but session_rewound never reaches the transcript, the composer stays read-only until reconnect or reload. The PR body is upfront about this and it's the same tradeoff the pre-existing inline-edit path carries — but inline edit is a rare, deliberate action, while Esc Esc is reflexive. The exposure widened even though the mechanism didn't. Not a defect; worth knowing when the first report arrives.
• The Test Plan's platform deferral points at coverage this PR's CI never runs. "macOS and Windows were not run locally; the change is browser-side only and is left to CI there" — but test_macos (ci.yml:1625-1634) and test_windows (ci.yml:1731-1741) are gated to merge_group / schedule / workflow_dispatch, so on a pull_request event they report skipped, which is exactly what they did on this head. Those legs will only see the merged tree via the nightly schedule: cron '17 19 * * *' on main. Given the change is browser-side JS and the ubuntu leg is green I don't think this hid anything, but the Tested-on table's ⚠️ reasoning was inaccurate, and that matters because it's the sentence a reviewer relies on to not check.
• No design doc for a change with this much concurrency semantics. AGENTS.md asks for one under docs/design/ (English + zh-CN) for non-trivial work touching multiple files, and this is 416 production lines across two packages with a four-piece ordering invariant. The PR body genuinely carries most of that rationale — arguably better than a design doc would, since it's tied to measured evidence — so this is a note about the convention, not a gap in the reasoning. The invariant that a snapshot listing must be ordered after every admitted rewind now lives in a doc comment on rewindTail, which is the right place, but it's the kind of thing that gets broken by someone who never reads it.

sequenceDiagram
    participant P1 as User (Web Shell tab)
    participant P2 as App.tsx handleCancel
    participant P3 as Daemon session
    participant P4 as DaemonSessionProvider
    participant P5 as takeBackCancelledPrompt
    participant P6 as acp-bridge control plane
    P1->>P2: Esc Esc (cancel the turn)
    P2->>P2: snapshot the in-flight prompt and its id
    P2->>P3: cancel
    P3-->>P4: turn_complete or turn_error (terminal frame)
    P4->>P5: prompt settled, outcome cancelled, originator stamped
    P5->>P5: nothing decided yet - inspect what the turn produced
    P5->>P1: restore text, images, files into the composer
    P5->>P5: hold prompts (composer read-only)
    P5->>P6: getRewindSnapshots (silent)
    P6->>P6: await rewindTail so the listing follows admitted rewinds
    P6-->>P5: newest snapshot
    P5->>P6: rewindSession (rewindFiles false, silent)
    P6->>P3: session rewind
    P3-->>P4: session_rewound
    P4->>P5: transcript caught up - hold lifts, prompts released

Testing evidence — the PR's own CI

What this section carries: check-run results for the reviewed head, read through the API. 81 check-runs on 16bd737bd3be827e4d4c312d71e30b0d0829a675 — 25 success, 53 skipped, 3 cancelled, 0 failure, 0 pending. All three pull_request-event workflow runs (Qwen Code CI, SDK Java, Web-shell Visuals) completed success.

The three cancelled checks are bot orchestration, not the PR's CI — fallback-comment, review-pr, delay-automatic-review, all superseded when the review lane was cut short by the merge. No log excerpt to quote, because nothing failed.

Two honest gaps in that signal, both about what green does not prove here:

The macOS and Windows unit legs are skipped by design on PR events, so the head that merged was only ever unit-tested on Linux. Not a defect in the PR — it's the CI configuration — but it means the Tested-on table's deferral to CI was deferring to a run that doesn't exist on this trigger.

More importantly, the suite passing does not by itself establish that the suite pins the change. The author reports 28 targeted mutations of the take-back path, each failing at least one new test, with one surviving mutation (the flat backoff, which only paces retries) — that is the right kind of evidence and it's a strong claim, but it is the author's own measurement, run on the author's own harness on Linux, not something I re-ran or could re-run from here. The same applies to the 19-scenario real-stack table (real qwen serve, real Web Shell in Chromium, scripted model) and to the two non-recurring misses the author disclosed against their own change — 2 misses across several hundred take-backs, both degrading to a plain stop rather than a wrong rewind. Disclosing those was the right call and is a point in the PR's favour; I'm attributing them, not adopting them.

Sandboxed verification would settle the part CI can't: @qwen-code /verify — that the hold genuinely survives an unknown rewind outcome (request lost, response lost, daemon unreachable) and that dropping either the originator fallback or the two-reading rule in rewindMissed fails a test, is not observable from a green suite, since all three are timing paths whose unit tests use controlled clocks. Post-merge this is a confirmation rather than a gate, so it's optional — but if anyone ever reports a composer stuck read-only after Esc Esc, that A/B is the fastest way to find out whether the guard or the timing is at fault.

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

_{Reviewed at 16bd737bd3be827e4d4c312d71e30b0d0829a675 · re-run with @qwen-code /triage}

Confidence: 4/5 — I went looking for the four things that break changes like this and all four came back clean on verification against the pre-PR tree; what's left is named nits and one honest reservation about where the complexity now lives.

No gate action is possible or appropriate here. The PR merged at 15:35 UTC on 2026-10-06 (28e0f3a8) with two human approvals already standing, 37 minutes after the /triage request that started this run. So this is a reflection, not a verdict with teeth: I'm not approving (an approval on a merged PR attests to nothing, and there's nothing left to gate), not requesting changes (I found nothing that would have justified it), and not deferring to a maintainer (there's no unresolved question that needs a human call — the two things worth the author's attention are in the Stage 2 comment and the author is a maintainer who will see them).

On the substance. My independent proposal before reading the diff was: reuse the existing edit-and-resend rewind and its prompt hold, and decide at the turn's terminal event rather than at cancel time, because at cancel time you cannot distinguish "nothing produced yet" from "nothing produced". The PR does exactly that — and the second half of it is the part I would probably have gotten wrong under time pressure, since deciding at cancel time is simpler, feels correct, and passes every test you'd think to write first. The PR body documents an intermediate build that made precisely that mistake and the scenario that caught it. That's the strongest single signal in this review: the author built the wrong version, found it, and kept the evidence instead of quietly rewriting history.

The simpler path I'd have reached for — letting the daemon own "a cancelled turn that produced nothing is dropped", so every client inherits it — is the one I raised in Stage 1, and I'll leave it as a question rather than a criticism. It would remove the need for the originator stamp, the snapshot round trip, and the bridge ordering fix, all of which exist only because the decision lives client-side. But it would also change behaviour for the TUI, which already does its own restore and didn't ask for this. Choosing the smaller blast radius over the cleaner architecture is a defensible call, and it's the one AGENTS.md would favour. If a third surface ever needs the same semantics, that's the moment to move it down.

Would I curse or thank the author in six months? Thank, mostly. The doc comments sit exactly where the invariants are non-obvious — on rewindTail, on the two-reading rule in rewindMissed, on why no timer may release a hold of unknown outcome — and they explain why rather than restating the code, which is the house style. The test names read as a specification of the guards rather than a list of scenarios. The mutation testing means the tests are load-bearing rather than decorative. And the two non-reproducing misses were disclosed against the author's own change, with the raw JSON kept, which is the behaviour I'd want from anyone touching a race-prone path.

My one real reservation, and it's the reason this is 4/5 and not 5/5: this adds 325 production lines to an App.tsx that is already 22,714 lines. The take-back machinery — in-flight prompt tracking, the settlement matcher, the hold lifecycle, the unknown-outcome poll — is a coherent unit with a single responsibility and a well-defined boundary, and it now lives inline in the largest file in the client alongside everything else. Extracting it into a hook would have made the state machine reviewable on its own terms, and would give the next person who touches cancel semantics a file they can hold in their head instead of a 22k-line one they have to search. To be fair: App.tsx was already in that condition, the inline edit-and-resend path this generalizes lives there too, and extracting it would have widened the diff and delayed a real fix — so I don't think this was the wrong call for this PR. But the file got meaningfully worse, and that debt compounds quietly rather than announcing itself. Worth a follow-up, not a revert.

Am I approving-equivalent because I ran out of reasons to say no? I don't think so. The specific failure modes I checked — a snapshot listing parking behind a long turn, a new error path for unknown sessions, an unpopulated field read as a fallback, a guard no test pins — each had a concrete answer in the base code, not just a plausible one. What I'd want before calling this 5/5 is the part nobody can supply from a static review: independent confirmation on a real stack that the hold survives a lost rewind response, which is what the /verify line in Stage 2 is for.

For the record, so nothing here is silently dropped: the two items worth tracking as follow-ups are the test_macos / test_windows event gating (the PR's Tested-on table deferred to CI legs that don't run on pull_request, and a future contributor will make the same assumption), and the missing design doc for a change whose central invariant is now recorded only in a doc comment.

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

_{Reviewed at 16bd737bd3be827e4d4c312d71e30b0d0829a675 · re-run with @qwen-code /triage}

Sandboxed verification: ⚠️ not run — skipped - workflow run

Skipped because the PR is not open for verification (state=MERGED, draft=false).

— Qwen Code · sandboxed verification