diff --git a/CHANGELOG.md b/CHANGELOG.md index 03e2b7d4..2c0639ab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Added + +- **`dl --ls` says which workspaces run an older image than their reference names** (#673). + A pull moves an image reference and leaves every running container on the image it was + created from, so long-lived workspaces drifted days behind and nothing said so. `dl --ls` + now prints one line under the table for each such workspace, naming the reference and + `dl recreate`, and `dl --ls --json` appends a `staleImage` key to its row. The check + reads devpod's `workspace_result.json` and asks docker for the container's image and the + reference's current one; where devpod built a derived image on top of the declared one, + it compares layers. Every doubt reads as current. + ## [0.60.1] - 2026-10-16 ### Changed diff --git a/docs/agents-using-dl.md b/docs/agents-using-dl.md index 911358de..08765960 100644 --- a/docs/agents-using-dl.md +++ b/docs/agents-using-dl.md @@ -162,6 +162,14 @@ included, is a reason not to destroy anything without a person in the loop, and caller that tests for `wouldLose` alone will force-delete exactly the rows `rm` itself would have stopped at. +Two keys are appended only when they have something to say, so a caller must not +expect them. `lastSweep` is described in [cleanup.md](cleanup.md). `staleImage` is a +string, the image reference the workspace's container is behind: the container runs an +older image than that reference now names, because a pull moved the reference and only a +new container picks it up. `dl recreate` moves it. The key is absent on a current +row and also on a row `dl` could not read about (no docker, no devpod result, a +reference this machine never pulled), so absence is not proof that a row is current. + `--json` is currently `--ls` only. Other commands report in English, so a caller that creates a workspace and then needs to address it should derive the id from a subsequent `--ls --json` rather than parsing the launch. diff --git a/rust/devlaunch-core/public-api.rest.txt b/rust/devlaunch-core/public-api.rest.txt index b9d88ac6..5d5a911a 100644 --- a/rust/devlaunch-core/public-api.rest.txt +++ b/rust/devlaunch-core/public-api.rest.txt @@ -2559,6 +2559,8 @@ pub devlaunch_core::flows::listing::DlView::cache_dir: &'a std::path::Path pub devlaunch_core::flows::listing::DlView::clones: &'a dyn devlaunch_core::flows::listing::ClonePathResolver pub devlaunch_core::flows::listing::DlView::storage: &'a devlaunch_core::domain::metadata::MetadataStorage pub struct devlaunch_core::flows::listing::ListedWorkspace +impl devlaunch_core::flows::listing::ListedWorkspace +pub fn devlaunch_core::flows::listing::ListedWorkspace::id(&self) -> &str impl core::clone::Clone for devlaunch_core::flows::listing::ListedWorkspace pub fn devlaunch_core::flows::listing::ListedWorkspace::clone(&self) -> devlaunch_core::flows::listing::ListedWorkspace impl core::cmp::Eq for devlaunch_core::flows::listing::ListedWorkspace @@ -2652,6 +2654,7 @@ pub fn devlaunch_core::flows::listing::describe_source(&devlaunch_core::clients: pub fn devlaunch_core::flows::listing::discover_repos_from_workspaces(&devlaunch_core::clients::git::Git<'_>, &[devlaunch_core::clients::devpod::Workspace]) -> devlaunch_core::flows::listing::RepoDiscovery pub fn devlaunch_core::flows::listing::flatten_repos(&indexmap::map::IndexMap>) -> alloc::vec::Vec pub fn devlaunch_core::flows::listing::head_branch_of(&devlaunch_core::clients::devpod::Workspace, &std::path::Path) -> core::option::Option +pub fn devlaunch_core::flows::listing::mark_stale_images(&mut [devlaunch_core::flows::listing::ListedWorkspace], &devlaunch_core::flows::stale_images::StaleImages) pub fn devlaunch_core::flows::listing::outstanding_sweep_notes(&devlaunch_core::domain::metadata::MetadataStorage) -> alloc::vec::Vec pub fn devlaunch_core::flows::listing::owner_of(&devlaunch_core::clients::devpod::Workspace, &std::path::Path) -> core::option::Option pub fn devlaunch_core::flows::listing::repo_of(&devlaunch_core::clients::devpod::Workspace, &std::path::Path) -> core::option::Option @@ -3189,6 +3192,33 @@ impl core::fmt::Debug for devlaunch_core::flows::session_manager::PaneDestinatio pub fn devlaunch_core::flows::session_manager::PaneDestination::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::session_manager::PaneDestination pub fn devlaunch_core::flows::session_manager::pane_destination(&dyn devlaunch_runner::Runner) -> devlaunch_core::flows::session_manager::PaneDestination +pub mod devlaunch_core::flows::stale_images +pub struct devlaunch_core::flows::stale_images::StaleImage +impl devlaunch_core::flows::stale_images::StaleImage +pub fn devlaunch_core::flows::stale_images::StaleImage::reference(&self) -> &str +impl core::clone::Clone for devlaunch_core::flows::stale_images::StaleImage +pub fn devlaunch_core::flows::stale_images::StaleImage::clone(&self) -> devlaunch_core::flows::stale_images::StaleImage +impl core::cmp::Eq for devlaunch_core::flows::stale_images::StaleImage +impl core::cmp::PartialEq for devlaunch_core::flows::stale_images::StaleImage +pub fn devlaunch_core::flows::stale_images::StaleImage::eq(&self, &devlaunch_core::flows::stale_images::StaleImage) -> bool +impl core::fmt::Debug for devlaunch_core::flows::stale_images::StaleImage +pub fn devlaunch_core::flows::stale_images::StaleImage::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::stale_images::StaleImage +pub struct devlaunch_core::flows::stale_images::StaleImages +impl devlaunch_core::flows::stale_images::StaleImages +pub fn devlaunch_core::flows::stale_images::StaleImages::is_empty(&self) -> bool +pub fn devlaunch_core::flows::stale_images::StaleImages::of(&self, &str) -> core::option::Option<&devlaunch_core::flows::stale_images::StaleImage> +impl core::clone::Clone for devlaunch_core::flows::stale_images::StaleImages +pub fn devlaunch_core::flows::stale_images::StaleImages::clone(&self) -> devlaunch_core::flows::stale_images::StaleImages +impl core::cmp::Eq for devlaunch_core::flows::stale_images::StaleImages +impl core::cmp::PartialEq for devlaunch_core::flows::stale_images::StaleImages +pub fn devlaunch_core::flows::stale_images::StaleImages::eq(&self, &devlaunch_core::flows::stale_images::StaleImages) -> bool +impl core::default::Default for devlaunch_core::flows::stale_images::StaleImages +pub fn devlaunch_core::flows::stale_images::StaleImages::default() -> devlaunch_core::flows::stale_images::StaleImages +impl core::fmt::Debug for devlaunch_core::flows::stale_images::StaleImages +pub fn devlaunch_core::flows::stale_images::StaleImages::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::stale_images::StaleImages +pub fn devlaunch_core::flows::stale_images::stale_images<'w>(&dyn devlaunch_runner::Runner, core::option::Option<&devlaunch_core::clients::devpod_home::DevpodHome>, impl core::iter::traits::collect::IntoIterator) -> devlaunch_core::flows::stale_images::StaleImages pub mod devlaunch_core::flows::workspace_clone pub enum devlaunch_core::flows::workspace_clone::EnsureBranchError pub devlaunch_core::flows::workspace_clone::EnsureBranchError::Branch(devlaunch_core::flows::branch_manager::BranchError) diff --git a/rust/devlaunch-core/src/clients/docker.rs b/rust/devlaunch-core/src/clients/docker.rs index 5b3d48c6..4b83d27d 100644 --- a/rust/devlaunch-core/src/clients/docker.rs +++ b/rust/devlaunch-core/src/clients/docker.rs @@ -217,6 +217,33 @@ pub(crate) fn exec_as_root( read(runner, &args, READ_INSIDE) } +/// A `--format`ted `docker inspect` over these names: what it printed, whatever +/// it exited. +/// +/// Whatever it exited, because `docker inspect` over several names prints a line +/// for every name it found and then exits 1 if any one of them was missing. A +/// container removed by hand, or an image reference this machine never pulled, +/// would otherwise cost every other name its answer. So each format the callers +/// pass puts the name's id in the line, and the caller matches on that id and +/// never on the line's position. +/// +/// `None` is a docker that did not run at all, which every caller reads as "could +/// not tell". +pub(crate) fn inspect_formatted( + runner: &dyn Runner, + kind: &str, + format: &str, + names: &NonEmpty, +) -> Option { + let mut args = vec!["inspect", "--type", kind, "--format", format]; + args.extend(names.iter().map(String::as_str)); + let spec = SpawnSpec::from(Invocation::new(PROGRAM).with_args(args.iter().copied())) + .with_timeout(INSPECT_THEM); + ran(runner.capture(&spec)) + .ok() + .map(|(_, CapturedText { stdout, .. })| stdout) +} + /// One bounded docker read: its stdout when it exited 0, and otherwise words /// for why not. fn read(runner: &dyn Runner, args: &[&str], timeout: Duration) -> Result { diff --git a/rust/devlaunch-core/src/flows/listing.rs b/rust/devlaunch-core/src/flows/listing.rs index 97fb7c9c..6cb40fd1 100644 --- a/rust/devlaunch-core/src/flows/listing.rs +++ b/rust/devlaunch-core/src/flows/listing.rs @@ -68,6 +68,7 @@ use crate::domain::model::{SweepNote, WorktreeInfo}; use crate::domain::workspace_state::{BareCache, CouldNotTell, NonEmpty}; use crate::flows::agent_worktrees::{self, Verdict}; use crate::flows::disk_usage::{self, DiskUsage}; +use crate::flows::stale_images::{StaleImage, StaleImages}; use crate::runner::Runner; use crate::timing; @@ -1002,6 +1003,28 @@ pub struct ListedWorkspace { /// to carry it at all. [`outstanding_sweep_notes`] is the reading that misses /// none of them, and is what `dl --ls` prints under the table. pub(crate) sweep: Option, + /// The reference this workspace's container is behind, where it is behind + /// one (devlaunch#673). Filled by [`mark_stale_images`] after the rows are + /// built, because the answer comes from one batched docker read over every + /// row and not from a question each row asks alone. + pub(crate) stale_image: Option, +} + +impl ListedWorkspace { + /// devpod's id for the workspace this row is about. + pub fn id(&self) -> &str { + &self.id + } +} + +/// Fill in which rows run an older image than their reference now names. +/// +/// Apart from [`enriched_listing`] so that its signature, which `wf` calls, does +/// not move. A row this reads nothing about keeps `None`. +pub fn mark_stale_images(rows: &mut [ListedWorkspace], stale: &StaleImages) { + for row in rows { + row.stale_image = stale.of(&row.id).cloned(); + } } /// The enriched listing: one row per workspace devpod lists, in devpod's order. @@ -1206,6 +1229,7 @@ fn enriched_row( clone, disk: DiskField::of(sizes, measurable.as_deref()), sweep, + stale_image: None, } } @@ -1371,6 +1395,16 @@ fn json_row(row: &ListedWorkspace) -> serde_json::Value { if let Some(note) = &row.sweep { insert(&mut value, "lastSweep", sweep_as_json(note)); } + // Appended, and absent unless stale, for the reason `lastSweep` is: a + // machine with nothing stale prints the document it printed before this key + // existed. + if let Some(stale) = &row.stale_image { + insert( + &mut value, + "staleImage", + serde_json::Value::String(stale.reference().to_owned()), + ); + } value } @@ -2712,6 +2746,35 @@ mod tests { assert!(outstanding_sweep_notes(&scene.storage).is_empty()); } + #[test] + fn a_stale_row_names_its_reference_last_and_the_others_say_nothing() { + // devlaunch#673. Appended after every key `wf` already reads, and absent + // on a current row, so a machine with nothing stale prints the golden. + let scene = Scene::build(); + let mut rows = scene.rows(Sizes::Skip); + let stale = StaleImages::of_pairs([("someone-elses", "ghcr.io/o/img:latest")]); + let clean = rows + .iter_mut() + .find(|row| row.id == scene.id("clean")) + .expect("a clean row"); + clean.stale_image = StaleImages::of_pairs([("clean", "ghcr.io/o/img:old")]) + .of("clean") + .cloned(); + + mark_stale_images(&mut rows, &stale); + let document = json_document(&rows); + let rows = rows_by_id(&document); + + let row = rows["someone-elses"].as_object().expect("an object"); + assert_eq!( + row.iter() + .next_back() + .map(|(key, value)| (key.as_str(), value)), + Some(("staleImage", &serde_json::json!("ghcr.io/o/img:latest"))) + ); + assert!(rows[scene.id("clean")].get("staleImage").is_none()); + } + #[test] fn a_note_with_nothing_quoted_still_says_which_trouble_it_was() { // `said: null` rather than an absent key or an empty string: a fetch killed diff --git a/rust/devlaunch-core/src/flows/mod.rs b/rust/devlaunch-core/src/flows/mod.rs index 6f49cd58..3e2bf87f 100644 --- a/rust/devlaunch-core/src/flows/mod.rs +++ b/rust/devlaunch-core/src/flows/mod.rs @@ -43,6 +43,8 @@ pub mod repo_manager; // it is one decision and no socket. pub mod session_manager; // binary surface — not part of the frozen wf API (#251 §7) +pub mod stale_images; +// binary surface — not part of the frozen wf API (#251 §7) pub mod workspace_clone; // Binary surface, outside the frozen workspace API. diff --git a/rust/devlaunch-core/src/flows/stale_images.rs b/rust/devlaunch-core/src/flows/stale_images.rs new file mode 100644 index 00000000..55bda8b1 --- /dev/null +++ b/rust/devlaunch-core/src/flows/stale_images.rs @@ -0,0 +1,606 @@ +//! Which workspaces run an older image than the reference they were made from +//! now names (devlaunch#673). +//! +//! A container keeps the image it was created from. A `docker pull` of the same +//! reference, by hand or by a daily job, moves the reference and leaves every +//! running container where it was; only the next create picks the new image up. +//! Nothing told anybody, so a long-lived workspace drifted days behind and, for a +//! repository that keys a remote build cache on what the image holds, built its +//! whole tree locally. +//! +//! # What "stale" means here +//! +//! devpod writes the facts down on the way out of every completed `up`, in +//! `workspace_result.json`: the container's id (`ContainerDetails.Id`), the image +//! reference the container was created from (`ContainerDetails.Config.Image`), +//! and the reference the devcontainer *declared* (`MergedConfig.image`), where it +//! declared one. Those two references are not always the same image. devpod builds +//! a derived image on top of the declared one when it has features to install or +//! a user id to remap, and creates the container from that. +//! +//! So there are two cases, and docker answers both: +//! +//! - **The container was created from the declared reference itself.** It is +//! stale when that reference now names an image other than the one the +//! container runs. The two ids say so. +//! - **The container was created from an image devpod derived.** The derived tag +//! never moves on a pull, so its id says nothing. The declared reference's +//! layers do: a derived image starts with every layer of the image it was built +//! from, so the container is stale when the declared reference's current layers +//! are no longer the start of the running image's layers **and** the declared +//! image was created after the running one. Layers that differ only say +//! "different". The creation time is what says "older": a prebuilt derived image +//! pulled from a registry can sit on a newer base than this machine's copy of +//! the declared reference, and a recreate would pull the same prebuild again, +//! so without the time the note would never clear. +//! +//! A rebuild of the declared image that changes only its configuration (an `ENV` +//! or a `LABEL`) keeps every layer, so a derived container reads as current +//! through it. That is a miss, not a false alarm. +//! +//! A devcontainer that declares no image (a Dockerfile build, or a compose file +//! whose service names it) falls to the first case with the reference the +//! container was created from. A tag devpod derived from a compose service never +//! moves, so that case reads as current. It is a miss, not a false alarm. +//! +//! # Every doubt reads as current +//! +//! No result file, a container docker no longer has, a reference this machine +//! cannot inspect, a docker that will not answer: each leaves the workspace out +//! of the answer. The only thing the answer drives is a suggestion to recreate, +//! and a recreate costs the processes in the container. A workspace this cannot +//! read about is one it has no grounds to ask anybody to recreate. + +use std::collections::{BTreeMap, BTreeSet}; + +use crate::clients::devpod_home::{DevpodHome, sole_workspace_result}; +use crate::clients::docker; +use crate::domain::workspace_state::NonEmpty; +use crate::runner::Runner; + +/// One workspace whose container runs an older image than its reference names. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct StaleImage { + reference: String, +} + +impl StaleImage { + /// The reference that now names a newer image: the one the devcontainer + /// declared, or the one the container was created from where it declared + /// none. + pub fn reference(&self) -> &str { + &self.reference + } +} + +/// The stale workspaces among the ones asked about, by workspace id. +/// +/// A workspace that is current and a workspace this could not read about are +/// both absent; see the module documentation for why they are one answer. +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub struct StaleImages { + by_workspace: BTreeMap, +} + +impl StaleImages { + /// What is stale about this workspace, if anything is. + pub fn of(&self, workspace_id: &str) -> Option<&StaleImage> { + self.by_workspace.get(workspace_id) + } + + pub fn is_empty(&self) -> bool { + self.by_workspace.is_empty() + } + + /// The answer these pairs of workspace id and reference would make, for a + /// test that needs one without a docker to read it from. + #[cfg(test)] + pub(crate) fn of_pairs<'a>(pairs: impl IntoIterator) -> Self { + Self { + by_workspace: pairs + .into_iter() + .map(|(id, reference)| { + ( + id.to_owned(), + StaleImage { + reference: reference.to_owned(), + }, + ) + }) + .collect(), + } + } +} + +/// What devpod recorded about one workspace's container. +#[derive(Clone, Debug, PartialEq, Eq)] +struct Created { + container: String, + /// `ContainerDetails.Config.Image`. + created_from: String, + /// `MergedConfig.image`, or `created_from` where the devcontainer declared + /// no image. + reference: String, +} + +/// One image as docker describes it: its id, when it was created, and its +/// layers, bottom first. +#[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, + layers: Vec, +} + +/// The stale workspaces among these. +/// +/// Costs one read of each workspace's result file, then at most two batched +/// `docker inspect` calls plus one per distinct reference. One per reference and +/// not one for all of them, because docker's answer for an image carries its id +/// and not the name it was asked by, and two references can spell one image. +/// Machines with many workspaces of one repository share one reference, so the +/// count is small in practice. +pub fn stale_images<'w>( + runner: &dyn Runner, + devpod_home: Option<&DevpodHome>, + workspace_ids: impl IntoIterator, +) -> StaleImages { + let created: Vec<(&str, Created)> = workspace_ids + .into_iter() + .filter_map(|id| Some((id, created(devpod_home, id)?))) + .collect(); + let Some(containers) = NonEmpty::of(unique(created.iter().map(|(_, c)| &c.container))) else { + return StaleImages::default(); + }; + let running = running_images(runner, &containers); + let running_images = images(runner, unique(running.values())); + let references: BTreeMap = unique(created.iter().map(|(_, c)| &c.reference)) + .into_iter() + .filter_map(|reference| { + let image = images(runner, [reference.clone()]).into_values().next()?; + Some((reference, image)) + }) + .collect(); + + let by_workspace = created + .into_iter() + .filter_map(|(id, created)| { + let running = running_images.get(running.get(&created.container)?)?; + let current = references.get(&created.reference)?; + is_stale(&created, running, current).then(|| { + ( + id.to_owned(), + StaleImage { + reference: created.reference, + }, + ) + }) + }) + .collect(); + StaleImages { by_workspace } +} + +/// Whether a container created as `created` and running `running` is behind +/// the image its reference names now, `current`. +fn is_stale(created: &Created, running: &Image, current: &Image) -> bool { + if running.id == current.id { + return false; + } + if created.created_from == created.reference { + return true; + } + !running.layers.starts_with(¤t.layers) && current.created > running.created +} + +/// What devpod's result file says about this workspace's container, if there is +/// exactly one result file and it names a container and an image. +fn created(devpod_home: Option<&DevpodHome>, workspace_id: &str) -> Option { + let path = sole_workspace_result(devpod_home, workspace_id)?; + let document: serde_json::Value = serde_json::from_slice(&std::fs::read(path).ok()?).ok()?; + let details = &document["ContainerDetails"]; + let created_from = text(&details["Config"]["Image"])?; + Some(Created { + container: text(&details["Id"])?, + reference: text(&document["MergedConfig"]["image"]).unwrap_or_else(|| created_from.clone()), + created_from, + }) +} + +fn text(value: &serde_json::Value) -> Option { + value + .as_str() + .filter(|text| !text.is_empty()) + .map(str::to_owned) +} + +fn unique<'a>(items: impl Iterator) -> BTreeSet { + items.cloned().collect() +} + +/// The image id each of these containers runs, by container id. +fn running_images(runner: &dyn Runner, containers: &NonEmpty) -> BTreeMap { + let Some(printed) = + docker::inspect_formatted(runner, "container", "{{.Id}} {{.Image}}", containers) + else { + return BTreeMap::new(); + }; + printed + .lines() + .filter_map(|line| { + let (container, image) = line.trim().split_once(' ')?; + Some((container.to_owned(), image.to_owned())) + }) + .collect() +} + +/// Each of these images that docker has, by image id. +fn images(runner: &dyn Runner, names: impl IntoIterator) -> BTreeMap { + let Some(names) = NonEmpty::of(names) else { + return BTreeMap::new(); + }; + let Some(printed) = docker::inspect_formatted( + runner, + "image", + "{{.Id}} {{.Created}} {{json .RootFS.Layers}}", + &names, + ) else { + return BTreeMap::new(); + }; + printed + .lines() + .filter_map(|line| { + let (id, rest) = line.trim().split_once(' ')?; + let (created, layers) = rest.split_once(' ')?; + let layers: Vec = serde_json::from_str(layers).ok()?; + Some(( + id.to_owned(), + Image { + id: id.to_owned(), + created: created.get(..19)?.to_owned(), + layers, + }, + )) + }) + .collect() +} + +#[cfg(test)] +mod tests { + use devlaunch_test_support::{FakeRunner, Response}; + + use super::*; + use crate::clients::devpod_home::{ScratchHome, devpod_home_with}; + + const CONTAINER_FORMAT: &str = "{{.Id}} {{.Image}}"; + const IMAGE_FORMAT: &str = "{{.Id}} {{.Created}} {{json .RootFS.Layers}}"; + + /// A devpod home whose one workspace was created from `created_from`, with + /// `declared` as the devcontainer's image where it declared one. + fn home_with(workspace_id: &str, created_from: &str, declared: Option<&str>) -> ScratchHome { + let home = devpod_home_with(&[("default", workspace_id, Some(()))]); + record_create(&home, workspace_id, "c1", created_from, declared); + home + } + + fn record_create( + home: &ScratchHome, + workspace_id: &str, + container: &str, + created_from: &str, + declared: Option<&str>, + ) { + let merged = match declared { + Some(image) => serde_json::json!({ "image": image }), + None => serde_json::json!({}), + }; + std::fs::write( + home.result("default", workspace_id), + serde_json::json!({ + "ContainerDetails": { "Id": container, "Config": { "Image": created_from } }, + "MergedConfig": merged, + }) + .to_string(), + ) + .expect("a create result"); + } + + fn line(id: &str, layers: &[&str]) -> String { + line_at(id, "2026-10-01T00:00:00.000000000Z", layers) + } + + fn line_at(id: &str, created: &str, layers: &[&str]) -> String { + format!( + "{id} {created} {}\n", + serde_json::to_string(layers).expect("layers") + ) + } + + fn inspect_container(fake: &FakeRunner, running: &str) { + fake.script( + [ + "docker", + "inspect", + "--type", + "container", + "--format", + CONTAINER_FORMAT, + ], + Response::stdout(format!("c1 {running}\n")), + ); + } + + fn inspect_image(fake: &FakeRunner, name: &str, printed: String) { + fake.script( + [ + "docker", + "inspect", + "--type", + "image", + "--format", + IMAGE_FORMAT, + name, + ], + Response::stdout(printed), + ); + } + + #[test] + fn a_container_made_from_the_reference_is_stale_once_the_reference_moves() { + let home = home_with("ws", "ghcr.io/o/img:latest", Some("ghcr.io/o/img:latest")); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:old"); + inspect_image(&fake, "sha256:old", line("sha256:old", &["l1"])); + inspect_image( + &fake, + "ghcr.io/o/img:latest", + line("sha256:new", &["l1", "l2"]), + ); + + let stale = stale_images(&fake, Some(&home), ["ws"]); + + assert_eq!( + stale.of("ws").map(StaleImage::reference), + Some("ghcr.io/o/img:latest") + ); + } + + #[test] + fn a_container_made_from_the_reference_is_current_while_the_reference_holds() { + let home = home_with("ws", "ghcr.io/o/img:latest", None); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:same"); + inspect_image(&fake, "sha256:same", line("sha256:same", &["l1"])); + inspect_image(&fake, "ghcr.io/o/img:latest", line("sha256:same", &["l1"])); + + assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); + } + + #[test] + fn a_derived_image_is_current_while_it_starts_with_the_references_layers() { + // devpod built `devpod-abc` on top of the declared image, so the two ids + // always differ and only the layers can say the base is the same. + 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("sha256:derived", &["l1", "l2", "f1"]), + ); + inspect_image( + &fake, + "ghcr.io/o/img:latest", + line("sha256:base", &["l1", "l2"]), + ); + + assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); + } + + #[test] + fn a_derived_image_is_stale_once_the_reference_has_layers_it_lacks() { + 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("sha256:derived", &["l1", "l2", "f1"]), + ); + inspect_image( + &fake, + "ghcr.io/o/img:latest", + line_at("sha256:base2", "2026-10-05T09:00:00Z", &["l1", "l3"]), + ); + + let stale = stale_images(&fake, Some(&home), ["ws"]); + + assert_eq!( + stale.of("ws").map(StaleImage::reference), + Some("ghcr.io/o/img:latest") + ); + } + + #[test] + fn a_derived_image_is_stale_when_the_references_layers_are_not_its_prefix() { + 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("sha256:derived", &["l1", "l2", "f1"]), + ); + inspect_image( + &fake, + "ghcr.io/o/img:latest", + line_at("sha256:base2", "2026-10-05T09:00:00Z", &["l1", "f1"]), + ); + + let stale = stale_images(&fake, Some(&home), ["ws"]); + + assert_eq!( + stale.of("ws").map(StaleImage::reference), + Some("ghcr.io/o/img:latest") + ); + } + + #[test] + fn a_prebuilt_image_on_a_newer_base_than_the_local_reference_is_current() { + // The derived image came from a registry, built on a base this machine + // has not pulled yet. The layers differ, but the running image is the + // newer one, and a recreate would pull the same prebuild again. + let home = home_with( + "ws", + "ghcr.io/o/img-devcontainer:abc", + Some("ghcr.io/o/img:latest"), + ); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:prebuilt"); + inspect_image( + &fake, + "sha256:prebuilt", + line_at( + "sha256:prebuilt", + "2026-10-06T08:00:00.5Z", + &["l1", "l9", "f1"], + ), + ); + inspect_image( + &fake, + "ghcr.io/o/img:latest", + line_at("sha256:base", "2026-10-01T00:00:00Z", &["l1", "l2"]), + ); + + assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); + } + + #[test] + fn each_workspace_is_judged_by_its_own_container() { + let home = + devpod_home_with(&[("default", "ws-a", Some(())), ("default", "ws-b", Some(()))]); + let reference = "ghcr.io/o/img:latest"; + record_create(&home, "ws-a", "ca", reference, Some(reference)); + record_create(&home, "ws-b", "cb", reference, Some(reference)); + let fake = FakeRunner::new(); + fake.script( + [ + "docker", + "inspect", + "--type", + "container", + "--format", + CONTAINER_FORMAT, + ], + Response::stdout("ca sha256:old\ncb sha256:new\n"), + ); + fake.script( + [ + "docker", + "inspect", + "--type", + "image", + "--format", + IMAGE_FORMAT, + "sha256:new", + "sha256:old", + ], + Response::stdout(format!( + "{}{}", + line("sha256:new", &["l2"]), + line("sha256:old", &["l1"]) + )), + ); + inspect_image(&fake, reference, line("sha256:new", &["l2"])); + + let stale = stale_images(&fake, Some(&home), ["ws-a", "ws-b"]); + + assert_eq!(stale.of("ws-a").map(StaleImage::reference), Some(reference)); + assert_eq!(stale.of("ws-b"), None); + let docker = fake.args_to("docker"); + let containers: Vec<_> = docker + .iter() + .filter(|args| args.contains(&"container".to_owned())) + .collect(); + assert_eq!(containers.len(), 1); + assert!(containers[0].ends_with(&["ca".to_owned(), "cb".to_owned()])); + let reference_inspects = docker + .iter() + .filter(|args| args.last().map(String::as_str) == Some(reference)) + .count(); + assert_eq!(reference_inspects, 1); + } + + #[test] + fn a_reference_docker_cannot_inspect_reads_as_current() { + // `docker inspect` over a reference nobody pulled prints nothing and + // exits 1. That is no grounds to ask for a recreate. + let home = home_with("ws", "ghcr.io/o/img:latest", None); + let fake = FakeRunner::new(); + inspect_container(&fake, "sha256:old"); + inspect_image(&fake, "sha256:old", line("sha256:old", &["l1"])); + fake.script( + [ + "docker", + "inspect", + "--type", + "image", + "--format", + IMAGE_FORMAT, + "ghcr.io/o/img:latest", + ], + Response::failed(1, "Error: No such image"), + ); + + assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); + } + + #[test] + fn a_container_docker_no_longer_has_reads_as_current() { + let home = home_with("ws", "ghcr.io/o/img:latest", None); + let fake = FakeRunner::new(); + fake.script( + ["docker", "inspect", "--type", "container"], + Response::failed(1, "Error: No such container: c1"), + ); + inspect_image(&fake, "ghcr.io/o/img:latest", line("sha256:new", &["l1"])); + + assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); + } + + #[test] + fn a_missing_container_costs_the_others_nothing() { + // docker prints a line for every container it found and exits 1 for the + // one it did not. The line it printed still counts. + let home = home_with("ws", "ghcr.io/o/img:latest", None); + let fake = FakeRunner::new(); + fake.script( + ["docker", "inspect", "--type", "container"], + Response::failed(1, "Error: No such container: gone").and_stdout("c1 sha256:old\n"), + ); + inspect_image(&fake, "sha256:old", line("sha256:old", &["l1"])); + inspect_image(&fake, "ghcr.io/o/img:latest", line("sha256:new", &["l2"])); + + assert!(stale_images(&fake, Some(&home), ["ws"]).of("ws").is_some()); + } + + #[test] + fn no_docker_reads_as_nothing_stale() { + let home = home_with("ws", "ghcr.io/o/img:latest", None); + let fake = FakeRunner::new(); + fake.script_missing("docker"); + + assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); + } + + #[test] + fn a_workspace_with_no_result_asks_docker_nothing() { + let home = devpod_home_with(&[("default", "ws", None)]); + let fake = FakeRunner::new(); + + assert!(stale_images(&fake, Some(&home), ["ws"]).is_empty()); + assert!(fake.argvs().is_empty()); + } +} diff --git a/rust/dl/src/commands.rs b/rust/dl/src/commands.rs index 69b49b2a..3b48e83a 100644 --- a/rust/dl/src/commands.rs +++ b/rust/dl/src/commands.rs @@ -32,6 +32,7 @@ use devlaunch_core::flows::pull_request; use devlaunch_core::flows::records::{Records, StartupError, open_records, open_storage}; use devlaunch_core::flows::repo_manager::CacheNotice; use devlaunch_core::flows::session_manager::{self, PaneDestination}; +use devlaunch_core::flows::stale_images::stale_images; use devlaunch_core::osext; use devlaunch_core::runner::{Exit, Runner}; @@ -313,7 +314,7 @@ fn render_list( sizes: Sizes, ) -> Ending { match output { - ListOutput::Table => render_table(context, cache, sizes), + ListOutput::Table => render_table(runner, context, cache, sizes), ListOutput::Json => render_json(runner, context, cache, sizes), } } @@ -325,7 +326,17 @@ fn render_list( /// under the table. That read is the point of devlaunch#480: the sweep is a /// detached child whose stderr is `/dev/null`, so the record is the only place a /// complaint of its can be left, and `--ls` is the only surface anybody reads. -fn render_table(context: &mut CommandContext<'_>, cache: &Path, sizes: Sizes) -> Ending { +/// +/// It also asks docker which containers run an older image than their reference +/// names (devlaunch#673), and says so under the table rather than in a column: +/// the column widths are a golden, and a stale image is a thing to act on, not a +/// property to scan for. +fn render_table( + runner: &dyn Runner, + context: &mut CommandContext<'_>, + cache: &Path, + sizes: Sizes, +) -> Ending { let table = match listing::workspace_table(context, cache, sizes) { Err(refused) => return refuse_listing(&refused), Ok(table) => table, @@ -333,6 +344,17 @@ fn render_table(context: &mut CommandContext<'_>, cache: &Path, sizes: Sizes) -> for line in render::table_lines(&table, sizes) { println!("{line}"); } + if let listing::WorkspaceTable::Rows(rows) = &table { + let ids: Vec<&str> = rows.iter().map(|row| row.id.as_str()).collect(); + let stale = stale_images(runner, DevpodHome::locate().as_ref(), ids.iter().copied()); + let notes = render::stale_image_notes( + ids.iter() + .filter_map(|id| Some((*id, stale.of(id)?.reference()))), + ); + for line in notes { + eprintln!("{line}"); + } + } say_sweep_notes(); Ending::Done } @@ -385,7 +407,13 @@ fn render_json( say_cache(directories.take_notices()); match listed { Err(refused) => refuse_listing(&refused), - Ok(rows) => { + Ok(mut rows) => { + let stale = stale_images( + runner, + DevpodHome::locate().as_ref(), + rows.iter().map(listing::ListedWorkspace::id), + ); + listing::mark_stale_images(&mut rows, &stale); println!( "{}", render::python_json_document(&listing::json_document(&rows)) diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index 1c1030b9..aa5f95d6 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -302,6 +302,25 @@ fn shared_account_notes(rows: &[claude_profiles::ProfileSummary]) -> Vec lines } +/// The lines `dl --ls` writes under its table for the workspaces whose container +/// runs an older image than its reference now names (devlaunch#673). +/// +/// One line per workspace, in the table's order, and each names the verb that +/// moves it: a pull moves the reference and nothing else, so a person who reads +/// the line has to be told that the container did not follow. +pub(crate) fn stale_image_notes<'a>( + stale: impl IntoIterator, +) -> Vec { + stale + .into_iter() + .map(|(id, reference)| { + format!( + "{id} runs an older image than {reference} now names. Run `dl {id} recreate` to move it." + ) + }) + .collect() +} + /// The lines `dl --ls` writes under its table for the repositories whose last /// background sweep left something to say. /// @@ -5964,6 +5983,23 @@ mod tests { assert!(sweep_notes(&[]).is_empty(), "a clean cache says nothing"); } + #[test] + fn each_stale_workspace_gets_one_line_in_the_order_given() { + assert_eq!( + stale_image_notes([ + ("zeta-main-1", "ghcr.io/o/img:latest"), + ("alpha-main-2", "ubuntu:24.04"), + ]), + [ + "zeta-main-1 runs an older image than ghcr.io/o/img:latest now names. \ + Run `dl zeta-main-1 recreate` to move it.", + "alpha-main-2 runs an older image than ubuntu:24.04 now names. \ + Run `dl alpha-main-2 recreate` to move it.", + ] + ); + assert!(stale_image_notes([]).is_empty()); + } + #[test] fn every_sweep_trouble_has_words_of_its_own() { // Five arms, five sentences, and no two the same: which condition it was is diff --git a/rust/dl/tests/read_side.rs b/rust/dl/tests/read_side.rs index 6f5a1a5a..bc7b4b0a 100644 --- a/rust/dl/tests/read_side.rs +++ b/rust/dl/tests/read_side.rs @@ -390,6 +390,86 @@ fn normalize_bytes(document: &str) -> String { .join("\n") } +// =========================================================================== +// --ls and a container behind its image reference (devlaunch#673) +// =========================================================================== + +const STALE_ID: &str = "blooop-devlaunch-main-4f3a2b1c"; +const STALE_REFERENCE: &str = "ghcr.io/blooop/devlaunch-devcontainer:latest"; + +/// The full scenario, with the first workspace's container created from +/// [`STALE_REFERENCE`] and a docker that says the reference has since moved. +/// +/// The container was created from the reference itself, so differing image ids +/// are the whole of the evidence; the time and the layers are there only because +/// the format asks for them. +fn world_with_a_stale_workspace() -> World { + let world = World::full(); + let records = world.path(&format!("devpod/contexts/default/workspaces/{STALE_ID}")); + std::fs::create_dir_all(&records).expect("a devpod record directory"); + std::fs::write( + records.join("workspace.json"), + format!(r#"{{"id": "{STALE_ID}"}}"#), + ) + .expect("a devpod record"); + std::fs::write( + records.join("workspace_result.json"), + format!( + r#"{{"ContainerDetails": {{"Id": "c1", "Config": {{"Image": "{STALE_REFERENCE}"}}}}, "MergedConfig": {{}}}}"# + ), + ) + .expect("a create result"); + let docker = world.path("bin/docker"); + std::fs::write( + &docker, + format!( + r#"#!/bin/sh +[ "$1" = inspect ] || exit 1 +case "$3 $6" in +"container c1") echo "c1 sha256:old" ;; +"image sha256:old") echo 'sha256:old 2026-10-01T00:00:00Z ["sha256:l1"]' ;; +"image {STALE_REFERENCE}") echo 'sha256:new 2026-10-05T00:00:00.123Z ["sha256:l2"]' ;; +*) exit 1 ;; +esac +"# + ), + ) + .expect("a fake docker"); + make_executable(&docker); + world +} + +#[test] +fn a_stale_workspace_is_named_under_the_table_and_the_table_is_untouched() { + let world = world_with_a_stale_workspace(); + let run = world.dl(&["--ls"]); + run.succeeded(); + assert_eq!(run.out, LS_TABLE); + assert_eq!( + run.err, + format!( + "{STALE_ID} runs an older image than {STALE_REFERENCE} now names. \ + Run `dl {STALE_ID} recreate` to move it.\n" + ) + ); +} + +#[test] +fn a_stale_workspace_carries_its_reference_last_in_its_json_row() { + let world = world_with_a_stale_workspace(); + let run = world.dl(&["--ls", "--json"]); + run.succeeded(); + let first_row_end = " \"nothingToLose\": true\n }\n }"; + assert_eq!(LS_JSON.matches(first_row_end).count(), 1); + let expected = LS_JSON.replace( + first_row_end, + &format!( + " \"nothingToLose\": true\n }},\n \"staleImage\": \"{STALE_REFERENCE}\"\n }}" + ), + ); + assert_eq!(run.out, expected); +} + // =========================================================================== // the completion commands // ===========================================================================