mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-08 11:13:44 +02:00
## 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>
58 lines
2.0 KiB
TypeScript
58 lines
2.0 KiB
TypeScript
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);
|
|
});
|
|
});
|