Clarify saved and effective provider selection

[PierrunoYT]

Summary

• report when ZERO_PROVIDER overrides a provider saved by providers use
• include the effective provider and override source in JSON output
• mark provider list entries as user-config/selectable or runtime-only
• explain why runtime-only providers cannot be selected with providers use

Validation

• go test ./...
• go vet ./...
• govulncheck ./...
• golangci-lint against new changes: 0 issues

Repository-wide golangci-lint still reports existing findings unrelated to this change.

Fixes #721

Summary by CodeRabbit

• New Features
  • providers list now labels profiles with case-sensitive selectable status and source (user-config vs resolved) in both text and --json.
• Bug Fixes
  • Provider identity and credential/key operations are now case-sensitive (with trimmed matching), preventing ambiguous case-only duplicates.
  • OAuth/provider flows now re-validate configuration immediately before saving tokens, improving correctness when config is changed during login.
  • Env-override behavior now reports selectable/deferred/resolved outcomes more accurately.
• Tests
  • Added/expanded coverage for selectability/source labeling, case-sensitivity, env-override labeling, and OAuth revalidation/preflight.

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

Walkthrough

Provider listings now distinguish persisted and runtime-resolved profiles. Provider selection reports environment override resolution, while configuration mutations require exact persisted names and reject ambiguous duplicates. Authentication, setup, and TUI flows preflight configuration before persistence.

Changes

Provider configuration and selection

Layer / File(s)	Summary
Provider identity validation and mutation internal/config/writer.go, internal/config/credentials.go, internal/config/*_test.go	Persisted provider names are validated for case-insensitive duplicates, while mutation targets use trimmed exact matching.
Active provider resolution internal/config/resolver.go, internal/config/resolver_test.go	Active-provider resolution prefers exact matches, supports unique case-insensitive fallback, and rejects ambiguity.

Provider CLI behavior

Layer / File(s)	Summary
Provider listing metadata internal/cli/command_center.go, internal/cli/command_center_test.go	Provider listings expose selectable and source metadata in JSON and text output.
Provider selection override handling internal/cli/provider_onboarding.go, internal/cli/provider_onboarding_test.go, internal/config/paths.go	providers use reports resolved, unresolved, and deferred environment overrides and rejects non-selectable or mismatched identities.

Authentication and persistence preflight

Layer / File(s)	Summary
OAuth save validation internal/cli/app.go, internal/cli/auth.go, internal/cli/auth_test.go, internal/oauth/manager.go	OAuth flows support before-save configuration validation, preventing token writes when configuration becomes invalid.
Provider write and TUI flow preflights internal/cli/provider_setup.go, internal/cli/setup.go, internal/tui/*	Provider setup, wizard, and device-login flows pass config paths and preflight configuration before persistence.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant ProvidersUse
  participant UserConfig
  participant Resolver
  ProvidersUse->>UserConfig: save requested provider
  ProvidersUse->>Resolver: resolve environment override
  Resolver-->>ProvidersUse: return resolved, unresolved, or deferred state
  ProvidersUse-->>ProvidersUse: report effectiveProvider and envProviderResolves

Loading

Possibly related PRs

• Gitlawb/zero#560: Overlaps provider listing rendering changes in internal/cli/command_center.go.

Suggested reviewers: gnanam1990, vasanthdev2004

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 39.13% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title matches the main change: clarifying saved versus effective provider selection.
Linked Issues check	✅ Passed	The PR adds source/selectability metadata, visual non-selectable markers, and better handling of override and identity cases required by #721.
Out of Scope Changes check	✅ Passed	The broader auth, setup, config, and TUI edits support the same provider-identity and selection behavior, so no clear unrelated scope stands out.

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[coderabbitai]

🧹 Nitpick comments (1)

internal/cli/command_center_test.go (1)

194-229: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Reset stderr alongside stdout for 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 confusing t.Fatalf error 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

📥 Commits

Reviewing files that changed from the base of the PR and between 60dc84e and 4b26069.

📒 Files selected for processing (4)

• internal/cli/command_center.go
• internal/cli/command_center_test.go
• internal/cli/provider_onboarding.go
• internal/cli/provider_onboarding_test.go

[Copilot]

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 use to report when ZERO_PROVIDER overrides the newly saved active provider, and include effective/override metadata in JSON output.
• Enhance providers list/current output (human + JSON) with selectable and source metadata 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.

[kevincodex1]

please rebase. to main and fix conflicts

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P2] Rebase this stale, superseded branch before merging.
  internal/cli/provider_onboarding.go
  The current head is not descended from main and has content conflicts in both this file and its test. The resulting base-to-head comparison spans 227 files rather than the PR's four-file merge-base diff. Main already contains #716's runtime-only-provider handling and #767's fix for #721; resolving these conflicts by taking this branch would remove both. Please rebase onto current main, retain the existing fixes, and submit the resolved diff for review rather than merging this head.

• [P2] Do not report an unresolvable ZERO_PROVIDER value as the effective provider.
  internal/cli/provider_onboarding.go:64
  A nonempty ZERO_PROVIDER is reported as the effective provider without checking that it resolves. For example, a stale ZERO_PROVIDER=runtime when no runtime profile exists prints Effective provider: runtime and suggests zero providers check runtime, but the next resolution fails before that check can run. Validate the override against the resolved provider list, or report it as invalid and direct the user to fix or unset it.

[Vasanthdev2004]

Heads-up @PierrunoYT: #767 merged and covers the core of this (the warning when ZERO_PROVIDER overrides a providers-use selection, #721). Your PR goes further though, and the extra parts are genuinely useful: the JSON effective-provider/override-source fields and the user-config-vs-runtime-only list marking are not in #767, so this is not fully superseded.

To carry it forward it would need a rebase onto main (post-#767) and a trim down to just the net-new parts (the JSON output, the list marking, and the runtime-only explanation), since the warning itself is now landed. Happy to review that, or if you would rather, fold those into a fresh focused PR. Let us know which you prefer.

[PierrunoYT]

Merged current main (dcf1a40) — the PR is MERGEABLE again — and addressed both findings plus the two bot comments.

[P2] Rebase this stale, superseded branch — done, and resolved away from this branch where main already had the behaviour. main's #767 covers the providers use override reporting with a cleaner design (the env read is injected through deps.getenv rather than a direct os.Getenv, so tests stay hermetic against an ambient ZERO_PROVIDER), and #716/#707 cover runtime-only provider handling. I kept all of that and dropped this branch's duplicate implementation and its two duplicate override tests, so the branch now adds only what main lacks:

• providers list / providers current: per-entry selectable + source in JSON, and a (not selectable via providers use) marker in the human output.
• providers use: an actionable error when the requested name is resolvable at runtime but cannot be saved (only providers saved in user config are selectable …).

Net diff against main is now 4 files / ~220 lines instead of the 227-file base-to-head comparison.

[P2] Do not report an unresolvable ZERO_PROVIDER value as the effective provider — fixed. New activeProviderEnvOverrideResolves checks, in order, the saved profile in the config file the command just wrote (so the check does not depend on ambient env) and then the resolver for env-synthesized profiles. When neither matches:

• the text note now reads "… but no provider named X can be resolved, so Zero cannot start until you unset ZERO_PROVIDER or point it at a saved provider" instead of calling it effective and suggesting zero providers check X;
• the JSON payload omits effectiveProvider entirely and reports overriddenByEnv, envProvider, and envProviderResolves: false.

TestRunProvidersUseFlagsUnresolvableEnvOverride covers both paths; main's resolvable-override tests still pass unchanged (their ZERO_PROVIDER=work is a saved profile, so it resolves and effectiveProvider is still reported).

@copilot: "runtime-only; not selectable/saved" over-claims for other persisted sources — valid, and it applies to the source value too, not just the prose. Project config and provider commands both contribute persisted profiles (mergeProjectConfig merges src.Providers), so those are not "runtime". The human marker is now (not selectable via providers use) — the only guaranteed claim — and the JSON source enum is user-config | resolved, documented next to the constants. The companion test comment about hard-coded wording is handled by the same rename.

@coderabbitai: reset stderr alongside stdout — already applied in fef6c5d; the second runWithDeps in TestRunProvidersListMarksUserAndRuntimeProfiles resets both buffers, and the new override test does the same.

Validation (Windows host, Go 1.26.5): go build ./..., go vet ./..., go test ./... -count=1 all pass; GOOS=linux|darwin|windows builds clean; gofmt clean on every changed file.

@jatmn @Vasanthdev2004 — the resolved diff is ready for another look; I can't use the reviewer-request button on this repo, hence the mention.

@coderabbitai review

[coderabbitai]

@PierrunoYT: Thanks for the detailed rebase and resolution notes. I’ll review the updated, reduced diff with particular attention to provider-source semantics, environment-override UX, and the new test coverage.

---

✏️ Learnings added

Learnt from: PierrunoYT
URL: https://github.com/Gitlawb/zero/pull/725

Timestamp: 2026-07-25T11:35:15.664Z
Learning: In this Go CLI, provider profiles may come from user config, project config, or provider commands; profiles resolved at runtime are not necessarily transient. For provider list/current output, `selectable` indicates whether `providers use` can persist the profile, and the documented JSON `source` values are `user-config` and `resolved`.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ 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.

[coderabbitai]

🧹 Nitpick comments (2)

internal/cli/provider_onboarding_test.go (1)

163-213: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Add a companion test for the resolver-fallback-resolves-true case.

This test nicely covers the fully-unresolvable override. Consider adding a sibling test where ZERO_PROVIDER names a provider that is not persisted but is resolvable via resolveCommandCenterConfig (e.g. set an ambient OPENAI_API_KEY and point the override at the env-derived provider name while switching to a different saved profile). That exercises the second branch of activeProviderEnvOverrideResolves (lines 148-156 in provider_onboarding.go), which currently only sees coverage via the persisted-check branch and the fully-unresolvable branch.

As per path instructions, "**/*_test.go: ... add regression tests for behavior changes."

🧪 Suggested additional test sketch

func TestRunProvidersUseFlagsResolvableEnvOverrideViaResolver(t *testing.T) {
	t.Setenv("OPENAI_API_KEY", "sk-env")
	configPath := providersUseOverrideConfig(t)
	deps := providerSetupDeps(configPath)
	deps.getenv = func(key string) string {
		switch key {
		case config.ActiveProviderEnv:
			return "openai"
		case "OPENAI_API_KEY":
			return "sk-env"
		default:
			return ""
		}
	}

	var stdout, stderr bytes.Buffer
	if code := runWithDeps([]string{"providers", "use", "fast", "--json"}, &stdout, &stderr, deps); code != exitSuccess {
		t.Fatalf("exit = %d, want %d: %s", code, exitSuccess, stderr.String())
	}
	var payload map[string]any
	if err := json.Unmarshal(stdout.Bytes(), &payload); err != nil {
		t.Fatalf("decode JSON: %v\n%s", err, stdout.String())
	}
	if payload["effectiveProvider"] != "openai" {
		t.Fatalf("expected env-derived override to resolve via resolver fallback, got %#v", payload)
	}
}

🤖 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/provider_onboarding_test.go` around lines 163 - 213, Add a
sibling regression test next to TestRunProvidersUseFlagsUnresolvableEnvOverride
that covers an environment override naming a provider absent from persisted
profiles but resolvable by resolveCommandCenterConfig, such as “openai” with an
injected OPENAI_API_KEY. Invoke providers use with --json and assert successful
execution reports that provider as effectiveProvider, exercising the
resolver-fallback branch of activeProviderEnvOverrideResolves while retaining
the existing unresolvable test.

Source: Path instructions

internal/cli/provider_onboarding.go (1)

137-158: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Resolver-fallback branch of activeProviderEnvOverrideResolves is untested.

This function has two independent resolution paths: the persisted-config check (line 149) and the resolver-fallback via resolveCommandCenterConfig/providerResolvedByName (lines 152-156). Only the persisted-true case ("work") and the fully-unresolvable case ("removed-profile") are covered by tests; the case where ZERO_PROVIDER names an env-derived-but-unpersisted provider (e.g. an ambient OPENAI_API_KEY-derived profile) is not tested, leaving one of the two logical branches unverified.

See companion comment on internal/cli/provider_onboarding_test.go for the suggested regression test.

🤖 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/provider_onboarding.go` around lines 137 - 158, Add a regression
test for activeProviderEnvOverrideResolves covering an env-derived provider that
is not persisted, such as one synthesized from an ambient OPENAI_API_KEY.
Configure the test so the persisted-profile check does not match,
resolveCommandCenterConfig succeeds, and providerResolvedByName finds the
override, asserting the function returns true while preserving existing
persisted and unresolvable cases.

🤖 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/provider_onboarding_test.go`:
- Around line 163-213: Add a sibling regression test next to
TestRunProvidersUseFlagsUnresolvableEnvOverride that covers an environment
override naming a provider absent from persisted profiles but resolvable by
resolveCommandCenterConfig, such as “openai” with an injected OPENAI_API_KEY.
Invoke providers use with --json and assert successful execution reports that
provider as effectiveProvider, exercising the resolver-fallback branch of
activeProviderEnvOverrideResolves while retaining the existing unresolvable
test.

In `@internal/cli/provider_onboarding.go`:
- Around line 137-158: Add a regression test for
activeProviderEnvOverrideResolves covering an env-derived provider that is not
persisted, such as one synthesized from an ambient OPENAI_API_KEY. Configure the
test so the persisted-profile check does not match, resolveCommandCenterConfig
succeeds, and providerResolvedByName finds the override, asserting the function
returns true while preserving existing persisted and unresolvable cases.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: f848d4aa-1f56-4478-ba84-ea66efece4ee

📥 Commits

Reviewing files that changed from the base of the PR and between fef6c5d and dcf1a40.

📒 Files selected for processing (4)

• internal/cli/command_center.go
• internal/cli/command_center_test.go
• internal/cli/provider_onboarding.go
• internal/cli/provider_onboarding_test.go

🚧 Files skipped from review as they are similar to previous changes (2)

• internal/cli/command_center_test.go
• internal/cli/command_center.go

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P2] Determine the effective override from the resolved active profile
  internal/cli/provider_onboarding.go:149
  A persisted row, or merely a matching entry in resolved.Providers, is not proof that ZERO_PROVIDER is the provider a subsequent invocation will use. For example, a saved OpenAI-compatible profile without a model passes the persisted fast path, so providers use fast --json reports envProviderResolves: true and effectiveProvider: "broken"; the next resolution instead fails because the active broken profile requires a model. Likewise, a provider command can leave the env-named profile in the list while setting a different active provider. Run the resolver for every override and require the successfully resolved ActiveProvider to match the override before emitting the effective-provider fields/note.

• [P3] Do not case-fold provider identities when labeling individual list entries selectable
  internal/cli/command_center.go:154
  Resolution merges provider names case-sensitively, but this map collapses work and WORK. If user config has work and project config (or a provider command) contributes WORK, both resolved entries are labeled selectable: true, source: "user-config". providers use WORK only updates the user-config work row, so it cannot select the displayed WORK entry. Preserve the concrete persisted identity when deriving the metadata (or explicitly reject/handle case-only collisions) and add a regression test.

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@internal/cli/command_center_test.go`:
- Around line 271-276: Update the assertions in the provider cases of the test
to compare Source directly against the documented JSON strings "user-config" and
"resolved" instead of production constants, while preserving the existing
Selectable checks and failure messages.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: aa142389-f1f1-48db-8ae1-a5138ccf7608

📥 Commits

Reviewing files that changed from the base of the PR and between dcf1a40 and 9fd27b8.

📒 Files selected for processing (4)

• internal/cli/command_center.go
• internal/cli/command_center_test.go
• internal/cli/provider_onboarding.go
• internal/cli/provider_onboarding_test.go

🚧 Files skipped from review as they are similar to previous changes (3)

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

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P2] Avoid executing the provider command just to render the override note
  internal/cli/provider_onboarding.go:150
activeProviderEnvOverrideResolves now performs a full config resolution after providers use has already written config.json. When ZERO_PROVIDER_COMMAND is configured, that resolution runs the configured shell command (and can wait up to five seconds); its side effects or failure are then discarded and reported as an unresolved ZERO_PROVIDER. This makes a previously config-only selection unexpectedly execute an external command after committing the change. Determine the override status without loading the provider command, or surface and handle that resolution failure before claiming a successful selection.

• [P2] Resolve the case-variant provider identity ambiguity in list selectability
  internal/cli/command_center.go:160
  The new exact-case lookup correctly treats a resolved WORK profile as distinct from persisted work, but ProviderPersisted and SetActiveProvider still match case-insensitively. Copying the displayed WORK into zero providers use WORK therefore exits successfully while silently activating the different persisted work profile. Case-only persisted duplicates have the inverse problem: both are reported selectable although the mutator always selects the first match. Align the mutation and listing identity rules (or reject ambiguous case variants) so a selected list entry cannot resolve to a different provider.

[PierrunoYT]

Addressed the Windows CI failure in commit 4749a7b.

Root cause: auth logout resolved the user config path but deleted API keys from the OS-default credential store. The test's XDG_CONFIG_HOME happened to align those paths on Unix, while Windows uses %AppData%, so the API key survived. Logout now opens the credential store beside the resolved config path, matching key creation/runtime lookup.

Validation:

• go test ./internal/cli -run 'TestRunAuthLogout(CleansCredentialsWhenConfigIsAmbiguous|ResolvesCatalogIdentity)' -count=1 -v
• go test ./...
• go vet ./...
• go run ./cmd/zero-release build
• go run ./cmd/zero-release smoke
• gofmt -l internal/cli/auth.go (clean)
• git diff HEAD^ --check

The pinned standalone lint/vulnerability tools could not be rerun locally because this Windows Go launcher uses a Go 1.24.3 base toolchain for go run package@version, while the repository targets Go 1.26.5. Both checks passed in the preceding CI run and will rerun on this push.

[jatmn]

I found issues that need to be addressed before this is ready.

The activeProvider hand-off regression from the prior round is fixed on a6bbf74 (TestProviderMutatorsHandOffCaseVariantActiveProvider). Logout also clears OAuth/API secrets before returning on invalid config, which is an improvement over main. The items below are verified on 4749a7b7; regressions introduced by this PR's case-identity and credential changes are separated from pre-existing gaps the branch touches but did not create.

Findings

• [P2] Delete the shared credential when removing the keyed case-duplicate row
  internal/cli/provider_onboarding.go:503, internal/tui/provider_manager.go:367
  PR regression. configContainsCaseVariantProvider is new here. It correctly keeps the key for remove WORK while work survives (TestRunProvidersRemoveCaseDuplicateKeepsSurvivorKey), but it also skips deletion when the keyed row is removed. On main, providers remove always called removeStoredProviderKeyAt; this PR added the skip. With {"providers":[{"name":"work","apiKeyStored":true},{"name":"WORK"}]}, zero providers remove work drops the keyed profile, leaves WORK without apiKeyStored, and keeps the encrypted key in the credstore. The survivor cannot load it (ApplyStoredAPIKey only runs when APIKeyStored is true). Please delete the key when the removed row owned apiKeyStored, or transfer the marker to the surviving case-variant before skipping deletion; mirror the same rule in the TUI cleanup path.

• [P2] Finish OAuth logout while you are hardening credential cleanup
  internal/cli/auth.go:503
  Pre-existing gap, PR-incomplete. manager.Logout has always deleted only the spelling the user typed. This PR correctly added dual keyStore.Delete for profile vs config row names, but OAuth still runs once with the raw argument. A profile saved as {name:"my-xai", catalogId:"xai"} with zero auth login xai stores the token under provider:xai; zero auth logout my-xai can delete API keys yet leave the OAuth token behind and still report success. The runtime already resolves candidates via ProviderProfile.OAuthLoginCandidates() / oauth.FirstStored (internal/tui/command_center.go:653). Please delete every stored OAuth candidate for the resolved profile, matching the API-key fix you just added.

Findings (lower priority)

• [P3] Clear apiKeyStored on every folded row during recovery logout
  internal/cli/auth.go:530
  PR regression. On main, logout had no config preflight and ClearProviderKeyStored matched case-insensitively, so recovery on ambiguous configs was less broken. This PR adds PreflightUserConfig, deletes secrets, then errors before ClearProviderKeyStored — which is now exact-match only. TestRunAuthLogoutCleansCredentialsWhenConfigIsAmbiguous documents the result: credentials gone, DEMO still has apiKeyStored:true, config unchanged. Either clear the marker on every case-variant row whose key was deleted, or say explicitly in the error that the marker must be edited by hand.

• [P3] Clear apiKeyStored on every folded row when removing a stored key in the TUI
  internal/tui/provider_wizard.go:1382
  PR regression. applyManageKeyChoice deletes the folded credstore entry but clears the marker only for the exact spelling chosen. On main, ClearProviderKeyStored was case-insensitive, so this path stayed consistent. After this PR's exact-marker rule, removing the key for WORK while work owns apiKeyStored:true deletes the shared secret yet leaves the marker on work. Apply the same case-variant awareness you added to providers remove and provider delete.

• [P3] Match in-memory provider list updates to exact-case deletes
  internal/tui/provider_manager.go:379, internal/tui/provider_manager.go:398
  Pre-existing drift; more visible after this PR. removeSavedProvider still uses EqualFold and was not changed here. deleteManagerSelection now calls exact-case RemoveProvider, so deleting a resolved-only WORK row can evict persisted work from the manager UI while it remains in config.json. Worth fixing for consistency with the exact-identity work in this PR, but not a regression from it.

Needs maintainer decision @Vasanthdev2004 @gnanam1990

• Ambiguous case-duplicate user config remains a hard startup failure with only CLI exact-name removal as repair
  internal/config/resolver.go:77, internal/config/writer.go:423
  Intentional new policy in this PR, not a defect. Blocking work/WORK at Resolve() is deliberate, and RemoveProvider can delete one exact duplicate without pre-validation. That is enough for zero providers remove WORK from a shell, but not from the TUI while Zero cannot resolve config. Please confirm this one-time manual-JSON migration is acceptable, or add a documented repair path that does not require a successful resolve first.

[PierrunoYT]

Addressed the review round on 4749a7b in 0479e9f:

• P2 — shared credential orphaned on case-variant remove: providers remove / TUI delete now transfer the apiKeyStored marker to the surviving case-variant row when the removed row owned it, instead of silently orphaning the still-shared credential-store secret. Added TestRunProvidersRemoveTransfersKeyMarkerToSurvivingCaseVariant and the TUI mirror TestProviderManagerDeleteTransfersKeyMarkerToSurvivingCaseVariant.
• P2 — OAuth logout incomplete for catalog-id logins: auth logout now deletes every login candidate (typed name, canonical persisted name, catalog id), not just the literal spelling typed. Added TestRunAuthLogoutDeletesCatalogIDToken covering the {name:"my-xai", catalogId:"xai"} repro.
• P3 — marker not cleared on every case-variant row: auth logout and the TUI "remove stored key" path now use a new config.ClearProviderKeyStoredCaseVariants (case-fold match) instead of exact-name clearing, and the ambiguous-config early-return in auth logout now says explicitly that a stale marker must be corrected by hand alongside the persisted-name conflict (writes are blocked entirely by ValidatePersistedProviderNames until that's fixed, so this can't be resolved automatically).
• P3 — in-memory list eviction mismatch: removeSavedProvider now uses exact-name matching to agree with RemoveProvider's exact identity, so deleting one case-variant row can no longer evict its surviving sibling from the manager UI list.

Left the "needs maintainer decision" repair-path item as-is — that's a policy call, not something to resolve unilaterally.

Validation: go build ./..., go vet ./..., go test ./... (full suite, all packages pass).

Requesting re-review.

[jatmn]

I found issues that need to be addressed before this is ready.

The 0479e9f follow-up addressed the prior round's keyed case-variant removal, multi-candidate OAuth logout (when config validation passes), and removeSavedProvider exact-identity alignment. The items below are verified on 0479e9f667fa4aff4cb3a7e002e2b7e65ebc6423 against d37de9214b1ba86542f4f5ef3b060bb96849763b. Regressions introduced or left incomplete by this branch are separated from pre-existing gaps and polish.

Findings

• [P2] Sync in-memory savedProviders after transferring apiKeyStored on case-variant delete
  internal/tui/provider_manager.go:373
deleteManagerSelection calls TransferProviderAPIKeyStoredMarker on disk when removing the keyed case-variant row, but only runs removeSavedProvider on the in-memory list. reloadProviderManagerRows rebuilds from savedProviders, so the survivor keeps APIKeyStored: false in memory while config.json has the marker. providerManagerCredState gates store lookups on profile.APIKeyStored, so the manager shows no credential until restart even though disk and the credstore are correct. TestProviderManagerDeleteTransfersKeyMarkerToSurvivingCaseVariant checks persisted config only, not the manager UI. Please patch the survivor in savedProviders (or reload that row from config) after a successful marker transfer.

• [P2] Delete API keys under every OAuth login candidate during logout
  internal/cli/auth.go:547
  This PR's logout hardening deletes OAuth tokens for profile name, canonical persisted name, and catalog id, but API-key deletion still only calls keyStore.Delete for the user-typed spelling and configProvider. A key stored under the catalog id (for example xai while the saved row is my-xai) survives zero auth logout my-xai. main only deleted the typed spelling once; this branch improved OAuth coverage but left the same asymmetry for API keys. Please delete credstore entries for the same candidate set used for OAuth logout.

• [P3] Resolve OAuth logout candidates even when unrelated config validation fails
  internal/cli/auth.go:496
runAuthLogout gates PersistedProviderIdentity, ProviderRow, and catalog-id candidate expansion on configErr == nil from PreflightUserConfig, even though those reads do not require a valid config — only writeConfigFile does. With an unrelated ambiguous file (for example demo/DEMO) and a profile {name:"my-xai", catalogId:"xai"} whose token was stored under xai, zero auth logout my-xai deletes only provider:my-xai and leaves the catalog-id token behind. This is a narrow recovery edge case in the new logout path (credentials are deleted before the marker write fails), but the candidate list should not depend on preflight passing. Please resolve identity and expand OAuth candidates regardless of validation, and gate only the marker-clearing write on configErr.

Findings (lower priority)

• [P3] Apply OAuth candidate resolution to auth refresh and auth status
internal/cli/auth.go:652, internal/cli/auth.go:687
runAuthRefresh / runAuthRefreshWatch and filterAuthStatuses still look up only oauth.ProviderKey(provider) with the raw argument. After zero auth login xai on {name:"codex", catalogId:"chatgpt"}, zero auth refresh codex and zero auth status codex report no token even though oauthLoginForProfile and the TUI gate would find the catalog-id login. This gap predates this branch (main behaved the same); this PR hardened logout but did not extend the same semantics here. Worth fixing for auth-CLI parity, but not a regression from this diff.

• [P3] Finish the exact-identity migration for in-memory provider list updates
  internal/tui/provider_manager.go:704, internal/tui/provider_wizard.go:736
  This PR moved removeSavedProvider to exact-name matching, but applySavedProviderEdit and upsertSavedProviderProfile still use EqualFold and update the first fold-equal entry — the same pre-existing drift noted in the prior review round. It only bites when savedProviders contains case-variant siblings (uncommon). Align these helpers with the exact-identity rule when convenient; not introduced by this PR.

• [P3] Include source in text providers list output, or narrow the claim
  internal/cli/command_center.go:441
  JSON list output and providers current text expose source (user-config vs resolved), but providers list text only appends (not selectable via providers use). This is claims/output drift, not a runtime failure. Please print source in list text as well, or update the PR summary to say JSON/providers current only.

Needs maintainer decision @Vasanthdev2004 @gnanam1990

• Ambiguous case-duplicate user config remains a hard startup failure with only CLI exact-name removal as repair
  internal/config/resolver.go:77, internal/config/writer.go:423
  Blocking work/WORK duplicates at Resolve() is deliberate in this PR, and RemoveProvider can delete one exact duplicate without pre-validation. That is enough for zero providers remove WORK from a shell, but not from the TUI while Zero cannot resolve config. Please confirm this one-time manual-JSON migration is acceptable, or add a documented repair path that does not require a successful resolve first.

[PierrunoYT]

Addressed the review round on 0479e9f in 780a219:

• P2 — TUI in-memory savedProviders not synced after marker transfer: deleteManagerSelection now mirrors the on-disk apiKeyStored transfer into m.savedProviders, so the manager list no longer shows the survivor as no credential until restart. Extended TestProviderManagerDeleteTransfersKeyMarkerToSurvivingCaseVariant to assert the in-memory state too.
• P2 — API key deletion missing catalog-id candidate on logout: auth logout now deletes credstore entries for the same candidate set used for OAuth (typed name, canonical persisted name, catalog id), not just the first two. Added TestRunAuthLogoutDeletesCatalogIDAPIKey.
• P3 — candidate resolution gated on unrelated config validity: PersistedProviderIdentity/ProviderRow calls no longer wait on PreflightUserConfig succeeding (they only read+parse raw JSON and never validate case-duplicate names) — only the final marker-clearing write still gates on configErr. Added TestRunAuthLogoutResolvesCandidatesDespiteUnrelatedAmbiguousConfig covering an unrelated demo/DEMO ambiguity alongside an unambiguous catalog-id profile being logged out.
• P3 (lower) — providers list text missing source: added source: <user-config|resolved> next to the not-selectable marker in text output, matching JSON and providers current.

Left the other two lower-priority P3s as-is since they're called out as pre-existing, not introduced by this branch: auth refresh/auth status OAuth candidate parity, and applySavedProviderEdit/upsertSavedProviderProfile still using EqualFold. Happy to take those on if you'd like them folded in too. Also left the maintainer-decision item untouched.

Validation: go build ./..., go vet ./..., full go test ./... (all packages pass).

Requesting re-review.

[jatmn]

I found issues that need to be addressed before this is ready.

The branch is mergeable and all required GitHub checks are green, but GitHub still shows the pull request as blocked on my earlier CHANGES_REQUESTED review from 0479e9f. Please dismiss or supersede that review after the items below are fixed.

The 780a219 follow-up addressed the prior round's in-memory apiKeyStored sync after case-variant delete, multi-candidate OAuth/API-key deletion on auth logout (including catalog-id keys), and logout candidate expansion when unrelated config validation fails. The items below are verified on 780a21968ca0ed9b5c5dc92e3390fa37e5661bad against merge-base 81a5e6cb716ba4b7ecf7ff3ff261dfa51fd01098.

Findings

• [P3] Fix the case-variant providers remove warning when marker transfer fails
  internal/cli/provider_onboarding.go:544
  When a case-variant survivor intentionally retains the shared credstore secret, keyErr from TransferProviderAPIKeyStoredMarker is reported as “could not be deleted and remains in the credential store.” That describes the wrong operation — deletion was skipped on purpose. Please distinguish marker-transfer failure from delete failure so users know the shared key may be unreachable on the survivor.

• [P3] Surface first-run setup failures from persistOAuthLoginProvider
internal/tui/onboarding.go:665
  After token OAuth during setup, _ = persistOAuthLoginProvider(...) is ignored while the wizard path stops with oauthErr when the same call fails. This pattern predates the branch, but the new ValidatePersistedProviderNames guards make profile creation fail more often after the oauth store already holds a token. Please match applyProviderWizardOAuth: set setup.oauthErr, stay on the provider step, and do not advance when persist fails.

• [P3] Give TUI OpenRouter the same save-failure recovery as CLI OpenRouter
  internal/tui/onboarding.go (setupOAuthCmd), internal/tui/provider_wizard.go (providerWizardOAuthCmdFor)
  runAuthOpenRouter now prints the minted key and exits non-zero when persistence fails (TestRunAuthOpenRouterFailsWhenTheKeyCannotBeSaved). TUI OpenRouter still holds the key only in memory through later persist with no echo on failure. This is a real parity gap within the same PR — please print the minted key (or equivalent recovery instructions) before returning a save error, same as CLI.

• [P3] Preflight config before device-code OAuth phase 1 in the TUI
  internal/tui/onboarding.go:578, internal/tui/provider_wizard.go:330
  This PR added config preflight before device OAuth phase 2 and before CLI interactive auth, but phase 1 still requests a device code with no check. With ambiguous case-duplicate config.json, the user can complete external authorization and only then fail on poll/BeforeSave. Please preflight in the prepare commands before PrepareDeviceLogin.

Findings (lower priority)

• [P3] Extend OAuth candidate resolution to auth refresh and auth status
internal/cli/auth.go:652, internal/cli/auth.go:622
runAuthLogout now mirrors OAuthLoginCandidates, but refresh/status still look up only oauth.ProviderKey(provider) from the raw argument. This gap predates the branch; logout hardening makes the asymmetry more visible. Worth aligning when convenient, not a regression from this diff.

• [P3] Expose selectable / source on zero config outputs, or narrow the PR claim
  internal/cli/command_center.go (formatConfigSummary, runConfig JSON)
  The PR implements the distinction on zero providers list|current, not zero config. If the summary is meant to cover all provider listings, please extend zero config; otherwise narrow the PR description.

Needs maintainer decision @Vasanthdev2004 @gnanam1990

• Ambiguous case-duplicate user config is now a hard Resolve() failure with CLI-only repair
  internal/config/resolver.go:77, internal/config/writer.go:423
  Blocking work/WORK duplicates at Resolve() is deliberate, and RemoveProvider can delete one exact duplicate without pre-validation. That is enough for zero providers remove WORK from a shell, but not from the TUI while Zero cannot resolve config. Please confirm this one-time manual-JSON migration is acceptable, or add a documented repair path that does not require a successful resolve first.

[PierrunoYT]

PierrunoYT addressed the four current review findings in 64f186f.

• providers remove now distinguishes an apiKeyStored marker-transfer failure from a credential deletion failure, warning that the shared key may be unreachable on the surviving case-variant row instead of falsely claiming deletion was attempted.
• First-run token OAuth no longer ignores persistOAuthLoginProvider failures: setup stays on the provider step and surfaces the save error rather than advancing with an unreachable stored token.
• First-run and provider-wizard OpenRouter flows now show the minted key with explicit recovery guidance when the later provider save fails, matching the CLI's no-key-loss behavior.
• Both TUI device-code prepare commands now preflight user config before requesting a device code, so invalid/ambiguous config fails before external authorization starts.

Added focused regressions for each TUI behavior. Validation passed:

• make fmt-check
• focused internal/tui / internal/cli tests
• go vet ./...
• go test ./...
• go run ./cmd/zero-release build
• go run ./cmd/zero-release smoke
• make lint-static (0 issues)
• make vulncheck (no vulnerabilities)
• git diff HEAD --check

The explicitly pre-existing refresh/status and zero config parity notes remain outside this focused follow-up, as does the maintainer-policy question on ambiguous-config repair.

@jatmn @Vasanthdev2004 — PierrunoYT requests a fresh review of the current head.

[jatmn]

Findings
[P1] Rebase this branch onto the current PR base before merging
.release-please-manifest.json:1
The supplied base is 8e26679, but its merge-base with this head is 81a5e6c, so this head does not contain the current mainline changes. Consequently the actual PR diff rolls the release metadata and package version back from 0.6.0 to 0.5.0, deletes the 0.6.0 changelog entry, and reverts 7,104 lines across 131 files, including ACP cancellation, sandbox, provider-retry, and swarm fixes that are unrelated to #721. Please rebase (or merge and resolve) the current base while preserving its changes, then submit the resulting focused diff for a fresh review.

[P2] Do not hide a case-distinct environment override
internal/cli/provider_onboarding.go:161
activeProviderEnvOverride treats ZERO_PROVIDER=WORK as identical to a newly saved user provider named work. That is not generally true: a workspace config (or provider-command result) can supply a separate exact-case WORK profile, and resolution selects that exact row. providers use work then omits both the warning and the JSON override fields even though subsequent runs use WORK and potentially different credentials or endpoint. Compare the resolved identity/source (or at least require exact equality before suppressing the override) so the command reports the provider that will actually be used.

[P1] Scope logout cleanup to the selected profile's credential identities
internal/cli/auth.go:513
PersistedProviderIdentity can select the first row with a matching catalog ID instead of a later exact-name row, and logout then uses that row's catalog ID as both an OAuth and API-key delete alias. Catalog IDs are shared by valid profiles: a stored-key work-xai, stored-key xai, and keyless personal-xai can all use catalogId:"xai". Logging out work-xai consequently deletes the shared xai OAuth token and the xai profile's API key, while only work-xai's marker is cleared. Resolve the exact target before catalog fallback, derive OAuth candidates only when that target can use them, and delete API keys only by proven profile ownership.

[P2] Reject exact duplicate persisted provider names
internal/config/writer.go:24
The new validator only rejects a repeated folded name when its spelling differs, so two rows named work (including names that trim to the same value) pass validation. That violates the stated one-credential-per-folded-name invariant: resolver merging silently coalesces the rows, while plaintext-key migration writes both values to the same normalized credential-store entry and can overwrite the first key. Reject any repeated folded identity before resolving or migrating.

[P2] Fail providers remove when its credential-marker handoff fails
internal/cli/provider_onboarding.go:503
In the case-variant path the selected row is removed before TransferProviderAPIKeyStoredMarker runs. If that write fails, the command reports a warning (or keyError in JSON) but still returns success, even though the surviving profile is left unmarked and cannot reach the shared stored key. Return a nonzero result for this partial failure, or make the remove/marker transfer transactional so scripts do not treat a broken credential handoff as complete.

[P1] Do not adopt an arbitrary profile sharing the catalog ID
internal/config/writer.go:89
AdoptPersistedCatalogProviderName calls PersistedProviderIdentity(catalogID), which returns the first row sharing that catalog ID. Multiple profiles may legitimately use one catalog provider: with an existing {name:"work-xai", catalogId:"xai"}, adding the default xai profile is silently retargeted and upserts into work-xai, replacing its endpoint/model and potentially its stored key. Restrict adoption to a uniquely proven case-variant default-name row; preserving an explicitly supplied default spelling is necessary too.

[P2] Use credential-store normalization to detect a key-sharing survivor
internal/cli/provider_onboarding.go:589
Survivor detection uses strings.EqualFold, whereas the credential store normalizes names with trimmed strings.ToLower. Those equivalence relations differ: s and long-s ſ pass persisted-name validation as distinct credential-store identities but compare equal here. Removing keyed s then retains its orphaned key and transfers the marker to ſ, whose credential-store lookup uses a different key. Compare the same trimmed lowercase representation used by credstore.normalizeProvider in both CLI and TUI removal paths.

[PierrunoYT]

Thanks @jatmn — all seven findings are addressed on f1f2b663.

[P1] Rebase onto the current base — merged upstream/main (8e266797) into the branch. The PR diff is now 25 files / +2458 −94, scoped to provider identity and credentials; the release-metadata rollback and the 131-file revert are gone.

[P1] Scope logout cleanup to the selected profile's credential identities — internal/cli/auth.go. Identity resolution moved to the new config.ResolvePersistedProviderIdentity, which prefers an exact name, then a case variant, then a catalog id — and refuses to guess when more than one row claims that catalog id. Candidate expansion adds the catalog id only when config.CatalogIdentityExclusive proves no other row claims it (by name or by catalogId), so it now gates both the OAuth and the API-key deletes. TestRunAuthLogoutLeavesSharedCatalogCredentialsAlone builds your exact three-profile config and asserts logout work-xai deletes only work-xai's key while the shared xai token and the xai profile's key survive; TestRunAuthLogoutPrefersTheExactlyNamedProfile covers the ordering half.

[P1] Do not adopt an arbitrary profile sharing the catalog ID — internal/config/writer.go. AdoptPersistedCatalogProviderName no longer calls PersistedProviderIdentity. It scans for rows whose NAME folds to the catalog's default name, returns unchanged when the exact spelling already exists (preserving an explicitly supplied default), and adopts only when exactly one case variant is found. A row that merely lists the same catalogId is left alone.

[P2] Reject exact duplicate persisted provider names — ValidatePersistedProviderNames now rejects any repeated folded identity, dropping the previous != name guard, and folds with credstore.NormalizeProvider so trimming is part of the identity. Distinct message for the duplicate case.

[P2] Do not hide a case-distinct environment override — internal/cli/provider_onboarding.go. activeProviderEnvOverride reports every spelling that is not literally the selection; the fold-suppression moved into activeProviderEnvOverrideSelectsSaved, which resolves (provider command excluded, as before) and suppresses only when the resolved active provider is exactly the row just written. TestRunProvidersUseCaseVariantEnvOverrideFollowsResolution covers both directions: no note when ZERO_PROVIDER=WORK resolves to the saved work, note when a project config contributes a separate exact-case WORK.

[P2] Fail providers remove when its credential-marker handoff fails — the command now returns nonzero in both text and --json modes, and the warning names the handoff rather than a deletion. I could not make it transactional: while both case-variant rows are present the config is ambiguous and writeConfigFile rejects every write, so removing one row is a precondition for the marker write, not something that can wrap it. That constraint is now a comment at the call site.

[P2] Use credential-store normalization to detect a key-sharing survivor — exported credstore.NormalizeProvider and switched both removal paths (survivingCaseVariantProviderName in the CLI, caseVariantProviderNameAfterRemoval in the TUI) to it. TestRunProvidersRemoveUsesCredentialStoreEquivalenceForSurvivors uses your s / ſ case: the key is deleted, keyRemoved is true, and ſ does not inherit a marker for an entry it cannot read.

Validation: go build ./..., go vet ./..., go test ./... all green.

On the maintainer question about ambiguous case-duplicate configs being a hard Resolve() failure with CLI-only repair — still deferred to @Vasanthdev2004 / @gnanam1990, unchanged from the previous rounds.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Use the credential-store identity when clearing stored-key markers
  internal/config/credentials.go:139
  This newly added helper compares names with strings.EqualFold, while the store itself uses credstore.NormalizeProvider (TrimSpace + ToLower). Those relations differ: s and Unicode long-s (ſ) are valid distinct provider/store identities, but EqualFold considers them equal. Logging out of s deletes only its store entry and then clears apiKeyStored on both profiles, leaving the still-present ſ key unreachable; the TUI's Remove Key flow takes the same path. Compare normalized store identities here and add a regression that preserves the unrelated marker.

• [P2] Keep the TUI's edit mirror exact-name aligned with the persisted mutation
  internal/tui/provider_manager.go:701
  This PR changes EditProvider to update the selected persisted row by exact name, but the follow-up in-memory updates still use EqualFold. Since the new validator permits distinct store identities such as s and ſ, editing the latter when the resolved list is ordered s, ſ mutates the former in memory; if the live session is on s, the line at 701 also rewrites its provider name and exported ZERO_PROVIDER for an edit to the other profile. The manager then disagrees with config.json until restart and can point spawned children at the wrong provider. Use the same exact identity for these UI mirror/session comparisons (and cover the distinct-Unicode case).

[PierrunoYT]

Addressed the latest requested changes in 6a708cc:

• ClearProviderKeyStoredCaseVariants now compares credstore.NormalizeProvider identities instead of strings.EqualFold, so deleting s does not clear the stored-key marker for distinct long-s ſ credentials.
• TUI persisted edits, savedProviders mirroring, live-session rename propagation, ZERO_PROVIDER, and restart guidance now stay aligned on trimmed exact provider names.
• EditProvider collision detection uses credential-store normalization so valid distinct identities such as s and ſ remain independently editable.
• Added focused Unicode regressions covering marker preservation and end-to-end TUI edit/session behavior.

Validation run with Go 1.26.5:

• go test ./internal/config ./internal/tui -run 'TestClearProviderKeyStored|TestProviderManager(EditKeepsDistinctUnicodeIdentityOutOfLiveSession|RenameFollowsLiveSession)'
• go test ./internal/config
• go test ./internal/tui
• make fmt-check
• go vet ./...
• go test ./...
• go run ./cmd/zero-release build
• go run ./cmd/zero-release smoke
• make lint-static (0 issues.)
• make vulncheck (No vulnerabilities found.)
• git diff HEAD --check

@jatmn please re-review the latest head when you have a chance.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Do not adopt an unrelated custom profile by case-folded name
  internal/config/writer.go:121
AdoptPersistedCatalogProviderName interprets a case-folded name match as proof that the existing row represents the requested catalog provider, but it never verifies the row's catalogId. A valid custom profile such as {name:"OpenRouter", catalogId:"custom-openai-compatible", baseURL:"https://corp.example/v1", model:"corp-model"} is therefore adopted by zero providers add openrouter. The caller then passes the adopted name to UpsertProvider, whose merge replaces the existing row's catalog ID, endpoint, model, transport/auth fields, and custom headers with OpenRouter's defaults; a pre-existing stored-key marker can survive that incompatible rewrite. The root cause is conflating display/name equivalence with provider ownership. Restrict adoption to a unique case-variant row whose catalog identity also matches the requested descriptor (or an equivalent explicitly supported identity); otherwise retain the requested default name and let the collision be reported rather than mutating a different profile. Please add a regression test with a custom OpenRouter-named row and a non-OpenRouter catalog ID.

• [P1] Keep distinct Unicode names out of destructive logout candidate expansion
  internal/config/writer.go:242
  The new logout flow expands its deletion set from the profile returned by ResolvePersistedProviderIdentity. That helper uses strings.EqualFold, while the credential store deliberately defines identity with the narrower credstore.NormalizeProvider rule: s and Unicode long-s ſ have separate stored API-key entries. With a saved s profile and its OAuth token/API key, zero auth logout ſ finds s as a folded name match, adds it to credentialCandidates, deletes its token and API key, and clears its apiKeyStored marker—even though ſ is an unsaved, distinct provider identity. The root cause is using a broad human-facing case comparison as authority for destructive credential ownership. Make destructive candidate expansion require an exact persisted name or the same credential-store normalization used by the store; audit the catalog-ID ownership checks under that same rule, and add a deny test proving logout ſ cannot affect a saved s credential.

• [P2] Let normal provider writes use the same identity rule as the credential store
  internal/config/writer.go:314
PreflightProviderWrite and UpsertProvider at line 343 still reject names with strings.EqualFold, even though this PR's validator, credential-store helper, delete path, marker clearing, and edit path deliberately treat s and Unicode long-s ſ as separate identities. Starting with a valid s profile, providers add ... --name ſ (and the setup/wizard paths that call the same preflight) now fails with a case-insensitive collision despite using a separate credential-store entry. This leaves the newly introduced identity contract internally inconsistent and makes the supported distinct profile pair hand-edit-only. Centralize persisted-profile collision checks on credstore.NormalizeProvider rather than mixing EqualFold and store normalization, then cover add/setup/wizard creation of the distinct pair.

• [P2] Do not retain a shared key after a failed TUI marker transfer
  internal/tui/provider_manager.go:370
  When deleting a case-variant row, the manager sets retainStoredKey before attempting the post-removal marker transfer. If TransferProviderAPIKeyStoredMarker fails after RemoveProvider has succeeded—for example because the follow-up config write fails—the manager only adds a warning and still calls cleanup with retainStoredKey=true. The secret remains in the credential store, but the surviving row retains apiKeyStored:false; ApplyStoredAPIKey intentionally does not consult the store without that marker, so the key is now unreachable while the UI has already reported Deleted <name>. The root cause is committing the retention decision before confirming the state that makes retention safe. Match the CLI's partial-failure behavior: only retain the key after a successful marker handoff, otherwise surface an incomplete delete and either preserve recoverable state or remove the unreachable credential deliberately. Add a failure-injection test for the post-removal marker write and assert that a completed-delete status is not shown with an inaccessible key.

[PierrunoYT]

Pushed 0279ed79, addressing all four findings from the 2026-08-01 20:08 review.

[P1] Do not adopt an unrelated custom profile by case-folded name — AdoptPersistedCatalogProviderName now requires the candidate row's catalog identity to agree with the requested descriptor, not just its name. A row that declares a different catalog id (your {name:"OpenRouter", catalogId:"custom-openai-compatible"} example) is left alone and the requested default name is kept, so the write reports the collision instead of rewriting another provider's endpoint/model/transport under a surviving stored-key marker. A row that declares no catalog id at all is still adopted — its name is the only identity it has, and that legacy shape is what adoption exists for. TestAdoptPersistedCatalogProviderNameIgnoresForeignCatalogRow covers the custom row (and asserts the follow-up PreflightProviderWrite reports the collision); TestAdoptPersistedCatalogProviderNameFollowsNameOnlyRow keeps the intended case alive.

[P1] Distinct Unicode names out of destructive expansion and [P2] normal writes use the store's rule are one fix, because they were the same inconsistency pointing in opposite directions. sameProviderIdentity (= credstore.NormalizeProvider equality) is now the single comparison behind identity resolution, CatalogIdentityExclusive, PreflightProviderWrite, UpsertProvider, RenameProvider, RemoveProvider's active-provider handoff, EnsureCatalogProvider's row reuse, ProviderPersistedCaseInsensitive, and migrateStoredProviderKey's same-entry short-circuit — that last one was the same bug inverted: EqualFold would have skipped a genuinely needed key migration between two distinct store entries. TestPersistedProviderIdentityRulesMatchTheCredentialStore pins the table: ſ does not resolve to a saved s, providers add --name ſ is accepted by both preflight and upsert, and an ASCII case variant is still rejected as a collision.

One correction worth flagging on the logout scenario, since it changes what a test there can prove: zero auth logout ſ never reached the credential deletion even before this change. credentialCandidates[0] is the spelling the user typed, and oauth.ValidateKey (^(provider|mcp):[A-Za-z0-9][A-Za-z0-9._-]{0,127}$) rejects provider:ſ, so the command exits before the OAuth or key-store loops run. The folded-name adoption was real, but the harm at that call site was blocked by the key validator, and a CLI-level deny test there passes for that reason whether or not the identity rule is fixed — I verified that by mutation. So the identity fix is pinned where it is reachable, in internal/config, and TestRunAuthLogoutKeepsDistinctUnicodeCredentials remains as an end-to-end guarantee test with a comment saying exactly which layer fires first rather than implying it pins the fix.

[P2] Do not retain a shared key after a failed TUI marker transfer — the manager no longer commits the retention decision before the handoff that makes retention safe. On failure it reports an incomplete delete naming the survivor and the recovery step ("the shared key is still in the credential store and WORK cannot reach it until you set a key for it again") instead of Deleted work., and keeps the secret rather than destroying it because a config write failed — matching zero providers remove, which also preserves the key and exits nonzero. transferProviderAPIKeyStoredMarker is a package var so the post-removal write can be failed deterministically; TestProviderManagerDeleteReportsFailedKeyMarkerHandoff injects that failure and asserts a completed-delete status is not shown while the key is unreachable.

Mutation-checked: reverting sameProviderIdentity to EqualFold fails the identity table, dropping the catalog check fails the adoption test, and restoring the old warning-only branch fails the TUI test.

Verification: go build ./..., go vet ./..., go test ./internal/... all pass; gofmt -l clean on the changed files.

[jatmn]

I found issues that need to be addressed before this is ready.

Findings

• [P1] Do not adopt a name-only custom profile as a catalog provider
  internal/config/writer.go:151
  The new adoption helper treats an OpenRouter row with an empty catalogId as proof that it is the catalog openrouter profile. That is not safe: a user can create a custom OpenAI-compatible profile named OpenRouter, pointed at a private gateway, with apiKeyStored:true and no catalog ID. Running zero providers add openrouter, zero setup openrouter, or the provider wizard reaches this helper, changes the requested profile name to OpenRouter, and lets UpsertProvider merge the OpenRouter defaults into that existing row. The base URL, model, transport/kind, and catalog ID are replaced, but the existing stored-key marker is preserved; on the next run the private-gateway secret is selected for the OpenRouter endpoint.

  The root cause is treating display-name equivalence as provider ownership. A missing catalog ID is absence of evidence, not evidence that the row belongs to the requested catalog. Restrict automatic adoption to a case variant whose catalog ID positively identifies the requested descriptor; otherwise leave the requested name unchanged and surface the existing collision for the user to resolve. Please cover CLI add, setup, and wizard persistence with a name-only custom OpenRouter row and a stored key, asserting that it is never rewritten and its key is never used for OpenRouter.

• [P2] Use credential-store identity for the active-provider fallback
  internal/config/resolver.go:941
  This PR deliberately makes credstore.NormalizeProvider the persisted-profile identity rule and therefore permits s and Unicode long-s ſ as separate profiles with separate secrets. The active-row fallback introduced here still uses strings.EqualFold, which is broader: strings.EqualFold("S", "s") and strings.EqualFold("S", "ſ") are both true, while credential-store normalization maps only S to s. With both valid rows present, a manually retained case-variant activeProvider: "S" or ZERO_PROVIDER=S finds no exact row, matches both in this fallback, and aborts as ambiguous instead of selecting the sole credential identity s; Zero cannot start.

  The root cause is a mixed identity model: mutations and credential ownership use normalized store identity, but runtime active-provider lookup still uses Unicode case folding. Centralize the persisted-provider lookup/equivalence rule and use it for the non-exact active-provider fallback as well, preserving exact-name precedence first. Add resolver coverage for {name:"s"} plus {name:"ſ"} with activeProvider and ZERO_PROVIDER set to S, and assert that it consistently resolves the s profile without reaching the ſ credential.