[Yoel-qin]

Problem

With no usable key for the active model's provider, the Action runner printed OPENAI_KEY not set and carried on, so every tool call failed against the placeholder with Incorrect API key provided: dummy_key — one full run later (#4195: 20 of 81 sampled owners). doctor (#4321) preflights this by hand, but unattended Action runs got no gate — the follow-up agreed in #4321 (#4324).

Solution

• doctor exposes its key resolution as a shared missing_provider_key_env_vars(): for the active provider of config.model, it returns the missing env-variable names (a placeholder key counts as missing), or None when the run can proceed — a usable key, ambient auth (Azure AD, the AWS credential chain), or a provider-scoped api_base, which only the run itself can judge.
• The runner resolves the event's PR — top-level pull_request, a comment's issue.pull_request, or a workflow_run's pull_requests[0] — applies repo settings from it, then gates on the active provider's key before any tool runs: on failure it raises SystemExit naming the env variables to set; the reason becomes the job's error line and nothing key-derived flows through a print or log sink (CodeQL clean).
• Events with no PR — a plain issue comment, a push-origin workflow_run — skip the gate and let the run decide, as on main.
• The old OPENAI_KEY not set print-and-continue branch is gone — the gate subsumes it, and non-OpenAI setups no longer see the misleading OpenAI-specific warning.
• The troubleshooting entry for dummy_key now describes the early stop instead of the old symptom.

Testing

• Gate tests: a missing key stops before any tool runs, names the env variable, and fails the action; repo settings are applied before the gate for PR, comment, and workflow_run events; events without a PR skip the gate; a passing gate proceeds exactly as before.
• Doctor-level: the shared check is None with a key, names OPENAI_API_KEY without one, and defers to the run for Azure ambient auth (with and without Azure AD settings), Bedrock, and a custom api_base — one test each.
• The runner core suite gets an autouse fixture defaulting the gate to pass (those tests exercise tools, not credentials); the gate has its own file. The doctor and runner suites and ruff pass locally; CI is green.

Fixes #4324

[the-pr-agent-bot]

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

🎫 Ticket compliance analysis 🔶 4195 - Fully compliant Compliant requirements: Retain the exact error message and its three troubleshooting causes. Replace the OpenAI-specific warning with provider-aware key resolution. 4324 - Partially compliant Compliant requirements: Reuse doctor's provider-key resolution and reject placeholder keys. Stop before tools run and mark the action failed when the check reports a missing key. Name the missing environment variable and point to workflow configuration without exposing key values. Non-compliant requirements: Preserve valid-key, settings-provided-key, and keyless-provider runs when the active provider is selected through repository settings.

⏱️ Estimated effort to review: 3 🔵🔵🔵⚪⚪

🧪 PR contains tests

🔒 No security concerns identified

⚡ Recommended focus areas for review Premature validation The gate runs before the event payload is read and repository settings are applied. A repository that selects a non-OpenAI model in .pr_agent.toml and supplies only that provider's credentials can therefore be rejected for missing OPENAI_API_KEY, because the check still sees the default OpenAI model. This also blocks repository-selected keyless providers. Run the check after repository settings have been loaded, but before tool execution. # Gate on the active provider's key before any tool runs: carrying on with the # placeholder only fails every call a full run later (#4195, #4324). Keyless # providers defer to the run, as in `pr-agent doctor`. missing_key_reason = missing_provider_key_reason() if missing_key_reason: print(f"Stopping before any tool runs: {missing_key_reason}. " "Add it to this workflow's env; 'pr-agent doctor' preflights the same check by hand.") _mark_action_failed() return

---

⚙️ Agent run details

• Model: openai/gpt-6.1-sol
• Tokens: 9,424 in / 645 out / 10,069 total
• Time cost: 14.3s
• AI calls: 1

[the-pr-agent-bot]

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible bug	Test repository configuration before validation Add a regression test requiring repository settings to be applied before the credential gate. The current gate runs before the event payload is read, so it can reject a deployment using the default provider even when repository settings select a different, correctly configured provider; move validation after apply_repo_settings. tests/unittest/test_github_action_runner_key_gate.py [37-54] -async def test_valid_key_proceeds_past_the_gate(monkeypatch, action_env, capsys): - monkeypatch.setattr(github_action_runner, "missing_provider_key_reason", lambda: None) +async def test_repository_settings_are_applied_before_key_gate(monkeypatch, action_env, tmp_path): + import json + + event_path = tmp_path / "event.json" + event_path.write_text( + json.dumps({ + "action": "opened", + "number": 1, + "sender": {"login": "contributor", "type": "User"}, + "repository": { + "full_name": "owner/repo", + "default_branch": "main", + }, + "pull_request": { + "number": 1, + "html_url": "https://github.com/owner/repo/pull/1", + "url": "https://api.github.com/repos/owner/repo/pulls/1", + "user": {"login": "contributor", "type": "User"}, + "draft": False, + }, + }), + encoding="utf-8", + ) + monkeypatch.setenv("GITHUB_EVENT_PATH", str(event_path)) monkeypatch.setattr( github_action_runner, "get_setting_or_env", lambda key, default=None: default, ) - status = github_action_runner._ActionStatus() - token = github_action_runner._action_status.set(status) - try: - # The gate passes; the missing event file then fails exactly as it would on main. - with pytest.raises(FileNotFoundError): - await github_action_runner.run_action() - finally: - github_action_runner._action_status.reset(token) + repository_settings_applied = False - assert "Stopping before any tool runs" not in capsys.readouterr().out - assert status.failed is False + def apply_settings(*args, **kwargs): + nonlocal repository_settings_applied + repository_settings_applied = True + class GateReached(Exception): + pass + + def check_key(): + assert repository_settings_applied, "Credential validation ran before repository settings" + raise GateReached + + monkeypatch.setattr(github_action_runner, "apply_repo_settings", apply_settings) + monkeypatch.setattr(github_action_runner, "missing_provider_key_reason", check_key) + + with pytest.raises(GateReached): + await github_action_runner.run_action() + Suggestion importance[1-10]: 7 __ Why: The gate runs before repository configuration can be loaded, risking rejection based on the default provider rather than the repository-selected provider. The proposed regression test usefully enforces the required ordering, although moving the gate after apply_repo_settings still requires a production-code change.	Medium

---

⚙️ Agent run details

• Model: openai/gpt-6.1-sol
• Tokens: 11,978 in / 2,528 out / 14,506 total
• Time cost: 62.7s
• AI calls: 2

[Yoel-qin]

The CodeQL round: the query taints anything derived near key handling, and treats redact_credentials as opaque, so printing the reason kept failing regardless of its actual content. The gate now renders only env var names from the constant provider table (the provider itself is a lookup key, never rendered) and raises SystemExit instead of printing — the query's sinks are logging, print and stdout/stderr writes, none of which a raise reaches. Verified against the CleartextLogging sink definitions in the CodeQL source.

[IsmaelMartinez]

Thanks @Yoel-qin for moving the gate after repo settings, which the bot caught first, and for keeping the message to env-var names only. One ask before merge: the gate stops setups that run fine today. Probing missing_provider_key_env_vars() with no key, an OPENAI.API_BASE gateway, Bedrock on the AWS credential chain and Ollama with its api_base all return missing, because the table lists an optional key for each. Suggestion inline, verified against the PR's tests.

[IsmaelMartinez]

Thanks for applying this and adding a test per case. My earlier suggestion missed two Azure paths the handler supports: companion credentials (AZURE_CLIENT_ID, AZURE_TENANT_ID, AZURE_CLIENT_SECRET) and AZURE_AD_TOKEN, so those runs still stop. Deferring azure like bedrock covers both (suggestion below); could you add a test for each?

[IsmaelMartinez]

Suggested change

	ambient = settings.get("AZURE_AD.CLIENT_ID", None) or provider.startswith("bedrock")
	ambient = settings.get("AZURE_AD.CLIENT_ID", None) or provider.startswith(("bedrock", "azure"))

[Yoel-qin]

Applied — azure defers on its own like bedrock now, with a test for azure with no Azure AD settings next to the others. AZURE_AD.CLIENT_ID still covers the configured-client case for any provider.

[IsmaelMartinez]

Thanks @Yoel-qin for deferring azure and adding its test; the ambient-auth cases all pass now. One ask: on issue_comment and workflow_run the gate runs before the repo's .pr_agent.toml is applied, so a repo that picks its provider there gets stopped against the default model on every /review comment. Main dispatches the same comment. Details inline.

[IsmaelMartinez]

On comment events pr_url at line 247 is empty, so repo settings are not applied when this gate runs. With anthropic/... in .pr_agent.toml and only ANTHROPIC_API_KEY set, a /review comment stops here. Line 247 is outside the diff, so here it is as code:

        pull_request = event_payload.get("pull_request") or event_payload.get("issue", {}).get("pull_request") or {}
        pr_url = pull_request.get("html_url")

workflow_run applies its settings at line 484, after this gate, so it needs the same treatment. Could you add an issue_comment case to test_repository_settings_are_applied_before_key_gate?

[Yoel-qin]

Good catch, thanks. I moved the URL lookup into a _settings_pr_url helper that handles all three payload shapes, so comment and workflow_run events get their repo settings applied before the gate too. The duplicate apply_repo_settings in the workflow_run branch is gone. Ordering test now has issue_comment and workflow_run cases.

[IsmaelMartinez]

Thanks @Yoel-qin, the _settings_pr_url helper covers comment and workflow_run events nicely. One ask I should have raised last round: events with no PR behind them (a comment on a plain issue, a workflow_run started by a push) still reach the gate with the default model, so a repo that sets its provider in .pr_agent.toml gets a red run where main stays green. Gating only when a PR URL resolved fixes both; suggestions inline.

[IsmaelMartinez]

Resolve the url outside the try so the gate below can use it:

Suggested change

	try:
	get_logger().info("Applying repo settings")
	pr_url = event_payload.get("pull_request", {}).get("html_url")
	pr_url = _settings_pr_url(event_payload, GITHUB_EVENT_NAME)
	pr_url = _settings_pr_url(event_payload, GITHUB_EVENT_NAME)
	try:
	get_logger().info("Applying repo settings")

[Yoel-qin]

Done.

[IsmaelMartinez]

No PR, no repo settings: let the run decide, as main does.

Suggested change

	missing_key = missing_provider_key_env_vars()
	missing_key = missing_provider_key_env_vars() if pr_url else None

[Yoel-qin]

Done — no PR means the gate stays out of the way, like on main. Added a test for both shapes: a plain issue comment and a push-origin workflow_run.