mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-23 09:07:46 +02:00
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work.
> - Agents can run in Daytona sandboxes. The Daytona plugin acquires a
lease before each run.
> - Some acquisitions time out. The report then says
`environmentAcquisitionPhase=create` and that the whole 300 s budget was
used.
> - The Daytona SDK create timeout is equal to the acquisition budget.
The outer timer always fires first. Thus the report never shows the
state that Daytona left the sandbox in.
> - Without that state, we cannot choose a fix. A stuck snapshot pull, a
slow start, and a missing allocation need different fixes.
> - This pull request adds one bounded, read-only provider lookup at the
create deadline. It records a fixed-enum provider state in the existing
acquisition diagnostic.
> - The benefit is that the next timeout identifies where the provider
stopped. Retry, budget, and cleanup behavior do not change.
## Linked Issues or Issue Description
Refs #15736 (bounded Daytona acquisition failure observations). This
pull request extends those observations.
**What happened?**
A Daytona lease acquisition timed out with `Daytona lease acquisition
timed out; allocation cleanup is pending`. The run-failure report gave
`environmentAcquisitionPhase=create`,
`environmentAcquisitionElapsedMs=300004`, and
`environmentAcquisitionBudgetMs=300000`. The report did not show the
Daytona sandbox state. The SDK create timeout is equal to the outer
budget, so the outer timer always fires first. The SDK never reports the
stuck state.
**Expected behavior**
When creation is still pending at the deadline, the report should show
where the provider left the allocation. Examples are `pulling_snapshot`,
`starting`, `error`, or `absent`.
**Steps to reproduce**
1. Configure a Daytona environment.
2. Start a run when Daytona does not finish create and start within the
acquisition budget.
3. Inspect the setup-failure report. Only the phase, the elapsed time,
and the budget are present.
**Paperclip version or commit**
`master` at ba04968.
## What Changed
- `packages/plugins/sdk`: Add an optional `providerState` field to
`PluginEnvironmentAcquisitionDiagnostic`. The values are a fixed enum:
`absent`, `creating`, `pending_build`, `building_snapshot`,
`pulling_snapshot`, `starting`, `started`, `error`, `build_failed`,
`other`, `unavailable`. The reader omits an unknown value and keeps the
other fields. Old envelopes stay valid.
- `packages/plugins/sandbox-providers/daytona`: When the acquisition
deadline fires while `client.create` is still pending, make one
`client.get` of the provisional `paperclip-create-<attempt>` name. The
lookup has a 5 s bound, inside the host's 30 s RPC margin. Report the
state only when the name and all ownership labels match. Otherwise
report `unavailable`. A not-found error reports `absent`.
- The lookup does not delete. It does not set `observedProviderLeaseId`.
It does not change retries, budgets, or the cleanup envelope.
- After the deadline, only the timer settles the call. A late create
result cannot replace the timeout error before the lookup completes. A
test proves this guard.
- `server`: Project the value to
`run_execution.environmentAcquisitionProviderState`. It uses the same
private, run-scoped receipt as the other acquisition fields.
- `doc/observability.md`: Document the new field and its limits.
## Verification
- `cd packages/plugins/sandbox-providers/daytona && npx vitest run
--config vitest.config.ts`: 392 passed, 6 skipped.
- `npx vitest run
packages/plugins/sdk/tests/environment-creation-cleanup.test.ts
server/src/services/__tests__/run-failure-diagnostics.test.ts
server/src/__tests__/environment-acquisition-diagnostics.test.ts
server/src/__tests__/run-failure-sentry-real-sdk.test.ts
server/src/__tests__/environment-runtime.test.ts`: 302 passed, 11
skipped.
- `tsc --noEmit` passes for `packages/plugins/sdk` and
`packages/plugins/sandbox-providers/daytona`.
- New tests cover state mapping, `other` for unlisted states, `absent`,
`unavailable` for lookup failure and label mismatch, a hung lookup
bounded to 5 s, no deletion, an unchanged cleanup envelope, and a late
create that settles during the lookup.
- A mutation check removed the late-settlement guard. The late-create
test then failed.
- The local server `tsc` was killed by the sandbox memory limit. CI must
confirm the server typecheck. The server change is one line.
## Risks
- Low risk. The timeout rejection can come up to 5 s later, and only for
a create-phase deadline. The host RPC margin is 30 s.
- The lookup is one extra read-only Daytona API call for each
create-phase timeout.
- This is a diagnostic. It does not fix the upstream cause. The next
timeout report must show the state before a fix is chosen.
- This is a Sentry run-failure report change. It is not a Telemetry
change. The new value is a fixed enum. It does not contain provider IDs,
causes, URLs, or paths.
> For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and
discuss it in `#dev` before opening the PR. Feature PRs that overlap
with planned core work may need to be redirected — check the roadmap
first. See `CONTRIBUTING.md`.
## Model Used
- Anthropic Claude Opus 5.5 (`claude-opus-5-5`), used through Claude
Code with tool use (shell, file edit, test runs). Context window and
reasoning configuration are set by the runtime.
## 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
- [x] I will address all Greptile and reviewer comments before
requesting merge
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-authored-by: Devin Foley <139239+devinfoley@users.noreply.github.com>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
159 lines
9.9 KiB
TypeScript
159 lines
9.9 KiB
TypeScript
import { createInterface } from "node:readline";
|
|
import { PassThrough } from "node:stream";
|
|
import { describe, expect, it } from "vitest";
|
|
import { definePlugin } from "../src/define-plugin.js";
|
|
import { PluginEnvironmentCreationCleanupError, environmentCreationCleanupErrorData, readEnvironmentCreationCleanupError, readEnvironmentAcquisitionDiagnostic } from "../src/environment-creation-cleanup.js";
|
|
import { createRequest, isJsonRpcResponse, JsonRpcCallError, parseMessage, serializeMessage, type JsonRpcResponse } from "../src/protocol.js";
|
|
import { startWorkerRpcHost } from "../src/worker-rpc-host.js";
|
|
|
|
const ownership = {
|
|
providerLeaseId: "paperclip-create-attempt-1", companyId: "company-1", environmentId: "environment-1",
|
|
runId: "run-1", attemptId: "attempt-1", accountFingerprint: "a".repeat(64),
|
|
labels: { "paperclip-provider": "daytona" },
|
|
};
|
|
const schema = "paperclip/environment-creation-cleanup/v1";
|
|
|
|
async function invoke(method: string, error: Error) {
|
|
const stdin = new PassThrough();
|
|
const stdout = new PassThrough();
|
|
const lines = createInterface({ input: stdout });
|
|
const worker = startWorkerRpcHost({ stdin, stdout, plugin: definePlugin({
|
|
async setup() {},
|
|
async onEnvironmentAcquireLease() { throw error; },
|
|
async onEnvironmentProbe() { throw error; },
|
|
async onEnvironmentDestroyLease() { throw error; },
|
|
}) });
|
|
try {
|
|
const response = new Promise<JsonRpcResponse>((resolve) => {
|
|
lines.on("line", (line) => {
|
|
const message = parseMessage(line);
|
|
if (isJsonRpcResponse(message)) resolve(message);
|
|
});
|
|
});
|
|
stdin.write(serializeMessage(createRequest(method, {}, "cleanup-test")));
|
|
return await response;
|
|
} finally {
|
|
worker.stop(); lines.close(); stdin.destroy(); stdout.destroy();
|
|
}
|
|
}
|
|
|
|
describe("failed environment creation ownership", () => {
|
|
it.each(["environmentAcquireLease", "environmentDestroyLease"])("crosses %s RPC as allowlisted ownership only", async (method) => {
|
|
const cleanup = { ...ownership, accessToken: "secret-token", config: { apiKey: "secret-key" } };
|
|
const error = new PluginEnvironmentCreationCleanupError([new Error("secret-provider-response")], "Cleanup required", cleanup);
|
|
const response = await invoke(method, error);
|
|
expect("error" in response && response.error).toMatchObject({ data: { schema, cleanup: ownership } });
|
|
if (!("error" in response) || !response.error) throw new Error("Expected RPC error");
|
|
expect(readEnvironmentCreationCleanupError(new JsonRpcCallError(response.error))).toEqual(ownership);
|
|
expect(JSON.stringify(response)).not.toContain("secret-");
|
|
});
|
|
|
|
it.each(["environmentAcquireLease", "environmentDestroyLease", "environmentProbe"])("does not forward arbitrary error data for %s", async (method) => {
|
|
const response = await invoke(method, Object.assign(new Error("provider failure"), { data: { apiKey: "secret-key", schema, cleanup: ownership } }));
|
|
expect("error" in response && response.error).not.toHaveProperty("data");
|
|
});
|
|
|
|
it("only forwards the typed envelope on supported acquisition calls", async () => {
|
|
const response = await invoke("environmentProbe", new PluginEnvironmentCreationCleanupError([], "Cleanup required", ownership));
|
|
expect("error" in response && response.error).not.toHaveProperty("data");
|
|
});
|
|
|
|
it.each([
|
|
{ providerLeaseId: "../other" }, { companyId: "" }, { environmentId: null }, { attemptId: "a".repeat(201) },
|
|
{ runId: "other/run" }, { observedProviderLeaseId: "../other" }, { accountFingerprint: "secret-key" }, { labels: { apiKey: "secret-key" } },
|
|
{ labels: { "paperclip-provider": "Bearer secret-key" } }, { labels: [] },
|
|
{ labels: Object.fromEntries(Array.from({ length: 17 }, (_, i) => [`paperclip-${String.fromCharCode(97 + i)}`, "x"])) },
|
|
])("rejects malformed evidence %j", (invalid) => {
|
|
const cleanup = { ...ownership, ...invalid } as typeof ownership;
|
|
expect(readEnvironmentCreationCleanupError({ data: { schema, cleanup } })).toBeNull();
|
|
expect(environmentCreationCleanupErrorData(new PluginEnvironmentCreationCleanupError([], "invalid", cleanup))).toBeUndefined();
|
|
});
|
|
|
|
it.each([null, {}, { data: { schema: "unknown", cleanup: ownership } }, { data: { schema, cleanup: [] } }])("rejects malformed envelopes %j", (value) => {
|
|
expect(readEnvironmentCreationCleanupError(value)).toBeNull();
|
|
});
|
|
});
|
|
|
|
|
|
describe("acquisition failure observations", () => {
|
|
const diagnostic = { phase: "shell", elapsedMs: 300_000, budgetMs: 300_000 } as const;
|
|
|
|
it("crosses the real acquire RPC without provider causes or ownership in the diagnostic", async () => {
|
|
const input = { ...diagnostic, url: "private-url", command: "private-command", token: "private-token" };
|
|
const response = await invoke("environmentAcquireLease", new PluginEnvironmentCreationCleanupError(
|
|
[new Error("private-provider-cause")], "Cleanup required", ownership, input,
|
|
));
|
|
if (!("error" in response) || !response.error) throw new Error("Expected RPC error");
|
|
expect(response.error.data).toEqual({ schema, cleanup: ownership, acquisitionDiagnostic: diagnostic });
|
|
expect(readEnvironmentAcquisitionDiagnostic(new JsonRpcCallError(response.error))).toEqual(diagnostic);
|
|
expect(JSON.stringify(response)).not.toContain("private-");
|
|
// The old ownership reader and envelope version remain valid.
|
|
expect(readEnvironmentCreationCleanupError(new JsonRpcCallError(response.error))).toEqual(ownership);
|
|
expect(readEnvironmentAcquisitionDiagnostic({ data: { schema, cleanup: ownership } })).toBeNull();
|
|
});
|
|
|
|
it("carries a fixed-enum provider state and omits unknown or private states", async () => {
|
|
const withState = { ...diagnostic, phase: "create", providerState: "pulling_snapshot" } as const;
|
|
const response = await invoke("environmentAcquireLease", new PluginEnvironmentCreationCleanupError([], "Cleanup required", ownership, withState));
|
|
if (!("error" in response) || !response.error) throw new Error("Expected RPC error");
|
|
expect(readEnvironmentAcquisitionDiagnostic(new JsonRpcCallError(response.error))).toEqual(withState);
|
|
for (const providerState of ["private-state", "STARTED", 3, null]) {
|
|
const error = new PluginEnvironmentCreationCleanupError([], "Cleanup required", ownership,
|
|
{ ...withState, providerState } as unknown as typeof withState);
|
|
// An unrecognized optional state never invalidates the other observations.
|
|
expect(readEnvironmentAcquisitionDiagnostic(error)).toEqual({ ...diagnostic, phase: "create" });
|
|
expect(JSON.stringify(environmentCreationCleanupErrorData(error, true))).not.toContain("private-");
|
|
}
|
|
});
|
|
|
|
it.each(["environmentDestroyLease", "environmentProbe"])("omits acquisition diagnostics from %s", async method => {
|
|
const response = await invoke(method, new PluginEnvironmentCreationCleanupError([], "Cleanup required", ownership, diagnostic));
|
|
expect("error" in response && response.error).not.toHaveProperty("data.acquisitionDiagnostic");
|
|
if (method === "environmentDestroyLease") expect("error" in response && response.error?.data).toEqual({ schema, cleanup: ownership });
|
|
});
|
|
|
|
it.each([
|
|
null, [], {}, { ...diagnostic, phase: "cleanup" }, { ...diagnostic, phase: "private-command" },
|
|
{ ...diagnostic, elapsedMs: -1 }, { ...diagnostic, elapsedMs: NaN }, { ...diagnostic, elapsedMs: Infinity },
|
|
{ ...diagnostic, elapsedMs: 604_800_001 }, { ...diagnostic, elapsedMs: 0.5 }, { ...diagnostic, elapsedMs: "300000" },
|
|
{ ...diagnostic, budgetMs: 0 }, { ...diagnostic, budgetMs: 86_400_001 }, { ...diagnostic, budgetMs: Infinity },
|
|
{ ...diagnostic, budgetMs: "private-token" }, Object.create(diagnostic),
|
|
])("omits malformed optional diagnostics without rejecting cleanup (%j)", invalid => {
|
|
const error = new PluginEnvironmentCreationCleanupError([], "Cleanup required", ownership, invalid as typeof diagnostic);
|
|
expect(readEnvironmentAcquisitionDiagnostic(error)).toBeNull();
|
|
expect(environmentCreationCleanupErrorData(error, true)).toEqual({ schema, cleanup: ownership });
|
|
});
|
|
|
|
it("omits accessor diagnostics without invoking their getters", () => {
|
|
let reads = 0;
|
|
const invalid = Object.defineProperty({ ...diagnostic }, "phase", { get() { reads++; throw new Error("private-getter"); } });
|
|
const error = new PluginEnvironmentCreationCleanupError([], "Cleanup required", ownership, invalid);
|
|
expect(environmentCreationCleanupErrorData(error, true)).toEqual({ schema, cleanup: ownership });
|
|
expect(reads).toBe(0);
|
|
});
|
|
|
|
it("does not read an inherited or accessor acquisitionDiagnostic envelope field", () => {
|
|
let reads = 0;
|
|
const accessor = Object.defineProperty({ schema, cleanup: ownership }, "acquisitionDiagnostic", {
|
|
get() { reads++; throw new Error("private-getter"); },
|
|
});
|
|
const inherited = Object.assign(Object.create({ acquisitionDiagnostic: diagnostic }), { schema, cleanup: ownership });
|
|
for (const data of [accessor, inherited]) {
|
|
expect(readEnvironmentAcquisitionDiagnostic({ data })).toBeNull();
|
|
expect(readEnvironmentCreationCleanupError({ data })).toEqual(ownership);
|
|
}
|
|
const typed = new PluginEnvironmentCreationCleanupError([], "Cleanup required", ownership);
|
|
Object.defineProperty(typed, "acquisitionDiagnostic", { get() { reads++; throw new Error("private-getter"); } });
|
|
expect(environmentCreationCleanupErrorData(typed, true)).toEqual({ schema, cleanup: ownership });
|
|
expect(reads).toBe(0);
|
|
});
|
|
|
|
it("does not serialize forged error properties or accept invalid ownership with valid diagnostics", async () => {
|
|
const forged = Object.assign(new Error("Cleanup required"), { acquisitionDiagnostic: diagnostic,
|
|
data: { schema, cleanup: ownership, acquisitionDiagnostic: diagnostic } });
|
|
const response = await invoke("environmentAcquireLease", forged);
|
|
expect("error" in response && response.error).not.toHaveProperty("data");
|
|
expect(readEnvironmentAcquisitionDiagnostic({ data: { schema, cleanup: { ...ownership, runId: "private/path" }, acquisitionDiagnostic: diagnostic } })).toBeNull();
|
|
});
|
|
});
|