Handover: validate the #673 stack (#674–#676) on a real host #677

Activity

1. blooop commented on Oct 9, 2026

   

   blooop

   on Oct 9, 2026

   OwnerAuthor

   More actions

   Update: the tests for defects 1 and 2 are written. This diff replaces the one in the issue body; it holds both fixes and the tests, on top of feat/refresh-stale at f5551ced.

   • Red: with the two fix lines undone, 6 tests fail (a_pane_id_is_read_as_herdrs_base32_number and five stale_images tests, whose fakes now say "ID").
   • Green: cargo test --locked -p devlaunch-core -p dl passes except four flows::lifecycle::tests (commits_pushed_by_url_do_not_stop_a_plain_rm and three others). Those four fail the same way without this diff, and CI rust passes them at f5551ced, so they depend on the first host. Re-check them on the new machine.

   Apply with git apply from the repo root:

   diff --git a/rust/devlaunch-core/src/clients/herdr.rs b/rust/devlaunch-core/src/clients/herdr.rs
   index 70cf5279..1ccc4665 100644
   --- a/rust/devlaunch-core/src/clients/herdr.rs
   +++ b/rust/devlaunch-core/src/clients/herdr.rs
   @@ -2100,7 +2100,9 @@ pub(crate) enum Saved {
    /// an internal number that `public_pane_numbers` maps to the public one.
    pub(crate) fn saved_argv(session_json: &str, pane_id: &str) -> Option<Option<Vec<String>>> {
        let (workspace_id, public) = pane_id.split_once(":p")?;
   -    let public: u64 = public.parse().ok()?;
   +    let public: u64 = public.chars().try_fold(0u64, |n, c| {
   +        "0123456789ABCDEFGHJKMNPQRSTVWXYZ".find(c.to_ascii_uppercase()).map(|d| n * 32 + d as u64)
   +    })?;
        let session: serde_json::Value = serde_json::from_str(session_json).ok()?;
        let Some(workspace) = session
            .get("workspaces")?
   @@ -2411,6 +2413,18 @@ mod resume_tests {
            assert_eq!(report.saved_in(&session(r#"["dl","other"]"#)), Saved::Other);
        }
    
   +    /// herdr numbers panes in Crockford base32: `w3:pAX` is public number 349,
   +    /// as a live herdr's `session.json` showed (`"226": 349`). Ids up to `p9`
   +    /// read the same in base 10, which is how a decimal parse passed the tests above.
   +    #[test]
   +    fn a_pane_id_is_read_as_herdrs_base32_number() {
   +        let json = session(r#"["dl","ws"]"#)
   +            .replace(r#""id":"w1","public_pane_numbers":{"1":1,"4":3}"#, r#""id":"w3","public_pane_numbers":{"1":1,"4":349}"#);
   +        assert_eq!(saved_argv(&json, "w3:pAX"), Some(Some(vec!["dl".to_owned(), "ws".to_owned()])));
   +        assert_eq!(saved_argv(&json, "w3:pax"), Some(Some(vec!["dl".to_owned(), "ws".to_owned()])));
   +        assert_eq!(saved_argv(&json, "w3:pAY"), Some(None));
   +    }
   +
        #[test]
        fn a_pane_with_no_argv_lacks_one() {
            let json = session(r#"["dl","ws"]"#).replace("\"agent_resume\"", "\"other\"");
   diff --git a/rust/devlaunch-core/src/flows/lifecycle/tests.rs b/rust/devlaunch-core/src/flows/lifecycle/tests.rs
   index 5311436e..27d99631 100644
   --- a/rust/devlaunch-core/src/flows/lifecycle/tests.rs
   +++ b/rust/devlaunch-core/src/flows/lifecycle/tests.rs
   @@ -91,7 +91,7 @@ fn devpod_home_recording(
        std::fs::write(
            &result,
            serde_json::json!({
   -            "ContainerDetails": { "Id": "container-id" },
   +            "ContainerDetails": { "ID": "container-id" },
                "MergedConfig": {},
                "SubstitutionContext": {
                    "LocalWorkspaceFolder": local_workspace_folder,
   diff --git a/rust/devlaunch-core/src/flows/stale_images.rs b/rust/devlaunch-core/src/flows/stale_images.rs
   index 2957684c..35bd21e9 100644
   --- a/rust/devlaunch-core/src/flows/stale_images.rs
   +++ b/rust/devlaunch-core/src/flows/stale_images.rs
   @@ -213,7 +213,7 @@ fn created(devpod_home: Option<&DevpodHome>, workspace_id: &str) -> Option<Creat
        let details = &document["ContainerDetails"];
        let created_from = text(&details["Config"]["Image"])?;
        Some(Created {
   -        container: text(&details["Id"])?,
   +        container: text(&details["ID"])?,
            reference: text(&document["MergedConfig"]["image"]).unwrap_or_else(|| created_from.clone()),
            created_from,
        })
   @@ -309,7 +309,7 @@ mod tests {
            std::fs::write(
                home.result("default", workspace_id),
                serde_json::json!({
   -                "ContainerDetails": { "Id": container, "Config": { "Image": created_from } },
   +                "ContainerDetails": { "ID": container, "Config": { "Image": created_from } },
                    "MergedConfig": merged,
                })
                .to_string(),
   diff --git a/rust/dl/tests/read_side.rs b/rust/dl/tests/read_side.rs
   index 0a5269b0..f505417b 100644
   --- a/rust/dl/tests/read_side.rs
   +++ b/rust/dl/tests/read_side.rs
   @@ -423,7 +423,7 @@ fn world_with_a_stale_workspace() -> World {
        std::fs::write(
            records.join("workspace_result.json"),
            format!(
   -            r#"{{"ContainerDetails": {{"Id": "c1", "Config": {{"Image": "{STALE_REFERENCE}"}}}}, "MergedConfig": {{}}}}"#
   +            r#"{{"ContainerDetails": {{"ID": "c1", "Config": {{"Image": "{STALE_REFERENCE}"}}}}, "MergedConfig": {{}}}}"#
            ),
        )
        .expect("a create result");
   @@ -962,7 +962,7 @@ fn world_with_stale(stale: &[(&str, &str, &str)]) -> World {
            std::fs::write(
                records.join("workspace_result.json"),
                format!(
   -                r#"{{"ContainerDetails": {{"Id": "{container}", "Config": {{"Image": "{STALE_REFERENCE}"}}}}, "MergedConfig": {{}}}}"#
   +                r#"{{"ContainerDetails": {{"ID": "{container}", "Config": {{"Image": "{STALE_REFERENCE}"}}}}, "MergedConfig": {{}}}}"#
                ),
            )
            .expect("a create result");

   Steps left (from the issue body): row 6 (nothing stale), step 4 with an idle agent, and a decision on findings 3, 5 and 6.

   🤖 Generated with Claude Code
2. blooop commented on Oct 9, 2026

   

   blooop

   on Oct 9, 2026

   OwnerAuthor

   More actions

   Update 2: findings 6 and 7 are fixed with tests, and findings 3 and 5 need a decision.

   Fixed, on top of the previous comment's diff (stale_images.rs):

   • Finding 6: when docker image inspect cannot find the running image, a container made from the reference itself is stale if its image id differs from the reference's id. A derived image is not judged without its layers.
   • Finding 7: Created is parsed as a jiff::Timestamp, not cut to 19 characters, so ...Z and ...+01:00 compare correctly.
   • Red: with the old behaviour, a_container_made_from_the_reference_is_stale_after_its_image_left_the_store and created_times_compare_across_utc_offsets fail. Green: all 15 stale_images tests pass. cargo clippy -p devlaunch-core --all-targets -D warnings is clean. The same four host-only lifecycle tests fail, as before.

   Apply after the previous comment's diff:

   diff --git a/rust/devlaunch-core/src/flows/stale_images.rs b/rust/devlaunch-core/src/flows/stale_images.rs
   index 35bd21e9..73db8011 100644
   --- a/rust/devlaunch-core/src/flows/stale_images.rs
   +++ b/rust/devlaunch-core/src/flows/stale_images.rs
   @@ -137,10 +137,10 @@ struct Created {
    #[derive(Clone, Debug, PartialEq, Eq)]
    struct Image {
        id: String,
   -    /// docker's `Created`, cut to whole seconds: `YYYY-MM-DDTHH:MM:SS`. docker
   -    /// writes it in UTC with a varying count of fractional digits, so the cut is
   -    /// what makes two of them compare in time order as strings.
   -    created: String,
   +    /// docker's `Created`, parsed. docker does not always write it in UTC: with
   +    /// the containerd image store a pulled image reads `...Z` and a local build
   +    /// `...+01:00`, so a cut-and-compare of the text is off by the offset.
   +    created: jiff::Timestamp,
        layers: Vec<String>,
    }
    
   @@ -177,9 +177,17 @@ pub fn stale_images<'w>(
        let by_workspace = created
            .into_iter()
            .filter_map(|(id, created)| {
   -            let running = running_images.get(running.get(&created.container)?)?;
   +            let running_id = running.get(&created.container)?;
                let current = references.get(&created.reference)?;
   -            is_stale(&created, running, current).then(|| {
   +            let stale = match running_images.get(running_id) {
   +                Some(running) => is_stale(&created, running, current),
   +                // With the containerd image store, a local build whose only tag
   +                // moved is gone from `docker image inspect` while a container
   +                // still runs it. Its id is enough when the container was made
   +                // from the reference itself; a derived image needs its layers.
   +                None => created.created_from == created.reference && *running_id != current.id,
   +            };
   +            stale.then(|| {
                    (
                        id.to_owned(),
                        StaleImage {
   @@ -269,7 +277,7 @@ fn images(runner: &dyn Runner, names: impl IntoIterator<Item = String>) -> BTree
                    id.to_owned(),
                    Image {
                        id: id.to_owned(),
   -                    created: created.get(..19)?.to_owned(),
   +                    created: created.parse().ok()?,
                        layers,
                    },
                ))
   @@ -582,6 +590,75 @@ mod tests {
            assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty());
        }
    
   +    /// With the containerd image store, a local build whose only tag moved is
   +    /// gone from `docker image inspect` while the container still runs it.
   +    fn running_image_gone(fake: &FakeRunner, id: &str) {
   +        fake.script(
   +            [
   +                "docker",
   +                "inspect",
   +                "--type",
   +                "image",
   +                "--format",
   +                IMAGE_FORMAT,
   +                id,
   +            ],
   +            Response::failed(1, format!("Error: No such image: {id}\n")),
   +        );
   +    }
   +
   +    #[test]
   +    fn a_container_made_from_the_reference_is_stale_after_its_image_left_the_store() {
   +        let home = home_with("ws", "dl-test:latest", Some("dl-test:latest"));
   +        let fake = FakeRunner::new();
   +        inspect_container(&fake, "sha256:old");
   +        running_image_gone(&fake, "sha256:old");
   +        inspect_image(&fake, "dl-test:latest", line("sha256:new", &["l1", "l2"]));
   +
   +        let stale = stale_images(&fake, Some(&home), ["ws"]);
   +
   +        assert_eq!(
   +            stale.of("ws").map(StaleImage::reference),
   +            Some("dl-test:latest")
   +        );
   +    }
   +
   +    #[test]
   +    fn a_derived_image_that_left_the_store_is_not_judged_without_its_layers() {
   +        let home = home_with("ws", "devpod-abc", Some("ghcr.io/o/img:latest"));
   +        let fake = FakeRunner::new();
   +        inspect_container(&fake, "sha256:derived");
   +        running_image_gone(&fake, "sha256:derived");
   +        inspect_image(&fake, "ghcr.io/o/img:latest", line("sha256:base2", &["l1"]));
   +
   +        assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty());
   +    }
   +
   +    /// 10:10 at +01:00 is 09:10 UTC, so a base pulled at 09:30 UTC is the newer
   +    /// one. Compared as text, "10:10" sorts after "09:30" and hid it.
   +    #[test]
   +    fn created_times_compare_across_utc_offsets() {
   +        let home = home_with("ws", "devpod-abc", Some("ghcr.io/o/img:latest"));
   +        let fake = FakeRunner::new();
   +        inspect_container(&fake, "sha256:derived");
   +        inspect_image(
   +            &fake,
   +            "sha256:derived",
   +            line_at(
   +                "sha256:derived",
   +                "2026-10-08T10:10:00.5+01:00",
   +                &["l1", "f1"],
   +            ),
   +        );
   +        inspect_image(
   +            &fake,
   +            "ghcr.io/o/img:latest",
   +            line_at("sha256:base2", "2026-10-08T09:30:00Z", &["l2"]),
   +        );
   +
   +        assert!(stale_images(&fake, Some(&home), ["ws"]).of("ws").is_some());
   +    }
   +
        #[test]
        fn a_missing_container_costs_the_others_nothing() {
            // docker prints a line for every container it found and exits 1 for the

   (cargo fmt also reflowed the herdr.rs lines from the previous diff. Run cargo fmt after you apply.)

   Finding 3, decision needed: a restarted agent loses its saved line.
   Cause: only aid gives dl a line to report (dl/src/lib.rs:573 passes None; aid/src/main.rs:251 is the only run_resumable caller). A typed-back dl ws -- … claude --resume <id> reports nothing and gets no DEVLAUNCH_HERDR_RESUME. This is deliberate: dl/src/lib.rs:579-581 says a dl that reads a line out of a command tail "would be guessing at another program's flags". Also, with DEVLAUNCH_HERDR=1, the old agent's SessionEnd hook runs pane release-agent (herdr.rs:1592), which can clear the saved line.
   Options:

   • (a) Recommended: recreate reports the exact line it typed into the pane again, after Restarted::Typed, and retries on resume_not_accepted. No guess, because the line is the one herdr held. Also check the order against the old agent's release.
   • (b) dl builds an AgentResume from a tail that ends in claude … --resume <id> (at the None arm of launch.rs:2888). This reverses the recorded decision. Nearest tests: an_agent_launched_in_a_pane_tells_herdr_how_to_start_it_again and a_consented_pane_hands_the_container_the_line_to_resume_by_id in launch.rs.

   Finding 5, decision needed: a recreate keeps a devpod-derived image.
   devpod v0.26.3 has no flag that forces the derived image to rebuild (up has --recreate and --reset only; build has no force flag). The derived tag is a hash of the config, not of the base image, so devpod reuses it every time.
   Options:

   • (a) Before up --recreate, when the workspace is stale and createdFrom ≠ the declared image, run docker rmi <createdFrom> without -f. If docker refuses (another container uses it), fall back to (b).
   • (b) --refresh-stale skips such a workspace and says that a recreate cannot move it, so it does not recreate it on every run.

   🤖 Generated with Claude Code