Pin the committed-session-index failed-build fallback with a regression test

[philcunliffe]

What was uncovered

#683's round-1 review found that a failed committed-session-index build was cached as "no committed rows" for the full SESSION_INDEX_REBUILD_MS window, un-seeding every session whose first exchange landed in it. The fix makes scanCommittedSessionIds return undefined on a discovery failure (message_projector.js:408), distinct from the empty Set a genuine empty scan returns, so mightHaveCommittedRows errs toward the per-session seed scan and the failed build clears itself out of built rather than standing in as authoritative.

That fix had zero coverage. The existing restart replay: a throwing storage degrades to not-seeded and never drops rows test passes either way, because it only asserts rows are not dropped, never that the index retries rather than trusting a failed build.

The new test

test/plugins/ai-gateway-message-projector.test.js -> committed-session index: a build that could not scan is not cached as "no committed rows"

A storage whose discoverCachePartitions throws on call 1 (the index build) and succeeds afterwards, holding one committed row for sess-committed. One listener, two projections:

1. sess-fresh (no committed rows): asserts the row is still emitted, and that discoverCachePartitions was called twice, i.e. the failed build fell through to the per-session scan instead of skipping it.
2. sess-committed (already committed), same rebuild window: asserts the index is rebuilt (call 3) and the session scans its own committed rows (call 4), so the committed row is seeded and deduped rather than re-emitted as a duplicate.

No source change: this is a coverage-only PR.

Evidence that it genuinely exercises the fallback

Broke the fallback in place by reverting return undefined to return ids in scanCommittedSessionIds, exactly the defect round 1 found:

not ok 36 - committed-session index: a build that could not scan is not cached as "no committed rows"
error: |-
a failed index build must err toward the per-session scan (build + scan), not skip it
1 !== 2
expected: 2
actual: 1
# pass 35
# fail 1

The second, behavioural half was confirmed separately (same broken source, with the call-count assert neutralised in a scratch copy so execution reaches it):

not ok 36 - committed-session index: a build that could not scan is not cached as "no committed rows"
error: |-
the committed row is seeded and deduped, not re-emitted as a duplicate
1 !== 0
expected: 0
actual: 1

So the broken code re-emits a duplicate part_id row for the committed session, which is the defect itself, not just a call-count difference.

Restoring return undefined:

ok 36 - committed-session index: a build that could not scan is not cached as "no committed rows"
# pass 36
# fail 0

Verification

• npm test: 3858 pass / 0 fail / 6 skipped, exit 0.
• npm run typecheck: clean, exit 0.
• Style: no semicolons, no em dashes in any added line.

Fixes#684

[philcunliffe]

Neutral review round: PR #688 (issue #684)

Verdict: APPROVE. No actionable findings. Coverage-only, one test added, no source touched. The test is a real gate, deterministic, and consistent with LLP 0204.

What I checked

Full diff (gh pr diff 688), the subsystem under test (hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js, createCommittedSessionIndex / scanCommittedSessionIds / seedSessionIfCommitted), CLAUDE.md conventions, and LLP 0204 (llp/0204-gateway-daemon-memory-leak.issue.md, the "Seed scan" bullet in ## Fix).

The gate the issue demanded (mutation-verified, both ways round)

Issue #684 required the test to fail against return ids and pass against return undefined. I ran it, I did not take it on faith.

Mutation	Result
baseline (unmutated HEAD)	36 pass / 0 fail
M1message_projector.js:408 catch: return undefined -> return ids	1 fail - only not ok 36 committed-session index: a build that could not scan...
M2 delete the self-clearing attempt.ids.then(...) block in rebuild() (message_projector.js:344-346)	1 fail - only not ok 36

Both halves of the fallback are pinned, and in each case the new test is the sole survivor-killer: no pre-existing test moved. That is exactly the gap #684 described ("reverting the fix left the suite green").

Determinism

No flake surface. Specifically:

• No wall clock.SESSION_INDEX_REBUILD_MS is never reached or consulted on either path. Call 1 returns early at if (ids === undefined) return true; call 2 finds built === undefined (cleared) and rebuilds unconditionally. The test never depends on elapsed time, unlike the neighbouring window tests that inject now.
• Microtask ordering is guaranteed, not lucky.rebuild() registers the self-clearing .then on attempt.ids synchronously, beforemightHaveCommittedRows awaits the same promise. Same promise, FIFO continuations, so the clear always lands before the awaiting caller resumes. No sleep, no setTimeout, no ordering race.
• Exchanges are sequential and awaited; the discoverCalls counter is only ever incremented on a single-threaded synchronous path.

Does it assert the right behaviour, or merely pass?

The right behaviour. It is not a vacuous "did not throw" assertion (which is precisely what the existing test at test/plugins/ai-gateway-message-projector.test.js:987 does, and why it missed the defect). It asserts the two user-visible consequences:

• test/plugins/ai-gateway-message-projector.test.js:1044-1049 - a failed build falls through to the per-session scan (discoverCalls === 2), so a fresh session still emits.
• test/plugins/ai-gateway-message-projector.test.js:1054-1058 - the next session rebuilds rather than trusting the failed build (discoverCalls === 4), so its already-committed row is seeded and deduped (second.length === 0) instead of re-emitted as a restart-replay duplicate. That duplicate is the actual defect The gateway daemon leaks until GC thrash: recorder retention, unbounded dedupe and seed scans (LLP 0204) #683's review found; the test asserts the duplicate's absence, not just a call count.

Design-doc consistency

Nothing locked in that the docs contradict. LLP 0204 ## Fix specifies the shared session-id index and the 10-minute rebuild window; it is silent on failed builds, and the undefined sentinel is a review-driven refinement that preserves the doc's stated envelope ("a stale miss only risks the duplicate seeding guards against"). Erring toward the scan on an unscannable build is strictly the conservative direction. LLP 0204 is Status: Draft, so no immutability concern either way. No @ref was added; correct - a test that needs a paragraph of prose to explain its own mechanics has that prose inline, and the rule is not to annotate mechanically.

Conventions

Clean. No semicolons in the added block, no U+2014 anywhere in the diff (grepped), no TypeScript, no inline import() types, no @typedef. The stub reuses the file's established /** @type {ExtendedQueryStorageService} */ (/** @type {unknown} */ ({...})) double-cast idiom, and ExtendedQueryStorageService is already imported via @import at line 18 with a repo-root-anchored .js specifier. The insertion sits above the committedStorage helper's JSDoc without detaching it.

Nits (recorded, NOT acted on)

• The exact-equality call counts (=== 2, === 4) will also trip on a benign future refactor, e.g. one that reuses a successful build's partition list for the seed scan instead of re-discovering. That is the brittleness inherent in the approach issue Committed-session-index failed-build fallback has no regression coverage (deferred from #683) #684 explicitly prescribed ("count discoverCachePartitions calls"), the assertion messages spell out what each count means, and a looser assertion would weaken the M2 kill. Leave it.

Checks run

• npm test: 3858 pass / 0 fail / 6 skipped (3862 subtests)
• npm run typecheck: clean, no output
• PR state: MERGEABLE / CLEAN, single commit on top of master

Nothing was changed or pushed. Reviewed in a detached worktree at 6127364f6e3d4768b8ae0b5d06127fdae0ca9758; the main checkout was not touched.