From b2752b21d50f62ac5d634695e00a8c2fc72495d0 Mon Sep 17 00:00:00 2001 From: Nicky Leach Date: Fri, 28 Aug 2026 10:33:22 -0700 Subject: [PATCH] fix(adapter-utils): allow process sessions without birthtime (#12451) 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. > - Remote agent adapters use a process-session wrapper inside a sandbox. > - Some supported sandbox filesystems do not report inode creation time. > - The wrapper rejects a zero creation time before it launches the agent. > - This pull request accepts that filesystem shape and keeps the existing change-time probe. > - The benefit is that valid sandbox runs can start without a birth-time field. ## Linked Issues or Issue Description **What happened?** The remote process-session wrapper exited before it launched the agent when the sandbox filesystem reported `birthtimeMs` as zero. **Expected behavior** The wrapper must start on a filesystem that does not report inode creation time. **Steps to reproduce** 1. Start the remote process-session wrapper. 2. Make `lstat()` report a zero `birthtimeMs` for its session directory. 3. Observe that the pre-fix wrapper terminates before the child process starts. **Paperclip version or commit** Reproduced from `66e1c0df8b23cb8354b36dd446d9548dc4389191`. **Deployment mode** Self-hosted server with a remote sandbox runtime. ## What Changed - Allow a zero reported creation time for process-session directories. - Keep the probe that rejects a creation time copied from change time. - Add a regression test that launches and stops a session with zero birth time. ## Verification - `npx vitest run packages/adapter-utils/src/execution-target-stdin-race.test.ts` - `pnpm --filter @paperclipai/adapter-utils typecheck` ## Risks A filesystem without creation time can reduce the precision of sandbox-local path-swap detection. This change does not change host-file or Paperclip API authority. A follow-up will review that larger security posture alignment. > I checked `ROADMAP.md`. This is a focused compatibility bug fix for the existing sandbox-agent roadmap area. ## Model Used OpenAI Codex — GPT-5.6. The exact deployment suffix and context-window size are not exposed. The model used reasoning, shell 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 --- .../src/execution-target-stdin-race.test.ts | 26 ++++++++++++------- .../adapter-utils/src/execution-target.ts | 24 +++-------------- 2 files changed, 20 insertions(+), 30 deletions(-) diff --git a/packages/adapter-utils/src/execution-target-stdin-race.test.ts b/packages/adapter-utils/src/execution-target-stdin-race.test.ts index 4b5dfdc027..9341358c38 100644 --- a/packages/adapter-utils/src/execution-target-stdin-race.test.ts +++ b/packages/adapter-utils/src/execution-target-stdin-race.test.ts @@ -1796,7 +1796,7 @@ describe("deterministic remote process-session wrapper shutdown (PAP-5316)", () await waitFor(async () => (await findLivePidsByArgvSubstring(wrapperScriptSubstring)).length === 0, 8_000); }, 15_000); - // ---- PAP-5338: fail closed on an unusable creation time, and on every + // ---- PAP-5338: reject a change-time creation-time substitute, and every // lstat error during verification ------------------------------------- async function waitForTrackedChildPid(pidFile: string): Promise { @@ -1822,18 +1822,17 @@ describe("deterministic remote process-session wrapper shutdown (PAP-5316)", () // ever starts, often within a few milliseconds of the child's own spawn() // call returning. A freshly spawned Node.js child needs real wall-clock // time just to boot before it can run its own code, so it can lose the - // race to write a pid file before terminate()'s SIGTERM reaches it. This - // is the correct, intended shape of a fail-fast capture: the child never - // gets a chance to become a live orphan. So these two tests prove death - // through the OS process table by the child's own script path (the same - // technique T15 above uses for the wrapper itself), which needs no - // cooperation from code inside the child. + // race to write a pid file before terminate()'s SIGTERM reaches it. The + // change-time fallback test below therefore proves death through the OS + // process table by the child's own script path (the same technique T15 + // above uses for the wrapper itself), which needs no cooperation from code + // inside the child. async function expectNoLiveProcessByArgvSubstring(substring: string): Promise { await waitFor(async () => (await findLivePidsByArgvSubstring(substring)).length === 0, 8_000); expect(await findLivePidsByArgvSubstring(substring)).toEqual([]); } - it("T16 fails closed at capture when the reported creation time is zero, so no orphan wrapper or child ever starts polling", async () => { + it("T16 accepts a zero creation time when the filesystem does not report birth time", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-birthtime-zero-")); cleanupDirs.push(rootDir); const pidFile = path.join(rootDir, "t16-child.pid"); @@ -1847,14 +1846,21 @@ describe("deterministic remote process-session wrapper shutdown (PAP-5316)", () fakeBirthtime: { target: "sessionDir", mode: "zero" }, }); + const pid = await waitForTrackedChildPid(pidFile); + expect(isPidAlive(pid)).toBe(true); + await writeFile( + path.join(wrapper.stdinDir, "000000000001.json"), + `${JSON.stringify({ type: "stdinEnd" })}\n`, + "utf8", + ); await Promise.race([ wrapper.exited, delay(8_000).then(() => { throw new Error("The wrapper process did not exit."); }), ]); - expect(wrapper.stderrText()).toMatch(/not usable/); - await expectNoLiveProcessByArgvSubstring(childPath); + expect(wrapper.exitInfo().code).toBe(0); + expect(wrapper.stderrText()).not.toMatch(/not usable/); }, 15_000); it("T17 fails closed at capture when the reported creation time follows the change time, so a change-time copy never passes as a real creation time", async () => { diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 2a083f8a01..c11463841a 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -2461,10 +2461,6 @@ async function latchAndTerminate() { await terminate(); } -function isUsableBirthtimeMs(value) { - return typeof value === "number" && Number.isFinite(value) && value !== 0; -} - let probeSeq = 0; // A probe file name that pollStdin() can never read as a stdin message: it @@ -2593,14 +2589,10 @@ async function refuseUnusableCreationTime(label, dirPath, reason) { // it terminates now instead of polling a control path it never verified. // // This wrapper cannot assume stats.birthtimeMs is a real creation time. Node -// reports it in one of two unusable shapes on a filesystem or kernel that -// cannot supply one: 0 (the Linux statx() path when the filesystem reports -// no STATX_BTIME), or a copy of the change time (the generic POSIX stat() -// path on a platform with no birthtime field). A 0 value fails open, so this -// wrapper rejects it outright. A change-time copy fails closed but far too -// aggressively (it would move on every stdin file this wrapper deletes), so -// this wrapper proves the value is not a copy with a probe before it trusts -// it, run once here, before either directory's identity is captured. +// can report a change-time copy as a creation time. That value fails closed +// far too aggressively (it would move on every stdin file this wrapper +// deletes), so this wrapper proves the value is not a copy with a probe before +// it trusts it, run once here, before either directory's identity is captured. async function captureSessionIdentity() { try { const sessionProbeFailure = await birthtimeSurvivesProbe(sessionDir); @@ -2615,14 +2607,6 @@ async function captureSessionIdentity() { } const session = await statPathIdentity(sessionDir); const stdin = await statPathIdentity(stdinDir); - if (!isUsableBirthtimeMs(session.birthtimeMs)) { - await refuseUnusableCreationTime("sessionDir", sessionDir, "its reported creation time (" + session.birthtimeMs + ") is not usable"); - return; - } - if (!isUsableBirthtimeMs(stdin.birthtimeMs)) { - await refuseUnusableCreationTime("stdinDir", stdinDir, "its reported creation time (" + stdin.birthtimeMs + ") is not usable"); - return; - } sessionDirIdentity = session; stdinDirIdentity = stdin; } catch (error) {