caveman sync's local-spend watermark has no lock, so two overlapping syncs upload the same rows twice

[AmirF194]

What happen

syncLocalSavings (packages/cli/src/index.ts) reads the sync watermark, queries the local spend DB for rows past it, POSTs them to /api/v1/imports, and only then writes the new watermark. There's no lock around that read-query-post-write sequence, so two processes that both fire it around the same time (two terminals finishing a cave wrap session close together, or two cave sync runs) can both read the same watermark before either writes it. Both then fetch the same/overlapping row set and both POST successfully, so the same spend/savings rows get uploaded twice.

It's the same shape as the telemetry watermark race fixed in #1116, just with no guard at all, not even the optimistic check telemetryTokenDelta had before that fix.

Ran it: built the CLI in a container, wrote a regression test using the same technique #1116's own test uses (a --require preload that delays fs.writeFileSync on the watermark file, to widen the exit race), pointed it at sync.json instead of config.json, seeded 2 rows in the local spend DB, and ran two caveman sync processes at once against a stub import endpoint. Both exited 0, both printed "synced 2 spans", the stub got 2 separate POSTs, 4 rows uploaded for 2 real spans.

No idea if the control-api backend dedupes imports server-side by span/trace id since that's not in this repo, so I can't say whether this actually double-counts on a real dashboard, only that the client sends the duplicate POST and the stub accepts both.

Expected

Each local spend row makes it to the dashboard exactly once, whichever process happens to sync it.

Before/after example

Input: two `caveman sync` processes racing, 2 unsynced rows
Got:   both POST the same 2 rows, watermark still ends up correct, but 4 rows landed server-side for 2 real spans
Want:  one process wins the race, the other sees the advanced watermark and sends nothing (or the same lock #1116 already uses)

Platform

Not tied to a particular wrapped agent, any two concurrent cave wrap/cave sync invocations trigger it.

Version / install method

HEAD 2fd153c6, built from source.

Happy to send a PR using the same lock as #1116 if that's useful, just say the word.

[JuliusBrussee]

Reproduced and fixed in #1103. Your diagnosis is exactly right, and the reproduction made it a 10-minute confirmation rather than a hunt — thank you. On the unfixed tree the race test gives 2 POSTs and 4 rows for 2 real spans, your reported symptom precisely.

One thing your issue couldn't have known to ask for: the #1116 lock could not be reused as-is. That claim holds across a local file write, so its 5s "holder must be dead" window is safe. This one holds across an unbounded network POST — a fixed 5s window would judge a live holder on a slow link dead, let a waiter steal the lock and re-POST, reintroducing the bug for exactly the users whose uploads are slowest. The holder now heartbeats the lock's mtime (3s) against a 15s window, so a killed holder is still reclaimed but a slow one is never robbed. The spin itself is now shared with telemetry rather than copied.

A waiter that can't get the lock in its budget sends nothing, leaves the watermark alone and exits 0 — its rows ride the next sync. The budget is short on purpose since syncAfterWrap runs on the wrap exit path.

Your open question — whether control-api dedupes imports server-side — is flagged for the maintainer. It decides whether this was cosmetic or actually double-counted live dashboards.

---

Generated by Claude Code