mirror of
https://github.com/PerpetualSoftware/pad.git
synced 2026-09-21 01:53:33 +00:00
xarmian
effea01666
handleUpdateItem built the activity's human-readable change list by diffing the item it read at the TOP of the request — before the permission checks, before the store's locks — against the row the store wrote. Anything a concurrent writer committed inside that window appeared in the difference and was stamped, with this request's actor and agent name, onto whoever sent the PATCH. Agent A sets status, agent B sets priority, and B's entry reads "status: open → done; priority: low → high" over B's name. Unlike BUG-2770, where a change went MISSING, here the timeline gains a confident false statement about who did what — and BUG-2770's debounce then merges that statement forward into the coalesced row, so it outlives the request that invented it. The fix is a carrier, not new machinery. The store has re-read the row under its own locks since TASK-2533, unconditionally, and diffs THAT snapshot for its status and assignment signals; the handler simply had no way to reach it. models.Item.PreUpdate hands the same snapshot back on the returned item — `json:"-"`, transient, populated only by the update path, following the LastMutation precedent. The title, role and assignment arms move onto it too, which makes the whole list committed-vs-committed: what this transaction wrote over, versus what it wrote. A missing pre-image DROPS the change list rather than falling back to the handler's stale read. The fallback is the defect wearing a warning: an entry that says nothing is recoverable, one that names the wrong author is not. The activity row is still written; a slog.Warn names the invariant. Also fixed, same lines: the title arm only recorded a rename when the field diff had produced nothing, so a PATCH that renamed AND edited a field silently dropped the rename. Two test seams are added (Server.afterItemPreRead, Store.afterItemPreLockRead), both nil in production and both documenting their reentrancy requirement. The second exists because of a mutation that SURVIVED the first matrix: with the rival's write landing before the store call, a pre-image taken from the store's pre-lock read is indistinguishable from one taken under the lock — the instrument could not see the difference the fix is about. That mutation now dies. Seven mutations aimed, six die; the seventh (aliasing the pre-image instead of copying it) survives by design and says so in the code. The Postgres leg earned its keep again: two store assertions compared `fields` blobs byte-wise, which passes on SQLite (TEXT, exact bytes) and fails on Postgres (JSONB, re-serialised) while proving nothing either way about which snapshot the blob came from. They compare by value now.