mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-13 07:12:00 +01:00
fix(native): do not claim a server restart for a held same-run retry (#20832)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - Native runs on a remote sandbox (Daytona) retry provider startup on
the same run
> - A due remote same-run retry re-enters the restart-recovery
classifier
> - When that classifier holds the run, the hold note says "Automatic
recovery stopped after a server restart". No server restarted, so the
user reads a false cause
> - This pull request records the trigger of the hold and words a
same-run-retry hold as a stopped automatic retry
> - The benefit is that a held task names its real cause
## Linked Issues or Issue Description
Refs #20808
Refs #20820
**What happened?**
Two Codex runs on one Daytona sandbox failed in `session.open` (`bwrap:
loopback: Failed RTM_NEWADDR: Operation not permitted`). The executor
saved a `bootstrap_retry`. Thirty seconds later, the due retry entered
restart recovery and stopped with `ambiguous_remote_recovery_evidence`.
The task said "Automatic recovery stopped after a server restart". No
server restart happened. The second run was the board's Retry of the
first, and it failed the same way.
**Expected behavior**
A hold that a same-run retry creates must not claim a server restart.
**Steps to reproduce**
1. `cd server && npx vitest run
src/services/native-runtime/native-runner-restart-recovery.integration.test.ts
-t "same-run retry"`
2. On master the recovery action has no `trigger`, and its note contains
"server restart".
**Paperclip version or commit**
master at 5f38226290.
## What Changed
- Restart recovery records `trigger: "same_run_retry"` in the recovery
action evidence when the held run was a saved `retryable_failure`. It
reads this from the coordinator, so a deferred check or a check after a
real restart records the same trigger.
- `executionRecoverySettlementNote` says "Automatic retry of this run
stopped" for such a hold. It does not name a cause, because the
classifier can also stop it for `execution_recovery_budget_exhausted`.
Holds without the field keep the old text.
- Tests: one integration case with the staging signature and one unit
case for the note.
## Verification
- Red on master: the integration case fails because `evidence.trigger`
is absent; the unit case fails because the note says "server restart".
- Green here: `npx vitest run
src/services/execution-recovery-resolution.test.ts
src/services/native-runtime/native-runner-restart-recovery.integration.test.ts
-t "server restart|same-run retry|exited remote runner|remote bootstrap
retry held"`: 19 pass on the rebased head.
- `npx tsc --noEmit -p server/tsconfig.json`: clean.
## Risks
- Low risk. The change adds one optional evidence field and one note
branch. Recovery decisions do not change.
- Saved state: holds that an older release wrote have no `trigger` and
keep the old note. A mid-run deploy reads the field only when present.
- Not fixed here: the Codex Planning startup fault itself (`bwrap:
loopback: Failed RTM_NEWADDR`). #20820 changes the external-sandbox
planning profile so Codex no longer uses its network-isolated filesystem
helper.
> 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 (Anthropic) Opus 5.5, model ID `claude-opus-5-5`, run in Claude
Code with tool use (shell, file edit) and extended reasoning.
## 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 (no
doc change needed)
- [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
464e1de9b1
commit
3ded7bbd91
4 files changed
+37
-1
No files matched your search
@@ -9,6 +9,13 @@ describe("executionRecoverySettlementNote", () => {
|
|||||||
expect(note).toContain("after a server restart");
|
expect(note).toContain("after a server restart");
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("does not claim a server restart for a hold a same-run retry created", () => {
|
||||||
|
const note = executionRecoverySettlementNote({ current: true, cause: "native_restart_recovery_blocked",
|
||||||
|
workspaceRestoreFailed: false, trigger: "same_run_retry" });
|
||||||
|
expect(note).not.toContain("server restart");
|
||||||
|
expect(note).toContain("Automatic retry of this run stopped");
|
||||||
|
});
|
||||||
|
|
||||||
it("keeps the workspace repair note for other holds with a restore failure", () => {
|
it("keeps the workspace repair note for other holds with a restore failure", () => {
|
||||||
expect(executionRecoverySettlementNote({ current: true, cause: "execution_finalization_deadline_exceeded",
|
expect(executionRecoverySettlementNote({ current: true, cause: "execution_finalization_deadline_exceeded",
|
||||||
workspaceRestoreFailed: true })).toContain("Workspace repair required");
|
workspaceRestoreFailed: true })).toContain("Workspace repair required");
|
||||||
|
|||||||
@@ -46,8 +46,13 @@ import { alias } from "drizzle-orm/pg-core";
|
|||||||
*/
|
*/
|
||||||
export function executionRecoverySettlementNote(input: {
|
export function executionRecoverySettlementNote(input: {
|
||||||
current: boolean; cause: string; workspaceRestoreFailed: boolean;
|
current: boolean; cause: string; workspaceRestoreFailed: boolean;
|
||||||
|
/** The hold's recorded trigger; a same-run retry is not a server restart. */
|
||||||
|
trigger?: unknown;
|
||||||
}): string {
|
}): string {
|
||||||
if (!input.current) return "Recovery closed because the task's owner, execution, or status changed. No work was replayed.";
|
if (!input.current) return "Recovery closed because the task's owner, execution, or status changed. No work was replayed.";
|
||||||
|
if (input.cause === "native_restart_recovery_blocked" && input.trigger === "same_run_retry") {
|
||||||
|
return "Automatic retry of this run stopped. Saved work is preserved; actions with unverified outcomes will not be repeated.";
|
||||||
|
}
|
||||||
if (input.cause === "native_restart_recovery_blocked") {
|
if (input.cause === "native_restart_recovery_blocked") {
|
||||||
return "Automatic recovery stopped after a server restart because the previous execution could not be verified. Saved work is preserved; actions with unverified outcomes will not be repeated.";
|
return "Automatic recovery stopped after a server restart because the previous execution could not be verified. Saved work is preserved; actions with unverified outcomes will not be repeated.";
|
||||||
}
|
}
|
||||||
@@ -696,7 +701,7 @@ export async function settleUnrecoverableExecutions(
|
|||||||
(!task.executionRunId || task.executionRunId === run.id) &&
|
(!task.executionRunId || task.executionRunId === run.id) &&
|
||||||
(!task.checkoutRunId || task.checkoutRunId === run.id);
|
(!task.checkoutRunId || task.checkoutRunId === run.id);
|
||||||
const note = executionRecoverySettlementNote({ current, cause: action.cause,
|
const note = executionRecoverySettlementNote({ current, cause: action.cause,
|
||||||
workspaceRestoreFailed: hasWorkspaceRestoreFailure(run.resultJson) });
|
workspaceRestoreFailed: hasWorkspaceRestoreFailure(run.resultJson), trigger: action.evidence.trigger });
|
||||||
await notifyUnsafeWorkspaceExportRecovery(tx, { companyId: task.companyId, issueId: task.id });
|
await notifyUnsafeWorkspaceExportRecovery(tx, { companyId: task.companyId, issueId: task.id });
|
||||||
let nativeFailureBlock = action.evidence.nativeFailureBlock;
|
let nativeFailureBlock = action.evidence.nativeFailureBlock;
|
||||||
let nativeBootstrapFailureBlock = action.evidence.nativeBootstrapFailureBlock;
|
let nativeBootstrapFailureBlock = action.evidence.nativeBootstrapFailureBlock;
|
||||||
|
|||||||
@@ -1807,6 +1807,10 @@ export async function claimNativeRestartRecoveries(input: {
|
|||||||
evidence: {
|
evidence: {
|
||||||
runId: row.run.id,
|
runId: row.run.id,
|
||||||
reason,
|
reason,
|
||||||
|
// A saved retryable failure is a same-run retry, not a restart
|
||||||
|
// hold. Read from the coordinator so a deferred or post-restart
|
||||||
|
// check of that retry records the same trigger.
|
||||||
|
...(row.coordinator.phase === "retryable_failure" ? { trigger: "same_run_retry" } : {}),
|
||||||
providerAttempt: row.coordinator.attempt,
|
providerAttempt: row.coordinator.attempt,
|
||||||
...(hold ? { automaticAdmissionExhausted: true, executorStopEvidence: "unverified",
|
...(hold ? { automaticAdmissionExhausted: true, executorStopEvidence: "unverified",
|
||||||
providerPid: row.run.processPid, checkpointPreserved: hasCheckpoint } : {}),
|
providerPid: row.run.processPid, checkpointPreserved: hasCheckpoint } : {}),
|
||||||
|
|||||||
@@ -40,6 +40,7 @@ import {
|
|||||||
} from "@paperclipai/db";
|
} from "@paperclipai/db";
|
||||||
import { beforeAll, describe, expect, it, vi } from "vitest";
|
import { beforeAll, describe, expect, it, vi } from "vitest";
|
||||||
import { heartbeatService } from "../heartbeat.js";
|
import { heartbeatService } from "../heartbeat.js";
|
||||||
|
import { executionRecoverySettlementNote } from "../execution-recovery-resolution.js";
|
||||||
|
|
||||||
import {
|
import {
|
||||||
canonicalNativeRuntimeContextDigest,
|
canonicalNativeRuntimeContextDigest,
|
||||||
@@ -3156,6 +3157,25 @@ process.on("SIGTERM", () => child.kill("SIGTERM"));
|
|||||||
kind: "blocked", runId: fixture.runId, reason })]);
|
kind: "blocked", runId: fixture.runId, reason })]);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
it("records that a held same-run retry was not a server restart", async () => {
|
||||||
|
// Staging run 61d2e06e: a remote session.open rejection saved by an older
|
||||||
|
// release, held when its due retry re-entered restart recovery. No server
|
||||||
|
// restarted, so the hold must not say one did.
|
||||||
|
const fixture = await seedExitedRemoteRunner("SAME-RUN-RETRY-HOLD", {
|
||||||
|
originalFailureCode: "provider_session_open_rejected" }, 1);
|
||||||
|
expect(await claimNativeRestartRecoveries({ db: fixture.db, controller: successor, restartKind: "hard",
|
||||||
|
runIds: [fixture.runId] })).toEqual([expect.objectContaining({
|
||||||
|
kind: "blocked", runId: fixture.runId, reason: "ambiguous_remote_recovery_evidence" })]);
|
||||||
|
const [action] = await fixture.db.select().from(issueRecoveryActions)
|
||||||
|
.where(eq(issueRecoveryActions.sourceIssueId, fixture.issueId));
|
||||||
|
expect(action).toMatchObject({ cause: "native_restart_recovery_blocked",
|
||||||
|
evidence: expect.objectContaining({ trigger: "same_run_retry" }) });
|
||||||
|
const note = executionRecoverySettlementNote({ current: true, cause: action.cause,
|
||||||
|
workspaceRestoreFailed: false, trigger: action.evidence.trigger });
|
||||||
|
expect(note).not.toContain("server restart");
|
||||||
|
expect(note).toContain("Automatic retry of this run stopped");
|
||||||
|
});
|
||||||
|
|
||||||
it.each<[string, Parameters<typeof seedUnlaunchedRemoteClaim>[1], { kind: string; reason: string }]>([
|
it.each<[string, Parameters<typeof seedUnlaunchedRemoteClaim>[1], { kind: string; reason: string }]>([
|
||||||
["a runner process start was requested", { bootstrapRetry: true, event: { eventType: "native.process_start_requested" } },
|
["a runner process start was requested", { bootstrapRetry: true, event: { eventType: "native.process_start_requested" } },
|
||||||
{ kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }],
|
{ kind: "blocked", reason: "ambiguous_remote_recovery_evidence" }],
|
||||||
|
|||||||
Reference in new issue
Block a user