-
Notifications
You must be signed in to change notification settings - Fork 0
Tighten Codex service failure classification #59
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -171,7 +171,23 @@ class BridgeError(RuntimeError): | |
|
|
||
| def classify_service_failure(error: str) -> str: | ||
| text = error.lower() | ||
| if any(word in text for word in ("permission", "unauth", "forbidden", "oidc", "token", "allow", "secret")): | ||
| auth_config_signals = ( | ||
| "permission denied", | ||
| "unauthorized", | ||
| "forbidden", | ||
| "oidc", | ||
| "missing bearer", | ||
| "missing token", | ||
| "invalid token", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When the script-based audit service rejects a request because the GitHub OIDC JWT is past Useful? React with 👍 / 👎. |
||
| "bad credentials", | ||
| "not allowed", | ||
| "allowlist", | ||
| "api key is required", | ||
| "no api key configured", | ||
| "secret is missing", | ||
| "secret not configured", | ||
|
Comment on lines
+184
to
+188
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
For request-time 401s there is no async job Useful? React with 👍 / 👎. |
||
| ) | ||
| if any(signal in text for signal in auth_config_signals): | ||
| return "auth_or_config_failure" | ||
| if any(word in text for word in ("quota", "rate limit", "too many active", "budget")): | ||
| return "quota_or_capacity_failure" | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the service clone step fails with GitHub’s usual
fatal: authentication failed ...message,_prepare_repo()wraps it asgit clone failed ..., but this new allowlist no longer matches that phrase. Those jobs are now markedunknown_failure, sorun_monthly_codex_audit.pywill skip the infra-failure path and, forauto, 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 👍 / 👎.