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/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..c554575006 --- /dev/null +++ b/packages/opencode/test/cli/auth-login-cancel.test.ts @@ -0,0 +1,135 @@ +// 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 }) +}) + +// 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)) +} + +// 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 = "" + child.onData((data) => { + output += data.toString() + }) + const exited = new Promise<{ exitCode: number }>((resolve) => child.onExit(resolve)) + try { + await waitFor( + () => output.includes(WAITING), + 60_000, + () => `never printed "${WAITING}": ${output}`, + ) + child.write("\x03") + const { exitCode } = await exitWithin(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", + }) + let output = "" + const decoder = new TextDecoder() + const drained = (async () => { + const reader = proc.stdout.getReader() + 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}`, + ) + proc.kill("SIGINT") + 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) +})