mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(native): retire a finished remote owner whose gone Runner left a delivery unacknowledged (#20677)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A native follow-up run on a remote sandbox first proves that the previous per-turn owner finished. Then it retires that owner's controller journal and starts a new Runner. > - The proof requires a settled journal. A command that the Runner did not acknowledge holds the owner. > - On staging, a per-turn owner committed its result and its Daytona sandbox was destroyed after the turn (`paperclip_runner_destroy_after_turn`, cleanup `success`). Its journal still held a `run.attach` or a `semantic_tool.result` that the Runner never acknowledged. > - The next run failed with `prior_owner_terminal_state_indeterminate; retirement_refused=journal_commands_unsettled(run.attach.pending)` (or `semantic_tool.result.pending`). A Retry or a new message gets the same journal, so it fails the same way. The task stays blocked. > - Retirement already excuses the Runner's own lifecycle commands when the prior sandbox lease ended on another sandbox. This pull request applies the same rule to deliveries that only that gone Runner could receive. > - The benefit is that these follow-ups start by themselves. Commands that carry user intent still hold the owner. ## Linked Issues or Issue Description **What happened?** A census of staging failures (48 h, Nov 9–10) found 6 failed runs with `prior_owner_terminal_state_indeterminate` and `retirement_refused=journal_commands_unsettled(run.attach.pending | semantic_tool.result.pending)`. The last one was at 18:56 UTC on Nov 10. In two examples that I checked: - The previous run succeeded. Its result was committed and its coordinator lease was released. - Its lease policy was `ephemeral`. The lease is `expired`, with `failureReason=paperclip_runner_destroy_after_turn` and `cleanupStatus=success`. - The follow-up got a new sandbox (a different provider lease) and refused in about 11 s. It started no Runner. The task became `blocked`, and the next action was "Send a message to continue". **Expected behavior** When the record proves that the previous Runner is gone, an undelivered attachment, tool answer or read cannot run. The follow-up retires the finished owner and starts a new Runner. Retirement replays no command. **Steps to reproduce** 1. Finish a per-turn remote native run whose controller journal keeps a `run.attach` or `semantic_tool.result` command in `pending`. 2. Let the sandbox lease of that run end (`expired` or `released`) on its own sandbox. 3. Start a follow-up run that gets a new sandbox. 4. Observe `retirement_refused=journal_commands_unsettled(run.attach.pending)`. **Paperclip version or commit** Staging builds of Nov 10. The refusal still reproduces on `master` at `48c6e996da` (the new tests fail there). ## What Changed - `native-session-executor.ts` (`readSettledRunnerdProjection`): when the prior run's sandbox leases ended on another sandbox (`remoteRunnerGone`, the existing proof), unfinished `run.attach`, `semantic_tool.result`, `session.snapshot` and `session.goal.get` commands no longer hold the retirement. These join the lifecycle commands that the rule already excused. - In the same case, a journaled `semantic_tool.result` answers its `semantic_tool.input`. The controller journals the answer after the tool ran. Only its delivery was outstanding. - No other proof changes. The prior run must still be terminal, with a committed result, a released coordinator lease, no recovery state, a stopped or fenced controller, a delivered `run.terminal`, and no live work after it. Quarantine copies must pass the same checks. - `doc/execution-semantics.md`: describe the rule. ## Verification - `server/src/services/native-runtime/native-recovered-owner-followup.test.ts` (real Postgres): - New: "retires a finished remote owner with an unacknowledged pending-attach / pending-tool-result when its sandbox lease ended on another sandbox". It fails on `master` with the exact staging errors `journal_commands_unsettled(run.attach.pending)` and `journal_commands_unsettled(semantic_tool.result.pending)`. It passes with this change, and the log shows the retirement. - Held cases stay held: a lease that is still retained, a lease on this run's sandbox, no lease record, and a tool input with no journaled answer (`journal_tool_input_unanswered`). - New: "keeps a stopped local owner held by an unacknowledged run.attach". A local Runner is not gone by record. - Whole file: 95 passed. - Neighbouring suites pass: `explicit-native-continuation.test.ts` and `native-controller-recovery.integration.test.ts` (460 tests). - `npx tsc --noEmit -p server/tsconfig.json` passes. - `pnpm verify` exits 2: a global CI input changed (`doc/execution-semantics.md`). Required CI supplies the broad coverage. - Live proof: not done. The redeploy chaos scenario (K15) is not on `master` yet. Tests only. ## Risks - The rule applies only when every sandbox lease of the prior run is `released` or `expired`, has a release time, and names a different provider lease than this run's. A reusable sandbox that this run resumed keeps the existing hold. - A command that carries user intent (`turn.start`, `turn.steer`, `request.resolve`, `interaction.receipt`, `external_provider.operation`, `session.goal.set`/`clear`, `session.close`, `run.cancel` and others) still holds the owner. - Retirement archives the old journal under `continuity-breaks` and replays nothing, as before. - No persisted shape changes. Old saved journals are read by the same rule. > 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 - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, used through Claude Code with tool use and code 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 <noreply@anthropic.com>
This commit is contained in:
1 parent
ad0543ae90
commit
cec903c6f0
3 files changed
+85
-4
No files matched your search
@@ -3258,6 +3258,26 @@ admission applies the same checks as for a per-turn owner. The journal of a
|
||||
warm owner has no `runner.suspend` command. A local warm Runner and a resumed
|
||||
sandbox that still holds the Runner keep the live adoption path.
|
||||
|
||||
A remote Runner whose sandbox lease for the prior run ended (`released` or
|
||||
`expired`, with a release time) on a different sandbox than this run's is gone
|
||||
by record. No process remains that could act on a command it never
|
||||
acknowledged. For such an owner, these unfinished commands do not hold the
|
||||
retirement:
|
||||
|
||||
- its own lifecycle commands (`runner.suspend`, `runner.drain`,
|
||||
`runner.shutdown`);
|
||||
- deliveries that only that Runner could receive: `run.attach`,
|
||||
`semantic_tool.result`, `session.snapshot` and `session.goal.get`. A
|
||||
journaled `semantic_tool.result` also answers its tool input, because the
|
||||
controller journals the answer after the tool ran.
|
||||
|
||||
Retirement replays none of these commands. Any other unfinished command, such
|
||||
as `turn.start`, `turn.steer`, `request.resolve` or
|
||||
`external_provider.operation`, still holds the owner. A tool input with no
|
||||
journaled answer still holds it with `journal_tool_input_unanswered`. A
|
||||
retained lease, a lease on this run's sandbox, or a missing lease record keeps
|
||||
every unfinished command decisive.
|
||||
|
||||
The provider session then follows the normal resume-or-replace policy. A
|
||||
resumed sandbox, a local warm session, a plugin task, a restart claim, or any
|
||||
missing proof keeps the existing `prior_owner_terminal_state_indeterminate`
|
||||
|
||||
@@ -92,7 +92,7 @@ describe.skipIf(!recoveryPostgresSupport.supported)("follow-up after a recovered
|
||||
* Only the database controller checkpoint and recovery pointer survive. */
|
||||
async function seedRecoveredPriorRun(control: "settled" | "pending-command" | "unanswered-tool"
|
||||
| "failed-turn-start" | "rejected-turn-start" | "indeterminate-turn-start" | "unknown-outcome-turn-start"
|
||||
| "unsuperseded-unknown-outcome-turn-start" | "failed-suspend" = "settled",
|
||||
| "unsuperseded-unknown-outcome-turn-start" | "failed-suspend" | "pending-attach" | "pending-tool-result" = "settled",
|
||||
options: { controller?: { bootId: string; pid: number; processStartedAt: Date }; provider?: "codex" | "acpx" | "opencode";
|
||||
lifecycle?: "warm" } = {}) {
|
||||
const issueId = randomUUID(), workspaceId = randomUUID(), runId = randomUUID();
|
||||
@@ -150,10 +150,16 @@ describe.skipIf(!recoveryPostgresSupport.supported)("follow-up after a recovered
|
||||
Object.assign(commands.find(command => command.type === "turn.start")!, unknownOutcome);
|
||||
}
|
||||
if (control === "failed-suspend") Object.assign(commands.at(-1)!, { status: "failed", result: { status: "failed" } });
|
||||
// Staging PAP-4196 and PAP-3443: the controller journaled a delivery to the
|
||||
// per-turn Runner that the Runner never acknowledged before its sandbox was
|
||||
// destroyed after the turn.
|
||||
if (control === "pending-attach") controller.queueCommand("run.attach", { provider: "fixture" });
|
||||
const toolInput = control === "unanswered-tool" || control === "pending-tool-result";
|
||||
if (control === "pending-tool-result") controller.queueCommand("semantic_tool.result", { sourceEventId: "unanswered-tool-input" });
|
||||
const event = (sourceSeq: number, sourceEventId: string, eventType: string) => ({ sourceSeq, sourceEventId, eventType,
|
||||
priority: 0 as const, envelope: { ...identity }, deliveryCount: 1, logicalEffectCount: 1 });
|
||||
controller.store.state.committedEvents.push(
|
||||
...(control === "unanswered-tool" ? [event(1, "unanswered-tool-input", "semantic_tool.input")] : []),
|
||||
...(toolInput ? [event(1, "unanswered-tool-input", "semantic_tool.input")] : []),
|
||||
event(2, "recovered-terminal", "run.terminal"));
|
||||
controller.store.state.ackedSourceSeq = 2;
|
||||
controller.setCheckpointPersistence(nativeRunnerCheckpointWriter(db, owner));
|
||||
@@ -760,6 +766,38 @@ describe.skipIf(!recoveryPostgresSupport.supported)("follow-up after a recovered
|
||||
expect(await followUp.start()).toBe(outcome);
|
||||
}, 60_000);
|
||||
|
||||
// Staging PAP-4196 (run.attach) and PAP-3443 (semantic_tool.result): the
|
||||
// per-turn owner committed its result and its sandbox was destroyed after the
|
||||
// turn, but its journal still held a delivery that its Runner never
|
||||
// acknowledged. No process remains to receive it, so the follow-up retires
|
||||
// the owner. A Runner whose sandbox may still exist keeps the owner held.
|
||||
it.each([
|
||||
{ control: "pending-attach", label: "ended on another sandbox", status: "expired", sameSandbox: false, recorded: true,
|
||||
outcome: ADMITTED },
|
||||
{ control: "pending-tool-result", label: "ended on another sandbox", status: "expired", sameSandbox: false, recorded: true,
|
||||
outcome: ADMITTED },
|
||||
{ control: "pending-attach", label: "still retained", status: "retained", sameSandbox: false, recorded: true,
|
||||
outcome: stranded("journal_commands_unsettled(run.attach.pending)") },
|
||||
{ control: "pending-tool-result", label: "ended on this sandbox", status: "expired", sameSandbox: true, recorded: true,
|
||||
outcome: stranded("journal_commands_unsettled(semantic_tool.result.pending)") },
|
||||
{ control: "pending-tool-result", label: "not recorded", status: "expired", sameSandbox: false, recorded: false,
|
||||
outcome: stranded("journal_commands_unsettled(semantic_tool.result.pending)") },
|
||||
{ control: "unanswered-tool", label: "ended on another sandbox", status: "expired", sameSandbox: false, recorded: true,
|
||||
outcome: stranded("journal_tool_input_unanswered") },
|
||||
] as const)("retires a finished remote owner with an unacknowledged $control when its sandbox lease $label",
|
||||
async ({ control, status, sameSandbox, recorded, outcome }) => {
|
||||
const prior = await seedRecoveredPriorRun(control, { provider: "opencode" });
|
||||
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);
|
||||
if (outcome === ADMITTED) expect(followUp.logs.join("")).toContain("finished without a saved runner checkpoint");
|
||||
}, 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);
|
||||
@@ -1072,6 +1110,17 @@ describe.skipIf(!recoveryPostgresSupport.supported)("follow-up after a recovered
|
||||
expect(await followUp.startLocal()).toBe(stranded("journal_commands_unsettled(turn.start.pending)"));
|
||||
}, 60_000);
|
||||
|
||||
it("keeps a stopped local owner held by an unacknowledged run.attach", async () => {
|
||||
// Only a remote Runner whose sandbox lease ended elsewhere is gone by record.
|
||||
const { followUp, root } = await seedUnsuspendedLocalOwner(await stoppedRunnerProcess());
|
||||
const path = join(root, "control-plane", "control-plane-state.json");
|
||||
const state = JSON.parse(await readFile(path, "utf8"));
|
||||
state.commands.push({ ...state.commands.at(-1), commandId: "command_unacknowledged_attach",
|
||||
controllerSeq: state.commands.length + 1, type: "run.attach", status: "pending", result: null });
|
||||
await writeFile(path, JSON.stringify(state));
|
||||
expect(await followUp.startLocal()).toBe(stranded("journal_commands_unsettled(run.attach.pending)"));
|
||||
}, 60_000);
|
||||
|
||||
it.each(["uncommitted result", "committed without result", "live controller lease", "unexpired controller lease", "recovery pending", "pending command",
|
||||
"unanswered tool input", "foreign quarantine"] as const)("keeps the prior owner held with %s", async scenario => {
|
||||
const prior = await seedRecoveredPriorRun(scenario === "pending command" ? "pending-command"
|
||||
|
||||
@@ -5668,6 +5668,12 @@ function moveNativeContinuityState(root: string, archiveRoot: string): void {
|
||||
type SettledRunnerdProjection = { bytes: string; refused?: never } | { bytes?: never; refused: string };
|
||||
|
||||
const LOCAL_RUNNER_LIFECYCLE_COMMANDS = new Set(["runner.suspend", "runner.drain", "runner.shutdown"]);
|
||||
/** Commands that only deliver something to the Runner that held this journal:
|
||||
* its attachment, an answer to a tool it already called, or a read. None
|
||||
* carries new user intent, and none can run without that Runner. When its
|
||||
* sandbox lease ended on another sandbox, no process remains to receive them. */
|
||||
const GONE_REMOTE_RUNNER_DELIVERY_COMMANDS = new Set(["run.attach", "semantic_tool.result", "session.snapshot",
|
||||
"session.goal.get"]);
|
||||
|
||||
function readSettledRunnerdProjection(root: string, identity: Record<string, string>,
|
||||
options: { localRunner?: boolean; controlBytes?: string; remoteRunnerGone?: boolean } = {}): SettledRunnerdProjection {
|
||||
@@ -5690,12 +5696,18 @@ 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.
|
||||
// A remote Runner whose sandbox lease ended elsewhere also cannot receive
|
||||
// a delivery that it never acknowledged.
|
||||
const goneCommandTypes = options.remoteRunnerGone ? new Set([...LOCAL_RUNNER_LIFECYCLE_COMMANDS, ...GONE_REMOTE_RUNNER_DELIVERY_COMMANDS])
|
||||
: LOCAL_RUNNER_LIFECYCLE_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.filter(command => !goneCommandTypes.has(String(command.type)) || command.status === "completed")
|
||||
: commands;
|
||||
const settledControl = settledCommands === commands ? control : { ...control, commands: settledCommands };
|
||||
// The controller journals a tool's answer after the tool ran. Only its
|
||||
// delivery can be outstanding, and only to a Runner that is gone.
|
||||
const answered = new Set(commands
|
||||
.filter(command => command.type === "semantic_tool.result" && command.status === "completed")
|
||||
.filter(command => command.type === "semantic_tool.result" && (command.status === "completed" || options.remoteRunnerGone))
|
||||
.map(command => record(command.payload).sourceEventId));
|
||||
if (!RUNNERD_CONTROL_PLANE_STATE_SCHEMAS.has(String(control.schema))) return refuse("journal_schema_unsupported");
|
||||
if (control.warmTransition != null) return refuse("journal_warm_transition");
|
||||
|
||||
Reference in new issue
Block a user