Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 14 additions & 3 deletions core/agent_harness/turns/action_driver.py
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
)
from core.llm.failure_classification import is_context_length_overflow
from core.llm.types import AgentLLMResponse, ToolCall
from core.llm_invoke_errors import remediate_missing_llm_credentials
from core.tool_framework.tags import SUMMARIZE_OBSERVATION_TAG
from platform.analytics.react_turn import run_react_agent_with_telemetry
from platform.observability.trace.prompts import persist_turn_system_prompt
Expand Down Expand Up @@ -1053,11 +1054,21 @@ def _run_action_turn(
client=llm_client,
error_text=error_text,
)
_render_tool_calling_error(args.output, error_text)
_persist_tool_calling_error(session, message, error_text)
from config.config import get_configured_llm_provider
from core.agent_harness.accounting.token_accounting import resolve_provider_name

provider = resolve_provider_name(llm_client) if llm_client is not None else None
display_text = (
remediate_missing_llm_credentials(
error_text, provider=provider or get_configured_llm_provider()
)
or error_text
)
_render_tool_calling_error(args.output, display_text)
_persist_tool_calling_error(session, message, display_text)
session.record("cli_agent", message, ok=False)
return ToolCallingTurnResult(
0, 0, 0, True, True, response_text=error_text, accounting_status="not_run"
0, 0, 0, True, True, response_text=display_text, accounting_status="not_run"
)

counts = _count_turn(result, session, history_start)
Expand Down
10 changes: 8 additions & 2 deletions core/agent_harness/turns/orchestrator.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
routing_input_from_result,
)
from core.agent_harness.turns.turn_snapshot import TurnSnapshot
from core.llm_invoke_errors import is_cli_timeout_error
from core.llm_invoke_errors import is_cli_timeout_error, remediate_missing_llm_credentials
from platform.harness_ports import preferred_evidence_sources_for
from platform.observability.trace.spans import component_span, emit_route

Expand Down Expand Up @@ -169,7 +169,13 @@ def _stream_response(
kind = "llm_timeout" if is_cli_timeout_error(exc) else "assistant_error"
stage_turn_error(session, kind, str(exc))
stage_turn_llm_failure(session, client=client)
output.render_error(f"assistant failed: {exc}")
from config.config import get_configured_llm_provider
from core.agent_harness.accounting.token_accounting import resolve_provider_name

remediation = remediate_missing_llm_credentials(
str(exc), provider=resolve_provider_name(client) or get_configured_llm_provider()
)
output.render_error(remediation or f"assistant failed: {exc}")
return None
return run_factory.build(
client=client,
Expand Down
42 changes: 42 additions & 0 deletions core/llm_invoke_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,28 @@ def is_cli_timeout_error(exc: BaseException) -> bool:
"billing is not enabled",
)
_QUOTA_PATTERNS = ("429", "quota", "rate limit", "too many requests", "credit")
# Provider-SDK phrasings for "no API key at all" (as opposed to an invalid one).
# Absence phrasings only — never a bare env-var-name match: rejected-key errors
# also cite *_API_KEY names and must keep their real authentication message.
_MISSING_KEY_PATTERNS = (
"missing credentials",
"api key is not set",
"missing api key",
"no api key",
"could not resolve authentication method", # anthropic SDK, key/token both unset
"to be set", # opensre wrapper: "requires ANTHROPIC_API_KEY to be set"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Broad phrase misclassifies configuration errors

When an Azure or custom provider has an API key but lacks its required base URL, the resulting requires ... BASE_URL to be set error matches this generic phrase and is replaced with API-key login guidance. The suggested auth login command cannot supply the missing base URL, so the shell hides the actionable configuration failure and remains unusable.

)
# A message matching any of these describes a key that exists but was rejected.
_REJECTED_KEY_VETO_PATTERNS = (
"invalid",
"incorrect",
"expired",
"revoked",
"401",
"403",
"unauthorized",
"forbidden",
)
_AUTH_PATTERNS = (
"authentication",
"unauthorized",
Expand All @@ -120,6 +142,26 @@ def is_cli_timeout_error(exc: BaseException) -> bool:
)


def remediate_missing_llm_credentials(message: str, *, provider: str | None = None) -> str | None:
"""Actionable replacement text when an LLM call failed for lack of any API key.

Returns ``None`` for every other failure (invalid key, quota, timeout, …)
so callers fall back to their existing rendering.
"""
text = message.lower()
if any(pattern in text for pattern in _REJECTED_KEY_VETO_PATTERNS):
return None
if not any(pattern in text for pattern in _MISSING_KEY_PATTERNS):
return None
target = provider.strip() if provider else "<provider>"
subject = f"No API key is set for {target}" if provider else "No LLM API key is set"
return (
f"{subject}. Run `/auth login {target}` to add one, or `/onboard` to rerun "
f"setup (from a terminal: `opensre auth login {target}`). "
f"(Provider detail: {message.strip()})"
)


def classify_provider_error_kind(message: str) -> str:
"""Bucket an LLM provider failure message for analytics filtering.

Expand Down
26 changes: 26 additions & 0 deletions surfaces/cli/commands/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
)
from surfaces.cli.llm_auth.service import (
AuthSetupError,
cli_subscription_install_error,
configure_api_key_provider,
configure_cli_subscription_provider,
logout_provider,
Expand All @@ -50,6 +51,17 @@ def _provider_choice(profile: ProviderAuthProfile) -> questionary.Choice:
return questionary.Choice(f"{profile.name} ({profile.label})", value=profile.name)


def _configured_profile_name() -> str | None:
"""The auth-profile name matching the install's active LLM provider, if any."""
from config.config import get_configured_llm_provider

configured = get_configured_llm_provider()
for profile in iter_auth_profiles():
if configured == profile.provider_value or configured in profile.all_names:
return profile.name
return None


def _prompt_provider() -> ProviderAuthProfile:
choices: list[questionary.Choice | questionary.Separator] = [
questionary.Separator(" "),
Expand All @@ -66,9 +78,19 @@ def _prompt_provider() -> ProviderAuthProfile:
_provider_choice(profile) for profile in iter_auth_profiles() if profile.kind == "api_key"
)

configured = _configured_profile_name()
default = next(
(
choice
for choice in choices
if isinstance(choice, questionary.Choice) and choice.value == configured
),
None,
)
provider = questionary.select(
"Choose a provider:",
choices=choices,
default=default,
qmark=QUESTIONARY_QMARK,
style=questionary_prompt_style(),
instruction="(use arrow keys)",
Expand Down Expand Up @@ -184,6 +206,10 @@ def auth_login(
validate=validate,
)
else:
install_error = cli_subscription_install_error(profile)
if install_error:
# Fail before the browser question — it cannot fix a missing binary.
raise AuthSetupError(install_error)
_maybe_open_setup_page(profile, enabled=open_browser)
result = configure_cli_subscription_provider(
profile=profile,
Expand Down
16 changes: 16 additions & 0 deletions surfaces/cli/llm_auth/service.py
Original file line number Diff line number Diff line change
Expand Up @@ -155,6 +155,22 @@ def _run_vendor_login(profile: ProviderAuthProfile, binary_path: str) -> None:
raise AuthSetupError(f"{profile.label} login exited with code {result.returncode}.")


def cli_subscription_install_error(profile: ProviderAuthProfile) -> str | None:
"""Install guidance when the profile's backing CLI is absent, else ``None``.

Lets the login command fail before any interactive prompt instead of after
the browser question.
"""
provider = provider_for_profile(profile)
if provider.credential_kind != WizardCredentialKind.CLI or provider.adapter_factory is None:
return None
adapter = provider.adapter_factory()
probe = adapter.detect()
if probe.installed:
return None
return f"{probe.detail} Install: {adapter.install_hint}"


def configure_cli_subscription_provider(
*,
profile: ProviderAuthProfile,
Expand Down
47 changes: 47 additions & 0 deletions tests/cli/test_auth_command.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,9 @@ def _fake_configure(**kwargs):
monkeypatch.setattr(
"surfaces.cli.commands.auth.configure_cli_subscription_provider", _fake_configure
)
monkeypatch.setattr(
"surfaces.cli.commands.auth.cli_subscription_install_error", lambda _profile: None
)

result = CliRunner().invoke(
cli,
Expand All @@ -140,6 +143,50 @@ def _fake_configure(**kwargs):
assert "Provider : codex" in result.output


def test_auth_login_missing_cli_fails_before_any_prompt(monkeypatch, tmp_path: Path) -> None:
"""A missing vendor binary must fail immediately, not after the browser question."""
# Arrange
_patch_auth_env(monkeypatch, tmp_path)
install_hint = "Codex CLI not found on PATH. Install: npm i -g @openai/codex"
prompts: list[str] = []

def _record_setup_page(profile, *, enabled):
prompts.append(profile.name)

def _must_not_configure(**_kwargs):
raise AssertionError("configure must not run when the CLI is missing")

monkeypatch.setattr(
"surfaces.cli.commands.auth.cli_subscription_install_error",
lambda _profile: install_hint,
)
monkeypatch.setattr("surfaces.cli.commands.auth._maybe_open_setup_page", _record_setup_page)
monkeypatch.setattr(
"surfaces.cli.commands.auth.configure_cli_subscription_provider", _must_not_configure
)

# Act
result = CliRunner().invoke(cli, ["auth", "login", "chatgpt"])

# Assert: guidance shown, no setup-page prompt, no configure attempt.
assert result.exit_code != 0
assert install_hint in result.output
assert prompts == []


def test_provider_chooser_defaults_to_the_configured_provider(monkeypatch) -> None:
"""Bare `/auth login` must preselect the install's provider, not the first row."""
import config.config as config_module
from surfaces.cli.commands.auth import _configured_profile_name

monkeypatch.setattr(config_module, "get_configured_llm_provider", lambda: "openai")
assert _configured_profile_name() == "openai"

# An OAuth install stores the backend provider; it maps to its profile name.
monkeypatch.setattr(config_module, "get_configured_llm_provider", lambda: "codex")
assert _configured_profile_name() == "chatgpt"


def test_auth_logout_deepseek_removes_keyring_secret(monkeypatch, tmp_path: Path) -> None:
_patch_auth_env(monkeypatch, tmp_path)
previous_backend = keyring.get_keyring()
Expand Down
71 changes: 71 additions & 0 deletions tests/core/agent/orchestration/test_agent_actions_harness.py
Original file line number Diff line number Diff line change
Expand Up @@ -1889,6 +1889,77 @@ def invoke(self, *_args: Any, **_kwargs: Any) -> Any:
assert staged.provider == "bedrock"


def test_missing_key_invoke_failure_answers_with_login_guidance() -> None:
"""A key-less turn must reply with the auth-login command, not raw SDK text."""
# Arrange: the exact OpenAI SDK message a deferred-key install surfaces.
error = (
"Missing credentials. Please pass an `api_key`, `workload_identity`, "
"`admin_api_key`, or set the `OPENAI_API_KEY` or `OPENAI_ADMIN_KEY` "
"environment variable."
)

class _MissingKeyLLM:
_model = "gpt-5.4-mini"
_provider_label = "OpenAI"

def tool_schemas(self, _tools: list[Any]) -> list[dict[str, Any]]:
return []

def invoke(self, *_args: Any, **_kwargs: Any) -> Any:
raise RuntimeError(error)

session = Session()

# Act
result = run_action_tool_turn(
"good morning",
session,
Console(force_terminal=False),
deps=ToolCallingDeps(llm_factory=_MissingKeyLLM),
)

# Assert: the user sees the remediation; telemetry keeps the raw error.
assert "`/auth login openai`" in result.response_text
assert "No API key is set for openai" in result.response_text
assert session.terminal.pop_pending_turn_error() == ("action_agent_error", error)


def test_missing_key_guidance_names_configured_provider_when_client_has_no_label(
monkeypatch,
) -> None:
"""A client with no provider label falls back to the configured provider name."""
# Arrange: same SDK message, but the failing client exposes no identity.
error = (
"Missing credentials. Please pass an `api_key`, `workload_identity`, "
"`admin_api_key`, or set the `OPENAI_API_KEY` or `OPENAI_ADMIN_KEY` "
"environment variable."
)

class _AnonymousFailingLLM:
def tool_schemas(self, _tools: list[Any]) -> list[dict[str, Any]]:
return []

def invoke(self, *_args: Any, **_kwargs: Any) -> Any:
raise RuntimeError(error)

import config.config as config_module

monkeypatch.setattr(config_module, "get_configured_llm_provider", lambda: "openai")
session = Session()

# Act
result = run_action_tool_turn(
"good morning",
session,
Console(force_terminal=False),
deps=ToolCallingDeps(llm_factory=_AnonymousFailingLLM),
)

# Assert
assert "`/auth login openai`" in result.response_text
assert "<provider>" not in result.response_text


def test_llm_invoke_failure_uses_built_client_without_second_factory_call() -> None:
"""Invoke-time failure must stage identity from the client that actually failed."""
error = "Bedrock model unavailable"
Expand Down
Loading