mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-10 12:37:55 +01:00
fix(recovery): continue a run refused by an inert cleanup quarantine with no click (#20260)
## Thinking Path
> - Paperclip is the open source app people use to manage AI agents for
work
> - When a native session's cleanup fails unrecoverably, the runtime
quarantines it in process. The next run for that task is refused with
`native_session_cleanup_quarantined`.
> - A board Retry or a new message already recovers from this.
`verifyStoppedNativeSessionForContinuation` proves that the old runner
and providers stopped, releases the quarantine and starts a fresh
session.
> - Without a click nothing recovers. The automatic safe-replacement
sweep proves stopped sessions only with the Codex replacement verifier,
which needs the failed run's own active turn. A run refused before
launch never has one, so the sweep skips it on every pass.
> - Restarts must recover on their own. This pull request lets the sweep
use the same continuation proof for a cleanup-quarantined run.
> - The benefit is that an inert quarantine turns into a fresh-session
continuation with no click, and an unproven one still keeps its board
Retry.
## Linked Issues or Issue Description
**What happened?**
In restart-chaos wave seed 9802 (local, G1, Claude), task t02's run
71f03e16 succeeded, but its runner cleanup could not prove it drained
and was quarantined. Follow-up run 0b4b404f was refused before launch
with `native_session_cleanup_quarantined`, and the task showed
"Automatic recovery stopped" until someone pressed Retry.
**Expected behavior**
When the quarantined session's owner run and every provider process are
proven stopped, recovery continues the task by itself in a fresh
session, without charging an attempt. When that cannot be proven, the
board Retry stays available.
**Steps to reproduce**
1. Have a native run succeed while its runner cleanup is quarantined.
2. Let the next run for the task be refused with
`native_session_cleanup_quarantined` before it launches.
3. Stop the old runner and provider processes, then run the
safe-replacement sweep. No successor is created.
**Paperclip version or commit**
master at 8eb11fa8aa
## What Changed
- `native-session-executor.ts`: new
`verifyStoppedNativeSessionForAutomaticReplacement`. It returns the
existing replacement proof, or, for a
`native_session_cleanup_quarantined` run with no replacement proof, the
continuation proof that the board Retry already uses (Codex and ACPX,
including an inherited owner run).
- `server/src/index.ts`: the safe-replacement sweep uses that verifier.
- The existing sweep then retires the quarantine, records
`native.stopped_text_turn_verified` and queues a successor with
`forceFreshSession: true` at the failed run's attempt number, so no
attempt is charged.
- `quarantined-task-retry.test.ts`: "recovers a quarantined $provider
source that never launched with no click…" for Codex and ACPX (Claude).
A stopped owner makes the sweep queue one fresh-session successor at the
same attempt. A running owner (Codex) or a live ACPX agent process
queues nothing and leaves the board Retry in place.
## Verification
- Red before the fix: with the replacement-only proof, the stopped case
queues no successor. It passes with the fallback, and the running-owner
case passes on both.
- `quarantined-task-retry` (35), `native-safe-replacement`,
`native-replacement-evidence`: pass.
- `PAPERCLIP_TYPECHECK_LOW_MEMORY=1 pnpm typecheck` in `server`: passes.
## Risks
- Low to medium. The fallback applies only to
`native_session_cleanup_quarantined` runs. It uses the stricter
continuation proof that already gates the board Retry: owner run
settled, local leases released, runner and every recorded provider pid
absent, and an unchanged retained state. The sweep keeps every other
gate (task ownership, failure-block receipt, attempt limit, effect
inventory).
- Previous-release story: no schema or saved-state change.
## Model Used
- Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via 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
- [x] All Paperclip CI gates are green
- [x] 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
1a007c4ef1
commit
68253e3175
3 files changed
+73
-10
No files matched your search
+2
-2
@@ -85,7 +85,7 @@ import { sentryReady, shutdownSentry, captureException } from "./sentry.js";
|
||||
import { setSentryRunLinkResolver } from "./sentry-links.js";
|
||||
import { createSentryRunLinkResolver } from "./services/sentry-run-links.js";
|
||||
import { waitForPendingRunFailureReports } from "./services/run-failure-report.js";
|
||||
import { verifyStoppedNativeSessionForReplacement } from "./services/native-runtime/native-session-executor.js";
|
||||
import { verifyStoppedNativeSessionForAutomaticReplacement } from "./services/native-runtime/native-session-executor.js";
|
||||
import { registerNativeCleanupReleaseWake } from "./services/native-runtime/native-cleanup-release-wake.js";
|
||||
import { logNativeCleanupQuarantines } from "./services/native-runtime/native-cleanup-quarantine-log.js";
|
||||
import { embeddedPostgresOwnerPort } from "./embedded-postgres-owner.js";
|
||||
@@ -1396,7 +1396,7 @@ async function startServerWithDatabaseTeardown(
|
||||
{ queue: DELIVERY_QUEUES.executionStatus, run: () => deliverExecutionStatuses(db), next: executionSchedule.status },
|
||||
{ queue: DELIVERY_QUEUES.executionDisposition, run: () => settleUnrecoverableExecutions(db), next: executionSchedule.disposition },
|
||||
...(heartbeat ? [
|
||||
{ queue: DELIVERY_QUEUES.executionReplacement, run: () => reconcileSafeNativeReplacements(db, new Date(), { verifyStoppedSession: run => verifyStoppedNativeSessionForReplacement(db, run) }), next: executionSchedule.replacement, onSettled: () => invalidateNativeReplacementDiscovery(db), settleEmptyWhilePaused: true },
|
||||
{ queue: DELIVERY_QUEUES.executionReplacement, run: () => reconcileSafeNativeReplacements(db, new Date(), { verifyStoppedSession: run => verifyStoppedNativeSessionForAutomaticReplacement(db, run) }), next: executionSchedule.replacement, onSettled: () => invalidateNativeReplacementDiscovery(db), settleEmptyWhilePaused: true },
|
||||
{ queue: DELIVERY_QUEUES.executionContinuation, run: async () => {
|
||||
await deliverReconciledExecutions(db, heartbeat.wakeup);
|
||||
await deliverPendingPauseResumes(db, heartbeat.wakeup);
|
||||
|
||||
@@ -3426,6 +3426,15 @@ export async function verifyStoppedNativeSessionForReplacement(
|
||||
} catch { return null; }
|
||||
}
|
||||
|
||||
export async function verifyStoppedNativeSessionForAutomaticReplacement(
|
||||
db: Db,
|
||||
run: typeof heartbeatRuns.$inferSelect,
|
||||
): Promise<{ evidence: Record<string, unknown>; retire: () => boolean } | null> {
|
||||
const replacement = await verifyStoppedNativeSessionForReplacement(db, run);
|
||||
if (replacement || run.errorCode !== "native_session_cleanup_quarantined") return replacement;
|
||||
return verifyStoppedNativeSessionForContinuation(db, run);
|
||||
}
|
||||
|
||||
/** Exact local cleanup only: the accepted result and original quarantine are
|
||||
* never rewritten. A failed/interrupted maintenance attempt is retained for
|
||||
* inspection, not retried from an older snapshot with unknown process owners. */
|
||||
|
||||
@@ -19,7 +19,8 @@ import { instanceSettingsService } from "./instance-settings.js";
|
||||
import { PROCESS_IDENTITY_RECORDED, PROCESS_START_REQUESTED } from "./native-local-process-stop.js";
|
||||
import { currentProcessStartedAtMs } from "./controller-process-identity.js";
|
||||
import { nativeSha256 } from "./native-runtime/canonical.js";
|
||||
import { verifyStoppedNativeSessionForContinuation } from "./native-runtime/native-session-executor.js";
|
||||
import { scopedRunnerdStateRoot, verifyStoppedNativeSessionForAutomaticReplacement, verifyStoppedNativeSessionForContinuation } from "./native-runtime/native-session-executor.js";
|
||||
import { reconcileSafeNativeReplacements } from "./native-runtime/native-safe-replacement.js";
|
||||
import { requestTaskRetry } from "./task-retry.js";
|
||||
import { DELIVERY_QUEUES, subscribeDeliveryWork } from "./delivery-work-notifications.js";
|
||||
import { registerNativeCleanupReleaseWake } from "./native-runtime/native-cleanup-release-wake.js";
|
||||
@@ -34,16 +35,20 @@ describe("task Retry after inherited native cleanup quarantine", () => {
|
||||
}, 90_000);
|
||||
afterAll(async () => { await db?.$client.end(); await postgres?.cleanup(); });
|
||||
|
||||
async function seed(base: string, transient = false) {
|
||||
async function seed(base: string, transient = false, provider: "codex" | "acpx" = "codex") {
|
||||
const companyId = randomUUID(), agentId = randomUUID(), issueId = randomUUID();
|
||||
const ownerId = randomUUID(), sourceId = randomUUID(), nativeSessionId = randomUUID(), runnerInstanceId = randomUUID();
|
||||
const workspaceId = randomUUID();
|
||||
const execution = (runId: string) => parseNativeExecutionInput({
|
||||
schema: "paperclip.native-execution-input.v1", provider: { kind: "codex", model: null },
|
||||
schema: "paperclip.native-execution-input.v1",
|
||||
provider: provider === "acpx" ? { kind: "acpx", agent: "claude", model: "claude-sonnet-5", permissionPolicy: "interactive",
|
||||
profile: { driverKind: "acpx_runtime", protocolVersion: 1, acpxVersion: "0.13.1", agent: "claude", agentProfileVersion: 1,
|
||||
agentServerPackage: "@zed-industries/claude-agent-acp", agentServerVersion: "1", agentRuntimePackage: null,
|
||||
agentRuntimeVersion: null, commandDigest: "fixture" } } : { kind: "codex", model: null },
|
||||
binding: { companyId, agentId, issueId, runId, executionWorkspaceId: transient ? runId : workspaceId },
|
||||
task: { identifier: "TEST", title: "Continue saved work", description: null, prompt: "Continue saved work", workMode: "standard" },
|
||||
workspace: { cwd: base, repoUrl: null, repoRef: null, branchName: null },
|
||||
session: { normalizedSessionId: nativeSessionId, driverKind: "codex_app_server", protocolVersion: 1,
|
||||
session: { normalizedSessionId: nativeSessionId, driverKind: provider === "acpx" ? "acpx_runtime" : "codex_app_server", protocolVersion: 1,
|
||||
lifecyclePolicy: { mode: "per_turn", idleTimeoutMs: null } },
|
||||
completionContract: { id: "contract", sha256: "sha", schemaVersion: "paperclip.completion-contract.v1",
|
||||
contract: { revision: "1", objective: "Continue saved work", criteria: [{ id: "done", requirement: "Save the result" }] } },
|
||||
@@ -96,16 +101,21 @@ describe("task Retry after inherited native cleanup quarantine", () => {
|
||||
await appendHeartbeatRunEvent(db, { companyId, runId: ownerId, agentId, eventType: event.eventType,
|
||||
payload: { prpEvent: normalized }, nativeSource: { sourceInstanceId: runnerInstanceId,
|
||||
sourceEventId: normalized.sourceEventId, sourceSeq: 1, protocolSchemaVersion: 1, canonicalPayload: normalized } });
|
||||
const root = join(base, nativeSha256({ schema: "paperclip.native-session-scope.v2", companyId, agentId,
|
||||
const root = provider === "acpx" ? scopedRunnerdStateRoot(execution(ownerId)) : join(base, nativeSha256({ schema: "paperclip.native-session-scope.v2", companyId, agentId,
|
||||
workspace: transient ? { kind: "transient", cwd: base, repoUrl: null, repoRef: null, branchName: null }
|
||||
: { kind: "managed", executionWorkspaceId: workspaceId },
|
||||
provider: { driverKind: "codex_app_server", identity: { kind: "codex" } }, normalizedSessionId: nativeSessionId }));
|
||||
const files = {
|
||||
const files: Record<string, Record<string, unknown>> = {
|
||||
"control-plane/control-plane-state.json": { schema: "paperclip.runner.durable.control-plane-state.v1", identity,
|
||||
committedEvents: [{ envelope: { payload: event } }], commands: [{ status: "pending", payload: { commandType: "tool.result" } }] },
|
||||
"runner/runner-state.json": { schema: "paperclip.runner.durable.state.v1", ...identity, lifecycle: "suspended", outbox: [] },
|
||||
"runner/codex-provider-state.json": { schema: "paperclip.runner.codex-provider-state.v1", lifecycle: "session_open",
|
||||
threadId: "retained-provider-thread", startupAttempt: null, pendingEvents: [], queuedEvents: [] },
|
||||
...(provider === "acpx" ? {
|
||||
"runner/acpx-provider-state.json": { schema: "paperclip.runner.acpx-provider-state.v3", lifecycle: "session_open",
|
||||
startupAttempt: null, pendingEvents: [], queuedEvents: [] },
|
||||
} : {
|
||||
"runner/codex-provider-state.json": { schema: "paperclip.runner.codex-provider-state.v1", lifecycle: "session_open",
|
||||
threadId: "retained-provider-thread", startupAttempt: null, pendingEvents: [], queuedEvents: [] },
|
||||
}),
|
||||
};
|
||||
await mkdir(join(root, "control-plane"), { recursive: true });
|
||||
await mkdir(join(root, "runner"), { recursive: true });
|
||||
@@ -273,6 +283,50 @@ describe("task Retry after inherited native cleanup quarantine", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it.each([
|
||||
{ provider: "codex", mode: "stopped" }, { provider: "codex", mode: "owner_running" },
|
||||
{ provider: "acpx", mode: "stopped" }, { provider: "acpx", mode: "provider_alive" },
|
||||
] as const)(
|
||||
"recovers a quarantined $provider source that never launched with no click once its owner is proven stopped ($mode)", async ({ provider, mode }) => {
|
||||
const base = await mkdtemp(join(tmpdir(), "quarantined-auto-continuation-"));
|
||||
const previous = process.env.PAPERCLIP_RUNNER_STATE_DIR;
|
||||
process.env.PAPERCLIP_RUNNER_STATE_DIR = base;
|
||||
try {
|
||||
const f = await seed(base, false, provider);
|
||||
await db.update(heartbeatRuns).set({ processPid: null, processGroupId: null, processStartedAt: null })
|
||||
.where(eq(heartbeatRuns.id, f.sourceId));
|
||||
if (provider === "acpx") await writeFile(join(f.root, "runner", "acpx-provider-state.json"), JSON.stringify({
|
||||
...f.files["runner/acpx-provider-state.json"],
|
||||
identity: { agentPid: mode === "provider_alive" ? process.pid : 99_999_997 } }));
|
||||
if (mode === "owner_running") await db.update(heartbeatRuns).set({ status: "running", finishedAt: null }).where(eq(heartbeatRuns.id, f.ownerId));
|
||||
await db.update(nativeRunFinalizations).set({ failureCode: "native_session_cleanup_quarantined", attempt: 1,
|
||||
failureDetail: { recoverable: false, recoveryMode: "exact_checkpoint_resume", originalFailureCode: "native_session_cleanup_quarantined",
|
||||
providerEventsExist: false, checkpointExists: true, providerSessionEstablished: true } })
|
||||
.where(eq(nativeRunFinalizations.runId, f.sourceId));
|
||||
const [task] = await db.select().from(issues).where(eq(issues.id, f.issueId));
|
||||
const [hold] = await db.select().from(issueRecoveryActions).where(eq(issueRecoveryActions.sourceIssueId, f.issueId));
|
||||
await db.update(issueRecoveryActions).set({ evidence: { ...hold!.evidence,
|
||||
nativeFailureBlock: { runId: f.sourceId, statusVersion: task!.statusVersion } } })
|
||||
.where(eq(issueRecoveryActions.id, hold!.id));
|
||||
const [before] = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.sourceId));
|
||||
await reconcileSafeNativeReplacements(db, new Date(), {
|
||||
verifyStoppedSession: run => verifyStoppedNativeSessionForAutomaticReplacement(db, run) });
|
||||
const [after] = await db.select().from(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.sourceId));
|
||||
const successorRunId = after?.failureDetail?.successorRunId;
|
||||
if (mode !== "stopped") {
|
||||
expect(successorRunId).toBeUndefined();
|
||||
return;
|
||||
}
|
||||
expect(typeof successorRunId).toBe("string");
|
||||
const [successor] = await db.select().from(heartbeatRuns).where(eq(heartbeatRuns.id, String(successorRunId)));
|
||||
expect(successor?.contextSnapshot).toMatchObject({ forceFreshSession: true });
|
||||
expect(successor?.scheduledRetryAttempt).toBe(before!.attempt);
|
||||
} finally {
|
||||
if (previous === undefined) delete process.env.PAPERCLIP_RUNNER_STATE_DIR; else process.env.PAPERCLIP_RUNNER_STATE_DIR = previous;
|
||||
await rm(base, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
|
||||
it("holds a Retry while the task's own provider session close is still retained, then starts it", async () => {
|
||||
const base = await mkdtemp(join(tmpdir(), "retained-close-retry-"));
|
||||
const previous = process.env.PAPERCLIP_RUNNER_STATE_DIR;
|
||||
|
||||
Reference in new issue
Block a user