merge-clearance: CodeRabbit rate-limit undetected when CR resolves status to success

[mujtaba3B]

Summary

merge-clearance reported CodeRabbit - ✅ verdict=clear, status=success, reviewed-head=yes on a PR that CodeRabbit had not reviewed at all. CR had posted its rate-limit marker comment and resolved its commit status to success with the description Review rate limited.

The verdict was CLEAR and the PR merged on a review that never happened. The local /eng:cr review happened to backstop it, but that was my discipline, not the gate's.

The gap: a fourth rate-limit shape

eng/hooks/scripts/merge-clearance.sh (~L443-485) detects rate-limiting in three branches, each gated on CR_STATUS_STATE:

Branch	Requires	Handles
1	state = missing + reviewed-head = no	CR never started
2	state = pending + reviewed-head = no	CR started, stuck pending
3	state = failure	CR resolved to failure with a rate-limit description

Observed here: state = success with description = "Review rate limited". That matches none of the three, so CR_RATE_LIMITED stays no and the marker comment is never consulted.

Branch 3's comment already anticipates the shape ("CR may have fully reviewed the PR already... then tripped the limit on a trailing commit") but binds it to failure. CR also resolves this to success.

Reproduction

mujtaba3B/mutwo-skills#175, HEAD 64afd9b2198dc4f074d0433016d18c8a72e1c753:

$ gh api repos/mujtaba3B/mutwo-skills/commits/64afd9b.../status \
    --jq '.statuses[] | select((.context|ascii_downcase)|contains("coderabbit"))'
context=CodeRabbit  state=success  description=Review rate limited

CR's only issue comment on the PR carries the stable marker, and it is CR's latest:

$ gh api repos/mujtaba3B/mutwo-skills/issues/175/comments \
    --jq '[.[] | select((.user.login|ascii_downcase)|contains("coderabbitai"))] | last | .body | contains("rate limited by coderabbit.ai")'
true

So mc_cr_rate_limited_latest would return yes if it were reached. It is not reached, because no branch fires for state = success.

Why this one matters more than the others

The three handled shapes all leave a visible "something is off" signal (missing / pending / failure). This one renders as a green check with a reassuring verdict line, so both gh pr checks and merge-clearance agree that CodeRabbit is fine.

~/dev/CLAUDE.md tells sessions to trust the clearance verdict over green GitHub checks precisely because the verdict is "strictly stronger". In this shape it is not: it inherits the same false positive and launders it into an explicit verdict=clear.

Suggested fix

Add a fourth branch, mirroring branch 3 but for success:

elif [ "$CR_STATUS_STATE" = "success" ]; then
  CR_FAILURE_RATE_LIMITED=$(mc_cr_failure_rate_limited "$CR_STATUS_STATE" "$(cr_status_description)" '[]')
fi

mc_cr_failure_rate_limited already reads the description first and falls back to the marker, which is the right shape for this case. It would need to accept success as a state. Like the other three, this should only RELAX via CR_RL_BACKSTOPPED (a current local /eng:cr), never on its own.

Worth a test: eng/hooks/tests/merge-clearance-lib.bats covers the other shapes; this one needs state=success + rate-limit description asserting the verdict does NOT come back clear without a backstop.

Note on assertions

Per the repo's own habit of asserting the block REASON rather than the outcome: a test here should assert that the CodeRabbit dimension reports rate-limited-and-backstopped, not merely that the overall verdict is non-clear, since an unrelated dimension could produce a passing-for-the-wrong-reason test.