Skip to main content

max / makenotwork

Unwind the release preflight, and name what blocked a checkout A preflight that refused on a later host left the earlier ones detached at the tag. The branches rode on the success value, which a refusal throws away, so nothing restored them -- and the next attempt refused those hosts for a detached HEAD, describing its own last run while reading as the operator's mistake. pin_release now splits: pin_hosts records each branch before it moves that host, and a failure reattaches every host it already pinned. The dirty gate is app-scoped on purpose, but the checkout it precedes is repo-wide, so a clean app directory and a refused checkout are consistent and between them said nothing actionable. On failure both the preflight and checkout_sha now list the repository's locally-modified tracked files, marking the ones outside the app. A failed restore logs the same list, since the file holding it up is usually one the build itself wrote.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-09 18:30 UTC
Signed with PGP, not checked
Commit: 5ef6b5b125872f0f4d2b2e055791bfae75973b63
Parent: 3fb2511
2 files changed, +362 insertions, -21 deletions
@@ -673,10 +673,16 @@
673 673 let (code, _) = self.run(host, &git_checkout_tag_cmd(&repo, &self.tag))?;
674 674 if code != 0 {
675 675 let (probe, _) = self.run(host, &git_tag_exists_cmd(&repo, &self.tag))?;
676 + // Ask the whole repository what is dirty, not just this app's
677 + // directory: the checkout that failed was repo-wide, so the file
678 + // holding it up need not be one this app would have compiled.
679 + let (_, status) = self.run(host, &git_repo_dirty_cmd(&repo))?;
680 + let (_, prefix) = self.run(host, &git_repo_prefix_cmd(&repo))?;
681 + let blocking = dirty_paths_blocking_checkout(&status, &prefix);
676 682 anyhow::bail!(
677 683 "checkout of {} failed on `{host}`: {}",
678 684 self.tag,
679 - checkout_failure_reason(&self.tag, probe == 0)
685 + checkout_failure_reason(&self.tag, probe == 0, blocking.as_deref())
680 686 );
681 687 }
682 688 let (code, tail) = self.run(host, &git_rev_parse_cmd(&repo))?;
@@ -1089,18 +1095,77 @@
1089 1095 }
1090 1096
1091 1097 /// The operator-facing explanation for a failed tag checkout, given whether the
1092 - /// tag turned out to exist locally.
1093 - pub fn checkout_failure_reason(tag: &str, tag_exists: bool) -> String {
1094 - if tag_exists {
1095 - format!(
1098 + /// tag turned out to exist locally and what [`dirty_paths_blocking_checkout`]
1099 + /// found in the working tree.
1100 + ///
1101 + /// With the file list in hand there is nothing left to guess at, so the
1102 + /// speculative "uncommitted changes?" is dropped in favour of naming them. It
1103 + /// stays for the case where the repository is clean and the checkout failed for
1104 + /// some other reason, because then a guess is all there is.
1105 + pub fn checkout_failure_reason(tag: &str, tag_exists: bool, blocking: Option<&str>) -> String {
1106 + match (tag_exists, blocking) {
1107 + (true, Some(paths)) => format!(
1108 + "tag {tag} exists but could not be checked out. `git checkout` acts on the whole \
1109 + repository, and these tracked files have local changes:\n {paths}\nCommit, stash \
1110 + or discard them before releasing."
1111 + ),
1112 + (true, None) => format!(
1096 1113 "tag {tag} exists but could not be checked out \
1097 1114 (uncommitted changes in the checkout?)"
1098 - )
1099 - } else {
1100 - format!("tag {tag} does not exist there (is it created and pushed?)")
1115 + ),
1116 + (false, _) => format!("tag {tag} does not exist there (is it created and pushed?)"),
1101 1117 }
1102 1118 }
1103 1119
1120 + /// Uncommitted changes to tracked files across the WHOLE repository, not just
1121 + /// the app's own directory.
1122 + ///
1123 + /// The gate is [`git_dirty_cmd`] and stays app-scoped for the reason given
1124 + /// there. This is diagnosis, run only once a checkout has already failed:
1125 + /// `git checkout <tag>` acts on the whole repository, so the file that blocked
1126 + /// it can sit outside the app entirely — at which point the app-scoped gate has
1127 + /// already reported a clean tree and the operator has nothing to go on. That is
1128 + /// what happened on pom's first hand-off release, where astra carried a
1129 + /// locally-resolved `server/Cargo.lock` and pom's own tree was spotless.
1130 + pub fn git_repo_dirty_cmd(repo: &str) -> String {
1131 + format!("git -C {repo} status --porcelain --untracked-files=no")
1132 + }
1133 +
1134 + /// The app's path relative to the repository root — `pom/` inside MNW, empty for
1135 + /// a repo holding one product. Porcelain status paths are repo-root relative,
1136 + /// so this is what decides whether a blocking file is the app's own.
1137 + pub fn git_repo_prefix_cmd(repo: &str) -> String {
1138 + format!("git -C {repo} rev-parse --show-prefix")
1139 + }
1140 +
1141 + /// Render the tracked files whose local changes block a repo-wide checkout, one
1142 + /// per line, marking the ones outside the app's own directory.
1143 + ///
1144 + /// `status` is [`git_repo_dirty_cmd`] output and `prefix` is
1145 + /// [`git_repo_prefix_cmd`] output. `None` when the repository is clean: then the
1146 + /// checkout failed for some other reason, and inventing a file to blame would be
1147 + /// worse than saying nothing.
1148 + pub fn dirty_paths_blocking_checkout(status: &str, prefix: &str) -> Option<String> {
1149 + let prefix = prefix.trim();
1150 + let lines: Vec<String> = status
1151 + .lines()
1152 + .filter_map(|l| {
1153 + // `XY path`, or `XY old -> new` for a rename. The destination is the
1154 + // path that exists in the working tree, so it is the one to name.
1155 + let path = l.get(3..)?.trim();
1156 + let path = path.rsplit(" -> ").next()?.trim();
1157 + if path.is_empty() {
1158 + return None;
1159 + }
1160 + if !prefix.is_empty() && !path.starts_with(prefix) {
1161 + return Some(format!("{path} (outside {prefix}, not part of this app)"));
1162 + }
1163 + Some(path.to_string())
1164 + })
1165 + .collect();
1166 + (!lines.is_empty()).then(|| lines.join("\n "))
1167 + }
1168 +
1104 1169 /// Uncommitted changes to TRACKED files, one `XY path` line each, empty when the
1105 1170 /// tree is clean.
1106 1171 ///
@@ -3433,4 +3498,56 @@
3433 3498 r#"{"status":"Invalid","message":"expected status:Accepted"}"#
3434 3499 ));
3435 3500 }
3501 +
3502 + /// Porcelain paths are repo-root relative, so an app that lives in a
3503 + /// subdirectory can tell its own files from a sibling product's. Both get
3504 + /// listed — the checkout was repo-wide and so is what blocked it — but only
3505 + /// the sibling's is marked, because that is the one the app-scoped dirty
3506 + /// gate just reported as clean.
3507 + #[test]
3508 + fn dirty_paths_mark_the_files_that_are_not_this_app_s() {
3509 + let status = " M server/Cargo.lock\n M pom/src/main.rs\n";
3510 + let out = dirty_paths_blocking_checkout(status, "pom/\n").unwrap();
3511 + assert!(
3512 + out.contains("server/Cargo.lock (outside pom/, not part of this app)"),
3513 + "{out}"
3514 + );
3515 + assert!(out.contains("pom/src/main.rs"), "{out}");
3516 + assert!(
3517 + !out.contains("pom/src/main.rs (outside"),
3518 + "the app's own file is not marked: {out}"
3519 + );
3520 + }
3521 +
3522 + /// A repo holding one product has no prefix, so nothing is "outside" it.
3523 + /// A rename is reported as `old -> new`, and the destination is the path
3524 + /// that exists in the working tree to go and look at.
3525 + #[test]
3526 + fn dirty_paths_handle_a_single_product_repo_and_renames() {
3527 + let out = dirty_paths_blocking_checkout("R a.rs -> b.rs\n M c.rs\n", "").unwrap();
3528 + assert_eq!(out, "b.rs\n c.rs");
3529 + assert!(dirty_paths_blocking_checkout("", "pom/").is_none());
3530 + assert!(dirty_paths_blocking_checkout("\n\n", "pom/").is_none());
3531 + }
3532 +
3533 + /// With the files in hand the message names them; with a clean tree it has
3534 + /// nothing to name and keeps the question. An absent tag is a different
3535 + /// failure and neither says anything about the working tree.
3536 + #[test]
3537 + fn checkout_failure_reason_names_files_when_it_has_them() {
3538 + let named = checkout_failure_reason("pom-v0.4.3", true, Some("server/Cargo.lock"));
3539 + assert!(named.contains("server/Cargo.lock"), "{named}");
3540 + assert!(
3541 + !named.contains("uncommitted changes in the checkout?"),
3542 + "no guessing once the files are known: {named}"
3543 + );
3544 + let guess = checkout_failure_reason("pom-v0.4.3", true, None);
3545 + assert!(
3546 + guess.contains("uncommitted changes in the checkout?"),
3547 + "{guess}"
3548 + );
3549 + let missing = checkout_failure_reason("pom-v0.4.3", false, Some("server/Cargo.lock"));
3550 + assert!(missing.contains("does not exist"), "{missing}");
3551 + assert!(!missing.contains("Cargo.lock"), "{missing}");
3552 + }
3436 3553 }
@@ -11,7 +11,7 @@
11 11 use crate::events::{self, Event};
12 12 use crate::state::AppState;
13 13 use anyhow::{Context, Result};
14 - use ops_exec::{Action, LogSink, Step as OpStep};
14 + use ops_exec::{Action, Executor, LogSink, Step as OpStep};
15 15 use std::path::PathBuf;
16 16 use std::sync::Arc;
17 17
@@ -46,12 +46,45 @@
46 46 /// known to be one value rather than a per-host answer. Resolving it here and
47 47 /// carrying it forward is what lets an artifact record name the source it was
48 48 /// built from without asking a build host to be honest about it afterwards.
49 + ///
50 + /// A refusal unwinds: every host this already detached goes back to the branch
51 + /// it was on. The hosts are pinned one after another, so a failure on the third
52 + /// used to leave the first two sitting on the tag with nothing to restore them —
53 + /// and the next attempt then refused for a detached HEAD, describing its own
54 + /// last run while reading as the operator's mistake. The barrier either pins all
55 + /// of them or leaves none of them moved.
49 56 async fn pin_release(
50 57 state: &AppState,
51 58 app: &AppId,
52 59 version: &Version,
53 60 targets: &[Target],
54 61 ) -> Result<Pinned> {
62 + let mut branches: Vec<(String, String)> = Vec::new();
63 + match pin_hosts(state, app, version, targets, &mut branches).await {
64 + Ok(sha) => Ok(Pinned { branches, sha }),
65 + Err(e) => {
66 + // Best-effort, and it cannot rescue the refusal: the release is
67 + // already not happening, so a restore that fails must not replace
68 + // the reason it was refused. `restore_branches` logs its own
69 + // failures for exactly that reason.
70 + restore_branches(state, app, &branches).await;
71 + Err(e)
72 + }
73 + }
74 + }
75 +
76 + /// The body of [`pin_release`]: pin each host in turn and return the commit they
77 + /// agreed on, recording into `branches` — before each checkout — the branch that
78 + /// host was on. `branches` is an out-parameter rather than a return value
79 + /// because the caller needs it on the failure path too, which is the whole point
80 + /// of the split.
81 + async fn pin_hosts(
82 + state: &AppState,
83 + app: &AppId,
84 + version: &Version,
85 + targets: &[Target],
86 + branches: &mut Vec<(String, String)>,
87 + ) -> Result<String> {
55 88 let cfg = state
56 89 .topo
57 90 .app(app)
@@ -85,7 +118,6 @@
85 118 );
86 119
87 120 let mut shas: Vec<(String, String)> = Vec::new();
88 - let mut branches: Vec<(String, String)> = Vec::new();
89 121 for host in &hosts {
90 122 let exec = state
91 123 .executors
@@ -169,9 +201,14 @@
169 201 .run_streaming(&probe, &mut sink)
170 202 .await
171 203 .is_ok_and(|o| o.status.success());
204 + // And which files, repo-wide. The gate above is app-scoped on
205 + // purpose, but the checkout it precedes is not, so a clean app
206 + // directory and a refused checkout are perfectly consistent — and
207 + // between them they say nothing the operator can act on.
208 + let blocking = repo_dirty_report(exec.as_ref(), repo).await;
172 209 anyhow::bail!(
173 210 "release preflight: `git checkout {tag}` failed on `{host}`: {}",
174 - engine::checkout_failure_reason(&tag, tag_exists)
211 + engine::checkout_failure_reason(&tag, tag_exists, blocking.as_deref())
175 212 );
176 213 }
177 214
@@ -229,10 +266,7 @@
229 266 short(first_sha),
230 267 mismatch.join(", "),
231 268 );
232 - Ok(Pinned {
233 - branches,
234 - sha: first_sha.clone(),
235 - })
269 + Ok(first_sha.clone())
236 270 }
237 271
238 272 /// What the preflight established: where each host's checkout was, and the one
@@ -244,6 +278,27 @@
244 278 sha: String,
245 279 }
246 280
281 + /// Which tracked files in `repo`'s whole repository have local changes, rendered
282 + /// for an operator and marked where they fall outside the app's own directory.
283 + ///
284 + /// Diagnosis, never a gate: both callers run it only after a `git checkout` has
285 + /// already failed, and both treat a probe that itself fails as "nothing to add"
286 + /// rather than as a second error. The reason the checkout failed is the message;
287 + /// this only says which file to go and look at.
288 + async fn repo_dirty_report(exec: &dyn Executor, repo: &str) -> Option<String> {
289 + async fn read(exec: &dyn Executor, cmd: String) -> String {
290 + let step = OpStep::shell(Action::Build, cmd);
291 + let mut sink = DiscardSink;
292 + match exec.run_streaming(&step, &mut sink).await {
293 + Ok(out) if out.status.success() => String::from_utf8_lossy(&out.stdout).into_owned(),
294 + _ => String::new(),
295 + }
296 + }
297 + let status = read(exec, engine::git_repo_dirty_cmd(repo)).await;
298 + let prefix = read(exec, engine::git_repo_prefix_cmd(repo)).await;
299 + engine::dirty_paths_blocking_checkout(&status, &prefix)
300 + }
301 +
247 302 /// Put every host's checkout back on the branch [`pin_release`] found it on.
248 303 ///
249 304 /// Best-effort and non-fatal: the release itself is already decided by the time
@@ -274,11 +329,20 @@
274 329 Ok(out) if out.status.success() => {
275 330 tracing::debug!(%host, %branch, "restored checkout to its branch");
276 331 }
277 - Ok(_) => tracing::error!(
278 - %host, %branch, %repo,
279 - "could not restore the checkout to its branch; it is left DETACHED at the \
280 - release tag, and commits made there will belong to no branch"
281 - ),
332 + // Name what held the restore up. The usual answer is a file the
333 + // build itself wrote — a regenerated `Cargo.lock`, most often — and
334 + // without it the next release refuses for uncommitted changes and
335 + // the operator gets to guess whether the edit was theirs.
336 + Ok(_) => {
337 + let blocking = repo_dirty_report(exec.as_ref(), repo)
338 + .await
339 + .unwrap_or_else(|| "(none; the working tree is clean)".into());
340 + tracing::error!(
341 + %host, %branch, %repo, %blocking,
342 + "could not restore the checkout to its branch; it is left DETACHED at the \
343 + release tag, and commits made there will belong to no branch"
344 + );
345 + }
282 346 Err(e) => tracing::error!(
283 347 %host, %branch, %repo, error = %e,
284 348 "could not restore the checkout to its branch; it is left DETACHED at the \
@@ -2392,6 +2456,19 @@
2392 2456 tauri_version: &str,
2393 2457 tag: Option<&str>,
2394 2458 recipe: &str,
2459 + ) {
2460 + init_git_app_shipping(repo, tauri_version, tag, recipe, "[\"linux/x86_64\"]");
2461 + }
2462 +
2463 + /// As [`init_git_app_with_recipe`], with the manifest's target list spelled
2464 + /// by the caller — a second target is what puts a second HOST in the
2465 + /// preflight, which is the only way to reach its unwind path.
2466 + fn init_git_app_shipping(
2467 + repo: &std::path::Path,
2468 + tauri_version: &str,
2469 + tag: Option<&str>,
2470 + recipe: &str,
2471 + targets: &str,
2395 2472 ) {
2396 2473 std::fs::create_dir_all(repo.join("src-tauri")).unwrap();
2397 2474 std::fs::write(
@@ -2399,7 +2476,7 @@
2399 2476 format!("{{\"version\":\"{tauri_version}\"}}"),
2400 2477 )
2401 2478 .unwrap();
2402 - std::fs::write(repo.join("bento.toml"), "targets = [\"linux/x86_64\"]\n").unwrap();
2479 + std::fs::write(repo.join("bento.toml"), format!("targets = {targets}\n")).unwrap();
2403 2480 std::fs::create_dir_all(repo.join("dist/recipes")).unwrap();
2404 2481 std::fs::write(repo.join("dist/recipes/linux.rhai"), recipe).unwrap();
2405 2482 // Isolate from the dev's global git config (which forces signed tags).
@@ -2793,6 +2870,153 @@
2793 2870 assert_eq!(builds, 0, "a refused preflight writes no build row");
2794 2871 }
2795 2872
2873 + /// A preflight that refuses on a LATER host puts the earlier ones back.
2874 + ///
2875 + /// The hosts are pinned one at a time, so by the time the second is refused
2876 + /// the first is already detached at the tag. Nothing used to restore it: the
2877 + /// branches were carried on the success value, which a refusal throws away.
2878 + /// The next attempt then refused that host for a detached HEAD — true, and
2879 + /// describing its own last run, which reads to the operator as their own
2880 + /// mistake. Hit three times releasing pom on 2026-08-09.
2881 + #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2882 + async fn a_refused_preflight_reattaches_the_hosts_it_already_pinned() {
2883 + let tmp = tempfile::tempdir().unwrap();
2884 + // fw13's checkout is tagged and clean, so it pins. mbp's has no v0.0.1,
2885 + // so it is refused after fw13 has already been moved.
2886 + let repo = tmp.path().join("demo");
2887 + init_git_app_shipping(
2888 + &repo,
2889 + "0.0.1",
2890 + Some("v0.0.1"),
2891 + "step(\"build\");\nsh_ok(build_host(), \"true\");\n",
2892 + "[\"linux/x86_64\", \"macos/aarch64\"]",
2893 + );
2894 + let other = tmp.path().join("demo-mbp");
2895 + init_git_app_shipping(
2896 + &other,
2897 + "0.0.1",
2898 + None,
2899 + "step(\"build\");\nsh_ok(build_host(), \"true\");\n",
2900 + "[\"linux/x86_64\", \"macos/aarch64\"]",
2901 + );
2902 + let branch_before = current_branch(&repo);
2903 + assert!(
2904 + !branch_before.is_empty(),
2905 + "fw13's checkout starts on a branch"
2906 + );
2907 +
2908 + let mut cfg = Config::for_tests(tmp.path());
2909 + cfg.pin_release_sha = true;
2910 + let pool = crate::db::open(&cfg.db_path).await.unwrap();
2911 + let topo = Topology::from_str_for_tests(&format!(
2912 + "[[host]]\nname = \"fw13\"\nssh = \"local\"\ntargets = [\"linux/x86_64\"]\n\
2913 + [[host]]\nname = \"mbp\"\nssh = \"local\"\ntargets = [\"macos/aarch64\"]\n\
2914 + [app.demo]\nrepo = \"{}\"\n[app.demo.repo_by_host]\nmbp = \"{}\"\n",
2915 + repo.display(),
2916 + other.display(),
2917 + ))
2918 + .unwrap();
2919 + let state = test_state(pool.clone(), topo, cfg);
2920 + let err = start_build(
2921 + state,
2922 + AppId::new("demo"),
2923 + Version::parse("0.0.1").unwrap(),
2924 + vec![
2925 + "linux/x86_64".parse().unwrap(),
2926 + "macos/aarch64".parse().unwrap(),
2927 + ],
2928 + )
2929 + .await
2930 + .unwrap_err();
2931 + let msg = format!("{err:#}");
2932 + assert!(
2933 + msg.contains("does not exist"),
2934 + "the refusal should still be mbp's missing tag, got: {msg}"
2935 + );
2936 + assert_eq!(
2937 + current_branch(&repo),
2938 + branch_before,
2939 + "a refused preflight must reattach the host it already pinned"
2940 + );
2941 + }
2942 +
2943 + /// A checkout refused by a dirty file OUTSIDE the app names that file, and
2944 + /// says it is outside.
2945 + ///
2946 + /// The dirty gate is scoped to the app's directory on purpose — an edit in
2947 + /// `server/` is not something pom's build compiles, and refusing pom's
2948 + /// release for it is how a gate gets bypassed. But `git checkout <tag>` acts
2949 + /// on the whole repository, and MNW is one `.git` over the server, sando,
2950 + /// multithreaded and pom. So the gate passes, the checkout fails, and
2951 + /// between them they used to say only "uncommitted changes in the checkout?"
2952 + /// about a directory that was demonstrably clean.
2953 + #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
2954 + async fn a_dirty_file_outside_the_app_is_named_as_such_when_it_blocks_the_checkout() {
2955 + let tmp = tempfile::tempdir().unwrap();
2956 + let root = tmp.path().join("monorepo");
2957 + let app = root.join("pom");
2958 + init_git_app(&app, "0.0.1", None);
2959 + let git = |args: &[&str]| {
2960 + let out = std::process::Command::new("git")
2961 + .args(args)
2962 + .current_dir(&root)
2963 + .env("GIT_CONFIG_GLOBAL", "/dev/null")
2964 + .env("GIT_CONFIG_SYSTEM", "/dev/null")
2965 + .env("GIT_AUTHOR_NAME", "t")
2966 + .env("GIT_AUTHOR_EMAIL", "t@t")
2967 + .env("GIT_COMMITTER_NAME", "t")
2968 + .env("GIT_COMMITTER_EMAIL", "t@t")
2969 + .output()
2970 + .expect("git runs");
2971 + assert!(
2972 + out.status.success(),
2973 + "git {args:?}: {}",
2974 + String::from_utf8_lossy(&out.stderr)
2975 + );
2976 + };
2977 + // `init_git_app` made `pom/` its own repo; the monorepo is the parent, so
2978 + // drop that and re-init one `.git` over both products.
2979 + std::fs::remove_dir_all(app.join(".git")).unwrap();
2980 + std::fs::create_dir_all(root.join("server")).unwrap();
2981 + std::fs::write(root.join("server/Cargo.lock"), "version = 4\n").unwrap();
2982 + git(&["init", "-q"]);
2983 + git(&["add", "-A"]);
2984 + git(&["commit", "-q", "-m", "init"]);
2985 + git(&["tag", "v0.0.1"]);
2986 + // The tag and the branch differ in `server/`, and the working copy of
2987 + // that file is modified — so the checkout cannot carry the edit across.
2988 + std::fs::write(root.join("server/Cargo.lock"), "version = 4\n# moved on\n").unwrap();
2989 + git(&["add", "-A"]);
2990 + git(&["commit", "-q", "-m", "server moves"]);
2991 + std::fs::write(
2992 + root.join("server/Cargo.lock"),
2993 + "version = 4\n# local edit\n",
2994 + )
2995 + .unwrap();
2996 +
2997 + let mut cfg = Config::for_tests(tmp.path());
2998 + cfg.pin_release_sha = true;
2999 + let pool = crate::db::open(&cfg.db_path).await.unwrap();
3000 + let state = test_state(pool.clone(), one_host_topo(&app), cfg);
3001 + let err = start_build(
3002 + state,
3003 + AppId::new("demo"),
3004 + Version::parse("0.0.1").unwrap(),
3005 + vec!["linux/x86_64".parse().unwrap()],
3006 + )
3007 + .await
3008 + .unwrap_err();
3009 + let msg = format!("{err:#}");
3010 + assert!(
3011 + msg.contains("server/Cargo.lock"),
3012 + "the refusal must name the file that blocked the checkout, got: {msg}"
3013 + );
3014 + assert!(
3015 + msg.contains("outside pom/"),
3016 + "and must say the file is not this app's, got: {msg}"
3017 + );
3018 + }
3019 +
2796 3020 /// The branch `repo` is on, empty on a detached HEAD.
2797 3021 fn current_branch(repo: &std::path::Path) -> String {
2798 3022 let out = std::process::Command::new("git")