feat: rethink credential storage data model and UX

cmd/cr/main_test.go

@@ -77,7 +77,7 @@ func TestRunConfigShowJSON(t *testing.T) {
 	}
 
 	var stdout, stderr bytes.Buffer
-	code := run([]string{"config", "show", "--json"}, strings.NewReader(""), &stdout, &stderr)
+	code := run([]string{"--profile", "home", "config", "show", "--json"}, strings.NewReader(""), &stdout, &stderr)
 	if code != 0 {
 		t.Fatalf("run exit code = %d, want 0; stderr = %q", code, stderr.String())
 	}
@@ -144,6 +144,13 @@ cases:
 func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) {
 	statedirtest.Hermetic(t)
 	t.Setenv("CODEREVIEW_KEYRING_PASSPHRASE", "test-passphrase")
+	path, err := config.Path()
+	if err != nil {
+		t.Fatalf("config Path: %v", err)
+	}
+	if err := config.Save(path, mainCredentialCommandTestConfig()); err != nil {
+		t.Fatalf("config Save: %v", err)
+	}
 	const (
 		firstSecret  = "distinctive-secret-one"
 		secondSecret = "distinctive-secret-two"
@@ -158,25 +165,26 @@ func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) {
 		wantCode int
 	}{
 		{
-			name: "init",
+			name: "set initial credential",
 			args: []string{
-				"--backend", "file",
-				"init",
-				"--non-interactive",
-				"--git-token-from-env", "CR_TOKEN_ONE",
+				"set-credential",
+				"--store", "test-file",
+				"--name", "codereview/default",
+				"--key", "git_token",
+				"--from-env", "CR_TOKEN_ONE",
 			},
 		},
 		{
 			name:     "config show",
-			args:     []string{"--backend", "file", "config", "show", "--json"},
+			args:     []string{"--profile", "default", "config", "show", "--json"},
 			wantCode: 0,
 		},
 		{
 			name: "existing credential failure",
 			args: []string{
-				"--backend", "file",
 				"set-credential",
-				"--ref", "codereview/default",
+				"--store", "test-file",
+				"--name", "codereview/default",
 				"--key", "git_token",
 				"--from-env", "CR_TOKEN_TWO",
 			},
@@ -185,9 +193,9 @@ func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) {
 		{
 			name: "set overwrite",
 			args: []string{
-				"--backend", "file",
 				"set-credential",
-				"--ref", "codereview/default",
+				"--store", "test-file",
+				"--name", "codereview/default",
 				"--key", "git_token",
 				"--from-env", "CR_TOKEN_TWO",
 				"--overwrite",
@@ -196,7 +204,7 @@ func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) {
 		},
 		{
 			name:     "config clear",
-			args:     []string{"--backend", "file", "config", "clear", "--json"},
+			args:     []string{"--profile", "default", "config", "clear", "--json"},
 			wantCode: 0,
 		},
 	}
@@ -216,14 +224,15 @@ func TestRunCredentialCommandsDoNotLeakSecrets(t *testing.T) {
 
 func mainTestConfig() config.File {
 	return config.File{
-		DefaultProfile: "home",
-		Keyring:        config.KeyringConfig{Backend: "memory"},
 		Profiles: map[string]config.Profile{
 			"home": {
 				Git: config.GitConfig{
-					Host:          "github.com",
-					AuthMode:      config.GitAuthModePAT,
-					CredentialRef: "codereview/home",
+					Host:     "github.com",
+					AuthMode: config.GitAuthModePAT,
+					Credential: config.CredentialLocation{
+						Store: config.LocalOSCredentialStoreID,
+						Name:  "codereview/home",
+					},
 				},
 				LLM: config.LLMConfig{
 					Provider: config.LLMProviderAnthropic,
@@ -236,6 +245,24 @@ func mainTestConfig() config.File {
 	}
 }
 
+func mainCredentialCommandTestConfig() config.File {
+	cfg := mainTestConfig()
+	cfg.Secrets = config.SecretsConfig{
+		Stores: map[string]config.SecretsStore{
+			"test-file": {
+				DisplayName: "Test file store",
+				Backend: config.SecretsStoreBackend{
+					Kind: config.SecretsBackendKind("file"),
+				},
+			},
+		},
+	}
+	profile := cfg.Profiles["home"]
+	profile.Git.Credential = config.CredentialLocation{Store: "test-file", Name: "codereview/default"}
+	cfg.Profiles = map[string]config.Profile{"default": profile}
+	return cfg
+}
+
 func assertNoSecretLeak(t *testing.T, output string, secrets ...string) {
 	t.Helper()
 	for _, secret := range secrets {

docs/config-command-surface-design.md

@@ -11,11 +11,11 @@ bootstrap, secret ingress, or the full long-term profile management API.
 
 This document is the semantic parent for:
 
-- `#167`: `cr config path` and default-profile commands
+- `#167`: `cr config path`
 - `#166`: `cr config route` management commands
 - `#169`: `cr config resolve-profile` route preview
 - `#168`: `cr config agent-source` management commands
-- `#294`: `cr config secrets-profile` management commands
+- `#294`: `cr config credential-store` inspection commands
 
 Related but out of scope for this batch:
 
@@ -49,10 +49,10 @@ The codebase already supports:
 - GitHub App reviewer auth
 
 The missing piece is command coverage. Today `cr config` exposes `show`,
-`clear`, and `llm models`, but path/default/route/route-preview/agent-source
-operations still require manual YAML edits. Named secrets-management profiles
-also need a non-interactive command surface, but that surface must remain
-config-only until the later secrets-profile resolver/selection tickets land.
+`clear`, and `llm models`, but path/route/route-preview/agent-source
+operations still require manual YAML edits. Credential-store inspection is
+available through `cr config credential-store`; creating, editing, and deleting
+configured stores belongs to the interactive `cr init` secrets-storage flow.
 
 ## Shared Command Rules
 
@@ -63,7 +63,7 @@ These rules apply across the batch unless a ticket explicitly narrows them.
 - Commands that operate on a profile should use the existing global
   `--profile <name>` flag.
 - When a command is not repository-aware, `--profile` selects the target
-  profile and otherwise falls back to `default_profile`.
+  profile; commands that need a profile must fail if none is selected.
 - Commands introduced in this batch must not invent a second profile-selection
   mechanism.
 
@@ -103,13 +103,11 @@ These rules apply across the batch unless a ticket explicitly narrows them.
 
 ## Ticket Ownership Matrix
 
-### `#167`: path and default profile
+### `#167`: path
 
 Owns:
 
 - `cr config path [--json]`
-- `cr config default get [--json]`
-- `cr config default set <profile>`
 
 Must not own:
 
@@ -120,8 +118,6 @@ Must not own:
 
 Required semantics:
 
-- `default set` changes only `default_profile`
-- `default set` validates that the target profile exists
 - path inspection reports resolved config location only; broader state-path
   expansion is optional follow-up work, not required by this ticket
 
@@ -137,7 +133,6 @@ Must not own:
 
 - route preview output beyond what is needed for route list/mutation
 - runtime route-resolution redesign
-- default-profile mutation
 
 Required semantics:
 
@@ -189,8 +184,7 @@ Must not own:
 Required semantics:
 
 - reuse the same repository-resolution path already used by runtime commands
-- report whether resolution came from explicit `--profile`, matched route, or
-  fallback to `default_profile`
+- report whether resolution came from explicit `--profile` or a matched route
 - remain a local preview command, not an execution path
 - if the existing runtime resolution path does not expose enough metadata for
   preview output, `#169` may add a shared resolution-result wrapper/helper that
@@ -237,40 +231,30 @@ Path normalization contract:
 This keeps mutation semantics deterministic and local-string-based rather than
 filesystem-state-based.
 
-### `#294`: secrets-management profile mutation
+### `#294`: credential-store inspection
 
 Owns:
 
-- `cr config secrets-profile list [--json]`
-- `cr config secrets-profile get <id> [--json]`
-- `cr config secrets-profile set <id> [--backend <kind>] [--label <text>] [--clear-label] [--op-timeout <duration>] [--op-vault-id <id>] [--op-item-title-prefix <text>] [--op-item-tag <text>] [--op-item-field-title <text>] [--op-connect-host <url>] [--op-connect-token-env <name>] [--op-service-token-env <name>] [--op-desktop-account-id <id>]`
-- `cr config secrets-profile default get [--json]`
-- `cr config secrets-profile default set <id>`
-- `cr config secrets-profile default unset`
-- `cr config secrets-profile remove <id>`
+- `cr config credential-store list [--json]`
+- `cr config credential-store get <id> [--json]`
 
 Must not own:
 
 - secret ingress or secret-value printing
-- runtime credential routing away from legacy `keyring.backend`
-- review-profile references to secrets-management profiles before the later selection tickets
+- creating, editing, deleting, or selecting credential stores for a write
+- any user-facing credential-store default
 
 Required semantics:
 
-- `list` and `default get` report the effective secrets-profile inventory/default, including the projected `legacy-default` compatibility entry when no explicit profiles exist
-- `default set` and `default unset` mutate only `config.secrets.default_profile`
-- `default set` rejects `legacy-default` because it is reserved/projected, not configured state
-- `set` is patch-style:
-  - create requires `--backend`
-  - update preserves omitted fields
-  - `--clear-label` explicitly clears the label
-  - `--label` plus `--clear-label` is a usage error
-  - whitespace-only `--label` is rejected
-  - update requires at least one mutation flag
-  - `--backend op`, `--backend op-connect`, and `--backend op-desktop` also require `--op-vault-id`; `op-connect` additionally requires `--op-connect-host`
-- `remove` is idempotent for already-absent configured profiles, but rejects `legacy-default` and blocks removing the configured default profile until it is unset or moved
+- `list` reports the effective credential-store inventory, including the
+  read-only built-in `local-os` store
+- `get` reports one effective credential store by id
+- configured stores are listed after `local-os` in stable order
+- `local-os` is projected and is never persisted under `secrets.stores`
+- JSON output must not include an `is_default` field for credential stores
 - backend validation must reuse shared credstore metadata rather than duplicating backend-name knowledge in command code
-- this ticket is config-only: mixed configs with both `keyring.backend` and `secrets.*` keep legacy runtime backend behavior until the later resolver ticket changes credential selection
+- old `config secrets-profile ...` subcommands are rejected instead of acting as
+  compatibility aliases
 
 ## `init` Boundary
 
@@ -300,7 +284,7 @@ The likely future design questions remain:
 
 - whether profile mutation should be patch-like or declarative replacement
 - whether field-specific subcommands and `profile set` should coexist
-- how profile mutation should interact with credential refs and route safety
+- how profile mutation should interact with credential locations and route safety
 
 Those questions belong to later work after the current true-up lands.