What's wrong
retention.ts:20 acquires yield* AppDatabase and immediately discards it — the comment at :17-19 says its only purpose is ordering. The forked sweep at :22 then calls a zero-arg closure (:35, :37) that resolves its own database handle.
grep "from '@/database'" confirms query-retention.ts:4, reconcile-queries.ts:4 and trace-retention.ts:4 (plus span-writer.ts:1) all reach the module singleton directly.
This contradicts the convention the codebase states for itself in app-database.ts:2-4:
"Every other service reaches SQLite through this — never through a direct @/database import."
And app-database.ts:101-104 deliberately captures const client = database because "building the service across a different client would otherwise close the wrong handle" — the design explicitly anticipates a non-singleton client, at which point retention and boot reconciliation would silently target a different database than every other service, with no type or test catching it.
The target shape already exists next door: span-writer.ts:7 takes client: typeof database = database, and trace-store.ts:55 calls appDatabase.execute((client) => writeSpans(spans, client)). The retention/reconcile trio is the outlier. api-test-helper.ts:7-10 names the debt outright: "Module-mock harness for the handful of modules that still reach the drizzle singleton directly."
Proposed change
deleteExpiredQueries(client, …), deleteExpiredSpans(client, …), markInterruptedQueries(client); retention.ts binds the acquired service and passes appDatabase.client.
The ordering dependency then stops being comment-enforced and becomes the actual data flow — the sweep cannot run before AppDatabase exists, because it has nowhere to get a client.
Keep the parameter required, unlike span-writer's default — that default is precisely what let this leak persist.
Scope
Three src/main/** signatures plus retention.ts. The three co-located tests get to drop setupApiMocks() and the pre-import ordering dance.
Cross-file note: boot.ts:16 needs markInterruptedQueries(appDatabase.client). It already does yield* AppDatabase at :12, so it is a one-line edit with no new import.
Land this together with the "collapse the tracer's persist seam" issue — both touch span-writer.ts:5.
Risks
Low. No schema, behaviour or persisted-state change, and a required parameter makes a missed call site a compile error.
Use typeof database rather than the exported AppDatabaseClient type, to avoid introducing a new src/main/** → src/server/** dependency direction.
Validation
The nine existing cases across the three test files carry over. Add the test that is impossible to write today: run a sweep against database A while B is the module singleton, and assert B is untouched.
Found in a codebase-wide simplification audit (F-S14-a). Confidence: high. This also shrinks api-test-helper.ts to nothing — see the separate issue on deleting it.
What's wrong
retention.ts:20acquiresyield* AppDatabaseand immediately discards it — the comment at:17-19says its only purpose is ordering. The forked sweep at:22then calls a zero-arg closure (:35,:37) that resolves its own database handle.grep "from '@/database'"confirmsquery-retention.ts:4,reconcile-queries.ts:4andtrace-retention.ts:4(plusspan-writer.ts:1) all reach the module singleton directly.This contradicts the convention the codebase states for itself in
app-database.ts:2-4:And
app-database.ts:101-104deliberately capturesconst client = databasebecause "building the service across a different client would otherwise close the wrong handle" — the design explicitly anticipates a non-singleton client, at which point retention and boot reconciliation would silently target a different database than every other service, with no type or test catching it.The target shape already exists next door:
span-writer.ts:7takesclient: typeof database = database, andtrace-store.ts:55callsappDatabase.execute((client) => writeSpans(spans, client)). The retention/reconcile trio is the outlier.api-test-helper.ts:7-10names the debt outright: "Module-mock harness for the handful of modules that still reach the drizzle singleton directly."Proposed change
deleteExpiredQueries(client, …),deleteExpiredSpans(client, …),markInterruptedQueries(client);retention.tsbinds the acquired service and passesappDatabase.client.The ordering dependency then stops being comment-enforced and becomes the actual data flow — the sweep cannot run before
AppDatabaseexists, because it has nowhere to get a client.Keep the parameter required, unlike
span-writer's default — that default is precisely what let this leak persist.Scope
Three
src/main/**signatures plusretention.ts. The three co-located tests get to dropsetupApiMocks()and the pre-import ordering dance.Cross-file note:
boot.ts:16needsmarkInterruptedQueries(appDatabase.client). It already doesyield* AppDatabaseat:12, so it is a one-line edit with no new import.Land this together with the "collapse the tracer's persist seam" issue — both touch
span-writer.ts:5.Risks
Low. No schema, behaviour or persisted-state change, and a required parameter makes a missed call site a compile error.
Use
typeof databaserather than the exportedAppDatabaseClienttype, to avoid introducing a newsrc/main/**→src/server/**dependency direction.Validation
The nine existing cases across the three test files carry over. Add the test that is impossible to write today: run a sweep against database A while B is the module singleton, and assert B is untouched.
Found in a codebase-wide simplification audit (F-S14-a). Confidence: high. This also shrinks
api-test-helper.tsto nothing — see the separate issue on deleting it.