fix(cli): warn when ZERO_PROVIDER overrides a providers-use selection (#721)

[Vasanthdev2004]

Closes #721.

The bug

zero providers use <name> writes activeProvider to config.json and reports Active provider set to <name>, even when ZERO_PROVIDER is set. applyEnv (resolver.go) makes ZERO_PROVIDER win over config.json unconditionally, so the reported switch is a silent no-op: providers current still shows the env-pinned provider.

The fix

Detect when ZERO_PROVIDER is set and names a different provider than the selection, and say so:

• text: a note to stderr naming the env var and the provider that stays active, and how to make the selection effective (unset ZERO_PROVIDER).
• --json: adds effectiveProvider and overriddenByEnv so a machine consumer is not misled either.

The config write is unchanged and still lands; only the reporting becomes honest. No warning when ZERO_PROVIDER is unset or already names the selection.

The env read is injected via a new appDeps.getenv (os.Getenv in production, deliberately left nil by fillAppDeps like exportActiveProvider) so the existing tests stay hermetic against a developer's ambient ZERO_PROVIDER (the reporter's setup) rather than newly failing on it.

Scope note

The issue also mentions providers use openai failing with "not found" for an env-derived profile. That path is already handled by the unpersisted-provider reporting (#707) and the env-derived-profile fix in #716, so this PR closes the remaining gap: the silent override on a successful switch.

Tests: text warning, JSON override fields, and the two no-warn cases (unset / matches). Package builds and the provider-use suite is green. (Pre-existing, unrelated TestBuildServeScopeKeepsLexicalPaths fails on main independently of this change.)

Summary by CodeRabbit

• New Features

  • Added visibility into provider selections overridden by the ZERO_PROVIDER environment variable.
  • Non-JSON output now warns when the environment setting takes precedence.
  • JSON output includes the requested provider, effective provider, and override source when applicable.

• Bug Fixes

  • Improved clarity around provider configuration conflicts, preventing misleading selection results.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 4011a72e-4d36-48d8-834f-ef3cdd69a36f

📥 Commits

Reviewing files that changed from the base of the PR and between e1975c1 and 3c51e79.

📒 Files selected for processing (3)

• internal/cli/app.go
• internal/cli/provider_onboarding.go
• internal/cli/provider_onboarding_test.go

---

Walkthrough

providers use now detects when ZERO_PROVIDER keeps a different provider effective after selection. JSON output exposes both providers, while text output emits a stderr warning. Environment access is injectable for hermetic tests and defaults to os.Getenv.

Changes

Provider override reporting

Layer / File(s)	Summary
Environment access wiring internal/cli/app.go	appDeps accepts an optional environment lookup function, with production defaults using os.Getenv.
Override detection and output internal/cli/provider_onboarding.go, internal/cli/provider_onboarding_test.go	providers use detects differing ZERO_PROVIDER values, reports overrides in text and JSON modes, and tests override, JSON, and no-warning cases.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant CLI
  participant runProvidersUse
  participant ZERO_PROVIDER
  participant config.json
  CLI->>runProvidersUse: select provider
  runProvidersUse->>config.json: save selected provider
  runProvidersUse->>ZERO_PROVIDER: read environment override
  ZERO_PROVIDER-->>runProvidersUse: effective provider
  runProvidersUse-->>CLI: success output with override details

Loading

Possibly related PRs

• Gitlawb/zero#725: Modifies providers use to detect ZERO_PROVIDER overrides and adjust output.
• Gitlawb/zero#348: Updates and consumes the active-provider environment override.
• Gitlawb/zero#716: Adjusts providers use behavior for environment-driven provider selection.

Suggested reviewers: pierrunoyt, gnanam1990, kevincodex1

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly states the main CLI change: warning when ZERO_PROVIDER overrides a providers-use selection.
Linked Issues check	✅ Passed	The PR implements the requested warning and saved-versus-effective provider reporting for issue #721.
Out of Scope Changes check	✅ Passed	The changes stay focused on the ZERO_PROVIDER override behavior and related tests, with no obvious unrelated scope.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/721-providers-use-env-override

---

_{Comment @coderabbitai help to get the list of available commands.}

[github-actions]

Zero automated PR review

Verdict: No blockers found

Blockers

• None found.

Validation

• [pass] Diff hygiene: git diff --check
• [pass] Tests: go test ./...
• [pass] Build: go run ./cmd/zero-release build
• [pass] Smoke build: go run ./cmd/zero-release smoke

Scope

Head: 3c51e7937785
Changed files (3): internal/cli/app.go, internal/cli/provider_onboarding.go, internal/cli/provider_onboarding_test.go

This deterministic review checks validation status and basic diff hygiene. A human reviewer still owns product judgment and design quality.

[gnanam1990]

APPROVE. The premise checks out, the dependency plumbing follows the file's existing convention, and both output modes are covered.

The load-bearing claim is correct

The warning is only worth printing if ZERO_PROVIDER really does beat the config write. It does, unconditionally — applyEnv at internal/config/resolver.go:560-563:

activeProvider := strings.TrimSpace(envValue(env, ActiveProviderEnv))
if activeProvider != "" {
    cfg.ActiveProvider = activeProvider
}

No precedence check, no merge — the env value simply replaces whatever providers use just persisted. So reporting the write as a bare success genuinely does read as a switch that silently has no effect, which is #721.

strings.EqualFold for the "is it actually different" test is the right comparison rather than a stricter one: provider-name resolution is case-insensitive elsewhere in the resolver (resolver.go:467), so ZERO_PROVIDER=OpenAI against a saved openai resolves to the same profile and correctly produces no warning. A case-sensitive check would have emitted a spurious warning there.

The dependency-injection shape is right

getenv follows the same pattern as the existing exportActiveProvider directly above it — set in defaultAppDeps(), deliberately not filled by fillAppDeps. I checked both ends of that:

• Production is fine: Run() (app.go:116) passes defaultAppDeps(), which sets getenv: os.Getenv, so the warning actually fires for real users. Worth confirming explicitly, since a dep that fillAppDeps skips could easily have ended up nil on the live path and made this a silent no-op.
• Tests stay hermetic: fillAppDeps leaves it nil, and activeProviderEnvOverride returns "" for a nil getenv, so an ambient ZERO_PROVIDER cannot make the new warning appear in unrelated tests.

Handling the JSON path separately with effectiveProvider / overriddenByEnv rather than only writing to stderr is the right call — a script consuming --json would otherwise have no way to see the override.

Unrelated observation, since it's adjacent to this PR's subject

internal/cli is not hermetic against an ambient ZERO_PROVIDER today, through a different path than the one this PR adds. With ZERO_PROVIDER=leaky exported, TestRunProvidersUseEnvDerivedJSONIncludesConfigPath fails on current main:

exit code = 3, want 0: [zero] no active provider configured: active provider "leaky" not found

That comes from resolveConfig reading the real process environment, not from anything here — I verified it reproduces identically on upstream/main without this branch. Flagging it only because you're the person currently in this code: since zero itself exports ZERO_PROVIDER via SetActiveProviderEnv, a developer can plausibly end up with it set and then see a confusing local test failure. A t.Setenv(config.ActiveProviderEnv, "") in the affected tests would close it. Separate PR, not this one.

Verification

gofmt and go vet ./internal/cli/ clean. All three new tests pass, including the two-case NoWarnWithoutEnvOverride (env unset / env matches). Full internal/cli suite compared against current main: identical five failures on both (TestExecScopeReRegistration…, TestRunAddDirDispatch…, both TestRunDoctor…, TestRunSandboxCheckJSON…) — all sandbox/network artifacts of my environment, and this PR introduces none.

Merge is kevin's call per the program gate.

[Vasanthdev2004]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[anandh8x]

Approved on the latest head. providers use now reports a conflicting ZERO_PROVIDER override in both text and JSON output while preserving the saved selection. Focused tests and all checks pass.