fix(ci-log): address review — prose tags, correct module doc, typed miss

- Remove the two issue-number tags from Rust comments (tracker-tag lint).
- Correct the module doc: the durable download is keyed by the per-repo
  run NUMBER + attempt segment (matches the code + persisted_logs doc),
  not a global id — the earlier 'global id' wording was stale.
- Replace the fragile e.to_string().contains("out of range") branch with
  a typed StreamerMiss enum (StepOutOfRange vs Unavailable), so a usage
  error can never silently fall through to the persisted-log path if a
  message string changes.

docs/tools/forge.md

@@ -165,16 +165,21 @@ plain comment show under `last comment`, not `reviews`.
   the run's internal global id, so the verb translates the run number
   first. Saves a zip to `/tmp/forge-artifact-<name>.zip` by default; pass
   `-o -` to stream to stdout.
-- `ci-log --run <n> [--job i] [--step i]` prints a CI run's job step
+- `ci-log --run <n> [--job i] [--step i] [--attempt n]` prints a CI
-  logs. `<n>` is the run number from the run-page URL (same value
+  run's job step logs. `<n>` is the run number from the run-page URL
-  `artifact-get` takes; `pr-status` surfaces it as a CI context's
+  (same value `artifact-get` takes; `pr-status` surfaces it as a CI
-  target_url). Forgejo exposes no REST endpoint for job logs, so the
+  context's target_url). Two log sources are tried in order: first the
-  verb drives the web run-view streamer the run page polls. `--job`
+  web run-view **streamer** the run page polls (rich per-step framing,
-  selects the job within the run (0-based, default 0); `--step` narrows
+  honors `--step`) — but that reads the live `act_runner` task record,
-  to one step. `act_runner` garbage-collects completed-run logs, so this
+  which Forgejo prunes once a run completes; then, when the streamer is
-  is reliable for live + recently-finished runs; when logs are gone the
+  pruned (500 / no lines), the **durable persisted-log download** the
-  verb says so rather than printing nothing. `--json` dumps the raw
+  run page's "view raw logs" link uses
-  run-view response.
+  (`…/runs/<n>/jobs/<job>/attempt/<a>/logs`), a flat whole-job log that
+  survives the prune (`--step` is not honored on this path). So quick /
+  older runs that the streamer can no longer serve still print instead
+  of erroring. `--job` selects the job (0-based, default 0); `--attempt`
+  picks the run attempt for the durable path (default 1; re-runs
+  increment it). `--json` wraps the output.
 - `ci-rerun` re-runs CI without pushing an empty commit (the old
   retrigger path, which littered PR history). Forgejo has no token-usable
   REST endpoint to re-run an *existing* run (the run-page rerun buttons

hive-forge/src/verbs/ci_log.rs

@@ -1,23 +1,30 @@
 //! `ci-log --run <run-number> [--job <idx>] [--step <idx>]` — print a CI
 //! Actions run's job step logs.
 //!
-//! Forgejo exposes no REST endpoint for Actions job logs; the only path
+//! Two log sources, tried in order, because each covers a different
-//! is the web run-view streamer the run page polls,
+//! lifecycle window:
-//! `POST <base>/<owner>/<repo>/actions/runs/<run>/jobs/<job>` with a
-//! `{"logCursors":[{"step":N,"cursor":C,"expanded":bool}]}` body. Unlike
-//! the artifact-download route (which keys off the run's GLOBAL id), this
-//! route uses the per-repo run NUMBER — the `runs/<n>` the UI shows and
-//! `pr-status` surfaces as a CI context `target_url` — so the caller's
-//! `--run` value is used directly, no id translation.
 //!
-//! Protocol: POST once with an empty cursor set to fetch the run state
+//! 1. **Live run-view streamer** — `POST <base>/<owner>/<repo>/actions/runs/
-//! and enumerate the job's steps, then POST again requesting each step's
+//!    <run>/jobs/<job>` with a `{"logCursors":[{"step":N,"cursor":C,
-//! log from cursor 0 (`expanded: true`) and print the assembled lines.
+//!    "expanded":bool}]}` body, the same endpoint the run page polls. It
+//!    gives rich per-step framing but reads the **live `act_runner` task
+//!    record**, which Forgejo prunes once a run completes — so a run that
+//!    finished a while ago returns `500 "task ... resource does not exist"`
+//!    or no lines. Keyed by the per-repo run NUMBER (the `runs/<n>` the UI
+//!    shows and `pr-status` surfaces as a CI context `target_url`).
+//! 2. **Durable persisted-log download** — when the streamer is pruned the
+//!    step-log files still exist on disk (the web UI reads them fine after
+//!    the streamer 500s). We fetch them via the web download route the run
+//!    page's "view raw logs" link uses,
+//!    `<base>/<owner>/<repo>/actions/runs/<run>/jobs/<job>/attempt/<n>/logs`,
+//!    keyed by the per-repo run NUMBER (same value `--run` takes) with the
+//!    attempt segment. This is a flat whole-job log with no per-step framing,
+//!    so `--step` is not honored on this path.
 //!
-//! Caveat: `act_runner` garbage-collects step logs for completed runs, so
+//! Result: live + recent runs get the rich streamed view; older / quickly-
-//! a run that finished a while ago may return no log lines. The verb is
+//! pruned runs (where `act_runner` has already pruned the live task record)
-//! reliable for in-progress and recently-finished runs; when no lines
+//! still print via the durable fallback instead of a bare
-//! come back it says so rather than printing nothing.
+//! "logs garbage-collected" error.
 
 use anyhow::{Result, bail};
 use clap::Args as ClapArgs;
@@ -35,8 +42,14 @@ pub struct Args {
     #[arg(long, default_value_t = 0)]
     job: u64,
     /// Print only this step's log (0-based). Omit to print every step.
+    /// Honored on the live-streamer path only; the durable persisted-log
+    /// fallback serves a flat whole-job log and ignores `--step`.
     #[arg(long)]
     step: Option<usize>,
+    /// Run attempt number for the durable persisted-log download (re-runs
+    /// increment it; default 1 covers the common single-attempt case).
+    #[arg(long, default_value_t = 1)]
+    attempt: u64,
 }
 
 /// Extract the step list from a run-view response
@@ -63,50 +76,46 @@ fn step_name(step: Option<&Value>) -> String {
     .to_owned()
 }
 
-/// # Errors
+/// Why the live streamer didn't produce logs, so `run` can tell a real usage
-///
+/// error (which must surface) from a pruned/empty run (which falls back to the
-/// Returns an error if the run/job can't be fetched (network, or a
+/// durable persisted-log download). Avoids brittle string-matching on the
-/// non-2xx such as `404` for an unknown run), if the run has no steps,
+/// error message to make that distinction.
-/// or if no log lines are returned (e.g. logs already garbage-collected).
+enum StreamerMiss {
-pub fn run(client: &Client, args: Args) -> Result<()> {
+    /// A requested `--step` index is past the job's step count — a usage
-    let repo = client.repo();
+    /// error; `run` surfaces it instead of masking it with the fallback.
-    let path = format!("/{repo}/actions/runs/{}/jobs/{}", args.run, args.job);
+    StepOutOfRange(String),
+    /// The streamer is reachable but has no log lines, OR a transport / HTTP
+    /// failure (e.g. the `500 "resource does not exist"` a pruned run
+    /// returns). Either way `run` falls back to the durable download.
+    Unavailable,
+}
 
+/// Try the live run-view streamer. `Ok(())` means it printed (or emitted JSON
+/// for) actual log lines. `Err(StreamerMiss::StepOutOfRange)` is a usage error
+/// to surface; `Err(StreamerMiss::Unavailable)` tells the caller to fall back
+/// to the durable persisted-log download.
+fn streamer_logs(client: &Client, path: &str, args: &Args) -> Result<(), StreamerMiss> {
     // 1. Discover the job's steps (state is returned regardless of cursors).
-    // A 404 (run/job gone) or 500 ("task ... resource does not exist", which
+    //    A transport error / pruned-task 500 surfaces here → Unavailable.
-    // the forge returns once act_runner has pruned a completed run's task
+    let view = client
-    // record) surfaces here as an Err — translate it into the same
+        .post_json_web(path, &json!({ "logCursors": [] }))
-    // logs-unavailable guidance rather than leaking the raw HTTP error.
+        .map_err(|_| StreamerMiss::Unavailable)?;
-    let view = match client.post_json_web(&path, &json!({ "logCursors": [] })) {
-        Ok(v) => v,
-        Err(e) => bail!(
-            "run #{} job {} unavailable — the run/job may not exist, or its logs were \
-             garbage-collected (act_runner prunes completed-run task records; ci-log \
-             is reliable on live + recently-finished runs). Underlying: {e}",
-            args.run,
-            args.job
-        ),
-    };
     let steps = steps_of(&view);
     if steps.is_empty() {
-        bail!(
+        return Err(StreamerMiss::Unavailable);
-            "run #{} job {} has no steps — run/job not found, or logs were \
-             garbage-collected (act_runner prunes completed-run logs). Try a \
-             live or recently-finished run.",
-            args.run,
-            args.job
-        );
     }
 
     // 2. Request the chosen step(s) from cursor 0, fully expanded.
     let want: Vec<usize> = match args.step {
-        Some(s) if s >= steps.len() => bail!(
+        Some(s) if s >= steps.len() => {
-            "step {s} out of range — run #{} job {} has {} step(s) (0..{})",
+            return Err(StreamerMiss::StepOutOfRange(format!(
-            args.run,
+                "step {s} out of range — run #{} job {} has {} step(s) (0..{})",
-            args.job,
+                args.run,
-            steps.len(),
+                args.job,
-            steps.len() - 1
+                steps.len(),
-        ),
+                steps.len() - 1
+            )));
+        }
         Some(s) => vec![s],
         None => (0..steps.len()).collect(),
     };
@@ -114,13 +123,10 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
         .iter()
         .map(|&s| json!({ "step": s, "cursor": 0, "expanded": true }))
         .collect();
-    let resp = client.post_json_web(&path, &json!({ "logCursors": cursors }))?;
+    let resp = client
+        .post_json_web(path, &json!({ "logCursors": cursors }))
+        .map_err(|_| StreamerMiss::Unavailable)?;
 
-    if client.json_mode() {
-        return crate::verbs::print_json(&resp);
-    }
-
-    // 3. Human output: a header per step, then its lines in order.
     let steps_log = resp
         .get("logs")
         .and_then(|l| l.get("stepsLog"))
@@ -128,14 +134,15 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
         .cloned()
         .unwrap_or_default();
     if steps_log.is_empty() {
-        bail!(
+        return Err(StreamerMiss::Unavailable);
-            "no log lines returned for run #{} job {} — logs were likely \
-             garbage-collected (act_runner prunes completed-run logs). ci-log \
-             is reliable on live and recently-finished runs.",
-            args.run,
-            args.job
-        );
     }
+
+    if client.json_mode() {
+        crate::verbs::print_json(&resp).map_err(|_| StreamerMiss::Unavailable)?;
+        return Ok(());
+    }
+
+    // Human output: a header per step, then its lines in order.
     for sl in &steps_log {
         let idx = sl
             .get("step")
@@ -155,6 +162,69 @@ pub fn run(client: &Client, args: Args) -> Result<()> {
     Ok(())
 }
 
+/// Durable fallback: download the persisted whole-job log via the web route
+/// the run page's "view raw logs" link uses,
+/// `<base>/<owner>/<repo>/actions/runs/<run>/jobs/<job>/attempt/<n>/logs`
+/// (keyed by the per-repo run NUMBER, with the attempt segment). Prints the
+/// text (or wraps it in JSON under `--json`). `--step` is not honored here —
+/// the persisted log is flat.
+fn persisted_logs(client: &Client, repo: &str, args: &Args) -> Result<()> {
+    let url = client.web_url(&format!(
+        "/{repo}/actions/runs/{}/jobs/{}/attempt/{}/logs",
+        args.run, args.job, args.attempt
+    ));
+    let bytes = client.get_bytes_raw(&url)?;
+    if bytes.is_empty() {
+        bail!(
+            "run #{} job {} returned an empty log from both the live streamer \
+             and the persisted-log download — the run may not exist or its logs \
+             were fully purged.",
+            args.run,
+            args.job
+        );
+    }
+    let text = String::from_utf8_lossy(&bytes);
+    if client.json_mode() {
+        return crate::verbs::print_json(&json!({
+            "run": args.run,
+            "job": args.job,
+            "source": "persisted",
+            "log": text,
+        }));
+    }
+    print!("{text}");
+    if !text.ends_with('\n') {
+        println!();
+    }
+    Ok(())
+}
+
+/// # Errors
+///
+/// Returns an error if the run/job can't be fetched from either source
+/// (network, an unknown run number, or a non-2xx), if a requested `--step`
+/// is out of range, or if both the live streamer and the persisted-log
+/// download yield no log content.
+pub fn run(client: &Client, args: Args) -> Result<()> {
+    let repo = client.repo();
+    let stream_path = format!("/{repo}/actions/runs/{}/jobs/{}", args.run, args.job);
+
+    // 1. Live run-view streamer first — rich per-step framing for in-progress
+    //    and recently-finished runs.
+    match streamer_logs(client, &stream_path, &args) {
+        Ok(()) => return Ok(()),
+        // An out-of-range `--step` is a real usage error, not a pruned run —
+        // surface it instead of masking it with the fallback.
+        Err(StreamerMiss::StepOutOfRange(msg)) => bail!(msg),
+        // Pruned task or reachable-but-no-lines → fall through to the durable
+        // download (survives the act_runner task prune that 500s the streamer).
+        Err(StreamerMiss::Unavailable) => {}
+    }
+
+    // 2. Durable persisted-log download (the pruned-run case this verb fixes).
+    persisted_logs(client, repo, &args)
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;