Tighten Codex service failure classification#59
Conversation
Co-Authored-By: Codex <noreply@openai.com>
🤖 Codex PR ReviewPlease ensure a human reviewer checks this PR before merging. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 32d159e504
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| auth_config_signals = ( | ||
| "permission denied", | ||
| "unauthorized", | ||
| "forbidden", | ||
| "oidc", | ||
| "missing bearer", | ||
| "missing token", | ||
| "invalid token", | ||
| "bad credentials", | ||
| "not allowed", | ||
| "allowlist", | ||
| "api key is required", | ||
| "no api key configured", | ||
| "secret is missing", | ||
| "secret not configured", |
There was a problem hiding this comment.
When the service clone step fails with GitHub’s usual fatal: authentication failed ... message, _prepare_repo() wraps it as git clone failed ..., but this new allowlist no longer matches that phrase. Those jobs are now marked unknown_failure, so run_monthly_codex_audit.py will skip the infra-failure path and, for auto, can fall back to API remediation instead of reporting the service auth/config outage. Please include the explicit authentication-failure wording here and in the mirrored classifiers.
Useful? React with 👍 / 👎.
| "allowlist", | ||
| "api key is required", | ||
| "no api key configured", | ||
| "secret is missing", | ||
| "secret not configured", |
There was a problem hiding this comment.
For request-time 401s there is no async job failure_category, so run_codex_service() relies on this tuple to decide whether to return the infra exit code. The service still emits config errors such as CODEX_AUDIT_SERVICE_ALLOWED_REPOSITORIES is required before a job is created; after replacing the broad allow match with only allowlist, those messages become unknown_failure, and auto runs can proceed to API fallback instead of posting the service-infra failure comment. Add explicit signals for these required service allowlist/config errors.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 428d439280
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| "oidc", | ||
| "missing bearer", | ||
| "missing token", | ||
| "invalid token", |
There was a problem hiding this comment.
When the script-based audit service rejects a request because the GitHub OIDC JWT is past exp, it raises PermissionError("Token expired") (scripts/codex_audit_service.py:557), and the client wraps that 401 body before this classifier runs. Because the narrowed signals only include missing token/invalid token, that request-time auth outage is now unknown_failure, so auto runs can fall back to API remediation instead of posting the service-infrastructure failure. Please keep an explicit token expired signal here and in the mirrored service classifier.
Useful? React with 👍 / 👎.
Summary
Tests