diff --git a/src/cli/commands/dev/__tests__/tty-guard.test.ts b/src/cli/commands/dev/__tests__/tty-guard.test.ts new file mode 100644 index 000000000..3c19b763d --- /dev/null +++ b/src/cli/commands/dev/__tests__/tty-guard.test.ts @@ -0,0 +1,84 @@ +import { launchTuiDevScreenWithPicker } from '../browser-mode'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Drive the REAL requireTTY by toggling process.stdin.isTTY / process.stdout.isTTY +// and spying on process.exit / console.error. Only the I/O boundaries are mocked: +// the Ink renderer and the DevScreen the picker renders. This pins the guard that +// PR #1640 centralized — the browser-mode harness picker must refuse to render in a +// non-TTY context instead of throwing Ink's "Raw mode is not supported" stack trace. +const mockRender = vi.fn((..._args: unknown[]) => ({ + unmount: vi.fn(), + waitUntilExit: () => Promise.resolve(), +})); + +vi.mock('ink', () => ({ + render: (...args: unknown[]) => mockRender(...args), +})); + +vi.mock('../../../tui/screens/dev/DevScreen', () => ({ DevScreen: () => null })); + +describe('dev browser-mode picker TTY guard', () => { + let exitSpy: ReturnType; + let errorSpy: ReturnType; + let stdoutWriteSpy: ReturnType; + const origStdinIsTTY = process.stdin.isTTY; + const origStdoutIsTTY = process.stdout.isTTY; + + beforeEach(() => { + exitSpy = vi.spyOn(process, 'exit').mockImplementation((code?: string | number | null) => { + throw new Error(`process.exit(${code})`); + }); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + stdoutWriteSpy = vi.spyOn(process.stdout, 'write').mockImplementation(() => true); + }); + + afterEach(() => { + process.stdin.isTTY = origStdinIsTTY; + process.stdout.isTTY = origStdoutIsTTY; + exitSpy.mockRestore(); + errorSpy.mockRestore(); + stdoutWriteSpy.mockRestore(); + vi.clearAllMocks(); + }); + + it('exits with code 1 and never renders in a non-TTY context', async () => { + process.stdin.isTTY = false; + process.stdout.isTTY = false; + + await expect(launchTuiDevScreenWithPicker('/tmp/project')).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('requires an interactive terminal')); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('exits when only stdin is not a TTY', async () => { + process.stdin.isTTY = false; + process.stdout.isTTY = true; + + await expect(launchTuiDevScreenWithPicker('/tmp/project')).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('exits when only stdout is not a TTY', async () => { + process.stdin.isTTY = true; + process.stdout.isTTY = false; + + await expect(launchTuiDevScreenWithPicker('/tmp/project')).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('renders the picker when both stdin and stdout are TTYs', async () => { + process.stdin.isTTY = true; + process.stdout.isTTY = true; + + await launchTuiDevScreenWithPicker('/tmp/project'); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(mockRender).toHaveBeenCalled(); + }); +}); diff --git a/src/cli/commands/dev/browser-mode.ts b/src/cli/commands/dev/browser-mode.ts index b40600e21..33ad73ebb 100644 --- a/src/cli/commands/dev/browser-mode.ts +++ b/src/cli/commands/dev/browser-mode.ts @@ -15,6 +15,7 @@ import { listMemoryRecords, retrieveMemoryRecords } from '../../operations/memor import { loadDeployedProjectConfig, resolveAgentOrHarness } from '../../operations/resolve-agent'; import { fetchTraceRecords, listTraces } from '../../operations/traces'; import { LayoutProvider } from '../../tui/context'; +import { requireTTY } from '../../tui/guards'; import { runCliDeploy } from '../deploy/progress'; import { render } from 'ink'; import path from 'node:path'; @@ -328,6 +329,7 @@ export async function launchTuiDevScreenWithPicker( workingDir: string, options?: { skipDeploy?: boolean } ): Promise { + requireTTY(); process.stdout.write(ENTER_ALT_SCREEN); const exitAltScreen = () => { diff --git a/src/cli/commands/export/__tests__/tty-guard.test.ts b/src/cli/commands/export/__tests__/tty-guard.test.ts new file mode 100644 index 000000000..294048798 --- /dev/null +++ b/src/cli/commands/export/__tests__/tty-guard.test.ts @@ -0,0 +1,87 @@ +import { registerExport } from '../index'; +import { Command } from '@commander-js/extra-typings'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Drive the REAL requireTTY (centralized inside renderTUI) by toggling +// process.stdin.isTTY / process.stdout.isTTY and spying on process.exit / +// console.error. Only the I/O boundaries renderTUI touches are mocked: the +// Ink renderer, telemetry, and post-command notices. The guard runs first in +// renderTUI, so a non-TTY context exits before any of these are reached. +const mockRender = vi.fn((..._args: unknown[]) => ({ waitUntilExit: () => Promise.resolve() })); + +vi.mock('ink', () => ({ + render: (...args: unknown[]) => mockRender(...args), +})); + +vi.mock('../../../telemetry', () => ({ + TelemetryClientAccessor: { + init: vi.fn().mockResolvedValue(undefined), + get: vi.fn().mockResolvedValue(null), + }, +})); + +vi.mock('../../../notices', () => ({ + printPostCommandNotices: vi.fn().mockResolvedValue(undefined), +})); + +describe('export harness TTY guard', () => { + let program: Command; + let exitSpy: ReturnType; + let errorSpy: ReturnType; + let writeSpy: ReturnType; + const origStdinIsTTY = process.stdin.isTTY; + const origStdoutIsTTY = process.stdout.isTTY; + + beforeEach(() => { + program = new Command(); + program.exitOverride(); + registerExport(program); + + // Swallow the alt-screen escape sequences renderTUI writes on the TTY path. + writeSpy = vi.spyOn(process.stdout, 'write').mockReturnValue(true); + exitSpy = vi.spyOn(process, 'exit').mockImplementation((code?: string | number | null) => { + throw new Error(`process.exit(${code})`); + }); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + }); + + afterEach(() => { + process.stdin.isTTY = origStdinIsTTY; + process.stdout.isTTY = origStdoutIsTTY; + writeSpy.mockRestore(); + exitSpy.mockRestore(); + errorSpy.mockRestore(); + vi.clearAllMocks(); + }); + + it('exits with code 1 and never renders the TUI in a non-TTY context', async () => { + process.stdin.isTTY = false; + process.stdout.isTTY = false; + + await expect(program.parseAsync(['export', 'harness'], { from: 'user' })).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('requires an interactive terminal')); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('exits when only stdout is not a TTY', async () => { + process.stdin.isTTY = true; + process.stdout.isTTY = false; + + await expect(program.parseAsync(['export', 'harness'], { from: 'user' })).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('renders the TUI when both stdin and stdout are TTYs', async () => { + process.stdin.isTTY = true; + process.stdout.isTTY = true; + + await program.parseAsync(['export', 'harness'], { from: 'user' }); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(mockRender).toHaveBeenCalled(); + }); +}); diff --git a/src/cli/commands/export/index.ts b/src/cli/commands/export/index.ts index 24587eb82..5e11c35e9 100644 --- a/src/cli/commands/export/index.ts +++ b/src/cli/commands/export/index.ts @@ -29,6 +29,7 @@ export function registerExport(program: Command): void { console.log(JSON.stringify({ success: false, error: '--name is required in non-interactive mode' })); process.exit(1); } + // renderTUI() guards for an interactive terminal before rendering. await renderTUI({ initialRoute: { name: 'export-harness' }, actionOnBack: 'exit' }); return; } diff --git a/src/cli/commands/import/__tests__/tty-guard.test.ts b/src/cli/commands/import/__tests__/tty-guard.test.ts new file mode 100644 index 000000000..218664cb2 --- /dev/null +++ b/src/cli/commands/import/__tests__/tty-guard.test.ts @@ -0,0 +1,81 @@ +import { registerImport } from '../command'; +import { Command } from '@commander-js/extra-typings'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Drive the REAL requireTTY by toggling process.stdin.isTTY / process.stdout.isTTY +// and spying on process.exit / console.error. Only the I/O boundaries are mocked: +// the Ink renderer and the ImportFlow screen. requireProject is stubbed to a no-op +// so the TTY guard (which runs after it) is what we exercise here. +const mockRender = vi.fn((..._args: unknown[]) => ({ clear: vi.fn(), unmount: vi.fn() })); + +vi.mock('../../../tui/guards', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + requireProject: vi.fn(), + }; +}); + +vi.mock('ink', () => ({ + render: (...args: unknown[]) => mockRender(...args), +})); + +vi.mock('../../../tui/screens/import', () => ({ ImportFlow: () => null })); + +describe('import non-source TTY guard', () => { + let program: Command; + let exitSpy: ReturnType; + let errorSpy: ReturnType; + const origStdinIsTTY = process.stdin.isTTY; + const origStdoutIsTTY = process.stdout.isTTY; + + beforeEach(() => { + program = new Command(); + program.exitOverride(); + registerImport(program); + + exitSpy = vi.spyOn(process, 'exit').mockImplementation((code?: string | number | null) => { + throw new Error(`process.exit(${code})`); + }); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + }); + + afterEach(() => { + process.stdin.isTTY = origStdinIsTTY; + process.stdout.isTTY = origStdoutIsTTY; + exitSpy.mockRestore(); + errorSpy.mockRestore(); + vi.clearAllMocks(); + }); + + it('exits with code 1 and does not render in a non-TTY context', async () => { + process.stdin.isTTY = false; + process.stdout.isTTY = false; + + await expect(program.parseAsync(['import'], { from: 'user' })).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('requires an interactive terminal')); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('exits when only stdin is not a TTY', async () => { + process.stdin.isTTY = false; + process.stdout.isTTY = true; + + await expect(program.parseAsync(['import'], { from: 'user' })).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('renders the interactive flow when both stdin and stdout are TTYs', async () => { + process.stdin.isTTY = true; + process.stdout.isTTY = true; + + await program.parseAsync(['import'], { from: 'user' }); + + expect(exitSpy).not.toHaveBeenCalled(); + expect(mockRender).toHaveBeenCalled(); + }); +}); diff --git a/src/cli/commands/import/command.ts b/src/cli/commands/import/command.ts index 965fc8767..5172f5d13 100644 --- a/src/cli/commands/import/command.ts +++ b/src/cli/commands/import/command.ts @@ -24,9 +24,12 @@ export const registerImport = (program: Command) => { .option('-y, --yes', 'Auto-confirm prompts') .action(async (cliOptions: { source?: string; target?: string; yes?: boolean }) => { if (!cliOptions.source) { - // No --source and no subcommand — launch interactive TUI - const { requireProject } = await import('../../tui/guards/project'); + // No --source and no subcommand — launch interactive TUI. + // requireProject() first so users who haven't cd'd to a project get the + // more actionable error before the TTY check (consistent with `view`). + const { requireProject, requireTTY } = await import('../../tui/guards'); requireProject(); + requireTTY(); const { render } = await import('ink'); const React = await import('react'); const { ImportFlow } = await import('../../tui/screens/import'); diff --git a/src/cli/commands/view/__tests__/tty-guard.test.ts b/src/cli/commands/view/__tests__/tty-guard.test.ts new file mode 100644 index 000000000..b7a19ec41 --- /dev/null +++ b/src/cli/commands/view/__tests__/tty-guard.test.ts @@ -0,0 +1,106 @@ +import { registerView } from '../command'; +import { Command } from '@commander-js/extra-typings'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +// Drive the REAL requireTTY by toggling process.stdin.isTTY / process.stdout.isTTY +// and spying on process.exit / console.error. Only the I/O boundaries are mocked: +// the Ink renderer and the TUI screens. requireProject is stubbed to a no-op so +// the TTY guard (which runs after it inside launchTuiList/launchTuiDetail) is what +// we exercise. All four job types are parameterized so a future refactor that +// moved the guard into a per-type branch would be caught. +const mockRender = vi.fn(); + +vi.mock('../../../tui/guards', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + requireProject: vi.fn(), + }; +}); + +vi.mock('ink', () => ({ + render: (...args: unknown[]) => mockRender(...args), +})); + +vi.mock('../../../tui/screens/recommendation', () => ({ RecommendationHistoryScreen: () => null })); +vi.mock('../../../tui/screens/run-eval', () => ({ BatchEvalHistoryScreen: () => null })); +vi.mock('../../../tui/screens/run-ab-test', () => ({ ABTestJobsHistoryScreen: () => null })); +vi.mock('../../../tui/screens/insights-jobs', () => ({ InsightsJobsScreen: () => null })); +vi.mock('../JobDetailScreen', () => ({ JobDetailScreen: () => null })); + +const JOB_TYPES = ['recommendation', 'batch-evaluation', 'ab-test', 'insights'] as const; + +describe('view TTY guard', () => { + let program: Command; + let exitSpy: ReturnType; + let errorSpy: ReturnType; + const origStdinIsTTY = process.stdin.isTTY; + const origStdoutIsTTY = process.stdout.isTTY; + + beforeEach(() => { + program = new Command(); + program.exitOverride(); + registerView(program); + + exitSpy = vi.spyOn(process, 'exit').mockImplementation((code?: string | number | null) => { + throw new Error(`process.exit(${code})`); + }); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); + }); + + afterEach(() => { + process.stdin.isTTY = origStdinIsTTY; + process.stdout.isTTY = origStdoutIsTTY; + exitSpy.mockRestore(); + errorSpy.mockRestore(); + vi.clearAllMocks(); + }); + + describe.each(JOB_TYPES)('view %s', type => { + it('guards the interactive list — exits and never renders in a non-TTY context', async () => { + process.stdin.isTTY = false; + process.stdout.isTTY = false; + + await expect(program.parseAsync(['view', type], { from: 'user' })).rejects.toThrow('process.exit(1)'); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('requires an interactive terminal')); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('renders the interactive list when both stdin and stdout are TTYs', async () => { + process.stdin.isTTY = true; + process.stdout.isTTY = true; + + // launchTuiList resolves a never-settling promise (the TUI owns the event + // loop until the user exits), so race the parse against the render rather + // than awaiting it. + void program.parseAsync(['view', type], { from: 'user' }); + await vi.waitFor(() => expect(mockRender).toHaveBeenCalled()); + + expect(exitSpy).not.toHaveBeenCalled(); + }); + }); + + it('guards the interactive detail — exits and never renders in a non-TTY context', async () => { + process.stdin.isTTY = false; + process.stdout.isTTY = true; + + await expect(program.parseAsync(['view', 'recommendation', 'rec-1'], { from: 'user' })).rejects.toThrow( + 'process.exit(1)' + ); + + expect(exitSpy).toHaveBeenCalledWith(1); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('renders the interactive detail when both stdin and stdout are TTYs', async () => { + process.stdin.isTTY = true; + process.stdout.isTTY = true; + + void program.parseAsync(['view', 'recommendation', 'rec-1'], { from: 'user' }); + await vi.waitFor(() => expect(mockRender).toHaveBeenCalled()); + + expect(exitSpy).not.toHaveBeenCalled(); + }); +}); diff --git a/src/cli/commands/view/command.tsx b/src/cli/commands/view/command.tsx index e656e8b3c..5254ce705 100644 --- a/src/cli/commands/view/command.tsx +++ b/src/cli/commands/view/command.tsx @@ -6,7 +6,7 @@ import { printBatchEvaluationDetail, printBatchEvaluationHistory } from '../../o import { printInsightsDetail, printInsightsHistory } from '../../operations/jobs/insights/format'; import { printRecommendationDetail, printRecommendationHistory } from '../../operations/jobs/recommendation/format'; import { runCliCommand } from '../../telemetry/cli-command-run'; -import { requireProject } from '../../tui/guards'; +import { requireProject, requireTTY } from '../../tui/guards'; import type { Command } from '@commander-js/extra-typings'; const TYPE_META: Record< @@ -95,6 +95,7 @@ function registerViewSubcommand(viewCmd: Command, type: JobType) { } async function launchTuiList(type: JobType): Promise { + requireTTY(); const [{ render }, { default: React }] = await Promise.all([import('ink'), import('react')]); if (type === 'ab-test') { @@ -114,6 +115,7 @@ async function launchTuiList(type: JobType): Promise { } async function launchTuiDetail(type: JobType, id: string): Promise { + requireTTY(); const [{ render }, { default: React }] = await Promise.all([import('ink'), import('react')]); const { JobDetailScreen } = await import('./JobDetailScreen'); render(React.createElement(JobDetailScreen, { type, id, onExit: () => process.exit(0) })); diff --git a/src/cli/tui/render.ts b/src/cli/tui/render.ts index 8a2f91434..269caddfc 100644 --- a/src/cli/tui/render.ts +++ b/src/cli/tui/render.ts @@ -6,6 +6,7 @@ import { type UpdateCheckResult } from '../update-notifier'; import { App, type InitialRoute } from './App'; import { clearExitAction, getExitAction } from './exit-action'; import { clearExitMessage, getExitMessage } from './exit-message'; +import { requireTTY } from './guards/tty'; import { render } from 'ink'; import React from 'react'; @@ -33,6 +34,10 @@ export interface RenderTUIOptions { * This is the entrypoint for all TUI operations. */ export async function renderTUI(options: RenderTUIOptions = {}) { + // Centralized interactive-terminal guard for every TUI entrypoint that goes + // through renderTUI. Raw `render()` call sites (import, view) guard inline. + requireTTY(); + const { initialRoute, updateCheck = Promise.resolve(null),