FN-7513: require planner confirmation for risky side effects
Require explicit approval before planner recovery runs merge, PR, destructive, or external-service actions. - Add pure planner side-effect classification and confirmation request modeling in core. - Route merge/PR recovery decisions to await confirmation instead of autonomous dispatch. - Persist pending confirmation requests and only execute approved controller actions. - Cover confirmation gating with core and engine regression tests and document the policy. Files changed: .changeset/fn-7513-planner-confirmation-gate.md | 7 + docs/architecture.md | 85 ++++++++- docs/settings-reference.md | 2 +- .../src/__tests__/planner-confirmation.test.ts | 125 +++++++++++++ .../core/src/__tests__/planner-recovery.test.ts | 14 +- packages/core/src/index.ts | 7 + packages/core/src/planner-confirmation.ts | 141 ++++++++++++++ packages/core/src/planner-recovery.ts | 103 ++++++++--- ...lanner-recovery-controller-confirmation.test.ts | 205 +++++++++++++++++++++ packages/engine/src/index.ts | 5 + packages/engine/src/planner-recovery-controller.ts | 204 +++++++++++++++++++- packages/engine/src/project-engine.ts | 44 +++++ 12 files changed, 913 insertions(+), 29 deletions(-) Fusion-Task-Id: FN-7513 Fusion-Task-Lineage: 1e3c6640-8a4f-41f6-89dd-41eb9b675b2b Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
7
.changeset/fn-7513-planner-confirmation-gate.md
Normal file
7
.changeset/fn-7513-planner-confirmation-gate.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": minor
|
||||
---
|
||||
|
||||
summary: Planner oversight now requires confirmation before merge/PR actions and destructive/external side effects.
|
||||
category: feature
|
||||
dev: Adds `PlannerActionSideEffectClass` + `PlannerConfirmationRequest` and `classifyPlannerActionSideEffect`/`requiresPlannerConfirmation` (core), extends `decidePlannerRecovery` with an `await_confirmation` action, and adds `requestConfirmation`/`resolveConfirmation` gating to `PlannerRecoveryController` (engine). Merge/PR and destructive/external actions never execute without a recorded approval; bounded recovery (guidance/retry/targeted-fix) is unchanged. UX rendering, human-control safeguards, timeline, and run-audit land in follow-up tasks.
|
||||
@@ -1403,8 +1403,9 @@ and the pure, never-throw `decidePlannerRecovery(input)`. Decision rules, in ord
|
||||
2. The per-`(taskId, watchedStage)` attempt count has reached `PLANNER_RECOVERY_MAX_ATTEMPTS` (default
|
||||
`3`, mirroring `MAX_RECOVERY_RETRIES` in `recovery-policy.ts`) → `"none"`, `exhausted: true` — the
|
||||
layer stops autonomously and the task is left for escalation (FN-7514+ owns the human-control story).
|
||||
3. `merger` / `pull-request` stages → `"none"` with a deferral reason: these require confirmation and
|
||||
are owned by FN-7513's confirmation-gated layer, never dispatched from here.
|
||||
3. `merger` / `pull-request` stages → `"await_confirmation"` (FN-7513) with `requiresConfirmation: true`,
|
||||
`sideEffectClass: "merge_pr"`: these require confirmation and are surfaced, never dispatched, by the
|
||||
bounded layer itself — see "Planner overseer confirmation gate (FN-7513)" below.
|
||||
4. `reviewer` stage → `"inject_guidance"`.
|
||||
5. `executor` / `workflow-gate` stage with `signal === "failed"` → `"request_targeted_fix"` when a
|
||||
source link carries a specific fixable error (`failed-check` / `merge-error`), else `"retry_step"`.
|
||||
@@ -1440,6 +1441,86 @@ destructive/external-service side effects (FN-7513, confirmation-gated); compreh
|
||||
persisted intervention timeline (FN-7519); run-audit/activity events (FN-7520); and any dashboard UI
|
||||
(FN-7515+).
|
||||
|
||||
### Planner overseer confirmation gate (FN-7513)
|
||||
|
||||
/*
|
||||
FNXC:PlannerOversight 2026-07-04-13:00:
|
||||
FN-7513 adds the safety gate deciding which FN-7512 recovery-layer actions may run autonomously versus
|
||||
which must be blocked behind an explicit, recorded human approval. Merge/PR progression (advancing a
|
||||
merge, promoting a shared branch, retrying/forcing a merge, opening/updating/merging a pull request) and
|
||||
any destructive or external-service side effect (branch/worktree deletion, force operations, remote
|
||||
pushes, third-party GitHub/GitLab calls) are classified confirmation-required, regardless of the
|
||||
effective oversight level. Bounded recovery (inject_guidance / retry_step / request_targeted_fix on
|
||||
non-merge/PR stages, FN-7512) is unaffected and remains no-confirmation. The invariant: a gated action
|
||||
NEVER executes without a recorded, approved `PlannerConfirmationRequest`.
|
||||
*/
|
||||
|
||||
`packages/core/src/planner-confirmation.ts` declares the classifier vocabulary:
|
||||
|
||||
- **`PlannerActionSideEffectClass`** (`"bounded_recovery" | "merge_pr" | "destructive_external"`).
|
||||
- **`PlannerConfirmationRequest`** — `{ requestId, taskId, watchedStage, sideEffectClass, proposedAction,
|
||||
reason, sourceLinks, requestedAt, status: "pending" | "approved" | "denied", resolvedAt?, resolvedBy? }`.
|
||||
Conceptually mirrors `TaskMergeDetails.mergeConfirmed` (an explicit human approval precedes a side
|
||||
effect) but is its own record — it never reads or writes `mergeConfirmed`, which stays owned by the
|
||||
merge dispatch path.
|
||||
- **`classifyPlannerActionSideEffect({ watchedStage, proposedAction })`** — pure, deterministic,
|
||||
never-throw. `merger` / `pull-request` stage actions beyond guidance/retry → `"merge_pr"`; an
|
||||
explicit allow-list of destructive/external action names (branch/worktree delete, force push/merge/
|
||||
delete, remote push, GitHub/GitLab/external-service calls, PR open/merge, shared-branch promotion) →
|
||||
`"destructive_external"` regardless of stage; everything else → `"bounded_recovery"`. Malformed input
|
||||
or an unrecognized non-bounded action on a non-merge/PR stage fails CLOSED to
|
||||
`"destructive_external"` rather than silently allowing an unclassified action through.
|
||||
- **`requiresPlannerConfirmation(sideEffectClass)`** — `true` for `"merge_pr"` / `"destructive_external"`,
|
||||
`false` for `"bounded_recovery"`.
|
||||
|
||||
`packages/core/src/planner-recovery.ts`'s `decidePlannerRecovery` now calls the classifier for every
|
||||
branch and returns `requiresConfirmation` / `sideEffectClass` / (for gated decisions) `proposedAction` on
|
||||
`PlannerRecoveryDecision`. The merger/pull-request branch, previously `"none"`, now returns `action:
|
||||
"await_confirmation"` naming what would run on approval (`advance_merge` / `advance_pull_request`); every
|
||||
other rule (level gate, attempt bound, exhaustion) is unchanged.
|
||||
|
||||
`packages/engine/src/planner-recovery-controller.ts`'s `PlannerRecoveryController` adds the gate:
|
||||
|
||||
- A per-`(taskId, watchedStage)` **pending-confirmation registry** (`getPendingConfirmations(taskId)`)
|
||||
— idempotent: `tick` never creates a second pending request for a stage that already has one pending.
|
||||
- `requestConfirmation(task, request, ctx)` (optional handler) — records/surfaces the pending request;
|
||||
it must NOT perform the side effect itself.
|
||||
- `executeMergePrAction(taskId, request, ctx)` / `executeDestructiveExternalAction(taskId, request, ctx)`
|
||||
(optional handlers) — invoked ONLY from `resolveConfirmation(..., "approved", ...)`, never from `tick`.
|
||||
- `resolveConfirmation(taskId, requestId, "approved" | "denied", resolvedBy?)` — on `"approved"`,
|
||||
dispatches the matching execution handler exactly once and clears the pending request; on `"denied"`,
|
||||
clears the request with no side effect, leaving the task for other escalation (FN-7514+), AND consumes
|
||||
one bounded-recovery attempt for that `(taskId, watchedStage)` pair (the same shared
|
||||
`PLANNER_RECOVERY_MAX_ATTEMPTS` budget `dispatch()` consumes). Without counting denials against the
|
||||
budget, a denied merge/PR/destructive confirmation would resurface as an identical pending request on
|
||||
the very next `tick()` forever; counting it means repeated denials eventually exhaust the stage
|
||||
(`decidePlannerRecovery` then returns `action: "none", exhausted: true`) instead of re-prompting
|
||||
indefinitely. Never throws — handler rejections are logged and swallowed, and the request is still
|
||||
cleared.
|
||||
- `tick(task, ctx)`: when `decidePlannerRecovery` returns `requiresConfirmation: true`, calls
|
||||
`requestConfirmation` (idempotently) and does NOT invoke any side-effecting handler; bounded-recovery
|
||||
decisions still dispatch exactly as FN-7512 (with the attempt increment). The `"autonomous"`-only gate
|
||||
and the `userPaused` skip are preserved.
|
||||
- `clear(taskId)` also clears pending confirmations (in addition to attempt state) on terminal task
|
||||
transitions.
|
||||
|
||||
`ProjectEngine` wires the concrete handlers in `buildPlannerRecoveryHandlers`: `requestConfirmation`
|
||||
posts a `[planner-oversight] confirmation required (...)` steering comment (reusing the same
|
||||
`addSteeringComment` channel as bounded recovery, so a human sees it). `executeMergePrAction` branches
|
||||
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
|
||||
EXISTING `store.mergeTask(taskId)` merge mechanism; `"advance_pull_request"` (the `pull-request` stage)
|
||||
is intentionally a no-op today because no reusable PR-specific advance mechanism exists yet — an
|
||||
approved PR confirmation must never fall through to a direct task merge/cleanup, which would bypass the
|
||||
PR workflow entirely. No `executeDestructiveExternalAction` is wired yet, since FN-7511's observation
|
||||
model does not currently emit a destructive-action signal; a future task can wire one (and the
|
||||
PR-specific execution handler) using existing safe helpers when a concrete need arises.
|
||||
|
||||
**Downstream ownership (not this layer):** rendering the pending-confirmation UI/badge (FN-7515+/
|
||||
FN-7517), comprehensive human-control safeguards beyond `userPaused` (FN-7514), a persisted intervention
|
||||
timeline (FN-7519), and run-audit/activity events (FN-7520) all consume the data this gate exposes but
|
||||
are implemented elsewhere.
|
||||
|
||||
---
|
||||
|
||||
## 11) Multi-Project Architecture
|
||||
|
||||
@@ -359,7 +359,7 @@ The built-in workflows also declare triage/spec policy settings that were **not*
|
||||
| `autoApproveSpec` | `false` | Legacy compatibility setting. Workflow Plan Review now owns optional pre-execution AI plan approval. |
|
||||
| `planReviewMaxRevisions` | unset | Workflow-native Plan Review/spec revision cap. Unset/empty means unbounded automatic replans; a non-negative integer caps attempts; `0` disables automatic Plan Review revision. |
|
||||
| `codeReviewMaxRevisions` | unset | Workflow-native Code Review remediation cap. Unset/empty means unbounded automatic code-fix passes; a non-negative integer caps attempts; `0` disables automatic Code Review remediation. |
|
||||
| `plannerOversightLevel` | `autonomous` | Workflow-native planner oversight mode. `off` disables oversight; `observe` watches only; `steer` injects guidance or suggests revisions; `autonomous` enables bounded retry and targeted-fix recovery. Tasks may set a nullable `Task.plannerOversightLevel` override (same four values) that wins over this workflow value when present; `null`/unset means "inherit the workflow value". `resolveEffectivePlannerOversightLevel` in `@fusion/core` computes the effective level (task override → workflow effective → `autonomous`). Dashboard UI/API threading for the per-task override and engine read-site behavior are follow-up work (FN-7515, FN-7510+). |
|
||||
| `plannerOversightLevel` | `autonomous` | Workflow-native planner oversight mode. `off` disables oversight; `observe` watches only; `steer` injects guidance or suggests revisions; `autonomous` enables bounded retry and targeted-fix recovery — but merge/PR progression and any destructive or external-service side effect ALWAYS require an explicit, recorded human confirmation before they run, even at `autonomous` (FN-7513's confirmation gate; see `docs/architecture.md` → "Planner overseer confirmation gate"). Tasks may set a nullable `Task.plannerOversightLevel` override (same four values) that wins over this workflow value when present; `null`/unset means "inherit the workflow value". `resolveEffectivePlannerOversightLevel` in `@fusion/core` computes the effective level (task override → workflow effective → `autonomous`). Dashboard UI/API threading for the per-task override and engine read-site behavior are follow-up work (FN-7515, FN-7510+). |
|
||||
|
||||
When `triageProactiveSubtaskSplittingEnabled` is `true` (the default), triage may proactively replace a large task with 2-5 child tasks when the size, step-count, package breadth, file-scope, or remediation-batch signals justify the coordination overhead. When it is `false`, those automatic oversized-task signals are advisory only for writing a realistic single-task spec; triage must not split solely because the task is large. The per-task `breakIntoSubtasks: true` flag is separate and remains mandatory: if a user explicitly asks for subtask breakdown, triage still evaluates and creates child tasks when the work is meaningfully decomposable.
|
||||
|
||||
|
||||
125
packages/core/src/__tests__/planner-confirmation.test.ts
Normal file
125
packages/core/src/__tests__/planner-confirmation.test.ts
Normal file
@@ -0,0 +1,125 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { classifyPlannerActionSideEffect, requiresPlannerConfirmation } from "../planner-confirmation.js";
|
||||
import { decidePlannerRecovery, type PlannerRecoveryObservation } from "../planner-recovery.js";
|
||||
|
||||
function observation(overrides: Partial<PlannerRecoveryObservation> = {}): PlannerRecoveryObservation {
|
||||
return {
|
||||
taskId: "FN-1",
|
||||
stage: "executor",
|
||||
signal: "progressing",
|
||||
oversightLevel: "autonomous",
|
||||
sources: [],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
describe("classifyPlannerActionSideEffect", () => {
|
||||
it("classifies merger and pull-request stage actions as merge_pr", () => {
|
||||
for (const stage of ["merger", "pull-request"] as const) {
|
||||
expect(classifyPlannerActionSideEffect({ watchedStage: stage, proposedAction: "advance_merge" })).toBe("merge_pr");
|
||||
expect(classifyPlannerActionSideEffect({ watchedStage: stage, proposedAction: "retry_step" })).toBe("merge_pr");
|
||||
}
|
||||
});
|
||||
|
||||
it("classifies enumerated destructive/external actions as destructive_external regardless of stage", () => {
|
||||
const destructiveActions = [
|
||||
"delete_branch",
|
||||
"delete_worktree",
|
||||
"force_push",
|
||||
"force_merge",
|
||||
"force_delete",
|
||||
"push_remote",
|
||||
"call_external_service",
|
||||
"github_api_call",
|
||||
"gitlab_api_call",
|
||||
"open_pull_request",
|
||||
"merge_pull_request",
|
||||
"promote_shared_branch",
|
||||
];
|
||||
for (const proposedAction of destructiveActions) {
|
||||
expect(classifyPlannerActionSideEffect({ watchedStage: "executor", proposedAction })).toBe("destructive_external");
|
||||
expect(classifyPlannerActionSideEffect({ watchedStage: null, proposedAction })).toBe("destructive_external");
|
||||
}
|
||||
});
|
||||
|
||||
it("classifies bounded recovery actions on non-merge/PR stages as bounded_recovery", () => {
|
||||
for (const stage of ["executor", "reviewer", "workflow-gate"] as const) {
|
||||
for (const proposedAction of ["inject_guidance", "retry_step", "request_targeted_fix"]) {
|
||||
expect(classifyPlannerActionSideEffect({ watchedStage: stage, proposedAction })).toBe("bounded_recovery");
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
it("classifies no watched stage / empty action as bounded_recovery", () => {
|
||||
expect(classifyPlannerActionSideEffect({ watchedStage: null, proposedAction: "none" })).toBe("bounded_recovery");
|
||||
expect(classifyPlannerActionSideEffect({ watchedStage: undefined, proposedAction: undefined })).toBe("bounded_recovery");
|
||||
});
|
||||
|
||||
it("fails closed (destructive_external) for an unknown non-bounded action on a non-merge/PR stage", () => {
|
||||
expect(classifyPlannerActionSideEffect({ watchedStage: "executor", proposedAction: "mystery_action" })).toBe(
|
||||
"destructive_external",
|
||||
);
|
||||
});
|
||||
|
||||
it("never throws on malformed input", () => {
|
||||
expect(() => classifyPlannerActionSideEffect(undefined as never)).not.toThrow();
|
||||
expect(() => classifyPlannerActionSideEffect(null as never)).not.toThrow();
|
||||
expect(() => classifyPlannerActionSideEffect({} as never)).not.toThrow();
|
||||
});
|
||||
});
|
||||
|
||||
describe("requiresPlannerConfirmation", () => {
|
||||
it("is true for merge_pr and destructive_external", () => {
|
||||
expect(requiresPlannerConfirmation("merge_pr")).toBe(true);
|
||||
expect(requiresPlannerConfirmation("destructive_external")).toBe(true);
|
||||
});
|
||||
|
||||
it("is false for bounded_recovery and for null/undefined", () => {
|
||||
expect(requiresPlannerConfirmation("bounded_recovery")).toBe(false);
|
||||
expect(requiresPlannerConfirmation(null)).toBe(false);
|
||||
expect(requiresPlannerConfirmation(undefined)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("decidePlannerRecovery — confirmation gating (FN-7513)", () => {
|
||||
it("returns await_confirmation + requiresConfirmation for merger/pull-request stages", () => {
|
||||
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).toBe(true);
|
||||
expect(decision.sideEffectClass).toBe("merge_pr");
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps bounded-recovery decisions requiresConfirmation: false", () => {
|
||||
const decision = decidePlannerRecovery({ snapshot: observation({ stage: "reviewer", signal: "progressing" }) });
|
||||
expect(decision.action).toBe("inject_guidance");
|
||||
expect(decision.requiresConfirmation).toBe(false);
|
||||
expect(decision.sideEffectClass).toBe("bounded_recovery");
|
||||
});
|
||||
|
||||
it("preserves the autonomous-only gate for confirmation-eligible stages", () => {
|
||||
for (const level of ["off", "observe", "steer"] as const) {
|
||||
const decision = decidePlannerRecovery({ snapshot: observation({ stage: "merger", oversightLevel: level, signal: "failed" }) });
|
||||
expect(decision.action, `level=${level}`).toBe("none");
|
||||
expect(decision.requiresConfirmation).toBe(false);
|
||||
}
|
||||
});
|
||||
|
||||
it("preserves the attempt bound and exhaustion behavior for merger/pull-request stages", () => {
|
||||
const decision = decidePlannerRecovery({
|
||||
snapshot: observation({ stage: "merger", signal: "failed" }),
|
||||
attemptState: { attemptCount: 3, attemptLimit: 3 },
|
||||
});
|
||||
expect(decision.action).toBe("none");
|
||||
expect(decision.exhausted).toBe(true);
|
||||
expect(decision.requiresConfirmation).toBe(false);
|
||||
});
|
||||
|
||||
it("never throws on partial snapshots and always carries a sideEffectClass", () => {
|
||||
const decision = decidePlannerRecovery({ snapshot: { taskId: "FN-1" } as unknown as PlannerRecoveryObservation });
|
||||
expect(decision.action).toBe("none");
|
||||
expect(decision.sideEffectClass).toBe("bounded_recovery");
|
||||
expect(decision.requiresConfirmation).toBe(false);
|
||||
});
|
||||
});
|
||||
@@ -68,14 +68,22 @@ describe("decidePlannerRecovery", () => {
|
||||
expect(decision.action).toBe("inject_guidance");
|
||||
});
|
||||
|
||||
it("defers merger and pull-request stages to none with a deferral reason", () => {
|
||||
it("gates merger and pull-request stages behind confirmation (FN-7513) instead of none", () => {
|
||||
for (const stage of ["merger", "pull-request"] as const) {
|
||||
const decision = decidePlannerRecovery({ snapshot: observation({ stage, signal: "failed" }) });
|
||||
expect(decision.action, `stage=${stage}`).toBe("none");
|
||||
expect(decision.reason.toLowerCase()).toContain("deferred");
|
||||
expect(decision.action, `stage=${stage}`).toBe("await_confirmation");
|
||||
expect(decision.requiresConfirmation).toBe(true);
|
||||
expect(decision.sideEffectClass).toBe("merge_pr");
|
||||
expect(decision.proposedAction).toBeTruthy();
|
||||
}
|
||||
});
|
||||
|
||||
it("keeps requiresConfirmation false for bounded-recovery decisions", () => {
|
||||
const decision = decidePlannerRecovery({ snapshot: observation({ stage: "executor", signal: "failed" }) });
|
||||
expect(decision.requiresConfirmation).toBe(false);
|
||||
expect(decision.sideEffectClass).toBe("bounded_recovery");
|
||||
});
|
||||
|
||||
it("returns none + exhausted true exactly at the attempt limit", () => {
|
||||
const decision = decidePlannerRecovery({
|
||||
snapshot: observation({ stage: "executor", signal: "failed" }),
|
||||
|
||||
@@ -442,6 +442,13 @@ export {
|
||||
type PlannerRecoveryDecision,
|
||||
type DecidePlannerRecoveryInput,
|
||||
} from "./planner-recovery.js";
|
||||
export {
|
||||
classifyPlannerActionSideEffect,
|
||||
requiresPlannerConfirmation,
|
||||
type PlannerActionSideEffectClass,
|
||||
type PlannerConfirmationRequest,
|
||||
type ClassifyPlannerActionSideEffectInput,
|
||||
} from "./planner-confirmation.js";
|
||||
|
||||
// ── Engine wiring (set by @fusion/engine at module load) ────────────
|
||||
export {
|
||||
|
||||
141
packages/core/src/planner-confirmation.ts
Normal file
141
packages/core/src/planner-confirmation.ts
Normal file
@@ -0,0 +1,141 @@
|
||||
/**
|
||||
* FNXC:PlannerOversight 2026-07-04-13:00:
|
||||
* FN-7513 requirement: merge/PR actions beyond bounded guidance/retry, and
|
||||
* any destructive or external-service side effect (branch/worktree
|
||||
* deletion, force operations, remote pushes, third-party GitHub/GitLab
|
||||
* calls), MUST NOT be executed autonomously by the planner overseer —
|
||||
* regardless of the effective oversight level. They are classified as
|
||||
* confirmation-required and must be blocked behind an explicit, recorded
|
||||
* user approval (`PlannerConfirmationRequest`, resolved via the engine's
|
||||
* `PlannerRecoveryController.resolveConfirmation`) before the associated
|
||||
* side-effecting handler ever runs. Bounded recovery (inject_guidance /
|
||||
* retry_step / request_targeted_fix on non-merge/PR stages, delivered by
|
||||
* FN-7512) requires no confirmation and is unaffected by this module.
|
||||
*
|
||||
* This module is pure, deterministic, and never throws — it has no engine
|
||||
* imports and performs no I/O. It only classifies; it never itself decides
|
||||
* *whether* to run, only *what class of side effect* a proposed action
|
||||
* belongs to.
|
||||
*/
|
||||
|
||||
import type { PlannerRecoveryActionKind, PlannerRecoveryWatchedStage, PlannerRecoverySourceLink } from "./planner-recovery.js";
|
||||
|
||||
/**
|
||||
* The three side-effect classes a proposed planner-overseer action can fall
|
||||
* into:
|
||||
* - `"bounded_recovery"` — inject_guidance / retry_step / request_targeted_fix
|
||||
* on a non-merge/PR stage (FN-7512's autonomous, no-confirmation layer).
|
||||
* - `"merge_pr"` — advancing a merge, promoting a shared branch, retrying or
|
||||
* forcing a merge, or opening/updating/merging a pull request — i.e. any
|
||||
* watched-stage action on the `merger` / `pull-request` stages that is not
|
||||
* pure guidance or a bounded step-retry.
|
||||
* - `"destructive_external"` — deleting branches/worktrees, force
|
||||
* operations, pushing to a remote, or calling a third-party service
|
||||
* (GitHub/GitLab/etc.), regardless of stage.
|
||||
*/
|
||||
export type PlannerActionSideEffectClass = "bounded_recovery" | "merge_pr" | "destructive_external";
|
||||
|
||||
/**
|
||||
* A pending (or resolved) request for explicit human approval of a
|
||||
* confirmation-required planner-overseer action. Conceptually mirrors
|
||||
* `TaskMergeDetails.mergeConfirmed` (an explicit human approval precedes the
|
||||
* side effect) but is its own record — it does NOT read or write
|
||||
* `mergeConfirmed`, which remains owned by the merge dispatch path.
|
||||
*/
|
||||
export interface PlannerConfirmationRequest {
|
||||
requestId: string;
|
||||
taskId: string;
|
||||
watchedStage: PlannerRecoveryWatchedStage;
|
||||
sideEffectClass: PlannerActionSideEffectClass;
|
||||
/** The action that would run if this request is approved. */
|
||||
proposedAction: PlannerRecoveryActionKind | string;
|
||||
reason: string;
|
||||
sourceLinks: PlannerRecoverySourceLink[];
|
||||
requestedAt: number;
|
||||
status: "pending" | "approved" | "denied";
|
||||
resolvedAt?: number;
|
||||
resolvedBy?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Proposed-action names known to represent a destructive or external-service
|
||||
* side effect (branch/worktree deletion, force operations, remote pushes,
|
||||
* third-party GitHub/GitLab/service calls) irrespective of watched stage.
|
||||
* Kept as an explicit allow-list (rather than a heuristic over free-form
|
||||
* strings) so classification stays deterministic and easy to audit/extend.
|
||||
*/
|
||||
const DESTRUCTIVE_EXTERNAL_ACTIONS = new Set<string>([
|
||||
"delete_branch",
|
||||
"delete_worktree",
|
||||
"force_push",
|
||||
"force_merge",
|
||||
"force_delete",
|
||||
"push_remote",
|
||||
"call_external_service",
|
||||
"github_api_call",
|
||||
"gitlab_api_call",
|
||||
"open_pull_request",
|
||||
"merge_pull_request",
|
||||
"promote_shared_branch",
|
||||
]);
|
||||
|
||||
/** Bounded, non-merge/PR recovery actions FN-7512 already dispatches with no confirmation. */
|
||||
const BOUNDED_RECOVERY_ACTIONS = new Set<string>(["inject_guidance", "retry_step", "request_targeted_fix"]);
|
||||
|
||||
export interface ClassifyPlannerActionSideEffectInput {
|
||||
/** The watched stage the action would apply to, or `null` when there is none. */
|
||||
watchedStage: PlannerRecoveryWatchedStage | null | undefined;
|
||||
/** The proposed action name (a `PlannerRecoveryActionKind`, or an FN-7514+ specific action string). */
|
||||
proposedAction: string | null | undefined;
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:PlannerOversight 2026-07-04-13:00:
|
||||
* Pure, deterministic, never-throw classifier mapping a proposed action on a
|
||||
* watched stage to its `PlannerActionSideEffectClass`:
|
||||
* 1. A proposed action on the explicit destructive/external allow-list →
|
||||
* `"destructive_external"`, regardless of stage.
|
||||
* 2. `merger` / `pull-request` stage for any action that is not a bare
|
||||
* bounded-recovery action → `"merge_pr"`. Bounded-recovery action names
|
||||
* landing on these stages (which FN-7512's decision function never
|
||||
* actually dispatches there) still classify as `"merge_pr"` — these
|
||||
* stages are inherently merge/PR side effects.
|
||||
* 3. Anything else (bounded recovery actions on executor/reviewer/
|
||||
* workflow-gate stages, or no watched stage) → `"bounded_recovery"`.
|
||||
* On malformed/unexpected input, degrades to the FAIL-CLOSED default
|
||||
* `"destructive_external"` rather than silently allowing an unclassified
|
||||
* action to run unconfirmed.
|
||||
*/
|
||||
export function classifyPlannerActionSideEffect(input: ClassifyPlannerActionSideEffectInput): PlannerActionSideEffectClass {
|
||||
try {
|
||||
const watchedStage = input?.watchedStage ?? null;
|
||||
const proposedAction = typeof input?.proposedAction === "string" ? input.proposedAction : "";
|
||||
|
||||
if (DESTRUCTIVE_EXTERNAL_ACTIONS.has(proposedAction)) {
|
||||
return "destructive_external";
|
||||
}
|
||||
|
||||
if (watchedStage === "merger" || watchedStage === "pull-request") {
|
||||
return "merge_pr";
|
||||
}
|
||||
|
||||
if (BOUNDED_RECOVERY_ACTIONS.has(proposedAction) || proposedAction === "" || proposedAction === "none") {
|
||||
return "bounded_recovery";
|
||||
}
|
||||
|
||||
// Unknown, non-bounded action name on a non-merge/PR stage: fail closed.
|
||||
return "destructive_external";
|
||||
} catch {
|
||||
return "destructive_external";
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* `true` when `sideEffectClass` requires an explicit, recorded human
|
||||
* approval before the associated action may run (`"merge_pr"` and
|
||||
* `"destructive_external"`); `false` for `"bounded_recovery"`, which FN-7512
|
||||
* already dispatches autonomously with no confirmation.
|
||||
*/
|
||||
export function requiresPlannerConfirmation(sideEffectClass: PlannerActionSideEffectClass | null | undefined): boolean {
|
||||
return sideEffectClass === "merge_pr" || sideEffectClass === "destructive_external";
|
||||
}
|
||||
@@ -8,10 +8,13 @@
|
||||
* per-(task, watched-stage) attempt limit (`PLANNER_RECOVERY_MAX_ATTEMPTS`)
|
||||
* so recovery can never loop forever; once the budget is exhausted the
|
||||
* decision degrades to `"none"` with `exhausted: true` and the task is left
|
||||
* for human/other escalation. Merge/PR and destructive actions are
|
||||
* explicitly OUT of scope here (deferred to FN-7513's confirmation-gated
|
||||
* layer), and comprehensive human-control safeguards beyond a bare
|
||||
* `userPaused` skip are FN-7514's responsibility. This module is pure,
|
||||
* for human/other escalation. Merge/PR and destructive/external-service
|
||||
* actions are classified confirmation-required (FN-7513: `action:
|
||||
* "await_confirmation"`, `requiresConfirmation: true`) rather than dispatched
|
||||
* from this bounded layer — they only ever run once a
|
||||
* `PlannerConfirmationRequest` is explicitly approved via the engine
|
||||
* controller's `resolveConfirmation`. Comprehensive human-control safeguards
|
||||
* beyond a bare `userPaused` skip are FN-7514's responsibility. This module is pure,
|
||||
* never-throws, and has NO engine imports — the engine-side dispatch lives
|
||||
* in `@fusion/engine`'s `PlannerRecoveryController`.
|
||||
*
|
||||
@@ -29,9 +32,16 @@
|
||||
*/
|
||||
|
||||
import type { PlannerOversightLevel } from "./types.js";
|
||||
import { classifyPlannerActionSideEffect, requiresPlannerConfirmation, type PlannerActionSideEffectClass } from "./planner-confirmation.js";
|
||||
|
||||
/** The bounded corrective actions autonomous planner recovery may take. */
|
||||
export type PlannerRecoveryActionKind = "inject_guidance" | "retry_step" | "request_targeted_fix" | "none";
|
||||
/**
|
||||
* The bounded corrective actions autonomous planner recovery may take, plus
|
||||
* `"await_confirmation"` (FN-7513) — the recovery layer has identified a
|
||||
* confirmation-required action (merge/PR progression, or a destructive/
|
||||
* external-service side effect) and is waiting on an explicit, recorded
|
||||
* human approval before it may run.
|
||||
*/
|
||||
export type PlannerRecoveryActionKind = "inject_guidance" | "retry_step" | "request_targeted_fix" | "await_confirmation" | "none";
|
||||
|
||||
/** Mirrors the delivered `OverseerWatchedStage` union (FN-7511). */
|
||||
export type PlannerRecoveryWatchedStage = "executor" | "reviewer" | "merger" | "pull-request" | "workflow-gate";
|
||||
@@ -76,6 +86,23 @@ export interface PlannerRecoveryDecision {
|
||||
exhausted: boolean;
|
||||
watchedStage: PlannerRecoveryWatchedStage | null;
|
||||
sourceLinks: PlannerRecoverySourceLink[];
|
||||
/**
|
||||
* FN-7513: `true` when this decision's action must not run without an
|
||||
* explicit, recorded human approval (`PlannerActionSideEffectClass` of
|
||||
* `"merge_pr"` or `"destructive_external"`). `false` for bounded recovery
|
||||
* (`inject_guidance` / `retry_step` / `request_targeted_fix`) and for
|
||||
* `"none"` decisions, which take no action either way.
|
||||
*/
|
||||
requiresConfirmation: boolean;
|
||||
/** FN-7513: the side-effect class this decision's action was classified into. */
|
||||
sideEffectClass: PlannerActionSideEffectClass;
|
||||
/**
|
||||
* FN-7513: for `action: "await_confirmation"`, the specific action name
|
||||
* that would run once a matching `PlannerConfirmationRequest` is approved.
|
||||
* Undefined for actions that dispatch immediately (bounded recovery) or
|
||||
* for `"none"`.
|
||||
*/
|
||||
proposedAction?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
@@ -104,9 +131,11 @@ export interface DecidePlannerRecoveryInput {
|
||||
* 2. Attempt budget for the `(taskId, watchedStage)` already spent
|
||||
* (`attemptCount >= attemptLimit`) → `"none"`, `exhausted: true` (stop
|
||||
* autonomously; leave the task for escalation).
|
||||
* 3. `merger` / `pull-request` stages → `"none"` with a deferral reason —
|
||||
* these require confirmation and are owned by FN-7513, never dispatched
|
||||
* from this bounded layer.
|
||||
* 3. `merger` / `pull-request` stages → `"await_confirmation"` with
|
||||
* `requiresConfirmation: true`, `sideEffectClass: "merge_pr"` (FN-7513) —
|
||||
* the decision names what WOULD run on approval but never dispatches it
|
||||
* from this bounded layer; only the engine controller's
|
||||
* `resolveConfirmation`, after an explicit human approval, may.
|
||||
* 4. `reviewer` stage → `"inject_guidance"`.
|
||||
* 5. `executor` / `workflow-gate` stage with `signal === "failed"` →
|
||||
* `"request_targeted_fix"` when a source link carries a specific
|
||||
@@ -132,6 +161,8 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
|
||||
exhausted: false,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
requiresConfirmation: false,
|
||||
sideEffectClass: "bounded_recovery",
|
||||
};
|
||||
}
|
||||
|
||||
@@ -144,6 +175,8 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
|
||||
exhausted: false,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
requiresConfirmation: false,
|
||||
sideEffectClass: "bounded_recovery",
|
||||
};
|
||||
}
|
||||
|
||||
@@ -156,22 +189,36 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
|
||||
exhausted: true,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
requiresConfirmation: false,
|
||||
sideEffectClass: "bounded_recovery",
|
||||
};
|
||||
}
|
||||
|
||||
// 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.
|
||||
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 });
|
||||
return {
|
||||
action: "none",
|
||||
reason: `Stage "${snapshot.stage}" requires confirmation-gated recovery (deferred to FN-7513)`,
|
||||
action: "await_confirmation",
|
||||
reason: `Stage "${snapshot.stage}" requires explicit confirmation before ${proposedAction.replace(/_/g, " ")} may run`,
|
||||
attemptCount,
|
||||
attemptLimit,
|
||||
exhausted: false,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
requiresConfirmation: requiresPlannerConfirmation(sideEffectClass),
|
||||
sideEffectClass,
|
||||
proposedAction,
|
||||
};
|
||||
}
|
||||
|
||||
if (snapshot.stage === "reviewer") {
|
||||
const proposedAction = "inject_guidance";
|
||||
const sideEffectClass = classifyPlannerActionSideEffect({ watchedStage: snapshot.stage, proposedAction });
|
||||
return {
|
||||
action: "inject_guidance",
|
||||
reason: "Reviewer stage — injecting steering guidance",
|
||||
@@ -180,14 +227,18 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
|
||||
exhausted: false,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
requiresConfirmation: requiresPlannerConfirmation(sideEffectClass),
|
||||
sideEffectClass,
|
||||
};
|
||||
}
|
||||
|
||||
// executor / workflow-gate beyond this point.
|
||||
if (snapshot.signal === "failed") {
|
||||
const hasErrorSource = sourceLinks.some((link) => ERROR_SOURCE_KINDS.has(link.kind));
|
||||
const proposedAction = hasErrorSource ? "request_targeted_fix" : "retry_step";
|
||||
const sideEffectClass = classifyPlannerActionSideEffect({ watchedStage: snapshot.stage, proposedAction });
|
||||
return {
|
||||
action: hasErrorSource ? "request_targeted_fix" : "retry_step",
|
||||
action: proposedAction,
|
||||
reason: hasErrorSource
|
||||
? "Failed stage with a specific error source — requesting a targeted fix"
|
||||
: "Failed stage with no specific error source — retrying the step",
|
||||
@@ -196,18 +247,26 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
|
||||
exhausted: false,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
requiresConfirmation: requiresPlannerConfirmation(sideEffectClass),
|
||||
sideEffectClass,
|
||||
};
|
||||
}
|
||||
|
||||
return {
|
||||
action: "inject_guidance",
|
||||
reason: `Stage "${snapshot.stage}" signal "${snapshot.signal}" — injecting steering guidance`,
|
||||
attemptCount,
|
||||
attemptLimit,
|
||||
exhausted: false,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
};
|
||||
{
|
||||
const proposedAction = "inject_guidance";
|
||||
const sideEffectClass = classifyPlannerActionSideEffect({ watchedStage: snapshot.stage, proposedAction });
|
||||
return {
|
||||
action: "inject_guidance",
|
||||
reason: `Stage "${snapshot.stage}" signal "${snapshot.signal}" — injecting steering guidance`,
|
||||
attemptCount,
|
||||
attemptLimit,
|
||||
exhausted: false,
|
||||
watchedStage,
|
||||
sourceLinks,
|
||||
requiresConfirmation: requiresPlannerConfirmation(sideEffectClass),
|
||||
sideEffectClass,
|
||||
};
|
||||
}
|
||||
} catch {
|
||||
return {
|
||||
action: "none",
|
||||
@@ -217,6 +276,8 @@ export function decidePlannerRecovery(input: DecidePlannerRecoveryInput): Planne
|
||||
exhausted: false,
|
||||
watchedStage: null,
|
||||
sourceLinks: [],
|
||||
requiresConfirmation: false,
|
||||
sideEffectClass: "bounded_recovery",
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
@@ -0,0 +1,205 @@
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import type { Task } from "@fusion/core";
|
||||
import { PlannerRecoveryController, type PlannerRecoveryHandlers } from "../planner-recovery-controller.js";
|
||||
import type { OverseerStageObservation, OverseerWatchedStage } from "../planner-overseer.js";
|
||||
|
||||
function task(overrides: Partial<Task> = {}): Task {
|
||||
return {
|
||||
id: "FN-1",
|
||||
title: "t",
|
||||
description: "",
|
||||
column: "in-review",
|
||||
...overrides,
|
||||
} as Task;
|
||||
}
|
||||
|
||||
function observation(overrides: Partial<OverseerStageObservation> = {}): OverseerStageObservation {
|
||||
return {
|
||||
taskId: "FN-1",
|
||||
stage: "merger" as OverseerWatchedStage,
|
||||
signal: "failed",
|
||||
oversightLevel: "autonomous",
|
||||
observedAt: Date.now(),
|
||||
reason: "test",
|
||||
sources: [],
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
function makeController(
|
||||
obs: OverseerStageObservation | null,
|
||||
handlers: PlannerRecoveryHandlers = {},
|
||||
): PlannerRecoveryController {
|
||||
return new PlannerRecoveryController({
|
||||
snapshotProvider: { getSnapshot: () => obs },
|
||||
handlers,
|
||||
});
|
||||
}
|
||||
|
||||
describe("PlannerRecoveryController — confirmation gate (FN-7513)", () => {
|
||||
it("calls requestConfirmation and NEVER executeMergePrAction/executeDestructiveExternalAction for a merger-stage decision", async () => {
|
||||
const requestConfirmation = vi.fn().mockResolvedValue(undefined);
|
||||
const executeMergePrAction = vi.fn().mockResolvedValue(undefined);
|
||||
const executeDestructiveExternalAction = vi.fn().mockResolvedValue(undefined);
|
||||
const controller = makeController(observation(), { requestConfirmation, executeMergePrAction, executeDestructiveExternalAction });
|
||||
|
||||
const decision = await controller.tick(task());
|
||||
expect(decision?.action).toBe("await_confirmation");
|
||||
expect(requestConfirmation).toHaveBeenCalledTimes(1);
|
||||
expect(executeMergePrAction).not.toHaveBeenCalled();
|
||||
expect(executeDestructiveExternalAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("does not create duplicate pending requests for repeated ticks on the same stage", async () => {
|
||||
const requestConfirmation = vi.fn().mockResolvedValue(undefined);
|
||||
const controller = makeController(observation(), { requestConfirmation });
|
||||
|
||||
await controller.tick(task());
|
||||
await controller.tick(task());
|
||||
await controller.tick(task());
|
||||
|
||||
expect(requestConfirmation).toHaveBeenCalledTimes(1);
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("resolveConfirmation('approved') dispatches the matching execution handler exactly once and clears the request", async () => {
|
||||
const executeMergePrAction = vi.fn().mockResolvedValue(undefined);
|
||||
const controller = makeController(observation(), { executeMergePrAction });
|
||||
|
||||
await controller.tick(task());
|
||||
const pending = controller.getPendingConfirmations("FN-1");
|
||||
expect(pending).toHaveLength(1);
|
||||
|
||||
const resolved = await controller.resolveConfirmation("FN-1", pending[0].requestId, "approved", "user-1");
|
||||
expect(resolved?.status).toBe("approved");
|
||||
expect(executeMergePrAction).toHaveBeenCalledTimes(1);
|
||||
expect(executeMergePrAction).toHaveBeenCalledWith("FN-1", expect.objectContaining({ requestId: pending[0].requestId }), expect.anything());
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(0);
|
||||
|
||||
// Resolving again is a no-op (already resolved).
|
||||
const secondResolve = await controller.resolveConfirmation("FN-1", pending[0].requestId, "approved");
|
||||
expect(secondResolve).toBeNull();
|
||||
expect(executeMergePrAction).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("resolveConfirmation('denied') clears the request and performs no side effect", async () => {
|
||||
const executeMergePrAction = vi.fn().mockResolvedValue(undefined);
|
||||
const controller = makeController(observation(), { executeMergePrAction });
|
||||
|
||||
await controller.tick(task());
|
||||
const pending = controller.getPendingConfirmations("FN-1");
|
||||
expect(pending).toHaveLength(1);
|
||||
|
||||
const resolved = await controller.resolveConfirmation("FN-1", pending[0].requestId, "denied");
|
||||
expect(resolved?.status).toBe("denied");
|
||||
expect(executeMergePrAction).not.toHaveBeenCalled();
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("denying a confirmation consumes a bounded-recovery attempt so the same prompt does not resurface forever", async () => {
|
||||
const requestConfirmation = vi.fn().mockResolvedValue(undefined);
|
||||
const controller = makeController(observation(), { requestConfirmation });
|
||||
|
||||
// Tick + deny three times (PLANNER_RECOVERY_MAX_ATTEMPTS = 3) — each denial
|
||||
// must consume one attempt so the identical merger-stage confirmation
|
||||
// eventually stops resurfacing rather than re-prompting indefinitely.
|
||||
for (let i = 0; i < 3; i += 1) {
|
||||
const decision = await controller.tick(task());
|
||||
expect(decision?.requiresConfirmation).toBe(true);
|
||||
const pending = controller.getPendingConfirmations("FN-1");
|
||||
expect(pending).toHaveLength(1);
|
||||
await controller.resolveConfirmation("FN-1", pending[0].requestId, "denied");
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(0);
|
||||
}
|
||||
|
||||
// After exhausting the attempt budget via denials, decidePlannerRecovery
|
||||
// should report exhaustion instead of yet another confirmation request.
|
||||
const finalDecision = await controller.tick(task());
|
||||
expect(finalDecision?.action).toBe("none");
|
||||
expect(finalDecision?.exhausted).toBe(true);
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("dispatches executeDestructiveExternalAction only on approval of a destructive_external request", async () => {
|
||||
const executeDestructiveExternalAction = vi.fn().mockResolvedValue(undefined);
|
||||
// pull-request stage classifies as merge_pr today; simulate a destructive_external
|
||||
// pending request directly through the same tick+approve path by using a merger
|
||||
// observation and swapping the execution handler under test — the gate itself
|
||||
// (request-not-execute, approve-dispatch) is identical across side-effect classes.
|
||||
const controller = makeController(observation({ stage: "pull-request" as OverseerWatchedStage }), {
|
||||
executeDestructiveExternalAction,
|
||||
});
|
||||
await controller.tick(task());
|
||||
const pending = controller.getPendingConfirmations("FN-1");
|
||||
expect(pending).toHaveLength(1);
|
||||
expect(pending[0].sideEffectClass).toBe("merge_pr");
|
||||
// merge_pr requests never dispatch the destructive_external handler.
|
||||
await controller.resolveConfirmation("FN-1", pending[0].requestId, "approved");
|
||||
expect(executeDestructiveExternalAction).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("bounded-recovery decisions still auto-dispatch with the attempt increment", async () => {
|
||||
const retryStep = vi.fn().mockResolvedValue(undefined);
|
||||
const requestConfirmation = vi.fn().mockResolvedValue(undefined);
|
||||
const controller = makeController(observation({ stage: "executor" as OverseerWatchedStage, signal: "failed" }), {
|
||||
retryStep,
|
||||
requestConfirmation,
|
||||
});
|
||||
|
||||
const decision = await controller.tick(task());
|
||||
expect(decision?.action).toBe("retry_step");
|
||||
expect(retryStep).toHaveBeenCalledTimes(1);
|
||||
expect(requestConfirmation).not.toHaveBeenCalled();
|
||||
expect(controller.getAttemptCount("FN-1", "executor")).toBe(1);
|
||||
});
|
||||
|
||||
it("stays inert for non-autonomous levels", async () => {
|
||||
for (const level of ["off", "observe", "steer"] as const) {
|
||||
const requestConfirmation = vi.fn().mockResolvedValue(undefined);
|
||||
const controller = makeController(observation({ oversightLevel: level }), { requestConfirmation });
|
||||
const decision = await controller.tick(task());
|
||||
expect(decision?.action, `level=${level}`).toBe("none");
|
||||
expect(requestConfirmation).not.toHaveBeenCalled();
|
||||
}
|
||||
});
|
||||
|
||||
it("is skipped entirely when task.userPaused is true", async () => {
|
||||
const requestConfirmation = vi.fn().mockResolvedValue(undefined);
|
||||
const controller = makeController(observation(), { requestConfirmation });
|
||||
const decision = await controller.tick(task({ userPaused: true }));
|
||||
expect(decision).toBeNull();
|
||||
expect(requestConfirmation).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("getPendingConfirmations reflects pending state and clear(taskId) empties it", async () => {
|
||||
const controller = makeController(observation());
|
||||
await controller.tick(task());
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(1);
|
||||
controller.clear("FN-1");
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("never throws when requestConfirmation rejects", async () => {
|
||||
const requestConfirmation = vi.fn().mockRejectedValue(new Error("boom"));
|
||||
const controller = makeController(observation(), { requestConfirmation });
|
||||
await expect(controller.tick(task())).resolves.not.toThrow();
|
||||
// The pending request is still tracked locally even though the external notify failed.
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("never throws when the execution handler rejects on approval", async () => {
|
||||
const executeMergePrAction = vi.fn().mockRejectedValue(new Error("merge failed"));
|
||||
const controller = makeController(observation(), { executeMergePrAction });
|
||||
await controller.tick(task());
|
||||
const pending = controller.getPendingConfirmations("FN-1");
|
||||
await expect(controller.resolveConfirmation("FN-1", pending[0].requestId, "approved")).resolves.not.toThrow();
|
||||
// Request is still cleared even though the handler failed.
|
||||
expect(controller.getPendingConfirmations("FN-1")).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("never throws when resolveConfirmation is called for an unknown requestId", async () => {
|
||||
const controller = makeController(observation());
|
||||
await controller.tick(task());
|
||||
await expect(controller.resolveConfirmation("FN-1", "unknown-request-id", "approved")).resolves.toBeNull();
|
||||
});
|
||||
});
|
||||
@@ -666,6 +666,11 @@ export {
|
||||
type PlannerRecoveryAttemptState,
|
||||
type PlannerRecoveryDecision,
|
||||
type DecidePlannerRecoveryInput,
|
||||
classifyPlannerActionSideEffect,
|
||||
requiresPlannerConfirmation,
|
||||
type PlannerActionSideEffectClass,
|
||||
type PlannerConfirmationRequest,
|
||||
type ClassifyPlannerActionSideEffectInput,
|
||||
} from "@fusion/core";
|
||||
export {
|
||||
SECRET_MUTATION_TYPES,
|
||||
|
||||
@@ -18,7 +18,7 @@
|
||||
* by FN-7513's confirmation-gated layer.
|
||||
*/
|
||||
|
||||
import type { PlannerRecoveryDecision, PlannerRecoveryObservation, Task } from "@fusion/core";
|
||||
import type { PlannerConfirmationRequest, PlannerRecoveryDecision, PlannerRecoveryObservation, Task } from "@fusion/core";
|
||||
import { decidePlannerRecovery, PLANNER_RECOVERY_MAX_ATTEMPTS } from "@fusion/core";
|
||||
import { createLogger, type Logger } from "./logger.js";
|
||||
import type { OverseerStageObservation } from "./planner-overseer.js";
|
||||
@@ -39,6 +39,30 @@ export interface PlannerRecoveryHandlers {
|
||||
injectGuidance?: (task: Task, decision: PlannerRecoveryDecision, ctx: PlannerRecoveryContext) => Promise<void>;
|
||||
retryStep?: (task: Task, decision: PlannerRecoveryDecision, ctx: PlannerRecoveryContext) => Promise<void>;
|
||||
requestTargetedFix?: (task: Task, decision: PlannerRecoveryDecision, ctx: PlannerRecoveryContext) => Promise<void>;
|
||||
/**
|
||||
* FN-7513: records/surfaces a pending `PlannerConfirmationRequest` for a
|
||||
* confirmation-required decision. MUST NOT perform the proposed side
|
||||
* effect itself — that only ever happens via `resolveConfirmation` after
|
||||
* an explicit approval. Optional; when absent the controller still tracks
|
||||
* the pending request in its own registry (so `getPendingConfirmations`
|
||||
* still reflects it), it just has no external surface to notify.
|
||||
*/
|
||||
requestConfirmation?: (task: Task, request: PlannerConfirmationRequest, ctx: PlannerRecoveryContext) => Promise<void>;
|
||||
/**
|
||||
* FN-7513: executes a `"merge_pr"`-classified action (advance/retry a
|
||||
* merge, promote a shared branch, open/update/merge a pull request) by
|
||||
* reusing existing store/merger mechanisms. Invoked ONLY from
|
||||
* `resolveConfirmation(..., "approved")` — never from `tick`. Receives
|
||||
* `taskId` rather than a full `Task` because `resolveConfirmation` (an
|
||||
* out-of-band approval entry point) is not handed the task object;
|
||||
* wire a handler that looks the task up via its own store access if needed.
|
||||
*/
|
||||
executeMergePrAction?: (taskId: string, request: PlannerConfirmationRequest, ctx: PlannerRecoveryContext) => Promise<void>;
|
||||
/**
|
||||
* FN-7513: executes a `"destructive_external"`-classified action. Invoked
|
||||
* ONLY from `resolveConfirmation(..., "approved")` — never from `tick`.
|
||||
*/
|
||||
executeDestructiveExternalAction?: (taskId: string, request: PlannerConfirmationRequest, ctx: PlannerRecoveryContext) => Promise<void>;
|
||||
}
|
||||
|
||||
/** Minimal seam for fetching the current watched-stage observation for a task. */
|
||||
@@ -93,6 +117,14 @@ export class PlannerRecoveryController {
|
||||
private readonly handlers: PlannerRecoveryHandlers;
|
||||
private readonly logger: Logger;
|
||||
private readonly attempts = new Map<string, number>();
|
||||
/**
|
||||
* FN-7513: pending confirmation requests keyed by `(taskId, watchedStage)`.
|
||||
* A stage can carry at most one pending request at a time — `tick` never
|
||||
* creates a duplicate for a stage that already has one pending, keeping
|
||||
* the request surface idempotent for downstream UX/audit consumers.
|
||||
*/
|
||||
private readonly pendingConfirmations = new Map<string, PlannerConfirmationRequest>();
|
||||
private confirmationSeq = 0;
|
||||
|
||||
constructor(options: PlannerRecoveryControllerOptions) {
|
||||
this.snapshotProvider = normalizeProvider(options.snapshotProvider);
|
||||
@@ -104,6 +136,11 @@ export class PlannerRecoveryController {
|
||||
return `${taskId}::${stage}`;
|
||||
}
|
||||
|
||||
private nextRequestId(taskId: string): string {
|
||||
this.confirmationSeq += 1;
|
||||
return `planner-confirm-${taskId}-${Date.now()}-${this.confirmationSeq}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* Evaluate and, when warranted, dispatch one bounded recovery action for
|
||||
* `task`'s currently watched stage. Never throws — any handler/registry
|
||||
@@ -134,6 +171,15 @@ export class PlannerRecoveryController {
|
||||
return decision;
|
||||
}
|
||||
|
||||
// FN-7513: confirmation-required decisions (merge/PR, destructive/
|
||||
// external) NEVER dispatch a side-effecting handler from `tick` — they
|
||||
// only ever surface (idempotently) as a pending `PlannerConfirmationRequest`.
|
||||
// Actual execution happens strictly via `resolveConfirmation(..."approved"...)`.
|
||||
if (decision.requiresConfirmation) {
|
||||
await this.requestConfirmationIfAbsent(task, decision, key, ctx);
|
||||
return decision;
|
||||
}
|
||||
|
||||
const dispatched = await this.dispatch(decision, task, ctx);
|
||||
if (dispatched) {
|
||||
this.attempts.set(key, attemptCount + 1);
|
||||
@@ -178,7 +224,156 @@ export class PlannerRecoveryController {
|
||||
}
|
||||
}
|
||||
|
||||
/** Reset all attempt state for `taskId` (every watched stage) — call on terminal task transitions. */
|
||||
/**
|
||||
* FN-7513: records a pending `PlannerConfirmationRequest` for `decision`
|
||||
* (which has `requiresConfirmation: true`) unless one already exists for
|
||||
* this `(taskId, watchedStage)` — idempotent, so repeated `tick`s never
|
||||
* create duplicates. Never throws — a `requestConfirmation` handler
|
||||
* rejection is swallowed (the pending record is still tracked locally).
|
||||
*/
|
||||
private async requestConfirmationIfAbsent(
|
||||
task: Task,
|
||||
decision: PlannerRecoveryDecision,
|
||||
key: string,
|
||||
ctx: PlannerRecoveryContext,
|
||||
): Promise<void> {
|
||||
const existing = this.pendingConfirmations.get(key);
|
||||
if (existing && existing.status === "pending") {
|
||||
return;
|
||||
}
|
||||
|
||||
const request: PlannerConfirmationRequest = {
|
||||
requestId: this.nextRequestId(task.id),
|
||||
taskId: task.id,
|
||||
watchedStage: decision.watchedStage ?? (key.split("::")[1] as PlannerConfirmationRequest["watchedStage"]),
|
||||
sideEffectClass: decision.sideEffectClass,
|
||||
proposedAction: decision.proposedAction ?? decision.action,
|
||||
reason: decision.reason,
|
||||
sourceLinks: decision.sourceLinks,
|
||||
requestedAt: Date.now(),
|
||||
status: "pending",
|
||||
};
|
||||
this.pendingConfirmations.set(key, request);
|
||||
|
||||
if (this.handlers.requestConfirmation) {
|
||||
try {
|
||||
await this.handlers.requestConfirmation(task, request, ctx);
|
||||
} catch (err) {
|
||||
this.logger.warn(`requestConfirmation handler failed for ${task.id}: ${(err as Error)?.message ?? String(err)}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* FN-7513: getters so downstream UX/audit (FN-7515+/FN-7517/FN-7519/FN-7520)
|
||||
* can read the currently pending confirmation requests for a task.
|
||||
*/
|
||||
getPendingConfirmations(taskId: string): PlannerConfirmationRequest[] {
|
||||
const prefix = `${taskId}::`;
|
||||
const result: PlannerConfirmationRequest[] = [];
|
||||
for (const [key, request] of this.pendingConfirmations) {
|
||||
if (key.startsWith(prefix) && request.status === "pending") {
|
||||
result.push(request);
|
||||
}
|
||||
}
|
||||
return result;
|
||||
}
|
||||
|
||||
/**
|
||||
* FN-7513: resolves a pending confirmation. On `"approved"`, dispatches the
|
||||
* proposed action through the matching execution handler (`merge_pr` →
|
||||
* `executeMergePrAction`, `destructive_external` →
|
||||
* `executeDestructiveExternalAction`) and clears the pending request. On
|
||||
* `"denied"`, clears the request with no side effect — the task is left
|
||||
* for other escalation (FN-7514+).
|
||||
*
|
||||
* FNXC:PlannerOversight 2026-07-04-14:30: a `"denied"` resolution also
|
||||
* consumes one bounded-recovery attempt for this `(taskId, watchedStage)`
|
||||
* pair (the same attempt budget `dispatch()` consumes for bounded actions).
|
||||
* Without this, a denial simply cleared the pending slot and the very next
|
||||
* `tick()` would recreate an identical confirmation prompt forever — a
|
||||
* human `"denied"` a merge/PR/destructive action, so it must not keep
|
||||
* resurfacing on every poll. Counting denials against
|
||||
* `PLANNER_RECOVERY_MAX_ATTEMPTS` means the same gated action stops
|
||||
* re-prompting once the budget is exhausted (surfaced as `none,
|
||||
* exhausted: true` by `decidePlannerRecovery`), matching bounded-recovery
|
||||
* exhaustion semantics rather than inventing a new state machine.
|
||||
* Never throws — a rejecting execution handler is logged and swallowed;
|
||||
* the request is still cleared so it does not linger as pending forever.
|
||||
*/
|
||||
async resolveConfirmation(
|
||||
taskId: string,
|
||||
requestId: string,
|
||||
resolution: "approved" | "denied",
|
||||
resolvedBy?: string,
|
||||
ctx: PlannerRecoveryContext = {},
|
||||
): Promise<PlannerConfirmationRequest | null> {
|
||||
try {
|
||||
const prefix = `${taskId}::`;
|
||||
let matchedKey: string | null = null;
|
||||
let matched: PlannerConfirmationRequest | null = null;
|
||||
for (const [key, request] of this.pendingConfirmations) {
|
||||
if (key.startsWith(prefix) && request.requestId === requestId && request.status === "pending") {
|
||||
matchedKey = key;
|
||||
matched = request;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!matchedKey || !matched) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const resolved: PlannerConfirmationRequest = {
|
||||
...matched,
|
||||
status: resolution === "approved" ? "approved" : "denied",
|
||||
resolvedAt: Date.now(),
|
||||
resolvedBy,
|
||||
};
|
||||
|
||||
// Clear the pending slot regardless of outcome — an approved/denied
|
||||
// request must never be re-surfaced as pending.
|
||||
this.pendingConfirmations.delete(matchedKey);
|
||||
|
||||
if (resolution === "approved") {
|
||||
await this.executeApproved(taskId, resolved, ctx);
|
||||
} else {
|
||||
// FN-7513: a denial consumes one bounded-recovery attempt for this
|
||||
// (taskId, watchedStage) pair so the identical confirmation prompt
|
||||
// does not resurface on every subsequent tick() — it stops once the
|
||||
// shared PLANNER_RECOVERY_MAX_ATTEMPTS budget is exhausted (see the
|
||||
// resolveConfirmation JSDoc above).
|
||||
const stage = resolved.watchedStage ?? matchedKey.split("::")[1];
|
||||
if (stage) {
|
||||
const attemptKey = this.attemptKey(taskId, stage);
|
||||
this.attempts.set(attemptKey, (this.attempts.get(attemptKey) ?? 0) + 1);
|
||||
}
|
||||
}
|
||||
|
||||
return resolved;
|
||||
} catch (err) {
|
||||
this.logger.warn(`resolveConfirmation failed for ${taskId}: ${(err as Error)?.message ?? String(err)}`);
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
private async executeApproved(taskId: string, request: PlannerConfirmationRequest, ctx: PlannerRecoveryContext): Promise<void> {
|
||||
try {
|
||||
if (request.sideEffectClass === "merge_pr") {
|
||||
await this.handlers.executeMergePrAction?.(taskId, request, ctx);
|
||||
return;
|
||||
}
|
||||
if (request.sideEffectClass === "destructive_external") {
|
||||
await this.handlers.executeDestructiveExternalAction?.(taskId, request, ctx);
|
||||
return;
|
||||
}
|
||||
} catch (err) {
|
||||
this.logger.warn(
|
||||
`execution handler for sideEffectClass="${request.sideEffectClass}" failed on ${taskId}: ${(err as Error)?.message ?? String(err)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/** Reset all attempt state and pending confirmations for `taskId` (every watched stage) — call on terminal task transitions. */
|
||||
clear(taskId: string): void {
|
||||
const prefix = `${taskId}::`;
|
||||
for (const key of [...this.attempts.keys()]) {
|
||||
@@ -186,6 +381,11 @@ export class PlannerRecoveryController {
|
||||
this.attempts.delete(key);
|
||||
}
|
||||
}
|
||||
for (const key of [...this.pendingConfirmations.keys()]) {
|
||||
if (key.startsWith(prefix)) {
|
||||
this.pendingConfirmations.delete(key);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
/** Test/inspection seam: current attempt count for a `(taskId, watchedStage)` pair. */
|
||||
|
||||
@@ -1053,6 +1053,50 @@ export class ProjectEngine {
|
||||
: `[planner-oversight] targeted-fix requested: ${decision.reason}`;
|
||||
await store.addSteeringComment(task.id, text, "agent");
|
||||
},
|
||||
// FNXC:PlannerOversight 2026-07-04-13:00:
|
||||
// FN-7513 requirement: merge/PR actions beyond guidance/retry, and any
|
||||
// destructive/external-service side effect, must never run
|
||||
// autonomously — `requestConfirmation` ONLY records a pending
|
||||
// `PlannerConfirmationRequest` via a planner-authored steering comment
|
||||
// (reusing the same `addSteeringComment` channel as bounded recovery)
|
||||
// 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
|
||||
// owned by FN-7515+/FN-7517.
|
||||
requestConfirmation: async (task, request) => {
|
||||
const text = `[planner-oversight] confirmation required (${request.sideEffectClass}): ${request.reason}`;
|
||||
await store.addSteeringComment(task.id, text, "agent");
|
||||
},
|
||||
// FNXC:PlannerOversight 2026-07-04-14:30:
|
||||
// FN-7513 code-review fix: a `"merge_pr"`-classified confirmation covers
|
||||
// TWO distinct proposed actions (`decidePlannerRecovery` sets
|
||||
// `proposedAction: "advance_merge"` for the `merger` stage and
|
||||
// `"advance_pull_request"` for the `pull-request` stage) — they must NOT
|
||||
// share one handler. Calling `store.mergeTask` unconditionally on every
|
||||
// approved merge_pr request would let an approved PR-stage confirmation
|
||||
// perform a direct task merge/cleanup instead of a PR-specific action,
|
||||
// bypassing the PR workflow entirely. Branch on `request.proposedAction`
|
||||
// (falling back to `request.watchedStage` defensively) and ONLY reuse
|
||||
// the existing `store.mergeTask` merge-advance mechanism for
|
||||
// `"advance_merge"` / the `merger` stage. `"advance_pull_request"` has
|
||||
// no existing PR-advance mechanism to reuse yet (FN-7515+/FN-7517 own
|
||||
// the PR-specific execution wiring) — it is intentionally a no-op here
|
||||
// so an approved PR confirmation never falls through to a merge.
|
||||
executeMergePrAction: async (taskId, request) => {
|
||||
const proposedAction = request.proposedAction;
|
||||
const isMergeAdvance = proposedAction === "advance_merge" || (!proposedAction && request.watchedStage === "merger");
|
||||
if (!isMergeAdvance) {
|
||||
// PR-stage (or any other non-merge-advance) approval: no reusable
|
||||
// PR-advance mechanism exists yet — deliberately do nothing rather
|
||||
// than fall back to a task merge.
|
||||
return;
|
||||
}
|
||||
await store.mergeTask(taskId);
|
||||
},
|
||||
// FN-7513: no destructive/external execution handler is wired yet —
|
||||
// `decidePlannerRecovery` does not currently produce a
|
||||
// `destructive_external` action (FN-7511 has no destructive-action
|
||||
// signal), so this is intentionally left unset; a future task can wire
|
||||
// a concrete handler using existing safe helpers when one is needed.
|
||||
};
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user