Skip to main content

max / makenotwork

Refuse a release build no host was pinned for The preflight compared the hosts that reported a commit, so a target the topology could not resolve a host for was dropped from the pin while it still built. Agreement among the hosts that answered read as agreement among all of them, which is the mixed-source release the barrier exists to stop. Every requested target must now resolve to a host, every host must report, and no report may be empty, before the commits are compared at all. Also separate a failed branch read from a detached HEAD. `symbolic-ref -q` exits non-zero for both, and a wrong repo path was being reported as an earlier release having left the tree detached.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-06 22:50 UTC
Signed with PGP, not checked
Commit: 19f471b53e89c6d9ecb5702af1e8461d7af18396
Parent: 1edb098
2 files changed, +109 insertions, -20 deletions
M bento/daemon/src/runner.rs +108 -19
@@ -32,6 +32,10 @@
32 32 /// under the daemon host's version. This runs before any target task spawns, so
33 33 /// a mixed-source release is stopped before a single artifact is built.
34 34 ///
35 + /// "All" means all: a host that did not report is a refusal, not an abstention.
36 + /// Comparing only the hosts that answered proves agreement among those, which is
37 + /// not the claim the barrier is making.
38 + ///
35 39 /// Returns the branch each host was on before it was pinned, for
36 40 /// [`restore_branches`] to put back once the build settles. A host that is
37 41 /// ALREADY detached has no branch to return to, and is refused: that state means
@@ -53,14 +57,28 @@
53 57 let tag = cfg.tag_for(version);
54 58
55 59 // The distinct hosts across the requested targets (order-stable).
60 + //
61 + // A target whose host the topology cannot resolve is refused rather than
62 + // skipped. Skipping it would leave that target out of the barrier while it
63 + // still builds, which is the shape of hole this whole preflight exists to
64 + // close: agreement proven among some hosts reads as agreement among all.
65 + // `resolve_targets` rejects an unbuildable target before `/build`, so this
66 + // is unreachable through the API and cheap to state anyway.
56 67 let mut hosts: Vec<String> = Vec::new();
57 68 for t in targets {
58 - if let Some(h) = state.topo.host_for(*t)
59 - && !hosts.contains(&h.name)
60 - {
69 + let h = state
70 + .topo
71 + .host_for(*t)
72 + .ok_or_else(|| anyhow::anyhow!("release preflight: no host can build {t}"))?;
73 + if !hosts.contains(&h.name) {
61 74 hosts.push(h.name.clone());
62 75 }
63 76 }
77 + anyhow::ensure!(
78 + !hosts.is_empty(),
79 + "release preflight: no build hosts for v{version}; refusing to build a release \
80 + nothing was pinned for"
81 + );
64 82
65 83 let mut shas: Vec<(String, String)> = Vec::new();
66 84 let mut branches: Vec<(String, String)> = Vec::new();
@@ -78,6 +96,15 @@
78 96 .await
79 97 .with_context(|| format!("release preflight: reading branch on `{host}`"))?;
80 98 let branch = String::from_utf8_lossy(&out.stdout).trim().to_string();
99 + // `symbolic-ref -q` exits non-zero BOTH on a detached HEAD and when it
100 + // could not read the repo at all (wrong path, no checkout, no git).
101 + // Those need opposite responses, so a stderr that says anything is
102 + // reported as itself rather than folded into the detached-HEAD advice.
103 + let why = String::from_utf8_lossy(&out.stderr).trim().to_string();
104 + anyhow::ensure!(
105 + out.status.success() || why.is_empty(),
106 + "release preflight: reading the branch of `{repo}` on `{host}` failed: {why}"
107 + );
81 108 anyhow::ensure!(
82 109 out.status.success() && !branch.is_empty(),
83 110 "release preflight: `{repo}` on `{host}` is on a detached HEAD, so there is \
@@ -153,22 +180,47 @@
153 180 shas.push((host.clone(), sha));
154 181 }
155 182
156 - // The barrier: every host must be on the same commit.
157 - if let Some((first_host, first_sha)) = shas.first() {
158 - let mismatch: Vec<String> = shas
159 - .iter()
160 - .filter(|(_, sha)| sha != first_sha)
161 - .map(|(h, sha)| format!("{h}={}", short(sha)))
162 - .collect();
163 - anyhow::ensure!(
164 - mismatch.is_empty(),
165 - "release preflight: build hosts are on different commits for v{version} \
166 - ({}={}, {}); refusing to build a release from mixed sources",
167 - first_host,
168 - short(first_sha),
169 - mismatch.join(", "),
170 - );
171 - }
183 + // The barrier: every participating host must have reported, and every report
184 + // must be the same commit. Reporting is checked first and separately from
185 + // agreement, because a barrier that compares only what it received cannot
186 + // tell unanimity from silence — three hosts agreeing while a fourth was
187 + // never asked is exactly the mixed-source release this refuses.
188 + let missing: Vec<&str> = hosts
189 + .iter()
190 + .filter(|h| !shas.iter().any(|(sh, _)| sh == *h))
191 + .map(String::as_str)
192 + .collect();
193 + anyhow::ensure!(
194 + missing.is_empty(),
195 + "release preflight: {} did not report a commit for v{version}; refusing to build \
196 + a release the barrier cannot vouch for",
197 + missing.join(", "),
198 + );
199 + let empty_shas: Vec<&str> = shas
200 + .iter()
201 + .filter(|(_, sha)| sha.is_empty())
202 + .map(|(h, _)| h.as_str())
203 + .collect();
204 + anyhow::ensure!(
205 + empty_shas.is_empty(),
206 + "release preflight: `git rev-parse HEAD` returned nothing on {} for v{version}",
207 + empty_shas.join(", "),
208 + );
209 +
210 + let (first_host, first_sha) = &shas[0];
211 + let mismatch: Vec<String> = shas
212 + .iter()
213 + .filter(|(_, sha)| sha != first_sha)
214 + .map(|(h, sha)| format!("{h}={}", short(sha)))
215 + .collect();
216 + anyhow::ensure!(
217 + mismatch.is_empty(),
218 + "release preflight: build hosts are on different commits for v{version} \
219 + ({}={}, {}); refusing to build a release from mixed sources",
220 + first_host,
221 + short(first_sha),
222 + mismatch.join(", "),
223 + );
172 224 Ok(branches)
173 225 }
174 226
@@ -2277,6 +2329,43 @@
2277 2329 assert_eq!(status, "ok", "a pinned build should succeed ({error})");
2278 2330 }
2279 2331
2332 + /// The barrier refuses a target no host can build, instead of leaving it out
2333 + /// of the pin and letting the remaining hosts vouch for the release.
2334 + ///
2335 + /// The hole this closes is silence reading as agreement: the comparison used
2336 + /// to run over the hosts that reported, so a target dropped for want of a
2337 + /// host still built while the other hosts' unanimity looked like proof the
2338 + /// whole release came from one commit.
2339 + #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2340 + async fn release_preflight_refuses_a_target_no_host_can_build() {
2341 + let tmp = tempfile::tempdir().unwrap();
2342 + let repo = tmp.path().join("demo");
2343 + init_git_app(&repo, "0.0.1", Some("v0.0.1"));
2344 + let mut cfg = Config::for_tests(tmp.path());
2345 + cfg.pin_release_sha = true;
2346 + let pool = crate::db::open(&cfg.db_path).await.unwrap();
2347 + // The topology declares fw13 (linux/x86_64) only, so macOS has no host.
2348 + let state = test_state(pool.clone(), one_host_topo(&repo), cfg);
2349 + let err = start_build(
2350 + state,
2351 + AppId::new("demo"),
2352 + Version::parse("0.0.1").unwrap(),
2353 + vec!["macos/aarch64".parse().unwrap()],
2354 + )
2355 + .await
2356 + .unwrap_err();
2357 + let msg = format!("{err:#}");
2358 + assert!(
2359 + msg.contains("no host can build macos/aarch64"),
2360 + "the error must name the unbuildable target, got: {msg}"
2361 + );
2362 + let builds: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM builds")
2363 + .fetch_one(&pool)
2364 + .await
2365 + .unwrap();
2366 + assert_eq!(builds, 0, "a refused preflight writes no build row");
2367 + }
2368 +
2280 2369 /// The preflight refuses a host whose tree has uncommitted changes to
2281 2370 /// tracked files.
2282 2371 ///
M bento/daemon/src/state.rs +1 -1
@@ -109,7 +109,7 @@
109 109 /// Generous on purpose. The thing being watched is a manual upload, which
110 110 /// happens on human timescales, so a minute of staleness costs nothing and
111 111 /// keeps the poll off MNW's back.
112 - pub const DISTRIBUTION_TTL: std::time::Duration = std::time::Duration::from_secs(60);
112 + pub const DISTRIBUTION_TTL: std::time::Duration = std::time::Duration::from_mins(1);
113 113
114 114 /// Build one host's EXEC executor: `LocalExec` for `ssh = "local"`, `AgentRpc`
115 115 /// for an agent-transport host (macOS in-session signing), `SshExec` otherwise —