Skip to content

feat(cli): add a doctor command to preflight the deployment - #4321

Merged
IsmaelMartinez merged 3 commits into
The-PR-Agent:mainfrom
Yoel-qin:feat/cli-doctor-command
Oct 24, 2026
Merged

IsmaelMartinez merged 3 commits into
The-PR-Agent:mainfrom
Yoel-qin:feat/cli-doctor-command

Conversation

@Yoel-qin

@Yoel-qin Yoel-qin commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

User description

Problem

A misconfigured deployment only fails at the end of a full run. In the sample from #4195, 20 of 81 owners running the action publicly failed with Incorrect API key provided: dummy_key: the runner warns OPENAI_KEY not set and carries on, and every tool call then fails against the placeholder. Wrong base URLs and quota errors surface the same way, one full run later (#4320).

The existing tests.health_test.main needs a repository checkout (it is not shipped), a GitHub token, and real PRs — there is nothing lightweight to gate a setup with.

Solution

pr-agent doctor — runs without --pr_url or a diff, never touches a git provider:

  • config.model is set
  • the active provider has a real key (env or settings — _captured_api_key, the same source a run uses; the value is never printed; providers unknown to the env table, i.e. keyless auth, defer to the ping)
  • a one-token completion against the configured model succeeds, through the real LiteLLMAIHandler.chat_completion path

One line per check on stdout; exit code 0 only when everything passes, so CI setups can gate on it. No new settings; no existing command changes (doctor joins only the CLI choices, not the agent dispatch).

Testing

  • Passing setup → all three checks pass, exit 0; missing key names the env variable, placeholder key fails, bad ping reports the failure reason — each exits 1.
  • doctor never constructs PRAgent (asserted), and the key value never appears in the output.
  • CLI regression suite (201 tests) and ruff pass.

Fixes #4320


PR Type

Enhancement, Tests, Documentation


Description

  • Add standalone pr-agent doctor deployment checks.

  • Validate model configuration, credentials, and endpoint connectivity.

  • Report check results with CI-friendly exit codes.

  • Document usage and test diagnostic behavior.


Diagram Walkthrough

flowchart LR
  cli["pr-agent doctor"]
  scope["CLI settings scope"]
  checks["Check model, credentials, and completion"]
  result["Print results and return exit code"]
  cli --> scope
  scope --> checks
  checks --> result
Loading

File Walkthrough

Relevant files
Enhancement
cli.py
Integrate standalone doctor command into CLI                         

pr_agent/cli.py

  • Add doctor to CLI command choices and help text.
  • Run diagnostics within the CLI settings scope before PR or diff
    processing.
  • Return diagnostic status without constructing PRAgent.
+10/-1   
doctor.py
Add deployment configuration and connectivity diagnostics

pr_agent/doctor.py

  • Resolve the active provider from the model and custom OpenAI base URL.
  • Check model configuration and captured credentials, rejecting
    placeholder keys.
  • Probe model connectivity through LiteLLMAIHandler.chat_completion and
    report failures.
  • Print per-check results and return success only when all checks pass.
+82/-0   
Tests
test_cli_doctor.py
Test doctor diagnostics and standalone execution                 

tests/unittest/test_cli_doctor.py

  • Cover successful diagnostics, missing credentials, placeholder keys,
    and completion failures.
  • Verify operation without PR context or PRAgent construction.
  • Check that credential values stay out of normal output.
  • Test provider resolution with a custom OpenAI base URL.
+109/-0 
Documentation
automations_and_usage.md
Document doctor usage and health-check differences             

docs/docs/usage-guide/automations_and_usage.md

  • Introduce pr-agent doctor under CLI health checks.
  • Describe diagnostic checks and CI-friendly exit status.
  • Distinguish packaged preflight checks from repository-based end-to-end
    health checks.
+9/-1     


⚙️ Agent run details
  • Model: openai/gpt-6.1-sol
  • Tokens: 8,480 in / 601 out / 9,081 total
  • Time cost: 15.6s
  • AI calls: 1

A misconfigured deployment only fails at the end of a full run: the
runner warns about a missing key, carries on, and every tool call then
fails against the placeholder (The-PR-Agent#4320, the dummy_key sample in The-PR-Agent#4195).

`pr-agent doctor` runs without a PR or diff and never touches a git
provider. It checks that config.model is set, that the active provider
has a real key (env or settings, never printed), and that a one-token
completion against the configured model succeeds, printing one line per
check. The exit code is 0 only when every check passes, so CI setups
can gate on it.
@the-pr-agent-bot

Copy link
Copy Markdown

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis ❌

4195 - Not compliant

Non-compliant requirements:

  • Add the exact error to the GitHub Action troubleshooting list.
  • Explain unavailable PR-event secrets and mitigation options.
  • Explain missing or misnamed secrets.
  • Explain provider credentials that do not match the default model.
  • Optionally recognize alternative OpenAI key variables in the runner warning.

4320 - Partially compliant

Compliant requirements:

  • Add a standalone command requiring no PR URL and accessing no git provider.
  • Report check results on stdout.
  • Reject placeholder credentials.
  • Report model endpoint failure reasons.
  • Return zero only when all implemented checks pass.
  • Introduce no settings and preserve existing command dispatch.

Non-compliant requirements:

  • Reliably identify the active provider and its missing credential variable.
  • Limit the model probe to a one-token completion.
  • Pass for working configurations using unprefixed non-OpenAI models.

Requires further human verification:

  • Verify successful completion and process exit status with real provider credentials.
⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪
🧪 PR contains tests
🔒 No security concerns identified
⚡ Recommended focus areas for review

Provider detection

An unprefixed model is not necessarily OpenAI: LiteLLM also recognizes models such as claude-3-5-sonnet-20241022 as Anthropic. With a valid Anthropic key and no OpenAI key, the completion can succeed while this check incorrectly requires OpenAI credentials and makes doctor exit nonzero. Resolve the provider using the same routing logic as the real completion rather than assuming every unprefixed model is OpenAI.

def _active_provider(model: str) -> str:
    """Resolve the litellm provider prefix of a model id, as a real call would."""
    if "/" in model:
        return model.split("/", 1)[0].strip().lower()
    # No prefix means litellm's OpenAI default; a custom base URL keeps that routing.
    return "custom_openai" if get_settings().get("OPENAI.API_BASE", None) else "openai"
Unbounded probe

The completion call supplies no output-token limit; asking for ok in the prompt does not enforce the promised one-token probe. In particular, reasoning models can consume substantial reasoning tokens before producing that short answer, making a CI health check unexpectedly slow and costly. Apply an explicit token budget through the completion path, accounting for the configured model's supported parameters.

async def _check_model_ping(handler: LiteLLMAIHandler, model: str) -> tuple[bool, str]:
    try:
        await handler.chat_completion(model, _PING_SYSTEM, _PING_USER)

⚙️ Agent run details
  • Model: openai/gpt-6.1-sol
  • Tokens: 9,876 in / 1,086 out / 10,962 total
  • Time cost: 22.3s
  • AI calls: 1

@the-pr-agent-bot

Copy link
Copy Markdown

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
Security
Prevent credential leaks from exceptions

Provider exceptions can contain credentials in request URLs, headers, or echoed
response bodies; truncating str(error) does not redact them, and the debug log emits
the full exception. Report only the exception type unless the diagnostic text has
passed through a credential-aware sanitizer, and add a failure-path test containing
a synthetic secret.

pr_agent/doctor.py [57-60]

     except Exception as error:
-        reason = str(error).split("\n")[0][:200]
-        get_logger().debug(f"Doctor model ping failed: {error!r}")
+        reason = type(error).__name__
+        get_logger().debug(f"Doctor model ping failed ({reason})")
         return False, f"model completion failed: {reason}"
Suggestion importance[1-10]: 8

__

Why: Reporting only the exception type prevents potentially sensitive exception text from reaching terminal output and debug logs. The existing diagnostic assertion in test_bad_ping_reports_the_reason would also need updating, and the proposed secret-bearing failure test remains to be added.

Medium
Possible bug
Resolve unprefixed model providers correctly

LiteLLM also recognizes unprefixed models from non-OpenAI providers, such as
claude-*, so defaulting every unprefixed model to openai can reject a working
deployment for lacking an unrelated key. Use LiteLLM's provider resolver for those
models, and let the ping report invalid model identifiers without aborting the
checks.

pr_agent/doctor.py [22-27]

 def _active_provider(model: str) -> str:
     """Resolve the litellm provider prefix of a model id, as a real call would."""
+    from litellm import get_llm_provider
+
     if "/" in model:
         return model.split("/", 1)[0].strip().lower()
-    # No prefix means litellm's OpenAI default; a custom base URL keeps that routing.
-    return "custom_openai" if get_settings().get("OPENAI.API_BASE", None) else "openai"
+    if get_settings().get("OPENAI.API_BASE", None):
+        return "custom_openai"
+    try:
+        return get_llm_provider(model=model)[1]
+    except Exception:
+        return "unknown"
Suggestion importance[1-10]: 7

__

Why: Using LiteLLM's provider resolver avoids incorrectly checking OpenAI credentials for recognized unprefixed models from other providers. The fallback also allows invalid identifiers to reach the existing ping diagnostic instead of aborting provider resolution.

Medium

⚙️ Agent run details
  • Model: openai/gpt-6.1-sol
  • Tokens: 13,021 in / 1,474 out / 14,495 total
  • Time cost: 38.6s
  • AI calls: 2

@Yoel-qin
Yoel-qin force-pushed the feat/cli-doctor-command branch from fbe80b5 to 47b3c46 Compare October 24, 2026 04:20
Two /improve findings: ping failures can quote credentials (request
URLs, echoed bodies) into stdout and debug logs - route the reason and
the debug repr through redact_credentials instead, keeping the failure
text diagnosable; and unprefixed models are not always OpenAI's -
litellm resolves claude-*, gemini-* itself, so ask its resolver for
the provider and defer unresolvable ids to the ping.
@Yoel-qin
Yoel-qin force-pushed the feat/cli-doctor-command branch from 47b3c46 to 0971919 Compare October 24, 2026 04:26
@Yoel-qin

Copy link
Copy Markdown
Contributor Author

Both adopted in 0971919, with one substitution: the failure reason goes through the repo's redact_credentials() rather than collapsing to the exception type — the issue's acceptance criteria promise a diagnosable reason for a bad base URL, and redaction keeps that without leaking anything quoted by the provider (covered by a synthetic-secret test). The provider resolver suggestion is in verbatim; unresolvable ids now defer to the ping.

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for turning #4320 into a working command so quickly, with the key never printed and the error text redacted. One fix before merge: with openai.api_base set, as with any OpenAI-compatible gateway, the handler keeps the key under openai, but doctor looks under custom_openai, so a working gateway fails the key check and exits 1. Inline below; with both suggestions the doctor tests and the full suite pass.

The bot's unbounded-probe finding is fine for a command run by hand. This won't reach the #4195 runs on its own, since those were Action runs with no key; would you be up for a follow-up where the Action runner stops on a failed key check instead of carrying on with the placeholder?

Comment thread pr_agent/doctor.py Outdated
Comment thread tests/unittest/test_cli_doctor.py Outdated
With openai.api_base set, LiteLLMHandler captures the key under
'openai', but doctor resolved the provider to 'custom_openai' and the
key check missed it - a working OpenAI-compatible gateway failed and
exited 1. Drop the branch: litellm's resolver already returns 'openai'
for gpt-* ids, and unknown ids defer to the ping.
@Yoel-qin

Copy link
Copy Markdown
Contributor Author

Fixed in 6583f54 — branch deleted, so a gateway setup resolves like the handler sees it. And yes to the follow-up: the runner stopping on a failed key check instead of carrying on with the placeholder is the half of #4195 doctor can't reach. I'll open the issue once this lands.

@IsmaelMartinez IsmaelMartinez left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks for the quick turnaround and for taking on the runner follow-up; approving and merging.

@IsmaelMartinez
IsmaelMartinez merged commit 3d97336 into The-PR-Agent:main Oct 24, 2026
12 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add a pr-agent doctor command to catch broken configuration before a run

2 participants