diff --git a/docs/design/2026-09-04-opentui-batch10-parity-closeout.md b/docs/design/2026-09-04-opentui-batch10-parity-closeout.md new file mode 100644 index 00000000000..d1096cd98b3 --- /dev/null +++ b/docs/design/2026-09-04-opentui-batch10-parity-closeout.md @@ -0,0 +1,230 @@ +# Batch 10 — OpenTUI parity closeout (dialogs, entry, composer, transcript, recording, resume) + +Design doc for the merged Batch 10/11 closeout. The two seam cuts were merged +into one PR on 2026-09-04 (author call, recorded in the ledger): U-33 and U-34 +both fill the `item-projection.ts` null arm and U-32/U-11 both touch the +steering drain path, so a seam split would open the same file to two reviews. +One item per commit keeps each round readable. Plan paragraph in the +[#8662 ledger](https://github.com/QwenLM/qwen-code/issues/8662). + +## Problem + +Three groups of gaps, all verified against main `9c320cb0cc` before planning: + +1. **Entry/composer gates (G-1, G-2, G-3)**: the auth dialog never auto-opens + (only the trigger is missing — the dialog itself is already mounted), no + follow-up suggestions exist under `ui/opentui/`, and the reserved + update-notification banner slot is unfilled. +2. **Projection/recording gaps (U-33, U-34, U-32, U-11)**: the `!` shell mode + is advertised but nonexistent, four dedicated-component history kinds + project to nothing, steered messages are visible (Batch 9) but not recorded + to the chat log, and the mid-turn queue lifecycle needs an authoritative + separator decision. +3. **The harness the batch's acceptance runs against** (U-31): the mem0-write + interactive spec self-spawns its PTY and never picks up the renderer + matrix, so its OpenTUI leg exercised ink — a false green inside the gating + leg, registered as an adjacent finding in the Batch 9 doc. + +U-31 was dispatched first because every other item's e2e acceptance runs +through that spec; the wiring immediately surfaced two app defects (U-36, +U-37) and two more behavioral divergences (all-cancelled continuation, +confirming-card flood), all fixed inside this batch rather than deferred — +the harness leg cannot go green while any of its five scenarios hangs or +floods. + +## Decision 1 — U-31: wire the self-spawning spec to the renderer matrix + +`external-context-mem0-write.test.ts` now calls `pickE2eRenderer` / +`resolveE2eCliCommand` / `e2eRendererEnv` like every matrix-aware spec, and +keeps its own `runInteractive` PTY harness (it drives scenarios +`InteractiveSession` cannot express: mid-turn hook confirmations with +content-visible bodies). The opentui leg carries `QWEN_TUI_RENDERER_STRICT`, +so a silent ink fallback fails the boot instead of passing as a false green. + +The wiring's first run turned the prediction from the Batch 9 doc into +measured fact: two app defects and an observation-channel trap, each covered +by the decisions below. + +## Decision 2 — the observation channel is the reconstructed screen, not raw PTY bytes + +OpenTUI redraws by cell diff: over a previously-blank background it emits the +glyph cell without the space cells a full-width row would carry, so +`waitForText` (a `stripAnsi(...).includes(...)` match on the raw byte stream, +with no whitespace normalization) on a multi-word row is a coin flip — four +scenarios timed out in the first wired run on strings the same code path had +rendered, and one passed only because diff history happened to align. The +spec's flow-gating assertions moved to xterm-headless screen reconstruction +(`waitForScreen` over the 110×38 viewport, polling 200 ms), the same channel +`InteractiveSession.screen()` uses, which is faithful on both legs. The ink +leg passed 5/5 with the screen-based spec unchanged, which pins the channel +as leg-neutral. Single-token matches (the suite-wide canary, the turn-done +marker) stay on `waitForText` deliberately: they are insensitive to the +mechanism and keep a second independent channel in the spec. + +## Decision 3 — U-36: config initialization is a shared once-guard, not a swallowed retry + +The OpenTUI leg hung on `Chat not initialized`: `config.initialize()` is +asynchronous while the first render is not, so a prompt submitted before +initialization settles reached the chat before the chat existed — and the +old `try { await config.initialize() } catch {}` in `livePromptEvents` +swallowed the failure, leaving no screen signal. Command loading already +calls `initialize()` concurrently and a second concurrent call rejects, so +"call it again and wait" was not available. + +`ensureConfigInitialized(config)` in `live-session.ts` keeps a +`WeakMap`-keyed shared promise: first caller wins, everyone awaits the same +settlement, a loser that observes `isInitialized()` proceeds. The entry calls +it fire-and-forget before the first render (so boot overlaps initialization +without serializing startup), and the turn path awaits the same promise. The +swallow is removed: a genuine initialization failure now propagates to the +screen instead of hanging. + +## Decision 4 — U-37: the MCP confirmation asks ink's question + +ink's MCP confirmation renders the question line +`Allow execution of MCP tool "{{tool}}" from server "{{server}}"?` +(`ToolConfirmationMessage.tsx` builds its own question; core supplies only +the title). OpenTUI showed title + display name + `server · tool` and no +question. The mcp case in `dialogs-confirm.tsx` gained the question line +through `t()`. The wired spec gates on exactly this row, so the fix is +e2e-observed on both legs. + +## Decision 5 — U-13: TextBody keeps its head and offers ctrl-s (scope widened from the notice) + +Registered as a cosmetic one-liner (add the `... N lines hidden ...` notice +`DiffBody` already has), the ledger's 2026-09-04 verification found the real +ink behavior is richer and OpenTUI's was worse than "no notice": +`TextBody` used a **tail** window (`tailWindow`) while ink's info confirmation +uses MaxSizedBox `overflowDirection 'bottom'` — it keeps the **head** and +hides the tail, with ShowMoreLines' hardcoded `Press ctrl-s to show more +lines` hint and a ctrl-s expand. A tail window also hides exactly the rows +that make a confirmation reviewable. + +`TextBody` now mirrors ink: `headWindowPhysical` in `messages.tsx`, the +hidden-tail indicator, the hint row, and a one-way ctrl-s expand +(`setExpanded(true)`, no collapse — ink's ShowMoreLines has no collapse +either). The window is **physical-height** aware: the mem0 confirmation's +`renderMemoryContentForConfirmation` is `JSON.stringify`, which collapses 142 +content lines into ONE logical row (~3.9k chars) that wraps to ~36 physical +rows — a logical-row window (the original `headWindow` draft) counts it as 1 +row and never fires. `headWindowPhysical` estimates `ceil(len/cols)` per +logical row and slices an over-budget row's head. Unit tests pin both the +multi-row and single-mega-row shapes. + +## Decision 6 — a whole-batch cancellation ends the turn without a follow-up request + +Found by scenario 2 of the wired spec: after the user rejects the hook +confirmation with Esc, OpenTUI sent a **second model request** where ink sends +none (`AssertionError: expected fakeModel.requests to have length 1 but got +2`). Debug-log forensics (token-count provenance lines 350 ms apart, +71 +tokens = the cancelled tool response) pinned the mechanism: the continuation +loop built `responseParts` from the cancelled call's `responseParts` and +submitted them as a ToolResult hop. + +ink pins the semantics (use-llm-stream.ts:5328-5358): when +`llmTools.every((tc) => tc.status === 'cancelled')` (and no duplicate +responses are pending), the cancelled functionResponses go to history via +`addHistory` and the turn ends. Ported into `livePromptEvents` as an +early-return **before** the steering drain, so steered texts that queued +during a cancelled batch stay parked for the post-turn queue instead of +riding a request that will never be sent. This is a pre-existing OpenTUI +behavioral difference, never observable before the wiring because scenario 2 +always died at the MCP gate first. + +Measured twice: the new unit test (fake scheduler honors `__cancelled`, +asserts one `sendMessageStream` call and one `addHistory` with +`role: 'user'`) goes red with `called 1 times, but got 2 times` when the +early-return is disabled, green when restored — the e2e failure's exact +signature. + +## Decision 7 — the confirmation is one surface: the dialog carries the payload + +Found by scenario 5: the MCP tool card's description is the tool's full args +JSON (core `mcp-tool.ts` returns `safeJsonStringify(this.params)` — ink +renders the same through ToolMessage), which wrapped to ~50 physical rows and +pushed the content-confirmation dialog off the 38-row viewport. Three root +layers, each discriminated by a progressively richer failure dump +(`.qwen/u31-opentui-fix{8,9,10,11*}-scenario5.log`): + +1. **The confirm transcript event never fired on the real path.** + `livePromptEvents` never emitted `type: 'confirm'` — only event-adapter's + pipeline did — so no card ever had `confirm: 'pending'`, and the pending + `(awaiting approval)` marker never rendered either. The fix emits + `type: 'confirm'` once per `awaiting_approval` entrance (deduped through + the same `waitingSeen` set that gates `onWaitingCall`, so a PreToolUse + 'ask' bounce re-marks the card), plus `type: 'confirm-resolved'` when a + call leaves the state — without the latter the card would keep claiming + "awaiting approval" while the call executes. A/B measured: without the + event the collapsed step times out on the full JSON flood; with it the + dialog appears. +2. **The expanded body overflowed.** ctrl-s worked but a full render + (~40 rows) inside a fixed alt-screen viewport clips at the bottom — + ink reaches the tail through terminal scrollback, which alt-screen does + not have. The expanded branch now renders a **tail** window + (`tailWindowPhysical`, the mirror of `headWindowPhysical`) budgeted as + terminal height minus `EXPANDED_BODY_RESERVE_ROWS` (dialog chrome plus the + transcript region that stays put above the dialog), with no hidden-lines + label — ink's observable expanded contract is "tail on screen, no + indicator". Unit tests pin the tail window's fits/mega-row/whole-row + shapes and the collapsed→expanded transition on the JSON payload. +3. **The card duplicates the payload the dialog already carries.** With the + dialog machinery working end-to-end, the only remaining predicate + violator was the card's own capped description and its + `... last N lines hidden ...` label lingering on screen after the body + expanded (ink renders the confirmation as ONE inline surface; its + expansion scrolls the card away). While `confirm === 'pending' && !done` + the card therefore renders **no description at all** — the dialog below + is the payload surface. + +The cap survives in generalized form: ink bounds EVERY tool card through the +static-area height distribution (MaxSizedBox), while OpenTUI's per-item +budget (`maxHistoryItemRows` = terminalHeight×4) never engages on a +single-logical-row JSON wrapping to dozens of physical rows — the resolved +card flooded all 38 rows and pushed the post-approval turn's output out of +the viewport (fix11 dump: `✓ mcp__external-context__…` + full JSON rows 02-37, +nothing else visible, despite `providerRequests=1 modelRequests=2` proving +the turn had completed). `capToolCardDescription` + `TOOL_CARD_DESCRIPTION_ROWS` +now bound every card description head to 5 wrapped rows with +`hiddenTailLinesLabel` summarizing the tail — the same character-based wrap +estimate as before, applied unconditionally instead of only when pending. The +per-item height distribution itself stays open as transcript-region work. + +## The nine ledger items (to be recorded as they land) + +U-6/G-1 (auth auto-open trigger), U-7/G-2 (follow-up suggestions), +U-9 (settings sub-dialog routing + `fillInput` owner), G-3 (update-check +wiring), U-33 (the `!` shell row), U-34 (four row shapes), U-32 (steer +recording), U-11 (queue separator decision), U-13 (landed — Decision 5). +Each lands as its own commit in this PR with its decision recorded here. + +## Coverage boundary + +Verified on the final state (both legs, `QWEN_CODE_LANG=en`): + +- `interactive/external-context-mem0-write.test.ts` — **5/5 on OpenTUI and + 5/5 on ink** (`.qwen/u31-opentui-final.log`, `.qwen/u31-ink-final.log`). + Scenario 5 exercises the whole Decision 7 chain on the OpenTUI leg: + collapsed head + literal markdown + `lines hidden` + ctrl-s → expanded tail + with `CONFIRM_TAIL` and no indicator → approval → turn completes. Each + fix layer was mutation-verified by a distinct red: no confirm event + (fix8), expanded clip (fix9), card label on screen (fix10), resolved-card + flood (fix11); the request counters inside the fix11 error message + (`providerRequests=1 modelRequests=2`) are what separated the render flood + from a real stall. +- **Post-approval sync changed on both legs**: `rig.waitForText` + reads the raw pty stream, which the OpenTUI leg redraws by cell diff — + rendered transcript rows never reliably appear there (the test already + used screen-based waits for the confirmation itself). The approve-branch + now polls request bodies (`fakeModel.requests.length >= 2`) — + renderer-independent, and the load-bearing post-approval semantics stay + pinned by the server-side assertions (`providerRequests`, + request-path/body). Ink behavior is unchanged in outcome. +- Unit: 138 tests across `messages.test.tsx` / `dialogs-confirm.test.tsx` / + `live-session.test.ts` / `live-session-model.test.ts` pin the window + arithmetic, the confirm/confirm-resolved sequence (incl. the hook-bounce + re-entrance), and the cap math — mock-level evidence only; the end-to-end + claim rests on the two e2e runs above. `npm run typecheck` and + `npm run lint` clean. +- Not covered: transcript per-item height distribution (the cap is the + interim bound), OpenTUI transcript autoscroll, and any renderer not in the + two tested legs. diff --git a/integration-tests/interactive/external-context-mem0-write.test.ts b/integration-tests/interactive/external-context-mem0-write.test.ts index e9f7415fd02..a5fe6dc5633 100644 --- a/integration-tests/interactive/external-context-mem0-write.test.ts +++ b/integration-tests/interactive/external-context-mem0-write.test.ts @@ -19,6 +19,11 @@ import { type FakeOpenAIServer, } from '../fake-openai-server.js'; import { TestRig, type } from '../test-helper.js'; +import { + e2eRendererEnv, + pickE2eRenderer, + resolveE2eCliCommand, +} from '../renderer-matrix.js'; const SANDBOX_MODE = process.env['QWEN_SANDBOX']?.toLowerCase().trim(); const IS_SANDBOX = Boolean( @@ -196,10 +201,14 @@ const ENVIRONMENT_KEYS = [ await type(ptyProcess, '\r'); if (scenario.expectsMcpConfirmation) { - expect( - await rig.waitForText('Allow execution of MCP tool', 30_000), + // Screen-based, not rig.waitForText: OpenTUI emits pty bytes by cell + // diff and drops spaces over previously-blank cells, so multi-word + // rows never appear verbatim in the raw stream on that leg. + await waitForScreen( + screen, + (value) => value.includes('Allow execution of MCP tool'), 'ordinary MCP confirmation did not appear', - ).toBe(true); + ); await type(ptyProcess, '\r'); } @@ -246,22 +255,32 @@ const ENVIRONMENT_KEYS = [ expect(confirmationScreen).toContain('`code-value`'); expect(confirmationScreen).toContain('under-value'); } else { - expect( - await rig.waitForText( - 'Save this exact content to the bound Mem0 repository memory?', - 30_000, - ), + await waitForScreen( + screen, + (value) => + value.includes( + 'Save this exact content to the bound Mem0 repository memory?', + ) && value.includes('Keep this'), 'content-visible Hook confirmation did not appear', - ).toBe(true); - expect(rig._interactiveOutput).toContain('Keep this'); + ); } await type(ptyProcess, scenario.approveWrite ? '\r' : '\x1b'); if (scenario.approveWrite) { - expect( - await rig.waitForText('MEM0_WRITE_E2E_DONE', 30_000), - 'fake model turn did not complete', - ).toBe(true); + // Sync on request bodies, not transcript text: the OpenTUI leg + // redraws by cell diff, so rendered rows never reliably appear in + // the raw pty stream (see the screen-based waits above). + const modelRequests = fakeModel.requests; + const turnCompleted = await rig.poll( + () => modelRequests.length >= 2, + 30_000, + 200, + ); + if (!turnCompleted) { + throw new Error( + `fake model turn did not complete. providerRequests=${providerRequests.length} modelRequests=${modelRequests.length}`, + ); + } } else { await new Promise((resolve) => setTimeout(resolve, 1000)); expect(fakeModel.requests).toHaveLength(1); @@ -432,6 +451,7 @@ function fakeMem0McpSource(integrationRoot: string): string { function runInteractive(rig: TestRig, ...args: string[]) { rig._interactiveOutput = ''; + const renderer = pickE2eRenderer(); const { Terminal } = xtermHeadless; const terminal = new Terminal({ cols: 110, @@ -441,14 +461,17 @@ function runInteractive(rig: TestRig, ...args: string[]) { }); let pendingWrite = Promise.resolve(); const ptyProcess = pty.spawn( - process.execPath, + resolveE2eCliCommand(renderer), [rig.bundlePath, '--no-chat-recording', ...args], { name: 'xterm-color', cols: 110, rows: 38, cwd: rig.testDir!, - env: process.env as Record, + env: { + ...process.env, + ...e2eRendererEnv(renderer), + } as Record, }, ); ptyProcess.onData((data) => { diff --git a/packages/cli/src/ui/opentui/dialogs-confirm.test.tsx b/packages/cli/src/ui/opentui/dialogs-confirm.test.tsx index c1cd279d58d..1511f57214d 100644 --- a/packages/cli/src/ui/opentui/dialogs-confirm.test.tsx +++ b/packages/cli/src/ui/opentui/dialogs-confirm.test.tsx @@ -60,6 +60,7 @@ vi.mock('@opentui/react', () => ({ useKeyboard: (handler: (key: unknown) => void) => { mocks.state.keyboardHandlers.push(handler); }, + useTerminalDimensions: () => ({ width: 110, height: 40 }), })); vi.mock('@opentui/react/jsx-runtime', () => mocks.buildJsxRuntime()); vi.mock('@opentui/react/jsx-dev-runtime', () => mocks.buildJsxRuntime()); @@ -135,7 +136,7 @@ describe('buildOutcomeOptions', () => { }); describe('OpenTuiToolConfirmation', () => { - function press(key: { name: string; sequence?: string }) { + function press(key: { name: string; sequence?: string; ctrl?: boolean }) { act(() => { for (const handler of mocks.state.keyboardHandlers) handler(key); }); @@ -231,4 +232,100 @@ describe('OpenTuiToolConfirmation', () => { undefined, ); }); + + it('renders the ink question line for MCP tool confirmations', () => { + const { container } = render( + {}} + />, + ); + expect(container.textContent).toContain( + 'Allow execution of MCP tool "context_remember" from server "external-context"?', + ); + }); + + it('keeps the head of a long info body and expands it on ctrl-s', () => { + const lines = [ + 'BODY_TOP', + ...Array.from( + { length: 24 }, + (_, index) => `body-line-${index.toString().padStart(2, '0')}`, + ), + 'BODY_TAIL', + ]; + const { container } = render( + {}} + />, + ); + const collapsed = container.textContent ?? ''; + expect(collapsed).toContain('BODY_TOP'); + expect(collapsed).toContain('... last 7 lines hidden ...'); + expect(collapsed).toContain('Press ctrl-s to show more lines'); + expect(collapsed).not.toContain('BODY_TAIL'); + + press({ name: 's', ctrl: true }); + const expanded = container.textContent ?? ''; + expect(expanded).toContain('BODY_TAIL'); + expect(expanded).not.toContain('lines hidden'); + expect(expanded).not.toContain('Press ctrl-s to show more lines'); + }); + + it('caps a single-line JSON payload by its wrapped height', () => { + const prompt = + 'Save this exact content to the bound Mem0 repository memory?\n' + + JSON.stringify(`CONFIRM_TOP ${'x'.repeat(3000)} CONFIRM_TAIL`); + const { container } = render( + {}} + />, + ); + const collapsed = container.textContent ?? ''; + expect(collapsed).toContain('CONFIRM_TOP'); + expect(collapsed).toContain('lines hidden'); + expect(collapsed).toContain('Press ctrl-s to show more lines'); + expect(collapsed).not.toContain('CONFIRM_TAIL'); + + press({ name: 's', ctrl: true }); + const expanded = container.textContent ?? ''; + // The expanded tail window surfaces the end of the payload (the alt-screen + // viewport has no scrollback, so the tail must be on screen) with no + // hidden-lines label left behind. + expect(expanded).toContain('CONFIRM_TAIL'); + expect(expanded).not.toContain('lines hidden'); + expect(expanded).not.toContain('Press ctrl-s to show more lines'); + }); }); diff --git a/packages/cli/src/ui/opentui/dialogs-confirm.tsx b/packages/cli/src/ui/opentui/dialogs-confirm.tsx index f4c512fdb45..4fef4a39075 100644 --- a/packages/cli/src/ui/opentui/dialogs-confirm.tsx +++ b/packages/cli/src/ui/opentui/dialogs-confirm.tsx @@ -42,7 +42,7 @@ import { type ToolCallConfirmationDetails, type ToolConfirmationPayload, } from '@qwen-code/qwen-code-core'; -import { useKeyboard } from '@opentui/react'; +import { useKeyboard, useTerminalDimensions } from '@opentui/react'; import { C } from './theme.js'; import { toOriginalKey } from './key-map.js'; import { @@ -53,7 +53,12 @@ import { type DialogListItem, } from './dialogs-shared.js'; import { renderDiffBody } from './diff-render.js'; -import { tailWindow } from './messages.js'; +import { + headWindowPhysical, + hiddenTailLinesLabel, + tailWindow, + tailWindowPhysical, +} from './messages.js'; import { sanitizeTerminalText } from '../utils/textUtils.js'; import type { ShellConfirmationResolution } from './commands-context.js'; import { t } from '../../i18n/index.js'; @@ -68,6 +73,15 @@ export interface PendingToolConfirmation { /** Max body rows before the tail window truncates (keeps dialogs bounded). */ const MAX_BODY_ROWS = 20; +/** + * Rows reserved above/below an EXPANDED body: dialog chrome (frame, title, + * options, footer) plus the transcript region that keeps its place above the + * dialog. The expanded tail window is budgeted as terminal height minus this + * reserve, so the end of the content — where the options still are — stays on + * screen (ink reaches the same visible outcome through terminal scrollback). + */ +const EXPANDED_BODY_RESERVE_ROWS = 20; + interface OutcomeOption { label: string; value: ToolConfirmationOutcome; @@ -127,18 +141,63 @@ function DiffBody({ fileDiff }: { fileDiff: string }) { ); } -/** Plain, sanitized, line-bounded text body. */ +/** + * Plain, sanitized text body. Long bodies keep their head (ink MaxSizedBox + * overflowDirection 'bottom' parity) with a hidden-tail indicator plus the + * ink ShowMoreLines hint; ctrl-s expands the full text. The cap counts + * WRAPPED rows — a single JSON-stringified payload line can wrap to dozens + * of physical rows, which a logical-row window never bounds. + */ function TextBody({ text }: { text: string }) { - const rows = useMemo(() => { - const clean = sanitizeTerminalText(text); - const window = tailWindow(clean.split('\n'), MAX_BODY_ROWS); - return window.visible; - }, [text]); + const [expanded, setExpanded] = useState(false); + const { width, height } = useTerminalDimensions(); + const rows = useMemo(() => sanitizeTerminalText(text).split('\n'), [text]); + const window = useMemo( + () => headWindowPhysical(rows, width, MAX_BODY_ROWS), + [rows, width], + ); + const expandedWindow = useMemo( + () => + tailWindowPhysical( + rows, + width, + Math.max(height - EXPANDED_BODY_RESERVE_ROWS, 1), + ), + [rows, width, height], + ); + + useKeyboard((key) => { + if (key.ctrl && toOriginalKey(key).name === 's') setExpanded(true); + }); + + if (expanded) { + // No hidden-lines indicator once expanded: ink's expanded screen shows + // the tail with no label (its head lives in terminal scrollback), and + // the alt-screen viewport has no scrollback to point at. + return ( + + {expandedWindow.visible.map((row, i) => ( + {row} + ))} + + ); + } + if (window.hiddenRows === 0) { + return ( + + {rows.map((row, i) => ( + {row} + ))} + + ); + } return ( - {rows.map((row, i) => ( + {window.visible.map((row, i) => ( {row} ))} + {hiddenTailLinesLabel(window.hiddenRows)} + Press ctrl-s to show more lines ); } @@ -180,6 +239,17 @@ function ConfirmationBody({ case 'mcp': return ( + + {sanitizeTerminalText( + t( + 'Allow execution of MCP tool "{{tool}}" from server "{{server}}"?', + { + tool: details.toolName, + server: details.serverName, + }, + ), + )} + {sanitizeTerminalText(details.toolDisplayName)} diff --git a/packages/cli/src/ui/opentui/event-adapter.ts b/packages/cli/src/ui/opentui/event-adapter.ts index e1f13ce8e68..16475fe9568 100644 --- a/packages/cli/src/ui/opentui/event-adapter.ts +++ b/packages/cli/src/ui/opentui/event-adapter.ts @@ -73,6 +73,9 @@ export type OpenTuiStreamEvent = visionBridgeNotice?: string; } | { type: 'confirm'; id: string; tool: string; title: string } + /** The call left awaiting_approval (approved, declined, or bounced): + * releases the transcript card's pending marker. */ + | { type: 'confirm-resolved'; id: string } /** Structured compression item (/compress command): rendered as the ink * CompressionMessage row (spinner/diamond + token counts) instead of the * flattened text projection. */ diff --git a/packages/cli/src/ui/opentui/live-session-model.ts b/packages/cli/src/ui/opentui/live-session-model.ts index e99ff2fe10b..3500fe8a0f7 100644 --- a/packages/cli/src/ui/opentui/live-session-model.ts +++ b/packages/cli/src/ui/opentui/live-session-model.ts @@ -317,6 +317,16 @@ export function foldLiveEvent( }); return items; } + case 'confirm-resolved': { + const i = findToolIndex(items, ev.id); + if (i >= 0) { + const t = items[i] as LiveToolItem; + if (t.confirm === 'pending') { + items[i] = { ...t, confirm: 'approved' }; + } + } + return items; + } case 'task-start': if (last?.kind === 'assistant' && last.streaming) items[items.length - 1] = { ...last, streaming: false }; diff --git a/packages/cli/src/ui/opentui/live-session.test.ts b/packages/cli/src/ui/opentui/live-session.test.ts index 6b34b324fec..16ae4daec78 100644 --- a/packages/cli/src/ui/opentui/live-session.test.ts +++ b/packages/cli/src/ui/opentui/live-session.test.ts @@ -25,6 +25,7 @@ import type { VisionBridgeModelSelection, } from '@qwen-code/qwen-code-core'; import { + ensureConfigInitialized, livePromptEvents, nextApprovalMode, resetPromptCountForTesting, @@ -137,7 +138,10 @@ vi.mock('@qwen-code/qwen-code-core', async (importOriginal) => { name: c.name ?? 'test_tool', args: c.args ?? {}, }, - status: 'success', + status: + ((c.args ?? {}) as { __cancelled?: boolean }).__cancelled === true + ? 'cancelled' + : 'success', response: { responseParts: [ { @@ -276,6 +280,90 @@ describe('livePromptEvents', () => { expect(options).toEqual({ type: SendMessageType.UserQuery }); }); + it('shares one initialization promise across callers', async () => { + const initialize = vi.fn(async () => {}); + const config = { initialize } as unknown as Config; + + await Promise.all([ + ensureConfigInitialized(config), + ensureConfigInitialized(config), + ]); + + expect(initialize).toHaveBeenCalledTimes(1); + }); + + it('waits for the shared initialization before the first send', async () => { + let resolveInitialize!: () => void; + const initialize = vi.fn( + () => + new Promise((resolve) => { + resolveInitialize = resolve; + }), + ); + const sendMessageStream = vi.fn(function* () { + yield { type: 'finished', value: {} }; + }); + const config = { + ...createFakeConfig(sendMessageStream), + initialize, + } as unknown as Config; + + const gen = livePromptEvents(config, 'hello'); + const drained = drain(gen); + + // The registry loader's initialize is still in flight; the turn must not + // send while it runs (the pre-fix behavior died with "Chat not + // initialized" here). + await new Promise((resolve) => setTimeout(resolve, 0)); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(sendMessageStream).not.toHaveBeenCalled(); + + resolveInitialize(); + await drained; + + expect(initialize).toHaveBeenCalledTimes(1); + expect(sendMessageStream).toHaveBeenCalledTimes(1); + }); + + it('proceeds when another caller already initialized the config', async () => { + const sendMessageStream = vi.fn(function* () { + yield { type: 'finished', value: {} }; + }); + const config = { + ...createFakeConfig(sendMessageStream), + initialize: vi.fn(async () => { + throw new Error('Config was already initialized'); + }), + getGeminiClient: () => ({ + sendMessageStream, + isInitialized: () => true, + }), + } as unknown as Config; + + await drain(livePromptEvents(config, 'hello')); + + expect(sendMessageStream).toHaveBeenCalledTimes(1); + }); + + it('surfaces a failed initialization instead of "Chat not initialized"', async () => { + const sendMessageStream = vi.fn(function* () {}); + const config = { + ...createFakeConfig(sendMessageStream), + initialize: vi.fn(async () => { + throw new Error('auth exploded'); + }), + getGeminiClient: () => ({ + sendMessageStream, + isInitialized: () => false, + }), + } as unknown as Config; + + await expect(drain(livePromptEvents(config, 'hello'))).rejects.toThrow( + 'auth exploded', + ); + expect(sendMessageStream).not.toHaveBeenCalled(); + }); + it('uses the ink promptId format and increments promptCount per turn', async () => { const sendMessageStream = vi.fn(function* () {}); const config = createFakeConfig(sendMessageStream); @@ -1265,6 +1353,55 @@ describe('livePromptEvents', () => { }); }); + it('yields a confirm event per awaiting_approval entrance (transcript pending marker)', async () => { + let calls = 0; + const sendMessageStream = vi.fn(function* (): Generator<{ + type: string; + value?: unknown; + }> { + calls += 1; + if (calls === 1) { + yield { + type: 'tool_call_request', + value: { + callId: 'b2', + name: 'run_shell_command', + args: { __bounceApproval: true }, + }, + }; + return; + } + yield { type: 'finished', value: {} }; + }); + const config = createFakeConfig(sendMessageStream); + + const events = (await drain( + livePromptEvents(config, 'q'), + )) as OpenTuiStreamEvent[]; + + // One event per entrance: the initial parking and the PreToolUse 'ask' + // bounce back under the same callId, with the pending marker released + // in between (the executing update drops the call from awaiting). + const confirms = events.filter( + (e) => e.type === 'confirm' || e.type === 'confirm-resolved', + ); + expect(confirms).toEqual([ + { + type: 'confirm', + id: 'b2', + tool: 'run_shell_command', + title: 'original', + }, + { type: 'confirm-resolved', id: 'b2' }, + { + type: 'confirm', + id: 'b2', + tool: 'run_shell_command', + title: 'Hook requested confirmation to run', + }, + ]); + }); + it('pushes the real invocation description once per callId (R1-104)', async () => { let calls = 0; const sendMessageStream = vi.fn(function* (): Generator<{ @@ -1329,6 +1466,34 @@ describe('livePromptEvents', () => { expect(events.some((e) => e.type === 'tool-description')).toBe(false); }); + it('ends the turn without a follow-up request when the whole batch was cancelled', async () => { + const sendMessageStream = oneToolBatchStream({ + callId: 'c1', + name: 'test_tool', + args: { __cancelled: true }, + }); + const addHistory = vi.fn(); + const config = { + ...createFakeConfig(sendMessageStream), + getGeminiClient: () => ({ sendMessageStream, addHistory }), + } as unknown as Config; + + await drain(livePromptEvents(config, 'run')); + + // ink use-llm-stream parity: cancelled responses go to history only, + // so the model is never asked to continue the turn. + expect(sendMessageStream).toHaveBeenCalledTimes(1); + expect(addHistory).toHaveBeenCalledTimes(1); + const content = addHistory.mock.calls[0][0] as { + role: string; + parts: Array<{ functionResponse?: { id?: string } }>; + }; + expect(content.role).toBe('user'); + expect(content.parts.some((p) => p.functionResponse?.id === 'c1')).toBe( + true, + ); + }); + describe('tool execution live output (outputUpdateHandler)', () => { it('streams tool-output events while the tool executes', async () => { const sendMessageStream = oneToolBatchStream({ diff --git a/packages/cli/src/ui/opentui/live-session.ts b/packages/cli/src/ui/opentui/live-session.ts index 121ce9f064d..4d33e470849 100644 --- a/packages/cli/src/ui/opentui/live-session.ts +++ b/packages/cli/src/ui/opentui/live-session.ts @@ -544,6 +544,34 @@ async function resolveSteeredPromptParts( return { parts, events, restore: [] }; } +/** + * Single shared `config.initialize()` for the OpenTUI app. Config.initialize() + * throws on re-entry instead of returning the in-flight promise (its private + * `initializationPromise` has no public accessor), so a submit landing while + * the command-registry loader's initialization is still running must await the + * SAME promise — awaiting `config.initialize()` directly rejects, and sending + * with `chat` unset dies with "Chat not initialized". The entry calls this + * once before the first render; the turn path awaits the cached promise. + */ +const initializationPromises = new WeakMap>(); + +export function ensureConfigInitialized(config: Config): Promise { + const pending = initializationPromises.get(config); + if (pending) return pending; + const started = config.initialize().catch((err) => { + initializationPromises.delete(config); + // Another caller (command loading) won the race. If its initialization + // settled with a chat, sending is safe; otherwise surface the failure. + if (config.getGeminiClient()?.isInitialized?.()) return; + throw err; + }); + // Fire-and-forget callers (the entry) must not trip unhandled-rejection + // handling; awaiters still receive the rejection. + started.catch(() => {}); + initializationPromises.set(config, started); + return started; +} + /** * Sends one user prompt through the real client and yields neutral events. * The caller (backend) drains this into the streaming model. @@ -559,11 +587,7 @@ export async function* livePromptEvents( signal?: AbortSignal, options?: LivePromptOptions, ): AsyncGenerator { - try { - await config.initialize(); - } catch { - /* already initialized by command loading / startup */ - } + await ensureConfigInitialized(config); const client = config.getGeminiClient(); const promptId = options?.promptId ?? nextLivePromptId(config); const abort = signal ?? new AbortController().signal; @@ -764,24 +788,39 @@ export async function* livePromptEvents( description: invocation.getDescription(), }); } - if (!options?.onWaitingCall) return; // Mirror the calls still awaiting approval: one that left the state // (resolved, or bounced back by a PreToolUse 'ask' hook under the - // same callId) must be able to surface its dialog again. + // same callId) must be able to surface its dialog again. Each + // entrance also marks the transcript card pending (event-adapter + // 'confirm' parity): it shows the awaiting marker and hides the + // card's duplicated description — the confirmation dialog below is + // the payload surface, and an MCP card's full JSON getDescription() + // would otherwise flood the column and push the dialog off-screen. const awaiting = new Set( calls .filter((c) => c.status === 'awaiting_approval') .map((c) => c.request.callId), ); for (const id of waitingSeen) { - if (!awaiting.has(id)) waitingSeen.delete(id); + if (!awaiting.has(id)) { + waitingSeen.delete(id); + // Release the transcript card's pending marker — without this it + // would keep claiming "awaiting approval" while the call runs. + live.push({ type: 'confirm-resolved', id }); + } } for (const c of calls) { if (c.status !== 'awaiting_approval') continue; const callId = c.request.callId; if (waitingSeen.has(callId)) continue; waitingSeen.add(callId); - options.onWaitingCall({ + live.push({ + type: 'confirm', + id: callId, + tool: c.request.name, + title: c.confirmationDetails.title, + }); + options?.onWaitingCall?.({ callId, name: c.request.name, confirmationDetails: c.confirmationDetails, @@ -833,6 +872,17 @@ export async function* livePromptEvents( }; if (resp?.responseParts) responseParts.push(...resp.responseParts); } + // ink use-llm-stream parity (:5328): when every tool in the batch was + // cancelled, the cancelled responses go to history only — no follow-up + // model request. Checked before the steering drain so surviving steered + // texts stay parked for the post-turn queue drain instead of riding a + // request that will never be sent. + if (completed.every((call) => call.status === 'cancelled')) { + if (responseParts.length > 0) { + await client.addHistory({ role: 'user', parts: responseParts }); + } + return; + } // Sampling boundary: drained steering rides after the tool responses as // genuine user content (original useGeminiStream mid-turn drain). if (!abort.aborted) { diff --git a/packages/cli/src/ui/opentui/messages.test.tsx b/packages/cli/src/ui/opentui/messages.test.tsx index 364b3937b05..a79146256df 100644 --- a/packages/cli/src/ui/opentui/messages.test.tsx +++ b/packages/cli/src/ui/opentui/messages.test.tsx @@ -23,10 +23,15 @@ vi.mock('@opentui/core', () => ({ import { GENERIC_TOOL_SUMMARIES, MAX_RESULT_DISPLAY_CHARACTERS, + TOOL_CARD_DESCRIPTION_ROWS, assistantMessageMeta, + capToolCardDescription, + headWindowPhysical, hiddenLinesLabel, + hiddenTailLinesLabel, maxHistoryItemRows, tailWindow, + tailWindowPhysical, thinkingMeta, toolCardDescription, toolCardName, @@ -36,6 +41,7 @@ import { truncateResultDisplayChars, truncateTokenLine, userMessageMeta, + STATUS_INDICATOR_WIDTH, } from './messages.js'; import { TOOL_STATUS } from '../constants.js'; import { C } from './theme.js'; @@ -183,6 +189,67 @@ describe('long-content caps (ink MaxSizedBox parity)', () => { expect(hiddenLinesLabel(4779)).toBe('... first 4779 lines hidden ...'); }); + it('keeps everything when the physical height fits', () => { + const rows = ['a'.repeat(150), 'b']; + expect(headWindowPhysical(rows, 102, 20)).toEqual({ + visible: rows, + hiddenRows: 0, + }); + }); + + it('caps a single over-long logical row by its wrapped height', () => { + const win = headWindowPhysical(['head', 'x'.repeat(5000)], 102, 20); + expect(win.visible[0]).toBe('head'); + expect(win.visible[1]).toBe('x'.repeat(18 * 100)); + expect(win.visible).toHaveLength(2); + // 1 + 50 physical rows total, 19 budgeted for content. + expect(win.hiddenRows).toBe(32); + }); + + it('keeps whole rows while they fit and slices the overflowing row', () => { + const rows = Array.from( + { length: 15 }, + (_, i) => `${i % 10}`.repeat(150), // 2 physical rows each at 100 cols + ); + const win = headWindowPhysical(rows, 102, 20); + expect(win.visible).toHaveLength(10); + expect(win.visible[9]).toBe('9'.repeat(100)); + // 30 physical rows total, 19 budgeted for content. + expect(win.hiddenRows).toBe(11); + }); + + it('renders the ink bottom-overflow hidden-tail indicator', () => { + expect(hiddenTailLinesLabel(1)).toBe('... last 1 line hidden ...'); + expect(hiddenTailLinesLabel(4779)).toBe('... last 4779 lines hidden ...'); + }); + + it('keeps everything when the physical height fits the tail budget', () => { + const rows = ['a'.repeat(150), 'b']; + expect(tailWindowPhysical(rows, 102, 20)).toEqual({ + visible: rows, + hiddenRows: 0, + }); + }); + + it('caps a single over-long logical row by its wrapped tail', () => { + const win = tailWindowPhysical(['head', 'x'.repeat(5000)], 102, 20); + expect(win.visible).toEqual(['x'.repeat(20 * 100)]); + // 1 + 50 physical rows total; the mega row's tail fills the budget. + expect(win.hiddenRows).toBe(31); + }); + + it('keeps the last whole rows whose height fits', () => { + const rows = Array.from( + { length: 15 }, + (_, i) => `${i % 10}`.repeat(150), // 2 physical rows each at 100 cols + ); + const win = tailWindowPhysical(rows, 102, 20); + expect(win.visible).toHaveLength(10); + expect(win.visible[0]).toBe('5'.repeat(150)); + expect(win.visible[9]).toBe('4'.repeat(150)); + expect(win.hiddenRows).toBe(10); + }); + it('truncates over-long results to the trailing characters', () => { const short = 'short output'; expect(truncateResultDisplayChars(short)).toBe(short); @@ -193,6 +260,37 @@ describe('long-content caps (ink MaxSizedBox parity)', () => { }); }); +describe('capToolCardDescription (transcript card flood bound)', () => { + it('leaves a description that fits the card budget untouched', () => { + const desc = 'Save this exact content to the bound memory?'; + expect( + capToolCardDescription( + desc, + 'mcp__mem0', + 110, + TOOL_CARD_DESCRIPTION_ROWS, + ), + ).toEqual({ description: desc, hiddenRows: 0 }); + }); + + it('keeps the head of an over-long description and counts the hidden rows', () => { + const desc = 'x'.repeat(1000); + const cols = 110 - STATUS_INDICATOR_WIDTH; + const cap = capToolCardDescription( + desc, + 'mcp__mem0', + 110, + TOOL_CARD_DESCRIPTION_ROWS, + ); + // The label row shares the budget: 4 description rows, the first one + // hosting the name inline. + const rows = Math.ceil(('mcp__mem0'.length + 1 + desc.length) / cols); + expect(cap.description).toBe('x'.repeat(4 * cols - 'mcp__mem0'.length - 1)); + expect(cap.hiddenRows).toBe(rows - 4); + expect(cap.hiddenRows).toBeGreaterThan(0); + }); +}); + describe('message meta (ink glyph/color parity)', () => { it('keeps the user/assistant prefixes', () => { expect(userMessageMeta().glyph).toBe('>'); diff --git a/packages/cli/src/ui/opentui/messages.tsx b/packages/cli/src/ui/opentui/messages.tsx index 48d442cf1ce..b6eecf096ea 100644 --- a/packages/cli/src/ui/opentui/messages.tsx +++ b/packages/cli/src/ui/opentui/messages.tsx @@ -122,6 +122,118 @@ export function hiddenLinesLabel(hiddenCount: number): string { return `... first ${hiddenCount} line${hiddenCount === 1 ? '' : 's'} hidden ...`; } +/** + * Physical-height head window (ink MaxSizedBox overflowDirection 'bottom' + * parity): the cap counts WRAPPED rows at width - 2 columns, because a + * single logical row — e.g. a JSON.stringify'd confirmation payload — can + * wrap to dozens of physical rows that a logical-row window never bounds. + * An over-budget tail logical row is sliced to its head characters; the + * hidden tail is summarized by hiddenTailLinesLabel. + */ +export function headWindowPhysical( + rows: readonly string[], + width: number, + maxRows: number, +): { visible: string[]; hiddenRows: number } { + const cols = Math.max(width - 2, 10); + const height = (row: string) => Math.max(1, Math.ceil(row.length / cols)); + const total = rows.reduce((sum, row) => sum + height(row), 0); + if (total <= maxRows) return { visible: [...rows], hiddenRows: 0 }; + const budget = Math.max(maxRows - 1, 1); + const visible: string[] = []; + let used = 0; + for (const row of rows) { + const h = height(row); + if (used + h <= budget) { + visible.push(row); + used += h; + continue; + } + const remaining = budget - used; + if (remaining > 0) { + visible.push(row.slice(0, remaining * cols)); + used = budget; + } + break; + } + return { visible, hiddenRows: Math.max(total - used, 1) }; +} + +/** ink MaxSizedBox bottom-overflow hidden-tail indicator text. */ +export function hiddenTailLinesLabel(hiddenCount: number): string { + return `... last ${hiddenCount} line${hiddenCount === 1 ? '' : 's'} hidden ...`; +} + +/** + * Physical-height tail window — the expand-side mirror of headWindowPhysical: + * keeps the LAST rows whose wrapped height fits the budget, slicing an + * over-budget head logical row to its tail characters. OpenTUI paints a + * fixed alt-screen viewport, so an expanded dialog body taller than the + * screen must surface its tail (where the content ends) instead of letting + * the screen clip it away. + */ +export function tailWindowPhysical( + rows: readonly string[], + width: number, + maxRows: number, +): { visible: string[]; hiddenRows: number } { + const cols = Math.max(width - 2, 10); + const height = (row: string) => Math.max(1, Math.ceil(row.length / cols)); + const total = rows.reduce((sum, row) => sum + height(row), 0); + if (total <= maxRows) return { visible: [...rows], hiddenRows: 0 }; + const budget = Math.max(maxRows, 1); + const visible: string[] = []; + let used = 0; + for (let i = rows.length - 1; i >= 0; i -= 1) { + const row = rows[i]; + const h = height(row); + if (used + h <= budget) { + visible.unshift(row); + used += h; + continue; + } + const remaining = budget - used; + if (remaining > 0) { + visible.unshift(row.slice(row.length - remaining * cols)); + used = budget; + } + break; + } + return { visible, hiddenRows: Math.max(total - used, 1) }; +} + +/** + * Row budget for a tool card's inline description. Ink bounds an over-tall + * card through the static-area height distribution (MaxSizedBox); OpenTUI + * caps the description head here until that distribution exists — without it + * an MCP tool's whole args JSON (one logical row wrapping to dozens of + * physical rows) floods the transcript column. + */ +export const TOOL_CARD_DESCRIPTION_ROWS = 5; + +/** + * Keeps the head of a description that would wrap past `maxRows` at the + * given width; the hidden tail is summarized by hiddenTailLinesLabel. The + * estimate is character-based (name + description wrap inside + * width - STATUS_INDICATOR_WIDTH columns), which is intentionally coarse. + */ +export function capToolCardDescription( + description: string, + name: string, + width: number, + maxRows: number, +): { description: string; hiddenRows: number } { + const cols = Math.max(width - STATUS_INDICATOR_WIDTH, 10); + const rows = Math.ceil((name.length + 1 + description.length) / cols); + if (rows <= maxRows) return { description, hiddenRows: 0 }; + const descRows = Math.max(maxRows - 1, 1); + const visibleChars = Math.max(descRows * cols - name.length - 1, 0); + return { + description: description.slice(0, visibleChars), + hiddenRows: Math.max(rows - descRows, 1), + }; +} + /** * Tool-card naming/status parity with the original ToolMessage: a card line * is `{glyph} {DisplayName} {description}`, where the display name comes diff --git a/packages/cli/src/ui/opentui/start-opentui-ui.tsx b/packages/cli/src/ui/opentui/start-opentui-ui.tsx index 3cb7eb2f18c..1147ce8e060 100644 --- a/packages/cli/src/ui/opentui/start-opentui-ui.tsx +++ b/packages/cli/src/ui/opentui/start-opentui-ui.tsx @@ -75,6 +75,7 @@ import { OpenTuiApp } from './opentui-app-shell.js'; import { OpenTuiRuntime } from './opentui-runtime.js'; import { OpenTuiTranscriptView } from './transcript-view.js'; import { useOpenTuiLiveTurn, type OpenTuiSubmitOptions } from './live-turn.js'; +import { ensureConfigInitialized } from './live-session.js'; import { consumeLastRenderError } from './opentui-error-boundary.js'; import { createExitGuard, exitGuardHint } from './exit-guard.js'; import { EXIT_CODE_INTERRUPT, exitSession } from './exit-lifecycle.js'; @@ -319,6 +320,10 @@ export async function startOpenTuiUI( const root = createRoot(renderer); let runtime: OpenTuiRuntime | null = null; try { + // Own the single awaited initialization before anything can load commands + // or submit: the registry loader's config.initialize() re-entry rejects, + // so the turn path must await this same shared promise instead. + void ensureConfigInitialized(config); const version = await getCliVersion(); if ( !settings.merged.ui?.hideWindowTitle && diff --git a/packages/cli/src/ui/opentui/transcript-view.tsx b/packages/cli/src/ui/opentui/transcript-view.tsx index e8ea808103f..021981b1f0f 100644 --- a/packages/cli/src/ui/opentui/transcript-view.tsx +++ b/packages/cli/src/ui/opentui/transcript-view.tsx @@ -21,9 +21,12 @@ import { C, SYNTAX } from './theme.js'; import { AnsiRows, MESSAGE_ICON, + TOOL_CARD_DESCRIPTION_ROWS, TodoRows, assistantMessageMeta, + capToolCardDescription, hiddenLinesLabel, + hiddenTailLinesLabel, maxHistoryItemRows, selectionProps, STATUS_INDICATOR_WIDTH, @@ -225,8 +228,21 @@ function ToolCard({ }) { const status = toolStatusMeta(item); const name = toolCardName(item.tool); + // While the call awaits approval the dialog below carries the payload — + // ink renders the confirmation as one surface there, and a duplicated + // description would flood the column and keep its hidden-lines label on + // screen after the body expands. Otherwise the description is capped to + // TOOL_CARD_DESCRIPTION_ROWS wrapped rows (ink MaxSizedBox parity). const description = - item.description ?? toolCardDescription(item.tool, item.args); + item.confirm === 'pending' && !item.done + ? '' + : (item.description ?? toolCardDescription(item.tool, item.args)); + const cap = capToolCardDescription( + description, + name, + width, + TOOL_CARD_DESCRIPTION_ROWS, + ); const suffix = toolCardSummarySuffix(item.done, item.summary); return ( @@ -242,13 +258,16 @@ function ToolCard({ {name} - {description ? ( + {cap.description ? ( - {` ${sanitizeTerminalText(description)}`} + {` ${sanitizeTerminalText(cap.description)}`} ) : null} {suffix ? {sanitizeTerminalText(suffix)} : null} + {cap.hiddenRows > 0 && ( + {hiddenTailLinesLabel(cap.hiddenRows)} + )} {item.confirm === 'pending' && !item.done ? ( (awaiting approval) ) : null}