mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-25 17:45:53 +01:00
fix(adapter-utils): bound the lease refresh exec to its own short timeout
The lease refresh reused the run's own adapter execution timeout, which can run up to the 4-hour sandbox default (or be unset entirely on other targets). stop() awaits the whole refresh chain before it removes sessionDir, so a stalled refresh could make stop() wait for that same budget instead of a few seconds. The refresh now carries its own fixed, short exec timeout, independent of the run's timeout. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
2 files changed
+87
-1
No files matched your search
@@ -2319,4 +2319,80 @@ describe("deterministic remote process-session wrapper shutdown (PAP-5316)", ()
|
||||
}),
|
||||
]);
|
||||
}, 30_000);
|
||||
|
||||
it("T31 the lease refresh exec carries its own short timeout, never the run's adapter timeout", async () => {
|
||||
const rootDir = await mkdtemp(path.join(os.tmpdir(), "paperclip-lease-refresh-timeout-"));
|
||||
cleanupDirs.push(rootDir);
|
||||
const childPath = path.join(rootDir, "quiet-child.mjs");
|
||||
await writeFile(childPath, "process.stdin.resume();\n", "utf8");
|
||||
|
||||
const refreshTimeoutsSeen: Array<number | undefined> = [];
|
||||
let counter = 0;
|
||||
const runner = {
|
||||
execute: async (input: {
|
||||
command: string;
|
||||
args?: string[];
|
||||
cwd?: string;
|
||||
env?: Record<string, string>;
|
||||
stdin?: string;
|
||||
timeoutMs?: number;
|
||||
onLog?: (stream: "stdout" | "stderr", chunk: string) => Promise<void>;
|
||||
}): Promise<RunProcessResult> => {
|
||||
const script = input.args?.[1] ?? "";
|
||||
if (script.includes("nohup node")) {
|
||||
return { exitCode: 0, signal: null, timedOut: false, stdout: "", stderr: "", pid: null, startedAt: null };
|
||||
}
|
||||
if (script.includes("set -C") && script.includes(LEASE_FILE_NAME)) {
|
||||
refreshTimeoutsSeen.push(input.timeoutMs);
|
||||
}
|
||||
counter += 1;
|
||||
const command =
|
||||
input.command === "bash" ? "/bin/bash" : input.command === "sh" ? "/bin/sh" : input.command;
|
||||
return runChildProcess(`lease-refresh-timeout-run-${counter}`, command, input.args ?? [], {
|
||||
cwd: input.cwd ?? process.cwd(),
|
||||
env: input.env ?? {},
|
||||
stdin: input.stdin,
|
||||
timeoutSec: Math.max(1, Math.ceil((input.timeoutMs ?? 30_000) / 1000)),
|
||||
graceSec: 5,
|
||||
onLog: input.onLog ?? (async () => {}),
|
||||
});
|
||||
},
|
||||
};
|
||||
// Mirrors DEFAULT_REMOTE_SANDBOX_ADAPTER_TIMEOUT_SEC (the 4-hour sandbox
|
||||
// adapter execution timeout). No `timeoutSec` is passed to the bridge
|
||||
// below, so this target value alone becomes the run's own `timeoutMs`.
|
||||
const FOUR_HOUR_RUN_TIMEOUT_MS = 4 * 60 * 60 * 1000;
|
||||
const target: AdapterSandboxExecutionTarget = {
|
||||
kind: "remote",
|
||||
transport: "sandbox",
|
||||
providerKey: "local-test",
|
||||
remoteCwd: rootDir,
|
||||
timeoutMs: FOUR_HOUR_RUN_TIMEOUT_MS,
|
||||
runner,
|
||||
};
|
||||
|
||||
const bridge = await startAdapterExecutionTargetProcessSessionBridge({
|
||||
runId: "run-lease-refresh-timeout",
|
||||
target,
|
||||
runtimeRootDir: path.posix.join(rootDir, ".paperclip-runtime", "acpx"),
|
||||
adapterKey: "acpx",
|
||||
command: process.execPath,
|
||||
args: [childPath],
|
||||
cwd: rootDir,
|
||||
env: {},
|
||||
onLog: async () => {},
|
||||
});
|
||||
expect(bridge).not.toBeNull();
|
||||
await waitFor(() => refreshTimeoutsSeen.length > 0, 4_000);
|
||||
await bridge!.stop();
|
||||
|
||||
// The run's own adapter timeout must never reach the refresh exec: a
|
||||
// stalled refresh bounded by it could make stop() wait up to 4 hours.
|
||||
expect(refreshTimeoutsSeen.length).toBeGreaterThan(0);
|
||||
for (const seen of refreshTimeoutsSeen) {
|
||||
expect(seen).toBeDefined();
|
||||
expect(seen).toBeLessThan(FOUR_HOUR_RUN_TIMEOUT_MS);
|
||||
expect(seen).toBeLessThanOrEqual(10_000);
|
||||
}
|
||||
}, 10_000);
|
||||
});
|
||||
@@ -1612,6 +1612,14 @@ const DEFAULT_PROCESS_SESSION_SHUTDOWN_WAIT_MS = 3_000;
|
||||
// this adds no new sandbox-to-host data flow.
|
||||
const DEFAULT_PROCESS_SESSION_LEASE_REFRESH_MS = 60_000;
|
||||
const DEFAULT_PROCESS_SESSION_LEASE_TTL_MS = 900_000;
|
||||
// The lease refresh runs one small control-plane command (`set -C` + `mv`),
|
||||
// never the long-lived agent process, so it gets its own short exec budget
|
||||
// instead of the run's adapter execution timeout. That run timeout can run
|
||||
// up to the 4-hour sandbox default, or be unset (no bound at all) on other
|
||||
// targets, and `stop()` awaits the whole refresh chain before it removes
|
||||
// `sessionDir` — so a stalled refresh reusing that timeout could make
|
||||
// `stop()` wait for the run's own budget instead of a few seconds.
|
||||
const PROCESS_SESSION_LEASE_REFRESH_EXEC_TIMEOUT_MS = 10_000;
|
||||
// The name does not end in ".json", so no `.json`-only reader (the stdin
|
||||
// poller) ever lists it.
|
||||
const PROCESS_SESSION_LEASE_FILE_NAME = ".paperclip-session-lease";
|
||||
@@ -1914,7 +1922,9 @@ export async function startAdapterExecutionTargetProcessSessionBridge(input: {
|
||||
args: shellCommandArgs(script),
|
||||
cwd: target.remoteCwd,
|
||||
env: { PAPERCLIP_SANDBOX_EXEC_CHANNEL: "bridge" },
|
||||
timeoutMs,
|
||||
// Bounded independently of the run's own `timeoutMs` -- see
|
||||
// `PROCESS_SESSION_LEASE_REFRESH_EXEC_TIMEOUT_MS`.
|
||||
timeoutMs: PROCESS_SESSION_LEASE_REFRESH_EXEC_TIMEOUT_MS,
|
||||
// A busy persistent session must never queue this behind an
|
||||
// in-run session command and starve the lease.
|
||||
bypassSession: true,
|
||||
|
||||
Reference in new issue
Block a user