FN-7569: skip re-asking manual plan approval for unchanged re-specified plans
Manual plan approval now skips re-asking for approval when a re-specification produces an identical plan to one already approved. - Add nullable Task.approvedPlanFingerprint field with DB migration 139 to track the approved PROMPT.md fingerprint - Skip re-parking at awaiting-approval when replan/plan-review-retry/self-healing rebound yields the same plan fingerprint as before - Require fresh approval when the plan content changes or when a plan is rejected - Leave Release Authorization, Workflow Plan Review, and auto-approve-all behavior unchanged - Add/extend tests across core (db, plan-approval, store-persistence), engine (triage), and dashboard (routes-github) to cover fingerprint comparison and idempotent re-approval - Update docs (settings-reference.md, workflow-steps.md) to describe the idempotent approval behavior - Add changeset for @runfusion/fusion (patch) Files changed: .changeset/fn-7569-plan-approval-idempotent.md | 7 + docs/settings-reference.md | 2 +- docs/workflow-steps.md | 2 + packages/core/src/__tests__/db.test.ts | 54 +++++++ packages/core/src/__tests__/plan-approval.test.ts | 31 +++- .../core/src/__tests__/store-persistence.test.ts | 39 +++++ packages/core/src/db.ts | 22 ++- packages/core/src/index.ts | 2 +- packages/core/src/plan-approval.ts | 23 +++ packages/core/src/store.ts | 20 ++- packages/core/src/types.ts | 13 ++ .../dashboard/src/__tests__/routes-github.test.ts | 69 +++++++- .../src/routes/register-task-workflow-routes.ts | 37 ++++- packages/engine/src/__tests__/triage.test.ts | 178 ++++++++++++++++++++- packages/engine/src/triage.ts | 58 +++++-- 15 files changed, 527 insertions(+), 30 deletions(-) Fusion-Task-Id: FN-7569 Fusion-Task-Lineage: 7d3855ae-6f45-4571-90db-cf1ae3b541dd Co-authored-by: Fusion (runfusion.ai) <noreply@runfusion.ai>
This commit is contained in:
gsxdsm
7
.changeset/fn-7569-plan-approval-idempotent.md
Normal file
7
.changeset/fn-7569-plan-approval-idempotent.md
Normal file
@@ -0,0 +1,7 @@
|
||||
---
|
||||
"@runfusion/fusion": patch
|
||||
---
|
||||
|
||||
summary: Manual plan approval no longer re-asks you to approve a plan you already approved when it hasn't changed.
|
||||
category: fix
|
||||
dev: FN-7569 — approving a plan records a fingerprint of the approved PROMPT.md (new nullable Task.approvedPlanFingerprint, migration 139). The manual plan-approval gate skips re-parking at awaiting-approval when a re-specification (replan, plan-review retry, self-healing rebound) produces the same plan; a changed plan or reject-plan still requires fresh approval. Release authorization, Workflow Plan Review, and auto-approve-all are unchanged.
|
||||
@@ -419,7 +419,7 @@ Security-sensitive file-browser escape hatches are project-only. `allowAbsoluteF
|
||||
| `overlapIgnorePaths` | `string[]` | `[]` | Optional project-relative file or directory paths to exclude from overlap blocking (for example `docs` or `generated/openapi.json`). Entries are trimmed, deduplicated, and must not be absolute or contain `..` traversal. |
|
||||
| `allowAbsoluteFileBrowserPaths` | `boolean` | `false` | Project-scoped Settings → General toggle for the workspace file browser. When enabled, slash-prefixed paths such as `/tmp` can be listed/read/written/downloaded through workspace file-browser routes while keeping existing file-size, binary, type, null-byte, traversal, and permission checks. Windows drive-letter paths remain blocked, and task-local file routes, memory APIs, worktree-copy validation, plugin bundle paths, and other validators are unchanged. |
|
||||
| `autoMerge` | `boolean` | `true` | Auto-finalize tasks from `in-review`. Tasks can override this per-task (including at create time in New Task modal via **Auto-merge** = Default/Enabled/Disabled); explicit overrides are tagged with `autoMergeProvenance: "user"`, while tasks left at **Default** keep following the live global setting and do not snapshot it when entering review. Legacy pre-FN-6245 in-review rows that were stamped `autoMerge: true` are marked `autoMergeProvenance: "legacy-stamp"` on startup and can be inspected/cleared with Settings → Merge → **Legacy auto-merge stamp cleanup**, `fn pr automerge-cleanup [--apply] [--json]`, or `reconcileLegacyAutoMergeStamps({ apply: true })` after operator review. For grouped branch flows, per-task `autoMerge` governs member→group-integration landing while group `autoMerge` governs group→default-branch promotion eligibility. |
|
||||
| `planApprovalMode` | `"workflow" \| "auto-approve-all" \| "require-all"` | `"auto-approve-all"` | Project-scoped override for the manual planning approval gate. Defaults to auto-approve-all (FN-7557) so new/unset projects skip the manual gate; `"workflow"` instead preserves the workflow-resolved `requirePlanApproval`; `"auto-approve-all"` moves every successfully specified task to `todo` without manual plan approval even when the selected workflow or stored workflow setting has `requirePlanApproval: true`; `"require-all"` parks every specified task at `status: "awaiting-approval"` regardless of workflow settings. Settings → Merge remains the full three-state editor; the Board Triage/intake **Auto-approve plan** switch is a binary shortcut for `"auto-approve-all"` vs `"workflow"`. This does not disable Workflow Plan Review, release authorization, or other non-plan safety gates. **FN-7559:** release authorization and the manual gate both use `status: "awaiting-approval"`, so a release-class task that still parks under `"auto-approve-all"` is NOT a broken auto-approve — it is the intentionally-not-bypassed release-authorization gate. The task carries `awaitingApprovalReason: "release-authorization"` in that case (undefined for a genuine manual hold), and the dashboard renders a distinct "Awaiting Release Authorization" label with the Approve/Reject Plan buttons hidden, instead of the generic manual-approval affordance. |
|
||||
| `planApprovalMode` | `"workflow" \| "auto-approve-all" \| "require-all"` | `"auto-approve-all"` | Project-scoped override for the manual planning approval gate. Defaults to auto-approve-all (FN-7557) so new/unset projects skip the manual gate; `"workflow"` instead preserves the workflow-resolved `requirePlanApproval`; `"auto-approve-all"` moves every successfully specified task to `todo` without manual plan approval even when the selected workflow or stored workflow setting has `requirePlanApproval: true`; `"require-all"` parks every specified task at `status: "awaiting-approval"` regardless of workflow settings. Settings → Merge remains the full three-state editor; the Board Triage/intake **Auto-approve plan** switch is a binary shortcut for `"auto-approve-all"` vs `"workflow"`. This does not disable Workflow Plan Review, release authorization, or other non-plan safety gates. **FN-7559:** release authorization and the manual gate both use `status: "awaiting-approval"`, so a release-class task that still parks under `"auto-approve-all"` is NOT a broken auto-approve — it is the intentionally-not-bypassed release-authorization gate. The task carries `awaitingApprovalReason: "release-authorization"` in that case (undefined for a genuine manual hold), and the dashboard renders a distinct "Awaiting Release Authorization" label with the Approve/Reject Plan buttons hidden, instead of the generic manual-approval affordance. **FN-7569:** approving a plan under `"workflow"`/`"require-all"` manual approval records a fingerprint (hash) of the exact approved `PROMPT.md`. If the task is later re-specified (a replan, a plan-review reviewer-outage retry, or a self-healing rebound back to triage) and produces the identical plan content, the manual gate is idempotent: it skips re-parking at `status: "awaiting-approval"` and proceeds straight to `todo`, so the operator is never asked to re-approve a plan they already approved. A genuinely changed `PROMPT.md` still re-asks, and rejecting a plan (Reject Plan) clears the fingerprint so the regenerated plan is treated as new. This idempotency check lives strictly inside the manual gate, after release authorization and Workflow Plan Review have already decided, and has no effect under `"auto-approve-all"` (which never reaches the manual gate). |
|
||||
| `maxAutoMergeRetries` | `number` | `3` | Project-scoped positive-integer cap for auto-merge conflict-resolution retries before Fusion parks or bounces a task for human/recovery handling. Unset, non-finite, zero, or negative values fall back to `3` to preserve historical behavior. |
|
||||
| `mergeRequestContractShadowEnabled` | `boolean` | `false` | Phase-1 FN-5741 write-only shadow flag (project/global setting). When enabled, executor/self-healing/merger persist merge-request records and `completion_handoff_accepted` markers for observation only; legacy mergeQueue + lifecycle remains authoritative. |
|
||||
| `mergeStrategy` | `"direct" \| "pull-request"` | `"direct"` | Completion mode (local direct merge vs PR-first). |
|
||||
|
||||
@@ -214,6 +214,8 @@ Workflow Plan Review is separate from manual plan approval. Project `planApprova
|
||||
|
||||
**FN-7559 — telling the three holds apart:** Plan Review parks a task with its own distinct statuses (`needs-replan` for a revision verdict, `plan-review-unavailable` for a reviewer-outage retry), so it never renders identically to a plan-approval hold. The release-authorization gate and the manual plan-approval gate, however, both use `status: "awaiting-approval"` — auto-approve-all bypasses the manual gate but never the release-authorization gate, so a release-class task (or a user-authored task missing the explicit authorization marker) still parks even with auto-approve-all on. To make that unambiguous to the operator, the task carries `awaitingApprovalReason: "release-authorization"` only when the release-authorization gate is the one holding it; the dashboard renders a distinct "Awaiting Release Authorization" label and hides the Approve/Reject Plan buttons for that hold instead of showing the generic manual-approval affordance.
|
||||
|
||||
**FN-7569 — manual plan approval is idempotent against unchanged plans:** approving a plan under the manual gate records a fingerprint of the exact approved `PROMPT.md`. If the same task is later re-specified — a `needs-replan` replan, a Plan Review reviewer-outage retry, or a self-healing rebound back to `triage` — and produces byte-identical `PROMPT.md` content, the manual gate detects the match and proceeds straight to `todo` instead of re-parking at `status: "awaiting-approval"`. A genuinely revised plan still produces a different fingerprint and re-asks as before, and using Reject Plan clears the fingerprint so the regenerated plan is always treated as new. This idempotency check runs only inside the manual gate, strictly after release authorization and Plan Review have already made their independent decisions, and never applies under `planApprovalMode: "auto-approve-all"` (which bypasses the manual gate entirely).
|
||||
|
||||
`builtin:legacy-coding` is backed by the original monolithic `BUILTIN_CODING_WORKFLOW_IR`: `planning` → `execute` → optional quality gates → `review` → merge region.
|
||||
|
||||
`builtin:stepwise-coding` displays as Coding (per-step review). It is backed by `BUILTIN_STEPWISE_CODING_WORKFLOW_IR`; it keeps the same lifecycle columns/traits while adding the default-on optional Plan Review before `parse-steps`, modeling per-step parse/execute/review/rework as authored graph structure, and retaining the post-foreach optional Code Review gate before its final review/merge region.
|
||||
|
||||
@@ -1876,6 +1876,60 @@ describe("schema migrations", () => {
|
||||
db.close();
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569: migration 139 adds approvedPlanFingerprint so manual plan approval can be
|
||||
* idempotent against unchanged plan content — an approved-then-re-specified task whose
|
||||
* PROMPT.md is unchanged should not be re-parked at awaiting-approval. Additive-only,
|
||||
* no backfill — legacy rows stay NULL, meaning "never approved" (falls back to today's
|
||||
* always-re-park behavior).
|
||||
*/
|
||||
it("migrates v138 databases by adding approvedPlanFingerprint column with legacy rows staying NULL (no backfill)", () => {
|
||||
tmpDir = makeTmpDir();
|
||||
const fusionDir = join(tmpDir, ".fusion");
|
||||
const db = new Database(fusionDir);
|
||||
|
||||
db.exec(`
|
||||
CREATE TABLE IF NOT EXISTS __meta (key TEXT PRIMARY KEY, value TEXT);
|
||||
CREATE TABLE IF NOT EXISTS tasks (
|
||||
id TEXT PRIMARY KEY,
|
||||
description TEXT NOT NULL,
|
||||
"column" TEXT NOT NULL,
|
||||
status TEXT,
|
||||
createdAt TEXT NOT NULL,
|
||||
updatedAt TEXT NOT NULL,
|
||||
executionMode TEXT DEFAULT 'standard',
|
||||
plannerOversightLevel TEXT,
|
||||
awaitingApprovalReason TEXT
|
||||
);
|
||||
CREATE TABLE IF NOT EXISTS config (
|
||||
id INTEGER PRIMARY KEY CHECK (id = 1),
|
||||
nextId INTEGER DEFAULT 1,
|
||||
nextWorkflowStepId INTEGER DEFAULT 1,
|
||||
settings TEXT DEFAULT '{}',
|
||||
workflowSteps TEXT DEFAULT '[]',
|
||||
updatedAt TEXT
|
||||
);
|
||||
`);
|
||||
db.exec("INSERT INTO __meta (key, value) VALUES ('schemaVersion', '138')");
|
||||
db.exec("INSERT INTO __meta (key, value) VALUES ('lastModified', '1000')");
|
||||
db.exec(`INSERT INTO tasks (id, description, "column", status, createdAt, updatedAt) VALUES ('FN-1', 'legacy', 'triage', 'awaiting-approval', '2026-01-01', '2026-01-01')`);
|
||||
|
||||
db.init();
|
||||
|
||||
expect(db.getSchemaVersion()).toBe(SCHEMA_VERSION);
|
||||
|
||||
const cols = db.prepare("PRAGMA table_info(tasks)").all() as Array<{ name: string }>;
|
||||
expect(cols.map((col) => col.name)).toContain("approvedPlanFingerprint");
|
||||
|
||||
const task = db.prepare("SELECT approvedPlanFingerprint FROM tasks WHERE id = 'FN-1'").get() as {
|
||||
approvedPlanFingerprint: string | null;
|
||||
};
|
||||
expect(task.approvedPlanFingerprint).toBeNull();
|
||||
|
||||
db.close();
|
||||
});
|
||||
|
||||
it("migrates v43 databases by adding task token-usage aggregate columns with null-compatible defaults", () => {
|
||||
tmpDir = makeTmpDir();
|
||||
const fusionDir = join(tmpDir, ".fusion");
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { describe, expect, it } from "vitest";
|
||||
import { resolvePlanApprovalRequired, type PlanApprovalMode } from "../plan-approval.js";
|
||||
import { computePlanApprovalFingerprint, resolvePlanApprovalRequired, type PlanApprovalMode } from "../plan-approval.js";
|
||||
|
||||
const workflowValues = [true, false, undefined] as const;
|
||||
|
||||
@@ -35,3 +35,32 @@ describe("resolvePlanApprovalRequired", () => {
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — computePlanApprovalFingerprint coverage: stable for identical content, normalizes only
|
||||
* trailing whitespace/newlines, and differs whenever the actual plan body changes.
|
||||
*/
|
||||
describe("computePlanApprovalFingerprint", () => {
|
||||
it("is stable for the same content across repeated calls", () => {
|
||||
const text = "# Task: FN-1\n\n## File Scope\n\n- a.ts\n";
|
||||
expect(computePlanApprovalFingerprint(text)).toBe(computePlanApprovalFingerprint(text));
|
||||
});
|
||||
|
||||
it("is unaffected by trailing whitespace or trailing newline differences", () => {
|
||||
const base = "# Task: FN-1\n\n## File Scope\n\n- a.ts";
|
||||
expect(computePlanApprovalFingerprint(base)).toBe(computePlanApprovalFingerprint(`${base}\n`));
|
||||
expect(computePlanApprovalFingerprint(base)).toBe(computePlanApprovalFingerprint(`${base}\n\n\n`));
|
||||
expect(computePlanApprovalFingerprint("line one \nline two")).toBe(computePlanApprovalFingerprint("line one\nline two"));
|
||||
});
|
||||
|
||||
it("differs when the plan content actually changes", () => {
|
||||
const original = "# Task: FN-1\n\n## File Scope\n\n- a.ts\n";
|
||||
const changed = "# Task: FN-1\n\n## File Scope\n\n- a.ts\n- b.ts\n";
|
||||
expect(computePlanApprovalFingerprint(original)).not.toBe(computePlanApprovalFingerprint(changed));
|
||||
});
|
||||
|
||||
it("produces a hex sha256-length digest", () => {
|
||||
expect(computePlanApprovalFingerprint("anything")).toMatch(/^[0-9a-f]{64}$/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -59,6 +59,45 @@ describe("TaskStore", () => {
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — approvedPlanFingerprint must survive create/update/null-clear round trips through
|
||||
* SQLite so the manual plan-approval gate can compare it against the freshly written PROMPT.md
|
||||
* on every re-specification, even after a full store reopen.
|
||||
*/
|
||||
describe("approvedPlanFingerprint persistence", () => {
|
||||
it("round-trips approvedPlanFingerprint through updateTask and getTask", async () => {
|
||||
const task = await harness.store().createTask({ description: "Approval fingerprint task" });
|
||||
expect(task.approvedPlanFingerprint).toBeUndefined();
|
||||
|
||||
const updated = await harness.store().updateTask(task.id, { approvedPlanFingerprint: "abc123" });
|
||||
expect(updated.approvedPlanFingerprint).toBe("abc123");
|
||||
|
||||
const detail = await harness.store().getTask(task.id);
|
||||
expect(detail.approvedPlanFingerprint).toBe("abc123");
|
||||
});
|
||||
|
||||
it("clears approvedPlanFingerprint with an explicit null update (reject-plan semantics)", async () => {
|
||||
const task = await harness.store().createTask({ description: "Approval fingerprint task" });
|
||||
await harness.store().updateTask(task.id, { approvedPlanFingerprint: "abc123" });
|
||||
|
||||
const cleared = await harness.store().updateTask(task.id, { approvedPlanFingerprint: null });
|
||||
expect(cleared.approvedPlanFingerprint).toBeUndefined();
|
||||
|
||||
const detail = await harness.store().getTask(task.id);
|
||||
expect(detail.approvedPlanFingerprint).toBeUndefined();
|
||||
});
|
||||
|
||||
it("returns approvedPlanFingerprint from listTasks", async () => {
|
||||
const task = await harness.store().createTask({ description: "Approval fingerprint list task" });
|
||||
await harness.store().updateTask(task.id, { approvedPlanFingerprint: "xyz789" });
|
||||
|
||||
const tasks = await harness.store().listTasks();
|
||||
const listed = tasks.find((t) => t.id === task.id);
|
||||
expect(listed?.approvedPlanFingerprint).toBe("xyz789");
|
||||
});
|
||||
});
|
||||
|
||||
// FNXC:Workspace 2026-06-24-15:30 (multiworkspace fn_task_done regression):
|
||||
// task.workspaceWorktrees previously had NO SQLite column / no rowToTask mapping, so
|
||||
// fn_acquire_repo_worktree's updateTask({workspaceWorktrees}) set it only in memory and the very
|
||||
|
||||
@@ -183,7 +183,7 @@ export function isFts5CorruptionError(error: unknown): boolean {
|
||||
|
||||
// ── Schema Definition ────────────────────────────────────────────────
|
||||
|
||||
const SCHEMA_VERSION = 138;
|
||||
const SCHEMA_VERSION = 139;
|
||||
|
||||
const TASKS_FTS_AUTOMERGE = 8;
|
||||
const TASKS_FTS_CRISISMERGE = 16;
|
||||
@@ -301,6 +301,7 @@ CREATE TABLE IF NOT EXISTS tasks (
|
||||
executionMode TEXT DEFAULT 'standard',
|
||||
plannerOversightLevel TEXT,
|
||||
awaitingApprovalReason TEXT,
|
||||
approvedPlanFingerprint TEXT,
|
||||
tokenUsageInputTokens INTEGER,
|
||||
tokenUsageOutputTokens INTEGER,
|
||||
tokenUsageCachedTokens INTEGER,
|
||||
@@ -5590,6 +5591,25 @@ export class Database {
|
||||
});
|
||||
}
|
||||
|
||||
if (version < 139) {
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — manual plan approval had no persisted record of what the operator actually
|
||||
* approved, so re-specification of an already-approved, unchanged plan (replan,
|
||||
* plan-review reviewer-outage retry, self-healing rebound to triage) re-triggered the
|
||||
* manual gate and re-parked the identical plan at "awaiting-approval", forcing the
|
||||
* operator to re-approve a plan they already approved. This nullable column stores only
|
||||
* a hash (computePlanApprovalFingerprint in packages/core/src/plan-approval.ts) of the
|
||||
* last operator-approved PROMPT.md, set by POST /tasks/:id/approve-plan and cleared by
|
||||
* POST /tasks/:id/reject-plan. Additive-only: NULL means never-approved (legacy rows) or
|
||||
* a rejected/cleared approval, and the manual gate falls back to today's always-re-park
|
||||
* behavior for those rows. No backfill.
|
||||
*/
|
||||
this.applyMigration(139, () => {
|
||||
this.addColumnIfMissing("tasks", "approvedPlanFingerprint", "TEXT");
|
||||
});
|
||||
}
|
||||
|
||||
}
|
||||
|
||||
/**
|
||||
|
||||
@@ -26,7 +26,7 @@ export type { AnthropicProviderRegistration } from "./anthropic-models.js";
|
||||
export { detectImageMimeFromBytes } from "./image-mime.js";
|
||||
export type { DetectedImageMime } from "./image-mime.js";
|
||||
export { redactSecrets } from "./redact-secrets.js";
|
||||
export { resolvePlanApprovalRequired } from "./plan-approval.js";
|
||||
export { computePlanApprovalFingerprint, resolvePlanApprovalRequired } from "./plan-approval.js";
|
||||
export type { PlanApprovalMode } from "./plan-approval.js";
|
||||
export { isActiveNearDuplicateColumn, isNearDuplicateCanonicalInactive } from "./near-duplicate-canonical.js";
|
||||
export type { NearDuplicateCanonicalState } from "./near-duplicate-canonical.js";
|
||||
|
||||
@@ -1,7 +1,30 @@
|
||||
import { createHash } from "node:crypto";
|
||||
import type { ProjectSettings } from "./types.js";
|
||||
|
||||
export type PlanApprovalMode = NonNullable<ProjectSettings["planApprovalMode"]>;
|
||||
|
||||
/**
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — manual plan approval was not idempotent against unchanged plan content: an
|
||||
* operator approving a plan (auto-approve-all off) had no persisted record of *what* they
|
||||
* approved, so any re-specification of the same task (replan, plan-review reviewer-outage
|
||||
* retry, self-healing rebound to triage) that re-ran finalizeApprovedTask re-triggered the
|
||||
* manual gate and re-parked an already-approved, byte-identical plan at "awaiting-approval".
|
||||
* computePlanApprovalFingerprint gives approve-plan a stable hash of the approved PROMPT.md
|
||||
* (Task.approvedPlanFingerprint) so the manual gate can skip re-parking when the freshly
|
||||
* written PROMPT.md is unchanged, while still re-asking when the plan genuinely changed or
|
||||
* was rejected. Normalizes only trailing whitespace/newlines so cosmetic write differences
|
||||
* (trailing newline, trailing spaces) never cause spurious re-approval.
|
||||
*/
|
||||
export function computePlanApprovalFingerprint(promptText: string): string {
|
||||
const normalized = promptText
|
||||
.split("\n")
|
||||
.map((line) => line.replace(/[ \t]+$/, ""))
|
||||
.join("\n")
|
||||
.replace(/\s+$/, "");
|
||||
return createHash("sha256").update(normalized, "utf8").digest("hex");
|
||||
}
|
||||
|
||||
/**
|
||||
* FNXC:PlanApproval 2026-06-26-00:00:
|
||||
* Per-project planApprovalMode controls the planning approval gate for every task in the project: require-all always parks approved specs for manual approval, auto-approve-all always bypasses the gate, and workflow/undefined preserves the workflow-resolved requirePlanApproval value.
|
||||
|
||||
@@ -272,6 +272,7 @@ interface TaskRow {
|
||||
executionMode: string | null;
|
||||
plannerOversightLevel: string | null;
|
||||
awaitingApprovalReason: string | null;
|
||||
approvedPlanFingerprint: string | null;
|
||||
tokenUsageInputTokens: number | null;
|
||||
tokenUsageOutputTokens: number | null;
|
||||
tokenUsageCachedTokens: number | null;
|
||||
@@ -446,6 +447,13 @@ const TASK_COLUMN_DESCRIPTORS: TaskColumnDescriptor[] = [
|
||||
* the same task never survives past the manual gate's own awaiting-approval.
|
||||
*/
|
||||
defineTaskColumn("awaitingApprovalReason", (task) => task.awaitingApprovalReason ?? null),
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — hash of the last operator-approved PROMPT.md (computePlanApprovalFingerprint).
|
||||
* Set by POST /tasks/:id/approve-plan, cleared by POST /tasks/:id/reject-plan, and consulted
|
||||
* by the manual plan-approval gate to skip re-parking an unchanged, already-approved plan.
|
||||
*/
|
||||
defineTaskColumn("approvedPlanFingerprint", (task) => task.approvedPlanFingerprint ?? null),
|
||||
defineTaskColumn("tokenUsageInputTokens", (task) => task.tokenUsage?.inputTokens ?? null),
|
||||
defineTaskColumn("tokenUsageOutputTokens", (task) => task.tokenUsage?.outputTokens ?? null),
|
||||
defineTaskColumn("tokenUsageCachedTokens", (task) => task.tokenUsage?.cachedTokens ?? null),
|
||||
@@ -2145,6 +2153,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
executionMode: (row.executionMode || undefined) as Task["executionMode"],
|
||||
plannerOversightLevel: (row.plannerOversightLevel || undefined) as Task["plannerOversightLevel"],
|
||||
awaitingApprovalReason: (row.awaitingApprovalReason || undefined) as Task["awaitingApprovalReason"],
|
||||
approvedPlanFingerprint: row.approvedPlanFingerprint || undefined,
|
||||
createdAt: row.createdAt,
|
||||
updatedAt: row.updatedAt,
|
||||
columnMovedAt: row.columnMovedAt || undefined,
|
||||
@@ -2701,7 +2710,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
"validatorModelProvider", "validatorModelId",
|
||||
"planningModelProvider", "planningModelId",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "resumeLimboCount", "graphResumeRetryCount", "resumeLimboTipSha", "resumeLimboStepSignature", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "worktreeSessionRetryCount", "completionHandoffLimboRecoveryCount", "verificationFailureCount", "mergeConflictBounceCount", "mergeAuditBounceCount", "mergeTransientRetryCount", "branchConflictRecoveryCount", "reviewerContextRetryCount", "reviewerFallbackRetryCount", "nextRecoveryAt",
|
||||
"error", "summary", "thinkingLevel", "executionMode", "plannerOversightLevel", "awaitingApprovalReason",
|
||||
"error", "summary", "thinkingLevel", "executionMode", "plannerOversightLevel", "awaitingApprovalReason", "approvedPlanFingerprint",
|
||||
"tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageCacheWriteTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt", "tokenUsageModelProvider", "tokenUsageModelId", "tokenUsagePerModel", "tokenBudgetSoftAlertedAt", "tokenBudgetHardAlertedAt", "tokenBudgetOverride",
|
||||
"createdAt", "updatedAt", "columnMovedAt", "firstExecutionAt", "cumulativeActiveMs", "columnDwellMs", "executionStartedAt", "executionCompletedAt",
|
||||
"dependencies", "steps", "customFields", "comments", "review", "reviewState", "workflowStepResults", "steeringComments",
|
||||
@@ -2797,7 +2806,7 @@ export class TaskStore extends EventEmitter<TaskStoreEvents> {
|
||||
"validatorModelProvider", "validatorModelId",
|
||||
"planningModelProvider", "planningModelId",
|
||||
"mergeRetries", "workflowStepRetries", "stuckKillCount", "resumeLimboCount", "graphResumeRetryCount", "resumeLimboTipSha", "resumeLimboStepSignature", "postReviewFixCount", "recoveryRetryCount", "taskDoneRetryCount", "worktreeSessionRetryCount", "completionHandoffLimboRecoveryCount", "verificationFailureCount", "mergeConflictBounceCount", "mergeAuditBounceCount", "mergeTransientRetryCount", "branchConflictRecoveryCount", "reviewerContextRetryCount", "reviewerFallbackRetryCount", "nextRecoveryAt",
|
||||
"error", "summary", "thinkingLevel", "executionMode", "plannerOversightLevel", "awaitingApprovalReason",
|
||||
"error", "summary", "thinkingLevel", "executionMode", "plannerOversightLevel", "awaitingApprovalReason", "approvedPlanFingerprint",
|
||||
"tokenUsageInputTokens", "tokenUsageOutputTokens", "tokenUsageCachedTokens", "tokenUsageCacheWriteTokens", "tokenUsageTotalTokens", "tokenUsageFirstUsedAt", "tokenUsageLastUsedAt", "tokenUsageModelProvider", "tokenUsageModelId", "tokenUsagePerModel", "tokenBudgetSoftAlertedAt", "tokenBudgetHardAlertedAt", "tokenBudgetOverride",
|
||||
"createdAt", "updatedAt", "columnMovedAt", "firstExecutionAt", "cumulativeActiveMs", "columnDwellMs", "executionStartedAt", "executionCompletedAt",
|
||||
"dependencies", "steps", "customFields", "attachments", "steeringComments",
|
||||
@@ -8417,7 +8426,7 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
|
||||
async updateTask(
|
||||
id: string,
|
||||
updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; workspaceWorktrees?: import("./types.js").Task["workspaceWorktrees"]; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; customFields?: Record<string, unknown>; currentStep?: number; blockedBy?: string | null; overlapBlockedBy?: string | null; assignedAgentId?: string | null; pausedByAgentId?: string | null; pausedReason?: string | null; tokenBudgetSoftAlertedAt?: string | null; worktrunkFallbackAlertedAt?: string | null; worktrunkFailure?: import("./types.js").Task["worktrunkFailure"] | null; tokenBudgetHardAlertedAt?: string | null; tokenBudgetOverride?: import("./types.js").TaskTokenBudgetOverride | null; dispatchStormCount?: number | null; lastDispatchAt?: string | null; assigneeUserId?: string | null; scopeOverride?: boolean | null; scopeOverrideReason?: string | null; scopeAutoWiden?: string[] | null; nodeId?: string | null; effectiveNodeId?: string | null; effectiveNodeSource?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; checkoutNodeId?: string | null; checkoutRunId?: string | null; checkoutLeaseRenewedAt?: string | null; checkoutLeaseEpoch?: number | null; paused?: boolean; baseBranch?: string | null; autoMerge?: boolean | null; branch?: string | null; executionStartBranch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; plannerOversightLevel?: import("./types.js").PlannerOversightLevel | null; awaitingApprovalReason?: import("./types.js").Task["awaitingApprovalReason"] | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; resumeLimboCount?: number | null; graphResumeRetryCount?: number | null; resumeLimboTipSha?: string | null; resumeLimboStepSignature?: string | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; worktreeSessionRetryCount?: number | null; completionHandoffLimboRecoveryCount?: number | null; verificationFailureCount?: number | null; mergeConflictBounceCount?: number | null; mergeAuditBounceCount?: number | null; mergeTransientRetryCount?: number | null; branchConflictRecoveryCount?: number | null; reviewerContextRetryCount?: number | null; reviewerFallbackRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; noCommitsExpected?: boolean | null; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; firstExecutionAt?: string | null; cumulativeActiveMs?: number | null; executionStartedAt?: string | null; executionCompletedAt?: string | null; review?: import("./types.js").TaskReview | null; reviewState?: import("./types.js").TaskReviewState | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; sourceMetadataPatch?: Record<string, unknown> | null; githubTracking?: import("./types.js").TaskGithubTracking | null; gitlabTracking?: (Omit<import("./types.js").TaskGitLabTracking, "item"> & { item?: import("./types.js").TaskGitLabTrackedItem | null }) | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; workflowTransitionNotification?: import("./types.js").Task["workflowTransitionNotification"] | null; missionId?: string | null; sliceId?: string | null },
|
||||
updates: { title?: string; description?: string; priority?: TaskPriority | null; prompt?: string; worktree?: string | null; workspaceWorktrees?: import("./types.js").Task["workspaceWorktrees"]; status?: string | null; dependencies?: string[]; steps?: import("./types.js").TaskStep[]; customFields?: Record<string, unknown>; currentStep?: number; blockedBy?: string | null; overlapBlockedBy?: string | null; assignedAgentId?: string | null; pausedByAgentId?: string | null; pausedReason?: string | null; tokenBudgetSoftAlertedAt?: string | null; worktrunkFallbackAlertedAt?: string | null; worktrunkFailure?: import("./types.js").Task["worktrunkFailure"] | null; tokenBudgetHardAlertedAt?: string | null; tokenBudgetOverride?: import("./types.js").TaskTokenBudgetOverride | null; dispatchStormCount?: number | null; lastDispatchAt?: string | null; assigneeUserId?: string | null; scopeOverride?: boolean | null; scopeOverrideReason?: string | null; scopeAutoWiden?: string[] | null; nodeId?: string | null; effectiveNodeId?: string | null; effectiveNodeSource?: string | null; checkedOutBy?: string | null; checkedOutAt?: string | null; checkoutNodeId?: string | null; checkoutRunId?: string | null; checkoutLeaseRenewedAt?: string | null; checkoutLeaseEpoch?: number | null; paused?: boolean; baseBranch?: string | null; autoMerge?: boolean | null; branch?: string | null; executionStartBranch?: string | null; baseCommitSha?: string | null; size?: "S" | "M" | "L"; reviewLevel?: number; executionMode?: import("./types.js").ExecutionMode | null; plannerOversightLevel?: import("./types.js").PlannerOversightLevel | null; awaitingApprovalReason?: import("./types.js").Task["awaitingApprovalReason"] | null; approvedPlanFingerprint?: string | null; mergeRetries?: number; workflowStepRetries?: number; stuckKillCount?: number | null; resumeLimboCount?: number | null; graphResumeRetryCount?: number | null; resumeLimboTipSha?: string | null; resumeLimboStepSignature?: string | null; postReviewFixCount?: number | null; recoveryRetryCount?: number | null; taskDoneRetryCount?: number | null; worktreeSessionRetryCount?: number | null; completionHandoffLimboRecoveryCount?: number | null; verificationFailureCount?: number | null; mergeConflictBounceCount?: number | null; mergeAuditBounceCount?: number | null; mergeTransientRetryCount?: number | null; branchConflictRecoveryCount?: number | null; reviewerContextRetryCount?: number | null; reviewerFallbackRetryCount?: number | null; nextRecoveryAt?: string | null; enabledWorkflowSteps?: string[]; noCommitsExpected?: boolean | null; modelProvider?: string | null; modelId?: string | null; validatorModelProvider?: string | null; validatorModelId?: string | null; planningModelProvider?: string | null; planningModelId?: string | null; thinkingLevel?: string | null; error?: string | null; summary?: string | null; sessionFile?: string | null; firstExecutionAt?: string | null; cumulativeActiveMs?: number | null; executionStartedAt?: string | null; executionCompletedAt?: string | null; review?: import("./types.js").TaskReview | null; reviewState?: import("./types.js").TaskReviewState | null; workflowStepResults?: import("./types.js").WorkflowStepResult[] | null; mergeDetails?: import("./types.js").MergeDetails | null; sourceIssue?: import("./types.js").TaskSourceIssue | null; sourceMetadataPatch?: Record<string, unknown> | null; githubTracking?: import("./types.js").TaskGithubTracking | null; gitlabTracking?: (Omit<import("./types.js").TaskGitLabTracking, "item"> & { item?: import("./types.js").TaskGitLabTrackedItem | null }) | null; tokenUsage?: import("./types.js").TaskTokenUsage | null; modifiedFiles?: string[] | null; workflowTransitionNotification?: import("./types.js").Task["workflowTransitionNotification"] | null; missionId?: string | null; sliceId?: string | null },
|
||||
runContext?: RunMutationContext,
|
||||
): Promise<Task> {
|
||||
return this.withTaskLock(id, () => this.updateTaskUnlocked(id, updates, runContext));
|
||||
@@ -9243,6 +9252,11 @@ ${TASK_UPSERT_SQL_ASSIGNMENTS}
|
||||
} else if (updates.awaitingApprovalReason !== undefined) {
|
||||
task.awaitingApprovalReason = updates.awaitingApprovalReason as import("./types.js").Task["awaitingApprovalReason"];
|
||||
}
|
||||
if (updates.approvedPlanFingerprint === null) {
|
||||
task.approvedPlanFingerprint = undefined;
|
||||
} else if (updates.approvedPlanFingerprint !== undefined) {
|
||||
task.approvedPlanFingerprint = updates.approvedPlanFingerprint;
|
||||
}
|
||||
if (updates.error === null) {
|
||||
task.error = undefined;
|
||||
} else if (updates.error !== undefined) {
|
||||
|
||||
@@ -2485,6 +2485,19 @@ export interface Task {
|
||||
* no hold or an ordinary manual-approval hold.
|
||||
*/
|
||||
awaitingApprovalReason?: "release-authorization";
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — records the computePlanApprovalFingerprint (packages/core/src/plan-approval.ts)
|
||||
* hash of the exact PROMPT.md content an operator last approved via POST /tasks/:id/approve-plan.
|
||||
* The manual plan-approval gate (packages/engine/src/triage.ts finalizeApprovedTask) compares this
|
||||
* against the freshly written PROMPT.md on every re-specification (replan, plan-review retry,
|
||||
* self-healing rebound to triage) and skips re-parking at "awaiting-approval" when they match, so an
|
||||
* unchanged, already-approved plan is never re-asked. A genuine spec change produces a different
|
||||
* fingerprint and still re-asks. POST /tasks/:id/reject-plan clears this field (null) alongside
|
||||
* deleting PROMPT.md so the regenerated plan is treated as new. Stores only a hash, never plan text.
|
||||
* Additive-only, nullable: legacy/never-approved rows stay NULL and behave exactly as before.
|
||||
*/
|
||||
approvedPlanFingerprint?: string;
|
||||
/** Thinking level for AI agent sessions — controls reasoning effort (off/minimal/low/medium/high) */
|
||||
thinkingLevel?: ThinkingLevel;
|
||||
/** Execution mode for task implementation.
|
||||
|
||||
@@ -2085,6 +2085,46 @@ describe("POST /tasks/:id/approve-plan", () => {
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-001", "todo");
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — approve-plan must persist an approvedPlanFingerprint hash of the current
|
||||
* on-disk PROMPT.md so a later re-specification of the SAME plan can skip re-asking.
|
||||
*/
|
||||
it("records an approvedPlanFingerprint hash of the on-disk PROMPT.md", async () => {
|
||||
const root = mkdtempSync(join(tmpdir(), "kb-dashboard-approve-plan-"));
|
||||
try {
|
||||
mkdirSync(join(root, ".fusion", "tasks", "FN-001"), { recursive: true });
|
||||
writeFileSync(join(root, ".fusion", "tasks", "FN-001", "PROMPT.md"), "# Task: FN-001\n\nApproved plan body.\n");
|
||||
|
||||
const localStore = createMockStore({
|
||||
getTask: vi.fn(),
|
||||
moveTask: vi.fn(),
|
||||
updateTask: vi.fn(),
|
||||
logEntry: vi.fn().mockResolvedValue(undefined),
|
||||
getRootDir: vi.fn().mockReturnValue(root),
|
||||
});
|
||||
const awaitingTask = { ...FAKE_TASK_DETAIL, column: "triage" as const, status: "awaiting-approval" as const };
|
||||
const movedTask = { ...FAKE_TASK_DETAIL, column: "todo" as const };
|
||||
(localStore.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(awaitingTask);
|
||||
(localStore.moveTask as ReturnType<typeof vi.fn>).mockResolvedValue(movedTask);
|
||||
(localStore.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue({ ...movedTask, status: undefined });
|
||||
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use("/api", createApiRoutes(localStore));
|
||||
|
||||
const res = await REQUEST(app, "POST", "/api/tasks/KB-001/approve-plan");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(localStore.updateTask).toHaveBeenCalledWith(
|
||||
"FN-001",
|
||||
expect.objectContaining({ status: undefined, approvedPlanFingerprint: expect.stringMatching(/^[0-9a-f]{64}$/) }),
|
||||
);
|
||||
} finally {
|
||||
rmSync(root, { recursive: true, force: true });
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("POST /tasks/:id/reject-plan", () => {
|
||||
@@ -2117,7 +2157,9 @@ describe("POST /tasks/:id/reject-plan", () => {
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.logEntry).toHaveBeenCalledWith("FN-001", "Plan rejected by user", "Specification will be regenerated");
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: undefined });
|
||||
// FN-7569: reject-plan clears any previously-recorded approval fingerprint so a
|
||||
// regenerated plan is always treated as new and requires fresh manual approval.
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: undefined, approvedPlanFingerprint: null });
|
||||
expect(res.body.column).toBe("triage");
|
||||
});
|
||||
|
||||
@@ -2199,7 +2241,30 @@ describe("POST /tasks/:id/reject-plan", () => {
|
||||
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/reject-plan");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: undefined });
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: undefined, approvedPlanFingerprint: null });
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — reject-plan must clear a previously-recorded approval fingerprint
|
||||
* regardless of its prior value, so a regenerated plan never inherits it.
|
||||
*/
|
||||
it("clears an existing approvedPlanFingerprint on reject", async () => {
|
||||
const awaitingTask = {
|
||||
...FAKE_TASK_DETAIL,
|
||||
column: "triage" as const,
|
||||
status: "awaiting-approval" as const,
|
||||
approvedPlanFingerprint: "deadbeef",
|
||||
};
|
||||
const updatedTask = { ...FAKE_TASK_DETAIL, column: "triage" as const, status: undefined, approvedPlanFingerprint: undefined };
|
||||
|
||||
(store.getTask as ReturnType<typeof vi.fn>).mockResolvedValue(awaitingTask);
|
||||
(store.updateTask as ReturnType<typeof vi.fn>).mockResolvedValue(updatedTask);
|
||||
|
||||
const res = await REQUEST(buildApp(), "POST", "/api/tasks/KB-001/reject-plan");
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-001", { status: undefined, approvedPlanFingerprint: null });
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
@@ -60,7 +60,7 @@ import {
|
||||
} from "@fusion/engine";
|
||||
import { buildBoardWorkflowsPayload } from "./board-workflows.js";
|
||||
import { isBackwardMoveBlockedByOpenPr, PR_OPEN_BLOCKS_MOVE_BACK_MESSAGE } from "./register-pull-requests-routes.js";
|
||||
import { isWorkspaceTask, type RunAuditEventInput } from "@fusion/core";
|
||||
import { computePlanApprovalFingerprint, isWorkspaceTask, type RunAuditEventInput } from "@fusion/core";
|
||||
import { ApiError, badRequest, conflict, notFound } from "../api-error.js";
|
||||
import type { ApiRoutesContext } from "./types.js";
|
||||
import { deriveAutoTaskBranch, derivePerTaskBranch, getBranchSelectionMode, resolveBranchSelection } from "./branch-selection.js";
|
||||
@@ -2754,11 +2754,34 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
// Log the approval
|
||||
await scopedStore.logEntry(task.id, "Plan approved by user");
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — persist a fingerprint of the exact PROMPT.md the operator just approved
|
||||
* so a later re-specification (replan, plan-review retry, self-healing rebound) that
|
||||
* produces the identical plan can skip re-parking at awaiting-approval. Read the
|
||||
* on-disk PROMPT.md directly (best-effort) since the task row does not always carry
|
||||
* full prompt text; a missing/unreadable file leaves the fingerprint unset and the
|
||||
* manual gate falls back to today's always-re-park behavior for this task.
|
||||
*/
|
||||
let approvedPlanFingerprint: string | undefined;
|
||||
try {
|
||||
const { readFile } = await import("node:fs/promises");
|
||||
const { join } = await import("node:path");
|
||||
const promptPath = join(scopedStore.getRootDir(), ".fusion", "tasks", task.id, "PROMPT.md");
|
||||
const promptText = await readFile(promptPath, "utf8");
|
||||
approvedPlanFingerprint = computePlanApprovalFingerprint(promptText);
|
||||
} catch {
|
||||
// No PROMPT.md to fingerprint (unusual for an awaiting-approval task) — leave unset.
|
||||
}
|
||||
|
||||
// Move to todo and clear status
|
||||
const updated = await scopedStore.moveTask(task.id, "todo");
|
||||
await scopedStore.updateTask(task.id, { status: undefined });
|
||||
await scopedStore.updateTask(task.id, {
|
||||
status: undefined,
|
||||
...(approvedPlanFingerprint ? { approvedPlanFingerprint } : {}),
|
||||
});
|
||||
|
||||
res.json({ ...updated, status: undefined });
|
||||
res.json({ ...updated, status: undefined, ...(approvedPlanFingerprint ? { approvedPlanFingerprint } : {}) });
|
||||
} catch (err: unknown) {
|
||||
if (err instanceof ApiError) {
|
||||
throw err;
|
||||
@@ -2797,7 +2820,13 @@ export function registerTaskWorkflowRoutes(ctx: ApiRoutesContext, deps: TaskWork
|
||||
await scopedStore.logEntry(task.id, "Plan rejected by user", "Specification will be regenerated");
|
||||
|
||||
// Clear status to return to normal triage state
|
||||
await scopedStore.updateTask(task.id, { status: undefined });
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — clear any previously-recorded approval fingerprint alongside the status
|
||||
* clear and PROMPT.md removal, so the regenerated plan is always treated as new and
|
||||
* requires fresh manual approval (it must never inherit the rejected plan's fingerprint).
|
||||
*/
|
||||
await scopedStore.updateTask(task.id, { status: undefined, approvedPlanFingerprint: null });
|
||||
|
||||
// Remove PROMPT.md to force regeneration
|
||||
const { rm } = await import("node:fs/promises");
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
|
||||
import type { TaskStore, Task, TaskDetail, Settings } from "@fusion/core";
|
||||
import { builtinSeamPrompt, MAX_TASK_LIST_TEXT_CHARS, renderTriagePolicyPlaceholders, resolveAgentPrompt } from "@fusion/core";
|
||||
import { builtinSeamPrompt, computePlanApprovalFingerprint, MAX_TASK_LIST_TEXT_CHARS, renderTriagePolicyPlaceholders, resolveAgentPrompt } from "@fusion/core";
|
||||
import {
|
||||
TriageProcessor,
|
||||
buildSpecificationPrompt,
|
||||
@@ -2734,6 +2734,182 @@ describe("requirePlanApproval setting", () => {
|
||||
expect(manualUpdateCall?.[1]).toMatchObject({ awaitingApprovalReason: null });
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — symptom repro + fix: manual plan approval must be idempotent against
|
||||
* unchanged plan content. An operator approves a plan (approvedPlanFingerprint gets
|
||||
* persisted on the task, mirroring POST /tasks/:id/approve-plan), then the SAME task
|
||||
* re-enters finalizeApprovedTask (replan / plan-review retry / self-healing rebound)
|
||||
* with byte-identical PROMPT.md. Today's code (pre-fix) would re-park at
|
||||
* awaiting-approval a second time; the fix must move straight to todo instead.
|
||||
*/
|
||||
describe("FN-7569: plan approval fingerprint idempotency", () => {
|
||||
const planText = "# Task: FN-IDEMPOTENT - Idempotent plan\n\n## Mission\n\nDo the thing.\n\n## File Scope\n\n- a.ts\n";
|
||||
const changedPlanText = "# Task: FN-IDEMPOTENT - Idempotent plan\n\n## Mission\n\nDo the thing, differently.\n\n## File Scope\n\n- a.ts\n- b.ts\n";
|
||||
|
||||
it("re-specifying the SAME approved plan skips the manual gate and moves straight to todo", async () => {
|
||||
const fingerprint = computePlanApprovalFingerprint(planText);
|
||||
const task = createTriageTask({
|
||||
id: "FN-IDEMPOTENT",
|
||||
status: "planning",
|
||||
approvedPlanFingerprint: fingerprint,
|
||||
} as Partial<Task>);
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(task),
|
||||
} as Partial<TaskStore>);
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
planText,
|
||||
{ requirePlanApproval: true, planApprovalMode: "require-all" } as Settings,
|
||||
);
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-IDEMPOTENT", "todo");
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-IDEMPOTENT", expect.objectContaining({ status: "awaiting-approval" }));
|
||||
expect(store.logEntry).toHaveBeenCalledWith(
|
||||
"FN-IDEMPOTENT",
|
||||
"Plan unchanged since prior approval — proceeding without re-approval",
|
||||
);
|
||||
});
|
||||
|
||||
it("re-specifying a CHANGED plan after prior approval still re-asks for approval", async () => {
|
||||
const fingerprint = computePlanApprovalFingerprint(planText);
|
||||
const task = createTriageTask({
|
||||
id: "FN-IDEMPOTENT-CHANGED",
|
||||
status: "planning",
|
||||
approvedPlanFingerprint: fingerprint,
|
||||
} as Partial<Task>);
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(task),
|
||||
} as Partial<TaskStore>);
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
changedPlanText,
|
||||
{ requirePlanApproval: true, planApprovalMode: "require-all" } as Settings,
|
||||
);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-IDEMPOTENT-CHANGED", expect.objectContaining({ status: "awaiting-approval", awaitingApprovalReason: null }));
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("never-approved task (no fingerprint) still parks at awaiting-approval on first specify", async () => {
|
||||
const task = createTriageTask({
|
||||
id: "FN-NEVER-APPROVED",
|
||||
status: "planning",
|
||||
} as Partial<Task>);
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(task),
|
||||
} as Partial<TaskStore>);
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
planText,
|
||||
{ requirePlanApproval: true, planApprovalMode: "require-all" } as Settings,
|
||||
);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-NEVER-APPROVED", expect.objectContaining({ status: "awaiting-approval" }));
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("rejected plan (fingerprint cleared to undefined) re-asks even though the same content was approved before", async () => {
|
||||
const task = createTriageTask({
|
||||
id: "FN-REJECTED-THEN-RESPECIFIED",
|
||||
status: "planning",
|
||||
approvedPlanFingerprint: undefined,
|
||||
} as Partial<Task>);
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(task),
|
||||
} as Partial<TaskStore>);
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
planText,
|
||||
{ requirePlanApproval: true, planApprovalMode: "require-all" } as Settings,
|
||||
);
|
||||
|
||||
expect(store.updateTask).toHaveBeenCalledWith("FN-REJECTED-THEN-RESPECIFIED", expect.objectContaining({ status: "awaiting-approval" }));
|
||||
expect(store.moveTask).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("auto-approve-all moves to todo regardless of fingerprint state (manual gate never reached)", async () => {
|
||||
const task = createTriageTask({
|
||||
id: "FN-AUTO-APPROVE-FINGERPRINT",
|
||||
status: "planning",
|
||||
approvedPlanFingerprint: computePlanApprovalFingerprint(changedPlanText),
|
||||
} as Partial<Task>);
|
||||
const store = createMockStore({
|
||||
getTask: vi.fn().mockResolvedValue(task),
|
||||
} as Partial<TaskStore>);
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
|
||||
await (processor as unknown as {
|
||||
finalizeApprovedTask(task: Task, writtenInput: string, settings: Settings): Promise<void>;
|
||||
}).finalizeApprovedTask(
|
||||
task,
|
||||
planText,
|
||||
{ requirePlanApproval: true, planApprovalMode: "auto-approve-all" } as Settings,
|
||||
);
|
||||
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-AUTO-APPROVE-FINGERPRINT", "todo");
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-AUTO-APPROVE-FINGERPRINT", expect.objectContaining({ status: "awaiting-approval" }));
|
||||
});
|
||||
|
||||
/*
|
||||
* FN-7569: exercise the recoverApprovedTask caller (planning-recovery self-heal),
|
||||
* not just finalizeApprovedTask directly, so the fingerprint short-circuit is
|
||||
* proven to reach every finalizeApprovedTask caller, not just a direct-call seam.
|
||||
*/
|
||||
it("recoverApprovedTask (self-healing planning recovery) skips re-park for an unchanged already-approved plan", async () => {
|
||||
const fingerprint = computePlanApprovalFingerprint(planText);
|
||||
await mkdir(join(rootDir, ".fusion", "tasks", "FN-RECOVER-IDEMPOTENT"), { recursive: true });
|
||||
await writeFile(join(rootDir, ".fusion", "tasks", "FN-RECOVER-IDEMPOTENT", "PROMPT.md"), planText);
|
||||
const store = createMockStore({
|
||||
getSettings: vi.fn().mockResolvedValue({
|
||||
maxConcurrent: 2,
|
||||
maxWorktrees: 4,
|
||||
pollIntervalMs: 10000,
|
||||
groupOverlappingFiles: false,
|
||||
autoMerge: true,
|
||||
requirePlanApproval: true,
|
||||
} as Settings),
|
||||
});
|
||||
const processor = new TriageProcessor(store, rootDir);
|
||||
|
||||
const recovered = await processor.recoverApprovedTask({
|
||||
id: "FN-RECOVER-IDEMPOTENT",
|
||||
description: "Recovered triage task",
|
||||
column: "triage",
|
||||
status: "planning",
|
||||
approvedPlanFingerprint: fingerprint,
|
||||
dependencies: [],
|
||||
steps: [],
|
||||
currentStep: 0,
|
||||
log: [
|
||||
{ timestamp: "2026-01-01T00:00:00.000Z", action: "Spec review: APPROVE" },
|
||||
],
|
||||
createdAt: "2026-01-01T00:00:00.000Z",
|
||||
updatedAt: "2026-01-01T00:02:00.000Z",
|
||||
});
|
||||
|
||||
expect(recovered).toBe(true);
|
||||
expect(store.moveTask).toHaveBeenCalledWith("FN-RECOVER-IDEMPOTENT", "todo");
|
||||
expect(store.updateTask).not.toHaveBeenCalledWith("FN-RECOVER-IDEMPOTENT", expect.objectContaining({ status: "awaiting-approval" }));
|
||||
});
|
||||
});
|
||||
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-12:30:
|
||||
* FN-7526 — auto-approve-all must NOT bypass Workflow Plan Review. A REVISE
|
||||
|
||||
@@ -28,6 +28,7 @@ import {
|
||||
compareTaskIdNumeric,
|
||||
resolveAgentMemoryInclusionMode,
|
||||
resolvePlanApprovalRequired,
|
||||
computePlanApprovalFingerprint,
|
||||
extractIntentSignature,
|
||||
findNearDuplicates,
|
||||
isNearDuplicateCanonicalInactive,
|
||||
@@ -2574,24 +2575,49 @@ export class TriageProcessor {
|
||||
*/
|
||||
if (resolvePlanApprovalRequired(settings)) {
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-21:35:
|
||||
* FN-7559: explicitly clear awaitingApprovalReason on the manual gate's own
|
||||
* awaiting-approval write so a stale "release-authorization" reason left over
|
||||
* from an earlier pass on this same task (e.g. a replan after the release
|
||||
* gate parked it, now passing the release gate but still requiring manual
|
||||
* approval) never survives into this genuinely-manual hold.
|
||||
* FNXC:PlanApproval 2026-07-04-22:41:
|
||||
* FN-7569 — idempotency short-circuit. Compare the freshly written PROMPT.md against
|
||||
* the fingerprint recorded when the operator last approved a plan for this task
|
||||
* (POST /tasks/:id/approve-plan, packages/core/src/plan-approval.ts). If they match,
|
||||
* this is a re-specification of an already-approved, unchanged plan (replan,
|
||||
* plan-review reviewer-outage retry, self-healing rebound to triage, duplicate-marker
|
||||
* retry) and must proceed straight through like an approved task rather than re-parking
|
||||
* at awaiting-approval and asking the operator to re-approve. A genuinely changed plan
|
||||
* (or one whose approval was cleared by reject-plan) produces a different/absent
|
||||
* fingerprint and falls through to the ordinary park below. This check lives strictly
|
||||
* inside the manual-gate branch, after release authorization and Plan Review have
|
||||
* already made their independent decisions, so it never weakens either of those gates
|
||||
* or auto-approve-all (which never reaches this branch at all).
|
||||
*/
|
||||
const approvalUpdates: Record<string, unknown> = { status: "awaiting-approval", awaitingApprovalReason: null };
|
||||
if (shouldApplyPromptDeclaredTitle && promptDeclaredTitle) {
|
||||
approvalUpdates.title = promptDeclaredTitle;
|
||||
const priorFingerprint = latestTransitionTask?.approvedPlanFingerprint ?? task.approvedPlanFingerprint;
|
||||
const currentFingerprint = computePlanApprovalFingerprint(written);
|
||||
if (priorFingerprint && priorFingerprint === currentFingerprint) {
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
"Plan unchanged since prior approval — proceeding without re-approval",
|
||||
);
|
||||
planLog.log(`${task.id} plan unchanged since prior approval — proceeding without re-approval`);
|
||||
} else {
|
||||
/*
|
||||
* FNXC:PlanApproval 2026-07-04-21:35:
|
||||
* FN-7559: explicitly clear awaitingApprovalReason on the manual gate's own
|
||||
* awaiting-approval write so a stale "release-authorization" reason left over
|
||||
* from an earlier pass on this same task (e.g. a replan after the release
|
||||
* gate parked it, now passing the release gate but still requiring manual
|
||||
* approval) never survives into this genuinely-manual hold.
|
||||
*/
|
||||
const approvalUpdates: Record<string, unknown> = { status: "awaiting-approval", awaitingApprovalReason: null };
|
||||
if (shouldApplyPromptDeclaredTitle && promptDeclaredTitle) {
|
||||
approvalUpdates.title = promptDeclaredTitle;
|
||||
}
|
||||
await this.store.updateTask(task.id, approvalUpdates);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
options.recoveryLogAction ?? "Specification approved by AI — awaiting manual approval",
|
||||
);
|
||||
planLog.log(`✓ ${task.id} specified and awaiting manual approval`);
|
||||
return;
|
||||
}
|
||||
await this.store.updateTask(task.id, approvalUpdates);
|
||||
await this.store.logEntry(
|
||||
task.id,
|
||||
options.recoveryLogAction ?? "Specification approved by AI — awaiting manual approval",
|
||||
);
|
||||
planLog.log(`✓ ${task.id} specified and awaiting manual approval`);
|
||||
return;
|
||||
}
|
||||
|
||||
if (shouldClearWorkflowRunStepInstances) {
|
||||
|
||||
Reference in New Issue
Block a user