Skip to main content

max / makenotwork

Build a release in Bento's own worktree, not the operator's checkout A release checked the tag out in the ordinary checkout and put the branch back afterwards. That pinned a tree somebody works in for the length of a build -- editing in Helix during a release meant editing tag content -- and the build then dirtied it, so the restore failed and left it detached. The preflight had to refuse a dirty or detached checkout to compensate, which is a release blocked by the state of a tree it no longer needs to read. Each build host now declares a worktree_root, and a release builds in <worktree_root>/<repo>/<app>: one git worktree per app, detached at the tag, created on first use and re-pinned with --force after. Forcing is safe here in a way it never was in the checkout, because nothing but Bento writes there. Recipes are unchanged -- repo() resolves through the run's repo_by_host, which the preflight now fills with these paths. The root has to sit under the machine's code tree: ~/Code/.cargo/config.toml redirects the in-house git deps to the working copies, and cargo finds it by walking up from the build directory. It is an artifact root automatically rather than a second thing to declare, since the binary a release collects is built there and two settings that must agree can drift. Out with it: the branch restore and the pin-time dirty baseline from 30b9b1a7, the detached-HEAD and dirty-tree refusals, and the checkout-failure diagnosis that named files in a tree Bento no longer touches.
Co-Authored-By
Claude Opus 5 (1M context) <noreply@anthropic.com>
Author: Max Johnson <me@maxj.phd> · 2026-08-24 17:56 UTC
Signed with PGP, not checked
Commit: e69ee8ebdbbb3be1ae43f92fc6b2f582bea8bb5b
Parent: 30b9b1a
3 files changed, +399 insertions, -510 deletions
M bento/daemon/src/engine.rs +137 -256
@@ -659,30 +659,32 @@
659 659 Ok((code, tail))
660 660 }
661 661
662 - /// Pin `host` to the release tag `v<version>` and return the commit it now
663 - /// has checked out. Fetch + checkout stream into the current step's log;
664 - /// the sha comes from a separate `rev-parse` so its stdout is only the sha.
662 + /// Assert `host`'s build tree is at the release tag and return the commit,
663 + /// which is what a recipe's `checkout` step logs. Fetch + checkout stream
664 + /// into the current step's log; the sha comes from a separate `rev-parse` so
665 + /// its stdout is only the sha.
666 + ///
667 + /// The preflight has already put this tree at the tag before any recipe ran —
668 + /// this is the same operation again, on purpose, so that a recipe's own
669 + /// `checkout` step is a real step with a real log rather than a claim about
670 + /// something that happened elsewhere. Re-running it is cheap and, because the
671 + /// tree is Bento's own worktree, forcing: a build that has already dirtied it
672 + /// must not be able to fail its own retry.
665 673 fn checkout_sha(self: &Arc<Self>, host: &str) -> Result<String> {
666 674 // Every command below runs ON `host`, so the path is that host's, not the
667 - // daemon's. Windows is why: its checkout is at `C:/Users/me/Code/...`.
675 + // daemon's. Windows is why: its worktree is under `C:/Users/me/Code/...`.
668 676 let repo = self.repo_for(host).to_string();
669 677 // A failing mirror is not a failing release: fetch is advisory, and only
670 678 // the checkout decides. Its output still streams into the step log, so an
671 679 // unreachable remote stays visible without being fatal.
672 680 let _ = self.run(host, &git_fetch_cmd(&repo))?;
673 - let (code, _) = self.run(host, &git_checkout_tag_cmd(&repo, &self.tag))?;
681 + let (code, err) = self.run(host, &git_worktree_pin_cmd(&repo, &self.tag))?;
674 682 if code != 0 {
675 683 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);
682 684 anyhow::bail!(
683 685 "checkout of {} failed on `{host}`: {}",
684 686 self.tag,
685 - checkout_failure_reason(&self.tag, probe == 0, blocking.as_deref())
687 + worktree_failure_reason(&self.tag, probe == 0, &err)
686 688 );
687 689 }
688 690 let (code, tail) = self.run(host, &git_rev_parse_cmd(&repo))?;
@@ -1070,7 +1072,7 @@
1070 1072 /// the library repos carry three (`astra`, `mnw`, `srht`), so chaining this into
1071 1073 /// the checkout with `&&` meant one unreachable mirror aborted the release and
1072 1074 /// reported it as a missing tag. The checkout below is the step allowed to fail;
1073 - /// this one only has to try. See [`git_checkout_tag_cmd`].
1075 + /// this one only has to try. See [`git_worktree_pin_cmd`].
1074 1076 ///
1075 1077 /// `repo` is interpolated UNQUOTED so a leading `~` is expanded by the remote
1076 1078 /// host's shell (the checkout path is trusted topology config, not user input),
@@ -1079,12 +1081,6 @@
1079 1081 format!("git -C {repo} fetch --all --tags --prune")
1080 1082 }
1081 1083
1082 - /// Pin the host's checkout to the release tag. Runs after [`git_fetch_cmd`], and
1083 - /// is the operation whose exit code decides whether the release proceeds.
1084 - pub fn git_checkout_tag_cmd(repo: &str, tag: &str) -> String {
1085 - format!("git -C {repo} checkout \"{tag}\"")
1086 - }
1087 -
1088 1084 /// Does `tag` resolve to a commit in this checkout? Run only when the checkout
1089 1085 /// has already failed, to say WHY: an absent tag is an untagged or unpushed
1090 1086 /// release, while a tag that resolves fine means the checkout was refused for a
@@ -1094,174 +1090,101 @@
1094 1090 format!("git -C {repo} rev-parse -q --verify \"refs/tags/{tag}^{{commit}}\"")
1095 1091 }
1096 1092
1097 - /// The operator-facing explanation for a failed tag checkout, given whether the
1098 - /// tag turned out to exist locally and what [`dirty_paths_blocking_checkout`]
1099 - /// found in the working tree.
1093 + /// Which repository `repo` belongs to, and where `repo` sits inside it, in one
1094 + /// call: `--show-toplevel` then `--show-prefix`, one per line.
1100 1095 ///
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!(
1113 - "tag {tag} exists but could not be checked out \
1114 - (uncommitted changes in the checkout?)"
1115 - ),
1116 - (false, _) => format!("tag {tag} does not exist there (is it created and pushed?)"),
1096 + /// Both halves are needed to build in a worktree of a repo holding several
1097 + /// products. The worktree is made of the repository (`~/Code/MNW`), and the
1098 + /// recipe has to be pointed at the app inside it (`<worktree>/pom`).
1099 + pub fn git_toplevel_and_prefix_cmd(repo: &str) -> String {
1100 + format!("git -C {repo} rev-parse --show-toplevel --show-prefix")
1101 + }
1102 +
1103 + /// Read [`git_toplevel_and_prefix_cmd`]'s two lines.
1104 + ///
1105 + /// The prefix is empty for a repo holding one product, where `repo` IS the
1106 + /// repository root — and git prints an empty second line for it, so a missing
1107 + /// line is a malformed answer rather than that case.
1108 + pub fn parse_toplevel_and_prefix(out: &str) -> Option<(String, String)> {
1109 + let mut lines = out.split('\n');
1110 + let toplevel = lines.next()?.trim().to_string();
1111 + let prefix = lines.next()?.trim().to_string();
1112 + (!toplevel.is_empty()).then_some((toplevel, prefix))
1113 + }
1114 +
1115 + /// The repository's own directory name, which is what names its worktrees:
1116 + /// `MNW` for `/home/max/Code/MNW`.
1117 + ///
1118 + /// Splits on `/` only. Git reports `--show-toplevel` with forward slashes on
1119 + /// every platform, Windows included, so this is the separator to read.
1120 + pub fn repo_dir_name(toplevel: &str) -> &str {
1121 + toplevel
1122 + .trim_end_matches('/')
1123 + .rsplit('/')
1124 + .next()
1125 + .unwrap_or(toplevel)
1126 + }
1127 +
1128 + /// Where the app being released sits inside its worktree: the worktree root for
1129 + /// a repo holding one product, `<worktree>/pom` for one holding several.
1130 + pub fn app_dir_in_worktree(worktree: &str, prefix: &str) -> String {
1131 + let prefix = prefix.trim_matches('/');
1132 + if prefix.is_empty() {
1133 + worktree.to_string()
1134 + } else {
1135 + format!("{}/{prefix}", worktree.trim_end_matches('/'))
1117 1136 }
1118 1137 }
1119 1138
1120 - /// Uncommitted changes to tracked files across the WHOLE repository, not just
1121 - /// the app's own directory.
1139 + /// Does this worktree already exist? Run before deciding whether to create one.
1122 1140 ///
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")
1141 + /// `rev-parse --git-dir` rather than a shell test, because the one non-unix
1142 + /// build host has no `test`: every command Bento renders for a host is a git
1143 + /// command or something a recipe wrote.
1144 + pub fn git_worktree_probe_cmd(worktree: &str) -> String {
1145 + format!("git -C \"{worktree}\" rev-parse --git-dir")
1132 1146 }
1133 1147
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")
1148 + /// Forget worktrees whose directories are gone. Run before creating one: a
1149 + /// directory somebody deleted by hand is still registered in the repository, and
1150 + /// `worktree add` refuses the path as in use rather than rebuilding it.
1151 + pub fn git_worktree_prune_cmd(toplevel: &str) -> String {
1152 + format!("git -C \"{toplevel}\" worktree prune")
1139 1153 }
1140 1154
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.
1155 + /// Create this app's build worktree, detached at the release tag. Git creates
1156 + /// the leading directories, so the worktree root needs no preparation.
1157 + pub fn git_worktree_add_cmd(toplevel: &str, worktree: &str, tag: &str) -> String {
1158 + format!("git -C \"{toplevel}\" worktree add --detach --force \"{worktree}\" \"{tag}\"")
1159 + }
1160 +
1161 + /// Put an existing build worktree at the release tag.
1143 1162 ///
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 - let path = porcelain_path(l)?;
1154 - if !prefix.is_empty() && !path.starts_with(prefix) {
1155 - return Some(format!("{path} (outside {prefix}, not part of this app)"));
1156 - }
1157 - Some(path.to_string())
1158 - })
1159 - .collect();
1160 - (!lines.is_empty()).then(|| lines.join("\n "))
1163 + /// `--force` discards whatever the last release left in it — a rewritten
1164 + /// `Cargo.lock`, most often — and that is safe here in a way it never was in the
1165 + /// ordinary checkout: nothing but Bento writes in this tree, so there is no edit
1166 + /// of anybody's to lose. Owning the tree is what buys the forcing.
1167 + pub fn git_worktree_pin_cmd(worktree: &str, tag: &str) -> String {
1168 + format!("git -C \"{worktree}\" checkout --detach --force \"{tag}\"")
1161 1169 }
1162 1170
1163 - /// The path one `git status --porcelain` line is about, or `None` for a line
1164 - /// that names none.
1165 - ///
1166 - /// `XY path`, or `XY old -> new` for a rename: the destination is the path that
1167 - /// exists in the working tree, so it is the one every caller wants. A path git
1168 - /// had to quote (one carrying a newline, a quote or a non-UTF-8 byte) arrives
1169 - /// C-quoted and surrounded by `"`, and is returned as git wrote it — see
1170 - /// [`build_written_paths`], which refuses to act on one rather than unescaping
1171 - /// it wrongly.
1172 - fn porcelain_path(line: &str) -> Option<&str> {
1173 - let path = line.get(3..)?.trim();
1174 - let path = path.rsplit(" -> ").next()?.trim();
1175 - (!path.is_empty()).then_some(path)
1176 - }
1177 -
1178 - /// Every tracked path with local changes, repo-root relative, from
1179 - /// [`git_repo_dirty_cmd`] output.
1180 - pub fn dirty_paths(status: &str) -> Vec<String> {
1181 - status
1182 - .lines()
1183 - .filter_map(porcelain_path)
1184 - .map(str::to_string)
1185 - .collect()
1186 - }
1187 -
1188 - /// What the build itself wrote: dirty now, and clean when the release pinned the
1171 + /// The operator-facing explanation for a worktree that could not be put at the
1189 1172 /// tag.
1190 1173 ///
1191 - /// This is the whole safety argument for discarding anything. The preflight
1192 - /// records the repository's dirty set at pin time, so a file already carrying
1193 - /// somebody's edits before the release started is in `baseline` and is left
1194 - /// alone; what is left is only what appeared while the recipe ran. In practice
1195 - /// that is `Cargo.lock`, which every build rewrites under the `[patch]` block in
1196 - /// `~/Code/.cargo/config.toml` and which is meaningless churn — but the argument
1197 - /// is about provenance, not about the filename, so nothing here knows what a
1198 - /// lockfile is.
1199 - ///
1200 - /// A C-quoted path (git quotes one holding a newline, a quote or a non-UTF-8
1201 - /// byte) is dropped rather than unescaped: getting that wrong means discarding
1202 - /// a file nobody asked about. Dropping it leaves the tree dirty and the restore
1203 - /// then fails loudly, which is the safe direction.
1204 - pub fn build_written_paths(baseline: &[String], now: &[String]) -> Vec<String> {
1205 - now.iter()
1206 - .filter(|p| !p.starts_with('"'))
1207 - .filter(|p| !baseline.contains(p))
1208 - .cloned()
1209 - .collect()
1210 - }
1211 -
1212 - /// Discard the build's own edits, so the tree can go back to its branch.
1213 - ///
1214 - /// `paths` are repo-root relative and are spelled with git's `:/` pathspec
1215 - /// prefix, which is root-anchored whatever directory `-C` names. That matters
1216 - /// for the repos holding several products: `repo` for pom is `~/Code/MNW/pom`,
1217 - /// and the lockfile a pom build rewrites is `pom/Cargo.lock` from the root.
1218 - ///
1219 - /// `checkout HEAD --` rather than `checkout --`, so a change that somehow
1220 - /// reached the index is undone too. HEAD here is the release tag, which is what
1221 - /// the build compiled; the branch checkout that follows moves these files on to
1222 - /// the branch's content.
1223 - pub fn git_discard_paths_cmd(repo: &str, paths: &[String]) -> String {
1224 - let specs: Vec<String> = paths.iter().map(|p| format!("\":/{p}\"")).collect();
1225 - format!("git -C {repo} checkout HEAD -- {}", specs.join(" "))
1226 - }
1227 -
1228 - /// Uncommitted changes to TRACKED files, one `XY path` line each, empty when the
1229 - /// tree is clean.
1230 - ///
1231 - /// `--untracked-files=no` on purpose: an untracked file is not built into the
1232 - /// binary and a build host accumulates them (editor scratch, stray logs), so
1233 - /// failing a release on one would be noise. A modified tracked file is the
1234 - /// opposite — it is exactly what `cargo build` would pick up instead of the
1235 - /// tagged content.
1236 - ///
1237 - /// Scoped to `repo` with `-- .` rather than asking about the whole repository,
1238 - /// which matters only for the repos holding more than one product. `repo` for
1239 - /// pom is `~/Code/MNW/pom` inside the MNW monorepo, and an edit in `server/` is
1240 - /// not something pom's build can compile. Refusing pom's release for it would be
1241 - /// a gate that fires on unrelated work, which is how a gate gets bypassed. For a
1242 - /// single-product repo `repo` is the root and this is the whole tree, unchanged.
1243 - pub fn git_dirty_cmd(repo: &str) -> String {
1244 - format!("git -C {repo} status --porcelain --untracked-files=no -- .")
1245 - }
1246 -
1247 - /// The branch a host's checkout is on, empty (and non-zero) on a detached HEAD.
1248 - /// Read BEFORE the release pins the tag, so the checkout can be put back
1249 - /// afterwards — see [`git_restore_branch_cmd`].
1250 - pub fn git_current_branch_cmd(repo: &str) -> String {
1251 - format!("git -C {repo} symbolic-ref -q --short HEAD")
1252 - }
1253 -
1254 - /// Put a checkout back on the branch it was on before the release pinned it to
1255 - /// the tag.
1256 - ///
1257 - /// The pin itself is correct and deliberate: a release must build the tagged
1258 - /// commit, not the branch tip. What was missing is the other half. Leaving the
1259 - /// tree detached is invisible — git does not warn, and commits made afterwards
1260 - /// succeed normally while belonging to no branch. makeover shipped 2.3.0 from
1261 - /// exactly that state on 2026-07-28: three commits, including the published one,
1262 - /// existed only as a detached HEAD on one machine, on no branch and no remote.
1263 - pub fn git_restore_branch_cmd(repo: &str, branch: &str) -> String {
1264 - format!("git -C {repo} checkout \"{branch}\"")
1174 + /// An absent tag is an untagged or unpushed release and is the common case, so
1175 + /// it is answered plainly. Anything else is git's own stderr, which says more
1176 + /// about a path that is not a worktree, or a worktree another release holds,
1177 + /// than a guess would.
1178 + pub fn worktree_failure_reason(tag: &str, tag_exists: bool, stderr: &str) -> String {
1179 + if !tag_exists {
1180 + return format!("tag {tag} does not exist there (is it created and pushed?)");
1181 + }
1182 + let stderr = stderr.trim();
1183 + if stderr.is_empty() {
1184 + format!("tag {tag} exists, and git said nothing about why")
1185 + } else {
1186 + stderr.to_string()
1187 + }
1265 1188 }
1266 1189
1267 1190 /// The command a host runs to report the commit it has checked out, for the
@@ -3558,101 +3481,59 @@
3558 3481 ));
3559 3482 }
3560 3483
3561 - /// Porcelain paths are repo-root relative, so an app that lives in a
3562 - /// subdirectory can tell its own files from a sibling product's. Both get
3563 - /// listed — the checkout was repo-wide and so is what blocked it — but only
3564 - /// the sibling's is marked, because that is the one the app-scoped dirty
3565 - /// gate just reported as clean.
3484 + /// Both shapes of repo: one holding several products, and one holding a
3485 + /// single crate, where git prints an empty prefix line.
3566 3486 #[test]
3567 - fn dirty_paths_mark_the_files_that_are_not_this_app_s() {
3568 - let status = " M server/Cargo.lock\n M pom/src/main.rs\n";
3569 - let out = dirty_paths_blocking_checkout(status, "pom/\n").unwrap();
3487 + fn toplevel_and_prefix_read_both_shapes_of_repo() {
3488 + let (top, prefix) =
3489 + parse_toplevel_and_prefix("/home/max/Code/MNW\npom/\n").expect("two lines");
3490 + assert_eq!(top, "/home/max/Code/MNW");
3491 + assert_eq!(prefix, "pom/");
3492 + // A repo holding one product: git prints an empty second line.
3493 + let (top, prefix) =
3494 + parse_toplevel_and_prefix("/home/max/Code/Libraries/pter\n\n").expect("two lines");
3495 + assert_eq!(top, "/home/max/Code/Libraries/pter");
3496 + assert_eq!(prefix, "");
3570 3497 assert!(
3571 - out.contains("server/Cargo.lock (outside pom/, not part of this app)"),
3572 - "{out}"
3573 - );
3574 - assert!(out.contains("pom/src/main.rs"), "{out}");
3575 - assert!(
3576 - !out.contains("pom/src/main.rs (outside"),
3577 - "the app's own file is not marked: {out}"
3498 + parse_toplevel_and_prefix("").is_none(),
3499 + "no answer is not an answer"
3578 3500 );
3579 3501 }
3580 3502
3581 - /// A repo holding one product has no prefix, so nothing is "outside" it.
3582 - /// A rename is reported as `old -> new`, and the destination is the path
3583 - /// that exists in the working tree to go and look at.
3584 3503 #[test]
3585 - fn dirty_paths_handle_a_single_product_repo_and_renames() {
3586 - let out = dirty_paths_blocking_checkout("R a.rs -> b.rs\n M c.rs\n", "").unwrap();
3587 - assert_eq!(out, "b.rs\n c.rs");
3588 - assert!(dirty_paths_blocking_checkout("", "pom/").is_none());
3589 - assert!(dirty_paths_blocking_checkout("\n\n", "pom/").is_none());
3504 + fn repo_dir_name_is_the_last_segment_on_every_platform() {
3505 + assert_eq!(repo_dir_name("/home/max/Code/MNW"), "MNW");
3506 + assert_eq!(repo_dir_name("/home/max/Code/MNW/"), "MNW");
3507 + // Git reports forward slashes on Windows too.
3508 + assert_eq!(repo_dir_name("C:/Users/me/Code/Apps/goingson"), "goingson");
3590 3509 }
3591 3510
3592 - /// With the files in hand the message names them; with a clean tree it has
3593 - /// nothing to name and keeps the question. An absent tag is a different
3594 - /// failure and neither says anything about the working tree.
3511 + /// The app's directory inside its worktree, for both repo shapes.
3595 3512 #[test]
3596 - fn checkout_failure_reason_names_files_when_it_has_them() {
3597 - let named = checkout_failure_reason("pom-v0.4.3", true, Some("server/Cargo.lock"));
3598 - assert!(named.contains("server/Cargo.lock"), "{named}");
3599 - assert!(
3600 - !named.contains("uncommitted changes in the checkout?"),
3601 - "no guessing once the files are known: {named}"
3513 + fn app_dir_in_worktree_follows_the_prefix() {
3514 + assert_eq!(
3515 + app_dir_in_worktree("/home/max/Code/.bento/MNW/pom", "pom/"),
3516 + "/home/max/Code/.bento/MNW/pom/pom"
3602 3517 );
3603 - let guess = checkout_failure_reason("pom-v0.4.3", true, None);
3604 - assert!(
3605 - guess.contains("uncommitted changes in the checkout?"),
3606 - "{guess}"
3518 + assert_eq!(
3519 + app_dir_in_worktree("/home/max/Code/.bento/pter/pter", ""),
3520 + "/home/max/Code/.bento/pter/pter"
3607 3521 );
3608 - let missing = checkout_failure_reason("pom-v0.4.3", false, Some("server/Cargo.lock"));
3522 + }
3523 +
3524 + /// A missing tag is the common failure and gets a plain answer; anything
3525 + /// else is git's own stderr, which says more than a guess.
3526 + #[test]
3527 + fn worktree_failure_reason_names_the_tag_or_repeats_git() {
3528 + let missing = worktree_failure_reason("pom-v0.4.5", false, "irrelevant");
3609 3529 assert!(missing.contains("does not exist"), "{missing}");
3610 - assert!(!missing.contains("Cargo.lock"), "{missing}");
3611 - }
3612 -
3613 - #[test]
3614 - fn dirty_paths_reads_porcelain_including_renames() {
3615 - let status = " M pom/Cargo.lock\nM server/src/main.rs\nR old.rs -> new.rs\n\n";
3616 - assert_eq!(
3617 - dirty_paths(status),
3618 - vec!["pom/Cargo.lock", "server/src/main.rs", "new.rs"]
3619 - );
3620 - assert!(dirty_paths("").is_empty(), "a clean tree names no files");
3621 - }
3622 -
3623 - /// Only what appeared while the build ran is the build's, and a path git had
3624 - /// to quote is nobody's business to discard.
3625 - #[test]
3626 - fn build_written_paths_subtracts_the_pin_time_baseline() {
3627 - let baseline = vec!["server/notes.txt".to_string()];
3628 - let now = vec![
3629 - "server/notes.txt".to_string(),
3630 - "pom/Cargo.lock".to_string(),
3631 - "\"weird\\nname\"".to_string(),
3632 - ];
3633 - assert_eq!(
3634 - build_written_paths(&baseline, &now),
3635 - vec!["pom/Cargo.lock"],
3636 - "the pre-existing edit stays, and the quoted path is left alone"
3637 - );
3638 - assert!(
3639 - build_written_paths(&baseline, &baseline).is_empty(),
3640 - "a build that wrote nothing has nothing discarded"
3641 - );
3642 - }
3643 -
3644 - /// The pathspecs are root-anchored, which is what makes this correct for the
3645 - /// repos holding several products: `repo` is `~/Code/MNW/pom` and the path
3646 - /// is `pom/Cargo.lock` from the root.
3647 - #[test]
3648 - fn git_discard_paths_cmd_anchors_pathspecs_at_the_repo_root() {
3649 - let cmd = git_discard_paths_cmd(
3650 - "~/Code/MNW/pom",
3651 - &["pom/Cargo.lock".to_string(), "wam/Cargo.lock".to_string()],
3652 - );
3653 - assert_eq!(
3654 - cmd,
3655 - "git -C ~/Code/MNW/pom checkout HEAD -- \":/pom/Cargo.lock\" \":/wam/Cargo.lock\""
3530 + let held = worktree_failure_reason(
3531 + "pom-v0.4.5",
3532 + true,
3533 + "fatal: '/home/max/Code/.bento/MNW/pom' already exists",
3656 3534 );
3535 + assert!(held.contains("already exists"), "{held}");
3536 + let silent = worktree_failure_reason("pom-v0.4.5", true, " ");
3537 + assert!(silent.contains("said nothing"), "{silent}");
3657 3538 }
3658 3539 }
M bento/daemon/src/runner.rs +173 -251
@@ -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, Executor, LogSink, Step as OpStep};
14 + use ops_exec::{Action, LogSink, Step as OpStep};
15 15 use std::path::PathBuf;
16 16 use std::sync::Arc;
17 17
@@ -25,66 +25,53 @@
25 25 async fn write_chunk(&mut self, _bytes: &[u8]) {}
26 26 }
27 27
28 - /// Preflight barrier: pin every host that will build a target for this release
29 - /// to the tag `v<version>` and refuse the build unless they all report the SAME
30 - /// commit. Recipes used to `git pull --ff-only` per host, so `mbp`/`astra`/`fw13`
31 - /// each built whatever `main` was at pull time and the artifacts were filed
32 - /// under the daemon host's version. This runs before any target task spawns, so
33 - /// a mixed-source release is stopped before a single artifact is built.
28 + /// Preflight barrier: put every host that will build a target for this release
29 + /// into a worktree detached at the tag `v<version>`, and refuse the build unless
30 + /// they all report the SAME commit. Recipes used to `git pull --ff-only` per
31 + /// host, so `mbp`/`astra`/`fw13` each built whatever `main` was at pull time and
32 + /// the artifacts were filed under the daemon host's version. This runs before
33 + /// any target task spawns, so a mixed-source release is stopped before a single
34 + /// artifact is built.
34 35 ///
35 36 /// "All" means all: a host that did not report is a refusal, not an abstention.
36 37 /// Comparing only the hosts that answered proves agreement among those, which is
37 38 /// not the claim the barrier is making.
38 39 ///
39 - /// Returns what each host looked like before it was pinned, for
40 - /// [`restore_branches`] to put back once the build settles, and the commit they
41 - /// all agreed on. A host that is ALREADY detached has no branch to return to,
42 - /// and is refused: that state means some earlier release never cleaned up, and
43 - /// any commits made in the meantime are sitting on no branch at all.
44 - ///
45 40 /// The agreed sha is the release's provenance, and this is the only place it is
46 41 /// known to be one value rather than a per-host answer. Resolving it here and
47 42 /// carrying it forward is what lets an artifact record name the source it was
48 43 /// built from without asking a build host to be honest about it afterwards.
49 44 ///
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.
45 + /// ## The tree a release builds in is Bento's, not the operator's
46 + ///
47 + /// This used to check the tag out in the ordinary checkout and put that tree
48 + /// back on its branch afterwards. Three things came out of that, and all three
49 + /// are gone with it (ruled 2026-08-24, task `6b808f26`):
50 + ///
51 + /// - A release pinned a tree somebody works in. Editing in Helix while one ran
52 + /// meant editing the tag's content for the length of the build.
53 + /// - The build dirtied that tree (`cargo` rewrites `Cargo.lock` under the
54 + /// `[patch]` block), so the restore's `git checkout <branch>` failed and left
55 + /// the tree detached at the tag — and the next release was refused for a
56 + /// detached HEAD, or for an uncommitted change nobody made.
57 + /// - The preflight therefore had to refuse a dirty or already-detached checkout,
58 + /// which is a release blocked by the state of a tree it no longer reads.
59 + ///
60 + /// A worktree costs the working files only — it shares the repository's object
61 + /// store — and is re-pinned with `--force`, which is safe precisely because
62 + /// nothing but Bento writes there. Where it lives is [`Host::worktree_root`],
63 + /// and that doc carries why the path has to sit under `~/Code`.
64 + ///
65 + /// Returns where each host will build, for the recipes to be pointed at, and the
66 + /// commit they all agreed on. Nothing to unwind on a refusal: a host prepared
67 + /// before the refusal is left holding a worktree at a tag, which is what the
68 + /// next release would put it at anyway.
56 69 async fn pin_release(
57 70 state: &AppState,
58 71 app: &AppId,
59 72 version: &Version,
60 73 targets: &[Target],
61 74 ) -> Result<Pinned> {
62 - let mut branches: Vec<PinnedHost> = 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<PinnedHost>,
87 - ) -> Result<String> {
88 75 let cfg = state
89 76 .topo
90 77 .app(app)
@@ -101,159 +88,24 @@
101 88 // close: agreement proven among some hosts reads as agreement among all.
102 89 // `resolve_targets` rejects an unbuildable target before `/build`, so this
103 90 // is unreachable through the API and cheap to state anyway.
104 - let mut hosts: Vec<String> = Vec::new();
91 + let mut hosts: Vec<&crate::topology::Host> = Vec::new();
105 92 for t in targets {
106 93 let h = state
107 94 .topo
108 95 .host_for(*t)
109 - .ok_or_else(|| anyhow::anyhow!("release preflight: no host can build {t}"))?;
110 - if !hosts.contains(&h.name) {
111 - hosts.push(h.name.clone());
96 + .ok_or_else(|| anyhow::anyhow!("no host can build {t}"))?;
97 + if !hosts.iter().any(|seen| seen.name == h.name) {
98 + hosts.push(h);
112 99 }
113 100 }
114 101 anyhow::ensure!(
115 102 !hosts.is_empty(),
116 - "release preflight: no build hosts for v{version}; refusing to build a release \
117 - nothing was pinned for"
103 + "no build hosts for v{version}; refusing to build a release nothing was pinned for"
118 104 );
119 105
120 - let mut shas: Vec<(String, String)> = Vec::new();
106 + let mut prepared: Vec<PinnedHost> = Vec::new();
121 107 for host in &hosts {
122 - let exec = state
123 - .executors
124 - .get(host)
125 - .ok_or_else(|| anyhow::anyhow!("no executor for host `{host}`"))?;
126 - let mut sink = DiscardSink;
127 - // Resolved per host, not once for the set: the checkouts need not be at
128 - // the same path on every machine, and pinning windows-x86 with the unix
129 - // path failed on the first git command of the release.
130 - let repo = cfg.repo_for(host);
131 -
132 - // Read the branch BEFORE pinning, while there is still one to read.
133 - let branch_cmd = OpStep::shell(Action::Build, engine::git_current_branch_cmd(repo));
134 - let out = exec
135 - .run_streaming(&branch_cmd, &mut sink)
136 - .await
137 - .with_context(|| format!("release preflight: reading branch on `{host}`"))?;
138 - let branch = String::from_utf8_lossy(&out.stdout).trim().to_string();
139 - // `symbolic-ref -q` exits non-zero BOTH on a detached HEAD and when it
140 - // could not read the repo at all (wrong path, no checkout, no git).
141 - // Those need opposite responses, so a stderr that says anything is
142 - // reported as itself rather than folded into the detached-HEAD advice.
143 - let why = String::from_utf8_lossy(&out.stderr).trim().to_string();
144 - anyhow::ensure!(
145 - out.status.success() || why.is_empty(),
146 - "release preflight: reading the branch of `{repo}` on `{host}` failed: {why}"
147 - );
148 - anyhow::ensure!(
149 - out.status.success() && !branch.is_empty(),
150 - "release preflight: `{repo}` on `{host}` is on a detached HEAD, so there is \
151 - no branch to restore it to after the release. An earlier release left it \
152 - that way; any commits made since are on no branch and may exist nowhere \
153 - else. Reattach it (`git checkout <branch>`, fast-forwarding if the commits \
154 - should be kept) before releasing."
155 - );
156 - branches.push(PinnedHost {
157 - host: host.clone(),
158 - branch,
159 - dirty_before: None,
160 - });
161 -
162 - // A dirty tree defeats the pin silently, which is worse than not pinning
163 - // at all. `git checkout <tag>` does not fail on local modifications to
164 - // files whose content is unchanged in the tag — it succeeds and KEEPS
165 - // them. So a host with edits in the working tree builds those edits
166 - // while reporting the tagged sha, and a second host with a clean tree
167 - // builds something else. The rev-parse barrier below cannot see it:
168 - // both hosts genuinely are on the same commit. Two architectures, two
169 - // different binaries, one tag containing neither.
170 - let dirty_cmd = OpStep::shell(Action::Build, engine::git_dirty_cmd(repo));
171 - let out = exec
172 - .run_streaming(&dirty_cmd, &mut sink)
173 - .await
174 - .with_context(|| format!("release preflight: reading tree state on `{host}`"))?;
175 - let dirty = String::from_utf8_lossy(&out.stdout);
176 - let dirty: Vec<&str> = dirty
177 - .lines()
178 - .map(str::trim)
179 - .filter(|l| !l.is_empty())
180 - .collect();
181 - anyhow::ensure!(
182 - dirty.is_empty(),
183 - "release preflight: `{repo}` on `{host}` has uncommitted changes to tracked \
184 - files, so the build would not be the tagged commit:\n {}\nCommit or stash \
185 - them before releasing.",
186 - dirty.join("\n "),
187 - );
188 -
189 - // Advisory: `fetch --all` is non-zero if any one of a repo's remotes is
190 - // unreachable, and blaming the tag for a dead mirror is what made this
191 - // preflight misreport. Only the checkout below is allowed to fail.
192 - let fetch = OpStep::shell(Action::Build, engine::git_fetch_cmd(repo));
193 - let _ = exec.run_streaming(&fetch, &mut sink).await;
194 -
195 - let checkout = OpStep::shell(Action::Build, engine::git_checkout_tag_cmd(repo, &tag));
196 - let out = exec
197 - .run_streaming(&checkout, &mut sink)
198 - .await
199 - .with_context(|| format!("release preflight: checkout on `{host}`"))?;
200 - if !out.status.success() {
201 - // Ask git why before telling the operator. The two causes need
202 - // opposite responses: tag-and-push, versus clean the working tree.
203 - let probe = OpStep::shell(Action::Build, engine::git_tag_exists_cmd(repo, &tag));
204 - let tag_exists = exec
205 - .run_streaming(&probe, &mut sink)
206 - .await
207 - .is_ok_and(|o| o.status.success());
208 - // And which files, repo-wide. The gate above is app-scoped on
209 - // purpose, but the checkout it precedes is not, so a clean app
210 - // directory and a refused checkout are perfectly consistent — and
211 - // between them they say nothing the operator can act on.
212 - let blocking = repo_dirty_report(exec.as_ref(), repo).await;
213 - anyhow::bail!(
214 - "release preflight: `git checkout {tag}` failed on `{host}`: {}",
215 - engine::checkout_failure_reason(&tag, tag_exists, blocking.as_deref())
216 - );
217 - }
218 -
219 - // The tree as the build inherits it. Whatever is dirty HERE was dirty
220 - // before the release and belongs to whoever wrote it; anything dirty at
221 - // the end that is not in this set was written by the build, and is the
222 - // only thing `restore_branches` is allowed to discard. Recorded after
223 - // the checkout rather than before it, because the checkout is what
224 - // decides which of the pre-existing edits survive onto the tag.
225 - //
226 - // Repo-wide, not app-scoped: `git checkout <branch>` on the way back is
227 - // repo-wide too, so a file outside the app is exactly as capable of
228 - // holding the restore up. pom's own releases dirtied `pom/Cargo.lock`
229 - // and one earlier one left `wam/Cargo.lock` behind.
230 - let dirty_now = OpStep::shell(Action::Build, engine::git_repo_dirty_cmd(repo));
231 - let baseline = match exec.run_streaming(&dirty_now, &mut sink).await {
232 - Ok(out) if out.status.success() => {
233 - Some(engine::dirty_paths(&String::from_utf8_lossy(&out.stdout)))
234 - }
235 - // A baseline that could not be read is not a reason to refuse a
236 - // release, but it must not be mistaken for "nothing was dirty":
237 - // that reading would let the restore discard somebody's edit. `None`
238 - // discards nothing, which is what this did before it discarded
239 - // anything at all.
240 - _ => None,
241 - };
242 - if let Some(pinned) = branches.last_mut() {
243 - pinned.dirty_before = baseline;
244 - }
245 -
246 - let rev = OpStep::shell(Action::Build, engine::git_rev_parse_cmd(repo));
247 - let out = exec
248 - .run_streaming(&rev, &mut sink)
249 - .await
250 - .with_context(|| format!("release preflight: rev-parse on `{host}`"))?;
251 - anyhow::ensure!(
252 - out.status.success(),
253 - "release preflight: `git rev-parse HEAD` failed on `{host}`"
254 - );
255 - let sha = String::from_utf8_lossy(&out.stdout).trim().to_string();
256 - shas.push((host.clone(), sha));
108 + prepared.push(prepare_worktree(state, app, cfg, host, &tag).await?);
257 109 }
258 110
259 111 // The barrier: every participating host must have reported, and every report
@@ -263,210 +115,172 @@
263 115 // never asked is exactly the mixed-source release this refuses.
264 116 let missing: Vec<&str> = hosts
265 117 .iter()
266 - .filter(|h| !shas.iter().any(|(sh, _)| sh == *h))
267 - .map(String::as_str)
118 + .filter(|h| !prepared.iter().any(|p| p.host == h.name))
119 + .map(|h| h.name.as_str())
268 120 .collect();
269 121 anyhow::ensure!(
270 122 missing.is_empty(),
271 - "release preflight: {} did not report a commit for v{version}; refusing to build \
272 - a release the barrier cannot vouch for",
123 + "{} did not report a commit for v{version}; refusing to build a release the \
124 + barrier cannot vouch for",
273 125 missing.join(", "),
274 126 );
275 - let empty_shas: Vec<&str> = shas
127 + let empty_shas: Vec<&str> = prepared
276 128 .iter()
277 - .filter(|(_, sha)| sha.is_empty())
278 - .map(|(h, _)| h.as_str())
129 + .filter(|p| p.sha.is_empty())
130 + .map(|p| p.host.as_str())
279 131 .collect();
280 132 anyhow::ensure!(
281 133 empty_shas.is_empty(),
282 - "release preflight: `git rev-parse HEAD` returned nothing on {} for v{version}",
134 + "`git rev-parse HEAD` returned nothing on {} for v{version}",
283 135 empty_shas.join(", "),
284 136 );
285 137
286 - let (first_host, first_sha) = &shas[0];
287 - let mismatch: Vec<String> = shas
138 + let first = &prepared[0];
139 + let mismatch: Vec<String> = prepared
288 140 .iter()
289 - .filter(|(_, sha)| sha != first_sha)
290 - .map(|(h, sha)| format!("{h}={}", short(sha)))
141 + .filter(|p| p.sha != first.sha)
142 + .map(|p| format!("{}={}", p.host, short(&p.sha)))
291 143 .collect();
292 144 anyhow::ensure!(
293 145 mismatch.is_empty(),
294 - "release preflight: build hosts are on different commits for v{version} \
146 + "build hosts are on different commits for v{version} \
295 147 ({}={}, {}); refusing to build a release from mixed sources",
296 - first_host,
297 - short(first_sha),
148 + first.host,
149 + short(&first.sha),
298 150 mismatch.join(", "),
299 151 );
300 - Ok(first_sha.clone())
152 +
153 + let sha = first.sha.clone();
154 + Ok(Pinned {
155 + build_dirs: prepared
156 + .into_iter()
157 + .map(|p| (p.host, p.build_dir))
158 + .collect(),
159 + sha,
160 + })
301 161 }
302 162
303 - /// What the preflight established: where each host's checkout was, and the one
304 - /// commit every host is now on.
163 + /// Put one host's worktree at the release tag and report the commit it landed
164 + /// on, creating the worktree the first time this app is released there.
165 + ///
166 + /// The worktree's own path is derived on the host rather than assumed: a repo
167 + /// holding several products is one `.git` over all of them, so `repo` for pom is
168 + /// `~/Code/MNW/pom` and the tree to make a worktree of is `~/Code/MNW`. One
169 + /// `rev-parse` answers both halves — which repository, and where the app sits
170 + /// inside it.
171 + async fn prepare_worktree(
172 + state: &AppState,
173 + app: &AppId,
174 + cfg: &crate::topology::AppConfig,
175 + host: &crate::topology::Host,
176 + tag: &str,
177 + ) -> Result<PinnedHost> {
178 + let exec = state
179 + .executors
180 + .get(&host.name)
181 + .ok_or_else(|| anyhow::anyhow!("no executor for host `{}`", host.name))?;
182 + let name = &host.name;
183 + // Resolved per host, not once for the set: the checkouts need not be at the
184 + // same path on every machine, and pinning windows-x86 with the unix path
185 + // failed on the first git command of the release.
186 + let repo = cfg.repo_for(name);
187 +
188 + let read = |cmd: String| async {
189 + let step = OpStep::shell(Action::Build, cmd);
190 + let mut sink = DiscardSink;
191 + exec.run_streaming(&step, &mut sink).await
192 + };
193 +
194 + let out = read(engine::git_toplevel_and_prefix_cmd(repo))
195 + .await
196 + .with_context(|| format!("reading `{repo}` on `{name}`"))?;
197 + let stderr = String::from_utf8_lossy(&out.stderr).trim().to_string();
198 + anyhow::ensure!(
199 + out.status.success(),
200 + "`{repo}` on `{name}` is not a git checkout: {stderr}"
201 + );
202 + let (toplevel, prefix) = engine::parse_toplevel_and_prefix(&String::from_utf8_lossy(
203 + &out.stdout,
204 + ))
205 + .ok_or_else(|| anyhow::anyhow!("could not read where `{repo}` is checked out on `{name}`"))?;
206 +
207 + let worktree = host
208 + .worktree_for(engine::repo_dir_name(&toplevel), app)
209 + .ok_or_else(|| {
210 + anyhow::anyhow!(
211 + "host `{name}` sets no worktree_root, so there is nowhere to build \
212 + tagged source"
213 + )
214 + })?;
215 +
216 + // Advisory, and deliberately run against the ordinary checkout: a worktree
217 + // shares its repository's object store, so a tag fetched here is a tag the
218 + // worktree can be put at. `fetch --all` exits non-zero if ANY remote fails
219 + // and the library repos carry three, so a dead mirror must not decide a
220 + // release. Only the checkout below is allowed to fail.
221 + let _ = read(engine::git_fetch_cmd(repo)).await;
222 +
223 + let exists = read(engine::git_worktree_probe_cmd(&worktree))
224 + .await
225 + .is_ok_and(|o| o.status.success());
226 + let pin = if exists {
227 + engine::git_worktree_pin_cmd(&worktree, tag)
228 + } else {
229 + // Prune first: a worktree directory somebody deleted by hand is still
230 + // registered in the repository, and `worktree add` then refuses the path
231 + // as already in use rather than rebuilding it.
232 + let _ = read(engine::git_worktree_prune_cmd(&toplevel)).await;
233 + engine::git_worktree_add_cmd(&toplevel, &worktree, tag)
234 + };
235 + let out = read(pin)
236 + .await
237 + .with_context(|| format!("pinning the worktree on `{name}`"))?;
238 + if !out.status.success() {
239 + // Ask git why before telling the operator. The usual answer is a tag
240 + // that was never created or never pushed, which is a different job from
241 + // anything about the worktree itself.
242 + let tag_exists = read(engine::git_tag_exists_cmd(repo, tag))
243 + .await
244 + .is_ok_and(|o| o.status.success());
245 + anyhow::bail!(
246 + "could not put `{worktree}` on `{name}` at {tag}: {}",
247 + engine::worktree_failure_reason(tag, tag_exists, &String::from_utf8_lossy(&out.stderr))
248 + );
249 + }
250 +
251 + let out = read(engine::git_rev_parse_cmd(&worktree))
252 + .await
253 + .with_context(|| format!("rev-parse on `{name}`"))?;
254 + anyhow::ensure!(
255 + out.status.success(),
256 + "`git rev-parse HEAD` failed in `{worktree}` on `{name}`"
257 + );
258 +
259 + Ok(PinnedHost {
260 + host: name.clone(),
261 + build_dir: engine::app_dir_in_worktree(&worktree, &prefix),
262 + sha: String::from_utf8_lossy(&out.stdout).trim().to_string(),
263 + })
264 + }
265 +
266 + /// What the preflight established: where each host builds this release, and the
267 + /// one commit every one of those trees is on.
305 268 struct Pinned {
306 - /// One entry per host the preflight moved, for [`restore_branches`].
307 - branches: Vec<PinnedHost>,
269 + /// `(host, directory the recipe builds in)`. Becomes the run's
270 + /// `repo_by_host`, so `repo()` in a recipe is the worktree and no recipe
271 + /// knows this happened.
272 + build_dirs: Vec<(String, String)>,
308 273 /// The commit all hosts agreed on. Empty when pinning is off (tests).
309 274 sha: String,
310 275 }
311 276
312 - /// One host's checkout as the preflight found it, and everything
313 - /// [`restore_branches`] needs to put it back.
314 - #[derive(Clone)]
277 + /// One host's prepared worktree, before the barrier has compared them.
315 278 struct PinnedHost {
316 279 host: String,
317 - /// The branch it was on before the tag was checked out.
318 - branch: String,
319 - /// Tracked files already carrying local changes once the tag was checked
320 - /// out, repo-root relative. `None` when the tree could not be read, which
321 - /// makes the restore discard nothing rather than guess.
322 - dirty_before: Option<Vec<String>>,
323 - }
324 -
325 - /// Which tracked files in `repo`'s whole repository have local changes, rendered
326 - /// for an operator and marked where they fall outside the app's own directory.
327 - ///
328 - /// Diagnosis, never a gate: both callers run it only after a `git checkout` has
329 - /// already failed, and both treat a probe that itself fails as "nothing to add"
330 - /// rather than as a second error. The reason the checkout failed is the message;
331 - /// this only says which file to go and look at.
332 - async fn repo_dirty_report(exec: &dyn Executor, repo: &str) -> Option<String> {
333 - async fn read(exec: &dyn Executor, cmd: String) -> String {
334 - let step = OpStep::shell(Action::Build, cmd);
335 - let mut sink = DiscardSink;
336 - match exec.run_streaming(&step, &mut sink).await {
337 - Ok(out) if out.status.success() => String::from_utf8_lossy(&out.stdout).into_owned(),
338 - _ => String::new(),
339 - }
340 - }
341 - let status = read(exec, engine::git_repo_dirty_cmd(repo)).await;
342 - let prefix = read(exec, engine::git_repo_prefix_cmd(repo)).await;
343 - engine::dirty_paths_blocking_checkout(&status, &prefix)
344 - }
345 -
346 - /// Put every host's checkout back the way [`pin_release`] found it: on its
347 - /// branch, and with the build's own leavings discarded.
348 - ///
349 - /// Best-effort and non-fatal: the release itself is already decided by the time
350 - /// this runs, and failing a green build because a `git checkout` did not take
351 - /// would be worse than the detached tree it is cleaning up. A failure is logged
352 - /// loudly instead, because the state it leaves behind is the silent one.
353 - ///
354 - /// The discard is the half added on 2026-08-24, and it is why a release used to
355 - /// need a person between it and the next one. `cargo build` rewrites
356 - /// `Cargo.lock` under the `[patch]` block in `~/Code/.cargo/config.toml` — churn
357 - /// CLAUDE.md documents as expected and meaningless — which leaves the tree dirty
Lines truncated
M bento/daemon/src/topology.rs +89 -3
@@ -226,18 +226,70 @@
226 226 /// true thing instead.
227 227 #[serde(default)]
228 228 pub pull_roots: Vec<PathBuf>,
229 + /// Absolute path ON THIS HOST holding the trees releases are built in:
230 + /// `<worktree_root>/<repo>/<app>`, one git worktree per app, detached at the
231 + /// release tag. `/home/max/Code/.bento`, `/Users/max/Code/.bento`,
232 + /// `C:/Users/me/Code/.bento`. Required of any host that declares targets.
233 + ///
234 + /// A release used to build in the ordinary checkout, which meant pinning a
235 + /// tree somebody else is working in to a tag for the length of the build —
236 + /// so editing in Helix during a release meant editing tag content. It also
237 + /// made every release end by putting that tree back, which is where the
238 + /// detached-HEAD and dirty-lockfile failures came from (task `8b5366e2`).
239 + /// Bento owning its own tree removes the shared state rather than tidying
240 + /// up after it.
241 + ///
242 + /// Two things decide the path, and both are load-bearing:
243 + ///
244 + /// - **Under `~/Code`**, because `~/Code/.cargo/config.toml`'s `[patch]`
245 + /// block redirects the in-house git dependencies to the working copies in
246 + /// the tree. Cargo finds that file by walking up from the build directory,
247 + /// so a tree outside `~/Code` silently resolves those dependencies from
248 + /// their remotes instead and builds something else.
249 + /// - **One directory**, not a sibling per repo: it is stated once here and
250 + /// never edited as repos come and go, the repos themselves stay untouched,
251 + /// and it is one path to exclude from tooling and one to delete to reclaim
252 + /// the space.
253 + ///
254 + /// It is an artifact root too (see [`Self::artifact_roots`]) rather than a
255 + /// second thing to declare beside `pull_roots`. The binary a release
256 + /// collects is built here, so a worktree root that was not pullable would
257 + /// fail every collect, and two settings that must agree are two settings
258 + /// that can drift.
259 + #[serde(default)]
260 + pub worktree_root: Option<PathBuf>,
229 261 }
230 262
231 263 impl Host {
232 - /// Every artifact root declared for this host, singular and plural merged.
233 - /// Empty means this host pulls nothing, which is the fail-closed default.
264 + /// Every artifact root declared for this host: `pull_root`, `pull_roots` and
265 + /// the worktree root, merged. Empty means this host pulls nothing, which is
266 + /// the fail-closed default.
234 267 pub fn artifact_roots(&self) -> Vec<PathBuf> {
235 268 self.pull_root
236 269 .iter()
237 270 .cloned()
238 271 .chain(self.pull_roots.iter().cloned())
272 + .chain(self.worktree_root.iter().cloned())
239 273 .collect()
240 274 }
275 +
276 + /// Where this app's release is built on this host:
277 + /// `<worktree_root>/<repo>/<app>`, `repo` being the checkout's directory
278 + /// name. `None` for a host with no worktree root, which cannot build.
279 + ///
280 + /// Joined with `/` rather than [`PathBuf::join`] because the result is a
281 + /// path on the REMOTE host, rendered into git commands there. The daemon is
282 + /// Linux and one build host is Windows, where `PathBuf` would render `\` and
283 + /// the remote shell would not thank us; git takes forward slashes on every
284 + /// platform.
285 + pub fn worktree_for(&self, repo_dir: &str, app: &AppId) -> Option<String> {
286 + let root = self.worktree_root.as_ref()?.display().to_string();
287 + Some(format!(
288 + "{}/{repo_dir}/{app}",
289 + root.trim_end_matches('/'),
290 + app = app.as_str()
291 + ))
292 + }
241 293 }
242 294
243 295 /// Every build host can, by definition, build and package. Keeping these the
@@ -493,9 +545,29 @@
493 545 /// disk exactly as [`Topology::load`] does. Tests write a real `bento.toml`
494 546 /// into a temp repo so they exercise the same path as production rather
495 547 /// than a parallel one.
548 + ///
549 + /// One difference from production, and it is about noise rather than
550 + /// coverage: a host that declares no `worktree_root` is given one here
551 + /// instead of being refused. Most tests never build from git — pinning is
552 + /// off, and their "repos" are plain directories — so making every fixture
553 + /// carry a path would say nothing except that the field exists. The tests
554 + /// that DO exercise the worktree set it themselves, to a temp directory, and
555 + /// `validate` still refuses a real config that omits it.
496 556 #[cfg(test)]
497 557 pub fn from_str_for_tests(s: &str) -> Result<Self> {
498 - Self::resolve(toml::from_str(s)?)
558 + let mut raw: RawTopology = toml::from_str(s)?;
559 + for h in &mut raw.hosts {
560 + if h.worktree_root.is_none() {
561 + // Per process: a test that pins without setting this is a bug,
562 + // and a shared path would make it show up as a stale directory
563 + // from an earlier run rather than as itself.
564 + h.worktree_root = Some(std::env::temp_dir().join(format!(
565 + "bento-tests-never-build-here-{}",
566 + std::process::id()
567 + )));
568 + }
569 + }
570 + Self::resolve(raw)
499 571 }
500 572
501 573 /// Every service reaches the box that runs it exactly one way.
@@ -590,6 +662,20 @@
590 662 h.name
591 663 );
592 664 }
665 + // A build host with no worktree root has nowhere to build: the
666 + // release would have to fall back to the ordinary checkout, which is
667 + // the shared mutable state the worktree exists to end. Refusing at
668 + // load makes that a startup error naming the host, rather than a
669 + // second code path nobody remembers is there.
670 + if !h.targets.is_empty() && h.worktree_root.is_none() {
671 + anyhow::bail!(
672 + "host `{}` declares buildable targets but sets no worktree_root; \
673 + releases build in `<worktree_root>/<repo>/<app>` and there is no \
674 + fallback. Set it to the `.bento` directory under that host's code \
675 + tree, e.g. worktree_root = \"/home/max/Code/.bento\"",
676 + h.name
677 + );
678 + }
593 679 if h.transport == HostTransport::Agent && h.agent_url.is_none() {
594 680 anyhow::bail!(
595 681 "host `{}` uses transport = \"agent\" but sets no agent_url",