-
Notifications
You must be signed in to change notification settings - Fork 6
fix(supervisor): detect gh auth failures to prevent clean_exit_no_signal retry waste (t198) #924
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 | ||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -4766,6 +4766,19 @@ extract_log_metadata() { | |||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "task_obsolete=$task_obsolete" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| # GitHub auth failure detection (t198): workers that complete their work but | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # can't push/create a PR because gh auth is expired. This is the #1 cause of | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # clean_exit_no_signal (79% of cases in production logs). Without this check, | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # the supervisor retries them 3x at Opus cost, each failing identically. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # Search the FULL log (not just tail) because auth failures appear in tool | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # output mid-log, not at the end. This is safe because gh auth patterns are | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # specific enough to avoid false positives from documentation content. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| local gh_auth_failure="false" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if grep -qiE 'gh auth (status|login).*fail|authentication token.*expired|not logged in|try authenticating|gh: To use .* in a non-interactive context|could not authenticate|failed to authenticate' "$log_file" 2>/dev/null; then | ||||||||||||||||||||||||||||||||||||||||||||||||||
| gh_auth_failure="true" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "gh_auth_failure=$gh_auth_failure" | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4769
to
+4780
Contributor
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. Tighten auth-failure regex to avoid false positives in full-log scan. Generic phrases like “not logged in” can appear in docs or code samples within logs, which would incorrectly block a task. Scope matches to gh/git auth error lines (e.g., Suggested refinement- if grep -qiE 'gh auth (status|login).*fail|authentication token.*expired|not logged in|try authenticating|gh: To use .* in a non-interactive context|could not authenticate|failed to authenticate' "$log_file" 2>/dev/null; then
+ if grep -qiE '(gh auth (status|login).*(fail|not logged in|authenticate)|gh:.*(not logged in|authenticate)|GitHub CLI.*(not logged in|authenticate)|fatal:.*authentication failed|remote:.*(authentication|permission) denied)' "$log_file" 2>/dev/null; then
gh_auth_failure="true"
fi📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| # Task tool parallelism tracking (t217): detect whether the worker used the | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # Task tool (mcp_task) to spawn sub-agents for parallel work. This is a | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # heuristic quality signal — workers that parallelise independent subtasks | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -5139,6 +5152,20 @@ evaluate_worker() { | |||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| # GitHub auth failure detection (t198): workers that complete their work but | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # can't push/create a PR because gh auth is expired or invalid. This is the | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # #1 cause of clean_exit_no_signal (79% of cases). Retrying is pointless — | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # the auth issue persists across retries. Block immediately so the human can | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # fix credentials, then reset the task. | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ "$meta_exit_code" == "0" && "$meta_signal" == "none" ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||
| local meta_gh_auth_failure | ||||||||||||||||||||||||||||||||||||||||||||||||||
| meta_gh_auth_failure=$(_meta_get "gh_auth_failure" "false") | ||||||||||||||||||||||||||||||||||||||||||||||||||
| if [[ "$meta_gh_auth_failure" == "true" ]]; then | ||||||||||||||||||||||||||||||||||||||||||||||||||
| echo "blocked:gh_auth_expired" | ||||||||||||||||||||||||||||||||||||||||||||||||||
| return 0 | ||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
| # Clean exit with no completion signal and no PR (checked DB + gh API above) | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # = likely incomplete. The agent finished cleanly but didn't emit a signal | ||||||||||||||||||||||||||||||||||||||||||||||||||
| # and no PR was found. Retry (agent may have run out of context or hit a | ||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||
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.
This use of
2>/dev/nullto suppressgrep's error output violates the repository's style guide (line 50), which states that2>/dev/nullis only acceptable when redirecting to log files. A safer and more idiomatic way to handle cases where the log file might not exist is to check for the file's existence before callinggrep.References
2>/dev/null, allowing it only when redirecting to log files. The current code uses it to hide potential 'file not found' errors fromgrep. (link)