Repository navigation
fix: prevent spurious sandbox restarts under concurrent agent usage - #31
Merged
Merged
Conversation
- Remove inode/mtime from GPG socket fingerprint in resolveGpgSocketInfo; socket path alone is sufficient and inode/mtime churn when the GPG agent restarts was causing hash mismatches that triggered unnecessary restarts - Move the validate+compare+stop+start cycle inside the file lock in ensureSandbox so only one agent at a time can make the restart decision; add double-checked locking so agents that waited for the lock re-validate before restarting, picking up a freshly started sandbox when possible
malaporte
marked this pull request as ready for review
April 10, 2026 14:23
|
🎉 This PR is included in version 0.20.3 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
Summary
When multiple agents use the same named sandbox concurrently, the sandbox could restart unexpectedly. Two root causes were identified and fixed.
Changes
stat.inoandstat.mtimeMsfrom the fingerprint computed inresolveGpgSocketInfo. The socket path + dir is sufficient to identify the config; inode/mtime churn whenever the GPG agent restarts was causing hash mismatches that triggered unnecessary sandbox restarts.ensureSandbox: The validate → compare → stop → start cycle is now fully enclosed by the file lock. Previously onlystartSandboxwas locked, leaving a window where two concurrent agents could both observe a hash mismatch and both callstopSandbox, killing a sandbox that was actively in use. The new flow uses double-checked locking: a fast unlocked path handles the common case (hash matches, return immediately), and the slow locked path re-validates after acquiring the lock so agents that waited pick up a freshly started sandbox without triggering another restart.Testing
bun run typecheck— cleanbun run test— 139/139 tests passbun run build— all platform binaries compile successfullyReviewer Notes
The double-checked locking pattern relies on
validateState+computeConfigHashbeing safe to call concurrently (they are — both are read-only with respect to the state file). ThewriteStatecall insidestartSandboxuses an atomic temp-file rename, so there is no torn-read risk on the state file itself.