diff --git a/.changeset/veto-progressing-does-not-clear.md b/.changeset/veto-progressing-does-not-clear.md new file mode 100644 index 0000000000..7d2f85798a --- /dev/null +++ b/.changeset/veto-progressing-does-not-clear.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: A task actively re-executing can no longer launder an empty reverted branch into done. +category: fix +dev: FN-8141 follow-up 3. `deriveExecutorSignalMemory` (packages/engine/src/overseer-noop-finalize-veto.ts) no longer lets a mid-execution `progressing` overseer observation clear the no-op-finalize veto. A failure park is superseded only by a clean-completion task-log marker (shared `CLEAN_COMPLETION_MARKERS` exported from @fusion/core) strictly newer than it; the executor stage emits no green-completion observation. `merger-ai.ts` threads `task.log` into the derivation. diff --git a/packages/core/src/completed-promotion-failure-provenance.ts b/packages/core/src/completed-promotion-failure-provenance.ts index e79de5c1c5..d84a2399f0 100644 --- a/packages/core/src/completed-promotion-failure-provenance.ts +++ b/packages/core/src/completed-promotion-failure-provenance.ts @@ -71,7 +71,18 @@ const FAILURE_PARK_MARKERS = [ * - "All steps complete — implicit fn_task_done" (executor.ts ~12095/~12402 — implicit-completion * success when all steps are done without an explicit tool call) */ -const CLEAN_COMPLETION_MARKERS = [ +/* + * FNXC:Lifecycle 2026-07-16-12:10: + * Exported so the FN-8141 follow-up overseer no-op-finalize veto + * (`deriveExecutorSignalMemory` in packages/engine/src/overseer-noop-finalize-veto.ts) + * consumes the SAME accepted-completion marker set instead of duplicating the + * strings. The executor-stage overseer timeline emits no "completed green" + * observation (only progressing/failed/stuck/blocked), so these durable task-log + * markers are the only evidence that can supersede a failure park. Importing the + * shared list means any change here (e.g. removing the stranded-completion + * promotion-output marker) is picked up by the veto automatically — no drift. + */ +export const CLEAN_COMPLETION_MARKERS = [ "Task marked done by agent", "All steps complete — implicit fn_task_done", ]; diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 1c15f8eedb..c67a2e4f55 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -723,7 +723,7 @@ export { } from "./no-op-completion-marker.js"; export { evaluateNoCommitsNoOpFinalize } from "./no-commits-finalize-guard.js"; export type { NoCommitsNoOpFinalizeEvaluation } from "./no-commits-finalize-guard.js"; -export { evaluateCompletedPromotionFailureProvenance } from "./completed-promotion-failure-provenance.js"; +export { evaluateCompletedPromotionFailureProvenance, CLEAN_COMPLETION_MARKERS } from "./completed-promotion-failure-provenance.js"; export type { CompletedPromotionFailureProvenanceEvaluation } from "./completed-promotion-failure-provenance.js"; export { evaluateSkipBypassTaint } from "./skip-bypass-taint-guard.js"; export type { SkipBypassTaintEvaluation } from "./skip-bypass-taint-guard.js"; diff --git a/packages/engine/src/__tests__/merger-ai.test.ts b/packages/engine/src/__tests__/merger-ai.test.ts index 959c4fc692..0a4767903e 100644 --- a/packages/engine/src/__tests__/merger-ai.test.ts +++ b/packages/engine/src/__tests__/merger-ai.test.ts @@ -763,14 +763,22 @@ describe("runAiMerge", () => { 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 () => { + /* + * FN-8141 follow-up 3 regression: a mid-execution `progressing` observation + * newer than the failure park must NOT clear the veto (the overseer emits + * `progressing` the instant a task re-enters execution, long before it + * finishes). The empty no-op finalize is still blocked to todo. + */ + it("FN-8141 follow-up 3: STILL vetoes when a later executor observation was only `progressing` (no completion)", 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. + const mainBefore = git(dir, "rev-parse main"); + // Timeline newest-first: progressing sits AFTER the failure park but is not + // "completed green" — it must not supersede the failure. store.getRunAuditEventsAsync = vi.fn(async () => [ { - id: "ev-green", taskId: "FN-1", target: "FN-1", timestamp: "2026-07-16T23:10:00.000Z", + id: "ev-progressing", 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" }, }, @@ -786,6 +794,48 @@ describe("runAiMerge", () => { 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).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(git(dir, "rev-parse main")).toBe(mainBefore); + }); + + /* + * The escape hatch stays intact: a GENUINELY re-executed green task (a + * clean-completion task-log marker NEWER than the failure park) is not vetoed + * and finalizes to done. + */ + it("FN-8141 follow-up 3: does NOT veto when a clean-completion task-log marker is newer than the failure park", 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" }], + log: [ + { action: "Executor stage parked failed with work incomplete", timestamp: "2026-07-16T22:40:00.000Z" }, + { action: "Task marked done by agent", timestamp: "2026-07-16T23:30:00.000Z" }, + ], + }); + store.getRunAuditEventsAsync = vi.fn(async () => [ + { + 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 })); diff --git a/packages/engine/src/__tests__/overseer-noop-finalize-veto.test.ts b/packages/engine/src/__tests__/overseer-noop-finalize-veto.test.ts index 53f0aad4e5..c39e52b792 100644 --- a/packages/engine/src/__tests__/overseer-noop-finalize-veto.test.ts +++ b/packages/engine/src/__tests__/overseer-noop-finalize-veto.test.ts @@ -33,6 +33,9 @@ function entry(overrides: Partial): PlannerInterventio const failedEntry = (overrides: Partial = {}) => entry({ reason: EXECUTOR_FAILED_INCOMPLETE_REASON, ...overrides }); +/** A durable clean-completion task-log marker at `ts` (see @fusion/core CLEAN_COMPLETION_MARKERS). */ +const completionLog = (ts: string) => [{ action: "Task marked done by agent", timestamp: ts }]; + const okTask: NoOpFinalizeExecutorVetoTask = { userPaused: false, paused: false, @@ -56,17 +59,55 @@ describe("deriveExecutorSignalMemory", () => { 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)", () => { + // THE follow-up-3 regression: a mid-execution `progressing` observation is NOT + // "completed green" and must NOT clear the failure park's veto. + it("a later `progressing` executor observation does NOT supersede an earlier failed one (still vetoed)", () => { // 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" }), + entry({ id: "progressing", 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(true); + expect(memory?.signal).toBe("failed"); + }); + + // The FN-8141 shape itself: progressing observations existed BETWEEN two failed + // parks; the newest relevant executor signal is failed → stays vetoed. + it("keeps the veto when progressing sits between two failed parks (FN-8141 timeline)", () => { + const memory = deriveExecutorSignalMemory([ + failedEntry({ id: "fail-2", timestamp: "2026-07-16T23:20:00.000Z" }), + entry({ id: "progressing", timestamp: "2026-07-16T23:00:00.000Z", reason: "Task is actively executing in-progress work" }), + failedEntry({ id: "fail-1", timestamp: "2026-07-16T22:40:00.000Z" }), + ]); + expect(memory?.incompleteWork).toBe(true); + }); + + it("a clean-completion task-log marker NEWER than the failure park supersedes it (not vetoed)", () => { + const memory = deriveExecutorSignalMemory( + [failedEntry({ id: "fail", timestamp: "2026-07-16T22:40:00.000Z" })], + completionLog("2026-07-16T23:10:00.000Z"), + ); + expect(memory?.incompleteWork).toBe(false); + expect(memory?.signal).toBe("progressing"); + }); + + it("a clean-completion task-log marker OLDER than the failure park does NOT supersede it (still vetoed)", () => { + const memory = deriveExecutorSignalMemory( + [failedEntry({ id: "fail", timestamp: "2026-07-16T22:40:00.000Z" })], + completionLog("2026-07-16T22:00:00.000Z"), + ); + expect(memory?.incompleteWork).toBe(true); + }); + + it("returns incompleteWork:false when there is no failure park at all", () => { + const memory = deriveExecutorSignalMemory([ + entry({ id: "progressing", timestamp: "2026-07-16T23:10:00.000Z", reason: "Task is actively executing in-progress work" }), + ]); expect(memory?.incompleteWork).toBe(false); expect(memory?.signal).toBe("progressing"); }); - it("ignores non-executor stages and non-observe actions when picking the newest signal", () => { + it("ignores non-executor stages and non-observe actions when locating the failure park", () => { 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" }), @@ -76,6 +117,23 @@ describe("deriveExecutorSignalMemory", () => { ]); expect(memory?.incompleteWork).toBe(true); }); + + // A completion marker beyond the bounded tail window must not be scanned in — + // preserves the cheap tail-only scan contract. + it("only scans the bounded task-log tail for completion markers", () => { + const padding = Array.from({ length: 300 }, (_, i) => ({ + action: "unrelated log line", + timestamp: `2026-07-16T2${(i % 3)}:00:00.000Z`, + })); + // Completion marker is at the HEAD (older than the 250-entry tail window). + const log = [{ action: "Task marked done by agent", timestamp: "2026-07-16T23:59:00.000Z" }, ...padding]; + const memory = deriveExecutorSignalMemory( + [failedEntry({ id: "fail", timestamp: "2026-07-16T22:40:00.000Z" })], + log, + ); + // The out-of-window completion marker is NOT seen → failure park still stands. + expect(memory?.incompleteWork).toBe(true); + }); }); describe("evaluateNoOpFinalizeExecutorVeto", () => { diff --git a/packages/engine/src/merger-ai.ts b/packages/engine/src/merger-ai.ts index 65268bd0c4..9006837982 100644 --- a/packages/engine/src/merger-ai.ts +++ b/packages/engine/src/merger-ai.ts @@ -1298,7 +1298,10 @@ export async function runAiMerge( let executorMemory = null as Awaited>; try { const timeline = await getPlannerInterventionTimeline(store, taskId); - executorMemory = deriveExecutorSignalMemory(timeline); + // FNXC:Lifecycle 2026-07-16-12:10 (follow-up 3): thread the durable task log + // so a mid-execution `progressing` observation cannot clear the veto — only a + // clean-completion marker newer than the failure park supersedes it. + executorMemory = deriveExecutorSignalMemory(timeline, task.log); } catch (err) { aiMergeLog.warn(`${taskId}: executor overseer-memory derivation failed (skipping veto): ${getErrorMessage(err)}`); } diff --git a/packages/engine/src/overseer-noop-finalize-veto.ts b/packages/engine/src/overseer-noop-finalize-veto.ts index 2ce9d6b235..2e848f09ab 100644 --- a/packages/engine/src/overseer-noop-finalize-veto.ts +++ b/packages/engine/src/overseer-noop-finalize-veto.ts @@ -22,12 +22,22 @@ * 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"). + * - `deriveExecutorSignalMemory` — reconstructs the executor signal from the + * durable `overseer:intervention` timeline the overseer already writes (no + * new persisted column; "the existing oversight state storage the controller + * uses") PLUS the durable task log for completion supersession. * - `evaluateNoOpFinalizeExecutorVeto` — the veto decision. * + * FNXC:Lifecycle 2026-07-16-12:10 (follow-up 3): + * A mid-execution `progressing` observation must NOT clear the veto. The overseer + * emits `progressing` ("Task is actively executing in-progress work") the moment + * a task re-enters execution, long before it finishes — so failed-incomplete → + * requeue → re-execution starts → dies/reverts again (no newer failed + * observation) once left `progressing` as the newest signal and defeated the + * veto. A failure park is now superseded ONLY by a clean-completion task-log + * marker STRICTLY newer than it (the executor stage emits no green-completion + * observation), never by an in-flight progressing/stuck/blocked signal. + * * 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. * @@ -41,7 +51,8 @@ * fight user-paused / autoMerge:false semantics; a human owns those tasks. */ -import type { ExecutorOverseerSignalMemory, PlannerInterventionEntry, Settings, Task } from "@fusion/core"; +import type { ExecutorOverseerSignalMemory, PlannerInterventionEntry, Settings, Task, TaskLogEntry } from "@fusion/core"; +import { CLEAN_COMPLETION_MARKERS } from "@fusion/core"; import { EXECUTOR_FAILED_INCOMPLETE_REASON } from "./planner-overseer.js"; import { evaluateOverseerHumanControl, @@ -77,50 +88,115 @@ export interface NoOpFinalizeExecutorVetoDecision { export const NO_OP_FINALIZE_EXECUTOR_VETO_REASON = "most recent executor-stage signal was failed-with-incomplete-work and no subsequent execution completed green"; +/** Minimal task-log shape the supersession check needs — narrowed for testability. */ +export type ExecutorSignalMemoryLogEntry = Pick; + +/** Bound the task-log tail scan; the merger calls this per empty-lane finalize. */ +const MAX_LOG_SCAN = 250; + /** - * 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. + * FNXC:Lifecycle 2026-07-16-12:10: + * Newest (most-recent-timestamp) durable clean-completion marker in the task log, + * as epoch-ms, or `null` when none is present / parseable. Scans the tail only + * (log is append-ordered) and reuses the SHARED `CLEAN_COMPLETION_MARKERS` set + * from `evaluateCompletedPromotionFailureProvenance` so the accepted-completion + * vocabulary stays single-sourced (no string drift; picks up sibling edits to + * that list automatically). Pure; never throws. + */ +function newestCleanCompletionMarkerMs( + taskLog: ReadonlyArray | null | undefined, +): number | null { + if (!taskLog || taskLog.length === 0) { + return null; + } + const scanFloor = Math.max(0, taskLog.length - MAX_LOG_SCAN); + let newestMs: number | null = null; + for (let i = taskLog.length - 1; i >= scanFloor; i--) { + const action = taskLog[i]?.action ?? ""; + if (!CLEAN_COMPLETION_MARKERS.some((marker) => action.includes(marker))) { + continue; + } + const ms = Date.parse(taskLog[i]?.timestamp ?? ""); + if (Number.isFinite(ms) && (newestMs === null || ms > newestMs)) { + newestMs = ms; + } + } + return newestMs; +} + +/** + * FNXC:Lifecycle 2026-07-16-12:10: + * Pure derivation of the executor-stage overseer signal memory from the durable + * `overseer:intervention` timeline PLUS the durable task log. 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. + * TIGHTENED (FN-8141 follow-up 3): the earlier version took the NEWEST executor + * observation and cleared `incompleteWork` whenever it was anything but the + * canonical failed reason. But the overseer emits a `progressing` observation + * ("Task is actively executing in-progress work") the moment a task re-enters + * execution — long before that execution finishes. So a shape of + * failed-incomplete → requeue → re-execution starts (progressing observed) → + * execution dies/reverts again with NO newer failed observation left the newest + * observation as `progressing` and DEFEATED the veto, laundering an empty no-op + * finalize to `done`. `progressing` is not "completed green". + * + * New rule — a failure park is superseded ONLY by genuine completion-family + * evidence NEWER than it, never by an in-flight `progressing`/`stuck`/`blocked` + * signal: + * 1. Find the newest executor `observe` whose reason is + * `EXECUTOR_FAILED_INCOMPLETE_REASON` (the failure park). No failure park at + * all ⇒ `incompleteWork: false`. + * 2. `incompleteWork` stays TRUE unless a clean-completion marker in the task + * log is STRICTLY NEWER than that failure park. The executor stage emits no + * "completed green" observation (planner-overseer.ts writes only + * progressing/failed/stuck/blocked for `executor`), so the durable task-log + * `CLEAN_COMPLETION_MARKERS` are the sole supersession evidence. + * 3. A malformed/unparseable failure timestamp fails SAFE (cannot prove a + * completion is newer ⇒ stays vetoed). */ export function deriveExecutorSignalMemory( entries: ReadonlyArray | null | undefined, + taskLog?: ReadonlyArray | null, ): ExecutorOverseerSignalMemory | null { - if (!entries || entries.length === 0) { - return null; - } - let newest: PlannerInterventionEntry | null = null; - for (const entry of entries) { + let newestObs: PlannerInterventionEntry | null = null; + let newestFailed: 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 (newestObs === null || entry.timestamp > newestObs.timestamp) { + newestObs = entry; + } + if (entry.reason === EXECUTOR_FAILED_INCOMPLETE_REASON) { + if (newestFailed === null || entry.timestamp > newestFailed.timestamp) { + newestFailed = entry; + } } } - if (!newest) { + // No executor observation at all → no memory to reason about. + if (!newestObs) { 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, - }; + // No failure park in the timeline → nothing to veto; the executor never + // parked failed-with-incomplete-work. + if (!newestFailed) { + const observedAt = Date.parse(newestObs.timestamp); + return { signal: "progressing", incompleteWork: false, observedAt: Number.isFinite(observedAt) ? observedAt : 0 }; + } + + const failedAtMs = Date.parse(newestFailed.timestamp); + const completionAtMs = newestCleanCompletionMarkerMs(taskLog); + // Supersession requires a clean completion STRICTLY newer than the failure + // park. A NaN failure timestamp cannot be proven older than any completion, so + // it fails safe (stays vetoed). + const superseded = completionAtMs !== null && Number.isFinite(failedAtMs) && completionAtMs > failedAtMs; + if (superseded) { + return { signal: "progressing", incompleteWork: false, observedAt: completionAtMs }; + } + return { signal: "failed", incompleteWork: true, observedAt: Number.isFinite(failedAtMs) ? failedAtMs : 0 }; } /**