Delegate keyring operations to CLI and simplify deployment identity#816
Open
Delegate keyring operations to CLI and simplify deployment identity#816
Conversation
EhabY
force-pushed
the
add-keyring-support-cli
branch
3 times, most recently
from
115f6cc to
b852660
Compare
EhabY
changed the title
EhabY
commented
Mar 4, 2026
| ): Promise<string | undefined> { | ||
| let binPath: string; | ||
| try { | ||
| binPath = await this.resolveBinary(url); |
Collaborator
Author
There was a problem hiding this comment.
This currently blocks to install the binary (on login, with keyring enabled, and CLI >= 2.29)
| ): Promise<void> { | ||
| let binPath: string; | ||
| try { | ||
| binPath = await this.resolveBinary(url); |
Collaborator
Author
There was a problem hiding this comment.
This currently blocks to install the binary (on logout, with keyring enabled, and CLI >= 2.29)
Replace @napi-rs/keyring (4 native .node binaries) with CLI subprocess calls. The extension now runs `coder login --use-token-as-session` to store tokens and `coder login token --url` to read them, keeping the credential format in sync with the CLI automatically. - Add CliCredentialManager that shells out to the Coder CLI - Update CliManager.configure() to accept binPath and delegate to CLI - Update LoginCoordinator to fetch CLI binary for keyring reads - Remove clearCredentials keyring cleanup (stale entries are harmless) - Remove @napi-rs/keyring dep, vendor script, supportedArchitectures - Delete KeyringStore and its tests
…both Make url the single source of truth for deployment identity. fetchBinary, configure, and clearCredentials now derive safeHostname via toSafeHost() internally, eliminating redundant parameters and preventing inconsistency. BinaryResolver and CliCredentialManager methods take url string instead of Deployment object.
…er logging Make storeToken resolve its own binary internally via the injected BinaryResolver, matching readToken and deleteToken. Remove the binPath parameter from both storeToken and configure since callers no longer need to supply it. Add locateBinary to CliManager as a cheap stat-only lookup that the container resolver tries before falling back to fetchBinary. Downgrade implementation-detail log messages from info/warn to debug, keeping info for significant events (server version, download start, stored/deleted token).
EhabY
force-pushed
the
add-keyring-support-cli
branch
from
b852660 to
9c992fe
Compare
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
@napi-rs/keyring(4 native.nodebinaries) with CLI subprocess calls (coder login --use-token-as-sessionto store,coder login token --urlto read,coder logout --url --yesto delete), keeping the credential format in sync with the CLI automaticallyurlthe single source of truth for deployment identity —fetchBinary,configure, andclearCredentialsnow derivesafeHostnameinternally viatoSafeHost(), eliminating redundant parametersKeyringStore, vendor script,supportedArchitecturesconfig, and@napi-rs/keyringdependency