From 91cccc23bdf89192bd139ef5ddd3bae17c78d713 Mon Sep 17 00:00:00 2001 From: Manoj Prabhakar Paidiparthy Date: Fri, 29 May 2026 11:10:05 -0700 Subject: [PATCH] fix(cli): align Azure provider env var with docs (prefer AZURE_OPENAI_API_KEY, accept legacy AZURE_OPENAI_KEY) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Aligns the CLI's Azure provider detection with the rest of the project. The README, architecture presentation, cheatsheet, upstream amplifier-module-provider-azure-openai, and Azure OpenAI Python SDK all use AZURE_OPENAI_API_KEY, but the CLI was only honoring AZURE_OPENAI_KEY. After this change, the CLI prefers AZURE_OPENAI_API_KEY, accepts AZURE_OPENAI_KEY as a deprecated legacy alias with a one-time [WARN] on stderr, and preserves detection precedence (ANTHROPIC_API_KEY > OPENAI_API_KEY > AZURE_OPENAI_API_KEY > OLLAMA_HOST). Mirrors the upstream Azure provider module's dual-accept pattern (see __init__.py lines 119-120), which already prefers AZURE_OPENAI_API_KEY and falls back to AZURE_OPENAI_KEY. Verification: python_check clean on all 8 changed .py files, 53 tests pass on targeted test suite, full pytest shows same 23 pre-existing failures as origin/main (+2 new tests passing, no new failures). Manual verification confirms detection works with both spellings, legacy fallback triggers deprecation warning. Separate from PR #22 (version-info-and-error-message). ๐Ÿค– Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com> --- README.md | 2 + .../2026-05-18-aaa-v2-phase-2-cli-mode-a.md | 9 ++- src/amplifier_agent_cli/admin/config_show.py | 10 +++- src/amplifier_agent_cli/provider_detect.py | 57 +++++++++++++++---- src/amplifier_agent_cli/provider_sources.py | 44 +++++++++++++- tests/cli/test_config_show.py | 3 +- tests/cli/test_doctor.py | 8 ++- tests/cli/test_provider_detect.py | 36 +++++++++++- tests/cli/test_provider_sources.py | 17 ++++-- tests/cli/test_single_turn.py | 8 ++- 10 files changed, 164 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 6edcf989..0f9d105f 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,8 @@ Provider is auto-detected from environment variables in this precedence: Override with `--provider `. No `settings.yaml` to maintain. +> **Deprecated alias:** `AZURE_OPENAI_KEY` (without `_API_`) is still accepted as a fallback for backwards compatibility and triggers a one-time stderr warning when used. Prefer `AZURE_OPENAI_API_KEY` โ€” the legacy name will be removed in a future release. + ## Modes | Mode | Invocation | Caller | Lifecycle | diff --git a/docs/plans/2026-05-18-aaa-v2-phase-2-cli-mode-a.md b/docs/plans/2026-05-18-aaa-v2-phase-2-cli-mode-a.md index 28d9275e..6455fa13 100644 --- a/docs/plans/2026-05-18-aaa-v2-phase-2-cli-mode-a.md +++ b/docs/plans/2026-05-18-aaa-v2-phase-2-cli-mode-a.md @@ -150,7 +150,9 @@ git commit -m "feat(cli): scaffold amplifier_agent_cli package and add click dep - Create: `src/amplifier_agent_cli/provider_detect.py` - Create: `tests/cli/test_provider_detect.py` -**Design intent (from checkpoint ยง3, D5):** auto-detect provider from env vars. Precedence: `ANTHROPIC_API_KEY` โ†’ `OPENAI_API_KEY` โ†’ `AZURE_OPENAI_KEY` โ†’ `OLLAMA_HOST`. Honor `--provider` override. Raise structured error `provider_not_configured` when nothing is set. +**Design intent (from checkpoint ยง3, D5):** auto-detect provider from env vars. Precedence: `ANTHROPIC_API_KEY` โ†’ `OPENAI_API_KEY` โ†’ `AZURE_OPENAI_API_KEY` โ†’ `OLLAMA_HOST`. Honor `--provider` override. Raise structured error `provider_not_configured` when nothing is set. + +> **Implementation update (2026-05-29, PR following #22):** the Azure env var was originally specified as `AZURE_OPENAI_KEY`, but the README, the upstream `amplifier-module-provider-azure-openai` module, and the Azure OpenAI Python SDK all use `AZURE_OPENAI_API_KEY`. The CLI now prefers `AZURE_OPENAI_API_KEY` and accepts the legacy `AZURE_OPENAI_KEY` spelling as a deprecated alias (one-time stderr warning when used). The code snippets and example tests below were authored before this alignment and still use the legacy spelling for historical fidelity; the ship-state precedence is the one quoted in this paragraph. **Step 1: Write the failing test** @@ -269,8 +271,9 @@ Create `src/amplifier_agent_cli/provider_detect.py`: ```python """Provider auto-detection from environment variables. -Precedence (locked, design ยง3 + D5): - ANTHROPIC_API_KEY > OPENAI_API_KEY > AZURE_OPENAI_KEY > OLLAMA_HOST +Precedence (locked, design ยง3 + D5; Azure var renamed 2026-05-29 โ€” see Task 2 note): + ANTHROPIC_API_KEY > OPENAI_API_KEY > AZURE_OPENAI_API_KEY > OLLAMA_HOST + (legacy alias AZURE_OPENAI_KEY still accepted, deprecated) `--provider` override (CLI flag) bypasses detection. Unknown overrides raise ProviderNotConfigured. diff --git a/src/amplifier_agent_cli/admin/config_show.py b/src/amplifier_agent_cli/admin/config_show.py index c9a711ef..d06c18be 100644 --- a/src/amplifier_agent_cli/admin/config_show.py +++ b/src/amplifier_agent_cli/admin/config_show.py @@ -46,9 +46,13 @@ def _resolve_provider() -> dict[str, Any]: result (covers any future file-based config). On ProviderNotConfigured, return value=None, source='unset'. """ - for env_var, provider_name in _PROVIDER_ENV_ORDER: - if os.environ.get(env_var): - return {"value": provider_name, "source": f"env:{env_var}"} + for provider_name, env_vars in _PROVIDER_ENV_ORDER: + # env_vars[0] is the preferred name; remaining entries are deprecated + # aliases. Source annotation reports the actual env var that supplied + # the credential so operators can spot uses of the legacy spelling. + for env_var in env_vars: + if os.environ.get(env_var): + return {"value": provider_name, "source": f"env:{env_var}"} try: name = detect_provider(override=None) diff --git a/src/amplifier_agent_cli/provider_detect.py b/src/amplifier_agent_cli/provider_detect.py index 80c4fe88..67f02508 100644 --- a/src/amplifier_agent_cli/provider_detect.py +++ b/src/amplifier_agent_cli/provider_detect.py @@ -1,23 +1,50 @@ """Provider auto-detection from environment variables. -Precedence order: ANTHROPIC_API_KEY > OPENAI_API_KEY > AZURE_OPENAI_KEY > OLLAMA_HOST. +Precedence order: ANTHROPIC_API_KEY > OPENAI_API_KEY > AZURE_OPENAI_API_KEY > OLLAMA_HOST. The --provider CLI flag overrides env-var detection entirely. + +Azure note: the documented (and upstream-module-preferred) env var is +``AZURE_OPENAI_API_KEY``. For backwards compatibility with the CLI's earlier +``AZURE_OPENAI_KEY`` spelling, the legacy name is still accepted as a +deprecated alias and triggers a one-time stderr warning. This mirrors the +behavior of ``amplifier-module-provider-azure-openai`` itself, which checks +``AZURE_OPENAI_API_KEY`` first and falls back to ``AZURE_OPENAI_KEY``. """ from __future__ import annotations import os +import sys from typing import Final KNOWN_PROVIDERS: Final[tuple[str, ...]] = ("anthropic", "openai", "azure-openai", "ollama") -_DETECTION_ORDER: Final[tuple[tuple[str, str], ...]] = ( - ("ANTHROPIC_API_KEY", "anthropic"), - ("OPENAI_API_KEY", "openai"), - ("AZURE_OPENAI_KEY", "azure-openai"), - ("OLLAMA_HOST", "ollama"), +#: Detection slots, walked in precedence order. Each slot lists the preferred +#: env var first, then any legacy aliases. The first slot with a truthy value +#: (in any of its vars) wins. When a legacy alias supplies the value, a +#: one-time stderr deprecation notice is emitted. +_DETECTION_ORDER: Final[tuple[tuple[str, tuple[str, ...]], ...]] = ( + # (provider_name, (preferred_var, *legacy_aliases)) + ("anthropic", ("ANTHROPIC_API_KEY",)), + ("openai", ("OPENAI_API_KEY",)), + ("azure-openai", ("AZURE_OPENAI_API_KEY", "AZURE_OPENAI_KEY")), + ("ollama", ("OLLAMA_HOST",)), ) +_DEPRECATION_NOTICE_EMITTED: set[str] = set() + + +def _emit_deprecation_notice(legacy_var: str, preferred_var: str) -> None: + """Emit a one-time stderr warning when a legacy env var triggers detection.""" + if legacy_var in _DEPRECATION_NOTICE_EMITTED: + return + _DEPRECATION_NOTICE_EMITTED.add(legacy_var) + print( + f"[WARN] {legacy_var} is deprecated; please set {preferred_var} instead. " + f"Support for {legacy_var} will be removed in a future release.", + file=sys.stderr, + ) + class ProviderNotConfigured(Exception): """Raised when no provider can be determined from env vars or an explicit override.""" @@ -34,8 +61,9 @@ def detect_provider(override: str | None) -> str: If *override* is given, validate it against KNOWN_PROVIDERS and return it. Otherwise, walk _DETECTION_ORDER and return the first provider whose env - var is set to a truthy value. Raise ProviderNotConfigured if nothing is - configured. + var is set to a truthy value. If no preferred env var is set, fall back + to _LEGACY_DETECTION_ORDER (emitting a one-time deprecation notice on + stderr). Raise ProviderNotConfigured if nothing is configured. """ if override is not None: if override not in KNOWN_PROVIDERS: @@ -43,13 +71,18 @@ def detect_provider(override: str | None) -> str: raise ProviderNotConfigured(f"Unknown provider '{override}'. Known providers: {known}.") return override - for env_var, provider in _DETECTION_ORDER: - if os.environ.get(env_var): - return provider + for provider, env_vars in _DETECTION_ORDER: + preferred_var = env_vars[0] + for index, env_var in enumerate(env_vars): + if os.environ.get(env_var): + if index > 0: + # Legacy alias supplied the value โ€” warn the user. + _emit_deprecation_notice(env_var, preferred_var) + return provider raise ProviderNotConfigured( "No provider configured. Set one of the following environment variables: " - "ANTHROPIC_API_KEY, OPENAI_API_KEY, AZURE_OPENAI_KEY, OLLAMA_HOST. " + "ANTHROPIC_API_KEY, OPENAI_API_KEY, AZURE_OPENAI_API_KEY, OLLAMA_HOST. " "Alternatively, pass --provider on the command line. " "See the README for setup instructions." ) diff --git a/src/amplifier_agent_cli/provider_sources.py b/src/amplifier_agent_cli/provider_sources.py index c3f19252..875fadb4 100644 --- a/src/amplifier_agent_cli/provider_sources.py +++ b/src/amplifier_agent_cli/provider_sources.py @@ -30,6 +30,7 @@ from __future__ import annotations import os +import sys from typing import Any, Final, TypedDict @@ -39,9 +40,25 @@ class _CatalogEntry(TypedDict): module: str source: str env_var: str + legacy_env_vars: tuple[str, ...] default_model: str +_LEGACY_ENV_VAR_NOTICE_EMITTED: set[str] = set() + + +def _emit_legacy_env_var_notice(legacy_var: str, preferred_var: str) -> None: + """Emit a one-time stderr warning when a legacy env var supplies credentials.""" + if legacy_var in _LEGACY_ENV_VAR_NOTICE_EMITTED: + return + _LEGACY_ENV_VAR_NOTICE_EMITTED.add(legacy_var) + print( + f"[WARN] {legacy_var} is deprecated; please set {preferred_var} instead. " + f"Support for {legacy_var} will be removed in a future release.", + file=sys.stderr, + ) + + #: Map provider short-name (matches ``provider_detect.KNOWN_PROVIDERS``) โ†’ #: the catalog row used to construct a ``mount_plan["providers"]`` entry. #: @@ -54,12 +71,14 @@ class _CatalogEntry(TypedDict): "module": "provider-anthropic", "source": "git+https://github.com/microsoft/amplifier-module-provider-anthropic@main", "env_var": "ANTHROPIC_API_KEY", + "legacy_env_vars": (), "default_model": "claude-opus-4-5", }, "openai": { "module": "provider-openai", "source": "git+https://github.com/microsoft/amplifier-module-provider-openai@main", "env_var": "OPENAI_API_KEY", + "legacy_env_vars": (), # gpt-5.5 chosen so the bundle's default `extended_thinking: true` lands # on a model that actually accepts the resulting `reasoning.effort` # parameter. With gpt-4o (non-reasoning), the OpenAI API 400s on every @@ -69,13 +88,21 @@ class _CatalogEntry(TypedDict): "azure-openai": { "module": "provider-azure-openai", "source": "git+https://github.com/microsoft/amplifier-module-provider-azure-openai@main", - "env_var": "AZURE_OPENAI_KEY", + # Preferred env var โ€” matches the README, the upstream + # ``amplifier-module-provider-azure-openai`` module, and the Azure + # OpenAI Python SDK convention. + "env_var": "AZURE_OPENAI_API_KEY", + # Accepted for backwards compatibility with the CLI's earlier + # ``AZURE_OPENAI_KEY`` spelling. Triggers a one-time stderr + # deprecation notice when consulted. Removable in a future release. + "legacy_env_vars": ("AZURE_OPENAI_KEY",), "default_model": "gpt-4o", }, "ollama": { "module": "provider-ollama", "source": "git+https://github.com/microsoft/amplifier-module-provider-ollama@main", "env_var": "OLLAMA_HOST", + "legacy_env_vars": (), "default_model": "llama3.2", }, } @@ -115,11 +142,24 @@ def build_provider_entry(provider_name: str) -> dict[str, Any]: f"Unknown provider {provider_name!r}. Known providers: {known}.", ) + preferred_var = entry["env_var"] + api_key = os.environ.get(preferred_var, "") + if not api_key: + # Fall back to legacy env vars (e.g. AZURE_OPENAI_KEY) for backwards + # compat. Emits a one-time stderr warning when a legacy var supplies + # the credential so users have a chance to migrate. + for legacy_var in entry["legacy_env_vars"]: + legacy_value = os.environ.get(legacy_var, "") + if legacy_value: + _emit_legacy_env_var_notice(legacy_var, preferred_var) + api_key = legacy_value + break + return { "module": entry["module"], "source": entry["source"], "config": { - "api_key": os.environ.get(entry["env_var"], ""), + "api_key": api_key, "default_model": entry["default_model"], "priority": 1, }, diff --git a/tests/cli/test_config_show.py b/tests/cli/test_config_show.py index 05973f4c..1ee27aac 100644 --- a/tests/cli/test_config_show.py +++ b/tests/cli/test_config_show.py @@ -47,7 +47,8 @@ def test_config_show_reports_provider_from_env(runner: CliRunner, tmp_path: Path "ANTHROPIC_API_KEY": "sk-test", # Explicitly unset the other provider keys so detection order is clean. "OPENAI_API_KEY": "", - "AZURE_OPENAI_KEY": "", + "AZURE_OPENAI_API_KEY": "", + "AZURE_OPENAI_KEY": "", # legacy alias "OLLAMA_HOST": "", "XDG_CONFIG_HOME": str(tmp_path / "config"), "XDG_CACHE_HOME": str(tmp_path / "cache"), diff --git a/tests/cli/test_doctor.py b/tests/cli/test_doctor.py index b3bfa0b7..f9cfd308 100644 --- a/tests/cli/test_doctor.py +++ b/tests/cli/test_doctor.py @@ -22,7 +22,13 @@ # Provider env var constants # --------------------------------------------------------------------------- -_PROVIDER_ENV_VARS = ("ANTHROPIC_API_KEY", "OPENAI_API_KEY", "AZURE_OPENAI_KEY", "OLLAMA_HOST") +_PROVIDER_ENV_VARS = ( + "ANTHROPIC_API_KEY", + "OPENAI_API_KEY", + "AZURE_OPENAI_API_KEY", + "AZURE_OPENAI_KEY", # legacy alias, still accepted + "OLLAMA_HOST", +) # --------------------------------------------------------------------------- diff --git a/tests/cli/test_provider_detect.py b/tests/cli/test_provider_detect.py index d42605a0..1359544a 100644 --- a/tests/cli/test_provider_detect.py +++ b/tests/cli/test_provider_detect.py @@ -10,12 +10,18 @@ # Helper # --------------------------------------------------------------------------- -_PROVIDER_ENV_VARS = ("ANTHROPIC_API_KEY", "OPENAI_API_KEY", "AZURE_OPENAI_KEY", "OLLAMA_HOST") +_PROVIDER_ENV_VARS = ( + "ANTHROPIC_API_KEY", + "OPENAI_API_KEY", + "AZURE_OPENAI_API_KEY", + "AZURE_OPENAI_KEY", # legacy alias, still accepted + "OLLAMA_HOST", +) @pytest.fixture(autouse=True) def _clear_provider_env(monkeypatch: pytest.MonkeyPatch) -> None: - """Remove all four provider env vars before every test.""" + """Remove every provider env var (including legacy aliases) before each test.""" for var in _PROVIDER_ENV_VARS: monkeypatch.delenv(var, raising=False) @@ -36,7 +42,31 @@ def test_detects_openai_when_only_openai_set(monkeypatch: pytest.MonkeyPatch) -> def test_detects_azure_when_only_azure_set(monkeypatch: pytest.MonkeyPatch) -> None: - monkeypatch.setenv("AZURE_OPENAI_KEY", "az-key-test") + monkeypatch.setenv("AZURE_OPENAI_API_KEY", "az-key-test") + assert detect_provider(None) == "azure-openai" + + +def test_detects_azure_via_legacy_env_var( + monkeypatch: pytest.MonkeyPatch, + capsys: pytest.CaptureFixture[str], +) -> None: + """Legacy AZURE_OPENAI_KEY still detects azure-openai (with deprecation warning).""" + # Reset the per-process one-shot tracker so this test always sees the warning. + from amplifier_agent_cli import provider_detect + + provider_detect._DEPRECATION_NOTICE_EMITTED.clear() + monkeypatch.setenv("AZURE_OPENAI_KEY", "az-key-legacy") + assert detect_provider(None) == "azure-openai" + captured = capsys.readouterr() + assert "AZURE_OPENAI_KEY" in captured.err + assert "deprecated" in captured.err.lower() + assert "AZURE_OPENAI_API_KEY" in captured.err + + +def test_preferred_azure_wins_over_legacy(monkeypatch: pytest.MonkeyPatch) -> None: + """If both AZURE_OPENAI_API_KEY and AZURE_OPENAI_KEY are set, the preferred one wins.""" + monkeypatch.setenv("AZURE_OPENAI_API_KEY", "preferred") + monkeypatch.setenv("AZURE_OPENAI_KEY", "legacy") assert detect_provider(None) == "azure-openai" diff --git a/tests/cli/test_provider_sources.py b/tests/cli/test_provider_sources.py index 048060d1..14cae560 100644 --- a/tests/cli/test_provider_sources.py +++ b/tests/cli/test_provider_sources.py @@ -31,15 +31,21 @@ def test_catalog_lists_all_four_detection_names() -> None: def test_catalog_entry_shape() -> None: - """Each catalog entry has module, source, env_var, default_model โ€” all str.""" + """Each catalog entry has module, source, env_var, legacy_env_vars, default_model.""" from amplifier_agent_cli.provider_sources import PROVIDER_CATALOG - required = {"module", "source", "env_var", "default_model"} + str_fields = {"module", "source", "env_var", "default_model"} + required = str_fields | {"legacy_env_vars"} for name, entry in PROVIDER_CATALOG.items(): missing = required - set(entry.keys()) assert not missing, f"provider {name!r} missing fields {missing}" - for key in required: + for key in str_fields: assert isinstance(entry[key], str), f"provider {name!r} field {key!r} must be str" + # legacy_env_vars is a tuple of strings (possibly empty) + legacy = entry["legacy_env_vars"] + assert isinstance(legacy, tuple), f"provider {name!r} legacy_env_vars must be a tuple" + for v in legacy: + assert isinstance(v, str), f"provider {name!r} legacy_env_vars entry {v!r} must be str" # module name should match the canonical "provider-" convention assert entry["module"].startswith("provider-") # source should be a git URI (everything mounts via amplifier-module-provider-X repos) @@ -52,7 +58,10 @@ def test_catalog_anthropic_uses_anthropic_api_key() -> None: assert PROVIDER_CATALOG["anthropic"]["env_var"] == "ANTHROPIC_API_KEY" assert PROVIDER_CATALOG["openai"]["env_var"] == "OPENAI_API_KEY" - assert PROVIDER_CATALOG["azure-openai"]["env_var"] == "AZURE_OPENAI_KEY" + # Azure uses the documented + upstream-preferred AZURE_OPENAI_API_KEY, + # with AZURE_OPENAI_KEY accepted as a deprecated legacy alias. + assert PROVIDER_CATALOG["azure-openai"]["env_var"] == "AZURE_OPENAI_API_KEY" + assert PROVIDER_CATALOG["azure-openai"]["legacy_env_vars"] == ("AZURE_OPENAI_KEY",) assert PROVIDER_CATALOG["ollama"]["env_var"] == "OLLAMA_HOST" diff --git a/tests/cli/test_single_turn.py b/tests/cli/test_single_turn.py index 3872355e..ec15ec8a 100644 --- a/tests/cli/test_single_turn.py +++ b/tests/cli/test_single_turn.py @@ -34,7 +34,13 @@ # Provider env var constants # --------------------------------------------------------------------------- -_PROVIDER_ENV_VARS = ("ANTHROPIC_API_KEY", "OPENAI_API_KEY", "AZURE_OPENAI_KEY", "OLLAMA_HOST") +_PROVIDER_ENV_VARS = ( + "ANTHROPIC_API_KEY", + "OPENAI_API_KEY", + "AZURE_OPENAI_API_KEY", + "AZURE_OPENAI_KEY", # legacy alias, still accepted + "OLLAMA_HOST", +) # ---------------------------------------------------------------------------