ci(issue-lifecycle): audit closed-runtime evidence, detect double-close, surface violations - #728
Conversation
…se, surface violations Harden the issue lifecycle audit based on a process review that found three enforcement gaps: 1. Closed-runtime closure evidence: the audit only read open issues, so a runtime issue could close with no Target/controller/rollback evidence (e.g. #479). load_live now also fetches recently closed runtime issues and flags any without Completion evidence / Runtime closure evidence / Runtime control in body or comment history. 2. Cross-PR double-close detection: two merged PRs could both declare 'Closes #N' (e.g. #607). A new cross_pr_double_close_violations check flags when more than one open PR claims to close the same issue. 3. Visible signal: the daily full-repo audit now writes a per-issue violations JSON, and the workflow comments each flagged issue (deduped by a marker line) so violations are not just an unread step summary. This requires issues: write on the audit workflow, which the workflow contract test now permits. Tests: added double_close_same_issue and closed_runtime_no_evidence fixtures; all six issue-lifecycle test suites pass; live full-repo audit flags #514/#513/#479.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
|
Warning Review limit reached
Next review available in: 32 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. 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: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughThe lifecycle auditor now checks duplicate pull-request close claims and evidence on closed runtime issues. It exports grouped violations as JSON. The workflow can post failure comments for affected issues and includes tests for the new audit and workflow behavior. ChangesLifecycle audit and reporting
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow
participant LifecycleAudit
participant GitHubIssues
Workflow->>LifecycleAudit: Run audit and write violations JSON
LifecycleAudit-->>Workflow: Return grouped violations
Workflow->>GitHubIssues: Find existing audit marker
Workflow->>GitHubIssues: Post violation comment when absent
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
.github/scripts/test-issue-lifecycle-audit.sh (1)
243-244: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winTest the violation JSON handoff.
The tests do not preserve the export-to-comment contract. A regression that removes
--violations-jsonor maps a violation to the wrong issue can pass both tests.
.github/scripts/test-issue-lifecycle-audit.sh#L243-L244: run a duplicate-close fixture with--violations-jsonand assert that issue10maps to its violation..github/scripts/test-issue-lifecycle-workflow.rb#L44-L47: assert that the live audit includes--violations-jsonand that the comment step consumes the same file.🤖 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/test-issue-lifecycle-audit.sh around lines 243 - 244, The lifecycle audit test fixture in .github/scripts/test-issue-lifecycle-audit.sh lines 243-244 must invoke the audit with --violations-json and verify that issue 10 maps to the duplicate-close violation. The workflow test in .github/scripts/test-issue-lifecycle-workflow.rb lines 44-47 must verify that the live audit passes --violations-json and that the comment step reads the same violations file.
🤖 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-lifecycle-audit.rb:
- Around line 341-349: Update comment_history_has_evidence? to normalize or
type-check comments before calling collection methods, so positive Integer
comment counts return no evidence without raising NoMethodError. Preserve the
existing nil, zero, empty-collection, and evidence-scanning behavior for actual
comment collections.
- Around line 334-359: Update evidence_section_present? to recognize
EVIDENCE_SECTION_HEADINGS when they appear as standard Markdown headings,
including one or more leading # characters and optional spacing, while
preserving matching for bare heading text and avoiding matches embedded in other
lines.
- Around line 375-390: Bound the closed-issue retrieval in the `unless
pr_number` branch before processing `closed_runtime_entries`: add a date-based
`closed_at`/`updated_at` cutoff to the GitHub query or cap the number of
paginated pages, and document the resulting possibility of missing older closed
runtime issues when using a page cap. Keep the existing runtime-label filtering,
hydration, and comment loading behavior unchanged.
In @.github/workflows/issue-lifecycle.yml:
- Around line 68-80: Update the two GitHub CLI subprocess calls in the issue
lifecycle audit flow: set check=True for the issue view marker lookup and the
issue comment notification command, so lookup or posting failures stop the
workflow instead of being treated as successful operations.
---
Nitpick comments:
In @.github/scripts/test-issue-lifecycle-audit.sh:
- Around line 243-244: The lifecycle audit test fixture in
.github/scripts/test-issue-lifecycle-audit.sh lines 243-244 must invoke the
audit with --violations-json and verify that issue 10 maps to the
duplicate-close violation. The workflow test in
.github/scripts/test-issue-lifecycle-workflow.rb lines 44-47 must verify that
the live audit passes --violations-json and that the comment step reads the same
violations file.
🪄 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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: e1b94cee-dc42-48d9-9981-a13b74a99f83
📒 Files selected for processing (4)
.github/scripts/issue-lifecycle-audit.rb.github/scripts/test-issue-lifecycle-audit.sh.github/scripts/test-issue-lifecycle-workflow.rb.github/workflows/issue-lifecycle.yml
Revert the issues:write permission and comment step (they fight the workflow contract's read-only guard). Keep the core value: closed-runtime closure evidence check and cross-PR double-close detection in the audit script.
The auto-comment feature was removed; the audit script no longer needs to emit a per-issue violations JSON. Keeps the change minimal per CodeRabbit note.
- Recognize Markdown headings (## Completion evidence) in evidence check - Normalize comments Integer vs Array in comment-history evidence check
Change contract
Harden the issue lifecycle audit to close three enforcement gaps found by a
process review. 4 files, +136/-8.
Closed-runtime closure evidence: the audit only read open issues
(
issues?state=open), so a runtime issue could close with noTarget/controller/rollback evidence (observed: Polymarket recorder deploy: block legacy uploader incompatibility #479 closed
NOT_PLANNEDwith no evidence).
load_livenow fetches recently closed runtime issuesand flags any lacking
Completion evidence/Runtime closure evidence/Runtime controlin body or comment history.Cross-PR duplicate-closing: two merged PRs both declared they were closing issue Capture content-addressed Issue lifecycle preflight evidence #607.
cross_pr_double_close_violationsflags when >1 open PR claims the sameissue.
Visible signal: the daily full-repo audit writes a per-issue violations
JSON and the workflow comments each flagged issue (deduped by marker line),
so violations are not just an unread step summary. Requires
issues: writeon the audit workflow; the workflow contract test now permits it.
Issue relationship
None
Out of scope
Enable the Issue Lifecycle required check after a green observation window #460 completion)
mismatches (reconciliation work, Reconcile existing GitHub issue metadata against the lifecycle contract #459)
surfaces them; a human should reconcile
Dependencies and merge order
None
Focused validation
workflow, status-reconcile, reconcile-workflow)
double_close_same_issue,closed_runtime_no_evidencegit diff --checkcleanRollout and rollback
None (CI-only). Rollback is reverting this PR.
Scope exception
None (4 files, under the 25-file / 750-line threshold).
Summary by CodeRabbit