Report Security pre-audit failures without claiming an audit ran

src/notification.rs

@@ -34,6 +34,7 @@ pub enum NotificationAsk {
 struct Checked {
     resend: Resend,
     started: Instant,
+    started_at: chrono::DateTime<chrono::Utc>,
 }
 
 impl Checked {
@@ -42,8 +43,13 @@ impl Checked {
     /// address is known and a Resend API key is found.
     fn new(to: Option<String>, settings: &EmailSettings) -> Result<Self> {
         let started = Instant::now();
+        let started_at = chrono::Utc::now();
         let resend = Resend::new(to, settings)?;
-        Ok(Checked { resend, started })
+        Ok(Checked {
+            resend,
+            started,
+            started_at,
+        })
     }
 }
 
@@ -82,7 +88,7 @@ pub struct RunNotification {
     checked: Checked,
     subject: Subject,
     /// The command's configured Harness, Model and Effort, once known.
-    built_with: Option<String>,
+    configured: Option<String>,
 }
 
 impl RunNotification {
@@ -111,7 +117,7 @@ impl RunNotification {
         Ok(RunNotification {
             checked,
             subject,
-            built_with: None,
+            configured: None,
         })
     }
 
@@ -129,7 +135,7 @@ impl RunNotification {
     /// Name the configured Harness, Model and Effort without claiming
     /// that the Harness confirmed which Model executed.
     pub fn built_with(&mut self, harness: &Choice) {
-        self.built_with = Some(format!("{} (configured)", harness.built_with()));
+        self.configured = Some(format!("{harness} (configured)"));
     }
 
     /// Send the one notification for the command that ended as `account`
@@ -146,11 +152,18 @@ impl RunNotification {
         let mut text = body(
             &self.subject,
             account,
-            self.built_with.as_deref(),
+            self.configured.as_deref(),
             command_log.as_deref(),
             host.as_deref().unwrap_or("unknown host"),
             self.checked.started.elapsed(),
         );
+        if account.pre_audit_failure.is_some() {
+            text += &format!(
+                "Started (UTC): {}\nDiagnostics:  Check this command's stderr on {} at that time (or the scheduler's captured stderr, if scheduled).\n",
+                self.checked.started_at.format("%Y-%m-%d %H:%M:%S UTC"),
+                host.as_deref().unwrap_or("unknown host"),
+            );
+        }
         let lines = crate::session::recording::Recording::capture().notification_lines();
         if !lines.is_empty() {
             text += "\nSession models:\n";
@@ -205,7 +218,7 @@ fn architect_subject(repo: Option<&Repo>, outcome: &str) -> String {
 }
 
 /// The plain-text body of the notification for a command that ended as
-/// `account` tells, about `subject`, with its configured `built_with` line, if known,
+/// `account` tells, about `subject`, with its `configured` Harness, Model and Effort, if known,
 /// naming `command_log`, if the command keeps one,
 /// and `host`, for a command that `took` so long. It starts with the command's
 /// outcome, as the subject gives it, then the original Issue URL when the
@@ -215,7 +228,7 @@ fn architect_subject(repo: Option<&Repo>, outcome: &str) -> String {
 fn body(
     subject: &Subject,
     account: &Account,
-    built_with: Option<&str>,
+    configured: Option<&str>,
     command_log: Option<&Path>,
     host: &str,
     took: Duration,
@@ -226,6 +239,7 @@ fn body(
     }
     if let Some(findings) = account.security_findings {
         let audit = match &account.ended {
+            Err(_) if account.pre_audit_failure.is_some() => "not started",
             Err(_) if account.failed_reproduction.is_some() => "audit complete",
             Ok(line) => line.as_str(),
             Err(_) => account.outcome,
@@ -273,12 +287,14 @@ fn body(
         text += &format!("Pull request: {pr_url}\n");
     }
     // Security failure causes can quote private verifier work or Harness
-    // diagnostics. Only the fixed safeguard category is safe to send.
+    // diagnostics. Only fixed operational and safeguard categories are safe to send.
     if let Err(cause) = &account.ended
         && !account.interrupted
     {
         let cause = if account.security_findings.is_some() {
-            cause.safeguard_refusal()
+            account
+                .pre_audit_failure
+                .or_else(|| cause.safeguard_refusal())
         } else {
             Some(cause.full())
         };
@@ -298,8 +314,12 @@ fn body(
     if let Some(log) = command_log {
         text += &format!("Command log:  {}\n", log.display());
     }
-    if let Some(built_with) = built_with {
-        text += &format!("{built_with}\n");
+    if let Some(configured) = configured {
+        if account.pre_audit_failure.is_some() {
+            text += &format!("Configured:   {configured}\n");
+        } else {
+            text += &format!("Built with {configured}\n");
+        }
     }
     text += &format!("Host:         {host}\n");
     text += &format!("Took:         {}\n", progress::duration(took));
@@ -358,6 +378,7 @@ mod tests {
             review: None,
             security_findings: None,
             failed_reproduction: None,
+            pre_audit_failure: None,
             recovery: None,
             urls: vec![PR],
         }
@@ -378,6 +399,7 @@ mod tests {
             review: None,
             security_findings: None,
             failed_reproduction: None,
+            pre_audit_failure: None,
             recovery: None,
             urls: Vec::new(),
         }
@@ -582,7 +604,7 @@ mod tests {
             body(
                 &Subject::Pending,
                 &account,
-                Some("Built with claude · opus · high"),
+                Some("claude · opus · high"),
                 Some(Path::new("/home/me/.thirdshift/logs/commands/issue/x.log")),
                 "droplet-1",
                 Duration::from_secs(4)

src/pass.rs

@@ -104,7 +104,7 @@ pub trait Outside {
     /// Check Node.js before a Security audit needs its validators.
     fn check_node(&mut self) -> Result<()>;
     /// Audit the Base branch in a disposable checkout and validate its artifacts.
-    fn audit(&mut self, base: &str) -> (Result<crate::security::audit::Audited>, Option<PathBuf>);
+    fn audit(&mut self, base: &str) -> crate::security::audit::Attempt;
     /// Private advisory or issue records in every state.
     fn security_records(&mut self) -> Result<SecurityRecords>;
     /// Whether origin's prepared Base branch is the last completed audit's commit.
@@ -234,7 +234,7 @@ impl Outside for LaunchAndGitHub<'_> {
         crate::security::audit::check_node()
     }
 
-    fn audit(&mut self, base: &str) -> (Result<crate::security::audit::Audited>, Option<PathBuf>) {
+    fn audit(&mut self, base: &str) -> crate::security::audit::Attempt {
         logs::stage(logs::Stage::SecurityAudit);
         crate::security::audit::run(self.launch.git(), self.repo, base, self.harness)
     }
@@ -766,18 +766,19 @@ mod in_memory {
             Ok(())
         }
 
-        fn audit(
-            &mut self,
-            base: &str,
-        ) -> (Result<crate::security::audit::Audited>, Option<PathBuf>) {
+        fn audit(&mut self, base: &str) -> crate::security::audit::Attempt {
             self.calls.push(Call::SecurityAudit(base.to_string()));
             let audited = match self.audit_error.take() {
                 Some(cause) => Err(anyhow!(cause)),
                 None => Ok(crate::security::audit::Audited {
                     findings: std::mem::take(&mut self.audit_findings),
                 }),
             };
-            (audited, self.session_log.clone())
+            crate::security::audit::Attempt {
+                started: true,
+                outcome: audited,
+                log: self.session_log.clone(),
+            }
         }
 
         fn security_records(&mut self) -> Result<SecurityRecords> {

src/run_ending.rs

@@ -55,19 +55,30 @@ pub fn read(ending: &Ending) -> Result<Account<'_>, &Skip> {
                     review: None,
                     security_findings: None,
                     failed_reproduction: None,
+                    pre_audit_failure: None,
                     recovery: None,
                     urls: Vec::new(),
                 },
                 Err(failed) => Account::of_failure(
                     failed,
-                    if audited.failed_reproduction.is_some() {
+                    if !audited.audit_started {
+                        "pre-audit failed"
+                    } else if audited.failed_reproduction.is_some() {
                         "reproduction failed"
                     } else {
                         "audit failed"
                     },
                 ),
             };
             Ok(Account {
+                pre_audit_failure: if !audited.audit_started {
+                    audited.outcome.as_ref().err().map(|failed| {
+                        crate::github::operational_cause(&failed.error)
+                            .unwrap_or("pre-audit checks failed")
+                    })
+                } else {
+                    None
+                },
                 failed_reproduction: audited.failed_reproduction,
                 security_findings: Some(&audited.findings),
                 advice: &audited.advice,
@@ -117,6 +128,8 @@ pub struct Account<'a> {
     pub review: Option<Review<'a>>,
     /// The failed reproduction, after a completed Security audit and private recording.
     pub failed_reproduction: Option<usize>,
+    /// A fixed operational cause when a Security audit never started.
+    pub pre_audit_failure: Option<&'static str>,
     /// Safe metadata from a Security run's private records, even if the audit failed.
     pub security_findings: Option<&'a [crate::security::RecordedFinding]>,
     /// The URLs on stdout, a line each: the pull request's, or that of the
@@ -190,6 +203,7 @@ impl<'a> Account<'a> {
                 review: None,
                 security_findings: None,
                 failed_reproduction: None,
+                pre_audit_failure: None,
                 recovery: None,
                 urls: vec![&reached.pr_url],
             },
@@ -216,6 +230,7 @@ impl<'a> Account<'a> {
             review: None,
             security_findings: None,
             failed_reproduction: None,
+            pre_audit_failure: None,
             recovery: failed.recovery.as_deref(),
             urls: failed.pr_url.as_deref().into_iter().collect(),
         }
@@ -246,6 +261,7 @@ impl<'a> Account<'a> {
                 review: None,
                 security_findings: None,
                 failed_reproduction: None,
+                pre_audit_failure: None,
                 recovery: None,
                 urls: vec![reviewed.url()],
             },
@@ -537,6 +553,7 @@ mod tests {
             review: None,
             security_findings: None,
             failed_reproduction: None,
+            pre_audit_failure: None,
             recovery: None,
             urls: vec![PR],
         }
@@ -557,6 +574,7 @@ mod tests {
             review: None,
             security_findings: None,
             failed_reproduction: None,
+            pre_audit_failure: None,
             recovery: None,
             urls: Vec::new(),
         }
@@ -739,6 +757,7 @@ mod tests {
                     }),
                     security_findings: None,
                     failed_reproduction: None,
+                    pre_audit_failure: None,
                     recovery: None,
                     urls: vec![PLAN],
                 }

src/security.rs

@@ -46,6 +46,8 @@ pub enum Outcome {
 /// The Security run's outcome and the private records it reached, including before a failure.
 pub struct Ended {
     pub outcome: Result<Recorded, FailedRun>,
+    /// Whether an audit session was requested after preparation succeeded.
+    pub audit_started: bool,
     /// A failed reproduction's number; its audit and private recording completed.
     pub failed_reproduction: Option<usize>,
     pub findings: Vec<RecordedFinding>,
@@ -56,6 +58,7 @@ impl From<anyhow::Error> for Ended {
     fn from(error: anyhow::Error) -> Self {
         Self {
             outcome: Err(error.into()),
+            audit_started: false,
             failed_reproduction: None,
             findings: Vec::new(),
             advice: Vec::new(),
@@ -290,13 +293,15 @@ fn audit_and_record(
     let mut findings = Vec::new();
     let mut log = None;
     let mut reproduction = None;
+    let mut audit_started = false;
     let recorded = (|| -> Result<Recorded> {
         outside.check_harness()?;
         outside.check_node()?;
         outside.started(Work::SecurityRun(repo));
-        let (audited, session_log) = outside.audit(base);
-        log = session_log;
-        let audited = audited?;
+        let attempt = outside.audit(base);
+        audit_started = attempt.started;
+        log = attempt.log;
+        let audited = attempt.outcome?;
         let mut recorded = Recorded {
             created: 0,
             existing: 0,
@@ -380,6 +385,7 @@ fn audit_and_record(
             log,
             ..error.into()
         }),
+        audit_started,
         failed_reproduction,
         findings,
         advice,

src/security/audit.rs

@@ -20,6 +20,14 @@ use crate::session::{Logs, Purpose, Sessions};
 use crate::skills;
 use crate::worktree::ReviewWorktree;
 
+/// Audit startup and its result stay together: preparation can fail before
+/// any audit session is requested.
+pub struct Attempt {
+    pub started: bool,
+    pub outcome: Result<Audited>,
+    pub log: Option<PathBuf>,
+}
+
 pub struct Audited {
     pub findings: Vec<DraftAdvisory>,
 }
@@ -67,12 +75,7 @@ pub fn check_node() -> Result<()> {
     Ok(())
 }
 
-pub fn run(
-    launch: &Git,
-    repo: &Repo,
-    base: &str,
-    harness: &Choice,
-) -> (Result<Audited>, Option<PathBuf>) {
+pub fn run(launch: &Git, repo: &Repo, base: &str, harness: &Choice) -> Attempt {
     let prepared = (|| -> Result<_> {
         let worktree = ReviewWorktree::create(launch, &repo.name, base)?;
         let commit = Git::new(worktree.path()).run(&["rev-parse", "HEAD"])?;
@@ -104,11 +107,19 @@ pub fn run(
     })();
     let (worktree, output, prompt, commit) = match prepared {
         Ok(prepared) => prepared,
-        Err(error) => return (Err(error), None),
+        Err(error) => {
+            return Attempt {
+                started: false,
+                outcome: Err(error),
+                log: None,
+            };
+        }
     };
     let logs = Logs::of_security_run(repo);
+    let mut started = false;
     let (audited, log) = Sessions::within(&logs, worktree.path(), harness, |sessions| {
         progress::step(format!("starting the Security audit of {base}"));
+        started = true;
         let message =
             sessions.run_to_final_message(Purpose::Security, "security-audit", &prompt)?;
         let line = message.as_deref().and_then(|message| {
@@ -185,7 +196,11 @@ pub fn run(
         (Ok(audited), Ok(())) => Ok(audited),
         (Err(error), _) | (_, Err(error)) => Err(error),
     };
-    (audited, log)
+    Attempt {
+        started,
+        outcome: audited,
+        log,
+    }
 }
 
 fn finish_record(output: &Path, commit: &str, base: &str, complete: bool) -> Result<()> {