From a704a844d106ea28882e73dc99effb0c73d17dab Mon Sep 17 00:00:00 2001 From: atlas Date: Sat, 19 Sep 2026 14:10:06 +0200 Subject: [PATCH] hive-forge: dependency remove accepts the forge's 201 Created MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The forge always answers 201 Created for a successful dependency removal, but the generated forgejo-api client only treats 200 as success for that endpoint, so every successful removal surfaced as ForgejoError::UnexpectedStatusCode(201). apply_and_verify caught this, read the dependency list back, saw the edge was gone, and printed a "converged despite a reported error" warning — appropriate for a genuinely unpredictable anomaly, but this mismatch is deterministic and not a real problem. Treat that specific status as success outright on the remove path instead of routing it through the read-back/warning path. The add path's different, genuinely intermittent 500-despite-success quirk is untouched, and a real remove failure still goes through the read-back and still surfaces if it does not converge. Closes #4531 --- hive-forge/src/verbs/dependency.rs | 35 ++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/hive-forge/src/verbs/dependency.rs b/hive-forge/src/verbs/dependency.rs index 76f5a6cb..7fd82521 100644 --- a/hive-forge/src/verbs/dependency.rs +++ b/hive-forge/src/verbs/dependency.rs @@ -13,6 +13,7 @@ use std::collections::HashSet; use anyhow::{Result, bail}; use clap::{Args as ClapArgs, Subcommand}; +use forgejo_api::ForgejoError; use forgejo_api::structs::IssueMeta; use serde_json::Value; @@ -70,14 +71,18 @@ pub fn run(client: &Client, args: Args) -> Result<()> { /// verify by reading the dependency list back rather than trusting the /// HTTP status this verb family returns. /// -/// Measured: the status code lies in both directions on this endpoint -/// pair. `create` can 500 on a call that actually wrote the -/// edge (an intermittent server-side double-processing, not a client -/// retry — ruled out separately). `remove` can answer `201 Created` on a -/// call that actually deleted the edge, which the typed client's -/// endpoint spec maps to an error since 201 isn't the expected status for -/// a deletion. So a `send()` error here only means "maybe" — the -/// authoritative signal is whether the edge is actually there afterward. +/// Measured: `create` can 500 on a call that actually wrote the edge (an +/// intermittent server-side double-processing, not a client retry — +/// ruled out separately). A `send()` error there only means "maybe" — +/// the authoritative signal is whether the edge is actually there +/// afterward. +/// +/// `remove`'s known quirk is handled below, before it ever reaches this +/// path: the forge always answers `201 Created` for a successful +/// deletion, which the generated client's endpoint spec doesn't expect +/// (it only treats 200 as success for that verb), so that specific +/// status is treated as success outright rather than routed through the +/// read-back below. /// /// Calls that errored are only re-classified as real failures if the /// read-back does NOT show the expected converged state (edge present @@ -107,7 +112,19 @@ fn apply_and_verify( .send() }; if let Err(e) = res { - call_errors.push((*dep, e)); + // `remove` always answers 201 for a successful deletion (not + // the 200 the generated client expects for that verb) — a + // deterministic, always-on mismatch, not a "something else + // went wrong after the write" anomaly, so it's not worth a + // read-back or a warning: just accept it. + let is_remove_201 = !adding + && matches!( + &e, + ForgejoError::UnexpectedStatusCode(code) if *code == reqwest::StatusCode::CREATED + ); + if !is_remove_201 { + call_errors.push((*dep, e)); + } } } if call_errors.is_empty() {