Skip to content

fix(serve): Canonicalize Live task bridge session IDs - #9819

Merged
doudouOUC merged 4 commits into
QwenLM:mainfrom
doudouOUC:fix/live-task-session-id-canonicalization
Aug 24, 2026
Merged

fix(serve): Canonicalize Live task bridge session IDs#9819
doudouOUC merged 4 commits into
QwenLM:mainfrom
doudouOUC:fix/live-task-session-id-canonicalization

Conversation

@doudouOUC

Copy link
Copy Markdown
Collaborator

What this PR does

This PR gives each Live task two explicit identities: the persisted transcript keeps its authoritative on-disk spelling, while every in-memory bridge operation uses the existing canonical caller-visible UUID. Live owner lookup, summary lookup, resume, prompt dispatch, Conversations directory relocation, and event polling now converge on that canonical bridge ID without changing the task ID returned on the wire.

Why it's needed

A legacy transcript can retain an uppercase UUID while ordinary REST or ACP restore registers the same session under its lowercase canonical UUID. Live task lookup previously carried the persisted spelling into the bridge, so it could miss an already-running canonical entry and resume a second entry for the same logical transcript. The duplicate entries shared one canonical Conversations directory but had separate prompt and event state. This closes the residual Live/bridge identity issue tracked in #9490 without broadening the lock-key or storage lookup work that remains there.

Reviewer Test Plan

How to verify

  • Seed an uppercase persisted UUID and register the lowercase UUID as an existing bridge entry. Address the task through the uppercase spelling and confirm no second resume occurs; prompt dispatch and event polling must use the lowercase entry.
  • Repeat with no resident entry. Confirm transcript metadata is still read through the uppercase persisted spelling, while resume, Conversations directory materialization and relocation, prompt dispatch, and event subscription all use the lowercase canonical UUID.
  • Confirm non-UUID and already-canonical task IDs retain their existing behavior.
  • The focused Live task suite passes 21 tests. The existing ACP mixed-case canonical bridge-key test and REST authoritative persisted-spelling test also pass. The full repository build and typecheck, changed-file Prettier and ESLint checks, and git diff --check pass.

Evidence (Before & After)

N/A — internal daemon identity handling with no TUI change.

Tested on

OS Status
🍏 macOS ✅ tested
🪟 Windows ⚠️ not tested
🐧 Linux ⚠️ not tested

Environment (optional)

macOS 26.4.1 arm64, Node.js v22.22.3, npm 10.9.8; unit tests and repository build/typecheck.

Risk & Scope

  • Main risk or tradeoff: A Live task may now expose its persisted spelling on the wire while using a different canonical key internally. The change keeps that split explicit and applies the canonical key only at bridge-facing consumers; storage and metadata reads retain the authoritative persisted spelling.
  • Not validated / out of scope: Windows and Linux were not run locally. General mixed-case storage discovery, UUIDv7 and undashed legacy lock-key folding, case-conflict handling, list merging, and the reserved standalone source enforcement items in PR2A follow-ups: enforcement breadth, lock-key folding, directory race adoption, integrity budget, error serialization #9490 remain out of scope. PR fix(serve): Repair persisted session lifecycle #9626's persisted lifecycle repair is unchanged.
  • Breaking changes / migration notes: None. Returned task IDs and persisted transcript names are unchanged.

Linked Issues

Part of #9490.

中文说明

本 PR 做了什么

本 PR 为每个 Live task 明确区分两种身份:持久化 transcript 保留磁盘上的权威拼写,所有内存 bridge 操作则统一使用现有的调用方可见 UUID 规范形式。Live owner 查询、summary 查询、恢复、prompt 分发、Conversations 目录迁移和事件轮询现在都会收敛到同一个 canonical bridge ID,同时不改变协议返回的 task ID。

为什么需要

历史 transcript 可能保留大写 UUID,而普通 REST 或 ACP 恢复会把同一会话注册到小写 canonical UUID 下。此前 Live task 查询会把持久化拼写继续传给 bridge,因此可能漏掉已经运行的小写 entry,并为同一逻辑 transcript 再恢复一个 entry。两个 entry 共享同一个规范化 Conversations 目录,却拥有独立的 prompt 与事件状态。本 PR 收口 #9490 中遗留的 Live/bridge 身份问题,不扩展该 issue 中仍待处理的锁 key 或存储查找工作。

Reviewer 测试计划

如何验证

  • 写入一个大写 UUID 的持久化会话,并把对应小写 UUID 注册为现有 bridge entry。通过大写拼写访问任务,确认不会发生第二次恢复;prompt 分发和事件轮询必须使用小写 entry。
  • 在没有 resident entry 的情况下重复测试。确认 transcript metadata 仍通过大写持久化拼写读取,而恢复、Conversations 目录创建与迁移、prompt 分发和事件订阅全部使用小写 canonical UUID。
  • 确认非 UUID 以及已经规范化的 task ID 保持原有行为。
  • 聚焦的 Live task 测试套件通过 21 项测试。现有 ACP mixed-case canonical bridge-key 测试和 REST 权威持久化拼写测试也通过。完整仓库 build、typecheck、变更文件的 Prettier 与 ESLint 检查以及 git diff --check 均通过。

证据(Before & After)

N/A —— 内部 daemon 身份处理,无 TUI 变化。

测试平台

OS 状态
🍏 macOS ✅ 已测试
🪟 Windows ⚠️ 未测试
🐧 Linux ⚠️ 未测试

环境(可选)

macOS 26.4.1 arm64、Node.js v22.22.3、npm 10.9.8;单元测试以及仓库 build/typecheck。

风险与范围

关联 Issue

#9490 的部分修复。

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

E2E test report

Scope

Verified that legacy uppercase transcript spelling remains the storage identity while Live task operations converge on one lowercase canonical bridge entry.

Baseline and reproduction

  • Global qwen v0.21.15 exposes the required serve/session surface, but a true CLI E2E would require seeding an uppercase transcript into the private internal Conversations runtime and invoking the internal Live task tool surface. That is not available as a stable public CLI workflow, so the deterministic package harness was used as the reproduction fallback.
  • Before the fix, the two new send-message cases fail: an existing lowercase bridge entry is resumed again under the uppercase persisted spelling, and a cold uppercase transcript is resumed with the uppercase bridge key.
  • After the fix, the existing canonical entry is reused. Cold resume, prompt dispatch, Conversations directory relocation, and wait/event polling use the lowercase canonical UUID, while metadata reads retain the uppercase persisted spelling.

Results

  • cd packages/cli && npx vitest run src/serve/live/live-task-service.test.ts --coverage.enabled=false — 21/21 passed.
  • ACP mixed-case canonical bridge-key contract — 1/1 passed.
  • REST authoritative persisted-spelling contract — 1/1 passed.
  • npm run build — passed.
  • npm run typecheck — passed.
  • Changed-file Prettier and ESLint checks plus git diff --check — passed.

Platform coverage

  • macOS 26.4.1 arm64: tested.
  • Windows and Linux: not tested locally; GitHub CI is the cross-platform signal.

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Qwen Triage finishedview run. See the stage comments in this thread for the result.

Qwen Triage 已完成 —— 查看运行。结果见本线程中的各阶段评论。

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

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 @qwen-code /review. See workflow logs.

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

(Re-run on head e066ac08: the gate findings below are unchanged. Since the previous pass the branch gained two commits — a test-only pin of the canonical owner lookup (2a6fd37), then the round-3 hardening (e066ac0) answering the four Critical findings from the /review round submitted under @wenshao's account.)

Template looks good ✓

Problem: observed and maintainer-reported. This is the queued Live/bridge identity residual from the #9341 review, tracked in #9490: a legacy uppercase transcript and its lowercase canonical bridge entry can coexist for one logical session, with shared Conversations directory but separate prompt and event state. The mechanism is documented in the #9490 thread, and the PR's tests are written to fail on the pre-fix behavior.

Direction: aligned — unchanged from the previous pass. REST internal restore and ACP session/load already key the bridge on the canonical lowercase UUID; the Live task path was the last caller that didn't converge. The wire contract stays intact: returned task IDs and persisted transcript names keep their spelling. Deferring the broader lock-key / storage-discovery items matches what #9490 queued.

Size: not a Stage 0 core path (single package, packages/cli/src/serve/live/). 115 production lines (coordinator +2/-1, task-service +71/-41) + 323 test lines across 4 files. Well under every threshold.

Approach: still minimal, and the new delta is reviewer-driven end to end: validate persisted ownership before accepting a canonical live owner, reject case-only twins as ambiguous, carry the caller-visible spelling separately for the wire and for metadata reads in the disappearance race, and register voice-resumed sessions by the canonical id. Each edit maps to a round-3 finding; no scope creep.

Risk: no elevated risk signals — none of the Stage 1e high-risk paths are touched.

Moving on to code review. 🔍

中文说明

感谢贡献!

(在头 e066ac08 上 re-run:以下门控结论不变。上一轮之后分支新增两个提交——先是一个仅测试的 canonical owner 查询钉固(2a6fd37),然后是回应 @wenshao 账号下 /review 轮次四条 Critical 发现的第 3 轮加固(e066ac0)。)

模板完整 ✓

问题:已观测、由维护者报告。这是 #9341 评审排队在 #9490 的 Live/bridge 身份遗留项:历史大写 transcript 与其小写 canonical bridge entry 可能同时挂在同一逻辑会话上,共享同一个 Conversations 目录,却有各自独立的 prompt 与事件状态。机制记录在 #9490 线程中,PR 的测试按修复前会失败的方式编写。

方向:对齐——与上一轮一致。REST 内部恢复与 ACP session/load 早已用规范小写 UUID 作为 bridge 键,Live task 路径是最后一个未收口的调用方。协议契约不变:返回的 task ID 与持久化 transcript 名称保持原拼写。不扩展锁键/存储发现范围,与 #9490 排队范围一致。

规模:不属于 Stage 0 核心路径(单包 packages/cli/src/serve/live/)。115 行生产代码(coordinator +2/-1、task-service +71/-41)+ 323 行测试,共 4 个文件,远低于所有阈值。

方案:依然是最小化的,且新 delta 完全由评审意见驱动:先验证持久化归属再接受 canonical live owner、大小写孪生冲突按歧义拒绝、为协议返回与消失竞态中的 metadata 读取单独保留调用方拼写、语音恢复会话以规范 id 注册。每处改动都对应第 3 轮的一条发现,无范围蔓延。

风险:无升级风险信号——未触及任何 Stage 1e 高风险路径。

进入代码审查 🔍

Qwen Code · qwen3.8-max

Reviewed at e066ac08852de69cf39419ea1dbf11b9a9b3bc88 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Code review

The review question this pass was narrower than "is the design right" — the previous pass settled that — it was: do the four round-3 Critical findings actually get fixed? Before reading the delta, my independent answers were: R3-1, canonicalize at the voice-resume producer (resumeSession call site) since the coordinator tracks whatever id the bridge returns; R3-2, run the exact-spelling storage-ownership scan before accepting a canonical live owner and reject conflicting owners; R3-3, carry the caller-visible spelling on the located task and use it for the latestTurn fallback; R3-4, read creation metadata with the persisted spelling since the live summary's id is canonical. The delta does exactly these four things — no simpler path missed, nothing extra.

Verified on head e066ac08:

  • R3-1 (voice resume)live-session-coordinator.ts now calls resumeSession({ sessionId: normalizeSessionIdForLookup(candidate.sessionId), … }) and tracking continues from resumed.sessionId, so a legacy uppercase transcript resumes into the lowercase bridge entry instead of spawning a twin. The new coordinator test asserts resumeSession is never called with the persisted uppercase spelling.
  • R3-2 (case-only-twin cross-routing)locateTask now calls findStoredTaskRuntimes(threadId) (exact-spelling sessionExists scan) first for any non-canonical id: more than one stored owner → ambiguous; exactly one stored owner while a different runtime owns the canonical live entry → ambiguous. Otherwise the stored owner wins. Already-canonical ids skip the scan entirely (bridgeSessionId === threadId), so the fast path is structurally untouched. The new test rejects a /conversations-stored vs /project-resident mismatch with Task id is ambiguous and confirms no prompt was dispatched.
  • R3-3 (wire leak)LocatedTask gains threadId (the caller/persisted spelling); latestTurnId() falls back to it instead of task.summary.sessionId. Grepped the head file: zero remaining summary.sessionId reads, so an empty uppercase transcript can no longer return a lowercase latestTurn.id beside an uppercase thread.id. The new no-user-turn test pins the spellings together.
  • R3-4 (disappearance race)ensureResident reads creation / live-conversation metadata via task.threadId (persisted spelling) while resume, directory materialization, cwd change, and prompt dispatch keep task.bridgeSessionId. The new test deterministically reproduces the window (summary lookup succeeds in locateTask, entry gone by ensureResident) and asserts metadata was read through the uppercase spelling while resume/prompt used the lowercase one.

The 2a6fd37 pin also holds: the event-polling test now asserts resolveLiveSessionOwner is called with the canonical id and never with the persisted spelling. Cost note: the stored-owner scan walks all workspaces, but only for non-canonical caller ids — a bounded, rare path. One standing non-blocking residue, unchanged from prior passes and explicitly deferred to the remaining #9490 items: the list-scan fallback matches item.sessionId === threadId exactly, so cross-spelling storage discovery remains open work. No critical findings on this head.

Test evidence — the PR's own CI

All pull_request workflow runs on this head completed green, including the unit suite on the reviewed commit: Test (ubuntu-latest, Node 22.x) ✅. The macOS/Windows unit jobs and the no-sandbox CLI integration tests stayed skipped behind fork-runner approval, same as prior passes — low-risk with the platform-independent suite green on Ubuntu.

Check Conclusion
Test (ubuntu-latest, Node 22.x) ✅ success
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Live Host (macos-latest) ✅ success
Real daemon E2E / Java 11 ✅ success
Secret scan (TruffleHog) ✅ success
Serve A/B (ubuntu-latest, Node 22.x) ✅ success
ubuntu-latest / Java 11 ✅ success
ubuntu-latest / Java 17 ✅ success
ubuntu-latest / Java 21 ✅ success
macos-latest / Java 21 ✅ success
windows-latest / Java 21 ✅ success
web-shell E2E Smoke (ubuntu-latest, Node 22.x) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

Two pieces of evidence beyond the unit suite. The Serve A/B job rebuilt base vs this head e066ac08 and reports no response changes across 12 scenarios — the wire-stability claim survives the round-3 hardening against live daemons, not just mocks. And the author's round-3 claims (49 focused Live tests green, build/typecheck green) are corroborated by the CI run on this commit rather than taken on faith.

Sandboxed verification would settle the residual gap: author tested macOS only, and Windows/Linux local runs are not verified — a sponsored @qwen-code /verify run is already in flight on this head (actions/runs/32706008023) and will post its own A/B load-bearing report here when it completes; read it with the usual fork-code skepticism.

Real-scenario testing: N/A for this run — unattended CI, and the change is internal daemon identity handling with no user-visible surface.

中文说明

代码审查:本轮的问题比"设计是否正确"更聚焦——上一轮已经解决设计问题——本轮要回答的是:第 3 轮的四条 Critical 发现是否真正被修复? 在读 delta 之前,我的独立方案是:R3-1 在语音恢复的生产端(resumeSession 调用点)做规范化,因为 coordinator 后续跟踪的是 bridge 返回的 id;R3-2 先做精确拼写的存储归属扫描、再接受 canonical live owner,冲突即拒绝;R3-3 在定位结果上单独携带调用方可见拼写,latestTurn 回退用它;R3-4 创建 metadata 用持久化拼写读取(live summary 的 id 是 canonical 的)。delta 恰好做了这四件事——没有漏掉更简路径,也没有夹带。

已在头 e066ac08 上逐条核实:R3-1——coordinator 以 normalizeSessionIdForLookup(candidate.sessionId) 调用 resumeSession,并从 resumed.sessionId 继续跟踪,大写历史 transcript 恢复进小写 bridge entry 而不再产生孪生,新测试断言从不以大写拼写调用;R3-2——locateTask 对非规范 id 先做 findStoredTaskRuntimes(精确拼写扫描),存储归属多于一个、或唯一存储归属与 canonical live owner 分属不同 runtime,都判为歧义;已规范 id 完全跳过扫描(bridgeSessionId === threadId),快路径结构上未动,新测试确认冲突被拒且未分发 prompt;R3-3——LocatedTask 新增 threadIdlatestTurnId() 回退到它,头文件中已无任何 summary.sessionId 读取(已 grep 确认),大写空 transcript 不会再在大写 thread.id 旁返回小写 latestTurn.id;R3-4——ensureResidenttask.threadId(持久化拼写)读 metadata,resume/目录实体化/cwd 变更/prompt 分发继续用 task.bridgeSessionId,新测试确定性地复现了消失窗口并分别断言两种拼写的去向。

2a6fd37 的钉固同样成立:事件轮询测试断言 resolveLiveSessionOwner 以 canonical id 调用、从不以持久化拼写调用。成本说明:存储归属扫描会遍历所有 workspace,但只对非规范调用方 id 触发——有界且罕见。遗留的非阻塞残留与前几轮一致、且 PR 已明确交给 #9490 剩余项:列表扫描回退按精确拼写匹配,跨拼写存储发现仍是待办。本头无阻塞发现。

测试证据:本头所有 pull_request 工作流均已完成且为绿色,包括被审提交上的单元测试套件 Test (ubuntu-latest, Node 22.x) ✅。macOS/Windows 单测与无沙箱 CLI 集成测试仍因 fork runner 批准而跳过——鉴于平台无关套件已在 Ubuntu 跑绿,风险较低。单测之外两项证据:Serve A/B 任务针对本头 e066ac08 重建 base 对比,12 个场景响应零变化——协议稳定性主张在真实 daemon 上经受住了第 3 轮加固;作者自报的第 3 轮验证结果(49 项聚焦 Live 测试、build/typecheck 通过)由本提交的 CI 运行佐证,而非仅凭声明。沙箱验证可补齐剩余缺口:作者仅在 macOS 上测试,Windows/Linux 本地运行未验证——一次由维护者赞助的 @qwen-code /verify 已在本头上运行中,完成后会在本帖发布 A/B 承重报告;请以对待 fork 代码的惯常审慎阅读该报告。

真实场景测试:本次不适用——无人值守 CI,且改动为内部 daemon 身份处理,无用户可见面。

Qwen Code · qwen3.8-max

Reviewed at e066ac08852de69cf39419ea1dbf11b9a9b3bc88 · re-run with @qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — the four round-3 Critical findings are each answered by a targeted fix plus a pinning regression test, verified line-by-line on this head, with CI green and the Serve A/B wire check clean on it; what keeps this from a 5 is that @wenshao's changes-requested round is formally still open — my read is that every finding is addressed, but dismissing that review is a human call.

Stepping back: this is what a good review-response cycle looks like. Round 3 found four real regressions the original canonicalization introduced or exposed — the voice producer registering the persisted spelling, case-only twins cross-routing between workspaces, the canonical summary leaking onto the wire, and the disappearance race demoting a canonical id to a storage key — and the answer was four surgical fixes with one deterministic test each, not a rewrite. My independent proposal for every one of them matches what landed. Re-checking the final state: the ambiguity checks are symmetric, already-canonical lookups take exactly the same path as before this PR, there are no remaining summary.sessionId reads to leak a spelling, and the diff carries nothing beyond the findings it answers. Six months from now this reads as a careful convergence, not a tangle.

What would keep me honest: the stored-owner scan adds a workspace walk for non-canonical ids (bounded and rare, noted in Stage 2), the exact-spelling list-scan residue remains open under #9490 as the PR states, and the author's macOS-only local testing is covered by the Ubuntu CI run rather than duplicated. None of that blocks.

Verdict: approve, pinned to e066ac08. All pull_request CI runs on this head are complete and green, so no deferred-approval marker is needed. Remaining gates after this: @wenshao to re-check round 3 and dismiss the changes-requested review if satisfied, and the second human approval main requires. The sponsored sandboxed verification still in flight will post its own report; if it surfaces anything, that thread is where to take it.

中文说明

置信度:4/5 —— 第 3 轮的四条 Critical 发现各自都有针对性修复加钉固回归测试作答,已在本头上逐行核实,CI 全绿、Serve A/B 协议检查干净;没给到 5 分是因为 @wenshao 的 changes-requested 轮次在形式上仍未关闭——我的判断是每条发现都已处理,但撤销该评审是人工决定。

整体来看:这是一次高质量的评审响应循环。第 3 轮找出了初版规范化引入或暴露的四个真实回归——语音生产端注册持久化拼写、大小写孪生跨 workspace 误路由、canonical summary 泄漏到协议、消失竞态把 canonical id 当存储键——而回应是四处外科手术式修复、各配一个确定性测试,不是重写。我对每一条的独立方案与最终落地的实现一致。复核最终状态:歧义检查是对称的,已规范化查询走的路径与本 PR 之前完全相同,不再有任何 summary.sessionId 读取会泄漏拼写,diff 也没有超出其所回应的发现。半年后再看,这是一次谨慎的收口,而不是一团乱麻。

如实说明保留意见:存储归属扫描为非规范 id 增加了一次 workspace 遍历(有界且罕见,Stage 2 已述);精确拼写列表扫描的残留按 PR 所述仍挂在 #9490 项下;作者仅在 macOS 本地测试,由 Ubuntu CI 运行覆盖而非重复验证。均不构成阻塞。

结论:批准,钉固在 e066ac08。本头所有 pull_request CI 运行已完成且为绿色,无需延迟批准标记。此后剩余的门槛:请 @wenshao 复核第 3 轮,若满意则撤销 changes-requested 评审;以及 main 要求的第二个人工批准。仍在进行中的赞助沙箱验证会自行发布报告;如有发现,请在那条线程中跟进。

Qwen Code · qwen3.8-max

Reviewed at e066ac08852de69cf39419ea1dbf11b9a9b3bc88 · re-run with @qwen-code /triage

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

⏸️ Deferring to @wenshao — the review itself is clean (the fix matches the queued #9490 scope and pins the right behavior), but this commit's unit suite never ran in CI: the Ubuntu job trips main's pre-existing workflow-size ratchet (cd-cua-driver.yml is 42519 bytes vs the recorded 29715) before vitest starts, and the merge commit will keep failing it until main is fixed. Either fix main's baseline and re-run @qwen-code /triage here, or sponsor @qwen-code /verify on this head to settle the canonical-entry claim. Needs a human call on this one.

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 23, 2026

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head e066ac0, drove a fixed endpoint set against each, and diffed the JSON responses. Only fields that changed are shown.

No response changes against the PR base across 12 scenario(s).

Qwen Code · serve A/B

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed. Suggestions are inline.

Not explored to full depth (tool budget reached): "agent 6c": did not run the new vitest specs ( live-task-service.test.ts ) to confirm they pass..

Not reviewed: reverse audit — stopped before round 4 by the review time budget.

中文说明

仅完成部分审查,审查缺口已披露。 建议见行内评论。

未探索到全部深度(达到工具调用预算):"agent 6c"did not run the new vitest specs ( live-task-service.test.ts ) to confirm they pass.

未审查:反向审计——评审时间预算不足,未能开始第 4 轮。

— qwen3.8-max via Qwen Code /review (v0.22.0)

Comment thread packages/cli/src/serve/live/live-task-service.ts
Comment thread packages/cli/src/serve/live/live-task-service.ts
Comment thread packages/cli/src/serve/live/live-task-service.ts
Comment thread packages/cli/src/serve/live/live-task-service.ts
Comment thread packages/cli/src/serve/live/live-task-service.ts

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking issues. LGTM! ✅

Not explored to full depth (tool budget reached): "agent 6c": did not trace the bridge's restoreSession cold-spawn path end-to-end to confirm which sessionId it passes into the spawned CLI child — Finding 1's mechanism d…; "agent 6a": none — no check was cut short..

Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/serve/live/live-task-service.ts:54 — [review] R1-1 two-spelling invariant (bridge canonical vs storage persisted) undocumented; summary.sessionId spelling is path-dependent
  • packages/cli/src/serve/live/live-task-service.ts:1079 — [review] R1-2 ensureResident missing-metadata SessionNotFoundError now carries the canonical id, unpinned by tests; inconsistent with locateTask failure paths
  • packages/cli/src/serve/live/live-task-service.ts:1143 — [review] R1-3 canonical reference to a legacy mixed-case session resolves only while resident; cold-path storage probes are exact-spelling
  • packages/cli/src/serve/live/live-task-service.ts:1145 — [review] R1-4 no test pins that resolveLiveSessionOwner receives the canonical id; revert mutation survives the suite
  • packages/cli/src/serve/live/live-task-service.ts:808 — [review] R1-5 wait cursors embed the caller's raw id spelling; a spelling flip between polls resets the cursor and misses wake events
中文说明

无阻断问题。LGTM!✅

未探索到全部深度(达到工具调用预算):"agent 6c"did not trace the bridge's restoreSession cold-spawn path end-to-end to confirm which sessionId it passes into the spawned CLI child — Finding 1's mechanism d…"agent 6a"none — no check was cut short.

收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。

— qwen3.8-max via Qwen Code /review (v0.22.0)

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ❌ not passed — blocked (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: 113 passed · 0 failed · 113 total

Flakiness gate: ✅ 1 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:❌ 不通过 · 阻塞(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:113 通过 · 0 失败 · 113 总计

抖动门:✅ 1 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR 9819 — fix(serve): Canonicalize Live task bridge session IDs

Verdict: blocked — 113/113 scripted assertions passed; verified head e519147416d3aca1019e19c15ce97969a592cfcb. The central claim is proven load-bearing by A/B (head 34/34 fixed-behavior assertions, base 30/30 bug-reproduction assertions), the three new tests are non-vacuous and precisely attributed, and every author-cited gate reproduces. The blocker is a regression found by the sibling sweep: live-session-coordinator.ts still registers resumed voice sessions under the persisted spelling, and once LiveTaskService probes only the canonical spelling, addressing such a session through Live tasks resumes a duplicate bridge entry and misroutes prompts and event subscriptions (reproduced on compiled code, §Findings F1).

中文摘要
  • 结论:blocked。PR 的核心主张(Live task 的 bridge 操作统一使用 canonical UUID)经 A/B 证明有效:head 侧 34/34 断言通过(不再二次 resume、prompt/事件轮询走小写 entry、wire 上仍返回持久化拼写);base 侧 30/30 断言复现原 bug(重复 entry、prompt 发往重复 entry、轮询读到过期持久化状态)。三个新测试非空转:在 base 源码上恰好这 3 个测试以行为性 mismatch 变红(18 绿),突变矩阵逐 hunk 归因,仅 1 个幸存 guard(ensureResident 的 resident 探测,串行流程冗余、并发竞态无测试钉住)。
  • 阻塞原因(F1)live-session-coordinator.ts 的语音 resume 路径仍以持久化拼写向 bridge 注册会话,而本 PR 之后 LiveTaskService 只以 canonical 拼写探测。对“大写 legacy transcript + 语音 resume + 之后用 Live task 寻址”这一配置,base 行为正确(复用 entry),head 反而 resume 出第二个 entry 并把 prompt/事件订阅引向它——对 PR 目标人群(legacy 大写拼写)是回归。已给出最小修复(coordinator 注册时 canonicalize)并在 scratch 树测得受影响套件 24/24 不变;语音端到端未驱动,属按构造收敛。
  • 未覆盖:per-commit 归因(shallow clone 不可达)、语音管线端到端、全仓测试、Windows/macOS、PR2A follow-ups: enforcement breadth, lock-key folding, directory race adoption, integrity budget, error serialization #9490 其余项(UUIDv7/无横线 legacy 折叠等,与 PR 声明的 scope 一致)。

Central claim + A/B

Central claim: addressing a mixed-case persisted task through the Live task surface must converge every bridge-facing operation (owner lookup, summary lookup, resume, prompt dispatch, Conversations directory materialization/relocation, event polling) on the canonical UUID, while storage reads and wire responses keep the persisted spelling.

Harness: live-task-ab.mjs drives the compiled LiveTaskService from each dist with a fake AcpSessionBridge encoding the upstream semantics the PR depends on (entries keyed case-sensitively; SessionNotFoundError on unknown ids; exact-id probes) and real on-disk storage (real SessionService reads: sessionExists, loadSession, readCreationMetadata, listSessions, plus the real listWorkspaceSessionsForResponse merge). Witness captures: 01-ab-head-fixed-cells.png, 02-ab-base-buggy-cells.png (raw logs in logs/ab-head.log, logs/ab-base.log).

Cell Head (fixed) Base (buggy)
S1 resident canonical entry, addressed UPPERCASE — resumes [] [UPPER] (duplicate)
S1 — bridge entries after send [lower] [UPPER, lower]
S1 — prompt dispatched to lower UPPER
S2 cold resume — resume/materialize/changeSessionCwd id lower / lower / lower UPPER ×3
S2 — metadata read through persisted spelling (sourceId flowed) yes yes
S2 — wire threadId UPPER UPPER
S3 non-hex id — identical both arms SessionNotFoundError('task-1'), no resume same
S3b undashed 32-hex mixed-case — identical both arms resume/prompt/wire = exact spelling same
S4 canonical lowercase — identical both arms resume/prompt/wire = lowercase same
S5 wait_threads poll — event queries subscribe+epoch+lastEventId on lower; timedOut=true zero queries on lower; immediate inactiveStatus wake from stale persisted summary
S6 read_thread — wire id + turns UPPER, 1 turn UPPER, 1 turn
S7/S8 voice entry registered under UPPERCASE (see F1) duplicate resume + subscribe on id with no entry correct reuse

Head arm: 34/34; base arm: 30/30 (base assertions encode the bug reproduction as the expectation, so its reds are the point).

Reviewer Test Plan walkthrough

Step Result
1. Uppercase persisted + lowercase resident; no second resume; dispatch/poll use lowercase Verified by S1/S5 + unit test reuses the canonical bridge entry… (21/21 at head)
2. No resident entry; metadata via uppercase; resume/materialize/relocation/dispatch/subscription via lowercase Verified by S2 (metadata read proven by sourceId flowing from the UPPERCASE-only transcript file into resumeSession) + unit test uses one canonical bridge id…
3. Non-UUID and already-canonical ids retain behavior Verified by S3/S3b/S4 — byte-identical behavior on both arms
4. Focused suite 21 tests; ACP + REST cited tests; build/typecheck/lint/format Reproduced: 21/21; transport.test.ts "keeps the bridge key canonical while isolating mixed-case storage" 1/1; server.test.ts "reads the authoritative persisted spelling…" 1/1; tsc --noEmit exit 0; ESLint + Prettier clean on both changed files, both gates proven live by planted violations

Findings

F1 (blocking) — Voice-path registration still uses the persisted spelling; canonical-only probing now duplicates those sessions

live-session-coordinator.ts createOrResumeCoordinator() resumes with sessionId: candidate.sessionId, where the candidate comes from the persisted listing (findRecentCompatibleSession / listRecentSessions); the file contains no normalizeSessionIdForLookup call, and the bridge keys entries by the exact requested id (restoreSessionbyId.get(req.sessionId), no normalization anywhere in packages/acp-bridge). The REST/ACP restore paths, by contrast, register under the canonical id (pre-existing, pinned by the cited tests). After this PR, LiveTaskService.locateTask probes only the canonical id, so a voice session registered under the persisted spelling is invisible to it:

Reproduction (compiled code, live-task-ab.mjs scenarios S7/S8; raw logs logs/ab-head.log / logs/ab-base.log):

node tmp/pr9819-verify-*/live-task-ab.mjs packages/cli/dist fixed          # S7: resumes=[lower], entries=[UPPER, lower], prompt→lower
node tmp/pr9819-verify-*/live-task-ab.mjs tmp/base-tree/packages/cli/dist buggy  # S7: resumes=[], entries=[UPPER], prompt→UPPER
  • Base (pre-PR): correct — the entry is found and reused; the prompt reaches the live voice session.
  • Head: resumeSession(lower) creates a second entry; sendPrompt targets the duplicate instead of the active call; wait_threads subscribes an id with no live entry (subscribe=[lower], times out) instead of the real one (subscribe=[UPPER] on base).

Blast radius: every LiveTaskService entry point (send_message_to_thread, wait_threads, and the read_thread/summary refresh path) against any live session whose bridge entry was registered under the persisted spelling — today, voice sessions resumed via live-host-coordinator.start('resume'), which is a client-reachable mode. The precondition (legacy uppercase live transcript) is the same population this PR targets, and for that population this configuration flips from working to broken.

Minimal suggested fix (measured for collateral only)

Canonicalize at the voice registration site, mirroring the pattern this PR applies elsewhere:

// live-session-coordinator.ts, createOrResumeCoordinator()
const resumed = await runtime.bridge.resumeSession({
  sessionId: normalizeSessionIdForLookup(candidate.sessionId),
  ...
});

Measured in a scratch copy: live-session-coordinator.test.ts 24/24 with and without the patch (identical counts). Not measured end-to-end: driving the real voice pipeline through a resumed mixed-case call was out of this round's budget; the divergence closure is by construction (both ends then canonicalize, which is exactly the invariant S7/S8 show broken). If the maintainer prefers the canonical-only invariant to live in one place, the alternative is a persisted-spelling fallback probe in locateTask, but that weakens the architecture this PR establishes.

F2 (completeness) — ensureResident residency-probe hunk is a redundant defence in serial flows; its race window is unpinned

Mutation m-resume-probe (revert only getSessionSummary(task.bridgeSessionId)task.summary.sessionId inside ensureResident) leaves all 21 tests green. In serial flows the revert is unobservable: when a canonical entry exists, locateTask already fetched the summary via the canonical id (summary.sessionId === bridgeSessionId); when none exists, both spellings miss. The hunk only matters in the concurrent-resume race (a canonical entry registered between locateTask and ensureResident), which no test exercises. Classification per the matrix rules: redundant defence — correct as written, keep it; the unpinned race is a coverage note, not a merge condition.

F3 (scope note, non-blocking) — Normalizer boundaries match the declared scope

Probes against the compiled normalizeSessionIdForLookup (05-normalizer-boundary-probes.png, 12/12): only dashed UUIDs with version nibble 1–5 and RFC variant fold to lowercase; uppercase UUIDv6/v7, undashed 32-hex, -agent--suffixed, padded, truncated, non-UUID, empty, and astral inputs pass through unchanged. The regex is fixed-length and anchored (no backtracking shape — no ReDoS ladder needed). No UUIDv7 producer exists in daemon code paths (sessions are randomUUID() v4; the only v7 hits are a bundled desktop resource and a test fixture), so the deferred shapes are legacy/external-only — consistent with the PR's stated #9490 deferrals. Note the interaction with F1: those deferred shapes are precisely the ones the voice path can still register, so F1's fix should reuse the same normalizer rather than a broader fold.

Not covered

  • Per-commit attribution: shallow depth-2 checkout; only the merge commit is reachable (git rev-list HEAD^1..HEAD^2 = 1 vs 2 commits in the snapshot). Verified the aggregate HEAD^1..HEAD diff only.
  • Voice pipeline end-to-end: F1 was reproduced at the service level with a fake bridge whose semantics were verified against acp-bridge source (exact-id keying, no normalization) and the real registry's probe semantics; a full realtime voice call through a resumed mixed-case transcript was not driven.
  • Whole-repo test suite / integration tests: only targeted gates ran (focused suite, cited tests, typecheck, lint, format). The PR's own CI covers the rest.
  • PR2A follow-ups: enforcement breadth, lock-key folding, directory race adoption, integrity budget, error serialization #9490 remainder (undashed/UUIDv7 folding, case-conflict handling, list merging, reserved standalone source enforcement) — declared out of scope by the PR and not re-litigated here.
  • listWorkspaceSessionsForResponse live-merge races — pre-existing, outside this diff.
  • Windows/macOS (author-declared).

Methodology

Environment: CI verify container (node:22-bookworm), merge-ref checkout (HEAD 0b11966, base tip 3a1f86d, PR head e519147), prebuilt at HEAD. Base arm: scratch worktree tmp/base-tree at HEAD^1, compiled with tsc --build (base core/acp-bridge/cli closure); its runtime @qwen-code/* bare imports resolve to the head dists via the root node_modules symlinks (readlink -f/__w/qwen-code/qwen-code/packages/core and .../acp-bridge) — a clean control because git diff --stat HEAD^1..HEAD proves the PR touches only the two packages/cli/src/serve/live/ files, so the shared runtime closure is byte-identical across arms. Source-level vacuity/mutation runs in the same worktree used the head test file with base/mutated source (test-file diff is purely additive). Harnesses (live-task-ab.mjs, mutate.mjs, matrix-run.sh, normalizer-probes.mjs) live in this artifact dir with raw logs under logs/; evidence PNGs under evidence/ via scripts/verify-capture.mjs. Assertion counts in assertions.json map 1:1 to the scripted checks enumerated above (34 head + 30 base + 21 vacuity + 10 matrix + 6 gates + 12 probes = 113; expected base reds count as passes).

Flakiness gate log

rounds=5 files=1 skipped=0
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


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/serve/live/live-task-service.test.ts: PPPPP

verdict: pass
summary: 1 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)

Evidence images

01-ab-head-fixed-cells

02-ab-base-buggy-cells

03-vacuity-inversion-base-red

04-mutation-matrix-live

05-normalizer-boundary-probes

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Triage re-run completed without a new review.

The bot already has a review of its own on e519147416d3aca1019e19c15ce97969a592cfcb, which still stands.

机器人在 e519147416d3aca1019e19c15ce97969a592cfcb 上已有自己的评审,且仍然有效。

The stage comments above were updated with the latest result. View workflow run.

上方各阶段评论已更新为最新结果。查看工作流运行

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Handled the incremental review findings on current head.

Review item Action
Document the two-spelling invariant Not adopted: the dedicated field name and mixed-case regression tests already pin the split, consistent with the repository comment policy.
Change the missing-metadata error spelling Not adopted: this failure belongs to canonical bridge materialization; caller-visible task IDs remain unchanged.
Add case-insensitive cold-storage discovery Deferred: separate #9490 residual outside this bridge-identity fix.
Pin canonical owner lookup Fixed in 2a6fd37f3e; the test now asserts the registry receives only the canonical ID.
Canonicalize wait cursor lineage Deferred: changes caller-visible cursor semantics beyond this PR scope.

Verification: live-task-service.test.ts 21/21 passed; repository build completed during dependency preparation; workspace typecheck passed. Independent test-engineer verification also confirmed the assertion targets the owner-lookup fallback regression.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No issues found. LGTM! ✅

Not explored to full depth (tool budget reached): "agent 5": running the live-task-service.test.ts vitest suite against HEAD to confirm the traced tests pass (worktree has no node_modules ; npm ci + monorepo build ex….

中文说明

未发现问题。LGTM!✅

未探索到全部深度(达到工具调用预算):"agent 5"running the live-task-service.test.ts vitest suite against HEAD to confirm the traced tests pass (worktree has no node_modules ; npm ci + monorepo build ex…

— qwen3.8-max via Qwen Code /review (v0.22.0)

@wenshao wenshao left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Partially reviewed — gaps disclosed.

Not reviewed: build-and-test — npm test --workspace="packages/cli" exceeded the 540-second review budget; the focused Live task suite passed 21/21.

Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.

Not reviewed: test-efficacy — the probe harness returned inconclusive with harnessValidated=null and no green baseline.

Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:

  • packages/cli/src/serve/live/live-task-service.test.ts:987 — [review] resident 测试未固定 canonical 直接查询

Mechanism health: this round did not close cleanly, so it withholds the incremental anchor — and the round it recovered had none either, so the next review re-reads the whole diff and will keep doing so until a round's marker carries an anchor again. (Stated, not acted on — this changes nothing about what the round posts.)

[Critical] R3-4: ensureResident 的消失竞态会把 canonical live-summary ID 当作持久化 transcript ID。大写 transcript 由小写 bridge entry 驻留后,若 entry 在 locateTask 与 ensureResident 的两次探测之间消失,metadata 读取会使用小写 task.summary.sessionId,在大小写敏感文件系统上找不到大写 transcript,并抛错而不是恢复。Witness: not run — 需要精确控制该并发窗口;代码路径确定地把 canonical summary ID 传给 exact-ID storage 读取。

中文说明

仅完成部分审查,审查缺口已披露。

未审查:build-and-test — npm test --workspace="packages/cli" exceeded the 540-second review budget; the focused Live task suite passed 21/21。

未审查:build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。

未审查:test-efficacy — the probe harness returned inconclusive with harnessValidated=null and no green baseline。

收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。

机制健康:本轮未能干净收尾,因而扣留了增量锚点,而它恢复到的那一轮也没有锚点,因此下一次评审将重读整个 diff——并会一直如此,直到某一轮的标记重新带上锚点。(仅陈述,不据此行动——这不改变本轮发布的任何内容。)

[Critical] R3-4: ensureResident 的消失竞态会把 canonical live-summary ID 当作持久化 transcript ID。大写 transcript 由小写 bridge entry 驻留后,若 entry 在 locateTask 与 ensureResident 的两次探测之间消失,metadata 读取会使用小写 task.summary.sessionId,在大小写敏感文件系统上找不到大写 transcript,并抛错而不是恢复。Witness: not run — 需要精确控制该并发窗口;代码路径确定地把 canonical summary ID 传给 exact-ID storage 读取。

— gpt-5.6-sol via Qwen Code /review (v0.22.0)

Comment thread packages/cli/src/serve/live/live-task-service.ts
Comment thread packages/cli/src/serve/live/live-task-service.ts
Comment thread packages/cli/src/serve/live/live-task-service.ts
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@doudouOUC

Copy link
Copy Markdown
Collaborator Author

Addressed review round 3 in e066ac0885.

Item Action
R3-1 Fixed — voice resume now registers legacy v1-v5 UUIDs with the canonical bridge ID; added a mixed-case voice-resume regression test.
R3-2 Fixed — mixed-case routing validates the exact persisted owner first and rejects a different canonical live owner as ambiguous.
R3-3 Fixed — the caller-visible ID remains separate from the canonical bridge ID and is used for empty-transcript latestTurn.id.
R3-4 Fixed — disappearance between locate and ensure-resident reads metadata with the persisted/caller spelling while resume, prompt, cwd, and event operations retain the canonical bridge ID.

Validation: 49 focused Live tests passed; the canonical ACP and REST restore tests passed; npm run build && npm run typecheck passed. Independent before/after reproduction confirmed all four regressions and fixes.

@doudouOUC

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Aug 24, 2026

Copy link
Copy Markdown
Collaborator

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: 221 passed · 0 failed · 221 total

Flakiness gate: ✅ 2 changed test file(s) x 5 identical rounds, no divergence

中文 — 判定:✅ 通过 · 可合入(agent 判定)

沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查

脚本断言:221 通过 · 0 失败 · 221 总计

抖动门:✅ 2 changed test file(s) x 5 identical rounds, no divergence

Verification report

PR 9819 — fix(serve): Canonicalize Live task bridge session IDs (round 2)

Verdict: merge-ready — 221/221 scripted assertions passed; verified head e066ac08852de69cf39419ea1dbf11b9a9b3bc88. This is a follow-up round; the previous round's blocker (F1, voice-path registration under the persisted spelling) is fixed by exactly the suggested minimal fix, pinned by a new test, and re-measured clean in the A/B (cell S7). The central claim holds under a fresh 48-vs-42-cell A/B against the base build, all seven new tests are proven non-vacuous by inversion and mutation, and every author-cited gate reproduces. One new non-blocking finding (N1): two pre-existing registration sites that consume persisted legacy ids still do not canonicalize — a residual sibling of F1 reachable only from pre-fix persisted state.

中文 — 判定:✅ 通过(agent 判定)

本轮为复审。上一轮的阻塞项 F1(语音 resume 路径以持久化拼写注册 bridge entry,导致 canonical-only 探测产生重复 entry)已被按上轮给出的最小修复建议原样修复live-session-coordinator.ts 注册时调用 normalizeSessionIdForLookup),并由新测试 registers a mixed-case resumed Live session by its canonical id 钉住(突变该 hunk 恰好该测试变红)。A/B 复测:head 侧 48/48 通过(S7 单一 canonical entry 被复用、prompt 与事件订阅均收敛小写);base 侧 42/42 按预期复现原 bug(重复 resume、过期持久化 summary 触发错误唤醒、跨 workspace 误路由)。

  • 中心主张:混合大小写持久化任务经 Live task 面寻址时,所有面向 bridge 的操作(owner 查询、summary 查询、resume、prompt 分发、Conversations 目录创建/迁移、事件轮询)收敛到 canonical UUID;存储读取与 wire 返回保留持久化拼写 —— 成立。
  • 新增交叉校验(存储归属 ≠ live 归属 → ambiguous):head 拒绝且无副作用;base 会把 prompt 误路由到存储所在 workspace —— 守卫动机成立,且被新测试精确钉住。
  • 7 个新测试全部非空转:base 源码 + head 测试恰好 7 个新测试红(42 个既有测试绿);突变矩阵 9 个单 hunk 突变体 8 个被杀、归因精确;ensureResident resident 探测拼写突变体仍然单独存活,组合行未超出 metadata 半区 → 维持"冗余防御"分类(F2 的竞态本体已被新测试钉住,属部分解决)。
  • 新发现 N1(低、非阻塞)scheduled-task-keepalive.ts 的复活/重水化与 create-sub-session.ts 的父会话恢复以持久化记录中的拼写注册且未 canonicalize;仅 pre-fix 遗留状态可喂入非规范拼写(新增调用方入口均已规范化)。静态追踪,未端到端驱动;属 PR 声明延后的 PR2A follow-ups: enforcement breadth, lock-key folding, directory race adoption, integrity budget, error serialization #9490 legacy-id 折叠范畴。
  • 未覆盖:per-commit 归因(shallow depth 2,本地仅 1 个可达 commit,快照有 4 个)、语音管线端到端、N1 路径端到端、全仓测试、Windows/Linux。

Previous-finding status (round 1 → new head)

# Finding (round 1) Severity Status at head e066ac0
F1 Voice-path registration uses the persisted spelling; canonical-only probing duplicates those sessions (S7/S8 regression) blocking fixed — the exact suggested fix applied: sessionId: normalizeSessionIdForLookup(candidate.sessionId) in createOrResumeCoordinator(). Pinned by the new coordinator test (mutant m-coordinator turns exactly it red, §Mutation matrix). A/B cell S7 re-measured: head reuses the single canonical entry (resumes=[], prompt+subscribe on lowercase); base was correct for this configuration by construction — the round-1 regression is resolved.
F2 ensureResident residency-probe hunk = redundant defence in serial flows; race window unpinned completeness partially addressed — commit 2a6fd37 adds uses persisted metadata if a canonical live task disappears, which pins the mid-flight disappearance race (mutant m-metadata-spelling is killed with that test among the reds). The probe-spelling axis (m-resume-probe) still survives alone and the combination row adds nothing beyond the metadata half → still classified redundant defence, correct as written. See N1 for the one residual producer that would have made the spelling axis matter.
F3 Normalizer boundaries match the declared scope scope note stands — 14/14 probes re-run at the new head (05-normalizer-boundary-probes.png): dashed v1–v5 RFC-variant UUIDs fold to lowercase; UUIDv6/v7, undashed 32-hex, agent-suffixed, padded, truncated, non-UUID, empty, and astral inputs pass through unchanged. packages/cli/src/config/session-id.ts is untouched by the PR.

Central claim + A/B

Central claim (carried over, re-measured): addressing a mixed-case persisted task through the Live task surface must converge every bridge-facing operation on the canonical UUID, while storage reads and wire responses keep the persisted spelling.

Harness: live-task-ab.mjs drives the compiled LiveTaskService from each arm's dist with a fake AcpSessionBridge encoding the upstream semantics verified against packages/acp-bridge/src/bridge.ts (entries keyed by the exact requested id — byId.get(req.sessionId)SessionNotFoundError on unknown ids, listWorkspaceSessions filtered by workspace cwd, no normalization anywhere in acp-bridge), a fake registry mirroring resolveLiveSessionOwner (exact-id owner index + exact-id summary probe), and real on-disk storage: transcripts seeded as JSONL in the real Storage.getProjectDir()/chats layout, read by the real SessionService (sessionExists, loadSession, readCreationMetadataIfReadable) and the real listWorkspaceSessionsForResponse merge. Witnesses: 01-ab-head-fixed-cells.png (48/48), 02-ab-base-buggy-cells.png (42/42; raw logs logs/ab-head.log, logs/ab-base.log).

Cell Head (fixed) Base (buggy)
S1 resident canonical entry, addressed UPPERCASE — resumes [] [UPPER] (duplicate)
S1 — entries after send [lower] [UPPER, lower]
S1 — prompt dispatched to lower UPPER
S2 cold resume — resume / materialize / changeSessionCwd id lower ×3 UPPER ×3
S2 — metadata read through persisted spelling (sourceId flowed from the UPPER-only transcript) yes yes
S2 — wire threadId UPPER UPPER
S3 non-hex id — identical both arms SessionNotFoundError('task-1'), no side effects same
S3b undashed 32-hex mixed-case — identical both arms resume/prompt/wire = exact spelling same
S4 canonical lowercase — identical both arms resume/prompt/wire = lowercase same
S5 wait_threads poll — event queries epoch/lastEventId/subscribe on lower (lastEventId 7); timedOut=true zero queries; immediate inactiveStatus wake from the stale persisted summary
S6 read_thread — wire id + turns UPPER, 1 turn same
S7 F1 cell: voice entry registered as the coordinator does (head: lower, base: UPPER) — resumes / entries / prompt / subscribe [] / [lower] / lower / lower [] / [UPPER] / UPPER / UPPER
S8 storage owner (conv) ≠ live owner (project) Task id is ambiguous, zero side effects on either runtime resolves, resumes+prompts on the storage workspace while the live call sits in the project workspace (the misroute the new guard closes)
S9 canonical resident entry, no persisted UPPER transcript converges on the live entry (prompt lower) SessionNotFoundError

S7 is the round-1 blocker, re-measured: at the previous head the same configuration produced resumes=[lower] + [UPPER, lower] entries (duplicate); at the new head the coordinator-registered canonical entry is found and reused, matching base's correct behavior for that configuration. S8 documents the new cross-check's rationale: base demonstrably misroutes the same address.

Reviewer Test Plan walkthrough (re-measured)

Step Result
1. Uppercase persisted + lowercase resident; no second resume; dispatch/poll use lowercase Verified — S1/S5 + reuses the canonical bridge entry… / polls a mixed-case task through its canonical bridge entry (suite 24/24)
2. No resident entry; metadata via uppercase; resume/materialize/relocation/dispatch/subscription via lowercase Verified — S2 (metadata read proven by sourceId flowing from the UPPER-only transcript into the canonical resumeSession) + uses one canonical bridge id…
3. Non-UUID and already-canonical ids retain behavior Verified — S3/S3b/S4 byte-identical on both arms
4. Focused suite + cited tests + build/typecheck/lint/format Reproduced — live-task-service.test.ts 24/24 and live-session-coordinator.test.ts 25/25 (the PR text's "21 tests" predates this round's three added tests); transport.test.ts "keeps the bridge key canonical while isolating mixed-case storage" 1/1; server.test.ts "reads the authoritative persisted spelling…" 1/1; tsc --noEmit exit 0; ESLint + Prettier clean on all four changed files, both gates proven live by planted violations (planted unused variable → 1 eslint error; planted formatting break → prettier --check fail; restored)

Vacuity and mutation

Inversion (03-vacuity-inversion-base-red.png, vacuity-inversion.sh): reverting only the two production hunks (tests stay at head) turns exactly the seven new mixed-case tests red — 6 in the service suite + registers a mixed-case resumed Live session by its canonical id — while all 42 pre-existing tests stay green (7 failed | 42 passed). Failures are behavioral (promise resolved … instead of rejecting, expected-vs-received values), not import/setup errors.

Mutation matrix (04-mutation-matrix-live.png; controls green: 24/24 and 25/25; full logs under logs/mutant-*.log):

Mutant (guard reverted) Suite result Red test(s) — attribution
m-normalize (locateTask canonicalization) 6 red all six mixed-case service tests — positive control in the mutated file
m-crosscheck (new stored-vs-live guard) 1 red rejects a mixed-case task whose storage and live owners differ — precise
m-resume-probe (ensureResident probe spelling) 0 red — survives
m-metadata-spelling (metadata read via canonical) 2 red uses persisted metadata if a canonical live task disappears, uses one canonical bridge id…
m-dispatch (prompt to addressed id) 3 red reuses…, uses persisted metadata…, uses one canonical bridge id…
m-wait-canonical (event-poll ids) 1 red polls a mixed-case task through its canonical bridge entry — precise
m-snapshot-canonical (snapshot watermarks) 1 red same poll test — precise
m-latestturn (latest-turn fallback id) 1 red keeps the caller-visible id when a mixed-case task has no user turn — precise
m-coordinator (voice registration id) 1 red registers a mixed-case resumed Live session by its canonical id — precise
combination m-resume-probe + m-metadata-spelling 2 red identical set to m-metadata-spelling alone

Adjudication of the survivor: m-resume-probe is a redundant defence, not a coverage gap to fill before merge. In serial flows summary.sessionId === bridgeSessionId whenever the summary came from the canonical probe; the spellings diverge only when the summary came from the persisted-list fallback and an entry registered under the persisted spelling exists — and after F1's fix no registration path produces such entries from new state (all caller-facing inputs are canonicalized: REST requireSessionId, ACP dispatch normalization, and now the voice coordinator). The combination row proves the metadata guard is the load-bearing half of the pair; the probe guard is correct as written. The one residual producer is N1 below, which is precisely why this stays in the report.

Findings

N1 (low, non-blocking) — two pre-existing registration sites consume persisted legacy ids without canonicalizing

packages/cli/src/serve/scheduled-task-keepalive.ts:350 and :553 revive/rehydrate bound-task sessions with bridge.resumeSession({ sessionId, … }) where sessionId comes verbatim from the durable cron-task record (collectBoundSessionIdstask.sessionId), and packages/cli/src/serve/create-sub-session.ts:424 restores a parent with the persisted parent-session id. Neither calls normalizeSessionIdForLookup. A pre-fix daemon could register voice sessions under an uppercase persisted spelling; durable state written by that daemon (bound-task records, persisted parent links) can therefore carry non-canonical ids, and a post-upgrade keepalive firing on such a record resurrects an entry the canonical-only Live task lookup cannot see — the same duplication shape as F1 (locateTask misses on the canonical probe, falls back to the persisted summary, resumes a second entry, misroutes the prompt).

Bounds: no path in the post-PR state can produce the triggering records — every caller-facing id source is now canonicalized (REST request-helpers.ts:requireSessionId, ACP dispatch.ts normalization at 1576/1579/1645/1810, voice coordinator via this PR), so only state persisted by a pre-fix daemon qualifies. The keepalive is started for live-conversation runtimes too (run-qwen-serve.ts:5639 runs it before the live-conversation early return), so the conversations workspace is not exempt. This is a static trace — not driven end-to-end — and it sits inside the legacy-id folding work the PR explicitly defers to #9490. Non-blocking; if that folding lands later, these two sites should reuse normalizeSessionIdForLookup rather than a broader fold (per F3's boundary table).

Note (informational) — PR text test count drift

The description's "focused Live task suite passes 21 tests" predates this round's additions; the suite now holds 24 tests (24/24 at head). No action needed.

Not covered

  • Per-commit attribution: depth-2 shallow checkout; git rev-list HEAD^1..HEAD^2 = 1 locally reachable commit vs 4 in the metadata snapshot (git rev-parse --is-shallow-repository = true). The old head e519147 is not locally reachable, so the delta between rounds was identified from the aggregate HEAD^1..HEAD diff plus the previous report, not by diffing old-head..new-head.
  • Voice pipeline end-to-end: cell S7 simulates the coordinator's registration state per its pinned hunk (the coordinator unit test proves the registration id; the A/B proves the service-side consequence). A full realtime voice call through a resumed mixed-case transcript was not driven.
  • N1 paths end-to-end: static trace only; no legacy bound-task/parent-link fixture was driven through keepalive.
  • Whole-repo test suite / integration tests: targeted gates only (focused suites, cited tests, typecheck, lint, format). The PR's own CI covers the rest.
  • PR2A follow-ups: enforcement breadth, lock-key folding, directory race adoption, integrity budget, error serialization #9490 remainder (undashed/UUIDv7 folding, case-conflict handling, list merging, reserved standalone source enforcement) — declared out of scope by the PR, not re-litigated.
  • listWorkspaceSessionsForResponse live-merge races — pre-existing, outside this diff.
  • Windows/Linux (author-declared).
  • Base-arm build note: tsc --build in the base worktree reported type errors confined to test files (worktree type-resolution: @testing-library/react types, and git-commit.ts copied after the build started) — environmental; the emitted JS the harness consumes was verified to be base code (normalizeSessionIdForLookup/bridgeSessionId grep counts 0 in the base dists). The base arm's bare @qwen-code/* imports resolve to the head tree via root node_modules symlinks (readlink -f/__w/qwen-code/qwen-code/packages/core and .../acp-bridge) — a clean control because git diff --name-only HEAD^1..HEAD shows the PR touches only the four packages/cli/src/serve/live/ files, so the shared runtime closure is byte-identical across arms.

Methodology

Environment: CI verify container (node:22-bookworm, Node v22.23.2), merge-ref checkout (HEAD e6b3b5c911, base tip f241c19ace = HEAD^1, PR head e066ac0885 = HEAD^2), prebuilt at HEAD. Base arm: scratch worktree tmp/base-tree at HEAD^1, compiled with tsc --build via scripts/build_package.js (base cli closure); realpath control and diff-stat control as described in Not covered. The A/B harness (live-task-ab.mjs) instantiates the compiled LiveTaskService with a fake bridge/registry encoding verified upstream semantics and real SessionService/Storage disk reads; each scenario seeds real JSONL transcripts and asserts arm-specific expectations (base expectations encode the bug reproduction, so base reds are scripted passes). Source-level vacuity/mutation runs mutate the head tree in place (mutate.mjs, named single-hunk mutants; matrix-run.sh runs each under vitest and restores via git checkout — tree verified clean afterward). Raw logs under logs/; harnesses and mutant manager in this artifact dir. Assertion counts in assertions.json: A/B 48 head + 42 base, vacuity inversion 49 (7 expected-red + 42 expected-green, name-verified), mutation matrix 12 rows (2 controls + 9 mutants + 1 combination), normalizer probes 14, focused suites 49, cited tests 2, gates 5 (typecheck, eslint clean + planted-live, prettier clean + planted-live) = 221, all passing.

Flakiness gate log

rounds=5 files=2 skipped=0
file packages/cli/src/serve/live/live-session-coordinator.test.ts: (cd packages/cli) npx --no-install vitest run ./src/serve/live/live-session-coordinator.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


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/serve/live/live-session-coordinator.test.ts: PPPPP
  packages/cli/src/serve/live/live-task-service.test.ts: PPPPP

verdict: pass
summary: 2 changed test file(s) x 5 identical rounds, no divergence

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/serve/live/live-session-coordinator.test.ts: P (exit 0)
round 1 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/live/live-session-coordinator.test.ts: P (exit 0)
round 2 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/live/live-session-coordinator.test.ts: P (exit 0)
round 3 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/live/live-session-coordinator.test.ts: P (exit 0)
round 4 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/live/live-session-coordinator.test.ts: P (exit 0)
round 5 · packages/cli/src/serve/live/live-task-service.test.ts: P (exit 0)

Evidence images

01-ab-head-fixed-cells

02-ab-base-buggy-cells

03-vacuity-inversion-base-red

04-mutation-matrix-live

05-normalizer-boundary-probes

Harness scripts and raw logs are in the workflow run artifacts (7-day retention).

Qwen Code · sandboxed verification

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, looks ready to ship. ✅

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Verified the three Criticals from the last round are addressed at head e066ac0:

  • R3-1 (voice resume producer divergence): live-session-coordinator now registers resumed sessions via normalizeSessionIdForLookup(candidate.sessionId), so the voice path converges on the same canonical bridge entry the lookup probes — no duplicate resume for a legacy uppercase transcript. Pinned by the new mixed-case registration test.
  • R3-2 (cross-workspace case-twin routing): when the caller's spelling differs from the canonical bridge id, locateTask now scans stored runtimes for the exact persisted spelling and refuses as ambiguous when the stored owner and the live owner disagree (or multiple runtimes hold the exact spelling) — the A-uppercase-stored / B-lowercase-resident shape fails closed instead of selecting the live twin and skipping the ownership scan. Pinned by the storage-vs-live owner test.
  • R3-3 (wire spelling leak): LocatedTask now carries the caller-visible threadId separately from the bridge id, and both the latestTurn fallback and the metadata reads use it — the wire response keeps the persisted uppercase identity instead of leaking the canonical lowercase spelling. Pinned by the no-user-turn wire-id test.

CI at approval time: 20 checks passing, none failing; review-pr automated check still pending on this head. All threads resolved.

@doudouOUC
doudouOUC dismissed wenshao’s stale review August 24, 2026 08:58

Already have 2 approves,3ks.

@doudouOUC
doudouOUC added this pull request to the merge queue Aug 24, 2026
Merged via the queue into QwenLM:main with commit b5aec66 Aug 24, 2026
175 of 180 checks passed
@doudouOUC
doudouOUC deleted the fix/live-task-session-id-canonicalization branch August 24, 2026 09:12
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.22.2.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants