From bb8900926970c19d159266be6b8196f98364c012 Mon Sep 17 00:00:00 2001 From: Sarav Date: Fri, 30 Oct 2026 07:42:22 +0530 Subject: [PATCH 1/2] fix(auth): Ctrl+C during browser sign-in exits 130 instead of 0 `auth login` waited for the browser callback under a clack spinner. In a terminal the spinner's raw-mode stdin turns Ctrl+C into `process.exit(0)`, so `auth login ... && altimate-code link` continued after a canceled sign-in. Without a terminal its SIGINT listener swallowed the signal and the process kept waiting until the sign-in timed out. - Replace the spinner on the OAuth "auto" wait with a log line and race the callback against SIGINT; on cancel, fail with "Sign-in canceled." and exit 130 - `index.ts`: set the default exit code 1 before `FormatError`, so a `CliError`'s own `exitCode` is no longer overwritten (this also makes the existing `fail("Cancelled.", 130)` in `learn` exit 130) - Subprocess tests for Ctrl+C in a PTY and SIGINT without a terminal; both fail on `main` Co-Authored-By: Claude Opus 5.5 --- packages/opencode/src/cli/cmd/providers.ts | 25 +++- packages/opencode/src/index.ts | 4 +- .../test/cli/auth-login-cancel.test.ts | 127 ++++++++++++++++++ 3 files changed, 150 insertions(+), 6 deletions(-) create mode 100644 packages/opencode/test/cli/auth-login-cancel.test.ts diff --git a/packages/opencode/src/cli/cmd/providers.ts b/packages/opencode/src/cli/cmd/providers.ts index c631091518..518a6dab8b 100644 --- a/packages/opencode/src/cli/cmd/providers.ts +++ b/packages/opencode/src/cli/cmd/providers.ts @@ -41,6 +41,18 @@ const cliTry = (message: string, fn: () => PromiseLike) => catch: (error) => new CliError({ message: message + errorMessage(error) }), }) +// altimate_change start — resolve a pending wait on Ctrl+C (SIGINT) instead of letting it run to timeout +const SIGNIN_CANCELED = Symbol("signin-canceled") +const untilSigint = (promise: Promise) => { + let onSigint = () => {} + const canceled = new Promise((resolve) => { + onSigint = () => resolve(SIGNIN_CANCELED) + process.once("SIGINT", onSigint) + }) + return Promise.race([promise, canceled]).finally(() => process.off("SIGINT", onSigint)) +} +// altimate_change end + const handlePluginAuth = Effect.fn("Cli.providers.pluginAuth")(function* ( plugin: { auth: PluginAuth }, provider: string, @@ -108,11 +120,14 @@ const handlePluginAuth = Effect.fn("Cli.providers.pluginAuth")(function* ( if (authorize.instructions) { yield* Prompt.log.info(authorize.instructions) } - const spinner = Prompt.spinner() - yield* spinner.start("Waiting for authorization...") - const result = yield* cliTry("Failed to authorize: ", () => authorize.callback()) + // altimate_change start — no clack spinner here: its raw-mode stdin turns Ctrl+C into + // process.exit(0), so a canceled sign-in looked successful to `&&` chains and scripts. + yield* Prompt.log.info("Waiting for authorization... (Ctrl+C to cancel)") + const result = yield* cliTry("Failed to authorize: ", () => untilSigint(authorize.callback())) + if (result === SIGNIN_CANCELED) return yield* fail("Sign-in canceled.", 130) + // altimate_change end if (result.type === "failed") { - yield* spinner.stop("Failed to authorize", 1) + yield* Prompt.log.error("Failed to authorize") } if (result.type === "success") { const saveProvider = result.provider ?? provider @@ -133,7 +148,7 @@ const handlePluginAuth = Effect.fn("Cli.providers.pluginAuth")(function* ( ...(result.metadata ? { metadata: result.metadata } : {}), }) } - yield* spinner.stop("Login successful") + yield* Prompt.log.success("Login successful") } } diff --git a/packages/opencode/src/index.ts b/packages/opencode/src/index.ts index 35a81853b6..2e78345968 100644 --- a/packages/opencode/src/index.ts +++ b/packages/opencode/src/index.ts @@ -286,13 +286,15 @@ try { await cli.parse() } } catch (e) { + // altimate_change start — set the default before FormatError so a CliError's own exitCode (e.g. 130) wins + process.exitCode = 1 const formatted = FormatError(e) + // altimate_change end if (formatted) UI.error(formatted) if (formatted === undefined) { UI.error("Unexpected error" + EOL) process.stderr.write(errorMessage(e) + EOL) } - process.exitCode = 1 } finally { // altimate_change start - telemetry flush // Flush any buffered telemetry events before exiting. diff --git a/packages/opencode/test/cli/auth-login-cancel.test.ts b/packages/opencode/test/cli/auth-login-cancel.test.ts new file mode 100644 index 0000000000..37359ab918 --- /dev/null +++ b/packages/opencode/test/cli/auth-login-cancel.test.ts @@ -0,0 +1,127 @@ +// Ctrl+C while `auth login` waits for a browser sign-in must exit 130, not 0. +// +// The wait used a clack spinner. In a terminal its raw-mode stdin turned Ctrl+C into +// `process.exit(0)`, so `auth login ... && altimate-code link` carried on after a canceled +// sign-in. Without a terminal its SIGINT listener swallowed the signal and the process kept +// waiting until the sign-in timed out. +import { afterAll, beforeAll, describe, expect, test } from "bun:test" +import fs from "node:fs/promises" +import os from "node:os" +import path from "node:path" +import { spawn as ptySpawn } from "@opencode-ai/core/pty/pty.bun" + +const cliEntry = path.resolve(import.meta.dir, "../../src/index.ts") +const argv = ["run", "--conditions=browser", cliEntry, "auth", "login", "-p", "fake-oauth"] +const WAITING = "Waiting for authorization" + +// An OAuth "auto" method whose browser callback never completes. +const fakeAuthPlugin = `export const FakeAuth = async () => ({ + auth: { + provider: "fake-oauth", + methods: [ + { + type: "oauth", + label: "Fake browser sign-in", + authorize: async () => ({ + url: "https://example.test/authorize", + method: "auto", + callback: () => new Promise(() => {}), + }), + }, + ], + }, +}) +` + +let root: string +let env: Record + +beforeAll(async () => { + root = await fs.mkdtemp(path.join(os.tmpdir(), "auth-login-cancel-")) + const pluginPath = path.join(root, "fake-auth.ts") + await fs.writeFile(pluginPath, fakeAuthPlugin) + env = { + ...(process.env as Record), + OPENCODE_TEST_HOME: root, + HOME: root, + XDG_CONFIG_HOME: path.join(root, ".config"), + XDG_DATA_HOME: path.join(root, ".local/share"), + XDG_STATE_HOME: path.join(root, ".local/state"), + XDG_CACHE_HOME: path.join(root, ".cache"), + OPENCODE_PURE: "1", + OPENCODE_DISABLE_AUTOUPDATE: "1", + OPENCODE_DISABLE_MODELS_FETCH: "1", + OPENCODE_DISABLE_PROJECT_CONFIG: "1", + OPENCODE_DISABLE_TELEMETRY: "1", + OPENCODE_AUTH_CONTENT: "{}", + OPENCODE_CONFIG_CONTENT: JSON.stringify({ plugin: [`file://${pluginPath}`] }), + } +}) + +afterAll(async () => { + await fs.rm(root, { recursive: true, force: true }) +}) + +const withTimeout = (promise: Promise, ms: number, label: string) => + Promise.race([ + promise, + new Promise((_, reject) => setTimeout(() => reject(new Error(`${label} after ${ms}ms`)), ms)), + ]) + +describe("auth login cancel", () => { + test("Ctrl+C in a terminal exits 130", async () => { + const child = ptySpawn(process.execPath, argv, { name: "xterm-256color", cols: 120, rows: 40, cwd: root, env }) + let output = "" + child.onData((data) => { + output += data.toString() + }) + const exited = new Promise<{ exitCode: number }>((resolve) => child.onExit(resolve)) + try { + await withTimeout( + (async () => { + while (!output.includes(WAITING)) await Bun.sleep(50) + })(), + 60_000, + `never printed "${WAITING}": ${output}`, + ) + child.write("\x03") + const { exitCode } = await withTimeout(exited, 10_000, "still running after Ctrl+C") + expect(exitCode).toBe(130) + expect(output).toContain("Sign-in canceled.") + } finally { + child.kill() + } + }, 90_000) + + test("SIGINT without a terminal exits 130 instead of waiting for the timeout", async () => { + const proc = Bun.spawn([process.execPath, ...argv], { + cwd: root, + env, + stdin: "ignore", + stdout: "pipe", + stderr: "pipe", + }) + try { + const reader = proc.stdout.getReader() + const decoder = new TextDecoder() + let output = "" + await withTimeout( + (async () => { + while (!output.includes(WAITING)) { + const { value, done } = await reader.read() + if (done) throw new Error(`exited before waiting: ${output}`) + output += decoder.decode(value) + } + })(), + 60_000, + `never printed "${WAITING}": ${output}`, + ) + reader.releaseLock() + proc.kill("SIGINT") + expect(await withTimeout(proc.exited, 10_000, "still running after SIGINT")).toBe(130) + expect(await new Response(proc.stderr).text()).toContain("Sign-in canceled.") + } finally { + proc.kill("SIGKILL") + } + }, 90_000) +}) From 633526f5da5097dbbf11fe8b95e94ae206919822 Mon Sep 17 00:00:00 2001 From: Sarav Date: Fri, 30 Oct 2026 07:58:35 +0530 Subject: [PATCH 2/2] test(auth): harden the sign-in cancel test and run it in the subprocess pass - Skip under `OPENCODE_SKIP_SUBPROCESS` and list it in CI's bounded `--max-concurrency=2` subprocess pass, like `cliIt` tests - Skip on Windows, which has no POSIX signals - Timeout messages now include the output captured at the time of failure, and timers and polling loops stop when the test ends Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 2 +- .../test/cli/auth-login-cancel.test.ts | 60 +++++++++++-------- 2 files changed, 35 insertions(+), 27 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 154e240ef6..a33850e799 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -337,7 +337,7 @@ jobs: # pass is isolated from the 11k main-suite tests so 2-per-worker is plenty bounded. # (Bun 1.3.x can segfault during cleanup after all tests pass — we parse the summary, not the # exit code. --timeout 90000 gives headroom for slow fs/spawn tests.) - SUBPROCESS_PATHS="test/cli/acp/ test/cli/smokes/ test/cli/serve/ test/cli/run/run-process.test.ts test/cli/mcp-add.test.ts test/cli/help/help-snapshots.test.ts" + SUBPROCESS_PATHS="test/cli/acp/ test/cli/smokes/ test/cli/serve/ test/cli/run/run-process.test.ts test/cli/mcp-add.test.ts test/cli/help/help-snapshots.test.ts test/cli/auth-login-cancel.test.ts" OPENCODE_SKIP_SUBPROCESS=1 bun test --timeout 90000 > /tmp/test-main.txt 2>&1 || true bun test --timeout 90000 --max-concurrency=2 $SUBPROCESS_PATHS > /tmp/test-sub.txt 2>&1 || true diff --git a/packages/opencode/test/cli/auth-login-cancel.test.ts b/packages/opencode/test/cli/auth-login-cancel.test.ts index 37359ab918..c554575006 100644 --- a/packages/opencode/test/cli/auth-login-cancel.test.ts +++ b/packages/opencode/test/cli/auth-login-cancel.test.ts @@ -62,13 +62,26 @@ afterAll(async () => { await fs.rm(root, { recursive: true, force: true }) }) -const withTimeout = (promise: Promise, ms: number, label: string) => - Promise.race([ - promise, - new Promise((_, reject) => setTimeout(() => reject(new Error(`${label} after ${ms}ms`)), ms)), - ]) +// Fails with `what()` evaluated at timeout, so the message carries the output captured so far. +async function waitFor(check: () => boolean, ms: number, what: () => string) { + const deadline = Date.now() + ms + while (!check()) { + if (Date.now() > deadline) throw new Error(`${what()} (after ${ms}ms)`) + await Bun.sleep(50) + } +} + +async function exitWithin(exited: Promise, ms: number, what: string) { + let timer: ReturnType | undefined + const timeout = new Promise((_, reject) => { + timer = setTimeout(() => reject(new Error(`${what} after ${ms}ms`)), ms) + }) + return Promise.race([exited, timeout]).finally(() => clearTimeout(timer)) +} -describe("auth login cancel", () => { +// Spawns the real CLI, so like cliIt tests it runs only in CI's bounded subprocess pass. +// Windows has no POSIX signals or Ctrl+C-as-SIGINT through a PTY. +describe.skipIf(!!process.env.OPENCODE_SKIP_SUBPROCESS || process.platform === "win32")("auth login cancel", () => { test("Ctrl+C in a terminal exits 130", async () => { const child = ptySpawn(process.execPath, argv, { name: "xterm-256color", cols: 120, rows: 40, cwd: root, env }) let output = "" @@ -77,15 +90,13 @@ describe("auth login cancel", () => { }) const exited = new Promise<{ exitCode: number }>((resolve) => child.onExit(resolve)) try { - await withTimeout( - (async () => { - while (!output.includes(WAITING)) await Bun.sleep(50) - })(), + await waitFor( + () => output.includes(WAITING), 60_000, - `never printed "${WAITING}": ${output}`, + () => `never printed "${WAITING}": ${output}`, ) child.write("\x03") - const { exitCode } = await withTimeout(exited, 10_000, "still running after Ctrl+C") + const { exitCode } = await exitWithin(exited, 10_000, "still running after Ctrl+C") expect(exitCode).toBe(130) expect(output).toContain("Sign-in canceled.") } finally { @@ -101,27 +112,24 @@ describe("auth login cancel", () => { stdout: "pipe", stderr: "pipe", }) - try { + let output = "" + const decoder = new TextDecoder() + const drained = (async () => { const reader = proc.stdout.getReader() - const decoder = new TextDecoder() - let output = "" - await withTimeout( - (async () => { - while (!output.includes(WAITING)) { - const { value, done } = await reader.read() - if (done) throw new Error(`exited before waiting: ${output}`) - output += decoder.decode(value) - } - })(), + for (let next = await reader.read(); !next.done; next = await reader.read()) output += decoder.decode(next.value) + })() + try { + await waitFor( + () => output.includes(WAITING), 60_000, - `never printed "${WAITING}": ${output}`, + () => `never printed "${WAITING}": ${output}`, ) - reader.releaseLock() proc.kill("SIGINT") - expect(await withTimeout(proc.exited, 10_000, "still running after SIGINT")).toBe(130) + expect(await exitWithin(proc.exited, 10_000, "still running after SIGINT")).toBe(130) expect(await new Response(proc.stderr).text()).toContain("Sign-in canceled.") } finally { proc.kill("SIGKILL") + await drained } }, 90_000) })