From 2ff8e2e13ed4a992765fa757ce88cb364207ce09 Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 9 Jul 2026 13:32:21 -0700 Subject: [PATCH] FN-7743: detect and recover stalled in-progress executor tasks in overseer Fix the planner overseer's executor-stage stall detection so hung in-progress tasks get unstuck instead of being reported as progressing forever. - Add configurable stuck-detection: the executor-stage overseer observation now emits signal: "stuck" once an in-progress task has been inactive past a new plannerOverseerExecutorStuckAfterMs threshold, feeding the existing decidePlannerRecovery -> bounded inject_guidance recovery path. - Register the new plannerOverseerExecutorStuckAfterMs setting in builtin-workflow-settings.ts and export it via core index.ts/index.gate.ts. - Preserve human-control withholds (user-paused / approval-blocked / autoMerge-off) taking precedence over stuck detection. - Add/extend tests covering planner-overseer, planner-recovery-controller, planner-recovery, and builtin-workflow-settings-triage. - Document the new setting in docs/architecture.md and docs/settings-reference.md. - Add changeset fn-7743-overseer-executor-stall.md (patch). Files changed: .changeset/fn-7743-overseer-executor-stall.md | 7 + docs/architecture.md | 31 +++++ docs/settings-reference.md | 3 +- .../builtin-workflow-settings-triage.test.ts | 22 ++++ .../core/src/__tests__/planner-recovery.test.ts | 12 ++ packages/core/src/builtin-workflow-settings.ts | 26 ++++ packages/core/src/index.gate.ts | 1 + packages/core/src/index.ts | 1 + .../engine/src/__tests__/planner-overseer.test.ts | 146 +++++++++++++++++++++ .../__tests__/planner-recovery-controller.test.ts | 52 +++++++- packages/engine/src/planner-overseer.ts | 88 ++++++++++++- packages/engine/src/project-engine.ts | 11 +- 12 files changed, 391 insertions(+), 9 deletions(-) Fusion-Task-Id: FN-7743 Fusion-Task-Lineage: 791852a2-5b77-48de-955a-1b3916616dfa Co-authored-by: Fusion (runfusion.ai) --- .changeset/fn-7743-overseer-executor-stall.md | 7 + docs/architecture.md | 31 ++++ docs/settings-reference.md | 3 +- .../builtin-workflow-settings-triage.test.ts | 22 +++ .../src/__tests__/planner-recovery.test.ts | 12 ++ .../core/src/builtin-workflow-settings.ts | 26 ++++ packages/core/src/index.gate.ts | 1 + packages/core/src/index.ts | 1 + .../src/__tests__/planner-overseer.test.ts | 146 ++++++++++++++++++ .../planner-recovery-controller.test.ts | 52 ++++++- packages/engine/src/planner-overseer.ts | 88 ++++++++++- packages/engine/src/project-engine.ts | 11 +- 12 files changed, 391 insertions(+), 9 deletions(-) create mode 100644 .changeset/fn-7743-overseer-executor-stall.md diff --git a/.changeset/fn-7743-overseer-executor-stall.md b/.changeset/fn-7743-overseer-executor-stall.md new file mode 100644 index 0000000000..50c5bda6d4 --- /dev/null +++ b/.changeset/fn-7743-overseer-executor-stall.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: The planner overseer now detects and recovers stalled in-progress tasks instead of leaving hung executors stuck. +category: fix +dev: FN-7743 — the executor-stage overseer observation now emits `signal: "stuck"` once an in-progress task has been inactive past a configurable threshold (`plannerOverseerExecutorStuckAfterMs`), feeding the existing `decidePlannerRecovery` → bounded `inject_guidance` path. Previously a non-paused in-progress task was always reported `progressing`, so a hung executor was never recovered. Human-control withholds (user-paused / approval-blocked / autoMerge-off) still take precedence. diff --git a/docs/architecture.md b/docs/architecture.md index d723e97769..ae318494d5 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -1386,6 +1386,37 @@ call happens here, and it emits no run-audit events or dashboard UI. Steering/re gates, human-control safeguards, and dashboard/UI/run-audit surfaces are deferred to FN-7512 through FN-7520; this module is the seam those subtasks read observations from. +#### Executor-stage stall detection (FN-7743) + +/* +FNXC:PlannerOversight 2026-07-09-00:00: +FN-7743 requirement: an ordinary in-progress task (FN-7732) sat stuck for hours with no recovery action +because `deriveSignalAndSources`'s `case "executor"` had NO staleness check — a non-paused in-progress +task always reported `signal: "progressing"` regardless of how long it had been idle, so +`decidePlannerRecovery` always returned `action: "none"`. +*/ + +The executor stage now detects a stalled/idle non-paused `in-progress` task and reports `signal: "stuck"` +instead of always `"progressing"`. `PlannerOverseerMonitor#observeTask` accepts an optional +`{ now?: () => number; executorStuckAfterMs?: number }` (both default: `Date.now` and the declared +`plannerOverseerExecutorStuckAfterMs` workflow-setting default) and threads them into the still-pure, +still-never-reads-settings `deriveSignalAndSources`. The staleness input is `task.columnMovedAt ?? +task.updatedAt` (mirroring the existing "age is measured from columnMovedAt when present, otherwise +updatedAt" convention already used by `stalePausedReviewThresholdMs`/`stalePausedTodoThresholdMs`) — the +best available store-backed proxy for "last execution activity" at this poll seam, since the live +in-session `StuckTaskDetector` heartbeat state (`packages/engine/src/stuck-task-detector.ts`) is +in-memory-only and unavailable here. A missing or unparseable timestamp degrades to `"progressing"` +(fail-safe — never fabricate a stall). The `stuck` reason buckets inactivity to whole hours so the FN-7577 +`stage|signal|reason` feed dedup stays effective (it must never embed an ever-changing millisecond value). + +`ProjectEngine.pollPlannerOverseer` resolves `plannerOverseerExecutorStuckAfterMs` from the task's +already-fetched effective workflow settings (`resolveExecutorStuckAfterMs`, fail-safe default 2 hours) once +per task per poll cycle — the same `workflowEffective` fetch already used for `plannerOversightLevel` — and +passes it into `observeTask`. `decidePlannerRecovery` already mapped executor `stuck` → `inject_guidance` +(no change needed there); the human-control withhold guard in `evaluateOverseerHumanControl` still runs +BEFORE any recovery decision, so a stuck-but-user-paused/approval-blocked/autoMerge-off task is still fully +withheld. + ### Planner overseer bounded autonomous recovery (FN-7512) /* diff --git a/docs/settings-reference.md b/docs/settings-reference.md index 43482d06aa..1c4c88f0bc 100644 --- a/docs/settings-reference.md +++ b/docs/settings-reference.md @@ -327,7 +327,7 @@ These groups moved out of project settings and into workflow settings (built-in |---|---| | **Step execution** | `workflowStepTimeoutMs`, `runStepsInNewSessions`, `maxParallelSteps`, `workflowStepScopeEnforcement`, `strictScopeEnforcement`, `verificationFixRetries`, `maxPostReviewFixes`, `buildRetryCount` | | **Review / approval** | Workflow values: `requirePrApproval`, `requirePlanApproval`, `reviewHandoffPolicy`, `maxReviewerContextRetries`, `maxReviewerFallbackRetries`, `planReviewMaxRevisions`, `codeReviewMaxRevisions`; project override: `planApprovalMode` | -| **Planner oversight** | `plannerOversightLevel` (workflow-native; values: `off`, `observe`, `steer`, `autonomous`); `plannerOversightNotificationLevel` (workflow-native; values: `silent`, `errors`, `important`, `all`) | +| **Planner oversight** | `plannerOversightLevel` (workflow-native; values: `off`, `observe`, `steer`, `autonomous`); `plannerOversightNotificationLevel` (workflow-native; values: `silent`, `errors`, `important`, `all`); `plannerOverseerExecutorStuckAfterMs` (workflow-native; number, default `7200000` = 2h) | | **Per-phase model lanes** | `executionProvider`/`executionModelId`, `planningProvider`/`planningModelId` (+ fallbacks), `validatorProvider`/`validatorModelId` (+ fallbacks) | ### Workflow-native triage policy settings @@ -363,6 +363,7 @@ The built-in workflows also declare triage/spec policy settings that were **not* | `codeReviewMaxRevisions` | unset | Workflow-native Code Review remediation cap. Unset/empty means unbounded automatic code-fix passes; a non-negative integer caps attempts; `0` disables automatic Code Review remediation. | | `plannerOversightLevel` | `autonomous` | Workflow-native planner oversight mode. `off` disables oversight; `observe` watches only; `steer` injects guidance or suggests revisions; `autonomous` enables bounded retry and targeted-fix recovery — but merge/PR progression and any destructive or external-service side effect ALWAYS require an explicit, recorded human confirmation before they run, even at `autonomous` (FN-7513's confirmation gate; see `docs/architecture.md` → "Planner overseer confirmation gate"). Tasks may set a nullable `Task.plannerOversightLevel` override (same four values) that wins over this workflow value when present; `null`/unset means "inherit the workflow value". `resolveEffectivePlannerOversightLevel` in `@fusion/core` computes the effective level (task override → workflow effective → `autonomous`). The per-task override is exposed in the dashboard as a "Planner oversight" selector (Inherit from workflow / Off / Observe / Steer / Autonomous recovery) in both the New Task dialog and Task Detail edit form, threaded through `createTask`/`updateTask` (FN-7515); the project/global default is set via the **Workflow Editor → Values** tab on the default workflow's `plannerOversightLevel` value, not in Project Settings. FN-7517 additionally exposes a quick inline oversight-level select in the Task Detail modal's meta-controls cluster (same `updateTask` override plumbing, no parallel path) plus manual nudge/stop-oversight/explain-current-action controls that call the overseer runtime directly — see `docs/dashboard-guide.md`. Engine read-site behavior beyond the FN-7513 confirmation gate remains follow-up work (FN-7510+). | | `plannerOversightNotificationLevel` | `important` | Workflow-native planner-overseer notification verbosity (FN-7518). `silent` suppresses overseer notifications; `errors` notifies only on failures/escalations; `important` (the default) notifies on interventions/recovery actions and errors; `all` notifies on every observation. Resolves through the generic `resolveEffectiveSettings` default path with no special-casing, alongside `plannerOversightLevel`. This is a declaration-only setting: the notification-emission gating that reads it lands downstream in FN-7519 (intervention timeline) and FN-7520 (run-audit/activity events). | +| `plannerOverseerExecutorStuckAfterMs` | `7200000` (2h) | Workflow-native executor-stage stall threshold (FN-7743). Milliseconds of executor-stage inactivity — no execution activity since the task's last column move/update (`columnMovedAt ?? updatedAt`) — before a non-paused `in-progress` task is reported `signal: "stuck"` instead of `"progressing"`, feeding the existing `decidePlannerRecovery` → bounded `inject_guidance` recovery path at the `autonomous` oversight level (no effect at `off`/`observe`/`steer`). Fixes the class of bug where a genuinely hung/idle executor (dead session, silent agent) was indistinguishable from a healthy one and was never nudged, retried, or escalated. A missing/malformed activity timestamp degrades to `"progressing"` (fail-safe — never fabricates a stall), and a user-paused/approval-blocked/`autoMerge:false` task is still fully withheld from any autonomous action regardless of this threshold. Resolves through the generic `resolveEffectiveSettings` default path alongside `plannerOversightLevel`. See `docs/architecture.md` → "Executor-stage stall detection (FN-7743)". | When `triageProactiveSubtaskSplittingEnabled` is `true` (the default), triage may proactively replace a large task with 2-5 child tasks when the size, step-count, package breadth, file-scope, or remediation-batch signals justify the coordination overhead. When it is `false`, those automatic oversized-task signals are advisory only for writing a realistic single-task spec; triage must not split solely because the task is large. The per-task `breakIntoSubtasks: true` flag is separate and remains mandatory: if a user explicitly asks for subtask breakdown, triage still evaluates and creates child tasks when the work is meaningfully decomposable. diff --git a/packages/core/src/__tests__/builtin-workflow-settings-triage.test.ts b/packages/core/src/__tests__/builtin-workflow-settings-triage.test.ts index f9315d98da..378c26daa1 100644 --- a/packages/core/src/__tests__/builtin-workflow-settings-triage.test.ts +++ b/packages/core/src/__tests__/builtin-workflow-settings-triage.test.ts @@ -89,6 +89,7 @@ describe("workflow-native built-in workflow settings", () => { expect(BUILTIN_OVERSIGHT_SETTINGS.map((setting) => setting.id)).toEqual([ "plannerOversightLevel", "plannerOversightNotificationLevel", + "plannerOverseerExecutorStuckAfterMs", ]); const oversight = BUILTIN_OVERSIGHT_SETTINGS[0]; expect(oversight).toMatchObject({ @@ -144,6 +145,27 @@ describe("workflow-native built-in workflow settings", () => { movedKeyIds.has("plannerOversightNotificationLevel"), "plannerOversightNotificationLevel should not be in MOVED_SETTINGS_KEYS", ).toBe(false); + + // FN-7743: executor-stall recovery threshold, declared alongside the other + // workflow-native oversight settings. + const executorStuckAfterMs = BUILTIN_OVERSIGHT_SETTINGS[2]; + expect(executorStuckAfterMs).toMatchObject({ + id: "plannerOverseerExecutorStuckAfterMs", + type: "number", + default: 2 * 60 * 60 * 1000, + }); + expect( + fullIds.has("plannerOverseerExecutorStuckAfterMs"), + "plannerOverseerExecutorStuckAfterMs should be in the full built-in catalog", + ).toBe(true); + expect( + movedIds.has("plannerOverseerExecutorStuckAfterMs"), + "plannerOverseerExecutorStuckAfterMs should not be in the moved-key catalog", + ).toBe(false); + expect( + movedKeyIds.has("plannerOverseerExecutorStuckAfterMs"), + "plannerOverseerExecutorStuckAfterMs should not be in MOVED_SETTINGS_KEYS", + ).toBe(false); }); it("renders placeholders from resolved settings and rejects dangling tokens", () => { diff --git a/packages/core/src/__tests__/planner-recovery.test.ts b/packages/core/src/__tests__/planner-recovery.test.ts index 5e351e9f1d..607cba45d5 100644 --- a/packages/core/src/__tests__/planner-recovery.test.ts +++ b/packages/core/src/__tests__/planner-recovery.test.ts @@ -212,4 +212,16 @@ describe("decidePlannerRecovery", () => { expect(decision.exhausted).toBe(true); expect(decision.attemptLimit).toBe(1); }); + + // FN-7743 invariant lock: a stalled non-paused in-progress task (the FN-7732 + // symptom) is surfaced by the overseer as `stage: "executor", signal: "stuck"`. + // This asserts the downstream mapping this fix depends on — executor `stuck` + // → `inject_guidance`, not `none` — already holds and stays locked, so a + // future regression here is caught even though FN-7743 itself only changes + // the observation INPUT, never this mapping. + it("FN-7743: maps an executor stuck signal to inject_guidance (the hung-executor recovery path)", () => { + const decision = decidePlannerRecovery({ snapshot: observation({ stage: "executor", signal: "stuck" }) }); + expect(decision.action).toBe("inject_guidance"); + expect(decision.requiresConfirmation).toBe(false); + }); }); diff --git a/packages/core/src/builtin-workflow-settings.ts b/packages/core/src/builtin-workflow-settings.ts index 44cf2f5659..6b0a982cce 100644 --- a/packages/core/src/builtin-workflow-settings.ts +++ b/packages/core/src/builtin-workflow-settings.ts @@ -439,6 +439,24 @@ export const BUILTIN_REVIEW_REVISION_SETTINGS: WorkflowSettingDefinition[] = [ * FNXC:PlannerOversight 2026-07-04-12:00: * FN-7518 adds `plannerOversightNotificationLevel`, a sibling workflow-native enum letting operators configure how noisy planner-overseer notifications are: Silent suppresses all, Errors only notifies on failures/escalations, Important (the default) notifies on interventions/recovery actions plus errors, and All notifies on every observation. Default is `important` (not `all`) to avoid noisy-by-default behavior. This setting stays workflow-native (out of project/global settings schemas and `MOVED_SETTINGS_KEYS`) and resolves through the generic `resolveEffectiveSettings` default path with no special-casing. This task only declares the setting — the emission gating that reads it lands downstream in FN-7519 (intervention timeline) and FN-7520 (run-audit/activity events). */ +/** + * FNXC:PlannerOversight 2026-07-09-00:00: + * FN-7743 requirement: an ordinary in-progress task (FN-7732) sat stuck for hours + * with no recovery action because the executor-stage overseer observation had no + * staleness detection — it always reported `signal: "progressing"` regardless of + * inactivity. `plannerOverseerExecutorStuckAfterMs` is the configurable inactivity + * threshold: once a non-paused in-progress task's last execution activity + * (`columnMovedAt ?? updatedAt`) is older than this, the executor stage reports + * `signal: "stuck"` instead, which already flows through `decidePlannerRecovery` + * into bounded `inject_guidance` recovery. Default 2 hours (7,200,000ms): long + * enough that a healthy, actively-working step (the vast majority of which finish + * well under 2h) is never nagged, short enough to actually recover a task that has + * gone dark for "hours" (the FN-7732 symptom) — mirrors the existing 2-hour + * convention `metaTaskStallAutoCloseMs` already uses for a comparable stall + * judgment call elsewhere in this codebase. + */ +export const DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS = 2 * 60 * 60 * 1000; + export const BUILTIN_OVERSIGHT_SETTINGS: WorkflowSettingDefinition[] = [ { id: "plannerOversightLevel", @@ -468,6 +486,14 @@ export const BUILTIN_OVERSIGHT_SETTINGS: WorkflowSettingDefinition[] = [ description: "Planner overseer notification verbosity: Silent suppresses overseer notifications; Errors only notifies on failures/escalations; Important notifies on interventions/recovery actions and errors; All notifies on every observation. Notification-emission gating that reads this value is follow-up work (FN-7519/FN-7520).", }, + { + id: "plannerOverseerExecutorStuckAfterMs", + name: "Executor stall threshold (ms)", + type: "number", + default: DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS, + description: + "Milliseconds of executor-stage inactivity (no progress since the task's last column move/update) before the planner overseer reports the in-progress task as stuck, triggering bounded autonomous recovery (Autonomous level only). Default 7200000 (2 hours). Set higher to avoid nagging long-running steps; set lower to recover hung executors faster.", + }, ]; export const BUILTIN_WORKFLOW_SETTINGS: WorkflowSettingDefinition[] = [ diff --git a/packages/core/src/index.gate.ts b/packages/core/src/index.gate.ts index 00459b006f..528356576a 100644 --- a/packages/core/src/index.gate.ts +++ b/packages/core/src/index.gate.ts @@ -238,6 +238,7 @@ export { BUILTIN_MOVED_WORKFLOW_SETTINGS, BUILTIN_TRIAGE_POLICY_SETTINGS, BUILTIN_OVERSIGHT_SETTINGS, + DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS, renderTriagePolicyPlaceholders, } from "./builtin-workflow-settings.js"; export { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a6e4125c6e..6e2362d35d 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -207,6 +207,7 @@ export { BUILTIN_MOVED_WORKFLOW_SETTINGS, BUILTIN_TRIAGE_POLICY_SETTINGS, BUILTIN_OVERSIGHT_SETTINGS, + DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS, renderTriagePolicyPlaceholders, } from "./builtin-workflow-settings.js"; export { diff --git a/packages/engine/src/__tests__/planner-overseer.test.ts b/packages/engine/src/__tests__/planner-overseer.test.ts index af4a8ffe54..db5d339d89 100644 --- a/packages/engine/src/__tests__/planner-overseer.test.ts +++ b/packages/engine/src/__tests__/planner-overseer.test.ts @@ -318,3 +318,149 @@ describe("PlannerOverseerMonitor.observeTask", () => { await expect(monitor.observeTask(undefined as unknown as OverseerTaskRef, "autonomous")).resolves.toBeNull(); }); }); + +// FN-7743: executor-stage stall detection. FN-7732 was a non-paused in-progress +// task that sat stuck for hours while the overseer always reported +// `signal: "progressing"` because there was no staleness check. These tests lock +// the invariant across the enumerated data states: stale (stuck), recent +// (progressing, unchanged), paused (blocked, unchanged), and missing/malformed +// timestamp (fail-safe progressing). +describe("PlannerOverseerMonitor.observeTask — FN-7743 executor stall detection", () => { + const THRESHOLD_MS = 2 * 60 * 60 * 1000; // 2h, mirrors the declared setting default + const NOW = Date.UTC(2026, 6, 9, 12, 0, 0); + + function isoMsAgo(ms: number): string { + return new Date(NOW - ms).toISOString(); + } + + it("reports stuck for a non-paused in-progress task whose last activity is older than the threshold", async () => { + const monitor = new PlannerOverseerMonitor(); + const task = taskFixture({ + column: "in-progress", + updatedAt: isoMsAgo(THRESHOLD_MS + 60 * 60 * 1000), // 3h idle + }); + + const observation = await monitor.observeTask(task, "autonomous", { + now: () => NOW, + executorStuckAfterMs: THRESHOLD_MS, + }); + + expect(observation?.signal).toBe("stuck"); + expect(observation?.stage).toBe("executor"); + expect(observation?.reason).toMatch(/inactive for over \d+h/); + }); + + it("prefers columnMovedAt over updatedAt when both are present", async () => { + const monitor = new PlannerOverseerMonitor(); + const task = taskFixture({ + column: "in-progress", + // updatedAt looks fresh, but columnMovedAt (the more specific signal) is stale. + updatedAt: isoMsAgo(1000), + columnMovedAt: isoMsAgo(THRESHOLD_MS + 60 * 60 * 1000), + }); + + const observation = await monitor.observeTask(task, "autonomous", { + now: () => NOW, + executorStuckAfterMs: THRESHOLD_MS, + }); + + expect(observation?.signal).toBe("stuck"); + }); + + it("remains progressing for a non-paused in-progress task with recent activity", async () => { + const monitor = new PlannerOverseerMonitor(); + const task = taskFixture({ + column: "in-progress", + updatedAt: isoMsAgo(5 * 60 * 1000), // 5 minutes ago — well under threshold + }); + + const observation = await monitor.observeTask(task, "autonomous", { + now: () => NOW, + executorStuckAfterMs: THRESHOLD_MS, + }); + + expect(observation?.signal).toBe("progressing"); + }); + + it("remains progressing at exactly the threshold boundary minus one ms, and flips to stuck at/after the boundary", async () => { + const monitor = new PlannerOverseerMonitor(); + const justUnder = taskFixture({ column: "in-progress", updatedAt: isoMsAgo(THRESHOLD_MS - 1) }); + const atThreshold = taskFixture({ column: "in-progress", updatedAt: isoMsAgo(THRESHOLD_MS) }); + + const obsUnder = await monitor.observeTask(justUnder, "autonomous", { now: () => NOW, executorStuckAfterMs: THRESHOLD_MS }); + expect(obsUnder?.signal).toBe("progressing"); + + const obsAt = await monitor.observeTask(atThreshold, "autonomous", { now: () => NOW, executorStuckAfterMs: THRESHOLD_MS }); + expect(obsAt?.signal).toBe("stuck"); + }); + + it("still reports blocked (unchanged) for a paused in-progress task even with a stale timestamp", async () => { + const monitor = new PlannerOverseerMonitor(); + const task = taskFixture({ + column: "in-progress", + paused: true, + pausedReason: "some-engine-park-reason", + updatedAt: isoMsAgo(THRESHOLD_MS + 60 * 60 * 1000), + }); + + const observation = await monitor.observeTask(task, "autonomous", { + now: () => NOW, + executorStuckAfterMs: THRESHOLD_MS, + }); + + expect(observation?.signal).toBe("blocked"); + }); + + it("fails safe to progressing when both updatedAt and columnMovedAt are missing", async () => { + const monitor = new PlannerOverseerMonitor(); + const task = taskFixture({ column: "in-progress", updatedAt: undefined, columnMovedAt: undefined }); + + const observation = await monitor.observeTask(task, "autonomous", { + now: () => NOW, + executorStuckAfterMs: THRESHOLD_MS, + }); + + expect(observation?.signal).toBe("progressing"); + }); + + it("fails safe to progressing when the timestamp is malformed/unparseable", async () => { + const monitor = new PlannerOverseerMonitor(); + const task = taskFixture({ column: "in-progress", updatedAt: "not-a-real-date" }); + + const observation = await monitor.observeTask(task, "autonomous", { + now: () => NOW, + executorStuckAfterMs: THRESHOLD_MS, + }); + + expect(observation?.signal).toBe("progressing"); + }); + + it("defaults executorStuckAfterMs to the declared 2h default when options are omitted", async () => { + const monitor = new PlannerOverseerMonitor(); + const staleTask = taskFixture({ column: "in-progress", updatedAt: isoMsAgo(3 * 60 * 60 * 1000) }); + const freshTask = taskFixture({ column: "in-progress", updatedAt: isoMsAgo(60 * 1000) }); + + const staleObs = await monitor.observeTask(staleTask, "autonomous", { now: () => NOW }); + const freshObs = await monitor.observeTask(freshTask, "autonomous", { now: () => NOW }); + + expect(staleObs?.signal).toBe("stuck"); + expect(freshObs?.signal).toBe("progressing"); + }); + + // FN-7577: the dedup key must stay stable within an hour bucket so an ever- + // changing millisecond-precise duration in the reason does not defeat dedup. + it("keeps the stuck reason stable within the same inactivity-hour bucket for feed dedup", async () => { + const store = { logEntry: vi.fn().mockResolvedValue(undefined) }; + const monitor = new PlannerOverseerMonitor({ store }); + const task = taskFixture({ column: "in-progress", updatedAt: isoMsAgo(THRESHOLD_MS + 5 * 60 * 1000) }); + + // Two polls a few seconds apart within the same inactivity-hour bucket. + await monitor.observeTask(task, "observe", { now: () => NOW, executorStuckAfterMs: THRESHOLD_MS }); + await monitor.observeTask(task, "observe", { now: () => NOW + 5000, executorStuckAfterMs: THRESHOLD_MS }); + expect(store.logEntry).toHaveBeenCalledTimes(1); + + // An hour-boundary crossing is a real state change — re-logs once. + await monitor.observeTask(task, "observe", { now: () => NOW + 60 * 60 * 1000, executorStuckAfterMs: THRESHOLD_MS }); + expect(store.logEntry).toHaveBeenCalledTimes(2); + }); +}); diff --git a/packages/engine/src/__tests__/planner-recovery-controller.test.ts b/packages/engine/src/__tests__/planner-recovery-controller.test.ts index e336c9998d..b3699c6ac2 100644 --- a/packages/engine/src/__tests__/planner-recovery-controller.test.ts +++ b/packages/engine/src/__tests__/planner-recovery-controller.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from "vitest"; -import { PLANNER_RECOVERY_MAX_ATTEMPTS } from "@fusion/core"; +import { AWAITING_APPROVAL_PAUSE_REASON, PLANNER_RECOVERY_MAX_ATTEMPTS } from "@fusion/core"; import type { Task } from "@fusion/core"; import { PlannerRecoveryController, type PlannerRecoveryHandlers } from "../planner-recovery-controller.js"; import type { OverseerStageObservation, OverseerWatchedStage } from "../planner-overseer.js"; @@ -189,4 +189,54 @@ describe("PlannerRecoveryController.tick", () => { expect(decision?.action).toBe("retry_step"); expect(retryStep).toHaveBeenCalledTimes(1); }); + + // FN-7743: a `stuck` executor observation (a genuinely hung/idle in-progress + // task, the FN-7732 symptom) must reach bounded recovery exactly like a + // `failed`/`blocked` one — `inject_guidance`, dispatched once per tick, with + // the attempt budget incrementing — and a withheld (user-paused) task must + // still dispatch nothing even though its stage is `stuck`. + describe("FN-7743 stuck executor observation", () => { + it("dispatches inject_guidance exactly once and increments the attempt budget for a stuck executor stage", async () => { + const injectGuidance = vi.fn().mockResolvedValue(undefined); + const controller = makeController(observation({ stage: "executor", signal: "stuck", reason: "Executor stage inactive for over 3h with no execution activity" }), { + injectGuidance, + }); + + const decision = await controller.tick(task()); + expect(decision?.action).toBe("inject_guidance"); + expect(injectGuidance).toHaveBeenCalledTimes(1); + expect(controller.getAttemptCount("FN-1", "executor")).toBe(1); + }); + + it("dispatches nothing for a stuck executor stage when the task is user-paused (human-control withhold wins)", async () => { + const injectGuidance = vi.fn().mockResolvedValue(undefined); + const controller = makeController(observation({ stage: "executor", signal: "stuck" }), { injectGuidance }); + + const decision = await controller.tick(task({ userPaused: true })); + expect(decision).toBeNull(); + expect(injectGuidance).not.toHaveBeenCalled(); + expect(controller.getAttemptCount("FN-1", "executor")).toBe(0); + }); + + it("dispatches nothing for a stuck executor stage when the task is approval-blocked (human-control withhold wins)", async () => { + const injectGuidance = vi.fn().mockResolvedValue(undefined); + const controller = makeController(observation({ stage: "executor", signal: "stuck" }), { injectGuidance }); + + const decision = await controller.tick(task({ paused: true, pausedReason: AWAITING_APPROVAL_PAUSE_REASON })); + expect(decision).toBeNull(); + expect(injectGuidance).not.toHaveBeenCalled(); + }); + + it("is inert for a stuck executor stage when effectiveLevel is off/observe/steer (no autonomous dispatch)", async () => { + for (const level of ["off", "observe", "steer"] as const) { + const injectGuidance = vi.fn().mockResolvedValue(undefined); + const controller = makeController(observation({ stage: "executor", signal: "stuck", oversightLevel: level }), { + injectGuidance, + }); + const decision = await controller.tick(task()); + expect(decision?.action, `level=${level}`).toBe("none"); + expect(injectGuidance).not.toHaveBeenCalled(); + } + }); + }); }); diff --git a/packages/engine/src/planner-overseer.ts b/packages/engine/src/planner-overseer.ts index 7f8ebf3128..c2063e3d65 100644 --- a/packages/engine/src/planner-overseer.ts +++ b/packages/engine/src/planner-overseer.ts @@ -14,7 +14,7 @@ * the seam every later planner-oversight subtask reads from. */ -import type { PlannerOversightLevel, PrInfo, Task } from "@fusion/core"; +import { DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS, type PlannerOversightLevel, type PrInfo, type Task } from "@fusion/core"; /** Alias for the `Task.reviewState` shape without requiring a separate core export. */ type OverseerTaskReviewState = NonNullable; @@ -53,7 +53,15 @@ export interface OverseerStageObservation { * full `Task` interface. */ export type OverseerTaskRef = Pick< Task, - "id" | "column" | "prInfo" | "reviewState" | "paused" | "pausedReason" | "workflowTransitionNotification" + | "id" + | "column" + | "prInfo" + | "reviewState" + | "paused" + | "pausedReason" + | "workflowTransitionNotification" + | "updatedAt" + | "columnMovedAt" >; /** @@ -127,10 +135,40 @@ export function resolveWatchedStage(task: Partial | null | unde } } +/** + * FNXC:PlannerOversight 2026-07-09-00:00: + * FN-7743 stall-detection inputs for `deriveSignalAndSources`'s `executor` branch. + * Passed in already-resolved (never read from settings inside this pure function, + * per the FN-7743 "keep derivation pure and testable" requirement): `now` is an + * injectable clock (defaults to `Date.now` at the `observeTask` call site) and + * `executorStuckAfterMs` is the resolved `plannerOverseerExecutorStuckAfterMs` + * workflow setting value (or its declaration default). + */ +export interface ExecutorStallSignalInput { + now: () => number; + executorStuckAfterMs: number; +} + +/** + * FNXC:PlannerOversight 2026-07-09-00:00: + * Validates a raw `plannerOverseerExecutorStuckAfterMs` workflow-setting value + * (which may be missing/malformed/legacy-orphaned per `resolveEffectiveSettings`'s + * never-throw contract) into a safe threshold, degrading to + * `DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS` for anything that is not a + * finite positive number. Pure, never throws. + */ +export function resolveExecutorStuckAfterMs(raw: unknown): number { + if (typeof raw === "number" && Number.isFinite(raw) && raw > 0) { + return raw; + } + return DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS; +} + function deriveSignalAndSources( taskId: string, stage: OverseerWatchedStage, task: Partial, + stallInput: ExecutorStallSignalInput, ): { signal: OverseerObservationSignal; reason: string; sources: OverseerSourceLink[] } { switch (stage) { case "executor": { @@ -141,6 +179,32 @@ function deriveSignalAndSources( sources: [{ kind: "agent-log", ref: taskId }], }; } + + // FNXC:PlannerOversight 2026-07-09-00:00: + // FN-7743: a non-paused in-progress task whose executor session has gone + // silent (dead/hung agent, no commits/heartbeat) was previously ALWAYS + // reported "progressing" forever (FN-7732 symptom) since nothing here + // checked staleness. Use `columnMovedAt ?? updatedAt` as the best + // available store-backed "last execution activity" proxy at this poll + // seam (the live in-session `StuckTaskDetector` heartbeat state is + // in-memory-only and not available here). A missing/malformed timestamp + // degrades to "progressing" — never fabricate a stall. The reason is + // bucketed to whole hours so the FN-7577 `stage|signal|reason` feed dedup + // stays effective (it must not embed an ever-changing millisecond value). + const activityTimestamp = task.columnMovedAt ?? task.updatedAt; + const activityAtMs = activityTimestamp ? Date.parse(activityTimestamp) : NaN; + if (Number.isFinite(activityAtMs) && stallInput.executorStuckAfterMs > 0) { + const inactiveMs = stallInput.now() - activityAtMs; + if (inactiveMs >= stallInput.executorStuckAfterMs) { + const inactiveHours = Math.max(1, Math.floor(inactiveMs / 3_600_000)); + return { + signal: "stuck", + reason: `Executor stage inactive for over ${inactiveHours}h with no execution activity`, + sources: [{ kind: "agent-log", ref: taskId }], + }; + } + } + return { signal: "progressing", reason: "Task is actively executing in-progress work", @@ -280,8 +344,20 @@ export class PlannerOverseerMonitor { * Observe a task's current watched stage and record a gated observation. * Returns `null` when the level is `"off"` or when no stage is currently * monitorable. Never throws. + * + * FNXC:PlannerOversight 2026-07-09-00:00: + * FN-7743: `options.now`/`options.executorStuckAfterMs` thread the clock and + * the resolved executor-stall threshold into the pure `deriveSignalAndSources` + * derivation. Both default (`Date.now`, `DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS`) + * so existing callers keep working unchanged; the poll seam + * (`project-engine.ts#pollPlannerOverseer`) resolves the real workflow-setting + * value once per cycle and passes it in explicitly. */ - async observeTask(task: OverseerTaskRef, level: PlannerOversightLevel): Promise { + async observeTask( + task: OverseerTaskRef, + level: PlannerOversightLevel, + options?: { now?: () => number; executorStuckAfterMs?: number }, + ): Promise { try { if (level === "off") { return null; @@ -292,13 +368,15 @@ export class PlannerOverseerMonitor { return null; } - const { signal, reason, sources } = deriveSignalAndSources(task.id, stage, task); + const now = options?.now ?? Date.now; + const executorStuckAfterMs = options?.executorStuckAfterMs ?? DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS; + const { signal, reason, sources } = deriveSignalAndSources(task.id, stage, task, { now, executorStuckAfterMs }); const observation: OverseerStageObservation = { taskId: task.id, stage, signal, oversightLevel: level, - observedAt: Date.now(), + observedAt: now(), reason, sources, }; diff --git a/packages/engine/src/project-engine.ts b/packages/engine/src/project-engine.ts index 1f95df1ec6..ecab1a4007 100644 --- a/packages/engine/src/project-engine.ts +++ b/packages/engine/src/project-engine.ts @@ -41,7 +41,7 @@ import { InProcessRuntime } from "./runtimes/in-process-runtime.js"; import type { WorktreePool } from "./worktree-pool.js"; import type { ProjectRuntimeConfig } from "./project-runtime.js"; import { PrMonitor } from "./pr-monitor.js"; -import { PlannerOverseerMonitor } from "./planner-overseer.js"; +import { PlannerOverseerMonitor, resolveExecutorStuckAfterMs } from "./planner-overseer.js"; import { PlannerRecoveryController, type PlannerRecoveryHandlers } from "./planner-recovery-controller.js"; import { evaluateOverseerHumanControl } from "./overseer-human-control-policy.js"; import type { PrNodeGithubOps } from "./pr-nodes.js"; @@ -2371,7 +2371,14 @@ export class ProjectEngine { if (level === "off") { continue; } - await overseer.observeTask(task, level); + // FN-7743: resolve the executor-stall threshold from the task's + // effective workflow settings (same `workflowEffective` fetch used + // for `plannerOversightLevel` above — no extra store round-trip) and + // pass it into `observeTask` so a genuinely idle non-paused + // in-progress task reports `signal: "stuck"` instead of always + // `progressing` (the FN-7732 symptom). + const executorStuckAfterMs = resolveExecutorStuckAfterMs(workflowEffective.plannerOverseerExecutorStuckAfterMs); + await overseer.observeTask(task, level, { executorStuckAfterMs }); // FN-7512: one guarded, autonomous-only bounded recovery tick at the // same passive seam FN-7511 uses for observation. Inert for every