diff --git a/src/commands/branch/merge.test.ts b/src/commands/branch/merge.test.ts index 327c610..6fad839 100644 --- a/src/commands/branch/merge.test.ts +++ b/src/commands/branch/merge.test.ts @@ -53,9 +53,13 @@ vi.mock('../../lib/analytics.js', () => ({ shutdownAnalytics: vi.fn(async () => {}), })); -const clackConfirmMock = vi.hoisted(() => vi.fn(async () => true)); -vi.mock('@clack/prompts', () => ({ - confirm: clackConfirmMock, +// prompts.js is mocked (not @clack/prompts) because merge goes through the +// TTY-safe wrapper: @clack/core opens a tty.WriteStream unconditionally, which +// throws EBADF in non-interactive shells. +const confirmMock = vi.hoisted(() => vi.fn(async () => true)); +vi.mock('../../lib/prompts.js', () => ({ + isInteractive: false, + confirm: confirmMock, isCancel: () => false, })); @@ -63,8 +67,8 @@ describe('branch merge', () => { beforeEach(() => { vi.clearAllMocks(); fsMock.writeFileSync.mockReset(); - clackConfirmMock.mockClear(); - clackConfirmMock.mockResolvedValue(true); + confirmMock.mockClear(); + confirmMock.mockResolvedValue(true); }); it('--dry-run prints rendered_sql + summary, does not call execute', async () => { @@ -136,11 +140,43 @@ describe('branch merge', () => { } finally { console.log = origLog; } - expect(clackConfirmMock).not.toHaveBeenCalled(); + expect(confirmMock).not.toHaveBeenCalled(); const { mergeBranchExecuteApi } = await import('../../lib/api/platform.js'); expect(mergeBranchExecuteApi).toHaveBeenCalledWith('b1', undefined); }); + // Regression: in a non-interactive shell the confirmation prompt crashed with + // 'uv_tty_init returned EBADF' after the full merge plan had been printed. + // Now the CLI says up front that -y is required and applies nothing. + it('non-interactive apply without -y errors actionably and does not execute', async () => { + const program = new Command().exitOverride(); + program.option('--json').option('--api-url ').option('-y, --yes'); + registerBranchMergeCommand(program); + const errors: string[] = []; + const origError = console.error; + console.error = (...args: unknown[]) => errors.push(args.map(String).join(' ')); + const origLog = console.log; + console.log = () => {}; + let exitCode: number | undefined; + const origExit = process.exit; + process.exit = ((code?: number) => { + if (exitCode === undefined) exitCode = code; + throw new Error('__exit__'); + }) as typeof process.exit; + try { + await program.parseAsync(['merge', 'feat-x'], { from: 'user' }).catch(() => {}); + } finally { + process.exit = origExit; + console.error = origError; + console.log = origLog; + } + expect(exitCode).toBe(1); + expect(errors.join('\n')).toContain('Re-run with -y'); + expect(confirmMock).not.toHaveBeenCalled(); + const { mergeBranchExecuteApi } = await import('../../lib/api/platform.js'); + expect(mergeBranchExecuteApi).not.toHaveBeenCalled(); + }); + it('conflict path exits with code 2 and prints per-conflict summary', async () => { const { mergeBranchDryRunApi } = await import('../../lib/api/platform.js'); (mergeBranchDryRunApi as Mock).mockResolvedValueOnce({ diff --git a/src/commands/branch/merge.ts b/src/commands/branch/merge.ts index 6f36e25..3be3d1c 100644 --- a/src/commands/branch/merge.ts +++ b/src/commands/branch/merge.ts @@ -1,6 +1,6 @@ import type { Command } from 'commander'; import { writeFileSync } from 'node:fs'; -import * as clack from '@clack/prompts'; +import * as prompts from '../../lib/prompts.js'; import { listBranchesApi, mergeBranchDryRunApi, @@ -37,6 +37,18 @@ export function registerBranchMergeCommand(branch: Command): void { const target = branches.find(b => b.name === name); if (!target) throw new CLIError(`Branch '${name}' not found.`); + // A non-interactive shell (CI, agent sandbox) can never answer the apply + // confirmation — and some of them have no usable TTY at all — so say what + // flag is needed up front instead of printing the whole merge plan first. + if (!opts.dryRun && !json && !yes && !prompts.isInteractive) { + throw new CLIError( + 'Applying a merge requires confirmation, but stdin is not interactive. ' + + 'Re-run with -y to apply, or with --dry-run to preview the SQL.', + 1, + 'MERGE_CONFIRMATION_REQUIRED', + ); + } + // Always compute diff first (cheap, gives the user a preview). const diff = await mergeBranchDryRunApi(target.id, apiUrl); @@ -89,10 +101,10 @@ export function registerBranchMergeCommand(branch: Command): void { // Confirm before executing (unless --yes or --json). if (!yes && !json) { const parentLabel = project.branched_from?.project_name ?? project.project_name; - const confirmed = await clack.confirm({ + const confirmed = await prompts.confirm({ message: `Apply this merge to parent project '${parentLabel}'?`, }); - if (clack.isCancel(confirmed) || !confirmed) { + if (prompts.isCancel(confirmed) || !confirmed) { outputInfo('Merge cancelled.'); return; }