From beaa220dc20beb1f6a83209ddc030f0228d79888 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 10 Jul 2026 01:19:53 +0200 Subject: [PATCH 1/2] fix(#2287): use is_conflict (409-only) for team create, supply explicit units --- hive-c0re/src/forge/repos.rs | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/hive-c0re/src/forge/repos.rs b/hive-c0re/src/forge/repos.rs index 2991715f..c090f523 100644 --- a/hive-c0re/src/forge/repos.rs +++ b/hive-c0re/src/forge/repos.rs @@ -561,7 +561,7 @@ async fn ensure_mirror_repo( /// merge/approval whitelist; the operator adds herself as a member via the /// forge UI / hivectl. `includes_all_repositories` so the gate applies to /// every repo in the org; `write` is enough to approve + merge. hive-c0re -/// never manages membership. Idempotent (422/409 = already exists). +/// never manages membership. Idempotent (409 = already exists). /// /// Must run for BOTH [`AGENTS_ORG`] and [`CONFIG_ORG`]: Gitea teams are /// org-scoped, so a config-repo branch-protection rule referencing @@ -569,14 +569,33 @@ async fn ensure_mirror_repo( /// there 422'd every `apply_config_repo_branch_protection`, leaving config /// repos unprotected — operator-merged config PRs then bypassed the deploy /// pipeline and silently didn't apply. +/// +/// Uses `is_conflict` (409 only) — NOT `is_already_exists` (which also +/// folds 422 into "already exists"). A 422 from `org_create_team` is a +/// real validation error (bad request shape, missing units, etc.) that +/// must surface so it can be fixed; the previous 422-swallowing hid the +/// true cause and left the team silently uncreated every boot. pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()> { + // Explicit units so Forgejo doesn't reject a null/absent units field. + // A `write`-permission team needs at minimum repo.code and repo.pulls + // to review and merge PRs. Include the full standard set so members + // can see the whole repo surface. + let units = Some(vec![ + "repo.code".to_owned(), + "repo.issues".to_owned(), + "repo.pulls".to_owned(), + "repo.releases".to_owned(), + "repo.wiki".to_owned(), + "repo.projects".to_owned(), + "repo.packages".to_owned(), + ]); let team = CreateTeamOption { can_create_org_repo: Some(false), description: Some("hyperhive operators — merge gate for agent repos".to_owned()), includes_all_repositories: Some(true), name: OPERATORS_TEAM.to_owned(), permission: Some(CreateTeamOptionPermission::Write), - units: None, + units, units_map: None, }; match api(token)?.org_create_team(org, team).await { @@ -584,10 +603,15 @@ pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()> tracing::info!(%org, "forge: created {OPERATORS_TEAM} team"); Ok(()) } - Err(e) if is_already_exists(&e) => { + // 409: team already exists — idempotent. + Err(e) if is_conflict(&e) => { tracing::debug!(%org, "forge: {OPERATORS_TEAM} team already exists"); Ok(()) } + // 422 and any other error: surface it — don't mask a real failure + // as "already exists". A 422 here typically means the request body + // is invalid (e.g. Forgejo rejected the units list or another + // field); it will repeat every boot until fixed. Err(e) => Err(e).with_context(|| format!("create team {org}/{OPERATORS_TEAM}")), } } From e9667f9c1ae5ee03f112e023a8f2c316cbcb3655 Mon Sep 17 00:00:00 2001 From: atlas Date: Fri, 10 Jul 2026 02:08:26 +0200 Subject: [PATCH 2/2] fix(#2287): reconcile team settings on 409 (upsert via org_edit_team) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 409 (team already exists), list the org teams to find the operators team id, then unconditionally PATCH to the desired settings via org_edit_team. This self-heals a team that was created with the wrong shape by an older code path (missing units, wrong permission) without touching membership (separate endpoint, operator-managed). Addresses mara's review: 'shouldnt we get, then change, then update'. Unconditional PATCH is simpler than GET→diff→conditional PATCH and safe here since we own units/permission/description fully. --- hive-c0re/src/forge/repos.rs | 68 ++++++++++++++++++++++++++---------- 1 file changed, 50 insertions(+), 18 deletions(-) diff --git a/hive-c0re/src/forge/repos.rs b/hive-c0re/src/forge/repos.rs index c090f523..de80776b 100644 --- a/hive-c0re/src/forge/repos.rs +++ b/hive-c0re/src/forge/repos.rs @@ -10,7 +10,8 @@ use anyhow::{Context, Result}; use forgejo_api::structs::{ AddCollaboratorOption, AddCollaboratorOptionPermission, CreateBranchProtectionOption, CreateOrgOption, CreateRepoOption, CreateTeamOption, CreateTeamOptionPermission, - EditRepoOption, MigrateRepoOptions, MigrateRepoOptionsService, Repository, + EditRepoOption, EditTeamOption, EditTeamOptionPermission, MigrateRepoOptions, + MigrateRepoOptionsService, Repository, }; use forgejo_api::{ApiErrorKind, ForgejoError}; use reqwest::StatusCode; @@ -575,37 +576,68 @@ async fn ensure_mirror_repo( /// real validation error (bad request shape, missing units, etc.) that /// must surface so it can be fixed; the previous 422-swallowing hid the /// true cause and left the team silently uncreated every boot. +/// Repo-unit access flags for the `operators` team. +/// Explicit list so Forgejo doesn't reject a null/absent `units` field; +/// a `write`-permission team needs at least `repo.code` + `repo.pulls` +/// to review and merge PRs. +const OPERATORS_TEAM_UNITS: &[&str] = &[ + "repo.code", + "repo.issues", + "repo.pulls", + "repo.releases", + "repo.wiki", + "repo.projects", + "repo.packages", +]; + pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()> { - // Explicit units so Forgejo doesn't reject a null/absent units field. - // A `write`-permission team needs at minimum repo.code and repo.pulls - // to review and merge PRs. Include the full standard set so members - // can see the whole repo surface. - let units = Some(vec![ - "repo.code".to_owned(), - "repo.issues".to_owned(), - "repo.pulls".to_owned(), - "repo.releases".to_owned(), - "repo.wiki".to_owned(), - "repo.projects".to_owned(), - "repo.packages".to_owned(), - ]); + let units_vec: Vec = OPERATORS_TEAM_UNITS.iter().map(|&s| s.to_owned()).collect(); let team = CreateTeamOption { can_create_org_repo: Some(false), description: Some("hyperhive operators — merge gate for agent repos".to_owned()), includes_all_repositories: Some(true), name: OPERATORS_TEAM.to_owned(), permission: Some(CreateTeamOptionPermission::Write), - units, + units: Some(units_vec.clone()), units_map: None, }; - match api(token)?.org_create_team(org, team).await { + let client = api(token)?; + match client.org_create_team(org, team).await { Ok(_) => { tracing::info!(%org, "forge: created {OPERATORS_TEAM} team"); Ok(()) } - // 409: team already exists — idempotent. + // 409: team already exists — reconcile settings to desired state so + // a team created with an older/wrong shape self-heals on next boot. + // List teams to find the id (required by org_edit_team), then + // unconditionally PATCH to the desired settings. Members are a + // separate endpoint; this never touches membership. Err(e) if is_conflict(&e) => { - tracing::debug!(%org, "forge: {OPERATORS_TEAM} team already exists"); + let (_headers, teams) = client + .org_list_teams(org) + .await + .with_context(|| format!("list teams for {org}"))?; + let team_id = teams + .into_iter() + .find(|t| t.name.as_deref() == Some(OPERATORS_TEAM)) + .and_then(|t| t.id) + .with_context(|| { + format!("{OPERATORS_TEAM} team not found in {org} after 409 conflict") + })?; + let edit = EditTeamOption { + can_create_org_repo: Some(false), + description: Some("hyperhive operators — merge gate for agent repos".to_owned()), + includes_all_repositories: Some(true), + name: OPERATORS_TEAM.to_owned(), + permission: Some(EditTeamOptionPermission::Write), + units: Some(units_vec), + units_map: None, + }; + client + .org_edit_team(team_id, edit) + .await + .with_context(|| format!("reconcile {org}/{OPERATORS_TEAM} team settings"))?; + tracing::debug!(%org, "forge: reconciled {OPERATORS_TEAM} team settings"); Ok(()) } // 422 and any other error: surface it — don't mask a real failure