hive-c0re: widen nix stderr predicate to cover error:/evaluation warning:, add tests

hive-c0re/src/lifecycle/mod.rs

@@ -806,7 +806,7 @@ pub async fn prebuild_toplevel(name: &str, flake_ref: &str, node_id: Option<u64>
     let pump_stderr = tokio::spawn(async move {
         let mut lines = BufReader::new(stderr).lines();
         while let Ok(Some(line)) = lines.next_line().await {
-            if is_real_nix_warning(&line) {
+            if is_actionable_nix_line(&line) {
                 tracing::warn!(target: "nix-prebuild", cmdline = %stderr_cmdline, "{line}");
             } else {
                 tracing::info!(target: "nix-prebuild", cmdline = %stderr_cmdline, "{line}");
@@ -893,19 +893,26 @@ pub fn ensure_agent_runtime_dir(name: &str) -> Result<()> {
         .with_context(|| format!("create agent runtime dir {}", dir.display()))
 }
 
-/// Whether a `nix`/`nixos-container` stderr line is an actual warning
-/// (nix's own convention: a genuine warning starts with `warning:`) rather
-/// than routine progress chatter (`copying N paths…`, `fetching git
-/// input…`, `structuredAttrs is enabled`, …) — nix writes the latter to
-/// stderr too, and it dwarfs the former in volume.
-fn is_real_nix_warning(line: &str) -> bool {
-    line.trim_start().starts_with("warning:")
+/// Whether a `nix`/`nixos-container` stderr line is worth WARN (nix's own
+/// warnings and errors) rather than routine progress chatter (`copying N
+/// paths…`, `fetching git input…`, `structuredAttrs is enabled`, …) — nix
+/// writes the latter to stderr too, and it dwarfs the former in volume.
+/// Matches nix's own prefixes (`warning:`, `evaluation warning:`,
+/// `error:`) rather than only `warning:`, so a real build failure
+/// (`error: builder for '...' failed`, `error: 1 dependencies ... failed
+/// to build`) never gets buried at INFO alongside the chatter.
+fn is_actionable_nix_line(line: &str) -> bool {
+    let trimmed = line.trim_start();
+    trimmed.starts_with("warning:")
+        || trimmed.starts_with("evaluation warning:")
+        || trimmed.starts_with("error:")
 }
 
 /// Build the per-line callback for `create_container_streaming` /
 /// `update_container_streaming`. Both ops share identical dispatch logic
-/// (stdout → info + `append_stdout`, stderr → warn + `append_stderr`); this
-/// helper avoids duplicating that match body across the two call sites.
+/// (stdout → info + `append_stdout`, stderr → warn for a nix warning/error
+/// and info otherwise + `append_stderr`); this helper avoids duplicating
+/// that match body across the two call sites.
 fn make_log_callback(
     logs: Option<std::sync::Arc<crate::build_logs::BuildLogs>>,
     log_id: Option<i64>,
@@ -920,7 +927,7 @@ fn make_log_callback(
             }
         }
         PrivStream::Stderr => {
-            if is_real_nix_warning(line) {
+            if is_actionable_nix_line(line) {
                 tracing::warn!(target: "nixos-container", cmdline = %cmdline, "{line}");
             } else {
                 tracing::info!(target: "nixos-container", cmdline = %cmdline, "{line}");

hive-c0re/src/lifecycle/tests.rs

@@ -421,3 +421,31 @@ fn ensure_state_dir_creates_the_work_dir() {
     assert!(notes_dir.is_dir());
     assert!(root.path().join(crate::paths::WORK_SUBDIR).is_dir());
 }
+
+/// Real build failures (`error:`) and both of nix's own warning spellings
+/// must stay WARN; routine progress chatter is the only thing demoted.
+/// Regression guard for a first-pass predicate that only matched
+/// `warning:` and silently buried `error:` lines at INFO.
+#[test]
+fn is_actionable_nix_line_keeps_warnings_and_errors_demotes_chatter() {
+    let actionable = [
+        "warning: Lock file entry is unlocked",
+        "  warning: leading whitespace",
+        "evaluation warning: this will be an error in a future release",
+        "error: builder for '/nix/store/foo.drv' failed with exit code 1",
+        "error: 1 dependencies of derivation '/nix/store/bar.drv' failed to build",
+    ];
+    for line in actionable {
+        assert!(is_actionable_nix_line(line), "{line}");
+    }
+
+    let chatter = [
+        "copying 3 paths",
+        "these 2 derivations will be built:",
+        "fetching git input 'git+file:///var/lib/hyperhive'",
+        "structuredAttrs is enabled",
+    ];
+    for line in chatter {
+        assert!(!is_actionable_nix_line(line), "{line}");
+    }
+}

hive-c0re/src/socket_server/mod.rs

@@ -209,9 +209,11 @@ async fn read_request_line<R: AsyncBufRead + Unpin>(
     })
 }
 
-/// Whether `e`'s root cause is the peer closing the connection — `serve`'s
-/// own `?`s convert a raw `std::io::Error` straight to `anyhow::Error` with
-/// no intervening `.context()`, so a direct downcast reaches it.
+/// Whether `e`'s root cause is the peer closing the connection.
+/// `downcast_ref` searches the whole error chain (anyhow's own
+/// `object_downcast`, not just the outermost type), so this finds a
+/// wrapped `std::io::Error` whether or not something added `.context()`
+/// on top of it.
 fn is_broken_pipe(e: &anyhow::Error) -> bool {
     e.downcast_ref::<std::io::Error>().is_some_and(|io_err| {
         matches!(
@@ -731,4 +733,31 @@ mod tests {
         assert_eq!(served, 1, "connection after EMFILE not served");
         assert_eq!(err.raw_os_error(), Some(libc::EBADF));
     }
+
+    /// `is_broken_pipe` must recognise both peer-closed kinds whether the
+    /// `io::Error` reaches it bare (how `serve`'s own `?`s produce it) or
+    /// wrapped in `.context()` (anyhow's `downcast_ref` walks the whole
+    /// chain, so wrapping doesn't hide it — confirmed here rather than
+    /// assumed), and must not misclassify an unrelated `io::Error` or a
+    /// non-io `anyhow::Error`.
+    #[test]
+    fn is_broken_pipe_matches_peer_closed_kinds_only() {
+        let broken_pipe = std::io::Error::from(std::io::ErrorKind::BrokenPipe);
+        let conn_reset = std::io::Error::from(std::io::ErrorKind::ConnectionReset);
+        let permission_denied = std::io::Error::from(std::io::ErrorKind::PermissionDenied);
+
+        assert!(is_broken_pipe(&anyhow::Error::new(broken_pipe)));
+        assert!(is_broken_pipe(&anyhow::Error::new(conn_reset)));
+        assert!(!is_broken_pipe(&anyhow::Error::new(permission_denied)));
+
+        let contextual = anyhow::Error::new(std::io::Error::from(std::io::ErrorKind::BrokenPipe))
+            .context("write response");
+        assert!(
+            is_broken_pipe(&contextual),
+            "anyhow's downcast_ref searches the whole chain, so a \
+             .context()-wrapped io::Error must still be found"
+        );
+
+        assert!(!is_broken_pipe(&anyhow::anyhow!("not an io error at all")));
+    }
 }