Summary
clearSession() correctly deletes from 24 session-scoped tables, but it is event-driven (session.deleted) and therefore blind to sessions removed from opencode.db by any path that doesn't emit the event. sweepOrphanedOpenCodeMessageIndexes exists as the backstop for exactly that case — but it reaps one table:
DELETE FROM message_history_index
So 23 of the 24 have no backstop. Measured on my store after an out-of-band prune: 1,992 of 6,443 session_meta rows orphaned (31%).
Filing because you clearly consider this class real enough to have built the sweep and its bookkeeping table. This is "the half that wasn't covered" plus a trap worth knowing before extending it.
Evidence
Cross-DB join against opencode.db:
session_meta rows total 6,443
orphaned (no session) 1,992 ← 31%
live 4,450
pi (see below) 1
Newest orphan's last_response_time was ~a week old, consistent with a one-shot prune rather than an ongoing leak. Cause on my box: an offline event-store prune deleted session rows directly without emitting session.deleted, so the event-driven cleanup never fired. Normal deletion is handled correctly — I checked clearSession() before assuming otherwise, and session_meta is in its delete list.
I reaped mine manually. Only ~2.0 MB, so this is hygiene rather than a space problem — the reason to care is that the rows are invisible and accumulate silently.
The trap, if you extend the sweep
This is the part I'd most want a future implementer to have. The obvious query —
DELETE FROM session_meta WHERE session_id NOT IN (SELECT id FROM oc.session)
— destroys Pi metadata. Pi sessions are legitimately absent from opencode.db by design. I had exactly one such row and nearly took it; the count is small enough that nobody would notice until Pi behaved oddly.
Any sweep of this class must scope on harness = 'opencode' first. The existing session_meta table has that column, so it's available.
Worth noting the asymmetry it creates: session_meta can be swept safely because it carries harness. Tables that don't carry a harness discriminator can't be, which may constrain how far the sweep can extend. (Related: I raised the same missing-discriminator point about the new memory_evidence table in #340's review.)
Verification helper
The arithmetic should close, which is a cheap correctness check for any implementation:
orphaned(harness='opencode') + pi_rows + live_rows == total_rows
On mine: 1,992 + 1 + 4,450 = 6,443 ✓. If it doesn't close, the predicate is wrong.
Scoping, honestly
The trigger — out-of-band session deletion — is not something a typical user hits; the event path covers ordinary deletion. So this is arguably low priority. Two reasons I filed it anyway:
- The infrastructure already exists (
message_history_orphan_sweep bookkeeping, dream-timer tick), so extending it to session_meta is small.
- The failure is silent. Nothing surfaces orphaned rows, and I only found them because I was chasing an unrelated stale row.
Happy to send a PR for a harness-scoped session_meta sweep reusing the existing bookkeeping pattern, if you want it. Equally happy to have this closed as working-as-intended — the event path really is the right primary mechanism, and I'd rather ask than assume the sweep's single-table scope was an oversight.
Summary
clearSession()correctly deletes from 24 session-scoped tables, but it is event-driven (session.deleted) and therefore blind to sessions removed fromopencode.dbby any path that doesn't emit the event.sweepOrphanedOpenCodeMessageIndexesexists as the backstop for exactly that case — but it reaps one table:So 23 of the 24 have no backstop. Measured on my store after an out-of-band prune: 1,992 of 6,443
session_metarows orphaned (31%).Filing because you clearly consider this class real enough to have built the sweep and its bookkeeping table. This is "the half that wasn't covered" plus a trap worth knowing before extending it.
Evidence
Cross-DB join against
opencode.db:Newest orphan's
last_response_timewas ~a week old, consistent with a one-shot prune rather than an ongoing leak. Cause on my box: an offline event-store prune deletedsessionrows directly without emittingsession.deleted, so the event-driven cleanup never fired. Normal deletion is handled correctly — I checkedclearSession()before assuming otherwise, andsession_metais in its delete list.I reaped mine manually. Only ~2.0 MB, so this is hygiene rather than a space problem — the reason to care is that the rows are invisible and accumulate silently.
The trap, if you extend the sweep
This is the part I'd most want a future implementer to have. The obvious query —
— destroys Pi metadata. Pi sessions are legitimately absent from
opencode.dbby design. I had exactly one such row and nearly took it; the count is small enough that nobody would notice until Pi behaved oddly.Any sweep of this class must scope on
harness = 'opencode'first. The existingsession_metatable has that column, so it's available.Worth noting the asymmetry it creates:
session_metacan be swept safely because it carriesharness. Tables that don't carry a harness discriminator can't be, which may constrain how far the sweep can extend. (Related: I raised the same missing-discriminator point about the newmemory_evidencetable in #340's review.)Verification helper
The arithmetic should close, which is a cheap correctness check for any implementation:
On mine: 1,992 + 1 + 4,450 = 6,443 ✓. If it doesn't close, the predicate is wrong.
Scoping, honestly
The trigger — out-of-band session deletion — is not something a typical user hits; the event path covers ordinary deletion. So this is arguably low priority. Two reasons I filed it anyway:
message_history_orphan_sweepbookkeeping, dream-timer tick), so extending it tosession_metais small.Happy to send a PR for a
harness-scopedsession_metasweep reusing the existing bookkeeping pattern, if you want it. Equally happy to have this closed as working-as-intended — the event path really is the right primary mechanism, and I'd rather ask than assume the sweep's single-table scope was an oversight.