Committed-session index stamps atMs at scan start, so a slow scan never serves a cache hit

[philcunliffe]

What was wrong

hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js stamped the committed-session index's freshness timestamp when its session_id scan started:

const attempt = { atMs: now(), ids: scanCommittedSessionIds(storage, log) }

while the freshness test in mightHaveCommittedRows reads now() - current.atMs < SESSION_INDEX_REBUILD_MS.

Root cause

The rebuild window was aging the attempt, not the answer. Any full session_id scan that itself takes longer than SESSION_INDEX_REBUILD_MS (10 min) resolves already stale, so the very next miss rebuilds at once: the index rebuilds back-to-back indefinitely and never serves a single cache hit, on exactly the table size that makes the index worth having. As the deferred finding from #683's round-2 review notes, this does not stampede (one chained rebuild at a time) and is still better than the pre-#683 one-scan-per-session, so it is a degradation of the optimisation, not a regression against master.

The fix

atMs starts at Infinity (never stale) and is stamped when the scan completes. The entry is still published to built synchronously, so concurrent callers keep sharing the in-flight scan and the round-1 stampede fix is untouched; only the timestamp is deferred. The stamp is chained onto the scan promise (rather than a floating .then) so it is structurally ordered before every awaiter, not dependent on microtask registration order. The self-clearing failed-build guard moved into that same handler unchanged.

Also updated: the SESSION_INDEX_REBUILD_MS JSDoc and the matching bullet in LLP 0204 (Status: Draft) now say the window is measured from scan completion.

Regression test

test/plugins/ai-gateway-message-projector.test.js - "committed-session index: a scan slower than the rebuild window still serves cache hits". It gates the stub's discoverCachePartitions on a promise, advances the injected clock past SESSION_INDEX_REBUILD_MS while the scan is in flight, releases it, and then asserts a later miss one millisecond after completion is served from the index.

FAIL before the fix (fixed test, source reverted to origin/master):

# Subtest: committed-session index: a scan slower than the rebuild window still serves cache hits
not ok 32 - committed-session index: a scan slower than the rebuild window still serves cache hits
  name: 'AssertionError'
  expected: 1
  actual: 2
  operator: 'strictEqual'
# tests 36
# pass 35
# fail 1

PASS after the fix, with the existing single-rebuild-under-concurrency and rebuild-after-window tests still green:

# tests 36
# pass 36
# fail 0

Verification

• npm test: # tests 3864 / # pass 3858 / # fail 0 / # skipped 6
• npm run typecheck: clean, exit 0
• No em dashes, no semicolons, JSDoc-only types.

🤖 Generated with Claude Code

Fixes #686

[philcunliffe]

Neutral review round: PR #689 @ da2cd4f7bfc97376ec1e3e5ec096641ac849d27d

Verdict: APPROVE. No actionable findings. Reviewed in a detached worktree off origin/fix/issue-686; the main checkout was never touched. Nothing was pushed, because nothing needed fixing.

Scope reviewed

3 files, 1 commit (code + doc + test landed together, as CLAUDE.md requires):
hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js (+34/-10), llp/0204-gateway-daemon-memory-leak.issue.md (+4/-2), test/plugins/ai-gateway-message-projector.test.js (+45).

Concurrency analysis (the risky part)

The fix rests on one invariant: no reader can ever observe atMs === Infinity. I verified this both statically and empirically.

Statically. atMs has exactly one reader in the whole repo, message_projector.js:388:

const current = built ?? rebuild()
const ids = await current.ids              // :384
...
if (now() - current.atMs < SESSION_INDEX_REBUILD_MS) return false   // :388

The read at :388 is unconditionally preceded by await current.ids at :384. current.ids is the chained promise (message_projector.js:357, attempt.ids = scan.then(...)), whose handler assigns attempt.atMs = now() at :358 before returning. Promise resolution is causally ordered after the handler returns, so the stamp is guaranteed visible to every awaiter. This is genuine structural ordering, not .then registration order, exactly as the PR body claims. (hypaware-core/plugins-workspace/claude/src/transcripts.js:134 has an unrelated field of the same name; not affected.)

The Infinity window is also closed on the publish side: built = attempt (:364) is executed after attempt.ids is reassigned (:357), with no intervening await, so no caller can ever pick up the raw, unchained scan from built. The one-tick sliver where attempt.ids === scan (between :353 and :357) is unreachable because attempt is not yet published anywhere.

Empirically. I inserted a probe immediately before :388:

if (current.atMs === Infinity) throw new Error("PROBE: reader observed atMs === Infinity")

and ran the full projector file: 36/36 pass, probe never fired across the slow-scan test, the N=6 concurrent-miss test, and the concurrent restart-replay test. Probe reverted; worktree confirmed clean (git status --porcelain empty).

Interaction with the round-1 stampede fix: preserved. mightHaveCommittedRows runs synchronously to its first await (the canScanCommittedRows guard is not an await), so built ?? rebuild() is atomic with respect to other callers, and rebuild() assigns built before yielding. On the post-window path, each caller re-tests built === current synchronously on resume, so the first resumer installs attempt2 and every later resumer takes next = built and shares it. Unchanged by this diff. Confirmed by the pre-existing N=6 test still green.

Interaction with the self-clearing failed-build guard: equivalent, and slightly safer. The guard moved inside the chained handler and now runs structurally before any awaiter resumes, where previously it depended on the guard's .then having been registered on scan before the awaiter's. Same effective ordering, now enforced rather than incidental. Two side benefits: the handler return ids preserves the value for awaiters, and the old floating .then (an unhandled-rejection hazard if scan ever rejected) is gone.

Failure-path bias. On the failure branch the handler stamps atMs and then clears built; the caller reads ids === undefined and returns true (err toward scanning) without reaching :388, so the stamp is inert. And in the hypothetical where a future non-awaiting reader did see Infinity, it reads as "fresh", biasing toward trusting a miss, which lands inside the failure envelope LLP 0204 already documents for a stale miss ("only risks the duplicate seeding guards against, which settlement/compaction still collapse"). Acceptable, and the code comment states the invariant.

Test quality

test/plugins/ai-gateway-message-projector.test.js:894 is deterministic and non-vacuous, not timing-dependent:

• Injected clock (now = () => clockMs), no wall clock.
• Explicit promise gate (scanGate / releaseScan) holds discoverCachePartitions open across the window; no setTimeout races. The single setImmediate fully drains the microtask queue, and the whole projector-to-scan chain is microtasks, so the build has reliably started before the clock advances.
• Non-vacuity verified: with message_projector.js reverted to origin/master and the test file kept, exactly this one test fails, expected: 1 / actual: 2 at the discoverCalls assertion, matching the PR body. At HEAD, 36/36 pass.
• Not flaky: 40 consecutive runs of the file, 0 failures.

Docs and conventions

• llp/0204-gateway-daemon-memory-leak.issue.md is Status: Draft (line 4), so it is editable under the CLAUDE.md immutability rule. Confirmed, not an Accepted/Active doc.
• The doc edit (lines 79-81) accurately describes the new behaviour, and the SESSION_INDEX_REBUILD_MS JSDoc (:317-326) matches. The @ref LLP 0204#fix at message_projector.js:181 still resolves (## Fix at 0204:59) and still applies. The nearby comment at :179 ("one scan per SESSION_INDEX_REBUILD_MS window") remains accurate, and is in fact more true after this change than before it.
• No semicolons on any added line. No em dashes (U+2014) anywhere in the diff; runtime/comment prose uses -. No inline import() types, no @typedef. JSDoc types only.

Verification run (in the review worktree, at da2cd4f7)

• npm test: # tests 3864 / # pass 3858 / # fail 0 / # skipped 6
• npm run typecheck: clean, exit 0
• node --test test/plugins/ai-gateway-message-projector.test.js: 36/36, x40 runs, 0 failures

Both match the numbers in the PR description.

Nits (not acted on, no change requested)

1. llp/0204:81 is a 39-character line mid-paragraph, and the SESSION_INDEX_REBUILD_MS JSDoc has similar ragged wrapping after the insert. Pure prose reflow, no meaning change.
2. message_projector.js:353 sets ids: scan and then overwrites it at :357. This is necessary (the handler closes over attempt, so attempt must exist first), just slightly redundant-looking.