Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 30 additions & 2 deletions bin/fm-pr-merge.sh
Original file line number Diff line number Diff line change
Expand Up @@ -791,19 +791,35 @@ FM_PR_GITHUB_QUEUE_METHODS=
FM_PR_GITHUB_QUEUE_STATUS=unreadable
github_read_queue_method() {
local methods line candidate method='' count=0 branch_path
local unrecognised=false conflicting=false
local unrecognised=false conflicting=false api_err api_err_text
FM_PR_GITHUB_QUEUE_METHOD=
FM_PR_GITHUB_QUEUE_METHODS=
FM_PR_GITHUB_QUEUE_STATUS=unreadable
command -v gh >/dev/null 2>&1 || return 0
[ -n "$FM_PR_GITHUB_BASE" ] || return 0
branch_path=$(github_urlencode_path_segment "$FM_PR_GITHUB_BASE")
api_err=$(mktemp "${TMPDIR:-/tmp}/fm-pr-merge-queue-rules.XXXXXX") || return 0
if ! methods=$(gh api \
--paginate "repos/$PR_OWNER/$PR_REPO/rules/branches/$branch_path" \
--jq '.[] | select(.type == "merge_queue") | "merge_method=" + (.parameters.merge_method // "")' \
2>/dev/null); then
2>"$api_err"); then
api_err_text=$(cat "$api_err" 2>/dev/null)
rm -f "$api_err"
# A plan-gated 403 on this endpoint ("Upgrade to GitHub Pro or make this
# repository public") means the repository's plan cannot expose branch
# rules at all, on GitHub or GitHub Enterprise Server - not that this
# script failed to read them. A repository that cannot have branch rules
# cannot have a merge_queue rule either, so that specific 403 resolves to
# no queue rather than the generic unreadable status. Any other failure
# (auth, rate limit, network, a 404, an unrelated 403) stays unreadable.
case "$api_err_text" in
*"Upgrade to GitHub Pro or make this repository public"*)
FM_PR_GITHUB_QUEUE_STATUS=none
;;
esac
return 0
fi
rm -f "$api_err"
while IFS= read -r line; do
[ -n "$line" ] || continue
case "$line" in
Expand Down Expand Up @@ -945,6 +961,18 @@ persist_accepted_merge_authority() {
return 1
}

# While away, a merge proceeds only when the base branch's rules prove no
# merge queue, because a queued merge can land after its away authority
# lapses; this holds regardless of which away authority (a named merge grant
# or a standing yolo=on posture) let the merge run at all. A repository whose
# plan does not expose branch rules at all (GitHub's "Upgrade to GitHub Pro or
# make this repository public" 403) proves that on its own, since such a
# repository cannot have a merge_queue rule either; see
# github_read_queue_method, which resolves that specific 403 to status=none.
# Every other failure to read the queue state (auth, rate limit, network, a
# 404, or an unrelated 403) stays unreadable and refuses the merge. The merge
# stays synchronous (--auto is refused earlier) and every other gate still
# applies.
refuse_github_queue_while_away() {
[ "$FM_PR_AWAY_POSTURE" = true ] || return 0
# Accepted confused-agent-grade limitation, as in bin/fm-lease-lib.sh, not an
Expand Down
2 changes: 1 addition & 1 deletion docs/architecture.md
Original file line number Diff line number Diff line change
Expand Up @@ -319,7 +319,7 @@ A check run is green when its current run is green, because GitHub leaves a canc
An attended `--allow-red <check-name>` may appear once, waives only GitHub checks with that exact name, and is refused while the away-posture record exists.
Because away merge authority is read from that record and then acted on by the forge, the authority read and synchronous forge command share the record's cross-subsystem lock, closing the common live-owner TOCTOU.
A lock that cannot be taken refuses the merge.
While the record exists, GitHub auto-merge and any base whose rules cannot prove the absence of a merge queue are refused before submission, and GitLab auto-merge flags or scheduled state are refused while an immediate merge is forced with a final `--auto-merge=false`.
While the record exists, GitHub auto-merge and any base whose rules cannot prove the absence of a merge queue are refused before submission, and GitLab auto-merge flags or scheduled state are refused while an immediate merge is forced with a final `--auto-merge=false`; a branch-rules read that fails only because the repository's plan does not expose branch rules at all (GitHub's plan-upgrade 403) proves the absence of a merge queue on its own and does not refuse, while every other failure to read that state still does.
This is deliberately confused-agent-grade, as `bin/fm-lease-lib.sh` defines that grade, rather than fully atomic.
A GitHub queue-rule or PR-base change after the queue-free preflight can still enqueue a merge that lands after its away grant lapses, and killing the lock-owning shell while its forge child survives lets stale-owner recovery admit archive or replacement before that child completes.
These are accepted limitations, not oversights; durable authority, landing re-verification, and child-lock handoff are outside this boundary.
Expand Down
62 changes: 62 additions & 0 deletions tests/fm-pr-merge.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -190,6 +190,10 @@ case "${1:-} ${2:-}" in
exit 0
;;
api\ *)
if [ -f "${FM_TEST_GH_RULES_FAIL_BODY:-}" ]; then
cat "$FM_TEST_GH_RULES_FAIL_BODY" >&2
exit 1
fi
if [ -f "${FM_TEST_GH_RULES_FAIL:-}" ]; then
exit 1
fi
Expand Down Expand Up @@ -381,6 +385,7 @@ run_pr_merge() {
FM_TEST_GH_MERGE_OUTPUT="$(cat "$case_dir/github-merge-output" 2>/dev/null || true)" \
FM_TEST_GH_GRAPHQL_FAIL="$case_dir/github-graphql-fail" \
FM_TEST_GH_RULES_FAIL="$case_dir/github-rules-fail" \
FM_TEST_GH_RULES_FAIL_BODY="$case_dir/github-rules-fail-body" \
FM_TEST_META_AT_MERGE="$case_dir/meta-at-merge" \
FM_TEST_AWAY_RECORD_AFTER_VIEW="$case_dir/away-record-after-view" \
FM_TEST_ROOT="$ROOT" \
Expand Down Expand Up @@ -833,6 +838,61 @@ test_github_unreadable_queue_rules_are_not_reported_as_no_queue() {
pass "fm-pr-merge distinguishes unreadable branch rules from a base with no merge queue"
}

# A repository whose plan does not expose branch rules answers the rules
# endpoint with a 403 whose body is GitHub's own plan-upgrade message, not a
# generic auth or rate-limit failure. That repository cannot have a
# merge_queue rule either, so it must read as no queue rather than unreadable
# - an attended read still fails the merge here only because the queue-aware
# outcome read (api graphql) was never set up for this case, exactly like the
# no-queue-rule case below; the queue read itself is proven by the absence of
# 'merge-queue' wording in the refusal.
test_github_plan_gated_403_reads_as_no_queue() {
local case_dir rc
case_dir=$(make_case github-plan-gated-403)
mkdir -p "$case_dir/wt"
add_gh_mocks "$case_dir" 8989898989898989898989898989898989898989
write_github_outcome "$case_dir" OPEN false false main
printf 'gh: Upgrade to GitHub Pro or make this repository public to enable this feature (HTTP 403)\n' \
> "$case_dir/github-rules-fail-body"
: > "$case_dir/gh-axi.log"
: > "$case_dir/gh.log"

set +e
run_pr_merge "$case_dir" task-x1 https://github.com/example/repo/pull/75 \
> "$case_dir/stdout" 2> "$case_dir/stderr"
rc=$?
set -e

expect_code 1 "$rc" "github-plan-gated-403: an unproved merge must fail"
assert_no_grep 'merge queue' "$case_dir/stderr" \
"github-plan-gated-403: a plan-gated 403 was read as an unreadable or present queue rule"
assert_no_grep 'could not be read' "$case_dir/stderr" \
"github-plan-gated-403: a plan-gated 403 was reported as an unreadable rules response"
pass "fm-pr-merge reads a plan-gated 403 on branch rules as no merge queue, not unreadable"
}

# The practical effect of the fix: while away under a standing yolo=on
# posture (no per-task merge grant), a private repository's plan-gated 403
# must no longer refuse the merge the way any other unreadable queue response
# does.
test_away_plan_gated_403_does_not_block_the_merge() {
local case_dir rc url head
head=cececececececececececececececececececece
url=https://github.com/example/repo/pull/91
case_dir=$(make_case away-plan-gated-403)
mkdir -p "$case_dir/wt" "$case_dir/home"
add_gh_mocks "$case_dir" "$head"
printf 'gh: Upgrade to GitHub Pro or make this repository public to enable this feature (HTTP 403)\n' \
> "$case_dir/github-rules-fail-body"
printf '\nyolo=on\n' >> "$case_dir/state/task-x1.meta"
write_away_record "$case_dir"
FM_TEST_HOME="$case_dir/home" run_pr_merge "$case_dir" task-x1 "$url" \
> "$case_dir/stdout" 2> "$case_dir/stderr" \
|| fail "away-plan-gated-403: a private repo's plan-gated 403 must not block an away merge"
assert_logged_gh_merge "$case_dir" 91 example/repo --squash
pass "away merge proceeds on a plan-gated 403 because that repository cannot have a merge queue"
}

test_github_no_queue_rule_says_nothing_about_a_queue() {
local case_dir rc
case_dir=$(make_case github-no-queue-rule)
Expand Down Expand Up @@ -2102,6 +2162,7 @@ test_github_accepted_queue_flags_do_not_echo_back_the_same_command
test_github_mismatched_queue_flags_still_name_the_retry
test_github_unrecognised_queue_method_still_names_the_queue
test_github_unreadable_queue_rules_are_not_reported_as_no_queue
test_github_plan_gated_403_reads_as_no_queue
test_github_no_queue_rule_says_nothing_about_a_queue
test_github_unmerged_fallback_cannot_replace_queue_aware_read
test_github_auto_merge_without_queue_refuses_legibly
Expand Down Expand Up @@ -3039,6 +3100,7 @@ test_allow_red_is_refused_while_away
test_allow_red_requires_one_separate_name
test_away_grant_and_yolo_and_hold_for_return
test_away_posture_refuses_asynchronous_merge_paths
test_away_plan_gated_403_does_not_block_the_merge
test_away_grant_does_not_bypass_red_or_identity
test_unreadable_away_record_refuses_merge
test_away_record_cannot_change_between_the_authority_read_and_the_merge
Expand Down
Loading