fix(libs): follow-ups to PR #604 review (build break + Container disposer + queryPage memory profile)

[sroussey]

Stacked on top of #604 — this branch is not a rewrite of that PR, it applies four independent fixups on top of it. Base is claude/beautiful-mayer-74ff08 so the diff is scoped to the follow-up work.

Commits

1. fix(task-graph): swap nonexistent storage.search for query in clearOlderThan

• Bug: RunPrivateTaskOutputRepository.clearOlderThan invoked this.storage.search(...) in the non-empty-excludeRunIds branch. ITabularStorage has no search method — the correct symbol is query. The call would TypeError at runtime as soon as the janitor was configured with a live-run exclude set (which the very next commit in fix(libs): janitor live-run guard, Container dispose-on-replace, vector default parity, StreamPump cancel #604 made the default). Type-checking missed it because the backing was typed at RunPrivateTaskOutputBacking = ITabularStorage<...> but the sole search call routed through the interface's method table at runtime.
• Fix: Swap search → query with the same criteria shape ({ createdAt: { value: cutoff, operator: "<" } }), matching the operator syntax already used by the deleteSearch calls on the same file.
• Test: New smoke test clearOlderThan with a non-empty excludeRunIds does not throw (in packages/test/src/test/task-graph-cache/CacheJanitor.test.ts) exercises the previously-broken branch end-to-end.

2. fix(task-graph/cache): require liveRunIds callback and drop unguarded fast-path

• Bug: CacheJanitorOptions.liveRunIds was optional. Callers who omitted it silently opted into a sweep that could reap in-flight runs' cache rows — a one-time console.warn isn't enough to make that safe. Related, RunPrivateTaskOutputRepository.clearOlderThan had an excludeRunIds.size === 0 early-return that bypassed the streaming path entirely and issued a single unbounded deleteSearch — a second code path with different memory characteristics from the exclude-set path.
• Fix: Make liveRunIds required (drop the ? on the option, drop the default and the one-time warn). Callers that genuinely never sweep concurrent with a live run pass () => [] explicitly. Also drop the excludeRunIds.size === 0 fast-path so the distinct-runId loop is the sole eviction implementation (correct for the empty-set case: it reaps everything). README example updated to match.
• Tests:
  • Updated existing CacheJanitor.test.ts cases to pass liveRunIds: () => [] explicitly.
  • New omitting liveRunIds is a compile-time error (uses @ts-expect-error).
  • New empty liveRunIds still respects the per-run indexed delete path — seeds 3 stale runs + 1 live-but-excluded run, calls sweepStaleRunPrivate with () => ["live"], asserts live-run rows survive and 3 stale sets are deleted.

3. fix(util/di): drain eviction disposals and clear stale factories on registerInstance

• Bug: fix(libs): janitor live-run guard, Container dispose-on-replace, vector default parity, StreamPump cancel #604's disposer fix for register / registerInstance fired the previous singleton's disposer with void. The caller — and dispose() itself — had no way to observe when the released resource actually shut down. A fast re-register + dispose sequence could clear container state while an async disposer was still holding a DB connection open. Separately, registerInstance() left any previously-registered factory in the factories map; a subsequent remove() + get() could resurrect the factory-built object instead of failing loudly.
• Fix:
  • Track in-flight eviction disposals in a pendingDisposals: Map<string, Promise<void>>.
  • New public awaitReplacement(token: string): Promise<void> — resolves once the pending disposal (if any) for token settles.
  • New public awaitRegistrations(): Promise<void> — awaits every pending disposal.
  • dispose() calls awaitRegistrations() at the top before iterating live singletons and clears pendingDisposals alongside the other maps in finally.
  • Identity-guarded .finally() clears each entry only if it hasn't been superseded by a newer replacement.
  • registerInstance() now also calls this.factories.delete(token) alongside its other state mutations.
• Tests (in packages/test/src/test/util/Container.test.ts):
  • register schedules eviction dispose and awaitReplacement resolves after it completes — gated disposer; asserts disposeCalled is false synchronously after register, then true after resolving the gate and awaiting awaitReplacement.
  • container.dispose() drains pending disposals from prior register replacements.
  • registerInstance eviction is drained by awaitReplacement.
  • registerInstance clears any previously-registered factory for the token — asserts (container as any).factories.has(token) === false after a register → registerInstance sequence.

4. perf(task-graph): stream distinct runIds via queryPage in clearOlderThan

• Bug: After commit 1 fixed the search→query typo, the still-buggy shape remained: clearOlderThan issued one query(...) for the ENTIRE stale row set (no limit) and walked the buffer to extract distinct runIds. On a table with millions of stale rows this materializes an O(row-count) buffer of Entity objects (blob column included) — memory hazard proportional to how stale-friendly the caller's cutoff is.
• Fix: Rewrite the sweep to iterate the stale set via cursor-paginated queryPage in bounded 500-row batches. seenRunIds is bounded by distinct runId count (typically much smaller than row count), not row count. Every page carries a limit so SQL backends push both the WHERE and the keyset predicate into the SELECT (BaseTabularStorage's composite-PK fallback still fetches per-page via query; SQL backends override queryPage with tuple-comparison pushdown). Default ordering is PK-ASC (runId-leading), so a run's rows sit contiguously and distinct runIds accumulate quickly. Also introduce a private deleteRunOlderThanAt(runId, cutoff) helper so the sweep dispatches per-run deletes against a single pre-computed cutoff and emits output_pruned exactly once at the end (the public deleteRunOlderThan still emits per-call).
• Tests:
  • sweepStaleRunPrivate does not materialize the full stale row set at once (in CacheJanitor.test.ts) seeds 5000 stale rows across 3 runs, wraps queryPage in a spy, and asserts (a) ≥10 pagination calls (5000 rows / 500 per page), (b) every call carries a bounded limit, (c) the sweep completes and reaps every stale row.
  • clearOlderThan preserves live runs even when they have stale rows in a new packages/test/src/test/task-graph-output-cache/RunPrivateTaskOutputRepository.test.ts — verifies the excludeRunIds contract when every row on the excluded run also predates the cutoff.

Verification

Run from /home/user/libs:

bun x turbo run build-types --filter=@workglow/task-graph
# → 4 tasks successful, 0 failures.

bun scripts/test.ts graph vitest
# → 76 test files, 741 tests passed (adds 5 new tests: 4 in CacheJanitor.test.ts
#   + 1 in RunPrivateTaskOutputRepository.test.ts).

bun scripts/test.ts util vitest
# → 44 test files, 652 tests passed / 10 skipped (adds 4 new Container tests).

Base

Stacked on #604 (base branch claude/beautiful-mayer-74ff08, head SHA b33165d). Do not squash — the 4 commits are intentionally separate so bug fixes and perf improvements can be reviewed independently.

---

Generated by Claude Code

[github-actions]

Coverage Report

Status	Category	Percentage	Covered / Total
🔵	Lines	62.74%	25556 / 40732
🔵	Statements	62.59%	26445 / 42251
🔵	Functions	63.99%	4856 / 7588
🔵	Branches	51.5%	12523 / 24312

File Coverage

No changed files found.

Generated in workflow #2630 for commit 0010a63 by the Vitest Coverage Report Action