mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 10:48:12 +02:00
perf(sandbox): fold dead sandbox start round trips (bridge dirs + handle-cache seed) (#10485)
## Thinking Path > - Paperclip is the control plane for AI agents. > - Sandbox startup uses bridge directories and a Daytona workspace handle. > - The cold path made repeat directory creation calls and one avoidable handle fetch. > - Those calls add delay but do not change state. > - This pull request folds the bridge directory setup into one exec, removes redundant process-session setup, and seeds the Daytona handle cache at acquire. > - The benefit is fewer deterministic host-to-sandbox round trips and faster cold starts. ## Linked Issues or Issue Description - Problem: Cold sandbox start does extra directory creation work and re-fetches a handle it already has. - Expected result: The startup path should create each directory once and reuse the fresh handle. - Related PRs I found on GitHub: #9280, #9293. ## What Changed - Added `makeDirs` to the bridge queue client and used one `mkdir -p` exec for the callback bridge directories. - Removed the two upfront `mkdir` execs for the process-session bridge stdin and events directories. - Seeded the Daytona sandbox handle cache at acquire so realize can reuse the fresh handle. - Reset the process-scoped cache in the compatibility test so the second sync run sees the expected exec count. ## Verification - `pnpm --filter @paperclipai/adapter-utils exec vitest run` - 351 passed, 4 skipped. - `pnpm --filter @paperclipai/sandbox-provider-daytona exec vitest run` - 91 passed. - `pnpm --filter @paperclipai/adapter-utils exec tsc --noEmit` - clean. - I checked `ROADMAP.md` for sandbox round-trip work. I found no duplicate planned core work for this change. ## Risks - Low risk. The change removes redundant calls and adds cache seeding. - A wrong cache scope would hide the handle. The seed now checks the lease scope and fails loudly. - The daytona package `tsc --noEmit` still depends on SDK types that are not installed in this isolated workspace. CI covers that path. ## Model Used - OpenAI GPT-5, tool-using, with code execution in the current workspace. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used with 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 linked public issues or described the issue in-PR - [x] I have not referenced internal or 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 - [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>
This commit is contained in:
1 parent
a6436126ce
commit
674d71548a
6 files changed
+259
-7
No files matched your search
@@ -194,6 +194,60 @@ describe("sandbox adapter execution targets", () => {
|
||||
});
|
||||
});
|
||||
|
||||
it("creates the process session directories only in the launch exec, not in upfront makeDir execs", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-process-session-makedir-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const childPath = path.join(rootDir, "noop-acp-child.mjs");
|
||||
await writeFile(childPath, "process.stdin.on('data', () => {});\n", "utf8");
|
||||
|
||||
const delegate = createLocalSandboxRunner();
|
||||
const execScripts: string[] = [];
|
||||
const runner = {
|
||||
execute: vi.fn(async (input: Parameters<typeof delegate.execute>[0]) => {
|
||||
execScripts.push(input.args?.[1] ?? "");
|
||||
return delegate.execute(input);
|
||||
}),
|
||||
};
|
||||
const target: AdapterSandboxExecutionTarget = {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "local-test",
|
||||
remoteCwd: rootDir,
|
||||
timeoutMs: 30_000,
|
||||
runner,
|
||||
};
|
||||
|
||||
const bridge = await startAdapterExecutionTargetProcessSessionBridge({
|
||||
runId: "run-process-session-makedir",
|
||||
target,
|
||||
runtimeRootDir: path.posix.join(rootDir, ".paperclip-runtime", "acpx"),
|
||||
adapterKey: "acpx",
|
||||
command: process.execPath,
|
||||
args: [childPath],
|
||||
cwd: rootDir,
|
||||
env: {},
|
||||
timeoutSec: 5,
|
||||
onLog: async () => {},
|
||||
});
|
||||
expect(bridge).not.toBeNull();
|
||||
|
||||
try {
|
||||
// No standalone `mkdir -p '<dir>/stdin'` or `.../events` exec runs before launch.
|
||||
const standaloneSessionDirExecs = execScripts.filter((script) =>
|
||||
/^mkdir -p '[^']*\/(stdin|events)'\s*$/.test(script),
|
||||
);
|
||||
expect(standaloneSessionDirExecs).toEqual([]);
|
||||
|
||||
// The launch exec creates both directories in one `mkdir -p` line.
|
||||
const launchExecs = execScripts.filter(
|
||||
(script) => script.includes("nohup") && /mkdir -p [^\n]*\/stdin[^\n]*\/events/.test(script),
|
||||
);
|
||||
expect(launchExecs.length).toBe(1);
|
||||
} finally {
|
||||
await bridge?.stop();
|
||||
}
|
||||
});
|
||||
|
||||
it("bridges bidirectional sandbox process sessions through a local ACPX-spawnable proxy", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-process-session-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
|
||||
@@ -1378,8 +1378,9 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: {
|
||||
shellCommand,
|
||||
});
|
||||
|
||||
await client.makeDir(stdinDir);
|
||||
await client.makeDir(eventsDir);
|
||||
// The launch exec below re-creates stdinDir and eventsDir with one `mkdir -p`,
|
||||
// and the remote script also creates them on start. No reader touches the two
|
||||
// directories before the launch exec runs, so upfront makeDir execs are redundant.
|
||||
await syncProcessSessionRemoteScript({
|
||||
runner,
|
||||
remoteCwd: target.remoteCwd,
|
||||
|
||||
@@ -438,6 +438,7 @@ describe("sandbox callback bridge", () => {
|
||||
const worker = await startSandboxCallbackBridgeWorker({
|
||||
client: {
|
||||
makeDir: async () => {},
|
||||
makeDirs: async () => {},
|
||||
listJsonFiles: async () => {
|
||||
throw new Error(
|
||||
"list /remote/.paperclip-runtime/gemini/paperclip-bridge/queue/requests failed with exit code 255: kex_exchange_identification: read: Connection reset by peer",
|
||||
@@ -1114,4 +1115,110 @@ describe("sandbox callback bridge", () => {
|
||||
},
|
||||
}));
|
||||
});
|
||||
|
||||
it("creates the bridge queue directories in one directory-creation exec", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-makedirs-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
|
||||
const queueDir = path.posix.join(rootDir, "queue");
|
||||
const directories = sandboxCallbackBridgeDirectories(queueDir);
|
||||
const makeDir = vi.fn(async () => {});
|
||||
const makeDirs = vi.fn(async () => {});
|
||||
|
||||
const worker = await startSandboxCallbackBridgeWorker({
|
||||
client: {
|
||||
makeDir,
|
||||
makeDirs,
|
||||
listJsonFiles: async () => [],
|
||||
readTextFile: async () => {
|
||||
throw new Error("unexpected readTextFile");
|
||||
},
|
||||
writeTextFile: async () => {},
|
||||
rename: async () => {},
|
||||
remove: async () => {},
|
||||
},
|
||||
queueDir,
|
||||
authorizeRequest: async () => null,
|
||||
handleRequest: async () => ({ status: 200, body: "ok" }),
|
||||
});
|
||||
|
||||
await worker.stop();
|
||||
|
||||
expect(makeDir).not.toHaveBeenCalled();
|
||||
expect(makeDirs).toHaveBeenCalledTimes(1);
|
||||
expect(makeDirs).toHaveBeenCalledWith([
|
||||
directories.rootDir,
|
||||
directories.requestsDir,
|
||||
directories.responsesDir,
|
||||
directories.logsDir,
|
||||
]);
|
||||
});
|
||||
|
||||
it("falls back to sequential makeDir when the queue client omits makeDirs", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-bridge-makedir-fallback-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
|
||||
const queueDir = path.posix.join(rootDir, "queue");
|
||||
const directories = sandboxCallbackBridgeDirectories(queueDir);
|
||||
const makeDir = vi.fn(async (_remotePath: string) => {});
|
||||
|
||||
// A queue client that predates the batched makeDirs method. The worker
|
||||
// must still create every queue directory through sequential makeDir.
|
||||
const worker = await startSandboxCallbackBridgeWorker({
|
||||
client: {
|
||||
makeDir,
|
||||
listJsonFiles: async () => [],
|
||||
readTextFile: async () => {
|
||||
throw new Error("unexpected readTextFile");
|
||||
},
|
||||
writeTextFile: async () => {},
|
||||
rename: async () => {},
|
||||
remove: async () => {},
|
||||
},
|
||||
queueDir,
|
||||
authorizeRequest: async () => null,
|
||||
handleRequest: async () => ({ status: 200, body: "ok" }),
|
||||
});
|
||||
|
||||
await worker.stop();
|
||||
|
||||
expect(makeDir.mock.calls.map((call) => call[0])).toEqual([
|
||||
directories.rootDir,
|
||||
directories.requestsDir,
|
||||
directories.responsesDir,
|
||||
directories.logsDir,
|
||||
]);
|
||||
});
|
||||
|
||||
it("runs one mkdir -p exec for makeDirs on the command-managed queue client", async () => {
|
||||
const runner = {
|
||||
execute: vi.fn(async (_input: { args?: string[] }) => ({
|
||||
exitCode: 0,
|
||||
signal: null,
|
||||
timedOut: false,
|
||||
stdout: "",
|
||||
stderr: "",
|
||||
pid: null,
|
||||
startedAt: new Date().toISOString(),
|
||||
})),
|
||||
};
|
||||
|
||||
const client = createCommandManagedSandboxCallbackBridgeQueueClient({
|
||||
runner,
|
||||
remoteCwd: "/workspace",
|
||||
timeoutMs: 30_000,
|
||||
});
|
||||
|
||||
// The command-managed client always provides the batched makeDirs method.
|
||||
expect(client.makeDirs).toBeDefined();
|
||||
await client.makeDirs?.(["/workspace/a", "/workspace/b", "/workspace/c"]);
|
||||
|
||||
expect(runner.execute).toHaveBeenCalledTimes(1);
|
||||
const call = runner.execute.mock.calls[0][0];
|
||||
const script = call.args?.[call.args.length - 1] ?? "";
|
||||
expect(script).toContain("mkdir -p");
|
||||
expect(script).toContain("/workspace/a");
|
||||
expect(script).toContain("/workspace/b");
|
||||
expect(script).toContain("/workspace/c");
|
||||
});
|
||||
});
|
||||
@@ -149,6 +149,11 @@ export interface SandboxCallbackBridgeDirectories {
|
||||
|
||||
export interface SandboxCallbackBridgeQueueClient {
|
||||
makeDir(remotePath: string): Promise<void>;
|
||||
// Optional batched directory create. The built-in clients create every
|
||||
// queue directory in one remote exec. A client that predates this method
|
||||
// omits it; the worker falls back to sequential `makeDir` calls, so an
|
||||
// external implementation stays compatible without a change.
|
||||
makeDirs?(remotePaths: string[]): Promise<void>;
|
||||
listJsonFiles(remotePath: string): Promise<string[]>;
|
||||
readTextFile(remotePath: string): Promise<string>;
|
||||
writeTextFile(remotePath: string, body: string): Promise<void>;
|
||||
@@ -359,6 +364,11 @@ export function createFileSystemSandboxCallbackBridgeQueueClient(): SandboxCallb
|
||||
makeDir: async (remotePath) => {
|
||||
await fs.mkdir(remotePath, { recursive: true });
|
||||
},
|
||||
makeDirs: async (remotePaths) => {
|
||||
for (const remotePath of remotePaths) {
|
||||
await fs.mkdir(remotePath, { recursive: true });
|
||||
}
|
||||
},
|
||||
listJsonFiles: async (remotePath) => {
|
||||
const entries = await fs.readdir(remotePath, { withFileTypes: true }).catch(() => []);
|
||||
return entries
|
||||
@@ -470,6 +480,13 @@ export function createCommandManagedSandboxCallbackBridgeQueueClient(input: {
|
||||
makeDir: async (remotePath) => {
|
||||
await runChecked(`mkdir ${remotePath}`, `mkdir -p ${shellQuote(remotePath)}`);
|
||||
},
|
||||
makeDirs: async (remotePaths) => {
|
||||
if (remotePaths.length === 0) {
|
||||
return;
|
||||
}
|
||||
const quoted = remotePaths.map((remotePath) => shellQuote(remotePath));
|
||||
await runChecked(`mkdir ${remotePaths.join(" ")}`, `mkdir -p ${quoted.join(" ")}`);
|
||||
},
|
||||
listJsonFiles: async (remotePath) => {
|
||||
const result = await runShell(
|
||||
input.runner,
|
||||
@@ -606,10 +623,21 @@ export async function startSandboxCallbackBridgeWorker(input: {
|
||||
const pollIntervalMs = normalizeTimeoutMs(input.pollIntervalMs, DEFAULT_BRIDGE_POLL_INTERVAL_MS);
|
||||
const maxBodyBytes = normalizeTimeoutMs(input.maxBodyBytes, DEFAULT_BRIDGE_MAX_BODY_BYTES);
|
||||
const directories = sandboxCallbackBridgeDirectories(input.queueDir);
|
||||
await input.client.makeDir(directories.rootDir);
|
||||
await input.client.makeDir(directories.requestsDir);
|
||||
await input.client.makeDir(directories.responsesDir);
|
||||
await input.client.makeDir(directories.logsDir);
|
||||
const queueDirectories = [
|
||||
directories.rootDir,
|
||||
directories.requestsDir,
|
||||
directories.responsesDir,
|
||||
directories.logsDir,
|
||||
];
|
||||
if (input.client.makeDirs) {
|
||||
await input.client.makeDirs(queueDirectories);
|
||||
} else {
|
||||
// Backward-compatible fallback for a queue client that omits the batched
|
||||
// makeDirs method. Create each queue directory with a single makeDir.
|
||||
for (const directory of queueDirectories) {
|
||||
await input.client.makeDir(directory);
|
||||
}
|
||||
}
|
||||
|
||||
let stopping = false;
|
||||
let inFlight = 0;
|
||||
|
||||
@@ -2136,6 +2136,35 @@ describe("Daytona sandbox provider plugin", () => {
|
||||
expect(mockGet).toHaveBeenCalledTimes(2);
|
||||
expect(second.process.executeCommand).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("realizes the workspace from the acquire-seeded handle without a client.get", async () => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
const sandbox = createMockSandbox({ id: "sandbox-seed" });
|
||||
mockCreate.mockResolvedValue(sandbox);
|
||||
|
||||
const base = { driverKey: "daytona", companyId: "company-1", environmentId: "env-1" };
|
||||
const config = { image: "node:20", timeoutMs: 300000, reuseLease: false };
|
||||
|
||||
const lease = await plugin.definition.onEnvironmentAcquireLease?.({
|
||||
...base,
|
||||
runId: "run-1",
|
||||
config,
|
||||
});
|
||||
expect(lease?.providerLeaseId).toBe("sandbox-seed");
|
||||
|
||||
const realize = await plugin.definition.onEnvironmentRealizeWorkspace?.({
|
||||
...base,
|
||||
lease: { providerLeaseId: lease!.providerLeaseId, metadata: lease!.metadata },
|
||||
workspace: { remotePath: "/home/daytona/paperclip-workspace" },
|
||||
config,
|
||||
});
|
||||
|
||||
// Acquire seeded the handle under the exact scope realize reads, so realize
|
||||
// reuses it and never pays a real REST re-fetch.
|
||||
expect(mockGet).not.toHaveBeenCalled();
|
||||
expect(sandbox.fs.createFolder).toHaveBeenCalledWith("/home/daytona/paperclip-workspace", "755");
|
||||
expect(realize?.cwd).toBe("/home/daytona/paperclip-workspace");
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -3017,6 +3046,11 @@ describe("daytona native file-sync hooks", () => {
|
||||
|
||||
// Same operation, now with an (empty) postUploadCommands array — must be
|
||||
// byte-identical: an absent/empty command list adds zero execs.
|
||||
// Reset the process-scoped handle cache so the second operation fetches its
|
||||
// own `withEmpty` handle. Both operations reuse the same providerLeaseId, so
|
||||
// without this reset the cache serves the first `baseline` handle again and
|
||||
// `withEmpty` records zero execs.
|
||||
__resetDaytonaSandboxHandleCacheForTest();
|
||||
const withEmpty = createMockSandbox();
|
||||
mockGet.mockResolvedValue(withEmpty);
|
||||
await plugin.definition.onEnvironmentSyncIn?.({
|
||||
|
||||
@@ -1070,6 +1070,21 @@ const sandboxHandleCache = (() => {
|
||||
}
|
||||
}
|
||||
|
||||
// Seed the cache with a handle the caller already holds (e.g. the fresh handle
|
||||
// from `createSandbox` on a cold acquire), so the next `get` under the same
|
||||
// scope reuses it instead of paying a real `client.get`. The seed must land
|
||||
// under the exact composite key the reader uses, or the reader misses and the
|
||||
// saved round trip is lost. Assert the handle belongs to the lease so a caller
|
||||
// that builds a wrong scope fails loudly here instead of caching a foreign
|
||||
// handle.
|
||||
function seed(scope: SandboxScope, sandbox: Sandbox): void {
|
||||
assertHandleMatchesLease(sandbox, scope.providerLeaseId);
|
||||
entries.set(sandboxHandleCacheKey(scope), {
|
||||
sandbox: Promise.resolve(sandbox),
|
||||
verifiedAtMs: handleFreshnessNow(),
|
||||
});
|
||||
}
|
||||
|
||||
function clear(scope: SandboxScope): void {
|
||||
entries.delete(sandboxHandleCacheKey(scope));
|
||||
}
|
||||
@@ -1078,7 +1093,7 @@ const sandboxHandleCache = (() => {
|
||||
entries.clear();
|
||||
}
|
||||
|
||||
return { get, clear, reset, markFresh };
|
||||
return { get, seed, clear, reset, markFresh };
|
||||
})();
|
||||
|
||||
/**
|
||||
@@ -1331,6 +1346,19 @@ const plugin = definePlugin({
|
||||
providerLeaseId: sandbox.id,
|
||||
config,
|
||||
});
|
||||
// Seed the handle cache with the fresh handle under the exact scope that
|
||||
// `onEnvironmentRealizeWorkspace` reads (providerLeaseId === sandbox.id).
|
||||
// Realize then reuses this handle instead of paying a real `client.get`.
|
||||
sandboxHandleCache.seed(
|
||||
{
|
||||
driverKey: params.driverKey,
|
||||
companyId: params.companyId,
|
||||
environmentId: params.environmentId,
|
||||
providerLeaseId: sandbox.id,
|
||||
config,
|
||||
},
|
||||
sandbox,
|
||||
);
|
||||
return {
|
||||
providerLeaseId: sandbox.id,
|
||||
metadata: leaseMetadata({
|
||||
|
||||
Reference in new issue
Block a user