Tracking issue for deferred AI-reviewer feedback on the four PR-helper scripts, merged as the initial Ruby versions so the next batch can use them:
All four were merged with verified behavior (unit tests + real-PR equivalence checks, rubocop clean, CI green). The CHANGES_REQUESTED reviews were all from advisory AI reviewers (CodeRabbit, Cursor Bugbot, Codex, Greptile); none was a confirmed blocker on the verified happy paths. Many of their findings are stale (they reference the earlier bash line numbers from before the Ruby rewrite). The legitimate, still-applicable items to handle in a follow-up batch:
Cross-cutting (all four helpers)
- Tighten PR-number validation to reject
0 (contract says "positive integer"; \A\d+\z currently accepts 0).
- Tighten
--repo validation: include?("/") accepts extra path segments (a/b/c) and empty owner/name; enforce exact OWNER/REPO.
- Consider capturing stderr (currently discarded via
err: File::NULL / 2>/dev/null) for more actionable diagnostics on failure.
pr-file-touch-map (#4065)
- Reachable-SHA (
headRefOid) head fetch is skipped when the head branch name fails validation. The OID fetch needs no valid branch name, so make it independent of valid_branch?(head_ref) (cursor "High", coderabbit "Major").
- Files API parser only treats
status == "renamed" as owning both paths; handle status == "copied" (with previous_filename) too (cursor "Medium").
- On Files-API row-count vs
changedFiles mismatch, the skill prose says to re-read changedFiles once before recording UNKNOWN; the helper goes straight to UNKNOWN. Behavior is safe (UNKNOWN => serial) but stricter than documented.
pr-ci-readiness (#4067)
- When
gh pr checks --required returns only cancelled/superseded rows (no active rows), fall back to the full gh pr checks list instead of treating it as the authoritative empty set (cursor "Medium", coderabbit "Major").
changelog-merged-prs (#4066)
- O(n)
gh api .../commits/SHA/pulls subprocess spawns for commits lacking an inline PR number; batch or cache for large ranges.
-- does not currently terminate option parsing (it is a no-op rather than a "rest are positional" marker).
Each PR's review threads have the full detail. Suggested approach: one batch lane per helper, verifying each finding against the current Ruby code (skip stale ones) before fixing.
Tracking issue for deferred AI-reviewer feedback on the four PR-helper scripts, merged as the initial Ruby versions so the next batch can use them:
All four were merged with verified behavior (unit tests + real-PR equivalence checks, rubocop clean, CI green). The
CHANGES_REQUESTEDreviews were all from advisory AI reviewers (CodeRabbit, Cursor Bugbot, Codex, Greptile); none was a confirmed blocker on the verified happy paths. Many of their findings are stale (they reference the earlier bash line numbers from before the Ruby rewrite). The legitimate, still-applicable items to handle in a follow-up batch:Cross-cutting (all four helpers)
0(contract says "positive integer";\A\d+\zcurrently accepts0).--repovalidation:include?("/")accepts extra path segments (a/b/c) and empty owner/name; enforce exactOWNER/REPO.err: File::NULL/2>/dev/null) for more actionable diagnostics on failure.pr-file-touch-map (#4065)
headRefOid) head fetch is skipped when the head branch name fails validation. The OID fetch needs no valid branch name, so make it independent ofvalid_branch?(head_ref)(cursor "High", coderabbit "Major").status == "renamed"as owning both paths; handlestatus == "copied"(withprevious_filename) too (cursor "Medium").changedFilesmismatch, the skill prose says to re-readchangedFilesonce before recording UNKNOWN; the helper goes straight to UNKNOWN. Behavior is safe (UNKNOWN => serial) but stricter than documented.pr-ci-readiness (#4067)
gh pr checks --requiredreturns only cancelled/superseded rows (no active rows), fall back to the fullgh pr checkslist instead of treating it as the authoritative empty set (cursor "Medium", coderabbit "Major").changelog-merged-prs (#4066)
gh api .../commits/SHA/pullssubprocess spawns for commits lacking an inline PR number; batch or cache for large ranges.--does not currently terminate option parsing (it is a no-op rather than a "rest are positional" marker).Each PR's review threads have the full detail. Suggested approach: one batch lane per helper, verifying each finding against the current Ruby code (skip stale ones) before fixing.