fix(overseer): the whole oversight loop was inert on a renamed board (#2898)
`resolveWatchedStage` keyed on the literals `in-progress`/`in-review`, so on a board that renames either it returned `null` for **every** card. That is three literals with an outsized blast radius. `observeTask` returns early on a null stage, so: - no `OverseerStageObservation` is recorded, - no `overseer:intervention` entry is emitted, - and `PlannerRecoveryController`, which consumes those observations, has nothing to steer, retry or targeted-fix. **The entire oversight loop was inert and silent about it** — the same shape as the self-healing sweeps whose queries returned empty arrays. ## I deferred this myself, on a cost argument that was wrong The audit note I wrote for this site said resolving inside `observeTask` "buys a workflow read per card per poll". Then I read the caller: the poll **already awaits `resolveEffectiveSettings` per task**. It is a per-task async loop regardless, so with an IR cache keyed by workflow the addition is *(distinct workflows)* resolutions, not *(cards)*. Pricing the fix before checking the caller cost a deferral. Worth recording, because "this needs a cost judgement" is the most comfortable place in this program to leave something. ## The review test is the three-trait union, deliberately `isReviewColumnRole` checks only `mergeBlocker || humanReview`. A board whose review lane carries `merge` (**mergeOrchestration**) — the built-in default's own shape — would classify as *not in review* and be skipped. Reaching for the obvious helper would have reintroduced the bug this change removes, through the helper meant to fix it. There is a case asserting exactly that. ## Wiring Both call sites, because either alone leaves a hole: | site | why it matters | |---|---| | the poll (`project-engine.ts`) | per-poll IR cache — a workflow edit is picked up next tick rather than served stale | | the manual nudge | otherwise a renamed board answers `no-active-stage` to an operator pressing the button | `columnFlags` is in the `unwired-lane-parameter` vocabulary, so the wiring cannot silently rot — the guard reports it if a future change drops the argument. Fail-soft throughout: an unresolvable workflow yields `undefined` and the callee falls back to the legacy ids, which is exactly today's behaviour. A v1 IR declares no columns, so it takes the same path. ## Revert proof (measured) Drop the `columnFlags` branch and **exactly the three renamed-lane cases fail**: ``` expected null to be "executor" expected null to be "merger" (mergeOrchestration lane) expected null to be "merger" (humanReview lane) ``` The legacy-id and neither-role cases stay green — the gate must still gate, and watching every column would be its own defect. ## Verification - `pnpm test:gate` — 161 / 487 / 13 / 71 passed - `pnpm lint` — clean - `tsc --noEmit` (`@fusion/engine`) — clean - `planner-overseer.test.ts` + `planner-recovery-controller-human-control.test.ts` — 64 passed - unwired-lane guard — 9/9, no new entries Carries the one-line SQL-baseline re-record (`team-analytics.ts: 6 → 3`) that #2864 left behind, same as my other open branches — main is red on it, and identical changes to that line merge without conflict. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
7
.changeset/overseer-watched-lanes.md
Normal file
7
.changeset/overseer-watched-lanes.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Planner oversight now watches tasks on boards with renamed lanes instead of silently watching nothing.
|
||||
category: fix
|
||||
dev: `resolveWatchedStage` keyed on the literal `in-progress`/`in-review`, so on a renamed board it returned null for every card — `observeTask` returned early, no observation was recorded, and `PlannerRecoveryController` had nothing to act on. It now takes the task's resolved `columnFlags`, supplied by `project-engine.ts` at both call sites with a per-poll IR cache.
|
||||
@@ -19,6 +19,54 @@ function taskFixture(overrides: Partial<OverseerTaskRef> = {}): OverseerTaskRef
|
||||
} as OverseerTaskRef;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-00:20:
|
||||
|
||||
THE INVARIANT: the watched stage comes from the column's ROLE, not from its id.
|
||||
|
||||
Keyed on the id, `resolveWatchedStage` returned null for every card on a renamed board — and
|
||||
`observeTask` returns early on a null stage, so no observation was recorded, no
|
||||
`overseer:intervention` was emitted, and `PlannerRecoveryController` had nothing to steer, retry or
|
||||
targeted-fix. The whole oversight loop went inert and said nothing about it, which is why this is
|
||||
worth a parameter rather than a fallback.
|
||||
|
||||
THE REVIEW TEST IS THE THREE-TRAIT UNION. `isReviewColumnRole` checks only `mergeBlocker ||
|
||||
humanReview`, so a board whose review lane carries `merge` (mergeOrchestration) — the default's own
|
||||
shape — would classify as not-in-review and be skipped. That case is asserted below precisely because
|
||||
reaching for the obvious helper would have reintroduced the bug this change removes.
|
||||
|
||||
REVERT PROOF, measured: drop the `columnFlags` branch and the three renamed-lane cases fail with
|
||||
`expected null to be "executor" / "merger"`.
|
||||
*/
|
||||
describe("resolveWatchedStage keys on the column role", () => {
|
||||
const wip = { countsTowardWip: true } as never;
|
||||
const mergeLane = { mergeOrchestration: true } as never;
|
||||
const humanReviewLane = { humanReview: true } as never;
|
||||
|
||||
it("classifies a RENAMED wip lane as the executor stage", () => {
|
||||
expect(resolveWatchedStage(taskFixture({ column: "building" }), wip)).toBe("executor");
|
||||
});
|
||||
|
||||
it("classifies a review lane that carries only mergeOrchestration", () => {
|
||||
// The union matters: `isReviewColumnRole` would answer false here and the card would be skipped.
|
||||
expect(resolveWatchedStage(taskFixture({ column: "signoff" }), mergeLane)).toBe("merger");
|
||||
});
|
||||
|
||||
it("classifies a review lane that carries humanReview", () => {
|
||||
expect(resolveWatchedStage(taskFixture({ column: "waiting" }), humanReviewLane)).toBe("merger");
|
||||
});
|
||||
|
||||
it("still returns null for a lane carrying neither role", () => {
|
||||
// The gate must still gate — watching every column would be its own defect.
|
||||
expect(resolveWatchedStage(taskFixture({ column: "shipped" }), { complete: true } as never)).toBeNull();
|
||||
});
|
||||
|
||||
it("falls back to the legacy ids when no flags are supplied", () => {
|
||||
expect(resolveWatchedStage(taskFixture({ column: "in-progress" }))).toBe("executor");
|
||||
expect(resolveWatchedStage(taskFixture({ column: "todo" }))).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe("resolveWatchedStage", () => {
|
||||
it("resolves an active in-progress task to executor", () => {
|
||||
expect(resolveWatchedStage(taskFixture({ column: "in-progress" }))).toBe("executor");
|
||||
|
||||
@@ -14,7 +14,7 @@
|
||||
* the seam every later planner-oversight subtask reads from.
|
||||
*/
|
||||
|
||||
import { DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS, type PlannerOversightLevel, type PrInfo, type Task } from "@fusion/core";
|
||||
import { DEFAULT_PLANNER_OVERSEER_EXECUTOR_STUCK_AFTER_MS, type PlannerOversightLevel, type PrInfo, type Task, type TraitFlags } from "@fusion/core";
|
||||
|
||||
/** Alias for the `Task.reviewState` shape without requiring a separate core export. */
|
||||
type OverseerTaskReviewState = NonNullable<Task["reviewState"]>;
|
||||
@@ -98,7 +98,10 @@ export type OverseerTaskRef = Pick<
|
||||
*
|
||||
* Never throws — missing/partial fields degrade to `null`.
|
||||
*/
|
||||
export function resolveWatchedStage(task: Partial<OverseerTaskRef> | null | undefined): OverseerWatchedStage | null {
|
||||
export function resolveWatchedStage(
|
||||
task: Partial<OverseerTaskRef> | null | undefined,
|
||||
columnFlags?: TraitFlags,
|
||||
): OverseerWatchedStage | null {
|
||||
try {
|
||||
if (!task) return null;
|
||||
|
||||
@@ -111,36 +114,40 @@ export function resolveWatchedStage(task: Partial<OverseerTaskRef> | null | unde
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-30-21:35 (audited — REAL and HIGH IMPACT, deferred):
|
||||
On a renamed board this returns `null` for every card, so the planner overseer watches NOTHING.
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-00:20:
|
||||
Keyed on the ROLE, because keyed on the id this returned null for every card on a renamed board.
|
||||
|
||||
Worth stating at full weight because the blast radius is larger than the three literals suggest:
|
||||
`observeTask` returns early on a null stage, so no observation is recorded, no
|
||||
`overseer:intervention` entry is emitted, and `PlannerRecoveryController` — which consumes those
|
||||
observations — has nothing to steer, retry or targeted-fix. The entire oversight loop is inert and
|
||||
silent about it, exactly like the self-healing sweeps whose queries returned empty arrays.
|
||||
The blast radius is why this is worth the parameter rather than a fallback: `observeTask` returns
|
||||
early on a null stage, so no observation is recorded, no `overseer:intervention` entry is emitted,
|
||||
and `PlannerRecoveryController` — which consumes those observations — has nothing to steer, retry
|
||||
or targeted-fix. The entire oversight loop went inert and said nothing about it, the same shape as
|
||||
the self-healing sweeps whose queries returned empty arrays.
|
||||
|
||||
NOT MECHANICAL, which is why it is flagged rather than converted. `resolveWatchedStage` is a pure
|
||||
sync function over a `Partial<OverseerTaskRef>` with no store and no task id it can resolve from,
|
||||
so the lane answer has to arrive as a parameter. Its only production caller, `observeTask`, IS
|
||||
async and the monitor does hold a store — but `observeTask` is called once per task per poll from
|
||||
`project-engine.ts`, so resolving inside it buys a workflow read per card per poll on a timer.
|
||||
THE FLAGS ARRIVE AS A PARAMETER because this function is pure and sync with no store and no task
|
||||
id to resolve from. The cost objection I recorded when first auditing this — "resolving inside
|
||||
`observeTask` buys a workflow read per card per poll" — turned out to be answered by the caller:
|
||||
`project-engine.ts`'s poll ALREADY awaits `resolveEffectiveSettings` per task, so it is a per-task
|
||||
async loop already, and an IR cache keyed by workflow makes the addition (distinct workflows)
|
||||
resolutions rather than (cards).
|
||||
|
||||
The shape that works is the one the board-load enrichment ended up with (#2845): resolve at the
|
||||
POLL, once, with an IR cache keyed by workflow, and pass the flags down. That is a change to
|
||||
`project-engine.ts`'s poll as much as to this file, and it is a cost judgement about a periodic
|
||||
engine loop rather than a rename — the same call `notification-service.ts` documents for its own
|
||||
two sites.
|
||||
THE REVIEW TEST IS THE THREE-TRAIT UNION, not `isReviewColumnRole`, which checks only
|
||||
`mergeBlocker || humanReview`. A board whose review lane carries `merge` (mergeOrchestration) —
|
||||
the default's own shape — would otherwise be classified as not-in-review and skipped, which is the
|
||||
bug this change is removing, arriving through the helper meant to fix it.
|
||||
|
||||
Left counted with no exemption marker so the census keeps pointing here. `columnFlags` is in the
|
||||
unwired-lane-parameter vocabulary, so whoever adds the parameter cannot leave it unwired.
|
||||
`columnFlags` is in the unwired-lane-parameter vocabulary, so the wiring cannot silently rot.
|
||||
*/
|
||||
const column = task.column;
|
||||
if (column !== "in-progress" && column !== "in-review") {
|
||||
if (column === undefined) return null;
|
||||
const isWip = columnFlags ? columnFlags.countsTowardWip === true : column === "in-progress";
|
||||
const isReview = columnFlags
|
||||
? Boolean(columnFlags.mergeOrchestration || columnFlags.mergeBlocker || columnFlags.humanReview)
|
||||
: column === "in-review";
|
||||
if (!isWip && !isReview) {
|
||||
return null;
|
||||
}
|
||||
|
||||
if (column === "in-progress") {
|
||||
if (isWip) {
|
||||
return "executor";
|
||||
}
|
||||
|
||||
@@ -476,14 +483,14 @@ export class PlannerOverseerMonitor {
|
||||
async observeTask(
|
||||
task: OverseerTaskRef,
|
||||
level: PlannerOversightLevel,
|
||||
options?: { now?: () => number; executorStuckAfterMs?: number },
|
||||
options?: { now?: () => number; executorStuckAfterMs?: number; columnFlags?: TraitFlags },
|
||||
): Promise<OverseerStageObservation | null> {
|
||||
try {
|
||||
if (level === "off") {
|
||||
return null;
|
||||
}
|
||||
|
||||
const stage = resolveWatchedStage(task);
|
||||
const stage = resolveWatchedStage(task, options?.columnFlags);
|
||||
if (!stage) {
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -20,6 +20,9 @@ import type {
|
||||
import {
|
||||
resolveProjectColumnsForRoles,
|
||||
REVIEW_ROLES,
|
||||
resolveWorkflowIrForTask,
|
||||
resolveColumnFlags,
|
||||
type TraitFlags,
|
||||
allowsAutoMergeProcessing,
|
||||
compareTasksByPriorityThenAgeAndId,
|
||||
emitOverseerConfirmation,
|
||||
@@ -1504,6 +1507,41 @@ export class ProjectEngine {
|
||||
return this.prMonitor;
|
||||
}
|
||||
|
||||
/*
|
||||
FNXC:WorkflowLifecycleColumns 2026-07-31-00:20:
|
||||
The flags for a task's OWN column, for the planner-overseer stage classification.
|
||||
|
||||
`resolveWatchedStage` is pure and sync with no store, so the answer has to be resolved here and
|
||||
passed in. Keyed on the id it returned null for every card on a renamed board, and `observeTask`
|
||||
returns early on a null stage — so no observation, no `overseer:intervention`, and nothing for
|
||||
`PlannerRecoveryController` to act on. The oversight loop was inert and silent about it.
|
||||
|
||||
The IR cache is what makes this affordable: the poll below already awaits `resolveEffectiveSettings`
|
||||
per task, so it is a per-task async loop regardless, and caching by workflow means the added work is
|
||||
(distinct workflows) resolutions rather than (cards). The cache is per-poll on purpose — a longer
|
||||
lifetime would serve stale lanes after a workflow edit, which is the failure this program exists to
|
||||
remove wearing a different hat.
|
||||
|
||||
Fail-soft: an unresolvable workflow returns undefined and the callee falls back to the legacy ids,
|
||||
which is exactly today's behaviour.
|
||||
*/
|
||||
private async resolveTaskColumnFlags(
|
||||
store: TaskStore,
|
||||
task: Pick<Task, "id" | "column">,
|
||||
irCache: Map<string, WorkflowIr>,
|
||||
): Promise<TraitFlags | undefined> {
|
||||
try {
|
||||
const ir = await resolveWorkflowIrForTask(store, task.id, irCache);
|
||||
/* `WorkflowIr` is a union and the v1 arm declares no columns — a v1 graph has no lane
|
||||
vocabulary to read, so undefined (legacy ids) is the correct answer for it. */
|
||||
const columns = (ir as { columns?: Array<{ id: string }> }).columns;
|
||||
const declared = columns?.find((column) => column.id === task.column);
|
||||
return declared ? resolveColumnFlags(declared as never) : undefined;
|
||||
} catch {
|
||||
return undefined;
|
||||
}
|
||||
}
|
||||
|
||||
/** Get the records-only PlannerOverseerMonitor (if initialized). See FN-7511. */
|
||||
getPlannerOverseer(): PlannerOverseerMonitor | undefined {
|
||||
return this.plannerOverseer;
|
||||
@@ -1591,7 +1629,10 @@ export class ProjectEngine {
|
||||
|
||||
let observation = this.plannerOverseer ? this.plannerOverseer.getObservations(taskId).slice(-1)[0] : undefined;
|
||||
if (!observation && this.plannerOverseer) {
|
||||
observation = (await this.plannerOverseer.observeTask(task, level)) ?? undefined;
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-07-31-00:20: the manual nudge classifies the same way the
|
||||
poll does, or a renamed board answers `no-active-stage` to an operator pressing the button. */
|
||||
const columnFlags = await this.resolveTaskColumnFlags(store, task, new Map());
|
||||
observation = (await this.plannerOverseer.observeTask(task, level, { columnFlags })) ?? undefined;
|
||||
}
|
||||
if (!observation) {
|
||||
return { applied: false, reason: "no-active-stage", task };
|
||||
@@ -2971,6 +3012,8 @@ export class ProjectEngine {
|
||||
// FNXC:PlannerOversight 2026-07-14-00:10: keep session-advisor human-control on live settings.
|
||||
this.sessionAdvisor?.setSettings(engineSettings);
|
||||
|
||||
/* One IR cache for the whole sweep: (distinct workflows) resolutions, not (cards). */
|
||||
const overseerIrCache = new Map<string, WorkflowIr>();
|
||||
for (const task of inFlight) {
|
||||
try {
|
||||
const workflowEffective = await resolveEffectiveSettings(store, { id: task.id }).catch(() => ({}) as Record<string, unknown>);
|
||||
@@ -3001,7 +3044,11 @@ export class ProjectEngine {
|
||||
// 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 });
|
||||
/* FNXC:WorkflowLifecycleColumns 2026-07-31-00:20: without this the stage is resolved from
|
||||
the legacy ids and every card on a renamed board classifies as null — see
|
||||
`resolveTaskColumnFlags`. The cache is per-poll so a workflow edit is picked up next tick. */
|
||||
const columnFlags = await this.resolveTaskColumnFlags(store, task, overseerIrCache);
|
||||
await overseer.observeTask(task, level, { executorStuckAfterMs, columnFlags });
|
||||
|
||||
// FN-7512: one guarded, autonomous-only bounded recovery tick at the
|
||||
// same passive seam FN-7511 uses for observation. Inert for every
|
||||
|
||||
@@ -10,7 +10,6 @@
|
||||
"packages/engine/src/restart-recovery-coordinator.ts": 4,
|
||||
"packages/core/src/async-mission-store-queries.ts": 3,
|
||||
"packages/core/src/task-store/task-artifacts-ops.ts": 3,
|
||||
"packages/engine/src/planner-overseer.ts": 3,
|
||||
"packages/core/src/agent-store.ts": 2,
|
||||
"packages/core/src/task-store/audit-ops.ts": 2,
|
||||
"packages/core/src/task-store/moves.ts": 2,
|
||||
@@ -21,6 +20,7 @@
|
||||
"packages/dashboard/src/github-tracking-state.ts": 2,
|
||||
"packages/dashboard/src/routes/register-task-workflow-routes.ts": 2,
|
||||
"packages/engine/src/auto-merge-finalization.ts": 2,
|
||||
"packages/engine/src/planner-overseer.ts": 2,
|
||||
"packages/core/src/eval-signal-collector.ts": 1,
|
||||
"packages/core/src/in-review-stall.ts": 1,
|
||||
"packages/core/src/mission-store.ts": 1,
|
||||
|
||||
Reference in New Issue
Block a user