From 14b7244bcb2981c906e783ca65d08431561de06e Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Sat, 11 Jul 2026 21:33:37 -0700 Subject: [PATCH] FN-7840: suppress advisory merger/pull-request await-confirmation interventions Stops decidePlannerRecovery from recording noisy advisory confirmation interventions for merger/pull-request stages that never actually block progress when auto-merge will proceed unattended. - decidePlannerRecovery now returns action "none" (no pending confirmation, no steering comment, no overseer:intervention entry) for merger/pull-request stages when autoMergeWillProceed === true, since this checkpoint is purely advisory in that case - Genuine human-approval blocks (autoMergeWillProceed === false) and the neutral pure-function default (undefined) keep the await_confirmation decision intact - Updated planner-recovery.test.ts to assert the new "none" outcome for the advisory case - Simplified planner-overseer-intervention-wiring.test.ts to match the reduced intervention surface - Added changeset documenting the fix as a patch-level bug fix Files changed: .changeset/fn-7840-advisory-merger-confirmations.md | 7 ++ packages/core/src/__tests__/planner-recovery.test.ts | 32 ++--- packages/core/src/planner-recovery.ts | 47 ++++--- packages/engine/src/__tests__/planner-overseer-intervention-wiring.test.ts | 135 +++++---------------- 4 files changed, 79 insertions(+), 142 deletions(-) Fusion-Task-Id: FN-7840 Fusion-Task-Lineage: 610a9003-f229-4e78-9948-ee0bb85193bc Co-authored-by: Fusion (runfusion.ai) --- .../fn-7840-advisory-merger-confirmations.md | 7 + .../src/__tests__/planner-recovery.test.ts | 32 +++-- packages/core/src/planner-recovery.ts | 47 ++++--- ...anner-overseer-intervention-wiring.test.ts | 133 ++++-------------- 4 files changed, 78 insertions(+), 141 deletions(-) create mode 100644 .changeset/fn-7840-advisory-merger-confirmations.md diff --git a/.changeset/fn-7840-advisory-merger-confirmations.md b/.changeset/fn-7840-advisory-merger-confirmations.md new file mode 100644 index 0000000000..1efa6d29bd --- /dev/null +++ b/.changeset/fn-7840-advisory-merger-confirmations.md @@ -0,0 +1,7 @@ +--- +"@runfusion/fusion": patch +--- + +summary: Stop recording advisory "merger awaiting-confirmation" planner interventions that never block auto-merge. +category: fix +dev: decidePlannerRecovery now returns action "none" for merger/pull-request stages when autoMergeWillProceed === true; the genuine human-approval (false) and neutral (undefined) confirmation paths are unchanged (FN-7840). diff --git a/packages/core/src/__tests__/planner-recovery.test.ts b/packages/core/src/__tests__/planner-recovery.test.ts index 607cba45d5..0dd051df90 100644 --- a/packages/core/src/__tests__/planner-recovery.test.ts +++ b/packages/core/src/__tests__/planner-recovery.test.ts @@ -103,26 +103,22 @@ describe("decidePlannerRecovery", () => { } }); - // FN-7692: the `reason` copy must accurately reflect whether the active - // auto-merge policy will advance the merge/PR unattended (advisory, - // non-blocking wording) or genuinely requires a human approval (blocking - // wording) — never the old unconditional "requires explicit confirmation - // before ... may run" claim, which was false whenever auto-merge would - // proceed on its own (observed on FN-7689). `action`/`requiresConfirmation`/ - // `sideEffectClass` must stay byte-for-byte identical across all three - // `autoMergeWillProceed` states — this is messaging-only. - it("produces accurate advisory copy when auto-merge will proceed unattended, for both merger and pull-request stages", () => { + // FNXC:PlannerOversight 2026-07-11-00:00: + // FN-7840 changes `autoMergeWillProceed: true` from a messaging-only advisory checkpoint into a true suppression path: no await_confirmation decision, no pending confirmation, no steering comment, and no intervention entry. The false/undefined paths remain the safety valve. + it("suppresses the merger/pull-request confirmation when auto-merge will proceed unattended", () => { for (const stage of ["merger", "pull-request"] as const) { const decision = decidePlannerRecovery({ snapshot: observation({ stage, signal: "failed" }), autoMergeWillProceed: true, }); - expect(decision.action, `stage=${stage}`).toBe("await_confirmation"); - expect(decision.requiresConfirmation, `stage=${stage}`).toBe(true); + expect(decision.action, `stage=${stage}`).toBe("none"); + expect(decision.requiresConfirmation, `stage=${stage}`).toBe(false); expect(decision.sideEffectClass, `stage=${stage}`).toBe("merge_pr"); + expect(decision.proposedAction, `stage=${stage}`).toBeUndefined(); expect(decision.reason, `stage=${stage}`).not.toMatch(/requires explicit confirmation before .* may run/); + expect(decision.reason, `stage=${stage}`).not.toMatch(/await|advisory|does not block progress/i); expect(decision.reason, `stage=${stage}`).toMatch(/automatically/i); - expect(decision.reason, `stage=${stage}`).toMatch(/advisory/i); + expect(decision.reason, `stage=${stage}`).toMatch(/no confirmation checkpoint recorded/i); } }); @@ -143,13 +139,15 @@ describe("decidePlannerRecovery", () => { it("uses neutral, non-overclaiming copy when the auto-merge policy is unknown to the caller", () => { for (const stage of ["merger", "pull-request"] as const) { const decision = decidePlannerRecovery({ snapshot: observation({ stage, signal: "failed" }) }); + expect(decision.action, `stage=${stage}`).toBe("await_confirmation"); + expect(decision.requiresConfirmation, `stage=${stage}`).toBe(true); expect(decision.reason, `stage=${stage}`).not.toMatch(/requires explicit confirmation before .* may run/); expect(decision.reason, `stage=${stage}`).not.toMatch(/automatically/i); expect(decision.reason, `stage=${stage}`).not.toMatch(/will not .* until a human explicitly approves/); } }); - it("keeps action/requiresConfirmation/sideEffectClass byte-for-byte unchanged across autoMergeWillProceed states (messaging-only guard)", () => { + it("diverges only the advisory autoMergeWillProceed=true case while preserving blocking and neutral confirmations", () => { for (const stage of ["merger", "pull-request"] as const) { const base = decidePlannerRecovery({ snapshot: observation({ stage, signal: "failed" }) }); const proceeds = decidePlannerRecovery({ @@ -160,12 +158,18 @@ describe("decidePlannerRecovery", () => { snapshot: observation({ stage, signal: "failed" }), autoMergeWillProceed: false, }); - for (const decision of [base, proceeds, blocks]) { + + // FNXC:PlannerOversight 2026-07-11-00:00: FN-7840 intentionally breaks the old messaging-only invariant only for autoMergeWillProceed === true; false and undefined keep the confirmation safety valve. + for (const decision of [base, blocks]) { expect(decision.action, `stage=${stage}`).toBe("await_confirmation"); expect(decision.requiresConfirmation, `stage=${stage}`).toBe(true); expect(decision.sideEffectClass, `stage=${stage}`).toBe("merge_pr"); expect(decision.proposedAction, `stage=${stage}`).toBe(base.proposedAction); } + expect(proceeds.action, `stage=${stage}`).toBe("none"); + expect(proceeds.requiresConfirmation, `stage=${stage}`).toBe(false); + expect(proceeds.sideEffectClass, `stage=${stage}`).toBe("merge_pr"); + expect(proceeds.proposedAction, `stage=${stage}`).toBeUndefined(); } }); diff --git a/packages/core/src/planner-recovery.ts b/packages/core/src/planner-recovery.ts index 304da7e664..622828f2dd 100644 --- a/packages/core/src/planner-recovery.ts +++ b/packages/core/src/planner-recovery.ts @@ -223,30 +223,37 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne }; } - // FNXC:PlannerOversight 2026-07-04-13:00: merger / pull-request stage - // actions beyond guidance/retry now surface as a confirmation-required - // `"await_confirmation"` decision (FN-7513) instead of the FN-7512 - // `"none"` deferral — the recovery layer identifies what WOULD run on - // approval, but never dispatches it itself. + /* + FNXC:PlannerOversight 2026-07-04-13:00: + Merger / pull-request stage actions beyond guidance/retry surface as a confirmation-required `"await_confirmation"` decision (FN-7513) instead of the FN-7512 `"none"` deferral — the recovery layer identifies what WOULD run on approval, but never dispatches it itself. + + FNXC:PlannerOversight 2026-07-08-00:00: + FN-7692 fix: the merger/pull-request `reason` string previously read "requires explicit confirmation before ... may run" UNCONDITIONALLY, which is false when the active auto-merge policy will advance the merge/PR unattended (observed on FN-7689: the Intervention Timeline claimed a hard block that the merge sailed past ~4 minutes later with no human approval). `input.autoMergeWillProceed` (threaded by the engine controller from the existing `allowsAutoMergeProcessing` predicate) selects accurate wording per policy state. + + FNXC:PlannerOversight 2026-07-11-00:00: + FN-7840: the merger/pull-request confirmation checkpoint is purely advisory whenever the active auto-merge policy will advance the merge unattended (autoMergeWillProceed === true) — in real tick() wiring this is the ONLY reachable state, because evaluateOverseerHumanControl withholds all oversight when allowsAutoMergeProcessing === false (the same predicate). Recording it produced a stream of "advisory and does not block progress" interventions the operator saw as pure noise. Suppress it: return action "none" (no pending confirmation, no steering comment, no overseer:intervention entry) for the advisory case. Genuine human-approval blocks (autoMergeWillProceed === false) and the neutral pure-function default (undefined) keep await_confirmation intact. + */ if (snapshot.stage === "merger" || snapshot.stage === "pull-request") { const proposedAction = snapshot.stage === "merger" ? "advance_merge" : "advance_pull_request"; const sideEffectClass = classifyPlannerActionSideEffect({ watchedStage: snapshot.stage, proposedAction }); const actionPhrase = proposedAction.replace(/_/g, " "); - // FNXC:PlannerOversight 2026-07-08-00:00: - // FN-7692 fix: this `reason` string previously read "requires explicit - // confirmation before ... may run" UNCONDITIONALLY, which is false when - // the active auto-merge policy will advance the merge/PR unattended - // (observed on FN-7689: the Intervention Timeline claimed a hard block - // that the merge sailed past ~4 minutes later with no human approval). - // `input.autoMergeWillProceed` (threaded by the engine controller from - // the existing `allowsAutoMergeProcessing` predicate) selects accurate - // wording per policy state; it is messaging-only and does not change - // `action`/`requiresConfirmation`/`sideEffectClass` below. - const reason = input.autoMergeWillProceed === true - ? `Stage "${snapshot.stage}" will ${actionPhrase} automatically under the active auto-merge policy — this confirmation checkpoint is advisory and does not block progress` - : input.autoMergeWillProceed === false - ? `Stage "${snapshot.stage}" will not ${actionPhrase} until a human explicitly approves — auto-merge is not enabled for this task` - : `Stage "${snapshot.stage}" is awaiting confirmation before ${actionPhrase} may run`; + if (input.autoMergeWillProceed === true) { + return { + action: "none", + reason: `Stage "${snapshot.stage}" will ${actionPhrase} automatically under the active auto-merge policy — no confirmation checkpoint recorded`, + attemptCount, + attemptLimit, + exhausted: false, + watchedStage, + sourceLinks, + requiresConfirmation: false, + sideEffectClass, + proposedAction: undefined, + }; + } + const reason = input.autoMergeWillProceed === false + ? `Stage "${snapshot.stage}" will not ${actionPhrase} until a human explicitly approves — auto-merge is not enabled for this task` + : `Stage "${snapshot.stage}" is awaiting confirmation before ${actionPhrase} may run`; return { action: "await_confirmation", reason, diff --git a/packages/engine/src/__tests__/planner-overseer-intervention-wiring.test.ts b/packages/engine/src/__tests__/planner-overseer-intervention-wiring.test.ts index 5f782b1e1c..33444f2fa2 100644 --- a/packages/engine/src/__tests__/planner-overseer-intervention-wiring.test.ts +++ b/packages/engine/src/__tests__/planner-overseer-intervention-wiring.test.ts @@ -1,9 +1,9 @@ /** * FNXC:PlannerOversight 2026-07-04-19:45: * FN-7551 engine-level end-to-end test: proves real overseer decision points - * — observation, retry, targeted-fix, steering (reviewer), confirmation - * request, confirmation resolution, and bounded-recovery escalation — - * populate the `overseer:intervention` run-audit timeline via the ACTUAL + * — observation, retry, targeted-fix, steering (reviewer), advisory + * confirmation suppression, and bounded-recovery escalation — populate the + * `overseer:intervention` run-audit timeline via the ACTUAL * production wiring in `project-engine.ts` (`PlannerOverseerMonitor#onObservation` * → the private `emitOverseerObservationDeduped`, the private * `buildPlannerRecoveryHandlers`, the private `emitOverseerEscalationDeduped`), @@ -18,6 +18,9 @@ * initialized by class-field initializers the constructor never runs here) * — this exercises the exact same code that runs inside * `pollPlannerOverseer`/`start()` in production, not a reimplementation. + * + * FNXC:PlannerOversight 2026-07-11-00:00: + * FN-7840 removes the only real-tick producer of advisory merger awaiting-confirmation interventions, so this live-wiring harness now proves suppression at the source rather than request/resolution emission for an unreachable advisory pending-confirmation path. */ import { afterEach, beforeEach, describe, expect, it } from "vitest"; @@ -200,81 +203,36 @@ describe("FN-7551 — overseer decision points populate the intervention timelin expect(steeringEntry?.stage).toBe("reviewer"); }); - it("merger/pull-request confirmation-required decision emits a request-confirmation entry; approving it emits a resolution entry with 'succeeded'", async () => { + // FNXC:PlannerOversight 2026-07-11-00:00: + // FN-7840 regression coverage: real tick() wiring reaches merger/pull-request oversight only when the same `allowsAutoMergeProcessing` predicate says auto-merge will proceed unattended. That advisory state must be silent — no pending confirmation, no request-confirmation intervention, and no merge-checkpoint steering comment/badge source. + it("suppresses advisory merger confirmations under active auto-merge policy", async () => { const task = await seedTask("in-review"); const { monitor, controllerFromMonitor: controller } = wireRealEngineOverseer(store); await monitor.observeTask(task, "autonomous"); // merger stage (plain in-review, no PR/reviewState) - const decision = await controller.tick(task); - expect(decision?.requiresConfirmation).toBe(true); + const firstDecision = await controller.tick(task, { settings: { autoMerge: true } }); + expect(firstDecision?.action).toBe("none"); + expect(firstDecision?.requiresConfirmation).toBe(false); + expect(firstDecision?.reason).toMatch(/automatically/i); - let timeline = getPlannerInterventionTimeline(store, task.id); - const requestEntry = timeline.find((e) => e.action === "request-confirmation"); - expect(requestEntry).toBeTruthy(); - expect(requestEntry?.outcome).toBe("awaiting-confirmation"); - - const pending = controller.getPendingConfirmations(task.id); - expect(pending).toHaveLength(1); - - await controller.resolveConfirmation(task.id, pending[0].requestId, "approved", "test-user"); - - timeline = getPlannerInterventionTimeline(store, task.id); - const confirmationEntries = timeline.filter((e) => e.action === "request-confirmation"); - expect(confirmationEntries.length).toBeGreaterThanOrEqual(2); - expect(confirmationEntries.some((e) => e.outcome === "succeeded")).toBe(true); - }); - - // FN-7692: the recorded `overseer:intervention` reason for a merger/ - // pull-request confirmation must accurately reflect whether auto-merge will - // proceed unattended (advisory copy) or genuinely requires a human approval - // (blocking copy) — reproducing the FN-7689 scenario where the timeline - // claimed a hard block that the merge sailed past unattended. A pending - // confirmation must still be recorded either way (no dispatch change). - it("records accurate advisory copy (not a false hard-block claim) when ctx.settings.autoMerge is truthy for an in-review merger task", async () => { - const task = await seedTask("in-review"); - const { monitor, controllerFromMonitor: controller } = wireRealEngineOverseer(store); - await monitor.observeTask(task, "autonomous"); // merger stage (plain in-review, no PR/reviewState) - - const decision = await controller.tick(task, { settings: { autoMerge: true } }); - expect(decision?.requiresConfirmation).toBe(true); - expect(decision?.action).toBe("await_confirmation"); + const secondDecision = await controller.tick(task, { settings: { autoMerge: true } }); + expect(secondDecision?.action).toBe("none"); + expect(secondDecision?.requiresConfirmation).toBe(false); const timeline = getPlannerInterventionTimeline(store, task.id); - const requestEntry = timeline.find((e) => e.action === "request-confirmation"); - expect(requestEntry).toBeTruthy(); - expect(requestEntry?.outcome).toBe("awaiting-confirmation"); - expect(requestEntry?.reason).not.toMatch(/requires explicit confirmation before .* may run/); - expect(requestEntry?.reason).toMatch(/automatically/i); + expect(timeline.filter((e) => e.action === "request-confirmation")).toHaveLength(0); + expect(controller.getPendingConfirmations(task.id)).toHaveLength(0); - // A pending confirmation is still recorded — no dispatch, no behavior change. - expect(controller.getPendingConfirmations(task.id)).toHaveLength(1); + const refreshedTask = await store.getTask(task.id); + const allCommentText = [ + ...(refreshedTask?.comments ?? []).map((comment) => comment.text), + ...(refreshedTask?.steeringComments ?? []).map((comment) => comment.text), + ]; + expect(allCommentText.some((text) => text.includes("[planner-oversight] merge checkpoint"))).toBe(false); }); - // Note: a genuinely-blocking `autoMergeWillProceed: false` state is - // exercised directly against the pure `decidePlannerRecovery` in - // `planner-recovery.test.ts` (@fusion/core). It is NOT independently - // reachable through this engine's real `tick()` wiring: `allowsAutoMerge - // Processing(task, settings) === false` is exactly the condition - // `evaluateOverseerHumanControl` uses to withhold ALL oversight action - // (including confirmation recording) BEFORE `decidePlannerRecovery` is ever - // called — so a real merger/pull-request confirmation entry can only ever - // be recorded when auto-merge WILL proceed. This is documented here rather - // than asserted redundantly to avoid a test that can never legitimately fail. - - it("denying a confirmation resolution emits a 'skipped' outcome entry", async () => { - const task = await seedTask("in-review"); - const { monitor, controllerFromMonitor: controller } = wireRealEngineOverseer(store); - await monitor.observeTask(task, "autonomous"); - await controller.tick(task); - const pending = controller.getPendingConfirmations(task.id); - expect(pending).toHaveLength(1); - - await controller.resolveConfirmation(task.id, pending[0].requestId, "denied"); - - const timeline = getPlannerInterventionTimeline(store, task.id); - const confirmationEntries = timeline.filter((e) => e.action === "request-confirmation"); - expect(confirmationEntries.some((e) => e.outcome === "skipped")).toBe(true); - }); + // FNXC:PlannerOversight 2026-07-11-00:00: + // FN-7840 makes the old request/approve/deny confirmation-resolution tests unreachable through the public real tick() seam: auto-merge true returns `none`, while auto-merge false is withheld by `evaluateOverseerHumanControl` before `decidePlannerRecovery`. The pure false/undefined safety-valve contract remains covered in @fusion/core; this engine harness documents the production reachability instead of injecting private pending-confirmation state. it("bounded-recovery exhaustion emits exactly one escalate entry across repeated polls of the same exhausted stage", async () => { const task = await seedTask("in-progress"); @@ -297,45 +255,6 @@ describe("FN-7551 — overseer decision points populate the intervention timelin expect(escalations[0].outcome).toBe("failed"); }); - it("exhaustion actually reached through real tick()s (three denials) emits escalate exactly once thereafter", async () => { - const task = await seedTask("in-review"); - // FNXC:PlannerOversight 2026-07-07-08:50: - // FN-7577 (2026-07-05) made PlannerRecoveryController.tick() drop the - // bounded-recovery attempt budget whenever a watched stage reports a - // HEALTHY/human-wait signal (progressing/complete/awaiting-human). A plain - // in-review task derives a "progressing" merger signal, so denials never - // accumulate through the real monitor wiring and exhaustion can't be - // reached — the 4th tick kept returning await_confirmation instead of the - // exhausted "none". This test's invariant is escalation DEDUP after - // exhaustion reached via real tick()s, so wire the controller to a PROBLEM - // (failed) merger snapshot (controllerWithSnapshot — the documented seam for - // branches the monitor's own signal-derivation cannot produce) whose signal - // holds the attempt budget, then drive three real denials to reach genuine - // exhaustion. The merger stage still surfaces await_confirmation regardless - // of signal (decidePlannerRecovery), so requiresConfirmation stays asserted. - const { controllerWithSnapshot, emitEscalation } = wireRealEngineOverseer(store); - const controller = controllerWithSnapshot(observation({ taskId: task.id, stage: "merger", signal: "failed" })); - - for (let i = 0; i < 3; i += 1) { - const decision = await controller.tick(task); - expect(decision?.requiresConfirmation).toBe(true); - const pending = controller.getPendingConfirmations(task.id); - await controller.resolveConfirmation(task.id, pending[0].requestId, "denied"); - } - - const finalDecision = await controller.tick(task); - expect(finalDecision?.action).toBe("none"); - expect(finalDecision?.exhausted).toBe(true); - - // The poll wires escalation emission itself (project-engine.ts), so drive - // it explicitly here with the real decision object, twice, to prove the dedup. - emitEscalation(task.id, finalDecision!); - emitEscalation(task.id, finalDecision!); - - const timeline = getPlannerInterventionTimeline(store, task.id); - expect(timeline.filter((e) => e.action === "escalate")).toHaveLength(1); - }); - it("oversight level 'off' and a human-control-withheld (userPaused) task never emit any steering/retry/fix/confirmation/escalation entry", async () => { const task = await seedTask("in-review"); const { monitor, controllerFromMonitor: controller } = wireRealEngineOverseer(store);