fix(changes): close the change feed's silent-failure paths

[cuibonobo]

Six bugs found reviewing the change-feed work from #81–#84. Five are in the resume layer #84 added, one is in discovery, and all of them share a failure mode: the server stops telling a client the truth, and the client is never told that it stopped.

Bounding the resume buffers — the other half of that review — is deliberately not here. It needs a config surface and a decision about where the ceiling goes, and it shouldn't hold up fixes that don't. It follows in #95.

What's fixed

1. Two different filters could share one resume buffer. resumeBufferKey() built its key with a positional JSON.stringify over an array, where an undefined element renders as null. That collapsed "no parentId filter" onto parentId: null — root records only, a filter the wire format defines.

A buffer opens exactly one ScopedStack.subscribe(), carrying the filter of whichever connection minted it, so whichever connected first decided what the other received:

first connection	second connection	what the second one got
?parentId=null	GET /changes	never hears about child records — no reset, no error
GET /changes	?parentId=null	receives changes outside its filter

Both violate the contract. The silent one is what wire-format.md § Change feed calls the behavior that makes a feed untrustworthy: a client that can't tell it missed something can't repair it either. Keying on an object fixes it directly — JSON.stringify drops an undefined property but not an undefined array element, so absent is now encoded as an absent key, which no present value can imitate.

2. A failed subscription permanently poisoned a buffer key. acquire() registered the buffer before awaiting subscribeToStack(), so a rejection left a buffer in the map that nothing was feeding and nothing would retry. Every later connection on that key got ready and then permanent silence. The buffer now lands in the map only once its subscription resolves, with in-flight attempts tracked separately so two connections racing on one key still open one subscription and share however it settles.

3. A throw in the stream body skipped every cleanup. The streamSSE callback had no try/finally and passed no onError, so a throw — the backlog replay awaits ScopedStack.get() per entry against the adapter — skipped both clearIntervals, unsubscribe() and unregister(). That leaked a live core subscription, a listener on the buffer, and a refcount that never returned to zero, so the buffer was never evicted. Hono catches the throw itself, so nothing surfaced beyond a bare console.error.

Cleanup now runs in a finally, and the throw is caught rather than left to hono — which answers one by writing the raw error message to the client as an error frame, on a stream any anonymous caller can open. It's logged with its request id like every other failure.

4. The session re-check could take the process down. lookupToken() had no rejection handler, so an unreachable token store raised an unhandled rejection instead of closing the stream. It now closes, which is the answer the client already handles via a 401 on reconnect.

5. Discovery could contradict the route it describes. WellknownRouteOptions carried changeFeedResume and changeFeedRecords, whose only effect was to print a different literal in the discovery response. Neither was a deployer lever, and createApp() passed neither — production was consistent because two defaults happened to line up, not by construction.

changeFeedRecords was the worse of the two: there is no route-side counterpart at all. parseIncludeRecord() honors ?include=record unconditionally, so that flag could only ever advertise something this server doesn't do. Not cosmetic — a client is entitled to act on discovery without asking again, since APIAdapter.subscribeChanges() against a server advertising no feed throws locally without sending a request.

What the flags bought was one conformance fixture, discovery-advertises-a-feed-that-neither-resumes-nor-includes-records, which describes a conformant server with neither capability. This server isn't that server, and satisfying a fixture about a different implementation isn't worth an option that lets this one misreport itself. Both fields are now literals, and the fixture is skipped with that reason — using the SKIPPED set assertCoverage() already takes and the createRecord block already uses.

ChangeRouteOptions.resume stays. Unlike those two it drives real spec-defined behavior — ready with no seq, then reset with reason not_supported — that a behavioral fixture exercises and that needs a way to be reached.

6. Smaller consistency fixes. The owner-token comparison here was a plain === where authMiddleware deliberately uses timingSafeEqual — safeCompare is now exported and reused, so the same secret is compared the same way in both places. Keepalives went out as raw writes outside FrameGate, and the backlog replay kept running permission checks after the gate had tripped; both now go through the gate, so "in-flight frames are bounded" holds for the whole stream rather than for record frames alone.

Verification

Each fix has a regression test, and each was confirmed to fail against the code before it — the two end-to-end filter tests and the keying test fail on the old resumeBufferKey, and both poisoning tests fail on the old acquire().

pnpm typecheck, pnpm lint, pnpm format:check and pnpm test all pass. 416 tests, from 409 on main — net of two deleted with the discovery flags.

Review note

Most of src/routes/changes.ts's diff is re-indentation from wrapping the body in try. git diff -w shows the real change.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FH88yDv3Fk2RmLsU7DU4Rz