feat(cli): see, answer and stop a background session - #10949
Conversation
`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.
|
Re-run at an unchanged head. Template looks good ✓ — all nine headings plus the Chinese Problem: real, and still only inside the stack. This is a Direction: aligned. Background Agent View sessions are an actively developed subsystem here — Size: 1749 changed lines = 683 production + 44 docs + 1022 test, across 16 files. One core path is touched: Approach: the split between the two new files is still the right one, and my independent read of it has not changed. The cost is that its three argv mechanisms do not agree on how to find the command. New this pass, and it is rebase work rather than a blocker on the diff as based: On that Risk: unchanged, and it is in the diff rather than in the process. Two measured Critical defects on No Stage 1e high-risk path match — I ran the changed-file list against the pattern set and nothing hit. The structural point is yours as a maintainer, not mine to gate on: because Not moving on to a fresh code review: the two Criticals carry into Stage 3, where the verdict stands as the previous pass left it. 中文说明本次是在未变动的 head 上重跑。 模板完整 ✓ —— 九个标题加中文 问题: 真实存在,而且仍然只存在于这个栈内部。这是 方向: 对齐。后台 Agent View session 在这里是一个正在积极开发的子系统 —— 规模: 1749 行改动 = 683 行生产代码 + 44 行文档 + 1022 行测试,涉及 16 个文件。触及一条核心路径: 方案: 两个新文件之间的切分仍然是对的,我对它的独立判断没有变化。 代价在于它的三套 argv 机制对「如何找到这条命令」并没有达成一致。 本轮新增,而且它属于 rebase 工作、不是对「以自身 base 为准的 diff」的拦截理由: 关于那处 风险: 未变,而且它在 diff 里、不在流程里。 Stage 1e 高风险路径无匹配 —— 我拿变更文件列表跑过那组模式,没有命中。 结构性问题属于你作为 maintainer 来定,不是我可以据此拦截的:因为 不进入一次全新的代码评审:两条 Critical 带入 Stage 3,结论维持上一轮的样子。 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
Re-run at an unchanged head — 2026-09-10 passHead is still Both Criticals re-verified against the exact reviewed commit, by reading the lines. I built and executed nothing from this PR — the CI path forbids it, and the agent environment here holds a write PAT that PR-derived code could read. So this is a read of the tree at
The mechanism-level diagnosis below also holds on this read: A third independent read agrees. New this pass:
|
| File | What changed |
|---|---|
docs/users/features/commands.md |
Adds the three subcommands to the session table and a usage section; rewrites the paragraph that said a background session cannot be answered or stopped. The -- hatch is scoped to dash-leading answers, which is narrower than the shapes that break (finding 4) |
packages/cli/src/agent-view/supervisor-process.ts |
The peek reply gains a roster entry, a redacted launch record and an answerable verdict; new private isSessionAnswerable mirrors the answer path refusal model |
packages/cli/src/agent-view/supervisor-process.test.ts |
Pins the producer side of the peek join, including that the launch env is stripped, and that answerable flips false once an answer is queued |
packages/cli/src/agent-view/supervisor-store.ts |
Exports findAgentViewRosterEntry as the single owner of the roster matching rule, and promotes redactAgentViewLaunch to an export |
packages/cli/src/agent-view/supervisor-store.test.ts |
Covers findAgentViewRosterEntry, asserting both sides of the id comparison are sanitized |
packages/cli/src/cli.ts |
versionTokenIndex counts positionals so a version token inside an answer reaches the answer parser instead of printing the version — the ordinal anchor behind finding 2 |
packages/cli/src/cli.test.ts |
Covers that exemption via the real resolveBootstrapRoute, and asserts other command chains keep the version intercept (12/12 at head) |
packages/cli/src/commands/sessions.ts |
Registers peek, answer and stop alongside list and ps |
packages/cli/src/commands/sessions.test.ts |
Now pins all five subcommands by name and registration order |
packages/cli/src/commands/sessions/control-commands.ts |
New, 290 lines. The three CommandModules, the fixed-index separator anchor behind finding 1, and the findRun-anchored raw tail that survives a prefix |
packages/cli/src/commands/sessions/control-commands.test.ts |
New, 393 lines over the argv machinery. Faithful mirror of config.ts for the unprefixed shape; never constructs the prefixed one (finding 3) |
packages/cli/src/commands/sessions/managed-control.ts |
New, 241 lines. The print and refusal decisions, supervisor injected, no I/O of its own |
packages/cli/src/commands/sessions/managed-control.test.ts |
New, 455 lines over those decisions (24 tests, not the 13 the body claims) |
packages/cli/src/commands/sessions/ps.ts |
Drops its local sanitize helper in favour of the shared one-line recipe — proven byte-identical on hostile input |
packages/cli/src/config/config.ts |
Calls insertAnswerTextSeparator on rawArgv before the yargs tree is built; the only core-path change, 6 lines |
packages/cli/src/ui/utils/textUtils.ts |
Adds sanitizeSingleLineTerminalText beside sanitizeTerminalText |
Testing
This section said "there is no test evidence for this head" twelve hours ago. That is now wrong and here is what replaced it.
CI on the PR itself — still structurally absent. .github/workflows/ci.yml filters pull_request to main and release/** (lines 21-24). This PR's base is feat/agent-view-bg-dispatch, so test, lint_and_static and the typecheck lane were never scheduled against it — not red, never invoked. The only pull_request-event workflow that ran on this head is tui-parity. Everything else below is bot orchestration.
| Check | Conclusion |
|---|---|
| TUI parity snapshots (ink vs opentui) | success |
| OpenTUI no-flicker gate | success |
| Remind on force-push | success |
| assign | success |
| authorize | success |
| delay-automatic-review | success |
| fallback-comment | success |
| label | success |
| review-pr | failure |
| ack-review-request | skipped |
| precheck-pr | skipped |
| publish-resolution | skipped |
| resolve-pr | skipped |
| review-config | skipped |
Two notes on that table, outside the region so they survive the finalize rewrite. The review-pr failure is the automated code-review job: it ran 2026-09-08 10:41:04 → 16:41:50 UTC, six hours and forty-six seconds, i.e. it exhausted its 21600-second budget, and the fallback comment posted at 16:41:37 says so verbatim. So head has had no automated code review either. The tui-parity success is real but narrow — it pins ink-versus-opentui snapshot parity, and nothing in this diff touches a rendered TUI surface.
Sandboxed verification — this is the new evidence, and it is what changes the verdict. A /verify round completed against this exact head (f480b3f3) in an isolated token-free container and posted its report at 03:06 UTC. It is advisory evidence, not a review or a CI check, but it drove the real compiled CLI as a child process against a real supervisor over its real unix socket with no mocks on the path under test, and it A/B'd against the base build:
- Central claim holds: 68/68. Base has no such command; head reaches the supervisor, resolves a unique prefix, refuses an ambiguous one (
Agent View session id aaaa is ambiguous. Use a longer id.), relays an unknown id and a not-waiting session verbatim with no stack, stops a seeded session with exit 0 and empty stderr, and with no supervisor reachable exits 1 having spawned zero processes. A liveness control proves the base arm is a working binary, not a broken build. - Compile evidence exists now. Four successful
packages/clibuilds — head by CI, base and the candidate fix and the restore by the verify round, allTSC_EXIT=0. This is the typecheck evidence the PR body says your machine could not produce. - Your own cited gate, run at head:
npx vitest run src/commands/sessions.test.ts src/commands/sessions/ src/agent-view/ src/cli.test.ts→ 22 files / 566 tests passed, 14.04s. Gate liveness proven by three planted source mutations each turning a named test red. - Untrusted-text defence: 59/59, zero leaks. 18 payloads walked through the real
peekpath —ESC[2J, OSC window title, OSC 52 clipboard write, cursor hide, device-status request, bidi RLO/LRM/RLM, LF and CRLF forging a fakeAnswer it with: … deadbeef "pwned"continuation at column 0, CR, TAB, VT, FF, NEL,U+2028,U+2029, NUL. No raw control byte and no forged hint line survives. Scaling is flat: 100× input growth moves wall clock by noise (1324 / 1157 / 1246 ms at 2 k / 20 k / 200 k padding), stdout a constant 380 bytes. - 233 of 247 harness assertions passed. Of the 14 failures, 6 are attributable to this PR (the two Criticals above, 4 and 2 cells) and 8 are harness-predicate or pre-existing-environment outcomes, each attributed by measurement —
--yoloshapes proven identical on both arms, so not this PR's. Nothing is left as an unexplained red.
Not verified, explicitly:
- Repo-wide lint and a standalone
tsc --noEmitwere never run as gates anywhere. Compile evidence is the fourpackages/clibuilds above, which is not the same thing. ci.yml's own lanes have never run on any commit of this PR, because of the base branch.- Test Plan steps 1 and 3 —
qwen --bg "…"reachingneeds input, andansweractually resuming a live session — need model credentials nobody had. These are the plan's load-bearing end-to-end claims and they remain unperformed. Steps 2, 4, 5 and 6 were performed in shape or in full. - Windows and macOS are unexercised (Linux container only). That matters mildly for
rawAnswerTail, which readshideBin(process.argv)directly. - No trial merge into current
main, so what lands after the stack merges is unverified. - Per-commit attribution was not possible: the checkout was depth-2, so only the merge commit was reachable and the aggregate 16-file diff is what was verified.
- PR text was treated as untrusted input throughout; nothing resembling an injection was observed in the title, body, commit messages or code comments.
I did not build or run anything from this PR in this session — this is an unattended CI re-run, so my own pass is static and every executed number above comes from that isolated verify lane, whose report I read as evidence and whose two Critical findings I then confirmed by reading the cited lines at f480b3f3 myself.
Sandboxed verification has now run, so the question is what a re-run would settle rather than whether to run one. @qwen-code /verify again after the anchoring fix is the lane that closes this: the specific claim it would settle is that qwen --debug sessions answer <id> please --help me and qwen --proxy <v> sessions answer <id> rerun -v now both deliver their text verbatim to the supervisor, which is not observable from the diff, is not observable from the current suite (the fix leaves 144 tests green), and is currently false at head. @qwen-code /tmux remains the better lane for Test Plan steps 1-6 as a walkthrough, though it will hit the same credential wall on steps 1 and 3. You have write access, so neither lane is gated, and neither needs a maintainer to sponsor it.
中文说明
代码评审
我在打开 diff 之前先写了自己的方案,和上一轮一样:sessions/ 下三个薄的子命令模块、一个共享的「连接或报告无 supervisor」辅助函数、状态行取自 deriveAgentViewPresentation、净化逻辑复用 ps。managed-control.ts 就是这个样子,而且写得不错 —— 241 行、自身不做 I/O、supervisor 连接注入因此每个判断都能在没有 socket 的情况下被测试、用 isPeekResponse 守住一个 unknown 的 IPC 回复而不是直接断言、并在 Doing: 一行会重复它所派生出的标题时将其抑制。isSessionAnswerable 去读 answer 路径真实的拒绝模型、而不是从等待状态重新推导,是正确的直觉;把单行净化配方提升为 sanitizeSingleLineTerminalText 而不是复制 ps.ts 的本地辅助函数,也是正确的选择 —— 而且这一点是被测量的、不是我推断的:一个带 ESC[2J 与 bidi RLO 的敌意 roster displayName,加上一个塞满 TAB 的 cwd,在 head 与 base 上渲染得逐字节相同,因此这个共享净化的重构是无回归的。我重新核对了它依赖的那些签名在这个 commit 上确实能解析。
有三处我会要求处理。前两处是 Critical,都是在真实编译产物上测出来的;在转述任何一条之前,我自己在 f480b3f3 上读过被引用的那些行。
1 —— Critical:sessions answer 之前的任何根级全局参数都会让答案分隔符失效,此时答案里的 help token 会打印帮助并以 0 退出、回答被丢弃。
insertAnswerTextSeparator 锚在固定下标上(if (argv[0] !== 'sessions' || argv[1] !== 'answer') return argv;),而 config.ts:585 是对 rawArgv = hideBin(process.argv) 调用它的。前面放一个 --debug,argv[0] 就不再是 sessions,分隔符永远不会被插入。yargs 随后看到那个 help token —— 而 forgetInheritedOptions 是故意把 help/h 留在已知集合里的,因为裸 --help 必须能用 —— 于是它打印子命令帮助并退出。handler 根本没有运行,也就意味着 rawAnswerTail 没有任何机会去补救。实测:--debug、-d、--bare、--safe-mode、--insecure 下 20 个带前缀单元格中有 15 个丢失答案;无前缀是 4/4。退出码 0、supervisor 调用 0 次、stdout 上是看起来合理的输出。
影响面才是它算 Critical 而不是小毛病的原因:每一个布尔型或取值型根级全局参数,都会以 CLI 接受、文档也在用的形状出现在子命令之前(qwen --debug …)。一个写着 qwen --debug sessions answer "$ID" "$TEXT" && notify 的包装脚本,会在什么都没送达的情况下报告成功。而且它和早前那个目标错位的 bug 一样残忍 —— 用户发现 answer 行为不对时会加 --debug 去查,而加 --debug 本身就是把它弄坏的那一步。
边界也要说清楚,因为这关系到严重度的判断:答案绝不会被送达到错误的 session(根本不会发起调用),而且文档里的 -- 逃生口能逐字节救回所有带前缀的形状。
2 —— Critical:取值型根级全局参数会重新启用 version 拦截,静默丢弃一个含 -v 的答案。
cli.ts:283-315 是靠数位置参数来识别这条命令链的。BASE_VALUE_FLAGS(cli.ts:132)只为覆盖六个 flag 的十一个 token 跳过取值槽 —— --model/-m、--fallback-model、--prompt/-p、--prompt-interactive/-i、--output-format/-o、--resume/-r —— 而且是为了保持与 base 的一致性而刻意为之。对其他任何取值型全局参数,它的值是一个非短横线 token,于是被当成第 1 个位置参数,firstPositional 变成那个值,sessions/answer 这一对永远不会被识别,inSessionsAnswerTail 保持 false,答案里的 -v 就返回了一个下标。版本号被打印,答案消失,退出码 0。实测:15 个带前缀单元格中 14 个丢失;唯一成功的是布尔型的 --debug。对照组让这个数据可信 —— 那十一个 BASE_VALUE_FLAGS 拼写9/9 全部送达,说明豁免机制本身是有效的,失败的是对命令链的识别。
归因是测量出来的、不是假设的:--proxy <v> 与 --telemetry-target <v> 都能让 sessions ps 以 0 退出并渲染出表格,在 head 与 base 上完全相同,因此这两个全局参数本身没有坏。这也不是回归 —— base 没有 sessions answer,而拦截本身对其他所有命令链都是正确 fail-closed 的,在 head 上被重新测量为 12/12(mcp remove victim -v help、sessions -v、-v sessions answer <id> x、--version --bg 等仍然都只打印版本、不执行任何东西)。
两条发现有同一个根因和同一个修法。rawAnswerTail 已经通过 findRun 锚在 token run 上,这正是它能扛住前缀的原因;另外两套机制一个锚在固定下标、一个锚在序号计数,这正是它们扛不住的原因。候选修复是被应用、编译并测量过的,不是目测的 —— 让 insertAnswerTextSeparator 锚在 findRun(argv, ['sessions','answer']) 上(三个 hunk),并让 versionTokenIndex 靠相邻 token 对而非位置来识别命令链(两个 hunk,同时删掉不再被读取的局部变量,否则 noUnusedLocals 会让构建失败)。缺陷 A 从 5/28 → 20/20(非 --yolo 的带前缀单元格),缺陷 B 从 1/15 → 15/15,BASE_VALUE_FLAGS 对照组保持 9/9,cli.test.ts 现有的钉子全部仍然成立,两侧套件都是 144/144。之后源码被还原、普查被重跑,以证明缺陷确实回归。
3 —— Suggestion:测试套件看不见这个轴,因此即便将来 CI 生效,这两条也不会被 CI 抓到。 这是即使修复落地之后我也不希望跳过的一部分。对两个缺陷同时应用候选修复,144 个测试依然全绿 —— 与 head 无法区分。原因只有一个 argv token 那么宽:control-commands.test.ts 里的 parseWithRootOptions 构造的 argv 是从 sessions 开始的(它必须如此,因为它要喂给 insertAnswerTextSeparator),而 withRawArgs 又用同一个数组设置 process.argv。这个 fixture 对无前缀形状忠实地镜像了 config.ts,却从未构造过带前缀的形状 —— 尽管它已经把 --debug、-d、--proxy 注册成了根级全局参数。所以修复应当连同钉住这个轴的 fixture 一起发布:用 ['--debug','sessions','answer',<id>,'please','--help','me'] 驱动真实的命令链,并断言对端收到的 text === 'please --help me'。
另外两件较小的事,都属于描述而非代码:
4 —— Test Plan 第三次与代码脱节,而且这次数字是被测量过的。 第 5 步要求评审者确认三个命令都打印 No background sessions are running;这个字符串在仓库里任何地方都不存在。真实措辞在两个 arm 上都被重新验证过,是 stderr 上以 No background supervisor is reachable, so there is nothing to show. 开头的三行。第 5 步的实质是确认了的 —— 退出码 1、零进程 spawn、没有 supervisor.json、指向 --bg。「21 个文件、470 个测试通过」在 head 上实测为 22 个文件 / 566 个测试;「managed-control.test.ts 中新增 13 个」实测那个文件里有 24 个测试。另外,文档把 -- 逃生口限定为「以短横线开头」的答案,但真正会坏的是中间的 token(rerun -v now、please --help me),而且文档与 --help 文本都没有提到「子命令前带根级全局参数」会改变结果 —— 因此严格照着那句话做的用户,恰恰在需要 -- 的场景下不会去用它。无论两条 Critical 是否修复,一行字就能补上这个缺口。
5 —— 一处我该向你自己做的订正。 上一轮告诉你「第 3、4 层中有一层今天是冗余的」,依据是 195fa468 上的第 9 轮探针。在 head 上这个说法不准确,而且是往对你不利的方向不准确。变异矩阵分别钉住了第 2、4、5 层:分隔符变恒等会让两个具名测试变红,rawAnswerTail 强制回退到 yargs 会让一个变红,移除 version 豁免会让一个变红。三者都是承重的、都有测试。只有第 3 层在产品路径上没被读到,而这一点我是靠阅读确认的、不是靠探针:handler 的 raw?.text ?? (argv.text ?? []).join(' ') 在一次 CLI 调用中永远到不了 argv.text,因为只要 argv._ 非空且 findRun 定位到那段 run,rawAnswerTail 就会返回值,而且它自己就会剥掉 --。rawAnswerTail 在比套件所钉住的更多形状上是承重的,这是纵深防御,不是死代码。所以:要删的是一份重复的折叠,而不是要拆解一个四层纠缠 —— 而发现 1 与 2 里的锚定不对称,才是真正产生代价的地方。
(时序图与文件清单见上方英文部分,此处不重复。)
测试
这一节在十二小时前写的是「这个 head 没有测试证据」。那句话现在是错的,下面是取代它的内容。
PR 自身的 CI —— 仍然结构性缺席。 .github/workflows/ci.yml 把 pull_request 过滤到 main 与 release/**(第 21-24 行)。本 PR 的 base 是 feat/agent-view-bg-dispatch,所以 test、lint_and_static 与类型检查这几条 lane 从未针对它被调度过 —— 不是变红,而是从未被触发。在这个 head 上唯一运行过的 pull_request 事件工作流是 tui-parity。表中其余都是机器人编排任务。
关于上表有两点说明(写在区域标记之外,以便在 finalize 重写后仍然保留)。review-pr 的失败是自动代码评审任务:它运行于 UTC 2026-09-08 10:41:04 → 16:41:50,六小时零四十六秒,也就是耗尽了 21600 秒的预算,而 16:41:37 发布的 fallback 评论逐字说明了这一点。因此 head 也没有经过自动代码评审。tui-parity 的 success 是真实的,但覆盖面很窄 —— 它钉的是 ink 与 opentui 的快照一致性,而本 diff 没有触及任何渲染出来的 TUI 表面。
沙箱验证 —— 这是新证据,也是改变结论的那一部分。 一轮 /verify 针对这个确切的 head(f480b3f3)在隔离、无凭证的容器中完成,并在 UTC 03:06 发布了报告。它是评审证据,不是一次评审、也不是一项 CI 检查;但它驱动的是真实编译出的 CLI(作为子进程、精确 argv 数组、不经过 shell),对端是真实的 supervisor 进程(走真实 unix socket),被测路径上没有 mock,并且与 base 构建做了 A/B:
- 中心主张成立:68/68。 base 没有这些命令;head 能连到 supervisor、解析唯一前缀、拒绝有歧义的前缀(
Agent View session id aaaa is ambiguous. Use a longer id.)、原样转述未知 id 与「未在等待输入」的 session 且不带堆栈、以退出码 0 且 stderr 为空停掉一个已播种的 session,并在没有 supervisor 可达时以 1 退出且零进程 spawn。一个存活性对照证明 base arm 是一个可用的二进制,而不是一个坏掉的构建。 - 编译证据现在有了。 四次成功的
packages/cli构建 —— head 由 CI 构建,base、候选修复与还原由验证轮构建,全部TSC_EXIT=0。这正是 PR 正文说你的机器无法产出的那份类型检查证据。 - 你自己在描述里引用的那条门禁命令,在 head 上运行:
npx vitest run src/commands/sessions.test.ts src/commands/sessions/ src/agent-view/ src/cli.test.ts→ 22 个文件 / 566 个测试通过,14.04 秒。门禁的活性由三处植入的源码变异证明,每一处都让一个具名测试变红。 - 不可信文本防御:59/59,零泄漏。 18 种载荷走过真实
peek路径 ——ESC[2J、OSC 窗口标题、OSC 52 剪贴板写入、隐藏光标、设备状态请求、bidi RLO/LRM/RLM、用 LF 与 CRLF 在第 0 列伪造一行假的Answer it with: … deadbeef "pwned"续行、CR、TAB、VT、FF、NEL、U+2028、U+2029、NUL。没有任何原始控制字节、也没有任何伪造的提示行存活下来。规模扩展是平的:输入增长 100 倍,墙钟时间的变化在噪声范围内(2 k / 20 k / 200 k 填充下分别为 1324 / 1157 / 1246 毫秒),stdout 恒为 380 字节。 - 247 条 harness 断言中 233 条通过。 14 条失败中,6 条可归因于本 PR(即上面两条 Critical,分别 4 与 2 个单元格),8 条是 harness 谓词或既有环境的结果,每一条都由测量归因 ——
--yolo形状被证明在两个 arm 上完全相同,因此不属于本 PR。没有一条被留作无法解释的红。
明确未验证的部分:
- 仓库级 lint 与独立的
tsc --noEmit在任何地方都从未作为门禁运行过。编译证据是上面那四次packages/cli构建,这与类型检查门禁不是一回事。 ci.yml自己的 lane 从未在本 PR 的任何 commit 上运行过,原因是 base 分支。- Test Plan 的第 1、3 步 ——
qwen --bg "…"走到needs input,以及answer真正让一个存活 session 恢复 —— 需要谁都没有的模型凭据。这两步是该计划中承重的端到端主张,仍未被执行。第 2、4、5、6 步已按形状或完整执行。 - Windows 与 macOS 未被验证(仅 Linux 容器)。这对
rawAnswerTail有轻微影响,因为它直接读取hideBin(process.argv)。 - 没有做与当前
main的试合并,因此栈合并之后真正落地的东西未经验证。 - 逐 commit 归因不可行:checkout 是 depth-2 的,因此只有那个 merge commit 可达,被验证的是聚合的 16 文件 diff。
- PR 文本全程被当作不可信输入处理;在标题、正文、commit message 与代码注释中都没有观察到任何类似注入的内容。
我在本次会话中没有构建或运行本 PR 的任何东西 —— 这是一次无人值守的 CI 重跑,因此我自己这一轮是静态的,上面每一个被执行出来的数字都来自那条隔离的验证 lane。我把它的报告当作证据来读,然后自己在 f480b3f3 上读过被引用的行,确认了那两条 Critical 发现。
沙箱验证现在已经跑过了,所以问题是重跑能确定什么,而不是要不要跑。在锚定修复之后再来一次 @qwen-code /verify,是能收口这件事的 lane:它能确定的具体主张是 qwen --debug sessions answer <id> please --help me 与 qwen --proxy <v> sessions answer <id> rerun -v now 都能把文本逐字送达 supervisor —— 这一点从 diff 看不出来,从当前套件也看不出来(修复之后 144 个测试全绿),而在 head 上它目前是假的。对 Test Plan 第 1-6 步的走查而言,@qwen-code /tmux 仍然是更合适的 lane,不过它会在第 1、3 步撞上同一堵凭据墙。你有写权限,所以两条 lane 都不受限,也不需要 maintainer 代为发起。
— Qwen Code · qwen3.8-max-2026-09-02
Reviewed at f480b3f3c4dfc68ffa25c9eaf20c96c350a0b582 · re-run with @qwen-code /triage
|
Confidence: 2/5 — unchanged from the last pass, and unchanged for the same reason: 🛑 Not approving. No new review submitted either — the Stepping back over the whole picture rather than the diff, since that is what this stage is for: Nothing changed, so I re-checked my own reasons instead of restating them. The head is byte-identical to the one the last pass reviewed and the My independent proposal, and how the PR compares. Read from the title and the motivation alone, what I would have written is close to Is the problem real? Yes, and I checked rather than accepted the framing. Would I curse whoever wrote this in six months? For The fixture has to ship with the fix. This is the part I would least want dropped, because it is the part that makes the next round observable. Applying the fix leaves every test green — indistinguishable from head — because New this pass: the stack has drifted, and it now costs something concrete. The base is 34 ahead and 252 behind Am I being a pushover, or am I being worn down by volume? The honest check: this PR is eleven rounds old, AGENTS.md says land only Critical fixes past roughly five, and the two things I am holding the gate on are Critical — silent success-direction failures on a command that mutates a running session, one of them triggered by the No deferred-approval marker in this comment, and there is nothing to defer to: I counted pending workflow runs with No @mention, and that is a decision rather than an omission. The shortest path to a 4/5 from here is one commit: settle the three anchors on 中文说明信心度:2/5 —— 与上一轮相同,而且理由相同: 🛑 不批准。同时也不提交新评审 —— 上一轮钉在 跳出 diff 看整体,因为这正是本阶段的用途: 什么都没变,所以我重新检查了自己的理由,而不是把它们复述一遍。 head 与上一轮评审、 我的独立方案,以及这个 PR 与之相比如何。 只看标题与动机,我会写出的东西与 问题真实吗? 真实,而且我是查过的、不是接受它的框定。今天的 六个月后我会不会骂写这段代码的人? 对 fixture 必须与修复一起发布。 这是我最不希望被丢掉的部分,因为它是让下一轮变得可观测的部分。应用修复之后所有测试依然全绿 —— 与 head 无法区分 —— 因为 本轮新增:这个栈已经漂移,而且现在开始产生具体代价。 base 领先 我是在当软柿子,还是被数量磨平了? 诚实的自检:这个 PR 已经十一轮之老,AGENTS.md 说超过大约五轮之后只落地 Critical 修复,而我用来拦住它的两件事正是 Critical —— 在一个会改变运行中 session 状态的命令上朝成功方向的静默失败,其中一条的触发条件是用户在命令行为不对时恰恰会去按的 本条评论里没有延迟批准的标记,而且也没有什么可以延迟等待:我数了这个 head 上 没有 @mention,这是一个决定而不是遗漏。 从这里到 4/5 的最短路径是一个 commit:把三个锚点都定在 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Three specific fixes before this should merge — details and the evidence for each are in my review comment above.
clean()inmanaged-control.tsstrips\r(already handled bysanitizeTerminalText) but keeps\nand\t, whichsanitizeTerminalTextpreserves deliberately.peekis a one-field-per-line render site, so model-authoredwaitingFor/summary/lastResultcan inject forged lines — including a fakeAnswer it with:hint.ps.tsstrips exactly[\t\n]on top of the shared helper for this reason; mirror it and add a\ncase to the sanitization test.deriveAgentViewPresentation({ state, activity })dropsrosterEntryandlaunch, so thelaunch.initialPromptfallback inderiveTitleis unreachable. When the summary is generic,sessions psshows the--bgprompt whilepeekshowsUntitled sessionfor the same session; when the summary is real,peekprints it twice (title andDoing:). Either carry them through the supervisor'speekresult —listalready returns full snapshots — or reuseAGENT_VIEW_UNTITLED_TITLEand fall back to the session id asmanaged-rows.tsdoes.- The docs say the id to type is "the short form
qwen sessions psand--bgprint", butps's human table has no id column and--bgprints the full UUID. Point atsessions ps --json/sessionId, or add the id to the table in the parent PR.
None of this is a design objection — the shape is right and the supervisor-op reuse is the correct call. Also note that ci.yml filters pull_request to base main / release/**, so no lint, typecheck or unit signal runs on this PR while its base is a feature branch; @qwen-code /tmux or @qwen-code /verify would supply it.
中文说明
合并之前有三处具体修改 —— 每一条的细节与依据都在上面的审查评论里。
managed-control.ts里的clean()去掉了\r(sanitizeTerminalText本来就已经处理了它),却保留了\n和\t,而这两者是sanitizeTerminalText刻意保留的。peek是「每个字段一行」的渲染场景,因此模型自己写的waitingFor/summary/lastResult可以注入伪造的行 —— 包括一条假的Answer it with:提示。ps.ts正是为此在共享 helper 之上去掉[\t\n];照它做,并在净化测试里补一个\n用例。deriveAgentViewPresentation({ state, activity })丢掉了rosterEntry与launch,于是deriveTitle里launch.initialPrompt那条兜底永远走不到。当 summary 是通用短语时,同一个 session 在sessions ps里显示--bg的 prompt,在peek里却显示Untitled session;当 summary 是真实描述时,peek会把它打印两遍(标题与Doing:)。要么把这两个字段带进 supervisor 的peek返回 ——list本来就返回完整 snapshot —— 要么复用AGENT_VIEW_UNTITLED_TITLE并像managed-rows.ts那样回退到 session id。- 文档说可键入的 id 是「
qwen sessions ps和--bg打印的短形式」,但ps的人类可读表格没有 id 列,--bg打印的是完整 UUID。请指向sessions ps --json/sessionId,或者在父 PR 里给表格加上 id。
这些都不是对设计的反对 —— 整体形态是对的,复用 supervisor 操作也是正确的选择。另外请注意:ci.yml 把 pull_request 限定在 base 为 main / release/**,所以只要本 PR 的 base 还是 feature 分支,就不会有任何 lint、类型检查或单测信号;@qwen-code /tmux 或 @qwen-code /verify 可以补上这个信号。
— Qwen Code · qwen3.7-max
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory.
中文说明
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory。
— qwen3.8-max via Qwen Code /review (v0.23.0)
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
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
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline. 1 Suggestion-level finding(s) could not be anchored to a changed line and were dropped; nothing further to act on here.
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28185 passed.
Deferred under the convergence posture (round 2, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/sessions/control-commands.ts:100 — [review] stop/answer handlers write results without ignoreBrokenPipe(); a departed reader turns a completed mutation into a crash-class exitpackages/cli/src/commands/sessions/control-commands.ts:83 — [review] the three new command builders are never executed in a test: positional names, types, demandOption unpinned
中文说明
已审查——无阻断问题。 建议见行内评论。 1 条建议级发现无法锚定到改动行,已丢弃;此处无需进一步处理。
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28185 passed。
收敛姿态下延后(第 2 轮,非阻断)——已记录,本轮不要求修改:共 2 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.23.0)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers. Suggestions are inline.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- stop/answer handlers write results without ignoreBrokenPipe() — already recorded in the round-2 deferred list (review 5108365153)
- docs promise a short id on surfaces that print none (commands.md:856) — already reported as R1-3 in round 1 (comment 3928436687), author acknowledged, still open
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28197 passed.
Deferred under the convergence posture (round 3, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/sessions/managed-control.ts:143 — [review] peek prints the raw state token while ps prints the display label for the same statepackages/cli/src/commands/sessions/control-commands.ts:99 — [review] stopCommand.handler is never exercised by any testpackages/cli/src/commands/sessions/managed-control.ts:143 — [review] '(no live process)' annotation is keyed to the PTY-host registry, not process livenesspackages/cli/src/commands/sessions/managed-control.ts:140 — [review] peek prints the Untitled placeholder verbatim while ps substitutes the session id
Convergence: round 3 posted 11 inline comment(s), 1 of them reported for the first time; the previous round posted 11 (4 new). Findings keep coming back to the same files: packages/cli/src/commands/sessions/managed-control.ts (findings in rounds 1, 2; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查——无阻断问题。 建议见行内评论。
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28197 passed。
收敛姿态下延后(第 3 轮,非阻断)——已记录,本轮不要求修改:共 4 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 3 轮发布了 11 条行内评论,其中 1 条是首次提出;上一轮发布了 11 条(其中 4 条首次提出)。发现反复回到同一批文件:packages/cli/src/commands/sessions/managed-control.ts(第 1、2 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.23.0)
`qwen sessions ps` prints no id column in its human table and `--bg` prints the full UUID, so the docs sentence promising "the short form `qwen sessions ps` and `--bg` print" described surfaces that do not exist. Reword to cite what actually prints a usable id — the full id at `--bg` launch and `sessionId` from `qwen sessions ps --json` — and correct the `shortSessionId` comment that repeated the claim: the eight-character handle works because ids resolve by unique prefix, not because any surface prints it. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtn1pqbuir
The gate comment claimed answering a non-waiting session queues a prompt; `queueAnswerForSessionLocked` throws `is not waiting for input.` instead. Word both the gate comment and the matching test comment after what the supervisor actually does. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtn1pqbuir
Three branches were asserted by nothing: - isPeekResponse's rejection path, which a weakened guard would let flow into the presentation code; a malformed reply (no `state`) now pins the one-line refusal. - answerManagedSession's NO_SUPERVISOR guard, exercised only through peek until now. - stopManagedSession's identical NO_SUPERVISOR guard. Each test turns red if its guard is deleted or the guard weakened. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtn1pqbuir
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed. Suggestions are inline.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R2-2 hand-written roster↔session join in the peek handler (supervisor-process.ts:1128) — still stands, already reported in round 2 (comment 3930137598); the file is outside this round's incremental diff, so no inline anchor
- R2-3 peek handler's rosterEntry join and redacted launch field pinned by no producer-side test (supervisor-process.ts:1131) — still stands, already reported in round 2 (comment 3930137605); the file is outside this round's incremental diff,…
Not explored to full depth (tool budget reached): "agent 1b": none — no check was cut short.; "agent 1d": none — no Budget gap: lines, all planned checks completed..
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory.
Deferred under the convergence posture (round 4, not a blocker) — recorded, not requested in this round:
packages/cli/src/commands/sessions/managed-control.test.ts:259 — [probe] D4-1 empty-answer refusal message 'An answer cannot be empty.' asserted nowhere — mutant 'Answer delivered.' survives (deferred under the round-2-5 code-age rule: anch…
Convergence: round 4 posted 6 inline comment(s), 1 of them reported for the first time; the previous round posted 11 (1 new). Findings keep coming back to the same files: packages/cli/src/commands/sessions/managed-control.ts (findings in rounds 1, 2; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push, or dropping this PR's reviews to --severity-floor critical, keeps the loop from re-deriving the same set. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
已审查。 建议见行内评论。
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未探索到全部深度(达到工具调用预算):"agent 1b":none — no check was cut short.;"agent 1d":none — no Budget gap: lines, all planned checks completed.。
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory。
收敛姿态下延后(第 4 轮,非阻断)——已记录,本轮不要求修改:共 1 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 4 轮发布了 6 条行内评论,其中 1 条是首次提出;上一轮发布了 11 条(其中 1 条首次提出)。发现反复回到同一批文件:packages/cli/src/commands/sessions/managed-control.ts(第 1、2 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,或将本 PR 的评审降到 --severity-floor critical,可以避免循环反复推导同一组发现。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.23.0)
The recipe that keeps one-line output unforgeable — sanitizeTerminalText plus the TAB/LF strip — was hand-duplicated between the sessions ps renderer and the managed session control renderer. Extract it as sanitizeSingleLineTerminalText next to the shared helper and use it from both, so the next policy change lands in one place. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtnaadh9j8
The message claimed no background sessions are running when the code only knows the supervisor socket is unreachable; detached workers keep their state on disk, so `qwen sessions ps` may still list them. Say the supervisor is not reachable, point at `qwen sessions ps` for what is on record, and keep the commands connect-only. Updates the tests that pinned the old wording. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtnaadh9j8
The comparison matched clean()'s untrimmed 300-cell output against a title derived trimmed and cut at 200 cells, so padded or over-long summaries printed twice again. Canonicalize the summary the way the title was derived from it before comparing, and pin both shapes. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtnaadh9j8
The Last: branch had no fixture supplying activity.lastResult, and at this layer the answer success message was asserted by nothing; deleting either stayed green. Assert the exact peek lines for a working session and 'Answer delivered.' by exact equality, mirroring the stop test. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtnaadh9j8
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Reviewed — no blockers.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R5-1 stopCommand.handler is never exercised by any test — already recorded in the round-3 deferred list (review 5109340440)
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28201 passed.
3 Suggestion(s) were drafted inline past the resolved critical posting floor — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s); the CLI moved them into the deferral list below (floor enforcement).
Deferred under the convergence posture (round 5, not a blocker) — the floor engaged early: the first-time-finding rate has not fallen for 2 consecutive round(s) — recorded, not requested in this round:
packages/cli/src/commands/sessions/managed-control.ts:161 — [review] R1-14: (fix-induced) The dedupe comparison added in cbebd09c5 closed the reported input (it compared two differently-transformed strings), but the new comparison re-derive…packages/cli/src/commands/sessions/managed-control.ts:171 — [review] R1-13: Still stands (open since round 1, not addressed in this round's delta): the answer-hint gate re-derives answerability from taskState === 'waiting' alone and misse…packages/cli/src/commands/sessions/control-commands.ts:83 — [review] R1-17: Still stands (open since round 1, not addressed in this round's delta): the free-text answer <session> <text> positional has no protection against dash-prefixed a…packages/cli/src/commands/sessions/managed-control.ts:145 — [probe] peek prints the session id without the clean() sanitization every sibling field gets — ESC/OSC/bidi bytes in a store-planted session dir name print raw in the header and th…packages/cli/src/commands/sessions/managed-control.test.ts:301 — [probe] the isPeekResponse guard-pin test only exercises the state-absent branch — the 'sessionId' in state clause is never decisive, so deleting it keeps the suite green (def…
中文说明
已审查——无阻断问题。
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28201 passed。
3 条 Suggestion 在已解析的 critical 发布下限之外被起草为行内评论——发布下限因首次发现速率连续 2 轮未下降而提前生效;CLI 已将其移入下方延后清单(下限强制执行)。
收敛姿态下延后(第 5 轮,非阻断)——发布下限因首次发现速率连续 2 轮未下降而提前生效——已记录,本轮不要求修改:共 5 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.23.0)
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Partially reviewed — gaps disclosed.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R5-1 stopCommand.handler is never exercised by any test — already reported (round-3 deferred list, review 5109340440)
- isPeekResponse guard-pin test only exercises the state-absent branch — already reported (round-5 deferred list, review 5117841656)
Not reviewed: reverse audit — did not converge within the reverse-audit round cap of 5.
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28212 passed.
Deferred under the convergence posture (round 6, not a blocker) — recorded, not requested in this round:
packages/cli/src/agent-view/supervisor-process.ts:1156 — [probe] isSessionAnswerable's live-attach, no-live-process and soft-question branches have no producer-side pin — each branch deletable with the suite greenpackages/cli/src/commands/sessions/control-commands.ts:113 — [probe] the -- fold merge branch is never tested with a non-empty positional head — head tokens can be silently dropped while printing 'Answer delivered.'packages/cli/src/agent-view/supervisor-process.ts:1162 — [probe] soft-question branch misses the answer path's readiness gate — stale 'attaching' and blocked processState advertise a guaranteed-refused hintpackages/cli/src/commands/sessions/managed-control.test.ts:341 — [probe] sanitization pin never injects LRM/RLM (U+200E/U+200F) — dropping them from BIDI_OVERRIDE_CHARS_REGEX keeps the whole suite greenpackages/cli/src/commands/sessions/managed-control.test.ts:344 — [probe] sanitization pin exercises only waitingFor/summary — Directory:/Last:/error-reason clean() sites removable with the suite greenpackages/cli/src/commands/sessions/managed-control.test.ts:67 — [probe] the Waiting: label and Answer it with: prefix are unpinned — either removable with the whole suite greenpackages/cli/src/commands/sessions/control-commands.ts:83 — [probe] sessions.test.ts wiring pin still asserts the removed 'answer <session> <text>' signature — self-referential mock keeps it green
中文说明
仅完成部分审查,审查缺口已披露。
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:反向审计——在 5 轮的反审轮数上限内未收敛。
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28212 passed。
收敛姿态下延后(第 6 轮,非阻断)——已记录,本轮不要求修改:共 7 条(原文未翻译,列表见上方英文部分)。
— qwen3.8-max via Qwen Code /review (v0.23.0)
The variadic-tail fix for dash-leading answers only protects unknown options: the root globals registered on the same yargs chain (--debug, --proxy, --telemetry*, ...) stay known inside the answer subtree, so a quoted flag set the option and vanished from the text while the CLI reported success. Forget the inherited options in the answer builder so quoted tokens become unknown again and stay in the text; a bare --help still shows the help. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtnn5c4cjw
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
2 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- R5-1 stopCommand.handler is never exercised by any test — already reported (round-3 deferred list, review 5109340440)
- the -- fold merge branch is never tested with a non-empty positional head — already reported (round-6 deferred list, review 5119010631)
Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at /home/github-runner/actions-runner-hk1-5/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-5--work-qwen-code-qwen-code/subagents/f934e32b-42b2-4832-8582-511566e89625 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-5/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-5--work-qwen-code-qwen-code/subagents/f934e32b-42b2-4832-8582-511566e89625'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at /home/github-runner/actions-runner-hk1-5/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-5--work-qwen-code-qwen-code/subagents/f934e32b-42b2-4832-8582-511566e89625 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-5/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-5--work-qwen-code-qwen-code/subagents/f934e32b-42b2-4832-8582-511566e89625'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory.
Deferred under the convergence posture (round 7, not a blocker) — recorded, not requested in this round; 3 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/commands/sessions/control-commands.ts:130 — [review] Critical [fails-closed] [new-surface] R6-1: (fix-induced) forgetInheritedOptions broke every root-flag position outside the free-text tail — qwen --debug sessions answer …packages/cli/src/commands/sessions/control-commands.ts:88 — [review] Critical [fails-closed] [new-surface] forget strips the session positional's type:'string' — all-digit session tokens (~2.3% of short ids) are coerced to Number and refuse…packages/cli/src/commands/sessions/control-commands.ts:127 — [probe] Critical [fails-closed] [new-surface] --session=zzz/--session/--no-session in the answer re-bind to the session positional in yargs' re-parse — argv.session corrupted to a…packages/cli/src/commands/sessions/control-commands.ts:140 — [review] --text stays a known option (registered after the forget): 'answer <id> a --text b c' silently delivers only 'a'; --session still recognizedpackages/cli/src/commands/sessions/control-commands.ts:130 — [review] answer --help still advertises the twelve forgotten root options — a user following the advertised --debug hits the R6-1 fix-induced misparsepackages/cli/src/commands/sessions/control-commands.ts:94 — [probe] unquoted --help/-h mid-answer is intercepted by yargs (exit 0, no delivery) while the describe promises only a bare --help shows help — wording fixpackages/cli/src/commands/sessions/control-commands.test.ts:137 — [probe] the round's regression tests only place root flags inside the text tail — the one position that parses correctly — so no test can witness the R6-1 fix
Convergence: round 7 posted 1 inline comment(s), 1 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/cli/src/commands/sessions/control-commands.ts (findings in round 6; 1 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (1 Critical(s)), the rate of first-time findings is not falling (this round 1, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
本轮确认的 2 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查:覆盖情况——无法读取 agent 的运行记录(no subagent transcripts at /home/github-runner/actions-runner-hk1-5/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-5--work-qwen-code-qwen-code/subagents/f934e32b-42b2-4832-8582-511566e89625 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-5/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-5--work-qwen-code-qwen-code/subagents/f934e32b-42b2-4832-8582-511566e89625'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.),本次运行无法证明 diff 的任何部分被读过。
未审查:验证——无法检查步骤 4 与步骤 5 是否运行(no subagent transcripts at /home/github-runner/actions-runner-hk1-5/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-5--work-qwen-code-qwen-code/subagents/f934e32b-42b2-4832-8582-511566e89625 (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-5/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-5--work-qwen-code-qwen-code/subagents/f934e32b-42b2-4832-8582-511566e89625'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.)。
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory。
收敛姿态下延后(第 7 轮,非阻断)——已记录,本轮不要求修改;其中 3 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface:未认证任何错误结果,且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中:共 7 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 7 轮发布了 1 条行内评论,其中 1 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:packages/cli/src/commands/sessions/control-commands.ts(第 6 轮已出过发现,本轮又有 1 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已解析为 critical 发布下限。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 1 条 Critical),首次发现的速率没有下降(本轮 1,上一轮 1),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.23.0)
`.version(false)` up the sessions chain deletes `version` from the key/type option groups but leaves the alias entry `v: ['version']` behind, and forgetInheritedOptions both kept `version`/`v` known and derived the inherited set from `table.key` alone — so a quoted `-v` was still parsed out of an answer (eating the token after it) and an answer of only `--version` was consumed as a flag and refused as empty. Forget the version option like the rest of the root globals: keep only the documented bare `--help` carve-out, and derive the inherited set from every group in the options table so alias leftovers are forgotten too. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtnvpz8zkc
|
Qwen Code review did not complete successfully. The review pipeline failed before a review could be posted. A transient error is retried automatically; if you are seeing this, retry with |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 124 passed · 6 failed · 130 total Flakiness gate: ✅ 5 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:124 通过 · 6 失败 · 130 总计 抖动门:✅ 5 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10949 deep verification —
|
| # | Cell | Oracle | HEAD | BASE |
|---|---|---|---|---|
| 1 | sessions peek <short> |
exit + stderr | 0, report on stdout | 1, usage error listing only list/ps |
| 2 | peek seeded session |
stdout fields | title + [aaaa1111], Waiting: relayed, no hint |
n/a (no command) |
| 3 | peek blocking-wait session |
hint suppressed | no Answer it with: |
n/a |
| 4 | peek <ambiguous prefix> |
exit + stderr wording | 1, supervisor's own ambiguity wording, no stack | n/a |
| 5 | peek <unknown id> |
stderr verbatim | 1, No Agent View session found for <id>. |
n/a |
| 6 | all three, no supervisor | exit + wording + spawn count | 1, 3-line report on stderr, supervisor.json never created, supervisor-process count unchanged |
1 (surface absent) |
| 7 | stop <id> |
exit + single stream | 0, Stopped. on stdout, stderr empty |
n/a |
| 8 | answer <id> "…" on a session that is not waiting |
refusal not claimed as success | 1, supervisor's wording, never Answer delivered. |
n/a |
| 9 | sessions ps with hostile roster name + cwd |
byte-identical stdout/stderr/exit | identical to base | identical to head |
Cell 9 is the no-regression proof for the shared-sanitizer refactor (ps.ts dropped its local
sanitize() for the new sanitizeSingleLineTerminalText). A ESC[2J- and bidi-laden
displayName renders as the six visible characters evil\u001b[2Jnamewi… — escapeAnsiCtrlCodes
rewrites the escape byte into literal text rather than dropping it, so the sequence is inert but
still legible — and a TAB-laden cwd loses its TAB, byte-for-byte the same on both arms; no
raw ESC or TAB survives into either. See logs/ps-head.txt / logs/ps-base.txt.
Witness: 05-ab-base-has-no-subcommands.png (base arm refusal), 06-global-prefix-shifts-answer-target.png (F1).
A reachable-state limit, measured rather than assumed
A real supervisor cannot be held in needs_input by a fixture: it reconciles a workerless
needs_input session to failed, and peek itself triggers the flip. p1-state-survival.mjs
measures it — the seeded state survives ~1.5 s after startup, is failed/exited by +3 s, and a
re-seed that survives idle is flattened by the very peek call meant to observe it
(logs/ + the probe's stdout). Holding one there needs a live worker, i.e. a model and
credentials this container does not have. So the State: waiting rendering is proven by h3
below, which drives the same compiled CLI against a programmable peer reply, while h1 covers the
contract cells the supervisor genuinely owns. This reproduces the handling, not a live
needs_input session end to end.
h3 — what peek does with a supervisor reply (44/44)
Same real compiled CLI; the peer is the real shipped createAgentViewSupervisorServer in its
own process (real socket, real newline framing, real auth token) returning a programmable peek
reply. Harness h3-peek.mjs, log logs/h3-run2.txt.
| Group | Result |
|---|---|
A. answerable consumption |
8/8 — true offers the hint; false suppresses it while State: waiting and the Waiting: line still print; absent (older supervisor) falls back to the waiting state; live:false adds (no live process) |
B. isPeekResponse guard |
6/6 — {}, null, {state:{}}, {state:null}, a bare string all give exit 1 and one clean line, no TypeError; a thrown supervisor error is relayed verbatim with no stack |
C. Doing: dedupe |
7/7 — suppressed when the title derives from the summary (exact, padded, and 400-chars-longer sharing the first 200 cells); printed when a roster displayName makes the title differ, so the dedupe does not eat real information |
| D. Sanitizer sibling sweep | 17/17, 0 leaks |
| E. Scaling ladder | 3/3, flat |
| F. Resolved id in the hint | 2/2 — typing aaaa or the full id both yield qwen sessions answer aaaa1111 "<your answer>" |
D — the sibling sweep found no open door. redactAgentViewActivity only strips queued-prompt
fields, so hostile waitingFor/summary/lastResult cross the wire raw and the new
sanitizeSingleLineTerminalText is the only defence — which makes it worth walking every
neighbouring shape, not just the reported one. All 17 were neutralized with no raw ESC and no
forged column-0 line: ESC[2J, OSC window title, OSC 52 clipboard write, ESC[?25l,
ESC[6n, bidi RLO U+202E, LRM/RLM U+200E/U+200F, LF and CRLF forging a fake
Answer it with: … deadbeef "pwned" continuation, CR, TAB, VT, FF, NEL U+0085, U+2028,
U+2029, NUL. Two hypotheses I expected to hold did not: CR is already stripped by
BARE_C0_CONTROL_CHARS_REGEX (\x0b-\x1f covers it), and U+2028/U+2029 produce no line
break in the output. Witness 03-sanitizer-sibling-sweep-zero-leaks.png.
E — no scaling problem. The same hostile shape at 2 k / 20 k / 200 k characters through the
real peek path: 2323 ms / 1854 ms / 1995 ms, stdout a constant 625 bytes, no raw ESC. A 100×
input growth moves the wall clock by noise, so there is no superlinear curve to report.
Witness 04-scaling-ladder-flat.png.
Mutation matrix
Each mutation was reverted in a finally block and verified by sha256 plus
git status --porcelain → "" (tree clean). m1-version-mutation.mjs, v1-vacuity.mjs,
v2-v3-recheck.mjs; logs logs/m1-run.txt, logs/v1-run.txt, logs/v23-run.txt,
logs/v2-raw.txt, logs/v3-raw.txt.
| Mutation | Suite / harness | Result | Classification |
|---|---|---|---|
M-version: revert commit 17b1befc in the compiled dist (candidates from table.key only) |
real CLI, 7 rows | 0/7 rows differ | see F2 — inert in the product |
V1: drop answerable consumption, gate the hint on the waiting state alone |
managed-control.test.ts |
RED, 3 × AssertionError: expected '…' not to contain 'qwen sessions answer' |
pinned — the field is load-bearing, the test is not vacuous |
| V2 (positive control, same file): disable the empty-answer guard | managed-control.test.ts |
RED, × refuses an empty answer without calling the supervisor, 1 failed / 23 passed |
control live — the runner really collects and exercises this file |
V3: revert commit 17b1befc in the source |
control-commands.test.ts |
RED, 2 failed / 15 passed: × keeps the version alias in the answer text, × delivers an answer that is only --version |
pinned by the fixture — see F2 |
M-answerable-producer: omit answerable from the supervisor's reply |
h3 group A3 | hint falls back to the waiting state | redundant-defence path works as documented |
M-version and V3 disagree, and the disagreement is the finding. The commit is load-bearing
against its own test fixture (V3 red without it) and unobservable in the shipped binary (M-version
0/7). Both are true because the fixture builds a bare yargs() chain, while the real binary
decides -v/--version in cli.ts before yargs parses anything. V2 is the positive control
that makes V3's red believable, and it lands in the same file as the mutant.
answerable is not a dead switch: it is produced by the real supervisor
(supervisor-process.ts:1135) and read at managed-control.ts; V1 kills three tests and h3-A3
shows the absent-field fallback.
Targeted gates
The PR's own cited command, run at HEAD from packages/cli:
npx vitest run src/commands/sessions.test.ts src/commands/sessions/ src/agent-view/ src/cli.test.ts --coverage.enabled=false
→ Test Files 22 passed (22) Tests 542 passed (542) Duration 63.34s
logs/gate-pr-cited.txt. Per file: control-commands.test.ts 17, managed-control.test.ts 24,
sessions.test.ts 3, supervisor-process.test.ts 87, supervisor-store.test.ts 18.
Gate liveness is proven by V2 (a planted source mutation turns exactly one test of the same file
red). These 542 are the PR's own tests and are not counted in assertions.json, which counts
only this round's 130 harness assertions.
Both arms compile: CI built HEAD before this job, and the base worktree's
npm run build -w packages/cli completed (EXIT=0, 2 m 15 s, tmp/base-build3.log). That is the
typecheck evidence the PR body says it could not produce.
Findings
F1 — Critical: a root global before the subcommand retargets answer at a session named "answer"
node tmp/pr10949-verify-20260905-060743/p2-global-prefix.mjs # recorded peer, exact wire bytes
node tmp/pr10949-verify-20260905-060743/p3-real-bound.mjs # bounded against a real supervisoranswer's positionals shift by one whenever any root global precedes the subcommand:
| argv | wire sessionId |
wire text |
exit |
|---|---|---|---|
sessions answer aaaa1111 after a global flag |
aaaa1111 |
after a global flag |
0 |
--debug sessions answer aaaa1111 after a global flag |
answer |
aaaa1111 after a global flag |
0 |
--debug sessions answer aaaa1111 go ahead |
answer |
aaaa1111 go ahead |
0 |
--debug sessions answer aaaa2222 go ahead |
answer |
aaaa2222 go ahead |
0 |
--debug sessions stop aaaa1111 |
aaaa1111 ✓ |
— | 0 |
--debug sessions peek aaaa1111 |
aaaa1111 ✓ |
— | 1 |
The session id the user typed is folded into the answer text and the literal subcommand word
becomes the target. peek and stop are unaffected — only answer, the one command with a
variadic tail.
Cause. forgetInheritedOptions derives "inherited" as every key in every group of the shared
options table except help/h. But sessionPositional(yargs) runs before it in the same
builder chain (forgetInheritedOptions(sessionPositional(yargs))), so the command's own
session key — with its type: 'string' and demandOption: true — is deleted along with the
root globals it was written to forget. text survives only because .positional('text', …) is
chained after the call. With session no longer a declared string positional, its assignment
degrades exactly when the argv reaching the subcommand is offset by a global flag.
Blast radius, bounded. Against a real supervisor this does not write to a wrong session:
qwen --debug sessions answer aaaa1111 "go ahead" returns exit 1 with
No Agent View session found for answer. (p3-real-bound.mjs, logs/), because "answer" cannot
prefix-match a uuid-shaped session directory. So the consequence is a lost answer plus a
misleading error for a valid, documented invocation shape — not a mis-targeted mutation. The
exit-0 / Answer delivered. rows above are what a permissive peer shows; they matter because they
isolate the mis-parse from the supervisor's refusal.
This shape is reachable in normal use, and cruelly so while debugging: a user whose answer fails
adds --debug, which changes the target of the command and produces a different, more confusing
error.
Suggested minimal fix (preserves the commit's intent)
Keep the command's own keys out of the "inherited" set. Registering them after the forget works too,
but naming them is the smaller change and states the invariant:
const keep = new Set(['help', 'h', 'session', 'text']);session and text are this command's own positional names; they are never inherited globals, and
keeping them costs nothing because a user cannot reach them as flags in any shape the docs
advertise. Not applied or measured here — the budget went to establishing the defect. The
fixture that would pin it: drive the real compiled CLI with ['--debug','sessions','answer',<id>,…]
and assert the wire sessionId === <id>, which no current test does (control-commands.test.ts
parses ['sessions','answer',…] on a bare yargs chain with no root global in front).
F2 — Critical: -v / --version anywhere in an answer silently discards it; the commit that claims to fix this is unobservable in the product
node tmp/pr10949-verify-20260905-060743/h2b-blast-radius.mjs # 12-shape matrix + escape hatches
node tmp/pr10949-verify-20260905-060743/m1-version-mutation.mjs # is commit 17b1befc observable?5 of 12 answer shapes lose the answer entirely — exit 0, the CLI version on stdout, and
zero supervisor calls:
argv after sessions answer <id> |
wire | outcome |
|---|---|---|
rerun -v now |
none | version printed, answer lost |
rerun --version now |
none | version printed, answer lost |
--version |
none | version printed, answer lost |
-v |
none | version printed, answer lost |
use --proxy -v |
none | version printed, answer lost |
use --model -v |
use --model -v |
delivered |
-p -v / --resume -v |
delivered | BASE_VALUE_FLAGS value slot |
rerun --version=false now |
delivered | =-form is not an exact match |
-- rerun -v now / -- --version |
delivered | documented escape hatch works |
rerun -d now |
delivered | control: not a version token |
Cause, and it is not in this PR's file. cli.ts routes any exact -v/--version token
appearing before -- to printBootstrapVersion() during bootstrap — before yargs parses anything
(hasVersionToken, and the "Base-parity version intercept" block whose own comment lists
mcp remove victim -v help among the shapes it deliberately catches). That intercept predates this
PR. forgetInheritedOptions operates on the yargs options table and therefore cannot reach it.
What this PR contributes, separately from the pre-existing intercept:
- It adds the first command whose positional payload is arbitrary free text, so the pre-existing
intercept now has a user-facing cost. - Its topmost commit
17b1befc"fix(cli): forget the version flag in sessions answer text"
widensforgetInheritedOptionsto scan every group for thev: ['version']alias, on the stated
grounds that this keeps-v/--versionknown. Reverting exactly that widening in the compiled
output changes 0 of 7 observable CLI rows (m1-version-mutation.mjs, dist restored and
sha256-verified). The widening is inert in the product. - Its two tests —
delivers an answer that is only --versionandkeeps the version alias in the answer text— pass at HEAD (logs/gate-pr-cited.txt, and a verbose re-run listing both by
name) while the shipped binary does the opposite. They are not vacuous in the mutation sense: V3
turns both red when the commit is reverted in source. They pass because the fixture's bare
yargs()chain omits thecli.tsbootstrap route that actually decides the behaviour. The
scenario never reaches the code that governs it.
Bounding the docs. The documented escape hatch is correct and works: qwen sessions answer 0f8e1c42 -- --force and -- rerun -v now both deliver byte-exact. But
docs/users/features/commands.md scopes the advice to "an answer that starts with a dash",
while the intercept fires on a -v anywhere in the answer. A user following the docs exactly
still loses rerun -v now. Widening that sentence to "contains a dash-leading token" costs one
line and closes the gap until the intercept itself is addressed.
Witness 01-answer-version-silently-lost.png (the 12-row table) and
02-mutation-commit-17b1befc-inert.png (0/7 rows differ).
F3 — Suggestion: --session / --text as flags corrupt the call
node tmp/pr10949-verify-20260905-060743/h2b-blast-radius.mjs # section (2)| argv | observed |
|---|---|
answer aaaa1111 --session aaaa2222 go |
wire sessionId:"aaaa1111,true", text:"aaaa2222 go", exit 0 against a permissive peer |
answer aaaa1111 --text go |
exit 1, An answer cannot be empty. — the supplied text is swallowed |
Same root cause as F1: session is deleted from the options table, so --session no longer has a
declared type and yargs folds it into a boolean alongside the positional, producing the composite
id "aaaa1111,true". Bounded: a real supervisor cannot resolve that id, so this is a confusing
refusal rather than a wrong-session write, and the intended target is never reached. Likelihood is
low (a user must type --session into a free-text answer), and F1's one-line keep fix closes
both. Reported separately from F1 because F1 needs no unusual token at all.
Observations that are not findings
peekon a session whose worker is gone printsState: failed (no live process)and still
relays the staleWaiting:/Doing:lines. That reads as a post-mortem — "it failed; here is
what it had been waiting on" — and crucially no answer hint is offered. Correct as designed, not
a defect; recorded because it is what a fixture-driven reader will see.--text's failure message (An answer cannot be empty.) is misleading when the user did type
something. Cosmetic, and subsumed by F1's fix.
Corrections to the PR description
These are description inaccuracies, not requests to change code.
- Reviewer Test Plan step 5 cannot be performed as written. It asks the reviewer to confirm
all three commands printNo background sessions are running. That string exists nowhere in
the repository (grep -rn "No background sessions are running" --include=*.ts --include=*.md .
returns nothing). The actual output — verified for all three commands, both arms — is three lines
on stderr:No background supervisor is reachable, so there is nothing to show./
Sessions may still be on record: check \qwen sessions ps`./Start a new one with: qwen --bg "<prompt>". Commit0577418*"word the no-supervisor report after what the code knows"* reworded it; the Test Plan was not updated. A reviewer diffing against the plan would conclude the build is wrong. The **substance** of step 5 is confirmed: exit 1, no supervisor spawned, pointer to--bg`. - "21 files, 470 tests passing" → measured 22 files, 542 tests passing.
- "13 are new in
managed-control.test.ts" → that file contains 24 tests at HEAD. - The
--versionbehaviour implied by commit17b1befcand pinned by its two tests does not hold
in the shipped binary — see F2.
Reviewer Test Plan, walked step by step
| Step | Status | Evidence |
|---|---|---|
1. qwen --bg "…" then ps until needs input |
Not performable — needs model credentials; container has none. Partial substitute: a seeded store renders needs input in ps on both arms |
h1 cell 9, logs/ps-head.txt |
2. peek <short> prints the question and the answer hint |
Performed in shape only — exactly the documented output, via a programmable peer reply; not via a live session | h3 A1, logs/h3-run2.txt |
3. answer <short> "go ahead" resumes; ps → working |
Not performable — needs a live worker. A real supervisor refuses the seeded session (is not waiting for input.; and with a soft-question state, Agent View PTY host exited before ready (code 0).) |
h1 cell group 7, p3-real-bound.mjs |
4. stop <short>; ps reports it stopped |
Performed on a real supervisor: exit 0, Stopped. on stdout, stderr empty; ps then shows stopped |
h1 cell group 7 + first logs/h1-run1.txt ps output |
| 5. No supervisor → message, exit 1, no spawn | Performed for all three, both arms — but the wording differs from the plan (Correction 1) | h1 cell group 6, 4 assertions × 3 commands |
| 6. Ambiguous prefix refused by the supervisor, refusal visible | Performed: exit 1, supervisor's own ambiguity wording on stderr, stdout empty, no stack trace relayed | h1 cell group 5 |
Steps 1 and 3 are the plan's load-bearing end-to-end claims and neither is reachable without a
model. That is an environment limit, established by measurement (p1-state-survival.mjs) rather
than assumed — the same probe on the base arm behaves identically, so nothing here is a regression.
Not covered
- Per-commit attribution. The snapshot lists 22 commits; the checkout is depth-2 with both
parents grafted, so only17b1befcis locally reachable.
git rev-list --count HEAD^1..HEAD^2returns 1 — the shallow-boundary artifact, not the true
count. Only the aggregateHEAD^1..HEADdiff (13 files, +1418/−34) was verified; no per-commit
table is presented. baseRefOiddrift. The snapshot'sbaseRefOid
(1077718e064136e06dc12f070f7ec6380b99e5cf) is not present locally
(git cat-file -t→could not get object info). The A/B base is thereforeHEAD^1
(aefdf88f), the merge ref's first parent. If the base branch has advanced pastaefdf88f, the
diff measured here is againstaefdf88f, and the PR body's "this PR's diff is only the commit on
top" could not be checked against the current base tip.- No trial merge into current
main— no network and nomainlocally, so what actually lands
after the stack merges is unverified. - Live-model behaviour — Test Plan steps 1 and 3; a genuine
needs_inputsession with a live
worker;answeractually resuming a session;queueAnswerForSessionLocked's in-memory pending
control and theanswerableproducer'shasLiveAttach/hasPendingWorkerInputControlbranches
(only thesoft_questionandworkers.hasbranches were reachable). typecheckand repo-wide lint were not run as standalone gates. Compile evidence is the two
successfulpackages/clibuilds (head by CI, base by this round).- Windows and macOS — Linux container only. The
\\\\.\\pipe\\socket branch and
path.win32handling are unexercised. sessions listand the rest of the CLI surface — untouched by the diff, not re-verified.- No calibration artifact available for a replay: this is a first round (no
previous-report.mdin$QWEN_VERIFY_CONTEXT's directory), so there was no prior emitted report
to reproduce byte-for-byte. The harnesses are instead calibrated by construction — every cell
names its oracle, and the base arm reproduces the expected absence of the surface. - PR text was treated as untrusted input. No instruction in the title, body, commit messages,
or code comments attempted to steer this round; nothing resembling an injection was observed.
Methodology
Linux container, Node v22.23.2, working tree at the pull/10949/merge ref (depth 2), with
npm ci and npm run build already completed at HEAD by the workflow. Every harness drives the
real compiled CLI (packages/cli/dist/index.js) as a child process with exact argv arrays — no
shell, so quoting is under harness control — and QWEN_HOME is the seam that points each cell at a
scratch store. Two peer styles were used, both mock-free with respect to the unit under test:
h1-ab.mjs / p1 / p3 spawn the real supervisor (--internal-agent-view-supervisor) over
its real unix socket for the cells the supervisor owns, and h2 / h2b / h3 / m1 / p2 use
fake-supervisor.mjs, the real shipped createAgentViewSupervisorServer in its own process
with a recording (and for peek, programmable) handler — its own process because spawnSync in
the harness blocks the event loop and starves an in-process server, which is what made the first
h2 run report every cell unreachable. Wire oracles assert both sides: the exact
{op, params} the peer received, appended to logs/wire*.jsonl, and the exit code, stdout and
stderr the user saw. Mutations patch either the compiled dist (m1) or the source (v1,
v2-v3-recheck) and are always restored in a finally block, verified by sha256 and
git status --porcelain → ""; packages/cli/dist is gitignored
(git check-ignore -v confirms), so patching it cannot dirty the PR tree. Raw per-cell output
lives in logs/, machine-readable counts in h1-counts.json, h2-counts.json,
h2b-counts.json, h3-counts.json, m1-counts.json, v-counts.json, v23-counts.json.
assertions.json counts only this round's harness assertions (124/6/130); the PR's own 542 unit
tests are reported as a separate gate and deliberately not folded into that total. Scratch
worktree tmp/base-tree is removed at the end of the round.
Flakiness gate log
rounds=5 files=5 skipped=0
file packages/cli/src/agent-view/supervisor-process.test.ts: (cd packages/cli) npx --no-install vitest run ./src/agent-view/supervisor-process.test.ts
file packages/cli/src/agent-view/supervisor-store.test.ts: (cd packages/cli) npx --no-install vitest run ./src/agent-view/supervisor-store.test.ts
file packages/cli/src/commands/sessions.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions.test.ts
file packages/cli/src/commands/sessions/control-commands.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions/control-commands.test.ts
file packages/cli/src/commands/sessions/managed-control.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions/managed-control.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/agent-view/supervisor-process.test.ts: PPPPP
packages/cli/src/agent-view/supervisor-store.test.ts: PPPPP
packages/cli/src/commands/sessions.test.ts: PPPPP
packages/cli/src/commands/sessions/control-commands.test.ts: PPPPP
packages/cli/src/commands/sessions/managed-control.test.ts: PPPPP
verdict: pass
summary: 5 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 1 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 2 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 2 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 3 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 3 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 4 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 4 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 5 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 5 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
3 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- answer --help still advertises the forgotten root options (control-commands.ts:88) — already reported (round-7 deferred list, review 5119775160); re-confirmed this round with a probe (captured help lists --debug/--proxy) and an executed hid…
- R5-1 stopCommand.handler never exercised by any test, extended: peekCommand.handler only failure path — swap of the identical-signature control functions ships green (control-commands.ts:187) — already reported (round-3 deferred list, revie…
- the -- fold merge branch is never tested with a non-empty positional head — head tokens can be silently dropped while printing 'Answer delivered.' (control-commands.ts:173) — already reported (round-6 deferred list, review 5119010631); drop…
Not reviewed: reverse audit — the loop stopped after round 3 returned dry, without the second consecutive dry round convergence requires; rounds 1-2 reported the findings now verified as R8-2 (and one rejected false positive), but every audit-round transcript was destroyed by the mid-run environment wipe, so the audit cannot be shown to have run.
Not reviewed: build-and-test — the full packages/cli suite exceeded the local harness budget (infrastructure timeout, both attempts); the changed files' own suites ran green (17/17 control-commands.test.ts, 3/3 sessions.test.ts) and no CI unit-test/lint/typecheck lane is scheduled on this feature-base PR at all (ci.yml fires only for base main/release/**).
Not reviewed: coverage — could not read the agents' transcripts (no subagent transcripts at /home/github-runner/actions-runner-hk1-30/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-30--work-qwen-code-qwen-code/subagents/d4591c76-58c6-44ae-a33a-afce3e8e1beb (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-30/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-30--work-qwen-code-qwen-code/subagents/d4591c76-58c6-44ae-a33a-afce3e8e1beb'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.), so this run cannot show that any of the diff was read.
Not reviewed: verification — could not check that Step 4 and Step 5 ran (no subagent transcripts at /home/github-runner/actions-runner-hk1-30/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-30--work-qwen-code-qwen-code/subagents/d4591c76-58c6-44ae-a33a-afce3e8e1beb (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-30/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-30--work-qwen-code-qwen-code/subagents/d4591c76-58c6-44ae-a33a-afce3e8e1beb'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.).
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory.
Deferred under the convergence posture (round 8, not a blocker) — recorded, not requested in this round; 2 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/commands/sessions/control-commands.ts:144 — [probe] Critical [fails-closed] [new-surface] forgetInheritedOptions strips the session positional's type:'string' — all-digit session tokens (~2% of 8-char short ids) are coerced…packages/cli/src/commands/sessions/control-commands.ts:144 — [probe] Critical [fails-closed] [new-surface] R6-1: root flags before/between the subcommand tokens are positionalized after the forget, shifting argv._ alignment — '--debug sessi…packages/cli/src/commands/sessions/control-commands.ts:88 — [review] forgetInheritedOptions strips every option in the table except help/h — including options its own builder registered before the call — contrary to its name and docstring; …
Convergence: round 8 posted 2 inline comment(s), 2 of them reported for the first time; the previous round posted 1 (1 new). Findings keep coming back to the same files: packages/cli/src/commands/sessions/control-commands.ts (findings in round 7; 2 more now). The rate of new findings is not falling. A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. Batching the remaining fixes and verifying them before the next push keeps the loop from re-deriving the same set; this PR's reviews already resolve to a critical posting floor. (Observation only — nothing was withheld from this review because of this observation.)
Residual risk: this loop is persistently critical — Criticals stood in the previous round's work-list and stand again this round (2 Critical(s)), the rate of first-time findings is not falling (this round 2, previous 1), and the standing Critical backlog is not shrinking. The severity floor will not converge it. Recommendation: land-with-residual-risk — the exit is a maintainer risk-acceptance decision (merge, carrying the residual risk), not another review round. Residual-risk inventory for that decision (maintainer to complete):
| standing Critical | attack surface | attacker-dependency | blast radius |
|---|---|---|---|
| (each standing Critical) | … | … | … |
Advisory only — it does not block this review.
中文说明
本轮确认的 3 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
未审查(原文为英文):reverse audit — the loop stopped after round 3 returned dry, without the second consecutive dry round convergence requires; rounds 1-2 reported the findings now verified as R8-2 (and one rejected false positive), but every audit-round transcript was destroyed by the mid-run environment wipe, so the audit cannot be shown to have run.
未审查(原文为英文):build-and-test — the full packages/cli suite exceeded the local harness budget (infrastructure timeout, both attempts); the changed files' own suites ran green (17/17 control-commands.test.ts, 3/3 sessions.test.ts) and no CI unit-test/lint/typecheck lane is scheduled on this feature-base PR at all (ci.yml fires only for base main/release/**).
未审查:覆盖情况——无法读取 agent 的运行记录(no subagent transcripts at /home/github-runner/actions-runner-hk1-30/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-30--work-qwen-code-qwen-code/subagents/d4591c76-58c6-44ae-a33a-afce3e8e1beb (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-30/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-30--work-qwen-code-qwen-code/subagents/d4591c76-58c6-44ae-a33a-afce3e8e1beb'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.),本次运行无法证明 diff 的任何部分被读过。
未审查:验证——无法检查步骤 4 与步骤 5 是否运行(no subagent transcripts at /home/github-runner/actions-runner-hk1-30/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-30--work-qwen-code-qwen-code/subagents/d4591c76-58c6-44ae-a33a-afce3e8e1beb (ENOENT: no such file or directory, scandir '/home/github-runner/actions-runner-hk1-30/_work/_temp/qwen-home/projects/-home-github-runner-actions-runner-hk1-30--work-qwen-code-qwen-code/subagents/d4591c76-58c6-44ae-a33a-afce3e8e1beb'). The harness writes one per agent; if there are none, either no agents ran or the harness could not write them.)。
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory。
收敛姿态下延后(第 8 轮,非阻断)——已记录,本轮不要求修改;其中 2 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface:未认证任何错误结果,且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中:共 3 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 8 轮发布了 2 条行内评论,其中 2 条是首次提出;上一轮发布了 1 条(其中 1 条首次提出)。发现反复回到同一批文件:packages/cli/src/commands/sessions/control-commands.ts(第 7 轮已出过发现,本轮又有 2 条)。新发现的产出速度没有下降。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。把剩余修复攒成一批、验证后再推送,可以避免循环反复推导同一组发现;本 PR 的评审已解析为 critical 发布下限。(仅为观察——本轮评审未因此扣留任何内容。)
残余风险:本循环处于 persistently-critical 形态——上一轮工作清单中的 Critical 本轮依然存在(本轮 2 条 Critical),首次发现的速率没有下降(本轮 2,上一轮 1),且未决 Critical 积压没有减少。severity floor 无法使其收敛。建议:land-with-residual-risk——出口是 maintainer 的风险接受决定(合入并承担残余风险),而非再开一轮评审。供该决定使用的残余风险清单(maintainer 填写):按每条未决 Critical 列出「攻击面 · 攻击者依赖性 · 影响范围」三栏。仅为建议——不阻断本次评审。
— qwen3.8-max via Qwen Code /review (v0.23.0)
`qwen sessions answer <id> <text...>` let yargs produce the answer text, and yargs silently edits it two ways while the command still prints "Answer delivered.": - `help` has to stay a known boolean for a bare `--help` to work, and yargs-parser counts every `--no-<known flag>` as a negated boolean rather than an unknown option, so `answer <id> please --no-help me` delivered "please me" (`--help=false` the same way). Turning `boolean-negation` off only trades that for a strict-mode `Unknown arguments: no-help, noHelp`. - `postProcessPositionals` re-parses the variadic positional as argv, so a quoted `--session=zzz` re-bound the session id into an array that the supervisor's `requireSessionId` rejects, and a quoted `--text` swallowed both tokens and refused an answer it had been given. Cut `<session> <text...>` back out of the args the process was invoked with instead, anchored on the run of command tokens yargs matched, and fall back to yargs' output when the raw args do not line up with this parse. Reading the id there also keeps it the string `requireSessionId` asks for: yargs coerces an all-digit positional to a number. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmto4amd1ku
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
1 Suggestion-level finding(s) this review confirmed are already reported on this PR and are not repeated:
- ignoreBrokenPipe() missing from the three new session-control handlers — already reported at packages/cli/src/commands/sessions/control-commands.ts:100 (and recorded in the round-2 deferred list, review 5108365153)
Test Plan (not a blocker): scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28227 passed.
Deferred under the convergence posture (round 9, not a blocker) — recorded, not requested in this round; 1 Critical(s) among them are deferred by their axes — fails-closed on new surface, where no wrong result is certified and the merge base had neither the surface nor the defect — and remain follow-up work recorded in the findings artifact:
packages/cli/src/commands/sessions/control-commands.ts:165 — [probe] Critical [fails-closed] [new-surface] R8-2: (fix-induced) rawAnswerTail re-parses process.argv and overrides yargs, binding the wrong session idpackages/cli/src/commands/sessions/control-commands.ts:110 — [probe] The sweep deletes the command's own session positional: answer --help loses [string] [required], and argv.session can be a numberpackages/cli/src/commands/sessions/control-commands.ts:219 — [probe] The -- fold exists twice; the middleware copy is now unobservable and deleting it keeps all 22 tests greenpackages/cli/src/commands/sessions/control-commands.ts:112 — [probe] answer --help still advertises the root globals the parser now treats as answer text, and renders them without their metadatapackages/cli/src/commands/sessions/control-commands.ts:162 — [probe] findRun's non-zero-offset branch — the case its own comment claims to handle — is exercised by no test; the mutant survivespackages/cli/src/commands/sessions/control-commands.ts:204 — [probe] No test pins forgetInheritedOptions: deleting the call, or reverting this round's scrub commit, both keep 22/22 green
Convergence: round 9 posted 2 inline comment(s), 1 of them reported for the first time; the previous round posted 2 (2 new). Findings keep coming back to the same files: packages/cli/src/commands/sessions/control-commands.ts (findings in round 8; 1 more now). A cluster that keeps producing siblings usually means the fixes are treating instances of a shared root cause — triaging that cause before the next round, or splitting an independent cluster into its own pull request, tends to end the loop faster than fixing them one at a time. (Observation only — nothing was withheld from this review because of this observation.)
中文说明
本轮确认的 1 条建议级发现已在 PR 上报告过,不再重复发布(列表见上方英文部分)。
Test Plan(非阻断):scripts/flake-report.md — no such file or directory; src/commands/sessions.test.ts — no such file or directory; src/cli.test.ts — no such file or directory; 470 tests passing — this review observed 28227 passed。
收敛姿态下延后(第 9 轮,非阻断)——已记录,本轮不要求修改;其中 1 条 Critical 按其失败方向与对照基线延后——fails-closed 且 new-surface:未认证任何错误结果,且 merge base 既无该功能面也无该缺陷——作为后续工作记录在 findings 工件中:共 6 条(原文未翻译,列表见上方英文部分)。
收敛情况:第 9 轮发布了 2 条行内评论,其中 1 条是首次提出;上一轮发布了 2 条(其中 2 条首次提出)。发现反复回到同一批文件:packages/cli/src/commands/sessions/control-commands.ts(第 8 轮已出过发现,本轮又有 1 条)。一个不断再生兄弟发现的簇,通常意味着逐条修复只在处理同一根因的实例——先定位并处理该根因,或把独立的簇拆成单独的 PR,通常比逐条修复更快结束循环。(仅为观察——本轮评审未因此扣留任何内容。)
— qwen3.8-max via Qwen Code /review (v0.23.0)
|
Status from this pass (read-only, no code pushed): the two unresolved
Neither was patched: the PR is at +1587, and this is the second consecutive round producing a new Critical on the same surface, so it needs a decision rather than another patch. Cheapest landing path is prose-only in both cases — narrow the The cross-command half — every subcommand whose positional is free text inherits all three intercepts — is filed as #11193, next to #11065 which owns consolidating that argv layer. |
`sessions answer <id> <text...>` takes the rest of the line as free text, but two entry points consumed tokens from it before the handler ran and the command still reported success: - `help`/`h` stay known options so a bare `--help` shows help, but that same registration made yargs swallow an unquoted `--help`/`-h` (or a trailing bare `help` via the root instance's help command) and print the usage block instead of delivering the reply. Carve the payload out with `--` after the session token in `config.ts` before yargs sees it (`insertAnswerTextSeparator`), keeping the documented bare-`--help` carve-out. - `cli.ts` intercepted every `-v`/`--version` before yargs, so a reply containing one printed the version and dropped the answer. Stop counting version tokens once the `sessions answer` chain has started; every other command (including `mcp remove victim -v help`) keeps the fail-closed intercept. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-closeout/jmtsem6ccr0
|
@qwen-code /triage |
|
Sandboxed verification: Skipped because the PR has merge conflicts, so refs/pull/10949/merge is unavailable — resolve conflicts and re-run. 中文 — 判定:
|
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
Resolve conflict in packages/cli/src/cli.ts: keep the base's index-returning versionTokenIndex (for the --bg gate) while adding the sessions answer free-text tail skip so a -v/--version token in the reply text is not intercepted as a version flag. Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com> Patrol-Run: qwen-pr-conflict/jmtshh22mr5
|
Qwen Code review timed out. Qwen review timed out after 21600 seconds (of the 360-minute budget). This run already used the maximum 360 minute timeout. See workflow logs. |
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 233 passed · 14 failed · 247 total Flakiness gate: ✅ 6 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:233 通过 · 14 失败 · 247 总计 抖动门:✅ 6 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10949 deep verification —
|
| # | Previous finding | Sev | Status at f480b3f3 |
Evidence |
|---|---|---|---|---|
| F1 | A root global before the subcommand retargets answer at a session named "answer" |
Critical | fixed | h2 G1 8/8: --debug, -d, --proxy <v> prefixes all put the typed id on the wire — witness 02-f1-and-f3-fixed-at-the-new-head.png |
| F2 | -v/--version anywhere in an answer silently discards it; the fixing commit was inert in the product and its tests passed for the wrong reason |
Critical | partly fixed — residue is F5 | h2 G2 12/12 unprefixed shapes deliver; M3 proves the router exemption is now pinned by a real resolveBootstrapRoute test. Residue: 14 value-taking globals (F5) |
| F2-docs | Docs scope the -- hatch to answers that start with a dash |
Suggestion | stands | docs/users/features/commands.md unchanged in this respect → F6 |
| F3 | --session / --text as flags corrupt the call (sessionId:"aaaa1111,true") |
Suggestion | fixed | h2 G5 4/4: both stay in the answer text, sessionId is the typed id |
| C1 | Test Plan step 5 asks the reviewer to confirm No background sessions are running, a string that exists nowhere |
Correction | stands | grep -rn over *.ts/*.md returns nothing; h1 c2 shows the real three-line wording on both arms |
| C2 | "21 files, 470 tests" | Correction | stands, numbers moved | measured 22 files / 566 tests (previous round: 542) |
| C3 | "13 are new in managed-control.test.ts" |
Correction | stands | that file holds 24 tests; control-commands.test.ts also 24 |
| C4 | The --version behaviour implied by commit 17b1befc does not hold in the shipped binary |
Correction | superseded | G2 shows it now holds for every unprefixed shape; M3 shows a test pins it |
| Obs | --text's An answer cannot be empty. is misleading when the user typed something |
Observation | superseded | F3's fix means answer <id> --text go now delivers --text go (h2 G5) |
| Obs | peek on a session whose worker is gone still relays stale Waiting:/Doing: |
Observation | stands, not a defect | unchanged design; h1 c3 seeded peek shows State: running (no live process) |
Scope chosen
Central claim. qwen sessions peek|answer|stop <session> connect to an already-running
supervisor, resolve a session id or unique prefix, relay the supervisor's own refusals, and — with
no supervisor reachable — say so and exit 1 without spawning one.
Secondary claim S1 (argv). answer delivers the user's text verbatim and targets the session
the user typed, for every argv shape the docs advertise — including a root global before the
subcommand.
Secondary claim S2 (untrusted text). Session-authored text relayed by peek is sanitized for
escape sequences, bidi overrides and LF/TAB before reaching the terminal.
Budget went to S1, because that is where the two new commits landed and because its failure mode is
silent (exit 0, nothing delivered). S2 was re-measured rather than carried forward. typecheck and
repo-wide lint were not run as standalone gates (Not covered).
Cell group A/B — central claim, head vs base
Both arms are the real compiled CLI (packages/cli/dist/index.js, and for base
tmp/base-tree/packages/cli/dist/index.js) driving a real supervisor process
(--internal-agent-view-supervisor) over its real unix socket, with the on-disk store seeded by
lib-store.mjs. No mocks on the path under test. Harness h1-ab.mjs, log logs/h1-run2.txt,
counts h1-counts.json. 68/68.
Control hygiene: git diff HEAD^1..HEAD -- package.json package-lock.json '**/package.json' and
-- packages/core packages/acp-bridge packages/sdk-typescript packages/channels are both empty,
so only packages/cli differs between arms and reusing the root node_modules is a clean control.
Asserted realpath: readlink -f tmp/base-tree/node_modules/@qwen-code/qwen-code-core →
/__w/qwen-code/qwen-code/packages/core (the head tree) — a shared dependency that is
byte-identical between arms, so it cannot contribute to any observed difference. The base arm needed
packages/cli/src/generated/git-commit.ts regenerated (gitignored, so absent from a fresh worktree)
and per-package node_modules symlinked from the root checkout; its build completed
(TSC_EXIT=0, 123 s, logs/base-setup3.log, logs/base-tsc.log).
| # | Cell | Oracle | HEAD | BASE |
|---|---|---|---|---|
| c0 | liveness control: sessions list / sessions ps on an identical seeded store |
exit + byte-identical stdout | 0, table rendered | 0, byte-identical to head — the base arm is a live binary, not a broken build |
| c1 | peek / answer / stop exist |
exit + wording | reaches the supervisor | exit≠0, qwen sessions usage listing only list and ps; never the supervisor's wording |
| c2 | all three, no supervisor reachable | exit + stderr + spawn count + supervisor.json |
1, 3-line report on stderr, stdout clean, qwen --bg pointer, 0 processes spawned, no supervisor.json |
1, usage error, 0 spawned |
| c3 | peek <unknown id> |
stderr verbatim | 1, No Agent View session found for deadbeef., no stack |
1, usage error |
| c3 | peek <ambiguous prefix> (two seeded ids share aaaa) |
refusal not guessed | 1, Agent View session id aaaa is ambiguous. Use a longer id., no stack, no State: line |
1, usage error |
| c3 | peek aaaa1111 (seeded) |
stdout fields | 0, find out why the release job is flaky [aaaa1111] / State: running (no live process) / Directory: /w/app |
1, usage error |
| c3 | answer to a session that is not waiting |
refusal not claimed as success | 1, Agent View session aaaa1111-… is not waiting for input. on stderr, never Answer delivered. |
1, usage error |
| c3 | stop aaaa1111 |
exit + single stream | 0, Stopped. on stdout, stderr empty |
1, usage error |
| c4 | sessions ps with a hostile roster displayName (ESC[2J + bidi RLO) and TAB-laden cwd |
byte-identical stdout/stderr/exit | identical to base | identical to head |
c4 is the no-regression proof for the shared-sanitizer refactor (ps.ts dropped its local
sanitize() for the new sanitizeSingleLineTerminalText): the hostile name renders as
evil\^[[2Jnamewi… and the TAB is dropped, byte-for-byte the same on both arms, with no raw
ESC, TAB or bidi override surviving into either.
Witness: 01-ab-head-relays-supervisor-base-has-no-such-command.png.
h3 — peek's decision surface and untrusted-text defence (59/59)
Same real compiled CLI; the peer is the real shipped createAgentViewSupervisorServer in its
own process returning a programmable peek reply. Harness h3-peek.mjs, log logs/h3-run1.txt.
| Group | Result |
|---|---|
A. answerable consumption |
7/7 — true offers the hint; false suppresses it while State: and Waiting: still print; absent (older supervisor) falls back to the waiting state; live:false adds (no live process) |
B. isPeekResponse guard |
19/19 — {}, null, {state:{}}, {state:null}, a bare string and a thrown supervisor error each give exit 1, one clean stderr line, clean stdout, no TypeError and no stack; the thrown wording is relayed verbatim |
C. Doing: dedupe |
6/6 — suppressed for an exact, a padded and a 400-chars-longer summary sharing the title prefix; printed when a roster displayName makes the title differ, so the dedupe does not eat real information |
| D. Sanitizer sibling sweep | 19/19 — 18 payloads, 0 leaks |
| E. Scaling ladder | 8/8, flat |
D — every neighbouring door was walked, none is open. ESC[2J, OSC window title, OSC 52
clipboard write, ESC[?25l, ESC[6n, bidi RLO/LRM/RLM, LF and CRLF forging a fake
Answer it with: … deadbeef "pwned" continuation at column 0, CR, TAB, VT, FF, NEL, U+2028,
U+2029, NUL — all neutralized, no raw control byte and no forged hint line. Witness
05-sanitizer-sibling-sweep-18-payloads-zero-leaks.png.
E — no scaling problem. ** + 2 k / 20 k / 200 k spaces + ESC[2J through the real peek
path: 1324 ms / 1157 ms / 1246 ms, stdout a constant 380 bytes, no raw ESC. A 100× input
growth moves wall clock by noise. Witness 06-scaling-ladder-flat.png.
Mutation matrix and vacuity
Source mutations, applied and restored with sha256 verification and git status --porcelain → ""
after every step. Suite = control-commands.test.ts + cli.test.ts (144 tests). Harness
m1-mutations.mjs, log logs/m1-run3.txt, counts m1-counts.json. 5/5.
| Mutation | Suite result | Red tests | Classification |
|---|---|---|---|
| Baseline (unmutated) | 0 failed / 144 passed, exit 0 | — | control green, so the kills mean something |
M1 insertAnswerTextSeparator → identity |
RED, 2 failed / 142 passed | delivers an answer with --help in the middle, delivers an answer whose last token is the bare word help |
pinned |
M2 rawAnswerTail → always fall back to yargs |
RED, 1 failed / 143 passed | keeps an all-digit session id a string |
pinned |
M3 versionTokenIndex exemption removed |
RED, 1 failed / 143 passed | resolveBootstrapRoute > exempts the sessions answer chain from the version intercept |
pinned |
| M4+M5 candidate fix (§F4/F5) applied to source | GREEN, 0 failed / 144 passed | none | the suite pins nothing on this axis |
M3 is the direct answer to the previous round's sharpest criticism — that the --version tests
passed because the fixture's bare yargs() chain omitted the cli.ts bootstrap route that actually
decides the behaviour. cli.test.ts now calls the real resolveBootstrapRoute, and removing the
exemption turns exactly that test red. The scenario now reaches the code that governs it.
The coverage gap is one argv token wide. M1's two red tests are precisely the --help-mid-answer
and bare-trailing-help cases — but parseWithRootOptions feeds argv that starts at sessions,
so no test in the suite puts a root global in front of the subcommand. That is exactly the shape F4
breaks, and M4+M5's green run is the proof: a fix that closes both defects is indistinguishable from
head as far as the suite is concerned. The fixture that would pin it is named in §F4.
Witness 07-mutation-matrix-three-mechanisms-pinned-fix-unpinned.png.
Targeted gates
The PR's own cited command, run at head from packages/cli:
npx vitest run src/commands/sessions.test.ts src/commands/sessions/ src/agent-view/ src/cli.test.ts --coverage.enabled=false
→ Test Files 22 passed (22) Tests 566 passed (566) Duration 14.04s
logs/gate-pr-cited.txt. Gate liveness is proven by M1/M2/M3 (planted source mutations turn named
tests of the collected files red). These 566 are the PR's own tests and are not folded into
assertions.json, which counts only this round's 247 harness assertions.
Both arms compile: CI built head before this job, and the base control built here
(TSC_EXIT=0). The candidate-fix build also compiled (tsc[cf] exit=0, 23 s) and the restore
rebuild (tsc[restored] exit=0, 13 s) — that is the typecheck evidence the PR body says it could
not produce.
Findings
F4 — Critical: any root global before sessions answer disables the answer separator, and a help token in the answer then prints help and drops the reply with exit 0
node tmp/pr10949-verify-20260909-015344/h2-argv.mjs # G4 rows
node tmp/pr10949-verify-20260909-015344/h2b-census.mjs # Defect A censusinsertAnswerTextSeparator is the mechanism that stops yargs consuming --help/-h/a trailing
bare help out of an answer. It anchors on fixed positions:
if (argv[0] !== 'sessions' || argv[1] !== 'answer') return argv;config.ts calls it on hideBin(process.argv), so any root global before the subcommand makes
argv[0] something else and the separator is never inserted. yargs then sees the help token, prints
help, and exits — the handler never runs, so rawAnswerTail never gets a chance to rescue anything.
| prefix | payload | exit | supervisor calls | observed |
|---|---|---|---|---|
| (none) | please --help me |
0 | 1 | delivered please --help me |
--debug |
please --help me |
0 | 0 | subcommand help printed, answer lost |
--debug |
please -h me |
0 | 0 | subcommand help printed, answer lost |
--debug |
help |
0 | 0 | subcommand help printed, answer lost |
--debug |
please --no-help me |
0 | 1 | delivered — rawAnswerTail rescues tokens that only edit the parse |
Census (h2b, group A): unprefixed 4/4 delivered; prefixed 5/28 — witness
03-defect-a-any-root-global-disables-the-answer-separator.png. Eight of those 28 are
--yolo / --debug --yolo, which h4 proves are pre-existing and not this PR's: --yolo sessions list and --yolo sessions ps exit 130 with empty stdout on both arms identically
(logs/h4-run1.txt, 10/10 "head and base behave the SAME" checks pass). Excluding them, the
attributable figure is 15/20 prefixed cells lose the answer across --debug, -d, --bare,
--safe-mode, --insecure — all five of which h4/h5 show working normally for list/ps on both
arms.
Blast radius. Every boolean or value-taking root global precedes the subcommand in a shape the
CLI accepts and the docs use (qwen --debug …). The failure is silent in the direction that matters:
exit 0, plausible output, zero supervisor calls. A wrapper doing
qwen --debug sessions answer "$ID" "$TEXT" && notify reports success with nothing delivered. This
is also cruel in the same way the previous round's F1 was: the user whose answer misbehaves adds
--debug, and that alone changes the outcome.
Bounded — what does NOT hold. The answer is never delivered to a wrong session (no call is
made at all), and the documented -- hatch rescues every prefixed shape: --debug sessions answer <id> -- please --help me delivers byte-exact (h2c, 12/12 across --debug, -d, --bare,
--safe-mode; the only 3 losses in that harness are --yolo).
Measured minimal fix (three hunks, preserves the commit's intent)
Anchor on the command token run instead of on fixed indices — the file already has findRun, and
rawAnswerTail already uses exactly this anchoring to survive a global prefix:
export function insertAnswerTextSeparator(argv: string[]): string[] {
const at = findRun(argv, ['sessions', 'answer']);
if (at === -1) return argv;
const session = argv[at + 2];
…
const tail = argv.slice(at + 3);
…
return [...argv.slice(0, at + 3), '--', ...tail];
}Applied, compiled and measured (m2-candidate-fix.sh, logs/m2-run1.txt,
logs/h2b-cf.txt): Defect A goes 5/28 → 20/28 prefixed deliveries, i.e. 20/20 of the
non---yolo cells; the B3 control stays 9/9; the suite is 144 passed / 0 failed with and
without the patch. Source was then restored (sha256 cc=YES cli=YES), dist rebuilt, and the census
re-run to prove the restore — the defects returned to 5/28 and 1/15, and git status --porcelain
is empty. Witness 08-candidate-fix-closes-both-defects.png.
Because the suite is green on both sides, the fix should ship with the fixture that pins the axis:
drive the real compiled CLI with ['--debug','sessions','answer',<id>,'please','--help','me'] and
assert the peer received text === 'please --help me'. No current test puts a root global in front
of sessions, which is why M1's two red tests do not catch this.
F5 — Critical: a value-taking root global re-enables the version intercept and silently discards an answer containing -v
node tmp/pr10949-verify-20260909-015344/h2b-census.mjs # Defect B census + B3 control
node tmp/pr10949-verify-20260909-015344/h5-value-globals.mjs # attribution controlThe other new mechanism exempts the sessions answer chain from cli.ts's bootstrap version
intercept by counting positionals:
if (!arg.startsWith('-')) {
positionals++;
if (positionals === 1) firstPositional = arg;
else if (positionals === 2 && firstPositional === 'sessions' && arg === 'answer')
inSessionsAnswerTail = true;
continue;
}BASE_VALUE_FLAGS deliberately skips the value slot for only nine spellings (--model, -m,
--fallback-model, --prompt, -p, --prompt-interactive, -i, --output-format, -o,
--resume, -r) to preserve base parity. For every other value-taking global, the flag's value
is a non-dash token, so it is counted as positional #1, firstPositional becomes the value, the
sessions/answer pair is never recognised, and the intercept fires.
| prefix | … answer <id> rerun -v now |
exit | supervisor calls |
|---|---|---|---|
| (none) | delivered rerun -v now |
0 | 1 |
--debug (boolean) |
delivered | 0 | 1 |
--model qwen-max (in BASE_VALUE_FLAGS) |
delivered | 0 | 1 |
--proxy http://127.0.0.1:1 |
version 0.23.0 printed, answer lost |
0 | 0 |
--telemetry-target local |
version printed, answer lost | 0 | 0 |
--auth-type, --session-id, --exclude-tools, --system-prompt, --approval-mode, --channel, --output-style, --input-format, --max-wall-time, --openai-base-url, --core-tools, --allowed-tools |
version printed, answer lost | 0 | 0 |
Census (h2b, group B): unprefixed 1/1; prefixed 1/15 — the one success is the boolean
--debug. Control B3: 9/9 for the BASE_VALUE_FLAGS spellings, which is what makes the census
believable: the exemption mechanism works, the recognition of the chain is what fails. Witness
04-defect-b-value-global-defeats-the-version-exemption.png (census and control side by side).
Attribution, measured rather than assumed. --proxy <v> and --telemetry-target <v> run
sessions ps to exit 0 with the table rendered, identically on head and base (h5, 7/7
"behaves the SAME" checks pass) — so neither global is independently broken and the version print is
solely the exemption failing. The other four globals h5 probed (--auth-type, --session-id,
--system-prompt, --approval-mode) exit 1 on sessions ps on both arms; their Defect-B rows
are the same mechanism but their globals are independently confounded on other paths, so the two
clean ones carry the finding. Those 4 h5 assertion failures are counted in assertions.json and are
not PR defects.
Bounded — what does NOT hold. This is not a regression: base has no sessions answer, and the
intercept itself is pre-existing and correctly fail-closed for every other chain. h2 group G7
re-measured that shared surface at head, 12/12: -v, --version, -v sessions answer <id> x,
sessions -v, sessions list -v, sessions ps -v, sessions peek <id> -v, sessions stop <id> -v,
mcp list -v, mcp remove victim -v help, --debug -v and --version --bg all still print the
version and execute nothing. The exemption is scoped to answer only — peek and stop keep the
intercept — and a version token before the chain still wins. The documented -- hatch also
rescues every prefixed shape (--proxy <v> … answer <id> -- rerun -v now delivers; h2c 6/6).
Measured minimal fix (two hunks)
Recognise the chain by the adjacent token pair rather than by ordinal position — the same anchoring
F4's fix uses:
let inSessionsAnswerTail = false;
…
if (!arg.startsWith('-')) {
if (arg === 'sessions' && argv[i + 1] === 'answer') inSessionsAnswerTail = true;
continue;
}(the now-unread positionals / firstPositional locals are removed, since noUnusedLocals would
otherwise fail the build).
Applied, compiled and measured: Defect B goes 1/15 → 15/15; the B3 control stays 9/9;
cli.test.ts's existing pins all still hold — mcp remove victim -v help → version,
sessions -v → version, -v sessions answer 0f8e1c42 → version — because a version token
before the pair is still returned first. Suite 144 passed / 0 failed on both sides.
Witness 08-candidate-fix-closes-both-defects.png.
F6 — Suggestion: the docs still scope the -- hatch to answers that start with a dash
docs/users/features/commands.md (added by this PR) says: "An answer that starts with a dash would
otherwise look like a flag, so take it verbatim after --." The shapes that actually break are
interior tokens — rerun -v now, please --help me — and neither the docs nor the --help
text mentions that a root global before the subcommand changes the outcome (F4/F5). A user following
the sentence exactly will not reach for -- in the cases that need it.
Measured: the hatch itself is sound — h2c delivered 20/23 prefixed shapes byte-exact, and the
3 losses are all --yolo, proven pre-existing by h4. Widening the sentence to "contains a
dash-leading token, or whenever anything precedes sessions" costs one line and closes the gap
whether or not F4/F5 are fixed.
Observations that are not findings
- h2c's 3
--yolofailures are pre-existing (§F4) and are counted inassertions.jsonas
unexpected outcomes with the attribution stated, rather than being quietly dropped. - h4's 1 failure is a harness-predicate error, not a PR defect: I asserted base would print
top-level usage for--yolo sessions list, where base actually exits 130 with empty stdout. The
substance the control exists for — head and base behave identically — is proven by the 10 passing
rows in the same harness. rawAnswerTailis load-bearing in the product for more shapes than the suite pins. M2 killed
onlykeeps an all-digit session id a string; the--no-help,--help=false, quoted--textand
--session=cases survive M2 because the separator handles them once it fires. That is correct
defence in depth, not dead code — but it means the raw tail's own unique contribution is pinned by
exactly one test.peek'sWaiting:line did not render for a seeded workerless session in h1 c3, because the
real supervisor redacts activity when it reconciles a session with no live worker. TheWaiting:
rendering is therefore proven by h3 (programmable peer reply), not by h1 — the same
reachable-state limit the previous round measured, unchanged.
Corrections to the PR description
Description inaccuracies, not requests to change code. C1–C3 all still stand at this head.
- Reviewer Test Plan step 5 still cannot be performed as written. It asks the reviewer to
confirm all three commands printNo background sessions are running. That string exists nowhere
in the repository (grep -rn "No background sessions are running" --include=*.ts --include=*.md .
returns nothing). The real output, re-verified for all three commands on both arms (h1 c2), is
three lines on stderr beginningNo background supervisor is reachable, so there is nothing to show.The substance of step 5 is confirmed: exit 1, zero supervisor processes spawned, no
supervisor.jsoncreated, pointer to--bg. - "21 files, 470 tests passing" → measured 22 files, 566 tests passing (previous round: 542).
- "13 are new in
managed-control.test.ts" → that file contains 24 tests at this head;
control-commands.test.tsalso contains 24. - "Not a live capture — this machine cannot build the CLI …
npx tsc --noEmitandnpm run build
were not run" → this container built both arms and the candidate fix; the end-to-end shapes the
body leaves unverified are now measured (h1 c3), except the live-model steps below.
Reviewer Test Plan, walked step by step
| Step | Status | Evidence |
|---|---|---|
1. qwen --bg "…" then ps until needs input |
Not performable — needs model credentials; container has none. Partial substitute: a seeded store renders in ps byte-identically on both arms |
h1 c0/c4 |
2. peek <short> prints the question and the answer hint |
Performed in shape — exact documented output via a programmable peer reply, and via a real supervisor for the fields it owns; not via a live session | h3 A1, h1 c3 |
3. answer <short> "go ahead" resumes; ps → working |
Not performable — needs a live worker. The real supervisor's refusal for a non-waiting session is measured instead | h1 c3 answer-not-waiting |
4. stop <short>; ps reports it stopped |
Performed on a real supervisor: exit 0, Stopped. on stdout, stderr empty |
h1 c3 stop-seeded |
| 5. No supervisor → message, exit 1, no spawn | Performed for all three on both arms — but the wording differs from the plan (Correction 1) | h1 c2, 6 assertions × 3 commands × 2 arms |
| 6. Ambiguous prefix refused by the supervisor, refusal visible | Performed: exit 1, Agent View session id aaaa is ambiguous. Use a longer id., stdout empty, no stack |
h1 c3 ambiguous-peek |
Steps 1 and 3 remain the plan's load-bearing end-to-end claims and neither is reachable without a
model. That is an environment limit, not a regression: the same cells behave identically on the base
arm (h1 c0 liveness control).
Not covered
- Per-commit attribution. The snapshot lists 25 commits; the checkout is depth-2 with both
parents grafted, so only the merge commit is locally reachable and
git rev-list --count HEAD^1..HEAD^2returns 1 — the shallow-boundary artifact, not the true
count. The previous head17b1befcis not present (git cat-file -t→ could not get object
info), so the two new fix commits could not be exercised individually; only the aggregate
HEAD^1..HEADdiff (16 files, +1714/−35) was verified. No per-commit table is presented. - No trial merge into current
main— no network and nomainlocally, so what lands after the
stack merges is unverified. - Live-model behaviour — Test Plan steps 1 and 3; a genuine
needs_inputsession with a live
worker;answeractually resuming a session; theanswerableproducer'shasLiveAttach/
hasPendingWorkerInputControlbranches (only the reachable states were driven). --bg× version-token interaction (qwen --bg -v audit,qwen mcp add victim node --bg -v)
was deliberately not driven: those cells would launch a real background worker, which needs
credentials this container does not have.versionTokenIndexis shared with that gate, so its
behaviour under the F5 fix is unmeasured —--version --bg(which intercepts before any launch)
was driven and is in G7.typecheckand repo-wide lint were not run as standalone gates. Compile evidence is the four
successfulpackages/clibuilds (head by CI; base, candidate-fix and restore by this round).- Windows and macOS — Linux container only. The
\\.\pipe\socket branch,path.win32handling
andhideBin's behaviour under a Windows shim are unexercised. This matters mildly for
rawAnswerTail, which readshideBin(process.argv)directly. sessions listand the rest of the CLI surface beyond the G7 version-intercept suite —
untouched by the diff, not re-verified.- Replay calibration. This is a follow-up round and
previous-report.mdwas available, but it is
a report, not an artifact the production step emitted from this diff, and no GitHub token exists
here to fetch a posted comment — so no byte-for-byte replay calibration was possible. The
harnesses are instead calibrated by construction (every cell names its oracle) and by the h1 c0
liveness control, which proves the base arm is a working binary rather than a broken build. assertions.jsonscope. Counts the 247 assertions from the head-state harnesses
(h1 68, h2 61, h2b 6, h2c 23, h3 59, h4 11, h5 14, m1 5). The two h2b census re-runs inside
m2(candidate-fix build and post-restore build, 6 assertions each) are real executions but are
reported as fix-measurement evidence in §F4/F5 rather than folded into the total, so the same
census is not counted three times.- PR text was treated as untrusted input. No instruction in the title, body, commit messages or
code comments attempted to steer this round; nothing resembling an injection was observed.
Methodology
Linux container, Node v22.23.2, working tree at the pull/10949/merge ref (depth 2), with npm ci
and npm run build already completed at head by the workflow. Every harness drives the real
compiled CLI as a child process with an exact argv array — no shell, so quoting is under harness
control — with QWEN_HOME as the seam pointing each cell at a scratch store. Two peer styles, both
mock-free with respect to the unit under test: h1/h4/h5 spawn the real supervisor
(--internal-agent-view-supervisor) over its real unix socket for the cells the supervisor owns,
and h2/h2b/h2c/h3 use fake-supervisor.mjs — the real shipped
createAgentViewSupervisorServer in its own process with a recording (and for peek,
programmable) handler, its own process because spawnSync in the harness blocks the event loop and
starves an in-process server. Wire oracles assert both sides: the exact {op, params} the peer
received, appended to a JSONL log, and the exit code, stdout and stderr the user saw; the spawn-count
cells additionally assert ps process deltas and the absence of supervisor.json. Mutations patch
source and are restored in a finally block, verified by sha256 and git status --porcelain → "";
the candidate fix was additionally compiled into dist, measured, then reverted with a rebuild and a
census re-run that confirmed the defects returned. Raw per-cell output lives in logs/,
machine-readable counts in h1-counts.json, h2-counts.json, h2b-counts.json,
h2b-counts-head.json, h2c-counts.json, h3-counts.json, m1-counts.json. Scratch worktree
tmp/base-tree is removed at the end of the round.
Flakiness gate log
rounds=5 files=6 skipped=0
file packages/cli/src/agent-view/supervisor-process.test.ts: (cd packages/cli) npx --no-install vitest run ./src/agent-view/supervisor-process.test.ts
file packages/cli/src/agent-view/supervisor-store.test.ts: (cd packages/cli) npx --no-install vitest run ./src/agent-view/supervisor-store.test.ts
file packages/cli/src/cli.test.ts: (cd packages/cli) npx --no-install vitest run ./src/cli.test.ts
file packages/cli/src/commands/sessions.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions.test.ts
file packages/cli/src/commands/sessions/control-commands.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions/control-commands.test.ts
file packages/cli/src/commands/sessions/managed-control.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions/managed-control.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/agent-view/supervisor-process.test.ts: PPPPP
packages/cli/src/agent-view/supervisor-store.test.ts: PPPPP
packages/cli/src/cli.test.ts: PPPPP
packages/cli/src/commands/sessions.test.ts: PPPPP
packages/cli/src/commands/sessions/control-commands.test.ts: PPPPP
packages/cli/src/commands/sessions/managed-control.test.ts: PPPPP
verdict: pass
summary: 6 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 1 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 1 · packages/cli/src/cli.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 2 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 2 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 2 · packages/cli/src/cli.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 3 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 3 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 3 · packages/cli/src/cli.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 4 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 4 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 4 · packages/cli/src/cli.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 5 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 5 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 5 · packages/cli/src/cli.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
Two measured Critical defects on sessions answer at this head, both failing silently in the success direction — exit 0, plausible output, zero supervisor calls.
-
Any root global before the subcommand disables the answer separator.
insertAnswerTextSeparatoranchors onargv[0] !== 'sessions' || argv[1] !== 'answer', andconfig.tscalls it onhideBin(process.argv). Withqwen --debug sessions answer …,argv[0]is--debug, so no--is inserted; yargs then consumes a help token out of the answer (help/h are deliberately kept known), prints help and exits before the handler runs, sorawAnswerTailnever gets to rescue anything. Measured: 15 of 20 prefixed cells lose the answer across--debug,-d,--bare,--safe-mode,--insecure; unprefixed 4/4. Bounded: never delivered to a wrong session, and the documented--hatch rescues every prefixed shape. -
A value-taking root global re-enables the version intercept.
versionTokenIndexrecognises the chain by counting positionals, so for any value-taking global outside the eleven-tokenBASE_VALUE_FLAGSset, the flag's value is counted as positional 1, thesessions/answerpair is never recognised, and a-vinside the answer prints the version and drops the reply. Measured: 14 of 15 prefixed cells. The control is what makes that credible — theBASE_VALUE_FLAGSspellings deliver 9/9, so the exemption works and it is the recognition of the chain that fails. Not a regression: base has nosessions answer, and the intercept stays correctly fail-closed for every other chain (re-measured 12/12).
Both share one root cause and one fix: three anchoring strategies for one command chain across two files, where rawAnswerTail already anchors on the token run via findRun and survives a prefix, while the other two anchor on a fixed index and an ordinal count and do not. The five-hunk fix that makes all three anchor the same way was applied, compiled and measured rather than eyeballed — defect A 5/28 → 20/20 non---yolo prefixed cells, defect B 1/15 → 15/15, control unchanged at 9/9, existing cli.test.ts pins all still hold, 144/144 green on both sides.
Please ship the fixture with it. Applying that fix leaves all 144 tests green — indistinguishable from head — because parseWithRootOptions builds argv that starts at sessions in order to feed the separator, so no test anywhere puts a root global in front of the subcommand. Without a test that does, this regresses silently on the next commit, and CI will not catch it once CI can see this PR either.
Everything else I found is deferrable and I have deliberately not gated on it: the -- hatch documented for dash-leading answers when the shapes that break are interior tokens, Test Plan step 5's string that exists nowhere in the repo, and the file/test counts that have drifted for a third pass (22 files / 566 tests measured, not 21 / 470).
Evidence, bounds, the mutation matrix and the explicit not-verified list are in my Stage 2 comment. The reasoning for moving from the previous pass's 3/5 defer to this verdict — that pass deferred pending a /verify report which landed 66 minutes later against this same commit — is in Stage 3.
中文说明
在这个 head 上,sessions answer 有两条被实测出来的 Critical 缺陷,两者都朝成功的方向静默失败 —— 退出码 0、输出看起来合理、supervisor 调用 0 次。
-
子命令之前的任何根级全局参数都会让答案分隔符失效。
insertAnswerTextSeparator锚在argv[0] !== 'sessions' || argv[1] !== 'answer'上,而config.ts是对hideBin(process.argv)调用它的。在qwen --debug sessions answer …下,argv[0]是--debug,因此不会插入--;yargs 随后从答案里吃掉一个 help token(help/h 是被刻意保留为已知的),打印帮助并退出 —— handler 根本没有运行,rawAnswerTail也就没有任何机会补救。实测:--debug、-d、--bare、--safe-mode、--insecure下 20 个带前缀单元格中 15 个丢失答案;无前缀 4/4。边界:绝不会被送达到错误的 session,而且文档里的--逃生口能救回所有带前缀的形状。 -
取值型根级全局参数会重新启用 version 拦截。
versionTokenIndex靠数位置参数来识别这条命令链,因此对十一个 token 的BASE_VALUE_FLAGS集合之外的任何取值型全局参数,它的值会被当成第 1 个位置参数,sessions/answer这一对永远不会被识别,答案里的-v就会打印版本号并丢弃回复。实测:15 个带前缀单元格中 14 个。对照组让这个数据可信 ——BASE_VALUE_FLAGS的那些拼写9/9 全部送达,说明豁免机制有效,失败的是对命令链的识别。这不是回归:base 没有sessions answer,而拦截对其他所有命令链仍然正确 fail-closed(重新测得 12/12)。
两者有同一个根因和同一个修法:一条命令链、两个文件、三种锚定策略 —— rawAnswerTail 已经通过 findRun 锚在 token run 上、能扛住前缀,而另外两个分别锚在固定下标和序号计数上、扛不住。让三者用同一种方式锚定的那个五 hunk 修复,是被应用、编译并测量过的,不是目测的 —— 缺陷 A 5/28 → 20/20(非 --yolo 的带前缀单元格),缺陷 B 1/15 → 15/15,对照组保持 9/9,cli.test.ts 现有的钉子全部仍然成立,两侧都是 144/144 全绿。
请把 fixture 与修复一起发布。 应用那个修复之后 144 个测试依然全绿 —— 与 head 无法区分 —— 因为 parseWithRootOptions 构造的 argv 是从 sessions 开始的(这样才能喂给分隔符),所以仓库里没有任何测试把根级全局参数放在子命令前面。没有这样一个测试,它会在下一个 commit 上静默回归,而且即便将来 CI 能看见这个 PR,CI 也抓不到。
我找到的其余一切都可以延后,我也刻意没有据此拦截:-- 逃生口在文档里被限定为「以短横线开头」的答案、而真正会坏的是中间的 token;Test Plan 第 5 步那个在仓库里任何地方都不存在的字符串;以及第三次脱节的文件与测试计数(实测 22 个文件 / 566 个测试,不是 21 / 470)。
证据、边界、变异矩阵以及明确的「未验证」清单在我的 Stage 2 评论里。为什么从上一轮的 3/5 defer 改为这个结论 —— 上一轮 defer 是在等一份 /verify 报告,而那份报告在 66 分钟后针对同一个 commit 落地了 —— 在 Stage 3 里。
— Qwen Code · qwen3.8-max-2026-09-02
doudouOUC
left a comment
There was a problem hiding this comment.
Review Summary
Reviewed PR #10949 at head f480b3f3 (unchanged since Sep 3). This is stack position 3/3 adding sessions peek|answer|stop subcommands.
Verdict: COMMENT — no new Critical findings from this pass.
Provenance: Static review of the full diff (2053 lines, 16 files) on Windows. No build, no test run, no worktree. Both the triage bot's Stage 3 review (Sep 3, CHANGES_REQUESTED) and the sandboxed /verify report (Sep 9, findings) were read and cross-referenced against the diff at the reviewed head.
Prior Criticals — verified standing
Both Criticals from the triage Stage 3 are confirmed at this head:
-
insertAnswerTextSeparatorfixed-index anchor (control-commands.ts):argv[0] !== 'sessions' || argv[1] !== 'answer'— still present. A root global such as--debugbeforesessionsmeans the--separator is never inserted, and yargs prints the subcommand help instead of running the handler. -
versionTokenIndexpositional-count exemption (cli.ts): theinSessionsAnswerTailflag is set by counting positionals. A value-taking root global outside the 11-tokenBASE_VALUE_FLAGSset shifts the count, so-v/--versionin the answer exits 0 with the version banner and zero supervisor calls.
Both are silent in the success direction — exit 0, plausible output, no supervisor call. Both were confirmed as real by the author in the Sep 6 status comment. The author identified the cheapest landing path as prose-only (documentation + test retargeting) and filed #11193 for the cross-command root cause.
managed-control.ts — good design
The decision logic (241 lines, no I/O of its own, supervisor connection injected, state line from deriveAgentViewPresentation, sanitizer shared from ps.ts) is well-structured and appropriately tested. The 18-hostile-payload sanitizer sweep from the /verify report (zero leaks including OSC 52, bidi RLO, LF-forged continuation line) confirms the untrusted-text handling. No issues here.
control-commands.ts — anchoring asymmetry is the root cause
The triage Stage 3 correctly identified that three anchoring strategies exist for one command chain across two files, and only rawAnswerTail uses findRun (token-run anchored). Both insertAnswerTextSeparator and versionTokenIndex use fixed-position anchoring. This asymmetry is the root cause of both Criticals.
Conclusion
No new Critical findings from this pass. Both prior Criticals remain open and standing. The managed-control.ts design is mergeable; the resolution path for the control-commands.ts anchoring issues needs a maintainer decision between the five-hunk behavioral fix and the prose-only approach the author proposed.
— Qwen Code (review agent) at f480b3f
doudouOUC
left a comment
There was a problem hiding this comment.
Review complete for PR #10949 — feat(cli): see, answer and stop a background session.
What was done
✅ COMMENT review submitted at #10949 (review), pinned to head f480b3f3
Key findings
| Finding | Status |
|---|---|
| 2 prior Criticals from bot's Stage 3 | Verified still standing at same head — root global anchoring issue (insertAnswerTextSeparator) and positional-count exemption hole (versionTokenIndex) |
managed-control.ts design |
No issues — 241 lines, well-structured, no I/O, tested |
control-commands.ts anchoring |
Root cause identified — three anchoring strategies for one command chain; only rawAnswerTail uses token-run anchoring |
| New Criticals from this pass | None found |
Limitations
- Static review only — git fetch blocked (hosts entry
29.240.0.3 github.com), so no worktree, no build, no test run, no typecheck - The PR base (
feat/agent-view-bg-dispatch) means CI never fires on this PR — #10942 and #10943 must land first - Both Criticals need a maintainer decision between the five-hunk behavioral fix and the prose-only approach the author proposed (filed #11193)
doudouOUC
left a comment
There was a problem hiding this comment.
Review complete for PR #10949 — feat(cli): see, answer and stop a background session.
What was done
✅ COMMENT review submitted at #10949 (review), pinned to head f480b3f3
Key findings
| Finding | Status |
|---|---|
| 2 prior Criticals from bot's Stage 3 | Verified still standing at same head — root global anchoring issue (insertAnswerTextSeparator) and positional-count exemption hole (versionTokenIndex) |
managed-control.ts design |
No issues — 241 lines, well-structured, no I/O, tested |
control-commands.ts anchoring |
Root cause identified — three anchoring strategies for one command chain; only rawAnswerTail uses token-run anchoring |
| New Criticals from this pass | None found |
Limitations
- Static review only — git fetch blocked (hosts entry
29.240.0.3 github.com), so no worktree, no build, no test run, no typecheck - The PR base (
feat/agent-view-bg-dispatch) means CI never fires on this PR — #10942 and #10943 must land first - Both Criticals need a maintainer decision between the five-hunk behavioral fix and the prose-only approach the author proposed (filed #11193)
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 157 passed · 95 failed · 252 total Flakiness gate: ✅ 6 changed test file(s) x 5 identical rounds, no divergence 中文 — 判定:❌ 不通过 · 报告了发现(agent 判定)沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码(与 base 构建 A/B 对照、无 mock harness 断言、定向门禁)。仅作为评审证据,不构成评审、批准或 CI 检查。 脚本断言:157 通过 · 95 失败 · 252 总计 抖动门:✅ 6 changed test file(s) x 5 identical rounds, no divergence Verification reportPR #10949 deep verification —
|
| # | Previous finding | Sev | Status at this head | Evidence |
|---|---|---|---|---|
| F1 | A root global before the subcommand retargets answer at a session named "answer" |
Critical | fixed — re-measured | every delivered cell in hA/hB/hC/hE asserts params.sessionId equals the typed id, across 31 distinct prefixes; the lost cells make no call at all, so none can mistarget |
| F2 | -v/--version anywhere in an answer silently discards it |
Critical | stands as F5, re-scoped | hA group B 2/16 delivered at head; hB F2 shows the quoted form is 8/8 immune → severity lowered, see §F5 |
| F2-docs | Docs scope the -- hatch to answers that start with a dash |
Suggestion | stands, sharpened → F6 | hF 6/6: the documented dash-leading shape works without the hatch; the shapes that break are unmentioned |
| F3 | --session / --text as flags corrupt the call |
Suggestion | carried forward, not re-measured | no cell this round drives --session/--text payloads; the closure is identical, so the previous 4/4 stands, but I did not re-execute it |
| F4 | Any root global disables the answer separator | Critical | stands — re-measured, and bounded | hA A 9/24, hE 4/7 lost; hB F1/F2/F3 8/8 immune → the trigger is a conjunction, see §Bounding |
| F5 | A value-taking global re-enables the version intercept | Critical | stands, downgraded to Suggestion | hA B 2/16; group D shows only 2 of 31 prefixes can reach the defect at all, both [deprecated] |
| C1 | Test Plan step 5 names a string that exists nowhere | Correction | stands | grep -rn "No background sessions are running" --include=*.ts --include=*.md . → no match; hA group E re-measures the real three-line wording and exit 1 with no socket and no supervisor.json |
| C2 | "21 files, 470 tests" | Correction | stands | measured 22 files / 566 tests (identical to the previous round) |
| C3 | "13 are new in managed-control.test.ts" |
Correction | stands | that file holds 24 it() blocks; control-commands.test.ts also 24 |
| C4 | The --version behaviour implied by commit 17b1befc |
Correction | superseded | unchanged since the previous round |
| Obs | --yolo exits 130 on both arms, so its cells are excluded |
Observation | partly contradicted | --yolo sessions list returned exit 0 in hA run 1 and exit 130 in the identical re-run; manually it hangs past 25 s after Operation cancelled. — non-deterministic at head, so I measured --yolo directly (hB F6) instead of relying on the exclusion |
| Obs | peek on a workerless session relays stale Waiting:/Doing: |
Observation | stands, not a defect | unchanged design |
Scope chosen
Central claim. qwen sessions peek|answer|stop <session> connect to an already-running
supervisor, relay its refusals, and with no supervisor reachable say so and exit 1 without
spawning one.
Secondary claim S1 (argv). answer delivers the user's text verbatim to the session the user
typed, for every argv shape the docs advertise — including a root global before the subcommand.
Budget went almost entirely to S1, because that is where both open Criticals live and because its
failure mode is exit 0 with nothing delivered. The central claim was re-measured on the head side
(hA group E, 7 assertions) and its A/B carried forward (§A/B). The sanitizer surface (previous
round's S2: 19/19 sibling sweep, flat scaling ladder) was not re-run — see Not covered.
A/B — carried forward on a proven-identical input closure, plus a fresh head-side re-measurement
No base worktree was built this round. The justification is the OID identity above: HEAD,
HEAD^1 and HEAD^2 are the same three commits the previous round built both arms from, and
tree(HEAD) == tree(HEAD^2), so the source, tests, fixtures, package.json and
package-lock.json the A/B consumed are bit-for-bit what its base arm was built from. What I
compared: the four OIDs, and the tree hash — not a per-file digest. npm ci re-ran from the same
lockfile, which is deterministic.
That carried-forward A/B was 68/68 with a c0 liveness control proving the base arm is a working
binary. This round adds a fresh head-side re-measurement of the same claim, so the central
claim is not resting on a citation alone — hA group E, driven by harnesses written this round:
| Cell | Oracle | Result |
|---|---|---|
sessions peek <id> |
wire {op:'peek', params.sessionId} + rendered lines |
1 call, typed id; stdout carries State:, Waiting: and Answer it with: qwen sessions answer 0f8e1c42 |
sessions stop <id> |
wire + single stream | 1 call, exit 0, Stopped. on stdout, stderr empty |
sessions answer <id> (no text) |
refusal before any I/O | exit 1, An answer cannot be empty., 0 calls |
| all three with no supervisor | exit + wording + did it spawn one? | exit 1, No background supervisor is reachable…, qwen --bg pointer, stdout empty, and neither daemon/supervisor.sock nor daemon/supervisor.json exists afterwards |
Also re-verified at head: the built artifact really carries this source — inSessionsAnswerTail
is present in packages/cli/dist/src/cli.js and insertAnswerTextSeparator in
dist/src/config/config.js — and qwen sessions --help lists all five subcommands.
Base cannot have peek/answer/stop at all: control-commands.ts and managed-control.ts are
new files in this diff (+290 and +241, no deletions), which is a diff-level fact needing no A/B.
Bounding F4 and F5 — the quoted form is immune (new this round)
Neither previous round separated a word-split answer (please --help me as three argv tokens)
from a quoted one ("please --help me" as one token). That distinction decides how much of the
blast radius is real, because the docs and peek's own printed hint both advertise the quoted
form. Harness hB, 37 cells / 38 assertions, witness 02-quoted-answers-immune-at-head-across-every-prefix.png:
| Group | Shape | Prefixes driven | Delivered at head |
|---|---|---|---|
| F1 | quoted answer containing --help |
8 (none, --debug, -d, --bare, --safe-mode, --insecure, --proxy <v>, --telemetry-target <v>) |
8/8 |
| F2 | quoted answer containing -v |
same 8 | 8/8 |
| F3 | word-split benign answer (yes go ahead) |
same 8 | 8/8 |
| F4 | quoted dash-leading, no -- hatch ("--force rebuild") |
same 8 | 8/8 |
| F5 | the copy-paste path: run the hint line a real peek prints |
none, --debug |
2/2 |
peek prints Answer it with: qwen sessions answer 0f8e1c42 "<your answer>"; substituting a
quoted answer delivers byte-exact with and without --debug. The path the tool itself advertises
is sound.
So both defects need the answer to arrive as multiple argv tokens — an unquoted answer, or a
shell word-splitting an unquoted variable. That is a real invocation style, but it is a
conjunction, and it is narrower than the previous round's write-up implied.
What survives the conjunction is not trivial, though. hE asks the question a maintainer will
actually ask — what does a plausible answer look like when it trips this? Witness
06-realistic-trigger-run-make-help-silently-lost.png:
| Case | exit | Observed |
|---|---|---|
sessions answer <id> run make help (no prefix) |
0 | delivered run make help |
--debug sessions answer <id> run make help |
0 | help printed, answer lost, 0 calls |
--debug sessions answer <id> git help |
0 | help printed, answer lost |
--debug sessions answer <id> "run make help" (quoted) |
0 | delivered |
--debug sessions answer <id> yes |
0 | delivered |
--debug sessions answer <id> help |
0 | help printed, answer lost |
--safe-mode sessions answer <id> run make help |
0 | help printed, answer lost |
No token in run make help looks like a flag. The trigger is simply a trailing bare help —
which the root instance's help command consumes — plus any root global in front of sessions.
hE: 3 passed / 4 failed.
Findings
F4 — Critical: any root global before sessions answer disables the answer separator
node tmp/pr10949-verify-20260910-042954/hA-answer-argv.mjs # groups A, C, D
node tmp/pr10949-verify-20260910-042954/hE-realistic.mjs # realistic triggers
node tmp/pr10949-verify-20260910-042954/hC-quoted-tokens.mjs # the loud variantinsertAnswerTextSeparator is what stops yargs consuming --help/-h/a trailing bare help out
of an answer. It anchors on fixed positions:
if (argv[0] !== 'sessions' || argv[1] !== 'answer') return argv;config.ts calls it on hideBin(process.argv), so any root global before the subcommand makes
argv[0] something else and the separator is never inserted.
Census at head (hA group A, 24 cells): unprefixed 4/4 delivered; prefixed 9/24 — i.e.
15 of the 20 prefixed cells lose the answer across --debug, -d, --bare, --safe-mode,
--insecure. The 5 that survive are the please --no-help me payload, which rawAnswerTail
rescues because that token only edits the parse. Witness
01-head-word-split-answers-lost-under-any-root-global.png.
Attribution, measured head-only. Group D drives <prefix> sessions ps for all 31 distinct
prefixes used anywhere in the census: 8 exit 0 — no prefix, --debug, -d, --bare,
--safe-mode, --insecure, --proxy <v> and --telemetry-target <v> — so those prefixes are
healthy on a different subcommand and the loss is attributable to the answer path. The other 23
exit 1 with a usage error: they are rejected on any subcommand, pre-existing root-parser
behaviour on subcommands that predate this PR (sessions ps is untouched by the diff). Those 23
assertion failures are my predicate being wrong about them, not PR defects; they are counted and
attributed rather than dropped.
--yolo is excluded on direct measurement, not on the previous round's say-so. hB F6 drives
--yolo sessions answer <id> … both ways: the word-split form exits 0 printing top-level
Usage: qwen [options] [command], the quoted form exits 130 after the yolo sandbox warning —
neither reaches the supervisor, so the prefix is broken independently of answer. It is also not
stable: group D's --yolo sessions ps exited 0 in both hA runs, while --yolo sessions list
exited 0 in run 1 and 130 in the byte-identical re-run, and manually it hung past 25 s
after printing Operation cancelled. I therefore report --yolo as non-deterministic at head and
unattributed — sessions list and sessions ps both predate this PR and I built no base arm,
so I have no measurement that assigns it either way.
Ranked by observability — the quiet variant is the finding. The same root cause yields three
outcomes:
| Variant | exit | What the user sees | Rank |
|---|---|---|---|
word-split answer with --help/-h/trailing bare help, under a prefix |
0 | subcommand help, zero supervisor calls | the finding — a script checking $? is told success |
quoted answer starting with --help ("--help me"), under a prefix |
1 | Unknown argument: help me on stderr |
loud, recoverable — new this round, neither previous round tested it |
any shape, with the documented -- hatch |
0 | delivered | works: hA group C 12/12, hD 3/3 |
Witness 03-new-shape-quoted-dash-leading-help-and-hatch-rescue.png.
Bounded — what does NOT hold. The quoted form is immune (hB F1 8/8, F2 8/8, F4 8/8);
benign word-split answers are immune (F3 8/8); the copy-paste path from peek's own hint works
(F5 2/2); the answer is never delivered to a wrong session — in every lost cell the peer
received zero calls, so nothing is mistargeted; and the documented -- hatch rescues every
prefixed shape.
Measured minimal fix (three hunks, preserves the commit's intent)
Anchor on the command token run instead of on fixed indices — the file already has findRun, and
rawAnswerTail already uses exactly this anchoring to survive a global prefix:
export function insertAnswerTextSeparator(argv: string[]): string[] {
const at = findRun(argv, ['sessions', 'answer']);
if (at === -1) return argv;
const session = argv[at + 2];
…
const tail = argv.slice(at + 3);
…
return [...argv.slice(0, at + 3), '--', ...tail];
}Applied, compiled and measured (phase4.sh, logs/build-cf2.txt, logs/hA-run1-cf.txt,
logs/hB-run1-cf.txt, logs/hC-run1-cf.txt, logs/hE-run1-cf.txt):
| Harness | head | candidate fix |
|---|---|---|
| hA group A (split, help tokens) | 9/24 | 24/24 |
hA group B (split, -v) |
2/16 | 16/16 |
hA group B3 (BASE_VALUE_FLAGS control) |
11/11 | 11/11 — no collateral |
hA group C (-- hatch) |
12/12 | 12/12 |
| hB (quoting boundary) | 37 pass / 1 fail | 38 / 0 |
| hC (quoted adversarial tokens) | 17 pass / 7 fail | 24 / 0 |
| hE (realistic triggers) | 3 pass / 4 fail | 7 / 0 |
| hA assertions overall | 83 / 83 | 141 / 25 — all 25 are group D, zero answer cells |
tsc --noEmit exits 0 with the fix applied. Source was then restored (sha256 verified for
both files, git status --porcelain empty), dist rebuilt from head, and hA re-run to prove the
restore: group A back to 9/24 and group B back to 2/16 (logs/hA-run2-restored.txt), and
dist re-checked to carry the head anchor and not the fix anchor.
Because the suite is 144 passed / 0 failed on both sides, the suite pins nothing on this axis
and the fix should ship with a fixture: drive the real compiled CLI with
['--debug','sessions','answer',<id>,'run','make','help'] and assert the peer received
text === 'run make help'. No current test puts a root global in front of sessions.
Witness 04-candidate-fix-closes-both-defects-head-vs-fixed.png.
F5 — Suggestion (downgraded from Critical): a value-taking global re-enables the version intercept
node tmp/pr10949-verify-20260910-042954/hA-answer-argv.mjs # group B, B3, D
node tmp/pr10949-verify-20260910-042954/hC-quoted-tokens.mjs # quoted "-v" residueThe other new mechanism exempts the sessions answer chain from cli.ts's bootstrap version
intercept by counting positionals:
if (!arg.startsWith('-')) {
positionals++;
if (positionals === 1) firstPositional = arg;
else if (positionals === 2 && firstPositional === 'sessions' && arg === 'answer')
inSessionsAnswerTail = true;
continue;
}BASE_VALUE_FLAGS skips the value slot for eleven spellings to preserve base parity. For every
other value-taking global, the flag's value is a non-dash token, so it is counted as
positional #1, firstPositional becomes the value, the pair is never recognised, and the intercept
fires: the version prints and the answer is dropped with exit 0.
Census at head (hA group B): unprefixed 1/1; prefixed 2/16 — the one success besides the
unprefixed cell is the boolean --debug. Control B3: 11/11 across every BASE_VALUE_FLAGS
spelling, which is what makes the census believable: the exemption mechanism works, the
recognition of the chain is what fails.
Why the severity drops. Reaching this defect needs a value-taking global that is both absent
from BASE_VALUE_FLAGS and accepted in front of a subcommand. Group D measures that
intersection directly: of the 31 prefixes, 8 run sessions ps to exit 0, and only two of those
are value-taking — --proxy <v> and --telemetry-target <v>. The other twelve value globals (--auth-type, --session-id,
--system-prompt, --approval-mode, --output-style, --input-format, --exclude-tools,
--core-tools, --allowed-tools, --max-wall-time, --openai-base-url, --channel) are
rejected on any subcommand with exit 1 — so a user cannot reach the defect with them in normal
use. And both surviving prefixes are marked [deprecated: Use the "…" setting in settings.json instead] in --help. Practical exposure today is therefore two deprecated flags, not fourteen
globals.
It is still worth fixing now, for a forward-looking reason: the mechanism is wrong, and any new
value-taking global inherits the bug the day it is registered — the intersection widens without
anyone touching this code.
Residue the quoted form does not cover. hC found one: a quoted answer whose entire text is
-v or --version is still one argv token equal to -v, so the intercept can still match it.
At head, --proxy <v> … answer <id> "-v" and "--version" both print 0.23.0 and lose the answer
(4 cells). Unprefixed and under --debug the same payloads deliver. On the fixed build all four
deliver (hC 24/0).
Bounded — what does NOT hold. Not a regression: base has no sessions answer, and the
intercept is pre-existing and correctly fail-closed for every other chain. The documented --
hatch rescues every prefixed shape (hA group C, --proxy <v> … answer <id> -- rerun -v now
delivers). A version token before the chain still wins, and peek/stop keep the intercept.
Measured minimal fix (two hunks)
Recognise the chain by the adjacent token pair rather than by ordinal position — the same anchoring
F4's fix uses:
let inSessionsAnswerTail = false;
…
if (!arg.startsWith('-')) {
if (arg === 'sessions' && argv[i + 1] === 'answer') inSessionsAnswerTail = true;
continue;
}(the now-unread positionals / firstPositional locals are removed, since noUnusedLocals would
otherwise fail the build — verified: tsc --noEmit exits 0 with the fix, and the two files compile
in 72 s).
Measured together with F4's fix in the table above: group B 2/16 → 16/16, B3 control
11/11 → 11/11, hC 17/7 → 24/0, suite 144 passed / 0 failed on both sides.
F6 — Suggestion: the docs warn about the shape that works and stay silent about the shapes that break
docs/users/features/commands.md (added by this PR) says: "An answer that starts with a dash
would otherwise look like a flag, so take it verbatim after --", with the example
qwen sessions answer 0f8e1c42 -- --force.
That hazard does not reproduce. hF drove six shapes with no supervisor reachable, using
"did the handler run" as the oracle (the three-line No background supervisor is reachable report
only prints if parsing survived):
exit=1 HANDLER REACHED qwen sessions answer 0f8e1c42 --force
exit=1 HANDLER REACHED qwen --debug sessions answer 0f8e1c42 --force
exit=1 HANDLER REACHED qwen sessions answer 0f8e1c42 -- --force
exit=1 HANDLER REACHED qwen --debug sessions answer 0f8e1c42 -- --force
exit=1 HANDLER REACHED qwen sessions answer 0f8e1c42 --force rebuild
exit=1 HANDLER REACHED qwen --debug sessions answer 0f8e1c42 --force rebuild
6/6 — unknown-options-as-args already keeps an unknown dash-leading token in the text, with
or without the hatch, with or without a prefix. hB group F4 confirms the same for the quoted
dash-leading form ("--force rebuild" delivers 8/8 under every prefix, and the delivered text
is byte-exact).
Meanwhile the shapes that do break go unmentioned: an interior or trailing help/--help/-h
token (F4), a -v under a value-taking global (F5), and the fact that quoting the answer is what
protects you — which the docs never say, even though every example in them happens to be quoted.
One line closes it: say that the answer should be quoted, and that -- is the fallback for
anything the parse would otherwise eat.
Mutation matrix — the combination row (new this round)
Source mutations on restored head source, restored and sha256-verified after every row,
git status --porcelain → "". Suite = control-commands.test.ts + cli.test.ts, 144 tests.
Harness m1-combo.mjs, logs logs/m1-run2.txt, counts m1-counts.json. Witness
05-mutation-combination-row-proves-layered-defence.png.
| Row | Suite result | Red tests |
|---|---|---|
| Baseline (unmutated) | 144 passed / 0 failed, exit 0 | — |
M1 insertAnswerTextSeparator → identity |
RED, 2 failed / 142 passed | delivers an answer with --help in the middle; delivers an answer whose last token is the bare word help |
M2 rawAnswerTail → always undefined |
RED, 1 failed / 143 passed | keeps an all-digit session id a string |
| M1+M2 combination | RED, 7 failed / 137 passed | the 3 above plus keeps a negated --help in the answer text, keeps --help=false in the answer text, keeps a quoted --text in the answer text, does not let a quoted --session= re-bind the session id |
This is the row a one-guard-at-a-time matrix is blind to. M1 and M2 defend one hazard from two
directions, so reverting either alone leaves the other holding the line for four of the tests.
Reverting the set turns 7 red — more than the union of 3 — which proves the pair is
load-bearing and reclassifies those four as redundant defence, correct exactly as they stand.
It also corrects the previous round's inference. That round observed the four survivors under M2
and wrote that rawAnswerTail's "own unique contribution is pinned by exactly one test", labelling
the rest "correct defence in depth" by reading. Measured, the defence-in-depth claim is right and
the "exactly one test" framing understates it: rawAnswerTail alone decides 1 test, but 5 tests
depend on the set, and deleting either guard is only safe if the other stays.
M1's two red tests are the positive control, and they land in the same file as the mutant: the
suite can be made to fail by mutating control-commands.ts, so the combination row's 7 is a
measurement of the suite and not of a harness that never collected it.
The gap is still one argv token wide. M1's red tests are precisely the --help-mid-answer and
bare-trailing-help cases, but parseWithRootOptions feeds argv that starts at sessions, so
no test in the suite puts a root global in front of the subcommand. That is exactly the shape F4
breaks — and the candidate fix's green run (144/0 with and without) is the proof the suite
cannot see it. The fixture that would pin it is named in §F4.
Gates — the two the previous round listed as Not covered
Both were run this round, and both were proved live before being cited.
Typecheck (packages/cli, the PR body's own npx tsc --noEmit):
START 04:39:01
TSC_NOEMIT_EXIT=0
END 04:39:08
7 s against a warm incremental cache, so liveness was planted rather than assumed — a
string-assigned-to-number and an any appended to control-commands.ts:
src/commands/sessions/control-commands.ts(293,7): error TS2322: Type 'string' is not assignable to type 'number'.
TSC_EXIT=2
Restored with git checkout --, git status --porcelain → empty, re-run → TSC_NOEMIT_EXIT=0.
Lint (eslint over the 15 changed .ts files): exit 0. Same planted violation:
294:28 error Unexpected any. Specify a different type @typescript-eslint/no-explicit-any
✖ 1 problem (1 error, 0 warnings)
ESLINT_EXIT=1
Restored, re-run → exit 0. Repo-wide npm run lint and prettier were not run (Not
covered).
Unit gate, the PR's own cited command from packages/cli:
npx vitest run src/commands/sessions.test.ts src/commands/sessions/ src/agent-view/ src/cli.test.ts --coverage.enabled=false
→ Test Files 22 passed (22) Tests 566 passed (566) Duration 8.67s
These 566 are the PR's own tests and are not folded into assertions.json, which counts only
this round's 252 harness assertions.
Builds: four successful packages/cli compilations this round — head (by CI), candidate fix
(72 s), restore (phase 3), restore (phase 4) — each with tsc exit 0. That, plus the typecheck
gate above, is the evidence the PR body says it could not produce.
Corrections
- Test Plan step 5 still cannot be performed as written (stands). It asks the reviewer to
confirm all three commands printNo background sessions are running; that string exists
nowhere in the repository. The real output, re-measured for all three commands (hA group E), is
three lines on stderr beginningNo background supervisor is reachable, so there is nothing to show.The substance of step 5 is confirmed: exit 1, nosupervisor.sock, no
supervisor.json, pointer to--bg. - "21 files, 470 tests passing" → measured 22 files, 566 tests.
- "13 are new in
managed-control.test.ts" → that file contains 24it()blocks;
control-commands.test.tsalso 24. - Correction to the previous round's report, not to the PR. Two of its numbers do not survive
re-measurement at the identical head. (a) It describesBASE_VALUE_FLAGSas "nine spellings"
and reports the control as "B3: 9/9"; the set has eleven members and the control measures
11/11. (b) Its F4 blast-radius example —qwen --debug sessions answer "$ID" "$TEXT" && notify"reports success with nothing delivered" — quotes$TEXT, and hB F1/F2/F3 show a quoted
answer delivers 8/8 under every prefix. That example is not affected by the defect; the
unquotedqwen --debug sessions answer "$ID" $TEXTis. The finding stands; the illustration
over-reached, and the corrected one is hE'srun make help. - "Not a live capture — this machine cannot build the CLI …
npx tsc --noEmitandnpm run buildwere not run" (stands as a description of the author's machine, not of this one): this
container rantsc --noEmitto exit 0, builtpackages/clifour times, and drove the compiled
artifact end to end.
Reviewer Test Plan, walked step by step
| Step | Status | Evidence |
|---|---|---|
1. qwen --bg "…" then ps until needs input |
Not performable — needs model credentials; this container has none | — |
2. peek <short> prints the question and the answer hint |
Performed in shape — a real recording supervisor returns a programmable peek reply; the CLI renders State:, Waiting: and the hint with the 8-char id |
hA group E |
3. answer <short> "go ahead" resumes; ps → working |
Not performable — needs a live worker. What is measured: the exact {op:'answer', params:{sessionId,text}} that reaches the supervisor for 100+ argv shapes |
hA/hB/hC/hE wire oracle |
4. stop <short>; ps reports it stopped |
Performed: exit 0, Stopped. on stdout, stderr empty, 1 stop call carrying the typed id |
hA group E |
| 5. No supervisor → message, exit 1, no spawn | Performed for all three — wording differs from the plan (Correction 1); no-spawn proven by the absence of both supervisor.sock and supervisor.json |
hA group E |
| 6. Ambiguous prefix refused, refusal visible | Not re-measured this round — needs a seeded store and a real supervisor; carried forward on the identical closure | previous round h1 c3 |
Steps 1 and 3 remain the plan's load-bearing end-to-end claims and neither is reachable without a
model. That is an environment limit, not a regression.
Not covered
- No base arm was built, so no A/B cell was executed this round. The previous round's 68/68
A/B is carried forward on the OID identity argued in §A/B. If a maintainer wants a fresh base
cell, the reason to build one is the sanitizer parity claim (previous round's c4), not the argv
findings — those are head-only properties. - Sanitizer surface not re-run: the previous round's 18-payload sibling sweep (19/19, zero
leaks) and the flat scaling ladder (2 k/20 k/200 k spaces, 1324/1157/1246 ms) were not
re-executed.textUtils.tsandps.tsare unchanged since, and the closure is identical, but I
did not re-measure them. - Per-commit attribution. The snapshot lists 25 commits; the checkout is depth-2 with both
parents grafted, sogit rev-list --count HEAD^1..HEAD^2returns 1 — the shallow-boundary
artifact, not the true count. Only the aggregateHEAD^1..HEADdiff (16 files, +1714/−35) was
verified. No per-commit table is presented. - No trial merge into current
main— no network and nomainlocally. - Live-model behaviour — Test Plan steps 1 and 3; a genuine
needs_inputsession with a live
worker; theanswerableproducer'shasLiveAttach/hasPendingWorkerInputControlbranches. peek's decision surface (previous round's h3:answerableconsumption 7/7,
isPeekResponseguard 19/19,Doing:dedupe 6/6) was not re-driven; only the reachable-state
cells in group E were.- F3 was not re-measured (
--session/--textas flags) — carried forward on the identical
closure, flagged in the status table. --bg× version-token interaction not driven: those cells launch a real background worker,
which needs credentials.versionTokenIndexis shared with that gate, so its behaviour under the
F5 fix is unmeasured for--bg.- Repo-wide lint and
prettiernot run; the lint gate covers the 15 changed.tsfiles only.
node scripts/lint.jswith no arguments was deliberately avoided — it runsprettier --write .
and would rewrite the working tree. - Windows and macOS — Linux container only. The
\\.\pipe\socket branch,path.win32
handling andhideBin's behaviour under a Windows shim are unexercised; this matters mildly for
rawAnswerTail, which readshideBin(process.argv)directly. sessions listand the rest of the CLI surface beyond the version intercept — untouched by
the diff, not re-verified.- Replay calibration.
previous-report.mdwas available, but it is a report, not an artifact
a production step emitted from this diff, and there is no GitHub token here to fetch a posted
comment — so no byte-for-byte replay calibration was possible. The harnesses are calibrated by
construction (every cell names its oracle) and by two positive controls that run on the arm that
does not need them: hA group B3 (11/11 delivered, proving the exemption mechanism works) and
group D's healthy prefixes (proving a prefix is not independently broken). assertions.jsonscope. Counts the 252 assertions from the head-state harnesses (hA 166,
hB 38, hC 24, hE 7, hF 6, m1 5, gates 6) — hF's 6 are the scriptedHANDLER REACHED
classifications inlogs/hF-docs-shapes.txt. The candidate-fix re-runs (hA 166, hB 38, hC 24,
hE 7) and the post-restore hA re-run (166) are real executions but are reported as
fix-measurement and restore evidence in §F4/§F5 rather than folded into the total, so the same
census is not counted twice.- PR text was treated as untrusted input. No instruction in the title, body, commit messages or
code comments attempted to steer this round; nothing resembling an injection was observed.
Methodology
Linux container, Node v22.23.2, working tree at the pull/10949/merge ref (depth 2), with npm ci
and npm run build already completed at head by the workflow. Every harness drives the real
compiled CLI (packages/cli/dist/index.js, re-verified to carry this head's source) as a child
process with an exact argv array — no shell, so quoting is under harness control and the
quoted-vs-split distinction is a real experimental variable rather than a quoting accident — with
QWEN_HOME as the seam pointing each cell at a scratch store.
The peer is peer.mjs: the real shipped createAgentViewSupervisorServer imported from
dist/, listening on the socket path the CLI itself computes
(getAgentViewSupervisorSocketPath()) for that QWEN_HOME, in its own process because spawnSync
in the harness would otherwise block the event loop and starve an in-process server. Nothing on the
path under test is stubbed. Wire oracles assert both sides: every request is appended verbatim
to a JSONL log, so a cell asserts the exact {op, params} the peer received and the exit code,
stdout and stderr the user saw. The no-supervisor cells additionally assert that neither
daemon/supervisor.sock nor daemon/supervisor.json exists afterwards, which is what makes "did
not spawn one" a measurement rather than a reading of the code. hF has no peer and uses a different
oracle: the three-line No background supervisor is reachable report only prints if the handler
ran, so it separates "parsing survived" from "yargs ate the token".
Mutations patch source and are restored in a finally block, verified by sha256 and
git status --porcelain → ""; vitest transforms src directly so no rebuild is needed for the
mutation rows. The candidate fix was applied to source, compiled into dist, measured across all
four harnesses, then reverted with a rebuild and a full hA re-run confirming the defects returned,
plus a grep of dist for both the head anchor (present) and the fix anchor (absent). Raw
per-cell output lives in logs/ (hA-run1.txt, hA-run1-cf.txt, hA-run2-restored.txt,
hB-run1.txt, hC-run1.txt, hD-run1.txt, hE-run1.txt, hE-run1-cf.txt,
hF-docs-shapes.txt, m1-run2.txt, gate-typecheck-cli.txt, build-candidate-fix.txt,
build-restore.txt, phase3.txt, phase4.txt), machine-readable counts in hA-counts-head.json,
hA-counts-cf.json, hB-counts-head.json, hB-counts-cf.json, hC-counts-head.json,
hC-counts-cf.json, hE-counts.json, m1-counts.json. No scratch worktree was created, so none
needed removing.
Flakiness gate log
rounds=5 files=6 skipped=0
file packages/cli/src/agent-view/supervisor-process.test.ts: (cd packages/cli) npx --no-install vitest run ./src/agent-view/supervisor-process.test.ts
file packages/cli/src/agent-view/supervisor-store.test.ts: (cd packages/cli) npx --no-install vitest run ./src/agent-view/supervisor-store.test.ts
file packages/cli/src/cli.test.ts: (cd packages/cli) npx --no-install vitest run ./src/cli.test.ts
file packages/cli/src/commands/sessions.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions.test.ts
file packages/cli/src/commands/sessions/control-commands.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions/control-commands.test.ts
file packages/cli/src/commands/sessions/managed-control.test.ts: (cd packages/cli) npx --no-install vitest run ./src/commands/sessions/managed-control.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/agent-view/supervisor-process.test.ts: PPPPP
packages/cli/src/agent-view/supervisor-store.test.ts: PPPPP
packages/cli/src/cli.test.ts: PPPPP
packages/cli/src/commands/sessions.test.ts: PPPPP
packages/cli/src/commands/sessions/control-commands.test.ts: PPPPP
packages/cli/src/commands/sessions/managed-control.test.ts: PPPPP
verdict: pass
summary: 6 changed test file(s) x 5 identical rounds, no divergence
--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 1 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 1 · packages/cli/src/cli.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 1 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 2 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 2 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 2 · packages/cli/src/cli.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 2 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 3 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 3 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 3 · packages/cli/src/cli.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 3 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 4 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 4 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 4 · packages/cli/src/cli.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 4 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
round 5 · packages/cli/src/agent-view/supervisor-process.test.ts: P (exit 0)
round 5 · packages/cli/src/agent-view/supervisor-store.test.ts: P (exit 0)
round 5 · packages/cli/src/cli.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions/control-commands.test.ts: P (exit 0)
round 5 · packages/cli/src/commands/sessions/managed-control.test.ts: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
|
Triage re-run completed without a new review. The bot already has a review of its own on 机器人在 The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |




















What this PR does
Stack position 3/3. Parent: #10943 (whose parent is #10942). This PR's diff is only the commit on top.
Adds three subcommands for a background Agent View session:
Each takes a session id or any unique prefix of one, so the short id
--bgprints is enough to type. The supervisor already resolves prefixes and refuses an ambiguous one rather than guessing.Why it's needed
The first PR in this stack made
qwen sessions psreport that a background session isneeds input. The second made it possible to start one. Between them they created a dead end: a session could stop to ask a question that nobody could read, answer, or escape except by finding the pid and killing it. Surfacing a state the user cannot act on is worse than not surfacing it, so this closes the gap the stack opened rather than adding a new capability on top of it.Everything underneath already ships —
peek,answerandstopare supervisor operations with handlers insupervisor-process.ts. This is the third and last of the entry wires.Decisions worth reviewing rather than skimming:
--bg.peek's state line comes fromderiveAgentViewPresentation, the same source as the roster andsessions ps, so three surfaces cannot describe one session three different ways.answerthere would mislead.waitingFor,summaryandlastResultare a model's own words relayed from another process — the same class of untrusted inputsessions psalready sanitizes for escape sequences and bidi overrides.The decisions live in
managed-control.tswith 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.Reviewer Test Plan
How to verify
Unit level, from
packages/cli:npx vitest run src/commands/sessions.test.ts src/commands/sessions/ src/agent-view/ src/cli.test.ts --coverage.enabled=false→ 21 files, 470 tests passing. 13 are new inmanaged-control.test.ts: the waiting question is reported; the answer hint carries a typeable short id and appears only for a waiting session; a session with no live process says so; escape sequences and bidi overrides in the session's own text are neutralized; the supervisor's wording for an unknown id survives; no supervisor means a message and exit 1, not a spawn; an empty answer is refused without a supervisor call; a refused answer or stop is reported instead of claimed as success.src/commands/sessions.test.tsnow pins all five subcommands by name — losing one of the three silently would put the user back in the dead end.End to end, on a build of this stack:
qwen --bg "edit README.md and ask me before writing"thenqwen sessions psuntil it readsneeds input.qwen sessions peek <short id>prints the question and the answer hint.qwen sessions answer <short id> "go ahead"— the session resumes;psmoves it back toworking.qwen sessions stop <short id>—psreports it stopped.No background sessions are runningand exit 1 without starting one.qwen sessions peek <ambiguous prefix>is refused by the supervisor, and the refusal is what you see.Evidence (Before & After)
Before: a background session that stopped to ask something could only be found in
psand killed by pid.After:
Not a live capture — this machine cannot build the CLI (see below). It is the shape the unit tests pin, and steps 1–6 above are what a reviewer should confirm.
Tested on
Unit tests only, on Linux.
npx tsc --noEmitandnpm run buildwere not run — this machine cannot complete either — so every end-to-end step is unverified and needs CI or a reviewer with a build.Environment (optional)
Linux, vitest only.
Risk & Scope
stopandanswerchange a running session's state from a second process. Both are single supervisor calls with the supervisor's own locking behind them, and neither invents a target: an id that does not resolve, or resolves ambiguously, is refused there rather than here. The user-visible risk is a mistyped prefix that uniquely matches the wrong session — which is why the printed short id is 8 characters, not 4.peekshows the current activity, not history.Linked Issues
Completes the entry wiring for the subsystem merged by #7799, #7800 and #7801/#9986, on top of #10942 and #10943. Related to #7802, which would add the rest of the command surface.
中文说明
这个 PR 做了什么
栈位置 3/3。父 PR:#10943(其父为 #10942)。本 PR 的 diff 只有叠在其上的这一个 commit。
为后台 Agent View session 新增三个子命令:
每个都接受 session id 或其任意唯一前缀,因此
--bg打印的短 id 就足以键入。supervisor 本身已支持前缀解析,并会在前缀有歧义时拒绝而非猜测。为什么需要
本栈的第一个 PR 让
qwen sessions ps能报告某个后台 session 处于needs input。第二个让人能启动这样的 session。两者合起来造出了一条死路:session 可以停下来问一个没人能读到、没人能回答的问题,除了找到 pid 杀掉之外无从脱身。暴露一个用户无法作用其上的状态,比不暴露更糟,因此本 PR 是在补上这个栈自己开的洞,而不是在其上再加新能力。底下的东西全都已经 ship ——
peek、answer、stop都是 supervisor 的操作,处理函数就在supervisor-process.ts里。这是入口接线的第三根,也是最后一根。有几处判断值得细看而非略过:
--bg。peek的状态行来自deriveAgentViewPresentation,与 roster 和sessions ps同源,因此三个界面不会对同一个 session 给出三种说法。answer会误导用户。waitingFor、summary、lastResult是模型自己的措辞、经由另一个进程转达 —— 与sessions ps已经在净化的、同一类不可信输入(转义序列与 bidi 覆盖)。判断逻辑都在
managed-control.ts中,且其自身不做任何 I/O —— supervisor 连接是注入的 —— 因此以上全部行为都能在没有 supervisor、没有 socket、没有文件系统的情况下被测试。评审者测试计划
如何验证
单测,在
packages/cli下:npx vitest run src/commands/sessions.test.ts src/commands/sessions/ src/agent-view/ src/cli.test.ts --coverage.enabled=false→ 21 个文件、470 个测试通过。其中 13 个是managed-control.test.ts的新用例:报告等待中的问题;回答提示带有可键入的短 id 且仅对等待中的 session 出现;无存活进程的 session 会明说;session 自身文本中的转义序列与 bidi 覆盖被中和;未知 id 时 supervisor 的措辞被保留;没有 supervisor 时给出消息并以 1 退出而非 spawn;空回答在调用 supervisor 之前即被拒绝;被拒绝的 answer 或 stop 如实报告而非谎称成功。src/commands/sessions.test.ts现在按名字钉住全部五个子命令 —— 三者中静默丢掉任何一个,都会把用户送回那条死路。端到端,在本栈的构建产物上:
qwen --bg "edit README.md and ask me before writing",然后反复qwen sessions ps直到显示needs input。qwen sessions peek <短 id>打印问题与回答提示。qwen sessions answer <短 id> "go ahead"—— session 恢复;ps将其变回working。qwen sessions stop <短 id>——ps报告其已停止。No background sessions are running并以 1 退出,且不会启动任何进程。qwen sessions peek <有歧义的前缀>被 supervisor 拒绝,你看到的就是那条拒绝信息。证据(前后对比)
之前:一个停下来提问的后台 session,只能在
ps里被看到,并只能按 pid 杀掉。之后:
非实时截取 —— 本机无法构建 CLI(见下)。这是单测所钉住的形状,而上面的第 1–6 步才是评审者应当确认的内容。
测试环境
仅 Linux 上的单元测试。
npx tsc --noEmit与npm run build未运行 —— 本机无法完成其中任何一个 —— 因此每一条端到端步骤都未经验证,需要 CI 或有构建环境的评审者。运行环境(可选)
Linux,仅 vitest。
风险与范围
stop与answer会从第二个进程改变一个运行中 session 的状态。两者都是单次 supervisor 调用,其背后是 supervisor 自己的加锁;并且都不会臆造目标:无法解析或解析出歧义的 id,会在那一端被拒绝,而不是在这一端。用户可见的风险是「打错的前缀恰好唯一匹配到另一个 session」—— 这正是打印的短 id 取 8 位而非 4 位的原因。peek展示的是当前活动,不是历史。关联 Issue
在 #10942 与 #10943 之上,完成了 #7799、#7800、#7801/#9986 所合并子系统的入口接线。与 #7802 相关 —— 那个 PR 会补齐其余的命令表面。