From 92100ac1f812e7b9ca519b2ba924b6f28e26f714 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 21 Sep 2026 20:27:46 +0200 Subject: [PATCH] hive-forge: ci-rerun --pr refuses instead of lying about a fix it can't achieve A workflow_dispatch run writes no commit status, so ci-rerun --pr could never clear the red (pull_request) check it claimed to be re-running for - it dispatched a fresh run and printed a success message regardless, even though the check stays red no matter how that run turns out. --pr now refuses up front, before dispatching, naming the mechanism and the working alternative (re-run from the web UI). --run and --branch are unchanged: --run's own PR-pseudo-ref resolution and --branch's direct dispatch are both untouched. Fixes the exit-code/honesty defect from #4613; the workflow_dispatch vs. pull_request event-type question (whether to close+reopen the PR to fire a real pull_request event) is a separate, parked decision. --- docs/tools/forge.md | 16 ++++++---- hive-forge/src/verbs/ci_rerun.rs | 55 +++++++++++++++++++++++++------- 2 files changed, 52 insertions(+), 19 deletions(-) diff --git a/docs/tools/forge.md b/docs/tools/forge.md index 29ba434a..21f6da80 100644 --- a/docs/tools/forge.md +++ b/docs/tools/forge.md @@ -103,7 +103,7 @@ hive-forge attach-comment 18042 /path/to/file # upload a file attachment to a c hive-forge attachment-get # download an attachment; prints resolved path to stdout hive-forge artifact-get pr1ma-paper-pdf --run 51 # download a CI run's Actions artifact zip (run number from the run-page URL) hive-forge ci-log --run 51 # print a CI run's job step logs (run number from the run-page URL); --job i / --step i to narrow -hive-forge ci-rerun --pr 42 # re-run CI without an empty commit (dispatches a fresh run; --run n / --branch name also work) +hive-forge ci-rerun --branch foo # re-run CI without an empty commit (dispatches a fresh run; --run n also works; --pr refuses, see below) hive-forge subscription --watch # subscribe to repo notifications hive-forge subscription --unwatch # unsubscribe hive-forge subscription --list # list every repo you watch (audit the notification firehose) @@ -293,12 +293,14 @@ to discover valid label names before triaging or to audit the label set. are CSRF-gated web routes a token POST 404s), so this dispatches a **fresh** run of the workflow via the workflow-dispatch API (`POST …/actions/workflows//dispatches {"ref":""}`). - Resolve the branch with exactly one of: `--pr ` (the PR's head - branch), `--run ` (the same run number `ci-log` / `artifact-get` - take — resolves the branch + workflow from that run), or `--branch -` (directly). `--workflow ` picks the workflow file for - `--pr` / `--branch` (default `ci.yml`). Dispatch re-runs the whole - workflow — there is no single-job variant. + Resolve the branch with `--run ` (the same run number `ci-log` / + `artifact-get` take — resolves the branch + workflow from that run) or + `--branch ` (directly). `--pr ` refuses instead of dispatching: + a workflow-dispatch run writes no commit status, so it can't clear a + red `(pull_request)` check on that PR's sha — re-run from the web UI + instead. `--workflow ` picks the workflow file for `--branch` + (default `ci.yml`). Dispatch re-runs the whole workflow — there is no + single-job variant. - `issue dependency add ` / `pr dependency add ` set "blocked by" links via Forgejo's dependency feature — the operator's preferred way to track blocking relationships over labels diff --git a/hive-forge/src/verbs/ci_rerun.rs b/hive-forge/src/verbs/ci_rerun.rs index 8eebc9a7..7964bb05 100644 --- a/hive-forge/src/verbs/ci_rerun.rs +++ b/hive-forge/src/verbs/ci_rerun.rs @@ -12,18 +12,18 @@ //! //! The branch (and, for `--run`, the workflow file) is resolved from the //! given handle: -//! - `--pr ` → the PR's head branch; dispatches `--workflow` (default -//! `ci.yml`) on it. +//! - `--pr ` → refuses instead of dispatching: a `workflow_dispatch` run +//! writes no commit status, so it cannot clear a red `(pull_request)` +//! check on the PR's sha — see `docs/scheduler/ci.md`'s "CI checks" for +//! why. Re-run from the web UI instead. //! - `--branch ` → dispatches `--workflow` on that branch directly. //! - `--run ` → looks the run up by its display number (same convention //! as `ci-log` / `artifact-get` / `ci-runs`) and dispatches the same //! workflow + ref the run used. A PR-triggered run's ref is a `#` //! pseudo-ref, not a real branch — that case resolves one hop further via -//! the same `branch_for_pr` lookup `--pr` uses. +//! `branch_for_pr`. //! -//! Dispatch re-runs the whole workflow (no single-job variant). ⚠️ `--pr` -//! verifies the code but doesn't reliably move the PR's own status -//! checks — see `docs/scheduler/ci.md`'s "CI checks" for why; re-check `pr status`. +//! Dispatch re-runs the whole workflow (no single-job variant). use anyhow::{Context as _, Result, bail}; use clap::Args as ClapArgs; @@ -60,12 +60,15 @@ pub struct Args { /// missing its ref), or if the dispatch POST fails (network, or a non-2xx /// such as `404` for an unknown workflow file or branch). pub fn run(client: &Client, args: Args) -> Result<()> { + if let Some(pr) = args.pr { + bail!(pr_refusal_message(pr)); + } + let repo = client.repo()?; - let (workflow, branch) = match (args.pr, args.run, args.branch.as_deref()) { - (Some(pr), _, _) => (args.workflow.clone(), branch_for_pr(client, pr)?), - (_, Some(run), _) => resolve_run(client, repo, run, &args.workflow)?, - (_, _, Some(branch)) => (args.workflow.clone(), branch.to_string()), - (None, None, None) => { + let (workflow, branch) = match (args.run, args.branch.as_deref()) { + (Some(run), _) => resolve_run(client, repo, run, &args.workflow)?, + (_, Some(branch)) => (args.workflow.clone(), branch.to_string()), + (None, None) => { bail!("ci-rerun: pass one of --pr , --run , or --branch ") } }; @@ -91,6 +94,19 @@ pub fn run(client: &Client, args: Args) -> Result<()> { Ok(()) } +/// Why `--pr` refuses rather than dispatching: a `workflow_dispatch` run +/// writes no commit status (see the module doc), so it cannot clear a red +/// `(pull_request)` check on this PR's sha no matter how the workflow run +/// itself turns out. Names the mechanism and the working alternative +/// rather than just "not supported", since the caller needs to know *why* +/// before deciding what to do instead. +fn pr_refusal_message(pr: u64) -> String { + format!( + "ci-rerun --pr {pr}: a workflow_dispatch run writes no commit status, so this cannot \ + clear a red (pull_request) check on PR #{pr} — re-run it from the web UI instead" + ) +} + /// Resolve a PR's head branch name (`head.ref`) — the branch a same-repo PR /// pushes to, which is the ref we dispatch the workflow on. fn branch_for_pr(client: &Client, pr: u64) -> Result { @@ -156,7 +172,7 @@ fn pr_number_from_run_ref(value: &str) -> Option { #[cfg(test)] mod tests { - use super::{ActionRun, pr_number_from_run_ref, run_dispatch_target}; + use super::{ActionRun, pr_number_from_run_ref, pr_refusal_message, run_dispatch_target}; use serde_json::json; /// Build a typed run record from an API-shaped JSON fixture. The @@ -226,4 +242,19 @@ mod tests { assert_eq!(pr_number_from_run_ref("#12a"), None); assert_eq!(pr_number_from_run_ref(""), None); } + + /// `run()` bails with this message before it ever builds a + /// `DispatchWorkflowOption` or touches the client — this pins the + /// message text without needing a live `Client` to exercise `run()` + /// itself. The message must name the mechanism (`workflow_dispatch` + /// writes no commit status), the PR, and the working alternative, + /// not just say the path is unsupported. + #[test] + fn pr_refusal_names_the_mechanism_and_the_alternative() { + let msg = pr_refusal_message(4199); // lint:allow: PR-shaped test fixture, not a tracker reference + assert!(msg.contains("workflow_dispatch")); + assert!(msg.contains("commit status")); + assert!(msg.contains("PR #4199")); // lint:allow: PR-shaped test fixture, not a tracker reference + assert!(msg.contains("web UI")); + } }