Make Ctrl+C cancel ably accounts login reliably

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`)
 
 ---
 

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

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<typeof AccountsLogin.flags>;
+
 /** 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<void> {
     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<void> {
     // 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");
     }
 
@@ -286,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);
       }
 
@@ -413,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");
     }
   }
@@ -719,7 +748,10 @@ export default class AccountsLogin extends ControlBaseCommand {
     }
   }
 
-  private async oauthLogin(flags: BaseFlags): Promise<OAuthTokens> {
+  private async oauthLogin(
+    flags: BaseFlags,
+    signal: AbortSignal,
+  ): Promise<OAuthTokens> {
     const oauthClient = new OAuthClient({
       oauthHost: flags["oauth-host"],
     });
@@ -754,38 +786,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 +855,8 @@ export default class AccountsLogin extends ControlBaseCommand {
       output: process.stdout,
     });
 
+    forwardSigintToProcess(rl);
+
     const logFn = this.log.bind(this);
 
     return new Promise((resolve) => {
@@ -857,6 +893,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) => {

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);
     }

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);
     }

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.

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<T>(
+  task: (signal: AbortSignal) => Promise<T>,
+): Promise<T> {
+  const controller = new AbortController();
+  const interrupted = new Promise<never>((_, 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");
+  });
+}