From 0e21a273012fa236d2c85aafb954bfa3b0cb4361 Mon Sep 17 00:00:00 2001 From: machjesusmoto Date: Mon, 13 Jul 2026 10:33:08 -0700 Subject: [PATCH] fix: forward onSpawn to hermes and process adapters for PID persistence (#8722) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - The adapter layer (hermes-local, process adapters) delegates agent execution to child processes via `runChildProcess()` > - `runChildProcess()` accepts an `onSpawn` callback to report child PID and process group info, but the hermes and process adapters were not forwarding `ctx.onSpawn` to this call > - Without PID persistence, the orphan reaper cannot distinguish live runs from abandoned processes, causing false-positive reaps and 5-minute timeout errors for active runs > - This pull request adds `onSpawn: ctx.onSpawn` to both adapter call sites and declares the option in the `runChildProcess` wrapper type > - The benefit is that the orphan reaper can now correctly track live child processes, eliminating false-positive reaps ## Linked Issues or Issue Description Fixes #8723 Fixes false-positive orphan reaps in hermes-local and process adapters by forwarding the `onSpawn` callback to `runChildProcess()`. All other adapters (claude-local, codex-local, cursor-local, gemini-local, grok-local, opencode-local, pi-local) already forward `ctx.onSpawn` — these two were the only ones missing it. ## What Changed - `server/src/adapters/utils.ts`: Added `onSpawn?` to the `runChildProcess()` options type so callers can forward the callback - `server/src/adapters/process/execute.ts`: Forward `ctx.onSpawn` to `runChildProcess()` - `packages/adapters/hermes/src/server/execute.ts`: Forward `ctx.onSpawn` to `runChildProcess()` ## Verification - `pnpm -r typecheck` passes across all packages - Confirmed all other adapters already forward `ctx.onSpawn` (12 grep matches across 9 adapter files) - The 3-line diff is additive only — no existing behavior is changed, only a previously-ignored callback is now forwarded ## Risks Low risk. This is a 3-line additive change. The `onSpawn` parameter is optional (`?`) so existing callers are unaffected. The callback is already well-established across all other adapters. ## Model Used Hermes Agent (by Nous Research) — xiaomi/mimo-v2.5-pro via OpenRouter, with tool use (file editing, git, GitHub API). ## 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 - [x] My branch name describes the change and contains no internal ticket id - [x] I have run tests locally and they pass (typecheck passes) - [x] I have added or updated tests where applicable (N/A — type-level fix only, no behavioral change) - [x] I have updated relevant documentation to reflect my changes (N/A — internal fix) - [x] I have considered and documented any risks above --------- Co-authored-by: Zephyr --- .../hermes/src/server/execute.onspawn.test.ts | 121 ++++++++++++++++++ .../adapters/hermes/src/server/execute.ts | 1 + server/src/adapters/process/execute.ts | 1 + server/src/adapters/utils.ts | 1 + 4 files changed, 124 insertions(+) create mode 100644 packages/adapters/hermes/src/server/execute.onspawn.test.ts diff --git a/packages/adapters/hermes/src/server/execute.onspawn.test.ts b/packages/adapters/hermes/src/server/execute.onspawn.test.ts new file mode 100644 index 0000000000..c323bf6887 --- /dev/null +++ b/packages/adapters/hermes/src/server/execute.onspawn.test.ts @@ -0,0 +1,121 @@ +/** + * Regression test for onSpawn forwarding in the hermes-local adapter. + * + * Ensures ctx.onSpawn is forwarded to runChildProcess() so the orphan + * reaper can track live child processes by PID, preventing false-positive + * reaps on runs whose updatedAt becomes stale. + * + * @see https://github.com/paperclipai/paperclip/issues/8723 + */ + +import { describe, expect, it, vi, beforeEach } from "vitest"; + +// Mock the adapter-utils server-utils module that execute.ts imports from. +// We intercept runChildProcess so we can inspect its opts without spawning +// a real child process. +vi.mock("@paperclipai/adapter-utils/server-utils", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + runChildProcess: vi.fn(async () => ({ + exitCode: 0, + signal: null, + timedOut: false, + stdout: "", + stderr: "", + })), + }; +}); + +// Mock fs and path resolution to avoid real file reads in execute() +vi.mock("node:fs/promises", () => ({ + readFile: vi.fn(async () => ""), + writeFile: vi.fn(async () => undefined), + mkdir: vi.fn(async () => undefined), + rm: vi.fn(async () => undefined), + access: vi.fn(async () => undefined), + readdir: vi.fn(async () => []), + stat: vi.fn(async () => ({ isFile: () => true, isDirectory: () => false })), +})); + +import { execute } from "./execute.js"; +import * as serverUtils from "@paperclipai/adapter-utils/server-utils"; + +function makeCtx(overrides: Record = {}) { + const onSpawn = vi.fn(async () => undefined); + return { + ctx: { + runId: "test-run-1", + agent: { + id: "agent-1", + companyId: "company-1", + name: "Hermes", + adapterType: "hermes_local", + adapterConfig: {}, + }, + runtime: { + sessionId: null, + sessionParams: null, + sessionDisplayId: null, + taskKey: null, + }, + config: { + command: "/usr/bin/hermes", + timeoutSec: 60, + graceSec: 5, + ...overrides, + }, + context: { + issueId: "issue-1", + wakeReason: "manual", + paperclipWake: null, + }, + onLog: vi.fn(async () => undefined), + onMeta: vi.fn(async () => undefined), + onSpawn, + } satisfies Record, + onSpawn, + }; +} + +describe("hermes-local adapter onSpawn forwarding", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + it("forwards ctx.onSpawn to runChildProcess", async () => { + const { ctx, onSpawn } = makeCtx(); + + // execute() will call runChildProcess internally. + // We expect it to propagate ctx.onSpawn. + // Because we mocked runChildProcess, the actual child doesn't spawn, + // but we can verify it was called with onSpawn. + try { + await execute(ctx as any); + } catch { + // execute may fail due to missing hermes binary / env — that's OK, + // we only care that runChildProcess was called with onSpawn. + } + + const mocked = vi.mocked(serverUtils.runChildProcess); + expect(mocked.mock.calls.length).toBeGreaterThan(0); + const lastCall = mocked.mock.calls[mocked.mock.calls.length - 1]; + const opts = lastCall[3] as Record; + expect(opts.onSpawn).toBe(onSpawn); + }); + + it("runChildProcess opts type includes onSpawn", () => { + // Type-level assertion: if onSpawn were removed from the type, + // this file would fail to compile. The runtime test above catches + // the behavioral case; this documents the contract. + const opts: Parameters[3] = { + cwd: "/tmp", + env: {}, + timeoutSec: 60, + graceSec: 5, + onLog: async () => undefined, + onSpawn: async () => undefined, + }; + expect(opts.onSpawn).toBeDefined(); + }); +}); diff --git a/packages/adapters/hermes/src/server/execute.ts b/packages/adapters/hermes/src/server/execute.ts index ae85c29857..0a0f76ff84 100644 --- a/packages/adapters/hermes/src/server/execute.ts +++ b/packages/adapters/hermes/src/server/execute.ts @@ -527,6 +527,7 @@ export async function execute( timeoutSec, graceSec, onLog: wrappedOnLog, + onSpawn: ctx.onSpawn, }); // ── Parse output ─────────────────────────────────────────────────────── diff --git a/server/src/adapters/process/execute.ts b/server/src/adapters/process/execute.ts index ff2bf82e85..77deb30c1c 100644 --- a/server/src/adapters/process/execute.ts +++ b/server/src/adapters/process/execute.ts @@ -50,6 +50,7 @@ export async function execute(ctx: AdapterExecutionContext): Promise Promise; + onSpawn?: (meta: { pid: number; processGroupId: number | null; startedAt: string }) => Promise; }, ): Promise { return _runChildProcess(runId, command, args, {