fix(FN-7577): stop planner overseer from "recovering" healthy in-progress tasks
decidePlannerRecovery fell through to inject_guidance for any non-failed executor/workflow-gate signal, including the healthy `progressing` signal. Under autonomous oversight this dispatched steering into the live agent of every healthy task — flipping the card badge to "recovering", burning a bounded-attempt slot, and consuming AI usage for no reason. - Only problem signals (`stuck`/`blocked`, plus the existing `failed` path) now trigger autonomous steering; healthy (`progressing`/`complete`) and human-wait (`awaiting-human`) signals return `none`. - PlannerRecoveryController.tick clears stale attempt/last-action records for a (taskId, stage) once its signal is healthy, so a recovered task drops from "recovering" back to "watching" and a later problem gets a fresh budget. - PlannerOverseerMonitor dedupes the activity-feed heartbeat: an unchanged (stage, signal, reason) observation logs once per change, not every tick. Invariant tests added across all signals for both fall-through stages. Fusion-Task-Id: FN-7577 Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/planner-overseer-no-recover-healthy-tasks.md
Normal file
7
.changeset/planner-overseer-no-recover-healthy-tasks.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Planner overseer no longer marks healthy in-progress tasks as "recovering" or steers them.
|
||||
category: fix
|
||||
dev: `decidePlannerRecovery` now returns `none` for healthy (`progressing`/`complete`) and `awaiting-human` executor/workflow-gate signals instead of falling through to `inject_guidance`; only `stuck`/`blocked`/`failed` trigger autonomous steering. Also dedupes the `PlannerOverseerMonitor` activity-feed heartbeat so an unchanged `(stage, signal, reason)` observation is logged once per change, not every poll tick. Fixes the "overseer recovering" badge appearing on every autonomous card and the needless AI-consuming guidance injections (FN-7577).
|
||||
@@ -68,6 +68,31 @@ describe("decidePlannerRecovery", () => {
|
||||
expect(decision.action).toBe("inject_guidance");
|
||||
});
|
||||
|
||||
// FN-7577: a healthy or human-wait signal must NOT trigger autonomous
|
||||
// steering on the executor/workflow-gate fall-through — steering a task that
|
||||
// reports it is progressing flipped every card's badge to "recovering" and
|
||||
// burned AI usage via a needless inject_guidance dispatch. Invariant across
|
||||
// both fall-through stages and both problem/healthy signal classes.
|
||||
it("returns none for healthy/human-wait signals on executor and workflow-gate stages", () => {
|
||||
for (const stage of ["executor", "workflow-gate"] as const) {
|
||||
for (const signal of ["progressing", "complete", "awaiting-human"] as const) {
|
||||
const decision = decidePlannerRecovery({ snapshot: observation({ stage, signal }) });
|
||||
expect(decision.action, `stage=${stage} signal=${signal}`).toBe("none");
|
||||
expect(decision.exhausted, `stage=${stage} signal=${signal}`).toBe(false);
|
||||
expect(decision.requiresConfirmation, `stage=${stage} signal=${signal}`).toBe(false);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("still steers on problem signals (stuck/blocked) for executor and workflow-gate stages", () => {
|
||||
for (const stage of ["executor", "workflow-gate"] as const) {
|
||||
for (const signal of ["stuck", "blocked"] as const) {
|
||||
const decision = decidePlannerRecovery({ snapshot: observation({ stage, signal }) });
|
||||
expect(decision.action, `stage=${stage} signal=${signal}`).toBe("inject_guidance");
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("gates merger and pull-request stages behind confirmation (FN-7513) instead of none", () => {
|
||||
for (const stage of ["merger", "pull-request"] as const) {
|
||||
const decision = decidePlannerRecovery({ snapshot: observation({ stage, signal: "failed" }) });
|
||||
|
||||
@@ -140,8 +140,19 @@ export interface DecidePlannerRecoveryInput {
|
||||
* 5. `executor` / `workflow-gate` stage with `signal === "failed"` →
|
||||
* `"request_targeted_fix"` when a source link carries a specific
|
||||
* fixable error (`failed-check` / `merge-error`), else `"retry_step"`.
|
||||
* 6. Any other `executor` / `workflow-gate` signal (stuck/blocked/
|
||||
* progressing/awaiting-human) → `"inject_guidance"`.
|
||||
* 6. `executor` / `workflow-gate` stage with a PROBLEM signal
|
||||
* (`stuck` / `blocked`) → `"inject_guidance"`.
|
||||
*
|
||||
* FNXC:PlannerOversight 2026-07-05-11:00:
|
||||
* A HEALTHY signal (`progressing` / `complete`) or a human-wait signal
|
||||
* (`awaiting-human`) yields `"none"` — steering a task that reports it is
|
||||
* actively progressing is a misfire: it flips the card's overseer badge to
|
||||
* "recovering", burns a bounded-attempt slot, and (because `inject_guidance`
|
||||
* feeds the LIVE agent) consumes AI usage for no reason. Only a signal that
|
||||
* actually indicates trouble may trigger autonomous steering (user report
|
||||
* FN-7577: "recovering" badge on every healthy in-progress card). Previously
|
||||
* this branch injected guidance on ANY non-`failed` signal, including
|
||||
* `progressing`.
|
||||
*/
|
||||
export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): PlannerRecoveryDecision {
|
||||
const attemptCount = input?.attemptState?.attemptCount ?? 0;
|
||||
@@ -252,7 +263,11 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
|
||||
};
|
||||
}
|
||||
|
||||
{
|
||||
// FNXC:PlannerOversight 2026-07-05-11:00: only PROBLEM signals warrant
|
||||
// autonomous steering. Healthy (`progressing`/`complete`) and human-wait
|
||||
// (`awaiting-human`) signals are a no-op so a fine, actively-progressing
|
||||
// task is never "recovered" (FN-7577).
|
||||
if (snapshot.signal === "stuck" || snapshot.signal === "blocked") {
|
||||
const proposedAction = "inject_guidance";
|
||||
const sideEffectClass = classifyPlannerActionSideEffect({ watchedStage: snapshot.stage, proposedAction });
|
||||
return {
|
||||
@@ -267,6 +282,18 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
|
||||
sideEffectClass,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
action: "none",
|
||||
reason: `Stage "${snapshot.stage}" signal "${snapshot.signal}" is healthy or awaiting a human — no autonomous steering`,
|
||||
attemptCount,
|
||||
attemptLimit,
|
||||
exhausted: false,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
requiresConfirmation: false,
|
||||
sideEffectClass: "bounded_recovery",
|
||||
};
|
||||
} catch {
|
||||
return {
|
||||
action: "none",
|
||||
|
||||
@@ -251,6 +251,32 @@ describe("PlannerOverseerMonitor.observeTask", () => {
|
||||
expect(store.logEntry).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
// FN-7577: an unchanged heartbeat (same stage/signal/reason) must not re-write
|
||||
// the activity feed on every poll tick — only a CHANGE re-logs; clear() resets
|
||||
// the dedup so a re-run re-logs its first observation.
|
||||
it("dedupes consecutive identical feed entries, re-logs on signal change, resets on clear", async () => {
|
||||
const store = { logEntry: vi.fn().mockResolvedValue(undefined) };
|
||||
const monitor = new PlannerOverseerMonitor({ store });
|
||||
const task = taskFixture({ column: "in-progress" });
|
||||
|
||||
// Three identical healthy ticks → a single feed entry.
|
||||
await monitor.observeTask(task, "observe");
|
||||
await monitor.observeTask(task, "observe");
|
||||
await monitor.observeTask(task, "observe");
|
||||
expect(store.logEntry).toHaveBeenCalledTimes(1);
|
||||
|
||||
// Signal flips (executor paused → "blocked") → re-logs once.
|
||||
const paused = { ...task, paused: true, pausedReason: "gate" };
|
||||
await monitor.observeTask(paused, "observe");
|
||||
await monitor.observeTask(paused, "observe");
|
||||
expect(store.logEntry).toHaveBeenCalledTimes(2);
|
||||
|
||||
// clear() drops the dedup key so the next identical observation re-logs.
|
||||
monitor.clear(task.id);
|
||||
await monitor.observeTask(paused, "observe");
|
||||
expect(store.logEntry).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
it("bounds the per-task ring buffer to the configured cap, keeping the most recent N", async () => {
|
||||
const monitor = new PlannerOverseerMonitor({ maxObservationsPerTask: 3 });
|
||||
const task = taskFixture({ column: "in-progress" });
|
||||
|
||||
@@ -84,6 +84,35 @@ describe("PlannerRecoveryController.tick", () => {
|
||||
expect(retryStep).toHaveBeenCalledTimes(PLANNER_RECOVERY_MAX_ATTEMPTS);
|
||||
});
|
||||
|
||||
// FN-7577: a stale recovery attempt must not keep a recovered task badged
|
||||
// "recovering" — a healthy/human-wait signal on the next tick clears the
|
||||
// per-(taskId, stage) attempt + last-action records, restoring a fresh budget.
|
||||
it("clears stale attempt records once the stage reports a healthy signal", async () => {
|
||||
const retryStep = vi.fn().mockResolvedValue(undefined);
|
||||
let current: OverseerStageObservation = observation({ signal: "failed" });
|
||||
const controller = new PlannerRecoveryController({
|
||||
snapshotProvider: { getSnapshot: () => current },
|
||||
handlers: { retryStep },
|
||||
});
|
||||
|
||||
await controller.tick(task());
|
||||
expect(controller.getAttemptCount("FN-1", "executor")).toBe(1);
|
||||
expect(controller.getLastAction("FN-1", "executor")).toBe("retry_step");
|
||||
|
||||
// Task recovers → healthy signal on the next tick clears the registry.
|
||||
current = observation({ signal: "progressing" });
|
||||
const healthy = await controller.tick(task());
|
||||
expect(healthy?.action).toBe("none");
|
||||
expect(controller.getAttemptCount("FN-1", "executor")).toBe(0);
|
||||
expect(controller.getLastAction("FN-1", "executor")).toBeUndefined();
|
||||
|
||||
// A later genuine failure starts from a fresh budget and dispatches again.
|
||||
current = observation({ signal: "failed" });
|
||||
await controller.tick(task());
|
||||
expect(retryStep).toHaveBeenCalledTimes(2);
|
||||
expect(controller.getAttemptCount("FN-1", "executor")).toBe(1);
|
||||
});
|
||||
|
||||
it("is inert when effectiveLevel/oversightLevel is off/observe/steer", async () => {
|
||||
for (const level of ["off", "observe", "steer"] as const) {
|
||||
const retryStep = vi.fn().mockResolvedValue(undefined);
|
||||
|
||||
@@ -255,6 +255,21 @@ export class PlannerOverseerMonitor {
|
||||
private readonly maxObservationsPerTask: number;
|
||||
private readonly observations = new Map<string, OverseerStageObservation[]>();
|
||||
|
||||
/*
|
||||
FNXC:PlannerOversight 2026-07-05-11:00:
|
||||
The overseer logs one activity-feed entry per poll tick. On the healthy path an
|
||||
executor task re-emits the identical `signal=progressing` heartbeat every tick,
|
||||
which spammed the task feed (user report FN-7577) with no new information and no
|
||||
lifecycle change. Dedup the feed write on the composite `stage|signal|reason`
|
||||
key so a log entry is only written when the observed situation CHANGES — mirrors
|
||||
the FN-7514 withheld-oversight dedup ("not re-emitted every poll while the reason
|
||||
is unchanged"). The in-memory ring buffer and `onObservation` callback are left
|
||||
intact (they are cheap / drive downstream emission façades); only the noisy feed
|
||||
logEntry is gated. Cleared alongside the ring buffer in `clear()` so a re-run of
|
||||
the same task re-logs its first observation.
|
||||
*/
|
||||
private readonly lastLoggedKey = new Map<string, string>();
|
||||
|
||||
constructor(options: PlannerOverseerMonitorOptions = {}) {
|
||||
this.store = options.store;
|
||||
this.onObservation = options.onObservation;
|
||||
@@ -299,9 +314,16 @@ export class PlannerOverseerMonitor {
|
||||
}
|
||||
|
||||
if (this.store?.logEntry) {
|
||||
await this.store
|
||||
.logEntry(task.id, `[planner-overseer] stage=${stage} signal=${signal}: ${reason}`)
|
||||
.catch(() => undefined);
|
||||
// FNXC:PlannerOversight 2026-07-05-11:00 — only write the feed entry when
|
||||
// the observed (stage, signal, reason) differs from the last one logged
|
||||
// for this task, so an unchanged heartbeat does not re-spam the feed.
|
||||
const loggedKey = `${stage}|${signal}|${reason}`;
|
||||
if (this.lastLoggedKey.get(task.id) !== loggedKey) {
|
||||
this.lastLoggedKey.set(task.id, loggedKey);
|
||||
await this.store
|
||||
.logEntry(task.id, `[planner-overseer] stage=${stage} signal=${signal}: ${reason}`)
|
||||
.catch(() => undefined);
|
||||
}
|
||||
}
|
||||
|
||||
return observation;
|
||||
@@ -327,6 +349,9 @@ export class PlannerOverseerMonitor {
|
||||
/** Clear recorded observations for a task (e.g. on task completion). */
|
||||
clear(taskId: string): void {
|
||||
this.observations.delete(taskId);
|
||||
// FNXC:PlannerOversight 2026-07-05-11:00 — drop the feed-dedup key too so a
|
||||
// re-run of the same task re-logs its first observation.
|
||||
this.lastLoggedKey.delete(taskId);
|
||||
}
|
||||
|
||||
/** Task IDs that currently retain at least one recorded observation. Used
|
||||
|
||||
@@ -265,6 +265,21 @@ export class PlannerRecoveryController {
|
||||
}
|
||||
|
||||
const key = this.attemptKey(task.id, snapshot.stage);
|
||||
|
||||
// FNXC:PlannerOversight 2026-07-05-11:00:
|
||||
// FN-7577: once a task's watched stage reports a HEALTHY (`progressing`/
|
||||
// `complete`) or human-wait (`awaiting-human`) signal, it is no longer
|
||||
// being recovered — drop any stale attempt / last-action records for the
|
||||
// (taskId, stage) so the card badge falls back from "recovering" to
|
||||
// "watching" on the next `GET /api/tasks` serialization, and a later
|
||||
// genuine problem starts from a fresh bounded budget. A still-problematic
|
||||
// signal (`stuck`/`blocked`/`failed`) keeps its attempts so the bound holds.
|
||||
const signal = snapshot.signal;
|
||||
if (signal === "progressing" || signal === "complete" || signal === "awaiting-human") {
|
||||
this.attempts.delete(key);
|
||||
this.lastActions.delete(key);
|
||||
}
|
||||
|
||||
const attemptCount = this.attempts.get(key) ?? 0;
|
||||
|
||||
const decision = decidePlannerRecovery({
|
||||
|
||||
Reference in New Issue
Block a user