feat(serve): add session turn navigation protocol - #10751
Merged
Conversation
doudouOUC
requested review from
LaZzyMan,
qqqys,
tanzhenxin,
wenshao and
yiliang114
as code owners
September 1, 2026 23:56
Collaborator
Author
Phase 1 E2E reportStatus: Passed BaselineThe released Local bundleRan the bundled local Observed result: {
"capability": true,
"initialTurns": 2,
"frozenTurnsAfterAppend": 2,
"freshTurnsAfterAppend": 3,
"anchoredTarget": "22222222-2222-4222-8222-222222222222",
"invalidAnchorRejected": true,
"tamperedSnapshotRejected": true
}This confirms capability discovery, stable old-snapshot pagination after append, fresh-snapshot visibility, exact anchored transcript location, and rejection of invalid anchors and modified snapshot tokens. The daemon stopped cleanly and the isolated runtime was removed. Additional verification
|
23 tasks
Collaborator
Collaborator
|
Re-run at new head Template: complete ✓ — unchanged, bilingual, with a concrete Reviewer Test Plan. Problem: unchanged — a documented Web Shell limitation (the compact rail can only represent turns already loaded; the transcript API is sequential-only), backed by #10750, which is open and triaged: Direction: unchanged — aligned with the session-management roadmap that issue sits on. Phase 1 (daemon/SDK protocol ahead of the Phase 2 browser layer and Phase 3 UI) is the delivery order the committed design record describes. Size: production logic still 1,435 lines (17 files, unchanged — the delta is test-only); tests now 1,342 (+103); design doc 732. One correction to prior passes: the author holds admin permission on this repo, and the core two-tier gate is maintainer-exempt — so the Stage 0 "maintainer awareness" escalation that capped earlier runs is withdrawn. The size stays recorded for the record, and I still see no useful way to split it: index, signed snapshots, and anchored reads depend on each other. Approach: unchanged and right-sized. The new delta is exactly the follow-up the sandboxed verification report asked for — format-preserving snapshot forgery, cold-cache frozen-prefix stability, plus timeout and flush-semantics pinning. No scope creep, no drive-by production edits. Risk: one revert-correlated high-risk path remains in the file set ( Moving on to code review 🔍 中文说明在新 head **模板：**完整 ✓——无变化，中英对照，Reviewer Test Plan 具体。 **问题：**无变化——Web Shell 的既有局限（紧凑导航条只能表示已加载的 turn；transcript API 为顺序式），由 #10750 佐证；该 issue 开放且已完成 triage： **方向：**无变化——与该 issue 所在的 session-management 路线图一致。Phase 1（先交付 daemon/SDK 协议，浏览器层与 UI 分别为 Phase 2/3）是已提交设计文档描述交付顺序。 **规模：**生产逻辑仍为 1,435 行（17 个文件，未变——本次增量纯测试）；测试现为 1,342（+103）；设计文档 732。对前几轮的一处更正：作者在本仓库持有 admin 权限，核心两档关卡对维护者豁免——因此此前各轮所依据的 Stage 0"维护者关注"升级予以撤回。规模仍记录在案；依然没有有意义的拆分方式：索引、签名快照与锚点读取互相依赖。 **方案：**范围合理，无变化。新增量正是沙箱验证报告所要求的跟进——格式保持的快照伪造测试、冷缓存下的冻结前缀稳定性，以及超时与 flush 语义的钉住。无范围蔓延，无顺手的生产代码改动。 **风险：**文件集中仍有一条与 revert 相关的高风险路径（ 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
Code review (re-run at
|
| File | What changed |
|---|---|
docs/design/web-shell/web-shell-global-turn-navigation.md |
Design record for the full three-phase plan; this PR implements Phase 1 |
integration-tests/cli/qwen-serve-routes.test.ts |
Capability envelope expectation plus a real-daemon owner-path E2E: indexing, anchored open, frozen-after-append, tampered-snapshot rejection |
packages/acp-bridge/src/bridge.test.ts |
Forwarding assertion upgraded to full-shape toEqual; new fake-timer test for a hung turn-index ext call rejecting with BridgeTimeoutError (this delta) |
packages/acp-bridge/src/bridge.ts |
New requestSessionTurnIndexPage calling the child ext method, exposed as getSessionTurnIndexPage |
packages/acp-bridge/src/bridgeTypes.ts |
Bridge request and response types for turn-index pages and anchored transcript fields |
packages/acp-bridge/src/status.ts |
Registers the qwen/status/session/turn_index ext-method name |
packages/acp-bridge/src/transcript-replay.ts |
stripGeneratedAttachmentTokens moved to core utils; no behavior change |
packages/cli/src/acp-integration/acpAgent.test.ts |
ACP error-taxonomy tests plus, this delta, flush-once semantics across a latest-then-frozen turn-index pair |
packages/cli/src/acp-integration/acpAgent.ts |
Validates and forwards atRecordId and snapshot; adds the sessionTurnIndex ext method with error mapping matching the transcript handler |
packages/cli/src/serve/capabilities.ts |
Advertises session_turn_navigation in the capability registry |
packages/cli/src/serve/multi-workspace-sessions.test.ts |
Multi-workspace tests for the two new surfaces |
packages/cli/src/serve/routes/session.ts |
Two new turn-index routes; transcript routes extended with atRecordId and snapshot; owner-resolved route now applies the response byte cap |
packages/cli/src/serve/server.test.ts |
Route-level tests for parameter combinations and snapshot stability, including 400-before-bridge cases |
packages/cli/src/serve/server/error-response.ts |
Maps InvalidSessionTranscriptTurnAnchorError to 400 invalid_turn_anchor in both error paths |
packages/cli/src/serve/server/error-response.test.ts |
Both serialized and direct invalid_turn_anchor mapping to the public 400 contract |
packages/cli/src/serve/server/telemetry-catalog.test.ts |
Drift guard count 62 to 63 — the fix for the red Test lane from the maintainer review |
packages/cli/src/serve/server/telemetry.test.ts |
Route catalog count 62 to 63 with the 61/2 split; attribution test for the workspace turn-index route |
packages/cli/src/serve/server/telemetry.ts |
Registers the owner turn-index route in the legacy catalog; regex matcher for the workspace-qualified route |
packages/core/src/index.ts |
Exports the snapshot codec functions, anchor error, and new types |
packages/core/src/services/session-transcript-reader.test.ts |
Projection, snapshot-stability, boundary, and tamper coverage — this delta adds cold-cache rebuild, cross-session binding, and format-preserving forgery |
packages/core/src/services/session-transcript-reader.ts |
Core change: navigation hints in the index, readTurnIndexPage, signed snapshots, anchored readPage with boundary expansion |
packages/core/src/utils/transcript-records.ts |
Receives stripGeneratedAttachmentTokens, moved from acp-bridge for sharing |
packages/sdk-typescript/src/daemon/DaemonClient.ts |
Client methods for both turn-index routes; query builders for anchored transcript options |
packages/sdk-typescript/src/daemon/DaemonSessionClient.ts |
Per-session getTurnIndexPage convenience bound to the client identity |
packages/sdk-typescript/src/daemon/index.ts |
Exports the new SDK types |
packages/sdk-typescript/src/daemon/types.ts |
SDK types for turn-index pages and anchored transcript options |
packages/sdk-typescript/src/index.ts |
Re-exports the new types from the package root |
packages/sdk-typescript/test/unit/DaemonClient.test.ts |
SDK REST-path tests, including direct-REST enforcement for workspace paging |
packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts |
Session-client identity binding test |
packages/sdk-typescript/test/unit/daemon-public-surface.test.ts |
Asserts the new types are part of the public SDK surface |
Testing evidence (the PR's own CI, read via API)
Per the triage rules I did not run any of this PR's code; below is the PR's own CI on 1125c095, fetched once via the API (no polling). Everything is settled — no pending lanes. The Serve A/B job re-ran at exactly this head and its updated comment shows the daemon responses identical except capabilities.features[] gaining session_turn_navigation — direct evidence the contract is additive at this commit. The Qwen Triage Finalize job keeps the table below current.
| Check | Conclusion |
|---|---|
Dependency CVE audit |
🚫 failure |
web-shell E2E Smoke (ubuntu-latest, Node 22.x) |
🚫 cancelled |
Classify PR |
✅ success |
Desktop Shell (ubuntu-22.04) |
✅ success |
Desktop Shell (windows-2022) |
✅ success |
Integration Tests (no-AK, No Sandbox) |
✅ success |
Live Host (macos-latest) |
✅ success |
OpenTUI no-flicker gate |
✅ success |
Post Coverage Comment (ubuntu-latest, 22.x) |
✅ success |
Real daemon E2E / Java 11 |
✅ success |
Secret scan (TruffleHog) |
✅ success |
Serve A/B (ubuntu-latest, Node 22.x) |
✅ success |
Test (ubuntu-latest, Node 22.x) |
✅ success |
TUI parity snapshots (ink vs opentui) |
✅ success |
macos-latest / Java 21 |
✅ success |
ubuntu-latest / Java 11 |
✅ success |
ubuntu-latest / Java 17 |
✅ success |
ubuntu-latest / Java 21 |
✅ success |
windows-latest / Java 21 |
✅ success |
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。
Classification of the two non-green lanes — from the diff and check identity, not from log claims:
- Dependency CVE audit (failure) — stale branch, not PR-caused. The audit fails on
fast-uri 3.0.0–3.1.5(high, four advisories) plus moderate/lowqs/uuidin the existing root dependency graph. This PR touches no manifest or lockfile (verified in the file list). Main merged chore(deps): bump fast-uri to 3.1.7 to clear the high-severity audit gate #10862 ("bump fast-uri to 3.1.7 to clear the high-severity audit gate") at 01:16 UTC — 49 minutes after this head's audit ran at 00:27 — and every Security Checks run on main since has passed. Updating the branch with main clears this lane. - web-shell E2E Smoke (cancelled) — inconclusive; the failure pattern looks environmental. On the retry attempt (self-hosted runner), all six first-attempt specs failed into retry — including specs unrelated to this PR's surface (GitHub PRs tab, collapsed-groups persistence, split persistence) — and the job was cancelled mid-retry; this PR changes no web-shell client code. Not verified: what cancelled the run. The lanes that actually drive this PR's daemon changes — Serve A/B and Real daemon E2E — are green at this head.
The author's babysit comment reports local mutation witnesses (each new test red with its guard removed) and a 135/135 reader suite — that is the author's claim and is not used as evidence here; the structural pinning argument above stands without it.
Sandboxed verification would settle what static review cannot: @qwen-code /verify — that turn-index and anchored-transcript behavior on a live daemon matches the contract (frozen snapshots after append, exact anchored pages, explicit rejection of tampered or foreign snapshots). A verify run is already in flight at this head (run 33718539824); its report lands in this thread when it completes. The author has write access, so no sponsored-run caveats apply.
Real-scenario (tmux) testing: N/A — this is a protocol-only delivery; the Web Shell navigation UI is Phase 3 and explicitly out of scope, so nothing user-visible changes in this PR.
中文说明
代码审查（在 1125c095 上重跑）
自上一轮审查的 head 97aafaaa 以来，增量为三个纯测试提交——生产代码零改动，经 compare API 确认——因此前几轮对生产协议的全部核对结论原样有效（签名快照绑定 sessionId、文件身份、字节长度与叶 UUID；锚点语义在 reader、REST、ACP 三层一致强制；有界边界扩展；投影剥离；路由卫生且不回退主运行时）。本轮审查的是增量本身，逐条对照其钉住的代码：
- 格式保持的快照伪造：解码 base64url JSON、递增
snapshotSize、保留原mac重编码。已对照编解码器核实：令牌为{...payload, mac}，mac为 payload 序列化串的 HMAC-SHA256，snapshotSize是受 MAC 约束的负载字段，且解码先做形状检查再做timingSafeEqual比对——该伪造格式完好，恰好命中 MAC 失配路径，与既有的形状损坏篡改用例互补。 - 冷缓存冻结前缀稳定性：追加稳定性测试现在在追加与冻结读取之间调用
resetSessionTranscriptIndexCacheForTest()——清空内存索引缓存（强制从磁盘重建），而持久化的工作区 HMAC 密钥保留——证明先前铸造的快照在真正的冷重建下仍解析到冻结前缀。 - 跨会话快照绑定：为某会话铸造的快照在另一会话的
readTurnIndexPage与锚点readPage上均被拒绝——会话绑定是强制的，不只是声明。 - bridge turn-index 超时：假定时器测试钉住挂起的
sessionTurnIndexext 调用以BridgeTimeoutError拒绝；转发断言由单字段抽查升级为完整形状toEqual加无意外副作用调用。 - 冻结页不得 flush：最新页+冻结页请求对中，transcript flush 恰好发生一次，冻结请求原样携带
snapshot/start/limit——钉住冻结前缀赖以成立的语义。
增量无阻断项。每个新测试在结构上钉住其守卫（缺失超时/会话绑定/缓存清空都会挂起或失败），因此钉住效力不依赖作者自述的变异见证运行（下文仅作作者声明记录，不作为证据）。前几轮的两项非阻断观察仍然成立：owner-resolved transcript 路由现同样套用响应字节上限；buildIndex fragment 聚合仍是最微妙处——现已更充分钉住。
测试证据（本 PR 自身 CI，经 API 读取）
按规则未运行本 PR 任何代码；以上为 1125c095 上一次性读取的 CI 信号（不轮询），各通道已全部落定。Serve A/B 已在本 head 上重跑，其评论显示 daemon 响应仅新增 session_turn_navigation 能力——该提交上协议增量性的直接证据。两条非绿通道的定性基于 diff 与检查身份而非日志声明：CVE 审计失败为分支过旧（main 在本 head 审计运行 49 分钟后合入 #10862 的 fast-uri 升级，其后 main 全部通过；本 PR 未触及任何 manifest/lockfile），更新分支即可消除；web-shell E2E Smoke 取消为不确定项，但失败形态指向环境因素（重试轮中全部六个首试失败含多个与本 PR 无关的 spec，且本 PR 未改任何 web-shell 客户端代码；未能核实取消原因）。真正驱动本 PR daemon 改动的 Serve A/B 与 Real daemon E2E 在本 head 上为绿。作者跟帖中的变异见证与 135/135 本地套件为作者自述，不作为证据使用。沙箱验证：@qwen-code /verify 的运行已在本 head 上进行中，完成后报告将发布于本线程；作者有写权限，无需 sponsored-run 注意事项。真实场景（tmux）测试：N/A——纯协议交付，Web Shell 导航 UI 属 Phase 3、明确范围外，本 PR 无任何用户可见变化。
— Qwen Code · qwen3.8-max
Reviewed at 1125c095b6b4a23263584768cacd87e0824fa80d · re-run with @qwen-code /triage
Collaborator
|
Confidence: 4/5 — code-wise this is ready: the production contract has survived three review rounds without moving a line since Stepping back honestly over the whole picture:
Not approving at this head, and not requesting changes either — there are no code defects to fix. After a branch update and a green re-run, 中文说明置信度：4/5 —— 就代码而言已经就绪：生产协议经三轮评审，自 整体判断：
本 head 上不批准、也不请求修改——没有需要修复的代码缺陷。更新分支并重跑转绿后，再次 — Qwen Code · qwen3.8-max Reviewed at |
Collaborator
|
✅ Superseded — the re-run at Original deferral (re-run at 97aafaa, 2026-09-02)⏸️ Deferring to @qqqys — the re-run review at
Assigned for visibility. If the Test lane lands green on （中文原文从略——该轮完整中文说明见本评论编辑历史。） — Qwen Code · qwen3.8-max |
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
qqqys
previously requested changes
Sep 2, 2026
qqqys
left a comment
qqqys
left a comment
Collaborator
There was a problem hiding this comment.
Requesting changes for one merge-blocking issue: the Test lane is deterministically red at head f1a8523d.
Critical — route-count drift guard not updated for the new route (inline at packages/cli/src/serve/server/telemetry-catalog.test.ts:101)
The PR registers GET /session/:id/turn-index and adds it to legacySessionTelemetryRoutes, updating the count assertions in telemetry.test.ts (62→63). But the independent both-directions drift guard in telemetry-catalog.test.ts still hard-codes the old count, so it fails on every run at this head:
FAIL src/serve/server/telemetry-catalog.test.ts > legacy session telemetry route drift guard > matches the explicit Express route registrations in both directions
AssertionError: expected [ 'DELETE /session/:id', …(62) ] to have a length of 62 but got 63
- Trigger: any Test-lane run at head. Confirmed in CI (job
100071520122— the single failing test out of 26,909) and reproduced locally atf1a8523d(cd packages/cli && npx vitest run src/serve/server/telemetry-catalog.test.ts). - Impact: Test (ubuntu) is red, so the PR cannot merge as-is.
- Fix direction: bump the hard-coded count in
telemetry-catalog.test.ts:101from 62 to 63 (thetoEqual(catalog)half already follows the catalog, so only the literal needs to move).
Everything else I checked reviewed clean, and CI agrees apart from that one guard: the snapshot token is HMAC-signed, versioned, and workspace-bound with tamper/staleness handled distinctly (400 vs 409); anchor mutual-exclusion and the snapshot requirement are enforced at the route, ACP, and reader layers; anchored reads expand only to a safe replay boundary under the byte budget; the preview projection excludes thoughts, tool calls/arguments/results, control context, and generated attachment tokens (pinned by the new core tests, 129/129 green in CI); ownership/archive/runtime-generation semantics mirror the existing transcript routes with no primary-runtime fallback; SDK methods force direct REST with correct URL encoding. The turn-navigation suites in core, cli, acp-bridge, and sdk-typescript all pass in CI.
阻塞合并的问题（一个）：新增的 GET /session/:id/turn-index 路由让 legacy session 路由数从 62 变为 63，telemetry.ts 目录和 telemetry.test.ts 的计数都已更新，但 telemetry-catalog.test.ts:101 的双向漂移守卫仍硬编码 toHaveLength(62)，导致 Test (ubuntu) 在 head f1a8523d 上确定性失败（CI job 100071520122，26,909 个测试中唯一失败项；本地同样复现）。修复方向：把该断言的字面值改为 63 即可。其余部分（快照签名/防篡改、锚点互斥校验、预览投影的隐私剥离、路由所有权语义、SDK 直连 REST）复查均无阻塞问题。
Collaborator
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
features[] |
— | "session_turn_navigation" |
— Qwen Code · serve A/B
qwen-code-ci-bot
previously requested changes
Sep 2, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
— qwen3.8-max via Qwen Code /review (v0.22.3)
Collaborator
Author
|
Manual babysit follow-up for 97aafaa:
Verification:
Remote CI is now rerunning on this commit. |
Collaborator
Author
|
@qwen-code /triage |
doudouOUC
enabled auto-merge
September 2, 2026 16:18
Collaborator
|
Sandboxed verification: ❌ not passed — findings reported (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: 47 passed · 0 failed · 47 total Flakiness gate: 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：47 通过 · 0 失败 · 47 总计 抖动门： Verification reportPR 10751 Deep Verification —
|
| # | Cell (oracle) | Base (HEAD^1) |
Head (PR) |
|---|---|---|---|
| 1 | /capabilities advertises session_turn_navigation |
absent (127 features) | present |
| 2 | GET /session/:id/turn-index |
404 | 200, totalTurns=4, ids [u1, ur1, uc1, u3], kinds [prompt, realtime, scheduled, prompt] |
| 3 | Projection: label/detail bounded public text only | n/a | label Visible prompt (token + hookContext stripped), detail Public answer (thought/tool-call excluded); leak scan over full body: 0 of 11 forbidden strings |
| 4 | Snapshot freeze across append | n/a | frozen totalTurns=4 vs fresh 5; appended u4 rejected as anchor under frozen snapshot (400) but resolves under fresh (200) |
| 5 | Anchored read atRecordId + snapshot |
params ignored — byte-identical page to the plain read, no targetRecordId/hasOlder |
200, targetRecordId=uc1, hasOlder=true, non-empty page |
| 6 | Fail-closed matrix (tamper ×2, cross-session, no-snapshot anchor, bare snapshot, assistant anchor, unknown anchor, start w/o snapshot, missing session, cross-session on qualified route, deleted file w/ snapshot) |
n/a | 11/11 explicit 400/404/409 with correct codes, no fallback |
| 7 | Workspace-qualified routes (/workspaces/:ws/...) |
404 | 200 index + anchored read (target=uc1) |
| 8 | Both-arm positive control: plain transcript reads of sessions A & B | 200, events present | 200, events present |
Score: base 7/7, head 38/38 (the one intermediate head FAIL was a wrong expectation in my harness — corrected: a session-A snapshot presented to a nonexistent session is rejected by the session-binding guard before file lookup, which is the right fail-closed order; the true 409 path was then proven by deleting the file after minting the snapshot).
Findings
Both findings are Suggestion-level completeness gaps: the shipped behavior is correct (proven above and by the probes below); what is missing is a test that pins it. Neither blocks merge, but both are cheap to add and each protects a guard a future refactor could silently remove.
F1 — Snapshot MAC verification is load-bearing but unpinned by any test
The only tampering tests (core unit 'supports an empty frozen snapshot and rejects tampering' and the integration test) flip the first base64url character of the snapshot. That corrupts the decoded JSON and is rejected by structural validation before the MAC is ever consulted. Proof: with the MAC check disabled (mutant M1: false && !hasValidCursorMacWithKey(...) in decodeSnapshotState), the entire core reader suite — including all 14 turn-navigation tests — still passed 134/134. The MAC is nonetheless the only guard against a format-preserving forgery: a scratch probe that decodes a genuine snapshot, edits snapshotSize in the JSON, keeps the original mac, and re-encodes is rejected with InvalidSessionTranscriptCursorError on clean source and passes decode under M1 (proceeding to file lookup — the E2E payload-edit cell's 400 becomes unreachable, its request flowing on to the size check instead). The sharpest real-world shape is re-binding a snapshot's sessionId to another existing session — the snapshot.sessionId !== sessionId check cannot catch a forged snapshot edited to match, only the MAC can.
Reproduce:
# in packages/core/src/services/session-transcript-reader.ts, decodeSnapshotState:
# prefix the MAC condition with `false &&`, then:
cd packages/core && npx vitest run src/services/session-transcript-reader.test.ts # still 134/134 greenSuggested test (measured: green on clean source, red under M1)
const genuine = encodeSessionTranscriptSnapshot(state, workspaceDir);
const decoded = JSON.parse(Buffer.from(genuine, 'base64url').toString('utf8'));
decoded['snapshotSize'] = decoded['snapshotSize'] + 1; // keep original mac
const forged = Buffer.from(JSON.stringify(decoded), 'utf8').toString('base64url');
await expect(reader.readTurnIndexPage(sessionId, { snapshot: forged }))
.rejects.toBeInstanceOf(InvalidSessionTranscriptCursorError);F2 — The frozen-prefix byte cap is load-bearing for cold-cache reads but unpinned
buildIndex reads only [0, snapshotSize-1] (forEachLineInSnapshot's createReadStream({ end: snapshotSize - 1 })), which is what keeps a frozen snapshot stable across appends. Mutant M3 (remove end) survived the whole turn-navigation suite (14/14 green) because the in-process index cache is keyed by filePath:dev:ino:snapshotSize:lastUpdated — the frozen read reuses the pre-append cache entry and never re-reads the file. The cap is the only protection on a cold cache (daemon restart, cache eviction), which is exactly the scenario signed durable snapshots exist for. Proof: with resetSessionTranscriptIndexCacheForTest() between append and frozen read, M3 breaks the freeze (frozen totalTurns grows to include the appended turn) while clean source stays frozen.
Reproduce:
# remove `end: snapshotSize - 1,` from the createReadStream call in
# forEachLineInSnapshot (packages/core/src/services/session-transcript-reader.ts)
cd packages/core && npx vitest run src/services/session-transcript-reader.test.ts -t 'turn navigation' # still 14/14 greenSuggested test (measured: green on clean source, red under M3)
Mint a snapshot, append a new prompt record, call resetSessionTranscriptIndexCacheForTest(), then read the frozen page and assert totalTurns is unchanged.
Mutation matrix summary
| Mutant | Guard | PR suite result | Independent probe | Classification |
|---|---|---|---|---|
| M1 MAC check disabled | snapshot tamper rejection | survives 134/134 | killed (forgery probe + E2E payload-edit cell) | coverage gap — test to write (F1) |
M2 notification exclusion removed |
invisible-kind filter / projection | killed — AssertionError: expected 5 to be 4 in 'builds stable sparse pages…' (intended values) |
— | pinned (positive control: the harness can make the suite red, in the mutated file) |
| M3 freeze byte cap removed | frozen-prefix stability | survives 14/14 (cache masks it) | killed (cold-cache probe) | coverage gap — test to write (F2) |
Witness: evidence/03-mutation-m2-killed.png, evidence/05-mutation-matrix.png. All mutants reverted; source verified byte-identical to the merge commit afterwards (diff clean, git status clean).
Gates (targeted, exact counts)
Witness: evidence/04-targeted-gates.png.
| Suite | Result |
|---|---|
core session-transcript-reader.test.ts |
134/134 |
core transcript-records.test.ts (new stripGeneratedAttachmentTokens) |
20/20 |
acp-bridge bridge.test.ts |
894/894 |
cli acpAgent.test.ts + multi-workspace-sessions.test.ts + server/error-response.test.ts |
798/798 |
cli server.test.ts -t turn |
136/136 (1041 filtered out) |
sdk DaemonClient / DaemonSessionClient / daemon-public-surface |
507/507 |
integration qwen-serve-routes.test.ts (real bundled daemon, incl. the PR's new owner-path test + capabilities baseline) |
38/38 |
Not covered
- Reviewer Test Plan step 5 (workspace-qualified reads through a secondary runtime, ownership unresolved/unavailable, fail-closed transcript semantics there): the qualified routes were verified against the primary bound workspace only; registering a secondary runtime and driving ownership ambiguity was out of budget.
- Per-commit attribution: the checkout is depth 2; only the test commit
97aafaaais locally reachable (the feature commitf1a8523dis past the shallow boundary —git rev-list HEAD^1..HEAD^2returns 1 vs the snapshot's 2 commits). Verified the aggregateHEAD^1..HEADdiff instead. - Repo-wide
lint/typecheck(author claims both green) — not re-run; the head bundle built by CI was exercised end-to-end instead. - Transcripts near the 256 MiB indexing ceiling; live sessions with an active prompt during anchored reads; archived-session turn-index reads; Windows/macOS platform behavior.
- Phase 2 browser data layer and Phase 3 virtualized rail (explicitly out of scope for this PR).
- The SDK was verified through its unit gates and the wire contract (raw HTTP), not through
DaemonClientcalls in my harness.
Methodology
Container node:22-bookworm, repo at merge commit fcc30afb. Head side: CI-built dist + bundle. Base side: git worktree add tmp/base-tree HEAD^1, rebuilt the four touched workspaces (tsc emits despite pre-existing type-resolution noise for telemetry typings; dist verified complete and feature-absent by grep), symlinked package-level node_modules from head (verified @qwen-code-free) plus a base-tree node_modules/@qwen-code layer pointing into base packages; realpath of @qwen-code/qwen-code-core from the base CLI confirmed inside the base tree; bundled via npm run bundle after regenerating web-templates. The harness ab-e2e.mjs (in this artifact dir) spawns each daemon with an isolated HOME/QWEN_HOME/workspace, writes the fixture JSONL via each tree's own Storage.getProjectDir(), and asserts raw HTTP responses; per-arm text logs are base-arm.txt/head-arm.txt. Mutation probes were applied to the head source, run via vitest, and reverted (byte-identical restore verified). Raw gate logs: /tmp/gate-{core-final,bridge,cli-focused,cli-server-turn,sdk,integration}.log.
Flakiness gate log
integration test, out of gate scope: integration-tests/cli/qwen-serve-routes.test.ts
rounds=5 files=11 skipped=1
file packages/acp-bridge/src/bridge.test.ts: (cd packages/acp-bridge) npx --no-install vitest run ./src/bridge.test.ts
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/serve/multi-workspace-sessions.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/multi-workspace-sessions.test.ts
file packages/cli/src/serve/server.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server.test.ts
file packages/cli/src/serve/server/error-response.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/error-response.test.ts
file packages/cli/src/serve/server/telemetry-catalog.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/telemetry-catalog.test.ts
file packages/cli/src/serve/server/telemetry.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/telemetry.test.ts
file packages/core/src/services/session-transcript-reader.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/session-transcript-reader.test.ts
file packages/sdk-typescript/test/unit/DaemonClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonClient.test.ts
file packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/DaemonSessionClient.test.ts
file packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: (cd packages/sdk-typescript) npx --no-install vitest run ./test/unit/daemon-public-surface.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/acp-bridge/src/bridge.test.ts: PP
packages/cli/src/acp-integration/acpAgent.test.ts: PP
packages/cli/src/serve/multi-workspace-sessions.test.ts: PP
packages/cli/src/serve/server.test.ts: PP
packages/cli/src/serve/server/error-response.test.ts: PP
packages/cli/src/serve/server/telemetry-catalog.test.ts: PP
packages/cli/src/serve/server/telemetry.test.ts: P
packages/core/src/services/session-transcript-reader.test.ts: P
packages/sdk-typescript/test/unit/DaemonClient.test.ts: P
packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P
packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P
verdict: timeout
summary: the 15-minute budget elapsed before two full rounds completed (1 done) — no flakiness signal either way
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/multi-workspace-sessions.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/telemetry-catalog.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/server/telemetry.test.ts: P (exit 0)
round 1 · packages/core/src/services/session-transcript-reader.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonClient.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/DaemonSessionClient.test.ts: P (exit 0)
round 1 · packages/sdk-typescript/test/unit/daemon-public-surface.test.ts: P (exit 0)
round 2 · packages/acp-bridge/src/bridge.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/multi-workspace-sessions.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/server/telemetry-catalog.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Collaborator
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- beforeRecordId+snapshot backward-flow coverage gap (packages/core/src/services/session-transcript-reader.ts:3352) — already deferred as D4-1 in round 4 (review 5106772767)
- integration-tests-outside-workspace collection gap (integration-tests/cli/qwen-serve-routes.test.ts:564) — already reported as R2-1 (comment 3917483691)
- turn-index bridge channel-closed test gap (packages/acp-bridge/src/bridge.ts:7371) — already deferred as D4-4 in round 4 (review 5106772767)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/session.ts:4623 — [probe] snapshot-bound archived reads: session_archived vs transcript_snapshot_unavailable across routespackages/core/src/services/session-transcript-reader.ts:2183 — [probe] lost turn_result lets the next settled turn's promptId attach to the wrong rail entrypackages/core/src/services/session-transcript-reader.ts:2615 — [probe] generic-label fallback for unreadable indexed turns ships untestedpackages/core/src/services/session-transcript-reader.ts:2033 — [probe] fragment-merge navigation-hint establishment ships untestedpackages/core/src/services/session-transcript-reader.ts:3470 — [probe] anchored reads' goal-state seeding is unpinnedpackages/cli/src/serve/routes/session.ts:4878 — [probe] workspace turn-index ENOENT failure mapping ships without boundary testspackages/core/src/services/session-transcript-reader.test.ts:4256 — [probe] anchored reads at realtime turns are untested; the pass-through is unpinned for this pathpackages/core/src/services/session-transcript-reader.test.ts:4429 — [probe] anchored expansion's record-budget floor is never exerciseddocs/design/web-shell/web-shell-global-turn-navigation.md:540 — [review] doc promises six telemetry recordings; only route latency exists and no phase owns the rest
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 9 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
Collaborator
Maintainer verification — round 2 (delta) at
|
| Round-1 item | State at 872e136 |
|---|---|
Finding 1 — merging main turned three pinned-count gates red (65 routes / 152 tags after the union) |
Closed. 872e136 re-derives them rather than guessing: 65 catalog entries with a 63/2 handler_resolved/pre_resolved split, GET /session/:id/turn-index classified explicitly, 152 registry tags (the /capabilities envelope advertises 135 of them in this configuration), docs index at 152. All three gates green. A fresh trial merge with today's origin/main (56f75adf, one commit past the PR's sync point) applies cleanly and leaves them green. |
Finding 2 — kind: "scheduled" has no producer inside qwen serve |
Still open, still not this PR's defect. SendMessageType.Cron is requested only by nonInteractiveCli.ts and the TUI's use-llm-stream.ts; Session.ts #executeCronPromptInner sends the cron prompt without it, so recordCronPrompt() never runs in the daemon. The reader's rule is correct and the seeded fixture exercises it — the gap is in cron dispatch. |
| R1-5 / D4-3 pinning suggestion (a fixture where the anchored expansion spans two records) | Not taken — the delta was the telemetry counts only. Still non-blocking, still worth doing in Phase 2. |
| S1 key-domain split, N2 64 KiB guard | Unchanged; not re-measured this round. |
What this round adds
Round 1 listed "a rebuilt base arm (the 'before')" under Not covered. This round builds it, plus a source-mutation negative control and a set of snapshot-invalidation cases round 1 did not exercise.
1. Rebuilt base arm — a real "before" on the same fixture
Both arms are built from source in their own worktrees (node esbuild.config.js → dist/cli.js) and driven against the same isolated QWEN_HOME, the same seeded 19-record transcript and the same requests.
base 60161cb (the main commit this PR merged) |
PR head 872e136 |
|
|---|---|---|
session_turn_navigation in /capabilities |
absent (134 advertised features) | advertised (135) |
GET /session/:id/turn-index |
404 | 200 |
GET /workspaces/:ws/session/:id/turn-index |
404 | 200 |
transcript?atRecordId=…&snapshot=FORGED |
200, byte-identical to the plain sequential page — both parameters silently ignored | 400 invalid_transcript_cursor |
| anchored read with a real snapshot | n/a | 200 with targetRecordId + hasOlder |
The seeded session is 19 persisted records covering every classifier branch; the index resolves to exactly 7 navigable turns, with the excluded kinds absent and the previews bounded:
goal_runtime,notification,mid_turn_user_message, a hook-context-only prompt and a whitespace-only prompt produce no entries;- the
cronturn's label comes fromsystemPayload.displayText(Nightly digest job), not the model-facing text; - the realtime answer attaches to the realtime turn, not to the ordinary prompt that precedes it;
- the
turn_resultpromptIdbinds to its own prompt turn and to no other; - an attachment-only turn and a turn whose only visible text is a generated
@attachment:///token both keep their ordinals with the genericPromptlabel; - a 200-code-point non-BMP prompt is capped at exactly 160 code points with no split surrogate.
A single scan of the serialized page for SECRET_THOUGHT…, the excluded-record texts, the hook context, the tool name, the tool arguments, the tool output and the attachment token finds none of them — and those strings are all verifiably present in the transcript file on disk, so the check is not vacuous.
2. Negative control — which assertions actually discriminate
Four mutations were applied to the PR source, core rebuilt, the daemon restarted, and the same suite replayed.
| Mutation | Broken |
|---|---|
drop the goal_runtime/notification/mid_turn_user_message exclusions in navigationKindForRecord |
14 / 40 — including the privacy assertion: the excluded records' text is republished in the public preview |
skip the snapshot HMAC comparison in decodeSnapshotState |
2 / 41 |
read the live file size instead of the frozen snapshotSize in readTurnIndexPage |
1 / 3 — the frozen page starts showing the appended turn |
let atRecordId resolve to any chain position instead of a navigation turn |
3 / 41 — assistant, excluded-record and unknown anchors all start returning 200 |
Worth reading carefully: with the HMAC gone, the forged snapshots are still refused — they just come back 409 transcript_snapshot_unavailable instead of 400. File identity, frozen size and frozen-leaf reproduction form an independent second gate, so the signature is defence in depth here rather than the only lock. That is a good property, not a defect.
3. Snapshot invalidation by file identity (new cases)
| Case | Result |
|---|---|
| transcript replaced with identical content but a new inode | 409 transcript_snapshot_unavailable |
| transcript truncated below the frozen size | 409 |
| transcript rewritten in place, same inode and same byte length, last record's uuid changed so the frozen prefix reproduces a different leaf | 409 |
| append after the snapshot, then a cold daemon (killed and restarted, index rebuilt from disk) | old snapshot still reports the frozen totalTurns, fresh read sees N+1, HMAC still verifies under the same workspace key |
4. Other wire-level checks round 1 did not list
beforeRecordId + a turn-index snapshot is accepted and tiles strictly older content next to the anchored page (no overlap on the anchor); limit=0 rejected and limit=500 accepted; start === totalTurns returns an empty page while start > totalTurns is 400; a session with conversation records but no navigable turn reports totalTurns: 0 and leaks nothing; an empty or record-less transcript answers exactly like the existing /transcript route (404 session_not_found), so the new route adds no new surface there.
5. Worst-case page size
520-turn session, every prompt >160 code points and every reply >240 code points, all non-BMP (4-byte) characters:
| Request | Body | Per turn | Caps held |
|---|---|---|---|
turn-index?limit=500 |
850.1 KiB | 1741 B | label 160 cp, detail 240 cp |
turn-index?limit=100 |
170.5 KiB | 1746 B | same |
| anchored transcript read at ordinal 250 | 13.7 KiB | — | targetRecordId correct, hasOlder: true |
The route serializes through serializeWorkspaceTranscriptResponse with a 32 MiB ceiling, so even this adversarial shape sits ~38× under it.
Full matrix
75/75 scripted assertions, all against a real daemon: protocol matrix on both route families (41), snapshot freeze across a live append (3), TypeScript SDK surface (6), snapshot invalidation and edge cases (11), cold restart (3), a real session driven end-to-end by a scripted model (6), and a real tool call/result pair (5).
Two of those are worth calling out because they use records the product itself wrote rather than a fixture: a live prompt shows up in the turn index without an explicit flush, carries the model answer as its bounded detail and is reconciled to its runtime promptId; and for a real tool turn, walking forward from the anchored page yields every tool call together with its update, with the tool name, arguments and output absent from the index preview.
Suites
At 872e136: core session-transcript-reader 135, cli server 1178, cli acpAgent 611, cli telemetry/telemetry-catalog/error-response/multi-workspace 263, acp-bridge bridge 895, sdk 509 — all green.
On the tree trial-merged with today's origin/main: the three previously-red gates 71, core session-transcript-reader 135, cli server + acpAgent + multi-workspace + error-response 1983 — all green.
Environment
| Arms | 872e136 and 60161cb in separate worktrees, each built to dist/cli.js via node esbuild.config.js |
| Host | Linux 6.12, Node v22.22.2 (the PR body marks Linux |
| Daemon | qwen serve --port 0 --token … --workspace wsA --workspace wsB, isolated QWEN_HOME, loopback only |
| Model | a scripted OpenAI-compatible server on loopback; no credentials, no network, no model spend |
| Oracle | raw HTTP status / code / JSON bodies, plus the transcript file on disk to prove the leak scans are non-vacuous |
Not covered
Windows and macOS; the real voice producer of realtime_message records; ACP over stdio directly (the bridge path was exercised through the owner route); Web Shell UI, which is Phase 2/3 by design. Round 1's S1 and N2 items were not re-measured — they were non-blocking there and no production line has moved since.
中文版本
维护者验证 — 第 2 轮（增量）基于 872e136
第 1 轮验证跑在 86c11db3（报告）。本轮在当前 head 重新测量，只汇报变化的部分与上一轮未覆盖的部分。
结论：可以合并。 第 1 轮唯一的阻塞项——同步 main 后三个计数型门禁变红——在本 head 已关闭，且对今天的 main tip 依然关闭。第 1 轮的 Finding 2（kind: "scheduled" 在 daemon 内没有生产者）保持不变，属于 #10750 的后续项而非本 PR 的阻塞项。本轮针对由该 head 构建的真实 qwen serve daemon 新增 75/75 条脚本断言全部通过，没有新的阻塞问题。
相对第 1 轮的增量
整个增量只有两个 commit：一次 origin/main 合并，以及一个只改三处固定计数的 test-only commit。生产代码一行未动。
| 第 1 轮条目 | 在 872e136 上的状态 |
|---|---|
Finding 1 — 合并 main 后三个计数型门禁变红（并集后为 65 路由 / 152 tag） |
已关闭。 872e136 是重新推导而非拍脑袋改数字：目录 65 条、handler_resolved/pre_resolved 拆分为 63/2、GET /session/:id/turn-index 被显式分类、registry 152 个 tag（该配置下 /capabilities 对外声明其中 135 个）、docs 索引同为 152。三个门禁全绿。用今天的 origin/main（56f75adf，比 PR 同步点新 1 个 commit）重新试合并，干净且门禁仍绿。 |
Finding 2 — kind: "scheduled" 在 qwen serve 内没有生产者 |
仍然存在，仍然不是本 PR 的缺陷。 只有 nonInteractiveCli.ts 与 TUI 的 use-llm-stream.ts 会传 SendMessageType.Cron；Session.ts #executeCronPromptInner 发送 cron prompt 时没带该类型，因此 daemon 内 recordCronPrompt() 永远不会执行。reader 的规则本身是对的，seed 夹具也验证了它——缺口在 cron 派发侧。 |
| R1-5 / D4-3 的钉测建议（构造锚点扩展跨两条记录的夹具） | 未采纳 —— 增量只改了 telemetry 计数。仍非阻塞，仍建议在 Phase 2 补上。 |
| S1 签名密钥域拆分、N2 64 KiB guard | 无变化；本轮未重测。 |
本轮新增的内容
第 1 轮把 "重建的 base 臂（before）" 列在未覆盖里。本轮补上了它，另外加了源码变异的反向对照，以及第 1 轮没有覆盖的一组快照失效场景。
1. 重建 base 臂 —— 同一夹具上的真实 "before"
两个臂都在各自 worktree 中从源码构建（node esbuild.config.js → dist/cli.js），跑在同一个隔离 QWEN_HOME、同一份 seed 的 19 条记录 transcript、同一批请求上。
base 60161cb（本 PR 合入的那个 main commit） |
PR head 872e136 |
|
|---|---|---|
/capabilities 中的 session_turn_navigation |
不存在（对外声明 134 个 feature） | 已声明（135 个） |
GET /session/:id/turn-index |
404 | 200 |
GET /workspaces/:ws/session/:id/turn-index |
404 | 200 |
transcript?atRecordId=…&snapshot=FORGED |
200，与普通顺序页逐字节相同 —— 两个参数被静默忽略 | 400 invalid_transcript_cursor |
| 带真实 snapshot 的锚定读取 | 不适用 | 200，带 targetRecordId 与 hasOlder |
seed 会话共 19 条持久化记录，覆盖分类器的每个分支；索引结果恰好是 7 个可导航 turn，被排除的类型确实不出现，预览也是有界的：
goal_runtime、notification、mid_turn_user_message、仅含 hook context 的 prompt、以及只有空白字符的 prompt 都不产生条目；cronturn 的 label 取自systemPayload.displayText（Nightly digest job），而非面向模型的文本；- realtime 的回答归属到 realtime turn，而不是它前面那个普通 prompt；
turn_result的promptId只绑定到自己的 prompt turn，不会串到别的 turn；- 仅附件的 turn、以及可见文本只有生成式
@attachment:///token 的 turn，都保留各自 ordinal 并使用通用Promptlabel； - 200 个码点的非 BMP prompt 被精确截到 160 个码点，且没有半个代理对。
对序列化后的页面做一次扫描，检索 SECRET_THOUGHT…、被排除记录的文本、hook context、工具名、工具参数、工具输出与附件 token，全部命中为 0 —— 而这些字符串确实都存在于磁盘上的 transcript 文件中，所以这个检查不是空洞的。
2. 反向对照 —— 哪些断言真的有判别力
对 PR 源码施加四个变异，重建 core，重启 daemon，重放同一套探针。
| 变异 | 被打破的断言 |
|---|---|
删掉 navigationKindForRecord 里对 goal_runtime/notification/mid_turn_user_message 的排除 |
14 / 40 —— 包括隐私断言：被排除记录的文本会重新出现在公开预览里 |
跳过 decodeSnapshotState 中的 snapshot HMAC 比对 |
2 / 41 |
readTurnIndexPage 读实时文件大小而非冻结的 snapshotSize |
1 / 3 —— 冻结页面开始显示新追加的 turn |
让 atRecordId 可以解析到链上任意位置，而不限于导航 turn |
3 / 41 —— assistant 锚点、被排除记录锚点、未知锚点全部开始返回 200 |
有一点值得留意：去掉 HMAC 之后，伪造的 snapshot 仍然会被拒绝，只是返回 409 transcript_snapshot_unavailable 而不是 400。文件身份、冻结大小、冻结叶子的可复现性构成了一道独立的第二道闸门，所以签名在这里是纵深防御，而不是唯一的锁。这是好性质，不是缺陷。
3. 基于文件身份的快照失效（新增场景）
| 场景 | 结果 |
|---|---|
| transcript 被同内容替换但 inode 变了 | 409 transcript_snapshot_unavailable |
| transcript 被截断到小于冻结大小 | 409 |
| transcript 原地重写，inode 与字节长度都不变，只把最后一条记录的 uuid 改掉，使冻结前缀复现出不同的叶子 | 409 |
| 先追加，再冷启动 daemon（杀掉重启，索引从磁盘重建） | 旧 snapshot 仍报告冻结的 totalTurns，新请求看到 N+1，HMAC 在同一 workspace 密钥下仍然校验通过 |
4. 第 1 轮未列出的其他线级检查
beforeRecordId 与 turn-index snapshot 可以同时使用，并与锚定页严格相邻地拼接更早的内容（锚点本身不重复）；limit=0 被拒、limit=500 被接受；start === totalTurns 返回空页，start > totalTurns 返回 400；有会话记录但没有可导航 turn 的会话返回 totalTurns: 0 且不泄漏任何内容；空文件或无有效会话记录的 transcript 与既有 /transcript 路由行为完全一致（404 session_not_found），也就是说新路由在这里没有引入新的暴露面。
5. 最坏情况的页面体积
520 个 turn 的会话，每个 prompt 超过 160 码点、每个回复超过 240 码点，全部使用非 BMP（4 字节）字符：
| 请求 | 响应体 | 每 turn | 上限是否守住 |
|---|---|---|---|
turn-index?limit=500 |
850.1 KiB | 1741 B | label 160 码点、detail 240 码点 |
turn-index?limit=100 |
170.5 KiB | 1746 B | 同上 |
| 在 ordinal 250 处的锚定 transcript 读取 | 13.7 KiB | — | targetRecordId 正确，hasOlder: true |
该路由经由 serializeWorkspaceTranscriptResponse 序列化，上限 32 MiB，因此即便这种对抗性形状也只有上限的约 1/38。
完整矩阵
75/75 条脚本断言，全部打在真实 daemon 上：两条路由族的协议矩阵（41）、跨真实追加的快照冻结（3）、TypeScript SDK 面（6）、快照失效与边界场景（11）、冷重启（3）、由脚本化模型端到端驱动的真实会话（6）、以及真实的工具调用/结果配对（5）。
其中两项值得单独提一句，因为用的是产品自己写入的记录而非手工夹具：实时 prompt 无需显式 flush 就会出现在 turn 索引中，带着模型回答作为有界 detail，并被反查到运行时 promptId；真实工具 turn 从锚定页向前走，每个工具调用都能连同其 update 一起取到，而工具名、参数和输出都不出现在索引预览中。
单测
在 872e136：core session-transcript-reader 135、cli server 1178、cli acpAgent 611、cli telemetry/telemetry-catalog/error-response/multi-workspace 263、acp-bridge bridge 895、sdk 509 —— 全绿。
在与今天 origin/main 试合并后的树上：此前变红的三个门禁 71、core session-transcript-reader 135、cli server + acpAgent + multi-workspace + error-response 1983 —— 全绿。
环境
| 两个臂 | 872e136 与 60161cb 各自独立 worktree，均通过 node esbuild.config.js 构建出 dist/cli.js |
| 主机 | Linux 6.12，Node v22.22.2（PR 正文将 Linux 标为 |
| Daemon | qwen serve --port 0 --token … --workspace wsA --workspace wsB，隔离 QWEN_HOME，仅 loopback |
| 模型 | loopback 上的脚本化 OpenAI 兼容服务；无凭证、无网络、无模型开销 |
| 断言口径 | 原始 HTTP 状态码 / code / JSON 响应体，并直接读取磁盘上的 transcript 文件以证明泄漏扫描非空洞 |
未覆盖
Windows 与 macOS；realtime_message 记录的真实语音生产方；直接走 stdio 的 ACP（bridge 路径已通过 owner 路由验证）；Web Shell UI —— 按设计属于 Phase 2/3。第 1 轮的 S1 与 N2 未重测：它们在那一轮就是非阻塞项，且此后生产代码没有变动。
🤖 Generated with Claude Code — Claude Opus 5 (1M context)
wenshao
approved these changes
Sep 4, 2026
yiliang114
reviewed
Sep 4, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed at 872e136 across correctness and security — no blocking issues found.
Turn model and boundaries check out: empty/anchor/out-of-range cases all surface structured 400/409 errors, pages are capped at 500 entries with no allocation proportional to start, and cross-session snapshot confusion fails closed. The new routes reuse the existing transcript authorization path (requireSessionId / assertSessionLoadable / runtime-generation assertions), so no weaker gate than the sibling transcript endpoint. Telemetry additions are route-attribution only, no content logged.
Two non-blocking consistency notes inline.
| } | ||
| }); | ||
|
|
||
| app.get('/workspaces/:workspace/session/:id/turn-index', async (req, res) => { |
Collaborator
There was a problem hiding this comment.
Not blocking: the turn-index response isn't passed through redactSdkSurfaceEvent(..., workspaceTrusted) the way the workspace transcript page route is (around line 4692). Labels/details are projected transcript content, and this sits behind the same session-resolution gates, so the impact is bounded — but if untrusted-secondary-workspace content ever needs redaction beyond events, this projection would need the equivalent treatment.
| throw error; | ||
| } | ||
| } | ||
| case SERVE_STATUS_EXT_METHODS.sessionTurnIndex: { |
Collaborator
There was a problem hiding this comment.
Not blocking: this handler type-checks snapshot but doesn't apply the REST-side workspaceTranscriptCursorExceedsLimit size cap or the 200-char atRecordId cap. Bounded by the daemon request-body limits and the reader's fail-closed decode, so this is a validation-consistency nit rather than a defect.
yiliang114
approved these changes
Sep 4, 2026
yiliang114
left a comment
yiliang114
left a comment
Collaborator
There was a problem hiding this comment.
LGTM. Reviewed the turn-navigation reader and both route surfaces at 872e136 — boundaries fail closed with structured errors, pagination is capped, and the new routes reuse the existing transcript authorization path. The two inline notes are non-blocking consistency items. CI is green on this head.
doudouOUC
dismissed stale reviews from qwen-code-ci-bot and qqqys
September 4, 2026 06:34
Already have 2 approves,3ks
chiga0
reviewed
Sep 4, 2026
chiga0
left a comment
chiga0
left a comment
Collaborator
There was a problem hiding this comment.
⚠️ Post-merge review — PR was merged before this review completed. Findings are for the record.
审查结论
整体设计质量高，安全机制完整。核心安全路径全部经过验证：
- ✅ Snapshot HMAC 签名：
createHmac('sha256', key)+timingSafeEqual时序安全比较，正确 - ✅ Cursor vs Snapshot 域分离：snapshot payload 含
kind: 'turn_index'，cursor 无此字段，MAC 覆盖不同内容，无域混淆风险 - ✅ 跨 session 保护：
snapshot.sessionId !== sessionId在 reader 和 ACP 两处均有检查 - ✅ 跨 workspace 保护：HMAC key 绑定 workspace（
getCursorHmacKeyPath(workspaceCwd)） - ✅ Snapshot 重放攻击防护：
snapshotSize + fileIdentity(dev/ino) + leafUuid三重验证 - ✅
nextPosition修复（原position + pageUuids.length，修正为pageStartPosition + pageUuids.length） - ✅ 双重
assertRuntimeGenerationOpen检查：bridge 调用前后各一次，防止竞态 - ✅ telemetry-catalog 路由计数：已修复为 65（qqqys 阻断性发现）
- ✅ 测试覆盖：含 leaf 变更检测、跨 session 拒绝、anchored 页前向延伸等关键 case
F1 · Minor · atRecordId 长度校验不一致
路由层 parseTranscriptTurnAnchorQuery（session.ts）对 atRecordId 设有 200 字符上限，ACP 路径 acpAgent.ts 仅检查 length === 0，无上限保护。
trigger → atRecordId 超过 200 字符时，路由层拒绝（HTTP 400），
ACP 路径放行 → 行为不一致，可能导致下游处理接受意外长度输入。
建议在 ACP 路径同步加入 rawAtRecordId.length > 200 检查，与路由层对齐。
（已与 yiliang114 确认为 validation-consistency nit，不阻断合并。）
Cross-check
| 审阅者 | 状态 | 验证结果 |
|---|---|---|
| qqqys | CHANGES_REQUESTED（route count 62→63） | ✅ 当前 head 已修复为 64→65 |
| qwen-code-ci-bot | 多轮 suggestion | ✅ 历史发现均已在当前 head 修复 |
| samuelhsin / wenshao / yiliang114 | APPROVE | ✅ 无冲突 |
This was referenced Sep 4, 2026
wenshao
added a commit
to zhuyuy/qwen-code
that referenced
this pull request
Sep 5, 2026
…ke point (QwenLM#11047) * fix(cli): route the transcript turn-index handler through the pin choke point The QwenLM#10988 guard fails the build on any direct mention of the runtime-root context runner outside the single choke point; the session turn-index page handler (from QwenLM#10751) still composed the routing by hand with loadSettingsCached, the exact decision runWithPinnedRuntimeBaseDirForRequest exists to make in one place (transcript pages are listed in the choke point's own doc). Behaviour is identical: same cached settings for the same cwd, same pin; the operation never used the local settings object. * test(cli): pin the turn-index handler's per-request routing and roster The choke-point guard permits the scoped-settings shape, so an edit back to this.runWithPinnedRuntimeBaseDir(this.settings, cwd, ...) would stay green without a behavioural test; mirror the six sibling routing tests. Also add the turn-index handler to the choke point's normative handler roster. --------- Co-authored-by: qwen-code-dev-bot <qwen-code-dev-bot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
8 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR delivers Phase 1 of #10750: the daemon and SDK contract required for session-wide Web Shell turn navigation.
It adds a bounded sparse index for durable ordinary, realtime, and scheduled turns; signed workspace-bound snapshots for stable metadata pagination; and snapshot-bound transcript reads that can open a bounded page at a selected persisted turn. Anchored reads expand to a safe replay boundary when necessary so tool call/result pairs are not split.
The contract is available through owner-resolved and workspace-qualified daemon routes, the ACP bridge, and the TypeScript SDK, and is advertised atomically through
session_turn_navigation. Preview projection exposes only bounded public prompt/assistant text and removes control context, generated attachment tokens, thoughts, tool calls, arguments, results, and raw payloads.The included design records the full three-phase delivery. This PR intentionally stops before the Phase 2 bounded browser data layer and Phase 3 virtualized rail UI.
Why it's needed
The current compact rail is derived from the transcript blocks already loaded in Web Shell. Initial restore, upward pagination, and block/byte retention limits mean it cannot represent every turn in a long session. Loading more transcript records up front would only move the memory limit.
The existing transcript API is sequential and cannot request a bounded page containing an arbitrary persisted turn. Separating a lightweight session-wide index from a bounded random-access transcript page enables complete navigation without downloading all intervening records or retaining the whole transcript in browser memory.
Reviewer Test Plan
How to verify
/capabilitiesadvertisessession_turn_navigation, and confirm the latest turn-index page includes only the three durable visible turn kinds with bounded public labels/details.totalTurnsremain unchanged, while a fresh request observes the appended turn.targetRecordId, computeshasOlderfrom the actual safe page boundary, and continues toward the same frozen tail.Evidence (Before & After)
Before: released
qwen 0.22.2did not advertisesession_turn_navigation; both turn-index routes returned 404; the transcript route ignoredatRecordIdandsnapshotand returned the same sequential page.After: the local
0.22.3bundle advertised the capability; an isolated daemon E2E returned two initial turns, kept the old snapshot at two after append, returned three from a fresh snapshot, located the exact middle turn, and rejected both an invalid anchor and a tampered snapshot. Focused tests passed across core (149), CLI (8), ACP bridge (11), and SDK (7), and fullnpm run build,npm run typecheck,npm run lint,npm run bundle, andgit diff --checkcompleted successfully.This is a protocol-only delivery, so visual Before/After evidence is N/A.
Tested on
Environment (optional)
Node.js 22 workspace build and an isolated loopback
qwen servedaemon using the bundled local CLI; no model request was made during the E2E.Risk & Scope
Linked Issues
Part of #10750
中文说明
本 PR 做了什么
本 PR 交付 #10750 的 Phase 1：为 Web Shell 全会话 turn 导航提供 daemon 与 SDK 协议基础。
实现包括：针对持久化普通、实时和定时 turn 的有界稀疏索引；用于稳定元数据分页、绑定 workspace 的签名快照；以及能够从选定持久化 turn 打开有界页面的快照绑定 transcript 读取。必要时，锚点读取会扩展到安全回放边界，避免拆开 tool call/result 配对。
该协议通过 owner-resolved 和 workspace-qualified daemon 路由、ACP bridge 与 TypeScript SDK 提供，并由
session_turn_navigation原子能力统一声明。预览投影只暴露有界的公开 prompt/assistant 文本，并移除控制上下文、生成的附件 token、thinking、tool call、参数、结果和原始 payload。随附设计记录完整的三个阶段。本 PR 刻意止于 Phase 2 的浏览器有界数据层和 Phase 3 的虚拟化导航 UI 之前。
为什么需要
当前紧凑导航条由 Web Shell 已加载的 transcript block 推导。初始恢复、向上分页以及 block/字节保留上限意味着它无法表示长会话中的全部 turn。预先加载更多 transcript 记录只会推迟内存上限出现的时间。
现有 transcript API 是顺序式的，无法请求包含任意持久化 turn 的有界页面。将轻量的全会话索引与有界随机访问 transcript 页面分离后，可以在不下载所有中间记录、不把完整 transcript 保留在浏览器内存中的前提下实现完整导航。
Reviewer 测试计划
如何验证
/capabilities声明session_turn_navigation，并确认最新 turn-index 页面只包含三种持久化可见 turn，且标签/摘要仅含有界公开内容。totalTurns不变，而不带 snapshot 的新请求能够看到新增 turn。targetRecordId，hasOlder按实际安全页面边界计算，且续页始终沿同一冻结尾部向后推进。证据（Before & After）
Before：已发布的
qwen 0.22.2不声明session_turn_navigation；两个 turn-index 路由都返回 404；transcript 路由忽略atRecordId和snapshot，返回与普通顺序读取完全相同的页面。After：本地
0.22.3bundle 声明了该能力；隔离 daemon E2E 初始返回两个 turn，追加后旧 snapshot 仍为两个，新 snapshot 返回三个；能够精确定位中间 turn，并拒绝非法锚点和篡改 snapshot。core（149）、CLI（8）、ACP bridge（11）和 SDK（7）的聚焦测试全部通过，完整的npm run build、npm run typecheck、npm run lint、npm run bundle与git diff --check也均成功完成。这是纯协议交付，因此视觉 Before/After 证据不适用。
测试平台
环境（可选）
Node.js 22 workspace build，以及使用本地 bundle 的隔离 loopback
qwen servedaemon；E2E 过程中没有发起模型请求。风险与范围
关联 Issue
#10750 的一部分