feat(config): explicit provider availability in config init#51
Merged
Conversation
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.
What & why
sk config initfiltered its provider picker down to providers it detected as available at that instant. Detection was ad-hoc per-provider sniffing that disagreed with how credentials are actually resolved at runtime:resolve_api_keyresolves an Anthropic key fromexplicit → providers.anthropic.api_key (config) → ANTHROPIC_API_KEY (env), but detection only checked the env var. A user whose key lived in config had a provider that worked but never appeared.This reframes the picker from capability-filter to intent-declaration: always show every provider, each labeled with an honest readiness state, and source that label from the same resolver the runtime uses so the two can't drift again.
Changes
resolve.rs— extract a sharedconfig_api_keyhelper and add a non-consumingcan_resolve_api_keyprobe that mirrorsresolve_api_key's config→env chain.resolve_api_key's observable behavior is unchanged.providers/config.rs— addProviderReadiness { Ready, NeedsSetup, NotInstalled }+provider_readiness(); remove theis_provider_availablegate and the env-onlyapi_provider_has_credentialssniffer. CLI providers → PATH check; API providers → cred probe;codex→ the sameload_codex_credsloader the runtime calls.cli/config.rs+main.rs—run_initnow offers every provider with a readiness label across all three paths (--provider, non-TTY, interactive). Selecting any provider — even non-ready — writes the config (invoking the command is the intent) and prints next-step guidance. ThreadsResolvedConfigin frommain.rs.docs/providers.md— documents the three readiness states.Readiness states
readyneeds setupnot installedKnown nuance (documented, not a bug)
At fresh init there's no config file yet (init refuses to overwrite an existing one), so Anthropic readiness reflects the env var only. The config-file-key path matters on surfaces that run against an existing config (re-init, future
doctor, runtime); unifying on the resolver makes those correct and prevents the drift that caused the original issue.Testing
cargo fmt --checkandcargo clippy --all-targets -- -D warningsclean.can_resolve_api_keyparity tests (probe agrees withresolve_api_key),provider_readinessmapping, and thereadiness_hint/next_step_hinthelpers.Follow-up (out of scope)
doctor/checks.rsstill referencesload_codex_credsindependently — a candidate to unify ontoprovider_readinesslater. The spec already scopeddoctorintegration as a follow-up.🤖 Spec + plan brainstormed, built via subagent-driven TDD, and reviewed before this PR.