FN-7736: prevent recovery/oversight from resuming approval-blocked tasks
Introduces a canonical awaiting-approval pause reason and predicate so recovery and oversight paths treat approval-blocked tasks as terminal-until-approved instead of eligible for early rebound. - Add isTaskBlockedOnApproval predicate and canonical "awaiting-approval" pause reason in @fusion/core (store.ts, task-merge.ts, index.ts/index.gate.ts) - Exclude approval-blocked tasks from paused-scope-decay rebound in self-healing.ts - Keep the planner overseer withholding oversight for approval-blocked tasks (overseer-human-control-policy.ts) - Executor and agent-heartbeat now recognize the approval-blocked state and avoid resuming it - Add regression tests across store-persistence, task-merge, overseer-human-control-policy, paused-scope-decay, and self-healing-paused-abort-recovery - Update docs/architecture.md with the new approval-hold invariant - Add changeset fn-7736-approval-hold.md (patch) Files changed: .changeset/fn-7736-approval-hold.md | 7 +++ docs/architecture.md | 64 ++++++++++++++++++++-- .../core/src/__tests__/store-persistence.test.ts | 18 ++++++ packages/core/src/__tests__/task-merge.test.ts | 34 ++++++++++++ packages/core/src/index.gate.ts | 2 + packages/core/src/index.ts | 2 + packages/core/src/store.ts | 18 +++++- packages/core/src/task-merge.ts | 34 ++++++++++++ .../executor-approval-gate-suspend.test.ts | 5 +- .../src/__tests__/heartbeat-executor.test.ts | 5 +- .../overseer-human-control-policy.test.ts | 44 +++++++++++++++ .../paused-scope-decay.test.ts | 44 +++++++++++++++ .../self-healing-paused-abort-recovery.test.ts | 21 +++++++ packages/engine/src/agent-heartbeat.ts | 8 ++- packages/engine/src/executor.ts | 13 ++++- .../engine/src/overseer-human-control-policy.ts | 45 +++++++++++---- packages/engine/src/self-healing.ts | 12 +++- 17 files changed, 351 insertions(+), 25 deletions(-) Fusion-Task-Id: FN-7736 Fusion-Task-Lineage: 67e05b7f-f621-4f9b-bc01-721ff05d715b Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-7736-approval-hold.md
Normal file
7
.changeset/fn-7736-approval-hold.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Recovery and oversight now wait for approval-blocked tasks instead of resuming them early.
|
||||
category: fix
|
||||
dev: Adds canonical `awaiting-approval` pause reason + `isTaskBlockedOnApproval` predicate; excludes the hold from paused-scope-decay rebound and keeps the planner overseer withholding (FN-7736).
|
||||
@@ -1540,20 +1540,27 @@ are implemented elsewhere.
|
||||
FN-7514 supplies the comprehensive human-control safeguard the FN-7512/FN-7513 layers deferred: the
|
||||
overseer must be fully inert — no steering, retry, targeted-fix, or FN-7513 confirmation-required action
|
||||
(merge/PR progression, destructive/external-service side effect) may fire, and no pending confirmation
|
||||
may even be recorded — whenever a task is (a) user-paused, or (b) ineligible for auto-merge processing
|
||||
per the FN-5147 `autoMerge:false` / PR-based human-review terminal contract.
|
||||
may even be recorded — whenever a task is (a) user-paused, (b) blocked on a pending human approval
|
||||
decision (FN-7736), or (c) ineligible for auto-merge processing per the FN-5147 `autoMerge:false` /
|
||||
PR-based human-review terminal contract.
|
||||
|
||||
`packages/engine/src/overseer-human-control-policy.ts` exports the pure predicate
|
||||
`evaluateOverseerHumanControl(task, settings)` (no I/O, mirrors the `recovery-policy.ts` style), returning
|
||||
`{ withhold: boolean; reason?: "user-paused" | "auto-merge-off-human-review" }`. It reuses
|
||||
`allowsAutoMergeProcessing` from `@fusion/core` VERBATIM for the auto-merge-off half — never re-derives
|
||||
the predicate inline. For the pause half, it distinguishes:
|
||||
`{ withhold: boolean; reason?: "user-paused" | "approval-blocked" | "auto-merge-off-human-review" }`. It
|
||||
reuses `allowsAutoMergeProcessing` from `@fusion/core` VERBATIM for the auto-merge-off half, and
|
||||
`isTaskBlockedOnApproval` from `@fusion/core` VERBATIM for the approval-hold half (checked FIRST, ahead of
|
||||
the user-pause/auto-merge-off checks — see the approval-hold contract below) — never re-derives either
|
||||
predicate inline. For the pause half, it distinguishes:
|
||||
|
||||
- Explicit user pause: `task.userPaused === true`, OR `task.paused === true` with NO `task.pausedReason`
|
||||
(the `fn_task_pause` tool / `TaskStore.pauseTask` never stamps a `pausedReason`).
|
||||
- Engine/self-healing park (NOT user pause): `task.paused === true` WITH a `pausedReason` (every
|
||||
self-healing park path — branch-conflict-unrecoverable, token_budget_exceeded,
|
||||
in-review-stall-deadlock, worktrunk_operation_failed, etc. — always stamps one).
|
||||
- Approval hold (also NOT user pause, checked before the user-pause branch above so it is never
|
||||
shadowed): `task.paused === true` WITH `task.pausedReason === "awaiting-approval"` (the canonical
|
||||
`AWAITING_APPROVAL_PAUSE_REASON`), OR `task.status === "awaiting-approval"`. See the approval-hold
|
||||
contract section below for the full rationale.
|
||||
|
||||
`PlannerRecoveryController.tick()` (`planner-recovery-controller.ts`) consults this guard FIRST — before
|
||||
the snapshot lookup, before `decidePlannerRecovery`, before FN-7513's confirmation classification. When
|
||||
@@ -1574,6 +1581,53 @@ same settings self-healing already gates lifecycle mutation on.
|
||||
**Downstream ownership (not this layer):** the dashboard UI/badges surfacing withheld state (FN-7515+),
|
||||
a persisted intervention timeline (FN-7519), and richer run-audit/activity presentation (FN-7520).
|
||||
|
||||
### Approval-hold contract for recovery and oversight (FN-7736)
|
||||
|
||||
A task can be blocked awaiting a human approval decision via two distinct mechanisms, and every
|
||||
automated recovery (self-healing) and oversight (planner overseer) path must refuse to rebound, requeue,
|
||||
resume, re-plan, or otherwise advance a task while either hold is active:
|
||||
|
||||
1. **Triage plan-approval gate** — parks the task spec at `status: "awaiting-approval"`. Already a member
|
||||
of `HARD_BLOCKING_TASK_STATUSES` (`packages/core/src/task-merge.ts`) and preserved by
|
||||
`GHOST_REVIEW_PRESERVED_STATUSES` in `self-healing.ts`; `POST /api/tasks/:id/approve-plan` is the only
|
||||
route that clears it.
|
||||
2. **Runtime tool-approval gate** — a gated tool call parks a RUNNING task via `pauseForApproval`
|
||||
(`executor.ts` and `agent-heartbeat.ts`), which calls `store.pauseTask(taskId, true, ..., {
|
||||
pausedByAgentId, pausedReason: AWAITING_APPROVAL_PAUSE_REASON })`. Before FN-7736 this call stamped
|
||||
only `paused: true` with NO durable `pausedReason`, so recovery/oversight code keying on
|
||||
`pausedReason` could not recognize the hold, and self-healing's `autoReboundPausedScopeDecay` could
|
||||
rebound the held task back to `todo` (defeating the approval gate) once a follower task's scope-decay
|
||||
threshold elapsed.
|
||||
|
||||
`packages/core/src/task-merge.ts` exports the canonical `AWAITING_APPROVAL_PAUSE_REASON =
|
||||
"awaiting-approval"` constant and the shared pure predicate `isTaskBlockedOnApproval(task)`, which returns
|
||||
`true` for EITHER hold shape (`task.paused === true && task.pausedReason ===
|
||||
AWAITING_APPROVAL_PAUSE_REASON`, or `task.status === "awaiting-approval"`). Both are re-exported via
|
||||
`index.ts`/`index.gate.ts` for engine consumption. Consulted at:
|
||||
|
||||
- **`SelfHealingManager.PAUSED_SCOPE_DECAY_EXCLUDED_REASONS`** (`self-healing.ts`) — includes
|
||||
`AWAITING_APPROVAL_PAUSE_REASON` alongside `branch-conflict-unrecoverable`, `worktrunk_operation_failed`,
|
||||
and `token_budget_exceeded`, so `autoReboundPausedScopeDecay` skips an approval-held holder task even
|
||||
when a follower is present and the age threshold has elapsed.
|
||||
- **`classifyPausedAbortWorkflowRecovery`** (`self-healing.ts`) — already skips any `task.paused === true`
|
||||
row before this task; the approval-hold shape is covered by that generic guard (regression-tested
|
||||
explicitly rather than relying on the incidental coverage).
|
||||
- **`evaluateOverseerHumanControl`** (`overseer-human-control-policy.ts`) — checks `isTaskBlockedOnApproval`
|
||||
FIRST, ahead of the accidental "paused with no reason" user-pause heuristic, returning
|
||||
`{ withhold: true, reason: "approval-blocked" }`. This ordering is load-bearing: once FN-7736 started
|
||||
stamping a durable `pausedReason` on the tool-approval hold, the old no-reason heuristic would stop
|
||||
matching it — the explicit `approval-blocked` branch prevents that from silently flipping the overseer
|
||||
from withhold to act.
|
||||
- **Every other self-healing sweep** that gates on generic `task.paused === true` (or `task.paused ||
|
||||
task.userPaused`) before taking a lifecycle-advancing action already skips an approval-held task
|
||||
regardless of `pausedReason`, and needed no change.
|
||||
|
||||
`TaskStore.pauseTask(id, paused, runContext?, agentOptions?)` accepts an optional `agentOptions.pausedReason`
|
||||
seam (widened for FN-7736) so the durable reason lands atomically with the pause write, avoiding a second
|
||||
racy `updateTask` call; unpausing (`pauseTask(id, false)`, used by the approval-resolution route) clears
|
||||
the caller-supplied `pausedReason` the same way it already clears `pausedByAgentId`/`userPaused`, so the
|
||||
hold is not sticky once the operator decides.
|
||||
|
||||
### Planner overseer runtime-state exposure (FN-7531)
|
||||
|
||||
/*
|
||||
|
||||
@@ -416,6 +416,24 @@ describe("TaskStore", () => {
|
||||
expect(updated.paused).toBe(true);
|
||||
expect(updated.pausedByAgentId).toBe("agent-other");
|
||||
});
|
||||
|
||||
// FN-7736: pauseTask's agentOptions.pausedReason seam durably stamps WHY a
|
||||
// task was paused (e.g. the canonical awaiting-approval reason) and the
|
||||
// reason is cleared on unpause, matching pausedByAgentId/userPaused.
|
||||
it("stamps pausedReason via agentOptions and clears it on unpause", async () => {
|
||||
const task = await harness.store().createTask({ description: "Approval-held task" });
|
||||
const paused = await harness.store().pauseTask(task.id, true, undefined, { pausedByAgentId: "agent-1", pausedReason: "awaiting-approval" });
|
||||
|
||||
expect(paused.paused).toBe(true);
|
||||
expect(paused.pausedReason).toBe("awaiting-approval");
|
||||
|
||||
const detail = await harness.store().getTask(task.id);
|
||||
expect(detail.pausedReason).toBe("awaiting-approval");
|
||||
|
||||
const unpaused = await harness.store().pauseTask(task.id, false);
|
||||
expect(unpaused.paused).toBeFalsy();
|
||||
expect(unpaused.pausedReason).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
describe("branch field persistence", () => {
|
||||
|
||||
@@ -5,6 +5,8 @@ import {
|
||||
HARD_BLOCKING_TASK_STATUSES,
|
||||
SCHEDULER_TRANSIENT_STATUSES,
|
||||
TASK_DONE_BYPASS_BLOCKER_MESSAGE,
|
||||
AWAITING_APPROVAL_PAUSE_REASON,
|
||||
isTaskBlockedOnApproval,
|
||||
getTaskCompletionBlocker,
|
||||
getTaskDoneBypassBlocker,
|
||||
getTaskHardMergeBlocker,
|
||||
@@ -875,3 +877,35 @@ describe("getTaskCompletionBlocker", () => {
|
||||
.resolves.toBe("task has unresolved dependencies: FN-999");
|
||||
});
|
||||
});
|
||||
|
||||
// FN-7736: isTaskBlockedOnApproval covers both approval-hold shapes (pause-reason
|
||||
// and awaiting-approval status) and must not false-positive on a bare user pause.
|
||||
describe("isTaskBlockedOnApproval", () => {
|
||||
it("is true when paused with the canonical approval pause reason", () => {
|
||||
expect(isTaskBlockedOnApproval({ paused: true, pausedReason: AWAITING_APPROVAL_PAUSE_REASON, status: undefined })).toBe(true);
|
||||
});
|
||||
|
||||
it("is true when status is awaiting-approval regardless of paused", () => {
|
||||
expect(isTaskBlockedOnApproval({ paused: false, pausedReason: undefined, status: "awaiting-approval" })).toBe(true);
|
||||
});
|
||||
|
||||
it("is true when both the pause-reason and status shapes are present", () => {
|
||||
expect(isTaskBlockedOnApproval({ paused: true, pausedReason: AWAITING_APPROVAL_PAUSE_REASON, status: "awaiting-approval" })).toBe(true);
|
||||
});
|
||||
|
||||
it("is false for a task with neither hold shape", () => {
|
||||
expect(isTaskBlockedOnApproval({ paused: false, pausedReason: undefined, status: undefined })).toBe(false);
|
||||
});
|
||||
|
||||
it("is false for a bare user pause (paused true, no reason) — must not conflate with approval hold", () => {
|
||||
expect(isTaskBlockedOnApproval({ paused: true, pausedReason: undefined, status: undefined })).toBe(false);
|
||||
});
|
||||
|
||||
it("is false when paused with a different (non-approval) pause reason", () => {
|
||||
expect(isTaskBlockedOnApproval({ paused: true, pausedReason: "branch-conflict-unrecoverable", status: undefined })).toBe(false);
|
||||
});
|
||||
|
||||
it("is false when pausedReason is the approval reason but paused is not true", () => {
|
||||
expect(isTaskBlockedOnApproval({ paused: false, pausedReason: AWAITING_APPROVAL_PAUSE_REASON, status: undefined })).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -897,6 +897,8 @@ export {
|
||||
resolveEffectiveAutoMerge,
|
||||
resolveEffectiveGroupAutoMerge,
|
||||
resolveTaskMergeTarget,
|
||||
AWAITING_APPROVAL_PAUSE_REASON,
|
||||
isTaskBlockedOnApproval,
|
||||
type MergeTargetResolution,
|
||||
type MergeTargetResolverOptions,
|
||||
} from "./task-merge.js";
|
||||
|
||||
@@ -906,6 +906,8 @@ export {
|
||||
resolveEffectiveAutoMerge,
|
||||
resolveEffectiveGroupAutoMerge,
|
||||
resolveTaskMergeTarget,
|
||||
AWAITING_APPROVAL_PAUSE_REASON,
|
||||
isTaskBlockedOnApproval,
|
||||
type MergeTargetResolution,
|
||||
type MergeTargetResolverOptions,
|
||||
} from "./task-merge.js";
|
||||
|
||||
@@ -9693,11 +9693,23 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
* Pause or unpause a task. Paused tasks are excluded from all automated
|
||||
* agent and scheduler interaction. Logs the action and emits `task:updated`.
|
||||
*/
|
||||
/*
|
||||
* FNXC:ApprovalHold 2026-07-09-00:05:
|
||||
* FN-7736: `agentOptions.pausedReason` is the minimal seam for durably
|
||||
* stamping WHY a task was paused (e.g. the canonical
|
||||
* `AWAITING_APPROVAL_PAUSE_REASON` from a tool-approval gate). Widening
|
||||
* this existing options bag avoids a second, racy `updateTask` write right
|
||||
* after `pauseTask` — the reason lands atomically with the pause itself.
|
||||
* On unpause the caller-supplied reason is cleared here (mirroring how
|
||||
* `pausedByAgentId`/`userPaused` are already cleared below); sweep-set
|
||||
* built-in reasons like `branch-conflict-unrecoverable` are cleared by
|
||||
* their own dedicated resume code paths and are unaffected.
|
||||
*/
|
||||
async pauseTask(
|
||||
id: string,
|
||||
paused: boolean,
|
||||
runContext?: RunMutationContext,
|
||||
agentOptions?: { pausedByAgentId?: string },
|
||||
agentOptions?: { pausedByAgentId?: string; pausedReason?: string },
|
||||
): Promise<Task> {
|
||||
return this.withTaskLock(id, async () => {
|
||||
const dir = this.taskDir(id);
|
||||
@@ -9713,9 +9725,13 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
if (paused && agentOptions?.pausedByAgentId) {
|
||||
task.pausedByAgentId = agentOptions.pausedByAgentId;
|
||||
}
|
||||
if (paused && agentOptions?.pausedReason) {
|
||||
task.pausedReason = agentOptions.pausedReason;
|
||||
}
|
||||
if (!paused) {
|
||||
task.pausedByAgentId = undefined;
|
||||
task.userPaused = undefined;
|
||||
task.pausedReason = undefined;
|
||||
}
|
||||
// When pausing an in-progress/in-review task, set status so the UI can show the state.
|
||||
// When unpausing, clear the "paused" status.
|
||||
|
||||
@@ -141,6 +141,40 @@ export function resolveTaskMergeTarget(
|
||||
return { branch: legacyFallback, source: "legacy-main", rejected };
|
||||
}
|
||||
|
||||
/*
|
||||
* FNXC:ApprovalHold 2026-07-09-00:00:
|
||||
* FN-7736: two distinct mechanisms park a task on a pending human approval —
|
||||
* (1) the triage plan-approval gate sets `task.status === "awaiting-approval"`
|
||||
* (already a HARD_BLOCKING_TASK_STATUSES member below), and (2) a gated tool
|
||||
* call parks a RUNNING task via `pauseForApproval` -> `store.pauseTask(id,
|
||||
* true, ...)`, which historically only set `paused:true` with no durable
|
||||
* `pausedReason`, so recovery/oversight code keying on `pausedReason` could
|
||||
* not recognize it and at least one sweep (self-healing's
|
||||
* `autoReboundPausedScopeDecay`) could rebound the held task back to `todo`
|
||||
* before the operator ever decided. `AWAITING_APPROVAL_PAUSE_REASON` is the
|
||||
* canonical, durable marker both `executor.ts` and `agent-heartbeat.ts`
|
||||
* `pauseForApproval` now stamp via `TaskStore.pauseTask`'s `pausedReason`
|
||||
* option, and `isTaskBlockedOnApproval` is the single shared predicate core
|
||||
* and engine code must consult before rebounding, requeuing, resuming,
|
||||
* re-planning, or otherwise advancing a task — it must return `true` for
|
||||
* EITHER hold shape so callers never have to special-case which mechanism
|
||||
* parked the task.
|
||||
*/
|
||||
export const AWAITING_APPROVAL_PAUSE_REASON = "awaiting-approval";
|
||||
|
||||
/**
|
||||
* Returns true when `task` is blocked on a pending human approval decision,
|
||||
* via either hold mechanism (see FNXC:ApprovalHold above). Every automated
|
||||
* recovery (self-healing) and oversight (planner overseer) path must treat
|
||||
* `true` as "take no lifecycle-advancing action on this task".
|
||||
*/
|
||||
export function isTaskBlockedOnApproval(
|
||||
task: Pick<Task, "paused" | "pausedReason" | "status">,
|
||||
): boolean {
|
||||
if (task.paused === true && task.pausedReason === AWAITING_APPROVAL_PAUSE_REASON) return true;
|
||||
return task.status === "awaiting-approval";
|
||||
}
|
||||
|
||||
export const HARD_BLOCKING_TASK_STATUSES = new Set([
|
||||
"failed",
|
||||
// ── User-attention / awaiting-handoff states ─────────────────────────
|
||||
|
||||
@@ -61,7 +61,10 @@ describe("TaskExecutor.buildActionGateContext pauseForApproval", () => {
|
||||
|
||||
await gateContext.pauseForApproval({ approvalRequestId: "apr-1", decision });
|
||||
|
||||
expect(store.pauseTask).toHaveBeenCalledWith("FN-1", true, undefined, expect.objectContaining({ pausedByAgentId: expect.any(String) }));
|
||||
// FN-7736: pauseForApproval must durably stamp the canonical
|
||||
// AWAITING_APPROVAL_PAUSE_REASON so recovery/oversight code can recognize
|
||||
// this hold via isTaskBlockedOnApproval (not just paused:true).
|
||||
expect(store.pauseTask).toHaveBeenCalledWith("FN-1", true, undefined, expect.objectContaining({ pausedByAgentId: expect.any(String), pausedReason: "awaiting-approval" }));
|
||||
expect(store.logEntry).toHaveBeenCalled();
|
||||
// Session suspension must be triggered synchronously (called, not merely
|
||||
// scheduled for some later tick) as part of this same pauseForApproval
|
||||
|
||||
@@ -758,7 +758,10 @@ describe("executeHeartbeat", () => {
|
||||
},
|
||||
});
|
||||
|
||||
expect(pauseTask).toHaveBeenCalledWith("FN-001", true, undefined, { pausedByAgentId: "agent-001" });
|
||||
// FN-7736: heartbeat pauseForApproval must mirror executor.ts and stamp
|
||||
// the canonical AWAITING_APPROVAL_PAUSE_REASON on the task, not just the
|
||||
// agent's pauseReason.
|
||||
expect(pauseTask).toHaveBeenCalledWith("FN-001", true, undefined, { pausedByAgentId: "agent-001", pausedReason: "awaiting-approval" });
|
||||
expect((store.updateAgentState as any)).toHaveBeenCalledWith("agent-001", "paused");
|
||||
expect((store.updateAgent as any)).toHaveBeenCalledWith("agent-001", { pauseReason: "awaiting-approval" });
|
||||
});
|
||||
|
||||
@@ -10,6 +10,7 @@ function task(overrides: Partial<OverseerHumanControlTask> = {}): OverseerHumanC
|
||||
userPaused: undefined,
|
||||
paused: undefined,
|
||||
pausedReason: undefined,
|
||||
status: undefined,
|
||||
autoMerge: undefined,
|
||||
prInfo: undefined,
|
||||
prInfos: undefined,
|
||||
@@ -83,4 +84,47 @@ describe("evaluateOverseerHumanControl", () => {
|
||||
const decision = evaluateOverseerHumanControl(task({ userPaused: true }), settings({ autoMerge: false }));
|
||||
expect(decision).toEqual({ withhold: true, reason: "user-paused" });
|
||||
});
|
||||
|
||||
// FN-7736: the planner overseer must keep withholding for a task blocked on
|
||||
// a pending human approval decision, via either hold mechanism, and must
|
||||
// NOT regress FN-7514's accidental "paused with no reason" hold once the
|
||||
// canonical durable reason is introduced.
|
||||
describe("approval hold (FN-7736)", () => {
|
||||
it("withholds with reason approval-blocked for the canonical pause-reason hold", () => {
|
||||
const decision = evaluateOverseerHumanControl(
|
||||
task({ paused: true, pausedReason: "awaiting-approval" }),
|
||||
settings(),
|
||||
);
|
||||
expect(decision).toEqual({ withhold: true, reason: "approval-blocked" });
|
||||
});
|
||||
|
||||
it("withholds with reason approval-blocked for the status-based hold (triage plan-approval gate)", () => {
|
||||
const decision = evaluateOverseerHumanControl(
|
||||
task({ paused: false, status: "awaiting-approval" }),
|
||||
settings(),
|
||||
);
|
||||
expect(decision).toEqual({ withhold: true, reason: "approval-blocked" });
|
||||
});
|
||||
|
||||
it("withholds approval-blocked (not auto-merge-off-human-review) even when settings.autoMerge is false", () => {
|
||||
const decision = evaluateOverseerHumanControl(
|
||||
task({ paused: true, pausedReason: "awaiting-approval" }),
|
||||
settings({ autoMerge: false }),
|
||||
);
|
||||
expect(decision).toEqual({ withhold: true, reason: "approval-blocked" });
|
||||
});
|
||||
|
||||
it("does not classify a bare user pause (no reason) as approval-blocked -- still user-paused", () => {
|
||||
const decision = evaluateOverseerHumanControl(task({ paused: true, pausedReason: undefined }), settings());
|
||||
expect(decision).toEqual({ withhold: true, reason: "user-paused" });
|
||||
});
|
||||
|
||||
it("does not classify an engine park with a different reason as approval-blocked", () => {
|
||||
const decision = evaluateOverseerHumanControl(
|
||||
task({ paused: true, pausedReason: "branch-conflict-unrecoverable" }),
|
||||
settings(),
|
||||
);
|
||||
expect(decision).toEqual({ withhold: false });
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -123,6 +123,8 @@ describe("reliability interactions: paused scope decay", () => {
|
||||
{ name: "excluded paused reason", holder: { paused: true, pausedReason: "branch-conflict-unrecoverable" as const } },
|
||||
{ name: "not paused", holder: { paused: false } },
|
||||
{ name: "age below threshold", holder: { paused: true }, settings: { pausedScopeDecayMs: 60_000 }, ageMs: 500 },
|
||||
// FN-7736: the canonical approval-hold reason must be excluded too.
|
||||
{ name: "approval-held (canonical reason)", holder: { paused: true, pausedReason: "awaiting-approval" as const } },
|
||||
])("no-op: $name", async ({ holder, settings, ageMs }) => {
|
||||
const now = Date.now();
|
||||
const effectiveAgeMs = ageMs ?? 31 * 60_000;
|
||||
@@ -136,4 +138,46 @@ describe("reliability interactions: paused scope decay", () => {
|
||||
const manager = new SelfHealingManager(store, { rootDir: process.cwd(), getExecutingTaskIds: () => new Set() });
|
||||
expect(await manager.autoReboundPausedScopeDecay()).toBe(0);
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:ApprovalHold 2026-07-09-00:20:
|
||||
* FN-7736 symptom-verification regression. Reproduces the exact original
|
||||
* failure shape (approval-held in-progress task, no pausedReason, follower
|
||||
* present, decay threshold elapsed) alongside a same-shaped control task
|
||||
* that IS paused but for an unrelated (non-approval) reason, proving the
|
||||
* assertion actually exercises the exclusion mechanism rather than a
|
||||
* vacuously-true "nothing ever reboundeds" check.
|
||||
*/
|
||||
it("symptom verification: leaves an approval-held task in place while still rebounding a control paused task", async () => {
|
||||
const now = Date.now();
|
||||
const approvalHeld = makeTask("FN-APPROVAL", {
|
||||
column: "in-progress",
|
||||
paused: true,
|
||||
pausedReason: "awaiting-approval",
|
||||
executionStartedAt: new Date(now - 31 * 60_000).toISOString(),
|
||||
columnMovedAt: new Date(now - 31 * 60_000).toISOString(),
|
||||
});
|
||||
const approvalFollower = makeTask("FN-APPROVAL-FOLLOWER", { column: "todo", blockedBy: "FN-APPROVAL" });
|
||||
const controlPaused = makeTask("FN-CONTROL", {
|
||||
column: "in-progress",
|
||||
paused: true,
|
||||
pausedReason: "some-other-reason",
|
||||
executionStartedAt: new Date(now - 31 * 60_000).toISOString(),
|
||||
columnMovedAt: new Date(now - 31 * 60_000).toISOString(),
|
||||
});
|
||||
const controlFollower = makeTask("FN-CONTROL-FOLLOWER", { column: "todo", blockedBy: "FN-CONTROL" });
|
||||
const { store, byId } = makeStore([approvalHeld, approvalFollower, controlPaused, controlFollower]);
|
||||
const manager = new SelfHealingManager(store, { rootDir: process.cwd(), getExecutingTaskIds: () => new Set() });
|
||||
|
||||
const count = await manager.autoReboundPausedScopeDecay();
|
||||
|
||||
// Only the control task is rebounded -- the approval-held task is untouched.
|
||||
expect(count).toBe(1);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-CONTROL", "todo", expect.anything());
|
||||
expect(store.moveTask).not.toHaveBeenCalledWith("FN-APPROVAL", "todo", expect.anything());
|
||||
expect(byId.get("FN-APPROVAL")?.column).toBe("in-progress");
|
||||
expect(byId.get("FN-APPROVAL")?.paused).toBe(true);
|
||||
expect(byId.get("FN-APPROVAL")?.pausedReason).toBe("awaiting-approval");
|
||||
expect(byId.get("FN-CONTROL")?.column).toBe("todo");
|
||||
});
|
||||
});
|
||||
|
||||
@@ -211,6 +211,27 @@ describe("recoverPausedAbortFailures", () => {
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
// FN-7736: explicit regression for the approval-hold shape (paused, canonical
|
||||
// awaiting-approval reason) -- classifyPausedAbortWorkflowRecovery's existing
|
||||
// generic `task.paused` skip already covers this, but the invariant
|
||||
// ("approval-blocked tasks are never advanced") must be asserted directly
|
||||
// rather than relying on incidental generic-pause coverage.
|
||||
it("skips an approval-held pause-abort park (canonical awaiting-approval reason)", async () => {
|
||||
const store = createMockStore([
|
||||
parkTask({ id: "FN-APPROVAL", paused: true, pausedReason: "awaiting-approval" }),
|
||||
]);
|
||||
const manager = new SelfHealingManager(store, {
|
||||
rootDir: "/tmp/test-project",
|
||||
getExecutingTaskIds: () => new Set<string>(),
|
||||
});
|
||||
|
||||
const recovered = await manager.recoverPausedAbortFailures();
|
||||
|
||||
expect(recovered).toBe(0);
|
||||
expect(store.updateTask).not.toHaveBeenCalled();
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("leaves guarded in-review pause-abort parks untouched", async () => {
|
||||
const candidates = [
|
||||
parkTask({ id: "FN-U", column: "in-review", error: IN_REVIEW_PARK_ERROR, steps: DONE_STEPS, userPaused: true, autoMerge: true }),
|
||||
|
||||
@@ -19,7 +19,7 @@
|
||||
|
||||
import type { AgentStore, AgentHeartbeatRun, HeartbeatInvocationSource, AgentHeartbeatConfig, AgentBudgetStatus, Message, MessageStore, TaskStore, TaskDetail, AgentRole, Agent, InboxTask, RunMutationContext, Settings, AgentConfigRevision, ReflectionStore, ChatStore, ChatRoom, ChatRoomMessage, AgentMemoryInclusionMode } from "@fusion/core";
|
||||
import { AutoClaimSnapshotManager, resolveFreshAutoClaimCandidates, type AutoClaimCandidate } from "./auto-claim-snapshot.js";
|
||||
import { ApprovalRequestStore, buildExecutionMemoryInstructions, isEphemeralAgent, hasAgentIdentity, resolveEffectiveAgentPermissionPolicy, canAgentTakeImplementationTask, canAgentTakeImplementationTaskForExplicitRouting, resolvePersistAgentThinkingLog, resolveAgentMemoryInclusionMode, FUSION_RUNTIME_SELF_AWARENESS } from "@fusion/core";
|
||||
import { ApprovalRequestStore, buildExecutionMemoryInstructions, isEphemeralAgent, hasAgentIdentity, resolveEffectiveAgentPermissionPolicy, canAgentTakeImplementationTask, canAgentTakeImplementationTaskForExplicitRouting, resolvePersistAgentThinkingLog, resolveAgentMemoryInclusionMode, FUSION_RUNTIME_SELF_AWARENESS, AWAITING_APPROVAL_PAUSE_REASON } from "@fusion/core";
|
||||
import type { ToolDefinition } from "@earendil-works/pi-coding-agent";
|
||||
import { Type, type Static } from "@earendil-works/pi-ai";
|
||||
import { createHash } from "node:crypto";
|
||||
@@ -1249,7 +1249,11 @@ export class HeartbeatMonitor {
|
||||
*/
|
||||
pauseForApproval: async ({ approvalRequestId, decision }) => {
|
||||
if (taskId && this.taskStore) {
|
||||
await this.taskStore.pauseTask(taskId, true, undefined, { pausedByAgentId: agent.id });
|
||||
// FNXC:ApprovalHold 2026-07-09-00:10: FN-7736 -- mirror executor.ts's
|
||||
// stamping of the canonical AWAITING_APPROVAL_PAUSE_REASON so
|
||||
// recovery/oversight code recognizes this hold on the task, not just
|
||||
// the agent's `pauseReason`.
|
||||
await this.taskStore.pauseTask(taskId, true, undefined, { pausedByAgentId: agent.id, pausedReason: AWAITING_APPROVAL_PAUSE_REASON });
|
||||
await this.taskStore.logEntry(
|
||||
taskId,
|
||||
`Approval required for ${decision.toolName}. Request ${approvalRequestId} created; task and agent paused awaiting decision.`,
|
||||
|
||||
@@ -11,7 +11,7 @@ import { existsSync, lstatSync, realpathSync } from "node:fs";
|
||||
import { readFile, rm, writeFile } from "node:fs/promises";
|
||||
import type { TaskStore, Task, TaskDetail, TaskTokenUsage, StepStatus, Settings, WorkflowStep, MissionStore, Slice, AgentState, AgentCapability, RunMutationContext, AgentHeartbeatConfig, Agent, AgentMemoryInclusionMode, ProjectSettings, MergeResult, WorkflowIrNode, WorkflowIrNodeKind, WorkflowStepResult as CoreWorkflowStepResult } from "@fusion/core";
|
||||
import { getUnmetSchedulingDependencies } from "./scheduler.js";
|
||||
import { RetryStormError, TaskDeletedError, serializeRetryStormError, isExperimentalFeatureEnabled, resolveWorkflowIrForTask, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, isSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, COMPLETION_SUMMARY_NODE_ID, upsertWorkflowStepResult } from "@fusion/core";
|
||||
import { RetryStormError, TaskDeletedError, serializeRetryStormError, isExperimentalFeatureEnabled, resolveWorkflowIrForTask, resolveColumnAgentBinding, resolveEffectiveAgent, instanceNodeId, getWorkflowExtensionRegistry, getBuiltinWorkflow, parseNoOpCompletionMarker, allowsAutoMergeProcessing, isSharedBranchGroupMemberIntegration, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, COMPLETION_SUMMARY_NODE_ID, upsertWorkflowStepResult, AWAITING_APPROVAL_PAUSE_REASON } from "@fusion/core";
|
||||
import { finalizeProvenAutoMergeTask } from "./auto-merge-finalization.js";
|
||||
import { mergeEffectiveSettings } from "./effective-settings.js";
|
||||
import type { TaskStep, WorkflowIr, WorkflowFieldDefinition, WorkflowColumnAgent, EffectiveAgentInput, WorkflowWorkEngineDispatchResult } from "@fusion/core";
|
||||
@@ -2278,7 +2278,16 @@ export class TaskExecutor {
|
||||
},
|
||||
pauseForApproval: async ({ approvalRequestId, decision }) => {
|
||||
if (taskId) {
|
||||
await this.store.pauseTask(taskId, true, this.getRunContextFor(taskId), { pausedByAgentId: actorId });
|
||||
/*
|
||||
FNXC:ApprovalHold 2026-07-09-00:10:
|
||||
FN-7736: stamp the canonical AWAITING_APPROVAL_PAUSE_REASON on the
|
||||
task (not just the agent) so recovery/oversight code can durably
|
||||
recognize this hold via isTaskBlockedOnApproval -- previously only
|
||||
`paused: true` was set with no reason, which self-healing's
|
||||
autoReboundPausedScopeDecay could rebound before the operator ever
|
||||
decided.
|
||||
*/
|
||||
await this.store.pauseTask(taskId, true, this.getRunContextFor(taskId), { pausedByAgentId: actorId, pausedReason: AWAITING_APPROVAL_PAUSE_REASON });
|
||||
await this.store.logEntry(
|
||||
taskId,
|
||||
`Approval required for ${decision.toolName}. Request ${approvalRequestId} created; task and agent paused awaiting decision.`,
|
||||
|
||||
@@ -5,11 +5,12 @@
|
||||
* — no steering, retry, targeted-fix, or FN-7513 confirmation-required
|
||||
* action (merge/PR progression, destructive git, external-service side
|
||||
* effect) may fire, and no pending confirmation may even be recorded —
|
||||
* whenever a task is (a) user-paused, or (b) not eligible for auto-merge
|
||||
* processing per the FN-5147 `autoMerge:false` / PR-based human-review
|
||||
* terminal contract. This module supplies a single PURE predicate
|
||||
* (`evaluateOverseerHumanControl`, no I/O) that the FN-7512/FN-7513 dispatch
|
||||
* seam (`PlannerRecoveryController.tick`) must consult BEFORE any action
|
||||
* whenever a task is (a) user-paused, (b) blocked on a pending human
|
||||
* approval decision, or (c) not eligible for auto-merge processing per the
|
||||
* FN-5147 `autoMerge:false` / PR-based human-review terminal contract. This
|
||||
* module supplies a single PURE predicate (`evaluateOverseerHumanControl`,
|
||||
* no I/O) that the FN-7512/FN-7513 dispatch seam
|
||||
* (`PlannerRecoveryController.tick`) must consult BEFORE any action
|
||||
* classification, confirmation gating, steering, retry, or dispatch —
|
||||
* mirroring the pure-decision style of `recovery-policy.ts` /
|
||||
* `overseer-confirmation-policy`-equivalent `planner-confirmation.ts`.
|
||||
@@ -29,7 +30,22 @@
|
||||
* DOES carry a `pausedReason` is therefore an engine-originated park, not
|
||||
* a user pause, and must NOT withhold oversight on that basis alone (the
|
||||
* separate `allowsAutoMergeProcessing` / autoMerge-off check still
|
||||
* applies independently).
|
||||
* applies independently) — UNLESS that `pausedReason` is specifically the
|
||||
* canonical `AWAITING_APPROVAL_PAUSE_REASON` (see below).
|
||||
*
|
||||
* FNXC:ApprovalHold 2026-07-09-00:25:
|
||||
* FN-7736: a task can also be blocked on a pending human tool-approval
|
||||
* decision (`pauseForApproval` -> `pauseTask(id, true, { pausedReason:
|
||||
* AWAITING_APPROVAL_PAUSE_REASON })`, or the triage plan-approval gate's
|
||||
* `status: "awaiting-approval"`). Before FN-7736 stamped a durable reason,
|
||||
* this hold shape (`paused:true`, no reason) was indistinguishable from —
|
||||
* and only accidentally covered by — the user-pause branch above. Once the
|
||||
* durable reason was introduced, that branch would STOP matching (the task
|
||||
* now carries a `pausedReason`), which would silently regress FN-7514's
|
||||
* hold. This module therefore checks `isTaskBlockedOnApproval` as its own
|
||||
* explicit, non-accidental branch, checked ahead of the user-pause /
|
||||
* auto-merge-off checks so introducing the durable reason can never flip an
|
||||
* approval-held task from withhold to act.
|
||||
*
|
||||
* Auto-merge-off / human-review half: delegates verbatim to
|
||||
* `allowsAutoMergeProcessing` from `@fusion/core` (the canonical FN-5147
|
||||
@@ -38,9 +54,9 @@
|
||||
*/
|
||||
|
||||
import type { Settings, Task } from "@fusion/core";
|
||||
import { allowsAutoMergeProcessing } from "@fusion/core";
|
||||
import { allowsAutoMergeProcessing, isTaskBlockedOnApproval } from "@fusion/core";
|
||||
|
||||
export type OverseerHumanControlWithholdReason = "user-paused" | "auto-merge-off-human-review";
|
||||
export type OverseerHumanControlWithholdReason = "user-paused" | "approval-blocked" | "auto-merge-off-human-review";
|
||||
|
||||
export interface OverseerHumanControlDecision {
|
||||
/** `true` when the overseer must take NO action of any kind for this task. */
|
||||
@@ -50,7 +66,7 @@ export interface OverseerHumanControlDecision {
|
||||
}
|
||||
|
||||
/** The minimal task shape the predicate needs — narrowed for testability and to keep the module engine-local/pure. */
|
||||
export type OverseerHumanControlTask = Pick<Task, "userPaused" | "paused" | "pausedReason" | "autoMerge" | "prInfo" | "prInfos">;
|
||||
export type OverseerHumanControlTask = Pick<Task, "userPaused" | "paused" | "pausedReason" | "status" | "autoMerge" | "prInfo" | "prInfos">;
|
||||
|
||||
/** The minimal settings shape the predicate needs (forwarded to `allowsAutoMergeProcessing`). */
|
||||
export type OverseerHumanControlSettings = Pick<Settings, "autoMerge">;
|
||||
@@ -58,9 +74,10 @@ export type OverseerHumanControlSettings = Pick<Settings, "autoMerge">;
|
||||
/**
|
||||
* Pure predicate — no I/O, no throws on well-formed input. Returns whether
|
||||
* the planner overseer must withhold ALL oversight action for `task`, and
|
||||
* why. Precedence: user-pause is checked first (it is the stronger signal —
|
||||
* a user explicitly stopped the world for this task), then the FN-5147
|
||||
* auto-merge-off / human-review terminal contract.
|
||||
* why. Precedence: the approval hold is checked first (FN-7736 — a durable,
|
||||
* unambiguous marker that must never be shadowed by the weaker
|
||||
* accidental-no-reason user-pause heuristic below it), then user-pause, then
|
||||
* the FN-5147 auto-merge-off / human-review terminal contract.
|
||||
*/
|
||||
export function evaluateOverseerHumanControl(
|
||||
task: OverseerHumanControlTask | null | undefined,
|
||||
@@ -73,6 +90,10 @@ export function evaluateOverseerHumanControl(
|
||||
return { withhold: true };
|
||||
}
|
||||
|
||||
if (isTaskBlockedOnApproval(task)) {
|
||||
return { withhold: true, reason: "approval-blocked" };
|
||||
}
|
||||
|
||||
const isUserPaused =
|
||||
task.userPaused === true || (task.paused === true && !task.pausedReason);
|
||||
if (isUserPaused) {
|
||||
|
||||
@@ -30,7 +30,7 @@ import { setImmediate as setImmediateCb } from "node:timers";
|
||||
import { existsSync, mkdirSync, readdirSync, readFileSync, realpathSync, rmSync, statSync, writeFileSync } from "node:fs";
|
||||
import { tmpdir } from "node:os";
|
||||
import { isAbsolute, join, relative, resolve } from "node:path";
|
||||
import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkflowColumnsEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, parseExplicitDuplicateMarker, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, resolveWorkflowIrForTask, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core";
|
||||
import { IN_REVIEW_STALL_DEADLOCK_LOG_PREFIX, IN_REVIEW_STALL_LOG_PREFIX, IN_REVIEW_STALL_TERMINAL_LOG_PREFIX, allowsAutoMergeProcessing, countRecentIdenticalStallEntries, detectDependencyCycle, detectSelfDefeatingDependency, evaluateNoCommitsNoOpFinalize, getInReviewStalledSignal, getInReviewStallReason, getPrimaryPrInfo, getStalePausedReviewSignal, getStalePausedTodoSignal, getTaskHardMergeBlocker, getTaskMergeBlocker, isEphemeralAgent, isMergeRequestContractShadowEnabled, isWorkflowColumnsEnabled, isWorkspaceTask, isSharedBranchGroupMemberIntegration, parseExplicitDuplicateMarker, resolveMaxAutoMergeRetries, resolveOptionalStepRevisionBudget, resolveOptionalReviewRevisionBudget, resolveWorkflowIrForTask, AWAITING_APPROVAL_PAUSE_REASON, type Agent, type AgentStore, type ChatStore, type MessageStore, type TaskStore, type Settings, type Task, type MergeDetails, type TaskPriority, type MergeResult, type WorkflowStepResult } from "@fusion/core";
|
||||
import type { MeshLeaseManager } from "./mesh-lease-manager.js";
|
||||
import { createLogger, schedulerLog } from "./logger.js";
|
||||
import { mergeEffectiveSettings } from "./effective-settings.js";
|
||||
@@ -795,10 +795,20 @@ export class SelfHealingManager {
|
||||
lastNtfyAt: number | null;
|
||||
} | null = null;
|
||||
|
||||
/*
|
||||
* FNXC:ApprovalHold 2026-07-09-00:15:
|
||||
* FN-7736: AWAITING_APPROVAL_PAUSE_REASON must be excluded here so a task
|
||||
* parked mid-execution on a pending tool-approval decision (`pauseForApproval`
|
||||
* -> `pauseTask(id, true, { pausedReason: AWAITING_APPROVAL_PAUSE_REASON })`)
|
||||
* is never rebounded to `todo` by this scope-decay sweep before the operator
|
||||
* approves or denies -- this was the reported symptom (a follower task's
|
||||
* scope-decay threshold elapsing could silently defeat the approval gate).
|
||||
*/
|
||||
private static readonly PAUSED_SCOPE_DECAY_EXCLUDED_REASONS = new Set([
|
||||
"branch-conflict-unrecoverable",
|
||||
"worktrunk_operation_failed",
|
||||
"token_budget_exceeded",
|
||||
AWAITING_APPROVAL_PAUSE_REASON,
|
||||
]);
|
||||
|
||||
constructor(
|
||||
|
||||
Reference in New Issue
Block a user