feat(serve): expose the background agents the supervisor is running - #10954
feat(serve): expose the background agents the supervisor is running#10954yiliang114 wants to merge 31 commits into
Conversation
`packages/cli/src/agent-view/` is 11,004 production lines with no consumer: the supervisor, the PTY workers and the session lifecycle all merged (#7799, #7800, #7801/#9986), while the two PRs that would give them an entry point are still open (#7802 commands, #7803 roster TUI, both since 2026-07-27). Nothing calls the subsystem, so nothing that subsystem knows can reach a user. This gives it its first one, at the smallest surface that carries real information: `qwen sessions ps` now lists managed sessions beside the interactive ones it always listed. The command walked only the live-process registry, which a supervisor's worker never writes — so a background session waiting for an answer was invisible to every listing. `managed-rows.ts` merges both sources into one row shape, deduplicating by session id (a managed worker can also register, and would otherwise be listed twice: once as `interactive`, once with its real state). Managed rows come first, because a session waiting on input is the usual reason to run this. Three decisions worth naming: - Rows are labelled by task state, not by the roster's display group. The group folds `ready`, `stopped` and `failed` into one `completed` bucket, which the roster UI can afford because it also paints an icon tone; a one-line table has no second channel, and printing "completed" beside a session that failed is a lie the user cannot see through. - The name comes from `deriveAgentViewPresentation`, so this listing and the roster cannot drift into describing one session two ways. Its `Untitled session` placeholder is the single override — the roster can afford identical rows because a user arrows onto one; here the session id is the only thing that tells two apart. The placeholder is now exported rather than duplicated as a literal. - A supervisor store that cannot be read degrades to the registry half and says so on stderr. Silently omitting a session that needs input is the failure this command exists to prevent, and stderr keeps `--json` stdout parseable. The table gains a STATE column; `--json` gains a `managed` discriminator on both row kinds, and carries each registry record through verbatim rather than looking it back up by session id — two records can share one (a stale writer, a restored transcript), which would emit one twice and drop the other.
The find-simplifications survey uses `packages/cli/src/agent-view` as its worked example for "everything says dead, git says five days old": a feature mid-wiring, so drop it silently — do not file it, do not mention it. The deletion verdict was right. The silence was not. Measured again on 2026-09-04, one month after that example was written: 11,004 production lines, still zero external references, up ~8,000 in four weeks. Its five merged PRs shipped the supervisor, the PTY workers and the lifecycle; the two that would give it an entry point have been open since 2026-07-27, the second at +31,699 lines across 104 files and conflicting. The gate kept firing on the directory's recency and the survey kept dropping it, so nothing ever said out loud that a five-figure subsystem had no caller. "Young" is not the fact the gate is reaching for — "still being wired up" is, and for a subsystem the two come apart. The gate now says to date the wiring rather than the code when it fires on a subsystem, and names the outcome: recent commits inside, no external reference, and an open stalled wiring PR is neither a deletion nor a drop. It is a different finding with a different owner, owed to that PR stack.
The Agent View supervisor, its PTY workers and its session lifecycle are all merged, and `dispatchAgentViewSession` records everything a supervisor needs to spawn a session. None of it could run: the entry was missing two wires. The first is why nothing worked end to end. The supervisor spawns itself as `qwen --internal-agent-view-supervisor` (`supervisor-runner.ts:107`) and nothing parsed that flag — the CLI's parser runs `.strict()`, so the process spawned to be a supervisor exited on an unknown argument instead of serving. Every path that dispatches a session waited on a supervisor that could never start. The entry now recognizes the flag and serves. The second is the user-facing half: `qwen --bg "<prompt>"` ensures a supervisor, records a session for it, prints the id and returns. The session shows up in `qwen sessions ps`, which learned to list managed sessions in the parent commit. Both are handled before the argv parser and behind a raw-argv scan, so an ordinary launch pays one `Array.includes` and loads none of it. `--bg` needs a prompt and a directory; routing it through the interactive startup path — auth, theme, extensions — would buy nothing and cost all of it. The flag is still declared in the option tables so `--help` lists it and the strict parser knows it. Two details worth the reviewer's attention: - The prompt is read as the default command's positional query, and the flags that consume the token after them are derived from the CLI's own option tables rather than listed by hand. A hand-written list would go stale the first time someone adds an option, and the cost of missing one is a flag's value silently becoming part of the prompt. - The scan stops at `--`. `qwen -p x -- --bg` passes `--bg` as the user's own data, and hijacking that launch into a dispatch would be a bug with no way for the user to work around it. The internal flag moves to its own module so the entry can recognize it without importing the supervisor runtime on every launch.
`qwen sessions ps` can report that a background session is waiting for
input. Until now that was a dead end: there was no way to read the
question, no way to answer it, and no way to end the session short of
finding its pid and killing it. Surfacing a state the user cannot act on
is worse than not surfacing it, and the previous two commits in this
stack created exactly that gap.
Three subcommands close it, each a thin call to a supervisor operation
that already ships:
qwen sessions peek <session>
qwen sessions answer <session> "<text>"
qwen sessions stop <session>
Each takes a session id or any unique prefix — the supervisor already
resolves prefixes, and refuses an ambiguous one rather than guessing, so
the short id `--bg` prints is enough to type.
Decisions worth reviewing:
- **They connect to a running supervisor and never start one.** Starting
a supervisor to ask it about sessions it cannot have would turn
"nothing is running" into a spawned process and an empty answer. With
none running, all three say so and point at `--bg`.
- **`peek`'s state line comes from `deriveAgentViewPresentation`**, the
same source as the roster and `sessions ps`, so three surfaces cannot
describe one session three different ways.
- **The answer hint is printed only when the session is actually
waiting.** Answering a working session queues a prompt instead, which
is a different operation; offering it there would mislead.
- **Everything the session wrote is sanitized before it reaches the
terminal.** `waitingFor`, `summary` and `lastResult` are a model's own
words relayed from another process — the same untrusted input
`sessions ps` already sanitizes for escape sequences and bidi
overrides.
- **A supervisor error is repeated, not reinterpreted.** It already words
an unknown id, an ambiguous prefix and a session that is not managed;
only the stack is dropped.
The decisions live in `managed-control.ts` with no I/O of their own — the
supervisor connection is injected — so all of the above is tested without
a supervisor, a socket or a filesystem.
The PR describes an invariant it never checked: a background worker is an ordinary interactive session, so it registers itself in the live-process registry under whatever `--session-id` it was spawned with, and `qwen sessions ps` deduplicates the registry against the roster by session id. If the two ever stopped agreeing, every background session would be listed twice — once as `interactive`, once with its real state — and nothing would have failed. Verified while tracing that path: `--session-id` does become the session's own id (`config.ts:2031-2066`), so the invariant holds today. This pins the seam that would silently break it.
The daemon has its own idea of what a session is, and it is not the one a background agent lives in: `standalone-session-service` tracks conversations the daemon itself hosts, in its own memory, while a session started with `qwen --bg` is owned by the Agent View supervisor. Nothing in the daemon reads the supervisor's roster — `rg 'listLiveSessions|session-registry' packages/cli/src/serve` is empty across 94,136 lines — so every daemon client, the Web Shell included, is blind to background agents. `GET /background-agents` reports them. The rows come from `managedSessionRows`, the same function `qwen sessions ps` renders, so the CLI and anything built on this route cannot describe one session two different ways — including the part that is easy to get wrong: a failed session is labelled `failed`, not folded into the roster's `completed` display group, because a caller with no icon tone has nothing else to carry the difference. Read-only by design. Answering or stopping a background agent goes through the supervisor's own socket, which the CLI already does; routing those through the daemon would put a second writer on state the supervisor owns. An unreadable store is a 503, not an empty list: a client that cannot tell "no agents" from "cannot look" would show an empty page to someone whose agent is waiting for an answer. This is the first half of the surface described in docs/plans/2026-09-04-background-agent-surfaces.md §3. The second half — a Web Shell panel that renders these rows and answers a waiting agent — is not in this PR.
|
Thanks for the PR — this is the cleanest-written description in the stack, and the two decisions you flagged for review are the right two to flag. Template looks good ✓ — every required heading is present, including the Chinese section. Problem: this is a
Direction: aligned. A read-only daemon surface for supervisor-owned sessions is a sensible half of the split, and declining to write the Web Shell panel blind rather than shipping something a maintainer has to rewrite was the right judgement — saying so out loud in the description is appreciated. One genuine scope question, raised as a question and not a block:
Size: not applicable. 92 production lines ( Approach: minimal and well-targeted — one new file, one import, one registration, no drive-by edits, no unrelated churn. The reuse of Risk: no Stage 1e high-risk path match. But there is a CI blind spot worth naming now, because the description leans on it:
CI covers neither. Moving on to code review. 🔍 中文说明感谢贡献 —— 这是整个 stack 里描述写得最清楚的一篇，你主动标出来待评审的两个判断，也正是该标出来的两个。 模板完整 ✓ —— 所有必需小标题都在，含中文部分。 问题： 这是一个
方向： 对齐。为 supervisor 拥有的 session 提供一个只读的 daemon 表面，是这个拆分中合理的一半；而没有盲写 Web Shell 面板、宁可交给看得见它的人，是正确的判断 —— 在描述里明说这一点也值得肯定。有一个真实的范围问题，以提问方式提出，不作为阻塞：
规模： 不适用。92 行生产代码（ 方案： 精简且目标明确 —— 一个新文件、一处 import、一次注册，没有顺手改动，没有无关噪音。复用 风险： 未命中 Stage 1e 高风险路径。但有一个 CI 盲区值得现在就点明，因为描述依赖它：
CI 两者都不覆盖。 进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Code reviewThe route itself is well built — small, pure projection, no new state, and the reuse of 1. The untrusted-workspace guard never fires in production (blocking)
if (deps.isWorkspaceTrusted?.() === false) {
deps.sendUntrustedWorkspaceResponse?.(res);
return;
}and registerBackgroundAgentRoutes(app, {});So There is no global gate that would make the per-route dep redundant. The only Failure scenario: The PR body says this route makes "no authentication decision … beyond honouring the untrusted-workspace hook the other routes use." As wired it does not honour it — it never receives it. Why the suite doesn't catch it: the untrusted test injects both deps itself, so it pins a switch no production caller ever sets — green, and proving nothing about the shipped daemon. It also asserts 2. The responder is optional, so fixing #1 the obvious way hangs the request (blocking)If you wire only the trust dep — Both findings have one fix: drop 3. "An unreadable store is a 503" holds for one of several unreadable storesThis is the second decision you asked for review on, so here's the precise reach.
Per-session The test can't see this because it stubs the entire lister ( Smaller things
Verified, and correctWorth recording what I checked and found sound, since none of it is covered by CI here:
Test evidenceThis run carried no test execution of its own — the review is static, and per the gate's rules I did not build or run any PR-derived code. The evidence below is the PR's own CI, read through the API on The headline is what is absent: Not verified, with reasons:
No check on Sandboxed verification would settle this: 中文说明代码审查路由本身写得不错 —— 小、纯投影、不引入新状态，复用 1. 生产环境中「不受信任工作区」守卫永远不会触发（阻塞）
不存在能让这个 per-route dep 变得多余的全局门禁： 失败场景： 在不受信任的主工作区上运行 PR 描述称本路由「除沿用其他路由使用的 untrusted-workspace 钩子外，不做任何鉴权决定」。按实际接线，它并没有沿用 —— 它根本没有收到这个钩子。 为什么测试没发现： 那个不受信任的用例自己注入了两个 dep，因此它钉住的是一个生产调用方从不设置的开关 —— 绿色，但对实际 daemon 什么也没证明。它断言的是 2. 响应器是可选的，因此按显而易见的方式修 #1 会让请求挂住（阻塞）如果只接上信任 dep，守卫会触发， 两个问题是同一个修法：从 3.「存储读不出来时返回 503」只在若干种「读不出来」中的一种成立这是你要求评审的第二个判断，所以给出精确的覆盖范围。 测试看不到这一点，因为它 stub 了整个 lister（ 更小的问题
已核实且正确的部分由于这里没有任何 CI 覆盖，值得记录我核对过并确认无误的部分：
测试证据本次运行没有执行任何测试 —— 审查是静态的，且按门禁规则我未构建或运行任何源自本 PR 的代码。下面的证据来自本 PR 自己的 CI，通过 API 在 要点在于缺失的部分： 未验证项及原因：
CI 结果表见上方英文区（此处不重复）。 沙箱验证可以定案： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 2/5 — the route is good work and the fix is about four lines, but a trust guard that is inert in production and has a green test asserting it is the one shape I can't wave through. Stepping back: my own independent proposal for this — before I read the diff — was a read-only What I'd thank you for in six months: What I'd curse: the dead guard. Not because the omission is subtle — Am I being a pushover here? I don't think so. Both blockers are mechanical, both are evidenced against sibling code in the same file at the same commit, and neither requires any judgement call about intent. The Two things I want to be fair about, because they cut the other way:
One line worth saying once for the whole stack rather than four times: none of #10942 / #10943 / #10949 / #10954 gets lint, typecheck or unit-test CI until it retargets to Requesting changes on findings 1 and 2. Both are the same edit: import Happy to re-run and approve once the guard is wired — the route underneath it is ready. 中文说明信心度：2/5 —— 路由本身是不错的工作，修复大约四行；但一个在生产环境中失效、同时还有一个绿色测试为其背书的信任守卫，是我无法放行的那种形态。 退一步看：在读 diff 之前，我自己的独立方案是 —— 在 六个月后我会感谢你的地方：以 我会抱怨的地方：那个失效的守卫。不是因为遗漏很隐蔽 —— 我在这里是在当软柿子吗？我不认为。两个阻塞项都是机械性的，都能在同一 commit 的同类代码上找到依据，且都不需要对意图做任何判断。与 有两点我要公平地说，因为它们指向另一面：
有一句话值得对整个 stack 说一次，而不是说四次：#10942 / #10943 / #10949 / #10954 中任何一个，在重新以 基于发现 1 与 2 请求修改。两者是同一处编辑：从 守卫接好之后，我很乐意重跑并批准 —— 它下面的路由已经准备好了。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@yiliang114 — requesting changes on the two blocking findings from my review above. Both are the same edit at the same call site.
1. The untrusted-workspace guard is inert as wired. server.ts:2241 registers the route with {}, so deps.isWorkspaceTrusted?.() === false evaluates undefined === false and never fires. Every sibling read-only listing in this file receives isWorkspaceTrusted: isPrimaryWorkspaceTrusted — registerGoalsRoutes at server.ts:3091 is the closest analogue. There is no global trust middleware that would make the per-route dep redundant. Net effect: on an untrusted primary workspace, GET /goals returns 403 while GET /background-agents returns 200 with the user-global roster — other projects' absolute cwd, and a name that deriveTitle fills from launch.initialPrompt when there's no display name or activity summary.
2. The responder is an optional dep, so the obvious fix for #1 hangs the request. Wiring only isWorkspaceTrusted makes the handler call deps.sendUntrustedWorkspaceResponse?.(res) (a no-op) and return without ending the response — Express never replies. goals.ts can neither fail open nor hang because it imports the helper.
The fix, for both: import sendUntrustedWorkspaceResponse from ../workspace-route-runtime.js, drop it from RegisterBackgroundAgentRoutesDeps, and pass isWorkspaceTrusted: isPrimaryWorkspaceTrusted at the registration. Then let the untrusted test assert the real 403 body ({ error: 'Workspace is not trusted.', code: 'untrusted_workspace' }) instead of stubbing the responder with { error: 'untrusted' } — today it pins a switch no production caller sets, and a body the real helper never produces.
Findings 3–6 (the narrower real reach of the 503, the dangling docs/plans/2026-09-04-background-agent-surfaces.md reference in committed code, state: string widening SessionRowState, the incomplete name JSDoc) are non-blocking — take or leave, though #3 is a claim the description asks reviewers to endorse.
The route underneath all this is good work and the reuse of managedSessionRows is the right seam. Happy to re-run and approve once the guard is wired.
Note for the record: Qwen Code CI does not run on this PR (ci.yml triggers pull_request on main / release/** only, and the base here is feat/agent-view-session-control), so the lint, typecheck and unit-test results in the description are unconfirmed by automation. This review was static; no PR-derived code was built or executed.
中文说明
@yiliang114 —— 基于上方审查中的两条阻塞发现请求修改。两者是同一调用点上的同一处编辑。
1. 不受信任工作区守卫按现状接线是失效的。 server.ts:2241 用 {} 注册该路由，因此 deps.isWorkspaceTrusted?.() === false 实际求值为 undefined === false，永不触发。本文件中每一处同类只读列举都接收 isWorkspaceTrusted: isPrimaryWorkspaceTrusted —— server.ts:3091 的 registerGoalsRoutes 是最接近的对照。不存在能让这个 per-route dep 变得多余的全局信任中间件。最终效果是：在不受信任的主工作区上，GET /goals 返回 403，而 GET /background-agents 返回 200 并给出用户全局 roster —— 其他项目的绝对 cwd，以及一个 name（当既无显示名也无活动摘要时，deriveTitle 会用 launch.initialPrompt 填充它）。
2. 响应器是可选 dep，因此 #1 的显而易见修法会让请求挂住。 只接上 isWorkspaceTrusted 会让 handler 调用 deps.sendUntrustedWorkspaceResponse?.(res)（空操作）然后 return，没有结束响应 —— Express 永不回复。goals.ts 既不会失效开放也不会挂住，因为它是 import 这个 helper。
两者的修法： 从 ../workspace-route-runtime.js 导入 sendUntrustedWorkspaceResponse，从 RegisterBackgroundAgentRoutesDeps 中去掉它，并在注册处传入 isWorkspaceTrusted: isPrimaryWorkspaceTrusted。然后让不受信任的测试断言真实的 403 响应体（{ error: 'Workspace is not trusted.', code: 'untrusted_workspace' }），而不是用 { error: 'untrusted' } stub 响应器 —— 现状是它钉住了一个生产调用方从不设置的开关，以及一个真实 helper 永不产生的响应体。
发现 3–6（503 的真实覆盖范围更窄、已提交代码中悬空的 docs/plans/2026-09-04-background-agent-surfaces.md 引用、state: string 放宽了 SessionRowState、name 的 JSDoc 不完整）不阻塞 —— 取舍由你，不过 #3 是描述请评审者背书的一个论断。
这一切之下的路由是不错的工作，复用 managedSessionRows 是正确的切分点。守卫接好之后，我很乐意重跑并批准。
留档说明：Qwen Code CI 不在本 PR 上运行（ci.yml 的 pull_request 只针对 main / release/** 触发，而此处 base 是 feat/agent-view-session-control），因此描述中的 lint、typecheck 与单测结果未获自动化确认。本次审查为静态审查；未构建或执行任何源自本 PR 的代码。
— Qwen Code · qwen3.8-max-2026-09-02
Reviewed at 85c5e36aa88a729edd905a98b21dde5be9b5e2bc
| | ---------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | ||
| | `serve/fs/` | `WorkspaceFileSystem` factory plus `policy.ts` (size/trust/binary checks), `paths.ts` (canonicalize, resolveWithin, symlink rejection), `audit.ts`, and typed `FsError` values. | | ||
| | `serve/routes/workspace-file-read.ts`, `workspace-file-write.ts` | HTTP handlers for `GET /file`, `GET /file/bytes`, `POST /file/write`, and `POST /file/edit`. | | ||
| | `serve/routes/background-agents.ts` | `GET /background-agents` — the sessions the Agent View supervisor is running, read from the roster rather than from the daemon's own session model. Read-only: acting on one goes through the supervisor's socket, which the CLI does. | |
There was a problem hiding this comment.
[Suggestion] R1-4: This row says the listing is "read from the roster", but roster membership neither includes nor excludes rows: the code readdirs the supervisor's session-state files (the jobs directory via listAgentViewSessionStates) and only joins roster entries for display titles. A session whose roster entry was removed — the adoption-failure rollback (supervisor-process.ts:789) or disconnected-adoption path (supervisor-process.ts:2964) — is still listed here; a maintainer following this doc would delete a roster entry to take a stale session off the surface and see it persist, and a client reconciling route rows against roster contents sees a mismatch where the code behaves as designed.
Witness:
probe against the real store through the production wiring:
X (state file, no roster entry — the exact post-rollback state) listed: true
Y (state + roster) listed: true
Z (roster entry, no state file) listed: false
| | `serve/routes/background-agents.ts` | `GET /background-agents` — the sessions the Agent View supervisor is running, read from the roster rather than from the daemon's own session model. Read-only: acting on one goes through the supervisor's socket, which the CLI does. | | |
| | `serve/routes/background-agents.ts` | `GET /background-agents` — the sessions known to the Agent View supervisor, including stopped and failed ones, read from the supervisor's session store (the roster supplies display names only) rather than from the daemon's own session model. Read-only: acting on one goes through the supervisor's socket, which the CLI does. | |
中文说明
该行写「从 roster 读取」，但 roster 成员关系既不会包含、也不会排除任何行：代码实际是 readdir supervisor 的会话状态文件（经 listAgentViewSessionStates 读取 jobs 目录），仅连接（join）roster 条目以取得显示标题。一个被删除了 roster 条目的会话 —— 接管失败回滚（supervisor-process.ts:789）或断连接管路径（supervisor-process.ts:2964）—— 仍会出现在此列表中；照此文档操作的维护者删除 roster 条目、想把过期会话从该表面移除时，会发现它依然存在；将路由返回行与 roster 内容对账的客户端，会在代码按设计运行时看到不一致。
证据：通过生产接线对真实存储的探针显示——X（有状态文件、无 roster 条目，即回滚后的确切状态）被列出：true；Y（状态 + roster）被列出：true；Z（仅有 roster 条目、无状态文件）被列出：false。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Deferred for this round - the fix budget went to the Critical gate wiring (R1-1/R1-6), the 503 envelope (R1-2) and the guard coverage (R1-5). The proposed rewording is accepted: the listing is read from the supervisor's session store, the roster supplies display names only. Planned as a follow-up on this branch; leaving this thread open until it lands.
| /** `working`, `needs input`, `ready`, `stopped` or `failed`. */ | ||
| state: string; |
There was a problem hiding this comment.
[Suggestion] R1-3: The state contract is carried only in this prose enumeration over a bare string, although the closed label set already exists as SessionRowState in managed-rows.ts and this route only projects managedSessionRows output. Adding a value to AgentViewTaskState forces a TASK_STATE update (exhaustive Record), which compiles and immediately flows new labels into this route's responses, while state: string and the five-label comment silently drift — clients switching on the documented labels meet a value they cannot match, and no type check anywhere can catch it because the contract lives in a comment.
Witness:
witness: not run — closest capability was a mutant probe adding a sixth AgentViewTaskState and running tsc; unnecessary because the Record<AgentViewTaskState, SessionRowState> exhaustiveness (managed-rows.ts:79-85) and the bare state: string declaration here are compile-time facts read directly off the definitions.
| /** `working`, `needs input`, `ready`, `stopped` or `failed`. */ | |
| state: string; | |
| /** Task state label produced by `managedSessionRows`. */ | |
| state: Exclude<SessionRowState, 'interactive'>; |
(SessionRowState comes from ../../commands/sessions/managed-rows.js — extend the existing import.)
Fix constraint: 'interactive' is assigned only by registryRow (managed-rows.ts:140), reached only via mergeSessionRows, which this route never calls — the Exclude must not be widened if registry rows are ever mixed in.
中文说明
状态契约仅由这段文字枚举承载、类型却是裸 string，而闭合的标签集合在 managed-rows.ts 中已以 SessionRowState 存在，且本路由只是投影 managedSessionRows 的输出。给 AgentViewTaskState 新增值会强制更新 TASK_STATE（穷举 Record），编译通过后新标签立即流入本路由的响应，而 state: string 与五值注释悄悄漂移 —— 按文档标签做 switch 的客户端会遇到无法匹配的值，且由于契约存在于注释中，任何类型检查都无法捕获。
证据：未运行 —— 最接近的能力是添加第六个 AgentViewTaskState 并运行 tsc 的突变探针；无需运行，因为 Record<AgentViewTaskState, SessionRowState> 的穷举性（managed-rows.ts:79-85）与此处裸 state: string 声明都是可直接从定义读出的编译期事实。
修复：用既有联合类型为字段定型 —— Exclude<SessionRowState, 'interactive'>，并扩展现有 import 引入 SessionRowState；文档注释改为指向类型而非重新枚举。
修复约束：'interactive' 仅由 registryRow（managed-rows.ts:140）赋值、且只经由 mergeSessionRows 到达，而本路由从不调用它 —— 若将来混入 registry 行，不得放宽该 Exclude。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Deferred for this round - the fix budget went to the Critical gate wiring (R1-1/R1-6), the 503 envelope (R1-2) and the guard coverage (R1-5). The proposed Exclude<SessionRowState, 'interactive'> typing (imported from managed-rows.js, without widening the Exclude if registry rows are ever mixed in) is accepted and planned as a follow-up on this branch; leaving this thread open until it lands.
The registration passed `{}`, so `deps.isWorkspaceTrusted?.() === false`
never fired: an untrusted primary workspace got 403 from every sibling
route but a full 200 listing here — sessionIds, names, cwds and pids.
Wire the same `isPrimaryWorkspaceTrusted` closure the sibling routes
use, and answer the refusal with `sendUntrustedWorkspaceResponse`
hard-imported from `workspace-route-runtime.js`, as every other route
does: the injected `(res: unknown) => void` responder dep could not
take the canonical helper under `strictFunctionTypes` in the first
place, and it was the only such injected dep across these routes.
A server-level test now builds `createServeApp` with an authoritative
untrusted primary workspace and expects 403 `untrusted_workspace`;
with the deps dropped from the registration it answers 200 and goes
red. The route-level refusal test registers the predicate alone and
asserts the canonical envelope.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
The machine key sat under `error` and the human detail under `message` — the only such shape among the daemon's 503s. A client classifying failures by `body.code` got `undefined` from exactly this route. Put the human message under `error` and the machine key under `code`, as 36 of the 47 sibling serve 503 sites do, and keep the raw error under `message`. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
`pid` and `startedAt` come from independent sources in `managedSessionRows`, so a store can spoil one without the other. The existing tests only covered both present and both absent, which let a condition-swap between the two omission guards survive; add the mixed case — a live worker with an unusable stamp keeps `pid` and drops `startedAt` — and verify it goes red under the swap. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
managedSessionRows mapped every snapshot the store returns, but the store also holds unmanaged tombstones, mid-removal snapshots and mid-adoption snapshots; an adopting snapshot reuses the id of a live registered session, so the merge replaced a registry row that knows a live pid with a pid-less ghost. Filter to ownership === 'managed', the shape the supervisor's own listing uses. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
The supervisor store is durable and nothing reaps it when no supervisor runs, so after a crash or a reboot a managed row could carry a pid that is dead or recycled to an unrelated process, and acting on it would signal the wrong target. Check both recorded pids with core's isPidAlive (worker first, then the PTY host, matching the supervisor's own idiom) and print `-` when neither lives. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
The reason text is foreign input, and a preserved LF would forge extra lines out of the single stderr note the sibling test pins. Feed a message carrying all four classes and assert none survive, so a downgrade to plain sanitizeTerminalText (which keeps LF and TAB) fails. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
The .name example prints session-generated text with jq -r; like the adjacent .cwd example, it must say that the value is rendered raw and to sanitize it when untrusted. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
Both intercepts ran before resolveBootstrapRoute and before the external Guard token scrub. Move them below both: only the default route reaches them (version keeps its base-parity win, subcommand launches fall through to the parser instead of becoming prompts), and the serve-only credential is deleted before either intercept can spawn the supervisor that would inherit it. The supervisor scan also stops at `--` now, matching the `--bg` scan and the file's other flag scans, so a supervisor flag passed as a prompt word can no longer hijack the launch into serving the shared socket. Adds entry-level runCliEntry tests for both intercepts; each goes red when its intercept block (or the scrub ordering, or the `--` cut) is removed. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
readBackgroundPrompt skipped any other flag and tried to model which flags take values, but the derivation was weaker than the parser it mirrored: array-typed options consume N tokens while the scan consumed one, inline-registered options like --sandbox-session-id sit in no option table, and nothing stopped a value flag from swallowing a flag-shaped next token. Each shape silently dispatched a wrong prompt to an unattended session. Since --bg forwards no flags to the worker anyway, decline the launch and name the flag instead. The error paths now write through the house writeStderrLine, the test captures it (the old no-op mock would have silently broken the stderr assertions on any switch), the word-join test asserts both joins instead of comma-operator-no-op'ing the first, and the reason-not-a-stack test pins the exact one-line output so a switch to error.stack goes red. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
sanitizeTerminalText deliberately preserves TAB and LF for multi-line render sites, so clean()'s CR-only strip was a no-op that let session-written text (a model's own waitingFor/summary/lastResult) forge continuation lines in peek's one-line labelled output — e.g. a fake `Answer it with:` hint at column 0. Drop TAB and LF at this one-line call site, exactly like the sibling renderer ps.ts does, and pin the LF case in the sanitization test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
Its closest siblings — the other prompt-carrying launch flags --prompt and --prompt-interactive — both have rows there, and the table is the reference for every way to hand the CLI a prompt at launch. Note the experimental status and the `qwen sessions ps` companion. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
peek derived its title from {state, activity} only: the supervisor's
peek response carried no roster entry or launch record, so deriveTitle
fell through to the untitled placeholder while `sessions ps` named the
same session — a renamed session or one whose worker summary is a
generic filtered phrase. Carry rosterEntry and launch through the
supervisor's peek response (the store reads listAgentViewSessionSnapshots
already does, launch redacted like the snapshot join) and feed them into
deriveAgentViewPresentation without reordering its precedence chain.
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
deriveTitle falls back to the activity summary when neither a roster name nor a launch prompt names the session, so a session adopted with 'Backgrounded from native session' showed that phrase as the title line and again as Doing: two lines later. Skip the Doing: line when it would repeat the printed title; a summary distinct from the title still gets its line. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
peek/answer/stop wrote failure output (exit code 1) to stdout, while list, ps and --bg all route errors through writeStderrLine: an answer against a stale id sent 'No Agent View session found for ...' into the success channel of `qwen sessions answer "$id" "$reply" > last.log`. Send failure lines to stderr and keep success lines on stdout, and add a control-commands test file driving the real handlers to pin the stream choice and the exit-code propagation. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtlz4ubdgq
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed. Suggestions are inline.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R1-4 docs row misstates the listing's data source — still standing from round 1, already reported (comment 3927603487)
- dangling docs/plans/2026-09-04-background-agent-surfaces.md reference in the module comment — already reported (triage stage-2 comment 5529335539 and stage-3 review 5104767493)
- name JSDoc omitting the session-id fallback as fourth source — already reported (triage stage-2 comment 5529335539)
Not explored to full depth (tool budget reached): "agent 6a": none — no check was cut short..
Not reviewed: reverse audit — stopped before round 5 by the review time budget.
中文说明
仅完成部分审查，审查缺口已披露。 建议见行内评论。
本轮确认的 3 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未探索到全部深度（达到工具调用预算）："agent 6a"：none — no check was cut short.。
未审查：反向审计——评审时间预算不足，未能开始第 5 轮。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| registerBackgroundAgentRoutes(app, { | ||
| isWorkspaceTrusted: isPrimaryWorkspaceTrusted, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R2-1: No server-level test exercises the trusted positive path through createServeApp for GET /background-agents, so the default listSnapshots fallback and the trusted side of the wired predicate are never executed. Every unit test injects listSnapshots, and the sole server-level test is refused at the trust gate before the store is read — so one-line mutations survive the whole suite green: wiring isWorkspaceTrusted: () => false gives trusted workspaces a permanent 403 on this route, and replacing the ?? listAgentViewSessionSnapshots default with a throwing function gives them a permanent 503.
Witness:
probe (scratch tree, QWEN_HOME isolated):
intact default-reader request → 200 with agents array (1 passed)
mutant ?? (async () => { throw … }) → probe "expected 503 to be 200" while existing suite 7 passed
mutant isWorkspaceTrusted: () => false → server test still "1 passed | 1176 skipped"
Add a trusted-path test beside the refusal test in server.test.ts: build createServeApp(tokenOpts, undefined, { bridge: fakeBridge(), primaryWorkspaceTrusted: true }), then the authenticated GET /background-agents asserting status 200 and Array.isArray(res.body.agents). The default reader touches the test machine's real supervisor store — ENOENT on the jobs dir returns [] (packages/cli/src/agent-view/supervisor-store.ts:341) but a machine with real sessions returns them — so assert status and shape only, not listing content. The new test is its own witness: it goes red if the trusted wiring breaks (constant-false predicate → 403, throwing default store → 503, registration removed → 404) — please remove the predicate from this registration, run it, and confirm it reds.
中文说明
没有服务端级测试经过 createServeApp 走 GET /background-agents 的受信任正向路径，因此默认的 listSnapshots 回退与已接线谓词的受信任一侧从未被执行。所有单测都自行注入 listSnapshots，唯一的服务端级测试在信任门禁处就被拒绝、到不了存储读取 —— 于是单行变异可以在整套测试保持绿色的情况下存活：把接线改成 isWorkspaceTrusted: () => false 会让受信任工作区在该路由上永久 403；把 ?? listAgentViewSessionSnapshots 默认值换成一个抛错函数则会让它永久 503。
证据：探针（scratch tree，QWEN_HOME 隔离）——完整代码下走默认读取器的请求 → 200 且返回 agents 数组（1 passed）；变异体 ?? (async () => { throw … }) → 探针 expected 503 to be 200，而既有套件仍 7 passed；变异体 isWorkspaceTrusted: () => false → 服务端测试仍 1 passed | 1176 skipped。
请在 server.test.ts 的拒绝用例旁补一个受信任路径测试：以 createServeApp(tokenOpts, undefined, { bridge: fakeBridge(), primaryWorkspaceTrusted: true }) 构建，随后带鉴权请求 GET /background-agents，断言状态 200 且 Array.isArray(res.body.agents)。默认读取器会触碰测试机真实的 supervisor 存储 —— jobs 目录 ENOENT 时返回 []（packages/cli/src/agent-view/supervisor-store.ts:341），但本机若真有 session 则会返回它们 —— 因此只断言状态与形状，不断言列表内容。新测试自身即验收见证：受信任接线被破坏时它会变红（谓词恒 false → 403、默认存储抛错 → 503、注册被移除 → 404）—— 请移除本注册处的谓词后运行确认其变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| function appWith(listSnapshots: () => Promise<AgentViewSessionSnapshot[]>) { | ||
| const app = express(); | ||
| registerBackgroundAgentRoutes(app, { | ||
| listSnapshots: listSnapshots as never, |
There was a problem hiding this comment.
[Suggestion] R2-2: The injected listSnapshots mock is cast as never here and again in the untrusted-workspace test below, erasing the compile-time check against the dep's declared contract typeof listAgentViewSessionSnapshots — and the cast is unnecessary. If listAgentViewSessionSnapshots's signature drifts (a required StoreOptions argument, or a changed snapshot return type), these unit tests still compile and pass because the cast hides the mismatch, while the production wiring — server.ts registers the route with no listSnapshots override, so the route invokes the real default store — breaks, and the test seam can no longer be caught disagreeing with the declared contract.
Witness:
probe (scratch tree):
both casts removed → tsc --noEmit exit 0; vitest 7 passed (casts unnecessary)
simulated drift, no cast → TS2322 at the injection line (exit 2)
identical drift with `as never` → tsc exit 0
| listSnapshots: listSnapshots as never, | |
| listSnapshots, |
Delete the second cast in the untrusted-workspace test the same way (listSnapshots: async () => [snapshot()] type-checks directly against the dep type).
中文说明
注入的 listSnapshots mock 在此处被转换为 as never，下方不受信任工作区的测试里也有同样一处，抹掉了针对 dep 声明契约 typeof listAgentViewSessionSnapshots 的编译期检查 —— 而该转换并无必要。若 listAgentViewSessionSnapshots 的签名漂移（新增必填的 StoreOptions 参数，或快照返回类型变化），这些单测仍能编译并通过，因为转换掩盖了不匹配；而生产接线 —— server.ts 注册该路由时未覆盖 listSnapshots，路由会调用真实的默认存储 —— 将会损坏，且测试接缝与声明契约的不一致将再也无法被发现。
证据：探针（scratch tree）——删除两处转换 → tsc --noEmit exit 0、vitest 7 passed（转换本不必要）；模拟签名漂移且不加转换 → 注入行报 TS2322（exit 2）；同样的漂移加 as never → tsc exit 0。
请同样删除不受信任工作区测试中的第二处转换（listSnapshots: async () => [snapshot()] 可直接通过 dep 类型检查；修法见上方英文 suggestion 块）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| registerBackgroundAgentRoutes(app, { | ||
| listSnapshots: (async () => [snapshot()]) as never, | ||
| isWorkspaceTrusted: () => false, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R2-3: This untrusted-workspace test injects a listSnapshots mock but never asserts it is not invoked, so the gate's ordering before the store read is unpinned. A refactor relocating the trust check below the store read passes the entire suite, while in production an untrusted daemon now reads the user's agent-view store before refusing — and when that store is unreadable it answers 503 background_agents_unavailable instead of 403 untrusted_workspace, so clients classifying by body.code misread a trust refusal as a store outage.
Witness:
probe (scratch tree):
INTACT → status 403, code "untrusted_workspace", listSnapshotsCalls 0
MUTANT (gate after store read) → status 503, code "background_agents_unavailable", listSnapshotsCalls 1
(all 7 existing route tests and the server-level test still green)
Make the injected mock a spy and pin the ordering: const listSnapshots = vi.fn(async () => [snapshot()]); injected here, then expect(listSnapshots).not.toHaveBeenCalled(); after the 403 assertions. That assertion is the witness — move the gate below the store read and the spy is called, so the test goes red; please apply that mutation and confirm it.
中文说明
这个不受信任工作区的测试注入了 listSnapshots mock，却从未断言它未被调用，因此「门禁先于存储读取」的顺序没有被钉住。若重构把信任检查挪到存储读取之后，整套测试依然通过；而生产中，不受信任的 daemon 会先读取用户的 agent-view 存储再拒绝 —— 当该存储不可读时，它会返回 503 background_agents_unavailable 而非 403 untrusted_workspace，按 body.code 分类的客户端会把一次信任拒绝误判为存储故障。
证据：探针（scratch tree）——完整代码 → 状态 403、code untrusted_workspace、listSnapshotsCalls 0；变异体（门禁移到存储读取之后）→ 状态 503、code background_agents_unavailable、listSnapshotsCalls 1（既有 7 个路由测试与服务端级测试仍全部为绿）。
请把注入的 mock 改成 spy 以钉住顺序：此处注入 const listSnapshots = vi.fn(async () => [snapshot()]);，然后在 403 断言之后加 expect(listSnapshots).not.toHaveBeenCalled();。该断言即验收见证 —— 把门禁移到存储读取之后，spy 就会被调用，测试随之变红；请施加该变异并确认。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| /** `working`, `needs input`, `ready`, `stopped` or `failed`. */ | ||
| state: string; |
There was a problem hiding this comment.
[Suggestion] R1-3: Still standing from round 1 (deferred then, with the proposed fix accepted for a later round): the state contract is carried only in this prose enumeration over a bare string, although the closed label set already exists as SessionRowState in managed-rows.ts and this route only projects managedSessionRows output. A state label added to or renamed in SessionRowState ships without a compile error here; HTTP clients classifying by the documented five labels silently receive a sixth. The description states "the field names are now a contract" — a prose enumeration over string cannot enforce it.
Witness:
not run — carried-over finding confirmed in round 1; the nearest capability (a type-level probe) is not re-run because the budget tail forbids re-verifying prior-round confirmations; still-stands re-ruled by direct read at HEAD (background-agents.ts:38 still declares state: string)
Type the field as the reachable set, importing the union from managed-rows.js:
import type { SessionRowState } from '../../commands/sessions/managed-rows.js';
/** `working`, `needs input`, `ready`, `stopped` or `failed`. */
state: Exclude<SessionRowState, 'interactive'>;中文说明
第 1 轮遗留、至今仍成立（当时作者推迟处理，并认可所提修法留待后续轮次）：state 的契约仅由这段基于裸 string 的文字枚举承载，而闭合的标签集合在 managed-rows.ts 中已以 SessionRowState 存在，且本路由只投影 managedSessionRows 的输出。若 SessionRowState 新增或重命名某个状态标签，此处不会报任何编译错误；按文档中五个标签分类的 HTTP 客户端会悄无声息地收到第六种。描述中写道「字段名从此成为契约」—— 基于 string 的文字枚举无法强制这一契约。
证据：未运行 —— 承接自第 1 轮的已确认发现；最近的手段（类型级探针）未重跑，因为预算尾段禁止对前轮已确认项重新验证；经 HEAD 直接阅读重新判定为仍然成立（background-agents.ts:38 仍声明 state: string）。
请把该字段类型改为可达集合，并从 managed-rows.js 导入该联合类型（代码见上方英文块）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
Review finding 2 on this PR: `SessionRow.state` was documented as "what the STATE column can say" and went straight into `--json`, so the machine contract was pinned to display copy. `'needs input'` carries a space, and the documented recipe was `jq -r 'select(.state == "needs input")'` — reword the column later and every script breaks silently, with no type error anywhere to warn. The stable enum was already in hand. The row now carries `presentation.taskState` (`running` | `waiting` | `ready` | `stopped` | `failed`) and `ps.ts` maps it to English at the one place that renders a table. This also removes an asymmetry the reviewer named: `state` was a *kind* discriminator for registry rows (`'interactive'`) and a *task* state for managed ones — one field with two meanings, which is why `managed` had to be added beside it. A registry row now has no `taskState` at all, which is the truth: it knows a process is alive and nothing more.
Follows the parent commit: `SessionRow` now carries `presentation.taskState` instead of the string the `qwen sessions ps` table prints, so this route's `state: string` would have emitted a display label — `"needs input"`, space and all — as a JSON field that clients script against. It now emits `taskState` with the presentation layer's own tokens (`running` | `waiting` | `ready` | `stopped` | `failed`), typed as `AgentViewTaskState` rather than widened to `string`, which was a separate non-blocking finding on this PR. A row whose `taskState` is somehow unset is skipped rather than emitted with the field missing. It cannot happen today — `managedSessionRows` only maps snapshots the supervisor owns — but an agent reported with no state at all is worse than one not reported, because the caller would have to invent a state to render it.
CI caught what a vitest-only run cannot: the previous commit annotated a test case with `AgentViewTaskState` but never imported it, so `tsc --build` failed with TS2304 and took the TUI gates down with it on every branch in the stack. The import edit had targeted an import block that had since gained another symbol, so the anchor never matched and the change was silently dropped. Verified the same way it should have been the first time: every file that names the type now imports it, and nothing anywhere still refers to the removed `SessionRowState` or `row.state`.
|
Retargeted this PR from Why. Changing the base alone does not re-trigger anything — GitHub emits What this changes for reviewers. The diff is now the whole stack (+2499/-66, 30 files) instead of this layer alone (+309/-0, 5 files). It shrinks by itself as the lower PRs land, and each layer is still readable on its own PR:
The merge is clean against Also, for anyone re-reading the stage-2/3 findings on this PR: all three were fixed on 2026-09-04, after that review was written.
|
|
Qwen Code review was cancelled before a review could be posted. Nothing failed and nothing is retried automatically: the run was cancelled — by an operator, an upstream event, or the job exceeding its execution time limit. If you still want a review of this PR, request one with |
🩺 serve daemon A/BBuilt the PR base vs this PR head ✅ No response changes against the PR base across 12 scenario(s). — Qwen Code · serve A/B |
CI caught this one: two cases in this file asserted `pid: 777`, and both started failing when the parent branch's pid-liveness check reached here through the stack merge. `managedSessionRows` now verifies a recorded worker pid before reporting it, so 777 — a dead process on the runner — is correctly dropped, and the row arrives with no pid at all. The fixture uses this process's own pid instead. That keeps the real liveness path under test rather than stubbing the check away, which is the point of the check: a recorded pid can be dead or recycled onto an unrelated process, and reporting it either way would be the bug. The complementary case — a dead pid being dropped — is already covered in `managed-rows.test.ts`, so it is not duplicated here.
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R3-16 dangling docs/plans/2026-09-04-background-agent-surfaces.md reference in the route header — already reported (triage stage-2 comment 5529335539 and stage-3 review 5104767493)
Not reviewed: build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Convergence: round 3 posted 24 inline comment(s), 22 of them reported for the first time; the previous round posted 4 (3 new). The rate of new findings is not falling. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
仅完成部分审查，审查缺口已披露。
本轮确认的 1 条建议级发现已在 PR 上报告过，不再重复发布（列表见上方英文部分）。
未审查（原文为英文）：build-and-test — Integration Tests (CLI, No Sandbox) was skipped in CI and its suite did not run locally.
未审查：反向审计——在 5 轮的反审轮数上限内未收敛。
收敛情况：第 3 轮发布了 24 条行内评论，其中 22 条是首次提出；上一轮发布了 4 条（其中 3 条首次提出）。新发现的产出速度没有下降。把剩余修复攒成一批、验证后再推送，或将本 PR 的评审降到 --severity-floor critical，可以避免循环反复推导同一组发现。（仅为观察——本轮评审未因此扣留任何内容。）
— qwen3.8-max via Qwen Code /review (v0.23.0)
| const { readBackgroundPrompt, runBackgroundDispatch } = await import( | ||
| './agent-view/background-entry.js' | ||
| ); | ||
| const read = readBackgroundPrompt(rawArgv); |
There was a problem hiding this comment.
[Critical] R3-5: [certifies-falsely] [new-surface] The --bg entry intercept feeds the unnormalized rawArgv to readBackgroundPrompt, while every other decision in runCliEntry consumes normalizeServeFastPathArgv(rawArgv). In the launch shape that normalization exists for — the first argv token is the CLI entry path (the yargs/Electron special case, modeled identically by the strip in config.ts) — the path token does not start with -, so it is collected as a prompt word: ['/x/dist/qwen-cli/cli.js', '--bg', 'audit the release'] dispatches a background session whose prompt is '/x/dist/qwen-cli/cli.js audit the release' — the session runs the wrong task, presented as a normal start. The same corruption hits ['.../dist/cli.js', '--bg', '--', 'data'], where the prompt becomes the bare path.
Witness:
probe (real runCliEntry, scratch tree):
runCliEntry(['--bg', 'audit the release'])
-> runBackgroundDispatch('audit the release') [control]
runCliEntry(['/repo/dist/cli.js', '--bg', 'audit the release'])
-> runBackgroundDispatch('/repo/dist/cli.js audit the release')
runCliEntry(['/repo/dist/qwen-cli/cli.js', '--bg', '--', 'data'])
-> runBackgroundDispatch('/repo/dist/qwen-cli/cli.js')
fix flip (readBackgroundPrompt(argv)): all three dispatch the clean prompt;
the 8 existing 'Agent View entry intercepts' tests stay green
| const read = readBackgroundPrompt(rawArgv); | |
| const read = readBackgroundPrompt(argv); |
The strip must keep matching exactly the three entry suffixes in packages/cli/src/utils/serve-fast-path-argv.ts:9-15 (/dist/qwen-cli/cli.js, /dist/cli.js, /dist/cli/cli.js); reusing the normalized argv already in scope satisfies that. In packages/cli/src/cli.test.ts (describe('Agent View entry intercepts')), please add a case asserting runCliEntry(['/x/dist/cli.js', '--bg', 'audit']) calls runBackgroundDispatch with 'audit', then remove the fix and confirm the test reds (with rawArgv the dispatch receives '/x/dist/cli.js audit').
中文说明
--bg 入口拦截把未归一化的 rawArgv 传给 readBackgroundPrompt，而 runCliEntry 中所有其他决策都消费 normalizeServeFastPathArgv(rawArgv)。在归一化为它而存在的启动形态下 —— argv 首 token 是 CLI 入口路径（yargs/Electron 特例，config.ts 中有同样的剥离）—— 路径 token 不以 - 开头，因此被当作提示词收集：['/x/dist/qwen-cli/cli.js', '--bg', 'audit the release'] 会派发一个提示词为 '/x/dist/qwen-cli/cli.js audit the release' 的后台会话 —— 会话执行的是错误的任务，表面上却像正常启动。['.../dist/cli.js', '--bg', '--', 'data'] 同样损坏，提示词变成裸路径。
证据：探针（真实 runCliEntry，scratch tree）——对照用例派发干净提示词；两种入口路径形态分别派发被路径污染的提示词与裸路径；改为 readBackgroundPrompt(argv) 后三者都派发干净提示词，既有 8 个入口拦截测试保持绿色。
修法见上方 suggestion 块。剥离逻辑必须继续精确匹配 serve-fast-path-argv.ts:9-15 的三个入口后缀；复用作用域内已归一化的 argv 即可满足。请在 cli.test.ts 的 Agent View entry intercepts 中补一个用例：断言 runCliEntry(['/x/dist/cli.js', '--bg', 'audit']) 以 'audit' 调用 runBackgroundDispatch，然后移除修复、确认该测试变红（rawArgv 下派发的是 '/x/dist/cli.js audit'）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Confirmed at 2416476. cli.ts:582 passes rawArgv to readBackgroundPrompt, while every other decision in runCliEntry consumes the normalized argv from normalizeServeFastPathArgv(rawArgv): resolveBootstrapRoute(argv), firstPositionalArg(argv) at cli.ts:573, and tryRunServeFastPath(argv).
The entry-path launch shape is not hypothetical - it is the reason two strips exist. normalizeServeFastPathArgv drops a first token ending in /dist/qwen-cli/cli.js, /dist/cli.js or /dist/cli/cli.js (utils/serve-fast-path-argv.ts:14), and parseArguments carries the identical strip commented "hack: if the first argument is the CLI entry point, remove it" (config/config.ts:574). In that shape the path token does not start with -, so the word loop in readBackgroundPrompt (agent-view/background-entry.ts:81-88) collects it and dispatches /x/dist/cli.js audit the release as the prompt.
Real, and the fix is the suggested one-liner: readBackgroundPrompt(argv). Caveat for whoever takes it - the strip is suffix-based, so a spawned child whose argv[0] is a TypeScript entrypoint (the DEV/tsx shape buildCurrentQwenCliArgv produces, agent-view/current-cli-argv.ts:18) is not covered by it. No current caller spawns --bg that way, so that is a caveat rather than a second bug.
No code lands this round - the PR is +2508/-66, over this sweep's 1500-addition scope fuse - so the fix is queued for a maintainer decision. Leaving the thread unresolved.
| records: readonly SessionRegistryRecord[], | ||
| managed: readonly SessionRow[], | ||
| ): SessionRow[] { | ||
| const managedIds = new Set(managed.map((row) => row.sessionId)); |
There was a problem hiding this comment.
[Critical] R3-10: [certifies-falsely] [new-surface] mergeSessionRows dedups registry records against managed rows by exact string equality on sessionId, but the two sources canonically carry different spellings of the same id: the managed side is forced to the lowercased store directory name (sanitizeSessionId lowercases at supervisor-store.ts:550; normalizeSessionState sets state.sessionId to path.basename(paths.sessionDir)), while the registry record keeps the raw spelling (registerSession writes fields.sessionId verbatim, and the worker registers config.getSessionId() after the supervisor respawns it with --resume=<raw spelling>). Mixed-case ids are legal (INTERNAL_SESSION_ID_REGEX is case-insensitive). So a managed session whose id contains uppercase is listed twice — one managed row with its real state, one interactive row — and --json emits two objects: exactly the double listing this function's own doc comment exists to prevent. The trigger is currently latent (the detach path that preserves the raw spelling is not wired to a user command yet, and --bg generates lowercase UUIDs), but the defect is in this diff's new merge code and wakes the day a mixed-case id reaches it.
Witness:
probe (scratch tree, vitest): registry record sessionId
'ABCDEF12-3456-1234-89AB-CDEF12345678' + managed snapshot lowercased
'abcdef12-...' on PR code -> expected [ ... ] to have a length of 1 but got 2
(managed row + interactive row); with case-insensitive dedup the probe
passes (1 row) and all 18 existing managed-rows tests stay green
Canonicalize both sides at comparison time with the store's own sanitizer — never rewrite either stored spelling:
const managedIds = new Set(managed.map((row) => sanitizeSessionId(row.sessionId)));
// ...
.filter((record) => !managedIds.has(sanitizeSessionId(record.sessionId)))The two spellings coexist by design — parseAdoptParams at packages/cli/src/agent-view/supervisor-process.ts:4182-4186: "The store lowercases directory names… The raw spelling is kept separately: the native session store is case-sensitive, so --resume must use it" — so the fix must canonicalize at comparison time only. In ps.test.ts, please add a test pairing managedSnapshot() (state.sessionId 'managed-1') with record({ sessionId: 'Managed-1' }) asserting exactly one row (the managed one), then restore the plain Set/has comparison and confirm it reds (two rows).
中文说明
mergeSessionRows 用 sessionId 的精确字符串相等来对注册表记录与 managed 行去重，但这两个来源对同一个 id 规范地保存着不同写法：managed 一侧被强制为小写的存储目录名（sanitizeSessionId 在 supervisor-store.ts:550 转小写；normalizeSessionState 把 state.sessionId 设为 path.basename(paths.sessionDir)），而注册表记录保留原始写法（registerSession 原样写入 fields.sessionId，supervisor 以 --resume=<原始写法> 重启 worker 后，worker 用 config.getSessionId() 注册）。混合大小写的 id 是合法的（INTERNAL_SESSION_ID_REGEX 大小写不敏感）。因此 id 含大写字母的 managed 会话会被列两次 —— 一行带真实状态的 managed 行、一行 interactive 行 —— --json 会输出两个对象：正是本函数自身注释声明要防止的重复列出。触发路径目前是潜伏的（保留原始写法的 detach 路径尚未接到用户命令，--bg 生成小写 UUID），但缺陷在本 diff 新增的合并代码里，一旦出现混合大小写 id 就会发作。
证据：探针（scratch tree，vitest）——注册表记录 ABCDEF12-… 与小写 managed 快照 abcdef12-… 在 PR 代码上得到 2 行（期望 1）；改为大小写不敏感去重后探针通过（1 行），既有 18 个 managed-rows 测试保持绿色。
修法：比较时用存储自己的归一化函数对两侧归一（绝不改写任一存储写法），代码见上方英文块。两种写法是设计上共存的 —— supervisor-process.ts:4182-4186 的 parseAdoptParams 注释：「存储把目录名转小写……原始写法另行保留：原生会话存储大小写敏感，--resume 必须用它」—— 因此只能在比较时归一。请在 ps.test.ts 补一个用例：managedSnapshot()（state.sessionId 为 'managed-1'）配 record({ sessionId: 'Managed-1' })，断言恰好一行（managed 行）；恢复普通 Set/has 比较后确认其变红（两行）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
There was a problem hiding this comment.
Confirmed at 2416476. mergeSessionRows dedups by exact string equality - new Set(managed.map((row) => row.sessionId)) at managed-rows.ts:179 and the !managedIds.has(record.sessionId) filter just below it - while the two sides canonically carry different spellings of the same id. sanitizeSessionId lowercases (agent-view/supervisor-store.ts:550), and parseAdoptParams deliberately keeps the raw spelling beside the sanitized one for --resume (agent-view/supervisor-process.ts:4187, :4191, :4205), with the comment above them stating that the store lowercases directory names while the native session store is case-sensitive.
Reachability at head, because it sets priority: the trigger is latent, not live. --bg dispatch generates randomUUID() (agent-view/supervisor-dispatch.ts:45), which is lowercase. The only path that can put a mixed-case id into the store while a registry record keeps the raw spelling is adoption, and adoption's only in-repo caller detachCurrentSessionToAgentView (agent-view/managed-detach.ts:32) is imported solely by managed-detach.test.ts - no user command reaches it. So nothing double-lists today.
It is still a real defect in this diff's new merge code, and it activates the day detach/adoption gets wired. The fix is comparison-time canonicalization only (sanitizeSessionId on both sides), never rewriting a stored spelling. Queued for a maintainer decision - fix here, or land it with the PR that wires detach. No code lands this round - the PR is +2508/-66, over this sweep's 1500-addition scope fuse - so the fix is queued for a maintainer decision. Leaving the thread unresolved.
|
|
||
| Experimental. Runs a prompt as a background session and returns immediately, printing the session id. | ||
|
|
||
| The session is owned by a supervisor process that outlives the shell you started it from, so closing that terminal does not stop the work. `qwen sessions ps` lists it, and says whether it is `working` or has stopped to ask you something. It is a full Qwen Code session, so — when `agents.crossSessionMessaging` is on — it also appears in another session's `list_agents` and can be addressed with `send_message` (see [Messaging Another Running Session](#6-messaging-another-running-session)). |
There was a problem hiding this comment.
[Suggestion] R3-24: This sends users to section 6, which promises each session appears in list_agents and can be addressed with send_message — but for a managed session the addressable name and the name qwen sessions ps --json records come from different derivations: send_message addresses by the registry name (deriveSessionName(cwd, sessionId) → <dir-basename>-<2-hex>; resolvePeerTarget matches record.name exactly), while the managed row's name is the presentation title (roster name / launch prompt / activity summary). The jq example at line 851 extracts exactly that managed title, implying it is an actionable address — and mergeSessionRows' dedup hides the registry row from ps, so no ps surface reveals the real one. A user who follows the documented flow (ps --json | jq … → send_message to=<title>) gets a not-found refusal.
Witness:
probe (real deriveSessionName, managedSessionRows, toPeerSessionInfo,
resolvePeerTarget):
ps --json managed-row .name = "release audit"
registry name (list_agents address) = "app-d2"
send_message to="release audit" -> none
send_message to="app-d2" -> one
State here (or in section 6) that a background session's messaging address is the name list_agents itself shows — derived from the session's working directory, not the title sessions ps prints — or include the registry name in managed ps --json rows.
中文说明
此处把用户引向第 6 节，该节承诺每个会话都会出现在 list_agents 中并可用 send_message 寻址 —— 但对 managed 会话，可寻址的名字与 qwen sessions ps --json 记录的名字来自不同派生：send_message 按注册表名寻址（deriveSessionName(cwd, sessionId) → <目录名>-<2位十六进制>；resolvePeerTarget 对 record.name 精确匹配），而 managed 行的 name 是展示标题（roster 名 / 启动提示词 / 活动摘要）。第 851 行的 jq 示例提取的恰是那个 managed 标题，暗示它是可用的地址 —— 而 mergeSessionRows 的去重又把注册表行从 ps 中隐藏，因此 ps 的任何表面都看不到真实地址。按文档流程操作的用户（ps --json | jq … → send_message to=<标题>）会得到「找不到」的拒绝。
证据：探针（真实 deriveSessionName、managedSessionRows、toPeerSessionInfo、resolvePeerTarget）——ps --json managed 行 .name = "release audit"；注册表名（list_agents 地址）"app-d2"；send_message to="release audit" → 无匹配；send_message to="app-d2" → 命中。
请在此处（或第 6 节）说明：后台会话的消息地址是 list_agents 自身显示的名字 —— 由会话工作目录派生，而不是 sessions ps 打印的标题；或在 managed 的 ps --json 行中加入注册表名。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| **managed** session is an Agent View session owned by a supervisor: it | ||
| writes no registry record, so it used to be invisible here. Managed |
There was a problem hiding this comment.
[Suggestion] R3-1: This (and the module header this diff adds at packages/cli/src/commands/sessions/managed-rows.ts:14) states managed sessions write no live-process registry record — contradicting the design issue this PR implements and the diff's own dedup. Issue #10951 records the measured fact that a --bg session "is launched as a full interactive session… so it registers in the live-process registry", and its fact-check re-verified registerSession is called unconditionally at startInteractiveUI.tsx:421; both hold at HEAD (buildNativeWorkerArgv launches --session-id <id> --prompt-interactive=<prompt>, and nothing on that path suppresses registration). A running --bg session was therefore already visible to the old registry-walking ps as an interactive row — what this change adds is its semantic state, not its visibility. mergeSessionRows' own comment acknowledges managed workers "can also write a registry record", and the test 'lists a session once when it is both managed and registered' pins the dedup that follows.
Witness:
witness: not run — proving the 'already visible to the old ps' half by
execution needs an A/B driving a real qwen --bg session (supervisor + PTY
worker + model-backed interactive startup) in both trees; the contradiction
is pinned by the quoted lines, including the diff's own dedup rationale
Reword both sites to the measured fact, e.g.: "A managed session is an Agent View session owned by a supervisor. Its worker launches as a full interactive session, so it can also write a live-process registry record; before this listing the registry could only show it as an interactive row with no state." The rewording must not assert or imply managed sessions never register — managed-rows.test.ts 'lists a session once when it is both managed and registered' and the unconditional registerSession at packages/cli/src/ui/startInteractiveUI.tsx:421 are the pins that invites deleting the dedup if it does.
中文说明
此处（以及本 diff 在 packages/cli/src/commands/sessions/managed-rows.ts:14 新增的模块头）声称 managed 会话不写活进程注册表记录 —— 与本 PR 声明实现的设计 issue 及 diff 自身的去重相矛盾。Issue #10951 记录了实测事实：--bg 会话「以完整交互式会话启动……因此会注册进活进程注册表」，其事实核查复核了 registerSession 在 startInteractiveUI.tsx:421 无条件调用；两者在 HEAD 均成立（buildNativeWorkerArgv 以 --session-id <id> --prompt-interactive=<prompt> 启动，该路径上没有任何豁免）。因此运行中的 --bg 会话本来就能被旧的、遍历注册表的 ps 看到（作为 interactive 行）—— 本改动增加的是它的语义状态，而不是它的可见性。mergeSessionRows 自己的注释承认 managed worker「也可能写注册表记录」，测试 'lists a session once when it is both managed and registered' 钉住了随之而来的去重。
证据：未运行 —— 要通过执行证明「旧 ps 本可见」这一半，需要在两棵树里 A/B 驱动一个真实 qwen --bg 会话（supervisor + PTY worker + 有模型支撑的交互式启动）；该矛盾由上述引文钉住，包括 diff 自身的去重理由。
请把两处改为实测事实，例如：「managed 会话是由 supervisor 拥有的 Agent View 会话。它的 worker 以完整交互式会话启动，因此也可能写活进程注册表记录；在本列表出现之前，注册表只能把它显示为没有状态的 interactive 行。」措辞不得声称或暗示 managed 会话从不注册 —— managed-rows.test.ts 的 'lists a session once when it is both managed and registered' 与 startInteractiveUI.tsx:421 的无条件 registerSession 正是相应的钉扎；若措辞失实，会诱使维护者删掉去重。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| An interactive session is emitted as its whole registry record, plus | ||
| `managed: false`: |
There was a problem hiding this comment.
[Suggestion] R3-3: The new JSON contract says the interactive row is the "whole registry record" and its field list omits ipcPath, which is emitted, while the deliberate stripping of the credential field ipcToken goes unmentioned — ps.ts emits { ...row.record, ipcToken: undefined, managed: false }, and SessionRegistryRecord carries optional ipcPath/ipcToken. Any session with cross-session messaging enabled therefore emits an undocumented ipcPath field in --json, and a reader auditing what ps --json spills into logs/pipelines cannot tell from this section — which presents itself as the contract to script against — that the inbox token is deliberately excluded.
Witness:
probe (drove the real handler with a record carrying both fields):
{ "ipcPath": "/tmp/qwen-inbox/sess-1.sock", "hasIpcToken": false }
Add ipcPath to the field list with a note that it appears when the session has a messaging socket, and say ipcToken is deliberately omitted.
中文说明
新的 JSON 契约说交互行是「整个注册表记录」，但字段列表遗漏了实际会输出的 ipcPath，同时刻意剥离凭据字段 ipcToken 一事未被提及 —— ps.ts 输出 { ...row.record, ipcToken: undefined, managed: false }，而 SessionRegistryRecord 带有可选的 ipcPath/ipcToken。因此任何启用跨会话消息的会话都会在 --json 中输出未文档化的 ipcPath 字段；审计 ps --json 会向日志/管道泄漏什么的读者，无法从这一自称「脚本契约」的小节看出收件箱 token 是被刻意排除的。
证据：探针（以同时携带两个字段的记录驱动真实 handler）——{ "ipcPath": "/tmp/qwen-inbox/sess-1.sock", "hasIpcToken": false }。
请把 ipcPath 加入字段列表并注明它在会话具有消息 socket 时出现，并说明 ipcToken 被刻意省略。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| const reason = error instanceof Error ? error.message : String(error); | ||
| writeStderrLine( | ||
| `Managed sessions could not be listed: ${sanitize(reason)}`, |
There was a problem hiding this comment.
[Suggestion] R3-15: The store-failure guard announces itself through writeStderrLine, which throws on a closed or broken stderr — so the exact failure the guard catches can still take the whole listing down. Run qwen sessions ps with stderr unavailable (2>&-, or piped to a reader that has exited) while the supervisor store is unreadable — the EACCES case the sibling test enacts: listAgentViewSessionSnapshots() rejects, the catch runs, and process.stderr.write throws (stdioHelpers.ts documents that writeStderrLine throws on EPIPE or a closed fd). The throw escapes readManagedRows, Promise.all rejects, handlePs aborts, and the registry rows listLiveSessions already returned are never emitted — the command fails wholesale instead of degrading to the registry half that the guard's own comment promises ("must not take the command down — the registry half still answers the question").
Witness:
probe (store EACCES + throwing stderr): the handler rejects —
'expected Error: write EPIPE { code: EPIPE } to be undefined' —
and the registry row is never emitted;
with the note on writeStderrLineSafe the probe passes and the
interactive row is emitted
Emit the note with writeStderrLineSafe from the same stdioHelpers.js module — the note is incidental and there is definitionally nowhere to report once stderr is gone; stdioHelpers.ts:51-58 ("Use it only where the write is incidental to the work at hand and failing it would destroy something real") is the criterion, and this site meets it, so prefer it over an ad-hoc try/catch. In ps.test.ts, please extend the existing store-failure tests with the stdioHelpers.js mock's writeStderrLine throwing, then assert run() resolves and stdout still contains the interactive row ('app-ab'); with plain writeStderrLine that assertion goes red.
中文说明
存储失败守卫通过 writeStderrLine 通告自身，而它在 stderr 关闭或损坏时会抛出 —— 于是守卫捕获的那个失败本身仍可能拖垮整个列表。在 supervisor 存储不可读时运行 qwen sessions ps 且 stderr 不可用（2>&-，或管道指向已退出的读者）—— 正是兄弟测试模拟的 EACCES 场景：listAgentViewSessionSnapshots() 拒绝，catch 执行，process.stderr.write 抛出（stdioHelpers.ts 记载 writeStderrLine 在 EPIPE 或 fd 关闭时抛出）。抛出逃出 readManagedRows，Promise.all 拒绝，handlePs 中止，listLiveSessions 已经返回的注册表行永远不会被输出 —— 命令整体失败，而不是按守卫自身注释承诺的那样降级到注册表一半（「不能拖垮命令 —— 注册表一半仍能回答问题」）。
证据：探针（存储 EACCES + stderr 抛出）——handler 拒绝：expected Error: write EPIPE { code: EPIPE } to be undefined，注册表行从未输出；改用 writeStderrLineSafe 输出该提示后，探针通过且交互行正常输出。
修法：用同一 stdioHelpers.js 模块的 writeStderrLineSafe 输出该提示 —— 该提示是附带信息，且 stderr 已不存在时按定义无处可报；stdioHelpers.ts:51-58（「仅当写入对工作本身是附带、且写失败会毁掉真实成果时使用」）即判断标准，本处满足，优于临时 try/catch。请在 ps.test.ts 扩展现有存储失败测试：令 stdioHelpers.js mock 的 writeStderrLine 抛出，断言 run() 正常 resolve 且 stdout 仍含交互行（'app-ab'）；使用普通 writeStderrLine 时该断言变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| if (rows.length === 0) { | ||
| writeStdoutLine('No other Qwen Code sessions are running.'); |
There was a problem hiding this comment.
[Suggestion] R3-12: The reworded empty-state message asserts that no Qwen Code sessions at all are running — beyond what the command checked — in two distinct states. (a) The supervisor store is unreadable: readManagedRows swallows the error and returns [], so the definitive message prints while stderr carries the failure note; a stdout-only consumer (qwen sessions ps | grep, a script) gets an affirmative absence claim although a managed session sitting on an unanswered question may exist — the exact "cannot tell no-agents from cannot-look" confusion the route half of this PR says it rejects. (b) Headless (qwen -p) runs are invisible to ps by design — they write no live-process registry record (only the two interactive-UI starters call registerSession: startInteractiveUI.tsx:421, start-opentui-ui.tsx:417-418), a blind spot documented in this file's own header — yet the pre-change wording's accurate "interactive" qualifier was dropped, so a user with a concurrent headless job is told nothing is running and may relaunch it. Both existing store-failure tests seed a registry record, so neither path is tested.
Witness:
probe (store EACCES + empty registry): stdout
['No other Qwen Code sessions are running.'] while stderr carries
'Managed sessions could not be listed: EACCES…'; threading the store
failure into the empty state flips the probe and keeps all 26 ps tests green
headless trigger verified by trace: the only registerSession call sites are
the two interactive-UI starters
Track the degradation (e.g. have readManagedRows return { rows, failed } or a flag) and print a qualified message such as 'No interactive or managed Qwen Code sessions could be listed.' when the managed read failed; keep the stderr note — it must stay on stderr only, per the documented contract "stderr keeps --json stdout parseable" (ps.ts:147-152), pinned by the test 'keeps --json stdout parseable when the store fails'. Please add a test with listLiveSessions.mockResolvedValue([]) and listAgentViewSessionSnapshots.mockRejectedValue(new Error('broken')), asserting stdout does not contain 'are running'; removing the qualification must make it red.
中文说明
改写后的空状态消息断言「没有任何 Qwen Code 会话在运行」—— 超出该命令实际检查的范围 —— 且有两种不同的情形。(a) supervisor 存储不可读：readManagedRows 吞掉错误并返回 []，于是这句确定性消息照常打印，而失败提示在 stderr；只读 stdout 的消费者（qwen sessions ps | grep、脚本）会得到肯定的「不存在」结论，尽管可能正有一个 managed 会话在等待回答 —— 正是本 PR 路由那一半声称拒绝的「分不清没有 agent 与看不了」。(b) 无头（qwen -p)运行按设计对 ps 不可见 —— 它们不写活进程注册表记录（只有两个交互式 UI 启动器调用 registerSession：startInteractiveUI.tsx:421、start-opentui-ui.tsx:417-418），这一盲点就记录在本文件自己的头部 —— 但改动前措辞里准确的「interactive」限定词被丢掉了，于是有并发无头任务的用户会被告知什么都没在运行，并可能重新启动它。既有的两个存储失败测试都预置了注册表记录，因此两条路径都没有测试。
证据：探针（存储 EACCES + 空注册表）——stdout 为 ['No other Qwen Code sessions are running.']，stderr 为 'Managed sessions could not be listed: EACCES…'；把存储失败状态接入空状态判断后探针翻转，26 个 ps 测试保持绿色。无头触发由追踪核实：registerSession 的调用点只有两个交互式 UI 启动器。
修法：追踪降级状态（例如让 readManagedRows 返回 { rows, failed } 或一个标志），当 managed 读取失败时打印限定措辞，如 'No interactive or managed Qwen Code sessions could be listed.'；stderr 提示保留 —— 且必须只留在 stderr，按文档契约「stderr 保证 --json 的 stdout 可解析」（ps.ts:147-152），由测试 'keeps --json stdout parseable when the store fails' 钉住。请补一个测试：listLiveSessions.mockResolvedValue([]) 且 listAgentViewSessionSnapshots.mockRejectedValue(new Error('broken'))，断言 stdout 不含 'are running'；移除限定措辞后该断言应变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| registerBackgroundAgentRoutes(app, { | ||
| isWorkspaceTrusted: isPrimaryWorkspaceTrusted, | ||
| }); |
There was a problem hiding this comment.
[Suggestion] R2-1: Still standing from round 2 — no server-level test exercises the trusted positive path through createServeApp for GET /background-agents, so the default listSnapshots fallback and the trusted side of the wired predicate are never executed. Every unit test injects listSnapshots, and the sole server-level test is refused at the trust gate before the store is read — so one-line mutations survive the whole suite green: wiring isWorkspaceTrusted: () => false gives trusted workspaces a permanent 403 on this route, and replacing the ?? listAgentViewSessionSnapshots default with a throwing function gives them a permanent 503.
Witness:
probe (scratch tree, QWEN_HOME isolated):
intact default-reader request -> 200 with agents array (1 passed)
mutant ?? (async () => { throw … }) -> probe 'expected 503 to be 200'
while existing suite 7 passed
mutant isWorkspaceTrusted: () => false -> server test still '1 passed | 1176 skipped'
Add a trusted-path test beside the refusal test in server.test.ts: build createServeApp(tokenOpts, undefined, { bridge: fakeBridge(), primaryWorkspaceTrusted: true }), then the authenticated GET /background-agents asserting status 200 and Array.isArray(res.body.agents). The default reader touches the test machine's real supervisor store — ENOENT on the jobs dir returns [] (supervisor-store.ts:341) but a machine with real sessions returns them — so assert status and shape only, not listing content. The new test is its own witness: it goes red if the trusted wiring breaks (constant-false predicate → 403, throwing default store → 503, registration removed → 404) — please remove the predicate from this registration, run it, and confirm it reds.
中文说明
第 2 轮遗留、仍然成立 —— 没有服务端级测试经过 createServeApp 走 GET /background-agents 的受信任正向路径，因此默认的 listSnapshots 回退与已接线谓词的受信任一侧从未被执行。所有单测都自行注入 listSnapshots，唯一的服务端级测试在信任门禁处就被拒绝、到不了存储读取 —— 于是单行变异可以在整套测试保持绿色的情况下存活：把接线改成 isWorkspaceTrusted: () => false 会让受信任工作区在该路由上永久 403；把 ?? listAgentViewSessionSnapshots 默认值换成抛错函数会让它永久 503。
证据：探针（scratch tree，QWEN_HOME 隔离）——完整代码下走默认读取器的请求 → 200 且返回 agents 数组（1 passed）；变异体 ?? (async () => { throw … }) → 探针 expected 503 to be 200，既有套件仍 7 passed；变异体 isWorkspaceTrusted: () => false → 服务端测试仍 1 passed | 1176 skipped。
请在 server.test.ts 的拒绝用例旁补一个受信任路径测试：以 createServeApp(tokenOpts, undefined, { bridge: fakeBridge(), primaryWorkspaceTrusted: true }) 构建，随后带鉴权请求 GET /background-agents，断言状态 200 且 Array.isArray(res.body.agents)。默认读取器会触碰测试机真实的 supervisor 存储 —— jobs 目录 ENOENT 时返回 []（supervisor-store.ts:341），但本机若真有 session 则会返回它们 —— 因此只断言状态与形状，不断言列表内容。新测试自身即验收见证：受信任接线被破坏时它会变红（谓词恒 false → 403、默认存储抛错 → 503、注册被移除 → 404）—— 请移除本注册处的谓词后运行确认其变红。
— qwen3.8-max via Qwen Code /review (v0.23.0)
| function appWith(listSnapshots: () => Promise<AgentViewSessionSnapshot[]>) { | ||
| const app = express(); | ||
| registerBackgroundAgentRoutes(app, { | ||
| listSnapshots: listSnapshots as never, |
There was a problem hiding this comment.
[Suggestion] R2-2: Still standing from round 2 — the injected listSnapshots mock is cast as never here and again in the untrusted-workspace test below, erasing the compile-time check against the dep's declared contract typeof listAgentViewSessionSnapshots — and the cast is unnecessary. If listAgentViewSessionSnapshots's signature drifts (a required StoreOptions argument, or a changed snapshot return type), these unit tests still compile and pass because the cast hides the mismatch, while the production wiring — server.ts registers the route with no listSnapshots override, so the route invokes the real default store — breaks, and the test seam can no longer be caught disagreeing with the declared contract.
Witness:
probe (scratch tree):
both casts removed -> tsc --noEmit exit 0; vitest 7 passed (casts unnecessary)
simulated drift, no cast -> TS2322 at the injection line (exit 2)
identical drift with `as never` -> tsc exit 0
| listSnapshots: listSnapshots as never, | |
| listSnapshots, |
Delete the second cast in the untrusted-workspace test the same way (listSnapshots: async () => [snapshot()] type-checks directly against the dep type).
中文说明
第 2 轮遗留、仍然成立 —— 注入的 listSnapshots mock 在此处被转换为 as never，下方不受信任工作区的测试里也有同样一处，抹掉了针对 dep 声明契约 typeof listAgentViewSessionSnapshots 的编译期检查 —— 而该转换并无必要。若 listAgentViewSessionSnapshots 的签名漂移（新增必填的 StoreOptions 参数，或快照返回类型变化），这些单测仍能编译并通过，因为转换掩盖了不匹配；而生产接线 —— server.ts 注册该路由时未覆盖 listSnapshots，路由会调用真实的默认存储 —— 将会损坏，且测试接缝与声明契约的不一致将再也无法被发现。
证据：探针（scratch tree）——删除两处转换 → tsc --noEmit exit 0、vitest 7 passed（转换本不必要）；模拟签名漂移且不加转换 → 注入行报 TS2322（exit 2）；同样的漂移加 as never → tsc exit 0。
修法见上方英文 suggestion 块；请同样删除不受信任工作区测试中的第二处转换（listSnapshots: async () => [snapshot()] 可直接通过 dep 类型检查）。
— qwen3.8-max via Qwen Code /review (v0.23.0)
2 participants
What this PR does
Stack position 4/4. Parent: #10949. Adds
GET /background-agentstoqwen serve: the sessions the Agent View supervisor is running, with what each one is doing.{ "agents": [ { "sessionId": "0f8e1c42-…", "name": "release audit", "state": "needs input", "cwd": "/w/app", "pid": 777, "startedAt": "2026-09-04T11:58:00.000Z" } ] }Why it's needed
The daemon has its own idea of what a session is, and it is not the one a background agent lives in.
standalone-session-servicetracks conversations the daemon itself hosts, in its own memory; a session started withqwen --bgis owned by the supervisor and recorded in the roster under~/.qwen/. Nothing in the daemon reads that roster —rg 'listLiveSessions|session-registry' packages/cli/src/servereturns nothing across 94,136 lines — so every daemon client, the Web Shell included, is blind to background agents.The rows come from
managedSessionRows, the same functionqwen sessions psrenders (#10942), so the CLI and anything built on this route cannot describe one session two different ways. That includes the part that is easy to get wrong: a failed session is labelledfailed, not folded into the roster'scompleteddisplay group. The roster UI can afford that fold because it also paints an icon tone; an HTTP client has nothing else to carry the difference.Two decisions worth reviewing:
What is deliberately not here
The Web Shell panel. This PR is the first half of
docs/plans/2026-09-04-background-agent-surfaces.md§3; the second half is a panel that renders these rows and answers a waiting agent.I did not write it, and the reason is worth stating rather than hiding:
packages/web-shellis 172 component files with Playwright visual tests, its sidebar sessions come throughDaemonSessionProvider, and the machine this was written on can neither build the Web Shell nor run Playwright. A panel written blind into that pipeline is the kind of change a maintainer ends up rewriting. The route is the part that can be built and tested honestly from here; the panel wants someone who can see it.Reviewer Test Plan
How to verify
cd packages/cli && npx vitest run src/serve/routes/background-agents.test.ts --coverage.enabled=false→ 6 tests. They cover a running agent reported with its pid and ISO start time, a failed session labelledfailedrather thancompleted, absent pid/startedAtomitted rather than invented (an unparseablecreatedAtmust not become 1970), an empty list, the 503 on an unreadable store, and an untrusted workspace listing nothing.eslintandprettier --checkare clean on the changed files.End to end, on a build of this stack:
qwen servein one terminal,qwen --bg "audit the release"in another.curl -s localhost:<port>/background-agents | jqlists the agent, initiallyworking.needs input.qwen sessions stop <id>, then confirm the route reports it stopped.Evidence (Before & After)
Before: no daemon route reported background agents; they existed only in the CLI.
After: the JSON above. It is the shape the unit tests pin — not a live capture, since this machine cannot run
qwen serveagainst a real supervisor.Tested on
Unit tests only, on Linux.
npx tsc --noEmitandnpm run buildwere not run here; CI covers both. The four end-to-end steps above are unverified.Environment (optional)
Linux, vitest only.
Risk & Scope
Linked Issues
Implements §3 step 1 of the design in #10951. Builds on #10942, #10943, #10949.
中文说明
这个 PR 做了什么
栈位置 4/4。父 PR：#10949。为
qwen serve新增GET /background-agents：列出 Agent View supervisor 正在运行的 session，以及每一个正在做什么。为什么需要
daemon 对「session 是什么」有自己的一套理解，而后台 agent 并不活在那套里。
standalone-session-service追踪的是 daemon 自己托管的会话，存在它自己的内存中；而qwen --bg启动的 session 由 supervisor 拥有，记录在~/.qwen/下的 roster 里。daemon 中没有任何代码读取那个 roster ——rg 'listLiveSessions|session-registry' packages/cli/src/serve在 94,136 行中零命中 —— 因此包括 Web Shell 在内的每一个 daemon 客户端，对后台 agent 都是盲的。数据行来自
managedSessionRows，与qwen sessions ps渲染的是同一个函数（#10942），因此 CLI 与基于本路由构建的任何东西，都不会对同一个 session 给出两种说法。这包括最容易出错的那一处：失败的 session 标注为failed，而不是被折叠进 roster 的completed展示分组。roster UI 之所以负担得起那次折叠，是因为它还会绘制图标色调；HTTP 客户端没有第二条通道来承载这个差别。两处判断值得评审：
刻意没有包含的部分
Web Shell 面板。 本 PR 是
docs/plans/2026-09-04-background-agent-surfaces.md§3 的前半；后半是一个渲染这些行、并能回答等待中 agent 的面板。我没有写它，理由值得明说而不是藏着：
packages/web-shell有 172 个组件文件并带 Playwright 视觉测试，其侧边栏的 session 来自DaemonSessionProvider，而撰写本 PR 的机器既不能构建 Web Shell 也不能运行 Playwright。在那条数据管线里盲写一个面板，属于维护者最终要重写的那类改动。路由是从这里能够诚实地构建与测试的部分；面板需要一个看得见它的人。评审者测试计划
如何验证
cd packages/cli && npx vitest run src/serve/routes/background-agents.test.ts --coverage.enabled=false→ 6 个测试。覆盖：运行中的 agent 连同 pid 与 ISO 起始时间被报告；失败的 session 标注为failed而非completed；缺失的 pid/startedAt被省略而不是臆造（无法解析的createdAt不得变成 1970）；空列表；存储不可读时的 503；不受信任的工作区不列出任何内容。改动文件的
eslint与prettier --check干净。端到端，在本栈的构建产物上：
qwen serve，另一个跑qwen --bg "audit the release"。curl -s localhost:<port>/background-agents | jq列出该 agent，初始为working。needs input。qwen sessions stop <id>，然后确认路由报告它已停止。证据（前后对比）
之前：没有任何 daemon 路由报告后台 agent；它们只存在于 CLI 中。
之后：上面那段 JSON。它是单测钉住的形状 —— 不是实时截取，因为本机无法针对真实 supervisor 运行
qwen serve。测试环境
仅 Linux 上的单元测试。
npx tsc --noEmit与npm run build未在此运行；CI 覆盖两者。上述四条端到端步骤未经验证。运行环境（可选）
Linux，仅 vitest。
风险与范围
关联 Issue
实现 #10951 中设计的 §3 第 1 步。构建于 #10942、#10943、#10949 之上。