Skip to content

fix(cli): show Goal objectives in session picker - #10295

Open
qqqys wants to merge 8 commits into
QwenLM:mainfrom
qqqys:fix/resume-goal-objective
Open

fix(cli): show Goal objectives in session picker#10295
qqqys wants to merge 8 commits into
QwenLM:mainfrom
qqqys:fix/resume-goal-objective

Conversation

@qqqys

@qqqys qqqys commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Goal-only sessions now use their persisted Goal objective as the fallback label in the resume picker when no custom title or user prompt exists. The same fallback is used in the preview, and picker search can match the displayed objective. Existing custom-title and prompt precedence remains unchanged.

Why it's needed

Sessions started with /goal can contain no ordinary user prompt, so /resume previously rendered them as (empty prompt) even though the Goal objective clearly identifies the work. This makes those sessions recognizable and searchable without changing their stored transcript format.

Reviewer Test Plan

How to verify

Create a session with /goal, leave it without a custom title or ordinary prompt, then open /resume. Confirm that the picker row and preview show the latest persisted Goal objective instead of (empty prompt), and that searching for text from the objective finds the session. Also confirm that a custom title still wins over a prompt or Goal objective, and a prompt still wins over the Goal objective.

Evidence (Before & After)

Before: Goal-only sessions appeared as (empty prompt) in the picker and had no useful preview title.

After: Goal-only sessions display their latest Goal objective in both locations and can be found by objective text. Automated regression coverage verifies the picker row, preview, search behavior, and latest-objective recovery.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

Environment (optional)

Node.js 22; targeted core and CLI unit tests, package typechecks, related ESLint checks, and the repository build.

Risk & Scope

  • Main risk or tradeoff: Sessions without both a title and prompt perform one existing bounded transcript-tail lookup to recover the latest Goal objective.
  • Not validated / out of scope: Manual interactive verification on Windows and Linux; session storage and Goal lifecycle behavior are unchanged.
  • Breaking changes / migration notes: None.

Linked Issues

Fixes #10265

中文说明

本 PR 的改动

仅包含 Goal 的会话在没有自定义标题和普通用户提示词时,现在会使用持久化的 Goal objective 作为 /resume 选择器中的后备名称。预览区使用同一后备逻辑,选择器搜索也可以匹配当前显示的 objective。原有的自定义标题和提示词优先级保持不变。

为什么需要

通过 /goal 启动的会话可能没有普通用户提示词,因此 /resume 之前会把它们显示为 (empty prompt),即使 Goal objective 已经清楚描述了工作内容。此改动让这些会话无需修改存储格式即可被识别和搜索。

Reviewer Test Plan

如何验证

使用 /goal 创建一个会话,不设置自定义标题,也不添加普通提示词,然后打开 /resume。确认选择器行和预览区显示最新持久化的 Goal objective,而不是 (empty prompt);同时确认使用 objective 中的文本搜索可以找到该会话。还应确认自定义标题仍优先于提示词和 Goal objective,提示词仍优先于 Goal objective。

前后对比证据

改动前:仅包含 Goal 的会话在选择器中显示为 (empty prompt),预览区也没有有用的标题。

改动后:仅包含 Goal 的会话在选择器和预览区都会显示最新 Goal objective,并可通过 objective 文本搜索。自动化回归测试覆盖了选择器行、预览、搜索行为和最新 objective 的恢复。

测试平台

OS 状态
🍏 macOS
🪟 Windows ⚠️
🐧 Linux ⚠️

环境(可选)

Node.js 22;执行了 core 与 CLI 定向单元测试、包级类型检查、相关 ESLint 检查以及仓库构建。

风险与范围

  • 主要风险或取舍:同时缺少标题和提示词的会话会使用现有的有界 transcript 尾部读取来恢复最新 Goal objective。
  • 未验证或范围外:未在 Windows 和 Linux 上手动进行交互验证;会话存储和 Goal 生命周期行为未改变。
  • 破坏性变更或迁移说明:无。

关联 Issue

Fixes #10265

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Qwen Triage finished — CI landed green on f4f202f and the deferred approval was posted. finalize run

Qwen Triage 已完成 —— f4f202f 的 CI 全绿,延迟审批已提交。查看 finalize 运行

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template ✓ — all sections filled in, including the bilingual summary and a reviewer test plan.

Problem — observed bug, not theory: the linked issue #10265 comes from a community user report (with screenshot) showing multiple /goal sessions all rendered as (empty prompt) in /resume on 0.22.x, making goal sessions impossible to tell apart. Clear reproduction context.

Direction — aligned. /goal is a first-class feature, and sessions it creates being unidentifiable in the resume picker undermines it. The change stays inside display/search fallback logic — session storage and the Goal lifecycle are untouched. No direct CHANGELOG reference, but the area is plainly relevant.

Size — touches core (packages/core/src/services/sessionService.ts): 64 production logic lines (SessionPicker.tsx 12 + sessionPickerUtils.ts 8 + sessionService.ts 44) vs. 132 test lines. Well under any escalation threshold.

Approach — minimal and pattern-conforming: an optional goalObjective field, populated only when title and prompt are both missing, reusing the same bounded tail/head-window reader customTitle already uses (same pooled scratch buffer), with a fallback through the already-loaded head records via the existing recoverGoalFromRecords (v2 + legacy). Picker row, preview, and search all consume the one value. No scope creep spotted — every edit serves the stated goal.

Risk — no changed file matches the revert-correlated high-risk paths. No elevated risk signals.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板 ✓ — 各部分齐全,包含中英双语说明和 reviewer 测试计划。

问题 — 真实已观测的 bug,不是理论问题:关联 issue #10265 来自社区用户反馈(附截图),0.22.x 上多个 /goal 会话在 /resume 中全部显示为 (empty prompt),goal 会话无法区分。复现背景清晰。

方向 — 对齐。/goal 是一等特性,它创建的会话在恢复列表里无法识别会削弱该特性。改动只落在展示/搜索的回退逻辑上,不触碰会话存储和 Goal 生命周期。CHANGELOG 无直接引用,但该领域明显相关。

规模 — 触及 core(packages/core/src/services/sessionService.ts):64 行生产逻辑(SessionPicker.tsx 12 + sessionPickerUtils.ts 8 + sessionService.ts 44),对应 132 行测试。远低于任何升级阈值。

方案 — 最小且符合既有模式:新增可选 goalObjective 字段,仅在标题与 prompt 都缺失时填充;复用 customTitle 已在使用的同一有界尾/头窗口读取(同一池化缓冲),并以已加载的头部记录经现有 recoverGoalFromRecords(v2 + legacy)兜底。列表行、预览和搜索共用同一取值。未发现范围蔓延——每处改动都服务于既定目标。

风险 — 改动文件均未命中与 revert 相关的高风险路径。无升级风险信号。

进入代码审查 🔍

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code review

Before reading the diff, my own proposal for this problem was: add an optional goalObjective to the session list item, populate it lazily only for sessions with neither title nor prompt, reuse the bounded tail/head-window reader that already serves customTitle, and show it as the last fallback before (empty prompt) in the row, the preview, and search. This PR does exactly that — I found no simpler path it missed.

Verified against the current tree:

  • recoverGoalFromRecords' return shape (v2payload.snapshot.goal?.objective, legacyobjective, otherwise undefined) matches how extractGoalObjectiveFromRecords consumes it, and GoalSnapshotV2.goal is GoalRecord | null, so the optional chaining is correct.
  • readLastJsonStringFieldSync(filePath, 'objective', '"subtype":"goal_state"', tailBuffer) reuses the bounded ≤128KB tail+head scan, and the listing path passes the existing pooled scratch buffer — no new per-session allocation, no full-file read.
  • The lookup only runs when both prompt and titleInfo.title are empty, so ordinary sessions pay nothing.
  • Both producers feeding the picker (listSessions, getSessionListItem) set the field; the third producer findSessionsByTitle only matches titled sessions, so it doesn't need to. The only (empty prompt) render site for session list items is the patched one (RewindSelector's copy is a different feature).
  • Display truncation reuses truncatePromptForDisplay, same as prompts. The ??|| change in the preview title is a small improvement for the empty-string edge; precedence stays title > prompt > objective.
  • One known boundary (not a defect): goal_state records are not re-anchored to EOF the way titles are, so a very long session whose goal was created early and never updated could miss the file-window read; the head-records fallback (first 10 lines, where a /goal session's creation record lives) catches that case. This is the same documented completeness tradeoff customTitle already lives with — picker latency over completeness.

No blockers, no convention violations. Tests cover the picker row, preview title, search matching, listing exposure, and latest-objective recovery — 132 test lines against 64 production lines.

Testing

Evidence from the PR's own CI at the reviewed commit, fetched via the API — no PR code is built or executed in this review. Linux unit suite and Serve A/B were still running at review time; the macOS/Windows platform variants and the integration suite are skipped in this run. No failures so far. The finalize job updates the table below once CI settles.

Final CI results for f4f202f (auto-updated by the triage finalize job after CI completed):

Check Conclusion
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Secret scan (TruffleHog) ✅ success
Serve A/B (ubuntu-latest, Node 22.x) ✅ success
Test (ubuntu-latest, Node 22.x) ✅ 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,失败项排在最前。

Sandboxed verification would settle the remaining gap: @qwen-code /tmux — whether the /resume picker actually renders the Goal objective for a real goal-only session (and search finds it) is not proven until the suite lands green, and the author self-reports testing on macOS only. The author is a collaborator, so the lane can be triggered directly.

Real-scenario (tmux) testing: not run — unattended CI never executes PR code; the TUI surface is exercised only by the isolated @qwen-code /tmux job above. The author's "tested on macOS" line is their own claim, not independently re-run evidence.

中文说明

代码审查

在看 diff 之前,我自己的方案是:给会话列表项加可选 goalObjective,仅在既无标题也无 prompt 的会话上惰性填充,复用已为 customTitle 服务的有界尾/头窗口读取,并在列表行、预览与搜索中作为 (empty prompt) 之前的最后回退。PR 的实现与此完全一致——没有找到更简的遗漏路径。

对照当前主干逐项验证:

  • recoverGoalFromRecords 的返回形态(v2payload.snapshot.goal?.objectivelegacyobjective,其余 undefined)与 extractGoalObjectiveFromRecords 的消费方式一致;GoalSnapshotV2.goalGoalRecord | null,可选链写法正确。
  • readLastJsonStringFieldSync(filePath, 'objective', '"subtype":"goal_state"', tailBuffer) 复用 ≤128KB 的有界尾+头扫描,列表路径传入现有池化缓冲——不新增每会话分配,不做全文件读取。
  • 仅当 prompttitleInfo.title 均为空时才执行查找,普通会话零开销。
  • 喂给选择器的两个生产者(listSessionsgetSessionListItem)都设置了该字段;第三个生产者 findSessionsByTitle 只匹配有标题的会话,无需设置。会话列表项唯一的 (empty prompt) 渲染点已被修改(RewindSelector 里的同名文案属于另一功能)。
  • 展示截断复用 truncatePromptForDisplay,与 prompt 一致。预览标题由 ?? 改为 ||,对空字符串边界是小幅改进;优先级保持 标题 > prompt > objective。
  • 一个已知边界(非缺陷):goal_state 记录不像标题那样被重新锚定到文件尾,极长会话中"早期创建且从未更新"的 goal 可能落在文件窗口读取之外;此时由头部记录回退(前 10 行,/goal 会话创建记录所在位置)兜底。这与 customTitle 已有的、文档化的"选择器延迟优先于完备性"取舍一致。

无阻塞项,无规范违规。测试覆盖列表行、预览标题、搜索匹配、列表字段暴露与最新 objective 恢复——132 行测试对应 64 行生产代码。

测试

证据来自该 PR 自身在被审提交上的 CI,经 API 获取——本审查不构建、不运行任何 PR 代码。审查时 Linux 单元测试与 Serve A/B 仍在运行;macOS/Windows 平台变体与集成套件本轮被跳过。目前无失败。CI 落定后 finalize 任务会更新上方表格。

沙箱验证可以补上剩余缺口:@qwen-code /tmux/resume 选择器在真实 goal 会话下是否确实渲染 Goal objective、搜索是否能命中,要等套件全绿才能确证;且作者自述仅在 macOS 上自测。作者是 collaborator,可直接触发该通道。

真实场景(tmux)测试:未执行——无人值守 CI 从不运行 PR 代码;TUI 表面仅由上述隔离的 @qwen-code /tmux 任务覆盖。作者"已在 macOS 测试"为其自述,非独立复核证据。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 5/5 — clean across every stage; would merge without hesitation once CI lands.

Stepping back: the problem is observed and specific (community report with screenshot, issue #10265), the solution is exactly the minimal approach I'd have proposed independently — an optional field, populated lazily through the same bounded reader customTitle already uses, consumed in row, preview, and search — and the review found no blockers, no scope creep, and no convention violations. Every edit in the diff serves the stated goal, and the test coverage (132 lines against 64 of production code) pins the picker row, the preview, the search behavior, and latest-objective recovery. If I had to maintain this in six months, I'd thank the author: it follows the file's own patterns instead of inventing new ones.

The only reservations are inherent to the established bounded-read pattern this PR correctly reuses (windowed reads trade completeness for picker latency), not defects of this diff.

CI at the reviewed commit is still in flight (Linux unit suite and Serve A/B; security checks already green), so approval is deferred until CI lands green on f4f202f6ac780e1e8c75e4435e8112da5f128092.

中文说明

置信度:5/5 — 各阶段均干净;CI 一通过即可毫不犹豫合入。

整体复盘:问题真实且具体(社区反馈附截图,issue #10265);方案与我独立提出的最小做法完全一致——新增可选字段,经由 customTitle 已在使用的同一有界读取器惰性填充,在列表行、预览与搜索中统一消费;审查未发现阻塞项、范围蔓延或规范违规。diff 中每处改动都服务于既定目标,测试覆盖(132 行测试对 64 行生产代码)钉住了列表行、预览、搜索行为与最新 objective 恢复。若六个月后维护这段代码,只会感谢作者:它沿用了文件自身的既有模式,而非另起炉灶。

唯一的保留意见来自该 PR 正确复用的既有有界读取模式的固有取舍(以选择器延迟优先于完备性),并非本 diff 的缺陷。

被审提交上的 CI 仍在进行(Linux 单元测试与 Serve A/B;安全检查已通过),因此在 CI 于 f4f202f6ac780e1e8c75e4435e8112da5f128092 全绿之前暂缓批准。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

🩺 serve daemon A/B

Built the PR base vs this PR head 9265a95, 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.

LGTM, looks ready to ship — CI landed green after the review. ✅

@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.

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

中文说明

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

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

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

Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/services/sessionService.ts
Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/services/sessionService.ts Outdated
…d label

Review findings on QwenLM#10295:

- R1-1 (Critical): the tail scan kept the last `objective` found on ANY
  `goal_state` line, so a `/goal clear` record — which persists `goal: null`
  and no objective at all — left the picker labelling the session with the
  goal the user had just cleared. `readLastMatchingLineFieldSync` reads the
  field from the LAST marker-carrying line instead, so the newest lifecycle
  record decides even when it omits the field. Its miss is now three-way:
  only `absent` (whole file scanned) lets the records fallback speak. A
  head-window hit is gone entirely — for a file bigger than the window it
  would resurrect a create record with an unknown number of later lifecycle
  records out of reach, and the parsed records are that same oldest slice of
  the file, so the honest answer there is no label.
- R1-5: the pasted-twice policy block is now one `resolveGoalObjective`
  helper called from both `listSessions` and `getSessionListItem`.
- R1-2/R1-3/R1-4/R1-7: tests for the legacy recovery arm, the
  prompt/title suppression guard at both producers, the `getSessionListItem`
  wiring, and the production file scan itself — the last one drives a real
  transcript whose goal record sits past the ten lines the records fallback
  parses, which is the only shape where a dead marker cannot hide behind it.
  The spy-based "prefers the latest objective" test is gone; it asserted the
  spy, not the code.
- R1-6: the other four label surfaces stay as they are; the asymmetry is now
  a documented decision on `SessionListItem.goalObjective` rather than an
  accident, with the shape a follow-up would take.

Mutation-verified: reverting the read to "last objective on any goal_state
line" reddens 3; letting the records fallback answer an out-of-window scan
reddens 1; a marker typo or a renamed field reddens 1 and 4; dropping the
suppression guard reddens 3; dropping the `getSessionListItem` wiring or the
legacy arm reddens 1 each.

Claude-Session: https://claude.ai/code/session_01VXsC4f71S6U6YkW82NRw7m

@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.

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

中文说明

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

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

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

Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/services/sessionService.test.ts
Comment thread packages/core/src/utils/sessionStorageUtils.ts Outdated
Comment thread packages/core/src/utils/sessionStorageUtils.ts
Comment thread packages/core/src/utils/sessionStorageUtils.ts Outdated
Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/utils/sessionStorageUtils.ts Outdated
Comment thread packages/core/src/utils/sessionStorageUtils.ts Outdated

@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 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 3, not a blocker) — recorded, not requested in this round:

  • packages/core/src/utils/sessionStorageUtils.ts:330 — [review] New reader inverts the sibling (key, marker) parameter order

Convergence: round 3 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 8 (8 new). Findings keep coming back to the same files: packages/core/src/services/sessionService.ts (findings in round 2; 1 more now); packages/core/src/utils/sessionStorageUtils.ts (findings in round 2; 1 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.)

中文说明

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

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

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

收敛情况:第 3 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 8 条(其中 8 条首次提出)。发现反复回到同一批文件:packages/core/src/services/sessionService.ts(第 2 轮已出过发现,本轮又有 1 条);packages/core/src/utils/sessionStorageUtils.ts(第 2 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。本轮没有未决的 Critical,因此"合入后把剩余 Suggestion 线程转到后续 issue"是一个可选的结束方式——已合入的 PR 不会继续发散。(仅为观察——本轮评审未因此扣留任何内容。)

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

Comment thread packages/core/src/utils/sessionStorageUtils.ts Outdated
Comment thread packages/core/src/services/sessionService.ts Outdated

@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.

1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:

  • Parameter-order inversion of readLastMatchingLineFieldSync / extractJsonStringFieldFromLastMatchingLine (marker before key, siblings are key-first) — already reported in the round-3 deferral list (review 5054292672)

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

Convergence: round 4 posted 4 inline comment(s), 4 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/core/src/services/sessionService.ts (findings in round 3; 2 more now); packages/core/src/utils/sessionStorageUtils.ts (findings in round 2; 2 more now). The rate of new findings is not falling. 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. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

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

本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。

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

收敛情况:第 4 轮发布了 4 条行内评论,其中 4 条是首次提出;上一轮发布了 2 条(其中 2 条首次提出)。发现反复回到同一批文件:packages/core/src/services/sessionService.ts(第 3 轮已出过发现,本轮又有 2 条);packages/core/src/utils/sessionStorageUtils.ts(第 2 轮已出过发现,本轮又有 2 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)

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

Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/utils/sessionStorageUtils.ts Outdated
Comment thread packages/core/src/services/sessionService.ts Outdated
Comment thread packages/core/src/utils/sessionStorageUtils.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.

Partially reviewed — gaps disclosed. Suggestions are inline.

⚠️ Round 5, and the diff has grown 3.1x since this review first measured it (171 → 526 source diff lines). The findings below are anchored to the current patch, so they can only say where this approach leaks — never that a different approach would retire all of them at once. Before fixing them, a human should decide whether the shape of the change is still right. Advisory only: this does not affect the verdict, and nothing here is a blocker.

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 probing — all probes inconclusive (harnessValidated: null; the probe runner tripped the repo's vitest prerequisite guard), neither validated nor refuted.

Convergence: round 5 posted 3 inline comment(s), 3 of them reported for the first time; the previous round posted 4 (4 new). Findings keep coming back to the same files: packages/core/src/utils/sessionStorageUtils.ts (findings in round 4; 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. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

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

⚠️ 第 5 轮,且自本审查首次测量以来 diff 已增长 3.1 倍(源码 diff 行数 171 → 526)。下方的发现都锚定在当前这版补丁上,因此它们只能指出这个方案在哪里漏了,而无法说明换一个方案就能一次性消除全部问题。在动手修复之前,应由人来判断这次改动的整体形态是否仍然正确。仅供参考:本段不影响判定结论,其中也没有任何阻断项。

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

未审查:test-efficacy probing — all probes inconclusive (harnessValidated: null; the probe runner tripped the repo's vitest prerequisite guard), neither validated nor refuted。

收敛情况:第 5 轮发布了 3 条行内评论,其中 3 条是首次提出;上一轮发布了 4 条(其中 4 条首次提出)。发现反复回到同一批文件:packages/core/src/utils/sessionStorageUtils.ts(第 4 轮已出过发现,本轮又有 2 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)

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

Comment thread packages/core/src/utils/sessionStorageUtils.ts
Comment thread packages/core/src/services/sessionService.test.ts Outdated
Comment thread packages/core/src/utils/sessionStorageUtils.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.

Partially reviewed — gaps disclosed.

⚠️ Round 6, and the diff has grown 3.1x since this review first measured it (171 → 526 source diff lines). The findings below are anchored to the current patch, so they can only say where this approach leaks — never that a different approach would retire all of them at once. Before fixing them, a human should decide whether the shape of the change is still right. Advisory only: this does not affect the verdict, and nothing here is a blocker.

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

Not reviewed: build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) unit matrices were skipped in CI; unit suites ran on Linux only.

Not reviewed: test-efficacy probing — all probes inconclusive (harnessValidated: null; the probe runner tripped the repo's vitest prerequisite guard), neither validated nor refuted.

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

  • packages/core/src/services/sessionService.test.ts:7317 — [review] Negative goal tests never assert the session item exists
  • packages/core/src/utils/sessionStorageUtils.ts:327 — [review] Brace-recovery fallback's positive path has no test
  • packages/core/src/services/sessionService.ts:1985 — [review] The records-recovery 'none' branch is unpinned
  • packages/core/src/services/sessionService.ts:1768 — [review] Whole-file inference rests on an undocumented cross-module invariant
  • packages/core/src/utils/sessionStorageUtils.ts:15 — [review] Production consumer of a 'unit tests only' export

Convergence: round 6 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 3 (3 new). Findings keep coming back to the same files: packages/core/src/utils/sessionStorageUtils.ts (findings in rounds 4, 5; 1 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. (Observation only — nothing was withheld from this review because of this observation.)

中文说明

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

⚠️ 第 6 轮,且自本审查首次测量以来 diff 已增长 3.1 倍(源码 diff 行数 171 → 526)。下方的发现都锚定在当前这版补丁上,因此它们只能指出这个方案在哪里漏了,而无法说明换一个方案就能一次性消除全部问题。在动手修复之前,应由人来判断这次改动的整体形态是否仍然正确。仅供参考:本段不影响判定结论,其中也没有任何阻断项。

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

未审查:build-and-test — Test (macos-latest, Node 22.x) and Test (windows-latest, Node 22.x) unit matrices were skipped in CI; unit suites ran on Linux only。

未审查:test-efficacy probing — all probes inconclusive (harnessValidated: null; the probe runner tripped the repo's vitest prerequisite guard), neither validated nor refuted。

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

收敛情况:第 6 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 3 条(其中 3 条首次提出)。发现反复回到同一批文件:packages/core/src/utils/sessionStorageUtils.ts(第 4、5 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)

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

let markerOffset = line.lastIndexOf(lineContains);
while (markerOffset >= 0) {
const recordStart = line.lastIndexOf('{', markerOffset);
if (recordStart >= 0 && line[recordStart - 1] !== ':') {

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.

[Critical] R5-1: (fix-induced) [certifies-falsely] [new-surface] The torn/glued-line Goal recovery in extractJsonStringFieldFromLastMatchingLine keeps producing new corners: four successive rounds patched siblings of this surface (byte discriminator, structural predicate, suffix-first ordering, fragment rejection), and the round-6 commit that closed R5-1's input opened a new entrance at the same loop, so this finding is carried forward under R5-1's id. The : guard added in 9265a95de2 cannot tell a goal_state fragment nested inside a torn record from a complete record glued after a torn prefix, and the scan still penetrates to older lines when the newest marker cannot be attributed. Entrance (1), opened by the R5-1 fix itself: a /goal clear record glued onto a torn prefix that ends right at a key's colon (e.g. {"type":"system","subtype":"note","systemPayload": followed directly by the clear record) has its opening { rejected by this guard, _recoverObjectsFromLine recovers nothing because the torn brace keeps depth ≥ 1, and the scan falls through to the older create record — a session whose Goal the user cleared is labelled with the cleared objective in the /resume picker (row, preview, and search), via both listSessions and getSessionListItem. Before this guard existed the clear's suffix parsed and answered matched-undefined, so the resurrection does not fire at the round-5 head. Entrance (2): a goal_state-shaped object nested in an array inside a torn non-Goal record (..."items":[{"type":"system","subtype":"goal_state","objective":"injected"}] cut at the element's }) passes the guard — { preceded by [ — parses, satisfies the shallow top-level predicate, and labels a prompt-less session with fabricated model text; the authoritative parseGoalStateRecordPayloadV2 reader rejects the same fragment, so the two readers disagree.

Witness:

Probe over committed code, end-to-end through the real SessionService:
entrance 1  ENTRANCE1_SERVICE_LIST {"exists":true,"prompt":"","goalObjective":"Write the release notes"}   <- cleared goal resurrected
entrance 2  ENTRANCE2_SERVICE_LIST {"exists":true,"prompt":"","goalObjective":"injected"}                   <- fabricated label
control (pinned torn-mid-string shape)  CONTROL_FUNCTION {"matched":true}  (value undefined)
minimal fail-closed candidate patch -> entrance 1 unlabelled, all 74 committed sessionStorageUtils tests green

Close it structurally rather than entrance by entrance: when the newest marker occurrence cannot be attributed to a standalone record (guard-rejected suffix and empty brace recovery), make that line decisive — answer matched-with-no-value or unmatched — and never penetrate to older lines; additionally interpret parsed candidates with the authoritative goal-payload parser (parseGoalStateRecordPayloadV2 via a caller-supplied callback) instead of the shallow top-level type/subtype predicate plus first-"objective"-string extraction, so the scan can never disagree with recoverGoalFromRecords.

Fix constraint: a clear record must remain matched-with-no-value, never unmatched, or a cleared goal resurrects the previous create's objective (pinned by 'does not resurrect an objective the user cleared'); keep 'rejects a nested marker at the end of a torn containing record' (packages/core/src/utils/sessionStorageUtils.test.ts) and 'ignores a nested Goal marker in a non-Goal record' (packages/core/src/services/sessionService.test.ts) green; the writer forces snapshot.activity to 'idle' (packages/core/src/services/chatRecordingService.ts:1796), so every record that writer emits today must keep being accepted.

Fix witness: extend the it.each at packages/core/src/services/sessionService.test.ts:7326 ('does not resurrect a clear glued after a %s torn record') with a torn prefix ending in : — it is red at this commit — and add a sessionStorageUtils.test.ts case for a torn record ending in a goal-shaped array element asserting no label — also red at this commit; removing the structural fix must turn them red again.

中文说明

extractJsonStringFieldFromLastMatchingLine 中针对截断/粘连行的 Goal 恢复逻辑不断产生新的漏洞入口:连续四轮修复的都是同一表面的兄弟问题(字节级判别、结构化谓词、后缀优先排序、嵌套片段拒绝),而关闭 R5-1 入口的第 6 轮提交在同一个循环上又打开了一个新入口,因此本发现沿用 R5-1 的编号继续报告。9265a95de2 新增的 : 守卫无法区分"截断记录内部嵌套的 goal_state 片段"与"粘连在截断前缀之后的完整记录",且当最新 marker 无法归属时扫描仍会穿透到更旧的行。入口 (1)(由 R5-1 修复本身引入):一条 /goal clear 记录粘连在恰好结束于键冒号的截断前缀之后(如 {"type":"system","subtype":"note","systemPayload": 后直接跟 clear 记录)时,clear 的开括号 { 被该守卫拒绝,_recoverObjectsFromLine 因截断花括号使深度始终 ≥ 1 而恢复不出任何内容,扫描随即回落到更早的 create 记录——用户已清除 Goal 的会话在 /resume 选择器(列表行、预览、搜索)中被标上已清除的 objective,listSessionsgetSessionListItem 均复现。在该守卫加入之前,clear 的后缀可以正常解析并返回 matched-undefined,第 5 轮代码上不会复活,因此这个入口正是 R5-1 修复打开的。入口 (2):嵌套在非 Goal 截断记录数组中的 goal_state 形状对象(..."items":[{"type":"system","subtype":"goal_state","objective":"injected"}] 恰在元素 } 处截断)——{ 前驱字符是 [,通过守卫,片段解析成功并满足浅层顶层谓词,使无提示词会话被标上来自模型文本的伪造标签;权威读取器 parseGoalStateRecordPayloadV2 会拒绝同一片段,两个读取器结论不一致。

证据(探针):在已提交代码上通过真实 SessionService 端到端运行,入口 1 返回 goalObjective: "Write the release notes"(已清除的 goal 被复活),入口 2 返回 goalObjective: "injected"(伪造标签);对照组(已固定的字符串中部截断形态)正确返回 value 为 undefined;应用最小化的 fail-closed 候选修复后入口 1 不再标记,且 sessionStorageUtils 全部 74 个测试保持绿色。

请从结构上封闭而非逐入口修补:当最新 marker 无法归属到独立记录(后缀被守卫拒绝且花括号恢复为空)时,让该行具有决定性——返回 matched 且无值,或 unmatched——绝不再穿透到更旧的行;同时用权威的 goal 载荷解析器(parseGoalStateRecordPayloadV2,通过调用方提供的回调)替代浅层顶层 type/subtype 谓词加首个 "objective" 字符串提取来解释解析出的候选,使扫描结果永远不会与 recoverGoalFromRecords 分歧。

修复约束:clear 记录必须保持"matched 且无值",绝不能变成未匹配,否则已清除的 goal 会复活上一条 create 的 objective(由 'does not resurrect an objective the user cleared' 固定);保持 'rejects a nested marker at the end of a torn containing record'(packages/core/src/utils/sessionStorageUtils.test.ts)与 'ignores a nested Goal marker in a non-Goal record'(packages/core/src/services/sessionService.test.ts)绿色;写入端强制 snapshot.activity'idle'(packages/core/src/services/chatRecordingService.ts:1796),因此该写入器当前产出的所有记录必须继续被接受。

修复验证:在 packages/core/src/services/sessionService.test.ts:7326 的 it.each'does not resurrect a clear glued after a %s torn record')中新增以 : 结尾的截断前缀变体——在本提交上是红的——并在 sessionStorageUtils.test.ts 新增截断记录以 goal 形状数组元素结尾的用例、断言不产生标签——在本提交上同样是红的;移除该结构性修复后两者必须重新变红。

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

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.

/resume picker shows "(empty prompt)" for /goal sessions — fall back to the Goal objective

2 participants