Skip to content

Commit 04546c8

Browse files
fix(runner): reconnect Daytona sessions after controller restart (#13691)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - The native runner can execute a task inside a Daytona sandbox. > - The sandbox can keep running when the Paperclip controller restarts. > - Recovery treated sandbox process IDs as local process IDs and selected the wrong recovery path. > - Live verification also found races between startup, shutdown, and queued task cleanup. > - This pull request verifies the existing remote owner and orders those transitions. > - Users can continue the same task and provider session after a controller restart. ## Linked Issues or Issue Description **What happened?** The Daytona `recover-controller` cases failed with `runner_state_identity_mismatch`. Remote process IDs can be absent on the controller or collide with unrelated local processes. Recovery then looked for remote state in the local runner directory. Later turns could also start before the previous executor released its sandbox resources. **Expected behavior** Reconnect to the original sandbox and authenticated runner. Preserve the task, provider session, and queued comments. Reject a replacement sandbox or mismatched identity. Do not start another provider during reattachment. **Steps to reproduce** Run the `everyday-workflows` `recover-controller` case for `runner-codex` or `runner-acpx-claude` in Daytona. The browser creates a Python tool, requests a revision, restarts the controller during execution, and queues another revision. It then downloads and tests the final ZIP. Related: #13682 is the preceding operational fix. #13291 addresses legacy sandbox conversation recovery, a different execution path. #13666 includes broader run-capacity work; this change guards cleanup of an existing native task executor. ## What Changed - Add remote runner recovery without interpreting sandbox PIDs on the controller. - Verify the original provider lease, remote workspace, durable state, process marker, and authenticated PRP authority before adoption. - Compare the process marker with live Linux boot identity and start ticks to reject PID reuse. Read virtual proc files through the guaranteed Node runtime; unavailable proof blocks adoption without blocking a fresh launch. - Make the E2E supervisor own the actual server process so forced restart cannot leave a late database closer behind. - Scope the chat delivery lease test to its own fixture instead of draining other tests’ pending deliveries. - Preserve provider-attempt counts and recorded evidence during reattachment. - Serialize an idle-session checkpoint with admission of the next native turn. - Wait for an in-progress startup to acknowledge restart detachment. Fail after a bounded deadline if it cannot. - Keep a queued comment waiting until the previous native task executor releases its resources. Allow unrelated tasks to continue. - Update the Daytona image's resolved lock digest to match current dependency manifests. - Add classifier, ownership, process, startup, checkpoint, and queued-admission regression tests. Document recovery behavior. ## Verification - 415 focused tests passed across native execution, restart recovery, workspace synchronization, queued admission, and real-process restart tests. The final Node-based fingerprint change passed all 375 native-session tests. - Runner harness unit tests: 394 passed. Chat integration shard 2: 335 passed after fixture isolation. - The exact fingerprint command succeeded twice in a disposable Daytona sandbox and returned the same identity; the sandbox was deleted. - 11 real-process restart integration tests passed, including absent and colliding remote PIDs. - Repository typecheck and final build passed. Broad local checks found machine-dependent database startup and timing failures; focused retries passed. The final-revision PR pipeline is green. One unrelated browser shard hit a five-second blank-page timeout on the first run and passed its targeted retry. - Final-revision local headed browser E2E: `everyday-workflows.runner-acpx-claude.daytona.recover-controller` passed on attempt 1 in 4.7 minutes, **40/40 checks**. Manual browser inspection confirmed Done, all three ZIPs, and delivery of the queued follow-up. All three runs succeeded using the same provider session. The harness downloaded and independently tested the final artifact. - Final-revision Daytona campaign: https://github.com/paperclipai/paperclip/actions/runs/35463999611 — **Codex passed first attempt (4.8 minutes); ACPX Claude passed first attempt (6.1 minutes)**. Campaign aggregation/publication is finishing; both test jobs succeeded. - Greptile reviewed `beb08d8493b3286f5bb988dead369ff8c96a395d`: **5/5**, no open findings. - Staging browser verification is pending selection of a disposable staging instance and removal of a Chrome extension UI block. ## Risks - Recovery now depends on the original sandbox remaining available. A replacement or mismatched identity still blocks adoption. - Shutdown waits up to 30 seconds for a native startup to reach a safe detach point. An unfinished startup returns a clear failure instead of a false detach receipt. - Queued native work on the same task waits for cleanup. Unrelated tasks remain eligible. - The image digest update rebuilds the Daytona runtime image. No database migration or public API change is included. ## Model Used OpenAI Codex, GPT-6, with repository inspection, code execution, and browser tools. The runtime does not expose the exact deployed model ID or context-window size. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and 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 (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [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>
1 parent aeef493 commit 04546c8

16 files changed

Lines changed: 995 additions & 55 deletions

doc/DEVELOPING.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1087,6 +1087,20 @@ that classification finishes.
10871087
- A verified live runner re-registers its existing PRP authority and reconnects
10881088
with the same operating-system PID. Paperclip does not spawn a competing
10891089
runner.
1090+
- For a running sandbox session, recovery checks the original provider lease,
1091+
remote workspace, durable runner identity, and process marker inside that
1092+
sandbox. The process marker must include the Linux boot ID and start ticks;
1093+
recovery compares them with the live process before adoption and signaling.
1094+
Collection uses the required Node runtime and is optional for fresh launches. Older
1095+
markers or images without that proof remain blocked for recovery. Remote
1096+
PIDs are never interpreted as controller-local PIDs. The runner must
1097+
authenticate to its existing PRP authority; reconnection neither
1098+
launches another provider nor consumes a provider retry. A replacement
1099+
sandbox or mismatched identity blocks adoption without overwriting evidence.
1100+
- Shutdown waits up to 30 seconds for an in-progress native startup to reach
1101+
its detach acknowledgement. It reports a startup deadline failure instead
1102+
of claiming that an unfinished bootstrap detached safely. A queued turn
1103+
waits for the previous executor to finish releasing its task resources.
10901104
- A verified dead runner starts a replacement from the same durable root and
10911105
resumes the same provider checkpoint. Only the operating-system PID changes.
10921106
- A runner that died before its first authenticated connection can restart on

docker/daytona-runner/Dockerfile

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ COPY cli/package.json ./cli/package.json
2828
# The complete resolved lock (including transitive integrity hashes) is reviewed.
2929
# Reject registry-time drift BEFORE installing packages or running lifecycle code.
3030
# Refresh this digest together with source/provider dependency changes.
31-
ARG PAPERCLIP_RUNNER_LOCK_SHA256=133cc415964e4ee3f348251c315d560cc8f8f51a8dddde2da84e065ba4ae3fa6
31+
ARG PAPERCLIP_RUNNER_LOCK_SHA256=d7d96cf0d98cf0946f6195e29ba173b03711a947a1c38f312b67cda56c254c22
3232
RUN pnpm install --resolution-only --ignore-scripts --no-frozen-lockfile \
3333
&& printf '%s pnpm-lock.yaml\n' "${PAPERCLIP_RUNNER_LOCK_SHA256}" > /tmp/provider-lock.sha256 \
3434
&& sha256sum -c /tmp/provider-lock.sha256 \

server/src/__tests__/chat-channels.integration.test.ts

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14335,7 +14335,18 @@ describeEmbeddedPostgres("chat channel control-plane integration", () => {
1433514335
.set({ nextAttemptAt: null })
1433614336
.where(eq(chatDeliveries.endpointId, endpoint.id));
1433714337

14338-
const processing = service.processPendingDeliveries();
14338+
// This suite shares its database. Drain only this fixture's conversation;
14339+
// a global recovery pass can renew unrelated retained deliveries with this
14340+
// test's failure hook and make its ownership assertion depend on test order.
14341+
const [firstDelivery] = await db
14342+
.select({ id: chatDeliveries.id })
14343+
.from(chatDeliveries)
14344+
.where(and(
14345+
eq(chatDeliveries.endpointId, endpoint.id),
14346+
eq(chatDeliveries.providerEventId, `${thread.thread.id}:${first.id}`),
14347+
));
14348+
if (!firstDelivery) throw new Error("Expected first delivery");
14349+
const processing = service.processPendingDeliveries(25, firstDelivery.id);
1433914350
await firstDeliveryEntered;
1434014351
await renewalAttempted;
1434114352
releaseFirstDelivery();
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { randomUUID } from "node:crypto";
2+
import { eq } from "drizzle-orm";
3+
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
4+
import { agents, companies, createDb, heartbeatRuns, issues } from "@paperclipai/db";
5+
import { getEmbeddedPostgresTestSupport, startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
6+
import { heartbeatService } from "../services/heartbeat.js";
7+
8+
const execute = vi.hoisted(() => vi.fn());
9+
vi.mock("../adapters/index.js", async () => {
10+
const actual = await vi.importActual<typeof import("../adapters/index.js")>("../adapters/index.js");
11+
return { ...actual, getServerAdapter: vi.fn(() => ({ supportsLocalAgentJwt: false, execute })) };
12+
});
13+
const support = await getEmbeddedPostgresTestSupport();
14+
const suite = support.supported ? describe : describe.skip;
15+
16+
suite("native terminal-run cleanup admission", () => {
17+
let db: ReturnType<typeof createDb>;
18+
let temporary: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
19+
beforeAll(async () => {
20+
temporary = await startEmbeddedPostgresTestDatabase("native-cleanup-admission-");
21+
db = createDb(temporary.connectionString);
22+
}, 60_000);
23+
afterAll(async () => {
24+
await db?.$client.end({ timeout: 0 });
25+
await temporary?.cleanup();
26+
});
27+
28+
it("holds the same task until its native executor settles while allowing unrelated tasks", async () => {
29+
const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID(), otherIssueId = randomUUID();
30+
await db.insert(companies).values({ id: companyId, name: "Cleanup admission", issuePrefix: "CLN", defaultResponsibleUserId: "owner" });
31+
await db.insert(agents).values({ id: agentId, companyId, name: "Worker", role: "engineer", status: "idle", adapterType: "process",
32+
adapterConfig: {}, permissions: {}, runtimeConfig: { heartbeat: { enabled: false, wakeOnDemand: true, maxConcurrentRuns: 20 } } });
33+
await db.insert(issues).values([issueId, otherIssueId].map(id => ({ id, companyId, title: "Recovery task", status: "todo", assigneeAgentId: agentId, responsibleUserId: "owner" })));
34+
let release!: () => void, entered!: () => void;
35+
const cleanupGate = new Promise<void>(resolve => { release = resolve; });
36+
const terminal = new Promise<void>(resolve => { entered = resolve; });
37+
let first = true;
38+
execute.mockImplementation(async (context: { runId: string; context: { issueId: string } }) => {
39+
if (first) {
40+
first = false;
41+
// Native finalization commits success before workspace/lease cleanup
42+
// finishes. Keep the real heartbeat executor alive across that boundary.
43+
await db.update(heartbeatRuns).set({ status: "succeeded", runtimeMode: "native", finishedAt: new Date() }).where(eq(heartbeatRuns.id, context.runId));
44+
entered();
45+
await cleanupGate;
46+
} else {
47+
await db.update(issues).set({ status: "done" }).where(eq(issues.id, context.context.issueId));
48+
}
49+
return { exitCode: 0, signal: null, timedOut: false, summary: "Completed" };
50+
});
51+
const heartbeat = heartbeatService(db);
52+
const nextId = randomUUID(), otherId = randomUUID();
53+
try {
54+
await heartbeat.invoke(agentId, "on_demand", { issueId }, "manual");
55+
await terminal;
56+
await db.insert(heartbeatRuns).values([
57+
{ id: nextId, companyId, agentId, status: "queued", invocationSource: "on_demand", responsibleUserId: "owner", contextSnapshot: { issueId } },
58+
{ id: otherId, companyId, agentId, status: "queued", invocationSource: "on_demand", responsibleUserId: "owner", contextSnapshot: { issueId: otherIssueId } },
59+
]);
60+
await heartbeat.resumeQueuedRuns();
61+
expect((await heartbeat.getRun(nextId))?.status).toBe("queued");
62+
expect((await heartbeat.getRun(otherId))?.status).not.toBe("queued");
63+
} finally {
64+
release();
65+
await heartbeat.drainActiveRunExecutions();
66+
}
67+
expect((await heartbeat.getRun(nextId))?.status).toBe("succeeded");
68+
expect(execute).toHaveBeenCalledTimes(3);
69+
}, 30_000);
70+
});

server/src/__tests__/native-workspace-sync.test.ts

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {
1212
nativeWorkspaceSyncInternals,
1313
readNativeWorkspaceSyncReference,
1414
resumeNativeWorkspaceSync,
15+
prepareNativeWorkspaceSync,
1516
} from "../services/native-runtime/native-workspace-sync.js";
1617

1718
const digest = "a".repeat(64);
@@ -94,6 +95,20 @@ describe("native workspace sync durable metadata", () => {
9495
).toThrow("native_workspace_sync_unexpected_existing_descriptor");
9596
});
9697

98+
it("refuses a different sandbox before preparing restart workspace sync", async () => {
99+
const select = vi.fn();
100+
await expect(prepareNativeWorkspaceSync({
101+
db: { select } as never, runId: "run", companyId: "company", workspaceId: "workspace",
102+
workspaceLocalDir: "/tmp/local-workspace", lease: { id: "lease", providerLeaseId: "replacement" } as never,
103+
target: { kind: "remote", transport: "sandbox", remoteCwd: "/workspace", providerKey: "daytona",
104+
sandboxLeaseAcquisition: { outcome: "replacement", providerLeaseId: "replacement" } } as never,
105+
restartRecovery: { kind: "reattach_remote_runner", runId: "run", leaseOwner: "controller", controllerGeneration: 2,
106+
providerAttempt: 1, restartKind: "graceful", recoveryRequestId: null,
107+
remote: { providerLeaseId: "original", remoteCwd: "/workspace" } },
108+
})).rejects.toThrow("native_remote_recovery_lease_mismatch");
109+
expect(select).not.toHaveBeenCalled();
110+
});
111+
97112
it("reads backward-compatible references and the resource disposition", () => {
98113
const base = {
99114
schema: "paperclip.native-workspace-sync/v1",

server/src/services/heartbeat.ts

Lines changed: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14839,6 +14839,7 @@ export function heartbeatService(
1483914839
const claims = dispositions.filter(
1484014840
(disposition): disposition is NativeRestartRecoveryClaim =>
1484114841
disposition.kind === "reattach_existing_runner" ||
14842+
disposition.kind === "reattach_remote_runner" ||
1484214843
disposition.kind === "resume_dead_runner" ||
1484314844
disposition.kind === "bootstrap_incomplete",
1484414845
);
@@ -14847,14 +14848,16 @@ export function heartbeatService(
1484714848
if (run) {
1484814849
const isClaim =
1484914850
disposition.kind === "reattach_existing_runner" ||
14851+
disposition.kind === "reattach_remote_runner" ||
1485014852
disposition.kind === "resume_dead_runner" ||
1485114853
disposition.kind === "bootstrap_incomplete";
1485214854
await appendRunEvent(run, {
1485314855
eventType: "native.recovery.transition",
1485414856
stream: "system",
1485514857
level: disposition.kind === "blocked" ? "warn" : "info",
1485614858
message:
14857-
disposition.kind === "reattach_existing_runner"
14859+
disposition.kind === "reattach_existing_runner" ||
14860+
disposition.kind === "reattach_remote_runner"
1485814861
? "Recovering the existing native runner process after server restart"
1485914862
: disposition.kind === "resume_dead_runner"
1486014863
? "Resuming the durable native provider session after runner process loss"
@@ -17129,6 +17132,23 @@ export function heartbeatService(
1712917132
}
1713017133

1713117134
const issueId = readNonEmptyString(context.issueId);
17135+
if (issueId && activeRunExecutions.size > 0) {
17136+
// Native finalization publishes success before workspace synchronization,
17137+
// provider suspension, and lease release finish. A queued comment must
17138+
// not acquire a fresh sandbox while its predecessor still owns that work.
17139+
// The executor's finally block retries this agent after removing its owner.
17140+
const [settlingOwner] = await db.select({ id: heartbeatRuns.id })
17141+
.from(heartbeatRuns)
17142+
.where(and(
17143+
eq(heartbeatRuns.companyId, run.companyId),
17144+
eq(heartbeatRuns.agentId, run.agentId),
17145+
eq(heartbeatRuns.runtimeMode, "native"),
17146+
inArray(heartbeatRuns.id, [...activeRunExecutions]),
17147+
sql`${heartbeatRuns.contextSnapshot}->>'issueId' = ${issueId}`,
17148+
))
17149+
.limit(1);
17150+
if (settlingOwner) return null;
17151+
}
1713217152
if (issueId) {
1713317153
const activePauseHold = await treeControlSvc.getActivePauseHoldGate(
1713417154
run.companyId,
@@ -19903,7 +19923,8 @@ export function heartbeatService(
1990319923
if (
1990419924
runOptions.nativeLeaseOwner &&
1990519925
run.runtimeMode === "native" &&
19906-
runOptions.nativeRestartRecovery?.kind !== "reattach_existing_runner"
19926+
runOptions.nativeRestartRecovery?.kind !== "reattach_existing_runner" &&
19927+
runOptions.nativeRestartRecovery?.kind !== "reattach_remote_runner"
1990719928
) {
1990819929
// A numeric PID or process-group ID is a liveness signal, never an
1990919930
// ownership capability: the OS may have recycled it after the service

server/src/services/native-runtime/native-restart-recovery.test.ts

Lines changed: 59 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,16 +9,72 @@ import {
99

1010
describe("native restart recovery classification", () => {
1111
it("does not reopen a failed checkpoint or reset an exhausted provider budget", () => {
12-
const evidence = { runnerPidAlive: false, runnerGroupAlive: false, processStartMatches: false, hasCheckpoint: true, hasProviderEvidence: true };
13-
expect(classifyNativeRunnerRecoveryEvidence({ ...evidence, checkpointFailed: true })).toMatchObject({ claimKind: null, reason: "provider_checkpoint_permanently_failed" });
14-
expect(classifyNativeRunnerRecoveryEvidence({ ...evidence, providerAttempt: 3 })).toMatchObject({ claimKind: null, reason: "execution_recovery_budget_exhausted" });
12+
const evidence = {
13+
runnerPidAlive: false,
14+
runnerGroupAlive: false,
15+
processStartMatches: false,
16+
hasCheckpoint: true,
17+
hasProviderEvidence: true,
18+
};
19+
expect(
20+
classifyNativeRunnerRecoveryEvidence({
21+
...evidence,
22+
checkpointFailed: true,
23+
}),
24+
).toMatchObject({
25+
claimKind: null,
26+
reason: "provider_checkpoint_permanently_failed",
27+
});
28+
expect(
29+
classifyNativeRunnerRecoveryEvidence({ ...evidence, providerAttempt: 3 }),
30+
).toMatchObject({
31+
claimKind: null,
32+
reason: "execution_recovery_budget_exhausted",
33+
});
1534
});
1635
it("keeps controller-only recovery out of the provider retry budget", () => {
1736
expect(nextNativeProviderAttempt(2, "reattach_existing_runner")).toBe(2);
37+
expect(nextNativeProviderAttempt(2, "reattach_remote_runner")).toBe(2);
1838
expect(nextNativeProviderAttempt(2, "bootstrap_incomplete")).toBe(3);
1939
expect(nextNativeProviderAttempt(2, "resume_dead_runner")).toBe(3);
2040
});
2141

42+
it("verifies remote ownership in the sandbox instead of interpreting its PIDs locally", () => {
43+
const evidence = {
44+
remoteSandbox: true,
45+
runnerPidAlive: true,
46+
runnerGroupAlive: true,
47+
processStartMatches: false,
48+
knownProviderProcessAlive: true,
49+
knownProviderProcessIdentityAmbiguous: true,
50+
hasCheckpoint: true,
51+
checkpointIdentityMatches: true,
52+
hasProviderEvidence: true,
53+
providerAttempt: 3,
54+
};
55+
expect(classifyNativeRunnerRecoveryEvidence(evidence).claimKind).toBe(
56+
"reattach_remote_runner",
57+
);
58+
expect(
59+
classifyNativeRunnerRecoveryEvidence({
60+
...evidence,
61+
checkpointIdentityMatches: false,
62+
}).claimKind,
63+
).toBeNull();
64+
expect(
65+
classifyNativeRunnerRecoveryEvidence({
66+
...evidence,
67+
checkpointFailed: true,
68+
}).claimKind,
69+
).toBeNull();
70+
expect(
71+
classifyNativeRunnerRecoveryEvidence({
72+
...evidence,
73+
hasCheckpoint: false,
74+
}).claimKind,
75+
).toBeNull();
76+
});
77+
2278
it.each([
2379
{
2480
name: "reattaches an exact live runner",

0 commit comments

Comments
 (0)