Add merge feasibility preflight and stale approval shadow gate

[justin808]

References #477

deferred_to_update_changelog

Rationale:

• Added branch-protection collection to the shared autonomous merge evidence helper so dispatch-time evaluation can bind the exact base branch protection policy.
• Added a stale-approval shadow signal (stale-approval-satisfiable) to autonomous-merge-eligibility when stale approvals can still satisfy branch protection.
• Extended the closeout renderer and receipt validator to accept and surface the new shadow gate.
• Extended the planning-time preflight to report branch protection, acting identity, self-approval impossibility, admin bypass availability, required status checks, and an explicit expected terminal.

Checks:

• ruby skills/pr-batch/bin/pr-security-preflight-test.rb -n test_pr_targets_report_merge_feasibility_branch_protection_and_stale_approval_defect
• ruby skills/pr-batch/bin/autonomous-merge-eligibility-test.rb -n test_stale_approval_satisfiable_is_reported_as_a_shadow_gate
• ruby skills/pr-batch/bin/autonomous-merge-eligibility-test.rb -n test_live_collection_returns_a_verdict_with_non_ascii_payload_in_a_c_locale
• ruby skills/pr-batch/bin/autonomous-merge-closeout-test.rb -n test_shadow_gate_signals_render_the_stale_approval_advisory
• ruby skills/pr-batch/bin/merge-assurance-test.rb -n test_autonomous_stale_approval_shadow_gate_survives_the_receipt_path
• git diff --check
• bin/lint (rubocop and shellcheck passed; markdownlint passed; the command stopped only because yamllint is not installed in this environment)

Residual:

• The executable preflight/dispatch path is implemented, but the prompt-template/doc mirror in skills/plan-pr-batch/SKILL.md and workflows/pr-processing.md still does not carry an explicit expected-terminal field. That is the narrow follow-up surface.

[coderabbitai]

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

• Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[claude]

Bug: KeyError crash whenever branch protection can't be fully resolved.

feasibility.fetch(:admin_bypass_available) assumes the key always exists, but merge_feasibility_unknown (lines 1532-1560) never includes admin_bypass_available: in the hash it returns. Any of these very common conditions will hit that path and then blow up here with an uncaught KeyError:

• the base branch has no branch protection configured at all (gh api .../branches/<branch>/protection returns 404, which run_gh_json turns into a StandardError caught at pr-security-preflight:414)
• required_pull_request_reviews is null (protection exists but doesn't require reviews)
• any of required_approving_review_count / dismiss_stale_reviews / require_last_push_approval / enforce_admins is missing

Since this is a security preflight script, crashing here means the tool aborts entirely instead of degrading to the UNKNOWN status it was designed to report. The only test added (test_pr_targets_report_merge_feasibility_branch_protection_and_stale_approval_defect) only exercises the fully-populated "infeasible" path, so this gap isn't caught.

Fix: add admin_bypass_available: nil to merge_feasibility_unknown's returned hash (or use feasibility[:admin_bypass_available] here instead of .fetch).

[claude]

This breaks nearly every existing test in autonomous-merge-evidence-test.rb.

This adds an unconditional api.call("#{prefix}/branches/#{initial_base_ref}/protection") inside collect, but the test doubles in skills/pr-batch/bin/autonomous-merge-evidence-test.rb (not touched by this PR) were never updated to stub that path. Both the shared complete_api helper and most individual test lambdas end their case with else raise "unexpected API path #{path}".

Since this new call happens after the head/base-movement check but before the later checks (final_force_push_watermark, changed_files/commits equality, commit SHAs must be unique, and all of normalize_file/normalize_review/normalize_comment, which only run when building the final returned hash), essentially every test whose expected CollectionError is raised at or after that point — including the plain happy-path test test_collects_every_page_and_rechecks_exact_head_and_base — will instead hit the unstubbed branch and raise a bare RuntimeError, which assert_raises(AutonomousMergeEvidence::CollectionError) won't catch (and the happy-path test isn't expecting a raise at all).

The PR's own "Checks" list only runs specific test names via -n on other files and never runs autonomous-merge-evidence-test.rb, which is presumably how this slipped through. Please add a branches/.../protection stub to the shared fakes in that file (and ideally a case for a repo with no branch protection configured, i.e. a 404, since that's a very common real-world state).

[claude]

Minor consistency nit: this stale-approval comparison (and the current-head-approval one at line 1636) compares commit_id to head_sha case-sensitively, whereas the equivalent shadow-gate logic in autonomous-merge-eligibility (line 456) downcases both sides before comparing. GitHub SHAs are conventionally already lowercase so this likely isn't triggering in practice, but the two nearly-identical "is this approval stale" implementations should probably share the same normalization (or better, share the same code) so they can't silently drift.

[claude]

Review summary

Focused review of the merge-feasibility preflight / stale-approval shadow gate. Left inline comments with details; the two bugs below are the ones worth blocking on.

1. pr-security-preflight crashes (KeyError) whenever branch protection can't be fully resolved. print_merge_feasibility unconditionally calls feasibility.fetch(:admin_bypass_available), but merge_feasibility_unknown never sets that key. Any repo/branch without branch protection configured (a 404 from the GitHub API — a very common, normal state) or with incomplete protection facts hits this and crashes the script instead of printing UNKNOWN. The only new test covers the fully-populated "infeasible" path, so this wasn't caught.

2. The collect() change in autonomous_merge_evidence.rb breaks most of the existing autonomous-merge-evidence-test.rb suite. It adds an unconditional call to branches/.../protection, but that test file's fake API doubles (complete_api and most per-test lambdas) don't stub that path and raise "unexpected API path" in their else branch. Since the new call happens before several of the later validity checks (and before all of normalize_file/normalize_review/normalize_comment, which run when building the final hash), most tests — including the plain happy-path one — will now error out. The PR's own checklist only runs specific tests via -n on other files and never runs this file, which is likely how this slipped through.

3. (minor) Stale-approval "is this commit_id the head?" comparisons are case-sensitive in pr-security-preflight but case-insensitive (.downcase) in autonomous-merge-eligibility. Probably harmless in practice since GitHub SHAs are lowercase, but worth aligning since these two near-duplicate implementations could otherwise drift.

Design-wise, making the new stale-approval-satisfiable signal shadow-only (non-blocking) for now, consistent with how reviewed-heads-limit was bootstrapped, looks like the right call given the PR's own stated residual follow-up.

[justin808]

🤖 Codex

Closing this stale implementation so issue #477 can be rebuilt narrowly from current main.

The branch is 77 commits behind, conflicts across eight of its nine paths, and its Validate run was cancelled. Two unresolved review findings are consequential: an ordinary UNKNOWN feasibility path can raise a key error, and an unconditional branch-protection API call breaks existing evidence collectors. The PR body also leaves the typed expected-terminal schema and consumer contract unfinished.

Issue #477 already preserves the replacement requirements: per-target typed feasible/infeasible/UNKNOWN results with exact reasons; branch-protection, actor, author, and distinct-approval evidence; no silent admin-bypass authority; dispatch-time freshness; heterogeneous batch support; schema validation and real consumer use; and a separate stale-approval advisory path.

The remote branch remains available as reference. Closing this PR does not close #477 or authorize a replacement merge.