mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-23 02:53:31 +00:00
xarmian
31aba0bb5b
CreateActivityDebounced merged by reading a row's metadata, combining it in Go, and writing the result back — a read-modify-write across two statements. Two writers that selected the SAME row both read the pre-merge blob, each merged its own change into it, and the second UPDATE erased the first one's change entry with no error and no trace. Since BUG-2763 the racing pair must be the same account, actor kind AND agent name, which is exactly the ordinary case: one editor's autosave burst, or one agent issuing concurrent PATCHes. The merge UPDATE now carries the blob the caller merged from as a compare-and-set arm, and the caller is a bounded attempt loop that re-reads its candidate each time — so a losing attempt merges into the WINNER's blob rather than the stale one it first read. Adding that arm gives one UPDATE two ways to affect zero rows, and they want OPPOSITE dispositions: a comment-linked row (TASK-2760's freeze) must NOT be retried, since the answer will not change, while a contended row must NOT start a new run, since retrying is what keeps the entry whole. debounceRowUnchanged asks the CAS arm's own question after a refusal to tell them apart — one extra primary-key probe, only on the rare refusal path. Both statements spell that arm with ONE shared constant so they cannot drift into asking different questions. Also fixed here, found by review of the same code: - mergeActivityMeta panicked on a JSON `null` blob. `null` unmarshals with no error and leaves the map nil; the overlay then assigns into it. Valid JSON, valid JSONB, storable in the TEXT column — a crash on a write path, not a bad merge. - The merge timestamp is now taken per ATTEMPT. Hoisted out of the loop, a retry backdated a row whose newest change was younger than the stamp, and created_at is what the cooldown window, the timeline ordering and the status-transition backfill all read. - Two comments corrected: one claimed a lost CAS proves another merge landed (deletion does it too), and maxDebounceCandidates' comment predicted a query plan it has no business predicting. DUAL DIALECT: activities.metadata is TEXT on SQLite and JSONB on Postgres, so the CAS is byte equality on one and jsonb equality on the other. Verified empirically on postgres:17-alpine, both discriminating legs (a stale expectation refuses, a matching one writes), which is what rules out the comparison being stuck true or stuck false there. Tests drive the interleaving through a Store test seam rather than racing goroutines: the defect needs the competing write to land strictly between one call's read and its write, and real goroutines produce that ordering only sometimes — a detector with an unknown rate reads as coverage without being it. Against the pre-fix behaviour the regression test fails with the competitor's change silently absent, which is the filed symptom verbatim. Eight mutations of the fix all die; three tests carry explicit notes naming what they cannot discriminate. Six pre-existing defects surfaced by the review are filed rather than folded: BUG-2776, BUG-2777, BUG-2778, BUG-2779, BUG-2781.