FUSI-034

docs/solutions/test-failures/store-fake-defects-that-masquerade-as-production-bugs.md

@@ -182,6 +182,68 @@ throwing before its renamed-lane assertion. The merge-queue-peek addition was
 See `dead-vi-mock-specifiers-fail-silently.md` for the related case where a mock
 factory is unwired rather than a store fake being incomplete.
 
+## FUSI-034: a prototype-only fake, and four more drifts that each looked like a product bug
+
+The `assertion-no-error-line` family (FUSI-020 Step 5, family 5 of 7) resolved six
+red engine suites, and five of them are this same catalogue: a hand-maintained test
+double drifted from a contract that moved, and the drift surfaced as an assertion
+failure attributed to production code.
+
+**The tell.** A `TypeError: Cannot read properties of undefined (reading 'has')`
+surfacing from deep inside `project-engine.ts` is a **test-double** defect, not a
+product crash. `internalEnqueueMerge` read `this.mergeRetryResetTaskIds.has(taskId)`
+on a receiver built by `Object.create(ProjectEngine.prototype)`, which runs **no
+class field initializers**, so every merge-lane field was `undefined`. A production
+`ProjectEngine` always has them. The two `Set` fields were added by FN-9317
+(`706c15560`); `_project-engine-merge-lane-fixture.ts` — whose own FNXC note exists
+precisely to prevent this drift — never learned them. This is the **second** time
+this fixture family has fallen behind production; the first was FUSI-030's missing
+`getTask` collaborator. The disappearance of the 5 unhandled rejections alongside the
+8 assertion failures is the tell that they were one defect surfacing on two paths.
+
+**The rule that generalises.** When a class field is added to a production class
+whose instances are faked via `Object.create(Prototype)` plus a hand-maintained state
+fixture, the fixture is now wrong — and `tsc` cannot see it. `packages/engine/tsconfig.json`
+is `{"include": ["src/**/*"], "exclude": ["src/__tests__/**/*"]}`, and these fakes are
+`as any`/`as never` cast anyway, so `pnpm verify:fast` is structurally blind to them and
+returns 0 whether the fixture is complete or not. **The only detector is running the
+suites.** A green typecheck is not evidence that a fake matches its class.
+
+Walking the full class-field diff (FN-5893) found a second latent gap the ratchet had
+not caught: `mergeBodySettleTimeoutMs` is declared at `project-engine.ts:1015` and read
+at `:1037`, *outside* the region the existing drift ratchet scans. Left undefined, the
+wait collapses to roughly 1 ms instead of the production 60 s, so any test that exercises
+the settle would have raced silently rather than failing loudly.
+
+**The post-merge evidence shape.** A completion guard that reads the workflow IR
+(`getRequiredPostMergeEvidenceBlocker`, FN-9370) turns *any* test store that exposes
+`getTaskWorkflowSelection` into a fixture that must also declare its post-merge gate
+evidence — the guard early-outs entirely when that method is absent, so exposing it for
+an unrelated reason silently opts the fake into a contract it never satisfied. Two
+suites drifted on the same change in the same way: one went from `expected 'blocked' to
+be 'done'`, the other from a merge-region node list that was one entry short. Note the
+direction: these are **completeness** defects, not the missing-method defects in the
+catalogue above. Exposing a method is not free.
+
+**The environment shape.** A `password authentication failed for user "<os-user>"`
+during `CREATE DATABASE` is a **provisioning gap, not a test defect**. The fixture's
+maintenance URL defaults to `postgresql://localhost:5432` with no user or password, so
+the client authenticates as the OS user (`mini` locally, `runner` in CI); CI is green
+only because the workflows pass `FUSION_PG_TEST_URL_BASE` with an explicit `postgres`
+user (`full-suite.yml:44-45`, `pr-checks.yml:206-207`). Route this to provisioning. Do
+**not** wrap `createPgLayer` in `try`/`catch` or teach the fixture to tolerate auth
+failure — that deletes the coverage the fixture exists to provide, and it converts a
+loud provisioning error into a silently skipped suite.
+
+**A fourth drift, from the same class, worth its own line:** a review gate's identity
+is its **optional group id**, not the inner template step id, because the executor
+resolves `effectiveWorkflowStepId = optionalGroupId ?? id.replace(/^graph:/, "")`. An
+audit assertion and a persisted-result fixture both encoded `"code-review-step"` /
+`"plan-review-step"` and went red together. When a test asserts an id that a
+constructed fixture *also* sets, derive it from the fixture's own field rather than
+retyping the literal — the drift is invisible precisely because the two copies agree
+with each other and disagree with production.
+
 ## Related
 
 - `docs/testing.md` — testing lanes and the taxonomy for trim-vs-keep.

packages/engine/src/__tests__/_project-engine-merge-lane-fixture.ts

@@ -1,6 +1,8 @@
 type MergeLaneState = {
   mergeQueue: string[];
   mergeActive: Set<string>;
+  mergeRetryResetTaskIds: Set<string>;
+  mergeEnqueueDeferredByRetryReset: Set<string>;
   capacityDeferredMergeTaskIds: Set<string>;
   capacityDeferredMergeReasons: Map<string, string>;
   capacityDeferredMerges: Map<string, unknown>;
@@ -19,6 +21,7 @@ type MergeLaneState = {
   workspaceBusyReenqueues: Map<string, number>;
   workspaceBusyReenqueueTimers: Set<ReturnType<typeof setTimeout>>;
   manualMergeResolvers: Map<string, unknown[]>;
+  mergeBodySettleTimeoutMs: number;
   shuttingDown: boolean;
   startupGeneration: number;
   started: boolean;
@@ -29,6 +32,21 @@ type MergeLaneState = {
  * Object.create(ProjectEngine.prototype) runs no class field initializers, so a prototype-only
  * merge fake starts with every merge-lane field undefined. FN-8871 requires this fixture to include
  * capacity and PR-retry merge state, preventing production drain additions from drifting test fakes.
+ *
+ * FNXC:MergeQueue 2026-09-26-05:45:
+ * The fixture fell behind production for the SECOND time (the first was FUSI-030's missing `getTask`
+ * collaborator). FN-9317 (`706c15560` "recover stalled in-review merges") added `mergeRetryResetTaskIds`
+ * and `mergeEnqueueDeferredByRetryReset` to ProjectEngine; this fixture never learned them, so
+ * `internalEnqueueMerge` threw `TypeError: Cannot read properties of undefined (reading 'has')` at
+ * project-engine.ts:3123 and 8 cases in merge-abort-clears-transient-status.test.ts went red with an
+ * error that read like a production crash. The tell is that a TypeError from deep inside
+ * project-engine.ts can only be a fake defect: a real engine always runs its field initializers.
+ * `mergeBodySettleTimeoutMs` is seeded here too. It is declared at project-engine.ts:1015, OUTSIDE the
+ * `// ── Auto-merge state ──` block that project-engine-merge-lane-fixture-drift.test.ts scans, so the
+ * ratchet cannot see it — but `awaitPriorMergeBodySettle` reads it, and on an unseeded prototype fake
+ * `setTimeout(fn, undefined)` collapses the 60s drain latch to ~1ms. The tests that reach it each
+ * override it to 1 by hand, which is exactly the smell: every consumer patches around the gap instead
+ * of the gap being closed. Seed it with its production default and leave those overrides alone.
  */
 export function seedMergeLaneState<T extends object>(
   engine: T,
@@ -37,6 +55,11 @@ export function seedMergeLaneState<T extends object>(
   const defaults: MergeLaneState = {
     mergeQueue: [],
     mergeActive: new Set(),
+    /* FNXC:MergeRetryAdmission 2026-09-26-05:45: the process-local fence between Chat's
+       status-none reset and queue admission (FN-9317). Empty is the production-equivalent default —
+       no reset is committing on a fresh engine. */
+    mergeRetryResetTaskIds: new Set(),
+    mergeEnqueueDeferredByRetryReset: new Set(),
     capacityDeferredMergeTaskIds: new Set(),
     capacityDeferredMergeReasons: new Map(),
     capacityDeferredMerges: new Map(),
@@ -57,6 +80,9 @@ export function seedMergeLaneState<T extends object>(
     workspaceBusyReenqueues: new Map(),
     workspaceBusyReenqueueTimers: new Set(),
     manualMergeResolvers: new Map(),
+    /* FNXC:MergeQueue 2026-09-26-05:45: the orphan-body drain latch, 60s in production
+       (project-engine.ts:1015). Undefined on a prototype fake collapses it to ~1ms. */
+    mergeBodySettleTimeoutMs: 60_000,
     shuttingDown: false,
     startupGeneration: 0,
     started: true,

packages/engine/src/__tests__/executor-paused-abort-todo-benign.test.ts

@@ -187,10 +187,33 @@ describe("pause-abort benign requeue-to-todo (FN-6782)", () => {
       expect.objectContaining({ graphResumeRetryCount: 1 }),
       expect.anything(),
     );
-    expect(store.updateTask).toHaveBeenCalledWith(
+    /*
+    FNXC:LifecycleContainment 2026-09-26-07:05:
+    The `status: "failed"` park this block used to assert NO LONGER HAPPENS, and asserting it would
+    now require a product change that AGENTS.md forbids ("A Behavior Change Owns Every Test That
+    Asserts the Old Behavior" cuts the other way here: containment owns this one). FN-207/FN-217
+    removed backward-move authority from automatic recovery. `route-graph-failure-to-execution-resume.ts:409`
+    now refuses to claim a WIP card it cannot move forward, and the live row is deliberately left
+    untouched: the probe for these exact inputs produced ZERO updateTask calls and the log line
+      "Workflow graph failed at node 'unknown' - automatic recovery cannot move 'in-progress'
+       backward; card remains in place"
+    This mirrors the established in-repo pattern (executor-prompt.test.ts:1680-1686): assert the row
+    was NOT parked and NOT moved, and assert the retention was narrated. No product source is edited.
+
+    The test's real subject is preserved and is still proven per variant: no transient in-place retry
+    (graphResumeRetryCount is not bumped, execute() is never re-dispatched). The durable inputs
+    (lastError, failureReason, exhausted graphResumeRetryCount) are what make this a terminal,
+    non-retryable failure, and the no-retry contract is what each variant still asserts.
+    */
+    expect(store.updateTask).not.toHaveBeenCalledWith(
       task.id,
       expect.objectContaining({ status: "failed" }),
-      undefined,
+      expect.anything(),
+    );
+    expect(store.moveTask).not.toHaveBeenCalled();
+    expect(task.column).toBe("in-progress");
+    expect(logText(store)).toContain(
+      "automatic recovery cannot move 'in-progress' backward; card remains in place",
     );
     expect(executeSpy).not.toHaveBeenCalled();
   });