feat(collection): refuse stale edits instead of silently overwriting them (#207) - #350
Merged
Merged
Conversation
…them (#207) ADR 0001 keeps scryme single-collection on purpose, and the common case for that is a household sharing one collection from two devices. Until now every mutation was last-write-wins: two people editing the same stack silently destroyed each other's change, with nothing to notice it by. The guard turns on the distinction that actually matters, rather than versioning everything: * **Relative edits stay unguarded.** `−`/`+` and "add to collection" are commutative — two people each adding a copy should end at +2, which is what last-write-wins already gives. Refusing the second would refuse a correct outcome and make the buttons feel broken. * **Absolute edits are guarded.** Setting a quantity, a finish, a condition, or deleting a stack replaces state the editor was *looking at*; if it moved underneath them, applying it destroys someone else's work. These carry the version they were shown and are refused on mismatch. Every mutation bumps `collection_card.version`, so an absolute edit notices a relative one that happened under it. A refusal returns the panel's **current state** with a note, at HTTP 409 — not a bare error. Failing without showing what the row is now would cost two edits rather than one: theirs, and the editor's own work with no way to re-apply it. The JSON API mirrors this, with `version` on `CollectionRowOut`, an optional `version` on PATCH and `?version=` on DELETE, and a 409 body carrying `current_version` so a client can retry deliberately rather than blindly. Sending no version opts out, so an already-open tab and existing API clients keep working exactly as before — this is a safety net, not a gate. htmx does not swap non-2xx responses, so a `htmx:beforeSwap` handler lets 409 through; without it the conflict body would be computed and then thrown away. Deliberately not included: the live-sync/SSE half. #207 scopes its own MVP to "version column + 409 + merge prompt" and says the stream should be shared with #166 — which is cross-app work. Migration 0034. 21 new tests; suite 1306 passing at 100% coverage. Verified against the dev instance: stale edit → 409 with the quantity untouched, current-version edit → 200, and a relative +1 applied while still bumping the version. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LxrePqFWyzQTcWaroSk4sH
![github-code-quality[bot]](https://avatars.githubusercontent.com/u/9919?s=60&v=4){kind=small}
Comment on lines
+271
to
+272
| assert (await client.delete( | ||
| f"/api/v1/collection/{stack.id}?version={stale}")).status_code == 409 |
Comment on lines
+276
to
+277
| assert (await client.delete( | ||
| f"/api/v1/collection/{stack.id}?version={stack.version}")).status_code == 200 |
| import sqlalchemy as sa | ||
| from alembic import op | ||
|
|
||
| revision: str = "0034_stack_version" |
| from alembic import op | ||
|
|
||
| revision: str = "0034_stack_version" | ||
| down_revision: str | None = "0033_client_token" |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes the MVP of #207 and the Settings & multi-device foundation milestone.
Why
ADR 0001 keeps scryme single-collection on purpose — and the common shape of that is a household sharing one collection from two devices. Until now every mutation was last-write-wins: two people editing the same stack silently destroyed each other's change, with nothing to notice it by. The ADR explicitly took on responsibility for making that safe.
The distinction that does the work
Rather than versioning every write, the guard turns on the difference between two kinds of edit:
−/+and Add to collection are commutative — two people each adding a copy should end at +2, which is exactly what last-write-wins already gives. Refusing the second would refuse a correct outcome and make the buttons feel broken.Every mutation bumps
version, so an absolute edit notices a relative one that happened under it.A refusal has to be useful
A 409 returns the panel's current state with a note, not a bare error. Failing without showing what the row is now would cost two edits instead of one — theirs, and the editor's own work with no way to re-apply it.
The JSON API mirrors it:
versiononCollectionRowOut, optionalversionin the PATCH body and?version=on DELETE, and a 409 body carryingcurrent_versionso a client can retry deliberately rather than blindly.Sending no version opts out, so an already-open tab and every existing API client keep working exactly as before. This is a safety net, not a gate.
One thing that would have silently broken it: htmx doesn't swap non-2xx responses, so the conflict body would have been computed and thrown away. A small
htmx:beforeSwaphandler lets 409 through.Deliberately not included
The live-sync / SSE half. #207 scopes its own MVP to "version column + 409-on-conflict + a merge prompt" and says the stream should be shared with #166 — which is cross-app work, currently parked.
Verification
ruffandmkdocs --strictclean. Migration 0034 applies and downgrades cleanly.409with the quantity untouched; a current-version edit →200; a relative+1applied while still bumping the version. Dev state restored afterwards.🤖 Generated with Claude Code
https://claude.ai/code/session_01LxrePqFWyzQTcWaroSk4sH