Sync → Prevent concurrent syncs from silently deleting files

[revett]

Resolves #83

Problem

• The remote manifest was uploaded with an unconditional PUT at the end of every sync, so when two devices synced at overlapping times the last writer won
• A manifest written without knowledge of the other device's push made that push look like a remote deletion on the next sync, and the file was then silently deleted locally with its bytes orphaned in the bucket

Changes

• Make the manifest upload conditional on the remote manifest still being exactly the version read at the start of the pass, via If-Match on its ETag, or If-None-Match: * on a first sync
• Losing the race now fails the pass loudly with a "sync again" message; state.json does not advance, and the next sync re-reads the fresh manifest and reconciles both devices' work with nothing lost
• Refuse to sync when a manifest read returns no ETag, rather than falling back to an unsafe unconditional upload
• Cover both races (existing manifest and racing first syncs) with unit tests, plus an end to end interleaved two device test against MinIO

Why

• Silently losing files breaks the core promise that no edit is ever lost, and overlapping syncs become routine the moment automatic sync exists
• Conditional writes are supported by R2, S3, and MinIO (verified against all documented or probed directly), so a plain optimistic check is the simplest fix, needing no locks or extra objects

Greptile Summary

This PR fixes the silent-deletion race (#83) by making the manifest upload conditional on the ETag read at the start of each sync pass — If-Match for an existing manifest, If-None-Match: * for a first sync. Losing the race now fails the pass loudly without advancing state.json, so the next sync re-reads the winner's manifest and reconciles both devices' work with nothing lost.

• sync.ts: readRemoteManifest now returns the manifest's ETag and refuses to proceed if none is present; syncOnce threads the ETag through to a conditional putObject call and surfaces a 412 as an explicit "sync again" error.
• storage.ts / errors.ts: GetResult gains an etag field, PutCondition and conditionHeaders are added, and 412 is mapped to the new "conflict" status.
• fake.ts: Fake storage grows a revision-based ETag map that enforces ifAbsent/ifMatch with real S3 semantics, including cleaning up etags on delete.
• Tests: Unit tests cover the no-etag refusal and both race shapes (existing manifest, racing first syncs); an end-to-end integration test reproduces the exact interleaving against a real bucket and verifies full convergence.

Confidence Score: 5/5

Safe to merge — the change is a well-scoped optimistic concurrency fix with no regressions to the happy path and comprehensive test coverage at both the unit and integration level.

The conditional-write logic in syncOnce is correct: it reads the ETag once, threads it through as If-Match (or If-None-Match: * on a first sync), and treats 412 as a clean retry signal without advancing state. The fake storage faithfully models real S3 semantics, the no-etag guard closes the unsafe-fallback path, and the two new race-condition unit tests plus the live-bucket interleaved test leave no obvious untested paths in the changed code.

No files require special attention.

Important Files Changed

Filename	Overview
src/storage/errors.ts	Adds HTTP 412 → "conflict" mapping; straightforward and correct.
src/storage/storage.ts	Adds etag to GetResult, PutCondition type, and conditionHeaders helper; passes headers to s3PutObject and surfaces the ETag from GET responses. Implementation is correct.
src/sync/fake.ts	Fake storage now tracks a revision-based etag map, enforces ifAbsent/ifMatch conditions correctly, and cleans up the etag map on deleteObject — matching real S3 semantics in all tested paths.
src/sync/sync.ts	Core fix: manifest upload is now conditional on the etag read at pass-start; a 412 surfaces as a loud "sync again" failure without advancing state.json. The no-etag guard and the firstSync/ifAbsent path are both handled correctly.
src/sync/sync.test.ts	New unit tests cover: manifest-without-etag refusal, two-device mid-pass race, and two first-syncs racing; all three use the monkey-patched fake correctly and assert the right post-race state.
src/sync/sync.itest.ts	New end-to-end interleaved-sync test against MinIO; the racingStorage wrapper correctly triggers B's full pass during A's manifest write, and the test verifies that both devices converge with no files lost after A retries.
src/storage/storage.itest.ts	Three new integration tests cover ETag round-trip, ifAbsent reject-on-existing, and ifMatch stale-reject; the fixed-key ifMatch test is now wrapped in try/finally for hermeticity.

Comments Outside Diff (1)

1. src/sync/fake.ts, line 111-113 (link)

   deleteObject leaves stale etag in the map

   store.delete(key) removes the content, but etags still holds the old revision for that key. If a future test does deleteObject then a conditional putObject with { kind: "ifMatch", etag: oldEtag }, the fake will succeed (old etag still matches), whereas real S3/R2 returns 412 because the object no longer exists. This diverges from the real behaviour the fake is meant to model, and would silently mask any bug in that path. Adding etags.delete(key) alongside store.delete(key) would close the gap.

_{Reviews (2): Last reviewed commit: "Address comments" | Re-trigger Greptile}