Skip to main content

max / makenotwork

Namespace release tags per app, and refuse a dirty tree before a release Two gaps the first pom release walked into. Bento spelled every release tag `v{version}`, which assumes one repo is one product. MNW is one .git over the server, sando, multithreaded and pom, each versioned separately, so a bare `v0.4.1` there names no product in particular -- and pom and multithreaded are both AT 0.4.1, so it is ambiguous the day it is created rather than eventually. An app now sets `tag_format`, defaulting to `v{version}` so a single-product repo needs nothing; pom sets `pom-v{version}`. The worse one: the preflight pinned every host to the tag and compared their shas, but never checked the trees were clean. `git checkout <tag>` does not fail on local modifications to files whose content is unchanged in the tag -- it succeeds and KEEPS them. So a host with edits in the working tree builds those edits while honestly reporting the tagged sha, and the sha barrier sees two hosts in agreement because they genuinely are on the same commit. Two architectures, two different binaries, one tag containing neither. fw13 had uncommitted changes in pom's serve path and astra did not, so the very first release would have shipped it. The preflight now refuses a host with uncommitted changes to tracked files, naming the host and the files, the way it already refuses a detached HEAD. Untracked files are tolerated: a build host accumulates editor scratch that never reaches a binary, and failing on it would train an operator to bypass the gate.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-07-30 23:45 UTC
Signed with PGP, not checked
Commit: 9362c05f8580ba078710b997d2d4836f2e617f42
Parent: 58083c0
5 files changed, +255 insertions, -21 deletions
M pom/bento.toml +6
@@ -17,6 +17,12 @@
17 17 # Workspace root carries the version.
18 18 version_path = "Cargo.toml"
19 19
20 + # MNW is one .git over the server, sando, multithreaded, pom and more, each
21 + # versioned separately, so the default `v{version}` names no product in
22 + # particular here. pom and multithreaded are both at 0.4.1 as it stands, which
23 + # makes a bare `v0.4.1` ambiguous the day it is created rather than eventually.
24 + tag_format = "pom-v{version}"
25 +
20 26 # Neither instance ships without the other. pom watches production, and an
21 27 # aarch64 build that quietly failed while x86_64 shipped leaves half the mesh on
22 28 # an old binary with nothing saying so.
M bento/deploy/bento.toml.example +6
@@ -76,6 +76,12 @@
76 76 #
77 77 # Each service host needs the privileged installer and its scoped sudoers line
78 78 # installed once. See pom/deploy/install-service.sh and bento-deploy.sudoers.
79 + #
80 + # pom also sets `tag_format = "pom-v{version}"` in its own bento.toml. MNW is one
81 + # .git over several separately-versioned products, so the default `v{version}`
82 + # names none of them in particular -- and pom and multithreaded are both at
83 + # 0.4.1, so a bare tag is ambiguous immediately. A repo holding one product needs
84 + # nothing here.
79 85
80 86 [app.pom]
81 87 repo = "~/Code/MNW/pom"
M bento/daemon/src/engine.rs +49 -17
@@ -147,6 +147,11 @@
147 147 /// the host NAMES cannot answer, since a build host and a deploy
148 148 /// destination are declared in different files and need not agree on one.
149 149 pub build_host_ssh: String,
150 + /// This release's git tag, rendered from the app's `tag_format`. Held here
151 + /// rather than derived from the version because a repo holding several
152 + /// products spells it per product (`pom-v0.4.1`), and the recipe, the
153 + /// preflight barrier and the failure message all have to agree on it.
154 + pub tag: String,
150 155 /// The app's checkout path (topology `repo`, `~`-prefixed). Recipes read it
151 156 /// via `repo()` to `cd` into the checkout — commands don't auto-cd, and each
152 157 /// `sh` is a fresh shell.
@@ -246,6 +251,7 @@
246 251 target: Target,
247 252 build_host: String,
248 253 build_host_ssh: String,
254 + tag: String,
249 255 repo: String,
250 256 features: Vec<String>,
251 257 kind: Kind,
@@ -267,6 +273,7 @@
267 273 target,
268 274 build_host,
269 275 build_host_ssh,
276 + tag,
270 277 repo,
271 278 features,
272 279 kind,
@@ -579,13 +586,13 @@
579 586 // the checkout decides. Its output still streams into the step log, so an
580 587 // unreachable remote stays visible without being fatal.
581 588 let _ = self.run(host, &git_fetch_cmd(&self.repo))?;
582 - let (code, _) = self.run(host, &git_checkout_tag_cmd(&self.repo, &self.version))?;
589 + let (code, _) = self.run(host, &git_checkout_tag_cmd(&self.repo, &self.tag))?;
583 590 if code != 0 {
584 - let (probe, _) = self.run(host, &git_tag_exists_cmd(&self.repo, &self.version))?;
591 + let (probe, _) = self.run(host, &git_tag_exists_cmd(&self.repo, &self.tag))?;
585 592 anyhow::bail!(
586 - "checkout of v{} failed on `{host}`: {}",
587 - self.version,
588 - checkout_failure_reason(&self.version, probe == 0)
593 + "checkout of {} failed on `{host}`: {}",
594 + self.tag,
595 + checkout_failure_reason(&self.tag, probe == 0)
589 596 );
590 597 }
591 598 let (code, tail) = self.run(host, &git_rev_parse_cmd(&self.repo))?;
@@ -929,32 +936,51 @@
929 936
930 937 /// Pin the host's checkout to the release tag. Runs after [`git_fetch_cmd`], and
931 938 /// is the operation whose exit code decides whether the release proceeds.
932 - pub fn git_checkout_tag_cmd(repo: &str, version: &Version) -> String {
933 - format!("git -C {repo} checkout \"v{version}\"")
939 + pub fn git_checkout_tag_cmd(repo: &str, tag: &str) -> String {
940 + format!("git -C {repo} checkout \"{tag}\"")
934 941 }
935 942
936 - /// Does `v<version>` resolve to a commit in this checkout? Run only when the
937 - /// checkout has already failed, to say WHY: an absent tag is an untagged or
938 - /// unpushed release, while a tag that resolves fine means the checkout was
939 - /// refused for a local reason (a dirty tree, most often) and the operator needs
940 - /// to hear that instead.
941 - pub fn git_tag_exists_cmd(repo: &str, version: &Version) -> String {
942 - format!("git -C {repo} rev-parse -q --verify \"refs/tags/v{version}^{{commit}}\"")
943 + /// Does `tag` resolve to a commit in this checkout? Run only when the checkout
944 + /// has already failed, to say WHY: an absent tag is an untagged or unpushed
945 + /// release, while a tag that resolves fine means the checkout was refused for a
946 + /// local reason (a dirty tree, most often) and the operator needs to hear that
947 + /// instead.
948 + pub fn git_tag_exists_cmd(repo: &str, tag: &str) -> String {
949 + format!("git -C {repo} rev-parse -q --verify \"refs/tags/{tag}^{{commit}}\"")
943 950 }
944 951
945 952 /// The operator-facing explanation for a failed tag checkout, given whether the
946 953 /// tag turned out to exist locally.
947 - pub fn checkout_failure_reason(version: &Version, tag_exists: bool) -> String {
954 + pub fn checkout_failure_reason(tag: &str, tag_exists: bool) -> String {
948 955 if tag_exists {
949 956 format!(
950 - "tag v{version} exists but could not be checked out \
957 + "tag {tag} exists but could not be checked out \
951 958 (uncommitted changes in the checkout?)"
952 959 )
953 960 } else {
954 - format!("tag v{version} does not exist there (is it created and pushed?)")
961 + format!("tag {tag} does not exist there (is it created and pushed?)")
955 962 }
956 963 }
957 964
965 + /// Uncommitted changes to TRACKED files, one `XY path` line each, empty when the
966 + /// tree is clean.
967 + ///
968 + /// `--untracked-files=no` on purpose: an untracked file is not built into the
969 + /// binary and a build host accumulates them (editor scratch, stray logs), so
970 + /// failing a release on one would be noise. A modified tracked file is the
971 + /// opposite — it is exactly what `cargo build` would pick up instead of the
972 + /// tagged content.
973 + ///
974 + /// Scoped to `repo` with `-- .` rather than asking about the whole repository,
975 + /// which matters only for the repos holding more than one product. `repo` for
976 + /// pom is `~/Code/MNW/pom` inside the MNW monorepo, and an edit in `server/` is
977 + /// not something pom's build can compile. Refusing pom's release for it would be
978 + /// a gate that fires on unrelated work, which is how a gate gets bypassed. For a
979 + /// single-product repo `repo` is the root and this is the whole tree, unchanged.
980 + pub fn git_dirty_cmd(repo: &str) -> String {
981 + format!("git -C {repo} status --porcelain --untracked-files=no -- .")
982 + }
983 +
958 984 /// The branch a host's checkout is on, empty (and non-zero) on a detached HEAD.
959 985 /// Read BEFORE the release pins the tag, so the checkout can be put back
960 986 /// afterwards — see [`git_restore_branch_cmd`].
@@ -2190,6 +2216,7 @@
2190 2216 "linux/x86_64".parse().unwrap(),
2191 2217 "fw13".into(),
2192 2218 "local".into(),
2219 + "v0.1.0".into(),
2193 2220 "/tmp".into(),
2194 2221 vec![],
2195 2222 Kind::App,
@@ -2227,6 +2254,7 @@
2227 2254 "linux/x86_64".parse().unwrap(),
2228 2255 "fw13".into(),
2229 2256 "local".into(),
2257 + "v0.1.0".into(),
2230 2258 "/tmp".into(),
2231 2259 features,
2232 2260 Kind::App,
@@ -2281,6 +2309,7 @@
2281 2309 "linux/x86_64".parse().unwrap(),
2282 2310 "fw13".into(),
2283 2311 "local".into(),
2312 + "v0.1.0".into(),
2284 2313 "/tmp".into(),
2285 2314 vec![],
2286 2315 Kind::App,
@@ -2783,6 +2812,7 @@
2783 2812 "linux/x86_64".parse().unwrap(),
2784 2813 "fw13".into(),
2785 2814 "local".into(),
2815 + "v0.1.0".into(),
2786 2816 "/tmp".into(),
2787 2817 vec![],
2788 2818 Kind::Library,
@@ -2864,6 +2894,7 @@
2864 2894 "linux/x86_64".parse().unwrap(),
2865 2895 "fw13".into(),
2866 2896 "local".into(),
2897 + "v0.1.0".into(),
2867 2898 "/tmp".into(),
2868 2899 vec![],
2869 2900 Kind::Service,
@@ -2943,6 +2974,7 @@
2943 2974 "linux/x86_64".parse().unwrap(),
2944 2975 "fw13".into(),
2945 2976 "local".into(),
2977 + "v0.1.0".into(),
2946 2978 "/tmp".into(),
2947 2979 vec![],
2948 2980 Kind::Service,
M bento/daemon/src/runner.rs +113 -4
@@ -48,6 +48,9 @@
48 48 .app(app)
49 49 .ok_or_else(|| anyhow::anyhow!("unknown app `{app}`"))?;
50 50 let repo = cfg.repo.clone();
51 + // Spelled per app: a repo holding one product tags `v0.4.1`, a repo holding
52 + // several tags `pom-v0.4.1`. See `AppConfig::tag_format`.
53 + let tag = cfg.tag_for(version);
51 54
52 55 // The distinct hosts across the requested targets (order-stable).
53 56 let mut hosts: Vec<String> = Vec::new();
@@ -85,13 +88,40 @@
85 88 );
86 89 branches.push((host.clone(), branch));
87 90
91 + // A dirty tree defeats the pin silently, which is worse than not pinning
92 + // at all. `git checkout <tag>` does not fail on local modifications to
93 + // files whose content is unchanged in the tag — it succeeds and KEEPS
94 + // them. So a host with edits in the working tree builds those edits
95 + // while reporting the tagged sha, and a second host with a clean tree
96 + // builds something else. The rev-parse barrier below cannot see it:
97 + // both hosts genuinely are on the same commit. Two architectures, two
98 + // different binaries, one tag containing neither.
99 + let dirty_cmd = OpStep::shell(Action::Build, engine::git_dirty_cmd(&repo));
100 + let out = exec
101 + .run_streaming(&dirty_cmd, &mut sink)
102 + .await
103 + .with_context(|| format!("release preflight: reading tree state on `{host}`"))?;
104 + let dirty = String::from_utf8_lossy(&out.stdout);
105 + let dirty: Vec<&str> = dirty
106 + .lines()
107 + .map(str::trim)
108 + .filter(|l| !l.is_empty())
109 + .collect();
110 + anyhow::ensure!(
111 + dirty.is_empty(),
112 + "release preflight: `{repo}` on `{host}` has uncommitted changes to tracked \
113 + files, so the build would not be the tagged commit:\n {}\nCommit or stash \
114 + them before releasing.",
115 + dirty.join("\n "),
116 + );
117 +
88 118 // Advisory: `fetch --all` is non-zero if any one of a repo's remotes is
89 119 // unreachable, and blaming the tag for a dead mirror is what made this
90 120 // preflight misreport. Only the checkout below is allowed to fail.
91 121 let fetch = OpStep::shell(Action::Build, engine::git_fetch_cmd(&repo));
92 122 let _ = exec.run_streaming(&fetch, &mut sink).await;
93 123
94 - let checkout = OpStep::shell(Action::Build, engine::git_checkout_tag_cmd(&repo, version));
124 + let checkout = OpStep::shell(Action::Build, engine::git_checkout_tag_cmd(&repo, &tag));
95 125 let out = exec
96 126 .run_streaming(&checkout, &mut sink)
97 127 .await
@@ -99,14 +129,14 @@
99 129 if !out.status.success() {
100 130 // Ask git why before telling the operator. The two causes need
101 131 // opposite responses: tag-and-push, versus clean the working tree.
102 - let probe = OpStep::shell(Action::Build, engine::git_tag_exists_cmd(&repo, version));
132 + let probe = OpStep::shell(Action::Build, engine::git_tag_exists_cmd(&repo, &tag));
103 133 let tag_exists = exec
104 134 .run_streaming(&probe, &mut sink)
105 135 .await
106 136 .is_ok_and(|o| o.status.success());
107 137 anyhow::bail!(
108 - "release preflight: `git checkout v{version}` failed on `{host}`: {}",
109 - engine::checkout_failure_reason(version, tag_exists)
138 + "release preflight: `git checkout {tag}` failed on `{host}`: {}",
139 + engine::checkout_failure_reason(&tag, tag_exists)
110 140 );
111 141 }
112 142
@@ -454,6 +484,10 @@
454 484 .app(&app)
455 485 .map(|a| a.features.clone())
456 486 .unwrap_or_default();
487 + let tag = state
488 + .topo
489 + .app(&app)
490 + .map_or_else(|| format!("v{version}"), |a| a.tag_for(&version));
457 491 // The opt-in all-targets-green publish gate: pass the declared target set
458 492 // (so `publish` can require every sibling green) only when the app turns it
459 493 // on; otherwise None leaves independent per-target publishing unchanged.
@@ -489,6 +523,7 @@
489 523 target,
490 524 build_host,
491 525 build_host_ssh,
526 + tag.clone(),
492 527 repo,
493 528 features,
494 529 // Gates the `verify` step's capability: a library's crate preflight is
@@ -2214,6 +2249,80 @@
2214 2249 assert_eq!(status, "ok", "a pinned build should succeed ({error})");
2215 2250 }
2216 2251
2252 + /// The preflight refuses a host whose tree has uncommitted changes to
2253 + /// tracked files.
2254 + ///
2255 + /// This is the hole the rev-parse barrier cannot see. `git checkout <tag>`
2256 + /// does NOT fail on local modifications to files whose content is unchanged
2257 + /// in the tag — it succeeds and keeps them. So a dirty host builds its edits
2258 + /// while honestly reporting the tagged sha, and a clean host builds
2259 + /// something else: two architectures, two different binaries, one tag
2260 + /// containing neither combination. Caught on pom's first release, where
2261 + /// fw13 had uncommitted changes in the serve path and astra did not.
2262 + #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2263 + async fn release_preflight_refuses_a_dirty_working_tree() {
2264 + let tmp = tempfile::tempdir().unwrap();
2265 + let repo = tmp.path().join("demo");
2266 + init_git_app(&repo, "0.0.1", Some("v0.0.1"));
2267 +
2268 + // Modify a TRACKED file, exactly as an editor session would.
2269 + let tracked = repo.join("src-tauri/tauri.conf.json");
2270 + let body = std::fs::read_to_string(&tracked).unwrap();
2271 + std::fs::write(&tracked, format!("{body}\n")).unwrap();
2272 +
2273 + let mut cfg = Config::for_tests(tmp.path());
2274 + cfg.pin_release_sha = true;
2275 + let pool = crate::db::open(&cfg.db_path).await.unwrap();
2276 + let state = test_state(pool.clone(), one_host_topo(&repo), cfg);
2277 + let err = start_build(
2278 + state,
2279 + AppId::new("demo"),
2280 + Version::parse("0.0.1").unwrap(),
2281 + vec!["linux/x86_64".parse().unwrap()],
2282 + )
2283 + .await
2284 + .unwrap_err();
2285 + let msg = format!("{err:#}");
2286 + assert!(
2287 + msg.contains("uncommitted changes") && msg.contains("tauri.conf.json"),
2288 + "the error must name the host and the files, got: {msg}"
2289 + );
2290 + let builds: i64 = sqlx::query_scalar("SELECT COUNT(*) FROM builds")
2291 + .fetch_one(&pool)
2292 + .await
2293 + .unwrap();
2294 + assert_eq!(builds, 0, "a refused preflight writes no build row");
2295 + }
2296 +
2297 + /// An UNTRACKED file does not fail a release. A build host accumulates
2298 + /// editor scratch and stray logs, none of which reach the binary, so failing
2299 + /// on them would be noise that trains an operator to bypass the gate.
2300 + #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2301 + async fn release_preflight_tolerates_untracked_files() {
2302 + let tmp = tempfile::tempdir().unwrap();
2303 + let repo = tmp.path().join("demo");
2304 + init_git_app(&repo, "0.0.1", Some("v0.0.1"));
2305 + std::fs::write(repo.join("scratch.log"), "noise").unwrap();
2306 +
2307 + let mut cfg = Config::for_tests(tmp.path());
2308 + cfg.pin_release_sha = true;
2309 + let pool = crate::db::open(&cfg.db_path).await.unwrap();
2310 + let state = test_state(pool.clone(), one_host_topo(&repo), cfg);
2311 + let build_id = start_build(
2312 + state,
2313 + AppId::new("demo"),
2314 + Version::parse("0.0.1").unwrap(),
2315 + vec!["linux/x86_64".parse().unwrap()],
2316 + )
2317 + .await
2318 + .unwrap();
2319 + let (status, error) = await_target(&pool, build_id).await;
2320 + assert_eq!(
2321 + status, "ok",
2322 + "untracked files must not fail a release ({error})"
2323 + );
2324 + }
2325 +
2217 2326 /// The preflight refuses the build (before any row is written) when the
2218 2327 /// release tag does not exist on the host — a missing/unpushed tag can't
2219 2328 /// silently fall back to whatever `main` is.
M bento/daemon/src/topology.rs +81
@@ -139,6 +139,8 @@
139 139 /// Service install destinations, one per target. Empty for any other kind.
140 140 #[serde(default, rename = "deploy")]
141 141 deploy: Vec<DeployTarget>,
142 + #[serde(default = "default_tag_format")]
143 + tag_format: String,
142 144 }
143 145
144 146 #[derive(Debug, Clone, Copy, PartialEq, Eq, Default, Deserialize)]
@@ -245,6 +247,20 @@
245 247 /// Service install destinations, one per target (see [`DeployTarget`]).
246 248 /// Empty unless `kind = "service"`.
247 249 pub deploy: Vec<DeployTarget>,
250 + /// How this app's release tag is spelled, with `{version}` substituted.
251 + /// Defaults to `v{version}`, which is right for a repo holding one product.
252 + ///
253 + /// It exists for the repos that hold several. MNW is one `.git` over the
254 + /// server, sando, multithreaded, pom and more, each versioned separately, so
255 + /// a bare `v0.4.1` there names no product in particular — and pom and
256 + /// multithreaded are both at 0.4.1, so it is ambiguous the day it is
257 + /// created rather than eventually. Those apps set `pom-v{version}` and the
258 + /// tag says which release it is.
259 + pub tag_format: String,
260 + }
261 +
262 + fn default_tag_format() -> String {
263 + "v{version}".into()
248 264 }
249 265
250 266 impl AppConfig {
@@ -252,6 +268,11 @@
252 268 pub fn deploy_for(&self, target: Target) -> Option<&DeployTarget> {
253 269 self.deploy.iter().find(|d| d.target == target)
254 270 }
271 +
272 + /// This app's release tag for `version`.
273 + pub fn tag_for(&self, version: &crate::domain::Version) -> String {
274 + self.tag_format.replace("{version}", &version.to_string())
275 + }
255 276 }
256 277
257 278 fn default_branch() -> String {
@@ -375,6 +396,7 @@
375 396 require_all_targets: m.require_all_targets,
376 397 targets: m.targets,
377 398 deploy: m.deploy,
399 + tag_format: m.tag_format,
378 400 },
379 401 );
380 402 }
@@ -411,6 +433,21 @@
411 433 anyhow::bail!("app `{name}` ships target {t} but no host declares it");
412 434 }
413 435 }
436 + // The tag reaches a remote login shell inside `git checkout "..."`,
437 + // and it must actually vary per release.
438 + anyhow::ensure!(
439 + app.tag_format.contains("{version}"),
440 + "app `{name}`: tag_format `{}` must contain `{{version}}`, or every \
441 + release would resolve to the same tag",
442 + app.tag_format
443 + );
444 + anyhow::ensure!(
445 + !app.tag_format
446 + .chars()
447 + .any(|c| matches!(c, '"' | '`' | '$' | ';' | '&' | '|' | '\\' | ' ')),
448 + "app `{name}`: tag_format `{}` contains shell metacharacters",
449 + app.tag_format
450 + );
414 451 validate_deploy(name, app)?;
415 452 }
416 453 // Capability/transport coherence: a host that declares buildable targets
@@ -563,6 +600,50 @@
563 600 assert!(load(bad).is_err());
564 601 }
565 602
603 + /// A repo holding one product tags `v0.4.1`; a repo holding several has to
604 + /// say which product a tag is for. MNW is one `.git` over the server, sando,
605 + /// multithreaded and pom, and pom and multithreaded are BOTH at 0.4.1 — so a
606 + /// bare `v0.4.1` there is ambiguous the day it is created, not eventually.
607 + #[test]
608 + fn tag_format_defaults_to_v_and_can_name_the_product() {
609 + let v = |s: &str| crate::domain::Version::parse(s).unwrap();
610 +
611 + let t = load(HOSTS).unwrap();
612 + let app = t.app(&"goingson".into()).unwrap();
613 + assert_eq!(app.tag_format, "v{version}");
614 + assert_eq!(app.tag_for(&v("0.4.1")), "v0.4.1");
615 +
616 + let (t, _dir) = load_with(
617 + HOSTS,
618 + "targets = [\"linux/x86_64\"]\ntag_format = \"pom-v{version}\"\n",
619 + )
620 + .unwrap();
621 + assert_eq!(
622 + t.app(&"goingson".into()).unwrap().tag_for(&v("0.4.1")),
623 + "pom-v0.4.1"
624 + );
625 + }
626 +
627 + /// The tag is interpolated into `git checkout "..."` on a remote host, and
628 + /// it has to actually vary per release. A format with no `{version}` would
629 + /// pin every release to one tag, which is worse than failing.
630 + #[test]
631 + fn tag_format_must_vary_and_stay_shell_safe() {
632 + let bad = |f: &str| {
633 + load_with(
634 + HOSTS,
635 + &format!("targets = [\"linux/x86_64\"]\ntag_format = \"{f}\"\n"),
636 + )
637 + .is_err()
638 + };
639 + assert!(bad("release"), "a constant tag pins every release together");
640 + assert!(bad("v{version}; rm -rf /"));
641 + assert!(bad("v{version}$(id)"));
642 + assert!(bad("v{version} extra"));
643 + assert!(!bad("pom-v{version}"));
644 + assert!(!bad("release/{version}"));
645 + }
646 +
566 647 /// A service's `[[deploy]]` entries resolve, and the target -> destination
567 648 /// binding is what the runner reads. The recipe never names a host, so this
568 649 /// mapping is the only thing deciding which box each binary lands on.