From 138643959181058a739737328e122b68c6113d56 Mon Sep 17 00:00:00 2001
From: damocles <damocles@hyperhive.local>
Date: Wed, 3 Jun 2026 12:40:23 +0200
Subject: [PATCH] fix(#1116): address review nits - dedup log callback, comment
 stderr truncation

---
 hive-c0re/src/lifecycle.rs | 72 ++++++++++++++++++--------------------
 hive-priv/src/main.rs      |  4 +++
 2 files changed, 38 insertions(+), 38 deletions(-)

diff --git a/hive-c0re/src/lifecycle.rs b/hive-c0re/src/lifecycle.rs
index 55f2a383..622e64ac 100644
--- a/hive-c0re/src/lifecycle.rs
+++ b/hive-c0re/src/lifecycle.rs
@@ -1206,14 +1206,38 @@ async fn set_nspawn_flags(
     crate::priv_client::write_nspawn_flags(container, &binds, isolation).await
 }
 
+/// 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.
+fn make_log_callback(
+    logs: Option<std::sync::Arc<crate::build_logs::BuildLogs>>,
+    log_id: Option<i64>,
+    cmdline: String,
+) -> impl FnMut(hive_sh4re::priv_proto::PrivStream, &str) {
+    use hive_sh4re::priv_proto::PrivStream;
+    move |stream, line| match stream {
+        PrivStream::Stdout => {
+            tracing::info!(target: "nixos-container", cmdline = %cmdline, "{line}");
+            if let (Some(h), Some(id)) = (&logs, log_id) {
+                h.append_stdout(id, line);
+            }
+        }
+        PrivStream::Stderr => {
+            tracing::warn!(target: "nixos-container", cmdline = %cmdline, "{line}");
+            if let (Some(h), Some(id)) = (&logs, log_id) {
+                h.append_stderr(id, line);
+            }
+        }
+    }
+}
+
 /// Execute a container operation via hive-priv and integrate with
 /// build_logs.sqlite. hive-priv runs as root and forwards output lines
 /// to hive-c0re in real time via the streaming priv protocol. Each line
 /// is appended to the build-log row as it arrives, so the dashboard
 /// shows live progress during long `nixos-container create` / `update` runs.
 async fn priv_run(kind: &str, name: &str) -> Result<()> {
-    use hive_sh4re::priv_proto::PrivStream;
-
     let container = container_name(name);
     let cmdline = format!("nixos-container {kind} {container}");
 
@@ -1230,45 +1254,17 @@ async fn priv_run(kind: &str, name: &str) -> Result<()> {
     // receives lines in real time rather than as a batch at completion.
     let result: Result<()> = match kind {
         "create" => {
-            let h = logs.clone();
-            let id = log_id;
-            crate::priv_client::create_container_streaming(name, move |stream, line| {
-                match stream {
-                    PrivStream::Stdout => {
-                        tracing::info!(target: "nixos-container", cmdline = %cmdline, "{line}");
-                        if let (Some(h), Some(id)) = (&h, id) {
-                            h.append_stdout(id, line);
-                        }
-                    }
-                    PrivStream::Stderr => {
-                        tracing::warn!(target: "nixos-container", cmdline = %cmdline, "{line}");
-                        if let (Some(h), Some(id)) = (&h, id) {
-                            h.append_stderr(id, line);
-                        }
-                    }
-                }
-            })
+            crate::priv_client::create_container_streaming(
+                name,
+                make_log_callback(logs, log_id, cmdline),
+            )
             .await
         }
         "update" => {
-            let h = logs.clone();
-            let id = log_id;
-            crate::priv_client::update_container_streaming(name, move |stream, line| {
-                match stream {
-                    PrivStream::Stdout => {
-                        tracing::info!(target: "nixos-container", cmdline = %cmdline, "{line}");
-                        if let (Some(h), Some(id)) = (&h, id) {
-                            h.append_stdout(id, line);
-                        }
-                    }
-                    PrivStream::Stderr => {
-                        tracing::warn!(target: "nixos-container", cmdline = %cmdline, "{line}");
-                        if let (Some(h), Some(id)) = (&h, id) {
-                            h.append_stderr(id, line);
-                        }
-                    }
-                }
-            })
+            crate::priv_client::update_container_streaming(
+                name,
+                make_log_callback(logs, log_id, cmdline),
+            )
             .await
         }
         "start" => crate::priv_client::start_container(name).await,
diff --git a/hive-priv/src/main.rs b/hive-priv/src/main.rs
index d6ab8a03..df5111b8 100644
--- a/hive-priv/src/main.rs
+++ b/hive-priv/src/main.rs
@@ -440,6 +440,10 @@ async fn container_run_streaming(
 
     let status = child.wait().await.context("wait nixos-container")?;
     if !status.success() {
+        // Only the last stderr line is embedded — the full stderr was
+        // already forwarded line-by-line as PrivEvent::Line messages and
+        // is captured in build_logs.sqlite by the caller. Keeping the
+        // error message short avoids bloating the anyhow chain.
         bail!(
             "nixos-container {} failed ({}): {}",
             args.join(" "),