loadConfig: let OCGO_API_KEY override the config file, as documented

cmd/ocgo/main.go

@@ -3298,11 +3298,28 @@ func saveConfig(cfg Config) error {
 }
 
 func loadConfig() (Config, error) {
-	cfg := Config{Host: defaultHost, Port: defaultPort, APIKey: os.Getenv("OCGO_API_KEY")}
+	// Precedence: explicit env var > config file > defaults.
+	//
+	// OCGO_API_KEY used to be seeded as the struct default BEFORE
+	// json.Unmarshal read the config file, so a stale `api_key` in
+	// config.json silently overwrote it -- the exact opposite of what the
+	// README promises ("You can also provide the key at runtime with an
+	// environment variable"). Setting the documented variable and watching
+	// every request still fail `401 Invalid API key`, with no indication
+	// which credential was actually used, is a rough way to lose an hour.
+	//
+	// Reading the file first and letting a non-empty env var win afterwards
+	// matches both the documentation and the usual 12-factor ordering, and
+	// makes it possible to source the key from a secret store at run time
+	// without ever writing it to disk.
+	cfg := Config{Host: defaultHost, Port: defaultPort}
 	b, err := os.ReadFile(configFile())
 	if err == nil {
 		_ = json.Unmarshal(b, &cfg)
 	}
+	if k := strings.TrimSpace(os.Getenv("OCGO_API_KEY")); k != "" {
+		cfg.APIKey = k
+	}
 	if cfg.APIKey == "" {
 		return cfg, errors.New("missing API key; run: ocgo setup")
 	}

cmd/ocgo/main_test.go

@@ -1140,3 +1140,61 @@ func TestSanitizeRawChatToolMessagesDropsLateToolMessage(t *testing.T) {
 		t.Fatalf("expected assistant after placeholder, got %+v", roles[2])
 	}
 }
+
+// writeTempConfig points HOME at a temp dir (configFile() resolves through
+// os.UserHomeDir) and writes a config.json holding fileKey.
+func writeTempConfig(t *testing.T, fileKey string) {
+	t.Helper()
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	dir := filepath.Join(home, ".config", "ocgo")
+	if err := os.MkdirAll(dir, 0o755); err != nil {
+		t.Fatal(err)
+	}
+	body := fmt.Sprintf(`{"api_key":%q,"host":"127.0.0.1","port":3456}`, fileKey)
+	if err := os.WriteFile(filepath.Join(dir, "config.json"), []byte(body), 0o600); err != nil {
+		t.Fatal(err)
+	}
+}
+
+func TestEnvKeyOverridesConfigFile(t *testing.T) {
+	// The regression this pins: OCGO_API_KEY used to be seeded as the struct
+	// default BEFORE the config file was unmarshalled over it, so a stale
+	// api_key silently won and every request failed 401 with no indication
+	// which credential was used -- while the README says the env var works.
+	writeTempConfig(t, "sk-stale-from-file")
+	t.Setenv("OCGO_API_KEY", "sk-fresh-from-env")
+	cfg, err := loadConfig()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if cfg.APIKey != "sk-fresh-from-env" {
+		t.Fatalf("APIKey = %q, want the env var to win over the config file", cfg.APIKey)
+	}
+}
+
+func TestConfigFileUsedWhenEnvUnset(t *testing.T) {
+	writeTempConfig(t, "sk-from-file")
+	t.Setenv("OCGO_API_KEY", "")
+	cfg, err := loadConfig()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if cfg.APIKey != "sk-from-file" {
+		t.Fatalf("APIKey = %q, want the config file value when no env var is set", cfg.APIKey)
+	}
+}
+
+func TestBlankEnvDoesNotClobberConfigFile(t *testing.T) {
+	// An exported-but-empty OCGO_API_KEY (e.g. `export OCGO_API_KEY=` in a
+	// profile) must not blank out a working config file.
+	writeTempConfig(t, "sk-from-file")
+	t.Setenv("OCGO_API_KEY", "   ")
+	cfg, err := loadConfig()
+	if err != nil {
+		t.Fatal(err)
+	}
+	if cfg.APIKey != "sk-from-file" {
+		t.Fatalf("APIKey = %q, want a whitespace-only env var ignored", cfg.APIKey)
+	}
+}