hive-forge: defer repo resolution to the accessor, not construction
This commit is contained in:
parent
cc2503d9c7
commit
fe7bf81d4a
14 changed files with 57 additions and 35 deletions
|
|
@ -7,7 +7,7 @@
|
||||||
|
|
||||||
use std::path::PathBuf;
|
use std::path::PathBuf;
|
||||||
|
|
||||||
use anyhow::{Context, Result, bail};
|
use anyhow::{Context, Result, anyhow, bail};
|
||||||
use forgejo_api::{Auth, ForgejoError};
|
use forgejo_api::{Auth, ForgejoError};
|
||||||
use reqwest::blocking::{Client as HttpClient, Response};
|
use reqwest::blocking::{Client as HttpClient, Response};
|
||||||
use reqwest::header::{ACCEPT, AUTHORIZATION, CONTENT_TYPE, HeaderMap, HeaderValue};
|
use reqwest::header::{ACCEPT, AUTHORIZATION, CONTENT_TYPE, HeaderMap, HeaderValue};
|
||||||
|
|
@ -38,8 +38,11 @@ pub struct Client {
|
||||||
/// rather than silently falling back to the internal one.
|
/// rather than silently falling back to the internal one.
|
||||||
forge_label: Option<String>,
|
forge_label: Option<String>,
|
||||||
/// Default repo used when a verb doesn't carry an explicit
|
/// Default repo used when a verb doesn't carry an explicit
|
||||||
/// `[repo]` override.
|
/// `[repo]` override. `None` when nothing resolved — resolution is
|
||||||
pub default_repo: String,
|
/// deferred to [`Client::repo`] rather than failing here, so a
|
||||||
|
/// repo-independent verb (`repo-search`, `repo-create`) never needs
|
||||||
|
/// one to exist.
|
||||||
|
default_repo: Option<String>,
|
||||||
/// Global `--json` flag — verbs that have a human-readable
|
/// Global `--json` flag — verbs that have a human-readable
|
||||||
/// default path branch on `client.json_mode()` to pick the
|
/// default path branch on `client.json_mode()` to pick the
|
||||||
/// JSON output shape instead.
|
/// JSON output shape instead.
|
||||||
|
|
@ -55,10 +58,12 @@ impl Client {
|
||||||
/// [`infer_repo_from_cwd`]); the `HIVE_FORGE_REPO` env var (kept as
|
/// [`infer_repo_from_cwd`]); the `HIVE_FORGE_REPO` env var (kept as
|
||||||
/// a last-resort override for callers that aren't in any checkout —
|
/// a last-resort override for callers that aren't in any checkout —
|
||||||
/// nothing in this hive's nix config sets it, so it's opt-in only).
|
/// nothing in this hive's nix config sets it, so it's opt-in only).
|
||||||
/// No repo resolving from any of those is an error rather than a
|
/// No repo resolving from any of those is **not** an error here — a
|
||||||
/// silent default — a single hardcoded fallback repo was exactly
|
/// single hardcoded fallback repo was exactly what made agents
|
||||||
/// what made agents assume it was "the only repo they're allowed to
|
/// assume it was "the only repo they're allowed to operate on", and
|
||||||
/// operate on".
|
/// eagerly failing here made every verb inherit a requirement it
|
||||||
|
/// might not have. It's an error only when a repo-scoped verb
|
||||||
|
/// actually asks for one — see [`Client::repo`].
|
||||||
///
|
///
|
||||||
/// `json_mode` comes from the global `--json` flag — per-verb
|
/// `json_mode` comes from the global `--json` flag — per-verb
|
||||||
/// output formatters key off it via `Client::json_mode`.
|
/// output formatters key off it via `Client::json_mode`.
|
||||||
|
|
@ -71,15 +76,9 @@ impl Client {
|
||||||
forge_label: Option<String>,
|
forge_label: Option<String>,
|
||||||
) -> Result<Self> {
|
) -> Result<Self> {
|
||||||
let (base, token) = resolve_credentials(forge_label.as_deref())?;
|
let (base, token) = resolve_credentials(forge_label.as_deref())?;
|
||||||
let Some(default_repo) = repo_override
|
let default_repo = repo_override
|
||||||
.or_else(infer_repo_from_cwd)
|
.or_else(infer_repo_from_cwd)
|
||||||
.or_else(|| std::env::var("HIVE_FORGE_REPO").ok())
|
.or_else(|| std::env::var("HIVE_FORGE_REPO").ok());
|
||||||
else {
|
|
||||||
bail!(
|
|
||||||
"hive-forge: no repo specified — pass -r/--repo, run from inside a \
|
|
||||||
git checkout with an `origin` remote, or set HIVE_FORGE_REPO"
|
|
||||||
)
|
|
||||||
};
|
|
||||||
|
|
||||||
let url = url::Url::parse(&base).with_context(|| format!("parse HIVE_FORGE_URL {base}"))?;
|
let url = url::Url::parse(&base).with_context(|| format!("parse HIVE_FORGE_URL {base}"))?;
|
||||||
let api = forgejo_api::sync::Forgejo::new(Auth::Token(&token), url)
|
let api = forgejo_api::sync::Forgejo::new(Auth::Token(&token), url)
|
||||||
|
|
@ -147,15 +146,26 @@ impl Client {
|
||||||
/// Return the active repo. `from_env` already folded the
|
/// Return the active repo. `from_env` already folded the
|
||||||
/// `-r/--repo` override into `default_repo`, so verbs just read
|
/// `-r/--repo` override into `default_repo`, so verbs just read
|
||||||
/// it as-is — no per-verb override plumbing.
|
/// it as-is — no per-verb override plumbing.
|
||||||
#[must_use]
|
///
|
||||||
pub fn repo(&self) -> &str {
|
/// # Errors
|
||||||
&self.default_repo
|
///
|
||||||
|
/// Errors when nothing resolved a repo (see `from_env`'s doc). Only
|
||||||
|
/// a verb that actually calls this (directly or via
|
||||||
|
/// [`Client::owner_repo`]) can fail this way — `repo-search` and
|
||||||
|
/// `repo-create` never do, so they run with no repo at all.
|
||||||
|
pub fn repo(&self) -> Result<&str> {
|
||||||
|
self.default_repo.as_deref().ok_or_else(|| {
|
||||||
|
anyhow!(
|
||||||
|
"hive-forge: no repo specified — pass -r/--repo, run from inside a \
|
||||||
|
git checkout with an `origin` remote, or set HIVE_FORGE_REPO"
|
||||||
|
)
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
/// The active repo split into `(owner, name)` for the typed
|
/// The active repo split into `(owner, name)` for the typed
|
||||||
/// client's per-segment path arguments.
|
/// client's per-segment path arguments.
|
||||||
pub fn owner_repo(&self) -> Result<(&str, &str)> {
|
pub fn owner_repo(&self) -> Result<(&str, &str)> {
|
||||||
split_repo(self.repo())
|
split_repo(self.repo()?)
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Build the full URL for a Forgejo attachment by UUID.
|
/// Build the full URL for a Forgejo attachment by UUID.
|
||||||
|
|
|
||||||
|
|
@ -229,8 +229,17 @@ fn run() -> Result<()> {
|
||||||
// from a transient flake; this turns "EOF while parsing a value at
|
// from a transient flake; this turns "EOF while parsing a value at
|
||||||
// line 1 column 0" into "repo typo-org/repo: EOF while parsing a
|
// line 1 column 0" into "repo typo-org/repo: EOF while parsing a
|
||||||
// value at line 1 column 0", which is diagnosable on sight.
|
// value at line 1 column 0", which is diagnosable on sight.
|
||||||
let repo = client.repo().to_owned();
|
//
|
||||||
dispatch(&client, verb).with_context(|| format!("repo {repo}"))
|
// `client.repo()` is fallible now (a repo-independent verb like
|
||||||
|
// `repo-search` may have none at all) — this wrapper must not
|
||||||
|
// become the thing that revives the eager-bail bug one frame down,
|
||||||
|
// so an unresolved repo just means no context is attached, not a
|
||||||
|
// failure. A verb that actually needs a repo still fails, from
|
||||||
|
// wherever it calls `client.repo()`/`client.owner_repo()` itself.
|
||||||
|
match client.repo().ok().map(str::to_owned) {
|
||||||
|
Some(repo) => dispatch(&client, verb).with_context(|| format!("repo {repo}")),
|
||||||
|
None => dispatch(&client, verb),
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
fn dispatch(client: &client::Client, verb: Verb) -> Result<()> {
|
fn dispatch(client: &client::Client, verb: Verb) -> Result<()> {
|
||||||
|
|
|
||||||
|
|
@ -82,7 +82,7 @@ fn resolve_run_id(client: &Client, repo: &str, run_number: u64) -> Result<i64> {
|
||||||
/// download fails (network, or a non-2xx status such as `404` for an
|
/// download fails (network, or a non-2xx status such as `404` for an
|
||||||
/// unknown artifact name), or if the output path can't be written.
|
/// unknown artifact name), or if the output path can't be written.
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
let run_id = resolve_run_id(client, repo, args.run)?;
|
let run_id = resolve_run_id(client, repo, args.run)?;
|
||||||
let url = client.web_url(&format!(
|
let url = client.web_url(&format!(
|
||||||
"/{repo}/actions/runs/{run_id}/artifacts/{}",
|
"/{repo}/actions/runs/{run_id}/artifacts/{}",
|
||||||
|
|
|
||||||
|
|
@ -201,7 +201,7 @@ fn persisted_logs(client: &Client, repo: &str, args: &Args) -> Result<bool> {
|
||||||
/// (network, an unknown run number, or a non-2xx), if a requested `--step`
|
/// (network, an unknown run number, or a non-2xx), if a requested `--step`
|
||||||
/// is out of range, or if both sources yield no log content.
|
/// is out of range, or if both sources yield no log content.
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
let stream_path = format!("/{repo}/actions/runs/{}/jobs/{}", args.run, args.job);
|
let stream_path = format!("/{repo}/actions/runs/{}/jobs/{}", args.run, args.job);
|
||||||
|
|
||||||
// `--step` only has meaning against the live streamer's per-step
|
// `--step` only has meaning against the live streamer's per-step
|
||||||
|
|
@ -259,7 +259,10 @@ fn no_log_message(client: &Client, args: &Args) -> String {
|
||||||
ones) or that the dispatch that was meant to create it actually \
|
ones) or that the dispatch that was meant to create it actually \
|
||||||
landed.",
|
landed.",
|
||||||
args.run,
|
args.run,
|
||||||
client.repo(),
|
// `run` already resolved a repo successfully before calling
|
||||||
|
// this far into the log lookup, so this can't actually be
|
||||||
|
// absent — the fallback text is defensive, not a real path.
|
||||||
|
client.repo().unwrap_or("<unknown repo>"),
|
||||||
),
|
),
|
||||||
// The existence lookup itself failed (network, auth) — don't let a
|
// The existence lookup itself failed (network, auth) — don't let a
|
||||||
// secondary failure mask the original "no log" finding.
|
// secondary failure mask the original "no log" finding.
|
||||||
|
|
|
||||||
|
|
@ -64,7 +64,7 @@ pub struct Args {
|
||||||
/// missing its ref), or if the dispatch POST fails (network, or a non-2xx
|
/// missing its ref), or if the dispatch POST fails (network, or a non-2xx
|
||||||
/// such as `404` for an unknown workflow file or branch).
|
/// such as `404` for an unknown workflow file or branch).
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
let (workflow, branch) = match (args.pr, args.run, args.branch.as_deref()) {
|
let (workflow, branch) = match (args.pr, args.run, args.branch.as_deref()) {
|
||||||
(Some(pr), _, _) => (args.workflow.clone(), branch_for_pr(client, pr)?),
|
(Some(pr), _, _) => (args.workflow.clone(), branch_for_pr(client, pr)?),
|
||||||
(_, Some(run), _) => resolve_run(client, repo, run, &args.workflow)?,
|
(_, Some(run), _) => resolve_run(client, repo, run, &args.workflow)?,
|
||||||
|
|
|
||||||
|
|
@ -56,7 +56,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
}
|
}
|
||||||
|
|
||||||
if runs.is_empty() {
|
if runs.is_empty() {
|
||||||
println!("no matching runs in {}", client.repo());
|
println!("no matching runs in {}", client.repo()?);
|
||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
for run in &runs {
|
for run in &runs {
|
||||||
|
|
|
||||||
|
|
@ -39,7 +39,7 @@ pub struct Args {
|
||||||
/// Returns an error if the `git clone` shellout fails (bad repo, auth
|
/// Returns an error if the `git clone` shellout fails (bad repo, auth
|
||||||
/// rejected, network) or the destination can't be derived.
|
/// rejected, network) or the destination can't be derived.
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
let dest = match args.dest {
|
let dest = match args.dest {
|
||||||
Some(d) => d,
|
Some(d) => d,
|
||||||
None => repo
|
None => repo
|
||||||
|
|
|
||||||
|
|
@ -45,7 +45,7 @@ pub struct Args {
|
||||||
/// writing the response to stdout.
|
/// writing the response to stdout.
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let body = body::resolve_required(args.body.as_deref(), args.body_file.as_deref(), "comment")?;
|
let body = body::resolve_required(args.body.as_deref(), args.body_file.as_deref(), "comment")?;
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
|
|
||||||
if !args.force {
|
if !args.force {
|
||||||
// Degrade open: a transport failure checking notifications must
|
// Degrade open: a transport failure checking notifications must
|
||||||
|
|
|
||||||
|
|
@ -67,7 +67,7 @@ pub struct Args {
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
// Truncation info: what sits outside the window we're about to show.
|
// Truncation info: what sits outside the window we're about to show.
|
||||||
// A `--tail` window has nothing after it (it ends at the thread's
|
// A `--tail` window has nothing after it (it ends at the thread's
|
||||||
// current end); a `--limit`/default window has nothing before it (it
|
// current end); a `--limit`/default window has nothing before it (it
|
||||||
|
|
|
||||||
|
|
@ -55,7 +55,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
// Guard the add path only — withdrawing a request is always safe.
|
// Guard the add path only — withdrawing a request is always safe.
|
||||||
// Re-requesting from someone who already reviewed dismisses that review
|
// Re-requesting from someone who already reviewed dismisses that review
|
||||||
// instead of no-op'ing (see the module doc-comment above).
|
// instead of no-op'ing (see the module doc-comment above).
|
||||||
if let Some(existing) = super::latest_reviews(client, client.repo(), args.number)?
|
if let Some(existing) = super::latest_reviews(client, client.repo()?, args.number)?
|
||||||
.into_iter()
|
.into_iter()
|
||||||
.find(|r| r.login == args.user && !r.superseded())
|
.find(|r| r.login == args.user && !r.superseded())
|
||||||
{
|
{
|
||||||
|
|
|
||||||
|
|
@ -73,7 +73,7 @@ pub struct Args {
|
||||||
/// merge POST itself returns a non-2xx (e.g. Forgejo `405` when the PR cannot
|
/// merge POST itself returns a non-2xx (e.g. Forgejo `405` when the PR cannot
|
||||||
/// be merged).
|
/// be merged).
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
let (owner, name) = client.owner_repo()?;
|
let (owner, name) = client.owner_repo()?;
|
||||||
let idx = index(args.number)?;
|
let idx = index(args.number)?;
|
||||||
let pull = client
|
let pull = client
|
||||||
|
|
|
||||||
|
|
@ -36,7 +36,7 @@ pub struct Args {
|
||||||
/// not-ready verdict is NOT an error — it's reported and reflected in
|
/// not-ready verdict is NOT an error — it's reported and reflected in
|
||||||
/// the process exit code instead.
|
/// the process exit code instead.
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
match (args.pr, args.sha) {
|
match (args.pr, args.sha) {
|
||||||
(Some(pr), _) => pr_status(client, repo, pr),
|
(Some(pr), _) => pr_status(client, repo, pr),
|
||||||
(None, Some(sha)) => sha_status(client, &sha),
|
(None, Some(sha)) => sha_status(client, &sha),
|
||||||
|
|
@ -192,7 +192,7 @@ pub(crate) fn fetch_combined_in(client: &Client, repo: &str, sha: &str) -> Resul
|
||||||
/// Statuses ride as their API-shape JSON so the render helpers stay
|
/// Statuses ride as their API-shape JSON so the render helpers stay
|
||||||
/// pure `Value` walkers.
|
/// pure `Value` walkers.
|
||||||
fn fetch_combined(client: &Client, sha: &str) -> Result<(String, Vec<Value>)> {
|
fn fetch_combined(client: &Client, sha: &str) -> Result<(String, Vec<Value>)> {
|
||||||
let combined = fetch_combined_in(client, client.repo(), sha)?;
|
let combined = fetch_combined_in(client, client.repo()?, sha)?;
|
||||||
Ok((combined.state, combined.statuses))
|
Ok((combined.state, combined.statuses))
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -63,7 +63,7 @@ pub struct Args {
|
||||||
/// permission on the repo, token missing/invalid) and any I/O error from
|
/// permission on the repo, token missing/invalid) and any I/O error from
|
||||||
/// writing the confirmation to stdout.
|
/// writing the confirmation to stdout.
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
let (owner, name) = client.owner_repo()?;
|
let (owner, name) = client.owner_repo()?;
|
||||||
let perm = args.permission.as_api();
|
let perm = args.permission.as_api();
|
||||||
client
|
client
|
||||||
|
|
|
||||||
|
|
@ -16,7 +16,7 @@ pub struct Args {
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn run(client: &Client, args: Args) -> Result<()> {
|
pub fn run(client: &Client, args: Args) -> Result<()> {
|
||||||
let repo = client.repo();
|
let repo = client.repo()?;
|
||||||
// Reading the thread clears its unread notification so the
|
// Reading the thread clears its unread notification so the
|
||||||
// read-before-comment guard (in `comment`) lets a reply through.
|
// read-before-comment guard (in `comment`) lets a reply through.
|
||||||
notify::mark_read_best_effort(client, repo, args.number);
|
notify::mark_read_best_effort(client, repo, args.number);
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue