diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml
index 5bebad38..0782308c 100644
--- a/.github/workflows/ci.yml
+++ b/.github/workflows/ci.yml
@@ -85,6 +85,55 @@ jobs:
           echo "$HELP" | grep -q "rules"
           rm -f ./opencodereview
 
+  # Runs the suite natively on Windows, which the cross-compile job below cannot
+  # do: it only proves the windows arms of the build-tag splits compile. GitHub
+  # does not support `container:` on Windows runners
+  # (actions/runner#904), so this job installs Go directly instead of reusing the
+  # golang:1.26.5 image the other jobs share.
+  windows:
+    runs-on: windows-latest
+    timeout-minutes: 20
+    steps:
+      - uses: actions/checkout@v7
+
+      - uses: actions/setup-go@v7
+        with:
+          go-version: '1.26.5'
+          cache: true
+
+      - name: Vet
+        run: go vet ./...
+
+      # No -race here: the race detector needs a working C toolchain on Windows,
+      # and races are OS-independent, so the Linux job above already covers them.
+      # This job is here for the OS-specific behavior instead. No coverage gate
+      # either -- the //go:build !windows test files legitimately drop the total
+      # below the 80% the Linux job enforces.
+      - name: Test
+        run: go test -count=1 ./...
+
+      - name: Build
+        run: go build -o opencodereview.exe ./cmd/opencodereview
+
+      # Same assertions as the Linux smoke test, under git-bash so the script is
+      # shared verbatim rather than reimplemented in PowerShell.
+      - name: Smoke test
+        shell: bash
+        run: |
+          ./opencodereview.exe --version
+          ./opencodereview.exe --version | grep -q "open-code-review"
+          HELP=$(./opencodereview.exe --help)
+          echo "$HELP" | grep -q "Commands:"
+          echo "$HELP" | grep -q "review"
+          echo "$HELP" | grep -q "scan"
+          echo "$HELP" | grep -q "delegate"
+          echo "$HELP" | grep -q "config"
+          echo "$HELP" | grep -q "llm"
+          echo "$HELP" | grep -q "viewer"
+          echo "$HELP" | grep -q "session"
+          echo "$HELP" | grep -q "rules"
+          rm -f ./opencodereview.exe
+
   cross-compile:
     runs-on: self-hosted
     timeout-minutes: 10
diff --git a/cmd/opencodereview/background_file_test.go b/cmd/opencodereview/background_file_test.go
index 827e52d7..39de56a8 100644
--- a/cmd/opencodereview/background_file_test.go
+++ b/cmd/opencodereview/background_file_test.go
@@ -4,6 +4,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 )
@@ -37,7 +38,14 @@ func TestResolveBackgroundFilePath(t *testing.T) {
 	})
 
 	t.Run("absolute unchanged", func(t *testing.T) {
+		// FromSlash is not enough on its own: it only swaps separators, and
+		// `\etc\context.md` is rooted but not absolute on Windows, where
+		// filepath.IsAbs wants a volume. Without the drive letter this case
+		// exercised the relative branch instead of the one it names.
 		abs := filepath.FromSlash("/etc/context.md")
+		if runtime.GOOS == "windows" {
+			abs = `C:\etc\context.md`
+		}
 		if got := resolveBackgroundFilePath(repo, abs); got != abs {
 			t.Errorf("resolveBackgroundFilePath = %q, want %q (absolute must be untouched)", got, abs)
 		}
diff --git a/cmd/opencodereview/config_cmd.go b/cmd/opencodereview/config_cmd.go
index 64d19190..92f6dc56 100644
--- a/cmd/opencodereview/config_cmd.go
+++ b/cmd/opencodereview/config_cmd.go
@@ -85,33 +85,77 @@ func runConfigSet(key, value string) error {
 	}
 
 	displayValue := value
-	normalizedKey := strings.ToLower(strings.ReplaceAll(key, "_", ""))
-	if strings.HasSuffix(normalizedKey, "apikey") || strings.HasSuffix(normalizedKey, "authtoken") {
+	if shouldMaskConfigValue(key) {
 		displayValue = maskKey(value)
 	}
 	fmt.Printf("Set %s = %s\n", key, displayValue)
+	if warning := legacyLLMShadowWarning(cfg.Provider, key); warning != "" {
+		fmt.Fprint(os.Stderr, warning)
+	}
 	return nil
 }
 
-func runConfigUnset(key string) error {
-	parts := strings.SplitN(key, ".", 2)
-	if len(parts) != 2 || parts[1] == "" {
-		return fmt.Errorf("unset supports custom_providers.<name> and mcp_servers.<name>")
-	}
+// shouldMaskConfigValue reports whether the echoed value of a config key holds a
+// secret and must be masked. Matching on the normalized suffix covers both
+// snake_case and Go field spellings of api_key/auth_token at any path depth,
+// while the *_cmd variants stay unmasked: a command line is not a secret.
+func shouldMaskConfigValue(key string) bool {
+	normalizedKey := strings.ToLower(strings.ReplaceAll(key, "_", ""))
+	return strings.HasSuffix(normalizedKey, "apikey") || strings.HasSuffix(normalizedKey, "authtoken")
+}
 
+func runConfigUnset(key string) error {
 	configPath, err := defaultConfigPath()
 	if err != nil {
 		return err
 	}
 
+	if key == "provider" {
+		return unsetActiveProvider(configPath)
+	}
+
+	parts := strings.SplitN(key, ".", 2)
+	if len(parts) != 2 || parts[1] == "" {
+		return fmt.Errorf("unset supports provider, custom_providers.<name>, and mcp_servers.<name>")
+	}
+
 	switch parts[0] {
 	case "custom_providers":
 		return unsetCustomProvider(configPath, parts[1])
 	case "mcp_servers":
 		return unsetMCPServer(configPath, parts[1])
 	default:
-		return fmt.Errorf("unset supports custom_providers.<name> and mcp_servers.<name>")
+		return fmt.Errorf("unset supports provider, custom_providers.<name>, and mcp_servers.<name>")
+	}
+}
+
+func unsetActiveProvider(configPath string) error {
+	cfg, err := loadOrCreateConfig(configPath)
+	if err != nil {
+		return fmt.Errorf("load config: %w", err)
+	}
+
+	cfg.Provider = ""
+	cfg.Model = ""
+	if err := saveConfig(configPath, cfg); err != nil {
+		return err
+	}
+
+	fmt.Println("Cleared active provider and model.")
+	return nil
+}
+
+func legacyLLMShadowWarning(provider, key string) string {
+	if provider == "" || !strings.HasPrefix(key, "llm.") {
+		return ""
+	}
+	section := "custom_providers"
+	if _, isPreset := llm.LookupProvider(provider); isPreset {
+		section = "providers"
 	}
+	return fmt.Sprintf("[ocr] WARNING: provider %q is active and takes precedence over llm.* settings.\n"+
+		"[ocr] Use 'ocr config set %s.%s.<field> <value>' to configure the active provider,\n"+
+		"[ocr] or run 'ocr config unset provider' to disable provider-based config.\n", provider, section, provider)
 }
 
 func unsetCustomProvider(configPath, name string) error {
@@ -190,6 +234,7 @@ func deleteCustomProvider(cfg *Config, name string) (bool, error) {
 // ProviderEntry holds per-provider configuration in the providers map.
 type ProviderEntry struct {
 	APIKey       string            `json:"api_key,omitempty"`
+	APIKeyCmd    string            `json:"api_key_cmd,omitempty"` // shell command whose stdout is the api key; used when api_key is empty
 	URL          string            `json:"url,omitempty"`
 	Protocol     string            `json:"protocol,omitempty"`
 	Model        string            `json:"model,omitempty"`
@@ -228,6 +273,7 @@ type Config struct {
 type LlmConfig struct {
 	URL          string            `json:"url,omitempty"`
 	AuthToken    string            `json:"auth_token,omitempty"`
+	AuthTokenCmd string            `json:"auth_token_cmd,omitempty"` // shell command whose stdout is the auth token; used when auth_token is empty
 	AuthHeader   string            `json:"auth_header,omitempty"`
 	Model        string            `json:"model,omitempty"`
 	Protocol     string            `json:"protocol,omitempty"`      // canonical protocol name; takes priority over UseAnthropic
@@ -333,6 +379,8 @@ func setConfigValue(cfg *Config, key, value string) error {
 		cfg.Llm.URL = value
 	case "llm.auth_token", "llm.AuthToken":
 		cfg.Llm.AuthToken = value
+	case "llm.auth_token_cmd", "llm.AuthTokenCmd":
+		cfg.Llm.AuthTokenCmd = value
 	case "llm.auth_header", "llm.AuthHeader":
 		normalized, err := llm.NormalizeAuthHeader(value)
 		if err != nil {
@@ -407,7 +455,7 @@ func setConfigValue(cfg *Config, key, value string) error {
 		}
 		cfg.Llm.ExtraBody = m
 	default:
-		return fmt.Errorf("unknown config key: %s\nSupported keys: provider, model, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_header, llm.model, llm.protocol, llm.use_anthropic, llm.extra_body, llm.extra_headers, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging\nProvider fields: api_key, url, protocol, model, models, auth_header, extra_body, extra_headers\nProtocol values: anthropic, openai, openai-responses\nMCP server fields: type, command, args, env, url, headers, tools, setup", key)
+		return fmt.Errorf("unknown config key: %s\nSupported keys: provider, model, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_token_cmd, llm.auth_header, llm.model, llm.protocol, llm.use_anthropic, llm.extra_body, llm.extra_headers, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging\nProvider fields: api_key, api_key_cmd, url, protocol, model, models, auth_header, extra_body, extra_headers\nProtocol values: anthropic, openai, openai-responses\nMCP server fields: type, command, args, env, url, headers, tools, setup", key)
 	}
 	return nil
 }
@@ -416,6 +464,8 @@ func applyProviderField(entry *ProviderEntry, field, key, value string) error {
 	switch field {
 	case "api_key":
 		entry.APIKey = value
+	case "api_key_cmd":
+		entry.APIKeyCmd = value
 	case "url":
 		entry.URL = value
 	case "protocol":
@@ -451,7 +501,7 @@ func applyProviderField(entry *ProviderEntry, field, key, value string) error {
 		}
 		entry.ExtraHeaders = parsed
 	default:
-		return fmt.Errorf("unknown provider field %q: supported fields are api_key, url, protocol, model, models, auth_header, extra_body, extra_headers", field)
+		return fmt.Errorf("unknown provider field %q: supported fields are api_key, api_key_cmd, url, protocol, model, models, auth_header, extra_body, extra_headers", field)
 	}
 	return nil
 }
diff --git a/cmd/opencodereview/config_cmd_test.go b/cmd/opencodereview/config_cmd_test.go
index 1cba3f99..e7e88650 100644
--- a/cmd/opencodereview/config_cmd_test.go
+++ b/cmd/opencodereview/config_cmd_test.go
@@ -1,8 +1,10 @@
 package main
 
 import (
+	"io"
 	"os"
 	"strconv"
+	"strings"
 	"testing"
 
 	"github.com/alibaba/open-code-review/internal/llm"
@@ -87,6 +89,56 @@ func TestSetConfigValueProviderEntry(t *testing.T) {
 	}
 }
 
+func TestSetConfigValueKeyCmdFields(t *testing.T) {
+	// A typo in any of these case labels would silently degrade to "unknown
+	// provider field" / "unknown config key", so assert the field each key writes.
+	const value = "op read op://dev/anthropic/api-key"
+	tests := []struct {
+		name string
+		key  string
+		got  func(cfg *Config) string
+	}{
+		{"preset provider api_key_cmd", "providers.anthropic.api_key_cmd", func(cfg *Config) string { return cfg.Providers["anthropic"].APIKeyCmd }},
+		{"custom provider api_key_cmd", "custom_providers.my-gateway.api_key_cmd", func(cfg *Config) string { return cfg.CustomProviders["my-gateway"].APIKeyCmd }},
+		{"llm auth_token_cmd", "llm.auth_token_cmd", func(cfg *Config) string { return cfg.Llm.AuthTokenCmd }},
+		{"llm AuthTokenCmd alias", "llm.AuthTokenCmd", func(cfg *Config) string { return cfg.Llm.AuthTokenCmd }},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			cfg := &Config{}
+			if err := setConfigValue(cfg, tt.key, value); err != nil {
+				t.Fatalf("setConfigValue %s: %v", tt.key, err)
+			}
+			if got := tt.got(cfg); got != value {
+				t.Errorf("%s = %q, want %q", tt.key, got, value)
+			}
+		})
+	}
+}
+
+func TestShouldMaskConfigValue(t *testing.T) {
+	// api_key/auth_token values are secrets; the *_cmd variants are command
+	// lines, so they print unmasked.
+	tests := []struct {
+		key  string
+		want bool
+	}{
+		{"llm.auth_token", true},
+		{"llm.auth_token_cmd", false},
+		{"providers.x.api_key", true},
+		{"providers.x.api_key_cmd", false},
+		{"providers.x.APIKeyCmd", false},
+		{"llm.AuthToken", true},
+	}
+	for _, tt := range tests {
+		t.Run(tt.key, func(t *testing.T) {
+			if got := shouldMaskConfigValue(tt.key); got != tt.want {
+				t.Errorf("shouldMaskConfigValue(%q) = %v, want %v", tt.key, got, tt.want)
+			}
+		})
+	}
+}
+
 func TestSetConfigValueProviderEntryNonPresetWritesCustomProvider(t *testing.T) {
 	cfg := &Config{}
 
@@ -950,14 +1002,115 @@ func TestSetConfigValueProviderClearsModel(t *testing.T) {
 }
 
 func TestRunConfigUnset_InvalidKey(t *testing.T) {
-	if err := runConfigUnset("provider"); err == nil {
-		t.Fatal("expected error for non custom_providers key")
-	}
 	if err := runConfigUnset("custom_providers."); err == nil {
 		t.Fatal("expected error for empty provider name")
 	}
 }
 
+func TestRunConfigSetWarnsWhenActiveProviderShadowsLegacyLLMConfig(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	configPath, err := defaultConfigPath()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := saveConfig(configPath, &Config{Provider: "dashscope"}); err != nil {
+		t.Fatalf("save config: %v", err)
+	}
+
+	stderr := captureConfigStderr(t, func() {
+		if err := runConfigSet("llm.url", "https://gateway.example/v1"); err != nil {
+			t.Fatalf("runConfigSet: %v", err)
+		}
+	})
+	if !strings.Contains(stderr, `provider "dashscope" is active`) {
+		t.Errorf("warning = %q", stderr)
+	}
+	if !strings.Contains(stderr, "providers.dashscope.<field>") || !strings.Contains(stderr, "config unset provider") {
+		t.Errorf("warning does not explain how to resolve precedence: %q", stderr)
+	}
+
+	cfg, err := loadOrCreateConfig(configPath)
+	if err != nil {
+		t.Fatalf("reload config: %v", err)
+	}
+	if cfg.Llm.URL != "https://gateway.example/v1" {
+		t.Errorf("llm.url = %q", cfg.Llm.URL)
+	}
+}
+
+func TestLegacyLLMShadowWarning(t *testing.T) {
+	if got := legacyLLMShadowWarning("", "llm.model"); got != "" {
+		t.Errorf("warning without active provider = %q", got)
+	}
+	if got := legacyLLMShadowWarning("dashscope", "providers.dashscope.url"); got != "" {
+		t.Errorf("warning for provider setting = %q", got)
+	}
+	if got := legacyLLMShadowWarning("dashscope", "Llm.model"); got != "" {
+		t.Errorf("warning for invalid mixed-case legacy key = %q", got)
+	}
+	if got := legacyLLMShadowWarning("dashscope", "llm.model"); !strings.Contains(got, "providers.dashscope.<field>") {
+		t.Errorf("preset-provider warning = %q", got)
+	}
+	if got := legacyLLMShadowWarning("my-gateway", "llm.model"); !strings.Contains(got, "custom_providers.my-gateway.<field>") {
+		t.Errorf("custom-provider warning = %q", got)
+	}
+}
+
+func TestRunConfigUnsetProviderClearsSelectionAndKeepsProviderEntries(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	configPath, err := defaultConfigPath()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := saveConfig(configPath, &Config{
+		Provider: "dashscope",
+		Model:    "legacy-model",
+		Providers: map[string]ProviderEntry{
+			"dashscope": {APIKey: "secret", Model: "provider-model"},
+		},
+	}); err != nil {
+		t.Fatalf("save config: %v", err)
+	}
+
+	if err := runConfigUnset("provider"); err != nil {
+		t.Fatalf("runConfigUnset(provider): %v", err)
+	}
+	cfg, err := loadOrCreateConfig(configPath)
+	if err != nil {
+		t.Fatalf("reload config: %v", err)
+	}
+	if cfg.Provider != "" || cfg.Model != "" {
+		t.Errorf("provider/model = %q/%q, want both empty", cfg.Provider, cfg.Model)
+	}
+	if got := cfg.Providers["dashscope"].APIKey; got != "secret" {
+		t.Errorf("provider entry was removed or changed: api_key = %q", got)
+	}
+}
+
+func captureConfigStderr(t *testing.T, fn func()) string {
+	t.Helper()
+	old := os.Stderr
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatal(err)
+	}
+	os.Stderr = w
+	defer func() { os.Stderr = old }()
+
+	fn()
+	if err := w.Close(); err != nil {
+		t.Fatal(err)
+	}
+	data, err := io.ReadAll(r)
+	if err != nil {
+		t.Fatal(err)
+	}
+	if err := r.Close(); err != nil {
+		t.Fatal(err)
+	}
+	return string(data)
+}
+
 func TestRunConfig_EmptyArgs(t *testing.T) {
 	err := runConfig(nil)
 	if err != nil {
diff --git a/cmd/opencodereview/flags.go b/cmd/opencodereview/flags.go
index 18250ed7..753a84ce 100644
--- a/cmd/opencodereview/flags.go
+++ b/cmd/opencodereview/flags.go
@@ -285,7 +285,7 @@ func parseConfigArgs(args []string) (configAction, error) {
 		}, nil
 	case "unset":
 		if len(args) < 2 {
-			return configAction{}, fmt.Errorf("usage: ocr config unset custom_providers.<name> | mcp_servers.<name>\ne.g., ocr config unset custom_providers.my-gateway\ne.g., ocr config unset mcp_servers.codegraph")
+			return configAction{}, fmt.Errorf("usage: ocr config unset <provider|custom_providers.<name>|mcp_servers.<name>>\nexamples:\n  ocr config unset provider\n  ocr config unset custom_providers.my-provider\n  ocr config unset mcp_servers.github")
 		}
 		return configAction{
 			subCmd: "unset",
@@ -301,6 +301,7 @@ func printConfigUsage() {
 
 Usage:
   ocr config set <key> <value>
+  ocr config unset provider                    Disable provider-based configuration
   ocr config unset custom_providers.<name>  Delete a custom provider
   ocr config unset mcp_servers.<name>       Delete an MCP server
   ocr config provider                       Interactive provider setup
@@ -329,6 +330,9 @@ Examples:
   # Delete a custom provider
   ocr config unset custom_providers.my-gateway
 
+  # Disable provider-based configuration and use legacy llm.* settings
+  ocr config unset provider
+
   # MCP server configuration (stdio transport)
   ocr config set mcp_servers.codegraph.command npx
   ocr config set mcp_servers.codegraph.args '["-y","@anthropic/codegraph-mcp"]'
@@ -351,8 +355,8 @@ Examples:
   ocr config set language English
   ocr config set telemetry.enabled true
 
-Supported keys: provider, model, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_header, llm.model, llm.protocol, llm.use_anthropic, llm.extra_body, llm.extra_headers, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging
-Provider fields: api_key, url, protocol, model, models, auth_header, extra_body, extra_headers
+Supported keys: provider, model, providers.<name>.<field>, custom_providers.<name>.<field>, mcp_servers.<name>.<field>, llm.url, llm.auth_token, llm.auth_token_cmd, llm.auth_header, llm.model, llm.protocol, llm.use_anthropic, llm.extra_body, llm.extra_headers, language, telemetry.enabled, telemetry.exporter, telemetry.otlp_endpoint, telemetry.content_logging
+Provider fields: api_key, api_key_cmd, url, protocol, model, models, auth_header, extra_body, extra_headers
 Protocol values: anthropic, openai, openai-responses
 MCP server fields: type, command, args, env, url, headers, tools, setup`)
 }
diff --git a/cmd/opencodereview/flags_test.go b/cmd/opencodereview/flags_test.go
index 55a8b9df..f036e714 100644
--- a/cmd/opencodereview/flags_test.go
+++ b/cmd/opencodereview/flags_test.go
@@ -1,6 +1,8 @@
 package main
 
 import (
+	"slices"
+	"strings"
 	"testing"
 	"time"
 )
@@ -198,6 +200,11 @@ func TestParseConfigArgs_UnsetMissingKey(t *testing.T) {
 	if err == nil {
 		t.Fatal("expected error for missing key")
 	}
+	for _, example := range []string{"ocr config unset provider", "ocr config unset custom_providers.my-provider", "ocr config unset mcp_servers.github"} {
+		if !strings.Contains(err.Error(), example) {
+			t.Errorf("error missing example %q: %v", example, err)
+		}
+	}
 }
 
 func TestParseConfigArgs_UnknownSubCmd(t *testing.T) {
@@ -226,6 +233,58 @@ func TestPrintDefaults(t *testing.T) {
 	fs.PrintDefaults()
 }
 
+// configFieldList returns the comma-separated names that follow prefix on the
+// one line of text starting with it.
+func configFieldList(t *testing.T, text, prefix string) []string {
+	t.Helper()
+	for _, line := range strings.Split(text, "\n") {
+		if !strings.HasPrefix(line, prefix) {
+			continue
+		}
+		var out []string
+		for _, field := range strings.Split(strings.TrimPrefix(line, prefix), ",") {
+			if field = strings.TrimSpace(field); field != "" {
+				out = append(out, field)
+			}
+		}
+		return out
+	}
+	t.Fatalf("no line starting with %q in:\n%s", prefix, text)
+	return nil
+}
+
+// These four lists are duplicated verbatim in printConfigUsage (what `ocr config`
+// and `ocr config --help` print) and in setConfigValue's unknown-key error.
+// api_key_cmd and llm.auth_token_cmd were added to the second copy and missed in
+// the first, so the primary discovery surface silently disagreed with the code.
+// Compared in order, since both copies are meant to be identical text.
+func TestPrintConfigUsage_ListsMatchSetConfigValueError(t *testing.T) {
+	usage := captureStdout(t, printConfigUsage)
+
+	err := setConfigValue(&Config{}, "definitely.not.a.key", "")
+	if err == nil {
+		t.Fatal("setConfigValue should reject an unknown key")
+	}
+	canonical := err.Error()
+
+	prefixes := []string{
+		"Supported keys: ",
+		"Provider fields: ",
+		"Protocol values: ",
+		"MCP server fields: ",
+	}
+	for _, prefix := range prefixes {
+		t.Run(strings.TrimSuffix(prefix, ": "), func(t *testing.T) {
+			want := configFieldList(t, canonical, prefix)
+			got := configFieldList(t, usage, prefix)
+			if !slices.Equal(got, want) {
+				t.Errorf("%q drifted between flags.go and config_cmd.go\n  flags.go:      %v\n  config_cmd.go: %v",
+					prefix, got, want)
+			}
+		})
+	}
+}
+
 func TestExpandShortFlags(t *testing.T) {
 	m := map[string]string{"c": "commit", "f": "format"}
 	tests := []struct {
diff --git a/cmd/opencodereview/provider_cmd.go b/cmd/opencodereview/provider_cmd.go
index f67da930..2a83abb3 100644
--- a/cmd/opencodereview/provider_cmd.go
+++ b/cmd/opencodereview/provider_cmd.go
@@ -235,13 +235,16 @@ func applyOfficialProviderConfig(configPath string, cfg *Config, result provider
 
 	preset, isPreset := llm.LookupProvider(result.provider)
 
-	if result.apiKey == "" {
+	// Mirror the resolver's precedence (static api_key -> api_key_cmd -> env var):
+	// an already-configured api_key_cmd satisfies the requirement, so picking a
+	// model for such a provider must not fail and abandon the save.
+	if result.apiKey == "" && cfg.Providers[result.provider].APIKeyCmd == "" {
 		if isPreset && preset.EnvVar != "" {
 			if os.Getenv(preset.EnvVar) == "" {
-				return fmt.Errorf("API key is required for provider %s (configure it or set $%s)", result.provider, preset.EnvVar)
+				return fmt.Errorf("API key is required for provider %s (configure it, set providers.%s.api_key_cmd, or set $%s)", result.provider, result.provider, preset.EnvVar)
 			}
 		} else {
-			return fmt.Errorf("API key is required for provider %s", result.provider)
+			return fmt.Errorf("API key is required for provider %s (configure it or set providers.%s.api_key_cmd)", result.provider, result.provider)
 		}
 	}
 
@@ -257,7 +260,8 @@ func applyOfficialProviderConfig(configPath string, cfg *Config, result provider
 	if result.apiKey != "" {
 		entry.APIKey = result.apiKey
 	} else {
-		// Confirmed empty key: clear saved api_key so resolver falls back to $ENV_VAR.
+		// Confirmed empty key: clear saved api_key so the resolver falls back to
+		// api_key_cmd (when set) or $ENV_VAR.
 		entry.APIKey = ""
 	}
 	cfg.Providers[result.provider] = entry
diff --git a/cmd/opencodereview/provider_cmd_test.go b/cmd/opencodereview/provider_cmd_test.go
index d1f03bba..a14a7d82 100644
--- a/cmd/opencodereview/provider_cmd_test.go
+++ b/cmd/opencodereview/provider_cmd_test.go
@@ -5,9 +5,37 @@ import (
 	"io"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 )
 
+// isolateLLMConnectionTest keeps the "Testing connection..." step that ends
+// every apply*Config call away from the developer's own machine. Without it
+// resolveConfigPath() falls back to ~/.opencodereview/config.json and `go test`
+// resolves a real endpoint: with providers.<name>.api_key_cmd configured that
+// runs the credential helper and blocks on a pinentry/Touch ID prompt for up to
+// the 60s credential timeout, and with a static key it fires a real request.
+//
+// The path points at a file that does not exist, so resolution fails fast the
+// way it already does on a machine with no config. HOME is redirected into an
+// empty temp dir as well, so the shell-rc strategy has nothing to read either.
+func isolateLLMConnectionTest(t *testing.T) {
+	t.Helper()
+	dir := t.TempDir()
+	t.Setenv("OCR_CONFIG_PATH", filepath.Join(dir, "no-such-config.json"))
+	// Both, because os.UserHomeDir reads USERPROFILE on Windows and never falls
+	// back to HOME -- setting HOME alone would leave the shell-rc strategy reading
+	// the real profile.
+	t.Setenv("HOME", dir)
+	t.Setenv("USERPROFILE", dir)
+	for _, k := range []string{
+		"OCR_LLM_URL", "OCR_LLM_TOKEN", "OCR_LLM_MODEL",
+		"ANTHROPIC_BASE_URL", "ANTHROPIC_AUTH_TOKEN", "ANTHROPIC_MODEL",
+	} {
+		t.Setenv(k, "")
+	}
+}
+
 func TestMaskKey(t *testing.T) {
 	tests := []struct {
 		name string
@@ -47,8 +75,12 @@ func TestSaveConfig(t *testing.T) {
 	if err != nil {
 		t.Fatalf("stat: %v", err)
 	}
-	if perm := info.Mode().Perm(); perm != 0o600 {
-		t.Errorf("perm = %o, want 600", perm)
+	// Windows reports 0666 regardless of the mode passed to OpenFile, so only the
+	// unix arms can assert the 0600 the config file is written with.
+	if runtime.GOOS != "windows" {
+		if perm := info.Mode().Perm(); perm != 0o600 {
+			t.Errorf("perm = %o, want 600", perm)
+		}
 	}
 
 	data, err := os.ReadFile(path)
@@ -206,6 +238,7 @@ func TestApplyOfficialProviderConfig_MissingFields(t *testing.T) {
 }
 
 func TestApplyOfficialProviderConfig_EmptyKeyClearsSavedAPIKey(t *testing.T) {
+	isolateLLMConnectionTest(t)
 	t.Setenv("DEEPSEEK_API_KEY", "sk-from-env")
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
@@ -240,7 +273,41 @@ func TestApplyOfficialProviderConfig_EmptyKeyClearsSavedAPIKey(t *testing.T) {
 	}
 }
 
+// A provider configured with only api_key_cmd must survive a trip through the
+// TUI: picking a model returns an empty apiKey, which must not be mistaken for
+// "no credential" and abandon the save.
+func TestApplyOfficialProviderConfig_APIKeyCmdSatisfiesRequirement(t *testing.T) {
+	isolateLLMConnectionTest(t)
+	t.Setenv("DEEPSEEK_API_KEY", "")
+	configPath := filepath.Join(t.TempDir(), "config.json")
+	cfg := &Config{
+		Providers: map[string]ProviderEntry{
+			"deepseek": {APIKeyCmd: "op read op://dev/deepseek/api-key"},
+		},
+	}
+
+	err := applyOfficialProviderConfig(configPath, cfg, providerTUIResult{
+		provider: "deepseek",
+		model:    "deepseek-v4-flash",
+		apiKey:   "",
+	})
+	if err != nil {
+		t.Fatalf("api_key_cmd should satisfy the API key requirement: %v", err)
+	}
+	diskCfg, err := loadOrCreateConfig(configPath)
+	if err != nil {
+		t.Fatalf("load config: %v", err)
+	}
+	if diskCfg.Provider != "deepseek" || diskCfg.Model != "deepseek-v4-flash" {
+		t.Errorf("save was abandoned: provider=%q model=%q", diskCfg.Provider, diskCfg.Model)
+	}
+	if got := diskCfg.Providers["deepseek"].APIKeyCmd; got != "op read op://dev/deepseek/api-key" {
+		t.Errorf("persisted api_key_cmd = %q, want it preserved", got)
+	}
+}
+
 func TestApplyCustomProviderConfig_EmptyKeyClearsSavedAPIKey(t *testing.T) {
+	isolateLLMConnectionTest(t)
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
 	cfg := &Config{
@@ -300,6 +367,7 @@ func TestProviderTUIResult_ResolvedModel(t *testing.T) {
 }
 
 func TestApplyOfficialProviderConfig_UsesSessionModelPick(t *testing.T) {
+	isolateLLMConnectionTest(t)
 	t.Setenv("QIANFAN_API_KEY", "sk-from-env")
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
diff --git a/cmd/opencodereview/provider_tui.go b/cmd/opencodereview/provider_tui.go
index b2ba1d17..bce95fa4 100644
--- a/cmd/opencodereview/provider_tui.go
+++ b/cmd/opencodereview/provider_tui.go
@@ -2,6 +2,7 @@ package main
 
 import (
 	"fmt"
+	"maps"
 	"os"
 	"sort"
 	"strings"
@@ -904,11 +905,36 @@ func officialProviderEnvKeySet(p llm.Provider) bool {
 	return p.EnvVar != "" && os.Getenv(p.EnvVar) != ""
 }
 
+// officialAPIKeyRequiredError mirrors the wording applyOfficialProviderConfig
+// uses for the same failure, so the interactive and non-interactive paths name
+// the same options in the same order (static key -> api_key_cmd -> env var).
 func officialAPIKeyRequiredError(p llm.Provider) string {
+	if p.Name == "" {
+		return "API key is required"
+	}
 	if p.EnvVar != "" {
-		return fmt.Sprintf("API key is required (or set $%s)", p.EnvVar)
+		return fmt.Sprintf("API key is required (configure it, set providers.%s.api_key_cmd, or set $%s)", p.Name, p.EnvVar)
+	}
+	return fmt.Sprintf("API key is required (configure it or set providers.%s.api_key_cmd)", p.Name)
+}
+
+// apiKeyCmdForStep returns the api_key_cmd already configured for the provider
+// the API-key step is editing, reading the same config entry loadExistingAPIKey
+// reads the static key from. The step serves the Official and Custom tabs; the
+// Manual tab has its own form and uses llm.auth_token_cmd instead.
+func (m providerTUIModel) apiKeyCmdForStep() string {
+	switch m.activeTab {
+	case tabOfficial:
+		if m.existingCfg == nil {
+			return ""
+		}
+		return m.existingCfg.Providers[m.currentProvider().Name].APIKeyCmd
+	case tabCustom:
+		if cp, ok := m.selectedCustomProvider(); ok {
+			return m.customProviderEntry(cp.name, cp.entry).APIKeyCmd
+		}
 	}
-	return "API key is required"
+	return ""
 }
 
 func (m providerTUIModel) apiKeyStepCanConfirm() (ok bool, errMsg string) {
@@ -918,6 +944,12 @@ func (m providerTUIModel) apiKeyStepCanConfirm() (ok bool, errMsg string) {
 	if !m.apiKeyMasked && strings.TrimSpace(m.apiKeyInput.Value()) != "" {
 		return true, ""
 	}
+	// Resolver precedence is static key -> api_key_cmd -> env var, so an already
+	// configured command satisfies the requirement: the field renders blank for
+	// such a provider and must still be confirmable.
+	if m.apiKeyCmdForStep() != "" {
+		return true, ""
+	}
 	if m.activeTab == tabOfficial {
 		p := m.currentProvider()
 		if officialProviderEnvKeySet(p) {
@@ -925,6 +957,9 @@ func (m providerTUIModel) apiKeyStepCanConfirm() (ok bool, errMsg string) {
 		}
 		return false, officialAPIKeyRequiredError(p)
 	}
+	if cp, ok := m.selectedCustomProvider(); ok && cp.name != "" {
+		return false, fmt.Sprintf("API key is required (configure it or set custom_providers.%s.api_key_cmd)", cp.name)
+	}
 	return false, "API key is required"
 }
 
@@ -1043,7 +1078,16 @@ func authHeaderFormError(raw string) string {
 	)
 }
 
-const manualAuthTokenRequiredError = "Auth token is required (whitespace-only input is not accepted)"
+const manualAuthTokenRequiredError = "Auth token is required (configure it or set llm.auth_token_cmd; whitespace-only input is not accepted)"
+
+// manualAuthTokenCmd returns the configured llm.auth_token_cmd, which the
+// resolver runs when llm.auth_token is empty.
+func (m providerTUIModel) manualAuthTokenCmd() string {
+	if m.existingCfg == nil {
+		return ""
+	}
+	return m.existingCfg.Llm.AuthTokenCmd
+}
 
 func (m providerTUIModel) handleCustomFormEnter() (tea.Model, tea.Cmd) {
 	switch m.cpStep {
@@ -1179,22 +1223,20 @@ func (m providerTUIModel) applyCreateCustomProvider() (tea.Model, tea.Cmd) {
 // map cloning) can safely mutate the returned value without aliasing the
 // original's slice or map fields.
 func cloneProviderEntry(v ProviderEntry) ProviderEntry {
-	out := ProviderEntry{
+	return ProviderEntry{
 		APIKey:     v.APIKey,
+		APIKeyCmd:  v.APIKeyCmd,
 		URL:        v.URL,
 		Protocol:   v.Protocol,
 		Model:      v.Model,
 		Models:     append([]string(nil), v.Models...),
 		AuthHeader: v.AuthHeader,
+		TimeoutSec: v.TimeoutSec,
+		// Shallow copy only: nested maps/slices inside a value are not cloned.
+		// maps.Clone keeps a nil map nil, matching the field's omitempty.
+		ExtraBody:    maps.Clone(v.ExtraBody),
+		ExtraHeaders: maps.Clone(v.ExtraHeaders),
 	}
-	if v.ExtraBody != nil {
-		out.ExtraBody = make(map[string]any, len(v.ExtraBody))
-		for k, val := range v.ExtraBody {
-			// Shallow copy only: nested maps/slices inside val are not cloned.
-			out.ExtraBody[k] = val
-		}
-	}
-	return out
 }
 
 func cloneCustomProvidersMap(src map[string]ProviderEntry) map[string]ProviderEntry {
@@ -1606,7 +1648,9 @@ func (m providerTUIModel) handleManualFormEnter() (tea.Model, tea.Cmd) {
 		m.manualStep = manualStepAuthToken
 		return m, m.manualTokenInput.Focus()
 	case manualStepAuthToken:
-		if strings.TrimSpace(m.manualTokenInput.Value()) == "" && m.manualTokenOriginal == "" {
+		// Same precedence as the provider tabs: an already configured
+		// llm.auth_token_cmd stands in for a typed or saved token.
+		if strings.TrimSpace(m.manualTokenInput.Value()) == "" && m.manualTokenOriginal == "" && m.manualAuthTokenCmd() == "" {
 			m.formError = manualAuthTokenRequiredError
 			return m, nil
 		}
@@ -1908,7 +1952,10 @@ func (m providerTUIModel) result() providerTUIResult {
 		return providerTUIResult{}
 
 	case tabManual:
-		apiKey := m.manualTokenInput.Value()
+		// Trim like the Official and Custom tabs: a whitespace-only token must
+		// never persist, or it wins precedence over a working auth_token_cmd
+		// and sends "Authorization: Bearer  ".
+		apiKey := strings.TrimSpace(m.manualTokenInput.Value())
 		if m.manualTokenMasked || (apiKey == "" && m.manualTokenOriginal != "") {
 			apiKey = m.manualTokenOriginal
 		}
@@ -2213,6 +2260,9 @@ func (m providerTUIModel) viewManualTab(s *strings.Builder) {
 				if m.manualTokenMasked && m.manualTokenOriginal != "" {
 					s.WriteString(tuiDimStyle.Render("    "+savedSecretReplaceHint(m.manualTokenOriginal)) + "\n")
 				}
+				if cmd := m.manualAuthTokenCmd(); cmd != "" {
+					s.WriteString(tuiDimStyle.Render(keyCmdConfiguredHintLine("    ", "llm.auth_token_cmd", cmd)) + "\n")
+				}
 			case manualStepAuthHeader:
 				s.WriteString("    " + m.manualAuthHeaderInput.View() + "\n")
 			}
@@ -2315,6 +2365,14 @@ func (m providerTUIModel) viewAPIKey(s *strings.Builder) {
 		s.WriteString("\n")
 	}
 
+	// Mirrors the env-var hint below: the step is already satisfied, so say so
+	// rather than leaving an empty field that looks unconfigured.
+	if cmd := m.apiKeyCmdForStep(); cmd != "" {
+		s.WriteString("\n")
+		s.WriteString(tuiDimStyle.Render(keyCmdConfiguredHintLine("  ", "api_key_cmd", cmd)))
+		s.WriteString("\n")
+	}
+
 	if m.activeTab == tabOfficial {
 		provider := m.currentProvider()
 		if envKey := os.Getenv(provider.EnvVar); envKey != "" {
@@ -2393,6 +2451,20 @@ func officialAPIKeyEnvSetHintLine(envVar string, hasSavedKey bool) string {
 	return "  " + officialAPIKeyEnvSetHint(envVar, hasSavedKey)
 }
 
+// keyCmdConfiguredHint explains why this step accepts an empty field. A
+// provider configured only by command renders a blank input -- the command line
+// is not the secret, but it is also not the value being edited here -- so
+// without this the user has no way to tell a credential is already wired up,
+// and no way to know that leaving the field empty is the correct action.
+// keyLabel names the config key so the hint points at what to edit instead.
+func keyCmdConfiguredHint(keyLabel, cmd string) string {
+	return fmt.Sprintf("%s is set (%s); leave empty to keep using it.", keyLabel, cmd)
+}
+
+func keyCmdConfiguredHintLine(indent, keyLabel, cmd string) string {
+	return indent + keyCmdConfiguredHint(keyLabel, cmd)
+}
+
 // --- Styles ---
 
 const tuiCursor = "▸"
diff --git a/cmd/opencodereview/provider_tui_funcs_test.go b/cmd/opencodereview/provider_tui_funcs_test.go
index 1462b3a7..61765044 100644
--- a/cmd/opencodereview/provider_tui_funcs_test.go
+++ b/cmd/opencodereview/provider_tui_funcs_test.go
@@ -3,6 +3,7 @@ package main
 import (
 	"os"
 	"path/filepath"
+	"reflect"
 	"strings"
 	"testing"
 
@@ -198,18 +199,26 @@ func TestRenderListName_Inactive(t *testing.T) {
 func TestCloneProviderEntry_WithExtraBody(t *testing.T) {
 	orig := ProviderEntry{
 		APIKey:     "key",
+		APIKeyCmd:  "op read op://dev/anthropic/api-key",
 		URL:        "http://localhost",
 		Protocol:   "openai",
 		Model:      "gpt-4",
 		Models:     []string{"gpt-4", "gpt-3.5"},
 		AuthHeader: "Authorization",
+		TimeoutSec: 45,
 		ExtraBody:  map[string]any{"temperature": 0.7, "stream": true},
+		ExtraHeaders: map[string]string{
+			"X-Trace": "on",
+		},
 	}
 	clone := cloneProviderEntry(orig)
 
 	if clone.APIKey != orig.APIKey || clone.URL != orig.URL || clone.Protocol != orig.Protocol {
 		t.Error("basic fields not copied")
 	}
+	if clone.APIKeyCmd != orig.APIKeyCmd {
+		t.Errorf("APIKeyCmd not copied: got %q, want %q", clone.APIKeyCmd, orig.APIKeyCmd)
+	}
 	if len(clone.Models) != 2 || clone.Models[0] != "gpt-4" {
 		t.Errorf("Models not cloned: %v", clone.Models)
 	}
@@ -229,6 +238,22 @@ func TestCloneProviderEntry_WithExtraBody(t *testing.T) {
 	if len(orig.Models) != 2 {
 		t.Error("modifying clone should not affect original Models")
 	}
+
+	if clone.TimeoutSec != orig.TimeoutSec {
+		t.Errorf("TimeoutSec not copied: got %d, want %d", clone.TimeoutSec, orig.TimeoutSec)
+	}
+	if clone.ExtraHeaders == nil {
+		// Fatal, not Error: writing to the nil map below would panic instead of
+		// reporting which field was dropped.
+		t.Fatal("ExtraHeaders should not be nil")
+	}
+	if clone.ExtraHeaders["X-Trace"] != "on" {
+		t.Errorf("ExtraHeaders not copied: %v", clone.ExtraHeaders)
+	}
+	clone.ExtraHeaders["X-New"] = "1"
+	if _, ok := orig.ExtraHeaders["X-New"]; ok {
+		t.Error("modifying clone should not affect original ExtraHeaders")
+	}
 }
 
 func TestCloneProviderEntry_NilExtraBody(t *testing.T) {
@@ -240,6 +265,42 @@ func TestCloneProviderEntry_NilExtraBody(t *testing.T) {
 	if clone.ExtraBody != nil {
 		t.Error("ExtraBody should remain nil")
 	}
+	if clone.ExtraHeaders != nil {
+		t.Error("ExtraHeaders should remain nil")
+	}
+}
+
+// cloneProviderEntry lists fields by hand, which is how timeout_sec and
+// extra_headers came to be silently dropped on the save-rollback paths. This
+// fails when a field is added to ProviderEntry but not to the clone: the
+// non-zero check forces the fixture to grow, and DeepEqual then catches the
+// omission. It catches a dropped field, not an aliased one -- DeepEqual
+// compares values, not identity; the sibling test above covers aliasing.
+func TestCloneProviderEntry_CopiesEveryField(t *testing.T) {
+	orig := ProviderEntry{
+		APIKey:       "key",
+		APIKeyCmd:    "op read op://dev/x/api-key",
+		URL:          "http://localhost",
+		Protocol:     "openai",
+		Model:        "gpt-4",
+		Models:       []string{"gpt-4"},
+		AuthHeader:   "Authorization",
+		TimeoutSec:   45,
+		ExtraBody:    map[string]any{"temperature": 0.7},
+		ExtraHeaders: map[string]string{"X-Trace": "on"},
+	}
+
+	rv := reflect.ValueOf(orig)
+	for i := range rv.NumField() {
+		if rv.Field(i).IsZero() {
+			t.Fatalf("fixture leaves %s zero-valued; set it so the clone is actually checked",
+				rv.Type().Field(i).Name)
+		}
+	}
+
+	if clone := cloneProviderEntry(orig); !reflect.DeepEqual(clone, orig) {
+		t.Errorf("clone dropped a field:\n got %+v\nwant %+v", clone, orig)
+	}
 }
 
 func TestCustomListCount(t *testing.T) {
@@ -1776,83 +1837,198 @@ func TestProviderTUI_ResultUsesSessionModelPickWhenSelectionEmpty(t *testing.T)
 	}
 }
 
-func TestApiKeyStepCanConfirm_OfficialEmptyWithoutEnv(t *testing.T) {
-	t.Setenv("DEEPSEEK_API_KEY", "")
-	cfg := &Config{
-		Provider: "deepseek",
-		Model:    "deepseek-v4-flash",
-		Providers: map[string]ProviderEntry{
-			"deepseek": {Model: "deepseek-v4-flash"},
+// apiKeyStepCanConfirm gates the final Enter of `ocr config provider`. It has to
+// mirror the resolver's precedence (static api_key -> api_key_cmd -> env var):
+// a provider configured with only api_key_cmd renders a blank key field, and
+// blocking it there made the feature unreachable from the documented wizard.
+func TestApiKeyStepCanConfirm(t *testing.T) {
+	tests := []struct {
+		name       string
+		env        string
+		cfg        *Config
+		customTab  bool
+		typedKey   string
+		wantOK     bool
+		wantErrMsg string
+	}{
+		{
+			name: "official saved api_key",
+			cfg: &Config{
+				Provider:  "deepseek",
+				Providers: map[string]ProviderEntry{"deepseek": {APIKey: "keep-me"}},
+			},
+			wantOK: true,
 		},
-	}
-	m := newProviderTUI(cfg, "")
-	m.activeTab = tabOfficial
-	m.step = stepAPIKey
-
-	ok, errMsg := m.apiKeyStepCanConfirm()
-	if ok {
-		t.Fatal("expected confirmation to be blocked")
-	}
-	if errMsg != "API key is required (or set $DEEPSEEK_API_KEY)" {
-		t.Errorf("errMsg = %q", errMsg)
-	}
-}
-
-func TestApiKeyStepCanConfirm_OfficialEmptyWithEnv(t *testing.T) {
-	t.Setenv("DEEPSEEK_API_KEY", "sk-from-env")
-	cfg := &Config{
-		Provider: "deepseek",
-		Model:    "deepseek-v4-flash",
-		Providers: map[string]ProviderEntry{
-			"deepseek": {Model: "deepseek-v4-flash"},
+		{
+			name:     "official typed key",
+			cfg:      &Config{Provider: "deepseek", Providers: map[string]ProviderEntry{"deepseek": {}}},
+			typedKey: "sk-typed",
+			wantOK:   true,
 		},
-	}
-	m := newProviderTUI(cfg, "")
-	m.activeTab = tabOfficial
-	m.step = stepAPIKey
-
-	ok, errMsg := m.apiKeyStepCanConfirm()
-	if !ok {
-		t.Fatalf("expected confirmation allowed, errMsg = %q", errMsg)
-	}
-}
-
-func TestApiKeyStepCanConfirm_CustomEmpty(t *testing.T) {
-	cfg := &Config{
-		Provider: "stepfun",
-		CustomProviders: map[string]ProviderEntry{
-			"stepfun": {APIKey: ""},
+		{
+			name: "official api_key_cmd only",
+			cfg: &Config{
+				Provider:  "deepseek",
+				Providers: map[string]ProviderEntry{"deepseek": {APIKeyCmd: "op read op://dev/deepseek/api-key"}},
+			},
+			wantOK: true,
+		},
+		{
+			name:       "official nothing configured",
+			cfg:        &Config{Provider: "deepseek", Providers: map[string]ProviderEntry{"deepseek": {}}},
+			wantOK:     false,
+			wantErrMsg: "API key is required (configure it, set providers.deepseek.api_key_cmd, or set $DEEPSEEK_API_KEY)",
+		},
+		{
+			name:   "official env var set",
+			env:    "sk-from-env",
+			cfg:    &Config{Provider: "deepseek", Providers: map[string]ProviderEntry{"deepseek": {}}},
+			wantOK: true,
+		},
+		{
+			name:      "custom saved api_key",
+			customTab: true,
+			cfg: &Config{
+				Provider:        "stepfun",
+				CustomProviders: map[string]ProviderEntry{"stepfun": {APIKey: "sk-custom"}},
+			},
+			wantOK: true,
+		},
+		{
+			name:      "custom api_key_cmd only",
+			customTab: true,
+			cfg: &Config{
+				Provider:        "stepfun",
+				CustomProviders: map[string]ProviderEntry{"stepfun": {APIKeyCmd: "op read op://dev/stepfun/api-key"}},
+			},
+			wantOK: true,
+		},
+		{
+			name:       "custom nothing configured",
+			customTab:  true,
+			cfg:        &Config{Provider: "stepfun", CustomProviders: map[string]ProviderEntry{"stepfun": {}}},
+			wantOK:     false,
+			wantErrMsg: "API key is required (configure it or set custom_providers.stepfun.api_key_cmd)",
 		},
 	}
-	m := newProviderTUI(cfg, "")
-	m.activeTab = tabCustom
-	m.customIdx = 0
-	m.step = stepAPIKey
 
-	ok, errMsg := m.apiKeyStepCanConfirm()
-	if ok {
-		t.Fatal("expected confirmation to be blocked")
-	}
-	if errMsg != "API key is required" {
-		t.Errorf("errMsg = %q", errMsg)
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			t.Setenv("DEEPSEEK_API_KEY", tc.env)
+			m := newProviderTUI(tc.cfg, "")
+			if tc.customTab {
+				m.activeTab = tabCustom
+				m.customIdx = 0
+			} else {
+				m.activeTab = tabOfficial
+			}
+			m.step = stepAPIKey
+			// loadExistingAPIKey is what the wizard runs on entering the step, and
+			// is the only thing that populates apiKeyOriginal / the mask.
+			m.loadExistingAPIKey()
+			if tc.typedKey != "" {
+				m.apiKeyInput.SetValue(tc.typedKey)
+			}
+
+			ok, errMsg := m.apiKeyStepCanConfirm()
+			if ok != tc.wantOK {
+				t.Fatalf("apiKeyStepCanConfirm() ok = %v, want %v (errMsg = %q)", ok, tc.wantOK, errMsg)
+			}
+			if errMsg != tc.wantErrMsg {
+				t.Errorf("errMsg = %q, want %q", errMsg, tc.wantErrMsg)
+			}
+		})
 	}
 }
 
-func TestApiKeyStepCanConfirm_MaskedSavedKey(t *testing.T) {
-	cfg := &Config{
-		Provider: "deepseek",
-		Providers: map[string]ProviderEntry{
-			"deepseek": {APIKey: "keep-me"},
+// The Manual tab's auth-token gate is the legacy twin of apiKeyStepCanConfirm:
+// llm.auth_token_cmd has to stand in for an empty field the same way.
+func TestHandleManualFormEnter_AuthTokenGate(t *testing.T) {
+	tests := []struct {
+		name        string
+		llmCfg      LlmConfig
+		typedToken  string
+		wantAdvance bool
+		// wantAPIKey is the token result() must persist once the step confirms.
+		wantAPIKey string
+	}{
+		{
+			name:        "saved auth_token",
+			llmCfg:      LlmConfig{URL: "http://existing", Model: "m", AuthToken: "tok-saved"},
+			wantAdvance: true,
+			wantAPIKey:  "tok-saved",
+		},
+		{
+			name:        "typed token",
+			llmCfg:      LlmConfig{URL: "http://existing", Model: "m"},
+			typedToken:  "tok-typed",
+			wantAdvance: true,
+			wantAPIKey:  "tok-typed",
+		},
+		{
+			name:        "auth_token_cmd only",
+			llmCfg:      LlmConfig{URL: "http://existing", Model: "m", AuthTokenCmd: "op read op://dev/gw/token"},
+			wantAdvance: true,
+		},
+		{
+			// auth_token_cmd opens the gate, so whitespace typed at this step
+			// confirms. It must not be saved as auth_token: a non-empty token
+			// wins precedence and would silently shadow the working command.
+			name:        "auth_token_cmd with whitespace-only token",
+			llmCfg:      LlmConfig{URL: "http://existing", Model: "m", AuthTokenCmd: "op read op://dev/gw/token"},
+			typedToken:  " ",
+			wantAdvance: true,
+		},
+		{
+			name:        "nothing configured",
+			llmCfg:      LlmConfig{URL: "http://existing", Model: "m"},
+			wantAdvance: false,
+		},
+		{
+			name:        "whitespace-only token",
+			llmCfg:      LlmConfig{URL: "http://existing", Model: "m"},
+			typedToken:  "   ",
+			wantAdvance: false,
 		},
 	}
-	m := newProviderTUI(cfg, "")
-	m.activeTab = tabOfficial
-	m.step = stepAPIKey
-	m.loadExistingAPIKey()
 
-	ok, errMsg := m.apiKeyStepCanConfirm()
-	if !ok {
-		t.Fatalf("expected confirmation allowed, errMsg = %q", errMsg)
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			m := newProviderTUI(&Config{Llm: tc.llmCfg}, "")
+			m.activeTab = tabManual
+			m.inManualForm = true
+			m.manualStep = manualStepAuthToken
+			if tc.typedToken != "" {
+				m.manualTokenInput.SetValue(tc.typedToken)
+			}
+
+			result, _ := m.handleManualFormEnter()
+			m2 := result.(providerTUIModel)
+
+			if tc.wantAdvance {
+				if m2.manualStep != manualStepAuthHeader {
+					t.Fatalf("manualStep = %d, want manualStepAuthHeader (%d); formError = %q",
+						m2.manualStep, manualStepAuthHeader, m2.formError)
+				}
+				if m2.formError != "" {
+					t.Errorf("formError = %q, want empty", m2.formError)
+				}
+				if got := m2.result().apiKey; got != tc.wantAPIKey {
+					t.Errorf("result().apiKey = %q, want %q", got, tc.wantAPIKey)
+				}
+				return
+			}
+			if m2.manualStep != manualStepAuthToken {
+				t.Fatalf("manualStep = %d, want to stay on manualStepAuthToken (%d)",
+					m2.manualStep, manualStepAuthToken)
+			}
+			if m2.formError != manualAuthTokenRequiredError {
+				t.Errorf("formError = %q, want %q", m2.formError, manualAuthTokenRequiredError)
+			}
+			if !strings.Contains(m2.formError, "llm.auth_token_cmd") {
+				t.Errorf("formError should name llm.auth_token_cmd, got %q", m2.formError)
+			}
+		})
 	}
 }
 
diff --git a/cmd/opencodereview/provider_tui_test.go b/cmd/opencodereview/provider_tui_test.go
index 69d41821..c20cd16b 100644
--- a/cmd/opencodereview/provider_tui_test.go
+++ b/cmd/opencodereview/provider_tui_test.go
@@ -2291,8 +2291,10 @@ func TestProviderTUI_OfficialApiKeyEmptyWithoutEnvBlocksEnter(t *testing.T) {
 	if m2.step != stepAPIKey {
 		t.Errorf("step = %d, want stepAPIKey", m2.step)
 	}
-	if m2.formError != "API key is required (or set $DASHSCOPE_API_KEY)" {
-		t.Errorf("formError = %q", m2.formError)
+	// The exact prose is pinned by TestApiKeyStepCanConfirm; this test covers the
+	// Enter-key wiring, so compare against the helper and never drift again.
+	if want := officialAPIKeyRequiredError(m2.currentProvider()); m2.formError != want {
+		t.Errorf("formError = %q, want %q", m2.formError, want)
 	}
 	if cmd != nil {
 		t.Error("Enter without key or env should not quit")
@@ -2354,8 +2356,10 @@ func TestProviderTUI_CustomExistingApiKeyEmptyBlocksEnter(t *testing.T) {
 	if m2.step != stepAPIKey {
 		t.Errorf("step = %d, want stepAPIKey", m2.step)
 	}
-	if m2.formError != "API key is required" {
-		t.Errorf("formError = %q, want %q", m2.formError, "API key is required")
+	// Prefix, not the full string: this test covers Enter-key gating, and the
+	// exact wording is pinned by TestApiKeyStepCanConfirm.
+	if !strings.HasPrefix(m2.formError, "API key is required") {
+		t.Errorf("formError = %q, want it to start with %q", m2.formError, "API key is required")
 	}
 	if cmd != nil {
 		t.Error("Enter with cleared key should not quit")
@@ -2600,6 +2604,7 @@ func TestProviderTUI_DeleteModelPreservesActiveModel(t *testing.T) {
 }
 
 func TestApplyCustomProviderConfigPreservesModelOrder(t *testing.T) {
+	isolateLLMConnectionTest(t)
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
 	models := []string{"test-model", "test-model-2", "bbb", "aaa", "test-model-3"}
@@ -2643,6 +2648,7 @@ func TestApplyCustomProviderConfigPreservesModelOrder(t *testing.T) {
 }
 
 func TestApplyManualConfigNormalizesAuthHeader(t *testing.T) {
+	isolateLLMConnectionTest(t)
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
 	cfg := &Config{}
@@ -2668,6 +2674,7 @@ func TestApplyManualConfigNormalizesAuthHeader(t *testing.T) {
 }
 
 func TestApplyCustomProviderConfigNormalizesAuthHeader(t *testing.T) {
+	isolateLLMConnectionTest(t)
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
 	cfg := &Config{
@@ -2816,6 +2823,7 @@ func TestEnterEditCustomProvider_ProtocolIndex(t *testing.T) {
 // mirrored for the two protocols that have a boolean equivalent so older
 // binaries can still read the config.
 func TestApplyManualConfig_DoubleWritesProtocolAndUseAnthropic(t *testing.T) {
+	isolateLLMConnectionTest(t)
 	dir := t.TempDir()
 	configPath := filepath.Join(dir, "config.json")
 
@@ -2920,3 +2928,65 @@ func TestProviderTUIResult_ManualProtocolIsCanonical(t *testing.T) {
 		}
 	}
 }
+
+func TestKeyCmdConfiguredHint(t *testing.T) {
+	got := keyCmdConfiguredHint("api_key_cmd", "op read op://dev/anthropic/api-key")
+	want := "api_key_cmd is set (op read op://dev/anthropic/api-key); leave empty to keep using it."
+	if got != want {
+		t.Errorf("hint = %q, want %q", got, want)
+	}
+}
+
+// A provider configured only by command renders a blank API-key field, so
+// without this hint there is nothing on screen distinguishing "credential
+// already wired up" from "nothing configured".
+func TestProviderTUI_ViewAPIKey_ShowsAPIKeyCmdHint(t *testing.T) {
+	cfg := &Config{
+		Provider: "deepseek",
+		Model:    "deepseek-v4-flash",
+		Providers: map[string]ProviderEntry{
+			"deepseek": {APIKeyCmd: "op read op://dev/deepseek/key", Model: "deepseek-v4-flash"},
+		},
+	}
+	m := newProviderTUI(cfg, "")
+	m.activeTab = tabOfficial
+	for i, p := range m.providers {
+		if p.Name == "deepseek" {
+			m.officialIdx = i
+			break
+		}
+	}
+	m.step = stepAPIKey
+	m.loadExistingAPIKey()
+	m.apiKeyInput.Focus()
+
+	got := stripANSI(m.View().Content)
+	want := "api_key_cmd is set (op read op://dev/deepseek/key); leave empty to keep using it."
+	if !strings.Contains(got, want) {
+		t.Errorf("view missing api_key_cmd hint; want %q; got:\n%s", want, got)
+	}
+}
+
+func TestProviderTUI_ViewAPIKey_NoCmdHintWhenUnset(t *testing.T) {
+	cfg := &Config{
+		Provider: "deepseek",
+		Model:    "deepseek-v4-flash",
+		Providers: map[string]ProviderEntry{
+			"deepseek": {Model: "deepseek-v4-flash"},
+		},
+	}
+	m := newProviderTUI(cfg, "")
+	m.activeTab = tabOfficial
+	for i, p := range m.providers {
+		if p.Name == "deepseek" {
+			m.officialIdx = i
+			break
+		}
+	}
+	m.step = stepAPIKey
+	m.loadExistingAPIKey()
+
+	if got := stripANSI(m.View().Content); strings.Contains(got, "api_key_cmd is set") {
+		t.Errorf("view should not claim api_key_cmd is set when it is not; got:\n%s", got)
+	}
+}
diff --git a/internal/config/allowlist/allowed_ext_test.go b/internal/config/allowlist/allowed_ext_test.go
index 111d3bb4..aea7132b 100644
--- a/internal/config/allowlist/allowed_ext_test.go
+++ b/internal/config/allowlist/allowed_ext_test.go
@@ -17,6 +17,10 @@ func TestIsAllowedExt(t *testing.T) {
 		{".astro", true},
 		{".ASTRO", true},
 		{".py", true},
+		{".php", true},
+		{".PHP", true},
+		{".phtml", true},
+		{".PHTML", true},
 		{".rs", true},
 		{".ets", true},
 		{".ETS", true},
@@ -42,6 +46,8 @@ func TestIsAllowedExt(t *testing.T) {
 		{".TFVARS", true},
 		{".bicep", true},
 		{".BICEP", true},
+		{".proto", true},
+		{".PROTO", true},
 		{".txt", false},
 		{".md", false},
 		{".png", false},
diff --git a/internal/config/allowlist/supported_file_types.json b/internal/config/allowlist/supported_file_types.json
index 430e135e..c976805d 100644
--- a/internal/config/allowlist/supported_file_types.json
+++ b/internal/config/allowlist/supported_file_types.json
@@ -28,6 +28,7 @@
   ".rake",
   ".gemspec",
   ".php",
+  ".phtml",
   ".swift",
   ".m",
   ".mm",
@@ -76,5 +77,6 @@
   ".jl",
   ".hcl",
   ".tfvars",
-  ".bicep"
+  ".bicep",
+  ".proto"
 ]
diff --git a/internal/config/rules/rule_docs/composer_json.md b/internal/config/rules/rule_docs/composer_json.md
new file mode 100644
index 00000000..20747f15
--- /dev/null
+++ b/internal/config/rules/rule_docs/composer_json.md
@@ -0,0 +1,39 @@
+#### Composer Manifest Review Principles
+> Focus on newly introduced correctness, reproducibility, security, and deployment defects. Inspect source usage, CI, containers, deployment configuration, and nearby workspace manifests before claiming a dependency or platform incompatibility. Do not turn preferences about exact pins versus compatible ranges into findings.
+
+#### Dependency Constraints and Resolution
+- Wildcard constraints such as `*`, unconstrained `dev-*` branches, or mutable VCS references introduced without a committed, current lock file where application builds must be reproducible, or in a reusable library where consumers resolve dependencies themselves. Compatible version ranges are normal for libraries and should not be flagged by default.
+- A changed constraint that unintentionally permits an incompatible major version, excludes the repository's supported range, or conflicts with another direct requirement.
+- The same package declared inconsistently across `require` and `require-dev`, or a production package available only through development dependencies.
+- A newly used package or mandatory PHP extension absent from `require`, causing clean production installs to fail.
+- Do not report a known vulnerability without reliable advisory evidence applicable to the resolved version range.
+
+#### PHP and Platform Compatibility
+- The `php` constraint contradicts syntax or APIs used by the changed code, the framework's supported range, or the runtime configured in CI and deployment.
+- A required native extension missing from `ext-*` requirements, or an extension requirement made mandatory even though the code has a working optional fallback.
+- `config.platform` masking a runtime or extension mismatch that will occur in production. Confirm the actual deployment platform before reporting.
+- Composer or plugin API requirements incompatible with the Composer version used by CI, containers, or release tooling.
+
+#### Autoloading and Package Layout
+- Incorrect PSR-4 namespace prefixes or paths, overlapping prefixes that resolve the wrong class, or moved classes left unreachable by autoload configuration.
+- Production classes placed only in `autoload-dev`, or test-only helpers exposed through production autoloading when that changes packaged behavior.
+- `autoload.files` additions that execute side effects on every Composer bootstrap or rely on an unsafe initialization order.
+- Classmap, exclusion, or files entries left stale after directories are moved or renamed.
+
+#### Scripts and Plugin Execution
+- Lifecycle scripts that run destructive commands, interpolate untrusted environment values into a shell, require interactive input in CI, or invoke tools not available from declared dependencies.
+- Composer scripts that recursively invoke Composer or make production installation depend on development-only packages or local state.
+- A newly required Composer plugin without an intentional `config.allow-plugins` decision, or wildcard/broad authorization that permits unexpected plugin code to execute during install or update.
+- Do not flag scripts or plugins solely because they execute code; establish a concrete unsafe command, trust-boundary change, or installation failure.
+
+#### Repositories and Supply Chain
+- `secure-http` disabled, plaintext repository URLs, embedded credentials, or newly introduced package sources without appropriate integrity and access controls.
+- Repository priority or canonical settings that can cause a private/public package to resolve from an unintended source.
+- `package` or VCS repositories pointing to mutable or unverifiable artifacts where reproducible source selection is required.
+- Secrets, tokens, or private repository credentials exposed in committed manifest data. Report an internal URL only when the manifest is publicly distributed and the URL itself reveals sensitive infrastructure information.
+
+#### Stability, Package Semantics, and Release Metadata
+- `minimum-stability` weakened so unrelated development packages can enter resolution, especially without `prefer-stable`; verify whether a narrowly constrained development dependency would suffice.
+- Incorrect `replace`, `provide`, or `conflict` declarations that can make Composer omit a required implementation or accept an incompatible package.
+- Changes to `type`, `bin`, installer paths, archive include/exclude rules, or framework `extra` metadata that break installation or packaging.
+- Published packages missing or invalid required metadata only when the repository is actually distributed as a package; do not apply publishing requirements to private applications.
diff --git a/internal/config/rules/rule_docs/php.md b/internal/config/rules/rule_docs/php.md
new file mode 100644
index 00000000..c7775c89
--- /dev/null
+++ b/internal/config/rules/rule_docs/php.md
@@ -0,0 +1,62 @@
+#### PHP Review Principles
+> Favor precision over recall: report only defects that are likely real in the changed code and its reachable context. Treat correctness and security findings as blocking; style-only suggestions are non-blocking. Account for the project's PHP version and framework conventions before reporting version- or lifecycle-dependent behavior.
+
+Before making a non-local claim, use `file_read` and `code_search` to verify callers, input sources, framework configuration, template context, and resource ownership. Do not duplicate findings reliably enforced by PHPStan, Psalm, PHP_CodeSniffer, the formatter, or the PHP compiler unless the diff demonstrates a concrete consequence those tools do not express.
+
+#### Type Juggling, Equality, and Null Semantics
+- Loose comparison (`==` or `!=`) whose coercion can make distinct security- or domain-sensitive values compare equal. Prefer strict comparison when operands are expected to have the same type; do not flag deliberate, validated normalization.
+- Truthiness or `empty()` checks that incorrectly treat `0`, `"0"`, `false`, `null`, and an empty value as equivalent when those states have different meanings.
+- `isset()` used when a present key with a `null` value must be distinguished from a missing key; use `array_key_exists()` when presence, rather than non-nullness, is the contract.
+- Nullable, union, or `false`-returning APIs whose failure value reaches code that assumes a usable object, scalar, or resource. Confirm the declared and runtime contract before flagging.
+- Numeric-string, arithmetic, or comparison behavior that depends on a different PHP version from the one supported by `composer.json`, CI, or deployment configuration.
+
+#### Arrays, Iteration, and Value Semantics
+- Array keys read without handling a reachable missing-key path, especially request data, decoded JSON, database rows, or optional configuration.
+- A `foreach` value variable iterated by reference and then reused without `unset()`, leaving it aliased to the final element and allowing later assignments to corrupt the array.
+- Array union (`+`), `array_merge`, spread syntax, or numeric-key reindexing used with semantics different from the intended overwrite and ordering behavior.
+- Callbacks or closures that capture a loop variable by reference and later observe an unintended final or mutated value.
+- Mutation during iteration that can skip, duplicate, or unexpectedly retain elements. Do not flag mutation whose traversal behavior is deliberate and locally evident.
+
+#### Errors, Exceptions, and API Contracts
+- `Throwable` or `Exception` caught and silently discarded, converted into success, or replaced with a misleading default on a path where the failure matters.
+- Catching a broad exception around unrelated operations so the handler cannot distinguish the expected failure from a programming or infrastructure defect.
+- A codebase contract inconsistently mixing exceptions, `false`, and `null` for the same failure, causing callers to miss an error path.
+- Cleanup, rollback, or response-finalization code that hides the primary exception or returns success after the operation failed.
+- Warnings or errors suppressed with `@` where suppression can turn a meaningful failure into invalid state. Do not flag a narrowly documented compatibility probe that checks the result safely.
+
+#### Resources, Transactions, and Request Lifecycle
+- Transactions, locks, database cursors, or resources in long-running processes not released, committed, or rolled back on every reachable path when delayed cleanup can exhaust capacity or break correctness. Do not flag ordinary request-scoped streams or files merely because PHP can release them at request shutdown.
+- Database transactions with early returns or exception paths that can leave the transaction open, or nested transaction assumptions unsupported by the active driver/framework.
+- cURL or stream operations lacking timeouts on a request or worker path where a remote endpoint can stall execution.
+- Session locks held across slow network, database, or CPU work when concurrent requests for the same session must proceed.
+- Do not report resources owned by a framework, dependency-injection container, generator consumer, or caller when ownership transfer is established by the surrounding code.
+
+#### Database and ORM Correctness
+- SQL assembled from untrusted values instead of parameter binding. Identifiers such as column names and sort directions cannot usually be bound and require an allowlist.
+- Raw ORM expressions, query fragments, or dynamic table/column names that bypass the framework's normal parameterization with attacker-controlled data.
+- Missing transaction boundaries when a changed multi-step write must be atomic, or side effects ordered so a rollback cannot restore consistency.
+- N+1 queries or repeated remote calls only when the loop is reachable at meaningful scale and eager loading or batching preserves behavior.
+- Mass-assignment exposure only when request-controlled fields reach a model and the framework's fillable/guarded/schema configuration does not already constrain them.
+
+#### Web and Template Security Boundaries
+Confirm attacker control and the output or execution context before reporting. Framework validation and auto-escaping may make an otherwise dangerous-looking operation safe.
+
+- Untrusted output rendered without context-appropriate escaping for HTML text, attributes, URLs, JavaScript, or CSS. For `.phtml` templates, verify whether the view helper already escapes the value and whether raw HTML is intentional and sanitized.
+- Authorization enforced only in a client, template, or hidden control rather than at the server-side operation; check route middleware, policies, voters, and controller guards before flagging.
+- State-changing browser requests missing required CSRF protection when cookie-based authentication makes cross-site invocation possible. Do not flag token-authenticated APIs that are not vulnerable to ambient credentials.
+- Redirects, response headers, or cookies built from untrusted data without validation or appropriate `Secure`, `HttpOnly`, and `SameSite` protections where those properties are required.
+- File uploads trusted by client filename, extension, or MIME header alone; verify server-side type checks, generated storage names, destination boundaries, and executable-file handling.
+- User-controlled paths used for filesystem access without normalization and boundary enforcement, enabling traversal, symlink escape, or unintended overwrite.
+
+#### Dynamic Execution, Deserialization, and Outbound Requests
+- `eval`, dynamic `include`/`require`, variable function calls, reflection, or shell commands reached by untrusted input without a strict allowlist.
+- `unserialize()` on attacker-controlled data, including signed data where key management or verification is absent. Prefer a non-executable format; `allowed_classes` reduces object injection but does not make arbitrary data trustworthy.
+- Shell commands built through concatenation or incomplete escaping. Prefer direct process APIs with separate arguments and validate option-like attacker-controlled values.
+- Outbound URLs derived from untrusted input without required scheme, host, port, redirect, and private-network restrictions, enabling SSRF or credential forwarding.
+- Weak randomness or password handling: predictable token generation, reversible password storage, manual password hashing, or non-constant-time comparison of secrets. Prefer `random_bytes`, `password_hash`, `password_verify`, and `hash_equals` as appropriate.
+- Secrets, session identifiers, authorization headers, passwords, private keys, or sensitive personal data logged, returned in errors, or embedded in source.
+
+#### Performance and Review Scope
+- Report performance issues only with evidence of meaningful data scale or a hot path: repeated queries, accidental full-result materialization, quadratic array operations, or expensive work repeated inside a loop.
+- Suggest tests only for concrete changed failure modes involving coercion, boundary values, errors, transactions, authorization, escaping, or framework configuration.
+- Do not make formatting, naming, import ordering, modern-syntax preferences, or advice already enforced by deterministic PHP tooling into blocking findings.
diff --git a/internal/config/rules/rule_docs/protobuf.md b/internal/config/rules/rule_docs/protobuf.md
new file mode 100644
index 00000000..5d06a342
--- /dev/null
+++ b/internal/config/rules/rule_docs/protobuf.md
@@ -0,0 +1,39 @@
+> Favor precision over recall: only raise an issue when you are confident it is a real defect, and stay silent when the surrounding context is unclear — a false alarm costs more reviewer trust than a missed minor issue. Treat security and correctness findings as blocking, and style or idiom suggestions as non-blocking.
+
+#### Obvious Typos or Spelling Errors
+- Spelling errors in message, field, enum, enum-value, service, or rpc names at their declaration sites; do not report spelling errors at reference sites
+- Comments or option strings with spelling errors that affect readability of the public API surface
+
+#### Field Numbers and Wire Compatibility
+- Reused or renumbered field tags that break existing clients or servers (Wire Compatibility)
+- Changing a field's type, label (`optional`/`repeated`/`required`), or oneof membership in a way that breaks wire or JSON compatibility
+- Deleting a field without adding both its number and name to `reserved`
+- Renaming a field without `json_name` consideration when JSON clients depend on the old name
+- Do not flag purely additive new fields with fresh numbers, or documentation-only comment changes
+
+#### Message and Field Design
+- Missing `optional` (proto3) where absence must be distinguishable from the zero value
+- `map` used where order matters, or `repeated` used where key lookup would be clearer
+- oneof fields that leave an invalid zero-state representable when an explicit sentinel was intended
+- Nested messages that re-encode the same domain concept already modeled elsewhere in the package
+- Do not report stylistic preference for `message` vs `group` (groups are legacy) when the schema is already consistent
+
+#### Enums and Defaults
+- First enum value is not a zero `*_UNSPECIFIED` (or equivalent) sentinel
+- Relying on implicit zero defaults across schema versions when clients treat zero as meaningful data
+- Inserting new enum values in the middle of an existing numeric range used by older clients
+- Do not flag additive enum values appended at the end with new numbers
+
+#### Services and RPC Design
+- Non-idempotent methods modeled as if they were safe to retry without client-visible side effects
+- Multiple rpcs sharing the same request or response message type when distinct contracts would prevent accidental field coupling
+- Unbounded client/server streaming without documented flow control, page size, or deadline expectations
+- Missing request or response message wrappers that force primitive/scalar request bodies
+- Do not flag standard google.api annotations or well-known types used correctly
+
+#### Security and Resource Limits
+- `google.protobuf.Any` accepted from untrusted input without type allowlisting
+- Unbounded `repeated`/`map` fields or recursive message depth on untrusted payloads with no application-level limits
+- Secrets, tokens, or credentials embedded in field defaults, examples, or comments
+- File paths, URLs, or SQL fragments carried as unconstrained strings without validation guidance at the service boundary
+- Do not report when limits are enforced outside the schema and that boundary is clearly documented
diff --git a/internal/config/rules/system_rules.json b/internal/config/rules/system_rules.json
index 0b599b46..0dd10066 100644
--- a/internal/config/rules/system_rules.json
+++ b/internal/config/rules/system_rules.json
@@ -7,6 +7,7 @@
     "**/build.gradle": "build_gradle.md",
     "**/package.json": "package_json.md",
     "**/Cargo.toml": "cargo_toml.md",
+    "**/composer.json": "composer_json.md",
     "**/*.{json,json5}": "json.md",
     ".github/workflows/**/*.{yaml,yml}": "github_workflows.md",
     ".github/**/*.{yaml,yml}": "github_config.md",
@@ -22,6 +23,8 @@
     "**/*.{cpp,cc,hpp}": "cpp.md",
     "**/*.c": "c.md",
     "**/*.py": "python.md",
+    "**/*.{php,phtml}": "php.md",
+    "**/*.proto": "protobuf.md",
     "**/*.po": "po.md",
     "**/*.pot": "pot.md",
     "**/*.{graphql,gql}": "graphql.md",
diff --git a/internal/config/rules/system_rules_test.go b/internal/config/rules/system_rules_test.go
index ed7bc926..dd978162 100644
--- a/internal/config/rules/system_rules_test.go
+++ b/internal/config/rules/system_rules_test.go
@@ -69,6 +69,8 @@ func TestResolve_DefaultRules(t *testing.T) {
 		{"submodule/pom.xml", "snapshot"},
 		{"src/main/resources/application.properties", "Configuration Error Detection"},
 		{"frontend/package.json", "latest"},
+		{"composer.json", "Composer Manifest Review Principles"},
+		{"packages/library/composer.json", "Dependency Constraints and Resolution"},
 		{"config/app.yaml", "yaml-key"},
 		{"deploy/values.yml", "yaml-key"},
 		{"src/pages/index.astro", "client:*"},
@@ -86,6 +88,8 @@ func TestResolve_DefaultRules(t *testing.T) {
 		{"crates/service/Cargo.toml", "Cargo Manifest Hygiene"},
 		{"scripts/deploy.py", "Mutable Default Arguments"},
 		{"src/app/main.py", "Mutable Default Arguments"},
+		{"public/index.php", "PHP Review Principles"},
+		{"templates/account/profile.phtml", "Web and Template Security Boundaries"},
 		{"locale/zh_CN/LC_MESSAGES/messages.po", "Placeholder Mismatch"},
 		{"i18n/app.po", "Plural Forms"},
 		{"locale/messages.pot", "Placeholder Consistency"},
@@ -98,6 +102,8 @@ func TestResolve_DefaultRules(t *testing.T) {
 		{"modules/network/vpc.hcl", "Overly Permissive Access"},
 		{"envs/prod.tfvars", "Hardcoded Secrets"},
 		{"infra/main.bicep", "Hardcoded Secrets"},
+		{"api/v1/user.proto", "Wire Compatibility"},
+		{"service.proto", "Wire Compatibility"},
 	}
 
 	for _, tt := range tests {
@@ -879,6 +885,67 @@ func TestResolveDetail_SystemGoPatternMatch(t *testing.T) {
 	}
 }
 
+func TestResolveDetail_SystemPHPPatternMatch(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	resolver, _, err := NewResolver(t.TempDir(), "")
+	if err != nil {
+		t.Fatalf("NewResolver: %v", err)
+	}
+	dr := resolver.(DetailResolver)
+
+	for _, path := range []string{"index.php", "src/Controller/UserController.php", "TEMPLATES/INDEX.PHTML"} {
+		t.Run(path, func(t *testing.T) {
+			detail := dr.ResolveDetail(path)
+			if detail.Source != "system" {
+				t.Errorf("expected source 'system', got %q", detail.Source)
+			}
+			if detail.Pattern != "**/*.{php,phtml}" {
+				t.Errorf("expected pattern '**/*.{php,phtml}', got %q", detail.Pattern)
+			}
+			for _, required := range []string{
+				"PHP Review Principles",
+				"foreach` value variable iterated by reference",
+				"unserialize()",
+				"PHPStan",
+			} {
+				if !strings.Contains(detail.Rule, required) {
+					t.Errorf("expected PHP rule to contain %q", required)
+				}
+			}
+		})
+	}
+}
+
+func TestResolveDetail_SystemComposerPatternPrecedesJSON(t *testing.T) {
+	t.Setenv("HOME", t.TempDir())
+	resolver, _, err := NewResolver(t.TempDir(), "")
+	if err != nil {
+		t.Fatalf("NewResolver: %v", err)
+	}
+	dr := resolver.(DetailResolver)
+
+	for _, path := range []string{"composer.json", "packages/library/composer.json", "PACKAGES/APP/COMPOSER.JSON"} {
+		t.Run(path, func(t *testing.T) {
+			detail := dr.ResolveDetail(path)
+			if detail.Source != "system" {
+				t.Errorf("expected source 'system', got %q", detail.Source)
+			}
+			if detail.Pattern != "**/composer.json" {
+				t.Errorf("expected pattern '**/composer.json', got %q", detail.Pattern)
+			}
+			for _, required := range []string{
+				"Composer Manifest Review Principles",
+				"config.allow-plugins",
+				"PSR-4",
+			} {
+				if !strings.Contains(detail.Rule, required) {
+					t.Errorf("expected Composer rule to contain %q", required)
+				}
+			}
+		})
+	}
+}
+
 func TestResolveDetail_ProjectOverridesSystem(t *testing.T) {
 	t.Setenv("HOME", t.TempDir())
 	dir := t.TempDir()
@@ -1170,7 +1237,10 @@ func TestResolveRuleEntries_SymlinkSafety(t *testing.T) {
 	// The extension check on the resolved path should reject .json.
 	symlinkPath := filepath.Join(dir, "evil.md")
 	if err := os.Symlink(sensitiveFile, symlinkPath); err != nil {
-		t.Fatal(err)
+		// Creating a symlink on Windows needs SeCreateSymbolicLinkPrivilege, which
+		// an unelevated CI account does not have. Same skip the other symlink tests
+		// in this repo already use.
+		t.Skipf("cannot create symlink: %v", err)
 	}
 
 	entries := []ProjectRuleEntry{
diff --git a/internal/diff/parser.go b/internal/diff/parser.go
index 9dd0fb4f..0cb09fcf 100644
--- a/internal/diff/parser.go
+++ b/internal/diff/parser.go
@@ -65,6 +65,11 @@ func ParseDiffText(ctx context.Context, diffText string, repoDir string, ref str
 		switch {
 		case strings.HasPrefix(line, "@@"):
 			inHunk = true
+		// The object IDs and mode in Git's extended "index" header are not
+		// useful review context. Keep index text in hunks, where it is file
+		// content and therefore carries a diff prefix.
+		case !inHunk && strings.HasPrefix(line, "index "):
+			continue
 		case !inHunk && binaryRe.MatchString(line):
 			current.IsBinary = true
 		// Extended header lines (unambiguous: content lines always carry a
diff --git a/internal/diff/parser_test.go b/internal/diff/parser_test.go
index b566bf58..78932707 100644
--- a/internal/diff/parser_test.go
+++ b/internal/diff/parser_test.go
@@ -2,9 +2,51 @@ package diff
 
 import (
 	"context"
+	"strings"
 	"testing"
 )
 
+func TestParseDiffText_StripsIndexHeadersFromPromptDiff(t *testing.T) {
+	diffText := `diff --git a/first.go b/first.go
+index 1234567..89abcde 100644
+--- a/first.go
++++ b/first.go
+@@ -1,1 +1,2 @@
+ first
++index added-content
+diff --git a/second.go b/second.go
+new file mode 100644
+index 0000000..7654321
+--- /dev/null
++++ b/second.go
+@@ -0,0 +1 @@
++package second
+`
+
+	diffs, err := ParseDiffText(context.Background(), diffText, t.TempDir(), "", nil)
+	if err != nil {
+		t.Fatalf("ParseDiffText: %v", err)
+	}
+	if len(diffs) != 2 {
+		t.Fatalf("expected 2 diffs, got %d", len(diffs))
+	}
+
+	for _, d := range diffs {
+		if strings.Contains(d.Diff, "\nindex ") {
+			t.Errorf("prompt diff contains index header:\n%s", d.Diff)
+		}
+	}
+	if !strings.Contains(diffs[0].Diff, "diff --git a/first.go b/first.go") {
+		t.Errorf("prompt diff lost git header:\n%s", diffs[0].Diff)
+	}
+	if !strings.Contains(diffs[0].Diff, "+index added-content") {
+		t.Errorf("prompt diff lost index-prefixed hunk content:\n%s", diffs[0].Diff)
+	}
+	if !diffs[1].IsNew {
+		t.Error("new-file metadata was not preserved")
+	}
+}
+
 // TestParseDiffText_Rename guards against issue #99: a renamed file must be
 // recognized via the "rename from"/"rename to" extended header lines so that
 // the parser reads content at the NEW path instead of warning about the old
diff --git a/internal/llm/keycmd.go b/internal/llm/keycmd.go
new file mode 100644
index 00000000..0060c88e
--- /dev/null
+++ b/internal/llm/keycmd.go
@@ -0,0 +1,130 @@
+package llm
+
+import (
+	"bytes"
+	"context"
+	"errors"
+	"fmt"
+	"os"
+	"os/exec"
+	"strings"
+	"time"
+)
+
+// keyCmdTimeout bounds how long an api_key_cmd / auth_token_cmd may run.
+// It is a package var (not const) so tests can shrink it.
+var keyCmdTimeout = 60 * time.Second
+
+// keyCmdWaitDelay bounds how long Wait keeps waiting on the child's stdout pipe
+// after the command's own deadline has passed. Package var (not const) so tests
+// can shrink it, same as keyCmdTimeout.
+var keyCmdWaitDelay = 5 * time.Second
+
+// keyCmdMaxOutput caps how much of a credential command's stdout we buffer.
+const keyCmdMaxOutput = 64 << 10
+
+// errKeyCmdOutputTooLarge aborts the stdout copy once the cap is hit. It never
+// reaches the caller: cappedBuffer.overflow is what produces the error message.
+var errKeyCmdOutputTooLarge = errors.New("credential command output exceeds cap")
+
+// cappedBuffer collects at most max bytes and records whether more were offered.
+// Refusing the write makes os/exec's copier close the pipe, so a runaway command
+// (`cat /dev/urandom`) dies of SIGPIPE instead of growing our heap without bound.
+type cappedBuffer struct {
+	max      int
+	buf      bytes.Buffer
+	overflow bool
+}
+
+func (b *cappedBuffer) Write(p []byte) (int, error) {
+	if b.buf.Len()+len(p) > b.max {
+		b.overflow = true
+		return 0, errKeyCmdOutputTooLarge
+	}
+	return b.buf.Write(p)
+}
+
+// resolveKeyCmd runs a credential-fetching shell command and returns its
+// trimmed, single-line stdout. label names the source (e.g.
+// `api_key_cmd for provider "x"`) and is used in error messages.
+//
+// The child's stderr is wired to the process stderr so interactive prompts
+// (pinentry, 1Password, `op`) stay visible, and its stdin to the process stdin
+// so those prompts can be answered. Any failure is a hard error, never a silent
+// fallback. The resolved credential is used in memory only and is never written
+// to config or logged.
+func resolveKeyCmd(cmd, label string) (string, error) {
+	ctx, cancel := context.WithTimeout(context.Background(), keyCmdTimeout)
+	defer cancel()
+
+	c := newKeyCmd(ctx, cmd)
+	c.Stderr = os.Stderr
+	// With Stdin nil, os/exec hands the child /dev/null, so a helper that needs
+	// to prompt for a passphrase gets EOF or refuses to prompt at all because it
+	// sees no tty. Safe to hand over os.Stdin because no code path resolves an
+	// endpoint while the bubbletea TUI (which also reads os.Stdin) is running:
+	// ResolveEndpoint's only callers are the non-TUI review/scan and `ocr llm
+	// test` paths. Adding an in-TUI connection test would break that.
+	c.Stdin = os.Stdin
+	// Buffer stdout through cappedBuffer rather than an *os.File so os/exec does
+	// the copying in its own goroutine: that is what lets WaitDelay force the
+	// pipe closed. exec.CommandContext SIGKILLs only the shell, so a grandchild
+	// (gpg-agent, pinentry, `op`) that inherited the stdout pipe keeps it open
+	// and Wait blocks on the read long past the timeout -- reproducible with
+	// api_key_cmd = "sleep 200 & printf tok". WaitDelay makes Wait give up
+	// shortly after the context dies.
+	out := &cappedBuffer{max: keyCmdMaxOutput}
+	c.Stdout = out
+	c.WaitDelay = keyCmdWaitDelay
+
+	err := c.Run()
+	// Checked first so a timeout reports as such instead of as the SIGKILL exit
+	// status it produces. (Run has already joined every stdout copier, so the
+	// buffer below is safe to read on all paths.)
+	if ctx.Err() == context.DeadlineExceeded {
+		// Wrap ctx.Err() so callers can errors.Is(err, context.DeadlineExceeded).
+		return "", fmt.Errorf("%s timed out after %s: %w", label, keyCmdTimeout, ctx.Err())
+	}
+	if out.overflow {
+		return "", fmt.Errorf("%s produced more than 64KiB of output", label)
+	}
+	// ErrWaitDelay only means an orphaned grandchild still holds the pipe; the
+	// command itself exited fine and its output is already buffered, so use it
+	// rather than surfacing an exec-internal error.
+	if err != nil && !errors.Is(err, exec.ErrWaitDelay) {
+		// Covers non-zero exit and command-not-found (the shell exits non-zero
+		// and prints its not-found message on the child's stderr). ExitError.Stderr
+		// stays nil because we assigned c.Stderr, so no output can leak here.
+		return "", fmt.Errorf("%s failed: %w", label, err)
+	}
+
+	// Trim a trailing line break; multi-line output past that is ambiguous and refused.
+	// ContainsAny (not Contains "\n") so a lone interior CR is caught too: TrimRight
+	// leaves it, TrimSpace below only strips the edges, and a CR inside a credential
+	// makes net/http reject the Authorization header with an opaque error.
+	trimmed := strings.TrimRight(out.buf.String(), "\r\n")
+	if strings.ContainsAny(trimmed, "\n\r") {
+		return "", fmt.Errorf("%s produced multi-line output; expected a single credential (pipe through 'head -n1' if your command prints more)", label)
+	}
+	// Same reason as the line-break check, wider net: httpguts.ValidHeaderFieldValue
+	// (what net/http enforces) rejects every byte below 0x20 except SP and TAB, plus
+	// DEL. A NUL or VT smuggled in by e.g. `printf 'sk-a\0b'` would otherwise reach
+	// net/http as the opaque `invalid header field value for "Authorization"`.
+	//
+	// Deliberately before the TrimSpace below, so a trailing control byte is an
+	// error naming its offset rather than silently stripped: only TAB, SP and the
+	// line breaks already handled above are things a credential command can
+	// plausibly append by accident. Offsets are therefore into the pre-TrimSpace
+	// string, which is what the command actually produced.
+	for i := 0; i < len(trimmed); i++ {
+		if b := trimmed[i]; (b < 0x20 && b != '\t') || b == 0x7f {
+			return "", fmt.Errorf("%s produced a control byte 0x%02X at offset %d; a credential must not contain control characters", label, b, i)
+		}
+	}
+
+	key := strings.TrimSpace(trimmed)
+	if key == "" {
+		return "", fmt.Errorf("%s produced empty output", label)
+	}
+	return key, nil
+}
diff --git a/internal/llm/keycmd_test.go b/internal/llm/keycmd_test.go
new file mode 100644
index 00000000..ddf7263f
--- /dev/null
+++ b/internal/llm/keycmd_test.go
@@ -0,0 +1,160 @@
+//go:build !windows
+
+package llm
+
+import (
+	"os"
+	"strings"
+	"testing"
+	"time"
+)
+
+func TestResolveKeyCmd(t *testing.T) {
+	tests := []struct {
+		name    string
+		cmd     string
+		want    string
+		wantErr string // substring the error must contain; "" means success
+	}{
+		{name: "success", cmd: "printf 'sk-test\\n'", want: "sk-test"},
+		{name: "trailing whitespace trimmed", cmd: "printf '  sk-test  \\n'", want: "sk-test"},
+		{name: "no trailing newline", cmd: "printf 'sk-test'", want: "sk-test"},
+		{name: "crlf line ending trimmed", cmd: "printf 'sk-crlf\\r\\n'", want: "sk-crlf"},
+		{name: "non-zero exit", cmd: "exit 3", wantErr: "failed: exit status 3"},
+		{name: "false", cmd: "false", wantErr: "failed:"},
+		{name: "empty output", cmd: "true", wantErr: "produced empty output"},
+		{name: "empty printf", cmd: "printf ''", wantErr: "produced empty output"},
+		{name: "whitespace-only output", cmd: "printf '  \\n'", wantErr: "produced empty output"},
+		{name: "multi-line output", cmd: "printf 'a\\nb\\n'", wantErr: "produced multi-line output"},
+		// A lone interior CR is a line break too, and one that survives both
+		// TrimRight("\r\n") and TrimSpace. Refuse it here rather than let it reach
+		// net/http, which rejects the Authorization header with an opaque error.
+		{name: "interior carriage return", cmd: "printf 'a\\rb'", wantErr: "produced multi-line output"},
+		{name: "multi-line error names the fix", cmd: "printf 'a\\nb\\n'", wantErr: "pipe through 'head -n1'"},
+		// Every other control byte net/http rejects (httpguts.ValidHeaderFieldValue:
+		// anything < 0x20 except TAB, plus DEL) must be named here rather than reach
+		// the request as an opaque "invalid header field value" failure.
+		{name: "nul byte", cmd: "printf 'sk-a\\0b'", wantErr: "control byte 0x00 at offset 4"},
+		{name: "vertical tab", cmd: "printf 'sk-a\\013b'", wantErr: "control byte 0x0B at offset 4"},
+		{name: "form feed", cmd: "printf 'sk-a\\014b'", wantErr: "control byte 0x0C at offset 4"},
+		{name: "delete byte", cmd: "printf 'sk-a\\177b'", wantErr: "control byte 0x7F at offset 4"},
+		// TAB is legal in a header value, so it survives (interior only; TrimSpace
+		// takes the edges).
+		{name: "interior tab kept", cmd: "printf 'sk-a\\tb\\n'", want: "sk-a\tb"},
+		{name: "command not found", cmd: "this-cmd-does-not-exist-xyz", wantErr: "failed:"},
+		// Boundary: exactly the cap is fine, one byte more is refused. The child
+		// dies of SIGPIPE as soon as we stop accepting, so this stays fast.
+		{name: "output exactly at cap", cmd: "head -c 65536 /dev/zero | tr '\\0' a", want: strings.Repeat("a", keyCmdMaxOutput)},
+		{name: "output over cap", cmd: "yes aaaaaaaaaa | head -c 200000 | tr -d '\\n'", wantErr: "produced more than 64KiB of output"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got, err := resolveKeyCmd(tt.cmd, "api_key_cmd for provider \"x\"")
+			if tt.wantErr != "" {
+				if err == nil {
+					t.Fatalf("expected error containing %q, got nil (output %q)", tt.wantErr, got)
+				}
+				if !strings.Contains(err.Error(), tt.wantErr) {
+					t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if got != tt.want {
+				t.Fatalf("got %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestResolveKeyCmd_Timeout(t *testing.T) {
+	origTimeout, origDelay := keyCmdTimeout, keyCmdWaitDelay
+	keyCmdTimeout = 50 * time.Millisecond
+	// `sleep 5` inherits the stdout pipe and outlives the SIGKILL'd shell, so
+	// without a shrunk WaitDelay this test waits the full default 5s.
+	keyCmdWaitDelay = 100 * time.Millisecond
+	t.Cleanup(func() { keyCmdTimeout, keyCmdWaitDelay = origTimeout, origDelay })
+
+	_, err := resolveKeyCmd("sleep 5 2>/dev/null", "api_key_cmd for provider \"x\"")
+	if err == nil {
+		t.Fatal("expected timeout error, got nil")
+	}
+	if !strings.Contains(err.Error(), "timed out after") {
+		t.Fatalf("error %q does not mention timeout", err.Error())
+	}
+}
+
+// A grandchild that inherited the stdout pipe keeps it open after the shell
+// exits, which used to block Wait until the grandchild died. WaitDelay bounds
+// that: this must finish in well under the 30s sleep.
+func TestResolveKeyCmd_WaitDelayBoundsOrphanHoldingPipe(t *testing.T) {
+	origTimeout, origDelay := keyCmdTimeout, keyCmdWaitDelay
+	keyCmdTimeout = 50 * time.Millisecond
+	keyCmdWaitDelay = 100 * time.Millisecond
+	t.Cleanup(func() { keyCmdTimeout, keyCmdWaitDelay = origTimeout, origDelay })
+
+	// The grandchild must keep the inherited *stdout* pipe open (that is the case
+	// under test) but not our stderr: it outlives the test, and `go test` reads
+	// the test binary's stderr until EOF, so leaving it attached would stall the
+	// run for the full sleep even though resolveKeyCmd returned immediately.
+	start := time.Now()
+	_, err := resolveKeyCmd("sleep 30 2>/dev/null & printf tok", `api_key_cmd for provider "x"`)
+	elapsed := time.Since(start)
+
+	if elapsed > 5*time.Second {
+		t.Fatalf("took %s; WaitDelay did not bound the orphaned grandchild", elapsed)
+	}
+	if err == nil {
+		t.Fatal("expected timeout error, got nil")
+	}
+	if !strings.Contains(err.Error(), "timed out after") {
+		t.Fatalf("error %q does not mention timeout", err.Error())
+	}
+}
+
+// TestResolveKeyCmd_StdinWired proves the child inherits our stdin: with Stdin
+// left nil, os/exec hands the child /dev/null, `read` sees EOF and prints
+// nothing, so this would fail with "produced empty output" instead.
+//
+// os.Stdin under `go test` is not a usable prompt source, so swap in a pipe.
+// Mutating the global is safe here: this test is not parallel, and the only
+// parallel tests in the package are subtests of TestResolveKeyCmd, which
+// finishes before any later top-level test starts.
+func TestResolveKeyCmd_StdinWired(t *testing.T) {
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("os.Pipe: %v", err)
+	}
+	defer r.Close()
+
+	orig := os.Stdin
+	os.Stdin = r
+	t.Cleanup(func() { os.Stdin = orig })
+
+	// Written and closed up front (well under the pipe buffer, so no blocking)
+	// so the child reads a full line and then EOF.
+	if _, err := w.WriteString("passphrase-from-stdin\n"); err != nil {
+		t.Fatalf("write to stdin pipe: %v", err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatalf("close stdin pipe writer: %v", err)
+	}
+
+	got, err := resolveKeyCmd(`read -r x; printf %s "$x"`, `api_key_cmd for provider "x"`)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got != "passphrase-from-stdin" {
+		t.Fatalf("got %q, want %q", got, "passphrase-from-stdin")
+	}
+}
+
+func TestResolveKeyCmd_LabelInError(t *testing.T) {
+	_, err := resolveKeyCmd("false", `auth_token_cmd for llm config`)
+	if err == nil || !strings.HasPrefix(err.Error(), "auth_token_cmd for llm config") {
+		t.Fatalf("expected label prefix in error, got %v", err)
+	}
+}
diff --git a/internal/llm/keycmd_unix.go b/internal/llm/keycmd_unix.go
new file mode 100644
index 00000000..03e91beb
--- /dev/null
+++ b/internal/llm/keycmd_unix.go
@@ -0,0 +1,14 @@
+//go:build !windows
+
+package llm
+
+import (
+	"context"
+	"os/exec"
+)
+
+// newKeyCmd builds the OS-specific shell invocation (sh -c on Unix) that runs a
+// credential command under ctx, so its timeout and cancellation are honored.
+func newKeyCmd(ctx context.Context, cmd string) *exec.Cmd {
+	return exec.CommandContext(ctx, "sh", "-c", cmd)
+}
diff --git a/internal/llm/keycmd_windows.go b/internal/llm/keycmd_windows.go
new file mode 100644
index 00000000..5e71e7b1
--- /dev/null
+++ b/internal/llm/keycmd_windows.go
@@ -0,0 +1,47 @@
+//go:build windows
+
+package llm
+
+import (
+	"context"
+	"os/exec"
+	"syscall"
+)
+
+// newKeyCmd builds the OS-specific shell invocation (cmd.exe /C on Windows) that runs a
+// credential command under ctx, so its timeout and cancellation are honored.
+// Spelled with the extension so a file named `cmd` on PATH cannot shadow the shell.
+//
+// The command line is handed over through SysProcAttr.CmdLine instead of Args
+// because os/exec quotes Args with syscall.EscapeArg, which targets
+// CommandLineToArgvW; cmd.exe is a documented exception with different unquoting
+// rules (see the exec.Command doc comment), and its escaping mangles any command
+// containing a double quote -- `op read "op://Private/My Vault/api-key"` would
+// arrive as a single literal filename. /S makes cmd.exe strip exactly the outer
+// pair of quotes we add and pass the rest through verbatim.
+//
+// Not escaping the interpolated cmd is deliberate rather than an injection hole:
+// api_key_cmd is a command line its author asked us to run, so they already have
+// arbitrary execution by design (`api_key_cmd = "whoami"` is a supported config,
+// and the Unix arm hands the same string to `sh -c`), and it is read only from
+// the user-level ~/.opencodereview/config.json -- never from the repository
+// under review. Escaping the inner quotes would defeat the single case CmdLine
+// exists for. See keycmd_windows_test.go for which quote shapes /S does and does
+// not keep as one command.
+//
+// Note that a command string is not portable between the two arms: %VAR% and ^
+// are cmd.exe metacharacters and $VAR expansion / \ escaping do not apply, so an
+// sh-authored api_key_cmd generally needs a Windows-specific rewrite.
+func newKeyCmd(ctx context.Context, cmd string) *exec.Cmd {
+	// Still built by CommandContext so ctx cancellation and WaitDelay behave
+	// exactly as on Unix; only the command-line construction differs.
+	c := exec.CommandContext(ctx, "cmd.exe")
+	// CmdLine is the whole command line including argv[0]; the executable itself
+	// still comes from c.Path. Args stays at Command's default ([]string{"cmd.exe"})
+	// rather than nil: syscall.StartProcess uses SysProcAttr.CmdLine verbatim when
+	// non-empty and never looks at argv, so the doc's "leaving Args empty" is not
+	// load-bearing here -- and a one-element Args keeps Cmd.String() from panicking
+	// on Args[1:].
+	c.SysProcAttr = &syscall.SysProcAttr{CmdLine: `cmd.exe /S /C "` + cmd + `"`}
+	return c
+}
diff --git a/internal/llm/keycmd_windows_test.go b/internal/llm/keycmd_windows_test.go
new file mode 100644
index 00000000..3cfa91d1
--- /dev/null
+++ b/internal/llm/keycmd_windows_test.go
@@ -0,0 +1,152 @@
+//go:build windows
+
+package llm
+
+import (
+	"context"
+	"os"
+	"strings"
+	"testing"
+	"time"
+)
+
+// TestNewKeyCmd_CmdLine locks in the two decisions in newKeyCmd that no runtime
+// test can observe: the command reaches cmd.exe through SysProcAttr.CmdLine
+// verbatim (not through Args, whose syscall.EscapeArg quoting mangles embedded
+// double quotes), and Args keeps its one-element default so Cmd.String() cannot
+// panic on Args[1:].
+func TestNewKeyCmd_CmdLine(t *testing.T) {
+	c := newKeyCmd(context.Background(), `op read "op://Private/My Vault/api-key"`)
+
+	want := `cmd.exe /S /C "op read "op://Private/My Vault/api-key""`
+	if c.SysProcAttr == nil {
+		t.Fatal("SysProcAttr is nil; the command line would be built from Args instead")
+	}
+	if got := c.SysProcAttr.CmdLine; got != want {
+		t.Errorf("CmdLine = %q, want %q", got, want)
+	}
+	if len(c.Args) == 0 {
+		t.Error("Args is empty; Cmd.String() indexes Args[1:] and panics on a nil slice")
+	}
+	// Panics if Args were nilled out.
+	if s := c.String(); s == "" {
+		t.Error("Cmd.String() returned empty")
+	}
+}
+
+func TestResolveKeyCmd(t *testing.T) {
+	tests := []struct {
+		name    string
+		cmd     string
+		want    string
+		wantErr string // substring the error must contain; "" means success
+	}{
+		{name: "success", cmd: "echo sk-test", want: "sk-test"},
+		// ECHO eats exactly one delimiter after the command token, so stdout here is
+		// "   sk-test  \r\n" -- the trim is what produces the credential.
+		{name: "surrounding whitespace trimmed", cmd: "echo    sk-test  ", want: "sk-test"},
+		// The case the CmdLine detour exists for: quotes and spaces must arrive at
+		// cmd.exe exactly as written. Routed through Args instead, EscapeArg would
+		// wrap and backslash-escape them and the output would carry the backslashes.
+		{name: "embedded quotes survive verbatim", cmd: `echo sk-"a b"-token`, want: `sk-"a b"-token`},
+		{name: "non-zero exit", cmd: "exit 3", wantErr: "failed: exit status 3"},
+		{name: "no output", cmd: "rem", wantErr: "produced empty output"},
+		{name: "blank line only", cmd: "echo.", wantErr: "produced empty output"},
+		// & is cmd.exe's command separator, so both echoes run and produce two lines.
+		{name: "multi-line output", cmd: "echo a& echo b", wantErr: "produced multi-line output"},
+		// The two rows below pin down what the outer quote pair we add does and does
+		// not protect, because "the command line could split" reads like a hole until
+		// you know which shapes actually split. /S makes cmd.exe strip the first
+		// character and the last quote and run the remainder unchanged, so a bare
+		// interior quote leaves the following & inside a quoted region: it stays one
+		// command and echo prints the & literally.
+		{name: "interior quote keeps & quoted", cmd: `echo A" & echo B`, want: `A" & echo B`},
+		// A doubled quote closes that region, so this & is a real separator and both
+		// echoes run. It is not a privilege boundary -- api_key_cmd is already a
+		// command line its author asked us to run -- but it is the one shape where the
+		// line splits, and the single-line guard is what stops the extra output from
+		// being mistaken for the credential.
+		{name: "doubled quote lets & split the line", cmd: `echo A"" & echo B`, wantErr: "produced multi-line output"},
+		{name: "command not found", cmd: "this-cmd-does-not-exist-xyz", wantErr: "failed:"},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			t.Parallel()
+			got, err := resolveKeyCmd(tt.cmd, `api_key_cmd for provider "x"`)
+			if tt.wantErr != "" {
+				if err == nil {
+					t.Fatalf("expected error containing %q, got nil (output %q)", tt.wantErr, got)
+				}
+				if !strings.Contains(err.Error(), tt.wantErr) {
+					t.Fatalf("error %q does not contain %q", err.Error(), tt.wantErr)
+				}
+				return
+			}
+			if err != nil {
+				t.Fatalf("unexpected error: %v", err)
+			}
+			if got != tt.want {
+				t.Fatalf("got %q, want %q", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestResolveKeyCmd_Timeout(t *testing.T) {
+	origTimeout, origDelay := keyCmdTimeout, keyCmdWaitDelay
+	keyCmdTimeout = 50 * time.Millisecond
+	keyCmdWaitDelay = 100 * time.Millisecond
+	t.Cleanup(func() { keyCmdTimeout, keyCmdWaitDelay = origTimeout, origDelay })
+
+	// ping, not timeout.exe: timeout.exe refuses to run when stdin is redirected,
+	// and resolveKeyCmd hands the child the test binary's stdin. Its stderr is
+	// redirected for the same reason the unix twin redirects it: the killed
+	// command's orphan would otherwise hold the test binary's stderr, which
+	// cmd/go reads to EOF, stalling the run past the point resolveKeyCmd returned.
+	_, err := resolveKeyCmd("ping -n 6 127.0.0.1 2>nul", `api_key_cmd for provider "x"`)
+	if err == nil {
+		t.Fatal("expected timeout error, got nil")
+	}
+	if !strings.Contains(err.Error(), "timed out after") {
+		t.Fatalf("error %q does not mention timeout", err.Error())
+	}
+}
+
+// TestResolveKeyCmd_StdinWired proves the child inherits our stdin: with Stdin
+// left nil, os/exec hands the child NUL, findstr reads EOF immediately and
+// prints nothing, so this would fail with "produced empty output" instead.
+func TestResolveKeyCmd_StdinWired(t *testing.T) {
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("os.Pipe: %v", err)
+	}
+	defer r.Close()
+
+	orig := os.Stdin
+	os.Stdin = r
+	t.Cleanup(func() { os.Stdin = orig })
+
+	if _, err := w.WriteString("passphrase-from-stdin\r\n"); err != nil {
+		t.Fatalf("write to stdin pipe: %v", err)
+	}
+	if err := w.Close(); err != nil {
+		t.Fatalf("close stdin pipe writer: %v", err)
+	}
+
+	// findstr "^" copies every stdin line to stdout; ^ is passed through verbatim
+	// under /S rather than treated as cmd.exe's escape character.
+	got, err := resolveKeyCmd(`findstr "^"`, `api_key_cmd for provider "x"`)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if got != "passphrase-from-stdin" {
+		t.Fatalf("got %q, want %q", got, "passphrase-from-stdin")
+	}
+}
+
+func TestResolveKeyCmd_LabelInError(t *testing.T) {
+	_, err := resolveKeyCmd("exit 1", `auth_token_cmd for llm config`)
+	if err == nil || !strings.HasPrefix(err.Error(), "auth_token_cmd for llm config") {
+		t.Fatalf("expected label prefix in error, got %v", err)
+	}
+}
diff --git a/internal/llm/resolver.go b/internal/llm/resolver.go
index 853f42fd..bc267f36 100644
--- a/internal/llm/resolver.go
+++ b/internal/llm/resolver.go
@@ -40,10 +40,11 @@ const (
 	// openai | openai-responses). Takes priority
 	// over OCR_USE_ANTHROPIC when set.
 	envOCRLLMProtocol = "OCR_LLM_PROTOCOL"
-	// envOCRLLMTimeout is a global override applied in ResolveEndpointWithModelOverride
-	// after any strategy resolves, rather than inside tryOCREnv like other OCR_LLM_* vars.
-	// This lets it override timeout for all resolution paths (OCR env, config file,
-	// provider config, Claude Code env, shell RC).
+	// envOCRLLMTimeout is a global override parsed at the top of
+	// ResolveEndpointWithModelOverride and applied to whichever strategy resolves,
+	// rather than inside tryOCREnv like other OCR_LLM_* vars. This lets it override
+	// timeout for all resolution paths (OCR env, config file, provider config,
+	// Claude Code env, shell RC).
 	envOCRLLMTimeout   = "OCR_LLM_TIMEOUT"
 	envOCRUseAnthropic = "OCR_USE_ANTHROPIC"
 )
@@ -68,6 +69,23 @@ func ResolveEndpoint(configPath string) (ResolvedEndpoint, error) {
 func ResolveEndpointWithModelOverride(configPath, modelOverride string) (ResolvedEndpoint, error) {
 	modelOverride = strings.TrimSpace(modelOverride)
 
+	// Both global env overrides are parsed before any strategy runs, even though
+	// they are applied to the resolved endpoint below. Parsing them after the loop
+	// would let a typo'd OCR_LLM_TIMEOUT ("30s") or an unparseable
+	// OCR_LLM_EXTRA_HEADERS abort resolution *after* api_key_cmd already prompted
+	// 1Password/pinentry/Touch ID for a credential that then gets discarded.
+	envTimeout, hasEnvTimeout, err := parseTimeoutEnv()
+	if err != nil {
+		return ResolvedEndpoint{}, err
+	}
+	var envHeaders map[string]string
+	if raw := os.Getenv(envOCRLLMExtraHeaders); raw != "" {
+		envHeaders, err = ParseExtraHeaders(raw)
+		if err != nil {
+			return ResolvedEndpoint{}, fmt.Errorf("%s: %w", envOCRLLMExtraHeaders, err)
+		}
+	}
+
 	strategies := []struct {
 		name string
 		fn   func() (ResolvedEndpoint, bool, error)
@@ -91,21 +109,13 @@ func ResolveEndpointWithModelOverride(configPath, modelOverride string) (Resolve
 			// OCR_LLM_TIMEOUT is a global override: applies regardless of
 			// which strategy resolved the endpoint, and takes precedence
 			// over config-file values when set.
-			envTimeout, ok, err := parseTimeoutEnv()
-			if err != nil {
-				return ResolvedEndpoint{}, fmt.Errorf("resolve %s: %w", s.name, err)
-			}
-			if ok {
+			if hasEnvTimeout {
 				ep.Timeout = envTimeout
 			}
 			// OCR_LLM_EXTRA_HEADERS is a global override: merges into
 			// extra headers regardless of which strategy resolved the
 			// endpoint. Env values take precedence over config-file values.
-			if raw := os.Getenv(envOCRLLMExtraHeaders); raw != "" {
-				envHeaders, err := ParseExtraHeaders(raw)
-				if err != nil {
-					return ResolvedEndpoint{}, fmt.Errorf("resolve %s: %w", s.name, err)
-				}
+			if envHeaders != nil {
 				if ep.ExtraHeaders == nil {
 					ep.ExtraHeaders = envHeaders
 				} else {
@@ -212,9 +222,10 @@ type llmFileConfig struct {
 	AuthToken    string            `json:"auth_token,omitempty"`
 	AuthHeader   string            `json:"auth_header,omitempty"`
 	Model        string            `json:"model,omitempty"`
-	Protocol     string            `json:"protocol,omitempty"`      // anthropic|openai|openai-responses; takes priority over use_anthropic
-	UseAnthropic *bool             `json:"use_anthropic,omitempty"` // pointer to distinguish unset from false; legacy fallback when protocol is empty
-	TimeoutSec   int               `json:"timeout_sec,omitempty"`   // per-request HTTP timeout in seconds
+	AuthTokenCmd string            `json:"auth_token_cmd,omitempty"` // shell command whose stdout is the auth token; used when auth_token is empty
+	Protocol     string            `json:"protocol,omitempty"`       // anthropic|openai|openai-responses; takes priority over use_anthropic
+	UseAnthropic *bool             `json:"use_anthropic,omitempty"`  // pointer to distinguish unset from false; legacy fallback when protocol is empty
+	TimeoutSec   int               `json:"timeout_sec,omitempty"`    // per-request HTTP timeout in seconds
 	ExtraBody    map[string]any    `json:"extra_body,omitempty"`
 	ExtraHeaders map[string]string `json:"extra_headers,omitempty"`
 }
@@ -222,6 +233,7 @@ type llmFileConfig struct {
 // providerEntryConfig represents a single provider entry in config.json.
 type providerEntryConfig struct {
 	APIKey       string            `json:"api_key,omitempty"`
+	APIKeyCmd    string            `json:"api_key_cmd,omitempty"` // shell command whose stdout is the api key; used when api_key is empty
 	URL          string            `json:"url,omitempty"`
 	Protocol     string            `json:"protocol,omitempty"`
 	Model        string            `json:"model,omitempty"`
@@ -281,14 +293,47 @@ func tryProviderConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,
 		return ResolvedEndpoint{}, false, fmt.Errorf("provider %q is set but not configured in %s section", cfg.Provider, section)
 	}
 
+	// Pick the credential source here, but run api_key_cmd only just before
+	// returning (see below): a config typo must not trigger a secret-manager
+	// prompt before the cheap validation below has had a chance to fail.
+	// A whitespace-only api_key is a typo, not a credential: treat it as unset so
+	// it cannot silently shadow a working api_key_cmd (which otherwise resolves to
+	// a 401 with the command never running). A key with real content is used
+	// verbatim -- unlike command stdout, which has a mechanical trailing newline
+	// to strip, a static value has no artifact that trimming must undo.
 	apiKey := entry.APIKey
-	if apiKey == "" {
-		if isPreset && preset.EnvVar != "" {
-			apiKey = os.Getenv(preset.EnvVar)
-		}
-	}
-	if apiKey == "" {
-		return ResolvedEndpoint{}, false, fmt.Errorf("provider %q has no api_key configured and no environment variable fallback found", cfg.Provider)
+	if strings.TrimSpace(apiKey) == "" {
+		apiKey = ""
+	}
+	// Same rule for the command: `sh -c "   "` exits 0 with no output, so a
+	// whitespace-only api_key_cmd would suppress the env fallback and then fail
+	// with "produced empty output". Treating it as unset keeps the typo from
+	// being more disruptive than the equivalent typo in api_key.
+	apiKeyCmd := entry.APIKeyCmd
+	if strings.TrimSpace(apiKeyCmd) == "" {
+		apiKeyCmd = ""
+	}
+	switch {
+	case apiKey != "":
+		// Static api_key always wins. Warn (don't error) if a command is also set,
+		// so a config that keeps api_key_cmd as a deliberate fallback still works.
+		if apiKeyCmd != "" {
+			fmt.Fprintf(os.Stderr, "[ocr] WARNING: provider %q has both api_key and api_key_cmd set; using the static api_key\n", cfg.Provider)
+		}
+	case apiKeyCmd == "" && isPreset && preset.EnvVar != "":
+		// Env var is the last resort: only when neither api_key nor api_key_cmd
+		// is set, and only for preset providers (custom ones have no fallback).
+		// Same whitespace rule as the static key above, so `export
+		// ANTHROPIC_API_KEY="  "` reports "no api_key configured" instead of
+		// sending `Authorization: Bearer  ` and getting an opaque 401.
+		if v := os.Getenv(preset.EnvVar); strings.TrimSpace(v) != "" {
+			apiKey = v
+		}
+	}
+	// No credential at all is still an error here, before any other validation:
+	// only the command's *execution* is deferred, not the emptiness check.
+	if apiKey == "" && apiKeyCmd == "" {
+		return ResolvedEndpoint{}, false, fmt.Errorf("provider %q has no api_key or api_key_cmd configured and no environment variable fallback found", cfg.Provider)
 	}
 
 	var url, protocol, authHeader, model string
@@ -389,6 +434,18 @@ func tryProviderConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,
 		url = ensureMessagesSuffix(url)
 	}
 
+	// Single api_key_cmd resolution site for both preset and custom providers,
+	// as late as possible: everything above can fail without running the
+	// command. apiKey is empty here only when api_key_cmd is set (guaranteed by
+	// the emptiness check above), and a failing command is a hard error.
+	if apiKey == "" {
+		resolved, err := resolveKeyCmd(apiKeyCmd, fmt.Sprintf("api_key_cmd for provider %q", cfg.Provider))
+		if err != nil {
+			return ResolvedEndpoint{}, false, err
+		}
+		apiKey = resolved
+	}
+
 	return ResolvedEndpoint{
 		URL:          url,
 		Token:        apiKey,
@@ -408,9 +465,30 @@ func tryLegacyLlmConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,
 	if modelOverride != "" {
 		model = modelOverride
 	}
-	if cfg.Llm.URL == "" || cfg.Llm.AuthToken == "" || model == "" {
+	// Fall through to later strategies when the legacy block is incomplete. This
+	// includes the case where neither auth_token nor auth_token_cmd is set — and,
+	// critically, an incomplete block (e.g. missing url) never runs auth_token_cmd.
+	// "Incomplete" is judged after modelOverride is applied above, so a block
+	// missing only `model` is complete under --model and does run the command;
+	// that is the documented contract of ResolveEndpointWithModelOverride.
+	// Whitespace-only auth_token is treated as unset, same as api_key above, so it
+	// cannot shadow a working auth_token_cmd; same rule for the command itself.
+	token := cfg.Llm.AuthToken
+	if strings.TrimSpace(token) == "" {
+		token = ""
+	}
+	tokenCmd := cfg.Llm.AuthTokenCmd
+	if strings.TrimSpace(tokenCmd) == "" {
+		tokenCmd = ""
+	}
+	if cfg.Llm.URL == "" || model == "" || (token == "" && tokenCmd == "") {
 		return ResolvedEndpoint{}, false, nil
 	}
+	// Static auth_token always wins; warn if a command is also set. The command
+	// itself runs only just before returning, after the validation below.
+	if token != "" && tokenCmd != "" {
+		fmt.Fprintln(os.Stderr, "[ocr] WARNING: llm config has both auth_token and auth_token_cmd set; using the static auth_token")
+	}
 
 	// llm.protocol (normalized) wins over use_anthropic when set.
 	protocol := ""
@@ -449,7 +527,18 @@ func tryLegacyLlmConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,
 		return ResolvedEndpoint{}, false, fmt.Errorf("OCR config file: %w", err)
 	}
 
-	return ResolvedEndpoint{URL: cfg.Llm.URL, Token: cfg.Llm.AuthToken, Model: model, Protocol: protocol, AuthHeader: authHeader, Source: "OCR config file", ExtraBody: cfg.Llm.ExtraBody, ExtraHeaders: cfg.Llm.ExtraHeaders, Timeout: timeout}, true, nil
+	// token is empty here only for an otherwise-complete block whose
+	// auth_token_cmd is set (guaranteed by the incompleteness check above), so a
+	// failing command is a hard error and an incomplete block never runs it.
+	if token == "" {
+		resolved, err := resolveKeyCmd(tokenCmd, "auth_token_cmd for llm config")
+		if err != nil {
+			return ResolvedEndpoint{}, false, err
+		}
+		token = resolved
+	}
+
+	return ResolvedEndpoint{URL: cfg.Llm.URL, Token: token, Model: model, Protocol: protocol, AuthHeader: authHeader, Source: "OCR config file", ExtraBody: cfg.Llm.ExtraBody, ExtraHeaders: cfg.Llm.ExtraHeaders, Timeout: timeout}, true, nil
 }
 
 // tryCCEnv reads Claude Code environment variables.
diff --git a/internal/llm/resolver_keycmd_test.go b/internal/llm/resolver_keycmd_test.go
new file mode 100644
index 00000000..8334f3f5
--- /dev/null
+++ b/internal/llm/resolver_keycmd_test.go
@@ -0,0 +1,441 @@
+//go:build !windows
+
+// Every test in this file drives a credential command, and all of them are POSIX
+// shell (`printf`, `exit N`), which would run through `cmd /C` on Windows.
+
+package llm
+
+import (
+	"encoding/json"
+	"io"
+	"os"
+	"path/filepath"
+	"strings"
+	"testing"
+)
+
+func writeConfigJSON(t *testing.T, cfg configFile) string {
+	t.Helper()
+	data, err := json.Marshal(cfg)
+	if err != nil {
+		t.Fatalf("marshal config: %v", err)
+	}
+	p := filepath.Join(t.TempDir(), "config.json")
+	if err := os.WriteFile(p, data, 0644); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+	return p
+}
+
+// (a) api_key_cmd resolves when no static key is present.
+func TestResolveEndpoint_ProviderAPIKeyCmd(t *testing.T) {
+	clearAllEnv(t)
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {APIKeyCmd: "printf 'sk-from-cmd\\n'", Model: "claude-sonnet-4-6"},
+		},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "sk-from-cmd" {
+		t.Errorf("Token = %q, want %q", ep.Token, "sk-from-cmd")
+	}
+}
+
+// (a2) the command runs exactly once per resolution. "No caching" is correct
+// today only because resolution happens once per process; a second call would
+// mean a second pinentry prompt per review.
+func TestResolveEndpoint_APIKeyCmdRunsExactlyOnce(t *testing.T) {
+	clearAllEnv(t)
+	counter := filepath.Join(t.TempDir(), "runs")
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {
+				APIKeyCmd: "echo run >> " + counter + "; printf 'sk-once\\n'",
+				Model:     "claude-sonnet-4-6",
+			},
+		},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "sk-once" {
+		t.Fatalf("Token = %q, want %q", ep.Token, "sk-once")
+	}
+	data, err := os.ReadFile(counter)
+	if err != nil {
+		t.Fatalf("read counter file: %v", err)
+	}
+	if got := strings.Count(string(data), "\n"); got != 1 {
+		t.Errorf("api_key_cmd ran %d times, want exactly 1 (counter file %q)", got, data)
+	}
+}
+
+// (b) static api_key wins even when api_key_cmd is also set — and the command
+// does not run at all. Asserting only on ep.Token would pass just as well if the
+// command ran and its output were discarded, which for a real config means a
+// pinentry/Touch ID prompt on every review that keeps a command as a fallback.
+func TestResolveEndpoint_ProviderStaticKeyWinsOverCmd(t *testing.T) {
+	clearAllEnv(t)
+	marker := filepath.Join(t.TempDir(), "ran")
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {
+				APIKey:    "sk-static",
+				APIKeyCmd: "touch " + marker + "; printf 'sk-from-cmd\\n'",
+				Model:     "claude-sonnet-4-6",
+			},
+		},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "sk-static" {
+		t.Errorf("Token = %q, want %q (static api_key must win)", ep.Token, "sk-static")
+	}
+	if _, err := os.Stat(marker); err == nil {
+		t.Error("api_key_cmd executed even though a static api_key was set")
+	}
+}
+
+// (b4) a whitespace-only api_key_cmd is a typo, not a command: it must not
+// suppress the env-var fallback the way a real command does. Same rule the
+// static api_key already follows.
+func TestResolveEndpoint_WhitespaceOnlyCmdFallsBackToEnv(t *testing.T) {
+	clearAllEnv(t)
+	t.Setenv("ANTHROPIC_API_KEY", "sk-from-env")
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {APIKeyCmd: "   ", Model: "claude-sonnet-4-6"},
+		},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "sk-from-env" {
+		t.Errorf("Token = %q, want %q (whitespace-only api_key_cmd must be treated as unset)", ep.Token, "sk-from-env")
+	}
+}
+
+// (b5) same rule on the legacy block: whitespace-only auth_token_cmd leaves the
+// block incomplete rather than running an empty command and hard-failing.
+func TestResolveEndpoint_LegacyWhitespaceOnlyCmdIsUnset(t *testing.T) {
+	clearAllEnv(t)
+	t.Setenv("ANTHROPIC_BASE_URL", "https://env.test")
+	t.Setenv("ANTHROPIC_AUTH_TOKEN", "sk-from-env")
+	t.Setenv("ANTHROPIC_MODEL", "m")
+	cfgPath := writeConfigJSON(t, configFile{
+		Llm: llmFileConfig{URL: "https://example.test", Model: "m", AuthTokenCmd: "  \t "},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "sk-from-env" {
+		t.Errorf("Token = %q, want %q (whitespace-only auth_token_cmd must be treated as unset)", ep.Token, "sk-from-env")
+	}
+}
+
+// captureStderr swaps os.Stderr for a pipe around fn and returns what was written.
+// Output here is tiny, so reading after the writer is closed avoids any pipe-buffer
+// deadlock without a goroutine.
+func captureStderr(t *testing.T, fn func()) string {
+	t.Helper()
+	r, w, err := os.Pipe()
+	if err != nil {
+		t.Fatalf("os.Pipe: %v", err)
+	}
+	orig := os.Stderr
+	os.Stderr = w
+	defer func() { os.Stderr = orig }()
+
+	fn()
+
+	if err := w.Close(); err != nil {
+		t.Fatalf("close pipe writer: %v", err)
+	}
+	out, err := io.ReadAll(r)
+	if err != nil {
+		t.Fatalf("read captured stderr: %v", err)
+	}
+	return string(out)
+}
+
+// (b2) when both api_key and api_key_cmd are set, a warning is emitted on stderr
+// and the resolved token is still the static api_key.
+func TestResolveEndpoint_BothSetWarnsAndUsesStaticKey(t *testing.T) {
+	clearAllEnv(t)
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {APIKey: "sk-static", APIKeyCmd: "printf 'sk-from-cmd\\n'", Model: "claude-sonnet-4-6"},
+		},
+	})
+	var ep ResolvedEndpoint
+	var err error
+	stderr := captureStderr(t, func() {
+		ep, err = ResolveEndpoint(cfgPath)
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "sk-static" {
+		t.Errorf("Token = %q, want %q (static api_key must win)", ep.Token, "sk-static")
+	}
+	// Match the message, not the log prefix, so this does not break when the
+	// warning prefix is restyled.
+	want := `provider "anthropic" has both api_key and api_key_cmd set; using the static api_key`
+	if !strings.Contains(stderr, want) {
+		t.Errorf("stderr %q does not contain warning %q", stderr, want)
+	}
+}
+
+// (e2) legacy path: both auth_token and auth_token_cmd set -> warning + static wins.
+func TestResolveEndpoint_LegacyBothSetWarnsAndUsesStaticToken(t *testing.T) {
+	clearAllEnv(t)
+	cfgPath := writeConfigJSON(t, configFile{
+		Llm: llmFileConfig{
+			URL:          "https://api.example.com/v1/messages",
+			AuthToken:    "legacy-static",
+			AuthTokenCmd: "printf 'legacy-from-cmd\\n'",
+			Model:        "claude-sonnet-4-6",
+		},
+	})
+	var ep ResolvedEndpoint
+	var err error
+	stderr := captureStderr(t, func() {
+		ep, err = ResolveEndpoint(cfgPath)
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "legacy-static" {
+		t.Errorf("Token = %q, want %q (static auth_token must win)", ep.Token, "legacy-static")
+	}
+	want := "llm config has both auth_token and auth_token_cmd set; using the static auth_token"
+	if !strings.Contains(stderr, want) {
+		t.Errorf("stderr %q does not contain warning %q", stderr, want)
+	}
+}
+
+// (b3) a whitespace-only api_key is a typo, not a credential: it must not shadow
+// the command (which used to resolve Token="   " -> 401, command never run), and
+// the both-set warning must stay quiet since nothing is really being shadowed.
+func TestResolveEndpoint_WhitespaceOnlyStaticKeyUsesCmd(t *testing.T) {
+	clearAllEnv(t)
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {APIKey: "   ", APIKeyCmd: "printf 'sk-from-cmd\\n'", Model: "claude-sonnet-4-6"},
+		},
+	})
+	var ep ResolvedEndpoint
+	var err error
+	stderr := captureStderr(t, func() {
+		ep, err = ResolveEndpoint(cfgPath)
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "sk-from-cmd" {
+		t.Errorf("Token = %q, want %q (whitespace-only api_key must not shadow api_key_cmd)", ep.Token, "sk-from-cmd")
+	}
+	if strings.Contains(stderr, "both api_key and api_key_cmd") {
+		t.Errorf("warned about a shadowed command that was actually used; stderr: %q", stderr)
+	}
+}
+
+// (e3b) the same whitespace rule reaches the env-var fallback, which is the last
+// source in the chain and had been exempt: a whitespace-only value there used to
+// resolve successfully and send `Authorization: Bearer  `, producing an opaque 401
+// instead of naming the missing credential.
+func TestResolveEndpoint_WhitespaceOnlyEnvVarIsNotACredential(t *testing.T) {
+	clearAllEnv(t)
+	t.Setenv("ANTHROPIC_API_KEY", "   ")
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {Model: "claude-sonnet-4-6"},
+		},
+	})
+	_, err := ResolveEndpoint(cfgPath)
+	if err == nil {
+		t.Fatal("expected an error: a whitespace-only env var is not a credential")
+	}
+	if !strings.Contains(err.Error(), "no api_key or api_key_cmd configured") {
+		t.Errorf("error %q does not name the missing credential", err.Error())
+	}
+}
+
+// (e4) same on the legacy path.
+func TestResolveEndpoint_LegacyWhitespaceOnlyStaticTokenUsesCmd(t *testing.T) {
+	clearAllEnv(t)
+	cfgPath := writeConfigJSON(t, configFile{
+		Llm: llmFileConfig{
+			URL:          "https://api.example.com/v1/messages",
+			AuthToken:    "\t\n ",
+			AuthTokenCmd: "printf 'legacy-from-cmd\\n'",
+			Model:        "claude-sonnet-4-6",
+		},
+	})
+	var ep ResolvedEndpoint
+	var err error
+	stderr := captureStderr(t, func() {
+		ep, err = ResolveEndpoint(cfgPath)
+	})
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "legacy-from-cmd" {
+		t.Errorf("Token = %q, want %q (whitespace-only auth_token must not shadow auth_token_cmd)", ep.Token, "legacy-from-cmd")
+	}
+	if strings.Contains(stderr, "both auth_token and auth_token_cmd") {
+		t.Errorf("warned about a shadowed command that was actually used; stderr: %q", stderr)
+	}
+}
+
+// (c) custom provider with api_key_cmd resolves (custom providers have no env fallback).
+func TestResolveEndpoint_CustomProviderAPIKeyCmd(t *testing.T) {
+	clearAllEnv(t)
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "my-gateway",
+		CustomProviders: map[string]providerEntryConfig{
+			"my-gateway": {
+				APIKeyCmd: "printf 'gw-token\\n'",
+				URL:       "https://gateway.internal.com/v1",
+				Protocol:  "openai",
+				Model:     "llama-3-8b",
+			},
+		},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "gw-token" {
+		t.Errorf("Token = %q, want %q", ep.Token, "gw-token")
+	}
+}
+
+// (d) a failing api_key_cmd is a hard error, not a silent fallback.
+func TestResolveEndpoint_ProviderAPIKeyCmdFailsHard(t *testing.T) {
+	clearAllEnv(t)
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {APIKeyCmd: "exit 7", Model: "claude-sonnet-4-6"},
+		},
+	})
+	_, err := ResolveEndpoint(cfgPath)
+	if err == nil {
+		t.Fatal("expected hard error from failing api_key_cmd, got nil")
+	}
+	if !strings.Contains(err.Error(), "api_key_cmd") {
+		t.Errorf("error %q does not mention api_key_cmd", err.Error())
+	}
+}
+
+// (d2) the property the design calls non-negotiable: a misconfigured credential
+// command must never silently downgrade to an env var. TestResolveEndpoint_
+// ProviderAPIKeyCmdFailsHard runs under clearAllEnv, so it would still pass if
+// someone reintroduced an env-var fallback on command failure; this one sets the
+// preset's env var so that regression cannot hide.
+func TestResolveEndpoint_APIKeyCmdFailureDoesNotFallBackToEnv(t *testing.T) {
+	clearAllEnv(t)
+	t.Setenv("ANTHROPIC_API_KEY", "env-api-key")
+	cfgPath := writeConfigJSON(t, configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {APIKeyCmd: "exit 7", Model: "claude-sonnet-4-6"},
+		},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err == nil {
+		t.Fatalf("expected hard error from failing api_key_cmd, got nil (Token %q)", ep.Token)
+	}
+	if !strings.Contains(err.Error(), "api_key_cmd") {
+		t.Errorf("error %q does not mention api_key_cmd", err.Error())
+	}
+	// Not an assertion on ep: every error path returns a zero ResolvedEndpoint, so
+	// ep.Token is "" by construction whenever err != nil. The witness that no
+	// fallback happened is err being non-nil at all -- with the env var set, a
+	// silent fallback would have returned success.
+}
+
+// (e) legacy auth_token_cmd resolves on an otherwise-complete llm block.
+func TestResolveEndpoint_LegacyAuthTokenCmd(t *testing.T) {
+	clearAllEnv(t)
+	cfgPath := writeConfigJSON(t, configFile{
+		Llm: llmFileConfig{
+			URL:          "https://api.example.com/v1/messages",
+			AuthTokenCmd: "printf 'legacy-token\\n'",
+			Model:        "claude-sonnet-4-6",
+		},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err != nil {
+		t.Fatalf("unexpected error: %v", err)
+	}
+	if ep.Token != "legacy-token" {
+		t.Errorf("Token = %q, want %q", ep.Token, "legacy-token")
+	}
+}
+
+// (e3) legacy path: an otherwise-complete llm block whose auth_token_cmd fails is
+// a hard error. The Claude Code env vars are set to prove it does not fall through
+// to that strategy -- a failing credential command must not be papered over by a
+// lower-priority source.
+func TestResolveEndpoint_LegacyAuthTokenCmdFailsHard(t *testing.T) {
+	clearAllEnv(t)
+	t.Setenv("ANTHROPIC_BASE_URL", "https://cc.example.com")
+	t.Setenv("ANTHROPIC_AUTH_TOKEN", "cc-env-token")
+	t.Setenv("ANTHROPIC_MODEL", "claude-sonnet-4-6")
+	cfgPath := writeConfigJSON(t, configFile{
+		Llm: llmFileConfig{
+			URL:          "https://api.example.com/v1/messages",
+			AuthTokenCmd: "exit 9",
+			Model:        "claude-sonnet-4-6",
+		},
+	})
+	ep, err := ResolveEndpoint(cfgPath)
+	if err == nil {
+		t.Fatalf("expected hard error from failing auth_token_cmd, got nil (Source %q, Token %q)", ep.Source, ep.Token)
+	}
+	if !strings.Contains(err.Error(), "auth_token_cmd") {
+		t.Errorf("error %q does not mention auth_token_cmd", err.Error())
+	}
+}
+
+// (f) an incomplete legacy block (missing url) with auth_token_cmd set does NOT
+// run the command and falls through to later strategies.
+func TestResolveEndpoint_LegacyIncompleteDoesNotRunCmd(t *testing.T) {
+	clearAllEnv(t)
+	// Command would exit non-zero if ever executed; if it ran, we'd see that
+	// error instead of the generic "no valid endpoint" fall-through error.
+	cfgPath := writeConfigJSON(t, configFile{
+		Llm: llmFileConfig{
+			AuthTokenCmd: "exit 9",
+			Model:        "claude-sonnet-4-6",
+			// URL intentionally omitted -> incomplete
+		},
+	})
+	_, err := ResolveEndpoint(cfgPath)
+	if err == nil {
+		t.Fatal("expected no-endpoint error, got nil")
+	}
+	if strings.Contains(err.Error(), "auth_token_cmd") {
+		t.Errorf("command should not have run for incomplete legacy config; error: %v", err)
+	}
+	if !strings.Contains(err.Error(), "no valid LLM endpoint") {
+		t.Errorf("expected fall-through no-endpoint error, got: %v", err)
+	}
+}
diff --git a/internal/llm/resolver_test.go b/internal/llm/resolver_test.go
index ec329788..a59b9c4e 100644
--- a/internal/llm/resolver_test.go
+++ b/internal/llm/resolver_test.go
@@ -266,6 +266,14 @@ func clearAllEnv(t *testing.T) {
 	} {
 		t.Setenv(k, "")
 	}
+	// Point os.UserHomeDir at an empty dir so the tryShellRC strategy cannot read
+	// the developer's (or a self-hosted CI runner's) real ~/.zshrc: one exporting
+	// the ANTHROPIC_* trio would resolve a live endpoint and break every test that
+	// asserts resolution fails. HOME covers Unix, USERPROFILE Windows; setting the
+	// one that does not apply is harmless.
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	t.Setenv("USERPROFILE", home)
 }
 
 func TestResolveEndpoint_ProviderAnthropic(t *testing.T) {
@@ -569,6 +577,38 @@ func TestResolveEndpoint_CustomProviderMissingFields(t *testing.T) {
 	}
 }
 
+func TestResolveEndpoint_CustomProviderNoEnvFallback(t *testing.T) {
+	clearAllEnv(t)
+	// A preset provider would pick this up; a custom provider must not, since it
+	// has no associated env var. The api_key/api_key_cmd precedence relies on it.
+	t.Setenv("ANTHROPIC_API_KEY", "env-api-key")
+
+	cfg := configFile{
+		Provider: "my-gateway",
+		CustomProviders: map[string]providerEntryConfig{
+			"my-gateway": {
+				URL:      "https://gateway.internal.com/v1",
+				Protocol: "openai",
+				Model:    "llama-3-70b",
+				// No api_key and no api_key_cmd.
+			},
+		},
+	}
+	data, _ := json.Marshal(cfg)
+	cfgPath := filepath.Join(t.TempDir(), "config.json")
+	if err := os.WriteFile(cfgPath, data, 0644); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+
+	_, err := ResolveEndpoint(cfgPath)
+	if err == nil {
+		t.Fatal("expected error: custom providers have no environment variable fallback")
+	}
+	if !strings.Contains(err.Error(), "no api_key or api_key_cmd configured") {
+		t.Errorf("error = %v, want the missing-credential error", err)
+	}
+}
+
 func TestResolveEndpoint_CustomProviderModelFromTopLevel(t *testing.T) {
 	clearAllEnv(t)
 
@@ -732,6 +772,110 @@ func TestResolveEndpointWithModelOverride_InvalidModelInPresetList(t *testing.T)
 	}
 }
 
+func TestResolveEndpointWithModelOverride_InvalidModelDoesNotRunAPIKeyCmd(t *testing.T) {
+	clearAllEnv(t)
+
+	// The command is guaranteed to fail, so the error it would produce doubles as
+	// a witness that it ran: a bad --model must fail on validation instead, with
+	// no secret-manager prompt.
+	cfg := configFile{
+		Provider: "anthropic",
+		Providers: map[string]providerEntryConfig{
+			"anthropic": {APIKeyCmd: "ocr-no-such-secret-command", Model: "claude-sonnet-4-6"},
+		},
+	}
+	data, _ := json.Marshal(cfg)
+	cfgPath := filepath.Join(t.TempDir(), "config.json")
+	if err := os.WriteFile(cfgPath, data, 0644); err != nil {
+		t.Fatalf("write config: %v", err)
+	}
+
+	_, err := ResolveEndpointWithModelOverride(cfgPath, "claude-opsu-4-6")
+	if err == nil {
+		t.Fatal("expected error for invalid model override")
+	}
+	if !strings.Contains(err.Error(), "not available for provider") {
+		t.Errorf("error message should mention model unavailability, got: %v", err)
+	}
+	if strings.Contains(err.Error(), "api_key_cmd") {
+		t.Errorf("api_key_cmd ran before model validation, got: %v", err)
+	}
+}
+
+// A bad global env override must be rejected before any strategy runs, for the
+// same reason as the model check above: OCR_LLM_TIMEOUT="30s" (the field wants a
+// bare integer) used to be parsed only after an endpoint resolved, so the user
+// authenticated to 1Password/Touch ID and then got a config error. Same witness
+// trick: the command cannot succeed, so its error proves it ran.
+func TestResolveEndpointWithModelOverride_BadEnvOverrideDoesNotRunAPIKeyCmd(t *testing.T) {
+	tests := []struct {
+		name     string
+		env      string
+		value    string
+		wantErr  string
+		wantErr2 string
+	}{
+		{
+			name:    "non-integer timeout",
+			env:     "OCR_LLM_TIMEOUT",
+			value:   "30s",
+			wantErr: "OCR_LLM_TIMEOUT must be an integer (seconds)",
+		},
+		{
+			name:    "negative timeout",
+			env:     "OCR_LLM_TIMEOUT",
+			value:   "-30",
+			wantErr: "OCR_LLM_TIMEOUT",
+		},
+		{
+			name:     "reserved extra header",
+			env:      "OCR_LLM_EXTRA_HEADERS",
+			value:    "authorization=leak",
+			wantErr:  "OCR_LLM_EXTRA_HEADERS",
+			wantErr2: "reserved header",
+		},
+		{
+			name:     "malformed extra header",
+			env:      "OCR_LLM_EXTRA_HEADERS",
+			value:    "no-equals-sign",
+			wantErr:  "OCR_LLM_EXTRA_HEADERS",
+			wantErr2: "expected key=value",
+		},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			clearAllEnv(t)
+			t.Setenv(tt.env, tt.value)
+
+			cfg := configFile{
+				Provider: "anthropic",
+				Providers: map[string]providerEntryConfig{
+					"anthropic": {APIKeyCmd: "ocr-no-such-secret-command", Model: "claude-sonnet-4-6"},
+				},
+			}
+			data, _ := json.Marshal(cfg)
+			cfgPath := filepath.Join(t.TempDir(), "config.json")
+			if err := os.WriteFile(cfgPath, data, 0644); err != nil {
+				t.Fatalf("write config: %v", err)
+			}
+
+			_, err := ResolveEndpoint(cfgPath)
+			if err == nil {
+				t.Fatalf("expected error for %s=%q", tt.env, tt.value)
+			}
+			if !strings.Contains(err.Error(), tt.wantErr) {
+				t.Errorf("error %q does not contain %q", err.Error(), tt.wantErr)
+			}
+			if tt.wantErr2 != "" && !strings.Contains(err.Error(), tt.wantErr2) {
+				t.Errorf("error %q does not contain %q", err.Error(), tt.wantErr2)
+			}
+			if strings.Contains(err.Error(), "api_key_cmd") {
+				t.Errorf("api_key_cmd ran before %s was validated, got: %v", tt.env, err)
+			}
+		})
+	}
+}
+
 func TestResolveEndpointWithModelOverride_ValidModelInCustomProviderList(t *testing.T) {
 	clearAllEnv(t)
 
diff --git a/internal/viewer/handler_test.go b/internal/viewer/handler_test.go
index c24349ce..53186be7 100644
--- a/internal/viewer/handler_test.go
+++ b/internal/viewer/handler_test.go
@@ -5,6 +5,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 )
@@ -67,6 +68,12 @@ func TestHandleRepos_UnreadableRoot(t *testing.T) {
 }
 
 func TestHandleRepos_PermissionDenied(t *testing.T) {
+	// Chmod(0000) on Windows only sets the read-only bit, so ReadDir still
+	// succeeds and the handler returns 200. (The Getuid guard below cannot cover
+	// this: Getuid returns -1 on Windows, never 0.)
+	if runtime.GOOS == "windows" {
+		t.Skip("unix permissions not enforced on Windows")
+	}
 	if os.Getuid() == 0 {
 		t.Skip("permission checks are bypassed for root")
 	}
diff --git a/internal/viewer/store_load_test.go b/internal/viewer/store_load_test.go
index a4da1dbf..d3d0bd5c 100644
--- a/internal/viewer/store_load_test.go
+++ b/internal/viewer/store_load_test.go
@@ -3,6 +3,7 @@ package viewer
 import (
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 )
 
@@ -392,6 +393,11 @@ func TestLoadSession_ToolCallWithoutRequest(t *testing.T) {
 }
 
 func TestDiscoverRepos_SkipsUnreadableSubdir(t *testing.T) {
+	// Chmod(0000) is only the read-only bit on Windows, so ReadDir still succeeds
+	// and the repo is discovered rather than skipped.
+	if runtime.GOOS == "windows" {
+		t.Skip("unix permissions not enforced on Windows")
+	}
 	if os.Getuid() == 0 {
 		t.Skip("permission checks are bypassed for root")
 	}
@@ -418,6 +424,11 @@ func TestDiscoverRepos_SkipsUnreadableSubdir(t *testing.T) {
 }
 
 func TestListSessions_SkipsUnreadableFiles(t *testing.T) {
+	// Chmod(0000) is only the read-only bit on Windows, so the "bad" file is still
+	// readable and gets counted as a second session.
+	if runtime.GOOS == "windows" {
+		t.Skip("unix permissions not enforced on Windows")
+	}
 	if os.Getuid() == 0 {
 		t.Skip("permission checks are bypassed for root")
 	}
diff --git a/pages/src/content/docs/en/configuration.md b/pages/src/content/docs/en/configuration.md
index 1f3c19e0..f941cd02 100644
--- a/pages/src/content/docs/en/configuration.md
+++ b/pages/src/content/docs/en/configuration.md
@@ -125,6 +125,41 @@ The `timeout_sec` keys are not supported by `ocr config set` — edit
 }
 ```
 
+### API key from a command
+
+Instead of storing a key in the config file, `api_key_cmd` fetches it at
+runtime from a secret manager (1Password, `pass`, `gopass`, …). Its trimmed,
+single-line stdout becomes the key. The same option is available for the
+legacy `llm` block as `auth_token_cmd`.
+
+```bash
+ocr config set providers.anthropic.api_key_cmd "op read op://dev/anthropic/api-key"
+```
+
+Precedence: a static `api_key` always wins (if both are set, the command is
+ignored and a warning is printed); otherwise `api_key_cmd` runs; only if
+neither is set does OCR fall back to the provider's environment variable.
+
+The command runs once per `ocr` invocation and must succeed: a non-zero exit,
+empty output, multi-line output, or more than 64KiB of output is a hard error
+(OCR never silently falls back). It must complete within 60 seconds, which
+includes any time you spend answering a prompt. The command inherits your
+terminal's stdin and stderr, so interactive prompts (pinentry, Touch ID) both
+appear and can be answered. If the command leaves a background daemon holding
+its stdout pipe (`gpg-agent`, a first-use `op` daemon), the credential still
+arrives but every `ocr` run pauses an extra 5 seconds waiting for that pipe to
+close — redirect the daemon's output (`>/dev/null 2>&1`) to get rid of the wait.
+
+On Windows the command runs through `cmd.exe`, not `sh`, so a command written
+for one is generally not portable to the other: `%VAR%` and `^` are `cmd.exe`
+metacharacters, while `$VAR` expansion and `\` escaping do not apply there.
+Quoted arguments are passed through verbatim, so
+`op read "op://Private/My Vault/api-key"` works as written.
+
+Since the value is executed as a shell command, `config.json` is trusted
+input — keep it owned by you and not writable by anyone else (OCR writes it
+with `0600` permissions).
+
 ### Verify connectivity
 
 ```bash
diff --git a/pages/src/content/docs/en/review-rules.md b/pages/src/content/docs/en/review-rules.md
index 0da420c0..647b8445 100644
--- a/pages/src/content/docs/en/review-rules.md
+++ b/pages/src/content/docs/en/review-rules.md
@@ -141,7 +141,8 @@ text the agent should follow:
 3. Try `~/.opencodereview/rule.json` in declaration order.
 4. Fall back to the embedded system rule layer.
 
-The embedded `system_rules.json` ships with these patterns (in order):
+Selected embedded `system_rules.json` patterns are shown below in relative
+matching order:
 
 | Pattern | Rule doc |
 |---|---|
@@ -151,6 +152,7 @@ The embedded `system_rules.json` ships with these patterns (in order):
 | `**/build.gradle` | `build_gradle.md` — Gradle dependencies. |
 | `**/package.json` | `package_json.md` — NPM dependencies / scripts. |
 | `**/Cargo.toml` | `cargo_toml.md` — Rust manifest. |
+| `**/composer.json` | `composer_json.md` — Composer dependencies, autoloading, scripts, plugins, and package configuration. |
 | `**/*.{json,json5}` | `json.md` — generic JSON (also matches `.json5`). |
 | `.github/workflows/**/*.{yaml,yml}` | `github_workflows.md` — GitHub Actions workflow YAML. |
 | `.github/**/*.{yaml,yml}` | `github_config.md` — other `.github` config YAML. |
@@ -163,6 +165,7 @@ The embedded `system_rules.json` ships with these patterns (in order):
 | `**/*.rs` | `rust.md` |
 | `**/*.{cpp,cc,hpp}` | `cpp.md` |
 | `**/*.c` | `c.md` |
+| `**/*.{php,phtml}` | `php.md` — PHP source and PHP templates. |
 | *(fallback)* | `default.md` |
 
 The resolved rule body becomes the `{{system_rule}}` placeholder in the
diff --git a/pages/src/content/docs/ja/configuration.md b/pages/src/content/docs/ja/configuration.md
index 64172888..a438397f 100644
--- a/pages/src/content/docs/ja/configuration.md
+++ b/pages/src/content/docs/ja/configuration.md
@@ -123,6 +123,41 @@ Ollama は API key を無視しますが、カスタム provider は空でない
 }
 ```
 
+### API key をコマンドで取得する
+
+key を設定ファイルに保存する代わりに、`api_key_cmd` で実行時にシークレット
+マネージャー（1Password、`pass`、`gopass` など）から取得できます。前後の空白を
+除いた 1 行の stdout が key になります。レガシーの `llm` ブロックにも同等の
+`auth_token_cmd` があります。
+
+```bash
+ocr config set providers.anthropic.api_key_cmd "op read op://dev/anthropic/api-key"
+```
+
+優先順位：静的な `api_key` が常に優先されます（両方設定されている場合はコマンドを
+無視し、警告を表示します）。それ以外の場合は `api_key_cmd` を実行します。どちらも
+設定されていない場合のみ、OCR は provider の環境変数にフォールバックします。
+
+コマンドは `ocr` 実行ごとに 1 回実行され、成功する必要があります。非ゼロ終了、
+空の出力、複数行の出力、64KiB を超える出力はいずれもハードエラーです（OCR が黙って
+フォールバックすることはありません）。コマンドはプロンプトへの応答時間も含めて
+60 秒以内に完了する必要があります。コマンドは端末の stdin と stderr を引き継ぐため、
+対話的なプロンプト（pinentry、Touch ID）は表示も応答も可能です。コマンドが stdout
+パイプを保持したままバックグラウンドのデーモン（`gpg-agent`、初回起動時の `op`
+デーモン）を残すと、認証情報は取得できるものの `ocr` の実行ごとにパイプが閉じるのを
+5 秒余分に待つことになるため、デーモンの出力をリダイレクト（`>/dev/null 2>&1`）
+してください。
+
+Windows ではコマンドは `sh` ではなく `cmd.exe` 経由で実行されるため、一方向けに
+書いたコマンドは通常そのままでは移植できません。`%VAR%` と `^` は `cmd.exe` の
+メタ文字であり、`$VAR` の展開や `\` によるエスケープは適用されません。引用符付きの
+引数はそのまま渡されるため、`op read "op://Private/My Vault/api-key"` は記述どおりに
+動作します。
+
+この値は shell コマンドとして実行されるため、`config.json` は信頼された入力です。
+自分の所有のまま、他のユーザーが書き込めない状態に保ってください（OCR は `0600`
+で書き込みます）。
+
 ### 接続性を検証する
 
 ```bash
diff --git a/pages/src/content/docs/ja/review-rules.md b/pages/src/content/docs/ja/review-rules.md
index 56a4110e..78984511 100644
--- a/pages/src/content/docs/ja/review-rules.md
+++ b/pages/src/content/docs/ja/review-rules.md
@@ -104,7 +104,7 @@ OCR は [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
 3. 宣言順に `~/.opencodereview/rule.json` を試します。
 4. 埋め込みのシステムルール層にフォールバックします。
 
-埋め込みの `system_rules.json` には次のパターンが同梱されています（順序どおり）:
+埋め込みの `system_rules.json` から主なパターンを相対的なマッチ順で示します:
 
 | パターン | ルールドキュメント |
 |---|---|
@@ -114,6 +114,7 @@ OCR は [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
 | `**/build.gradle` | `build_gradle.md`: Gradle 依存関係。 |
 | `**/package.json` | `package_json.md`: NPM 依存関係 / スクリプト。 |
 | `**/Cargo.toml` | `cargo_toml.md`: Rust manifest。 |
+| `**/composer.json` | `composer_json.md`: Composer の依存関係、自動読み込み、スクリプト、プラグイン、パッケージ設定。 |
 | `**/*.{json,json5}` | `json.md`: 汎用 JSON（`.json5` にも一致）。 |
 | `.github/workflows/**/*.{yaml,yml}` | `github_workflows.md`: GitHub Actions ワークフロー YAML。 |
 | `.github/**/*.{yaml,yml}` | `github_config.md`: その他の `.github` 設定 YAML。 |
@@ -126,6 +127,7 @@ OCR は [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
 | `**/*.rs` | `rust.md` |
 | `**/*.{cpp,cc,hpp}` | `cpp.md` |
 | `**/*.c` | `c.md` |
+| `**/*.{php,phtml}` | `php.md`: PHP ソースと PHP テンプレート。 |
 | *(fallback)* | `default.md` |
 
 解決されたルール本文は、plan および main task prompt 内の `{{system_rule}}` プレースホルダーの内容になります。
diff --git a/pages/src/content/docs/zh/configuration.md b/pages/src/content/docs/zh/configuration.md
index ffb0914c..cef477ae 100644
--- a/pages/src/content/docs/zh/configuration.md
+++ b/pages/src/content/docs/zh/configuration.md
@@ -117,6 +117,35 @@ provider 没有环境变量回退），所以设任意占位值即可。模型
 }
 ```
 
+### 通过命令获取 API key
+
+除了把 key 直接写进配置文件，还可以用 `api_key_cmd` 在运行时从密钥管理器
+（1Password、`pass`、`gopass` 等）获取。命令去除首尾空白后的单行 stdout 即为
+key。旧版 `llm` 配置块也有对应的 `auth_token_cmd`。
+
+```bash
+ocr config set providers.anthropic.api_key_cmd "op read op://dev/anthropic/api-key"
+```
+
+优先级：静态 `api_key` 始终优先（两者都设置时忽略命令并打印警告）；否则运行
+`api_key_cmd`；只有两者都未设置时，OCR 才回退到 provider 对应的环境变量。
+
+命令在每次 `ocr` 调用时运行一次，且必须成功：非零退出、空输出、多行输出或超过
+64KiB 的输出都会被视为硬错误（OCR 绝不会静默回退）。命令须在 60 秒内完成，这也
+包括你回应提示所花的时间。命令会继承你终端的 stdin 和 stderr，因此交互式提示
+（pinentry、Touch ID）既能显示也能作答。如果命令留下了仍持有其 stdout 管道的后台
+守护进程（`gpg-agent`、首次使用时启动的 `op` 守护进程），凭据依然能取到，但每次
+`ocr` 调用都会额外等待 5 秒直到该管道关闭——把守护进程的输出重定向掉
+（`>/dev/null 2>&1`）即可消除这段等待。
+
+在 Windows 上命令通过 `cmd.exe` 而非 `sh` 执行，因此为其中一方编写的命令通常
+无法直接移植到另一方：`%VAR%` 和 `^` 是 `cmd.exe` 的元字符，而 `$VAR` 展开和 `\`
+转义在那里并不适用。带引号的参数会原样传递，因此
+`op read "op://Private/My Vault/api-key"` 可以按原样使用。
+
+由于这个值会作为 shell 命令执行，`config.json` 属于可信输入——请确保它归你所有、
+其他用户不可写（OCR 写入时使用 `0600` 权限）。
+
 ### 验证连通性
 
 ```bash
diff --git a/pages/src/content/docs/zh/review-rules.md b/pages/src/content/docs/zh/review-rules.md
index 12bed5e1..6c95d4d2 100644
--- a/pages/src/content/docs/zh/review-rules.md
+++ b/pages/src/content/docs/zh/review-rules.md
@@ -125,7 +125,7 @@ OCR 用 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
 3. 按声明顺序试 `~/.opencodereview/rule.json`。
 4. 回退到内嵌系统规则层。
 
-内嵌 `system_rules.json` 自带这些模式（按序）：
+以下是内嵌 `system_rules.json` 的部分模式，按相对匹配顺序排列：
 
 | 模式 | 规则文档 |
 |---|---|
@@ -135,6 +135,7 @@ OCR 用 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
 | `**/build.gradle` | `build_gradle.md`——Gradle 依赖。 |
 | `**/package.json` | `package_json.md`——NPM 依赖 / 脚本。 |
 | `**/Cargo.toml` | `cargo_toml.md`——Rust manifest。 |
+| `**/composer.json` | `composer_json.md`——Composer 依赖、自动加载、脚本、插件和包配置。 |
 | `**/*.{json,json5}` | `json.md`——通用 JSON（也匹配 `.json5`）。 |
 | `.github/workflows/**/*.{yaml,yml}` | `github_workflows.md`——GitHub Actions 工作流 YAML。 |
 | `.github/**/*.{yaml,yml}` | `github_config.md`——其他 `.github` 配置 YAML。 |
@@ -147,6 +148,7 @@ OCR 用 [`bmatcuk/doublestar/v4`](https://pkg.go.dev/github.com/bmatcuk/doublest
 | `**/*.rs` | `rust.md` |
 | `**/*.{cpp,cc,hpp}` | `cpp.md` |
 | `**/*.c` | `c.md` |
+| `**/*.{php,phtml}` | `php.md`——PHP 源代码和 PHP 模板。 |
 | *(fallback)* | `default.md` |
 
 解析出的规则正文成为 plan 和 main task prompt 中 `{{system_rule}}` 占位符的内容。