mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - After a restart, the control plane stops and keeps the sandbox of an orphaned run lease, then destroys it once nothing references it > - #19964 marked such a sandbox only inside its stop intent; #19976 added the top-level marker the destroy looks for > - A stop still pending when #19976 shipped lost the intent on success, so the destroy never saw that sandbox > - This pull request carries the orphan marker forward when a stop intent that says `recoveredOrphanedRunLease` is confirmed > - The benefit is that those sandboxes are destroyed like every other retained orphan ## Linked Issues or Issue Description Refs #19964 and #19976. This is a follow-up gap from the Phase 2 ownership migration. No duplicate PR was found. **What happened?** Between #19964 and #19976, the shipped builds were `canary/v2026.1108.0-canary.6` and `canary.7`. In that window the orphaned-run-lease settle wrote `recoveredOrphanedRunLease: true` only on the `sandboxStopAndRetain` intent. `settleStopOnlyCleanup` removes the intent when the stop is confirmed, and the retained-orphan kind lists only leases with the top-level `orphanedRunLeaseRecovery` marker. So an orphan whose stop was still pending at upgrade stays `released` with a stopped sandbox forever. **Expected behavior** Once its stop is confirmed and nothing references it, the orphan is destroyed through the retained-orphan settle, whichever build wrote the intent. **Steps to reproduce** 1. Write a `pending_cleanup` lease with a `sandboxStopAndRetain` intent that has `recoveredOrphanedRunLease: true` and no top-level marker (what canary.6/.7 wrote). 2. Confirm the stop through `settleStopOnlyCleanup`. 3. Run an ownership reconciliation pass. Before this PR the retained-orphan kind never lists the lease. **Paperclip version or commit** `master` before this PR. Rows written by `canary/v2026.1108.0-canary.6` and `canary.7`. ## What Changed - `settleStopOnlyCleanup` writes the top-level `orphanedRunLeaseRecovery` marker (`fromStopIntent: true`) when a confirmed stop's intent carries `recoveredOrphanedRunLease: true` and the lease has no marker yet. - An ordinary stop-and-retain (no recovery flag) is not marked. ## Verification - Red, before the change: "keeps the orphan marker for a stop intent saved before the top-level marker existed" failed. - Green: `environment-retained-orphan-sandbox` and `environment-run-lease-ownership` 36/36; `environment-runtime` and `heartbeat-pending-cleanup-sweep` 246/246. - Counter-case: an ordinary stop-and-retain intent stays unmarked, and the retained-orphan pass does not settle it. ## Risks - Not covered: an orphan whose stop was confirmed *while* canary.6/.7 ran kept no durable marker at all. Its intent was removed and the receipt never carried the flag, so it cannot be told apart from an intentional stop-and-retain. Those rows exist only on instances that ran those two canaries during about three hours, and the provider's own auto-delete bounds them. ## Model Used - Anthropic Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use. ## 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: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
1 parent
cefd17a82b
commit
ba867dea39
2 files changed
+37
-1
No files matched your search
@@ -21,7 +21,7 @@ import { heartbeatService, type HeartbeatEnvironmentRuntime } from "../services/
|
||||
import { hasRecoveredCleanupStopReceipt } from "../services/remote-execution-termination.js";
|
||||
import { taskChatRetryExecutionStopped } from "../services/chat-channels/task-retry-authority.js";
|
||||
import { createOwnershipReportState, runOwnershipReconciliationPass } from "../services/recovery/ownership-invariant-report.js";
|
||||
import { ORPHANED_SANDBOX_RECOVERY_KEY } from "../services/sandbox-stop-and-retain.js";
|
||||
import { ORPHANED_SANDBOX_RECOVERY_KEY, settleStopOnlyCleanup } from "../services/sandbox-stop-and-retain.js";
|
||||
|
||||
const support = await getEmbeddedPostgresTestSupport();
|
||||
const suite = support.supported ? describe : describe.skip;
|
||||
@@ -111,6 +111,37 @@ suite("retained orphan sandbox on the shared ownership path", () => {
|
||||
recoveredOrphanedRunLease: true, ownerProof: "pid_dead", runId });
|
||||
});
|
||||
|
||||
it("keeps the orphan marker for a stop intent saved before the top-level marker existed", async () => {
|
||||
const runId = await run("failed");
|
||||
const id = randomUUID(), providerLeaseId = `sandbox://fake/${id}`, attemptId = randomUUID();
|
||||
const intent = { schema: "paperclip.sandbox-stop-and-retain.v1", requestId: attemptId, companyId, runId, leaseId: id,
|
||||
provider: "fake", providerLeaseId, builtinProvider: "fake", recoveredOrphanedRunLease: true, ownerProof: "pid_dead" };
|
||||
await db.insert(environmentLeases).values({ id, companyId, environmentId, issueId, heartbeatRunId: runId,
|
||||
status: "pending_cleanup", cleanupStatus: "failed", failureReason: "sandbox_stop_pending", leasePolicy: "ephemeral",
|
||||
provider: "fake", providerLeaseId, metadata: { sandboxStopAndRetain: intent, pendingCleanupAttemptId: attemptId,
|
||||
pendingCleanupInFlight: true } });
|
||||
const settled = await settleStopOnlyCleanup(db, await row(id), { attemptId, receipt: { providerLeaseId, state: "stopped" } });
|
||||
expect(settled).toMatchObject({ status: "released", cleanupStatus: "success" });
|
||||
expect(settled?.metadata?.[ORPHANED_SANDBOX_RECOVERY_KEY]).toMatchObject({ recoveredOrphanedRunLease: true,
|
||||
ownerProof: "pid_dead", runId, fromStopIntent: true });
|
||||
const { kind } = await pass();
|
||||
expect(kind).toMatchObject({ settled: 1 });
|
||||
expect((await row(id))).toMatchObject({ status: "pending_cleanup", failureReason: "retained_orphan_sandbox_released" });
|
||||
});
|
||||
|
||||
it("does not mark an ordinary stop-and-retain as an orphan", async () => {
|
||||
const runId = await run("failed");
|
||||
const id = randomUUID(), providerLeaseId = `sandbox://fake/${id}`, attemptId = randomUUID();
|
||||
const intent = { schema: "paperclip.sandbox-stop-and-retain.v1", requestId: attemptId, companyId, runId, leaseId: id,
|
||||
provider: "fake", providerLeaseId, builtinProvider: "fake" };
|
||||
await db.insert(environmentLeases).values({ id, companyId, environmentId, issueId, heartbeatRunId: runId,
|
||||
status: "pending_cleanup", cleanupStatus: "failed", leasePolicy: "ephemeral", provider: "fake", providerLeaseId,
|
||||
metadata: { sandboxStopAndRetain: intent, pendingCleanupAttemptId: attemptId, pendingCleanupInFlight: true } });
|
||||
const settled = await settleStopOnlyCleanup(db, await row(id), { attemptId, receipt: { providerLeaseId, state: "stopped" } });
|
||||
expect(settled?.metadata?.[ORPHANED_SANDBOX_RECOVERY_KEY]).toBeUndefined();
|
||||
expect((await pass()).kind).toMatchObject({ settled: 0 });
|
||||
});
|
||||
|
||||
it("destroys a retained orphan that nothing references, through the cleanup sweep", async () => {
|
||||
const leaseId = await retainedOrphan(await run("failed"));
|
||||
const { kind } = await pass();
|
||||
|
||||
@@ -184,6 +184,10 @@ export async function settleStopOnlyCleanup(db: Db, lease: Lease, options: { att
|
||||
const receipt = remoteTerminationReceipt(lease, options.receipt), stopped = receipt?.state === "stopped";
|
||||
const now = new Date();
|
||||
const removeIntent: SQL = stopped ? sql`- 'sandboxStopAndRetain'` : sql``;
|
||||
const recordedIntent = lease.metadata?.[SANDBOX_STOP_AND_RETAIN_KEY] as Record<string, unknown> | undefined;
|
||||
const legacyOrphan = stopped && recordedIntent?.recoveredOrphanedRunLease === true && !lease.metadata?.[ORPHANED_SANDBOX_RECOVERY_KEY]
|
||||
? { [ORPHANED_SANDBOX_RECOVERY_KEY]: { recoveredOrphanedRunLease: true, ownerProof: recordedIntent.ownerProof ?? null,
|
||||
runId: lease.heartbeatRunId, recoveredAt: now.toISOString(), fromStopIntent: true } } : {};
|
||||
const [updated] = await writeEnvironmentCleanup(db, async tx => {
|
||||
if (stopped) await notifyRetainedNativeSessionCleanup(tx, { companyId: lease.companyId, runId: lease.heartbeatRunId! });
|
||||
return tx.update(environmentLeases).set({
|
||||
@@ -199,6 +203,7 @@ export async function settleStopOnlyCleanup(db: Db, lease: Lease, options: { att
|
||||
: { builtinProvider: lease.provider, method: "builtin.stopLease" }),
|
||||
confirmedAt: receipt.confirmedAt,
|
||||
} } : {}),
|
||||
...legacyOrphan,
|
||||
pendingCleanupInFlight: false, pendingCleanupLeaseExpiresAtMs: 0,
|
||||
})}::jsonb`,
|
||||
}).where(stopOnlyAttempt(lease, options.attemptId)).returning();
|
||||
|
||||
Reference in new issue
Block a user