diff --git a/doc/sandbox-work-folders.md b/doc/sandbox-work-folders.md index 20c3a0f7f4..fc4a57e3f0 100644 --- a/doc/sandbox-work-folders.md +++ b/doc/sandbox-work-folders.md @@ -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. diff --git a/packages/adapter-utils/src/cancellable-sandbox-command.test.ts b/packages/adapter-utils/src/cancellable-sandbox-command.test.ts index 3c39cd5448..757add3964 100644 --- a/packages/adapter-utils/src/cancellable-sandbox-command.test.ts +++ b/packages/adapter-utils/src/cancellable-sandbox-command.test.ts @@ -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); diff --git a/packages/adapter-utils/src/cancellable-sandbox-command.ts b/packages/adapter-utils/src/cancellable-sandbox-command.ts index 8eec966646..084ad2c2a3 100644 --- a/packages/adapter-utils/src/cancellable-sandbox-command.ts +++ b/packages/adapter-utils/src/cancellable-sandbox-command.ts @@ -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'); }); `; diff --git a/server/src/__tests__/heartbeat-process-metadata.test.ts b/server/src/__tests__/heartbeat-process-metadata.test.ts index 477e513c4d..0b16cfd206 100644 --- a/server/src/__tests__/heartbeat-process-metadata.test.ts +++ b/server/src/__tests__/heartbeat-process-metadata.test.ts @@ -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((resolve) => { ready = resolve; }); + const finishPreparation = new Promise((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; diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 9f88ef5d13..f89d5d4223 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -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,