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.
This commit is contained in:
parent
afdfce67ec
commit
92100ac1f8
2 changed files with 52 additions and 19 deletions
|
|
@ -103,7 +103,7 @@ hive-forge attach-comment 18042 /path/to/file # upload a file attachment to a c
|
|||
hive-forge attachment-get <uuid> # 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/<workflow>/dispatches {"ref":"<branch>"}`).
|
||||
Resolve the branch with exactly one of: `--pr <n>` (the PR's head
|
||||
branch), `--run <n>` (the same run number `ci-log` / `artifact-get`
|
||||
take — resolves the branch + workflow from that run), or `--branch
|
||||
<name>` (directly). `--workflow <file>` 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 <n>` (the same run number `ci-log` /
|
||||
`artifact-get` take — resolves the branch + workflow from that run) or
|
||||
`--branch <name>` (directly). `--pr <n>` 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 <file>` picks the workflow file for `--branch`
|
||||
(default `ci.yml`). Dispatch re-runs the whole workflow — there is no
|
||||
single-job variant.
|
||||
- `issue dependency <n> add <dep...>` / `pr dependency <n> add <dep...>`
|
||||
set "blocked by" links via Forgejo's dependency feature — the
|
||||
operator's preferred way to track blocking relationships over labels
|
||||
|
|
|
|||
|
|
@ -12,18 +12,18 @@
|
|||
//!
|
||||
//! The branch (and, for `--run`, the workflow file) is resolved from the
|
||||
//! given handle:
|
||||
//! - `--pr <n>` → the PR's head branch; dispatches `--workflow` (default
|
||||
//! `ci.yml`) on it.
|
||||
//! - `--pr <n>` → 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 <name>` → dispatches `--workflow` on that branch directly.
|
||||
//! - `--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
|
||||
//! 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 <n>, --run <n>, or --branch <name>")
|
||||
}
|
||||
};
|
||||
|
|
@ -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<String> {
|
||||
|
|
@ -156,7 +172,7 @@ fn pr_number_from_run_ref(value: &str) -> Option<u64> {
|
|||
|
||||
#[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"));
|
||||
}
|
||||
}
|
||||
|
|
|
|||
Loading…
Reference in a new issue