Handle nil keyring gracefully and fix gRPC v3 broker startup race#1603
Open
vzhang-stripe wants to merge 2 commits into
Open
Handle nil keyring gracefully and fix gRPC v3 broker startup race#1603vzhang-stripe wants to merge 2 commits into
vzhang-stripe wants to merge 2 commits into
Conversation
Return a typed errKeychainUnavailable error when config.KeyRing is nil instead of panicking on a nil pointer dereference. Refactor the gRPC v3 plugin broker to start the CoreCLIHelper server with a short publish delay so non-Go plugins can register pending dials before the connection info arrives. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> Committed-By-Agent: claude
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.
Fix intermittent V3 plugin keychain initialization failures
Summary
Fix an intermittent
directoryplugin failure where plugin commands can exit with:Error: Keychain not initialized. Call initKeychain(coreCLIHelper) first.The main fix is in the V3 host/plugin gRPC sequencing. We now start the plugin
RunCommandfirst and publish theCoreCLIHelperbroker slightly later, which avoids a race in the current non-Go plugin bootstrap. This PR also makes true keyring/backend issues fail with a clearer host-side error instead of the misleading initialization message.Root cause
The
directoryplugin uses the TS plugin bootstrap, which can drop unsolicited brokerConnInfoif it arrives before the plugin has registered a pending dial for that service ID.Before this change, the host published the
CoreCLIHelperbroker immediately. If the plugin had not yet started waiting on that broker ID, the connection info could be lost. The plugin would then time out during helper setup, never finishinitKeychain(coreCLIHelper), and later fail withKeychain not initialized.This matches the observed behavior:
What changed
RunCommandsetup so the plugin RPC starts before helper broker publication.system keychain is unavailableinstead of surfacing asKeychain not initialized.Why this approach
The more principled fix is in the TS bootstrap: buffer early broker
ConnInfoinstead of dropping it. This PR fixes the problem from the CLI host side so existing plugins work reliably without requiring an immediate bootstrap rollout.Testing
go test ./pkg/pluginsgo test -race ./pkg/plugins