Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions packages/cli/src/llm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1198,6 +1198,33 @@ export async function main() {
// startInteractiveUI) and so the first paint uses the refined theme
// when the probe finishes in time.
await themeAutoDetectionComplete;
// Renderer dispatch for the ink→OpenTUI migration: QWEN_TUI_RENDERER
// selects the experimental backend only on a runtime that can drive it;
// every other case — including a failed renderer probe inside the entry —
// falls through to ink, which stays the default renderer.
const { selectTuiRenderer } = await import(
'./ui/opentui/renderer-selection.js'
);
if (selectTuiRenderer().renderer === 'opentui') {

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

[R1-2 · minor] The gate-rejection path — the one every currently-supported runtime takes — says nothing at all, and the PR description has the two fallback paths swapped.

witness: executed (gate probed with 11 inputs at head) + grep for any consumer of selection.reason.

selectTuiRenderer() returns a reason whose own doc says "Human-readable reason, for debug logging. Never shown to the user" (renderer-selection.ts:33), and renderer-selection.test.ts:101 even asserts its wording (toContain('native FFI')). But nothing reads it: this line destructures only .renderer, and there is no other selectTuiRenderer consumer in packages/cli/src outside the module and its test. So on Node 22/24 — i.e. .nvmrc, engines.node >=22.0.0, and every CI runner — QWEN_TUI_RENDERER=opentui is accepted by the user, rejected by the gate, and produces no output whatsoever: the CLI looks like the flag was ignored, which is the same symptom as the bug the PR's own "Before" section describes ("the flag was inert").

The asymmetry is what makes it worth a line, because the code already knows how to do this: the construction-failure path prints — start-opentui-ui.tsx:282-285 writes Warning: OpenTUI renderer unavailable — <msg> (falling back to ink) to stderr. So the loud path is the rare one and the mute path is the universal one, while the Risk section says the opposite ("a failed renderer probe falls back to ink silently"). Both statements can't be right; the silent-vs-loud split is worth deciding on purpose.

Cheapest resolution, either direction: if (selection.renderer !== 'opentui' && process.env.QWEN_TUI_RENDERER) debugLogger.info(selection.reason) at the call site — or drop the "for debug logging" promise from the docblock. Note the current if (selectTuiRenderer().renderer === 'opentui') form also recomputes the selection rather than holding it, so a named local would let the reason be logged without calling the gate twice.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 5b1dd15: the refusal path now logs selection.reason to the debug log (STARTUP tag), and the boot-failure path warns on stderr. The PR description's swapped fallback paths are corrected in the body.

const { startOpenTuiUI } = await import(

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This await import rejects when the OpenTUI FFI is unavailable on a gate-passing runtime: the entry chunk evaluates theme.ts at module scope (buildSyntax() → SyntaxStyle.fromStyles → resolveRenderLib() throws), so the failure happens before startOpenTuiUI's createCliRenderer try/catch can run — the ink fallback below is unreachable for this failure class and the process exits with An unexpected critical error occurred. Reproduced live at this head (scratch build with only MIN_NODE_VERSION relaxed to reach the FFI path on Node 24): QWEN_TUI_RENDERER=opentui → immediate crash, ink never boots. Real-world trigger: Node ≥ 26.4.0 without --experimental-ffi (required per @opentui/core's README, referenced in the gate comment, but never checked or passed), or a failed native-binary load on Bun/Node. Fix: try/catch around this import (and the call) → warn + fall through to ink.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 5b1dd15. The dispatch block now wraps the entry import and the boot call in try/catch: a module-evaluation failure (theme.ts SYNTAX → resolveRenderLib() throwing on a gate-passing runtime with a broken FFI) lands there, warns on stderr, and falls through to ink. The sibling contract inside the entry is pinned by tests in start-opentui-ui.test.tsx (renderer-creation rejection → false; boot-body rejection → teardown + false).

'./ui/opentui/start-opentui-ui.js'
);
const started = await startOpenTuiUI(
config,
settings,
startupWarnings,
process.cwd(),
initializationResult!,
{
postRenderConnectIde: deferIdeConnection,
extensionRefreshState,
},
);
if (started) {
clearCorruptionEnvVars();
return;
}
}
const { startInteractiveUI } = await import('./ui/startInteractiveUI.js');
await startInteractiveUI(
config,
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/ui/opentui/commands-registry.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -351,7 +351,7 @@ describe('OPEN_TUI_COMMAND_ROUTES (built-in registry parity)', () => {
['history', ['message']],
['restore', ['message', 'tool']],
['setup-github', ['tool']],
['goal', ['goal_control', 'message', 'submit_prompt']],
['goal', ['goal_control', 'message']],
['cd', ['confirm_action', 'message']],
['init', ['confirm_action', 'message', 'submit_prompt']],
];
Expand Down
5 changes: 1 addition & 4 deletions packages/cli/src/ui/opentui/commands-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -290,10 +290,7 @@ export const OPEN_TUI_COMMAND_ROUTES: readonly CommandRouteSpec[] = [
gatedBy: 'managed-memory',
},
{ name: 'forget', results: ['message'], gatedBy: 'managed-memory' },
{
name: 'goal',
results: ['goal_control', 'message', 'submit_prompt'],
},
{ name: 'goal', results: ['goal_control', 'message'] },
{ name: 'memory', results: ['dialog'], dialogs: ['memory'] },
{
name: 'model',
Expand Down
81 changes: 81 additions & 0 deletions packages/cli/src/ui/opentui/dialogs-confirm.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
/**
* @license
* Copyright 2026 Qwen
* SPDX-License-Identifier: Apache-2.0
*/

/**
* Outcome-option construction for the tool-confirmation dialog: the
* always-allow rows depend on the optional `hideAlwaysAllow` flag that only
* some confirmation-detail union members carry (ask_user_question has no such
* field), so the gate must narrow before reading it.
*/

import { describe, it, expect, vi } from 'vitest';

// theme.ts builds a SyntaxStyle at module scope, which needs the OpenTUI
// native FFI — unavailable in the test runtime. Stub the graphics surface.
vi.mock('@opentui/core', () => ({
SyntaxStyle: { fromStyles: () => ({}) },
MouseButton: { LEFT: 0 },
}));

import {
ToolConfirmationOutcome,
type ToolCallConfirmationDetails,
} from '@qwen-code/qwen-code-core';
import { buildOutcomeOptions } from './dialogs-confirm.js';

const onConfirm = async () => {};

const execDetails = (
hideAlwaysAllow?: boolean,
): ToolCallConfirmationDetails => ({
type: 'exec',
title: 'Run command',
onConfirm,
hideAlwaysAllow,
command: 'ls -la',
rootCommand: 'ls',
});

const askDetails = (): ToolCallConfirmationDetails => ({
type: 'ask_user_question',
title: 'A question',
questions: [
{
question: 'Pick one',
header: 'Choice',
options: [{ label: 'A', description: 'option a' }],
},
],
onConfirm,
});

describe('buildOutcomeOptions', () => {
it('offers allow-once, both always-allow rows, and cancel by default', () => {
const values = buildOutcomeOptions(execDetails()).map((o) => o.value);
expect(values).toEqual([
ToolConfirmationOutcome.ProceedOnce,
ToolConfirmationOutcome.ProceedAlwaysProject,
ToolConfirmationOutcome.ProceedAlwaysUser,
ToolConfirmationOutcome.Cancel,
]);
});

it('drops the always-allow rows when hideAlwaysAllow is set', () => {
const values = buildOutcomeOptions(execDetails(true)).map((o) => o.value);
expect(values).toEqual([
ToolConfirmationOutcome.ProceedOnce,
ToolConfirmationOutcome.Cancel,
]);
});

it('handles details without the hideAlwaysAllow field at all', () => {
// ask_user_question has no hideAlwaysAllow — reading it unguarded is a
// type error and would misrender the dialog for every question card.
const values = buildOutcomeOptions(askDetails()).map((o) => o.value);
expect(values).toContain(ToolConfirmationOutcome.ProceedOnce);
expect(values).toContain(ToolConfirmationOutcome.Cancel);
});
});
Loading
Loading