diff --git a/src/cli/commands/deploy/__tests__/deploy-non-tty.test.ts b/src/cli/commands/deploy/__tests__/deploy-non-tty.test.ts new file mode 100644 index 000000000..e42f47db3 --- /dev/null +++ b/src/cli/commands/deploy/__tests__/deploy-non-tty.test.ts @@ -0,0 +1,59 @@ +import { runCLI } from '../../../../test-utils/index.js'; +import { randomUUID } from 'node:crypto'; +import { mkdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +/** + * Regression test for issue #982 (the command named in the bug title): + * `agentcore deploy` (and any TUI-mode invocation) in a non-TTY context + * must NOT crash with the Ink "Raw mode is not supported" error and must + * NOT exit 0. It must exit 1 with a clear interactive-terminal message so + * CI pipelines can detect the failure. + * + * The TUI path is reached when no flags are provided (and also for --diff). + * Flag-bearing modes (--yes, --json, --dry-run, --target, --verbose) route + * to the non-interactive CLI path which does not require a TTY and is not + * the subject of this regression. + */ +describe('deploy command non-TTY behavior (issue #982)', () => { + let testDir: string; + let projectDir: string; + + beforeAll(async () => { + testDir = join(tmpdir(), `agentcore-deploy-tty-${randomUUID()}`); + projectDir = join(testDir, 'project'); + const configDir = join(projectDir, 'agentcore'); + await mkdir(configDir, { recursive: true }); + // Minimal valid project marker so requireProject() passes — guard + // ordering means requireTTY() will fire first, but we keep a real + // project dir so an accidental ordering regression surfaces clearly. + await writeFile(join(configDir, 'agentcore.json'), JSON.stringify({ name: 'test', version: 1 }, null, 2), 'utf-8'); + }); + + afterAll(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + it('exits 1 with a clear message when run without flags in non-TTY (interactive path)', async () => { + // runCLI uses stdio: ['ignore', 'pipe', 'pipe'] — both stdin and stdout + // are non-TTY, exactly the scenario from the bug report. + const result = await runCLI(['deploy'], projectDir); + + expect(result.exitCode).toBe(1); + + const combined = `${result.stdout}\n${result.stderr}`; + expect(combined.toLowerCase()).toContain('requires an interactive terminal'); + expect(combined).not.toContain('Raw mode is not supported'); + }); + + it('exits 1 cleanly with --diff in non-TTY (TUI diff path)', async () => { + const result = await runCLI(['deploy', '--diff'], projectDir); + + expect(result.exitCode).toBe(1); + + const combined = `${result.stdout}\n${result.stderr}`; + expect(combined).not.toContain('Raw mode is not supported'); + }); +}); diff --git a/src/cli/commands/import/__tests__/command.test.ts b/src/cli/commands/import/__tests__/command.test.ts new file mode 100644 index 000000000..6435cf851 --- /dev/null +++ b/src/cli/commands/import/__tests__/command.test.ts @@ -0,0 +1,58 @@ +import { runCLI } from '../../../../test-utils/index.js'; +import { randomUUID } from 'node:crypto'; +import { mkdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; + +/** + * Regression test for issue #982: + * Running `agentcore import` (interactive TUI path) with a non-TTY stdin/stdout + * must NOT crash with the Ink "Raw mode is not supported" error and must NOT + * exit 0. It must exit 1 with a clear interactive-terminal message so that + * CI pipelines can detect the failure. + */ +describe('import command non-TTY behavior (issue #982)', () => { + let testDir: string; + let projectDir: string; + + beforeAll(async () => { + testDir = join(tmpdir(), `agentcore-import-tty-${randomUUID()}`); + projectDir = join(testDir, 'project'); + const configDir = join(projectDir, 'agentcore'); + await mkdir(configDir, { recursive: true }); + // Minimal valid project marker so requireProject() passes. + await writeFile(join(configDir, 'agentcore.json'), JSON.stringify({ name: 'test', version: 1 }, null, 2), 'utf-8'); + }); + + afterAll(async () => { + await rm(testDir, { recursive: true, force: true }); + }); + + it('exits non-zero with a clear message when stdin is not a TTY', async () => { + // runCLI uses stdio: ['ignore', 'pipe', 'pipe'] — both stdin and stdout + // are non-TTY, exactly the scenario from the bug report. + const result = await runCLI(['import'], projectDir); + + expect(result.exitCode).toBe(1); + + const combined = `${result.stdout}\n${result.stderr}`; + expect(combined.toLowerCase()).toContain('requires an interactive terminal'); + expect(combined).not.toContain('Raw mode is not supported'); + }); + + it('does not apply the TTY guard to --source path (pins guard placement)', async () => { + // The requireTTY() guard must remain inside the no-source branch only. + // If it gets accidentally hoisted to the top of the action, this test + // will start failing with the interactive-terminal message instead of + // the source-file-validation message. + const bogusSource = join(projectDir, 'definitely-does-not-exist.yaml'); + const result = await runCLI(['import', '--source', bogusSource], projectDir); + + expect(result.exitCode).toBe(1); + + const combined = `${result.stdout}\n${result.stderr}`; + expect(combined).toContain('Source file not found'); + expect(combined.toLowerCase()).not.toContain('requires an interactive terminal'); + }); +}); diff --git a/src/cli/commands/import/command.ts b/src/cli/commands/import/command.ts index 381167aaa..10ffc4214 100644 --- a/src/cli/commands/import/command.ts +++ b/src/cli/commands/import/command.ts @@ -1,3 +1,4 @@ +import { requireProject, requireTTY } from '../../tui/guards'; import { handleImport } from './actions'; import { ANSI } from './constants'; import { registerImportEvaluator } from './import-evaluator'; @@ -22,8 +23,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. + // Call requireTTY() before requireProject() so non-TTY contexts always + // exit with the plain-text message rather than going through any Ink + // rendering path (FatalError uses Ink, which can produce malformed + // output to a piped stream). + requireTTY(); requireProject(); const { render } = await import('ink'); const React = await import('react'); diff --git a/src/cli/tui/guards/__tests__/tty.test.ts b/src/cli/tui/guards/__tests__/tty.test.ts new file mode 100644 index 000000000..c6d693616 --- /dev/null +++ b/src/cli/tui/guards/__tests__/tty.test.ts @@ -0,0 +1,84 @@ +import { requireTTY } from '../tty.js'; +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; + +describe('requireTTY', () => { + let originalStdinIsTTY: boolean | undefined; + let originalStdoutIsTTY: boolean | undefined; + let exitSpy: ReturnType; + let errorSpy: ReturnType; + + beforeEach(() => { + // Snapshot the live values inside beforeEach so a previous test file in + // the same worker that mutated process.stdin/stdout.isTTY without + // restoring cannot poison our restoration. Also re-define with + // configurable: true to make subsequent restoration deterministic + // across Node versions where the original descriptor for piped streams + // may not be configurable. + originalStdinIsTTY = process.stdin.isTTY; + originalStdoutIsTTY = process.stdout.isTTY; + Object.defineProperty(process.stdin, 'isTTY', { + value: originalStdinIsTTY, + configurable: true, + writable: true, + }); + Object.defineProperty(process.stdout, 'isTTY', { + value: originalStdoutIsTTY, + configurable: true, + writable: true, + }); + + // Throw from process.exit so we can assert without actually exiting the test runner. + exitSpy = vi.spyOn(process, 'exit').mockImplementation(((code?: number) => { + throw new Error(`__exit__:${code}`); + }) as never); + errorSpy = vi.spyOn(console, 'error').mockImplementation(() => { + /* swallow output during tests */ + }); + }); + + afterEach(() => { + Object.defineProperty(process.stdin, 'isTTY', { value: originalStdinIsTTY, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: originalStdoutIsTTY, configurable: true }); + exitSpy.mockRestore(); + errorSpy.mockRestore(); + }); + + it('exits 1 with an interactive-terminal message when stdin is not a TTY', () => { + Object.defineProperty(process.stdin, 'isTTY', { value: false, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + + expect(() => requireTTY()).toThrow(/__exit__:1/); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledTimes(1); + const msg = String(errorSpy.mock.calls[0]?.[0] ?? ''); + expect(msg).toMatch(/requires an interactive terminal/); + }); + + it('exits 1 with an interactive-terminal message when stdout is not a TTY', () => { + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: false, configurable: true }); + + expect(() => requireTTY()).toThrow(/__exit__:1/); + expect(exitSpy).toHaveBeenCalledWith(1); + expect(errorSpy).toHaveBeenCalledTimes(1); + const msg = String(errorSpy.mock.calls[0]?.[0] ?? ''); + expect(msg).toMatch(/requires an interactive terminal/); + }); + + it('exits 1 when both stdin and stdout are non-TTY', () => { + Object.defineProperty(process.stdin, 'isTTY', { value: false, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: false, configurable: true }); + + expect(() => requireTTY()).toThrow(/__exit__:1/); + expect(exitSpy).toHaveBeenCalledWith(1); + }); + + it('is a no-op when both stdin and stdout are TTYs', () => { + Object.defineProperty(process.stdin, 'isTTY', { value: true, configurable: true }); + Object.defineProperty(process.stdout, 'isTTY', { value: true, configurable: true }); + + expect(() => requireTTY()).not.toThrow(); + expect(exitSpy).not.toHaveBeenCalled(); + expect(errorSpy).not.toHaveBeenCalled(); + }); +});