mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-21 08:22:20 +02:00
fix(cli): materialize the managed install before the onboarding service install (#12148)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Interactive onboarding offers to install Paperclip as a background service, defaulting to yes > - The service definition targets the managed command shim, but an ephemeral npx run never installs it, and the service step never checks > - The result is a crash-looping service, a doctor hint about a nonexistent port conflict, and a first run that ends with nothing serving > - This pull request materializes the managed install before registering the service, or declines with the repair path > - The benefit is that saying yes to the service prompt yields a working service — or an honest explanation ## Linked Issues or Issue Description **What happened?** On a machine with no managed install, `npx paperclipai@2026.824.0 onboard` (interactive), accepting the background-service prompt, produced: a LaunchAgent pointing at `~/.local/bin/paperclipai` (which does not exist), launchd exit code 78 in a KeepAlive crash loop, doctor reporting "inactive but the configured port is serving another Paperclip process — stop the conflicting foreground process" (no such process existed), and "Service health: fetch failed". Reproduced twice on a clean field. `latest` has carried this path since v2026.817.0 shipped; CI never sees it because `--yes` onboarding skips the service prompt. **Expected behavior** Accepting the service prompt installs a working service (materializing the managed payload and shim first when needed), and doctor diagnoses a missing service binary as exactly that. **Steps to reproduce** On macOS with no `~/.local/bin/paperclipai`: `npx paperclipai@latest onboard`, accept the service prompt, then `launchctl print gui/$UID/ing.paperclip.paperclipai` (exit code 78, spawn scheduled) and `paperclipai doctor`. **Paperclip version or commit** `2026.824.0` (path present since #10045). ## What Changed - `cli/src/onboard-service.ts`: after the user opts in, an `ensureServiceShim` step checks the service shim path. Missing + managed-store location → run `installCommand` pinned to the onboarding version (payload, shim, PATH block), then proceed. Missing + custom `PAPERCLIP_SHIM_PATH`, or install failure → decline with `paperclipai install` / `paperclipai service install` guidance and install nothing. - `cli/src/checks/service-health-check.ts`: the runtime check diagnoses a missing service binary with the install repair hint (instead of the port-conflict hint); an inactive service with a healthy responder gets a `warn` attributing the foreign process instead of a plain "Healthy" pass. - Tests: new cases for shim materialization ordering, decline-on-failure, missing-binary diagnosis, and foreign-responder attribution; existing fixtures updated to inject the new dependencies. ## Verification - `vitest run` on both touched suites: 15 pass. - `tsc --noEmit` error count identical to the master baseline (16 pre-existing, all in `server/`, none in changed files). - The live failure was reproduced on macOS before the fix (twice, clean field) and the mechanism confirmed in source: `install()` writes the definition and bootstraps launchd only; `install-store` was previously reachable solely from the `install`/`update` commands. ## Risks - Low: the new path runs only when the user opts into the service and the shim is absent. The managed install resolves the pinned onboarding version from the public registry; on failure the flow declines exactly as it does on unsupported platforms. `--yes` quickstarts, Docker, and managed installs are untouched. ## Model Used Claude Fable 5 (Claude Code) ## Pre-submission 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 following the relevant issue template
This commit is contained in:
1 parent
fa40a1b8d5
commit
faad235aa2
5 files changed
+363
-17
No files matched your search
@@ -1,4 +1,5 @@
|
||||
import fs from "node:fs/promises";
|
||||
import { constants as fsConstants } from "node:fs";
|
||||
import os from "node:os";
|
||||
import path from "node:path";
|
||||
import { execFile } from "node:child_process";
|
||||
@@ -33,6 +34,7 @@ export interface ServiceManager {
|
||||
restart(): Promise<void>;
|
||||
status(): Promise<ServiceStatus>;
|
||||
logs(follow: boolean, lines: number): Promise<void>;
|
||||
installedExecutablePath(): Promise<string | null>;
|
||||
enableLinger?(): Promise<void>;
|
||||
}
|
||||
|
||||
@@ -75,6 +77,45 @@ export function resolveServiceShimPath(homeDir = os.homedir()): string {
|
||||
return process.env.PAPERCLIP_SHIM_PATH?.trim() || path.join(homeDir, ".local", "bin", "paperclipai");
|
||||
}
|
||||
|
||||
// The installed definition, not the current environment, is the truth
|
||||
// about what the service executes: PAPERCLIP_SHIM_PATH may have changed
|
||||
// or been unset since the definition was written.
|
||||
function unescapeSystemd(value: string): string {
|
||||
return value.replace(/\\\\|\\"|\$\$|%%/g, (m) =>
|
||||
m === "\\\\" ? "\\" : m === '\\"' ? '"' : m === "$$" ? "$" : "%",
|
||||
);
|
||||
}
|
||||
|
||||
function unescapeXml(value: string): string {
|
||||
return value.replace(/&(amp|lt|gt|quot|apos);/g, (_, name: string) =>
|
||||
name === "amp" ? "&" : name === "lt" ? "<" : name === "gt" ? ">" : name === "quot" ? '"' : "'",
|
||||
);
|
||||
}
|
||||
|
||||
export function extractExecutableFromSystemdUnit(content: string): string | null {
|
||||
const match = content.match(/^ExecStart="((?:\\.|[^"\\])*)"/m);
|
||||
return match ? unescapeSystemd(match[1]) : null;
|
||||
}
|
||||
|
||||
export function extractExecutableFromLaunchdPlist(content: string): string | null {
|
||||
const match = content.match(/<key>ProgramArguments<\/key>\s*<array>\s*<string>([^<]+)<\/string>/);
|
||||
return match ? unescapeXml(match[1]) : null;
|
||||
}
|
||||
|
||||
// The service definition executes this path directly: existence is not
|
||||
// enough — a directory or a non-executable file would satisfy fs.access's
|
||||
// default mode and still crash the supervisor at spawn.
|
||||
export async function isExecutableFile(filePath: string): Promise<boolean> {
|
||||
try {
|
||||
const stats = await fs.stat(filePath);
|
||||
if (!stats.isFile()) return false;
|
||||
await fs.access(filePath, fsConstants.X_OK);
|
||||
return true;
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
export function systemdServiceName(instanceId: string): string {
|
||||
return instanceId === "default" ? "paperclipai.service" : `paperclipai-${instanceId}.service`;
|
||||
}
|
||||
@@ -174,6 +215,14 @@ export class SystemdServiceManager implements ServiceManager {
|
||||
return renderSystemdUnit({ instanceId: this.instanceId, shimPath: this.shimPath, homeDir: this.homeDir });
|
||||
}
|
||||
|
||||
async installedExecutablePath(): Promise<string | null> {
|
||||
try {
|
||||
return extractExecutableFromSystemdUnit(await fs.readFile(this.definitionPath, "utf8"));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private async ensureCurrent(): Promise<boolean> {
|
||||
const changed = await writeIfChanged(this.definitionPath, this.renderDefinition());
|
||||
if (changed) await this.runner("systemctl", ["--user", "daemon-reload"]);
|
||||
@@ -242,6 +291,14 @@ export class LaunchdServiceManager implements ServiceManager {
|
||||
|
||||
renderDefinition(): string { return renderLaunchdPlist({ instanceId: this.instanceId, shimPath: this.shimPath, homeDir: this.homeDir, stdoutPath: this.stdoutPath, stderrPath: this.stderrPath }); }
|
||||
|
||||
async installedExecutablePath(): Promise<string | null> {
|
||||
try {
|
||||
return extractExecutableFromLaunchdPlist(await fs.readFile(this.definitionPath, "utf8"));
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
async install(options: ServiceInstallOptions): Promise<{ changed: boolean }> {
|
||||
await fs.mkdir(path.dirname(this.stdoutPath), { recursive: true });
|
||||
const changed = await writeIfChanged(this.definitionPath, this.renderDefinition());
|
||||
|
||||
Reference in new issue
Block a user