fix(cli): Recover sessions across archive races - #9513
Conversation
github-actions
Bot
added
the
review/self-reported
🩺 serve daemon A/BBuilt the PR base vs this PR head
|
| field | PR base (before) | this PR (after) |
|---|---|---|
_status |
409 |
200 |
code |
"session_conflict" |
— |
error |
"Session \"a0000000-0000-4000-8000-00000000da05\" exists in both active and archived directories. Delete the session with POST /sessions/delete before loading." |
— |
session-restore-case-twins
| field | PR base (before) | this PR (after) |
|---|---|---|
error |
"Multiple persisted sessions match \"a0000000-0000-4000-8000-00000000da03\" by case." |
"Session \"a0000000-0000-4000-8000-00000000da03\" exists in both active and archived directories. Delete the session with POST /sessions/delete before loading." |
— Qwen Code · serve A/B
…lready gone A child deleted between the lstat and the realpath, or a root that vanished mid-inspection, was rewritten as 'identity_changed' and then surfaced as 'Live conversation directory must be an owned direct child' — a plain Error with no .code pointing at permissions and symlinks when the directory was simply deleted. Restore the ENOENT-race -> false contract of discardEmptyConversationDirectory (QwenLM#9489, item 4). Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…torage-aligned Item 2 of QwenLM#9489: readExistingMetadata read the location, read the metadata, then re-read the location and returned undefined on mismatch, so an archive landing between the probes made lock-free resolvers report a healthy session as session_not_found. Creation metadata is immutable, so one tolerant read per state (active first, then archived) decides deterministically; the location probes are gone and a path-safety charset gate keeps the joined transcript path a single segment. Item 3: the parent-lineage gate required strict RFC-4122 v1-v5 ids while the store resolves far looser names, so persisted parents written by older builds (nil, v6/v7, agent-suffixed ids) turned loadable children into SessionNotFoundError, and the -agent- allowance could never resolve. Drop the shape gate and let storage resolution decide, keeping only the self-reference rejection. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
…ck restore scans Items 1 and 5 of QwenLM#9489. Item 1: a session persisted in both active and archived states — left behind by a crash inside archiveSessions — hard-failed ACP session/load and session/resume with session_conflict while plain CLI --resume kept loading the active copy. findSessionIdIgnoringCase now resolves the requested spelling first (and a single both-states candidate) instead of throwing, and assertSessionLoadable treats 'conflict' as loadable from the active copy. Mutating surfaces keep refusing: unarchive still conflicts via assertSessionArchived, and multi-runtime ownership arbitration stays strict so a conflicted internal copy cannot claim a session an ordinary workspace serves. Item 5: both restore handlers ran findSessionIdIgnoringCase twice per request — once as a pre-lock guard whose result REST discarded and the ACP twin kept as a stale storageSessionId fallback consumed exactly in the TOCTOU where the in-lock resolve returned undefined. The pre-lock guards are gone (the in-lock resolve is authoritative and both handlers now agree), the exact-spelling fast path removes directory scans from the common case entirely, and the remaining scan uses async readdir so a large chats tree no longer blocks the daemon event loop. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Canonicalize live task keys before resident bridge operations, and keep known case-conflict responses when the optional storage recheck fails. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
cd0477b to
c6cab84
Compare
|
Please do not rebase or force-push to an active PR as it invalidates existing review comments. Note for future reference, the bots always squash all changes into a single commit automatically as part of the integration. 中文请勿对活跃的 PR 执行 rebase 或 force-push，因为这会使已有的评审评论失效。另外，供日后参考：作为集成流程的一部分，机器人始终会自动将所有改动压缩（squash）为单个提交。 |
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
✅ Qwen Triage finished — CI landed green on ✅ Qwen Triage 已完成 —— |
|
Change since the last gate at Template looks good ✓ Problem: observed, with evidence — #9489 tracks four regressions introduced by the merged #9341, each reported by @wenshao against head Direction: aligned — restoring behaviour narrowed by the author's own merged PR, squarely on the daemon/ACP/REST session surface. No auth/sandbox/telemetry/public-contract territory. Size: 314 production lines vs 870 test lines vs 10 docs lines across 22 files, recomputed at the merged head. Core paths carry 4 production lines (one-line changes each in Approach: minimal and 1:1 with the linked issue. The merge itself added no scope: I diffed every PR-owned file between the old and new heads — 13 of 22 are byte-identical, the 9 changed ones are exactly the files Risk: Stage 1e matches one revert-correlated path — Moving on to code review. 🔍 中文说明上次 gate（ 模板完整 ✓ 问题：已观测到，且有证据——#9489 追踪已合并 #9341 引入的四项回归，均由 @wenshao 在 head 方向：对齐——恢复作者自己已合并 PR 收窄的行为，完全在 daemon/ACP/REST 会话面，不涉及 auth/沙箱/遥测/公共契约。 规模：314 生产行 vs 870 测试行 vs 10 文档行，共 22 个文件（已在合并后 head 重新计算）。核心路径仅 4 个生产行（ 方案：最小且与所链 issue 一一对应。合并本身未增加范围：逐一比对了新旧 head 上全部 22 个 PR 文件——13 个字节级一致，9 个有变化的恰好是 main 也修改过的文件；合并提交的 combined diff 只显示一处冲突（ 风险：Stage 1e 命中一个回滚相关路径—— 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewThe diff itself was reviewed clean at
No new findings. The suggestion-level deferrals from rounds 6–8 (D7-1…D7-5) stand as recorded, non-blocking. Testing evidence (this PR's own CI + sandboxed lane)Unattended run — PR code is never executed here; this quotes the PR's own CI on the reviewed commit. On Final CI results for
One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行（取最新一次运行），省略 skipped，失败项排在最前。 Sandboxed verification: the central claim is behavioural (a raced-archive session becomes recoverable instead of 409), and the mutation matrix that pinned all 14 guard points was measured at the pre-merge head — the delta since is merge-only, but it is a delta in exactly the files that carry the claim. A 中文说明代码审查：diff 本身在
无新发现。第 6–8 轮的建议级推迟项（D7-1…D7-5）维持记录在案，均非阻塞。 测试证据：无人值守运行——此处从不执行 PR 代码，仅引用 PR 自身 CI。 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 4/5 — the one open question after eight clean rounds was whether the merge of Stepping back over the whole arc:
Reservations, named for the record: the suggestion-level deferrals from rounds 6–8 (D7-1…D7-5, incl. the untested ordinary-collision guard, which fails toward the safe ambiguous-owner response) are tracked in-thread and should land as a follow-up issue around merge; the standing Verdict: approve — approval deferred until CI lands green on this head; the marker below carries the commit pin. 中文说明总体判断：八轮干净审查之后唯一悬而未决的问题是合并 回看整条弧线：
保留意见，记录在案：第 6–8 轮的建议级推迟项（D7-1…D7-5，含未测试的普通碰撞守卫——它朝安全的歧义应答方向失败）已在帖中跟踪，应在合并前后建后续 issue；当前 PR 上的 结论：批准——批准推迟到本 head 的 CI 全绿，下方标记携带提交钉扎。 — Qwen Code · qwen3.8-max Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Needs some rethinking — see my notes above. Two concrete items: (1) the merge-commit CI failures in session-telemetry.test.ts are coupled to the activeInRuntime change (the prior merge in this area, #9512, had this check green), and (2) item 5's exact-spelling fast path is described and test-planned but not implemented. 🙏
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Batch case-insensitive transcript lookups for multi-thread waits and preserve organization metadata when live and persisted session IDs differ only by case. 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>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
6 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- session-archive.ts:571 non-self case conflicts are flattened into SessionConflictError's hardcoded both-states message — already recorded in the round-6 deferred list (session-archive.ts:567)
- integration-tests/cli/qwen-serve-routes.test.ts is collected by no workspace test command — already recorded as F6 in round 3; this round ran the suite locally against the bundled build (36/36 green)
- transport.test.ts:4639 ACP conflict test seeds byte-identical fixtures and cannot detect which copy was adopted — already reported as R2-2 (round 2, byte-identical-fixtures pattern)
- multi-workspace-sessions.test.ts:4514 both-states export assertion cannot detect which state was served — already recorded as N2 (round-3 deferred list)
- server.test.ts:23715 'reads and exports the active copy of an exact persisted conflict' cannot detect which copy was served — same byte-identical-fixtures pattern already reported as R2-2 / recorded as N2
- server.test.ts:25323 '%s restores active/archive conflicted sessions from the active copy' cannot detect which copy the restore used — same byte-identical-fixtures pattern already reported as R2-2 / recorded as N2
Not explored to full depth (tool budget reached): "agent 1c": none — but I did not run the full typecheck/build (soft-budget decision); compilation-level coherence was verified by reading imports and signatures instead..
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/conversations/session-source.test.ts:276 — [probe] D7-1 conflicted-transcript test cannot detect which copy was adopted (mutant adopting archived passes all 49 tests)packages/cli/src/serve/routes/session.ts:1804 — [probe] D7-2 new ordinary-collision guard on the live-owner branch has no test pairing (disabling mutant keeps 5381 tests green)packages/cli/src/serve/routes/session.ts:1843 — [probe] D7-3 unqualified transcript reads 409 on multi-workspace daemons while qualified reads and load return 200packages/cli/src/serve/conversations/conversation-workspace.ts:213 — [probe] D7-4 discard-path ENOENT narrowing unpinned — broadening to all io_error ships greenpackages/core/src/services/sessionService.test.ts:2898 — [probe] D7-5 resolver rewrites dropped the pin that the requested spelling itself is returned when readable (occupancy-self-skip mutant passes all 18 tests)
中文说明
本轮确认的 6 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 1c"：none — but I did not run the full typecheck/build (soft-budget decision); compilation-level coherence was verified by reading imports and signatures instead.。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 5 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Addressed the round-7 blocking feedback in
Validation: core SessionService 176/176; multi-workspace sessions 120/120; repository build and typecheck; changed-file Prettier/ESLint; diff check. |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed.
7 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- test-matrix finding — liveOwner-branch ordinary-collision block untested (routes/session.ts:1805) — already recorded as D7-2 (round-7 deferred list)
- chunk-1 reverse-audit finding — non-self case conflicts flattened into SessionConflictError's hardcoded both-states message — already recorded in the round-6 deferred list (session-archive.ts:567)
- chunk-1 reverse-audit finding — stale archived copy never reconciled after both-states adoption — already recorded in the round-6 deferred list (session-archive.ts:589) and blocker thread 3832630516
- chunk-3 reverse-audit finding — allowActiveConflict unthreaded through the internal-entry loop and multi-workspace scan — already recorded as D7-3 (round-7 deferred list), R1-7 thread
- chunk-6 reverse-audit finding — requested-spelling self-resolution pin dropped by the readdir spy migration — already recorded as D7-5 (round-7 deferred list)
- chunk-1 reverse-audit (round 4) finding — transport both-states test seeds byte-identical fixtures — already reported as R2-2 (round 2)
- chunk-4 reverse-audit (round 4) finding — server.test.ts both-states export test seeds byte-identical fixtures — already reported as R2-2 / recorded as N2 (round 3)
Not explored to full depth (tool budget reached): chunk 6: running sessionService.test.ts findSessionIdIgnoringCase tests (worktree has no installed dependencies; install + build prerequisite exceeds budget).
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/conversations/session-source.ts:109 — [probe] SAFE_TRANSCRIPT_NAME_PATTERN admits '.' and '..' dot segmentspackages/cli/src/serve/server/session-archive.ts:546 — [probe] Both-states conflict-vs-adoption verdict decided by readdir enumeration order
中文说明
已审查。
本轮确认的 7 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）：chunk 6：running sessionService.test.ts findSessionIdIgnoringCase tests (worktree has no installed dependencies; install + build prerequisite exceeds budget)。
收敛姿态下延后（第 8 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.21.15)
doudouOUC
changed the title
|
@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: 2991 passed · 0 failed · 2991 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：2991 通过 · 0 失败 · 2991 总计 抖动门： Verification reportPR #9513 Deep Verification (follow-up round 6) — fix(cli): Recover sessions across archive racesVerdict: 中文 — 判定：✅ 通过（agent 判定）沙箱验证在隔离、无凭证的容器中执行（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁、变异矩阵）。仅作为评审证据，不构成评审、批准或 CI 检查。
Previous-finding status (follow-up round 6)Round 5 verified head
Scope selectionCentral claim (re-measured at the new base): a session whose transcript is readable in both active and archived states — the residue of a crash inside Secondary claims (re-covered by gates + mutation kills): creation-metadata reads tolerate a concurrent archive move behind a path-safety gate; legacy parent ids (nil/v6/v7/agent-suffixed) resolve again; ENOENT races during directory-identity inspection report "already gone"; the catalog scan is async and runs once inside the restore lock. Round-5→6 delta: the narrowing kept items 1–5 of #9489 and dropped the organization/live-task mixed-case expansion (which is what carried F3). The delta's surviving guards were all re-proven by fresh kills (matrix below). Explicitly out of scope (also under Not covered): daemon E2E with a live model, Windows/case-insensitive-filesystem paths, repo-wide typecheck/lint, the workflow's 5-round flakiness gate. Central claim — A/B load-bearing proofService-level harness (both trees' compiled dist, arm-coded expectations)Harness
Result: 32/32 assertions (16 per arm); the seven flip cells (C2, C7, C8, C10, C11, C13, C14) flip, all controls hold on both arms. Base-control hygiene: the PR changes no Wire-level A/B (real spawned daemon)The PR's own integration test
Witness: Mutation matrix (14 fresh kills at the new head, 0 survivors)Scratch worktree
Survivors: 0/14 after correction. Positive controls: every cited suite was made red by a point mutation collected through the same command that ran it green unmutated. The M-batch episode is the documented "control lives in a second file" case: the guard protects workspace-qualified internal transcript/export reads, which only Targeted gates (at the merge commit)Witness:
Gate liveness is proven by the mutation round itself. (Round 5's Findings (non-blocking notes only)No blocking findings. The central claim held its A/B at the new base and at the wire; every freshly-run guard held its mutation kill; the sole prior substantive finding (F3) was fixed by the narrowing and re-measured clean. N1 (nit, carried) — F1 order-dependent cascade in config.test.tsRe-measured fresh; signature unchanged from rounds 1–5 (see status table). This is a suite-hygiene issue (a failed earlier test leaks state that times out the lease test at ~15 s), not a defect in this PR's production code. N2 (nit, residual of superseded F2) — fallback door branches pinned collectivelyThe three N3 (note) — async-readdir pinned by mock retargeting, not behaviorM-async's 15 reds are the suite's deliberate implementation pin (the PR retargeted the mocks to Not covered
MethodologyEnvironment: — Qwen Code · sandboxed verification Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
Resolved the merge conflict with current
No review threads were involved in this conflict resolution. |
|
@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: 3021 passed · 0 failed · 3021 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：3021 通过 · 0 失败 · 3021 总计 抖动门： Verification reportSandboxed verification: ✅ passed — merge-ready (agent verdict) Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates, mutation matrix. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 3021 passed · 0 failed · 3021 total 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁、变异矩阵）。仅作为评审证据，不构成评审、批准或 CI 检查。
Verification reportPR #9513 Deep Verification (follow-up round 7) — fix(cli): Recover sessions across archive racesVerdict: 中文 — 判定：✅ 通过（agent 判定）见上方中文摘要；下文表格为唯一数字来源。 Previous-finding status (follow-up round 7)Round 6 verified head
Scope selectionCentral claim (re-measured at the new base): a session whose transcript is readable in both active and archived states — the residue of a crash inside Secondary claims (re-covered by gates + mutation kills): creation-metadata reads tolerate a concurrent archive move behind a path-safety gate ( Round-6→7 delta probes: (a) the main refactor's new boundary guard Explicitly out of scope (also under Not covered): daemon E2E with a live model, Windows/case-insensitive-filesystem paths, repo-wide typecheck/lint, the workflow's 5-round flakiness gate. Central claim — A/B load-bearing proofService-level harness (both trees' compiled dist, arm-coded expectations)Harness
Result: 40/40 assertions (20 per arm); the eleven flip cells (C2, C5b, C5c, C7, C7b, C8, C10, C11, C13, C13b, C14) flip, all controls hold on both arms. Base-control hygiene: the PR changes no Wire-level A/B (real spawned daemon)The PR's own integration test
Witness: Mutation matrix (18 runs at the new head; 15 guards pinned, 3 survivors classified)Scratch worktree
Positive controls: every cited suite was made red by a point mutation collected through the same command that ran it green unmutated. Layered-guard combination check: M-doors (all doors) and M-batch/M-doors-wq (per-door) together show the door set and the batch pre-check each carry independent reds; no hidden redundancy among them. Targeted gates (at the merge commit)Witness:
Gate liveness is proven by the mutation round itself (15 kills across these exact suites). Findings (non-blocking notes only)No blocking findings. The central claim held its A/B at the new base and at the wire; every guard that had a pin kept it; the prior substantive finding (F3) remains fixed. N1 (nit, carried) — F1 order-dependent cascade in config.test.tsRe-measured fresh; signature unchanged from rounds 1–6 (status table). Suite-hygiene issue, not a PR defect. N2 (nit, carried) — fallback doors pinned collectivelyUnchanged: one server.test.ts test carries all three N4 (nit, new) — the live-owner ordinary-collision scan is unpinnedThe effective diff adds a fail-closed block to the live-owner path of N5 (note) — one environmental skip in server.test.ts
Not covered
MethodologyEnvironment: — Qwen Code · sandboxed verification Flakiness gate logEvidence 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. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
LGTM, looks ready to ship — CI landed green after the review. ✅
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM on the narrowed scope. Verified against head 97868f7:
- Both-states same-spelling recovery consistently reads the ACTIVE copy across REST, daemon ACP, and embedded ACP (all four resolution sites treat
candidate === requestedas recoverable-from-active); differently-spelled and ambiguous conflicts still surfacesession_conflicton every surface, archive/unarchive stay strict, and delete now takes the lease and removes both copies plus sidecars — no stranded-archived wedge. - The race surfaces fail as misses, not wrong positives: creation metadata is immutable and keyed by exact id behind the transcript-name gate, the ENOENT-during-identity window returns "already gone" with re-inspection on reappearance, and the restore path does one catalog lookup under the lifecycle lock (the removed pre-lock probe was only a stale fallback, so nothing is lost).
- Parent-id resolution is gated by the SAFE_TRANSCRIPT_NAME_PATTERN + Windows-device check at the one read site every consumer shares, so transcript-derived ids cannot construct a path outside the store.
Two non-blocking observations for follow-up:
assertSessionLoadable's allowActiveConflict branch discards a successfulfindSessionIdIgnoringCasere-probe and still throwsSessionConflictError— if a concurrent delete/unarchive RESOLVES the conflict between the two probes, the client sees a permanent-looking 409 for a now-loadable session. Worst case equals pre-PR behavior (not a regression), but using the re-probe's result would deliver the race tolerance this PR is for.- Active-copy preference is positional — no writer provenance/integrity distinguishes the two copies. That matches the existing single-uid local-storage trust model (the archived store was always a trusted read source, and this PR creates no new capability), but it is worth stating that rule in the design record, ideally with a log line when a same-spelled both-states conflict is resolved by preference.
CI is green on head (Test ubuntu, Serve A/B, real-daemon E2E, Desktop Shell, secret scan all pass).
|
Released in v0.22.0. |
wenshao
left a comment
wenshao
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round:
packages/cli/src/serve/routes/session.ts:1812 — [review] Please add route-level coverage for the internal-live-owner versus ordinary-runtime ambiguity check; current tests do not cover unqualified transcript/export with both owners present.
[Critical] packages/cli/src/serve/routes/session.ts — The initial persisted internal-runtime scan calls activeInRuntime(runtime) with the default strict conflict handling. After a daemon restart, a non-resident internal Conversations session that exists under the exact ID in both active and archived storage has no live-owner entry, so this scan records session_conflict and unqualified transcript/export returns 409 instead of selecting the active copy. Pass allowActiveConflict=true for this read-only probe while preserving metadata, generation, and cross-runtime ambiguity checks. (中文：daemon 重启后，未驻留的 internal Conversations 会话若同一 ID 同时存在于 active 和 archived，会被初始扫描按严格冲突处理并返回 409；读取探测应允许精确冲突但仍保留元数据、generation 和跨 runtime 歧义检查。)
[Critical] packages/cli/src/serve/routes/session.ts — The final ordinary-runtime scan also calls activeInRuntime(runtime) strictly. With no live owner and an exact active/archive pair only in a secondary ordinary runtime, unqualified transcript/export returns 409. A focused probe reproduced expected 409 to be 200; changing this scan to activeInRuntime(runtime, true) made it pass while activeRuntimes.length still preserves cross-runtime ambiguity detection. (中文：最终 ordinary runtime 扫描同样使用严格模式；当精确 active/archive 对只存在于次级 ordinary runtime 时会错误返回 409。定向探针复现，改为允许精确冲突后通过，且后续长度检查仍保留跨 runtime 歧义检测。)
中文说明
仅完成部分审查，审查缺口已披露。
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛姿态下延后（第 9 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
[Critical] packages/cli/src/serve/routes/session.ts — The initial persisted internal-runtime scan calls activeInRuntime(runtime) with the default strict conflict handling. After a daemon restart, a non-resident internal Conversations session that exists under the exact ID in both active and archived storage has no live-owner entry, so this scan records session_conflict and unqualified transcript/export returns 409 instead of selecting the active copy. Pass allowActiveConflict=true for this read-only probe while preserving metadata, generation, and cross-runtime ambiguity checks. (中文：daemon 重启后，未驻留的 internal Conversations 会话若同一 ID 同时存在于 active 和 archived，会被初始扫描按严格冲突处理并返回 409；读取探测应允许精确冲突但仍保留元数据、generation 和跨 runtime 歧义检查。)
[Critical] packages/cli/src/serve/routes/session.ts — The final ordinary-runtime scan also calls activeInRuntime(runtime) strictly. With no live owner and an exact active/archive pair only in a secondary ordinary runtime, unqualified transcript/export returns 409. A focused probe reproduced expected 409 to be 200; changing this scan to activeInRuntime(runtime, true) made it pass while activeRuntimes.length still preserves cross-runtime ambiguity detection. (中文：最终 ordinary runtime 扫描同样使用严格模式；当精确 active/archive 对只存在于次级 ordinary runtime 时会错误返回 409。定向探针复现，改为允许精确冲突后通过，且后续长度检查仍保留跨 runtime 歧义检测。)
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| if ( | ||
| error instanceof SessionIdCaseConflictError && | ||
| error.reason === 'case_conflict' && | ||
| error.candidateSessionId === sessionId |
There was a problem hiding this comment.
[Critical] A sole legacy uppercase UUID persisted in both active and archived storage is still unrestorable. loadSession/resumeSession normalize the request to lowercase before this comparison, while findSessionIdIgnoringCase() returns the uppercase persisted spelling; candidateSessionId === sessionId therefore fails and returns session_conflict although there is only one spelling. A focused probe reproduced both failures, and returning the sole persisted candidate made both pass. Please represent a sole spelling present in both states separately and restore that persisted spelling without requiring equality to the normalized request.
中文
[严重] 仅以旧版大写 UUID 拼写同时存在于 active 和 archived 中的会话仍无法恢复。loadSession/resumeSession 会先把请求规范化为小写，而 findSessionIdIgnoringCase() 返回持久化的大写拼写，因此这里的 candidateSessionId === sessionId 会失败并返回 session_conflict，即使实际上只有一种拼写。定向探针复现了 load 和 resume 的失败；返回唯一持久化候选后两者均通过。请把“同一唯一拼写同时存在于两个状态”建模为独立结果，并直接恢复该持久化拼写，不要要求它等于规范化后的请求。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| // moved after it was already read correctly. | ||
| return ( | ||
| (await store.readCreationMetadataIfReadable(sessionId, 'active')) ?? | ||
| (await store.readCreationMetadataIfReadable(sessionId, 'archived')) |
There was a problem hiding this comment.
[Critical] This fallback can classify the selected active transcript with metadata from the archived copy. readCreationMetadataIfReadable() returns undefined not only for absence, but also for incomplete/corrupt transcripts and suppressed read errors. In an exact active/archive conflict, callers then load the active copy while using archived sourceType, sourceId, or parentSessionId, so ownership and lineage can come from a different transcript. Please distinguish an archive-move absence from a present-but-unreadable active file and require metadata from the selected active copy.
中文
[严重] 这里的回退可能用 archived 副本的元数据来分类实际选中的 active transcript。readCreationMetadataIfReadable() 不仅在文件不存在时返回 undefined，在 transcript 不完整、损坏或读取错误被吞掉时也会返回。对于 active/archive 精确冲突，调用方随后加载 active 副本，却使用 archived 的 sourceType、sourceId 或 parentSessionId，导致所有权和血缘来自另一个 transcript。请区分“因归档移动而暂时不存在”和“active 文件存在但不可读”，并要求元数据来自实际选中的 active 副本。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| return ( | ||
| (await store.readCreationMetadataIfReadable(sessionId, 'active')) ?? | ||
| (await store.readCreationMetadataIfReadable(sessionId, 'archived')) | ||
| ); |
There was a problem hiding this comment.
[Critical] The fixed active-then-archived probe loses a transcript that is unarchived between the two reads. If the active read misses while the file is archived, then another process moves it to active before the archived read, both reads return undefined and callers classify the now-active session as absent. A deterministic probe reproduced this; retrying active after the archived miss fixed it. Please use a bounded read/recheck loop that covers both archive directions.
中文
[严重] 固定的 active→archived 探测顺序会漏掉在两次读取之间被取消归档的 transcript。第一次读取 active 时文件仍在 archived；随后另一个进程把它移到 active；第二次读取 archived 也会 miss，最终两次都返回 undefined，调用方把已经 active 的会话判定为不存在。确定性探针已复现；在 archived miss 后重试 active 可以修复。请使用有界的读取/复查循环，同时覆盖两个移动方向。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| // wider than the storage enumeration pattern: parent ids written by older | ||
| // builds (nil/v6/v7 UUIDs, agent-suffixed ids) must stay resolvable, while | ||
| // path separators can never reach the joined transcript path. | ||
| const SAFE_TRANSCRIPT_NAME_PATTERN = /^[A-Za-z0-9._-]{1,128}$/; |
There was a problem hiding this comment.
[Critical] The new 128-character cap rejects valid persisted agent-suffixed parent IDs. isValidSessionId accepts an unbounded -agent-... suffix, and IDs longer than 128 characters can still fit safely below the filesystem component limit with .jsonl; this gate rejects them before consulting storage, so children lose lineage classification. Please validate a safe single path component without an arbitrary 128-character cap, or derive the limit from the actual filename limit.
中文
[严重] 新增的 128 字符上限会拒绝合法且已经持久化的 agent 后缀父会话 ID。isValidSessionId 接受不限长度的 -agent-... 后缀，而超过 128 字符的 ID 加上 .jsonl 后仍可能安全低于文件系统组件上限；这里却在访问存储前直接拒绝，导致子会话丢失血缘分类。请仅校验安全的单一路径组件，不要使用任意的 128 字符上限，或根据真实文件名上限推导限制。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| for (const ordinaryRuntime of workspaceRegistry.list()) { | ||
| const ordinaryService = | ||
| createWorkspaceRuntimeSessionService(ordinaryRuntime); | ||
| if (await ordinaryService.sessionExistsInAnyState(sessionId)) { |
There was a problem hiding this comment.
[Critical] This ordinary-runtime collision scan is exact-case only. On a case-sensitive filesystem, an internal runtime can own lowercase id while an ordinary runtime persists uppercase ID; sessionExistsInAnyState(id) calls exact-case getSessionLocation() and returns false, so the internal transcript is selected instead of ambiguous_session_owner. Please use case-insensitive any-state resolution here while preserving conflict and I/O semantics.
中文
[严重] 这里对普通 runtime 的碰撞扫描只检查完全相同的大小写。在大小写敏感文件系统上，internal runtime 可以拥有小写 id，而 ordinary runtime 持久化大写 ID；sessionExistsInAnyState(id) 只调用精确大小写的 getSessionLocation()，因此返回 false，最终选择 internal transcript，而不是报告 ambiguous_session_owner。请改用大小写不敏感的任意状态解析，并保留冲突和 I/O 错误语义。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| if (location === 'conflict') { | ||
| if (options.allowActiveConflict) { | ||
| try { | ||
| await service.findSessionIdIgnoringCase(sessionId); |
There was a problem hiding this comment.
[Critical] A disappearing archived half still turns a now-unambiguous active session into session_conflict. After the initial location read sees an exact pair, findSessionIdIgnoringCase() can return the sole active ID normally if the archived file disappears; this return value is ignored, so execution falls through to SessionConflictError. Please consume the resolver result and return active when it resolves to the requested persisted spelling.
中文
[严重] archived 一半在解析期间消失时，已经变得无歧义的 active 会话仍会被错误地转成 session_conflict。初始位置读取看到精确冲突后，如果 archived 文件消失，findSessionIdIgnoringCase() 会正常返回唯一 active ID；但这里忽略了返回值，随后直接落入 SessionConflictError。请使用解析结果，在它解析到请求的持久化拼写时返回 active。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| error.reason === 'case_conflict' && | ||
| error.candidateSessionId === sessionId | ||
| ) { | ||
| return 'active'; |
There was a problem hiding this comment.
[Critical] candidateSessionId === sessionId does not prove the exact active/archive pair is the only case-equivalent spelling. findSessionIdIgnoringCase() throws immediately when it encounters that exact pair, before examining a second mixed-case transcript, and this branch then selects active despite genuine ambiguity. Please enumerate all matching spellings first and expose the exact-pair exception only when no additional spelling exists.
中文
[严重] candidateSessionId === sessionId 并不能证明 active/archive 精确对是唯一的大小写等价拼写。findSessionIdIgnoringCase() 遇到该精确对时会立即抛出，尚未检查另一个混合大小写 transcript；这里随后仍选择 active，掩盖了真实歧义。请先枚举全部匹配拼写，仅在不存在额外拼写时才返回“精确对可接受”的结果。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| export async function assertSessionRestorable( | ||
| workspaceCwd: string, | ||
| sessionId: string, | ||
| requestedSessionId: string, |
There was a problem hiding this comment.
[Critical] The final guarded restore check no longer revalidates case-insensitive ambiguity. A spelling can be resolved first, then another case-equivalent transcript can appear before assertSessionRestorable(); this helper checks only the selected exact spelling and restores it although ownership is now ambiguous. A focused probe resolved 'active' instead of rejecting, while final case-insensitive re-resolution fixed it. Please combine resolution and validation under the final guard or repeat the case-insensitive check here.
中文
[严重] 最终受保护的恢复检查不再重新验证大小写不敏感歧义。先解析出某个拼写后，另一个大小写等价 transcript 可以在 assertSessionRestorable() 前出现；该函数只检查已选中的精确拼写，因此即使所有权已经歧义仍会恢复。定向探针中它错误地返回了 'active'；在最终阶段重新执行大小写不敏感解析后通过。请把解析和验证合并到最终 guard 内，或在这里再次执行大小写不敏感检查。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
| throw new SessionWriterShutdownError(); | ||
| } | ||
| if (location === 'conflict' || location === 'archived') { | ||
| if (location === 'archived') { |
There was a problem hiding this comment.
[Critical] The newly added collected regression test currently fails. releases a pending lease while a real baseline read is gated cannot find pending-baseline.lock and then reports an unhandled SessionWriterUnavailableError; the isolated changed suite reported this failure after all scoped builds succeeded. Please reconcile conflict adoption with pending lease acquisition/cleanup so the lock remains present while the baseline read is gated and initialization does not reject unhandled.
中文
[严重] 本 PR 新增且会被收集执行的回归测试目前失败。releases a pending lease while a real baseline read is gated 找不到 pending-baseline.lock，随后又出现未处理的 SessionWriterUnavailableError；所有相关构建成功后，隔离运行的变更测试套件仍复现该失败。请协调冲突接管与 pending lease 的获取/清理，使基线读取被阻塞时锁仍存在，并避免初始化产生未处理 rejection。
— gpt-5.6-sol@954e5164 via Qwen Code /review (v0.22.0)
5 participants
What this PR does
Restores four session-lifecycle behaviors regressed by the PR2A provenance and identity tightening, while keeping broader mixed-case daemon compatibility out of scope.
REST, daemon ACP, and embedded ACP load/resume recover an exact same-spelling session left readable in both active and archived storage from the active copy, including writer-lease activation. Transcript and export reads follow the same active-copy rule, including workspace-qualified internal conversation routes. Differently spelled or ambiguous case conflicts remain rejected; archive and unarchive remain strict, while explicit delete acquires the writer lease and removes both copies.
Creation-metadata reads tolerate a concurrent active-to-archive move and resolve legacy parent identifiers within a path-safe transcript-name boundary. Conversation directories that disappear during identity inspection are treated as already gone.
Case-insensitive persisted-session lookup uses asynchronous directory reads, and restore performs the catalog lookup once under the lifecycle lock instead of probing before and inside the lock. The E2E plan lowercases
uuidgenoutput so fixtures match Qwen Code's canonical lowercase session-ID format.Why it's needed
After PR2A tightening, interrupted archive operations could leave a readable session that load/resume refused; transcript/export reads and delete then offered inconsistent recovery behavior. Archive moves could make creation metadata appear missing, legacy child sessions could lose parent lineage, and concurrent directory deletion could surface as an identity error. These are the four regressions tracked by #9489. The restore path also scanned the transcript catalog twice, adding avoidable I/O and another race window.
Normal Qwen Code-generated session IDs are already lowercase, so broader mixed-case Live task, listing, organization, and scheduled-task compatibility is intentionally excluded from this PR.
Reviewer Test Plan
How to verify
session_conflict, and that archive/unarchive remain strict for any both-state conflict.uuidgensnippets and confirm the generated session IDs are lowercase.Latest verification passed: 176/176 session-service tests; 120/120 multi-workspace session tests; full workspace build and typecheck; and changed-file ESLint, Prettier, and diff checks. The current PR SHA has no failing CI checks; every check that executed is passing.
Evidence (Before & After)
N/A — backend session-lifecycle fix; no TUI or user-interface changes.
Tested on
Environment
Node.js v22.22.3 on macOS; package-local Vitest plus full workspace build and typecheck.
Risk & Scope
Linked Issues
Fixes #9489
中文说明
本 PR 做了什么
恢复 PR2A provenance 与 identity 收紧后回归的四项 session 生命周期行为，同时明确不在本 PR 中扩展 daemon 的通用大小写兼容能力。
当完全相同拼写的 session ID 同时存在于 active 与 archived 存储且两份都可读时，REST、daemon ACP 与 embedded ACP 的 load/resume 都以 active 副本恢复，包括 writer lease 激活流程。Transcript 与 export 读取遵循相同的 active 副本语义，workspace-qualified 的内部 conversation 路由也一致。拼写不同或存在歧义的大小写冲突仍然拒绝；archive/unarchive 继续严格拒绝，而显式 delete 会先获取 writer lease，再删除两份副本。
创建元数据读取可以容忍并发的 active-to-archive 移动，并在路径安全的 transcript 名称边界内解析旧版 parent 标识。Conversation 目录在 identity 检查期间消失时，会被视为已经删除。
持久化 session 的大小写无关查询改用异步目录读取；restore 在生命周期锁内只执行一次 catalog 查询，不再在加锁前和锁内重复探测。E2E 计划会把
uuidgen输出转成小写，使测试 fixture 与 Qwen Code 的 canonical 小写 session ID 格式一致。为什么需要
PR2A 收紧后，中断的 archive 操作可能留下仍可读取、但 load/resume 拒绝恢复的 session；transcript/export 读取与 delete 也因此出现不一致的恢复行为。Archive 移动可能让创建元数据暂时表现为缺失，旧版 child session 可能丢失 parent lineage，并发删除目录可能暴露为 identity 错误。这些就是 #9489 跟踪的四项回归。Restore 路径还会重复扫描 transcript catalog，既增加不必要的 I/O，也扩大了竞态窗口。
Qwen Code 正常生成的 session ID 已经是小写，因此更广泛的 mixed-case Live task、列表、organization 与 scheduled-task 兼容性明确不属于本 PR 范围。
Reviewer 测试计划
如何验证
session_conflict，且 archive/unarchive 对任何双状态冲突都继续严格拒绝。uuidgen片段，确认生成的 session ID 为小写。最新验证已通过：session-service 测试 176/176；multi-workspace session 测试 120/120；全工作区 build 与 typecheck；以及改动文件的 ESLint、Prettier 和 diff 检查。当前 PR SHA 没有失败的 CI check，所有实际执行的 check 均已通过。
前后证据
N/A — 后端 session 生命周期修复，没有 TUI 或用户界面变化。
测试环境
环境信息
macOS 上的 Node.js v22.22.3；执行了 package-local Vitest、全工作区 build 与 typecheck。
风险与范围
关联 Issue
Fixes #9489