diff --git a/hive-c0re/src/lifecycle/mod.rs b/hive-c0re/src/lifecycle/mod.rs index 15633a7c..f2654c70 100644 --- a/hive-c0re/src/lifecycle/mod.rs +++ b/hive-c0re/src/lifecycle/mod.rs @@ -801,6 +801,21 @@ fn make_log_callback( } } +/// Put a failed non-streaming op's error into its build-log row, which has no +/// other output: `start` runs through `call`, not the streaming protocol. The +/// error carries `nixos-container`'s stderr. +fn record_failure( + logs: Option<&crate::build_logs::BuildLogs>, + log_id: Option, + err: &anyhow::Error, +) { + if let (Some(h), Some(id)) = (logs, log_id) { + for line in format!("{err:#}").lines() { + h.append_stderr(id, line); + } + } +} + /// A `create` or `update` asked for without a closure. The /// public entry points take the closure, so this guards the string dispatch /// below only. @@ -867,7 +882,13 @@ async fn priv_run_inner( } None => Err(missing_toplevel(kind, name)), }, - "start" => crate::priv_client::start_container(name).await, + "start" => { + let started = crate::priv_client::start_container(name).await; + if let Err(e) = &started { + record_failure(logs.as_deref(), log_id, e); + } + started + } "stop" => crate::priv_client::stop_container(name).await, "kill" => crate::priv_client::kill_container(name).await, "destroy" => crate::priv_client::destroy_container(name).await, diff --git a/hive-c0re/src/lifecycle/tests.rs b/hive-c0re/src/lifecycle/tests.rs index 1124da33..56632b51 100644 --- a/hive-c0re/src/lifecycle/tests.rs +++ b/hive-c0re/src/lifecycle/tests.rs @@ -514,3 +514,20 @@ fn hive_facts_leave_unset_values_out() { assert_eq!(facts.egress_no_proxy, None); assert_eq!(facts.name, None); } + +/// A failed `start` leaves its error in the op's build-log row; without it the +/// row is empty. A missing row id records nothing and does not panic. +#[test] +fn a_failed_start_puts_its_error_in_the_op_output() { + let dir = tempfile::tempdir().expect("tempdir"); + let logs = crate::build_logs::BuildLogs::open(dir.path()).expect("open"); + let id = logs + .start("updo", "start", "nixos-container start h-updo", None) + .expect("start"); + let err = anyhow::anyhow!("nixos-container start h-updo failed (exit status: 1): Job failed"); + record_failure(Some(&logs), Some(id), &err); + let full = logs.get_full(id).expect("get").expect("row"); + assert!(full.stderr.contains("Job failed"), "{}", full.stderr); + record_failure(Some(&logs), None, &err); + record_failure(None, Some(id), &err); +} diff --git a/hive-priv/src/container_config.rs b/hive-priv/src/container_config.rs index 197747c7..8c3e083a 100644 --- a/hive-priv/src/container_config.rs +++ b/hive-priv/src/container_config.rs @@ -345,12 +345,22 @@ fn validate_bind_path(path: &str) -> Result<()> { /// flags so nspawn mounts them on top of the already-mounted trees. /// Config mounts are matched by shape, not by a name list: the set is /// dynamic, growing with each child bound into a parent. +/// +/// Only a bind whose host source has a `.git` entry gets the overlay: nspawn +/// must create the mount point inside the read-only bind, which fails +/// (`EROFS`) where there is nothing to mount over. An archetype agent's +/// config is an empty directory. The entry is looked up without following a +/// symlink, and a symlink named `.git` counts as present, so a path that +/// leads somewhere else is still hidden. fn git_overlay_flags(binds: &[BindMount]) -> Vec { binds .iter() - .map(|b| b.container_path.as_str()) - .filter(|p| *p == "/knowledge" || (p.starts_with("/agents/") && p.ends_with("/config"))) - .map(|p| format!("--tmpfs={p}/.git")) + .filter(|b| { + let p = b.container_path.as_str(); + p == "/knowledge" || (p.starts_with("/agents/") && p.ends_with("/config")) + }) + .filter(|b| std::fs::symlink_metadata(Path::new(&b.host_path).join(".git")).is_ok()) + .map(|b| format!("--tmpfs={}/.git", b.container_path)) .collect() } @@ -641,6 +651,20 @@ mod tests { } } + /// A source dir holding a `.git` directory, as a repo checkout does. + fn repo_source() -> String { + let dir = scratch(); + std::fs::create_dir(dir.join(".git")).unwrap(); + dir.to_string_lossy().into_owned() + } + + fn bind_from(host_path: &str, container_path: &str) -> BindMount { + BindMount { + host_path: host_path.to_owned(), + ..bind(container_path) + } + } + /// Every bound git repo gets its `.git` overlaid — the knowledge tree /// and *each* config mount, an agent's own plus every child's. /// @@ -649,10 +673,11 @@ mod tests { /// silently stop covering new ones. #[test] fn every_bound_git_repo_gets_its_dot_git_hidden() { + let src = repo_source(); let flags = git_overlay_flags(&[ - bind("/knowledge"), - bind("/agents/atlas/config"), - bind("/agents/kiddo/config"), + bind_from(&src, "/knowledge"), + bind_from(&src, "/agents/atlas/config"), + bind_from(&src, "/agents/kiddo/config"), ]); assert_eq!( flags, @@ -669,15 +694,41 @@ mod tests { /// keeps working clones of its own. #[test] fn non_repo_mounts_are_left_alone() { + let src = repo_source(); let flags = git_overlay_flags(&[ - bind("/agents/atlas/state"), - bind("/work"), - bind("/applied"), - bind("/agents/atlas/config-notes"), + bind_from(&src, "/agents/atlas/state"), + bind_from(&src, "/work"), + bind_from(&src, "/applied"), + bind_from(&src, "/agents/atlas/config-notes"), ]); assert!(flags.is_empty(), "overlaid a non-repo mount: {flags:?}"); } + /// A config mount whose source has no `.git` (an archetype agent's empty + /// directory) gets no overlay: nspawn cannot create the mount point in a + /// read-only bind. The sibling with a `.git` is the presence control. + #[test] + fn a_source_without_dot_git_gets_no_overlay() { + let empty = scratch().to_string_lossy().into_owned(); + let flags = git_overlay_flags(&[ + bind_from(&empty, "/agents/updo/config"), + bind_from(&empty, "/knowledge"), + bind_from(&repo_source(), "/agents/atlas/config"), + ]); + assert_eq!(flags, ["--tmpfs=/agents/atlas/config/.git"]); + } + + /// A symlink named `.git` is not followed: it counts as present, even + /// dangling, so the path it leads to is hidden rather than skipped. + #[test] + fn a_symlinked_dot_git_is_not_followed() { + let dir = scratch(); + std::os::unix::fs::symlink("/nonexistent-target", dir.join(".git")).unwrap(); + let src = dir.to_string_lossy().into_owned(); + let flags = git_overlay_flags(&[bind_from(&src, "/agents/atlas/config")]); + assert_eq!(flags, ["--tmpfs=/agents/atlas/config/.git"]); + } + /// The whole rendered body, pinned literally — including the values, /// so changing the restart policy is a visible test edit rather than a /// silent one.