From 392f16cbc0a93db06f079d131902adb235513e73 Mon Sep 17 00:00:00 2001 From: atlas Date: Mon, 21 Sep 2026 21:37:22 +0200 Subject: [PATCH] hive-forge: ci-rerun --run refuses on a PR-triggered run too --run against a run that was itself PR-triggered has the identical defect #4632 fixed for --pr: a workflow_dispatch run writes no commit status, so it can't clear the red (pull_request) check on that PR's sha no matter how the dispatched run turns out. resolve_run already distinguishes this case -- it recognizes a run's prettyref as a PR pseudo-ref (#) via pr_number_from_run_ref, then used to call branch_for_pr to keep going. It now bails with the same pr_refusal_message --pr uses instead, before ever building a dispatch request. branch_for_pr has no other caller (--pr refuses before touching it too, since #4632), so it's removed rather than left dead. Push (non-PR-triggered) and --branch are untouched. Regenerated docs/tools/forge-cli.md from clap help; corrected docs/tools/forge.md's claim that --run always works. --- docs/tools/forge-cli.md | 2 +- docs/tools/forge.md | 5 +- hive-forge/src/verbs/ci_rerun.rs | 95 +++++++++++++++++--------------- 3 files changed, 55 insertions(+), 47 deletions(-) diff --git a/docs/tools/forge-cli.md b/docs/tools/forge-cli.md index 13b21a3f..eac3fd7e 100644 --- a/docs/tools/forge-cli.md +++ b/docs/tools/forge-cli.md @@ -1371,7 +1371,7 @@ Re-run CI without an empty commit. Pass one of `--pr `, `--run `, or `--br ###### **Options:** * `--pr ` — Doesn't work: a `workflow_dispatch` run writes no commit status, so it can't clear a red `(pull_request)` check — re-run from the web UI instead. Mutually exclusive with `--run` / `--branch` -* `--run ` — Re-run the same workflow on the same branch this run used. The run number is the `runs/` in the run-page URL. Mutually exclusive with `--pr` / `--branch` +* `--run ` — Re-run the same workflow on the same branch this run used. The run number is the `runs/` in the run-page URL. Refuses instead of dispatching if the run was itself PR-triggered, same as `--pr` and for the same reason. Mutually exclusive with `--pr` / `--branch` * `--branch ` — Re-run `--workflow` on this branch. Mutually exclusive with `--pr` / `--run` * `--workflow ` — Workflow file to run (default `ci.yml`). Ignored for `--run`, which uses the run's own workflow diff --git a/docs/tools/forge.md b/docs/tools/forge.md index 21f6da80..d84360f7 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 --branch foo # re-run CI without an empty commit (dispatches a fresh run; --run n also works; --pr refuses, see below) +hive-forge ci-rerun --branch foo # re-run CI without an empty commit (dispatches a fresh run; --run n also works unless that run was PR-triggered; --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) @@ -295,7 +295,8 @@ to discover valid label names before triaging or to audit the label set. (`POST …/actions/workflows//dispatches {"ref":""}`). 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: + `--branch ` (directly). `--pr `, and `--run ` against a run + that was itself PR-triggered, both refuse 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` diff --git a/hive-forge/src/verbs/ci_rerun.rs b/hive-forge/src/verbs/ci_rerun.rs index 8f666276..be4ed614 100644 --- a/hive-forge/src/verbs/ci_rerun.rs +++ b/hive-forge/src/verbs/ci_rerun.rs @@ -20,8 +20,8 @@ //! - `--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 -//! `branch_for_pr`. +//! pseudo-ref, not a real branch — that case refuses for the same reason +//! `--pr` does (same underlying run, same red check, same sha). //! //! Dispatch re-runs the whole workflow (no single-job variant). @@ -30,7 +30,7 @@ use clap::Args as ClapArgs; use forgejo_api::structs::{ActionRun, DispatchWorkflowOption}; use super::ci_common::find_run_by_number; -use crate::client::{Client, index}; +use crate::client::Client; #[derive(ClapArgs)] pub struct Args { @@ -40,8 +40,9 @@ pub struct Args { #[arg(long, conflicts_with_all = ["run", "branch"])] pr: Option, /// Re-run the same workflow on the same branch this run used. The run - /// number is the `runs/` in the run-page URL. Mutually exclusive - /// with `--pr` / `--branch`. + /// number is the `runs/` in the run-page URL. Refuses instead of + /// dispatching if the run was itself PR-triggered, same as `--pr` and + /// for the same reason. Mutually exclusive with `--pr` / `--branch`. #[arg(long, conflicts_with_all = ["pr", "branch"])] run: Option, /// Re-run `--workflow` on this branch. Mutually exclusive with @@ -56,10 +57,12 @@ pub struct Args { /// # Errors /// -/// Returns an error if none of `--pr` / `--run` / `--branch` is given, if a -/// `--pr` / `--run` handle can't be resolved (unknown PR/run, or a run -/// missing its ref), or if the dispatch POST fails (network, or a non-2xx -/// such as `404` for an unknown workflow file or branch). +/// Returns an error if none of `--pr` / `--run` / `--branch` is given, if +/// `--pr` is given or `--run` names a PR-triggered run (refuses before +/// dispatching — see the module doc), if a `--run` handle can't otherwise +/// be resolved (unknown run, or a run 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)); @@ -95,12 +98,14 @@ 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. +/// Why `--pr` (and `--run` against a PR-triggered run) 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. Shared by both refusal sites so the same underlying run gets +/// the same story regardless of which handle named it. fn pr_refusal_message(pr: u64) -> String { format!( "ci-rerun --pr {pr}: a workflow_dispatch run writes no commit status, so this cannot \ @@ -108,25 +113,13 @@ fn pr_refusal_message(pr: u64) -> String { ) } -/// 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 { - let (owner, name) = client.owner_repo()?; - let pull = client - .api() - .repo_get_pull_request(owner, name, index(pr)?) - .send()?; - pull.head - .and_then(|h| h.r#ref) - .with_context(|| format!("ci-rerun: PR #{pr} has no head.ref")) -} - /// Resolve the run whose display number is `run_number` to the /// `(workflow, branch)` to dispatch a fresh run of it. `fallback_workflow` /// is used when the run carries no workflow file name. When the run's own -/// ref is a PR pseudo-ref (`#`, not a real branch — workflow-dispatch -/// 500s on it), resolves the PR's actual head branch instead, same as -/// `--pr` would. +/// ref is a PR pseudo-ref (`#`, not a real branch), the run was +/// triggered by that PR — refuses with [`pr_refusal_message`], same as +/// `--pr` would, instead of dispatching a run whose status can't clear the +/// PR's red check. fn resolve_run( client: &Client, repo: &str, @@ -137,20 +130,18 @@ fn resolve_run( .with_context(|| format!("ci-rerun: run #{run_number} not found in {repo}"))?; let (workflow, branch) = run_dispatch_target(&run, fallback_workflow) .with_context(|| format!("ci-rerun: run #{run_number} has no ref"))?; - let branch = match pr_number_from_run_ref(&branch) { - Some(pr) => branch_for_pr(client, pr) - .with_context(|| format!("ci-rerun: run #{run_number} was triggered by PR #{pr}"))?, - None => branch, - }; + if let Some(pr) = pr_number_from_run_ref(&branch) { + bail!(pr_refusal_message(pr)); + } Ok((workflow, branch)) } /// Pull the `(workflow-file, ref)` dispatch target out of a run record: /// `prettyref` is the ref the run ran on (the branch name for push / -/// dispatch runs — PR-event runs carry a `#` pseudo-ref instead, resolved -/// one call site up in [`resolve_run`]), and `workflow_id` is the workflow -/// file name (e.g. `ci.yml`), falling back to `fallback_workflow` when -/// absent. `None` only when the run has no ref. +/// dispatch runs — PR-event runs carry a `#` pseudo-ref instead, turned +/// into a refusal one call site up in [`resolve_run`]), and `workflow_id` +/// is the workflow file name (e.g. `ci.yml`), falling back to +/// `fallback_workflow` when absent. `None` only when the run has no ref. fn run_dispatch_target(run: &ActionRun, fallback_workflow: &str) -> Option<(String, String)> { let branch = run.prettyref.as_deref().filter(|s| !s.is_empty())?; let workflow = run @@ -218,10 +209,10 @@ mod tests { #[test] fn pr_triggered_run_keeps_its_pseudo_ref_at_this_layer() { // resolve_run (untested here — does I/O) is what turns this into a - // real branch; the pure extractor must NOT do that resolution - // itself, or a run with a genuine branch named e.g. "#weird" (not - // possible in git, but worth pinning the boundary) would be handled - // in two different places. + // refusal; the pure extractor must NOT do that recognition itself, + // or a run with a genuine branch named e.g. "#weird" (not possible + // in git, but worth pinning the boundary) would be handled in two + // different places. let run = run_from(json!({ "prettyref": "#4199", "workflow_id": "ci.yml" })); // lint:allow: PR-shaped test fixture, not a tracker reference assert_eq!( run_dispatch_target(&run, "fallback.yml"), @@ -258,4 +249,20 @@ mod tests { assert!(msg.contains("PR #4199")); // lint:allow: PR-shaped test fixture, not a tracker reference assert!(msg.contains("web UI")); } + + /// `resolve_run` (untested here — does I/O) bails with exactly this + /// message once it recognizes a run's dispatch target as a PR pseudo- + /// ref: this pins the same discrimination (`pr_number_from_run_ref`) + /// and the same message constructor (`pr_refusal_message`) that `--pr` + /// uses, so `--run` against a PR-triggered run tells the same story as + /// `--pr` against that run's own PR — not a second, differently-worded + /// refusal for the identical underlying defect. + #[test] + fn run_triggered_by_pr_refuses_with_the_shared_message() { + let run = run_from(json!({ "prettyref": "#4199", "workflow_id": "ci.yml" })); // lint:allow: PR-shaped test fixture, not a tracker reference + let (_, branch) = run_dispatch_target(&run, "fallback.yml").unwrap(); + let pr = pr_number_from_run_ref(&branch) + .expect("a PR-triggered run's dispatch target is a PR pseudo-ref"); + assert_eq!(pr_refusal_message(pr), pr_refusal_message(4199)); // lint:allow: PR-shaped test fixture, not a tracker reference + } }