tidy-secrets: one sequence for a vault mutation - #245
Merged
Conversation
The four backend mutators — keyringStore and encStore, Set and Delete — each opened with the same three moves: validate the Ref, take c.mu, take the cross-process vault lock. The paragraph above them already explains what that lock is for: Migrate reads from one backend, writes to the other, verifies, and only then deletes the source, so a concurrent writer landing between the write and the read-back turns a verified handover into a delete of the last remaining copy. A fifth mutator that took only the mutex would reopen exactly that window, and it would look no different from its neighbours while doing it. writeLocked makes the sequence one thing, with the Ref check — whose StorageKey becomes a keyring entry and a map key inside the encrypted file — at the front of it. Chain.Set and Chain.Delete deliberately stay out: they announce after the write, today while c.mu is still held, and moving that out from under the mutex changes when concurrent writes become visible to a watcher. That is a behaviour question, not a rename. make ci green. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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
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.
Nightly tidy over
internal/secrets+internal/session.secrets:writeLocked— one sequence for the four backend mutators (validate Ref → mutex → cross-process vault lock)Checked and deliberately not done:
Chain.Set/Chain.Deleteshare the same prologue but announce afterwards while still holdingc.mu; routing them through the helper would move that announcement out from under the mutex, which changes when a concurrent write becomes visible to a watcher.MemoryManager.List/FindByTokeneach snapshot the session map under the lock in the same six lines. Two callers, no drift — below the bar.config.md's secrets invariants spot-checked against the code:DefaultKeyringTimeout= 3s,envValue'sAGENTHUB_and reserved-keyguards both present.