Skip to content

auto-rebase: unguarded conflict-comment aborts the whole run when a PR hits the 2500-comment cap — blocks the ring1→stable soak gate #594

Closed

Description

@don-petry

Summary

A single comment-capped PR aborts every auto-rebase run on a repo. auto-rebase-reusable.yml's merge-conflict branch posts a conflict-resolution comment via gh pr comment (line ~235) without an error guard. When the target PR has hit GitHub's 2500-comment cap, that command exits non-zero and — because the step runs under bash -e (shell: /usr/bin/bash -e {0}) — the entire step fails, even though other PRs were rebased successfully earlier in the same run.

Evidence

petry-projects/bmad-bgreat-suite PR #205 (CI Failure Analyst…, OPEN) is at exactly 2500 comments. Every auto-rebase run that reaches it dies:

PR #205 (add/ci-failure-analyst-v2) is 8 commit(s) behind main — updating branch
  Update failed: {"message":"merge conflict between base and head", ... "status":"422"}
  Posting conflict resolution request
GraphQL: Commenting is disabled on issues with more than 2500 comments (addComment)
##[error]Process completed with exit code 1.

4 runs failed with this identical signature in the auto-rebase v2.1.0 soak window (all on bmad-bgreat-suite): 28484669277, 28481353704, 28314060392, 28311198868. PRs #341/#335/#334 were updated fine in the same runs — only the #205 comment killed them.

Impact

This is the canary-rollout blocker for auto-rebase (initiative #495, gate standard #548). The gate correctly classifies it BLOCKED / PRE_EXISTING (cum_fail=4, reusable byte-identical to prior), so it will not roll back — but auto-rebase can never reach stable while a capped PR keeps poisoning the ring1 (bmad) runs. Same failure class as the pr-review-mention comment-cap loop (#553).

Root cause

auto-rebase-reusable.yml line ~235:

gh pr comment "$PR_NUMBER" --repo "$REPO" --body "$CONFLICT_BODY"

The sibling delete on line ~217 is already guarded (... || true); the post is not. A best-effort conflict notification must not be fatal to the core function (rebasing the other PRs).

Fix (primary — the reusable)

Guard the post so a comment-side failure is logged, not fatal (mirrors #553):

gh pr comment "$PR_NUMBER" --repo "$REPO" --body "$CONFLICT_BODY" \
  || echo "::warning::could not post conflict comment on #$PR_NUMBER (comment cap / API error) — continuing"

Introduced by the #482 reusables initiative (auto-rebase-reusable.yml). Add a regression test to auto-rebase-tests.yml stubbing a non-zero gh pr comment and asserting the run still succeeds and continues to the next PR.

Secondary (the capped PR)

bmad-bgreat-suite #205 has 2500 comments and cannot be commented on again. Surface to the owner to resolve/close #205 (and audit what generated 2500 comments there). Not something the release automation should mutate.

Enhancement — auto-triage this class in the release workflow

The canary-rollout gate already classifies REGRESSION vs PRE_EXISTING and supports a per-agent benign_failure_classes allowlist (#1025 P2), but that matches on failed-step name only — too coarse here (the step name Update behind non-Dependabot PRs also covers real failures). Proposal: extend the gate's failure triage to inspect the log signature (error text) of an in-window failure and auto-classify known-environmental patterns — more than 2500 comments, secondary-rate-limit, HTTP 5xx — as benign/environmental, distinct from a genuine step failure. That would let a comment-cap failure be excluded from cum_fail automatically (defense-in-depth) even before the source fix lands, without masking real regressions. Tracked against canary-rollout #495 / soak standard #548.

Refs

#482 (introducing initiative) · #553 (precedent comment-cap fix) · #495 / #548 (canary-rollout + soak standard) · petry-projects/.github-private #1045 (promote path that surfaced this).

Activity

  1. don-petry commented on Jul 3, 2026

    @don-petry
    ContributorAuthor

    Dev-Lead Implementation Plan

    Issue: #594 — auto-rebase: unguarded conflict-comment aborts the whole run when a PR hits the 2500-comment cap

    Scope

    Make the merge-conflict resolution comment in auto-rebase-reusable.yml best-effort so a comment-side failure (GitHub's 2500-comment cap, secondary rate limit, transient 5xx) is logged and swallowed instead of killing the whole bash -e step, letting the loop continue rebasing the other PRs.

    Implementation Checklist

    • Add .github/scripts/auto-rebase/lib/comments.sh with a testable auto_rebase_post_comment_best_effort helper (posts via gh pr comment; on failure emits ::warning::… — continuing and returns 0) — mirrors the extract-logic-to-scripts convention so it can be bats-tested instead of trapped inline in YAML
    • Source comments.sh in .github/workflows/auto-rebase-reusable.yml and route the conflict post (line ~235) through the helper (fix mirrors fix(pr-review-mention): break the ack self-comment recursion #553)
    • Add comments.sh to the shellcheck step in .github/workflows/auto-rebase-tests.yml
    • Document the helper in .github/scripts/auto-rebase/README.md

    Tests to Write

    • test/workflows/auto-rebase/comments.bats: stub a non-zero gh pr comment → helper returns 0 (non-fatal) and logs the ::warning::… continuing line
    • success case: stub gh exit 0 → returns 0, posts the comment, no warning
    • regression: a bash -e loop over two PRs where the first PR's comment fails still reaches and processes the second PR

    Test Command

    bats --print-output-on-failure test/workflows/auto-rebase/ (plus shellcheck -x on the auto-rebase libs and bash .dev-lead/scripts/dev-lead-lint.sh)

    Note on scope

    Primary (reusable) fix only. The capped PR (bmad-bgreat-suite #205) and the release-workflow log-signature auto-triage enhancement are separate follow-ups called out in the issue and are out of scope here.

  2. don-petry commented on Jul 3, 2026

    @don-petry
    ContributorAuthor

    Dev-Lead: Implementation Complete

    Plan Execution

    • Added .github/scripts/auto-rebase/lib/comments.sh with best-effort auto_rebase_post_comment_best_effort helper
    • Routed the conflict-resolution post (reusable line ~235) through the helper — a comment-side failure is now logged, not fatal (mirrors fix(pr-review-mention): break the ack self-comment recursion #553)
    • Added comments.sh to the shellcheck step in auto-rebase-tests.yml
    • Documented the helper + rationale in the auto-rebase README
    • TDD regression tests written first (red), then green

    Test Results

    1..22
    ok 1 post_comment_best_effort: comment-cap failure is non-fatal (returns 0)
    ok 2 post_comment_best_effort: failure logs a continue warning naming the PR
    ok 3 post_comment_best_effort: success returns 0, posts the comment, no warning
    ok 4 post_comment_best_effort: a capped PR does not abort a bash -e loop over multiple PRs
    ok 5 has_current_approval: single APPROVED review counts as approved
    ok 6 has_current_approval: no reviews is not approved
    ok 7 has_current_approval: only COMMENTED reviews are not approved
    ok 8 has_current_approval: reviewer who approved then requested changes is not approved
    ok 9 has_current_approval: reviewer who requested changes then approved is approved
    ok 10 has_current_approval: a later COMMENTED review does not cancel an approval
    ok 11 has_current_approval: a dismissed approval is not approved
    ok 12 has_current_approval: one approver among several reviewers counts
    ok 13 has_ready_label: label present
    ok 14 has_ready_label: label absent
    ok 15 has_ready_label: empty labels array
    ok 16 pr_eligible review-ready: non-draft + approved is eligible
    ok 17 pr_eligible review-ready: non-draft + ready label is eligible
    ok 18 pr_eligible review-ready: non-draft but neither approved nor labelled is ineligible
    ok 19 pr_eligible review-ready: draft is ineligible even when approved
    ok 20 pr_eligible all: always eligible (legacy fan-out escape hatch)
    ok 21 pr_eligible all: eligible even for drafts (preserves prior behavior)
    ok 22 pr_eligible: unknown mode errors out (exit 2)
    

    Also green: shellcheck -x on both auto-rebase libs (CI parity) and bash .dev-lead/scripts/dev-lead-lint.sh (all checks passed); both workflow YAMLs parse.

    Files Changed

    • .github/scripts/auto-rebase/lib/comments.sh (new): auto_rebase_post_comment_best_effort PR_NUMBER REPO BODY — posts via gh pr comment; on any failure (2500-comment cap / secondary rate limit / transient 5xx) emits ::warning::… — continuing and returns 0.
    • .github/workflows/auto-rebase-reusable.yml: source comments.sh; the conflict post now calls the helper instead of a bare gh pr comment, so it can no longer abort the bash -e step and starve the other PRs of a rebase.
    • test/workflows/auto-rebase/comments.bats (new): stub a non-zero gh pr comment → non-fatal + warning; success case; and a bash -e loop over two PRs where the first (capped) PR's comment fails yet the loop still reaches the second PR.
    • .github/workflows/auto-rebase-tests.yml: shellcheck now also covers comments.sh.
    • .github/scripts/auto-rebase/README.md: documents the new helper and the auto-rebase: unguarded conflict-comment aborts the whole run when a PR hits the 2500-comment cap — blocks the ring1→stable soak gate #594 rationale.

    Notes

  3. don-petry commented on Jul 3, 2026

    @don-petry
    ContributorAuthor

    Second instance — same root cause blocks pr-review-mention too

    The canary gate also holds pr-review-mention at ring1→stable (cum_fail=1), and it's the identical comment-cap class:

    • Run 28318567437 on petry-projects/bmad-bgreat-suite, step Post acknowledgement comment:
      GraphQL: Commenting is disabled on issues with more than 2500 comments (addComment)
      ##[error]Process completed with exit code 1.
      

    #553 added the marker-based trigger guard to stop the self-comment loop, but the ack-post itself is still unguarded, so a comment-capped target PR makes it exit non-zero → the run fails.

    So both remaining #482 ring1→stable blockers are one bug class (unguarded best-effort comment under bash -e), both surfacing on bmad-bgreat-suite where PR #205 (and possibly others) sit at the 2500-comment cap:

    1. auto-rebase-reusable.yml — conflict comment (~L235)
    2. pr-review-mention reusable — Post acknowledgement comment step

    Fix both: guard the gh pr comment/gh api ... addComment with || echo "::warning::…" so a comment-side failure is logged, not fatal. The proposed gate log-signature triage enhancement (auto-classify more than 2500 comments / rate-limit / 5xx as environmental) would unblock both reusables' soak automatically even before the source fixes land.

    Once both reusables are guarded (and/or the enhancement lands), the canary timer (now armed, CANARY_AUTO_PROMOTE=true) will auto-promote auto-rebase + pr-review-mention to stable — completing the 6/6 #482 rollout with no manual step.

  4. added a commit that references this issue on Jul 3, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    dev-leadFor dev-lead agent pickup

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions