hive-forge: ci-rerun --run refuses on a PR-triggered run too

--run <id> 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 (#<n>) 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.
This commit is contained in:
atlas 2026-09-21 21:37:22 +02:00
commit 392f16cbc0
3 changed files with 55 additions and 47 deletions

View file

@ -20,8 +20,8 @@
//! - `--run <n>` → 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 `#<n>`
//! 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<u64>,
/// Re-run the same workflow on the same branch this run used. The run
/// number is the `runs/<n>` in the run-page URL. Mutually exclusive
/// with `--pr` / `--branch`.
/// number is the `runs/<n>` 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<u64>,
/// 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<String> {
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 (`#<n>`, 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 (`#<n>`, 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 `#<n>` 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 `#<n>` 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
}
}