fix(engine): overseer vetoes no-op merge finalize when the last executor signal was failed-with-incomplete-work (#2258)
## What & why FN-8141 (\"Update pi SDK … verify Kimi K3 end to end\") was **laundered into `done`** despite producing zero net changes. The executor reverted the impossible work 5 times; the planner overseer emitted `stage=executor signal=failed` (\"Executor stage parked failed with work incomplete\") **twice**, then an hour later — because the overseer is **stage-scoped and memoryless** — classified the same task `stage=merger signal=progressing` and let the AI merger's **empty no-op finalize** promote it to `done`. No reviewer ever saw it (skipped steps request no review; the merge-review pass reviews an empty diff). This restores the invariant: **a task whose most-recent executor signal is failed-with-incomplete-work, with no subsequent green completion, must not reach `done` via a zero-diff no-op merge finalize.** ## Change Two pure, unit-testable, never-throw functions (`packages/engine/src/overseer-noop-finalize-veto.ts`), following the FN-7514 `evaluateOverseerHumanControl` precedent (pure predicate + ids/outcomes-only audit metadata): - **`deriveExecutorSignalMemory`** — reconstructs the most-recent executor signal from the **durable `overseer:intervention` timeline the overseer already writes** (no new persisted column / migration; \"the existing oversight state storage the controller uses\"). A later green executor observation supersedes an earlier failure, which is how \"no subsequent execution completed green\" is derived. Keys on the newly-exported `EXECUTOR_FAILED_INCOMPLETE_REASON` constant (already load-bearing for FN-7577 feed dedup) as the single source of truth. - **`evaluateNoOpFinalizeExecutorVeto`** — the veto decision. Wired into the AI **empty-merge lane** (`merger-ai.ts`), composed with (and independent of) the FN-6461 no-commits guard: on veto it sets `error`, writes a durable log entry, emits `overseer:no-op-finalize-vetoed-failed-executor`, and moves the task back to `todo` with progress preserved — mirroring the FN-6461 blocked lane. The move-to-todo transition takes the task out of the merge lane, so the event isn't re-emitted every poll (equivalent to the `overseer:oversight-withheld-human-control` per-(taskId, reason) dedup). Independent of the sibling Task 2 merger-layer lineage guard: both can fire; **either alone stops FN-8141**. ## Surface enumeration - **Empty (zero-diff) no-op merge** — vetoed when memory is failed-incomplete. ✅ - **Non-empty (real squash landed) merge** — **never** vetoed (reviewers/merge review cover real diffs). ✅ - **failed-incomplete then later green execution** — memory superseded → no veto. ✅ - **No executor memory / store lacks the async reader** — fail open (no veto); FN-6461 + sibling guard remain the safety net. ✅ - **user-paused / autoMerge:false / approval-blocked** — defers to FN-7514 human-control; never fights those semantics. ✅ - **Timeline entry filtering** — only `stage=executor` + `action=observe` entries count as signals (retry/escalate messages ignored). ✅ ## Test evidence `pnpm --filter @fusion/engine exec vitest run src/__tests__/overseer-noop-finalize-veto.test.ts src/__tests__/merger-ai.test.ts --silent=passed-only --reporter=dot` ``` Test Files 2 passed (2) Tests 48 passed (48) ``` Covers: derivation (failed→veto, failed-then-green→no-veto, non-executor/non-observe ignored, empty→null); evaluator (veto, green, no-memory, non-empty never-vetoed, user-paused defer, autoMerge:false defer, missing-task fail-open); and an engine integration test driving an **FN-8141-shaped** empty merge through `runAiMerge` → asserts move-to-todo + `overseer:no-op-finalize-vetoed-failed-executor` audit event + main untouched, plus the later-green case finalizing done. `@fusion/core` builds clean (`pnpm --filter @fusion/core build`). **Note on `pnpm verify:fast`:** it currently fails to build `@fusion/engine`, but **only** in `auth-storage.ts` / `pi.ts` / `provider-registration.ts` — the pre-existing pi-SDK breakage that *is* this incident (pi 0.80.x removed `AuthStorage`/`ModelRuntime`/`ModelRegistry`; tracked as FN-8145). Verified identical errors with my changes stashed; **my diff touches none of those files and adds zero new type errors** (tsc reports all program errors before failing — none were in my files). 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **Bug Fixes** - Prevented zero-change tasks from being incorrectly finalized when the latest executor attempt failed with unfinished work. - Preserved task progress and returned affected tasks to **Todo** for continued processing. - Allowed finalization to proceed after a subsequent successful executor result. - Maintained existing human-control and non-empty merge behavior. - Added audit visibility for blocked finalization events. <!-- end of auto-generated comment: release notes by coderabbit.ai --> Co-authored-by: Claude Opus <noreply@anthropic.com>
This commit is contained in:
gsxdsm
7
.changeset/overseer-noop-finalize-veto.md
Normal file
7
.changeset/overseer-noop-finalize-veto.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Block a zero-change task from completing when its executor last failed with work unfinished.
|
||||
category: fix
|
||||
dev: FN-8141. Adds `evaluateNoOpFinalizeExecutorVeto` + `deriveExecutorSignalMemory` (pure, engine-local) giving the merger cross-stage memory of the most-recent executor overseer signal (derived from the durable `overseer:intervention` timeline). The AI empty-merge lane (`merger-ai.ts`) now vetoes a no-op finalize — moving the task back to `todo` with progress preserved and emitting `overseer:no-op-finalize-vetoed-failed-executor` — when the latest executor signal was failed-with-incomplete-work and no later execution completed green. Non-empty merges are never vetoed; defers to the FN-7514 human-control contract (user-paused / autoMerge:false).
|
||||
File diff suppressed because one or more lines are too long
@@ -1353,6 +1353,41 @@ export interface PrThreadState {
|
||||
updatedAt: number;
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:Lifecycle 2026-07-16-09:40:
|
||||
* FN-8141 cross-stage overseer memory. FN-8141 was laundered into `done`
|
||||
* because the planner overseer is stage-scoped and memoryless: it emitted
|
||||
* `stage=executor signal=failed` (parked failed with work incomplete) twice,
|
||||
* then an hour later saw `stage=merger signal=progressing` and let an empty
|
||||
* no-op merge finalize the task `done` — nothing connected the failed executor
|
||||
* verdict to the merger's finalize decision.
|
||||
*
|
||||
* This is the derived (NOT persisted-as-a-column) most-recent executor-stage
|
||||
* overseer signal, reconstructed on demand from the durable
|
||||
* `overseer:intervention` timeline the overseer already writes (see
|
||||
* `deriveExecutorSignalMemory` in the engine). It is the evidence the
|
||||
* merger-layer no-op-finalize veto (`evaluateNoOpFinalizeExecutorVeto`) reads
|
||||
* to refuse completing a zero-diff task whose executor never finished green.
|
||||
* Since the executor stage only exists while a task is `in-progress`, a later
|
||||
* green re-execution appends a non-`failed` executor observation that becomes
|
||||
* the newest entry (clearing `incompleteWork`) — this is how "no subsequent
|
||||
* execution completed green" is derived: the memory always reflects the LATEST
|
||||
* executor observation.
|
||||
*/
|
||||
export interface ExecutorOverseerSignalMemory {
|
||||
/** The most recent executor-stage `OverseerObservationSignal` (bare string to avoid pulling the engine stage taxonomy into core). */
|
||||
signal: string;
|
||||
/**
|
||||
* True iff `signal` is the failed-with-incomplete-work executor shape
|
||||
* (the overseer's `signal: "failed"` executor observation — "Executor stage
|
||||
* parked failed with work incomplete"). A later `progressing`/`complete`/etc.
|
||||
* executor observation supersedes it, deriving `false`.
|
||||
*/
|
||||
incompleteWork: boolean;
|
||||
/** epoch-ms (or intervention-entry timestamp) of the observation that produced this memory. */
|
||||
observedAt: number;
|
||||
}
|
||||
|
||||
export interface Task {
|
||||
id: string;
|
||||
/** Immutable lineage identity used for durable commit/task attribution. */
|
||||
|
||||
@@ -33,6 +33,7 @@ import {
|
||||
REVIEW_VERDICT_MARKER,
|
||||
AiMergeBlockedError,
|
||||
} from "../merger-ai.js";
|
||||
import { EXECUTOR_FAILED_INCOMPLETE_REASON } from "../planner-overseer.js";
|
||||
|
||||
const RM = { recursive: true, force: true, maxRetries: 5, retryDelay: 50 } as const;
|
||||
const tracked = new Set<string>();
|
||||
@@ -710,6 +711,86 @@ describe("runAiMerge", () => {
|
||||
);
|
||||
});
|
||||
|
||||
/*
|
||||
* FN-8141 guard (3) — executor-signal veto — exercised IN ISOLATION.
|
||||
* The sibling guards (1) step-evidence and (2) already-landed-proof already
|
||||
* catch the exact FN-8141 shape (covered by the tests above). These two tests
|
||||
* prove guard (3) blocks independently on DIFFERENT evidence: a task that
|
||||
* PASSES guard (1) (all steps `done`, none skipped) and SKIPS guard (2)
|
||||
* (`noCommitsExpected`) — only the durable executor overseer signal reveals
|
||||
* the executor never finished green.
|
||||
*/
|
||||
it("FN-8141: vetoes an empty no-op finalize when the last executor signal was failed-with-incomplete-work", async () => {
|
||||
const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" });
|
||||
git(dir, "merge -q fusion/fn-1"); // fold branch work into main → branch is now empty
|
||||
const { store, task } = makeStore(dir, {
|
||||
noCommitsExpected: true,
|
||||
steps: [
|
||||
{ name: "Plan", status: "done" },
|
||||
{ name: "Execute", status: "done" },
|
||||
],
|
||||
});
|
||||
// Durable overseer timeline: newest executor observation is failed-incomplete.
|
||||
store.getRunAuditEventsAsync = vi.fn(async () => [
|
||||
{
|
||||
id: "ev-fail-2", taskId: "FN-1", target: "FN-1", timestamp: "2026-07-16T22:40:00.000Z",
|
||||
domain: "database", mutationType: "overseer:intervention", runId: "r2", agentId: "overseer",
|
||||
metadata: { stage: "executor", reason: EXECUTOR_FAILED_INCOMPLETE_REASON, action: "observe", outcome: "succeeded" },
|
||||
},
|
||||
]);
|
||||
const auditDb: unknown[] = [];
|
||||
const priorRecord = store.recordRunAuditEvent;
|
||||
store.recordRunAuditEvent = vi.fn((e: any) => { auditDb.push(e); return priorRecord?.(e); });
|
||||
const mainBefore = git(dir, "rev-parse main");
|
||||
|
||||
const result = await runAiMerge(store, dir, "FN-1", { manual: true }, {
|
||||
mergeAgent: vi.fn(async () => { /* nothing to do */ }),
|
||||
reviewAgent: vi.fn(async () => "REVIEW_VERDICT: approve"),
|
||||
});
|
||||
|
||||
// Vetoed to todo — NOT laundered to done.
|
||||
expect(result.merged).toBe(false);
|
||||
expect(result.noOp).toBe(false);
|
||||
expect(task.column).toBe("todo");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-1", "todo", expect.objectContaining({ preserveProgress: true, moveSource: "engine" }));
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-1", "done", expect.anything());
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-1",
|
||||
expect.stringContaining("Finalize blocked (overseer failed-executor veto)"),
|
||||
expect.stringContaining("ai-empty-merge"),
|
||||
);
|
||||
expect(auditDb.some((e: any) => e.mutationType === "overseer:no-op-finalize-vetoed-failed-executor")).toBe(true);
|
||||
expect(git(dir, "rev-parse main")).toBe(mainBefore);
|
||||
});
|
||||
|
||||
it("FN-8141: does NOT veto an empty no-op finalize when a later executor observation was green", async () => {
|
||||
const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" });
|
||||
git(dir, "merge -q fusion/fn-1");
|
||||
const { store, task } = makeStore(dir, { noCommitsExpected: true, steps: [{ name: "Execute", status: "done" }] });
|
||||
// Timeline newest-first: a green executor observation supersedes the failure.
|
||||
store.getRunAuditEventsAsync = vi.fn(async () => [
|
||||
{
|
||||
id: "ev-green", taskId: "FN-1", target: "FN-1", timestamp: "2026-07-16T23:10:00.000Z",
|
||||
domain: "database", mutationType: "overseer:intervention", runId: "r3", agentId: "overseer",
|
||||
metadata: { stage: "executor", reason: "Task is actively executing in-progress work", action: "observe", outcome: "succeeded" },
|
||||
},
|
||||
{
|
||||
id: "ev-fail", taskId: "FN-1", target: "FN-1", timestamp: "2026-07-16T22:40:00.000Z",
|
||||
domain: "database", mutationType: "overseer:intervention", runId: "r2", agentId: "overseer",
|
||||
metadata: { stage: "executor", reason: EXECUTOR_FAILED_INCOMPLETE_REASON, action: "observe", outcome: "succeeded" },
|
||||
},
|
||||
]);
|
||||
|
||||
const result = await runAiMerge(store, dir, "FN-1", { manual: true }, {
|
||||
mergeAgent: vi.fn(async () => { /* nothing to do */ }),
|
||||
reviewAgent: vi.fn(async () => "REVIEW_VERDICT: approve"),
|
||||
});
|
||||
|
||||
expect(result.noOp).toBe(true);
|
||||
expect(task.column).toBe("done");
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-1", "done", expect.objectContaining({ moveSource: "engine", preserveProgress: true }));
|
||||
});
|
||||
|
||||
it("fails loudly when an executed, never-merged task has no branch (possible lost work)", async () => {
|
||||
const { dir } = initRepoWithBranch({ branch: "fusion/fn-1" });
|
||||
// branch points at a ref that doesn't exist; task was executed (baseCommitSha) and never merged.
|
||||
|
||||
@@ -0,0 +1,126 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import type { ExecutorOverseerSignalMemory, PlannerInterventionEntry } from "@fusion/core";
|
||||
import { EXECUTOR_FAILED_INCOMPLETE_REASON } from "../planner-overseer.js";
|
||||
import {
|
||||
deriveExecutorSignalMemory,
|
||||
evaluateNoOpFinalizeExecutorVeto,
|
||||
NO_OP_FINALIZE_EXECUTOR_VETO_REASON,
|
||||
type NoOpFinalizeExecutorVetoTask,
|
||||
} from "../overseer-noop-finalize-veto.js";
|
||||
|
||||
/**
|
||||
* FNXC:Lifecycle 2026-07-16-09:40:
|
||||
* FN-8141 invariant coverage for the overseer-layer no-op-finalize veto. Tests
|
||||
* assert the GENERAL invariant across all enumerated surfaces (not only the
|
||||
* exact FN-8141 shape): failed-incomplete→no-green ⇒ veto; failed-then-green ⇒
|
||||
* no veto; non-empty merge ⇒ never vetoed; user-paused / autoMerge:false ⇒
|
||||
* defer to the FN-7514 human-control contract.
|
||||
*/
|
||||
|
||||
function entry(overrides: Partial<PlannerInterventionEntry>): PlannerInterventionEntry {
|
||||
return {
|
||||
id: overrides.id ?? "ev-1",
|
||||
taskId: overrides.taskId ?? "FN-1",
|
||||
timestamp: overrides.timestamp ?? "2026-07-16T22:00:00.000Z",
|
||||
stage: overrides.stage ?? "executor",
|
||||
reason: overrides.reason ?? "Task is actively executing in-progress work",
|
||||
action: overrides.action ?? "observe",
|
||||
outcome: overrides.outcome ?? "succeeded",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
const failedEntry = (overrides: Partial<PlannerInterventionEntry> = {}) =>
|
||||
entry({ reason: EXECUTOR_FAILED_INCOMPLETE_REASON, ...overrides });
|
||||
|
||||
const okTask: NoOpFinalizeExecutorVetoTask = {
|
||||
userPaused: false,
|
||||
paused: false,
|
||||
pausedReason: undefined,
|
||||
status: undefined,
|
||||
autoMerge: true,
|
||||
prInfo: undefined,
|
||||
prInfos: undefined,
|
||||
};
|
||||
|
||||
const incompleteMemory: ExecutorOverseerSignalMemory = { signal: "failed", incompleteWork: true, observedAt: 1 };
|
||||
|
||||
describe("deriveExecutorSignalMemory", () => {
|
||||
it("returns null when there are no intervention entries", () => {
|
||||
expect(deriveExecutorSignalMemory(null)).toBeNull();
|
||||
expect(deriveExecutorSignalMemory([])).toBeNull();
|
||||
});
|
||||
|
||||
it("derives incompleteWork from the newest executor failed-incomplete observation", () => {
|
||||
const memory = deriveExecutorSignalMemory([failedEntry({ timestamp: "2026-07-16T22:40:00.000Z" })]);
|
||||
expect(memory).toEqual({ signal: "failed", incompleteWork: true, observedAt: Date.parse("2026-07-16T22:40:00.000Z") });
|
||||
});
|
||||
|
||||
it("a later green executor observation supersedes an earlier failed one (no subsequent-green derivation)", () => {
|
||||
// Timeline is newest-first, as getPlannerInterventionTimeline returns it.
|
||||
const memory = deriveExecutorSignalMemory([
|
||||
entry({ id: "green", timestamp: "2026-07-16T23:10:00.000Z", reason: "Task is actively executing in-progress work" }),
|
||||
failedEntry({ id: "fail", timestamp: "2026-07-16T22:40:00.000Z" }),
|
||||
]);
|
||||
expect(memory?.incompleteWork).toBe(false);
|
||||
expect(memory?.signal).toBe("progressing");
|
||||
});
|
||||
|
||||
it("ignores non-executor stages and non-observe actions when picking the newest signal", () => {
|
||||
const memory = deriveExecutorSignalMemory([
|
||||
// Newest overall, but a merger observation — must be ignored.
|
||||
entry({ id: "merger", stage: "merger", timestamp: "2026-07-16T23:40:00.000Z", reason: "Task is in the merge/integration phase" }),
|
||||
// Newer than the failed one, but a retry action (recovery message, not a signal) — ignored.
|
||||
entry({ id: "retry", stage: "executor", action: "retry", timestamp: "2026-07-16T23:00:00.000Z", reason: "retrying step" }),
|
||||
failedEntry({ id: "fail", timestamp: "2026-07-16T22:40:00.000Z" }),
|
||||
]);
|
||||
expect(memory?.incompleteWork).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
describe("evaluateNoOpFinalizeExecutorVeto", () => {
|
||||
it("vetoes an empty merge when the most-recent executor signal is failed-with-incomplete-work", () => {
|
||||
const decision = evaluateNoOpFinalizeExecutorVeto({ mergeIsEmpty: true, task: okTask, memory: incompleteMemory });
|
||||
expect(decision.veto).toBe(true);
|
||||
expect(decision.reason).toBe(NO_OP_FINALIZE_EXECUTOR_VETO_REASON);
|
||||
});
|
||||
|
||||
it("does NOT veto when a later execution completed green (memory not incompleteWork)", () => {
|
||||
const greenMemory: ExecutorOverseerSignalMemory = { signal: "progressing", incompleteWork: false, observedAt: 2 };
|
||||
expect(evaluateNoOpFinalizeExecutorVeto({ mergeIsEmpty: true, task: okTask, memory: greenMemory }).veto).toBe(false);
|
||||
});
|
||||
|
||||
it("does NOT veto when there is no executor memory at all", () => {
|
||||
expect(evaluateNoOpFinalizeExecutorVeto({ mergeIsEmpty: true, task: okTask, memory: null }).veto).toBe(false);
|
||||
});
|
||||
|
||||
it("NEVER vetoes a non-empty (real squash landed) merge, even with failed-incomplete memory", () => {
|
||||
const decision = evaluateNoOpFinalizeExecutorVeto({ mergeIsEmpty: false, task: okTask, memory: incompleteMemory });
|
||||
expect(decision.veto).toBe(false);
|
||||
});
|
||||
|
||||
it("defers (no veto) for a user-paused task per the FN-7514 human-control contract", () => {
|
||||
const paused: NoOpFinalizeExecutorVetoTask = { ...okTask, userPaused: true };
|
||||
const decision = evaluateNoOpFinalizeExecutorVeto({ mergeIsEmpty: true, task: paused, memory: incompleteMemory });
|
||||
expect(decision.veto).toBe(false);
|
||||
expect(decision.deferredForHumanControl).toBe(true);
|
||||
expect(decision.humanControlReason).toBe("user-paused");
|
||||
});
|
||||
|
||||
it("defers (no veto) for an autoMerge:false / human-review task", () => {
|
||||
const humanReview: NoOpFinalizeExecutorVetoTask = { ...okTask, autoMerge: false };
|
||||
const decision = evaluateNoOpFinalizeExecutorVeto({
|
||||
mergeIsEmpty: true,
|
||||
task: humanReview,
|
||||
memory: incompleteMemory,
|
||||
settings: { autoMerge: false },
|
||||
});
|
||||
expect(decision.veto).toBe(false);
|
||||
expect(decision.deferredForHumanControl).toBe(true);
|
||||
expect(decision.humanControlReason).toBe("auto-merge-off-human-review");
|
||||
});
|
||||
|
||||
it("does not veto a missing task (fails open — other guards remain)", () => {
|
||||
expect(evaluateNoOpFinalizeExecutorVeto({ mergeIsEmpty: true, task: null, memory: incompleteMemory }).veto).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -44,6 +44,7 @@ import {
|
||||
assertNotWorkspaceTaskMerge,
|
||||
buildTaskLineageTrailer,
|
||||
evaluateNoCommitsNoOpFinalize,
|
||||
getPlannerInterventionTimeline,
|
||||
getPrimaryPrInfo,
|
||||
getTaskMergeBlocker,
|
||||
normalizeMergeAdvanceAutoSyncMode,
|
||||
@@ -69,6 +70,7 @@ import { withRateLimitRetry } from "./rate-limit-retry.js";
|
||||
import { checkSessionError } from "./usage-limit-detector.js";
|
||||
import { accumulateSessionTokenUsage } from "./session-token-usage.js";
|
||||
import { createRunAuditor, generateSyntheticRunId, type RunAuditor } from "./run-audit.js";
|
||||
import { deriveExecutorSignalMemory, evaluateNoOpFinalizeExecutorVeto } from "./overseer-noop-finalize-veto.js";
|
||||
import { createLogger } from "./logger.js";
|
||||
import {
|
||||
buildAutostashLabel,
|
||||
@@ -1226,6 +1228,13 @@ export async function runAiMerge(
|
||||
* preserved so an operator (or reviewer) sees it instead of it laundering into `done`.
|
||||
* task.error keeps recoverStrandedCompletedTodoTasks from re-promoting the unchanged task (it
|
||||
* excludes any task with `task.error` set), mirroring the FN-6461 blocked lane above.
|
||||
*
|
||||
* FNXC:Lifecycle 2026-07-16-09:40:
|
||||
* Empty-lane guard ORDER (each blocks BEFORE finalizeMerged; first blocker wins; all coexist):
|
||||
* (1) FN-6461/#2254 step-evidence guard (`evaluateNoCommitsNoOpFinalize`, above)
|
||||
* (2) #2259 already-landed-proof guard (this block, commit-expected only)
|
||||
* (3) FN-8141 executor-signal veto (`evaluateNoOpFinalizeExecutorVeto`, below)
|
||||
* They use INDEPENDENT evidence, so any one alone stops the FN-8141 laundering shape.
|
||||
*/
|
||||
if (task.noCommitsExpected !== true) {
|
||||
const landedProof = await proveEmptyMergeAlreadyLanded(task, branch, integrationBranch, projectRootDir);
|
||||
@@ -1267,6 +1276,71 @@ export async function runAiMerge(
|
||||
`AI merge: ${branch} had no net changes vs ${integrationBranch} but work already landed (proof=${landedProof.strategy}${landedProof.sha ? ` sha=${landedProof.sha.slice(0, 8)}` : ""}) — finalizing as no-op`,
|
||||
);
|
||||
}
|
||||
|
||||
/*
|
||||
* FNXC:Lifecycle 2026-07-16-09:40:
|
||||
* FN-8141 overseer-layer backstop — guard (3) in the empty-lane order above.
|
||||
* Independent of, and composed with, the FN-6461/#2254 step-evidence guard
|
||||
* and the #2259 already-landed-proof guard (this one keys on the cross-stage
|
||||
* executor overseer signal, derived from the durable `overseer:intervention`
|
||||
* timeline). EITHER of the three alone must stop the FN-8141 laundering
|
||||
* shape. Only the zero-diff no-op lane is in scope — a real squash landing
|
||||
* never reaches here. `evaluateNoOpFinalizeExecutorVeto` is pure and defers
|
||||
* to the FN-7514 human-control contract, so it never fights user-paused /
|
||||
* autoMerge:false tasks.
|
||||
*/
|
||||
// Derive the most-recent executor signal from the durable
|
||||
// `overseer:intervention` timeline (best-effort — a store without the async
|
||||
// reader, or a query failure, degrades to `null` = no veto, so other guards
|
||||
// remain the safety net).
|
||||
let executorMemory = null as Awaited<ReturnType<typeof deriveExecutorSignalMemory>>;
|
||||
try {
|
||||
const timeline = await getPlannerInterventionTimeline(store, taskId);
|
||||
executorMemory = deriveExecutorSignalMemory(timeline);
|
||||
} catch (err) {
|
||||
aiMergeLog.warn(`${taskId}: executor overseer-memory derivation failed (skipping veto): ${getErrorMessage(err)}`);
|
||||
}
|
||||
const executorVeto = evaluateNoOpFinalizeExecutorVeto({ mergeIsEmpty: true, task, memory: executorMemory, settings });
|
||||
if (executorVeto.veto) {
|
||||
const vetoReason = executorVeto.reason ?? "overseer failed-executor no-op-finalize veto";
|
||||
await store.updateTask(taskId, { error: vetoReason });
|
||||
await store.logEntry(
|
||||
taskId,
|
||||
`Finalize blocked (overseer failed-executor veto): ${vetoReason} — moving back to todo with progress preserved`,
|
||||
JSON.stringify({
|
||||
executorSignal: executorMemory?.signal,
|
||||
executorSignalObservedAt: executorMemory?.observedAt,
|
||||
branch,
|
||||
integrationBranch,
|
||||
lane: "ai-empty-merge",
|
||||
}, null, 2),
|
||||
);
|
||||
await audit.database({
|
||||
type: "overseer:no-op-finalize-vetoed-failed-executor" as Parameters<typeof audit.database>[0]["type"],
|
||||
target: taskId,
|
||||
metadata: {
|
||||
reason: vetoReason,
|
||||
executorSignal: executorMemory?.signal,
|
||||
executorSignalObservedAt: executorMemory?.observedAt,
|
||||
branch,
|
||||
integrationBranch,
|
||||
lane: "ai-empty-merge",
|
||||
},
|
||||
});
|
||||
await store.moveTask(taskId, "todo", { preserveProgress: true, moveSource: "engine" } as Parameters<TaskStore["moveTask"]>[2]);
|
||||
return {
|
||||
task,
|
||||
branch,
|
||||
merged: false,
|
||||
noOp: false,
|
||||
ok: true,
|
||||
reason: vetoReason,
|
||||
error: vetoReason,
|
||||
worktreeRemoved: false,
|
||||
branchDeleted: false,
|
||||
};
|
||||
}
|
||||
|
||||
await log(`AI merge: ${branch} had no net changes vs ${integrationBranch} — finalizing as no-op`);
|
||||
const noOpFinalized = await finalizeMerged(store, projectRootDir, taskId, task, branch, integrationBranch, landResult.tipSha, audit, log, { empty: true }, mergeTarget, groupRouting, options.syncGroupPr);
|
||||
await runPushAfterMergeStep({ store, projectRootDir, taskId, settings, integrationBranch, audit, log, options, result: noOpFinalized });
|
||||
|
||||
180
packages/engine/src/overseer-noop-finalize-veto.ts
Normal file
180
packages/engine/src/overseer-noop-finalize-veto.ts
Normal file
@@ -0,0 +1,180 @@
|
||||
/**
|
||||
* FNXC:Lifecycle 2026-07-16-09:40:
|
||||
* FN-8141 overseer-layer backstop against no-op finalize laundering.
|
||||
*
|
||||
* Incident: FN-8141 was impossible as specced (an SDK bump broke verify every
|
||||
* attempt). The executor reverted the work 5 times; the planner overseer
|
||||
* emitted `stage=executor signal=failed` ("Executor stage parked failed with
|
||||
* work incomplete") TWICE, then — because the overseer is stage-scoped and
|
||||
* memoryless — an hour later classified the same task `stage=merger
|
||||
* signal=progressing` and let the AI merger's EMPTY (zero net changes vs main)
|
||||
* no-op finalize promote the task to `done`. No reviewer ever saw it (skipped
|
||||
* steps request no review; the merge-review pass reviews an empty diff).
|
||||
*
|
||||
* Restored invariant: a task whose MOST RECENT executor-stage signal is
|
||||
* failed-with-incomplete-work, with NO subsequent execution session completing
|
||||
* green, must NOT reach `done` via a zero-diff no-op merge finalize. It takes
|
||||
* the blocked path instead (error set, durable log entry,
|
||||
* `overseer:no-op-finalize-vetoed-failed-executor` run-audit event, moved back
|
||||
* to `todo` with progress preserved — mirroring the FN-6461 no-commits blocked
|
||||
* lane in `merger-ai.ts`).
|
||||
*
|
||||
* Two pure, unit-testable pieces (no I/O, never throw), following the FN-7514
|
||||
* `evaluateOverseerHumanControl` precedent (pure predicate + ids/outcomes-only
|
||||
* audit metadata):
|
||||
* - `deriveExecutorSignalMemory` — reconstructs the most-recent executor
|
||||
* signal from the durable `overseer:intervention` timeline the overseer
|
||||
* already writes (no new persisted column; "the existing oversight state
|
||||
* storage the controller uses").
|
||||
* - `evaluateNoOpFinalizeExecutorVeto` — the veto decision.
|
||||
*
|
||||
* This composes with, and is independent of, the merger-layer lineage-proof
|
||||
* guard (a sibling change): both can fire, and EITHER alone must stop FN-8141.
|
||||
*
|
||||
* Scope guards, by construction:
|
||||
* - Only a zero-diff (empty) merge is in scope. A NON-empty merge (a real
|
||||
* squash landed) is NEVER vetoed here — reviewers / merge review cover real
|
||||
* diffs; this guard is only for the completion-laundering shape.
|
||||
* - The guard DEFERS (never vetoes) whenever the FN-7514 human-control
|
||||
* predicate withholds oversight (user-paused, approval-blocked, or
|
||||
* `autoMerge:false` / PR-based human-review terminal contract) — it must not
|
||||
* fight user-paused / autoMerge:false semantics; a human owns those tasks.
|
||||
*/
|
||||
|
||||
import type { ExecutorOverseerSignalMemory, PlannerInterventionEntry, Settings, Task } from "@fusion/core";
|
||||
import { EXECUTOR_FAILED_INCOMPLETE_REASON } from "./planner-overseer.js";
|
||||
import {
|
||||
evaluateOverseerHumanControl,
|
||||
type OverseerHumanControlWithholdReason,
|
||||
} from "./overseer-human-control-policy.js";
|
||||
|
||||
/** Minimal task shape the veto needs — narrowed for testability + the human-control delegation. */
|
||||
export type NoOpFinalizeExecutorVetoTask = Pick<
|
||||
Task,
|
||||
"userPaused" | "paused" | "pausedReason" | "status" | "autoMerge" | "prInfo" | "prInfos"
|
||||
>;
|
||||
|
||||
export interface NoOpFinalizeExecutorVetoDecision {
|
||||
/** `true` when the empty no-op finalize must be blocked (task → todo, progress preserved). */
|
||||
veto: boolean;
|
||||
/**
|
||||
* Present only when `veto` is `true`. A CONSTANT string (no interpolated
|
||||
* timestamps/ids) so the run-audit dedup per (taskId, reason) — mirroring
|
||||
* `overseer:oversight-withheld-human-control` — is stable across polls.
|
||||
*/
|
||||
reason?: string;
|
||||
/**
|
||||
* `true` when the guard deferred to the FN-7514 human-control contract and
|
||||
* therefore did NOT veto (user-paused / approval-blocked / autoMerge-off).
|
||||
* Audit-only signal; `veto` is `false` in this case.
|
||||
*/
|
||||
deferredForHumanControl?: boolean;
|
||||
/** The human-control withhold reason, when `deferredForHumanControl` is `true`. */
|
||||
humanControlReason?: OverseerHumanControlWithholdReason;
|
||||
}
|
||||
|
||||
/** The constant veto reason — kept stable for (taskId, reason) audit dedup. */
|
||||
export const NO_OP_FINALIZE_EXECUTOR_VETO_REASON =
|
||||
"most recent executor-stage signal was failed-with-incomplete-work and no subsequent execution completed green";
|
||||
|
||||
/**
|
||||
* FNXC:Lifecycle 2026-07-16-09:40:
|
||||
* Pure derivation of the most-recent executor-stage overseer signal from the
|
||||
* durable `overseer:intervention` timeline (newest-first, as
|
||||
* `getPlannerInterventionTimeline` returns it). Considers ONLY passive
|
||||
* observations (`action === "observe"`) on the `executor` stage — steering/
|
||||
* retry/escalate entries also carry `stage: "executor"` but their `reason` is a
|
||||
* recovery message, not a signal. Returns `null` when there is no executor
|
||||
* observation to reason about. Never throws.
|
||||
*
|
||||
* `incompleteWork` is `true` iff the newest executor observation's reason is the
|
||||
* canonical `EXECUTOR_FAILED_INCOMPLETE_REASON`; any later observation
|
||||
* (progressing/stuck/blocked/...) supersedes it, which is how "no subsequent
|
||||
* execution completed green" is derived.
|
||||
*/
|
||||
export function deriveExecutorSignalMemory(
|
||||
entries: ReadonlyArray<PlannerInterventionEntry> | null | undefined,
|
||||
): ExecutorOverseerSignalMemory | null {
|
||||
if (!entries || entries.length === 0) {
|
||||
return null;
|
||||
}
|
||||
let newest: PlannerInterventionEntry | null = null;
|
||||
for (const entry of entries) {
|
||||
if (!entry || entry.stage !== "executor" || entry.action !== "observe") {
|
||||
continue;
|
||||
}
|
||||
if (newest === null || entry.timestamp > newest.timestamp) {
|
||||
newest = entry;
|
||||
}
|
||||
}
|
||||
if (!newest) {
|
||||
return null;
|
||||
}
|
||||
const incompleteWork = newest.reason === EXECUTOR_FAILED_INCOMPLETE_REASON;
|
||||
const observedAt = Date.parse(newest.timestamp);
|
||||
return {
|
||||
// The timeline does not carry the raw signal enum; map the one reason we
|
||||
// act on back to its signal and label everything else "progressing"
|
||||
// (any non-failed executor observation is, for veto purposes, "not
|
||||
// failed-with-incomplete-work").
|
||||
signal: incompleteWork ? "failed" : "progressing",
|
||||
incompleteWork,
|
||||
observedAt: Number.isFinite(observedAt) ? observedAt : 0,
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Pure predicate — no I/O, no throws on well-formed input. Decides whether an
|
||||
* EMPTY (zero net changes) merge finalize for `task` must be vetoed because the
|
||||
* overseer's cross-stage memory says the executor last parked
|
||||
* failed-with-incomplete-work and nothing completed green since.
|
||||
*
|
||||
* Precedence:
|
||||
* 1. `mergeIsEmpty === false` → never veto (real diff; reviewers cover it).
|
||||
* 2. Missing task → never veto (nothing to reason about; fail open here —
|
||||
* the FN-6461 guard and the sibling lineage guard remain the safety nets).
|
||||
* 3. FN-7514 human-control withholds → DEFER (no veto; a human owns the task).
|
||||
* 4. `memory.incompleteWork === true` → VETO.
|
||||
* 5. Otherwise → no veto.
|
||||
*/
|
||||
export function evaluateNoOpFinalizeExecutorVeto(input: {
|
||||
/** Whether the landed merge produced zero net changes vs the integration branch. */
|
||||
mergeIsEmpty: boolean;
|
||||
task: NoOpFinalizeExecutorVetoTask | null | undefined;
|
||||
/** Derived most-recent executor overseer signal (see `deriveExecutorSignalMemory`). */
|
||||
memory: ExecutorOverseerSignalMemory | null | undefined;
|
||||
/** Engine settings for the human-control `allowsAutoMergeProcessing` check; defaults to auto-merge-on. */
|
||||
settings?: Pick<Settings, "autoMerge"> | null;
|
||||
}): NoOpFinalizeExecutorVetoDecision {
|
||||
const { mergeIsEmpty, task, memory, settings } = input;
|
||||
|
||||
// (1) A real squash landing is out of scope — never vetoed here.
|
||||
if (!mergeIsEmpty) {
|
||||
return { veto: false };
|
||||
}
|
||||
|
||||
// (2) No task to reason about — fail open; other guards remain in force.
|
||||
if (!task) {
|
||||
return { veto: false };
|
||||
}
|
||||
|
||||
// (3) FN-7514 precedent: never fight user-paused / approval-blocked /
|
||||
// autoMerge:false-human-review. Defer to the human in the loop.
|
||||
const humanControl = evaluateOverseerHumanControl(task, settings ?? { autoMerge: true });
|
||||
if (humanControl.withhold) {
|
||||
return {
|
||||
veto: false,
|
||||
deferredForHumanControl: true,
|
||||
humanControlReason: humanControl.reason,
|
||||
};
|
||||
}
|
||||
|
||||
// (4) Cross-stage memory says the executor last parked
|
||||
// failed-with-incomplete-work and nothing progressed since.
|
||||
if (memory && memory.incompleteWork === true) {
|
||||
return { veto: true, reason: NO_OP_FINALIZE_EXECUTOR_VETO_REASON };
|
||||
}
|
||||
|
||||
// (5) Executor last seen healthy (or no memory) → allow the no-op finalize.
|
||||
return { veto: false };
|
||||
}
|
||||
@@ -30,6 +30,18 @@ export type OverseerWatchedStage = (typeof OVERSEER_WATCHED_STAGES)[number];
|
||||
/** Normalized signal describing how a watched stage is currently progressing. */
|
||||
export type OverseerObservationSignal = "progressing" | "stuck" | "failed" | "blocked" | "awaiting-human" | "complete";
|
||||
|
||||
/**
|
||||
* FNXC:Lifecycle 2026-07-16-09:40:
|
||||
* FN-8141: the CONSTANT reason string for the executor stage's
|
||||
* failed-with-incomplete-work observation. It is already load-bearing — the
|
||||
* FN-7577 feed dedup keys on `stage|signal|reason`, so this string must never
|
||||
* embed per-failure detail (see the derivation at `deriveSignalAndSources`).
|
||||
* Exported as the single source of truth so the cross-stage no-op-finalize veto
|
||||
* derivation (`deriveExecutorSignalMemory`) can recognize this observation in
|
||||
* the durable `overseer:intervention` timeline without duplicating the literal.
|
||||
*/
|
||||
export const EXECUTOR_FAILED_INCOMPLETE_REASON = "Executor stage parked failed with work incomplete";
|
||||
|
||||
/** A link back to the concrete evidence an observation was derived from. */
|
||||
export interface OverseerSourceLink {
|
||||
kind: "agent-log" | "review-comment" | "failed-check" | "merge-error" | "pr-state";
|
||||
@@ -191,7 +203,7 @@ function deriveSignalAndSources(
|
||||
if (task.status === "failed") {
|
||||
return {
|
||||
signal: "failed",
|
||||
reason: "Executor stage parked failed with work incomplete",
|
||||
reason: EXECUTOR_FAILED_INCOMPLETE_REASON,
|
||||
sources: [{ kind: "agent-log", ref: taskId }],
|
||||
};
|
||||
}
|
||||
|
||||
@@ -817,7 +817,23 @@ export type DatabaseMutationType =
|
||||
* Emitted at most once per taskId while the blocking provenance persists (deduped in-memory).
|
||||
* Metadata: { taskId, reason: "failure-provenance", sweep: "stuck-in-progress" | "stranded-todo", marker?: string }
|
||||
*/
|
||||
| "task:reconcile-stranded-completed-no-action";
|
||||
| "task:reconcile-stranded-completed-no-action"
|
||||
/**
|
||||
* FNXC:Lifecycle 2026-07-16-09:40:
|
||||
* FN-8141 no-action lifecycle event: the AI empty-merge lane vetoed a
|
||||
* zero-diff (no net changes) no-op finalize because the task's cross-stage
|
||||
* overseer memory (derived from the durable `overseer:intervention` timeline)
|
||||
* shows the MOST RECENT executor-stage signal was failed-with-incomplete-work
|
||||
* with no subsequent green completion (`evaluateNoOpFinalizeExecutorVeto`).
|
||||
* The task is moved back to `todo` with progress preserved instead of reaching
|
||||
* `done` — mirroring the FN-6461 `task:no-commits-finalize-blocked-incomplete-steps`
|
||||
* blocked lane. The move-to-todo transition takes the task out of the merge
|
||||
* lane, so the event is not re-emitted every poll (equivalent to the
|
||||
* `overseer:oversight-withheld-human-control` per-(taskId, reason) dedup).
|
||||
* Metadata (ids/outcomes-only): { reason; branch; integrationBranch; lane:
|
||||
* "ai-empty-merge"; executorSignal?; executorSignalObservedAt? }
|
||||
*/
|
||||
| "overseer:no-op-finalize-vetoed-failed-executor";
|
||||
|
||||
// ── Filesystem mutation types ─────────────────────────────────────────────────
|
||||
|
||||
|
||||
Reference in New Issue
Block a user