fix(issue-format): handle nested fences and format retry - #3036
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. |
Workflow source neededPR #3036 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. |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 2 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 PR updates both issue-format validator copies to recognize indented fences and lowercase backticked targets. It also updates format-guard and optimizer workflows to revalidate issues after ChangesIssue format validation and workflow triggering
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
Suggested labels: Sequence Diagram(s)sequenceDiagram
participant IssueOptimizer
participant GitHub
participant FormatGuard
IssueOptimizer->>GitHub: Remove agents:format label
IssueOptimizer->>FormatGuard: Dispatch workflow with issue number
FormatGuard->>GitHub: Revalidate the issue
🚥 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: ab4142b483
ℹ️ 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".
Automated Status SummaryHead SHA: f26fac0
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
|
Restrict the format guard to agents:format unlabeled events so labeled applications do not race the optimizer, and explicitly dispatch the guard after a failed optimizer releases the format lease because GITHUB_TOKEN label edits do not emit downstream issue workflows. Co-authored-by: Cursor <cursoragent@cursor.com>
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/scripts/issue_format.py (1)
171-180: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy liftPreserve fence indentation in both validator copies.
Both
_headingsand_without_fenced_codecan close an outer fence at a deeper nested marker because the fence state has no indentation. This can expose commands or headings that remain inside a code example.
.github/scripts/issue_format.py#L171-L180: Track opening indentation when_headingsmatches a closing fence..github/scripts/issue_format.py#L217-L226: Apply the same indentation-aware state to_without_fenced_code.templates/consumer-repo/.github/scripts/issue_format.py#L171-L180: Keep the consumer_headingsimplementation consistent with the primary validator.templates/consumer-repo/.github/scripts/issue_format.py#L217-L226: Keep the consumer_without_fenced_codeimplementation consistent with the primary validator.🤖 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/scripts/issue_format.py around lines 171 - 180, Make fence tracking indentation-aware in both _headings and _without_fenced_code, storing the opening fence’s indentation and requiring a closing marker to match the appropriate indentation before clearing the state. Apply the same changes consistently at .github/scripts/issue_format.py lines 171-180 and 217-226, and templates/consumer-repo/.github/scripts/issue_format.py lines 171-180 and 217-226.
🤖 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 `@tests/scripts/test_issue_format.py`:
- Around line 43-60: Add regression coverage in
test_list_indented_fenced_acceptance_command_is_ignored for indented tilde
fences, parameterized across both validator_path values, and assert they remain
ignored. Add a negative validation case using a function-category task with a
backticked file target to exercise the exclusion near the parser branch at line
139, asserting the expected report outcome.
---
Outside diff comments:
In @.github/scripts/issue_format.py:
- Around line 171-180: Make fence tracking indentation-aware in both _headings
and _without_fenced_code, storing the opening fence’s indentation and requiring
a closing marker to match the appropriate indentation before clearing the state.
Apply the same changes consistently at .github/scripts/issue_format.py lines
171-180 and 217-226, and templates/consumer-repo/.github/scripts/issue_format.py
lines 171-180 and 217-226.
🪄 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: e5dc9075-577e-44d6-908e-04a5a4d5dd21
📒 Files selected for processing (6)
.github/scripts/issue_format.py.github/workflows/agents-issue-format-guard.ymltemplates/consumer-repo/.github/scripts/issue_format.pytemplates/consumer-repo/.github/workflows/agents-issue-format-guard.ymltests/scripts/test_issue_format.pytests/workflows/test_agents_issue_optimizer_format_trigger.py
|
Runner dispatch state for autofix on PR #3036. Do not edit. |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/workflows/test_agents_issue_optimizer_format_trigger.py (1)
90-101: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winTest the event condition semantically.
The
labeled_formatcheck compares string positions. It does not evaluate the Boolean expression. A future(labeled && agents:format)clause placed after the firstagents:formatoccurrence can still pass this test. Assert the specific format clause, or evaluate representativelabeledandunlabeledevents for both workflows.As per path instructions,
**/*.pyrequires prioritizing correctness and test coverage for changed behavior.🤖 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 `@tests/workflows/test_agents_issue_optimizer_format_trigger.py` around lines 90 - 101, Strengthen test_format_guard_rechecks_after_format_lease_release by validating the parsed condition semantically rather than relying on string positions. Assert that labeled agents:format events do not satisfy the guard while unlabeled agents:format events do, for both GUARD_PATH and CONSUMER_GUARD_PATH, using the existing workflow-condition evaluation approach or an equivalent targeted clause assertion.Source: Path instructions
🤖 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-optimizer.yml:
- Around line 747-750: Add actions: write to the optimize_issue.permissions
configuration in both .github/workflows/agents-issue-optimizer.yml (lines
747-750) and
templates/consumer-repo/.github/workflows/agents-issue-optimizer.yml (lines
600-603) so both retry workflow dispatches can execute; also ensure the consumer
App installation grants Actions: write, since job permissions cannot elevate its
token.
In `@tests/workflows/test_agents_issue_optimizer_format_trigger.py`:
- Around line 104-112: Strengthen
test_format_optimizer_dispatches_guard_after_failed_lease_release by asserting,
for both workflow texts, that the lease-removal command appears before the gh
workflow run agents-issue-format-guard.yml dispatch and that the dispatch
includes the exact || echo "::warning::could not dispatch format guard retry
after lease release" fallback. Keep the existing command-presence checks.
---
Outside diff comments:
In `@tests/workflows/test_agents_issue_optimizer_format_trigger.py`:
- Around line 90-101: Strengthen
test_format_guard_rechecks_after_format_lease_release by validating the parsed
condition semantically rather than relying on string positions. Assert that
labeled agents:format events do not satisfy the guard while unlabeled
agents:format events do, for both GUARD_PATH and CONSUMER_GUARD_PATH, using the
existing workflow-condition evaluation approach or an equivalent targeted clause
assertion.
🪄 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: 8121f6f2-8719-4415-bba1-c4163460ab50
📒 Files selected for processing (5)
.github/workflows/agents-issue-format-guard.yml.github/workflows/agents-issue-optimizer.ymltemplates/consumer-repo/.github/workflows/agents-issue-format-guard.ymltemplates/consumer-repo/.github/workflows/agents-issue-optimizer.ymltests/workflows/test_agents_issue_optimizer_format_trigger.py
Add actions:write to optimize_issue job permissions in root and consumer optimizer workflows, extend issue-format regression tests per review, and rebaseline the optimizer template-drift allowlist fingerprints. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
tests/workflows/test_agents_issue_optimizer_format_trigger.py (1)
104-118: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winAdd coverage for failed lease cleanup.
These assertions prove ordering only. They pass even when lease removal fails and the dispatch still runs. Add a regression assertion that failed removal prevents dispatch, while an already-absent label is handled explicitly.
As per path instructions,
**/*.pyrequires prioritizing correctness, error handling, and test coverage.🤖 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 `@tests/workflows/test_agents_issue_optimizer_format_trigger.py` around lines 104 - 118, Add regression coverage in test_format_optimizer_dispatches_guard_after_failed_lease_release for the lease-cleanup shell flow: assert that dispatch is skipped when removing the agents:format label fails, while an already-absent label is handled explicitly and still permits the intended retry behavior. Keep the existing ordering and dispatch-argument assertions, and apply the checks to both workflow files.Source: Path instructions
.github/workflows/agents-issue-optimizer.yml (1)
748-751: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winPrevent retry dispatch before lease release.
Both optimizer workflows continue to dispatch after lease removal fails. The test covers ordering only, so it does not detect this race.
.github/workflows/agents-issue-optimizer.yml#L748-L751: Dispatch only after successful removal or verified label absence.templates/consumer-repo/.github/workflows/agents-issue-optimizer.yml#L601-L604: Apply the same lease-state check.tests/workflows/test_agents_issue_optimizer_format_trigger.py#L104-L118: Add a failure-path assertion that failed removal prevents dispatch.🤖 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-optimizer.yml around lines 748 - 751, Prevent format-guard retry dispatch until the lease is confirmed released: update the cleanup/dispatch flow in .github/workflows/agents-issue-optimizer.yml (748-751) and templates/consumer-repo/.github/workflows/agents-issue-optimizer.yml (601-604) to dispatch only after successful label removal or verified lease-label absence; otherwise skip dispatch. Extend tests/workflows/test_agents_issue_optimizer_format_trigger.py (104-118) with a failure-path assertion confirming failed removal does not dispatch the workflow.Source: Path instructions
🤖 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.
Outside diff comments:
In @.github/workflows/agents-issue-optimizer.yml:
- Around line 748-751: Prevent format-guard retry dispatch until the lease is
confirmed released: update the cleanup/dispatch flow in
.github/workflows/agents-issue-optimizer.yml (748-751) and
templates/consumer-repo/.github/workflows/agents-issue-optimizer.yml (601-604)
to dispatch only after successful label removal or verified lease-label absence;
otherwise skip dispatch. Extend
tests/workflows/test_agents_issue_optimizer_format_trigger.py (104-118) with a
failure-path assertion confirming failed removal does not dispatch the workflow.
In `@tests/workflows/test_agents_issue_optimizer_format_trigger.py`:
- Around line 104-118: Add regression coverage in
test_format_optimizer_dispatches_guard_after_failed_lease_release for the
lease-cleanup shell flow: assert that dispatch is skipped when removing the
agents:format label fails, while an already-absent label is handled explicitly
and still permits the intended retry behavior. Keep the existing ordering and
dispatch-argument assertions, and apply the checks to both workflow files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: 9957ef47-6d34-4ad5-b42f-4b070955c8b3
📒 Files selected for processing (5)
.github/workflows/agents-issue-optimizer.ymlconfig/template-drift-allowlist.txttemplates/consumer-repo/.github/workflows/agents-issue-optimizer.ymltests/scripts/test_issue_format.pytests/workflows/test_agents_issue_optimizer_format_trigger.py
|
Runner dispatch state for codex on PR #3036. Do not edit. |
|
Autofix updated these files:
|
Summary
agents:formatafter failure.Validation
uv run --isolated --with pytest --with pyyaml python -m pytest -q tests/scripts/test_issue_format.py tests/workflows/test_agents_issue_optimizer_format_trigger.py(98 passed)Source fix for active review threads on Portable-Alpha-Extension-Model#2217 and Trend_Model_Project#5807; corrected consumer sync must replace those PRs before merge.
Summary by CodeRabbit
Bug Fixes
agents:formatlabel is added or removed, including after failed processing.Tests