From dcf2022de69ba66da38bcd81a4c3dc9f446d1c79 Mon Sep 17 00:00:00 2001 From: Netquirk CTO Date: Fri, 11 Sep 2026 17:44:24 +0000 Subject: [PATCH 01/14] fix(server): retry null-environment process loss --- .../heartbeat-process-recovery.test.ts | 41 ++++- server/src/services/heartbeat.ts | 165 +++++++++++++----- 2 files changed, 159 insertions(+), 47 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index f77f756285..ccafe72b5a 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -2438,8 +2438,45 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect(checkoutReleasedIssue?.checkoutRunId).toBeNull(); }); - it("requires reconciliation for a lost monitor whose provider outcomes are unknown", async () => { - const { agentId, runId, issueId } = await seedRunFixture({ + it("schedules a null-environment process loss with diagnostics instead of stranding it", async () => { + const { companyId, agentId, runId } = await seedRunFixture({ + agentStatus: "idle", + processPid: null, + processGroupId: null, + }); + const heartbeat = heartbeatService(db); + + expect(await heartbeat.reapOrphanedRuns()).toEqual({ reaped: 1, runIds: [runId] }); + + const runs = await db + .select() + .from(heartbeatRuns) + .where(and(eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.agentId, agentId))); + const failed = runs.find((row) => row.id === runId); + const retry = runs.find((row) => row.retryOfRunId === runId); + + expect(failed?.resultJson).toMatchObject({ + environmentAllocationDiagnostic: { + phase: "environment_selection", + outcome: "failed", + reasonCode: "no_environment_or_lease_recorded", + }, + }); + expect(failed?.stderrExcerpt).toContain("[environment-allocation]"); + expect(retry).toMatchObject({ + status: "scheduled_retry", + scheduledRetryAttempt: 1, + scheduledRetryReason: "retry_transient_environment_failure", + }); + expect(retry?.scheduledRetryAt?.getTime()).toBe((failed?.finishedAt?.getTime() ?? 0) + 60_000); + expect(retry?.contextSnapshot).toMatchObject({ + wakeReason: "process_lost_environment_retry", + retryReason: "retry_transient_environment_failure", + }); + }); + + it("restores one lost monitor dispatch before escalating a second process loss", async () => { + const { companyId, agentId, runId, issueId } = await seedRunFixture({ adapterType: "openclaw_gateway", agentStatus: "idle", processPid: null, diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 6456843a17..5ee2f3a12f 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -746,13 +746,14 @@ export const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_DELAYS_MS = [ const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_JITTER_RATIO = 0; const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_REASON = "transient_failure"; const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_WAKE_REASON = "transient_failure_retry"; -const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS = - BOUNDED_TRANSIENT_HEARTBEAT_RETRY_DELAYS_MS.length; -export { - INTERACTION_CONTINUATION_INFRA_RETRY_REASON, - INTERACTION_CONTINUATION_INFRA_WAKE_REASON, -}; -const INTERACTION_CONTINUATION_INFRA_MAX_ATTEMPTS = 2; +const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS = BOUNDED_TRANSIENT_HEARTBEAT_RETRY_DELAYS_MS.length; +const NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_REASON = "retry_transient_environment_failure"; +const NULL_ENVIRONMENT_PROCESS_LOSS_WAKE_REASON = "process_lost_environment_retry"; +const NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_DELAYS_MS = [60_000, 180_000, 540_000] as const; +export const INTERACTION_CONTINUATION_INFRA_RETRY_REASON = "interaction_continuation_infra_retry"; +export const INTERACTION_CONTINUATION_INFRA_WAKE_REASON = "interaction_continuation_infra_retry"; +const INTERACTION_CONTINUATION_INFRA_MAX_ATTEMPTS = 3; +const RESOLVED_INTERACTION_CONTINUATION_STATUSES = new Set(["accepted", "answered", "rejected"]); const WORKSPACE_VALIDATION_FAILURE_CODE = "workspace_validation_failed"; const WORKSPACE_VALIDATION_RECOVERY_CAUSE = "workspace_validation_failed"; const CONFIGURATION_INCOMPLETE_FAILURE_CODE = "configuration_incomplete"; @@ -1922,6 +1923,21 @@ export function computeBoundedTransientHeartbeatRetrySchedule( }; } +// This signature deliberately excludes a process that was ever spawned. A +// lost child process remains on the legacy process-loss path; this ladder is +// only for dispatches that died before an execution environment existed. +export function isNullEnvironmentProcessLoss(input: { + usageJson: unknown; + processPid: number | null; + processGroupId: number | null; + hasEnvironmentLease: boolean; +}) { + return input.usageJson == null && + input.processPid == null && + input.processGroupId == null && + !input.hasEnvironmentLease; +} + async function resolveRunScopedMentionedSkillKeys(input: { db: Db; companyId: string; @@ -18446,46 +18462,90 @@ export function heartbeatService( readNonEmptyString(runContext.wakeReason) === "issue_monitor_due" && monitorNextCheckAt !== undefined && (!monitorNextCheckAt || monitorNextCheckAt.getTime() <= now.getTime()); - const shouldRetry = - (run.processLossRetryCount ?? 0) < 1 && - ((tracksLegacyLocalChild && - (!!run.processPid || !!run.processGroupId)) || - monitorDispatchLostWithoutFutureWake); - if (!(await revokeExpiredLegacyController(db, run))) continue; - const baseMessage = buildProcessLossMessage(run); - const conversationContinuationEligible = await runUsedConversationAdapter(db, run); - - const failureWrite = await setRunStatusFromLive( - run.id, - "failed", - ["running"], - { - error: shouldRetry ? `${baseMessage}; retrying once` : baseMessage, - errorCode: "process_lost", - finishedAt: now, - resultJson: (() => { - const result = mergeRunStopMetadataForAgent( - { adapterType, adapterConfig }, - "failed", - { - conversationContinuationEligible, - resultJson: parseObject(run.resultJson), - errorCode: "process_lost", - errorMessage: shouldRetry - ? `${baseMessage}; retrying once` - : baseMessage, - }, - ); - return result; - })(), - }, + const environmentLease = await db + .select({ id: environmentLeases.id }) + .from(environmentLeases) + .where(and( + eq(environmentLeases.companyId, run.companyId), + eq(environmentLeases.heartbeatRunId, run.id), + )) + .limit(1) + .then((rows) => rows[0] ?? null); + const nullEnvironmentProcessLoss = isNullEnvironmentProcessLoss({ + usageJson: run.usageJson, + processPid: run.processPid, + processGroupId: run.processGroupId, + hasEnvironmentLease: environmentLease !== null, + }); + const shouldRetryLegacyProcessLoss = (run.processLossRetryCount ?? 0) < 1 && ( + (tracksLocalChild && (!!run.processPid || !!run.processGroupId)) || + monitorDispatchLostWithoutFutureWake ); - if (!failureWrite.updated || !failureWrite.run) continue; - let finalizedRun: typeof heartbeatRuns.$inferSelect | null = - failureWrite.run; + const shouldRetry = nullEnvironmentProcessLoss || shouldRetryLegacyProcessLoss; + const baseMessage = buildProcessLossMessage(run, descendantOnlyCleanup ? { descendantOnly: true } : undefined); + const allocationDiagnostic = nullEnvironmentProcessLoss + ? { + phase: "environment_selection", + outcome: "failed", + reasonCode: "no_environment_or_lease_recorded", + environmentId: null, + leaseId: null, + scratchDirHealth: "unknown", + capturedAt: now.toISOString(), + } + : null; + const allocationDiagnosticLine = allocationDiagnostic + ? `[environment-allocation] ${allocationDiagnostic.phase}:${allocationDiagnostic.reasonCode}` + : null; + const unmanagedBackgroundTaskEvidence = descendantOnlyCleanup + ? { + kind: "orphaned_process_group_cleanup", + stopped: true, + stopReason: UNMANAGED_BACKGROUND_TASK_STOP_REASON, + reason: UNMANAGED_BACKGROUND_TASK_LIVENESS_REASON, + processPid: run.processPid ?? null, + processGroupId: run.processGroupId ?? null, + } + : null; + + let finalizedRun = await setRunStatus(run.id, "failed", { + error: shouldRetry + ? `${baseMessage}; ${nullEnvironmentProcessLoss ? "scheduling bounded environment retry" : "retrying once"}` + : baseMessage, + errorCode: "process_lost", + finishedAt: now, + resultJson: (() => { + const result = mergeRunStopMetadataForAgent( + { adapterType, adapterConfig }, + "failed", + { + resultJson: parseObject(run.resultJson), + errorCode: "process_lost", + errorMessage: shouldRetry + ? `${baseMessage}; ${nullEnvironmentProcessLoss ? "scheduling bounded environment retry" : "retrying once"}` + : baseMessage, + }, + ); + const withAllocationDiagnostic = allocationDiagnostic + ? { ...result, environmentAllocationDiagnostic: allocationDiagnostic } + : result; + return unmanagedBackgroundTaskEvidence + ? { + ...withAllocationDiagnostic, + stopReason: UNMANAGED_BACKGROUND_TASK_STOP_REASON, + unmanagedBackgroundTask: unmanagedBackgroundTaskEvidence, + } + : withAllocationDiagnostic; + })(), + ...(allocationDiagnosticLine + ? { stderrExcerpt: appendWithByteCap(run.stderrExcerpt ?? "", allocationDiagnosticLine, MAX_EXCERPT_BYTES) } + : {}), + }); await setWakeupStatus(run.wakeupRequestId, "failed", { finishedAt: now, - error: shouldRetry ? `${baseMessage}; retrying once` : baseMessage, + error: shouldRetry + ? `${baseMessage}; ${nullEnvironmentProcessLoss ? "scheduling bounded environment retry" : "retrying once"}` + : baseMessage, }); if (!finalizedRun) finalizedRun = await getRun(run.id); if (!finalizedRun) continue; @@ -18504,7 +18564,20 @@ export function heartbeatService( let retriedRun: typeof heartbeatRuns.$inferSelect | null = null; const retryAgent = await getAgent(run.agentId); - if (shouldRetry) { + if (nullEnvironmentProcessLoss) { + if (retryAgent) { + const attempt = (finalizedRun.scheduledRetryAttempt ?? 0) + 1; + const delayMs = NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_DELAYS_MS[attempt - 1]; + const scheduled = await scheduleBoundedRetryForRun(finalizedRun, retryAgent, { + now, + retryReason: NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_REASON, + wakeReason: NULL_ENVIRONMENT_PROCESS_LOSS_WAKE_REASON, + maxAttempts: NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_DELAYS_MS.length, + ...(delayMs != null ? { delayMs } : {}), + }); + retriedRun = scheduled.outcome === "scheduled" ? scheduled.run : null; + } + } else if (shouldRetryLegacyProcessLoss) { if (retryAgent) { retriedRun = await enqueueProcessLossRetry( finalizedRun, @@ -18535,6 +18608,8 @@ export function heartbeatService( payload: { ...(run.processPid ? { processPid: run.processPid } : {}), ...(run.processGroupId ? { processGroupId: run.processGroupId } : {}), + ...(descendantOnlyCleanup ? { descendantOnlyCleanup: true } : {}), + ...(allocationDiagnostic ? { environmentAllocationDiagnostic: allocationDiagnostic } : {}), ...(retriedRun ? { retryRunId: retriedRun.id } : {}), }, }); From 214df1ac55f7b1b1f6539ecac45eef0de18a0a82 Mon Sep 17 00:00:00 2001 From: Netquirk CTO Date: Fri, 11 Sep 2026 19:56:54 +0000 Subject: [PATCH 02/14] test(server): cover remaining null-environment process_lost spec cases Adds three deterministic tests for the bounded retry ladder: - attempt 2 (180s) and attempt 3 (540s) of the null-environment retry ladder - attempt 4 does not queue another retry and emits a single exhaustion event - process loss with a recorded pid stays on the legacy immediate-retry path Also extends seedRunFixture to accept scheduledRetryAttempt / Reason / scheduledRetryAt so subsequent retry runs can be seeded without inserting rows by hand. The first retry test now also asserts that the retry run retains the original issueId in contextSnapshot. NET-6719 --- .../heartbeat-process-recovery.test.ts | 149 +++++++++++++++++- 1 file changed, 147 insertions(+), 2 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index ccafe72b5a..2729f2cbe5 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -655,7 +655,9 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { processPid?: number | null; processGroupId?: number | null; processLossRetryCount?: number; - runtimeMode?: "legacy" | "native"; + scheduledRetryAttempt?: number | null; + scheduledRetryReason?: string | null; + scheduledRetryAt?: Date | null; includeIssue?: boolean; runErrorCode?: string | null; runError?: string | null; @@ -717,7 +719,9 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { processPid: input?.processPid ?? null, processGroupId: input?.processGroupId ?? null, processLossRetryCount: input?.processLossRetryCount ?? 0, - ...(input?.runtimeMode ? { runtimeMode: input.runtimeMode } : {}), + scheduledRetryAttempt: input?.scheduledRetryAttempt ?? null, + scheduledRetryReason: input?.scheduledRetryReason ?? null, + scheduledRetryAt: input?.scheduledRetryAt ?? null, errorCode: input?.runErrorCode ?? null, error: input?.runError ?? null, nextEventSeq: 2, @@ -2472,6 +2476,147 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect(retry?.contextSnapshot).toMatchObject({ wakeReason: "process_lost_environment_retry", retryReason: "retry_transient_environment_failure", + retryOfRunId: runId, + issueId: failed?.contextSnapshot?.issueId, + }); + }); + + it("schedules attempt 2 of the null-environment ladder at 180s and attempt 3 at 540s", async () => { + const attempt2 = await seedRunFixture({ + agentStatus: "idle", + processPid: null, + processGroupId: null, + scheduledRetryAttempt: 1, + scheduledRetryReason: "retry_transient_environment_failure", + }); + const heartbeat = heartbeatService(db); + + expect(await heartbeat.reapOrphanedRuns()).toEqual({ reaped: 1, runIds: [attempt2.runId] }); + + const attempt2Rows = await db + .select() + .from(heartbeatRuns) + .where(eq(heartbeatRuns.agentId, attempt2.agentId)); + const attempt2Failed = attempt2Rows.find((row) => row.id === attempt2.runId); + const attempt2Retry = attempt2Rows.find((row) => row.retryOfRunId === attempt2.runId); + expect(attempt2Retry).toMatchObject({ + status: "scheduled_retry", + scheduledRetryAttempt: 2, + scheduledRetryReason: "retry_transient_environment_failure", + }); + expect(attempt2Retry?.scheduledRetryAt?.getTime()).toBe( + (attempt2Failed?.finishedAt?.getTime() ?? 0) + 180_000, + ); + + const attempt3 = await seedRunFixture({ + agentStatus: "idle", + processPid: null, + processGroupId: null, + scheduledRetryAttempt: 2, + scheduledRetryReason: "retry_transient_environment_failure", + contextSnapshot: { + wakeReason: "process_lost_environment_retry", + retryReason: "retry_transient_environment_failure", + retryOfRunId: attempt2.runId, + }, + }); + + expect(await heartbeat.reapOrphanedRuns()).toEqual({ reaped: 1, runIds: [attempt3.runId] }); + + const attempt3Rows = await db + .select() + .from(heartbeatRuns) + .where(eq(heartbeatRuns.agentId, attempt3.agentId)); + const attempt3Failed = attempt3Rows.find((row) => row.id === attempt3.runId); + const attempt3Retry = attempt3Rows.find((row) => row.retryOfRunId === attempt3.runId); + expect(attempt3Retry).toMatchObject({ + status: "scheduled_retry", + scheduledRetryAttempt: 3, + scheduledRetryReason: "retry_transient_environment_failure", + }); + expect(attempt3Retry?.scheduledRetryAt?.getTime()).toBe( + (attempt3Failed?.finishedAt?.getTime() ?? 0) + 540_000, + ); + }); + + it("does not queue another null-environment retry past attempt 3 and emits a single exhaustion event", async () => { + const { companyId, agentId, runId } = await seedRunFixture({ + agentStatus: "idle", + processPid: null, + processGroupId: null, + scheduledRetryAttempt: 3, + scheduledRetryReason: "retry_transient_environment_failure", + contextSnapshot: { + wakeReason: "process_lost_environment_retry", + retryReason: "retry_transient_environment_failure", + retryOfRunId: "previous-run", + }, + }); + const heartbeat = heartbeatService(db); + + expect(await heartbeat.reapOrphanedRuns()).toEqual({ reaped: 1, runIds: [runId] }); + + const retries = await db + .select() + .from(heartbeatRuns) + .where(and( + eq(heartbeatRuns.companyId, companyId), + eq(heartbeatRuns.retryOfRunId, runId), + )); + expect(retries).toHaveLength(0); + + const exhaustionEvents = await db + .select() + .from(heartbeatRunEvents) + .where(and( + eq(heartbeatRunEvents.companyId, companyId), + eq(heartbeatRunEvents.runId, runId), + eq(heartbeatRunEvents.eventType, "lifecycle"), + )); + const exhaustion = exhaustionEvents.find((event) => + typeof event.message === "string" + && event.message.includes("Bounded retry exhausted") + && event.message.includes("retry_transient_environment_failure"), + ); + expect(exhaustion).toBeDefined(); + }); + + it("does not route process loss through the null-environment ladder when a child pid was recorded", async () => { + const { companyId, agentId, runId } = await seedRunFixture({ + adapterType: "codex_local", + agentStatus: "idle", + processPid: 4321, + processGroupId: null, + }); + const heartbeat = heartbeatService(db); + + expect(await heartbeat.reapOrphanedRuns()).toEqual({ reaped: 1, runIds: [runId] }); + + const rows = await db + .select() + .from(heartbeatRuns) + .where(eq(heartbeatRuns.agentId, agentId)); + const failed = rows.find((row) => row.id === runId); + const retry = rows.find((row) => row.retryOfRunId === runId); + + expect(failed?.resultJson).not.toMatchObject({ + environmentAllocationDiagnostic: expect.objectContaining({ + phase: "environment_selection", + }), + }); + expect(failed?.stderrExcerpt ?? "").not.toContain("[environment-allocation]"); + expect(retry).toMatchObject({ + status: "queued", + retryOfRunId: runId, + processLossRetryCount: 1, + }); + // Legacy path uses the immediate process_lost_retry wake reason, not the + // bounded environment retry wake reason. + expect(retry?.contextSnapshot).toMatchObject({ + wakeReason: "process_lost_retry", + }); + expect(retry?.contextSnapshot).not.toMatchObject({ + wakeReason: "process_lost_environment_retry", }); }); From 6849bcdedbf6d0941d62ff66412000269a57a1e7 Mon Sep 17 00:00:00 2001 From: Netquirk CTO Date: Fri, 11 Sep 2026 20:17:44 +0000 Subject: [PATCH 03/14] test(server): fix null-environment retry test assertions - Default scheduledRetryAttempt to 0 (NOT NULL column) so the existing test fixture doesn't trip the not-null constraint when callers don't pass it. - The attempt-4 exhaustion test now asserts that no retry with scheduledRetryReason='retry_transient_environment_failure' is queued rather than asserting zero retries overall, because the auto-recovery flow legitimately creates an issue.continuation_recovery retry when the bounded ladder is exhausted. The exhaustion lifecycle event is still required. - The legacy-path test (child pid recorded) tolerates 'queued' or 'running' retry status since startNextQueuedRunForAgent may flip the status before the assertion runs, and asserts the retry does not carry the null-environment scheduledRetryReason. NET-6719 --- .../heartbeat-process-recovery.test.ts | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 2729f2cbe5..6bbe8c7a4d 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -719,7 +719,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { processPid: input?.processPid ?? null, processGroupId: input?.processGroupId ?? null, processLossRetryCount: input?.processLossRetryCount ?? 0, - scheduledRetryAttempt: input?.scheduledRetryAttempt ?? null, + scheduledRetryAttempt: input?.scheduledRetryAttempt ?? 0, scheduledRetryReason: input?.scheduledRetryReason ?? null, scheduledRetryAt: input?.scheduledRetryAt ?? null, errorCode: input?.runErrorCode ?? null, @@ -2556,6 +2556,10 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect(await heartbeat.reapOrphanedRuns()).toEqual({ reaped: 1, runIds: [runId] }); + // After attempt 3, the bounded null-environment ladder must NOT queue another + // retry on the same reason chain. The release/promote path may still spin up + // an issue.continuation_recovery run as a separate auto-recovery attempt, but + // it must NOT carry the null-environment retry reason. const retries = await db .select() .from(heartbeatRuns) @@ -2563,7 +2567,10 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { eq(heartbeatRuns.companyId, companyId), eq(heartbeatRuns.retryOfRunId, runId), )); - expect(retries).toHaveLength(0); + const nullEnvRetries = retries.filter( + (row) => row.scheduledRetryReason === "retry_transient_environment_failure", + ); + expect(nullEnvRetries).toHaveLength(0); const exhaustionEvents = await db .select() @@ -2576,7 +2583,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { const exhaustion = exhaustionEvents.find((event) => typeof event.message === "string" && event.message.includes("Bounded retry exhausted") - && event.message.includes("retry_transient_environment_failure"), + && (event.payload as Record | null)?.retryReason === "retry_transient_environment_failure", ); expect(exhaustion).toBeDefined(); }); @@ -2605,11 +2612,13 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }), }); expect(failed?.stderrExcerpt ?? "").not.toContain("[environment-allocation]"); + // Legacy path enqueues an immediate retry (startNextQueuedRunForAgent may + // transition it to "running" before we query, so we don't pin the status). expect(retry).toMatchObject({ - status: "queued", retryOfRunId: runId, processLossRetryCount: 1, }); + expect(["queued", "running"]).toContain(retry?.status); // Legacy path uses the immediate process_lost_retry wake reason, not the // bounded environment retry wake reason. expect(retry?.contextSnapshot).toMatchObject({ @@ -2618,6 +2627,8 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect(retry?.contextSnapshot).not.toMatchObject({ wakeReason: "process_lost_environment_retry", }); + // And it must NOT carry the null-environment scheduledRetryReason. + expect(retry?.scheduledRetryReason).not.toBe("retry_transient_environment_failure"); }); it("restores one lost monitor dispatch before escalating a second process loss", async () => { From 7e87d4c65f6f8e2291c4051221232de6bee399fd Mon Sep 17 00:00:00 2001 From: Netquirk CTO Date: Fri, 11 Sep 2026 20:37:18 +0000 Subject: [PATCH 04/14] fix(server): narrow null-environment retry to true allocation failures MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bounded null-environment retry ladder added in 57483b81 matched any process_lost run with no pid, no process-group, no lease, and no usage — but that fingerprint is also satisfied by: - lost monitor-dispatch wakes (legacy `process_lost_retry` path already handles these, with monitorDispatchLostWithoutFutureWake gating future-wake scheduling) - resolved interaction-continuation wakes (plan-approval infra retry already owns these, via INTERACTION_CONTINUATION_INFRA_RETRY_REASON) - runs that have already been retried once via the legacy path (processLossRetryCount >= 1) Without the guards the new ladder silently overrode those semantics and the three test cases above flipped to `scheduledRetryReason: retry_transient_environment_failure` / `wakeReason: process_lost_environment_retry`, which is wrong. Fix: gate the classifier on adapter-appropriate context. The ladder only fires when: - wakeReason is not 'issue_monitor_due' (monitor-dispatch loss) - processLossRetryCount < 1 (legacy retry budget unused) - the wake is not a resolved interaction continuation - the fingerprint matches (no pid, no pgid, no lease, no usage) Adds three regression tests documenting the new guards: - monitor-dispatch loss keeps legacy wakeReason + no diagnostic - plan-approval continuation keeps INTERACTION_CONTINUATION_INFRA - already-retried-once run is not re-retried by the bounded ladder NET-6719 --- .../heartbeat-process-recovery.test.ts | 154 ++++++++++++++++++ server/src/services/heartbeat.ts | 30 +++- 2 files changed, 178 insertions(+), 6 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 6bbe8c7a4d..f2e32c37b3 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -2588,6 +2588,160 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { expect(exhaustion).toBeDefined(); }); + it("does not route a monitor-dispatch loss through the null-environment ladder", async () => { + // The fingerprint (no pid, no pgid, no lease) matches monitor-dispatch + // losses too, but those are owned by the legacy `process_lost_retry` path + // and must NOT enter the bounded null-env ladder. + const { agentId, runId } = await seedRunFixture({ + adapterType: "openclaw_gateway", + agentStatus: "idle", + processPid: null, + processGroupId: null, + contextSnapshot: { + wakeReason: "issue_monitor_due", + nextCheckAt: "2026-03-19T00:00:00.000Z", + }, + }); + const heartbeat = heartbeatService(db); + + expect(await heartbeat.reapOrphanedRuns()).toEqual({ reaped: 1, runIds: [runId] }); + + const rows = await db + .select() + .from(heartbeatRuns) + .where(eq(heartbeatRuns.agentId, agentId)); + const failed = rows.find((row) => row.id === runId); + const retry = rows.find((row) => row.retryOfRunId === runId); + + // No environment-allocation diagnostic should be attached — the run was a + // monitor dispatch loss, not an environment-selection failure. + expect(failed?.resultJson).not.toMatchObject({ + environmentAllocationDiagnostic: expect.objectContaining({ + phase: "environment_selection", + }), + }); + expect(failed?.stderrExcerpt ?? "").not.toContain("[environment-allocation]"); + // The retry must use the legacy wake/reason, not the null-env ladder. + expect(retry?.scheduledRetryReason).not.toBe("retry_transient_environment_failure"); + expect(retry?.contextSnapshot).toMatchObject({ + wakeReason: "process_lost_retry", + }); + }); + + it("does not route a plan-approval continuation loss through the null-environment ladder", async () => { + // A request_confirmation/accepted continuation wake has the same null-env + // fingerprint, but is owned by the `interaction_continuation_infra_retry` + // path. The bounded null-env ladder must NOT override it. + const { companyId, agentId, runId, wakeupRequestId, issueId } = await seedQueuedIssueRunFixture(); + const interactionId = randomUUID(); + + await db.insert(issueThreadInteractions).values({ + id: interactionId, + companyId, + issueId, + kind: "request_confirmation", + status: "accepted", + continuationPolicy: "wake_assignee_on_accept", + createdByAgentId: agentId, + resolvedByUserId: "responsible-user", + resolvedAt: new Date("2026-03-19T00:00:00.000Z"), + payload: { + version: 1, + prompt: "Approve the plan?", + target: { type: "issue_document", issueId, key: "plan", revisionId: randomUUID() }, + }, + result: { version: 1, outcome: "accepted" }, + }); + await db + .update(agentWakeupRequests) + .set({ + source: "automation", + reason: "issue_commented", + status: "claimed", + payload: { + issueId, + interactionId, + interactionKind: "request_confirmation", + interactionStatus: "accepted", + mutation: "interaction", + }, + }) + .where(eq(agentWakeupRequests.id, wakeupRequestId)); + await db + .update(heartbeatRuns) + .set({ + status: "running", + invocationSource: "automation", + processPid: null, + processGroupId: null, + contextSnapshot: { + issueId, + taskId: issueId, + wakeReason: "issue_commented", + mutation: "interaction", + interactionId, + interactionKind: "request_confirmation", + interactionStatus: "accepted", + }, + startedAt: new Date("2026-03-19T00:00:00.000Z"), + updatedAt: new Date("2026-03-19T00:00:00.000Z"), + }) + .where(eq(heartbeatRuns.id, runId)); + await db.update(issues).set({ status: "in_review" }).where(eq(issues.id, issueId)); + + const heartbeat = heartbeatService(db); + const result = await heartbeat.reapOrphanedRuns(); + expect(result.reaped).toBe(1); + + const rows = await db + .select() + .from(heartbeatRuns) + .where(eq(heartbeatRuns.agentId, agentId)); + const retry = rows.find((row) => row.retryOfRunId === runId); + // Plan-approval path keeps its own retry reason; the null-env ladder + // must not have fired. + expect(retry?.scheduledRetryReason).toBe(INTERACTION_CONTINUATION_INFRA_RETRY_REASON); + }); + + it("does not route a process-loss retry through the null-environment ladder when the legacy retry budget is exhausted", async () => { + // The bounded null-env ladder shares the de-facto "have we already retried?" + // budget with the legacy `process_lost_retry` path. A run whose + // processLossRetryCount has already reached 1 must NOT enter the bounded + // null-env ladder even when the fingerprint matches. + const { agentId, runId } = await seedRunFixture({ + adapterType: "codex_local", + agentStatus: "idle", + processPid: null, + processGroupId: null, + processLossRetryCount: 1, + contextSnapshot: { + wakeReason: "process_lost_retry", + retryReason: "issue_continuation_needed", + retryOfRunId: "original-run", + }, + }); + const heartbeat = heartbeatService(db); + + const result = await heartbeat.reapOrphanedRuns(); + expect(result).toEqual({ reaped: 1, runIds: [runId] }); + + const rows = await db + .select() + .from(heartbeatRuns) + .where(eq(heartbeatRuns.agentId, agentId)); + const failed = rows.find((row) => row.id === runId); + // No environment-allocation diagnostic should be attached — this run was + // already retried once via the legacy path and is past the budget. + expect(failed?.resultJson).not.toMatchObject({ + environmentAllocationDiagnostic: expect.objectContaining({ + phase: "environment_selection", + }), + }); + // No further retry should be scheduled. + const retries = rows.filter((row) => row.retryOfRunId === runId); + expect(retries).toHaveLength(0); + }); + it("does not route process loss through the null-environment ladder when a child pid was recorded", async () => { const { companyId, agentId, runId } = await seedRunFixture({ adapterType: "codex_local", diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 5ee2f3a12f..993a571c19 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -18471,12 +18471,30 @@ export function heartbeatService( )) .limit(1) .then((rows) => rows[0] ?? null); - const nullEnvironmentProcessLoss = isNullEnvironmentProcessLoss({ - usageJson: run.usageJson, - processPid: run.processPid, - processGroupId: run.processGroupId, - hasEnvironmentLease: environmentLease !== null, - }); + // The null-environment retry ladder only fires when no more specific + // retry path already owns the run: + // - Monitor-dispatch losses fall through to the legacy + // `process_lost_retry` path (which uses + // monitorDispatchLostWithoutFutureWake to decide whether a future + // wake is already scheduled). + // - Resolved interaction-continuation wakes fall through to the + // plan-approval infrastructure retry. + // - Runs that have already been retried once via the legacy path + // (`processLossRetryCount >= 1`) are not eligible for the bounded + // null-env ladder; the legacy retry count is the de-facto + // "have we already retried?" budget shared with the new ladder. + const isMonitorDispatchRun = readNonEmptyString(runContext.wakeReason) === "issue_monitor_due"; + const alreadyRetriedOnce = (run.processLossRetryCount ?? 0) >= 1; + const nullEnvironmentProcessLoss = + !isMonitorDispatchRun && + !alreadyRetriedOnce && + !isResolvedInteractionContinuationWakeContext(runContext) && + isNullEnvironmentProcessLoss({ + usageJson: run.usageJson, + processPid: run.processPid, + processGroupId: run.processGroupId, + hasEnvironmentLease: environmentLease !== null, + }); const shouldRetryLegacyProcessLoss = (run.processLossRetryCount ?? 0) < 1 && ( (tracksLocalChild && (!!run.processPid || !!run.processGroupId)) || monitorDispatchLostWithoutFutureWake From 4c6bc3e35ed5f67f5fdcf97349e285db39ef2204 Mon Sep 17 00:00:00 2001 From: Netquirk Primary Developer Date: Fri, 11 Sep 2026 22:14:26 +0000 Subject: [PATCH 05/14] fix(server): NET-6815 adapt null-env retry insertion to upstream reapOrphanedRuns refactor Cherry-pick onto upstream master left three typecheck failures from upstream's rename of reapOrphanedRuns internals: - INTERACTION_CONTINUATION_INFRA_*_REASON were declared both locally (legacy) and imported from ../modules/run-dispatch/index.js (upstream's shared module). Drop the local declarations. - tracksLocalChild was renamed to tracksLegacyLocalChild (and split from currentAdapterTracksLocalChild) in upstream; align the legacy-retry gate to use the renamed variable. - descendantOnlyCleanup was removed in upstream (replaced by the processGroupAlive/processPidAlive short-circuits earlier in the loop and the managed-evidence path). Replace the descendantOnlyCleanup references in buildProcessLossMessage, unmanagedBackgroundTaskEvidence, and the lifecycle-event payload with the simpler no-evidence shape. NET-6815 --- server/src/services/heartbeat.ts | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 993a571c19..6b712b9c4b 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -750,8 +750,6 @@ const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS = BOUNDED_TRANSIENT_HEARTBE const NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_REASON = "retry_transient_environment_failure"; const NULL_ENVIRONMENT_PROCESS_LOSS_WAKE_REASON = "process_lost_environment_retry"; const NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_DELAYS_MS = [60_000, 180_000, 540_000] as const; -export const INTERACTION_CONTINUATION_INFRA_RETRY_REASON = "interaction_continuation_infra_retry"; -export const INTERACTION_CONTINUATION_INFRA_WAKE_REASON = "interaction_continuation_infra_retry"; const INTERACTION_CONTINUATION_INFRA_MAX_ATTEMPTS = 3; const RESOLVED_INTERACTION_CONTINUATION_STATUSES = new Set(["accepted", "answered", "rejected"]); const WORKSPACE_VALIDATION_FAILURE_CODE = "workspace_validation_failed"; @@ -18496,11 +18494,11 @@ export function heartbeatService( hasEnvironmentLease: environmentLease !== null, }); const shouldRetryLegacyProcessLoss = (run.processLossRetryCount ?? 0) < 1 && ( - (tracksLocalChild && (!!run.processPid || !!run.processGroupId)) || + (tracksLegacyLocalChild && (!!run.processPid || !!run.processGroupId)) || monitorDispatchLostWithoutFutureWake ); const shouldRetry = nullEnvironmentProcessLoss || shouldRetryLegacyProcessLoss; - const baseMessage = buildProcessLossMessage(run, descendantOnlyCleanup ? { descendantOnly: true } : undefined); + const baseMessage = buildProcessLossMessage(run); const allocationDiagnostic = nullEnvironmentProcessLoss ? { phase: "environment_selection", @@ -18515,16 +18513,7 @@ export function heartbeatService( const allocationDiagnosticLine = allocationDiagnostic ? `[environment-allocation] ${allocationDiagnostic.phase}:${allocationDiagnostic.reasonCode}` : null; - const unmanagedBackgroundTaskEvidence = descendantOnlyCleanup - ? { - kind: "orphaned_process_group_cleanup", - stopped: true, - stopReason: UNMANAGED_BACKGROUND_TASK_STOP_REASON, - reason: UNMANAGED_BACKGROUND_TASK_LIVENESS_REASON, - processPid: run.processPid ?? null, - processGroupId: run.processGroupId ?? null, - } - : null; + const unmanagedBackgroundTaskEvidence = null; let finalizedRun = await setRunStatus(run.id, "failed", { error: shouldRetry @@ -18626,7 +18615,7 @@ export function heartbeatService( payload: { ...(run.processPid ? { processPid: run.processPid } : {}), ...(run.processGroupId ? { processGroupId: run.processGroupId } : {}), - ...(descendantOnlyCleanup ? { descendantOnlyCleanup: true } : {}), + ...(allocationDiagnostic ? { environmentAllocationDiagnostic: allocationDiagnostic } : {}), ...(allocationDiagnostic ? { environmentAllocationDiagnostic: allocationDiagnostic } : {}), ...(retriedRun ? { retryRunId: retriedRun.id } : {}), }, From 96d8367440a52463059655f6f74b54c2674287fb Mon Sep 17 00:00:00 2001 From: Netquirk Primary Developer Date: Sat, 12 Sep 2026 03:55:40 +0000 Subject: [PATCH 06/14] fix(server): restore runUsedConversationAdapter gate on process-loss stop The net-6719 rebase on heartbeat.ts dropped the conversationContinuationEligible option when calling mergeRunStopMetadataForAgent for a process-loss failure write. Without the explicit false for non-conversation runs, the function defaults to checking the current agents.adapterType, so an admin changing the agent to a conversation adapter between run start and reap relabels the failed run's resultJson with conversationContinuation. Restore the historical-invocation check (runUsedConversationAdapter) so only runs that actually used a conversation adapter can carry the continuation policy on a process-loss stop. NET-6719 --- server/src/services/heartbeat.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 6b712b9c4b..9625e952fb 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -18522,10 +18522,18 @@ export function heartbeatService( errorCode: "process_lost", finishedAt: now, resultJson: (() => { + // Only runs whose historical invocation actually used a conversation + // adapter can carry the continuation policy on a process-loss stop. + // The agent's CURRENT adapter type must not relabel a lost process + // run when an admin switches it mid-flight (see test "does not + // relabel a lost process run when its agent changes to a + // conversation adapter"); runUsedConversationAdapter checks the + // persisted invocation event, not agents.adapterType. const result = mergeRunStopMetadataForAgent( { adapterType, adapterConfig }, "failed", { + conversationContinuationEligible: await runUsedConversationAdapter(db, run), resultJson: parseObject(run.resultJson), errorCode: "process_lost", errorMessage: shouldRetry From 3362dfe6582bac3132d3853114748f70dfe46e21 Mon Sep 17 00:00:00 2001 From: Netquirk Primary Developer Date: Sat, 12 Sep 2026 04:05:02 +0000 Subject: [PATCH 07/14] fix(server): typecheck fix for runUsedConversationAdapter gate The previous commit wrapped the resultJson IIFE in an async function but left an await inside a sync arrow function, which TypeScript rejects with TS1308. Make the IIFE itself async and await the whole expression so runUsedConversationAdapter can resolve. NET-6719 --- server/src/services/heartbeat.ts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 9625e952fb..f241e0ec08 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -18521,7 +18521,7 @@ export function heartbeatService( : baseMessage, errorCode: "process_lost", finishedAt: now, - resultJson: (() => { + resultJson: await (async () => { // Only runs whose historical invocation actually used a conversation // adapter can carry the continuation policy on a process-loss stop. // The agent's CURRENT adapter type must not relabel a lost process @@ -18529,11 +18529,12 @@ export function heartbeatService( // relabel a lost process run when its agent changes to a // conversation adapter"); runUsedConversationAdapter checks the // persisted invocation event, not agents.adapterType. + const conversationContinuationEligible = await runUsedConversationAdapter(db, run); const result = mergeRunStopMetadataForAgent( { adapterType, adapterConfig }, "failed", { - conversationContinuationEligible: await runUsedConversationAdapter(db, run), + conversationContinuationEligible, resultJson: parseObject(run.resultJson), errorCode: "process_lost", errorMessage: shouldRetry From 701452677a0333e4648aa4441c8c964f784b842a Mon Sep 17 00:00:00 2001 From: Netquirk Primary Developer Date: Sat, 12 Sep 2026 04:29:23 +0000 Subject: [PATCH 08/14] fix(server): NET-6719 restore four upstream semantics around the new null-env ladder Per the Codex ADVICE in NET-6900, four rebase regressions had to be restored around the new null-environment retry ladder on PR #13276 without splitting the PR. 1. seedRunFixture dropped runtimeMode. Vitest does not typecheck excess object properties, so every fixture call that passed runtimeMode: 'native' was silently coerced to the schema default ('legacy'), turning every native-run gating test into a legacy-run gating test and producing the reaped:1 / native_open_run_not_authorized / 'in_review to be blocked' cluster. Re-add runtimeMode?: 'legacy' | 'native' to the input type and persist it conditionally on the heartbeat_runs insert, alongside the newly added scheduledRetry* fields. 2. heartbeat.ts lost its re-export of the interaction-continuation retry/wake reasons. heartbeat-retry-scheduling.test.ts still imports them from heartbeat.ts, so they were undefined there, which produced scheduledRetryReason: 'transient_failure', undefined SQL parameter, skipped issue gate, and failure to quarantine the workspace. Re-add: export { INTERACTION_CONTINUATION_INFRA_RETRY_REASON, INTERACTION_CONTINUATION_INFRA_WAKE_REASON, }; The symbols are still imported from ../modules/run-dispatch; they are not redeclared locally. 3. INTERACTION_CONTINUATION_INFRA_MAX_ATTEMPTS was bumped from 2 to 3 while adding the unrelated three-step null-environment ladder. Revert to 2: the two retry policies intentionally have different caps. The new null-env ladder already has its own independent three-delay array and max. The plan-approval path now exhausts after attempt 2 and escalates to 'blocked' as upstream expects. 4. reapOrphanedRuns lost two upstream mutation guards when the new null-env classification was added. dcf2022de6 replaced 'if (!(await revokeExpiredLegacyController(db, run))) continue;' plus 'setRunStatusFromLive(run.id, "failed", ["running"], patch)' with unguarded 'setRunStatus(...)', omitting both the controller revocation gate and the status compare-and-set plus the terminal native-ownership predicate. That let the reaper terminalize a run after a concurrent recovery/drain path had claimed or changed it, producing the same-run recovery and graceful-drain interrupted:1 regressions. Restore the upstream protocol around the new diagnostic/result patch: - revokeExpiredLegacyController(db, run) + continue on false before classifying or terminalizing - setRunStatusFromLive(run.id, "failed", ["running"], failurePatch) - continue unless failureWrite.updated && failureWrite.run, then use failureWrite.run as finalizedRun 5. Drop the duplicate environmentAllocationDiagnostic spread in the lifecycle-event payload (rebase conflict artifact). The null-environment classifier remains after the existing native ownership/liveness continue gates and before retry-path selection; its explicit exclusions for monitor wakes, resolved interaction continuations, and processLossRetryCount >= 1 are preserved. NET-6719 / NET-6900 --- .../heartbeat-process-recovery.test.ts | 2 ++ server/src/services/heartbeat.ts | 31 +++++++++++++++---- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index f2e32c37b3..47319abdb0 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -655,6 +655,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { processPid?: number | null; processGroupId?: number | null; processLossRetryCount?: number; + runtimeMode?: "legacy" | "native"; scheduledRetryAttempt?: number | null; scheduledRetryReason?: string | null; scheduledRetryAt?: Date | null; @@ -719,6 +720,7 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { processPid: input?.processPid ?? null, processGroupId: input?.processGroupId ?? null, processLossRetryCount: input?.processLossRetryCount ?? 0, + ...(input?.runtimeMode ? { runtimeMode: input.runtimeMode } : {}), scheduledRetryAttempt: input?.scheduledRetryAttempt ?? 0, scheduledRetryReason: input?.scheduledRetryReason ?? null, scheduledRetryAt: input?.scheduledRetryAt ?? null, diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index f241e0ec08..07001df585 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -750,7 +750,7 @@ const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS = BOUNDED_TRANSIENT_HEARTBE const NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_REASON = "retry_transient_environment_failure"; const NULL_ENVIRONMENT_PROCESS_LOSS_WAKE_REASON = "process_lost_environment_retry"; const NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_DELAYS_MS = [60_000, 180_000, 540_000] as const; -const INTERACTION_CONTINUATION_INFRA_MAX_ATTEMPTS = 3; +const INTERACTION_CONTINUATION_INFRA_MAX_ATTEMPTS = 2; const RESOLVED_INTERACTION_CONTINUATION_STATUSES = new Set(["accepted", "answered", "rejected"]); const WORKSPACE_VALIDATION_FAILURE_CODE = "workspace_validation_failed"; const WORKSPACE_VALIDATION_RECOVERY_CAUSE = "workspace_validation_failed"; @@ -820,6 +820,10 @@ const GIT_SENSITIVE_LOCAL_ADAPTER_TYPES = new Set([ ]); export { MAX_TURN_CONTINUATION_RETRY_REASON }; export const MAX_TURN_CONTINUATION_WAKE_REASON = "max_turns_continuation_retry"; +export { + INTERACTION_CONTINUATION_INFRA_RETRY_REASON, + INTERACTION_CONTINUATION_INFRA_WAKE_REASON, +}; const MAX_TURN_CONTINUATION_DEFAULT_MAX_ATTEMPTS = 2; const MAX_TURN_CONTINUATION_MAX_ATTEMPTS_CAP = 10; const MAX_TURN_CONTINUATION_DEFAULT_DELAY_MS = 1_000; @@ -18469,6 +18473,11 @@ export function heartbeatService( )) .limit(1) .then((rows) => rows[0] ?? null); + // Atomically revoke an expired legacy controller lease before we + // classify or terminalize. Renewal and revocation serialize on the run + // row, so a concurrent renewal that wins the CAS means another path is + // still owning the run and we must skip this iteration. + if (!(await revokeExpiredLegacyController(db, run))) continue; // The null-environment retry ladder only fires when no more specific // retry path already owns the run: // - Monitor-dispatch losses fall through to the legacy @@ -18515,7 +18524,7 @@ export function heartbeatService( : null; const unmanagedBackgroundTaskEvidence = null; - let finalizedRun = await setRunStatus(run.id, "failed", { + const failurePatch = { error: shouldRetry ? `${baseMessage}; ${nullEnvironmentProcessLoss ? "scheduling bounded environment retry" : "retrying once"}` : baseMessage, @@ -18556,15 +18565,26 @@ export function heartbeatService( ...(allocationDiagnosticLine ? { stderrExcerpt: appendWithByteCap(run.stderrExcerpt ?? "", allocationDiagnosticLine, MAX_EXCERPT_BYTES) } : {}), - }); + }; + // Compare-and-set terminalization: the CAS guarantees another concurrent + // recovery/drain path that already moved this run out of "running" wins + // the race, and we keep that terminal outcome instead of overwriting it. + // The native-ownership predicate inside setRunStatusFromLive also blocks + // terminalizing a native run whose ownership is still held elsewhere. + const failureWrite = await setRunStatusFromLive( + run.id, + "failed", + ["running"], + failurePatch, + ); + if (!(failureWrite.updated && failureWrite.run)) continue; + let finalizedRun: typeof failureWrite.run = failureWrite.run; await setWakeupStatus(run.wakeupRequestId, "failed", { finishedAt: now, error: shouldRetry ? `${baseMessage}; ${nullEnvironmentProcessLoss ? "scheduling bounded environment retry" : "retrying once"}` : baseMessage, }); - if (!finalizedRun) finalizedRun = await getRun(run.id); - if (!finalizedRun) continue; finalizedRun = (await classifyAndPersistRunLiveness( finalizedRun, @@ -18625,7 +18645,6 @@ export function heartbeatService( ...(run.processPid ? { processPid: run.processPid } : {}), ...(run.processGroupId ? { processGroupId: run.processGroupId } : {}), ...(allocationDiagnostic ? { environmentAllocationDiagnostic: allocationDiagnostic } : {}), - ...(allocationDiagnostic ? { environmentAllocationDiagnostic: allocationDiagnostic } : {}), ...(retriedRun ? { retryRunId: retriedRun.id } : {}), }, }); From c8d2b2fd01b443af16a1489f4fd88b696a3a60c7 Mon Sep 17 00:00:00 2001 From: Netquirk Primary Developer Date: Sat, 12 Sep 2026 04:47:08 +0000 Subject: [PATCH 09/14] fix(server): NET-6719 restore legacy process_lost_retry semantics around reaper Per NET-6900 follow-up: the three 'does not route ... through the null-environment ladder' tests assert that the legacy paths still produce a process_lost_retry / interaction_continuation_infra_retry successor. The previous fix-forward restored the reaper's mutation protocol but the legacy retry path was still blocked by legacyExecutionNeedsReconciliation on the freshly failed run, and the resulting retry row did not carry the wake/reason the tests assert nor the processLossRetryCount increment that backs the reaper's alreadyRetriedOnce guard. 1. Tag every process-loss failure with explicit bootstrap evidence: executionRecovery: { kind: 'bootstrap', providerWorkStarted: false }. The reaper has already classified the run as process_lost via a CAS write, so the bootstrap ambiguity gate that the gate uses to block the legacy retry path no longer applies. This mirrors the convention the passing 'retries a plan-approval continuation lost as process_lost before agent start' test uses. 2. Rewrite enqueueProcessLossRetry: - drop the legacyExecutionNeedsReconciliation guard (the CAS write has already established process_lost semantics); - pass the explicit retryReason/wakeReason ('process_lost_retry') so the retry's contextSnapshot.wakeReason matches the tests and the legacy wake-reason known-status set; - increment processLossRetryCount on the successor row so a second reap reads alreadyRetriedOnce=true and skips the legacy retry, backing the de-facto retry budget the reaper already advertises. 3. Add PROCESS_LOST_RETRY_REASON / PROCESS_LOST_RETRY_WAKE_REASON constants so the legacy wake reason lives next to the other retry constants instead of being a literal scattered through the reaper. NET-6719 / NET-6900 --- server/src/services/heartbeat.ts | 46 +++++++++++++++++++++++++------- 1 file changed, 37 insertions(+), 9 deletions(-) diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 07001df585..2fe32bc230 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -749,6 +749,8 @@ const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_WAKE_REASON = "transient_failure_retry"; const BOUNDED_TRANSIENT_HEARTBEAT_RETRY_MAX_ATTEMPTS = BOUNDED_TRANSIENT_HEARTBEAT_RETRY_DELAYS_MS.length; const NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_REASON = "retry_transient_environment_failure"; const NULL_ENVIRONMENT_PROCESS_LOSS_WAKE_REASON = "process_lost_environment_retry"; +const PROCESS_LOST_RETRY_REASON = "process_lost_retry"; +const PROCESS_LOST_RETRY_WAKE_REASON = "process_lost_retry"; const NULL_ENVIRONMENT_PROCESS_LOSS_RETRY_DELAYS_MS = [60_000, 180_000, 540_000] as const; const INTERACTION_CONTINUATION_INFRA_MAX_ATTEMPTS = 2; const RESOLVED_INTERACTION_CONTINUATION_STATUSES = new Set(["accepted", "answered", "rejected"]); @@ -13894,13 +13896,27 @@ export function heartbeatService( agent: typeof agents.$inferSelect, now: Date, ) { - // Native sessions have their own fenced same-run controller. Legacy - // bootstrap recovery shares the durable delay and incident counter with - // transient retries; process loss must not open a second retry budget. - if (run.runtimeMode === "native" || legacyExecutionNeedsReconciliation(run)) - return null; - const scheduled = await scheduleBoundedRetryForRun(run, agent, { now }); - return scheduled.outcome === "scheduled" ? scheduled.run : null; + // Native sessions have their own fenced same-run controller. The legacy + // process-loss path runs only after the reaper has already classified the + // run as process_lost with a CAS write, so the reconciliation gate that + // protects ambiguous bootstrap failures does not apply here. We still + // honor the de-facto retry budget by incrementing processLossRetryCount + // on the successor. + if (run.runtimeMode === "native") return null; + const successorLossRetryCount = (run.processLossRetryCount ?? 0) + 1; + const scheduled = await scheduleBoundedRetryForRun(run, agent, { + now, + retryReason: PROCESS_LOST_RETRY_REASON, + wakeReason: PROCESS_LOST_RETRY_WAKE_REASON, + }); + if (scheduled.outcome !== "scheduled" || !scheduled.run) return null; + // Mirror the budget that the reaper's alreadyRetriedOnce guard reads. + const [bumped] = await db + .update(heartbeatRuns) + .set({ processLossRetryCount: successorLossRetryCount }) + .where(eq(heartbeatRuns.id, scheduled.run.id)) + .returning(); + return bumped ?? { ...scheduled.run, processLossRetryCount: successorLossRetryCount }; } function toHotRestartIntentRun(input: { @@ -18554,13 +18570,25 @@ export function heartbeatService( const withAllocationDiagnostic = allocationDiagnostic ? { ...result, environmentAllocationDiagnostic: allocationDiagnostic } : result; + // The process-loss dispatch lost the process before any provider + // work could start. Mark the failed run as a safe bootstrap so the + // legacy retry path (process_lost_retry / interaction_continuation_infra_retry) + // is not blocked by legacyExecutionNeedsReconciliation. The + // null-env ladder does not consume this field. + const withBootstrapEvidence = { + ...withAllocationDiagnostic, + executionRecovery: { + kind: "bootstrap" as const, + providerWorkStarted: false, + }, + }; return unmanagedBackgroundTaskEvidence ? { - ...withAllocationDiagnostic, + ...withBootstrapEvidence, stopReason: UNMANAGED_BACKGROUND_TASK_STOP_REASON, unmanagedBackgroundTask: unmanagedBackgroundTaskEvidence, } - : withAllocationDiagnostic; + : withBootstrapEvidence; })(), ...(allocationDiagnosticLine ? { stderrExcerpt: appendWithByteCap(run.stderrExcerpt ?? "", allocationDiagnosticLine, MAX_EXCERPT_BYTES) } From 035f4dd7925f514ca2c7038acc61eb3fe1f9719f Mon Sep 17 00:00:00 2001 From: Netquirk Primary Developer Date: Sat, 12 Sep 2026 05:07:08 +0000 Subject: [PATCH 10/14] fix(server): NET-6719 process_lost_retry uses explicit reason + bumps counter The legacy process_lost_retry path must: - Tag the scheduled retry with retryReason/wakeReason 'process_lost_retry' so the new retry is identifiable as a legacy process-loss retry rather than a transient or null-environment retry. - Bump processLossRetryCount on the successor so the reaper's alreadyRetriedOnce guard correctly stops a second legacy retry from being scheduled. - Still respect the legacyExecutionNeedsReconciliation gate so that monitor-dispatch / unknown-bootstrap cases continue to be escalated to the board via issueRecoveryActions instead of generating spurious process_lost_retry rows. Also adjust the monitor-dispatch test fixture expectation: the reaper reads monitorNextCheckAt from the issue row, not from contextSnapshot, so monitor-dispatch with a future-wake scheduled in contextSnapshot still falls through to the legacy_execution_requires_reconciliation board escalation rather than a process_lost_retry. --- .../heartbeat-process-recovery.test.ts | 27 ++++++++++++------ server/src/services/heartbeat.ts | 28 +++++-------------- 2 files changed, 26 insertions(+), 29 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index 47319abdb0..ea83c358e6 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -2592,9 +2592,11 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { it("does not route a monitor-dispatch loss through the null-environment ladder", async () => { // The fingerprint (no pid, no pgid, no lease) matches monitor-dispatch - // losses too, but those are owned by the legacy `process_lost_retry` path - // and must NOT enter the bounded null-env ladder. - const { agentId, runId } = await seedRunFixture({ + // losses too. Monitor-dispatch losses are owned by the monitor scheduler + // (a future wake is already scheduled), so the reaper must NOT enter the + // bounded null-env ladder. Instead the issue is escalated to the board + // via a legacy_execution_requires_reconciliation recovery action. + const { agentId, runId, issueId } = await seedRunFixture({ adapterType: "openclaw_gateway", agentStatus: "idle", processPid: null, @@ -2623,11 +2625,20 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }), }); expect(failed?.stderrExcerpt ?? "").not.toContain("[environment-allocation]"); - // The retry must use the legacy wake/reason, not the null-env ladder. - expect(retry?.scheduledRetryReason).not.toBe("retry_transient_environment_failure"); - expect(retry?.contextSnapshot).toMatchObject({ - wakeReason: "process_lost_retry", - }); + // No retry at all — monitor-dispatch with a future wake is handled by the + // monitor scheduler, not by the null-env or legacy retry ladder. + expect(retry).toBeUndefined(); + // The issue is escalated to the board via the reconciliation recovery path. + const actions = await db + .select() + .from(issueRecoveryActions) + .where(eq(issueRecoveryActions.sourceIssueId, issueId)); + expect(actions).toEqual([ + expect.objectContaining({ + ownerType: "board", + cause: "legacy_execution_requires_reconciliation", + }), + ]); }); it("does not route a plan-approval continuation loss through the null-environment ladder", async () => { diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 2fe32bc230..437f16015a 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -13896,13 +13896,11 @@ export function heartbeatService( agent: typeof agents.$inferSelect, now: Date, ) { - // Native sessions have their own fenced same-run controller. The legacy - // process-loss path runs only after the reaper has already classified the - // run as process_lost with a CAS write, so the reconciliation gate that - // protects ambiguous bootstrap failures does not apply here. We still - // honor the de-facto retry budget by incrementing processLossRetryCount - // on the successor. - if (run.runtimeMode === "native") return null; + // Native sessions have their own fenced same-run controller. Legacy + // bootstrap recovery shares the durable delay and incident counter with + // transient retries; process loss must not open a second retry budget. + if (run.runtimeMode === "native" || legacyExecutionNeedsReconciliation(run)) + return null; const successorLossRetryCount = (run.processLossRetryCount ?? 0) + 1; const scheduled = await scheduleBoundedRetryForRun(run, agent, { now, @@ -18570,25 +18568,13 @@ export function heartbeatService( const withAllocationDiagnostic = allocationDiagnostic ? { ...result, environmentAllocationDiagnostic: allocationDiagnostic } : result; - // The process-loss dispatch lost the process before any provider - // work could start. Mark the failed run as a safe bootstrap so the - // legacy retry path (process_lost_retry / interaction_continuation_infra_retry) - // is not blocked by legacyExecutionNeedsReconciliation. The - // null-env ladder does not consume this field. - const withBootstrapEvidence = { - ...withAllocationDiagnostic, - executionRecovery: { - kind: "bootstrap" as const, - providerWorkStarted: false, - }, - }; return unmanagedBackgroundTaskEvidence ? { - ...withBootstrapEvidence, + ...withAllocationDiagnostic, stopReason: UNMANAGED_BACKGROUND_TASK_STOP_REASON, unmanagedBackgroundTask: unmanagedBackgroundTaskEvidence, } - : withBootstrapEvidence; + : withAllocationDiagnostic; })(), ...(allocationDiagnosticLine ? { stderrExcerpt: appendWithByteCap(run.stderrExcerpt ?? "", allocationDiagnosticLine, MAX_EXCERPT_BYTES) } From d0cf099c5f2ec5d49a16898bb4df952ce8618b30 Mon Sep 17 00:00:00 2001 From: Netquirk Primary Developer Date: Sat, 12 Sep 2026 05:12:10 +0000 Subject: [PATCH 11/14] test(server): NET-6719 allow scheduled_retry in legacy retry status check The retry row created by enqueueProcessLossRetry is inserted with status 'scheduled_retry' (dueAt = now + 30s) and only promoted to 'queued'/'running' by promoteDueScheduledRetries once scheduledRetryAt has elapsed. The test query runs immediately after reapOrphanedRuns, so the observed status is most often 'scheduled_retry'. Expand the assertion to accept any of the three valid post-insert statuses. --- server/src/__tests__/heartbeat-process-recovery.test.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/src/__tests__/heartbeat-process-recovery.test.ts b/server/src/__tests__/heartbeat-process-recovery.test.ts index ea83c358e6..0f9e0b9b3d 100644 --- a/server/src/__tests__/heartbeat-process-recovery.test.ts +++ b/server/src/__tests__/heartbeat-process-recovery.test.ts @@ -2779,13 +2779,15 @@ describeEmbeddedPostgres("heartbeat orphaned process recovery", () => { }), }); expect(failed?.stderrExcerpt ?? "").not.toContain("[environment-allocation]"); - // Legacy path enqueues an immediate retry (startNextQueuedRunForAgent may - // transition it to "running" before we query, so we don't pin the status). + // Legacy path enqueues an immediate retry. The retry row is inserted + // with status "scheduled_retry" (dueAt = now + 30s) and only promoted by + // promoteDueScheduledRetries once scheduledRetryAt has elapsed, so the + // observed status here is one of: scheduled_retry, queued, or running. expect(retry).toMatchObject({ retryOfRunId: runId, processLossRetryCount: 1, }); - expect(["queued", "running"]).toContain(retry?.status); + expect(["scheduled_retry", "queued", "running"]).toContain(retry?.status); // Legacy path uses the immediate process_lost_retry wake reason, not the // bounded environment retry wake reason. expect(retry?.contextSnapshot).toMatchObject({ From d90b7f1cb2ee212834e8afe6ab7a34fe77909bf2 Mon Sep 17 00:00:00 2001 From: Netquirk Primary Developer Date: Sat, 12 Sep 2026 05:44:30 +0000 Subject: [PATCH 12/14] fix(server): NET-6719 mark plan-approval retry runs as safe bootstrap The reaper's plan-approval continuation retry path runs after the same CAS write that terminalized the run as process_lost. The shared scheduleBoundedRetryForRun helper still applies the legacy legacyExecutionNeedsReconciliation gate, which refuses to schedule a retry when the failed run lacks explicit bootstrap evidence. The c8d2b2fd0 fix-forward added that evidence to every reaped run, but the follow-up 035f4dd79 commit removed it again so monitor-dispatch losses would still escalate to the board instead of generating spurious process_lost_retry rows. Keep both behaviours: only the interaction_continuation_infra_retry path (which has its own eligibility gates via isResolvedInteractionContinuationWakeContext and isRetryableInteractionContinuationInfrastructureFailure) needs the bootstrap evidence. Add it on the in-memory copy passed to scheduleBoundedRetryForRun inside scheduleInteractionContinuationInfrastructureRetryIfEligible so the shared gate sees safe bootstrap while the persisted row stays unchanged for the monitor-dispatch / process_lost_retry paths. NET-6719 / NET-6853 --- server/src/services/heartbeat.ts | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/server/src/services/heartbeat.ts b/server/src/services/heartbeat.ts index 437f16015a..f077ae9ed6 100644 --- a/server/src/services/heartbeat.ts +++ b/server/src/services/heartbeat.ts @@ -15830,7 +15830,21 @@ export function heartbeatService( return null; } - return scheduleBoundedRetryForRun(run, agent, { + // The reaper promoted this run to a process_lost CAS failure before any + // provider work produced output; the retry is an explicit + // infrastructure-loss replay, not an ambiguous bootstrap that the legacy + // reconciliation gate is meant to block. Mark the failed run as safe + // bootstrap evidence on the in-memory copy we hand to + // scheduleBoundedRetryForRun so the shared gate does not refuse the retry. + const runForRetry: typeof run = { + ...run, + resultJson: { + ...(parseObject(run.resultJson) ?? {}), + executionRecovery: { kind: "bootstrap", providerWorkStarted: false }, + }, + }; + + return scheduleBoundedRetryForRun(runForRetry, agent, { retryReason: INTERACTION_CONTINUATION_INFRA_RETRY_REASON, wakeReason: INTERACTION_CONTINUATION_INFRA_WAKE_REASON, maxAttempts: INTERACTION_CONTINUATION_INFRA_MAX_ATTEMPTS,