fix: harden issue-format guard review follow-ups - #2963
Conversation
|
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: 53 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 (6)
📝 WalkthroughWalkthroughThe issue format validator now enforces recommended sections, preserves nested headings, and accepts verification-prefixed gates. The guard workflows refine event filtering, stale-label removal, optimizer routing, and formatted-label restoration. Tests and CI documentation reflect these rules. ChangesIssue format guard
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant GitHubIssueEvent
participant IssueFormatValidator
participant GuardWorkflow
participant Optimizer
GitHubIssueEvent->>GuardWorkflow: eligible issue or label event
GuardWorkflow->>IssueFormatValidator: validate issue format
IssueFormatValidator-->>GuardWorkflow: conforming or non-conforming result
GuardWorkflow->>GuardWorkflow: remove stale agents:formatted when required
GuardWorkflow->>Optimizer: route non-conforming issue
GuardWorkflow->>GuardWorkflow: restore agents:formatted after successful unheld revalidation
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 256cb3aacd
ℹ️ 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".
| + "); replace with a measurable check." | ||
| ) | ||
| report.ok = not report.missing_required and not report.problems | ||
| report.ok = not report.missing_required and not report.missing_recommended and not report.problems |
There was a problem hiding this comment.
Keep recommended sections advisory
When an issue contains the documented required Tasks and Acceptance Criteria sections but omits Why, Scope, Non-Goals, or the explicitly optional Implementation Notes, this condition now returns exit code 1. The guard consequently marks a contract-conforming issue non-processable and dispatches the optimizer, even though templates/consumer-repo/docs/AGENT_ISSUE_FORMAT.md still classifies all four as recommended rather than required. Either exclude missing_recommended from ok or update the canonical consumer contract and its advisory wording as part of this behavior change.
AGENTS.md reference: AGENTS.md:L60-L65
Useful? React with 👍 / 👎.
…hanges #2962 restored a 22-line rationale docstring to the fleet issue-format validator; this branch had replaced it with a one-line summary and added a shebang. Conflict resolved by keeping both: #2963's shebang and logic, and the docstring. The docstring's description of the contract needed one correction to stay truthful against this branch's behaviour. `Report.ok` now requires `not missing_recommended`, so a missing "recommended" section fails the run and causes the guard to apply `agents:format` -- while the message still calls it an advisory. The docstring now records that tension rather than repeating the old required/recommended split as if it still held. Both copies remain byte-for-byte in lockstep. tests/scripts/test_issue_format.py: 13 passed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Rebased onto
|
Automated Status SummaryHead SHA: 24032cd
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
|
There was a problem hiding this comment.
Pull request overview
This PR tightens the issue-format validation contract and follow-up routing so only fully “ready” issues (including the documented readiness sections) are treated as agent-processable, while also improving guard behavior around nested Markdown headings and label-driven revalidations.
Changes:
- Make missing “recommended/readiness” sections non-conforming (gating
report.ok) and expand accepted verification wording (e.g., “Verify…”). - Update the issue-format-guard workflow to restrict label-event runs to relevant hold/exemption labels and to clear stale
agents:formattedstate when edits invalidate an issue. - Add regression tests covering nested headings, “Implementation Notes” not satisfying “Tasks”, and the expanded gate patterns.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/scripts/test_issue_format.py | Adds/updates regression tests for readiness gating, nested headings, and gate matching. |
| .github/scripts/issue_format.py | Hardens section parsing (heading levels), tightens alias matching, expands gate regex, and gates on readiness sections. |
| templates/consumer-repo/.github/scripts/issue_format.py | Mirrors the validator changes for consumer template sync parity. |
| .github/workflows/agents-issue-format-guard.yml | Restricts label-trigger execution, clears stale formatted state on invalid edits, and safely removes agents:formatted during reroute. |
| templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml | Mirrors the workflow changes for consumer template sync parity. |
| docs/ci/WORKFLOWS.md | Updates documentation for the format-guard workflow’s stricter readiness + nested heading behavior. |
Suppressed comments (2)
.github/scripts/issue_format.py:139
report.oknow treats missingRECOMMENDEDsections as non-conforming, which makesReport.as_markdown()'s "agent-processable with advisories" branch unreachable and also causes the user-facing output to still describe these blocking sections as "Recommended but absent". That’s confusing for issue authors and makes the report text inconsistent with the new gating behavior.
Consider updating Report.as_markdown() (and possibly the missing_recommended naming / wording) so the markdown clearly communicates that these sections are required for agent processing under the new rules, and remove the now-dead advisory path.
+ ", ".join(f"`{section}`" for section in self.missing_required)
templates/consumer-repo/.github/scripts/issue_format.py:139
report.oknow treats missingRECOMMENDEDsections as non-conforming, which makesReport.as_markdown()'s "agent-processable with advisories" branch unreachable and also causes the user-facing output to still describe these blocking sections as "Recommended but absent". That’s confusing for issue authors and makes the report text inconsistent with the new gating behavior.
Consider updating Report.as_markdown() (and possibly the missing_recommended naming / wording) so the markdown clearly communicates that these sections are required for agent processing under the new rules, and remove the now-dead advisory path.
+ ", ".join(f"`{section}`" for section in self.missing_required)
|
Runner dispatch state for codex on PR #2963. Do not edit. |
|
Runner dispatch state for autofix on PR #2963. Do not edit. |
|
Autofix updated these files:
|
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/agents-issue-format-guard.yml (1)
79-91: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winDo not stop optimizer routing when stale-label cleanup fails.
set -emakes a failedgh issue edit --remove-label "agents:formatted"terminate the job. The invalid issue then does not reach the optimizer route. This can occur during an API error or a concurrent label update.
.github/workflows/agents-issue-format-guard.yml#L79-L91: make label removal non-fatal and continue to the routing step.templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml#L79-L91: apply the same failure handling.🤖 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 79 - 91, The stale-label cleanup in the routing workflow is currently fatal when gh issue edit fails. In .github/workflows/agents-issue-format-guard.yml lines 79-91 and templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml lines 79-91, make the agents:formatted removal failure non-fatal so execution always continues to the optimizer routing step while preserving successful cleanup and its message.
🤖 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/scripts/issue_format.py:
- Around line 27-39: Make recommended sections advisory consistently: update the
policy text at .github/scripts/issue_format.py:27-39 and
templates/consumer-repo/.github/scripts/issue_format.py:27-39, exclude
missing_recommended from Report.ok at both files’ lines 185-187, and keep both
validators identical. Extend tests/scripts/test_issue_format.py:77-83 to assert
report.ok while retaining the advisory-message check, and update
docs/ci/WORKFLOWS.md:157 to distinguish required from recommended sections.
---
Outside diff comments:
In @.github/workflows/agents-issue-format-guard.yml:
- Around line 79-91: The stale-label cleanup in the routing workflow is
currently fatal when gh issue edit fails. In
.github/workflows/agents-issue-format-guard.yml lines 79-91 and
templates/consumer-repo/.github/workflows/agents-issue-format-guard.yml lines
79-91, make the agents:formatted removal failure non-fatal so execution always
continues to the optimizer routing step while preserving successful cleanup and
its message.
🪄 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: dc592474-93c4-40fc-a77a-450da5331539
📒 Files selected for processing (6)
.github/scripts/issue_format.py.github/workflows/agents-issue-format-guard.ymldocs/ci/WORKFLOWS.mdtemplates/consumer-repo/.github/scripts/issue_format.pytemplates/consumer-repo/.github/workflows/agents-issue-format-guard.ymltests/scripts/test_issue_format.py
1fd6ed1 to
0718666
Compare
Summary
Validation
python -m pytest tests/scripts/test_issue_format.py tests/scripts/test_validate_template_completeness.py tests/scripts/test_validate_template_sync.py tests/scripts/test_sync_manifest_compiler.py -q(78 passed)Summary by CodeRabbit
Bug Fixes
Documentation
Tests