fix(web-shell): message edit fails closed while the transcript window is incomplete - #10419
fix(web-shell): message edit fails closed while the transcript window is incomplete#10419yiliang114 wants to merge 9 commits into
Conversation
Message edit computed the rewind target from the rendered transcript window (a window-local user-turn ordinal), while the daemon's rewind snapshots are indexed session-globally. When the session history exceeds the loaded window, editing the last user message rewound to an earlier snapshot than requested (silent history loss) or failed with `rewind.empty` after the composer was already replaced. Fail closed: do not offer the edit affordance while older history is unloaded (hasOlderHistory) or was dropped for window capacity (historyCapacityReached). The shell-echo numbering skew suggested in the issue does not apply at the message level (echoes carry role `user_shell` and are excluded from numbering); a regression test pins this, along with the complete-window behavior. Fixes #10385 Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Thanks for splitting this out of the #9811 review!
Moving on to code review. 🔍 中文说明感谢把这个修复从 #9811 的 review 中拆出来！
进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewMy independent take before reading the diff: for a split-out fix riding #9811, the right scope is a fail-closed gate on the affordance now, with the authoritative session-global index source deferred — the alternative touches the daemon SDK boundary and is its own project. The PR does exactly this, so the approach matches what I would have shipped. Verified against the base branch code (
No critical findings. Non-blocking notes:
Test evidenceThis stacked PR has no CI coverage by design, not by accident:
Author-reported results (clearly attributed, not independently re-run — triage never executes PR code): 162/162 tests pass in Sandboxed verification would settle this: 中文说明代码审查看 diff 之前我的独立判断：对随 #9811 进入的拆分修复，正确的范围是现在先做失败即关闭的门控，把会话全局的权威索引来源留到后续——后者要动 daemon SDK 边界，是另一个项目。PR 正是这么做的，与我会采用的方案一致。 已对照基线分支代码（
无阻塞性问题。非阻塞备注：
测试证据这个堆叠 PR 按设计就没有 CI 覆盖，不是意外： 作者自报结果（明确标注为作者声明，未独立复跑——triage 从不执行 PR 代码）： 沙箱验证可以补上这一环： — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — clean static review of a minimal, correctly-scoped fail-closed fix; the one deduction is that the test suite could not be independently executed on this branch (structural, see below). Stepping back: this PR does one thing and does it the way I would have done it. The independent proposal for a split-out fix riding #9811 was exactly "gate the affordance now, source the index authoritatively later", and the diff is precisely that minimal set — two added conditions, two added memo deps, four tests. No drive-by changes, no scope creep, and the comment on the gate explains the invariant for the next reader. The tests pin both trigger conditions plus the complete-window regression, and the whole mechanism was traced end-to-end against the base branch — the gate can only err toward "editing unavailable", never toward "rewinds too far", which is the right trade for a data-loss-adjacent bug. Six months from now this reads as an honest, bounded fix, not debt. The 4/5 rather than 5/5 is the testing gap, stated plainly: stacked branches receive no CI by design ( The only loose end is the one from Stage 1: give the class-closing half of #10385 a follow-up issue when this lands. Verdict: approve, pinned to the reviewed commit. ✅ 中文说明置信度：4/5 —— 对一个范围正确、最小化的失败即关闭修复的干净静态审查；唯一扣分项是本分支上无法独立执行测试套件（结构性原因，见下）。 退一步看：这个 PR 只做一件事，而且做法与我会采用的一致。对随 #9811 进入的拆分修复，独立判断的方案就是"现在门控编辑入口，索引来源的权威化留到后续"，而 diff 恰好是这个最小集合——两个新增条件、两个新增 memo 依赖、四个测试。没有顺手改动，没有范围蔓延，门控上的注释也为后来者解释了不变量。测试钉住了两个触发条件和完整窗口的回归行为，整个机制也已在基线分支上端到端核实——门控只可能偏向"不可编辑"，绝不会偏向"回退过头"，对一个接近数据丢失的 bug 来说这是正确的取舍。六个月后回看，这是一个诚实、有边界的修复，不是债务。 给 4/5 而非 5/5 是测试缺口，直说：堆叠分支按设计没有 CI（ 唯一的遗留点是 Stage 1 提到的：本 PR 落地时，为 #10385 的彻底闭环部分建一个后续 issue。 结论：批准，钉在所审查的提交上。✅ — Qwen Code · qwen3.8-max Reviewed at |
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. ✅
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": verify the default maxBlocks / maxRetainedBytes cap values (exported as DAEMON_SESSION_DEFAULT_MAX_BLOCKS ; definition not located in budget) to quantify how…; "agent reverse-audit (round 1)": trace LiveMessageList embedders (e.g. split-view/side-task usages) to confirm they pass hasOlderHistory / historyCapacityReached through props.; "agent reverse-audit (round 1)": check whether cron/notification promptIds ( ########cron… , ########notification… ) create FHS snapshots that shift the global idx space and could make a con…; "agent 5": none — no check was cut short.; "agent reverse-audit (round 3)": did not execute the VS Code embedded flow end-to-end to observe the composer.editExpired toast live (static trace only, every hop read at HEAD)..
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
Test Plan (not a blocker): client/components/MessageList.dom.test.tsx — no such file or directory; components/MessageList.dom.test.tsx — no such file or directory; Tests 162 passed — this review observed 4943, 450 passed; 2 passed — this review observed 4943, 450 passed.
中文说明
仅完成部分审查，审查缺口已披露。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：verify the default maxBlocks / maxRetainedBytes cap values (exported as DAEMON_SESSION_DEFAULT_MAX_BLOCKS ; definition not located in budget) to quantify how…；"agent reverse-audit (round 1)"：trace LiveMessageList embedders (e.g. split-view/side-task usages) to confirm they pass hasOlderHistory / historyCapacityReached through props.；"agent reverse-audit (round 1)"：check whether cron/notification promptIds ( ########cron… , ########notification… ) create FHS snapshots that shift the global idx space and could make a con…；"agent 5"：none — no check was cut short.；"agent reverse-audit (round 3)"：did not execute the VS Code embedded flow end-to-end to observe the composer.editExpired toast live (static trace only, every hop read at HEAD).。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
Test Plan（非阻断）：client/components/MessageList.dom.test.tsx — no such file or directory; components/MessageList.dom.test.tsx — no such file or directory; Tests 162 passed — this review observed 4943, 450 passed; 2 passed — this review observed 4943, 450 passed。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| expect(onEditUserMessage).toHaveBeenCalledWith(1, 'q'); | ||
| }); | ||
|
|
||
| it('does not count user_shell echoes when numbering user turns', () => { |
There was a problem hiding this comment.
[Critical] R1-2: This test pins only half of the echo-skew question the PR description settles. editableUserTurn numbers every rendered role === 'user' message, but rewind snapshots are indexed by admitted daemon turns only (Session.ts makeSnapshot deliberately skips locally handled commands). Locally echoed slash commands — /stats, /status, /about, /context; none are hidden by VSCODE_HIDDEN_SLASH_COMMANDS — append kind: 'user' blocks with no meta via echoLocalCommandIfIdle (App.tsx:10498) → appendLocalUserTranscriptMessage, and the adapter renders them role: 'user', so they ARE counted while every gate flag correctly stays false on a complete window. The affordance then lands on the echo row itself, or passes an inflated ordinal for the next real prompt; editUserMessage finds no snapshot at that index → rewind.empty after the composer was replaced (standalone), or composer.editExpired at re-submit with subsequent sends kept cancelling until the editing state is cleared (the shipped VS Code flow, EmbeddedApp.tsx:1371-1414). One routine /stats breaks message editing for the rest of the live session view; the drift clears only on reload. A second entrance of the same root: a definitely-rejected send's optimistic block persists (only attachments are removed) and is counted too, with no snapshot counterpart. The user_shell case pinned here covers !-style shell echoes only — the description's "no numbering distortion at the message level" holds for those, not for slash-command echoes.
Witness (probe in the PR's own harness at the reviewed commit; it flips):
[..., echo('/stats')] → edit button rendered ON the echo row; onEditUserMessage(2, '/stats')
[..., echo, ..., userMsg('u2')] → clickEdit('u2') passes (2, 'q') against daemon snapshot indices {0,1}
same array minus the echo → clickEdit('u2') passes (1, 'q') ← matches the daemon index
Suggested fix: tag local echoes at creation (e.g. meta: { source: 'local_command' } in appendLocalUserTranscriptMessage — the adapter already lifts meta.source) and skip such blocks in the editableUserTurn producer (both the numbering and the lastId candidate); applying the same marker to never-dispatched optimistic blocks also closes the rejected-send entrance. Failing closed whenever such a block is present is the minimal in-PR alternative.
Fix acceptance criterion: a DOM test mounting [userMsg('u1'), asstMsg('a1'), localEchoMsg('e1'), userMsg('u2')] with the local marker — assert no edit-e1 button and clickEdit(c, 'u2') calling onEditUserMessage with (1, 'q'); removing the producer skip must turn it red.
中文说明
这个测试只钉住了 PR 描述所下的“回显序号偏差”结论的一半。editableUserTurn 对每一个渲染出的 role === 'user' 消息编号，但回退快照只按守护进程真正受理的轮次索引（Session.ts 的 makeSnapshot 有意跳过本地处理的命令）。本地回显的 slash 命令 —— /stats、/status、/about、/context，均未被 VSCODE_HIDDEN_SLASH_COMMANDS 隐藏 —— 经 echoLocalCommandIfIdle（App.tsx:10498）→ appendLocalUserTranscriptMessage 追加不带 meta 的 kind: 'user' 块，适配器把它们渲染为 role: 'user'，因此它们会被计入编号，而窗口完整时所有门控标志都正确地保持为 false。于是编辑入口要么落在回显行本身上，要么为下一条真实消息传入偏大的序号；editUserMessage 在该索引上找不到快照 → 独立端在 composer 已被替换后抛 rewind.empty；发货的 VS Code 流程（EmbeddedApp.tsx:1371-1414）则在重新提交时抛 composer.editExpired，并在编辑状态被清除前持续取消后续发送。一次再平常不过的 /stats 就会让当前会话视图里的消息编辑在整个会话期间失效（只有重载才恢复）。同一根因的第二个入口：被彻底拒绝的发送留下的乐观块仍然存在（只移除了附件）且同样被计入编号，却没有对应快照。这里钉住的 user_shell 用例只覆盖 ! 形式的 shell 回显 —— 描述中“消息层不存在序号偏差”的结论对 shell 回显成立，对 slash 命令回显不成立。
验证探针（在被审提交上用 PR 自带的测试框架运行，可翻转）：含回显时编辑按钮渲染在回显行上、传入 (2, '/stats')；回显之后的真实消息点击编辑传入 (2, 'q')，而守护进程快照索引只有 {0,1}；去掉回显后传入 (1, 'q')，与守护进程索引一致。
建议修复：在创建时给本地回显打标记（如在 appendLocalUserTranscriptMessage 中加 meta: { source: 'local_command' } —— 适配器已提升 meta.source），并在 editableUserTurn 生产者中跳过这类块（编号与 lastId 候选都跳过）；同一标记用于从未分发的乐观块即可同时关闭被拒发送的入口。退一步的最小方案：只要存在这类块就失败关闭。
验收标准：新增 DOM 测试，以 [userMsg('u1'), asstMsg('a1'), localEchoMsg('e1'), userMsg('u2')]（回显带本地标记）挂载，断言没有 edit-e1 按钮、clickEdit(c, 'u2') 以 (1, 'q') 调用 onEditUserMessage；移除生产者中的跳过逻辑必须让该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
Confirmed real at the reviewed head, but escalating instead of fixing in this pass — the correct fix exceeds the 3-file budget for this worker and needs a design decision.
Verified evidence:
- Snapshots skip local commands:
Session.ts(~5232) placesmakeSnapshotafter the slash-command/hook early returns, with a comment stating locally handled commands must not create phantom snapshots that desync the snapshot index. - Echo path:
echoLocalCommandIfIdle(App.tsx:7356; call sites 10498/10514 for/stats,/status,/about) →store.appendLocalUserMessage(text)with no meta → adapter rendersrole: 'user'(transcriptToMessages.ts:470-483 liftsmeta.sourcewhen present) → counted byeditableUserTurn(MessageList.tsx:2892, counts everyrole === 'user'). /stats//status//about//contextare absent fromVSCODE_HIDDEN_SLASH_COMMANDS(EmbeddedApp.tsx:83-103).- Rejected-send entrance confirmed: on a definite admission rejection the optimistic block persists with attachments stripped (actions.ts ~929-941) and no
user.text.deltaever merges into it.
Design questions blocking a worker-pass fix:
- Tagging surface spans ≥4 files: App.tsx echo wrapper, ChatPane.tsx direct echoes (
/goal~780,/context~1054 — the latter is a toolbar button, not a rare path), theeditableUserTurnproducer in MessageList.tsx, plus tests. - Marker mechanism matters:
appendTextDeltamerges event meta into the existing block meta (existing.meta = { ...existing.meta, ...event.meta }), so tagging optimistic prompt blocks would survive daemon admission and wrongly skip admitted turns from numbering. Only never-dispatched blocks may be tagged, which argues for per-call-site tagging or an SDK-level local-origin flag (API change). - Semantics decision: skip-and-renumber vs fail-closed-while-present for the echo case, and how the rejected-send entrance should interact with the retry affordance.
Leaving unresolved for maintainer decision.
A non-retryable load-older failure latches the session provider at hasMore=false, capacityReached=false, paginationError=true while the rendered window is still missing older turns. The edit gate only checked the first two flags, so the affordance reappeared and passed a window-local ordinal to the session-global rewind snapshot lookup — the #10385 defect class. Include historyPaginationError in the gate and in the render callback's dependency array. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
The new gate flags were pinned by no test: every affordance case mounted once with fixed props. Extend rerenderMessages to forward the edit callback and history flags, then flip hasOlderHistory and historyCapacityReached without changing the messages array identity. Dropping the flag entries from the render callback's dependency array makes both transition tests fail (mutation-probed). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-3 window-completeness invariant enforced at a single consumption site — already reported (comment 3883274771); author declined with verified single-consumer evidence and resolved
Not explored to full depth (tool budget reached): "agent reverse-audit (round 1)": full trace of the session-switch commit ordering in DaemonSessionProvider (loadSession/clearSession vs store.reset vs connection.sessionId) to confirm or rule o…; "agent reverse-audit (round 1)": daemon-side confirmation that the rewind-snapshot turn ledger (FileHistoryService idx + getRewindableUserTurnCount) matches the client role === 'user' count f…; "agent 1a": run the new vitest suite ( packages/web-shell MessageList.dom.test.tsx ) to confirm it is green — the review worktree has no node_modules , so execution woul….
Test Plan (not a blocker): client/components/MessageList.dom.test.tsx — no such file or directory; components/MessageList.dom.test.tsx — no such file or directory.
中文说明
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 1)"：full trace of the session-switch commit ordering in DaemonSessionProvider (loadSession/clearSession vs store.reset vs connection.sessionId) to confirm or rule o…；"agent reverse-audit (round 1)"：daemon-side confirmation that the rewind-snapshot turn ledger (FileHistoryService idx + getRewindableUserTurnCount) matches the client role === 'user' count f…；"agent 1a"：run the new vitest suite ( packages/web-shell MessageList.dom.test.tsx ) to confirm it is green — the review worktree has no node_modules , so execution woul…。
Test Plan（非阻断）：client/components/MessageList.dom.test.tsx — no such file or directory; components/MessageList.dom.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.2)
| !hasOlderHistory && | ||
| !historyCapacityReached && | ||
| !historyPaginationError && |
There was a problem hiding this comment.
[Critical] R2-1: The three flags negated here are treated as a complete signal that the window-local user-turn ordinal equals the session-global rewind-snapshot index, but the history provider has incomplete-window states that none of the three express. Proven by running the real provider: recordIds are stamped only during replay of persisted transcripts, never on the live event stream (bridge.ts ~6660), so when a client-side retention trim evicts the oldest blocks of a session lived from creation, onTruncation fires with no retained block carrying a recordId; the provider's fail-closed branch (DaemonSessionProvider.tsx ~858-877) then drops hasMore=false while preserving the other two flags at false — nothing records the eviction. The edit affordance is then offered on the last user message of a window that is missing persisted older turns; clicking it passes the window-local ordinal to editUserMessage (App.tsx:9439), which matches it against session-global snapshots and silently rewinds to an earlier turn, discarding everything after it — exactly the #10385 defect this gate exists to close.
Witness (real DaemonSessionProvider run, jsdom harness, maxBlocks: 2, live-only session — 3 user turns streamed, then count-trimmed):
PROBE-B observed: {"hasMore":false,"capacityReached":false,"paginationError":false,"loading":false,"windowKinds":["user","assistant"],"userBlocksInWindow":1,"userTurnsStreamed":3}
The window holds 1 of 3 user turns while all three gate flags read false. Two sibling states share the same root: the re-open byte gate (~836-856) only re-arms hasMore when postTrimRetainedBytes < byteCap — a single oversized block keeps it false after the trim; and the initial replay load (~1827-1834) computes historyHasMore false whenever the daemon lacks session_transcript_pagination (older external daemons) or no anchor recordId resolves, even when the replay itself was truncated. Default caps are large (50k blocks / 128MiB), so production reach needs a long pure-live session or a host-configured smaller window — but once reached, the state is deterministic.
Suggested fix: express window-completeness positively instead of as the complement of the three load flags — on an anchor-less eviction trim (and the truncated-replay initial-load states), have the provider latch a signal the gate can read (e.g. set capacityReached=true there, or add a fourth historyWindowIncomplete flag), and add it to this gate and to the render-callback dependency array below.
Fix acceptance criterion: a DaemonSessionProvider test asserting that after an anchor-less eviction trim the exposed history state still signals the window is incomplete, plus a case in this suite mounting with that signal set and asserting [data-testid="edit-u5"] is null — removing either the provider latch or the gate check must turn them red.
中文说明
这里取反的三个标志被当作"窗口内局部用户轮序号等于会话全局回退快照索引"的完备信号，但历史提供器存在这三者都无法表达的不完整窗口状态。通过运行真实提供器证实：recordId 只在重放持久化转录时打标，实时事件流上从不打标（bridge.ts ~6660），因此当一个从头活跃的会话被客户端保留策略裁剪掉最旧的块时，onTruncation 触发而留存块中没有任何一个携带 recordId；提供器的失败关闭分支（DaemonSessionProvider.tsx ~858-877）随即把 hasMore 置为 false，另外两个标志保持 false —— 驱逐发生得无声无息。此时窗口明明缺少已持久化的更早轮次，编辑入口却仍会出现在最后一条用户消息上；点击后把窗口内局部序号传给 editUserMessage（App.tsx:9439），后者按会话全局快照匹配，静默回退到更早的轮次，丢弃其后所有内容 —— 正是本门控要关闭的 #10385 缺陷。
验证证据（真实 DaemonSessionProvider 运行，jsdom 框架，maxBlocks: 2，纯实时会话 —— 流式输入 3 个用户轮后按数量裁剪）：窗口只剩 3 个用户轮中的 1 个，而三个门控标志全部为 false（见上方 PROBE-B 输出）。
同根因还有两个兄弟状态：重开字节门（~836-856）仅在 postTrimRetainedBytes < byteCap 时才重新置位 hasMore —— 单个超大块就能让裁剪后仍不满足；初始重放加载（~1827-1834）在守护进程不支持 session_transcript_pagination（较老的外部守护进程）或无法解析锚点 recordId 时，即使重放本身被截断，也会把 historyHasMore 算成 false。默认上限很大（5 万块 / 128MiB），生产上需要一个长时间的纯实时会话或宿主配置的更小窗口才会触发 —— 但一旦触发，状态是确定性的。
建议修复：用正向信号表达窗口完整性，而不是三个加载标志的取反 —— 在无锚点驱逐裁剪（以及截断重放的初始加载状态）时，让提供器置位一个门控可读的信号（如在那里置 capacityReached=true，或新增第四个 historyWindowIncomplete 标志），并把它加入本门控和下方的渲染回调依赖数组。
验收标准：DaemonSessionProvider 测试断言无锚点驱逐裁剪后暴露的历史状态仍标记窗口不完整；并在本套件中新增一个以该信号挂载的用例，断言 [data-testid="edit-u5"] 为 null —— 移除提供器置位或门控检查都必须让测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.2)
There was a problem hiding this comment.
Confirmed real at this head by code read — escalating instead of fixing in this pass, the correct fix is a provider-contract change beyond this worker's budget.
Verified evidence in DaemonSessionProvider.tsx:
- Anchor-less live-trim branch (~856-874): the state update is gated on
if (history.hasMore). A live-only session loads withhasMore=false(initial replay load ~1827 computeshistoryHasMore=falsewith no persisted history), so when a retention trim evicts oldest blocks and no retained block carries a recordId, nothing records the eviction — all three gate flags stay false on a truncated window. Matches the reported PROBE-B shape exactly. - Re-anchor branch byte gate (~836-856):
hasMoreis only re-armed whenolderHistoryReachable(pagination feature &&postTrimRetainedBytes < byteCap); a single oversized block leaves every flag false after the trim. capacityReachedis latched only on the replay-rebuild trim (~2184) and rejected-page (~3761) paths — never on live streaming trims.- recordIds flow in only via
evidenceCursoron replayed persisted records (mappers.ts~602); live-streamed blocks carry none, which is what makes live trims anchor-less. maxBlocks/maxRetainedBytesare host-configurable provider options (types.ts), so the state is reachable deterministically once a small window is configured or defaults are outlived.
Design questions blocking a worker-pass fix:
- No existing flag can express all three states honestly: latching
capacityReached=truemisstates the truncated-replay initial-load state (no capacity event occurred) and feeds the provider's own re-open-on-eviction logic (~884) and rejected-page footprint accounting;paginationErroris not an error;hasMore=truewould offer a load-older affordance the exclusive-before anchor contract can never satisfy — exactly what the fail-closed branch comment refuses. - The honest fix is a positive window-completeness signal (e.g. a fourth
historyWindowIncompleteflag), which is a provider API change:types.ts+DaemonSessionProvider.tsx(three latch sites) + both wiring call sites (App.tsx13247-13252,ChatPane.tsx1318-1321) + the MessageList gate/deps + tests — over the 3-file budget, with flag naming/semantics and the older-daemon truncated-replay case to decide.
Leaving unresolved for maintainer decision.
…flip test The historyPaginationError entry in the memoized render-callback dependency array had no re-render transition test: a mutation removing the entry kept the whole suite green because the mount-level pagination-error case creates the callback with current values on first render. Add a sibling-style flip test (mount with the flag set, re-render the identical messages array with it cleared) that goes red when the entry is removed, mirroring the existing hasOlderHistory and historyCapacityReached transition tests. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 1a": running the new DOM tests under vitest — the review worktree has no node_modules (root or packages/web-shell ), and a full monorepo install plus prerequisite….
Test Plan (not a blocker): client/components/MessageList.dom.test.tsx — no such file or directory; components/MessageList.dom.test.tsx — no such file or directory.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/web-shell/client/components/MessageList.dom.test.tsx:6534 — [probe] clickEdit re-implements the file-scoped click helperpackages/web-shell/client/components/MessageList.dom.test.tsx:6530 — [probe] no test pins the edit gate's !isResponding conditionpackages/web-shell/client/components/MessageList.dom.test.tsx:6549 — [probe] unflushed auto load-older chain emits act warningspackages/web-shell/client/components/MessageList.dom.test.tsx:6581 — [probe] no test pins the affordance on an assistant-tailed windowpackages/web-shell/client/components/MessageList.dom.test.tsx:6600 — [probe] prepend/grown-window turn-index recompute never exercisedpackages/web-shell/client/components/MessageList.dom.test.tsx:6587 — [probe] content argument provenance never pinnedpackages/web-shell/client/components/MessageList.dom.test.tsx:6603 — [probe] no false->true flag-flip (withdrawal) test
[Critical] R2-1: This round's re-check rules this blocker still stands — the gate (MessageList.tsx:5450-5452) still reads exactly the three flags (!hasOlderHistory && !historyCapacityReached && !historyPaginationError), MessageList.tsx is unchanged since the finding, and the provider states this entry names are live at this head: on an anchor-less retention trim of a live-only session the fail-closed branch (DaemonSessionProvider.tsx ~857-874) latches hasMore=false while capacityReached and paginationError stay false, so the window is missing persisted older turns while all three gate flags read false; the edit affordance is then offered on the last user message and clicking it passes the window-local ordinal to editUserMessage (App.tsx:9439), which matches it against session-global snapshots and silently rewinds to an earlier turn, discarding everything after it — exactly the #10385 defect this gate exists to close. Round-2 probe on the real provider observed the window holding 1 of 3 user turns while all three flags read false; round-3 re-read the anchor-less branch at dfec5b5, and the author confirmed real and escalated for the provider-contract fix. Suggested fix: express window-completeness positively instead of as the complement of the three load flags — on an anchor-less eviction trim (and the truncated-replay initial-load states) have the provider latch a signal the gate can read (e.g. set capacityReached=true there, or add a fourth historyWindowIncomplete flag) — and add it to this gate and the render-callback dependency array. Fix acceptance criterion: a DaemonSessionProvider test asserting that after an anchor-less eviction trim the exposed history state still signals the window is incomplete, plus a case in this suite mounting with that signal set and asserting [data-testid=edit-u5] is null — removing either the provider latch or the gate check must turn them red.
中文说明
未探索到全部深度（达到工具调用预算）："agent 1a"：running the new DOM tests under vitest — the review worktree has no node_modules (root or packages/web-shell ), and a full monorepo install plus prerequisite…。
Test Plan（非阻断）：client/components/MessageList.dom.test.tsx — no such file or directory; components/MessageList.dom.test.tsx — no such file or directory。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 7 条（原文未翻译，列表见上方英文部分）。
[Critical] R2-1: This round's re-check rules this blocker still stands — the gate (MessageList.tsx:5450-5452) still reads exactly the three flags (!hasOlderHistory && !historyCapacityReached && !historyPaginationError), MessageList.tsx is unchanged since the finding, and the provider states this entry names are live at this head: on an anchor-less retention trim of a live-only session the fail-closed branch (DaemonSessionProvider.tsx ~857-874) latches hasMore=false while capacityReached and paginationError stay false, so the window is missing persisted older turns while all three gate flags read false; the edit affordance is then offered on the last user message and clicking it passes the window-local ordinal to editUserMessage (App.tsx:9439), which matches it against session-global snapshots and silently rewinds to an earlier turn, discarding everything after it — exactly the #10385 defect this gate exists to close. Round-2 probe on the real provider observed the window holding 1 of 3 user turns while all three flags read false; round-3 re-read the anchor-less branch at dfec5b5, and the author confirmed real and escalated for the provider-contract fix. Suggested fix: express window-completeness positively instead of as the complement of the three load flags — on an anchor-less eviction trim (and the truncated-replay initial-load states) have the provider latch a signal the gate can read (e.g. set capacityReached=true there, or add a fourth historyWindowIncomplete flag) — and add it to this gate and the render-callback dependency array. Fix acceptance criterion: a DaemonSessionProvider test asserting that after an anchor-less eviction trim the exposed history state still signals the window is incomplete, plus a case in this suite mounting with that signal set and asserting [data-testid=edit-u5] is null — removing either the provider latch or the gate check must turn them red.
— qwen3.8-max via Qwen Code /review (v0.22.3)
…wind-turn-index The base branch independently landed the hasOlderHistory edit gate; keep this PR's superset that also fails closed on historyCapacityReached and terminal historyPaginationError, along with the explanatory comment. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R3 deferred
clickEditre-implements the file-scoped click helper (MessageList.dom.test.tsx:6531) — already reported (round-3 review 5057091635 deferral list); code unchanged since
Not explored to full depth (tool budget reached): "agent 4": none — no check was cut short..
Not reviewed: reverse audit — stopped before round 4 by the review time budget.
Test Plan (not a blocker): client/components/MessageList.dom.test.tsx — no such file or directory; components/MessageList.dom.test.tsx — no such file or directory.
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 4"：none — no check was cut short.。
未审查：反向审计——评审时间预算不足，未能开始第 4 轮。
Test Plan（非阻断）：client/components/MessageList.dom.test.tsx — no such file or directory; components/MessageList.dom.test.tsx — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.22.3)
| expect(c.querySelector('[data-testid="edit-u5"]')).not.toBeNull(); | ||
| }); | ||
|
|
||
| it('does not count user_shell echoes when numbering user turns', () => { |
There was a problem hiding this comment.
[Critical] R1-2: [fails-closed] This round's re-check rules this blocker still stands at 452ee22 — the base merge moved the echo path into sdk-typescript but changed nothing about the mechanism. editableUserTurn numbers every rendered role === 'user' message, but rewind snapshots are indexed by admitted daemon turns only. Locally echoed slash commands (/stats, /status, /about, /context) append kind:'user' blocks with no meta via echoLocalCommandIfIdle (App.tsx:7407) → store.appendLocalUserMessage → appendLocalUserTranscriptMessage (sdk-typescript/src/daemon/ui/transcript.ts:145), and the adapter renders meta-less blocks as role:'user' (adapters/transcriptToMessages.ts:419), so they ARE counted while every gate flag correctly stays false on a complete window. The affordance then lands on the echo row itself, or passes an inflated ordinal for the next real prompt; editUserMessage (App.tsx:9491) finds no snapshot at that index and throws rewind.empty after the composer was replaced. One routine /stats breaks message editing for the rest of the live session view; the drift clears only on reload. A second entrance of the same root: a definitely-rejected send's optimistic block persists and is counted too, with no snapshot counterpart. The user_shell case pinned here covers !-style shell echoes only.
Witness (fresh DOM probe at 452ee22 in this review's scratch tree; it flips):
[..., echo('/stats')] -> edit button rendered ON the echo row; onEditUserMessage(1, '/stats') vs snapshots {0}
[..., echo, a1, u2] -> clickEdit('u2') passes (2, 'q') against snapshot indices {0,1}
same array minus the echo -> clickEdit('u2') passes (1, 'q') <- matches the daemon index
echo as role 'user_shell' -> not counted, no button on the echo row
Suggested fix: tag local echoes at creation (e.g. meta: { source: 'local_command' } in the appendLocalUserMessage call chain) and skip such blocks in the editableUserTurn producer (both the numbering and the lastId candidate); applying the same marker to never-dispatched optimistic blocks also closes the rejected-send entrance. Failing closed whenever such a block is present is the minimal in-PR alternative.
Fix acceptance criterion: a DOM test mounting [userMsg('u1'), asstMsg('a1'), localEchoMsg('e1'), userMsg('u2')] with the local marker — assert no edit-e1 button and clickEdit(c, 'u2') calling onEditUserMessage with (1, 'q'); removing the producer skip must turn it red.
中文说明
本轮复查裁定该阻断在 452ee22 上依然成立 —— base 合并把回显路径挪进了 sdk-typescript，但机制本身没有任何变化。editableUserTurn 对每一个渲染出的 role === 'user' 消息编号，但回退快照只按守护进程真正受理的轮次索引。本地回显的 slash 命令（/stats、/status、/about、/context）经 echoLocalCommandIfIdle（App.tsx:7407）→ store.appendLocalUserMessage → appendLocalUserTranscriptMessage（sdk-typescript/src/daemon/ui/transcript.ts:145）追加不带 meta 的 kind:'user' 块，适配器把不带 meta 的块渲染为 role:'user'（adapters/transcriptToMessages.ts:419），因此它们会被计入编号，而窗口完整时所有门控标志都正确地保持为 false。于是编辑入口要么落在回显行本身上，要么为下一条真实消息传入偏大的序号；editUserMessage（App.tsx:9491）在该索引上找不到快照，在 composer 已被替换后抛出 rewind.empty。一次再平常不过的 /stats 就会让当前会话视图里的消息编辑在整个会话期间失效（只有重载才恢复）。同一根因的第二个入口：被彻底拒绝的发送留下的乐观块仍然存在且同样被计入编号，却没有对应快照。这里钉住的 user_shell 用例只覆盖 ! 形式的 shell 回显。
验证探针（本轮在 452ee22 上的独立临时树中运行的全新 DOM 探针，可翻转）：含回显时编辑按钮渲染在回显行上、相对快照集 {0} 传入 (1, '/stats')；回显之后的真实消息点击编辑传入 (2, 'q')，而快照索引只有 {0,1}；去掉回显后传入 (1, 'q')，与守护进程索引一致；回显以 user_shell 角色渲染时不被计入、回显行上无按钮。
建议修复：在创建时给本地回显打标记（如在 appendLocalUserMessage 调用链中加 meta: { source: 'local_command' }），并在 editableUserTurn 生产者中跳过这类块（编号与 lastId 候选都跳过）；同一标记用于从未分发的乐观块即可同时关闭被拒发送的入口。退一步的最小方案：只要存在这类块就失败关闭。
验收标准：新增 DOM 测试，以 [userMsg('u1'), asstMsg('a1'), localEchoMsg('e1'), userMsg('u2')]（回显带本地标记）挂载，断言没有 edit-e1 按钮、clickEdit(c, 'u2') 以 (1, 'q') 调用 onEditUserMessage；移除生产者中的跳过逻辑必须让该测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
There was a problem hiding this comment.
Re-assertion of the open R1-2 thread (PRRT_kwDOPB-92c6dRhRj) — same file, same echo-skew finding, unchanged at this head. The author already responded there (3884091068) and the fix is deliberately human-gated: per-call-site tagging vs. an SDK-level flag spans >3 files and needs a design call before code lands. Tracking continues in the original thread; this duplicate stays open but unactioned until that decision is made.
There was a problem hiding this comment.
Same blocker as the original R1-2 thread (PRRT_kwDOPB-92c6dRhRj), where the author has already responded. The fix remains human-gated: tagging local-echo/rejected-send blocks and skipping them in the editableUserTurn producer is a >3-file design decision spanning the sdk-typescript transcript store (appendLocalUserTranscriptMessage), the client adapter, and the test suite. The branch is being actively revised by the PR team (head 14cfa47 merged today); keeping this open until that work lands. No code changes from our side this round.
| // (or none at all). Fail closed (#10385). | ||
| !hasOlderHistory && | ||
| !historyCapacityReached && | ||
| !historyPaginationError && |
There was a problem hiding this comment.
[Critical] R2-1: [certifies-falsely] This round's re-check rules this blocker still stands at 452ee22 — the gate still reads exactly the three flags, and the provider still has incomplete-window states none of them express. (a) Anchor-less live retention trim: DaemonSessionProvider.tsx:839-876 re-opens hasMore only when olderHistoryReachable (pagination feature advertised AND postTrimRetainedBytes < byteCap); when it is not, and in the uncomputable-anchor branch, nothing is latched while older turns were evicted. (b) Truncated replay on a daemon that does not advertise session_transcript_pagination (or has no firstPersistedRecordId): historyHasMore computes false (DaemonSessionProvider.tsx:1830-1837) and the injection (1855-1864) publishes hasMore:false, capacityReached:false, paginationError:false over a truncated window. In either state the gate opens, the affordance is offered on the last user message, and clicking it passes the window-local ordinal to editUserMessage, which matches it against session-global snapshots and silently rewinds to an earlier turn, discarding everything after it — exactly the #10385 defect this gate exists to close. This round's four finder agents independently rediscovered these entrances; they fold into this re-post under the original id.
Witness (probe driving the real DaemonSessionProvider at 452ee22):
[no-pagination-feature] -> {"hasMore":false,"capacityReached":false,"paginationError":false}, window holds 1 of the session's user turns, truncation marker visible
[feature-but-no-anchor] -> {"hasMore":false,"capacityReached":false,"paginationError":false}, same shape
[feature-and-anchor] -> {"hasMore":true,...} <- flips, proving the probe discriminates
Suggested fix: express window-completeness positively instead of as the complement of the three load flags — on an anchor-less eviction trim and the truncated-replay injection states have the provider latch a signal the gate can read (e.g. set capacityReached=true there, or add a fourth historyWindowIncomplete flag), and add it to this gate and the render-callback dependency array. The fix must not re-open the load-older affordance anchor-less: DaemonSessionProvider.tsx:895-898 — if (!paginationSupported || !anchored) { return; } — and the fail-closed eviction branch (862-876) deliberately drops both anchors. Fix acceptance criterion: a DaemonSessionProvider test asserting that after an anchor-less eviction trim or a truncated replay the exposed history state still signals the window is incomplete, plus a case in this suite mounting with that signal set and asserting [data-testid=edit-u5] is null; removing either the provider latch or the gate check must turn them red.
中文说明
本轮复查裁定该阻断在 452ee22 上依然成立 —— 门控仍然只读取这三个标志，而 provider 仍存在这三个标志都无法表达的不完整窗口状态。(a) 无锚点的在线保留裁剪：DaemonSessionProvider.tsx:839-876 仅在 olderHistoryReachable（通告了分页功能且 postTrimRetainedBytes < byteCap）时才重新打开 hasMore；不满足时、以及重锚点不可计算分支中，较早轮次已被驱逐却没有置位任何标志。(b) 未通告 session_transcript_pagination（或没有 firstPersistedRecordId）的守护进程上的截断重放：historyHasMore 计算为 false（DaemonSessionProvider.tsx:1830-1837），注入（1855-1864）在截断的窗口上发布 hasMore:false, capacityReached:false, paginationError:false。在这两种状态下门控都会打开，编辑入口出现在最后一条用户消息上，点击后把窗口内局部序号传给 editUserMessage，与全局快照匹配后静默回退到更早的轮次，丢弃其后的一切 —— 正是本门控要关闭的 #10385 缺陷。本轮四个探查代理独立重新发现了这些入口；现以原编号并入本条重发。
验证探针（在 452ee22 上驱动真实 DaemonSessionProvider）：[无分页功能] 与 [有功能但无锚点] 两个分支均在窗口只含会话 1 个用户轮、截断标记可见的情况下发布 {"hasMore":false,"capacityReached":false,"paginationError":false}；[有功能且有锚点] 分支翻转为 {"hasMore":true,...}，证明探针具有区分力。
建议修复：把窗口完整性表达为正向信号，而不是三个加载标志的补集 —— 在无锚点驱逐裁剪与截断重放注入状态中，让 provider 置位一个门控可读的信号（如在那里设 capacityReached=true，或新增第四个 historyWindowIncomplete 标志），并把它加入本门控与渲染回调依赖数组。修复不得在无锚点时重新打开加载更早内容的入口：DaemonSessionProvider.tsx:895-898 —— if (!paginationSupported || !anchored) { return; } —— 失败关闭的驱逐分支（862-876）有意丢弃两个锚点。验收标准：DaemonSessionProvider 测试断言无锚点驱逐裁剪或截断重放后暴露的历史状态仍发出窗口不完整信号，并在本套件中以该信号置位挂载、断言 [data-testid=edit-u5] 为 null；移除 provider 置位或门控检查任一项都必须让测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
There was a problem hiding this comment.
Re-assertion of the open R2-1 thread (PRRT_kwDOPB-92c6dUzxW) — same line (MessageList.tsx:5452), same finding: the gate reads the three negated flags as a complete window-completeness signal while the provider has incomplete-window states none of them express. The author already responded there (3885451398); the fix is human-gated on the provider-contract decision (positive window-completeness signal vs. a fourth flag, ~5 files). Tracking continues in the original thread; this duplicate stays open until that decision is made.
There was a problem hiding this comment.
Same blocker as the original R2-1 thread (PRRT_kwDOPB-92c6dUzxW), where the author has already responded. The fix remains human-gated: expressing window-completeness positively (a provider latch on anchor-less eviction / truncated replay, or a fourth flag) is a provider-contract design decision in DaemonSessionProvider that the PR team owns. The branch is being actively revised by the PR team (head 14cfa47 merged today); keeping this open until that decision lands. No code changes from our side this round.
| // hasMore=false and merges nothing), the window-local | ||
| // user-turn ordinal is not the session-global turn index, | ||
| // so offering edit here would rewind to the wrong snapshot | ||
| // (or none at all). Fail closed (#10385). |
There was a problem hiding this comment.
[Critical] R4-1: [fails-closed] The gate's certified invariant — window-local user-turn ordinal equals the session-global rewind-snapshot index — breaks even on a COMPLETE window once a session exceeds 100 admitted user turns. FileHistoryService caps snapshots at MAX_SNAPSHOTS = 100 (packages/core/src/services/fileHistoryService.ts:103) and evicts the oldest (816-820), while the rewind_snapshots RPC reports turnIndex: idx — the position in the evicted array (packages/cli/src/acp-integration/acpAgent.ts:8543). getRewindableUserTurnCount is uncapped (Session.ts:3981-3994), so above the cap the reported set stays 0..99 while the transcript ordinal keeps counting. A session with 101 admitted user turns fits entirely in the window (DEFAULT_MAX_BLOCKS = 1000), all three flags are false, and this gate offers edit — but the last message's ordinal (100) never matches the reported 0..99 set, so every click throws rewind.empty after the composer was replaced: the affordance is offered yet deterministically dead in exactly the state this comment certifies as safe. The same idx < rewindableTurnCount filter also drops the newest snapshots when context compression shrinks the API-history user-turn count, so compressed sessions fail identically below 100 turns (verified in mechanism; over-rewind is structurally excluded — the probe confirmed every surviving entry still maps to its correct turn).
Witness (probe running the real FileHistoryService at 452ee22):
ARM A (N=100): reportedTurnIndexes 0..99, lastOrdinal 99 -> lookup resolves, edit works
ARM B (N=101): firstPromptId rebased to turn 2, lastOrdinal 100 -> lookup 'rewind.empty', edit dead
ARM C (50 snapshots, rewindableTurnCount=30): only idx 0..29 reported -> 'rewind.empty', overRewindPossible: false
Deterministic split at exactly MAX_SNAPSHOTS + 1.
Suggested fix: stop deriving the rewind target from a positional index — resolve it by promptId/record id (the daemon already resolves promptId → snapshot, acpAgent.ts:11366-11382), or have the RPC report eviction-aware global ordinals; if the fix must stay inside this PR's client-side gate, fail closed when the window's user-turn count exceeds the resolvable snapshot count (≤ MAX_SNAPSHOTS). The fix must respect MAX_SNAPSHOTS = 100 (fileHistoryService.ts:103) and the positional turnIndex: idx contract (acpAgent.ts:8543) that the rewind handler's promptId → array-position resolution (acpAgent.ts:11382) relies on. Fix acceptance criterion: a case in this describe mounting a complete window of 101 user messages and asserting the edit button is NOT rendered (or that the handler receives a snapshot-resolvable id, if the promptId fix is chosen); removing the guard must turn it red.
中文说明
门控所认证的不变量 —— 窗口内用户轮序号等于会话全局回退快照索引 —— 在会话超过 100 个受理轮次后，即使窗口完整也会失效。FileHistoryService 把快照上限设为 MAX_SNAPSHOTS = 100（packages/core/src/services/fileHistoryService.ts:103）并驱逐最旧的（816-820），而 rewind_snapshots RPC 报告的是 turnIndex: idx —— 被驱逐后数组中的位置（packages/cli/src/acp-integration/acpAgent.ts:8543）。getRewindableUserTurnCount 不设上限（Session.ts:3981-3994），因此超过上限后上报集合仍是 0..99，而转录序号继续增长。101 个受理轮的会话完全放得进窗口（DEFAULT_MAX_BLOCKS = 1000），三个标志全为 false，门控放行编辑 —— 但最后一条消息的序号（100）永远匹配不上 0..99 的上报集合，每次点击都在 composer 已被替换后抛出 rewind.empty：入口被提供、在这段注释认证为安全的状态里却确定性地失效。同一 idx < rewindableTurnCount 过滤器也会在上下文压缩减少 API 历史用户轮数时丢掉最新的快照，因此压缩后的会话在 100 轮以下同样失效（机制已验证；过度回退在结构上被排除 —— 探针确认所有幸存条目仍映射到正确的轮次）。
验证探针（在 452ee22 上运行真实 FileHistoryService）：ARM A（N=100）上报 0..99、最后序号 99 → 查找到，编辑可用；ARM B（N=101）firstPromptId 重定基到第 2 轮、最后序号 100 → 'rewind.empty'，编辑失效；ARM C（50 个快照、rewindableTurnCount=30）只上报 0..29 → 'rewind.empty'，overRewindPossible: false。分裂点恰在 MAX_SNAPSHOTS + 1。
建议修复：不要再从位置索引推导回退目标 —— 按 promptId/记录 id 解析（守护进程已能把 promptId 解析到快照，acpAgent.ts:11366-11382），或让 RPC 上报感知驱逐的全局序号；若修复必须留在本 PR 的客户端门控内，则在窗口用户轮数超过可解析快照数（≤ MAX_SNAPSHOTS）时失败关闭。修复必须尊重 MAX_SNAPSHOTS = 100（fileHistoryService.ts:103）与位置式 turnIndex: idx 契约（acpAgent.ts:8543）—— 回退处理器的 promptId → 数组位置解析（acpAgent.ts:11382）依赖它。验收标准：在本 describe 中新增以 101 条用户消息的完整窗口挂载的用例，断言编辑按钮不渲染（若选择 promptId 修复，则断言处理器收到可解析的快照 id）；移除该守卫必须让测试变红。
— qwen3.8-max via Qwen Code /review (v0.22.3)
There was a problem hiding this comment.
Verified as a real, distinct angle at 452ee22 — checked the anchors at this head:
MAX_SNAPSHOTS = 100with oldest-first eviction — packages/core/src/services/fileHistoryService.ts:103, 816-820rewind_snapshotsreports the positionalturnIndex: idxfiltered byidx < rewindableTurnCount— packages/cli/src/acp-integration/acpAgent.ts:8528-8543getRewindableUserTurnCount()is uncapped and counts API-history user turns — packages/cli/src/acp-integration/session/Session.ts:3981-3994 (note: lives in cli/acp-integration/session, not core)
So the gate's certified invariant (window-local ordinal == session-global snapshot index) breaks on a COMPLETE window past 100 admitted turns, and the rewindableTurnCount filter reproduces it below 100 under context compression. Distinct from R2-1: the window is complete and all three flags are correctly false here, so R2-1's positive completeness signal would not close this entrance. Leaving open — the durable fix (promptId-keyed snapshot resolution, which the daemon already supports for rewind execution, or eviction-aware ordinals) crosses core/cli and needs a design call rather than a client-side gate tweak in this PR.
There was a problem hiding this comment.
Verified against head 14cfa47 — the mechanism holds at every cited point:
MAX_SNAPSHOTS = 100with oldest-eviction:packages/core/src/services/fileHistoryService.ts:103,816-820rewind_snapshotsreports positionalturnIndex: idxover the post-eviction array, filtered byidx < rewindableTurnCount:packages/cli/src/acp-integration/acpAgent.ts:8528,8538-8543getRewindableUserTurnCountis uncapped and derived from API history (so compression shrinks it):packages/cli/src/acp-integration/session/Session.ts:3990- the gate reads only the three window flags, no snapshot-cap signal:
packages/web-shell/client/components/MessageList.tsx:5450-5452 editUserMessagematchesentry.turnIndex === turnIndexand throwsrewind.emptyafter the composer is replaced:packages/web-shell/client/App.tsx:9545-9548- a 101-turn session fits one complete window:
DEFAULT_MAX_BLOCKS = 1_000,packages/sdk-typescript/src/daemon/ui/transcript.ts:31
Classification: this is the same invariant class as R1-2/R2-1 — the gate's certified invariant (window-local user-turn ordinal == session-global rewind-snapshot index) is broken by server-side state (snapshot-cap eviction, compression-shrunk turn count) that none of the gate's client-side flags can express. No <=3-file fix exists: id-based resolution or eviction-aware ordinals changes the rewind-snapshot RPC contract across acpAgent.ts / Session.ts / fileHistoryService.ts plus the client gate, and a client-only fail-closed is impossible because the client cannot know the resolvable snapshot count without a provider signal. Folding into the same human-gated provider-contract design decision tracked in the original R2-1 thread (PRRT_kwDOPB-92c6dUzxW). No code changes this round; the branch is being actively revised by the PR team.
…wind-turn-index Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…wind-turn-index Keep historyCapacityReached/historyPaginationError in the display-item callback deps: the #10385 fail-closed edit condition reads both inside the callback body. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…x' into prmerge-10419
…wind-turn-index Keeps the R1-1 historyPaginationError fail-closed gate term and its useCallback dependency entry where the base sync (8a51002, 3cf5e5f) independently landed the historyCapacityReached term; both terms are preserved in the merged gate. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Base sync + conflict resolution (patrol closeout):
|
|
Superseded — closing. The same-design fix is already on |
2 participants
What this PR does
Message edit in the WebShell transcript computed the rewind target from the rendered transcript window:
MessageListnumbersrole === 'user'messages inside the loadedmessageswindow and passed that window-local ordinal toonEditUserMessage, while the daemon's rewind snapshots are indexed session-globally. This PR fails closed: the edit affordance is no longer offered while the window is known incomplete —hasOlderHistory(older pages not loaded yet) orhistoryCapacityReached(older blocks dropped for window capacity). When the window is complete the behavior is unchanged.Why it's needed
When the session's user-turn count exceeds the loaded transcript window, the window-local ordinal of the last user message is smaller than its session-global turn index. Editing the last message then either resolves to an earlier snapshot than requested (silent over-truncation of session history) or finds no matching snapshot and fails with
rewind.emptyafter the composer was already replaced (Fixes #10385, split out of the review on #9811).Reviewer Test Plan
How to verify
Component-level reproduction against the real
MessageList(inpackages/web-shell/client/components/MessageList.dom.test.tsx, describeuser message edit affordance (issue #10385)): render a window containing only the last two of five session-global user turns withhasOlderHistory={true}(and separatelyhistoryCapacityReached={true}) and anonEditUserMessagespy.1instead of the session-global4). The two incomplete-window tests fail.Regression tests in the same block: with a complete window, editing is offered only on the last user message and passes its ordinal (unchanged behavior), and
user_shellechoes do not skew the numbering — they carryrole: 'user_shell', not'user', so the shell-echo distortion suggested in the issue does not exist at the message level (pinned by the test).cd packages/web-shell npx vitest run --config vitest.config.ts client/components/MessageList.dom.test.tsxResult:
Test Files 1 passed (1),Tests 162 passed (162)(includes the 4 new tests).npm run typecheck,eslint, andprettier --checkare clean on the touched files.Evidence (Before & After)
Before (on this branch without the
MessageList.tsxgate, new tests only):After (with the gate):
Tests 162 passed (162).Tested on
Environment (optional)
Unit/component tests only (
vitest, jsdom):npm ciat repo root on the #9811 head86c1c689, then the vitest/typecheck/eslint/prettier commands above insidepackages/web-shell.Risk & Scope
RewindDialog.promptTextForTurndisplay-text numbering noted in the issue thread; end-to-end run against a live daemon.Linked Issues
Fixes #10385
This PR targets
codex/vscode-web-shell-cutoverbecause the defect only exists on the #9811 branch (main has no edit affordance); the fix rides into main with #9811.中文说明
这个 PR 做了什么
WebShell 会话记录里的"编辑消息"原来从渲染出来的转录窗口计算回退目标：
MessageList只对已加载messages窗口内的role === 'user'消息编号，并把这个窗口内局部序号传给onEditUserMessage；而守护进程的 rewind 快照是按会话全局索引的。本 PR 采用失败即关闭策略：当窗口已知不完整时（hasOlderHistory，更早的分页尚未加载；或historyCapacityReached，更早的块因窗口容量被丢弃）不再提供编辑入口。窗口完整时行为不变。为什么需要
当会话的用户轮数超过已加载的转录窗口时，最后一条用户消息的窗口内局部序号会小于它的会话全局轮号。此时编辑最后一条消息要么命中比预期更早的快照（静默过度截断会话历史），要么找不到匹配快照、在 composer 已被替换后才报
rewind.empty（修复 #10385，从 #9811 的 review 中拆出）。Reviewer 测试计划
如何验证
针对真实
MessageList的组件级复现（位于packages/web-shell/client/components/MessageList.dom.test.tsx，describe 名为user message edit affordance (issue #10385)）：构造"会话全局共 5 个用户轮、窗口只含最后 2 个"的场景，分别传hasOlderHistory={true}和historyCapacityReached={true}，并挂onEditUserMessage间谍。1而非会话全局的4）。两个不完整窗口用例失败。同一块里的回归用例：窗口完整时编辑只在最后一条用户消息上提供且传入其序号（行为不变）；
user_shell回显不影响编号——它们的角色是user_shell而非user，因此 issue 中提到的 shell 回显序号偏差在消息层并不存在（已用测试钉住）。cd packages/web-shell npx vitest run --config vitest.config.ts client/components/MessageList.dom.test.tsx结果：
Test Files 1 passed (1)、Tests 162 passed (162)（含 4 个新用例）。改动文件的npm run typecheck、eslint、prettier --check均通过。证据（修改前后）
修改前（在本分支上去掉
MessageList.tsx门控、只保留新测试）：修改后（加门控）：
Tests 162 passed (162)。测试环境
环境（可选）
仅单元/组件测试（
vitest、jsdom）：在 #9811 head86c1c689上仓库根目录npm ci，然后在packages/web-shell内执行上述 vitest/typecheck/eslint/prettier 命令。风险与范围
RewindDialog.promptTextForTurn展示文本编号；对真实运行守护进程的端到端验证。关联 Issue
Fixes #10385
本 PR 以
codex/vscode-web-shell-cutover为目标分支，因为缺陷只存在于 #9811 分支上（main 没有编辑功能）；修复将随 #9811 一起进入 main。