feat: implement issue #1077 — dev-lead: issues closed as "completed" without the fix landing (premature closure) - #1080
Conversation
…without the fix landing (premature closure)
|
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: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds a scheduled ChangesPremature-closure detection and audit
Estimated code review effort: 3 (Moderate) | ~30 minutes Sequence Diagram(s)sequenceDiagram
participant Workflow as Scheduled Workflow
participant AuditScript as premature-closure-audit.sh
participant GitHubAPI as GitHub API
participant DetectLib as premature-closure-detect.sh
Workflow->>AuditScript: trigger (cron / dispatch, DRY_RUN, REPO, thresholds)
AuditScript->>GitHubAPI: list issues closed since cutoff (state_reason=completed)
loop each candidate issue
AuditScript->>GitHubAPI: fetch issue timeline
GitHubAPI-->>AuditScript: closing/merged PR status
AuditScript->>DetectLib: minutes_between(created, closed)
AuditScript->>DetectLib: premature_closure_reasons(state_reason, minutes, merged_pr)
DetectLib-->>AuditScript: reason or empty
alt live mode and reason present
AuditScript->>GitHubAPI: reopen issue, add label, post comment
else dry-run
AuditScript->>AuditScript: log notice only
end
end
AuditScript->>DetectLib: generate_premature_closure_report(candidates)
AuditScript->>Workflow: write GITHUB_STEP_SUMMARY, export GITHUB_ENV vars
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 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 |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1080 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
There was a problem hiding this comment.
Pull request overview
Adds a repo-local “premature closure” backstop to detect and (optionally) re-open issues that were closed as completed without a merged PR landing the fix, addressing dev-lead’s premature-closure behavior reported in #1077.
Changes:
- Introduces a pure detection library (
premature_closure_reasons,minutes_between, report renderer) with Bats coverage. - Adds a
premature-closure-audit.shwrapper that scans recently closed completed issues, generates a report, and (in live mode) re-opens + labels + comments idempotently. - Adds a scheduled/dispatchable workflow to run the audit and wires the new unit test into
lint.yml; documents the workflow exception inAGENTS.md.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_premature_closure_detect.bats | Unit tests pinning the detector’s boundary conditions and report rendering. |
| scripts/lib/premature-closure-detect.sh | New pure-function library for identifying premature-closure candidates and rendering report sections. |
| scripts/premature-closure-audit.sh | New audit wrapper that queries GitHub, applies detection, emits a report, and optionally re-opens issues. |
| .github/workflows/premature-closure-audit.yml | New scheduled + dispatch workflow to run the audit with a LIVE_MODE/DRY_RUN gate. |
| .github/workflows/lint.yml | Adds the new Bats test to the lint workflow’s test list. |
| AGENTS.md | Documents the new repo-specific workflow exception and its staging semantics. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a premature-closure detection and audit system to prevent tracking issues from being prematurely closed as completed without a merged closing PR. It includes a detection library, an audit script, and comprehensive BATS unit tests. The review feedback focuses on improving the shell scripts' performance and robustness, specifically by avoiding subshells and external commands (like tr) inside loops, ensuring the portability of the date command across Linux and macOS, avoiding the fragile cmd1 && cmd2 || cmd3 pattern, and using $BATS_TEST_TMPDIR for cleaner test execution.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/premature-closure-audit.yml:
- Around line 45-46: Set persist-credentials to false on the Checkout agent repo
step using actions/checkout so the GH_TOKEN is not written into local git
config. Update the checkout configuration in the workflow job that runs the
gh-based script, and keep using the existing GH_TOKEN env for auth instead of
persisted git credentials.
In `@scripts/lib/premature-closure-detect.sh`:
- Around line 1-2: The sourced library currently enables strict mode with set
-euo pipefail, which leaks shell option changes into callers. Remove strict mode
from premature-closure-detect.sh so it stays side-effect free, and move the
strict-mode setup into the executable entrypoints that source it, keeping the
library functions unchanged.
- Around line 141-147: The markdown table in the report rendering loop writes
the issue `title` and `reason` directly, which can break formatting if they
contain table-reserved characters or newlines. Update the `while IFS=$'\t' read
-r num url title reason` block in `premature-closure-detect.sh` to normalize or
escape those fields before the final `printf`, so the `#%s` row output stays
valid for any issue data.
In `@scripts/premature-closure-audit.sh`:
- Around line 105-108: The already_flagged helper can fail open because the gh
api pagination output is piped directly into grep, which can be treated as a
pipeline failure under pipefail and miss an existing COMMENT_MARKER. Update
already_flagged to first capture the issue comment bodies from gh api into a
variable or temporary file, then run grep -qF against that buffered content so
the match check in scripts/premature-closure-audit.sh is reliable.
🪄 Autofix (Beta)
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: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: c34fc555-ca3a-4047-b7d8-a9282be5df09
📒 Files selected for processing (6)
.github/workflows/lint.yml.github/workflows/premature-closure-audit.ymlAGENTS.mdscripts/lib/premature-closure-detect.shscripts/premature-closure-audit.shtests/test_premature_closure_detect.bats
|
@coderabbitai resolve |
Dev-Lead — fix-reviews (applied)Changes committed and pushed. |
✅ Action performedComments resolved and changes approved. |
Dev-Lead — waiting on PR blockers (intent: review-changes)PR: #1080 |
|
Note @don-petry I reviewed this PR and no code changes were needed, but it still has blocking checks or reviews (failing or cancelled checks, or changes-requested reviews), so I cannot mark it done yet. I'll re-check automatically. |
Dev-Lead — review-changes (applied)Changes committed and pushed. |
|
Dev-Lead — review-changes (no-changes)No changes were needed for this PR. |
donpetry-bot
left a comment
donpetry-bot
left a comment
There was a problem hiding this comment.
Automated review — APPROVED ✓
Risk: MEDIUM
Reviewed commit: 645a598ee31718c84b14692612f46e6375dacee2
Review mode: triage-approved (single reviewer)
Summary
Adds a staged, weekly premature-closure audit (issue #1077 backstop): a pure detection library (scripts/lib/premature-closure-detect.sh), an orchestration script (scripts/premature-closure-audit.sh), a new workflow with read-only permissions and a LIVE_MODE gate, 20 bats tests wired into lint.yml, and an AGENTS.md exception entry. 623 additions, 0 deletions, 6 files.
Linked issue analysis
Closes #1077 (dev-lead closing issues as completed without a merged fix). The PR substantively implements the issue's recommended actions #3 (machine-checkable closure post-condition: flags issues closed as completed within PC_MAX_OPEN_MINUTES of opening with no merged closing PR) and #2 (in live mode, re-opens and labels dev-lead:needs-human so attempted-but-unresolved issues stay visibly open). Detection is fail-safe: an undeterminable merged-PR lookup suppresses the flag, and an idempotent comment marker prevents repeat re-opens. Action #1 (gating closure at the dev-lead source) is enforced operationally by this backstop rather than by changing the agent itself, which matches the issue's staged-backstop framing.
Findings
No blocking findings.
- Workflow hygiene is solid: checkout pinned to a full SHA (v6.0.3), persist-credentials: false, least-privilege permissions (issues: read), concurrency group, 30-min timeout, and a staged rollout where the LIVE_MODE repo variable is the authoritative gate (dispatch input cannot bypass it; default is dry-run).
- Prior bot-review findings were all addressed and their threads resolved: grep-under-set-e in already_flagged, GNU/BSD date portability fallbacks, markdown table-cell escaping in the report, persist-credentials, and strict-mode moved out of the sourced library.
- Detection library is defensive: non-numeric PC_MAX_OPEN_MINUTES falls back to the default instead of zeroing the window; missing/garbage inputs degrade without crashing; all boundaries (strict <, case-insensitivity, truthiness) are pinned by 20 bats tests now run in lint.yml.
- Minor (non-blocking): the workflow permissions block gates only the default GITHUB_TOKEN while the script authenticates with DON_PETRY_BOT_GH_PAT, so the 'live mode requires issues: write' staging note is slightly misleading — the real gate is LIVE_MODE. Worth a doc tweak someday, not a defect.
- Secret scan: the run_secret_scanning MCP tool was not available in this run; the gitleaks CI check is green and the diff introduces no credential material (secrets are referenced via the Actions secrets context only).
CI status
All required checks green at 645a598: shellcheck, bats, unit-tests, Lint, CodeQL (actions + python), gitleaks secret scan, SonarCloud, agent-shield, Agent Security Scan, holdout-guard, template-drift, validate-agent-profiles, permissions/stub-structure checks, CodeRabbit. CodeRabbit's earlier CHANGES_REQUESTED review was dismissed after fixes; all 12 review threads are resolved.
Reviewed automatically by the PR-review agent (single-reviewer mode: fable 5). Reply if you need a human review.
Closes #1077
Implemented by dev-lead agent. Please review.
Summary by CodeRabbit
New Features
Documentation