fix(session): preserve source titles when branching - #9764
Conversation
Forked sessions could fall back to a UUID fragment when the source name came from its first prompt rather than a custom title. - Forward the active picker name through Web Shell branch requests - Resolve prompt-backed display names and include them in collision scans - Allocate the first free numeric suffix and normalize nested forks - Preserve explicit side-task names and add regression coverage
github-actions
Bot
added
the
review/self-reported
|
@qwen-code-ci-bot please take a look |
|
Re-run at head Template ✓ — all sections present, bilingual, with before/after screenshots. Problem: observed bug, verified twice over. Linked issue #9762 with screenshots, and the maintainer's two-arm local run reproduced it exactly on the merge-base arm ( Direction: aligned. Session naming is an actively maintained surface (design doc updated in the same PR), with no auth/sandbox/telemetry/public-contract exposure. The round-1 web-shell contract change (client sending a name) was reverted in Size: core paths touched ( Approach: minimal. The fallback chain (explicit name → normalized custom title → picker display name → last-resort fallback), stripping generated suffixes before re-suffixing, and making the collision scan see what the picker sees are all needed for the stated goal; I don't see a materially simpler path. Both round-1 blockers are resolved: the CLI Risk: Moving on to code review. 🔍 中文说明在 head 模板完整 ✓ —— 各部分齐全，中英双语，附 before/after 截图。 问题：已观测到的 bug，且被双重验证。关联 issue #9762 附截图；维护者双臂本地实测在 merge-base 臂上精确复现（以首条 prompt 命名的源会话分支出 方向：对齐。会话命名是持续维护的界面（同 PR 更新了设计文档），不触及 auth/sandbox/telemetry/公共契约。第 1 轮中的 web-shell 契约改动（客户端传名称）已在 规模：触及核心路径（ 方案：最小化。回退链（显式名 → 归一化后的自定义标题 → picker 展示名 → 最后兜底）、重新加后缀前先剥掉已生成的后缀、让冲突扫描与 picker 所见一致 —— 均为目标所必需；没有更简路径。第 1 轮的两个阻塞项均已解决：CLI 风险： 进入代码审查。🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code review (head
|
| Check | Conclusion |
|---|---|
| Test (ubuntu-latest, Node 22.x) | ✅ success |
| web-shell E2E Smoke (ubuntu-latest, Node 22.x) | ✅ success |
| Serve A/B (ubuntu-latest, Node 22.x) | ✅ success |
| Real daemon E2E / Java 11 | ✅ success |
| Desktop Shell (ubuntu-22.04) | ✅ success |
| Desktop Shell (windows-2022) | ✅ 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 |
| Classify PR | ✅ success |
| Dependency CVE audit | ✅ success |
| Secret scan (TruffleHog) | ✅ success |
| Test (macos-latest, Node 22.x) | ⏭️ skipped |
| Test (windows-latest, Node 22.x) | ⏭️ skipped |
| Integration Tests (CLI, No Sandbox) | ⏭️ skipped |
The central claim is behavioural — branches land on the lowest free Title(N) of the source's picker-visible name family — and the unit suite is mock-based, so CI alone does not pin it. That gap is already covered from two sides: the maintainer's two-arm real-environment run at this exact head passed the reviewer test plan 4/4 against the real daemon, CLI, and browser, including adversarial probes (empty/blank titles, legacy (Branch) tokens, 3 concurrent branches, a dense Dense(1..120) family), and the sponsored @qwen-code /verify run he triggered was still in flight at review time — worth reading its report before merge as the independent confirmation of the same claim.
中文说明
代码审查（head 1359f61）
我在读 diff 前独立拟定的方案 —— 在两条分支路径上解析源会话的 picker 展示名（customTitle || firstPrompt）、让冲突扫描与 picker 所见一致、改用最小可用数字后缀 —— 与本 PR 的形态完全一致，因此审查重点是找漏洞而非方向。在该 head 上仔细核对过：
- 回退链（两条路径）：显式
name→ 归一化后的自定义标题 →SessionService.getSessionDisplayName()→ 最后兜底（daemon 路径用 session-id 前缀，CLI/branch用首条 prompt）。该差异是有意的，设计文档与两处调用点均有说明；新方法仅在前两个来源都为空时才被 await，且有测试钉住这一惰性。 - 锚点已对照代码树核实：
SESSION_FILE_PATTERN无g标志（getSessionDisplayName中的.test不存在lastIndex状态性 bug）；MAX_PROMPT_SCAN_LINES = 10限定每文件头部读取；normalizeDerivedBranchTitle经由现有export * from './services/sessionService.js'桶导出触达两个导入方 —— 无遗漏导出。 - 看似无界的循环：旧代码上限 99 个候选后回退
Date.now()；新的for (let n = 1; ; n++)最多迭代taken.size + 1次，因为taken来自一次有界目录扫描的有限集。严格更简单，大小写不敏感比较与项目作用域均保留。 - 消费者图谱：
findSessionTitlesByPrefix唯一生产消费者是computeUniqueBranchTitle，其两个调用方（ACP 分支路由与useBranchCommand）均已连同测试更新。新的SessionService.getSessionDisplayName与packages/webui/.../mappers.ts中 picker 自身的解析器一致（维护者已按三种名称形态核对），不是第二份漂移的展示名定义。 - 已披露的成本真实且有界：无标题会话在冲突扫描中从廉价跳过变为一次 ≤10 行的头部读取。这是匹配 picker 语义的固有成本，仅在分支时发生；维护者在真实 265 会话目录上实测约 +200 ms（中位数）并认为可接受（为其测量结果，非本评审复测）。
无关键发现，无规范违反。评审各轮遗留的非阻塞事项 —— 均未复现为 bug，记录在案以免遗漏：ACP 路由读取自定义标题前未像 CLI 路径那样 flush()（维护者 6 次紧凑竞态尝试未复现陈旧读）；Draft (1) 与 Draft(1) —— 后缀剥离要求 ( 前为非空白字符，因生成的后缀从不带空格，读起来是有意的；PR 描述仍提及已回退的 Web Shell 名称转发；归档（archive/）transcript 仍在扫描范围外（既有行为）。
测试证据 —— 被审 commit 上本 PR 自己的 CI
无人值守运行 —— 不执行 PR 代码；以上为 1359f61 上本 PR 的 CI，经 API 一次性拉取。凡运行的全部通过：24 项成功、0 项失败。macOS/Windows 单测矩阵与 Integration Tests (CLI, No Sandbox) 在该 fork PR 上被跳过（任务门控）—— 如实披露，不掩盖。维护者的双臂报告在本 head 上本地跑过这些套件（sessionService 190/190、useBranchCommand + acpAgent 513/513、eslint、tsc 通过 —— 为其数字，注明出处，本评审未独立复跑）。
核心断言是行为性的 —— 分支落在源会话 picker 可见名称族中最小可用的 Title(N) —— 而单测基于 mock，仅凭 CI 无法钉住。该缺口已从两侧覆盖：维护者在本 head 上的双臂真实环境运行对真实 daemon、CLI 与浏览器通过了审阅测试计划 4/4，含对抗性探测（空/空白标题、旧式 (Branch) 词元、3 并发分支、稠密的 Dense(1..120) 名称族）；他触发的赞助 @qwen-code /verify 运行评审时仍在进行中 —— 合并前值得阅读其报告，作为对同一断言的独立确认。
— Qwen Code · qwen3.8-max
Reviewed at 1359f614116b03e0f4c1031b37dc9f684f9d43d7 · re-run with @qwen-code /triage
|
Confidence: 4/5 — solid and now fully evidenced at this head; the fifth point is withheld for named non-blocking nits (the un-flushed ACP title read, the stale PR description), not for doubt. Stepping back: this PR went through seven review rounds and came out the other side genuinely better, not just bigger. Both round-1 blockers were fixed properly — the CLI The remaining notes are hygiene, not blockers — recorded in the Stage 2 comment so they aren't silently dropped. Verdict: approve. No PR CI runs are pending on this commit, so the approval is posted now, pinned to it. This approval supersedes my earlier change requests, which stood on older heads (rounds 1, 2, 5, 6) and have since been addressed. 中文说明置信度：4/5 —— 扎实，且在本 head 上证据齐全；保留最后 1 分是给点名的非阻塞事项（ACP 路径未 flush 的标题读取、过时的 PR 描述），而非存疑。 退一步看：这个 PR 历经七轮评审，是真的变得更好，而不是变得更大。第 1 轮的两个阻塞项都被妥善修复 —— CLI 其余事项属合并卫生而非阻塞 —— 已记录在 Stage 2 评论，以免被悄悄丢弃。 结论：批准。本 commit 上没有进行中的 PR CI 运行，故现在发布批准，并钉在该 commit 上。此批准取代我早前的修改请求 —— 那些请求停留在旧 head（第 1、2、5、6 轮）上，相关问题均已解决。 — Qwen Code · qwen3.8-max Reviewed at |
🖼️ web-shell visual previewRendered against a mock daemon (no real backend): the PR base vs this PR head Screenshots · before / afterFull-resolution recordings (.webm) are attached to the workflow run. — Qwen Code · web-shell visuals |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
doudouOUC
left a comment
doudouOUC
left a comment
There was a problem hiding this comment.
审阅摘要
本次审阅验证了 PR #9764 的当前快照 fa1630ee。
变更概述
将分支会话标题从旧式 (Branch) / (Branch N) 约定改为数字 (N) 后缀格式。核心变更包括：
- 新增
normalizeBranchBaseName归一化基础名称（ACP 路径） - 新增
getSessionDisplayName方法，在自定义标题缺失时回退到首条 prompt - 重写
findSessionTitlesByPrefix以支持基于 prompt 的展示名称 - 简化
computeUniqueBranchTitle，移除MAX_BRANCH_COLLISION_SCAN上限 - Web Shell 的
App.tsx向分支请求传递sessionDisplayName
现有发现（均仍有效）
2 个 Critical 阻塞项：
-
CLI
/branch路径未修复 —useBranchCommand.ts:170仍只从首条 prompt 派生基础名，忽略自定义标题。const baseName = name ?? deriveFirstPrompt(provisional.conversation.messages);应添加outgoingRecording?.getCurrentCustomTitle()作为中间回退。 -
Playwright 冒烟 e2e 未更新 —
web-shell.smoke.spec.ts:129断言toEqual({ atRecordId })，但新代码发送{ name, atRecordId }。PR 的 CI 失败web-shell E2E Smoke证实了这一点。
6 个 Suggestion：
normalizeBranchBaseName会剥离用户显式提供的(N)后缀- 旧式
(Branch)剥离分支无测试覆盖 - CLI 路径未应用相同的名称归一化
- UUID 回退路径丢失了测试覆盖
getSessionDisplayName的错误路径未测试- 展示名称的获取逻辑在
getSessionDisplayName和findSessionTitlesByPrefix中重复实现
结论
COMMENT — 2 个 Critical 阻塞项仍存在，且 web-shell E2E Smoke CI 检查失败。变更方向正确，核心逻辑稳健，但在关闭 #9762 前需修复上述两个 Critical 阻塞项。
Review found that CLI and ACP still derived branch names differently, while the Web Shell smoke test asserted the previous request body. - Preserve explicit names while normalizing only derived title suffixes - Reuse Core display-name and title helpers across branch entry points - Cover legacy, fallback, and isolation paths and update the smoke E2E
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
LGTM. Verified the naming machinery and the round-1 follow-ups at head 7f24b47:
- Allocation terminates and matches the picker:
computeUniqueBranchTitlebuildstakenfrom one prefix scan whose display-name derivation mirrors the picker (customTitle, else first prompt, project-ownership-checked), compares case-insensitively, and iteratesTitle(n)from 1 — the set is finite so the loop always ends, and legacyTitle (Branch)names (space before paren) can't collide with the newTitle(n)family. normalizeDerivedBranchTitlestrips(Branch)/(Branch N)and a trailing non-space(n), so re-branching a numbered session stays in the original family instead of nestingTitle(1)(1); explicit side-task names pass through unchanged, andgetSessionDisplayNameguards the session-id pattern before any file read.- The round-1 Criticals are addressed: the CLI
/branchpath now consumes the same core functions (computeUniqueBranchTitle+normalizeDerivedBranchTitle), so both clients share one convention and one recipe; the newnamerequest field is additive (an older daemon ignores it and falls back to its own naming) and the smoke spec pins the new contract end to end.
One minor, non-blocking edge: the requested-name path skips normalization when no atRecordId accompanies the name (baseName stays as-requested), so a source titled Foo(2) could yield Foo(2)(1) on that route — the primary Web Shell flow always branches from a response record, where normalization applies, so this is latent only.
CI note: two checks fail on head for reasons unrelated to this change — Test (ubuntu) fails in agents/team/test-utils/coordination-harness.test.ts (message routing), a file this PR doesn't touch; the web-shell E2E Smoke dies at CHECKOUT with EACCES removing a leftover scratch dir from another PR's review workspace (9748) on the shared runner. Neither is caused by this diff; worth a rerun once the runner dir is cleared.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Not explored to full depth (tool budget reached): "agent 5": executing the changed vitest suites (review worktree has no node_modules / dist ; npm ci + npm run build + test runs would exceed the tool budget) — findin….
中文说明
未探索到全部深度（达到工具调用预算）："agent 5"：executing the changed vitest suites (review worktree has no node_modules / dist ; npm ci + npm run build + test runs would exceed the tool budget) — findin…。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const baseName = | ||
| requestedName === undefined | ||
| ? (derivedBaseName ?? sessionId.slice(0, 8)) | ||
| : requestedName === sourceDisplayName | ||
| ? (derivedBaseName ?? requestedName) | ||
| : requestedName; |
There was a problem hiding this comment.
[Critical] normalizeDerivedBranchTitle() can return an empty string, and ?? does not fall back on '' — so this loses the empty-result guard the deleted deriveForkBaseName had (stripped && stripped.length > 0 ? stripped : sessionId.slice(0, 8)). A session renamed to exactly (Branch) or (Branch 2) (renameSession accepts any non-empty string ≤200 chars) normalizes to '', then derivedBaseName ?? sessionId.slice(0, 8) keeps '' and computeUniqueBranchTitle('') titles the new branch literally (1), where the pre-refactor code fell back to the session-id base. The same gap exists at two sibling sites: the side-task path in this file persists an empty custom_title via forkSession (whose guard is title !== undefined) and returns { title: '', displayName: '' } to the client, and useBranchCommand.ts skips its deriveFirstPrompt fallback and also yields (1).
Witness (scratch-tree probe against the real exported functions at this commit):
NORM("(Branch)") = "" NORM("(Branch 2)") = ""
HEAD branch-path baseName = "" -> final title "(1)" with || fallback -> "550e8400(1)"
HEAD side-task title passed to forkSession = "" (record gets written) with || fallback -> "550e8400"
HEAD CLI final title = "(1)" pre-refactor CLI -> "help me fix the login bug(1)"
The probe flips when the implied one-line fix is applied. Treat an empty normalization result as absent — (derivedBaseName || sessionId.slice(0, 8)) here and (derivedBaseName || requestedName) in the equality branch — or make normalizeDerivedBranchTitle return string | undefined so the existing ?? fallbacks engage at all three sites.
中文说明
normalizeDerivedBranchTitle() 可能返回空字符串，而 ?? 不会对 '' 回退——因此丢失了被删除的 deriveForkBaseName 原本具有的空结果守卫（stripped && stripped.length > 0 ? stripped : sessionId.slice(0, 8)）。将会话重命名为恰好 (Branch) 或 (Branch 2)（renameSession 接受任意非空且 ≤200 字符的字符串）后，归一化结果为 ''，随后 derivedBaseName ?? sessionId.slice(0, 8) 保留 ''，computeUniqueBranchTitle('') 会把新分支命名为字面量 (1)；而重构前的代码会回退到 sessionId 前缀。同样的缺口还存在于另外两处：本文件的 side-task 路径会通过 forkSession（其守卫是 title !== undefined）持久化一个空的 custom_title 并向客户端返回 { title: '', displayName: '' }；useBranchCommand.ts 则会跳过 deriveFirstPrompt 回退，同样产生 (1)。
见证（在本提交的临时树中对真实导出函数执行的探针）：归一化 "(Branch)"/"(Branch 2)" 均得到 ""；当前分支路径 baseName 为 ""，最终标题 "(1)"，改用 || 回退后为 "550e8400(1)"；side-task 路径传给 forkSession 的 title 为 ""（记录会被写入），修复后为 "550e8400"；CLI 最终标题 "(1)"，重构前为 "help me fix the login bug(1)"。应用隐含的一行修复后探针结果翻转。
建议：把空的归一化结果视为缺失——此处用 (derivedBaseName || sessionId.slice(0, 8))、相等分支用 (derivedBaseName || requestedName)——或让 normalizeDerivedBranchTitle 返回 string | undefined，使现有 ?? 回退在三处都生效。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Thanks, confirmed and fixed in 8aac1c3abf. normalizeDerivedBranchTitle now returns string | undefined, so exact (Branch) and (Branch 2) results engage the existing session-id fallback instead of reaching computeUniqueBranchTitle as an empty base. Added ordinary-branch regressions expecting 550e8400(1). Core 190/190 and ACP 464/464 pass.
| if (title === undefined) { | ||
| const sourceCustomTitle = recording?.getCurrentCustomTitle(); | ||
| title = sourceCustomTitle | ||
| ? normalizeDerivedBranchTitle(sourceCustomTitle) | ||
| : sessionId.slice(0, 8); | ||
| } |
There was a problem hiding this comment.
[Critical] Same empty-normalization gap as the branch path above, with a worse outcome here: when the source's custom title is exactly (Branch)/(Branch 2), normalizeDerivedBranchTitle(sourceCustomTitle) returns '' and the truthiness check on sourceCustomTitle passes it through, so forkSession is called with title: '' — its guard is title !== undefined, so it persists an empty custom_title record and the route returns { title: '', displayName: '' } to the client. The deleted deriveForkBaseName fell back to sessionId.slice(0, 8) in exactly this case. Suggested fix: title = (sourceCustomTitle ? normalizeDerivedBranchTitle(sourceCustomTitle) : '') || sessionId.slice(0, 8); — or the shared string | undefined return type on the normalizer.
中文说明
与上方分支路径相同的空归一化缺口，此处后果更重：当源会话的自定义标题恰好是 (Branch)/(Branch 2) 时，normalizeDerivedBranchTitle(sourceCustomTitle) 返回 ''，而对 sourceCustomTitle 的真值判断会让它直接通过，于是 forkSession 被以 title: '' 调用——其守卫是 title !== undefined，因此会持久化一条空的 custom_title 记录，路由向客户端返回 { title: '', displayName: '' }。被删除的 deriveForkBaseName 在这种情况下会回退到 sessionId.slice(0, 8)。建议修复：title = (sourceCustomTitle ? normalizeDerivedBranchTitle(sourceCustomTitle) : '') || sessionId.slice(0, 8);——或让归一化函数统一返回 string | undefined。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Fixed in 8aac1c3abf. The shared normalizer now returns undefined for an empty result, and the side-task path falls back to sessionId.slice(0, 8) before calling forkSession. Added a regression proving (Branch) produces non-empty title/displayName 550e8400 and never persists an empty custom title.
| (sourceCustomTitle | ||
| ? normalizeDerivedBranchTitle(sourceCustomTitle) | ||
| : deriveFirstPrompt(provisional.conversation.messages)); |
There was a problem hiding this comment.
[Critical] Third site of the empty-normalization gap: if the source's custom title is exactly (Branch)/(Branch 2), normalizeDerivedBranchTitle(sourceCustomTitle) returns '', the truthiness check on sourceCustomTitle accepts it, and the deriveFirstPrompt fallback is skipped — computeUniqueBranchTitle('') then names the branch literally (1). Pre-refactor this path always fell back to the non-empty first-prompt base. Suggested fix: (sourceCustomTitle ? normalizeDerivedBranchTitle(sourceCustomTitle) : '') || deriveFirstPrompt(provisional.conversation.messages) — or the shared string | undefined return type on the normalizer so ?? engages.
中文说明
空归一化缺口的第三处：若源会话的自定义标题恰好是 (Branch)/(Branch 2)，normalizeDerivedBranchTitle(sourceCustomTitle) 返回 ''，对 sourceCustomTitle 的真值判断会接受它，从而跳过 deriveFirstPrompt 回退——computeUniqueBranchTitle('') 会把分支命名为字面量 (1)。重构前该路径总是回退到非空的首条 prompt 基础名。建议修复：(sourceCustomTitle ? normalizeDerivedBranchTitle(sourceCustomTitle) : '') || deriveFirstPrompt(provisional.conversation.messages)——或让归一化函数统一返回 string | undefined 以使 ?? 回退生效。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Fixed in 8aac1c3abf. CLI /branch now treats an empty normalized custom title as absent and falls back through the source picker display name or the existing first-prompt fallback. Exact (Branch) and (Branch 2) cases now produce help me fix the login bug(1) in the focused fixture; the full CLI file passes 35/35.
| const sourceCustomTitle = recording?.getCurrentCustomTitle(); | ||
| title = sourceCustomTitle | ||
| ? normalizeDerivedBranchTitle(sourceCustomTitle) | ||
| : sessionId.slice(0, 8); |
There was a problem hiding this comment.
[Suggestion] This diff deletes the shared deriveForkBaseName helper and inlines two divergent copies of the fork base-name derivation in the same case block — this side-task copy and the branch-path copy ~40 lines above. The side-task copy silently omits the branch path's persisted-display-name fallback and its requestedName === sourceDisplayName normalization, with no comment saying the omission is deliberate. This is a present-day divergence, not only future maintenance: with no live title but a persisted one, the branch fork derives from the persisted display name while the side-task fork falls back to the UUID fragment; an echoed display name is normalized on one path and kept verbatim on the other. The next change to the derivation rule must land in both blocks — miss one and the two surfaces re-fragment the picker naming this PR exists to unify. Consider extracting one small shared helper (branch path layers its extra logic on top), or stating at this site that the divergence is intentional.
中文说明
本 diff 删除了共享的 deriveForkBaseName 辅助函数，并在同一个 case 块中内联了两份行为不一致的 fork 基础名推导——即此处的 side-task 副本与上方约 40 行的分支路径副本。side-task 副本悄悄省略了分支路径的持久化展示名回退以及 requestedName === sourceDisplayName 归一化，且没有注释说明这是有意为之。这是当下就存在的分歧，而不仅是未来的维护风险：当没有活动标题但存在持久化标题时，分支 fork 从持久化展示名推导，而 side-task fork 回退到 UUID 片段；被回显的展示名在一条路径上被归一化，在另一条路径上被原样保留。下次修改推导规则时必须同时改两处——漏掉一处，两个入口就会重新割裂本 PR 要统一的命名族。建议抽取一个小的共享辅助函数（分支路径在其上叠加额外逻辑），或在此处注释说明分歧是有意的。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Thanks for raising this. I kept the side-task persisted-display fallback intentionally unchanged in 8aac1c3abf: side tasks retain their pre-existing explicit name -> live recorder title -> UUID semantics and separate session_source: side_task identity, while ordinary branch naming follows picker semantics. Both paths still share the Core derived-title normalizer, including the empty-result fix. A configurable helper spanning the two different fallback policies would add abstraction without changing the requested behavior.
There was a problem hiding this comment.
Decline upheld in this round's triage. The two derivations stay separate by design: side tasks keep explicit name → normalized recorder title → session-id prefix and their own side_task identity, while ordinary branches follow picker display-name semantics. Both paths already share the Core derived-title normalizer (including the empty-result fallback fixed in 8aac1c3), so the recognition grammar cannot fragment again; a helper spanning two different fallback policies would add abstraction without changing behavior. Leaving this thread open with the recorded reason.
中文说明
本轮分诊维持拒绝（Decline）。两份推导有意保持分离：side task 保留 显式名称 → 归一化后的记录器标题 → session-id 前缀 链及其独立的 side_task 身份，普通分支则遵循选择器展示名语义。两条路径已共享 Core 的派生标题归一化函数（含 8aac1c3 修复的空结果回退），识别语法不会再次割裂；一个横跨两种不同回退策略的辅助函数只会增加抽象而不改变行为。线程保持打开并附已记录的理由。
| const resolveSourceDisplayName = | ||
| requestedName === undefined || atRecordId !== undefined; |
There was a problem hiding this comment.
[Suggestion] The daemon distinguishes a user-chosen name from the client echoing the source's display name only via exact string equality here, and only when atRecordId is set — an undocumented cross-client contract. Two concrete costs: (1) a genuinely explicit branch name identical to the source display name (e.g. an ACP client calling sessionBranch with name: 'Source session(2)' against a source titled Source session(2)) is silently re-normalized to Source session(1), contradicting the sentence this same diff adds to the design doc ("Explicitly requested names remain unchanged before suffix allocation"); (2) Web Shell always echoes sessionDisplayName as name, so any future client change to the echoed string (trimming, truncation, a stale catalog entry) breaks the equality and re-introduces stacked suffixes Source session(2)(1) — the exact regression this PR removes. The smoke test cannot catch this because it mocks the daemon and asserts only the request body. Prefer making the protocol unambiguous at the producer (Web Shell omits name when the user didn't provide one), then reduce this to requestedName === undefined ? derived : requestedName; if daemon-side robustness for echoing clients is the intent, document it here and reconcile the design-doc sentence.
中文说明
daemon 仅靠此处的精确字符串相等来区分“用户选择的名字”与“客户端回显的源会话展示名”，且只在设置了 atRecordId 时生效——这是一条未文档化的跨客户端契约。两个具体代价：(1) 与源展示名恰好相同的显式分支名（例如 ACP 客户端对名为 Source session(2) 的源会话以 name: 'Source session(2)' 调用 sessionBranch）会被悄悄重归一化为 Source session(1)，与本 diff 同时新增的设计文档语句（“显式请求的名称在后缀分配前保持不变”）矛盾；(2) Web Shell 总是把 sessionDisplayName 作为 name 回显，因此未来任何对被回显字符串的改动（裁剪、截断、过期的目录项）都会破坏相等判断，重新引入嵌套后缀 Source session(2)(1)——正是本 PR 要消除的回归。冒烟测试抓不住这一点，因为它 mock 了 daemon 且只断言请求体。建议在生产端消除歧义（用户未提供名字时 Web Shell 不发送 name），然后把此处简化为 requestedName === undefined ? derived : requestedName；若本意是兼容会回显的客户端，请在此处注明并同步修正设计文档语句。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Fixed in 8aac1c3abf by removing the ambiguous echo contract. Historical Web Shell response branching no longer sends name; the daemon derives the source picker name when no name is supplied and treats every supplied name as explicit. The App tests and Chromium smoke now pin { atRecordId }, and an explicit historical Source session(2) remains Source session(2)(1).
| const displayName = await this.readSessionDisplayNameFromFile( | ||
| filePath, | ||
| titleInfo, | ||
| ); |
There was a problem hiding this comment.
[Suggestion] The broadened collision scan now performs a sequential ≤10-record head-read + JSON parse + project-membership check for every untitled session file (up to MAX_FILES_TO_PROCESS = 10000) before the prefix filter can run; untitled files used to be skipped after the cheap title tail-read. Measured in a scratch-tree probe over 2000 untitled fixture files: 258.6 / 215.8 / 211.1 ms per full scan → ~0.1 ms/file warm lower bound → ~1.06 s projected at the 10,000-file cap, on every branch in dense projects (real first records are larger than the ~300-char fixture, so this is a floor). The cost is inherent to the new picker-parity semantics, but nothing pins the per-file budget — the existing perf test in useBranchCommand.test.ts asserts scan call count only, so a future edit can quietly widen the per-file read. Consider extending that pin (or this function's docstring, whose old text advertised skipping prompt extraction) to assert at most one bounded head-read per untitled file.
中文说明
放宽后的冲突扫描现在会对每一个无自定义标题的会话文件（上限 MAX_FILES_TO_PROCESS = 10000）顺序执行 ≤10 条记录的头部读取 + JSON 解析 + 项目归属检查，之后才能运行前缀过滤；此前无标题文件在廉价的标题尾读之后即被跳过。在临时树中对 2000 个无标题 fixture 文件的探针实测：整轮扫描 258.6 / 215.8 / 211.1 ms → 约 0.1 ms/文件的热态下限 → 在 10,000 文件上限下推算约 1.06 s，且密集项目中每次分支都会发生（真实首条记录比约 300 字符的 fixture 更大，因此这只是下限）。该成本是新的“与选择器一致”语义的固有代价，但每文件的读取预算没有被钉住——useBranchCommand.test.ts 中现有的性能测试只断言扫描调用次数，未来的改动可能悄悄放宽每文件读取。建议扩展该断言（或本函数的 docstring——旧文案曾声明跳过 prompt 提取），断言每个无标题文件至多一次有上限的头部读取。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Addressed in 8aac1c3abf. The Core contract now documents one head read bounded by MAX_PROMPT_SCAN_LINES for each untitled candidate, and the prompt-only fixture asserts the exact per-file call budget [[file, 10]]. This pins the existing bounded cost without adding caching or concurrency machinery.
| try { | ||
| return await this.readSessionDisplayNameFromFile(filePath); | ||
| } catch { | ||
| return undefined; | ||
| } |
There was a problem hiding this comment.
[Suggestion] getSessionDisplayName is tested on its happy paths plus the empty-file and other-project cases, but the catch that downgrades read failures to undefined and the SESSION_FILE_PATTERN guard above it have no test — removing the try/catch would ship green. That matters because acpAgent.ts awaits this method inside the branch extMethod whose only catch maps BranchPointInvalidError; any other throw rejects the whole branch request. If the source's chats file does not exist or is transiently unreadable (branching a live session before its first record is persisted, or concurrent cleanup), unguarded jsonl.readLines throws ENOENT and the branch fails outright instead of degrading to the UUID-fragment base. Two cases would pin it: a sessionId with no <sessionId>.jsonl on disk resolves to undefined (does not throw), and an id failing SESSION_FILE_PATTERN (e.g. 'not-a-session') resolves to undefined.
中文说明
getSessionDisplayName 已有正常路径及空文件、其他项目情形的测试，但把读取失败降级为 undefined 的 catch 以及其上方的 SESSION_FILE_PATTERN 守卫没有测试——删除该 try/catch 也能全绿通过。这很重要，因为 acpAgent.ts 在分支 extMethod 中 await 该方法，而该处唯一的 catch 只映射 BranchPointInvalidError；其他任何抛错都会使整个分支请求被拒绝。如果源会话的聊天记录文件不存在或暂时不可读（在首条记录落盘前对活动会话分支、或并发清理），未加保护的 jsonl.readLines 会抛 ENOENT，分支将直接失败而不是降级为 UUID 前缀基础名。两个用例即可钉住：磁盘上不存在 <sessionId>.jsonl 的 sessionId 应解析为 undefined（不抛错）；不满足 SESSION_FILE_PATTERN 的 id（如 'not-a-session'）应解析为 undefined。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Addressed in 8aac1c3abf. Added formal tests for both a valid but missing session UUID and invalid id not-a-session; both resolve to undefined without throwing. Existing empty-file and foreign-project cases remain covered. The full Core test file passes 190/190.
| name ?? | ||
| (sourceCustomTitle | ||
| ? normalizeDerivedBranchTitle(sourceCustomTitle) | ||
| : deriveFirstPrompt(provisional.conversation.messages)); |
There was a problem hiding this comment.
[Suggestion] For untitled source sessions this CLI fallback still uses deriveFirstPrompt (whitespace collapsed, sliced to 100 chars, no ellipsis) while the daemon path this diff adds uses picker display-name semantics (extractFirstPromptFromRecords → truncatePromptForDisplay: raw whitespace/newlines, 200 code points + ...) — contradicting the design-doc rule this diff adds ("Use the source session's picker display name (customTitle || prompt) as the base") and the PR's cross-client alignment goal. Witness (scratch-tree probe, same untitled source with a multi-line >100-char pasted error block): the daemon arm produced the raw-newline title truncated at the 185-char slice, while the CLI arm produced "Error: Cannot read properties of undefined (reading 'map') at UserList.render (src/components/UserLi(1)" — the equality assertion failed on baseline and passed once the CLI arm fell back to getSessionDisplayName. The CLI fork title also mismatches the CLI's own picker, which renders the raw form via extractFirstPromptFromRecords, and the two different strings are invisible to each other's collision scan. Consider routing the CLI fallback through the same picker-display-name extraction, or documenting the CLI's divergence in the design-doc section this diff adds.
中文说明
对无自定义标题的源会话，此处的 CLI 回退仍使用 deriveFirstPrompt（空白折叠、截断到 100 字符、无省略号），而本 diff 新增的 daemon 路径使用选择器展示名语义（extractFirstPromptFromRecords → truncatePromptForDisplay：保留原始空白/换行、200 码点 + ...）——这与本 diff 新增的设计文档规则（“使用源会话的选择器展示名（customTitle || prompt）作为基础名”）及 PR 的跨客户端对齐目标矛盾。见证（临时树探针，同一个多行且超过 100 字符的粘贴报错块作为首条 prompt）：daemon 侧产生保留换行、按 185 字符截断的标题，CLI 侧产生折叠后的 "Error: ... UserLi(1)"——基线下标题相等断言失败，令 CLI 侧回退到 getSessionDisplayName 后通过。CLI 分支标题与 CLI 自己的选择器也不一致（选择器经 extractFirstPromptFromRecords 渲染原始形式），且两个不同的字符串互相看不见对方的冲突扫描。建议让 CLI 回退复用相同的选择器展示名提取，或在本 diff 新增的设计文档章节中明确记录 CLI 的差异。
— qwen3.8-max via Qwen Code /review (v0.22.0)
There was a problem hiding this comment.
Fixed in 8aac1c3abf. For an untitled source, CLI /branch now calls SessionService.getSessionDisplayName(oldSessionId) and uses that picker-compatible value before the local first-prompt degradation fallback. Added a multiline >100-character regression that preserves the picker display name; CLI 35/35 and the target Chromium smoke both pass.
Round-two review found empty normalized titles could bypass fallbacks, while client-echoed names made explicit-name semantics ambiguous. - Treat empty derived titles as absent across branch entry points - Stop Web Shell name echoing and align CLI fallback with the picker - Pin missing-session, invalid-id, and bounded-scan behavior
|
@qwen-code-ci-bot /takeover |
yiliang114
added
the
autofix/takeover
|
🤝 Takeover engaged: the autofix loop now manages this PR — it will address new review feedback and resolve base conflicts until the label is removed or the round cap is reached. Remove the 中文说明🤝 已接管：autofix 循环现在管理此 PR —— 将持续处理新的评审反馈与 base 冲突，直到移除标签或达到轮次上限。移除 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/core/src/services/sessionService.ts:3065 — [probe] Title slice counts UTF-16 units and can split a surrogate pair fed by the new display-name path
Convergence: round 3 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 8 (8 new). Findings keep coming back to the same files: packages/cli/src/acp-integration/acpAgent.ts (findings in round 2; 2 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. No Critical finding is open on this round, so merging and moving the remaining Suggestion threads to a follow-up issue is available as an ending — a merged pull request cannot diverge further. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
收敛姿态下延后（第 3 轮，非阻断）——已记录，本轮不要求修改：共 1 条（原文未翻译，列表见上方英文部分）。
收敛情况：第 3 轮发布了 3 条行内评论，其中 3 条是首次提出；上一轮发布了 8 条（其中 8 条首次提出）。发现反复回到同一批文件：packages/cli/src/acp-integration/acpAgent.ts（第 2 轮已出过发现，本轮又有 2 条）。一个不断再生兄弟发现的簇，通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因，或把独立的簇拆成单独的 PR，通常比逐条修复更快结束循环。本轮没有未决的 Critical，因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.22.0)
| normalizeDerivedBranchTitle: vi.fn( | ||
| (baseName: string) => | ||
| baseName | ||
| .trim() | ||
| .replace(/\s*\(Branch(?:\s+\d+)?\)$/, '') | ||
| .replace(/(\S)\(\d+\)$/, '$1') | ||
| .trim() || undefined, | ||
| ), |
There was a problem hiding this comment.
[Suggestion] This mock copies the real normalizeDerivedBranchTitle regex body verbatim instead of reusing the implementation — the same vi.mock factory already passes other pure helpers through importOriginal (e.g. parseInvocationContext, emptyGoalSnapshot). The adjacent computeUniqueBranchTitle stub deliberately simplifies to ${baseName}(1) for determinism, but this mock simplifies nothing — it duplicates. This PR just rewrote the normalization scheme once ( (Branch N) → (N)); the next such change will update sessionService.ts without this mock, and the ACP route tests will silently keep exercising the stale regex: expectations like 'Source session(2)' → 'Source session(1)' would validate the mock's old behaviour and stay green while the shipped function behaves differently — the integration tests would endorse a normalization that no longer exists.
| normalizeDerivedBranchTitle: vi.fn( | |
| (baseName: string) => | |
| baseName | |
| .trim() | |
| .replace(/\s*\(Branch(?:\s+\d+)?\)$/, '') | |
| .replace(/(\S)\(\d+\)$/, '$1') | |
| .trim() || undefined, | |
| ), | |
| normalizeDerivedBranchTitle: vi.fn( | |
| (await importOriginal<typeof import('@qwen-code/qwen-code-core')>()) | |
| .normalizeDerivedBranchTitle, | |
| ), |
中文说明
这个 mock 逐字复制了真实 normalizeDerivedBranchTitle 的正则实现，而不是复用真实实现——同一个 vi.mock 工厂已经通过 importOriginal 引入了其他纯函数（如 parseInvocationContext、emptyGoalSnapshot）。旁边的 computeUniqueBranchTitle 桩是为了确定性而刻意简化为 ${baseName}(1)，但这个 mock 没有任何简化——只是重复实现。本 PR 刚刚重写过一次归一化方案（ (Branch N) → (N)）；下次此类变更只会更新 sessionService.ts 而不会更新这个 mock，ACP 路由测试会继续悄悄验证过期的正则：像 'Source session(2)' → 'Source session(1)' 这样的断言仍会通过，但验证的是 mock 的旧行为，而真实函数的行为已经不同——集成测试将为一个已不存在的归一化方案背书。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| title = sourceCustomTitle | ||
| ? (normalizeDerivedBranchTitle(sourceCustomTitle) ?? | ||
| sessionId.slice(0, 8)) |
There was a problem hiding this comment.
[Suggestion] The side-task path's derived-title branch — no explicit name and the source custom title normalizes to a non-empty result — has no test, so the normalization applied there is unpinned. The only three sessionSideTask tests are: an explicit name (custom title never consulted), custom title '(Branch)' (normalizes to empty → session-id fallback), and the atRecordId rejection. A regression reverting this path to the pre-PR semantics — strip only (Branch…), keep a Title(N) suffix, i.e. the old deriveForkBaseName behaviour — passes all three existing tests (the '(Branch)' case falls back to 550e8400 under both old and new semantics), yet a user creating a side task from a session titled My Project(2) without a name would get My Project(2) instead of My Project.
Witness (scratch-tree mutant probe at this commit):
mutant (pre-PR deriveForkBaseName semantics): side-task tests still green 4/4
probe (custom title 'My Project(2)', no name):
real code -> forkSession title: "My Project"
mutant -> expected "title": "My Project", received "My Project(2)"
Add one case alongside the existing ones: no name, getCurrentCustomTitle returns 'My Project(2)', expect forkSession called with expect.objectContaining({ title: 'My Project' }).
中文说明
side-task 路径中“推导出的标题”分支——未提供显式 name 且源会话自定义标题归一化结果非空——没有测试，该处应用的归一化因此未被钉住。现有 sessionSideTask 测试只有三个：显式名称（完全不查询自定义标题）、自定义标题为 '(Branch)'（归一化为空 → 回退到 session-id）、以及拒绝 atRecordId。若该路径回归到 PR 之前的语义——只剥离 (Branch…)、保留 Title(N) 后缀（即旧的 deriveForkBaseName 行为）——三个现有测试全部仍能通过（'(Branch)' 用例在新旧语义下都回退为 550e8400），但从名为 My Project(2) 的会话不带名称创建 side task 的用户会得到 My Project(2) 而不是 My Project。
见证（在本提交的临时树中执行的变异探针）：变异体（PR 前的 deriveForkBaseName 语义）下 side-task 测试仍为 4/4 全绿；探针用例（自定义标题 'My Project(2)'、无 name）结果翻转——真实代码持久化 title: "My Project"，变异体持久化 title: "My Project(2)"。
建议在现有用例旁补充一个：不提供 name，getCurrentCustomTitle 返回 'My Project(2)'，断言 forkSession 被以 expect.objectContaining({ title: 'My Project' }) 调用。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const baseName = | ||
| requestedName ?? derivedBaseName ?? sessionId.slice(0, 8); |
There was a problem hiding this comment.
[Suggestion] When normalizeDerivedBranchTitle(sourceCustomTitle) returns undefined, the empty-result fallback diverges between clients: this ACP/daemon route (used by Web Shell) falls back to the session-id prefix, while CLI /branch falls back to the first prompt — the same source session yields different branch titles depending on which client branches it, undercutting the cross-client alignment this PR exists for. Concretely: a source session manually renamed to exactly (Branch) or (Branch 2), branched without an explicit name, becomes 550e8400(1) via Web Shell but help me fix the login bug(1) via CLI — here persistedDisplayName is only fetched when sourceCustomTitle === undefined, so a custom title that normalizes to empty never reaches the prompt fallback. Both outcomes are pinned by this PR's own tests ('(Branch)' → 550e8400(1) in acpAgent.test.ts; ['(Branch)', 'help me fix the login bug(1)'] in useBranchCommand.test.ts), so the divergence is observable and persistent. The trigger is rare (a title that is exactly a legacy/generated suffix token), but a maintainer comparing the two clients sees unexplained different titles for identical operations. Either align the chains — when normalization returns undefined, fall back to the source's first prompt here as well (note getSessionDisplayName won't serve this directly since it prefers customTitle, the very value that normalized to empty) — or document the divergence as intentional at both sites and in the design-doc section this diff adds.
中文说明
当 normalizeDerivedBranchTitle(sourceCustomTitle) 返回 undefined 时，空结果回退在两个客户端之间不一致：此 ACP/daemon 路由（Web Shell 使用）回退到 session-id 前缀，而 CLI /branch 回退到首条 prompt——同一个源会话仅因创建分支的客户端不同就会得到不同的分支标题，削弱了本 PR 要实现的跨客户端一致性。具体地：将一个手动重命名为恰好 (Branch) 或 (Branch 2) 的源会话不带名称创建分支，经 Web Shell 得到 550e8400(1)，经 CLI 却得到 help me fix the login bug(1)——此处 persistedDisplayName 仅在 sourceCustomTitle === undefined 时才获取，因此归一化为空的自定义标题永远到不了 prompt 回退。两种结果都被本 PR 自己的测试钉住（acpAgent.test.ts 中 '(Branch)' → 550e8400(1)；useBranchCommand.test.ts 中 ['(Branch)', 'help me fix the login bug(1)']），因此该分歧可观察且会长期存在。触发条件罕见（标题恰好是旧式/生成的后缀记号），但维护者对比两个客户端时会看到相同操作产生无法解释的不同标题。建议要么对齐两条链——当归一化返回 undefined 时此处同样回退到源会话的首条 prompt（注意 getSessionDisplayName 无法直接满足，因为它优先取 customTitle，而那正是归一化为空的值）——要么在两处代码及本 diff 新增的设计文档章节中明确记录该分歧是有意的。
— qwen3.8-max via Qwen Code /review (v0.22.0)
…vergence Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
|
🔀 Base updated: red check(s) [review-pr] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [review-pr] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 5, not a blocker) — recorded, not requested in this round:
packages/core/src/services/sessionService.ts:3076 — [review] User-authored (N) custom titles are stripped as if they were generated fork suffixespackages/core/src/services/sessionService.test.ts:6089 — [review] The $ end-anchor in the numeric-suffix strip regex is pinned by no testpackages/cli/src/ui/hooks/useBranchCommand.test.ts:392 — [review] The CLI hook's display-name-fetch guard is never asserted
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 5 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const baseName = | ||
| requestedName ?? derivedBaseName ?? sessionId.slice(0, 8); |
There was a problem hiding this comment.
[Critical] R4-1: When the source session's current custom title is exactly the empty string, this derivation chain leaks it all the way to computeUniqueBranchTitle, producing the degenerate branch title (1). '' is not undefined, so persistedDisplayName is never fetched (the guard is sourceCustomTitle === undefined); sourceCustomTitle ?? persistedDisplayName keeps ''; the falsy ternary yields derivedBaseName = ''; and requestedName ?? '' ?? sessionId.slice(0, 8) keeps '' because ?? only falls through on null/undefined — so the fork is announced, persisted, and listed as (1) instead of the documented session-id-prefix fallback 550e8400(1). An empty title is writable through the shipped qwen/control/session/title control route (validation is string-type + max-length only, so '' passes and recordCustomTitle('') persists it) and through load-path hydration of a custom_title: "" record. The deleted deriveForkBaseName guarded exactly this case, and the sibling side-task route (truthy check + ?? sessionId.slice(0, 8)) and CLI /branch (truthy ternary falls through to deriveFirstPrompt) still handle '' correctly — the defect is isolated to this route. A whitespace-only display name from getSessionDisplayName also trims to empty inside the allocator through the same missing guard.
Witness (scratch-tree probe against the real route at this commit):
PR (unfixed): expect title '550e8400(1)' -> received { "title": "(1)" }
core probe: computeUniqueBranchTitle('') => "(1)", prefix "("
PR + suggested fix: same probe => '550e8400(1)'; full acpAgent.test.ts 475/475 pass
Normalize the derived base the way the requested name already is:
const baseName =
requestedName ??
(derivedBaseName?.trim() || undefined) ??
sessionId.slice(0, 8);and apply the same trim-to-undefined treatment to sourceDisplayName in useBranchCommand.ts. Add the missing sourceTitle: '' → 550e8400(1) regression row to the ACP title table.
中文说明
当源会话当前的自定义标题恰好为空字符串时，这条派生链会把它一路泄漏到 computeUniqueBranchTitle，产生退化分支标题 (1)。'' 不是 undefined，因此 persistedDisplayName 不会被获取（守卫是 sourceCustomTitle === undefined）；sourceCustomTitle ?? persistedDisplayName 保留 ''；falsy 三元分支使 derivedBaseName = ''；而 requestedName ?? '' ?? sessionId.slice(0, 8) 因为 ?? 只对 null/undefined 回退，仍保留 ''——于是新分支被公告、持久化并显示为 (1)，而不是文档约定的 session-id 前缀回退 550e8400(1)。空标题可以通过已发布的 qwen/control/session/title 控制路由写入（其校验仅为字符串类型 + 最大长度，'' 可通过并被 recordCustomTitle('') 持久化），也可在加载路径中由 custom_title: "" 记录水合而来。被删除的 deriveForkBaseName 恰好守卫了这种情况，且同文件的 side-task 路由（真值判断 + ?? sessionId.slice(0, 8)）与 CLI /branch（falsy 三元回退到 deriveFirstPrompt）至今仍正确处理 ''——缺口仅限于本路由。来自 getSessionDisplayName 的纯空白展示名也会经由同一缺失的守卫在分配器内被 trim 为空。
见证（在本提交的临时树中对真实路由执行的探针）：当前代码期望标题 '550e8400(1)'，实际收到 { "title": "(1)" }；核心探针 computeUniqueBranchTitle('') 返回 "(1)"，前缀为 "("；应用建议修复后同一探针返回 '550e8400(1)'，且 acpAgent.test.ts 全量 475/475 通过。
建议按请求名称已有的归一化方式同样处理派生基础名（如上代码），并对 useBranchCommand.ts 中的 sourceDisplayName 做同样的 trim 转 undefined 处理；同时在 ACP 标题表中补充 sourceTitle: '' → 550e8400(1) 的回归用例行。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| base, remove an existing generated fork suffix, and append the lowest available | ||
| numeric suffix: `Title(1)`, `Title(2)`, and so on. Explicitly requested names | ||
| remain unchanged before suffix allocation. If a custom title normalizes to | ||
| nothing (it was exactly a suffix token), no picker name survives: the daemon |
There was a problem hiding this comment.
[Suggestion] R4-2: This parenthetical misdescribes the fallback trigger: normalizeDerivedBranchTitle('(2)') returns '(2)' (the numeric-strip regex (\S)\(\d+\)$ requires a non-whitespace char before the paren, and '(2)' has none), so a title that is exactly a numeric suffix token never normalizes to nothing — only exactly (Branch) / (Branch N) titles do. The daemon/CLI fallback divergence this paragraph documents is therefore unreachable for the numeric token class the sentence names. Verified by executing the shipped helper at this commit: ('(2)') → '(2)', ('(Branch)') → undefined, ('(Branch 2)') → undefined.
| nothing (it was exactly a suffix token), no picker name survives: the daemon | |
| nothing (it was exactly a legacy `(Branch)` or `(Branch N)` token), no picker name survives: the daemon |
中文说明
此处的括号说明错误描述了回退触发条件：normalizeDerivedBranchTitle('(2)') 返回 '(2)'（数字剥离正则 (\S)\(\d+\)$ 要求括号前有一个非空白字符，而 '(2)' 没有），因此恰好是数字后缀词的标题永远不会归一化为空——只有恰好是 (Branch) / (Branch N) 的标题才会。所以本段所描述的 daemon/CLI 回退分歧，对于该句所指的数字后缀词类是不可达的。已在本提交上执行随代码发布的助手函数验证：('(2)') → '(2)'，('(Branch)') → undefined，('(Branch 2)') → undefined。建议将括号内文字改为“恰好是旧式 (Branch) 或 (Branch N) 词”，或修改归一化函数使纯数字标题也触发回退。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 AutoFix ran out of time before finishing (timeout (2700000ms)) (attempt 3/100) — it will retry on the next scan.
See the Qwen Autofix agent step logs for model/tool output. 中文说明🤖 AutoFix 在完成前耗尽了时间（timeout (2700000ms)）（第 3/100 次尝试）—— 将在下次扫描时重试。 Run log: https://github.com/QwenLM/qwen-code/actions/runs/32723225818 🧠 Handled by Qwen Code · model/模型 |
|
🔀 Base updated: red check(s) [Test (ubuntu-latest, Node 22.x)] pass on current main — merged current main via update-branch; CI will re-run. 中文说明🔀 已更新 base：红色检查 [Test (ubuntu-latest, Node 22.x)] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main，CI 将重新运行。 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/acpAgent.ts:1148 — [review] normalizeRequestedBranchName's empty-after-trim branch is unpinned by any test; a whitespace-only requested name mutant survives the whole suitedocs/design/web-shell/assistant-response-session-branching.md:707 — [review] R4-2 still stands — the parenthetical misdescribes the fallback trigger: normalizeDerivedBranchTitle('(2)') returns '(2)', so a purely numeric suffix token never n…
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 6 轮，非阻断）——已记录，本轮不要求修改：共 2 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
| const baseName = | ||
| requestedName ?? derivedBaseName ?? sessionId.slice(0, 8); |
There was a problem hiding this comment.
[Critical] R4-1: When the source session's current custom title is exactly the empty string, this derivation chain leaks it all the way to computeUniqueBranchTitle, producing the degenerate branch title (1). '' is not undefined, so persistedDisplayName is never fetched (the guard is sourceCustomTitle === undefined); sourceCustomTitle ?? persistedDisplayName keeps ''; the falsy ternary yields derivedBaseName = ''; and requestedName ?? '' ?? sessionId.slice(0, 8) keeps '' because ?? only falls through on null/undefined — so the fork is announced, persisted, and listed as (1) instead of the documented session-id-prefix fallback 550e8400(1). An empty title is writable through the shipped qwen/control/session/title control route (validation is string-type + max-length only, so '' passes and recordCustomTitle('') persists it) and through load-path hydration of a custom_title: "" record. The sibling side-task route (truthy check + ?? sessionId.slice(0, 8)) and CLI /branch (truthy ternary falling through to the display-name / first-prompt chain) still handle '' correctly — the defect is isolated to this route.
Witness (round-5 scratch-tree probe against the real route; the anchored code is unchanged since, and the mechanism re-traces at this commit):
PR (unfixed): expect title '550e8400(1)' -> received { "title": "(1)" }
core probe: computeUniqueBranchTitle('') => "(1)", prefix "("
Normalize the derived base the way the requested name already is:
const baseName =
requestedName ??
(derivedBaseName?.trim() || undefined) ??
sessionId.slice(0, 8);中文说明
当源会话当前的自定义标题恰好为空字符串时，这条派生链会把它一路泄漏到 computeUniqueBranchTitle，产生退化分支标题 (1)。'' 不是 undefined，因此 persistedDisplayName 不会被获取（守卫是 sourceCustomTitle === undefined）；sourceCustomTitle ?? persistedDisplayName 保留 ''；falsy 三元分支使 derivedBaseName = ''；而 requestedName ?? '' ?? sessionId.slice(0, 8) 因为 ?? 只对 null/undefined 回退，仍保留 ''——于是新分支被公告、持久化并显示为 (1)，而不是文档约定的 session-id 前缀回退 550e8400(1)。空标题可以通过已发布的 qwen/control/session/title 控制路由写入（校验仅为字符串类型 + 最大长度，'' 可通过并被 recordCustomTitle('') 持久化），也可在加载路径中由 custom_title: "" 记录水合而来。同文件的 side-task 路由（真值判断 + ?? sessionId.slice(0, 8)）与 CLI /branch（falsy 三元回退到展示名/首条 prompt 链）至今仍正确处理 ''——缺口仅限于本路由。
见证（第 5 轮临时树中对真实路由执行的探针；锚定代码此后未变，且该机制在本提交重新追踪成立）：期望标题 '550e8400(1)'，实际收到 { "title": "(1)" }；核心探针 computeUniqueBranchTitle('') 返回 "(1)"，前缀为 "("。
建议按请求名称已有的归一化方式同样处理派生基础名（如上代码）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Addressed the latest review feedback (round 4/100). What changed, and what I pushed back on: · 已处理最新评审反馈（第 4/100 轮）。改动内容与我反驳保留之处如下： Autofix address-review summary — PR #9764Commit: Feedback triage and decisions
Reproduction (before any fix)Per the source-blind verification rule, the defect was reproduced on the pre-round code before changing anything:
Changes
Mutation probes
Both new guards are therefore witnessed by tests that fail without them. Disclosed (not run / not requested)
Verification
中文说明Autofix address-review 摘要 — PR #9764提交： 反馈分类与处理决定
复现（修复前）按照来源无关的验证规则，在修改任何代码之前先在修复前的代码上复现了该缺陷：
变更内容
变异探针
因此两个新守卫均有“缺失即失败”的测试见证。 已披露（未运行 / 本轮不要求）
验证
Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 Re-review when you have a moment. After round 100 this bot stops and leaves the PR for a human. · 有空请复审；第 100 轮后本 bot 停止并将 PR 交给人工。 🧠 Handled by Qwen Code · model/模型 |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round:
packages/cli/src/acp-integration/acpAgent.ts:11142 — [review] Side-task route's falsy-custom-title fallback branch has no test; a mutation survives the whole suitepackages/cli/src/acp-integration/acpAgent.ts:1149 — [review] normalizeRequestedBranchName's blank/non-string rejection is unpinned by any testpackages/cli/src/ui/hooks/useBranchCommand.ts:150 — [review] The CLI hook's getSessionDisplayName lazy-read guard has no negative assertion
中文说明
仅完成部分审查，审查缺口已披露。
未审查：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally。
收敛姿态下延后（第 7 轮，非阻断）——已记录，本轮不要求修改：共 3 条（原文未翻译，列表见上方英文部分）。
— qwen3.8-max via Qwen Code /review (v0.22.0)
|
🤖 Reviewed the latest feedback — no changes needed. Why, point by point: · 已审阅最新反馈——无需改动。逐点说明原因如下： Autofix round: no action taken (PR #9764)No code changes were made this round and no commit was created. The branch remains at Triage of this round's feedback
OutcomeNothing was requested and nothing was actionable this round: no reproducible Required defect claim, no 中文说明Autofix 本轮：未执行任何操作（PR #9764）本轮未做任何代码修改，也未创建任何提交。分支保持在 本轮反馈的分类处理
结论本轮没有任何请求，也没有任何可操作项：没有可复现的 Required 缺陷主张，没有 Base-conflict check · 基分支冲突检查: no conflict with main. · 与 main 无冲突。 🧠 Handled by Qwen Code · model/模型 |
Local real-environment verification — PR #9764I built a two-arm local environment and ran the branch flows for real (no mocks, no Arms — PR head What was actually driven
Verdict: the fix does what it says on Linux. Reviewer test plan 4/4 pass, and I found one quantified cost plus four non-blocking notes. 1. Web Shell — the reported bug reproduces on main and is gone on the PR
Identical scripted sequence on both arms: one prompt
Test plan ①②③④ all pass, and the source session is unchanged in every case. 2. CLI
|
| Client | main 65c2bb01 |
PR 1359f614 |
|---|---|---|
CLI /branch |
…and add int (Branch) (cut at 100 chars) |
…timeout path in the gateway(1) (153 chars) |
| Web Shell / daemon | 11119765 (Branch) (UUID fragment) |
…timeout path in the gateway(1) (153 chars) |
On main the same session gets two different branch names depending on which client you use. On the PR both clients produce the byte-identical title. I also checked the new getSessionDisplayName against what the picker itself resolves — they agree on all three shapes (custom title, prompt-backed, and "first prompt beyond the 10-line head window" → both show no name), so the helper is not a second, drifting definition of the display name.
4. Adversarial probes (all against the real daemon)
| Probe | Result |
|---|---|
| Branch fired the instant the first turn goes idle, ×6 | … first prompt(1) every time — no read-before-flush fallback observed |
| 3 concurrent branches of the same source | (1) (2) (3), zero duplicate names |
Source title (Branch) / (Branch 2) |
falls back to session-id prefix → 97640200(1), as documented |
Source title "" / " " (bot finding R4-1) |
ffff9764(1) — fixed at head, no degenerate (1) |
Source title 42(7) |
42(1) — generated suffix stripped |
| 440-char first prompt | title 188 chars (≤ SESSION_TITLE_MAX_LENGTH), no lone surrogate |
Dense family Dense(1..120) |
Dense(121); the now-unbounded for (let n = 1; ; n++) is bounded by the finite taken set |
5. The cost the PR flags, measured
findSessionTitlesByPrefix now head-reads every untitled session file instead of skipping it after the cheap title tail-read.
| Catalog | main | PR |
|---|---|---|
computeUniqueBranchTitle over a copy of a real 265-session catalog (126 MB, 263 untitled), warm cache |
33 ms median | 207 ms median |
| synthetic untitled catalogs, 200 records each — 50 / 200 / 500 sessions | 4 / 14 / 37 ms | 21 / 86 / 192 ms |
end-to-end POST /session/:id/branch on the real daemon, 287 transcripts on disk |
64 ms median | 261 ms median |
So a branch in a heavy workspace costs roughly +200 ms once, inside the source session's history-mutation lock. I think that is acceptable for a one-shot user action, and worth writing down.
One thing worth knowing if anyone tries to optimise this later: over the same 265 files, readLines(f, 1) takes 121 ms and readLines(f, 10) takes 144 ms — the cost is the per-file stream open, not the 10-line budget, so narrowing the head window would only recover ~15%. Reusing the picker's existing listing would be the only real lever, and that is clearly out of scope here.
6. Non-blocking notes
- The PR description is stale. It still says the change "forwards the active Web Shell name with branch requests", but the final diff does not touch
packages/web-shell— that commit-1 change was reverted in8aac1c3in favour of the daemon deriving the name itself. Worth fixing the description before merge so the commit message that lands is accurate. flush()asymmetry. CLI/branchdoesawait outgoingRecording?.flush()before callinggetSessionDisplayName; the ACP branch route reads it with no flush. I could not reproduce a stale read in 6 tight-race attempts, but a one-lineawait recording.flush()would make the two routes symmetric for free.Draft (1)vsDraft(1).normalizeDerivedBranchTitle's numeric alternative is/(\S)\(\d+\)$/, which requires a non-space character before(, while the legacy alternative tolerates leading whitespace. Measured:Draft(1)→Draft(2)(same family),Draft (1)→Draft (1)(1)(new nested family). That reads deliberate — only generated suffixes are stripped, and generated ones never have the space — but the design doc's "remove an existing generated fork suffix" line does not say so, and item ③ of the test plan describesTitle(1)(1)as the thing to avoid.- Archived transcripts are outside the scan (
archive/is skipped bySESSION_FILE_PATTERN), so unarchiving can surface a duplicate name. Same on main, just noting it is not fixed here.
7. Gates
packages/coresessionService.test.ts— 190/190 passpackages/cliuseBranchCommand.test.ts+acpAgent.test.ts— 513/513 passeslint --max-warnings 0on all six changed files — cleantsc --noEmit -p packages/core— clean- PR CI — 22 pass / 0 fail
Tested on
| OS | Status |
|---|---|
| 🍏 macOS | ✅ (author) |
| 🪟 Windows | |
| 🐧 Linux | ✅ (this run) |
LGTM from my side — happy to merge once the description in note 1 is corrected.
中文说明
PR #9764 本地真实环境验证
我在本地搭了双臂环境，把分支流程真跑了一遍（无 mock、无 page.route），给合并决策提供依据。
双臂 —— PR head 1359f614 对比 merge base 65c2bb01，各自用 node esbuild.config.js 打出 dist/cli.js。
git diff 65c2bb01..1359f614 -- packages/web-shell packages/webui packages/sdk-typescript packages/acp-bridge 为空，所以浏览器臂能精确隔离出 daemon/core 的改动。
实际驱动的层次
| 层 | 环境 |
|---|---|
| 浏览器 | 真实 Chromium → packages/web-shell vite dev server → QWEN_DAEMON_URL → 真实 qwen serve daemon → 真实磁盘 transcript |
| Daemon | 每臂一个 qwen serve，隔离 QWEN_HOME，走真实 HTTP API（POST /session、/prompt、/branch、/side-task、PATCH /metadata） |
| CLI | tmux 里的真实交互 TUI（130×40），真实按键：prompt → /rename → /branch |
| Core | 真实 SessionService 读真实磁盘 *.jsonl，包含一份真实 265 会话目录的拷贝（126 MB，其中 263 个无自定义标题） |
| 模型 | 脚本化的 OpenAI 兼容服务，两臂收到完全相同的字节，排除模型侧差异 |
结论：在 Linux 上该修复行为符合描述。Reviewer Test Plan 四项全过；发现一处已量化的开销，以及四条不阻塞合并的备注。
1. Web Shell —— 问题在 main 上复现，在 PR 上消失
两臂执行完全相同的脚本序列：一条 prompt 创建 MR 描述生成 Skill，然后从源会话分支 3 次、从分支再分支 1 次，另加一个自定义标题会话和一个旧式 (Branch 2) 会话。
| 场景 | main 65c2bb01 |
PR 1359f614 |
|---|---|---|
| 源会话 picker 名称（来自首条 prompt） | 创建 MR 描述生成 Skill |
创建 MR 描述生成 Skill |
| 分支 #1 | dddddddd (Branch) |
创建 MR 描述生成 Skill(1) |
| 分支 #2 | dddddddd (Branch 2) |
创建 MR 描述生成 Skill(2) |
| 分支 #3 | dddddddd (Branch 3) |
创建 MR 描述生成 Skill(3) |
| 从分支再分支 | dddddddd (Branch 4) |
创建 MR 描述生成 Skill(4) —— 无嵌套 |
side task，显式 Side task(2) |
Side task(2) |
Side task(2) —— 原样保留 |
| 源会话带自定义标题 | My custom title (Branch) |
My custom title(1) |
源会话为旧式 Legacy title (Branch 2) |
Legacy title (Branch) |
Legacy title(1) |
测试计划 ①②③④ 全部通过，且每种情况下源会话名称都保持不变。
2. CLI /branch —— 自定义标题现在被正确继承
两臂按键完全相同。屏幕上的提示文案一模一样，所以可靠的判据是写入 fork JSONL 的 custom_title 记录：main 上 fork 忽略会话的自定义标题、退回用首条 prompt 重新推导；PR 上则继承 picker 名称。
3. 跨客户端一致性（这是我最想确认的一点）
同一个无自定义标题的源会话，首条 prompt 150 字符，分别从两个客户端各分支一次：
| 客户端 | main 65c2bb01 |
PR 1359f614 |
|---|---|---|
CLI /branch |
…and add int (Branch)（截断到 100 字符） |
…timeout path in the gateway(1)（153 字符） |
| Web Shell / daemon | 11119765 (Branch)（UUID 片段） |
…timeout path in the gateway(1)（153 字符） |
main 上同一个会话，从不同客户端分支会得到两个不同的名字；PR 上两个客户端产出逐字节一致的标题。我另外把新增的 getSessionDisplayName 与 picker 自身解析出的名称做了对照，三种形态（自定义标题、prompt 派生、首条 prompt 落在 10 行头部窗口之外）结果完全一致，因此这个 helper 不会成为第二套会漂移的展示名定义。
4. 对抗性探测（全部针对真实 daemon）
| 探测 | 结果 |
|---|---|
| 首轮对话一进入 idle 就立刻分支，×6 | 每次都是 … first prompt(1)，未观察到 flush 前读盘导致的回退 |
| 同一源会话 3 个并发分支 | (1) (2) (3)，无重名 |
源标题为 (Branch) / (Branch 2) |
回退到 session-id 前缀 → 97640200(1)，与文档一致 |
源标题为 "" / " "（bot 提的 R4-1） |
ffff9764(1) —— 当前 head 已修复，不会退化成 (1) |
源标题为 42(7) |
42(1) —— 生成式后缀被正确剥离 |
| 首条 prompt 440 字符 | 标题 188 字符（≤ SESSION_TITLE_MAX_LENGTH），无孤立代理项 |
密集族 Dense(1..120) |
Dense(121)；改成无上限的 for (let n = 1; ; n++) 由有限的 taken 集合保证终止 |
5. PR 自己标注的风险，实测数据
findSessionTitlesByPrefix 现在会对每个无自定义标题的会话文件做头部读取，而不再在廉价的标题尾读之后直接跳过。
| 目录规模 | main | PR |
|---|---|---|
真实 265 会话目录拷贝（126 MB，263 个无标题）上的 computeUniqueBranchTitle，缓存已热 |
中位 33 ms | 中位 207 ms |
| 合成无标题目录，每会话 200 条记录 —— 50 / 200 / 500 会话 | 4 / 14 / 37 ms | 21 / 86 / 192 ms |
真实 daemon 上端到端 POST /session/:id/branch，磁盘 287 份 transcript |
中位 64 ms | 中位 261 ms |
也就是说，在会话很多的工作区里做一次分支大约多花 200 ms，且发生在源会话的 history-mutation 锁内。作为一次性用户操作我认为可以接受，但值得写下来。
如果之后有人想优化，有个数据可以参考：同样 265 个文件，readLines(f, 1) 耗时 121 ms，readLines(f, 10) 耗时 144 ms —— 开销主要在每文件的流打开，而不是 10 行的读取预算，所以缩小头部窗口最多只能省约 15%。真正的杠杆是复用 picker 已有的列表结果，那显然超出本 PR 范围。
6. 不阻塞合并的备注
- PR 描述已经过时。 描述里仍然写着"通过 Web Shell 的分支请求传递当前会话名称"，但最终 diff 根本没有改
packages/web-shell—— commit 1 里的那处改动已在8aac1c3中回退，改为由 daemon 自己推导名称。建议合并前修正描述，避免落库的 commit message 与实现不符。 flush()不对称。 CLI/branch在调用getSessionDisplayName之前会await outgoingRecording?.flush()，而 ACP 分支路由没有 flush 就直接读盘。我用 6 次紧凑竞态尝试都没复现出读到陈旧数据，但加一行await recording.flush()就能让两条路径对称，成本为零。Draft (1)与Draft(1)。normalizeDerivedBranchTitle的数字分支是/(\S)\(\d+\)$/，要求(前必须是非空白字符，而旧式分支则允许前导空白。实测：Draft(1)→Draft(2)（同一族），Draft (1)→Draft (1)(1)（新的嵌套族）。这看起来是有意为之 —— 只剥离生成式后缀，而生成式后缀不会带空格 —— 但设计文档里"remove an existing generated fork suffix"这句并没有说明这一点，而测试计划第 ③ 条又把Title(1)(1)描述成要避免的形态。- 归档 transcript 不在扫描范围内（
archive/被SESSION_FILE_PATTERN过滤掉），所以取消归档后可能出现重名。这一点 main 上也一样，只是说明本 PR 没有顺带解决。
7. 门禁
packages/coresessionService.test.ts—— 190/190 通过packages/cliuseBranchCommand.test.ts+acpAgent.test.ts—— 513/513 通过- 六个改动文件的
eslint --max-warnings 0—— 干净 tsc --noEmit -p packages/core—— 干净- PR CI —— 22 通过 / 0 失败
测试平台
| OS | Status |
|---|---|
| 🍏 macOS | ✅（作者） |
| 🪟 Windows | |
| 🐧 Linux | ✅（本次） |
从我这边看 LGTM —— 备注 1 的描述修正之后就可以合并。
Verified with Claude Code (Claude Opus 5, 1M context).
|
@qwen-code /triage |
|
Sandboxed verification: ✅ passed — merge-ready (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 766 passed · 0 failed · 766 total Flakiness gate: ✅ 3 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：766 通过 · 0 失败 · 766 总计 抖动门：✅ 3 changed test file(s) x 5 identical rounds, no divergence Verification reportPR 9764 Deep Verification —
|
| Cell | Oracle | head | base (control) |
|---|---|---|---|
| S1 untitled session display name | getSessionDisplayName(src) |
'Fix login bug' ✅ |
method absent — UUID fallback was the only path ✅ |
| S2 allocation, empty workspace | computeUniqueBranchTitle |
Fix login bug(1) ✅ |
Fix login bug (Branch) ✅ |
S3 …(1), …(2) taken |
allocation | Fix login bug(3) ✅ |
Fix login bug (Branch) (family invisible) ✅ |
S4 untitled session whose prompt is Fix login bug(1) |
prefix scan + allocation | scan returns it; allocates (2) ✅ |
scan returns []; legacy name ✅ |
S5 custom title Fix login bug(1) |
normalize + allocate | base Fix login bug → (2) ✅ |
no normalizer; nests Fix login bug(1) (Branch) ✅ |
| S6 fork loop end-to-end | forked JSONL contents | (1) then (2), source unchanged; fork-of-fork → (3) ✅ |
n/a (primitives absent) |
S7 foreign-project Title(1) |
allocation | does not occupy the family → (1) ✅ |
n/a |
| S8 scan window | prompt at line 10 vs line 13 | resolved / undefined ✅ |
n/a |
| Total | 16/16 | 6/6 |
Witnesses: 01-ab-head-numeric-allocation.png, 02-ab-base-control.png.
Entry-point wiring (the other half of the central claim — which base name each route feeds to the allocator) is pinned by the PR's own route-level tests, which boot the real QwenAgent.extMethod / the real useBranchCommand hook: mutations M4/M5 below prove they are load-bearing. The composed chain (route resolves base name → real allocator + scan) is exactly what the tests and the harness cover from opposite ends.
The "matches the picker" sub-claim was verified structurally: the new readSessionDisplayNameFromFile returns customTitle || firstPrompt using the same private extractFirstPromptFromRecords, the same MAX_PROMPT_SCAN_LINES = 10 head window, and the same project-membership gate as the picker hydration paths (sessionService.ts lines 1907/3219 vs 3108–3130).
Mutation matrix (vacuity + positive controls)
Each guard the PR introduces was reverted in place (scratch edit, restored afterward; tree verified clean) and the affected suites re-run. All five mutants killed; zero survivors. Every kill was the intended behavioral assertion — representative failure for M1: AssertionError: expected '创建 MR 描述生成 Skill (Branch)' to be '创建 MR 描述生成 Skill(3)' (04-mutation-m1-kill.png).
| Mutant | Guard removed | Suite | Result |
|---|---|---|---|
| M1 | computeUniqueBranchTitle → legacy (Branch) scheme |
core sessionService | 1 red / 199 |
| M1 | same | cli useBranchCommand | 17 red / 20 |
| M2 | prefix scan drops prompt-backed names (custom titles only) | core sessionService | 1 red / 199 |
| M3 | normalizeDerivedBranchTitle → identity |
core sessionService | 5 red / 195 |
| M3 | same | cli useBranchCommand | 5 red / 32 |
| M3 | same | cli acpAgent routes | 7 red / 469 |
| M4 | daemon route: no persisted display-name fallback | cli acpAgent routes | 3 red / 473 |
| M5 | CLI /branch: no display-name lookup |
cli useBranchCommand | 1 red / 36 |
Positive controls: each mutant was killed inside the very suite/file its coverage claim rests on (M2 in the core suite that also passed 199 unchanged, M4 in the acpAgent command, M5 in the useBranchCommand command), so "killed" cannot be an artifact of a dead harness. No survivors, so no survivor classification was needed. The head gates (200 + 513 green) are the unmutated controls.
Corrections
- PR body mechanism description is stale relative to the final diff. The description says the change "forwards the active Web Shell name with branch requests". In the aggregate
HEAD^1..HEADdiff the Web Shell sends no name:branchCurrentSession(undefined, atRecordId)(packages/web-shell/client/App.tsx:8294), the smoke spec asserts a name-less request body ({ atRecordId }), and the daemon resolves the display name server-side (recording.getCurrentCustomTitle()→sessionService.getSessionDisplayName()). Per the commit messages the echo existed mid-series and was deliberately removed in round 2 ("Stop Web Shell name echoing"). The user-visible goal is still met; only the described mechanism is inaccurate. This is a correction to the description, not a request to change code.
Findings
No blocking findings. Two informational boundary notes:
- (info) One-level normalization of manually nested custom titles.
normalizeDerivedBranchTitlestrips only the outermost generated suffix: a user-manually-renamed titleT(1)(2)normalizes toT(1), so a branch from it allocates inside theT(1)family (T(1)(1)). Probed inharness-siblings.mjsN2 — uniqueness is preserved and the generated flow never produces nesting (S6c: fork →T(1), fork-of-fork →T(3), neverT(1)(1)), so this is reachable only through a manual rename to an already-nested form. Not a defect; recorded so the behavior is known. The spaced siblingSprint (2)is likewise left intact — pinned intentionally by the PR's own tests. - (info) Dense-family and scan-cost bounds measured, no cliff. The removed
MAX_BRANCH_COLLISION_SCAN = 99cap is safe: the allocation loop terminates by set finiteness — 500 taken slots allocatedT(501)in 98 ms (N4). The new prompt-record scan cost the description accepts is real but small: a prefix scan over 300 untitled sessions completed in 44 ms with a 10-line head read per file (N5/N6).03-sibling-sweep-boundaries.png.
Not covered
- Per-commit attribution. Depth-2 checkout:
git rev-list HEAD^1..HEAD^2reports 1 commit at the shallow boundary vs 9 in the metadata snapshot, so individual commits (incl. the three fix rounds) were not exercised separately; the aggregateHEAD^1..HEADdiff is what was verified. - Live end-to-end through a real daemon + Web Shell UI (branch button → picker rename) and the interactive
/branchTUI path. The evidence chain is route-level agent/hook tests (real handler code, mocked allocator or mocked scan at the seams) joined to the mock-free core harness (real allocator + real scan); no TUI/browser was booted. - Playwright web-shell smoke spec was not executed (browser infra out of scope); statically checked that its branch assertion (
{ atRecordId }request body, noname) matches the final client code. - Repo-wide test suite and repo-wide lint/typecheck (only the two affected workspaces were gated); Windows/manual validation (author-marked untested); migration of pre-existing
(Branch)titles (the PR explicitly ships none — legacy names normalize only when branched from). - The base control build compiled with type-resolution errors (
ajv,ignored.ts) caused by the nested worktree missing per-packagenode_modulesat compile time; the emitted JS was verified behaviorally (the base arm cells) rather than by clean compile.
Methodology
CI verify container (node:22-bookworm), merge-ref checkout at depth 2. Base control: git worktree add tmp/base-tree HEAD^1, core rebuilt there with the repo's TypeScript (per-package node_modules symlinked from the head tree — realpath asserted as /__w/qwen-code/qwen-code/packages/core/node_modules; valid control because the PR changes no package.json/lockfile, and the harness imports the base dist/ by absolute path, so no workspace symlink can smuggle head code into the base cells). Mutation runs reverted key hunks in place via scratch edits, ran the affected vitest suites, and restored the files (git status clean afterward); backups in mutation-backup/. CLI vitest aliases @qwen-code/qwen-code-core to core source, so core mutations propagate to CLI suites without rebuilds. Gates: npx vitest run per changed test file; npm run typecheck per affected workspace (gate liveness witnessed by the same tsc invocation reporting real errors on the broken base compile). Raw logs: per-cell harness output in evidence/*.png, full-text reruns in logs/ (harness-head.txt, harness-base.txt, siblings.txt, gate-core.txt, gate-cli.txt, mutation-matrix.txt); harness sources harness-core-ab.mjs, harness-siblings.mjs in this directory. Assertion accounting: A/B harness 16 (head) + 6 (base control) + siblings 21 + head gates 200 + 513 + mutation kill-runs 8 (each expected-red counted as a pass) + typecheck 2 = 766 pass / 0 fail.
Flakiness gate log
rounds=5 files=3 skipped=0
file packages/cli/src/acp-integration/acpAgent.test.ts: (cd packages/cli) npx --no-install vitest run ./src/acp-integration/acpAgent.test.ts
file packages/cli/src/ui/hooks/useBranchCommand.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/hooks/useBranchCommand.test.ts
file packages/core/src/services/sessionService.test.ts: (cd packages/core) npx --no-install vitest run ./src/services/sessionService.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/acp-integration/acpAgent.test.ts: PPPPP
packages/cli/src/ui/hooks/useBranchCommand.test.ts: PPPPP
packages/core/src/services/sessionService.test.ts: PPPPP
verdict: pass
summary: 3 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/hooks/useBranchCommand.test.ts: P (exit 0)
round 1 · packages/core/src/services/sessionService.test.ts: P (exit 0)
round 2 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/hooks/useBranchCommand.test.ts: P (exit 0)
round 2 · packages/core/src/services/sessionService.test.ts: P (exit 0)
round 3 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 3 · packages/cli/src/ui/hooks/useBranchCommand.test.ts: P (exit 0)
round 3 · packages/core/src/services/sessionService.test.ts: P (exit 0)
round 4 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 4 · packages/cli/src/ui/hooks/useBranchCommand.test.ts: P (exit 0)
round 4 · packages/core/src/services/sessionService.test.ts: P (exit 0)
round 5 · packages/cli/src/acp-integration/acpAgent.test.ts: P (exit 0)
round 5 · packages/cli/src/ui/hooks/useBranchCommand.test.ts: P (exit 0)
round 5 · packages/core/src/services/sessionService.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
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. ✅
…quisites (QwenLM#9998) QwenLM#9764 taught /branch to read the outgoing recorder's custom title and the source session's display name before forking. The QwenLM#9844 regression suite's fake environment predates that and lacks both methods, so on main every /branch scenario throws before the fork and three tests fail. Complete the fakes; no production change.
Merge of main's persisted-session lifecycle (QwenLM#9626/QwenLM#9764) with the locked sidecar-move refactor: moveSessionPrSidecar gains the optional generation fence (threaded to writeSessionPrs assertCanCommit), conflict-repair paths merge split PR sidecars under the lock, and the fence tests assert the fence rides on the locked move.
* fix(core): sync loaded-skill state with history eviction Refs QwenLM#6762 Split from PR QwenLM#8900 per maintainer direction review: this keeps only the eviction-state sync half (loaded-skills tracking reconciled with history evictions across microcompaction, /compress-fast, memory pressure compaction, client retry, and ACP settle). The /unskill command is deferred to a separate change pending the QwenLM#6762 design discussion. * fix(core): key skill-hook dedup on the whole prepared config The dedup identity keyed only on type + command/url, but the frontmatter admits multiple hooks per matcher distinguished solely by fields that key ignores (timeout/shell for command hooks, headers/timeout for HTTP hooks): the second of such a pair was silently skipped even on the skill's first registration. Key on the whole prepared config instead — frontmatter configs carry no functions, so the structural key stays stable across reload cycles. Adds regression tests for both shapes. * fix(core): address round-2 review — setHistory reconcile, strip-occurrence gate * fix(core): gate skill-body residency on SkillTool provenance - Record exact body outputs in SkillTool as residency provenance; all residency checks fail closed against a fresh process (empty set) instead of trusting the two public markers (spoofable via the shared functionResponse{name:'skill'} shape) - Make setHistory's reconcile the single loaded-skill sync: remove the five post-setHistory second writes (3x syncSkillEvictions, hard-rescue reconcile, tryCompress blanket clear) and delete the now-dead helpers - Correct the settle-reconcile comment to name its five pre-try skip paths; resume stays intentionally un-enumerated (fail-closed reconcile plus one bounded duplicate body beats a marker-based resume door that would reopen the injection window) - Tests: provenance gating (injection/genuine/spoof-stripped), stateful Set oracle for clear-before-track order, microcompact F2 witness and size-path kept-body twin, mixed-batch unload, isRetry settle arming, wire-after-setup hard-rescue, reconcile-only tryCompress, and the three second-write sites flipped to single-writer assertions * fix(core): classify stripped skill responses by call id (R4-1) The scheduler's persistence gate rewrites large genuine skill bodies into <persisted-output> stubs before they enter history. The strip path's provenance/shape check skipped such stubs, leaving the skill tracked with no resident body — the QwenLM#6762 deadlock. unloadSkillsFromEntries now classifies every stripped skill response by call-id resolution: fail-open direction, since over-un-tracking self-heals with one duplicate body while under-un-tracking deadlocks reload; the resident-sibling filter keeps precision. Also pins the R4 findings: provenance recording both halves through the real SkillTool, microcompact provenance wiring at the client call site, the ambiguous call-id policy on both strip and reconcile sides, provenance-mode targeted unload, compressFast's setHistory reconcile, and the forked-chat compression/restore observable effects. Rewrites the evictedSkillNames docstrings to the diagnostic-only contract. * test(cli): complete stale fakes in session-swap telemetry test QwenLM#9764 added getCurrentCustomTitle/getSessionDisplayName calls to useBranchCommand after QwenLM#9844's test fakes were written, so the suite throws before forkSession on any branch carrying both. Fill the two missing fake methods; main CI has not run since before either landed. * refactor(core): reduce skill-eviction sync to conservative clear at rewrite boundaries Per QwenLM#9500 review: drop the process-wide body provenance set and history scans, the evictedSkillNames/unresolvedEvictedSkills diagnostics, the exact-reconcile vs targeted-unload algorithms, and the ACP settle machinery. Instead, conservatively clear loaded-skill tracking at the three destructive history-rewrite boundaries (setHistory covers every compaction path, truncateHistory, stripOrphanedUserEntriesFromHistory), guarded so forked chats never touch the parent's tracker. Over-clearing self-heals with at most one duplicate body on the next invoke; a stale entry made the skill permanently unreloadable. Kept: authoritative-vs-forked ownership guard, idempotent hook and allowedTools re-registration on reload, and a small behavioral test set. Exact lifecycle semantics, explicit unload, and richer eviction diagnostics belong under QwenLM#6762 in follow-up PRs. --------- Co-authored-by: 俊良 <zzj542558@alibaba-inc.com> Co-authored-by: yiliang114 <yiliang.yyl@alibaba-inc.com> Co-authored-by: 易良 <1204183885@qq.com>
|
Released in v0.22.2. |
6 participants
What this PR does
This PR makes branched sessions inherit the source session's picker display name instead of falling back to a UUID fragment. It normalizes existing legacy
(Branch)and numeric fork suffixes, allocates the lowest availableTitle(N)name across custom-title and prompt-backed display names, forwards the active Web Shell name with branch requests, applies the numeric convention to CLI branching, and preserves explicit side-task names unchanged.Why it's needed
Sessions whose visible name came from their first prompt could produce a random-looking, UUID-derived title after branching. Keeping branches in the same numbered name family makes copied sessions recognizable and avoids reusing an existing branch display name.
Reviewer Test Plan
How to verify
Title(1)using the source picker name asTitle.Title(1)andTitle(2), branch the source again, and confirm the next session is namedTitle(3)while the source session remains unchanged.Title(1)(1).Side task(2)and confirm that exact name is preserved.Evidence (Before & After)
Before
After
Tested on
Environment (optional)
Risk & Scope
(Branch)convention to numeric(N)suffixes; the commit does not migrate existing session titles and identifies no API or data migration.Linked Issues
Closes #9762
What this PR does
本 PR 让分支会话继承源会话在会话选择器中的展示名称，而不是回退为 UUID 片段。它会归一化已有的旧式
(Branch)和数字 fork 后缀，在自定义标题与基于首条 prompt 的展示名称中选择最小可用的Title(N)名称，通过 Web Shell 的分支请求传递当前会话名称，将数字编号约定应用于 CLI 分支，并保持显式 side task 名称不变。Why it's needed
当会话的可见名称来自首条 prompt 时，分支后的标题可能退化为看起来随机的 UUID 派生名称。让分支保持在同一个编号名称族中，可以使复制出的会话易于识别，并避免复用已存在的分支展示名称。
Reviewer Test Plan
How to verify
Title，命名为Title(1)。Title(1)和Title(2)的会话，再次从源会话创建分支，确认新会话命名为Title(3)，且源会话保持不变。Title(1)(1)之类的嵌套名称。Side task(2)之类的显式名称创建 side task，并确认该名称被原样保留。Evidence (Before & After)
审阅的 commit 未提供截图、录屏或运行时日志。
Tested on
Environment (optional)
Risk & Scope
(Branch)约定改为数字(N)后缀；该 commit 不会迁移已有会话标题，也未发现 API 或数据迁移。Linked Issues
Closes #9762