mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-14 17:22:00 +01:00
fix(daytona): retry a sandbox create that timed out before any agent work (#20828)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - On a Daytona sandbox environment, every new task run first creates a fresh sandbox > - When one Daytona create did not start, the plugin waited the full 300 s acquisition budget > - The run then failed `setup_failed` with `lease_acquire_failed`. No agent work had started, but nothing retried it > - On staging this hit two agents in 15 minutes. Each attempt used about 5 minutes, and the task stopped until a person pressed Retry. The manual Retry then got a sandbox in 28 s > - This pull request ends a stuck create early, adopts a create that started late, and routes the timeout to the bounded bootstrap retry that already exists for transient provider refusals > - The benefit is that a stuck create costs about 90 s and recovers by itself. The task shows Retry only after the bounded attempts are used up ## Linked Issues or Issue Description Refs #20626 (bounded resume of a reusable sandbox), Refs #20212 (wait out a Daytona state change), Refs #20609 (force a stuck stop). **What happened?** A run on a Daytona sandbox environment failed after 307 s with `Failed to acquire lease for environment "Paperclip Sandbox" (sandbox): Daytona lease acquisition timed out; allocation cleanup is pending`. The run recorded `errorCode: setup_failed`, `failureCause: lease_acquire_failed`, and `executionRecovery: { kind: "bootstrap", providerWorkStarted: false }`. The task became blocked with Retry. No automatic retry was scheduled. **Steps to reproduce** 1. Use a Daytona sandbox environment with the default `timeoutMs` (300,000). 2. Start a run while Daytona keeps the new sandbox in `creating`. 3. See that the run waits 300 s, fails `setup_failed`, and is not retried. **Expected behavior** A create that has not started before provider work must not cost 5 minutes. The run must retry by itself within bounds. A late sandbox that has started must be used. A sandbox that was abandoned must be removed. **Root cause** - `packages/plugins/sandbox-providers/daytona/src/plugin.ts` (`onEnvironmentAcquireLease`): the only bound on the create step was the whole acquisition budget (`timeoutMs`, default 300 s). - `server/src/services/environment-provider-unavailable-error.ts` (`readTransientEnvironmentProviderFailure`): only an HTTP status or errno on the create call counted as transient. The timeout envelope carries neither, so the run fell through to generic `setup_failed`, which `isTransientWorkspacePreparationCode` does not retry. ## What Changed - Daytona: one fresh acquisition is now capped at 180 s. A create that has not returned after 90 s is looked up once by its creation name. A sandbox in `started` is adopted, and setup continues. A sandbox in `pending_build`, `building_snapshot`, or `pulling_snapshot` keeps waiting inside the cap. Any other state ends the attempt with the existing cleanup envelope and an acquisition diagnostic whose elapsed time reached the expired budget. - Daytona: a late SDK failure no longer runs the compensating delete for a sandbox that was adopted. - Host: `readTransientEnvironmentProviderFailure` reads the validated acquisition diagnostic. An acquisition whose first failure was its own deadline is transient. The run fails with `environment_provider_unavailable`, category `timeout`, and uses the bounded bootstrap retry (30 s, then 60 s). An earlier setup failure stays `setup_failed`. Message text is never parsed. - Host: the task reason for this case is plain: "The sandbox for ... did not finish starting in time. No agent work had started. Paperclip retries automatically ...". - Docs: `doc/execution-semantics.md` and `doc/plugins/SANDBOX_PROVIDER_CAPABILITIES.md`. ## Verification Red before the fix, green after: - `server/src/services/heartbeat/run-completion.test.ts` "routes a fresh sandbox create that timed out through bounded bootstrap recovery" uses the staging error shape through the orchestrator. Red: `errorCode` was `setup_failed`. Green: `environment_provider_unavailable`, category `timeout`, bounded retry scheduled. The `started` and `early_failure` cases stay `setup_failed` with no retry. - `server/src/services/environment-provider-unavailable-error.test.ts`: deadline diagnostic is transient (red, then green). An early failure is not. The task message is plain. - `packages/plugins/sandbox-providers/daytona/src/plugin.test.ts`: "ends a create still pending at 90 seconds" (red: still pending at 90 s; green) and "adopts a create that started by 90 seconds" (red; green, and a late SDK rejection does not delete it). "keeps waiting for a create that is still preparing its snapshot" is a safety case. - Existing tests that encoded the 300 s acquisition budget now use the 180 s cap. Commands run locally: - `npx vitest run src/plugin.test.ts` (Daytona): 414 passed. - `npx vitest run src/services/heartbeat/run-completion.test.ts src/services/environment-provider-unavailable-error.test.ts src/__tests__/environment-acquisition-diagnostics.test.ts src/__tests__/task-interruption-query.test.ts`: 229 tests passed. The run-completion file reports an `afterAll` embedded-Postgres teardown timeout. The same timeout occurs on unchanged `origin/master`, so it is a local platform fault. - Daytona plugin `pnpm typecheck` and server `pnpm typecheck`: pass. - `pnpm verify`: asked for broader CI coverage, because `doc/execution-semantics.md` is a global CI input. Required CI supplies that coverage. Upgrade safety: nothing new is persisted. A task already blocked by this failure keeps `canRetry: true`. Its Retry starts a new run with a new creation record, and that run uses the new path. On staging, the manual Retry of the affected task got a fresh lease in 28 s. The creation record of the earlier attempt stays with the creation reconciler. Not verified: a live Daytona create that stays in `creating`. That cannot be forced on demand. ## Risks - A legitimately slow create that stays in `creating` or `starting` for more than 90 s now ends the attempt and retries. Snapshot preparation states still wait inside the 180 s cap. Staging creates normally take about 20 to 30 s. - An abandoned create is removed by the creation reconciler. It can stay alive until that reconciler settles the record (the existing settle window). This PR does not change that window. - Other sandbox providers get the host classification only when they send the same acquisition diagnostic. ## Model Used Claude Opus 5.5 (claude-opus-5-5), Anthropic, used through Claude Code with tool use (shell, file edits, browser for read-only staging API reads). ## 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 - [ ] All Paperclip CI gates are green - [ ] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [ ] 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:
1 parent
889b33da72
commit
57f1afdf19
7 files changed
+268
-40
No files matched your search
@@ -49,6 +49,24 @@ describe("readTransientEnvironmentProviderFailure", () => {
|
||||
expect(readTransientEnvironmentProviderFailure(rpcError)).toEqual({ httpStatus: 429 });
|
||||
});
|
||||
|
||||
const stagingLeaseTimeout = (diagnostic: { phase: string; elapsedMs: number; budgetMs: number; providerState?: string }) =>
|
||||
Object.assign(new Error("Daytona lease acquisition timed out; allocation cleanup is pending"), {
|
||||
data: { schema: "paperclip/environment-creation-cleanup/v1",
|
||||
cleanup: { providerLeaseId: "paperclip-create-05f81d77", companyId: "company", environmentId: "environment",
|
||||
runId: "f41857b6", attemptId: "05f81d77", accountFingerprint: "a".repeat(64),
|
||||
labels: { "paperclip-provider": "daytona", "paperclip-create-attempt": "05f81d77" } },
|
||||
acquisitionDiagnostic: diagnostic },
|
||||
});
|
||||
|
||||
it("treats an acquisition that reached its deadline as transient", () => {
|
||||
const timeout = stagingLeaseTimeout({ phase: "create", elapsedMs: 300_004, budgetMs: 300_000, providerState: "creating" });
|
||||
expect(readTransientEnvironmentProviderFailure(new Error("outer", { cause: timeout }))).toEqual({ acquisitionTimedOut: true });
|
||||
});
|
||||
|
||||
it("does not retry an acquisition whose first failure came before its deadline", () => {
|
||||
expect(readTransientEnvironmentProviderFailure(stagingLeaseTimeout({ phase: "workspace", elapsedMs: 100, budgetMs: 300_000 }))).toBeNull();
|
||||
});
|
||||
|
||||
it("never parses message text", () => {
|
||||
expect(readTransientEnvironmentProviderFailure(new Error("HTTP 502 Bad Gateway ECONNRESET"))).toBeNull();
|
||||
});
|
||||
@@ -70,6 +88,15 @@ describe("EnvironmentProviderUnavailableError", () => {
|
||||
expect(error.providerFailure.retryability).toBe("transient");
|
||||
});
|
||||
|
||||
it("names an acquisition timeout in plain words", () => {
|
||||
const error = new EnvironmentProviderUnavailableError({
|
||||
environmentName: "Paperclip Sandbox", driver: "sandbox", evidence: { acquisitionTimedOut: true }, cause: null,
|
||||
});
|
||||
expect(error.providerFailure).toMatchObject({ category: "timeout", retryability: "transient", phase: "startup" });
|
||||
expect(error.providerFailure.message).toContain("did not finish starting");
|
||||
expect(error.providerFailure.message).not.toMatch(/ETIMEDOUT|undefined/);
|
||||
});
|
||||
|
||||
it("falls back to a generic provider label for an unsafe driver key", () => {
|
||||
const error = new EnvironmentProviderUnavailableError({
|
||||
environmentName: "env", driver: "bad driver <x>", evidence: { httpStatus: 502 }, cause: null,
|
||||
|
||||
@@ -1,10 +1,11 @@
|
||||
import { readEnvironmentAcquireErrorDiagnostic } from "@paperclipai/plugin-sdk";
|
||||
import { readEnvironmentAcquireErrorDiagnostic, readEnvironmentAcquisitionDiagnostic } from "@paperclipai/plugin-sdk";
|
||||
import type { ProviderFailure } from "@paperclipai/shared";
|
||||
|
||||
/**
|
||||
* Setup failure code for a lease acquisition that the environment provider
|
||||
* refused with a transient status (HTTP 408/425/429/5xx or a connection-level
|
||||
* errno) before any agent work started. The run is retried with the bounded
|
||||
* errno), or that reached its own acquisition deadline, before any agent work
|
||||
* started. The run is retried with the bounded
|
||||
* bootstrap budget. A provisional allocation whose cleanup could not be
|
||||
* confirmed stays owned by its own creation record, which the cleanup
|
||||
* reconciler settles independently; every retry is a new creation attempt
|
||||
@@ -20,7 +21,7 @@ const TRANSIENT_ERROR_CODES = new Set([
|
||||
"UND_ERR_CONNECT_TIMEOUT", "UND_ERR_SOCKET",
|
||||
]);
|
||||
|
||||
type TransientProviderEvidence = { httpStatus?: number; errorCode?: string };
|
||||
type TransientProviderEvidence = { httpStatus?: number; errorCode?: string; acquisitionTimedOut?: true };
|
||||
|
||||
function own(value: unknown, key: string): unknown {
|
||||
try {
|
||||
@@ -31,10 +32,13 @@ function own(value: unknown, key: string): unknown {
|
||||
/**
|
||||
* Read transient provider evidence from structured fields only: the validated
|
||||
* plugin RPC envelope, or the status/errno an in-process provider attached to
|
||||
* its error. Message text is never parsed.
|
||||
* its error. An acquisition whose first failure was its own deadline is
|
||||
* transient too. Message text is never parsed.
|
||||
*/
|
||||
export function readTransientEnvironmentProviderFailure(error: unknown): TransientProviderEvidence | null {
|
||||
for (let cause = error, depth = 0; cause && depth < 5; cause = own(cause, "cause"), depth++) {
|
||||
const acquisition = readEnvironmentAcquisitionDiagnostic(cause);
|
||||
if (acquisition && acquisition.elapsedMs >= acquisition.budgetMs) return { acquisitionTimedOut: true };
|
||||
const envelope = readEnvironmentAcquireErrorDiagnostic(cause);
|
||||
const statusValue = envelope?.httpStatus ?? [own(cause, "status"), own(cause, "statusCode")]
|
||||
.find(value => Number.isInteger(value));
|
||||
@@ -52,7 +56,7 @@ export function readTransientEnvironmentProviderFailure(error: unknown): Transie
|
||||
|
||||
function category(evidence: TransientProviderEvidence): ProviderFailure["category"] {
|
||||
if (evidence.httpStatus === 429) return "rate_limit";
|
||||
if (evidence.httpStatus === 408 || evidence.httpStatus === 504
|
||||
if (evidence.acquisitionTimedOut || evidence.httpStatus === 408 || evidence.httpStatus === 504
|
||||
|| evidence.errorCode === "ETIMEDOUT" || evidence.errorCode === "UND_ERR_CONNECT_TIMEOUT") return "timeout";
|
||||
return "overloaded";
|
||||
}
|
||||
@@ -66,8 +70,10 @@ export class EnvironmentProviderUnavailableError extends Error {
|
||||
readonly providerFailure: ProviderFailure;
|
||||
|
||||
constructor(input: { environmentName: string; driver: string; evidence: TransientProviderEvidence; cause: unknown }) {
|
||||
const status = input.evidence.httpStatus !== undefined ? `HTTP ${input.evidence.httpStatus}` : input.evidence.errorCode!;
|
||||
super(`Environment provider for "${input.environmentName}" was temporarily unavailable (${status}).`, { cause: input.cause });
|
||||
const status = input.evidence.httpStatus !== undefined ? `HTTP ${input.evidence.httpStatus}` : input.evidence.errorCode;
|
||||
super(status
|
||||
? `Environment provider for "${input.environmentName}" was temporarily unavailable (${status}).`
|
||||
: `Environment provider for "${input.environmentName}" did not finish starting the sandbox in time.`, { cause: input.cause });
|
||||
this.name = "EnvironmentProviderUnavailableError";
|
||||
this.httpStatus = input.evidence.httpStatus ?? null;
|
||||
this.errorCode = input.evidence.errorCode ?? null;
|
||||
@@ -76,7 +82,9 @@ export class EnvironmentProviderUnavailableError extends Error {
|
||||
provider: /^[a-zA-Z0-9_.:/-]{1,80}$/.test(input.driver) ? input.driver : "environment",
|
||||
phase: "startup",
|
||||
category: category(input.evidence),
|
||||
message: `The provider for "${input.environmentName}" was temporarily unavailable while Paperclip was starting it (${status}). `
|
||||
message: (status
|
||||
? `The provider for "${input.environmentName}" was temporarily unavailable while Paperclip was starting it (${status}). `
|
||||
: `The sandbox for "${input.environmentName}" did not finish starting in time. `)
|
||||
+ "No agent work had started. Paperclip retries automatically; if those retries are used up, use Retry once the provider is available.",
|
||||
...(input.evidence.httpStatus !== undefined ? { httpStatus: input.evidence.httpStatus } : {}),
|
||||
...(input.evidence.errorCode !== undefined ? { providerCode: input.evidence.errorCode } : {}),
|
||||
|
||||
@@ -1403,6 +1403,33 @@ describe.skipIf(!support.supported && !process.env.PAPERCLIP_TEST_DATABASE_URL)(
|
||||
} else expect(f.deps.scheduleBoundedRetryForRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// Staging run f41857b6: the Daytona create was still pending at the
|
||||
// acquisition deadline, so the worker returned the cleanup envelope with a
|
||||
// create-phase diagnostic and no provider status.
|
||||
const daytonaLeaseTimeout = (elapsedMs: number) => new JsonRpcCallError({ code: -32000,
|
||||
message: "Daytona lease acquisition timed out; allocation cleanup is pending",
|
||||
data: { schema: "paperclip/environment-creation-cleanup/v1",
|
||||
cleanup: { providerLeaseId: "paperclip-create-05f81d77", companyId: "company", environmentId: "environment",
|
||||
runId: "f41857b6", attemptId: "05f81d77", accountFingerprint: "a".repeat(64),
|
||||
labels: { "paperclip-provider": "daytona", "paperclip-create-attempt": "05f81d77" } },
|
||||
acquisitionDiagnostic: { phase: "create", elapsedMs, budgetMs: 300_000, providerState: "creating" } } });
|
||||
|
||||
it.each(["fresh", "started", "early_failure"] as const)("routes a %s sandbox create that timed out through bounded bootstrap recovery", async scenario => {
|
||||
const f = await fixture();
|
||||
const error = await acquireThroughOrchestrator(daytonaLeaseTimeout(scenario === "early_failure" ? 100 : 300_004));
|
||||
f.deps.scheduleBoundedRetryForRun.mockResolvedValueOnce({ outcome: "not_scheduled", reason: "Fixture scheduler boundary", issueId: null });
|
||||
await f.completion.failRunSetup(f.setup(error, scenario === "started"));
|
||||
const failed = await read(f.run.id);
|
||||
const retried = scenario === "fresh";
|
||||
expect(failed).toMatchObject({ status: "failed", errorCode: retried ? "environment_provider_unavailable" : "setup_failed" });
|
||||
expect(failed.error).toContain("Daytona lease acquisition timed out");
|
||||
if (retried) {
|
||||
expect(failed.resultJson).toMatchObject({ executionRecovery: { kind: "bootstrap", providerWorkStarted: false },
|
||||
providerFailure: { provider: "sandbox", phase: "startup", category: "timeout", retryability: "transient" } });
|
||||
expect(f.deps.scheduleBoundedRetryForRun).toHaveBeenCalledWith(expect.objectContaining({ id: f.run.id }), f.agent);
|
||||
} else expect(f.deps.scheduleBoundedRetryForRun).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it.each(["fresh", "started", "untrusted", "denied", "retained", "session", "runner"] as const)("bounds only an owned pre-dispatch chat binding wait: %s", async scenario => {
|
||||
const f = await fixture({ runtimeMode: scenario === "retained" ? "native" : "legacy",
|
||||
...(scenario === "session" ? { nativeSessionId: randomUUID() } : {}),
|
||||
|
||||
Reference in new issue
Block a user