fix(cli): deadline-based service wait, no new dependency, honest URL

Address review and policy: drop the open dependency (platform opener
via execFile - the lockfile is CI-owned and e2e installs frozen), count
probe time against the readiness deadline, and announce the URL only
when onboarding configured the default config path the supervisor
actually reads - otherwise say which path the service reads and point
at service status.
This commit is contained in:
Devin Foley committed 2026-08-25 01:28:23 -07:00
1 parent 6c12246e6b
commit c664a5de76
5 files changed
+52 -14

No files matched your search

-1
View File
@@ -56,7 +56,6 @@
"dotenv": "^17.4.2",
"commander": "^13.1.0",
"embedded-postgres": "^18.1.0-beta.16",
"open": "^11.0.1",
"picocolors": "^1.1.1"
},
"devDependencies": {
+15 -2
View File
@@ -165,10 +165,12 @@ describe("shouldOfferForegroundStart", () => {
describe("announceServiceReady", () => {
function readyDeps(overrides: Record<string, unknown> = {}) {
let clock = 0;
return {
probeHealth: vi.fn(async () => true),
openBrowser: vi.fn(async () => undefined),
sleep: vi.fn(async () => undefined),
now: vi.fn(() => { clock += 10; return clock; }),
info: vi.fn(),
success: vi.fn(),
warn: vi.fn(),
@@ -188,11 +190,22 @@ describe("announceServiceReady", () => {
it("polls until the service becomes healthy", async () => {
const probeHealth = vi.fn(async () => false).mockResolvedValueOnce(false).mockResolvedValueOnce(false).mockResolvedValue(true);
const deps = readyDeps({ probeHealth });
const ready = await announceServiceReady({ baseUrl: "http://127.0.0.1:3100", interactive: true, timeoutMs: 50, pollIntervalMs: 10 }, deps);
const ready = await announceServiceReady({ baseUrl: "http://127.0.0.1:3100", interactive: true, timeoutMs: 100, pollIntervalMs: 10 }, deps);
expect(ready).toBe(true);
expect(probeHealth).toHaveBeenCalledTimes(3);
});
it("counts probe time against the deadline instead of only sleeps", async () => {
let clock = 0;
const deps = readyDeps({
probeHealth: vi.fn(async () => { clock += 2_000; return false; }),
now: vi.fn(() => clock),
});
const ready = await announceServiceReady({ baseUrl: "http://127.0.0.1:3100", interactive: true, timeoutMs: 6_000, pollIntervalMs: 1_000 }, deps);
expect(ready).toBe(false);
expect(deps.probeHealth).toHaveBeenCalledTimes(3);
});
it("prints the URL instead of opening a browser on non-interactive runs", async () => {
const deps = readyDeps();
await announceServiceReady({ baseUrl: "http://127.0.0.1:3100", interactive: false }, deps);
@@ -209,7 +222,7 @@ describe("announceServiceReady", () => {
it("warns with service diagnostics when the service never answers", async () => {
const deps = readyDeps({ probeHealth: vi.fn(async () => false) });
const ready = await announceServiceReady({ baseUrl: "http://127.0.0.1:3100", interactive: true, timeoutMs: 30, pollIntervalMs: 10 }, deps);
const ready = await announceServiceReady({ baseUrl: "http://127.0.0.1:3100", interactive: true, timeoutMs: 25, pollIntervalMs: 10 }, deps);
expect(ready).toBe(false);
expect(deps.openBrowser).not.toHaveBeenCalled();
const warned = (deps.warn as ReturnType<typeof vi.fn>).mock.calls[0]?.[0] as string;
+22 -4
View File
@@ -43,6 +43,7 @@ import {
resolveDefaultBackupDir,
resolveDefaultEmbeddedPostgresDir,
resolveDefaultLogsDir,
resolveDefaultConfigPath,
resolvePaperclipInstanceId,
} from "../config/home.js";
import { bootstrapCeoInvite } from "./auth-bootstrap-ceo.js";
@@ -129,6 +130,23 @@ function resolvePathFromEnv(rawValue: string | undefined): string | null {
return path.resolve(expandHomePrefix(rawValue.trim()));
}
async function announceInstalledService(options: { configPath: string; baseUrl: string }): Promise<void> {
const serviceConfigPath = resolveDefaultConfigPath(resolvePaperclipInstanceId());
if (path.resolve(options.configPath) === path.resolve(serviceConfigPath)) {
await announceServiceReady({
baseUrl: options.baseUrl,
interactive: Boolean(process.stdin.isTTY && process.stdout.isTTY),
});
return;
}
p.log.info(
[
`The background service reads ${serviceConfigPath}, not ${options.configPath}.`,
`Check ${pc.cyan("paperclipai service status")} for the served address.`,
].join("\n"),
);
}
function describeServerBinding(server: Pick<PaperclipConfig["server"], "bind" | "customBindHost" | "host" | "port">): string {
const bind = server.bind ?? inferBindModeFromHost(server.host);
const detail =
@@ -459,9 +477,9 @@ export async function onboard(opts: OnboardOptions): Promise<void> {
printManagedInstallHint();
const serviceInstalled = await handleOnboardService(opts);
if (serviceInstalled) {
await announceServiceReady({
await announceInstalledService({
configPath,
baseUrl: buildLocalBaseUrl(existingConfig.server.host, existingConfig.server.port),
interactive: Boolean(process.stdin.isTTY && process.stdout.isTTY),
});
}
@@ -731,9 +749,9 @@ export async function onboard(opts: OnboardOptions): Promise<void> {
const serviceInstalled = await handleOnboardService(opts);
if (serviceInstalled) {
await announceServiceReady({
await announceInstalledService({
configPath,
baseUrl: buildLocalBaseUrl(server.host, server.port),
interactive: Boolean(process.stdin.isTTY && process.stdout.isTTY),
});
}
+15 -4
View File
@@ -170,6 +170,7 @@ export type ServiceReadyDependencies = {
probeHealth: (healthUrl: string) => Promise<boolean>;
openBrowser: (url: string) => Promise<void>;
sleep: (ms: number) => Promise<void>;
now: () => number;
info: (message: string) => void;
success: (message: string) => void;
warn: (message: string) => void;
@@ -185,11 +186,18 @@ const defaultServiceReadyDependencies: ServiceReadyDependencies = {
return false;
}
},
// The platform opener keeps this dependency-free; a failure only means the
// caller prints the address instead.
openBrowser: async (url) => {
const mod = await import("open");
await mod.default(url);
const { execFile } = await import("node:child_process");
const command = process.platform === "darwin" ? "open" : process.platform === "win32" ? "cmd" : "xdg-open";
const args = process.platform === "win32" ? ["/c", "start", "", url] : [url];
await new Promise<void>((resolve, reject) => {
execFile(command, args, (error) => (error ? reject(error) : resolve()));
});
},
sleep: (ms) => new Promise((resolve) => setTimeout(resolve, ms)),
now: () => Date.now(),
info: (message) => p.log.message(pc.dim(message)),
success: (message) => p.log.success(message),
warn: (message) => p.log.warn(message),
@@ -208,8 +216,10 @@ export async function announceServiceReady(
const pollIntervalMs = options.pollIntervalMs ?? 1_000;
const healthUrl = `${options.baseUrl}/api/health`;
deps.info(`Waiting for the server at ${options.baseUrl} ...`);
const attempts = Math.max(1, Math.ceil(timeoutMs / pollIntervalMs));
for (let attempt = 0; attempt < attempts; attempt += 1) {
// Probe time counts against the deadline: a slow probe must not stretch
// the configured timeout.
const deadline = deps.now() + timeoutMs;
for (;;) {
if (await deps.probeHealth(healthUrl)) {
deps.success(`Paperclip is running at ${options.baseUrl}`);
if (options.interactive) {
@@ -223,6 +233,7 @@ export async function announceServiceReady(
deps.info(`Open ${options.baseUrl} in your browser to get started.`);
return true;
}
if (deps.now() >= deadline) break;
await deps.sleep(pollIntervalMs);
}
deps.warn(
-3
View File
@@ -109,9 +109,6 @@ importers:
embedded-postgres:
specifier: ^18.1.0-beta.16
version: 18.1.0-beta.16(patch_hash=55uhvnotpqyiy37rn3pqpukhei)
open:
specifier: ^11.0.1
version: 11.0.1
picocolors:
specifier: ^1.1.1
version: 1.1.1