mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 21:05:21 +02:00
DottaandPaperclip
1ee1275f11
## Thinking Path > - Paperclip is the open source control plane people use to manage AI agents for work. > - Local agent heartbeats need durable process identity so the server can supervise them. > - The ACPX runtime owns the child process used by `codex_local` sessions. > - ACPX did not expose the child PID and start time to the Paperclip adapter. > - Warm ACPX runtimes can also serve a later heartbeat without a new spawn event. > - A hot restart could therefore classify a live Codex run as lost because its heartbeat row had no process identity. > - This pull request forwards ACPX spawn identity, reuses it for compatible warm heartbeats, and fails closed when identity cannot be persisted. > - The benefit is reliable hot-restart adoption for eligible local Codex runs. ## Linked Issues or Issue Description No matching public GitHub issue was found. **What happened?** A `codex_local` heartbeat could run through ACPX without a persisted `processPid` or `processStartedAt`. A Paperclip hot restart then had no durable identity for the live ACP child. Recovery could classify the run as `process_lost` even while the child was still alive. **Expected behavior** ACPX reports the real child PID and start time before the first prompt. A compatible warm runtime reports the same known identity to each later heartbeat that reuses the child. ACPX stops the child if the identity is invalid or persistence fails. Hot-restart recovery can then adopt the live run. **Steps to reproduce** 1. Start a `codex_local` heartbeat through the ACPX execution lane. 2. Keep the run active during a Paperclip hot restart. 3. Inspect the heartbeat row before this change. 4. Observe that the process identity can be null and recovery cannot adopt the live child. **Reproduced on** - Paperclip `master` before this change. - Linux source deployment. - `codex_local` with ACPX `0.12.0`. ## What Changed - Add an awaited `onAgentSpawn` lifecycle hook to the patched ACPX runtime. - Forward the ACP child PID and start time through the adapter `onSpawn` callback. - Keep a mutable callback sink for cached runtimes so a later respawn updates the current heartbeat. - Reuse the last known process identity when a compatible warm heartbeat reuses the existing child. - Kill the ACP child and fail session startup when the PID is invalid or identity persistence rejects. - Add ACPX and heartbeat recovery tests for callback ordering, warm reuse, failure cleanup, durable row identity, and hot-restart adoption. - Document the one-time drain required when an installed pre-fix run already lacks process metadata. ## Verification - `PAPERCLIP_HOME="$PAPERCLIP_RUN_SCRATCH_DIR/test-home-execute-escalated" pnpm exec vitest run packages/adapter-utils/src/acpx-engine/execute.test.ts` — 89 passed. - `PAPERCLIP_HOME="$PAPERCLIP_RUN_SCRATCH_DIR/test-home-recovery-escalated" pnpm exec vitest run server/src/__tests__/heartbeat-process-recovery.test.ts` — 92 passed. - `PAPERCLIP_HOME="$PAPERCLIP_RUN_SCRATCH_DIR/test-home-remote-smoke-escalated" pnpm exec vitest run packages/adapter-utils/src/acpx-engine/remote-spawn-smoke.test.ts` — 3 passed. - `PAPERCLIP_HOME="$PAPERCLIP_RUN_SCRATCH_DIR/test-home-ci-repro-escalated" pnpm exec vitest run server/src/__tests__/heartbeat-dependency-scheduling.test.ts` — 6 passed. - `pnpm --filter @paperclipai/adapter-utils typecheck` — passed. - Reverse and forward dry-run application of `patches/acpx@0.12.0.patch` — passed. - `git diff --check` — passed. - `git diff --exit-code origin/master...HEAD -- pnpm-lock.yaml` — passed. - `git diff --exit-code origin/master...HEAD -- .github/workflows` — passed. ## Risks - Runtime risk is low to moderate. ACPX now awaits process-identity persistence during child startup. - ACPX kills the child when persistence fails. This prevents an unsupervised process, but it makes that heartbeat fail visibly. - A compatible warm heartbeat reuses the identity of the existing ACP child. Regression tests verify that identity is persisted before the next prompt. - The change updates the vendored ACPX patch. Package installation must apply that patch. - There are no schema, migration, public API, UI, workflow, or lockfile changes. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex used GPT-5.3-Codex for the earlier implementation. - OpenAI Codex used GPT-5 for the lifecycle-hook revision and the current fail-closed review fix. The runtime did not expose a more specific snapshot ID or context-window size. Both runs used reasoning, repository tools, and code execution. ## 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 - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
100 lines
4.2 KiB
Diff
100 lines
4.2 KiB
Diff
diff --git a/dist/live-checkpoint-ClPCSdrW.js b/dist/live-checkpoint-ClPCSdrW.js
|
|
index 243c9d13bcba520923b63adfddad75cf2d94362d..336a1699b80b9e99416f9da4346ca73ee2ad1626 100644
|
|
--- a/dist/live-checkpoint-ClPCSdrW.js
|
|
+++ b/dist/live-checkpoint-ClPCSdrW.js
|
|
@@ -1532,7 +1532,7 @@ const ZED_TAG_KEYS = /* @__PURE__ */ new Set([
|
|
"RedactedThinking",
|
|
"ToolUse"
|
|
]);
|
|
-const MAP_OBJECT_PATHS = /* @__PURE__ */ new Set(["request_token_usage", "messages.Agent.tool_results"]);
|
|
+const MAP_OBJECT_PATHS = /* @__PURE__ */ new Set(["request_token_usage", "messages.Agent.tool_results", "acpx.session_options.env"]);
|
|
const OPAQUE_VALUE_PATHS = /* @__PURE__ */ new Set([
|
|
"agent_capabilities",
|
|
"messages.Agent.content.ToolUse.input",
|
|
@@ -2557,7 +2557,7 @@ function readCommandLineChar(state) {
|
|
escaping: false,
|
|
hasPart: true
|
|
};
|
|
- if (state.ch === "\\" && state.quote !== "'") return {
|
|
+ if (process.platform !== "win32" && state.ch === "\\" && state.quote !== "'") return {
|
|
current: state.current,
|
|
quote: state.quote,
|
|
escaping: true,
|
|
@@ -3959,7 +3959,24 @@ var AcpClient = class {
|
|
- this.attachAgentLifecycleObservers(child);
|
|
+ this.attachAgentLifecycleObservers(child);
|
|
+ if (this.options.onAgentSpawn) {
|
|
+ try {
|
|
+ if (typeof child.pid !== "number" || child.pid <= 0) {
|
|
+ throw new Error("ACPX agent spawn did not expose a valid process id.");
|
|
+ }
|
|
+ await this.options.onAgentSpawn({ pid: child.pid, startedAt: this.agentStartedAt });
|
|
+ } catch (error) {
|
|
+ try {
|
|
+ child.kill("SIGKILL");
|
|
+ } catch {}
|
|
+ throw error;
|
|
+ }
|
|
+ }
|
|
const startupStderr = [];
|
|
child.stderr.on("data", (chunk) => {
|
|
this.captureStartupStderr(startupStderr, chunk);
|
|
+ if (this.options.onAgentStderr) {
|
|
+ this.options.onAgentStderr(chunk.toString());
|
|
+ return;
|
|
+ }
|
|
if (!this.options.verbose) return;
|
|
process.stderr.write(chunk);
|
|
});
|
|
@@ -3994,7 +4011,7 @@ var AcpClient = class {
|
|
geminiAcp: isGeminiAcpCommand(spawnCommand, args),
|
|
copilotAcp: isCopilotAcpCommand(spawnCommand, args),
|
|
claudeAcp: isClaudeAcpCommand(spawnCommand, args),
|
|
- spawnOptions: buildAgentSpawnOptions(this.options.cwd, this.options.authCredentials, this.options.sessionOptions?.env)
|
|
+ spawnOptions: buildAgentSpawnOptions(this.options.spawnCwd ?? this.options.cwd, this.options.authCredentials, this.options.sessionOptions?.env)
|
|
};
|
|
}
|
|
logAgentLaunch(plan) {
|
|
diff --git a/dist/runtime.d.ts b/dist/runtime.d.ts
|
|
index ccdbe5b032521518022223733049b8b38793473b..3d4e04231e78efeecd8540735b08e1e43547ff2d 100644
|
|
--- a/dist/runtime.d.ts
|
|
+++ b/dist/runtime.d.ts
|
|
@@ -266,6 +266,9 @@ type AcpRuntimeOptions = {
|
|
timeoutMs?: number;
|
|
probeAgent?: string;
|
|
verbose?: boolean;
|
|
+ onAgentStderr?: (chunk: string) => void;
|
|
+ onAgentSpawn?: (meta: { pid: number; startedAt: string }) => Promise<void>;
|
|
+ spawnCwd?: string;
|
|
onPermissionRequest?: (req: AcpPermissionRequest, ctx: {
|
|
signal: AbortSignal;
|
|
}) => Promise<AcpPermissionDecision | undefined>;
|
|
diff --git a/dist/runtime.js b/dist/runtime.js
|
|
index 6c9cc999e50a11c399c68b3a0f1b7af4bc2317c0..33b5054b2906502d1d4b512bfa259bd2ba5a9f05 100644
|
|
--- a/dist/runtime.js
|
|
+++ b/dist/runtime.js
|
|
@@ -744,7 +744,8 @@ var AcpRuntimeManager = class {
|
|
this.deps = deps;
|
|
}
|
|
createClient(options) {
|
|
- return this.deps.clientFactory?.(options) ?? new AcpClient(options);
|
|
+ const clientOptions = { ...options, onAgentStderr: this.options.onAgentStderr, onAgentSpawn: this.options.onAgentSpawn, spawnCwd: this.options.spawnCwd };
|
|
+ return this.deps.clientFactory?.(clientOptions) ?? new AcpClient(clientOptions);
|
|
}
|
|
async readPendingPersistentClient(record, options) {
|
|
const pendingClient = this.pendingPersistentClients.get(record.acpxRecordId);
|
|
diff --git a/dist/session-options-jkYbBxGE.d.ts b/dist/session-options-jkYbBxGE.d.ts
|
|
index 9d37f377fb6a0828e0d2bc5a48754f3aa71509a4..680bc080fc5d6ffd266ed1b27d3d5056add9d980 100644
|
|
--- a/dist/session-options-jkYbBxGE.d.ts
|
|
+++ b/dist/session-options-jkYbBxGE.d.ts
|
|
@@ -84,6 +84,9 @@ type AcpClientOptions = {
|
|
terminal?: boolean;
|
|
suppressSdkConsoleErrors?: boolean;
|
|
verbose?: boolean;
|
|
+ onAgentStderr?: (chunk: string) => void;
|
|
+ onAgentSpawn?: (meta: { pid: number; startedAt: string }) => Promise<void>;
|
|
+ spawnCwd?: string;
|
|
sessionOptions?: {
|
|
model?: string;
|
|
allowedTools?: string[];
|