From ffd7723e472184664f03f8a4cffdfebde3a9ab2e Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 21 Oct 2026 10:29:22 +0100 Subject: [PATCH 1/2] fix(accounts:login): make Ctrl+C cancel login reliably Ctrl+C during `ably accounts login` either did nothing or left Node reporting "Detected unsettled top-level await". The SIGINT handler added for polling never ran in a real terminal: - ora's default stdin discarding puts the TTY in raw mode and then never reads the Ctrl+C byte, so no SIGINT is raised. Disable it for the login, status and support ask spinners. - The handler called this.exit(130) inside a signal listener, which surfaces as an uncaught ExitError rather than a clean exit. - After the spinner has run, inquirer's re-raised SIGINT at a select prompt is delivered only on a later loop turn; with the prompt closed Node runs out of work first and exits with code 13. Add withSigintCancellation, which owns SIGINT for the whole login flow, aborts polling, keeps the loop alive on a TTY so re-raised signals are delivered, and rejects so the command exits 130 from its async path. Readline prompts now forward Ctrl+C as a process SIGINT instead of silently pausing. Refs DX-1967 Co-Authored-By: Claude Opus 5.5 --- docs/Exit-Codes.md | 1 + docs/Project-Structure.md | 3 +- src/commands/accounts/login.ts | 75 +++++-- src/commands/status.ts | 5 +- src/commands/support/ask.ts | 2 +- src/utils/prompt-confirmation.ts | 8 +- src/utils/sigint-cancellation.ts | 84 ++++++++ .../commands/accounts-login-sigint.test.ts | 186 ++++++++++++++++++ test/tty/tty-test-helper.ts | 11 +- test/unit/commands/accounts/login.test.ts | 32 +++ test/unit/utils/prompt-confirmation.test.ts | 6 +- test/unit/utils/sigint-cancellation.test.ts | 70 +++++++ 12 files changed, 445 insertions(+), 38 deletions(-) create mode 100644 src/utils/sigint-cancellation.ts create mode 100644 test/tty/commands/accounts-login-sigint.test.ts create mode 100644 test/unit/utils/sigint-cancellation.test.ts diff --git a/docs/Exit-Codes.md b/docs/Exit-Codes.md index 2fa147ffa..e4c134056 100644 --- a/docs/Exit-Codes.md +++ b/docs/Exit-Codes.md @@ -55,6 +55,7 @@ This document describes the exit codes used by the Ably CLI, particularly in int Exit codes are handled in: - `src/commands/interactive.ts`: Sets exit code 42 for user exit - `src/utils/sigint-exit.ts`: Handles SIGINT behavior and exit code 130 +- `src/utils/sigint-cancellation.ts`: Turns Ctrl+C into exit code 130 for commands that combine spinners and prompts (e.g. `accounts login`) --- diff --git a/docs/Project-Structure.md b/docs/Project-Structure.md index 8f4e72829..a0d1abfe2 100644 --- a/docs/Project-Structure.md +++ b/docs/Project-Structure.md @@ -89,6 +89,7 @@ This document outlines the directory structure of the Ably CLI project. │ ├── prompt-confirmation.ts # Y/N confirmation prompts │ ├── prompt-value.ts # Free-text and masked value prompts │ ├── readline-helper.ts # Readline utilities for interactive mode +│ ├── sigint-cancellation.ts # Ctrl+C cancellation for spinner + prompt flows (e.g. login) │ ├── sigint-exit.ts # SIGINT/Ctrl+C handling (exit code 130) │ ├── string-distance.ts # Levenshtein distance for fuzzy matching │ ├── terminal-diagnostics.ts # Terminal capability detection @@ -132,7 +133,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 (prompt, running command, confirmation) +│ │ └── commands/ # Ctrl+C tests (interactive prompt, running command, confirmation, accounts login) │ ├── e2e/ # End-to-end tests against real Ably │ │ ├── auth/ # Auth E2E tests │ │ ├── bench/ # Benchmark E2E tests diff --git a/src/commands/accounts/login.ts b/src/commands/accounts/login.ts index 901950bc6..928e6e2b6 100644 --- a/src/commands/accounts/login.ts +++ b/src/commands/accounts/login.ts @@ -1,4 +1,4 @@ -import { Flags } from "@oclif/core"; +import { Flags, Interfaces } from "@oclif/core"; import chalk from "chalk"; import * as readline from "node:readline"; import ora from "ora"; @@ -21,8 +21,15 @@ import openUrl from "../../utils/open-url.js"; import { formatResource } from "../../utils/output.js"; import { promptForConfirmation } from "../../utils/prompt-confirmation.js"; import { promptForValue } from "../../utils/prompt-value.js"; +import { + forwardSigintToProcess, + InterruptError, + withSigintCancellation, +} from "../../utils/sigint-cancellation.js"; import { pickUniqueAlias, slugifyAccountName } from "../../utils/slugify.js"; +type LoginFlags = Interfaces.InferredFlags; + /** Offered as the default when the guided local flow prompts for a URL. */ const DEFAULT_LOCAL_URL = "http://localhost:8081"; @@ -99,6 +106,22 @@ export default class AccountsLogin extends ControlBaseCommand { public async run(): Promise { const { flags } = await this.parse(AccountsLogin); + // Login strings together a spinner and several interactive prompts, and + // the libraries behind them leave Ctrl+C unhandled when combined. Own + // SIGINT for the whole flow and exit from here instead. + try { + await withSigintCancellation((signal) => this.login(flags, signal)); + } catch (error) { + if (error instanceof InterruptError) { + // Match the conventional exit code for SIGINT (128 + signal number). + this.exit(130); + } + + throw error; + } + } + + private async login(flags: LoginFlags, signal: AbortSignal): Promise { // Display ASCII art logo if not in JSON mode and the caller hasn't // opted out (e.g. `ably init` already prints the logo and delegates here). if (!this.shouldOutputJson(flags) && !flags["skip-logo"]) { @@ -114,8 +137,11 @@ export default class AccountsLogin extends ControlBaseCommand { let oauthTokens: OAuthTokens; try { - oauthTokens = await this.oauthLogin(flags); + oauthTokens = await this.oauthLogin(flags, signal); } catch (error) { + // A cancelled login has already been reported by the spinner; `run` + // handles the exit. + if (signal.aborted) throw error; this.fail(error, flags, "accountLogin"); } @@ -719,7 +745,10 @@ export default class AccountsLogin extends ControlBaseCommand { } } - private async oauthLogin(flags: BaseFlags): Promise { + private async oauthLogin( + flags: BaseFlags, + signal: AbortSignal, + ): Promise { const oauthClient = new OAuthClient({ oauthHost: flags["oauth-host"], }); @@ -754,38 +783,40 @@ export default class AccountsLogin extends ControlBaseCommand { this.log("Open the URL above in your browser to authorize."); } + // ora discards stdin by default, which puts the terminal in raw mode so + // Ctrl+C arrives as a byte rather than a signal. Its stdin-discarder then + // never reads that byte (it attaches with prependListener, which doesn't + // start a fresh stdin flowing), so Ctrl+C would be silently swallowed. const spinner = this.shouldOutputJson(flags) ? undefined - : ora("Waiting for authorization...").start(); - - // Wire up SIGINT so Ctrl+C during polling aborts the fetch and exits - // cleanly. Without this, the unsettled top-level await in bin/run.js - // triggers a confusing "Detected unsettled top-level await" warning - // from Node when the user hits Ctrl+C. - const abortController = new AbortController(); - const onSigint = () => { - abortController.abort(); - spinner?.fail("Authentication cancelled."); - // Match the conventional exit code for SIGINT (128 + signal number). - this.exit(130); - }; - process.once("SIGINT", onSigint); + : ora({ + discardStdin: false, + text: "Waiting for authorization...", + }).start(); + + // Abort listeners run synchronously on Ctrl+C, so the spinner is stopped + // before the interruption unwinds the command. + const onAbort = () => spinner?.fail("Authentication cancelled."); + signal.addEventListener("abort", onAbort, { once: true }); try { const tokens = await oauthClient.pollForToken( deviceResponse.deviceCode, deviceResponse.interval, deviceResponse.expiresIn, - abortController.signal, + signal, ); spinner?.succeed("Authentication successful."); return tokens; } catch (error) { - spinner?.fail("Authentication failed."); + if (!signal.aborted) { + spinner?.fail("Authentication failed."); + } + throw error; } finally { - process.removeListener("SIGINT", onSigint); + signal.removeEventListener("abort", onAbort); } } @@ -821,6 +852,8 @@ export default class AccountsLogin extends ControlBaseCommand { output: process.stdout, }); + forwardSigintToProcess(rl); + const logFn = this.log.bind(this); return new Promise((resolve) => { @@ -857,6 +890,8 @@ export default class AccountsLogin extends ControlBaseCommand { output: process.stdout, }); + forwardSigintToProcess(rl); + return new Promise((resolve) => { const askForAppName = () => { rl.question("Enter a name for your app: ", (appName) => { diff --git a/src/commands/status.ts b/src/commands/status.ts index 27479c637..e6e47522b 100644 --- a/src/commands/status.ts +++ b/src/commands/status.ts @@ -37,7 +37,10 @@ export default class StatusCommand extends AblyBaseCommand { const spinner = isInteractive || isJson ? null - : ora("Checking Ably service status...").start(); + : ora({ + discardStdin: false, + text: "Checking Ably service status...", + }).start(); if (isInteractive) { this.logProgress("Checking Ably service status", flags); } diff --git a/src/commands/support/ask.ts b/src/commands/support/ask.ts index 4f07ab574..128d408e6 100644 --- a/src/commands/support/ask.ts +++ b/src/commands/support/ask.ts @@ -38,7 +38,7 @@ export default class AskCommand extends ControlBaseCommand { const spinner = isInteractive || this.shouldOutputJson(flags) ? null - : ora("Thinking...").start(); + : ora({ discardStdin: false, text: "Thinking..." }).start(); if (isInteractive && !this.shouldOutputJson(flags)) { this.logProgress("Thinking", flags); } diff --git a/src/utils/prompt-confirmation.ts b/src/utils/prompt-confirmation.ts index b58e1dd77..3f34646bd 100644 --- a/src/utils/prompt-confirmation.ts +++ b/src/utils/prompt-confirmation.ts @@ -1,12 +1,6 @@ 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"; - } -} +import { InterruptError } from "./sigint-cancellation.js"; /** * Prompts the user for confirmation with a yes/no question. diff --git a/src/utils/sigint-cancellation.ts b/src/utils/sigint-cancellation.ts new file mode 100644 index 000000000..ec5db805b --- /dev/null +++ b/src/utils/sigint-cancellation.ts @@ -0,0 +1,84 @@ +// Ctrl+C handling for commands that mix spinners and interactive prompts. +// +// Two behaviours combine to make Ctrl+C unreliable without this: +// +// - Prompts such as legacy inquirer handle Ctrl+C by closing their readline +// and re-raising SIGINT with `process.kill`. Once any JS SIGINT listener +// exists (ora installs one via signal-exit), that signal is only delivered +// on a later event-loop turn. Signal handles don't keep the loop alive, so +// with the prompt closed Node can run out of work first and exit with +// "Detected unsettled top-level await" (exit code 13). +// - A listener cannot exit an oclif command by throwing: `this.exit()` throws +// an ExitError, and from inside an event listener that surfaces as an +// uncaught exception rather than a clean exit. +// +// `withSigintCancellation` owns SIGINT for the duration of a task, keeps the +// event loop alive so a re-raised signal is always delivered, and turns Ctrl+C +// into a rejection the command can handle on its normal async path. + +import type * as readline from "node:readline"; + +/** + * Raised when the user presses Ctrl+C, either while `withSigintCancellation` + * owns SIGINT or at a `promptForConfirmation` prompt. + */ +export class InterruptError extends Error { + constructor() { + super("Interrupted by user"); + this.name = "InterruptError"; + } +} + +/** + * Run `task`, rejecting with an `InterruptError` when the user presses Ctrl+C. + * + * The task receives an `AbortSignal` that is aborted on Ctrl+C, so in-flight + * work (fetches, polling loops, spinners) can stop and clean up. Abort + * listeners run synchronously, before the rejection reaches the caller. + * + * The task's own promise is raced rather than awaited, because some prompts + * (legacy inquirer) never settle once force-closed. + */ +export async function withSigintCancellation( + task: (signal: AbortSignal) => Promise, +): Promise { + const controller = new AbortController(); + const interrupted = new Promise((_, reject) => { + controller.signal.addEventListener( + "abort", + () => reject(new InterruptError()), + { once: true }, + ); + }); + const onSigint = () => controller.abort(); + + process.on("SIGINT", onSigint); + // Keep the event loop alive until the task settles, so a SIGINT re-raised + // by a prompt that has already closed its input is still delivered. Prompts + // only see Ctrl+C as a keypress on a TTY; elsewhere this would turn a prompt + // stuck on closed stdin into a hang. + const keepAlive = process.stdin.isTTY + ? setInterval(() => {}, 1 << 30) + : undefined; + try { + return await Promise.race([task(controller.signal), interrupted]); + } finally { + clearInterval(keepAlive); + process.removeListener("SIGINT", onSigint); + } +} + +/** + * Re-raise Ctrl+C at a readline prompt as a process SIGINT. + * + * Without a SIGINT listener, readline swallows Ctrl+C and only pauses its + * input, leaving the prompt's promise pending forever. Closing the prompt and + * re-raising lets whoever owns SIGINT (`withSigintCancellation`, or Node's + * default handler) end the command. + */ +export function forwardSigintToProcess(rl: readline.Interface): void { + rl.on("SIGINT", () => { + rl.close(); + process.kill(process.pid, "SIGINT"); + }); +} diff --git a/test/tty/commands/accounts-login-sigint.test.ts b/test/tty/commands/accounts-login-sigint.test.ts new file mode 100644 index 000000000..eb6980f09 --- /dev/null +++ b/test/tty/commands/accounts-login-sigint.test.ts @@ -0,0 +1,186 @@ +/** + * TTY-dependent Ctrl+C tests for `ably accounts login`. + * + * Both failures only reproduce in a real terminal, so they can't be covered by + * unit tests: + * - ora's stdin-discarder puts the TTY in raw mode and then never reads the + * Ctrl+C byte, so Ctrl+C during "Waiting for authorization..." was ignored. + * - After that spinner has run, inquirer's re-raised SIGINT at a select + * prompt was never delivered: the prompt had closed its input, so Node + * ran out of work first and reported "Detected unsettled top-level await". + * + * A local HTTP server stands in for both the OAuth device flow and the + * Control API so the command runs end to end. + * + * Run locally with: pnpm test:tty + * NOT run in CI — GitHub Actions runners have no TTY. + */ +import { describe, it, expect, afterEach, beforeEach } from "vitest"; +import http from "node:http"; +import type { AddressInfo } from "node:net"; +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; + +import { + spawnTty, + waitForOutput, + sendCtrlC, + killTty, + type TtyProcess, +} from "../tty-test-helper.js"; + +interface FakeServer { + host: string; + close: () => Promise; +} + +/** + * Serve the OAuth device flow and the Control API endpoints login calls. + * `grantToken` decides whether polling succeeds straight away or keeps + * returning `authorization_pending`. + */ +async function startFakeServer(grantToken: boolean): Promise { + const routes: Record [number, unknown]> = { + "/oauth/authorize_device": () => [ + 200, + { + device_code: "device-code", + expires_in: 600, + interval: 1, + user_code: "ABCD-EFGH", + verification_uri: "http://localhost/oauth/device", + verification_uri_complete: + "http://localhost/oauth/device?user_code=ABCD-EFGH", + }, + ], + "/oauth/token": () => + grantToken + ? [200, { access_token: "access", refresh_token: "refresh" }] + : [400, { error: "authorization_pending" }], + "/api/v1/me": () => [ + 200, + { + account: { id: "acc-1", name: "Test Account" }, + user: { email: "user@example.com" }, + }, + ], + "/api/v1/me/accounts": () => [200, [{ id: "acc-1", name: "Test Account" }]], + "/api/v1/accounts/acc-1/apps": () => [ + 200, + [{ accountId: "acc-1", id: "app-1", name: "Test App" }], + ], + // Two keys, so login has to prompt for one. + "/api/v1/apps/app-1/keys": () => [ + 200, + ["key-1", "key-2"].map((id) => ({ + appId: "app-1", + capability: {}, + created: 0, + id, + key: `app-1.${id}:secret`, + modified: 0, + name: `Key ${id}`, + revocable: true, + status: 0, + })), + ], + }; + + const server = http.createServer((req, res) => { + const route = routes[new URL(req.url ?? "/", "http://x").pathname]; + const [status, body] = route ? route() : [404, { error: "not_found" }]; + res.writeHead(status, { "content-type": "application/json" }); + res.end(JSON.stringify(body)); + }); + + await new Promise((resolve) => server.listen(0, "localhost", resolve)); + const { port } = server.address() as AddressInfo; + + return { + host: `localhost:${port}`, + close: () => + new Promise((resolve) => { + server.closeAllConnections(); + server.close(() => resolve()); + }), + }; +} + +describe("accounts login - Ctrl+C handling (TTY)", () => { + const timeout = 20000; + let proc: TtyProcess | null = null; + let server: FakeServer | null = null; + let configDir: string; + + beforeEach(() => { + configDir = fs.mkdtempSync(path.join(os.tmpdir(), "ably-login-tty-")); + }); + + afterEach(async () => { + if (proc) { + await killTty(proc); + proc = null; + } + await server?.close(); + server = null; + fs.rmSync(configDir, { force: true, recursive: true }); + }); + + function spawnLogin(host: string): TtyProcess { + return spawnTty( + [ + "accounts", + "login", + "--no-browser", + "--skip-logo", + "--oauth-host", + host, + "--control-host", + host, + ], + { + ABLY_ACCESS_TOKEN: undefined, + ABLY_API_KEY: undefined, + ABLY_CLI_CONFIG_DIR: configDir, + // Exercise the real exit path rather than test mode's throwing one. + ABLY_CLI_TEST_MODE: undefined, + ABLY_INTERACTIVE_MODE: "false", + ABLY_TOKEN: undefined, + }, + ); + } + + it( + "exits with 130 when Ctrl+C is pressed while waiting for authorization", + async () => { + server = await startFakeServer(false); + proc = spawnLogin(server.host); + + await waitForOutput(proc, "Waiting for authorization"); + sendCtrlC(proc); + + const { code } = await proc.exitPromise; + expect(code).toBe(130); + expect(proc.output).toContain("Authentication cancelled."); + expect(proc.output).not.toContain("ExitError"); + }, + timeout, + ); + + it( + "exits with 130 when Ctrl+C is pressed at a prompt after authorization", + async () => { + server = await startFakeServer(true); + proc = spawnLogin(server.host); + + await waitForOutput(proc, "Select a key"); + sendCtrlC(proc); + + const { code } = await proc.exitPromise; + expect(code).toBe(130); + expect(proc.output).not.toContain("unsettled top-level await"); + }, + timeout, + ); +}); diff --git a/test/tty/tty-test-helper.ts b/test/tty/tty-test-helper.ts index af9ef3960..13d1540da 100644 --- a/test/tty/tty-test-helper.ts +++ b/test/tty/tty-test-helper.ts @@ -43,10 +43,13 @@ export function spawnTty( args: string[] = ["interactive"], env: Record = {}, ): TtyProcess { - const childEnv = { - ...DEFAULT_ENV, - ...env, - }; + // node-pty stringifies values, so drop undefined entries to let callers + // unset inherited variables rather than set them to "undefined". + const childEnv = Object.fromEntries( + Object.entries({ ...DEFAULT_ENV, ...env }).filter( + (entry): entry is [string, string] => entry[1] !== undefined, + ), + ); const pty = nodePty.spawn("node", [BIN_PATH, ...args], { name: "xterm-256color", diff --git a/test/unit/commands/accounts/login.test.ts b/test/unit/commands/accounts/login.test.ts index cf19e4f47..75428b86e 100644 --- a/test/unit/commands/accounts/login.test.ts +++ b/test/unit/commands/accounts/login.test.ts @@ -522,6 +522,38 @@ describe("accounts:login command", () => { expect(authEvent).toHaveProperty("userCode", "TEST-CODE"); expect(authEvent).toHaveProperty("verificationUri"); }); + + it("exits with 130 and no error when interrupted while polling", async () => { + nock("https://ably.com").post("/oauth/authorize_device").reply(200, { + device_code: "dc_test", + expires_in: 300, + interval: 0.01, + user_code: "TEST-CODE", + verification_uri: "https://ably.com/device", + verification_uri_complete: + "https://ably.com/device?user_code=TEST-CODE", + }); + // Interrupt from inside the first poll, so the SIGINT can't arrive + // before the command is listening for it. + nock("https://ably.com") + .post("/oauth/token") + .reply(() => { + setImmediate(() => process.emit("SIGINT")); + return [400, { error: "authorization_pending" }]; + }) + .persist(); + + const { error, stdout } = await runCommand( + ["accounts:login", "--json"], + import.meta.url, + ); + + expect(error?.oclif?.exit).toBe(130); + const errorEvents = parseNdjsonLines(stdout).filter( + (e) => e.type === "error", + ); + expect(errorEvents).toEqual([]); + }); }); describe("local server login", () => { diff --git a/test/unit/utils/prompt-confirmation.test.ts b/test/unit/utils/prompt-confirmation.test.ts index f6c166426..083c57569 100644 --- a/test/unit/utils/prompt-confirmation.test.ts +++ b/test/unit/utils/prompt-confirmation.test.ts @@ -20,10 +20,8 @@ vi.mock("node:readline", () => ({ }, })); -import { - InterruptError, - promptForConfirmation, -} from "../../../src/utils/prompt-confirmation.js"; +import { promptForConfirmation } from "../../../src/utils/prompt-confirmation.js"; +import { InterruptError } from "../../../src/utils/sigint-cancellation.js"; describe("promptForConfirmation", () => { beforeEach(() => { diff --git a/test/unit/utils/sigint-cancellation.test.ts b/test/unit/utils/sigint-cancellation.test.ts new file mode 100644 index 000000000..83e3c8076 --- /dev/null +++ b/test/unit/utils/sigint-cancellation.test.ts @@ -0,0 +1,70 @@ +import { describe, it, expect, vi, afterEach } from "vitest"; +import type * as readline from "node:readline"; + +import { + forwardSigintToProcess, + InterruptError, + withSigintCancellation, +} from "../../../src/utils/sigint-cancellation.js"; + +describe("withSigintCancellation", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("resolves with the task result and removes its SIGINT listener", async () => { + const before = process.listenerCount("SIGINT"); + + const result = await withSigintCancellation(async () => "done"); + + expect(result).toBe("done"); + expect(process.listenerCount("SIGINT")).toBe(before); + }); + + it("propagates task errors", async () => { + await expect( + withSigintCancellation(async () => { + throw new Error("boom"); + }), + ).rejects.toThrow("boom"); + }); + + it("aborts the signal and rejects with InterruptError on SIGINT", async () => { + const before = process.listenerCount("SIGINT"); + let taskSignal: AbortSignal | undefined; + + const pending = withSigintCancellation((signal) => { + taskSignal = signal; + // A task that never settles, like a force-closed inquirer prompt. + return new Promise(() => {}); + }); + + process.emit("SIGINT"); + + await expect(pending).rejects.toBeInstanceOf(InterruptError); + expect(taskSignal?.aborted).toBe(true); + expect(process.listenerCount("SIGINT")).toBe(before); + }); +}); + +describe("forwardSigintToProcess", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("closes the prompt and re-raises SIGINT on the process", () => { + const kill = vi.spyOn(process, "kill").mockImplementation(() => true); + const close = vi.fn(); + const handlers = new Map void>(); + const rl = { + close, + on: (event: string, handler: () => void) => handlers.set(event, handler), + } as unknown as readline.Interface; + + forwardSigintToProcess(rl); + handlers.get("SIGINT")?.(); + + expect(close).toHaveBeenCalledOnce(); + expect(kill).toHaveBeenCalledWith(process.pid, "SIGINT"); + }); +}); From 1855bffb69b88b0127e3e85493933b7637d39791 Mon Sep 17 00:00:00 2001 From: Andy Ford Date: Wed, 21 Oct 2026 14:04:14 +0100 Subject: [PATCH 2/2] squash! fix(accounts:login): make Ctrl+C cancel login reliably Exit 130 on Ctrl+C at login's confirmation prompts promptForConfirmation now rejects with InterruptError on Ctrl+C. Login caught that in its app-fetch and outer catch blocks, logging "Could not fetch apps" or failing with exit 1. Let the interruption through to run() so it exits 130, and cover the create-app confirmation in the TTY test. Co-Authored-By: Claude Opus 5.5 --- src/commands/accounts/login.ts | 3 +++ .../commands/accounts-login-sigint.test.ts | 26 ++++++++++++++++--- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/src/commands/accounts/login.ts b/src/commands/accounts/login.ts index 928e6e2b6..d6e7dd5d1 100644 --- a/src/commands/accounts/login.ts +++ b/src/commands/accounts/login.ts @@ -312,6 +312,7 @@ export default class AccountsLogin extends ControlBaseCommand { } } } catch (error) { + if (error instanceof InterruptError) throw error; this.logWarning(`Could not fetch apps: ${errorMessage(error)}`, flags); } @@ -439,6 +440,8 @@ export default class AccountsLogin extends ControlBaseCommand { } } } catch (error) { + // Ctrl+C at a confirmation prompt; `run` handles the exit. + if (error instanceof InterruptError) throw error; this.fail(error, flags, "accountLogin"); } } diff --git a/test/tty/commands/accounts-login-sigint.test.ts b/test/tty/commands/accounts-login-sigint.test.ts index eb6980f09..08374f10d 100644 --- a/test/tty/commands/accounts-login-sigint.test.ts +++ b/test/tty/commands/accounts-login-sigint.test.ts @@ -38,9 +38,13 @@ interface FakeServer { /** * Serve the OAuth device flow and the Control API endpoints login calls. * `grantToken` decides whether polling succeeds straight away or keeps - * returning `authorization_pending`. + * returning `authorization_pending`; with `noApps`, login offers to create + * one instead of prompting for a key. */ -async function startFakeServer(grantToken: boolean): Promise { +async function startFakeServer( + grantToken: boolean, + { noApps = false } = {}, +): Promise { const routes: Record [number, unknown]> = { "/oauth/authorize_device": () => [ 200, @@ -68,7 +72,7 @@ async function startFakeServer(grantToken: boolean): Promise { "/api/v1/me/accounts": () => [200, [{ id: "acc-1", name: "Test Account" }]], "/api/v1/accounts/acc-1/apps": () => [ 200, - [{ accountId: "acc-1", id: "app-1", name: "Test App" }], + noApps ? [] : [{ accountId: "acc-1", id: "app-1", name: "Test App" }], ], // Two keys, so login has to prompt for one. "/api/v1/apps/app-1/keys": () => [ @@ -183,4 +187,20 @@ describe("accounts login - Ctrl+C handling (TTY)", () => { }, timeout, ); + + it( + "exits with 130 when Ctrl+C is pressed at a confirmation after authorization", + async () => { + server = await startFakeServer(true, { noApps: true }); + proc = spawnLogin(server.host); + + await waitForOutput(proc, "create your first app"); + sendCtrlC(proc); + + const { code } = await proc.exitPromise; + expect(code).toBe(130); + expect(proc.output).not.toContain("Could not fetch apps"); + }, + timeout, + ); });