diff --git a/src/openharness/config/settings.py b/src/openharness/config/settings.py index 2da9600..3ac810f 100644 --- a/src/openharness/config/settings.py +++ b/src/openharness/config/settings.py @@ -249,8 +249,12 @@ def resolve_model_setting( return _CLAUDE_ALIAS_TARGETS[normalized] return normalize_anthropic_model_name(configured) - if provider in {"openai", "openai_codex", "copilot"} and normalized in {"default", "best"}: - return "gpt-5.4" + if provider in {"openai", "openai_codex", "copilot"}: + if normalized in {"default", "best"}: + return "gpt-5.4" + # Bare version numbers like "5.4" → "gpt-5.4" + if normalized and normalized[0].isdigit(): + return f"gpt-{configured}" return configured @@ -457,6 +461,11 @@ class Settings(BaseModel): profile_name, profile = self.resolve_profile() next_provider = (self.provider or "").strip() or profile.provider next_api_format = (self.api_format or "").strip() or profile.api_format + # When api_format switches to "openai" but provider is still the + # default "anthropic", infer provider as "openai" so model resolution + # and other provider-dependent logic uses the correct path. + if next_api_format == "openai" and next_provider == "anthropic": + next_provider = "openai" next_base_url = self.base_url if self.base_url is not None else profile.base_url flat_model = (self.model or "").strip() resolved_profile_model = resolve_model_setting( @@ -576,6 +585,31 @@ class Settings(BaseModel): ) storage_provider = auth_source_provider_name(auth_source) + + # Look up the provider-specific environment variable first. The flat + # ``self.api_key`` field is a legacy single-slot value that usually + # holds an Anthropic key. When the active profile points at a + # *different* provider (e.g. ``openai_api_key``), blindly returning + # ``self.api_key`` sends the wrong credential to the wrong backend. + # Checking the env var first ensures the correct key is used when the + # user has both ANTHROPIC_API_KEY and OPENAI_API_KEY configured. + env_var = { + "anthropic_api_key": "ANTHROPIC_API_KEY", + "openai_api_key": "OPENAI_API_KEY", + "dashscope_api_key": "DASHSCOPE_API_KEY", + }.get(auth_source) + if env_var: + env_value = os.environ.get(env_var, "") + if env_value: + return ResolvedAuth( + provider=provider or storage_provider, + auth_kind="api_key", + value=env_value, + source=f"env:{env_var}", + state="configured", + ) + + # Fall back to the flat api_key field (settings.json / --api-key). explicit_key = "" if profile.credential_slot else self.api_key if explicit_key: return ResolvedAuth( @@ -600,22 +634,6 @@ class Settings(BaseModel): state="configured", ) - env_var = { - "anthropic_api_key": "ANTHROPIC_API_KEY", - "openai_api_key": "OPENAI_API_KEY", - "dashscope_api_key": "DASHSCOPE_API_KEY", - }.get(auth_source) - if env_var: - env_value = os.environ.get(env_var, "") - if env_value: - return ResolvedAuth( - provider=provider or storage_provider, - auth_kind="api_key", - value=env_value, - source=f"env:{env_var}", - state="configured", - ) - stored = load_credential(storage_provider, "api_key") if stored: return ResolvedAuth( @@ -650,7 +668,11 @@ def _apply_env_overrides(settings: Settings) -> Settings: if model: updates["model"] = model - base_url = os.environ.get("ANTHROPIC_BASE_URL") or os.environ.get("OPENHARNESS_BASE_URL") + base_url = ( + os.environ.get("ANTHROPIC_BASE_URL") + or os.environ.get("OPENAI_BASE_URL") + or os.environ.get("OPENHARNESS_BASE_URL") + ) if base_url: updates["base_url"] = base_url diff --git a/tests/test_config/test_settings.py b/tests/test_config/test_settings.py index 4d61e97..d6e53a4 100644 --- a/tests/test_config/test_settings.py +++ b/tests/test_config/test_settings.py @@ -46,6 +46,7 @@ class TestSettings: def test_resolve_api_key_missing_raises(self, monkeypatch): monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) s = Settings() with pytest.raises(ValueError, match="No API key found"): s.resolve_api_key() @@ -64,14 +65,72 @@ class TestSettings: assert s.model != updated.model assert s is not updated + def test_resolve_auth_prefers_env_over_flat_api_key_for_openai(self, monkeypatch): + """When api_format=openai, resolve_auth() should use OPENAI_API_KEY + from the environment rather than the flat api_key field which may + contain an Anthropic key from settings.json.""" + monkeypatch.setenv("OPENAI_API_KEY", "sk-openai-correct") + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + s = Settings(api_key="sk-ant-wrong-provider", api_format="openai") + s = s.sync_active_profile_from_flat_fields() + auth = s.resolve_auth() + assert auth.value == "sk-openai-correct" + assert "OPENAI" in auth.source + + def test_resolve_auth_falls_back_to_flat_api_key(self, monkeypatch): + """When no provider-specific env var is set, resolve_auth() should + still fall back to the flat api_key field.""" + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + s = Settings(api_key="sk-fallback-key") + s = s.sync_active_profile_from_flat_fields() + auth = s.resolve_auth() + assert auth.value == "sk-fallback-key" + + def test_env_overrides_picks_up_openai_base_url(self, tmp_path: Path, monkeypatch): + """_apply_env_overrides should pick up OPENAI_BASE_URL for relay + providers that use OpenAI-compatible format.""" + monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False) + monkeypatch.delenv("OPENHARNESS_BASE_URL", raising=False) + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.setenv("OPENAI_BASE_URL", "https://relay.example.com/v1") + monkeypatch.setenv("OPENAI_API_KEY", "sk-relay-key") + path = tmp_path / "settings.json" + path.write_text(json.dumps({})) + s = load_settings(path) + assert s.base_url == "https://relay.example.com/v1" + + def test_anthropic_base_url_takes_precedence_over_openai(self, tmp_path: Path, monkeypatch): + """ANTHROPIC_BASE_URL should take precedence over OPENAI_BASE_URL.""" + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.setenv("ANTHROPIC_BASE_URL", "https://anthropic-relay.example.com") + monkeypatch.setenv("OPENAI_BASE_URL", "https://openai-relay.example.com/v1") + path = tmp_path / "settings.json" + path.write_text(json.dumps({})) + s = load_settings(path) + assert s.base_url == "https://anthropic-relay.example.com" + class TestLoadSaveSettings: - def test_load_missing_file_returns_defaults(self, tmp_path: Path): + def test_load_missing_file_returns_defaults(self, tmp_path: Path, monkeypatch): + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False) + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.delenv("OPENHARNESS_BASE_URL", raising=False) + monkeypatch.delenv("ANTHROPIC_MODEL", raising=False) + monkeypatch.delenv("OPENHARNESS_MODEL", raising=False) path = tmp_path / "nonexistent.json" s = load_settings(path) assert s == Settings().materialize_active_profile() - def test_load_existing_file(self, tmp_path: Path): + def test_load_existing_file(self, tmp_path: Path, monkeypatch): + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False) + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.delenv("ANTHROPIC_MODEL", raising=False) + monkeypatch.delenv("OPENHARNESS_MODEL", raising=False) path = tmp_path / "settings.json" path.write_text(json.dumps({"model": "claude-opus-4-20250514", "verbose": True, "fast_mode": True})) s = load_settings(path) @@ -80,7 +139,13 @@ class TestLoadSaveSettings: assert s.fast_mode is True assert s.api_key == "" # default preserved - def test_save_and_load_roundtrip(self, tmp_path: Path): + def test_save_and_load_roundtrip(self, tmp_path: Path, monkeypatch): + monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENAI_API_KEY", raising=False) + monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False) + monkeypatch.delenv("OPENAI_BASE_URL", raising=False) + monkeypatch.delenv("ANTHROPIC_MODEL", raising=False) + monkeypatch.delenv("OPENHARNESS_MODEL", raising=False) path = tmp_path / "settings.json" original = Settings(api_key="sk-roundtrip", model="claude-opus-4-20250514", verbose=True) save_settings(original, path)