feat(cli): Add standalone conversation isolation primitives - #9341
Conversation
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
@qwen-code /triage |
|
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: 2069 passed · 7 failed · 2076 total 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2069 通过 · 7 失败 · 2076 总计 Verification reportPR 9341 Deep Verification —
|
| cell | base | head | verdict |
|---|---|---|---|
| clean-legacy | loadable | loadable | preserved |
| explicit-live | loadable | loadable | preserved |
| truncated creation record | loadable (promoted) | rejected | flipped — fixed |
| garbage bytes | loadable (promoted) | rejected | flipped — fixed |
scalar line (42) |
loadable (promoted) | rejected | flipped — fixed |
array line ([1,2,3]) |
loadable (promoted) | rejected | flipped — fixed |
}{-glued complete record |
loadable | loadable | preserved (recovery kept) |
| damage past 10-record window | loadable | loadable | preserved (bounded scan) |
| valid head + damage within window | loadable (promoted) | rejected | flipped — fixed |
Mechanism nuance (accurate attribution): the four corrupt-head cells are rejected at head's new location gate (getSessionLocation → undefined because the head record is unreadable) plus integrity; the valid-head/damage-within-window cell is rejected purely by the new readLinesWithIntegrity complete flag in readCreationMetadataIfReadable. Base promotes all five because readCreationMetadata coerces an empty tolerant read to {}, which classifies as legacy standalone. 20/20 scripted assertions.
Secondary claims (all confirmed):
- Case lookup (
02-case-conflict.mjs, 8/8, witness02-case-conflict-ab.png): unique mixed-case resolves the persisted spelling on both arms; case-only duplicates and active+archived same-spelling throwSessionIdCaseConflictErrorat head but return a first-match/id at base. - Reserved source + lineage (
03-reserved-source.mjs, 19/19, witness03-reserved-source-ab.png): base's restore predicate continues for a transcript claimingsourceType:'standalone'(the hole); head rejects it (404 decision). Both arms refuse explicit standalone and standalone+sourceId on the Live path; legacy child of legacy parent loadable on both; self/cyclic lineage refused on both; explicit standalone child loadable without parent at head (design line 184).
Corrections
None to prior rounds (first verification round). One factual note for reviewers: the PR body states "targeted final tests passed … the server/ACP Agent/Live-task suites." That is not reproducible on Linux — see Finding 1.
Findings
F1 — Interface migration left test stubs behind: 5 unit tests red at delivered head (highest severity)
The PR moved restore/classification to the store interface, calling the new SessionService.readCreationMetadataIfReadable. The delivered tests still stub only the legacy method:
packages/cli/src/serve/server.test.ts—allows exact organization updates…,rejects generic REST creation and relocates loadable Live restores, and the PR's own newuses authoritative persisted spelling…all get404(expected200) because the real (unstubbed)readCreationMetadataIfReadablereads an absent file → undefined → 404.packages/cli/src/serve/live/live-task-service.test.ts—resumes an existing projectless task…andrestores Live source identity…throwTypeError: store.readCreationMetadataIfReadable is not a function(the PR addedgetSessionLocationto the mock class but not this method).
Repro: cd packages/cli && npx vitest run src/serve/server.test.ts src/serve/live/live-task-service.test.ts → 5 failed | 1753 passed (witness 04-gate-red-server-livetask-at-head.png). A/A control proves causation: both files are green at base (aa-live-task-base.txt 15/15, aa-server-lifecycle-base.txt lifecycle 18/18). Production code is not broken (real SessionService implements the method; real files exist), but the PR ships red tests on Linux CI and the restore behaviors these tests encode are currently pinned by nothing green.
Measured fix (witness 05-gate-green-after-measured-fix.png, patch logs/candidate-fix-interface-gap.patch): add a readCreationMetadataIfReadable mock delegating to the existing readCreationMetadata stub, and add the method to the live-task mock class. Result: server.test.ts lifecycle 19/19 and full server.test.ts + live-task-service.test.ts 1000/1000; hostile fixture (delivered) red, benign fixtures unchanged.
Suggested fix (measured)
// server.test.ts, beside each existing readCreationMetadata spy:
const readCreationMetadataIfReadable = vi
.spyOn(SessionService.prototype, 'readCreationMetadataIfReadable')
.mockImplementation((candidateId) => readCreationMetadata(candidateId));
// …and readCreationMetadataIfReadable.mockRestore() in the finally block.
// live-task-service.test.ts, in the mock SessionService class:
async readCreationMetadataIfReadable(sessionId: string) {
return (await this.sessionExists(sessionId))
? await this.readCreationMetadata(sessionId)
: undefined;
}F2 — Two new tests encode a macOS-only inode assumption; deterministic failure on Linux
conversation-directory-identity.test.ts > rejects same-path replacement against an expected identity and conversation-workspace.test.ts > …rejects replaced standalone child identities replace a directory via rm + mkdir at the same path and expect a new inode. Measured on this box: both overlay (/tmp) and ext4 (/__w) reuse the freed inode (04-identity-inode-probe.mjs; witness 07-identity-tests-red-on-linux-inode-reuse.png), so the guard legitimately reports the "replacement" as identical and the tests fail. This holds regardless of TMPDIR (re-run on ext4 gate-cli-identity-ext4.txt, still 2 failed). The production guard is correct: a rename-over replacement that truly changes the inode is rejected with unexpected_identity (04-identity-inode-probe.mjs 3/3). The PR's own test matrix marks Linux "未测试".
Repro: cd packages/cli && npx vitest run src/utils/conversation-directory-identity.test.ts src/serve/conversations/conversation-workspace.test.ts → 2 failed | 16 passed.
Measured fix: build the replacement by rename-ing a sibling directory over the target (a fresh inode on every POSIX filesystem) instead of rm+mkdir. Result: those two files 18/18 and the 5-file small gate 59/59 (fix-measure-identity.txt, fix-measure-smallgate.txt).
F3 — Coverage gap: self-lineage guard not pinned for the explicit-standalone-child shape (surviving mutant)
Mutation M3 (delete the normalizeSessionIdForLookup(parent)===normalizeSessionIdForLookup(sessionId) check in session-source.ts) survived the delivered session-source.test.ts (19/19 still green): the delivered SELF_ID case ({parentSessionId: SELF_ID}, no sourceType) is rejected anyway because a self-parent is never top-level. But the guard is load-bearing for a self-referencing explicit standalone child ({sourceType:'standalone', parentSessionId: SELF}), which hits the explicit-child early return before any parent read: with the guard removed that shape classifies as loadable. My probe test (mutation-M3-selfprobe-mutated.txt red, -restored.txt green) demonstrates the flip. Classification: coverage gap, not dead code and not a production defect — the guard exists and is correct; no delivered test asserts it for this shape. Suggest adding the probe fixture to session-source.test.ts.
Mutation matrix summary
| mutant | guard removed | pinning suite | result |
|---|---|---|---|
| M1 | integrity complete flag forced true |
core jsonl-utils + corruption | killed (4 red) — witness 06-mutation-m1-integrity-disabled.png |
| M2 | cross-spelling case-conflict throw | core sessionService case block | killed (1 red) |
| M3 | self-lineage comparison | cli session-source | survived delivered suite; killed by added probe (coverage gap F3) |
M1's reds land in core, not in session-source.test.ts, because that suite's store is stubbed at the contract boundary — the leaf is genuinely pinned by the core corruption/jsonl-utils tests, so the two layers are each covered by the suite closest to them. Positive control: each killed mutant failed on the exact intended assertion (quoted in logs/mutation-M*.txt).
Targeted gates (head, as delivered)
| gate | result |
|---|---|
| core: jsonl-utils + sessionService + corruption | 209/209 pass |
| cli small: session-source, session-id-admission, error-response, directory-identity, conversation-workspace | 57/59 (2 fail = F2) |
| cli big: server, acp-http transport, acpAgent, live-task-service | 1753/1758 (5 fail = F1); transport 326/326 and acpAgent 432/432 pass |
With the measured fixes the small gate is 59/59 and server.test.ts + live-task-service.test.ts is 1000/1000. No repo-wide gate was claimed or run.
Not covered
- Per-commit attribution: checkout is depth 2 (merge, base, head only); the 4 commits in
$QWEN_VERIFY_CONTEXTare not all individually reachable, so the aggregateHEAD^1..HEADdiff was verified, not each commit. - Windows:
isSameConversationPathwin32 branches and case-insensitive FS behavior not exercised (Linux container). - True multi-process daemon E2E: restore/admission verified at dist/component level plus the PR's own in-process
server.test.ts/transport.test.ts; no live daemon over a real socket was booted. - Performance / concurrency: no load or race testing of the new scan-window reads;
MAX_PROMPT_SCAN_LINES=10bounds the read, but no timing ladder was run (input is daemon-written transcripts, not outsider text, so ReDoS-style scaling was deemed low-value). - Lint/format/typecheck repo-wide: CI builds head (typecheck) green per the environment contract; no separate
npm run lintrun. - Docs-only changes (
standalone-daemon-sessions.md,2026-08-14-standalone-pr2-core.md): reviewed for claim context, not behaviorally tested.
Methodology
Environment: CI merge-ref checkout at f1123dc2 (base a35a23cd68, head e00ed9f9f6), node:22-bookworm-class container, Node v22.23.2; npm ci + npm run build pre-run. A/B base side built in git worktree tmp/base-tree at HEAD^1, rebuilding only packages/core + packages/cli; internal @qwen-code/qwen-code-core/qwen-code links were re-pointed into the base tree and base nested node_modules symlinked from head, then control validated by build fingerprint — base dist lacks SessionIdCaseConflictError/readCreationMetadataIfReadable, head dist has both, so neither cell could silently load the other's code. Harnesses 01–04 drive compiled dist/ output with real files (no stubs of the unit under test); mutation matrix applied single-point source edits in-tree, ran the pinning vitest suite, then git checkout -- restored (tree left clean, git status --porcelain empty). Evidence PNGs rendered via scripts/verify-capture.mjs; raw per-run logs live in logs/. Assertion counts in assertions.json map 1:1 to executed scripted checks (harness check() calls + vitest tests at delivered head); the 7 fail entries are the delivered-head test failures in F1/F2.
Re-running harnesses 01–03: they import BOTH builds, and the scratch base worktree was removed after the A/B cells were captured (per the verify workflow). Rebuild it with: git worktree add tmp/base-tree <baseRefOid>; ln -s head's packages/{core,cli}/node_modules into the worktree's packages; mirror root node_modules/@qwen-code/* into tmp/base-tree/node_modules/@qwen-code/ with qwen-code-core/qwen-code pointing at the base tree and everything else at head; symlink root node_modules/@lydell into the base tree (the core tsconfig paths mapping resolves ../../node_modules/@lydell/... relative to the tree); run npm run generate, then npm run build -w packages/core -w packages/cli inside the worktree. Harness 04 is head-only and reruns as-is.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Re-run (round 10), triggered by the author after @wenshao's re-verification at this head. Head moved
Gate passes on template/problem/direction/approach; the final verdict waits on Stage 3. 🔍 中文说明重跑（第 10 轮），由作者在 @wenshao 于本 head 完成重新验证后触发。Head 从
门禁在模板/问题/方向/方案上通过；最终结论见 Stage 3。🔍 — Qwen Code · qwen3.8-max Reviewed at |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 4 scenario(s). — Qwen Code · serve A/B |
Code reviewReviewed the full delta
Status of the two Criticals from the bot's round-7 review, checked against this head
Polish item carried from @wenshao's re-verification (his finding, attributed as such)The CLI misattributes an unreadable session store to an id collision — "already exists" advice that cannot help when enumeration itself failed. Reusing the daemon's Test evidenceUnattended run — no PR code is built or executed here; the evidence is the PR's own CI at Real-scenario coverage at this exact head exists and is on this thread: @wenshao's re-verification drove real daemons, the real CLI, and a case-insensitive ext4 mount across base vs head with nine negative controls — maintainer-produced evidence, attributed as such, not re-run here. The one behavioural surface no available lane can settle: the
中文说明代码审查在树内完整审查了
bot 第 7 轮 review 的两个 Critical 在本 head 上的状态
承接 @wenshao 重新验证的打磨项（他的发现，如实归属）CLI 把"session store 不可读"误报为 id 冲突——枚举本身失败时，"already exists" 的建议无法生效。在 CLI 分支复用 daemon 的 测试证据无人值守运行——此处不构建、不执行任何 PR 代码；证据为 API 一次性拉取的、 本 head 上的真实场景覆盖已存在于本线程：@wenshao 的重新验证以真实 daemon、真实 CLI 与大小写不敏感的 ext4 挂载对 base 与 head 做了 A/B，附九组反向对照——维护者产出的证据，如实归属，此处未重跑。唯一没有任何通道可以落定的行为面： （CI 明细见上方表格，finalize 任务会在 CI 落定后原地更新该表。） — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 3/5 — clean delta, decisive CI finally green on the fix commits, and both open Criticals are tracked decisions rather than unknowns; the cap is the standing Stage 0 policy for a fork PR with 1,570 production lines across core packages, not doubt about this head. Stepping back: this is what ten review rounds should look like when the discipline holds. The two fix commits close exactly what they name — the batch lifecycle deadlocking on case twins, the CLI still minting mixed-case transcripts — and the resolver unification removes the duplicated algorithm that was the root cause of the escape landing in one arm only. Every fix is pinned by tests running both directions. The deferrals are tracked in three real, open issues (#9488, #9489, #9490) instead of bloating a tenth-round diff, which is the right posture at this round count. And the evidence at this head is unusually strong: @wenshao re-ran the full real-daemon A/B with nine negative controls hours before this trigger, and @yiliang114 approved at exactly this commit. What keeps this from an approval is not any of that. It is, first, policy: the Stage 0 escalation — fork PR, 1,570 production lines across
No review is submitted this round: this run found no new blocking issues, so there is nothing new to request changes on, and approval is capped by policy. The bot's round-7 One CI note: ⏸️ Deferring to @wenshao and @yiliang114. What this PR needs from here: (1) the call on R7-1 — accept the #9488 deferral or require the fix in-PR; (2) the on-thread acceptance of the O1 trade-off for R2-3 (a release-note commitment suffices); (3) final sign-off — 中文说明置信度：3/5 —— delta 干净，决定性 CI 首次在修复 commit 上落绿，两个未决 Critical 都是被跟踪的裁断项而非未知项；封顶来自 Stage 0 对"fork PR、1,570 行生产改动横跨核心包"的既有策略，而非对本 head 的疑虑。 退一步看：在纪律保持的前提下，这正是第十轮 review 应有的形态。两个修复 commit 恰好关闭了它们点名的内容——批量生命周期在大小写孪生上死锁、CLI 仍在铸造 mixed-case transcript——resolver 统一则移除了让 occupancy 逃逸只落进单臂的根因（重复的算法）。每个修复都有双向测试钉住。延迟项被跟踪在三个真实且开放的 issue（#9488、#9489、#9490）中，而不是膨胀第十轮 diff——在这个轮次上这是正确姿态。本 head 上的证据也异常充分：@wenshao 在本次触发前数小时刚以九组反向对照重跑了完整的真实 daemon A/B，@yiliang114 恰在本 commit 上批准。 使它不能被批准的不是这些。其一是策略：Stage 0 升级——fork PR、横跨
本轮不提交 review：本轮未发现新的阻断项，无可要求修改之事；批准又被策略封顶。bot 第 7 轮的 一条 CI 说明： ⏸️ 移交 @wenshao 与 @yiliang114。本 PR 接下来需要：(1) 对 R7-1 的裁断——接受 #9488 延迟，或要求在本 PR 内修复；(2) 在线程内对 R2-3 的 O1 取舍的接受（承诺写入 release note 即可）；(3) 最终签核—— — Qwen Code · qwen3.8-max Reviewed at |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
The ubuntu Test job failed on ten PR2A cases that pass on macOS: - Export SessionIdCaseConflictError from the worktree test's core mock and give its SessionService double findSessionIdIgnoringCase, since loadSession now resolves persisted spelling before reading metadata. - Add readCreationMetadataIfReadable to the Live task fake and mirror it onto the three server lifecycle spies so the fail-closed store interface sees the same fixture metadata as the legacy tolerant readCreationMetadata path it replaced. - Pin the original inode via rename in the two same-path replacement cases. ext4/overlayfs recycle a freed inode immediately, so rm+mkdir at the same path could satisfy the recorded device+inode identity on Linux runners and make a real replacement look valid.
CI fix pushed (commit 9736211)The ubuntu
Also noted in the job log: an Verification (macOS, this branch)
Note for reviewers: the two same-path replacement assertions previously relied on inode numbers differencing after |
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 2113 passed · 0 failed · 2113 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2113 通过 · 0 失败 · 2113 总计 Verification reportPR 9341 Deep Verification (round 2) —
|
| # | finding (round 1) | severity | status at head 97362119bd |
|---|---|---|---|
| F1 | Interface migration left test stubs behind — 5 unit tests red on Linux (server.test.ts ×3 got 404; live-task-service.test.ts ×2 threw readCreationMetadataIfReadable is not a function) |
blocking | fixed. Re-measured at new head: server.test.ts + live-task-service.test.ts green inside the 5-file big gate (1786/1786, round 1: 1753/1758 with 5 red). Commit 4 added readCreationMetadataIfReadable to the three server lifecycle spies (verified at server.test.ts:31564/31803/31933) and to the live-task fake (live-task-service.test.ts:79). Witness 06-big-gate-green-formerly-f1.png. |
| F2 | Two new tests encoded a macOS-only inode assumption (rm+mkdir at the same path reuses the freed inode on Linux) — deterministic red on Linux |
blocking | fixed. Re-measured: both suites green in the small gate (61/61, round 1: 57/59 with 2 red). Both tests now pin the original inode via rename (verified conversation-directory-identity.test.ts:103, conversation-workspace.test.ts:312); harness 04's rename-over probe independently confirms the production guard rejects a real inode change on this Linux box. Witness 05-small-gate-green-formerly-f2.png. |
| F3 | Coverage gap: self-lineage guard survives the delivered session-source.test.ts for the explicit-standalone-child shape (mutant M3) |
suggestion | stands. Re-measured, not diffed: mutant M3 (guard → false) again leaves the delivered suite 19/19 green, while the recreated probe goes red with expected { kind: 'standalone', … } to be undefined. No test for this shape was added in commit 4. See Findings. Witness 08-mutation-m3-selflineage-survivor.png. |
Central claim + A/B
Central claim: provenance classification fails closed when a transcript's creation metadata is truncated/malformed/missing, while clean legacy transcripts and fully-recoverable }{-glued records stay loadable; tolerant readers elsewhere are unchanged.
Mock-free A/B (01-failclosed-ab.mjs): real SessionService from each arm's own dist build over real transcript files under a temp runtime dir. Base arm drives readLoadableLiveConversationMetadata with the pre-PR callback; head arm passes the service as the new store. Build fingerprints asserted on both arms (base dist lacks SessionIdCaseConflictError/readCreationMetadataIfReadable/readLinesWithIntegrity; head dist has all three — grep counts 0 vs 5/1/4), so neither cell could silently load the other's code. Witness: 01-failclosed-ab-base-vs-head.png.
| cell | base | head | verdict |
|---|---|---|---|
| clean-legacy (no metadata) | loadable | loadable | preserved |
explicit-live (default + realtime_voice:*) |
loadable | loadable | preserved |
| truncated creation record | loadable (promoted) | rejected | flipped — fixed |
| garbage bytes | loadable (promoted) | rejected | flipped — fixed |
scalar line (42) |
loadable (promoted) | rejected | flipped — fixed |
array line ([1,2,3]) |
loadable (promoted) | rejected | flipped — fixed |
}{-glued complete records |
loadable | loadable | preserved (recovery kept) |
| damage past 10-record window | loadable | loadable | preserved (bounded scan) |
| valid head + damage within window | loadable (promoted) | rejected | flipped — fixed |
| tolerant-vs-strict on same damaged file | — | tolerant {}, strict undefined |
tolerant reader unchanged |
Mechanism attribution (re-verified): the four corrupt-head cells are rejected at head's location gate (unreadable head record → getSessionLocation undefined) plus integrity; the valid-head/damage-within-window cell is rejected purely by the integrity complete flag — an attribution probe (logs/attribution-probe.txt) shows that cell resolves location=active while the strict read returns undefined. Harness self-caught a fixture bug on first run (non-hex session ids tripped SESSION_FILE_PATTERN and masked the mechanism); after fixing to hex-only ids all 15/15 assertions pass.
Secondary claims (all re-confirmed):
- Case lookup (
02-case-conflict-ab.mjs, 11/11, witness02-case-conflict-ab.png): unique mixed-case resolves the persisted spelling on both arms; case-only duplicates within active and same-spelling-across-active+archived throwSessionIdCaseConflictErrorat head (withgetSessionLocationreportingconflictfor the latter) while base returns a first match / the conflicted id; absent id →undefinedon both arms. - Reserved source + lineage (
03-reserved-source-ab.mjs, 18/18, witness03-reserved-source-ab.png): the hole cell — a transcript claimingsourceType:'standalone'+sourceId— passes base's generic-restore decision (raw tolerant read continues) and is rejected by head's new reserved-source check; explicit standalone top-level and explicit standalone child-without-parent are loadable at head only (new feature, live path still refuses both); legacy child of legacy parent loadable on both arms; self-lineage and cyclic lineage refused on both arms; the F3 shape (self-referencing explicit standalone child) refused at head. - Directory identity (
04-identity-probe.mjs, 12/12, witness04-identity-probe.png, head-only — the module is new): root created 0700 with dev+inode recorded; equivalent canonical root path accepted, foreign pathunexpected_identity; rename-over replacement (inode verified to change: recorded → new) rejectedunexpected_identity; symlink escape rejectednot_directory; group/world-readable rejectedwrong_modeand re-inspects clean afterchmod 0700; unknown id inspects asundefined.
Corrections
- Round 1 noted the PR body's claim "targeted final tests passed … the server/ACP Agent/Live-task suites" was not reproducible on Linux at head
e00ed9f9f6. At the new head97362119bdthose suites are green on Linux (big gate 1786/1786), so the PR body's statement is now accurate — this corrects round 1's note, not the code. - Round 1's big gate did not include
acpAgent.worktree.test.ts; commit 4's message reports the Ubuntu Test job failed ten PR2A cases (vs round 1's measured seven), the difference being worktree-test cases round 1 never ran. Round 1's red count was therefore a lower bound on CI failures; this round's gate includes the worktree file (green at head; it needed theSessionIdCaseConflictErrorexport in its core mock plus afindSessionIdIgnoringCasedouble — verified atacpAgent.worktree.test.ts:176/331).
Findings
F3 (carried over, suggestion) — self-lineage guard not pinned for the explicit-standalone-child shape
Mutant M3 (delete normalizeSessionIdForLookup(parentSessionId) === normalizeSessionIdForLookup(sessionId) in session-source.ts) survives the delivered session-source.test.ts (19/19 green under mutation): the delivered SELF_ID case has no sourceType, so it is rejected anyway by the parent-must-be-top-level rule and never exercises the explicit comparison. The guard is load-bearing for a self-referencing explicit standalone child ({sourceType:'standalone', parentSessionId: SELF}), which reaches the explicit-child early return before any parent read — with the guard deleted, that shape classifies {kind:'standalone', persistence:'explicit'} (probe failure: expected { kind: 'standalone', … } to be undefined). Classification unchanged from round 1: coverage gap, not dead code, not a production defect.
Suggested fix (probe fixture that kills the mutant; green at delivered head)
// in session-source.test.ts
const SELF_EXPLICIT = '550e8400-e29b-41d4-a716-44665544000f';
const explicitSelfStore: ConversationSessionMetadataStore = {
async getSessionLocation() {
return 'active';
},
async readCreationMetadataIfReadable() {
return { sourceType: 'standalone', parentSessionId: SELF_EXPLICIT };
},
};
// rejects a self-referencing explicit standalone child:
await expect(
readLoadableConversationSession(SELF_EXPLICIT, explicitSelfStore),
).resolves.toBeUndefined();
// control — non-self parent stays loadable:
// readCreationMetadataIfReadable returns parentSessionId of a DIFFERENT id
// -> result is { kind: 'standalone', persistence: 'explicit' }.Nit (new, optional): the repaired server.test.ts spies delegate readCreationMetadataIfReadable to the same tolerant readCreationMetadata stub, so the route-level tests cannot distinguish a regression that swaps the strict store method back for the tolerant one. The strict/tolerant split is pinned elsewhere (core corruption tests under M1, plus harness 01's tolerant-vs-strict assertion end to end), so this only narrows what the server suite would catch.
Mutation matrix (round 2)
| mutant | guard removed | pinning suite | result |
|---|---|---|---|
| M1 | integrity complete flag forced true |
core jsonl-utils + corruption | killed — 5 red: "reports incomplete recovery for a truncated record / trailing garbage / a non-object value / an invalid middle fragment" + "distinguishes clean legacy metadata from an unreadable transcript head" (witness 07-mutation-m1-integrity-disabled.png) |
| M2 | case-conflict throws → first-candidate returns | core sessionService | killed — 2 red: "rejects case-only duplicate spellings instead of choosing by enumeration order", "rejects one spelling that exists in both active and archive state" |
| M3 | self-lineage comparison → false |
cli session-source | survived delivered 19/19; killed by added probe → coverage gap F3 (witness 08-mutation-m3-selflineage-survivor.png) |
| M4 | TOCTOU location recheck removed | cli session-source | killed — 1 red: "rejects a transcript that disappears while its metadata is read" |
Positive controls: M1 and M2 each failed on exactly the intended assertions (quoted above; full logs logs/mutation-M*.txt); unmutated controls are the green gate rows below. Mutation runs are evidence for suite liveness and are not counted in assertions.json (same convention as round 1).
Targeted gates (head, as delivered)
| gate | result |
|---|---|
| core: jsonl-utils + sessionService + corruption | 209/209 pass |
| cli small: session-source, session-id-admission, error-response, directory-identity, conversation-workspace | 61/61 pass (round 1: 57/59) |
| cli big: server, acp-http transport, acpAgent, acpAgent.worktree, live-task-service | 1786/1786 pass (round 1: 1753/1758; worktree file now included) |
No repo-wide gate was claimed or run.
Not covered
- Per-commit attribution: checkout is depth 2 (merge, base tip, PR head only);
git rev-list HEAD^1..HEAD^2returns 1 commit while the metadata lists 4 — the shallow-boundary gap. The aggregateHEAD^1..HEADdiff was verified; the delta since round 1 (commit 4) was reconstructed by verifying each claimed repair is present in the delivered test files, since the previous heade00ed9f9f6is not locally reachable. - Windows:
isSameConversationPathwin32 branches and case-insensitive FS behavior not exercised (Linux container). - True multi-process daemon E2E: restore/admission verified at dist/component level plus the PR's own in-process server/transport suites; no live daemon over a real socket was booted.
- Performance / concurrency: no load or race testing of the scan-window reads;
MAX_PROMPT_SCAN_LINES=10bounds the read and the inputs are daemon-written transcripts, not outsider text, so no timing ladder was run. - Lint/format/typecheck repo-wide: CI builds head (typecheck) green per the environment contract; no separate lint run.
- Docs-only changes (
standalone-daemon-sessions.md,2026-08-14-standalone-pr2-core.md): reviewed for claim context, not behaviorally tested. - Mutation coverage beyond M1–M4: other guards the PR introduces (e.g.
getSessionLocation's pattern pre-filter, directory-identity owner/mode checks under a different uid) were not mutated.
Methodology
Environment: CI merge-ref checkout at 3cc1b7808c (base 18c9763f46 = HEAD^1, head 97362119bd = HEAD^2), node:22-bookworm-class container, Node v22.23.2; npm ci + npm run build pre-run at HEAD. The base moved since round 1 (a35a23cd68 → 18c9763f46), so both arms were re-measured from scratch rather than carried. A/B base side built in git worktree tmp/base-tree at HEAD^1, rebuilding only packages/core + packages/cli (~80 s); the base worktree got its own node_modules/@qwen-code/ with qwen-code-core/qwen-code symlinked into the base tree (realpath asserted: …/tmp/base-tree/packages/core) and all other entries pointing at head's, @lydell mirrored for the core tsconfig paths mapping, and head's package-level node_modules symlinked (neither contains @qwen-code, so no confound); the PR leaves package.json/package-lock.json untouched, making the shared root node_modules a clean control. Controls validated by build fingerprint on every harness run. Harnesses 01–04 drive compiled dist/ output with real files (no stubs of the unit under test). Mutation runs applied single-point source edits in-tree, ran the pinning vitest suite, then git checkout -- restored (final git status --porcelain empty; scratch probe test removed). Gates ran at delivered head before any mutation. Evidence PNGs rendered via scripts/verify-capture.mjs; raw per-run logs live in logs/. assertions.json counts map 1:1 to executed scripted checks: 209 + 61 + 1786 gate tests + 56 harness check() calls + 1 attribution probe = 2113, with 0 unexpected outcomes.
Re-running harnesses 01–03: they import BOTH builds; the base worktree was removed after capture. Rebuild it with: git worktree add tmp/base-tree <baseRefOid>; create tmp/base-tree/node_modules/@qwen-code/ with qwen-code-core/qwen-code → ../../packages/{core,cli} and every other entry → head's node_modules/@qwen-code/<name>; symlink head's node_modules/@lydell and head's packages/{core,cli}/node_modules into the base tree; then npm run generate && npm run build -w packages/core -w packages/cli inside the worktree. Harness 04 is head-only and reruns as-is.
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
Local verification — real
|
| scenario | base 5492009 |
head 9736211 |
|
|---|---|---|---|
| L0 | healthy Live transcript restores | 200 | 200 |
| S1 | explicit standalone, top-level | 404 session_not_found |
404 session_not_found |
| S2 | explicit standalone, child | 404 session_not_found |
404 session_not_found |
| S3 | legacy standalone parent (control) | 200 | 200 |
| C1 | truncated creation record | 200 | 404 session_not_found |
| C2 | garbage first line | 200 | 404 session_not_found |
| C3 | scalar JSONL line | 200 | 404 session_not_found |
| C4 | complete }{-glued creation record |
200 | 200 |
| G1 | clean legacy transcript, no metadata | 200 | 200 |
| P1 | orphan child, parent missing | 200 | 404 session_not_found |
| P2 | self-referential lineage | 404 session_not_found |
404 session_not_found |
| A1 | same id in active and archive | 409 session_conflict |
409 session_conflict |
| M1 | lowercase request, UPPERCASE on disk | 404 session_not_found |
200 |
| M2 | case-only duplicate spellings | 200 | 409 session_conflict |
| R1 | POST /session with sourceType: "standalone" |
200 — session created | 400 reserved_session_source |
| R2 | POST /session with sourceType: "webshell" |
200 | 200 |
All five claims in the reviewer test plan hold end to end, and every new guard is load-bearing — base does the wrong thing in all seven changed rows. Equally important, the nine unchanged rows show no regression: a healthy Live transcript, a clean metadata-free legacy transcript, a legacy standalone parent, and a fully recoverable }{-glued creation record all still restore.
M1 is the sharpest single piece of evidence. With only AAAAAAAA-7777-….jsonl on disk and a lowercase id in the request, head returned 200 and materialised conversation-<sha256(UPPERCASE id)> — it bound the persisted spelling, not the caller's. base returned 404 and created no directory at all.
2. The fail-closed rule stays inside the Conversations runtime
I planted the identical damaged transcript in an ordinary workspace project and in the Conversations project:
| base | head | |
|---|---|---|
ordinary workspace — GET …/transcript |
200 | 200 |
ordinary workspace — POST /session/:id/load |
200 | 200 |
Conversations — GET …/transcript |
200 | 404 |
Ordinary project sessions are untouched, exactly as the PR claims. Worth stating explicitly for the record: inside Conversations the change closes the reader route too, not only restore — a damaged Live transcript becomes unreadable through the daemon, not merely unrestorable.
3. Private-directory identity is behaviour-preserving
I materialised three private conversation directories through real restores, then tampered with them on disk and restored again:
| tamper | base | head |
|---|---|---|
| directory replaced by a symlink pointing outside the root | 500 (rejected) | 500 (rejected) |
directory made world-writable (0777) |
500 (rejected) | 500 (rejected) |
| directory deleted and recreated (same path, new inode) | 200 | 200 |
Identical on both arms. The new conversation-directory-identity.ts is an extraction with typed failure reasons for PR2B, not a behaviour change at the daemon boundary — the right shape for a PR that says it publishes no new surface.
4. Test suites — including the platform the bot flagged
The earlier sandboxed run reported 7 red cases on the Linux leg, and 97362119b was pushed to fix them. I re-ran the 13 touched test files on both platforms, and also on the pre-fix commit, to confirm the fix is what closed them:
| tree | platform | result |
|---|---|---|
head 9736211 |
macOS 26 · node 24.18.1 | 2015 passed · 0 failed |
head 9736211 |
Linux · node 22.23.2 · uid 1000 | 2015 passed · 0 failed |
pre-fix e00ed9f |
Linux · node 22.23.2 · uid 1000 | 10 failed · 2005 passed |
The 10 red cases I reproduce at e00ed9f are exactly the set 97362119b claims to fix (acpAgent.worktree VP1/VP2/VP2b, live-task-service ×2, server.test ×3, conversation-directory-identity, conversation-workspace) — my count matches the author's fix note rather than the bot's 7, which is presumably a difference in how that harness aggregates. The CI regression is genuinely closed.
Observations — none blocking
O1 · The fail-closed rule is wider than "creation metadata", and it is permanent.
readCreationMetadataIfReadable requires every one of the first MAX_PROMPT_SCAN_LINES (10) records to be fully recoverable, not just the records that carry creation metadata. Measured on a transcript of 1 creation record + 24 turns with a single truncated line:
| torn line | base | head |
|---|---|---|
| record #3 | 200 | 404 |
| record #6 | 200 | 404 |
| record #10 | 200 | 404 |
| record #11 | 200 | 200 |
| record #21 | 200 | 200 |
A tear at record #10 is as fatal as a tear at record #1, and nothing ever rewrites those lines, so the conversation is permanently unrestorable. The PR body describes this as "truncated creation metadata", which understates the blast radius. If you want the safety without the extra reach, the integrity requirement could stop at the last record that actually contributed a session_source / parent_session payload rather than covering the whole 10-line scan window. Your call — the current behaviour is a defensible reading of "fail closed", it is just broader than the PR text suggests.
O2 · Restore is now O(sessions on disk).
head replaces the exact-match stat with findSessionIdIgnoringCase, which readdir()s both the active and the archive chats directories on every restore. Median POST /session/:id/load round trip:
| sessions on disk | base | head | delta |
|---|---|---|---|
| 50 | 5.90 ms | 6.42 ms | +0.5 ms |
| 5,000 | 5.83 ms | 10.93 ms | +5.1 ms |
| 20,000 | 6.23 ms | 22.63 ms | +16.4 ms |
base is flat; head grows linearly. The absolute cost is small, but it is paid on every restore, and the call site in acpAgent.ts is explicitly wrapped in profiler.time('existence_check', …) — someone considered it latency-sensitive. Worth keeping in mind for PR2B rather than changing here.
O3 · Minor: the new sanitizes standalone child filesystem errors test is root-fragile.
It chmods a directory to 0o000 and expects an io_error; root bypasses that, so the case fails when the suite runs as root (I hit it before switching my container to uid 1000). GitHub Actions runs non-root so CI is unaffected, and the repo already has unguarded 0o000 tests elsewhere — but the guarded idiom (it.skipIf(process.platform === 'win32' || process.getuid?.() === 0), e.g. cost-ledger.test.ts) exists if you'd like this one to survive container-based runs.
O4 · Minor: a redundant parent read.
For a legacy standalone child, readLoadableConversationSession reads and classifies the parent, and then readLoadableLiveConversationMetadata reads and classifies it again — three extra filesystem ops (getSessionLocation ×2 plus one metadata read) per restore. The second read is also a small TOCTOU window, though it fails closed either way. Passing the already-classified parent through would remove both.
What I did not verify
- The
session/newreserved-source guard on the ACP transport: my WebSocket probe never completed the/acpupgrade (the request landed on the SSE handler and returned 406), so I have no live evidence there. That path is covered by this PR's owntransport.test.tsadditions, which pass on both platforms. - Windows.
- Anything in PR2B — no standalone routes, capabilities, SDK types or UI exist yet to exercise.
Verdict
From my side this is good to merge. The primitives do what the PR says they do, every new guard is load-bearing against the base build, nothing regresses on healthy or legacy transcripts, the containment discipline holds, and the Linux CI regression is genuinely fixed. O1 is the one thing I would want accepted consciously rather than by default, since it turns a torn line anywhere in a conversation's first ten records into permanent loss of access to that conversation.
中文版
本地验证 —— 真实 qwen serve daemon，base vs head
我在本地把 head 97362119bd 与 base 5492009bb2（与 main 的 merge-base）做了对照验证。两棵树分别在独立 worktree 里从源码构建（npm install && npm run build && npm run bundle），然后分别驱动各自的 dist/cli.js serve。下面每一条结果都来自一个真实运行中的 daemon 进程返回的 HTTP 状态码/响应体，读的是磁盘上真实的 chats/<sessionId>.jsonl —— 没有 vitest 替身，没有打桩的 SessionService，没有 mock 文件系统。
要让这些关键路径真正可达，需要两个前提：
- 这些分类代码只在内部 Conversations runtime 上运行，所以 harness 通过
POST /live/start把它发布出来，再用cwd = ~/Documents/Qwen Code/Conversations定位会话。一条健康的 Live transcript 能以200恢复并物化出私有的conversation-<sha256>目录 —— 这一点成立，后面的矩阵才有意义。 - macOS 默认卷上造不出仅大小写不同的重名文件，所以 daemon 的
HOME放在为本次验证创建的大小写敏感 APFS 卷上，用来复现 CI runner 上 ext4 的行为。
1. 行为矩阵 —— 16 个场景，7 处变化
| 场景 | base 5492009 |
head 9736211 |
|
|---|---|---|---|
| L0 | 健康 Live transcript 可恢复 | 200 | 200 |
| S1 | explicit standalone，顶层 | 404 session_not_found |
404 session_not_found |
| S2 | explicit standalone，子会话 | 404 session_not_found |
404 session_not_found |
| S3 | legacy standalone 父会话（对照） | 200 | 200 |
| C1 | 被截断的 creation record | 200 | 404 session_not_found |
| C2 | 首行是垃圾数据 | 200 | 404 session_not_found |
| C3 | JSONL 行是标量 | 200 | 404 session_not_found |
| C4 | 完整可恢复的 }{ 粘连记录 |
200 | 200 |
| G1 | 干净的 legacy transcript，无 metadata | 200 | 200 |
| P1 | 孤儿子会话，父会话不存在 | 200 | 404 session_not_found |
| P2 | 自引用 lineage | 404 session_not_found |
404 session_not_found |
| A1 | 同一 id 同时在 active 和 archive | 409 session_conflict |
409 session_conflict |
| M1 | 请求用小写 id，磁盘上是大写 | 404 session_not_found |
200 |
| M2 | 仅大小写不同的重名 transcript | 200 | 409 session_conflict |
| R1 | POST /session 带 sourceType: "standalone" |
200 —— 会话被创建 | 400 reserved_session_source |
| R2 | POST /session 带 sourceType: "webshell" |
200 | 200 |
Reviewer 测试计划里的五条主张端到端都成立，而且每个新加的闸门都是有效的 —— 在全部 7 处变化的行上，base 的行为都是错的。同样重要的是，另外 9 行没有变化，说明没有回归：健康的 Live transcript、干净无 metadata 的 legacy transcript、legacy standalone 父会话、以及完整可恢复的 }{ 粘连 creation record，都仍然能正常恢复。
M1 是单条最有力的证据。 磁盘上只有 AAAAAAAA-7777-….jsonl，请求用小写 id，head 返回 200，并且物化出的目录是 conversation-<sha256(大写 id)> —— 它绑定的是持久化的拼写，而不是调用方给的拼写。base 返回 404，什么目录都没建。
2. fail-closed 规则被限制在 Conversations runtime 内部
我把完全相同的损坏 transcript 分别放进一个普通 workspace 项目和 Conversations 项目：
| base | head | |
|---|---|---|
普通 workspace —— GET …/transcript |
200 | 200 |
普通 workspace —— POST /session/:id/load |
200 | 200 |
Conversations —— GET …/transcript |
200 | 404 |
普通项目会话完全不受影响，与 PR 的说法一致。有一点需要明确记录：在 Conversations 内部，这个改动同时关闭了读取路由，而不只是 restore —— 一条损坏的 Live transcript 通过 daemon 变得不可读，而不仅仅是不可恢复。
3. 私有目录 identity 的行为保持不变
我通过真实 restore 物化出三个私有会话目录，然后在磁盘上做手脚，再次 restore：
| 篡改方式 | base | head |
|---|---|---|
| 目录被替换成指向根目录之外的软链 | 500（拒绝） | 500（拒绝） |
目录被改成 world-writable（0777） |
500（拒绝） | 500（拒绝） |
| 目录被删除后重建（同路径、新 inode） | 200 | 200 |
两侧完全一致。新增的 conversation-directory-identity.ts 是为 PR2B 抽取出的、带类型化失败原因的模块，在 daemon 边界上并没有改变行为 —— 对一个声称不发布任何新表面的 PR 来说，这正是应有的形状。
4. 测试套件 —— 包括 bot 标红的那条平台腿
之前的沙箱运行在 Linux 腿上报了 7 个红用例，随后推了 97362119b 来修。我在两个平台上重跑了这 13 个被改动的测试文件，并且在修复前的提交上也跑了一遍，用来确认确实是这个提交关掉了它们：
| 树 | 平台 | 结果 |
|---|---|---|
head 9736211 |
macOS 26 · node 24.18.1 | 2015 通过 · 0 失败 |
head 9736211 |
Linux · node 22.23.2 · uid 1000 | 2015 通过 · 0 失败 |
修复前 e00ed9f |
Linux · node 22.23.2 · uid 1000 | 10 失败 · 2005 通过 |
我在 e00ed9f 上复现出的这 10 个失败用例，正好就是 97362119b 声称修复的那一组（acpAgent.worktree VP1/VP2/VP2b、live-task-service ×2、server.test ×3、conversation-directory-identity、conversation-workspace）—— 我的计数与作者的修复说明一致，而不是 bot 的 7，差异应该出在那套 harness 的聚合方式上。CI 回归确实已经修好了。
观察项 —— 都不阻塞合并
O1 · fail-closed 的影响面比「creation metadata」更宽，而且是永久的。
readCreationMetadataIfReadable 要求前 MAX_PROMPT_SCAN_LINES（10）条记录每一条都完整可恢复，而不只是携带 creation metadata 的那几条。在「1 条 creation record + 24 轮对话、只截断其中一行」的 transcript 上实测：
| 被截断的行 | base | head |
|---|---|---|
| 第 3 条记录 | 200 | 404 |
| 第 6 条记录 | 200 | 404 |
| 第 10 条记录 | 200 | 404 |
| 第 11 条记录 | 200 | 200 |
| 第 21 条记录 | 200 | 200 |
第 10 条被截断和第 1 条被截断一样致命，而且这些行永远不会被重写，所以这个会话就永久不可恢复了。PR 正文把它描述为「truncated creation metadata」，低估了影响面。如果希望保留这份安全性但收窄影响，可以把完整性要求只覆盖到「最后一条真正贡献了 session_source / parent_session 载荷的记录」，而不是整个 10 行扫描窗口。这个由你们定 —— 现在的行为是对「fail closed」的一种合理解读，只是比 PR 文字所说的更宽。
O2 · restore 现在是 O(磁盘上会话数)。
head 把精确匹配的 stat 换成了 findSessionIdIgnoringCase，后者在每次 restore 时都会 readdir() active 与 archive 两个 chats 目录。POST /session/:id/load 往返中位数：
| 磁盘上的会话数 | base | head | 增量 |
|---|---|---|---|
| 50 | 5.90 ms | 6.42 ms | +0.5 ms |
| 5,000 | 5.83 ms | 10.93 ms | +5.1 ms |
| 20,000 | 6.23 ms | 22.63 ms | +16.4 ms |
base 是平的，head 线性增长。绝对开销不大，但它落在每一次 restore 上；而且 acpAgent.ts 里的调用点明确包在 profiler.time('existence_check', …) 里 —— 说明有人认为这条路径对延迟敏感。建议在 PR2B 里留意，而不是在本 PR 改。
O3 · 小问题：新增的 sanitizes standalone child filesystem errors 用例对 root 敏感。
它把目录 chmod 成 0o000 并期望得到 io_error；root 会绕过权限检查，所以以 root 跑套件时这个用例会失败（我在把容器切到 uid 1000 之前就撞到了）。GitHub Actions 以非 root 运行，因此 CI 不受影响；仓库里也已有其它未加守卫的 0o000 用例。但如果希望它在基于容器的运行环境里也稳，仓库已有现成写法可用（it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)，例如 cost-ledger.test.ts）。
O4 · 小问题：父会话被重复读取。
对 legacy standalone 子会话，readLoadableConversationSession 会读取并分类父会话，随后 readLoadableLiveConversationMetadata 又读取并分类了一次 —— 每次 restore 多出 3 次文件系统操作（getSessionLocation ×2 加一次 metadata 读）。第二次读取也带来一个很小的 TOCTOU 窗口，尽管两种情况下都是 fail closed。把已分类的父会话结果传下去就能同时消除这两点。
我没有验证的部分
- ACP 传输上的
session/new保留 source 闸门：我的 WebSocket 探针始终没能完成/acp升级（请求落到了 SSE 处理器并返回 406），所以这条路径我没有实测证据。它由本 PR 自带的transport.test.ts覆盖，且在两个平台上都通过。 - Windows。
- PR2B 的任何内容 —— 目前还没有 standalone 路由、capability、SDK 类型或 UI 可供驱动。
结论
从我这边看，可以合并。这些 primitives 确实做到了 PR 所声称的事情，每个新闸门相对 base 构建都是有效的，健康与 legacy transcript 都没有回归，containment 纪律成立，Linux CI 回归也确实修好了。O1 是唯一一件我希望被有意识地接受、而不是默认接受的事情：它会把「一条会话前 10 条记录里任意一行被截断」变成对该会话访问权限的永久丢失。
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Approved after a full pass over the diff at 97362119 plus spot-checks against the head tree.
What I verified:
- Scope matches the PR2A claim: classification / identity / admission primitives only. No new routes, capabilities, SDK, or UI surface (no new
app.<verb>registrations; the only additions are reserved-source guards). - Source classification (
session-source.ts): explicit standalone requires the reservedstandalonesourceType withsourceId === undefined; lineage rejects self-parents vianormalizeSessionIdForLookupcomparison and requires the parent to classify as a valid top-level session; the location is re-checked after every metadata read so an archive race yieldsundefined, not a stale classification. - Fail-closed reads:
readCreationMetadataIfReadablegoes throughreadLinesWithIntegrity; truncated/malformed heads and ENOENT returnundefined, while the tolerantreadLines/parseLineTolerantbehavior is preserved untouched for ordinary readers (glued}{records still recover when complete). - Case handling:
findSessionIdIgnoringCasenow scans both active and archived storage before deciding and throws the typedSessionIdCaseConflictErroron case-only duplicates instead of taking an exact-match shortcut. It maps consistently to REST 409session_conflict, RPCerrorKind: session_conflict, and the ACP agentsession_id_conflict. Restore paths use the persisted spelling end-to-end (metadata read,materialize, worktree sidecar), andrequireSessionIdboundary normalization keeps lock keys and resolution consistent for UUID-shaped ids. - Directory identity: lstat/realpath with dev+inode pinning, owner/mode checks, and root revalidation before and after child inspection. The Live path maps typed errors back to the pre-PR messages, so existing behavior is preserved; the standalone prepare/inspect/ensure helpers fail closed on compromised, replaced, or non-empty directories.
- Guard parity: reserved-standalone creation is rejected on both creation surfaces (REST 400
reserved_session_source, ACP INVALID_PARAMS), and generic restore rejects reserved-standalone metadata with 404 on both REST and ACP. The two remainingparseSessionSourcecall sites are session-list filters, not creation surfaces. SessionIdCaseConflictErrorreaches the CLI via the core barrel's existingexport * from './services/sessionService.js'.
CI on head 97362119: Test (ubuntu), Serve A/B, web-shell E2E smoke, Desktop Shell, secret scan, and dependency audit all pass; the round-2 sandboxed verification reports 2113/2113 assertions.
One non-blocking observation: the restore path's archiveCoordinator lock is keyed on the requested sessionId while the file operations use the resolved persisted spelling. Boundary lowercasing makes this a no-op for UUID-shaped caller ids today; worth keeping in mind when PR2B adds more lifecycle entry points.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not explored to full depth (tool budget reached): "agent reverse-audit (round 4)": verify skipGeminiInitialization exists on ConfigParameters and that Config.relocateWorkingDirectory() actually refreshes memory/file-discovery/file-history/…; "agent reverse-audit (round 4)": confirm existence of #bindGoalRuntime / #restoreWorktreeOnResume / #restoreBackgroundAgentsOnResume / NativeLspService / createAndStoreSession / hydrateSession…; "agent reverse-audit (round 2)": trace the bridge restoreSession spawn path in packages/acp-bridge/src/bridge.ts end-to-end to confirm the Session's transcript read/write is keyed by the br…; "agent reverse-audit (round 5)": did not read the acpAgent.ts sessionCd handler to confirm the "current no-op return" same-path premise behind the repair bullet.; "agent reverse-audit (round 5)": did not individually verify ensureAuthenticated/setupFileSystem/startNonInteractiveOpenAILogHousekeeping/createAndStoreSession/hydrateSessionRestoreFileHistory/…, and 5 more.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
未探索到全部深度（达到工具调用预算）："agent reverse-audit (round 4)"：verify skipGeminiInitialization exists on ConfigParameters and that Config.relocateWorkingDirectory() actually refreshes memory/file-discovery/file-history/…；"agent reverse-audit (round 4)"：confirm existence of #bindGoalRuntime / #restoreWorktreeOnResume / #restoreBackgroundAgentsOnResume / NativeLspService / createAndStoreSession / hydrateSession…；"agent reverse-audit (round 2)"：trace the bridge restoreSession spawn path in packages/acp-bridge/src/bridge.ts end-to-end to confirm the Session's transcript read/write is keyed by the br…；"agent reverse-audit (round 5)"：did not read the acpAgent.ts sessionCd handler to confirm the "current no-op return" same-path premise behind the repair bullet.；"agent reverse-audit (round 5)"：did not individually verify ensureAuthenticated/setupFileSystem/startNonInteractiveOpenAILogHousekeeping/createAndStoreSession/hydrateSessionRestoreFileHistory/…，另有 5 条。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
— qwen3.8-max via Qwen Code /review (v0.21.13)
The restore handlers resolved the persisted (possibly uppercase) spelling of a session id only inside the shared coordinator guard, while batch delete locks its exclusive guard on the raw caller ids. A restore of the normalized request id therefore raced a concurrent batch delete of the persisted-spelled id on case-sensitive volumes. Resolve the persisted spelling before acquiring the shared guard and key runSharedMany on the resolved id so both sides contend on the same key, in both the REST and ACP restore handlers. Regression tests assert the guard key at both transports.
…I create Batch delete/archive/unarchive locked on canonical keys but still deduped raw spellings, so two case variants of one id deadlocked the batch. CLI --session-id now stores the lowercase spelling so new mixed-case transcripts stop accumulating. Co-authored-by: Cursor <cursoragent@cursor.com>
Review-comment round (e4e9ef5)Landed the two Critical items this PR introduced and should own. Everything else stays deferred per the five-round rule.
9488 is the first follow-up after merge: mixed-case restore now succeeds, so the sibling-endpoint gap is newly reachable. |
CI Test checks out PR HEAD, then runs check-workflow-size.sh added on main in QwenLM#9517. Without this merge the script is missing and the job exits 127.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Code review is clean: all 50 review threads across rounds R1-R4 are resolved (Critical + Suggestion), no merge conflict. Approving the code.
Heads-up on CI (not code-related):
Live Host (macos-latest)failed atbun installwithGET api.github.com/repos/whiskeysockets/eslint-config/tarball/299e838 - 504(transient network).Test (ubuntu-latest)failed at theCheck workflow file sizestep with.github/scripts/check-workflow-size.sh: No such file or directory— that script exists onmainbut not on this branch, so the branch is behind main.
Please sync with main (merge/rebase) so the missing script is picked up and CI can actually run the test suite, then re-run the failed checks.
Re-verification at
|
| Check | Result |
|---|---|
| Touched core suites (3 files) | 223 passed / 0 failed |
Touched CLI suites + all of src/serve/conversations (17 files) |
2,489 passed, 1 skipped / 0 failed |
packages/core src/services + jsonl-utils |
2,398 passed, 1 failed |
packages/cli src/serve (full) |
5,291 passed, 6 failed |
eslint --max-warnings 0 on all 31 touched TypeScript files |
clean |
tsc --noEmit for packages/core and packages/cli |
clean |
The 7 failures reproduce identically on the new base worktree (re-confirmed this round: 6 in packages/cli/src/serve, 1 in session-writer-lease) — root-user environment artifacts, not regressions.
Findings
Still no blocking defect. Status of my three earlier findings, plus two new notes:
1. (unchanged, still the release-note item) The fail-closed window is the first 10 records. Re-measured at this head: a Live transcript torn at line 5 is not loadable through the internal path, the same tear at line 13 is. Blast radius is unchanged — conversations that crashed within their first ten records.
2. (unchanged) Agent-suffixed ids are never classified, because getSessionLocation() enforces SESSION_FILE_PATTERN. Every REST/ACP restore path still pre-gates on getSessionLocation/assertSessionLoadable, so it stays latent; LiveTaskService.ensureResident() remains the only caller without that pre-gate.
3. (unchanged) A transcript present in both active and archive is terminal — unclassifiable and 409 from the resolver, with no repair affordance.
4. (new, small) The CLI misattributes an unreadable session store to an id collision. ceaf5a9476 makes loadCliConfig treat any resolver failure as "occupied", which is the right default. But the user-facing line is then Error: Session Id <id> already exists (active or archived). Delete or unarchive it first. — advice that cannot help, because the id does not exist. Probe: replace <project>/chats/archive with a regular file, so enumeration fails with ENOTDIR; base starts the session, the PR prints the "already exists" error and exits. Worth noting that the daemon already gets this right — POST /session answers 503 session_id_admission_unavailable ("Unable to verify persisted state…") on both arms. Reusing that wording in the CLI branch would cost one if.
5. (new, informational) An explicit standalone child is accepted whenever its parent cannot be produced — not only when the parent was archived or deleted. Measured: a parent whose transcript is torn, and a parent sitting in an active+archive conflict, both read as "parent gone" and the child is classified standalone/explicit. That follows from the self-describing rule and is arguably correct, but the code comment and the PR description both frame the exemption as "archived away or deleted"; a torn or ambiguous parent is a different situation and may deserve a word in the comment.
One coverage gap worth stating plainly: the new inodeVerifiable fallback (FAT/exFAT and SMB mounts where stat().ino === 0) could not be exercised end-to-end here. No filesystem available on this Linux host reports inode 0 — the kernel's FAT drivers synthesise inode numbers, and there is no FAT tooling installed — so I verified hasVerifiableInode() as a pure function (0 and 0n → false, real inodes → true) and read the call sites. That degraded path rests on the PR's unit tests alone.
Cost check, re-measured with 5,001 sibling transcripts: findSessionIdIgnoringCase() came out at 2.8 ms on the PR and 4.4 ms on base this run, against 4.4/2.9 the other way round last time — the two arms are within noise of each other.
Verdict
The review round did real work: three lineage shapes that were silently accepted are now rejected, the private directory and the batch lifecycle both collapse case variants, and the resolver's two duplicated arms are now one. Everything reproduces on real daemons, a real CLI and a real case-insensitive filesystem, every fix is discriminated by a negative control, and the 7,699 surrounding tests show no regression. Still recommending merge, with finding 1 in the release notes and findings 4 and 5 as optional polish.
中文说明
在 62d5c5a1 上的重新验证 —— 距上次报告新增 8 个提交
我把整套环境针对更新后的分支重跑了一遍：base = 新的 merge-base 3b3818db 对比 PR head = 62d5c5a1，两臂都从源码重新构建出可运行的 dist/cli.js，大小写不敏感的用例仍跑在 loop 挂载的 +casefold ext4 上。凡是与上次相比发生变化的行为，我额外测量了上一个 head 04ac635d，以便把差异归因到这一轮评审，而不是归因到从 main 的 merge。
上次报告的结论在新 head 上全部成立，本轮新增的四项行为也都在真实进程上复现。
1. 新 head 上依然成立
保留的 standalone source 在 REST（400 reserved_session_source）与 ACP（-32602）上都被拒绝；与崩溃截断 transcript 发生大小写碰撞的 caller-supplied id 仍被 409 session_id_conflict 拒绝；17 个 provenance 场景仍然按原样 fail closed；大小写解析器仍抛出带类型的冲突；由 base daemon 写下的、闸门之前的 standalone source transcript，仍可被 PR daemon resume 并出现在列表中。对照组（sourceType: "myapp"、Live 保留 source、未占用的 caller id）在两臂上均无变化。（见上方第 1 张截图）
2. explicit standalone 的 lineage 现在需要被证明，而不是被假定
readLoadableConversationSession() 会读取 parent 并报告它所证明的分类。上一个 head 接受的三种形态现在被拒绝；而"parent 已消失"这一自描述场景仍被有意接受。（见 lineage 截图）
3. 批量生命周期、私有目录与 CLI id 现在都会折叠大小写变体
三项新的真实进程结果，上次报告中均未涉及：
- 用大写拼写依次调用
POST /sessions/archive→/sessions/unarchive→/sessions/delete（磁盘上是小写 transcript）：base 三次都返回notFound且文件原封不动；PR 依次归档、取消归档，并最终删除了持久化拼写对应的文件。 - 对同一个 id 的两种拼写调用
materializeConversationDirectory()：base 在 Conversations 根下创建两个私有目录，PR 只创建一个。 - 对全新 id 执行
qwen --session-id <大写>：base 写出<大写>.jsonl，PR 写出<小写>.jsonl，后续任何大小写变体都会解析到同一个 transcript。
（见 CLI 与 ACP/生命周期/迁移两张截图）
4. 分类与大小写解析矩阵，已重新测量
本轮新增两行，都印证了 49dec69a3b 的 occupancy 修复：当请求拼写与大小写孪生文件同时存在且都不可读时，该 id 被判为已占用（unreadable_transcript）；当请求拼写不可读而孪生可读时，解析器返回可读的那个拼写。（见分类与大小写解析两张截图）
5. 反向对照，已重跑并扩充
现在共 9 组，其中两组针对本轮新修复。每一组都能让证据翻转回去。（见反向对照截图）
6. 62d5c5a1 上的测试、lint、typecheck
| 检查项 | 结果 |
|---|---|
| 触及的 core 套件（3 个文件） | 223 通过 / 0 失败 |
触及的 CLI 套件 + 整个 src/serve/conversations（17 个文件） |
2,489 通过、1 跳过 / 0 失败 |
packages/core 的 src/services + jsonl-utils |
2,398 通过，1 失败 |
packages/cli 的 src/serve（全量） |
5,291 通过，6 失败 |
对 31 个触及的 TypeScript 文件执行 eslint --max-warnings 0 |
干净 |
packages/core 与 packages/cli 的 tsc --noEmit |
干净 |
这 7 个失败在新的 base worktree 上完全一致地复现（本轮已重新确认：packages/cli/src/serve 6 个，session-writer-lease 1 个）—— 属于 root 用户环境造成的，并非回归。
结论性发现
仍无阻塞性缺陷。此前三点发现的状态，外加两点新说明：
1.（不变，仍建议写进 release note）fail-closed 的窗口是前 10 条记录。 在新 head 上重测：第 5 行被截断的 Live transcript 在内部路径下不可加载，同样的截断发生在第 13 行则可加载。影响面不变 —— 在前十条记录内崩溃的会话。
2.（不变）agent 后缀 id 永远不会被分类，因为 getSessionLocation() 强制 SESSION_FILE_PATTERN。所有 REST/ACP restore 路径仍先经过 getSessionLocation/assertSessionLoadable，因此这仍是潜在问题；LiveTaskService.ensureResident() 仍是唯一没有该前置检查的调用点。
3.（不变）同时存在于 active 与 archive 的 transcript 是终局状态 —— 无法分类，解析器只回 409，且没有修复入口。
4.（新增，轻微）CLI 把"session store 不可读"误报成"id 已被占用"。 ceaf5a9476 让 loadCliConfig 把解析器的任何失败都当作"已占用"，这个默认是对的。但用户看到的那一行是 Error: Session Id <id> already exists (active or archived). Delete or unarchive it first. —— 这条建议无法生效，因为该 id 根本不存在。探针：把 <project>/chats/archive 换成普通文件，使枚举以 ENOTDIR 失败；base 正常启动会话，PR 打印"already exists"并退出。值得一提的是 daemon 侧已经处理得很好 —— POST /session 在两臂上都返回 503 session_id_admission_unavailable（"Unable to verify persisted state…"）。在 CLI 分支复用这套措辞只需要一个 if。
5.（新增，说明性）只要 parent 无法被产出，explicit standalone child 就会被接受 —— 不限于 parent 被归档或删除的情形。实测：parent 的 transcript 被截断，以及 parent 处于 active+archive 冲突态，两种情况都被读成"parent 已消失"，child 仍被分类为 standalone/explicit。这是自描述规则的自然结果，也说得通；但代码注释与 PR 描述都把这个豁免表述为"archived away or deleted"，而"被截断"或"处于歧义态"的 parent 是另一回事，注释里或许值得补一句。
另有一处覆盖缺口需要如实说明：新增的 inodeVerifiable 兜底路径（FAT/exFAT 与部分 SMB 挂载，stat().ino === 0）在这里无法端到端复现。本机没有任何文件系统会报告 inode 0 —— 内核的 FAT 驱动会合成 inode 号，而且机器上没有 FAT 工具链 —— 因此我只验证了 hasVerifiableInode() 作为纯函数的行为（0 与 0n → false，真实 inode → true）并通读了调用点。该降级路径目前仅由本 PR 自带的单测覆盖。
开销复测（单 project 下 5,001 个同级 transcript）：本轮 findSessionIdIgnoringCase() 在 PR 上 2.8 ms、base 上 4.4 ms，而上一轮是 4.4/2.9 反过来 —— 两臂在噪声范围内没有差别。
结论
这一轮评审确实解决了实际问题：三种此前被静默接受的 lineage 形态现在被拒绝；私有目录与批量生命周期都会折叠大小写变体；解析器原本重复的两条分支合并为一条。所有行为都在真实 daemon、真实 CLI 与真实大小写不敏感文件系统上复现，每项修复都有反向对照加以判别，周边 7,699 条测试没有回归。仍然建议合并，把第 1 条发现写入 release note，第 4、5 条作为可选打磨。
Verified locally with Claude Code (model: Claude Opus 5, 1M context).
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 2823 passed · 0 failed · 2823 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2823 通过 · 0 失败 · 2823 总计 抖动门： Verification reportPR 9341 Deep Verification (round 8) —
|
| # | finding (round 7) | severity | status at new head |
|---|---|---|---|
| F1 | Interface migration left test stubs behind | blocking | fixed (holds). Big gate re-run green at the new head/base merged tree: 2323/2323 across 8 files, two agreeing rounds (logs/gate-cli-big.txt, gate-cli-big-r2.txt). |
| F2 | macOS-only inode assumption in replacement tests | blocking | fixed (holds). Harness 04's replacement cell rename-pins the child directory and measures a real inode change on Linux (ino 7866215 -> 7866218) → unexpected_identity against the expected identity, then restores and revalidates clean (04-identity-probe.png). |
| F3 | Self-lineage conjunct unpinned (mutant M3) | suggestion | stands (re-measured). M3 (conjunct → false) again survives the delivered session-source.test.ts 35/35; the tsx probe kills it: a case-variant self-parent is REJECTED at head but classifies {standalone/explicit} under M3 (parent read misses on a case-sensitive FS, self-describing branch admits). Coverage gap; code correct. |
| F4 | Cleanup-guard normalization unpinned (mutant M6) | suggestion | stands (re-measured). M6 (three cleanup-guard reads at acpAgent.ts:5184/5594/5762 → raw config.getSessionId()) survives the full acpAgent.test.ts 441/441 (logs/m6.txt). Positive control M8 (load-path adoption removed) killed on exactly expected '550e8400-…' to be '550E8400-…' with the resume variant green (logs/m8.txt) — the suite is live on the spelling axis; the survival is specific to the cleanup-guard axis. Coverage gap, not a production defect. |
| F5 | Entries-after-inspect ordering unpinned | suggestion | fixed (holds). M9 (entries read before the final identity re-inspection) killed by exactly rejects as not_empty when an entry appears during the final identity re-inspection — 1 failed | 14 passed (logs/m9.txt). |
| F6 | Third case-spelling race on the coordinator | suggestion | fixed (holds). Harness 05 cells A–D: base proceeds / head throws SessionArchivingError on every case-variant race; M11 (all three canonicalization sites reverted) killed by the case-fold test — 1 failed | 46 passed (logs/m11.txt). |
| nit | strict-reader spies delegate to the tolerant stub | optional | stands. Re-read at the new head: three sites — server.test.ts:32985, :33225, :33355 — all .mockImplementation(async (…) => readMetadata(…)/readCreationMetadata(…)), so route-level tests still cannot distinguish a regression swapping readCreationMetadataIfReadable back for the tolerant method. The strict/tolerant split remains pinned by harness 01's tolerant-vs-strict cell + the core gate. |
Central claim + A/B
Central claim (unchanged): provenance classification fails closed when a transcript's creation metadata is truncated/malformed/missing, while clean legacy transcripts and fully-recoverable }{-glued records stay loadable; tolerant readers elsewhere are unchanged.
Mock-free A/B (harness/01-failclosed-ab.mjs, 29/29, witness 01-failclosed-ab-base-vs-head.png): real SessionService from each arm's own dist build over real transcript files under a per-arm hermetic runtimeBaseDir. Base arm drives the pre-PR tolerant callback wiring; head arm passes the service as the strict store. Build fingerprints asserted in-harness on both arms (base dist lacks readCreationMetadataIfReadable / SessionIdCaseConflictError / readLoadableConversationSession; head has all three).
| cell | base | head | verdict |
|---|---|---|---|
| clean-legacy (no metadata) | loadable | loadable (standalone/legacy) | preserved |
explicit-live (default + realtime_voice:bridge-1) |
loadable | loadable (live/explicit) | preserved |
| truncated creation record | loadable (promoted) | rejected | flipped — fixed |
| garbage bytes | loadable (promoted) | rejected | flipped — fixed |
scalar line (42) |
loadable (promoted) | rejected | flipped — fixed |
array line ([1,2,3]) |
loadable (promoted) | rejected | flipped — fixed |
| valid head + damage within window | loadable (promoted) | rejected | flipped — fixed |
}{-glued complete records |
loadable | loadable (standalone/legacy) | preserved (recovery kept) |
| damage past the 10-record scan window | loadable | loadable | preserved (bounded scan) |
| tolerant-vs-strict on same damaged file | tolerant {}, location active |
tolerant {}, strict undefined, location active |
tolerant reader unchanged |
Secondary claims (all re-confirmed at new head/base)
- Case lookup (
02-case-conflict-ab.mjs, 24/24, witness02-case-conflict-ab.png): unique mixed-case resolves the persisted spelling on both arms; case-only duplicate pair → base silently picks, head throwsSessionIdCaseConflictErrorwith no candidate; same spelling readable in both states →conflictlocation on both arms, base resolves silently / head throws naming the spelling; R5-1 (present-but-unreadable different-spelling twin): base FREE (the hole) / headunreadable_transcriptnaming the twin; R5-2 (valid session + unreadable same-spelling state): resolves on both arms; crashed-first-run 0-byte own file FREE on both arms; foreign-project head under a different spelling flips free→occupied; hardlinked two-owner pair → base picks / head refuses; non-UUID names invisible on both arms. Delta D2 cells: own 0-byte + readable twin resolves the twin on both arms (per-candidate escape does not over-block, cell J); own 0-byte + unreadable twin — the exact49dec69afix-2 shape — base FREE / head OCCUPIES (cell K). - Reserved source + lineage (
03-reserved-source-ab.mjs, 19/19, witness03-reserved-source-ab.png): forged source handed back verbatim by tolerant reads on both arms while head classification rejects it;isReservedStandaloneSessionSourceis sourceType-only by design (thesourceId === undefinedconjunct lives at the call sites — proven by the forged cell); explicit standalone top-level and child classify at head only, with provenparentSource; legacy child of legacy parent reportsparentSource {standalone/legacy}; self-describing child with a gone parent still classifies; exact-self, case-variant-self (F3 axis), and cyclic lineage rejected; child of a damaged parent flips base-promoted → head-rejected; invalid parent id rejected. - Directory identity (
04-identity-probe.mjs, 26/26, head-only, witness04-identity-probe.png): root 0700 with dev+inode+inodeVerifiable:trueon ext4; equivalent path accepted; foreign pathunexpected_identity; rename-pinned child replacement against the captured identity rejected with a measured inode change, restore revalidates clean; symlinked childnot_directory; 0755wrong_modethen clean after chmod; unknown id missing; materialize idempotent, nameconversation-<sha256(id)>; distinct spellings hash to distinct keys; prepare empty-ok /not_empty; ensurerecreated→ready; rename-pinned foreign replacement vs expected →unexpected_identity. - Coordinator + batch (
05-coordinator-ab.mjs, 25/25, witness05-coordinator-ab.png): cells A–D (case-variant races) proceed on base / throwSessionArchivingErrorat head; E same-spelling contention throws on both arms (validity control); F/G agent-suffixed and non-UUID ids NOT case-folded on either arm (by design); I/J head-only collapse integrity (one canonical lock key for two spellings, drained after). Delta D5 cells (e4e9ef5): batch delete/archive/unarchive of a[lower, UPPER]pair — base processes 2 results per session (double-processing the one transcript), head processes 1 canonical operation with zero errors and the correct end state each time. - Create admission (
06-admission-ab.mjs, 14/14, witness06-admission-ab.png): absent id granted on both arms; persisted exact spelling and mixed-case caller vs lowercase transcript rejected on both arms; R5-1 twin: base ADMITS (mints the case-only twin) → head rejectssession_id_conflict/persisted; readable duplicate pair rejected on both arms; EACCES on the archived chats dir and ELOOP on the worktree sidecar both fail closed as retryablesession_id_admission_unavailableon both arms (the ceaf5a9 guard holds).
Delta verification (the 3 commits + merge since round 7)
Depth-2 checkout: only the aggregate HEAD^1..HEAD diff is locally reachable; the delta was scoped from the metadata's commit bodies + the aggregate diff, and each theme verified behaviorally:
49dec69afix 1 (D1) — the inode guard made a directory fail to equal itself. The three comparison sites now degrade on inode-less filesystems:createConversationRootIdentityrecordsinodeVerifiable: falseinstead of throwing,hasRootIdentityfalls back to device, andhasExpectedDirectoryIdentityrequires inode match only when both sides are provable (code read). Behavior: harness 04 D1 cells drive the real comparators with the identity objects an inode-less filesystem produces — an inode-less root revalidates via the fallback; a replaced root under inode-less identity still establishes (the documented weaker guarantee — "cannot prove unchanged" is not "changed"); the same replacement IS detected (identity_changed) when inodes exist. Pinned: mutant M17 (fallback removed) killed by the delivereddegrades instead of failing on a filesystem that reports no inodetest (1 failed | 9 passed), which simulatesino: 0at the lstat seam. The creation-side half on a real inode-less filesystem is Not covered (no exFAT/SMB mount in this container).49dec69afix 2 (D2) — the occupancy escape was per-arm instead of per-candidate. Head resolver skips only the requested spelling's own file in the occupancy loop. Harness 02 cells J/K prove both directions (readable twin still resolves; unreadable twin still occupies). Pinned: mutant M14 (per-arm early return restored) killed byfindSessionIdIgnoringCase > lets an unreadable case twin keep occupying the id—promise resolved "undefined" instead of rejecting(1 failed | 170 passed). The commit body also notes the resolver's two arms were unified — verified by inspection: onefindSessionIdIgnoringCaseimplementation exists at head (the duplication is why the escape had landed in only one copy).49dec69afix 3 (D3) — private directory canonicalized inside the workspace.ConversationWorkspace.directoryKey()normalizes before hashing; harness 04 D3 cells: mixed-case and lowercase ids materialize the same directory named from the lowercase id, and discard via mixed case removes it. Not pinned by the delivered suite: mutant M15 (canonicalization removed) survivesconversation-workspace.test.ts15/15; the tsx probe kills it (same directory → two directories,conversation-355ca9…vsconversation-691dab…). New finding F7 below.f716cc6e(D4) — keep the identity module out of the core barrel. Verified three ways: the cli module imports onlynode:*(no@qwen-codeimport — code read); core'spackages/core/src/index.tsexports nohasVerifiableInode/file-identity(grep); and the repo's own closure gate passes —DEV=true npm run bundle+node scripts/check-serve-fast-path-bundle.js→ "Startup bundle closure checks passed" (exit 0,logs/fast-path-check.txt). The predicate is restated locally with a comment recording why it is not imported.e4e9ef52(D5) — batch collapse + CLI lowercase create. Batch delete/archive/unarchive dedupe throughnormalizeSessionIdForLookup(all three sites): harness 05 D5 cells flip base double-processing → head single canonical operation; mutant M16 (rawSetat the three batch sites, coordinator untouched) killed by exactly the three deliveredcollapses case-variant spellings in one batch to a single …tests (3 failed | 44 passed). CLI--session-idlowercasing:config.ts:2098normalizes throughnormalizeSessionIdForLookup; pinned byconfig.test.ts:2015(getSessionId()=== lowercase), green in both big-gate rounds. Base-arm note: the measured base manifestation is double-processing (the racing second entry landednotFound/alreadyArchivedin this run rather than erroring); the commit body's "deadlock" framing is the same root cause under different timing — either way head collapses the pair to one operation.62d5c5a1— merge of origin/main to bringcheck-workflow-size.shinto the CI test checkout (CI plumbing). Clean auto-merge (git diff --cc HEADempty); all measurements ran on the merged tree.
Findings
No blocking findings this round. In severity order (all carried/new suggestions are coverage-gap reports, not merge conditions):
F7 (new, suggestion) — the D3 canonical directory keying is unpinned
Mutant M15 (directoryKey returns the raw caller spelling) survives the delivered conversation-workspace.test.ts 15/15; the tsx probe kills it (mixed-case and lowercase ids materialize two distinct directories instead of one). The guard is load-bearing: LiveTaskService.ensureResident() passes ids originating in tool arguments, and without the canonicalization one session resolves to two private directories (the defect 49dec69a fix 3 describes). Classification: coverage gap on a fix-3 guard; the code itself is correct (harness 04 D3 cells green at head). A fixture that materializes a mixed-case id through ConversationWorkspace and asserts the directory name derives from the lowercase spelling would kill M15. Not shipped here (verifier does not author PR tests). Not a merge condition.
F3 (carried from round 6, suggestion) — self-lineage conjunct's decisive axis unpinned
Re-measured: M3 survives the delivered suite 35/35; the probe kills it (case-variant self-parent flips REJECTED → {standalone, explicit}). Unchanged classification: coverage gap; both probes green at head. The round-6 suggested fixture (one session-source case whose parentSessionId is a case variant of the session id) would kill M3.
F4 (carried from round 3, suggestion) — cleanup-guard normalization still unpinned
Re-measured: M6 survives 441/441; M8 positive control killed with clean attribution (load variant red on expected '550e8400-…' to be '550E8400-…', resume variant green). Unchanged classification: coverage gap; the guard is load-bearing on the live-restore failure path with non-canonical persisted spellings.
nit (carried from round 3, optional) — strict-reader spies delegate to the tolerant stub
Stands; three sites at the new head (server.test.ts:32985/33225/33355). Route-level tests cannot distinguish a regression swapping readCreationMetadataIfReadable back for the tolerant readCreationMetadata; the split remains pinned by harness 01's tolerant-vs-strict cell and the core gate.
Mutation matrix (round 8)
| mutant | guard removed / reverted | pinning suite | result |
|---|---|---|---|
| M3 | self-lineage conjunct → false |
session-source (35) | survived — then probe-killed: case-variant self-parent flips REJECTED → standalone/explicit → F3 carries |
| M6 | three cleanup-guard reads → raw config.getSessionId() |
acpAgent (full, 441) | survived 441/441 — coverage gap F4 |
| M8 | load-path persisted-spelling adoption removed | acpAgent (filtered) | killed — expected '550e8400-…' to be '550E8400-…', resume variant green; positive control for M6 |
| M9 | prepareStandaloneDirectory entries-before-inspect |
conversation-workspace (15) | killed — 1 red: rejects as not_empty when an entry appears during the final identity re-inspection → F5 fix holds |
| M11 | coordinator canonicalization at all three sites → raw keys | session-archive (47) | killed — 1 red: case-fold race test resolves 'exclusive' instead of rejecting → F6 fix holds |
| M13 | admission sidecar loop swallows non-ENOENT | session-id-admission (14) | killed — 1 red: returns retryable unavailable for a non-ENOENT sidecar error → ceaf5a9 guard holds |
| M14 | resolver per-candidate escape → per-arm early return | sessionService (171) | killed — 1 red: lets an unreadable case twin keep occupying the id → 49dec69 fix 2 pinned |
| M15 | directoryKey canonicalization removed (D3) |
conversation-workspace (15) | survived — then probe-killed: same directory → two distinct directories → new gap F7 |
| M16 | batch dedupe at three functions → raw Set |
session-archive (47) | killed — 3 red: collapses case-variant spellings in one batch × delete/archive/unarchive → e4e9ef5 pinned |
| M17 | inode-less identity fallback removed (D1) | conversation-directory-identity (10) | killed — 1 red: degrades instead of failing on a filesystem that reports no inode → 49dec69 fix 1 pinned |
Every mutant was applied by exact-match replacement with occurrence-count assertions and restored with git checkout --; git status --porcelain empty after each restore. Mutation runs are suite-liveness evidence and are not counted in assertions.json (same convention as rounds 1–7); probe head-arm assertions (2) are counted, mutant arms are not. Witness 07-mutation-matrix.png. Not re-run this round: round-6/7 mutants M1/M2/M2b/M4/M5/M5b/M10a–c/M12/M12b — their guards are unchanged by the 3 delta commits, and their pinning suites all ran green in this round's gates.
Targeted gates (head, as delivered)
| gate | round 1 | round 2 |
|---|---|---|
| core: jsonl-utils + sessionService + corruption | 223/223 pass | 223/223 pass |
| cli small: session-source (35), session-id-admission (14), error-response, conversation-directory-identity (10), conversation-workspace (15), dispatch-error, session-archive (47) | 137/137 pass across 7 files | 137/137 pass |
| cli big: server, acp-http transport, acpAgent (441), acpAgent.worktree, live-task-service, config, multi-workspace-sessions, conversation-runtime-manager (touched by the diff, added to the gate this round) | 2323/2323 pass across 8 files | 2323/2323 pass |
Witness 08-gates-two-rounds.png. Gate liveness: the mutation matrix turned these same suites red on their intended assertions (M9/M11/M13/M14/M16/M17/M8). Counts grew vs round 7 (core 222→223, small 133→137, big 2297→2323) from the delta commits' new tests. No repo-wide gate was claimed or run.
Not covered
- Per-commit attribution: checkout is depth 2;
git rev-list HEAD^1..HEAD^2returns 1 while the metadata lists 30 commits — the shallow-boundary gap. The 3 delta commits were verified as themes against the aggregate diff + their commit bodies, not individually. - Base-move enumeration: intermediate base commits between round-7's base (
39fc769d3a) andHEAD^1(3b3818d) are not locally reachable; the overlap was measured on the merge result instead (git diff --cc HEADempty = clean auto-merge). All measurements ran on the merged tree. - Real inode-less filesystem (D1 creation half): no exFAT/FAT/inode-less SMB mount in this container. The comparison fallbacks were driven through the real comparators with the identity objects an inode-less FS produces (harness 04 D1 cells); the creation path under
ino: 0is pinned by the delivered lstat-seam test (M17 killed it). A real mount would add end-to-end confirmation; not available here. - Windows / a real case-insensitive filesystem: alias collapse verified via hardlink (two backed owners → conflict, not a silent pick); the single-owner collapse path (
owners.length === 1) and theisSameConversationPathwin32 branch are unreachable on this ext4 container by construction. - True multi-process daemon E2E: restore/admission/batch verified at dist/component level plus the PR's own in-process server/transport suites (real loopback HTTP); no live daemon over a real socket was booted.
- Real-race interleavings: harness 05's lock cells use deterministic interposition (one side holds the lock set, the other attempts). They reproduce the shape of each race, not a contended-writer trigger; the base-arm batch double-processing manifested as a raced second entry (notFound/alreadyArchived) rather than the commit body's deadlock framing.
- F3/F4/F7 pinning fixtures not shipped (verifier does not author PR tests; all three fixtures described in Findings).
- Timing ladder: not run — no new scanner over free-form untrusted text this round; the delta adds no regexes over outsider input.
- Performance/concurrency load: not run.
- Lint/format/typecheck repo-wide: CI builds head green per the environment contract; no separate lint run.
- Docs-only changes (
standalone-pr2-core.mdplan, architecture doc): reviewed for claim context, not behaviorally tested. - Mutation coverage beyond the matrix: other guards (e.g.
getSessionLocation's pattern pre-filter, identity owner checks under a different uid,retryPendingConfigCleanupnormalization atacpAgent.ts:12881) were not mutated; most are pinned by delivered tests that ran green in the gates. - PR text: no verifier-directed instructions found in the PR title/body/commit messages this round (none to report).
Methodology
Environment: CI merge-ref checkout (HEAD = merge commit 35974f89, base tip 3b3818d = HEAD^1, PR head 62d5c5a1 = HEAD^2), node:22-bookworm-class container, Node v22.23.2, uid 1000 (non-root — chmod-based EACCES fixtures are effective); npm ci + npm run build pre-run at HEAD. The PR leaves package.json/package-lock.json untouched — measured: the two lockfiles are byte-identical (0 package diffs between HEAD^1 and HEAD), making the shared root node_modules a clean control. A/B base side built in git worktree tmp/base-tree at HEAD^1 (rebuild only packages/core + packages/cli; log logs/base-build.log, exit 0). The base worktree got its own node_modules: every root entry re-linked except the @qwen-code scope, which was relinked into the base tree's packages, plus the per-package nested node_modules version-conflict nests (ajv/fdir/ignore/mime under packages/core; missing them reproduced wrong-version type errors on the first attempt — fixed and rebuilt). Realpath assertions before trusting the control: readlink -f tmp/base-tree/node_modules/@qwen-code/qwen-code-core → …/tmp/base-tree/packages/core (and qwen-code → base packages/cli). Harnesses 01–06 import each arm's compiled dist directly and drive real files through the SessionService(cwd, { runtimeBaseDir }) seam into per-arm scratch storage; M3/M15 probes import src via tsx so single-file source mutations take effect without a rebuild; every other mutant ran through the suites' normal source compilation. Mutants: applied by exact-match text replacement (occurrence counts asserted), run, restored with git checkout --; git status --porcelain verified empty after every restore. D4 verified with the repo's own scripts/check-serve-fast-path-bundle.js against a DEV=true npm run bundle metafile. Raw logs in logs/ (h01–h06, m3–m17, gate-*, base-build.log, bundle.log, fast-path-check.txt); harness sources in harness/ are rerunnable. Evidence captures produced with scripts/verify-capture.mjs (8 images in evidence/). Assertion counting: harness checks 137 + probe head-arm assertions 2 + fast-path bundle check 1 + core gate 223 + cli small gate 137 + cli big gate 2323 = 2823, all scripted, all executed; fail counts only unexpected outcomes — none. Gate flakiness: two full rounds of all three gate groups agreed (36 green file-runs).
Flakiness gate log
rounds=5 files=18 skipped=0
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/acp-integration/acpAgent.worktree.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.worktree.test.ts
file packages/cli/src/config/config.test.ts: (cd packages/cli) npx --no-install vitest run ./src/config/config.test.ts
file packages/cli/src/serve/acp-http/dispatch-error.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/acp-http/dispatch-error.test.ts
file packages/cli/src/serve/acp-http/transport.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/acp-http/transport.test.ts
file packages/cli/src/serve/conversations/conversation-runtime-manager.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/conversation-runtime-manager.test.ts
file packages/cli/src/serve/conversations/conversation-workspace.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/conversation-workspace.test.ts
file packages/cli/src/serve/conversations/session-source.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/conversations/session-source.test.ts
file packages/cli/src/serve/live/live-task-service.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/live/live-task-service.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/session-archive.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/server/session-archive.test.ts
file packages/cli/src/serve/session-id-admission.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/session-id-admission.test.ts
file packages/cli/src/utils/conversation-directory-identity.test.ts: (cd packages/cli) npx --no-install vitest run ./src/utils/conversation-directory-identity.test.ts
file packages/core/src/services/sessionService.corruption.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/sessionService.corruption.test.ts
file packages/core/src/services/sessionService.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/sessionService.test.ts
file packages/core/src/utils/jsonl-utils.test.ts: (cd packages/core) npx --no-install vitest run ./src/utils/jsonl-utils.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/acp-integration/acpAgent.test.ts: PPP
packages/cli/src/acp-integration/acpAgent.worktree.test.ts: PPP
packages/cli/src/config/config.test.ts: PPP
packages/cli/src/serve/acp-http/dispatch-error.test.ts: PPP
packages/cli/src/serve/acp-http/transport.test.ts: PPP
packages/cli/src/serve/conversations/conversation-runtime-manager.test.ts: PPP
packages/cli/src/serve/conversations/conversation-workspace.test.ts: PPP
packages/cli/src/serve/conversations/session-source.test.ts: PPP
packages/cli/src/serve/live/live-task-service.test.ts: PPP
packages/cli/src/serve/multi-workspace-sessions.test.ts: PPP
packages/cli/src/serve/server.test.ts: PPP
packages/cli/src/serve/server/error-response.test.ts: PPP
packages/cli/src/serve/server/session-archive.test.ts: PPP
packages/cli/src/serve/session-id-admission.test.ts: PPP
packages/cli/src/utils/conversation-directory-identity.test.ts: PPP
packages/core/src/services/sessionService.corruption.test.ts: PP
packages/core/src/services/sessionService.test.ts: PP
packages/core/src/utils/jsonl-utils.test.ts: PP
verdict: timeout
summary: only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/acp-integration/acpAgent.worktree.test.ts: P (exit 0)
round 1 · packages/cli/src/config/config.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/acp-http/transport.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/conversations/conversation-runtime-manager.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/conversations/conversation-workspace.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/conversations/session-source.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/live/live-task-service.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/session-archive.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/session-id-admission.test.ts: P (exit 0)
round 1 · packages/cli/src/utils/conversation-directory-identity.test.ts: P (exit 0)
round 1 · packages/core/src/services/sessionService.corruption.test.ts: P (exit 0)
round 1 · packages/core/src/services/sessionService.test.ts: P (exit 0)
round 1 · packages/core/src/utils/jsonl-utils.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.worktree.test.ts: P (exit 0)
round 2 · packages/cli/src/config/config.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/acp-http/transport.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/conversation-runtime-manager.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/conversation-workspace.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/conversations/session-source.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/live/live-task-service.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/session-archive.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/session-id-admission.test.ts: P (exit 0)
round 2 · packages/cli/src/utils/conversation-directory-identity.test.ts: P (exit 0)
round 2 · packages/core/src/services/sessionService.corruption.test.ts: P (exit 0)
round 2 · packages/core/src/services/sessionService.test.ts: P (exit 0)
round 2 · packages/core/src/utils/jsonl-utils.test.ts: P (exit 0)
round 3 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 3 · packages/cli/src/acp-integration/acpAgent.worktree.test.ts: P (exit 0)
round 3 · packages/cli/src/config/config.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/acp-http/dispatch-error.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/acp-http/transport.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/conversations/conversation-runtime-manager.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/conversations/conversation-workspace.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/conversations/session-source.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/multi-workspace-sessions.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/server.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/server/error-response.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/server/session-archive.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/session-id-admission.test.ts: P (exit 0)
round 3 · packages/cli/src/utils/conversation-directory-identity.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
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Already have 2 approves,3ks.
|
Review 补充说明（对应 review #4979624548） No blocking findings. Checked:
Unreviewed dimensions (verdict capped, not a clean result):
Reviewed with AI assistance. |
Conflict: the SDK browser-bundle budget — main bumped 198→199KB for persistent session attachments while this branch needed headroom for the session PR binding types; resolved at 200KB with both reasons recorded. Also fixes a latent type error currently on main: QwenLM#9477 added deleteSessionAttachments to the deleteDaemonSessions bridge contract, but the batch-delete test from QwenLM#9341 still passes a closeSession-only mock; CI did not catch it because the verify check is skipped on main pushes.
…nLM#9551) `deleteDaemonSessions` takes `bridge: Pick<AcpSessionBridge, 'closeSession' | 'deleteSessionAttachments'>` since QwenLM#9477, which updated every mock that existed when it was written. QwenLM#9341 landed in parallel and added one more — "collapses case-variant spellings in one batch to a single delete" — with a `{ closeSession }` bridge. Each PR was green on its own merge ref; main is red combined, so `npm ci` fails the build for every branch cut from it: src/serve/server/session-archive.test.ts(1069,7): error TS2741: Property 'deleteSessionAttachments' is missing in type '{ closeSession: Mock<Procedure> }' Adds the same `vi.fn().mockResolvedValue(undefined)` its neighbours already pass. The test asserts on the delete result, not on the spy, so its meaning is unchanged: 48/48 still pass.
* test(ci): stage on-disk session state in the serve A/B The serve A/B drives every scenario against a freshly started, empty daemon, so the entire session-admission surface — case resolution, transcript integrity, active/archive conflicts, reserved sources — is unreachable and a PR that rewrites it diffs as "no response changes". QwenLM#9341 is the worked example: the posted A/B reported no change across 4 scenarios while the same build pair, driven with transcripts on disk, answers differently on six requests. Scenarios can now stage transcripts before their request and capture a reduced projection of the response, and the HTTP status is recorded on every capture so a status-only difference is visible. Six session-admission scenarios use that: a healthy restore, the legacy uppercase spelling, case-only twins, an unreadable transcript, one id in both the active and the archive directory, and creation carrying a source type. The staged fixtures depend on the on-disk project layout, which the harness mirrors rather than imports. If that mirror ever drifts the transcripts land nowhere and every staged scenario would quietly answer 404 on both arms, so the healthy restore doubles as a canary that fails the drive instead of publishing a reassuring all-clear. * test(ci): address the R1 review round on the serve A/B harness Clears the capture directory before a drive writes into it, so a re-run can never let an earlier run's files stand in for scenarios this run did not capture, and writes a completion marker once every scenario is captured. A baseline without that marker is now reported as partial, because a base drive that stopped part-way leaves the scenarios it never reached rendering as "this PR adds these responses" — the same shape a genuinely new scenario produces. Both arms are driven by the head checkout's harness, so a capture pair always carries the status field on both sides and the compatibility shim for a base that predates it was unreachable; it and its tests are removed rather than left to teach a transition the wiring cannot produce. Non-object response bodies are now nested instead of spread, which dropped scalars and re-keyed arrays. The source-type scenario probed a type today's daemon does not reserve, so it never reached the refusal branch it was named for. It is split: one scenario pins the source the daemon actually reserves, the other keeps an ordinary type that a future reservation would move from admitted to refused. A second canary covers the archive directory, which nothing certified before — a drifted archive name would have left the conflict scenario loading from the active copy on both arms and diffing clean. The remaining inert request body key is gone too; the client id is read from a header, never the body. The harness tests were passing under mutations they appeared to cover: the staging routing, the projection guards and the fixed-id requirement are now pinned by assertions that fail when those are inverted. * test(ci): close the R2 gaps in the serve A/B harness The completion marker was declared twice, once by the writer and once by the reader, with nothing pinning the copies together: renaming one side left both suites green while CI would either flag every complete baseline as truncated or stop noticing truncated ones. The drive now owns the constant and the diff imports it. Two invariants the code asserted in comments were not enforced. A response body carrying its own status key overwrote the status the harness saw, so a status-only regression on such a route would have diffed as an unchanged body; the harness value now wins. And the canary check — the harness's only drift alarm — had no test at all: inverting it so it could never fire left every test passing. It is now a named helper with tests on both branches. The archive canary pinned an exact status, which conflates its precondition with the product's decision: if an archived-only load ever becomes loadable, the precondition still held, but the drive would abort and suppress the very row the captures already contained. It now fails only on the one answer that means the staged file was never seen. Finally, nothing pinned that a staged scenario probes an id it actually staged. Staging the wrong id answers 404 on both arms, captures identically, and drops that branch out of coverage with every test green. * test(ci): close the R3 gaps in the serve A/B harness The completion marker proved that some drive finished, never that this run's did. The only reset lived inside the drive script, which does not run when an arm is skipped before it starts — no merge-base resolved, the base checkout failing, or its build dying — and on the persistent pool the capture paths outlive a run. An inherited baseline then arrived complete, marker included, so neither degraded-baseline warning fired and the comment would have diffed this head against another run's base. The workflow now clears both capture paths in an unconditional step, which is the only place that covers a skipped arm. The in-script reset also turned a write-only script into an unguarded recursive delete of a path taken straight off the command line, which the documented local usage invites a reader to mistype. It now refuses any directory that holds something other than captures. The healthy canary's premise was wrong: the product validates transcripts record by record and fails open, so a fixture whose records stop validating restores as an empty session and still answers 200. Measured against a real daemon, a wholly drifted fixture passed the canary and left every staged scenario probing an empty daemon — the false all-clear this harness exists to prevent. The canary now keeps a replay-size witness in its capture and fails when it is zero. Three test gaps behind the same theme: the marker's writer, the comment subcommand that CI actually invokes, and the mixed-case scenario's existence were all unpinned, and the staged-id check asserted against the union of every scenario's staging rather than the one under test. The capture loop is extracted so its ordering is testable without a daemon. * test(ci): cover the setup-failure abort in the serve A/B capture loop The capture loop was extracted so its ordering could be pinned without a daemon, and three of its four abort branches were covered — but not the one that fires when a scenario's setup request fails. Dropping that throw left the whole suite green while a capture would be recorded against a daemon where the setup never took effect, which is the masked diff the branch exists to prevent. * test(ci): close the R5 gaps in the serve A/B harness * fix(ci): send an admitted source in the serve A/B unreserved-source witness --------- Co-authored-by: wenshao <nigolaschao777@gmail.com>
|
Released in v0.21.15. |
5 participants
What this PR does
This PR delivers standalone-session PR2A: the internal source, identity, and admission primitives required before the standalone lifecycle service is added. It classifies explicit standalone, compatible legacy projectless, and Live transcripts without creating a second runtime or catalog; validates active/archive location, and validates parent lineage for legacy children; reserves the standalone source from generic creation; and carries the classification through REST, ACP, and Live-task restore paths.
It also adds deterministic private-directory identity checks, makes case-insensitive session lookup authoritative across active and archived storage, and fails closed when transcript creation metadata is truncated, malformed, missing, conflicting, or changes location during lookup. Existing tolerant transcript recovery remains available for ordinary readers, including complete
}{-glued records.The standalone architecture document and the reviewed PR2 implementation plan are updated to lock the PR2A/PR2B split and the downstream containment boundaries. This PR does not publish standalone routes, capabilities, SDK APIs, or UI behavior.
Why it's needed
Standalone sessions need a trustworthy way to distinguish their persisted source and private directory from Live and workspace sessions before lifecycle operations can be implemented. Treating an empty metadata result as proof of a legacy standalone session can otherwise promote a damaged Live or child transcript into a top-level standalone session, while non-authoritative case lookup or directory comparison can bind the wrong persisted identity.
These primitives give PR2B one fail-closed foundation for source provenance, parent lineage, session spelling, runtime ownership, and private-directory identity without falling back to the primary workspace or expanding the public product surface prematurely.
Reviewer Test Plan
How to verify
Local validation completed with
npm run build,npm run typecheck, andnpm run lint, plus final touched-file ESLint and Prettier checks. Targeted final tests passed for 209 Core JSONL/SessionService cases and 19 CLI source-classification cases; the broader PR2A verification also passed the source/workspace/identity, server/ACP Agent/Live-task, and ACP transport suites.Evidence (Before & After)
N/A — internal source, identity, admission, tests, and design changes with no user-visible or TUI output.
Tested on
Environment (optional)
macOS 26.4.1, Node.js 24.12.0, npm 10.9.8, local workspace runtime with sandbox-independent unit and build verification.
Risk & Scope
Linked Issues
Refs #8908
Builds on #9181 and follows #8890.
中文说明
本 PR 做了什么
本 PR 交付 standalone session 的 PR2A：在增加 standalone 生命周期服务之前所需的内部 source、identity 与 admission primitives。它在不创建第二套 runtime 或 catalog 的前提下，对 explicit standalone、兼容的 legacy projectless 与 Live transcript 进行分类；验证 active/archive location，并对 legacy child 验证 parent lineage；禁止 generic creation 使用保留的 standalone source；并把分类结果贯通到 REST、ACP 与 Live-task restore 路径。
它还增加 deterministic private-directory identity 校验，使大小写不敏感的 session lookup 在 active 与 archived storage 上保持 authoritative，并在 transcript creation metadata 被截断、损坏、缺失、冲突或读取期间发生 location 变化时 fail closed。普通 reader 仍保留既有 tolerant transcript recovery，包括完整的
}{glued records。Standalone 架构文档与已审计的 PR2 实施计划同步更新，锁定 PR2A/PR2B 拆分及后续 containment 边界。本 PR 不发布 standalone routes、capabilities、SDK APIs 或 UI 行为。
为什么需要
在实现 lifecycle operations 之前，standalone session 需要一种可信方式，把其 persisted source 与 private directory 同 Live 和 workspace session 区分开。否则，把空 metadata result 当作 legacy standalone 的证明，可能把损坏的 Live 或 child transcript 提升为 top-level standalone；非 authoritative 的大小写 lookup 或 directory comparison 也可能绑定错误的 persisted identity。
这些 primitives 为 PR2B 提供一套 fail-closed 基础，统一处理 source provenance、parent lineage、session spelling、runtime ownership 与 private-directory identity，同时不回退到 primary workspace，也不提前扩大公开产品表面。
Reviewer 测试计划
如何验证
本地验证已完成
npm run build、npm run typecheck与npm run lint，并对最终变更文件执行 ESLint 与 Prettier 检查。最终定向测试通过 209 个 Core JSONL/SessionService 用例和 19 个 CLI source-classification 用例；更广泛的 PR2A 验证还通过了 source/workspace/identity、server/ACP Agent/Live-task 与 ACP transport 套件。证据（Before & After）
N/A —— 仅涉及内部 source、identity、admission、测试与设计，无用户可见或 TUI 输出变化。
测试平台
环境（可选）
macOS 26.4.1、Node.js 24.12.0、npm 10.9.8，本地 workspace runtime，执行了不依赖 sandbox 的单元测试与构建验证。
风险与范围
关联 Issue
Refs #8908
基于 #9181，并承接 #8890。