diff --git a/hive-c0re/src/lifecycle/mod.rs b/hive-c0re/src/lifecycle/mod.rs index f2654c706..15633a7cc 100644 --- a/hive-c0re/src/lifecycle/mod.rs +++ b/hive-c0re/src/lifecycle/mod.rs @@ -801,21 +801,6 @@ 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. @@ -882,13 +867,7 @@ async fn priv_run_inner( } None => Err(missing_toplevel(kind, name)), }, - "start" => { - let started = crate::priv_client::start_container(name).await; - if let Err(e) = &started { - record_failure(logs.as_deref(), log_id, e); - } - started - } + "start" => crate::priv_client::start_container(name).await, "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 56632b513..1124da337 100644 --- a/hive-c0re/src/lifecycle/tests.rs +++ b/hive-c0re/src/lifecycle/tests.rs @@ -514,20 +514,3 @@ 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 a012c85a6..197747c70 100644 --- a/hive-priv/src/container_config.rs +++ b/hive-priv/src/container_config.rs @@ -345,27 +345,12 @@ 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. -/// -/// `/knowledge` is overlaid unconditionally: its directory is created empty -/// and cloned into later, so a container started before the clone must -/// already hide the `.git` that arrives. A config mount gets the overlay only -/// when its host source has a `.git` entry: nspawn must create the mount -/// point inside the read-only bind, which fails (`EROFS`) where there is -/// nothing to mount over, as in an archetype agent's empty config 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() - .filter(|b| { - let p = b.container_path.as_str(); - p == "/knowledge" - || (p.starts_with("/agents/") - && p.ends_with("/config") - && std::fs::symlink_metadata(Path::new(&b.host_path).join(".git")).is_ok()) - }) - .map(|b| format!("--tmpfs={}/.git", b.container_path)) + .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")) .collect() } @@ -656,20 +641,6 @@ 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. /// @@ -678,11 +649,10 @@ 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_from(&src, "/knowledge"), - bind_from(&src, "/agents/atlas/config"), - bind_from(&src, "/agents/kiddo/config"), + bind("/knowledge"), + bind("/agents/atlas/config"), + bind("/agents/kiddo/config"), ]); assert_eq!( flags, @@ -699,49 +669,15 @@ 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_from(&src, "/agents/atlas/state"), - bind_from(&src, "/work"), - bind_from(&src, "/applied"), - bind_from(&src, "/agents/atlas/config-notes"), + bind("/agents/atlas/state"), + bind("/work"), + bind("/applied"), + bind("/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(&repo_source(), "/agents/atlas/config"), - ]); - assert_eq!(flags, ["--tmpfs=/agents/atlas/config/.git"]); - } - - /// `/knowledge` is cloned into after the container may have started, so - /// its overlay does not wait for a `.git` to exist. - #[test] - fn an_empty_knowledge_source_still_gets_the_overlay() { - let empty = scratch().to_string_lossy().into_owned(); - let flags = git_overlay_flags(&[bind_from(&empty, "/knowledge")]); - assert_eq!(flags, ["--tmpfs=/knowledge/.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.