feat(attachments): §16 quiescent byte GC — orphaned-workspace purge#291
Open
Stvad wants to merge 5 commits into
Open
feat(attachments): §16 quiescent byte GC — orphaned-workspace purge#291Stvad wants to merge 5 commits into
Stvad wants to merge 5 commits into
Conversation
…purge) Give byteStore.purgeWorkspace its first live caller: a quiescent, grace-windowed reference-GC sweep that reclaims the local OPFS byte prefix of a workspace the active user no longer has access to (revoke / leave / workspace delete). The model (docs/media-attachments/byte-gc-design.md): - Reference set = the local `blocks` table (live + soft-deleted media). Revoke physically drops a workspace's rows, so its prefix goes unreferenced -> reclaimable; a soft-deleted tombstone keeps its hash -> retained (undo-safe). row_events / server blocks_history are deliberately NOT counted (counting them would make revoke un-reclaimable, and the client can't read the encrypted server history). - Branch A (built): a stored workspace prefix absent from the local synced `workspaces` list is orphaned; purge the whole prefix (no content-key derivation) behind a grace window + the down-lane lock, on a settled-synced remote session, with a sole-copy guard for un-uploaded bytes. - The grace window is load-bearing: it survives the checksum-wipe transient that can briefly empty the local DB (never a live zero-crossing); markers persist across sessions so the clock keeps accruing. - Branch B (per-content-key selective reap), remote-object GC, and quota eviction are designed in the doc and deferred (reference-permanent in v1). Not the reverted eager MediaRevocationPurger (b3ce6f7): this observes the settled truth under the lock, not a transient membership signal, so it needs no epoch/retry machinery. New: gcMarkerStore (durable grace markers), mediaGc (pure sweep + tests), assetGc (app wiring + gating tests), MediaGcSweeper (idle-scheduled user-scoped mount), byteStore.listWorkspaceIds. Full gate green (tsc + eslint + 4210 tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdXcVVBv5yV7x8Ag97dafo
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
…ad records A `failed` upload record's bytes were never uploaded (and there is no §9 recovery actor yet), so they are an un-uploaded sole copy exactly like `staged`/`pending`. The orphaned-workspace purge now defers on any of the three states, erring toward retention over dropping a sole copy (the §16 posture). A stuck `failed` record pins its workspace until it is cleared/recovered — an accepted, bounded leak. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdXcVVBv5yV7x8Ag97dafo
…test) Round-1 /areview follow-ups (all nit-level; no defects were found): - byteStore.purgeWorkspace: the caveat comment no longer says the §16 GC has "no caller today" — assetGc.runMediaGcSweep is now that caller and holds the lock. - mediaGc header: reword the over-stated "never throws per workspace" — a purge throw ends the sweep early and the rest retry next sweep (retention-biased). - MediaGcSweeper: note the first-sync trigger binds to the mount-time user, and why that's a latency-only detail (runMediaGcSweep re-reads the live user). - assetGc.test: pin the purge LOCK NAME (downLaneLockName) — the mutual exclusion with the down-lane depends on it, and the old runSingleOwner mock ignored it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdXcVVBv5yV7x8Ag97dafo
… write-amp) Full /areview fan-out (data-loss + races/lifecycle + quality) found NO blocker/major/minor defect — the loop converged. Applied the nits worth doing: - Correct the purge-safety comments (byteStore.purgeWorkspace, assetGc): the down-lane lock only excludes the DOWN-LANE put; capture/demand-resolve puts aren't under it. The real guard is that the GC only purges a workspace orphaned past its grace window (never accessible), which by construction isn't being captured into or viewed. Flagged that a future accessible-workspace reuse (Branch B) would need its own exclusion. - Avoid write amplification: only clear an accessible workspace's grace marker when one actually exists (cheap readonly get), instead of a commit-durable readwrite delete per accessible workspace every sweep. - Test cleanup: replace the 3-deep status ternary in the upload-store mock with a lookup. Left for greenlight: extracting the coalesced deep-idle scheduler shared verbatim by MediaGcSweeper and MediaDownLaneReplicator (a refactor of existing code; the repo has a stated inline-mount bias). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GdXcVVBv5yV7x8Ag97dafo
…heduler
MediaGcSweeper and MediaDownLaneReplicator hand-rolled the identical cancelled +
scheduled coalescing + scheduleDeepIdle machinery. Extract it to
`coalescedDeepIdlePass(work, warnLabel) -> { schedulePass, cancel }`; each mount
keeps its own work fn, its own triggers, and its own decision to fire an initial
pass (the down-lane does; the GC waits for first-sync settle) — the divergent
parts stay inline, matching the split laneArming.ts already draws for triggers.
Behavior-preserving: the existing MediaDownLaneReplicator.test.tsx (idle-defer +
coalesce-on-workspace-switch) stays green, and laneSchedule.test.ts pins the
helper's contract directly (defer, coalesce, re-arm, cancel-after-teardown,
log-only on a rejected pass).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GdXcVVBv5yV7x8Ag97dafo
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.
What & why
Lands the deferred §16 reference-counted byte GC for the media-attachments plugin — the keystone that gives
byteStore.purgeWorkspaceits first live caller. Until now the OPFS byte store grew monotonically; nothing reclaimed revoked/left-workspace bytes.v1 = "Branch A": a quiescent, grace-windowed sweep that purges the whole local OPFS byte prefix of a workspace the active user no longer has access to (revoke / leave / workspace delete). Design + full model:
docs/media-attachments/byte-gc-design.md.The model
blockstable (live + soft-deleted media). Revoke physically drops a workspace's rows, so its prefix goes unreferenced → reclaimable; a soft-deleted tombstone keeps its hash → retained (undo-safe).row_events/ the server's encryptedblocks_historyare deliberately not counted — counting them would make revoke un-reclaimable, and the client can't read the server history anyway.workspaceslist (SELECT id FROM workspaces— revoke drops the row). A stored prefix absent from it is orphaned.(user,workspace)down-lane lock the replicator holds, closing theput-vs-purge race documented onpurgeWorkspace. Gated onisRemoteSyncActive()+hasSynced. Sole-copy guard skips a workspace that still holds un-uploaded bytes.MediaRevocationPurger(b3ce6f7e): this observes the settled truth under the lock, not a transient membership signal, so it needs no epoch/retry machinery.Deferred (designed in the doc, not built here)
Branch B (per-content-key selective reap — staged orphans / replaced media within a live workspace), remote-object GC (reference-permanent — accepted monotonic remote growth), and quota-pressure eviction. All ride the same content-refcount infrastructure.
Files
mediaGc.ts(pure sweep + grace logic),assetGc.ts(app wiring + gating),gcMarkerStore.ts(durable grace markers),MediaGcSweeper.tsx(idle-scheduled user-scoped mount), plus tests for each.byteStore.ts(+listWorkspaceIds),assetDownLane.ts(exportdownLaneLockName),index.ts(wire the mount),design.html(§16 status).Verification
tscclean; fulleslintclean; 4210 tests pass (63 new acrossbyteStore/gcMarkerStore/mediaGc/assetGc).🤖 Generated with Claude Code
Generated by Claude Code