fix: auto-approve plans whose approval predates the prompt-hygiene injection

An operator was re-asked to approve a plan they had already approved and that had not changed. POST /tasks/:id/approve-plan fingerprints the on-disk PROMPT.md, so a plan approved before the `## Original Description` hygiene injection (applyOriginalDescription) shipped carries a hash over PRE-injection content. On the task's next pass the injection rewrites PROMPT.md, the fingerprint moves, and FN-7569's idempotency short-circuit misses — so the manual gate re-parks an unchanged, already-approved plan. finalizeApprovedTask now also compares the recorded fingerprint against the as-read (pre-injection) content. This does not weaken the gate: `written` diverges from `writtenInput` only via that injection, so both arms hash bytes the operator actually approved — only the representation differs. A genuinely changed plan matches neither arm and still parks. On a legacy match the stored fingerprint is migrated forward, so the reconciliation is one-time per task rather than a comparison carried forever. The migration is a direct updateTask — the taskUpdates batch is flushed well before this gate runs. Covers both finalizeApprovedTask callers (direct + recoverApprovedTask), asserts the changed-plan safety edge still parks, and asserts no redundant fingerprint write when the approval is already post-hygiene. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-07-15 13:17:24 -07:00
parent 3b938887c9
commit 5e5fa9a2be
3 changed files with 180 additions and 1 deletions
--- a/.changeset/legacy-plan-approval-fingerprint.md
+++ b/.changeset/legacy-plan-approval-fingerprint.md
@@ -0,0 +1,7 @@
+---
+"@runfusion/fusion": patch
+---
+
+summary: Stop re-asking approval for plans approved before the Original Description update.
+category: fix
+dev: `approve-plan` fingerprints the on-disk PROMPT.md, so plans approved before the `## Original Description` hygiene injection (`applyOriginalDescription`) shipped carry a hash over pre-injection content. The injection then rewrote the prompt and moved the hash, defeating FN-7569's idempotency short-circuit and re-parking unchanged plans at `awaiting-approval`. `finalizeApprovedTask` now also compares the recorded fingerprint against the as-read (pre-injection) content — safe because `written` diverges from `writtenInput` only via that injection, so both arms hash bytes the operator actually approved — and migrates the stored fingerprint forward on a legacy match so the reconciliation is one-time per task. A genuinely changed plan matches neither arm and still parks.
--- a/packages/engine/src/tests/triage.test.ts
+++ b/packages/engine/src/tests/triage.test.ts
@@ -3122,6 +3122,143 @@ describe("requirePlanApproval setting", () => {
      expect(store.moveTask).not.toHaveBeenCalled();
    });

+    /*
+    FNXC:PlanApproval 2026-07-15-15:10:
+    Legacy approvals — recorded before the `## Original Description` hygiene injection existed —
+    must still auto-approve. approve-plan hashes the on-disk PROMPT.md, so those tasks carry a
+    fingerprint over PRE-injection content; once the injection ships it rewrites the prompt, the
+    fingerprint moves, and the operator gets re-asked to approve a plan they already approved and
+    that has not changed.
+
+    ## Surface Enumeration
+    - Legacy fingerprint + unchanged plan -> auto-approve (the reported symptom).
+    - Legacy fingerprint + unchanged plan -> stored fingerprint migrated forward, so the
+      reconciliation happens once per task rather than on every pass.
+    - Legacy fingerprint + CHANGED plan -> must still park. This is the safety edge: the
+      tolerance must not become "any prior approval approves any later plan".
+    - Current (post-injection) fingerprint -> unchanged behavior, no spurious migration write.
+    - Both finalizeApprovedTask callers (direct + recoverApprovedTask) share this gate.
+    */
+    it("auto-approves a plan whose fingerprint predates the prompt-hygiene injection", async () => {
+      // A pre-injection approval: the operator approved the raw plan, before the hygiene
+      // injection existed, so the recorded hash is over PRE-injection content.
+      const legacyFingerprint = computePlanApprovalFingerprint(planText);
+      const task = createTriageTask({
+        id: "FN-LEGACY-FP",
+        status: "planning",
+        approvedPlanFingerprint: legacyFingerprint,
+      } as Partial<Task>);
+      const store = createMockStore({
+        getTask: vi.fn().mockResolvedValue(task),
+      } as Partial<TaskStore>);
+      await mkdir(join(rootDir, ".fusion", "tasks", "FN-LEGACY-FP"), { recursive: true });
+      await writeFile(join(rootDir, ".fusion", "tasks", "FN-LEGACY-FP", "PROMPT.md"), planText);
+      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-LEGACY-FP", "todo");
+      expect(store.updateTask).not.toHaveBeenCalledWith("FN-LEGACY-FP", expect.objectContaining({ status: "awaiting-approval" }));
+      // Migrated forward to the post-injection hash, so this is a one-time reconciliation.
+      const migratedFingerprint = computePlanApprovalFingerprint(approvedOnDisk(planText, "Triage task"));
+      expect(store.updateTask).toHaveBeenCalledWith("FN-LEGACY-FP", { approvedPlanFingerprint: migratedFingerprint });
+      expect(migratedFingerprint).not.toBe(legacyFingerprint);
+    });
+
+    it("still re-asks approval for a CHANGED plan when the prior approval predates prompt hygiene", async () => {
+      // The safety edge: legacy tolerance must not approve a plan the operator never saw.
+      const legacyFingerprint = computePlanApprovalFingerprint(planText);
+      const task = createTriageTask({
+        id: "FN-LEGACY-FP-CHANGED",
+        status: "planning",
+        approvedPlanFingerprint: legacyFingerprint,
+      } as Partial<Task>);
+      const store = createMockStore({
+        getTask: vi.fn().mockResolvedValue(task),
+      } as Partial<TaskStore>);
+      await mkdir(join(rootDir, ".fusion", "tasks", "FN-LEGACY-FP-CHANGED"), { recursive: true });
+      await writeFile(join(rootDir, ".fusion", "tasks", "FN-LEGACY-FP-CHANGED", "PROMPT.md"), changedPlanText);
+      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-LEGACY-FP-CHANGED", expect.objectContaining({ status: "awaiting-approval" }));
+      expect(store.moveTask).not.toHaveBeenCalled();
+      expect(store.updateTask).not.toHaveBeenCalledWith("FN-LEGACY-FP-CHANGED", expect.objectContaining({ approvedPlanFingerprint: expect.anything() }));
+    });
+
+    it("recoverApprovedTask auto-approves a legacy pre-hygiene fingerprint too", async () => {
+      const legacyFingerprint = computePlanApprovalFingerprint(planText);
+      await mkdir(join(rootDir, ".fusion", "tasks", "FN-LEGACY-RECOVER"), { recursive: true });
+      await writeFile(join(rootDir, ".fusion", "tasks", "FN-LEGACY-RECOVER", "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-LEGACY-RECOVER",
+        description: "Recovered triage task",
+        column: "triage",
+        status: "planning",
+        approvedPlanFingerprint: legacyFingerprint,
+        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-LEGACY-RECOVER", "todo");
+      expect(store.updateTask).not.toHaveBeenCalledWith("FN-LEGACY-RECOVER", expect.objectContaining({ status: "awaiting-approval" }));
+    });
+
+    it("does not write a fingerprint migration when the approval is already post-hygiene", async () => {
+      const approvedPlan = approvedOnDisk(planText, "Triage task");
+      const task = createTriageTask({
+        id: "FN-FP-NO-MIGRATE",
+        status: "planning",
+        approvedPlanFingerprint: computePlanApprovalFingerprint(approvedPlan),
+      } 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,
+        approvedPlan,
+        { requirePlanApproval: true, planApprovalMode: "require-all" } as Settings,
+      );
+
+      expect(store.moveTask).toHaveBeenCalledWith("FN-FP-NO-MIGRATE", "todo");
+      // Already current — no redundant fingerprint write on every pass.
+      expect(store.updateTask).not.toHaveBeenCalledWith("FN-FP-NO-MIGRATE", expect.objectContaining({ approvedPlanFingerprint: expect.anything() }));
+    });
+
    it("never-approved task (no fingerprint) still parks at awaiting-approval on first specify", async () => {
      const task = createTriageTask({
        id: "FN-NEVER-APPROVED",
--- a/packages/engine/src/triage.ts
+++ b/packages/engine/src/triage.ts
@@ -2780,14 +2780,49 @@ export class TriageProcessor {
       * already made their independent decisions, so it never weakens either of those gates
       * or auto-approve-all (which never reaches this branch at all).
       */
+      /*
+       * FNXC:PlanApproval 2026-07-15-15:10:
+       * Accept a fingerprint recorded BEFORE the `## Original Description` hygiene injection
+       * existed (applyOriginalDescription, added 2026-07-14), so the short-circuit above is not
+       * defeated for plans approved by an older build.
+       *
+       * Why this is needed: approve-plan fingerprints the on-disk PROMPT.md. For a plan approved
+       * before the injection shipped, that recorded hash is over PRE-injection content. On the
+       * task's next pass the injection rewrites PROMPT.md, `currentFingerprint` moves, and the
+       * operator is asked to re-approve a plan they already approved and that has not changed.
+       *
+       * `written` diverges from `writtenInput` ONLY via that injection (the sole rewrite in this
+       * method), so `writtenInput` IS the as-approved content for such a task, and hashing it
+       * recovers the legacy fingerprint exactly. This does not weaken the gate: both arms compare
+       * against bytes the operator actually approved — only the representation differs. A plan
+       * that genuinely changed matches neither arm and still parks.
+       *
+       * Migrate the stored fingerprint forward on a legacy match so this is a one-time
+       * reconciliation per task rather than a comparison carried forever.
+       */
      const priorFingerprint = latestTransitionTask?.approvedPlanFingerprint ?? task.approvedPlanFingerprint;
      const currentFingerprint = computePlanApprovalFingerprint(written);
-      if (priorFingerprint && priorFingerprint === currentFingerprint) {
+      const preHygieneFingerprint = written === writtenInput
+        ? currentFingerprint
+        : computePlanApprovalFingerprint(writtenInput);
+      const matchesPriorApproval = Boolean(priorFingerprint)
+        && (priorFingerprint === currentFingerprint || priorFingerprint === preHygieneFingerprint);
+      if (matchesPriorApproval) {
        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`);
+        if (priorFingerprint !== currentFingerprint) {
+          // Direct write, not `taskUpdates` — that batch was already flushed above (line ~2579),
+          // long before this gate runs.
+          await this.store.updateTask(task.id, { approvedPlanFingerprint: currentFingerprint });
+          await this.store.logEntry(
+            task.id,
+            "Approved plan fingerprint migrated to current prompt hygiene format",
+          );
+          planLog.log(`${task.id} approved plan fingerprint migrated to post-hygiene content`);
+        }
      } else {
        /*
         * FNXC:PlanApproval 2026-07-04-21:35: