Skip to content

fix(shell): answer key-less LLM turns with auth-login guidance instead of raw provider errors - #5000

Merged
YauhenBichel merged 3 commits into
mainfrom
fix/add-missing-keys-for-deferred-account
Aug 14, 2026
Merged

fix(shell): answer key-less LLM turns with auth-login guidance instead of raw provider errors#5000
YauhenBichel merged 3 commits into
mainfrom
fix/add-missing-keys-for-deferred-account

Conversation

@YauhenBichel

@YauhenBichel YauhenBichel commented Aug 14, 2026

Copy link
Copy Markdown
Collaborator

Follow-up to #4994. Deferring the LLM key during onboarding is now reachable, but the first shell turn on a key-less install answered with the provider SDK's raw exception text ("Missing credentials. Please pass an api_key, workload_identity, admin_api_key, or set the OPENAI_API_KEY …") — no hint of what to actually do.

  • core/llm_invoke_errors.py — new remediate_missing_llm_credentials(): recognizes the "no key at all" provider phrasings and returns actionable text naming the exact command (opensre auth login <provider>, or opensre onboard to rerun setup), with the provider detail kept in parentheses. Every other failure kind (invalid key, quota, timeout) returns None so existing rendering is unchanged.
  • core/agent_harness/turns/action_driver.py — the action-agent failure path renders and returns the remediation as the turn's response; telemetry still stages the raw error so analytics classification is unaffected.
  • core/agent_harness/turns/orchestrator.py — same treatment on the answer-stream path; the assistant failed: … fallback stays for other errors.
  • Tests: unit coverage of the helper against the exact SDK message (plus invalid-key / quota / timeout negatives), and an action-path regression asserting the reply carries the login command while staged telemetry keeps the raw text.

Demo

image

@github-actions

Copy link
Copy Markdown
Contributor

Greptile code review

This repo uses Greptile for automated review. Before merge, aim for Confidence Score: 5/5 with zero unresolved review threads — see CONTRIBUTING.md.

Run a review — add a PR comment with:

@greptile review

Give it ~5-10 minutes (sometimes longer) for results, then fix feedback and re-trigger until you reach Confidence Score: 5/5.

Optional: automate with the greploop skill.

@YauhenBichel

Copy link
Copy Markdown
Collaborator Author

@greptile review

@greptile-apps

greptile-apps Bot commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This follow-up narrows missing-credential classification and replaces raw key-less LLM errors with actionable authentication guidance while preserving raw telemetry.

  • Adds remediation rendering to action-agent and streamed-answer failure paths.
  • Adds configured-provider defaults and missing-vendor-CLI preflight behavior to auth login.
  • Extends regression coverage for credential classification, telemetry, and CLI authentication behavior.

Confidence Score: 4/5

The PR is not yet safe to merge because reachable missing-base-URL failures are replaced with incorrect API-key login guidance.

Provider validation errors such as a missing Azure or custom-provider base URL reach the changed action failure handler and match the generic "to be set" credential pattern, directing users toward an authentication command that cannot repair the actual configuration.

Files Needing Attention: core/llm_invoke_errors.py, core/agent_harness/turns/action_driver.py

Important Files Changed

Filename Overview
core/llm_invoke_errors.py Adds missing-credential remediation, but the generic "to be set" phrase incorrectly captures reachable missing-base-URL configuration failures.
core/agent_harness/turns/action_driver.py Applies remediation to action-turn failures while retaining raw telemetry, but exposes the helper's configuration-error misclassification.
core/agent_harness/turns/orchestrator.py Applies the same remediation to streamed-answer failures while preserving existing fallback rendering.
surfaces/cli/commands/auth.py Defaults provider selection to the configured profile and performs CLI-install checks before browser prompting.
surfaces/cli/llm_auth/service.py Adds structured missing-vendor-CLI installation guidance for subscription profiles.
tests/core/runtime/test_llm_invoke_errors.py Covers expected missing-key and rejected-key messages but omits non-key configuration errors containing the broad "to be set" phrase.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Shell turn] --> B[Build or invoke LLM client]
  B -->|Success| C[Normal agent response]
  B -->|Exception| D[Stage raw failure telemetry]
  D --> E{Missing API key?}
  E -->|Yes| F[Render auth login and onboard guidance]
  E -->|No| G[Render existing provider error]
Loading

Reviews (3): Last reviewed commit: "added missing phrasing for keys" | Re-trigger Greptile

Comment thread core/llm_invoke_errors.py Outdated
@YauhenBichel

Copy link
Copy Markdown
Collaborator Author

@greptile review

@YauhenBichel
YauhenBichel marked this pull request as ready for review August 14, 2026 08:54
@YauhenBichel
YauhenBichel merged commit da4befb into main Aug 14, 2026
31 checks passed
@YauhenBichel
YauhenBichel deleted the fix/add-missing-keys-for-deferred-account branch August 14, 2026 08:54
@github-actions

Copy link
Copy Markdown
Contributor

🧑‍💻 @YauhenBichel has entered the contributor hall of fame. Merged. Done. Shipped. Go touch grass (then come back with another PR). 🌱


👋 Join us on Discord - OpenSRE : hang out, contribute, or hunt for features and issues. Everyone's welcome.

Comment thread core/llm_invoke_errors.py
"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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant