mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-26 18:31:34 +01:00
fix(runner): wait for remote process containment before reuse
Await the ownership-fenced remote termination monitor before session close resolves. Bound EC2 verification checks to retain failure evidence before fleet shutdown. Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
e211fd3beb
commit
648e056332
3 files changed
+45
-6
No files matched your search
@@ -167,6 +167,8 @@ jobs:
|
||||
echo "Image: $immutable" >> "$GITHUB_STEP_SUMMARY"
|
||||
- name: Verify repository on EC2
|
||||
if: inputs.verify_source
|
||||
# Leave time for artifact retention before the fleet's one-hour lifetime.
|
||||
timeout-minutes: 38
|
||||
run: |
|
||||
set -uo pipefail
|
||||
pnpm install --frozen-lockfile --ignore-scripts
|
||||
@@ -174,8 +176,11 @@ jobs:
|
||||
for check in 'pnpm -r typecheck' 'pnpm test:run' 'pnpm check:token-gates' 'pnpm test:e2e:runner:typecheck' 'pnpm test:e2e:runner:unit' 'pnpm build'; do
|
||||
label="$(echo "$check" | tr -cs 'a-zA-Z0-9' '-')"
|
||||
echo "Starting $check"
|
||||
bash -c "$check" > "remote-verification/$label.log" 2>&1 || status=1
|
||||
echo "Finished $check"
|
||||
check_status=0
|
||||
timeout --signal=TERM --kill-after=15s 15m bash -c "$check" > "remote-verification/$label.log" 2>&1 || check_status=$?
|
||||
printf '%s\t%s\n' "$check_status" "$check" >> remote-verification/check-status.tsv
|
||||
if [ "$check_status" -ne 0 ]; then status=1; fi
|
||||
echo "Finished $check (exit $check_status)"
|
||||
done
|
||||
exit "$status"
|
||||
- name: Build canonical eval report viewer
|
||||
|
||||
@@ -1089,6 +1089,30 @@ it("quiesces the control route before checkpoint and containment regardless of p
|
||||
expect(failedCheckpointSteps).toEqual(["release", "checkpoint", "kill"]);
|
||||
});
|
||||
|
||||
it("does not release a session before asynchronous process containment settles", async () => {
|
||||
let finishKill!: () => void;
|
||||
const killed = new Promise<void>((resolve) => { finishKill = resolve; });
|
||||
const steps: string[] = [];
|
||||
const released = runnerdRecoveryInternals.releaseRunnerProcessOwnership({
|
||||
runnerSettled: false,
|
||||
release: () => { steps.push("route-closed"); },
|
||||
checkpoint: () => { steps.push("checkpoint"); },
|
||||
forceKill: async () => { steps.push("kill-dispatched"); await killed; steps.push("process-exited"); },
|
||||
}).then(() => { steps.push("session-reusable"); });
|
||||
await new Promise<void>(resolve => setImmediate(resolve));
|
||||
expect(steps).toEqual(["route-closed", "checkpoint", "kill-dispatched"]);
|
||||
finishKill();
|
||||
await released;
|
||||
expect(steps).toEqual(["route-closed", "checkpoint", "kill-dispatched", "process-exited", "session-reusable"]);
|
||||
});
|
||||
|
||||
it("refuses session reuse when asynchronous process containment fails", async () => {
|
||||
await expect(runnerdRecoveryInternals.releaseRunnerProcessOwnership({
|
||||
runnerSettled: true, release: null, checkpoint: null,
|
||||
forceKill: async () => { throw new Error("remote process still alive"); },
|
||||
})).rejects.toThrow("remote process still alive");
|
||||
});
|
||||
|
||||
it("waits for the exact durable suspension command behind prior close work", async () => {
|
||||
const commands = [
|
||||
{
|
||||
|
||||
@@ -678,7 +678,7 @@ async function releaseRunnerProcessOwnership(input: {
|
||||
runnerSettled: boolean;
|
||||
checkpoint:
|
||||
((settlement: "settled" | "unsettled") => Promise<void> | void) | null;
|
||||
forceKill: () => void;
|
||||
forceKill: () => Promise<void> | void;
|
||||
release: (() => Promise<void> | void) | null;
|
||||
}): Promise<void> {
|
||||
let releaseFailure: unknown;
|
||||
@@ -701,7 +701,7 @@ async function releaseRunnerProcessOwnership(input: {
|
||||
} catch (error) {
|
||||
checkpointFailure = error;
|
||||
} finally {
|
||||
input.forceKill();
|
||||
await input.forceKill();
|
||||
}
|
||||
if (checkpointFailure !== undefined) throw checkpointFailure;
|
||||
if (releaseFailure !== undefined) throw releaseFailure;
|
||||
@@ -4269,8 +4269,18 @@ class DurablePrpCodexTransport implements CodexAppServerTransport {
|
||||
adoptedRunner && !this.#adoptedRunnerAuthenticated
|
||||
? null
|
||||
: this.#controlPlaneCheckpoint,
|
||||
forceKill: () => {
|
||||
this.#handle?.child.kill("SIGKILL");
|
||||
forceKill: async () => {
|
||||
const handle = this.#handle;
|
||||
if (!handle || this.#evidence.runnerExited) return;
|
||||
handle.child.kill("SIGKILL");
|
||||
// A remote kill dispatches an asynchronous, ownership-fenced RPC.
|
||||
// Do not release the session for reuse until its process monitor has
|
||||
// settled: the successor would otherwise overwrite that ownership
|
||||
// marker while the previous runner still holds the fixed listener.
|
||||
const result = await waitForProcess(handle, 15_000);
|
||||
this.#evidence.runnerExited = true;
|
||||
this.#evidence.runnerExitCode = result.code;
|
||||
this.#evidence.runnerSignal = result.signal as NodeJS.Signals | null;
|
||||
},
|
||||
release: this.#controlPlaneRelease,
|
||||
});
|
||||
|
||||
Reference in new issue
Block a user