fix(agents): exempt closed issues from the format guard - #2997
Conversation
The guard fetched `state` in its `gh issue view --json` call and never read it, so a closed issue was still validated, commented on, and routed to the optimizer. The optimizer's rewrite re-fired the guard, and the pair looped against work that was already delivered. Fine-Art-Archive#464 was closed at 05:08Z and kept formatting until 22:44Z — 17.5 hours and several hundred workflow runs after close. Read the field that was already being fetched, in both the root and consumer template copies. The check is case-insensitive because `gh` returns CLOSED while the REST API returns closed. Tests execute the real "Resolve issue" step lifted from the workflow YAML against a fixture payload, with `gh` stubbed, and assert exempt=true for a closed issue while an otherwise identical open issue is still processed. Closes part of #2991. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Bugbot is not enabled for your account, so this pull request was not reviewed. Enable Bugbot in the Cursor dashboard to get automatic reviews on future PRs. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 45 minutes Your organization has reached its usage spending cap. Adjust your spending cap in the billing tab. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe issue-format guard now exempts closed issues in both workflow copies. Tests verify closed and open issue behavior, case-insensitive state handling, and consistency between the workflows. ChangesIssue Format Guard
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Workflow source neededPR #2997 needs either a linked GitHub issue or one valid non-issue Workflow Source before PR metadata automation can manage it safely. Please do one of:
Once a valid source is present, this warning will not be reposted. |
There was a problem hiding this comment.
Pull request overview
Exempts closed GitHub issues from the agents-issue-format-guard workflow so the guard doesn’t validate/comment/dispatch formatting work on already-closed issues (preventing the format-guard ↔ optimizer loop seen in Fine-Art-Archive#464). Adds a focused test that executes the real “Resolve issue” step script from both the root workflow and the consumer template copy.
Changes:
- Add a closed-issue exemption in the guard’s “Resolve issue” step (root + consumer template).
- Add an integration-style pytest that runs the extracted step script with
ghstubbed and asserts on$GITHUB_OUTPUT. - Add a drift-oriented assertion to help ensure both workflow copies carry the closed-state logic.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
tests/workflows/test_issue_format_guard_closed_issues.py |
New tests executing the real “Resolve issue” step for both workflow copies and asserting closed issues set exempt=true. |
templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml |
Exempt closed issues early in the guard to prevent routing/validation loops in consumer repos. |
.github/workflows/agents-issue-format-guard.yml |
Mirror the same closed-issue exemption in the root workflow copy. |
Suppressed comments (1)
tests/workflows/test_issue_format_guard_closed_issues.py:59
- Building PATH via
shutil.os.environ['PATH']can raise a KeyError in environments where PATH is unset, and it still depends onshutil.os. Preferos.environ.get(...)after importingos.
"PATH": f"{bin_dir}:{shutil.os.environ['PATH']}",
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| import shutil | ||
| import subprocess | ||
| from pathlib import Path |
| for path in GUARD_PATHS: | ||
| assert "ascii_downcase" in _resolve_step_script(path), f"{path} lacks the closed-state check" |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5a69951a43
ℹ️ 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".
| # produces a body edit, that edit re-fires this guard, and the pair loops | ||
| # against work that is already delivered — see Fine-Art-Archive#464, which | ||
| # kept formatting for 17.5 hours after it was closed. | ||
| if [[ "$(jq -r '.state // "" | ascii_downcase' issue.json)" == "closed" ]]; then |
There was a problem hiding this comment.
Recheck state before mutating or dispatching
When an issue closes after the Resolve issue snapshot but before later steps execute, this exemption remains false for the run. The stale-format step at lines 120-123 can then remove agents:formatted, and although the routing step deliberately refreshes live.json at lines 138-140, it checks only hold labels before dispatching the optimizer. Recheck the live state at both side-effect boundaries so a run already in progress cannot continue modifying or routing an issue after closure.
Useful? React with 👍 / 👎.
| jq -r '.body // ""' issue.json > body.md | ||
| exempt=false | ||
| held=false | ||
| # A closed issue has nothing left to format. Routing one to the optimizer |
There was a problem hiding this comment.
Update the documented exemption contract
This changes the consumer-facing guard contract, but docs/ci/WORKFLOWS.md:157 still says the guard validates manual dispatch and lists only durable/wontfix and bot issues as exempt, while .github/sync-manifest.yml:95 retains the same incomplete description. Document the closed-issue exemption alongside the synchronized workflow change so operators and consumers see the behavior the template now implements.
AGENTS.md reference: AGENTS.md:L62-L65
Useful? React with 👍 / 👎.
Automated Status SummaryHead SHA: 24a4bcd
Coverage Overview
Coverage Trend
Top Coverage Hotspots (lowest coverage)
Low Coverage Files (<50.0%)
Updated automatically; will refresh on subsequent CI/Docker completions. Keepalive checklistScopeNo scope information available Tasks
Acceptance criteria
|
|
Runner dispatch state for autofix on PR #2997. Do not edit. |
|
Runner dispatch state for codex on PR #2997. Do not edit. |
|
Autofix updated these files:
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/agents-issue-format-guard.yml:
- Around line 64-71: Update the route step in both
.github/workflows/agents-issue-format-guard.yml lines 64-71 and
templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml lines
64-71 to check the re-fetched live.json state after the live issue fetch; when
live.json.state is closed, set the existing exemption and stop routing before
labeling, dispatching, or commenting. Keep both workflow copies synchronized.
In `@templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml`:
- Around line 64-71: Update the optimizer dispatch route to re-check the freshly
fetched live issue state from live.json immediately before dispatch, not only
the initial issue.json state. When the live state is closed, skip optimizer
routing and mark the issue exempt using the existing control flow.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 3c7107dc-b8dc-43b8-a834-2cc35d5b06d4
📒 Files selected for processing (3)
.github/workflows/agents-issue-format-guard.ymltemplates/consumer-repo/.github/workflows/agents-issue-format-guard.ymltests/workflows/test_issue_format_guard_closed_issues.py
| # A closed issue has nothing left to format. Routing one to the optimizer | ||
| # produces a body edit, that edit re-fires this guard, and the pair loops | ||
| # against work that is already delivered — see Fine-Art-Archive#464, which | ||
| # kept formatting for 17.5 hours after it was closed. | ||
| if [[ "$(jq -r '.state // "" | ascii_downcase' issue.json)" == "closed" ]]; then | ||
| echo "Issue is closed — nothing to format." | ||
| exempt=true | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Check the re-fetched issue state before dispatch.
Both workflows only check the state in issue.json. If the issue closes before the route step fetches live.json, the workflow can still label, dispatch, and comment on the closed issue.
.github/workflows/agents-issue-format-guard.yml#L64-L71: stop the route step whenlive.json.stateis closed.templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml#L64-L71: apply the same route-step guard through the workflow sync source.
Proposed guard after the live.json fetch
gh issue view "$NUMBER" --repo "$GITHUB_REPOSITORY" \
--json number,body,labels,state,author > live.json
+ if [[ "$(jq -r '.state // "" | ascii_downcase' live.json)" == "closed" ]]; then
+ echo "Issue is now closed — skipping optimizer dispatch."
+ exit 0
+ fi
jq -r '.body // ""' live.json > body.md📍 Affects 2 files
.github/workflows/agents-issue-format-guard.yml#L64-L71(this comment)templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml#L64-L71
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/agents-issue-format-guard.yml around lines 64 - 71, Update
the route step in both .github/workflows/agents-issue-format-guard.yml lines
64-71 and
templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml lines
64-71 to check the re-fetched live.json state after the live issue fetch; when
live.json.state is closed, set the existing exemption and stop routing before
labeling, dispatching, or commenting. Keep both workflow copies synchronized.
| # A closed issue has nothing left to format. Routing one to the optimizer | ||
| # produces a body edit, that edit re-fires this guard, and the pair loops | ||
| # against work that is already delivered — see Fine-Art-Archive#464, which | ||
| # kept formatting for 17.5 hours after it was closed. | ||
| if [[ "$(jq -r '.state // "" | ascii_downcase' issue.json)" == "closed" ]]; then | ||
| echo "Issue is closed — nothing to format." | ||
| exempt=true | ||
| fi |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Re-check the live issue state before optimizer dispatch.
The initial closed-state check does not protect a run when the issue closes before the route step re-fetches live.json. That route can still dispatch the optimizer for a closed issue.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml`
around lines 64 - 71, Update the optimizer dispatch route to re-check the
freshly fetched live issue state from live.json immediately before dispatch, not
only the initial issue.json state. When the live state is closed, skip optimizer
routing and mark the issue exempt using the existing control flow.
Addresses task 1 of #2991 (the closed-issue exemption). The attempt cap from that issue is not in this PR.
Problem
The guard's
Resolve issuestep fetchesstate:...and then never reads it.
exemptandheldwere derived from labels, author and body text only. A closed issue was therefore still validated, commented on, and routed to the optimizer — whose rewrite re-fired the guard, looping the pair against work that was already delivered.stranske/Fine-Art-Archive#464was closed at2026-08-08T05:08:02Zand kept formatting until22:44Z: 17.5 hours and several hundred workflow runs after close, against a PR that had already merged.Change
Read the field that was already being fetched, in both the root guard and the consumer template copy (the latter is what the 13 consumer repos run):
exemptshort-circuits before validation, so a closed issue produces no validation, no comment, and no dispatch. The match is case-insensitive becauseghreturnsCLOSEDwhile the REST API returnsclosed.Tests
tests/workflows/test_issue_format_guard_closed_issues.pylifts the realResolve issuestep out of the workflow YAML and executes it against a fixture payload withghstubbed, asserting on the actual$GITHUB_OUTPUT— rather than asserting on workflow text. Both guard copies are parameterised.test_closed_issue_is_exempt— the [Follow-up] Unmet criteria from PR #463 #464 shape setsexempt=true.test_open_issue_is_still_processed— an otherwise identical open issue staysexempt=false, so the exemption is narrow.test_closed_state_is_matched_case_insensitively—CLOSEDandclosedboth exempt.test_both_guard_copies_carry_the_closed_check— root and template cannot drift on this.Deliberate-break demonstration: removing the check from the root guard fails
test_closed_issue_is_exempt[.github/workflows/agents-issue-format-guard.yml],test_closed_state_is_matched_case_insensitively[...]andtest_both_guard_copies_carry_the_closed_check(3 failed, 4 passed) — the exact production symptom. Reverted, 7 passed.Also verified:
check_template_drift.pyreportsunallowlisted drift: 0, andtests/workflows/test_agents_issue_optimizer_format_trigger.py+tests/scripts/test_issue_format.pystill pass (40 passed).🤖 Generated with Claude Code
Summary by CodeRabbit
Bug Fixes
Tests