Clarify saved and effective provider selection#725
Conversation
Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
WalkthroughProvider CLI listings now distinguish saved and runtime-only profiles. Provider selection reports environment overrides, effective providers, and clearer errors for non-selectable runtime profiles, with coverage for JSON and human-readable output. ChangesProvider CLI reporting
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant ProvidersUse
participant UserConfig
participant ActiveProviderEnv
participant ConnectivityCheck
ProvidersUse->>UserConfig: save requested active provider
ProvidersUse->>ActiveProviderEnv: read active-provider override
ActiveProviderEnv-->>ProvidersUse: return effective provider
ProvidersUse->>ConnectivityCheck: check effective provider
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
internal/cli/command_center_test.go (1)
194-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReset
stderralongsidestdoutfor clean failure messages.While this test passes correctly, it is a good practice to reset both output buffers before reusing them. This ensures that if the second command fails,
stderr.String()only contains the relevant error from the second execution, preventing confusingt.Fatalferror messages containing leftover artifacts.♻️ Proposed refactor
} stdout.Reset() + stderr.Reset() if code := runWithDeps([]string{"providers", "list"}, &stdout, &stderr, deps); code != exitSuccess { t.Fatalf("code=%d stderr=%s", code, stderr.String())🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@internal/cli/command_center_test.go` around lines 194 - 229, Reset stderr alongside stdout before the second runWithDeps invocation in TestRunProvidersListMarksUserAndRuntimeProfiles, so failure reporting reflects only the second command’s output.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@internal/cli/command_center_test.go`:
- Around line 194-229: Reset stderr alongside stdout before the second
runWithDeps invocation in TestRunProvidersListMarksUserAndRuntimeProfiles, so
failure reporting reflects only the second command’s output.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 86a322da-ff12-4cef-bf1f-5f77bcde8327
📒 Files selected for processing (4)
internal/cli/command_center.gointernal/cli/command_center_test.gointernal/cli/provider_onboarding.gointernal/cli/provider_onboarding_test.go
There was a problem hiding this comment.
Pull request overview
This PR improves clarity around provider selection by distinguishing the provider saved in user config from the provider that is actually effective at runtime (especially when ZERO_PROVIDER is set), and by enriching provider listing output so UIs can tell which entries are user-selectable vs runtime-derived.
Changes:
- Update
providers useto report whenZERO_PROVIDERoverrides the newly saved active provider, and include effective/override metadata in JSON output. - Enhance
providers list/currentoutput (human + JSON) withselectableandsourcemetadata to distinguish user-config providers from runtime-only entries. - Add tests covering the override messaging/JSON payload and the selectable/source metadata in provider lists.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| internal/cli/provider_onboarding.go | Adds override detection for ZERO_PROVIDER to clarify saved vs effective provider; enriches JSON output and error messaging for non-selectable providers. |
| internal/cli/provider_onboarding_test.go | Adds coverage for override messaging/JSON and for improved error text when selecting runtime-only providers. |
| internal/cli/command_center.go | Adds provider list metadata (selectable, source) and updates formatting to mark runtime-only/non-selectable entries. |
| internal/cli/command_center_test.go | Adds coverage for new selectable/source JSON fields and for the runtime-only marker in human output. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !provider.Selectable { | ||
| line += " (runtime-only; not selectable/saved)" | ||
| } |
| if !strings.Contains(stdout.String(), "runtime-only; not selectable/saved") { | ||
| t.Fatalf("runtime marker missing: %s", stdout.String()) | ||
| } |
Summary
ZERO_PROVIDERoverrides a provider saved byproviders useproviders useValidation
go test ./...go vet ./...govulncheck ./...Repository-wide golangci-lint still reports existing findings unrelated to this change.
Fixes #721
Summary by CodeRabbit
New Features
providers list --jsonincludes per-provider selectability plus a source indicator.providers usereports effective provider selection and whether an environment-based override is active.Bug Fixes