mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-08 01:57:31 +01:00
fix(daytona): drop the stale session id on every sandbox restart (#19050)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Native agent runs can execute inside a Daytona sandbox through the Daytona sandbox provider plugin > - The plugin runs every command for a lease in one persistent Daytona process session and keeps that session id in memory > - Daytona auto-stops an idle sandbox, and a stopped sandbox loses all of its process sessions > - Only the execute and resume hooks dropped the stored session id before a restart. Workspace realize, sync-in, sync-out, runner ingress, and interactive setup also restart a stopped sandbox, but they kept the dead id > - When host-side workspace preparation stays idle for longer than the auto-stop interval, sync-in restarts the sandbox, and the next command sends the dead id. The native run then fails with `native_session_interrupted: session not found` > - This pull request moves the session-id clear into the one shared start helper, so every restart path drops the stale id > - The benefit is that a run that waits through a long preparation opens a fresh session instead of failing ## Linked Issues or Issue Description **What happened?** Native runs on Daytona failed with `native_session_interrupted` and the error `session not found`. The failure came about 4 seconds after the provider session started. In every observed run, the workspace pack phase took longer than 15 minutes (917 s to 1442 s). This is longer than the default Daytona `autoStopInterval` of 15 minutes. **Expected behavior** After Daytona restarts an auto-stopped sandbox, the next command opens a new process session and the run continues. **Steps to reproduce** 1. Run one command on a Daytona lease, so the plugin opens and stores a session. 2. Let the sandbox stop (Daytona auto-stop after an idle gap). 3. Call workspace realize or sync-in on the lease. The hook restarts the sandbox. 4. Run the next command. It uses the stored session id, and Daytona answers `session not found`. No related open PR exists. The undeployed native fixes on master (#18992, #19025, #19026, #19004) do not touch the Daytona session store. ## What Changed - `ensureSandboxStarted` in the Daytona plugin now takes the lease scope. When the sandbox is not running, it clears the stored session id before it starts or recovers the sandbox. - All seven callers (execute, resume, realize workspace, interactive setup, runner ingress, sync-in, sync-out) pass their lease scope. The compiler enforces the scope on any new caller. - The execute hook no longer has its own copy of the clear. The resume hook keeps its clear because it also closes duplex channels before the restart. - Added three regression tests: a fresh session after realize restarts an auto-stopped sandbox, a fresh session after sync-in restarts it, and no new session when realize finds the sandbox already running. ## Verification - Red before the fix: `pnpm exec vitest run src/plugin.test.ts -t "auto-stopped sandbox|already running"` in `packages/plugins/sandbox-providers/daytona`. The two restart tests failed with `expected "vi.fn()" to be called 2 times, but got 1 times`. The running-sandbox safety test passed. - Green after the fix: the full Daytona plugin suite (`pnpm exec vitest run`) passes, 542 passed and 8 skipped. - `pnpm typecheck` in the Daytona plugin package passes. - Not done: a live Daytona run that idles past the auto-stop interval. The fix needs deployment before the staging failure can be checked again. ## Risks - Low risk. The clear runs only when the sandbox is not in the `started` state. A stopped sandbox has no live session, so no live shell can leak. A running sandbox keeps its stored session, and a test covers this case. - The recover path for a recoverable `error` state now also drops the id. A recovered sandbox restarts, so its old session is also gone. - Duplex channels on a restarted sandbox are still closed only by the resume hook. This PR does not change that behavior. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use (shell, file edits, browser reads of staging run records). ## 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
DottaandClaude Opus 5.5 authored and GitHub committed
1 parent
9c6993f0aa
commit
367aa16800
2 files changed
+111
-28
No files matched your search
@@ -3019,6 +3019,90 @@ describe("Daytona sandbox provider plugin", () => {
|
||||
expect(sandbox.process.deleteSession).toHaveBeenCalledWith(sessionId);
|
||||
});
|
||||
|
||||
// An idle gap longer than `autoStopInterval` (for example, a long host-side
|
||||
// workspace pack) lets Daytona stop the sandbox. Workspace realize and
|
||||
// sync-in then restart it before the next command. A restarted sandbox has
|
||||
// no process sessions, so the next command must open a fresh session
|
||||
// instead of sending the dead id and failing with "session not found".
|
||||
const autoStoppedSandbox = () => {
|
||||
const sandbox = createMockSandbox({ state: "started" });
|
||||
sandbox.start.mockImplementation(async () => {
|
||||
sandbox.state = "started";
|
||||
});
|
||||
return sandbox;
|
||||
};
|
||||
|
||||
it("opens a fresh session after workspace realize restarts an auto-stopped sandbox", async () => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
const sandbox = autoStoppedSandbox();
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await plugin.definition.onEnvironmentExecute?.(sessionExecParams());
|
||||
expect(sandbox.process.createSession).toHaveBeenCalledTimes(1);
|
||||
const staleSessionId = sandbox.process.createSession.mock.calls[0]![0] as string;
|
||||
|
||||
sandbox.state = "stopped";
|
||||
await plugin.definition.onEnvironmentRealizeWorkspace?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: { providerLeaseId: "sandbox-123", metadata: { remoteCwd: "/home/daytona/paperclip-workspace" } },
|
||||
workspace: { localPath: "/tmp/workspace" },
|
||||
} as never);
|
||||
expect(sandbox.start).toHaveBeenCalledTimes(1);
|
||||
|
||||
await plugin.definition.onEnvironmentExecute?.(sessionExecParams());
|
||||
expect(sandbox.process.createSession).toHaveBeenCalledTimes(2);
|
||||
const freshSessionId = sandbox.process.createSession.mock.calls[1]![0] as string;
|
||||
expect(freshSessionId).not.toBe(staleSessionId);
|
||||
const lastExec = sandbox.process.executeSessionCommand.mock.calls.at(-1)!;
|
||||
expect(lastExec[0]).toBe(freshSessionId);
|
||||
});
|
||||
|
||||
it("opens a fresh session after sync-in restarts an auto-stopped sandbox", async () => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
const sandbox = autoStoppedSandbox();
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await plugin.definition.onEnvironmentExecute?.(sessionExecParams());
|
||||
expect(sandbox.process.createSession).toHaveBeenCalledTimes(1);
|
||||
|
||||
sandbox.state = "stopped";
|
||||
await plugin.definition.onEnvironmentSyncIn?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: { providerLeaseId: "sandbox-123", metadata: { remoteCwd: "/home/daytona/paperclip-workspace" } },
|
||||
operations: [],
|
||||
} as never);
|
||||
expect(sandbox.start).toHaveBeenCalledTimes(1);
|
||||
|
||||
await plugin.definition.onEnvironmentExecute?.(sessionExecParams());
|
||||
expect(sandbox.process.createSession).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("keeps the live session when realize finds the sandbox already running", async () => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
const sandbox = autoStoppedSandbox();
|
||||
mockGet.mockResolvedValue(sandbox);
|
||||
|
||||
await plugin.definition.onEnvironmentExecute?.(sessionExecParams());
|
||||
await plugin.definition.onEnvironmentRealizeWorkspace?.({
|
||||
driverKey: "daytona",
|
||||
companyId: "company-1",
|
||||
environmentId: "env-1",
|
||||
config: { timeoutMs: 300000, reuseLease: false },
|
||||
lease: { providerLeaseId: "sandbox-123", metadata: { remoteCwd: "/home/daytona/paperclip-workspace" } },
|
||||
workspace: { localPath: "/tmp/workspace" },
|
||||
} as never);
|
||||
await plugin.definition.onEnvironmentExecute?.(sessionExecParams());
|
||||
|
||||
expect(sandbox.start).not.toHaveBeenCalled();
|
||||
expect(sandbox.process.createSession).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("emits a session.open span on create and a session.close span on delete", async () => {
|
||||
process.env.DAYTONA_API_KEY = "host-key";
|
||||
const sandbox = createMockSandbox();
|
||||
|
||||
@@ -539,8 +539,15 @@ async function terminateAtProvider<T>(scope: SandboxScope, operation: string, ac
|
||||
return withLivenessTimeout(operation, timeoutMs + LIVENESS_START_TIMEOUT_MARGIN_MS, action);
|
||||
}
|
||||
|
||||
async function ensureSandboxStarted(sandbox: Sandbox, timeoutSeconds: number): Promise<void> {
|
||||
// Start (or recover) a sandbox that is not running. Every restart path goes
|
||||
// through here with the lease scope, because a restart destroys every Daytona
|
||||
// process session. The stored session id for the lease is dropped before the
|
||||
// restart, so the next command opens a fresh session instead of sending a dead
|
||||
// id and failing with "session not found". A running sandbox keeps its live
|
||||
// session, so the id is left in place when no restart happens.
|
||||
async function ensureSandboxStarted(sandbox: Sandbox, timeoutSeconds: number, scope: SandboxScope): Promise<void> {
|
||||
if (sandbox.state === "started") return;
|
||||
sandboxHandleSessionStore.clear(scope);
|
||||
// Bound the lifecycle call just past its own SDK deadline. A normal slow start
|
||||
// finishes within `timeoutSeconds`; only a connection-level hang the SDK
|
||||
// deadline misses reaches this wrapper bound.
|
||||
@@ -2891,7 +2898,7 @@ const plugin = definePlugin(trackDaytonaHooks({
|
||||
await traceLeaseStep("lease.channel_cleanup", () => closeDaytonaDuplexChannelsForLease(params.providerLeaseId));
|
||||
}
|
||||
const resumedFromState = sandbox.state ?? null;
|
||||
await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs));
|
||||
await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs), scope);
|
||||
try {
|
||||
return await traceLeaseStep("lease.verify", async () => {
|
||||
const remoteCwd = await traceLeaseStep("lease.workspace", () => resolveSandboxWorkingDirectory(sandbox, false));
|
||||
@@ -3144,7 +3151,7 @@ const plugin = definePlugin(trackDaytonaHooks({
|
||||
};
|
||||
await withSandboxActivityGate(scope, async () => {
|
||||
const sandbox = await getSandbox(scope, { bypassTeardownGate: true });
|
||||
await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs));
|
||||
await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs), scope);
|
||||
await sandbox.fs.createFolder(remoteCwd, "755");
|
||||
});
|
||||
}
|
||||
@@ -3239,7 +3246,7 @@ const plugin = definePlugin(trackDaytonaHooks({
|
||||
};
|
||||
}
|
||||
|
||||
await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs));
|
||||
await ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs), scope);
|
||||
const remoteCwd = await resolveSandboxWorkingDirectory(sandbox);
|
||||
const shellCommand = await detectSandboxShellCommand(sandbox, toTimeoutSeconds(config.timeoutMs));
|
||||
const connection = params.includeConnectionPayload === true
|
||||
@@ -3458,13 +3465,10 @@ const plugin = definePlugin(trackDaytonaHooks({
|
||||
providerLeaseId,
|
||||
config,
|
||||
};
|
||||
if (sandbox.state !== "started") {
|
||||
// A provider restart destroys Daytona process sessions. Drop the stale
|
||||
// session id before starting the sandbox so runnerd recovery opens a
|
||||
// new session instead of retrying a dead one for its whole grace.
|
||||
sandboxHandleSessionStore.clear(scope);
|
||||
}
|
||||
await ensureSandboxStarted(sandbox, toTimeoutSeconds(resolveTimeoutMs(params.timeoutMs, config)));
|
||||
// A provider restart destroys Daytona process sessions; the start helper
|
||||
// drops the stale session id so runnerd recovery opens a new session
|
||||
// instead of retrying a dead one for its whole grace.
|
||||
await ensureSandboxStarted(sandbox, toTimeoutSeconds(resolveTimeoutMs(params.timeoutMs, config)), scope);
|
||||
// Dispatch the command. A normal command runs in the persistent session:
|
||||
// the provider opens the one session on a cache miss and runs every command
|
||||
// in it. The provider never falls back to a one-shot command to open a
|
||||
@@ -3507,23 +3511,18 @@ const plugin = definePlugin(trackDaytonaHooks({
|
||||
throw new Error("Daytona runner ingress requires a provider lease id.");
|
||||
}
|
||||
const config = parseDriverConfig(params.config);
|
||||
const scope: SandboxScope = {
|
||||
driverKey: params.driverKey,
|
||||
companyId: params.companyId,
|
||||
environmentId: params.environmentId,
|
||||
providerLeaseId,
|
||||
config,
|
||||
};
|
||||
return await withSandboxActivityGate(
|
||||
{
|
||||
driverKey: params.driverKey,
|
||||
companyId: params.companyId,
|
||||
environmentId: params.environmentId,
|
||||
providerLeaseId,
|
||||
config,
|
||||
},
|
||||
scope,
|
||||
async () => {
|
||||
const sandbox = await acquireRunnerIngress("sandbox_lookup", () => getSandbox({
|
||||
driverKey: params.driverKey,
|
||||
companyId: params.companyId,
|
||||
environmentId: params.environmentId,
|
||||
providerLeaseId,
|
||||
config,
|
||||
}));
|
||||
await acquireRunnerIngress("sandbox_start", () => ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs)));
|
||||
const sandbox = await acquireRunnerIngress("sandbox_lookup", () => getSandbox(scope));
|
||||
await acquireRunnerIngress("sandbox_start", () => ensureSandboxStarted(sandbox, toTimeoutSeconds(config.timeoutMs), scope));
|
||||
await acquireRunnerIngress("sandbox_refresh", () => withLivenessTimeout(
|
||||
"sandbox.refreshData",
|
||||
config.livenessTimeoutMs,
|
||||
@@ -3584,7 +3583,7 @@ const plugin = definePlugin(trackDaytonaHooks({
|
||||
sandboxHandleWritableDirs.recordWritableTargets(scope, params.operations);
|
||||
return await withSandboxActivityGate(scope, async () => {
|
||||
const sandbox = await getSandbox(scope, { bypassTeardownGate: true });
|
||||
await ensureSandboxStarted(sandbox, timeoutSeconds);
|
||||
await ensureSandboxStarted(sandbox, timeoutSeconds, scope);
|
||||
const result = await performSyncIn({
|
||||
sandbox,
|
||||
operations: params.operations,
|
||||
@@ -3617,7 +3616,7 @@ const plugin = definePlugin(trackDaytonaHooks({
|
||||
return await withSandboxActivityGate(scope, async () => {
|
||||
const sandbox = await withEnvironmentSyncTransferStep("sandbox_access", async () => {
|
||||
const resolved = await getSandbox(scope, { bypassTeardownGate: true });
|
||||
await ensureSandboxStarted(resolved, timeoutSeconds);
|
||||
await ensureSandboxStarted(resolved, timeoutSeconds, scope);
|
||||
return resolved;
|
||||
});
|
||||
const result = await performSyncOut({
|
||||
|
||||
Reference in new issue
Block a user