From 2ea633a2f1a39ef397a1905d880e209d365f18c0 Mon Sep 17 00:00:00 2001 From: Huskies Agent Date: Tue, 21 Jul 2026 14:57:18 +0000 Subject: [PATCH] huskies: merge 1242 refactor script/release builds project images so they can't drift --- script/build-project-images | 17 +- script/release | 23 +- .../matrix/bot/messages/on_room_message.rs | 1 - .../chat/transport/matrix/project_rebuild.rs | 312 +----------------- server/src/http/gateway/mcp.rs | 1 - 5 files changed, 22 insertions(+), 332 deletions(-) diff --git a/script/build-project-images b/script/build-project-images index 37960b16..c6ad41e1 100755 --- a/script/build-project-images +++ b/script/build-project-images @@ -4,19 +4,10 @@ set -euo pipefail # Build all project images in dependency order: # huskies → huskies-project-base → huskies-project- (one per stack fragment) # -# Run this after `script/docker_rebuild` or whenever you add a new stack. -# Safe to re-run: each step re-tags the image with the latest layers. -# -# IMPORTANT (story 1231): this script is NOT part of `script/release`. The -# huskies-project-* images bake whatever `huskies` binary happened to be built -# locally the last time this ran — as of this comment that's 0.13.0, several -# releases behind the current fleet artifact. `project-rebuild` self-heals a -# sled that comes back on a stale baked binary (it re-upgrades it in place -# from the gateway's published artifact), but that costs an extra -# download+restart cycle every time. Run `script/build-project-images` after -# every `script/release` — ideally as a step in the release flow itself — so -# freshly rebuilt sleds start on a current binary instead of relying on the -# self-heal. +# Called automatically by `script/release` (story 1242) so the huskies-project-* +# images never drift from the version being published. Also safe to run +# standalone after `script/docker_rebuild` or whenever you add a new stack — +# each step re-tags the image with the latest layers. cd "$(dirname "$0")/.." diff --git a/script/release b/script/release index 25ce44f2..0311924a 100755 --- a/script/release +++ b/script/release @@ -1,16 +1,6 @@ #!/usr/bin/env bash set -euo pipefail -# NOTE (story 1231): this script publishes the `huskies` binary artifact -# (consumed by the `upgrade`/`upgrade all` chat commands) but does NOT -# rebuild the huskies-project-* Docker images that `project-rebuild` uses. -# Those images bake whatever binary was locally built the last time -# `script/build-project-images` ran, so they silently drift stale otherwise -# (currently baking 0.13.0). Run `script/build-project-images` after this -# script — or better, fold it into this release flow — so a `project-rebuild` -# right after a release doesn't need to self-heal a stale sled back up to the -# version just published here. - # ── Configuration ────────────────────────────────────────────── GITEA_URL="https://code.crashlabs.io" REPO="crashlabs/huskies" @@ -97,6 +87,19 @@ cross build --release --target x86_64-unknown-linux-musl echo "==> Building Linux arm64 (static musl via cross)..." cross build --release --target aarch64-unknown-linux-musl +# ── Build project images ───────────────────────────────────────── +# Rebuild the huskies-project-* Docker images from this exact source tree +# (the version-bump commit above already landed, so build.rs's `git +# rev-parse HEAD` embeds the matching git hash) so they never drift from +# the binary being published below. A release that can't produce these +# images fails loudly here, before anything is tagged, pushed, or published. +echo "==> Building project images..." +if ! "${SCRIPT_DIR}/script/build-project-images"; then + echo "Error: failed to build huskies-project-* images at ${VERSION}." + echo "Release aborted — nothing was tagged, pushed, or published." + exit 1 +fi + # ── Package ──────────────────────────────────────────────────── DIST="target/dist" rm -rf "$DIST" diff --git a/server/src/chat/transport/matrix/bot/messages/on_room_message.rs b/server/src/chat/transport/matrix/bot/messages/on_room_message.rs index 831ea54c..2ff5d12f 100644 --- a/server/src/chat/transport/matrix/bot/messages/on_room_message.rs +++ b/server/src/chat/transport/matrix/bot/messages/on_room_message.rs @@ -924,7 +924,6 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message( rebuild_cmd.drain_timeout_secs, rebuild_cmd.force, store, - ctx.gateway_port, &ctx.services.project_root, ) .await diff --git a/server/src/chat/transport/matrix/project_rebuild.rs b/server/src/chat/transport/matrix/project_rebuild.rs index ecb96536..bef5e8b2 100644 --- a/server/src/chat/transport/matrix/project_rebuild.rs +++ b/server/src/chat/transport/matrix/project_rebuild.rs @@ -20,7 +20,6 @@ use crate::service::gateway::io::save_config; use std::collections::BTreeMap; use std::path::Path; use std::sync::Arc; -use std::time::Duration; use tokio::sync::RwLock; /// Default seconds to wait for in-flight work to drain before refusing. @@ -102,7 +101,6 @@ pub async fn handle_project_rebuild( drain_timeout_secs: u64, force: bool, projects_store: &Arc>>, - gateway_port: Option, config_dir: &Path, ) -> String { // ── 1. Validate project ────────────────────────────────────────────────── @@ -142,21 +140,6 @@ pub async fn handle_project_rebuild( ); } - // ── 1b. Capture the running binary's git_hash before we tear it down ────── - // Best-effort: an unreachable or pre-version-endpoint sled just yields `None`, - // in which case the post-rebuild reconciliation below can't compare old vs - // new and falls back to whatever the published fleet artifact says. - let http_client = reqwest::Client::builder() - .timeout(Duration::from_secs(15)) - .build() - .unwrap_or_default(); - let old_git_hash = match project_url.as_deref() { - Some(url) => super::sled_upgrade::fetch_sled_version(&http_client, url) - .await - .map(|(_version, hash)| hash), - None => None, - }; - // ── 2. Drain check ─────────────────────────────────────────────────────── let container_name = format!("huskies-{name}"); if !force @@ -317,20 +300,6 @@ pub async fn handle_project_rebuild( crate::slog!("[project-rebuild] Rebuilt '{name}': image={image_hash} container={container_id}"); - // ── 7. Make sure the rebuilt sled isn't running an older binary than before ── - let artifact_source = super::sled_upgrade::resolve_artifact_source(gateway_port); - let version_note = reconcile_post_rebuild_version( - name, - old_git_hash.as_deref(), - &container_url, - artifact_source, - projects_store, - config_dir, - &http_client, - 60, - ) - .await; - format!( "Project **{name}** rebuilt.\n\ - New image: `{image}` (`{image_short}…`)\n\ @@ -338,103 +307,10 @@ pub async fn handle_project_rebuild( - State: `pipeline.db` and CRDT preserved (same volume bind-mount)\n\ - Port: {port} (unchanged)\n\ \n\ - Use `switch {name}` then `status` to verify the pipeline.{version_note}" + Use `switch {name}` then `status` to verify the pipeline." ) } -/// Ensure the freshly-rebuilt sled at `container_url` isn't running an older -/// binary than the one it had before the rebuild (AC 1–3, story 1231). -/// -/// Base images are only refreshed by a manual `script/build-project-images` -/// run, so `project-rebuild` can otherwise silently downgrade a sled that was -/// live-upgraded past whatever binary is baked into the image. This waits for -/// the new container to come up, compares its `git_hash` against the -/// gateway's published fleet artifact, and — when they differ — self-heals by -/// running the same in-place upgrade `upgrade ` uses. When there's no -/// published artifact to converge to, it falls back to comparing against the -/// pre-rebuild hash so a real downgrade is at least surfaced as a warning -/// instead of passing silently. -/// -/// `artifact_source` is the already-resolved published-artifact lookup (see -/// [`super::sled_upgrade::resolve_artifact_source`]) — `Err` means no fleet -/// artifact has been published yet. -/// -/// Returns a Markdown snippet (starting with `\n\n`) to append to the rebuild -/// reply, or an empty string when the sled already matches the current build. -#[allow(clippy::too_many_arguments)] -async fn reconcile_post_rebuild_version( - name: &str, - old_git_hash: Option<&str>, - container_url: &str, - artifact_source: Result<(String, Option), String>, - projects_store: &Arc>>, - config_dir: &Path, - client: &reqwest::Client, - health_timeout_secs: u64, -) -> String { - let health_url = format!("{}/health", container_url.trim_end_matches('/')); - if !super::sled_upgrade::wait_for_health(client, &health_url, health_timeout_secs).await { - return format!( - "\n\n⚠️ **Warning:** the rebuilt sled did not respond to `/health` within 60s — \ - cannot verify its binary version. Check `docker logs huskies-{name}` and run \ - `upgrade {name}` once it's reachable." - ); - } - - let Some((_new_version, new_hash)) = - super::sled_upgrade::fetch_sled_version(client, container_url).await - else { - return "\n\n⚠️ **Warning:** the rebuilt sled is healthy but `/api/version` is unavailable — \ - cannot verify it isn't running a stale binary baked into the base image." - .to_string(); - }; - - let (source_url, expected_hash) = match artifact_source { - Ok(v) => v, - Err(_) => { - // No published fleet artifact to converge to — the best we can do is - // flag an actual change from what was running before the rebuild. - return match old_git_hash { - Some(old) if old != new_hash => format!( - "\n\n⚠️ **Warning:** the rebuilt sled is now on `{new_hash}` (was `{old}`) and \ - no fleet artifact is published to verify or fix this. Run `release` then \ - `upgrade {name}`." - ), - _ => String::new(), - }; - } - }; - - if expected_hash.as_deref() == Some(new_hash.as_str()) { - return String::new(); // base image already bakes the current fleet build. - } - - // The base image baked an older (or merely different) binary than the fleet - // is currently on — self-heal the same way `upgrade ` would. - let outcome = super::sled_upgrade::run_sled_upgrade( - name, - container_url, - &source_url, - expected_hash, - projects_store, - config_dir, - |_| async {}, - ) - .await; - - if outcome.contains("matches published artifact") || outcome.starts_with("upgraded to v") { - format!( - "\n\n♻️ The base image baked an older binary (`{new_hash}`); self-healed in place — \ - {outcome}" - ) - } else { - format!( - "\n\n⚠️ **Warning:** the base image baked an older binary (`{new_hash}`) and the \ - automatic self-heal failed: {outcome}\nRun `upgrade {name}` manually." - ) - } -} - /// Wait for active Claude agent processes in the container to exit. /// /// Polls every 5 seconds until no `claude` processes remain or `timeout_secs` elapses. @@ -634,7 +510,7 @@ mod tests { async fn rebuild_unknown_project_returns_error() { let store = make_store(vec![]); let dir = tempfile::tempdir().unwrap(); - let result = handle_project_rebuild("nonexistent", 0, true, &store, None, dir.path()).await; + let result = handle_project_rebuild("nonexistent", 0, true, &store, dir.path()).await; assert!( result.contains("not found"), "expected 'not found': {result}" @@ -654,7 +530,7 @@ mod tests { }, )]); let dir = tempfile::tempdir().unwrap(); - let result = handle_project_rebuild("myapp", 0, true, &store, None, dir.path()).await; + let result = handle_project_rebuild("myapp", 0, true, &store, dir.path()).await; assert!( result.contains("host_path"), "expected 'host_path' mention: {result}" @@ -674,7 +550,7 @@ mod tests { }, )]); let dir = tempfile::tempdir().unwrap(); - let result = handle_project_rebuild("myapp", 0, true, &store, None, dir.path()).await; + let result = handle_project_rebuild("myapp", 0, true, &store, dir.path()).await; assert!( result.contains("does not exist"), "expected 'does not exist': {result}" @@ -710,8 +586,7 @@ mod tests { )]); let config_dir = tempfile::tempdir().unwrap(); - let result = - handle_project_rebuild("myapp", 0, true, &store, None, config_dir.path()).await; + let result = handle_project_rebuild("myapp", 0, true, &store, config_dir.path()).await; // (a) Step naming: one of several possible failure steps depending on what Docker // binaries are available in the test environment, or a success reply. @@ -732,181 +607,4 @@ mod tests { "project 'myapp' must remain registered after failed rebuild: {result}" ); } - - // ── reconcile_post_rebuild_version (AC 1–3) ──────────────────────────── - - /// Spawn a minimal HTTP server that answers every `/health` request with - /// 200 and every `/api/version` request with `git_hash`. Serves - /// connections in a loop so both `wait_for_health` and - /// `fetch_sled_version` can hit it independently within one test. - fn spawn_version_server(git_hash: &str) -> (String, tokio::task::JoinHandle<()>) { - let git_hash = git_hash.to_string(); - let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); - listener.set_nonblocking(true).unwrap(); - let listener = tokio::net::TcpListener::from_std(listener).unwrap(); - let port = listener.local_addr().unwrap().port(); - let handle = tokio::spawn(async move { - loop { - let Ok((mut stream, _)) = listener.accept().await else { - break; - }; - let git_hash = git_hash.clone(); - tokio::spawn(async move { - use tokio::io::{AsyncReadExt, AsyncWriteExt}; - let mut buf = [0u8; 4096]; - let n = stream.read(&mut buf).await.unwrap_or(0); - let req = String::from_utf8_lossy(&buf[..n]); - let path = req.lines().next().unwrap_or("").to_string(); - let body = if path.contains("/api/version") { - serde_json::json!({"version": "0.14.2", "git_hash": git_hash}).to_string() - } else { - "ok".to_string() - }; - let resp = format!( - "HTTP/1.1 200 OK\r\nContent-Type: application/json\r\nContent-Length: {}\r\n\r\n{}", - body.len(), - body - ); - let _ = stream.write_all(resp.as_bytes()).await; - }); - } - }); - (format!("http://127.0.0.1:{port}"), handle) - } - - #[tokio::test] - async fn reconcile_empty_when_new_hash_matches_published_artifact() { - let (url, srv) = spawn_version_server("current-hash"); - let store = make_store(vec![]); - let config_dir = tempfile::tempdir().unwrap(); - let client = reqwest::Client::new(); - - let note = reconcile_post_rebuild_version( - "myapp", - Some("old-hash"), - &url, - Ok(( - "http://unused".to_string(), - Some("current-hash".to_string()), - )), - &store, - config_dir.path(), - &client, - 2, - ) - .await; - - assert_eq!(note, "", "already-current build must produce no note"); - srv.abort(); - } - - #[tokio::test] - async fn reconcile_empty_when_no_artifact_and_hash_unchanged() { - let (url, srv) = spawn_version_server("same-hash"); - let store = make_store(vec![]); - let config_dir = tempfile::tempdir().unwrap(); - let client = reqwest::Client::new(); - - let note = reconcile_post_rebuild_version( - "myapp", - Some("same-hash"), - &url, - Err("no published artifact".to_string()), - &store, - config_dir.path(), - &client, - 2, - ) - .await; - - assert_eq!(note, "", "unchanged hash must produce no note"); - srv.abort(); - } - - #[tokio::test] - async fn reconcile_warns_when_no_artifact_and_hash_changed() { - let (url, srv) = spawn_version_server("stale-baked-hash"); - let store = make_store(vec![]); - let config_dir = tempfile::tempdir().unwrap(); - let client = reqwest::Client::new(); - - let note = reconcile_post_rebuild_version( - "myapp", - Some("newer-hash-that-was-running"), - &url, - Err("no published artifact".to_string()), - &store, - config_dir.path(), - &client, - 2, - ) - .await; - - assert!(note.contains("Warning"), "expected a warning: {note}"); - assert!( - note.contains("stale-baked-hash") && note.contains("newer-hash-that-was-running"), - "warning should name both hashes: {note}" - ); - srv.abort(); - } - - #[tokio::test] - async fn reconcile_warns_when_health_probe_fails() { - let store = make_store(vec![]); - let config_dir = tempfile::tempdir().unwrap(); - let client = reqwest::Client::builder() - .timeout(std::time::Duration::from_millis(200)) - .build() - .unwrap(); - - let note = reconcile_post_rebuild_version( - "myapp", - Some("old-hash"), - "http://127.0.0.1:1", // nothing listens on port 1 - Err("no published artifact".to_string()), - &store, - config_dir.path(), - &client, - 2, - ) - .await; - - assert!( - note.contains("did not respond to `/health`"), - "expected a health-probe warning: {note}" - ); - } - - #[tokio::test] - async fn reconcile_attempts_self_heal_and_reports_failure_when_artifact_unreachable() { - let (url, srv) = spawn_version_server("stale-baked-hash"); - let store = make_store(vec![]); - let config_dir = tempfile::tempdir().unwrap(); - let client = reqwest::Client::new(); - - let note = reconcile_post_rebuild_version( - "myapp", - Some("stale-baked-hash"), - &url, - Ok(( - "http://127.0.0.1:1/api/artifacts/huskies-linux-arm64".to_string(), - Some("current-fleet-hash".to_string()), - )), - &store, - config_dir.path(), - &client, - 2, - ) - .await; - - assert!( - note.contains("self-heal failed"), - "mismatch should trigger a self-heal attempt that reports failure: {note}" - ); - assert!( - note.contains("stale-baked-hash"), - "note should name the stale hash: {note}" - ); - srv.abort(); - } } diff --git a/server/src/http/gateway/mcp.rs b/server/src/http/gateway/mcp.rs index cc4e3f3b..decf450c 100644 --- a/server/src/http/gateway/mcp.rs +++ b/server/src/http/gateway/mcp.rs @@ -1211,7 +1211,6 @@ async fn handle_project_rebuild_tool( drain_timeout_secs, force, &state.projects, - Some(state.port), &state.config_dir, ) .await;