mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(native): retire a finished remote owner whose suspend never finished once its sandbox leases ended (#20211)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before a per-turn follow-up starts a new runner, it retires the previous turn's finished owner. Retirement requires the owner's controller journal to be settled > - A local runner that is proven stopped can no longer act on its own lifecycle commands, so an unfinished `runner.suspend` from it does not hold the follow-up > - On a remote per-turn sandbox the same thing happens when a restart or drain lands while the runner is suspending. But the remote path had no equivalent rule, so the unfinished `runner.suspend` held the follow-up even after the previous run's sandbox lease had ended > - This pull request treats a remote owner's lifecycle commands as settled once every sandbox lease of that run has been released or expired, and none of them is the sandbox the follow-up now uses > - The benefit is that a C1 follow-up after a restart on Daytona continues on its own ## Linked Issues or Issue Description Refs #20140 **What happened?** Daytona per-turn restart-chaos K13 storm w-20261109T074128-s9503 (revision9046884811, image 01246d43): 60 of 65 passed. Three C1 follow-ups failed with `runner_state_identity_mismatch: prior_owner_terminal_state_indeterminate; retirement_refused=journal_commands_unsettled(runner.suspend.pending)`: - t04 codex, run `c7446d0c`; - t16 claude, run `4b2c770f`; - t39 claude, run `3ef7d934`. For t04, the previous owner `fc585314` committed its `run.terminal`, and its journal kept only `runner.suspend` pending. Its per-turn lease (`5576f27d…`) expired at 07:43:52, and the follow-up ran on a new sandbox. Each Retry succeeded. **Expected behavior** Once the previous run's sandbox leases have all ended, and the follow-up runs on another sandbox, the previous runner cannot act on its suspend. The finished owner is retired and the follow-up continues. The owner stays held while any of these holds: - a lease is active, retained or pending cleanup; - the follow-up runs on the same sandbox; - no lease is recorded. **Steps to reproduce** 1. Finish a remote per-turn run whose controller journal keeps `runner.suspend` pending after its committed run terminal, and let its sandbox lease end. 2. Start a follow-up on a new sandbox. Before this change, retirement refuses with `journal_commands_unsettled(runner.suspend.pending)`. **Paperclip version or commit**9046884811(wave). Branch rebased on current master. ## What Changed - `native-session-executor.ts`: - the new `priorRemoteSandboxLeasesEnded` checks the previous run's `environment_leases`: at least one lease, every one `released` or `expired` with `releasedAt`, and none on the current target's provider lease; - `inspectFinishedPerTurnRunnerdOwner` passes the result as `remoteRunnerGone` to `readSettledRunnerdProjection`, for the live root and for quarantined copies. That function then ignores unfinished runner lifecycle commands (`runner.suspend`, `runner.drain`, `runner.shutdown`), exactly as it already does for a stopped local runner. - `native-recovered-owner-followup.test.ts`: a red/green test with lease variants. A lease that ended or was released on another sandbox is admitted. A retained lease, an ended lease on the same sandbox, or no lease record stays held. ## Verification - Red/green: on master, the two admitted cases fail with exactly the production error; with the change they are admitted. The three held cases hold on both. - `vitest run` of the recovered-owner, remote-warm and restart-recovery test files and `native-session-executor.test.ts`: 7 files, 1634 tests passed (14 skipped). - `PAPERCLIP_TYPECHECK_LOW_MEMORY=1 pnpm typecheck` (`server`): passed. ## Risks - Low risk. Only runner lifecycle commands become settled, and only when the database proves every sandbox of the previous run has ended and the follow-up runs elsewhere. Unfinished turn, tool or result commands, a missing run terminal and unanswered tool input still hold. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), via Claude Code with tool use (shell, file edits, test execution). ## 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
fc5c79a327
commit
b1049b174e
2 files changed
+45
-5
No files matched your search
@@ -670,6 +670,27 @@ describe.skipIf(!recoveryPostgresSupport.supported)("follow-up after a recovered
|
||||
expect(await archivedOwners(root)).toEqual([prior.runId]);
|
||||
}, 60_000);
|
||||
|
||||
it.each([
|
||||
{ label: "ended on another sandbox", status: "expired", sameSandbox: false, recorded: true, outcome: ADMITTED },
|
||||
{ label: "released on another sandbox", status: "released", sameSandbox: false, recorded: true, outcome: ADMITTED },
|
||||
{ label: "still retained", status: "retained", sameSandbox: false, recorded: true,
|
||||
outcome: stranded("journal_commands_unsettled(runner.suspend.pending)") },
|
||||
{ label: "ended on this sandbox", status: "expired", sameSandbox: true, recorded: true,
|
||||
outcome: stranded("journal_commands_unsettled(runner.suspend.pending)") },
|
||||
{ label: "not recorded", status: "expired", sameSandbox: false, recorded: false,
|
||||
outcome: stranded("journal_commands_unsettled(runner.suspend.pending)") },
|
||||
])("retires a finished remote owner whose suspend never finished when its sandbox lease is $label", async ({ status, sameSandbox, recorded, outcome }) => {
|
||||
const prior = await seedRecoveredPriorRun("pending-command");
|
||||
const followUp = await admitFollowUp(prior);
|
||||
if (recorded) {
|
||||
await db.insert(environments).values({ id: followUp.environmentId, name: `ended-${followUp.environmentId}`, driver: "sandbox" });
|
||||
await db.insert(environmentLeases).values({ companyId, environmentId: followUp.environmentId, issueId: prior.issueId,
|
||||
heartbeatRunId: prior.runId, provider: "daytona", providerLeaseId: sameSandbox ? followUp.providerLeaseId : `sandbox-${prior.runId}`,
|
||||
status, releasedAt: status === "retained" ? null : new Date(), metadata: { remoteCwd: "/workspace" } });
|
||||
}
|
||||
expect(await followUp.start()).toBe(outcome);
|
||||
}, 60_000);
|
||||
|
||||
it("admits again when a retry rebuilds the same projection from the database", async () => {
|
||||
const prior = await seedRecoveredPriorRun();
|
||||
const followUp = await admitFollowUp(prior);
|
||||
|
||||
@@ -5429,7 +5429,7 @@ type SettledRunnerdProjection = { bytes: string; refused?: never } | { bytes?: n
|
||||
const LOCAL_RUNNER_LIFECYCLE_COMMANDS = new Set(["runner.suspend", "runner.drain", "runner.shutdown"]);
|
||||
|
||||
function readSettledRunnerdProjection(root: string, identity: Record<string, string>,
|
||||
options: { localRunner?: boolean; controlBytes?: string } = {}): SettledRunnerdProjection {
|
||||
options: { localRunner?: boolean; controlBytes?: string; remoteRunnerGone?: boolean } = {}): SettledRunnerdProjection {
|
||||
const refuse = (refused: string): SettledRunnerdProjection => ({ refused });
|
||||
try {
|
||||
if (!isSafeNativeStateDirectory(root)) return refuse("root_unsafe");
|
||||
@@ -5449,7 +5449,7 @@ function readSettledRunnerdProjection(root: string, identity: Record<string, str
|
||||
const control = record(JSON.parse(bytes));
|
||||
const commands = (Array.isArray(control.commands) ? control.commands : []).map(record);
|
||||
// A stopped local Runner can no longer act on its own lifecycle commands.
|
||||
const settledCommands = options.localRunner && lifecycle === "not_suspended" && Array.isArray(control.commands)
|
||||
const settledCommands = ((options.localRunner && lifecycle === "not_suspended") || options.remoteRunnerGone) && Array.isArray(control.commands)
|
||||
? commands.filter(command => !LOCAL_RUNNER_LIFECYCLE_COMMANDS.has(String(command.type)) || command.status === "completed")
|
||||
: commands;
|
||||
const settledControl = settledCommands === commands ? control : { ...control, commands: settledCommands };
|
||||
@@ -5553,11 +5553,16 @@ async function inspectFinishedPerTurnRunnerdOwner(input: {
|
||||
let originalBytes: string;
|
||||
try { originalBytes = readBoundedNativeFile(statePath, NATIVE_CONTROL_PLANE_STATE_MAX_BYTES, "runner_durable_identity_too_large").toString("utf8"); }
|
||||
catch { return refuse("journal_unreadable"); }
|
||||
const projectionOptions = { localRunner, controlBytes: input.projectedControlBytes };
|
||||
const { binding } = input.execution;
|
||||
const remoteRunnerGone = !localRunner && await priorRemoteSandboxLeasesEnded(input.db, {
|
||||
companyId: binding.companyId, runId: identity.runId,
|
||||
currentProviderLeaseId: target?.kind === "remote" && target.transport === "sandbox"
|
||||
? target.sandboxLeaseAcquisition?.providerLeaseId : undefined,
|
||||
});
|
||||
const projectionOptions = { localRunner, controlBytes: input.projectedControlBytes, remoteRunnerGone };
|
||||
const projection = readSettledRunnerdProjection(input.root, identity, projectionOptions);
|
||||
if (projection.refused !== undefined) return refuse(projection.refused);
|
||||
const controlBytes = projection.bytes;
|
||||
const { binding } = input.execution;
|
||||
let prior: { run: typeof heartbeatRuns.$inferSelect; owner: typeof nativeRunFinalizations.$inferSelect } | undefined;
|
||||
try {
|
||||
[prior] = await input.db.select({ run: heartbeatRuns, owner: nativeRunFinalizations }).from(heartbeatRuns)
|
||||
@@ -5619,7 +5624,7 @@ async function inspectFinishedPerTurnRunnerdOwner(input: {
|
||||
for (const name of readdirSync(quarantine)) {
|
||||
if (!name.startsWith(`${basename(input.root)}.`)) continue;
|
||||
if (quarantinedBootstrapOfOwnerHeldNoProviderWork(resolve(quarantine, name), identity, localRunner)) continue;
|
||||
const copy = readSettledRunnerdProjection(resolve(quarantine, name), identity, { localRunner });
|
||||
const copy = readSettledRunnerdProjection(resolve(quarantine, name), identity, { localRunner, remoteRunnerGone });
|
||||
if (copy.refused !== undefined) return refuse(`quarantine_${copy.refused}`);
|
||||
}
|
||||
}
|
||||
@@ -5643,6 +5648,20 @@ export function quarantinedBootstrapOfOwnerHeldNoProviderWork(root: string, iden
|
||||
|| (!localRunner && runnerdStateProvesNoRemoteProviderTurn(root));
|
||||
}
|
||||
|
||||
async function priorRemoteSandboxLeasesEnded(db: Db, input: {
|
||||
companyId: string; runId: string; currentProviderLeaseId?: string;
|
||||
}): Promise<boolean> {
|
||||
try {
|
||||
const leases = await db.select({ status: environmentLeases.status, releasedAt: environmentLeases.releasedAt,
|
||||
providerLeaseId: environmentLeases.providerLeaseId }).from(environmentLeases)
|
||||
.where(and(eq(environmentLeases.companyId, input.companyId), eq(environmentLeases.heartbeatRunId, input.runId)));
|
||||
return leases.length > 0 && leases.every(lease => (lease.status === "released" || lease.status === "expired")
|
||||
&& lease.releasedAt !== null && lease.providerLeaseId !== input.currentProviderLeaseId);
|
||||
} catch {
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
async function retireFinishedPerTurnRunnerdOwner(input: Parameters<typeof inspectFinishedPerTurnRunnerdOwner>[0])
|
||||
: Promise<{ retired: true } | { retired: false; refused: string }> {
|
||||
const proof = await inspectFinishedPerTurnRunnerdOwner(input);
|
||||
|
||||
Reference in new issue
Block a user