root/fusion
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

U4: scope the user-pause safeguard to lifecycle MUTATION, not observation (re-ratified) (#2486)

Browse Source 
Stacked on #2482. Base is
`feature/workflow-vocabulary-u4-override-layer` — do not merge before
it.

Implements the coordinator's **re-ratification** of the user-pause
safeguard with a narrower definition.

## The invariant, written into the code

> The user-pause safeguard means **NEVER MUTATE LIFECYCLE STATE** of a
user-paused card. It does **NOT** mean never observe one.

Respecting a pause exists to stop the engine acting on a card *behind*
the operator who paused it — moving, rebounding, archiving, resuming. A
read-only diagnostic does the opposite: it tells that same operator what
their paused card is doing. Blinding them to their own paused work is
not safety; it is the engine deciding they should not be told.

The sentence is in the source, because the distinction is the whole
point and a future reader will otherwise re-broaden it.

## Why it needed narrowing

Ratified broadly first, that reading was caught suppressing the very
sweeps it was meant to protect. `surfaceStalePausedTodos` exists to
report cards that have sat paused too long — routing it through a
reconciler that suppresses paused cards turns a diagnostic into one that
**silently reports nothing**. Measured, not argued: #2484 proves that
sweep surfaces user-paused cards on current main.

## Scoped by action, never by sweep

`OBSERVATIONAL_ACTIONS` is an **allow-list**, so a newly added mutating
action is suppressed by default — the scoping fails closed. A sweep
cannot opt itself out.

`RecoveryActionKind` deliberately names actions the policy vocabulary
cannot yet author (`rebound`, `archive`, `requeue`, `resume`). The
scoping is only testable if those exist as values, and **a rule that
cannot be tested is a rule that erodes**. `parseWorkflowIr` keeps a
closed action list, so nothing becomes authorable by being named.

## Which field — chosen, not inherited

The gate reads `userPaused`, **not** `paused`. The two diverge
(`branch-group-ops.ts:128` says so outright), and `paused` also covers
engine-authored automation pauses like dispatch-storm, which carry no
operator intent to respect — gating on it would suppress recovery from
the engine's own throttles. The safeguard defers to a **human**
decision, so it keys on the field that records one.

## Both halves kept

The broad case is **narrowed, not deleted**: one test proves mutation is
still suppressed, one proves observation is now permitted. A future
reader must be able to tell the scoping was *deliberate* rather than
eroded by someone who found the broad rule inconvenient.

**Mutation-verified in both directions**, since either error is silent:

| mutation | result |
|---|---|
| re-broaden (suppress observation) | **2 tests fail** |
| over-narrow (`rebound` treated observational) | **3 tests fail** |

## Verification

46 tests green (28 safety + 18 inheritance); tsc clean; lint clean;
merge gate green (299+10+71).

No changeset: `@fusion/engine` is private.

🤖 Generated with [Claude Code](https://claude.com/claude-code)
This commit is contained in:
gsxdsm
2026-07-27 17:37:20 -07:00
committed by GitHub
parent 89284df85e
commit 17fdf2f0ae
2 changed files with 124 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files

97
packages/engine/src/__tests__/recovery-policy-safety.test.ts
View File

@@ -28,6 +28,7 @@ import type { Task, WorkflowIr } from "@fusion/core";
import {
decideRecovery,
isLifecycleMutatingAction,
isSuppressedBySafeguard,
resolveColumnRecovery,
RECOVERY_POLICY_KEYS,
@@ -127,42 +128,82 @@ describe("recovery policy safety invariant — safeguards live OUTSIDE the polic
});
describe("behavioral: a policy that tries to disable a safeguard has no effect", () => {
it("cannot switch off the user-pause safeguard", () => {
/*
The load-bearing case for the `surface` action. A user-paused card must not
receive engine-authored signals, and no policy may say otherwise.
*/
const paused = task({ userPaused: true } as Partial<Task>);
/*
FNXC:WorkflowRecoveryPolicy 2026-07-27-21:20 (U4 — RE-RATIFIED, narrowed):
The user-pause safeguard means NEVER MUTATE LIFECYCLE STATE of a user-paused
card. It does NOT mean never observe one.
// Honest policy: suppressed.
const honest = decideRecovery(paused, ir(), LATER);
expect(honest).toEqual({ suppressed: "user-paused" });
// Hostile policy attempting every spelling of "ignore the pause": identical.
const hostile = decideRecovery(
paused,
ir({ ignoreUserPause: true, respectUserPause: false, userPaused: false }),
LATER,
);
expect(hostile).toEqual({ suppressed: "user-paused" });
The broad case below is NARROWED, not deleted. Keeping both halves is the
point: a future reader must be able to tell the scoping was DELIBERATE rather
than eroded by someone who found the broad rule inconvenient.
*/
it("SUPPRESSES a lifecycle-mutating action on a user-paused card", () => {
/* The half that must never weaken: the engine must not move, rebound,
archive, requeue or resume a card behind the operator who paused it. */
for (const action of ["rebound", "archive", "requeue", "resume"] as const) {
expect(isSuppressedBySafeguard({ userPaused: true }, action)).toBe("user-paused");
expect(isLifecycleMutatingAction(action)).toBe(true);
}
});
it("still acts on an unpaused card, so the guard is not vacuously suppressing everything", () => {
/* Without this, a reconciler that suppressed EVERYTHING would pass the test
above while doing nothing — the "dead guard passes tests" failure mode. */
it("PERMITS an observational action on a user-paused card", () => {
/*
The half that was wrong before. `surface` writes no lifecycle state — it
tells the operator what their own paused card is doing. Suppressing it
turned `surfaceStalePausedTodos`, whose entire purpose is reporting paused
cards, into a diagnostic that silently reported nothing.
*/
expect(isSuppressedBySafeguard({ userPaused: true }, "surface")).toBeUndefined();
expect(isLifecycleMutatingAction("surface")).toBe(false);
});
it("still surfaces a stale user-paused card end-to-end", () => {
/* The scoping must reach the actual reconcile outcome, not just the
predicate — otherwise the sweep is still blind in practice. */
const paused = task({ userPaused: true } as Partial<Task>);
const outcome = decideRecovery(paused, ir(), LATER);
expect(outcome).toEqual({
decision: expect.objectContaining({ taskId: "FN-1", action: "surface" }),
});
});
it("suppression is BY ACTION, so no policy can opt a mutating action out", () => {
/*
A hostile policy attempting every spelling of "ignore the pause" changes
nothing: the scoping keys on the action's own nature, never on anything a
workflow author can write.
*/
const hostile = ir({ ignoreUserPause: true, respectUserPause: false, userPaused: false });
expect(resolveColumnRecovery(hostile, "drafting")).toBeDefined();
for (const action of ["rebound", "archive", "requeue", "resume"] as const) {
expect(isSuppressedBySafeguard({ userPaused: true }, action)).toBe("user-paused");
}
});
it("a NEW action is suppressed by default, because observational is an allow-list", () => {
/* Guards the direction of the default: adding a mutating action without
touching the safeguard must fail closed, not open. */
expect(isLifecycleMutatingAction("rebound")).toBe(true);
expect(isSuppressedBySafeguard({ userPaused: true }, "rebound")).toBe("user-paused");
});
it("gates on userPaused, NOT on the broader paused flag", () => {
/*
Chosen deliberately: `paused` also covers automation pauses (dispatch-storm)
that carry no operator intent to respect, and the two fields diverge
(branch-group-ops.ts:128). The safeguard defers to a HUMAN decision, so it
keys on the field that records one.
*/
expect(isSuppressedBySafeguard({ userPaused: false }, "rebound")).toBeUndefined();
expect(isSuppressedBySafeguard({ userPaused: undefined }, "rebound")).toBeUndefined();
});
it("acts on an unpaused card, so the guard is not vacuously suppressing everything", () => {
const outcome = decideRecovery(task(), ir(), LATER);
expect(outcome).toEqual({
decision: expect.objectContaining({ taskId: "FN-1", action: "surface", code: "stale-paused-todo" }),
});
});
it("routes the user-pause decision through the single safeguard chokepoint", () => {
/* Pins that suppression comes from `isSuppressedBySafeguard` and not from an
incidental check elsewhere, so there stays exactly one place to audit. */
expect(isSuppressedBySafeguard({ userPaused: true }, "surface")).toBe("user-paused");
expect(isSuppressedBySafeguard({ userPaused: false }, "surface")).toBeUndefined();
expect(isSuppressedBySafeguard({ userPaused: undefined }, "surface")).toBeUndefined();
});
});
describe("scope limits stated rather than implied", () => {

65
packages/engine/src/recovery-reconciler.ts
View File

@@ -185,21 +185,66 @@ export function resolveColumnRecovery(ir: WorkflowIr, columnId: string): Workflo
}
/*
FNXC:WorkflowRecoveryPolicy 2026-07-27-14:05 (U4):
THE safeguard chokepoint. Every safeguard suppression flows through here so there
is exactly one place to audit, and so the safety test has a single seam to assert
against. Returns the suppressing safeguard, or `undefined` when none applies.
FNXC:WorkflowRecoveryPolicy 2026-07-27-21:10 (U4 — RE-RATIFIED, narrowed):
Every recovery action, whether or not the policy vocabulary can author it yet.
`surface` writes no lifecycle state, so only the user-pause safeguard is
load-bearing for it — a user-paused card must not have engine-authored signals
attributed to it. The remaining five gate lifecycle-mutating actions and are
wired when those actions land.
Declared beyond the authorable set on purpose. The user-pause safeguard is scoped
BY ACTION, so the scoping is only testable if the mutating actions exist as
values — and a rule that cannot be tested is a rule that erodes. `parseWorkflowIr`
keeps a CLOSED action list, so nothing here becomes authorable by being named.
*/
export type RecoveryActionKind = "surface" | "rebound" | "archive" | "requeue" | "resume";
/**
* OBSERVATIONAL actions: they read state and report it. They write no lifecycle
* field, move no card, and change nothing an operator would have to undo.
*/
const OBSERVATIONAL_ACTIONS: ReadonlySet<RecoveryActionKind> = new Set<RecoveryActionKind>(["surface"]);
/** True when an action changes lifecycle state rather than merely reporting it. */
export function isLifecycleMutatingAction(action: RecoveryActionKind): boolean {
return !OBSERVATIONAL_ACTIONS.has(action);
}
/*
FNXC:WorkflowRecoveryPolicy 2026-07-27-21:10 (U4 — THE safeguard chokepoint):
Every safeguard suppression flows through here, so there is exactly one place to
audit and the safety test has a single seam to assert against.
THE INVARIANT, in the words that make the distinction survive a future reader:
The user-pause safeguard means NEVER MUTATE LIFECYCLE STATE of a user-paused
card. It does NOT mean never observe one.
The purpose of respecting a pause is to stop the engine acting on a card BEHIND
the operator who paused it — moving it, rebounding it, archiving it, resuming it.
A read-only diagnostic does the opposite: it tells that same operator what their
paused card is doing. Blinding them to their own paused work is not safety; it is
the engine deciding they should not be told.
This was ratified BROADLY first (suppress every action on a user-paused card) and
re-ratified narrowly after that reading was caught suppressing the very sweeps it
was meant to protect: `surfaceStalePausedTodos` exists to report cards that have
sat paused too long, so the broad rule turned a diagnostic into one that silently
reported nothing. Scoping is therefore BY ACTION, never by sweep — a sweep cannot
opt itself out, and a new mutating action is suppressed by default because
`OBSERVATIONAL_ACTIONS` is an allow-list.
WHICH FIELD, chosen deliberately rather than inherited from whatever was nearest:
this gate reads `userPaused` — the explicit operator park — NOT `paused`. The two
diverge (see `branch-group-ops.ts:128`: "userPaused remains true but legacy
`paused` is false"). `paused` also covers automation pauses such as
dispatch-storm, which are engine-authored and carry no operator intent to respect,
so gating lifecycle mutation on `paused` would suppress recovery from the engine's
own throttles. The safeguard exists to defer to a HUMAN decision, so it keys on
the field that records one.
*/
export function isSuppressedBySafeguard(
task: Pick<Task, "userPaused">,
action: RecoveryDecision["action"],
action: RecoveryActionKind,
): "user-paused" | undefined {
if (action === "surface" && task.userPaused === true) return "user-paused";
if (!isLifecycleMutatingAction(action)) return undefined;
if (task.userPaused === true) return "user-paused";
return undefined;
}