fix(libs): janitor live-run guard, Container dispose-on-replace, vector default parity, StreamPump cancel#604
Open
sroussey wants to merge 4 commits into
Open
Conversation
…ht cache-row deletion CacheJanitor.sweepStaleRunPrivate previously called RunPrivateTaskOutputRepository.clearOlderThan unconditionally, which deleted every row older than the cutoff — including rows belonging to long-running in-flight runs (createdAt is the row write time, not run liveness). Add a liveRunIds: () => Iterable<string> snapshot accessor on CacheJanitor. Default to () => [] with a one-time console.warn since defaulting is unsafe when any run is ever concurrent with the sweep. Plumb the snapshot Set through to clearOlderThan, which now takes excludeRunIds and, when present, enumerates distinct old runIds and calls deleteRunOlderThan per runId not in the exclude set (ITabularStorage has no NOT IN operator).
…ment Container.register and Container.registerInstance overwrote a cached singleton without invoking its disposer protocol. A DB connection, file handle, or event subscriber registered then re-registered would leak the original. Extract the disposer chain into invokeDisposer (the protocol probe) and a disposeService eviction wrapper that catches errors with console.warn so a buggy disposer cannot block re-registration. dispose() retains its own try/catch + AggregateError collection by calling invokeDisposer directly. Inherited (parent-owned) instances are skipped — child cannot dispose an instance the parent still hands out.
…ld to 0 (match SQL backends) InMemoryVectorStorage and IndexedDbVectorStorage defaulted scoreThreshold to -Infinity, while every SQL backend (Sqlite / Postgres / Supabase / SqliteAiVectorStorage) defaults to 0. Identical app code returned a different result set depending on the backing — a portability foot-gun. Switch in-memory + IndexedDB defaults to 0. Callers that want every result regardless of correlation pass scoreThreshold: -Infinity explicitly (which is exactly what the SQL backends already required). Inverted the two validation tests that previously asserted the negative-correlation behaviour.
…mFromTaskEvents createStreamFromTaskEvents wired stream_chunk / stream_end / status listeners onto the source task but only released them on a normal stream_end or terminal task status. A consumer that called reader.cancel() mid-stream (downstream abort, consumer-side limit hit) left every listener attached: the task kept emitting into a closed controller and the listeners pinned GC. Hoist the shared cleanup closure to underlying-source scope so the ReadableStream cancel handler can call it. cleanup is already idempotent via the closed flag, so concurrent cancel + stream_end is safe.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacks on #602. Four independent follow-up fixes.
1. (CRITICAL)
CacheJanitorlive-run guardsweepStaleRunPrivatecalledclearOlderThanunconditionally —createdAtis the row write time, not run liveness, so a long-running in-flight run whose rows predate the cutoff would have its cache deleted out from under it.CacheJanitornow accepts aliveRunIds: () => Iterable<string>snapshot (default() => []+ one-timeconsole.warn) and passes the live set intoclearOlderThan(olderThanInMs, excludeRunIds). The repo enumerates distinct old runIds and callsdeleteRunOlderThanper runId not in the exclude set (ITabularStoragehas noNOT INoperator).2. (HIGH)
Container.registerdispose-on-replaceregister/registerInstanceoverwrote a cached singleton without invoking its disposer. A DB connection, file handle, or event subscriber re-registered would leak the original. Extract the disposer chain intoinvokeDisposer(protocol probe) +disposeService(eviction wrapper that catches errors withconsole.warnso a buggy disposer cannot block re-registration).dispose()retains itsAggregateErrorcollection by callinginvokeDisposerdirectly. Inherited (parent-owned) instances are skipped.3. (HIGH) Vector default
scoreThresholdparityInMemoryVectorStorageandIndexedDbVectorStoragedefaultedscoreThresholdto-Infinity; every SQL backend (Sqlite / Postgres / Supabase / SqliteAi) defaults to0. Identical app code returned different result sets depending on the backing — a portability foot-gun. Switched in-memory + IndexedDB defaults to0. Callers that want every result regardless of correlation passscoreThreshold: -Infinityexplicitly. Inverted the two existing validation tests asserting the old behaviour.4. (HIGH)
StreamPump.createStreamFromTaskEventscancel handlerThe wired stream_chunk / stream_end / status listeners only released on a normal stream_end or terminal task status. A consumer calling
reader.cancel()mid-stream left every listener attached: the task kept emitting into a closed controller and the listeners pinned GC. Hoistcleanupout ofstart()so thecancelhandler can call it.cleanupis idempotent.Verification
bun run build:packagesclean (incl. types).bun scripts/test.ts graph vitest— 736 passed.bun scripts/test.ts util vitest— 648 passed, 10 skipped.bun scripts/test.ts storage vitest— 1484 passed, 13 skipped.Generated by Claude Code