TL;DR
- The problem —
/profile open in two tabs, and the second save silently deletes what the first one added. saveChildren diffs fresh server state against a stale payload, so tab 2 reads [A, B], compares it to its own [A], and deletes B. Nothing on either screen says so, and credentials are what the badge attests to.
- The fix —
practitioners.updated_at as an optimistic-concurrency token: the read already returns it, so send it back with the save, put .eq("updated_at", …) on the profile update, and treat zero rows as a refusal telling the person to reload first. It covers the child tables too, as long as a save always touches the profile row — true today, and a coupling worth naming.
- To decide — whether the editor should reload itself instead. A Server Action returning the new state closes the common case without a token; the token is the thing that is correct under concurrency.
Full context — the reasoning, the constraints and what has already been ruled out. This is the part a coding agent should read.
From the review on #125.
The profile editor saves the whole record with no revision check anywhere in the read → plan → write cycle, and saveChildren re-reads the saved rows at the moment of writing. That combination is what makes it bite rather than merely last-write-wins: the diff is taken against fresh server state and a stale payload.
Failing case: /profile open in two tabs, profile holds credential A. Tab 1 adds B and saves. Tab 2 — still showing only A — changes the bio and saves. saveChildren reads [A, B], diffs it against tab 2's [A], and deletes B. Nothing on either screen says so. Same shape for services.
One person owns a profile, so this needs two tabs or a page left open across a save, and it is a bargain every form makes. What makes it worth an issue rather than a shrug is which rows disappear: credentials are what the badge attests to, and the deletion is invisible in both tabs.
The cheap version
practitioners.updated_at as an optimistic-concurrency token. The read already returns it; send it back with the save, put .eq("updated_at", …) on the profile update, and treat zero rows as a refusal — "this profile changed in another tab. Reload it before saving, or your changes here will overwrite what was saved there."
That covers the child tables too without a token of their own, provided the profile row is always touched by a save. It is today, and that is a coupling worth naming rather than relying on.
Worth checking first
Whether the editor should be reloading itself instead. A Server Action returning the new state, or the page revalidating into a keyed remount, would close the window for the common case — one person, two tabs, both theirs — without a token at all. The token is the thing that is correct under concurrency; the reload is the thing that stops most of it happening.
TL;DR
/profileopen in two tabs, and the second save silently deletes what the first one added.saveChildrendiffs fresh server state against a stale payload, so tab 2 reads[A, B], compares it to its own[A], and deletesB. Nothing on either screen says so, and credentials are what the badge attests to.practitioners.updated_atas an optimistic-concurrency token: the read already returns it, so send it back with the save, put.eq("updated_at", …)on the profile update, and treat zero rows as a refusal telling the person to reload first. It covers the child tables too, as long as a save always touches the profile row — true today, and a coupling worth naming.Full context — the reasoning, the constraints and what has already been ruled out. This is the part a coding agent should read.
From the review on #125.
The profile editor saves the whole record with no revision check anywhere in the read → plan → write cycle, and
saveChildrenre-reads the saved rows at the moment of writing. That combination is what makes it bite rather than merely last-write-wins: the diff is taken against fresh server state and a stale payload.Failing case:
/profileopen in two tabs, profile holds credentialA. Tab 1 addsBand saves. Tab 2 — still showing onlyA— changes the bio and saves.saveChildrenreads[A, B], diffs it against tab 2's[A], and deletesB. Nothing on either screen says so. Same shape for services.One person owns a profile, so this needs two tabs or a page left open across a save, and it is a bargain every form makes. What makes it worth an issue rather than a shrug is which rows disappear: credentials are what the badge attests to, and the deletion is invisible in both tabs.
The cheap version
practitioners.updated_atas an optimistic-concurrency token. The read already returns it; send it back with the save, put.eq("updated_at", …)on the profile update, and treat zero rows as a refusal — "this profile changed in another tab. Reload it before saving, or your changes here will overwrite what was saved there."That covers the child tables too without a token of their own, provided the profile row is always touched by a save. It is today, and that is a coupling worth naming rather than relying on.
Worth checking first
Whether the editor should be reloading itself instead. A Server Action returning the new state, or the page revalidating into a keyed remount, would close the window for the common case — one person, two tabs, both theirs — without a token at all. The token is the thing that is correct under concurrency; the reload is the thing that stops most of it happening.