From c4b1f5b9eb9f8a584133e08726996283b0c6ec27 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 18:58:12 +0000 Subject: [PATCH 1/8] fix: a launch killed mid setup pass was never provisioned again `devpod up` finishes, devpod writes its create result, and what runs next is dl's own `devpod ssh --command`: the hostname, the terminal title, the onboarding memo, `gh` and `claude`. A Ctrl-C there leaves the container running with none of it, and nothing about that container says so. `devpod status` answers `Running` and devpod's create record is complete, which is exactly what the fast-attach arm asks, so every later `dl ` attaches in one round trip and provisions nothing. The workspace is permanently half built and the only recovery is knowing to type `dl up`. A pass now writes down that it is running before its trip, in `tool-verdicts/.pass`, and removes it when the pass answers. dl's signal handler `_exit`s without unwinding, so a record left standing is the one thing on the host that says a pass was cut short. It is anchored to the same `workspace_result.json` mtime the verdict marker is, so one left by a container since rebuilt describes nothing standing. Positive evidence, never the absence of it: a workspace brought up by VS Code, a hand-typed `devpod up`, or a build older than the record has no record either, and those go on attaching in one round trip. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC --- CHANGELOG.md | 28 ++ docs/workspace-tools.md | 25 ++ rust/devlaunch-core/public-api.api.txt | 6 + rust/devlaunch-core/src/flows/launch.rs | 166 +++++++++ rust/devlaunch-core/src/flows/provision.rs | 166 ++++++++- .../src/flows/provision/verdict_cache.rs | 170 ++++++++++ rust/dl/src/render.rs | 5 + rust/dl/tests/resumed_setup.rs | 316 ++++++++++++++++++ 8 files changed, 880 insertions(+), 2 deletions(-) create mode 100644 rust/dl/tests/resumed_setup.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d598d76..5f8010b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,34 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Fixed + +- **A launch cut short between `devpod up` and the end of dl's setup pass left a + workspace that never got provisioned, and no later launch ever noticed.** The + `up` finishes, devpod writes its create result, and what runs next is dl's own + `devpod ssh --command`: the hostname, the terminal title, the onboarding memo, + `gh` and `claude`. Ctrl-C there and the container is left running with none of + it. Nothing about that container says so. `devpod status` answers `Running` and + devpod's create record is complete, which is exactly what the fast attach asks, + so `dl ` attaches in one round trip and provisions nothing, and does + the same on every launch after that. The workspace is permanently half built and + the recovery is knowing to type `dl up`, which nothing says either. + That is the "sometimes it works" of it: whether you get a whole workspace + depends on which second the Ctrl-C landed in. + + A pass now writes down that it is running, before its trip, in + `${XDG_CACHE_HOME:-~/.cache}/devlaunch/tool-verdicts/.pass`, and + removes it when the pass answers. dl's signal handler `_exit`s without unwinding, + so a record left standing is the one thing on the host that says a pass was cut + short, and a launch that finds one runs the pass before it hands over a shell. + The record carries the same `workspace_result.json` mtime the verdict marker + does, so one left by a container that has since been rebuilt describes nothing + standing and is ignored. + + It is positive evidence, never the absence of it. A workspace brought up by VS + Code, by a hand-typed `devpod up`, or by a build older than the record has no + record either, and those go on attaching in one round trip as before. + ### Changed - **`LaunchVerb::Attach` carries a `RemoteCommand` rather than an diff --git a/docs/workspace-tools.md b/docs/workspace-tools.md index 3f0b4122..4ade2979 100644 --- a/docs/workspace-tools.md +++ b/docs/workspace-tools.md @@ -491,6 +491,31 @@ Nothing has to be cleaned up, and there is nothing to invalidate by hand: the markers are compared, never trusted on age, and deleting the whole directory costs one round trip on each workspace's next launch. +### A pass that was interrupted + +The trip above takes seconds, and a Ctrl-C can land in the middle of it. What that +leaves is the awkward case: `devpod up` has already finished, so devpod wrote its +create result and the container is running, and everything the Ctrl-C stopped was +dl's. The container looks finished to everything that can be asked about it, so a +later launch takes the fast path and provisions nothing, forever. + +So a pass writes down that it is running before it makes its trip, in +`.pass` beside the marker, and removes it when the pass answers. +`dl`'s signal handler exits without unwinding, which is why the evidence has to be +a file left standing rather than one written on the way out: nothing runs on the +way out. A launch that finds one still standing for the container standing now runs +the pass before it hands over a shell, and says so. + +The record is anchored to the same `workspace_result.json` mtime the marker is, so +one left by a container that has since been rebuilt describes nothing that exists +and is ignored. It is read as positive evidence and never as the absence of it: a +workspace brought up by VS Code, by a hand-typed `devpod up`, or by a build older +than the record has no record either, and those keep attaching in one round trip. + +A pass that ran and *failed* closes the record like any other. A refused trip is a +pass that happened and said so, and re-running it on every attach afterwards would +be a container re-attempting one failing thing forever. + ### What to bake so a launch does no work at all To make every `dl` launch of an image stop at trip 1. The probe asks a **login** diff --git a/rust/devlaunch-core/public-api.api.txt b/rust/devlaunch-core/public-api.api.txt index 97bc07c7..98ddfe4d 100644 --- a/rust/devlaunch-core/public-api.api.txt +++ b/rust/devlaunch-core/public-api.api.txt @@ -397,6 +397,7 @@ pub struct devlaunch_core::api::ToolProvisioning<'e> impl<'e> devlaunch_core::flows::launch::ToolProvisioning<'e> pub fn devlaunch_core::flows::launch::ToolProvisioning<'e>::from_env(&std::path::Path, &'e mut dyn devlaunch_core::notices::Notices) -> Self impl devlaunch_core::flows::launch::Provision for devlaunch_core::flows::launch::ToolProvisioning<'_> +pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::pass_never_finished(&self, &str) -> bool pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::provision_tools(&self, &dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, core::option::Option<&str>) -> core::result::Result, devlaunch_core::flows::provision::DevpodMissing> pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::remembered_claude(&self, &str) -> core::option::Option pub struct devlaunch_core::api::WorkspaceCloneManager<'r> @@ -418,9 +419,11 @@ pub trait devlaunch_core::api::Notices pub fn devlaunch_core::api::Notices::say(&mut self, T) impl devlaunch_core::notices::Notices for alloc::vec::Vec pub trait devlaunch_core::api::Provision +pub fn devlaunch_core::api::Provision::pass_never_finished(&self, &str) -> bool pub fn devlaunch_core::api::Provision::provision_tools(&self, &dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, core::option::Option<&str>) -> core::result::Result, devlaunch_core::flows::provision::DevpodMissing> pub fn devlaunch_core::api::Provision::remembered_claude(&self, &str) -> core::option::Option impl devlaunch_core::flows::launch::Provision for devlaunch_core::flows::launch::ToolProvisioning<'_> +pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::pass_never_finished(&self, &str) -> bool pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::provision_tools(&self, &dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, core::option::Option<&str>) -> core::result::Result, devlaunch_core::flows::provision::DevpodMissing> pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::remembered_claude(&self, &str) -> core::option::Option pub fn devlaunch_core::api::enriched_listing(&mut devlaunch_core::flows::listing::CommandContext<'_>, &devlaunch_core::flows::listing::DlView<'_>, devlaunch_core::flows::listing::Sizes) -> core::result::Result, devlaunch_core::clients::devpod::ListingUnreadable> @@ -641,6 +644,7 @@ pub struct devlaunch_core::flows::launch::ToolProvisioning<'e> impl<'e> devlaunch_core::flows::launch::ToolProvisioning<'e> pub fn devlaunch_core::flows::launch::ToolProvisioning<'e>::from_env(&std::path::Path, &'e mut dyn devlaunch_core::notices::Notices) -> Self impl devlaunch_core::flows::launch::Provision for devlaunch_core::flows::launch::ToolProvisioning<'_> +pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::pass_never_finished(&self, &str) -> bool pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::provision_tools(&self, &dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, core::option::Option<&str>) -> core::result::Result, devlaunch_core::flows::provision::DevpodMissing> pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::remembered_claude(&self, &str) -> core::option::Option pub trait devlaunch_core::flows::launch::ColdMachinery<'r> @@ -650,9 +654,11 @@ impl<'r> devlaunch_core::flows::launch::ColdMachinery<'r> for devlaunch_core::fl pub fn devlaunch_core::flows::launch::ColdPath<'r, '_>::open(&mut self) -> core::result::Result, devlaunch_core::flows::launch::ColdRefused> pub fn devlaunch_core::flows::launch::ColdPath<'r, '_>::recorded(&mut self) -> core::option::Option<&devlaunch_core::domain::metadata::MetadataStorage> pub trait devlaunch_core::flows::launch::Provision +pub fn devlaunch_core::flows::launch::Provision::pass_never_finished(&self, &str) -> bool pub fn devlaunch_core::flows::launch::Provision::provision_tools(&self, &dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, core::option::Option<&str>) -> core::result::Result, devlaunch_core::flows::provision::DevpodMissing> pub fn devlaunch_core::flows::launch::Provision::remembered_claude(&self, &str) -> core::option::Option impl devlaunch_core::flows::launch::Provision for devlaunch_core::flows::launch::ToolProvisioning<'_> +pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::pass_never_finished(&self, &str) -> bool pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::provision_tools(&self, &dyn devlaunch_runner::Runner, &str, devlaunch_core::flows::provision::PassOccasion, core::option::Option<&str>) -> core::result::Result, devlaunch_core::flows::provision::DevpodMissing> pub fn devlaunch_core::flows::launch::ToolProvisioning<'_>::remembered_claude(&self, &str) -> core::option::Option pub enum devlaunch_core::flows::lifecycle::DeleteStalled diff --git a/rust/devlaunch-core/src/flows/launch.rs b/rust/devlaunch-core/src/flows/launch.rs index ae8aeb1f..a97669ce 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -559,6 +559,10 @@ pub enum LaunchNotice { /// devpod holds a record for this workspace and no create result, so an `up` /// started and never finished. Its container may well be running. CreateNeverFinished { workspace_id: String }, + /// dl's own setup pass over this container began and never finished -- the + /// launch carrying it was killed. The container is sound; the pass is being run + /// again before this launch hands over a session. + SetupPassNeverFinished { workspace_id: String }, /// `dl up` found it already running: nothing to build and nothing to /// wait for. AlreadyRunning { workspace_id: String }, @@ -1321,6 +1325,23 @@ pub trait Provision { fn remembered_claude(&self, _workspace_id: &str) -> Option { None } + + /// Whether a setup pass over the container standing now began and never + /// finished, from the host's own records and without a round trip. + /// + /// Asked on the fast-attach arm, which is the one path that opens a session + /// having provisioned nothing. A `dl` killed between `devpod up` and the end of + /// its pass leaves a container devpod calls Running *and* finished creating, so + /// [`CreateRecord`] cannot tell it from a workspace that is ready -- and every + /// later launch of it takes that same fast path and provisions nothing again. + /// + /// **Positive evidence, never the absence of it.** `false` by default, and + /// `false` for every doubt in the real implementation: a workspace brought up by + /// VS Code, by a hand-typed `devpod up`, or by a build older than the record has + /// no record either, and those must go on attaching in one round trip. + fn pass_never_finished(&self, _workspace_id: &str) -> bool { + false + } } /// A launch that lends nothing — `DEVLAUNCH_NO_TOOLS`, and every test that is not @@ -1434,6 +1455,10 @@ impl Provision for ToolProvisioning<'_> { fn remembered_claude(&self, workspace_id: &str) -> Option { self.verdicts.remembered_claude(workspace_id) } + + fn pass_never_finished(&self, workspace_id: &str) -> bool { + self.verdicts.pass_never_finished(workspace_id) + } } // =========================================================================== @@ -4211,6 +4236,11 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { spec: raw_spec.to_owned(), }); } + // Before the session, because the session is what the missing + // tools are missing *from*: a shell handed over first would be + // the one that goes looking for a `claude` the interrupted pass + // never installed. + self.finish_the_interrupted_pass(placement)?; let session = self.attach(placement, verb.command()); self.forced_refresh(); return session; @@ -4411,6 +4441,39 @@ impl<'a, 'r, 'l> Launch<'a, 'r, 'l> { Ok(refused) } + /// Run the setup pass again when the last one over this container was cut + /// short, and do nothing at all otherwise. + /// + /// The fast-attach arm's whole worth is that it opens a session in one round + /// trip, so this asks the host's records and not the container: a workspace with + /// no interrupted pass behind it -- which is every workspace but the one this is + /// for -- pays a file read and nothing else. + /// + /// [`PassOccasion::TopUp`] because the container never stopped, which is that + /// arm's definition. It does not mean fewer stages run: the occasion decides + /// only whether a remembered verdict may excuse the trip, and there is no + /// verdict here -- the pass that would have written one is the pass that was + /// interrupted. + fn finish_the_interrupted_pass(&mut self, placement: &Placement) -> Result<(), LaunchAborted> { + if !self.provision.pass_never_finished(placement.workspace_id()) { + return Ok(()); + } + self.notices.say(LaunchNotice::SetupPassNeverFinished { + workspace_id: placement.workspace_id().to_owned(), + }); + let seen = self + .provision + .provision_tools( + self.context.runner(), + placement.workspace_id(), + PassOccasion::TopUp, + self.container_title(placement.title()).as_deref(), + ) + .map_err(|DevpodMissing| LaunchAborted::DevpodNotRun(NotRun::NotInstalled))?; + self.claude_seen.set(seen); + Ok(()) + } + fn attach( &mut self, placement: &Placement, @@ -4918,6 +4981,9 @@ mod tests { /// What the host's records say about a workspace no pass ran for, which is /// what `dl`'s real implementation reads out of its verdict cache. claude_remembered: Option, + /// Whether the host's records say the last pass over this container was cut + /// short. Read from the same cache the field above is. + pass_never_finished: bool, } impl RecordingProvision { @@ -4972,6 +5038,10 @@ mod tests { fn remembered_claude(&self, _workspace_id: &str) -> Option { self.claude_remembered } + + fn pass_never_finished(&self, _workspace_id: &str) -> bool { + self.pass_never_finished + } } fn no_notices() -> Vec { @@ -9726,6 +9796,102 @@ mod tests { ); } + /// The launch that is cut short between `devpod up` and the end of dl's own + /// setup pass, which is the moment this whole record exists for. + /// + /// A Ctrl-C there leaves a container devpod calls `Running` *and* finished + /// creating -- devpod wrote its result on the way out of the `up`, and + /// everything after it was dl's. So [`CreateRecord`] answers `Completed`, the + /// fast-attach arm fires, and the session opens in a container with no `gh`, no + /// `claude`, no hostname and no zellij. Nothing about the container says + /// otherwise, so every later `dl ` does the same thing: the workspace is + /// permanently half-built, and the only recovery is knowing to type + /// `dl up`. + /// + /// The container itself is sound, so this does not re-run the `up` the way an + /// unfinished *create* does. It runs the pass that did not finish, and then + /// attaches. + #[test] + fn a_running_workspace_whose_setup_pass_never_finished_is_provisioned_before_attaching() { + let scene = Scene::new() + .with_running("myws") + .with_create_completed("myws"); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + parts.provision.pass_never_finished = true; + let mut cold = NeverCold; + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + + let launched = launch.run("myws", &LaunchVerb::Attach { command: None }, None); + + drop(launch); + assert_eq!( + launched, + Ok(Launched::Session(Session::RemoteExit { status: 0 })), + "the session still opens: the container is sound, only dl's work was not" + ); + assert_eq!( + parts.provision.occasions(), + [PassOccasion::TopUp], + "the container never stopped, so the pass is a top-up and not an after-up" + ); + assert!( + !scene + .devpod_commands() + .iter() + .any(|argv| argv.first().map(String::as_str) == Some("up")), + "a sound container is not rebuilt to finish dl's own pass: {:?}", + scene.devpod_commands() + ); + assert!( + parts.said.contains(&LaunchNotice::SetupPassNeverFinished { + workspace_id: "myws".to_owned() + }), + "the extra round trip is explained: {:?}", + parts.said + ); + } + + /// The other side of it, and the one that keeps the fast path fast: a workspace + /// with no interrupted pass behind it still attaches in one round trip. + /// + /// Without this, "did the last pass finish" could be answered by provisioning on + /// every warm attach -- which would pass the test above and put a `devpod ssh` + /// on the hottest path dl has, for every workspace dl never provisioned. + #[test] + fn a_warm_attach_with_no_interrupted_pass_behind_it_still_runs_none() { + let scene = Scene::new() + .with_running("myws") + .with_create_completed("myws"); + let updater = SelfInvocation::new("dl"); + let completion = scene.cache_dir().join("completion.json"); + let mut parts = launching(&scene.runner, &updater, &completion); + let mut cold = NeverCold; + let mut launch = Launch::new( + &mut parts.context, + &mut parts.refresh, + &mut cold, + &parts.provision, + &scene.host, + &mut parts.chatter, + &mut parts.said, + ); + + let _ = launch.run("myws", &LaunchVerb::Attach { command: None }, None); + + drop(launch); + assert_eq!(parts.provision.occasions(), [], "no pass at all"); + } + /// The other side of the same check: a create devpod *did* finish still takes /// the fast path. Without this, "is it set up" could be answered by refusing /// every fast attach, which would pass the test above and cost every warm diff --git a/rust/devlaunch-core/src/flows/provision.rs b/rust/devlaunch-core/src/flows/provision.rs index a2e83e60..934516b0 100644 --- a/rust/devlaunch-core/src/flows/provision.rs +++ b/rust/devlaunch-core/src/flows/provision.rs @@ -2354,6 +2354,12 @@ fn provision( // launch after upgrading, which is the trade `switches` made too. && verdicts.has_claude_memo(workspace) { + // Including a record some killed launch left open, which is the one route + // out of here that reaches neither place a pass is normally closed. Closing + // it is the right answer and not only the terminating one: the marker being + // trusted means a pass probed *this* container provisioned, so whatever the + // interrupted pass had left to do, there is nothing left to do now. + verdicts.end_pass(workspace); // No probe ran here, and the session that follows still has to decide // whether to forward the host's login, so the answer comes from what the // last pass remembered. @@ -2365,6 +2371,15 @@ fn provision( // Before the pass, not after it: see [`VerdictCache::observe`]. let observed = verdicts.and_then(|verdicts| verdicts.observe(workspace)); + // And the record that this pass is running, opened here for the same "before" + // and a second reason of its own. The trip below is seconds long, `dl`'s signal + // handler `_exit`s without unwinding, and a container whose pass was cut short + // is one devpod calls Running and finished creating -- so every later launch + // fast-attaches to it and the pass never runs again. Nothing about the container + // says otherwise, which is why the host writes it down first. + if let (Some(verdicts), Some(observed)) = (verdicts, observed) { + verdicts.begin_pass(workspace, observed); + } let host_home = host.and_then(|layout| layout.home.to_str()); let found = match setup_pass(runner, workspace, switches, title, host_home, events) { @@ -2543,6 +2558,14 @@ fn remember( if let (Some(verdicts), Some(observed)) = (verdicts, observed) { verdicts.remember_claude(workspace, claude, observed); } + // Both routes out of the pass come through here, which is why the record opened + // before the trip is closed here rather than at each `return`. It closes for a + // pass that *failed* as much as one that worked: a trip devpod refused is a pass + // that happened and said so, and leaving it open would put that same refused + // trip on every attach from then on. + if let Some(verdicts) = verdicts { + verdicts.end_pass(workspace); + } } /// Say what became of one stage, unless what became of it was nothing. @@ -3049,21 +3072,39 @@ fi /// registry, so a test driving it without the guard writes into whatever document /// a concurrent measured test installed. In the fixture rather than per test, so /// no test has to remember. - #[derive(Debug)] struct Trips { answers: Vec, stdout: String, seen: Mutex>, + /// See [`Trips::watching`]. `None` for the tests that only count trips. + #[allow(clippy::type_complexity)] + during: Option bool + Send + Sync>>, + saw: Mutex>, /// See [`timing::exclusive`]. Last field, so it is dropped last. _serialized: timing::Exclusive, } + impl std::fmt::Debug for Trips { + /// Hand-written because the watcher is a closure. What a failure message + /// wants of this value is what it recorded, which is the rest of it. + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("Trips") + .field("answers", &self.answers) + .field("stdout", &self.stdout) + .field("seen", &self.seen) + .field("saw", &self.saw) + .finish_non_exhaustive() + } + } + impl Trips { fn new(exits: &[i32]) -> Self { Self { answers: exits.iter().copied().map(Answer::Exited).collect(), stdout: String::new(), seen: Mutex::new(Vec::new()), + during: None, + saw: Mutex::new(Vec::new()), _serialized: timing::exclusive(), } } @@ -3073,6 +3114,8 @@ fi answers: answers.to_vec(), stdout: String::new(), seen: Mutex::new(Vec::new()), + during: None, + saw: Mutex::new(Vec::new()), _serialized: timing::exclusive(), } } @@ -3083,6 +3126,24 @@ fi self } + /// Run `during` at the moment each trip is in flight, keeping what it + /// answered. + /// + /// The one thing a fake runner can see that a recording of the argv cannot: + /// what was true on the host *while* devpod was running. A `dl` killed + /// mid-pass is killed exactly here, and nothing about it is observable after + /// the call returns. + #[must_use] + fn watching(mut self, during: impl Fn() -> bool + Send + Sync + 'static) -> Self { + self.during = Some(Box::new(during)); + self + } + + /// What [`Self::watching`] saw, one answer per trip, in order. + fn watched(&self) -> Vec { + self.saw.lock().expect("the recording").clone() + } + fn trips(&self) -> Vec { self.seen.lock().expect("the recording").clone() } @@ -3121,7 +3182,12 @@ fi StdinPlan::Inherit | StdinPlan::Null => None, }, }); - self.answers[(seen.len() - 1).min(self.answers.len() - 1)] + let answer = self.answers[(seen.len() - 1).min(self.answers.len() - 1)]; + drop(seen); + if let Some(during) = &self.during { + self.saw.lock().expect("the recording").push(during()); + } + answer } fn outcome(&self, answer: Answer) -> Outcome { @@ -6525,6 +6591,102 @@ fi ) } + #[test] + fn the_pass_is_open_while_the_trip_is_and_closed_once_it_answers() { + // The record a killed `dl` leaves. `dl`'s signal handler `_exit`s without + // unwinding, so the only moment at which the host can be told "a pass is + // running" is before the trip, and the only proof it was told is what is + // standing *during* the trip -- which is the seconds-long window a Ctrl-C + // actually lands in. + let remembering = Remembering::new(); + let verdicts = remembering.verdicts(); + let watching = remembering.verdicts(); + let runner = Trips::new(&[0]) + .reporting(REPORT_PROVISIONED) + .watching(move || watching.pass_never_finished("myws")); + + pass_of(&runner, PassOccasion::AfterUp, &verdicts).expect("devpod answered"); + + assert_eq!( + runner.watched(), + [true], + "the trip ran with no record of the pass carrying it" + ); + assert!( + !verdicts.pass_never_finished("myws"), + "the pass answered, so there is nothing left to finish" + ); + } + + #[test] + fn a_pass_devpod_refused_is_a_pass_that_happened() { + // A trip the OS would not make is not an interrupted pass: it ran, it + // failed, and it said so. Leaving the record open would put that same + // refused trip on every attach from then on, which is the "re-attempts + // forever" shape `verdict_cache` refuses for `ShimKept`. + let remembering = Remembering::new(); + let verdicts = remembering.verdicts(); + let runner = Trips::answering(&[Answer::Blocked]); + + pass_of(&runner, PassOccasion::AfterUp, &verdicts).expect("devpod is installed"); + + assert!(!verdicts.pass_never_finished("myws")); + } + + #[test] + fn a_top_up_that_makes_no_trip_opens_no_pass() { + // The cached arm returns before the trip, so there is no pass for a Ctrl-C + // to interrupt -- and a record opened for it would never be closed. + let remembering = Remembering::new(); + let verdicts = remembering.verdicts(); + let first = Trips::new(&[0]).reporting(REPORT_PROVISIONED); + pass_of(&first, PassOccasion::TopUp, &verdicts).expect("devpod answered"); + + let again = Trips::new(&[0]).reporting(REPORT_PROVISIONED); + assert_eq!( + pass_of(&again, PassOccasion::TopUp, &verdicts), + Ok(Provisioning::CachedProvisioned) + ); + + assert!(!verdicts.pass_never_finished("myws")); + } + + #[test] + fn a_cached_answer_closes_a_pass_that_was_left_open() { + // The arm that returns before the trip has to close the record too, or the + // one launch that could act on it would be the one that cannot: a top-up + // whose cache fully answers makes no trip, so it would reach neither of the + // two places the record is closed, and the launch after it would ask again, + // and so would every launch after that. + // + // Closing it is also the right answer rather than only the terminating one. + // The marker being trusted means a pass probed this same container + // provisioned, so whatever the interrupted pass had left to do, there is + // nothing left to do now. + let remembering = Remembering::new(); + let verdicts = remembering.verdicts(); + let first = Trips::new(&[0]).reporting(REPORT_PROVISIONED); + pass_of(&first, PassOccasion::TopUp, &verdicts).expect("devpod answered"); + // A pass opened over the same container and never closed, which is the state + // a killed `dl` leaves. + let observed = verdicts.observe("myws").expect("an anchor"); + verdicts.begin_pass("myws", observed); + assert!(verdicts.pass_never_finished("myws")); + + let again = Trips::new(&[0]).reporting(REPORT_PROVISIONED); + assert_eq!( + pass_of(&again, PassOccasion::TopUp, &verdicts), + Ok(Provisioning::CachedProvisioned) + ); + + assert_eq!(again.count(), 0, "still no round trip"); + assert!( + !verdicts.pass_never_finished("myws"), + "the cached answer left the record open, so every launch after this \ + one asks again and none of them can ever answer" + ); + } + #[test] fn a_top_up_with_a_trusted_verdict_makes_no_trip() { // The saving, stated as the only thing that can be measured about it: zero diff --git a/rust/devlaunch-core/src/flows/provision/verdict_cache.rs b/rust/devlaunch-core/src/flows/provision/verdict_cache.rs index f45eedc1..694cd2eb 100644 --- a/rust/devlaunch-core/src/flows/provision/verdict_cache.rs +++ b/rust/devlaunch-core/src/flows/provision/verdict_cache.rs @@ -258,6 +258,82 @@ impl VerdictCache { }; write_atomically(&self.marker(workspace_id), &text); } + + /// Where the in-flight record for this workspace lives. + fn in_flight(&self, workspace_id: &str) -> PathBuf { + self.marker(workspace_id).with_extension("pass") + } + + /// Note that a setup pass over the container [`Self::observe`] identified is + /// about to run, and has not finished. + /// + /// Written *before* the trip and removed by [`Self::end_pass`] after it, so the + /// file surviving a launch is the one thing on this host that says the pass was + /// cut short. `dl`'s signal handler `_exit`s without unwinding + /// (`devlaunch_runner::interrupt`), which is exactly why the evidence has to be + /// a file left standing rather than one written on the way out: nothing runs on + /// the way out. + /// + /// Anchored like everything else here, and against the same failure: a record + /// left by an interrupted pass over a container that has since been rebuilt + /// describes a container that is not standing, and reading it would send every + /// later launch on a round trip it does not owe. + /// + /// Silent about every way of not working, as [`Self::record`] is. A record that + /// could not be written costs one interrupted launch its recovery and nothing + /// else -- which is today's behaviour, exactly. + pub(crate) fn begin_pass(&self, workspace_id: &str, observed: Observed) { + let Observed(result_mtime) = observed; + let Ok(text) = serde_json::to_string(&InFlight { result_mtime }) else { + return; + }; + write_atomically(&self.in_flight(workspace_id), &text); + } + + /// The pass [`Self::begin_pass`] opened has finished. + /// + /// Called for a pass that *ran*, however it turned out -- a trip devpod refused + /// and an install that failed are both passes that happened, and re-running them + /// on every attach afterwards is the "re-attempts forever" behaviour the module + /// note refuses for [`Provisioning::ShimKept`](super::Provisioning::ShimKept). + /// What this exists to catch is the pass that never got an answer at all, + /// because the process carrying it was killed. + pub(crate) fn end_pass(&self, workspace_id: &str) { + let _ = std::fs::remove_file(self.in_flight(workspace_id)); + } + + /// Whether a setup pass over the container standing now began and never + /// finished. + /// + /// **Positive evidence, and it has to be.** The opposite reading -- "no record + /// of a finished pass, so run one" -- is true of every workspace `dl` never + /// provisioned: one created by VS Code, by a hand-typed `devpod up`, or by a + /// build older than this file. Those must keep attaching in one round trip, so + /// the question asked here is the narrow one, and every doubt answers *no*: no + /// record, a record that will not parse, or one whose container is not the one + /// standing. + pub(crate) fn pass_never_finished(&self, workspace_id: &str) -> bool { + let Some(text) = std::fs::read_to_string(self.in_flight(workspace_id)).ok() else { + return false; + }; + let Ok(in_flight) = serde_json::from_str::(&text) else { + return false; + }; + let Some(result) = sole_workspace_result(self.devpod_home.as_ref(), workspace_id) else { + return false; + }; + Stamp::of(&result) == Some(in_flight.result_mtime) + } +} + +/// What one in-flight record says. +/// +/// One field, and a struct rather than the bare [`Stamp`] it wraps, for +/// [`Marker`]'s reason: the check is the parse, so a file a later build writes with +/// a second field it needs is not silently read as this one. +#[derive(Clone, Copy, Debug, PartialEq, Eq, Deserialize, Serialize)] +struct InFlight { + result_mtime: Stamp, } /// The container a pass is about, as [`VerdictCache::observe`] read it. @@ -477,6 +553,13 @@ mod tests { (cache, home, verdicts) } + /// Open a pass over the container standing now, as [`provision`] does before + /// its trip -- and, unlike [`provision`], never close it. + fn began(verdicts: &VerdictCache, workspace_id: &str) { + let observed = verdicts.observe(workspace_id).expect("an anchor"); + verdicts.begin_pass(workspace_id, observed); + } + /// Remember `seen` against the container standing now, as [`provision`] does. fn remembered_under(verdicts: &VerdictCache, workspace_id: &str, seen: Option) { let observed = verdicts.observe(workspace_id).expect("an anchor"); @@ -873,6 +956,93 @@ mod tests { assert!(!verdicts.trusted("other", Switches::INSTALLING)); } + #[test] + fn a_pass_that_began_and_never_ended_is_still_unfinished() { + // The whole of what this file adds. A `dl` killed between `devpod up` and + // the end of its setup pass leaves a container devpod calls Running and + // finished creating, so every later launch fast-attaches to it and the pass + // never runs again. Nothing else on the host can tell that apart from a + // container whose pass ran to completion, because the cache records only + // success and an absent record is what a workspace dl never touched has too. + let (_cache, _home, verdicts) = anchored(); + + began(&verdicts, "ws"); + + assert!(verdicts.pass_never_finished("ws")); + } + + #[test] + fn an_interrupted_pass_over_a_container_since_rebuilt_asks_for_nothing() { + // The anchor earns its keep here as much as anywhere. A `--recreate` after + // the interruption is a whole new container with a pass of its own, and a + // record left by the old one would put a round trip on every attach from + // then on -- for a pass that already ran, over a container that is gone. + let (_cache, home, verdicts) = anchored(); + began(&verdicts, "ws"); + assert!(verdicts.pass_never_finished("ws")); + + rewritten(&result_in(&home, "ws"), Duration::from_secs(30)); + + assert!(!verdicts.pass_never_finished("ws")); + } + + #[test] + fn nothing_unreadable_asks_for_a_pass_either() { + // Every doubt reads as "no interrupted pass", which is the direction that + // costs nothing: a workspace dl never provisioned has no record at all, and + // it must go on attaching in one round trip. + let (cache, _home, verdicts) = anchored(); + assert!(!verdicts.pass_never_finished("ws"), "no record"); + + let path = cache.path().join(MARKERS_DIR).join("ws.pass"); + std::fs::create_dir_all(path.parent().expect("a parent")).expect("the directory"); + for text in ["", "{", "{}", "null", r#"{"result_mtime": "yesterday"}"#] { + std::fs::write(&path, text).expect("the record"); + assert!(!verdicts.pass_never_finished("ws"), "{text:?}"); + } + } + + #[test] + fn a_host_that_cannot_find_the_container_asks_for_nothing() { + // No `workspace_result.json` to compare against is a doubt like any other. + // It is also what a scratch `XDG_CACHE_HOME` and a machine with no devpod + // home both look like, and neither is a launch that owes a round trip. + let unfinished = devpod_home_with(&[("default", "ws", None)]); + let cache = cache(); + for home in [Some(DevpodHome::at(unfinished.path())), None] { + let verdicts = VerdictCache::under(cache.path(), home.clone()); + std::fs::create_dir_all(cache.path().join(MARKERS_DIR)).expect("the directory"); + std::fs::write( + cache.path().join(MARKERS_DIR).join("ws.pass"), + r#"{"result_mtime":{"secs":1,"nanos":0}}"#, + ) + .expect("the record"); + assert!(!verdicts.pass_never_finished("ws"), "{home:?}"); + } + } + + #[test] + fn one_workspaces_interrupted_pass_says_nothing_about_another() { + let home = devpod_home_with(&[("default", "ws", Some(())), ("default", "other", Some(()))]); + let cache = cache(); + let verdicts = VerdictCache::under(cache.path(), Some(DevpodHome::at(home.path()))); + + began(&verdicts, "ws"); + + assert!(verdicts.pass_never_finished("ws")); + assert!(!verdicts.pass_never_finished("other")); + } + + #[test] + fn a_pass_that_ended_left_nothing_to_finish() { + let (_cache, _home, verdicts) = anchored(); + + began(&verdicts, "ws"); + verdicts.end_pass("ws"); + + assert!(!verdicts.pass_never_finished("ws")); + } + #[test] fn a_stamp_is_the_mtime_it_was_read_from() { // The round trip the equality check depends on: what `Stamp::of` reads back diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index a28e6a91..23994d74 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -2809,6 +2809,11 @@ pub(crate) fn launch_notice(notice: &LaunchNotice) -> Option { for its create, so attaching to it would land as root in a container whose setup \ did not run. Bringing it up instead." ), + LaunchNotice::SetupPassNeverFinished { workspace_id } => format!( + "Workspace {workspace_id}: the last launch's setup pass did not finish, so its \ + tools, hostname and shell config may be missing. Running it again before \ + attaching." + ), LaunchNotice::AlreadyRunning { workspace_id } => { format!("Workspace {workspace_id} is already running.") } diff --git a/rust/dl/tests/resumed_setup.rs b/rust/dl/tests/resumed_setup.rs new file mode 100644 index 00000000..04c6551e --- /dev/null +++ b/rust/dl/tests/resumed_setup.rs @@ -0,0 +1,316 @@ +//! A launch cut short *after* `devpod up` and *before* dl's setup pass finished, +//! and what the next launch does about it. +//! +//! The sibling of `dl/tests/interrupt.rs`, and the moment just after the one that +//! file judges. There the `devpod up` is still running, so the next launch runs it +//! again and everything lands. Here the `up` has already finished: devpod has +//! written its `workspace_result.json`, the container is up, and what the Ctrl-C +//! interrupts is the `devpod ssh --command` that carries dl's own stages -- the +//! hostname, the terminal title, the onboarding memo, `gh` and `claude`. +//! +//! Nothing about that container says it is unfinished. `devpod status` answers +//! `Running` and devpod's create record is complete, which is what the fast-attach +//! arm asks, so every later `dl ` attaches straight into a container whose +//! setup never ran -- forever, and identically, so the workspace is not broken in a +//! way anyone can see. It is the "sometimes it works" the report is about: whether +//! you get a whole workspace depends on which second the Ctrl-C landed in. +//! +//! So a run that opens a pass writes down that it did, and a run that finds that +//! record still standing over the container standing now finishes the pass before +//! it hands over a shell. +//! +//! Linux-only, like the sibling suite, and it spells devpod's own paths on purpose: +//! `devlaunch-core/tests/devpod_layout.rs` scopes its one-spelling rule to that +//! crate's `src` precisely so an end-to-end test can check that what dl wrote +//! landed where devpod will look for it, without routing the check through the code +//! under test. + +use std::path::{Path, PathBuf}; +use std::process::Command; +use std::time::{Duration, Instant}; + +use devlaunch_test_support::KeepingCoverage; + +fn repo_root() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("..") + .join("..") + .canonicalize() + .expect("the repository root") +} + +/// The cold world `dl/tests/launch.rs` builds, with a `devpod` that keeps devpod's +/// *records* as well as its state. +/// +/// The scenario's fake devpod answers `status`, `list` and `up` out of one JSON +/// file and writes nothing under `DEVPOD_HOME`. Real devpod writes +/// `workspace.json` on the way in and `workspace_result.json` on the way out of a +/// completed `up`, and both halves of this launch read them: the fast-attach arm +/// asks whether the create finished, and the host's memory of a pass is anchored to +/// the result file's mtime so that a rebuilt container is never described by an +/// older container's record. A fake that skipped them would make every question +/// here unanswerable and the test vacuous. +struct World { + root: PathBuf, + _scratch: tempfile::TempDir, +} + +impl World { + fn cold() -> Self { + let scratch = tempfile::Builder::new() + .prefix("dlresume") + .tempdir_in("/tmp") + .expect("a scratch directory under /tmp"); + let root = scratch.path().to_path_buf(); + let built = Command::new("python3") + .arg(Path::new(env!("CARGO_MANIFEST_DIR")).join("tests/launch_scenario.py")) + .arg(&root) + .arg(repo_root().join("test/fixtures/devpod_shim.py")) + .output() + .expect("python3 is installed"); + assert!( + built.status.success(), + "launch_scenario.py failed: {}", + String::from_utf8_lossy(&built.stderr) + ); + + let devpod = root.join("bin/devpod"); + let original = std::fs::read_to_string(&devpod).expect("the scenario's devpod"); + let delegate = original + .lines() + .find(|line| line.starts_with("exec ")) + .expect("the delegate exec line"); + // Without the `exec`, so the `up` arm gets to keep going afterwards. + let run = delegate.strip_prefix("exec ").expect("an exec line"); + // `$DL_BLOCK_PASS` is what makes one run the interrupted one: the setup pass + // is the `ssh --command` trip, and blocking it holds `dl` at exactly the + // moment a Ctrl-C during provisioning arrives. The session `ssh` carries no + // `--command`, so it is never the one that blocks. + let script = format!( + "#!/bin/sh\n\ + if [ \"$1\" = \"up\" ]; then\n\ + \x20 {run} \"$@\" || exit $?\n\ + \x20 ws=$2; prev=\n\ + \x20 for a in \"$@\"; do\n\ + \x20 if [ \"$prev\" = \"--id\" ]; then ws=$a; fi\n\ + \x20 prev=$a\n\ + \x20 done\n\ + \x20 d=\"$DEVPOD_HOME/contexts/default/workspaces/$ws\"\n\ + \x20 mkdir -p \"$d\"\n\ + \x20 printf '{{}}' > \"$d/workspace.json\"\n\ + \x20 printf '{{}}' > \"$d/workspace_result.json\"\n\ + \x20 exit 0\n\ + fi\n\ + if [ \"$1\" = \"ssh\" ] && [ -n \"$DL_BLOCK_PASS\" ]; then\n\ + \x20 case \" $* \" in\n\ + \x20 *\" --command \"*)\n\ + \x20 echo \"$$\" > \"$DL_PASS_PID\"\n\ + \x20 : > \"$DL_PASS_STARTED\"\n\ + \x20 exec sleep 300 ;;\n\ + \x20 esac\n\ + fi\n\ + {delegate}\n" + ); + std::fs::write(&devpod, script).expect("rewrite devpod"); + use std::os::unix::fs::PermissionsExt as _; + std::fs::set_permissions(&devpod, std::fs::Permissions::from_mode(0o755)) + .expect("keep devpod executable"); + + World { + root, + _scratch: scratch, + } + } + + fn path(&self, relative: &str) -> PathBuf { + self.root.join(relative) + } + + /// A `dl blooop/devlaunch@cold` over this world. + /// + /// `DEVLAUNCH_NO_TOOLS` so the pass carries its stages and stops there: the + /// stages are not tools work, so the trip this test counts still happens, and + /// the lend and the network install that would follow it do not. What is being + /// counted is whether a pass ran at all. + fn dl(&self) -> Command { + let root = self.root.display().to_string(); + let mut command = Command::new(env!("CARGO_BIN_EXE_dl")); + command + .arg("blooop/devlaunch@cold") + .env_clear() + .keeping_coverage() + .env("PATH", format!("{root}/bin:/usr/bin:/bin")) + .env("HOME", format!("{root}/home")) + .env("XDG_CACHE_HOME", format!("{root}/cache")) + .env("XDG_CONFIG_HOME", format!("{root}/config")) + .env("DEVPOD_HOME", format!("{root}/devpod")) + .env("DEVPOD_SHIM_STATE", format!("{root}/shim-state.json")) + .env("DEVPOD_SHIM_LOG", format!("{root}/shim-log.jsonl")) + .env("DEVPOD_SHIM_CONFIG", format!("{root}/shim-config.json")) + .env("DEVLAUNCH_NO_GH_TOKEN", "1") + .env("DEVLAUNCH_NO_TOOLS", "1") + .env( + "DL_PASS_STARTED", + self.path("pass.started").display().to_string(), + ) + .env("DL_PASS_PID", self.path("pass.pid").display().to_string()) + .env("GIT_SSH_COMMAND", "false") + .env("GIT_CONFIG_GLOBAL", "/dev/null") + .env("GIT_CONFIG_SYSTEM", "/dev/null"); + command + } + + /// Run `dl` to completion and answer how many setup passes it made. + fn launched(&self) -> Passes { + self.truncate_log(); + let done = self.dl().output().expect("the dl binary runs"); + assert!( + done.status.success(), + "dl failed ({:?}): {}", + done.status.code(), + String::from_utf8_lossy(&done.stderr) + ); + Passes { + setup_trips: self.setup_trips(), + said: String::from_utf8_lossy(&done.stderr).into_owned(), + } + } + + /// Run `dl` up to the setup pass and SIGTERM it there. + /// + /// SIGTERM rather than SIGINT for one reason that is the harness's and not the + /// behaviour's: a job this test backgrounds inherits an ignored SIGINT under + /// POSIX job control, which `dl` honours on purpose (`dl/tests/interrupt.rs` + /// pins that asymmetry). Both signals reach the same drain and the same + /// `_exit`, which is the part that matters here: nothing unwinds, so nothing + /// tidies up after the pass that was running. + fn interrupted_mid_pass(&self) { + let started = self.path("pass.started"); + let mut child = self + .dl() + .env("DL_BLOCK_PASS", "1") + .stdout(std::process::Stdio::null()) + .stderr(std::process::Stdio::null()) + .spawn() + .expect("the dl binary runs"); + assert!( + wait_for(|| started.exists()), + "the setup pass never started, so the interrupt would prove nothing" + ); + assert!( + Command::new("kill") + .args(["-TERM", &child.id().to_string()]) + .status() + .expect("kill is installed") + .success(), + "sending SIGTERM to dl" + ); + let status = child.wait().expect("dl exits"); + assert_eq!( + status.code(), + Some(143), + "dl's own interrupted ending, 128 + SIGTERM" + ); + self.reap_the_blocked_trip(); + } + + /// Kill the trip `dl` left blocking, because `dl` does not. + /// + /// Not the behaviour under test, and not a defect this test is asserting the + /// absence of. The pass's `devpod ssh --command` stays in `dl`'s own process + /// group on purpose -- `devlaunch_runner`'s `passthrough` explains why only + /// `devpod up` leads a group of its own -- so a terminal Ctrl-C reaches it from + /// the kernel and fells it. What does not reach it is the `kill
` this + /// harness sends, which is aimed at one pid. Left alone the fake trip would + /// outlive the suite, and a test that leaves processes behind is one that makes + /// the next one flaky. + fn reap_the_blocked_trip(&self) { + let Ok(pid) = std::fs::read_to_string(self.path("pass.pid")) else { + return; + }; + let _ = Command::new("kill").args(["-KILL", pid.trim()]).status(); + } + + fn truncate_log(&self) { + std::fs::write(self.path("shim-log.jsonl"), "").expect("an empty log"); + } + + /// How many `devpod ssh --command` trips the log holds -- the setup pass, and + /// the only devpod call a launch makes that is dl provisioning the container. + fn setup_trips(&self) -> usize { + std::fs::read_to_string(self.path("shim-log.jsonl")) + .unwrap_or_default() + .lines() + .filter(|line| line.contains("\"ssh\"") && line.contains("\"--command\"")) + .count() + } + + /// The in-flight records dl's cache holds, by file name. + fn open_passes(&self) -> Vec { + let Ok(entries) = std::fs::read_dir(self.path("cache/devlaunch/tool-verdicts")) else { + return Vec::new(); + }; + let mut found: Vec = entries + .flatten() + .map(|entry| entry.file_name().to_string_lossy().into_owned()) + .filter(|name| name.ends_with(".pass")) + .collect(); + found.sort(); + found + } +} + +/// What one completed `dl` did. +#[derive(Debug)] +struct Passes { + setup_trips: usize, + said: String, +} + +fn wait_for(mut ready: impl FnMut() -> bool) -> bool { + let deadline = Instant::now() + Duration::from_secs(30); + while Instant::now() < deadline { + if ready() { + return true; + } + std::thread::sleep(Duration::from_millis(25)); + } + false +} + +#[test] +fn a_launch_killed_mid_pass_is_finished_by_the_next_one_and_only_the_next_one() { + let world = World::cold(); + + // One launch, killed while its setup pass is on the wire. The container it + // leaves is running and devpod calls its create complete, which is the whole + // difficulty: nothing about it is distinguishable from a workspace that is ready. + world.interrupted_mid_pass(); + assert_eq!( + world.open_passes().len(), + 1, + "the killed run left no record that its pass was running: {:?}", + world.open_passes() + ); + + // The next launch. Before this, it attached in one round trip and provisioned + // nothing -- the fast-attach arm asks devpod's create record, and devpod's + // create really did finish. + let second = world.launched(); + assert_eq!( + second.setup_trips, 1, + "the interrupted pass was not finished: {second:?}" + ); + assert!( + second.said.contains("setup pass did not finish"), + "the extra round trip went unexplained: {}", + second.said + ); + + // And the launch after that pays nothing, which is the half that keeps the fast + // path fast: the record is closed by the pass that finished, not left standing + // for every attach from then on. + let third = world.launched(); + assert_eq!(third.setup_trips, 0, "the fast attach stopped being fast"); + assert_eq!(world.open_passes(), Vec::::new()); +} From 28f46b04a2aed75160d82025019820eeb009b359 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 19:06:31 +0000 Subject: [PATCH 2/8] fix: the pass closed its record when the probe answered, not when it ended Found by all three review axes independently, and measured: with a probe that answers absent, the in-flight record stands during trip one and is gone during trip two. `remember` ran the close, and `remember` sits immediately after `setup_pass`. But a pass is up to three trips, and the two after the probe are the long ones: the lend streams the host's binaries, and the install fetches a `claude` of a few hundred megabytes. The probe is ~1.7s. So the record was open for the window nobody interrupts and shut for the window everybody does, and a Ctrl-C during a cold install left exactly the container the change exists to stop existing: running, create complete, no marker, no record, fast-attached forever with no tools. The close moves out of `remember` and out to `provision`, decided by what the pass answered. `Provisioning::reached_the_container` is that decision, exhaustive so a later arm has to be classified by somebody looking at it. Second finding, from the Types axis: `TripRefused` now leaves the record standing. It is the OS declining to make the trip, so nothing was learned about the container and nothing was done to it, which is the state the record describes. `a_pass_devpod_refused_is_a_pass_that_happened` asserted the opposite and is replaced: it conflated a trip that ran and failed with one that never happened, and erasing the record on the second is reachable from the recovery pass itself -- one refused trip and nothing could ever recover the workspace. The prose the same defect falsified goes with it: `begin_pass`, `end_pass`, and the two paragraphs in docs/workspace-tools.md that described a pass of one trip. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC --- docs/workspace-tools.md | 26 ++-- rust/devlaunch-core/src/flows/provision.rs | 138 +++++++++++++++--- .../src/flows/provision/verdict_cache.rs | 19 ++- 3 files changed, 144 insertions(+), 39 deletions(-) diff --git a/docs/workspace-tools.md b/docs/workspace-tools.md index 4ade2979..9868d9a9 100644 --- a/docs/workspace-tools.md +++ b/docs/workspace-tools.md @@ -493,14 +493,16 @@ one round trip on each workspace's next launch. ### A pass that was interrupted -The trip above takes seconds, and a Ctrl-C can land in the middle of it. What that -leaves is the awkward case: `devpod up` has already finished, so devpod wrote its -create result and the container is running, and everything the Ctrl-C stopped was -dl's. The container looks finished to everything that can be asked about it, so a -later launch takes the fast path and provisions nothing, forever. - -So a pass writes down that it is running before it makes its trip, in -`.pass` beside the marker, and removes it when the pass answers. +A pass is up to three trips and the slow ones are the last two: the lend streams +the host's binaries, and the install fetches a `claude` of a few hundred megabytes. +A Ctrl-C lands in the middle of that often. What it leaves is the awkward case: +`devpod up` has already finished, so devpod wrote its create result and the +container is running, and everything the Ctrl-C stopped was dl's. The container +looks finished to everything that can be asked about it, so a later launch takes +the fast path and provisions nothing, forever. + +So a pass writes down that it is running before its first trip, in +`.pass` beside the marker, and removes it after its last. `dl`'s signal handler exits without unwinding, which is why the evidence has to be a file left standing rather than one written on the way out: nothing runs on the way out. A launch that finds one still standing for the container standing now runs @@ -512,9 +514,11 @@ and is ignored. It is read as positive evidence and never as the absence of it: workspace brought up by VS Code, by a hand-typed `devpod up`, or by a build older than the record has no record either, and those keep attaching in one round trip. -A pass that ran and *failed* closes the record like any other. A refused trip is a -pass that happened and said so, and re-running it on every attach afterwards would -be a container re-attempting one failing thing forever. +A pass whose install ran and *failed* closes the record like any other: it happened +and it said so, and re-running it on every attach afterwards would be a container +re-attempting one failing thing forever. The one outcome that leaves the record +standing is a trip the OS would not make at all, because nothing was learned about +the container and nothing was done to it, which is the state the record is for. ### What to bake so a launch does no work at all diff --git a/rust/devlaunch-core/src/flows/provision.rs b/rust/devlaunch-core/src/flows/provision.rs index 934516b0..bcf9f190 100644 --- a/rust/devlaunch-core/src/flows/provision.rs +++ b/rust/devlaunch-core/src/flows/provision.rs @@ -2234,6 +2234,31 @@ pub enum Provisioning { } impl Provisioning { + /// Whether a trip got through to the container, whatever it then found. + /// + /// What decides whether the in-flight record [`provision`] opens is closed + /// again. Every arm but one is a pass that reached the container and did + /// something there, or established that there was nothing to do; only + /// [`Self::TripRefused`] is the OS declining to make the trip, which leaves the + /// container exactly as it was and this host knowing nothing new about it. + /// + /// Exhaustive rather than `matches!` on the one arm, for + /// [`Self::tools_present`]'s reason: a later arm has to be classified by + /// somebody who is looking at it, and an `_` would classify it as *reached*, + /// which is the direction that loses a workspace. + pub(crate) fn reached_the_container(&self) -> bool { + match self { + Self::AlreadyProvisioned + | Self::CachedProvisioned + | Self::Lent + | Self::ShimKept + | Self::Installed + | Self::InstallRefused { .. } + | Self::Disabled => true, + Self::TripRefused { .. } => false, + } + } + /// Whether the tools are now there — the bool Python returns. Only this /// module's tests read it; the binary matches the arms directly. #[cfg_attr(not(test), allow(dead_code))] @@ -2372,14 +2397,57 @@ fn provision( // Before the pass, not after it: see [`VerdictCache::observe`]. let observed = verdicts.and_then(|verdicts| verdicts.observe(workspace)); // And the record that this pass is running, opened here for the same "before" - // and a second reason of its own. The trip below is seconds long, `dl`'s signal - // handler `_exit`s without unwinding, and a container whose pass was cut short - // is one devpod calls Running and finished creating -- so every later launch - // fast-attaches to it and the pass never runs again. Nothing about the container - // says otherwise, which is why the host writes it down first. + // and a second reason of its own. `dl`'s signal handler `_exit`s without + // unwinding, and a container whose pass was cut short is one devpod calls + // Running and finished creating -- so every later launch fast-attaches to it + // and the pass never runs again. Nothing about the container says otherwise, + // which is why the host writes it down first. if let (Some(verdicts), Some(observed)) = (verdicts, observed) { verdicts.begin_pass(workspace, observed); } + + let outcome = run_the_pass( + runner, workspace, switches, title, host, verdicts, observed, events, + ); + + // Closed out here, from what the pass answered, and that placement is the whole + // of what makes the record worth keeping. It used to close beside the memo, the + // moment the probe trip returned -- which covers ~1.7s of a pass that runs for + // minutes, since the lend streams the host's binaries and the install fetches a + // ~300MB `claude`. A Ctrl-C in the part that was left uncovered is the ordinary + // one, so the record was open for the window nobody interrupts and shut for the + // window everybody does. + // + // A trip that never got through is the one outcome that leaves it standing. + // Nothing was learned about the container and nothing was done to it, which is + // exactly the state the record describes; closing there would erase the only + // evidence a later launch could act on, and it is reachable from the recovery + // pass itself. + if let (Some(verdicts), Ok(pass)) = (verdicts, &outcome) + && pass.provisioning.reached_the_container() + { + verdicts.end_pass(workspace); + } + outcome +} + +/// The pass: the probe trip, then the lend, then the install. +/// +/// Split from [`provision`] only so the in-flight record can be opened before all +/// three and closed after all three. Folding it back means closing the record at +/// each of the six places this returns, and the two that must *not* close it are +/// the two easiest to miss. +#[allow(clippy::too_many_arguments)] +fn run_the_pass( + runner: &dyn Runner, + workspace: &str, + switches: Switches, + title: Option<&str>, + host: Option<&HostLayout>, + verdicts: Option<&VerdictCache>, + observed: Option, + events: &mut dyn Notices, +) -> Result { let host_home = host.and_then(|layout| layout.home.to_str()); let found = match setup_pass(runner, workspace, switches, title, host_home, events) { @@ -2558,14 +2626,6 @@ fn remember( if let (Some(verdicts), Some(observed)) = (verdicts, observed) { verdicts.remember_claude(workspace, claude, observed); } - // Both routes out of the pass come through here, which is why the record opened - // before the trip is closed here rather than at each `return`. It closes for a - // pass that *failed* as much as one that worked: a trip devpod refused is a pass - // that happened and said so, and leaving it open would put that same refused - // trip on every attach from then on. - if let Some(verdicts) = verdicts { - verdicts.end_pass(workspace); - } } /// Say what became of one stage, unless what became of it was nothing. @@ -6619,18 +6679,56 @@ fi } #[test] - fn a_pass_devpod_refused_is_a_pass_that_happened() { - // A trip the OS would not make is not an interrupted pass: it ran, it - // failed, and it said so. Leaving the record open would put that same - // refused trip on every attach from then on, which is the "re-attempts - // forever" shape `verdict_cache` refuses for `ShimKept`. + fn the_pass_is_open_for_the_install_trip_and_not_only_the_probe() { + // The trip the record is *for*. A probe that answers absent is followed by + // the install, which fetches `gh` and `claude`: minutes against the probe's + // ~1.7s, and so the window nearly every Ctrl-C lands in. Closing the record + // when the probe answered left that window exactly as it was before any of + // this existed -- the container comes up, the install is killed, and every + // launch afterwards fast-attaches into a container with no tools. + let remembering = Remembering::new(); + let verdicts = remembering.verdicts(); + let watching = remembering.verdicts(); + let runner = Trips::new(&[0]) + .reporting(REPORT_ABSENT) + .watching(move || watching.pass_never_finished("myws")); + + let outcome = pass_of(&runner, PassOccasion::AfterUp, &verdicts); + + assert_eq!(outcome, Ok(Provisioning::Installed)); + assert_eq!( + runner.watched(), + [true, true], + "the install ran with no record of the pass carrying it" + ); + assert!( + !verdicts.pass_never_finished("myws"), + "and the record closes once the whole pass has answered" + ); + } + + #[test] + fn a_trip_that_never_got_through_leaves_the_pass_open() { + // Replaces a test that asserted the opposite, and the opposite was wrong. + // `TripRefused` is the OS declining to make the trip at all, so nothing was + // learned about the container and nothing was done to it -- which is the + // one state the record exists to describe. Closing it there erases the only + // evidence a later launch could act on, and it is reachable from the + // recovery pass itself: one refused trip and the workspace can never be + // recovered by anything. + // + // Not the "re-attempts forever" shape `verdict_cache` refuses for + // `ShimKept`. That is a trip that *ran* and would go on failing; this is a + // trip that did not happen, on a host that is broken in a way the pass + // reports every time. let remembering = Remembering::new(); let verdicts = remembering.verdicts(); let runner = Trips::answering(&[Answer::Blocked]); - pass_of(&runner, PassOccasion::AfterUp, &verdicts).expect("devpod is installed"); + let outcome = pass_of(&runner, PassOccasion::AfterUp, &verdicts); - assert!(!verdicts.pass_never_finished("myws")); + assert!(matches!(outcome, Ok(Provisioning::TripRefused { .. }))); + assert!(verdicts.pass_never_finished("myws")); } #[test] diff --git a/rust/devlaunch-core/src/flows/provision/verdict_cache.rs b/rust/devlaunch-core/src/flows/provision/verdict_cache.rs index 694cd2eb..dcb63b4c 100644 --- a/rust/devlaunch-core/src/flows/provision/verdict_cache.rs +++ b/rust/devlaunch-core/src/flows/provision/verdict_cache.rs @@ -267,9 +267,11 @@ impl VerdictCache { /// Note that a setup pass over the container [`Self::observe`] identified is /// about to run, and has not finished. /// - /// Written *before* the trip and removed by [`Self::end_pass`] after it, so the - /// file surviving a launch is the one thing on this host that says the pass was - /// cut short. `dl`'s signal handler `_exit`s without unwinding + /// Written before the pass's *first* trip and removed by [`Self::end_pass`] + /// after its last, so the file surviving a launch is the one thing on this host + /// that says the pass was cut short. A pass is up to three trips and the probe + /// is the short one, so a record that closed when the probe answered would be + /// open for ~1.7s of a pass that runs for minutes. `dl`'s signal handler `_exit`s without unwinding /// (`devlaunch_runner::interrupt`), which is exactly why the evidence has to be /// a file left standing rather than one written on the way out: nothing runs on /// the way out. @@ -292,12 +294,13 @@ impl VerdictCache { /// The pass [`Self::begin_pass`] opened has finished. /// - /// Called for a pass that *ran*, however it turned out -- a trip devpod refused - /// and an install that failed are both passes that happened, and re-running them - /// on every attach afterwards is the "re-attempts forever" behaviour the module + /// Called for a pass that got through to the container, however it turned out: + /// an install that ran and failed is a pass that happened, and re-running it on + /// every attach afterwards is the "re-attempts forever" behaviour the module /// note refuses for [`Provisioning::ShimKept`](super::Provisioning::ShimKept). - /// What this exists to catch is the pass that never got an answer at all, - /// because the process carrying it was killed. + /// A trip the OS would not make is the other thing and is not called for here -- + /// see [`Provisioning::reached_the_container`](super::Provisioning::reached_the_container), + /// which is where the two are told apart. pub(crate) fn end_pass(&self, workspace_id: &str) { let _ = std::fs::remove_file(self.in_flight(workspace_id)); } From e33723d729b1b112f6d6f08d01bbfba11f407049 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 19:07:54 +0000 Subject: [PATCH 3/8] test: the page named a file with nothing holding it to the code CLAUDE.md's standing rule: a second hand-maintained copy of a fact is allowed only if a test named beside it diffs it against the first. `.pass` was spelled by hand in docs/workspace-tools.md against `in_flight`'s `with_extension("pass")`, and the sibling claims in that same file each have a contract module. This one had nothing. It is the kind of claim a reader acts on rather than reads: the page tells somebody a killed launch leaves that file beside the marker, so they go looking for it to see whether a workspace is stuck, or to clear one by hand. A page naming a file that is not there is worse than a page naming none. Two assertions, both read out of `VerdictCache` rather than out of a constant, so a rename moves them with it. Renaming the extension to `inflight` fails the first with the new name in the message. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC --- rust/devlaunch-core/src/flows/provision.rs | 6 ++ .../provision/interrupted_pass_contract.rs | 81 +++++++++++++++++++ .../src/flows/provision/verdict_cache.rs | 4 +- 3 files changed, 89 insertions(+), 2 deletions(-) create mode 100644 rust/devlaunch-core/src/flows/provision/interrupted_pass_contract.rs diff --git a/rust/devlaunch-core/src/flows/provision.rs b/rust/devlaunch-core/src/flows/provision.rs index bcf9f190..8de31ea9 100644 --- a/rust/devlaunch-core/src/flows/provision.rs +++ b/rust/devlaunch-core/src/flows/provision.rs @@ -2739,6 +2739,12 @@ mod zellij_contract; #[cfg(test)] mod herdr_tab_contract; +/// The file name `docs/workspace-tools.md` publishes for a pass that was killed, +/// held against the path the verdict cache writes. Beside the other three for +/// `zellij_contract`'s reason, and reading the same section splitter. +#[cfg(test)] +mod interrupted_pass_contract; + #[cfg(test)] mod tests { //! # What this pins, and how diff --git a/rust/devlaunch-core/src/flows/provision/interrupted_pass_contract.rs b/rust/devlaunch-core/src/flows/provision/interrupted_pass_contract.rs new file mode 100644 index 00000000..c12ea7bd --- /dev/null +++ b/rust/devlaunch-core/src/flows/provision/interrupted_pass_contract.rs @@ -0,0 +1,81 @@ +//! The file name `docs/workspace-tools.md` publishes for an interrupted pass, held +//! against the path the cache actually writes. +//! +//! Beside `lending_contract` and `zellij_contract` for their reason, and reading the +//! same section splitter. What it guards is narrower than either: the page tells a +//! reader that a killed launch leaves `.pass` beside the marker, and a +//! reader who believes that goes looking for the file -- to see whether a workspace +//! is stuck, or to clear one by hand. A page naming a file that is not there is +//! worse than a page that names none. +//! +//! Two facts, because the sentence makes two claims: what the file is called, and +//! that it sits beside the marker rather than somewhere of its own. Both are read +//! out of [`VerdictCache`] rather than out of a constant, so a rename that moves the +//! path moves the assertion with it. +//! +//! Everything else in the section is explanation and gets no assertions, for the +//! reason `lending_contract` gives about the trip-by-trip narrative. + +use super::lending_contract::{CONTRACT_DOC, contract_doc, section}; +use super::verdict_cache::VerdictCache; + +/// The section carrying what this file guards, matched on its heading for +/// `lending_contract`'s reason. +const HEADING: &str = "### A pass that was interrupted"; + +/// The two paths this workspace's records live at, as the cache spells them. +fn recorded(workspace_id: &str) -> (String, String) { + let dir = tempfile::tempdir().expect("a scratch cache directory"); + let verdicts = VerdictCache::under(dir.path(), None); + let named = |path: std::path::PathBuf| { + path.strip_prefix(dir.path()) + .expect("a path under the scratch cache") + .display() + .to_string() + }; + ( + named(verdicts.in_flight(workspace_id)), + named(verdicts.marker(workspace_id)), + ) +} + +#[test] +fn the_page_names_the_file_a_killed_launch_leaves() { + let (in_flight, _) = recorded("myws"); + let extension = in_flight + .rsplit_once('.') + .expect("the record carries an extension") + .1 + .to_owned(); + + assert!( + section(&contract_doc(), HEADING).contains(&format!(".{extension}")), + "{CONTRACT_DOC} no longer names the record an interrupted pass leaves, which is \ + `.{extension}`; a reader told to look for it would find nothing" + ); +} + +#[test] +fn the_record_really_is_beside_the_marker() { + // "beside the marker" is the page's own word for where to look, and it is the + // half a reader uses to find the directory at all: the marker's own path is + // published one subsection up, and this sentence hangs off it rather than + // repeating it. + let (in_flight, marker) = recorded("myws"); + let directory = |path: &str| { + path.rsplit_once('/') + .map(|(dir, _)| dir.to_owned()) + .unwrap_or_default() + }; + + assert_eq!( + directory(&in_flight), + directory(&marker), + "{CONTRACT_DOC} says the record sits beside the marker and it no longer does" + ); + assert!( + section(&contract_doc(), HEADING).contains("beside the marker"), + "{CONTRACT_DOC} no longer says where the record is, so nothing tells a reader \ + which directory to look in" + ); +} diff --git a/rust/devlaunch-core/src/flows/provision/verdict_cache.rs b/rust/devlaunch-core/src/flows/provision/verdict_cache.rs index dcb63b4c..0d253935 100644 --- a/rust/devlaunch-core/src/flows/provision/verdict_cache.rs +++ b/rust/devlaunch-core/src/flows/provision/verdict_cache.rs @@ -103,7 +103,7 @@ impl VerdictCache { /// file's name: devpod itself uses the id as a directory name under its own /// contexts, so an id that could not be a path component is one no workspace /// this could be asked about has. - fn marker(&self, workspace_id: &str) -> PathBuf { + pub(super) fn marker(&self, workspace_id: &str) -> PathBuf { self.markers_dir.join(format!("{workspace_id}.json")) } @@ -260,7 +260,7 @@ impl VerdictCache { } /// Where the in-flight record for this workspace lives. - fn in_flight(&self, workspace_id: &str) -> PathBuf { + pub(super) fn in_flight(&self, workspace_id: &str) -> PathBuf { self.marker(workspace_id).with_extension("pass") } From 4eb231e7b0f5ec310416b2a1dc04e2f6365628e8 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 19:08:29 +0000 Subject: [PATCH 4/8] docs: the changelog entry was misfiled and described the fix before the fix Two things. `### Fixed` went in above `### Changed`, where every released section in the file orders Added, Changed, Fixed. And the entry still said the record is removed "when the pass answers", which is what the placement did before the previous commit moved it: the sentence described the defect rather than the behaviour. The paragraph that replaces it says which trip the record closes after and why that is most of the fix, since a reader deciding whether to upgrade wants the window and not the mechanism. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC --- CHANGELOG.md | 64 +++++++++++++++++++++++++++++----------------------- 1 file changed, 36 insertions(+), 28 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f8010b7..3545488e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,34 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -### Fixed - -- **A launch cut short between `devpod up` and the end of dl's setup pass left a - workspace that never got provisioned, and no later launch ever noticed.** The - `up` finishes, devpod writes its create result, and what runs next is dl's own - `devpod ssh --command`: the hostname, the terminal title, the onboarding memo, - `gh` and `claude`. Ctrl-C there and the container is left running with none of - it. Nothing about that container says so. `devpod status` answers `Running` and - devpod's create record is complete, which is exactly what the fast attach asks, - so `dl ` attaches in one round trip and provisions nothing, and does - the same on every launch after that. The workspace is permanently half built and - the recovery is knowing to type `dl up`, which nothing says either. - That is the "sometimes it works" of it: whether you get a whole workspace - depends on which second the Ctrl-C landed in. - - A pass now writes down that it is running, before its trip, in - `${XDG_CACHE_HOME:-~/.cache}/devlaunch/tool-verdicts/.pass`, and - removes it when the pass answers. dl's signal handler `_exit`s without unwinding, - so a record left standing is the one thing on the host that says a pass was cut - short, and a launch that finds one runs the pass before it hands over a shell. - The record carries the same `workspace_result.json` mtime the verdict marker - does, so one left by a container that has since been rebuilt describes nothing - standing and is ignored. - - It is positive evidence, never the absence of it. A workspace brought up by VS - Code, by a hand-typed `devpod up`, or by a build older than the record has no - record either, and those go on attaching in one round trip as before. - ### Changed - **`LaunchVerb::Attach` carries a `RemoteCommand` rather than an @@ -71,6 +43,42 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 still differ, and `publish.yml` now separates a re-run over the commit it published from a push moving the version onto somebody else's tag. +### Fixed + +- **A launch cut short between `devpod up` and the end of dl's setup pass left a + workspace that never got provisioned, and no later launch ever noticed.** The + `up` finishes, devpod writes its create result, and what runs next is dl's own + `devpod ssh --command`: the hostname, the terminal title, the onboarding memo, + `gh` and `claude`. Ctrl-C there and the container is left running with none of + it. Nothing about that container says so. `devpod status` answers `Running` and + devpod's create record is complete, which is exactly what the fast attach asks, + so `dl ` attaches in one round trip and provisions nothing, and does + the same on every launch after that. The workspace is permanently half built and + the recovery is knowing to type `dl up`, which nothing says either. + That is the "sometimes it works" of it: whether you get a whole workspace + depends on which second the Ctrl-C landed in. + + A pass now writes down that it is running, before its first trip, in + `${XDG_CACHE_HOME:-~/.cache}/devlaunch/tool-verdicts/.pass`, and + removes it after its last. dl's signal handler `_exit`s without unwinding, so a + record left standing is the one thing on the host that says a pass was cut + short, and a launch that finds one runs the pass before it hands over a shell. + The record carries the same `workspace_result.json` mtime the verdict marker + does, so one left by a container that has since been rebuilt describes nothing + standing and is ignored. + + **After the last trip and not the first**, which is most of the fix rather than + a detail of it. A pass is up to three trips and the probe is the ~1.7s one; the + lend streams the host's binaries and the install fetches a `claude` of a few + hundred megabytes. A record closed when the probe answered would be open for the + window nobody interrupts and shut for the window everybody does. The one outcome + that leaves it standing is a trip the OS would not make at all, because nothing + was learned about the container and nothing was done to it. + + It is positive evidence, never the absence of it. A workspace brought up by VS + Code, by a hand-typed `devpod up`, or by a build older than the record has no + record either, and those go on attaching in one round trip as before. + ## [0.37.0] - 2026-09-09 ### Fixed From c608245f61b7cd2cbffa6403a24b190369f5df5c Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 19:09:58 +0000 Subject: [PATCH 5/8] release: 0.38.0 The [Unreleased] section as it stands becomes the release: the `RemoteCommand` split, the version-collision guard, and the interrupted setup pass. **0.38.0 and not 0.37.1.** `LaunchVerb::Attach` carries a `RemoteCommand` rather than an `Option`, which is a break for anyone driving `devlaunch-core` directly, and `Provision` grows a method. The method is defaulted so no implementer has to move, but the enum change is not available for a patch reading. What a `dl` user gets is the interrupted pass: a launch killed while the container was being provisioned no longer leaves a workspace that every later launch attaches to without provisioning. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC --- CHANGELOG.md | 2 ++ README.md | 4 ++-- rust/Cargo.lock | 10 +++++----- rust/Cargo.toml | 2 +- 4 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3545488e..b0506538 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.38.0] - 2026-09-09 + ### Changed - **`LaunchVerb::Attach` carries a `RemoteCommand` rather than an diff --git a/README.md b/README.md index 41f21fc7..44dd4bb9 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ one argument instead of a clone, a config file and a build command. [![GitHub pull-requests merged](https://badgen.net/github/merged-prs/blooop/devlaunch)](https://github.com/blooop/devlaunch/pulls?q=is%3Amerged) [![GitHub release](https://img.shields.io/github/release/blooop/devlaunch.svg)](https://GitHub.com/blooop/devlaunch/releases/) [![PyPI](https://img.shields.io/pypi/v/devlaunch)](https://pypi.org/project/devlaunch/) -[![Conda](https://img.shields.io/badge/conda-v0.37.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) +[![Conda](https://img.shields.io/badge/conda-v0.38.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) [![License](https://img.shields.io/github/license/blooop/devlaunch)](https://opensource.org/license/mit/) [![Platform](https://img.shields.io/badge/platform-linux--64-blue)](https://github.com/blooop/devlaunch/releases) [![Pixi Badge](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/prefix-dev/pixi/main/assets/badge/v0.json)](https://pixi.sh) @@ -276,7 +276,7 @@ clone, and [docs/cleanup.md](docs/cleanup.md) says what it carries one past and ```bash $ dl --version -dl 0.37.0 +dl 0.38.0 ``` `--devcontainer ` picks a non-default `devcontainer.json`. A bare name means diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 7f7fd40d..0384eadc 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -13,7 +13,7 @@ dependencies = [ [[package]] name = "aid" -version = "0.37.0" +version = "0.38.0" dependencies = [ "devlaunch-test-support", "dl", @@ -437,7 +437,7 @@ dependencies = [ [[package]] name = "devlaunch-core" -version = "0.37.0" +version = "0.38.0" dependencies = [ "devlaunch-runner", "devlaunch-test-support", @@ -455,7 +455,7 @@ dependencies = [ [[package]] name = "devlaunch-runner" -version = "0.37.0" +version = "0.38.0" dependencies = [ "libc", "portable-pty", @@ -464,7 +464,7 @@ dependencies = [ [[package]] name = "devlaunch-test-support" -version = "0.37.0" +version = "0.38.0" dependencies = [ "devlaunch-runner", "serde", @@ -506,7 +506,7 @@ dependencies = [ [[package]] name = "dl" -version = "0.37.0" +version = "0.38.0" dependencies = [ "clap", "devlaunch-core", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index c0e1a25c..ff7a0106 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -11,7 +11,7 @@ members = [ # The single source of the version (docs/rust-rewrite-plan.md: cutover ships # 0.1.0, version read from Cargo.toml). [workspace.package] -version = "0.37.0" +version = "0.38.0" edition = "2024" license = "MIT" repository = "https://github.com/blooop/devlaunch" From 671fbb19bda8cd303e46d1048fc85599f271dd9e Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 19:14:49 +0000 Subject: [PATCH 6/8] Revert "release: 0.38.0" This reverts commit c608245f61b7cd2cbffa6403a24b190369f5df5c. --- CHANGELOG.md | 2 -- README.md | 4 ++-- rust/Cargo.lock | 10 +++++----- rust/Cargo.toml | 2 +- 4 files changed, 8 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0506538..3545488e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] -## [0.38.0] - 2026-09-09 - ### Changed - **`LaunchVerb::Attach` carries a `RemoteCommand` rather than an diff --git a/README.md b/README.md index 44dd4bb9..41f21fc7 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ one argument instead of a clone, a config file and a build command. [![GitHub pull-requests merged](https://badgen.net/github/merged-prs/blooop/devlaunch)](https://github.com/blooop/devlaunch/pulls?q=is%3Amerged) [![GitHub release](https://img.shields.io/github/release/blooop/devlaunch.svg)](https://GitHub.com/blooop/devlaunch/releases/) [![PyPI](https://img.shields.io/pypi/v/devlaunch)](https://pypi.org/project/devlaunch/) -[![Conda](https://img.shields.io/badge/conda-v0.38.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) +[![Conda](https://img.shields.io/badge/conda-v0.37.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) [![License](https://img.shields.io/github/license/blooop/devlaunch)](https://opensource.org/license/mit/) [![Platform](https://img.shields.io/badge/platform-linux--64-blue)](https://github.com/blooop/devlaunch/releases) [![Pixi Badge](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/prefix-dev/pixi/main/assets/badge/v0.json)](https://pixi.sh) @@ -276,7 +276,7 @@ clone, and [docs/cleanup.md](docs/cleanup.md) says what it carries one past and ```bash $ dl --version -dl 0.38.0 +dl 0.37.0 ``` `--devcontainer ` picks a non-default `devcontainer.json`. A bare name means diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 0384eadc..7f7fd40d 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -13,7 +13,7 @@ dependencies = [ [[package]] name = "aid" -version = "0.38.0" +version = "0.37.0" dependencies = [ "devlaunch-test-support", "dl", @@ -437,7 +437,7 @@ dependencies = [ [[package]] name = "devlaunch-core" -version = "0.38.0" +version = "0.37.0" dependencies = [ "devlaunch-runner", "devlaunch-test-support", @@ -455,7 +455,7 @@ dependencies = [ [[package]] name = "devlaunch-runner" -version = "0.38.0" +version = "0.37.0" dependencies = [ "libc", "portable-pty", @@ -464,7 +464,7 @@ dependencies = [ [[package]] name = "devlaunch-test-support" -version = "0.38.0" +version = "0.37.0" dependencies = [ "devlaunch-runner", "serde", @@ -506,7 +506,7 @@ dependencies = [ [[package]] name = "dl" -version = "0.38.0" +version = "0.37.0" dependencies = [ "clap", "devlaunch-core", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index ff7a0106..c0e1a25c 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -11,7 +11,7 @@ members = [ # The single source of the version (docs/rust-rewrite-plan.md: cutover ships # 0.1.0, version read from Cargo.toml). [workspace.package] -version = "0.38.0" +version = "0.37.0" edition = "2024" license = "MIT" repository = "https://github.com/blooop/devlaunch" From fbdfe3eebe34938d19c650b358cb5b2e07e8f87d Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 19:17:56 +0000 Subject: [PATCH 7/8] fix: the public surface grew a LaunchNotice variant nobody wrote down CI's `public-api` job caught it: `LaunchNotice::SetupPassNeverFinished` is a variant of a public enum that `api` re-exports, so it is four rows of `public-api.api.txt` and I added none of them. The six `pass_never_finished` rows I did add came back clean, so the placement rule held; what I missed is that a notice is public surface at all. The snapshots are hand-maintained on this branch because `cargo-public-api` needs a nightly toolchain the devcontainer does not carry, which is exactly the gap this job exists to cover. Rows taken from the job's own diff. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC --- rust/devlaunch-core/public-api.api.txt | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/rust/devlaunch-core/public-api.api.txt b/rust/devlaunch-core/public-api.api.txt index 98ddfe4d..ea953a39 100644 --- a/rust/devlaunch-core/public-api.api.txt +++ b/rust/devlaunch-core/public-api.api.txt @@ -81,6 +81,8 @@ pub devlaunch_core::api::LaunchNotice::SessionManagerReady::pane_id: alloc::stri pub devlaunch_core::api::LaunchNotice::SessionManagerReady::socket: alloc::string::String pub devlaunch_core::api::LaunchNotice::SessionManagerUnavailable pub devlaunch_core::api::LaunchNotice::SessionManagerUnavailable::reason: alloc::string::String +pub devlaunch_core::api::LaunchNotice::SetupPassNeverFinished +pub devlaunch_core::api::LaunchNotice::SetupPassNeverFinished::workspace_id: alloc::string::String pub devlaunch_core::api::LaunchNotice::SshCommand pub devlaunch_core::api::LaunchNotice::SshCommand::argv: alloc::vec::Vec pub devlaunch_core::api::LaunchNotice::SshConfigUnlocatable @@ -576,6 +578,8 @@ pub devlaunch_core::flows::launch::LaunchNotice::SessionManagerReady::pane_id: a pub devlaunch_core::flows::launch::LaunchNotice::SessionManagerReady::socket: alloc::string::String pub devlaunch_core::flows::launch::LaunchNotice::SessionManagerUnavailable pub devlaunch_core::flows::launch::LaunchNotice::SessionManagerUnavailable::reason: alloc::string::String +pub devlaunch_core::flows::launch::LaunchNotice::SetupPassNeverFinished +pub devlaunch_core::flows::launch::LaunchNotice::SetupPassNeverFinished::workspace_id: alloc::string::String pub devlaunch_core::flows::launch::LaunchNotice::SshCommand pub devlaunch_core::flows::launch::LaunchNotice::SshCommand::argv: alloc::vec::Vec pub devlaunch_core::flows::launch::LaunchNotice::SshConfigUnlocatable From acc02f4f211acc764ff0fe8e2b21382d9f900f78 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Wed, 9 Sep 2026 19:18:04 +0000 Subject: [PATCH 8/8] release: 0.39.0, not the 0.38.0 that shipped without this #597 cut and merged 0.38.0 while this branch was in review, so the `release: 0.38.0` commit here re-cut a version that already exists. It is reverted and the entry refiled under 0.39.0. The merge is what makes this worth spelling out. `git merge origin/main` put my `### Fixed` block *inside* the shipped `## [0.38.0]` heading with no conflict -- blooop/devlaunch#527's failure exactly, arriving the way it always does, which is silently. main would have claimed 0.38.0 fixed the interrupted setup pass, and 0.38.0 does not: it is built from 74a1d88. The 0.38.0, 0.37.0 and 0.36.0 sections are byte-identical to `git show origin/main:CHANGELOG.md`, which is the check that says this refiles the entry rather than editing the record. Claude-Session: https://claude.ai/code/session_01JCdaLQ1KgAe8G9QNUeNzyC --- CHANGELOG.md | 74 +++++++++++++++++++++++++------------------------ README.md | 4 +-- rust/Cargo.lock | 10 +++---- rust/Cargo.toml | 2 +- 4 files changed, 46 insertions(+), 44 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b0506538..dcfd360c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,44 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +## [0.39.0] - 2026-09-09 + +### Fixed + +- **A launch cut short between `devpod up` and the end of dl's setup pass left a + workspace that never got provisioned, and no later launch ever noticed.** The + `up` finishes, devpod writes its create result, and what runs next is dl's own + `devpod ssh --command`: the hostname, the terminal title, the onboarding memo, + `gh` and `claude`. Ctrl-C there and the container is left running with none of + it. Nothing about that container says so. `devpod status` answers `Running` and + devpod's create record is complete, which is exactly what the fast attach asks, + so `dl ` attaches in one round trip and provisions nothing, and does + the same on every launch after that. The workspace is permanently half built and + the recovery is knowing to type `dl up`, which nothing says either. + That is the "sometimes it works" of it: whether you get a whole workspace + depends on which second the Ctrl-C landed in. + + A pass now writes down that it is running, before its first trip, in + `${XDG_CACHE_HOME:-~/.cache}/devlaunch/tool-verdicts/.pass`, and + removes it after its last. dl's signal handler `_exit`s without unwinding, so a + record left standing is the one thing on the host that says a pass was cut + short, and a launch that finds one runs the pass before it hands over a shell. + The record carries the same `workspace_result.json` mtime the verdict marker + does, so one left by a container that has since been rebuilt describes nothing + standing and is ignored. + + **After the last trip and not the first**, which is most of the fix rather than + a detail of it. A pass is up to three trips and the probe is the ~1.7s one; the + lend streams the host's binaries and the install fetches a `claude` of a few + hundred megabytes. A record closed when the probe answered would be open for the + window nobody interrupts and shut for the window everybody does. The one outcome + that leaves it standing is a trip the OS would not make at all, because nothing + was learned about the container and nothing was done to it. + + It is positive evidence, never the absence of it. A workspace brought up by VS + Code, by a hand-typed `devpod up`, or by a build older than the record has no + record either, and those go on attaching in one round trip as before. + ## [0.38.0] - 2026-09-09 ### Changed @@ -45,42 +83,6 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 still differ, and `publish.yml` now separates a re-run over the commit it published from a push moving the version onto somebody else's tag. -### Fixed - -- **A launch cut short between `devpod up` and the end of dl's setup pass left a - workspace that never got provisioned, and no later launch ever noticed.** The - `up` finishes, devpod writes its create result, and what runs next is dl's own - `devpod ssh --command`: the hostname, the terminal title, the onboarding memo, - `gh` and `claude`. Ctrl-C there and the container is left running with none of - it. Nothing about that container says so. `devpod status` answers `Running` and - devpod's create record is complete, which is exactly what the fast attach asks, - so `dl ` attaches in one round trip and provisions nothing, and does - the same on every launch after that. The workspace is permanently half built and - the recovery is knowing to type `dl up`, which nothing says either. - That is the "sometimes it works" of it: whether you get a whole workspace - depends on which second the Ctrl-C landed in. - - A pass now writes down that it is running, before its first trip, in - `${XDG_CACHE_HOME:-~/.cache}/devlaunch/tool-verdicts/.pass`, and - removes it after its last. dl's signal handler `_exit`s without unwinding, so a - record left standing is the one thing on the host that says a pass was cut - short, and a launch that finds one runs the pass before it hands over a shell. - The record carries the same `workspace_result.json` mtime the verdict marker - does, so one left by a container that has since been rebuilt describes nothing - standing and is ignored. - - **After the last trip and not the first**, which is most of the fix rather than - a detail of it. A pass is up to three trips and the probe is the ~1.7s one; the - lend streams the host's binaries and the install fetches a `claude` of a few - hundred megabytes. A record closed when the probe answered would be open for the - window nobody interrupts and shut for the window everybody does. The one outcome - that leaves it standing is a trip the OS would not make at all, because nothing - was learned about the container and nothing was done to it. - - It is positive evidence, never the absence of it. A workspace brought up by VS - Code, by a hand-typed `devpod up`, or by a build older than the record has no - record either, and those go on attaching in one round trip as before. - ## [0.37.0] - 2026-09-09 ### Fixed diff --git a/README.md b/README.md index 44dd4bb9..807e1c01 100644 --- a/README.md +++ b/README.md @@ -19,7 +19,7 @@ one argument instead of a clone, a config file and a build command. [![GitHub pull-requests merged](https://badgen.net/github/merged-prs/blooop/devlaunch)](https://github.com/blooop/devlaunch/pulls?q=is%3Amerged) [![GitHub release](https://img.shields.io/github/release/blooop/devlaunch.svg)](https://GitHub.com/blooop/devlaunch/releases/) [![PyPI](https://img.shields.io/pypi/v/devlaunch)](https://pypi.org/project/devlaunch/) -[![Conda](https://img.shields.io/badge/conda-v0.38.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) +[![Conda](https://img.shields.io/badge/conda-v0.39.0-brightgreen?logo=anaconda)](https://prefix.dev/channels/blooop/packages/devlaunch) [![License](https://img.shields.io/github/license/blooop/devlaunch)](https://opensource.org/license/mit/) [![Platform](https://img.shields.io/badge/platform-linux--64-blue)](https://github.com/blooop/devlaunch/releases) [![Pixi Badge](https://img.shields.io/endpoint?url=https://raw.githubusercontent.com/prefix-dev/pixi/main/assets/badge/v0.json)](https://pixi.sh) @@ -276,7 +276,7 @@ clone, and [docs/cleanup.md](docs/cleanup.md) says what it carries one past and ```bash $ dl --version -dl 0.38.0 +dl 0.39.0 ``` `--devcontainer ` picks a non-default `devcontainer.json`. A bare name means diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 0384eadc..226ff68c 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -13,7 +13,7 @@ dependencies = [ [[package]] name = "aid" -version = "0.38.0" +version = "0.39.0" dependencies = [ "devlaunch-test-support", "dl", @@ -437,7 +437,7 @@ dependencies = [ [[package]] name = "devlaunch-core" -version = "0.38.0" +version = "0.39.0" dependencies = [ "devlaunch-runner", "devlaunch-test-support", @@ -455,7 +455,7 @@ dependencies = [ [[package]] name = "devlaunch-runner" -version = "0.38.0" +version = "0.39.0" dependencies = [ "libc", "portable-pty", @@ -464,7 +464,7 @@ dependencies = [ [[package]] name = "devlaunch-test-support" -version = "0.38.0" +version = "0.39.0" dependencies = [ "devlaunch-runner", "serde", @@ -506,7 +506,7 @@ dependencies = [ [[package]] name = "dl" -version = "0.38.0" +version = "0.39.0" dependencies = [ "clap", "devlaunch-core", diff --git a/rust/Cargo.toml b/rust/Cargo.toml index ff7a0106..1c78176d 100644 --- a/rust/Cargo.toml +++ b/rust/Cargo.toml @@ -11,7 +11,7 @@ members = [ # The single source of the version (docs/rust-rewrite-plan.md: cutover ships # 0.1.0, version read from Cargo.toml). [workspace.package] -version = "0.38.0" +version = "0.39.0" edition = "2024" license = "MIT" repository = "https://github.com/blooop/devlaunch"