diff --git a/packages/cli-engine/package.json b/packages/cli-engine/package.json index 11a9b67e..029d25ac 100644 --- a/packages/cli-engine/package.json +++ b/packages/cli-engine/package.json @@ -1,6 +1,6 @@ { "name": "@prisma/cli-engine", - "version": "0.6.0", + "version": "0.6.1", "description": "The execution engine of the unified Prisma CLI.", "type": "module", "exports": { diff --git a/packages/cli-engine/src/context.ts b/packages/cli-engine/src/context.ts index 1e33be70..24c19db6 100644 --- a/packages/cli-engine/src/context.ts +++ b/packages/cli-engine/src/context.ts @@ -219,10 +219,10 @@ export interface PromptSurface { * `token` is the natural noun of what is being consented to — an app * name, a hostname. Supplying one changes both halves of the prompt: * interactively the user must type the token exactly instead of - * answering yes/no, and non-interactively the consent is granted by - * `--confirm ` on the command line (one `--confirm` value per - * consent). Without a token there is no non-interactive way to - * consent at all. + * answering yes/no, and `--confirm ` on the command line grants + * the consent up front in any session, so no prompt is shown (one + * `--confirm` value per consent). Without a token there is no + * non-interactive way to consent at all. */ readonly consent: ( question: string, diff --git a/packages/cli-engine/src/execution/engine.ts b/packages/cli-engine/src/execution/engine.ts index d08bee93..00091426 100644 --- a/packages/cli-engine/src/execution/engine.ts +++ b/packages/cli-engine/src/execution/engine.ts @@ -154,8 +154,8 @@ export interface RunState { usageErrorText: string | undefined; internalErrorText: string | undefined; stricliStderr: string; - /** The stdin iterator a prompt opened, closed when the run settles so - * a real process's stdin never keeps the event loop alive. */ + /** The stdin iterator a prompt opened, returned when the run settles + * so a real process's stdin never keeps the event loop alive. */ stdinIterator: AsyncIterator | undefined; /** The run's raw argv — consulted only to derive which flag NAMES * were explicitly passed for the settlement snapshot. */ @@ -227,6 +227,19 @@ export function buildEngine( return new EngineImpl(spec, options?.now, options?.delay); } +/** Returning the iterator is how the engine tells the host it has + * stopped reading. On a real terminal the host's iterator may be + * awaiting a keystroke that never comes, and its return cannot settle + * before the host stops reading, so the run does not wait for it. */ +function releaseStdin(state: RunState): void { + const iterator = state.stdinIterator; + if (iterator === undefined) { + return; + } + state.stdinIterator = undefined; + void iterator.return?.()?.catch(() => {}); +} + /** Resolves on the timer OR on the signal, whichever comes first — the * caller decides what an abort means, and no timer outlives the run. */ function waitFor(ms: number, signal: AbortSignal): Promise { @@ -438,7 +451,7 @@ export class EngineImpl implements Engine { }); } finally { unsubscribe(); - await state.stdinIterator?.return?.(); + releaseStdin(state); } const exitCode = state.settledExitCode !== undefined diff --git a/packages/cli-engine/src/execution/prompts.ts b/packages/cli-engine/src/execution/prompts.ts index 5bf9d7f7..a668595e 100644 --- a/packages/cli-engine/src/execution/prompts.ts +++ b/packages/cli-engine/src/execution/prompts.ts @@ -5,10 +5,10 @@ * default resolves to it without displaying; one without a default * HALTS the invocation with a structured error (the engine renders the * errored envelope, exit 2). consent is structurally undefaultable: --yes - * never grants it, and outside an interactive terminal the only thing - * that can is a matching `--confirm ` when the consent declares a - * token. Cancellation (EOF at the prompt) is a distinct structured error - * mapped to exit 3. + * never grants it. A matching `--confirm ` grants it in every + * session, interactive or not, before anything is rendered; outside an + * interactive terminal that is the only thing that can. Cancellation + * (EOF at the prompt) is a distinct structured error mapped to exit 3. * * Rendering is two-tier: real TTYs (isTty.stdin AND stdin.setRawMode * present, no scripted answers) render through @clack/prompts via @@ -412,10 +412,10 @@ export function makePromptSurface(invocation: Invocation): PromptSurface { }, consent: async (question, opts) => { const token = opts?.token; + if (token !== undefined && consumeConfirmValue(state, token)) { + return true; + } if (state.yes || !state.interactive) { - if (token !== undefined && consumeConfirmValue(state, token)) { - return true; - } throw consentUnavailable(question, state, token); } if (token !== undefined) { diff --git a/packages/cli-engine/src/runtime.ts b/packages/cli-engine/src/runtime.ts index 7f4c3da2..7bbb620e 100644 --- a/packages/cli-engine/src/runtime.ts +++ b/packages/cli-engine/src/runtime.ts @@ -177,6 +177,12 @@ export interface HostProcess { readonly stdin: { isTTY?: boolean; setRawMode?(enabled: boolean): unknown; + /** Whether a stdin still being read keeps the process alive. The + * engine reads through the bin's adapter, which unrefs stdin once + * a run has finished prompting so an idle process can exit, and + * refs it again when the next run starts reading. */ + ref?(): unknown; + unref?(): unknown; [Symbol.asyncIterator](): AsyncIterator; }; on(event: "SIGINT" | "SIGTERM", listener: () => void): unknown; diff --git a/packages/cli-engine/tests/clack-prompts.test.ts b/packages/cli-engine/tests/clack-prompts.test.ts index 92b8fbd1..b9c391ed 100644 --- a/packages/cli-engine/tests/clack-prompts.test.ts +++ b/packages/cli-engine/tests/clack-prompts.test.ts @@ -21,10 +21,16 @@ const ENTER = "\r"; const CTRL_C = "\x03"; const BACKSPACE = "\x7f"; -function keystrokeStdin(keys: readonly string[]) { +/** `returnSettles: false` behaves like Node's own stdin iterator, whose + * return cannot settle while it is awaiting a keystroke. */ +function keystrokeStdin( + keys: readonly string[], + opts: { returnSettles: boolean } = { returnSettles: true }, +) { let cursor = 0; const encoder = new TextEncoder(); const rawModeCalls: boolean[] = []; + let returned = 0; const stdin = { setRawMode: (enabled: boolean) => { rawModeCalls.push(enabled); @@ -39,13 +45,15 @@ function keystrokeStdin(keys: readonly string[]) { cursor += 1; setTimeout(() => resolve({ done: false, value }), 5); }), - return: async (): Promise> => ({ - done: true, - value: undefined, - }), + return: (): Promise> => { + returned += 1; + return opts.returnSettles + ? Promise.resolve({ done: true, value: undefined }) + : new Promise(() => {}); + }, }), }; - return { stdin, rawModeCalls }; + return { stdin, rawModeCalls, returnCount: () => returned }; } function promptCli(run: (prompt: PromptSurface) => Promise) { @@ -84,8 +92,9 @@ function promptCli(run: (prompt: PromptSurface) => Promise) { async function runInteractive( run: (prompt: PromptSurface) => Promise, keys: readonly string[], + opts?: { returnSettles: boolean }, ) { - const { stdin, rawModeCalls } = keystrokeStdin(keys); + const { stdin, rawModeCalls, returnCount } = keystrokeStdin(keys, opts); let stdout = ""; let stderr = ""; const runtime: Runtime = { @@ -121,7 +130,7 @@ async function runInteractive( const exitCode = await promptCli(run).run(["probe"], runtime); // biome-ignore lint/suspicious/noControlCharactersInRegex: ANSI stripping const plainStderr = stderr.replace(/\x1b\[[0-9;]*[A-Za-z]/g, ""); - return { exitCode, stdout, stderr, plainStderr, rawModeCalls }; + return { exitCode, stdout, stderr, plainStderr, rawModeCalls, returnCount }; } function answerIn(plainStderr: string): string | undefined { @@ -309,6 +318,20 @@ describe("clack tier channels and raw mode", () => { }); }); +describe("clack tier stdin release", () => { + test("the run settles even when the stdin iterator's return never does", async () => { + const result = await runInteractive( + (prompt) => prompt.confirm("Proceed?", { default: true }), + [ENTER], + { returnSettles: false }, + ); + + expect(result.exitCode).toBe(0); + expect(answerIn(result.plainStderr)).toBe("true"); + expect(result.returnCount()).toBe(1); + }); +}); + describe("clack tier cancellation", () => { test("\\x03 during a prompt maps to CLI.PROMPT_CANCELLED, exit 3", async () => { const result = await runInteractive( diff --git a/packages/cli-engine/tests/interaction-affordances.test.ts b/packages/cli-engine/tests/interaction-affordances.test.ts index 41d5d5e5..4c900ef3 100644 --- a/packages/cli-engine/tests/interaction-affordances.test.ts +++ b/packages/cli-engine/tests/interaction-affordances.test.ts @@ -99,6 +99,36 @@ describe("consent tokens", () => { expect(errorOf(result)?.summary).toContain("exactly prod-db"); }); + test("--confirm with the token grants the consent interactively without prompting", async () => { + const cli = createTestCli({ + commands: { probe: promptProbe(dropDatabase) }, + now: EPOCH, + }); + const result = await cli.run(["probe", "--confirm", "prod-db"], { + ...INTERACTIVE, + stdin: "", + }); + + expect(result.exitCode).toBe(0); + expect(result.presented?.data).toEqual({ answer: true }); + expect(result.stderr).not.toContain("type prod-db to confirm"); + }); + + test("--confirm with a different value still prompts interactively", async () => { + const cli = createTestCli({ + commands: { probe: promptProbe(dropDatabase) }, + now: EPOCH, + }); + const result = await cli.run(["probe", "--confirm", "staging-db"], { + ...INTERACTIVE, + stdin: "prod-db\n", + }); + + expect(result.exitCode).toBe(0); + expect(result.presented?.data).toEqual({ answer: true }); + expect(result.stderr).toContain("type prod-db to confirm"); + }); + test("--confirm with the token grants the consent non-interactively", async () => { const cli = createTestCli({ commands: { probe: promptProbe(dropDatabase) }, diff --git a/packages/cli/package.json b/packages/cli/package.json index fbd114d0..a4d29c43 100644 --- a/packages/cli/package.json +++ b/packages/cli/package.json @@ -49,7 +49,7 @@ }, "dependencies": { "@manypkg/tools": "^2.1.2", - "@prisma/cli-engine": "workspace:0.6.0", + "@prisma/cli-engine": "workspace:0.6.1", "@prisma/composer-cli": "0.21.0", "@prisma/compute-sdk": "0.42.0", "@prisma/management-api-sdk": "1.69.0", diff --git a/packages/cli/src/runtime.ts b/packages/cli/src/runtime.ts index 9daa5e5d..4ba5599f 100644 --- a/packages/cli/src/runtime.ts +++ b/packages/cli/src/runtime.ts @@ -115,16 +115,39 @@ function memoizedConfigLoader( }; } -export async function assembleRuntime(proc: HostProcess): Promise { - const stdin: InputStream = { +/** + * The engine returns the iterator when it has finished prompting. Node's + * stdin iterator cannot honour that while it is awaiting a keystroke, + * and a terminal being read keeps the process alive, so the adapter + * unrefs stdin itself and settles at once. The underlying return is + * left to complete whenever it can, and the next run refs stdin again + * before it reads. + */ +export function makeStdin(proc: HostProcess): InputStream { + return { setRawMode: proc.stdin.isTTY === true && proc.stdin.setRawMode !== undefined ? (enabled) => { proc.stdin.setRawMode?.(enabled); } : undefined, - [Symbol.asyncIterator]: () => proc.stdin[Symbol.asyncIterator](), + [Symbol.asyncIterator]: () => { + proc.stdin.ref?.(); + const chunks = proc.stdin[Symbol.asyncIterator](); + return { + next: () => chunks.next(), + return: async () => { + proc.stdin.unref?.(); + void chunks.return?.()?.catch(() => {}); + return { done: true, value: undefined }; + }, + }; + }, }; +} + +export async function assembleRuntime(proc: HostProcess): Promise { + const stdin = makeStdin(proc); warnOnDeprecatedStateFileEnvVar(proc); const apiBaseUrl = getApiBaseUrl(proc.env); const authBaseUrl = getAuthBaseUrl(proc.env); diff --git a/packages/cli/tests/bin.test.ts b/packages/cli/tests/bin.test.ts index 61e34041..c814b76f 100644 --- a/packages/cli/tests/bin.test.ts +++ b/packages/cli/tests/bin.test.ts @@ -215,6 +215,32 @@ describe("assembleRuntime", () => { expect(proc.stderrText).toBe("err"); }); + it("unrefs stdin when the engine returns its iterator, and refs it for the next reader", async () => { + const calls: string[] = []; + const proc = makeProcess({ isTty: { stdin: true } }); + Object.assign(proc.stdin, { + ref: () => calls.push("ref"), + unref: () => calls.push("unref"), + [Symbol.asyncIterator]: () => ({ + next: () => new Promise>(() => {}), + return: () => new Promise>(() => {}), + }), + }); + const runtime = await assembleRuntime(proc); + + const iterator = runtime.stdin[Symbol.asyncIterator](); + void iterator.next(); + const returned = await Promise.race([ + iterator.return?.(), + new Promise<"hung">((resolve) => setTimeout(() => resolve("hung"), 200)), + ]); + + expect(returned).toEqual({ done: true, value: undefined }); + expect(calls).toEqual(["ref", "unref"]); + runtime.stdin[Symbol.asyncIterator](); + expect(calls).toEqual(["ref", "unref", "ref"]); + }); + /** * The engine reads stderr's width at render time so a terminal * resized mid-run reports its new size. That only holds if the bin diff --git a/packages/cli/tests/fixtures/prompting-bin.ts b/packages/cli/tests/fixtures/prompting-bin.ts new file mode 100644 index 00000000..574e9169 --- /dev/null +++ b/packages/cli/tests/fixtures/prompting-bin.ts @@ -0,0 +1,39 @@ +// A bin that prompts once through the real process adapter, for the +// pseudo-terminal exit test. Prints the run's exit code once the engine +// has settled, so the driver can tell "settled but still alive" from +// "never settled". +import { type Block, createCli, defineCommand } from "@prisma/cli-engine"; +import { ok } from "@prisma/cli-engine/protocol"; +import { assembleRuntime } from "../../src/runtime"; + +const probe = defineCommand({ + help: { summary: "Prompt probe" }, + handler: async (_args, ctx) => { + const answer = await ctx.prompt.confirm("Proceed?", { default: true }); + return ok( + ctx.present( + { data: { answer } }, + { + human: (): readonly Block[] => [ + { kind: "summary", status: "ok", text: `answer=${answer}` }, + ], + stdout: () => [], + json: () => ({ answer }), + next: () => [], + }, + ), + ); + }, +}); + +const cli = createCli({ + name: "probe", + version: "0.0.0", + commandFamilies: [], + groups: {}, + commands: { probe }, +}); +const runtime = await assembleRuntime(process); +const exitCode = await cli.run(["probe", "--interactive"], runtime); +process.stderr.write(`[settled ${exitCode}]\n`); +process.exitCode = exitCode; diff --git a/packages/cli/tests/fixtures/pty-driver.py b/packages/cli/tests/fixtures/pty-driver.py new file mode 100644 index 00000000..62321bd5 --- /dev/null +++ b/packages/cli/tests/fixtures/pty-driver.py @@ -0,0 +1,45 @@ +# Runs a command under a pseudo-terminal, answers the first prompt with +# Enter, and reports whether the process exited on its own. Output: +# one JSON object on stdout. +import fcntl, json, os, pty, select, signal, struct, subprocess, sys, termios, time + +PROMPT, SETTLED = b"Proceed?", b"[settled" +argv = sys.argv[1:] +master, slave = pty.openpty() +fcntl.ioctl(slave, termios.TIOCSWINSZ, struct.pack("HHHH", 40, 100, 0, 0)) +child = subprocess.Popen(argv, stdin=slave, stdout=slave, stderr=slave, close_fds=True) +os.close(slave) + +out = b"" +deadline = time.monotonic() + 20 +answered = settled = False +exited = None +while time.monotonic() < deadline: + ready, _, _ = select.select([master], [], [], 0.1) + if ready: + try: + chunk = os.read(master, 4096) + except OSError: + chunk = b"" + out += chunk + if not answered and PROMPT in out: + time.sleep(0.3) + os.write(master, b"\r") + answered = True + if not settled and SETTLED in out: + settled = True + deadline = time.monotonic() + 5 + exited = child.poll() + if exited is not None: + break + +if exited is None: + child.send_signal(signal.SIGKILL) + child.wait() +print(json.dumps({ + "answered": answered, + "settled": settled, + "exitedOnItsOwn": exited is not None, + "exitCode": exited, + "output": out.decode("utf-8", "replace")[-2000:], +})) diff --git a/packages/cli/tests/pty-exit.test.ts b/packages/cli/tests/pty-exit.test.ts new file mode 100644 index 00000000..3a183e51 --- /dev/null +++ b/packages/cli/tests/pty-exit.test.ts @@ -0,0 +1,50 @@ +/** + * A prompting run in a real terminal must let the process exit once the + * command has finished. Node's stdin keeps the event loop alive while it + * is being read, and its async iterator cannot be returned while it is + * awaiting a keystroke, so the bin's stdin adapter has to release it + * itself. Only a pseudo-terminal shows this: a fake stdin has no handle + * to keep the loop alive. + */ +import { execFileSync, spawnSync } from "node:child_process"; +import { fileURLToPath } from "node:url"; +import { describe, expect, it } from "vitest"; + +const DRIVER = fileURLToPath( + new URL("./fixtures/pty-driver.py", import.meta.url), +); +const BIN = fileURLToPath( + new URL("./fixtures/prompting-bin.ts", import.meta.url), +); +const TSX = fileURLToPath(new URL("../node_modules/.bin/tsx", import.meta.url)); + +function python(): string | undefined { + const probe = spawnSync("python3", ["-c", "import pty"], { stdio: "ignore" }); + return probe.status === 0 ? "python3" : undefined; +} + +describe.skipIf(python() === undefined || process.platform === "win32")( + "a prompting run under a pseudo-terminal", + () => { + it("exits on its own after the result is presented", () => { + const report = JSON.parse( + execFileSync("python3", [DRIVER, TSX, BIN], { + encoding: "utf8", + timeout: 60_000, + env: { ...process.env, PRISMA_DISABLE_TELEMETRY: "1" }, + }), + ) as { + answered: boolean; + settled: boolean; + exitedOnItsOwn: boolean; + exitCode: number | null; + output: string; + }; + + expect(report.answered, report.output).toBe(true); + expect(report.settled, report.output).toBe(true); + expect(report.exitedOnItsOwn, report.output).toBe(true); + expect(report.exitCode).toBe(0); + }); + }, +); diff --git a/packages/prisma/package.json b/packages/prisma/package.json index d0b4245e..6e240427 100644 --- a/packages/prisma/package.json +++ b/packages/prisma/package.json @@ -50,7 +50,7 @@ }, "dependencies": { "@manypkg/tools": "^2.1.2", - "@prisma/cli-engine": "workspace:0.6.0", + "@prisma/cli-engine": "workspace:0.6.1", "@prisma/composer-cli": "0.21.0", "@prisma/compute-sdk": "0.42.0", "@prisma/management-api-sdk": "1.69.0", diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 5c6988eb..1263ccee 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -27,7 +27,7 @@ importers: specifier: ^2.1.2 version: 2.1.2 '@prisma/cli-engine': - specifier: workspace:0.6.0 + specifier: workspace:0.6.1 version: link:../cli-engine '@prisma/composer-cli': specifier: 0.21.0 @@ -210,7 +210,7 @@ importers: specifier: ^2.1.2 version: 2.1.2 '@prisma/cli-engine': - specifier: workspace:0.6.0 + specifier: workspace:0.6.1 version: link:../cli-engine '@prisma/composer-cli': specifier: 0.21.0