Follow-up: deferred review findings from PR #690

[philcunliffe]

Deferred review findings from PR #690 (fix/issue-685, head 3bb9a5240c5443369e85048e367f26dace3c607d). PR #690 exhausted its review rounds with these still open; triage classified both as non-blocking for that PR, so they are recorded here for follow-up. Full evidence is in the two review comments on #690.

1. Pre-existing defect: scanCommittedMessageIds violates its own "NEVER throws" contract, and a rejected seed promise is memoized forever (severity: high; pre-existing on master, NOT introduced by #690)

Where:

• hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js:488, function scanCommittedMessageIds: the for (const part of partitions ?? []) loop sits OUTSIDE the try/catch, which only guards the await storage.discoverCachePartitions(...) call. A storage whose discoverCachePartitions resolves to a truthy non-iterable (contract violation) makes the loop throw, so the async function rejects.
• hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js:462: the JSDoc for that function claims it "NEVER throws". That claim is false for the input above.
• hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js:285-290, seedSeenMessagesForSession: the seed promise is memoized in seedPromises synchronously and never removed on rejection, so a rejected seed is cached per session for the listener's lifetime.

Mechanism and observable symptom: projectExchange awaits the seed at message_projector.js:233. Under storage that violates the discover contract on EVERY call, every projectExchange rejects, source.js (around line 228) catches and logs it, and the row is dropped. Because seedPromises memoizes the rejected promise, later exchanges for the same session short-circuit on the poisoned memo and emit no warn at all: silent, total, per-session-permanent loss of ai_gateway_messages capture. Review round 2 on #690 measured five exchanges producing two warn lines and zero rows.

Why deferred rather than blocking #690: the defect exists on master; #690 touches none of these lines, and reverting #690 would not fix it. In the trigger scenario, pre-#690 master is strictly worse (the first committed-session-index scan rejection is an unhandled rejection that kills the daemon, losing the proxy and every dataset), while post-#690 the daemon survives and emits error_kind: 'scan_rejected' as the operator signal. #690 does not introduce or worsen this; it un-masks it.

Suggested fix shape (from review round 2 on #690): make scanCommittedMessageIds honour its documented contract, e.g. guard the loop so a malformed partitions degrades to "seeded nothing" (which IS the documented best-effort behaviour for this function, unlike the session-index scan where an empty result would be a wrong authoritative answer); plus a decision about not memoizing a rejected seed promise in seedPromises (or normalizing the seed so it cannot reject). Needs its own regression test driven by storage that is malformed on every call, asserting rows still emit and no poisoned memo persists.

2. MERGE-SEQUENCING NOTE: hard conflict with PR #689 (not a defect in either PR)

git merge-tree --write-tree 3bb9a52 da2cd4f conflicts on both hypaware-core/plugins-workspace/ai-gateway/src/message_projector.js and test/plugins/ai-gateway-message-projector.test.js. Whichever of #689 / #690 merges second hits a normal content conflict that the merge rung or a human resolves; git will not silently auto-merge.

The correct composed resolution keeps both changes, never either rebuild() wholesale:

• Keep Committed-session index survives a throwing scan instead of wedging it #690's .catch on the scan promise (normalizing a rejection to undefined, with the error_kind: 'scan_rejected' warn).
• Then layer Committed-session index stamps atMs at scan start, so a slow scan never serves a cache hit #689's completion-stamping chain (attempt.ids = scan.then(...) stamping atMs at completion) on top of the caught promise.

Taking #689's rebuild() wholesale would be dangerous: its chain has no rejection handler and a fulfillment-only guard, so on a rejecting scan built would never clear and every later mightHaveCommittedRows would await the same rejected promise: permanent, silent, total loss of ai_gateway_messages capture. Mitigating: each PR's regression test fails a lossy resolution in the opposite direction (#690's test fails if the .catch is dropped; #689's test fails if completion-stamping is dropped), so a bad resolution cannot pass the suite. Review round 1 on #690 includes the correct composed form; it additionally needs the error_kind field added in 3bb9a52.

3. Nits carried forward from review (no change requested; style only)

• message_projector.js:359/373: the inner .then((ids) => ...) parameter shadows the outer const ids.
• If log.warn itself threw inside the new .catch, the derived promise could reject again. Purely theoretical (fields are plain strings); the identical exposure predates Committed-session index survives a throwing scan instead of wedging it #690 at the discover_failed site.

Backlink: #690