From 4a61a2501e424698fb9e711a5fad7a8ff4448efd Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Tue, 10 Nov 2026 19:22:26 -0600 Subject: [PATCH] fix(storage): never promise a storage retry no storage wait owns (#20676) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work > - A run that hits a file-storage outage gets a bounded storage wait: a retry that starts again after storage is back > - The run error says "Paperclip will retry automatically when storage is back" > - On staging, a server restart reattached a remote native runner while its turn was still running. A storage outage then failed the reattachment > - Restart recovery kept the workspace under a no-replay hold, so the run took the code `native_continuation_requires_reconciliation`. Only a `storage_unavailable` run gets a storage wait, so no retry was scheduled. The run still showed the storage retry promise, and the task stayed blocked for about 15 hours > - This pull request makes the storage text follow the run's final code and the real storage wait outcome, and reads old rows truthfully > - The benefit is that every storage message is true: it promises a retry only when one is scheduled, and it offers a working Retry when nothing retries the run ## Linked Issues or Issue Description **What happened?** A native run on a remote sandbox was reattached after a server restart. Its provider turn was still in flight. File storage failed during the reattachment. Restart recovery settled the run as `native_continuation_requires_reconciliation`, kept its files, and did not replay the unfinished turn (this is the existing policy for a turn with an unknown outcome). The run error said "Paperclip will retry automatically when storage is back". No storage wait was scheduled, so nothing retried. The task stayed blocked until an operator clicked Retry. **Root cause** `server/src/services/heartbeat/run-completion.ts` chose the message and the error code separately. The message used `isStorageUnavailableError(err)`. The code used a chain where `nativeTerminalFailureCode` wins over `storage_unavailable`. The storage wait is scheduled only when the code is `storage_unavailable` (and no native terminal failure is recorded). So the message promised a wait that the code made impossible. The same false promise stayed when the storage wait was refused (`not_scheduled`) or its write failed. **Expected behavior** A storage failure promises an automatic retry only when the storage wait is scheduled. Otherwise it offers Retry. Rows saved by earlier releases read the same way after deploy, with no migration. **Steps to reproduce** Fail a native run with a storage-unavailable error after its native coordinator is settled as `terminal_failure` / `native_continuation_requires_reconciliation`. See the new regression in `run-completion.test.ts`. ## What Changed - `storage-unavailable.ts`: add the held messages (outage and rejected credentials). They say Paperclip is not retrying automatically and offer Retry or a message. Add `storageUnavailableHeldMessage()` and `presentStorageFailureMessage()`. - `run-completion.ts` (`failRun`): compute the error code first. Use the storage retry promise only when the code is `storage_unavailable`. Under any other code (for example a native recovery hold), use the held message. No storage wait is created for such a run, because it could only be refused. - `run-completion.ts` (`failRun` and `failRunSetup`): replace `markStorageWaitExhausted` with `settleStorageWaitMessage`. It keeps the promise only when a retry is scheduled. It writes the exhausted text when the budget is spent, and the held text when the wait was refused, its write failed, or a case handoff took over. - `run-completion.ts`: when the promise is withdrawn, the failure's `error` event (written before the retry decision) gets the same final text, so the transcript does not keep the promise after reload. - `run-completion.ts` (`failRunSetup`): a lost controller lease keeps its own code and text, not the storage promise. - `run-lifecycle.ts`: `decorateHeartbeatRunRuntimeStatus` presents a saved storage retry promise under a code other than `storage_unavailable` with the held text. This covers run detail and issue run presence. `listRuns` in `run-state.ts` applies the same projection, so the company run list and the Inbox failure read agree. Rows from earlier releases read truthfully with no migration. - `doc/run-log-events.md`: document which runs get a storage wait and what each message means. Previous-release story (upgrade-safe run state): this change adds no persisted shape. Rows saved by the previous release carry forward. A row with the storage promise under another code reads with the held text. A `storage_unavailable` row keeps its text. The native hold policy is unchanged: an unfinished turn with an unknown outcome is not replayed automatically, and the existing Retry continues it. ## Verification - Red before the fix (assertion failures, not setup errors): the new `run-completion.test.ts` cases received "Paperclip will retry automatically when storage is back" under `native_continuation_requires_reconciliation`, and the `run-lifecycle.test.ts` cases read the saved promise unchanged. - Green after the fix: - `cd server && npx vitest run src/services/heartbeat/run-completion.test.ts src/services/heartbeat/run-lifecycle.test.ts -t storage` (13 passed) - `cd server && npx vitest run src/services/heartbeat/run-completion.test.ts src/services/heartbeat/run-lifecycle.test.ts src/storage/storage-unavailable.test.ts` (213 passed) - `cd server && npx tsc --noEmit -p tsconfig.json` (clean) - Existing storage cases still pass: a scheduled storage wait keeps its promise and suppresses rapid recovery; an exhausted wait writes the exhausted text. - `pnpm verify` exited 2 (a doc is a global CI input), so required CI supplies the broad coverage. - The full `run-completion.test.ts` file passed every test locally. Its `afterAll` hook timed out under machine load (load average about 17), also with the new tests skipped. ## Risks - Low risk. The change affects only the text of failed runs and one read projection. It does not change retry scheduling, recovery ownership, or replay policy. - The read projection matches this repository's own two promise constants under a code other than `storage_unavailable`. A future change to those constants must keep the projection in sync. - Error events already written by earlier releases keep their original text in the run log. Live events already published before a withdrawn promise are not resent. ## Model Used - Claude Opus 5.5 (`claude-opus-5-5`), Anthropic, via Claude Code with tool use (shell, file edits, browser reads for GET-only diagnostics). ## 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 --- doc/run-log-events.md | 14 +++++ .../services/heartbeat/run-completion.test.ts | 60 ++++++++++++++++++- .../src/services/heartbeat/run-completion.ts | 56 ++++++++++++----- .../services/heartbeat/run-lifecycle.test.ts | 21 ++++++- .../src/services/heartbeat/run-lifecycle.ts | 3 + .../src/services/heartbeat/run-state.test.ts | 14 +++++ server/src/services/heartbeat/run-state.ts | 3 + server/src/storage/storage-unavailable.ts | 25 ++++++++ 8 files changed, 180 insertions(+), 16 deletions(-) diff --git a/doc/run-log-events.md b/doc/run-log-events.md index fd7624cb8f9..c0e89d5da38 100644 --- a/doc/run-log-events.md +++ b/doc/run-log-events.md @@ -658,6 +658,20 @@ for file storage before ; it was still unavailable." Its payload holds `reason: "storage_outage_wait"`, `purpose` (the step), `waitedMs`, and `recovered`. The record stays in the instance run log. +When the outage outlasts that wait, the run fails. Only a run recorded with +`errorCode: "storage_unavailable"` gets a storage wait: a bounded retry +(`retryReason: "storage_unavailable_wait"`) that starts again after storage is +back. Its error says "Paperclip will retry automatically when storage is back" +only while that retry is scheduled. When the retry budget is spent, the error +says so and asks for Retry. When no storage wait follows, the error says +"Paperclip is not retrying it automatically. Retry or send a message to +continue." That happens when a native recovery already holds the run under +another code (for example `native_continuation_requires_reconciliation`, where +its files are kept and its unfinished turn is not replayed), or when the retry +is refused or cannot be written. A run saved by an earlier release with the +retry promise under another code reads with the same Retry text, with no +migration. + ## Durable storage timing A saved remote workspace is one encrypted archive in durable object storage. diff --git a/server/src/services/heartbeat/run-completion.test.ts b/server/src/services/heartbeat/run-completion.test.ts index 77458f2a094..89ae9892542 100644 --- a/server/src/services/heartbeat/run-completion.test.ts +++ b/server/src/services/heartbeat/run-completion.test.ts @@ -33,7 +33,7 @@ import { PluginWorkerUnavailableError } from "../plugin-worker-unavailable-error import { ReviewedChatBindingDeniedError, ReviewedChatBindingNotReadyError } from "../reviewed-chat-binding-diagnostics.js"; import { environmentTaskErrorCodeData, JsonRpcCallError, PluginEnvironmentTaskError, PluginWorkspaceOperationError, workspaceOperationErrorData } from "@paperclipai/plugin-sdk"; import { RUNNER_REVISION_MISMATCH_RETRY } from "../runner-revision-mismatch.js"; -import { STORAGE_UNAVAILABLE_CODE, STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE, STORAGE_UNAVAILABLE_MESSAGE, STORAGE_UNAVAILABLE_RETRY } from "../../storage/storage-unavailable.js"; +import { STORAGE_CREDENTIALS_REJECTED_HELD_MESSAGE, STORAGE_UNAVAILABLE_CODE, STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE, STORAGE_UNAVAILABLE_HELD_MESSAGE, STORAGE_UNAVAILABLE_MESSAGE, STORAGE_UNAVAILABLE_RETRY } from "../../storage/storage-unavailable.js"; import { withS3FailureDiagnostics } from "../../storage/s3-failure.js"; import { workspaceProviderCallFailure } from "../workspace-provider-failure.js"; import { logger } from "../../middleware/logger.js"; @@ -857,6 +857,64 @@ describe.skipIf(!support.supported && !process.env.PAPERCLIP_TEST_DATABASE_URL)( error: STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE }); }); + // Staging incident shape: a server restart reattached a remote native runner whose turn was + // still in flight. A storage outage then failed the reattachment, restart + // recovery kept the workspace under a no-replay hold, and the run reported + // "Paperclip will retry automatically when storage is back" with no storage + // wait scheduled. The task sat blocked for 15 hours behind a false promise. + it.each([ + ["an outage", "ServiceUnavailable", 503, STORAGE_UNAVAILABLE_HELD_MESSAGE], + ["rejected credentials", "ExpiredToken", 403, STORAGE_CREDENTIALS_REJECTED_HELD_MESSAGE], + ] as const)("never promises a storage retry when native recovery holds the run after %s", async (_label, name, status, expected) => { + const f = await fixture({ runtimeMode: "native" }); + const [issue] = await db.insert(issues).values({ companyId: f.run.companyId, title: "Daily staging review", + assigneeAgentId: f.agent.id, status: "in_progress" }).returning(); + await db.update(agentWakeupRequests).set({ runId: f.run.id, reason: "issue_monitor_due", requestedByActorType: "system" }) + .where(eq(agentWakeupRequests.id, f.wake.id)); + const [run] = await db.update(heartbeatRuns).set({ nativeIssueId: issue.id, + contextSnapshot: { issueId: issue.id, wakeReason: "issue_monitor_due", wakeSource: "automation" } }) + .where(eq(heartbeatRuns.id, f.run.id)).returning(); + await appendHeartbeatRunEvent(db, { companyId: run.companyId, runId: run.id, agentId: run.agentId, + eventType: "turn.started", stream: "system", level: "info", message: "Provider turn started" }); + // Restart recovery's workspace preservation already settled the coordinator. + await db.insert(nativeRunFinalizations).values({ companyId: run.companyId, issueId: issue.id, runId: run.id, + phase: "terminal_failure", attempt: 1, controllerGeneration: 3, recoveryState: "blocked", + failureCode: "native_continuation_requires_reconciliation" }); + const outage = await withS3FailureDiagnostics("put_object", async () => { + throw Object.assign(new Error("Injected storage outage"), { name, $metadata: { httpStatusCode: status, attempts: 3 } }); + }).catch(error => error as Error); + await f.completion.failRun({ ...f.failure(outage), run, issueId: issue.id, requiredWorkspaceRestoreEvidence: {}, legacyAdapterEntered: false }); + const failed = await read(run.id); + expect(failed).toMatchObject({ status: "failed", errorCode: "native_continuation_requires_reconciliation", error: expected }); + expect(failed.error).not.toMatch(/retry automatically/i); + expect(failed.error).toContain("Retry"); + // No storage wait is created for a run native recovery owns: it could only be refused. + expect(f.deps.scheduleBoundedRetryForRun).not.toHaveBeenCalled(); + const events = await db.select().from(heartbeatRunEvents).where(and(eq(heartbeatRunEvents.runId, run.id), + eq(heartbeatRunEvents.eventType, "error"))); + expect(events.map(event => event.message)).toEqual([expected]); + }); + + it.each([ + ["run", "not scheduled"], ["setup", "not scheduled"], ["run", "scheduling failed"], ["setup", "scheduling failed"], + ] as const)("withdraws the storage retry promise when no storage wait could be scheduled (%s, %s)", async (phase, mode) => { + const f = await fixture(); + const outage = await withS3FailureDiagnostics("put_object", async () => { + throw Object.assign(new Error("Injected storage outage"), { name: "ServiceUnavailable", $metadata: { httpStatusCode: 503 } }); + }).catch(error => error as Error); + if (mode === "not scheduled") f.deps.scheduleBoundedRetryForRun.mockResolvedValueOnce({ outcome: "not_scheduled", + reason: "Scheduled retry suppressed because the agent is not invokable", errorCode: "agent_not_invokable", issueId: null } as never); + else f.deps.scheduleBoundedRetryForRun.mockRejectedValueOnce(new Error("retry write failed")); + if (phase === "setup") await f.completion.failRunSetup(f.setup(outage)); + else await f.completion.failRun(f.failure(outage)); + expect(f.deps.scheduleBoundedRetryForRun).toHaveBeenCalledWith(expect.objectContaining({ id: f.run.id }), f.agent, STORAGE_UNAVAILABLE_RETRY); + expect(await read(f.run.id)).toMatchObject({ status: "failed", errorCode: STORAGE_UNAVAILABLE_CODE, + error: STORAGE_UNAVAILABLE_HELD_MESSAGE }); + const events = await db.select().from(heartbeatRunEvents).where(and(eq(heartbeatRunEvents.runId, f.run.id), + eq(heartbeatRunEvents.eventType, "error"))); + expect(events.map(event => event.message)).toEqual([STORAGE_UNAVAILABLE_HELD_MESSAGE]); + }); + it.each(["execution", "setup"] as const)("names the nested cause of a generic %s failure and keeps SQL text off the board", async phase => { const f = await fixture(); const logged = vi.spyOn(logger, "error"); diff --git a/server/src/services/heartbeat/run-completion.ts b/server/src/services/heartbeat/run-completion.ts index 847d63bda94..08a95dc55a5 100644 --- a/server/src/services/heartbeat/run-completion.ts +++ b/server/src/services/heartbeat/run-completion.ts @@ -24,7 +24,7 @@ import { ENVIRONMENT_PROVIDER_UNAVAILABLE_CODE, findEnvironmentProviderUnavailab import { REPOSITORY_CATALOG_UNAVAILABLE_CODE, findRepositoryCatalogUnavailableError } from "../repository-catalog-unavailable-error.js"; import { findWorkspaceProviderFailure } from "../workspace-provider-failure.js"; import { isRunnerRevisionMismatch, RUNNER_REVISION_MISMATCH_CODE, RUNNER_REVISION_MISMATCH_RETRY } from "../runner-revision-mismatch.js"; -import { isStorageUnavailableError, STORAGE_UNAVAILABLE_CODE, STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE, STORAGE_UNAVAILABLE_RETRY, storageUnavailableMessage } from "../../storage/storage-unavailable.js"; +import { isStorageUnavailableError, STORAGE_CREDENTIALS_REJECTED_MESSAGE, STORAGE_UNAVAILABLE_CODE, STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE, STORAGE_UNAVAILABLE_MESSAGE, STORAGE_UNAVAILABLE_RETRY, storageUnavailableHeldMessage, storageUnavailableMessage } from "../../storage/storage-unavailable.js"; import { type RunSessionOutcome, resolveNextSessionState, normalizeUsageTotals, resolveCacheAdjustedCostUsd, resolveLedgerBiller, resolveLedgerCostStatus, normalizeLedgerBillingType } from "./run-state.js"; import { readGitConnectionFailure } from "../git-connection-failure.js"; import { CONFIGURATION_INCOMPLETE_FAILURE_CODE, ConfigurationIncompleteFailure } from "./run-preparation.js"; @@ -38,8 +38,8 @@ import { accountRunCost } from "../run-cost-accounting.js"; import { isConversation, settleConversationTurn } from "../agent-conversations.js"; import { legacyExecutionNeedsReconciliationWithEvidence } from "../legacy-execution-recovery.js"; import { isWorkspaceGitScanError } from "../workspace-git-operation-scheduler.js"; -import { and, eq, isNull, sql } from "drizzle-orm"; -import { heartbeatRuns, issues, nativeRunFinalizations, nativeRunResults } from "@paperclipai/db"; +import { and, eq, inArray, isNull, sql } from "drizzle-orm"; +import { heartbeatRunEvents, heartbeatRuns, issues, nativeRunFinalizations, nativeRunResults } from "@paperclipai/db"; import { HttpError } from "../../errors.js"; import { logger } from "../../middleware/logger.js"; import { normalizeResponsibleUserDenialCode } from "../responsible-user-denial-run-outcomes.js"; @@ -1255,17 +1255,22 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC const interruptedExactResume = exactResumeCause ? { ...exactResumeCause, originalMessage: redactFailureText(exactResumeCause.originalMessage) } : null; - const message = interruptedExactResume - ? describeRefusedExactResume(interruptedExactResume, thrownMessage) - : isStorageUnavailableError(err) ? storageUnavailableMessage(err) : thrownMessage; + const storageUnavailable = isStorageUnavailableError(err); const failureErrorCode = workspaceValidationFailure?.code ?? configurationIncompleteFailure?.code ?? nonRetryablePreflightFailureCode(err) ?? recordedResponsibleUserDenialCode ?? nativeTerminalFailureCode ?? - (isStorageUnavailableError(err) ? STORAGE_UNAVAILABLE_CODE : null) ?? + (storageUnavailable ? STORAGE_UNAVAILABLE_CODE : null) ?? "adapter_failed"; + // Only a `storage_unavailable` run gets a storage wait. Under any other code + // (for example a native recovery hold that keeps the files without + // replaying an unfinished turn) nothing retries it, so offer Retry instead. + const message = interruptedExactResume + ? describeRefusedExactResume(interruptedExactResume, thrownMessage) + : !storageUnavailable ? thrownMessage + : failureErrorCode === STORAGE_UNAVAILABLE_CODE ? storageUnavailableMessage(err) : storageUnavailableHeldMessage(err); const failureCause = GENERIC_RUN_FAILURE_CODES.has(failureErrorCode) ? thrownFailureCause : null; logger.error({ err: identityRedactor.redact({ message: err instanceof Error ? err.message : String(err), stack: err instanceof Error ? err.stack : undefined }), runId, errorCode: failureErrorCode, failureCause: thrownFailureCause }, "heartbeat execution failed"); @@ -1395,6 +1400,7 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC } await measureRunPhase("completion.summary", () => refreshContinuationSummaryForRun(livenessRun, agent)); if (await pendingCaseHandoffForRun(db, livenessRun.id, livenessRun.companyId)) { + await settleStorageWaitMessage(livenessRun, null, err); await releaseIssueExecutionAndPromote(livenessRun, { suppressImmediateRecovery: true }); await settleAgent(); return; @@ -1411,14 +1417,16 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC return null; }) : null; - if (storageWait?.outcome === "retry_exhausted") await markStorageWaitExhausted(livenessRun); const nativePreDispatchRetry = storageWait === null ? await scheduleNativePreDispatchRetry(livenessRun, agent) : null; + let fallbackRetry: { outcome: string } | null | undefined = nativePreDispatchRetry; if (storageWait?.outcome !== "scheduled" && storageWait?.outcome !== "retry_exhausted" && nativePreDispatchRetry === null) { - await scheduleInteractionContinuationInfrastructureRetryIfEligible( + fallbackRetry = await scheduleInteractionContinuationInfrastructureRetryIfEligible( livenessRun, agent, ); } + // Another retry that took over after a failed storage wait keeps the promise. + await settleStorageWaitMessage(livenessRun, fallbackRetry?.outcome === "scheduled" ? fallbackRetry : storageWait, err); await releaseIssueExecutionAndPromote(livenessRun, { // Native recovery owns the original heartbeat run through // exhaustion. Once its durable coordinator has classified a @@ -1480,10 +1488,26 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC } } - async function markStorageWaitExhausted(run: typeof heartbeatRuns.$inferSelect) { - await db.update(heartbeatRuns).set({ error: STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE, updatedAt: new Date() }) + /** + * A `storage_unavailable` failure first promises its storage wait. Keep that + * promise only when the wait was scheduled. An exhausted wait says so; a wait + * that was refused, could not be written, or was preempted by a case handoff + * offers Retry instead, because nothing else will retry the run. + */ + async function settleStorageWaitMessage(run: typeof heartbeatRuns.$inferSelect, + wait: { outcome: string } | null | undefined, cause: unknown) { + if (run.errorCode !== STORAGE_UNAVAILABLE_CODE || wait?.outcome === "scheduled") return; + const error = wait?.outcome === "retry_exhausted" ? STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE : storageUnavailableHeldMessage(cause); + await db.update(heartbeatRuns).set({ error, updatedAt: new Date() }) .where(and(eq(heartbeatRuns.id, run.id), eq(heartbeatRuns.errorCode, STORAGE_UNAVAILABLE_CODE))) - .catch((error: unknown) => logger.warn({ err: error, runId: run.id }, "failed to record an exhausted storage wait")); + .catch((writeError: unknown) => logger.warn({ err: writeError, runId: run.id }, "failed to record why no storage wait follows")); + // The failure's error event was written before the retry decision. Its + // transcript text must not keep a promise that this run no longer makes. + await db.update(heartbeatRunEvents).set({ message: error }) + .where(and(eq(heartbeatRunEvents.companyId, run.companyId), eq(heartbeatRunEvents.runId, run.id), + eq(heartbeatRunEvents.eventType, "error"), + inArray(heartbeatRunEvents.message, [STORAGE_UNAVAILABLE_MESSAGE, STORAGE_CREDENTIALS_REJECTED_MESSAGE]))) + .catch((writeError: unknown) => logger.warn({ err: writeError, runId: run.id }, "failed to correct the storage failure event")); } async function failRunSetup(input: FailHeartbeatRunSetupInput) { @@ -1501,7 +1525,9 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC const storageUnavailable = isStorageUnavailableError(outerErr); // Bounded code from the error or its first three causes; never message text. const thrownFailureCause = failureCauseFromError(outerErr, providerFailure); - const message = storageUnavailable ? storageUnavailableMessage(outerErr) : withoutSqlStatementText(redactCurrentUserText( + // A lost controller lease takes the code below and its own retry, so only a + // `storage_unavailable` failure carries the storage wait's text. + const message = storageUnavailable && !controllerLeaseLost ? storageUnavailableMessage(outerErr) : withoutSqlStatementText(redactCurrentUserText( identityRedactor.redact(outerErr instanceof Error ? outerErr.message : "Unknown setup failure"), @@ -1709,6 +1735,7 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC failedRun, ).catch(() => failedRun); let storageWaitScheduled = false; + let storageWait: { outcome: string } | null = null; let nativePreDispatchRetryScheduled = false; const setupFailureIssueId = readNonEmptyString( parseObject(livenessRun.contextSnapshot).issueId, @@ -1783,12 +1810,13 @@ export function createHeartbeatRunCompletion(db: Db, dependencies: HeartbeatRunC return null; }); reviewedChatRetryScheduled = retry?.outcome === "scheduled"; - if (livenessRun.errorCode === STORAGE_UNAVAILABLE_CODE && retry?.outcome === "retry_exhausted") await markStorageWaitExhausted(livenessRun); + storageWait = retry; storageWaitScheduled = livenessRun.errorCode === STORAGE_UNAVAILABLE_CODE && (retry?.outcome === "scheduled" || retry?.outcome === "retry_exhausted"); nativePreDispatchRetryScheduled = retry?.outcome === "scheduled" && isNativePreDispatchTransientFailure(livenessRun); } } + await settleStorageWaitMessage(livenessRun, storageWait, outerErr); await releaseIssueExecutionAndPromote(livenessRun, { suppressImmediateRecovery: storageWaitScheduled || nativePreDispatchRetryScheduled || readNonEmptyString( diff --git a/server/src/services/heartbeat/run-lifecycle.test.ts b/server/src/services/heartbeat/run-lifecycle.test.ts index 7efd1c71021..21b3ed744d7 100644 --- a/server/src/services/heartbeat/run-lifecycle.test.ts +++ b/server/src/services/heartbeat/run-lifecycle.test.ts @@ -11,7 +11,9 @@ import { getHeartbeatRunRuntimeStatus } from "../heartbeat-run-runtime-status.js import { createHeartbeatRunState } from "./run-state.js"; import { heartbeatService } from "../heartbeat.js"; import { createHeartbeatRunPreparation } from "./run-preparation.js"; -import { createHeartbeatLifecycle, persistHeartbeatRunProcessMetadata, type HeartbeatLifecycleDependencies } from "./run-lifecycle.js"; +import { createHeartbeatLifecycle, decorateHeartbeatRunRuntimeStatus, persistHeartbeatRunProcessMetadata, type HeartbeatLifecycleDependencies } from "./run-lifecycle.js"; +import { STORAGE_CREDENTIALS_REJECTED_HELD_MESSAGE, STORAGE_CREDENTIALS_REJECTED_MESSAGE, STORAGE_UNAVAILABLE_CODE, + STORAGE_UNAVAILABLE_HELD_MESSAGE, STORAGE_UNAVAILABLE_MESSAGE } from "../../storage/storage-unavailable.js"; import { DELIVERY_QUEUES, subscribeDeliveryWork } from "../delivery-work-notifications.js"; vi.mock("../live-events.js", () => ({ publishLiveEvent: vi.fn() })); @@ -52,6 +54,23 @@ describe("heartbeat lifecycle module boundary", () => { for (const callback of Object.values(deps)) if (vi.isMockFunction(callback)) expect(callback).not.toHaveBeenCalled(); }); + // Rows saved by earlier releases can carry the storage retry promise + // under a code that no storage wait owns. They read truthfully after deploy, + // with no migration; a run a storage wait owns keeps its promise. + it.each([ + ["an outage", STORAGE_UNAVAILABLE_MESSAGE, STORAGE_UNAVAILABLE_HELD_MESSAGE], + ["rejected credentials", STORAGE_CREDENTIALS_REJECTED_MESSAGE, STORAGE_CREDENTIALS_REJECTED_HELD_MESSAGE], + ])("reads a previously saved storage retry promise under native recovery truthfully (%s)", (_label, saved, expected) => { + const held = decorateHeartbeatRunRuntimeStatus({ id: "held-native-run", status: "failed", + errorCode: "native_continuation_requires_reconciliation", error: saved }); + expect(held.error).toBe(expected); + const waiting = decorateHeartbeatRunRuntimeStatus({ id: "run-with-storage-wait", status: "failed", + errorCode: STORAGE_UNAVAILABLE_CODE, error: saved }); + expect(waiting.error).toBe(saved); + const other = decorateHeartbeatRunRuntimeStatus({ id: "other", status: "failed", errorCode: "adapter_failed", error: "Adapter failed" }); + expect(other.error).toBe("Adapter failed"); + }); + it("ignores absent wake requests without accessing the database", async () => { const database = guardedDatabase(); const lifecycle = createHeartbeatLifecycle(database.db, callbacks(database.db)); diff --git a/server/src/services/heartbeat/run-lifecycle.ts b/server/src/services/heartbeat/run-lifecycle.ts index 7c6189b3863..da232abd907 100644 --- a/server/src/services/heartbeat/run-lifecycle.ts +++ b/server/src/services/heartbeat/run-lifecycle.ts @@ -208,6 +208,7 @@ import type { recoveryService } from "../recovery/index.js"; import type { companySkillService } from "../company-skills.js"; import type { budgetService, BudgetServiceHooks } from "../budgets.js"; import type { issueTreeControlService } from "../issue-tree-control.js"; +import { presentStorageFailureMessage } from "../../storage/storage-unavailable.js"; type HeartbeatRun = typeof heartbeatRuns.$inferSelect; @@ -369,6 +370,8 @@ export function decorateHeartbeatRunRuntimeStatus< return { ...run, + // Saved runs from earlier releases may promise a storage retry no wait owns. + ...("error" in run ? { error: presentStorageFailureMessage(run as { error?: string | null; errorCode?: string | null }) } : {}), currentStatusMessage: currentStatus?.message ?? null, currentStatusUpdatedAt: currentStatus?.updatedAt ?? null, currentToolName: currentStatus?.currentToolName ?? null, diff --git a/server/src/services/heartbeat/run-state.test.ts b/server/src/services/heartbeat/run-state.test.ts index 956da72b4ce..e3d78e5208b 100644 --- a/server/src/services/heartbeat/run-state.test.ts +++ b/server/src/services/heartbeat/run-state.test.ts @@ -21,6 +21,7 @@ import * as legacy from "../heartbeat.js"; import * as extracted from "./run-state.js"; import { readRunCancellation } from "../run-cancellation.js"; import { decideQueuedCommentQueueSteering } from "../issue-queued-comment-queue.js"; +import { STORAGE_UNAVAILABLE_CODE, STORAGE_UNAVAILABLE_HELD_MESSAGE, STORAGE_UNAVAILABLE_MESSAGE } from "../../storage/storage-unavailable.js"; // SQL_ASCII needs a separate database cluster. Inspect its selected SQL here; // the PostgreSQL tests below exercise the session queries and transactions. @@ -397,6 +398,19 @@ describePostgres("heartbeat run state database wiring", () => { expect(await state.listRuns(companyId, foreign.agent.id)).toEqual([]); }); + it("lists a previously saved storage retry promise under native recovery truthfully", async () => { + const { companyId, agent, state } = await fixture(); + const [held] = await db.insert(heartbeatRuns).values({ companyId, agentId: agent.id, status: "failed", + errorCode: "native_continuation_requires_reconciliation", error: STORAGE_UNAVAILABLE_MESSAGE }).returning(); + const [waiting] = await db.insert(heartbeatRuns).values({ companyId, agentId: agent.id, status: "failed", + errorCode: STORAGE_UNAVAILABLE_CODE, error: STORAGE_UNAVAILABLE_MESSAGE }).returning(); + for (const summary of [false, true]) { + const rows = await state.listRuns(companyId, agent.id, 100, { summary }); + expect(rows.find(row => row.id === held.id)?.error).toBe(STORAGE_UNAVAILABLE_HELD_MESSAGE); + expect(rows.find(row => row.id === waiting.id)?.error).toBe(STORAGE_UNAVAILABLE_MESSAGE); + } + }); + it("preserves legacy context text, native issue precedence and summary privacy", async () => { const { companyId, agent, issue, state } = await fixture(); const contexts: unknown[] = [null, [], "legacy", 3, true, {}, { diff --git a/server/src/services/heartbeat/run-state.ts b/server/src/services/heartbeat/run-state.ts index 3150bff2e5e..9a0d3c04f65 100644 --- a/server/src/services/heartbeat/run-state.ts +++ b/server/src/services/heartbeat/run-state.ts @@ -57,6 +57,7 @@ import { import { isUuidLike } from "@paperclipai/shared"; import { HEARTBEAT_RUN_LIST_DEFAULT_LIMIT, HEARTBEAT_RUN_LIST_MAX_LIMIT } from "@paperclipai/shared"; import { isUnsafeSessionWorkspaceCwd } from "../session-workspace-cwd.js"; +import { presentStorageFailureMessage } from "../../storage/storage-unavailable.js"; export const EXECUTION_REVIEW_PARTICIPANT_RECOVERY_WAKE_REASON = "execution_review_participant_recovery"; @@ -1868,6 +1869,8 @@ export function createHeartbeatRunState(db: Db) { return { ...rest, + // Saved runs from earlier releases may promise a storage retry no wait owns. + ...("error" in rest ? { error: presentStorageFailureMessage(rest) ?? null } : {}), contextSnapshot: summarizeHeartbeatRunContextSnapshot({ issueId: contextIssueId, taskId: contextTaskId, diff --git a/server/src/storage/storage-unavailable.ts b/server/src/storage/storage-unavailable.ts index 2772bf7701e..111cc0649fc 100644 --- a/server/src/storage/storage-unavailable.ts +++ b/server/src/storage/storage-unavailable.ts @@ -11,6 +11,13 @@ export const STORAGE_UNAVAILABLE_MESSAGE = "File storage was unavailable, so thi export const STORAGE_CREDENTIALS_REJECTED_MESSAGE = "File storage rejected Paperclip's storage credentials (they may have expired or been rotated), so this attempt stopped before it could save its work. Paperclip will retry automatically when valid credentials are available."; +/** For a storage failure that no storage wait owns, such as a run whose native + * recovery keeps its files without replaying an unfinished turn. Nothing + * retries it automatically, so the text offers Retry instead of promising one. */ +export const STORAGE_UNAVAILABLE_HELD_MESSAGE = "File storage was unavailable, so this attempt stopped before it could save its work. Paperclip is not retrying it automatically. Retry or send a message to continue."; + +export const STORAGE_CREDENTIALS_REJECTED_HELD_MESSAGE = "File storage rejected Paperclip's storage credentials (they may have expired or been rotated), so this attempt stopped before it could save its work. Paperclip is not retrying it automatically. Retry or send a message to continue."; + export const STORAGE_UNAVAILABLE_EXHAUSTED_MESSAGE = "File storage stayed unavailable or kept rejecting Paperclip's storage credentials through every automatic retry, so Paperclip stopped retrying this task. Check the storage service and its credentials, then Retry."; export const STORAGE_UNAVAILABLE_RETRY = { @@ -66,6 +73,24 @@ export function storageUnavailableMessage(error: unknown): string { return isStorageCredentialRejection(error) ? STORAGE_CREDENTIALS_REJECTED_MESSAGE : STORAGE_UNAVAILABLE_MESSAGE; } +/** The text for a storage failure that no storage wait owns. */ +export function storageUnavailableHeldMessage(error: unknown): string { + return isStorageCredentialRejection(error) ? STORAGE_CREDENTIALS_REJECTED_HELD_MESSAGE : STORAGE_UNAVAILABLE_HELD_MESSAGE; +} + +/** + * Only a run recorded as `storage_unavailable` has a storage wait. Releases + * before this check could save the retry promise under another code (a + * native recovery hold), where nothing retries. Reads present those rows with + * the held text, so they are truthful after deploy without a migration. + */ +export function presentStorageFailureMessage(run: { error?: string | null; errorCode?: string | null }): string | null | undefined { + if (run.errorCode === STORAGE_UNAVAILABLE_CODE) return run.error; + if (run.error === STORAGE_UNAVAILABLE_MESSAGE) return STORAGE_UNAVAILABLE_HELD_MESSAGE; + if (run.error === STORAGE_CREDENTIALS_REJECTED_MESSAGE) return STORAGE_CREDENTIALS_REJECTED_HELD_MESSAGE; + return run.error; +} + export function isStorageUnavailableError(error: unknown): boolean { const recorded = recordedStorageFailure(error); if (!recorded) return false;