From 6a4382bee88a615d0d323e2bcc74ccf8028acc7e Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 18 Jul 2026 15:34:09 +0200 Subject: [PATCH] fix(#2570): recognize 422 team-already-exists so operators-team provisioning stops warning Forgejo returns team-already-exists as HTTP 422 ValidationFailed, not 409 Conflict, so the 409-only guard in ensure_operators_team missed it and logged a spurious warning every boot (and skipped the settings reconcile). Add a lenient discriminator that also treats a 422 whose message says already-exists as benign. --- hive-c0re/src/forge/repos.rs | 74 +++++++++++++++++++++++++++++++----- 1 file changed, 64 insertions(+), 10 deletions(-) diff --git a/hive-c0re/src/forge/repos.rs b/hive-c0re/src/forge/repos.rs index 445886b5..99443318 100644 --- a/hive-c0re/src/forge/repos.rs +++ b/hive-c0re/src/forge/repos.rs @@ -119,6 +119,33 @@ fn is_conflict(e: &ForgejoError) -> bool { } } +/// Whether a rendered error message names the "already exists" case. The +/// discriminator that tells a *benign* already-exists 422 apart from a +/// *real* validation 422 (invalid units, etc.). Case-insensitive. +fn message_says_already_exists(rendered: &str) -> bool { + rendered.to_lowercase().contains("already exists") +} + +/// Whether `e` is Forgejo saying the resource already exists — matching +/// the 409 conflict shape *and* the 422 shape this Forgejo build actually +/// returns for a duplicate team: `validation failed: team already exists`. +/// A 409-only [`is_conflict`] check misses that 422, so the caller fires +/// a spurious "provisioning failed" warning every boot and skips its +/// settings reconcile. This still surfaces *other* 422s (bad request +/// body) as real failures — only a 422 whose message names the +/// already-exists case is folded in. +fn is_already_exists_lenient(e: &ForgejoError) -> bool { + if is_conflict(e) { + return true; + } + let is_unprocessable = match e { + ForgejoError::ApiError(api) => matches!(api.error_kind(), ApiErrorKind::ValidationFailed), + ForgejoError::UnexpectedStatusCode(s) => *s == StatusCode::UNPROCESSABLE_ENTITY, + _ => false, + }; + is_unprocessable && message_says_already_exists(&e.to_string()) +} + /// Whether an error is Forgejo saying 404 — the resource is absent, /// as opposed to a transport / auth / server failure. fn is_not_found(e: &ForgejoError) -> bool { @@ -714,12 +741,15 @@ pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()> tracing::info!(%org, "forge: created {OPERATORS_TEAM} team"); Ok(()) } - // 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) => { + // Team already exists — reconcile settings to desired state so a + // team created with an older/wrong shape self-heals on next boot. + // Forgejo signals the duplicate as a 409 conflict OR (this build) a + // 422 `validation failed: team already exists`; both mean the same + // thing, so fold both in via `is_already_exists_lenient` — a 409-only + // check missed the 422 and warned every boot. List teams to find the + // id (required by org_edit_team), then unconditionally PATCH to the + // desired settings. Members are a separate endpoint; untouched here. + Err(e) if is_already_exists_lenient(&e) => { let (_headers, teams) = client .org_list_teams(org) .await @@ -747,10 +777,10 @@ pub(super) async fn ensure_operators_team(org: &str, token: &str) -> Result<()> tracing::debug!(%org, "forge: reconciled {OPERATORS_TEAM} team settings"); 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. + // Any OTHER error (a 422 that is NOT already-exists, or a transport + // / auth failure): surface it — don't mask a real failure. A + // non-already-exists 422 means the request body is invalid (e.g. + // Forgejo rejected the units list); it repeats every boot until fixed. Err(e) => Err(e).with_context(|| format!("create team {org}/{OPERATORS_TEAM}")), } } @@ -992,3 +1022,27 @@ pub async fn create_agent_repo(agent: &str, repo: &str, core_token: &str) -> Res tracing::info!(%agent, %repo, "forge: created agent repo in {AGENTS_ORG} with operator merge gate"); Ok(format!("{AGENTS_ORG}/{repo}")) } + +#[cfg(test)] +mod tests { + use super::message_says_already_exists; + + #[test] + fn already_exists_message_is_recognised() { + // The exact 422 body this Forgejo build returns for a duplicate team. + assert!(message_says_already_exists( + "validation failed: team already exists [org_id: 10, name: operators]" + )); + // Case-insensitive. + assert!(message_says_already_exists("Repository Already Exists")); + } + + #[test] + fn real_validation_error_is_not_treated_as_already_exists() { + // A genuine bad-request 422 must still surface, not be folded in. + assert!(!message_says_already_exists( + "validation failed: units must not be empty" + )); + assert!(!message_says_already_exists("not found")); + } +}