merge_guard_post mints token for the WRONG PR when approval text contains an unrelated #NNNN (false 'token does not match' rejections)

[michael-wojcik]

Correction (this issue was initially misfiled). It first claimed the merge rejections came from the Claude Code harness ("not any PACT hook"), then from escaped-quote byte-mismatch. Both were wrong — asserted without verifying. After grepping + probing the live hooks, the rejections are from PACT's own merge-guard, and the root cause is a specific, confirmed bug in merge_guard_post.py. Body rewritten to the verified finding.

Problem

A legitimately user-approved squash-merge (PR #1029) was repeatedly rejected by merge_guard_pre.py with "Authorization token exists but does not match this operation" — succeeding only when the command was reduced to a bare form. The merge-guard is a deliberate, valuable security control (prevents API-based bypass of merge approval); this is a false-rejection bug in it, not a request to weaken it.

Root cause (confirmed by probing the live hooks)

The token's pr_number context is minted from the AskUserQuestion question prose:

# merge_guard_post.py:204  (extract_context)
pr_match = re.search(r"#(\d+)|PR\s*(\d+)|pull\s*request\s*(\d+)", question, re.IGNORECASE)

…but read/validated from the anchored gh pr merge <N> command:

# merge_guard_pre.py  (_extract_pr_number → _GH_PR_NUMBER_RE)
#   anchored to `gh pr merge|close`, with a value-taking-flag guard

These are asymmetric. The mint side grabs the FIRST #NNNN anywhere in the approval text. When that text contains an unrelated issue/PR reference before the real PR — e.g. an umbrella issue (#1028) embedded in the squash commit --subject, or a "related #NNN" — the token is minted for the wrong PR. The read side then correctly extracts the real PR from the command, the two disagree, and _token_matches_command returns False → the genuine, approved merge is rejected.

Empirical evidence (live extract_context, this session)

AskUserQuestion text (abridged)	minted pr_number	command targets	result
…gh pr merge 1029 --squash --subject "…(#1028)" --body-file …	1028 (from #1028)	1029	MISMATCH → rejected
…squash-merge of PR #1029 … gh pr merge 1029 --squash	1029 (from #1029)	1029	match → merged

The bare form only succeeded because #1029 happened to be the first #NNNN in its text. The "use the bare command" workaround is therefore fragile coincidence, not a fix — any approval prose mentioning another #NNNN first re-triggers it.

Proposed fix

Make the mint-side extraction symmetric with the read-side:

1. In extract_context, extract the PR number from the gh pr merge|close <N> command structure embedded in the question (reuse / mirror _extract_pr_number + _GH_PR_NUMBER_RE), not a loose #(\d+) prose match.
2. Fallback ordering: prefer the command-anchored positional; fall back to #NNNN / PR N prose only when no gh pr merge|close <N> is present in the text.
3. Regression test: an AskUserQuestion text whose --subject/prose contains an unrelated #NNNN before the real PR positional MUST mint a token for the real PR (assert pr_number == "1029" for the table's row 1 shape).

Sibling-asymmetry note

The same mint-vs-read asymmetry may exist on the branch and operation_type axes (post-side extract_context uses prose regexes; pre-side uses anchored command detection). Worth an audit pass alongside the PR-number fix — see below.

Also confirmed NOT bugs (by-design, keep)

• cd <repo> && gh pr merge … → "Compound destructive command rejected" is correct: the guard intentionally authorizes one destructive op per approval. The fix is to not compound, not to relax the guard.
• The op-type + PR-number token-matching model is sound; only the mint-side PR-number extraction is brittle.

Scope

A real, confirmed PACT code bug in merge_guard_post.py — fully within PACT's control, fixable with a symmetric-extraction change + regression test. (The earlier "platform-gate documentation" framing was wrong and is corrected here.)

[michael-wojcik]

Corrected after investigation (prompted by a maintainer question). Original framing was wrong twice: (1) attributed to the Claude Code harness, then (2) to escaped-quote byte-mismatch. Verified root cause: merge_guard_post.extract_context line 204 mints pr_number from a loose #(\d+) prose match, which grabbed the umbrella #1028 from the squash --subject instead of the real PR 1029. Live-probe evidence: extract_context on the flag-rich approval text → pr_number=1028; on the bare text → 1029. Command targets 1029 → token mismatch. Fix = symmetric command-anchored extraction (mirror merge_guard_pre._extract_pr_number).

[michael-wojcik]

Sibling-asymmetry audit results (3-axis adversarial workflow, verified end-to-end against the live hooks).

The PR-number false-reject generalizes beyond a #NNNN in the --subject — confirmed cases:

• Per issue #1028, approve the merge — gh pr merge for PR 1029? → mints pr=1028 → blocks the 1029 merge (any tracking-issue hash left of the real PR poisons it).
• Merge approved — this lands the fix from issue #1028. Proceed? (command merges PR 1029) → mints pr=1028 → blocked (operator cites only the tracking issue, never number-scopes the PR).

Two MORE false-reject axes confirmed (same mint-prose-vs-read-command root):

• branch: an approval naming branch origin:feature mints branch='origin' (prose capture stops at the colon) while the command deletes 'origin:feature' → mismatch → blocked. Same for release@1.0 → mints 'release'.
• operation_type: approval "force push that will delete the branch feat/x" (command is a branch-delete) mints op_type=force-push, command is branch-delete → cross-op mismatch → blocked.

All are the same fix proposed here (command-anchored, symmetric extraction).

⚠️ The audit also found the false-AUTHORIZE (security) direction of this same asymmetry — split out as #1032 (4 end-to-end-confirmed authorization bypasses, incl. a benign verification question authorizing a force-push to main). #1031 (usability/false-reject) and #1032 (security/false-authorize) share the root cause; the symmetric-extraction + fail-closed-on-sparse-token fix addresses both.

Note — a third brittleness class surfaced while filing this: the guard's is_dangerous_command scans the raw command string, so a benign gh issue comment whose body merely quotes dangerous git commands (as examples) is itself flagged dangerous and rejected. Worth folding into the robustness work — the guard should classify the operation, not pattern-match substrings inside quoted arguments.

[michael-wojcik]

Live recurrence (PR #1034 merge, 2026-06-26) — a no-token-minted variant of the false-reject

Another live instance, with a manifestation distinct from the body's "wrong-PR mint": here the guard minted no valid token at all, rejecting a fully user-authorized merge.

What happened

A user-approved squash-merge of PR #1034 was rejected with requires user approval via AskUserQuestion (the no-token reject, NOT the does not match mismatch). The approving AskUserQuestion's QUESTION text:

• did lead with the real PR (PR #1034 …) — i.e. it satisfied the "first #NNNN = real PR" guidance, yet still failed;
• was padded with distractor numbers (version 4.4.42, all 6 reviewers, the commit SHA 704abadc);
• placed the gh pr merge 1034 --squash command only in the OPTION description, not the question prose.

→ merge_guard_post did not mint a token that merge_guard_pre would accept for gh pr merge 1034 --squash.

The fix that worked

Re-asking with a minimal question — Confirm: squash-merge PR #1034 into main now via + literal gh pr merge 1034 --squash + ?, with the explicit command in the question text and #1034 as the only number — minted + matched on the first retry. The failed first attempt did not poison/consume the retry.

What this adds

The false-reject is not only "leftmost #NNNN grabs the wrong PR" (the umbrella-ref case in the body). It also fires as a total mint-failure when the merge intent isn't cleanly in the QUESTION prose — the operative gh pr merge <N> lives only in the option and the question carries non-PR numbers. That's a real hazard for any orchestrator writing a rich, informative merge-confirmation question (the natural thing to do — CI status, version, reviewer count).

The proposed command-anchored, symmetric extraction (mint the PR from the gh pr merge|close <N> structure wherever it appears in the approval text — question or selected option — rather than loose prose) would fix this variant too: it would find gh pr merge 1034 in the option and mint 1034 regardless of question padding.

Operator workaround (until fixed)

Merge-confirmation AskUserQuestion: put the bare gh pr merge <N> --squash in the question text, make the PR# the only number, strip version/count/SHA distractors.

[michael-wojcik]

Logged live-probe PASS — gated closure satisfied (per the #924 gated-issue-close discipline).

Post-install live-probe of the INSTALLED plugin v4.4.43 merge_guard hooks (the real installed cache build, driven via synthesized platform stdin frames — NOT the pytest suite), run through two INDEPENDENT harnesses (a primary security probe + a clean-room independent verifier that re-derived the envelope shapes from source without reading the primary). Session scoping was intentionally inert, so results isolate the (op,target) mint↔read symmetry axis.

#1031 (false-REJECT — must AUTHORIZE the real target): PASS (3/3, both harnesses converge).

• R-A — prose distractor #1028 present; minted {operation_type: merge, pr_number: 1029} from the clicked option; executed gh pr merge 1029 --squash → PRE allow (exit 0), token consumed.
• R-B — padded question (version 4.4.42 / all 6 reviewers / a commit SHA) ignored; the command lived only in the clicked option's description; minted pr_number: 1034 → allow. (Also served as the verifier's positive control proving the harness can mint.)
• R-C — branch-delete target retained the full colon-bearing name origin:feature (not truncated at the colon) → allow.

v4.4.43 correctly authorizes legitimate command-anchored approvals. Closing per the gated live-probe acceptance criterion.