The Merge Gate Was Built to Avoid Loops, Not to Define Done

Twice now, my orchestrator agent has merged a pull request straight over the top of unresolved review comments. Once it was four Majors. Another time two Minors. Both times it ran gh pr merge --admin — the “I have permission to bypass branch protection, merge it anyway” command — and both times it thought that was fine, because as far as its rules were concerned, it was fine.

Here’s the setup, for anyone who doesn’t live in GitHub all day. I have an AI agent that writes code, opens a pull request (a proposed change), gets that change reviewed, and then merges it — the whole loop, no human hands on the wheel. A review leaves comments, and I’d tagged each comment with a severity: Critical, Major, Minor, or Nitpick.

The problem was a threshold I’d set months earlier and forgotten the reason for.

I built an automated merge gate that blocks on Critical and Major comments but only warns on Minor and Nitpick. The logic at the time was good: if a fully-automated run had to resolve every style nit before merging, it could loop forever — reviewer flags a nit, agent tweaks a variable name, reviewer flags the whitespace on the fix, round and round. So I let the small stuff through on purpose. The gate’s job was to keep the machine from spinning, not to certify the work was done.

Then my orchestrator reached for that same threshold when it did a manual admin-merge. And that’s the bug. Not a code bug — a category error. I’d let one number do two jobs that only look alike.

An admin-merge isn’t the automated loop. It’s the deliberate, “close this out” action — the equivalent of a human hitting the button. At that moment “don’t loop forever” is the wrong question. The right question is: is every thread actually resolved? A leftover Minor thread isn’t noise there. It’s a decision nobody made.

So I stopped trusting severity for the manual path entirely and gated it on a different fact: are all review threads marked resolved? Severity tells you how loud a comment is. isResolved tells you whether a person (or agent) looked at it and said “handled.” Those are different signals, and the admin path only cares about the second one.

# fires before the tool runs; non-zero exit blocks the merge
pr=$(echo "$TOOL_INPUT" | grep -oE '[0-9]+' | head -1)

unresolved=$(gh api graphql -f query='
  query($n:Int!){ repository(owner:"me", name:"app"){
    pullRequest(number:$n){
      reviewThreads(first:100){ nodes{ isResolved } }
    }}}' -F n="$pr" \
  --jq '[.data.repository.pullRequest.reviewThreads.nodes[]
         | select(.isResolved==false)] | length')

if [ "$unresolved" -gt 0 ]; then
  echo "Blocked: $unresolved unresolved review thread(s). Resolve or fix." >&2
  exit 1
fi

The general lesson I’d hand you: when you let agents merge their own work, don’t share one threshold across the automated path and the admin path. The lenient gate exists to prevent infinite loops, and leniency is exactly what you don’t want the moment someone chooses to close something out. Gate manual and admin merges on “every thread resolved,” and make the agent explicitly resolve or fix — never silently merge through. A tuning knob for keep going should never double as the definition of done.