`attachmentBytes` wrote DECRYPTED bytes to `cacheDir/att-<id>` and nothing
ever removed them. So a disappearing message expired, the bubble went, and
the photo or voice note stayed on disk indefinitely — the exact failure the
feature exists to prevent. Spec §6.11: attachments MUST be purged with the
message that referenced them.
🔴 **A sweep, not a purge at each deletion point, and that changed what the
fix had to be.** FOUR paths drop a row — message expiry, story expiry,
delete-for-me and clear-thread — so threading "here is what I removed" out
of each would still miss anything a future path drops, and would never
reach the orphans already on disk. Asking instead "what is still
referenced?" is complete by construction and self-healing.
🔴 **Refs are found by SHAPE.** An attachment reference serialises with
`id`, `key` and `digest` together and nothing else in the vault has all
three, so `attachmentIdsInUse` walks every record and matches on that. A
hand-written list of stores is wrong the day somebody adds a store and
does not update it — and the sweep would then delete attachments still in
use, which is far worse than the leak.
⚠️ **That is not hypothetical: enumerating by hand gets it wrong today.**
The obvious sources are history and stories. The vault ALSO holds refs in
the story archive, in per-contact avatars and in this account's own
avatar. A sweep built on the obvious two would have deleted every avatar
on the device the first time it ran, and blanked the user's own story
archive — which is the one thing that archive exists to prevent.
Three more safety properties, each with a test that fails without it:
fails closed when the in-use set cannot be built (a partial answer is what
turns a cleanup into data loss); a grace period so a sweep racing a fetch
cannot delete bytes on their way to the screen; and a throttle inside the
function, since it reads every vault record and is called from the poll
loop.
⚠️ The cost is that a file survives to the next sweep rather than going at
the instant of expiry — named in the code rather than left implied.
17 tests. Every guard proved by breaking it. ⚠️ One did NOT hold at first:
removing the `/` -> `_` filename mangling broke nothing, because every id
in the suite was slash-free — the guard was untested by the tests meant to
be guarding it. Added the case; it now fails as it should.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
