mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-11-11 05:11:49 +01:00
fix(native): let Retry continue after a run that failed before native selection (#20482)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - Before Paperclip starts a new message or a Retry after a failed native run, it proves that the run's local process stopped > - A native run can fail before native selection, for example when a budget reservation is refused or a preparation query fails. That run never had a runner, a native session or a process ID > - The stop gate still waited for a stop record of that process. Retry returned 202, then the task stayed on `retryWait.reason=process_identity_missing` and no run started > - Native selection writes the run's native issue, native session, runner instance and coordinator in one transaction. A runner launch needs all of them, and the executor records `native.process_start_requested` before any spawn > - This pull request treats a settled run with none of that evidence as having nothing to stop > - The benefit is that Retry and new messages continue after a failure that happened before the agent started ## Linked Issues or Issue Description **What happened?** In local restart-chaos database-load wave `w-20261110T050818-s20386`, all 4 tasks failed before any agent process started. One example is t01, run `f7e2ee55`. A preparation query failed, and the run ended `adapter_failed` with no native session, no runner instance, no process ID and no launch event. Retry returned 202. For 16 or more minutes, `retryWait` then stayed on `process_identity_missing` and no run started. **Expected behavior** If the previous run never had a process, there is nothing to verify, so Retry and new input continue. **Steps to reproduce** 1. Make a native run fail before native selection (for example, a refused budget reservation or a failed query). 2. Click Retry, or send a message. Before this change, the input waits on `process_identity_missing` and does not continue. ## What Changed - `native-local-process-stop.ts`: `nativeRunRefusedBeforeLaunch` (renamed `nativeRunNeverLaunched`) now also proves "never launched" for a settled native run in this state: - no process ID and no process group; - no native issue, no native session and no runner instance (native selection writes them together). A coordinator row requires the native issue by foreign key, so the native-issue check also rules out a coordinator; - no launch request, no identity record, no `adapter.invoke` and no runner-authored event (any event with a source event ID). The existing case, a run that the cleanup quarantine refused before launch, does not change. - `explicit-native-continuation.test.ts`: the guard "failed for another reason without a process record" now separates real pre-selection failures from the shapes that must stay held. - Admitted: a run that failed before it requested a launch. This case is red on `master`. - Still held on `process_identity_missing`: a run that has a runner event, a run with a prepared native session, and a run with a coordinator, for `failed`, `timed_out` and `interrupted` runs. A `timed_out` run that failed before launch is admitted. The chat suite's `unreconciled` case (a native issue and phase, no session) also stays held. ## Verification - Red/green: on `master`, `routes input after a run failed before it requested a launch` fails with `process_identity_missing`. It passes with this change. - `vitest run src/services/explicit-native-continuation.test.ts src/services/quarantined-task-retry.test.ts src/__tests__/issue-confirm-run-stopped-routes.test.ts`: 3 files, 462 tests pass. This includes every existing "keeps the hold for missing_process" and historical-run case. - Server `tsc --noEmit` passes. ## Risks - Low risk. The new proof needs structural evidence that no runner could exist: no native selection and no launch or runner event. A run that reached native selection, or that has any launch evidence, keeps every existing stop proof. - The proof does not depend on the build: a runner from any build needs the native session and the runner instance. Saved runs from older builds that were selected keep those fields, so they stay held exactly as before. - Relates to #20477, which fixes the restart window inside an answer run's attachment. This pull request is independent of it. ## 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 Refs #19829 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
1 parent
3e572475c8
commit
52b906b112
3 files changed
+65
-20
No files matched your search
@@ -99,25 +99,57 @@ const support = externalUrl ? { supported: true } : await getEmbeddedPostgresTes
|
||||
comment: "local_cleanup", retry: "cleanup_quarantined" },
|
||||
{ shape: "quarantined after it requested a launch", errorCode: "native_session_cleanup_quarantined", launched: true,
|
||||
comment: "process_identity_missing", retry: "process_identity_missing" },
|
||||
{ shape: "failed for another reason without a process record", errorCode: "adapter_failed", launched: false,
|
||||
comment: "process_identity_missing", retry: "process_identity_missing" },
|
||||
])("routes input after a run $shape to the gate that can clear it", async ({ errorCode, launched, comment, retry }) => {
|
||||
// w-20261110T050818-s20386: a run that failed before it asked for a runner
|
||||
// (here a failed preparation query) never had a process to stop.
|
||||
{ shape: "failed before it requested a launch", errorCode: "adapter_failed", launched: false,
|
||||
comment: null, retry: null },
|
||||
{ shape: "failed after a runner event without a process record", errorCode: "adapter_failed", launched: false,
|
||||
runnerEvent: true, comment: "process_identity_missing", retry: "process_identity_missing" },
|
||||
{ shape: "failed before launch after its native session was prepared", errorCode: "adapter_failed", launched: false,
|
||||
prepared: true, comment: "process_identity_missing", retry: "process_identity_missing" },
|
||||
{ shape: "failed without a process record after native selection", errorCode: "adapter_failed", launched: false,
|
||||
coordinator: true, comment: "process_identity_missing", retry: "process_identity_missing" },
|
||||
{ shape: "timed out before it requested a launch", status: "timed_out", errorCode: "timeout", launched: false,
|
||||
comment: null, retry: null },
|
||||
{ shape: "timed out after a runner event without a process record", status: "timed_out", errorCode: "timeout", launched: false,
|
||||
runnerEvent: true, comment: "process_identity_missing", retry: "process_identity_missing" },
|
||||
{ shape: "was interrupted without a process record after native selection", status: "interrupted", errorCode: "process_lost", launched: false,
|
||||
coordinator: true, comment: "process_identity_missing", retry: "process_identity_missing" },
|
||||
] as Array<{ shape: string; status?: "timed_out" | "interrupted"; errorCode: string; launched: boolean; runnerEvent?: boolean; prepared?: boolean; coordinator?: boolean;
|
||||
comment: string | null; retry: string | null }>)("routes input after a run $shape to the gate that can clear it", async ({
|
||||
status, errorCode, launched, runnerEvent, prepared, coordinator, comment, retry }) => {
|
||||
// s7532/t04: a Retry's run failed at the quarantine fence before it asked
|
||||
// for a runner. Input then waited for a stop record of a process that
|
||||
// never existed ("process_identity_missing", Retry still offered) instead
|
||||
// of the quarantine's own gate, which names the action that clears it.
|
||||
const f = await seed();
|
||||
await db.update(heartbeatRuns).set({ processPid: null, errorCode }).where(eq(heartbeatRuns.id, f.sourceRunId));
|
||||
await db.update(heartbeatRuns).set({ processPid: null, errorCode, ...(status ? { status } : {}) })
|
||||
.where(eq(heartbeatRuns.id, f.sourceRunId));
|
||||
// A run that failed before native selection has no native issue and no
|
||||
// coordinator. A coordinator row requires the run's native issue (foreign
|
||||
// key), so the coordinator rows keep it and are held by that evidence.
|
||||
if (errorCode !== "native_session_cleanup_quarantined" && !coordinator) {
|
||||
await db.delete(nativeRunFinalizations).where(eq(nativeRunFinalizations.runId, f.sourceRunId));
|
||||
await db.update(heartbeatRuns).set({ nativeIssueId: null }).where(eq(heartbeatRuns.id, f.sourceRunId));
|
||||
}
|
||||
if (launched) await db.insert(heartbeatRunEvents).values({ companyId: f.companyId, runId: f.sourceRunId, agentId: f.agentId,
|
||||
seq: 1, eventType: "native.process_start_requested", stream: "system", level: "info", message: "launch requested",
|
||||
payload: { hostname: "another-host" } });
|
||||
if (runnerEvent) await db.insert(heartbeatRunEvents).values({ companyId: f.companyId, runId: f.sourceRunId, agentId: f.agentId,
|
||||
seq: 1, eventType: "session.started", stream: "system", level: "info", message: "runner event",
|
||||
sourceEventId: "runner-event-1", payload: {} });
|
||||
if (prepared) await db.update(heartbeatRuns).set({ nativeIssueId: f.issueId, nativeSessionId: randomUUID(), runnerInstanceId: randomUUID() })
|
||||
.where(eq(heartbeatRuns.id, f.sourceRunId));
|
||||
const { result, blocked } = await readAdmission(f, false);
|
||||
expect(result).toBeNull();
|
||||
expect(blocked).toEqual([comment]);
|
||||
if (comment === null) expect(result).not.toBeNull();
|
||||
else expect(result).toBeNull();
|
||||
expect(blocked).toEqual(comment === null ? [] : [comment]);
|
||||
const retryBlocked: string[] = [];
|
||||
expect(await admitExplicitNativeContinuation({ ...f, db, dryRun: true, reason: "retry_failed_run",
|
||||
failedRunId: f.sourceRunId, onBlocked: reason => retryBlocked.push(reason) })).toBeNull();
|
||||
expect(retryBlocked).toEqual([retry]);
|
||||
const retried = await admitExplicitNativeContinuation({ ...f, db, dryRun: true, reason: "retry_failed_run",
|
||||
failedRunId: f.sourceRunId, onBlocked: reason => retryBlocked.push(reason) });
|
||||
if (retry === null) expect(retried).not.toBeNull();
|
||||
else expect(retried).toBeNull();
|
||||
expect(retryBlocked).toEqual(retry === null ? [] : [retry]);
|
||||
});
|
||||
|
||||
it("keeps admission and the complete locked session profile while omitting prompt data", async () => {
|
||||
|
||||
@@ -16,7 +16,7 @@ import { appendHeartbeatRunEvent } from "./heartbeat-run-events.js";
|
||||
import { canContinueCancelledRun, userStopBoundaryAt } from "./run-cancellation.js";
|
||||
import { readQueuedInteractionResponse } from "./queued-interaction-response.js";
|
||||
import { isCancelledNativeStartup, isFailedNativePreparation, isFailedNativePreparationRetryAncestor } from "./cancelled-native-startup.js";
|
||||
import { hasNativeLocalProcessStop, hasHistoricalSuspendedNativeSession, nativeRunRefusedBeforeLaunch, recordedLocalExecutionStopped, recordUnidentifiedLocalRunnerStop } from "./native-local-process-stop.js";
|
||||
import { hasNativeLocalProcessStop, hasHistoricalSuspendedNativeSession, nativeRunNeverLaunched, recordedLocalExecutionStopped, recordUnidentifiedLocalRunnerStop } from "./native-local-process-stop.js";
|
||||
import { completeTerminatedRemoteNativeSessionCleanup, nativeSessionCleanupBlock } from "../vendor/paperclip-runner/index.js";
|
||||
import { hasRecoveredCleanupStopReceipt, hasRemoteTerminationReceipt, remoteLeaseCleanupScope } from "./remote-execution-termination.js";
|
||||
import { z } from "zod";
|
||||
@@ -514,7 +514,7 @@ export async function admitExplicitNativeContinuation(input: {
|
||||
if (!run.processPid && !run.processGroupId &&
|
||||
!await hasNativeLocalProcessStop(db, companyId, run.id) &&
|
||||
!await hasHistoricalSuspendedNativeSession(db, run) &&
|
||||
!await nativeRunRefusedBeforeLaunch(db, run) &&
|
||||
!await nativeRunNeverLaunched(db, run) &&
|
||||
!(run.runtimeMode === "native" &&
|
||||
await recordUnidentifiedLocalRunnerStop(db, run, { record: !input.dryRun }))) return blocked("process_identity_missing", "The previous run has no verified stop record. Paperclip cannot start this message yet.");
|
||||
if (run.processPid && !processStopped(run.processPid)) return blocked("process_running", "Waiting for the previous process to stop. Your message will start automatically.");
|
||||
|
||||
@@ -238,17 +238,30 @@ function record(value: unknown): Record<string, unknown> {
|
||||
return value && typeof value === "object" && !Array.isArray(value) ? value as Record<string, unknown> : {};
|
||||
}
|
||||
|
||||
/** A native run that an admission fence refused before it requested any
|
||||
* runner launch (a quarantined prior-session cleanup is checked before the
|
||||
* provider starts) never had a process: there is nothing to stop. Any launch
|
||||
* request or identity record keeps the normal stop proofs. */
|
||||
export async function nativeRunRefusedBeforeLaunch(db: Db, run: Pick<typeof heartbeatRuns.$inferSelect,
|
||||
"id" | "companyId" | "runtimeMode" | "status" | "errorCode" | "processPid" | "processGroupId">): Promise<boolean> {
|
||||
if (run.runtimeMode !== "native" || run.status !== "failed" || run.processPid || run.processGroupId ||
|
||||
run.errorCode !== "native_session_cleanup_quarantined") return false;
|
||||
/** A settled native run that never had a runner has nothing to stop:
|
||||
* - an admission fence (a quarantined prior-session cleanup) refused it before
|
||||
* it requested a launch; or
|
||||
* - it failed before native selection (for example a refused budget
|
||||
* reservation or a failed preparation query). Selection records the run's
|
||||
* native issue, native session, runner instance and coordinator, and a
|
||||
* runner launch needs them, so without any of them no runner or provider for this run can exist,
|
||||
* in any build.
|
||||
* Any launch request, identity record or runner event keeps the normal stop
|
||||
* proofs. */
|
||||
export async function nativeRunNeverLaunched(db: Db, run: Pick<typeof heartbeatRuns.$inferSelect,
|
||||
"id" | "companyId" | "runtimeMode" | "status" | "errorCode" | "processPid" | "processGroupId" |
|
||||
"nativeSessionId" | "runnerInstanceId" | "nativeIssueId">): Promise<boolean> {
|
||||
if (run.runtimeMode !== "native" || !SETTLED_RUN_STATUSES.includes(run.status) || run.processPid || run.processGroupId) return false;
|
||||
const quarantinedBeforeLaunch = run.status === "failed" && run.errorCode === "native_session_cleanup_quarantined";
|
||||
if (!quarantinedBeforeLaunch) {
|
||||
// A coordinator row requires the run's native issue (foreign key), so the
|
||||
// native issue also rules out a coordinator.
|
||||
if (run.nativeSessionId || run.runnerInstanceId || run.nativeIssueId) return false;
|
||||
}
|
||||
const [launch] = await db.select({ id: heartbeatRunEvents.id }).from(heartbeatRunEvents).where(and(
|
||||
eq(heartbeatRunEvents.companyId, run.companyId), eq(heartbeatRunEvents.runId, run.id),
|
||||
inArray(heartbeatRunEvents.eventType, [PROCESS_START_REQUESTED, PROCESS_IDENTITY_RECORDED]),
|
||||
or(isNotNull(heartbeatRunEvents.sourceEventId),
|
||||
inArray(heartbeatRunEvents.eventType, ["adapter.invoke", PROCESS_START_REQUESTED, PROCESS_IDENTITY_RECORDED])),
|
||||
)).limit(1);
|
||||
return !launch;
|
||||
}
|
||||
|
||||
Reference in new issue
Block a user