One module for how a Run's ending shows and how a child Run's is read back

src/base_fix.rs

@@ -32,10 +32,6 @@ const LABELS: [(&str, &str); 2] = [
     (READY_FOR_AGENT, "Ready for an agent to take on"),
 ];
 
-/// What starts the line on stderr saying what became of the Base fix a Run
-/// took, which a Spec run reads back from a Ticket's Run.
-pub const REPORT: &str = "Base fix: ";
-
 /// What a Run asks about a Base fix, by its command or, without `base-fix`
 /// or `no-base-fix`, by the User config.
 #[derive(Debug, Clone, PartialEq, Eq)]
@@ -75,9 +71,7 @@ pub struct Advice {
 }
 
 impl Advice {
-    /// Whether `message`, a line a Run printed on stderr, is one of these:
-    /// a child Run's is relayed, but is neither its cause nor its session
-    /// log.
+    /// Whether `message`, a line a Run printed on stderr, is one of these.
     pub fn is_line(message: &str) -> bool {
         [BASE_CHECK, RETRY_WITH, OR_SET].iter().any(|label| {
             message

src/child_run.rs

@@ -13,11 +13,11 @@ use std::time::Duration;
 use anyhow::{Context, Result, anyhow};
 
 use crate::args;
-use crate::base_fix::{self, Advice, BaseFixAsk};
-use crate::failed_run;
+use crate::base_fix::BaseFixAsk;
 use crate::interrupt;
 use crate::issue::IssueUrl;
 use crate::progress;
+use crate::run_ending;
 
 /// How often to check whether a child Run has ended, or been interrupted.
 const POLL: Duration = Duration::from_millis(100);
@@ -160,28 +160,17 @@ pub fn name_this_process() {
 /// child, which is waited for as it goes down its Failed run path, and it
 /// ended `Interrupted`. Otherwise it reached its goal if it exits 0, with
 /// the Base fix it reported, and failed otherwise, with the cause and
-/// session log it ended on.
+/// session log it showed, or with its exit status as the cause if it showed
+/// none.
 pub fn wait(number: u64, mut child: Child) -> Result<Ended> {
     // Relay on its own thread, so this one can watch for an interrupt.
     let stderr = child.stderr.take().context("no stderr from the Run")?;
-    // A failed Run ends on its error, then its session log if it has one,
-    // with any advice between them.
     let relay = thread::spawn(move || -> std::io::Result<_> {
-        let mut last_lines: [Option<String>; 2] = [None, None];
-        let mut base_fix = None;
+        let mut reader = run_ending::Reader::default();
         for line in BufReader::new(stderr).lines() {
-            let line = line?;
-            let message = progress::relay(format_args!("#{number}"), &line).to_string();
-            if let Some(report) = message.strip_prefix(base_fix::REPORT) {
-                base_fix = Some(report.to_string());
-            }
-            // Between its cause and its session log, and neither.
-            if Advice::is_line(&message) {
-                continue;
-            }
-            last_lines = [last_lines[1].take(), Some(message)];
+            reader.read(progress::relay(number, &line?));
         }
-        Ok((last_lines, base_fix))
+        Ok(reader)
     });
     let mut passed_on = false;
     let status = loop {
@@ -202,7 +191,7 @@ pub fn wait(number: u64, mut child: Child) -> Result<Ended> {
         }
         thread::sleep(POLL);
     };
-    let (last_lines, base_fix) = relay
+    let reader = relay
         .join()
         .map_err(|_| anyhow!("the relay of the Run for #{number} panicked"))?
         .with_context(|| format!("could not read the Run for #{number}"))?;
@@ -213,26 +202,17 @@ pub fn wait(number: u64, mut child: Child) -> Result<Ended> {
         .context("no stdout from the Run")?
         .read_to_string(&mut stdout)
         .with_context(|| format!("could not read the Run for #{number}"))?;
-    if status.success() {
-        return Ok(Ended::Reached {
-            pr_url: stdout.lines().last().map(String::from),
-            base_fix,
-        });
-    }
-    if interrupt::requested() {
-        return Ok(Ended::Interrupted);
-    }
-    let [before, last] = last_lines;
-    let (cause, log) = match last {
-        Some(last) => match last.strip_prefix(failed_run::SESSION_LOG) {
-            Some(log) => (before, Some(log.to_string())),
-            None => (Some(last), None),
+    let ending = reader.finish(&stdout, status.success());
+    Ok(match ending.outcome {
+        Ok(pr_url) => Ended::Reached {
+            pr_url,
+            base_fix: ending.base_fix,
+        },
+        Err(_) if interrupt::requested() => Ended::Interrupted,
+        Err(failure) => Ended::Failed {
+            cause: failure.cause.unwrap_or_else(|| format!("the Run {status}")),
+            log: failure.log,
         },
-        None => (None, None),
-    };
-    Ok(Ended::Failed {
-        cause: cause.unwrap_or_else(|| format!("the Run {status}")),
-        log,
     })
 }
 

src/failed_run.rs

@@ -13,10 +13,6 @@ use crate::issue::IssueUrl;
 use crate::progress;
 use crate::worktree::Worktree;
 
-/// What starts the line naming a Failed run's session log on stderr, which
-/// a Spec run reads back from a Ticket's Run.
-pub const SESSION_LOG: &str = "session log: ";
-
 /// Why a Run did not end with a ready PR, and what the user should see.
 pub struct FailedRun {
     pub error: anyhow::Error,

src/main.rs

@@ -23,21 +23,20 @@ mod prompts_page;
 mod questions;
 mod resend_key;
 mod run;
+mod run_ending;
 mod session;
 mod spec_run;
 mod update;
 mod worktree;
 
-use std::io::Write;
 use std::process::ExitCode;
 
 use architect::{Outcome, Reviewed};
 use args::{ArchitectArgs, Command, RunArgs};
-use base_fix::{Advice, BaseFixAsk};
+use base_fix::BaseFixAsk;
 use config::UserConfig;
-use failed_run::FailedRun;
 use notification::{ArchitectNotification, NotificationAsk, RunNotification};
-use run::{Ended, Goal, StartedBy};
+use run::{Goal, StartedBy};
 use spec_run::Parallel;
 
 const HELP: &str = "\
@@ -274,7 +273,7 @@ fn main() -> ExitCode {
         child.as_ref().map_or(StartedBy::Command, StartedBy::Child),
         base_fix,
     );
-    let code = run_outcome(&ended);
+    let code = run_ending::show(&ended);
     if let Some(notification) = notification {
         notification.send(&ended);
     }
@@ -347,14 +346,9 @@ fn architect(args: ArchitectArgs) -> ExitCode {
         _ => None,
     };
     let code = match (&ended, &dispatched) {
-        (_, Some(dispatched)) => run_outcome(dispatched),
-        (Ok(outcome), None) => {
-            // Also on stderr, so the outcome shows even when stdout is captured.
-            progress::step(format_args!("{outcome}"));
-            outcome.urls().into_iter().for_each(print_url);
-            ExitCode::SUCCESS
-        }
-        (Err(failed), None) => report(failed, &[]),
+        (_, Some(dispatched)) => run_ending::show(dispatched),
+        (Ok(outcome), None) => run_ending::show_architect(outcome),
+        (Err(failed), None) => run_ending::show_failure(failed),
     };
     if let Some(notification) = notification {
         notification.send(&ended, dispatched.as_ref());
@@ -374,29 +368,6 @@ fn asked<N>(
     }
 }
 
-/// How a Run or a Spec run that `ended` shows: what became of its Base fix,
-/// if it took one, then its pull request's URL on stdout once it reached its
-/// goal, or as a Failed run does.
-fn run_outcome(ended: &Ended) -> ExitCode {
-    // Before the outcome, which a failed child Run's last lines are read as.
-    if let Some(report) = &ended.base_fix {
-        progress::step(format_args!("{}{report}", base_fix::REPORT));
-    }
-    match &ended.outcome {
-        Ok(reached) => {
-            // Also on stderr, so the outcome shows even when stdout is captured.
-            progress::step(format_args!(
-                "PR {} is {}",
-                reached.pr_url,
-                reached.goal.outcome()
-            ));
-            print_url(&reached.pr_url);
-            ExitCode::SUCCESS
-        }
-        Err(failed) => report(failed, &ended.advice),
-    }
-}
-
 /// The User config, after offering Setup where there is none, or the failure
 /// to exit with, its error reported.
 fn user_config() -> Result<UserConfig, ExitCode> {
@@ -411,30 +382,6 @@ fn failure(error: &anyhow::Error) -> ExitCode {
     ExitCode::FAILURE
 }
 
-/// How a Failed run, or a failed Architect run, shows: its cause, its
-/// `advice`, if it has any, and its session log on stderr, and its pull
-/// request's URL, if it left one, on stdout.
-fn report(failed: &FailedRun, advice: &[Advice]) -> ExitCode {
-    progress::step(format_args!("{:#}", failed.error));
-    for line in advice {
-        progress::step(line);
-    }
-    if let Some(log) = &failed.log {
-        progress::step(format_args!("{}{}", failed_run::SESSION_LOG, log.display()));
-    }
-    if let Some(pr_url) = &failed.pr_url {
-        print_url(pr_url);
-    }
-    ExitCode::FAILURE
-}
-
-/// A pull request's URL, or the URL of the issue an Architect run ended on,
-/// on stdout. A failed write, as once the terminal has closed, is ignored, so
-/// the Run notification still goes.
-fn print_url(url: &str) {
-    let _ = writeln!(std::io::stdout(), "{url}");
-}
-
 /// The end of a command other than a Run: the line that says how it went,
 /// or its error, on stderr.
 fn outcome(result: anyhow::Result<impl std::fmt::Display>) -> ExitCode {

src/progress.rs

@@ -21,30 +21,77 @@ const STAMP: &str = "%H:%M:%S";
 
 /// Print one of thirdshift's own steps, stamped with the time now.
 pub fn step(message: impl Display) {
-    write_line(Local::now().format(STAMP), message);
+    write_line(&stamped(message));
 }
 
-/// Print `line`, from the stderr of a child thirdshift, under `label`,
-/// keeping the time the child stamped it with (or stamping it now, if it
-/// has none), and return its message: the line without prefix or time.
-pub fn relay(label: impl Display, line: &str) -> &str {
-    let line = line.strip_prefix(PREFIX).unwrap_or(line);
-    match split_stamp(line) {
-        Some((time, message)) => {
-            write_line(time, format_args!("{label}: {message}"));
-            message
-        }
-        None => {
-            step(format_args!("{label}: {line}"));
-            line
+/// The progress line [`step`] prints for `message`.
+pub fn stamped(message: impl Display) -> String {
+    progress_line(Local::now().format(STAMP), message)
+}
+
+/// What a line from the stderr of a child thirdshift is.
+#[derive(Debug, PartialEq, Eq)]
+pub enum ChildLine<'a> {
+    /// One of the child's own progress lines, with its message: the line
+    /// without prefix or time.
+    Own(&'a str),
+    /// A progress line the child relayed from a child of its own.
+    Relayed,
+    /// A line that continues the one before it, as the later lines of an
+    /// error of several lines do.
+    Continuation,
+}
+
+impl<'a> ChildLine<'a> {
+    /// What `line` is: a progress line if it starts with their prefix,
+    /// relayed if its message then starts as [`relayed`] starts one.
+    pub fn of(line: &'a str) -> Self {
+        let Some(rest) = line.strip_prefix(PREFIX) else {
+            return ChildLine::Continuation;
+        };
+        let message = split_stamp(rest).map_or(rest, |(_, message)| message);
+        let from_its_own_child = message
+            .strip_prefix('#')
+            .and_then(|rest| rest.split_once(": "))
+            .is_some_and(|(number, _)| {
+                !number.is_empty() && number.bytes().all(|byte| byte.is_ascii_digit())
+            });
+        if from_its_own_child {
+            ChildLine::Relayed
+        } else {
+            ChildLine::Own(message)
         }
     }
 }
 
-fn write_line(time: impl Display, message: impl Display) {
+/// Print `line`, from the stderr of the child thirdshift running issue
+/// `number`, as [`relayed`] gives it, and return what the line is.
+pub fn relay(number: u64, line: &str) -> ChildLine<'_> {
+    write_line(&relayed(number, line));
+    ChildLine::of(line)
+}
+
+/// The progress line that relays `line`, from the stderr of the child
+/// thirdshift running issue `number`: its message after `#<number>: `,
+/// keeping the time the child stamped it with (or stamped now, if it has
+/// none).
+pub fn relayed(number: u64, line: &str) -> String {
+    let rest = line.strip_prefix(PREFIX).unwrap_or(line);
+    match split_stamp(rest) {
+        Some((time, message)) => progress_line(time, format_args!("#{number}: {message}")),
+        None => stamped(format_args!("#{number}: {rest}")),
+    }
+}
+
+/// The progress line for `message`, stamped with `time`.
+fn progress_line(time: impl Display, message: impl Display) -> String {
+    format!("{PREFIX}{time} {message}")
+}
+
+fn write_line(line: &str) {
     // Ignored if it fails, as it does once the terminal has closed: the Run
     // still has to clean up and send its Run notification.
-    let _ = writeln!(std::io::stderr(), "{PREFIX}{time} {message}");
+    let _ = writeln!(std::io::stderr(), "{line}");
 }
 
 /// A line without its prefix split into its `HH:MM:SS` stamp and message,
@@ -569,6 +616,61 @@ mod tests {
         }
     }
 
+    #[test]
+    fn a_relayed_line_keeps_its_time_under_the_issues_number() {
+        assert_eq!(
+            relayed(21, "thirdshift: 12:14:49 could not push issue-21"),
+            "thirdshift: 12:14:49 #21: could not push issue-21"
+        );
+    }
+
+    #[test]
+    fn a_relayed_line_with_no_time_is_stamped_now() {
+        let line = relayed(21, "hint: fetch first");
+        let (_, message) = split_stamp(line.strip_prefix(PREFIX).unwrap()).unwrap();
+        assert_eq!(message, "#21: hint: fetch first");
+    }
+
+    #[test]
+    fn a_childs_line_that_starts_with_the_prefix_is_its_own_progress_line() {
+        assert_eq!(
+            ChildLine::of("thirdshift: 12:14:49 could not push issue-21"),
+            ChildLine::Own("could not push issue-21")
+        );
+        assert_eq!(
+            ChildLine::of(&stamped("#21 failed: claude exited 1")),
+            ChildLine::Own("#21 failed: claude exited 1")
+        );
+    }
+
+    #[test]
+    fn a_childs_line_without_the_prefix_continues_the_one_before() {
+        for line in [
+            "hint: fetch first",
+            "12:14:49 thirdshift: pushing",
+            " thirdshift: 12:14:49 pushing",
+            "#8: claude exited 1",
+            "",
+        ] {
+            assert_eq!(ChildLine::of(line), ChildLine::Continuation, "{line}");
+        }
+    }
+
+    #[test]
+    fn a_line_a_child_relayed_is_not_its_own_progress_line() {
+        for line in [
+            "thirdshift: 12:14:49 claude exited 1",
+            "hint: fetch first",
+            "thirdshift: 12:14:49 #9: relayed before",
+        ] {
+            assert_eq!(
+                ChildLine::of(&relayed(8, line)),
+                ChildLine::Relayed,
+                "{line}"
+            );
+        }
+    }
+
     #[test]
     fn unknown_and_malformed_lines_are_skipped() {
         let mut progress = Progress::default();