feat(cli): OpenTUI migration batch 4 — dialogs, commands, and session-rewind - #10383
Conversation
… dialogs scaffolding Foundation batch of the OpenTUI migration tracked in #8662. Adds the renderer-neutral foundation modules under ui/opentui: theme family, a11y (plain-text, screen-reader), clipboard, key-map, mouse hit/caret, link-click + osc8 parity, early-input, exit guard/lifecycle, kitty negotiation, event-adapter, item-projection, slash dispatch (+ command parsing), commands context/output, help content, input history, and the dialog scaffolding primitives (core/shared) with the theme dialog. Two helpers land inside ui/opentui rather than utils/ to respect the utils leaf-layer rule (#9737). Stacked on the infra batch: consumes ui/model streaming model and @OpenTui deps. No reachable ink code changes beyond a one-line export addition in the shared osc8 module.
Upstream renamed setGeminiMdFileCount to setMemoryFileCount in the command UI contract; the rebase onto main surfaced the mismatch at build. Rename the host interface member, the bridge wiring, the dispatch stub, and the test mock to match.
Third landing batch of the OpenTUI migration (#8662): live-session stream fold and model, message rendering (markdown heal, MCP progressive, client tool runs, text batching), transcript adapter with resume/session-switch, sticky todos, the composer (input-prompt view/key/model), mouse rows and scrollbar, unified-diff rendering, and session-compaction notice. All additive — no reachable ink code path is touched, ink remains the default. Carries the first consumer of the remend dependency deferred from the infra batch, placed in devDependencies per the renderer-deps convention. The stacked-skill completion helpers import from the relocated ui/commands module following the upstream rename.
…-rewind Adds the dialog layer and command-routing infrastructure for the OpenTUI renderer: 19 dialog modules (auth, extensions, MCP, memory-status, misc, model family, modes, permissions, settings, stats/skills, help overlay, arena host, folder-trust gate), the commands registry with slash-to-dialog routing, the commands dispatcher (action interpreter connecting the slash gateway to the session and dialog layer), and the session-rewind viewer with its history-folding model. Also exports `isUserTextContent` from historyMapping so the rewind model can classify user turns without duplicating the predicate. Everything is additive: no reachable ink code path is touched, the default renderer stays ink, and the dep-direction gate passes. Stacked on the live-session batch. 56 test files / 886 tests, all green.
Round-2 review fixes (17 Critical + 10 Suggestion resolved in code):
- dialogs-shared: move number-select flush out of the setState updater
(StrictMode double-fires onSelect); split setActiveIndex (ink
SET_ACTIVE_INDEX, lands on any in-range row) from highlightIndex
(arrow keys skip disabled rows) so wheel navigation never sticks
- event-adapter: chat_compressed notice mirrors ink formatCount ('~'
prefix for estimated counts); vision_bridge_notice renders
summary\nnotice; explicit projections for task_execution /
findings_list / terminal_image keep multi-MB payloads off the
transcript; retry-countdown-clear forwards isContinuation
- slash-dispatch: isSlashCommandInput drops the '?' branch (ink gate
routes ? input to the model); executeSlashCommand races the action
against the abort signal; dialog effects carry the
OpenDialogActionReturn payload; projected added-item text surfaces
alongside non-handled effects (notice); message-shaped items project
to their text; ui.history comes from env; absent sessionStats stamp
now, not epoch; telemetry parity (recordSkillInvocation /
recordAutoSkillCommandUsage / makeSlashCommandEvent)
- item-projection: model stats render per-(model,source) sections with
N/A for unpriced entries; Tool Calls line uses ASCII x like ink;
redactProxy deduplicated via systemInfoFields export
- theme: palette/syntax colors resolve through color-utils toHex before
parseColor (ink CSS names / *bright names no longer degrade to
magenta); unresolvable values stay unset
- key-map: kitty 'kpenter' normalizes to 'return'; resolveCommands
exposes ink's key fan-out (Ctrl+C fires QUIT + CLEAR_INPUT)
- a11y: hardWrap delegates to wrap-ansi (word-boundary parity with
ink's screen-reader path); markdown reducer tracks fence length,
keeps fence-like lines literal inside fences and inner backticks in
multi-backtick spans; stripAnsi delegates to strip-ansi plus a
private-parameter CSI pass (SGR mouse, DEC save/restore)
- clipboard: OSC 52 write gated on a TTY (stderr preferred), tests spy
the stream instead of writing real sequences to the runner's terminal
- exit-guard: independent per-key arm windows like ink
- dialogs-theme: diff preview pane receives syntaxStyle/filetype
qwen-code-ci-bot
left a comment
qwen-code-ci-bot
left a comment
There was a problem hiding this comment.
@chiga0 Thanks for the fourth batch. This one stops at the template gate — the body is missing three sections the PR template asks for:
- Tested on — the OS table. The test-suite claim (56 files / 886 tests) has no environment annotation, and CI here runs on macOS, Windows, and Linux.
- Risk & Scope — main risk or tradeoff, what is not validated / out of scope, breaking changes. The prose in "What this PR does" covers part of it ("everything is additive…"), but the sibling batches (#10134, #10146, #10368) all carry this section explicitly — for a ~16.5k-line batch it is the section reviewers reach for first.
- Linked Issues — #8662 and #10368 are referenced in prose, but the formal section (without a closing keyword, since the tracking issue stays open) is what lets reviewers and gates follow the stack.
The template's bilingual <details> block is also expected — the current Chinese paragraph is a condensed summary rather than the per-section translation the template asks for.
Fill those in (no code changes needed) and re-trigger with @qwen-code /triage; the gate picks it up from there.
中文说明
@chiga0 感谢提交第四批。本次在模板门禁处停下——PR 正文缺少 PR 模板 要求的三个小节：
- Tested on——操作系统表格。测试套件的声明（56 文件 / 886 用例）没有环境标注，而 CI 会在 macOS、Windows、Linux 上运行。
- Risk & Scope——主要风险或权衡、未验证/超出范围的部分、破坏性变更。"What this PR does" 的正文覆盖了一部分（"全部为增量……"），但同系列批次（#10134、#10146、#10368）都显式包含该小节——对一个约 1.65 万行的批次，它是评审者最先看的小节。
- Linked Issues——#8662 和 #10368 目前只在正文中提及，正式小节（因跟踪 issue 保持开启，不使用 closing keyword）才能让评审者和门禁跟踪这个 stacked 依赖链。
此外，模板要求双语 <details> 块——目前的中文段落是浓缩摘要，而非模板要求的逐节对应翻译。
补齐以上小节（无需改代码）后，用 @qwen-code /triage 重新触发，门禁会从模板检查继续。
— Qwen Code · qwen3.8-max
…review - kitty-negotiation: KITTY_REPLY_RE requires at least one flag digit (\d+), so an echoed bare query \x1b[?u in PTY/CI environments no longer resolves true and locks the renderer into kitty mode on a terminal that never answers queries; the accumulation buffer keeps only a 256-byte tail (bounded memory, bounded rescan under byte floods); the settle-window drain is removed — an EventEmitter data listener cannot consume chunks from other listeners, so late replies flow to the renderer's input parser like any other terminal noise - a11y-screen-reader: ScreenReaderOutputWriter sanitizes written content (stripAnsi + drop bare C0/C1 controls, keep newlines) so the plain-text-only contract is enforced at the writer instead of trusting every future caller — smuggled OSC 52 clipboard writes or title/cursor sequences cannot execute on the main screen
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Review — batch 4 (dialogs, commands, session-rewind)
Deep tier on the routing/contract surfaces (a parity reimplementation whose counterpart is not in the diff), scope-limited on the presentational dialog bodies. No blockers. One test-validity finding worth fixing before this lands, plus three contract/docs nits.
Disclosure first — CI provides no code evidence on this PR
The check set is authorize, label, triage, precheck-pr, publish-*, ack-review-request, resolve-pr, review-config, tmux-testing, verify — 9 of them SKIPPED, 2 pending, and not one of them builds, typechecks, lints, or tests the changed files. A rollup of green review-automation checks is an absence of evidence, not a pass. So rungs 1-2 were run locally instead:
npx vitest run src/ui/opentui/→ 56 files / 886 tests passnpx tsc --noEmit -p packages/cli/tsconfig.json→ exit 0- 18-mutation matrix across
commands-registry.ts,commands-dispatch.ts,slash-gateway.ts,session-rewind-model.ts,dialog-data.ts,folder-trust-gate.tsx→ 15 killed, 3 survived
Environment for every witness above: darwin 25.5, APFS case-insensitive, node ≥22, local worktree. Windows and Linux are unreviewed dimensions — which is the same gap the triage bot's Tested on request is pointing at.
What I verified clean (naming the side I inspected)
routeDialogToOpenTuiis genuinely exhaustive. All 33 members ofOpenDialogActionReturn['dialog']are covered, with anevercheck that makes a new kind a compile error. I compared it arm-by-arm against the ink original atui/hooks/slashCommandProcessor.ts:1190-1313— the six model-family kinds collapse to{dialog:'model', mode}with the mode ink's boolean flags imply,persistScopeis forwarded on all six, the fourarena_*modes match, and bothawaitsites correspond.sessionId/nameinterception matches ink.commands-dispatch.ts:736-744interceptsresume-with-sessionIdand allbranchresults before routing, exactly as ink does. I checked this because droppingsessionIdin the router looked like a contract asymmetry; it is not — the field is consumed upstream.- The 69-entry route table matches its source of truth. Against
services/BuiltinCommandLoader.ts:112-186: 69/69 canonical names, zero missing, zero extra. All 11altNamessets match the real command objects, and all four gates (workflows,managed-memory,folder-trust,lsp) sit on exactly the right five entries. commands-registry.test.tsis not a circular fixture. It importsloadInteractiveCommandsand compares against real command objects — proven empirically, since dropping'connect'from theauthaliases is killed.- The
isUserTextContentexport is the right predicate. I specifically checked whether the new consumer neededisRealUserTurnsemantics instead. It does not: the two take disjoint types (ContentvsHistoryItem) and sit on opposite sides of the same walk.rewindApiCutPoint(session-rewind-model.ts:49) is the API-side half and mirrorscomputeApiTruncationIndex's API-side logic — same start index, same predicate, same-1on underflow. setMemoryFileCountrename is correct — matchesui/commands/types.ts:114;setGeminiMdFileCountno longer exists anywhere in the tree.- House style: no cross-package relative imports, no
any, no non-kebab-case.tsfilenames in the new set.
Deferred, recorded rather than threaded
Three further mutation survivors, all low-value, none worth a thread:
folder-trust-gate.tsx:117-119— droppingTRUST_PARENTfrom the trusted set survives. The test atfolder-trust-gate.test.tsx:222-230selectsTRUST_PARENTbut asserts only thatsetValuewas called, never the resulting restart state. With that mutation, "trust parent folder" would spuriously relaunch the CLI.dialog-data.ts:629— swapping['user','workspace']to['workspace','user']survives. Thebreakmakes the order load-bearing for a rule present in both scopes; nothing pins which scope wins.slash-gateway.ts:70—?? false→?? truesurvives. The docblock's own "false before the dispatcher attaches" contract has zero coverage;canRunDuringStreamingis never called inslash-gateway.test.ts.
Forward-looking note on rewindApiCutPoint
It has no non-test caller yet, so nothing below is a defect at this commit. But it deliberately omits two guards that ink's computeApiTruncationIndex enforces — the findLastSuccessfulCompressionIndex early return (historyMapping.ts:153-155) and the marker-less auto-compaction fail-loud (historyMapping.ts:176-182, which returns -1 rather than truncating to [prelude, summary, ack] and dropping every real turn). Both live on the UI-history side that rewindApiCutPoint's signature does not receive. Whoever wires this up must supply them, or rewind will target the wrong turn on a compressed session.
Unreviewed dimensions
- The presentational dialog bodies (
dialogs-auth/mcp/settings/arena/permissions/extensions/stats-skills, ~5.5k lines) were read at the routing and contract level only, not line-by-line for render correctness. - No rung-3 terminal run: these components are not mounted on any reachable path in this batch, so there is no runtime to exercise.
- Windows / Linux behaviour — witness is darwin only.
- Base is
feat/tui-live-session-batch, notmain; I reviewed the batch-4 delta only.
Cross-checked against the existing qwen-code-ci-bot CHANGES_REQUESTED: it is a PR-template gate (missing Tested on, Risk & Scope, Linked Issues) with no code findings, so there was nothing to confirm or refute on the code side.
Reviewed with AI assistance.
…at/tui-live-session-batch
…ferred race The previous test asserted branchNames was populated after dispatch, but a fire-and-forget void call passed because microtasks drained before the check. Replace with a Promise.race that proves dispatch was still pending (blocked on the closed gate) before the gate was resolved — a void call resolves dispatch immediately, making the race return 'resolved' instead of the sentinel.
|
@qwen-code /triage |
|
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: 36 passed · 0 failed · 36 total Flakiness gate: 中文 — 判定：✅ 通过 · 可合入（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：36 通过 · 0 失败 · 36 总计 抖动门： Verification reportPR 10383 Deep Verification — OpenTUI migration batch 4 (dialogs, commands, session-rewind)Verdict: 中文摘要结论：
Central claim and A/B proofCentral claim (from the PR body): "Everything is additive: no reachable ink code path is touched, nothing imports these modules yet, and the default renderer stays ink." This is falsifiable: the shipped artifact is the esbuild bundle from
Residual after normalization was zero bytes — even the Witness: Control purity. FindingsNone. No blocking and no suggestion-level issues found. Two verified micro-observations, reported for completeness rather than as findings:
GatesAll scripted; witness
Vacuity / mutation matrixThree mutants on the central modules; each unmutated control green, each mutant killed by its intended test with a real assertion mismatch (not a collection error), each file restored (
M1 directly verifies the second commit's claim: the strengthened deferred-race assertion catches the fire-and-forget regression it was written for. M3 exercises the one new cross-module consumer of the exported Not covered
MethodologyEnvironment: the CI verify container ( Flakiness gate logEvidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention). — Qwen Code · sandboxed verification |
…larify gate test Two docblock comments said "67 modules" but the table has 69 entries (verified against BuiltinCommandLoader.ts). Fixed to 69. The gated-commands test comment now explains that TypeScript enforces the CommandGate type at compile time, so the runtime loop is unnecessary — the literal name list is the intentional guard for a bogus gatedBy on an ungated command.
…at/tui-live-session-batch
…feat/tui-dialogs-batch
- /theme route results include 'message': themeCommand returns a MessageActionReturn under NO_COLOR, so the declared results were incomplete - drop the unreachable 'branch' member from OpenTuiDialogRequest and make routeDialogToOpenTui throw on dialog-branch instead: /branch is a host action intercepted unconditionally by the dispatcher (ink parity), and a compile-time exclusion is not expressible because OpenDialogActionReturn is a single interface with a union dialog field — the loud throw guards against a future refactor dropping the interception; the branch route no longer advertises a dialogs entry no renderer opens - derive gate coverage from the loader instead of a hardcoded name list: the coverage test loads with every gate ON (plus the checkpointing flag the /restore factory needs) and asserts set equality between route names and registered built-ins — no escape hatches — and a new test proves every gatedBy route is genuinely absent from a gates-off load, so a bogus gate on an always-registered command fails
…ation All 15 findings from the independent static review verified in source and fixed (no false positives; none deferred): - quit effect carries QuitActionReturn.messages projected to text on a notice field — ink renders them via QuittingDisplay and the payload was permanently lost (Important #1) - error and finished branches emit retry-countdown-clear like ink's handleErrorEvent/handleFinishedEvent, so a terminal event inside the countdown window no longer leaves a stale retry row (#2) - projectContextUsage renders the compaction-threshold ladder and the per-item detail sections (tools/memory/skills, ink's sort order) when showDetails is on — /context detail transcripts no longer show strictly less than the compact view (#3) - projectMcpStatus honors showSchema (parameter JSON under each tool) and showTips, so /mcp schema is distinguishable from /mcp (#4) - SlashDispatchEnv.settings is required: the real CommandContext.services.settings is non-null and a null surfaced as a generic command failure on first .merged read (#5) - dead singleColumn flag removed from the help width layout (clamp makes it always false; ink has no single-column mode) (#6) - truncated SS3 tail (bare ESC O) is stripped like the truncated CSI tail, so a captured half F1-F4 no longer leaks 'O' into the composer (#7) - readBufferRow trims cellColumns alongside text, so URLs ending at end-of-row on a wide character hit-test on both halves of the cell (#8) - kitty probe writes guarded: a synchronous stream throw settles the probe (restores raw mode, removes the listener) instead of leaking (#9) - eraseLines reuses the ansi-escapes helper (already a repo dependency) instead of a byte-identical hand-rolled copy (#10) - truncateText passthrough wrapper dropped; callers use the exported truncateHelpText directly (#11) - SkillsList truncate keeps total length n like ink, so the description column no longer shifts by one cell when a name truncates (#12) - model_fallback names pass through sanitizeDisplayText like ink (#13) - selectIndex fires onHighlight before onSelect (ink dispatches SET_ACTIVE_INDEX then SELECT_CURRENT), keeping highlight-driven stay-open dialogs synced on mouse input (#14) - mcp_app without fallbackText renders empty instead of JSON-dumping the embedded HTML; projectAbout hides Base URL when selectedAuthType is empty, matching ink's formatBaseUrl (#15)
…at/tui-foundation-batch
…at/tui-live-session-batch
Code Coverage Summary
CLI Package - Full Text ReportCore Package - Full Text ReportFor detailed HTML reports, please see the 'coverage-reports-22.x-ubuntu-latest' artifact from the main CI run. |
* feat(cli): OpenTUI foundation modules — theme, a11y, clipboard, keys, dialogs scaffolding Foundation batch of the OpenTUI migration tracked in QwenLM#8662. Adds the renderer-neutral foundation modules under ui/opentui: theme family, a11y (plain-text, screen-reader), clipboard, key-map, mouse hit/caret, link-click + osc8 parity, early-input, exit guard/lifecycle, kitty negotiation, event-adapter, item-projection, slash dispatch (+ command parsing), commands context/output, help content, input history, and the dialog scaffolding primitives (core/shared) with the theme dialog. Two helpers land inside ui/opentui rather than utils/ to respect the utils leaf-layer rule (QwenLM#9737). Stacked on the infra batch: consumes ui/model streaming model and @OpenTui deps. No reachable ink code changes beyond a one-line export addition in the shared osc8 module. * fix(cli): import originals instead of forking slash parser and dialog scope utils * fix(cli): address R1 review findings in OpenTUI foundation modules * fix(cli): align OpenTUI command host with the memory-file-count rename Upstream renamed setGeminiMdFileCount to setMemoryFileCount in the command UI contract; the rebase onto main surfaced the mismatch at build. Rename the host interface member, the bridge wiring, the dispatch stub, and the test mock to match. * feat(cli): OpenTUI migration live-session and input batch Third landing batch of the OpenTUI migration (QwenLM#8662): live-session stream fold and model, message rendering (markdown heal, MCP progressive, client tool runs, text batching), transcript adapter with resume/session-switch, sticky todos, the composer (input-prompt view/key/model), mouse rows and scrollbar, unified-diff rendering, and session-compaction notice. All additive — no reachable ink code path is touched, ink remains the default. Carries the first consumer of the remend dependency deferred from the infra batch, placed in devDependencies per the renderer-deps convention. The stacked-skill completion helpers import from the relocated ui/commands module following the upstream rename. * fix(cli): address R2 review findings in OpenTUI foundation modules Round-2 review fixes (17 Critical + 10 Suggestion resolved in code): - dialogs-shared: move number-select flush out of the setState updater (StrictMode double-fires onSelect); split setActiveIndex (ink SET_ACTIVE_INDEX, lands on any in-range row) from highlightIndex (arrow keys skip disabled rows) so wheel navigation never sticks - event-adapter: chat_compressed notice mirrors ink formatCount ('~' prefix for estimated counts); vision_bridge_notice renders summary\nnotice; explicit projections for task_execution / findings_list / terminal_image keep multi-MB payloads off the transcript; retry-countdown-clear forwards isContinuation - slash-dispatch: isSlashCommandInput drops the '?' branch (ink gate routes ? input to the model); executeSlashCommand races the action against the abort signal; dialog effects carry the OpenDialogActionReturn payload; projected added-item text surfaces alongside non-handled effects (notice); message-shaped items project to their text; ui.history comes from env; absent sessionStats stamp now, not epoch; telemetry parity (recordSkillInvocation / recordAutoSkillCommandUsage / makeSlashCommandEvent) - item-projection: model stats render per-(model,source) sections with N/A for unpriced entries; Tool Calls line uses ASCII x like ink; redactProxy deduplicated via systemInfoFields export - theme: palette/syntax colors resolve through color-utils toHex before parseColor (ink CSS names / *bright names no longer degrade to magenta); unresolvable values stay unset - key-map: kitty 'kpenter' normalizes to 'return'; resolveCommands exposes ink's key fan-out (Ctrl+C fires QUIT + CLEAR_INPUT) - a11y: hardWrap delegates to wrap-ansi (word-boundary parity with ink's screen-reader path); markdown reducer tracks fence length, keeps fence-like lines literal inside fences and inner backticks in multi-backtick spans; stripAnsi delegates to strip-ansi plus a private-parameter CSI pass (SGR mouse, DEC save/restore) - clipboard: OSC 52 write gated on a TTY (stderr preferred), tests spy the stream instead of writing real sequences to the runner's terminal - exit-guard: independent per-key arm windows like ink - dialogs-theme: diff preview pane receives syntaxStyle/filetype * fix(cli): harden kitty probe and screen-reader writer per maintainer review - kitty-negotiation: KITTY_REPLY_RE requires at least one flag digit (\d+), so an echoed bare query \x1b[?u in PTY/CI environments no longer resolves true and locks the renderer into kitty mode on a terminal that never answers queries; the accumulation buffer keeps only a 256-byte tail (bounded memory, bounded rescan under byte floods); the settle-window drain is removed — an EventEmitter data listener cannot consume chunks from other listeners, so late replies flow to the renderer's input parser like any other terminal noise - a11y-screen-reader: ScreenReaderOutputWriter sanitizes written content (stripAnsi + drop bare C0/C1 controls, keep newlines) so the plain-text-only contract is enforced at the writer instead of trusting every future caller — smuggled OSC 52 clipboard writes or title/cursor sequences cannot execute on the main screen * fix(cli): address ytahdn independent review findings in OpenTUI foundation All 15 findings from the independent static review verified in source and fixed (no false positives; none deferred): - quit effect carries QuitActionReturn.messages projected to text on a notice field — ink renders them via QuittingDisplay and the payload was permanently lost (Important QwenLM#1) - error and finished branches emit retry-countdown-clear like ink's handleErrorEvent/handleFinishedEvent, so a terminal event inside the countdown window no longer leaves a stale retry row (QwenLM#2) - projectContextUsage renders the compaction-threshold ladder and the per-item detail sections (tools/memory/skills, ink's sort order) when showDetails is on — /context detail transcripts no longer show strictly less than the compact view (QwenLM#3) - projectMcpStatus honors showSchema (parameter JSON under each tool) and showTips, so /mcp schema is distinguishable from /mcp (QwenLM#4) - SlashDispatchEnv.settings is required: the real CommandContext.services.settings is non-null and a null surfaced as a generic command failure on first .merged read (QwenLM#5) - dead singleColumn flag removed from the help width layout (clamp makes it always false; ink has no single-column mode) (QwenLM#6) - truncated SS3 tail (bare ESC O) is stripped like the truncated CSI tail, so a captured half F1-F4 no longer leaks 'O' into the composer (QwenLM#7) - readBufferRow trims cellColumns alongside text, so URLs ending at end-of-row on a wide character hit-test on both halves of the cell (QwenLM#8) - kitty probe writes guarded: a synchronous stream throw settles the probe (restores raw mode, removes the listener) instead of leaking (QwenLM#9) - eraseLines reuses the ansi-escapes helper (already a repo dependency) instead of a byte-identical hand-rolled copy (QwenLM#10) - truncateText passthrough wrapper dropped; callers use the exported truncateHelpText directly (QwenLM#11) - SkillsList truncate keeps total length n like ink, so the description column no longer shifts by one cell when a name truncates (QwenLM#12) - model_fallback names pass through sanitizeDisplayText like ink (QwenLM#13) - selectIndex fires onHighlight before onSelect (ink dispatches SET_ACTIVE_INDEX then SELECT_CURRENT), keeping highlight-driven stay-open dialogs synced on mouse input (QwenLM#14) - mcp_app without fallbackText renders empty instead of JSON-dumping the embedded HTML; projectAbout hides Base URL when selectedAuthType is empty, matching ink's formatBaseUrl (QwenLM#15) * fix(cli): address R3 review findings in OpenTUI foundation modules - executeSlashCommand catch checks the abort signal first: an ESC- cancelled command (action rejects AbortError) returns handled with no failure telemetry or error message, mirroring ink's processor — the race promise never resolves when the signal is already aborted at addEventListener time - submit effect carries the full SubmitPromptActionReturn contract (modelOverride, onComplete, refreshContextFilesOnWrite) so the backend can honor /model <id> <prompt>, /dream's manual-run record, and /remember's context refresh like ink instead of silently degrading them - closing fences cannot carry info text (CommonMark): a ```js line inside an open block is literal body, not an early close that drops the block and inverts parse state for the rest of the document - info items append their linkUrl/linkText footer (ink's InfoMessage renders it; headless/SSH users need the printed URL, e.g. /bug) - the screen-reader sanitize keeps TAB: it separates words in tool/model output and deleting it fused adjacent tokens * fix(cli): address R4 review findings in OpenTUI foundation modules - a11y-plain-text: split on all CommonMark line endings so CRLF markdown opens/closes fences correctly; private-param CSI regex covers ECMA-48 intermediate bytes; DCS/SOS/PM/APC and unterminated OSC sequences consumed before strip-ansi; code-span pattern mirrors ink's INLINE_CODE_SPAN_PATTERN (non-empty content, closing-run lookbehind) - dialogs-shared: clearNumberBuffer called from setActiveIndex, selectIndex, and resyncKey block so wheel/hover/click/resync can't commit a stale numeric-flush selection the user never made - event-adapter: tool_call_response carries visionBridgeNotice on the tool-result event (ink ToolMessage renders the egress disclosure) - item-projection: projectContextUsage reads memoryFiles as { path, tokens } (ContextMemoryDetail), not { name, tokens } - key-map: 10 kp* keypad-navigation aliases (kpleft→left, …) and super flag folded into meta (ink Cmd+Enter = newline, not submit) - link-click: cellColumns no longer truncated to trimmed text length (preserves the wide-glyph right-half boundary); findUrlAtRow end boundary is width-aware (stringWidth of the last glyph) - slash-dispatch: submit effect carries PartListUnion content + a textContent string for text-only consumers (image parts survive); toggleVimEnabled and startNewSession seams wired from env; abort race resolves immediately for an already-aborted signal - clipboard: OSC 52 self-write removed — copyToClipboard's existing fallback (writeOsc52 / wrapForMultiplexer) is the single source - a11y-screen-reader: appendStatic skips clean === '\n' (ink's hasStaticOutput guard) * fix(cli): address QwenLM#10383 R1 findings in foundation modules - event-adapter: finished branch emits retry-countdown-clear BEFORE the info notice so the countdown row is actually cleared (the fold only pops when the last item is the retry row) - item-projection: /mcp tips now include all 5 lines ink renders (added OAuth auth tip and Ctrl+T toggle tip) - link-click: wide-glyph end boundary uses the last code point (not UTF-16 code unit) so non-BMP emoji are measured correctly by stringWidth - a11y-plain-text: CSI_SEQUENCE replaces PRIVATE_PARAM_CSI — drops the marker requirement so any CSI (with or without private parameter marker, with or without intermediate bytes) is fully consumed * fix(cli): address QwenLM#10383 R2 Critical findings in slash-dispatch - abort race: already-aborted signal now skips command.action entirely (result = undefined) instead of eagerly evaluating it as a Promise.race argument — the action's side effects (clear, persist, addItem) must not run on a cancelled submission - parent command telemetry: logEvent (slash_command SUCCESS) is now called before the early return for parent commands with subCommands (help listing) and bare handled — matching ink's finally-block logging * fix(cli): address QwenLM#10368 R2 review findings in live-session batch - input-prompt: convert OpenTUI display-width cursor coordinates to code-point positions at the component boundary — the pinned @opentui/core reports logicalCursor.col/offset and el.cursorOffset in terminal-cell units (edit-buffer.zig), while the ported ink helpers work in code points; wide characters previously shifted placeholder backspace, the backslash continuation check, completion targeting, and history edge compares - input-prompt: bump both search sequence refs when Esc dismisses the completion dropdown so an in-flight search resolving afterwards cannot re-open it and hijack Enter - live-session-model: carry the vision-bridge egress disclosure through the tool-result fold (ink ToolMessage renders it under the result) - messages: recognize the producers' two-L 'cancelled' summary spelling so canceled tools get the CANCELED glyph with strikethrough instead of the red ERROR glyph - session-switch: wrap /resume and /branch in the telemetry swap transaction (begin before the outgoing-session capture, commit at the UI re-key, abort after a rolled-back swap) — restores the usage aggregate on failed swaps and rejects concurrent switches - tests: display-width fake editor, wide-char placeholder/continuation witnesses, Esc invalidation, fold notice, cancelled spelling, and the three swap-transaction lifecycle cases * fix(cli): declare cursorOffset on the FakeEditor test interface The display-width fake added in 656e996 implements a cursorOffset getter/setter but the interface it is cast through never declared the member, so tsc --build fails with TS2339 at the two reads in the wide-char placeholder test. Typecheck ran before that commit's files were staged and missed it. * test(cli): stub listStartingRunIds in the session-switch fake registry The workflow-run registry gained listStartingRunIds with the workflow tasks feature on main; backgroundWorkUtils iterates it when describing blocking work, so the fake registry in session-switch.test.ts now implements it (empty) to match the interface the merged code expects. * fix(opentui): address yiliang114 review findings (4 P2 + 1 P3) - transcript-adapter: FIFO queue for id-less tool call pairing so tool-start and tool_result share the same minted id - session-switch: move uiSwapped=true to right after startNewSession (the first irreversible host mutation) preventing core/UI divergence on mid-sequence throw; same fix for branch handler - session-switch: add error item when /resume targets an unloadable session instead of returning silently - diff-render: run diff content through escapeAnsiCtrlCodes matching the ink text-boundary convention (useTurnDiffs.ts) - package.json: move remend from devDependencies to dependencies (imported from production source markdown-heal.ts) * fix(transcript): address R6 review — thinking latch, cancelled status, text join - Replace one-shot `closed` latch with `thinkingOpen` state so [thought, text, thought, ...] patterns emit matching thinking-end for each burst (P2) - Mirror live path: treat cancelled tool status as failed, not ok (P3) - Join user text parts with newline instead of empty string (P3) - Regenerate package-lock.json so remend is in dependencies (P2) * fix(opentui): address review-pr bot R3 critical findings - session-compaction: add missing COMPRESSION_FAILED_EMPTY_SUMMARY, OUTPUT_TRUNCATED, and API_ERROR cases to match ink compression-text.ts - live-session: distinguish cancelled from error in tool-end summary so toolStatusMeta renders strikethrough instead of red X - transcript-adapter: gate slash_command replay on phase=invocation to prevent double-replay (recorder writes both invocation and result) - input-prompt: add key.meta/key.option to DELETE_WORD_BACKWARD branch to match the guard condition that intercepts Alt+Backspace * fix(opentui): address round-5 review findings R2-5 R3-2 R3-12 R4-1 R2-5: update session-compaction.test.ts to assert the three new parity texts (EMPTY_SUMMARY, OUTPUT_TRUNCATED, API_ERROR) added in 63a7f3c; the old assertion that EMPTY_SUMMARY returned '' is stale. R3-2: transcript-adapter replay producer folded cancelled tool status into summary 'error' (red ✕) instead of 'cancelled' (strikethrough). Add the cancelled branch to match live-session.ts. R3-12: hidden slash-command invocations (hiddenInvocation: true for /auth, /help, /settings, /status, bare /effort, /btw) replayed as visible user rows and entered composer history. Gate them on the hiddenInvocation flag in the invocation filter. R4-1: modelOverride was only carried on the first UserQuery send; ToolResult continuation sends omitted it, so a per-turn model override silently reverted to the session default after the first tool batch. Propagate modelOverride into every continuation send.
|
@qwen-code /triage |
|
Sandboxed verification: ❌ not passed — findings reported (agent verdict) - workflow run Ran the PR in an isolated, token-free container: A/B against the base build, mock-free harness assertions, targeted gates. Advisory evidence for human reviewers — not a review, an approval, or a CI check. Scripted assertions: 34 passed · 0 failed · 34 total Flakiness gate: 中文 — 判定：❌ 不通过 · 报告了发现（agent 判定）沙箱验证在隔离、无凭证的容器中执行了该 PR 的代码（与 base 构建 A/B 对照、无 mock harness 断言、定向门禁）。仅作为评审证据，不构成评审、批准或 CI 检查。 脚本断言：34 通过 · 0 失败 · 34 总计 抖动门： Verification reportPR 10383 Deep Verification — Round 2 (follow-up): OpenTUI migration batch 4 (dialogs, commands, session-rewind)Verdict: 中文摘要结论：
Previous-round finding status (follow-up round)Round 1 (verified head
Round-1 central measurements re-run at the new head (not carried forward): the bundle A/B (new base = main), the opentui suite (twice), Central claim and A/B proofCentral claim (PR body): "Everything is additive: no reachable ink code path is touched, nothing imports these modules yet, and the default renderer stays ink." Falsifiable exactly as in round 1: the shipped artifact is the esbuild bundle from
Noise normalization, named and bounded: N1 per-arm Control purity. From inside the base worktree, workspace dependencies resolve through the root Corrections (description vs current head)These are corrections to the PR description, not requests to change code. The branch moved a long way since the body was written (R1–R4 fix rounds, a rebase onto main):
FindingsF1 —
|
| arm | probe expects rollback | probe expects commit (ink) |
|---|---|---|
| head (moved flag) | P1 PASS — coreIds [target, old], swapAbort=1, swapCommit=0, UI re-keyed |
P2: correctly FAILs |
| base/ink ordering (M6) | P3: correctly FAILs | P4 PASS — coreIds [target], swapCommit=1 |
Witness 03-split-brain-probe-matrix.png (5/5 cells incl. tree-restore). Additionally: the full session-switch.test.ts suite is green under the base ordering too (M6 survival, exit 0) — the suite pins neither placement, so this moved silently; the stale in-file comments still assert the old invariant (session-switch.ts:187-188 "The commit point is the UI-side session re-key: from here on a failure must not roll core back…" and the branch-side session-switch.ts:367 "The UI re-key commits the swap"), now contradicting the code; and the file's own header promises "the SAME rollback semantics" as ink.
Bounds: the opentui tree is unreachable from the shipped bundle (A/B above: 0 inputs both arms), so there is zero user-visible impact today; this activates when the renderer is wired in a later batch. If every host display method provably cannot throw the window is theoretical, but the transcribe seam is genuine parsing of on-disk session shapes.
Suggested fix (measured): restore the base/ink placement — uiSwapped = true immediately after host.startNewSession(…) in both functions (and drop or move the now-stale commit-point comments). Measured results, same harnesses: the commit-probe passes under it (P4), the rollback-probe fails under it (P3 — the bad shape disappears), and the entire session-switch suite stays green (M6 run, exit 0). Since the suite pins nothing along this axis, the fix should land with the probe scenario as a regression test (host replay step throws after the re-key ⇒ assert no core rollback, telemetry committed, error item surfaced) — otherwise the next refactor can move the flag back with every test still green.
F2 — remend added as a duplicate devDependency while staying a production import; lockfile node carries dev: true (hygiene, no observable impact)
Base had remend ^1.3.1 in packages/cli dependencies only (it is imported by production code, opentui/markdown-heal.ts:17). The delta adds it to devDependencies as well, and the lockfile node gained "dev": true. Measured: regenerating the lockfile in a scratch worktree with npm 10.9.8 (npm install --package-lock-only) leaves the remend state unchanged (committed == regenerated; the only diffs are unrelated musl-libc/peer metadata churn), so the committed lockfile is not stale — but the combination is misleading: a production import whose node is marked dev: true, plus a redundant duplicate entry. No install path in the repo uses --omit=dev/--production (grep of Dockerfile/Makefile/workflows: none), the module is bundle-unreachable (0 remend inputs in both arms' bundle graphs — consumers are vitest-only), and the published npm package's manifest lists remend in dependencies, so end users are unaffected. Direction: drop the redundant devDependencies entry and regenerate the lockfile so the node loses dev: true.
F-note — buildCompletionContext still supplies the dead setGeminiMdFileCount key (pre-existing, carried on base)
opentui/input-prompt.tsx:129 builds its completion context with setGeminiMdFileCount: () => {} while CommandContext declares setMemoryFileCount (types.ts:114); the double cast (as unknown as CommandContext) hides the drift. Unobservable today — completion functions read services.config only, and the one runtime reader (directoryCommand.tsx:266) runs through the dispatcher's context, which supplies the declared key — but it is precisely the dead-key drift the round-1 rename had fixed and the rebase dropped. Not introduced by this PR's effective diff (byte-identical to base); noted so it is not silently re-lost.
Gates
All scripted; counts are exact.
| Gate | Result |
|---|---|
vitest run src/ui/opentui/ (packages/cli), run twice |
56 files / 949 tests, 0 failures, exit 0 both runs (console-summary oracle; PR body's 887 is stale) |
tsc --noEmit in packages/cli |
clean, exit 0 |
node scripts/check-tui-dep-direction.mjs at HEAD |
PASS — 1331 core files / 9857 specifiers, 4 ui/model files |
| dep-direction gate live proof (6 synthetic fixtures) | 6/6: clean fixture passes; ink-import, react-in-core, bare-cli-import, relative-escape from ui/model, and vi.mock('@opentui/core') each flip exit 0→1 with the violating file named (05-dep-direction-gate-live-proof.png) |
| reformatted core test compiles+passes | packages/core tool-registry.test.ts: 1 file / 60 tests, 0 failures |
| reformatted companion file typechecks | vscode-ide-companion tsc --noEmit exit 0 |
| mutation matrix M1–M5 | 5/5 killed with real assertion mismatches, every unmutated control green, every file restored (git diff --exit-code clean) (04-mutation-matrix-5-of-5-killed.png) |
| M6 (base ordering) suite survival | exit 0 — suite pins neither flag placement |
| working tree after all verification | clean |
Mutation matrix detail
| Mutant | Production change | Killing test | Failure observed |
|---|---|---|---|
| M1 (carried) | commands-dispatch.ts: await this.host.handleBranch(…) → void |
/branch awaits handleBranch |
expected 'resolved' to be Symbol(pending) |
| M2 (carried) | commands-registry.ts: case 'mcp' → { dialog: 'hooks' } |
maps each dialog kind… |
expected { dialog: 'hooks' } to deeply equal { dialog: 'mcp' } |
| M3 (carried) | session-rewind-model.ts: cut-point seen === occurrence + 1 |
rewindApiCutPoint (2 tests) |
expected 2 to be +0, expected 3 to be 1 |
| M4 (new) | dialogs-shared.tsx: delete the items re-sync block (ink INITIALIZE parity) |
3 new useDialogSelect items re-sync tests |
expected 2 to be +0, expected 1 to be +0 |
| M5 (new) | slash-dispatch.ts: remove the pre-abort skip (action runs on an aborted signal) |
strengthened R3-1 (expect(action).not.toHaveBeenCalled()) |
expected "spy" to not be called at all, but actually been called 1 times |
| M6 (reverse) | session-switch.ts: restore base/ink flag ordering |
(expected survivor) | suite exit 0 — survived, classified: coverage gap along the commit-point axis; the discriminating fixture is the split-brain probe (see F1) |
Positive control: M1–M5 each turned their suite red with the intended assertion mismatch; collection-error shapes were screened out by the classifier. M4/M5 additionally prove the two delta fixes (items re-sync; pre-abort skip) are load-bearing — the tests added with them pin exactly the changed behavior.
Not covered
- Per-commit verification. Depth-2 checkout: only the merge commit, base tip, and PR head exist locally. The metadata snapshot lists 53 commits;
git rev-list HEAD^1..HEAD^2reaches 1 — the known shallow-boundary artifact, so the gap is real, not a small PR. The old round-1 headf35afd87is not object-reachable either, so the delta since round 1 could not be diffed directly; new probing was steered by the delta commit subjects in the metadata (R1–R4 fixes, the R2 Critical fixes) and by re-verifying the aggregate diff. Per-commit attribution remains out of reach. - Trial merge against a newer main. The base IS the main tip (
2b8f73c1) and the merge ref composed without conflict, so mergeability into that main is established by construction; drift against a newer main cannot be measured in this checkout. - Runtime behavior of the new modules in a live renderer. Unreachable by design (proven above); evidence of correctness is the suite + mutation matrix + probes. This is shape coverage (routing tables, state machines, swap protocols), not a wired-renderer scenario.
- Line-by-line ink parity beyond the measured seams. The session-switch commit-point seam, the
useDialogSelectINITIALIZE seam, and the slash-dispatch abort race were checked against ink directly; the remaining parity surface relies on the suites. - Full flakiness protocol. Round 1 ran 5 rounds × 14 files (3 completed, timeout; agreed). This round re-ran the whole 56-file subset twice, both green; the 5-round protocol was not repeated on budget grounds.
- Repo-wide test suite, lint/format gates, integration tests — unaffected surfaces for an additive-only, bundle-unreachable batch (the two out-of-tree formatting touches verified individually above).
Methodology
Environment: the CI verify container (node:22-bookworm), working tree at refs/pull/10383/merge (depth 2); npm ci + npm run build completed before the round. The A/B drove the repository's own esbuild.config.js verbatim in both arms (DEV=true for the metafile); the base arm ran in a scratch worktree under tmp/ with per-package node_modules directories symlinked from the root tree and per-arm generated files regenerated in-arm (generate-git-commit-info.js, web-templates/build.mjs), legitimated by the unchanged dependency closure and the test-only core diff; metafile inputs were canonicalized via realpath before comparison, and shipped bytes compared under the N1–N3 normalization named above. The dep-direction gate was live-proven via six synthetic repo-root fixtures so neither tree was touched. Mutations were exact-string edits applied by mutation-runner.mjs (single-occurrence asserted, unmutated control green first, file restored and git diff --exit-code re-verified after each run). The split-brain probe (probe-split-brain.mjs) wrote a scratch vitest file into packages/cli/src/ui/opentui/ only while no other vitest run was in flight and deleted it in a finally; all five probe cells were re-captured live. All harnesses live in this directory (ab-compare.mjs, ab-bytes.mjs, gate-live.mjs + gate-fixtures/, mutation-runner.mjs, probe-split-brain.mjs); raw logs: build-base.log, build-head.log, ab-compare.log, ab-bytes.log, gate-suite.log, gate-suite-run2.log, gate-tsc.log, gate-live.log, mutation-run.log, m6-suite.log, probe-split-brain.log.
Flakiness gate log
rounds=5 files=18 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/commands-registry.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/commands-registry.test.ts
file packages/cli/src/ui/opentui/dialog-data.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialog-data.test.ts
file packages/cli/src/ui/opentui/dialogs-auth.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-auth.test.tsx
file packages/cli/src/ui/opentui/dialogs-extensions.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-extensions.test.tsx
file packages/cli/src/ui/opentui/dialogs-mcp.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-mcp.test.ts
file packages/cli/src/ui/opentui/dialogs-memory-status.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-memory-status.test.ts
file packages/cli/src/ui/opentui/dialogs-misc.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-misc.test.tsx
file packages/cli/src/ui/opentui/dialogs-model.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-model.test.ts
file packages/cli/src/ui/opentui/dialogs-permissions.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-permissions.test.ts
file packages/cli/src/ui/opentui/dialogs-settings.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-settings.test.ts
file packages/cli/src/ui/opentui/dialogs-shared.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/dialogs-shared.test.tsx
file packages/cli/src/ui/opentui/folder-trust-gate.test.tsx: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/folder-trust-gate.test.tsx
file packages/cli/src/ui/opentui/session-rewind.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/session-rewind.test.ts
file packages/cli/src/ui/opentui/session-switch.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/session-switch.test.ts
file packages/cli/src/ui/opentui/slash-dispatch.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/slash-dispatch.test.ts
file packages/cli/src/ui/opentui/slash-gateway.test.ts: (cd packages/cli) npx --no-install vitest run ./src/ui/opentui/slash-gateway.test.ts
file packages/core/src/tools/tool-registry.test.ts: (cd packages/core) npx --no-install vitest run ./src/tools/tool-registry.test.ts
per-file results (P=pass F=fail I=infra-exit, one letter per run):
packages/cli/src/ui/opentui/commands-dispatch.test.ts: PP
packages/cli/src/ui/opentui/commands-registry.test.ts: PP
packages/cli/src/ui/opentui/dialog-data.test.ts: PP
packages/cli/src/ui/opentui/dialogs-auth.test.tsx: PP
packages/cli/src/ui/opentui/dialogs-extensions.test.tsx: PP
packages/cli/src/ui/opentui/dialogs-mcp.test.ts: P
packages/cli/src/ui/opentui/dialogs-memory-status.test.ts: P
packages/cli/src/ui/opentui/dialogs-misc.test.tsx: P
packages/cli/src/ui/opentui/dialogs-model.test.ts: P
packages/cli/src/ui/opentui/dialogs-permissions.test.ts: P
packages/cli/src/ui/opentui/dialogs-settings.test.ts: P
packages/cli/src/ui/opentui/dialogs-shared.test.tsx: P
packages/cli/src/ui/opentui/folder-trust-gate.test.tsx: P
packages/cli/src/ui/opentui/session-rewind.test.ts: P
packages/cli/src/ui/opentui/session-switch.test.ts: P
packages/cli/src/ui/opentui/slash-dispatch.test.ts: P
packages/cli/src/ui/opentui/slash-gateway.test.ts: P
packages/core/src/tools/tool-registry.test.ts: P
verdict: timeout
summary: the 15-minute budget elapsed before two full rounds completed (1 done) — no flakiness signal either way
--- 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/commands-registry.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialog-data.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-auth.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-extensions.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-mcp.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-memory-status.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-misc.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-model.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-permissions.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-settings.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/dialogs-shared.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/opentui/folder-trust-gate.test.tsx: P (exit 0)
round 1 · packages/cli/src/ui/opentui/session-rewind.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/session-switch.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/slash-dispatch.test.ts: P (exit 0)
round 1 · packages/cli/src/ui/opentui/slash-gateway.test.ts: P (exit 0)
round 1 · packages/core/src/tools/tool-registry.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/opentui/commands-dispatch.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/opentui/commands-registry.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/opentui/dialog-data.test.ts: P (exit 0)
round 2 · packages/cli/src/ui/opentui/dialogs-auth.test.tsx: P (exit 0)
round 2 · packages/cli/src/ui/opentui/dialogs-extensions.test.tsx: P (exit 0)
Evidence images
Harness scripts and raw logs are in the workflow run artifacts (7-day retention).
— Qwen Code · sandboxed verification
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Reviewed at head aa0fdd64.
- Prior review rounds are dismissed and 78/78 threads are resolved; the batch is dominated by new, not-yet-wired OpenTUI modules (dialogs, commands dispatcher, gateway, trust gate, session-rewind), so the merge-time blast radius on the live renderers is minimal.
- Checked the few edits that do touch shared code:
dialogs-sharedcursor key-follow resync is an ink-parity fix for shrinking lists and is covered by new tests;historyMappingonly widens an export;session-switchkeeps the telemetry transaction committed on the not-found path (test-pinned). - The folder-trust gate is a faithful port of the ink flow (decision persisted via
trustedFolders, restart on flip, write failure aborts silently to stderr rather than half-applying). - No new Critical issues found. Code CI on this head has no failures so far (
review-pr/Test still running).
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
Independent pass at aa0fdd6. Local verification: opentui subset 56 files / 949 tests all green, packages/cli tsc clean. Security dimension is clean — folder-trust-gate keys trust correctly and Esc fails safe, dialogs-permissions matches ink parity, and no untrusted content reaches command dispatch. Three correctness findings inline, all in the new batch-4 modules; they're edge cases the suite doesn't cover yet, so worth addressing before this batch wires into a reachable path. Not approving this round; happy to re-check once they land.
|
Thanks for the clean retarget onto main after #10368 landed. Template: complete ✓ — all required sections are present, so the earlier template-gate request is resolved. (Minor: the Chinese block is a condensed summary rather than the per-section translation the template asks for; not worth another round-trip.) Problem: this is batch 4 of the planned OpenTUI migration, so the "problem" is the agreed plan itself, not a bug claim: tracking issue #8662 is open, the migration design doc is on Direction: aligned — the migration is maintainer-directed and lands batch by batch so each unit stays reviewable, exactly as this one does. Size: ~10,950 production lines vs ~5,930 test lines vs 3 manifest/lockfile lines. That is far past the 1,000-line advisory threshold; normally I'd suggest splitting, but the #8662 batch plan deliberately defines these units, so this is advisory only. Core-path exposure: one test file ( Approach: the shape is right — 33 new files in the isolated Risk: no high-risk path matches; no elevated risk signals beyond the items above. Moving on to code review. 🔍 中文说明感谢在 #10368 合入后干净地重新基于 main。 模板：完整 ✓——所有必需小节齐全，此前的模板门禁请求已解决。（小问题：中文块是浓缩摘要而非模板要求的逐段翻译，不值得再走一轮。） 问题：这是 OpenTUI 迁移计划的第 4 批，"问题"即已达成共识的计划本身，而非 bug 声明：跟踪 issue #8662 开启中，迁移设计文档已在 main 上，第 1–3 批（#10134、#10146、#10368）已合入。这类批次不需要复现。其可验证的声明是结构性的——纯增量、运行中的 CLI 不可达——这些已在 retarget 之前的 head 上由沙箱 方向：对齐——迁移由维护者主导，按批落地以保证每个单元可独立评审，本批正是如此。 规模：生产代码约 10,950 行、测试约 5,930 行、清单/锁文件 3 行。远超 1,000 行建议阈值；通常应建议拆分，但 #8662 的批次计划刻意定义了这些单元，故仅作提示。核心路径暴露：仅一个测试文件（ 方案：形态正确——33 个新文件位于隔离的 风险：无高风险路径命中；除上述事项外无升级风险信号。 进入代码审查 🔍 — Qwen Code · qwen3.8-max Reviewed at |
Code reviewIndependent baseline first: for a dialogs + command-routing batch stacked on the live-session batch, I'd want exactly what this PR does — new files only in an isolated namespace, one exported predicate reused instead of duplicated ( One item needs a fix before merge — the
Non-blocking:
Everything else holds up. The review-fix delta since the last verification passes is careful and tested: the session-switch unarmed-transaction settlement on missing sessions, the sequenceDiagram
participant P1 as User input
participant P2 as OpenTuiSlashGateway
participant P3 as OpenTuiSlashDispatcher
participant P4 as commands registry
participant P5 as routeDialogToOpenTui
participant P6 as Dialog component
P1->>P2: raw slash input
P2->>P2: stacked-skills parse, extension resolution
P2->>P3: parsed command
P3->>P4: look up command
P4-->>P3: entry with result kinds
P3->>P5: ink dialog-switch enum
P5-->>P6: typed OpenTUI dialog request
P6-->>P1: rendered dialog
TestingThis is a CI-path run — no PR code was built or executed here. The evidence below is the PR's own CI on the reviewed head, plus the two isolated verification runs cited above (both on pre-retarget heads). Nothing in this batch is user-visible yet — no caller imports it — so there is no terminal behavior to drive even on a local run:
The main unit suite was still in flight at review time; the finalize pass updates the table once CI settles. The earlier "Browser daemon SDK bundle" red was a main-branch regression fixed by #10602, and this head merges main after that fix. Sandboxed verification would settle the one open evidentiary gap: a fresh 中文说明代码审查：独立基线与 PR 方案一致——新文件全部位于隔离命名空间，复用一个已导出的谓词而非复制，注册表→分发器→网关分层，零 ink 改动。 合入前需修一项——retarget 引入的 非阻塞： 其余站得住：自上一轮验证以来的评审修复增量谨慎且有测试——session-switch 未武装事务在会话缺失时的结算、 测试：本次为 CI 路径——未构建、未执行任何 PR 代码。上表为被审 head 上 PR 自己的 CI；主单元测试套件评审时仍在运行，收尾工作流会在 CI 落定后更新表格。此前 "Browser daemon SDK bundle" 红是 main 回归，已由 #10602 修复，本 head 已合入该修复。本批尚无任何调用方，无用户可见行为，真实场景终端证据不适用（N/A）。补一句：对 retarget 后的 head 重新跑一次 — Qwen Code · qwen3.8-max Reviewed at |
|
Confidence: 2/5 — the batch itself is among the best-verified PRs this stack has seen (sandboxed A/B, real-renderer harness, mutation matrix, four review rounds), but the retarget introduced one objective defect — the duplicate Stepping back: the implementation matches my independent proposal almost exactly, the additive-and-unreachable claim survived an A/B bundle proof and a real-renderer run, and the R3/R4 delta since those verifications is careful, tested work. I looked hard for a reason to approve as-is — the retarget was "just merges of main," after all — and the manifest state is the one place the premise "everything is additive, nothing outside the batch changes" does not hold: this diff, and not main, adds the duplicate devDependency and the So: request changes, narrowly. @chiga0 — drop the (Note: the earlier template-gate request-changes from the first triage run is resolved by the updated PR body. The standing request-changes gate on this PR is left in place — re-runs update comments, not reviews — and the single open item is the — Qwen Code · qwen3.8-max Reviewed at |
|
Triage re-run completed without a new review.
The stage comments above were updated with the latest result. View workflow run. 上方各阶段评论已更新为最新结果。查看工作流运行。 |
…istory - session-rewind: add useRef re-entrancy guard to prevent double onRewind from batched key events in single stdin chunk - session-rewind: dispatch restore-error on onRewind rejection so the dialog recovers from dead 'restoring' phase back to 'pick' - commands-dispatch: pass Date.now()-based timestamps to addItem instead of array indices in load_history branch
qwen-code-dev-bot
left a comment
qwen-code-dev-bot
left a comment
There was a problem hiding this comment.
Re-reviewed at head e3ab6017 (my earlier approval on aa0fdd64 was auto-dismissed by the push).
- The net increment since that head is three files, all in the not-yet-wired OpenTUI modules, and all clear improvements from the latest review round: the command dispatcher's history replay no longer collides with existing item ids (
now + index), and the rewind selector gains arestore-errortransition plus a re-entry guard so a failed restore returns the user to the picker instead of swallowing the error or double-firing. - 81/81 review threads are resolved; no open findings.
- No new Critical issues in the increment or the surrounding code. CI on this head is still settling (Test and review-pr running, no failures so far); per the channel convention the call is on the review itself.
yiliang114
left a comment
yiliang114
left a comment
There was a problem hiding this comment.
The three P2 findings from my earlier review are all addressed in e3ab601:
startRestorenow dispatchesrestore-errorononRewindrejection and the reducer returns the dialog from the dead 'restoring' phase back to 'pick' with clean selection state.- A
useRefre-entrancy guard prevents a doubleonRewindfrom batched key events in a single stdin chunk. load_historypassesDate.now() + indextimestamps toaddIteminstead of array indices.
Verified locally on the new head: session-rewind + commands-dispatch tests 79/79 pass. The rest of the batch was reviewed earlier with no other blocking findings.
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
Round 4 — cross-check + minor finding
Scope: packages/cli/src/ui/opentui/ new files (dialogs, commands-dispatch, commands-registry, slash-gateway, session-rewind-model); plus side-effect changes outside opentui (historyMapping.ts export, tool-registry.test.ts reformat, diff-manager.ts reformat, package.json devDep). Not reviewed: design doc under docs/design/.
CI (disclosure):
Test (ubuntu-latest, Node 22.x)— pending at review time.Test (macos-latest, Node 22.x)andTest (windows-latest, Node 22.x)— SKIPPED (fork authorization gate). PR claims macOS arm64 Node v24 vitest green; CI has not verified it.Integration Tests (CLI, No Sandbox)— SKIPPED.Integration Tests (no-AK, No Sandbox)— PASS.Desktop Shell(ubuntu + windows) — PASS.
Cross-check against prior rounds:
All critical findings from R1 and R2 rounds verified at current head:
- R1-1 (MCP dialog indexes raw
servers) — FIXED:flatServersnow derived fromgroupedServers.flatMap(). - R1-2 (editor dialog scope highlight) — FIXED:
applyScope()andhighlightScope()both callsetScope(item.value). - R1-3 (settings dialog missing
excludeWorkspaceRestricted) — FIXED:excludeWorkspaceRestricted: selectedScope === SettingScope.Workspaceon line 323. - R1-4 (
retry-countdown-cleardropsisContinuation) — FIXED: testevent-adapter.test.ts:357("forwards isContinuation on the countdown clear") verifies the emitter. - R1-6 (stats dialog reads process-global telemetry) — FIXED:
uiTelemetryService.getMetricsForSession(sessionId)when session ID available. - R1-12 (
MODE_DESCusesauto_editkey vsauto-editenum) — FIXED:MODE_DESCnow keyed"auto-edit". - R2-1 (
buildModelEntriesfor image mode gates onimageOnlyonly) — FIXED: now usesisImageGenerationCapable(model). - session-rewind restore-error / double-invoke / load_history timestamp — confirmed FIXED (verified 2026-09-01 earlier today).
Minor finding — trivially actionable:
See inline comment on commands-dispatch.ts:398.
No blocking findings. The PR is the author's own, so APPROVE is not available here — reporting clean via COMMENT instead.
Checked: routeDialogToOpenTui switch exhaustiveness (compile-time never guard, clean); historyMapping.ts export additivity (pure additive, no behavior change); rewindApiCutPoint positional logic (correct); OpenTuiSlashGateway concurrency guard (correct). Ran: CI rung 0 checked above. Execution rungs 1-3 not run (working tree not available; PR code not yet wired to any execution path).
Reviewed with AI assistance.
|
已确认 wenshao 的 maintainer verification 报告中的 7 项 findings。计划按建议执行：本批先行合入（当前代码不可达、无用户影响），F1（HelpOverlay 74col 溢出）/F3（SkillsDialog 列布局）/F4（ApprovalModeDialog scope picker）在后续 batch 6/7 激活前修复，F2 测试覆盖同步补充。F5/F6/F7 作为迁移计划上下文记录。感谢详细的 real-renderer 验证和 mutation matrix！ |
chiga0
left a comment
chiga0
left a comment
There was a problem hiding this comment.
R5 review — 1 blocker + 3 execution-witnessed test holes
Note: this PR was merged (07:41Z) while this review was in flight, and the blocker below is now on main via 8096ece889. Posting here for the record on this diff; tracking should move to a follow-up.
Scope of this pass. Executed on linux / node v24.20.0 / vitest 3.2.7 against e3ab6017: full src/ui/opentui/ suite (56 files / 949 tests, green after installing remend), plus A/B probes and a 6-mutation matrix on session-switch.ts, dialogs-shared.tsx, commands-registry.ts. Four parallel read-lenses covered commands-registry/slash-gateway, session-rewind-*, dialogs-{auth,permissions,mcp,extensions}, dialog-data/commands-dispatch, each against its ink counterpart as oracle.
Not verified, stated so it isn't mistaken for a pass:
- CI:
Test (ubuntu-latest, Node 22.x)was stillin_progressat review time;Test (macos),Test (windows),Integration Tests (CLI),build-cliallSKIPPED. No completed full-test check exists for this head. My green run is linux/node24 only — no Windows/macOS evidence on either side. - Root
tsc --noEmitOOM'd in my environment (node heap limit), so "typecheck clean" is not something I confirmed. dialog-data.ts/commands-dispatch.tsinternals and the settings/arena/misc/model/modes/stats renderers are read-lens only, not executed.
Cross-check against R1–R4
Every R5 finding below is new: no prior comment touches the uiSwapped commit point, the two surviving mutants in the R4 useDialogSelect re-sync, the route-table truthiness assertion, /goal's submit_prompt, or rewindApiCutPoint's missing guards.
Confirmed already filed (not re-reported here, just verified still standing at head): MCP AUTHENTICATE step unreachable = R2-24 · extensions async rejections swallowed = R1-5 · "Tab to complete" not implemented = R2-31 · /restore missing gate = R2-12 · Shell ignores onClose = R2-27.
Verified and correctly dropped as non-issues: removing the Session … could not be loaded. line matches ink (useResumeCommand.ts:161-167 returns silently) · the remend devDependency + lock "dev": true state is pre-existing on origin/main, not authored by this PR.
Deferred to the body as one-liners (round ≥5 posting floor — no new threads)
dialogs-auth.tsx:801-830drops ink'sisAuthenticatinglatch (useAuth.ts:96-101): Enter repeat fires N concurrentapplyProviderInstallPlanread-modify-writes on settings.json.dialogs-auth.tsx:771-781the!props.configearly return registers no key handler, so its own "esc to close" is false (only exit is quit). Consequence of R2-27, not previously enumerated.dialog-data.ts:513-517settings.setValueafter a committedconfig.switchModelis unwrapped, so it rejects and loses ink's distinct "switched, but could not save" signal (ModelDialog.tsx:1026-1050); no re-entrancy latch onapplyModelSelectioneither.commands-dispatch.ts:281,301nomountedRefguard post-dispose()(slashCommandProcessor.ts:319-321), so a latehost.addIteminside acatchcan surface as an unhandled rejection.session-rewind.tsx:71-75turnCountis fixed at mount byuseReducer's initialArg and no action updates it, whileuserTurnsrecomputes every render —restore-errorreturns topickwith stale bounds. Latent until a caller exists.slash-gateway.ts:56-60attach()never clearsinitError.
Reviewed with AI assistance.
| Date.now(), | ||
| ); | ||
| } | ||
| uiSwapped = true; |
There was a problem hiding this comment.
[R5-1 · Critical] Moving uiSwapped = true past the display steps reopens the rollback window this batch had closed, and on /branch it deletes the session it just created.
witness: executed — A/B on the same probe, only the flag placement differing.
The commit point is still documented as the re-key — :187-188 says "The commit point is the UI-side session re-key: from here on a failure must not roll core back OR undo the telemetry replay", and commit 656e996c9a that moved it says "commit at the UI re-key". The code now commits only after setSessionName / clearPendingState / clearItems / loadHistory / resetTranscript. ink sets it one line after startNewSession() and names the reason (hooks/useResumeCommand.ts:230-236): rolling core back after the re-key leaves "UI on the resumed session but recorder writing into the old JSONL (split-brain)".
Trigger: any display step throws after host.startNewSession() — host is caller-supplied, so this is live the moment the batch is wired.
/resume: UI shows the new session, core rolls back toold-session, telemetry swap is aborted. The transcript on screen is the new session's; the user's next message is recorded into the abandoned session./branch(same move at:365): worse, because:407if (forkCreated && !uiSwapped)then callssessionService.removeSession(newSessionId)— the branch the UI is currently displaying is deleted from disk, while core returns to the original.
Probe results (linux, node v24.20.0, vitest 3.2.7):
| probe | at head | with uiSwapped back at the re-key |
|---|---|---|
/resume, loadHistory throws |
FAIL expected 'old-session' to be 'new-sess' |
PASS |
/branch, loadHistory throws |
FAIL expected 'old-session' to be '48b3d1fa-9016-…' |
PASS |
/branch, fork deletion |
FAIL removeSession was called with the UI-keyed branch id |
PASS |
And nothing pins it either way: with the flag restored, the whole directory is still green — 58 files / 951 tests pass under both placements. The R4-4 test added here covers the swap-slot settlement, not the commit point; the only failure seam in the harness is failClientInitialize, which fires before the re-key and therefore cannot see this window.
Suggested fix: move both assignments back to immediately after host.startNewSession(...) (before the display steps), and add a test whose display step throws after the re-key asserting core is not rolled back and removeSession is not called — that test fails at head and passes after the fix.
| if (followed !== activeIndex) { | ||
| const next = | ||
| followed >= 0 | ||
| ? followed |
There was a problem hiding this comment.
[R5-2 · Major] Two mutants of this new re-sync survive the suite, including the one the follows the active item by key test is named for.
witness: executed — mutation matrix, both mutants leave dialogs-shared.test.tsx at 10/10 green.
- Key-following can be deleted outright. Replace the ternary with
const next = computeInitialActiveIndex(initialIndex, items);— i.e. never follow the key, always reset — and all 10 tests pass. The reason is thatinitialIndexdefaults to0(:164), and in the reorder test the key-follow also yields0, sofollows the active item by key when the list is reorderedcannot distinguish the two branches it claims to separate. To kill it, a fixture needs the surviving key to land at a non-initial index. :207itemsRef.current = items;in theresyncKeybranch is untested. Delete it and the suite stays green — even though its comment states the exact hazard it prevents ("the key-follow below must not override it with the previous view's key"). That is the R1-10 / R2-18 stranded-cursor path: on a view swap the hook gets a newresyncKeyand a freshitemsidentity in the same render, so without this line the key-follow runs against the previous view's array. It is load-bearing, not dead — consumers pass non-memoized arrays (dialogs-permissions.tsx:235,244,289,dialogs-settings.tsx:425,dialogs-theme.tsx:166), so both changes co-occur.
The re-sync block as a whole is not vacuous — deleting it entirely (if (false && …)) fails 2 tests. It is specifically the branch selection and the view-swap interaction that are unpinned.
| dialog, | ||
| } as OpenDialogActionReturn); | ||
| expect(request).toBeTruthy(); | ||
| expect(request.dialog).toBeTruthy(); |
There was a problem hiding this comment.
[R5-3 · Major] This assertion pins the 33-member route table only by truthiness, so any wrong-but-present dialog target survives.
witness: executed. Mutating commands-registry.ts case 'subagent_list': return { dialog: 'subagent_create' }; — i.e. /agents list opens the create dialog — leaves commands-registry.test.ts at 15/15 green. The test title advertises "routes every dialog kind" parity with the ink switch, but nothing asserts the routed kind equals the input kind.
One line closes it: expect(request.dialog).toBe(dialog) inside the loop (the arena modes are the known exception — they map onto one dialog with a mode, so those cases need the mode asserted instead).
| { name: 'forget', results: ['message'], gatedBy: 'managed-memory' }, | ||
| { | ||
| name: 'goal', | ||
| results: ['goal_control', 'message', 'submit_prompt'], |
There was a problem hiding this comment.
[R5-4 · Minor] /goal declares submit_prompt, which goalCommand cannot return.
witness: read-verified at head, not executed. The only return sites in ui/commands/goalCommand.ts are :74 { type: 'message', messageType: 'error', … } and :83 type: 'goal_control'; the substring submit does not occur in that file. The table's header states it was "checked against the ORIGINAL sources", so this row is the claim being wrong rather than a runtime hazard — nothing consumes results yet.
Worth fixing while the table is still the only audit handle: an over-declared kind is how a later consumer ends up offering a path the command never produces.
| * Returns -1 when the history holds fewer real user prompts than | ||
| * requested (e.g. the turn was absorbed by chat compression). | ||
| */ | ||
| export function rewindApiCutPoint( |
There was a problem hiding this comment.
[R5-5 · Major] The docblock claims ink computeApiTruncationIndex parity, but this drops all three guards that make that function safe — including the case its own parenthetical says it returns -1 for.
witness: read-verified at head against utils/historyMapping.ts:118-198; no production caller exists yet, so latent.
Missing versus computeApiTruncationIndex:
- Compression refusal (
:146-147):compressionIndex !== -1 && targetIndex <= compressionIndex → -1. Without it, in a session that has been compacted the pick list still shows pre-compaction turns,occurrencecounts only the surviving API prompts, and the function returns a valid-but-wrong index →truncateHistorysilently deletes the wrong span. ink returns-1andAppContainer.tsx:3896-3911surfaces "Cannot rewind to a turn that was compressed". - First-turn shortcut (
:170-177):uiUserTurnCount === 0 → return startIndex. - Marker-less auto-compaction fail-loud (
:171-181): thestartIndex > getStartupContextLength(apiHistory)check that refuses to truncate to[prelude, summary, ack].
The position arithmetic itself is equivalent (occurrence = uiUserTurnCount + 1 yields the same index), so this is purely the guards. The docblock's own closing line — "Returns -1 when … the turn was absorbed by chat compression" — describes the case it structurally cannot detect. Also note nothing maps selectedTurnIndex → occurrence anywhere in the tree yet (git grep rewindApiCutPoint → this file plus its test), and the function is not in the re-export block at session-rewind.tsx:31-45, so whoever wires the caller has no signal that the ordinal must be computed against a compression-aware boundary.
5 participants
What this PR does
Fourth batch of the OpenTUI migration landing plan (#8662), stacked on the live-session & input batch (#10368): the dialog layer and command-routing infrastructure for the OpenTUI renderer.
Dialogs (19 modules): auth, extensions, MCP, memory-status, misc (effort/delete/resume/branch/rewind/diff/stats), model family (primary/fast/voice/vision/compaction/image), modes (approval-mode), permissions, settings (full settings + system-info panels), stats/skills, help overlay, the dialog arena host, and the folder-trust gate.
Command layer: the commands registry mapping every slash command to its result kinds and dialog targets, with
routeDialogToOpenTuitranslating the ink dialog-switch enum into typed OpenTUI dialog requests; the commands dispatcher (action interpreter connecting the slash gateway to the session and dialog layer); and the slash gateway routing raw user input through stacked-skills parsing and extension resolution.Session rewind: the history-folding model (
session-rewind-model.ts) that groups conversation turns into rewind checkpoints, plus the rewind viewer component.isUserTextContentinhistoryMapping.tsis exported so the rewind model can classify user turns without duplicating the predicate.33 new files, ~16 500 lines including tests; 56 test files / 887 tests, all green.
Everything is additive: no reachable ink code path is touched, nothing imports these modules yet, and the default renderer stays ink. The batch is self-contained on top of the live-session batch and adds no dependency on the backend or renderer-activation batches that follow. The dep-direction gate passes.
Why it's needed
The migration lands batch by batch so each unit is reviewable on its own merits. This batch brings the complete dialog surface and slash-command routing into the OpenTUI tree, leaving backend composition and renderer activation to complete the path to a flag-reachable renderer.
Reviewer Test Plan
All new code is unreachable from the running CLI (no wiring yet), so there is no user-visible behavior change to exercise; verification is build, types, and the test suite.
How to verify
Build all workspaces and run the typecheck — both are clean. Run the dependency-direction gate (
npm run check:tui-dep-direction): it passes. Run the CLI test suite:cd packages/cli && npx vitest run src/ui/opentui/— 56 files / 887 tests, all green. To confirm the ink path is untouched, note the diff adds files only underpackages/cli/src/ui/opentui/plus the one-lineisUserTextContentexport inhistoryMapping.ts.Evidence (Before & After)
N/A — no user-visible change; modules are not yet wired into the renderer dispatch.
Tested on
macOS (darwin arm64), Node.js v24, vitest v3.2.7. Full opentui subset: 56 files / 887 tests green. Typecheck (
tsc --noEmit) clean. Dependency-direction gate passes.Risk & Scope
Risk: Low. All 33 new files are under
packages/cli/src/ui/opentui/— an isolated namespace with no callers yet. The only changes outside that directory are: (1) a one-lineexportaddition tohistoryMapping.ts(additive, no behaviour change); (2) a method-rename fix ininput-prompt.tsx(setGeminiMdFileCount→setMemoryFileCount, aligning with upstream #9900). No ink code paths are touched.Scope: CLI package only. No changes to
packages/core, auth, providers, models, config, tools, or services.Linked Issues
Part of #8662 (OpenTUI migration, batch 4 of 7). Stacked on #10368.
Checklist
中文说明：OpenTUI 迁移第四批（Dialogs & commands）——19 个对话框模块（auth、extensions、MCP、memory-status、misc、模型族、modes、permissions、settings、stats/skills、帮助覆盖层、对话框 arena 宿主、文件夹信任门禁），命令注册表（slash 命令→对话框路由），命令分发器，slash 网关，以及会话 rewind 查看器与历史折叠模型。全部为增量代码，不触碰 ink 可达路径，默认渲染器仍为 ink。stacked 在 live-session 批（#10368）之上。构建、类型检查、依赖方向门禁全过，56 文件 / 887 用例全绿。