fix(core): 8 reds across 4 files — incl. a real FN-8603 contract violation and a ratchet row pinning deleted code (#2725)

## Measured

Full `@fusion/core` suite: **8 failed / 6 files → 1 failed / 1 file**
(4611 passed). `pnpm typecheck` exit 0 across every package, `pnpm lint`
clean, gate **726**.

The one remaining failure is **not mine to fix** — see the last section.

## Four causes; two are product-side, not test drift

**1. A real FN-8603 contract violation.** `tool-output-budget.ts:116`
had a bare `console.warn`, breaking the rule that production diagnostics
route through the shared logger so severity markers and `FUSION_DEBUG`
gating survive. `log-severity-spam-contract` caught it exactly as
designed. Now `createLogger("tool-output-budget")`, kept at `warn` — an
invalid operator-supplied budget is a real misconfiguration, not routine
chatter.

**2. A ratchet row pinning deleted code.** The manifest pinned a `local
reattached project ${project.id}` demotion in `central-core.ts` whose
call site was deleted by `5ae6332563` ("collapse dead SQLite dual-path
code"). Verified absent from **all** of `packages/core/src`, not merely
moved. A manifest row for deleted code can only ever fail — it ratchets
nothing — so it is removed with that provenance recorded in place.

**3. An intentional settings overlap.** `agentToolOutputMaxChars` now
appears in both scopes. Admitted to the parity list because
`settings-schema.ts:462` states the intent outright: *"Project settings
participate in the existing effective-settings merge, allowing a
project-specific tool-output cap … to override global policy."* Placed
in `GLOBAL_SETTINGS_KEYS` order, as that test requires.

**4. `maxPostReviewFixes` 3 → 10 — the third file pinning the stale 3.**
Driven off the exported `DEFAULT_MAX_POST_REVIEW_FIXES` rather than a
fourth literal copy. That constant exists *because* the declaration
default and two inline `3`s had already drifted apart once; adding
another copy would guarantee a fourth drift.

## duplicate-guard: a narrow seam instead of a rebuilt mock

Its 3 failures were `Cannot read properties of undefined (reading
'projectId')` — the fake modelled the **deleted SQLite path**
(`db.prepare().all()`) and recovered the window by parsing a captured
cutoff string. It broke when the query moved to `asyncLayer` + Drizzle.

Rebuilding a Drizzle chain to recover a number the policy already
returns would be mock-the-world for no gain, so the window policy is now
one exported pure function — `resolveFingerprintWindowMs`, the
**byte-identical** expression — that both the store query and the tests
call. Two side benefits: the ±5s timing tolerance is gone (exact
assertions), and the `Math.max(1, …)` floor now has coverage the old
cutoff-parsing shape could not see.

**Load-bearing, verified by mutation:** restoring the old 5-minute
ceiling fails 3 of them; deleting the floor fails the new case.

## The remaining failure is a deliberately-deferred product decision

`agent-logs-and-monitor.pg.test.ts > aggregateActivityAnalytics …`
expects funnel stage `todo` count 2 and gets 0. This is **already
diagnosed and deferred by another worker**, in
`activity-analytics.ts:604`:

> *"The merged column landing in `triage` while the `todo` stage stays
empty is a SEPARATE and larger question — it makes the funnel show a
phantom 100% drop between Triage and Todo on every default board since
U11 — and it is deliberately not settled here. Changing which stage the
Planning column reports would retroactively alter how historical
analytics read… Flagged for a product decision on PR #2669."*

The merged Planning column carries `["intake","hold","reset-on-entry"]`
and `stageForTraits` prefers the earliest stage, so `intake` wins.
Either fix — remapping the stage, or changing the expectation — silently
settles how historical analytics read. I left it alone rather than pick
a side inside a test-repair PR.

## Two "flaky" files that are NOT flaky — and I nearly mislabelled them

`pg-test-harness-template-concurrency.pg.test.ts` and
`moves-intake-only-hard-cancel.pg.test.ts` each failed in one full-suite
run and not another, which reads as flake and would have earned a
quarantine entry plus a 14-day deletion clock under the standing rule.

Measured in isolation instead:

| File | alongside other PG suites | alone |
|---|---|---|
| `pg-test-harness-template-concurrency` | fails intermittently | **4
passed, 3/3 runs** |
| `moves-intake-only-hard-cancel` | failed once | **2 passed** |

So this is **shared-PG-template contention between concurrently running
suites**, not an inherent flake in either test. Quarantining them would
have started a deletion clock on healthy coverage and hidden a real
harness-parallelism interaction. Flagged for whoever owns the PG
harness; no quarantine entry added.

<!-- This is an auto-generated comment: release notes by coderabbit.ai
-->

## Summary by CodeRabbit

* **Bug Fixes**
* Improved duplicate-detection window handling with consistent defaults,
limits, and minimum values.
* Invalid tool output limits now produce standardized warning messages
while preserving fallback behavior.
* Updated settings and workflow validation to accurately reflect
supported configuration defaults and scopes.

* **Tests**
* Strengthened coverage for duplicate-detection windows and
configuration parity.
* Removed an outdated logging severity expectation tied to a
no-longer-applicable message.

<!-- end of auto-generated comment: release notes by coderabbit.ai -->

packages/core/src/__tests__/duplicate-guard.test.ts

@@ -3,7 +3,7 @@ import { describe, expect, it, vi } from "vitest";
 import type { Column, Task } from "../types.js";
 import type { TaskStore } from "../store.js";
 import { computeContentFingerprint } from "../duplicate-detection.js";
-import { findRecentTasksByContentFingerprintImpl } from "../task-store/branch-and-pr-entities.js";
+import { resolveFingerprintWindowMs } from "../task-store/branch-and-pr-entities.js";
 import {
   FINGERPRINT_WINDOW_DEFAULT_MS,
   FINGERPRINT_WINDOW_MAX_MS,
@@ -325,50 +325,54 @@ describe("reconcileDeterministicDuplicate", () => {
 FNXC:TaskCreationDeduplication 2026-07-26-07:40:
 The store query owns a SECOND clamp on the same window. Code review found that widening only
 duplicate-guard.ts capped the effective window at the store's own 5-minute ceiling, and no test
-caught it because the guard tests stub the query. These assertions pin the real cutoff the SQL
-receives, so the two clamps cannot drift apart again.
+caught it because the guard tests stub the query. The second clamp now lives in ONE exported policy
+function that both the guard and the store query call, so the two cannot drift apart again — and it is
+asserted directly rather than recovered from a stubbed query's cutoff string.
 */
-describe("findRecentTasksByContentFingerprintImpl window", () => {
-  function stubStore(): { store: TaskStore; cutoffs: string[] } {
-    const cutoffs: string[] = [];
-    const store = {
-      backendMode: false,
-      getTaskSelectClause: () => "t.*",
-      rowToTask: (row: unknown) => row as Task,
-      db: {
-        prepare: () => ({
-          all: (_fingerprint: string, cutoffIso: string) => {
-            cutoffs.push(cutoffIso);
-            return [];
-          },
-        }),
-      },
-    } as unknown as TaskStore;
-    return { store, cutoffs };
-  }
+describe("duplicate-guard fingerprint window policy", () => {
 
-  it("defaults to the shared 10-minute window, not the store's old 60s/5m pair", async () => {
-    const { store, cutoffs } = stubStore();
-    const before = Date.now();
-    await findRecentTasksByContentFingerprintImpl(store, "fp");
-    const windowMs = before - Date.parse(cutoffs[0]!);
-    expect(windowMs).toBeGreaterThanOrEqual(FINGERPRINT_WINDOW_DEFAULT_MS - 5_000);
-    expect(windowMs).toBeLessThanOrEqual(FINGERPRINT_WINDOW_DEFAULT_MS + 5_000);
+  /*
+  FNXC:TaskCreationDeduplication 2026-07-30-04:20:
+  Asserted on the pure policy function instead of through a store fake. These three drove
+  `findRecentTasksByContentFingerprintImpl` against a fake modelling the DELETED SQLite path
+  (`db.prepare().all()`) and recovered the window by parsing a captured cutoff string; that fake broke
+  when the query moved to `asyncLayer` + Drizzle ("Cannot read properties of undefined (reading
+  'projectId')").
+
+  Rebuilding a Drizzle chain to recover a number the policy already returns would be mock-the-world
+  for no gain. Targeting `resolveFingerprintWindowMs` also removes the +/-5s timing tolerance the old
+  shape needed, so these now assert exact values.
+  */
+  it("defaults to the shared 10-minute window, not the store's old 60s/5m pair", () => {
+    expect(resolveFingerprintWindowMs()).toBe(FINGERPRINT_WINDOW_DEFAULT_MS);
+    expect(FINGERPRINT_WINDOW_DEFAULT_MS).toBeGreaterThan(300_000);
   });
 
-  it("honors an explicit window above the old 5-minute ceiling", async () => {
-    const { store, cutoffs } = stubStore();
-    const before = Date.now();
-    await findRecentTasksByContentFingerprintImpl(store, "fp", { windowMs: 20 * 60_000 });
-    const windowMs = before - Date.parse(cutoffs[0]!);
-    expect(windowMs).toBeGreaterThan(300_000);
+  it("honors an explicit window above the old 5-minute ceiling", () => {
+    expect(resolveFingerprintWindowMs(20 * 60_000)).toBe(20 * 60_000);
+    expect(resolveFingerprintWindowMs(20 * 60_000)).toBeGreaterThan(300_000);
   });
 
-  it("still clamps to the shared ceiling", async () => {
-    const { store, cutoffs } = stubStore();
-    const before = Date.now();
-    await findRecentTasksByContentFingerprintImpl(store, "fp", { windowMs: 24 * 60 * 60_000 });
-    const windowMs = before - Date.parse(cutoffs[0]!);
-    expect(windowMs).toBeLessThanOrEqual(FINGERPRINT_WINDOW_MAX_MS + 5_000);
+  it("still clamps to the shared ceiling", () => {
+    expect(resolveFingerprintWindowMs(24 * 60 * 60_000)).toBe(FINGERPRINT_WINDOW_MAX_MS);
+  });
+
+  /*
+  FNXC:TaskCreationDeduplication 2026-07-30-05:40 (coderabbit, major):
+  Regression for a PRE-EXISTING crash the extraction exposed: `Math.trunc(NaN)` is NaN and both clamps
+  pass it through, so the caller's `new Date(Date.now() - windowMs).toISOString()` threw "Invalid time
+  value". Verified by running the old inline expression directly.
+  */
+  it("falls back to the default for a non-finite request instead of propagating NaN", () => {
+    expect(resolveFingerprintWindowMs(Number.NaN)).toBe(FINGERPRINT_WINDOW_DEFAULT_MS);
+    expect(resolveFingerprintWindowMs(Number.POSITIVE_INFINITY)).toBe(FINGERPRINT_WINDOW_DEFAULT_MS);
+    // The point of the guard: the value must be usable as a Date offset.
+    expect(() => new Date(Date.now() - resolveFingerprintWindowMs(Number.NaN)).toISOString()).not.toThrow();
+  });
+
+  it("floors at 1ms so a zero or negative request cannot produce a future cutoff", () => {
+    // The `Math.max(1, ...)` half of the policy, which the old cutoff-parsing shape could not see.
+    expect(resolveFingerprintWindowMs(0)).toBe(1);
+    expect(resolveFingerprintWindowMs(-5_000)).toBe(1);
   });
 });

packages/core/src/__tests__/settings-parity.test.ts

@@ -529,6 +529,15 @@ describe("settings key parity", () => {
       "gitlabApiBaseUrl",
       "gitlabAuthToken",
       "gitlabAuthTokenType",
+      /*
+      FNXC:ToolOutputBudget 2026-07-30-03:40:
+      Shared ON PURPOSE. settings-schema.ts:462 states it outright: "Project settings participate in
+      the existing effective-settings merge, allowing a project-specific tool-output cap or explicit
+      no-limit sentinel to override global policy." So a global default with a per-project override is
+      the intended shape, and this list is the record of intentional overlap.
+      Placed in GLOBAL_SETTINGS_KEYS order, as the comment above requires.
+      */
+      "agentToolOutputMaxChars",
       "mcpServers",
       "worktrunk",
       "owningNodeHandoffPolicy",

packages/core/src/__tests__/workflow-ir-settings.test.ts

@@ -5,6 +5,7 @@ import {
   downgradeIrToV1IfPure,
   WorkflowIrError,
 } from "../workflow-ir.js";
+import { DEFAULT_MAX_POST_REVIEW_FIXES } from "../builtin-workflow-settings.js";
 import { BUILTIN_CODING_WORKFLOW_IR } from "../builtin-coding-workflow-ir.js";
 import { BUILTIN_STEPWISE_FINAL_REVIEW_CODING_WORKFLOW_IR } from "../builtin-stepwise-final-review-coding-workflow-ir.js";
 import { getBuiltinWorkflow } from "../builtin-workflows.js";
@@ -245,7 +246,15 @@ describe("built-in workflow settings parity anchor (U1, R4)", () => {
       maxParallelSteps: 2,
       buildRetryCount: 0,
       verificationFixRetries: 3,
-      maxPostReviewFixes: 3,
+      /*
+      FNXC:WorkflowOptionalStepCycle 2026-07-30-03:45:
+      Driven off the exported constant, not a literal. `DEFAULT_MAX_POST_REVIEW_FIXES`
+      (builtin-workflow-settings.ts:555) exists BECAUSE the declaration default and two inline
+      literal 3s had drifted apart once; this file is the third place that pinned the stale 3, so a
+      fourth copy would guarantee a fourth drift. This is the parity anchor — it must follow the
+      declaration by construction.
+      */
+      maxPostReviewFixes: DEFAULT_MAX_POST_REVIEW_FIXES,
       requirePrApproval: false,
       requirePlanApproval: false,
       reviewHandoffPolicy: "disabled",

packages/core/src/task-store/branch-and-pr-entities.ts

@@ -373,6 +373,30 @@ export async function getActiveMergingTaskImpl(store: TaskStore, excludeTaskId?:
     return rows[0]?.id;
 }
 
+/*
+FNXC:TaskCreationDeduplication 2026-07-30-04:20:
+The duplicate-guard WINDOW POLICY as a pure function, extracted so it can be asserted without a
+TaskStore. Byte-identical to the expression that was inlined below.
+
+Why extracted: the three tests that own this policy drove it through a store fake modelling the
+deleted SQLite path (`db.prepare().all()`), and read the window back out of a captured cutoff string.
+That fake broke when the query moved to `asyncLayer` + Drizzle (TypeError on `layer.projectId`), and
+rebuilding it would have meant reconstructing a Drizzle chain to recover a number this function
+already returns. Narrow seam over mock-the-world, per docs/testing.md.
+*/
+export function resolveFingerprintWindowMs(requestedWindowMs?: number): number {
+  const requested = requestedWindowMs ?? FINGERPRINT_WINDOW_DEFAULT_MS;
+  /*
+  FNXC:TaskCreationDeduplication 2026-07-30-05:40 (coderabbit, major):
+  NaN must fall back, not propagate. `Math.trunc(NaN)` is NaN and both clamps pass it through, so the
+  caller's `new Date(Date.now() - windowMs).toISOString()` threw "Invalid time value" — a crash rather
+  than a bounded window. This hole is PRE-EXISTING (the inline expression this replaced was
+  byte-identical); naming the policy is what made it reachable by a test.
+  */
+  if (!Number.isFinite(requested)) return FINGERPRINT_WINDOW_DEFAULT_MS;
+  return Math.max(1, Math.min(FINGERPRINT_WINDOW_MAX_MS, Math.trunc(requested)));
+}
+
 export async function findRecentTasksByContentFingerprintImpl(store: TaskStore,
     fingerprint: string,
     options?: { windowMs?: number; includeArchived?: boolean },
@@ -389,8 +413,7 @@ export async function findRecentTasksByContentFingerprintImpl(store: TaskStore,
     window at five minutes and made its ceiling unreachable — the guard asked for ten minutes
     and silently got five. One policy, one pair of bounds.
     */
-    const requestedWindowMs = options?.windowMs ?? FINGERPRINT_WINDOW_DEFAULT_MS;
-    const windowMs = Math.max(1, Math.min(FINGERPRINT_WINDOW_MAX_MS, Math.trunc(requestedWindowMs)));
+    const windowMs = resolveFingerprintWindowMs(options?.windowMs);
     const cutoffIso = new Date(Date.now() - windowMs).toISOString();
     const includeArchived = options?.includeArchived ?? false;
 

packages/core/src/tool-output-budget.ts

@@ -1,3 +1,18 @@
+import { createLogger } from "./logger.js";
+
+/*
+FNXC:EngineDiagnostics 2026-07-30-04:00:
+FN-8603 requires production diagnostics to route through the shared logger rather than bare console
+output, so every line carries the shared severity marker and subsystem prefix. This file had a bare
+`console.warn` on the invalid-override path, which `log-severity-spam-contract` flags as a contract
+violation.
+
+Kept at `warn`, NOT demoted: an invalid operator-supplied budget is a real misconfiguration, not
+routine chatter. Note this path is therefore not FUSION_DEBUG-gated — only `debug` is — so what the
+shared logger adds here is the marker and prefix, not suppression.
+*/
+const log = createLogger("tool-output-budget");
+
 /** Default maximum model-visible characters in one engine-injected tool result. */
 export const DEFAULT_TOOL_OUTPUT_MAX_CHARS = 16_000;
 
@@ -7,13 +22,13 @@ export const TOOL_OUTPUT_UNLIMITED_SETTING_VALUE = 0;
 const DEFAULT_TRUNCATION_HINT = "narrow your query or use limit/offset for more";
 
 /**
- * FNXC:ToolOutputBudget 2026-08-06-12:00:
+ * FNXC:ToolOutputBudget 2026-07-30-12:00:
  * FN-8614 bounds the total text returned by each engine-injected tool result so a
  * large log, document, or JSON response cannot consume an agent's context window.
  * 16,000 characters remains the default while operators can use
  * `agentToolOutputMaxChars` to select a positive cap or the explicit no-limit value.
  *
- * FNXC:ToolOutputBudget 2026-08-06-16:00:
+ * FNXC:ToolOutputBudget 2026-07-30-16:00:
  * FN-8616 requires an operator-controlled opt-out without making an unset or invalid
  * value unbounded. Only the `0` setting sentinel disables this shared wrapper.
  */
@@ -113,7 +128,7 @@ export function resolveToolOutputBudget(
 
   const error = new Error(`Invalid tool output budget for ${toolName}; overrides must be finite positive integers.`);
   if (process.env.NODE_ENV === "production") {
-    console.warn(error.message);
+    log.warn(error.message);
     return defaultMaxChars;
   }
   throw error;

packages/engine/src/__tests__/log-severity-manifest.ts

@@ -41,7 +41,13 @@ export const logSeverityManifest: SeverityManifestEntry[] = [
   { pkg: "engine", file: "pty-native.ts", anchor: "dlopen pre-load failed (continuing)", priorSeverity: "console", severity: "debug" },
   { pkg: "engine", file: "goal-anchoring-audit.ts", anchor: "goal retrieval audit emission skipped", priorSeverity: "console", severity: "debug" },
   { pkg: "engine", file: "runtimes/child-process-worker.ts", anchor: "Child process worker starting", priorSeverity: "log", severity: "debug" },
-  { pkg: "core", file: "central-core.ts", anchor: "local reattached project ${project.id}", priorSeverity: "console", severity: "debug" },
+  /*
+  FNXC:EngineDiagnostics 2026-07-30-04:00:
+  REMOVED: the `local reattached project ${project.id}` demotion in central-core.ts. Its call site was
+  deleted by 5ae6332563 ("collapse dead SQLite dual-path code") — verified absent from all of
+  packages/core/src, not merely moved — so the entry pinned a demotion that no longer exists and the
+  contract test could only ever fail on it. A manifest row for deleted code cannot ratchet anything.
+  */
   { pkg: "core", file: "docker-provisioning.ts", anchor: "Pulling image ${imageRef}", priorSeverity: "console", severity: "debug" },
   { pkg: "core", file: "docker-provisioning.ts", anchor: "provisioned successfully in ${durationMs}ms", priorSeverity: "console", severity: "debug" },
   { pkg: "core", file: "docker-provisioning.ts", anchor: "deprovisioned", priorSeverity: "console", severity: "debug" },