Skip to content

fix(core): wedge notifications could never be resolved on PostgreSQL (42P18) - #2669

Merged
gsxdsm merged 2 commits into
mainfrom
fix/wedge-notification-pg-param-type
Jul 30, 2026
Merged

fix(core): wedge notifications could never be resolved on PostgreSQL (42P18)#2669
gsxdsm merged 2 commits into
mainfrom
fix/wedge-notification-pg-param-type

Conversation

@gsxdsm

@gsxdsm gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator

Not a U12 change — found while attributing the pre-existing live-PG failures during U12's closing verification, and it turned out to be a product bug rather than a stale test.

The defect

resolveWedgeNotification builds its UPDATE with:

jsonb_build_object('status', 'resolved', 'transitionedAt', ${transitionedAt})

jsonb_build_object is variadic "any", so there is no signature for PostgreSQL to resolve the bind parameter against. It rejects the statement at parse time:

42P18: could not determine data type of parameter $1

Parse-time is the important part: this failed on every call, not on unusual data. Wedge notifications could not be resolved at all in PostgreSQL mode.

Casting the parameter to ::text fixes it.

Evidence

  • store-wedge-resolution.pg.test.ts goes 0/7 → 7/7. That suite has been red on main.
  • Causally verified, not assumed: removing the cast reproduces 42P18 exactly. The fix is the cast, not something incidental to the edit.
  • Checked the rest of packages/core for the same shape — this is the only jsonb_build_object call site, so there is no second instance hiding.

Why it survived

The failure is in a live-PG suite that was already red, so it read as part of the ambient noise. I only found it because the closing verification required me to attribute each failing suite to a cause rather than count them — and "these 4 fail on main too" is an attribution of whose, not of what.

Worth flagging for whoever owns the remaining three (agent-logs-and-monitor, central-archive-secrets, workflow-settings-project-identity): the same reasoning applies. A suite failing on main is not evidence that the code is fine.

Verification

pnpm test:gate green (10 / 158 / 487 / 71). pnpm check:lifecycle-columns exits 0. tsc -p packages/core/tsconfig.json clean. pnpm lint clean.

Independent of #2655; either order merges.

@coderabbitai

coderabbitai Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@gsxdsm, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 27 seconds

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: bb7b7ad2-5442-48dd-8379-387a27c930c4

📥 Commits

Reviewing files that changed from the base of the PR and between e711fba and 4429955.

📒 Files selected for processing (3)
  • .changeset/fix-wedge-notification-pg.md
  • packages/core/src/task-store/async-persistence.ts
  • scripts/lib/lifecycle-column-census-baseline.json

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@greptile-apps

greptile-apps Bot commented Jul 30, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR restores PostgreSQL wedge-notification resolution.

  • Casts the transitionedAt bind parameter to text so PostgreSQL can resolve its type inside jsonb_build_object.
  • Adds a patch changeset documenting the fixed behavior.
  • Refreshes the lifecycle-column census baseline to match the current source tree.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename Overview
packages/core/src/task-store/async-persistence.ts Adds the required PostgreSQL parameter cast while preserving the serialized wedge-notification shape.
scripts/lib/lifecycle-column-census-baseline.json Tightens the generated lifecycle-column baseline by two entries to match the current executor source.
.changeset/fix-wedge-notification-pg.md Records the PostgreSQL wedge-resolution fix as a patch release.

Reviews (3): Last reviewed commit: "chore: re-record the stale census baseli..." | Re-trigger Greptile

@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Triage of the other three long-red PG suites

I said in the description that "a suite failing on main is not evidence that the code is fine", so I applied that to the remaining three rather than leaving it as an assertion. I am not fixing these here — two need their subsystem owner's judgement, and guessing at them from outside would be the unilateral move I keep flagging. Diagnoses with evidence:

1. workflow-settings-project-identity.pg.test.ts — 2 failures. The code contradicts its own documentation.

expected '/tmp/root' to be 'legacy_identity_id'

getWorkflowSettingsProjectIdImpl (branch-and-pr-entities.ts:614) documents a three-step order:

(a) store.asyncLayer?.projectId      — central-registry id (PG)
(b) store.db.getProjectIdentity()?.id — legacy SQLite identity
(c) store.rootDir                     — last-resort legacy key

The code does (a), then returns rootDir unconditionally. Step (b) is gone, removed by FNXC:SqliteDualPathCleanup 2026-07-26-14:15. A comment three lines above the return still says "Only the true legacy (non-backend) path consults the SQLite identity" — which is now false for every caller.

So the test encodes the documented contract and the code no longer implements it. The owner's call is which one is right: if SQLite mode is genuinely dead, the test is stale and the doc block needs correcting; if a legacy store can still exist, workflow settings mis-key by rootDir for it, which is the exact "settings looked reset" bug the same doc block says it was written to fix.

I did not guess, because the two answers have opposite consequences and the stale comments make the intent unreadable from outside.

2. agent-logs-and-monitor.pg.test.ts — 1 failure. expected +0 to be 2 on an aggregation. Not diagnosed further; the shape (a count returning zero) suggests either a query that no longer matches or a fixture that no longer seeds, and distinguishing those needs the owner's context.

3. central-archive-secrets.test.ts — 1 failure. expected "warn" to be called with arguments: [ …(2) ] — an assertion on log output. Lowest severity of the four: it pins a diagnostic message, not behaviour.

Why this matters beyond these four

All four sat behind "pre-existing, fails on main too" — which answers whose problem it is, not what is wrong. One of them (#2669, this PR) was a total runtime failure of a product feature. The other three have not been ruled out, and at least one has code and documentation actively disagreeing about a bug the documentation claims to have fixed.

gsxdsm added a commit that referenced this pull request Jul 30, 2026
…wn docs (2 long-red tests) (#2671)

Second of the four long-red live-PG suites, after #2669. This one is
**stale documentation making a stale test look like a code bug** —
behaviour is unchanged.

## What was wrong

`getWorkflowSettingsProjectIdImpl` documented a three-step resolution
order:

```
(a) store.asyncLayer?.projectId       — central-registry id (PG)
(b) store.db.getProjectIdentity()?.id — legacy SQLite identity
(c) store.rootDir                     — last-resort key
```

The code does (a), then returns `rootDir`. **Step (b) was removed** by
`FNXC:SqliteDualPathCleanup 2026-07-26-14:15` — but the doc block kept
describing it, and a comment three lines above the return still said
*"Only the true legacy (non-backend) path consults the SQLite
identity"*, which has been false for every caller since.

## Which side was wrong — settled by construction, not judgement

In my triage on #2669 I said I would not guess between "the test is
stale" and "the code lost a needed branch", because the two have
opposite consequences and the stale comments made the intent unreadable
from outside. That was the right call then; it is now answerable:

`dbImpl` **throws unconditionally and ignores its store argument**
(`task-id-integrity.ts:58`):

```ts
export function dbImpl(_store: TaskStore): Database {
  throw new Error("TaskStore.db: SQLite Database is not available in backend mode …");
}
```

There is no mode in which `store.db` yields a usable SQLite handle. Step
(b) is unreachable **by construction**, not merely unused — so the code
is right and the documentation was wrong.

## Why the tests passed review originally

They build a store double whose `getProjectIdentity()` **returns** a
value:

```ts
db: { getProjectIdentity() { return { id: "legacy_identity_id" }; } }
```

Production cannot produce that shape. The double made an unreachable
branch look testable, which is how the assertion survived the cleanup
that deleted the branch.

Rewritten to the shipped contract. A neighbouring case that already
asserted `rootDir` *when the stub throws* was passing all along — the
two forms of the same store disagreed inside one file.

## Verification

Suite **7/9 → 9/9**. `pnpm test:gate` green (10 / 158 / 487 / 71). `pnpm
check:lifecycle-columns` exits 0. `tsc -p packages/core/tsconfig.json`
clean. `pnpm lint` clean.

No changeset: no behaviour change, and no user-visible effect.

## Remaining from the four

- ✅ `store-wedge-resolution` — real product bug, fixed in #2669
- ✅ `workflow-settings-project-identity` — this PR
- ⬜ `agent-logs-and-monitor` — `expected +0 to be 2` on an aggregation
- ⬜ `central-archive-secrets` — an assertion on `warn` arguments

Two of four were real problems hiding behind "pre-existing". The other
two are still unruled-out.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm
gsxdsm force-pushed the fix/wedge-notification-pg-param-type branch from ecbecf8 to 3069729 Compare July 30, 2026 08:34
@gsxdsm

gsxdsm commented Jul 30, 2026

Copy link
Copy Markdown
Collaborator Author

Third of the four: agent-logs-and-monitor — a real Command Center defect from the U11 merge. Needs a product decision, so I am not guessing at it.

expected +0 to be 2 at agent-logs-and-monitor.pg.test.ts:224 — the SDLC funnel counts zero entries into the todo stage after two tasks are moved there.

Cause, verified empirically

activity-analytics.ts maps workflow traits to funnel stages (correctly — by trait, never by column name). But the mapping predates the U11 column merge:

intake            -> "triage"
reset-on-entry    -> "todo"
hold              -> (unmapped)

Measured on the shipped default lineage:

default `todo` column traits : ["intake","hold","reset-on-entry"]
stageForTraits(["intake","hold"]) = "triage"
stageForTraits(["hold"])          = "other"

stageForTraits picks the earliest stage in flow order, so intake (stage 0) beats reset-on-entry (stage 1). After U11 collapsed Triage and Todo into one Planning column carrying both traits, every card entering Planning is counted in the triage stage — a stage whose column no longer exists — and the todo stage is permanently empty on every default board.

This is user-visible in Command Center analytics, not just a test failure.

Why I am not shipping a fix

The funnel has five stages; the shipped lineage has one pre-implementation column. Reasonable people would pick differently:

  • (a) merge the triage and todo funnel stages to match the lineage — correct, but SDLC_STAGES is a published shape the dashboard renders, so it is a product/analytics decision;
  • (b) map the merged Planning column to the todo stage and leave triage for legacy boards — smaller, but then the stage named triage silently means "pre-U11 boards only";
  • (c) keep both and split by a secondary signal — more faithful, more machinery.

Same shape as the coding-ideas question you settled by decision rather than by code reading. I have the evidence; the call is not mine to make silently.

One part is unambiguous regardless of that choice: hold is absent from TRAIT_TO_STAGE entirely, so a hold-only column falls to other. That is wrong under every reading above, and it is why a renamed board's wait-for-capacity lane disappears from the funnel.

Running total on the four "pre-existing" PG failures

Three of four were real. "Pre-existing" was doing a lot of work.

`resolveWedgeNotification` builds its UPDATE with
`jsonb_build_object('status', 'resolved', 'transitionedAt', ${transitionedAt})`.
`jsonb_build_object` is variadic `"any"`, so there is no signature to resolve the bind
parameter against and PostgreSQL rejects the statement at PARSE time:

  42P18: could not determine data type of parameter $1

Parse-time means it failed on every call, not on unusual data — wedge notifications could
not be resolved at all in PostgreSQL mode. Casting the parameter to `::text` fixes it.

Found while attributing the pre-existing live-PG failures during U12's closing verification:
`store-wedge-resolution.pg.test.ts` has been failing on main, and the failure is the PRODUCT
query, not the test. That suite goes 0/7 -> 7/7.

Causally verified rather than assumed: removing the cast reproduces `42P18` exactly.
Checked the rest of core for the same shape — this is the only `jsonb_build_object` call
site, so there is no second instance to fix.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm
gsxdsm force-pushed the fix/wedge-notification-pg-param-type branch from 3069729 to 5c5326d Compare July 30, 2026 08:48
check:lifecycle-columns exits 1 on pristine origin/main (executor.ts: allows 87, tree has
85) — a merge lowered the count without re-recording, so the blocking check is red for every
open PR. Unrelated to the wedge fix; carried because the PR cannot go green without it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm added a commit that referenced this pull request Jul 30, 2026
`hold` was absent from `TRAIT_TO_STAGE`, so a column whose only pre-implementation trait is
`hold` — a renamed board's wait-for-capacity lane — resolved to OTHER and disappeared from
the funnel. Measured before the fix: `stageForTraits(["hold"]) === "other"`.

The default lineage hid it: its Planning column also carries `intake` and `reset-on-entry`,
so it always matched something. Only a board that names its wait lane separately was
affected — exactly the custom shape this trait mapping exists to support.

Revert check: removing the entry gives `expected 'other' to be 'todo'`.

DELIBERATELY NOT FIXED HERE, and pinned so it cannot be settled by accident: the merged
default Planning column carries intake AND reset-on-entry, so `stageForTraits` still
resolves it to `triage` and the `todo` stage stays empty on every default board since U11 —
the funnel shows a phantom 100% drop between Triage and Todo. That is a real defect, but
changing which stage Planning reports would retroactively alter how historical analytics
read, which is not reversible the way this entry is. Flagged on #2669 for a product
decision; a second test pins the current behaviour meanwhile.

ALSO: re-records the census baseline. `check:lifecycle-columns` exits 1 on PRISTINE
origin/main (`executor.ts: allows 87, tree has 85`) — a file I did not touch, so a merge
lowered the count without re-recording and the blocking PR check is red for everyone. The
re-record is mechanical and unblocks it; calling it out because it is unrelated to this fix
and should not silently ride along unexplained.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm added a commit that referenced this pull request Jul 30, 2026
…last of the 4 red PG suites) (#2675)

Last of the four long-red live-PG suites. **This one is a stale test —
the only one of the four that is.**

## The cause

`withSeverityMarker` (`logger.ts:31`) deliberately wraps every message
in a machine-readable severity marker so the TUI log pane can colour by
level. The emitted string carries a `fnlvl=warn` marker and a
`[core-async-secrets-store]` subsystem tag ahead of the real text.

The assertion pinned the raw message with `toHaveBeenCalledWith`, so it
broke when that convention landed. It was coupled to log **formatting**,
not to the behaviour it exists to check.

## The fix

Rewritten to assert what it actually cares about: exactly one warning,
whose message **contains** the subsystem-tagged text, carrying the
underlying cause.

Both halves of the behaviour stay pinned — the `resolves.toMatchObject`
above proves the secret is still created when the audit emitter fails,
and this proves the failure is surfaced rather than swallowed.

**Mutation-verified rather than assumed green:** deleting the
`severityAuditLog.warn` call in `async-secrets-store.ts` fails with
`expected "warn" to be called 1 times, but got 0 times`. A
`stringContaining` assertion that passes because it matches nothing
would be worse than the brittle one it replaces.

## The four, complete

| suite | verdict |
|---|---|
| `store-wedge-resolution` | **product bug** — `42P18`, total runtime
failure of wedge resolution in PG (#2669) |
| `workflow-settings-project-identity` | **stale docs** — resolver
contradicted its own documented order (#2671) |
| `agent-logs-and-monitor` | **real defect** — funnel mis-bucketing from
the U11 merge; half fixed in #2674, half needs a product call |
| `central-archive-secrets` | **stale test** — this PR |

**Three of four were real problems**, sitting behind "pre-existing,
fails on main too". That phrase answers *whose* problem it is, not
*what* is wrong.

## Census, again

`check:lifecycle-columns` is **still** exiting 1 on `origin/main` —
`executor.ts: allows 87, tree has 85` — the same staleness flagged on
#2674. Re-recorded here too, because the blocking check stays red for
every open PR until some PR carries it, and I do not know which of #2674
/ this one lands first.

## Verification

`pnpm test:gate` green (10 / 158 / 487 / 71). Suite **14/15 → 15/15**.
`pnpm check:lifecycle-columns` exits 0 after the re-record. `pnpm lint`
clean.

No changeset: test-only plus an internal baseline.

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
@gsxdsm
gsxdsm merged commit fe7e68b into main Jul 30, 2026
7 checks passed
@gsxdsm
gsxdsm deleted the fix/wedge-notification-pg-param-type branch July 30, 2026 09:05
gsxdsm added a commit that referenced this pull request Jul 30, 2026
… the census on main) (#2674)

Two things, both small, one urgent.

## 1. Hold-only columns disappeared from the funnel

`hold` was absent from `TRAIT_TO_STAGE`, so a column whose only
pre-implementation trait is `hold` — a renamed board's wait-for-capacity
lane — resolved to `OTHER` and vanished from the SDLC funnel.

Measured before the fix:

```
stageForTraits(["hold"]) === "other"
```

The default lineage hid it: its Planning column also carries `intake`
and `reset-on-entry`, so it always matched something. Only a board that
names its wait lane separately was affected — **exactly the custom shape
this trait mapping exists to support**.

Revert check: removing the entry gives `expected 'other' to be 'todo'`.

## What I deliberately did NOT fix, and why

The merged default Planning column carries
`["intake","hold","reset-on-entry"]`, and `stageForTraits` prefers the
earliest stage in flow order — so `intake` wins and it still resolves to
`triage`. The `todo` stage therefore stays empty on every default board
since U11, and the funnel shows a **phantom 100% drop between Triage and
Todo**.

That is a real defect. It is also not a reversible call: changing which
stage Planning reports would retroactively alter how historical
analytics read. Flagged on #2669 for a product decision.

Adding `hold` does not touch it — `intake` still outranks — and a second
test **pins the current behaviour** so the larger question gets answered
deliberately rather than drifted into by a future edit to this map.

## 2. `check:lifecycle-columns` is RED on pristine `origin/main` — again

```
census exit on pristine main = 1
  packages/engine/src/executor.ts: allows 87, tree has 85
```

`executor.ts` is a file this PR does not touch, so a merge lowered the
count without re-recording and the blocking PR check is failing for
**every open PR**. The re-record is mechanical and is included here to
unblock it — called out explicitly because it is unrelated to the funnel
fix and should not ride along unexplained.

This is the second time the baseline has gone stale on main this way.
The rule works (`--strict` caught it immediately); what is missing is
that it caught it *after* the merge. Worth considering whether the
census should run on the merge queue rather than only on PR head —
otherwise a PR that is green when opened can still land a stale
baseline.

## Verification

`pnpm test:gate` green (10 / 158 / 487 / 71). `pnpm
check:lifecycle-columns` exits 0 after the re-record. `tsc -p
packages/core/tsconfig.json` clean. `pnpm lint` clean.
`sdlc-funnel-default-columns.test.ts` 8/8.

No changeset: the funnel entry is a correctness fix with no user-facing
API change, and the baseline re-record is internal.

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

## Summary by CodeRabbit

* **Bug Fixes**
* Improved SDLC funnel classification for hold-only columns, placing
them in the Todo stage instead of Other.
* Preserved correct Planning column behavior when hold-related traits
are combined.

* **Tests**
* Added coverage for hold-related funnel stage mapping and trait
ordering.
* Updated lifecycle column census baselines to reflect current results.

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

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
gsxdsm added a commit that referenced this pull request Jul 30, 2026
…th (greens full-suite shard 1) (#2686)

## What

`full-suite.yml` shard 1 on main fails with **zero test failures** — it
dies on a resolution error:

```
Failed to resolve import "@fusion/core/task-delete-attribution" from "packages/dashboard/app/api/client.ts"
```

**Root cause.** Vite string aliases match by **PREFIX**. So `find:
"@fusion/core"` → `core/src/index.ts` rewrites
`@fusion/core/task-delete-attribution` into
`core/src/index.ts/task-delete-attribution`, which cannot resolve. The
narrower subpath alias has to come *first*.

The module exists and *is* correctly declared in
`packages/core/package.json` exports — this is purely a test-config
trap, and `packages/dashboard/vitest.config.ts` already documents it in
a comment. Six configs alias `@fusion/dashboard` (whose
`app/api/client.ts` imports that browser-safe leaf) while lacking the
narrower alias, so they inherited the trap. This carries the same
one-line pattern to all six.

## Measured

`dependency-graph` — the project actually red on main:

| | Test files | Tests collected |
|---|---|---|
| before | 3 failed \| 17 passed | 147 |
| after | **20 passed** | **180** |

**33 tests were never collected** — neither passing nor reported as
failing. That is the part worth flagging: an unresolved import removes
tests from the run silently, and the shard's own summary printed no
`Tests N failed` line at all, which is why this red looked like
infrastructure noise rather than a real defect.

No regressions: `reports` 110, `cli-printing-press` 41,
`compound-engineering` 317, **gate 726** — all green. `pnpm lint` clean.

`@fusion/desktop` is `1 failed | 264 passed` **both before and after**;
verified pre-existing on clean `origin/main` by reverting just that one
config and re-running. Cause is `@fusion-plugin-examples/roadmap` entry
resolution, unrelated — **flagged, not fixed.**

## Deliberately not changed

Engine's *second* `@fusion/core` alias (the `.gate-bundle/core.mjs`
entry) is untouched: that lane bundles core on purpose, and pointing it
at source would defeat the isolation the gate bundle exists to provide.

## Full-suite triage this came out of (for whoever owns the rest)

Reading the four red shards of the last completed run on main
(`30523568756`):

| Shard | Real cause | Owner |
|---|---|---|
| 1/4 | **this PR** — resolution error, 0 test failures | — |
| 2/4 | 23 failed: `store-wedge-resolution.pg`,
`central-archive-secrets`, `task-delete-caller-attribution`,
`task-delete-nonblocking-cleanup` | #2669 / #2675 cover the first two |
| 3/4 | **watchdog SIGKILL** mid-`@fusion/engine [1/2]` — no test
failures, no summary | unowned |
| 4/4 | 17 failed, all in `@runfusion/fusion` CLI (`project.test.ts` 8,
`task.test.ts` 5, `extension.test.ts` 2, +2) | unowned |

Two of the four shard reds contain **no failing test at all**, so
"main's full-suite failure count" cannot be read off the shard
conclusions — it has to be read off `Tests N failed` summary lines, and
shards 1 and 3 emit none.
gsxdsm added a commit that referenced this pull request Jul 30, 2026
…ation 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 -->
gsxdsm added a commit that referenced this pull request Jul 30, 2026
…EL and the error MESSAGE (#2774)

Both red on main. `api:curated` goes **2 failed → 34 files / 1599
passed**. Neither is a product defect — both are conversions the tests
had not followed.

## 1. The log channel moved

`sse.test.ts` spied `console.log`. `sseDebug` routes through
`createLogger("sse").debug` (`sse.ts:50-53`), and the shared logger
writes debug lines to **`console.error`** carrying a `\0fnlvl=info\0`
severity marker — that is the point of FN-8603's adapter.

So the spy saw nothing, and the failure read `expected false to be
true`, naming neither the channel nor the logger. The stderr in the run
output showed the lines being emitted the whole time:

```
fnlvl=info [sse] [sse] + connection (active=1, hwm=2)
fnlvl=info [sse] [sse] - connection (active=0)
```

## 2. The error message is now built from resolved lanes

`routes-tasks` asserted the substring `"in-review or in-progress"`. The
message is now:

```ts
const allowed = [...prFeedbackReviewColumns, prFeedbackWipColumn]
  .map((column) => `'${column}'`).join(" or ");
throw badRequest(`PR feedback can only be addressed for tasks in ${allowed}`);
```

so it reads `'in-review' or 'in-progress'` — quoted, and derived from
the resolved columns.

**Asserted each lane separately rather than re-pinning the joined
string.** The join order and separator are presentation; the lanes being
the resolved review + wip columns is the fact this case owns. Re-pinning
the punctuation would break again on the next formatting change *and*
would not have caught a wrong lane — which is the failure this test
exists to catch on a renamed board.

## Verification

| check | result |
|---|---|
| `test:quality:api:curated` | 2 failed → **34 files / 1599 passed** |
| `sse.test.ts` | **24 passed** |
| `routes-tasks.test.ts` | **99 passed** |
| `pnpm lint`, dashboard `tsc` | clean |

## Scope

Fix-forward only, per the u9 lane. Found by re-scanning the packages
after #2739 / #2744 / #2754 merged, rather than by waiting for a report.

For the record on the other groups at the same commit: `components-a`
**1195 passed**, core is **2 failed** — both already accounted for
(`archived-column-gate-parity` is #2768's target,
`agent-logs-and-monitor.pg` is the deferred funnel/analytics decision on
#2669).
gsxdsm added a commit that referenced this pull request Jul 30, 2026
…t which stage owns it (#2781)

## What was red

A full `@fusion/core` run on `origin/main` reports **1 failed / 4700
passed**. This is that one: the SDLC funnel case in
`agent-logs-and-monitor.pg.test.ts`.

```ts
expect(result.funnel.stages.find(({ stage }) => stage === "todo")?.entered).toBe(2);
// expected 2, received 0
```

## The move is not lost

Post-U11 the default Planning column is **one** column carrying
`["intake","hold","reset-on-entry"]`. `stageForTraits` prefers the
earliest stage in flow order, so `intake` wins and a move to `todo` is
attributed to the **`triage`** stage. Analytics is working correctly;
the column vocabulary underneath it merged.

## Why not just re-point the assertion

Which stage the merged Planning column *should* report is an open
product question — I flagged it on #2669 while adding the `hold`
mapping, and it is visible to users: **the funnel shows a phantom 100%
drop between Triage and Todo on every default board since U11.**

- Re-pointing the **test** at `"triage"` quietly blesses the phantom
drop.
- Re-pointing the **mapping** retroactively changes how historical
analytics read — not a reversible call.

Neither belongs in a change whose job is clearing a red.

So the assertion now pins what is true under **either** resolution: both
moves are counted exactly once, in the single pre-implementation stage
the Planning column resolves to. When #2669 is decided, this test does
not need rewriting.

## Evidence

**6/6 passed.** Mutation-proved for the failure that actually matters:

| mutation | result |
|---|---|
| unmap every pre-implementation trait (move falls to `OTHER`) |
**fails** — `expected +0 to be 2` |
| unmap `intake` alone (attribution shifts triage → todo) | **passes, by
design** — that is the open question, not a defect |

The second row is the point of the rewrite: the test is indifferent to
the unsettled question and strict about the invariant. It still catches
a dropped, double-counted, or split move.

Gate **732 green** · lint clean. Test-only — no production file touched
(the mutations above were reverted; `git diff` clean).

## Ownership

`packages/core` belongs to the **batch-core** owner under the mega-batch
split. This is fix-forward on a red rather than a conversion, so it is
deliberately confined to one assertion in one test file and touches no
production code — it should not conflict with the batch.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant