diff --git a/hive-forge/src/client.rs b/hive-forge/src/client.rs index 5e79fa48..62fa83f7 100644 --- a/hive-forge/src/client.rs +++ b/hive-forge/src/client.rs @@ -7,7 +7,7 @@ use std::path::PathBuf; -use anyhow::{Context, Result, bail}; +use anyhow::{Context, Result, anyhow, bail}; use forgejo_api::{Auth, ForgejoError}; use reqwest::blocking::{Client as HttpClient, Response}; 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. forge_label: Option, /// Default repo used when a verb doesn't carry an explicit - /// `[repo]` override. - pub default_repo: String, + /// `[repo]` override. `None` when nothing resolved — resolution is + /// 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, /// Global `--json` flag — verbs that have a human-readable /// default path branch on `client.json_mode()` to pick the /// JSON output shape instead. @@ -55,10 +58,12 @@ impl Client { /// [`infer_repo_from_cwd`]); the `HIVE_FORGE_REPO` env var (kept as /// 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). - /// No repo resolving from any of those is an error rather than a - /// silent default — a single hardcoded fallback repo was exactly - /// what made agents assume it was "the only repo they're allowed to - /// operate on". + /// No repo resolving from any of those is **not** an error here — a + /// single hardcoded fallback repo was exactly what made agents + /// assume it was "the only repo they're allowed to operate on", and + /// 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 /// output formatters key off it via `Client::json_mode`. @@ -71,15 +76,9 @@ impl Client { forge_label: Option, ) -> Result { 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(|| 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" - ) - }; + .or_else(|| std::env::var("HIVE_FORGE_REPO").ok()); 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) @@ -147,15 +146,26 @@ impl Client { /// Return the active repo. `from_env` already folded the /// `-r/--repo` override into `default_repo`, so verbs just read /// it as-is — no per-verb override plumbing. - #[must_use] - pub fn repo(&self) -> &str { - &self.default_repo + /// + /// # Errors + /// + /// 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 /// client's per-segment path arguments. 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. diff --git a/hive-forge/src/main.rs b/hive-forge/src/main.rs index 537d441f..1b5cdbfc 100644 --- a/hive-forge/src/main.rs +++ b/hive-forge/src/main.rs @@ -229,8 +229,17 @@ fn run() -> Result<()> { // 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 // 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<()> { diff --git a/hive-forge/src/verbs/artifact_get.rs b/hive-forge/src/verbs/artifact_get.rs index 3edce3bf..59bc4ca9 100644 --- a/hive-forge/src/verbs/artifact_get.rs +++ b/hive-forge/src/verbs/artifact_get.rs @@ -82,7 +82,7 @@ fn resolve_run_id(client: &Client, repo: &str, run_number: u64) -> Result { /// 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. 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 url = client.web_url(&format!( "/{repo}/actions/runs/{run_id}/artifacts/{}", diff --git a/hive-forge/src/verbs/ci_log.rs b/hive-forge/src/verbs/ci_log.rs index 33312182..a918d6ce 100644 --- a/hive-forge/src/verbs/ci_log.rs +++ b/hive-forge/src/verbs/ci_log.rs @@ -201,7 +201,7 @@ fn persisted_logs(client: &Client, repo: &str, args: &Args) -> Result { /// (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. 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); // `--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 \ landed.", 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(""), ), // The existence lookup itself failed (network, auth) — don't let a // secondary failure mask the original "no log" finding. diff --git a/hive-forge/src/verbs/ci_rerun.rs b/hive-forge/src/verbs/ci_rerun.rs index 82cec1a8..9aeab801 100644 --- a/hive-forge/src/verbs/ci_rerun.rs +++ b/hive-forge/src/verbs/ci_rerun.rs @@ -64,7 +64,7 @@ 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<()> { - let repo = client.repo(); + 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)?, diff --git a/hive-forge/src/verbs/ci_runs.rs b/hive-forge/src/verbs/ci_runs.rs index ac46e920..6f36e0d7 100644 --- a/hive-forge/src/verbs/ci_runs.rs +++ b/hive-forge/src/verbs/ci_runs.rs @@ -56,7 +56,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> { } if runs.is_empty() { - println!("no matching runs in {}", client.repo()); + println!("no matching runs in {}", client.repo()?); return Ok(()); } for run in &runs { diff --git a/hive-forge/src/verbs/clone.rs b/hive-forge/src/verbs/clone.rs index 9409caa0..b0c2a555 100644 --- a/hive-forge/src/verbs/clone.rs +++ b/hive-forge/src/verbs/clone.rs @@ -39,7 +39,7 @@ pub struct Args { /// Returns an error if the `git clone` shellout fails (bad repo, auth /// rejected, network) or the destination can't be derived. pub fn run(client: &Client, args: Args) -> Result<()> { - let repo = client.repo(); + let repo = client.repo()?; let dest = match args.dest { Some(d) => d, None => repo diff --git a/hive-forge/src/verbs/comment.rs b/hive-forge/src/verbs/comment.rs index 0f2e4297..01a278c9 100644 --- a/hive-forge/src/verbs/comment.rs +++ b/hive-forge/src/verbs/comment.rs @@ -45,7 +45,7 @@ pub struct Args { /// writing the response to stdout. pub fn run(client: &Client, args: Args) -> Result<()> { 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 { // Degrade open: a transport failure checking notifications must diff --git a/hive-forge/src/verbs/comments.rs b/hive-forge/src/verbs/comments.rs index 1d7a3190..65678aeb 100644 --- a/hive-forge/src/verbs/comments.rs +++ b/hive-forge/src/verbs/comments.rs @@ -67,7 +67,7 @@ pub struct Args { } 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. // A `--tail` window has nothing after it (it ends at the thread's // current end); a `--limit`/default window has nothing before it (it diff --git a/hive-forge/src/verbs/pr_assign_reviewer.rs b/hive-forge/src/verbs/pr_assign_reviewer.rs index 3a0a1cc7..a36d941b 100644 --- a/hive-forge/src/verbs/pr_assign_reviewer.rs +++ b/hive-forge/src/verbs/pr_assign_reviewer.rs @@ -55,7 +55,7 @@ pub fn run(client: &Client, args: Args) -> Result<()> { // Guard the add path only — withdrawing a request is always safe. // Re-requesting from someone who already reviewed dismisses that review // 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() .find(|r| r.login == args.user && !r.superseded()) { diff --git a/hive-forge/src/verbs/pr_merge.rs b/hive-forge/src/verbs/pr_merge.rs index 1f450b4c..cdd99970 100644 --- a/hive-forge/src/verbs/pr_merge.rs +++ b/hive-forge/src/verbs/pr_merge.rs @@ -73,7 +73,7 @@ pub struct Args { /// merge POST itself returns a non-2xx (e.g. Forgejo `405` when the PR cannot /// be merged). pub fn run(client: &Client, args: Args) -> Result<()> { - let repo = client.repo(); + let repo = client.repo()?; let (owner, name) = client.owner_repo()?; let idx = index(args.number)?; let pull = client diff --git a/hive-forge/src/verbs/pr_status.rs b/hive-forge/src/verbs/pr_status.rs index 2d44014a..68c38727 100644 --- a/hive-forge/src/verbs/pr_status.rs +++ b/hive-forge/src/verbs/pr_status.rs @@ -36,7 +36,7 @@ pub struct Args { /// not-ready verdict is NOT an error — it's reported and reflected in /// the process exit code instead. pub fn run(client: &Client, args: Args) -> Result<()> { - let repo = client.repo(); + let repo = client.repo()?; match (args.pr, args.sha) { (Some(pr), _) => pr_status(client, repo, pr), (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 /// pure `Value` walkers. fn fetch_combined(client: &Client, sha: &str) -> Result<(String, Vec)> { - let combined = fetch_combined_in(client, client.repo(), sha)?; + let combined = fetch_combined_in(client, client.repo()?, sha)?; Ok((combined.state, combined.statuses)) } diff --git a/hive-forge/src/verbs/repo_add_collaborator.rs b/hive-forge/src/verbs/repo_add_collaborator.rs index ce00d236..2784b661 100644 --- a/hive-forge/src/verbs/repo_add_collaborator.rs +++ b/hive-forge/src/verbs/repo_add_collaborator.rs @@ -63,7 +63,7 @@ pub struct Args { /// permission on the repo, token missing/invalid) and any I/O error from /// writing the confirmation to stdout. pub fn run(client: &Client, args: Args) -> Result<()> { - let repo = client.repo(); + let repo = client.repo()?; let (owner, name) = client.owner_repo()?; let perm = args.permission.as_api(); client diff --git a/hive-forge/src/verbs/view.rs b/hive-forge/src/verbs/view.rs index 63fc226c..8cc19e01 100644 --- a/hive-forge/src/verbs/view.rs +++ b/hive-forge/src/verbs/view.rs @@ -16,7 +16,7 @@ pub struct Args { } 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 // read-before-comment guard (in `comment`) lets a reply through. notify::mark_read_best_effort(client, repo, args.number);