feat(serve): Add cursor-paged transcript replay endpoint - #6525
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
636fb98 to
3d67eb5
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push,因为这会使已有的评审评论失效。另外,供日后参考:作为集成流程的一部分,机器人始终会自动将所有改动压缩(squash)为单个提交。 |
|
Thanks for the PR! (Re-run after test isolation fix and merge conflict resolution.) Template looks good ✓ — all required sections present, bilingual description, reviewer test plan with exact commands. Problem: Real gap left by #6482. Bounded Direction: Aligned. Cursor-paged transcript replay is a standard pattern for daemon APIs with large session histories. The approach (freeze snapshot on first page, page by active ChatRecord count, HMAC-signed cursors) is sound and doesn't mutate live session state. No direct Claude Code CHANGELOG reference, but daemon session management APIs are core to qwen-serve. Size: 1,024 production logic lines in core paths ( Approach: Scope feels right for what it does. The four-package split (core reader → CLI route + bridge → SDK client → docs) follows the existing architecture cleanly. The Test isolation (previously flagged): The Moving on to code review. 🔍 中文说明感谢贡献!(测试隔离修复 + 合并冲突解决后的重新审查。) 模板完整 ✓ —— 所有必需章节齐全,双语描述,包含精确命令的 reviewer test plan。 问题: #6482 留下的真实缺口。有界的 方向: 对齐。游标分页的 transcript replay 是大型 session 历史的 daemon API 标准模式。方案(首页冻结快照、按 active ChatRecord 分页、HMAC 签名 cursor)合理且不改变 live session 状态。 规模: 核心路径 1,024 行生产逻辑( 方案: 范围合理。四包拆分(core reader → CLI route + bridge → SDK client → docs)遵循现有架构。 测试隔离(之前标记的问题): 集成测试中的 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
There was a problem hiding this comment.
Pull request overview
Adds a new cursor-paged GET /session/:id/transcript endpoint that returns id-less replay frames reconstructed from the persisted active-session JSONL, without mutating live session/EventBus state. This extends the serve daemon + ACP bridge + SDK surface so clients can fetch the full persisted transcript in stable pages (separate from bounded /load replay).
Changes:
- Add a core
SessionTranscriptReaderthat freezes a snapshot, indexes active parent chains, and pagesChatRecords with an opaque cursor. - Wire a new ACP status method + bridge plumbing + serve route, including error mapping and capability registration.
- Add SDK types/client helper + tests, and document the new protocol/usage.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/services/session-transcript-reader.ts | Implements snapshot-frozen, cursor-paged transcript reading + index cache. |
| packages/core/src/services/session-transcript-reader.test.ts | Unit tests for paging/branch selection/snapshot freezing/fragment aggregation. |
| packages/core/src/index.ts | Exposes transcript reader/cursor API from the core barrel. |
| packages/cli/src/acp-integration/session/HistoryReplayer.ts | Adds paged replay support + pending tool call continuation state. |
| packages/cli/src/acp-integration/acpAgent.ts | Implements qwen/status/session/transcript using SessionTranscriptReader + paged replay conversion. |
| packages/cli/src/acp-integration/acpAgent.test.ts | Tests the new ACP ext method behavior and id-less replay frames. |
| packages/acp-bridge/src/status.ts | Registers the new qwen/status/session/transcript ext method name. |
| packages/acp-bridge/src/bridgeTypes.ts | Adds bridge request/response types for transcript paging. |
| packages/acp-bridge/src/bridge.ts | Implements getSessionTranscriptPage() bridge call to the ACP child. |
| packages/cli/src/serve/acp-session-bridge.ts | Re-exports new bridge transcript types through the serve bridge surface. |
| packages/cli/src/serve/server/session-archive.ts | Updates assertSessionLoadable() to return location (used by transcript route). |
| packages/cli/src/serve/routes/session.ts | Adds GET /session/:id/transcript route + query parsing + archive/loadability gating. |
| packages/cli/src/serve/server/error-response.ts | Maps transcript-specific child error kinds to stable HTTP codes. |
| packages/cli/src/serve/capabilities.ts | Registers session_transcript capability. |
| packages/cli/src/serve/server.test.ts | Adds route-level tests covering paging and error behavior. |
| packages/sdk-typescript/src/daemon/types.ts | Adds DaemonSessionTranscriptPage* types. |
| packages/sdk-typescript/src/daemon/DaemonClient.ts | Adds getSessionTranscriptPage() REST helper (forces REST even with transport). |
| packages/sdk-typescript/src/daemon/index.ts | Re-exports transcript types from the daemon barrel. |
| packages/sdk-typescript/src/index.ts | Re-exports transcript types from the SDK root barrel. |
| packages/sdk-typescript/test/unit/DaemonClient.test.ts | Unit tests for getSessionTranscriptPage() behavior. |
| packages/sdk-typescript/scripts/build.js | Updates browser bundle size budget for the new helper. |
| docs/users/qwen-serve.md | Documents /transcript usage and distinction from bounded /load. |
| docs/developers/qwen-serve-protocol.md | Adds protocol/capability docs + endpoint contract for /session/:id/transcript. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Code Review (re-run)Reviewed the core implementation in detail:
Route handler ( Bridge ( No critical blockers found. No security holes, no correctness bugs, no AGENTS.md violations. The implementation follows project conventions (ESM, strict TypeScript, no Build & Test Results
Real-Scenario TestingSpawned a real Daemon log during the run showed only expected 中文说明代码审查(重新审查)详细审查了核心实现:
路由处理器( Bridge( 无关键阻塞问题。 无安全漏洞、无正确性 bug、无 AGENTS.md 违规。 构建与测试结果
真实场景测试启动真实 Daemon 日志仅显示预期的 WARN 行 —— 无崩溃或未捕获异常。 — Qwen Code · qwen3.7-max |
ReflectionThis PR has been through a thorough review cycle — multiple rounds of critical findings from both the bot and maintainer @wenshao, all addressed across several commits. The current state is solid. Going back to my independent proposal from Stage 2a: I would have built something similar — a new The motivation is genuine: bounded The code is straightforward for what it does. 967 lines in the reader is a lot, but it's a genuinely new read path over JSONL files with non-trivial requirements (frozen snapshots, cursor integrity, memory bounds). The test coverage is thorough — 58 unit tests plus 4 integration tests covering the real daemon data path. After seeing it run: all 32 integration tests pass (including the previously-failing metadata test), and the real-daemon curl tests confirm correct HTTP behavior for both success and error cases. The results match what the PR promised. The test isolation fix ( One note: the 1,024 production lines in core paths is substantial for a fork PR. The maintainer has already validated extensively, but the scale means a human should give final sign-off on the architectural fit. VerdictFeature is correct, well-tested, and fills a real gap. All previous critical findings are resolved. The test isolation issue is fixed. Build, typecheck, lint, and all tests green. Approving. ✅ 中文说明反思这个 PR 经历了充分的审查周期 —— 来自 bot 和维护者 @wenshao 的多轮关键发现,全部在多个 commit 中解决。当前状态扎实。 回到我在 Stage 2a 的独立方案:我会构建类似的东西 —— core 中新的 动机真实:#6482 的有界 代码对于所做的事情来说直接明了。reader 的 967 行很多,但这是一个真正全新的 JSONL 读取路径,有非平凡的需求(冻结快照、cursor 完整性、内存边界)。测试覆盖充分 —— 58 个单元测试加 4 个集成测试覆盖真实 daemon 数据路径。 运行验证后:32/32 集成测试通过(包括之前失败的 metadata 测试),真实 daemon curl 测试确认了成功和错误场景的正确 HTTP 行为。结果与 PR 承诺一致。 测试隔离修复( 一点说明: 核心路径 1,024 行生产代码对于 fork PR 来说是相当大量的。维护者已做过充分验证,但规模意味着需要人类对架构适配做最终确认。 结论功能正确、测试充分、填补真实缺口。之前所有关键发现已解决。测试隔离问题已修复。构建、类型检查、lint 和所有测试全绿。 批准。✅ — Qwen Code · qwen3.7-max |
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/acp-bridge/src/bridge.ts:4998 |
getSessionTranscriptPage bridge method has zero unit tests — timeout, channel closure, and idle timer behavior are untested |
Add bridge.test.ts tests for success, timeout, and channel-closed rejection paths |
packages/cli/src/acp-integration/acpAgent.ts:6000 |
ENOENT error mapping untested — both cursor (transcript_snapshot_unavailable) and no-cursor (resourceNotFound) branches lack coverage |
Mock SessionTranscriptReader to throw ENOENT, verify both branches map correctly |
packages/cli/src/acp-integration/acpAgent.ts:8393 |
getTranscriptReplayConfig cache — only cache-miss path tested; cache-hit and stale-disposal paths have no tests |
Add tests verifying config reuse on same settings and disposal on settings change |
— qwen3.7-max via Qwen Code /review
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/cli/src/acp-integration/acpAgent.ts:580 |
collectHistoryReplayUpdatesPage catch block returns hardcoded 'Replay conversion failed for this page' — loses the actual error message that the non-paged collectHistoryReplayUpdates preserves. API consumers cannot distinguish failure modes. |
Use replayError: \Replay conversion failed for this page: ${error instanceof Error ? error.message : String(error)}`` |
packages/cli/src/serve/routes/session.ts:745 |
activeRuntimes.length > 1 branch in multi-workspace fallback scan is untested. The liveOwner.kind === 'ambiguous' path (from resolveLiveSessionOwner) is covered, but the post-scan ambiguous case is not. |
Add a test where resolveLiveSessionOwner returns notFound but getSessionLocation returns 'active' for two workspaces. |
packages/core/src/services/session-transcript-reader.ts:420 |
aggregateRecords merge rules for usageMetadata (latest wins), toolCallResult (first wins), model (first wins), and timestamp (latest wins) have no test coverage. Only message.parts concatenation is tested. |
Extend the aggregation test with multi-fragment records carrying distinct values for each field, asserting the documented merge rules. |
packages/core/src/services/session-transcript-reader.ts:907 |
readPage passes cursor.replay through to the response, but no reader-level test verifies this roundtrip. The acpAgent handler depends on page.replay for cross-page replay state. |
Add a test that encodes a cursor with a replay object, calls readPage, and asserts page.replay matches. |
— qwen3.7-max via Qwen Code /review
Limit transcript index builds to bounded snapshots and surface oversized transcript errors as 413 responses. Give transcript status calls a dedicated timeout and update the capabilities integration baseline. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Sign transcript cursors so forged snapshot sizes cannot bypass the index cache, and keep hasMore tied to persisted record availability when replay conversion returns a partial page. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Avoid generating the transcript cursor HMAC key while importing the core barrel so unrelated tests with narrow crypto mocks can load core without requiring randomBytes. Keep the VS Code companion crypto mock partial so it only replaces the auth-token UUID behavior it asserts on. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. CI note: Test (ubuntu-latest, Node 22.x) is failing. Suggestion-level recommendations are in the Suggestion summary comment below.
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestion-level recommendations are in the Suggestion summary comment below.
Mark bounded replay truncation frames as having a transcript endpoint, sanitize paged transcript replay conversion errors, and remove the core reader's incomplete pre-encoded cursor field so cursors are only emitted after replay continuation state is merged. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Runtime verification report (real daemon, not mocks)I ran this branch as a real Verdict: blocking. The design underneath is sound — once I patched the crash locally, ~everything else in the PR description verified clean (details below). But the endpoint as shipped cannot serve a single successful request. Harness (click to expand)
🔴 Blocker 1 — every call to the endpoint kills the ACP child
Daemon stderr: Root cause. const config = await this.newSessionConfig(cwd, [], settings, sessionId, false);
const message = `Error: Session Id ${argv['sessionId']} already exists (active or archived). Delete or unarchive it first.`;
writeStderrLine(message);
process.exit(1); // ← kills the shared ACP child
Blast radius: the daemon runs one Suggested fix: don't reuse the transcript's 🔴 Blocker 2 — the cursor can never reach page 2 on a default daemonThis one is independent of Blocker 1 and survives fixing it. The cursor HMAC key is a per-process random, created lazily in the ACP child: // packages/core/src/services/session-transcript-reader.ts:146
cursorHmacKey ??= crypto.randomBytes(32);…and Causal A/B, same build (Blocker 1 patched locally), same session, no live sessions:
The same failure occurs on any child crash or restart mid-pagination, even with live sessions: Two problems here:
Suggested fix: derive/persist the signing key outside the child's lifetime (daemon-owned, or sign in ✅ What verifies cleanWith a one-token local patch (
Cursor tamper matrix — 9/9 forgeries rejected with
Teeth check (neutered 🟡 Other findings (non-blocking)1. Deleted transcript returns 2.
Normal transcripts are fine (~5 ms/MiB, linear). But a single 64 MiB record — a base64 image, a big 3. A full 4. The "frozen snapshot" is pinned by 5. The cursor is signed but not encrypted. It's plain base64url JSON, so 6. Why CI didn't catch Blocker 1Every new test stubs out the thing that crashes:
All of the PR's own suites pass on my checkout (core reader 6/6, Highest-value test to add: one real Reproduce# 1. build + run a real daemon (isolated HOME, mock OpenAI at $OPENAI_BASE_URL)
node packages/cli/dist/index.js serve --port 4179 --workspace "$WS" --no-web
# 2. real session + a few turns
SID=$(curl -s -XPOST localhost:4179/session -H 'content-type: application/json' \
-d '{"sessionScope":"thread"}' | jq -r .sessionId)
curl -s -XPOST localhost:4179/session/$SID/prompt -H 'content-type: application/json' \
-d '{"prompt":[{"type":"text","text":"hi"}]}'
# 3. Blocker 1 — 500, and the ACP child is gone
DPID=$(ss -tlnp | grep 4179 | grep -o 'pid=[0-9]*' | cut -d= -f2)
ps --ppid $DPID -o pid= # child alive
curl -s -w ' [%{http_code}]\n' "localhost:4179/session/$SID/transcript?limit=5"
ps --ppid $DPID -o pid= # child gone; any live session is now 404
# 4. Blocker 2 — after patching (1), on a default daemon with no live session:
# page 1 -> 200 ; page 2 with nextCursor -> 400 invalid_transcript_cursor
# add --channel-idle-timeout-ms 600000 and it passes.Happy to re-run the whole matrix once the two blockers are addressed — the harness is scripted and the rest of the feature already behaves exactly as the PR description claims. 中文版运行时验证报告(真实 daemon,非 mock)我把这个分支跑成了真实的 结论:阻塞性问题。 底层设计本身是站得住的 —— 我在本地把 crash 打上补丁之后,PR 描述里声称的行为几乎全部验证通过(见下)。但按当前代码,这个 endpoint 一次成功请求都跑不出来。 验证环境
🔴 阻塞问题 1 —— 每次调用都会杀死 ACP 子进程对一个已存在的 session(也就是这个路由唯一接受的输入)调用 daemon stderr: 根因。 const config = await this.newSessionConfig(cwd, [], settings, sessionId, false);
const message = `Error: Session Id ${argv['sessionId']} already exists (active or archived). Delete or unarchive it first.`;
writeStderrLine(message);
process.exit(1); // ← 杀死共享的 ACP 子进程
影响范围:daemon 只有一个 建议修复: 不要把 transcript 的 🔴 阻塞问题 2 —— 默认配置的 daemon 上,cursor 永远走不到第 2 页这个问题独立于阻塞问题 1,修掉 1 之后依然存在。 cursor 的 HMAC key 是子进程内的进程级随机值,懒初始化: // packages/core/src/services/session-transcript-reader.ts:146
cursorHmacKey ??= crypto.randomBytes(32);而 因果 A/B(同一构建,已本地修复阻塞 1;同一 session;无 live session):
即便存在 live session,只要子进程崩溃/重启,同样会复现: 这里有两个问题:
建议修复: 把签名密钥放到子进程生命周期之外(daemon 持有,或改在 ✅ 验证通过的部分在本地打了一处补丁(
cursor 篡改矩阵 —— 9/9 全部被
有效性(teeth)验证:把 🟡 其他问题(非阻塞)1. 删除 transcript 文件返回 2.
正常 transcript 没问题(约 5 ms/MiB,线性)。但一条 64 MiB 的 record —— base64 图片、大文件 3. 每一页都会构造并 4. 「冻结的 snapshot」是靠 5. cursor 只做了签名,没有加密。 它就是明文 base64url JSON,任何客户端都能读到 6. transcript handler 里的 为什么 CI 没有拦住阻塞问题 1新增的每个测试恰好都把会崩的那块 mock 掉了:
PR 自带的测试套在我的 checkout 上全部通过(core reader 6/6、 性价比最高的补测: 在 复现步骤# 1. 构建并启动真实 daemon(隔离 HOME,$OPENAI_BASE_URL 指向 mock OpenAI)
node packages/cli/dist/index.js serve --port 4179 --workspace "$WS" --no-web
# 2. 真实 session + 几轮对话
SID=$(curl -s -XPOST localhost:4179/session -H 'content-type: application/json' \
-d '{"sessionScope":"thread"}' | jq -r .sessionId)
curl -s -XPOST localhost:4179/session/$SID/prompt -H 'content-type: application/json' \
-d '{"prompt":[{"type":"text","text":"hi"}]}'
# 3. 阻塞问题 1 —— 500,且 ACP 子进程消失
DPID=$(ss -tlnp | grep 4179 | grep -o 'pid=[0-9]*' | cut -d= -f2)
ps --ppid $DPID -o pid= # 子进程存活
curl -s -w ' [%{http_code}]\n' "localhost:4179/session/$SID/transcript?limit=5"
ps --ppid $DPID -o pid= # 子进程消失;任何 live session 现在都是 404
# 4. 阻塞问题 2 —— 修复 (1) 之后,在无 live session 的默认 daemon 上:
# 第 1 页 -> 200 ;带 nextCursor 的第 2 页 -> 400 invalid_transcript_cursor
# 加上 --channel-idle-timeout-ms 600000 就能通过。两个阻塞问题修好后我可以把整套矩阵重跑一遍 —— 验证脚本都已经写好了,而且这个特性的其余部分完全符合 PR 描述。 |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
🧪 Maintainer local validation — PR #6525Built this PR from source on macOS and ran the full test plan plus a live end-to-end run against a real Commit validated: ✅ Build & static checks — all green
✅ Feature tests — all green
✅ Live end-to-end run against a real daemonSpawned a real Confirmed against the PR's stated contract: pages carry id-less
|
| Scenario | Result |
|---|---|
merge-base main (no transcript block), full file |
✅ 28/28 |
| PR HEAD, full file, clean & isolated (×3) | ❌ 31/32 — metadata fails 3/3 |
| oversized test + metadata (only these two) | ❌ metadata fails (5.7 s) |
| archived/conflicting (small files) + metadata | ✅ both pass (0.7 s) |
| PR HEAD + one-line cleanup (below), full file (×2) | ✅ 32/32 |
Impact: Integration Tests (CLI, No Sandbox) (vitest run --root ./integration-tests cli) runs this whole file, so it will go red deterministically once it runs. Not a product bug — the /transcript endpoint's 413 handling is correct (see live run above); this is purely test hygiene.
Proposed fix (verified → 32/32 green):
code: 'transcript_too_large',
maxBytes: SESSION_TRANSCRIPT_MAX_INDEX_BYTES,
});
+ // Remove the 256MiB sparse fixture so later tests'
+ // listWorkspaceSessions(REPO_ROOT) doesn't choke on it.
+ rmSync(filePath, { force: true });
});
});(Equivalent alternatives: scope the transcript fixtures to a dedicated cwd instead of REPO_ROOT, or clean the chats/ dir in an afterEach.)
Verdict
Feature: LGTM — solid coverage, live HTTP matches the documented contract, build/typecheck/lint green. One blocker before merge: the 1-line test cleanup above, otherwise the integration CI job fails deterministically.
🇨🇳 中文版本(点击展开)
🧪 Maintainer 本地验证 — PR #6525
我在本地(macOS)从源码构建了该 PR,运行了完整的测试计划,并对真实的 qwen serve daemon 做了端到端实测。功能本身是正确的、覆盖充分。 但我发现了一个会导致 CI 集成测试任务失败的测试隔离问题(仅测试代码,不是产品 bug)—— 详情和一行修复见下文。
验证的 commit: 13f8f90(qwen: adopt transcript review suggestions (#6525))· 环境: macOS (Darwin 24.6),Node v22.23.1,npm 10.9.8
✅ 构建与静态检查 —— 全部通过
| 检查项 | 命令 | 结果 |
|---|---|---|
| 空白字符 | git diff --check |
✅ 干净 |
| 构建 | npm run build |
✅ exit 0 |
| 类型检查 | npm run typecheck |
✅ 全部 workspace |
| 打包 | npm run bundle |
✅ exit 0 |
| Lint | core / cli / acp-bridge / sdk(定向) | ✅ exit 0 |
✅ 功能测试 —— 全部通过
| 测试套件 | 结果 |
|---|---|
core · session-transcript-reader.test.ts |
✅ 30 通过 |
cli · server.test.ts — GET /session/:id/transcript |
✅ 18 通过 |
cli · acpAgent.test.ts — qwen/status/session/transcript |
✅ 8 通过 |
cli · HistoryReplayer.test.ts |
✅ 32 通过 |
acp-bridge · bridge.test.ts |
✅ 368 通过 |
core · config.test.ts(含 lenientToolWarmup) |
✅ 371 通过 |
sdk-typescript · DaemonClient.test.ts — transcript |
✅ 2 通过 |
集成 · qwen-serve-routes.test.ts → transcript 块(真实 daemon,单独运行) |
✅ 4 通过 |
✅ 对真实 daemon 的端到端实测
启动了真实的 node dist/index.js serve 进程,写入持久化 JSONL transcript,并通过真实 HTTP 驱动 endpoint。游标分页和每一个文档记录的错误码都完全符合协议文档:
### PAGE 1 — GET /session/:id/transcript?limit=1 (HTTP 200)
{ sessionId: "99999999-…-1234567890ab", hasMore: true,
nextCursor: "eyJ2IjoxLCJzZXNzaW9uSWQiOiI5OTk5…(signed)",
eventCount: 1, eventTypes: ["session_update"], anyEventBusIds: false }
### PAGE 2 / PAGE 3 → hasMore: true,跟随 nextCursor
### PAGE 4 → hasMore: false ← 分页结束
PAGINATION COMPLETE:4 页,4 条无 id 的 session_update 事件,hasMore 最终为 false
### ERR ?limit=0 → 400 invalid_transcript_limit {maxLimit: 500}
### ERR ?cursor=not-a-cursor → 400 invalid_transcript_cursor
### ERR 未知 session → 404
### ERR 已归档 session → 409 session_archived
### ERR 超大快照 → 413 transcript_too_large {snapshotSize, maxBytes}
✅ 所有真实 HTTP 行为都与文档契约一致。
已确认符合契约:每页只有无 id 的 session_update 帧,不含 EventBus 的 id/lastEventId,nextCursor 为签名/不透明字符串,hasMore 在冻结快照最后一页翻转为 false。
⚠️ 发现 —— 测试隔离问题会导致 CI 集成任务失败(仅测试代码,一行可修)
运行整个 integration-tests/cli/qwen-serve-routes.test.ts 文件时,已存在的兄弟测试 PATCH /session/:id/metadata > updates displayName 会稳定失败(3/3 干净隔离运行):
× qwen serve — PATCH /session/:id/metadata > updates displayName 4097ms (retry x2)
→ expected undefined to be 'Integration Test Session'
Tests 1 failed | 31 passed (32)
根因(已二分定位): 新增测试 rejects oversized transcript snapshots with 413(第 485 行)执行了
truncateSync(filePath, SESSION_TRANSCRIPT_MAX_INDEX_BYTES + 1),在共享 workspace 的 chats/ 目录里留下一个 256 MiB 的稀疏 JSONL 文件且从不清理。后面的 metadata 测试调用 listWorkspaceSessions(REPO_ROOT),就会去读这个 256 MiB 文件(约 4–5.7 秒),最终返回的 live session 没有 displayName。
| 场景 | 结果 |
|---|---|
merge-base main(无 transcript 块)完整文件 |
✅ 28/28 |
| PR HEAD 完整文件,干净隔离(×3) | ❌ 31/32 —— metadata 3/3 失败 |
| oversized 测试 + metadata(只跑这两个) | ❌ metadata 失败(5.7 秒) |
| archived/conflicting(小文件)+ metadata | ✅ 都通过(0.7 秒) |
| PR HEAD + 一行清理(见下)完整文件(×2) | ✅ 32/32 |
影响: Integration Tests (CLI, No Sandbox)(vitest run --root ./integration-tests cli)会跑整个文件,所以一旦运行就会稳定变红。这不是产品 bug —— /transcript endpoint 的 413 处理是正确的(见上面的实测);纯粹是测试卫生问题。
建议修复(已验证 → 32/32 通过):
code: 'transcript_too_large',
maxBytes: SESSION_TRANSCRIPT_MAX_INDEX_BYTES,
});
+ // 删除 256MiB 稀疏文件,避免后续测试的
+ // listWorkspaceSessions(REPO_ROOT) 被它拖垮。
+ rmSync(filePath, { force: true });
});
});(等价方案:把 transcript 的 fixture 写到一个专用 cwd 而不是 REPO_ROOT,或在 afterEach 里清理 chats/ 目录。)
结论
功能:LGTM —— 覆盖充分,真实 HTTP 行为符合文档契约,构建/类型检查/lint 全绿。合并前有一个阻塞项: 上面这一行测试清理,否则集成 CI 任务会稳定失败。
|
@qwen-code /resolve |
|
Qwen Code resolved the merge conflicts and pushed the branch update. Merge conflict resolution summary — PR #6525Base branch
Conflicted file
Conflict descriptionBoth branches added new helper functions at the same insertion point (after
ResolutionKept both sides. The functions are independent additions that serve different features (transcript pagination vs. approval mode override) and are all referenced by route handlers later in the file:
All required imports ( Commit
|
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
…#6525) The transcript-paging integration suite wrote ~6 persisted chats/*.jsonl sessions into the daemon's project dir and never removed them. Because vitest runs a file's suites sequentially, those leftover sessions widened a pre-existing race in the later 'PATCH /session/:id/metadata > updates displayName' test (a freshly-created session can exist on disk but not yet appear in the listWorkspaceSessions page), making it fail deterministically in the no-AK smoke run. Add an afterAll to the transcript suite that removes the project chats/ dir, restoring a clean session list for subsequent suites. Verified: full no-AK suite now passes 43/43 across repeated runs.
…wenLM#6525) The record() helper derived the ISO timestamp seconds from text.length, producing invalid values (e.g. 00:00:013) once a record's text reached 10+ chars — harmless today only because no test asserted startTime. Replace it with a monotonic base+offset timestamp (always valid, strictly increasing). Also assert the previously-unchecked required SessionTranscriptRecordPage fields (sessionId, filePath, startTime, lastUpdated); the strict-ISO checks on startTime/lastUpdated guard against the timestamp-helper class of bug.
wenshao
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
— qwen3.7-max via Qwen Code /review
| return base; | ||
| } | ||
|
|
||
| async function readAggregatedRecords( |
There was a problem hiding this comment.
[Suggestion] readAggregatedRecords performs sequential readSegmentRecords calls for each UUID in the page. Since these reads are independent (no data dependency between UUIDs), they can be parallelized with Promise.all, reducing wall-clock time on high-latency storage from O(N) sequential reads to roughly a single read plus aggregation overhead.
| async function readAggregatedRecords( | |
| async function readAggregatedRecords( | |
| index: TranscriptIndex, | |
| uuids: string[], | |
| ): Promise<ChatRecord[]> { | |
| const handle = await fsp.open(index.filePath, 'r'); | |
| try { | |
| const results = await Promise.all( | |
| uuids.map(async (uuid) => { | |
| const entry = index.byUuid.get(uuid); | |
| if (!entry) return null; | |
| const physicalRecords: ChatRecord[] = []; | |
| for (const segment of entry.segments) { | |
| physicalRecords.push( | |
| ...(await readSegmentRecords(handle, index.filePath, segment, uuid)), | |
| ); | |
| } | |
| return physicalRecords.length > 0 | |
| ? aggregateRecords(physicalRecords) | |
| : null; | |
| }), | |
| ); | |
| return results.filter((r): r is ChatRecord => r !== null); | |
| } finally { | |
| await handle.close(); | |
| } | |
| } |
— qwen3.7-max via Qwen Code /review
| } | ||
| this.transcriptReplayConfigCache.clear(); | ||
| } | ||
|
|
There was a problem hiding this comment.
[Suggestion] When a cache entry with an in-flight pending promise is replaced by a new entry (settings changed between concurrent calls), the old pending Config is disposed in the current !== entry branch. However, disposeTranscriptReplayConfig catches disposal failures at debug level only — if the tool registry's stop() fails, resources (child processes, sockets from MCP transport) leak silently. For a long-running daemon, this could compound over time.
Consider attaching a fire-and-forget cleanup directly to the promise rather than relying on the post-await check:
void pending.then(config => {
if (this.transcriptReplayConfigCache.get(key) !== entry) {
this.disposeTranscriptReplayConfig(config);
}
}).catch(() => {});— qwen3.7-max via Qwen Code /review
| ): Promise<Config> { | ||
| const key = path.resolve(cwd); | ||
| const cached = this.transcriptReplayConfigCache.get(key); | ||
| if (cached?.settings === settings) { |
There was a problem hiding this comment.
[Suggestion] Coalesced callers sharing the same pending promise can both enter the staleness check when settings change mid-load. The first continuation to run disposes the Config; the second returns the same (now disposed) Config to its caller. That caller then runs transcript replay against a Config whose tool registry has been stopped.
Consider ensuring only one continuation owns the disposal path — e.g., by wrapping the shared promise so each caller gets its own .then() chain, or by tracking which caller is responsible for cleanup.
— qwen3.7-max via Qwen Code /review
| expect(bridge.sessionTranscriptCalls).toHaveLength(0); | ||
| }); | ||
|
|
||
| it.each(['501', '0', 'abc', '-1', '1&limit=2'])( |
There was a problem hiding this comment.
[Suggestion] The it.each block tests 501, 0, abc, -1, and 1&limit=2 as invalid, but doesn't test that limit=1 and limit=500 (the valid boundaries) are accepted. An explicit boundary test would guard against off-by-one errors in the validation logic.
— qwen3.7-max via Qwen Code /review
| ); | ||
| } | ||
|
|
||
| it('returns a paged transcript and does not expose EventBus cursors', async () => { |
There was a problem hiding this comment.
[Suggestion] No test verifies the default behavior when no limit query parameter is provided. The route's parseTranscriptLimitQuery returns undefined when no limit is present, which is a distinct code path. Consider adding a test that sends GET /session/:id/transcript without a limit param and asserts the bridge receives no limit field.
— qwen3.7-max via Qwen Code /review
| }); | ||
| }); | ||
|
|
||
| it('does not evict cached indexes when a new index exceeds the byte budget alone', async () => { |
There was a problem hiding this comment.
[Suggestion] The byte-budget cache eviction is only tested in the negative direction (an oversized new index doesn't evict). There is no positive test where adding a moderate-size index causes the total to exceed the budget and the oldest entry IS evicted. The while (getIndexCacheBytes() > getIndexCacheMaxBytes()) loop is a distinct code path that is never exercised.
— qwen3.7-max via Qwen Code /review
| events: BridgeEvent[]; | ||
| nextCursor?: string; | ||
| hasMore: boolean; | ||
| startTime?: string; |
There was a problem hiding this comment.
[Suggestion] startTime and lastUpdated are typed as optional (string | undefined), but the implementation in acpAgent.ts always sets them from page.startTime / page.lastUpdated (which are required fields on SessionTranscriptRecordPage). Consider making them required in the interface to match the actual contract, or document why they might be absent.
— qwen3.7-max via Qwen Code /review
| @@ -212,6 +215,7 @@ export async function assertSessionLoadable( | |||
| if (location === 'conflict') { | |||
There was a problem hiding this comment.
[Suggestion] assertSessionLoadable changed its return type from Promise<void> to Promise<SessionLocation>. All 3 existing callers (load route, export route, dispatch) ignore the return value, so they are unaffected — but the type change is untested. Consider adding a test that asserts the return value for completeness.
— qwen3.7-max via Qwen Code /review
|
@qwen-code /triage |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
All previous critical findings resolved. Build, typecheck, lint green. 32/32 integration tests pass (test isolation fix verified). Real-daemon endpoint tests confirm correct HTTP behavior for all documented error codes.
The session_transcript capability fills the gap left by bounded /load from #6482. HMAC-signed cursors, file identity tracking, and bounded index cache are solid defensive measures. Clean four-package architecture.
One advisory note: 1,024 production lines in core paths — maintainer @wenshao has already validated extensively.
✅ Local verification report (maintainer)Built and exercised this PR end-to-end on the PR head. Build, typecheck, lint, every targeted unit suite, the real-daemon integration suite, and a live Environment — commit What I ran
Live daemon walkthroughStarted a real Cursor pagination — Guarantees & error contract (HTTP status → body Side-effect-free — after every call above: This confirms the core promise: One note for reviewers running this locally (not a defect)The integration suite executes the esbuild bundle at 🇨🇳 中文版本✅ 本地验证报告(维护者)我在 PR 最新 commit 上对本 PR 做了完整的端到端验证。构建、类型检查、lint、所有针对性单元测试、真实 daemon 集成测试,以及一次实时 环境 — commit 执行的检查
实时 daemon 走查启动了一个真实的 Cursor 分页 — 语义保证与错误契约(HTTP 状态码 → body 无副作用 — 以上所有调用之后: 这验证了核心承诺: 给本地复现的 reviewer 的一点提示(非缺陷)集成测试通过 |
What this PR does
This PR adds a cursor-paged
GET /session/:id/transcriptendpoint for active persisted sessions. The endpoint freezes the current JSONL transcript snapshot on the first page, pages by activeChatRecordcount, reconstructs the active parent chain from lightweight transcript metadata, and asks the ACP child process to convert each page into id-less replay events without attaching a client, seeding the EventBus, creating a live session, or changing the live replay window.It also adds the child-side read-only status method, bridge and route plumbing, replay continuation for cross-page tool calls and cumulative usage, a bounded in-process transcript index cache, SDK client/types support, capability registration, and protocol/user documentation that distinguishes bounded
/loadreplay from full persisted/transcriptpages.Why it's needed
PR #6482 bounded live replay so
/loadcan safely restore a compact live window. Clients still need a way to inspect or rebuild the full persisted transcript without forcing/loadto materialize the entire history or mutate live session state, especially for long sessions where replay can be too large to return in one response.Reviewer Test Plan
How to verify
Reviewers can create or reuse a long active persisted session, confirm
/session/:id/loadstill returns bounded/truncated live replay behavior, then call/session/:id/transcript?limit=<n>and follownextCursoruntilhasMoreis false. The expected result is a complete sequence of id-lesssession_updatereplay frames with stable pagination over the frozen snapshot, no EventBus ids orlastEventId, and no change to live SSE cursor/window state.Local validation run:
npm run build;npm run typecheck;npm run lint --workspace @qwen-code/qwen-code-core;npm run lint --workspace @qwen-code/qwen-code;npm run lint --workspace @qwen-code/acp-bridge;cd packages/sdk-typescript && npx eslint src/daemon/DaemonClient.ts src/daemon/types.ts src/daemon/index.ts src/index.ts;cd packages/core && npx vitest run src/services/session-transcript-reader.test.ts;cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts -t "qwen/status/session/transcript";cd packages/cli && npx vitest run src/serve/server.test.ts -t "GET /session/:id/transcript";cd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.ts -t "transcript";git diff --check.Evidence (Before & After)
N/A. This is a daemon API, SDK, and documentation change with no TUI or visual UI changes.
Tested on
Environment (optional)
macOS local checkout, Node.js v22.22.3, npm 10.9.8.
Risk & Scope
ChatRecordcan still produce a large response because this PR pages records rather than splitting one record.npm run lint --workspace @qwen-code/sdkcurrently fails before linting code with an ESLint rule-loading TypeError in the SDK test tree, so SDK validation used targeted source lint plus SDK typecheck and the relevant SDK unit test./loadbehavior remains bounded live replay; clients should use/transcriptwhen they need full persisted transcript replay pages.Linked Issues
Related to #6482.
中文说明
What this PR does
这个 PR 为 active persisted session 新增 cursor 分页的
GET /session/:id/transcriptendpoint。endpoint 在第一页冻结当前 JSONL transcript snapshot,按 activeChatRecord数量分页,通过轻量 transcript metadata 重建 active parent chain,并让 ACP child process 把每一页转换成不带 id 的 replay events;整个过程不会 attach client、seed EventBus、创建 live session,也不会改变 live replay window。同时,这个 PR 新增 child-side read-only status method、bridge 和 route 链路、跨页 tool call 与 cumulative usage 的 replay continuation、进程内 bounded transcript index cache、SDK client/types 支持、capability 注册,以及协议/用户文档,明确区分 bounded
/loadreplay 和 full persisted/transcriptpages。Why it's needed
PR #6482 已经让 live replay 有界,保证
/load可以安全恢复 compact live window。客户端仍然需要一种方式查看或重建完整 persisted transcript,但不能让/loadmaterialize 全量历史,也不能影响 live session 状态;这对长 session 尤其重要,因为完整 replay 可能太大,无法一次响应返回。Reviewer Test Plan
How to verify
Reviewer 可以创建或复用一个较长的 active persisted session,先确认
/session/:id/load仍然返回 bounded/truncated live replay 行为,然后调用/session/:id/transcript?limit=<n>并持续跟随nextCursor,直到hasMore为 false。预期结果是获得完整的不带 id 的session_updatereplay frames,分页基于冻结 snapshot 保持稳定,不包含 EventBus ids 或lastEventId,并且不会改变 live SSE cursor/window 状态。本地验证已运行:
npm run build;npm run typecheck;npm run lint --workspace @qwen-code/qwen-code-core;npm run lint --workspace @qwen-code/qwen-code;npm run lint --workspace @qwen-code/acp-bridge;cd packages/sdk-typescript && npx eslint src/daemon/DaemonClient.ts src/daemon/types.ts src/daemon/index.ts src/index.ts;cd packages/core && npx vitest run src/services/session-transcript-reader.test.ts;cd packages/cli && npx vitest run src/acp-integration/acpAgent.test.ts -t "qwen/status/session/transcript";cd packages/cli && npx vitest run src/serve/server.test.ts -t "GET /session/:id/transcript";cd packages/sdk-typescript && npx vitest run test/unit/DaemonClient.test.ts -t "transcript";git diff --check。Evidence (Before & After)
N/A。这是 daemon API、SDK 和文档改动,没有 TUI 或视觉 UI 改动。
Tested on
Environment (optional)
macOS 本地 checkout,Node.js v22.22.3,npm 10.9.8。
Risk & Scope
ChatRecord仍然可能产生较大的响应,因为本 PR 按 record 分页,不拆分单条 record。npm run lint --workspace @qwen-code/sdk会在 SDK test tree 中因 ESLint rule-loading TypeError 于 lint 代码前失败,因此 SDK 验证采用 targeted source lint、SDK typecheck 和相关 SDK unit test。/load行为仍然是 bounded live replay;需要完整 persisted transcript replay pages 的客户端应使用/transcript。Linked Issues
Related to #6482.