diff --git a/.changeset/hungry-spoons-shake.md b/.changeset/hungry-spoons-shake.md new file mode 100644 index 000000000..58b6a8b10 --- /dev/null +++ b/.changeset/hungry-spoons-shake.md @@ -0,0 +1,28 @@ +--- +'stash': patch +--- + +Fix `stash impl` and `stash init` hanging on CI runners that allocate a TTY. + +Four prompts decided whether to run interactively without going through the +shared TTY helper, so on a CI runner with an allocated TTY they rendered a clack +prompt and blocked forever on `/dev/tty` — a silent hang with no error and no +timeout: + +- `stash impl` gated on an inline `process.env.CI !== 'true'`, which only + recognised the exact lowercase spelling. Runners that set `CI=1` or `CI=TRUE` + blocked on the plan-summary confirmation or the agent-target picker. +- `stash init`'s offer to chain into `stash plan`, and its Proxy-vs-SDK + question, gated on `process.stdout.isTTY` and did not consult `CI` at all — + so they hung on any CI runner with a TTY, whatever the spelling. Gating on + stdout was also the wrong stream: a redirected stdin still hangs a prompt. +- `stash impl --continue-without-plan` confirmed the flag with a second prompt + that was not gated at all, so a CI run with no plan on disk blocked there even + though the flag had already granted consent. The flag is now taken as consent + in non-interactive runs and only re-confirmed interactively. + +All four now use the shared `isInteractive()` helper (stdin is a TTY and `CI` +is not set to `1`/`true` in any case), matching `stash plan`. Non-interactive +runs take the path they always should have: `stash init` skips the chain offer +and prints the `plan --target` hint, the Proxy-vs-SDK question defaults to +SDK-only, and `stash impl` proceeds without prompting. diff --git a/packages/cli/src/commands/impl/__tests__/impl.test.ts b/packages/cli/src/commands/impl/__tests__/impl.test.ts index 9ecaf9f7a..2b25c6359 100644 --- a/packages/cli/src/commands/impl/__tests__/impl.test.ts +++ b/packages/cli/src/commands/impl/__tests__/impl.test.ts @@ -2,9 +2,20 @@ import fs from 'node:fs' import os from 'node:os' import path from 'node:path' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { CliExit } from '../../../cli/exit.js' import { implCommand } from '../index.js' import { howToProceedStep } from '../steps/how-to-proceed.js' +// `implCommand` reaches the plan-summary checkpoint via `p.confirm`, which +// reads /dev/tty. Stub just that export (the rest of @clack/prompts stays +// real) so the CI-detection cases can assert whether the prompt was reached. +const confirmMock = vi.hoisted(() => vi.fn()) +vi.mock('@clack/prompts', async () => { + const actual = + await vi.importActual('@clack/prompts') + return { ...actual, confirm: confirmMock } +}) + let originalIsTTY: boolean | undefined let originalCwd: string let tmpDir: string @@ -99,3 +110,138 @@ describe('implCommand — TTY handling', () => { expect(runSpy).not.toHaveBeenCalled() }) }) + +describe('implCommand — CI detection with a TTY attached', () => { + // Regression: the gate used to be an inline `process.env.CI !== 'true'`, + // which only recognised the exact lowercase spelling. A CI runner that sets + // CI=1 or CI=TRUE *and* allocates a TTY made `stash impl` believe it was + // interactive, so it blocked on the plan-summary confirm (or the + // agent-target picker) forever — a hang, not an error. The gate now goes + // through `isInteractive()` in config/tty.ts, whose `isCiEnv()` accepts + // 1/true in any case. + beforeEach(() => { + setIsTTY(true) + confirmMock.mockReset() + confirmMock.mockResolvedValue(true) + }) + + afterEach(() => { + vi.unstubAllEnvs() + }) + + for (const ciValue of ['1', 'TRUE', 'true']) { + it(`treats CI=${ciValue} as non-interactive even with a TTY`, async () => { + vi.stubEnv('CI', ciValue) + const runSpy = vi + .spyOn(howToProceedStep, 'run') + .mockResolvedValue({} as never) + + await expect(implCommand({}, {})).resolves.toBeUndefined() + + // Neither blocking prompt may be reached. + expect(confirmMock).not.toHaveBeenCalled() + expect(runSpy).not.toHaveBeenCalled() + }) + } + + it('does not re-confirm --continue-without-plan under CI when no plan exists', async () => { + // The flag is the consent. Prompting again under CI blocks on /dev/tty, + // and the confirm is default-no, so a resolved prompt would cancel a run + // the user explicitly asked for. + vi.stubEnv('CI', '1') + fs.rmSync(path.join(tmpDir, '.cipherstash', 'plan.md')) + const runSpy = vi + .spyOn(howToProceedStep, 'run') + .mockResolvedValue({} as never) + + await expect( + implCommand({ 'continue-without-plan': true }, {}), + ).resolves.toBeUndefined() + + expect(confirmMock).not.toHaveBeenCalled() + expect(runSpy).not.toHaveBeenCalled() + }) + + it('is interactive when CI is unset and stdin is a TTY', async () => { + vi.stubEnv('CI', '') + const runSpy = vi + .spyOn(howToProceedStep, 'run') + .mockResolvedValue({} as never) + + await implCommand({}, {}) + + expect(confirmMock).toHaveBeenCalledTimes(1) + expect(runSpy).toHaveBeenCalledTimes(1) + }) + + it('performs the handoff under CI=1 when --target is given', async () => { + // Non-interactive means "don't prompt", not "don't run". A regression + // that made the command bail early under CI would keep the not-called + // cases above green — this locks the automation happy path. + vi.stubEnv('CI', '1') + const runSpy = vi + .spyOn(howToProceedStep, 'run') + .mockResolvedValue({} as never) + + await implCommand({}, { target: 'agents-md' }) + + expect(confirmMock).not.toHaveBeenCalled() + expect(runSpy).toHaveBeenCalledTimes(1) + expect(runSpy.mock.calls[0][0].handoff).toBe('agents-md') + }) + + it('runs the selected handoff under CI with --continue-without-plan and no plan', async () => { + // The specific path the `if (isTTY)` gate at index.ts opened up: flag + + // target, no plan on disk — it must run the handoff without re-confirming. + vi.stubEnv('CI', '1') + fs.rmSync(path.join(tmpDir, '.cipherstash', 'plan.md')) + const runSpy = vi + .spyOn(howToProceedStep, 'run') + .mockResolvedValue({} as never) + + await implCommand( + { 'continue-without-plan': true }, + { target: 'agents-md' }, + ) + + expect(confirmMock).not.toHaveBeenCalled() + expect(runSpy).toHaveBeenCalledTimes(1) + expect(runSpy.mock.calls[0][0].handoff).toBe('agents-md') + }) + + it('exits 1 with the plan hint under CI=1 when no plan and no --continue-without-plan', async () => { + // CI=1 flips isTTY to false, rerouting a no-plan / no-flag run out of the + // interactive `select` and into the `else if (!isTTY)` error + exit(1). It + // must fail loudly, not render the "No plan found" select and block. + vi.stubEnv('CI', '1') + fs.rmSync(path.join(tmpDir, '.cipherstash', 'plan.md')) + const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => { + throw new Error('process.exit') + }) as never) + + await expect(implCommand({}, {})).rejects.toThrow('process.exit') + + expect(exitSpy).toHaveBeenCalledWith(1) + expect(confirmMock).not.toHaveBeenCalled() + }) + + it('still confirms --continue-without-plan interactively, and declining aborts', async () => { + // The honour-the-prompt half of the `if (isTTY)` gate: with a TTY and CI + // unset the default-no confirm must still fire, and declining it must abort + // (CancelledError → CliExit) rather than fall through to the handoff. + vi.stubEnv('CI', '') + fs.rmSync(path.join(tmpDir, '.cipherstash', 'plan.md')) + confirmMock.mockReset() + confirmMock.mockResolvedValue(false) + const runSpy = vi + .spyOn(howToProceedStep, 'run') + .mockResolvedValue({} as never) + + await expect( + implCommand({ 'continue-without-plan': true }, {}), + ).rejects.toBeInstanceOf(CliExit) + + expect(confirmMock).toHaveBeenCalledTimes(1) + expect(runSpy).not.toHaveBeenCalled() + }) +}) diff --git a/packages/cli/src/commands/impl/index.ts b/packages/cli/src/commands/impl/index.ts index 861778389..044d82e6f 100644 --- a/packages/cli/src/commands/impl/index.ts +++ b/packages/cli/src/commands/impl/index.ts @@ -2,6 +2,7 @@ import { existsSync, readFileSync } from 'node:fs' import { resolve } from 'node:path' import * as p from '@clack/prompts' import { CliExit } from '../../cli/exit.js' +import { isInteractive as isInteractiveTty } from '../../config/tty.js' import { type AgentEnvironment, detectAgents } from '../init/detect-agents.js' import { effectiveStep, @@ -166,11 +167,13 @@ export async function implCommand( const planPath = resolve(cwd, PLAN_REL_PATH) const planExists = existsSync(planPath) const continueWithoutPlan = flags['continue-without-plan'] === true - // Interactive only when stdin is a real TTY and we're not in CI — the - // same gate the encrypt commands use. `process.stdout.isTTY` alone is - // wrong: a redirected stdin still hangs the agent-target picker (clack - // `select` reads from /dev/tty). - const isTTY = Boolean(process.stdin.isTTY) && process.env.CI !== 'true' + // Interactive only when stdin is a real TTY and we're not in CI — via the + // shared `isInteractive()` (config/tty.ts) so this gate stays identical to + // every other prompt gate (its `isCiEnv()` treats `CI=1`/`CI=TRUE` as CI too, + // which a bare `CI !== 'true'` inline would miss). `process.stdout.isTTY` + // alone is wrong: a redirected stdin still hangs the agent-target picker + // (clack `select` reads from /dev/tty). + const isTTY = isInteractiveTty() let planStep: PlanStep | undefined @@ -227,7 +230,13 @@ export async function implCommand( } else { // No plan on disk. Branch on flag / TTY / interactive. if (continueWithoutPlan) { - await confirmContinueWithoutPlan() + // The flag *is* the consent. Re-prompting for it non-interactively + // is the same hang this gate exists to close (clack `confirm` reads + // /dev/tty), and `confirmContinueWithoutPlan` is default-no, so a + // prompt that did resolve would cancel a run the user asked for. + if (isTTY) { + await confirmContinueWithoutPlan() + } } else if (!isTTY) { p.log.error( `No plan at \`${PLAN_REL_PATH}\`. Run \`${cli} plan\` first, or pass --continue-without-plan to skip planning.`, diff --git a/packages/cli/src/commands/init/__tests__/init-command.test.ts b/packages/cli/src/commands/init/__tests__/init-command.test.ts index 3064d93f5..39755c4d1 100644 --- a/packages/cli/src/commands/init/__tests__/init-command.test.ts +++ b/packages/cli/src/commands/init/__tests__/init-command.test.ts @@ -1,5 +1,5 @@ import * as p from '@clack/prompts' -import { beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import { CliExit } from '../../../cli/exit.js' import { messages } from '../../../messages.js' import type { InitState } from '../types.js' @@ -174,3 +174,106 @@ describe('initCommand — honest summary', () => { ) }) }) + +describe('initCommand — CI detection on the `stash plan` chain offer', () => { + // Regression: this gate was `process.stdout.isTTY`, which consulted CI not + // at all. A CI runner with an allocated TTY reached the confirm and blocked + // on /dev/tty forever — a hang, not an error. It also asked about the wrong + // stream: a redirected stdin still hangs the prompt. Now `isInteractive()` + // (stdin + isCiEnv, which accepts 1/true in any case). + const originalStdinIsTTY = process.stdin.isTTY + const originalStdoutIsTTY = process.stdout.isTTY + + // Force BOTH streams. A CI runner that allocates a TTY has stdin *and* + // stdout as TTYs — that is the configuration this gate used to hang in, and + // setting stdin alone would let the old `process.stdout.isTTY` gate pass + // these tests for the wrong reason (vitest's own stdout is not a TTY). + beforeEach(() => { + Object.defineProperty(process.stdin, 'isTTY', { + value: true, + configurable: true, + }) + Object.defineProperty(process.stdout, 'isTTY', { + value: true, + configurable: true, + }) + }) + + afterEach(() => { + Object.defineProperty(process.stdin, 'isTTY', { + value: originalStdinIsTTY, + configurable: true, + }) + Object.defineProperty(process.stdout, 'isTTY', { + value: originalStdoutIsTTY, + configurable: true, + }) + vi.unstubAllEnvs() + }) + + for (const ciValue of ['1', 'TRUE', 'true']) { + it(`skips the chain offer under CI=${ciValue} even with a TTY`, async () => { + vi.stubEnv('CI', ciValue) + + await expect(initCommand({}, {})).resolves.toBeUndefined() + + // Non-interactive must skip the offer, not fail: init still completes, + // and steers the user at `plan --target` instead of blocking. + expect(vi.mocked(p.confirm)).not.toHaveBeenCalled() + expect(vi.mocked(p.outro)).toHaveBeenCalledWith( + expect.stringContaining('--target'), + ) + }) + } + + it('offers the chain when CI is unset and stdin is a TTY', async () => { + vi.stubEnv('CI', '') + vi.mocked(p.confirm).mockResolvedValueOnce(false) + + await expect(initCommand({}, {})).resolves.toBeUndefined() + + expect(vi.mocked(p.confirm)).toHaveBeenCalledTimes(1) + }) + + it('skips the chain offer when stdin is redirected even if stdout is a TTY', async () => { + // The gate keys off stdin, not stdout: `stash init < /dev/null` from a + // terminal (stdin piped, stdout still a TTY) must not reach the confirm. + // Reinstating `process.stdout.isTTY` would keep the CI loop above green, + // so only this stdin/stdout split pins the correct stream. + vi.stubEnv('CI', '') + Object.defineProperty(process.stdin, 'isTTY', { + value: false, + configurable: true, + }) + Object.defineProperty(process.stdout, 'isTTY', { + value: true, + configurable: true, + }) + + await expect(initCommand({}, {})).resolves.toBeUndefined() + + expect(vi.mocked(p.confirm)).not.toHaveBeenCalled() + expect(vi.mocked(p.outro)).toHaveBeenCalledWith( + expect.stringContaining('--target'), + ) + }) + + it('chains into `stash plan` when the offer is accepted', async () => { + // The accept arm (outro + planCommand() + early return) is what the gate + // change made reachable; the only other confirm test resolves `false`. + vi.stubEnv('CI', '') + vi.mocked(p.confirm).mockResolvedValueOnce(true) + const { planCommand } = await import('../../plan/index.js') + + await expect(initCommand({}, {})).resolves.toBeUndefined() + + expect(vi.mocked(planCommand)).toHaveBeenCalledTimes(1) + expect(vi.mocked(p.outro)).toHaveBeenCalledWith( + expect.stringContaining('handing off to `stash plan`'), + ) + // The accept path returns early — it must not also print the --target hint. + expect(vi.mocked(p.outro)).not.toHaveBeenCalledWith( + expect.stringContaining('--target'), + ) + }) +}) diff --git a/packages/cli/src/commands/init/index.ts b/packages/cli/src/commands/init/index.ts index 517eaa65d..cd16d4ca2 100644 --- a/packages/cli/src/commands/init/index.ts +++ b/packages/cli/src/commands/init/index.ts @@ -1,5 +1,6 @@ import * as p from '@clack/prompts' import { CliExit } from '../../cli/exit.js' +import { isInteractive } from '../../config/tty.js' import { messages } from '../../messages.js' import { HANDOFF_CHOICES } from '../impl/steps/how-to-proceed.js' import { planCommand } from '../plan/index.js' @@ -165,7 +166,13 @@ export async function initCommand( // multi-command flow. Drafting a plan is fast (~1–3 min of agent // thinking) and produces a reviewable artifact — `stash impl` is the // separate, slower verb that actually mutates code. - if (process.stdout.isTTY) { + // + // Gated on the shared `isInteractive()` (config/tty.ts), the same helper + // every other prompt uses. `process.stdout.isTTY` was wrong on both + // counts: it ignored CI entirely (a runner with an allocated TTY blocked + // here forever, since clack `confirm` reads /dev/tty), and it asked about + // the wrong stream — a redirected stdin still hangs the prompt. + if (isInteractive()) { const proceed = await p.confirm({ message: `Continue to \`${cli} plan\` now to draft your encryption plan?`, initialValue: true, diff --git a/packages/cli/src/commands/init/steps/__tests__/resolve-proxy-choice.test.ts b/packages/cli/src/commands/init/steps/__tests__/resolve-proxy-choice.test.ts new file mode 100644 index 000000000..9dd72eb2c --- /dev/null +++ b/packages/cli/src/commands/init/steps/__tests__/resolve-proxy-choice.test.ts @@ -0,0 +1,157 @@ +import * as p from '@clack/prompts' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { InitProvider, InitState } from '../../types.js' +import { resolveProxyChoiceStep } from '../resolve-proxy-choice.js' + +vi.mock('@clack/prompts', () => ({ + select: vi.fn(), + isCancel: vi.fn(() => false), + log: { info: vi.fn(), success: vi.fn(), warn: vi.fn(), error: vi.fn() }, +})) + +const provider = {} as InitProvider +const baseState = {} as InitState + +const originalStdinIsTTY = process.stdin.isTTY +const originalStdoutIsTTY = process.stdout.isTTY + +/** + * Force BOTH streams. A CI runner that allocates a TTY has stdin *and* stdout + * as TTYs — that is the configuration this gate used to hang in, and setting + * stdin alone would let the old `process.stdout.isTTY` gate pass these tests + * for the wrong reason (vitest's own stdout is not a TTY). + */ +function setTty(value: boolean) { + Object.defineProperty(process.stdin, 'isTTY', { value, configurable: true }) + Object.defineProperty(process.stdout, 'isTTY', { value, configurable: true }) +} + +function restoreTty() { + Object.defineProperty(process.stdin, 'isTTY', { + value: originalStdinIsTTY, + configurable: true, + }) + Object.defineProperty(process.stdout, 'isTTY', { + value: originalStdoutIsTTY, + configurable: true, + }) +} + +beforeEach(() => { + vi.clearAllMocks() +}) + +afterEach(() => { + restoreTty() + vi.unstubAllEnvs() +}) + +describe('resolveProxyChoiceStep — flag short-circuit', () => { + it('returns state untouched when --proxy/--no-proxy already set it', async () => { + setTty(true) + vi.stubEnv('CI', '') + + const state = { ...baseState, usesProxy: true } + await expect(resolveProxyChoiceStep.run(state, provider)).resolves.toEqual( + state, + ) + expect(vi.mocked(p.select)).not.toHaveBeenCalled() + }) + + it('returns state untouched when --no-proxy set usesProxy to false', async () => { + // The guard is `state.usesProxy !== undefined`; `false` is the value that + // distinguishes it from a truthiness check. Rewriting it as + // `if (state.usesProxy)` would silently re-prompt for --no-proxy (and log + // the misleading "No --proxy flag set" notice non-interactively). + setTty(true) + vi.stubEnv('CI', '') + + const state = { ...baseState, usesProxy: false } + await expect(resolveProxyChoiceStep.run(state, provider)).resolves.toEqual( + state, + ) + expect(vi.mocked(p.select)).not.toHaveBeenCalled() + // An explicit --no-proxy is not "no flag set" — don't log the default notice. + expect(vi.mocked(p.log.info)).not.toHaveBeenCalled() + }) +}) + +describe('resolveProxyChoiceStep — CI detection with a TTY attached', () => { + // Regression: this gate was `process.stdout.isTTY`, which consulted CI not + // at all. On a CI runner with an allocated TTY the clack `select` rendered + // and blocked on /dev/tty forever — a hang, not an error. `isInteractive()` + // gates on stdin and treats CI=1/TRUE/true alike. + beforeEach(() => { + setTty(true) + }) + + for (const ciValue of ['1', 'TRUE', 'true']) { + it(`defaults to SDK-only under CI=${ciValue} without prompting`, async () => { + vi.stubEnv('CI', ciValue) + + const result = await resolveProxyChoiceStep.run(baseState, provider) + + // Non-interactive proceeds with the safe default rather than failing: + // SDK-only is what the flagless interactive prompt defaults to anyway. + expect(vi.mocked(p.select)).not.toHaveBeenCalled() + expect(result.usesProxy).toBe(false) + expect(vi.mocked(p.log.info)).toHaveBeenCalledWith( + expect.stringContaining('defaulting to SDK-only mode'), + ) + }) + } + + it('prompts when CI is unset and stdin is a TTY', async () => { + vi.stubEnv('CI', '') + vi.mocked(p.select).mockResolvedValueOnce(true) + + const result = await resolveProxyChoiceStep.run(baseState, provider) + + expect(vi.mocked(p.select)).toHaveBeenCalledTimes(1) + expect(result.usesProxy).toBe(true) + }) + + it('falls back to SDK-only when the interactive prompt is cancelled', async () => { + vi.stubEnv('CI', '') + vi.mocked(p.select).mockResolvedValueOnce(Symbol('cancel')) + vi.mocked(p.isCancel).mockReturnValueOnce(true) + + const result = await resolveProxyChoiceStep.run(baseState, provider) + + expect(result.usesProxy).toBe(false) + }) +}) + +describe('resolveProxyChoiceStep — no TTY', () => { + it('defaults to SDK-only when stdin is not a TTY, CI unset', async () => { + setTty(false) + vi.stubEnv('CI', '') + + const result = await resolveProxyChoiceStep.run(baseState, provider) + + expect(vi.mocked(p.select)).not.toHaveBeenCalled() + expect(result.usesProxy).toBe(false) + }) + + it('does not prompt when stdin is piped even though stdout is a TTY', async () => { + // setTty() can't express this — it moves both streams together. The fix + // gates on stdin, so a redirected stdin (`stash init < /dev/null` from a + // terminal) must not reach the prompt even while stdout is still a TTY. + // This is the case that verifies the changeset's "wrong stream" claim; + // only the CI axis distinguishes old from new anywhere else. + Object.defineProperty(process.stdin, 'isTTY', { + value: false, + configurable: true, + }) + Object.defineProperty(process.stdout, 'isTTY', { + value: true, + configurable: true, + }) + vi.stubEnv('CI', '') + + const result = await resolveProxyChoiceStep.run(baseState, provider) + + expect(vi.mocked(p.select)).not.toHaveBeenCalled() + expect(result.usesProxy).toBe(false) + }) +}) diff --git a/packages/cli/src/commands/init/steps/resolve-proxy-choice.ts b/packages/cli/src/commands/init/steps/resolve-proxy-choice.ts index 78f4d8026..1aaef5537 100644 --- a/packages/cli/src/commands/init/steps/resolve-proxy-choice.ts +++ b/packages/cli/src/commands/init/steps/resolve-proxy-choice.ts @@ -1,4 +1,5 @@ import * as p from '@clack/prompts' +import { isInteractive } from '../../../config/tty.js' import type { InitProvider, InitState, InitStep } from '../types.js' /** @@ -7,8 +8,8 @@ import type { InitProvider, InitState, InitStep } from '../types.js' * interactive prompt, and stored on state.usesProxy. * * The prompt is non-blocking: cancellation falls back to false (SDK-only, - * the default). In non-TTY contexts without a flag, defaults to false and - * logs an info message. + * the default). In non-interactive contexts without a flag, defaults to false + * and logs an info message. */ export const resolveProxyChoiceStep: InitStep = { id: 'resolve-proxy-choice', @@ -19,8 +20,13 @@ export const resolveProxyChoiceStep: InitStep = { return state } - // In TTY mode, prompt the user - if (process.stdout.isTTY) { + // Prompt only when it's safe to: stdin is a real TTY and we're not in CI. + // Via the shared `isInteractive()` (config/tty.ts) so this gate matches + // every other prompt gate. Keying off `process.stdout.isTTY` was wrong on + // both counts — it ignored CI entirely (a runner with an allocated TTY + // blocked here forever), and a redirected stdin still hangs clack `select`, + // which reads from /dev/tty. + if (isInteractive()) { const choice = await p.select({ message: 'Are you planning to query encrypted data via CipherStash Proxy, or directly via the SDK?', @@ -48,7 +54,7 @@ export const resolveProxyChoiceStep: InitStep = { return { ...state, usesProxy: choice } } - // Non-TTY: default to false and log + // Non-interactive: default to false and log p.log.info( 'No --proxy flag set; defaulting to SDK-only mode (no `stash db push` in default flows).', ) diff --git a/packages/cli/tests/e2e/impl-pty-ci.e2e.test.ts b/packages/cli/tests/e2e/impl-pty-ci.e2e.test.ts new file mode 100644 index 000000000..44b749649 --- /dev/null +++ b/packages/cli/tests/e2e/impl-pty-ci.e2e.test.ts @@ -0,0 +1,58 @@ +import fs from 'node:fs' +import os from 'node:os' +import path from 'node:path' +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { render } from '../helpers/pty.js' + +/** + * E2E: the real-hang reproducer. Under a genuine PTY (`process.stdin.isTTY` + * is true) with `CI` set to a non-`'true'` spelling, the pre-fix inline + * `CI !== 'true'` gate treated the run as interactive and blocked on the + * plan-summary confirm forever — a hang, not an error. + * + * The unit suites mock `@clack/prompts` and assert the gate's boolean; only a + * real PTY proves the process terminates. `pty.ts` `render()` defaults + * `env.CI` to `'true'`, which is exactly why that regression never surfaced + * here before — overriding to `1` / `TRUE` is the precise reproducer. + * `impl-non-tty.e2e.test.ts` uses `runPiped`, which gives `isTTY === false` + * by construction and cannot reach this path. + */ +describe('stash impl — PTY + CI (real-hang reproducer)', () => { + let tmpDir: string + + beforeEach(() => { + tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-impl-pty-ci-e2e-')) + fs.mkdirSync(path.join(tmpDir, '.cipherstash')) + fs.writeFileSync( + path.join(tmpDir, '.cipherstash', 'context.json'), + JSON.stringify({ + integration: 'postgresql', + packageManager: 'npm', + schemas: [], + }), + ) + // A plan on disk is what makes the pre-fix code reach the plan-summary + // confirm (the prompt that hung); without one it exits earlier. + fs.writeFileSync(path.join(tmpDir, '.cipherstash', 'plan.md'), '# Plan') + }) + + afterEach(() => { + if (tmpDir && fs.existsSync(tmpDir)) { + fs.rmSync(tmpDir, { recursive: true, force: true }) + } + }) + + for (const ciValue of ['1', 'TRUE']) { + it(`exits instead of hanging under a PTY with CI=${ciValue}`, async () => { + const r = render(['impl'], { cwd: tmpDir, env: { CI: ciValue } }) + // A regression re-opens the hang; the kill is the backstop so the suite + // fails on timeout instead of blocking. The fix exits in ~100ms. + const timer = setTimeout(() => r.kill('SIGKILL'), 10_000) + const { exitCode } = await r.exit + clearTimeout(timer) + + expect(exitCode).toBe(0) + expect(r.output).not.toContain('Proceed with implementation') + }) + } +})