Add attachment garbage collection sweep#92
Merged
Conversation
Adds Stack.collectAttachmentGarbage() / ScopedStack.collectAttachmentGarbage() (owner-only): an explicit sweep that deletes attachment bytes no longer reachable from any record, live or soft-deleted, via an attachment Association or a file-ref content field (#63). _attachment@1 metadata records never themselves count as references, but a grace period (default 24h) protects files whose newest metadata record — or, for bare bytes with none, the blob's own modifiedAt — is too recent, covering the legitimate upload-then-associate window. Deletion goes through deleteAttachment() itself, so a file that races (referenced or already gone by delete time) is skipped rather than aborting the sweep. Adds an optional StackBlobAdapter.listFiles() capability (implemented in blob-adapter-disk and MemoryAdapter, wired through LocalAdapter) so the sweep can also find bare-bytes orphans left by a putAttachment() that stored bytes but crashed before writing metadata. Also fixes two latent gaps in deleteAttachment()'s non-atomic fallback uncovered while building this: its reference check and metadata-cleanup scan didn't pass includeDeleted, so a soft-deleted (recoverable) record's attachment reference didn't block deletion, and a soft-deleted metadata record wasn't cleaned up — both already handled correctly by the atomic adapter path, now consistent. Fixes combineAdapters() silently dropping optional adapter capabilities (deleteUnreferencedAttachmentRecords, listFiles) instead of forwarding them when the underlying part implements them.
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
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.
Summary
Closes #64.
Attachment bytes were only ever removed by an explicit
deleteAttachment(fileId)call — normal app flows delete records, and nothing noticed when the last record referencing a file went away. This adds an explicit, owner-invoked sweep to find and remove those orphans.Stack.collectAttachmentGarbage(opts?)/ScopedStack.collectAttachmentGarbage(opts?)(owner-only): collects attachment bytes no longer reachable from any record — live or soft-deleted — via anattachmentAssociation or afile-refcontent field (file-ref field kind: content-held file references are invisible to the reference checks the spec says exist #63). A soft-deleted record is recoverable viaundelete()and must find its attachments intact, so it still counts as a reference._attachment@1metadata records never themselves count as references (else nothing would ever be garbage), but a file's newest metadata record — or, for bare bytes with none, the blob's own storage timestamp — must be older thangraceMs(default 24h) to be collected. This protects the legitimate upload-then-associate window.deleteAttachment()itself, so its usual conflict check runs once more per file at delete time; a file that races (referenced again, or already gone) is skipped, not treated as a sweep failure — the sweep always completes and reports what it actually collected.dryRun: truecomputes and returns what would be deleted without deleting anything.StackBlobAdapter.listFiles()capability (same "optional method, checked for truthiness" pattern asdeleteUnreferencedAttachmentRecords), implemented inblob-adapter-diskandMemoryAdapter, wired throughLocalAdapter. This is what finds bare-bytes orphans — bytes with zero metadata records at all, left by aputAttachment()that stored bytes but crashed before writing metadata — which are otherwise undiscoverable. An adapter without it still gets full protection for the common case.Two latent bugs fixed along the way
deleteAttachment()'s non-atomic fallback path didn't passincludeDeletedon its reference check or its metadata-cleanup scan, so a soft-deleted (recoverable) record's attachment reference didn't block deletion, and a soft-deleted metadata record was never cleaned up. The atomic adapter path already got this right (it doesn't filter ondeleted_atat all); the fallback now matches.combineAdapters()silently dropped optional adapter capabilities (deleteUnreferencedAttachmentRecords, and nowlistFiles) instead of forwarding them when the underlying part actually implements them — meaning a record adapter that supports the atomic delete path lost that capability the moment it was composed viacombineAdapters().Test plan
packages/core/tests/stack.test.ts—collectAttachmentGarbage: basic collection, live/soft-deleted/file-ref reference protection, default and explicit grace period,reclaimedBytesaccounting,dryRun, bare-bytes orphan collection vialistFiles(), graceful behavior whenlistFiles()is absent, and a simulated delete-time race that's skipped without aborting the sweep. Plus regression tests for the twoincludeDeletedfixes.packages/core/tests/scoped-stack.test.ts— owner-only gating (owner/non-owner/anonymous).packages/core/tests/combine.test.ts(new) — optional-capability forwarding round-trips exactly (present when implemented, absent when not), plus basic composition and lifecycle behavior.packages/blob-adapter-disk/tests/blob.test.ts—listFiles()enumeration, sizes, timestamps, post-delete removal, ignoring stray non-fileId files.packages/adapter-local/tests/local.test.ts—listFiles()through the real combined adapter.🤖 Generated with Claude Code
Generated by Claude Code