Skip to content

fix(cli): close OpenTUI submit-path gaps, restore its E2E leg - #10831

Merged
chiga0 merged 11 commits into
mainfrom
fix/opentui-interactive-e2e-gaps
Sep 3, 2026
Merged

fix(cli): close OpenTUI submit-path gaps, restore its E2E leg#10831
chiga0 merged 11 commits into
mainfrom
fix/opentui-interactive-e2e-gaps

Conversation

@chiga0

@chiga0 chiga0 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

What this PR does

Brings the OpenTUI renderer's submit path in line with the ink renderer in four places: a composer submission now carries the raw text the user typed alongside the content that goes to the model, an @-mention typed by the user reaches the model as the file content it stands for, the approval-mode status text is actually drawn instead of only computed, and a slash command typed while the model is still responding waits for that turn to end instead of running concurrently with the open stream. The wait exempts the quit family exactly as ink's does, so an exit typed mid-turn still exits — and cancels the turn it is leaving instead of racing it.

The held-command queue behaves like ink's message queue in the corners too: a defer verdict that lands after the turn already ended still drains, a command that opens a dialog pauses the replay until the dialog closes, a command that starts a turn parks the commands behind it, and an exit discards both mid-turn queues so nothing replays into the cleanup chain.

It also puts the OpenTUI interactive E2E leg back into e2e.yml — restoring the job exactly as #10829 removed it — and bumps the workflow size baseline to match, which the size ratchet requires in the same PR.

Why it's needed

The OpenTUI interactive E2E leg went red on main, and #10829 took the leg out of CI rather than excluding files, registering its restoration as gated on the parity gaps the leg had surfaced (#8662 U-17…U-20). The gaps are real renderer defects rather than anything the tests assert wrongly, so closing them is what un-gates the leg:

  • Hooks that fire on a submitted prompt received no raw composer text, so anything keyed to that field never ran — including the external-context provider's automatic recall, which bails out without it. @-mentions typed by the user also reached the model as literal text instead of the file content they stand for, and the expanded payload had nowhere to be distinguished from what the user actually typed.
  • A session started in an auto-accept mode showed no indicator at all, so there was nothing on screen to confirm the mode took effect. The status string was already being computed for the prompt chrome; it was simply never rendered. This is not only cosmetic for the test suite: the protocol-tag retry case polls the terminal for exactly that mode text as its readiness signal, so it timed out at startup and never reached its assertions.
  • /compress typed while a response was still streaming did nothing whatsoever — no compression, no telemetry, no message. The command ran against a chat the open stream was still writing. ink holds such a command and runs it once the turn ends; only commands that explicitly opt into running mid-stream are dispatched immediately, and ink deliberately exempts its quit family from that wait so an exit can stop a turn.

Restoring the leg is the point of the exercise: without it the renderer's submit path has no automated OpenTUI signal at all, and the leg is what caught these. Its own runtime contract was never wrong — the pinned Bun version and the strict renderer selection are exactly what make a silent ink fallback fail the leg instead of passing as a false green — so it comes back byte-for-byte rather than rewritten.

One correction to the record: the four red files trace to four gaps, not the three #10829 listed — the context-compression failure needed its own mid-turn gate. And the same file carries a fifth case, a /compress submission with trailing instructions, that was red on the first two main runs and green on the third; it submits mid-turn too, so it is the same mechanism. Both are verified here rather than assumed, because the restored leg runs the whole directory.

Reviewer Test Plan

How to verify

Build and bundle, then drive the CLI under the OpenTUI renderer.

  1. Approval-mode indicator. Start a session whose approval mode is YOLO, auto-accept-edits, or auto. Expect the mode named next to the prompt (YOLO mode / Accepting edits / Auto mode, localized). Before this change nothing was drawn. Plan and default modes still show no status text, matching ink's prompt.
  2. Submitted-prompt provenance and @ expansion. With a UserPromptSubmit hook that dumps its stdin to a file, submit @somefile.txt inspect this. Expect the hook's prompt field to contain the file's content, submitted_prompt to contain exactly the text that was typed (no file content), and the transcript row to show the typed text rather than the expanded payload. Then make the model call a tool and confirm the continuation's hook input has no submitted_prompt at all. Three more rules on the same path: a mention that cannot be read (point it at a path that does not exist) settles as a failed tool card in the transcript and the submission is dropped rather than sent unexpanded — before this change the composer cleared and nothing happened, with no trace of why; a slash command's generated payload that happens to contain @… is not expanded (ink's processQuery returns it before its own mention check); and a mention typed while a turn is still responding is queued like any other mid-turn text — if the queue drains into the running turn as steering it reaches the model as typed (Migrate TUI rendering layer from ink to OpenTUI (tracking) #8662 U-21, a registered divergence), and if it survives to become the next turn it expands like an idle submit, provenance included.
  3. Mid-turn slash command. Ask something that keeps the model streaming for a while, then type /compress and press Enter before the response finishes. Expect a notice that the command was queued, then the compression to happen once the response ends (a chat_compression telemetry event lands). Before this change nothing happened at all.
  4. Commands allowed mid-stream are unaffected. Type /help (or /about, /theme, /tools, /tasks, /docs, /hooks, /lsp) while the model is still responding: it runs immediately, as before.
  5. Mid-turn quit exits. While the model is still streaming, type /quit (or /exit). Expect the CLI to exit at once with the graceful exit path — the in-flight request is cancelled before the exit drains, so cleanup never races a stream that is still writing, and neither the held-command queue nor the steering queue replays after the exit. Before the mid-turn gate existed this worked; the first cut of the gate queued it, which review caught.
  6. No regression to mid-turn prompts. A plain prompt typed mid-turn still steers the running turn, and a ?btw side-question still goes straight to the model instead of being queued.
  7. The leg runs on this branch. The E2E workflow triggers on push to main, the nightly schedule and manual dispatch, so a pull request never exercises it. It was dispatched against this branch directly; the results are in the evidence below.

Evidence (Before & After)

After — the leg itself, in the environment that was failing. Dispatched four times against this branch as it moved (ubuntu-latest + Bun 1.3.14 + live model). The completed runs:

Dispatch Head Result
33638984513 47f594ae13 job success
33642038701 8bc4fcabc2 job success — Test Files 8 passed | 1 skipped (9), Tests 14 passed | 2 skipped (16), 87.3s
33651967463 7b6525878a job success
33702961416 000fdbf164 (current head) job success

The same job on the previous three main runs reported 4 failed | 4 passed | 1 skipped, and the four files above were the failures. Strict renderer selection is what makes this a real OpenTUI signal: every spawned CLI on this leg runs under Bun with the renderer pinned and QWEN_TUI_RENDERER_STRICT=1, so a boot that fell back to ink fails the leg.

After — locally, on the merged tree (macOS, Bun, OpenTUI, live model for the compression cases), all five cases in one process:

✓ Interactive Mode > should trigger chat compression with /compress command        172445ms
✓ Interactive Mode > should forward /compress instructions through to the side-query 59682ms
✓ external context auto recall > retrieves from the submitted prompt…               5656ms
✓ submitted prompt provenance > keeps expanded file content out of submitted_prompt… 6654ms
✓ Interactive protocol tag retry guard > retries protocol leaks…                     8697ms
Test Files  4 passed (4)      Tests  5 passed | 1 skipped (6)

The one skip in that file is a Windows-only exclusion; the two compression cases run on Linux, macOS and CI alike.

Before — the mechanism, reproduced deterministically. A throwaway harness whose fake server keeps the first SSE stream open for 25s after emitting its text: under ink, the mid-stream /compress is queued and reports Chat history compressed from ~2433 to ~59 tokens. once the stream ends; under OpenTUI the same harness produced no telemetry and no output at all. After this change the OpenTUI run passes the same way ink does, and the queued notice is visible in the terminal capture. The harness was not committed.

Supporting checks. The renderer's own unit directory (packages/cli/src/ui/opentui) passes on the final head — 67 files, 1054 tests — covering the mid-turn gate (deferred until idle, immediate for a command that opted in, quit exempt by resolved command including its /exit altName, verdicts landing after the idle edge, dialogs pausing the replay, exits discarding both queues), provenance riding the first send of a turn but not its tool-result continuation, the @-mention rules at submit (expanded where the prompt enters the stream for user-typed text, unexpanded for generated payloads, a failed read settling as a failed card instead of silence), and the approval-mode text the composer now draws. Every one of those submit-path pins was checked by reverting the corresponding fix and confirming the test goes red, across all three review rounds. npm run typecheck, npm run build, npm run lint and npm run bundle clean on the merged tree; the restored workflow parses, npm run test:scripts and the workflow-size gate pass with the bumped baseline, and Prettier reports no drift in the touched files.

The CI unit lane, for the record. Test (ubuntu-latest, Node 22.x) has failed on this PR twice for non-code reasons: once by hitting the shared-host 120-minute ceiling with tests still making progress (run 33651850433), and once at the workflow-size ratchet after main lowered the e2e.yml baseline mid-flight (run 33697494983) — this PR restores the job block that removal shrank, so the baseline is bumped here with the reason in the commit message. On this macOS sandbox the full packages/cli suite has 3 red files outside this PR's diff (AuthDialog, two review command files), all timeout-shaped; nothing in the diff can reach them — the only inbound import of the touched directory from outside it is a dynamic import in llm.tsx whose test mocks the touched entry — and the ubuntu lane is the arbiter for the suite as a whole. The Dependency CVE audit red check fails on main's own runs (e.g. 33675732009 at 8fde141a24); this PR touches no manifest.

Tested on

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux

Environment

npm run build && npm run bundle, then the interactive leg under Bun with the OpenTUI renderer pinned and the sandbox disabled. The compression and provenance cases use a live model; the others use the fake OpenAI server.

Risk & Scope

  • Main risk or tradeoff: a slash command typed mid-turn now waits for the turn instead of running at once. That is ink's behaviour, but it is a visible change for anyone who got used to the OpenTUI renderer dispatching commands concurrently with a stream, and it lengthens the window a waiting command occupies — the compression case is ~66s on CI against a five-minute budget, because the command now runs after the model finishes rather than against it. Commands that opt into mid-stream execution are unaffected, and btw side-questions are exempt exactly as in ink. A deferred command reports itself with a one-line notice rather than ink's queue badge; queued commands run in submission order at the idle transition, pause behind a turn or dialog one of them opened, and an exit discards them rather than replaying them into cleanup.
  • Second risk: the restored leg is post-merge and scheduled only — it is not in the merge queue, so it cannot block a merge, and it can fail on main the way it did before. That is the signal fix(ci): Take the OpenTUI interactive e2e leg out of CI #10829 removed and this PR asks for back.
  • Known gaps this PR deliberately leaves open (each registered on Migrate TUI rendering layer from ink to OpenTUI (tracking) #8662 rather than silently dropped): the OpenTUI transcript still does not render the output rows a slash command adds to command history, so a deferred command's result message is not visible even though the command ran (predates this PR, affects every slash command). Text drained as in-flight steering still reaches the model raw — ink expands that hop too (resolveSteeredMessages), and reproducing its read-timeout-and-restore half-way was the worse option (U-21). The bare quit tokens exit, quit, :q, :q!, :wq, :wq! are still submitted to the model as prompts; only the slash forms are exempt from the gate (pre-existing, U-22). OpenTUI exit paths do not call LlmClient.requestShutdown() the way ink's quit action does (U-23). No deterministic E2E case exists for a mid-turn /quit; it is pinned by units driving the real built-in (U-24). With a vision bridge configured, an unsupported @-mentioned image now reaches the model unbridged where ink converts it with applyVisionBridgeIfNeeded — verified during review; porting the prompt-side bridge is a renderer feature addition, deferred rather than half-built (U-25). Also unchanged: the compress command's "already compressing" guard cannot fire because the command context snapshots the pending item. One improvement to note: @-expansion now rides the turn's abort signal — it happens inside the turn, so an Esc during a mention read cancels the submission instead of being ignored, which was not true before this PR.
  • Breaking changes / migration notes: none.

Linked Issues

Part of the ink → OpenTUI renderer migration (#8662) — closes U-17, U-18, U-19 and U-20 there, which is the gate #10829 put on restoring this leg, and registers U-21…U-25 for what it deliberately leaves out. Redesigns nothing from that PR; it reverts its workflow removal once the gate is satisfied.

中文说明

这个 PR 做了什么

把 OpenTUI 渲染器的提交路径与 ink 渲染器在四处对齐:composer 提交时除了送给模型的内容,还会带上用户真正键入的原文;用户键入的 @ 引用以它所代表的文件内容送给模型,而不再是字面文本;审批模式的状态文字真正被绘制出来,而不只是算出来;在模型仍在响应时键入的斜杠命令会等这一轮结束再执行,而不是和仍在进行的流并发运行。等待机制像 ink 一样豁免 quit 家族:回合进行中键入的退出命令仍然立即退出——并且先取消正在离开的那一轮,而不是和它抢跑。

被扣住的命令队列在边角行为上也对齐 ink 的消息队列:门禁的判定结果落在空闲边沿之后仍会继续派发;打开了对话框的命令会把后续重放暂停到对话框关闭;自己开启新一轮的命令会让身后的命令排队等待;退出会把两条流中队列一并丢弃,不让任何东西重放进清理链。

同时把 OpenTUI 交互式 E2E 腿放回 e2e.yml——按 #10829 删除时的原样恢复那个 job——并按体积棘轮的要求在同一 PR 内更新 workflow 体积基线。

为什么需要

OpenTUI 交互式 E2E 腿在 main 上变红,#10829 选择把整条腿移出 CI 而不是排除文件,并把恢复条件登记为该腿暴露出的渲染器缺口被关闭(#8662 U-17…U-20)。这些缺口是真实的渲染器缺陷,不是测试断言写错,因此修掉它们正是解锁恢复的路径:

  • 提交提示词时触发的 hook 拿不到 composer 原文,因此所有依赖该字段的逻辑都没有运行——包括 external-context 提供的自动召回(缺字段就直接退出)。用户键入的 @ 引用也以字面文本送给模型,而不是它代表的文件内容;展开后的内容与实际键入的文本无处区分。
  • 以自动接受模式启动的会话完全没有指示器,屏幕上无法确认模式已生效。状态文本本来就已经为 prompt chrome 算好了,只是从未被渲染。对测试而言这也不只是外观问题:协议标签重试用例就以这条模式文本作为就绪信号去轮询终端,因此它在启动阶段就超时,根本没跑到自己的断言。
  • 在响应仍在流式输出时键入 /compress 完全没有任何反应——没有压缩、没有遥测、没有提示。命令是对着一个仍被流写入的会话执行的。ink 会把这类命令留到本轮结束后再执行;只有显式声明可以在流中运行的命令才会立即派发,而且 ink 刻意让 quit 家族绕过这个等待,好让退出能叫停一轮。

把这条腿恢复回来才是这件事的意义所在:没有它,渲染器的提交路径在 OpenTUI 侧完全没有任何自动化 CI 信号,而正是这条腿发现了这些缺陷。它自身的运行时约定从来没错——固定的 Bun 版本加上严格的渲染器选定,恰恰是让"静默回退到 ink"必须判红而不是假绿的机制——所以它是一字不差地恢复,而不是重写。

对记录的一点更正:四个变红的文件对应四个缺口,而不是 #10829 写的三个——上下文压缩这个失败需要它自己的流中门禁。而且同一个文件还有第五个用例:带尾随指令的 /compress 提交,它在头两次 main 运行变红、第三次变绿;它同样是流中提交,因此是同一个机制。这两点都是实测确认的,不是推断,因为恢复后的腿跑的是整个目录。

评审测试计划

如何验证

先构建并打包,然后在 OpenTUI 渲染器下驱动 CLI。

  1. 审批模式指示器。 以 YOLO、自动接受编辑或自动模式启动会话,期望提示符旁显示模式名(YOLO mode / Accepting edits / Auto mode,会本地化)。改动前什么都不画。Plan 与 default 模式仍不显示状态文本,与 ink 的提示符一致。
  2. 提交来源与 @ 展开。 配置一个把 stdin 落盘的 UserPromptSubmit hook,提交 @somefile.txt inspect this。期望 hook 的 prompt 字段包含文件内容,submitted_prompt 恰好是键入的文本(不含文件内容),transcript 里显示键入文本而非展开后的内容。随后让模型调用一次工具,确认续跑那次的 hook 输入完全没有 submitted_prompt。同一条路径上还有三条规则:指向读不到路径的引用会以一张失败的工具卡片在 transcript 里收场,提交被丢弃而不是把未展开的文本发给模型——改动前 composer 会清空、什么都不发生、不留任何解释;斜杠命令生成的内容里即便出现 @… 也不展开(ink 的 processQuery 在自己的引用检查之前就把它返回了);回合进行中键入的引用和其它流中文本一样先进队列——若队列在工具边界被排空为 steering,则按原文送达(Migrate TUI rendering layer from ink to OpenTUI (tracking) #8662 U-21,已登记的分歧点),若存活到成为下一轮,则像空闲提交一样展开且带来源。
  3. 流中的斜杠命令。 问一个会让模型持续输出一段时间的问题,在响应结束前键入 /compress 并回车。期望先看到"已排队"的提示,然后在响应结束后真正执行压缩(落下 chat_compression 遥测事件)。改动前完全没有任何反应。
  4. 允许流中执行的命令不受影响。 模型仍在响应时键入 /help(或 /about/theme/tools/tasks/docs/hooks/lsp),仍会立即执行。
  5. 流中的退出命令会退出。 模型仍在流式输出时键入 /quit(或 /exit)。期望 CLI 立即走正常退出路径——在途请求先被取消,退出清理不会与仍在写入的流抢跑,被扣住的命令队列与 steering 队列都不会在退出之后重放。流中门禁出现之前本来就是这个行为;门禁的第一版把它排了队,是评审抓住的。
  6. 流中的普通提示词无回归。 回合进行中键入的普通提示词仍会作为 steering 送给当前回合,?btw 侧问仍直接送给模型而不被排队。
  7. 这条腿在本分支上确实跑过。 E2E workflow 只在 push 到 main、夜间定时和手动触发时运行,所以 pull request 永远不会跑到它。这里直接对本分支做了多次手动触发,结果见下方证据。

证据(改动前后)

改动后——在本该失败的环境里跑这条腿本身。 随分支推进共手动触发四次(ubuntu-latest + Bun 1.3.14 + 真实模型)。已完成的运行:

触发 Head 结果
33638984513 47f594ae13 job 成功
33642038701 8bc4fcabc2 job 成功 —— Test Files 8 passed | 1 skipped (9)Tests 14 passed | 2 skipped (16),87.3s
33651967463 7b6525878a job 成功
33702961416 000fdbf164(当前 head) job 成功

同样这个 job 在之前三次 main 运行里报的是 4 failed | 4 passed | 1 skipped,上面四个文件就是当时的失败项。严格的渲染器选定保证这是真实的 OpenTUI 信号:这条腿上每个被拉起的 CLI 都在 Bun 下运行、渲染器被钉住且带 QWEN_TUI_RENDERER_STRICT=1,因此启动时若回退到 ink 会让这条腿判红。

改动后——本地,在合并后的树上(macOS、Bun、OpenTUI,压缩类用例用真实模型),五个用例在同一进程:

✓ Interactive Mode > should trigger chat compression with /compress command        172445ms
✓ Interactive Mode > should forward /compress instructions through to the side-query 59682ms
✓ external context auto recall > retrieves from the submitted prompt…               5656ms
✓ submitted prompt provenance > keeps expanded file content out of submitted_prompt… 6654ms
✓ Interactive protocol tag retry guard > retries protocol leaks…                     8697ms
Test Files  4 passed (4)      Tests  5 passed | 1 skipped (6)

该文件里唯一的跳过是 Windows 排除;两个压缩用例在 Linux、macOS 和 CI 上都会执行。

改动前——机制层面的确定性复现。 一次性脚本:假服务器在吐出文本后把第一条 SSE 流保持打开 25 秒。ink 下,流中提交的 /compress 被排队,并在流结束后输出 Chat history compressed from ~2433 to ~59 tokens.;OpenTUI 下同一脚本既没有遥测也没有任何输出。改动后 OpenTUI 的运行与 ink 一样通过,终端捕获里能看到"已排队"的提示。该脚本未提交。

辅助检查。 渲染器自身的单测目录(packages/cli/src/ui/opentui)在最终 head 上全部通过——67 个文件、1054 条——覆盖流中门禁(延后到空闲、声明可流中执行的命令立即运行、按解析出的命令名豁免 quit(含 /exit 别名)、判定结果晚于空闲边沿、对话框暂停重放、退出丢弃两条队列)、"来源字段只随一轮的首次请求、不随工具结果续跑"、提交时的 @ 引用规则(用户键入的文本在进入流处展开、生成内容不展开、读取失败以失败卡片收场而不是沉默)、以及 composer 现在绘制的审批模式文本。这些提交路径锚点在三轮评审中都逐一通过"回退对应修复后用例应变红"验证。合并后的树上 npm run typechecknpm run buildnpm run lintnpm run bundle 均干净;恢复后的 workflow 可正常解析,npm run test:scripts 与更新基线后的体积门禁通过,触碰过的文件在 Prettier 下没有格式漂移。

CI 单测通道的记录。 Test (ubuntu-latest, Node 22.x) 在本 PR 上两次变红均非代码原因:一次是共享宿主机上跑满 120 分钟上限(run 33651850433,测试仍在推进);一次是 main 在中途调低了 e2e.yml 的体积基线导致棘轮触发(run 33697494983)——本 PR 恢复的正是当初被删除而使文件变小的 job 块,因此在同一 PR 内更新基线并在提交说明里写明原因。在本机 macOS 沙箱上,packages/cli 完整套件有 3 个红文件且都在本 PR 触碰范围之外(AuthDialog、两个 review 命令文件),全部是超时形态;本 diff 够不到它们——触碰目录在目录外唯一的入向引用是 llm.tsx 里的一处动态 import,而它的测试直接 mock 了被触碰的入口——整套件以 ubuntu 通道为准。Dependency CVE audit 这条红在 main 自己的运行上也红(如 8fde141a24 上的 33675732009);本 PR 不碰任何 manifest。

测试环境

OS Status
🍏 macOS
🪟 Windows ⚠️
🐧 Linux

环境

npm run build && npm run bundle,然后在 Bun 下钉住 OpenTUI 渲染器、关闭沙箱运行交互式测试腿。压缩与来源类用例使用真实模型,其余使用假的 OpenAI 服务器。

风险与范围

  • 主要风险/权衡:流中键入的斜杠命令现在会等待本轮结束,而不是立刻执行。这与 ink 一致,但对习惯了 OpenTUI 渲染器与流并发派发命令的人来说是可见的行为变化,并且拉长了等待命令所占用的时间窗——压缩用例在 CI 上约 66 秒,而超时预算是 5 分钟,因为命令现在是在模型说完之后才执行,而不是与它并发。声明可在流中执行的命令不受影响,btw 侧问也和 ink 一样豁免。被延后的命令用一行提示告知,而不是 ink 的队列角标;队列按提交顺序在空闲切换时执行,在其中某条开启的回合或对话框后面暂停,退出时被丢弃而不是重放进清理链。
  • 第二个风险:恢复后的腿只在合并后与定时触发——它不在 merge queue 里,因此不会阻塞合并,也可能像之前那样在 main 上失败。这正是 fix(ci): Take the OpenTUI interactive e2e leg out of CI #10829 拿掉、而本 PR 请求换回来的信号。
  • 本 PR 刻意留下的已知缺口(每一条都登记在 Migrate TUI rendering layer from ink to OpenTUI (tracking) #8662 上,而不是悄悄丢弃):OpenTUI 的 transcript 仍然不渲染斜杠命令写入命令历史的输出行,因此被延后的命令虽然执行了,其结果消息仍不可见(本 PR 之前就存在,影响所有斜杠命令)。被排空为 in-flight steering 的文本仍以原文送达模型——ink 在那个 hop 也会展开(resolveSteeredMessages),但在红腿压力下只复刻它一半的超时加重试是更差的选择(U-21)。裸退出词 exitquit:q:q!:wq:wq! 仍作为提示词发给模型,门禁只豁免斜杠形式(先前就存在,U-22)。OpenTUI 的退出路径不像 ink 的 quit action 那样调用 LlmClient.requestShutdown()(U-23)。流中 /quit 没有确定性的 E2E 用例,由驱动真实内建命令的单测锚定(U-24)。配置了 vision bridge 时,不受支持的 @ 引用图片现在会未经桥接地送达模型,而 ink 会用 applyVisionBridgeIfNeeded 转换——评审中已核实;移植 prompt 侧桥接是一项渲染器功能新增,刻意延后而不是半途搭建(U-25)。同样未改动:compress 命令的"正在压缩"守卫无法触发,因为命令上下文对 pending item 取的是快照。一点改进值得说明:@ 展开现在搭乘回合的 abort 信号——展开发生在回合内部,因此提及读取期间的 Esc 会取消这次提交而不是被无视,这在改动之前是不成立的。
  • 破坏性变更/迁移说明:无。

关联 Issue

属于 ink → OpenTUI 渲染器迁移(#8662)的一部分——关闭其中的 U-17、U-18、U-19、U-20(正是 #10829 为恢复本条腿设定的门禁),并登记 U-21…U-25 作为刻意留下的部分。它不重写那个 PR 的任何设计,只是在门禁满足后撤销它对 workflow 的删除。

…d mid-turn gate

Three gaps left the OpenTUI interactive E2E leg red while the ink leg stayed
green on the same run:

- a composer submission never carried its raw text, so UserPromptSubmit hooks
  saw no submitted_prompt (and the external-context auto-recall keyed off it
  never fired), and `@path` mentions reached the model unexpanded;
- the approval-mode status text was computed but never rendered, so a YOLO
  session showed nothing to wait on at startup;
- a slash command submitted mid-turn was dispatched at once instead of waiting
  for idle, so `/compress` raced the still-open stream and silently did nothing.

The mid-turn gate is the ink rule: only a command that opted into
canRunDuringStreaming runs while a turn responds, everything else the
dispatcher would take is held and drained on the idle transition.
@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

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

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Thanks for the PR!

Template looks good ✓

Problem: observed, not theoretical. The OpenTUI interactive E2E leg went red on main — I checked the cited run 33621485985: a push run on main whose E2E Interactive - OpenTUI renderer (bun) job failed, and the description maps each of the four failing tests to a concrete submit-path gap with reproduction notes.

Direction: aligned. This is repair work on the active ink → OpenTUI migration (#8662, open, roadmap/terminal-ux) — closing gaps the migration itself left behind, and the CHANGELOG already carries the OpenTUI migration infra batch. No direction concerns.

Size: no core paths touched — all nine files live in packages/cli/src/ui/opentui/. Roughly 176 production lines and 110 test lines (additions + deletions). No thresholds in play.

Approach: scope feels right — three fixes, each mapped to an observed gap, and each reuses an existing ink primitive (handleAtCommand, isBtwCommand, the per-command canRunDuringStreaming opt-in flag, existing i18n keys) instead of forking a parallel implementation. One question, not a blocker: the approval-mode indicator doesn't map to any of the four failing tests. It reads like a genuine parity gap found along the way — fine to keep here given it is ~20 lines of the same parity work, but worth saying so explicitly.

Risk: no high-risk path matches; no elevated risk signals.

Moving on to code review. 🔍

中文说明

感谢贡献!

模板完整 ✓

问题:已观测到的问题,不是理论性的。OpenTUI 交互式 E2E 在 main 上变红——核对过所引用的 run 33621485985:这是一次 main 上的 push 运行,其中 E2E Interactive - OpenTUI renderer (bun) job 失败,且描述把四个失败用例逐一对应到具体的提交路径缺口,并附了复现说明。

方向:对齐。这是 ink → OpenTUI 迁移(#8662,开放中,roadmap/terminal-ux)的修复工作——补齐迁移自身留下的缺口,CHANGELOG 中也已有 OpenTUI 迁移基础设施的条目。无方向性顾虑。

规模:未触及核心路径——九个文件全部位于 packages/cli/src/ui/opentui/。约 176 行生产代码、110 行测试(增删合计)。不涉及任何阈值。

方案:范围合理——三处修复各自对应一个已观测到的缺口,且都复用了现有的 ink 原语(handleAtCommandisBtwCommand、逐命令的 canRunDuringStreaming 选择加入标志、现有 i18n key),而不是另起一套平行实现。一个问题,不是阻塞项:审批模式指示器并不对应四个失败用例中的任何一个。看起来是顺手发现的真实 parity 缺口——鉴于只有约 20 行且同属 parity 工作,放在本 PR 里可以,但建议明确说明。

风险:未命中高风险路径;无升级风险信号。

进入代码审查 🔍

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Code review

The approach matches what I'd have proposed independently: thread the raw composer text through the existing send-options seam (rather than reconstructing provenance downstream), render the status string that was already computed, and gate mid-turn slash submissions on the existing per-command opt-in flag. Good reuse throughout — ink's handleAtCommand for @-expansion, isBtwCommand/isAtCommand from the shared utils, existing i18n keys, no new parallel machinery.

Details I verified against the base code:

  • OpenTuiSlashGateway.ready only ever resolves (settled by both attach and failInit), so the new gate awaiting it in onSubmit cannot hang or throw; a failed init falls through to dispatch, which reports it.
  • Unknown commands typed mid-turn (/nope) defer and report unknown at the idle flush — mirroring ink's queue; /path/to/file and ?btw correctly bypass the gate.
  • Provenance rides an explicit UserQuery first send and is omitted on ToolResult continuations — matching core, which honours submitted_prompt on UserQuery only (client.ts), and matching ink, which always submits an explicit UserQuery.
  • The deferred-flush effect clears the ref before dispatching (no double-dispatch on re-render) and re-queues the remaining commands when a submit_prompt outcome starts a new turn, instead of racing that stream.

No correctness blockers found. Three non-blocking notes:

  1. Silent no-op on a bad @-mention at submit. onSubmit calls handleAtCommand without an addItem seam, so a mention that fails to resolve returns silently — ink surfaces an error tool group there. The pre-existing transcript gap limits the blast radius, but this path is user-visible silence.
  2. Mid-turn @-mentions expand, then discard. While a turn streams, the steering queue only keeps the raw text (documented as unchanged behavior), but expansion now runs at enqueue anyway — file reads plus a chat-recording entry whose result is dropped. Cheap to skip the expansion when streaming, if you want to tighten this.
  3. The approval-mode indicator has no test pin. The other two fixes grew unit coverage; the indicator (input-prompt.tsx) didn't, and no CI leg exercises it — see below.

The mid-turn gate is the one genuinely new flow, for orientation:

sequenceDiagram
    participant P1 as User
    participant P2 as OpenTuiApp onSubmit
    participant P3 as Slash gateway
    participant P4 as Deferred queue
    P1->>P2: submit text while a turn streams
    P2->>P3: mustDeferDuringStreaming (awaits ready)
    P3-->>P2: true when the command has no streaming opt-in
    P2->>P4: hold the command, notify queued
    Note over P2,P4: streaming flips false at turn end
    P2->>P3: dispatch held commands in submission order
    P3-->>P2: outcome per command
    alt a submit_prompt outcome starts a new turn
        P2->>P4: remaining commands wait for the next idle
    end
Loading

Test evidence (this PR's own CI — unattended run, no local execution)

One important wrinkle: #10829 merged today and removed the OpenTUI interactive leg from CI entirely, so the four oracle tests for this PR (context-compress-interactive, external-context-auto-recall, submitted-prompt-provenance, protocol-tags-interactive under QWEN_E2E_RENDERER=opentui) currently have no CI leg at all — not on this PR, not post-merge; e2e.yml now pins QWEN_E2E_RENDERER=ink only. The PR description's framing ("fixes the four failures in the e2e-interactive-opentui job") predates that removal — the product fixes stand regardless, but nothing in CI can confirm them.

What the PR's CI does cover, as of this fetch (the finalize job updates the table when CI settles):

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

Check Conclusion
Post Coverage Comment 🚫 cancelled
Test (ubuntu-latest, Node 22.x) 🚫 cancelled
web-shell E2E Smoke (ubuntu-latest, Node 22.x) 🚫 cancelled
Classify PR ✅ success
Dependency CVE audit ✅ success
Desktop Shell (ubuntu-22.04) ✅ success
Desktop Shell (windows-2022) ✅ success
Integration Tests (no-AK, No Sandbox) ✅ success
OpenTUI no-flicker gate ✅ success
Secret scan (TruffleHog) ✅ success
TUI parity snapshots (ink vs opentui) ✅ success

One row per check name (latest run); skipped checks omitted; failures sort first. / 每个检查名一行(取最新一次运行),省略 skipped,失败项排在最前。

The unit suite and the no-AK integration leg are still running. The two checks most relevant to a parity PR — TUI parity snapshots (ink vs opentui) and the OpenTUI no-flicker gate — are already green on this commit.

Sandboxed verification would settle the rest: @qwen-code /tmux — the four oracle tests have no CI leg since #10829, and the approval-mode indicator has no test pin at all, so a real-session run is currently the only way to see the mid-turn /compress queue-and-execute and the mode indicator actually render. The author has write access, so /tmux can be triggered directly; alternatively the interactive suite can be run locally with npm run test:integration:interactive:opentui:sandbox:none per the comment in e2e.yml.

Not verified: the four interactive failures themselves (no CI leg runs them; on the CI path we do not execute PR code). The description's "Evidence (Before & After)" output is the author's claim from local runs, not independently re-run here.

中文说明

代码审查

实现思路与我独立的方案一致:把用户键入的原文经由既有的 send-options 接缝透传(而不是在下游重建来源信息)、渲染本来就计算好的状态文本、并用逐命令既有的选择加入标志来约束流中的斜杠命令。复用做得好——@ 展开用 ink 的 handleAtCommand,共享工具里的 isBtwCommand/isAtCommand,现有 i18n key,没有新造平行机制。

对照基线代码核实过的细节:

  • OpenTuiSlashGateway.ready 只会 resolve(attachfailInit 都会结算它),所以 onSubmit 里新加的门禁 await 它不会挂起或抛错;初始化失败会落到 dispatch,由后者报告。
  • 流中键入的未知命令(/nope)会被延后,并在空闲冲刷时报 unknown——与 ink 的队列一致;/path/to/file?btw 正确地绕过门禁。
  • 来源字段只随显式的 UserQuery 首次请求、ToolResult 续跑不带——与 core 只在 UserQuery 上接受 submitted_prompt 的行为(client.ts)一致,也与 ink 总是显式提交 UserQuery 一致。
  • 延后冲刷的 effect 在派发前先清空 ref(重渲染不会导致重复派发);当某个 submit_prompt 结果开启新一轮时,其余命令被重新入队等待下一次空闲,而不是与新流抢跑。

未发现正确性阻塞项。三条非阻塞备注:

  1. 提交时 @ 引用解析失败会静默无操作。 onSubmit 调用 handleAtCommand 时没有 addItem 接缝,解析失败的引用会静默返回——ink 会在那里显示错误的 tool group。已有的 transcript 缺口限制了影响面,但这条路径对用户是无声的。
  2. 流中的 @ 引用会先展开再丢弃。 回合进行中,steering 队列只保留原文(已声明为不变行为),但提交入队时仍会执行展开——产生文件读取与 chat-recording 记录,而展开结果被丢弃。如想收紧,可在流中跳过展开。
  3. 审批模式指示器没有测试锚点。 另外两处修复都补了单元测试;指示器(input-prompt.tsx)没有,也没有任何 CI 腿覆盖它——见下文。

流中门禁是唯一真正新增的流程,示意图供导航:用户流中提交 → 门禁判定需延后 → 命令入队并提示 → 回合结束(streaming 变 false)后按提交顺序逐个派发;若某个结果是 submit_prompt 并开启新一轮,其余命令等待下一次空闲。

测试证据(本 PR 自身 CI——无人值守运行,不在本地执行)

一个重要变化:#10829 今天已合入,OpenTUI 交互腿已从 CI 完全移除,因此本 PR 的四个锚定用例(OpenTUI 渲染器下的 context-compress-interactiveexternal-context-auto-recallsubmitted-prompt-provenanceprotocol-tags-interactive)目前没有任何 CI 腿覆盖——本 PR 没有,合入后也没有;e2e.yml 现在只固定跑 QWEN_E2E_RENDERER=ink。PR 描述中"修复 e2e-interactive-opentui job 的四个失败"的表述早于该移除——产品修复本身依然成立,但 CI 已无法为其作证。

本 PR CI 实际覆盖的部分,截至本次抓取(finalize 任务会在 CI 落定后就地更新表格,见英文部分的机器可读区域)。单元测试与 no-AK 集成腿仍在运行。对 parity PR 最相关的两项检查——**TUI parity snapshots(ink vs opentui)**与 OpenTUI no-flicker gate——在该提交上已经转绿。

沙箱验证可以补上剩余部分:@qwen-code /tmux——自 #10829 起四个锚定用例没有任何 CI 腿,审批模式指示器也完全没有测试锚点,因此跑一次真实会话是目前唯一能看到流中 /compress 排队并执行、以及模式指示器真正渲染出来的方式。作者有写权限,可直接触发 /tmux;也可以按 e2e.yml 中的说明在本地运行 npm run test:integration:interactive:opentui:sandbox:none

未验证:四个交互失败用例本身(没有 CI 腿运行它们;CI 路径上不执行 PR 代码)。描述中"证据(改动前后)"一节是作者本地运行的声明,此处未独立复跑。

Qwen Code · qwen3.8-max

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

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Confidence: 4/5 — clean, minimal parity fix with good reuse and solid unit coverage; only non-blocking nits, but the behavioral oracle has no CI leg to run it.

Stepping back: the motivation is solid — these are real gaps that made a main-branch E2E leg fail deterministically, not speculative hardening, and the diff is exactly the minimal set: thread provenance, render the already-computed status, gate mid-turn commands like ink does. My independent proposal would have landed on the same three fixes using the same ink primitives, so there's no simpler path being missed. The code reads like it will be easy to maintain in six months — the comments say why (ink parity) at every non-obvious point, and the deferred-queue logic handles its edge cases (double-dispatch, submit_prompt re-queueing) correctly.

Reservations, all non-blocking: the bad-@-mention silent no-op and the wasted mid-turn expansion noted in my review, and the fact that the indicator fix ships without any test pin. The bigger structural point is not this PR's fault: since #10829 removed the OpenTUI interactive leg, nothing in CI — on this PR or after merge — exercises these behaviors under OpenTUI, so the strongest available evidence is the green parity/no-flicker gates, the new unit tests, and the author's local runs. I'd encourage a /tmux pass before or shortly after merge so the visible surface is actually seen once.

Approval is deferred until CI lands green on this commit; the finalize job posts the commit-pinned approval if everything settles green, and withholds it if anything lands red or the head moves.

中文说明

置信度:4/5 —— 干净、最小化的 parity 修复,复用得当、单测覆盖扎实;只有非阻塞的小问题,但行为锚定用例没有 CI 腿可以运行它。

退一步看:动机扎实——这些是让 main 分支 E2E 腿确定性失败的真实缺口,不是理论性加固;diff 也恰好是最小集合:透传来源字段、渲染已算好的状态文本、像 ink 一样约束流中命令。我的独立方案会落在同样的三处修复、复用同样的 ink 原语,没有更简的路径被遗漏。代码六个月后维护起来也不会令人头疼——每个非显而易见处都有说明"为什么"(与 ink 对齐)的注释,延后队列逻辑的边界情况(重复派发、submit_prompt 重新入队)处理正确。

保留意见,均为非阻塞:审查中提到的坏 @ 引用静默无操作、流中展开后被丢弃的浪费,以及指示器修复没有任何测试锚点。更大的结构性问题不在本 PR:自 #10829 移除 OpenTUI 交互腿后,无论在哪个阶段,CI 都不再在 OpenTUI 下执行这些行为,因此目前最强的证据是转绿的 parity / no-flicker 门禁、新增单测与作者的本地运行。建议在合入前后跑一次 /tmux,让可见界面真正被看到一次。

批准将推迟到该提交的 CI 全部转绿后执行:CI 全部转绿时由 finalize 任务提交锚定该提交的批准;若有检查变红或提交移动,则不予批准并说明。

Qwen Code · qwen3.8-max

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

#10829 removed the `e2e-interactive-opentui` job to keep main green, and
registered its removal as gated on the parity gaps the leg surfaced (#8662
U-17…U-20). The commits above this one close them: the approval-mode
indicator is drawn, `submitted_prompt` rides the first send, `@file` goes
through query preparation, and a slash command submitted mid-turn waits for
idle instead of racing the open stream.

Restoring the job verbatim rather than rewriting it: nothing about its
runtime contract was wrong, and the pinned Bun version plus the strict
renderer pin are what make a silent ink fallback fail the leg.

The four failures traced to four gaps, not the three #10829 recorded — the
context-compression case needed its own mid-turn gate. The context-compress
file additionally carries a fifth case (`/compress` with trailing
instructions) that was red on the first two `main` runs and passed the
third: it submits mid-turn too, so it is the same mechanism, and the leg
that runs the whole directory is what judges it.

The design doc named the leg as removed in two places; both now describe a
leg that reported the gaps, left CI, and came back with them closed.
@chiga0 chiga0 changed the title fix(cli): close the OpenTUI submit-path gaps behind the red interactive E2E leg fix(cli): close OpenTUI submit-path gaps, restore its E2E leg Sep 2, 2026
@chiga0

chiga0 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

E2E test report — OpenTUI interactive leg

What ran, and where

The leg this PR restores has no pull_request trigger — e2e.yml runs on push to main, the nightly schedule, and manual dispatch — so a green check on the PR would not have proven anything about it. It was dispatched against the pushed branch head (47f594ae13, which merges main at 867bb94a1f) and ran in the environment that was failing:

Job E2E Interactive - OpenTUI renderer (bun) · run 33638984513 · job 100276967465
Host ubuntu-latest, Bun 1.3.14 (pinned), Node from .nvmrc
Renderer QWEN_E2E_RENDERER=opentui → every spawned CLI runs under Bun with QWEN_TUI_RENDERER=opentui and QWEN_TUI_RENDERER_STRICT=1
Model live (OPENAI_API_KEY / BASE_URL / MODEL from repo secrets)
Scope the whole integration-tests/interactive directory, minus the two files the leg always excludes

Result

Test Files  8 passed | 1 skipped (9)
     Tests  14 passed | 2 skipped (16)
  Duration  78.22s (tests 211.24s)

The five cases that had been red on main, in that run:

case previously now
context-compress-interactive › should trigger chat compression with /compress red on runs 33609632018, 33615039100, 33621485985 ✓ 32132ms
context-compress-interactive › should forward /compress instructions through to the side-query red on 33609632018, 33615039100; green on 33621485985 ✓ 33963ms
external-context-auto-recall › retrieves from the submitted prompt… red on all three ✓ 5766ms
protocol-tags-interactive › retries protocol leaks across SSE disconnect… red on all three ✓ 6124ms
submitted-prompt-provenance › keeps expanded file content out of submitted_prompt… red on all three ✓ 37084ms

The baseline for comparison is the job's own last main execution: Test Files 4 failed | 4 passed | 1 skipped (9), with each failing test retried twice.

Locally, on the merged tree

Same five cases in one vitest process (macOS, Bun, OpenTUI, live model for the compression cases) — run to get per-case timings and to catch anything Linux-only ordering hides:

✓ Interactive Mode > should trigger chat compression with /compress command          172445ms
✓ Interactive Mode > should forward /compress instructions through to the side-query  59682ms
✓ external context auto recall > retrieves from the submitted prompt…                 5656ms
✓ submitted prompt provenance > keeps expanded file content out of…                   6654ms
✓ Interactive protocol tag retry guard > retries protocol leaks…                       8697ms
Test Files  4 passed (4)      Tests  5 passed | 1 skipped (6)      exit 0

The terminal capture shows the new gate firing mid-turn — Queued /compress — it will run when the current response ends. — followed by the compression once the stream closed. Almost all of the compression case's wall time is now spent waiting for the model to finish its first answer, which is the intended behaviour and also why the same case reads 172s locally and 66s on CI; against the 5-minute test budget that leaves headroom on CI but not an unlimited amount.

Supporting: unit tests for the touched area 6 files / 147 tests green on the merged tree; npm run build, npm run bundle, npm run typecheck exit 0; the restored workflow parses and Prettier reports no drift.

What this evidence does not cover

  • Windows. The leg does not run there and all four files exclude win32, so nothing here speaks for Windows.
  • The rest of the dispatch run. The other jobs in run 33638984513 (Linux shards, docker shards, macOS shards, channel-plugin) were still in progress when this was written; they are not evidence for or against this PR. The changed code is confined to the renderer's own directory and the workflow file, so the ink path is untouched by construction rather than by test.
  • Single-run confidence. This is one execution of a model-dependent suite. The compression cases are the timing-sensitive ones — one of them flipped between main runs before this PR — so it is the next few nightly executions that establish the leg is stable, not this run.
  • The renderer's remaining parity gaps. Restoring the leg buys signal, not parity. OpenTUI still does not render the output rows a slash command adds to command history, so a queued command's result message stays invisible even though the command ran; that is tracked separately and is out of scope here.

…chrome

A submission that arrives while a turn responds becomes steering, which
carries text only, so expanding its @ mention read files the hop would
drop. A declined expansion also stayed silent: the composer cleared and
nothing reached the model. Report the failed-read reason instead, and pin
the approval-mode indicator plus all three submit-path rules in units.
@chiga0

chiga0 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

Review notes — all three addressed on 8bc4fcabc2

The head moved after your review (merge of main at 867bb94a1f, then the leg restoration, then this), so the notes below are answered against the current tree. Re-running /triage on the new head.

1. Silent no-op on a bad @-mention. Fixed, one line: the decline path now reports the reason on the notice slot the shell already has. handleAtCommand has exactly one decline path — the failed file read, the only shouldProceed: false in the module — and it always appends its error display last, so the notice carries that display's text. What I did not do is fake ink's transcript error group: rendering an added history row is the same unwired seam that already keeps a deferred command's output invisible (documented under Risk & Scope), so inventing a partial version of it here would have made the gap harder to see. That is now called out explicitly in the PR description rather than left as an implicit difference.

2. Mid-turn @-mentions expand, then discard. Fixed as you framed the cheap version: expansion is skipped when streaming, because a submission that arrives mid-turn becomes steering and steering carries text only — the read result was always going to be dropped. Observable behaviour for steering is unchanged (raw text reaches the model); what changed is that nothing reads the files to throw their content away.

3. The approval-mode indicator had no test pin. Now it has one, in the composer's own component-test file: YOLO mode / Accepting edits / Auto mode each render for their mode, and nothing renders for plan / default, which is ink's rule. Asserted through the same t() call the component makes, so a Chinese locale can't flip the pin. Honest limit: only the three positive cases are mutation-checked (removing the render turns them red); the two negative cases pass with or without the fix and only guard against over-drawing.

On the premise attached to that note — no CI leg exercises it — that was true of the tree you reviewed and is no longer true of this one. #10829 removed the leg on main after this PR opened; it is back in the second commit here (47f594ae13), byte-for-byte the job #10829 deleted.

Which also answers the Stage 1 question about whether the indicator maps to a failing case. It maps to protocol-tags-interactive, and the mapping is not inferential: in job 100219267217 — the red run on main — that test fails with

AssertionError: CLI did not start up in interactive mode: expected false to be true
    103|  expect(isReady, 'CLI did not start up in interactive mode').to…

and the poll behind isReady is /YOLO (模式|mode)/i over the PTY output (protocol-tags-interactive.test.ts:99). The readiness gate is the mode text; the test never reached its retry assertions. So the indicator is one of the four failures, not a parity gap picked up along the way — the PR description now says so, and #8662 records it as U-17.

Verification on 8bc4fcabc2

  • Renderer directory packages/cli/src/ui/opentui: 67 files / 1044 tests green; repository-wide typecheck, build and bundle exit 0; eslint clean on the touched files.
  • The two @-sensitive cases in the leg, re-run locally against the new bundle (macOS, Bun, QWEN_TUI_RENDERER_STRICT=1, live model): submitted-prompt-provenance ✓ 5304ms, external-context-auto-recall ✓ 4898ms.
  • Each new submit-path pin was mutation-checked by reverting the corresponding fix and confirming the test goes red.
  • Limits of that: the declined-mention notice and the mid-turn skip are pinned in units only — no leg case submits an @-mention mid-turn, and no leg case mentions an unreadable path. A real-session pass over both is requested below.
  • The full leg is green on 47f594ae13 in CI (see the E2E report comment). It is dispatched again on this head, still queued behind that run in its concurrency group — I left the older run alone rather than cancelling it, so I am not claiming a head-exact CI number until it lands.

@chiga0

chiga0 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /tmux Real-session pass over the changed submit path — branch fix/opentui-interactive-e2e-gaps, head 8bc4fcabc2, run under the OpenTUI renderer with strict selection (QWEN_TUI_RENDERER=opentui, QWEN_TUI_RENDERER_STRICT=1). Five surface checks, in this order:

  1. Start a session in YOLO. The prompt row must name the mode (YOLO mode, or Accepting edits / Auto mode for those modes; localized text is fine). Then switch to plan and to default: no mode text should be drawn there. This is what the leg's readiness poll greps for, so a blank row means the renderer fell back to ink or the fix is not live.
  2. Idle @-mention with a UserPromptSubmit hook dumping its stdin: submit @somefile.txt inspect this. The hook's prompt must hold the file's content and submitted_prompt exactly the typed text; the transcript row shows the typed text.
  3. Idle mention of a path that cannot be read: submit @no-such-file.txt hello. The submission must not reach the model, and a one-line notice must say why. Before this change the composer cleared and nothing at all was reported.
  4. Mid-turn slash command: ask something that streams for a while, then submit /compress before the response ends. Expect a "queued" notice, then the compression running once the stream closes. /help typed mid-turn must still run immediately.
  5. Mid-turn @-mention: while a response is still streaming, submit @somefile.txt and this. The model request must carry the text exactly as typed, with no file content — the expansion is skipped on purpose, because that submission becomes steering.

@chiga0

chiga0 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

@qwen-code /triage

@qwen-code-ci-bot

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

Copy link
Copy Markdown
Collaborator

Sandboxed verification: ✅ passed — merge-ready (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: 37 passed · 0 failed · 37 total

Flakiness gate: ⚠️ timeout — only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed

中文 — 判定:✅ 通过 · 可合入(agent 判定)

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

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

抖动门:⚠️ timeout — only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed

Verification report

PR 10831 Deep Verification — fix(cli): close OpenTUI submit-path gaps, restore its E2E leg

Verdict: merge-ready — 37/37 scripted assertions passed (fail: 0; every base-arm red is an encoded, intended control). Verified head: 8bc4fcabc2011cf1bdc6a48c2a510f87a7768e24 (git rev-parse HEAD^2), merge commit 1a70af4d, base tip 4f21287. Three non-blocking findings below (one pre-existing race the restored leg can trip on slow runners, one coverage gap, one observation).

中文摘要
  • 结论merge-ready。37/37 脚本化断言通过,fail=0(base 侧的所有红都是预设对照)。
  • A/B 结论:用 base tip(4f21287)单独重建了对照 bundle(与 head 仅差本 PR 的 cli 源码,marker grep 验证纯净),在真实 PTY + 真实 SSE 假服务器下端到端对比:head 4/4 绿,base 4/4 红,且每个红都恰好落在 PR 声称修复的行为上——审批模式指示器 base 完全不绘制(protocol-tags 就绪轮询超时);submitted_prompt 在 base 的 hook 输入中缺失;external-context 自动召回在 base 收到 0 个 provider 请求;流中输入 /compress 在 base 上无声无息(无排队提示、无模型请求、无 chat_compression 遥测),而 head 显示排队提示、在流结束后才发出压缩请求并落下遥测事件。
  • 发现(均非本 PR 引入/非阻塞):(1) opentui 存在一个预先存在的启动竞态——composer 就绪早于 config.initialize() 完成(本机窗口约 1.2s),窗口内的首次提交报 "Chat not initialized";A/A 证明 head 与 base 同样复现。本 PR 恢复的腿在负载较高的 runner 上可能因此偶发变红(详见 Findings)。(2) live-turn.ts 的 displayText 路由(transcript 显示键入原文而非展开内容)与延迟队列 drain 的再排队分支没有任何测试锚定(变异存活)。(3) 通过补全菜单键入 /compress 时观察到压缩 side-query 发出两次(约 150–350ms 间隔),疑为菜单路径双派发,建议人工确认。
  • 未覆盖:需要真实模型的压缩用例(本环境无凭据);恢复的 job 与 fix(ci): Take the OpenTUI interactive e2e leg out of CI #10829 删除版本的逐字节比对(shallow 历史不可达);逐提交归因(同上);yamllint(容器内无安装权限);Windows;交互式目录中其余需要真实凭据的文件。

Central claim and A/B

Central claim: a slash command submitted while a model turn is still streaming is held back and only runs once the turn ends (ink parity); opted-in commands are unaffected. Secondary: (a) submitted_prompt provenance + @-expansion submit path, (b) the approval-mode indicator is actually drawn.

The control is a second bundle built in a git worktree at HEAD^1 = 4f21287 (tmp/base-tree during the run), differing from the head bundle only in packages/cli/src (PR touches no package.json/package-lock.json/packages/core/packages/web-templates — verified by empty git diff --stat over those paths; nested packages/*/node_modules were symlinked from head and are lockfile-identical third-party trees). Purity proven by marker grep: the head bundle contains the new Queued … current response ends notice string, the base bundle does not (logs/base-esbuild-2.log, marker commands in logs/). All runs below use Bun 1.3.14 (the exact pin the restored workflow installs), QWEN_TUI_RENDERER=opentui, QWEN_TUI_RENDERER_STRICT=1.

Cell Oracle HEAD BASE
Boot indicator (boot-indicator-probe.mjs, --yolo) rendered screen after Type your message YOLO mode drawn nothing drawn (01-indicator-boot-ab.png)
protocol-tags readiness poll /YOLO (模式|mode)/ then retry assertions pass in 25.6 s red: readiness timeout, never reached assertions
submitted-prompt provenance (hook stdin capture) submitted_prompt = typed text; prompt ⊇ file canary; continuation carries none pass in 27.0 s red: hook input lacks submitted_prompt
external-context auto-recall provider saw {query: submittedPrompt}; model request ⊇ both canaries pass in 24.1 s red: provider requests []
mid-turn gate (midturn-gate-interactive.test.ts, hold-open SSE) Queued /compress notice mid-stream; zero model requests while queued; compression request only after wire-level stream end; chat_compression telemetry pass in 35.5 s ([PR10831-AB] queued_notice=true requests_after_queue=1 compression_after_stream_end=true compression_event=true) red: queued_notice=false, requests stayed 1 throughout, no compression request, no telemetry (02-leg-head.png, 03-leg-base.png)

All four base failures carry the exact intended messages (e.g. expected [] to deeply equal [ { authorization: "Bearer bound-context-token", … } ], expected { …(6) } to match object with submitted_prompt missing) — behavioral mismatches, not environment errors. The base leg's gate cell also asserts two raw passes (requests_after_queue == 1, turn_ended == true on the wire), proving the base stream completed normally and the reds are the gate's absence, not a harness failure.

One environment accommodation, A/A-proven rather than assumed: this container exhibits a pre-existing opentui startup race (Finding 1) that fails any first submission landing ~1 s after readiness, on both arms identically. The leg cells above therefore settle 8 s after readiness before typing; the same 8 s settle was applied to both arms, and the unsettled A/A probes (05-startup-race-timeline.png) document the race itself.

Corrections

None — first verification round; no earlier bot claims to correct. (One record note: the snapshot baseRefOid 867bb94a… had drifted from the merge-ref base tip 4f21287; per the environment contract the merge-ref base was used and the drift is noted here.)

Findings (non-blocking; none introduced by this PR)

1. Pre-existing opentui startup race: first submit inside the init window → Chat not initialized (Suggestion)

The OpenTUI composer renders Type your message before config.initialize() completes. livePromptEvents calls config.initialize() and catches every throw — the catch assumes "already initialized", but Config.initialize() sets initialized = true synchronously at the start, so a concurrent second call throws Config was already initialized while this.chat still does not exist; the submit then proceeds to client.sendMessageStreamgetChat() throws Chat not initialized, which the user sees as ✖︎ Chat not initialized (and [MICROCOMPACTION] microcompactHistory failed in the debug log). Measured timeline in this container: init started 16:12:36.841, submit landed ~16:12:37.3, init completed 16:12:38.045.

Reproduce (either arm — A/A identical):

# from repo root; bun 1.3.14 on PATH or BUN_BIN set
node tmp/pr10831-verify-20260902-154325/scratch-race/probe.mjs dist/cli.js <scratch-home> <scratch-cwd>
# types a prompt immediately after "Type your message" -> ✖︎ Chat not initialized

Why it matters for this PR: the leg this PR restores polls Type your message and then types at once — exactly the racy shape. On ubuntu-latest the init window closes before the keystrokes land (hence the PR's green CI runs); on loaded runners it may not, and the restored post-merge/nightly leg could go red on main with this error instead of a real gap. The race predates the PR (base bundle A/A reproduces it; none of the PR's hunks touch the readiness/init ordering).

Direction for a follow-up fix (sketch, not applied/measured here)

Make livePromptEvents await the in-flight initialization instead of swallowing the throw: when initialize() rejects with "already initialized", await the config's initializationPromise (Config already stores it) before sendMessageStream. Alternatively gate composer readiness on initializationSettled. This is core/cli-seam work belonging to a separate PR; it is stated as a direction, not measured in this round.

2. Coverage gap: live-turn.ts display routing and the drain re-queue branch are pinned by no test (Suggestion)

Mutation row R5 (04-mutation-matrix.png): reverting live-turn.ts to base leaves live-turn.test.ts + opentui-app-shell.test.tsx + live-session.test.ts all green. Unpinned behaviors:

  • displayText = options?.submittedPrompt ?? contentText — the transcript/user item shows the raw composer text rather than the @-expanded payload, and steering pushes the raw text. No unit test asserts it, and the E2E provenance test asserts only the hook/model side. The fixture that would go red: a live-turn test driving submit('EXPANDED', undefined, { submittedPrompt: 'raw typed' }) and asserting the applied user item text is 'raw typed'.
  • the drain loop's re-queue branch in opentui-app-shell.tsx (a submit_prompt outcome mid-drain pushes the remaining queued commands to the next idle transition) and multi-command ordering — no test queues more than one command.

The combined end-to-end gate is covered by the E2E A/B above; this is completeness reporting, not a merge condition.

3. Observation: two compression side-queries per typed /compress (Nit, needs human confirmation)

In both head gate-harness runs the compression request appeared twice (requests i=1, i=2, ~150–350 ms apart), while the unit-level queue drains exactly once per queued entry. The likely cause is the PTY typing path going through the slash-completion menu (menu-selection + composer submit both dispatching). A real user typing /compress + Enter sees the same menu, so this deserves a human look: if the double dispatch is real, /compress mid-turn would run two compressions. It does not affect any A/B cell here (both arms typed identically; base issued zero requests).

Not covered

  • Live-model legs (context-compress-interactive.test.ts, both /compress cases): need real model credentials, absent in this container. The fake-server gate harness covers the same mid-turn mechanism; the live compression path remains attested only by the PR's own CI run (run 33638984513).
  • Byte-for-byte restoration claim ("restoring the job exactly as it was removed"): the fix(ci): Take the OpenTUI interactive e2e leg out of CI #10829 deletion is unreachable at depth 2. Structural verification done instead: the job parses, its run command mirrors the repo's test:integration:interactive:opentui:sandbox:none script plus the same cron/channel-plugin excludes the ink leg uses, action SHAs match sibling usages, Bun pinned 1.3.14 with the stated rationale.
  • Per-commit attribution: only 8bc4fca is locally reachable; the snapshot lists 4 commits. The aggregate HEAD^1..HEAD diff was verified.
  • yamllint: not installable in-container (pip3: Permission denied); compensated by js-yaml structural parse + actionlint (proven live with a planted violation).
  • Windows; the rest of the interactive/ directory (other files need live credentials or were out of the PR's delta); tui-parity and other CI lanes; the opentui transcript gap the PR itself declares out of scope.
  • The settled leg variants differ from the originals only by an 8 s post-readiness delay (documented inline); the original no-delay shape is exactly what Finding 1 addresses.

Methodology

Environment: the CI verify container (node:22-bookworm, non-root), merge-ref checkout at depth 2, npm ci/npm run build pre-done, head bundle pre-built at dist/cli.js (marker-checked against the PR source). Bun 1.3.14 installed into a writable prefix (network available; no GitHub token used anywhere). Base control: git worktree add tmp/base-tree HEAD^1, esbuild re-bundled in-tree with nested node_modules symlinked from head (readlink -f tmp/base-tree/packages/cli/node_modules/@opentui/core → head's nested dir; legitimate because package-lock.json is untouched by the PR and packages/core/web-templates are byte-identical between arms); bundle purity then proven by marker grep both ways. Harnesses are mock-free at the unit-under-test boundary: real bundles driven through real PTYs (@lydell/node-pty + @xterm/headless, the repo's TestRig/InteractiveSession plumbing) against real loopback SSE servers — including a purpose-built hold-open server (midturn-gate-interactive.test.ts) that keeps stream 0 open 15 s and timestamps every request, so "compression ran after the stream ended" is a wire fact, not a screen reading. E2E oracles note that OpenTUI renders notices as separately positioned spans, so stripped-PTY substring checks allow span gaps. Unit mutation rows reverted single files (git checkout HEAD^1 -- <file>) or applied one-line mutations, ran the targeted vitest files, restored, and confirmed git status clean after every row; the positive control (R0) lands in the same file as its mutant. Ink A/A (provenance under QWEN_E2E_RENDERER=ink) passed on head, isolating the opentui-specific startup race as environmental for this round. Gates: full opentui unit suite (1044/1044), repo npm run typecheck (rc=0), eslint + prettier on all changed files, actionlint on workflows (live-gate proven by a planted violation). Raw logs and harness sources: tmp/pr10831-verify-20260902-154325/logs/, evidence/, plus the .test.ts/.mjs harnesses in the same directory.

Flakiness gate log

rounds=5 files=4 skipped=0
file packages/cli/src/ui/opentui/commands-dispatch.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/commands-dispatch.test.ts
file packages/cli/src/ui/opentui/input-prompt.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/input-prompt.test.tsx
file packages/cli/src/ui/opentui/live-session.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/live-session.test.ts
file packages/cli/src/ui/opentui/opentui-app-shell.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/opentui-app-shell.test.tsx


per-file results (P=pass F=fail I=infra-exit, one letter per run):
  packages/cli/src/ui/opentui/commands-dispatch.test.ts: PPP
  packages/cli/src/ui/opentui/input-prompt.test.tsx: PPP
  packages/cli/src/ui/opentui/live-session.test.ts: PPP
  packages/cli/src/ui/opentui/opentui-app-shell.test.tsx: PP

verdict: timeout
summary: only 2 of 5 rounds fit the 15-minute budget; the completed rounds agreed

--- per-invocation detail (full copy in the artifact) ---
round 1 · packages/cli/src/ui/opentui/commands-dispatch.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/input-prompt.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/opentui/live-session.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/opentui-app-shell.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/opentui/commands-dispatch.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/opentui/input-prompt.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/opentui/live-session.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/opentui/opentui-app-shell.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/opentui/commands-dispatch.test.ts: P (exit 0)
round 3 · packages/cli/src/ui/opentui/input-prompt.test.tsx: P (exit 0)
round 3 · packages/cli/src/ui/opentui/live-session.test.ts: P (exit 0)

Evidence images

01-indicator-boot-ab

02-leg-head

03-leg-base

04-mutation-matrix

05-startup-race-timeline

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

Qwen Code · sandboxed verification

@qwen-code-dev-bot qwen-code-dev-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed at head 8bc4fcab.

  • The four parity gaps are closed with correct ink semantics, each verified in code: the mid-turn gate defers only dispatcher-taken slash commands (btw and /-path passthrough keep their submit-immediately behavior), the deferred queue drains in submission order on the streaming→idle transition without re-deferring; submitted_prompt provenance rides the first UserQuery only, with continuations omitting it by construction; @-expansion runs off-stream only (steering stays text-only per its contract), and its decline path — previously a silent vanishing submission — now surfaces the failed-read notice; the approval-mode status text is actually rendered and localized.
  • The restored E2E leg is byte-for-byte the one removed in #10829 (pinned Bun, strict renderer mode), which is exactly what that PR's un-gating condition called for, and the design doc's gap ledger is updated in the same commit — including the honest correction that the fifth failing case shared the mid-turn mechanism rather than being a sixth gap.
  • ~226 lines of new tests across the five changed modules pin the queue ordering, expansion skip mid-turn, provenance plumbing, and indicator rendering. CI has no failures on this head (lanes still running after the push); no prior reviews or open threads. Per the channel convention the call is on the review itself.

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review — fix(cli): close OpenTUI submit-path gaps, restore its E2E leg

No blocking findings. Approval withheld: reviewer is the PR author.


Scope

Reviewed: all 12 changed files — 7 TypeScript source + test files in packages/cli/src/ui/opentui/, e2e.yml, and the design doc.
Not covered: integration-test execution (requires live API keys + bun runtime); approval-mode indicator visual rendering.


Triage: Standard

Touches runtime UI behaviour (submit-path dispatch, streaming gate, provenance threading) with test coverage, but no persisted format or wire protocol. Reversible.


Checked (all clean)

Item Result
mustDeferDuringStreaming semantic inversion Correctly inverts canRunDuringStreaming: btw/path pass-through → false (don't defer); unknown commands → true (defer, safe default); known commands → derived from canRunDuringStreaming. All 6 cases covered by the revised test. ✅
takesAsSlashCommand refactor Private helper is logically equivalent to the old two-condition guard in handle() — same startsWith checks, same path-separator bail-out. ✅
slash-gateway made async mustDeferDuringStreaming now awaits this.ready before answering, so mid-turn gate decisions wait for the command registry to load instead of defaulting to false (let it run) on an unpopulated registry. Previous sync form was a latent false-green. ✅
Deferred command queue Queue cleared synchronously before the async IIFE; concurrent effect fires cannot double-process the same commands. submit_prompt mid-queue correctly re-enqueues the tail and returns, waiting for the next idle edge. ✅
submittedPrompt threading Flows from app-shell → OpenTuiSubmitOptionsLivePromptOptionssendOptions on first UserQuery only; tool-result continuations omit it by construction. Test asserts both first-call and second-call shapes. ✅
sendOptions always explicit on first send Previously undefined when no modelOverride — now always { type: UserQuery }. Core-side this was a no-op (treated as a user query anyway), but making it explicit unblocks provenance attachment. ✅
@-expansion mid-turn skip isAtCommand expansion only runs when !streaming; mid-turn submission forwards raw text as steering (expansion would be read and dropped anyway). ✅
Approval-mode indicator rendering chrome.statusText was computed but never rendered before. Now rendered in the JSX. t() wrapper added so tests assert through the same translation call the component makes, preventing locale-flip silent breaks. ✅
CI leg restored byte-for-byte e2e-interactive-opentui job command is identical to the one removed in #10829. Comment reverted to match. ✅
.size-baseline not required Restored e2e.yml is ~24,400 bytes (baseline 21,437 + allowance 4,096 = 25,533 ceiling — within bounds). ✅

Cross-check

qwen-code-dev-bot approved at 8bc4fcab: confirmed same four gap-closures (mid-turn gate, queue drain, submitted_prompt provenance, @-expansion off-stream only). Consistent with my findings. No missed items.


Reviewed with AI assistance.

yiliang114
yiliang114 previously approved these changes Sep 2, 2026

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the full diff at 8bc4fca and ran the touched suites locally.

What I checked

  • submittedPrompt provenance: rides the first UserQuery send only; ToolResult continuations omit it by construction — matches core's rule that only UserQuery honours submitted_prompt.
  • @-mention parity: idle submissions expand via handleAtCommand with the raw composer text kept as provenance; mid-turn submissions forward as plain text (steering drops expansion); a declined expansion surfaces the read failure instead of silently sending.
  • Approval-mode indicator is now actually rendered (with t() for i18n), asserted for YOLO/AUTO_EDIT/AUTO and absent for PLAN/DEFAULT.
  • Mid-turn slash gate: mustDeferDuringStreaming correctly exempts non-command input (/-paths, btw side-questions), held commands replay in order on the idle edge, and a submit_prompt outcome re-queues the rest rather than racing the new stream.
  • Restored e2e leg: pinned bun 1.3.14, fork-PR skip, 60min timeout, strict renderer pin rationale documented.

Local verification at head

  • commands-dispatch + input-prompt + live-session + opentui-app-shell → 143/143 passed
  • TUI parity snapshots and OpenTUI no-flicker gate already green in CI.

LGTM, approving.

qqqys
qqqys previously requested changes Sep 2, 2026

@qqqys qqqys left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Critical: /quit (and /exit) typed mid-turn is now queued instead of exiting

Wherepackages/cli/src/ui/opentui/opentui-app-shell.tsx:374-381 (the new mid-turn gate at the top of onSubmit), with the answer coming from mustDeferDuringStreaming in packages/cli/src/ui/opentui/commands-dispatch.ts.

Triggerstreaming === true and the user submits /quit or /exit.

quitCommand carries no canRunDuringStreaming opt-in (packages/cli/src/ui/commands/quitCommand.ts; the opt-ins today are about/bug/docs/editor/help/hooks/lsp/settings/tasks), so the new gate defers it exactly like /compress.

Verified at head 8bc4fcabc2

Real OpenTuiSlashDispatcher against the real built-in commands, no stubs:

mustDeferDuringStreaming >>> {"/quit":true,"/exit":true,"  /quit  ":true,"/help":false,"/compress":true,"/clear":true,":q":false,"plain prompt":false}
quitCommand.canRunDuringStreaming >>> undefined | helpCommand >>> true

Shell half of the same production path (streaming: true, dispatcher answering what it really answers for /quit):

mid-turn /quit >>> onQuit calls: 0 | dispatched texts: []
after idle     >>> onQuit calls: 1 | dispatched texts: ["/quit"]

with the notice Queued /quit — it will run when the current response ends. rendered in place of an exit.

Isolating the cause: removing only the new gate hunk (file restored byte-identical, sha256 d8d90daf85b8…) flips the mid-turn result to onQuit calls: 1 | dispatched texts: ["/quit"]. So the hunk is the sole cause and this is a regression against the PR's base, not pre-existing behavior — before it, /quit reached onQuitexitSession(0) immediately regardless of streaming.

152 tests across the six touched opentui suites are green at head (including the ones added in 8bc4fcabc2); nothing pins quit either way.

Impact

The exit command does not exit. The turn keeps running — and keeps spending tokens — after the user asked to leave, and the CLI exits only when it ends; if the turn never completes, the queued /quit never runs at all.

The keyboard cascade is not an equivalent escape hatch: while streamingRef.current is true, Ctrl+C/Ctrl+D call interrupt() and return (packages/cli/src/ui/opentui/start-opentui-ui.tsx:192-210), so reaching the two-press exit window still requires the turn to stop first — and that path leaves via EXIT_CODE_INTERRUPT instead of the graceful exitSession(0).

Why this is not ink parity

ink places an explicit bypass before its queue — packages/cli/src/ui/AppContainer.tsx:2881-2889:

// Quit must bypass reminders and the message queue so it can stop an
// active stream without consuming one-shot session state.
if (['/quit', '/exit', 'exit', 'quit', ':q', ':q!', ':wq', ':wq!'].includes(userPromptText.trim())) {
  void handleSlashCommand('/quit');
  return;
}

The PR's Risk & Scope says the mid-turn wait "is ink's behaviour". That holds for /compress and the rest, but ink deliberately exempts quit for the reason in that comment. (:q escapes the gate here only incidentally — takesAsSlashCommand rejects it, and the dispatcher hands non-slash text back to the model.)

Fix direction

Exempt quit in onSubmit ahead of the gate at line 374, mirroring ink's list. Setting canRunDuringStreaming: true on quitCommand would also work mechanically, but that flag is shared with ink's own mid-stream fast path, so the local guard is the smaller and safer change.


Everything else I checked in this PR held up, for the record: the restored leg cannot pass as a false green (resolveE2eCliCommand hard-fails without bun and e2eRendererEnv pins QWEN_TUI_RENDERER_STRICT=1, integration-tests/renderer-matrix.ts:46-66); the three new t() keys exist in the locale maps; the pinned Bun 1.3.14 matches DEFAULT_BUN_VERSION (scripts/build-standalone-release.js:64); QWEN_SKIP_PREPARE is the pattern the other four e2e legs already use; and the new job id is unique with no dangling references. The three fixes in 8bc4fcabc2 (mid-turn expansion skipped, declined expansion reported, indicator pinned) also look right.

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 2 — correction of my round-1 verdict. My earlier review (5091324625) concluded "no blocking findings" at this same head/base. That was wrong: the cross-check against @qqqys's changes-requested review surfaced a blocker I had missed, and I have now verified it myself at head 8bc4fcab. Everything else in round 1 stands; the verdict does not.

R2-1 — Blocker (confirmed, at head 8bc4fcab): /quit and /exit typed mid-turn are queued until idle instead of exiting. Confirming qqqys's finding with my own verification of every link:

  • quitCommand.ts has zero occurrences of canRunDuringStreaming, so the new gate in opentui-app-shell.tsx:374 defers /quit//exit exactly like /compress (mustDeferDuringStreaming in commands-dispatch.ts returns true for both).
  • This diverges from ink by design, not by accident: AppContainer.tsx:2881-2890 places an explicit bypass before its queue for ['/quit', '/exit', 'exit', 'quit', ':q', ':q!', ':wq', ':wq!'] — "Quit must bypass reminders and the message queue so it can stop an active stream without consuming one-shot session state." The opentui gate has no such exemption. (My round-1 ink comparison started reading at AppContainer.tsx:2925 and missed the bypass immediately above — that is the miss.)
  • No equivalent escape hatch while streaming: start-opentui-ui.tsx:204-207 — Ctrl+C/Ctrl+D call interrupt() and return while streamingRef.current; the two-press exit window only arms once idle, and leaves via EXIT_CODE_INTERRUPT rather than the graceful exitSession(0).
  • Regression against this PR's own base: before the gate existed, opentui dispatched /quit immediately regardless of streaming.
  • Impact: after the user asks to leave, the turn keeps running (and spending tokens) until it ends; a turn that never completes means the queued /quit never runs.

Fix direction agreed with qqqys: exempt quit in onSubmit ahead of the gate, mirroring ink's list; setting canRunDuringStreaming: true on quitCommand would work mechanically but that flag is shared with ink's own mid-stream fast path, so the local guard is the smaller change.

Round-1 findings status: none were filed; the round-1 scope ledger and verification remain valid —

Checked (unchanged from round 1): the workflow restore is byte-for-byte the job #10829 removed (66/66 lines identical by mechanical comparison) · core honours submittedPrompt only on a non-empty UserQuery (client.ts:2789, 3121-3131, 3156-3174), and live-session.ts rides it on the first send only · the one handleAtCommand decline path always appends a string resultDisplay last (atCommandProcessor.ts:938-962) · YAML parses, workflow suites 226/226, size ratchet exit 0 · CI premise re-verified: run 33638984513's opentui leg is SUCCESS (the run's other failures are unrelated legs), and it executed at 47f594ae — one commit before final head; the post-run delta (mid-turn @-expansion skip + decline notice + their tests) is unit-covered and the leg's four files don't touch those paths, but no leg run exists at the exact head sha.

Ran (linux, head 8bc4fcab): vitest packages/cli src/ui/opentui67 files, 1044/1044 passed (after repairing my sandbox's nested-dep resolution: packages/core/node_modules/ajv@8.20.0 and cli wrap-ansi@10.0.0 — the initial broken resolution produced 49 load-failures plus one spurious a11y failure that disappeared once it matched the lock; noted so nobody re-derives a defect from it) · two mutation probes killed (removing the statusText render flips the 3 positive indicator pins; removing the !streaming && guard flips the mid-turn @ pin) · typecheck run here: it reports errors, but none in any file this PR touches — they are this sandbox resolving workspace packages to a co-checked-out clone's stale built types rather than head's sources (first attempt OOM'd; retry's errors all sit in unrelated serve/acp-integration/peerMessaging/bridge files), so it neither confirms nor disputes the PR's "typecheck clean" claim.

Not covered: live-model/Bun execution (needs CI secrets) · Windows (leg is ubuntu-only).

Reviewed with AI assistance.

@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

tmux real-user testing: timeout - workflow run

The tmux test did not complete before the time limit. This is not a pass/fail result for the affected flow; see the workflow run and artifacts for details.

No report.md or tmux-readable-full.log was found in tmux-results, so detailed report sections are omitted.

Qwen Code · tmux real-user testing

…turn quit

The commit above kept mid-turn @ mentions out of expansion on the stated
premise that steering drops the expanded payload. That premise was wrong:
ink expands steered messages too (resolveSteeredMessages), so the
composer-side guard was a parity loss rather than a parity guard. Expansion
moves to where a prompt enters the stream, which is where ink does it for a
fresh turn (processQuery): the idle submit and the queued text that becomes
the next turn are both covered, the transcript keeps what the user typed,
and the read cards ink renders through handleAtCommand's addItem now render
here as settled tool-start/tool-result/tool-end events. A declined expansion
is the failed read, and it reports itself on that card instead of sending the
unexpanded text to the model. The follow-on turn built from the mid-turn
queue carries the raw text as UserPromptSubmit provenance for the same
reason ink does.

Text drained as in-flight steering still rides raw. ink expands that hop
with a 10s read timeout and a queue restore on cancel; reproducing it
half-way under a red CI leg was the worse option, so the divergence is
stated in the live-turn module docstring and registered in #8662 instead.

Review round 2 also found a blocker in the mid-turn gate this branch added:
the quit command carries no canRunDuringStreaming opt-in, so /quit and /exit
typed during a response were queued until it ended. ink runs its quit family
ahead of the message queue precisely so an exit can stop an active stream,
and its quit action cancels the ongoing request before the exit drains. The
gate now exempts the resolved quit command, which covers /exit as an altName,
and the shell interrupts the turn before handing the exit to the entry so the
cleanup drain does not race a stream that is still writing.

Pinned in units at both seams and mutation-checked: the gate answers from
the real built-in quit command, the shell interrupts before it exits, the
stream layer expands a string prompt and reports a failed read, and the
queued follow-on turn arrives raw with its provenance. No ink-path file is
touched.
@chiga0
chiga0 dismissed stale reviews from yiliang114 and qwen-code-dev-bot via 7b65258 September 2, 2026 15:57
@chiga0

chiga0 commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

R2-1 fixed at head 7b6525878a — a mid-turn /quit (or /exit) exits immediately again.

@qqqys your repro was right and my round-1 "no blocking findings" verdict was wrong. Two changes, at the layers ink puts them:

  1. The gate exempts quit. mustDeferDuringStreaming now returns false for the command the parser resolved as quit, ahead of the canRunDuringStreaming check — so /exit is covered through the altName rather than through a token list, and a future opt-in flag on the command cannot re-introduce the queue. ink's bypass sits ahead of its message queue in AppContainer.handleFinalSubmit for exactly the reason quoted there.
  2. The shell stops the turn before handing over the exit. The quit branch now calls onInterrupt() and then onQuit(messages). ink's quit action cancels the ongoing request first; without that, exitSession's drain (runExitCleanup: recording flush, config.shutdown, MCP stop) runs concurrently with a stream that is still writing — the failure mode exit-lifecycle.ts exists to prevent. The Ctrl+C cascade already interrupts before its own exit; /quit now does the same.

Verification — real dispatcher and real shell, not a stub asserting a stub:

  • commands-dispatch.test.ts builds the real OpenTuiSlashDispatcher around the real built-in quitCommand: /quitfalse, " /exit "false, /cleartrue, and handle('/exit'){ kind: 'quit' }, so what the gate lets through is an exit rather than a prompt handed to the model.
  • opentui-app-shell.test.tsx renders the real shell with streaming: true and a quit outcome: dispatched at once, no Queued … notice, onQuit called with the messages, and onInterrupt ordered before it.
  • Both mutation-checked: deleting the exemption fails the dispatcher test with expected true to be false; deleting the interrupt fails the shell ordering assertion. Sources restored afterwards, git diff clean.
  • Renderer directory packages/cli/src/ui/opentui: 67 files / 1047 tests green. typecheck, build, bundle clean. No ink-path file is touched by this branch.

Coverage boundary, stated plainly: there is no live E2E case for a mid-turn /quit. Holding a turn open deterministically needs a stream-hold knob the shared fake OpenAI server does not have, and adding that plus a case in both renderer legs is a larger change than this fix, so it is registered as a follow-up in #8662 rather than silently dropped. The restored OpenTUI leg was green at the previous head (run 33642038701E2E Interactive - OpenTUI renderer (bun): success) and has been re-dispatched at this one (run 33651967463).

Also in this commit, found by re-reading the same path rather than from the review: @-expansion moved out of the composer into the stream layer, which is where ink does it. My earlier code skipped expansion mid-turn on the premise that steering drops the expanded payload; ink expands steered messages too (resolveSteeredMessages), so that guard was a parity loss, not a parity guard. Now an idle submit and queued text that becomes the next turn are both expanded, the read renders as a settled tool card, a failed read reports itself on that card instead of the submission vanishing, and the queued follow-on carries the raw text as submitted_prompt. The in-flight steering hop still rides raw: ink expands it with a 10s read timeout and a queue restore on cancel, and half-implementing that under a red leg was the worse option — the divergence is recorded in the live-turn module docstring and in #8662.

One pre-existing gap this brushes against, unchanged here: ink also quits on the bare tokens exit, quit, :q, :q!, :wq, :wq!. OpenTUI has no equivalent, so those reach the model as prompts. The gate never deferred them (they are not slash commands), so this PR neither fixes nor worsens that; it is registered in #8662 too.

@github-actions

github-actions Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Code Coverage Summary

Package Lines Statements Functions Branches
CLI N/A% N/A% N/A% N/A%
Core N/A% N/A% N/A% N/A%
CLI Package - Full Text Report
CLI full-text-summary.txt not found at: coverage_artifact/cli/coverage/full-text-summary.txt
Core Package - Full Text Report
Core full-text-summary.txt not found at: coverage_artifact/core/coverage/full-text-summary.txt

For detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run.

@qwen-code-ci-bot qwen-code-ci-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not linted (tool limitation, not a blocker): the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted.

Test Plan (not a blocker): interactive/context-compress-interactive.test.tsno such file or directory; interactive/external-context-auto-recall.test.tsno such file or directory; interactive/protocol-tags-interactive.test.tsno such file or directory; interactive/submitted-prompt-provenance.test.tsno such file or directory.

中文说明

未检查(工具限制,非阻断):the executable-script lint — .github/workflows/e2e.yml: actionlint embedded-shell source mapping is not yet supported — not linted。

Test Plan(非阻断):interactive/context-compress-interactive.test.tsno such file or directory; interactive/external-context-auto-recall.test.tsno such file or directory; interactive/protocol-tags-interactive.test.tsno such file or directory; interactive/submitted-prompt-provenance.test.tsno such file or directory

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

Comment thread packages/cli/src/ui/opentui/opentui-app-shell.tsx
Comment thread packages/cli/src/ui/opentui/live-session.ts Outdated
Comment thread packages/cli/src/ui/opentui/opentui-app-shell.tsx Outdated
Comment thread packages/cli/src/ui/opentui/opentui-app-shell.tsx
Comment thread packages/cli/src/ui/opentui/live-turn.ts Outdated
Comment thread packages/cli/src/ui/opentui/live-session.ts Outdated
Comment thread packages/cli/src/ui/opentui/live-turn.ts
Comment thread packages/cli/src/ui/opentui/opentui-app-shell.tsx Outdated
Comment thread packages/cli/src/ui/opentui/opentui-app-shell.tsx Outdated
Comment thread packages/cli/src/ui/opentui/live-session.ts
@wenshao

wenshao commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

@qwen-code /takeover

@qwen-code-dev-bot qwen-code-dev-bot added the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Sep 2, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

🔀 Base updated: red check(s) [Dependency CVE audit] pass on current main — merged current main via update-branch; CI will re-run.

中文说明

🔀 已更新 base:红色检查 [Dependency CVE audit] 在当前 main 上通过 —— 已通过 update-branch 合入当前 main,CI 将重新运行。

@chiga0 chiga0 removed the autofix/takeover Summon the autofix loop to manage this PR (remove to release; needs triage+) label Sep 3, 2026
@qwen-code-dev-bot

Copy link
Copy Markdown
Collaborator

👋 Takeover released: the autofix loop will no longer engage this PR (an in-flight round, if any, completes its bounded work). Re-apply autofix/takeover (or comment @qwen-code /takeover) to re-engage.

中文说明

👋 已释放:autofix 循环不再介入此 PR(在飞的一轮如有,将完成其有界工作)。重新打上 autofix/takeover 标签(或评论 @qwen-code /takeover)即可再次接管。

chiga0 and others added 3 commits September 3, 2026 09:11
…eline

Restoring the OpenTUI interactive e2e job that #10829 removed grows
e2e.yml by 4454 bytes, past the 21437+4096 ratchet. The growth is the
job block itself, not drift: bump the recorded size to 25891 in the
same PR, as the gate instructs.

@chiga0 chiga0 left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Round 3 — the head moved twice since round 2 (6c2dfdc4 reviewed in full; final head 92af4f55 adds only a merge of main whose sole file is an unrelated vscode-ide-companion/NOTICES.txt, so all verification carries over).

Prior-round blocker — fixed and pinned. My R2-1 / @qqqys's /quit finding is resolved: mustDeferDuringStreaming exempts the resolved quit command (so the /exit altName is covered) ahead of the opt-in check (commands-dispatch.ts:390-398); the quit outcome discards both mid-turn queues, interrupts the stream, then exits (opentui-app-shell.tsx:360-375handleQuitexitSession(0)), mirroring ink's "cancel the ongoing request before the exit drains". Pinned by three tests (commands-dispatch.test.ts:416, opentui-app-shell.test.tsx:544/:743); my mutation probe (removing the exemption line) turns the dispatcher pin red.

Cross-check on the ci-bot's findings (frozen before reading them; verdicts at head):

  • Its round-1 series is correctly dispositioned: the nine implemented fixes are real at head (verified in the c27d7d49c5 diff — the tests cite R1-1/R1-5/R1-7/R1-12 by name, and the suite is green below); the R1-11 decline holds — I checked its basis at head: ink's popNextSubmission really is plain-first (useMessageQueue.ts:296), so "one turn late" matches ink rather than diverging from it; R1-13 (prompt-side vision bridge missing under vision-bridge configs) is verified real and deferred to the follow-up queue by agreement — disclosed, not silently dropped.
  • Its round-2 series was filed after the last code commit and nothing since addresses it (the takeover was released; only merges landed). I verified both Criticals independently at head — they hold:
    • R3-1 (their R2-1), Blocker: the drain loop runs on its own pending snapshot and never consults exit state. The composer quit path clears the ref, but an already-running drain keeps its snapshot, and the two exit paths that bypass the shell — Ctrl+C/Ctrl+D two-press (start-opentui-ui.tsx:210) and the render-error bailout (:237) — never clear it at all. exitSession keeps the tree mounted while it drains the cleanup chain (exit-lifecycle.ts:46-62), and the isExitInProgress() latch that would guard this sits unused. Held commands (a /compress model call, or a submit_prompt starting a fresh turn) can dispatch into teardown.
    • R3-2 (their R2-2), Blocker: the end-of-turn replay joins ALL queued mid-turn texts into one turn (live-turn.ts finally-block), draining the queue irreversibly before expansion; the one decline of the expander — a failed or aborted @-read — returns without sending, so every queued message is lost, violating the module's own "user input is never silently dropped" invariant. ink pops one submission per settle precisely so an abort risks only that one.
    • R3-3 (their R2-3), Suggestion: the multimodal gate test passes no options, so the typeof prompt === 'string' guard is the only expansion defense never exercised — the bot's named mutant (deleting typeof) survives the test as written; production always carries provenance on this seam, so the mutant would throw in real use.
  • The bot's round-2 deferred probe (input typed during the drain batch busy-rejected) stays recorded-but-not-requested per its own convergence note.

Verification at head (linux): vitest packages/cli src/ui/opentui67 files, 1054/1054 passed · vitest scripts suites (e2e-workflow/e2e-shard-retry/workflow-size) → 227/227 · size ratchet exit 0 · e2e.yml restore still byte-for-byte the job #10829 removed (66/66 lines, re-checked at final head 92af4f55 — it survived three merges of main) · quit-exemption mutation killed. Typecheck caveat unchanged from round 2 (this sandbox resolves workspace types against a co-clone's stale build; nothing in PR-touched files).

Not covered: live-model/Bun leg at this head (no run cited past 47f594ae+33642038701; the leg's four files don't touch the drain/steering paths, but a fresh dispatch would pin it) · Windows N/A.

Verdict: no approval (I am the author, and two confirmed blockers — R3-1/R3-2 — are open on record and verified at head). The quit regression that blocked round 2 is properly fixed; what remains is the drain's exit-blindness and the joined-queue drop, both on the ci-bot's threads, both needing either a fix or an explicit maintainer disposition before merge.

Reviewed with AI assistance.

yiliang114
yiliang114 previously approved these changes Sep 3, 2026

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed the delta pushed after my earlier approval (8bc4fca6c2dfdc: commits 7b65258, c27d7d4, 000fdbf).

What I checked

  • Quit exemption: matched on the resolved command (name === 'quit'), so the /exit altName is covered; the shell discards both mid-turn queues and interrupts the in-flight stream before exiting, with the ordering asserted (popQueue → interrupt → quit).
  • Idle-safe drain: the deferredRevision push nonce lands a held command whose defer verdict arrives after the idle edge; the drain is gated on streaming || dialog, an open_dialog outcome re-queues the rest behind the dialog, and a submit_prompt outcome still pauses the queue behind the new turn.
  • Expansion layer move: @-mentions now expand in livePromptEvents gated on provenance (submittedPrompt defined + string prompt), so generated slash payloads stay unexpanded, queued mid-turn text expands as the follow-on turn, attachments ride through untouched, and a declined read reports on the card instead of sending. The steering hop stays raw, disclosed as #8662.
  • Test changes adapt rather than weaken: the shell-level expansion tests moved to live-session with equivalent decline/failure coverage; e2e.yml size baseline matches the head file byte-for-byte (25891).

No blockers found in the delta. Re-approving (previous approval auto-dismissed by the push). CI on this head is still running; ubuntu result worth confirming before merge.

…lay one submission per turn

Round-2 review findings:

- R2-1: the drain consulted only streaming/dialog, so the exits that
  bypass the shell's quit branch (Ctrl+C/Ctrl+D double press, render-error
  bailout) could not stop an in-flight snapshot from dispatching held
  commands against a tearing-down session. The drain now reads the shared
  exit latch at the edge and between dispatches.
- R2-2: end-of-turn replay joined every queued mid-turn text into one
  turn, so the expander's one reachable decline (an abort landing inside
  the @-mention read) swallowed the whole queue. Replay is now one
  submission per chained turn, matching ink's pop-one-per-settle drain.
- R2-3: the multimodal pass-through test now carries production-shaped
  options so the gate's string check decides (the provenance short-circuit
  no longer masks it), and its comment cites the real ink symbol.

Each fix is mutation-checked: reverting it turns the added test red.

@yiliang114 yiliang114 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the follow-up commit 07962f5 against the round-2 findings:

  • R2-1 (drain vs exit): the drain effect now consults the shared exitSession latch (isExitInProgress()) both at the edge and between dispatches, closing the exits that bypass the shell's quit branch (Ctrl+C/Ctrl+D double press, render-error bailout). Verified the latch: exitSession is idempotent, sets it before draining cleanup, and start-opentui-ui.tsx routes the interrupt exit through it.
  • R2-2 (queue replay): end-of-turn replay now pops exactly one submission ([text, ...remaining]) and chains it as a single provenance-carrying turn, matching ink's pop-one-per-settle drain — a decline (e.g. abort landing inside an @-expansion read) can no longer swallow the whole queue.
  • R2-3: the multimodal pass-through test now carries production-shaped options so the gate's string check decides.

New tests accompany each fix (live-turn/app-shell/live-session). No new blockers in the delta. Re-approving (previous approval auto-dismissed by the push).

@yiliang114
yiliang114 dismissed qwen-code-ci-bot’s stale review September 3, 2026 03:26

Addressed in 07962f5: R2-1 (drain now gated on the shared exit latch at the edge and between dispatches), R2-2 (replay pops one submission per chained turn), R2-3 (multimodal test carries production-shaped options). Dismissing to unblock the updated head.

@qwen-code-dev-bot qwen-code-dev-bot left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed at head 07962f57 (this pass covers the work since my earlier review of the sharding-era batches and my last-round concerns).

  • Both gaps I flagged last round are fixed in exactly the right shape and pinned by new tests: the held-command drain now consults the shared exit latch at the effect edge and between dispatches (covering the exit paths that bypass this shell's quit branch), and the end-of-turn replay pops ONE queued submission per settle so a decline inside an @-expansion consumes only its own message — matching ink's per-settle drain, with the remainder kept for later boundaries.
  • The earlier round's R2-series threads are all closed — 14/14 overall.
  • CI: the one failure on this head is the web-shell polls an expanded workspace once per 30 s tick Playwright case (a wall-clock-polling spec that lives in packages/web-shell, which this PR's cli/opentui-only diff never touches) — an unrelated contention flake worth a follow-up to that suite, not a merge blocker here; per the channel convention the call is on the review itself.
  • The three standing CHANGES_REQUESTED tickets all predate these fixes; the newest bot review is still running on this head.

@yiliang114
yiliang114 dismissed stale reviews from qqqys and qwen-code-ci-bot September 3, 2026 03:28

Addressed in 7b65258: quit is exempted from the mid-turn defer gate (matched on the resolved command, covering the /exit altName), and the shell interrupts the stream then exits (ordering asserted by test).

@chiga0
chiga0 added this pull request to the merge queue Sep 3, 2026
Merged via the queue into main with commit b556049 Sep 3, 2026
162 of 167 checks passed
@chiga0

chiga0 commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator Author

Round 4 — posted post-merge (the PR merged while this review was running; head/base unchanged, so the verification below applies to exactly what shipped). The one new commit since round 3 (07962f57, 5 files, all packages/cli/src/ui/opentui/) addresses exactly the three findings left open on record. I re-verified each at head with my own probes:

Ledger vs round 3:

  • R3-1 (drain exit-blindness) — fixed. isExitInProgress() is now consulted at the drain edge and before each dispatch in the loop, which is what the round-3 thread prescribed; the source-clearing on the composer quit path stays. My probe: removing both checks turns does not replay held commands once an exit drain is in flight red.
  • R3-2 (joined replay consumed the whole queue on a decline) — fixed. End-of-turn replay now promotes one queued submission per chained turn (ink's pop-one-per-settle), so a decline — e.g. an abort inside an @-expansion read — costs only its own text; the tail survives. My probe: rejoining the queue back into one submission turns consumes only its own submission when a replayed turn declines red.
  • R3-3 (multimodal test didn't exercise the string guard) — fixed. The test now passes production-shaped options (submittedPrompt: 'describe this: '), so typeof prompt === 'string' decides instead of the provenance short-circuit. My probe: deleting the typeof operand turns the test red (the author's stated mutation-check claim, independently confirmed).
  • R2-1 (my round-2 /quit blocker) — still fixed; this commit doesn't touch the gate.

New (round 4, minor, body one-liner per thread budget):

  • R4-1: the in-loop isExitInProgress() check is not test-pinned — removing only that line (edge guard intact) leaves the whole suite green, because the new test flips the latch before the idle edge. The mid-drain case it guards (exit starts while a command is in flight) is exactly the scenario R3-1 was about; a future refactor could drop the line without anything going red. A second test that flips the latch after the first dispatch resolves would pin it.

Ran (linux, head 07962f57): vitest src/ui/opentui67 files, 1056/1056 passed · four mutation probes (3 killed as above; the surviving one is R4-1) · earlier e2e.yml byte-parity vs the job #10829 removed still holds (this commit doesn't touch workflows; suites from round 3 carry).

Not covered: live-model/Bun leg at this head (leg files don't touch drain/replay paths; the queue-promotion rewrite is unit-pinned only) · Windows N/A · typecheck sandbox caveat unchanged (workspace types resolve against a stale co-clone build).

Cross-check: no reviewer filed anything new at this head beyond what's dispositioned above; @yiliang114 and the dev-bot both approved 07962f57, and the round-2 CHANGES_REQUESTED thread has the fix reply on it.

Verdict: no blocking findings on the shipped head — the three open blockers are fixed with pinned tests (and two human/bot approvals were already recorded on 07962f57). R4-1 is a follow-up candidate, not an objection to the merge.

Reviewed with AI assistance.

wenshao added a commit to wenshao/qwen-code that referenced this pull request Sep 3, 2026
@wenshao

wenshao commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

PR 10831 — local deep verification (maintainer round)

Verdict: merge-ready — 61/61 scripted assertions passed, 0 unexpected failures. Verified head 07962f57961c5518f9b91ec9d4c9f413ed09ae38 (merge base 055e831556ed9772664dcedb23c6e647c478fc90). The PR's central claim — that the OpenTUI submit path now matches ink in the four named places, which is what un-gates the restored interactive E2E leg — is load-bearing: the repo's own three deterministic restored test files go 3/3 green on head and 0/3 on base with byte-identical test files, my independent 7-cell parity harness reproduces each gap on base and its closure on head, and 9 of the 10 new guards die when mutated. Two non-blocking corrections to the PR's own test-plan wording and one coverage gap are recorded below; none is a code defect.

中文摘要

结论:merge-ready(可合并) — 61 条脚本化断言全部通过,0 个意外失败。验证头 07962f5796,合并基 055e831556

  • 核心结论(A/B):本 PR 声称 OpenTUI 提交路径在四处与 ink 对齐,从而解除被 fix(ci): Take the OpenTUI interactive e2e leg out of CI #10829 摘掉的 OpenTUI 交互式 E2E leg 的封印。用仓库自带的三个确定性测试文件做 A/B(测试文件在 base/head 字节相同,仅渲染器源码不同):head 3/3 通过,base 0/3 全红,且每个 base 失败恰好对应 PR 声称修复的一个缺口(见 04-restored-e2e-ab.png)。我另写的 7 格 parity harness(真实 CLI + 真 pty + 环回假 OpenAI 服务器)在 head 全绿、在 base 按预期全红(见下表与 01a/01b02a/02b 截图)。变异矩阵:PR 新增的 10 个守卫中 9 个被单点变异杀死,唯一幸存者 M9(gate 不 await registry 就绪)判定为覆盖缺口而非死代码(见 05-mutation-matrix.png)。
  • 工作流侧:恢复的 job 块与 fix(ci): Take the OpenTUI interactive e2e leg out of CI #10829 删除的 66 行逐字节相同;.size-baseline 的 bump 是必需的(不 bump 则 head 的 e2e.yml 超出旧上限 358 字节);三个 pin 的 action SHA 都能解引用到声称的 tag;workflow-size 门禁 201 个测试通过。
  • Findings(均不阻塞):① PR 的 Reviewer Test Plan 第 2 步说"指向一个不存在的路径"会出现失败卡片并丢弃提交——实测三个 arm 都不会,提交会按字面发送(CLI 自己的 debug 日志确认走的是 "not found → No valid file paths" 分支;描述不准确,代码与 ink 行为一致);② 在 bun 运行时下,不可读(mode 000)的 @-mention 完全不会被解析(无失败卡片、按字面发送),原因是 bun 的 fs.realpath 对不可读文件抛 EACCES 而 node 不会——用 ink-on-bun 的 A/A 对照证明这是运行时属性而非本 PR 缺陷(见 06-aa-control-bun-runtime.png);③ M9 覆盖缺口:没有任何测试断言 mid-turn gate 自身对 registry 就绪的 await。
  • 未覆盖context-compress-interactive.test.ts 需要真实模型,未在本轮运行(其中途 /compress 行为由 harness 的 C2 格以假服务器确定性覆盖);abort 落在 @-展开读取期间的 decline 分支未做 E2E 触发;Windows 未测。

Central claim and the A/B

Central claim. A composer submission under the OpenTUI renderer now (a) carries the raw typed text as UserPromptSubmit provenance, (b) expands user-typed @-mentions to file content where the prompt enters the stream, (c) draws the approval-mode status text, and (d) holds a mid-turn slash command behind the open stream — quit exempt — instead of racing it. The restored e2e-interactive-opentui leg is green because those four gaps are closed.

Primary A/B — the repo's own restored tests. Same test blobs on both arms (sha256-identical; integration-tests/ is untouched by the PR), same renderer pin (QWEN_E2E_RENDERER=opentui → bun + QWEN_TUI_RENDERER_STRICT=1), same loopback fake OpenAI server. Witness: 04-restored-e2e-ab.png.

test file head 07962f5796 base 055e831556 gap it pins
submitted-prompt-provenance.test.ts PASS 2.1 s FAIL — hook input lacks submitted_prompt provenance
external-context-auto-recall.test.ts PASS 2.1 s FAIL — recall produced no results provenance (recall bails without it)
protocol-tags-interactive.test.ts PASS 4.9 s FAIL 30.2 s — readiness polls the mode text approval-mode indicator

Restored OpenTUI E2E leg: the repo's own tests, head vs base

Secondary A/B — my 7-cell parity harness (harness/zz-verify-opentui-parity.test.ts, copied byte-identically into both worktrees; real CLI bundle under a real pty, wire oracle = the fake server's recorded request bodies). VERIFY_EXPECT_ARM flips the expectation per arm so a base-arm red is counted as the control passing, not as a failure.

cell oracle head base witness
C1 approval-mode indicator YOLO mode on the parsed screen drawn not drawn (control) 01a/01b
C2 mid-turn /compress queued notice mid-turn; compression side-query after the turn; request count frozen at 1 while the stream is open notice + side-query (2 requests) nothing at all (1 request forever) 02a/02b
C3 mid-turn /help (opted in) help overlay opens, no queue notice, request count stays 1 overlay opened, no notice, 1 request overlay opened, no notice, 1 request (base has no gate to queue it)
C4 mid-turn /quit behind a queued command exits in ~3.4 s; queued command never replays exit 3.4 s, no replay exit 3.4 s, no replay
C5 readable @-mention at idle file canary present in the model's user message canary present literal @mention only (control) 03
C6 unreadable @-mention (mode 000) submission never silently dropped request arrives request arrives 06
C7 unresolvable @-mention what reaches the model, recorded literal text, no card literal text, no card 06

C1 — approval-mode indicator. Base draws nothing under the composer; head draws YOLO mode.

base 055e831556 head 07962f5796
base: no mode indicator head: YOLO mode drawn

C2 — mid-turn /compress. Base: the command vanishes, no notice, no compression request ever. Head: the queued notice appears while the stream is still open, and the compression side-query lands once the turn ends.

base 055e831556 head 07962f5796
base: nothing happens head: queued notice

C5 — @-mention expansion at the stream layer (head): the transcript keeps what was typed while the model receives the file content.

head: mention expands to file content

C2 is the load-bearing cell: on head the fake server sees exactly one request while the stream is held (the command did not race it), then a second request whose body carries core's getCompressionPrompt() marker once the turn ends; on base the second request never arrives. C4 shows the exit latch works on both arms and that the held queue is discarded rather than replayed behind the exit.

Corrections to the PR's description

1. The test plan's "path that does not exist" repro does not do what it says. Reviewer Test Plan step 2 instructs pointing an @-mention "at a path that does not exist" and expecting "a failed tool card in the transcript and the submission is dropped rather than sent unexpanded". Measured on all three arms (head-opentui, base-opentui, ink-on-node; 3 reps each in harness/mention-matrix.mjs): the submission is sent unexpanded, with the @ intact, and no card appears. The CLI's own debug log names the branch: Path no-such-file-9x7q2.txt not found … will be skippedNo valid file paths found in @ commands to read.shouldProceed: true with the raw query. The new declined path in expandAtMentions (live-session.ts) is reachable only when readManyFiles throws, which its source does solely on abort (signal?.throwIfAborted() / isAbortError rethrows) — i.e. an Esc landing inside a large @ read. The unit test reports a failed @-mention read instead of sending the unexpanded text mocks shouldProceed: false directly, so nothing in the suite exercises a real input that reaches it. This is a description-accuracy issue, not a code defect: OpenTUI and ink agree on the observable behaviour, and the defensive branch mirrors ink's.

2. live-session.ts's comment "the one decline is a failed read" is imprecise for the same reason — the decline is an abort during the read; a failed read (per-file EACCES) yields an Error-status card and still proceeds.

Findings (non-blocking)

F1 — under bun, an unreadable @-mention is never resolved; the failed-read card cannot appear on this leg. Informational, not introduced by this PR. On ink-on-node a mode-000 mention resolves, renders Failed to read …: EACCES: permission denied, and the error text rides to the model as the file's content (3/3 reps). On head-opentui it does not (3/3). The tempting reading — a parity gap in the PR's new expansion path — is wrong: holding the renderer fixed and swapping only the runtime reproduces the absence with the ink code (ink-on-bun 0/3, identical to head-opentui). Cause, measured directly: fs.realpath on a mode-000 file returns OK under node and throws EACCES under bun, and resolveAtCommandQuery's realpath gate then skips the mention. Witness: 06-aa-control-bun-runtime.png. Worth knowing for anyone debugging @-mentions on the restored leg; the leg cannot dodge it by switching runtime, because OpenTUI under node fails to boot at all under QWEN_TUI_RENDERER_STRICT=1 (measured: the CLI never reaches the prompt).

A/A control: the mode-000 divergence tracks the runtime, not the renderer

F2 — the restored leg's readiness signal is the very text gap #2 fixes. protocol-tags-interactive.test.ts polls the terminal for the approval-mode text as its startup readiness signal, so on base it burns its full 30 s timeout before any assertion runs. Not a defect — it is why the leg was red — but it means a future regression in the indicator will surface as a startup timeout in an unrelated-looking file.

Not covered

  • context-compress-interactive.test.ts (the fourth restored file) needs a live model; it did not run in this round. Its mid-turn /compress behaviour is covered deterministically by harness cell C2 against the fake server, and by the PR's own dispatched CI runs cited in the description.
  • The abort-during-@-read decline branch (the only real input shape that reaches expanded.declined) was not triggered end-to-end; it is pinned only by the mocked unit test.
  • C3 needed a submit-retry in the harness: the composer occasionally swallows a fast Enter keystroke burst (observed once on base), so bootAndStartFirstTurn re-presses Enter until the fake server sees the turn. With that, C3 is clean on both arms.
  • Windows: the leg does not run there and neither did this round.
  • Per-commit attribution across the PR's 11 commits: verified as the aggregate base..head diff; the four behavioural fixes and the CI commit were not bisected individually.
  • The repo-wide unit suite and the ubuntu CI lanes: not re-run; the affected workspace's suite and the targeted gates below are what this round measured.

Targeted gates

gate result
packages/cli opentui unit suite, head 67 files, 1056 tests passed
same suite, base 67 files, 1033 tests passed (Δ +23 tests, all new, 0 failures either side)
npm run typecheck, head clean (the gate proved live: it caught two type errors in my own harness file before I fixed them)
workflow-size ratchet (scripts/tests/workflow-size.test.js) 201 tests passed
workflow-side assertions (harness/wf-checks.mjs) 16/16
pinned action SHAs dereference to claimed tags 3/3 (checkout df4cb1c→v6.0.3 via annotated tag, setup-node 48b55a0→v6.4.0, setup-bun 0c5077e→v2.2.0)

Workflow-side highlights: the restored job block is line-identical, in order, to the 66 lines #10829 removed; the .size-baseline bump is required by this PR's own growth (with the base baseline the head e2e.yml is 358 bytes past the 25 533-byte ceiling, while base itself sat 2 252 bytes under it — which is why main never went red); the leg pins bun-version: 1.3.14, matching DEFAULT_BUN_VERSION in scripts/build-standalone-release.js; and a missing bun is a hard error rather than a node fallback.

Mutation matrix

Suite under mutation: the five opentui unit files the PR touches (160 tests). Each mutation is a single-point edit to one production file; git checkout restores it afterwards and the post-run git status on packages/cli/src/ui/opentui/ was clean. Witness: 05-mutation-matrix.png.

# mutation suite verdict
ctl unmutated control 160 passed green
M1 drop isExitInProgress() from the held-command drain 1 failed KILLED
M2 drop the quit exemption in mustDeferDuringStreaming 1 failed KILLED
M3 drop onInterrupt() from the quit branch 2 failed KILLED
M4 stop forwarding submittedPrompt on the first send 2 failed KILLED
M5 disable @-expansion at the stream layer 2 failed KILLED
M6 send the unexpanded text when the read declines 1 failed KILLED
M7 never draw the approval-mode status text 3 failed KILLED
M8 drain the whole mid-turn queue into one turn (pop-all) 1 failed KILLED
M9 answer the defer verdict without awaiting registry readiness 160 passed SURVIVED
M10 defer every command, ignoring the canRunDuringStreaming opt-in 1 failed KILLED

Mutation matrix as printed

Nine of the ten new guards are load-bearing against the suite that claims to pin them. The single survivor is classified below; no mutant regressed from killed to survived relative to the control, and the control itself is green, so the kills mean something.

F3 — M9 survivor: coverage gap, not dead code (non-blocking). OpenTuiSlashGateway.mustDeferDuringStreaming awaits this.ready so the defer verdict is answered from the real registry. Delete that await and all 160 tests stay green. The clause is not dead: for a submission that lands in the window before the dispatcher-attach effect runs, the mutated gate answers "do not defer" from a null dispatcher while dispatch() still awaits readiness — the two paths differ in exactly that window, which is the race the app-shell's deferredRevision nonce comment describes. What is missing is a test that puts a submission in that window; the existing drains a held command whose defer verdict lands after the idle edge pins the drain side, not the gate's own await. Suggested pin (collapsed):

minimal test sketch for the M9 window

Construct the gateway, do not let the dispatcher-attach effect resolve, set streaming, submit a non-opt-in command, then resolve readiness and let the turn end: with the await the command must surface as queued and run at idle; without it the command is dispatched immediately. Assert the queued notice, which only the awaiting gate produces.

Methodology

Two detached worktrees (/Users/wenshao/pr10831-verify/{head,base}) at the resolved headRefOid/baseRefOid from gh pr view, each with its own npm ci (exit 0), npm run build and npm run bundle; nothing in the maintainer's working tree was touched. Every cell drives the real dist/cli.js under a real pty — bun for the OpenTUI renderer, node for ink, exactly as integration-tests/renderer-matrix.ts does, with QWEN_TUI_RENDERER_STRICT=1 so a silent ink fallback fails loudly (verified: OpenTUI under node never reaches the prompt) — against a loopback fake OpenAI SSE server that records every request body, with the ambient agent-session environment and any real model credentials scrubbed. Two independent oracles per cell: the wire (what the fake server received) and the parsed terminal cell grid (@xterm/headless, never the raw ANSI stream). Screenshots use the same xterm→SVG→sharp pipeline as scripts/verify-capture.mjs, fed by the live pty. Raw logs, harness sources and per-cell diagnostics are in this run's logs/ and harness/; the harness file is byte-identical in both worktrees (sha256 4858333f80f41a2e71775e7f3de569eb61064c3f5918bc63eaddb9010d9c2fde, recorded in logs/harness-sha.txt). Assertion counting rule: one entry per cell-oracle, per workflow check, per mutation, per gate run; boot-readiness waits and the PR's own 1056 unit tests are not enumerated.


Local round artifacts (report, harnesses, raw per-cell logs, diagnostics): /Users/wenshao/git/qwen-code-x3/tmp/pr10831-verify-20260903-111319/ · evidence branch: wenshao/qwen-code@assets-pr10831 under pr10831/ · verified head: 07962f5796 · assertions: 61 pass / 0 fail.

@wenshao

wenshao commented Sep 3, 2026

Copy link
Copy Markdown
Collaborator

Addendum to the maintainer round above — three things it listed as not covered, plus a mutation refinement

A second local rig ran in parallel with the round above (different machine session, different harness, same conclusion). Rather than repeat what it already established, this comment carries only what is new or different: the fourth restored E2E file actually run, wire proof that the compaction took effect, a bundle-level reproduction of the R2-1 quit regression, and a correction to the exit-latch mutation result.

Provenance. Tested tree = PR head 07962f5796 merged with origin/main fa5f7a11a0. The PR merged (b5560494b2) while this was running, so this is a post-merge confirmation: for all 8 files the PR touched, my blobs are identical to what landed on main (git hash-object vs git rev-parse origin/main:<path>, 8/8 same). Rig: real dist/cli.js under bun 1.3.14, QWEN_TUI_RENDERER=opentui + QWEN_TUI_RENDERER_STRICT=1, node-pty + @xterm/headless, isolated QWEN_HOME, and a fake OpenAI server that holds the first SSE stream open for a fixed window and ledgers every request body. BEFORE arm = the same tree with only the six PR source files reverted to pre-merge main (03f20983f3), re-bundled (cli.js sha256 587bd1e5… vs 6103f96e…).

1. The fourth restored file ran — and so did the whole leg, on both arms

context-compress-interactive.test.ts needs a live model, which is why both rounds above left it out. Pointing the suite at a deterministic fake endpoint (with an isolated QWEN_HOME — an ambient ~/.qwen silently overrode OPENAI_BASE_URL on my first attempt and sent these cases at a real model, which is what makes them slow and flaky locally) makes it runnable:

AFTER   Test Files  8 passed | 1 skipped (9)          Tests  14 passed | 2 skipped (16)
BEFORE  Test Files  3 failed | 5 passed | 1 skipped   Tests  3 failed | 11 passed | 2 skipped
        red on BEFORE: submitted-prompt-provenance, external-context-auto-recall, protocol-tags-interactive

The AFTER line matches CI run 33642038701 exactly — Test Files 8 passed | 1 skipped (9), Tests 14 passed | 2 skipped (16). Both compression cases pass (should trigger chat compression with /compress command 29.2 s, should forward /compress instructions through to the side-query 29.6 s).

One honest caveat: context-compress-interactive went green on my BEFORE arm too. With a deterministic server the first stream ends before /compress is typed, so the submission is not actually mid-turn — the CI red depended on a live model still streaming. That is a fidelity gap in the rig, and §2 closes it deliberately.

2. The compaction did not just fire — it replaced the history

Holding the first stream open for a fixed 30 s and typing /compress at ~t+4 s makes the mid-turn case deterministic. Beyond "a side-query lands", I checked the next turn's request body:

BEFORE AFTER
notice on screen none Queued /compress — it will run when the current response ends.
compression side-query never +30.2 s — the instant the stream ended
following turn's history still carries the 2 000-word filler carries COMPRESSED_SUMMARY_CANARY, filler gone

mid-turn gate

3. The quit exemption, reproduced at the bundle level

The round above kills this mutant against the unit suite. I also built it as a shipped-bundle mutant — one line deleted (if (commandToExecute?.name === 'quit') return false;), re-bundled, driven through a real pty — so the user-visible failure is on record: mid-turn /quit prints Queued /quit — it will run when the current response ends. and the process is still alive 20 s later. Unmutated, /quit and /exit both exit (code 0) ~1.5 s after Enter, against a 60 s stream hold. That is @qqqys's R2-1 exactly, and the fix closes it. (Third row of the image above.)

4. Correction: either exit-latch check can be deleted alone and the suite stays green

The matrix above records M1 — drop isExitInProgress() from the held-command drain → KILLED. That holds when both call sites go at once. Deleting them one at a time, against the full src/ui/opentui suite (1056 tests), neither is pinned:

mutant suite
delete only the drain-edge isExitInProgress() 1056/1056 green — SURVIVED
delete only the in-loop isExitInProgress() 1056/1056 green — SURVIVED
delete await this.ready in mustDeferDuringStreaming 1056/1056 green — SURVIVED (= the M9/F3 already filed)
12 others (quit exemption, btw exemption, provenance send, @ expansion, decline drop, holdsUi pause, one-submission replay, status text, queue push, quit clears queue, onPopQueue, onInterrupt) KILLED, 1–5 tests each

@chiga0's round-4 note flagged the in-loop check as unpinned; the edge check is unpinned too, and that is the one the round-3 thread was actually about. So the follow-up on #8662 should pin both sites, not one. I could not build an E2E probe that separates the shipped build from the both-latches-removed build (queue /compress, then Ctrl+C×2 / Ctrl+D×2 behaved identically), so this stays a coverage gap rather than a demonstrated defect.

5. Queue corners, measured on the wire

/approval-mode then /compress, both typed mid-stream, one clock for keystrokes and requests:

  • t+4.1 s / t+5.9 s — both queued (single notice slot: the second Queued … overwrites the first, so only one is visible).
  • ~t+27 s — turn ends, /approval-mode drains and its dialog opens.
  • t+27 s → t+67 s — zero compression traffic for the whole 40 s the dialog is up.
  • t+67.1 s — Esc closes the dialog; the compression side-query leaves in the same second.

And with /compress queued, a mid-turn /quit exits at t+7.3 s having sent zero compression requests — the queue is discarded, not replayed into cleanup.

queue and provenance

Smaller confirmations (agreeing with the round above)

  • Approval-mode indicator across all five modes, not just YOLO: yoloYOLO mode, auto-editAccepting edits, autoAuto mode, plan/default→nothing, and BEFORE draws nothing in any of them. approval mode
  • Provenance: submitted_prompt exactly the typed text, prompt carrying EXPANDED_FILE_CANARY_7585, and the tool-result continuation carrying no submitted_prompt at all; BEFORE has neither and sends the literal mention to the model.
  • No regression on the immediate paths: /help mid-stream opens at once (both arms); ?btw reaches the model at +2.3 s while the stream is held to +40 s; plain mid-turn text is queued and becomes the next turn at +40.8 s.
  • Counterfactual on the units: reverting the six source files turns 23 tests red across all 5 touched files. npm run typecheck clean; workflow-size.test.js 201/201; e2e.yml exactly 25 891 bytes = the bumped baseline.
  • I hit the "unreadable @-mention passes through silently" behaviour too (mode 000, plus a nonexistent path and a path outside the workspace — all three sent unexpanded with no card). The round above already explains it: bun's fs.realpath throws EACCES where node's does not, so it is a runtime property, not a renderer gap. Nothing to add beyond confirming it reproduces here.

Not covered here

Windows. Wire-level proof that a mid-turn /quit cancels the in-flight request before cleanup — my server never observes a socket close on client abort (undici holds it), so I can only show the process exits at t+7 s against a 60 s hold. npm run test:scripts had 3 unrelated red files on this sandbox, all Test timed out in install-script / qwen-autofix; the workflow-size gate itself is green.

Verdict: unchanged — the merged behaviour is what the PR describes. The one thing I would carry forward is §4: pin both exit-latch sites, not just the in-loop one.

中文说明

对上一轮维护者验证的补充 —— 它列为"未覆盖"的三项,外加一处变异结论的修正

上一轮维护者验证与本轮在同机并行进行(不同会话、不同装置,结论一致)。为避免重复,本条只写新增或不同的部分:真正跑起来的第四个 E2E 文件、压缩"确实生效"的链路证据、R2-1 退出回归在打包产物层面的复现,以及退出闩变异结论的一处修正。

溯源。 测试树 = PR head 07962f5796 合并 origin/main fa5f7a11a0。验证过程中 PR 已被合入(b5560494b2),因此这是合入后确认:本 PR 触碰的 8 个文件,我的 blob 与落到 main 上的完全一致(git hash-objectgit rev-parse origin/main:<path> 对比,8/8 相同)。装置:真实 dist/cli.js 跑在 bun 1.3.14 下,QWEN_TUI_RENDERER=opentui + QWEN_TUI_RENDERER_STRICT=1node-pty + @xterm/headless,隔离 QWEN_HOME,配一台吐完文本后把第一条 SSE 流保持打开固定时长并记录每个请求体的假 OpenAI 服务器。BEFORE 臂 = 同一棵树,把本 PR 的六个源文件回退到合入前 main03f20983f3)后重新打包(cli.js sha256 587bd1e5… vs 6103f96e…)。

1. 第四个恢复文件真正跑了 —— 而且整条腿在两臂都跑了

context-compress-interactive.test.ts 需要真实模型,这也是上面两轮都把它排除的原因。把套件指向确定性的假端点(并使用隔离的 QWEN_HOME —— 我第一次尝试时宿主 ~/.qwen 静默覆盖了 OPENAI_BASE_URL,把这些用例发到了真实模型上,这正是它们在本地又慢又抖的原因)之后,它是可以跑的:

AFTER   Test Files  8 passed | 1 skipped (9)          Tests  14 passed | 2 skipped (16)
BEFORE  Test Files  3 failed | 5 passed | 1 skipped   Tests  3 failed | 11 passed | 2 skipped
        BEFORE 变红:submitted-prompt-provenance、external-context-auto-recall、protocol-tags-interactive

AFTER 这一行与 CI run 33642038701 完全一致 —— Test Files 8 passed | 1 skipped (9)Tests 14 passed | 2 skipped (16)。两个压缩用例均通过(should trigger chat compression with /compress command 29.2 秒、should forward /compress instructions through to the side-query 29.6 秒)。

一点必须如实说明的保留:context-compress-interactive 在我的 BEFORE 臂上也是绿的。用确定性服务器时,第一条流在 /compress 键入之前就结束了,那次提交并不真的处于回合中 —— CI 上的红依赖于真实模型仍在输出。这是装置保真度的差距,§2 就是专门为此补上的。

2. 压缩不只是"发出去了",而是真的换掉了历史

把第一条流固定保持 30 秒、在约 t+4 秒键入 /compress,让流中场景变成确定性事件。除了"侧查询落地"之外,我还核对了下一轮的请求体:

BEFORE AFTER
屏幕提示 Queued /compress — it will run when the current response ends.
压缩侧查询 从未发出 +30.2 秒 —— 正是流结束的瞬间
之后那一轮的历史 仍带着 2000 词填充文本 COMPRESSED_SUMMARY_CANARY,填充文本消失

3. quit 豁免,在打包产物层面复现

上一轮用单测杀死了这个变异体。我额外把它做成已发布 bundle 的变异体 —— 删掉一行(if (commandToExecute?.name === 'quit') return false;)后重新打包,用真实 pty 驱动 —— 于是用户可见的失败被记录了下来:流中 /quit 打印 Queued /quit — it will run when the current response ends.,进程20 秒后仍然活着。未变异时,/quit/exit 都在回车后约 1.5 秒退出(退出码 0),而流的保持时长是 60 秒。这正是 @qqqys 的 R2-1,修复确实闭合了它。(见上图第三行。)

4. 修正:两处退出闩检查,任意一处单独删除,套件都保持全绿

上面的矩阵记录为 M1 —— 从被扣命令的 drain 中删掉 isExitInProgress() → KILLED。这在两处调用点同时删除时成立。但逐一删除,在完整 src/ui/opentui 套件(1056 条)下,两处都没有被钉住:

变异体 套件
只删 drain 边沿isExitInProgress() 1056/1056 全绿 —— 存活
只删循环内isExitInProgress() 1056/1056 全绿 —— 存活
mustDeferDuringStreaming 里的 await this.ready 1056/1056 全绿 —— 存活(即已记录的 M9/F3)
其余 12 个(quit 豁免、btw 豁免、来源字段发送、@ 展开、decline 丢弃、holdsUi 暂停、每轮一条重放、状态文本、队列 push、quit 清队列、onPopQueueonInterrupt 全部被杀,各杀 1–5 条

@chiga0 的第 4 轮备注指出循环内那处没被钉住;边沿那处同样没有,而且它才是第 3 轮线程真正讨论的那一处。所以 #8662 上的跟进应当把两处都钉住,而不是只钉一处。我没能构造出让"已合入构建"与"两处闩都删掉的构建"表现不同的 E2E 探针(先排队 /compress,再连按两次 Ctrl+C / Ctrl+D,两者行为一致),因此这仍只是覆盖率缺口,而非已证实的缺陷。

5. 队列边角,按链路计时

流中先后键入 /approval-mode/compress,按键与请求共用同一时钟:

  • t+4.1 秒 / t+5.9 秒 —— 两条都进队列(单一提示槽:第二条 Queued … 覆盖第一条,用户只看得到一条)。
  • 约 t+27 秒 —— 回合结束,/approval-mode 出队执行,对话框打开。
  • t+27 秒 → t+67 秒 —— 对话框存在的整整 40 秒里,链路上压缩流量。
  • t+67.1 秒 —— Esc 关闭对话框;压缩侧查询在同一秒发出。

另外,在 /compress 已排队时键入流中 /quit:t+7.3 秒退出,全程压缩请求 —— 队列被丢弃,没有重放进清理链。

其余确认(与上一轮结论一致)

  • 审批模式指示器覆盖全部五种模式而不只是 YOLO:yoloYOLO modeauto-editAccepting editsautoAuto modeplan/default→不画,且 BEFORE 在任何模式下都不画。
  • 来源字段:submitted_prompt 恰好是键入原文,promptEXPANDED_FILE_CANARY_7585,工具结果续跑那次完全没有 submitted_prompt;BEFORE 两者皆无,且把字面引用发给了模型。
  • "立即执行"路径无回归:流中 /help 立即打开(两臂一致);?btw 在 +2.3 秒到达模型,此时流仍保持到 +40 秒;流中普通文本进队列并在 +40.8 秒成为下一轮。
  • 单测反事实:回退六个源文件后,5 个被触碰文件上共 23 条变红npm run typecheck 干净;workflow-size.test.js 201/201;e2e.yml 恰好 25 891 字节 = 更新后的基线。
  • 我也遇到了"不可读 @ 引用被静默透传"的现象(权限 000,外加一个不存在的路径和一个工作区外的路径,三种都未展开直接发送且无卡片)。上一轮已经给出解释:bun 的 fs.realpath 会抛 EACCES 而 node 不会,所以这是运行时属性而非渲染器缺口。这里只是确认能复现,无需补充。

本轮未覆盖

Windows。流中 /quit "在清理前取消在途请求"缺少链路级证据 —— 客户端 abort 时我的服务器观察不到 socket 关闭(undici 会保持连接),所以我只能证明进程在 t+7 秒退出,而流的保持时长是 60 秒。npm run test:scripts 在本机有 3 个无关文件变红,全部是 install-script / qwen-autofixTest timed out;workflow 体积门禁本身是绿的。

结论不变:合入后的行为与 PR 描述一致。 唯一值得带走的是 §4:请把两处退出闩都钉住,而不只是循环内那一处。

wenshao added a commit to CanReader/qwen-code that referenced this pull request Sep 3, 2026
* docs: Design the OpenTUI submit and exit parity batch

Six gaps where the ink renderer has a mechanism the OpenTUI port carried
only the shadow of: bare quit tokens, the missing client shutdown signal
on exit, mid-turn steering that skips the vision bridge and @-expansion,
an e2e leg that cannot hold a stream open, and the noflicker scenario's
outcome being judged globally.

* fix(cli): Let bare quit tokens reach the OpenTUI dispatcher

Typing `exit`, `quit`, `:q` or `:wq` in the OpenTUI renderer sent them to
the model as prose, and mid-turn they queued behind the stream instead of
stopping it. ink rewrites that family to `/quit` ahead of its message
queue; the port had the exemption but not the rewrite, so the gate let a
quit through only in the one spelling that already looked like a command.

* fix(cli): Stop background work before an OpenTUI session exits

Every OpenTUI exit drained the cleanup chain without first telling the
client that shutdown was imminent, so memory extract, dream and skill
review could spawn new agent work during the exit window — the one thing
that keeps a process from finishing. ink makes that signal on its quit
path; here all three exits share one drain, so the signal belongs to the
drain.

* fix(cli): Resolve OpenTUI mid-turn steering like an idle prompt

Text the composer queues while a turn runs reached the model raw: no
`@path` expansion, no read deadline, and no prompt-side vision bridge, so
an image could ride as `inlineData` to a model that cannot read it while
ink converts it and discloses the egress. Both hops now resolve a steered
message the way an idle submission does — expansion under ink's 10 s read
timeout, then the bridge — and a turn that dies mid-resolution hands its
texts back to the queue instead of losing them.

The bridge can also pick a full-turn vision model mid-turn; the rest of
the turn stays on it, including the model named in the mapper's own
notices, so the override is read per boundary rather than once per turn.

Part of U-21 and U-25 in QwenLM#8662.

* test(cli): Pin the exit check between queued command dispatches

Round four of QwenLM#10831 noted that the deferred-command drain checks the
shared exit latch twice but only the pre-loop check had a test: deleting
the per-iteration one left the suite green. This adds the crossing that
check exists for — the exit begins while an earlier command is still
awaiting its outcome — by flipping the latch inside the first dispatch and
requiring the second never to run.

Part of U-23 in QwenLM#8662.

* fix(scripts): Require the offline parity gate's base to fail

The offline no-flicker scenario runs a fixture emitter that injects clears
and unbalanced DEC 2026 as its base side, yet the runner accepted both-pass
as a passing outcome. A base fixture that stopped emitting its defect would
have kept the gate green while proving nothing at all.

Scenarios can now declare their base a defect fixture with
`expectBaseFailure`, which turns both-pass into a failure — set on the
offline scenario only, since the credentials scenario compares against ink
and the self-test override path asserts both-pass deliberately. A gate
failure names its own cause in the report and on the console line.

Part of U-15 in QwenLM#8662.

* test(integration): Hold the fake model stream open mid-turn

Every mid-turn behaviour in the submit path is invisible to a test that
cannot say "the CLI is streaming right now": the fake server wrote each SSE
chunk synchronously, and awaiting its handler holds a turn only before the
first byte. FakeOpenAIResponse gains holdAfterChunks + holdUntil, so a test
can stop the stream after a delta it can see on screen and release it on
purpose. close() tears held connections down, which is pinned because
"the run cannot hang on my instrument" is exactly the claim an instrument
like this must not make untested.

Four cases use it, one file for both renderers: /quit and a bare `exit`
typed into a live stream, a slash command held back mid-turn, and a steered
@file mention. The last two assert on captured request bodies rather than
screen text, because OpenTUI repaints by diffing cells and renders no slash
command output at all.

Closes U-24 in QwenLM#8662.

* docs: Record the batch-8 measurement boundary and adjacent gaps

The design doc said each new case was checked red-then-green before its own
fix commit. What was done instead is mutation probes, and two of the four
leave the OpenTUI leg green — so replace the claim with the measured table
and its cause: a submission that skips the mid-turn gate is steered into the
same turn and still reaches the model only after the held turn ends, and
process teardown closes the socket however the turn ends. Both halves are
pinned at unit level, which is a different claim.

Register the three gaps this batch found and deliberately did not fix:
steered text has no transcript echo (U-26), neither hop warns about an
unsupported image format (U-27), and OpenTUI keeps slash command output in a
history no view renders (U-28) — the last is why the first version of the
deferral case failed for a reason unrelated to the gate under test.

* docs: Correct the mid-turn requeue rule in the batch-8 design

The design doc said a timed-out mid-turn read hands its text back to the
queue. The code requeues on an abort only — a timeout or a declined read is
dropped with a warning, which is what ink does and what the steering test
pins — so the sentence described a behaviour the port deliberately does not
have.

* docs: Measure both of the batch-8 drain's exit-latch checks

Deleting the in-loop check reddens exactly the test written for it, and deleting the
pre-loop check reddens nothing at unit level. The pair is what makes the documented
redundancy a measurement instead of an argument, so both now sit in the coverage
boundary alongside the two mutations the E2E leg cannot see.

---------

Co-authored-by: Shaojin Wen <shaojin.wensj@alibaba-inc.com>
@qwen-code-ci-bot

Copy link
Copy Markdown
Collaborator

Released in v0.23.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants