FN-7692: fix misleading merger confirmation copy under active auto-merge

Correct the planner-oversight confirmation messaging so it no longer claims a hard block when the active auto-merge policy will actually advance the merge/pull-request stage unattended.

- decidePlannerRecovery accepts an additive, messaging-only `autoMergeWillProceed` flag and picks accurate reason wording (advisory vs. genuine human-approval block vs. neutral/unknown) for merger/pull-request await_confirmation decisions
- PlannerRecoveryController.tick threads `allowsAutoMergeProcessing(task, settings)` into decidePlannerRecovery as `autoMergeWillProceed`
- project-engine's requestConfirmation steering comment prefix changed from "confirmation required" to neutral "merge checkpoint" so it doesn't contradict the now-accurate reason text
- added regression tests in planner-recovery.test.ts and planner-overseer-intervention-wiring.test.ts
- added changeset and doc note

Files changed:
 .changeset/fn-7692-merger-confirmation-copy.md     |  7 +++
 docs/architecture.md                               | 10 +++-
 packages/core/src/__tests__/planner-recovery.test.ts    | 66 ++++++++++++++++++++++
 packages/core/src/planner-recovery.ts              | 36 +++++++++++-
 packages/engine/src/__tests__/planner-overseer-intervention-wiring.test.ts | 37 ++++++++++++
 packages/engine/src/planner-recovery-controller.ts | 14 ++++-
 packages/engine/src/project-engine.ts               | 11 +++-
 7 files changed, 176 insertions(+), 5 deletions(-)

Fusion-Task-Id: FN-7692

Fusion-Task-Lineage: 187684b8-1d24-425d-85d4-627587469908

Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
2026-07-08 14:34:25 -07:00
parent 0514aabde7
commit 67cc02750c
7 changed files with 176 additions and 5 deletions

View File

@@ -0,0 +1,7 @@
---
"@runfusion/fusion": patch
---
summary: Fix misleading merger "awaiting-confirmation" copy that claimed a hard block when auto-merge advances the merge automatically.
category: fix
dev: `decidePlannerRecovery` now accepts an additive `autoMergeWillProceed` flag (threaded from `allowsAutoMergeProcessing` in `PlannerRecoveryController.tick`) that only shapes the confirmation `reason` string; no gating/behavior change to `action`/`requiresConfirmation`/`sideEffectClass`.

View File

@@ -1510,8 +1510,14 @@ other rule (level gate, attempt bound, exhaustion) is unchanged.
transitions. transitions.
`ProjectEngine` wires the concrete handlers in `buildPlannerRecoveryHandlers`: `requestConfirmation` `ProjectEngine` wires the concrete handlers in `buildPlannerRecoveryHandlers`: `requestConfirmation`
posts a `[planner-oversight] confirmation required (...)` steering comment (reusing the same posts a `[planner-oversight] merge checkpoint (...)` steering comment (reusing the same
`addSteeringComment` channel as bounded recovery, so a human sees it). `executeMergePrAction` branches `addSteeringComment` channel as bounded recovery, so a human sees it). FN-7692: the wrapper prefix is
deliberately neutral ("checkpoint", not "confirmation required") because the trailing `reason` string
accurately states whether the merge/PR will advance automatically under the active auto-merge policy
(advisory) or genuinely awaits a human approval (blocking) — `decidePlannerRecovery`'s additive
`autoMergeWillProceed` input, threaded from the existing `allowsAutoMergeProcessing` predicate, selects
the accurate wording; this is messaging-only and does not change `action`/`requiresConfirmation`/
`sideEffectClass`. `executeMergePrAction` branches
on `request.proposedAction` (falling back to `request.watchedStage` defensively) rather than treating on `request.proposedAction` (falling back to `request.watchedStage` defensively) rather than treating
every approved `"merge_pr"` request identically: ONLY `"advance_merge"` (the `merger` stage) reuses the every approved `"merge_pr"` request identically: ONLY `"advance_merge"` (the `merger` stage) reuses the
EXISTING `store.mergeTask(taskId)` merge mechanism; `"advance_pull_request"` (the `pull-request` stage) EXISTING `store.mergeTask(taskId)` merge mechanism; `"advance_pull_request"` (the `pull-request` stage)

View File

@@ -103,6 +103,72 @@ 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", () => {
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.sideEffectClass, `stage=${stage}`).toBe("merge_pr");
expect(decision.reason, `stage=${stage}`).not.toMatch(/requires explicit confirmation before .* may run/);
expect(decision.reason, `stage=${stage}`).toMatch(/automatically/i);
expect(decision.reason, `stage=${stage}`).toMatch(/advisory/i);
}
});
it("produces accurate blocking copy when auto-merge will NOT proceed unattended, for both merger and pull-request stages", () => {
for (const stage of ["merger", "pull-request"] as const) {
const decision = decidePlannerRecovery({
snapshot: observation({ stage, signal: "failed" }),
autoMergeWillProceed: false,
});
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.reason, `stage=${stage}`).toMatch(/will not .* until a human explicitly approves/);
expect(decision.reason, `stage=${stage}`).not.toMatch(/advisory/i);
}
});
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.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)", () => {
for (const stage of ["merger", "pull-request"] as const) {
const base = decidePlannerRecovery({ snapshot: observation({ stage, signal: "failed" }) });
const proceeds = decidePlannerRecovery({
snapshot: observation({ stage, signal: "failed" }),
autoMergeWillProceed: true,
});
const blocks = decidePlannerRecovery({
snapshot: observation({ stage, signal: "failed" }),
autoMergeWillProceed: false,
});
for (const decision of [base, proceeds, 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);
}
}
});
it("keeps requiresConfirmation false for bounded-recovery decisions", () => { it("keeps requiresConfirmation false for bounded-recovery decisions", () => {
const decision = decidePlannerRecovery({ snapshot: observation({ stage: "executor", signal: "failed" }) }); const decision = decidePlannerRecovery({ snapshot: observation({ stage: "executor", signal: "failed" }) });
expect(decision.requiresConfirmation).toBe(false); expect(decision.requiresConfirmation).toBe(false);

View File

@@ -120,6 +120,24 @@ export interface DecidePlannerRecoveryInput {
snapshot: PlannerRecoveryObservation | null | undefined; snapshot: PlannerRecoveryObservation | null | undefined;
/** Current attempt state for this `(taskId, watchedStage)`; omit for a fresh stage. */ /** Current attempt state for this `(taskId, watchedStage)`; omit for a fresh stage. */
attemptState?: PlannerRecoveryAttemptState; attemptState?: PlannerRecoveryAttemptState;
/**
* FNXC:PlannerOversight 2026-07-08-00:00:
* FN-7692 requirement: additive, messaging-only signal for whether the
* active auto-merge policy will advance a `merger`/`pull-request`
* `"await_confirmation"` decision WITHOUT a human clicking approve
* (`allowsAutoMergeProcessing(task, settings)` from `@fusion/core`'s
* `task-merge.ts`, computed by the caller). This ONLY shapes the `reason`
* string built for that branch below — it must never influence `action`,
* `requiresConfirmation`, `sideEffectClass`, `proposedAction`, attempt
* accounting, or any other decision field. `true` = auto-merge will
* proceed unattended (the confirmation is advisory, not a real block);
* `false` = the stage genuinely will not advance without human approval;
* `undefined` = policy unknown to the caller — use neutral, non-
* overclaiming wording that asserts neither outcome. Fixes FN-7689's
* misleading "requires explicit confirmation" copy shown even though
* auto-merge proceeded unattended a few minutes later.
*/
autoMergeWillProceed?: boolean;
} }
/** /**
@@ -213,9 +231,25 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
if (snapshot.stage === "merger" || snapshot.stage === "pull-request") { if (snapshot.stage === "merger" || snapshot.stage === "pull-request") {
const proposedAction = snapshot.stage === "merger" ? "advance_merge" : "advance_pull_request"; const proposedAction = snapshot.stage === "merger" ? "advance_merge" : "advance_pull_request";
const sideEffectClass = classifyPlannerActionSideEffect({ watchedStage: snapshot.stage, proposedAction }); 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`;
return { return {
action: "await_confirmation", action: "await_confirmation",
reason: `Stage "${snapshot.stage}" requires explicit confirmation before ${proposedAction.replace(/_/g, " ")} may run`, reason,
attemptCount, attemptCount,
attemptLimit, attemptLimit,
exhausted: false, exhausted: false,

View File

@@ -224,6 +224,43 @@ describe("FN-7551 — overseer decision points populate the intervention timelin
expect(confirmationEntries.some((e) => e.outcome === "succeeded")).toBe(true); 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 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);
// A pending confirmation is still recorded — no dispatch, no behavior change.
expect(controller.getPendingConfirmations(task.id)).toHaveLength(1);
});
// 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 () => { it("denying a confirmation resolution emits a 'skipped' outcome entry", async () => {
const task = await seedTask("in-review"); const task = await seedTask("in-review");
const { monitor, controllerFromMonitor: controller } = wireRealEngineOverseer(store); const { monitor, controllerFromMonitor: controller } = wireRealEngineOverseer(store);

View File

@@ -41,7 +41,7 @@
*/ */
import type { PlannerConfirmationRequest, PlannerRecoveryDecision, PlannerRecoveryObservation, Settings, Task } from "@fusion/core"; import type { PlannerConfirmationRequest, PlannerRecoveryDecision, PlannerRecoveryObservation, Settings, Task } from "@fusion/core";
import { decidePlannerRecovery, PLANNER_RECOVERY_MAX_ATTEMPTS } from "@fusion/core"; import { allowsAutoMergeProcessing, decidePlannerRecovery, PLANNER_RECOVERY_MAX_ATTEMPTS } from "@fusion/core";
import { createLogger, type Logger } from "./logger.js"; import { createLogger, type Logger } from "./logger.js";
import type { OverseerStageObservation } from "./planner-overseer.js"; import type { OverseerStageObservation } from "./planner-overseer.js";
import { import {
@@ -282,9 +282,21 @@ export class PlannerRecoveryController {
const attemptCount = this.attempts.get(key) ?? 0; const attemptCount = this.attempts.get(key) ?? 0;
// FNXC:PlannerOversight 2026-07-08-00:00:
// FN-7692: reuse the existing `allowsAutoMergeProcessing` predicate
// (same fallback semantics as the `evaluateOverseerHumanControl` guard
// above — `ctx.settings ?? { autoMerge: true }`) to tell
// `decidePlannerRecovery` whether the pending merger/pull-request
// confirmation is actually advisory (auto-merge will proceed unattended)
// or a genuine block. Messaging-only: this does not add a settings
// lookup beyond what `ctx.settings` already provides, and does not
// change the confirmation-required gating below.
const autoMergeWillProceed = allowsAutoMergeProcessing(task, ctx.settings ?? { autoMerge: true });
const decision = decidePlannerRecovery({ const decision = decidePlannerRecovery({
snapshot: snapshot as unknown as PlannerRecoveryObservation, snapshot: snapshot as unknown as PlannerRecoveryObservation,
attemptState: { attemptCount, attemptLimit: PLANNER_RECOVERY_MAX_ATTEMPTS }, attemptState: { attemptCount, attemptLimit: PLANNER_RECOVERY_MAX_ATTEMPTS },
autoMergeWillProceed,
}); });
if (decision.action === "none") { if (decision.action === "none") {

View File

@@ -1415,8 +1415,17 @@ export class ProjectEngine {
// so a human sees it; it never performs the side effect itself. The // so a human sees it; it never performs the side effect itself. The
// dashboard confirmation UI/badge that lets a human act on this is // dashboard confirmation UI/badge that lets a human act on this is
// owned by FN-7515+/FN-7517. // owned by FN-7515+/FN-7517.
// FNXC:PlannerOversight 2026-07-08-00:00:
// FN-7692 fix: this prefix previously read "confirmation required"
// unconditionally, which contradicted `request.reason` once FN-7692
// made that reason accurately advisory under an active auto-merge
// policy. "checkpoint" is neutral and consistent whether the trailing
// `reason` describes an advisory (auto-merge will proceed) or a
// genuine block (human approval required) — messaging-only, no change
// to the `addSteeringComment` channel/timing or `emitOverseerConfirmation`
// below.
requestConfirmation: async (task, request) => { requestConfirmation: async (task, request) => {
const text = `[planner-oversight] confirmation required (${request.sideEffectClass}): ${request.reason}`; const text = `[planner-oversight] merge checkpoint (${request.sideEffectClass}): ${request.reason}`;
await store.addSteeringComment(task.id, text, "agent"); await store.addSteeringComment(task.id, text, "agent");
this.emitOverseerInterventionSafe(() => this.emitOverseerInterventionSafe(() =>
emitOverseerConfirmation({ emitOverseerConfirmation({