diff --git a/cli/src/__tests__/auth-command-registration.test.ts b/cli/src/__tests__/auth-command-registration.test.ts index 76b3c51d..98553760 100644 --- a/cli/src/__tests__/auth-command-registration.test.ts +++ b/cli/src/__tests__/auth-command-registration.test.ts @@ -13,6 +13,15 @@ describe("registerClientAuthCommands", () => { expect(login).toBeDefined(); expect(login?.options.filter((option) => option.long === "--company-id")).toHaveLength(1); }); + + it("registers the --no-browser flag on login", () => { + const program = new Command(); + const auth = program.command("auth"); + registerClientAuthCommands(auth); + + const login = auth.commands.find((command) => command.name() === "login"); + expect(login?.options.some((option) => option.long === "--no-browser")).toBe(true); + }); }); describe("client auth API commands", () => { diff --git a/cli/src/__tests__/open-url.test.ts b/cli/src/__tests__/open-url.test.ts new file mode 100644 index 00000000..d5c31d7b --- /dev/null +++ b/cli/src/__tests__/open-url.test.ts @@ -0,0 +1,57 @@ +import { EventEmitter } from "node:events"; +import { afterEach, describe, expect, it, vi } from "vitest"; + +// Lets each test decide what the next spawned child does, while the +// node:child_process mock below is hoisted above the board-auth import. +const mocks = vi.hoisted(() => ({ spawn: vi.fn() })); + +vi.mock("node:child_process", async () => { + const actual = await vi.importActual("node:child_process"); + return { ...actual, spawn: mocks.spawn }; +}); + +import { openUrl } from "../client/board-auth.js"; + +function fakeChild(): EventEmitter & { unref: () => void } { + const child = new EventEmitter() as EventEmitter & { unref: () => void }; + child.unref = () => {}; + return child; +} + +describe("openUrl", () => { + afterEach(() => { + vi.clearAllMocks(); + }); + + it("resolves true when the browser opener launches", async () => { + mocks.spawn.mockImplementation(() => { + const child = fakeChild(); + queueMicrotask(() => child.emit("spawn")); + return child; + }); + + await expect(openUrl("https://example.com")).resolves.toBe(true); + }); + + it("resolves false instead of crashing when the opener is missing", async () => { + // Headless container case: e.g. no `xdg-open`. spawn reports this via an + // async 'error' event, not a thrown exception — openUrl must swallow it and + // resolve false rather than letting an unhandled 'error' abort the process. + mocks.spawn.mockImplementation(() => { + const child = fakeChild(); + const error = Object.assign(new Error("spawn xdg-open ENOENT"), { code: "ENOENT" }); + queueMicrotask(() => child.emit("error", error)); + return child; + }); + + await expect(openUrl("https://example.com")).resolves.toBe(false); + }); + + it("resolves false when spawn throws synchronously", async () => { + mocks.spawn.mockImplementation(() => { + throw new Error("synchronous spawn failure"); + }); + + await expect(openUrl("https://example.com")).resolves.toBe(false); + }); +}); diff --git a/cli/src/client/board-auth.ts b/cli/src/client/board-auth.ts index 7c1121ec..0f6fe8e8 100644 --- a/cli/src/client/board-auth.ts +++ b/cli/src/client/board-auth.ts @@ -1,4 +1,4 @@ -import { spawn } from "node:child_process"; +import { spawn, type ChildProcess } from "node:child_process"; import fs from "node:fs"; import path from "node:path"; import pc from "picocolors"; @@ -60,6 +60,11 @@ function normalizeApiBase(apiBase: string): string { return apiBase.trim().replace(/\/+$/, ""); } +function isTruthyEnv(value: string | undefined): boolean { + const v = value?.trim().toLowerCase(); + return v === "1" || v === "true" || v === "yes"; +} + export function resolveBoardAuthStorePath(overridePath?: string): string { if (overridePath?.trim()) return path.resolve(overridePath.trim()); if (process.env.PAPERCLIP_AUTH_STORE?.trim()) return path.resolve(process.env.PAPERCLIP_AUTH_STORE.trim()); @@ -169,25 +174,28 @@ async function requestJson(url: string, init?: RequestInit): Promise { return response.json() as Promise; } -export function openUrl(url: string): boolean { - const platform = process.platform; - try { - if (platform === "darwin") { - const child = spawn("open", [url], { detached: true, stdio: "ignore" }); - child.unref(); - return true; +export async function openUrl(url: string): Promise { + const { command, args } = + process.platform === "darwin" + ? { command: "open", args: [url] } + : process.platform === "win32" + ? { command: "cmd", args: ["/c", "start", "", url] } + : { command: "xdg-open", args: [url] }; + + return new Promise((resolve) => { + let child: ChildProcess; + try { + child = spawn(command, args, { detached: true, stdio: "ignore" }); + } catch { + resolve(false); + return; } - if (platform === "win32") { - const child = spawn("cmd", ["/c", "start", "", url], { detached: true, stdio: "ignore" }); + child.once("error", () => resolve(false)); + child.once("spawn", () => { child.unref(); - return true; - } - const child = spawn("xdg-open", [url], { detached: true, stdio: "ignore" }); - child.unref(); - return true; - } catch { - return false; - } + resolve(true); + }); + }); } export async function loginBoardCli(params: { @@ -198,6 +206,8 @@ export async function loginBoardCli(params: { command?: string; storePath?: string; print?: boolean; + openBrowser?: boolean; + publicBaseUrl?: string; }): Promise<{ token: string; approvalUrl: string; userId?: string | null }> { const apiBase = normalizeApiBase(params.apiBase); const createUrl = `${apiBase}/api/cli-auth/challenges`; @@ -213,15 +223,25 @@ export async function loginBoardCli(params: { }), }); - const approvalUrl = challenge.approvalUrl ?? `${apiBase}${challenge.approvalPath}`; + const publicBase = params.publicBaseUrl?.trim() || process.env.PAPERCLIP_PUBLIC_URL?.trim(); + const approvalUrl = publicBase + ? `${normalizeApiBase(publicBase)}${challenge.approvalPath}` + : challenge.approvalUrl ?? `${apiBase}${challenge.approvalPath}`; + if (params.print !== false) { console.error(pc.bold("Board authentication required")); console.error(`Open this URL in your browser to approve CLI access:\n${approvalUrl}`); } - const opened = openUrl(approvalUrl); - if (params.print !== false && opened) { - console.error(pc.dim("Opened the approval page in your browser.")); + const wantBrowser = params.openBrowser !== false && !isTruthyEnv(process.env.PAPERCLIP_NO_BROWSER); + const opened = wantBrowser ? await openUrl(approvalUrl) : false; + if (params.print !== false) { + const browserMessage = !wantBrowser + ? "Browser open skipped — open the URL above to approve." + : opened + ? "Opened the approval page in your browser." + : "Couldn't open a browser automatically — open the URL above to approve."; + console.error(pc.dim(browserMessage)); } const expiresAtMs = Date.parse(challenge.expiresAt); diff --git a/cli/src/commands/client/auth.ts b/cli/src/commands/client/auth.ts index 1d1b1551..549f119c 100644 --- a/cli/src/commands/client/auth.ts +++ b/cli/src/commands/client/auth.ts @@ -16,6 +16,7 @@ import { interface AuthLoginOptions extends BaseClientOptions { instanceAdmin?: boolean; + browser?: boolean; } interface AuthLogoutOptions extends BaseClientOptions {} @@ -32,6 +33,7 @@ export function registerClientAuthCommands(auth: Command): void { .command("login") .description("Authenticate the CLI for board-user access") .option("--instance-admin", "Request instance-admin approval instead of plain board access", false) + .option("--no-browser", "Don't try to open a browser; just print the approval URL") .action(async (opts: AuthLoginOptions) => { try { const ctx = resolveCommandContext(opts); @@ -40,6 +42,7 @@ export function registerClientAuthCommands(auth: Command): void { requestedAccess: opts.instanceAdmin ? "instance_admin_required" : "board", requestedCompanyId: ctx.companyId ?? null, command: "paperclipai auth login", + openBrowser: opts.browser, }); printOutput( { diff --git a/cli/src/commands/client/cloud.ts b/cli/src/commands/client/cloud.ts index c48a8779..1e3046f1 100644 --- a/cli/src/commands/client/cloud.ts +++ b/cli/src/commands/client/cloud.ts @@ -366,7 +366,7 @@ async function authorizeWithBrowser( try { console.error(`Open this URL to approve cloud sync:\n${authorizeUrl.toString()}`); - if (!openUrl(authorizeUrl.toString())) { + if (!(await openUrl(authorizeUrl.toString()))) { throw new Error("Could not open a browser."); } const code = await callback.waitForCode(state); @@ -410,7 +410,7 @@ async function authorizeWithDeviceCode( console.error(pc.bold("Cloud device authorization required")); console.error(`Open: ${response.verificationUri}`); console.error(`Code: ${response.userCode}`); - if (opts.openBrowser) openUrl(response.verificationUri); + if (opts.openBrowser) await openUrl(response.verificationUri); const expiresAt = resolveDeviceCodeExpiresAt(response.expiresAt); const intervalMs = Math.max(500, (response.intervalSeconds ?? 5) * 1000); diff --git a/cli/src/commands/client/company.ts b/cli/src/commands/client/company.ts index 682b82ac..ee702d1b 100644 --- a/cli/src/commands/client/company.ts +++ b/cli/src/commands/client/company.ts @@ -1594,7 +1594,7 @@ export function registerCompanyCommands(program: Command): void { initialValue: true, }); if (!p.isCancel(openImportedCompany) && openImportedCompany) { - if (openUrl(companyUrl)) { + if (await openUrl(companyUrl)) { p.log.info(`Opened ${companyUrl}`); } else { p.log.warn(`Could not open your browser automatically. Open this URL manually:\n${companyUrl}`);