From 223068e2ff0a1dc2f2a77ab8a4964ff4c163f7ee Mon Sep 17 00:00:00 2001 From: Priya Raman Date: Tue, 4 Aug 2026 06:40:12 +0000 Subject: [PATCH] fix(sandbox): keep bridge queue-directory setup on the startup step The paperclip callback bridge reset the active startup-step store for the whole worker start. That also unparented the awaited queue-directory setup execs, which are startup work and must keep the `bridge.paperclip` step. Move the store reset to the long-lived poll loop inside the worker. The `makeDir` setup now keeps the active step, so its `sandbox.exec` spans stay parented with the correct `criticalPath` flag. The loop still runs with an empty store, so each run-time exec span opens unparented. Add a regression test that reads the active step during setup and during the first poll, and proves the boundary sits at the loop, not the whole worker. Co-authored-by: Paperclip --- .../adapter-utils/src/execution-target.ts | 15 ++-- .../src/sandbox-callback-bridge.test.ts | 68 +++++++++++++++++++ .../src/sandbox-callback-bridge.ts | 11 ++- 3 files changed, 84 insertions(+), 10 deletions(-) diff --git a/packages/adapter-utils/src/execution-target.ts b/packages/adapter-utils/src/execution-target.ts index 7ebbc53f8f..2ee9c0f049 100644 --- a/packages/adapter-utils/src/execution-target.ts +++ b/packages/adapter-utils/src/execution-target.ts @@ -1799,13 +1799,12 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: { // this flag is enabled. Only intended for active debugging in trusted // environments. const bridgeDebugEnabled = isBridgeDebugEnabled(process.env); - // Start the long-lived callback-bridge worker loop outside the measured - // bridge step store. The loop reads and writes queue files with run-time - // execs for the whole run, not startup work, so a worker `sandbox.exec` span - // must not parent to the ended `bridge.paperclip` step or copy its - // `criticalPath` flag. `runWithoutActiveStep` empties the store for the loop - // that the worker start schedules, so every later poll tick stays unparented. - worker = await runWithoutActiveStep(() => startSandboxCallbackBridgeWorker({ + // `startSandboxCallbackBridgeWorker` keeps its awaited queue-directory + // setup on the active `bridge.paperclip` step, and resets the store only + // for its long-lived poll loop (see `runWithoutActiveStep` inside that + // function). So the startup `mkdir` execs stay parented and every later + // loop `sandbox.exec` span stays unparented with no stale `criticalPath`. + worker = await startSandboxCallbackBridgeWorker({ client, queueDir, maxBodyBytes, @@ -1842,7 +1841,7 @@ export async function startAdapterExecutionTargetPaperclipBridge(input: { body: await readBridgeForwardResponseBody(response, maxBodyBytes), }; }, - })); + }); server = await startSandboxCallbackBridgeServer({ runner, remoteCwd: target.remoteCwd, diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts index 73fc3b472a..f36f63bbef 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.test.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.test.ts @@ -5,6 +5,7 @@ import path from "node:path"; import { promisify } from "node:util"; import { afterEach, describe, expect, it, vi } from "vitest"; +import { getActiveStepContext, measureStartupStep } from "./acpx-engine/startup-timing.js"; import { prepareCommandManagedRuntime } from "./command-managed-runtime.js"; import { authorizeSandboxCallbackBridgeRequestWithRoutes, @@ -471,6 +472,73 @@ describe("sandbox callback bridge", () => { } }); + it("keeps the queue-directory setup on the startup step but resets the poll loop store", async () => { + // The worker starts inside the measured `bridge.paperclip` step. Its awaited + // queue-directory setup is startup work, so a `makeDir` `sandbox.exec` span + // must keep the active step and its `criticalPath` flag. The long-lived poll + // loop runs run-time execs for the whole run, so a loop `sandbox.exec` span + // must open unparented with no stale flag. This test reads the active step in + // both places and proves the boundary sits at the loop, not the whole worker. + let setupStep: ReturnType | "unset" = "unset"; + let loopStep: ReturnType | "unset" = "unset"; + let resolveFirstPoll: () => void = () => {}; + const firstPoll = new Promise((resolve) => { + resolveFirstPoll = resolve; + }); + + const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-step-store-")); + cleanupDirs.push(rootDir); + const queueDir = path.posix.join(rootDir, "queue"); + + const worker = await measureStartupStep( + {}, + () => 0, + "bridge.paperclip", + () => + startSandboxCallbackBridgeWorker({ + client: { + makeDir: async () => { + setupStep = getActiveStepContext(); + }, + makeDirs: async () => { + setupStep = getActiveStepContext(); + }, + listJsonFiles: async () => { + loopStep = getActiveStepContext(); + resolveFirstPoll(); + return []; + }, + readTextFile: async () => { + throw new Error("unexpected readTextFile"); + }, + writeTextFile: async () => { + throw new Error("unexpected writeTextFile"); + }, + rename: async () => { + throw new Error("unexpected rename"); + }, + remove: async () => {}, + }, + queueDir, + authorizeRequest: async () => null, + handleRequest: async () => ({ status: 200, body: "ok" }), + }), + { criticalPath: false }, + ); + + await firstPoll; + await worker.stop(); + + // The setup ran on the active step, so its exec span parents to the step. + expect(setupStep).not.toBe("unset"); + expect(setupStep).not.toBeNull(); + expect((setupStep as { criticalPath?: boolean }).criticalPath).toBe(false); + + // The loop ran outside that store, so its exec span opens unparented with no + // stale `criticalPath` flag. + expect(loopStep).toBeNull(); + }); + it("serializes remote response writes so stop does not recreate a late orphaned response", async () => { const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-response-lock-")); cleanupDirs.push(rootDir); diff --git a/packages/adapter-utils/src/sandbox-callback-bridge.ts b/packages/adapter-utils/src/sandbox-callback-bridge.ts index d990adbef3..b21d8a1874 100644 --- a/packages/adapter-utils/src/sandbox-callback-bridge.ts +++ b/packages/adapter-utils/src/sandbox-callback-bridge.ts @@ -3,6 +3,7 @@ import { promises as fs } from "node:fs"; import os from "node:os"; import path from "node:path"; +import { runWithoutActiveStep } from "./acpx-engine/startup-timing.js"; import type { CommandManagedRuntimeRunner } from "./command-managed-runtime.js"; import { preferredShellForSandbox, shellCommandArgs } from "./sandbox-shell.js"; import type { RunProcessResult } from "./server-utils.js"; @@ -745,7 +746,13 @@ export async function startSandboxCallbackBridgeWorker(input: { } }; - const loop = (async () => { + // Start the long-lived poll loop outside the measured startup-step store. + // The `makeDir` calls above are startup work and must keep the active + // `bridge.paperclip` step. The loop runs run-time execs for the whole run, + // so each loop `sandbox.exec` span must not parent to the ended step or copy + // its `criticalPath` flag. `runWithoutActiveStep` empties the store for the + // loop only; Node keeps the empty store on every later poll continuation. + const loop = runWithoutActiveStep(() => (async () => { try { while (true) { const fileNames = await input.client.listJsonFiles(directories.requestsDir); @@ -785,7 +792,7 @@ export async function startSandboxCallbackBridgeWorker(input: { settleResolve(); } } - })(); + })()); void loop;