Fence legacy sandbox dispatch against startup cancellation
Reconcile persisted cancellation with newly registered scopes, check cancellation before dispatch, and preserve external child termination signals. Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
066b4f0160
commit
1caad1bf9e
|
|
@ -425,7 +425,10 @@ Legacy sandbox cancellation stops the owned remote CLI process group or ACP
|
|||
process session before waiting for run teardown and the final file flush. The
|
||||
host sends a command-scoped cancellation marker for CLI execution; remote PIDs
|
||||
are never passed to the host process killer. Cancellation is persisted before
|
||||
stopping execution so its exit cannot admit an automatic retry. Failed stop
|
||||
stopping execution so its exit cannot admit an automatic retry. Scope registration
|
||||
rechecks durable run status, and cancellation rechecks newly registered scopes
|
||||
before acknowledgement, preventing cancelled startup work from dispatching.
|
||||
The supervisor preserves externally delivered child termination signals. Failed stop
|
||||
requests remain visible and can be retried explicitly. Local and native runner
|
||||
cancellation retain their existing authorities.
|
||||
|
||||
|
|
|
|||
|
|
@ -7,6 +7,23 @@ import { runAdapterExecutionTargetProcess } from "./execution-target.js";
|
|||
import { beginAdapterRunCancellation, cancelAdapterRunExecution, finishAdapterRunCancellation } from "./adapter-run-cancellation.js";
|
||||
|
||||
describe("sandbox CLI cancellation through the execution target", () => {
|
||||
it.each(["SIGTERM", "SIGKILL"] as const)("preserves a child's external %s termination", async (signal) => {
|
||||
const runId = randomUUID();
|
||||
beginAdapterRunCancellation(runId);
|
||||
const runner: CommandManagedRuntimeRunner = { execute: async (input) => new Promise((resolve, reject) => {
|
||||
const child = spawn(input.command === "node" ? process.execPath : input.command, input.args ?? [], { stdio: "ignore" });
|
||||
child.once("error", reject);
|
||||
child.once("close", (exitCode, signal) => resolve({ exitCode, signal, stdout: "", stderr: "", timedOut: false, pid: child.pid ?? null, startedAt: null }));
|
||||
}) };
|
||||
try {
|
||||
const result = await executeCancellableSandboxCommand(runId, runner, {
|
||||
command: process.execPath, args: ["-e", `process.kill(process.pid, '${signal}')`],
|
||||
}, 100);
|
||||
expect(result.signal).toBe(signal);
|
||||
expect(result.exitCode).toBeNull();
|
||||
} finally { finishAdapterRunCancellation(runId); }
|
||||
});
|
||||
|
||||
it("retains a failed cancellation for an explicit retry", async () => {
|
||||
const runId = randomUUID();
|
||||
beginAdapterRunCancellation(runId);
|
||||
|
|
|
|||
|
|
@ -32,11 +32,18 @@ const poll = setInterval(() => { if (cancelled()) stop(); }, 100);
|
|||
for (const name of ['SIGTERM', 'SIGINT', 'SIGHUP']) process.on(name, stop);
|
||||
const cleanup = () => { clearInterval(poll); clearTimeout(escalation); try { fs.unlinkSync(marker); } catch (e) { if (e.code !== 'ENOENT') throw e; } };
|
||||
child.once('error', () => { exited = true; cleanup(); process.exitCode = 127; });
|
||||
child.once('exit', code => {
|
||||
child.once('exit', (code, childSignal) => {
|
||||
// Do not leave a shell/tool child holding the provider's output pipe after
|
||||
// the CLI exits in response to cancellation. Never retain a PID kill timer.
|
||||
if (stopping) signal('SIGKILL');
|
||||
exited = true; cleanup(); process.exitCode = stopping ? 143 : (code ?? 128);
|
||||
exited = true; cleanup();
|
||||
if (!stopping && childSignal) {
|
||||
// Preserve the real termination signal instead of reporting exit code 128.
|
||||
process.removeAllListeners(childSignal);
|
||||
process.kill(process.pid, childSignal);
|
||||
return;
|
||||
}
|
||||
process.exitCode = stopping ? 143 : (code ?? 1);
|
||||
});
|
||||
process.once('exit', () => { if (!exited) signal('SIGKILL'); });
|
||||
`;
|
||||
|
|
|
|||
|
|
@ -6,6 +6,7 @@ import * as processes from "../services/hot-restart.js";
|
|||
import * as adapters from "../adapters/index.js";
|
||||
import * as orchestration from "../services/environment-run-orchestrator.js";
|
||||
import * as compatibility from "../services/legacy-sandbox-workspace.js";
|
||||
import * as cancellation from "@paperclipai/adapter-utils/adapter-run-cancellation";
|
||||
import { bindAdapterRunStop, hasAdapterRunCancellation } from "@paperclipai/adapter-utils/adapter-run-cancellation";
|
||||
import * as executionTargets from "@paperclipai/adapter-utils/execution-target";
|
||||
import { heartbeatService, persistHeartbeatRunProcessMetadata } from "../services/heartbeat.js";
|
||||
|
|
@ -68,7 +69,41 @@ describe("heartbeat process identity persistence", () => {
|
|||
}
|
||||
}, 30_000);
|
||||
|
||||
it("cancels the remote adapter and waits for teardown before acknowledging", async () => {
|
||||
it("does not dispatch when cancellation precedes the remote cancellation scope", async () => {
|
||||
const originalOrchestrator = orchestration.environmentRunOrchestrator;
|
||||
vi.spyOn(orchestration, "environmentRunOrchestrator").mockImplementation((...args) => {
|
||||
const actual = originalOrchestrator(...args);
|
||||
return { ...actual, realizeForRun: async (input) => ({
|
||||
...await actual.realizeForRun(input),
|
||||
executionTarget: { kind: "remote", transport: "sandbox", remoteCwd: "/remote/task", shellCommand: "sh" } as never,
|
||||
}) };
|
||||
});
|
||||
vi.spyOn(compatibility, "hasLegacySandboxWorkspace").mockReturnValue(true);
|
||||
let ready!: () => void, release!: () => void;
|
||||
const preparing = new Promise<void>((resolve) => { ready = resolve; });
|
||||
const finishPreparation = new Promise<void>((resolve) => { release = resolve; });
|
||||
vi.spyOn(executionTargets, "prepareGitHubOperationLaunchers").mockImplementation(async (input) => {
|
||||
ready(); await finishPreparation; return input.env;
|
||||
});
|
||||
vi.spyOn(executionTargets, "cleanupGitHubOperationLaunchers").mockResolvedValue(undefined);
|
||||
const execute = vi.fn(async () => ({ exitCode: 0, signal: null, timedOut: false }));
|
||||
vi.spyOn(adapters, "getServerAdapter").mockReturnValue({ supportsLocalAgentJwt: false, execute } as never);
|
||||
const heartbeat = heartbeatService(db);
|
||||
try {
|
||||
const queued = await heartbeat.invoke(agentId, "on_demand", {}, "manual");
|
||||
expect(queued).not.toBeNull();
|
||||
await preparing;
|
||||
expect(hasAdapterRunCancellation(queued!.id)).toBe(false);
|
||||
await heartbeat.cancelRun(queued!.id);
|
||||
expect((await heartbeat.getRun(queued!.id))?.status).toBe("cancelled");
|
||||
release();
|
||||
await heartbeat.drainActiveRunExecutions();
|
||||
expect(execute).not.toHaveBeenCalled();
|
||||
expect(hasAdapterRunCancellation(queued!.id)).toBe(false);
|
||||
} finally { release(); await heartbeat.drainActiveRunExecutions(); }
|
||||
}, 30_000);
|
||||
|
||||
it.each([false, true])("cancels the remote adapter and waits for teardown (scope lookup raced: %s)", async (scopeLookupRaced) => {
|
||||
const originalOrchestrator = orchestration.environmentRunOrchestrator;
|
||||
vi.spyOn(orchestration, "environmentRunOrchestrator").mockImplementation((...args) => {
|
||||
const actual = originalOrchestrator(...args);
|
||||
|
|
@ -105,6 +140,9 @@ describe("heartbeat process identity persistence", () => {
|
|||
const queued = await heartbeat.invoke(agentId, "on_demand", {}, "manual");
|
||||
expect(queued).not.toBeNull();
|
||||
await started;
|
||||
// Model the initial lookup occurring before registration. The final
|
||||
// lookup after persisting cancellation must still notify the scope.
|
||||
if (scopeLookupRaced) vi.spyOn(cancellation, "hasAdapterRunCancellation").mockReturnValueOnce(false);
|
||||
let acknowledged = false;
|
||||
pending = heartbeat.cancelRun(queued!.id).then((result) => { acknowledged = true; return result; });
|
||||
await interrupted;
|
||||
|
|
|
|||
|
|
@ -1,4 +1,4 @@
|
|||
import { beginAdapterRunCancellation, cancelAdapterRunExecution, finishAdapterRunCancellation, hasAdapterRunCancellation } from "@paperclipai/adapter-utils/adapter-run-cancellation";
|
||||
import { beginAdapterRunCancellation, cancelAdapterRunExecution, finishAdapterRunCancellation, hasAdapterRunCancellation, throwIfAdapterRunCancelled } from "@paperclipai/adapter-utils/adapter-run-cancellation";
|
||||
import { measureSandboxOperation, runWithSandboxPerformanceTrace, setSandboxPerformanceRunAttributes } from "./sandbox-performance.js";
|
||||
import { initializeRunIdentity } from "./run-identity.js";
|
||||
import { startNativeGitHubCallbackBridge } from "./native-github-bridge.js";
|
||||
|
|
@ -21151,6 +21151,11 @@ export function heartbeatService(
|
|||
if (nativeRuntimeResolution.kind === "legacy"
|
||||
&& executionTarget?.kind === "remote" && executionTarget.transport === "sandbox") {
|
||||
beginAdapterRunCancellation(run.id);
|
||||
// Register first, then read the durable status. A cancellation that
|
||||
// preceded registration must not be erased by a fresh active scope.
|
||||
if ((await getRun(run.id))?.status !== "running") {
|
||||
throw Object.assign(new Error("Run stopped before adapter dispatch"), { code: "ADAPTER_RUN_CANCELLED" });
|
||||
}
|
||||
}
|
||||
const localAgentJwtScope =
|
||||
issueRef?.workMode === "skill_test"
|
||||
|
|
@ -21671,8 +21676,9 @@ export function heartbeatService(
|
|||
}
|
||||
const guardedDispatch =
|
||||
await measureSandboxOperation("heartbeat.dispatch_resolved_interaction_continuation_with_atomic_gate", { operationIndex: 147 }, async () => (dispatchResolvedInteractionContinuationWithAtomicGate(
|
||||
(markDispatchStarted) =>
|
||||
adapter.execute({
|
||||
(markDispatchStarted) => {
|
||||
throwIfAdapterRunCancelled(run.id);
|
||||
return adapter.execute({
|
||||
runId: run.id,
|
||||
agent,
|
||||
runtime: runtimeForAdapter,
|
||||
|
|
@ -21716,7 +21722,8 @@ export function heartbeatService(
|
|||
}, executionTarget?.kind === "remote" ? "remote" : "local")));
|
||||
},
|
||||
authToken: authToken ?? undefined,
|
||||
}),
|
||||
});
|
||||
},
|
||||
)));
|
||||
if (!guardedDispatch.dispatched) return;
|
||||
adapterResult = await measureSandboxOperation("heartbeat.guarded_dispatch.result_promise", { operationIndex: 150 }, async () => (guardedDispatch.resultPromise));
|
||||
|
|
@ -26125,6 +26132,10 @@ export function heartbeatService(
|
|||
: {}),
|
||||
});
|
||||
|
||||
// Scope initialization may have raced the earlier lookup. Persist first,
|
||||
// then notify any scope now present; later scopes read this terminal state.
|
||||
if (run.runtimeMode !== "native") await cancelAdapterRunExecution(run.id);
|
||||
|
||||
await setWakeupStatus(run.wakeupRequestId, "cancelled", {
|
||||
finishedAt,
|
||||
error: reason,
|
||||
|
|
|
|||
Loading…
Reference in New Issue