From 0b6957a1b28b69bbf7d589f94a6df7086b4db6ba Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Fri, 17 Jul 2026 14:19:28 -0700 Subject: [PATCH] FN-8241: log Plan Review revisions distinctly Classify requested Plan Review changes as normal specification revision activity rather than workflow failures. - Replace failure log actions for deterministic and reviewer-requested revisions - Preserve failed workflow-result state for replan and recovery processing - Cover revision logging across deterministic, reviewer, and retry paths Files changed: .../triage-review-spec-external-integration.test.ts | 5 ++++- packages/engine/src/__tests__/triage.test.ts | 11 +++++++++++ packages/engine/src/triage.ts | 14 ++++++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) Fusion-Task-Id: FN-8241 Fusion-Task-Lineage: 316370ac-36c2-45b5-944f-602375167cdd Co-authored-by: Fusion (runfusion.ai) --- ...triage-review-spec-external-integration.test.ts | 5 ++++- packages/engine/src/__tests__/triage.test.ts | 11 +++++++++++ packages/engine/src/triage.ts | 14 ++++++++++++-- 3 files changed, 27 insertions(+), 3 deletions(-) diff --git a/packages/engine/src/__tests__/triage-review-spec-external-integration.test.ts b/packages/engine/src/__tests__/triage-review-spec-external-integration.test.ts index b5a144acfc..b00d2e7a6b 100644 --- a/packages/engine/src/__tests__/triage-review-spec-external-integration.test.ts +++ b/packages/engine/src/__tests__/triage-review-spec-external-integration.test.ts @@ -164,9 +164,12 @@ describe("triage deterministic plan validation for external integration evidence expect(result).toBe("blocked"); expect(reviewStep).not.toHaveBeenCalled(); expect(store.updateTask).toHaveBeenCalledWith(taskId, expect.objectContaining({ status: "needs-replan" })); + expect((store.logEntry as ReturnType).mock.calls.some( + ([id, action]) => id === taskId && action === "[pre-merge] Workflow step failed: Plan Review", + )).toBe(false); expect(store.logEntry).toHaveBeenCalledWith( taskId, - "[pre-merge] Workflow step failed: Plan Review", + "AI spec revision requested", expect.stringContaining("External-integration evidence gaps"), ); } finally { diff --git a/packages/engine/src/__tests__/triage.test.ts b/packages/engine/src/__tests__/triage.test.ts index 31e8f9d966..7e484e5d1a 100644 --- a/packages/engine/src/__tests__/triage.test.ts +++ b/packages/engine/src/__tests__/triage.test.ts @@ -1698,6 +1698,14 @@ Planner rewrote mission without the raw request. expect.objectContaining({ workflowStepId: "plan-review", status: "failed", verdict: "REVISE" }), ]), })); + expect((store.logEntry as ReturnType).mock.calls.some( + ([id, action]) => id === "FN-PLAN-REVISE" && action === "[pre-merge] Workflow step failed: Plan Review", + )).toBe(false); + expect(store.logEntry).toHaveBeenCalledWith( + "FN-PLAN-REVISE", + "AI spec revision requested", + expect.stringContaining("Missing verification."), + ); }); it("keeps the task in triage with retry backoff when Plan Review is unavailable", async () => { @@ -2247,6 +2255,9 @@ Planner rewrote mission without the raw request. "AI spec revision requested", expect.stringContaining(feedback), ); + expect((retryStore.logEntry as ReturnType).mock.calls.some( + ([id, action]) => id === taskId && action === "[pre-merge] Workflow step failed: Plan Review", + )).toBe(false); expect(retryStore.updateTask).toHaveBeenCalledWith(taskId, expect.objectContaining({ workflowStepResults: expect.arrayContaining([ expect.objectContaining({ workflowStepId: "plan-review", status: "failed", verdict: "REVISE" }), diff --git a/packages/engine/src/triage.ts b/packages/engine/src/triage.ts index 68ffc58da0..1a13f120d5 100644 --- a/packages/engine/src/triage.ts +++ b/packages/engine/src/triage.ts @@ -2339,7 +2339,12 @@ export class TriageProcessor { startedAt, completedAt, }); - await this.store.logEntry(task.id, "[pre-merge] Workflow step failed: Plan Review", diagnostic); + /* + FNXC:TriagePlanReview 2026-07-17-14:04: + A requested Plan Review revision is normal replan flow, not a terminal workflow-step + failure. Log only the dedicated revision action here; the pre-merge failed action is + reserved for terminal failures such as RetryStormError. + */ await this.store.logEntry( task.id, "AI spec revision requested", @@ -2502,7 +2507,12 @@ export class TriageProcessor { startedAt, completedAt, }); - await this.store.logEntry(task.id, "[pre-merge] Workflow step failed: Plan Review", review.review); + /* + FNXC:TriagePlanReview 2026-07-17-14:04: + Reviewer REVISE and RETHINK outcomes re-enter planning through the same normal revision + flow as deterministic evidence gaps. Their single human-facing log is the revision action; + retain the failed workflow-result projection for replan/recovery without emitting a failed log. + */ await this.clearPlanReviewRecoveryBudget(task); const reviseFeedback = review.review || review.summary || "(no feedback captured)"; await this.store.logEntry(