Relationships
#3550 Regression from #3546: legacy (hashless) cache files lose their mtime trust, so a peer's deletion can be undone and untouched files are re-uploaded (seed the own record once)
Opened by stack72 · 10/16/2026· Shipped 10/16/2026
Regression in released @swamp/s3-datastore and @swamp/gcs-datastore 2026.10.16.2 (swamp-club#3546, PR #505, commit 81f5baa7f). swamp-extensions has issues turned off, so this is tracked here.
Why it's urgent: hashless entries are files nobody has pushed since content hashing landed (2026-05-24). Data versions are written once and never re-pushed, so every v2 S3 or GCS repo older than late May holds many of them. On their first sync after upgrading:
- a peer's
swamp data gcdeletions can be undone (the #2999 class); - every untouched legacy file can be re-uploaded on the first full push.
What follows is the #3546 agent's own write-up, kept as written.
Follow-up to swamp-club #3546 (PR #505, commit 81f5baa7f).
Scope: @swamp/s3-datastore and @swamp/gcs-datastore, v2 file-sync path.
Background
#3546 added a local-only, never-synced record per cache,
.datastore-local-mtimes.json. For each file it holds the mtime, size and
sha256 the file had when this copy last wrote or checked it. Only this record
may now skip a hash, or vouch that a file is unchanged. The index's
localMtime is the last pusher's mtime and no longer decides anything.
That is right for entries with a sha256: with no record, the code hashes the file, which costs time but stays correct. Entries without a sha256 have no hash to fall back on. Those are files nobody has pushed since content hashing landed on 2026-05-24. A freshly upgraded copy has no own record for any file it downloaded before the upgrade, so those files get the most conservative treatment.
Regressions this leaves (vs. main before #505)
- A file a peer deleted can come back (the #2999 class, for legacy files
only).
removeIfUnchangedkeeps a hashless file unless an own record matches. Its index entry is gone, so the next push walk sees no entry,fileNeedsPushreturns true, and the file is uploaded again. That undoes the peer'sswamp data gc. The window lasts until this copy pushes or downloads each legacy file again. - One-off re-upload.
fileNeedsPushpushes any same-size hashless entry with no own record. A large old repo re-uploads every untouched legacy file on its first full-walk push after upgrading. - A stale file can overwrite a newer one (unlikely). That re-upload can overwrite newer same-size content. This needs a peer still pushing without sha256 (an extension version from before 2026-05-24), and this copy never downloading the change (pull is size-only for hashless entries).
Proposed fix
Seed the own record once, for hashless entries only, from this copy's
on-disk .datastore-index.json, and only when .datastore-local-mtimes.json
does not exist yet.
- For each on-disk index entry with no
sha256whoselocalMtimeequals the local file's current mtime, and whose size equals the local file's size, record{ mtime, size }with no sha256. - Do not seed entries that have a sha256. They keep the #3546 behaviour: hash once with no record, which is how the issue's bug is prevented.
- Write the seeded record straight away, so the seeding runs once per cache. Afterwards an empty or partial record means "nothing verified", never "please seed again".
This trusts hashless entries exactly as much as main did. It is no weaker
than before, and the #3546 bug cannot happen on these entries anyway, because
it needs a sha256 that differs.
Where it goes
loadOwnMtimes()indatastore/s3/extensions/datastores/_lib/s3_cache_sync.tsanddatastore/gcs/extensions/datastores/_lib/gcs_cache_sync.ts. On ENOENT only (not on a corrupt file), readthis.indexPath, seed, and mark the record dirty so the nextflushOwnMtimes()persists it. A corrupt record should still start empty, with no seeding, so it can never re-trust foreign mtimes.- Seed from the on-disk index, never from the in-memory index or a remote pull. The on-disk file can still hold a pusher's raw payload; see "Open questions".
Tests (both providers, in-memory mock clients)
- Upgrade then peer deletion: a hashless entry in the on-disk index with a
matching
localMtime, no own-record file, and the remote no longer listing it. A pull removes the file, and a later push does not re-upload it. - Upgrade then push: the same hashless entry, unchanged, no own-record file.
pushChangeduploads nothing (count data PUTs). - Not seeded: a hashed entry with a matching foreign
localMtimeand different bytes is still replaced on pull. This is the #3546 regression test, which must stay green. - A corrupt own-record file does not trigger seeding.
- Seeding happens once: delete an entry from the record, reload, and confirm it is not seeded again.
Open questions
- How much can the on-disk index's
localMtimebe trusted? Before #222 the on-disk index could hold the raw remote payload, which carries the pusher's mtimes. For hashless entries, trusting it is exactly whatmaindid, so seeding adds no new risk. But it also does nothing to make hashless entries safer. - Should seeding live in a shared helper? The two cache-sync files duplicate each other today, which matches how #3546 was done.
Cost
Small: about 30 lines per provider, plus tests. Both manifests get a version bump.
Added by review
- The proposed fix is right: seed once, hashless entries only, from the on-disk index, only when the record file is absent, never from a corrupt record. It trusts legacy entries exactly as much as
maindid before #3546. Hashed entries keep #3546's behaviour. - Ship it as its own extension release, promptly, and say in the release notes that it fixes a regression from the previous release.
- Add one test beyond the five above: a repo with many hashless legacy files upgrades, and its first serve-driven sync (or
datastore sync) uploads nothing for unchanged files. Count the requests, so the re-upload storm is pinned.
Blocked by: nothing.
Shipped
Click a lifecycle step above to view its details.
system commented 10/16/2026, 4:49:47 PM
Classified automatically when this issue was filed.
- Source: Extensions
If you feel this classification is incorrect, add a ripple to tell us so.
Sign in to post a ripple.