diff --git a/hive-c0re/src/forge/repos.rs b/hive-c0re/src/forge/repos.rs index de80776b..f8aa5703 100644 --- a/hive-c0re/src/forge/repos.rs +++ b/hive-c0re/src/forge/repos.rs @@ -714,21 +714,20 @@ fn main_branch_protection_option() -> CreateBranchProtectionOption { } } -/// Whether a branch-protection create failed because a rule for the -/// branch already exists: 409/422 ([`is_already_exists`]) or the 200 -/// Forgejo answers instead of 201 for a duplicate rule (unlisted in -/// the endpoint spec, so it surfaces as `UnexpectedStatusCode(200)`). -fn is_protection_already_present(e: &ForgejoError) -> bool { - is_already_exists(e) - || matches!(e, ForgejoError::UnexpectedStatusCode(s) if *s == StatusCode::OK) -} - /// Apply the operator merge-gate branch protection to `repo`'s default /// branch: only [`OPERATORS_TEAM`] members can merge, and an /// approving review from that team is required — so the author (a write-level -/// agent, not in the team) cannot merge its own PR. Idempotent: an existing -/// rule for the branch (200/409/422) is treated as success. +/// agent, not in the team) cannot merge its own PR. +/// +/// Idempotent, but **verify-don't-trust**: a create failure is ambiguous — +/// "rule already exists" (success) OR a silent rejection that created NO rule +/// (e.g. a 422 where `OPERATORS_TEAM` doesn't exist in `AGENTS_ORG`). The old +/// code folded 200/409/422 into `Ok` and left the repo unprotected with no +/// error — a fail-open merge gate. So on any create error, GET the `main` rule +/// and only treat it as success if the rule is actually present (the exact fix +/// already applied to [`apply_config_repo_branch_protection`]). async fn apply_operator_branch_protection(repo: &str, token: &str) -> Result<()> { + let client = api(token)?; let mut rule = main_branch_protection_option(); rule.enable_merge_whitelist = Some(true); rule.merge_whitelist_teams = Some(vec![OPERATORS_TEAM.to_owned()]); @@ -736,19 +735,28 @@ async fn apply_operator_branch_protection(repo: &str, token: &str) -> Result<()> rule.approvals_whitelist_teams = Some(vec![OPERATORS_TEAM.to_owned()]); rule.required_approvals = Some(1); rule.block_on_official_review_requests = Some(true); - match api(token)? + let Err(create_err) = client .repo_create_branch_protection(AGENTS_ORG, repo, rule) .await + else { + tracing::info!(%repo, "forge: applied operator branch protection"); + return Ok(()); + }; + match client + .repo_get_branch_protection(AGENTS_ORG, repo, "main") + .await { Ok(_) => { - tracing::info!(%repo, "forge: applied operator branch protection"); + tracing::debug!( + %repo, create_error = %create_err, + "forge: operator branch protection already present" + ); Ok(()) } - Err(e) if is_protection_already_present(&e) => { - tracing::debug!(%repo, "forge: branch protection already present"); - Ok(()) - } - Err(e) => Err(e).with_context(|| format!("create branch protection {AGENTS_ORG}/{repo}")), + Err(check_err) => anyhow::bail!( + "branch protection for {AGENTS_ORG}/{repo} not applied: create failed \ + ({create_err}); GET main rule failed ({check_err}), no `main` rule present" + ), } }