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) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-7840-advisory-merger-confirmations.md
Normal file
7
.changeset/fn-7840-advisory-merger-confirmations.md
Normal file
@@ -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).
|
||||
@@ -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();
|
||||
}
|
||||
});
|
||||
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in New Issue
Block a user