fix(cli): close the OpenTUI renderer's parity gaps against ink - #11630
Conversation
The OpenTUI dialog offered one generic four-row list for every tool confirmation. That cost three things users rely on when deciding whether to grant access. The always-allow rows did not say what they were allowing, so approving "Always allow in this project" gave no hint that the grant covers every `touch` invocation rather than the one command on screen. They were also offered in an untrusted folder, where a durable permission rule for a workspace the user has not trusted is not a decision the dialog should put in front of them. Plan and edit confirmations got the wrong choices outright: a plan was never offered the chance to restore the approval mode it replaced, and an edit was offered the project/user-scoped persistence outcomes instead of the session-wide allow-always that an edit actually supports. Approval rows are numbered again, and a digit key picks one, matching the inline prompt. Each type now also shows its own question line, so an execution approval names the command root being permitted.
The OpenTUI renderer constructor installs a bare `globalThis.window` to hang its requestAnimationFrame shim on. web-tree-sitter's UMD wrapper probes `window.document.currentScript` when it is first evaluated, so the first dynamic import after that point throws, and the parser latches that failure permanently for the rest of the process. Every OpenTUI session therefore silently lost AST-based shell analysis: permission rules came back empty, so always-allow confirmations lost their scope and persisted nothing, and read-only detection and command-safety classification dropped to their conservative fallbacks. Initialise the parser before the renderer exists, while `window` is still undefined. Adds a one-line re-export from the core package index; web-tree-sitter stays dynamically imported, so the deferred-runtime invariant is unchanged.
The status row was truncated to a single line, so a narrow terminal dropped the model segment that ink wraps onto a second row. The hint row now carries the approval-mode name the composer stopped drawing, since ink uses that text as an aria-label rather than a visible row and this renderer has no aria surface. The hint row is still truncated, so it cannot grow the footer mid-turn. The loading indicator advertised an output-token estimate it never received: the character counter and the receiving flag were declared as props but no caller set them, so the estimate stayed pinned at zero and the direction arrow always pointed down. Both now come from the live turn, counting model text, thoughts and tool-call arguments the way ink does, and flipping back to the waiting phase when tool results go to the model. The footer also hid while a completion list was open only in ink; here it drew under the dropdown and pushed the composer around.
The slash dropdown never populated a command's source badge, so skill, plugin and MCP rows lost the marker ink draws and the shared label column was sized without counting it. Descriptions were emitted verbatim, so a multi-line command blurb kept its frontmatter line breaks and an over-wide one spilled onto a second row, doubling the height of the popup. The row budget also ignored that the dropdown nests its own side margins inside the composer's, which left two columns too many for the description and wrapped its tail. A label and its argument hint were concatenated into one run, so a hint too long for the column word-wrapped the whole string and grew the row to three lines where ink produces two. They are now laid out as separate children and broken at the column edge, which puts the continuation at the hint's own offset exactly as ink does. Shift+Tab was unbound, so the footer advertised a cycle shortcut that did nothing and the mode could only be changed through the dialog. The shell now holds the mode the way ink's indicator does: it cycles on the keystroke, adopts the dialog's choice into the same state, reports a refused change instead of repainting a mode it does not hold, and explains an entry into auto mode — including at startup, where no keystroke ever runs. Windows accepts a bare Tab because some terminals there cannot distinguish the two, and the completion consumers both claim the key first, so a Tab spent accepting a suggestion never reaches the cycle. The cycle order now comes from the shared list of modes rather than a copy of the enum's declaration order, so it cannot drift from the one ink walks.
Both of ink's routes into auto mode gate the entry notices on the session not already holding it. The rotation could never violate that, so the guard looked redundant here and was left out — but the approval-mode dialog opens with the current mode already selected, so a bare Enter re-picks it. The first-time message survives that because it is acknowledged in settings; the notice listing the allow rules auto mode stripped does not, and would reprint on every re-pick. The gate now sits where both routes meet, and the comment that claimed ink's slash command had no such gate is corrected: it does.
The dropdown sized its label column the same way whatever was being completed. That is right for slash commands, where every row shares one half-width command column so the descriptions line up, but a file list is not a command list: ink only shares a column there when a row carries a description to line up against, and lets a plain path take the whole row. Clamping it to half the width instead wrapped any path longer than that onto a second row, mid-word, so one entry occupied two lines and pushed the rest of the list down. The column now follows the mode being completed, and the cell keeps the shared width only where a description has to line up against it. The minimum readable description width that caps the shared column moves next to the other constants the two renderers have to agree on, rather than being copied out of one of them.
…column The label column gained a branch for rows that carry a description, mirroring the other renderer. That branch cannot fire here: this renderer's `@` completion only ever asks the file index, and its results carry no description, so the only rows that reach the dropdown with one are slash-command rows — which already take the shared half-width column. The column is therefore either that column or none, and the minimum readable description width that capped the dead branch goes back to being local to the renderer that uses it.
Every popup this shell renders spanned the terminal edge to edge, while ink wraps its popups in a two-column margin and caps their width, so a border ran from column 2 to column 97 and stopped there. The wrapper now supplies both, which is what makes a dialog read as a dialog rather than a full-screen mode. The confirmations stay outside it: their body measures the terminal width to estimate how its text wraps, so narrowing the box without also narrowing that measurement would corrupt the estimate. The help dialog opened on its command list rather than its overview, and its tab keys did not match the ones the footer advertises. Both now follow ink: the overview is the opening tab, Tab and Shift+Tab cycle it in the two directions the hint promises, the arrow and page keys move the command window and are inert on a tab that has none, and no other key closes or navigates. Closing on a bare letter key meant a typo dismissed the dialog. That command window was a fixed eighteen rows whatever the terminal height, which is more than the body budget leaves once the tab's own introduction line, its gap and the scroll hint are counted. The overflow was resolved by dropping the gap under the introduction and clipping the hint away entirely, so the scroll position the hint reports was simply absent. The window is now sized to what the budget leaves after that chrome, and paging moves by the window that is actually on screen. Below a 42-row terminal this shows a shorter list than ink does, which is the smaller loss: ink keeps its eighteen rows and clips the hint instead. The model dialog drew its detail rule twenty characters wide against ink's full-width one, and showed no line under a model's title. ink folds the runtime and discontinued markers into the row description as well as the title, so a runtime model with nothing of its own to say still gets an explanatory line; the entries now carry that, and the rule is spelled out to the frame's inner width because there is no single-sided border here to draw one with.
A frame-by-frame comparison of both renderers over 14 pty scenarios turned up a set of divergences that were invisible in unit tests. This groups them by topic. Tool output is a snapshot, not an increment. Core hands the shell the whole accumulated display on every progress event, so a running tool card that appended painted the streamed text once per event. The event field is renamed to say so, the card replaces instead of accumulates, and the throttle that stops a redundant re-emit now compares lengths. Shell mode loses its tail-deduplication tail and writes the same string to the card and to the model history, so the two cannot disagree. The transcript and the composer get ink's horizontal margins, which makes the dropdown, the attachments and the body columns land where ink puts them. The status rows gain the prefixes ink draws -- a warning row had none at all -- and the error row collapses its hint onto the same line instead of a second one, with the prefix pinned so a wrapped body cannot shrink it away. The footer's loading indicator moves to ink's indent, and an armed quit warning now renders even while a dialog, a confirmation or a dropdown has the rest of the footer hidden. Previously the gate that hid the footer also hid the warning, so pressing Ctrl+C once with a dialog open gave no feedback at all. Shift+Tab cycles the approval mode from the top level, so it keeps working while a dialog or a confirmation has the composer unmounted -- the on-screen hint advertises it in exactly those states. The Windows fallback that also accepts a bare Tab is narrowed to bare Tab only, since a terminal that can tell them apart would otherwise cycle twice. The context-files announcement is latched with the predicate ink uses and re-armed when the transcript resets, so a new session in the same renderer announces its context files again. The model dialog carries the three guards ink has. A successful pick no longer also announces the model that survived, and neither a second Escape nor a second Enter landing mid-apply can announce or start a second switch. Dialog selection markers use ink's chevron rather than a filled bullet, and the auth dialog takes the shared icon glyphs. Adds the bilingual design doc recording each decision, the coverage boundary and the follow-ups.
ink leaves a blank row above most conversation items and decides it per item type in its history renderer; this renderer printed every item flush against the one before it, so a completed turn read as one unbroken block where ink reads as separated rows. The comparison harness had been folding the gap away: it reduces each captured frame to its non-blank rows, so a capture could be reported byte-identical while the two renderers disagreed on every vertical gap between items. Measuring the transcript region alone, as a run-length pattern of blank and content rows, isolates it and is unaffected by the two renderers anchoring to opposite ends of the screen. After the fix that pattern matches ink's group for group in every capture that carries a conversation item. Each item is now wrapped in a box carrying the margin resolved from its kind. Two kinds have no ink counterpart -- a task card, because ink renders a subagent as the tool that spawned it, and an image row, because ink draws images inline inside the message that carries them -- and both take zero so they stay flush against the tool row beside them. The margin sits on the wrapper while the per-item row cap applies to the item's own content, so the two are additive and neither can clip the other. The design documents record the mapping in both languages, along with two limits of the evidence: layout props are unobservable in the unit test runtime, so this is verified by frames only, and the blank rows ink shows around its banner come from how it batches permanent writes rather than from a layout rule, so they are deliberately not reproduced.
… tool-result paths An approval-mode switch now confirms the calls it would not have parked. ink pairs the switch with the release; the selection rule deciding which calls qualify was already ported here but had no caller, so the renderer had the rule and not the behaviour. A committed thought names its duration the way ink does, and the key its collapsed hint advertises is now bound at the entry, so it still lands while a dialog or a confirmation owns the screen. A tool result's structured payload is resolved in one place. Five of the six paths that turn a result display into events flattened straight to text, which put a todo list's raw JSON on screen, on resume and on replay, while the card's own checkbox-list renderer sat unused.
The follow-up list reported two of ink's authentication progress screens as missing here, on the grounds that a login could not be completed without them. Neither branch is reachable in ink: no provider in the registry declares that auth type, so the single write site for the pending auth type cannot produce it, and the external-auth state those screens read is assigned null at both of its write sites. Record the check so the omission is not re-reported as a gap. The MCP server startup approval in the same bullet did survive: ink opens that dialog whenever its approval queue is non-empty and this renderer has no counterpart, so it stays.
A server list checked into a project was never offered for approval in this renderer, so a gated server stayed silently disconnected and nothing told the user why or what to do about it. ink opens the dialog whenever its approval queue is non-empty. The queue, the decision that persists against a hash of the config, the un-gating and the reconnect all live in a renderer-agnostic hook, so this is a view and a mount point rather than a second copy of the policy. It ranks above the shell and tool confirmations, as in ink. Geometry was measured against ink at a hundred columns rather than assumed, and the first attempt was wrong twice: inset three with a box one narrower than the shared popup width, and numbered rows. Declining lands on the same composer and footer row in both legs. Two residuals are recorded instead of matched — ink's right border is clipped away by its own overflowing margin, and one wrapped body line carries an extra leading space here.
qqqys
left a comment
qqqys
left a comment
There was a problem hiding this comment.
COMMENT — no Critical found in what I read, but I could not complete the code gates on this diff within budget, so I am escalating rather than approving.
Scope note: 55 files, +4248/-548, roughly 1300 lines of production logic once the two design docs (+1208) and the ~1750 lines of test are set aside. I spent the budget on the surfaces where a defect would be silent and structural — a cross-module symbol move, a cross-module event-field rename, the new core export, and the approval rework — and cleared the first three. What follows separates what is verified from what is not.
Prior review history — nothing to re-verify
No review of any state has been filed on this PR and there are no review comments. The only thread entry is a triage-running lifecycle notice. There is no historical blocking issue.
Verified clean at this head
The normalizeDescription move is complete — no build break. It moved from packages/cli/src/ui/components/SuggestionsDisplay.tsx to packages/cli/src/ui/utils/suggestions.ts, and the old file now imports rather than re-exports it, so any surviving importer of the old path would fail to compile. There is exactly one importer repo-wide, the co-located SuggestionsDisplay.test.tsx, and this PR updates it — the import list drops the symbol and the now-misplaced describe('normalizeDescription') block is removed. The normalizeDescription in packages/core/src/memory/indexer.ts:153 is a separate file-local function, not a reference to this one, so there is no collision.
The tool-output field rename is consistent end to end. shell-mode.ts switched from emitting { type: 'tool-output', id, delta } to { …, output: cumulative }, which renames a field on a union consumed in four places. All four agree at this head: the union declares { type: 'tool-output'; id: string; output: string } (streaming-model.ts:34), the reducer reads event.output (streaming-model.ts:194), live-session-model.ts:325 reads ev.output, live-session.ts:856 and transcript-adapter.ts:138 both emit output. No producer or consumer is left on delta.
The switch from tail-append to whole-output is not a duplication bug. streaming-model.ts:194 assigns items[index] = { ...tool, output: event.output } and live-session-model.ts:325 assigns output: for both tool-output and tool-result, so both are replacement semantics. That is what makes the new finalOutput = prefixText + mainContent in shell-mode.ts correct rather than doubling the streamed text, and the LLM-history write is byte-identical to before — it previously passed ${prefixText}${mainContent} and now passes finalOutput, which is that same string.
The new core export resolves. packages/core/src/index.ts:721 adds export { initParser as initShellAstParser } from './utils/shellAstParser.js', and shellAstParser.ts:587 declares export async function initParser(): Promise<void>. It is the only initShellAstParser in the barrel, so the aliased named export cannot collide with the file's surrounding export * statements.
Gate 1 — the approval rework is security-bearing and I could not certify it fail-closed
dialogs-confirm.tsx (+299/-38) and opentui-dialog-mount.tsx (+105/-26) replace what the file's own header describes as the "deny-everything confirmation bridge" that Batch 5 shipped, with real per-type approval options resolving through confirmationDetails.onConfirm. That trade is the point of the change and is reasonable — but it removes a blanket fail-closed guarantee and replaces it with per-branch behaviour, which is exactly the kind of surface that needs branch-level confirmation rather than a skim.
What I did not establish, and what a clearance needs: that every non-affirmative path resolves to ToolConfirmationOutcome.Cancel and never to a Proceed* outcome. Concretely — the default: arm of the per-type switch (dialogs-confirm.tsx:230) for a confirmation type this renderer has no case for; dialog dismiss, escape, and focus-loss; an empty or filtered-to-empty options array; and the hideAlwaysAllow path (:241-242) that suppresses the always-allow rows for explicit-interaction and PM-ask rules, where a persisted allow rule must not be created. The new gated-MCP-server approval in opentui-dialog-mount.tsx needs the same treatment, since it adds a prompt where previously there was none.
This is not a defect claim — I found no evidence of a fail-open path. It is the one place in the diff where an error would be silent, security-relevant, and invisible to the parity snapshots, and I ran out of budget before reading the 946-line file properly.
Gate 2 — the rest of the production delta is unread
Not examined: opentui-app-shell.tsx (+254/-38), opentui-footer.tsx (+127/-36), input-prompt.tsx (+118/-46), transcript-view.tsx (+76/-33), event-adapter.ts (+50/-41), live-turn.ts (+55/-2), messages.tsx, help-overlay.tsx, help-content.ts, dialog-data.ts, dialogs-model/auth/misc/modes/arena, dialogs-shared.tsx, input-prompt-model.ts, client-tool-run.ts, live-session.ts, start-opentui-ui.tsx, streaming-model.ts beyond the union and reducer case above, and commands-dispatch.ts beyond the two changes noted below.
Two things I read in passing and did not fully verify, both worth a second look by whoever picks this up: commands-dispatch.ts now calls refreshNeededListener() synchronously from the constructor when the extension-refresh latch is already set, which runs before the renderer has mounted; and shell-mode.ts changed the streamed event from an append-delta to a full-output replacement, which is consistent with the reducers I checked but is a contract change for any consumer outside the four.
Gate 3 — nothing has certified the build at this head
Every substantive lane is still pending: Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox), OpenTUI no-flicker gate and TUI parity snapshots (ink vs opentui). Only Classify PR, both Desktop Shell jobs, assign, authorize, label and Remind on force-push have completed, all green. Pending checks are not a blocker under this gate and I did not wait on them, but they matter more than usual here: this diff renames a field on a cross-module event union and moves an exported symbol between modules, which are precisely the two shapes typecheck exists to catch. I verified both by reading rather than relying on a green lane; a completed Lint & Static would confirm the other ~50 files I did not read.
Gate 4 — the repository's own core-infrastructure rule points to escalation
The diff adds an export to packages/core/src/index.ts, the public barrel, and changes both packages/core and packages/cli, so it is a cross-package change touching a core path. Under AGENTS.md's two-tier gate that is Tier 2: clearance requires 100% confidence with every downstream consumer named, and any doubt escalates. The 1000+ production-line advisory also applies. It is fix-typed rather than refactor, so the large-scope hard block does not engage. Gates 1 and 2 are that doubt.
What would let this clear
The cheapest path is a completed Lint & Static plus Test at this head, which retires Gate 3 outright, plus a branch-level confirmation of the Cancel-on-every-non-affirmative-path property in Gate 1 — the default: arm, dismiss/escape, empty options, and the hideAlwaysAllow rule. Gate 2 then reduces to a maintainer's judgement about whether a renderer-parity sweep of this size needs a full read before merge, which is a scope question rather than a defect.
|
Thanks for the PR — the evidence bar here is unusually high. A scripted pty harness driving both renderers through 18 scenarios (36 legs, 96 frames) with the before/after grids pasted inline rather than asserted makes the problem statement easy to accept, and the places where a claim has no frame evidence are named as such instead of being glossed. That honesty is what makes the rest of the description trustworthy. Template — complete ✓, including the bilingual section and the design doc in both languages. Problem — observed, not theoretical. Each defect in the description has either a frame capture or an explicit reason it could not be captured (the reasoning-row changes have no frames because the harness's fake model has no Direction — aligned. The renderer is opt-in behind Size — 4,796 changed lines across 55 files:
Core is touched, but only by Approach — the individual fixes are well-targeted, and several are genuine de-duplications rather than added surface: the locally-copied The scope question is bundling, not any single fix. Roughly fifteen independent defects land together across the transcript, composer, footer, dialogs and keyboard, plus one new feature. Two things I'd genuinely consider splitting out:
If you cut 80% of this — the tool-output snapshot fix, the footer/status prefixes, the Ctrl+C quit warning, the Shift+Tab top-level binding — would the renderer already be usable for Phase 2 validation? Probably close. That is the argument for splitting; it is a question, not a blocker. Risk — no Stage 1e high-risk path matched (
Moving on to code review. 🔍 中文说明感谢贡献 —— 这个 PR 的证据标准相当高。用脚本化的 pty harness 驱动两个渲染器跑 18 个场景（36 条 leg、96 帧），并把前后帧网格直接贴在正文里而不是口头断言，这让问题描述很容易被接受；而那些没有帧证据的地方也被明确点名，而不是含糊带过。正是这种坦诚让描述的其余部分值得信任。 模板 —— 完整 ✓，包含中英双语说明和两种语言的设计文档。 问题 —— 是已观测到的缺陷，不是理论性加固。描述里的每个缺陷都配有帧捕获，或明确说明了为什么无法捕获（思考行没有帧，是因为 harness 的假模型没有 方向 —— 对齐。渲染器目前由 规模 —— 55 个文件、4,796 行改动：生产逻辑 1,755 行（30 个文件）、测试 1,833 行（23 个文件）、设计文档 1,208 行（2 个文件）、生成/schema 0 行。 确实触及了 core，但只有 方案 —— 单点修复都很有针对性，而且有几处是真正的去重而非新增表面：本地复制的 范围上的问题是「打包」，而不是任何单个修复。大约十五个互相独立的缺陷一起落地，横跨会话记录、输入框、页脚、对话框与键盘，外加一个新功能。有两处我会认真考虑拆出去：
如果把这个 PR 砍掉 80% —— 只留工具输出快照修复、页脚/状态行前缀、Ctrl+C 退出警告、Shift+Tab 顶层绑定 —— 渲染器是否已经可用于 Phase 2 验证？大概已经很接近了。这是主张拆分的理由；它是一个问题，不是拦截项。 风险 —— Stage 1e 的高风险路径没有命中（
进入代码审查 🔍 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Verdict: No blocking findings on my side (self-authored, so posting as a comment, not an approval).
What I ran
- Tree at
5ed55b0e,packages/core/distbuilt in-tree;env -u QWEN_RUNTIME_DIR npx vitest run src/ui/opentui src/ui/utils/suggestions.test.ts src/ui/model/streaming-model.test.ts→ 76 files / 1403 tests all green, including every pin this PR adds. - Spot-checks that landed where the body claims: the tool-result precedence now funnels through one
toolResultEventfor all four call sites (live, resume, compacted, shell) so it cannot drift; the model-dialog triple-latch (committed/closeLatch/inFlight) matches ink and resets are unnecessary since the mount unmounts;adoptApprovalModereleases exactly whatselectAutoApprovalsselects (the rule existed, this wires the caller); the Windows bare-Tab fallback is gated on!key.shiftwith the completion consumers above it returning, so the missingshouldBlockTabguard is genuinely not needed; help scroll clamping kills the held-key runaway. - Frame-level e2e I cannot run here (no model auth), so the visual/pty parity conclusions ride on the PR's own harness data (36 legs / 96 frames). CI legs are still in flight at the time of writing.
Re: @qqqys's review — baseline mismatch, none of the four findings exist on the current head
That review states its baseline as 0091971e, a head the 06:40Z force-push replaced with 5ed55b0e. Checked each finding against the current head:
- R1-1 (update check freezes the renderer at import): the cited chain does not exist here.
MCPHealthPill.tsx:24is a plainif; the file imports only react/ink/theme/useMCPHealth.checkForUpdatesOnStartupandnotification-state.tsappear nowhere in the tree. A module-scope-invocation scan overFooter.tsx,MCPHealthPill.tsx,useMCPHealth.ts,updateCheck.ts,handleAutoUpdate.tsreturns zero hits — there is no import-time update check on this head to freeze anything. - R1-2 (Ctrl+C dead while a dialog is up): fixed, in exactly the shape proposed —
start-opentui-ui.tsx:213-236runs the inkhandleExitcascade (parked confirmation settles first as Cancel, then turn interrupt, then the two-press exit window with the hint on the bottom bar). - R1-3 / R1-4 (quota / tips substring matching):
allocation quotano longer appears anywhere incli/src,tipRegistry.tshas notext.includes, and neither file is touched by this PR's 55 files. The concern is inherited frommain, not introduced here.
Author: worth replying to that review to confirm which findings the squash folded in, so the next retest does not run against a dead commit.
Two non-blocking notes
- Esc auto-repeat can skip a gated server (pre-existing, both renderers).
useMcpApproval.handleMcpApprovalSelectis async and callssetQueue((q) => q.slice(1))only after awaitingapprovals.setState(...), whilecurrentcomes from the closure'squeue[0]. Two Escape keydowns before the first await settles both act on server A, and the twoslice(1)updates drop B with no decision recorded — B is silently absent for the session (not connected, not persisted, re-asked next startup). Ink'sMCPServerApprovalDialog.tsx:42-43wires Escape to the same handler identically, so this PR only extends exposure to the new renderer; parity holds. Suggested fix is hook-side (reject re-entry, or drop bycurrent.namerather than by position). - Stale PR description: it says the worktree isolation change was moved here from #11613, but no worktree files are in the 55-file diff at this head — a casualty of the squashes. Given the cross-baseline confusion already happened once today, I'd update the body (and mention it in the commit).
Disclosure
review-execution was not mounted for this run (no live model credentials): no pty/frame assertions from me. The release path is unit-covered only — the design doc itself flags that at Decision 17; I have no negative-control scenario to add beyond that note.
Overall: the risky parts (mode-switch release, MCP approval priority over the tool confirmation, structured-payload precedence) are the ones I expected to find problems in, and they check out; 1403 tests green.
Code reviewI wrote down what I'd do before reading the diff: fix each defect at its own site, and reuse ink's policy and precedence rather than re-deriving them. The PR matches that and beats it in one respect worth calling out — instead of patching each site it removed the duplication that let the sites drift. Three separate copies of the diff → todos → ANSI precedence became one No critical blockers found. What follows is what I checked, because the useful part of this review is which of the plausible failure modes turn out to be real. The tool-output snapshot change. All four producers were updated consistently — The one-line core change carries the most consequential fix in the PR, and it is easy to read straight past under a "parity" title. The confirmation-dialog narrowing does not crash. The MCP dialog adds a view, not a policy. Approval-mode release. Reuses The Shift+Tab split is faithful. Ink's The help-overlay key removals are parity, not a regression. Dropping The thinking row is character-exact against ink. Removed exports are all clean. I also had the arithmetic and symbol-resolution cross-checked independently against the main checkout and the ink counterparts; that pass reached the same conclusion on all thirteen files it covered and confirmed Non-blocking observations
Two things I raised and then checked rather than leaving as questions. The footer status line now wraps to two rows instead of truncating to one, replacing a comment that cited #8667/#8666 about the footer growing mid-turn; those turned out to be about unbounded growth (the merged #8667 adds a persistent queued-count badge; #8666 is the open bug it works around), the two-row bound matches ink's The central change, as a flowsequenceDiagram
participant P1 as Scheduler
participant P2 as shell-mode (producer)
participant P3 as tool-output event
participant P4 as foldLiveEvent
participant P5 as Tool card
participant P6 as LLM history
P1->>P2: progress event carrying the whole accumulated display
P2->>P3: output is the cumulative snapshot (not an increment)
P3->>P4: the card replaces what it shows
P4->>P5: rendered output
P2->>P6: the same finalOutput string
Note over P5,P6: one string, two consumers, so they cannot drift
Files changed — production sources (30 of 55 shown; the other 25 are 23 test files and 2 design docs)
Testing evidenceThis is an unattended CI run, so I did not build or execute anything from this PR — the evidence below is the PR's own CI, read through the API for the reviewed commit. There are no failures to excerpt: every completed check is green or skipped. The two checks that matter most for this PR's specific claim are green —
Not verified by me, and not verifiable from the diff or from a green suite: the claims the author themselves flags as having no frame evidence. A passing suite proves the tests pass, not that the untested behaviour holds — and for several of these no test would fail if the behaviour were wrong. Sandboxed verification would settle this: 中文说明代码审查。 我在读 diff 之前先写下了自己的方案：在每个缺陷各自的位置修复，并复用 ink 的策略与优先级判断而不是重新推导。这个 PR 达到了这个标准，并且在一点上做得更好 —— 它没有逐点打补丁，而是删掉了导致各处漂移的重复代码。三份各自独立的 diff → todos → ANSI 优先级判断合并为一个 未发现严重阻塞项。 下面列出我实际核对的内容，因为这次审查真正有价值的是判断哪些看起来可能出问题的地方确实出了问题。 工具输出快照改动。 四个生产方全部一致更新 —— 这一行 core 改动承载了本 PR 最要紧的修复，而且在「parity」这个标题下很容易被直接读过去。 确认对话框的类型收窄不会崩溃。 MCP 对话框新增的是视图，不是策略。 审批模式释放。 复用了 Shift+Tab 的拆分是忠实的。 ink 的 help 浮层按键的移除是 parity，不是退化。 去掉 思考行与 ink 逐字符一致。 被删除的导出都是干净的。 算术与符号解析这部分我还另做了一次独立交叉核对（对照 main 检出与 ink 的对应实现），结论与我一致，覆盖的十三个文件全部干净，并确认 非阻塞观察：
有两处我先提出、然后去查证而没有留作疑问。页脚状态行现在会折成两行而不是截断成一行，替换掉的那条注释曾引用 #8667/#8666 说明页脚不得在回合中途长高；查证结果是那两个 issue 关心的是无上限增长（已合并的 #8667 增加了一个常驻的排队计数徽标，#8666 是它绕开的那个未关闭 bug），两行这个上限与 ink 的 测试证据。 这是一次无人值守的 CI 运行，所以我没有构建或执行本 PR 的任何代码 —— 下面的证据是 PR 自己的 CI，通过 API 针对被审查的提交读取。没有失败可摘录：所有已完成的检查不是绿就是跳过。 对本 PR 具体主张而言最重要的两个检查是绿的 —— 我未验证、且无法从 diff 或绿色套件验证的部分： 作者自己标明没有帧证据的那些主张。套件通过证明的是测试通过，而不是未被测试的行为成立 —— 而这其中有几项即使行为是错的也不会有任何测试失败。 沙箱化验证可以定这件事： — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
|
Confidence: 3/5 — clean review, capped by policy rather than by doubt: a 1,755-production-line PR that reaches into Going back to the proposal I wrote before opening the diff — fix each defect at its own site, reuse ink's policy rather than re-deriving it — the PR does that and then goes one better, by deleting the duplicated precedence tables and the copied approval-mode array that let the two renderers drift in the first place. That is the part I'd thank the author for in six months. Most of these defects were not wrong logic in one place; they were two places holding the same fact, and the fix is that there is now one. I looked hard for the failure modes this shape of PR usually carries and could not find one that blocks. The four producers of the renamed The core re-export deserves a sentence of its own, because it is one line and it is the most valuable change here. Under this renderer the shell AST parser was being initialised after So why 3/5 and not 4. The cap is the two-tier core gate, and I want to be precise about what triggered it. The core footprint is a single line; the escalation fires on total production size, not on core risk. That is the rule as written and I am applying it, but a maintainer reading this should know the size is entirely in the renderer and the core change is the safest line in the diff. Beyond the cap there are two things I genuinely cannot settle from here, and they are the reason this is a defer rather than a rubber stamp:
One specific correction to the record, because it contradicts a claim in the author's self-review. That review says the help scroll clamping "kills the held-key runaway". It does at 76 columns and wider, which is where anyone will normally look. Below that the mount computes its clamp from lines wrapped at For the avoidance of doubt about what counts as evidence here: the author's own comment reports 76 files / 1,403 tests green locally. That is the author's claim and I have not treated it as evidence. What I relied on is the PR's CI, which is independent and green on the legs that matter — @qqqys — deferring to you rather than approving. You are the most recent human reviewer and the owner map resolves no area for this PR (it carries no labels), so you are who the deterministic resolver lands on. To be explicit about what I am asking you for: not a second opinion on correctness, which I am reasonably confident about, but the two calls I cannot make — whether the unverified behavioural surface above needs a 中文说明信心度：3/5 —— 审查是干净的，压到 3 分是规则所致而非我存疑：一个 1,755 行生产逻辑、且触及 回到我在打开 diff 之前写下的方案 —— 在每个缺陷各自的位置修复、复用 ink 的策略而不是重新推导 —— 这个 PR 做到了，而且还更进一步：它删掉了那些让两个渲染器互相漂移的重复优先级表和被复制的审批模式数组。这是六个月后我会感谢作者的地方。这些缺陷大多不是某一处逻辑写错了，而是两个地方各存着同一个事实；修复的本质是现在只剩一个。 我认真去找这类形状的 PR 通常会带的失败模式，没找到能构成阻塞的。改名后的 那一行 core 重新导出值得单独说一句，因为它只有一行，却是这里最有价值的改动。在该渲染器下，shell AST 解析器原本是在 那么为什么是 3/5 而不是 4。 压分来自两层 core 闸门，我想说清楚触发它的究竟是什么。core 的足迹只有一行；升级是按生产代码总量触发的，不是按 core 风险。规则就是这么写的，我照它执行 —— 但读到这里的人应该知道，体量全在渲染器里，而那处 core 改动是整个 diff 中最安全的一行。 除了这条压分之外，还有两件我在这里确实无法定论的事，它们才是这次「转交」而不是「走过场批准」的原因：
有一处需要更正记录，因为它与作者自评中的说法相左。那份自评说 help 滚动的钳制「消除了按住按键失控」。在 76 列及以上确实如此，而那也正是平时会去看的地方。低于 76 列时，挂载点按 为避免「什么才算证据」这件事含混：作者自己的评论报告了本地 76 个文件 / 1,403 个测试全绿。那是作者的主张，我没有把它当作证据。我依据的是 PR 自己的 CI —— 它是独立的，并且在要紧的分支上是绿的： @qqqys —— 我转交给你，而不是批准。你是最近一位人类审查者，而 owner 映射对本 PR 解析不出任何领域（它没有标签），所以确定性解析器落到的就是你。我想明确我在请你做什么：不是请你对正确性给第二意见 —— 那一点我比较有把握 —— 而是那两个我做不了的决定：上面那批未被验证的行为面，在落地之前是否需要跑一次 — Qwen Code · qwen3.8-max-2026-09-02 Reviewed at |
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
APPROVE — no Critical found. I read the surfaces @qqqys escalated on and can retire Gates 1, 2 and 3 at head 5ed55b0e. Gate 1 in particular I traced branch by branch rather than skimmed, since it is the one place in this diff where an error would be silent and security-relevant.
Gate 1 — the approval rework is fail-closed on every non-affirmative path
Certified against the five branches named in the escalating review:
1. The default: arm cannot ship unhandled. dialogs-confirm.tsx:230-232 is const exhaustive: never = details; return exhaustive; — a compile-time exhaustiveness guard, not a runtime fallback. Adding a confirmation type to the union without a case here makes details non-never and fails typecheck, so an unhandled type cannot silently reach production. Lint & Static is green at this head, which means every current member has a case. This is stronger than a runtime return { options: [Cancel] } would have been.
2. Escape and double-settle. dialogs-confirm.tsx:537-541 settles ToolConfirmationOutcome.Cancel on escape, and settle is guarded by settledRef (:528-533), so a keystroke racing a selection cannot resolve the same call twice or upgrade a Cancel into a Proceed.
3. Empty and filtered-to-empty options. dialogs-confirm.tsx:669-673 settles via onAnswered(null) → settle(Cancel) when there are no questions or no options, from an effect rather than during render. No path where an empty list resolves to a Proceed or leaves the call unsettled.
4. hideAlwaysAllow cannot create a durable rule, and cannot shift the cursor onto one. dialogs-confirm.tsx:251-252 passes isTrustedFolder && !hideAlways as showAlwaysAllow, which suppresses only the ProceedAlwaysProject / ProceedAlwaysUser pushes (:140-154, :175-179). Cancel is pushed unconditionally afterwards (:155-158, :180-183), so the decline row survives every filter. The AUTO-fallback splice at :261-270 inserts ProceedOnceAndSwitchToDefault at cancelIndex, i.e. immediately before Cancel, so suppressing or inserting rows never moves an existing index onto a more permissive value.
This gate matches ink exactly: components/messages/ToolConfirmationMessage.tsx:256 and :383 use the same isTrustedFolder && !confirmationDetails.hideAlwaysAllow condition, and ink also pushes "Yes, allow once" first. Worth stating explicitly because it is the one behaviour that looks like a loosening: the default selected row is index 0 = ProceedOnce, so a bare Enter approves once. That is pre-existing ink behaviour (RadioSelect starts at 0) rather than something this PR introduces, and it is a one-time allow, never a persisted rule. The old deny-everything bridge was stricter than ink, not correct-by-design — matching ink here is the point of the change.
5. Gated MCP approval adds a view, not a second policy. opentui-app-shell.tsx:493 calls the renderer-agnostic useMcpApproval(config) — the same hook ink drives from AppContainer.tsx:3894 — and this PR does not modify it (git diff on packages/cli/src/ui/hooks/useMcpApproval.ts is empty). dialogs-confirm.tsx:74 imports only the PendingMcpServer type. So the queue, the hash-keyed persisted decision, the un-gating and the reconnect are ink's code verbatim; there is no new route by which a server can start unapproved. The ranking claim is literally implemented: the ternary at opentui-app-shell.tsx:934-963 tests mcpApproval.isMcpApprovalDialogOpen first, then activeToolCall, then activeModal.kind === 'shell'. While the MCP dialog holds the slot the tool confirmation is not rendered, so a waiting call is parked rather than auto-resolved.
Gate 2 — both in-passing concerns check out
The commands-dispatch.ts synchronous replay is not a pre-mount mutation. The dispatcher is constructed inside a useEffect (opentui-app-shell.tsx:595-600), so the constructor's refreshNeededListener() → host.addItem() runs post-commit, not during render. The replayedExtensionRefresh WeakSet keyed on the latch owner rather than the construction is deliberate and test-pinned (commands-dispatch.test.ts "replays a latch set before mount once, not once per dispatcher"). I confirmed the latch semantics make this safe rather than lossy: markExtensionsChanged() returns early without re-emitting once latched, so subscribing alone genuinely does drop the startup notice; and a second change after /reload-plugins clears the latch does re-emit and is caught by the live subscription.
The shell-mode.ts contract change is safe because the producer emits snapshots, not deltas. This is the PR's central claim and I verified it from the producer side rather than the reducer side. Every caller of updateOutput passes an accumulated display: core/src/tools/shell.ts:2415-2419 passes cumulativeOutput, tools/agent/agent.ts:1446 passes this.currentDisplay, tools/workflow/workflow.ts:1073 passes buildLivePhaseTreeDisplay(entry). Core's own consumer treats it as a replacement — coreToolScheduler.ts:4934-4937 assigns liveOutput: compactOutput, and the adjacent comment at :4922-4924 describes exactly that as "the accumulated command output". So output: (replace) at live-session.ts:855-856 and shell-mode.ts:120 is correct and the previous delta: was the actual bug. The heartbeat payload (shell.ts:2684) is the one non-display chunk and core already routes it around liveOutput.
Also cleared: normalizeDescription has exactly one importer and the co-located test moved with it; initShellAstParser resolves to shellAstParser.ts:587 with no barrel collision.
One thing neither review flagged, in the PR's favour. The MESSAGE_ICON → shared ICON dedup in messages.tsx is not byte-identical — the shared constants append U+FE0E (constants.ts:36-45). That is the intended fix, not a regression: per constants.ts:31-35, VS15 is zero-width under string-width but forces narrow presentation so CJK terminals stop measuring East-Asian-Width-Ambiguous glyphs as 2 columns against ink's 1. The deleted local copies were the divergence. TOOL_STATUS is value-identical. Separately, the eager initShellAstParser() warm-up (start-opentui-ui.tsx:343-345) is idempotent (shellAstParser.ts:588), hang-proofed by the parserInitFailed short-circuit (:589-593), and .catch()-wrapped so failure only logs — and it prevents web-tree-sitter's UMD wrapper from latching a permanent failure that would silently downgrade permission rules and command-safety classification for the whole session.
Gate 3 — retired by CI
All substantive lanes are green at 5ed55b0e: Lint & Static, Test (ubuntu-latest, Node 22.x), Integration Tests (no-AK, No Sandbox), OpenTUI no-flicker gate and TUI parity snapshots (ink vs opentui). That last one is the meaningful gate for a parity sweep, and it covers the presentational surface I did not read line by line (footer geometry, transcript margins, popup widths).
Gate 4 — scope
1334 production lines excluding tests and docs, fix-typed so the large-scope hard block does not engage; the 1000+ advisory applies and this review is the awareness it asks for. The cross-package surface is one additive line in the core barrel with zero pre-existing consumers, so "name every downstream consumer" is satisfied trivially — an aliased named export can only break on collision, and there is none.
Non-blocking
commands-dispatch.ts:196— thereplayedExtensionRefreshWeakSet is never cleared, so if the latch is cleared by/reload-plugins, re-set by a later disk change, and the shell then rebuilds the dispatcher, the rebuilt instance will not re-announce. The live subscription already fired at re-set time so the user still sees one notice; only a rebuild-after-notice loses it, and the transcript may or may not survive that rebuild. Dropping the state from the WeakSet onExtensionsReloadedwould make the replay track the latch's generation rather than its identity. INFO-toast impact only — not worth holding the PR.opentui-app-shell.tsx:934-935— ifisMcpApprovalDialogOpenwere ever true with a nullcurrentMcpApproval, the chain falls through to the tool confirmation. The hook should make that unreachable; the fall-through direction shows a dialog rather than hanging, so it is benign either way.
The evidence bar on this PR is unusually good — a scripted pty harness driving both renderers through the same sessions, with the places that have no frame evidence named as such in the design doc instead of glossed. Two residuals are recorded there as unmatched rather than claimed fixed, which is the right call.
|
Follow-up to my approval — one real finding I traced after filing, plus one I checked and cleared. Neither is a merge blocker for me (my APPROVE stands); posting so it is on record and can be fixed here or in a follow-up. The fold never clears a structured payload, so an ANSI-then-binary tool freezes its card
const next: LiveToolItem = { ...t, output: ev.type === 'tool-output' ? ev.output : ev.display };
if (ev.type === 'tool-result' && ev.diff) next.diff = ev.diff;
if (ev.type === 'tool-result' && ev.todos) next.todos = ev.todos;
if (ev.type === 'tool-result' && ev.ansi) next.ansi = ev.ansi;A later This is reachable from the shell tool. In On the OpenTUI side that becomes a Before this PR the live path only special-cased Cheapest fix is to clear the structured fields whenever an event carries plain text — in the Checked and cleared: an empty
|
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Approved at head 5ed55b0e.
Required CI is green at this commit — Test (ubuntu-latest, Node 22.x), Lint & Static, Integration Tests (no-AK, No Sandbox), web-shell E2E Smoke, TUI parity snapshots (ink vs opentui) and the OpenTUI no-flicker gate all completed successfully, as did both Desktop Shell lanes; only review-pr and triage (the reviewer pipelines) are still running. That retires the "nothing has certified the build at this head" gate from the escalation review — which matters more here than usual precisely because that review flagged a cross-module field rename and a moved exported symbol as the two shapes typecheck exists to catch.
Answering the escalation's Gate 1 directly — the new approval surface is fail-closed by construction, and I read it branch by branch:
- There is exactly one call site of
confirmationDetails.onConfirmindialogs-confirm.tsx:settle()at :528-535, latched bysettledRefso a double keystroke cannot resolve twice. - Decline paths all resolve
Cancel, not a proceed: Esc at :539-541 with the comment tying it to the renderedNo (esc)row;ask_user_questionwithanswers === nullat :555; the standalone question/info flows at :746-747 and :812; and the new gated-MCP-server dialog mapping Esc toMcpApprovalChoice.REJECTat :885-887, which is ink's escape-to-deny convention for that dialog. - The
default:arm of the per-type switch is not a runtime fallback at all — it isconst exhaustive: never = details; return exhaustive;(:230-233, again at :457), so a confirmation type added later fails the build instead of silently rendering an allow list. The greenLint & Staticlane is what makes that a guarantee rather than an intention. hideAlwaysAllowand an untrusted folder cannot create a durable rule:showAlwaysAllowis computed asisTrustedFolder && !hideAlways(:251-253) and only ever removes the always-allow rows; it cannot add one.- The lists can never be empty and cannot shift onto a proceed value: every per-type branch pushes
Cancellast (:157, :182, :226), andOutcomeSelectcarriesoption.valuethrough toonChoose(:479-486) rather than resolving a selection by position, so a filtered list cannot make row N mean row N+1's outcome. - The AUTO-fallback insertion at :258-271 splices
ProceedOnceAndSwitchToDefaultbefore Cancel, which is the one place position would matter — and it is a proceed-with-mode-change, not a silent persist.
On the repository's core-infrastructure rule (the escalation's Gate 4): the entire packages/core surface of this diff is one aliased line, index.ts:721 export { initParser as initShellAstParser } from './utils/shellAstParser.js'. Its only production consumer is start-opentui-ui.tsx (import at :45, awaited at :343 behind a .catch), so a failed shell-parser warm-up cannot fail startup, and the name is unique in the barrel so it cannot collide with the surrounding export * lines.
The two structural moves check out independently: the tool-output union now declares output: string and all four producers and consumers use it (streaming-model.ts:34/190, live-session-model.ts:316/325, live-session.ts:856, shell-mode.ts:120, transcript-adapter.ts:136), with delta remaining only on the text/thinking events whose append semantics are unchanged; and normalizeDescription now lives in utils/suggestions.ts with SuggestionsDisplay.tsx importing it and both renderers' consumers (input-prompt.tsx) resolving through it.
No new Critical found. Three honest limits and two carried notes:
- I did not read the whole ~1300 lines of production logic. I read the security-bearing approval/mount surface, the two cross-module moves, and the event-contract change; the rest is per-component view parity, which is guarded by the two green renderer-specific gates named above rather than by my reading.
- I could not run the CLI renderer suites here:
packages/clitests fail to collect in this checkout on an unresolvable@modelcontextprotocol/clientbehind the core barrel. So my test evidence is CI's greenTest/parity/no-flicker lanes plus the two prior local runs reported on this PR (76 files / 1403 tests), not a run of mine. - The notes already on the PR still stand and are worth acting on: Escape auto-repeat can drop a queued gated MCP server with no decision recorded (async
handleMcpApprovalSelectslices by position after an await — pre-existing and identical in ink, so parity holds and this PR only widens exposure; the fix belongs hook-side, rejecting re-entry or dropping bycurrent.name), and the description still claims a worktree-isolation change carried over from #11613 that is not in the 55-file diff at this head.
|
Thanks for tracing these after filing — the first one was real and I have fixed it. Since this PR is merged, the fix landed on a follow-up branch as 1. The fold never clears a structured payload — fixedRather than clearing in the two branches you named, the fold now derives const structured = ev.type === 'tool-result' ? ev : undefined;
const next: LiveToolItem = {
...t,
output: ev.type === 'tool-output' ? ev.output : ev.display,
diff: structured?.diff,
todos: structured?.todos,
ansi: structured?.ansi,
};Before writing it I checked the two things that decide whether this shape is safe. First, every producer of these events picks the structured form or the flattened form and never emits both for one result — the live chunk mapper, Fold test added as you suggested: One deliberate omission: 2. Empty
|
6 participants
What this PR does
Closes the gap between the OpenTUI renderer and the existing ink renderer across the transcript, the composer, the footer, the dialogs and the keyboard, and adds the one startup approval this renderer was missing entirely. Every defect here was found by driving both renderers through the same scripted terminal sessions and comparing the captured frames line by line, so the list reflects what a user actually sees rather than what the code appears to do.
The largest correction is to how a running tool reports its output. The scheduler hands the shell the whole accumulated display on every progress event, so a card that treated each event as an increment repainted what it already had and grew the text on every tick. Tool output is now modelled as the snapshot it actually is: the card replaces what it shows, the throttle that avoids a redundant repaint compares lengths rather than trailing text, and the string written into the model's history is the same string the card shows, so the two cannot drift apart. The tail-deduplication this made unnecessary is gone.
A tool result that carries a structured payload is now recognised by one precedence shared by every path that can produce one, so a to-do list renders as a to-do list instead of as the raw JSON dump it was falling back to, and a diff or an ANSI payload keeps its own rendering. Before this the live path and the settled path disagreed about which shape a result had, and the live path lost.
Switching the approval mode to one that would not have parked a waiting confirmation now releases the confirmations it would not have parked in the first place, instead of leaving them on screen asking for a decision the new mode has already taken. The release set is the same set ink computes, including the restriction of the edit-mode release to editing tools.
A committed thinking row now names how long the model thought, using the same one-second threshold below which ink says the thought was brief rather than reporting a duration, and the key its own hint advertises for expanding thinking is bound, so the hint is not offering a keystroke that does nothing.
A project that declares gated MCP servers now asks for approval before starting them. This renderer had no such dialog at all, so a server declared in the project's own configuration was never offered and simply stayed disconnected, with nothing on screen to explain why. The queue, the per-server summary, the persisted decision and the approve-all path reuse the policy ink already uses; only the view is new, and it is ranked above both the shell and the tool confirmation, where ink ranks it.
The transcript and the composer now sit inside the horizontal margins ink uses. That single change is what makes the completion dropdown, the attachment list and the message columns land where ink puts them. The dropdown's own internal arithmetic is untouched, because it was already computing the right widths from a box that was two columns too narrow on each side.
Status rows gained the prefixes ink draws. The warning row had none at all, so a wrapped warning lost its marker and overflowed the row by a column. The error row now puts its retry hint on the same line instead of a second one, matching ink, with the prefix pinned so a wrapped body cannot squeeze it away.
The footer's loading indicator moved to ink's indent, its second row joins segments the way ink does, its token estimate is the real one rather than a placeholder, and an armed quit warning now renders even while a dialog, a confirmation or a dropdown has the rest of the footer hidden. Before this, the gate that hid the footer also hid the warning, so a first Ctrl+C with a dialog open produced no feedback whatsoever and the user had no way to learn that a second press would exit.
Shift+Tab now cycles the approval mode from the top level rather than from the composer, so it keeps working while a dialog or a confirmation has the composer unmounted — which is exactly the state the on-screen hint advertises it in. The Windows fallback that also accepts a bare Tab, for terminals that cannot tell the two apart, is narrowed to a bare Tab only, so a terminal that can tell them apart no longer advances the mode twice for one keystroke.
The context-files announcement is latched with the predicate ink uses and re-armed when the transcript resets, so starting a new session inside an already-running renderer announces its context files again instead of staying silent for the rest of the process.
Extension refresh notices that latched before this renderer mounted are replayed on mount, so the one notice telling the user to reload their plugins is no longer dropped by a subscription that started too late. The replay is keyed on the owner of the latch rather than on the construction of the dispatcher, because the latch outlives it.
The model dialog carries the three guards ink has. A successful pick no longer also announces the model that survived, and neither a second Escape nor a second Enter landing while an apply is still in flight can announce a second time or start a second switch.
Dialog selection markers use ink's chevron rather than a filled bullet, and the authentication dialog takes the shared icon glyphs, which also brings the text-presentation selector those icons carry. Transcript items take the per-type top margin ink gives them, so a run of items of the same type no longer reads as one block. The confirmation dialog offers the per-type approval options ink offers, and the shell parser is warmed before first paint so the initial prompt is not charged for it.
Adds a bilingual design document recording each decision, the coverage boundary and the follow-ups.
Why it's needed
The OpenTUI renderer is reachable today, but switching to it gave a visibly different and in places broken experience. A running shell command repainted its output on every progress tick. A to-do list rendered as a JSON dump. Pressing Ctrl+C once with a dialog open silently did nothing. Cycling to a mode that auto-approves left the confirmation on screen anyway. A new session in an already-running process never announced its context files. A project's own MCP servers were never offered for approval and never connected. The mode-cycling hint the footer advertised did not work in the dialogs where it was displayed. None of this was caught by the unit tests, because each component was individually correct — the defects were in how the pieces were composed and in assumptions about what upstream hands them.
Reviewer Test Plan
How to verify
Run the CLI with the OpenTUI renderer and walk the following; each is a behaviour that was wrong before this branch.
@-mention and check the dropdown's left edge lines up with the composer's text, and that a long mention label is not truncated to half the row.Evidence (Before & After)
Captured with a scripted pty harness that drives both renderers through 18 scenarios and writes every frame to a text grid: 36 legs, 96 frames, no leg in error. Below is the same scenario, same terminal size, same fake model, before this branch and after — the tool has just printed one line. Long absolute paths and the branch name in the footer are elided with
…; nothing else is edited.Before (a running tool card, the footer, and the status rows), from the pre-fix capture:
After (same scenario), with the ink leg underneath for reference:
The doubled tool output, the missing prefixes, the missing context-files and extension notices, the duplicated mode segment and the missing cycling hint are all gone. The error row is the same story, and now matches ink character for character:
The to-do list and the new MCP approval dialog, both against the ink leg from the same run:
The two dialogs carry the same text. The visible differences are ink clipping its own right-hand corners, because it sets a left margin on a full-width box, and this renderer indenting the wrapped continuation of one sentence by a column that ink does not.
Measured across all 48 frame pairs in the final run, 2 pairs are identical row for row and 46 differ; counting rows present in only one renderer, after normalising the version string away, the total is 359. That is narrower than the 414 the same measurement gave before this branch was rebased onto current main, though the narrowing is mostly main's own ink output moving, not this branch improving, since this branch's source did not change between the two measurements. The divergence is concentrated: five frames account for 162 of those 359 rows, and every other frame differs by 17 rows or fewer. Four of the five are confirmation and question frames that ink renders with no box around them, and together they are 128 rows; those four are the follow-up named below. The fifth is the MCP dialog described just above.
Tested on
Windows is not tested on a real terminal. The bare-Tab fallback that exists because some Windows terminals cannot distinguish Shift+Tab from Tab is covered by unit tests that pin the platform, including the case where a bare Tab was already spent on a completion and must not also cycle the mode.
Environment
npm run build && npm run bundle, then driven through a pty at 100x40 and 60x24. Typecheck clean over the whole repository, lint and format clean over the touched trees. Unit tests: 470 files, 9160 tests, of which 21 fail — all 21 fail identically on currentmainfrom a separate checkout of it, and none of the failing files is touched here. The two files involved are a cursor-rendering suite whose four failures have the same names in both trees, and an authentication-dialog suite in which 12 to 14 of 26 cases time out on both trees, with the exact subset varying between runs of the same tree; run alone, the two trees produce byte-identical failure sets.Risk & Scope
Waiting for user confirmationspinner row has no counterpart here. Not ported: ink's queued-message body and its edit hint above the composer, and the footer's background-task, MCP-health, worktree, workflow and skill-review segments. A replayed extension notice cannot carry its reason, because the latch it replays from exposes none. This renderer deliberately keeps its own wording for a skipped update check rather than adopting ink's update-failed line. On Windows a terminal that delivers Shift+Tab as a bare Tab while a dialog has the composer unmounted will not cycle; closing that without risking a double cycle on other platforms needs a guarantee about key-handler ordering that the underlying library does not give.Design document: English · 中文
Linked Issues
Part of #8662.
中文说明
这个 PR 做了什么
把 OpenTUI 渲染器与既有 ink 渲染器之间的差距，在会话记录、输入框、页脚、对话框和键盘五条线上补齐，并补上了这个渲染器完全缺失的那一道启动审批。这里的每一个缺陷都是用脚本驱动两个渲染器跑同一批终端会话、再逐行比对截取到的画面发现的，所以这份清单反映的是用户实际看到的东西，而不是代码看上去做了什么。
最大的一处修正是运行中的工具如何汇报自己的输出。调度器在每一个进度事件里交给 shell 的都是到目前为止累积的完整显示内容，所以把每个事件当成增量的卡片会把已经画过的内容再画一遍，文本随每个 tick 增长。工具输出现在按它本来的样子建模为快照：卡片替换自己显示的内容，避免重复绘制的节流改为比较长度而不是比较尾部文本，写进模型历史的字符串与卡片显示的是同一个，两者因此不可能再各自漂移。为此变得多余的一段尾部去重逻辑被删掉了。
带有结构化载荷的工具结果，现在由所有可能产生它的路径共用同一套优先级来识别，于是待办清单渲染成待办清单，而不是退回到它原先那副原始 JSON dump 的样子，diff 与 ANSI 载荷也各自保住自己的渲染。在此之前，实时路径与结算路径对结果的形状判断不一致，而实时路径输了。
把审批模式切到一个本来不会挂起等待确认的模式时，现在会释放那些它本来不会挂起的确认框，而不是让它们继续留在屏幕上、索要一个新模式已经替用户做出过的决定。释放集合与 ink 计算的是同一个，包括编辑模式只对编辑类工具释放这一条限制。
一条已提交的思考现在会报出模型思考了多久，使用 ink 同一个「一秒以下」的阈值——低于它就说「思考很短」而不是报一个时长——并且它自己提示里写的那个用于展开思考的键真的被绑定了，于是那句提示不再是在提供一个按下去什么也不会发生的按键。
声明了受门禁 MCP 服务器的项目，现在会在启动它们之前征求批准。这个渲染器原先根本没有这样一个对话框，所以项目自己配置里声明的服务器从来不会被提供出来，只是静静地保持未连接，屏幕上没有任何东西解释原因。队列、每个服务器的摘要、持久化的决定以及「全部批准」这条路径，都复用 ink 已在使用的策略；只有视图是新的，并且它排在 shell 确认与工具确认之前，与 ink 的排位一致。
会话记录与输入框现在处在 ink 使用的左右边距之内。正是这一处改动让补全下拉、附件列表和消息列落在 ink 放它们的位置。下拉自身的内部算术没有动，因为它本来就算出了正确的宽度，只是依据的那个盒子每边窄了两列。
状态行补上了 ink 会画的前缀。警告行原本一个前缀都没有，所以一条换行的警告会丢掉它的标记，并且向右溢出一列。错误行现在把它的重试提示放在同一行而不是第二行，与 ink 一致，同时把前缀钉住，使换行的正文无法把它挤掉。
页脚的加载指示器移到了 ink 的缩进，第二行按 ink 的方式拼接各段，token 估算改用真实值而不是占位值，并且已经武装的退出警告现在即使在对话框、确认框或下拉把页脚其余部分隐藏时也会渲染。在此之前，隐藏页脚的那个门禁同时把警告也隐藏了，所以在对话框打开时按第一次 Ctrl+C 完全没有任何反馈，用户无从得知再按一次就会退出。
Shift+Tab 现在从顶层而不是从输入框循环审批模式，所以在对话框或确认框把输入框卸载掉的时候它依然有效——而那恰恰是屏幕上的提示所宣称的状态。Windows 上那条同时接受裸 Tab 的兜底（有些终端分不清两者）被收窄为只接受裸 Tab，因此分得清的终端不会再为一次按键把模式推进两次。
上下文文件播报使用 ink 所用的谓词做闩锁，并在会话记录重置时重新武装，所以在已经运行着的渲染器里开启新会话会再次播报它的上下文文件，而不是在进程余下的时间里一直沉默。
在本渲染器挂载之前就已经闩锁上的扩展刷新提示，会在挂载时回放，于是那一条告诉用户去重新加载插件的提示不再因为订阅起得太晚而被丢掉。回放以闩锁的持有者为键，而不是以分发器的构造为键，因为闩锁比它活得久。
model 对话框带上了 ink 有的三道守卫。一次成功的选取不再顺带播报那个存活下来的模型，第二次 Escape、或者在一次 apply 仍在进行中时落下的第二次 Enter，都不会再播报第二次或发起第二次切换。
对话框的选择标记使用 ink 的尖角符而不是实心圆点，认证对话框改用共享的图标字形，这也带上了那些图标所携带的文本呈现选择符。会话条目取得 ink 给它们的按类型上边距，于是连续几条同类型条目不再读作一整块。确认框给出 ink 会给的按类型批准选项，shell 解析器在首帧绘制之前预热，首次提示因此不必为它付费。
新增一份双语设计文档，记录每一个决定、覆盖边界与后续工作。
为什么需要
OpenTUI 渲染器今天是可达的，但切过去会得到一个明显不同、且有些地方是坏的体验。一个运行中的 shell 命令会在每个进度 tick 上重画它的输出。一份待办清单渲染成 JSON dump。在对话框打开时按一次 Ctrl+C 会悄无声息地什么也不做。循环到一个会自动批准的模式，确认框却照样留在屏幕上。已经在运行的进程里开一个新会话，永远不会播报它的上下文文件。项目自己的 MCP 服务器从来不会被提供审批，也从来不连接。页脚所宣称的模式循环提示，恰恰在它显示出来的那些对话框里不起作用。这些都没有被单元测试抓到，因为每个组件单独看都是正确的——缺陷出在这些部件如何组合，以及对上游交给它们的东西所作的假设上。
评审测试计划
如何验证
用 OpenTUI 渲染器运行 CLI，走一遍下面这些；每一条在本分支之前都是错的。
@提及，检查下拉的左边缘与输入框文本对齐，并且一个很长的提及标签不会被截断到半行。证据（前后对比）
用一个脚本化的 pty harness 采集，它驱动两个渲染器跑 18 个场景，并把每一帧写成文本网格：36 条腿、96 帧，没有任何一条腿报错。下面是同一个场景、同一终端尺寸、同一 fake model，在本分支之前与之后的样子——此时工具刚打印出一行。很长的绝对路径与页脚里的分支名用
…省略，此外未作任何编辑。之前（运行中的工具卡片、页脚，以及状态行），取自修复前的采集：
之后（同一场景），下面附上 ink 那一腿作为参照：
重复的工具输出、缺失的前缀、缺失的上下文文件与扩展提示、重复的模式段以及缺失的循环提示，全部消失了。错误行是同一个故事，并且现在与 ink 逐字一致：
待办清单与新增的 MCP 审批对话框，都与同一次采集里的 ink 腿对照：
两个对话框承载的文本相同。看得见的差异是 ink 裁掉了自己右侧的两个角（因为它在一个整宽的盒子上设了左边距），以及本渲染器把某一个句子的换行续行多缩进了一列，而 ink 没有。
在最终那次采集的全部 48 对帧上度量，2 对逐行相同、46 对不同；只统计出现在单一渲染器里的行，并把版本号归一化掉之后，总数是 359。这比本分支 rebase 到当前 main 之前同一度量给出的 414 要窄，但这个收窄主要来自 main 自己的 ink 输出动了，而不是本分支变好了——因为两次度量之间本分支的源码没有变化。分歧是集中的：五帧占了这 359 行里的 162 行，其余每一帧的差异都不超过 17 行。这五帧里有四帧是 ink 不套框渲染的确认框与提问框帧，合起来 128 行，正是下面点名的后续工作；第五帧是上文刚描述的 MCP 对话框。
测试环境
Windows 没有在真实终端上测试。那条因为部分 Windows 终端分不清 Shift+Tab 与 Tab 而存在的裸 Tab 兜底，由钉住平台的单元测试覆盖，其中包括「裸 Tab 已经被一次补全用掉、因此不得同时循环模式」这一种情形。
环境
npm run build && npm run bundle，随后在 100x40 与 60x24 的 pty 下驱动。typecheck 在全仓干净，lint 与格式在所触及的树上干净。单元测试：470 个文件、9160 个用例，其中 21 个失败——这 21 个在从当前main单独检出的一份代码上以同样方式失败，而失败的文件没有一个被本 PR 触及。涉及的是两个文件：一个光标渲染套件，它的 4 个失败在两棵树里名字相同；以及一个认证对话框套件，两棵树上 26 个用例里都有 12 到 14 个超时，而确切的子集在同一棵树的两次运行之间也会变；单独运行时，两棵树产出的失败集合逐字相同。风险与范围
Waiting for user confirmation转轮行在这里没有对应物。未移植：ink 的排队消息正文与它在输入框上方的编辑提示，以及页脚的后台任务、MCP 健康、worktree、workflow 与 skill 评审各段。回放的扩展提示带不上它的原因，因为它据以回放的闩锁没有暴露原因。本渲染器刻意保留自己那条「更新检查已跳过」的措辞，而不采用 ink 的更新失败那一行。在 Windows 上，一个把 Shift+Tab 送成裸 Tab 的终端，在对话框已把输入框卸载时不会循环；要在不让其他平台冒双重循环风险的前提下补上这一点，需要底层库给出关于按键处理顺序的保证，而它没有。设计文档：English · 中文
关联 Issue
属于 #8662 的一部分。