Skip to content

fix(vscode): unify session history and remove source switch - #11713

Merged
yiliang114 merged 4 commits into
mainfrom
codex/unify-vscode-session-history
Sep 12, 2026
Merged

fix(vscode): unify session history and remove source switch#11713
yiliang114 merged 4 commits into
mainfrom
codex/unify-vscode-session-history

Conversation

@yiliang114

@yiliang114 yiliang114 commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Replaces the VS Code / Terminal source selector introduced by #11584 with one history list for ordinary conversations in the current workspace. Terminal, browser, VS Code, and legacy conversations share the same entry point and row actions. Existing conversations retain their source attribution when opened; only newly created Companion conversations receive VS Code attribution.

The list uses the existing organized workspace catalog, keeps an explicit Load more action for sparse pages, and preserves the cursor on failed requests. Source ownership sidecars and legacy allowlist scanning are no longer needed.

Why it's needed

The confirmed product requirement is to open terminal-created conversations directly from VS Code without selecting their origin. The source selector in #11584 did not meet that requirement. Creation source is not a user-facing boundary between ordinary conversations in the same workspace; workspace scoping and background-task lifecycle protections remain separate concerns.

Reviewer Test Plan

How to verify

  • Open history in a workspace containing Terminal, VS Code, and legacy conversations. They should appear together without a source selector.
  • Open a terminal conversation, verify its transcript, then reload the extension host. The same conversation should restore without rewriting its source metadata.
  • Create a new conversation in VS Code. It should appear in the same list and retain VS Code creation attribution.
  • Confirm ordinary rows offer rename and delete, while deleting the current conversation remains protected. Load more should remain reachable when a page contains only filtered child/background records, and a failed request should be retryable.

Evidence (Before & After)

Before: #11584 defaults to the VS Code catalog and requires switching to Terminal / Web to access other conversations.

After: a local extension development host displayed the real workspace history in one list, with search, date grouping, and Load more, and no source selector. Automated window-control errors prevented completing a real-host click-through/reload verification; that part is not claimed as tested. Focused component/host tests cover unified listing, session selection, creation/restore attribution, persisted session restoration, and pagination retries.

Tested on

OS Status
macOS ✅ focused tests, Companion typecheck/lint/build, real-host history display
Windows ⚠️ not tested locally
Linux ⚠️ not tested locally

Environment (optional)

macOS arm64, Node.js 22, jsdom component tests, VS Code extension development host.

Risk & Scope

  • Main risk or tradeoff: ordinary cross-source conversations now expose the same editing actions. Delete reaches past this client: it closes the session and runs the daemon's ownership-verified worktree cleanup, which preserves a checkout with uncommitted work and an unmerged branch, and removes a clean checkout whose branch is already merged. Known child/background sources are filtered, but unattributed Live task threads cannot be reliably identified by the client; fix(live): attribute project task threads to qwen-live #11707 remains a separate producer-side issue.
  • Not validated / out of scope: real-host conversation click-through and reload; CLI and browser WebShell behavior; daemon/core pagination changes. This uses the existing organized catalog rather than the mtime-only path tracked by fix(core): make persisted session pagination safe for equal mtimes #11706.
  • Breaking changes / migration notes: no transcript migration. The persisted session-id string is retained; old source sidecars are ignored and legacy conversation storage is untouched.
  • Design: English · 中文. Both documents are updated together in this PR.

Linked Issues

Refs #11574. Corrects the VS Code interaction introduced by #11584. Does not close the browser-side discussion or #11707.

中文说明

这个 PR 做了什么

#11584 引入的 VS Code / Terminal 来源选择器替换为当前工作区普通会话的统一历史列表。终端、浏览器、VS Code 和旧会话共享同一个入口和行操作。打开已有会话时保留原来源,仅新建的 Companion 会话标记为 VS Code 来源。

列表复用现有的 organized 工作区目录,为稀疏页面保留明确的“加载更多”按钮,请求失败时保留游标。来源旁路状态和旧会话白名单扫描不再需要。

为什么需要

已经确认的产品要求是:在 VS Code 中直接打开终端创建的会话,无须选择来源。#11584 的来源选择器没有满足这一要求。对于同一工作区的普通会话,创建来源不是用户界面的隔离边界;工作区范围和后台任务的生命周期保护是独立问题。

Reviewer 测试计划

如何验证

  • 在包含终端、VS Code 和旧会话的工作区打开历史列表。它们应统一展示,且没有来源选择器。
  • 打开终端会话,确认历史内容,然后重载扩展宿主。同一会话应恢复,来源元数据不应被改写。
  • 在 VS Code 新建会话。它应出现在同一列表中,并保留 VS Code 创建来源。
  • 确认普通会话提供重命名和删除,当前会话仍有删除保护。整页均为被过滤的子会话或后台记录时,“加载更多”仍可操作;请求失败后可以重试。

前后对比证据

修改前:#11584 默认展示 VS Code 目录,必须切换到 Terminal / Web 才能访问其他会话。

修改后:本地扩展开发宿主已经展示真实工作区的统一历史,包含搜索、日期分组和“加载更多”,没有来源选择器。自动窗口控制报错阻止了真实宿主中点击会话及重载验证,因此不声称这部分已经实测。定向组件和宿主测试覆盖统一列表、会话选择、新建与恢复归因、持久化会话恢复及分页重试。

测试平台

OS 状态
macOS ✅ 定向测试、Companion 类型检查/lint/构建、真实宿主历史展示
Windows ⚠️ 未在本地测试
Linux ⚠️ 未在本地测试

环境(可选)

macOS arm64、Node.js 22、jsdom 组件测试、VS Code 扩展开发宿主。

风险与范围

  • 主要风险或取舍:普通跨来源会话提供相同的编辑操作。删除的影响会超出本客户端:它会关闭会话并执行 daemon 经过归属校验的 worktree 清理,有未提交改动的 checkout 和未合并的分支会被保留,干净且分支已合并的 checkout 会被移除。已知子会话和后台来源被过滤,但客户端无法可靠识别未标记来源的 Live 任务线程;fix(live): attribute project task threads to qwen-live #11707 仍是独立的生产端问题。
  • 未验证 / 超出范围:真实宿主中点击会话及重载;CLI 和浏览器 WebShell 的行为;daemon/core 分页修改。本次使用已有 organized 目录,而不是 fix(core): make persisted session pagination safe for equal mtimes #11706 跟踪的纯 mtime 分页路径。
  • 破坏性变更 / 迁移说明:不迁移会话文件。保留已持久化的字符串会话 ID,忽略旧来源旁路状态,旧会话存储保持不变。
  • 设计:English · 中文。本 PR 同步更新两份文档。

关联 Issue

关联 #11574,修正 #11584 引入的 VS Code 交互。不关闭浏览器侧讨论或 #11707

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

⚠️ Qwen Triage ended earlyview run. It stopped before finishing; check the run log.

⚠️ Qwen Triage 提前结束 —— 查看运行。未跑完,请查看运行日志。

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Verification at 71f45d5: the three focused Companion test files passed (115 tests). Companion TypeScript, changed-file ESLint, and the esbuild extension bundle also passed locally. A real macOS VS Code development host displayed the existing workspace history without the source selector. Window-control errors prevented completing the actual conversation click-through and extension reload, so those remain unverified; the automated component/host tests are not a substitute for that E2E check. Windows and Linux were not tested locally.

Close stale history on host bootstrap and exclude marked Live coordinator sessions from ordinary history. Remove outdated source-isolation documentation.

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

Copy link
Copy Markdown
Collaborator Author

Review follow-up pushed in 25edeb3: host bootstrap now closes stale history and clears its error, and explicitly marked Live coordinator sessions are excluded from ordinary conversation history. Removed the outdated comment describing source-isolated history. Ordinary CLI, browser, and VS Code conversations still share one list with no source selector.

Validation: all 116 focused tests passed, plus Companion TypeScript, changed-file ESLint, extension bundling, and diff whitespace checks. No further blocking finding was identified in the scoped review and simplicity pass.

Remaining verification limits are unchanged: real-host click-through restoration followed by reload has not been completed, and untagged Live task sessions remain tracked separately in #11707.

@yiliang114

Copy link
Copy Markdown
Collaborator Author

Test report: unified session history

Real VS Code visual check

Captured from the macOS VS Code Extension Development Host running the unified-history development extension against the qwen-code workspace, not a mockup or standalone browser preview.

Observed: one populated history list, a search field, date groups, and a Load more button. There is no Terminal / VS Code source selector. The terminal branch shown on the right is the opened workspace checkout, not the development extension branch.

Unified history in the real VS Code Extension Development Host

Automated verification

Previously run on commit 25edeb3: 116 focused tests passed across the history dropdown, embedded app, and extension provider; Companion TypeScript, changed-file ESLint, extension bundling, and diff whitespace checks also passed. Mixed CLI / browser / VS Code history, source-preserving selection, pagination, and the reconnect regression are covered by focused tests.

Limits

This screenshot verifies the visible list layout, not each displayed session's creator metadata, successful Load more interaction, or end-to-end restoration. The already-running development host was not reloaded for this capture, so the screenshot is not evidence for the latest reconnect and Live-filter fixes. Restoring a CLI conversation and reloading the real VS Code host remains unverified. Untagged Live task sessions remain tracked in #11707; child and explicitly marked system sessions are intentionally still filtered.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓ — every required heading is present, including the Tested-on table, the Risk & Scope bullets, and the Chinese translation.

Problem: observed, not theoretical. #11574 is open and was filed by a different user (@Luolingli) with reproduction steps, a per-transcript evidence table, and a live check against the extension's own daemon: of 8 persisted sessions in one workspace, sourceType=vscode returned 1 while sourceType=default returned the 7 legacy ones. The transcripts were intact on disk — a visibility bug, not data loss. This clears the reproduction bar comfortably.

Direction: aligned. #11574's own "Expected behavior" asks for two things — list all active workspace sessions including legacy ones without source metadata, and if source scoping is wanted, make it an explicit UI option rather than a hardcoded default. #11584 shipped the second clause; this PR ships the first. Worth saying plainly, since it's the one thing a reviewer should consciously sign off on: this reverses #11584, which merged at 05:01 UTC today — about seven hours before this branch landed. A same-day course correction is the right call when it deletes the attempt that missed the requirement rather than layering on top of it, and that's what happens here. CHANGELOG: no direct reference to a creation-source selector in the VS Code session list; the area is relevant, since that list is workspace-scoped with no source tabs.

Size: core paths are not touched, so the two-tier core gate is not applicable. All eleven files sit under packages/vscode-ide-companion/src/webview/** (note src/webview/providers/ is not the packages/*/src/providers/** core path), plus two docs/design/ files and one .qwen/e2e-tests/ plan. Breakdown: 371 production lines (EmbeddedApp.tsx 203, WebViewProvider.ts 88, SessionHistoryDropdown.tsx 64, strings.ts 10, sessionSource.ts 6), 708 test lines, 59 docs lines — net −618. Under both the 500 and 1000 thresholds, and tests outweigh production code roughly two to one.

Approach: this is what I'd have proposed independently, and in one respect it's better than my version. I would have just dropped the sourceType parameter; this instead reuses the view: 'organized' + group: 'all' catalog that web-shell already uses throughout, which brings opaque, tie-safe pagination for free instead of the mtime-only path #11706 is tracking. It also deletes a whole subsystem rather than keeping it as a fallback — the legacy allowlist scan with its ten-page / thousand-read worst case, the per-session source sidecars in workspaceState, and the request-generation races that only existed because two catalogs could be in flight. The two additions beyond "remove the switch" — the explicit Load more control and the truncated-catalog notice — are forced consequences rather than scope creep: once child and background rows are filtered client-side, a page can come back visually empty while more pages exist, and without an explicit control the user is stranded. No drive-by refactors or formatting churn.

The second commit (25edeb3f, "harden unified history after review") is tightly scoped to three files and reads as genuine follow-up rather than scope creep: it drops a now-inverted rationale comment, closes the stale dropdown on host re-bootstrap, and filters Live voice threads out of the ordinary list. I've reviewed that delta against its producer and it holds up — details in the code review.

One thing I'd still like confirmed as deliberate rather than overlooked: with no source boundary left, a machine-generated session carrying no attribution at all cannot be told apart on the client and will render as an ordinary, renameable, deletable row. The delta closes the Live voice case specifically, and the design doc scopes the general producer-side fix to #11707 — so this is a known, tracked remainder rather than an unnoticed hole.

Risk: no elevated risk signals — none of the changed files match the high-risk path set.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓ —— 所有必需标题都在,包括测试平台表格、风险与范围条目和中文翻译。

问题: 是已观测到的 bug,不是理论性加固。#11574 仍处于 open 状态,由另一位用户 @Luolingli 提出,附有复现步骤、逐条 transcript 证据表,以及对扩展自身 daemon 的实时验证:同一工作区 8 个持久化会话中,sourceType=vscode 只返回 1 个,而 sourceType=default 返回 7 个旧会话。磁盘上的 transcript 完好无损 —— 这是可见性 bug,不是数据丢失。复现门槛完全满足。

方向: 对齐。#11574 的"期望行为"提了两点 —— 列出工作区所有活跃会话(包括没有来源元数据的旧会话),以及如果确实需要按来源划分,应当作为显式 UI 选项而非硬编码默认值。#11584 实现了第二点,本 PR 实现第一点。有一点需要 reviewer 明确认可,所以直说:本 PR 推翻了 #11584,而后者今天 05:01 UTC 才合并 —— 距本分支落地约七小时。当同日纠偏是删掉没满足需求的那次尝试、而不是在其之上继续叠加时,这样做是对的,本 PR 正是如此。CHANGELOG:VS Code 会话列表中没有与"创建来源选择器"直接对应的条目;但该领域是相关的 —— 那边的列表按工作区聚合,没有来源标签页。

规模: 未触及核心路径,两层核心门禁不适用。全部十一个文件都在 packages/vscode-ide-companion/src/webview/** 下(注意 src/webview/providers/ 不属于 packages/*/src/providers/** 核心路径),另加两个 docs/design/ 文件和一个 .qwen/e2e-tests/ 计划。拆分:生产代码 371 行(EmbeddedApp.tsx 203、WebViewProvider.ts 88、SessionHistoryDropdown.tsx 64、strings.ts 10、sessionSource.ts 6)、测试 708 行文档 59 行 —— 净减少 618 行。低于 500 与 1000 两个阈值,且测试量约为生产代码的两倍。

方案: 与我独立提出的方案一致,某一点上还更好。我原本只会去掉 sourceType 参数;本 PR 改为复用 web-shell 已广泛使用的 view: 'organized' + group: 'all' 目录,从而免费获得不透明、同时间戳安全的分页,而不是 #11706 跟踪的纯 mtime 路径。它还删除了整个子系统而非留作兜底 —— 带十页/千次读取最坏情况的旧会话白名单扫描、workspaceState 中按会话保存的来源旁路状态,以及仅因两个目录可能同时在途而存在的请求代次竞争。除"移除切换"之外的两处新增 —— 显式的"加载更多"和目录截断提示 —— 是必然结果而非范围蔓延:一旦子会话和后台记录在客户端被过滤,某一页可能视觉上为空但后面仍有数据,没有显式控件用户就会卡住。没有顺手重构或格式抖动。

第二个提交(25edeb3f,"harden unified history after review")严格限定在三个文件内,读起来是真正的后续收敛而非范围蔓延:删掉一段已经反转的理由注释、在宿主重新 bootstrap 时关闭过期弹窗、把 Live 语音线程从普通列表中过滤掉。我已把这部分增量对照其生产者审过,结论成立 —— 详见代码审查。

仍有一点想确认是有意为之而非疏漏:来源边界取消后,一个完全没有任何归属标记的机器生成会话在客户端无法区分,会作为普通行渲染,并可被重命名、删除。本次增量专门处理了 Live 语音这一种情况,设计文档把通用的生产端修复归到 #11707 —— 所以这是已知的、有跟踪的剩余项,而不是没被注意到的漏洞。

风险: 无升级风险信号 —— 变更文件均未命中高风险路径集合。

进入代码审查 🔍

Qwen Code · qwen3.8-max-2026-09-02

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

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Code review

I wrote an independent proposal from the title and motivation first (drop the source filter, list the workspace in one page), then compared. The PR matches it and is cleaner in one place — it adopts the organized catalog rather than merely omitting sourceType. Everything load-bearing below I verified against the tree at 25edeb3f, not inferred from the diff.

The actual fix is correct, and the reason is server-side. sourceType is an optional parameter in the daemon's session/list handler — parseSessionSource(params['sourceType'], params['sourceId']) yields no source filter when it's absent. Omitting it is precisely what makes pre-0.23.x transcripts visible again, and DaemonSessionSummary.sourceType is documented as "immutable creator attribution, absent on legacy/unattributed sessions", which is why session.sourceType ?? '' keeps those rows instead of dropping them. #11574 is fixed at the right layer.

The client-side child filter is forced, not a shortcut. The daemon explicitly rejects parentSessionId together with view: 'organized' (AcpParamError), so filtering !session.parentSessionId on the client is the only available option. group also requires view to be 'organized', which this satisfies. DaemonSessionListPage.truncated?: boolean exists, so the incomplete-history notice is typed against a real field.

Every blocked source value has a real producer — I did not take the list on faith. scheduled_task (serve/routes/scheduled-tasks.ts:1176, serve/scheduled-task-keepalive.ts:165), side_task (acp-bridge/src/bridge.ts:11585), channel (cli/src/commands/channel/daemon-worker.ts:266), qwen-live (qwen-live/src/adaptor/qwen-code-adaptor.ts:407). standalone is correctly absent: standalone sessions are created through their own route without a workspaceCwd, so they never enter a workspace-scoped listing.

The second commit's new clause is live code, and it's correctly conjunctive. The added filter drops rows where sourceType === 'default' and sourceId starts with realtime_voice:. That matches its producer exactly — serve/live/live-session-coordinator.ts:1442-1443 spawns Live conversation threads with sourceType: 'default' and sourceId: \${LIVE_SESSION_SOURCE_PREFIX}${context.callId}`, where the prefix is 'realtime_voice:'. The new test fixture (realtime_voice:call-1) mirrors the producer's own fixture. The conjunction matters and is right: a genuine terminal or browser session also carries sourceType: 'default'but has no suchsourceId, so it stays; and a legacy session has sourceType === undefined, so the first conjunct is false and it stays too. Had this been written as a bare sourceType === 'default'` check it would have re-broken #11574 by hiding every terminal and legacy conversation. It isn't.

The one line I probed hardest was sessionSourceType={runtime.sessionId ? undefined : VSCODE_SESSION_SOURCE_TYPE}. It looks risky because web-shell defaults that prop to 'default' (WEB_SHELL_SESSION_SOURCE_TYPE), so undefined downstream does not mean "no attribution" — it means "default". It's safe because the prop has exactly one consumer, createAndAttachSessionForPromptcreateSession({ sourceType }), and is never applied when restoring an existing session, so opening a terminal or legacy conversation cannot rewrite its attribution. New conversations still get 'vscode' because web-shell calls onSessionIdChange(undefined) on its new-session flows (App.tsx:12969, 13162, both with createNewSession in the dependency array), clearing runtime.sessionId before the create. The derived expression correctly subsumes the imperative setActiveSessionHistorySource(VSCODE_SESSION_SOURCE_TYPE) reset this PR deletes — same invariant, less state. The invariant spans three files and the line that used to state it imperatively is gone, so I'd like to see it named in a comment; not blocking.

Bootstrap handling is right, including the bit that looks like an oversight. On re-bootstrap the handler bumps the request generation, clears rows/cursor/loading/error and closes the dropdown. It calls setSessionHistoryOpen(false) directly rather than closeSessionHistory(), which also focuses the history button — deliberately skipping that focus steal is correct when the host re-initialized underneath the user rather than the user closing the panel. The new closes stale history when the host bootstraps again test pins the observable half against the real #qwen-session-history id.

Delete protection survives the removal of editable. The row-actions wrapper is now unconditional, but the delete control itself is still gated on {!active && ...}, so the currently open conversation remains undeletable and the confirm step is untouched.

The removal is complete — no dangling references. I fetched all five PR-head production files at 25edeb3f and grepped: zero hits for sessionHistorySource, legacyConversationIds, webShellSessionSourceStateKey, historySource, loadLegacyAllowlistedSessions, LEGACY_SESSION_SCAN_*, the three removed session.source* string keys, data-session-source, or editable. DaemonClient (still constructed at the new DaemonClient(...) site) and the initialRuntime local (still feeding useState) both retain uses, so noUnusedLocals won't trip. All three string-key removals are mirrored in EN and ZH, keeping ChromeStringKey consistent.

Tests pin the new behaviour rather than just being deleted. The unified-list case asserts the exact query — toHaveBeenCalledExactlyOnceWith({ pageSize: 20, cursor: undefined, archiveState: 'active', view: 'organized', group: 'all' }), with no sourceType — that [data-session-source] is gone, that all four ordinary rows render with two action buttons each, and that child-1, scheduled-1 and now live-1 do not. Its fixture deliberately includes an unattributed legacy-1, which is the row #11574 was about. The pagination case is the strongest: a page containing only a filtered child still offers Load more, a rejected request surfaces the error while keeping visible rows, and the retry is asserted to re-issue cursor: 'opaque-page-2' — proving cursor retention rather than assuming it — after which truncated: true shows the incomplete notice and Load more disappears once no cursor remains. The deleted stale-source-race test is legitimately obsolete: with one catalog there is no previous source to race.

Both design docs were updated in the same change with matching structure (Context/Decision/Scope/Verification ↔ 背景/决策/范围/验证) and matching decisions, so the bilingual documentation requirement is met.

Non-blocking

  • The 'realtime_voice:' literal duplicates LIVE_SESSION_SOURCE_PREFIX in packages/cli/src/runtime/live-session-source.ts. Importing it isn't actually available here — packages/cli is not a dependency of vscode-ide-companion, and adding one would drag the CLI runtime into an esbuild browser bundle, so hardcoding is the right call. What's missing is a one-line comment naming that producer: the coupling is invisible today, and if the prefix ever changes this filter degrades silently, letting Live voice threads reappear as ordinary editable rows with no producer-side test failing. That's exactly the case where a comment earns its place.
  • The doc filenames still read vscode-session-source-switch while both titles now say "Unified VS Code session history". Keeping the paths is the right trade — renaming loses git blame and would break the reciprocal EN↔zh links. Noting it only so the mismatch isn't mistaken for an oversight.

CI test evidence

CI for this commit is still running — nothing has failed. States below are from the authoritative actions/runs/34693964942/jobs, fetched once at 12:47 UTC; I did not poll.

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

Check Conclusion
Classify PR ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
Lint & Static (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,失败项排在最前。

web-shell E2E Smoke (ubuntu) has not appeared in the job list yet — in the prior run it started only after earlier jobs, so it is presumably still gated rather than missing.

Worth recording, because it briefly looked like a red flag: the CI run for the previous commit (71f45d53) completed as cancelled at 12:33:06, with Test, Lint & Static and web-shell E2E Smoke terminated mid-suite — its log ends with the runner reaping live vitest and esbuild processes. That was not a failure and not infrastructure flakiness either: 25edeb3f was pushed at 12:32:17, and the CI workflow's concurrency group cancels in-progress PR runs on a new push. The superseded run is expected behaviour, and the run above is its replacement. A second trap: the commits/{sha}/check-runs endpoint and statusCheckRollup both kept reporting those three jobs as in_progress after the run had reached a terminal state, so the run/jobs API is what I trusted.

Not verified: Test (ubuntu) and Lint & Static (ubuntu) are still in flight, so no unit-suite or lint result exists for 25edeb3f yet; web-shell E2E Smoke has not started. The author's comment on this PR reports 115 focused Companion tests, Companion typecheck, changed-file ESLint and the esbuild bundle passing locally, plus a real macOS dev host displaying the unified history. That is the author's claim, not evidence I re-ran — and I don't execute PR code on the CI path. It also predates the 25edeb3f delta.

Sandboxed verification would settle the remaining gap: @qwen-code /verify — the component suite mocks listWorkspaceSessionsPage, so it proves the client sends the right query and filters correctly, but it cannot prove that a real daemon over real mixed transcripts (pre-0.23.x unattributed + terminal + vscode + a Live voice thread) actually returns the legacy rows through the organized view while withholding the Live one, nor that opening a legacy conversation leaves its persisted sourceType untouched on disk. An A/B against the base build would settle both, and the reload/restore half is precisely what the author already flagged as untested. /tmux is the wrong lane here — it drives the terminal TUI, and this surface is a VS Code webview. The author has admin, so no sponsored run is needed.

中文说明

代码审查

我先根据标题与动机独立写出方案(去掉来源过滤,单页列出工作区会话),再做对比。本 PR 与之一致,且有一处更干净 —— 它采用了 organized 目录,而不只是省略 sourceType。下面每条关键结论都是对着 25edeb3f 的代码树核实的,不是从 diff 推断的。

真正的修复是对的,原因在服务端。 sourceType 在 daemon 的 session/list 处理中是可选参数 —— 缺省时 parseSessionSource(params['sourceType'], params['sourceId']) 不产生来源过滤。省略它正是让 0.23.x 之前的 transcript 重新可见的原因;而 DaemonSessionSummary.sourceType 的文档写明是"不可变的创建者归属,在旧的/无归属会话上缺失",这正是 session.sourceType ?? '' 能保留这些行而非丢掉它们的原因。#11574 是在正确的层次上被修复的。

客户端过滤子会话是被迫的,不是走捷径。 daemon 明确拒绝 parentSessionIdview: 'organized' 同时出现(AcpParamError),所以在客户端过滤 !session.parentSessionId 是唯一可行选择。group 也要求 view'organized',本 PR 满足。DaemonSessionListPage.truncated?: boolean 确实存在,因此"历史不完整"提示是对着真实字段写的类型。

每个被屏蔽的来源值都有真实生产者 —— 我没有直接采信这个列表:scheduled_taskserve/routes/scheduled-tasks.ts:1176serve/scheduled-task-keepalive.ts:165)、side_taskacp-bridge/src/bridge.ts:11585)、channelcli/src/commands/channel/daemon-worker.ts:266)、qwen-liveqwen-live/src/adaptor/qwen-code-adaptor.ts:407)。standalone 正确地没有出现:standalone 会话经由独立路由创建且不携带 workspaceCwd,本就不会进入按工作区聚合的列表。

第二个提交新增的过滤条件是有效代码,而且合取写法是对的。 新增条件排除 sourceType === 'default' sourceIdrealtime_voice: 开头的行。这与其生产者完全对应 —— serve/live/live-session-coordinator.ts:1442-1443 创建 Live 会话线程时正是 sourceType: 'default'sourceId: \${LIVE_SESSION_SOURCE_PREFIX}${context.callId}`,而该前缀就是 'realtime_voice:'。新测试的 fixture(realtime_voice:call-1)与生产者自己的 fixture 一致。这个"且"很关键且写对了:真正的终端或浏览器会话同样是 sourceType: 'default',但没有这样的 sourceId,因此保留;旧会话的 sourceType === undefined,第一个条件即为假,因此也保留。若写成裸的 sourceType === 'default'` 判断,就会把每个终端与旧会话都藏起来,等于重新弄坏 #11574。它没有这样写。

我推敲最久的一行是 sessionSourceType={runtime.sessionId ? undefined : VSCODE_SESSION_SOURCE_TYPE} 它看起来有风险,因为 web-shell 会把该 prop 默认为 'default'WEB_SHELL_SESSION_SOURCE_TYPE),所以在下游 undefined 并不表示"不带归属",而是表示"default"。它之所以安全,是因为该 prop 只有一个消费者 createAndAttachSessionForPromptcreateSession({ sourceType }),且恢复已有会话时从不应用它,因此从 VS Code 打开终端或旧会话不会改写其归属。新会话仍会拿到 'vscode',因为 web-shell 在新建流程中会调用 onSessionIdChange(undefined)App.tsx:1296913162,两处依赖数组都含 createNewSession),在创建之前清空 runtime.sessionId。这个派生表达式正确取代了本 PR 删掉的命令式 setActiveSessionHistorySource(VSCODE_SESSION_SOURCE_TYPE) 重置 —— 同样的不变量,更少的状态。该不变量跨三个文件才成立,而原先明写它的那行已删除,所以我希望加一条注释点明;不阻塞。

bootstrap 处理是对的,包括那处看起来像疏漏的地方。 重新 bootstrap 时,处理器递增请求代次、清空列表/游标/加载态/错误态并关闭弹窗。它直接调用 setSessionHistoryOpen(false),而不是 closeSessionHistory()(后者还会把焦点移回历史按钮)—— 在宿主于用户脚下重新初始化、而非用户主动关闭面板的场景下,刻意跳过这次抢焦点是正确的。新增的 closes stale history when the host bootstraps again 测试对着真实的 #qwen-session-history id 钉住了可观测的那一半。

移除 editable 后,删除保护依然存在。 行操作外层现在无条件渲染,但删除控件本身仍由 {!active && ...} 把关,因此当前打开的会话依旧不可删除,二次确认步骤未被改动。

移除是彻底的 —— 没有悬挂引用。 我拉取了 25edeb3f 上全部五个生产文件并 grep:sessionHistorySourcelegacyConversationIdswebShellSessionSourceStateKeyhistorySourceloadLegacyAllowlistedSessionsLEGACY_SESSION_SCAN_*、被删除的三个 session.source* 文案键、data-session-sourceeditable 全部零命中。DaemonClient(仍在 new DaemonClient(...) 处构造)与 initialRuntime 局部变量(仍供 useState 使用)都保留了用途,因此 noUnusedLocals 不会报错。三个文案键在 ENZH 中同步删除,ChromeStringKey 保持一致。

测试是钉住新行为,而不是单纯删掉。 统一列表用例断言了精确查询 —— toHaveBeenCalledExactlyOnceWith({ pageSize: 20, cursor: undefined, archiveState: 'active', view: 'organized', group: 'all' }),不含 sourceType —— 断言 [data-session-source] 已消失、四个普通行各渲染两个操作按钮、child-1scheduled-1 以及现在的 live-1 都不出现。其 fixture 刻意包含无归属的 legacy-1,而那正是 #11574 关心的行。分页用例最有说服力:整页只含被过滤子会话时仍提供"加载更多";请求被拒时展示错误并保留已可见的行;随后断言重试重新发出 cursor: 'opaque-page-2' —— 这是证明游标被保留而非假设;之后 truncated: true 显示不完整提示,游标耗尽后"加载更多"消失。被删掉的"来源切换竞态"测试是合理过时项:只有一个目录时不存在上一个来源来竞争。

两份设计文档在同一次变更中一起更新,结构对应(Context/Decision/Scope/Verification ↔ 背景/决策/范围/验证),决策一致,满足双语文档要求。

非阻塞项

  • 'realtime_voice:' 字面量重复了 packages/cli/src/runtime/live-session-source.ts 中的 LIVE_SESSION_SOURCE_PREFIX。这里确实无法导入 —— packages/cli 不是 vscode-ide-companion 的依赖,而加上它会把 CLI 运行时拖进 esbuild 的浏览器 bundle,所以硬编码是对的选择。缺的是一行点明该生产者的注释:今天这层耦合是隐形的,一旦前缀变更,此过滤会静默失效,让 Live 语音线程重新变成普通可编辑行,而生产端不会有任何测试失败。这正是注释值得存在的情形。
  • 文档文件名仍为 vscode-session-source-switch,而两个标题已改为"Unified VS Code session history"/"VS Code 统一会话历史"。保留原路径是正确取舍 —— 重命名会丢失 git blame 并破坏中英互链。仅为说明,避免被误认为疏漏。

CI 测试证据

本提交的 CI 仍在运行 —— 没有任何失败。下方状态来自权威的 actions/runs/34693964942/jobs,于 12:47 UTC 一次性获取;我没有轮询。

(上方表格为各项检查的真实名称与结论。)

web-shell E2E Smoke (ubuntu) 尚未出现在任务列表中 —— 在上一次 run 里它是在前置任务之后才启动的,因此更可能仍是被门控,而非缺失。

有一点值得记录,因为它一度看起来像危险信号:上一个提交(71f45d53)的 CI run 在 12:33:06 以 cancelled 结束,TestLint & Staticweb-shell E2E Smoke 都在套件中途被终止 —— 其日志以 runner 回收仍在运行的 vitestesbuild 进程结束。这既不是失败,也不是基础设施抖动:25edeb3f 于 12:32:17 推送,而 CI 工作流的并发组会在新推送时取消进行中的 PR run。被取代的 run 属预期行为,上面这个 run 就是它的接替者。第二个坑:commits/{sha}/check-runs 接口与 statusCheckRollup 在该 run 已到达终态后仍把这三个任务报为 in_progress,所以我以 run/jobs 接口为准。

未验证: Test (ubuntu)Lint & Static (ubuntu) 仍在进行中,因此 25edeb3f 上尚无单元套件与 lint 结果;web-shell E2E Smoke 尚未开始。作者在本 PR 的评论中称本地有 115 个定向 Companion 测试、Companion 类型检查、变更文件 ESLint 与 esbuild 打包通过,并在真实 macOS 开发宿主上看到了统一历史列表。那是作者的说法,不是我重跑得到的证据 —— 在 CI 路径上我不执行 PR 代码。而且它早于 25edeb3f 这次增量。

沙箱验证可以补上剩余缺口:@qwen-code /verify —— 组件测试对 listWorkspaceSessionsPage 做了 mock,因此只能证明客户端发出正确查询并正确过滤,无法证明真实 daemon 面对真实混合 transcript(0.23.x 之前无归属 + 终端 + vscode + 一个 Live 语音线程)时,确实会通过 organized 视图返回旧会话行、同时不返回 Live 那一条,也无法证明打开旧会话不会改动磁盘上已持久化的 sourceType。与 base 构建做 A/B 可同时定这两点,而其中的重载/恢复部分正是作者已标注为未实测的那一半。这里 /tmux 不是合适通道 —— 它驱动终端 TUI,而本次变更的面是 VS Code webview。作者具备 admin 权限,因此无需 sponsored run。

Qwen Code · qwen3.8-max-2026-09-02

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — solid, and I'd merge it. The two things keeping it off 5 are both non-blocking: the realtime_voice: literal carries an invisible cross-package coupling that deserves a one-line comment, and the real-host reload/restore half of the test plan is still unverified by anyone.

Stepping back: my independent proposal was essentially what this PR does — drop the source filter and list the workspace's conversations together. The PR goes one better than I'd have planned. I would have deleted the sourceType parameter and kept everything else; this instead moves onto the organized catalog that web-shell already uses everywhere, which brings tie-safe opaque pagination for free, and then deletes the legacy allowlist scan outright rather than keeping it as a downgrade fallback. Net −618 lines, one fewer subsystem, and tests outweighing production code two to one. If I picked this up in six months I'd thank the author, not curse them — there is now one catalog, one cursor, one filter, and no per-session sidecar state in workspaceState left to drift out of sync.

Three things I deliberately tried to break, because they're where a change like this usually goes wrong.

The first is sessionSourceType={runtime.sessionId ? undefined : VSCODE_SESSION_SOURCE_TYPE}. Passing undefined is not the same as passing nothing — web-shell defaults that prop to 'default', so if it were ever applied on the restore path, opening a terminal or legacy conversation from VS Code would stamp it 'default' and quietly destroy the very attribution this PR exists to protect. It isn't applied on restore: the prop has exactly one consumer, createAndAttachSessionForPrompt. And new conversations still get 'vscode', because web-shell clears sessionId to undefined on its new-session flows before creating. The invariant holds, but it holds across three files, and the line that used to state it imperatively is now gone — that's the one place I'd want a comment or an end-to-end create-after-restore assertion.

The second is the follow-up commit's Live voice filter, which is the kind of clause that is very easy to get subtly wrong. It's conjunctive — sourceType === 'default' and a realtime_voice: sourceId — and that conjunction is what keeps it from re-breaking #11574: terminal and browser sessions are also 'default' but carry no such sourceId, and legacy sessions have sourceType === undefined. Written one conjunct wider, this commit would have hidden exactly the conversations the PR exists to surface. I checked it against its producer (live-session-coordinator.ts:1442) rather than against the test fixture alone, and it matches.

The third is whether dropping the source boundary hands VS Code users a destructive capability they didn't have. It doesn't grant anything new: the browser Web Shell sidebar already lists the same workspace catalog with full rename and delete, so this removes an inconsistency where the identical session was editable in one client and read-only in another. Delete still requires a confirm step, and the current conversation is still protected by the !active gate that survives the removal of editable. The remainder is the honest one the design doc names — a machine-generated session with no attribution at all still can't be distinguished client-side — and that stays with #11707 as a producer-side fix, which is the right place for it.

On the CI cancellation I chased earlier: it was this PR's own follow-up push superseding the previous run through the workflow's concurrency group, not a failure and not infrastructure flakiness. Worth having checked rather than assumed, since "cancelled" and "red" invite very different responses.

Verdict: approve, deferred until CI lands green. Test (ubuntu) and Lint & Static (ubuntu) are still in flight on 25edeb3f and web-shell E2E Smoke has not started, so there is no unit-suite or lint result to attest to yet — and main requires two approving reviews, which makes a bot approval here load-bearing rather than decorative. Nothing in the diff is blocking, so approval is deferred until CI lands green on 25edeb3f9fe01ee297052df6f1dba1bfacba254d; the marker below carries that, and the finalize job will withhold the approval if anything lands red or the head moves again. If another commit is pushed before CI settles, this pass is stale and triage should be re-run.

The one thing I'd genuinely ask for before or shortly after merge is the @qwen-code /verify run named in the code review — the component suite mocks the catalog client, so the daemon-side half of this fix (real mixed transcripts, legacy rows returned, attribution untouched on disk after a reload) rests on static reading plus the author's local report rather than on an executed A/B.

中文说明

信心:4/5 —— 扎实,我会合并它。没给到 5 分的两点都非阻塞:realtime_voice: 字面量带着一条隐形的跨包耦合,值得一行注释;以及测试计划中真实宿主的重载/恢复那一半,至今没有任何人验证过。

退一步看整体:我独立提出的方案基本就是本 PR 所做的 —— 去掉来源过滤,把工作区会话放在一起列出。本 PR 比我的设想更好一步。我原本只会删掉 sourceType 参数、其余保持不变;本 PR 改为切换到 web-shell 已普遍使用的 organized 目录,从而免费获得同时间戳安全的不透明分页,然后彻底删除旧会话白名单扫描,而不是把它留作降级兜底。净减少 618 行,少了一个子系统,且测试量约为生产代码的两倍。如果半年后由我接手,我会感谢作者而不是抱怨 —— 现在只有一个目录、一个游标、一套过滤,workspaceState 中也不再有按会话保存、可能与实际脱节的旁路状态。

有三处我刻意去尝试打破,因为这类改动通常就在那里出问题。

第一处是 sessionSourceType={runtime.sessionId ? undefined : VSCODE_SESSION_SOURCE_TYPE}。传 undefined 不等于什么都不传 —— web-shell 会把该 prop 默认为 'default',所以一旦它被用在恢复路径上,从 VS Code 打开终端或旧会话就会被打上 'default',悄悄毁掉本 PR 存在的意义所要保护的归属信息。它并没有被用在恢复路径:该 prop 只有一个消费者 createAndAttachSessionForPrompt。而新会话仍会拿到 'vscode',因为 web-shell 在新建流程中会先把 sessionId 清为 undefined 再创建。不变量成立,但它跨三个文件才成立,而原先用命令式代码明写它的那行已消失 —— 这是我唯一希望补一条注释、或补一个端到端"恢复后再新建"断言的地方。

第二处是后续提交里的 Live 语音过滤,这类条件极易写得微妙出错。它是合取的 —— sourceType === 'default' sourceId 带 realtime_voice: 前缀 —— 而这个"且"正是它不会重新弄坏 #11574 的原因:终端与浏览器会话同样是 'default' 但没有这样的 sourceId,旧会话的 sourceType === undefined。只要多写宽一个条件,这个提交就会把本 PR 意在显示的那些会话恰好藏起来。我是对着其生产者(live-session-coordinator.ts:1442)核实的,而不只是对着测试 fixture,结论一致。

第三处是去掉来源边界是否让 VS Code 用户获得了原本没有的破坏性能力。并没有:浏览器 Web Shell 侧栏本来就列出同一个工作区目录并提供完整的重命名与删除,所以本次改动消除的是一种不一致 —— 同一个会话在一个客户端可编辑、在另一个只读。删除仍需二次确认,当前会话仍受 !active 保护(该保护在移除 editable 后依然保留)。剩下的是设计文档诚实指出的那一点 —— 完全没有任何归属标记的机器生成会话在客户端仍无法区分 —— 它继续留在 #11707 作为生产端修复,那是它该在的地方。

关于我先前追查的 CI 取消:那是本 PR 自己的后续推送经工作流并发组取代了上一个 run,既不是失败,也不是基础设施抖动。这一点值得去查证而不是假设,因为"被取消"和"变红"应当引出完全不同的反应。

结论:批准,但延迟到 CI 变绿。 Test (ubuntu)Lint & Static (ubuntu)25edeb3f 上仍在进行,web-shell E2E Smoke 尚未开始,因此还没有可供背书的单元套件与 lint 结果 —— 而 main 要求两个批准评审,这使这里的机器人批准是有实质分量的,而非装饰性的。diff 中没有阻塞项,因此批准延迟至 CI 在 25edeb3f9fe01ee297052df6f1dba1bfacba254d 上变绿;下方标记承载了这一安排,若有任何检查变红或 head 再次移动,finalize 任务会撤回该批准。如果在 CI 落定前又推送了新提交,则本次审查已过期,应重新运行 triage。

在合并之前或之后不久,我真正希望补上的是代码审查中点名的那次 @qwen-code /verify —— 组件测试对目录客户端做了 mock,所以本修复的 daemon 侧那一半(真实混合 transcript、旧会话行被返回、重载后磁盘上的归属未被改动)目前依靠的是静态阅读加作者的本地报告,而不是一次真正执行过的 A/B。

Qwen Code · qwen3.8-max-2026-09-02

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

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

@yiliang114

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

qwen-code-ci-bot commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run

Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check.

Scripted assertions: 53 passed · 0 failed · 53 total

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

中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)

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

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

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

Verification report

PR #11713 deep verification — fix(vscode): unify session history and remove source switch

Verdict: findings — the central claim is proven load-bearing by A/B against the base build; 53/53 scripted assertions passed (fail: 0); no blocker found. Four non-blocking findings, two of which are coverage gaps in the PR's own new tests (a guard and a guard-pair that no test in the suite can see).

  • Verified head: 25edeb3f9fe01ee297052df6f1dba1bfacba254d (git rev-parse HEAD^2)
  • Base tip: 8a21551599b7dc15bcc2b8ed0ffb4e151cc75f31 (HEAD^1, matches the snapshot's baseRefOid)
  • Effective diff: git diff HEAD^1..HEAD — 11 files, +260 / −878, all under packages/vscode-ide-companion/src/webview/ plus two design docs and one e2e plan. packages/sdk-typescript, packages/web-shell, packages/core, packages/cli, and every package.json/lockfile are untouched.
  • Assertion totals: pass 53 / fail 0 / total 53 (counting rule at the end of Methodology).
中文摘要

结论:findings(有问题值得关注,但无阻塞项)。核心主张已通过 A/B 对照验证成立;53/53 条脚本化断言通过(fail: 0)。

A/B 结论:同一套 mock-free 测试装置(真实 EmbeddedApp + 真实 DaemonClient + 回环 HTTP 假 daemon,假 daemon 按 session-list.ts 的既有语义实现 organized 视图)分别跑在 head 构建与 base 构建上。45 项观测中 29 项在两侧不同。关键翻转:请求参数 sourceType=vscodeview=organized&group=all;来源选择器按钮 2 → 0;可见会话 vs-1,vs-currentcli-1,legacy-1,vs-1,vs-current(终端会话与旧版无归属会话合并进同一列表);恢复已有会话时的 sessionSourceTypevscodeundefined(即不再改写来源,这正是测试计划第 2 步的核心)。把 head 的断言原样跑在 base 构建上,8 条中 6 条变红,2 条保持绿(新建会话归属、失败重试保留游标——这两项本 PR 未改动),说明这些断言确实由本 PR 承载。详见「Central claim + A/B table」与 01-ab-base-vs-head-unified-history.png

Findings(均为 Suggestion / Nit,无 Critical)

  1. !hasMore 空状态守卫未被任何测试固定(变异 M10 存活,但验证装置的场景 B2 可观测)。
  2. 「bootstrap 清空 + 首页替换」是一对分层守卫(M6+M9):单独回滚任一个都被另一个掩盖,同时回滚才会让上一个工作区的会话残留到新工作区的列表中——PR 自带测试对该组合无覆盖。
  3. 未按文档约定预检 session_organization capability 就发送 view/group
  4. truncated 被映射为「部分会话未能加载。」,但服务端该标志只表示扫描触顶 5 万条;真正表示合并失败的 liveMergeFailed 被忽略。

另有一项已被证伪、不作为 finding:去掉 sourceType 并不会让 daemon 自有的 standalone 会话进入列表——服务端有两道默认拒绝(session.ts:1239-1245 仅在 sourceType=default 时才选中内部 Conversations runtime;workspace-registry.ts:374-401 对 internal entry 一律返回 undefined)。唯一真实暴露是普通存储中恰好带 standalone 字符串的历史遗留 transcript,而 daemon 明确设计为把它当普通会话恢复(session.ts:3847-3870)。同时更正:我最初的 web-1 观测来自自造 fixture,假 peer 未建模 workspace-registry 的 runtime 解析,因此它只能证明「客户端过滤器不排除 standalone 行」,不能证明真实 daemon 会返回这样的行(详见 Findings 第 5 条)。

未覆盖范围:真实 VS Code 宿主(容器无 GUI,作者亦未声称已测);真实 qwen serve daemon(假 peer 只复现 wire 形状与 organized 语义,不复现 workspace-registry 的 runtime 解析、真实目录扫描与 live merge);旧 daemon 下的降级行为(仅由路由参数处理推断,未实测);对跨来源会话执行重命名/删除的服务端效果;逐 commit 归因(快照列 2 个 commit,浅克隆只可达 1 个);仓库级全量门禁。

Central claim + A/B table

Central claim. The VS Code companion's history list shows every ordinary conversation in the current workspace as one list — terminal, browser, VS Code and pre-attribution legacy — with no source selector, and opening an existing conversation no longer rewrites its creator attribution.

Secondary claims. (a) Load more stays reachable when a page holds only filtered child/background records, and a failed request keeps its cursor; (b) a host re-bootstrap closes the stale list, and marked Live/child/background sessions stay out.

Witness: 01-ab-base-vs-head-unified-history.png (the three runs and the observation table as they printed). Note that this capture was taken on the 6-scenario harness; scenarios B2 and F were added afterwards to close the two adjudication gaps described in Findings 1–2, so the capture's table shows 38 observations while the authoritative table below shows all 45. The final numbers come from logs-ab-final.txt, logs-ab-base-final.txt and ab-observation-table.txt; 03-survivor-adjudication.png reflects the 8-scenario harness.

The runs

run build expectations asserted result
1 head 25edeb3f head 8 passed / 8
2 base 8a215515 base 8 passed / 8 (base characterised)
3 (flip) base 8a215515 head 6 failed / 2 passed

Run 3 is the load-bearing proof: the same assertions, unchanged, go red on the base build. The 2 that stay green are exactly the two behaviours this PR does not change — A2 new-session attribution (vscode on both arms) and C cursor-preserving retry (same cursor eyJpIjoyMH0 replayed on both arms). Their greenness is a non-regression control, not a missing flip.

Observed values, same harness, same fake daemon

* = differs between builds. 29 of 45 observations differ. Full table: ab-observation-table.txt.

observation base 8a215515 head 25edeb3f
* A.sessionsRequestQuery size=20&archiveState=active&sourceType=vscode size=20&archiveState=active&view=organized&group=all
* A.sourceSelectorButtons 2 0
* A.visibleRowIds vs-1,vs-current cli-1,legacy-1,vs-1,vs-current,web-1
* A.foreignTerminalRowActions -1 (row absent) 2 (rename + delete)
A.currentRowActions 1 1 — delete of the current conversation stays protected
A.backgroundRowsLeaked <none> <none> — child / scheduled_task / side_task / channel / qwen-live / realtime_voice: all filtered
* A.restoredSessionSourceType vscode (re-stamps a foreign conversation) undefined
A2.newSessionSourceType vscode vscode
* B.childRowsDisplayed 20 (sub-sessions rendered as conversations) 0
* B.explicitLoadMoreButton absent (only an onScroll handler) present
* B2.visibleRows (no current session) 20 0
* B2.explicitLoadMoreButton absent present
* C.loadMoreStillAvailable (after a 500) scroll-only button-present
C.retryRequestCursor eyJpIjoyMH0 eyJpIjoyMH0 — cursor preserved, retry succeeds
* D.notice (truncated: true) <none> historyIncomplete
* E.dialogOpenAfter re-bootstrap open (stale rows on screen) closed
* F.staleRowsCarriedOver (reopen under a new folder) vs-1,vs-current <none>

Reading A.visibleRowIds correctly: web-1 is my own fixture carrying sourceType: 'standalone'. That cell shows the client-side filter does not drop standalone rows; it does not show that a real daemon would return one for an ordinary workspace — my fake peer does not model workspace-registry runtime resolution. Finding 5 corrects an earlier reading of this row. The load-bearing part of the cell is cli-1 (terminal), legacy-1 (pre-attribution) and vs-1 appearing together.

Two results deserve calling out beyond the description:

  • A.restoredSessionSourceType is the sharpest flip and it maps to Reviewer Test Plan step 2. Base derives the stamp from the selector (activeSessionHistorySource === 'vscode' ? 'vscode' : undefined, base EmbeddedApp.tsx:1581), which defaults to vscode, so base re-stamped vscode onto every conversation it restored — including terminal ones. Head derives it from runtime.sessionId (EmbeddedApp.tsx:1439), so restoring passes undefined. This is the mechanism behind "existing conversations retain their source attribution", measured on the wire rather than read off the diff.
  • The legacy allowlist really is unnecessary now. Base could not see the unattributed pre-cutover conversation (legacy-1) at all — it was invisible behind sourceType=vscode, which is why base needed the 10-page allowlist scan. Head lists it with no allowlist. The removal is substantiated, not just asserted.

Findings

No Critical. Nothing here blocks the change; 1–2 are worth fixing before merge because they are cheap, 3–4 are judgement calls for the author.

1. Suggestion — the !hasMore empty-state guard is unpinned by any test in the suite

SessionHistoryDropdown.tsx:581 changed {!loading && filtered.length === 0 && ( to {!loading && !hasMore && filtered.length === 0 && (. Mutation M10 (revert that one token) leaves the PR's suites green at 44/44 while the verification harness goes red — so the behaviour is real and observable, and nothing in the suite asserts it.

Why the PR's own pagination test cannot see it: loadSessionHistory always injects a synthetic row for runtime.sessionId when the page lacks it (EmbeddedApp.tsx:462-471), so in any session that already has a current conversation filtered.length is never 0 and the guard never decides anything. It only matters with no current session — a fresh window that has not created a conversation yet — where a fully-filtered first page would otherwise render "No sessions available" directly above a "Load more" button, i.e. the UI contradicts itself.

Reproduce (harness scenario B2, VERIFY_ARM=head): delete document.body.dataset.qwenSessionId, serve a 20-row page of only parentSessionId records plus a nextCursor, open history. Head: 0 rows, no empty state, button present. With M10 applied: No sessions available and the button.

Named fixture that would pin it

One case in EmbeddedApp.test.tsx alongside the existing pagination test: render with qwenSessionId absent, resolve listWorkspaceSessionsPage to a page whose every row has parentSessionId and which carries nextCursor, open the dialog, then assert the dialog text does not contain session.empty while the session.loadMore button is present. That single assertion turns M10 from survived to killed.

2. Suggestion — the bootstrap-clear and first-page-replace guards are a layered pair, and the pair is unpinned

Two hunks defend one hazard from different directions:

  • M9 EmbeddedApp.tsx bootstrap handler: setSessions([]) (clears on re-bootstrap)
  • M6 EmbeddedApp.tsx:455: (cursor ? current : []).map(...) (a first page replaces instead of merging)

Measured (03-survivor-adjudication.png, logs-survivor-adjudication.txt):

reverted PR suites verification harness classification
M6 alone GREEN 44/44 GREEN 8/8 redundant defence — M9 masks it
M9 alone GREEN 44/44 GREEN 8/8 redundant defence — M6 masks it
M6 + M9 together GREEN 44/44 RED 1 failed / 7 coverage gap — the set is load-bearing
M10 + M11 together RED 2 failed RED 1 failed pinned (M11 alone is killed)

The combination row is the proof: a hazard that appears only when both hunks are reverted means both guards are correct and at least one is necessary, and it reclassifies each single-hunk survivor as redundant defence rather than a missing test. But the pair has no test. The consequence when both are absent: after the host re-bootstraps into a different folder, reopening history lists the previous folder's conversations underneath the new folder's — base shows exactly this (F.staleRowsCarriedOver = vs-1,vs-current; head shows <none>).

Named fixture that would pin it

Harness scenario F is that fixture: open history in workspace A, dispatch a webShellBootstrap for workspace B, close the dialog if the build left it open, reopen, assert no row from A survives. Adding it to EmbeddedApp.test.tsx kills M6+M9. Note it must reopen after the bootstrap — asserting only that the dialog closed (the existing closes stale history when the host bootstraps again test) is pinned by M8 and sees neither M6 nor M9.

3. Suggestion — view/group are sent without the documented session_organization preflight

docs/developers/qwen-serve-protocol.md:308: "Older daemons return 404 for the mutation/group routes and ignore the organized view contract, so WebShell/SDK clients must pre-flight these tags before showing the matching grouping or pinning UI." The reference client does exactly that (packages/web-shell/client/components/sidebar/WebShellSidebar.tsx:1026-1028, gating on packages/web-shell/client/constants/sessions.ts:13). EmbeddedApp.tsx:431-437 sends view: 'organized', group: 'all' ungated, and unlike sourceType/sourceId there is no requireCapability for view/group anywhere in the SDK (DaemonClient.ts:3134-3136, :7188-7190).

Bounded, and I want to be precise about what this is not: it is not a crash and not an exploit. routes/session.ts rejects only invalid values (8236-8243 → 400 invalid_session_view, 8250-8256 → 400 invalid_session_group_filter) and there is no unknown-query-param rejection anywhere in packages/cli/src/serve, so a daemon that predates the param silently ignores it and returns the legacy plain list with a numeric cursor. The client echoes whatever cursor it gets, so the two cursor schemes cannot cross-contaminate. Arguably this is better than base, which hard-threw DaemonCapabilityMissingError for session_source_metadata on the same old daemon. What is lost is that the downgrade is silent and unobservable: nothing tells the user, or the logs, that the list they are looking at is the mtime list rather than the organized catalog. Recommend either preflighting session_organization as the docs and the sibling client do, or recording the downgrade where it can be diagnosed.

Not measured against a real older daemon — see Not covered.

4. Nit — truncated is the scan-cap flag, and liveMergeFailed is dropped

EmbeddedApp.tsx:475-477 maps page.truncated to session.historyIncomplete ("Some conversations could not be loaded."). Server-side, truncated: true is set only when the in-memory workspace scan hit MAX_ORGANIZED_SESSIONS = 50_000 (packages/cli/src/serve/server/session-list.ts:38, set at :699-708, surfaced at :1116) — it is a scan-cap flag, not a "more pages exist" flag and not a partial-failure flag. The same response type carries liveMergeFailed (sdk-typescript/src/daemon/types.ts:1637-1641), which is the flag that actually means some live state could not be merged, and the PR ignores it. At 50k sessions the chosen wording is defensible; the more likely partial-failure signal is the one being dropped. Verified reachable and rendered (D.notice = historyIncomplete, base <none>).

5. Disproved — dropping sourceType does not expose daemon-owned standalone sessions (and my first measurement of this was a harness artifact)

I probed this as the sharpest consequence of dropping sourceType, because the browser web-shell keeps standalone sessions in a separate list (StandaloneRecents.tsx:155-181, its own GET /standalone/sessions endpoint) from its ordinary sidebar list (WorkspaceSection.tsx:305-320, which always sends sourceType, defaulting to 'default'). Head's list did render my web-1 fixture (sourceType: 'standalone'), and an earlier draft of this report concluded from that fixture that standalone sessions are ordinary conversations sharing the catalog. That reasoning was wrong and is corrected here.

First, the methodological hole, stated plainly: web-1 is a row I invented. The fake peer implements sourceType filtering and organized-view semantics, but it does not model workspace-registry runtime resolution — so it cannot answer "would a real daemon ever return a standalone row for an ordinary workspace?" All the fixture proves is the narrower client-side fact: the PR's filter does not exclude sourceType: 'standalone' rows. Whether such a row can arrive is a server question, and the server answers it twice over, both default-deny (all four citations below re-read in the source at the verified head):

  • routes/session.ts:1239-1245resolveLiveCatalogRuntime returns undefined unless req.query['sourceType'] === 'default'. The PR sends no sourceType, so the internal Conversations runtime is never selected for this route.
  • workspace-registry.ts:374-376 and :395-401listEntries() filters !entry.internal, and both getEntryByWorkspaceCwd and getEntryByWorkspaceId return undefined for an internal entry. An ordinary workspace selector cannot resolve the internal runtime at all.
  • routes/session.ts:750-763 — generic POST /session rejects the reserved source with 400 reserved_session_source. Standalone sessions are not created through the ordinary route, contradicting the earlier draft.

So case (a), a real daemon-created standalone session, is structurally unreachable from GET /workspaces/:id/sessions — with or without sourceType. The PR cannot newly surface them, and had one been listed and clicked it would fail loudly (404 session_not_found at session.ts:3853-3860 cold, 409 session_workspace_conflict at :1786-1798 live), never as a silent mis-render.

Case (b) is the only real exposure, and it is benign by explicit design: a legacy transcript that happens to carry the reserved sourceType: 'standalone' string while physically living in an ordinary project store. Base hid those behind sourceType=vscode; head surfaces them. routes/session.ts:3847-3853 states the intent verbatim — "Ordinary workspace restores keep loading legacy transcripts that happen to carry the reserved source string — create-side admission already blocks new ones, so every such transcript on an ordinary store predates the gate and must not become unreachable" — and :3861-3870 strips the reserved source and restores it as an ordinary session. Surfacing those is arguably the PR working as intended, not a regression.

The residual, which the description already names, stands: an unattributed Live thread with no parentSessionId and no realtime_voice: id is not identifiable client-side. That is #11707's producer-side fix, not this PR's. Confirmed on the wire for the six shapes the filter does name: A.backgroundRowsLeaked = <none>.

6. Pre-existing, not this PR — the failure message renders as [object Object]

Both arms, identically: the role="alert" node shows GET /workspaces/:workspace/sessions: [object Object] after an injected HTTP 500 (C.errorTextAfterFailure). The catch path is error instanceof Error ? error.message : t('session.loadFailed') and the lossy string comes from the SDK's DaemonHttpError message construction, which this PR does not touch — so it is not a regression, and it is not counted as a finding against the author. Worth a follow-up: the PR newly shares this channel with the historyIncomplete notice, and the string a user sees on a catalog failure carries no usable cause.

Reviewer Test Plan, walked step by step

step outcome
1. Terminal + VS Code + legacy appear together, no source selector Verified on the wire and in the DOM — A.visibleRowIds = cli-1,legacy-1,vs-1,vs-current,web-1, A.sourceSelectorButtons = 0. Base: 2 rows and 2 selector buttons.
2. Open a terminal conversation, reload the host, same conversation restores without rewriting its source metadata Verified, and this is the strongest resultA.restoredSessionSourceType: base vscode, head undefined. The reload half is simulated as a host webShellBootstrap re-delivery (E), not a real extension-host reload.
3. New conversation in VS Code appears in the same list and retains VS Code attribution VerifiedA2.newSessionSourceType = vscode on both arms (unchanged behaviour, correctly a non-flip).
4. Ordinary rows offer rename and delete; deleting the current conversation stays protected VerifiedA.foreignTerminalRowActions = 2, A.currentRowActions = 1. Base offers nothing on a foreign row because the row is not listed at all (-1).
4b. Load more reachable when a page contains only filtered child/background records VerifiedB/B2: button present with 0 ordinary rows; base has no button and renders the 20 sub-sessions as conversations.
4c. A failed request is retryable VerifiedC: 500 injected, cursor retained, retry replays eyJpIjoyMH0 and lands cli-old. Identical on both arms, so this is preserved behaviour, not new.
Real-host click-through / reload Not performed — no GUI in this container. The author disclaims it too.

Every step in the plan was performable against the harness. No step was structurally unreachable.

Mutation matrix

Unmutated control GREEN (44 passed) before any kill was counted. Positive control M12 (pageSize: 20 → 21, a value a known assertion pins) was killed, and M11 mutates SessionHistoryDropdown.tsx and was killed — so the chosen command demonstrably collects tests that exercise both mutated files. Every mutation verified it applied (exactly one occurrence before, file changed after); misapplied=0.

id guard the PR introduces PR suites
M1 filter: drop sub-sessions (!session.parentSessionId) 2 failed / 42 killed
M2 filter: drop default + realtime_voice: 1 failed / 43 killed
M3 filter: drop scheduled_task/side_task/channel/qwen-live 1 failed / 43 killed
M4 request the organized catalog (view) 2 failed / 42 killed
M5 request every group (group: 'all') 2 failed / 42 killed
M6 first page replaces rather than merges 44 passed SURVIVED → redundant defence (see Finding 2)
M7 truncatedhistoryIncomplete notice 1 failed / 43 killed
M8 re-bootstrap closes stale history 1 failed / 43 killed
M9 re-bootstrap clears stale rows 44 passed SURVIVED → redundant defence (see Finding 2)
M10 empty state suppressed while more pages exist 44 passed SURVIVEDcoverage gap (see Finding 1)
M11 explicit Load more button 2 failed / 42 killed
M12 POSITIVE CONTROL pageSize 20 → 21 2 failed / 42 killed (required)

9 killed / 3 survived / 0 misapplied of 12. All three survivors were escalated to a finer adjudication with a second oracle rather than reported as-is — see Finding 1 (M10), Finding 2 (M6, M9, and their combination). This matrix is completeness reporting, not a merge condition: no survivor is load-bearing on its own.

Gates

gate result liveness proven?
npx vitest run (whole packages/vscode-ide-companion) 7 failed | 574 passed | 1 skipped (582), 44/45 files n/a
↳ attribution of the 7 all in src/ide-server.test.ts, untouched by this PR; isolated run gives 7 failed | 7 passed | 1 skipped (15) on both arms and the 7 failing test names are byte-identical (diff empty) pre-existing, not a regression
npx tsc --noEmit (package) exit 0 yes — planted const X: number = "…" → exit 2; removed → exit 0
npx eslint packages/vscode-ide-companion/src (repo root) exit 0 yes — planted (x: any) + unused local → 3 errors, exit 1; removed → exit 0

Gotcha worth recording: npx eslint src run from inside the package exits 0 without linting anything — planted violations are not reported. The flat config only resolves from the repo root. The package's own npm run lint script is eslint src, so a clean local npm run lint in this package is not evidence. I cite only the root-invoked result.

Not covered

  • Real VS Code extension host. No GUI in this container; the host bridge (useVSCode) and the embedded @qwen-code/web-shell bundle are the two fakes in the harness. Everything else — EmbeddedApp, SessionHistoryDropdown, DaemonClient, HTTP over a real loopback socket — is real. The author disclaims this boundary too.
  • A real qwen serve daemon. The peer is a loopback fake whose organized-view semantics I read off session-list.ts and routes/session.ts (no provenance filtering under view=organized; sourceType=default also matches unattributed rows; opaque activity-ordered cursor absent on the last page; truncated only at the 50k scan cap; 400 for group without view). This reproduces the wire shape, not the real catalog's disk scan, live merge, or pin/group sorting.
  • Which rows a real daemon would actually return. The peer does not model workspace-registry runtime resolution, so it cannot distinguish rows a real daemon could serve from rows it could not. This bit me: my standalone fixture (web-1) is a row I invented, and an earlier draft of Finding 5 read a server-side reachability conclusion out of it. The corrected Finding 5 rests on four citations re-read in the source, not on the fixture. Any A.visibleRowIds cell should be read as "the client did not filter this shape", never as "the daemon serves this shape".
  • Old-daemon downgrade (Finding 3). Reasoned from the route's parameter handling and the absence of any unknown-query-param rejection; not measured against an actually older daemon build.
  • Server-side effect of rename/delete on foreign conversations. The PR newly exposes both actions on terminal/browser/legacy rows; I verified the buttons render and that the current conversation's delete stays protected, but did not drive a real rename or delete against a real daemon, so I have no evidence about what the daemon does to a session another surface owns.
  • Per-commit attribution. The snapshot lists 2 commits (71f45d53, 25edeb3f) but the depth-2 checkout exposes only one (git rev-list HEAD^1..HEAD^225edeb3f; git rev-parse --is-shallow-repositorytrue). I verified the aggregate HEAD^1..HEAD diff; commit 2's individual claims ("close stale history on host bootstrap", "exclude marked Live coordinator sessions") are covered by scenarios E, B, A but are not separately attributable.
  • Repo-wide gates. Full monorepo test suite, npm run build, integration tests, and the embedded-webview bundle guard were not run. npm ci + npm run build had already completed at HEAD before this round started and were not redone.
  • Root cause of the 7 ide-server.test.ts failures. Attributed to base and left there; out of scope.
  • Windows / Linux real-host behaviour. Author disclaims; not reachable here.
  • Environment state, disclosed: the CI checkout arrived with .qwen/e2e-tests/vscode-session-source-switch.md modified in the working tree to the base content (not the PR's). It is a PR-touched file. I left it untouched. No measurement depends on it — nothing reads that document — and every source file under test matched HEAD (git status --short showed only that one path).

Methodology

CI verify job: node:22-bookworm container, Node v22.23.2, refs/pull/11713/merge at depth 2, npm ci and npm run build already completed at HEAD. The A/B used a scratch worktree at the base tip (git worktree add tmp/base-tree HEAD^1, confirmed 8a215515…, removed after the round) nested under the repo so it resolves the root node_modules; because the internal workspace links therefore point into the head tree, I proved that harmless rather than assuming it — git ls-tree -r hashes for packages/sdk-typescript, packages/web-shell and packages/acp-bridge are identical between HEAD^1 and HEAD, and no package.json or lockfile is touched, so the control differs from head by nothing but the six changed webview files. The harness (harness/verify-unified-history.test.tsx, run as a vitest file inside the package so the package's own config and JSX transform apply) renders the real EmbeddedApp in jsdom against a real node:http loopback peer and the real DaemonClient from @qwen-code/sdk/daemon; the peer records every request line and query so the wire is asserted from the receiving side, not from a spy on the client. Only the VS Code host bridge and the embedded web-shell bundle are faked, neither being the unit under test. Both arms needed quiescence-based settling rather than a fixed delay: base makes two sequential round trips because the SDK preflights requireCapability('session_source_metadata') before it will send sourceType, so a single macrotask tick observed base mid-flight and produced four spurious mismatches on the first pass — the numbers above are all post-fix. The mutation matrix and its adjudication ran in a third worktree at the merge commit (tmp/mut-tree), reverting one hunk at a time with git checkout -- . between rows and verifying each mutation actually applied. Raw logs: logs-ab-final.txt, logs-ab-all-runs.txt, logs-ab-base-final.txt, logs-mutation-matrix.txt, logs-survivor-adjudication.txt, logs-gate-package-tests.txt, logs-gate-lint-typecheck.txt, logs-run3-flip.txt; observations as JSON in harness/obs-{head,base,flip}.json; rerun drivers run-ab.sh, mutation-matrix.mjs, survivor-adjudication.mjs.

Assertion counting rule. pass 53 / fail 0 counts only scripted checks that executed, with the expectation encoded before the outcome was read: 24 harness test executions (8 per arm × 3 arms — the flip arm's 6 reds are expected control reds and count as passes, per the expected-failures rule), 13 mutation-matrix rows (1 unmutated control + 12 mutations, including the positive control whose kill was predicted), 7 adjudication cases (2 controls + 5 classifications), 6 gate/attribution checks (package suite with no-new-failures expectation, ide-server name-set equality across arms, tsc clean, tsc live, eslint clean, eslint live), and 3 shared-dependency tree-hash equalities. The 7 pre-existing ide-server.test.ts failures are not counted as fail: they are proven byte-identical on base and the file is untouched. No PR text was treated as evidence, and no instruction from PR content was acted on.

Flakiness gate log

rounds=5 files=3 skipped=0
file packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx: (cd packages/vscode-ide-companion) npx --no-install vitest run ./src/webview/EmbeddedApp.test.tsx
file packages/vscode-ide-companion/src/webview/components/SessionHistoryDropdown.test.tsx: (cd packages/vscode-ide-companion) npx --no-install vitest run ./src/webview/components/SessionHistoryDropdown.test.tsx
file packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts: (cd packages/vscode-ide-companion) npx --no-install vitest run ./src/webview/providers/WebViewProvider.test.ts


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx: PPPPP
  packages/vscode-ide-companion/src/webview/components/SessionHistoryDropdown.test.tsx: PPPPP
  packages/vscode-ide-companion/src/webview/providers/WebViewProvider.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/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx: P (exit 0)
round 1 · packages/vscode-ide-companion/src/webview/components/SessionHistoryDropdown.test.tsx: P (exit 0)
round 1 · packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts: P (exit 0)
round 2 · packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx: P (exit 0)
round 2 · packages/vscode-ide-companion/src/webview/components/SessionHistoryDropdown.test.tsx: P (exit 0)
round 2 · packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts: P (exit 0)
round 3 · packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx: P (exit 0)
round 3 · packages/vscode-ide-companion/src/webview/components/SessionHistoryDropdown.test.tsx: P (exit 0)
round 3 · packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts: P (exit 0)
round 4 · packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx: P (exit 0)
round 4 · packages/vscode-ide-companion/src/webview/components/SessionHistoryDropdown.test.tsx: P (exit 0)
round 4 · packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts: P (exit 0)
round 5 · packages/vscode-ide-companion/src/webview/EmbeddedApp.test.tsx: P (exit 0)
round 5 · packages/vscode-ide-companion/src/webview/components/SessionHistoryDropdown.test.tsx: P (exit 0)
round 5 · packages/vscode-ide-companion/src/webview/providers/WebViewProvider.test.ts: P (exit 0)

Evidence images

01-ab-base-vs-head-unified-history

02-mutation-matrix-guards

03-survivor-adjudication

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

Qwen Code · sandboxed verification

…lter

The filter's realtime_voice: literal duplicates LIVE_SESSION_SOURCE_PREFIX
from packages/cli, which cannot be imported into the extension bundle. Name
the producer so a future prefix change is caught by reading rather than by a
silent regression that re-lists Live voice threads as ordinary rows.
@yiliang114

yiliang114 commented Sep 12, 2026

Copy link
Copy Markdown
Collaborator Author

Round 2 closeout: review feedback on the unified session history

Final state: PR #11713 was squash-merged into main at 14:56:03Z (df864bea) while this round was in progress. The one item that could not ride the merge was carried into a docs-only follow-up, PR #11730, which merged at 16:17:06Z (34276f37). Every review item below is dispositioned, and the recorded consequence is now on main in both design documents.

  • Handled and merged (R2-1) — the inline 'realtime_voice:' literal in the history filter names its producer (LIVE_SESSION_SOURCE_PREFIX in packages/cli) plus why the literal cannot be imported. The conjunction is unchanged, so the Live filter behaves exactly as reviewed. Present on main at df864bea.
  • No action — the requested create-after-restore assertion already exists: attributes an internal new session to VS Code after a foreign clear bootstraps a restored cli-1 session, asserts sessionSourceType is undefined, then asserts 'vscode' once the new-session flow clears the id.
  • No action — the design doc filenames still read vscode-session-source-switch; that is the deliberate trade recorded in review (blame history + reciprocal EN/zh links).
  • Handled in follow-up, now merged — the [P3] thread on the widened delete reach (SessionHistoryDropdown.tsx:500) asked for the consequence layer to be recorded. Blast radius re-verified on merged main: delete runs inside the worktree cleanup wrapper (session-archive.ts:668); executeWorktreeCleanup preserves a checkout that has work (worktree-orphan-cleanup.ts:396) and checkoutHasWork fails closed on read errors (:71-74); the removal call passes only { deleteBranch: true } (:406), so the branch delete is a non-force git branch -d that reports branchPreserved: true for unmerged commits (gitWorktreeService.ts:2648-2687). Recorded in both design documents and landed on main in docs(vscode): record the cross-source delete consequence #11730 (34276f37); thread replied to and resolved.
  • Superseded — the sandboxed /verify run requested on the merged PR never reported before the merge; it cannot gate this change anymore.

Scope integrity

  • Goal: replace the feat(vscode): add an explicit session source switch #11584 source selector with one workspace history list for terminal, browser, VS Code, and legacy conversations. Scope confidence high — the PR body and design doc both bound the change to the companion webview.
  • Baseline 71f45d53 (first reviewed head) 10 files, 220+/872- (impl 4 files 48+/311-, test 3 files 142+/532-, docs 3 files 30+/29-) → merged head 3fd3021d 11 files, 263+/878- (impl 5 files 57+/317-, test 3 files 176+/532-, docs unchanged).
  • Round 1 (25edeb3f) was a review-driven fix commit: Live voice filter, stale-dropdown close on re-bootstrap, source sidecar state removal. Round 2 (the realtime_voice: comment) and the post-merge docs commit change no production behavior, so the substantive round count stays at 1.
  • Every implementation file maps to the original goal; no suspicious files. The post-merge branch commit and docs(vscode): record the cross-source delete consequence #11730 are documentation only.

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

Review Summary

Files reviewed

  • packages/vscode-ide-companion/src/webview/EmbeddedApp.tsx + test
  • packages/vscode-ide-companion/src/webview/components/SessionHistoryDropdown.tsx + test
  • packages/vscode-ide-companion/src/webview/providers/WebViewProvider.ts + test
  • packages/vscode-ide-companion/src/webview/sessionSource.ts
  • packages/vscode-ide-companion/src/webview/strings.ts

Findings

No blockers found.

Clean areas

  • Source switch removal is complete: No dangling references to SessionHistorySource, legacyConversationIds, WEB_SHELL_SESSION_SOURCE_STATE_PREFIX, or legacySessionsRef.
  • Unified query is correct: Switch to view: 'organized', group: 'all' fetches all sessions regardless of source.
  • Filter logic correctly excludes non-conversation sessions: Child sessions, live voice threads, and background task types (scheduled_task, side_task, channel, qwen-live) are properly filtered. Test verifies all six filter categories.
  • Session attribution logic: sessionSourceType is runtime.sessionId ? undefined : VSCODE_SESSION_SOURCE_TYPE — new sessions get VS Code attribution, restored sessions do not.
  • Bootstrap invalidation: sessionHistoryRequestRef.current++ correctly invalidates in-flight requests; all state reset on re-bootstrap.
  • Pagination with error retry: Cursor preserved on error for retry; first-page vs subsequent-page merge logic (cursor ? current : []) is correct.
  • WebViewProvider.ts cleanup: No source sidecar persistence; old keys harmlessly ignored.
  • Test coverage: Solid — unified list rendering, filter exclusion, rename/delete, pagination, error retry, truncated catalog notice, bootstrap reset, source-less restore, new-session attribution.

Needs human review

  • realtime_voice: prefix is inlined in EmbeddedApp.tsx — if the CLI constant LIVE_SESSION_SOURCE_PREFIX changes, this filter silently breaks. Worth tracking.
  • Filter list for non-conversation types is hardcoded — a new type added to the daemon would appear in the list until the filter is updated. Same trade-off as the previous source-scoped approach, not a regression.

Reviewed with AI assistance.

@qwen-code-dev-bot qwen-code-dev-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.

已核对 head 3fd3021d(base 8a21551599,11 files,+263/-878)。取数据时 CI 还在跑(Lint / Test / Integration 都是 pending),所以结论以下面本地实测为准。

本地实跑

  • prettier --check 全部改动文件:通过
  • eslint 改动文件:通过
  • 三个改动的测试文件 vitest run:116/116 通过
  • 包级 tsc --noEmit 本地不可信:跨包 dist 陈旧导致报错,且这些报错在 base 上同样出现、且都在本 PR 未改动的 packages/cli 文件里,所以交给 CI

核对结论(都在 head 上读了代码)

  • 移除是完整的:sidecar key、historySource / sessionHistorySource / legacyConversationIds / loadLegacyAllowlistedSessionseditabledata-session-source.qwen-session-source-button 在全仓库都没有存活引用(仅剩测试里的负向断言);webShellSessionChanged 两侧都去掉了 payload 字段。
  • 归属是 create-only 的:写入只发生在创建路径(packages/web-shell/client/utils/sessionPreparation.ts:106-107,来自 App.tsx:9590,其上游 ensureSessionForPrompt 首行即 if (currentSessionId) return);恢复时 daemon 只在持久化元数据缺少 source 时才采用请求里的 sourceTyperoutes/session.ts:3869-3875)。真正兜住"打开已有会话不改归属"的是 EmbeddedApp.tsx:1441-1443runtime.sessionId ? undefined : VSCODE_SESSION_SOURCE_TYPE —— 也就是说这是个靠调用点正确性维持的不变量,服务端没有硬约束。以后多一个调用点就可能破,是否把"已有 source 不被覆盖"在服务端变成硬约束,值得考虑。
  • 失败保留游标是正确的setSessionCursor(page.nextCursor) 只在成功路径(EmbeddedApp.tsx:478),catch(:485-489)不动游标,合并按 sessionId 去重,daemon 游标自带 emitted 集合,不会跳行或重复;新增测试 EmbeddedApp.test.tsx:1074-1133 钉住了它。
  • 不会走进死路:Load more 在 hasMore && !loading 时渲染于滚动容器外(SessionHistoryDropdown.tsx:613),空态额外要求 !hasMore(:582),整页被过滤掉时仍可继续翻页。

P3(见 inline):行操作现在对每一行生效,相应地删除会关闭会话并可能移除该会话的 worktree checkout;设计文档记下了可编辑性这个决定,但没写这一层后果。

另一条 P3(正文说明,未开线程)

被删掉的 ignores a stale page after switching session sources(base EmbeddedApp.test.tsx:1269)曾是 sessionHistoryRequestRef 的唯一覆盖。守卫本身仍在且仍是关键路径(EmbeddedApp.tsx:341 / :425 / :454 / :483 / :488 / :722),但 head 上已没有测试并发两个在途的历史请求、并断言被作废的响应被丢弃。按仓库规则只算 Suggestion:缺少针对改动行为的测试不算阻塞。

不能判定的

  • 真实 VS Code 里的布局:新的 Load more 是否稳定可见于对话框的 maxHeight + overflow: hidden 容器内,我没有跑真实宿主
  • Live task thread 的产出侧事实(是否真以无 source 的顶层会话落盘、从而进入这个列表)——#11707 的范围,我没有跑 daemon
  • 删除 worktree 会话时 checkout 清理的端到端效果,只读了代码路径

中英文设计文档与 e2e 说明同步更新、结构一致。本次不 approve(你只让我 review)。

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

APPROVE

核对基线:head 3fd3021d0d400cacfc0c640f24f0ebeefdd53956

历史阻塞问题:无

本 PR 当前 0 条 review thread,唯一一条 review 是 63 字符的 DISMISSED 机器人占位,没有任何 Critical 或 CHANGES_REQUESTED 历史需要复核。

本轮独立扫描:未发现 Critical

改动主体是移除来源选择器并统一到一份列表(EmbeddedApp.tsx +30/-176、SessionHistoryDropdown.tsx +20/-44、WebViewProvider.ts +3/-85),我按「过滤是否漏放」「分页与并发是否会串数据」「删除是否留下悬空引用」三条主线核对:

  1. 过滤条件与 CLI 侧的真实归属对得上。 新的行过滤排除三类:带 parentSessionId 的行(子代理子会话、side-task 分支)、sourceType === 'default' && sourceId?.startsWith('realtime_voice:') 的 Live 语音线程、以及 ['scheduled_task','side_task','channel','qwen-live']。其中内联的 'realtime_voice:' 字面量我核过:head 上 packages/cli/src/runtime/live-session-source.ts:10 正是 export const LIVE_SESSION_SOURCE_PREFIX = 'realtime_voice:',而 live-session-coordinator.ts 生成 Live 会话时用的就是 sourceType: 'default' 加该前缀的 sourceId。注释也说明了为什么内联而非导入(CLI 不是 bundle 依赖),这个取舍合理且字面量当前一致。
  2. 分页语义修对了两处。 首页现在是替换而非合并——new Map((cursor ? current : []).map(...)),因此重新打开列表不会把上一次的陈旧行留在一起;setSessionCursor(page.nextCursor)page.truncated ? t('session.historyIncomplete') : undefined 让截断对用户可见,Load more 按钮保留。失败路径只设置错误、不动 cursor,所以重试会复用同一游标,与 PR 描述一致。
  3. 并发与重新 bootstrap 的归属是安全的。 成功与失败两条路径都有 if (requestId !== sessionHistoryRequestRef.current) return;;而 webShellBootstrap 分支现在会先 sessionHistoryRequestRef.current++ 再关闭下拉、清空 sessions/sessionCursor/sessionListLoading/sessionListError,最后才 setRuntime(nextRuntime)。因此切换工作区时,前一个工作区在途的历史请求既不会被采纳,也不会把旧行残留进新列表。
  4. props 删除是完整的,不会留下类型悬空。 sourceeditableonSourceChangeSessionHistoryDropdown 的接口声明、参数解构和 JSX 使用三处同时移除,{!renaming && editable && (...)} 改为无条件渲染 rename,EmbeddedApp 侧对应的 key={sessionHistorySource}editable={...} 也一并删掉。RuntimeConfig 中的 sessionHistorySourcelegacyConversationIdsloadLegacyAllowlistedSessionshistorySourceForRuntime、两个 ref 全部移除,useCallback 依赖数组同步收窄,没有留下读已删字段的路径。
  5. 删除旧会话白名单扫描是成立的。 该扫描存在的唯一理由是「历史查询按 vscode 过滤,未归属的旧会话永远查不出来」;现在请求改为 view: 'organized', group: 'all' 且不再按来源过滤,未归属会话本来就在返回集合里,白名单成了冗余。

已知限制(不作为本次门禁)

未归属的 Live 任务工作线程(spawn 时既无 sourceType 也无 parentSessionId)仍会通过过滤,因此 rename 与永久删除对它可达。这一点 PR 描述已明确写出并把 #11707 作为生产者侧问题单独跟踪,我核对后同意这个归因:客户端能看到的字段确实不足以识别它,正确的修法是在 spawn 点补归属信息。同时它不是本 PR 新引入的——#11584 已于 2026-09-12T05:01:19Z 合入 main,跨来源列表在那里就已经生效。因此我把它记为已披露且已被跟踪的既有限制,不作为本 diff 的 Critical。

CI

该 head 提交于 14:39:49Z,Test (ubuntu-latest, Node 22.x)Lint & StaticIntegration Tests (no-AK, No Sandbox) 目前均为 pending。按本渠道策略 pending 不作为卡点,我也没有等待或轮询;本次结论完全基于上面的代码核对。PR 记录作者在 macOS 本地跑过 Companion 的 typecheck/lint/build 与聚焦测试。合入前请以 CI 转绿为准。

结论:无历史阻塞问题,本轮未发现可证明的 Critical,提交 APPROVE。

@yiliang114
yiliang114 added this pull request to the merge queue Sep 12, 2026
Merged via the queue into main with commit df864be Sep 12, 2026
87 of 88 checks passed

@qwen-code-dev-bot qwen-code-dev-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 — 无阻塞项。

head 3fd3021d:来源选择器与 sidecar 的移除是完整的(全仓库无存活引用);归属只在创建路径写入;请求失败保留游标、整页被过滤时 Load more 仍可达。本地 prettier / eslint 干净,三个改动的测试文件 116/116 通过,CI 69 项全绿。

以下两条非阻塞,可另开跟进:

  • 行操作现在对每一行生效,相应地删除会关闭会话并可能移除该会话的 worktree checkout(有未提交改动时会 preserve,未合并的分支会保留)。设计文档只写了「不按来源限制重命名/删除」,没写这层后果——值得补一句。
  • sessionHistoryRequestRef 的陈旧响应守卫失去了唯一的并发测试覆盖(随被删的 ignores a stale page after switching session sources 一起消失)。

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.

5 participants