Skip to content

Commit e5eb696

Browse files
committed
test(cli): close the eight TTY-gate coverage gaps from PR review
Adds the tests auxesis flagged as missing on #704, each verified against the implementation: - resolve-proxy-choice: stdin-piped/stdout-TTY split (the "wrong stream" claim) and the --no-proxy (usesProxy:false) short-circuit. - init chain offer: redirected-stdin skip, and the accept arm that chains into `stash plan` (planCommand + early return). - impl: still runs under CI=1 with --target (plus a --continue-without-plan variant), the interactive confirm + decline-aborts path, and the no-plan/no-flag reroute into the exit(1) branch. - impl PTY e2e: real-hang reproducer under a genuine TTY with CI=1/TRUE (the headline changeset claim; the unit tests only lock the boolean). Test-only change; no changeset required.
1 parent 945743b commit e5eb696

4 files changed

Lines changed: 211 additions & 0 deletions

File tree

packages/cli/src/commands/impl/__tests__/impl.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import fs from 'node:fs'
22
import os from 'node:os'
33
import path from 'node:path'
44
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
5+
import { CliExit } from '../../../cli/exit.js'
56
import { implCommand } from '../index.js'
67
import { howToProceedStep } from '../steps/how-to-proceed.js'
78

@@ -172,4 +173,75 @@ describe('implCommand — CI detection with a TTY attached', () => {
172173
expect(confirmMock).toHaveBeenCalledTimes(1)
173174
expect(runSpy).toHaveBeenCalledTimes(1)
174175
})
176+
177+
it('performs the handoff under CI=1 when --target is given', async () => {
178+
// Non-interactive means "don't prompt", not "don't run". A regression
179+
// that made the command bail early under CI would keep the not-called
180+
// cases above green — this locks the automation happy path.
181+
vi.stubEnv('CI', '1')
182+
const runSpy = vi
183+
.spyOn(howToProceedStep, 'run')
184+
.mockResolvedValue({} as never)
185+
186+
await implCommand({}, { target: 'agents-md' })
187+
188+
expect(confirmMock).not.toHaveBeenCalled()
189+
expect(runSpy).toHaveBeenCalledTimes(1)
190+
expect(runSpy.mock.calls[0][0].handoff).toBe('agents-md')
191+
})
192+
193+
it('runs the selected handoff under CI with --continue-without-plan and no plan', async () => {
194+
// The specific path the `if (isTTY)` gate at index.ts opened up: flag +
195+
// target, no plan on disk — it must run the handoff without re-confirming.
196+
vi.stubEnv('CI', '1')
197+
fs.rmSync(path.join(tmpDir, '.cipherstash', 'plan.md'))
198+
const runSpy = vi
199+
.spyOn(howToProceedStep, 'run')
200+
.mockResolvedValue({} as never)
201+
202+
await implCommand(
203+
{ 'continue-without-plan': true },
204+
{ target: 'agents-md' },
205+
)
206+
207+
expect(confirmMock).not.toHaveBeenCalled()
208+
expect(runSpy).toHaveBeenCalledTimes(1)
209+
expect(runSpy.mock.calls[0][0].handoff).toBe('agents-md')
210+
})
211+
212+
it('exits 1 with the plan hint under CI=1 when no plan and no --continue-without-plan', async () => {
213+
// CI=1 flips isTTY to false, rerouting a no-plan / no-flag run out of the
214+
// interactive `select` and into the `else if (!isTTY)` error + exit(1). It
215+
// must fail loudly, not render the "No plan found" select and block.
216+
vi.stubEnv('CI', '1')
217+
fs.rmSync(path.join(tmpDir, '.cipherstash', 'plan.md'))
218+
const exitSpy = vi.spyOn(process, 'exit').mockImplementation((() => {
219+
throw new Error('process.exit')
220+
}) as never)
221+
222+
await expect(implCommand({}, {})).rejects.toThrow('process.exit')
223+
224+
expect(exitSpy).toHaveBeenCalledWith(1)
225+
expect(confirmMock).not.toHaveBeenCalled()
226+
})
227+
228+
it('still confirms --continue-without-plan interactively, and declining aborts', async () => {
229+
// The honour-the-prompt half of the `if (isTTY)` gate: with a TTY and CI
230+
// unset the default-no confirm must still fire, and declining it must abort
231+
// (CancelledError → CliExit) rather than fall through to the handoff.
232+
vi.stubEnv('CI', '')
233+
fs.rmSync(path.join(tmpDir, '.cipherstash', 'plan.md'))
234+
confirmMock.mockReset()
235+
confirmMock.mockResolvedValue(false)
236+
const runSpy = vi
237+
.spyOn(howToProceedStep, 'run')
238+
.mockResolvedValue({} as never)
239+
240+
await expect(
241+
implCommand({ 'continue-without-plan': true }, {}),
242+
).rejects.toBeInstanceOf(CliExit)
243+
244+
expect(confirmMock).toHaveBeenCalledTimes(1)
245+
expect(runSpy).not.toHaveBeenCalled()
246+
})
175247
})

packages/cli/src/commands/init/__tests__/init-command.test.ts

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -234,4 +234,46 @@ describe('initCommand — CI detection on the `stash plan` chain offer', () => {
234234

235235
expect(vi.mocked(p.confirm)).toHaveBeenCalledTimes(1)
236236
})
237+
238+
it('skips the chain offer when stdin is redirected even if stdout is a TTY', async () => {
239+
// The gate keys off stdin, not stdout: `stash init < /dev/null` from a
240+
// terminal (stdin piped, stdout still a TTY) must not reach the confirm.
241+
// Reinstating `process.stdout.isTTY` would keep the CI loop above green,
242+
// so only this stdin/stdout split pins the correct stream.
243+
vi.stubEnv('CI', '')
244+
Object.defineProperty(process.stdin, 'isTTY', {
245+
value: false,
246+
configurable: true,
247+
})
248+
Object.defineProperty(process.stdout, 'isTTY', {
249+
value: true,
250+
configurable: true,
251+
})
252+
253+
await expect(initCommand({}, {})).resolves.toBeUndefined()
254+
255+
expect(vi.mocked(p.confirm)).not.toHaveBeenCalled()
256+
expect(vi.mocked(p.outro)).toHaveBeenCalledWith(
257+
expect.stringContaining('--target'),
258+
)
259+
})
260+
261+
it('chains into `stash plan` when the offer is accepted', async () => {
262+
// The accept arm (outro + planCommand() + early return) is what the gate
263+
// change made reachable; the only other confirm test resolves `false`.
264+
vi.stubEnv('CI', '')
265+
vi.mocked(p.confirm).mockResolvedValueOnce(true)
266+
const { planCommand } = await import('../../plan/index.js')
267+
268+
await expect(initCommand({}, {})).resolves.toBeUndefined()
269+
270+
expect(vi.mocked(planCommand)).toHaveBeenCalledTimes(1)
271+
expect(vi.mocked(p.outro)).toHaveBeenCalledWith(
272+
expect.stringContaining('handing off to `stash plan`'),
273+
)
274+
// The accept path returns early — it must not also print the --target hint.
275+
expect(vi.mocked(p.outro)).not.toHaveBeenCalledWith(
276+
expect.stringContaining('--target'),
277+
)
278+
})
237279
})

packages/cli/src/commands/init/steps/__tests__/resolve-proxy-choice.test.ts

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,23 @@ describe('resolveProxyChoiceStep — flag short-circuit', () => {
5757
)
5858
expect(vi.mocked(p.select)).not.toHaveBeenCalled()
5959
})
60+
61+
it('returns state untouched when --no-proxy set usesProxy to false', async () => {
62+
// The guard is `state.usesProxy !== undefined`; `false` is the value that
63+
// distinguishes it from a truthiness check. Rewriting it as
64+
// `if (state.usesProxy)` would silently re-prompt for --no-proxy (and log
65+
// the misleading "No --proxy flag set" notice non-interactively).
66+
setTty(true)
67+
vi.stubEnv('CI', '')
68+
69+
const state = { ...baseState, usesProxy: false }
70+
await expect(resolveProxyChoiceStep.run(state, provider)).resolves.toEqual(
71+
state,
72+
)
73+
expect(vi.mocked(p.select)).not.toHaveBeenCalled()
74+
// An explicit --no-proxy is not "no flag set" — don't log the default notice.
75+
expect(vi.mocked(p.log.info)).not.toHaveBeenCalled()
76+
})
6077
})
6178

6279
describe('resolveProxyChoiceStep — CI detection with a TTY attached', () => {
@@ -115,4 +132,26 @@ describe('resolveProxyChoiceStep — no TTY', () => {
115132
expect(vi.mocked(p.select)).not.toHaveBeenCalled()
116133
expect(result.usesProxy).toBe(false)
117134
})
135+
136+
it('does not prompt when stdin is piped even though stdout is a TTY', async () => {
137+
// setTty() can't express this — it moves both streams together. The fix
138+
// gates on stdin, so a redirected stdin (`stash init < /dev/null` from a
139+
// terminal) must not reach the prompt even while stdout is still a TTY.
140+
// This is the case that verifies the changeset's "wrong stream" claim;
141+
// only the CI axis distinguishes old from new anywhere else.
142+
Object.defineProperty(process.stdin, 'isTTY', {
143+
value: false,
144+
configurable: true,
145+
})
146+
Object.defineProperty(process.stdout, 'isTTY', {
147+
value: true,
148+
configurable: true,
149+
})
150+
vi.stubEnv('CI', '')
151+
152+
const result = await resolveProxyChoiceStep.run(baseState, provider)
153+
154+
expect(vi.mocked(p.select)).not.toHaveBeenCalled()
155+
expect(result.usesProxy).toBe(false)
156+
})
118157
})
Lines changed: 58 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,58 @@
1+
import fs from 'node:fs'
2+
import os from 'node:os'
3+
import path from 'node:path'
4+
import { afterEach, beforeEach, describe, expect, it } from 'vitest'
5+
import { render } from '../helpers/pty.js'
6+
7+
/**
8+
* E2E: the real-hang reproducer. Under a genuine PTY (`process.stdin.isTTY`
9+
* is true) with `CI` set to a non-`'true'` spelling, the pre-fix inline
10+
* `CI !== 'true'` gate treated the run as interactive and blocked on the
11+
* plan-summary confirm forever — a hang, not an error.
12+
*
13+
* The unit suites mock `@clack/prompts` and assert the gate's boolean; only a
14+
* real PTY proves the process terminates. `pty.ts` `render()` defaults
15+
* `env.CI` to `'true'`, which is exactly why that regression never surfaced
16+
* here before — overriding to `1` / `TRUE` is the precise reproducer.
17+
* `impl-non-tty.e2e.test.ts` uses `runPiped`, which gives `isTTY === false`
18+
* by construction and cannot reach this path.
19+
*/
20+
describe('stash impl — PTY + CI (real-hang reproducer)', () => {
21+
let tmpDir: string
22+
23+
beforeEach(() => {
24+
tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'stash-impl-pty-ci-e2e-'))
25+
fs.mkdirSync(path.join(tmpDir, '.cipherstash'))
26+
fs.writeFileSync(
27+
path.join(tmpDir, '.cipherstash', 'context.json'),
28+
JSON.stringify({
29+
integration: 'postgresql',
30+
packageManager: 'npm',
31+
schemas: [],
32+
}),
33+
)
34+
// A plan on disk is what makes the pre-fix code reach the plan-summary
35+
// confirm (the prompt that hung); without one it exits earlier.
36+
fs.writeFileSync(path.join(tmpDir, '.cipherstash', 'plan.md'), '# Plan')
37+
})
38+
39+
afterEach(() => {
40+
if (tmpDir && fs.existsSync(tmpDir)) {
41+
fs.rmSync(tmpDir, { recursive: true, force: true })
42+
}
43+
})
44+
45+
for (const ciValue of ['1', 'TRUE']) {
46+
it(`exits instead of hanging under a PTY with CI=${ciValue}`, async () => {
47+
const r = render(['impl'], { cwd: tmpDir, env: { CI: ciValue } })
48+
// A regression re-opens the hang; the kill is the backstop so the suite
49+
// fails on timeout instead of blocking. The fix exits in ~100ms.
50+
const timer = setTimeout(() => r.kill('SIGKILL'), 10_000)
51+
const { exitCode } = await r.exit
52+
clearTimeout(timer)
53+
54+
expect(exitCode).toBe(0)
55+
expect(r.output).not.toContain('Proceed with implementation')
56+
})
57+
}
58+
})

0 commit comments

Comments
 (0)