From f0f0adf5143fe3536e198855d3af5abaf8c2e2cb Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Thu, 25 Jun 2026 06:11:11 +0000 Subject: [PATCH 1/4] fix(cli): guard import/view/export TUI launches with requireTTY (#982) Non-TTY interactive TUI commands crashed with Ink's 'Raw mode is not supported on the current process.stdin' and, for import (no --source), exited 0 so CI/scripts could not detect the failure. deploy was already guarded by requireTTY (PR #949); import/view/export were not. Add requireTTY() before each unguarded interactive launch: - export/index.ts: before renderTUI for export-harness - import/command.ts: in the no-source branch before render(ImportFlow) - view/command.tsx: at the top of launchTuiList and launchTuiDetail requireProject()/--json paths untouched. Adds unit tests covering the three command suites. Refs aws/agentcore-cli#982 --- .../export/__tests__/tty-guard.test.ts | 46 +++++++++++++++ src/cli/commands/export/index.ts | 2 + .../import/__tests__/tty-guard.test.ts | 56 +++++++++++++++++++ src/cli/commands/import/command.ts | 2 + .../commands/view/__tests__/tty-guard.test.ts | 52 +++++++++++++++++ src/cli/commands/view/command.tsx | 4 +- 6 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 src/cli/commands/export/__tests__/tty-guard.test.ts create mode 100644 src/cli/commands/import/__tests__/tty-guard.test.ts create mode 100644 src/cli/commands/view/__tests__/tty-guard.test.ts 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..9d2e20354 --- /dev/null +++ b/src/cli/commands/export/__tests__/tty-guard.test.ts @@ -0,0 +1,46 @@ +import { registerExport } from '../index'; +import { Command } from '@commander-js/extra-typings'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockRequireTTY = vi.fn(); +const mockRenderTUI = vi.fn(); + +vi.mock('../../../tui/guards', () => ({ + requireTTY: () => mockRequireTTY(), +})); + +vi.mock('../../../tui/render', () => ({ + renderTUI: (...args: unknown[]) => mockRenderTUI(...args), +})); + +describe('export harness TTY guard', () => { + let program: Command; + + beforeEach(() => { + program = new Command(); + program.exitOverride(); + registerExport(program); + }); + + afterEach(() => { + vi.resetAllMocks(); + }); + + it('calls requireTTY and never renders the TUI in a non-TTY context', async () => { + mockRequireTTY.mockImplementation(() => { + throw new Error('process.exit'); + }); + + await expect(program.parseAsync(['export', 'harness'], { from: 'user' })).rejects.toThrow('process.exit'); + + expect(mockRequireTTY).toHaveBeenCalled(); + expect(mockRenderTUI).not.toHaveBeenCalled(); + }); + + it('renders the TUI when a TTY is present', async () => { + await program.parseAsync(['export', 'harness'], { from: 'user' }); + + expect(mockRequireTTY).toHaveBeenCalled(); + expect(mockRenderTUI).toHaveBeenCalled(); + }); +}); diff --git a/src/cli/commands/export/index.ts b/src/cli/commands/export/index.ts index 24587eb82..71241df8d 100644 --- a/src/cli/commands/export/index.ts +++ b/src/cli/commands/export/index.ts @@ -1,5 +1,6 @@ import { serializeResult } from '../../../lib/result'; import { ANSI, COMMAND_DESCRIPTIONS } from '../../constants'; +import { requireTTY } from '../../tui/guards'; import { renderTUI } from '../../tui/render'; import { handleExportHarness } from './harness-action'; import type { Command } from '@commander-js/extra-typings'; @@ -29,6 +30,7 @@ export function registerExport(program: Command): void { console.log(JSON.stringify({ success: false, error: '--name is required in non-interactive mode' })); process.exit(1); } + requireTTY(); 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..122b3d123 --- /dev/null +++ b/src/cli/commands/import/__tests__/tty-guard.test.ts @@ -0,0 +1,56 @@ +import { registerImport } from '../command'; +import { Command } from '@commander-js/extra-typings'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockRequireTTY = vi.fn(); +const mockRequireProject = vi.fn(); +const mockRender = vi.fn(); + +vi.mock('../../../tui/guards/tty', () => ({ + requireTTY: () => mockRequireTTY(), +})); + +vi.mock('../../../tui/guards/project', () => ({ + requireProject: () => mockRequireProject(), +})); + +vi.mock('ink', () => ({ + render: (...args: unknown[]) => { + mockRender(...args); + return { clear: vi.fn(), unmount: vi.fn() }; + }, +})); + +vi.mock('../../../tui/screens/import', () => ({ ImportFlow: () => null })); + +describe('import non-source TTY guard', () => { + let program: Command; + + beforeEach(() => { + program = new Command(); + program.exitOverride(); + registerImport(program); + }); + + afterEach(() => { + vi.resetAllMocks(); + }); + + it('calls requireTTY and does not render when the guard exits in a non-TTY context', async () => { + mockRequireTTY.mockImplementation(() => { + throw new Error('process.exit'); + }); + + await expect(program.parseAsync(['import'], { from: 'user' })).rejects.toThrow('process.exit'); + + expect(mockRequireTTY).toHaveBeenCalled(); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('renders the interactive flow when a TTY is present', async () => { + await program.parseAsync(['import'], { from: 'user' }); + + expect(mockRequireTTY).toHaveBeenCalled(); + expect(mockRender).toHaveBeenCalled(); + }); +}); diff --git a/src/cli/commands/import/command.ts b/src/cli/commands/import/command.ts index 965fc8767..9dc9d1f7d 100644 --- a/src/cli/commands/import/command.ts +++ b/src/cli/commands/import/command.ts @@ -25,6 +25,8 @@ export const registerImport = (program: Command) => { .action(async (cliOptions: { source?: string; target?: string; yes?: boolean }) => { if (!cliOptions.source) { // No --source and no subcommand — launch interactive TUI + const { requireTTY } = await import('../../tui/guards/tty'); + requireTTY(); const { requireProject } = await import('../../tui/guards/project'); requireProject(); const { render } = await import('ink'); 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..917408168 --- /dev/null +++ b/src/cli/commands/view/__tests__/tty-guard.test.ts @@ -0,0 +1,52 @@ +import { registerView } from '../command'; +import { Command } from '@commander-js/extra-typings'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +const mockRequireTTY = vi.fn(); +const mockRequireProject = vi.fn(); +const mockRender = vi.fn(); + +vi.mock('../../../tui/guards', () => ({ + requireProject: () => mockRequireProject(), + requireTTY: () => mockRequireTTY(), +})); + +vi.mock('ink', () => ({ + render: (...args: unknown[]) => mockRender(...args), +})); + +vi.mock('../../../tui/screens/recommendation', () => ({ RecommendationHistoryScreen: () => null })); +vi.mock('../JobDetailScreen', () => ({ JobDetailScreen: () => null })); + +describe('view TTY guard', () => { + let program: Command; + + beforeEach(() => { + program = new Command(); + program.exitOverride(); + registerView(program); + mockRequireTTY.mockImplementation(() => { + throw new Error('process.exit'); + }); + }); + + afterEach(() => { + vi.clearAllMocks(); + }); + + it('guards the interactive list and never renders in a non-TTY context', async () => { + await expect(program.parseAsync(['view', 'recommendation'], { from: 'user' })).rejects.toThrow('process.exit'); + + expect(mockRequireTTY).toHaveBeenCalled(); + expect(mockRender).not.toHaveBeenCalled(); + }); + + it('guards the interactive detail and never renders in a non-TTY context', async () => { + await expect(program.parseAsync(['view', 'recommendation', 'rec-1'], { from: 'user' })).rejects.toThrow( + 'process.exit' + ); + + expect(mockRequireTTY).toHaveBeenCalled(); + expect(mockRender).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) })); From 5edc3081e542d0cc49a33b2090fb77f2a42d3a0b Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Thu, 25 Jun 2026 15:28:04 +0000 Subject: [PATCH 2/4] test(cli): drive real requireTTY guard; centralize in renderTUI - centralize the interactive-terminal guard at the top of renderTUI() so every TUI entrypoint (export-harness and any future renderTUI caller) is covered by default; keep inline guards only on the raw render() sites (import, view) - rewrite the three tty-guard tests to exercise the real requireTTY by toggling process.stdin.isTTY/process.stdout.isTTY and spying on process.exit + console.error, mocking only the I/O boundaries (mirrors feedback/consent-prompt.test.ts) instead of mocking requireTTY - parameterize the view list guard over all four job types - make requireTTY-vs-requireProject ordering consistent: requireProject() first in import (matches view) --- .../export/__tests__/tty-guard.test.ts | 75 +++++++++++++---- src/cli/commands/export/index.ts | 3 +- .../import/__tests__/tty-guard.test.ts | 71 +++++++++++----- src/cli/commands/import/command.ts | 9 +- .../commands/view/__tests__/tty-guard.test.ts | 84 +++++++++++++++---- src/cli/tui/render.ts | 5 ++ 6 files changed, 186 insertions(+), 61 deletions(-) diff --git a/src/cli/commands/export/__tests__/tty-guard.test.ts b/src/cli/commands/export/__tests__/tty-guard.test.ts index 9d2e20354..80c7cd354 100644 --- a/src/cli/commands/export/__tests__/tty-guard.test.ts +++ b/src/cli/commands/export/__tests__/tty-guard.test.ts @@ -2,45 +2,86 @@ import { registerExport } from '../index'; import { Command } from '@commander-js/extra-typings'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const mockRequireTTY = vi.fn(); -const mockRenderTUI = vi.fn(); +// 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(() => ({ waitUntilExit: () => Promise.resolve() })); -vi.mock('../../../tui/guards', () => ({ - requireTTY: () => mockRequireTTY(), +vi.mock('ink', () => ({ + render: (...args: unknown[]) => mockRender(...args), })); -vi.mock('../../../tui/render', () => ({ - renderTUI: (...args: unknown[]) => mockRenderTUI(...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; + const origStdinIsTTY = process.stdin.isTTY; + const origStdoutIsTTY = process.stdout.isTTY; + const origWrite = process.stdout.write; beforeEach(() => { program = new Command(); program.exitOverride(); registerExport(program); + + // Swallow the alt-screen escape sequences renderTUI writes on the TTY path. + process.stdout.write = vi.fn().mockReturnValue(true) as unknown as typeof process.stdout.write; + exitSpy = vi.spyOn(process, 'exit').mockImplementation((code?: string | number | null) => { + throw new Error(`process.exit(${code})`); + }); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => undefined); }); afterEach(() => { - vi.resetAllMocks(); + process.stdin.isTTY = origStdinIsTTY; + process.stdout.isTTY = origStdoutIsTTY; + process.stdout.write = origWrite; + exitSpy.mockRestore(); + errorSpy.mockRestore(); + vi.clearAllMocks(); }); - it('calls requireTTY and never renders the TUI in a non-TTY context', async () => { - mockRequireTTY.mockImplementation(() => { - throw new Error('process.exit'); - }); + 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'); + await expect(program.parseAsync(['export', 'harness'], { from: 'user' })).rejects.toThrow('process.exit(1)'); - expect(mockRequireTTY).toHaveBeenCalled(); - expect(mockRenderTUI).not.toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('requires an interactive terminal')); + expect(mockRender).not.toHaveBeenCalled(); }); - it('renders the TUI when a TTY is present', async () => { + 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(mockRequireTTY).toHaveBeenCalled(); - expect(mockRenderTUI).toHaveBeenCalled(); + expect(exitSpy).not.toHaveBeenCalled(); + expect(mockRender).toHaveBeenCalled(); }); }); diff --git a/src/cli/commands/export/index.ts b/src/cli/commands/export/index.ts index 71241df8d..5e11c35e9 100644 --- a/src/cli/commands/export/index.ts +++ b/src/cli/commands/export/index.ts @@ -1,6 +1,5 @@ import { serializeResult } from '../../../lib/result'; import { ANSI, COMMAND_DESCRIPTIONS } from '../../constants'; -import { requireTTY } from '../../tui/guards'; import { renderTUI } from '../../tui/render'; import { handleExportHarness } from './harness-action'; import type { Command } from '@commander-js/extra-typings'; @@ -30,7 +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); } - requireTTY(); + // 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 index 122b3d123..c6af819f7 100644 --- a/src/cli/commands/import/__tests__/tty-guard.test.ts +++ b/src/cli/commands/import/__tests__/tty-guard.test.ts @@ -2,55 +2,80 @@ import { registerImport } from '../command'; import { Command } from '@commander-js/extra-typings'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const mockRequireTTY = vi.fn(); -const mockRequireProject = vi.fn(); -const mockRender = vi.fn(); +// 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(() => ({ clear: vi.fn(), unmount: vi.fn() })); -vi.mock('../../../tui/guards/tty', () => ({ - requireTTY: () => mockRequireTTY(), -})); - -vi.mock('../../../tui/guards/project', () => ({ - requireProject: () => mockRequireProject(), -})); +vi.mock('../../../tui/guards', async importOriginal => { + const actual = await importOriginal(); + return { + ...actual, + requireProject: vi.fn(), + }; +}); vi.mock('ink', () => ({ - render: (...args: unknown[]) => { - mockRender(...args); - return { clear: vi.fn(), unmount: vi.fn() }; - }, + 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(() => { - vi.resetAllMocks(); + process.stdin.isTTY = origStdinIsTTY; + process.stdout.isTTY = origStdoutIsTTY; + exitSpy.mockRestore(); + errorSpy.mockRestore(); + vi.clearAllMocks(); }); - it('calls requireTTY and does not render when the guard exits in a non-TTY context', async () => { - mockRequireTTY.mockImplementation(() => { - throw new Error('process.exit'); - }); + 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'); + await expect(program.parseAsync(['import'], { from: 'user' })).rejects.toThrow('process.exit(1)'); - expect(mockRequireTTY).toHaveBeenCalled(); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledWith(expect.stringContaining('requires an interactive terminal')); expect(mockRender).not.toHaveBeenCalled(); }); - it('renders the interactive flow when a TTY is present', async () => { + 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(mockRequireTTY).toHaveBeenCalled(); + expect(exitSpy).not.toHaveBeenCalled(); expect(mockRender).toHaveBeenCalled(); }); }); diff --git a/src/cli/commands/import/command.ts b/src/cli/commands/import/command.ts index 9dc9d1f7d..5172f5d13 100644 --- a/src/cli/commands/import/command.ts +++ b/src/cli/commands/import/command.ts @@ -24,11 +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 { requireTTY } = await import('../../tui/guards/tty'); - requireTTY(); - 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 index 917408168..b7a19ec41 100644 --- a/src/cli/commands/view/__tests__/tty-guard.test.ts +++ b/src/cli/commands/view/__tests__/tty-guard.test.ts @@ -2,51 +2,105 @@ import { registerView } from '../command'; import { Command } from '@commander-js/extra-typings'; import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; -const mockRequireTTY = vi.fn(); -const mockRequireProject = vi.fn(); +// 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', () => ({ - requireProject: () => mockRequireProject(), - requireTTY: () => mockRequireTTY(), -})); +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); - mockRequireTTY.mockImplementation(() => { - throw new Error('process.exit'); + + 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('guards the interactive list and never renders in a non-TTY context', async () => { - await expect(program.parseAsync(['view', 'recommendation'], { from: 'user' })).rejects.toThrow('process.exit'); + 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; - expect(mockRequireTTY).toHaveBeenCalled(); - expect(mockRender).not.toHaveBeenCalled(); + 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 and never renders in a non-TTY context', async () => { + 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' + 'process.exit(1)' ); - expect(mockRequireTTY).toHaveBeenCalled(); + 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/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), From 379e02d7f6e52813b68fefc2573a88f1acd7f3d8 Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Thu, 25 Jun 2026 15:32:11 +0000 Subject: [PATCH 3/4] fix(test): type ink render mocks to satisfy tsc --noEmit --- src/cli/commands/export/__tests__/tty-guard.test.ts | 8 ++++---- src/cli/commands/import/__tests__/tty-guard.test.ts | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/cli/commands/export/__tests__/tty-guard.test.ts b/src/cli/commands/export/__tests__/tty-guard.test.ts index 80c7cd354..294048798 100644 --- a/src/cli/commands/export/__tests__/tty-guard.test.ts +++ b/src/cli/commands/export/__tests__/tty-guard.test.ts @@ -7,7 +7,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // 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(() => ({ waitUntilExit: () => Promise.resolve() })); +const mockRender = vi.fn((..._args: unknown[]) => ({ waitUntilExit: () => Promise.resolve() })); vi.mock('ink', () => ({ render: (...args: unknown[]) => mockRender(...args), @@ -28,9 +28,9 @@ 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; - const origWrite = process.stdout.write; beforeEach(() => { program = new Command(); @@ -38,7 +38,7 @@ describe('export harness TTY guard', () => { registerExport(program); // Swallow the alt-screen escape sequences renderTUI writes on the TTY path. - process.stdout.write = vi.fn().mockReturnValue(true) as unknown as typeof process.stdout.write; + writeSpy = vi.spyOn(process.stdout, 'write').mockReturnValue(true); exitSpy = vi.spyOn(process, 'exit').mockImplementation((code?: string | number | null) => { throw new Error(`process.exit(${code})`); }); @@ -48,7 +48,7 @@ describe('export harness TTY guard', () => { afterEach(() => { process.stdin.isTTY = origStdinIsTTY; process.stdout.isTTY = origStdoutIsTTY; - process.stdout.write = origWrite; + writeSpy.mockRestore(); exitSpy.mockRestore(); errorSpy.mockRestore(); vi.clearAllMocks(); diff --git a/src/cli/commands/import/__tests__/tty-guard.test.ts b/src/cli/commands/import/__tests__/tty-guard.test.ts index c6af819f7..218664cb2 100644 --- a/src/cli/commands/import/__tests__/tty-guard.test.ts +++ b/src/cli/commands/import/__tests__/tty-guard.test.ts @@ -6,7 +6,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; // 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(() => ({ clear: vi.fn(), unmount: vi.fn() })); +const mockRender = vi.fn((..._args: unknown[]) => ({ clear: vi.fn(), unmount: vi.fn() })); vi.mock('../../../tui/guards', async importOriginal => { const actual = await importOriginal(); From cee06c43b9c09302dc6961dac2dfb5825236ac2d Mon Sep 17 00:00:00 2001 From: Aidan Daly Date: Thu, 25 Jun 2026 16:10:56 +0000 Subject: [PATCH 4/4] fix(cli): guard dev browser-mode picker with requireTTY (#982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit launchTuiDevScreenWithPicker called render() without a requireTTY() guard, so in a non-TTY context it threw Ink's "Raw mode is not supported" stack trace plus a partial alt-screen paint — a counterexample to the centralized guard PR #1640 added. Call requireTTY() at the top of the picker (before the alt-screen write and render) and add a tty-guard test that drives the real guard for this path. --- .../commands/dev/__tests__/tty-guard.test.ts | 84 +++++++++++++++++++ src/cli/commands/dev/browser-mode.ts | 2 + 2 files changed, 86 insertions(+) create mode 100644 src/cli/commands/dev/__tests__/tty-guard.test.ts 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 = () => {