From 36bb0f1b092847b7a1e7075f621de93356eafaba Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 21 Oct 2026 10:37:33 +0100 Subject: [PATCH] fix(interactive): keep the shell alive on Ctrl+C at a y/n prompt Pressing Ctrl+C at a promptForConfirmation prompt inside `ably interactive` exited the whole shell with code 0 instead of cancelling the command. With no SIGINT listener, readline handles Ctrl+C by silently closing the confirm's interface, so its promise never settles. The shell's readline is paused while a command runs, so nothing keeps the event loop alive and Node drains it and exits. It was not the shell's readline 'close' handler: that would exit 42 in wrapper mode, and --trace-exit shows no process.exit call. The prompt now closes and rejects with an InterruptError on Ctrl+C. Callers already route rejections through fail(), and the shell's catch returns to the prompt. It rejects rather than answering "no" because several callers (login, invalid-key removal, init) carry on when the answer is no. Co-Authored-By: Claude Opus 5.5 --- docs/Exit-Codes.md | 1 + docs/Project-Structure.md | 2 +- src/utils/prompt-confirmation.ts | 23 ++++++- .../interactive-confirm-sigint.test.ts | 65 +++++++++++++++++++ test/unit/utils/prompt-confirmation.test.ts | 35 ++++++++-- 5 files changed, 117 insertions(+), 9 deletions(-) create mode 100644 test/tty/commands/interactive-confirm-sigint.test.ts diff --git a/docs/Exit-Codes.md b/docs/Exit-Codes.md index 6a10290c0..2fa147ffa 100644 --- a/docs/Exit-Codes.md +++ b/docs/Exit-Codes.md @@ -33,6 +33,7 @@ This document describes the exit codes used by the Ably CLI, particularly in int ### Single Ctrl+C - **At empty prompt**: Shows "^C" and message about typing 'exit' to quit - **During command execution**: Interrupts the command and returns to prompt +- **At a command's y/n confirmation prompt**: Cancels the command ("Interrupted by user") and returns to prompt - **With partial command typed**: Clears the line and returns to prompt ### Double Ctrl+C (within 500ms) diff --git a/docs/Project-Structure.md b/docs/Project-Structure.md index 07c5fa8c6..8f4e72829 100644 --- a/docs/Project-Structure.md +++ b/docs/Project-Structure.md @@ -132,7 +132,7 @@ This document outlines the directory structure of the Ably CLI project. │ │ └── interactive-mode.test.ts # Interactive REPL integration tests │ ├── tty/ # TTY-dependent tests (node-pty, local only) │ │ ├── tty-test-helper.ts # PTY spawn/wait/write helpers -│ │ └── commands/ # Interactive SIGINT tests +│ │ └── commands/ # Interactive SIGINT tests (prompt, running command, confirmation) │ ├── e2e/ # End-to-end tests against real Ably │ │ ├── auth/ # Auth E2E tests │ │ ├── bench/ # Benchmark E2E tests diff --git a/src/utils/prompt-confirmation.ts b/src/utils/prompt-confirmation.ts index eb61131a5..b58e1dd77 100644 --- a/src/utils/prompt-confirmation.ts +++ b/src/utils/prompt-confirmation.ts @@ -1,9 +1,20 @@ import * as readline from "node:readline"; +/** Rejection reason when the user presses Ctrl+C at a confirmation prompt. */ +export class InterruptError extends Error { + constructor() { + super("Interrupted by user"); + this.name = "InterruptError"; + } +} + /** * Prompts the user for confirmation with a yes/no question. * Accepts both "y" and "yes" as affirmative responses (case-insensitive). * + * Ctrl+C rejects with an `InterruptError` rather than answering "no", because + * some callers carry on with their flow when the answer is no. + * * @param message - The confirmation message to display to the user * @param options.defaultYes - If true, an empty answer counts as yes and the * default suffix becomes " [Y/n]". Use only for non-destructive prompts. @@ -27,7 +38,17 @@ export function promptForConfirmation( ? message : `${message} ${suffix}`; - return new Promise((resolve) => { + return new Promise((resolve, reject) => { + // Without a SIGINT listener, readline handles Ctrl+C by silently closing + // the interface, leaving this promise pending forever. Inside + // `ably interactive` nothing else then keeps the event loop alive, so Node + // drains it and the whole shell exits with code 0. + rl.on("SIGINT", () => { + rl.close(); + process.stdout.write("\n"); + reject(new InterruptError()); + }); + rl.question(promptMessage, (answer) => { rl.close(); const response = answer.toLowerCase().trim(); diff --git a/test/tty/commands/interactive-confirm-sigint.test.ts b/test/tty/commands/interactive-confirm-sigint.test.ts new file mode 100644 index 000000000..b1a26b574 --- /dev/null +++ b/test/tty/commands/interactive-confirm-sigint.test.ts @@ -0,0 +1,65 @@ +/** + * Ctrl+C at a y/n confirmation prompt inside `ably interactive` must cancel + * the command and return to the shell prompt, not exit the shell. + * + * Run locally with: pnpm test:tty + * NOT run in CI — GitHub Actions runners have no TTY. + */ +import { describe, it, expect, afterEach } from "vitest"; + +import { + spawnTty, + waitForOutput, + writeTty, + sendCtrlC, + killTty, + PROMPT_PATTERN, + type TtyProcess, +} from "../tty-test-helper.js"; + +describe("Interactive Mode - Ctrl+C at a confirmation prompt (TTY)", () => { + const timeout = 20000; + let proc: TtyProcess | null = null; + + afterEach(async () => { + if (proc) { + await killTty(proc); + proc = null; + } + }); + + it( + "cancels the command and keeps the shell running", + async () => { + proc = spawnTty(["interactive"], { + // Run the real exit path rather than test-mode stubs. + ABLY_CLI_TEST_MODE: undefined, + // `push devices remove` prompts before making any network call. + ABLY_API_KEY: "app.key:secret", + }); + + await waitForOutput(proc, PROMPT_PATTERN); + + writeTty(proc, "push devices remove dev1\r"); + await waitForOutput(proc, "[y/n]"); + + sendCtrlC(proc); + + // The command unwinds with an interrupt error and the shell shows a + // fresh prompt instead of exiting. + await waitForOutput(proc, /Interrupted by user[\s\S]*ably>/); + expect(proc.exited).toBe(false); + + // The shell still accepts and runs commands. + writeTty(proc, "version\r"); + await waitForOutput(proc, "Version: "); + expect(proc.exited).toBe(false); + + writeTty(proc, "exit\r"); + const { code } = await proc.exitPromise; + // 42 = user exit via `exit` in wrapper mode + expect(code).toBe(42); + }, + timeout, + ); +}); diff --git a/test/unit/utils/prompt-confirmation.test.ts b/test/unit/utils/prompt-confirmation.test.ts index bbbb827aa..f6c166426 100644 --- a/test/unit/utils/prompt-confirmation.test.ts +++ b/test/unit/utils/prompt-confirmation.test.ts @@ -1,17 +1,29 @@ import { describe, it, expect, vi, beforeEach } from "vitest"; let mockQuestion: (query: string, callback: (answer: string) => void) => void; +let mockClose: ReturnType; +let mockListeners: Record void>; vi.mock("node:readline", () => ({ - createInterface: () => ({ - close: vi.fn(), - question: (query: string, callback: (answer: string) => void) => { - mockQuestion(query, callback); - }, - }), + createInterface: () => { + mockClose = vi.fn(); + mockListeners = {}; + return { + close: mockClose, + on: (event: string, listener: () => void) => { + mockListeners[event] = listener; + }, + question: (query: string, callback: (answer: string) => void) => { + mockQuestion(query, callback); + }, + }; + }, })); -import { promptForConfirmation } from "../../../src/utils/prompt-confirmation.js"; +import { + InterruptError, + promptForConfirmation, +} from "../../../src/utils/prompt-confirmation.js"; describe("promptForConfirmation", () => { beforeEach(() => { @@ -46,6 +58,15 @@ describe("promptForConfirmation", () => { expect(capturedQuery).toBe("Are you sure? [y/n]"); }); + it("rejects with InterruptError and closes the prompt on Ctrl+C", async () => { + vi.spyOn(process.stdout, "write").mockReturnValue(true); + mockQuestion = () => {}; + const result = promptForConfirmation("Delete this?"); + mockListeners.SIGINT(); + await expect(result).rejects.toBeInstanceOf(InterruptError); + expect(mockClose).toHaveBeenCalled(); + }); + describe("defaultYes", () => { it("appends [Y/n] suffix when defaultYes is true", async () => { let capturedQuery = "";