fix(web-shell): Harden prompt admission ownership - #8955
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
E2E test reportValidated after rebasing onto
The real-daemon cases cover an active A turn across A→B→A, a structured target 504 that preserves A and its waiter, transactional source preservation, and serialization of non-equivalent load/resume requests. The request ledger asserts exactly one source Existing React |
|
Re-run at the fix commit Template still looks good ✓ Problem: unchanged from the first pass — #8923 is an observed bug with a real request sequence captured in an embedded deployment ( Direction: aligned, unchanged — session-navigation integrity is the linked issue's roadmap item; no public protocol, SDK, or daemon changes. Size: core module paths are not touched (single package). Production logic is now 1,146 lines in Approach: the fix commit is tightly scoped — it addresses exactly the five findings from the last review round (re-arm instead of wipe/remap, background-notification exclusion, owner-match early return in the reset effect, catch-side restore, owner-scoped local-id fallback) and adds seven tests pinning each restored affordance. No drive-by changes. The remaining S-level suggestions are deferred in-thread with reasons, per the five-round rule. Risk: no elevated risk signals — none of the changed files match the revert-correlated high-risk paths. Moving on to code review. 🔍 中文说明在修复提交 模板依然完整 ✓ 问题：与第一轮结论一致 —— #8923 是已观测到的 bug，嵌入式部署中捕获到真实请求序列（会话 A 活跃 turn 期间的 方向：对齐，未变 —— 会话导航完整性是关联 issue 的 roadmap 项；未改动公开协议、SDK 或 daemon。 规模：未触及核心模块路径（单一 package）。生产代码现为 方案：修复提交范围收敛 —— 恰好处理上一轮 review 的五个发现（以重新武装替代 wipe/remap、排除后台通知、reset effect 中 owner 匹配时提前返回、catch 侧恢复、owner 作用域的本地 id 兜底），并为每个恢复的入口新增钉住测试，共七个。无夹带改动。其余 S 级建议已按五轮规则在 thread 中逐条记录延期原因。 风险：无升级风险信号 —— 改动文件均未命中与 revert 相关的高风险路径。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Verification of the fix round at
|
| Check | Conclusion |
|---|---|
Capture web-shell visuals (ubuntu-latest, Node 22.x) |
✅ success |
Classify PR |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。
Integration Tests (CLI, No Sandbox) and the macOS/Windows test legs are skipped for fork PRs as usual — not a gap introduced by this PR.
Sandboxed verification: the first @qwen-code /verify run already proved the load-bearing navigation claim on the earlier head — the identical test file flipped 32 red on the base build vs green on the PR build, and all six single-guard mutants were killed with no off-target effects. A second /verify is in flight for this head (run); it will re-run the A/B and mutation matrix over the re-arm machinery added in this commit. Its result lands in the verification status comment and is the one piece of evidence still outstanding — PR CI above is already green and the new behaviors are pinned by the targeted tests.
Not verified: the author's local Windows/Linux runs (not run here; web-shell unit tests are platform-independent under jsdom) and the author-run mutation numbers (superseded by the independent /verify runs).
中文说明
修复轮验证（0ab4f16）： 作者推送了一个聚焦提交，逐一回应上一轮确认的五个发现。本次对照所审 head 逐一复核了修复、追踪了相关状态机并通读了新测试，五项全部成立。
- turn-error retry 再次失败（原为永久抑制）：
handleRetry的 catch 在失败为 daemon turn error 时暂存被重试的 payload，新的rearmFailedTurnErrorRetry在不同的 turn_error 块出现后重新给出 retry——校验 owner 仍匹配、要求原错误块仍在 transcript 中（transcript 被替换则清空暂存，fail-closed），恢复 payload 引用并把 identity 指向新错误、清空retriedTurnErrorIdRef使 remap 分支不再抑制。两种 promptId 情形均按 effect 时序推演收敛。由 fresh/reused promptId 各一例的it.each钉住。附带诚实修复：套件原先的isDaemonTurnErrormock 恒为 false，旧测试根本无法覆盖该 catch 路径——这正是第一轮漏掉回归的原因，现已改为真实标记检测。 - 后台通知杀死 retry 入口：三处扫描均跳过
background_notification块，渲染与 reconcile 重新一致；两类 retry 各有钉住测试。 - 新会话首个 prompt 失去可 retry 性：owner reset layout effect 在 owner 已匹配当前 connection 时提前返回（与上轮建议同形），reset 路径同时清空新暂存；关键是此前"因错误原因通过"的测试已修正——connection 提交移到 submit continuation 写入 refs 之后的
act()内，按真实 React 时序触发 layout effect，移除提前返回即会红。 handleRetrycatch 缺失恢复：!admitted失败现与handleFailedPromptRetry同路径恢复，两条 retry 路径对齐；由网络拒绝测试钉住。- copy-on-write 下乐观块 identity：按建议落地设计决策——
allowLocalId兜底仅在 retry owner 快照仍 current 时启用，同 owner 克隆继续按本地 id 匹配，跨 owner/attachment 的 id 复用保持 fail-closed；五个门控一致应用，外加 replayed 稳定记录的重绑定分支；由克隆测试与 replay 测试钉住。
另：cancelled-retry 条目引入单调 attemptId 并按 kind 归并，乱序准入结算保留较新 retry，有专门测试，设计契约同步补充一句。作者 inline 回复与修复一一对应；延期的 S 级项按五轮规则逐条记录。
非阻塞备注：CancelledRetryState.errorId 仍只写不读（已延期的清理项）；applyCancelledRetryState 未用本地 id 兜底，但被围栏隔离的失败块在用户处于其他会话时实际不可能被克隆，属理论 corner；新 head 的沙箱 /verify 将在本提交之上重跑 A/B 与突变矩阵，见下。
CI 证据：所审 head CI 全绿，零失败（表格如上，finalize 会自动更新该区域）。fork PR 常规 skipped 项不构成缺口。沙箱验证：第一次 /verify 已在前一 head 上证明导航侧负载性声明（同一测试文件在 base 构建上 32 红、PR 构建全绿；六个单点守卫突变体全部被杀且无脱靶）；第二次 /verify 正在本 head 上运行，将覆盖本次新增的重新武装机制，结果见验证状态评论——这是唯一未落定的证据，PR CI 已全绿且新行为均有定向测试钉住。未验证：作者本地 Windows/Linux 运行（web-shell 单测经 jsdom 平台无关）与作者自报 mutation 数字（以独立 /verify 为准）。
— Qwen Code · qwen3.8-max
Reviewed at 0ab4f1623706dce2e3915287b78322d70a9c4fee · re-run with @qwen-code /triage
|
Confidence: 4/5 — the fix round landed exactly where the last review said the seams were, every restored affordance now has a pinning test, and CI is green; the one open item is the in-flight sandboxed re-verification of this head. Going back to the independent proposal from the first pass — re-arm on a new same-session error instead of wipe/remap, and skip the owner reset when the submit continuation already advanced the owner — this commit implements precisely those two seams, plus the three supporting fixes, without scope creep. The regression I called deterministic in both directions (fresh and reused promptId) now has a two-armed The remaining nits are non-blocking and recorded: Review-state bookkeeping, since this thread has history: this approval supersedes the bot's earlier changes-requested review (same account, latest review wins — that review gated the previous head 中文说明总体判断：4/5 —— 修复轮恰好落在上轮指出的接缝上，每个恢复的入口都有钉住测试，CI 全绿；唯一未落定项是本 head 的沙箱复验仍在运行。 回到第一轮的独立方案——新同会话错误出现时重新武装而非 wipe/remap、submit continuation 已推进 owner 时跳过 owner reset——本提交精确实现了这两个接缝及三项配套修复，无范围蔓延。此前判定为双向确定性的回归（fresh/reused promptId）现有双臂 其余均为非阻塞并已记录： Review 状态记录：本批准覆盖机器人此前的 changes-requested（同一账号最新评审生效，该评审门禁的是旧 head — 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 / afterℹ️ No screenshot changed against the PR base — but this PR edits 1 render-shaping file:
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 Full-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): PR #8955 "fix(web-shell): Harden prompt admission ownersh...: none — all planned checks completed (diff fully read, suite executed, two mutation runs performed and reverted).; You are review agent reverse-audit — Reverse audit agen...: none — all checks above completed; no probe/mutation executed for finding 2 (stated in its confidence).; You are review agent reverse-audit — Reverse audit agen...: none — the full assigned range was read and all candidate checks above were completed.; chunk 5: none — I completed all checks I planned. Remaining: nothing unfinished. I have not verified the hunk contents past line 5172 (chunk 6), and cross-file callers o…; PR #8955 "fix(web-shell): Harden prompt admission ownersh...: none — all planned checks completed within budget., and 6 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）：PR #8955 "fix(web-shell): Harden prompt admission ownersh...：none — all planned checks completed (diff fully read, suite executed, two mutation runs performed and reverted).；You are review agent reverse-audit — Reverse audit agen...：none — all checks above completed; no probe/mutation executed for finding 2 (stated in its confidence).；You are review agent reverse-audit — Reverse audit agen...：none — the full assigned range was read and all candidate checks above were completed.；chunk 5：none — I completed all checks I planned. Remaining: nothing unfinished. I have not verified the hunk contents past line 5172 (chunk 6), and cross-file callers o…；PR #8955 "fix(web-shell): Harden prompt admission ownersh...：none — all planned checks completed within budget.，另有 6 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.10)
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): This PR hardens WebShell prompt ownership across session ...: none — all planned checks completed within budget.; This PR hardens WebShell prompt ownership across session ...: I did not verify whether the daemon server unconditionally assigns SSE event IDs to turn_error events (that code is outside the web-shell package). If the dae….
[Critical] R1-3: retry owner useLayoutEffect's general reset path clobbers submit-state refs during session creation. When a user opens a new chat (no active session) and submits a prompt, ensureSessionForPrompt() allocates a session, calling setConnection inside createSession. The submit flow continuation (microtask) sets lastSubmittedPromptRef and retryOwnerRef before React processes the setConnection state update. The subsequent retry owner useLayoutEffect fires, finds the owner already matches (so workspaceBecameKnown is false), falls through to the general reset, and clobbers lastSubmittedPromptRef = '' and lastSubmittedSourceVersionRef = -1. When a turn error arrives, canRetry checks lastSubmittedPromptRef.current.length > 0 which is false — the retry hint is never shown for the first prompt after session creation. Confirmed by a timing probe: the layout effect fires after the async continuation. The existing test 'keeps the first prompt retryable through its lazy session commit' passes only because its mock bypasses setConnection by mutating mockConnection directly, which does not trigger the layout effect at the correct time. Suggested fix: add a guard at the top of the layout effect that returns early when the owner already matches the current connection state.
中文说明
未探索到全部深度（达到工具调用预算）：This PR hardens WebShell prompt ownership across session ...：none — all planned checks completed within budget.；This PR hardens WebShell prompt ownership across session ...：I did not verify whether the daemon server unconditionally assigns SSE event IDs to turn_error events (that code is outside the web-shell package). If the dae…。
[Critical] R1-3: retry owner useLayoutEffect's general reset path clobbers submit-state refs during session creation. When a user opens a new chat (no active session) and submits a prompt, ensureSessionForPrompt() allocates a session, calling setConnection inside createSession. The submit flow continuation (microtask) sets lastSubmittedPromptRef and retryOwnerRef before React processes the setConnection state update. The subsequent retry owner useLayoutEffect fires, finds the owner already matches (so workspaceBecameKnown is false), falls through to the general reset, and clobbers lastSubmittedPromptRef = '' and lastSubmittedSourceVersionRef = -1. When a turn error arrives, canRetry checks lastSubmittedPromptRef.current.length > 0 which is false — the retry hint is never shown for the first prompt after session creation. Confirmed by a timing probe: the layout effect fires after the async continuation. The existing test 'keeps the first prompt retryable through its lazy session commit' passes only because its mock bypasses setConnection by mutating mockConnection directly, which does not trigger the layout effect at the correct time. Suggested fix: add a guard at the top of the layout effect that returns early when the owner already matches the current connection state.
— glm-5.2 via Qwen Code /review (v0.21.10)
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Reviewing for the admission-ownership invariants (series #8678). Full diff read (App.tsx +818/-160, design doc, test suite). Approving — no P0/P1 found.
Ownership/fencing
- Admission source (owner snapshot, sessionId, workspace cwd, source version, write-gate generation) is captured before
onSubmitBeforeand rechecked after it, afterensureSessionForPrompt, and before composer commit /clearFollowup/ lastSubmitted recording — so draft and retry state stay with the source composer when the gate closes mid-flight.ownAllocationSucceededcorrectly exempts only the prompt that allocated the session itself, and requires the connection to have adopted that allocation. - Enqueue path adds
sourceOwner.isCurrent(), plan-mode prep re-checks the write-gate generation before sending, anddeferComposerCommitnow also covers an in-flight shared lazy-session creation. All consistent with the updated design doc.
Transcript identity
- Replacing raw block-id matching with reference identity + stable fallbacks (
sourceRecordIdsfor user messages,promptId/eventIdfor turn errors) is the right fix against transcript replacement reusing local ids; the empty-transcript anchor case is handled and mismatches fail closed.setFailedPromptRetryupdaters now key on the identity-object reference, avoiding sessionId/messageId collisions.
Cancelled-retry deferral
- The session-keyed defer/restore queue restores only when writes are unblocked, the transcript is loaded, and (for workspace-unknown entries) the captured owner is still current or the same owner supplied the workspace (re-key migration); an owner change discards. Unmount invalidates continuations via
appMountedRef. TheSymbol-keyedpromptPreparationOwnerRefprevents a stale finally from clobbering a newer preparation, and both retry entry points are gated on it.
Tests pin the invariants behaviorally (ownerVersion bumps, transcript replacement with reused local error ids, A→B→A owner cycles, admission responses lost mid-flight, workspace enrichment), asserting sendPrompt call counts and UI state rather than snapshots.
CI (head 0b79905): Qwen Code CI green — Test (ubuntu, Node 22) success (includes the web-shell suite), web-shell E2E Smoke success, Desktop Shell ubuntu/windows success; windows/macOS matrix and CLI integration jobs skipped by design. No flake failures.
Non-blocking nits (P3):
- In
applyCancelledRetryState's failed-prompt branch, if identity matches viasourceRecordIdsbut the block id changed (rehydrated transcript),displayMessages.some(id === failed.messageId)never becomes true, so the entry stayspendinguntil an owner change clears it.handleFailedPromptRetryremapsmessageIdin the analogous case; doing the same here would close the gap. Fail-safe direction (retry affordance not restored), very narrow. - The restore layout effect re-runs on every transcript frame while pending entries exist — negligible cost today, just worth keeping in mind if the transcript update cadence grows.
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 3343 passed · 0 failed · 3343 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：3343 通过 · 0 失败 · 3343 总计 Verification reportPR #8955 Deep Verification —
|
| Arm | Tree | Production code | Oracle | Result |
|---|---|---|---|---|
| head | merge ref 0911747636 |
PR head App.tsx |
vitest, mocked daemon adapters, real component tree | 422/422 pass |
| control | worktree at HEAD^1 = 0e8a361b37 |
base App.tsx |
same test file, copied verbatim into the base tree | 32 failed / 390 passed |
Witnesses: 01-ab-head-arm-422-green.png (head arm as it printed), 02-ab-base-arm-32-red.png (control arm as it printed).
All 32 red cells are behavioral assertion mismatches, e.g.:
- base arm, direct-send race:
AssertionError: expected "spy" to not be called at all, but actually been called 1 times— the base sent the stale prompt (sendPrompt("first", …)) after the navigation changed the owner; head cancels it. - base arm, queued A→B→A cycle: same assertion on the enqueue spy — the base enqueued the stale prompt because its sessionId-equality check passes again once navigation returns to A; the PR's owner-snapshot check is what closes that hole.
- base arm, reused local error id:
expected "spy" to be called 1 times, but got 2 timesplus stale retry buttons still rendered — the base offered a retry for a replacement transcript that reused the reducer-local block id.
Central-claim cells (each red on base, green on head):
| Scenario | Test | base | head |
|---|---|---|---|
| direct submit, navigation during lazy session prep | cancels an approved submission when navigation occurs during session preparation |
❌ stale send | ✅ cancelled |
| default submit, navigation during prep | cancels a default submission when navigation occurs during session preparation |
❌ stale send | ✅ cancelled |
| approved submit, App unmounted before hook returns | does not commit an approved submission after the App unmounts |
❌ sends | ✅ inert |
| queued submit, A→B→A owner cycle | cancels an approved queued submission after an A-to-B-to-A owner cycle |
❌ stale enqueue | ✅ cancelled |
| queued submit, App unmounted | does not enqueue an approved queued submission after the App unmounts |
❌ stale enqueue | ✅ cancelled |
| cancelled retry during navigation | defers retry restoration until navigation commits |
❌ restores early | ✅ deferred |
| replacement transcript reuses local error id | drops a known-workspace retry when a replacement reuses its local error id |
❌ stale retry | ✅ dropped |
| stable replay (positive control) | keeps a known-workspace retry across a stable error replay |
✅ green on both arms | ✅ green |
The full 32-name red set is asserted exact-set by harness/ab-assert.mjs (16/16 checks pass): no missing, no extra red cells. Of the 32, 29 are genuinely new tests introduced by this PR and 3 are pre-existing tests whose bodies were strengthened (e.g. settles a prompt retry after a same-id attachment replacement). 10 further new tests pass on both arms — regression-style coverage for behavior the base already had.
Control purity: the base worktree reuses the main tree's node_modules (symlinked); the PR touches no package.json/package-lock.json, and the internal workspace deps resolve identically on both arms (readlink -f node_modules/@qwen-code/sdk → packages/sdk-typescript, unchanged by this PR). The only difference between arms is App.tsx at HEAD^1 vs head.
Mutation matrix (vacuity + per-guard load-bearing proof)
Unmutated control in the mutation worktree: 422/422 green (same harness, same node_modules). One guard reverted per mutant; full 422-test suite per mutant. Witness: 03-mutation-matrix-six-mutants.png (live re-run of all six mutants).
| Mutant | Guard reverted | Suite result | Killed tests (exact set asserted) |
|---|---|---|---|
| M1 | post-ensureSessionForPrompt admission recheck (send path) |
2 failed / 420 passed | both navigation-during-preparation cancellation tests |
| M2 | enqueue-path appMountedRef + owner-snapshot checks |
2 failed / 420 passed | A→B→A queued cycle; unmount enqueue |
| M3 | matchesTurnErrorIdentity → reducer-local id only |
4 failed / 418 passed | reused-error-id drop, stable-replay keep, stale turn-error restore, uncertain-response drop |
| M4 | matchesUserMessageIdentity → reducer-local id only |
6 failed / 416 passed | reused-user-id drop, rehydration, restore-after-switching, anchor-change drop, same-id settle, uncertain failed-response drop |
| M5 | sessionWriteBlocked in the cancelled-retry restore gate |
3 failed / 419 passed | deferred restoration; cached-retry migration; workspace-unknown owner drop |
| M6 | side-effect ordering reverted (composer commit/clear + follow-up clear moved before the gate, gate itself intact) | 8 failed / 414 passed | both nav-prep tests failing at expect(editorClear).not.toHaveBeenCalled() (App.test.tsx:11170), plus the onSubmitBefore-reject and lazy-session families |
Zero survivors; zero off-target kills in any mutant (each killed set is exact, asserted by harness/matrix-assert.mjs). Notes:
- M3 kills in both directions: id-only matching both drops the reused-id impostor test and loses the stable-replay keep test — the stable-identity machinery is what makes the two distinguishable at all.
- M6 is the finer-mutation control: with the gate still present,
sendPromptis never called, but the eagereditorRef.current?.clear()leaks — proving the ordering change (side effects moved after the rechecks) is independently pinned, not just the gate's existence. - Positive control: the unmutated run is green and every mutant went red on the intended behavioral assertions (expected-vs-received spy-call messages), so the harness demonstrably can fail.
Reviewer Test Plan, walked step by step
- Delayed
onSubmitBefore/ lazy prep, direct + queued submission, A→B→A, then release — no stale send/enqueue, composer and follow-up intact, retry only after settled. — Exercised by the nav-prep cancellation cells (both submission kinds), the unmount cells, the queued A→B→A cell, and the restoration cells (defers retry restoration until navigation commits,restores … after switching away during admission,waits for the source transcript before restoring a cancelled retry). Composer/follow-up intactness is pinned by the M6-sensitive assertions. ✅ all green on head, red on base where the guard is new. - Failed-prompt and turn-error retries under attachment replacement; stable replay keeps exactly one retry; reused local IDs expose nothing. — Exercised by the M3/M4 test families incl.
does not expose a duplicate retry when a stable replay passes through an empty transcriptandrehydrates a rejected failed-prompt retry after attachment reset. ✅ green on head. - Hanging real daemon turn, A→B→A, source stays active, ledger clean. — Real-daemon integration suite
integration-tests/cli/qwen-serve-webui-session-switching.test.ts: 5/5 pass on the merge ref (24.6 s), incl.keeps an active source running across A to B to A navigation. Witness log:logs/session-switching-head.log. ✅ matches the PR's claimed 5/5.
Shape-vs-cause note: the unit-level cells reproduce the wire shape of the race (pending continuations resolved after simulated navigation of the mocked connection) rather than real host-hook latency against a live daemon; the real-daemon suite covers the daemon/session layer around navigation but drives webui primitives, not the App guards. Together they bracket the claim from both sides.
Corrections
None required — no inaccurate claims from earlier review rounds were encountered, and the PR description's quantitative claims (422/422 App suite, 5/5 session switching, mutation checks failing when owner/generation/transcript-identity guards are removed) were each independently reproduced.
Findings
None blocking. Observations, for the record:
- (Info) The PR's own test-plan platform table marks Linux as untested (
⚠️ ). This round ran the entire unit suite and the real-daemon suite on Linux (node:22-bookworm, Node v22.23.2) — all green; the component tests are jsdom-based and platform-agnostic. - (Info) The write-gate generation guard was not isolated as a standalone mutant: the generation recheck on the
onSubmitBeforepath pre-exists on base, and the new plan-prompt generation check is pinned indirectly (does not send a deferred plan prompt after an interrupted navigationis among the 32 base-red cells). Owner and transcript-identity guards were isolated (M2/M3/M4), matching the PR's "owner, generation, or transcript-identity" claim for two of the three named axes directly and the third indirectly. - (Info) No injection-style steering instructions were found in the PR title/body/commit message.
Not covered
- Playwright e2e and visual suites (
test:e2e,test:e2e:visuals) — browser-based, out of scope for this round. - The rest of the integration suite (cli/interactive beyond the session-switching file).
- Windows/macOS platform-specific behavior (container is Linux; unit tests run under jsdom).
- DataWorks host-controller behavior, daemon prompt/cancel semantics beyond the session-switching suite, branch/fork navigation — declared out of scope by the PR and not exercisable here.
- Finer single-point mutants beyond the six guard families (e.g. the
workspaceBecameKnownretry migration, the plan-prompt generation check, the shell-command preparation owner) — covered by the base-red cell set but not individually mutated. - Per-commit attribution is trivially complete: exactly one commit in the PR.
Methodology
Environment: CI verify container (node:22-bookworm), Node v22.23.2, npm ci + npm run build pre-run at the merge ref. A/B control built as git worktree add tmp/base-tree HEAD^1 with the HEAD test file copied in and the package's node_modules symlinked (lockfile untouched by the PR; readlink -f asserts internal workspace deps resolve to unchanged packages). Mutations ran in a second worktree (tmp/head-mut, merge ref), applied as single-hunk patches generated by harness/make-patches.py, each reverted before the next. Suites: npx vitest run client/App.test.tsx --config vitest.config.ts (~30 s/run) and the workspace-wide npx vitest run; real-daemon suite via cross-env QWEN_SANDBOX=false npx vitest run cli/qwen-serve-webui-session-switching.test.ts from integration-tests/. Gate liveness proven by planted violations (ESLint caught 2 planted errors, exit 1; tsc reported TS2322 for a planted type error) then removed. Scripted assertion harnesses: harness/ab-assert.mjs (16 checks), harness/matrix-assert.mjs (12 checks), harness/gates-assert.mjs (10 checks); evidence images via scripts/verify-capture.mjs. Raw per-arm and per-mutant logs: logs/.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
LGTM. No critical issues found. Key suggestions: (1) workspace check in admissionOwnerIsCurrent may be over-permissive when the original workspace was known — consider only waiving the check when workspaceCwd was undefined; (2) retryOwnerRef layout effect mutates previousOwner fields in-place, prefer immutable updates; (3) stale entries may accumulate in cancelledRetryStatesRef when known workspace changes; (4) no test coverage for queued-prompt owner/write-gate cancellation path. Also check whether sessionOwnerGuard is referentially stable across renders to avoid unnecessary effect re-runs.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Handled the current review round in commit
Verification on the exact commit:
|
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Fresh sweep of the new head (delta from 0b79905 → 0ab4f16, one commit, 3 files) against the Critical findings from the prior rounds. Approving — no P0/P1.
Critical fixes — verified
- Layout-effect clobber on lazy session creation (wenshao R1-3): the retry-owner layout effect now returns early when
retryOwnerMatchesCurrent(previousOwner, …)(App.tsx ~4213), placed after the workspaceBecameKnown re-key branch and before the general reset. The send continuation writes the full owner object (incl.snapshot), sopreviousOwneris authoritative when the effect fires post-setConnection— submit-state refs survive, and a genuine session switch still falls through to reset. The previously masking test was restructured to commit the connection change through a real rerender after send, so the fix is pinned. - Failed retry could never be retried again (R1-1 wipe / R1-34 promptId-reuse suppression): new
failedTurnErrorRetryRefstash +rearmFailedTurnErrorRetry. The catch stashes the payload onisDaemonTurnError, and the re-arm runs at the top of the blocks effect, re-pinning identity onto the distinct new error and nullingretriedTurnErrorIdRefbefore the remap/wipe branches evaluate. Fail-closed is retained: re-arm requires the captured owner still current AND the original error block still present in the transcript (transcript replacement discards the stash), and the send path + wipe branch both clear it. I traced the admitted/not-admitted orderings against the deferred-queue restore and thesettled && idleclearing effect — the mechanisms converge rather than fight. - background_notification user-kind blocks (R1-12): skipped in
getLatestUserBlock/findUserMessageByIdentity, andgetRetryableTurnErrorscans past them instead of breaking — both retry affordances survive notification injection. - No local-id fallback (wenshao R1-1):
allowLocalIdadded tomatchesUserMessageIdentity/findUserMessageByIdentity, gated onowner.snapshot.isCurrent()at all five call sites — owner-scoped, no unconditional id fallback, so cross-attachment id collisions stay closed. - handleRetry catch missing restore (wenshao R1-2):
!admittednow routes throughrestoreOrDeferCancelledRetry, mirroring the failed-prompt path. - Bonus hardening: monotonic
attemptId+mergeCancelledRetryEntriesso a newer retry supersedes an older same-kind retry when admission callbacks settle out of order; design doc updated with the invariant. Also closes my earlier P3 nit —applyCancelledRetryStatenow remapsmessageId/identity onto the matched replayed record instead of parkingpendingforever.
Tests: each fix is pinned behaviorally — reused-vs-fresh promptId re-offer matrix, resend-fails-before-admission restore, out-of-order admission supersede (asserts the newer payload is resent), background-notification visibility for both retry kinds, replayed-record restoration with changed block id + display text, and the restructured lazy-session first-prompt test.
CI (0ab4f16): Test (ubuntu, Node 22) ✅ incl. web-shell suite, web-shell E2E Smoke ✅, Desktop Shell ubuntu/windows ✅; macOS/windows Test and CLI integration skipped by design. No flake failures. Remaining in-flight/cancelled checks are bot automation (review-pr, route), not CI gates.
Non-blocking nits (P3):
- The re-arm's replacement guard keys on
block.id === failedRetry.errorIdpresence rather than reference identity; a replayed transcript reusing that local error id could keep the stash alive. Narrow and bounded (owner must stay current, restored payload is the user's own last prompt) — reference-based presence would be marginally tighter. retryTranscriptIsCurrentinhandleRetryomits theallowLocalIdflag its failed-prompt counterpart passes — intentional given promptId/eventId-based turn-error identity, but worth a one-line comment someday.
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 7259 passed · 0 failed · 7259 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：7259 通过 · 0 失败 · 7259 总计 Verification reportPR #8955 Deep Verification (round 2) —
|
| # | Round-1 finding | Severity | Status at new head 0ab4f162 |
|---|---|---|---|
| 1 | Author's platform table marks Linux untested ( |
Info | Stands (author table unchanged). Re-measured here on Linux (node:22-bookworm, Node v22.23.2): App suite 430/430, workspace suite 3333/3333, real-daemon suite 5/5 — all green at the new head. |
| 2 | Write-gate generation guard not isolated as a standalone mutant; pinned indirectly via does not send a deferred plan prompt after an interrupted navigation among base-red cells |
Info | Stands, partially strengthened. That test is still among the 37 base-red cells at the new head; additionally mutant M1 deletes admissionSourceIsCurrent(), which contains the generation comparison, so the guard is covered inside M1's 2 kills. A generation-only single-point mutant was still not run (see Not covered). |
| 3 | No injection-style steering instructions in PR text | Info | Stands. Round-2 metadata (title, body, both commit messages) re-scanned: no steering content. |
No declined or deferred rows existed in round 1, so there is nothing to re-adjudicate there.
Scope selection
Central claim — asynchronous continuations (host admission hook onSubmitBefore, lazy session preparation, queued-prompt admission) that return after a session navigation (A→B or A→B→A) or App unmount are side-effect-free: no stale send, no stale enqueue, no composer commit/clear, no follow-up clear.
Secondary claims
- Cancelled failed-prompt / turn-error retries are retained for their logical source session and restored only after navigation settles; a different owner or a transcript that merely reuses reducer-local block IDs fails closed.
- Retry identity matching uses stable identities (same in-memory block, persisted
sourceRecordIds,promptId/eventId) — a stable replay keeps exactly one retry.
Diff under test (aggregate HEAD^1..HEAD): packages/web-shell/client/App.tsx (+986/−160), packages/web-shell/client/App.test.tsx (+5637/−3122), design doc (+31/−6). No package.json/lockfile changes anywhere; no files outside web-shell + docs (verified by git diff --stat). The metadata snapshot lists 2 commits, but git rev-list HEAD^1..HEAD^2 returns only the grafted head commit — per-commit attribution is out of reach (see Not covered). Note the snapshot's baseRefOid (0e8a361b…) is stale relative to the rebuilt merge ref; per the environment contract HEAD^1 (1570e6c73f) is the authoritative base tip, and headRefOid matches HEAD^2 exactly.
Central claim — A/B load-bearing proof
The identical HEAD test file (430 tests — round 1 had 422; the second commit added 8) was run against both builds:
| Arm | Tree | Production code | Oracle | Result |
|---|---|---|---|---|
| head | merge ref 019664d1dc |
PR head App.tsx |
vitest v3.2.4, mocked daemon adapters, real component tree | 430/430 pass |
| control | worktree at HEAD^1 = 1570e6c73f |
base App.tsx |
same 430-test file, copied verbatim (sha256-verified) | 37 failed / 393 passed |
Witnesses: 01-ab-head-arm-430-green.png (head arm summary as printed), 02-ab-base-arm-37-red.png (control arm summary as printed), 04-ab-assert-flip-set.png (scripted flip-set assertions as printed).
All 37 red cells are behavioral assertion mismatches — e.g. base arm, direct-send race and unmount cells: AssertionError: expected "spy" to not be called at all, but actually been called 1 times (the base sent the stale prompt after the owner changed / the App unmounted); queued cell: same shape on the enqueue spy. All 37 are tests newly introduced by this PR (title-census against the base test file: 37 new, 0 pre-existing; round 1 had 29 new + 3 strengthened — the strengthened titles were replaced by new ones in the second commit).
Representative cells (each red on base, green on head; full 37-name set asserted exact by harness/ab-assert.mjs):
| Scenario | Test | base | head |
|---|---|---|---|
| direct submit, navigation during lazy session prep | cancels an approved submission when navigation occurs during session preparation |
❌ stale send | ✅ cancelled |
| default submit, navigation during prep | cancels a default submission when navigation occurs during session preparation |
❌ stale send | ✅ cancelled |
| approved submit, App unmounted before hook returns | does not commit an approved submission after the App unmounts |
❌ sends | ✅ inert |
| queued submit, A→B→A owner cycle | cancels an approved queued submission after an A-to-B-to-A owner cycle |
❌ stale enqueue | ✅ cancelled |
| queued submit, App unmounted | does not enqueue an approved queued submission after the App unmounts |
❌ stale enqueue | ✅ cancelled |
| cancelled retry during navigation | defers retry restoration until navigation commits |
❌ restores early | ✅ deferred |
| replacement transcript reuses local error id | drops a known-workspace retry when a replacement reuses its local error id |
❌ stale retry | ✅ dropped |
| replacement transcript reuses local user id | drops a known-workspace failed retry when a replacement reuses its local user id |
❌ stale retry | ✅ dropped |
| stable replay (positive control) | keeps a known-workspace retry across a stable error replay |
✅ green on both arms | ✅ green |
The last row is the positive control run on both arms: base happens to keep the stable replay because its id-only matching also accepts the impostor — the M3 mutant below shows the head's stable-identity machinery is what keeps the replay while dropping the impostor.
Control purity: the base worktree reuses the root node_modules (walk-up resolution) plus a symlinked packages/web-shell/node_modules for the nested @datafe-open/* deps. The PR touches no package.json/lockfile, and internal workspace deps resolve identically on both arms (readlink -f node_modules/@qwen-code/sdk → packages/sdk-typescript, @qwen-code/webui → packages/webui; git diff --stat over those packages = 0 lines). The only difference between arms is App.tsx at HEAD^1 vs head. (Initial base-arm run failed collection on @datafe-open/markdown-chart until the nested node_modules was linked — a harness issue, fixed and re-run; the failing attempt is preserved in logs/base-arm.log history.)
Mutation matrix (vacuity + per-guard load-bearing proof)
Unmutated control in the mutation worktree (tmp/head-mut, merge ref): 430/430 green (same harness, same node_modules). One guard reverted per mutant via harness/apply-mutant.mjs (each anchor asserted to occur exactly once); full 430-test suite per mutant. Witness: 03-mutation-matrix-six-mutants.png (live run of harness/matrix-assert.mjs, 25/25 checks).
| Mutant | Guard reverted | Suite result | Killed tests |
|---|---|---|---|
| M1 | post-ensureSessionForPrompt admission recheck (send path, incl. generation comparison) |
2 failed / 428 passed | both navigation-during-preparation cancellation tests |
| M2 | enqueue-path appMountedRef + owner-snapshot checks |
2 failed / 428 passed | A→B→A queued cycle; unmount enqueue |
| M3 | matchesTurnErrorIdentity → reducer-local id only |
4 failed / 426 passed | reused-error-id drop; stale turn-error restore; uncertain-response drop; stable-replay keep (both directions) |
| M4 | matchesUserMessageIdentity → reducer-local id only |
7 failed / 423 passed | reused-user-id drop; rehydration; restore-after-switching; anchor-change drop; stable-user-record replay; same-id settle; uncertain failed-response drop |
| M5 | sessionWriteBlocked in the cancelled-retry restore gate |
3 failed / 427 passed | deferred restoration; cached-retry migration; workspace-unknown owner drop |
| M6 | side-effect ordering reverted (composer commit/clear + follow-up clear moved before the gate, gate intact) | 6 failed / 424 passed | both nav-prep tests; shared-rejected-preparation clear; lazy-session admission family (delivery-unknown scoping, allocation scoping, first-failed-message tracking) |
Zero survivors; zero off-family kills (every killed name is asserted inside that mutant's curated family by harness/matrix-assert.mjs). Notes:
- M3 kills in both directions again at the new head: id-only matching both admits the reused-id impostor (drop tests die) and loses the stable-replay keep test — the positive-control row of the A/B and the M3 kill are the same two-sided evidence from opposite sides.
- M6 is the finer-mutation control: with the gate still present,
sendPromptis never called, but the eagereditorRef.current?.clear()/clearFollowup()leak — the nav-prep tests die on the intactness assertions, proving the ordering change is independently pinned, not just the gate's existence. Round 1's M6 killed 8; the second commit reshaped the lazy-session family, now 6. - M4 kills 7 (round 1: 6) — the new
drops a cancelled failed-prompt retry when its transcript anchor changestest is additionally pinned. - Positive control: unmutated run green; every mutant went red on intended behavioral assertions (expected-vs-received spy-call and rendering messages), so the harness demonstrably can fail.
Reviewer Test Plan, walked step by step
- Delayed
onSubmitBefore/ lazy prep, direct + queued submission, A→B→A, then release — no stale send/enqueue, composer and follow-up intact, retry only after settled. — Exercised by the nav-prep cancellation cells (both submission kinds), the unmount cells, the queued A→B→A cell, and the restoration cells (defers retry restoration until navigation commits,restores … after switching away during admission,waits for the source transcript before restoring a cancelled retry). Composer/follow-up intactness is pinned by the M6-sensitive assertions. ✅ all green on head; red on base wherever the guard is new. - Failed-prompt and turn-error retries under attachment replacement; stable replay keeps exactly one retry; reused local IDs expose nothing. — Exercised by the M3/M4 families incl.
does not expose a duplicate retry when a stable replay passes through an empty transcript(green on both arms — regression-style) andrehydrates a rejected failed-prompt retry after attachment reset(base-red). ✅ green on head. - Hanging real daemon turn, A→B→A, source stays active, ledger clean. — Real-daemon integration suite
integration-tests/cli/qwen-serve-webui-session-switching.test.ts: 5/5 pass on the merge ref (24.5 s), incl.keeps an active source running across A to B to A navigation. Witness:logs/session-switching-head.log. ✅ matches the PR's claimed 5/5.
Shape-vs-cause note (unchanged from round 1): the unit cells reproduce the wire shape of the race (pending continuations resolved after simulated navigation of the mocked connection), not real host-hook latency against a live daemon; the real-daemon suite covers the daemon/session layer around navigation but drives webui primitives, not the App guards. Together they bracket the claim from both sides.
Corrections
None. No inaccurate claims from earlier rounds or bot comments were encountered; round-1's quantitative claims were reproduced at that round's head, and the PR description's claims (full App suite green, 5/5 session switching, mutation checks failing when owner/generation/transcript-identity guards are removed) were each independently reproduced at the new head (430/430, 5/5, and mutants M1–M6 all red).
Findings
None blocking. Observations, for the record:
- (Info) The real-daemon suite's stderr carries React
act(...)warnings —An update to DaemonSessionProvider inside a test was not wrapped in act(...)— from the webui package's provider. This PR touches no webui code (git diff --statoutside web-shell/docs = 0 lines), and the suite passes 5/5; recorded as pre-existing test noise, not attributed to the PR. Base-attribution would require a full base bundle rebuild, not spent here. - (Info, carried from round 1) The write-gate generation guard still has no standalone single-point mutant; it is pinned indirectly (base-red plan-prompt cell) and inside M1's deleted
admissionSourceIsCurrent()gate. - (Info) No injection-style steering instructions in the PR's title, body, or either commit message.
Not covered
- Per-commit delta attribution. The metadata lists 2 commits (
0b799056,0ab4f162), but the depth-2 merge-ref checkout reaches only the grafted head (git rev-list HEAD^1..HEAD^2= 1 commit;git cat-file -t 0b799056…fails). What changed specifically in the "review feedback" commit cannot be isolated; the aggregateHEAD^1..HEADdiff is what was verified. - Playwright e2e and visual suites (
test:e2e,test:e2e:visuals) — browser-based, out of scope. - The rest of the integration suite (cli/interactive beyond the session-switching file).
- Windows/macOS platform-specific behavior (container is Linux; unit tests run under jsdom).
- DataWorks host-controller behavior, daemon prompt/cancel semantics beyond the session-switching suite, branch/fork navigation, legacy source visibility — declared out of scope by the PR and not exercisable here.
- Finer single-point mutants beyond the six guard families (e.g.
workspaceBecameKnownretry migration, plan-prompt generation check in isolation, shell-command preparation owner) — covered by the base-red cell set and in part by M1, but not individually mutated. - Base-side attribution of the act() warning in finding 1.
Methodology
Environment: CI verify container (node:22-bookworm), Node v22.23.2, npm ci + npm run build pre-run at merge ref 019664d1dc; npm run bundle re-run (exit 0) so the integration suite's dist/cli.js reflects the merge ref. A/B control built as git worktree add tmp/base-tree HEAD^1 with the HEAD test file copied in (sha256-identical) and packages/web-shell/node_modules symlinked (lockfile and every package.json untouched by the PR; readlink -f asserts internal workspace deps resolve to unchanged packages); mutations ran in tmp/head-mut (merge ref), applied by harness/apply-mutant.mjs (single-occurrence-anchored string replacements), each reverted before the next (git checkout verified clean). Suites: npx vitest run App.test.tsx --config vitest.config.ts (~32–40 s/run), the workspace-wide npx vitest run --config vitest.config.ts (181 files), and QWEN_SANDBOX=false npx vitest run cli/qwen-serve-webui-session-switching.test.ts from integration-tests/. Gates proven live before citing: planted TS2322 (tsc exit 2, named the planted file) and planted unused variable (ESLint exit 1, no-unused-vars), both removed after. Scripted harnesses: harness/ab-assert.mjs (9 checks), harness/matrix-assert.mjs (25 checks), harness/gates-assert.mjs (17 checks); evidence images via scripts/verify-capture.mjs. Assertion accounting: 9 vitest runs × 430 (head, base, control, M1–M6; base-arm reds counted as passes per the expected-failures rule encoded in ab-assert.mjs) + workspace suite 3333 + daemon suite 5 + harness checks 51 = 7259. Raw per-arm/per-mutant logs: logs/. Worktrees removed after capture.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
ytahdn
left a comment
ytahdn
left a comment
There was a problem hiding this comment.
Reviewed the current head (0ab4f16) against the prompt-safe navigation design contract — static functional review of App.tsx (+986/−160), cross-checked against the resolved R1 threads. Approving: no Critical/Important findings.
What I verified
- Identity model: block object reference → persisted
sourceRecordIds→ turn-errorpromptId/eventId; reducer-local block ids are only trusted within the same live attachment (allowLocalIdgated onsnapshot.isCurrent()). This correctly closes the local-id-reuse hazard across transcript replacement. - Owner model:
snapshot.isCurrent()is session-client instance identity, so same-id attachment replacement is caught;retryOwnerMatchesCurrentchecks sessionId + sourceVersion + workspace/snapshot, with fail-closed drops for workspace-unknown retries whose owner is gone. - Side-effect ordering:
lastSubmitted*refs,retryOwnerRefrefresh,clearFollowup(), and composer commit/clear all moved after both gates (post admission-hook, postensureSessionForPrompt) — stale continuations are genuinely side-effect free. - Retry retention:
attemptIdsupersedes out-of-order settlements; per-logical-session restoration is gated on navigation settle + transcript not loading/catching-up; rehydration is restricted to a matching preceding identity or an empty transcript. - Preparation races: the symbol preparation-ownership token prevents concurrent preparations from clearing each other's
isPreparingPrompt; the plan-mode.finally()+ token guard cannot leave the preparing state stuck. - R1 Critical fixes present at this head: turn-error rearm via
failedTurnErrorRetryRef/rearmFailedTurnErrorRetry,background_notificationskip ingetLatestUserBlock, local-id fallback for clone-on-write, andrestoreOrDeferCancelledRetryon not-admitted failures.
Notes (non-blocking)
- In the
.catchhandlers ofhandleFailedPromptRetry/handleRetry, the!retryOwnerIsCurrentearly return skipsrestoreOrDeferCancelledRetryif admission rejects (rather than cancels) while the user has navigated away. Low likelihood, pre-existing semantics; the message body remains in the transcript, only the retry affordance is lost. - Deferred
cancelledRetryStatesRefbuckets (which may carry base64 images) persist for sessions never revisited until App unmount (discussed in R1-7; bounded by sessions visited × 2 kinds).
Verification note: my review was static (no local test/typecheck run); the PR's stated evidence is 422/422 App suite plus mutation checks, and the CI Linux suite is green on this head.
already have 2 approved，3ks
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
|
Released in v0.21.11. |
5 participants
What this PR does
This PR hardens WebShell prompt ownership across session navigation. Direct and queued submissions now revalidate the App lifetime, logical session owner, composer source, and write-gate generation after asynchronous host admission and lazy session preparation, before clearing follow-ups, committing the composer, sending, or enqueueing.
Cancelled failed-prompt and turn-error retries are retained for their logical source session and restored only after navigation settles. Restoration uses persisted user-record identities or stable prompt/event identities instead of reducer-local block IDs, supports same-owner workspace enrichment and stable transcript replay, and fails closed when an attachment or transcript belongs to a different owner. The prompt-safe navigation design contract is updated to document these guarantees.
Why it's needed
#8882 preserves the source session while a target restore is prepared, but asynchronous WebShell continuations could still outlive a navigation transition. A host admission hook or shared lazy-session preparation could return after A→B or A→B→A and commit a stale draft, clear follow-up state, enqueue work, or lose the source retry. Transcript reducers also reuse local block IDs, so using those IDs across attachment replacement could expose a retry for the wrong payload.
The new checks keep stale continuations side-effect free, preserve source-owned draft and retry state, and ensure a retry is offered only when the current transcript has a stable identity match.
Reviewer Test Plan
How to verify
Use a delayed
onSubmitBeforehook in session A, begin a direct or queued submission, navigate A→B→A while the hook or lazy session preparation is pending, and then release it. Confirm no stale send or enqueue occurs, the composer and follow-up remain intact, and a valid source retry becomes available only after A is current and navigation has settled.Repeat with a failed-prompt retry and a turn-error retry while replacing the session attachment. Confirm a stable persisted replay retains exactly one retry, while a new transcript that only reuses the same reducer-local ID exposes no retry and cannot resend the old payload. With a hanging real daemon turn, navigate A→B→A and confirm A remains active and the request ledger contains no navigation-generated cancel, prompt replay, continuation, or mid-turn mutation.
Evidence (Before & After)
Before: focused regressions reproduced a direct send and queued enqueue after the navigation gate changed, and mutation tests reproduced hidden valid retries or stale retry UI after attachment/source-version changes.
After: the full WebShell App suite passes 422/422; the real-daemon session-switching suite passes 5/5 on the latest main; targeted mutation checks fail when the owner, generation, or transcript-identity guards are removed and pass with this change.
Tested on
Environment (optional)
macOS, Node.js 22.22.3, npm 10.9.8, local WebShell Vitest, and the bundled real-daemon integration harness with
QWEN_SANDBOX=false.Risk & Scope
Linked Issues
Refs #8923
中文说明
本 PR 做了什么
本 PR 加固了 WebShell 在会话导航期间的 prompt 所有权。直接提交和排队提交在异步宿主准入及懒会话准备完成后、清除 follow-up、提交 composer、发送或入队之前，会重新校验 App 生命周期、逻辑会话 owner、composer source 和 write-gate generation。
被取消的 failed-prompt 与 turn-error retry 会保留在其逻辑源会话中，并且只在导航结束后恢复。恢复逻辑使用持久化 user record identity 或稳定的 prompt/event identity，而不是 reducer 局部 block ID；它支持同 owner 的 workspace 补全和稳定 transcript replay，并在 attachment 或 transcript 属于不同 owner 时 fail closed。Prompt-safe 导航设计契约也同步记录了这些保证。
为什么需要
#8882 在目标 restore 准备期间保留源会话，但异步 WebShell continuation 仍可能晚于导航 transition 返回。宿主准入 hook 或共享懒会话准备可能在 A→B 或 A→B→A 之后继续提交旧 draft、清除 follow-up 状态、将任务入队，或者丢失源会话 retry。Transcript reducer 还会复用局部 block ID，因此跨 attachment replacement 使用这些 ID 可能为错误 payload 暴露 retry。
新的校验让过期 continuation 不产生副作用，保留源会话拥有的 draft 和 retry 状态，并且只在当前 transcript 具备稳定 identity 匹配时提供 retry。
Reviewer 测试计划
如何验证
在会话 A 中使用延迟的
onSubmitBeforehook，开始一次直接提交或排队提交，在 hook 或懒会话准备仍 pending 时执行 A→B→A，然后释放它。确认不会发生过期 send 或 enqueue，composer 和 follow-up 保持不变，并且有效的源会话 retry 只会在 A 成为当前会话且导航结束后恢复。分别对 failed-prompt retry 和 turn-error retry 重复 attachment replacement。确认具有稳定持久 identity 的 replay 只保留一个 retry，而仅复用相同 reducer 局部 ID 的新 transcript 不显示 retry，也不能重发旧 payload。使用挂起真实 daemon turn 时执行 A→B→A，确认 A 仍保持 active，并且请求账本中没有导航生成的 cancel、prompt replay、continuation 或 mid-turn mutation。
证据（修改前与修改后）
修改前：聚焦回归测试复现了导航 gate 变化后的直接 send 和 queued enqueue；mutation 测试也复现了 attachment/source-version 变化后有效 retry 被隐藏或过期 retry UI 被保留的问题。
修改后：完整 WebShell App 测试 422/422 通过；基于最新 main 的真实 daemon 会话切换测试 5/5 通过；移除 owner、generation 或 transcript-identity guard 时，定向 mutation 测试会失败，保留本次修复时则通过。
已测试平台
环境（可选）
macOS、Node.js 22.22.3、npm 10.9.8、本地 WebShell Vitest，以及设置
QWEN_SANDBOX=false的 bundle 真实 daemon 集成测试环境。风险与范围
关联 Issue
Refs #8923