feat(cli): List persisted sessions for trusted workspaces - #6558
Merged
Conversation
Add trusted non-primary active persisted session discovery for plural workspace session list routes. Preserve live-only fallback behavior when no active persisted sessions exist, and keep archived or organized non-primary list options gated. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Contributor
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
Collaborator
|
Thanks for the PR! Template looks good ✓ Problem: This is a feature completion PR — Phase 2b already made trusted non-primary persisted load/resume workspace-aware, but the session list routes couldn't discover active persisted sessions in secondary workspaces. This closes that discovery gap. Linked to #6378. Not a bug fix — it's a planned incremental feature. Direction: Clearly aligned with the ongoing multi-workspace effort. The design doc and protocol doc are updated to match. No product direction concerns. Size: 22 production lines + 5 deletions in Approach: The implementation is minimal and clean — adds a Moving on to code review. 🔍 中文说明感谢贡献！ 模板完整 ✓ 问题：这是功能补全 PR —— Phase 2b 已让 trusted non-primary persisted load/resume 支持 workspace-aware，但 session list routes 无法发现 secondary workspace 中的 active persisted sessions。关联 #6378。属于计划中的增量功能，不是 bug fix。 方向：与 multi-workspace 开发方向一致，设计文档和协议文档同步更新，无产品方向问题。 规模： 方案：实现极简 —— 新增 进入代码审查 🔍 — Qwen Code · qwen3.7-max |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the CLI serve workspace session listing routes to include active persisted sessions for trusted non-primary workspaces, merging them with live session summaries without duplicates, while preserving the previous live-only fallback behavior when a non-primary workspace has no active persisted sessions.
Changes:
- Add a persisted-session “probe” for non-primary workspace list routes and switch to persisted-backed listing when active sessions exist.
- Update non-primary list option rejection (archived/organized) with a new error code/message.
- Add tests covering persisted/live merging, numeric cursor pagination for persisted-backed lists, and additional selector/error behaviors; update protocol/design docs accordingly.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/cli/src/serve/routes/session.ts | Adds persisted-session probing and conditional selection between persisted-backed vs live-only listing; updates non-primary option rejection code/message. |
| packages/cli/src/serve/multi-workspace-sessions.test.ts | Adds coverage for persisted+live merge, encoded-cwd selectors, numeric cursor pagination, and unsupported options/errors. |
| docs/developers/qwen-serve-protocol.md | Documents trusted non-primary persisted session discovery + cursor semantics (numeric vs live opaque). |
| docs/design/daemon-multi-workspace-phase2a-sessions.md | Updates phase notes to describe the Phase 2b persisted session discovery behavior and constraints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+252
to
+258
| const hasActivePersistedSessions = async (workspaceCwd: string) => { | ||
| const page = await new SessionService(workspaceCwd).listSessions({ | ||
| archiveState: 'active', | ||
| size: 1, | ||
| }); | ||
| return page.items.length > 0; | ||
| }; |
Collaborator
Code ReviewThe implementation is clean and minimal — exactly what you'd want for this feature. What it does: Adds a Correctness: The Minor observation (non-blocking): No AGENTS.md violations. Scope is tight — no drive-by refactors, no unnecessary abstractions, no code in the wrong place. Test ResultsAll three test files the author mentioned pass cleanly on the PR branch: New tests cover: workspace id selectors, encoded cwd selectors, persisted/live merge dedup, numeric cursor pagination, unsupported options rejection, workspace mismatch, and untrusted workspace rejection. Comprehensive. Tmux testing: N/A. This is a server-side HTTP API feature (daemon route handler). No CLI TUI changes to capture — the behavior is exercised through the HTTP test suite above, which simulates 中文说明代码审查实现极简 —— 正是这个功能该有的样子。 做了什么： 新增 正确性： 小观察（不阻塞）： 无 AGENTS.md 违规。 范围紧凑，没有顺手重构、不必要的抽象或代码放错位置。 测试结果作者提到的三个测试文件全部通过： 新增测试覆盖：workspace id selector、encoded cwd selector、persisted/live merge 去重、numeric cursor pagination、unsupported options 拒绝、workspace mismatch 和 untrusted workspace 拒绝。覆盖全面。 Tmux 测试：不适用。 这是服务端 HTTP API 功能（daemon route handler），没有 CLI TUI 变更 —— 行为通过上述 HTTP 测试套件验证。 — Qwen Code · qwen3.7-max |
Collaborator
ReflectionThis is a clean, well-scoped feature completion. The multi-workspace Phase 2b work made load/resume workspace-aware, but users couldn't discover active persisted sessions in secondary workspaces through the list routes. This PR closes that gap with 22 production lines — a probe + branch that reuses the existing persisted/live merge path. The implementation matches what I'd have done independently: check for active persisted sessions, and if they exist, route through the already-correct Tests are thorough — 21 new/existing tests in the multi-workspace file, plus the related rate-limit and server tests all pass. Typecheck is clean across all packages. One minor non-blocking observation: the double Approving. ✅ 中文说明总结这是一个范围合理的功能补全。Phase 2b 的 multi-workspace 工作让 load/resume 支持了 workspace-aware，但用户无法通过 list routes 发现 secondary workspace 中的 active persisted sessions。这个 PR 用 22 行生产代码 —— 一个探测 + 分支复用已有的 persisted/live merge 路径 —— 补齐了这个缺口。 实现方案与我的独立方案一致：检查是否有 active persisted sessions，如果有就走已有的 测试全面 —— multi-workspace 文件中 21 个新增/现有测试，加上相关的 rate-limit 和 server 测试全部通过。Typecheck 在所有 package 中都干净。 一个小观察（不阻塞）：双重 批准合并 ✅ — Qwen Code · qwen3.7-max |
qwen-code-ci-bot
approved these changes
Jul 9, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
LGTM, looks ready to ship. ✅
qwen-code-ci-bot
previously approved these changes
Jul 9, 2026
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
No issues found. LGTM! ✅
— qwen3.7-max via Qwen Code /review
wenshao
reviewed
Jul 9, 2026
wenshao
left a comment
wenshao
left a comment
Collaborator
There was a problem hiding this comment.
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/cli/src/serve/routes/session.ts:252-257 |
hasActivePersistedSessions lacks error handling — non-ENOENT filesystem errors (EACCES, EIO) propagate to the outer catch and return a 500 for the entire listing endpoint, when the probe should fail open to the live-only fallback |
Wrap the body in a try-catch that returns false on error, so transient I/O issues gracefully fall back to live-only listing instead of failing the request |
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Comment on lines
+252
to
+262
| const hasActivePersistedSessions = async (workspaceCwd: string) => { | ||
| try { | ||
| const page = await new SessionService(workspaceCwd).listSessions({ | ||
| archiveState: 'active', | ||
| size: 1, | ||
| }); | ||
| return page.items.length > 0; | ||
| } catch { | ||
| return false; | ||
| } | ||
| }; |
Comment on lines
+935
to
+936
| it('falls back to live-only listing when persisted probing fails', async () => { | ||
| await withRuntimeDir(async () => { |
1 task
qwen-code-ci-bot
previously requested changes
Jul 9, 2026
Collaborator
Suggestions — commit
|
| File | Issue | Suggested fix |
|---|---|---|
packages/cli/src/serve/routes/session.ts:252-262 |
hasActivePersistedSessions probe creates a SessionService and calls listSessions({size:1}), discards the result, then listWorkspaceSessionsForResponse creates a second SessionService and repeats the full listSessions call. This is redundant synchronous I/O on the hot path for every non-primary workspace listing that has persisted sessions. |
Fold the probe into listWorkspaceSessionsForResponse — let it attempt the persisted merge and fall back to live-only if listSessions throws, using a single SessionService instance. Alternatively, replace the probe with a lightweight readdirSync + regex match without stat/sort/JSONL reads. |
— qwen3.7-max via Qwen Code /review
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Collaborator
Runtime verification report — PR #6558 @
|
| Behaviour | Result |
|---|---|
| Trusted non-primary: active persisted sessions listed (by workspace id and encoded cwd) | ✅ 2/2 returned; merge-base returns sessions=0 |
Live ∪ persisted merged, deduped by sessionId |
✅ appears once, carries live clientCount / hasActivePrompt |
archiveState=archived, view=organized, view=organized&group=… on non-primary |
✅ 400 non_primary_session_list_option_not_supported |
Untrusted non-primary → 403 untrusted_workspace (id, encoded cwd, plural alias, archived variant) |
✅ 10/10, and no persisted data leaked |
| Trust gate runs before the new store probe | ✅ untrusted workspaces never touch disk |
| Unknown absolute cwd | ✅ 400 workspace_mismatch |
Primary route (incl. archiveState=archived) |
✅ unchanged, still 200 |
| Live-only fallback when the store has no active sessions | ✅ preserved, opaque cursor still paginates |
packages/cli serve suites |
✅ multi-workspace-sessions 22/22, server.test.ts 619/619 (641 total) |
npm run typecheck |
✅ clean (worth stating — CI runs lint + test:ci but not tsc) |
Merge-base (d8084c6) under the identical driver — for comparison
The ✘ marks here are the expected absences: sessions=0 for persisted discovery, the old non_primary_live_sessions_only code, and no numeric cursor.
🔴 Finding 1 — a client's pagination cursor is invalidated by an unrelated write (new in this PR)
The two branches this PR now selects between mint incompatible cursor formats:
listWorkspaceSessionsForResponse→ numeric mtime cursor, e.g."1783569663671.999"listLiveWorkspaceSessionsForResponse→ opaque base64url cursor, e.g."eyJhY3Rpdml0eVRpbWUiOjE3…"
The branch is chosen per-request from mutable disk state (await hasActivePersistedSessions(key)), but the cursor handed to the client is a stateless token. So if the persisted store gains (or loses) its first active session between page 1 and page 2, the daemon rejects the cursor it issued moments earlier:
GET /workspace/:id/sessions?size=1 → 200, nextCursor="eyJhY3Rpdml0eVRpbWUi…"
(a prompt runs in that workspace; the session logger writes chats/<id>.jsonl)
GET /workspace/:id/sessions?size=1&cursor=eyJhY3Rpdml0… → 400 invalid_cursor
Invalid cursor: "eyJhY3Rpdml0eVRpbWUi…" is not a valid numeric cursor
This needs no test scaffolding and no planted files — it is exactly what happens when a user opens a trusted secondary workspace, starts a couple of sessions, lists them, and then sends their first prompt. I reproduced it end-to-end by driving a real POST /session/:id/prompt against a fake OpenAI server and letting the session logger persist chats/<id>.jsonl naturally.
The reverse direction breaks too: a numeric cursor is handed out, the last active persisted session is archived/deleted, and page 2 returns 400 invalid_cursor: "1783569663671.999" is not a valid live cursor.
A/B — same driver, same daemon, same prompt; only session.ts differs:
On merge-base the non-primary route was always live-only, so its cursor format was stable and this could not happen. The PR's own two cursor tests (pages active persisted … with numeric cursors, pages live … with a stable cursor) both hold the store constant across the two page requests, which is why they don't catch it.
Suggested fix — don't re-probe the store when the client already holds a cursor; let the cursor's own shape decide which branch serves it. This also skips the probe entirely on pagination requests, which addresses Finding 3 for the common case:
+ // The persisted-backed list issues a numeric mtime cursor; the live-only
+ // fallback issues an opaque base64url cursor. The two are not
+ // interchangeable, so a cursor already handed to a client must keep being
+ // served by the branch that minted it.
+ const isPersistedSessionCursor = (cursor: string) => {
+ const trimmed = cursor.trim();
+ if (trimmed === '') return false;
+ const parsed = Number(trimmed);
+ return Number.isFinite(parsed) && parsed >= 0;
+ };
- const result =
- runtime.primary || (await hasActivePersistedSessions(key))
- ? await listWorkspaceSessionsForResponse(runtime.bridge, key, options)
- : listLiveWorkspaceSessionsForResponse(runtime.bridge, key, options);
+ const usePersistedList =
+ runtime.primary ||
+ (cursor !== undefined && cursor.trim() !== ''
+ ? isPersistedSessionCursor(cursor)
+ : await hasActivePersistedSessions(key));
+ const result = usePersistedList
+ ? await listWorkspaceSessionsForResponse(runtime.bridge, key, options)
+ : listLiveWorkspaceSessionsForResponse(runtime.bridge, key, options);I applied this locally and re-ran everything: the real-prompt repro flips to 200, both cursor directions survive, and the suites stay green (multi-workspace-sessions 22/22, server.test.ts 619/619). A self-describing/tagged cursor would be the more durable fix, but this keeps the PR's contract intact.
🟡 Finding 2 — size stops bounding the page once one persisted session exists
Once the store has ≥1 active persisted session, the non-primary route switches to listWorkspaceSessionsForResponse, whose first page appends every live-only session without slicing to pageSize:
store empty → GET ?size=1 → 1 session (+ nextCursor)
plant 1 persisted → GET ?size=1 → 5 sessions (1 persisted + 4 live-only)
To be fair to the PR: I ran the same probe against the primary workspace and it overshoots identically (?size=1 → 3 sessions). So this is inherited from the existing primary merge path, not invented here. But it is a new observable behaviour for /workspace/:id/sessions on non-primary workspaces — the baseline honoured size=1 exactly — and the doc update only mentions the cursor semantics change. Worth either a doc line or a follow-up that slices the merged first page.
🟡 Finding 3 — the probe doubles the store scan
hasActivePersistedSessions() does a full readdirSync + statSync over every session file, and then listWorkspaceSessionsForResponse() immediately repeats the identical scan. With N=2000 persisted sessions, same daemon, same returned page:
| Route | Store scans | Median latency |
|---|---|---|
| primary | 1× listSessions |
17.3 ms |
| trusted non-primary | probe + listSessions |
23.4 ms |
That's +6.1 ms (1.35×, +35%) of pure duplicated I/O per request. Not a blocker, and it's warm-cache so cold would be worse. The Finding 1 fix removes the probe from all cursor requests; skipping it whenever the caller didn't need the live-vs-persisted distinction would remove the rest.
One related note: hasActivePersistedSessions swallows every error and returns false. This is clearly deliberate — there's a falls back to live-only listing when persisted probing fails test that chmod 000s the chats dir. But the consequence is that an unreadable store silently downgrades a trusted non-primary list to live-only with a different cursor format, where the primary route would surface 500 session_list_failed for the same store. Combined with Finding 1, a transient store error is a second way to invalidate a client's in-flight cursor.
Verdict
The feature is correct, the trust gate is airtight, and the guardrails for archived/organized/grouped views hold. Finding 1 is a real client-visible regression reachable by ordinary use of a trusted secondary workspace, and the fix is small and validated. Findings 2 and 3 are fine as follow-ups.
How to reproduce
# two-workspace daemon, isolated QWEN_HOME
node packages/cli/dist/index.js serve --port 0 --token T --hostname 127.0.0.1 \
--no-web --workspace "$WS_A" --workspace "$WS_B"
# 1. two live sessions in the trusted secondary, empty persisted store
curl -H "Authorization: Bearer T" -H 'Content-Type: application/json' \
-d "{\"cwd\":\"$WS_B\"}" $BASE/session
curl -H "Authorization: Bearer T" -H 'Content-Type: application/json' \
-d "{\"cwd\":\"$WS_B\",\"sessionScope\":\"thread\"}" $BASE/session
# 2. page 1 → opaque live cursor
curl -H "Authorization: Bearer T" "$BASE/workspace/$WS_B_ID/sessions?size=1"
# 3. send a prompt → the session logger writes chats/<id>.jsonl
curl -H "Authorization: Bearer T" -H 'Content-Type: application/json' \
-d '{"prompt":[{"type":"text","text":"hi"}]}' $BASE/session/$SID/prompt
# 4. page 2 with the cursor from step 2 → 400 invalid_cursor
curl -H "Authorization: Bearer T" "$BASE/workspace/$WS_B_ID/sessions?size=1&cursor=$CURSOR"Environment: Linux, Node v22.22.2, worktree at 1aecc27, merge-base d8084c6.
🇨🇳 中文版报告
运行时验证报告 —— PR #6558 @ 1aecc27
我用真实的双 workspace qwen serve daemon 做了验证（不是 mock 的 route harness）：node packages/cli/dist/index.js serve --workspace <A> --workspace <B> --no-web，在 workspace B 自己的 chats/ store 里放入 persisted sessions，通过 HTTP 创建 live sessions，并用 fake OpenAI server 支撑那个需要真实 prompt 的探针。下面每条结论都与 merge-base（d8084c6）做了 A/B：只替换 packages/cli/src/serve/routes/session.ts 重新构建，并通过 grep 构建产物 dist 里的新符号来确认当前跑的是哪一侧。
功能符合描述，trust gate 也是可靠的。 但我发现了一个我认为应当在合并前修掉的行为回归，外加两个较小的问题。三者都只影响新的 non-primary 代码路径，primary route 未受影响。
✅ 已确认正常的部分
| 行为 | 结果 |
|---|---|
| 可信 non-primary：列出 active persisted sessions（workspace id 与 encoded cwd 两种 selector） | ✅ 返回 2/2；merge-base 返回 sessions=0 |
live ∪ persisted 合并，按 sessionId 去重 |
✅ 只出现一次，且带上 live 的 clientCount / hasActivePrompt |
non-primary 上的 archiveState=archived、view=organized、view=organized&group=… |
✅ 400 non_primary_session_list_option_not_supported |
不可信 non-primary → 403 untrusted_workspace（id、encoded cwd、复数别名、archived 变体） |
✅ 10/10，且没有泄漏任何 persisted 数据 |
| trust gate 在新的 store 探测之前执行 | ✅ 不可信 workspace 永远不会碰磁盘 |
| 未知绝对 cwd | ✅ 400 workspace_mismatch |
primary route（含 archiveState=archived） |
✅ 行为不变，仍为 200 |
| store 中没有 active session 时的 live-only fallback | ✅ 保留，opaque cursor 仍可翻页 |
packages/cli serve 测试 |
✅ multi-workspace-sessions 22/22，server.test.ts 619/619（合计 641） |
npm run typecheck |
✅ 通过（值得一提：CI 只跑 lint + test:ci，不跑 tsc） |
🔴 问题 1 —— 一个无关的写入会让客户端手上的 cursor 失效（本 PR 新引入）
本 PR 现在要在两个分支之间做选择，而这两个分支产生的 cursor 格式互不兼容：
listWorkspaceSessionsForResponse→ 数值型 mtime cursor，例如"1783569663671.999"listLiveWorkspaceSessionsForResponse→ 不透明 base64url cursor，例如"eyJhY3Rpdml0eVRpbWUiOjE3…"
分支是每次请求根据可变的磁盘状态（await hasActivePersistedSessions(key)）来决定的，但发给客户端的 cursor 是一个无状态 token。因此，如果 persisted store 在第 1 页和第 2 页之间新增（或失去）了它的第一个 active session，daemon 就会拒绝它自己刚刚签发的 cursor：
GET /workspace/:id/sessions?size=1 → 200, nextCursor="eyJhY3Rpdml0eVRpbWUi…"
（该 workspace 中跑了一个 prompt；session logger 写入 chats/<id>.jsonl）
GET /workspace/:id/sessions?size=1&cursor=eyJhY3Rpdml0… → 400 invalid_cursor
Invalid cursor: "eyJhY3Rpdml0eVRpbWUi…" is not a valid numeric cursor
这不需要任何测试脚手架，也不需要手工放置文件 —— 它正是用户打开一个可信 secondary workspace、创建几个 session、列出它们、然后发出第一个 prompt 时会发生的事。我通过对 fake OpenAI server 发起真实的 POST /session/:id/prompt，让 session logger 自然地写出 chats/<id>.jsonl，端到端复现了它。
反方向同样会坏：先拿到数值 cursor，随后最后一个 active persisted session 被归档/删除，第 2 页返回 400 invalid_cursor: "1783569663671.999" is not a valid live cursor。
在 merge-base 上，non-primary route 永远是 live-only，因此 cursor 格式是稳定的，不会发生这种情况。本 PR 自带的两个 cursor 测试（pages active persisted … with numeric cursors、pages live … with a stable cursor）在两次翻页请求之间都保持 store 不变，所以没能覆盖到。
建议的修复 —— 当客户端已经持有 cursor 时不要再去探测 store，让 cursor 自身的形态来决定由哪个分支服务。这同时让所有翻页请求都跳过探测，也顺带缓解了问题 3：
+ const isPersistedSessionCursor = (cursor: string) => {
+ const trimmed = cursor.trim();
+ if (trimmed === '') return false;
+ const parsed = Number(trimmed);
+ return Number.isFinite(parsed) && parsed >= 0;
+ };
- const result =
- runtime.primary || (await hasActivePersistedSessions(key))
- ? await listWorkspaceSessionsForResponse(runtime.bridge, key, options)
- : listLiveWorkspaceSessionsForResponse(runtime.bridge, key, options);
+ const usePersistedList =
+ runtime.primary ||
+ (cursor !== undefined && cursor.trim() !== ''
+ ? isPersistedSessionCursor(cursor)
+ : await hasActivePersistedSessions(key));
+ const result = usePersistedList
+ ? await listWorkspaceSessionsForResponse(runtime.bridge, key, options)
+ : listLiveWorkspaceSessionsForResponse(runtime.bridge, key, options);我在本地应用后重跑了全部验证：真实 prompt 的复现变为 200，两个方向的 cursor 都能存活，测试也保持全绿（multi-workspace-sessions 22/22，server.test.ts 619/619）。更彻底的做法是给 cursor 加上自描述前缀/标签，但上面的改动能在不破坏本 PR 契约的前提下解决问题。
🟡 问题 2 —— 一旦存在 persisted session，size 就不再限制页大小
当 store 中出现 ≥1 个 active persisted session，non-primary route 会切到 listWorkspaceSessionsForResponse，而它的第一页会把所有 live-only session 追加进来，且不按 pageSize 截断：
store 为空 → GET ?size=1 → 1 个 session（并带 nextCursor）
放入 1 个 persisted → GET ?size=1 → 5 个 session（1 persisted + 4 live-only）
公平地说：我对 primary workspace 跑了同样的探针，它也同样超额（?size=1 → 3 个 session）。所以这是从既有 primary merge 路径继承来的，并非本 PR 发明。但对 non-primary 的 /workspace/:id/sessions 而言，这是新出现的可观测行为（baseline 会严格遵守 size=1），而文档更新只提到了 cursor 语义的变化。建议补一句文档，或后续把合并后的首页做截断。
🟡 问题 3 —— 探测使 store 扫描翻倍
hasActivePersistedSessions() 会对每个 session 文件做一次完整的 readdirSync + statSync，紧接着 listWorkspaceSessionsForResponse() 又原样重复同一次扫描。在 N=2000 个 persisted session、同一 daemon、返回同一页的条件下：
| Route | store 扫描次数 | 中位延迟 |
|---|---|---|
| primary | 1× listSessions |
17.3 ms |
| 可信 non-primary | 探测 + listSessions |
23.4 ms |
即每次请求多出 +6.1 ms（1.35×，+35%） 的纯重复 I/O。这不是阻塞项，而且是热缓存下的数字，冷缓存只会更差。问题 1 的修复可以让所有 cursor 请求跳过探测；如果调用方本就不需要区分 live 与 persisted，剩下的部分也可以省掉。
还有一点相关说明：hasActivePersistedSessions 吞掉了所有异常并返回 false。这显然是有意为之 —— PR 里就有一个 falls back to live-only listing when persisted probing fails 测试，通过 chmod 000 把 chats 目录设为不可读。但由此带来的后果是：一个不可读的 store 会静默地把可信 non-primary 的列表降级为 live-only，并且 cursor 格式随之改变；而同样的 store 在 primary route 上会抛出 500 session_list_failed。结合问题 1，store 的瞬时故障就成了让客户端手中 cursor 失效的第二条路径。
结论
功能实现是正确的，trust gate 无懈可击，archived/organized/grouped 视图的护栏也都成立。问题 1 是一个真实、用户可见的回归，在正常使用可信 secondary workspace 时即可触发，且修复很小并已验证。问题 2 和问题 3 可以作为后续跟进。
环境：Linux，Node v22.22.2，worktree 位于 1aecc27，merge-base d8084c6。
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
Collaborator
There was a problem hiding this comment.
Reviewed — no blockers. Downgraded from Approve to Comment: CI failing: Test (ubuntu-latest, Node 22.x).
— qwen3.7-max via Qwen Code /review
Collaborator
Author
|
[codex] Checked the runtime report against current head |
qqqys
approved these changes
Jul 9, 2026
qqqys
left a comment
qqqys
left a comment
Collaborator
There was a problem hiding this comment.
The current head resolves the critical cursor-mode flip: paginated requests now keep the listing path stable based on the cursor format, and the regression test covers persisted sessions appearing between pages. I rechecked the latest diff and current checks are green; no critical blocker found.
wenshao
approved these changes
Jul 9, 2026
Collaborator
Re-verification —
|
| Request | Route | Median |
|---|---|---|
| first page (no cursor) | primary | 14.5 ms |
| first page (no cursor) | trusted non-primary — probe still runs | 22.5 ms (+8.0 ms, 1.55×) |
| cursor page | primary | 13.7 ms |
| cursor page | trusted non-primary — probe skipped | 13.9 ms (+0.3 ms, 1.02×) |
So the duplicated readdir + stat scan now only hits the first page of a non-primary list. Fine to leave as-is.
🟡 Finding 2 — unchanged (and still fine)
?size=1 on a trusted non-primary workspace with 1 persisted + 4 live-only sessions still returns 5 sessions, because listWorkspaceSessionsForResponse appends live-only sessions to the first page unsliced. As before, the primary control overshoots identically, so this is inherited, not introduced here. Doc line or follow-up, your call.
Suite status at 402cda0
| Check | Result |
|---|---|
multi-workspace-sessions.test.ts |
✅ 23/23 |
server.test.ts |
✅ 619/619 |
npm run typecheck |
✅ clean |
| driver (persisted discovery, dedup merge, guardrails, both cursor directions) | ✅ 21/22 — only the inherited Finding 2 row |
| untrusted secondary (id / encoded cwd / plural alias / archived) | ✅ 10/10, no data leaked, trust gate still runs before the probe |
| adversarial cursor sweep | ✅ 29/29 |
LGTM from my side — the regression I reported is genuinely gone, the fix is the narrow one, and nothing else moved.
🇨🇳 中文版
复验 —— 402cda0（codex: stabilize workspace session cursors）
我用之前那份报告里同一套真实 daemon 测试脚手架，对新的 head 重跑了一遍。问题 1 已修复，并在 HTTP 层面得到确认，而且我没能把新的判定逻辑攻破。 问题 3 顺带解决了一半。问题 2 保持原样（仍然是从 primary 继承来的行为）。
在 402cda0 上重新构建，并先确认新代码确实进了 dist（isNumericSessionCursor 出现 2 次），然后才开始跑测试。
✅ 问题 1 —— 已修复
自然复现路径 —— 在可信 secondary 中创建两个 live session，列出第 1 页，然后发送一个真实的 prompt，让 session logger 写出 chats/<id>.jsonl，再用 daemon 刚刚签发的 cursor 请求第 2 页 —— 现在返回 200，而不是 400 invalid_cursor。反方向（先拿到数值 cursor，随后最后一个 active persisted session 被归档/删除）同样返回 200 并给出一个干净的空页，因此正在翻页的客户端会正常结束而不是报错。
你新增的 keeps live cursor pagination stable when persisted sessions appear mid-page 测试覆盖了正方向。反方向（数值 cursor → store 被清空）目前只有我的 driver 覆盖；它是通过的，但如果你想把它钉死，测试套件里还没有。
✅ 新的判定逻辑经得起对抗性输入
isNumericSessionCursor 现在决定了由哪个 parser 去处理一个畸形 cursor，所以我扫了 13 种 cursor 形态 × {primary, 可信 non-primary} × {store 为空, store 非空}。没有任何 5xx，没有静默的全表倾泻，并且 secondary 在每一种畸形输入上都与 primary 表现一致。
三点观察，都不是阻塞项：
0x10、1e3、+5会被当作合法 cursor 接受，返回200加一个空页而不是400，因为isNumericSessionCursor和parseSessionCursor都走Number()。这是parseSessionCursor既有的行为 —— primary route 在main上就是这样 —— 而且现在两者是一致的，这才是关键。把两者都收紧成/^\d+(\.\d+)?$/可以作为独立的清理项。- 在 store 非空的 secondary 上传入一个格式正确的 live cursor，会返回
200 (n=0)而不是报错。这是有意的「快照连续性」语义（继续由签发该 cursor 的分支来服务），客户端会正常结束。知道它是故意的就好。 - 纯空白 cursor 在 non-primary 上会走 live parser（
cursor !== ''没有 trim），而在 primary 上走 numeric parser。两者都返回400 invalid_cursor，只是文案不同。无害；改成cursor.trim() !== ''就完全一致了。
✅ 问题 3 —— 翻页请求不再为探测付费
由于修复在存在 cursor 时短路了 hasActivePersistedSessions()，翻页请求的开销现在和 primary route 一样。在每个 workspace N=2000 个 persisted session 下重新测量：
| 请求 | Route | 中位延迟 |
|---|---|---|
| 首页（无 cursor） | primary | 14.5 ms |
| 首页（无 cursor） | 可信 non-primary —— 仍会探测 | 22.5 ms（+8.0 ms，1.55×） |
| 翻页（带 cursor） | primary | 13.7 ms |
| 翻页（带 cursor） | 可信 non-primary —— 跳过探测 | 13.9 ms（+0.3 ms，1.02×） |
也就是说，重复的 readdir + stat 扫描现在只会打到 non-primary 列表的首页。保持现状即可。
🟡 问题 2 —— 未变（也没关系）
在有 1 个 persisted + 4 个 live-only session 的可信 non-primary workspace 上，?size=1 仍会返回 5 个 session，因为 listWorkspaceSessionsForResponse 会把 live-only session 不截断地追加到首页。和之前一样，primary 对照组同样超额，所以这是继承而来、并非本 PR 引入。补一句文档或后续跟进，由你决定。
402cda0 上的测试状态
| 检查项 | 结果 |
|---|---|
multi-workspace-sessions.test.ts |
✅ 23/23 |
server.test.ts |
✅ 619/619 |
npm run typecheck |
✅ 通过 |
| driver（persisted 发现、去重合并、护栏、两个 cursor 方向） | ✅ 21/22 —— 只剩继承来的问题 2 那一行 |
| 不可信 secondary（id / encoded cwd / 复数别名 / archived） | ✅ 10/10，无数据泄漏，trust gate 仍在探测之前执行 |
| 对抗性 cursor 扫描 | ✅ 29/29 |
我这边 LGTM —— 我报告的回归确实消失了，修复也是最小范围的，其它行为没有变动。
doudouOUC
dismissed
qwen-code-ci-bot’s stale review
July 9, 2026 06:20
Already have 2 approved ,3ks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this PR does
This PR lets trusted non-primary workspace session list routes return active persisted sessions from that workspace and merge matching live session summaries without duplicates. The plural workspace session routes keep the existing selector contract, trust gate, response schema, and live-only fallback behavior when a non-primary workspace has no active persisted sessions.
It also keeps archived, organized, and grouped non-primary list views gated for later phases, updates the multi-workspace design notes and serve protocol documentation, and adds coverage for workspace id selectors, encoded cwd selectors, persisted/live merging, persisted pagination, unsupported options, workspace mismatch, and untrusted workspace rejection.
Why it's needed
Phase 2b already made trusted non-primary persisted load/resume workspace-aware, but users still could not discover active historical sessions in secondary workspaces from the existing workspace session list routes. This closes that discovery gap without exposing archived sessions, organization state, mutation routes, SDK changes, or broader workspace-qualified surfaces.
Reviewer Test Plan
How to verify
Run the targeted CLI serve tests and confirm trusted non-primary workspace session listing includes stored active sessions, merges a matching live summary once, preserves numeric cursor pagination for persisted-backed lists, preserves the existing live-only fallback cursor behavior when no active persisted sessions exist, returns
workspace_mismatchfor unknown absolute cwd selectors, and rejects untrusted or unsupported non-primary list requests.Commands used locally:
cd packages/cli && npx vitest run src/serve/multi-workspace-sessions.test.ts && npx vitest run src/serve/rate-limit.test.ts && npx vitest run src/serve/server.test.ts -t "GET /workspace/:id/sessions"; from repo root:npm run typecheck && npm run build.Evidence (Before & After)
Before: trusted non-primary workspace session listing was live-only, so active persisted sessions in a secondary workspace were not discoverable from the plural workspace session routes. After: trusted non-primary workspace session listing returns active persisted sessions from that workspace, merges matching live state, and still rejects archived or organized non-primary views.
Tested on
Environment (optional)
Local npm workspace on macOS with repository npm scripts.
Risk & Scope
Linked Issues
Related to #6378.
中文说明
这个 PR 做了什么
这个 PR 让可信 non-primary workspace 的 session list route 返回该 workspace 的 active persisted sessions，并把匹配的 live session summary 去重合并进去。plural workspace session routes 保持现有 selector contract、trust gate、response schema，并且在 non-primary workspace 没有 active persisted sessions 时保留原来的 live-only fallback 行为。
同时，archived、organized、grouped non-primary list views 仍然留到后续阶段；本 PR 更新了 multi-workspace 设计说明和 serve protocol 文档，并补充了 workspace id selector、encoded cwd selector、persisted/live merge、persisted pagination、unsupported options、workspace mismatch 和 untrusted workspace rejection 的测试覆盖。
为什么需要
Phase 2b 已经让 trusted non-primary persisted load/resume 支持 workspace-aware，但用户仍然无法通过现有 workspace session list routes 发现 secondary workspace 里的 active 历史会话。这个 PR 补齐 discovery 闭环，同时不暴露 archived sessions、organization state、mutation routes、SDK 改动或更广泛的 workspace-qualified surfaces。
Reviewer Test Plan
如何验证
运行目标 CLI serve tests，确认 trusted non-primary workspace session listing 会包含 stored active sessions，匹配 live summary 时只出现一次，persisted-backed list 保持 numeric cursor pagination，没有 active persisted sessions 时保留现有 live-only fallback cursor 行为，unknown absolute cwd selector 返回
workspace_mismatch，untrusted 或 unsupported non-primary list requests 会被拒绝。本地使用的命令：
cd packages/cli && npx vitest run src/serve/multi-workspace-sessions.test.ts && npx vitest run src/serve/rate-limit.test.ts && npx vitest run src/serve/server.test.ts -t "GET /workspace/:id/sessions"；仓库根目录下：npm run typecheck && npm run build。Evidence (Before & After)
Before：trusted non-primary workspace session listing 是 live-only，因此 secondary workspace 中的 active persisted sessions 无法通过 plural workspace session routes 被发现。After：trusted non-primary workspace session listing 会返回该 workspace 的 active persisted sessions，合并匹配的 live state，并继续拒绝 archived 或 organized non-primary views。
Tested on
Environment (optional)
macOS 本地 npm workspace，使用仓库 npm scripts。
Risk & Scope
Linked Issues
关联 #6378。