Repository navigation
Retry concurrent capability adds on the latest snapshot (CL-7216) - #600
Merged
TheGreatAxios merged 6 commits intoSep 4, 2026
Conversation
Two overlapping POSTs against the same definition must both land their pins. The suite uses a live in-memory asset so a last-write-wins clobber is observable.
Capability-add was a lock-free read-modify-write of the definition asset, so two overlapping POSTs could each snapshot the same tree and the later write would drop the earlier pin. The add now re-reads under a per-asset lock and reapplies the losing edit on the winner tree instead of last-write-wins.
PUT instructions, PUT skills, and DELETE model still last-write-wins clobber a concurrent edit of the same asset. The suite races those routes so a lost prompt, skill pin, or model-clear is observable.
PUT instructions, PUT skills, and DELETE model were still lock-free read-modify-writes of the same asset, so a concurrent edit could snapshot the old tree and clobber the other field. They now share the capability-add per-asset lock and reapply on the latest snapshot. Hub is a single replica; the lock is an in-process chain, not a distributed advisory lock.
POST pin was still a lock-free read-modify-write of the same asset, so a concurrent model-clear could last-write-wins drop the pin (or the pin could restore the model). The suite races those two routes so a lost skill or model-clear is observable.
POST /pin was still a lock-free read-modify-write of the definition asset, so a concurrent DELETE model could snapshot the old tree and clobber the pin (or the pin could restore the model). It now shares the capability-add per-asset lock and reapplies on the latest snapshot.
Contributor
Author
|
Fixes CL-7216 |
Contributor
Author
Self-reviewVerdict: the retry-on-latest-snapshot fix is correct — the per-asset promise-chain lock makes the stale-snapshot check atomic, so a losing writer reapplies on the winner's tree instead of clobbering it. No defects found; nothing changed.
Tests run (from |
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.
Summary
POST /:definitionId/capabilitieswas a lock-free read-modify-write of the definition asset. Two overlapping adds against the same definition could snapshot the same tree; the later write silently dropped the earlier pin (last-write-wins).The add now retries the loser against the latest snapshot. A per-asset lock makes the stale-snapshot check atomic so the losing edit reapplies on the winner tree instead of clobbering it. Both capability-adds land.
The same helper is used by the workflow-run capability route, which had the identical RMW.
Test
two concurrent capability-adds on the same definition both landfires two overlapping POSTs of different tool packages on onedefinitionIdand asserts the stored definition carries both pins.Out of scope
CL-7261 batched / multi-select UI (canceled).
Linear: CL-7216
Fixes CL-7216