From c61164853c70d80ea7dd7ad2a0bc276d7745feba Mon Sep 17 00:00:00 2001 From: gsxdsm Date: Thu, 25 Jun 2026 22:38:12 -0700 Subject: [PATCH] test(FN-7039): update stale visitedNodeIds fixtures for code-review optional-group Prior WIP added a default-on code-review optional-group to builtin:coding on the success path (execute -> browser-verification -> code-review -> review). 9 exact visitedNodeIds expectations across 3 suites predated that node and were failing. Updated expectations to match the real traversal (insert code-review in position). No product code touched; no assertions weakened. Co-Authored-By: Claude Opus 4.8 (1M context) --- ...aph-executor-retry-coding-workflow.test.ts | 9 ++++- ...rkflow-graph-merge-region-collapse.test.ts | 39 +++++++++++++++++-- .../__tests__/workflow-task-runtime.test.ts | 11 +++++- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/packages/engine/src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts b/packages/engine/src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts index 8a706712c2..6f24fe4252 100644 --- a/packages/engine/src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-executor-retry-coding-workflow.test.ts @@ -102,7 +102,14 @@ describe("WorkflowGraphExecutor built-in coding workflow retries", () => { expect(result.outcome).toBe("failure"); // U6: with browser-verification disabled (bypassed), the group node sits // between execute and review where the workflow-step seam used to. - expect(result.visitedNodeIds).toEqual(["start", "planning", "execute", "browser-verification", "review"]); + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "execute", + "browser-verification", + "code-review", + "review", + ]); }); it("respects a per-node maxRetries override", async () => { diff --git a/packages/engine/src/__tests__/workflow-graph-merge-region-collapse.test.ts b/packages/engine/src/__tests__/workflow-graph-merge-region-collapse.test.ts index 2dc62da92a..df27b432f0 100644 --- a/packages/engine/src/__tests__/workflow-graph-merge-region-collapse.test.ts +++ b/packages/engine/src/__tests__/workflow-graph-merge-region-collapse.test.ts @@ -66,7 +66,15 @@ describe("WorkflowGraphExecutor merge-region collapse", () => { expect(result.outcome).toBe("success"); expect(merge).toHaveBeenCalledOnce(); expect(calls).toEqual(["merge"]); - expect(result.visitedNodeIds).toEqual(["start", "planning", "execute", "browser-verification", "review", "merge"]); + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "execute", + "browser-verification", + "code-review", + "review", + "merge", + ]); expect(result.context["node:merge:outcome"]).toBe("success"); expectNoRawMergeRegionVisits(result.visitedNodeIds); }); @@ -79,7 +87,15 @@ describe("WorkflowGraphExecutor merge-region collapse", () => { expect(result.outcome).toBe("failure"); expect(merge).toHaveBeenCalledOnce(); - expect(result.visitedNodeIds).toEqual(["start", "planning", "execute", "browser-verification", "review", "merge"]); + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "execute", + "browser-verification", + "code-review", + "review", + "merge", + ]); expect(result.context["node:merge:outcome"]).toBe("failure"); expect(result.context["node:merge:value"]).toBe("FileScopeViolationError"); expectNoRawMergeRegionVisits(result.visitedNodeIds); @@ -98,7 +114,14 @@ describe("WorkflowGraphExecutor merge-region collapse", () => { expect(result.outcome).toBe("failure"); expect(merge).not.toHaveBeenCalled(); - expect(result.visitedNodeIds).toEqual(["start", "planning", "execute", "browser-verification", "review"]); + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "execute", + "browser-verification", + "code-review", + "review", + ]); expect(result.visitedNodeIds).not.toContain("merge"); expectNoRawMergeRegionVisits(result.visitedNodeIds); }); @@ -113,7 +136,15 @@ describe("WorkflowGraphExecutor merge-region collapse", () => { expect(result.outcome).toBe("success"); expect(merge).toHaveBeenCalledOnce(); - expect(result.visitedNodeIds).toEqual(["start", "planning", "execute", "browser-verification", "review", "merge"]); + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "execute", + "browser-verification", + "code-review", + "review", + "merge", + ]); expectNoRawMergeRegionVisits(result.visitedNodeIds); }, ); diff --git a/packages/engine/src/__tests__/workflow-task-runtime.test.ts b/packages/engine/src/__tests__/workflow-task-runtime.test.ts index 5cc7c90f37..0f94cf17ba 100644 --- a/packages/engine/src/__tests__/workflow-task-runtime.test.ts +++ b/packages/engine/src/__tests__/workflow-task-runtime.test.ts @@ -298,7 +298,15 @@ describe("WorkflowTaskRuntime", () => { // U6: no `workflow-step` seam; the bypassed browser-verification group node // sits between execute and review in the visited sequence. expect(calls).toEqual(["planning", "prepare-worktree", "execute", "review", "merge"]); - expect(result.visitedNodeIds).toEqual(["start", "planning", "execute", "browser-verification", "review", "merge"]); + expect(result.visitedNodeIds).toEqual([ + "start", + "planning", + "execute", + "browser-verification", + "code-review", + "review", + "merge", + ]); }); it("runs the pre-merge browser-verification optional-group once when enabled, before review", async () => { @@ -338,6 +346,7 @@ describe("WorkflowTaskRuntime", () => { // The group container node, then its inner template step (run once). "browser-verification", "browser-verification::browser-verification-step", + "code-review", "review", "merge", ]);