diff --git a/.changeset/fn-8569-reports-health-error-unrecoverable.md b/.changeset/fn-8569-reports-health-error-unrecoverable.md new file mode 100644 index 0000000000..3d58df796d --- /dev/null +++ b/.changeset/fn-8569-reports-health-error-unrecoverable.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Surface unrecoverable direct-report failures in Reports Health Check. +category: fix +dev: Health classification now honors pause markers even when a live state is stale. diff --git a/docs/architecture.md b/docs/architecture.md index 4aa2b4a907..90333db333 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -682,6 +682,7 @@ Runtime action-gate flow (v1): - `GridlockDetector` (`gridlock-detector.ts`) — detects all-blocked todo pipelines and emits notification events (plus explicit clear signals when gridlock resolves) - `TransientErrorDetector` (`transient-error-detector.ts`) — retriable error classification - Durable agent error recovery (FN-7835/FN-7844/FN-7859/FN-7878/FN-7884): a heartbeat-managed, runtime-enabled non-ephemeral agent that lands in `state:"error"` remains timer-eligible and clears `lastError` by transitioning `error → active` at the next heartbeat run entry when `lastError` is recoverable. Generic/unknown errors are recoverable by default; immediate `error-unrecoverable` parking is reserved for operator-actionable auth/model/billing/quota failures, while stale worktree/module-resolution errors stay on their dedicated self-healing suppression/rebuild path. Recovery is bounded by one shared `heartbeatErrorRecovery` attempt budget (`MAX_HEARTBEAT_ERROR_RECOVERY_ATTEMPTS`, settings-overridable through the engine's optional cast-based knob) across both the timer path and `SelfHealingManager.recoverOrphanedAgents()`. Self-healing is the stale-agent backstop and still stores `durableErrorRecovery` cooldown/stale-module metadata, but it writes/reads the shared heartbeat counter and emits the same `agent:auto-recover-error-state` / `agent:error-retry-exhausted` audit surface with `source:"self-healing"`. The sweep flips `error → active` before `restartDurableAgentHeartbeat()` calls `executeHeartbeat()`, preventing run-entry recovery from re-counting or double-emitting for the same recovery. Success resets the shared counter and clears legacy sweep retry state; budget exhaustion parks the agent `paused` with `pauseReason:"error-retry-exhausted"`. On engine startup, `SelfHealingManager.resetDurableAgentErrorStateOnStartup()` runs before the steady-state sweep and treats restart as an explicit operator retry: eligible `error` and `error-retry-exhausted` durable agents have shared/legacy retry metadata reset, `lastError` and the exhaustion pause cleared, state set to `active`, heartbeat re-armed, and `agent:reset-error-state-on-startup` emitted without applying the sweep's staleness/cooldown/exhaustion gates. Non-recoverable durable heartbeat errors are not restarted; timer, startup, and sweep paths preserve exclusions for disabled runtime agents, ephemeral agents, active executions, user pauses, `error-unrecoverable` parks, operator-actionable errors, and stale worktree/module-resolution suppression. +- Reports Health Check (FN-8569): engine-side `classifyReportHealth` treats any non-empty `pauseReason` as authoritative over `state`, so a live-looking row with an `error-unrecoverable`, retry-exhausted, or model-unavailable park marker is rendered operator-actionable rather than healthy. The classifier deliberately excludes `lastError`, which remains diagnostic history; `@fusion/core` must not import this engine helper. `AgentStore.updateAgentState()` performs best-effort `pauseReason` cleanup only when resuming from `paused`/`error` into a live state, preserves `lastError`, and does not make state/marker writes atomic because independent marker writers can still create a desynced row. - `SelfHealingManager` (`self-healing.ts`) — auto-unpause/maintenance recovery actions - Batch 1 maintenance includes `reconcile-orphaned-task-dirs` (FN-6783), a paused-safe housekeeping step that calls `TaskStore.reconcileOrphanedTaskDirs()` so valid live `.fusion/tasks/{ID}/task.json` records missing from PostgreSQL become visible without waiting for process restart. The guard skips any ID already present in active, soft-deleted, archived, or tombstoned storage and emits `task:reconcile-orphaned-task-dir` only for recovered rows. - Batch 1 maintenance also includes `reconcile-phantom-committed-reservations` (FN-7069), which calls `TaskStore.reconcilePhantomCommittedReservations()` for committed task-ID reservations that have no live/soft-deleted/archived task row and no `.fusion/tasks/{ID}/task.json`. The sweep prunes orphaned `activityLog` rows and `agents`/cascaded `agentRuns`, preserves `runAuditEvents`, and keeps the reservation `committed` per FN-5105 so the ID is permanently reserved rather than resurrected or handed out again. diff --git a/packages/core/src/__tests__/agent-store-pause-marker-clear.test.ts b/packages/core/src/__tests__/agent-store-pause-marker-clear.test.ts new file mode 100644 index 0000000000..d7d8501313 --- /dev/null +++ b/packages/core/src/__tests__/agent-store-pause-marker-clear.test.ts @@ -0,0 +1,65 @@ +import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it } from "vitest"; +import { AgentStore } from "../agent-store.js"; +import { + createSharedPgTaskStoreTestHarness, + pgDescribe, + type SharedPgTaskStoreHarness, +} from "../__test-utils__/pg-test-harness.js"; + +pgDescribe("AgentStore pause marker resume cleanup (FN-8569)", () => { + const h: SharedPgTaskStoreHarness = createSharedPgTaskStoreTestHarness({ + prefix: "fusion_pause_marker_clear", + }); + let agentStore: AgentStore; + + beforeAll(h.beforeAll); + afterAll(h.afterAll); + beforeEach(async () => { + await h.beforeEach(); + agentStore = new AgentStore({ rootDir: h.rootDir(), asyncLayer: h.layer(), taskStore: h.store() }); + await agentStore.init(); + }); + afterEach(async () => { + try { agentStore?.close(); } catch { /* best-effort */ } + await h.afterEach(); + }); + + it("clears only stale pauseReason when a parked agent resumes", async () => { + const agent = await agentStore.createAgent({ name: "Parked Agent", role: "engineer" }); + await agentStore.updateAgentState(agent.id, "active"); + await agentStore.updateAgent(agent.id, { + pauseReason: "error-unrecoverable", + lastError: "credential access requires operator repair", + }); + await agentStore.updateAgentState(agent.id, "paused"); + + const resumed = await agentStore.updateAgentState(agent.id, "active"); + const persisted = await agentStore.getAgent(agent.id); + + expect(resumed.pauseReason).toBeUndefined(); + expect(persisted).toMatchObject({ + state: "active", + lastError: "credential access requires operator repair", + }); + expect(persisted?.pauseReason).toBeUndefined(); + }); + + it("preserves marker writes for paused states and direct post-resume interleavings", async () => { + const agent = await agentStore.createAgent({ name: "Interleaved Agent", role: "engineer" }); + await agentStore.updateAgentState(agent.id, "active"); + await agentStore.updateAgent(agent.id, { pauseReason: "user-requested" }); + const parked = await agentStore.updateAgentState(agent.id, "paused"); + expect(parked.pauseReason).toBe("user-requested"); + + const stillParked = await agentStore.updateAgentState(agent.id, "paused"); + expect(stillParked.pauseReason).toBe("user-requested"); + + await agentStore.updateAgentState(agent.id, "active"); + const desynced = await agentStore.updateAgent(agent.id, { pauseReason: "error-unrecoverable" }); + expect(desynced).toMatchObject({ state: "active", pauseReason: "error-unrecoverable" }); + await expect(agentStore.getAgent(agent.id)).resolves.toMatchObject({ + state: "active", + pauseReason: "error-unrecoverable", + }); + }); +}); diff --git a/packages/core/src/agent-store.ts b/packages/core/src/agent-store.ts index 2032dc8f6b..1329761197 100644 --- a/packages/core/src/agent-store.ts +++ b/packages/core/src/agent-store.ts @@ -1434,9 +1434,21 @@ export class AgentStore extends EventEmitter { ); } + /* + * FNXC:AgentStore 2026-07-24-12:00: + * FN-8569 observed CEO Reports Health rows with a live state and stale + * `error-unrecoverable` marker. Resuming from paused/error performs + * best-effort pauseReason cleanup only; it is not an atomicity guarantee + * because independent updateAgent marker writes can recreate desync. The + * engine-side reports-health classifier remains the authoritative defense. + * Preserve lastError as diagnostic history for operator triage. + */ + const clearsPauseReasonOnResume = (currentState === "paused" || currentState === "error") + && (newState === "active" || newState === "idle" || newState === "running"); const updated: Agent = { ...agent, state: newState, + ...(clearsPauseReasonOnResume && { pauseReason: undefined }), updatedAt: new Date().toISOString(), }; diff --git a/packages/engine/src/__tests__/heartbeat-executor.test.ts b/packages/engine/src/__tests__/heartbeat-executor.test.ts index 941f638fc1..e38f9e5804 100644 --- a/packages/engine/src/__tests__/heartbeat-executor.test.ts +++ b/packages/engine/src/__tests__/heartbeat-executor.test.ts @@ -277,6 +277,26 @@ describe("executeHeartbeat", () => { expect(section).toContain("healthy"); }); + it("FN-8569: renders error-unrecoverable reports as operator-actionable across desynced states", async () => { + const now = new Date().toISOString(); + const store = createStoreWithAgentForExec(); + vi.mocked(store.getAgentsByReportsTo).mockResolvedValue([ + { id: "agent-paused", name: "Paused Report", state: "paused", pauseReason: "error-unrecoverable", lastHeartbeatAt: now, updatedAt: now } as Agent, + { id: "agent-active", name: "Active Desync", state: "active", pauseReason: "error-unrecoverable", lastHeartbeatAt: now, updatedAt: now } as Agent, + { id: "agent-running", name: "Running Desync", state: "running", pauseReason: "error-unrecoverable", lastHeartbeatAt: now, updatedAt: now } as Agent, + ]); + const monitor = new HeartbeatMonitor({ store, taskStore: mockTaskStore, rootDir: "/tmp" }); + + const section = await (monitor as any).buildReportsHealthSection("agent-001", store); + + expect(section).toContain("| Paused Report | paused |"); + expect(section).toContain("| Active Desync | active |"); + expect(section).toContain("| Running Desync | running |"); + expect(section).toContain("**needs operator repair** (error-unrecoverable)"); + expect(section).not.toMatch(/\| (Paused Report|Active Desync|Running Desync) \|[^\n]*\| healthy \|/); + expect(section).toContain("For reports that **need operator repair**: notify the operator"); + }); + it("FN-8184: reports runtimeConfig cadence with one multiplier application and strict stale boundary", async () => { vi.useFakeTimers(); const now = new Date("2026-01-01T12:00:00.000Z"); @@ -385,7 +405,7 @@ describe("executeHeartbeat", () => { expect(section).not.toContain("**stale** assignment"); }); - it("buildReportsHealthSection classifies stuck agents", async () => { + it("buildReportsHealthSection classifies error-state agents as operator-actionable", async () => { const now = Date.now(); const store = createStoreWithAgentForExec(); vi.mocked(store.getAgentsByReportsTo).mockResolvedValue([ @@ -394,10 +414,9 @@ describe("executeHeartbeat", () => { const monitor = new HeartbeatMonitor({ store, taskStore: mockTaskStore, rootDir: "/tmp", heartbeatTimeoutMs: 60_000 }); const section = await (monitor as any).buildReportsHealthSection("agent-001", store); - expect(section).toContain("**stuck**"); + expect(section).toContain("**needs operator repair**"); expect(section).toContain("Actions for Unresponsive Reports"); - expect(section).toContain("first check task-log/step progress, the active heartbeat run, and worktree/session liveness"); - expect(section).toContain("If work is live, do not stop or reassign it"); + expect(section).toContain("For reports that **need operator repair**: notify the operator"); }); it("buildReportsHealthSection classifies stale agents", async () => { diff --git a/packages/engine/src/__tests__/reports-health.test.ts b/packages/engine/src/__tests__/reports-health.test.ts new file mode 100644 index 0000000000..c335825c6e --- /dev/null +++ b/packages/engine/src/__tests__/reports-health.test.ts @@ -0,0 +1,73 @@ +import { describe, expect, it } from "vitest"; +import { classifyReportHealth, type ReportHealthInput } from "../reports-health.js"; + +const baseInput: ReportHealthInput = { + state: "active", + pauseReason: undefined, + heartbeatAgeMs: 1_000, + heartbeatTimeoutMs: 10_000, + staleThresholdMs: 20_000, + staleParkedAssignment: false, +}; + +describe("classifyReportHealth", () => { + it.each(["paused", "active", "running", "idle"] as const)( + "renders error-unrecoverable marker as operator-actionable for %s state", + (state) => { + const result = classifyReportHealth({ ...baseInput, state, pauseReason: "error-unrecoverable" }); + + expect(result).toMatchObject({ + bucket: "operator-actionable", + cellText: expect.stringContaining("needs operator repair"), + }); + expect(result.cellText).not.toContain("healthy"); + }, + ); + + it("tolerates a marker interleaved onto a resumed persisted row", () => { + const persistedDesync = { + state: "active", + pauseReason: "error-unrecoverable", + }; + + expect(classifyReportHealth({ ...baseInput, ...persistedDesync }).bucket).toBe("operator-actionable"); + }); + + it.each(["error-retry-exhausted", "heartbeat-model-unavailable"])( + "renders %s marker as operator-actionable", + (pauseReason) => { + expect(classifyReportHealth({ ...baseInput, pauseReason }).bucket).toBe("operator-actionable"); + }, + ); + + it.each(["user-requested", "awaiting-approval", "budget-exhausted"])( + "renders non-operator marker %s as paused", + (pauseReason) => { + expect(classifyReportHealth({ ...baseInput, pauseReason })).toEqual({ + bucket: "paused", + cellText: `paused (${pauseReason})`, + }); + }, + ); + + it("preserves existing state and freshness buckets when no marker exists", () => { + expect(classifyReportHealth({ ...baseInput, state: "error" }).bucket).toBe("operator-actionable"); + expect(classifyReportHealth({ ...baseInput, staleParkedAssignment: true }).bucket).toBe("stale-assignment"); + expect(classifyReportHealth({ ...baseInput, state: "running", heartbeatAgeMs: 20_001 }).bucket).toBe("stuck"); + expect(classifyReportHealth({ ...baseInput, state: "active", heartbeatAgeMs: 20_001 }).bucket).toBe("stale"); + expect(classifyReportHealth({ ...baseInput, state: "idle", heartbeatAgeMs: 20_001 }).bucket).toBe("stale"); + expect(classifyReportHealth({ ...baseInput, state: "paused" })).toEqual({ bucket: "paused", cellText: "paused" }); + expect(classifyReportHealth({ ...baseInput, state: undefined })).toEqual({ bucket: "healthy", cellText: "healthy" }); + }); + + it.each([undefined, "", " "])("treats empty marker %j as unmarked", (pauseReason) => { + expect(classifyReportHealth({ ...baseInput, pauseReason })).toEqual({ bucket: "healthy", cellText: "healthy" }); + }); + + it("does not accept lastError as a classification input", () => { + const withResidualError = { ...baseInput, lastError: "long residual diagnostic text" }; + const withoutResidualError = { ...baseInput }; + + expect(classifyReportHealth(withResidualError)).toEqual(classifyReportHealth(withoutResidualError)); + }); +}); diff --git a/packages/engine/src/agent-heartbeat.ts b/packages/engine/src/agent-heartbeat.ts index 6d07aa3dde..4427e5b1f0 100644 --- a/packages/engine/src/agent-heartbeat.ts +++ b/packages/engine/src/agent-heartbeat.ts @@ -97,6 +97,7 @@ import { trimPromptMd, trimTaskDescription, trimTriggeringComments } from "./hea import { detectDeicticReference, extractAntecedentCandidates, renderAmbiguityPromptBlock, scoreReferentConfidence } from "./room-ambiguity.js"; import { countActiveAgentMembers, decideRoomCoordination, detectTaskFilingIntent, renderRoomCoordinationPromptBlock } from "./room-coordination.js"; import { evaluateParkedAgentTaskLink, isParkedTaskColumn, type AgentTaskLinkExecutionProof } from "./task-agent-sync.js"; +import { classifyReportHealth } from "./reports-health.js"; import { accumulateSessionTokenUsage, captureSessionTokenBaseline } from "./session-token-usage.js"; const promptSizeLog = createLogger("prompt-size"); @@ -3655,28 +3656,30 @@ export class HeartbeatMonitor { } } - let health = "healthy"; - if (staleParkedAssignment) { - health = "**stale** assignment"; - } else if (report.state === "paused") { - health = report.pauseReason ? `paused (${report.pauseReason})` : "paused"; - } else if (report.state === "error") { - health = "**stuck**"; - } else if (report.state === "running") { - health = heartbeatAgeMs <= heartbeatTimeoutMs * 2 ? "healthy" : "**stuck**"; - } else if ((report.state === "active" || report.state === "idle") && heartbeatAgeMs > staleThresholdMs) { - health = "**stale**"; + const classification = classifyReportHealth({ + state: report.state, + pauseReason: report.pauseReason, + heartbeatAgeMs, + heartbeatTimeoutMs, + staleThresholdMs, + staleParkedAssignment, + }); + if (classification.bucket === "stale") { heartbeatLog.log(`[reports-health] stale report ${report.id} intervalSource=${intervalSource} staleThresholdMs=${staleThresholdMs} heartbeatAgeMs=${heartbeatAgeMs}`); } const task = renderedTask; const state = renderedState; const heartbeat = formatRelativeTime(report.lastHeartbeatAt); - return `| ${report.name} | ${state} | ${task} | ${heartbeat} | ${health} |`; + return { + classification, + row: `| ${report.name} | ${state} | ${task} | ${heartbeat} | ${classification.cellText} |`, + }; })); - const hasStuck = rows.some((row) => row.includes("**stuck**")); - const hasStale = rows.some((row) => row.includes("**stale**")); + const hasStuck = rows.some(({ classification }) => classification.bucket === "stuck"); + const hasStale = rows.some(({ classification }) => classification.bucket === "stale" || classification.bucket === "stale-assignment"); + const hasOperatorActionable = rows.some(({ classification }) => classification.bucket === "operator-actionable"); const actionLines = ["### Actions for Unresponsive Reports"]; if (hasStuck) { @@ -3693,6 +3696,9 @@ export class HeartbeatMonitor { if (hasStale) { actionLines.push("- For **stale** reports: the agent may have lost its heartbeat trigger — create a follow-up task to investigate."); } + if (hasOperatorActionable) { + actionLines.push("- For reports that **need operator repair**: notify the operator and create a follow-up task; do not reassign work until the parked agent's configuration or access issue is repaired."); + } return [ "## Reports Health Check", @@ -3701,7 +3707,7 @@ export class HeartbeatMonitor { "", "| Name | State | Task | Last Heartbeat | Health |", "|------|-------|------|----------------|--------|", - ...rows, + ...rows.map(({ row }) => row), "", ...actionLines, ].join("\n"); diff --git a/packages/engine/src/index.ts b/packages/engine/src/index.ts index d346294d31..35c278583a 100644 --- a/packages/engine/src/index.ts +++ b/packages/engine/src/index.ts @@ -1,4 +1,10 @@ export { AgentLogger, type AgentLoggerOptions, summarizeToolArgs } from "./agent-logger.js"; +export { + classifyReportHealth, + type ReportHealthBucket, + type ReportHealthClassification, + type ReportHealthInput, +} from "./reports-health.js"; export { reloadExemptTools, addToExemptTools, getExemptToolNames } from "./agent-action-gate.js"; export type { AgentActionGateContext } from "./agent-action-gate.js"; export { createFusionAuthStorage, createFusionModelRegistry } from "./auth-storage.js"; diff --git a/packages/engine/src/reports-health.ts b/packages/engine/src/reports-health.ts new file mode 100644 index 0000000000..3fa65607b4 --- /dev/null +++ b/packages/engine/src/reports-health.ts @@ -0,0 +1,70 @@ +export type ReportHealthBucket = + | "healthy" + | "stale-assignment" + | "stale" + | "stuck" + | "paused" + | "operator-actionable"; + +export interface ReportHealthInput { + state: string | undefined; + pauseReason: string | undefined; + heartbeatAgeMs: number; + heartbeatTimeoutMs: number; + staleThresholdMs: number; + staleParkedAssignment: boolean; +} + +export interface ReportHealthClassification { + bucket: ReportHealthBucket; + cellText: string; +} + +const OPERATOR_ACTIONABLE_PAUSE_REASONS = new Set([ + "error-unrecoverable", + "error-retry-exhausted", + "heartbeat-model-unavailable", +]); + +/** + * FNXC:ReportsHealth 2026-07-24-12:00: + * FN-8569 requires pause markers to outrank the state column because independent + * state and marker writes can persist a live-looking state alongside an + * error-unrecoverable park marker. This classifier must be truthful for every + * persisted shape, including desyncs that best-effort store cleanup cannot prevent. + * `lastError` is deliberately excluded: it is diagnostic history, not a health + * input. Keep this classifier engine-side so @fusion/core never depends on engine. + */ +export function classifyReportHealth(input: ReportHealthInput): ReportHealthClassification { + const pauseReason = input.pauseReason?.trim(); + if (pauseReason) { + if (OPERATOR_ACTIONABLE_PAUSE_REASONS.has(pauseReason)) { + return { + bucket: "operator-actionable", + cellText: `**needs operator repair** (${pauseReason})`, + }; + } + return { + bucket: "paused", + cellText: `paused (${pauseReason})`, + }; + } + + if (input.state === "error") { + return { bucket: "operator-actionable", cellText: "**needs operator repair**" }; + } + if (input.staleParkedAssignment) { + return { bucket: "stale-assignment", cellText: "**stale** assignment" }; + } + if (input.state === "running" && input.heartbeatAgeMs > input.heartbeatTimeoutMs * 2) { + return { bucket: "stuck", cellText: "**stuck**" }; + } + if ((input.state === "active" || input.state === "idle") && input.heartbeatAgeMs > input.staleThresholdMs) { + return { bucket: "stale", cellText: "**stale**" }; + } + if (input.state === "paused") { + return { bucket: "paused", cellText: "paused" }; + } + + return { bucket: "healthy", cellText: "healthy" }; +}