Conversation
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0c416dca-8ccc-4c14-aaa5-c277fa501663 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
…ure issues Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0c416dca-8ccc-4c14-aaa5-c277fa501663 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/0c416dca-8ccc-4c14-aaa5-c277fa501663 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
|
||
| // Known AI provider API hostnames and their associated credential names. | ||
| // The host patterns are matched against the "host" field in audit.jsonl entries. | ||
| const FIREWALL_AUTH_PROVIDER_HOSTS = /** @type {Array<{provider: string, pattern: RegExp, credential: string}>} */ [ |
There was a problem hiding this comment.
@copilot pass list of registered agentic engine as env var to determine providers (if they have a port, they are a provider)
There was a problem hiding this comment.
Done in commit 3af8d05.
Added getEngineAPIHosts() in notify_comment.go that emits GH_AW_ENGINE_API_HOSTS (comma-separated API hostnames) for each engine type — e.g. for Copilot it emits all four *.githubcopilot.com inference endpoints; Claude → api.anthropic.com; Codex → api.openai.com; Gemini → generativelanguage.googleapis.com. Custom engine.api-target overrides the defaults.
In JS, buildRegisteredProviderEntries() reads GH_AW_ENGINE_API_HOSTS + GH_AW_ENGINE_ID to build the detection list dynamically (falls back to hardcoded patterns when the env var is absent).
|
|
||
| for (const line of content.split("\n")) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed || trimmed[0] !== "{") continue; |
There was a problem hiding this comment.
Done in commit 3af8d05. Added a per-line regex check (/"status"\s*:\s*40[13]/.test(trimmed)) before JSON.parse, so lines that can't possibly contain a 401/403 status are skipped without incurring the parse cost. This is now the third pass in the three-pass strategy: file-level scan → per-line scan → full JSON parse.
| try { | ||
| return "\n" + renderTemplateFromFile(templatePath, { providers: providersList }); | ||
| } catch { | ||
| // Template not available — return inline message |
There was a problem hiding this comment.
@copilot template should always be available, remove fallback
There was a problem hiding this comment.
Done in commit 3af8d05. Removed the try/catch fallback — buildCredentialAuthErrorContext now calls renderTemplateFromFile directly without the inline fallback message.
🧪 Test Quality Sentinel ReportTest Quality Score: 82/100✅ Excellent test quality
Test Classification DetailsView All 16 Test Classifications
Minor Observation — Similar Provider Detection TestsThe four provider-detection tests ( Suggestion: These could be collapsed into a single table-driven test: it.each([
{ host: api.enterprise.githubcopilot.com:443, status: 401, provider: Copilot, credKey: COPILOT_GITHUB_TOKEN },
{ host: api.openai.com:443, status: 401, provider: OpenAI, credKey: OPENAI_API_KEY },
{ host: api.anthropic.com:443, status: 403, provider: Anthropic, credKey: ANTHROPIC_API_KEY },
{ host: generativelanguage.googleapis.com:443, status: 403, provider: Gemini, credKey: GEMINI_API_KEY },
])("$provider auth rejection", ({ host, status, provider, credKey }) => { ... });This makes it trivial to add future providers and removes the duplication penalty. Language SupportTests analyzed:
Scoring Breakdown
Verdict
📖 Understanding Test ClassificationsDesign Tests (High Value) verify what the system does:
Implementation Tests (Low Value) verify how the system does it:
Goal: Shift toward tests that describe the system's behavioral contract — the promises it makes to its users and collaborators. References:
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a credentials-focused alert to agentic workflow failure issues/comments by scanning the firewall audit.jsonl for HTTP 401/403 responses to known AI provider endpoints, helping maintainers quickly identify invalid/missing API credentials.
Changes:
- Added
parseFirewallAuthErrors+buildCredentialAuthErrorContextand wired{credential_auth_error_context}into failure issue/comment templates. - Introduced a new prompt template (
credential_auth_error.md) and added unit tests covering provider detection and path derivation. - Updated multiple workflow lockfiles (permissions and MCP CLI mounting/allowlist changes).
Show a summary per file
| File | Description |
|---|---|
| actions/setup/md/credential_auth_error.md | New markdown template for the credential authentication alert. |
| actions/setup/md/agent_failure_issue.md | Inserts {credential_auth_error_context} into the failure issue template. |
| actions/setup/md/agent_failure_comment.md | Inserts {credential_auth_error_context} into the failure comment template. |
| actions/setup/js/handle_agent_failure.cjs | Implements firewall auth rejection parsing and alert context generation; injects into failure template context. |
| actions/setup/js/handle_agent_failure.test.cjs | Adds unit tests for auth rejection parsing and alert context rendering/path derivation. |
| .github/workflows/unbloat-docs.lock.yml | Updates compiled workflow (permissions + prompt assembly changes + MCP CLI mounting). |
| .github/workflows/tidy.lock.yml | Updates compiled workflow (permissions + prompt assembly changes + MCP CLI mounting). |
| .github/workflows/plan.lock.yml | Adds pull-requests: write permission to activation job. |
| .github/workflows/pdf-summary.lock.yml | Adds pull-requests: write permission to activation job. |
| .github/workflows/mergefest.lock.yml | Adds pull-requests: write permission to activation job. |
| .github/workflows/brave.lock.yml | Adds pull-requests: write permission to activation job. |
| .github/workflows/ace-editor.lock.yml | Adds pull-requests: write permission to activation job. |
Copilot's findings
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Files reviewed: 12/12 changed files
- Comments generated: 6
| actions: read | ||
| contents: read | ||
| issues: write | ||
| pull-requests: write |
There was a problem hiding this comment.
This workflow sets pull-requests: write, but it only triggers when github.event.issue.pull_request == null and the GitHub MCP server is configured read-only (GITHUB_READ_ONLY: "1"). Please remove pull-requests: write (or downgrade to pull-requests: read if required) to keep least-privilege.
| pull-requests: write |
| actions: read | ||
| contents: read | ||
| issues: write | ||
| pull-requests: write |
There was a problem hiding this comment.
This workflow adds pull-requests: write, but the workflow’s interactions appear limited to PR context/commenting (issues endpoint) and the GitHub MCP server is configured read-only (GITHUB_READ_ONLY: "1"). Please downgrade to pull-requests: read (or remove entirely if not needed) to follow least-privilege.
| pull-requests: write | |
| pull-requests: read |
| - name: Mount MCP servers as CLIs | ||
| id: mount-mcp-clis | ||
| continue-on-error: true | ||
| env: | ||
| MCP_GATEWAY_API_KEY: ${{ steps.start-mcp-gateway.outputs.gateway-api-key }} |
There was a problem hiding this comment.
This PR includes workflow behavioral changes (e.g., mounting MCP servers as CLIs and expanding allowed tool patterns) in addition to the firewall auth alert work described in the PR metadata. Please either update the PR description to cover these workflow changes or split them into a separate PR so reviewers can assess them independently.
| for (const line of content.split("\n")) { | ||
| const trimmed = line.trim(); | ||
| if (!trimmed || trimmed[0] !== "{") continue; | ||
|
|
There was a problem hiding this comment.
parseFirewallAuthErrors iterates with content.split("\n"), which duplicates the entire audit file in memory as an array of lines. For large audit.jsonl files this can be a significant memory hit; consider streaming line-by-line (e.g., readline over a read stream) and optionally early-exiting once all known providers have been found.
| contents: read | ||
| discussions: write | ||
| issues: write | ||
| pull-requests: write |
There was a problem hiding this comment.
This workflow sets pull-requests: write, but the workflow only runs for issue comments on non-PR issues / discussions and the GitHub MCP server is configured as read-only (GITHUB_READ_ONLY: "1"). Please drop the permission entirely (or reduce to pull-requests: read if something truly needs PR read access) to keep least-privilege.
| pull-requests: write |
| actions: read | ||
| contents: read | ||
| issues: write | ||
| pull-requests: write |
There was a problem hiding this comment.
This workflow sets pull-requests: write, but it is triggered only for issues / issue comments on non-PR issues (and workflow_dispatch), and the GitHub MCP server is configured read-only (GITHUB_READ_ONLY: "1"). Please remove pull-requests: write (or downgrade to pull-requests: read if required).
| pull-requests: write |
|
🛠️ Agentic Maintenance updated this pull request branch. |
|
@copilot review all comments |
…er-line pre-filter, remove fallback Agent-Logs-Url: https://github.com/github/gh-aw/sessions/190dd3bf-379a-4fef-8228-7164022e7524 Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
|
Just as a heads up, I was blocked by some firewall rules while working on your feedback. Expand below for details. Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
Agent-Logs-Url: https://github.com/github/gh-aw/sessions/87d3453f-96fb-4b79-a698-6b83cd79e4ce Co-authored-by: pelikhan <4175913+pelikhan@users.noreply.github.com>
Reviewed all automated reviewer comments: Code change ( Lock file permissions (
|
Summary
When an agentic workflow fails, the failure issue/comment now includes a 🔑 Credential Authentication Failed alert when the firewall audit log (
audit.jsonl) contains HTTP 401/403 responses from AI provider API endpoints.This helps maintainers quickly identify missing, expired, or invalid API credentials as the root cause of workflow failures.
Changes
New:
getEngineAPIHosts()innotify_comment.goThe workflow compiler now emits a
GH_AW_ENGINE_API_HOSTSenvironment variable containing the comma-separated list of the engine's primary AI inference API hostnames (e.g. all four*.githubcopilot.comvariants for Copilot,api.anthropic.comfor Claude,api.openai.comfor Codex,generativelanguage.googleapis.comfor Gemini). An explicitengine.api-targetoverride takes precedence over the defaults.New:
parseFirewallAuthErrors(auditJsonlPath)Parses the firewall
audit.jsonlJSONL log for authentication rejection entries using a three-pass approach for performance on large files:JSON.parsecostProviders are resolved dynamically from
GH_AW_ENGINE_API_HOSTS/GH_AW_ENGINE_ID(set by the compiler). Falls back to a hardcoded list of known public provider API hosts when the env var is absent:COPILOT_GITHUB_TOKEN) —*.githubcopilot.comCODEX_API_KEY/OPENAI_API_KEY) —api.openai.comANTHROPIC_API_KEY) —api.anthropic.comGEMINI_API_KEY) —generativelanguage.googleapis.comNew:
buildRegisteredProviderEntries()Reads
GH_AW_ENGINE_API_HOSTSandGH_AW_ENGINE_IDto build the detection list dynamically, mapping engine IDs to human-readable labels and credential names. Falls back to hardcoded patterns when the env var is not set.New:
buildCredentialAuthErrorContext([auditJsonlPathOverride])Builds the formatted alert string for inclusion in the failure issue/comment. Derives the
audit.jsonlpath fromGH_AW_AGENT_OUTPUT. Renders thecredential_auth_error.mdtemplate directly (no inline fallback).New:
actions/setup/md/credential_auth_error.mdPrompt template for the credentials alert, rendered with a
{providers}list of affected providers.Updated:
agent_failure_issue.md/agent_failure_comment.mdAdded
{credential_auth_error_context}placeholder, placed immediately after{secret_verification_context}.Tests
129 unit tests (19 new) covering:
GH_AW_ENGINE_API_HOSTSenv var (known and custom engines)GH_AW_AGENT_OUTPUT