Merge refreshed provider identity and catalog fixes into transaction stack

Retain the reviewed resume implementation and port the unnamed stored-credential collision guard into the provider transaction boundary.

Amp-Thread-ID: https://ampcode.com/threads/T-01a0dcb1-0a19-7184-849e-38dbf12020d5
Co-authored-by: Pierre Bruno <pierrebruno@hotmail.ch>

Author: ampagent

internal/cli/provider_onboarding.go

@@ -58,7 +58,7 @@ func runProvidersUse(args []string, stdout io.Writer, stderr io.Writer, deps app
 	if err != nil {
 		return writeAppError(stderr, err.Error(), exitCrash)
 	}
-	resolvedName, persisted, err := resolvePersistedProviderName(configPath, options.name)
+	_, persisted, err := resolvePersistedProviderName(configPath, options.name)
 	if err != nil {
 		return writeAppError(stderr, err.Error(), exitCrash)
 	}
@@ -74,7 +74,10 @@ func runProvidersUse(args []string, stdout io.Writer, stderr io.Writer, deps app
 			return exit
 		}
 	} else {
-		options.name = resolvedName
+		options.name, err = resolveProviderMutationName(configPath, options.name, deps)
+		if err != nil {
+			return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash)
+		}
 	}
 	cfg, err := config.SetActiveProvider(configPath, options.name)
 	if err != nil {
@@ -622,7 +625,9 @@ func resolveProviderMutationName(path, name string, deps appDeps) (string, error
 		return "", err
 	}
 	row, lookup := config.LookupProviderName(names, name)
-	if lookup == config.ProviderNameExact {
+	// Concrete rows take precedence over catalog aliases, including requests
+	// that differ only in case from the concrete row's name.
+	if lookup.Resolved() {
 		owner, err := config.ProviderRowOwnershipAt(path, names, row)
 		if err != nil {
 			return "", err

internal/cli/provider_onboarding_test.go

@@ -13,6 +13,102 @@ import (
 	"github.com/Gitlawb/zero/internal/config"
 )
 
+func TestProviderCommandsRejectConcreteCatalogAlias(t *testing.T) {
+	for _, source := range []string{"project", "environment"} {
+		for _, command := range []string{"use", "remove", "rename"} {
+			for _, address := range []string{"openai", "OPENAI", "work"} {
+				for _, jsonOutput := range []bool{false, true} {
+					t.Run(fmt.Sprintf("%s/%s/%s/json=%t", source, command, address, jsonOutput), func(t *testing.T) {
+						setCLIUserConfigRoot(t)
+						clearProviderEnv(t)
+						t.Setenv("ZERO_CRED_STORAGE", "encrypted-file")
+						path := filepath.Join(t.TempDir(), "config.json")
+						writeProviderOnboardingConfig(t, path, config.FileConfig{
+							ActiveProvider: "other",
+							Providers: []config.ProviderProfile{
+								{Name: "work", CatalogID: "openai", ProviderKind: config.ProviderKindOpenAICompatible, BaseURL: "https://user.example/v1", Model: "user-model", APIKeyStored: true},
+								{Name: "other", ProviderKind: config.ProviderKindOpenAICompatible, BaseURL: "https://other.example/v1", Model: "other-model"},
+							},
+						})
+						workspace := t.TempDir()
+						if source == "project" {
+							writeProviderOnboardingConfig(t, filepath.Join(workspace, ".zero", "config.json"), config.FileConfig{Providers: []config.ProviderProfile{
+								{Name: "openai", ProviderKind: config.ProviderKindOpenAICompatible, BaseURL: "https://project.example/v1", Model: "project-model"},
+							}})
+						} else {
+							t.Setenv("OPENAI_API_KEY", "env-key")
+							t.Setenv("OPENAI_BASE_URL", "https://env.example/v1")
+						}
+						resolveOptions, err := config.DefaultResolveOptions(workspace)
+						if err != nil {
+							t.Fatal(err)
+						}
+						resolveOptions.UserConfigPath = path
+						names, err := config.ResolveProviderSourceNames(resolveOptions)
+						if _, lookup := config.LookupProviderName(names, "openai"); err != nil || lookup != config.ProviderNameExact {
+							t.Fatalf("fixture must contain a concrete openai row: names=%v err=%v", names, err)
+						}
+						before, err := os.ReadFile(path)
+						if err != nil {
+							t.Fatal(err)
+						}
+						store, err := config.ProviderKeyStoreAt(filepath.Dir(path))
+						if err != nil {
+							t.Fatal(err)
+						}
+						if err := store.Set("work", "user-key"); err != nil {
+							t.Fatal(err)
+						}
+						deps := providerSetupDeps(path)
+						deps.getwd = func() (string, error) { return workspace, nil }
+						args := []string{"providers", command, address}
+						if command == "rename" {
+							args = append(args, "renamed")
+						}
+						if jsonOutput {
+							args = append(args, "--json")
+						}
+						var stdout, stderr bytes.Buffer
+						code := runWithDeps(args, &stdout, &stderr, deps)
+						if address == "work" {
+							if code != exitSuccess || stdout.Len() == 0 {
+								t.Fatalf("exact saved row rejected: exit=%d stderr=%q", code, stderr.String())
+							}
+							cfg := readFileConfig(t, path)
+							switch command {
+							case "use":
+								if cfg.ActiveProvider != "work" {
+									t.Fatalf("activeProvider = %q, want work", cfg.ActiveProvider)
+								}
+							case "remove":
+								if len(cfg.Providers) != 1 || cfg.Providers[0].Name != "other" {
+									t.Fatal("exact saved row was not removed")
+								}
+							case "rename":
+								if cfg.Providers[0].Name != "renamed" {
+									t.Fatal("exact saved row was not renamed")
+								}
+							}
+							return
+						}
+						if code != exitCrash || stdout.Len() != 0 || !strings.Contains(stderr.String(), "isn't saved in config.json") {
+							t.Errorf("concrete row should report ownership error, exit=%d stdout=%q stderr=%q", code, stdout.String(), stderr.String())
+						}
+						after, err := os.ReadFile(path)
+						if err != nil || !bytes.Equal(before, after) {
+							t.Errorf("concrete row request changed user config: %v", err)
+						}
+						key, ok, err := store.Get("work")
+						if err != nil || !ok || key != "user-key" {
+							t.Errorf("concrete row request changed user credential: present=%t error=%v", ok, err)
+						}
+					})
+				}
+			}
+		}
+	}
+}
+
 func TestProviderMutationsRejectProjectCaseSibling(t *testing.T) {
 	for _, command := range []string{"remove", "rename"} {
 		t.Run(command, func(t *testing.T) {

internal/config/writer.go

@@ -180,6 +180,11 @@ func RepairUnnamedProvider(path string, replacement string) (result FileConfig,
 				name = "openai"
 			}
 		}
+		if cfg.Providers[unnamed].APIKeyStored {
+			if sibling, collides := conflictingProviderRowName(*cfg, unnamed, legacyName); collides && sibling != legacyName {
+				return fmt.Errorf("the unnamed provider's stored credential uses identity %q, which conflicts with persisted provider %q; manually rename the sibling %q in config.json and reconcile any of its credential references before retrying `zero providers repair-config`; --name cannot safely move the stored credential", legacyName, sibling, sibling)
+			}
+		}
 		if conflict, collides := conflictingProviderRowName(*cfg, unnamed, name); collides {
 			if explicit {
 				return fmt.Errorf(

internal/config/writer_test.go

@@ -1590,6 +1590,27 @@ func TestRepairUnnamedProviderAllowsExplicitUniqueName(t *testing.T) {
 	}
 }
 
+func TestRepairUnnamedStoredProviderCaseCollisionExplainsManualRecovery(t *testing.T) {
+	path := filepath.Join(t.TempDir(), "config.json")
+	before := writeConfigFixture(t, path, FileConfig{
+		ActiveProvider: "legacy",
+		Providers: []ProviderProfile{
+			{APIKeyStored: true},
+			{Name: "LEGACY"},
+		},
+	}, 0o600)
+	for _, name := range []string{"", "work", "Legacy"} {
+		_, _, err := RepairUnnamedProvider(path, name)
+		if err == nil || !strings.Contains(err.Error(), `manually rename the sibling "LEGACY" in config.json`) {
+			t.Fatalf("repair with name %q must explain the manual recovery: %v", name, err)
+		}
+		after, readErr := os.ReadFile(path)
+		if readErr != nil || !bytes.Equal(before, after) {
+			t.Fatal("refused repair changed config")
+		}
+	}
+}
+
 func TestEnsureCatalogProviderValidatesBeforeExistingProfileShortcut(t *testing.T) {
 	path := filepath.Join(t.TempDir(), "config.json")
 	before := writeConfigFixture(t, path, FileConfig{Providers: []ProviderProfile{{Name: "xai"}, {Name: "XAI"}}}, 0o600)

internal/tui/command_center.go

@@ -633,11 +633,11 @@ func (m model) switchProviderModel(providerName, modelID string) (model, string,
 		switch {
 		case err != nil:
 			persistErr = err
-			persistNote = "\nNote: the switch applies to this session, but config.json could not be read: " + redaction.RedactString(err.Error(), redaction.Options{})
+			persistNote = "\nNote: the switch applies to this session, but config.json could not be read."
 		case owner.UserBacked:
 			if _, err := config.SetActiveProviderModel(m.userConfigPath, owner.PersistedName, target.Model); err != nil {
 				persistErr = err
-				persistNote = "\nNote: the switch applies to this session, but config.json was not updated: " + redaction.RedactString(err.Error(), redaction.Options{})
+				persistNote = "\nNote: the switch applies to this session, but config.json was not updated."
 			} else {
 				persisted = true
 				// Reconcile the in-memory list the manager and picker read from,

internal/tui/command_center_test.go

@@ -207,6 +207,9 @@ func TestSwitchProviderModelReportsPersistenceFailures(t *testing.T) {
 		if !strings.Contains(status, "config.json could not be read") {
 			t.Fatalf("status = %q, want a persistence note", status)
 		}
+		if strings.Count(status, err.Error()) != 1 {
+			t.Fatal("persistence error must appear exactly once in the switch status")
+		}
 		// The session did switch, which is exactly why the note has to be there.
 		if next.providerName != "OpenAI" {
 			t.Fatalf("providerName = %q, want OpenAI", next.providerName)
@@ -227,6 +230,9 @@ func TestSwitchProviderModelReportsPersistenceFailures(t *testing.T) {
 		if !strings.Contains(status, "config.json was not updated") {
 			t.Fatalf("status = %q, want the active-provider persistence note", status)
 		}
+		if strings.Count(status, err.Error()) != 1 {
+			t.Fatal("persistence error must appear exactly once in the switch status")
+		}
 	})
 
 	t.Run("env-derived provider stays silent", func(t *testing.T) {

internal/tui/model_test.go

@@ -1277,26 +1277,27 @@ func TestResumeAfterDeletingLiveProjectRow(t *testing.T) {
 				m.providerWizard = nil
 			}
 			wantProvider, wantModel, wantRemoved := m.providerName, m.modelName, m.removedLiveRow
-			liveClient := m.provider
+			liveClient, liveProfile := m.provider, m.providerProfile
 			if tc.sameSession {
 				target = previous
 			}
 			next, message := m.handleResumeCommand(target.SessionID)
 			if message != "" || next.activeSession.SessionID != target.SessionID {
 				t.Fatalf("resume failed: %s", message)
 			}
-			if next.provider != liveClient || len(built) != 0 {
-				t.Fatal("resume must keep the live client without building another provider")
-			}
-			if next.providerProfile.Name != "WORK" || next.providerProfile.BaseURL != "https://project.example.com/v1" || next.providerProfile.Model != "project-model" || next.providerProfile.APIKey != "sk-project" {
-				t.Fatal("resume changed the live project provider profile")
+			// Resume loads history, not a new provider. Labels and retained row
+			// ownership must still describe the client that answers the next turn.
+			if next.provider != liveClient || len(built) != 0 || !reflect.DeepEqual(next.providerProfile, liveProfile) {
+				t.Fatal("resume changed the live client or its endpoint/credential profile")
 			}
 			if next.removedLiveRow != wantRemoved || next.providerName != wantProvider || next.modelName != wantModel || next.activeProviderRowName() != wantProvider {
 				t.Fatalf("resume identity = (%q, %q, %q, %q), want (%q, %q, %q, %q)", next.removedLiveRow, next.providerName, next.modelName, next.activeProviderRowName(), wantRemoved, wantProvider, wantModel, wantProvider)
 			}
-			summary := plainRender(t, transcriptText(next.transcript))
-			if !tc.sameSession && (!strings.Contains(summary, "project-model") || !strings.Contains(summary, "(recorded: resumed-model)")) {
-				t.Fatalf("resume summary must distinguish the live model from the recorded model: %s", summary)
+			if !transcriptContains(next.transcript, "model: project-model") || !transcriptContains(next.transcript, "provider: WORK") {
+				t.Fatal("resume summary does not describe the live project client")
+			}
+			if !tc.sameSession && !transcriptContains(next.transcript, "(recorded: resumed-model)") {
+				t.Fatal("resume summary does not distinguish the recorded model from the live model")
 			}
 			assertUserRowUntouched(t, next, before)
 		})

internal/tui/session.go

@@ -236,8 +236,8 @@ func (m model) handleResumeCommand(args string) (model, string) {
 	m.activeSession = *session
 	m.pendingSessionTitle = ""
 	m.sessionEvents = append([]sessions.Event{}, events...)
-	// Resume restores history, not the client. Keep the live identity, including
-	// a removed provider row, until an explicit provider switch replaces it.
+	// Resume replaces conversation history, not the live client. Keep its
+	// identity (including a removed row) and model until an actual provider switch.
 	if m.providerName == "" {
 		m.removedLiveRow = ""
 		m.providerName = session.Provider