Skip to content

Comments

fix: prevent auto-closing failed issues and preserve em-dash content#2112

Closed
marcusquinn wants to merge 1 commit intomainfrom
bugfix/issue-sync-fixes
Closed

fix: prevent auto-closing failed issues and preserve em-dash content#2112
marcusquinn wants to merge 1 commit intomainfrom
bugfix/issue-sync-fixes

Conversation

@marcusquinn
Copy link
Owner

@marcusquinn marcusquinn commented Feb 21, 2026

Summary

Testing

Files Changed

File Change
.agents/scripts/issue-sync-lib.sh Rewrite metadata stripping to preserve em-dash content
.agents/scripts/supervisor/issue-sync.sh Replace gh issue close with comment + needs-review label for failed tasks

Summary by CodeRabbit

  • Bug Fixes
    • Improved task description parsing to more accurately extract and clean metadata while preserving important content.
    • Enhanced issue synchronization failure handling to flag problematic issues for review instead of auto-closing them, allowing for better issue tracking and resolution workflows.

…in task descriptions

- Supervisor no longer closes issues when tasks fail after max retries;
  instead posts a failure comment and adds 'needs-review' label, keeping
  the issue open for human triage (fixes #2106, #1763, #1762)
- parse_task_line() now strips metadata fields individually instead of
  truncating at the first tag, preserving em-dash content in descriptions
@gemini-code-assist
Copy link

Summary of Changes

Hello @marcusquinn, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request addresses two key issues: preventing the automatic closure of failed tasks in the supervisor system and improving the parsing of issue descriptions to correctly handle em-dash content. These changes enhance the robustness of task management by ensuring human review for failed tasks and improving data integrity in issue descriptions.

Highlights

  • Supervisor auto-close prevention: Failed tasks are no longer automatically closed. Instead, a failure comment is posted, and a "needs-review" label is added, keeping the issue open for human intervention.
  • Em-dash description parsing: The "parse_task_line()" function now strips metadata fields individually, ensuring that em-dash content in issue descriptions is preserved instead of being truncated.
Changelog
  • .agents/scripts/issue-sync-lib.sh
    • Rewrote the description extraction logic in "parse_task_line()" to individually remove metadata fields (tags, estimates, dates, assignees, refs) using multiple "sed" commands.
    • Introduced a "full_text" variable to capture the entire description before metadata stripping, ensuring em-dash content is not lost.
    • Added new "sed" patterns to specifically remove various metadata types like tags, estimates, date fields, assignee, blocked-by, blocks, model, category, GH refs, PR refs, and plan links.
    • .agents/scripts/supervisor/issue-sync.sh
    • Replaced "gh issue close" with "gh issue comment" for failed tasks, preventing automatic closure.
    • Added logic to apply a "needs-review" label to failed issues using "gh issue edit".
    • Updated the log message to reflect that issues are now "flagged for review" instead of "closed as failed".
    • Removed the "return 0" statement after handling failed tasks, allowing subsequent label updates to proceed.
Activity
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Feb 21, 2026

Walkthrough

Modified task description parsing in issue-sync-lib.sh to apply multi-stage text sanitization that preserves content while removing metadata, and updated failure handling in issue-sync.sh to flag issues for review instead of auto-closing on sync failures.

Changes

Cohort / File(s) Summary
Task Description Parsing
.agents/scripts/issue-sync-lib.sh
Rewrote parse_task_line description extraction to apply granular, sequential sanitization: extract full text after task ID, then pipeline through tag removal, estimate pattern stripping, metadata field cleanup (logged, started, completed, actual, verified, assignee, blocked-by, blocks, model, category, ref, pr, plan links), and whitespace trimming.
Issue Sync Failure Handling
.agents/scripts/supervisor/issue-sync.sh
Changed terminal failure path in sync_issue_status_label to post a failure comment and apply "needs-review" label instead of auto-closing the issue; allows subsequent state logic to apply additional labels and leaves issue open for manual review.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • #2029: Directly modifies the same parse_task_line function for description extraction and sanitization logic.
  • #1504: Updates sync_issue_status_label failure/comment handling with similar close-vs-flag behavioral changes.
  • #1391: Changes issue-closing behavior to avoid auto-close in cases requiring manual review, aligned with this PR's flag-for-review pattern.

Poem

🔧 Descriptions stripped clean, yet meaning stays,
Multi-stage pipelines brighten parsing days,
When failures strike, we pause and flag for care,
No hasty closes—let reviewers beware! 📋✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 inconclusive)

Check name Status Explanation Resolution
Linked Issues check ❓ Inconclusive Issue #2106 focuses on investigating filesystem permission failures for task t1165.1, but PR changes address issue auto-close prevention and description parsing, which appear tangential to the root cause investigation objective. Clarify the relationship between these code changes and the permission_denied_write_operations investigation—does fixing auto-close or parsing prevent the failures, or are these separate fixes bundled together?
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately summarizes both main changes: preventing auto-close of failed issues and preserving em-dash content in descriptions.
Out of Scope Changes check ✅ Passed Both file changes directly address the stated PR objectives: preventing auto-close of failed issues and preserving em-dash content in task descriptions.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/issue-sync-fixes

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 38 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Sat Feb 21 16:42:39 UTC 2026: Code review monitoring started
Sat Feb 21 16:42:40 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 38

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 38
  • VULNERABILITIES: 0

Generated on: Sat Feb 21 16:42:42 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link

Copy link

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request makes two valuable fixes. First, it prevents failed tasks from being automatically closed on GitHub, instead adding a needs-review label and a comment, which improves the triage process for failed tasks. Second, it refactors the task description parser to correctly handle em-dashes and other special content by individually removing metadata fields instead of truncating the description. My review includes suggestions to improve script efficiency by consolidating multiple sed commands and to adhere to the repository's style guide and rules by removing blanket error suppression on gh commands, which will improve debuggability.

Comment on lines +91 to 100
description=$(echo "$full_text" |
sed -E 's/ #[a-z][a-z0-9-]*//g' | # Remove tags
sed -E 's/ ~[0-9]+[hmd]( \(ai:[^)]+\))?//g' | # Remove estimate
sed -E 's/ (logged|started|completed|actual|verified):[^ ]+//g' | # Remove date fields
sed -E 's/ (assignee|blocked-by|blocks|model|category):[^ ]+//g' | # Remove other metadata
sed -E 's/ ref:GH#[0-9]+//g' | # Remove GH ref
sed -E 's/ pr:#[0-9]+//g' | # Remove PR ref
sed -E 's/ → \[todo\/PLANS\.md#[^]]+\]//g' | # Remove plan link
sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//' || # Trim whitespace
echo "")

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Chaining multiple sed commands via pipes can be inefficient as each one starts a new process. You can consolidate these into a single sed call using the -e option for each expression. This improves both performance and readability.

Suggested change
description=$(echo "$full_text" |
sed -E 's/ #[a-z][a-z0-9-]*//g' | # Remove tags
sed -E 's/ ~[0-9]+[hmd]( \(ai:[^)]+\))?//g' | # Remove estimate
sed -E 's/ (logged|started|completed|actual|verified):[^ ]+//g' | # Remove date fields
sed -E 's/ (assignee|blocked-by|blocks|model|category):[^ ]+//g' | # Remove other metadata
sed -E 's/ ref:GH#[0-9]+//g' | # Remove GH ref
sed -E 's/ pr:#[0-9]+//g' | # Remove PR ref
sed -E 's/ → \[todo\/PLANS\.md#[^]]+\]//g' | # Remove plan link
sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//' || # Trim whitespace
echo "")
description=$(echo "$full_text" | sed -E \
-e 's/ #[a-z][a-z0-9-]*//g' \
-e 's/ ~[0-9]+[hmd]( \(ai:[^)]+\))?//g' \
-e 's/ (logged|started|completed|actual|verified):[^ ]+//g' \
-e 's/ (assignee|blocked-by|blocks|model|category):[^ ]+//g' \
-e 's/ ref:GH#[0-9]+//g' \
-e 's/ pr:#[0-9]+//g' \
-e 's/ → \[todo\/PLANS\.md#[^]]+\]//g' \
-e 's/^[[:space:]]+//; s/[[:space:]]+$//' ||
echo "")
References
  1. Optimize shell script pipelines by replacing 'grep | sed' combinations with a single, more efficient 'sed' command where possible to improve performance. This principle extends to consolidating multiple 'sed' commands for efficiency.

Comment on lines +462 to +463
gh issue comment "$issue_number" --repo "$repo_slug" \
--body "$fail_comment" 2>/dev/null || true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Using 2>/dev/null suppresses all error messages from the gh command, which can hide important issues like authentication failures, network problems, or invalid issue numbers. The || true already prevents the script from exiting on failure. According to the repository style guide (line 50) and repository rules, 2>/dev/null should not be used for blanket error suppression. Please remove it to allow errors to be visible for debugging.

Suggested change
gh issue comment "$issue_number" --repo "$repo_slug" \
--body "$fail_comment" 2>/dev/null || true
gh issue comment "$issue_number" --repo "$repo_slug" \
--body "$fail_comment" || true
References
  1. Line 50: 2>/dev/null is acceptable ONLY when redirecting to log files, not blanket suppression. (link)
  2. Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
  3. In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting when a command like 'jq' fails on an optional lookup. Do not suppress stderr with '2>/dev/null' so that actual syntax or system errors remain visible for debugging.

Comment on lines 464 to +465
gh issue edit "$issue_number" --repo "$repo_slug" \
"${remove_args[@]}" 2>/dev/null || true
log_verbose "sync_issue_status_label: closed #$issue_number as failed ($task_id)"
return 0
--add-label "needs-review" "${remove_args[@]}" 2>/dev/null || true

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the previous comment, using 2>/dev/null here suppresses potentially important error messages from the gh command. This violates the repository style guide (line 50) and repository rules which advise against blanket error suppression. Please remove 2>/dev/null to aid in debugging potential issues.

Suggested change
gh issue edit "$issue_number" --repo "$repo_slug" \
"${remove_args[@]}" 2>/dev/null || true
log_verbose "sync_issue_status_label: closed #$issue_number as failed ($task_id)"
return 0
--add-label "needs-review" "${remove_args[@]}" 2>/dev/null || true
gh issue edit "$issue_number" --repo "$repo_slug" \
--add-label "needs-review" "${remove_args[@]}" || true
References
  1. Line 50: 2>/dev/null is acceptable ONLY when redirecting to log files, not blanket suppression. (link)
  2. Avoid using '2>/dev/null' for blanket suppression of command errors in shell scripts to ensure that authentication, syntax, or system issues remain visible for debugging.
  3. In shell scripts with 'set -e' enabled, use '|| true' to prevent the script from exiting when a command like 'jq' fails on an optional lookup. Do not suppress stderr with '2>/dev/null' so that actual syntax or system errors remain visible for debugging.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
.agents/scripts/issue-sync-lib.sh (1)

91-100: Consider consolidating 8 piped sed calls into one.

8 separate process forks per task line is acceptable for single invocations but becomes a bottleneck when parse_task_line is called in a loop over hundreds of tasks (e.g., during the ~400-issue backfill mentioned in the PR). All these substitutions can be expressed as a single sed with multiple -e expressions.

♻️ Proposed refactor — single `sed` invocation
-	description=$(echo "$full_text" |
-		sed -E 's/ #[a-z][a-z0-9-]*//g' |                                  # Remove tags
-		sed -E 's/ ~[0-9]+[hmd]( \(ai:[^)]+\))?//g' |                      # Remove estimate
-		sed -E 's/ (logged|started|completed|actual|verified):[^ ]+//g' |  # Remove date fields
-		sed -E 's/ (assignee|blocked-by|blocks|model|category):[^ ]+//g' | # Remove other metadata
-		sed -E 's/ ref:GH#[0-9]+//g' |                                     # Remove GH ref
-		sed -E 's/ pr:#[0-9]+//g' |                                        # Remove PR ref
-		sed -E 's/ → \[todo\/PLANS\.md#[^]]+\]//g' |                       # Remove plan link
-		sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//' ||                   # Trim whitespace
-		echo "")
+	description=$(echo "$full_text" |
+		sed -E \
+			-e 's/ #[a-z][a-z0-9-]*//g' \
+			-e 's/ ~[0-9]+[hmd]( \(ai:[^)]+\))?//g' \
+			-e 's/ (logged|started|completed|actual|verified):[^ ]+//g' \
+			-e 's/ (assignee|blocked-by|blocks|model|category):[^ ]+//g' \
+			-e 's/ ref:GH#[0-9]+//g' \
+			-e 's/ pr:#[0-9]+//g' \
+			-e 's/ ref:todo\/tasks\/[^ ]+//g' \
+			-e 's/ → \[todo\/PLANS\.md#[^]]+\]//g' \
+			-e 's/^[[:space:]]+//; s/[[:space:]]+$//' ||
+		echo "")
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/scripts/issue-sync-lib.sh around lines 91 - 100, The pipeline that
builds description from full_text currently spawns eight sed processes; inside
parse_task_line replace the chained sed calls that pipe from echo "$full_text"
with a single sed invocation using multiple -e expressions (or a single -E
script with semicolon-separated commands) that preserves each existing regex
(tag removal, estimate, date fields, metadata keys, GH/PR refs, plan link, and
trimming) and keep the existing "|| echo """ fallback; update the command that
sets description so it still reads from full_text and yields the same final
output but with one sed process instead of eight.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In @.agents/scripts/issue-sync-lib.sh:
- Around line 83-100: The description sanitization pipeline fails to strip
"ref:todo/tasks/..." references; update the pipeline that assigns description
(the sed chain that currently removes "ref:GH#...", "pr:#", and plan links) to
also remove patterns like "ref:todo/tasks/..." by adding a sed rule that strips
" ref:todo/tasks/<non-space>" (e.g., a sed -E replacement for "
ref:todo/tasks/[^ ]+") into the same chain that produces the description
variable so find_related_files and generated issue descriptions no longer leak
those refs.

In @.agents/scripts/supervisor/issue-sync.sh:
- Around line 467-468: The comment on the fall-through after the "failed" case
is misleading because state_to_status_label("failed") returns an empty string so
the subsequent non-terminal label logic is a no-op; update the comment near the
fall-through to state that the fall-through is intentional to let the "blocked"
branch (which yields a non-empty label) reuse the label-update logic and that
"failed" produces no label, or alternatively change the comment to say
"fall-through to shared label handling; no-op for failed (empty label)";
reference state_to_status_label, the "failed" and "blocked" cases, and the
guarded blocks that check if [[ -n "$new_label" ]] to guide where to change the
text.
- Around line 460-468: The script currently attempts to add the "needs-review"
label with gh issue edit which will silently fail if the label doesn't exist;
update the label creation flow by ensuring the "needs-review" label is created
(use the existing pattern that calls gh label create --force before gh issue
edit, e.g. in ensure_status_labels or the same place add_model_label is invoked)
so the --add-label call cannot fail, and add "needs-review" to the
ALL_STATUS_LABELS array so it is cleaned up during subsequent state transitions
(reference ensure_status_labels and ALL_STATUS_LABELS to locate where to add the
create call and the new label entry).

---

Nitpick comments:
In @.agents/scripts/issue-sync-lib.sh:
- Around line 91-100: The pipeline that builds description from full_text
currently spawns eight sed processes; inside parse_task_line replace the chained
sed calls that pipe from echo "$full_text" with a single sed invocation using
multiple -e expressions (or a single -E script with semicolon-separated
commands) that preserves each existing regex (tag removal, estimate, date
fields, metadata keys, GH/PR refs, plan link, and trimming) and keep the
existing "|| echo """ fallback; update the command that sets description so it
still reads from full_text and yields the same final output but with one sed
process instead of eight.

Comment on lines +83 to 100
# Extract FULL description (everything after task ID, including em-dash content)
# We'll clean out metadata fields separately, but preserve the full text
local full_text
full_text=$(echo "$line" | sed -E 's/^[[:space:]]*- \[.\] t[0-9]+(\.[0-9]+)* //' || echo "")

# Extract description by removing all metadata fields but keeping em-dash content
# Strategy: remove tags, estimate, and key:value pairs, but preserve text after em dash
local description
description=$(echo "$line" | sed -E 's/^[[:space:]]*- \[.\] t[0-9]+(\.[0-9]+)* //' |
sed -E 's/ (#[a-z]|~[0-9]|→ |logged:|started:|completed:|ref:|actual:|blocked-by:|blocks:|assignee:|verified:).*//' ||
description=$(echo "$full_text" |
sed -E 's/ #[a-z][a-z0-9-]*//g' | # Remove tags
sed -E 's/ ~[0-9]+[hmd]( \(ai:[^)]+\))?//g' | # Remove estimate
sed -E 's/ (logged|started|completed|actual|verified):[^ ]+//g' | # Remove date fields
sed -E 's/ (assignee|blocked-by|blocks|model|category):[^ ]+//g' | # Remove other metadata
sed -E 's/ ref:GH#[0-9]+//g' | # Remove GH ref
sed -E 's/ pr:#[0-9]+//g' | # Remove PR ref
sed -E 's/ → \[todo\/PLANS\.md#[^]]+\]//g' | # Remove plan link
sed -E 's/^[[:space:]]+//; s/[[:space:]]+$//' || # Trim whitespace
echo "")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

ref:todo/tasks/ metadata not stripped from description.

The sanitization pipeline removes ref:GH#, pr:#, and plan links, but ref:todo/tasks/ references (used in find_related_files at line 546) are not stripped. Any task line containing ref:todo/tasks/prd.md will have that text leak into the generated issue description.

🛠️ Proposed fix — add `ref:todo/tasks/` pattern to the pipeline
 		sed -E 's/ ref:GH#[0-9]+//g' |                                     # Remove GH ref
 		sed -E 's/ pr:#[0-9]+//g' |                                        # Remove PR ref
+		sed -E 's/ ref:todo\/tasks\/[^ ]+//g' |                            # Remove task file ref
 		sed -E 's/ → \[todo\/PLANS\.md#[^]]+\]//g' |                       # Remove plan link
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/scripts/issue-sync-lib.sh around lines 83 - 100, The description
sanitization pipeline fails to strip "ref:todo/tasks/..." references; update the
pipeline that assigns description (the sed chain that currently removes
"ref:GH#...", "pr:#", and plan links) to also remove patterns like
"ref:todo/tasks/..." by adding a sed rule that strips "
ref:todo/tasks/<non-space>" (e.g., a sed -E replacement for " ref:todo/tasks/[^
]+") into the same chain that produces the description variable so
find_related_files and generated issue descriptions no longer leak those refs.

Comment on lines +460 to 468
# DO NOT auto-close failed tasks - they need human review
# Add needs-review label and post failure comment, but keep issue OPEN
gh issue comment "$issue_number" --repo "$repo_slug" \
--body "$fail_comment" 2>/dev/null || true
gh issue edit "$issue_number" --repo "$repo_slug" \
"${remove_args[@]}" 2>/dev/null || true
log_verbose "sync_issue_status_label: closed #$issue_number as failed ($task_id)"
return 0
--add-label "needs-review" "${remove_args[@]}" 2>/dev/null || true
log_verbose "sync_issue_status_label: flagged #$issue_number for review ($task_id failed)"
# Don't return here - let the non-terminal state logic handle label updates
;;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

needs-review label will silently fail to apply if it doesn't exist on the repo.

GitHub CLI's --add-label requires the label to already exist in the repository — it will error (silently swallowed by 2>/dev/null || true) if needs-review hasn't been created. Since this label is the core deliverable of the PR fix, a silent failure means failed tasks appear to stay open but carry no needs-review signal.

The established in-file pattern (add_model_label, lines 178–184) always calls gh label create --force before gh issue edit --add-label. needs-review also needs to be added to ALL_STATUS_LABELS (line 344) so it is cleaned up in subsequent state transitions — right now it will persist as a zombie label when a failed task is re-queued, retried, and eventually succeeds/closes.

🛠️ Proposed fix

In ensure_status_labels (line 21+), add:

 	gh label create "status:done" --repo "$repo_slug" --color "6F42C1" --description "Task is complete" --force 2>/dev/null || true
+	gh label create "needs-review" --repo "$repo_slug" --color "E4E669" --description "Failed task needs human review" --force 2>/dev/null || true
 	return 0

In ALL_STATUS_LABELS (line 344):

-ALL_STATUS_LABELS="status:available,status:queued,status:claimed,status:in-review,status:blocked,status:verify-failed,status:needs-testing,status:done"
+ALL_STATUS_LABELS="status:available,status:queued,status:claimed,status:in-review,status:blocked,status:verify-failed,status:needs-testing,status:done,needs-review"
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/scripts/supervisor/issue-sync.sh around lines 460 - 468, The script
currently attempts to add the "needs-review" label with gh issue edit which will
silently fail if the label doesn't exist; update the label creation flow by
ensuring the "needs-review" label is created (use the existing pattern that
calls gh label create --force before gh issue edit, e.g. in ensure_status_labels
or the same place add_model_label is invoked) so the --add-label call cannot
fail, and add "needs-review" to the ALL_STATUS_LABELS array so it is cleaned up
during subsequent state transitions (reference ensure_status_labels and
ALL_STATUS_LABELS to locate where to add the create call and the new label
entry).

Comment on lines +467 to 468
# Don't return here - let the non-terminal state logic handle label updates
;;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Misleading fall-through comment — non-terminal label logic is a no-op for failed.

The comment says "let the non-terminal state logic handle label updates", but state_to_status_label("failed") returns "" (line 333), so new_label is empty and both guarded blocks at lines 484–488 and 492–500 (if [[ -n "$new_label" ]]) are skipped entirely. The fall-through is harmless but the comment misrepresents the intent; it matches the blocked case only because blocked gets a non-empty label. Update the comment to reflect the actual behaviour.

✏️ Proposed fix
-		# Don't return here - let the non-terminal state logic handle label updates
+		# Return early — failed is a terminal state; label and comment already applied above.
+		return 0

Or, if intentional fall-through is desired for future flexibility, correct the comment:

-		# Don't return here - let the non-terminal state logic handle label updates
+		# Fall through intentionally (new_label is empty for failed; no further label is applied)
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
# Don't return here - let the non-terminal state logic handle label updates
;;
# Return early — failed is a terminal state; label and comment already applied above.
return 0
;;
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In @.agents/scripts/supervisor/issue-sync.sh around lines 467 - 468, The comment
on the fall-through after the "failed" case is misleading because
state_to_status_label("failed") returns an empty string so the subsequent
non-terminal label logic is a no-op; update the comment near the fall-through to
state that the fall-through is intentional to let the "blocked" branch (which
yields a non-empty label) reuse the label-update logic and that "failed"
produces no label, or alternatively change the comment to say "fall-through to
shared label handling; no-op for failed (empty label)"; reference
state_to_status_label, the "failed" and "blocked" cases, and the guarded blocks
that check if [[ -n "$new_label" ]] to guide where to change the text.

marcusquinn added a commit that referenced this pull request Feb 21, 2026
Three bugs fixed in sync_issue_status_label():

1. Failed tasks were auto-closed — now they post a failure comment
   and add needs-review label, keeping the issue OPEN for human triage.

2. Verified/deployed tasks were auto-closed even without a merged PR —
   now they verify the PR state is MERGED before closing. If no merged
   PR exists, the issue is flagged for human review instead.

3. needs-review label was not in ensure_status_labels() or
   ALL_STATUS_LABELS — now it is created on-demand and cleaned up
   on subsequent state transitions.

Reopened 5 incorrectly auto-closed issues: #2106, #1998, #1764, #1763, #1762.

Supersedes PR #2112 (which only fixed the failed-state bug).
@marcusquinn
Copy link
Owner Author

Superseded by PR #2115 which includes this fix plus two additional fixes:

  1. Verified/deployed tasks now require merged PR evidence to close (not just failed tasks)
  2. needs-review label added to ensure_status_labels() and ALL_STATUS_LABELS for proper lifecycle management

marcusquinn added a commit that referenced this pull request Feb 21, 2026
* fix: never auto-close issues without merged PR evidence

Three bugs fixed in sync_issue_status_label():

1. Failed tasks were auto-closed — now they post a failure comment
   and add needs-review label, keeping the issue OPEN for human triage.

2. Verified/deployed tasks were auto-closed even without a merged PR —
   now they verify the PR state is MERGED before closing. If no merged
   PR exists, the issue is flagged for human review instead.

3. needs-review label was not in ensure_status_labels() or
   ALL_STATUS_LABELS — now it is created on-demand and cleaned up
   on subsequent state transitions.

Reopened 5 incorrectly auto-closed issues: #2106, #1998, #1764, #1763, #1762.

Supersedes PR #2112 (which only fixed the failed-state bug).

* fix: address review feedback — grep false positive and failed-state reopen

1. Replace grep -qi 'MERGED' with dedicated gh pr view --json state
   query. The old pattern matched the field label 'merged:' in the jq
   output, causing ALL PRs to be treated as merged (false positive).

2. Add explicit reopen logic for failed state. The fall-through was a
   no-op because state_to_status_label('failed') returns empty, so
   the reopen block gated on new_label never fired. Now explicitly
   checks if issue is closed and reopens it for human review.

* fix: remove 2>/dev/null from gh pr view calls to surface auth/API errors

Addresses Gemini review feedback: blanket stderr suppression hides
authentication failures, API rate limits, and network issues. The
|| echo "" fallback still handles expected errors (non-existent PR).

* fix: needs-review label was simultaneously added and removed

CodeRabbit found that needs-review in ALL_STATUS_LABELS caused the
remove_args loop to include --remove-label needs-review, negating the
--add-label needs-review in the same gh issue edit call.

Fix: remove needs-review from ALL_STATUS_LABELS (it's a triage flag,
not a status label). Also merge redundant dual gh pr view calls into
one API request, and use distinct label color (E99695) to avoid
duplicating status:claimed color.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

t1300: Investigate t1165.1 repeated permission_denied_write_operations failures

1 participant