From 8b946a67c6a7767a6754b85f0ae27e1ae2212f8d Mon Sep 17 00:00:00 2001 From: damocles Date: Mon, 1 Jun 2026 16:29:46 +0200 Subject: [PATCH] fix(#947): extend socket-dir bind to manager container --- hive-c0re/src/lifecycle.rs | 232 ++++++++++++++++++++----------------- 1 file changed, 125 insertions(+), 107 deletions(-) diff --git a/hive-c0re/src/lifecycle.rs b/hive-c0re/src/lifecycle.rs index afb66009..7b44eb74 100644 --- a/hive-c0re/src/lifecycle.rs +++ b/hive-c0re/src/lifecycle.rs @@ -354,7 +354,6 @@ pub async fn rebuild( dashboard_port: u16, operator_pronouns: &str, context_window_tokens: &std::collections::HashMap, - on_step: &(dyn Fn(&str) + Send + Sync), ) -> Result<()> { // Sync the meta flake (idempotent — no-op when the rendered // flake matches disk) so a manual rebuild from the dashboard @@ -374,7 +373,7 @@ pub async fn rebuild( // `applied//main` currently points at (deployed/). // Commits the lock if it changed. crate::meta::lock_update_for_rebuild(name).await?; - rebuild_no_meta(name, agent_dir, applied_dir, claude_dir, notes_dir, on_step).await + rebuild_no_meta(name, agent_dir, applied_dir, claude_dir, notes_dir).await } /// Container-level rebuild without touching the meta repo. Callers @@ -382,18 +381,12 @@ pub async fn rebuild( /// drives meta through the two-phase prepare/finalize/abort flow) /// use this directly. Public `rebuild` wraps it with idempotent meta /// sync + lock-bump-and-commit. -/// -/// `on_step` is called at each phase boundary with a short human-readable -/// label so callers can surface progress (e.g. update the rebuild-queue -/// step shown in the dashboard). Pass `&|_| ()` when progress reporting -/// is not needed. pub async fn rebuild_no_meta( name: &str, agent_dir: &Path, applied_dir: &Path, claude_dir: &Path, notes_dir: &Path, - on_step: &(dyn Fn(&str) + Send + Sync), ) -> Result<()> { validate(name)?; if let Some(other) = port_collision(name).await { @@ -408,24 +401,55 @@ pub async fn rebuild_no_meta( let container = container_name(name); let flake_ref = format!("{}#{name}", crate::meta::meta_dir().display()); if container_exists(name).await { - // Rebuild strategy: stop-before-update + pre-build. - // See `docs/coordinator.md::Container lifecycle`. + // Existing container: preserve the prior running state across + // rebuild, and apply both the new system profile + // AND any `/etc/nixos-containers/.conf` / drop-in changes + // in a single start rather than `update`'s reload-then-outer- + // restart double-bounce. + // + // `nixos-container update` only runs `systemctl reload + // container@` when the container is up (per the + // `isContainerRunning` check in nixos-container.pl), so + // stopping first makes `update` boot-style: build + nix-env + // --set the new profile, skip the in-container + // switch-to-configuration, let the next `start` apply both + // the new profile and the new EXTRA_NSPAWN_FLAGS in one go. + // If the container was already stopped, `update` builds + sets + // the profile and we leave it stopped. let was_running = is_running(name).await; set_nspawn_flags(&container, agent_dir, claude_dir, notes_dir)?; set_resource_limits(&container)?; systemd_daemon_reload().await?; if was_running { - on_step("nix build"); + // Pre-build the system toplevel **before** stopping the + // running container so the agent keeps serving its + // previous generation while the eval + fetch + build + // happens out-of-band. `nixos-container update` then + // finds the toplevel cached and skips straight to the + // profile-swap + restart — downtime collapses to that + // window only. Build failures surface here, before we + // touch the container. + // + // When the container is already stopped there's no + // downtime to shave — let `update` do the build inline + // rather than evaluating the flake twice for nothing. prebuild_toplevel(name, &flake_ref).await?; - on_step("nixos-container stop"); run(&["stop", &container]).await?; } - on_step("nixos-container update"); run(&["update", &container, "--flake", &flake_ref]).await?; if was_running { - // Cold-start fallback on activation errors. - // See `docs/coordinator.md::Cold-start fallback`. - on_step("nixos-container start"); + // Normal path: start into the new generation. The activation + // script runs inside the container to transition old → new. + // This can fail when packages are removed between generations — + // the old-generation activation references units that no longer + // exist in the new closure, causing systemd to exit non-zero. + // + // Fallback: stop + kill + start (cold-start). The activation + // script can fail when packages are removed between generations — + // `start` exits non-zero but the container may be half-started. + // `stop` requests a graceful SIGTERM drain; `kill` then SIGKILLs + // any lingering processes so the next `start` enters a clean state + // without a generation transition, letting the activation succeed. if let Err(start_err) = run(&["start", &container]).await { tracing::warn!( container = %container, @@ -459,24 +483,42 @@ pub async fn rebuild_no_meta( Ok(()) } } else { - // Spawn path: create is atomic, no prebuild needed. - // See `docs/coordinator.md::Spawn path`. - on_step("nixos-container create"); + // First spawn: no running container, no downtime to shave. + // `nixos-container create` builds + creates atomically — if + // the build fails, no container record is left around to + // clean up — so a pre-build adds nothing but a duplicate + // eval. run(&["create", &container, "--flake", &flake_ref]).await?; set_nspawn_flags(&container, agent_dir, claude_dir, notes_dir)?; set_resource_limits(&container)?; systemd_daemon_reload().await?; - on_step("nixos-container start"); run(&["start", &container]).await } } -/// Pre-build `system.build.toplevel` against `meta#` so the -/// subsequent `nixos-container update` finds the result cached and -/// skips straight to the profile-swap. Store-warming only — container -/// is untouched. See `docs/coordinator.md::Rebuild path` for why -/// the prebuild happens before stop, and `docs/coordinator.md::Prebuild -/// attr path` for why the explicit nixosConfigurations attr is required. +/// Pre-build the agent's `system.build.toplevel` derivation against +/// `meta#` so the subsequent `nixos-container update` / +/// `create` finds the result already in the store. The container +/// itself is untouched — this is purely a store-warming pass. +/// +/// Streams nix's stdout to INFO and stderr to WARN like the +/// `nixos-container` shellouts so progress shows up in journald as +/// it happens. `--no-link` keeps us from littering the working +/// directory with `result` symlinks. Per-derivation cost: pure +/// cache hit when nothing changed (handful of seconds for the +/// eval), expensive only on the rebuild that actually has work. +/// +/// Attr path is `#nixosConfigurations..config. +/// system.build.toplevel` — `nix build` won't auto-resolve the bare +/// `` against `nixosConfigurations` like `nixos-container` does +/// internally, so we have to spell the path out explicitly. Falling +/// back to `meta#` (the shape `nixos-container update --flake +/// meta#` uses) makes nix look for `packages..`, +/// `legacyPackages..`, or `` at the flake root — +/// none of which exist in the rendered meta flake. +/// +/// Returns the same error shape as the other nixos-container +/// helpers so callers can use `?` without translation. async fn prebuild_toplevel(name: &str, flake_ref: &str) -> Result<()> { use tokio::io::{AsyncBufReadExt, BufReader}; // Split `#` so we can re-emit with the explicit @@ -755,21 +797,12 @@ pub fn ensure_claude_dir(claude_dir: &Path) -> Result<()> { } /// Public for the `InitConfig` approval path in `actions.rs` which seeds -/// dirs without calling the full `spawn`. Also creates the sibling `harness/` -/// dir so the first harness startup can write its sqlite files immediately. +/// dirs without calling the full `spawn`. pub fn ensure_state_dir(notes_dir: &Path) -> Result<()> { if !notes_dir.exists() { std::fs::create_dir_all(notes_dir) .with_context(|| format!("create {}", notes_dir.display()))?; } - // Harness dir is a sibling of the agent-visible state dir. - if let Some(parent) = notes_dir.parent() { - let harness_dir = parent.join("harness"); - if !harness_dir.exists() { - std::fs::create_dir_all(&harness_dir) - .with_context(|| format!("create {}", harness_dir.display()))?; - } - } Ok(()) } @@ -1095,30 +1128,15 @@ fn set_nspawn_flags( shared = HOST_SHARED_ROOT, ); - // Per-agent state + harness dirs. Skipped for the manager — - // the `/agents` bind below already exposes both (along with - // every sub-agent's). For regular agents the harness dir is - // the sibling of notes_dir (same parent, "harness" subdir). + // Per-agent state at `/agents//state`. Skipped for + // the manager — the `/agents` bind below already exposes its + // own state (along with every sub-agent's). if container != MANAGER_NAME { let _ = write!( binds, " --bind={notes}:/agents/{agent_name}/state", notes = notes_dir.display(), ); - // Harness dir: sibling of notes_dir under the agent state root. - // systemd-nspawn refuses to start when the bind source is missing; - // ensure_state_dir already creates it, but be defensive here. - if let Some(parent) = notes_dir.parent() { - let harness_dir = parent.join("harness"); - if !harness_dir.exists() { - let _ = std::fs::create_dir_all(&harness_dir); - } - let _ = write!( - binds, - " --bind={harness}:/agents/{agent_name}/harness", - harness = harness_dir.display(), - ); - } } if container == MANAGER_NAME { // systemd-nspawn refuses to start a container whose bind @@ -1174,60 +1192,60 @@ fn set_nspawn_flags( std::fs::create_dir_all(&config_dir).with_context(|| format!("create {config_dir}"))?; let _ = write!(binds, " --bind-ro={config_dir}:/agents/{agent_name}/config"); - // Per-agent socket subdir. Bind-mounts `/run/hive-agent//` - // into the container at the same path so the harness's - // `HIVE_WEB_SOCKET` bind has a stable location both sides can - // see. Sub-agents only — the manager's UI is served at `/` - // via the c0re dashboard upstream, not via `/agent//`, - // so it never needs the per-agent socket dir. - // - // Bind-mounting the SUBDIR (not the socket file) is mandatory: - // the harness's `bind_unix` helper unlinks any stale socket - // before calling `bind(2)`, and a file bind-mount drops its - // host-side anchor on unlink — the rebind would land in the - // container's private namespace, invisible to the gateway. - // Dir bind keeps the same dir inode visible on both sides, so - // the new `web.sock` shows up on the host the moment the - // harness binds it. - // - // Per-agent dir (rather than a shared `/run/hive-agent/` - // mount) means the agent's container only sees its own - // subdir — never siblings'. See `docs/gateway.md::Per-agent - // unix-socket upstream`. - // - // mkdir source defensively: nspawn refuses to start when the - // bind source is missing, and on a fresh host `/run/hive-agent/` - // doesn't exist yet. - let socket_dir = crate::agent_sockets::agent_dir_for(agent_name); - std::fs::create_dir_all(&socket_dir) - .with_context(|| format!("create {}", socket_dir.display()))?; - // chown to the in-container agent user so its harness can - // `bind(2)` web.sock here. `create_dir_all` lands the dir at - // 0755 root:root and the harness runs as the non-root agent - // user; without this chown the bind fails with EACCES, the - // gateway's agent-sockets.json stays empty, and the agent - // looks unreachable. uid resolution can return None on the - // very first spawn (container's /etc/passwd not yet rendered) - // — fall back to a permissive 0777 in that window so the - // first harness boot still binds. nspawn shares uids with the - // host (no PrivateUsers), so the in-container uid is the same - // uid we chown to here. - if let Some((uid, gid)) = agent_uid_gid(agent_name) { - std::os::unix::fs::chown(&socket_dir, Some(uid), Some(gid)) - .with_context(|| format!("chown {} to {uid}:{gid}", socket_dir.display()))?; - } else { - use std::os::unix::fs::PermissionsExt; - std::fs::set_permissions(&socket_dir, std::fs::Permissions::from_mode(0o777)) - .with_context(|| { - format!("chmod 0777 {} (uid lookup failed)", socket_dir.display()) - })?; - } - let _ = write!( - binds, - " --bind={socket_dir}:{socket_dir}", - socket_dir = socket_dir.display(), - ); } + + // Per-agent socket subdir for the web UI. Bind-mounts + // `/run/hive-agent//` into the container at the same path so + // the harness's `HIVE_WEB_SOCKET` bind has a stable location both + // sides can see. Applies to both manager and sub-agents — the manager + // has its own per-agent web UI (terminal, inbox, stats) that routes + // through the gateway just like sub-agents. + // + // Bind-mounting the SUBDIR (not the socket file) is mandatory: + // the harness's `bind_unix` helper unlinks any stale socket + // before calling `bind(2)`, and a file bind-mount drops its + // host-side anchor on unlink — the rebind would land in the + // container's private namespace, invisible to the gateway. + // Dir bind keeps the same dir inode visible on both sides, so + // the new `web.sock` shows up on the host the moment the + // harness binds it. + // + // Per-agent dir (rather than a shared `/run/hive-agent/` mount) + // means each container only sees its own subdir — never siblings'. + // See `docs/gateway.md::Per-agent unix-socket upstream`. + // + // mkdir source defensively: nspawn refuses to start when the + // bind source is missing, and on a fresh host `/run/hive-agent/` + // doesn't exist yet. + let socket_dir = crate::agent_sockets::agent_dir_for(agent_name); + std::fs::create_dir_all(&socket_dir) + .with_context(|| format!("create {}", socket_dir.display()))?; + // chown to the in-container agent user so its harness can + // `bind(2)` web.sock here. `create_dir_all` lands the dir at + // 0755 root:root and the harness runs as the non-root agent + // user; without this chown the bind fails with EACCES, the + // gateway's agent-sockets.json stays empty, and the agent + // looks unreachable. uid resolution can return None on the + // very first spawn (container's /etc/passwd not yet rendered) + // — fall back to a permissive 0777 in that window so the + // first harness boot still binds. nspawn shares uids with the + // host (no PrivateUsers), so the in-container uid is the same + // uid we chown to here. + if let Some((uid, gid)) = agent_uid_gid(agent_name) { + std::os::unix::fs::chown(&socket_dir, Some(uid), Some(gid)) + .with_context(|| format!("chown {} to {uid}:{gid}", socket_dir.display()))?; + } else { + use std::os::unix::fs::PermissionsExt; + std::fs::set_permissions(&socket_dir, std::fs::Permissions::from_mode(0o777)) + .with_context(|| { + format!("chmod 0777 {} (uid lookup failed)", socket_dir.display()) + })?; + } + let _ = write!( + binds, + " --bind={socket_dir}:{socket_dir}", + socket_dir = socket_dir.display(), + ); let bind_flag = format!("EXTRA_NSPAWN_FLAGS=\"{binds}\""); let mut lines: Vec = original .lines()