fix: name the agent binary devpod picked from a hostname (#560 section 3)

CHANGELOG.md

@@ -49,15 +49,29 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
   attach that runs no `up` prints neither line, because it asked devpod for
   nothing.
 
-- **`docs/cli.md` now names the cause of `inject agent … exit status 126`.** devpod
-  picks its agent binary by globbing `uname -a` for `arm`, and `uname -a` carries
-  the container's hostname, so a workspace whose branch contains `alarm`, `warm`,
-  `charm`, `swarm`, `harm` or `armature` gets the arm64 agent on an x86 host and a
-  launch that dies saying only "not executable". `dl` writes the workspace id into
-  that hostname itself, so it is one of the ways the name gets there. The entry
-  carries the one-line check, the way to unblock a container that is already in
-  that state, and the reason a recreate undoes it. The match itself is devpod's and
-  is not fixed here.
+- **A failed `up` of a workspace whose id contains `arm` names the agent binary
+  devpod picked.** devpod chooses which agent to inject by globbing `uname -a` for
+  `arm`, and `uname -a` carries the container's hostname, so a workspace whose
+  branch contains `alarm`, `warm`, `charm`, `swarm`, `harm` or `armature` gets the
+  arm64 agent on an x86 host and a launch that dies saying only `exit status 126`,
+  which means "not executable" and names neither the architecture nor the word that
+  chose it. `dl` writes the workspace id into that hostname itself, so it is one of
+  the ways the name gets there.
+
+  **The line is a conditional, and deliberately.** `devpod up` runs with `dl`'s own
+  terminal, because an image build's progress belongs on your screen rather than
+  through a pipe, so `dl` never reads devpod's message: what it holds when the
+  build fails is an exit code and a name. That is enough to know the trap is set
+  and not enough to know it fired, so the sentence says what to look for in
+  devpod's own output above it instead of asserting what happened. An `up` that
+  failed on an image pull for a workspace called `alarm-clock` gets one sentence it
+  can ignore, and a host whose own architecture is ARM gets none, because there
+  devpod's guess is the right one.
+
+  `docs/cli.md` carries the whole diagnosis: the one-line check for any running
+  container, the `docker cp` that unblocks one already in that state, and why a
+  recreate undoes it. The match itself is devpod's, upstream, and is not fixed
+  here.
 
 ### Changed
 

docs/cli.md

@@ -677,6 +677,24 @@ branch contains `alarm`, `warm`, `charm`, `swarm`, `harm`, `farm` or `armature`
 a candidate, and `dl`'s own setup pass puts the workspace id into the hostname, so
 `dl` is one of the ways the name gets there.
 
+`dl` says this itself when an `up` refuses for a workspace whose id contains
+`arm`, and says it as a conditional:
+
+```
+If devpod said 'inject agent' and 'exit status 126' above, this is why: the workspace id
+devlaunch-feature-armature-17uu contains 'arm', devpod picks its agent binary by matching
+'uname -a' against '*arm*', ...
+```
+
+**It is a conditional because `dl` never reads devpod's message.** `devpod up`
+runs with `dl`'s own terminal, so an image build's progress goes straight to you
+rather than through `dl`, and what `dl` holds when the build fails is an exit code
+and a workspace id. That is enough to know the trap is set and not enough to know
+it fired, so the line tells you what to look for in devpod's output above it. An
+`up` that failed on an image pull for a workspace called `alarm-clock` gets one
+sentence it can ignore. Nothing is said on a host whose own architecture is ARM,
+where devpod's guess is the right one.
+
 ```bash
 # the mechanism, in any running container
 docker exec <container> sh -c 'case "$(uname -a)" in *arm*) echo ARM;; *) echo NOT;; esac'

docs/rust-rewrite-plan.md

@@ -198,6 +198,7 @@ Nothing diverges silently.
 | 32 | **`--rm` is docker's `--rm`, and the flag-spelled verbs are retired.** `dl <ws> --rm` and `dl <ws> --rm -- <cmd>` now hand over a session and delete the workspace once it ends — what row 30's `--autorm` did, under the name docker gives it. The word `rm` is unchanged and is the only way to delete one *now*, so `docker rm` / `docker run --rm` is the whole of the grammar and no spelling has to be read twice. Three withdrawals pay for it, each refused at exit 1 rather than reinterpreted: `--autorm` (`--autorm is now spelled --rm: …`); `--stop`, whose only reason to exist was being a flag (rows 15 and 30) and which cannot stay a *cancelling* suffix beside a `--rm` that runs the line; and row 30's suffix override itself, so `aid <ws> 'review this pr' --rm` now runs the review and deletes afterwards where it used to delete instead, and `dl prune <ws> --rm` is row 31's refusal rather than a removal (with `--force` also on the line it is the `--force`-beside-`--rm` refusal instead, named first because the pair is the more confused half). `--force` still does not compose with `--rm` — docker keeps `-f` on `rm` too — and `dl <ws> rm --rm` is refused as the two requests it is. Retired flags are answered *before* anything else the line got wrong, since both moved on account of `--rm`'s new meaning. aid peels the retired spellings so dl refuses them by name instead of joining them into a prompt, and builds no agent command for such a line, so nothing is booted on the way to exit 1. `Overridden`, `pick_target` and the `--rm overrode the rest of the line` notice are gone with the override. | Row 30 bought "and now delete it" as a suffix and paid for it with a flag whose meaning was unguessable from its spelling: `--rm` cancelled the line, `--autorm` ran it, and the two looked like a pair. docker had already split the same problem the other way — a verb for now, a `run` flag for after — and taking that split makes the common line (`aid <ws> <prompt> --rm`: send the agent in, get the disk back) the one the short spelling names, at the cost of the rarer one, which `dl rm` and a pick does in fewer keystrokes than recalling a long prompt to append to it. The withdrawals are recognised rather than deleted, for row 31's reason: a flag dropped from `Cli` is clap's `unexpected argument` at exit 2, naming the spelling and not the replacement — and the spelling is exactly what cannot explain a line that stopped working because a *different* flag changed meaning. Pinned by the `retired_flag` / `rm_` tests in `dl/src/cli.rs`, `the_retired_flag_spellings_name_the_words_that_replaced_them` and `autorm_is_refused_with_the_spelling_that_replaced_it` in `dl/tests/lifecycle.rs`, the `rm_on_exit_` tests in `dl/tests/launch.rs`, and `a_retired_spelling_starts_no_agent_and_is_handed_to_dl_to_refuse` in `aid/src/rewrite.rs`. |
 | 33 | **A warm launch of a triple says how far behind its checkout is.** `dl owner/repo@branch` against a workspace devpod already knows prints one line before the attach banner when the clone's `HEAD` is behind the `refs/remotes/origin/<branch>` that clone last fetched, naming the count both ways (`its checkout is 37 commits behind origin/main … (and 3 of its own it has not pushed)`). Python printed nothing, and neither did earlier Rust. The launch still runs no fetch: the report is one `rev-list` against a local repository, so it says how the checkout stands against a ref of whatever age and never claims to know the remote. Silent when the counts agree, when the checkout is only ahead, when there is no clone on disk, for a bare workspace name (no triple, so no branch to name and no clone path to derive), and for a warm resolution addressed by an id `metadata.json` recorded rather than the one the triple derives (devlaunch#88's arm, where the derived clone path is not this container's source). `reset`'s help line and the README table stopped saying "Clean slate: remove everything, recreate" in the same change, because that promise reads as one about the checkout and `reset` cannot keep it. | The warm arm makes no git call and never will (devlaunch#144, built by #149 and #150), and a launch that looks like it verified new work when it verified neither the commit nor the image invalidates whatever was concluded inside the container. A fetch on attach was the tempting fix and is the wrong trade; the fact was available locally the whole time. Pinned by the `checkout`/`warm_triple` tests in `flows::launch` and by `a_warm_triple_whose_checkout_is_behind_says_how_far_and_still_runs_no_git_fetch` plus `a_bare_workspace_name_reports_no_checkout_however_stale_it_is` in `dl/tests/launch.rs`, whose world is `launch_scenario.py`'s `--stale-checkout`. blooop/devlaunch#560 §1. |
 | 34 | **Every `devpod up` says whether it forwarded dotfiles.** One line per `up`: the repository and script it passed, or that devpod's context options name none and where to set one. Python read `devpod context options`, silently forwarded `--dotfiles`/`--dotfiles-script` or silently omitted them, and left no way from the terminal to tell which had happened. An attach that runs no `up` prints neither line. | Three plausible causes and no observation to cut between them is what turned "the dotfiles never landed" into a fortnight: `DOTFILES_URL` is read out of `devpod context options` and out of nothing else, not the process environment and not `~/.devpod/config.yaml`, and `context-options.json` is dl's *cache* of that answer rather than an input. The line costs nothing on a path that is already spawning a container build, and it makes a silent policy checkable. Pinned by `an_up_says_which_dotfiles_it_asked_devpod_for_and_the_argv_agrees` in `dl/tests/launch.rs`, which asserts the sentence and the argv together so the two cannot drift. blooop/devlaunch#560 §2. |
+| 35 | **A refused `devpod up` of a workspace whose id contains `arm` gains one line of dl's own.** The refusal still renders nothing else and still exits with devpod's status; this is a second line beside it, phrased as a conditional (`If devpod said 'inject agent' and 'exit status 126' above, this is why: …`) naming the workspace id, the `*arm*` glob, the hostname `uname -a` carries, and the `docker cp` that unblocks a container already in that state. Python printed nothing. Silent for every other refusal, for an id with no `arm` in it, and on a host whose own architecture reads as ARM, where devpod's guess is correct. | devpod's `is_arm` globs `uname -a`, which prints the nodename beside the machine, so a branch named `armature` or `alarm` gets the arm64 agent on an x86 host and a launch that dies saying only "not executable" -- and dl is one of the ways the name gets there, since its setup pass sets the container's hostname to the workspace id. **The conditional is not hedging, it is the honest limit**: `devpod up` runs as a passthrough so its progress reaches the terminal directly, dl therefore never reads devpod's message, and what it holds is an exit code and a name. Matching devpod's stderr instead would mean piping the build through dl -- changing what devpod renders and, since `Runner::session` starts its child with `OwnGroup::No`, dropping the process group that lets a Ctrl-C `killpg` a build instead of orphaning it holding the launch lock (row 27, #304). One conditional sentence on a failed launch is the cheaper trade. `clients::devpod::reads_as_arm` is the transcription of devpod's four globs and answers for a machine name as readily as for a hostname, which is what the ARM-host exemption reads. Pinned by the `reads_as_arm` tests in `clients/devpod.rs`, the `arm_agent_hint` tests in `dl/src/render.rs`, and `a_failed_up_of_an_arm_named_workspace_names_the_agent_binary_devpod_picked` plus `an_ordinary_workspace_whose_up_failed_gets_no_arm_explanation` in `dl/tests/launch.rs`, whose world is `launch_scenario.py`'s `--arm-branch`. blooop/devlaunch#560 §3, whose upstream half (glob `uname -m`) is not fixed here. |
 
 Additions require a PR that updates this table; the row number is cited by any
 per-binary harness branch.

rust/devlaunch-core/public-api.rest.txt

@@ -118,6 +118,7 @@ pub fn devlaunch_core::clients::devpod::Workspace::eq(&self, &devlaunch_core::cl
 impl core::fmt::Debug for devlaunch_core::clients::devpod::Workspace
 pub fn devlaunch_core::clients::devpod::Workspace::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result
 impl core::marker::StructuralPartialEq for devlaunch_core::clients::devpod::Workspace
+pub fn devlaunch_core::clients::devpod::reads_as_arm(&str) -> bool
 pub mod devlaunch_core::clients::devpod_home
 pub enum devlaunch_core::clients::devpod_home::RepointFailure
 pub devlaunch_core::clients::devpod_home::RepointFailure::NotADevpodRecord
@@ -1514,6 +1515,7 @@ pub devlaunch_core::flows::launch::LaunchRefusal::UnknownWorkspace::name: alloc:
 pub devlaunch_core::flows::launch::LaunchRefusal::UnsafeSpec(devlaunch_core::domain::workspace_id::UnsafeName)
 pub devlaunch_core::flows::launch::LaunchRefusal::UpRefused
 pub devlaunch_core::flows::launch::LaunchRefusal::UpRefused::exit: devlaunch_runner::Exit
+pub devlaunch_core::flows::launch::LaunchRefusal::UpRefused::workspace_id: alloc::string::String
 impl core::clone::Clone for devlaunch_core::flows::launch::LaunchRefusal
 pub fn devlaunch_core::flows::launch::LaunchRefusal::clone(&self) -> devlaunch_core::flows::launch::LaunchRefusal
 impl core::cmp::Eq for devlaunch_core::flows::launch::LaunchRefusal

rust/devlaunch-core/src/clients/devpod.rs

@@ -259,6 +259,53 @@ pub(crate) fn says_it_is_blocked(line: &str) -> bool {
     line.contains("Trying to lock workspace")
 }
 
+/// The globs devpod's injected shell script tests `uname -a` against to decide
+/// it is on an ARM machine.
+///
+/// Transcribed from `is_arm` in devpod's `pkg/inject/inject.sh`, whose four
+/// patterns are `*arm*`, `*arm64*`, `*aarch*` and `*aarch64*`. Two of those are
+/// subsumed by the other two -- anything holding `arm64` holds `arm` -- and they
+/// are kept anyway, because this is a mirror of somebody else's list and a
+/// mirror that has been tidied is one nobody can diff against the original.
+const DEVPOD_ARM_GLOBS: [&str; 4] = ["arm", "arm64", "aarch", "aarch64"];
+
+/// Whether devpod would read a container named this way as an ARM machine.
+///
+/// **This is a bug in devpod that devlaunch walks into by name** (devlaunch#560).
+/// `uname -a` prints the *nodename* along with the machine, so the test above is
+/// satisfied by a hostname as readily as by an architecture, and
+/// `createBinaryLoader` in `pkg/agent/inject.go` takes the boolean straight
+/// through to `arch = "arm64"`. The wrong agent downloads, `performVersionCheck`
+/// cannot execute it, and the launch dies with `exit status 126` -- "not
+/// executable", which is true and names neither the architecture nor the string
+/// that chose it.
+///
+/// devlaunch is one of the ways the string gets there: the setup pass runs
+/// `sudo hostname <workspace-id>` in every container it opens, and a workspace id
+/// is derived from the branch. So `feature/armature` becomes
+/// `devlaunch-feature-armature-<hash>` becomes a container that reads as ARM, and
+/// every branch holding `alarm`, `warm`, `charm`, `swarm`, `harm` or `farm` is the
+/// same trap.
+///
+/// Asked of the workspace id rather than of a `uname -a` this process cannot see,
+/// which is what makes it a *prediction* and the reason its one caller phrases the
+/// answer as a possibility. dl runs `devpod up` as a passthrough -- an image build
+/// belongs on the user's terminal -- so devpod's own sentence is never read, and a
+/// reader has to be told what to look for in it rather than told it happened.
+///
+/// The fix is upstream and one line: glob `uname -m`, which prints the machine
+/// alone. `blooop/devpod`'s `fix/detect-arch-with-uname-m` carries it.
+///
+/// **Case-sensitive, because a shell `case` is.** Nothing is folded here even
+/// though a workspace id arrives lowercased by [`slug`](crate::domain::workspace_id)
+/// and so could not carry an `ARM` anyway: this predicate answers "would devpod's
+/// script match", and a fold would make it answer something devpod does not ask.
+pub fn reads_as_arm(container_name: &str) -> bool {
+    DEVPOD_ARM_GLOBS
+        .iter()
+        .any(|glob| container_name.contains(glob))
+}
+
 /// An outcome, split into "it ran, this is what came back" and "it did not".
 ///
 /// One function over all four arms rather than a `_ =>` at each call site: the
@@ -2370,4 +2417,58 @@ mod tests {
             StatusUnreadable::NotRun(NotRun::NotInstalled)
         );
     }
+
+    // ------------------------------------------- devpod's own arch detection
+
+    #[test]
+    fn a_name_holding_arm_anywhere_reads_as_arm_because_the_glob_is_a_substring() {
+        // devlaunch#560 §3. Not a word match and not a prefix: devpod's script
+        // globs `*arm*`, so an ordinary English branch name is enough. Every name
+        // below is one somebody would write without a thought about architecture.
+        for name in [
+            "devlaunch-feature-armature-17uu",
+            "app-alarm-clock-3j1t",
+            "dl-warm-start-np10",
+            "x-charm-1",
+            "x-swarm-1",
+            "x-harm-1",
+            "x-farm-1",
+            "repo-aarch-notes-1",
+        ] {
+            assert!(reads_as_arm(name), "{name}");
+        }
+    }
+
+    #[test]
+    fn a_name_with_no_arm_in_it_reads_as_nothing() {
+        for name in [
+            "devlaunch-main-3j1t",
+            "devlaunch-feature-auth-np10",
+            "",
+            // The letters, out of order and separated: a substring is a substring.
+            "mar-ram-amr",
+        ] {
+            assert!(!reads_as_arm(name), "{name}");
+        }
+    }
+
+    #[test]
+    fn the_glob_is_not_folded_because_a_shell_case_is_not() {
+        // A faithful mirror of somebody else's `case` matters more here than being
+        // generous: devpod would not match this, so neither may the prediction. A
+        // workspace id cannot carry it anyway -- `slug` lowercases -- which is
+        // exactly why a fold would be an untestable kindness.
+        assert!(!reads_as_arm("devlaunch-ARM-3j1t"));
+    }
+
+    #[test]
+    fn the_same_predicate_answers_for_a_machine_as_for_a_hostname() {
+        // `uname -a` prints both, which is the whole bug, so one glob covers both
+        // questions and the caller that asks "is this host itself ARM" needs no
+        // second list.
+        assert!(reads_as_arm("aarch64"));
+        assert!(reads_as_arm("arm64"));
+        assert!(reads_as_arm("armv7l"));
+        assert!(!reads_as_arm("x86_64"));
+    }
 }