From 0a72434993786646c1ac55dbdbeb3b57391473cf Mon Sep 17 00:00:00 2001 From: Siaochuan Date: Thu, 9 Apr 2026 17:36:12 -0700 Subject: [PATCH] Add provider-scoped env key resolution --- src/openharness/config/settings.py | 78 ++++++++++++++++------------ src/openharness/swarm/spawn_utils.py | 5 ++ tests/test_config/test_settings.py | 61 +++++++++++++++++++++- tests/test_swarm/test_spawn_utils.py | 18 +++++++ 4 files changed, 128 insertions(+), 34 deletions(-) create mode 100644 tests/test_swarm/test_spawn_utils.py diff --git a/src/openharness/config/settings.py b/src/openharness/config/settings.py index bea3df0d..6bf2dd3f 100644 --- a/src/openharness/config/settings.py +++ b/src/openharness/config/settings.py @@ -284,6 +284,26 @@ def auth_source_uses_api_key(auth_source: str) -> bool: return auth_source.endswith("_api_key") +def auth_source_env_var_candidates(auth_source: str) -> tuple[str, ...]: + """Return env vars to probe for an auth source in precedence order.""" + mapping = { + "anthropic_api_key": ("OPENHARNESS_ANTHROPIC_API_KEY", "ANTHROPIC_API_KEY"), + "openai_api_key": ("OPENHARNESS_OPENAI_API_KEY", "OPENAI_API_KEY"), + "dashscope_api_key": ("OPENHARNESS_DASHSCOPE_API_KEY", "DASHSCOPE_API_KEY"), + "moonshot_api_key": ("OPENHARNESS_MOONSHOT_API_KEY", "MOONSHOT_API_KEY"), + } + return mapping.get(auth_source, ()) + + +def resolve_auth_env_value(auth_source: str) -> tuple[str, str] | None: + """Return the first configured env var/value pair for an auth source.""" + for env_var in auth_source_env_var_candidates(auth_source): + env_value = os.environ.get(env_var, "") + if env_value: + return env_var, env_value + return None + + def credential_storage_provider_name(profile_name: str, profile: ProviderProfile) -> str: """Return the storage namespace used for this profile's credential. @@ -527,19 +547,15 @@ def resolve_api_key(self) -> str: if self.api_key: return self.api_key - env_key = os.environ.get("ANTHROPIC_API_KEY", "") - if env_key: - return env_key - - # Also check OPENAI_API_KEY for openai-format providers - openai_key = os.environ.get("OPENAI_API_KEY", "") - if openai_key: - return openai_key + env_resolved = resolve_auth_env_value(profile.auth_source) + if env_resolved: + _, env_value = env_resolved + return env_value raise ValueError( - "No API key found. Set ANTHROPIC_API_KEY (or OPENAI_API_KEY for openai-format " - "providers) environment variable, or configure api_key in " - "~/.openharness/settings.json" + "No API key found. Set an OPENHARNESS_* provider key " + "(preferred) or the matching native provider environment variable, " + "or configure api_key in ~/.openharness/settings.json" ) def resolve_auth(self) -> ResolvedAuth: @@ -606,22 +622,16 @@ def resolve_auth(self) -> ResolvedAuth: storage_provider = credential_storage_provider_name(profile_name, profile) - env_var = { - "anthropic_api_key": "ANTHROPIC_API_KEY", - "openai_api_key": "OPENAI_API_KEY", - "dashscope_api_key": "DASHSCOPE_API_KEY", - "moonshot_api_key": "MOONSHOT_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", - ) + env_resolved = resolve_auth_env_value(auth_source) + if env_resolved: + env_var, env_value = env_resolved + return ResolvedAuth( + provider=provider or storage_provider, + auth_kind="api_key", + value=env_value, + source=f"env:{env_var}", + state="configured", + ) explicit_key = "" if profile.credential_slot else self.api_key if explicit_key: @@ -666,14 +676,17 @@ def merge_cli_overrides(self, **overrides: Any) -> Settings: def _apply_env_overrides(settings: Settings) -> Settings: """Apply supported environment variable overrides over loaded settings.""" updates: dict[str, Any] = {} - model = os.environ.get("ANTHROPIC_MODEL") or os.environ.get("OPENHARNESS_MODEL") + profile_name, profile = settings.resolve_profile() + del profile_name + + model = os.environ.get("OPENHARNESS_MODEL") or os.environ.get("ANTHROPIC_MODEL") if model: updates["model"] = model base_url = ( - os.environ.get("ANTHROPIC_BASE_URL") + os.environ.get("OPENHARNESS_BASE_URL") + or 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 @@ -686,8 +699,9 @@ def _apply_env_overrides(settings: Settings) -> Settings: if max_turns: updates["max_turns"] = int(max_turns) - api_key = os.environ.get("ANTHROPIC_API_KEY") or os.environ.get("OPENAI_API_KEY") - if api_key: + env_resolved = resolve_auth_env_value(profile.auth_source) + if env_resolved: + _, api_key = env_resolved updates["api_key"] = api_key api_format = os.environ.get("OPENHARNESS_API_FORMAT") diff --git a/src/openharness/swarm/spawn_utils.py b/src/openharness/swarm/spawn_utils.py index 4b592ec2..e3b300a1 100644 --- a/src/openharness/swarm/spawn_utils.py +++ b/src/openharness/swarm/spawn_utils.py @@ -61,8 +61,13 @@ # These are read by settings._apply_env_overrides() and must survive across # tmux boundaries so teammates use the same provider as the leader. "OPENHARNESS_API_FORMAT", + "OPENHARNESS_PROVIDER", "OPENHARNESS_BASE_URL", "OPENHARNESS_MODEL", + "OPENHARNESS_ANTHROPIC_API_KEY", + "OPENHARNESS_OPENAI_API_KEY", + "OPENHARNESS_DASHSCOPE_API_KEY", + "OPENHARNESS_MOONSHOT_API_KEY", "OPENAI_API_KEY", ] diff --git a/tests/test_config/test_settings.py b/tests/test_config/test_settings.py index 3bdcb10a..654698e3 100644 --- a/tests/test_config/test_settings.py +++ b/tests/test_config/test_settings.py @@ -35,16 +35,26 @@ def test_resolve_api_key_from_instance(self): assert s.resolve_api_key() == "sk-test-123" def test_resolve_api_key_from_env(self, monkeypatch): + monkeypatch.delenv("OPENHARNESS_ANTHROPIC_API_KEY", raising=False) monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-env-456") s = Settings() assert s.resolve_api_key() == "sk-env-456" + def test_resolve_api_key_prefers_openharness_env(self, monkeypatch): + monkeypatch.setenv("OPENHARNESS_ANTHROPIC_API_KEY", "sk-oh-456") + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-env-456") + s = Settings() + assert s.resolve_api_key() == "sk-oh-456" + def test_resolve_api_key_instance_takes_precedence(self, monkeypatch): + monkeypatch.delenv("OPENHARNESS_ANTHROPIC_API_KEY", raising=False) monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-env-456") s = Settings(api_key="sk-instance-789") assert s.resolve_api_key() == "sk-instance-789" def test_resolve_api_key_missing_raises(self, monkeypatch): + monkeypatch.delenv("OPENHARNESS_ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENHARNESS_OPENAI_API_KEY", raising=False) monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) monkeypatch.delenv("OPENAI_API_KEY", raising=False) s = Settings() @@ -69,6 +79,7 @@ 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.delenv("OPENHARNESS_OPENAI_API_KEY", raising=False) 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") @@ -77,9 +88,21 @@ def test_resolve_auth_prefers_env_over_flat_api_key_for_openai(self, monkeypatch assert auth.value == "sk-openai-correct" assert "OPENAI" in auth.source + def test_resolve_auth_prefers_openharness_env_for_openai(self, monkeypatch): + monkeypatch.setenv("OPENHARNESS_OPENAI_API_KEY", "sk-oh-openai") + 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-oh-openai" + assert auth.source == "env:OPENHARNESS_OPENAI_API_KEY" + 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("OPENHARNESS_ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENHARNESS_OPENAI_API_KEY", raising=False) monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) monkeypatch.delenv("OPENAI_API_KEY", raising=False) s = Settings(api_key="sk-fallback-key") @@ -90,8 +113,8 @@ def test_resolve_auth_falls_back_to_flat_api_key(self, monkeypatch): 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_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") @@ -100,8 +123,18 @@ def test_env_overrides_picks_up_openai_base_url(self, tmp_path: Path, monkeypatc s = load_settings(path) assert s.base_url == "https://relay.example.com/v1" + def test_openharness_base_url_takes_precedence_over_native_vars(self, tmp_path: Path, monkeypatch): + monkeypatch.setenv("OPENHARNESS_BASE_URL", "https://openharness-relay.example.com") + 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://openharness-relay.example.com" + 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("OPENHARNESS_BASE_URL", raising=False) 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") @@ -110,14 +143,34 @@ def test_anthropic_base_url_takes_precedence_over_openai(self, tmp_path: Path, m s = load_settings(path) assert s.base_url == "https://anthropic-relay.example.com" + def test_load_settings_uses_profile_specific_env_key(self, tmp_path: Path, monkeypatch): + monkeypatch.setenv("ANTHROPIC_API_KEY", "sk-ant-wrong") + monkeypatch.setenv("OPENHARNESS_OPENAI_API_KEY", "sk-oh-openai") + path = tmp_path / "settings.json" + path.write_text( + json.dumps( + { + "active_profile": "openai-compatible", + "profiles": Settings().profiles, + }, + default=lambda value: value.model_dump() if hasattr(value, "model_dump") else value, + ), + encoding="utf-8", + ) + s = load_settings(path) + assert s.active_profile == "openai-compatible" + assert s.api_key == "sk-oh-openai" + class TestLoadSaveSettings: def test_load_missing_file_returns_defaults(self, tmp_path: Path, monkeypatch): + monkeypatch.delenv("OPENHARNESS_ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENHARNESS_OPENAI_API_KEY", raising=False) + monkeypatch.delenv("OPENHARNESS_BASE_URL", raising=False) 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" @@ -125,6 +178,8 @@ def test_load_missing_file_returns_defaults(self, tmp_path: Path, monkeypatch): assert s == Settings().materialize_active_profile() def test_load_existing_file(self, tmp_path: Path, monkeypatch): + monkeypatch.delenv("OPENHARNESS_ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENHARNESS_OPENAI_API_KEY", raising=False) monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) monkeypatch.delenv("OPENAI_API_KEY", raising=False) monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False) @@ -140,6 +195,8 @@ def test_load_existing_file(self, tmp_path: Path, monkeypatch): assert s.api_key == "" # default preserved def test_save_and_load_roundtrip(self, tmp_path: Path, monkeypatch): + monkeypatch.delenv("OPENHARNESS_ANTHROPIC_API_KEY", raising=False) + monkeypatch.delenv("OPENHARNESS_OPENAI_API_KEY", raising=False) monkeypatch.delenv("ANTHROPIC_API_KEY", raising=False) monkeypatch.delenv("OPENAI_API_KEY", raising=False) monkeypatch.delenv("ANTHROPIC_BASE_URL", raising=False) diff --git a/tests/test_swarm/test_spawn_utils.py b/tests/test_swarm/test_spawn_utils.py new file mode 100644 index 00000000..352bff2c --- /dev/null +++ b/tests/test_swarm/test_spawn_utils.py @@ -0,0 +1,18 @@ +from __future__ import annotations + +from openharness.swarm.spawn_utils import build_inherited_env_vars + + +def test_build_inherited_env_vars_includes_openharness_auth_vars(monkeypatch): + monkeypatch.setenv("OPENHARNESS_PROVIDER", "openai") + monkeypatch.setenv("OPENHARNESS_BASE_URL", "https://relay.example.com/v1") + monkeypatch.setenv("OPENHARNESS_OPENAI_API_KEY", "sk-oh-openai") + monkeypatch.setenv("OPENHARNESS_ANTHROPIC_API_KEY", "sk-oh-anthropic") + + env = build_inherited_env_vars() + + assert env["OPENHARNESS_AGENT_TEAMS"] == "1" + assert env["OPENHARNESS_PROVIDER"] == "openai" + assert env["OPENHARNESS_BASE_URL"] == "https://relay.example.com/v1" + assert env["OPENHARNESS_OPENAI_API_KEY"] == "sk-oh-openai" + assert env["OPENHARNESS_ANTHROPIC_API_KEY"] == "sk-oh-anthropic"