fix(cli): handle headless browser-open failure in board auth (#8328)
## Thinking Path > - Paperclip is the open-source app for managing AI agents, very commonly self-hosted as a headless Docker container. > - Its CLI authorizes board/admin actions through an interactive browser-approval flow (`auth login`, and the auto-recover path behind commands like `run list`). > - That flow calls `openUrl()`, which `spawn`s the OS browser opener (`xdg-open` on Linux). > - In a headless container there is no `xdg-open`; `spawn` reports the missing binary asynchronously via an `'error'` event, which the surrounding `try/catch` cannot catch, so Node aborts the process before the approval can be polled. > - This makes CLI board auth impossible in the most common self-hosted deployment shape. > - This PR makes `openUrl` error-handled, async, and truthful, and adds headless affordances. > - The benefit is that board-authenticated CLI commands degrade gracefully and work headless instead of crashing. ## Linked Issues or Issue Description Closes #7941 ## What Changed - `openUrl` (`cli/src/client/board-auth.ts`) is now async and attaches an `'error'` listener to the spawned opener: resolves `false` on async spawn failure (missing binary) or sync throw, and `true` only on a successful `'spawn'`. Fixes the unhandled-`'error'` crash and makes the return value honest. - `loginBoardCli` prints an accurate "couldn't open a browser" message, supports `--no-browser` / `PAPERCLIP_NO_BROWSER` to skip the open attempt, and renders the approval URL from `PAPERCLIP_PUBLIC_URL` (or `publicBaseUrl`) so it's reachable from a remote operator's browser. - Updated the three other `openUrl` call sites (`cloud.ts` ×2, `company.ts`) to `await` it. - Added the `auth login --no-browser` flag. - Tests: new `open-url.test.ts` (launch → true, async ENOENT → false, sync throw → false); extended auth-command-registration test for `--no-browser`. ## Verification - `pnpm --filter paperclipai typecheck` — clean. - `pnpm exec vitest run cli/src/__tests__/open-url.test.ts cli/src/__tests__/auth-command-registration.test.ts` — pass. - Manual, in a headless container with no `xdg-open`: `pnpm paperclipai auth login -C <company-id>` now prints the approval URL and waits (previously crashed with `spawn xdg-open ENOENT`); `--no-browser` skips the open attempt; `PAPERCLIP_PUBLIC_URL=...` renders a reachable approval URL; completing approval in a browser stores the credential and `run list` works. ## Risks Low. `openUrl` became async; all four call sites updated to `await`. Desktop behavior is unchanged (successful spawn still resolves true and opens the browser). No API, schema, or migration changes. ## Model Used Claude Opus 4.8 (`claude-opus-4-8`) via Claude Code, with extended thinking and tool use, used to diagnose the bug and draft the fix and tests. Human-reviewed and tested on a live headless deployment. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR - [x] I have not referenced internal/instance-local Paperclip issues or links - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [ ] If this change affects the UI, I have included before/after screenshots (N/A — CLI only) - [ ] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] I will address all Greptile and reviewer comments before requesting merge Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -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", () => {
|
||||
|
||||
@@ -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<typeof import("node:child_process")>("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);
|
||||
});
|
||||
});
|
||||
@@ -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<T>(url: string, init?: RequestInit): Promise<T> {
|
||||
return response.json() as Promise<T>;
|
||||
}
|
||||
|
||||
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<boolean> {
|
||||
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<boolean>((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);
|
||||
|
||||
@@ -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(
|
||||
{
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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}`);
|
||||
|
||||
Reference in New Issue
Block a user