From 3ceddeab6d5ad046d6de4c3868c319a1c8dd7871 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sat, 29 Aug 2026 18:58:32 +0100 Subject: [PATCH 1/3] feat: keep dl's own copy of the volume names, and reclaim from it A workspace deleted by a bare `devpod delete` outside `dl` leaves its two named volumes standing and takes the only record of their names with it. devlaunch#325 reads those names out of devpod's `workspace_result.json` at delete time, which closed the leak for every delete that goes through `dl` and did nothing for the ones that do not. So `dl` now takes a second read of the same document at the tail of every completed `up`, before the provisioning pass, and keeps it as a per workspace file under its own cache beside the tool verdict markers. `dl --prune` reclaims from those copies: the domain is the set of copies rather than a clone walk, and the one precondition per copy is that no workspace `devpod list` returns carries that id. Nothing is synthesized. Every name that reaches `docker volume rm` still came out of a substitution devpod performed and wrote down, at whichever of the two reads, and `tests/volume_names.rs` holds the crate to that by construction: one module builds a name, one call site removes one. A copy can be wrong in exactly two ways and neither is caught by trust. An already gone volume is a `--force` no-op; a held one is a docker refusal, reported, with the copy kept so the retry survives. A copy is dropped once, on proof: a removal that came back removed for a workspace devpod does not list. The 39 orphaned volumes measured on the reference host stay. No route reaches them that is not the pattern devlaunch#451 refuses. Closes #456. --- docs/cleanup.md | 56 +- docs/development.md | 8 + rust/devlaunch-core/src/flows/kept_copies.rs | 655 +++++++++++++++++ rust/devlaunch-core/src/flows/launch.rs | 257 +++++++ rust/devlaunch-core/src/flows/lifecycle.rs | 736 +++++++++++++++---- rust/devlaunch-core/src/flows/mod.rs | 2 + rust/devlaunch-core/tests/volume_names.rs | 134 ++++ rust/dl/src/commands.rs | 17 +- rust/dl/src/render.rs | 82 ++- 9 files changed, 1805 insertions(+), 142 deletions(-) create mode 100644 rust/devlaunch-core/src/flows/kept_copies.rs create mode 100644 rust/devlaunch-core/tests/volume_names.rs diff --git a/docs/cleanup.md b/docs/cleanup.md index 3f8bd90a..7fd09ff0 100644 --- a/docs/cleanup.md +++ b/docs/cleanup.md @@ -55,6 +55,48 @@ be made worse by it: frees](#the-disk-neither-command-frees). That boundary is about images now, and deliberately stays there. +### The volumes of a workspace devpod has already forgotten + +The read above happens at delete time, immediately before `devpod delete` takes +devpod's record away with the workspace. That closes the leak for every delete +that goes through `dl`. It does nothing for a workspace deleted some other way: a +bare `devpod delete`, or a devpod home that was cleared out. devpod removes its +own record and leaves the volumes, and after that there is nothing on the machine +that names them. + +So `dl` keeps its own copy. At the end of every `up` that completed, it reads the +same two fields out of the same `workspace_result.json` and writes them to a small +per workspace file under its cache, beside the tool verdict markers: +`~/.cache/devlaunch/workspace-copies/.json`. `dl --prune` reclaims from +those copies, and the whole of what it asks per copy is: does any workspace +`devpod list` returns carry that id? Where none does, the workspace is gone and +its volumes are leftovers. + +Nothing is invented at either read. Every name `dl` hands `docker volume rm` still +came out of a substitution devpod performed and wrote down, which is the rule +above and is unchanged. The alternative, matching `-pixi` and +`dind-var-lib-docker-` against `docker volume ls`, stays refused: the +`docker-in-docker` feature writes that second name in every devcontainer tool that +runs it, so the candidates `dl` cannot attribute are exactly the ones that belong +to somebody else, and a volume is not an image. The wrong answer there is data +loss, not a rebuild. + +A copy can still be wrong, and it can be wrong in exactly two ways. It can name a +volume that is already gone, which `docker volume rm --force` treats as a success +and says nothing about. It can name a volume something else now holds, which +Docker refuses: `volume is in use`, reported, nothing removed, and the copy kept so +the retry is still there. Neither is caught by trusting the file. A copy is dropped +once, when a removal came back removed for a workspace devpod does not list, which +is the one moment it is provably pointless. + +Two consequences worth knowing. A run pointed at a scratch cache +(`XDG_CACHE_HOME=...`) finds no copies at all, so it names no volume and removes +none, which is what makes the scratch convention safe here by construction. And the +**39 orphaned volumes measured on the reference host are out of scope**: their +records died before any of this existed, no route reaches them that is not the +pattern above, and they stay. See [the disk neither command +frees](#the-disk-neither-command-frees). + ### What a delete says while it does it `dl rm` names the workspace going in and again once it has gone, both on @@ -224,8 +266,9 @@ until now nothing removed them: measured on one host, **52 clone directories for all-or-nothing: the only way to get the 4 GB back was to destroy the 7.86 GB too, and every bare cache with it. -`dl --prune` removes exactly the clone directories no live workspace opens. It -never deletes a devpod workspace, a container, an image or a volume, never +`dl --prune` removes exactly the clone directories no live workspace opens, and +reclaims the Docker volumes of workspaces devpod no longer lists. It never +deletes a devpod workspace, a container or an image, never touches a repo's `.bare` cache (0.08 GB for seven repos, and it is what makes the next clone of a repo fast), and never looks outside `/devlaunch/repos`. Every directory it finds is one of three things: @@ -331,10 +374,11 @@ exception: that is where somebody is deciding what is worth deleting. (devlaunch#325). Deleting a workspace now removes the named volumes its devcontainer created, see [what a delete takes with it](#what-a-delete-takes-with-it), so a disclaimer that still covered them would -be describing a leak that has been fixed. The `--prune` half of the pair still -frees no volume at all, and that is not an oversight either: it removes clone -*directories* and never deletes a workspace, so there is no workspace whose -volumes it could be taking. +be describing a leak that has been fixed. The `--prune` half of the pair frees +volumes too, and its reason changed rather than disappeared: it still never +deletes a workspace, it reclaims the volumes of one devpod has already forgotten, +from [the copy `dl` keeps](#the-volumes-of-a-workspace-devpod-has-already-forgotten). +The 39 orphans above are not among them. Nothing reaches those but a pattern. **It is a sentence, not a measurement.** `dl` runs no `docker` command to print it, so there is nothing to be slow and nothing to fail where Docker is absent, diff --git a/docs/development.md b/docs/development.md index 54cd9536..0a08eacf 100644 --- a/docs/development.md +++ b/docs/development.md @@ -149,6 +149,14 @@ working tree; on a host, `./dev.sh` installs it as `dl-next`/`aid-next` beside t print a `-dev` version so a working-tree build is never mistaken for a released one. See [AGENTS.md](../AGENTS.md). +One thing to know about pointing a run at a scratch cache (`XDG_CACHE_HOME=/tmp/...`), which +AGENTS.md recommends for anything near storage. A run that *creates* a workspace writes its copy +of that workspace's volume names into the cache it is about to throw away, and that copy is the +only record of those names once devpod's own record goes. Remove such workspaces with `dl rm` +before discarding the scratch cache, or their volumes outlive everything that could name them. See +[the volumes of a workspace devpod has already +forgotten](cleanup.md#the-volumes-of-a-workspace-devpod-has-already-forgotten). + ### The Quickstart's demo GIFs The four GIFs in the Quickstart come from VHS tapes in `docs/demo/`: diff --git a/rust/devlaunch-core/src/flows/kept_copies.rs b/rust/devlaunch-core/src/flows/kept_copies.rs new file mode 100644 index 00000000..45a65e2d --- /dev/null +++ b/rust/devlaunch-core/src/flows/kept_copies.rs @@ -0,0 +1,655 @@ +//! devlaunch's own copy of what devpod substituted into a workspace, kept so the +//! volumes of a workspace devpod has forgotten are still reclaimable by *reading*. +//! +//! Both volume names come from one file: devpod's `workspace_result.json`, whose +//! `SubstitutionContext` records what `${localWorkspaceFolderBasename}` and +//! `${devcontainerId}` expanded to. `flows::lifecycle` reads it at delete time, +//! immediately before `devpod delete` destroys it. That read closed the leak going +//! forward (devlaunch#325) and left one population behind: a workspace deleted by a +//! bare `devpod delete` outside `dl` takes the record away and leaves the volumes, +//! and after that there is nothing on the machine that names them. +//! +//! So this module takes a **second read of the same document**, at the tail of a +//! completed `up`, and keeps the answer. Nothing is synthesized: every name dl ever +//! hands `docker volume rm` still originates in a substitution devpod performed and +//! wrote down. A pattern over `docker volume ls` would name volumes *nobody ever +//! recorded*, which is somebody else's disk, and devlaunch#451 refuses it as a +//! design rather than deferring it. +//! +//! # A copy can be wrong in exactly two ways, and neither is caught by trust +//! +//! It can name a volume that is already gone, which `docker volume rm --force` +//! makes a silent success. It can name a volume something else now holds, which +//! docker refuses: measured on docker 29.7.2, a volume held by a container in state +//! `exited` exits 1 with `volume is in use`. Both are answered by something other +//! than believing the copy. What a copy cannot do is the thing a pattern does. +//! +//! The second guard is where the copies are *consulted*, not here: only for a +//! workspace no `devpod list` names. A devpod home copied between machines is the +//! dishonest case, and [`super::provision::verdict_cache`]'s mtime-equality anchor +//! is unavailable to it — the anchor dies with the workspace, which is the whole +//! occasion this exists for. +//! +//! # Its own marker, beside the tool verdicts rather than inside them +//! +//! Sibling to [`super::provision::verdict_cache`]'s markers and modelled on them, +//! down to the atomic write. Not a field on one, because that marker's *presence* +//! means `AlreadyProvisioned` and a copy has to be written on every completed `up` +//! whatever the verdict; one file with two write conditions makes one condition +//! govern the other. Not a `metadata.json` field either: that file is the pinned +//! shared format with a schema version, and its worktree records are what `--prune` +//! *drops* — the record would be destroyed by the command best placed to use it. +//! +//! # Where this diverges from the verdict cache: a copy is dropped +//! +//! "Nothing ever deletes a marker" is that module's rule, and the reason it holds +//! there is that a delete would be a second unproven mechanism saying what the +//! mtime comparison already says. Here there is no comparison to make: a copy is +//! about a workspace that may be gone, and the proof arrives from docker. So +//! [`KeptCopies::forget`] is called on exactly one occasion — a removal that came +//! back removed, for a workspace devpod does not list — and never on a refusal, so +//! the retry survives. + +use std::path::{Path, PathBuf}; + +use serde::{Deserialize, Serialize}; + +use crate::clients::devpod_home::{DevpodHome, sole_workspace_result}; +use crate::domain::workspace_state::NonEmpty; + +/// The directory the copies live in, under devlaunch's cache directory. +/// +/// Beside `tool-verdicts`, deliberately: two per-workspace stores with two write +/// conditions, and a reader of either can see at a glance which it is holding. +const COPIES_DIR: &str = "workspace-copies"; + +/// Where devlaunch keeps its copies of what devpod substituted. +/// +/// A path parameter rather than a read of the process environment, for +/// [`super::provision::verdict_cache::VerdictCache`]'s reason: the binary resolves +/// the cache directory once and hands it down, and a store that resolved its own +/// could disagree with the one the launch already resolved. It is also what makes +/// the map's hard constraint hold by construction — a run pointed at a scratch +/// `XDG_CACHE_HOME` finds no copies at all, so it names no volume and removes +/// none. +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct KeptCopies { + dir: PathBuf, +} + +impl KeptCopies { + /// The copies devlaunch keeps under `cache_dir`. + pub fn under(cache_dir: &Path) -> Self { + Self { + dir: cache_dir.join(COPIES_DIR), + } + } + + /// This workspace's copy. + /// + /// The id goes into the name unescaped, as + /// [`super::provision::verdict_cache`]'s marker does and for its reason: 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 copy_path(&self, workspace_id: &str) -> PathBuf { + self.dir.join(format!("{workspace_id}.json")) + } + + /// Copy what devpod recorded about this workspace, having just brought it up. + /// + /// **The write verb, and it does the read itself** — one document, one read, + /// one marker. Handing this the names instead would put a second parse of + /// `workspace_result.json` in the launch flow, and two parses of one document + /// are two answers waiting to disagree. + /// + /// An `up` that never completed leaves no result beside the record, so + /// [`sole_workspace_result`] finds nothing and this writes nothing. That + /// residual cannot be closed by anybody: devpod writes the result at the tail of + /// `devPodUp` after every failing branch has returned, so a create that died + /// leaves the dind volume standing with no record of it anywhere on the machine. + /// It stays small rather than open-ended because the names are deterministic for + /// a folder plus config: any later completed `up` of the same workspace records + /// the same names, and the volume is nameable again. + /// + /// Silent about every way of not working, exactly as the verdict cache's write + /// is. A copy that could not be written is a launch that behaves as it did + /// before this existed, and a launch is not worth failing over that. + pub(crate) fn keep(&self, workspace_id: &str, devpod_home: Option<&DevpodHome>) { + let Some(result) = sole_workspace_result(devpod_home, workspace_id) else { + return; + }; + let Ok(bytes) = std::fs::read(&result) else { + return; + }; + let recorded = parse_result(&bytes); + if recorded.is_empty() { + // Nothing devpod substituted and no image it named: a document this + // could not read is a document it copies nothing out of, which leaves + // the workspace exactly where it was before the copy existed. + return; + } + let Ok(text) = serde_json::to_string(&recorded) else { + return; + }; + write_atomically(&self.copy_path(workspace_id), &text); + } + + /// The volume names this workspace's copy holds, or nothing where there is no + /// copy to read. + /// + /// **The read verb.** `None` rather than an empty list, for the reason + /// `flows::lifecycle::devcontainer_volumes` answers `None`: a caller must not be + /// able to read "nothing was recorded" as "recorded nothing", and an empty list + /// would have to mean both. Every doubt reads as no copy — a file that is not + /// there, will not parse, or carries a shape this build does not know. + pub(crate) fn volumes(&self, workspace_id: &str) -> Option> { + NonEmpty::of(read_copy(&self.copy_path(workspace_id))?.volumes) + } + + /// Every workspace this cache holds a copy for, sorted. + /// + /// The reclaim's domain, and it is deliberately not a walk of the clone + /// directories. A copy whose clone the user deleted by hand names volumes no + /// clone-shaped walk will ever reach, and reasoning over an enumeration that + /// does not cover what it affects is the defect class devlaunch#445 exists to + /// close. + /// + /// Sorted so two runs over an unchanged cache read alike; a directory that is + /// not there is no copies rather than an error, which is what a fresh install + /// and a scratch cache both are. + pub(crate) fn copied(&self) -> Vec { + let Ok(entries) = std::fs::read_dir(&self.dir) else { + return Vec::new(); + }; + let mut ids: Vec = entries + .flatten() + .filter_map(|entry| { + let path = entry.path(); + (path.extension()? == "json") + .then(|| path.file_stem()?.to_str().map(str::to_owned)) + .flatten() + }) + .collect(); + ids.sort(); + ids + } + + /// Drop this workspace's copy. + /// + /// **The drop verb, and its one occasion is proof** — see the module note. A + /// copy that is not there is not a failure: the delete path and the reclaim can + /// both arrive here about the same workspace, and the second one has nothing to + /// do. + pub(crate) fn forget(&self, workspace_id: &str) { + let _ = std::fs::remove_file(self.copy_path(workspace_id)); + } +} + +/// What one copy says. +/// +/// Versioned by "unparseable means absent", which is the whole of the format story: +/// a copy a later build writes differently fails to deserialize and reads as no +/// copy, costing one workspace's volumes their record rather than costing anybody a +/// migration. +#[derive(Clone, Debug, Default, PartialEq, Eq, Deserialize, Serialize)] +struct Copy { + /// The derived volume names, in the order they were declared — **the names, + /// not the substituted values they were built from.** Freezing them at read + /// time is what keeps each one traceable to one real substitution: re-deriving + /// later under a changed template would produce a string devpod never + /// substituted, which is the pattern route arriving by the back door. + volumes: Vec, + /// `ContainerDetails.Config.Image` — the image reference the container devpod + /// built or pulled actually ran, whether or not anything named it. + /// + /// Read from the same document at the same moment as the volume names + /// (devlaunch#450), because versioning a small per-workspace file twice within + /// one map would be two migrations for one fact. **Nothing here removes an + /// image**; devlaunch#458 is the reclaim that reads this. + image: Option, + /// `MergedConfig.image` — the reference the devcontainer *named*, where it + /// named one at all. Kept apart from `image` so a reference the config declared + /// stays distinguishable from one devpod derived; a devcontainer built from a + /// Dockerfile declares none. + declared_image: Option, +} + +impl Copy { + /// Whether this copy would say nothing at all, in which case none is written. + fn is_empty(&self) -> bool { + self.volumes.is_empty() && self.image.is_none() && self.declared_image.is_none() + } +} + +/// Read one copy, or nothing where there is nothing readable there. +fn read_copy(path: &Path) -> Option { + serde_json::from_str(&std::fs::read_to_string(path).ok()?).ok() +} + +/// Everything devlaunch keeps out of one `workspace_result.json`, from one parse. +/// +/// Total over anything the file could hold: bytes that are not JSON, or JSON of +/// another shape, answer the empty copy rather than an error. Nothing here is worth +/// a diagnostic — a result devlaunch cannot read is a result it copies nothing out +/// of, which is exactly what it did before this existed. +fn parse_result(bytes: &[u8]) -> Copy { + let Ok(document) = serde_json::from_slice::(bytes) else { + return Copy::default(); + }; + Copy { + volumes: substitutions_in(&document).volume_names(), + image: non_empty_string(&document["ContainerDetails"]["Config"]["Image"]), + declared_image: non_empty_string(&document["MergedConfig"]["image"]), + } +} + +/// What devpod recorded substituting into one workspace's devcontainer. +/// +/// Only the two fields the volume names are built from. Both optional because +/// devpod omits an empty one, and a record written by a devpod that never learned +/// about `${devcontainerId}` has neither. +/// +/// There is deliberately **no provenance field**, here or on the names. A +/// `from_pattern: bool`, or a `Provenance` enum with a `Pattern` arm, would make an +/// inferred name representable and then rely on nobody building one, which is a +/// comment wearing a type's clothes. Instead there is no constructor for an +/// inferred name at all: names exist only as the return of +/// [`Substitutions::volume_names`], and a `Substitutions` is only ever built by +/// [`substitutions_in`] over bytes devpod wrote or devlaunch copied. Where the +/// distinction genuinely has to be visible is the report, and it belongs to the +/// occasion rather than the value — see `flows::lifecycle::SweepOccasion`. +#[derive(Debug, Default, PartialEq, Eq)] +pub(crate) struct Substitutions { + /// `SubstitutionContext.LocalWorkspaceFolder` — the host directory devpod + /// opened, whose basename is what `${localWorkspaceFolderBasename}` expanded + /// to. + local_workspace_folder: Option, + /// `SubstitutionContext.DevContainerID` — what `${devcontainerId}` expanded + /// to. + devcontainer_id: Option, +} + +impl Substitutions { + /// The volume names these substitutions imply, in the order they were + /// declared. Empty where neither field was recorded. + /// + /// The two name **templates** are this repository's own devcontainer and the + /// `docker-in-docker` feature's, which is devlaunch#325's scope; the two + /// *values* in them are devpod's own. This is the only place either template is + /// spelled, and `devlaunch-core/tests/volume_names.rs` holds the crate to that. + pub(crate) fn volume_names(&self) -> Vec { + let basename = self + .local_workspace_folder + .as_deref() + .map(Path::new) + .and_then(Path::file_name) + .map(|name| name.to_string_lossy().into_owned()); + [ + basename.map(|basename| format!("{basename}-pixi")), + self.devcontainer_id + .as_deref() + .map(|id| format!("dind-var-lib-docker-{id}")), + ] + .into_iter() + .flatten() + .collect() + } +} + +/// The two substituted values devpod's create result records. +pub(crate) fn substitutions_in(document: &serde_json::Value) -> Substitutions { + let context = &document["SubstitutionContext"]; + Substitutions { + local_workspace_folder: non_empty_string(&context["LocalWorkspaceFolder"]), + devcontainer_id: non_empty_string(&context["DevContainerID"]), + } +} + +/// Read the two substituted values off devpod's create result. +/// +/// Total for [`parse_result`]'s reason, and the same function underneath: the live +/// read at delete time and the kept copy's read must agree about what a record says +/// by construction rather than by two implementations being written alike. +pub(crate) fn parse_substitutions(bytes: &[u8]) -> Substitutions { + let Ok(document) = serde_json::from_slice::(bytes) else { + return Substitutions::default(); + }; + substitutions_in(&document) +} + +/// The string this value holds, where it holds a non-empty one. Empty is dropped +/// so a blank field cannot build the volume name `-pixi`. +fn non_empty_string(value: &serde_json::Value) -> Option { + match value.as_str() { + Some(text) if !text.is_empty() => Some(text.to_owned()), + _ => None, + } +} + +/// *text* into *path*, via a temp file in the same directory. +/// +/// The shape [`super::provision::verdict_cache`] and +/// [`super::completion_cache`] both write with, and here for the same property: a +/// reader arriving mid-write sees the old copy or the new one, never half of +/// either, and half a copy is a copy that does not parse and so reads as absent +/// anyway. +/// +/// Silent: the caller is best-effort. +fn write_atomically(path: &Path, text: &str) { + let Some(parent) = path.parent() else { + return; + }; + if std::fs::create_dir_all(parent).is_err() { + return; + } + let mut name = path.file_name().map(ToOwned::to_owned).unwrap_or_default(); + name.push(format!(".{}.tmp", std::process::id())); + let staged = path.with_file_name(name); + if std::fs::write(&staged, text).is_err() { + return; + } + let _ = std::fs::rename(&staged, path); +} + +#[cfg(test)] +mod tests { + //! The copy store on its own, over a scratch cache and a scratch devpod home. + //! + //! No docker and no devpod are involved anywhere here: what this module does is + //! read one file devpod left behind and write one of its own, and the whole of + //! it is decided by the state of two temporary directories. + + use super::*; + use crate::clients::devpod_home::{ScratchHome, devpod_home_with}; + + /// A cache directory, and the copies kept under it. + fn a_cache() -> (tempfile::TempDir, KeptCopies) { + let dir = tempfile::tempdir().expect("a scratch cache"); + let copies = KeptCopies::under(dir.path()); + (dir, copies) + } + + /// A devpod home whose create result for `workspace_id` records what devpod + /// substituted, in devpod's own shape: `SubstitutionContext` beside + /// `ContainerDetails` and `MergedConfig`. + fn a_completed_up(workspace_id: &str, folder: &str, devcontainer_id: &str) -> ScratchHome { + a_result( + workspace_id, + serde_json::json!({ + "ContainerDetails": { + "Id": "container-id", + "Config": { "Image": "vsc-devlaunch-4f2a-uid" }, + }, + "MergedConfig": { "image": "mcr.microsoft.com/devcontainers/base:jammy" }, + "SubstitutionContext": { + "LocalWorkspaceFolder": folder, + "ContainerWorkspaceFolder": "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/workspaces/whatever", + "DevContainerID": devcontainer_id, + }, + }), + ) + } + + /// A devpod home holding `document` as `workspace_id`'s create result. + fn a_result(workspace_id: &str, document: serde_json::Value) -> ScratchHome { + let home = devpod_home_with(&[("default", workspace_id, Some(()))]); + std::fs::write(home.result("default", workspace_id), document.to_string()) + .expect("a create result"); + home + } + + fn names(copies: &KeptCopies, workspace_id: &str) -> Vec { + copies + .volumes(workspace_id) + .map(|names| names.iter().cloned().collect()) + .unwrap_or_default() + } + + /// Both names, from one read of one document, in the order the templates are + /// declared. + #[test] + fn a_completed_up_is_copied_with_both_volume_names() { + let (_dir, copies) = a_cache(); + let home = a_completed_up("myws", "/home/someone/repos/o/r/repo-main-ab12", "abcdef"); + + copies.keep("myws", Some(&home)); + + assert_eq!( + names(&copies, "myws"), + ["repo-main-ab12-pixi", "dind-var-lib-docker-abcdef"] + ); + } + + /// The whole population this exists for: devpod's record dies with the + /// workspace, and the copy is in devlaunch's cache, so it does not. + #[test] + fn a_copy_outlives_the_devpod_record_its_names_came_from() { + let (_dir, copies) = a_cache(); + let home = a_completed_up("myws", "/repos/o/r/repo-main-ab12", "abcdef"); + copies.keep("myws", Some(&home)); + + std::fs::remove_dir_all(home.path()).expect("a bare `devpod delete` outside dl"); + + assert_eq!( + names(&copies, "myws"), + ["repo-main-ab12-pixi", "dind-var-lib-docker-abcdef"] + ); + } + + /// An `up` that died in its lifecycle hooks leaves the workspace record with no + /// result beside it. Nothing is named, so nothing is written, and the reclaim + /// finds no copy to act on — unchanged from before this existed. + #[test] + fn an_up_that_never_completed_is_not_copied_at_all() { + let (_dir, copies) = a_cache(); + let home = devpod_home_with(&[("default", "myws", None)]); + + copies.keep("myws", Some(&home)); + + assert_eq!(copies.volumes("myws"), None); + assert_eq!(copies.copied(), Vec::::new()); + } + + /// The image reference rides on the same copy, read from the same document at + /// the same moment (devlaunch#450), and the two references stay apart: what the + /// container ran, and what the devcontainer named. + #[test] + fn the_copy_carries_the_image_reference_from_the_same_read() { + let (dir, copies) = a_cache(); + let home = a_completed_up("myws", "/repos/o/r/repo-main-ab12", "abcdef"); + + copies.keep("myws", Some(&home)); + + let written = read_copy(&dir.path().join(COPIES_DIR).join("myws.json")).expect("a copy"); + assert_eq!(written.image.as_deref(), Some("vsc-devlaunch-4f2a-uid")); + assert_eq!( + written.declared_image.as_deref(), + Some("mcr.microsoft.com/devcontainers/base:jammy") + ); + } + + /// A devcontainer built from a Dockerfile names no image, and a devpod that + /// recorded no substitutions names no volume. Neither is a reason to lose the + /// other, and a document that says neither is not written down at all. + #[test] + fn each_recorded_fact_is_copied_without_the_others() { + let (_dir, copies) = a_cache(); + + let from_a_dockerfile = a_result( + "built", + serde_json::json!({ + "ContainerDetails": { "Config": { "Image": "vsc-built-9a" } }, + "MergedConfig": {}, + "SubstitutionContext": { "LocalWorkspaceFolder": "/repos/o/r/built" }, + }), + ); + copies.keep("built", Some(&from_a_dockerfile)); + assert_eq!(names(&copies, "built"), ["built-pixi"]); + + let unreadable = a_result("garbled", serde_json::json!({ "MergedConfig": {} })); + copies.keep("garbled", Some(&unreadable)); + assert_eq!(copies.copied(), ["built"]); + } + + /// Every doubt reads as no copy: a file that is not there, one that will not + /// parse, and one whose volume list is empty — which must not read as "recorded + /// nothing" (the `NonEmpty` the ticket asks for, with no empty-list state). + #[test] + fn every_unreadable_copy_names_no_volume() { + let (dir, copies) = a_cache(); + let store = dir.path().join(COPIES_DIR); + std::fs::create_dir_all(&store).expect("a copies directory"); + + assert_eq!(copies.volumes("never-written"), None); + + std::fs::write(store.join("garbled.json"), "{not json").expect("a truncated copy"); + assert_eq!(copies.volumes("garbled"), None); + + std::fs::write(store.join("empty.json"), r#"{"volumes":[]}"#).expect("an empty copy"); + assert_eq!(copies.volumes("empty"), None); + } + + /// The domain of the reclaim: the set of copies, sorted, and nothing else in + /// the directory joins it. + #[test] + fn the_copies_are_enumerated_and_nothing_else_in_the_directory_is() { + let (dir, copies) = a_cache(); + for id in ["bee", "ant"] { + let home = a_completed_up(id, &format!("/repos/o/r/{id}"), id); + copies.keep(id, Some(&home)); + } + std::fs::write(dir.path().join(COPIES_DIR).join("notes.txt"), "not a copy") + .expect("a stray file"); + + assert_eq!(copies.copied(), ["ant", "bee"]); + } + + /// A scratch `XDG_CACHE_HOME` is the map's hard constraint, and this is what + /// makes it hold by construction: a cache with no copies in it names no volume, + /// so a scratch run removes none. + #[test] + fn a_cache_with_no_copies_names_no_volume() { + let (_dir, copies) = a_cache(); + + assert_eq!(copies.copied(), Vec::::new()); + assert_eq!(copies.volumes("myws"), None); + } + + /// Dropped once, and dropping one that is already gone is not a complaint: the + /// delete path and the reclaim can both arrive about the same workspace. + #[test] + fn a_dropped_copy_is_gone_and_dropping_it_twice_is_no_complaint() { + let (_dir, copies) = a_cache(); + let home = a_completed_up("myws", "/repos/o/r/repo-main-ab12", "abcdef"); + copies.keep("myws", Some(&home)); + + copies.forget("myws"); + assert_eq!(copies.volumes("myws"), None); + assert_eq!(copies.copied(), Vec::::new()); + + copies.forget("myws"); + copies.forget("never-there"); + } + + /// A later completed `up` rewrites the copy rather than leaving the first one + /// standing. That is what keeps the residual small: a workspace whose first + /// `up` died records nothing, and the next one that finishes records the names. + #[test] + fn a_later_completed_up_replaces_the_copy() { + let (_dir, copies) = a_cache(); + copies.keep( + "myws", + Some(&a_completed_up("myws", "/repos/o/r/before", "aaaa")), + ); + + copies.keep( + "myws", + Some(&a_completed_up("myws", "/repos/o/r/after", "bbbb")), + ); + + assert_eq!( + names(&copies, "myws"), + ["after-pixi", "dind-var-lib-docker-bbbb"] + ); + } + + /// A machine whose devpod records cannot be located has no document to copy + /// from, which is nothing to write rather than something to fail over. + #[test] + fn a_host_with_no_devpod_home_copies_nothing() { + let (_dir, copies) = a_cache(); + + copies.keep("myws", None); + + assert_eq!(copies.copied(), Vec::::new()); + } + + /// Ids are unique per context, not globally, and `sole_workspace_result` + /// answers nothing to the ambiguity. Copying from whichever context the walk + /// reached first would name a *different* workspace's volumes, and the copy + /// outlives the record that could have corrected it. + #[test] + fn a_workspace_two_contexts_hold_is_copied_from_neither() { + let (_dir, copies) = a_cache(); + let home = devpod_home_with(&[("default", "myws", Some(())), ("work", "myws", Some(()))]); + + copies.keep("myws", Some(&home)); + + assert_eq!(copies.copied(), Vec::::new()); + } + + /// Each recorded substitution names its own volume, and one recorded without + /// the other still gets the volume it can name. Both spellings are devpod's. + #[test] + fn each_recorded_substitution_names_its_own_volume() { + let both = Substitutions { + local_workspace_folder: Some("/repos/o/r/repo-branch-abcd".to_owned()), + devcontainer_id: Some("deadbeef".to_owned()), + }; + assert_eq!( + both.volume_names(), + ["repo-branch-abcd-pixi", "dind-var-lib-docker-deadbeef"] + ); + + let no_id = Substitutions { + local_workspace_folder: Some("/repos/o/r/repo-branch-abcd".to_owned()), + devcontainer_id: None, + }; + assert_eq!(no_id.volume_names(), ["repo-branch-abcd-pixi"]); + + assert!(Substitutions::default().volume_names().is_empty()); + + // A blank field names nothing: `-pixi` and `dind-var-lib-docker-` are not + // volumes anybody meant, and asking docker about them is asking about + // somebody else's disk. + let blank = parse_substitutions( + serde_json::json!({ + "SubstitutionContext": { "LocalWorkspaceFolder": "", "DevContainerID": "" }, + }) + .to_string() + .as_bytes(), + ); + assert_eq!(blank.volume_names(), Vec::::new()); + } + + /// Bytes that are not JSON, and JSON of another shape, name nothing rather + /// than failing: a result devlaunch cannot read is a result it removes no + /// volumes from. + #[test] + fn a_result_that_is_not_a_devpod_record_names_nothing() { + for bytes in [ + &b"not json at all"[..], + b"[]", + br#"{"SubstitutionContext": "a string"}"#, + b"{}", + ] { + assert_eq!( + parse_substitutions(bytes), + Substitutions::default(), + "{}", + String::from_utf8_lossy(bytes) + ); + } + } +} diff --git a/rust/devlaunch-core/src/flows/launch.rs b/rust/devlaunch-core/src/flows/launch.rs index 6b9eda04..cf9da251 100644 --- a/rust/devlaunch-core/src/flows/launch.rs +++ b/rust/devlaunch-core/src/flows/launch.rs @@ -71,6 +71,7 @@ use crate::domain::locks::{self, Contention, LockError}; use crate::domain::metadata::MetadataStorage; use crate::domain::spec::{self, DevcontainerPath, SpecIdentity, WorkspaceSpec}; use crate::domain::workspace_id::{NamePart, UnsafeName, WorkspaceId, validate_ref_name}; +use crate::flows::kept_copies::KeptCopies; use crate::flows::lifecycle::{ self, KnownWorkspace, LifecycleNotice, Refresh, RefreshReason, StopOutcome, }; @@ -198,6 +199,17 @@ pub struct Host { pub(crate) devpod_home: Option, } +impl Host { + /// devlaunch's copies of what devpod substituted, under this host's cache. + /// + /// Derived here rather than carried as a field, so the store and everything + /// else the cache holds cannot be pointed at two different directories: there + /// is one `cache_dir` and this is a view of it. + pub(crate) fn kept_copies(&self) -> KeptCopies { + KeptCopies::under(&self.cache_dir) + } +} + impl Host { /// The host this process is running on, with `cache_dir` supplied. /// @@ -1284,6 +1296,15 @@ fn up_under_stage( // before devpod wrote that file: the stale result still matches, and the // hostname nobody set stays unset. Nothing the host can read tells that // apart from a container that never stopped. + // + // The copy is kept on this arm too, and it is not a tidiness: the sibling + // this launch waited out may not have been `dl` at all, and `--purge` + // destroys the cache holding the copies while leaving a foreign workspace's + // volumes standing (devlaunch#452). It costs one write on a path that has + // already established the create completed, which is the same condition the + // copy is written under below. The claim it records is "dl brought this up", + // never "dl created it". + host.kept_copies().keep(identity, host.devpod_home.as_ref()); let seen = provision .provision_tools(context.runner(), identity, PassOccasion::TopUp, title) .map_err(|DevpodMissing| NotRun::NotInstalled)?; @@ -1319,6 +1340,13 @@ fn up_under_stage( // Only after a successful `up`: there is no container to install into // otherwise. Inside the lock, for the reason it was taken above. if let Some(identity) = request.naming.identity() { + // The tail of a completed `up`, and **before** the pass rather than after + // it. devpod has just rewritten `workspace_result.json`, which is the one + // document naming this container's volumes, and the pass below can fail and + // take the whole launch with it while the container and its volumes stand. + // A copy written on the other side of that pass would be the one launch + // whose volumes nothing ever names. See [`crate::flows::kept_copies`]. + host.kept_copies().keep(identity, host.devpod_home.as_ref()); // A devpod that went missing between the `up` that just worked and the pass // that follows it takes the launch with it, as Python's exception does: there // is no session to hand over without the binary that opens one. @@ -3526,6 +3554,36 @@ mod tests { self } + /// devpod's record for a workspace whose create finished, carrying what + /// devpod substituted into the devcontainer. + /// + /// The shape is devpod's own — `SubstitutionContext` beside + /// `ContainerDetails` and `MergedConfig` — because the copy this launch + /// keeps is a read of exactly this document and nothing else. + fn with_substitutions( + self, + workspace_id: &str, + folder: &str, + devcontainer_id: &str, + ) -> Self { + let home = self.devpod_home(); + self.write_record(workspace_id); + std::fs::write( + home.result("default", workspace_id), + serde_json::json!({ + "ContainerDetails": { "Config": { "Image": "vsc-repo-main-ab12-uid" } }, + "MergedConfig": {}, + "SubstitutionContext": { + "LocalWorkspaceFolder": folder, + "DevContainerID": devcontainer_id, + }, + }) + .to_string(), + ) + .expect("a create result"); + self + } + /// devpod's record for a workspace whose create *aborted*: the record, and /// no result beside it. A `postCreateCommand` that exits non-zero leaves /// exactly this, with the container still up. @@ -4310,6 +4368,205 @@ mod tests { assert_eq!(outcome, Err(NotRun::NotInstalled)); } + // ----------------------------------------- the copy of the volume names + + /// A pass that reports what devlaunch's kept copy said at the moment it ran. + /// + /// The ordering is the whole of what this seam is about, and a test that only + /// looked at the cache afterwards could not tell "written before the pass" from + /// "written after it". This can: the pass reads the copy from inside itself. + struct CopyWatchingProvision { + copies: KeptCopies, + seen: Mutex>>>, + } + + impl CopyWatchingProvision { + fn over(cache_dir: &Path) -> Self { + Self { + copies: KeptCopies::under(cache_dir), + seen: Mutex::new(Vec::new()), + } + } + + /// What the copy said on each pass, in order. + fn seen(&self) -> Vec>> { + self.seen + .lock() + .unwrap_or_else(PoisonError::into_inner) + .clone() + } + } + + impl Provision for CopyWatchingProvision { + fn provision_tools( + &self, + _runner: &dyn Runner, + workspace_id: &str, + _occasion: PassOccasion, + _title: Option<&str>, + ) -> Result, DevpodMissing> { + let named = self + .copies + .volumes(workspace_id) + .map(|names| names.iter().cloned().collect()); + self.seen + .lock() + .unwrap_or_else(PoisonError::into_inner) + .push(named); + Ok(None) + } + + fn remembered_claude(&self, _workspace_id: &str) -> Option { + None + } + } + + /// The names in the copy this cache holds for `workspace_id`. + fn copied_volumes(cache_dir: &Path, workspace_id: &str) -> Option> { + KeptCopies::under(cache_dir) + .volumes(workspace_id) + .map(|names| names.iter().cloned().collect()) + } + + /// **Before the provisioning pass, not after it.** Provisioning can fail and + /// take the launch with it while the container and its volumes stand, and a + /// copy written on the other side of that would be the one launch whose volumes + /// nothing ever names. + #[test] + fn a_completed_up_copies_the_volume_names_before_the_provisioning_pass() { + let scene = Scene::new().with_substitutions("myws", "/repos/o/r/repo-main-ab12", "abcdef"); + let provision = CopyWatchingProvision::over(scene.cache_dir()); + let mut context = CommandContext::new(&scene.runner); + let token = HostToken::new(); + let request = UpRequest::new( + "owner/repo", + Naming::Create { + workspace_id: "myws", + }, + ); + + let outcome = workspace_up( + &mut context, + &scene.host, + &token, + &ClaudeSeen::new(), + &provision, + &request, + None, + &mut no_notices(), + ); + + assert_eq!(outcome, Ok(UpOutcome::Started)); + assert_eq!( + provision.seen(), + vec![Some(vec![ + "repo-main-ab12-pixi".to_owned(), + "dind-var-lib-docker-abcdef".to_owned(), + ])], + "the copy has to be there by the time the pass runs" + ); + } + + /// The other arm of the same `up`, and it is not optional: the sibling this + /// launch waited out may not have been `dl` at all, and `--purge` would + /// otherwise destroy a foreign workspace's copy while leaving its volumes + /// standing (devlaunch#452). The copy claims "dl brought this up", never "dl + /// created it". + #[test] + fn a_sibling_that_won_the_race_still_leaves_this_launch_a_copy() { + let scene = Scene::new().with_running("myws").with_substitutions( + "myws", + "/repos/o/r/repo-main-ab12", + "abcdef", + ); + let provision = CopyWatchingProvision::over(scene.cache_dir()); + let request = UpRequest::new( + "owner/repo", + Naming::Create { + workspace_id: "myws", + }, + ); + + let (outcome, _) = contended_up(&scene, &request, &provision); + + assert_eq!(outcome, Ok(UpOutcome::SkippedSiblingWon)); + assert_eq!( + provision.seen(), + vec![Some(vec![ + "repo-main-ab12-pixi".to_owned(), + "dind-var-lib-docker-abcdef".to_owned(), + ])] + ); + } + + /// An `up` devpod refused is not a completed `up`, so there is nothing devpod + /// wrote for this launch to copy. Whatever the cache already held is left + /// exactly as it was. + #[test] + fn an_up_devpod_refused_copies_nothing() { + let scene = Scene::new(); + scene + .runner + .script(["devpod", "up"], Response::failed(1, "no such provider\n")); + let mut context = CommandContext::new(&scene.runner); + let token = HostToken::new(); + let request = UpRequest::new( + "owner/repo", + Naming::Create { + workspace_id: "myws", + }, + ); + + let outcome = workspace_up( + &mut context, + &scene.host, + &token, + &ClaudeSeen::new(), + &NoProvisioning, + &request, + None, + &mut no_notices(), + ); + + assert!( + matches!(outcome, Ok(UpOutcome::Refused { .. })), + "{outcome:?}" + ); + assert_eq!(copied_volumes(scene.cache_dir(), "myws"), None); + } + + /// An `up` that finished without devpod recording a create result names + /// nothing, and this is where that case arrives: a create that died in its + /// lifecycle hooks. No copy, and the launch is otherwise untouched. + #[test] + fn an_up_devpod_recorded_nothing_for_copies_nothing() { + let scene = Scene::new(); + let provision = CopyWatchingProvision::over(scene.cache_dir()); + let mut context = CommandContext::new(&scene.runner); + let token = HostToken::new(); + let request = UpRequest::new( + "owner/repo", + Naming::Create { + workspace_id: "myws", + }, + ); + + let outcome = workspace_up( + &mut context, + &scene.host, + &token, + &ClaudeSeen::new(), + &provision, + &request, + None, + &mut no_notices(), + ); + + assert_eq!(outcome, Ok(UpOutcome::Started)); + assert_eq!(provision.seen(), vec![None]); + assert_eq!(copied_volumes(scene.cache_dir(), "myws"), None); + } + // --------------------------------------------------------- the up argv #[test] diff --git a/rust/devlaunch-core/src/flows/lifecycle.rs b/rust/devlaunch-core/src/flows/lifecycle.rs index b9eb9ba2..7953fbc4 100644 --- a/rust/devlaunch-core/src/flows/lifecycle.rs +++ b/rust/devlaunch-core/src/flows/lifecycle.rs @@ -20,11 +20,15 @@ //! volumes its devcontainer created ([`VolumeSweep`]), and refuses when the //! clone holds work that exists nowhere else. That refusal is the one judgement //! dl makes on its own account (see [`crate::domain::workspace_state`]). -//! - **`dl --prune`** removes the clone *directories* no live workspace opens. It -//! never touches a devpod workspace, a container, an image or a volume, and it -//! leaves every bare cache alone. **Still true of volumes** after devlaunch#325, -//! and not an oversight: it deletes no workspace, so there is no workspace whose -//! volumes it could be taking. +//! - **`dl --prune`** removes the clone *directories* no live workspace opens, and +//! reclaims the docker volumes of workspaces devpod no longer lists, from +//! devlaunch's own copies of what devpod substituted +//! ([`crate::flows::kept_copies`]). It never touches a devpod workspace, a +//! container or an image, and it leaves every bare cache alone. The volume half +//! arrived with devlaunch#456 and its reason is the inverse of the old one: prune +//! still deletes no workspace, and *because* it deletes none, the volumes it +//! reclaims belong to workspaces something else already deleted — where devpod's +//! own record is gone and devlaunch's copy is the only thing that names them. //! - **`dl --purge`** deletes the workspaces devlaunch created — volumes and all — //! and its whole cache directory. Ownership-scoped //! ([`crate::flows::listing::workspace_ownership`]), and it names what it leaves @@ -60,7 +64,7 @@ //! leave a stale snapshot behind. use std::cell::RefCell; -use std::collections::{BTreeMap, HashMap}; +use std::collections::{BTreeMap, HashMap, HashSet}; use std::path::{Path, PathBuf}; use std::time::Duration; @@ -79,6 +83,7 @@ use crate::domain::model::WorktreeInfo; use crate::domain::workspace_state::{self, CouldNotTell, Losses, NonEmpty, Unsaved}; use crate::flows::completion_cache; use crate::flows::disk_usage::{self, DiskUsage}; +use crate::flows::kept_copies::{self, KeptCopies}; use crate::flows::listing::{ self, ClonePathResolver, CommandContext, WorkspaceOwnership, json_as_python_writes_it, }; @@ -122,8 +127,12 @@ pub enum LifecycleNotice { /// rather than a delete to fail. Carries the refusal itself and not a /// rendering of it — and can only be built from a refusal, because /// [`VolumeRefusal`] holds no arm for a sweep that went fine. + /// + /// `occasion` says which read named them, because that is what tells somebody + /// where to look: devpod's own record, or devlaunch's copy of one. VolumesNotRemoved { workspace_id: String, + occasion: SweepOccasion, refusal: VolumeRefusal, }, /// A clone directory went and its `metadata.json` record could not be @@ -921,6 +930,26 @@ pub enum VolumeSweep { Refused(VolumeRefusal), } +/// Which read named the volumes one sweep was about. +/// +/// **Two arms, and the third one is the point.** The distinction the design turns +/// on is between a name that was *read* and a name that was *inferred*, and it is +/// deliberately not on the name: a `Provenance` field with a `Pattern` arm would +/// make an inferred name representable and then rely on nobody building one, which +/// is a comment wearing a type's clothes. There is no constructor for an inferred +/// name at all (see [`crate::flows::kept_copies`]), so what is left to say belongs +/// to the *occasion* — which document this particular sweep read. Two arms and no +/// third, so adding a pattern arm later is a compile error at every match rather +/// than a branch nobody notices. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SweepOccasion { + /// devpod's own create result, read while the workspace still had one — at the + /// tail of a delete, immediately before `devpod delete` takes it away. + DevpodResult, + /// devlaunch's kept copy, for a workspace devpod no longer lists. + KeptCopy, +} + /// Why a deleted workspace's docker volumes are still on this machine. /// /// Apart from [`VolumeSweep`]'s three silent arms rather than among them, so that @@ -979,6 +1008,7 @@ pub fn workspace_delete( clones: &WorkspaceCloneManager<'_>, storage: &mut MetadataStorage, devpod_home: Option<&DevpodHome>, + copies: &KeptCopies, workspace_id: &str, insistence: Insistence, persistence: Persistence, @@ -1057,11 +1087,20 @@ pub fn workspace_delete( } }; let volumes = sweep_volumes(context.runner(), named); - if let VolumeSweep::Refused(refusal) = &volumes { - notices.say(LifecycleNotice::VolumesNotRemoved { + match &volumes { + VolumeSweep::Refused(refusal) => notices.say(LifecycleNotice::VolumesNotRemoved { workspace_id: workspace_id.to_owned(), + occasion: SweepOccasion::DevpodResult, refusal: refusal.clone(), - }); + }), + // Dropped on proof, and the proof is the same one `--prune`'s reclaim + // drops a copy on: a removal that came back removed, for a workspace + // devpod no longer has. The copy named volumes that are gone, so it is + // pointless — and left standing it would have the next `--prune` report + // reclaiming volumes that went with the workspace. `Refused` keeps it, so + // the retry survives; the two silent arms name nothing and prove nothing. + VolumeSweep::Removed => copies.forget(workspace_id), + VolumeSweep::NothingNamed | VolumeSweep::NoDocker => {} } refresh.ask(context.runner(), RefreshReason::Forced); Ok(DeleteOutcome::Deleted { clone, volumes }) @@ -1126,74 +1165,10 @@ fn devcontainer_volumes( workspace_id: &str, ) -> Option> { let result = sole_workspace_result(devpod_home, workspace_id)?; - let recorded = parse_substitutions(&std::fs::read(result).ok()?); + let recorded = kept_copies::parse_substitutions(&std::fs::read(result).ok()?); NonEmpty::of(recorded.volume_names()) } -/// What devpod recorded substituting into one workspace's devcontainer. -/// -/// Only the two fields the volume names are built from. Both optional because -/// devpod omits an empty one, and a record written by a devpod that never learned -/// about `${devcontainerId}` has neither. -#[derive(Debug, Default, PartialEq, Eq)] -struct Substitutions { - /// `SubstitutionContext.LocalWorkspaceFolder` — the host directory devpod - /// opened, whose basename is what `${localWorkspaceFolderBasename}` expanded - /// to. - local_workspace_folder: Option, - /// `SubstitutionContext.DevContainerID` — what `${devcontainerId}` expanded - /// to. - devcontainer_id: Option, -} - -impl Substitutions { - /// The volume names these substitutions imply, in the order they were - /// declared. Empty where neither field was recorded. - fn volume_names(&self) -> Vec { - let basename = self - .local_workspace_folder - .as_deref() - .map(Path::new) - .and_then(Path::file_name) - .map(|name| name.to_string_lossy().into_owned()); - [ - basename.map(|basename| format!("{basename}-pixi")), - self.devcontainer_id - .as_deref() - .map(|id| format!("dind-var-lib-docker-{id}")), - ] - .into_iter() - .flatten() - .collect() - } -} - -/// Read the two substituted values off devpod's create result. -/// -/// Total over anything the file could hold: bytes that are not JSON, or JSON of -/// another shape, answer the empty set rather than an error. Nothing here is worth -/// a diagnostic — a result devlaunch cannot read is a result it removes no volumes -/// from, which is exactly what it did before this existed. -fn parse_substitutions(bytes: &[u8]) -> Substitutions { - let Ok(document) = serde_json::from_slice::(bytes) else { - return Substitutions::default(); - }; - let context = &document["SubstitutionContext"]; - Substitutions { - local_workspace_folder: non_empty_string(&context["LocalWorkspaceFolder"]), - devcontainer_id: non_empty_string(&context["DevContainerID"]), - } -} - -/// The string this value holds, where it holds a non-empty one. Empty is dropped -/// so a blank field cannot build the volume name `-pixi`. -fn non_empty_string(value: &serde_json::Value) -> Option { - match value.as_str() { - Some(text) if !text.is_empty() => Some(text.to_owned()), - _ => None, - } -} - /// `devpod delete [--ignore-not-found] [--force]` — argv-exact. /// /// The two flags answer two different questions and are not two spellings of one. @@ -1345,6 +1320,7 @@ pub enum PurgeStep { /// worth naming about it. The three sweeps that went fine say nothing. VolumesNotRemoved { workspace_id: String, + occasion: SweepOccasion, refusal: VolumeRefusal, }, } @@ -1410,6 +1386,15 @@ impl PurgeOutcome { /// /// A cache that does not come away completely is reported rather than raised: see /// [`remove_tree_as_far_as_it_goes`] for why it is removed as far as it goes. +/// +/// **The ordering here is a property and not a habit.** The workspaces are deleted +/// first, which sweeps each one's volumes from devpod's own live record, and only +/// then is the cache removed — and that cache holds devlaunch's copies of those +/// same names ([`crate::flows::kept_copies`]). Interrupted between the two, the +/// copies that are lost belong to workspaces already swept, so nothing is lost that +/// was not already reclaimed. The reverse ordering is the one that loses bytes +/// permanently: it would destroy every copy and leave the volumes standing, which +/// is exactly the orphan population devlaunch#451 declined to reclaim. pub fn purge_all_data( context: &mut CommandContext<'_>, plan: &PurgePlan, @@ -1438,6 +1423,7 @@ pub fn purge_all_data( if let VolumeSweep::Refused(refusal) = sweep_volumes(context.runner(), named) { on_step(PurgeStep::VolumesNotRemoved { workspace_id: workspace.id.clone(), + occasion: SweepOccasion::DevpodResult, refusal, }); } @@ -2092,6 +2078,43 @@ pub struct Reclaimable { pub promotion: Promotion, } +/// One workspace's volumes this run will reclaim, and the names it will reclaim. +/// +/// **The names ride on the record, never as a plan-wide list.** That is this map's +/// second precedent, and the reason is the one [`PrunePlan`] gives for having no +/// `force` field: a list beside the entries is a list the acting pass can read +/// against the wrong entry, and the plan and the act would then disagree about +/// which volumes belonged to which workspace. +/// +/// [`NonEmpty`] rather than a `Vec`, so an entry naming nothing has no +/// representation: a copy that names no volume is not in the plan at all. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ReclaimableVolumes { + /// The workspace devpod no longer lists, whose copy named these. + pub workspace_id: String, + /// The volume names devlaunch copied out of devpod's create result at the tail + /// of the last completed `up` of this workspace. + pub names: NonEmpty, +} + +/// One workspace's volumes the acting pass would not remove, and why. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct VolumesKept { + pub workspace_id: String, + pub because: VolumesKeptBecause, +} + +/// Why one workspace's volumes are staying. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum VolumesKeptBecause { + /// devpod lists this workspace again. It did not when the plan was printed, and + /// a live workspace's volumes are not leftovers — the same shrinking-only + /// direction the clone re-classification moves in. + ListedAgain, + /// docker would not release them. The copy is kept, so the retry survives. + Refused(VolumeRefusal), +} + /// One clone directory this run will leave standing, and why. #[derive(Debug, Clone, PartialEq, Eq)] pub struct Kept { @@ -2122,12 +2145,16 @@ pub struct PrunePlan { keeping: Vec, /// Worktree records whose directory is definitively not there any more. stale_records: Vec, + /// The volumes of the workspaces devpod has forgotten, from devlaunch's own + /// copies of what devpod substituted. A second enumeration beside the clone + /// walk rather than a column on it: see [`prune_plan`]. + reclaiming: Vec, } impl PrunePlan { /// Whether this run would change nothing at all. pub fn nothing_to_do(&self) -> bool { - self.removing.is_empty() && self.stale_records.is_empty() + self.removing.is_empty() && self.stale_records.is_empty() && self.reclaiming.is_empty() } /// What the whole run would free. @@ -2154,6 +2181,11 @@ impl PrunePlan { pub fn stale_records(&self) -> &[WorktreeInfo] { &self.stale_records } + + /// The volumes this run will reclaim from devlaunch's kept copies. + pub fn reclaiming(&self) -> &[ReclaimableVolumes] { + &self.reclaiming + } } /// The directory `--prune` scans, canonicalised once. @@ -2230,10 +2262,28 @@ pub enum PruneError { /// clone *after* the lock is released, so a clone whose launch has finished cloning /// and not yet registered a workspace is briefly indistinguishable from a stale /// one. +/// +/// # The volumes are a second enumeration, not a column on the first +/// +/// `--prune` also reclaims the docker volumes of workspaces devpod has forgotten, +/// read from devlaunch's own copies ([`crate::flows::kept_copies`]), and the domain +/// of that pass is **the set of copies** rather than the set of clone directories. +/// A copy whose clone the user deleted by hand names volumes no clone-shaped walk +/// will ever reach, and reasoning over an enumeration that does not cover what it +/// affects is the defect class devlaunch#445 exists to close. The precondition per +/// copy is one thing: no workspace devpod lists carries that id. +/// +/// `--prune` is the surface because it already owns "the workspace is gone and its +/// leftovers are ours", and because without this every prune that removed an +/// orphaned clone *manufactured* a permanent orphan — devpod's record died at the +/// outside delete, so once the clone went the volumes were unreachable forever. It +/// still deletes no workspace, no container and no image. +#[allow(clippy::too_many_arguments)] pub fn prune_plan( clones: &WorkspaceCloneManager<'_>, storage: &MetadataStorage, workspaces: &[Workspace], + copies: &KeptCopies, placement: &ClonePlacement, insistence: Insistence, notices: &mut dyn Notices, @@ -2308,9 +2358,33 @@ pub fn prune_plan( removing, keeping, stale_records, + reclaiming: reclaimable_volumes(copies, workspaces), }) } +/// The copies whose workspace devpod no longer lists, each carrying its own names. +/// +/// The one precondition, and it is asked of `workspaces` rather than of the disk: +/// devpod's listing is the only thing that can say a workspace is still alive, and +/// a copy for a live one names volumes that are in use. Answering it from the +/// listing the caller already has is what lets the acting pass ask it again for +/// free, under the second listing it already pays for. +fn reclaimable_volumes(copies: &KeptCopies, workspaces: &[Workspace]) -> Vec { + let live: HashSet<&str> = workspaces.iter().map(|it| it.id.as_str()).collect(); + copies + .copied() + .into_iter() + .filter(|workspace_id| !live.contains(workspace_id.as_str())) + .filter_map(|workspace_id| { + let names = copies.volumes(&workspace_id)?; + Some(ReclaimableVolumes { + workspace_id, + names, + }) + }) + .collect() +} + /// `metadata.json`'s worktree records, keyed by the directory each names. /// /// Which directory a record names is @@ -2411,6 +2485,12 @@ pub struct PruneReport { /// unfinished, and the clones that *did* go are still gone — which is why this /// is a report and not an abort. pub refused: Vec, + /// The workspaces whose volumes went, and which volumes. Their copies are + /// dropped: the removal is what proved them pointless. + pub reclaimed: Vec, + /// The workspaces whose volumes stayed, and why. Their copies are kept, so + /// every one of these is retryable. + pub volumes_kept: Vec, } impl PruneReport { @@ -2460,6 +2540,7 @@ pub fn prune_clones( context: &mut CommandContext<'_>, clones: &WorkspaceCloneManager<'_>, storage: &mut MetadataStorage, + copies: &KeptCopies, plan: &PrunePlan, notices: &mut dyn Notices, ) -> Result { @@ -2489,6 +2570,8 @@ pub fn prune_clones( removed: Vec::new(), withheld: Vec::new(), refused: Vec::new(), + reclaimed: Vec::new(), + volumes_kept: Vec::new(), }; let mut forget: Vec = Vec::new(); for ((owner, repo), reclaimables) in by_repo { @@ -2533,6 +2616,10 @@ pub fn prune_clones( } } } + // Outside every repo lock too, and for a stronger reason than the record drop + // below: these volumes belong to workspaces with no clone directory in this + // plan at all, so no repository lock is about them. + reclaim_volumes(context.runner(), copies, plan, &workspaces, &mut report); // Outside every repo lock, because the repo lock is what protects the // *directory* work and a record drop touches only `metadata.json`, which has a // lock of its own. Keeping it out means a repository is held for exactly as @@ -2544,6 +2631,60 @@ pub fn prune_clones( Ok(PruneOutcome::Acted(report)) } +/// Remove the volumes the plan named, and drop the copies the removal made +/// pointless. +/// +/// **The precondition is re-asked here, under the second listing the acting pass +/// already pays for**, exactly as each clone directory is re-classified before it +/// goes. The plan was taken before the user answered it, and a workspace can come +/// back to devpod's listing in between — a launch of that very id, or a +/// `--reconcile` — at which point its volumes are a live workspace's and not +/// leftovers. The approved set can therefore shrink and can never grow, which is +/// the direction that costs a command rather than somebody's disk. +/// +/// The removal itself is [`sweep_volumes`], the same one the delete path uses, so +/// there is one place in devlaunch where a name reaches `docker volume rm` and one +/// place where docker's answer becomes a verdict. +fn reclaim_volumes( + runner: &dyn Runner, + copies: &KeptCopies, + plan: &PrunePlan, + workspaces: &[Workspace], + report: &mut PruneReport, +) { + let live: HashSet<&str> = workspaces.iter().map(|it| it.id.as_str()).collect(); + for reclaimable in &plan.reclaiming { + if live.contains(reclaimable.workspace_id.as_str()) { + report.volumes_kept.push(VolumesKept { + workspace_id: reclaimable.workspace_id.clone(), + because: VolumesKeptBecause::ListedAgain, + }); + continue; + } + match sweep_volumes(runner, Some(reclaimable.names.clone())) { + // Dropped once, on proof. This is the deliberate divergence from + // `verdict_cache`'s "nothing ever deletes a marker": there a delete + // would be a second unproven mechanism, here it is conditioned on the + // removal that made the copy pointless. + VolumeSweep::Removed => { + copies.forget(&reclaimable.workspace_id); + report.reclaimed.push(reclaimable.clone()); + } + // Kept, so the retry survives — a volume some container still holds is + // one this run could not have taken anyway. + VolumeSweep::Refused(refusal) => report.volumes_kept.push(VolumesKept { + workspace_id: reclaimable.workspace_id.clone(), + because: VolumesKeptBecause::Refused(refusal), + }), + // A machine with no docker never made these volumes, so there is + // nothing here to have failed and nothing to say — the silence the + // delete path keeps for the same arm. `NothingNamed` is unreachable: + // the names ride on the entry and a `NonEmpty` has no empty state. + VolumeSweep::NoDocker | VolumeSweep::NothingNamed => {} + } + } +} + /// Drop one worktree record. /// /// Removing a clone without this is what left `metadata.json` describing workspaces @@ -3464,6 +3605,11 @@ mod tests { record } + /// devlaunch's copies of what devpod substituted, under this world's cache. + fn copies(&self) -> KeptCopies { + KeptCopies::under(&self.cache) + } + fn branches_on_record(&self) -> Vec { let mut branches: Vec = self .storage @@ -3500,6 +3646,7 @@ mod tests { &clones, &world.storage, &workspaces, + &world.copies(), &placement, insistence, &mut ignoring(), @@ -4240,6 +4387,7 @@ mod tests { }, PurgeStep::VolumesNotRemoved { workspace_id: "r-main-aa".to_owned(), + occasion: SweepOccasion::DevpodResult, refusal: VolumeRefusal::Docker { exit: Exit::Code(1), stderr: "volume is in use\n".to_owned(), @@ -4655,12 +4803,14 @@ mod tests { let mut context = CommandContext::new(&world.devpod); let mut refresh = Refresh::new(&world.updater, &world.cache_path); + let copies = world_cache.copies(); let outcome = workspace_delete( &mut context, &mut refresh, &clones, &mut world_cache.storage, None, + &copies, "myws", Insistence::NotInsisted, Persistence::Ordinary, @@ -4690,12 +4840,14 @@ mod tests { let mut context = CommandContext::new(&world.devpod); let mut refresh = Refresh::new(&world.updater, &world.cache_path); + let copies = world_cache.copies(); workspace_delete( &mut context, &mut refresh, &clones, &mut world_cache.storage, None, + &copies, "myws", Insistence::Insisted, Persistence::Ordinary, @@ -4722,12 +4874,14 @@ mod tests { let mut context = CommandContext::new(&world.devpod); let mut refresh = Refresh::new(&world.updater, &world.cache_path); + let copies = world_cache.copies(); workspace_delete( &mut context, &mut refresh, &clones, &mut world_cache.storage, None, + &copies, "myws", Insistence::Insisted, Persistence::Wedged, @@ -4768,12 +4922,14 @@ mod tests { let mut context = CommandContext::new(&world.devpod); let mut refresh = Refresh::new(&world.updater, &world.cache_path); + let copies = world_cache.copies(); workspace_delete( &mut context, &mut refresh, &clones, &mut world_cache.storage, None, + &copies, "myws", Insistence::Insisted, persistence, @@ -4811,12 +4967,14 @@ mod tests { let mut refresh = Refresh::new(&world.updater, &world.cache_path); let mut stalls = 0; + let copies = world_cache.copies(); workspace_delete( &mut context, &mut refresh, &clones, &mut world_cache.storage, None, + &copies, "myws", Insistence::NotInsisted, Persistence::Ordinary, @@ -4846,12 +5004,14 @@ mod tests { let mut refresh = Refresh::new(&world.updater, &world.cache_path); let mut stalls = 0; + let copies = world_cache.copies(); workspace_delete( &mut context, &mut refresh, &clones, &mut world_cache.storage, None, + &copies, "myws", Insistence::NotInsisted, Persistence::Ordinary, @@ -4884,12 +5044,14 @@ mod tests { let mut context = CommandContext::new(&world.devpod); let mut refresh = Refresh::new(&world.updater, &world.cache_path); + let copies = world_cache.copies(); let outcome = workspace_delete( &mut context, &mut refresh, &clones, &mut world_cache.storage, None, + &copies, "myws", Insistence::Insisted, Persistence::Wedged, @@ -4917,12 +5079,14 @@ mod tests { let mut context = CommandContext::new(&world.devpod); let mut refresh = Refresh::new(&world.updater, &world.cache_path); + let copies = world_cache.copies(); let outcome = workspace_delete( &mut context, &mut refresh, &clones, &mut world_cache.storage, None, + &copies, "myws", Insistence::Insisted, Persistence::Wedged, @@ -4951,12 +5115,14 @@ mod tests { let mut context = CommandContext::new(&world.devpod); let mut refresh = Refresh::new(&updater, &cache_path); + let copies = world.copies(); let outcome = workspace_delete( &mut context, &mut refresh, &clones, &mut world.storage, None, + &copies, "r-main-aa", Insistence::NotInsisted, Persistence::Ordinary, @@ -4991,12 +5157,14 @@ mod tests { let mut refresh = Refresh::new(&updater, &cache_path); let mut notices = Vec::new(); + let copies = world.copies(); let outcome = workspace_delete( &mut context, &mut refresh, &clones, &mut world.storage, None, + &copies, "r-main-aa", Insistence::NotInsisted, Persistence::Ordinary, @@ -5039,12 +5207,14 @@ mod tests { let mut refresh = Refresh::new(&updater, &cache_path); let mut notices = Vec::new(); + let copies = world.copies(); let outcome = workspace_delete( &mut context, &mut refresh, &clones, &mut world.storage, None, + &copies, "r-main-aa", Insistence::NotInsisted, Persistence::Ordinary, @@ -5150,12 +5320,14 @@ mod tests { let mut context = CommandContext::new(&self.world.devpod); let mut refresh = Refresh::new(&self.updater, &self.cache_path); let mut notices = Vec::new(); + let copies = self.world.copies(); let outcome = workspace_delete( &mut context, &mut refresh, &clones, &mut self.world.storage, Some(&self.home), + &copies, "r-main-aa", Insistence::NotInsisted, Persistence::Ordinary, @@ -5301,6 +5473,7 @@ mod tests { assert!( notices.contains(&LifecycleNotice::VolumesNotRemoved { workspace_id: "r-main-aa".to_owned(), + occasion: SweepOccasion::DevpodResult, refusal, }), "{notices:?}" @@ -5366,58 +5539,11 @@ mod tests { } } - /// Each name is built from its own recorded field, so a devpod that recorded - /// one and not the other still gets the volume it can name. Both spellings are - /// devpod's own, which is why they are written out here rather than assembled. - #[test] - fn each_recorded_substitution_names_its_own_volume() { - let both = Substitutions { - local_workspace_folder: Some("/host/clones/repo-branch-abcd".to_owned()), - devcontainer_id: Some("f00d".to_owned()), - }; - assert_eq!( - both.volume_names(), - ["repo-branch-abcd-pixi", "dind-var-lib-docker-f00d"] - ); - - let no_id = Substitutions { - devcontainer_id: None, - ..both - }; - assert_eq!(no_id.volume_names(), ["repo-branch-abcd-pixi"]); - - // A blank field names nothing: `-pixi` and `dind-var-lib-docker-` are not - // volumes anybody meant, and asking docker about them is asking about - // somebody else's disk. - let blank = parse_substitutions( - serde_json::json!({ - "SubstitutionContext": { "LocalWorkspaceFolder": "", "DevContainerID": "" }, - }) - .to_string() - .as_bytes(), - ); - assert_eq!(blank.volume_names(), Vec::::new()); - } - - /// A create result devlaunch cannot read removes no volumes, and says nothing - /// about it: that is exactly the behaviour of every build before this existed, - /// and it is not worth a diagnostic on a delete that otherwise worked. - #[test] - fn a_create_result_that_is_not_the_expected_shape_names_nothing() { - for bytes in [ - &b"not json at all"[..], - b"[]", - br#"{"SubstitutionContext": "a string"}"#, - b"{}", - ] { - assert_eq!( - parse_substitutions(bytes), - Substitutions::default(), - "{}", - String::from_utf8_lossy(bytes) - ); - } - } + // The two tests that used to sit here — each recorded substitution naming its + // own volume, and a create result of another shape naming nothing — moved with + // the parse itself into `flows::kept_copies`, which is now the one module that + // turns a devpod record into a volume name. There is one parser, so the live + // read at delete time and the kept copy's read agree by construction. // ======================================================================= // the delete guard @@ -6829,6 +6955,7 @@ mod tests { &clones, &storage, &workspaces, + &KeptCopies::under(dir.path()), &placement, Insistence::NotInsisted, &mut ignoring(), @@ -6993,10 +7120,12 @@ mod tests { let clones = clones_for(&four.world.repos_dir, &four.world.devpod); let mut context = CommandContext::new(&four.world.devpod); + let copies = four.world.copies(); let outcome = prune_clones( &mut context, &clones, &mut four.world.storage, + &copies, &plan, &mut ignoring(), ) @@ -7032,10 +7161,12 @@ mod tests { let clones = clones_for(&four.world.repos_dir, &four.world.devpod); let mut context = CommandContext::new(&four.world.devpod); + let copies = four.world.copies(); let outcome = prune_clones( &mut context, &clones, &mut four.world.storage, + &copies, &plan, &mut ignoring(), ) @@ -7073,10 +7204,12 @@ mod tests { let clones = clones_for(&four.world.repos_dir, &four.world.devpod); let mut context = CommandContext::new(&four.world.devpod); + let copies = four.world.copies(); let outcome = prune_clones( &mut context, &clones, &mut four.world.storage, + &copies, &plan, &mut ignoring(), ) @@ -7107,10 +7240,12 @@ mod tests { let clones = clones_for(&world.repos_dir, &world.devpod); let mut context = CommandContext::new(&world.devpod); + let copies = world.copies(); let outcome = prune_clones( &mut context, &clones, &mut world.storage, + &copies, &plan, &mut ignoring(), ) @@ -7149,10 +7284,12 @@ mod tests { let clones = clones_for(&four.world.repos_dir, &four.world.devpod); let mut context = CommandContext::new(&four.world.devpod); + let copies = four.world.copies(); let outcome = prune_clones( &mut context, &clones, &mut four.world.storage, + &copies, &plan, &mut ignoring(), ) @@ -7172,10 +7309,12 @@ mod tests { let clones = clones_for(&four.world.repos_dir, &four.world.devpod); { let mut context = CommandContext::new(&four.world.devpod); + let copies = four.world.copies(); prune_clones( &mut context, &clones, &mut four.world.storage, + &copies, &plan, &mut ignoring(), ) @@ -7198,10 +7337,12 @@ mod tests { let clones = clones_for(&four.world.repos_dir, &four.world.devpod); let mut context = CommandContext::new(&four.world.devpod); + let copies = four.world.copies(); prune_clones( &mut context, &clones, &mut four.world.storage, + &copies, &plan, &mut ignoring(), ) @@ -7217,6 +7358,347 @@ mod tests { ); } + // ======================================================================= + // prune: reclaiming from devlaunch's kept copy (devlaunch#456) + // ======================================================================= + + /// devlaunch's kept copy of what devpod substituted for `workspace_id`, written + /// the way a completed `up` writes it and then left without the devpod record + /// it came from. + /// + /// The scratch devpod home is dropped before this returns, which is the whole + /// point: what a bare `devpod delete` outside `dl` leaves behind is a copy in + /// devlaunch's cache and no record anywhere else. + fn a_copy_of(world: &World, workspace_id: &str, folder: &str, devcontainer_id: &str) { + let home = devpod_home_recording(workspace_id, folder, devcontainer_id); + world.copies().keep(workspace_id, Some(&home)); + drop(home); + } + + /// What the plan would reclaim, as `(workspace, names)` pairs. + fn reclaiming(plan: &PrunePlan) -> Vec<(String, Vec)> { + plan.reclaiming() + .iter() + .map(|it| { + ( + it.workspace_id.clone(), + it.names.iter().cloned().collect::>(), + ) + }) + .collect() + } + + /// The plan enumerates the **copies**, not the clone directories, and a copy + /// carries its own names — so the plan and the acting pass cannot disagree + /// about which volumes belonged to which entry. + #[test] + fn a_prune_plans_the_volumes_of_a_workspace_devpod_has_forgotten() { + let world = World::empty(); + a_copy_of(&world, "r-main-aa", "/repos/o/r/r-main-aa", "abcdef"); + + let plan = plan_for(&world, Insistence::NotInsisted); + + assert_eq!( + reclaiming(&plan), + [( + "r-main-aa".to_owned(), + vec![ + "r-main-aa-pixi".to_owned(), + "dind-var-lib-docker-abcdef".to_owned() + ] + )] + ); + assert!( + !plan.nothing_to_do(), + "a run whose only work is the volumes" + ); + } + + /// The precondition, per copy: no workspace devpod lists carries that id. A + /// live workspace's volumes are not leftovers, and removing them would be the + /// worst thing in this file. + #[test] + fn a_prune_plans_no_volumes_for_a_workspace_devpod_still_lists() { + let world = World::empty(); + a_copy_of(&world, "r-main-aa", "/repos/o/r/r-main-aa", "abcdef"); + let clone = world.repo_dir.join("r-main-aa"); + std::fs::create_dir_all(&clone).expect("a clone directory"); + world.devpod.lists(&[listed("r-main-aa", &clone)]); + + let plan = plan_for(&world, Insistence::NotInsisted); + + assert_eq!(reclaiming(&plan), []); + assert!(plan.nothing_to_do()); + } + + /// The map's hard constraint, and this is what makes it hold by construction + /// rather than by a guard: a run pointed at a scratch cache reads copies that + /// are not there, so it names no volume and runs no docker command at all. + #[test] + fn a_prune_over_a_cache_with_no_copies_names_no_volume() { + let mut world = World::empty(); + let plan = plan_for(&world, Insistence::NotInsisted); + let clones = clones_for(&world.repos_dir, &world.devpod); + let copies = world.copies(); + let mut context = CommandContext::new(&world.devpod); + + prune_clones( + &mut context, + &clones, + &mut world.storage, + &copies, + &plan, + &mut ignoring(), + ) + .expect("the pass ran"); + + assert_eq!(reclaiming(&plan), []); + assert_eq!(world.devpod.docker_argvs(), Vec::>::new()); + } + + /// The reason the domain is the set of copies and not a clone walk + /// (devlaunch#445): a copy whose clone the user deleted by hand names volumes no + /// clone-shaped enumeration will ever reach. + #[test] + fn a_copy_whose_clone_was_deleted_by_hand_is_still_in_the_plan() { + let world = World::empty(); + a_copy_of(&world, "r-main-aa", "/repos/o/r/r-main-aa", "abcdef"); + assert!( + !world.repo_dir.join("r-main-aa").exists(), + "no clone directory for this copy at all" + ); + + let plan = plan_for(&world, Insistence::NotInsisted); + + assert_eq!( + reclaiming(&plan) + .into_iter() + .map(|(id, _)| id) + .collect::>(), + ["r-main-aa"] + ); + } + + /// **The whole regression in one run.** A workspace deleted by a bare `devpod + /// delete` outside `dl` leaves the volumes and no record; pruned, both volumes + /// go, in one `docker volume rm --force`, and the copy that named them is + /// dropped because the removal proved it pointless. + #[test] + fn a_prune_reclaims_the_volumes_of_a_workspace_devpod_forgot_and_drops_the_copy() { + let mut world = World::empty(); + a_copy_of(&world, "r-main-aa", "/repos/o/r/r-main-aa", "abcdef"); + let plan = plan_for(&world, Insistence::NotInsisted); + let clones = clones_for(&world.repos_dir, &world.devpod); + let copies = world.copies(); + let mut context = CommandContext::new(&world.devpod); + + let outcome = prune_clones( + &mut context, + &clones, + &mut world.storage, + &copies, + &plan, + &mut ignoring(), + ) + .expect("the pass ran"); + + let PruneOutcome::Acted(report) = &outcome else { + panic!("expected the pass to act, got {outcome:?}"); + }; + assert_eq!( + world.devpod.docker_argvs(), + [[ + "volume", + "rm", + "--force", + "r-main-aa-pixi", + "dind-var-lib-docker-abcdef" + ]] + ); + assert_eq!( + report + .reclaimed + .iter() + .map(|it| it.workspace_id.clone()) + .collect::>(), + ["r-main-aa"] + ); + assert_eq!( + world.copies().copied(), + Vec::::new(), + "the copy is dropped on proof" + ); + } + + /// The first of the two ways a copy can be wrong, and docker is what catches + /// it: a volume some container still holds is refused, nothing is removed, and + /// the copy is **kept** so the retry survives. + #[test] + fn a_copy_naming_a_held_volume_is_a_reported_refusal_and_the_copy_stays() { + let mut world = World::empty(); + a_copy_of(&world, "r-main-aa", "/repos/o/r/r-main-aa", "abcdef"); + world.devpod.fake.script( + ["docker", "volume", "rm"], + Response::failed(1, "volume is in use\n"), + ); + let plan = plan_for(&world, Insistence::NotInsisted); + let clones = clones_for(&world.repos_dir, &world.devpod); + let copies = world.copies(); + let mut context = CommandContext::new(&world.devpod); + + let outcome = prune_clones( + &mut context, + &clones, + &mut world.storage, + &copies, + &plan, + &mut ignoring(), + ) + .expect("the pass ran"); + + let PruneOutcome::Acted(report) = &outcome else { + panic!("expected the pass to act, got {outcome:?}"); + }; + assert!(report.reclaimed.is_empty()); + assert_eq!( + report.volumes_kept, + [VolumesKept { + workspace_id: "r-main-aa".to_owned(), + because: VolumesKeptBecause::Refused(VolumeRefusal::Docker { + exit: Exit::Code(1), + stderr: "volume is in use\n".to_owned(), + }), + }] + ); + assert_eq!( + world.copies().copied(), + ["r-main-aa"], + "kept on a refusal, so the retry stays possible" + ); + } + + /// The precondition is re-asked under the second listing the acting pass + /// already pays for. A workspace that came back between the plan and the act is + /// a live workspace again, and its volumes are not leftovers. + #[test] + fn a_workspace_back_in_the_listing_between_the_plan_and_the_act_keeps_its_volumes() { + let mut world = World::empty(); + a_copy_of(&world, "r-main-aa", "/repos/o/r/r-main-aa", "abcdef"); + let plan = plan_for(&world, Insistence::NotInsisted); + assert_eq!(reclaiming(&plan).len(), 1); + let clone = world.repo_dir.join("r-main-aa"); + std::fs::create_dir_all(&clone).expect("a clone directory"); + world.devpod.lists(&[listed("r-main-aa", &clone)]); + let clones = clones_for(&world.repos_dir, &world.devpod); + let copies = world.copies(); + let mut context = CommandContext::new(&world.devpod); + + let outcome = prune_clones( + &mut context, + &clones, + &mut world.storage, + &copies, + &plan, + &mut ignoring(), + ) + .expect("the pass ran"); + + let PruneOutcome::Acted(report) = &outcome else { + panic!("expected the pass to act, got {outcome:?}"); + }; + assert_eq!(world.devpod.docker_argvs(), Vec::>::new()); + assert!(report.reclaimed.is_empty()); + assert_eq!( + report.volumes_kept, + [VolumesKept { + workspace_id: "r-main-aa".to_owned(), + because: VolumesKeptBecause::ListedAgain, + }] + ); + assert_eq!(world.copies().copied(), ["r-main-aa"]); + } + + /// A host with no docker never made these volumes, so there is nothing here to + /// have failed and nothing to say — the same silence the delete path keeps. + /// The copy stays, because nothing proved it pointless. + #[test] + fn a_prune_on_a_machine_with_no_docker_says_nothing_about_the_volumes() { + let mut world = World::empty(); + a_copy_of(&world, "r-main-aa", "/repos/o/r/r-main-aa", "abcdef"); + world.devpod.fake.script_missing("docker"); + let plan = plan_for(&world, Insistence::NotInsisted); + let clones = clones_for(&world.repos_dir, &world.devpod); + let copies = world.copies(); + let mut context = CommandContext::new(&world.devpod); + + let outcome = prune_clones( + &mut context, + &clones, + &mut world.storage, + &copies, + &plan, + &mut ignoring(), + ) + .expect("the pass ran"); + + let PruneOutcome::Acted(report) = &outcome else { + panic!("expected the pass to act, got {outcome:?}"); + }; + assert!(report.reclaimed.is_empty()); + assert!(report.volumes_kept.is_empty()); + assert_eq!(world.copies().copied(), ["r-main-aa"]); + } + + /// A delete has already swept these volumes from devpod's own live record, so + /// the copy that named them is pointless — dropped on the same proof the + /// reclaim drops one on, and for the same reason. Left standing, it would have + /// the next `--prune` report reclaiming volumes that went with the workspace. + #[test] + fn a_delete_drops_the_copy_of_the_volumes_it_swept() { + let mut deleting = a_world_ready_to_delete(); + a_copy_of( + &deleting.world, + "r-main-aa", + "/host/clones/opened-as", + "dc9a8b7c", + ); + + let (outcome, _) = deleting.delete(); + + assert!( + matches!( + outcome, + DeleteOutcome::Deleted { + volumes: VolumeSweep::Removed, + .. + } + ), + "{outcome:?}" + ); + assert_eq!(deleting.world.copies().copied(), Vec::::new()); + } + + /// Kept on a refusal here too, and for the reclaim's reason: the volumes are + /// still on the machine, so the copy is still the only thing that names them. + #[test] + fn a_delete_docker_refused_keeps_the_copy() { + let mut deleting = a_world_ready_to_delete(); + a_copy_of( + &deleting.world, + "r-main-aa", + "/host/clones/opened-as", + "dc9a8b7c", + ); + deleting.world.devpod.fake.script( + ["docker", "volume", "rm"], + Response::failed(1, "volume is in use\n"), + ); + + deleting.delete(); + + assert_eq!(deleting.world.copies().copied(), ["r-main-aa"]); + } + // ======================================================================= // reconcile (devlaunch#88) // ======================================================================= diff --git a/rust/devlaunch-core/src/flows/mod.rs b/rust/devlaunch-core/src/flows/mod.rs index 11009a3b..95c5b0ff 100644 --- a/rust/devlaunch-core/src/flows/mod.rs +++ b/rust/devlaunch-core/src/flows/mod.rs @@ -8,6 +8,8 @@ pub mod completion_cache; // binary surface — not part of the frozen wf API (#251 §7) pub mod disk_usage; // binary surface — not part of the frozen wf API (#251 §7) +pub mod kept_copies; +// binary surface — not part of the frozen wf API (#251 §7) pub mod kill; // binary surface — not part of the frozen wf API (#251 §7) pub mod launch; diff --git a/rust/devlaunch-core/tests/volume_names.rs b/rust/devlaunch-core/tests/volume_names.rs new file mode 100644 index 00000000..bdf623dd --- /dev/null +++ b/rust/devlaunch-core/tests/volume_names.rs @@ -0,0 +1,134 @@ +//! Where a docker volume name can come from, and where it can be removed. +//! +//! devlaunch#325's rule is that **no volume name is ever synthesized from a +//! pattern**: every name handed to `docker volume rm` originates in a substitution +//! devpod performed and wrote down. devlaunch#451 asked for a test that no removal +//! is issued for a name from anywhere else, and asked for it *by construction* — +//! the absence of a path rather than a guard firing at runtime. +//! +//! That is what this is. Two spellings and one call site: +//! +//! - The two name **templates** — `-pixi` and +//! `dind-var-lib-docker-` — are written in `flows::kept_copies` +//! and nowhere else, so a name can only be built by +//! `Substitutions::volume_names`, and a `Substitutions` can only be built by +//! parsing bytes devpod wrote or devlaunch copied. There is no constructor for an +//! inferred name to reach. +//! - `clients::docker::remove_volumes` is *called* from exactly one place, +//! `flows::lifecycle::sweep_volumes`, so there is one door and it takes the names +//! those two reads produce. +//! +//! Neither half is a behaviour test; both are guards, in `tests/devpod_layout.rs`'s +//! shape and for its reason. The failure they catch is not a wrong answer — it is a +//! second way to build a name, which agrees with the first until somebody points it +//! at `docker volume ls`. + +use std::path::{Path, PathBuf}; + +/// The module allowed to spell a volume name's template. +const NAMER: &str = "flows/kept_copies.rs"; + +/// How a volume name gets built: the format templates themselves, with their +/// interpolation braces, so the whole names a *test fixture* writes out +/// (`"repo-main-ab12-pixi"`) do not match. A fixture asserting what devlaunch asked +/// docker for is the assertion, and routing it through the code under test is what +/// this guard is for elsewhere. +const TEMPLATES: [&str; 2] = ["{basename}-pixi", "dind-var-lib-docker-{"]; + +/// The one function that can remove a volume, and the one module that may call it. +const REMOVAL: &str = "remove_volumes("; +const DOOR: &str = "flows/lifecycle.rs"; +const DOORWAY: &str = "clients/docker.rs"; + +fn crate_src() -> PathBuf { + Path::new(env!("CARGO_MANIFEST_DIR")).join("src") +} + +/// Every `.rs` file under `src`, relative to it, sorted. +fn sources(root: &Path) -> Vec { + let mut found = Vec::new(); + walk(root, root, &mut found); + found.sort(); + found +} + +fn walk(root: &Path, dir: &Path, found: &mut Vec) { + for entry in std::fs::read_dir(dir).expect("a readable source directory") { + let path = entry.expect("a readable directory entry").path(); + if path.is_dir() { + walk(root, &path, found); + } else if path.extension().is_some_and(|ext| ext == "rs") { + found.push( + path.strip_prefix(root) + .expect("a path under the source root") + .to_path_buf(), + ); + } + } +} + +/// Which files hold `needles`, and on which lines. +fn mentions(root: &Path, needles: &[&str]) -> Vec<(PathBuf, usize)> { + let mut found = Vec::new(); + for relative in sources(root) { + let text = std::fs::read_to_string(root.join(&relative)).expect("a readable source file"); + for (index, line) in text.lines().enumerate() { + if needles.iter().any(|needle| line.contains(needle)) { + found.push((relative.clone(), index + 1)); + } + } + } + found +} + +#[test] +fn only_one_module_can_build_a_volume_name() { + let root = crate_src(); + let strays: Vec = mentions(&root, &TEMPLATES) + .into_iter() + .filter(|(file, _)| file != Path::new(NAMER)) + .map(|(file, line)| format!(" src/{}:{line}", file.display())) + .collect(); + assert!( + strays.is_empty(), + "a volume name is built outside {NAMER}, so there is now more than one way to \ + make one:\n{}", + strays.join("\n") + ); +} + +/// The other half, so the guard cannot pass by the templates having moved out of +/// the crate entirely — which is how a guard like this quietly stops guarding. +#[test] +fn that_module_does_build_them() { + let root = crate_src(); + let inside = mentions(&root, &TEMPLATES) + .into_iter() + .filter(|(file, _)| file == Path::new(NAMER)) + .count(); + assert!( + inside > 0, + "no module builds a volume name, so this guard is guarding nothing" + ); +} + +/// One door to `docker volume rm`, so every name that reaches docker went through +/// the one function that turns docker's answer into a verdict. +/// +/// Both reads — devpod's live create result at delete time, and devlaunch's kept +/// copy at prune time — arrive at that same door, which is what makes "no name from +/// anywhere else" checkable at all. +#[test] +fn only_one_module_can_remove_a_volume() { + let root = crate_src(); + let strays: Vec = mentions(&root, &[REMOVAL]) + .into_iter() + .filter(|(file, _)| file != Path::new(DOOR) && file != Path::new(DOORWAY)) + .map(|(file, line)| format!(" src/{}:{line}", file.display())) + .collect(); + assert!( + strays.is_empty(), + "`docker volume rm` is reachable outside {DOOR}:\n{}", + strays.join("\n") + ); +} diff --git a/rust/dl/src/commands.rs b/rust/dl/src/commands.rs index c0ddc7f1..dc47493f 100644 --- a/rust/dl/src/commands.rs +++ b/rust/dl/src/commands.rs @@ -16,6 +16,7 @@ use devlaunch_core::domain::workspace_id::WorkspaceId; use devlaunch_core::domain::xdg; use devlaunch_core::flows::completion::{self, FileState, InstallError, Installed, RcChange}; use devlaunch_core::flows::completion_cache::{self, Refreshed}; +use devlaunch_core::flows::kept_copies::KeptCopies; use devlaunch_core::flows::kill; use devlaunch_core::flows::launch::LaunchNotice; use devlaunch_core::flows::lifecycle::{ @@ -109,7 +110,7 @@ pub(crate) fn dispatch( Command::UpdateCache { force } => render_update_cache(runner, &mut context, cache, force), Command::Refresh => render_refresh(&mut context, cache), Command::Install { rc } => render_install(&mut context, cache, rc.as_deref()), - Command::Prune { yes, force } => render_prune(runner, &mut context, yes, force), + Command::Prune { yes, force } => render_prune(runner, &mut context, cache, yes, force), Command::Reconcile { yes } => render_reconcile(runner, &mut context, refresh, yes), Command::Purge { yes } => render_purge(&mut context, cache, yes), // The two arms that name a verb are the two that can be `rme`, and the @@ -1018,6 +1019,9 @@ fn remove_addressed<'r>( // answer down. `None` is a machine with no home directory, where devpod has // no records to read and so no volume names to derive. DevpodHome::locate().as_ref(), + // The same cache directory everything else here hangs off, so the copy this + // delete drops is the one a launch of this workspace wrote. + &KeptCopies::under(cache), workspace_id, insistence, removal.persistence(), @@ -1176,15 +1180,17 @@ fn announce_lock_waits(records: &mut Records<'_>) { fn render_prune( runner: &dyn Runner, context: &mut CommandContext<'_>, + cache: &Path, yes: bool, force: bool, ) -> Ending { - prune_clone_directories(runner, context, yes, force).with_the_boundary() + prune_clone_directories(runner, context, cache, yes, force).with_the_boundary() } fn prune_clone_directories( runner: &dyn Runner, context: &mut CommandContext<'_>, + cache: &Path, yes: bool, force: bool, ) -> Cleanup { @@ -1213,10 +1219,15 @@ fn prune_clone_directories( Insistence::NotInsisted }; let mut notices: Vec = Vec::new(); + // devlaunch's copies of what devpod substituted, under the cache the binary + // already resolved. This is what makes a run pointed at a scratch + // `XDG_CACHE_HOME` find no copies and so remove no volume. + let copies = KeptCopies::under(cache); let plan = match lifecycle::prune_plan( &records.clones, &records.storage, &workspaces, + &copies, &placement, insistence, &mut notices, @@ -1237,7 +1248,7 @@ fn prune_clone_directories( let Records { storage, clones, .. } = &mut records; - let acted = lifecycle::prune_clones(context, clones, storage, &plan, &mut notices); + let acted = lifecycle::prune_clones(context, clones, storage, &copies, &plan, &mut notices); say(¬ices); match acted { Err(refused) => Cleanup::Raised(refuse_prune(&refused)), diff --git a/rust/dl/src/render.rs b/rust/dl/src/render.rs index 5db68258..23c9f189 100644 --- a/rust/dl/src/render.rs +++ b/rust/dl/src/render.rs @@ -32,8 +32,8 @@ use devlaunch_core::flows::launch::{ }; use devlaunch_core::flows::lifecycle::{ Insistence, KeptBecause, LifecycleNotice, NotAdopted, Objection, Promotion, PrunePlan, - PruneReport, PurgeOutcome, PurgePlan, PurgeStep, ReconcilePlan, RemovalRefused, Unlocatable, - VolumeRefusal, + PruneReport, PurgeOutcome, PurgePlan, PurgeStep, ReconcilePlan, RemovalRefused, SweepOccasion, + Unlocatable, VolumeRefusal, VolumesKeptBecause, }; use devlaunch_core::flows::listing::{LastUsed, SizeCell, Sizes, TableRow, WorkspaceTable}; use devlaunch_core::flows::migration::{Listing, MigrationReport}; @@ -914,8 +914,9 @@ fn lifecycle_notice(notice: &LifecycleNotice) -> Option { } LifecycleNotice::VolumesNotRemoved { workspace_id, + occasion, refusal, - } => volumes_not_removed(workspace_id, refusal), + } => volumes_not_removed(workspace_id, *occasion, refusal), LifecycleNotice::RecordNotDropped { path, refusal } => { format!( "Could not drop the record for {}: {}", @@ -1126,13 +1127,34 @@ fn not_removed(refusal: &RemoveWorkspaceError) -> String { /// a notice and `--purge` as a step, the vocabularies differ but the sentence does /// not, and two copies each pinned by its own test is how they come to differ by a /// word. -fn volumes_not_removed(workspace_id: &str, refusal: &VolumeRefusal) -> String { +fn volumes_not_removed( + workspace_id: &str, + occasion: SweepOccasion, + refusal: &VolumeRefusal, +) -> String { format!( - "Failed to remove the Docker volumes for {workspace_id}: {}", + "Failed to remove the {} for {workspace_id}: {}", + named_volumes(occasion), volumes_refused(refusal) ) } +/// Which read named the volumes a sweep was about, as the noun phrase in the +/// sentence about them. +/// +/// The one place the two-arm occasion becomes English, and a match rather than a +/// field on the name: a third read would be a compile error here, which is the +/// refusal enforced by exhaustiveness rather than by prose. The wording differs +/// because where to look differs. devpod's record is gone with the workspace; a +/// kept copy is a file under devlaunch's cache, and a volume this could not release +/// is one something on the machine is still holding. +fn named_volumes(occasion: SweepOccasion) -> &'static str { + match occasion { + SweepOccasion::DevpodResult => "Docker volumes", + SweepOccasion::KeptCopy => "Docker volumes devlaunch recorded", + } +} + /// Why a deleted workspace's Docker volumes are still on this machine. /// /// docker's own stderr where docker spoke, trimmed of the newline it ends on @@ -1765,8 +1787,9 @@ pub(crate) fn purge_step(step: &PurgeStep) -> Line { )), PurgeStep::VolumesNotRemoved { workspace_id, + occasion, refusal, - } => Line::Err(volumes_not_removed(workspace_id, refusal)), + } => Line::Err(volumes_not_removed(workspace_id, *occasion, refusal)), } } @@ -1899,6 +1922,28 @@ pub(crate) fn prune_plan_lines(plan: &PrunePlan) -> Vec { } lines.push(String::new()); } + if !plan.reclaiming().is_empty() { + // Every name, not a count. These are volumes, and a volume is not an image: + // deleting one that still matters is data loss rather than a rebuild, so + // what is about to be removed is on the screen before the question is asked. + lines.push(format!( + "Reclaiming the Docker volumes of {} workspace(s) devpod no longer lists:", + plan.reclaiming().len() + )); + for reclaimable in plan.reclaiming() { + lines.push(format!( + " - {}: {}", + reclaimable.workspace_id, + reclaimable + .names + .iter() + .cloned() + .collect::>() + .join(", ") + )); + } + lines.push(String::new()); + } if !plan.stale_records().is_empty() { lines.push(format!( "Dropping {} record(s) of directories already gone.", @@ -1986,6 +2031,27 @@ pub(crate) fn prune_report_lines(report: &PruneReport) -> Vec { kept_because(&withheld.because) )); } + if !report.reclaimed.is_empty() { + lines.push(format!( + "Reclaimed the Docker volumes of {} workspace(s) devpod no longer lists.", + report.reclaimed.len() + )); + } + for kept in &report.volumes_kept { + lines.push(match &kept.because { + // Said in the plan's own words, because the plan is what promised it: + // a workspace back in devpod's listing is the volume half of the + // withheld line above, and for the same reason. + VolumesKeptBecause::ListedAgain => format!( + "Left the Docker volumes of {}: devpod lists that workspace again. That was \ + not so when the plan above was printed.", + kept.workspace_id + ), + VolumesKeptBecause::Refused(refusal) => { + volumes_not_removed(&kept.workspace_id, SweepOccasion::KeptCopy, refusal) + } + }); + } if !report.refused.is_empty() { let by_hand: Vec = report .refused @@ -3723,6 +3789,7 @@ mod tests { assert_eq!( lifecycle_notice(&LifecycleNotice::VolumesNotRemoved { workspace_id: "ws".to_owned(), + occasion: SweepOccasion::DevpodResult, refusal: VolumeRefusal::Docker { exit: Exit::Code(1), stderr: "Error response from daemon: remove ws-pixi: volume is in use\n" @@ -3740,6 +3807,7 @@ mod tests { assert_eq!( lifecycle_notice(&LifecycleNotice::VolumesNotRemoved { workspace_id: "ws".to_owned(), + occasion: SweepOccasion::DevpodResult, refusal: VolumeRefusal::Docker { exit: Exit::Signal(9), stderr: String::new(), @@ -3750,6 +3818,7 @@ mod tests { assert_eq!( lifecycle_notice(&LifecycleNotice::VolumesNotRemoved { workspace_id: "ws".to_owned(), + occasion: SweepOccasion::DevpodResult, refusal: VolumeRefusal::NotRun { failure: OsFailure { kind: std::io::ErrorKind::PermissionDenied, @@ -3773,6 +3842,7 @@ mod tests { assert_eq!( purge_step(&PurgeStep::VolumesNotRemoved { workspace_id: "ws".to_owned(), + occasion: SweepOccasion::DevpodResult, refusal: VolumeRefusal::Docker { exit: Exit::Code(1), stderr: "volume is in use\n".to_owned(), From 084c570bb6a4a96d6a623a91b2702ef8bdf868b6 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sat, 29 Aug 2026 19:07:39 +0100 Subject: [PATCH 2/3] docs: say where the copy lives, and what it does not reach `docs/cleanup.md`'s "the `--prune` half of the pair still frees no volume at all" is rewritten rather than deleted: prune still deletes no workspace, and because it deletes none, the volumes it reclaims belong to workspaces something else already deleted. The new section beside it says where the copy lives, what makes it safe to be wrong, and that the 39 measured orphans stay. `docs/development.md` gains the sentence the scratch-cache convention needs: a scratch run that creates a workspace writes its copy into the cache it then throws away. `public-api.rest.txt` is hand-edited. This container has neither prerequisite `scripts/public-api-snapshots.sh` needs, which docs/development.md names as exactly this case: provisional until CI's public-api job agrees. Nothing promised moved, so `public-api.api.txt` is untouched. --- CHANGELOG.md | 38 ++++++++++++++ README.md | 4 +- rust/devlaunch-core/public-api.rest.txt | 68 +++++++++++++++++++++++-- 3 files changed, 105 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index cf5049ca..963a674a 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] +### Added + +- **`dl --prune` now reclaims the Docker volumes of workspaces devpod has already + forgotten.** Deleting a workspace through `dl` has removed its two named volumes + since devlaunch#325, and the names come from devpod's own record of what it + substituted, read at delete time and never guessed from a pattern. That leaves + one hole, and it is the one that was measured: a workspace deleted by a bare + `devpod delete` outside `dl`, which removes devpod's record and leaves the + volumes. After that there is nothing on the machine that names them. + + So `dl` keeps its own copy. At the tail of every `up` that completed, it reads + the same two fields out of the same `workspace_result.json` and writes them to a + small per workspace file under its cache, beside the tool verdict markers. A + prune reclaims from those copies, and the whole of what it asks per copy is + whether any workspace `devpod list` returns carries that id. Where none does, + the workspace is gone and its volumes are leftovers. + + Nothing is invented at either read, which is the point: every name that reaches + `docker volume rm` still came out of a substitution devpod performed and wrote + down. Matching `-pixi` and `dind-var-lib-docker-` against `docker + volume ls` stays refused, because the `docker-in-docker` feature writes that + second name in every devcontainer tool that runs it, so the candidates `dl` + cannot attribute are exactly the ones belonging to somebody else. + + A copy can still be wrong, in exactly two ways, and neither is answered by + believing the file. It can name a volume that is already gone, which `docker + volume rm --force` treats as a success. It can name one something else now + holds, which Docker refuses with `volume is in use`, reported, nothing removed, + and the copy kept so the retry is still there. A copy is dropped once, when a + removal came back removed for a workspace devpod does not list. + + Two things follow. A run pointed at a scratch `XDG_CACHE_HOME` finds no copies, + so it names no volume and removes none, which is what makes the scratch + convention safe here by construction. And the 39 orphaned volumes holding 37.28 + GB measured on the reference host are **out of scope and stay**: their records + died before any of this existed, and no route reaches them that is not the + pattern above. `docs/cleanup.md` carries the whole of it. + ## [0.25.0] - 2026-08-28 ### Fixed diff --git a/README.md b/README.md index 9798c205..6b6faca7 100644 --- a/README.md +++ b/README.md @@ -253,7 +253,7 @@ instead. [docs/cli.md](docs/cli.md) has the full `--rm` contract, including whic | `dl --ls` | List every workspace | | `dl --ls --json` | The same, machine-readable, with what each workspace would lose if deleted | | `dl --ls --size` | Add what deleting each one would free. Opt-in: it walks every file | -| `dl --prune` | Remove the clone directories no workspace opens any more | +| `dl --prune` | Remove the clone directories no workspace opens any more, and reclaim the volumes of workspaces devpod has forgotten | | `dl --reconcile` | Re-point workspaces whose recorded source folder went missing. Deletes nothing | | `dl --purge` | Remove devlaunch's own workspaces and caches | | `dl --install` | Install shell completions | @@ -368,7 +368,7 @@ different jobs: | Command | Takes | Leaves | |---|---|---| -| `dl --prune` | Clone directories no workspace opens | Every workspace, container, image and volume | +| `dl --prune` | Clone directories no workspace opens, and the volumes of workspaces devpod no longer lists | Every workspace, container and image | | `dl --purge` | The workspaces devlaunch created, and its caches | Workspaces it did not create, named before it asks | | `dl --reconcile` | Nothing | Repairs records that stopped matching the disk | diff --git a/rust/devlaunch-core/public-api.rest.txt b/rust/devlaunch-core/public-api.rest.txt index 5a480c85..57ce78c4 100644 --- a/rust/devlaunch-core/public-api.rest.txt +++ b/rust/devlaunch-core/public-api.rest.txt @@ -839,6 +839,18 @@ impl core::fmt::Debug for devlaunch_core::flows::disk_usage::DiskUsage pub fn devlaunch_core::flows::disk_usage::DiskUsage::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::disk_usage::DiskUsage pub fn devlaunch_core::flows::disk_usage::describe_usage(&devlaunch_core::flows::disk_usage::DiskUsage) -> alloc::string::String +pub mod devlaunch_core::flows::kept_copies +pub struct devlaunch_core::flows::kept_copies::KeptCopies +impl devlaunch_core::flows::kept_copies::KeptCopies +pub fn devlaunch_core::flows::kept_copies::KeptCopies::under(&std::path::Path) -> Self +impl core::clone::Clone for devlaunch_core::flows::kept_copies::KeptCopies +pub fn devlaunch_core::flows::kept_copies::KeptCopies::clone(&self) -> devlaunch_core::flows::kept_copies::KeptCopies +impl core::cmp::Eq for devlaunch_core::flows::kept_copies::KeptCopies +impl core::cmp::PartialEq for devlaunch_core::flows::kept_copies::KeptCopies +pub fn devlaunch_core::flows::kept_copies::KeptCopies::eq(&self, &devlaunch_core::flows::kept_copies::KeptCopies) -> bool +impl core::fmt::Debug for devlaunch_core::flows::kept_copies::KeptCopies +pub fn devlaunch_core::flows::kept_copies::KeptCopies::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::kept_copies::KeptCopies pub mod devlaunch_core::flows::kill pub enum devlaunch_core::flows::kill::ContainerRefusal pub devlaunch_core::flows::kill::ContainerRefusal::NotRun @@ -1408,6 +1420,7 @@ pub devlaunch_core::flows::lifecycle::LifecycleNotice::RecordNotDropped pub devlaunch_core::flows::lifecycle::LifecycleNotice::RecordNotDropped::path: std::path::PathBuf pub devlaunch_core::flows::lifecycle::LifecycleNotice::RecordNotDropped::refusal: devlaunch_core::domain::metadata::MetadataError pub devlaunch_core::flows::lifecycle::LifecycleNotice::VolumesNotRemoved +pub devlaunch_core::flows::lifecycle::LifecycleNotice::VolumesNotRemoved::occasion: devlaunch_core::flows::lifecycle::SweepOccasion pub devlaunch_core::flows::lifecycle::LifecycleNotice::VolumesNotRemoved::refusal: devlaunch_core::flows::lifecycle::VolumeRefusal pub devlaunch_core::flows::lifecycle::LifecycleNotice::VolumesNotRemoved::workspace_id: alloc::string::String impl core::clone::Clone for devlaunch_core::flows::lifecycle::LifecycleNotice @@ -1512,6 +1525,7 @@ pub devlaunch_core::flows::lifecycle::PurgeStep::NotDeleted::exit: devlaunch_run pub devlaunch_core::flows::lifecycle::PurgeStep::NotDeleted::stderr: alloc::string::String pub devlaunch_core::flows::lifecycle::PurgeStep::NotDeleted::workspace_id: alloc::string::String pub devlaunch_core::flows::lifecycle::PurgeStep::VolumesNotRemoved +pub devlaunch_core::flows::lifecycle::PurgeStep::VolumesNotRemoved::occasion: devlaunch_core::flows::lifecycle::SweepOccasion pub devlaunch_core::flows::lifecycle::PurgeStep::VolumesNotRemoved::refusal: devlaunch_core::flows::lifecycle::VolumeRefusal pub devlaunch_core::flows::lifecycle::PurgeStep::VolumesNotRemoved::workspace_id: alloc::string::String impl core::clone::Clone for devlaunch_core::flows::lifecycle::PurgeStep @@ -1576,6 +1590,18 @@ impl core::fmt::Debug for devlaunch_core::flows::lifecycle::SpawnRefused pub fn devlaunch_core::flows::lifecycle::SpawnRefused::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::Copy for devlaunch_core::flows::lifecycle::SpawnRefused impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::SpawnRefused +pub enum devlaunch_core::flows::lifecycle::SweepOccasion +pub devlaunch_core::flows::lifecycle::SweepOccasion::DevpodResult +pub devlaunch_core::flows::lifecycle::SweepOccasion::KeptCopy +impl core::clone::Clone for devlaunch_core::flows::lifecycle::SweepOccasion +pub fn devlaunch_core::flows::lifecycle::SweepOccasion::clone(&self) -> devlaunch_core::flows::lifecycle::SweepOccasion +impl core::cmp::Eq for devlaunch_core::flows::lifecycle::SweepOccasion +impl core::cmp::PartialEq for devlaunch_core::flows::lifecycle::SweepOccasion +pub fn devlaunch_core::flows::lifecycle::SweepOccasion::eq(&self, &devlaunch_core::flows::lifecycle::SweepOccasion) -> bool +impl core::fmt::Debug for devlaunch_core::flows::lifecycle::SweepOccasion +pub fn devlaunch_core::flows::lifecycle::SweepOccasion::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Copy for devlaunch_core::flows::lifecycle::SweepOccasion +impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::SweepOccasion pub enum devlaunch_core::flows::lifecycle::StopOutcome pub devlaunch_core::flows::lifecycle::StopOutcome::DevpodRefused pub devlaunch_core::flows::lifecycle::StopOutcome::DevpodRefused::exit: devlaunch_runner::Exit @@ -1616,6 +1642,17 @@ pub fn devlaunch_core::flows::lifecycle::VolumeSweep::eq(&self, &devlaunch_core: impl core::fmt::Debug for devlaunch_core::flows::lifecycle::VolumeSweep pub fn devlaunch_core::flows::lifecycle::VolumeSweep::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::VolumeSweep +pub enum devlaunch_core::flows::lifecycle::VolumesKeptBecause +pub devlaunch_core::flows::lifecycle::VolumesKeptBecause::ListedAgain +pub devlaunch_core::flows::lifecycle::VolumesKeptBecause::Refused(devlaunch_core::flows::lifecycle::VolumeRefusal) +impl core::clone::Clone for devlaunch_core::flows::lifecycle::VolumesKeptBecause +pub fn devlaunch_core::flows::lifecycle::VolumesKeptBecause::clone(&self) -> devlaunch_core::flows::lifecycle::VolumesKeptBecause +impl core::cmp::Eq for devlaunch_core::flows::lifecycle::VolumesKeptBecause +impl core::cmp::PartialEq for devlaunch_core::flows::lifecycle::VolumesKeptBecause +pub fn devlaunch_core::flows::lifecycle::VolumesKeptBecause::eq(&self, &devlaunch_core::flows::lifecycle::VolumesKeptBecause) -> bool +impl core::fmt::Debug for devlaunch_core::flows::lifecycle::VolumesKeptBecause +pub fn devlaunch_core::flows::lifecycle::VolumesKeptBecause::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::VolumesKeptBecause pub struct devlaunch_core::flows::lifecycle::Adoptable pub devlaunch_core::flows::lifecycle::Adoptable::clone: std::path::PathBuf pub devlaunch_core::flows::lifecycle::Adoptable::context: alloc::string::String @@ -1664,6 +1701,7 @@ impl devlaunch_core::flows::lifecycle::PrunePlan pub fn devlaunch_core::flows::lifecycle::PrunePlan::freed(&self) -> devlaunch_core::flows::disk_usage::DiskUsage pub fn devlaunch_core::flows::lifecycle::PrunePlan::keeping(&self) -> &[devlaunch_core::flows::lifecycle::Kept] pub fn devlaunch_core::flows::lifecycle::PrunePlan::nothing_to_do(&self) -> bool +pub fn devlaunch_core::flows::lifecycle::PrunePlan::reclaiming(&self) -> &[devlaunch_core::flows::lifecycle::ReclaimableVolumes] pub fn devlaunch_core::flows::lifecycle::PrunePlan::removing(&self) -> &[devlaunch_core::flows::lifecycle::Reclaimable] pub fn devlaunch_core::flows::lifecycle::PrunePlan::root(&self) -> &std::path::Path pub fn devlaunch_core::flows::lifecycle::PrunePlan::stale_records(&self) -> &[devlaunch_core::domain::model::WorktreeInfo] @@ -1676,8 +1714,10 @@ impl core::fmt::Debug for devlaunch_core::flows::lifecycle::PrunePlan pub fn devlaunch_core::flows::lifecycle::PrunePlan::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::PrunePlan pub struct devlaunch_core::flows::lifecycle::PruneReport +pub devlaunch_core::flows::lifecycle::PruneReport::reclaimed: alloc::vec::Vec pub devlaunch_core::flows::lifecycle::PruneReport::refused: alloc::vec::Vec pub devlaunch_core::flows::lifecycle::PruneReport::removed: alloc::vec::Vec +pub devlaunch_core::flows::lifecycle::PruneReport::volumes_kept: alloc::vec::Vec pub devlaunch_core::flows::lifecycle::PruneReport::withheld: alloc::vec::Vec impl devlaunch_core::flows::lifecycle::PruneReport pub fn devlaunch_core::flows::lifecycle::PruneReport::finished(&self) -> bool @@ -1716,6 +1756,17 @@ pub fn devlaunch_core::flows::lifecycle::Reclaimable::eq(&self, &devlaunch_core: impl core::fmt::Debug for devlaunch_core::flows::lifecycle::Reclaimable pub fn devlaunch_core::flows::lifecycle::Reclaimable::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::Reclaimable +pub struct devlaunch_core::flows::lifecycle::ReclaimableVolumes +pub devlaunch_core::flows::lifecycle::ReclaimableVolumes::names: devlaunch_core::domain::workspace_state::NonEmpty +pub devlaunch_core::flows::lifecycle::ReclaimableVolumes::workspace_id: alloc::string::String +impl core::clone::Clone for devlaunch_core::flows::lifecycle::ReclaimableVolumes +pub fn devlaunch_core::flows::lifecycle::ReclaimableVolumes::clone(&self) -> devlaunch_core::flows::lifecycle::ReclaimableVolumes +impl core::cmp::Eq for devlaunch_core::flows::lifecycle::ReclaimableVolumes +impl core::cmp::PartialEq for devlaunch_core::flows::lifecycle::ReclaimableVolumes +pub fn devlaunch_core::flows::lifecycle::ReclaimableVolumes::eq(&self, &devlaunch_core::flows::lifecycle::ReclaimableVolumes) -> bool +impl core::fmt::Debug for devlaunch_core::flows::lifecycle::ReclaimableVolumes +pub fn devlaunch_core::flows::lifecycle::ReclaimableVolumes::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::ReclaimableVolumes pub struct devlaunch_core::flows::lifecycle::ReconcilePlan impl devlaunch_core::flows::lifecycle::ReconcilePlan pub fn devlaunch_core::flows::lifecycle::ReconcilePlan::adopting(&self) -> &[devlaunch_core::flows::lifecycle::Adoptable] @@ -1788,6 +1839,17 @@ pub fn devlaunch_core::flows::lifecycle::Unlocatable::eq(&self, &devlaunch_core: impl core::fmt::Debug for devlaunch_core::flows::lifecycle::Unlocatable pub fn devlaunch_core::flows::lifecycle::Unlocatable::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::Unlocatable +pub struct devlaunch_core::flows::lifecycle::VolumesKept +pub devlaunch_core::flows::lifecycle::VolumesKept::because: devlaunch_core::flows::lifecycle::VolumesKeptBecause +pub devlaunch_core::flows::lifecycle::VolumesKept::workspace_id: alloc::string::String +impl core::clone::Clone for devlaunch_core::flows::lifecycle::VolumesKept +pub fn devlaunch_core::flows::lifecycle::VolumesKept::clone(&self) -> devlaunch_core::flows::lifecycle::VolumesKept +impl core::cmp::Eq for devlaunch_core::flows::lifecycle::VolumesKept +impl core::cmp::PartialEq for devlaunch_core::flows::lifecycle::VolumesKept +pub fn devlaunch_core::flows::lifecycle::VolumesKept::eq(&self, &devlaunch_core::flows::lifecycle::VolumesKept) -> bool +impl core::fmt::Debug for devlaunch_core::flows::lifecycle::VolumesKept +pub fn devlaunch_core::flows::lifecycle::VolumesKept::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::VolumesKept pub struct devlaunch_core::flows::lifecycle::Withheld pub devlaunch_core::flows::lifecycle::Withheld::because: devlaunch_core::flows::lifecycle::KeptBecause pub devlaunch_core::flows::lifecycle::Withheld::path: std::path::PathBuf @@ -1803,14 +1865,14 @@ pub fn devlaunch_core::flows::lifecycle::apply_reconciliation(&mut devlaunch_cor pub fn devlaunch_core::flows::lifecycle::child_work(&std::path::Path, devlaunch_core::flows::lifecycle::RefreshReason) -> devlaunch_core::flows::lifecycle::ChildWork pub fn devlaunch_core::flows::lifecycle::guard_removal(&str, devlaunch_core::domain::workspace_state::Unsaved, devlaunch_core::flows::lifecycle::Insistence) -> devlaunch_core::flows::lifecycle::Guarded pub fn devlaunch_core::flows::lifecycle::objection(&devlaunch_core::domain::workspace_state::Unsaved) -> core::option::Option -pub fn devlaunch_core::flows::lifecycle::prune_clones(&mut devlaunch_core::flows::listing::CommandContext<'_>, &devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &mut devlaunch_core::domain::metadata::MetadataStorage, &devlaunch_core::flows::lifecycle::PrunePlan, &mut dyn devlaunch_core::notices::Notices) -> core::result::Result -pub fn devlaunch_core::flows::lifecycle::prune_plan(&devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &devlaunch_core::domain::metadata::MetadataStorage, &[devlaunch_core::clients::devpod::Workspace], &devlaunch_core::flows::lifecycle::ClonePlacement, devlaunch_core::flows::lifecycle::Insistence, &mut dyn devlaunch_core::notices::Notices) -> core::result::Result +pub fn devlaunch_core::flows::lifecycle::prune_clones(&mut devlaunch_core::flows::listing::CommandContext<'_>, &devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &mut devlaunch_core::domain::metadata::MetadataStorage, &devlaunch_core::flows::kept_copies::KeptCopies, &devlaunch_core::flows::lifecycle::PrunePlan, &mut dyn devlaunch_core::notices::Notices) -> core::result::Result +pub fn devlaunch_core::flows::lifecycle::prune_plan(&devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &devlaunch_core::domain::metadata::MetadataStorage, &[devlaunch_core::clients::devpod::Workspace], &devlaunch_core::flows::kept_copies::KeptCopies, &devlaunch_core::flows::lifecycle::ClonePlacement, devlaunch_core::flows::lifecycle::Insistence, &mut dyn devlaunch_core::notices::Notices) -> core::result::Result pub fn devlaunch_core::flows::lifecycle::purge_all_data(&mut devlaunch_core::flows::listing::CommandContext<'_>, &devlaunch_core::flows::lifecycle::PurgePlan, core::option::Option<&devlaunch_core::clients::devpod_home::DevpodHome>, &mut dyn core::ops::function::FnMut(devlaunch_core::flows::lifecycle::PurgeStep)) -> core::result::Result pub fn devlaunch_core::flows::lifecycle::purge_plan(&mut devlaunch_core::flows::listing::CommandContext<'_>, &std::path::Path) -> core::result::Result pub fn devlaunch_core::flows::lifecycle::reconcile_plan(&devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &devlaunch_core::domain::metadata::MetadataStorage, &[devlaunch_core::clients::devpod::Workspace], &devlaunch_core::flows::lifecycle::ClonePlacement, &mut dyn devlaunch_core::notices::Notices) -> devlaunch_core::flows::lifecycle::ReconcilePlan pub fn devlaunch_core::flows::lifecycle::sweep_repo_fetches(&devlaunch_core::flows::repo_manager::RepositoryManager<'_>, &mut devlaunch_core::domain::metadata::MetadataStorage) -> devlaunch_core::flows::lifecycle::SweepReport pub fn devlaunch_core::flows::lifecycle::unsaved_work_in(&devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &devlaunch_core::domain::metadata::MetadataStorage, &devlaunch_core::clients::git::Git<'_>, &std::path::Path, &str, &mut dyn devlaunch_core::notices::Notices) -> devlaunch_core::domain::workspace_state::Unsaved -pub fn devlaunch_core::flows::lifecycle::workspace_delete(&mut devlaunch_core::flows::listing::CommandContext<'_>, &mut devlaunch_core::flows::lifecycle::Refresh<'_>, &devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &mut devlaunch_core::domain::metadata::MetadataStorage, core::option::Option<&devlaunch_core::clients::devpod_home::DevpodHome>, &str, devlaunch_core::flows::lifecycle::Insistence, devlaunch_core::flows::lifecycle::Persistence, &mut dyn core::ops::function::FnMut(devlaunch_core::flows::lifecycle::DeleteStalled), &mut dyn devlaunch_core::notices::Notices) -> core::result::Result +pub fn devlaunch_core::flows::lifecycle::workspace_delete(&mut devlaunch_core::flows::listing::CommandContext<'_>, &mut devlaunch_core::flows::lifecycle::Refresh<'_>, &devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &mut devlaunch_core::domain::metadata::MetadataStorage, core::option::Option<&devlaunch_core::clients::devpod_home::DevpodHome>, &devlaunch_core::flows::kept_copies::KeptCopies, &str, devlaunch_core::flows::lifecycle::Insistence, devlaunch_core::flows::lifecycle::Persistence, &mut dyn core::ops::function::FnMut(devlaunch_core::flows::lifecycle::DeleteStalled), &mut dyn devlaunch_core::notices::Notices) -> core::result::Result pub fn devlaunch_core::flows::lifecycle::workspace_state(&dyn devlaunch_runner::Runner, &str, devlaunch_core::clients::devpod::Patience) -> core::result::Result pub fn devlaunch_core::flows::lifecycle::workspace_stop(&mut devlaunch_core::flows::listing::CommandContext<'_>, &mut devlaunch_core::flows::lifecycle::Refresh<'_>, &str) -> core::result::Result pub mod devlaunch_core::flows::listing From 4f4c7598eda7f135d9b9ea78362f0fb4c15627ec Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Sat, 29 Aug 2026 19:11:51 +0100 Subject: [PATCH 3/3] fix: take CI's regenerated snapshots, and codespell's word Three things this container could not check for itself, all of them named in docs/development.md as exactly that case. `SweepOccasion` sorts *after* `StopOutcome`, not before: ordering is not a partition property, so nothing local catches a block written in the wrong position and only CI's regeneration can say. Taken from its diff verbatim. `public-api.api.txt` moves too, and it is a change to the **promised** surface rather than a routine one: `api::workspace_delete` gains the copy store as its sixth parameter, because a delete that sweeps a workspace's volumes is also the moment its copy is provably pointless. One re-exported declaration, and its only caller in this repository is `dl`. And codespell prefers "unparsable". --- rust/devlaunch-core/public-api.api.txt | 2 +- rust/devlaunch-core/public-api.rest.txt | 24 ++++++++++---------- rust/devlaunch-core/src/flows/kept_copies.rs | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/rust/devlaunch-core/public-api.api.txt b/rust/devlaunch-core/public-api.api.txt index 3986e334..24b8a110 100644 --- a/rust/devlaunch-core/public-api.api.txt +++ b/rust/devlaunch-core/public-api.api.txt @@ -33,5 +33,5 @@ pub fn devlaunch_core::api::identity(&str) -> core::result::Result serde_json::value::Value pub fn devlaunch_core::api::parse(&str) -> devlaunch_core::domain::spec::WorkspaceSpec<'_> pub fn devlaunch_core::api::resolve_devcontainer_ref(&str) -> core::result::Result -pub fn devlaunch_core::api::workspace_delete(&mut devlaunch_core::flows::listing::CommandContext<'_>, &mut devlaunch_core::flows::lifecycle::Refresh<'_>, &devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &mut devlaunch_core::domain::metadata::MetadataStorage, core::option::Option<&devlaunch_core::clients::devpod_home::DevpodHome>, &str, devlaunch_core::flows::lifecycle::Insistence, devlaunch_core::flows::lifecycle::Persistence, &mut dyn core::ops::function::FnMut(devlaunch_core::flows::lifecycle::DeleteStalled), &mut dyn devlaunch_core::notices::Notices) -> core::result::Result +pub fn devlaunch_core::api::workspace_delete(&mut devlaunch_core::flows::listing::CommandContext<'_>, &mut devlaunch_core::flows::lifecycle::Refresh<'_>, &devlaunch_core::flows::workspace_clone::WorkspaceCloneManager<'_>, &mut devlaunch_core::domain::metadata::MetadataStorage, core::option::Option<&devlaunch_core::clients::devpod_home::DevpodHome>, &devlaunch_core::flows::kept_copies::KeptCopies, &str, devlaunch_core::flows::lifecycle::Insistence, devlaunch_core::flows::lifecycle::Persistence, &mut dyn core::ops::function::FnMut(devlaunch_core::flows::lifecycle::DeleteStalled), &mut dyn devlaunch_core::notices::Notices) -> core::result::Result pub fn devlaunch_core::api::workspace_stop(&mut devlaunch_core::flows::listing::CommandContext<'_>, &mut devlaunch_core::flows::lifecycle::Refresh<'_>, &str) -> core::result::Result diff --git a/rust/devlaunch-core/public-api.rest.txt b/rust/devlaunch-core/public-api.rest.txt index 57ce78c4..d67e4328 100644 --- a/rust/devlaunch-core/public-api.rest.txt +++ b/rust/devlaunch-core/public-api.rest.txt @@ -1590,18 +1590,6 @@ impl core::fmt::Debug for devlaunch_core::flows::lifecycle::SpawnRefused pub fn devlaunch_core::flows::lifecycle::SpawnRefused::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::Copy for devlaunch_core::flows::lifecycle::SpawnRefused impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::SpawnRefused -pub enum devlaunch_core::flows::lifecycle::SweepOccasion -pub devlaunch_core::flows::lifecycle::SweepOccasion::DevpodResult -pub devlaunch_core::flows::lifecycle::SweepOccasion::KeptCopy -impl core::clone::Clone for devlaunch_core::flows::lifecycle::SweepOccasion -pub fn devlaunch_core::flows::lifecycle::SweepOccasion::clone(&self) -> devlaunch_core::flows::lifecycle::SweepOccasion -impl core::cmp::Eq for devlaunch_core::flows::lifecycle::SweepOccasion -impl core::cmp::PartialEq for devlaunch_core::flows::lifecycle::SweepOccasion -pub fn devlaunch_core::flows::lifecycle::SweepOccasion::eq(&self, &devlaunch_core::flows::lifecycle::SweepOccasion) -> bool -impl core::fmt::Debug for devlaunch_core::flows::lifecycle::SweepOccasion -pub fn devlaunch_core::flows::lifecycle::SweepOccasion::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result -impl core::marker::Copy for devlaunch_core::flows::lifecycle::SweepOccasion -impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::SweepOccasion pub enum devlaunch_core::flows::lifecycle::StopOutcome pub devlaunch_core::flows::lifecycle::StopOutcome::DevpodRefused pub devlaunch_core::flows::lifecycle::StopOutcome::DevpodRefused::exit: devlaunch_runner::Exit @@ -1615,6 +1603,18 @@ impl core::fmt::Debug for devlaunch_core::flows::lifecycle::StopOutcome pub fn devlaunch_core::flows::lifecycle::StopOutcome::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result impl core::marker::Copy for devlaunch_core::flows::lifecycle::StopOutcome impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::StopOutcome +pub enum devlaunch_core::flows::lifecycle::SweepOccasion +pub devlaunch_core::flows::lifecycle::SweepOccasion::DevpodResult +pub devlaunch_core::flows::lifecycle::SweepOccasion::KeptCopy +impl core::clone::Clone for devlaunch_core::flows::lifecycle::SweepOccasion +pub fn devlaunch_core::flows::lifecycle::SweepOccasion::clone(&self) -> devlaunch_core::flows::lifecycle::SweepOccasion +impl core::cmp::Eq for devlaunch_core::flows::lifecycle::SweepOccasion +impl core::cmp::PartialEq for devlaunch_core::flows::lifecycle::SweepOccasion +pub fn devlaunch_core::flows::lifecycle::SweepOccasion::eq(&self, &devlaunch_core::flows::lifecycle::SweepOccasion) -> bool +impl core::fmt::Debug for devlaunch_core::flows::lifecycle::SweepOccasion +pub fn devlaunch_core::flows::lifecycle::SweepOccasion::fmt(&self, &mut core::fmt::Formatter<'_>) -> core::fmt::Result +impl core::marker::Copy for devlaunch_core::flows::lifecycle::SweepOccasion +impl core::marker::StructuralPartialEq for devlaunch_core::flows::lifecycle::SweepOccasion pub enum devlaunch_core::flows::lifecycle::VolumeRefusal pub devlaunch_core::flows::lifecycle::VolumeRefusal::Docker pub devlaunch_core::flows::lifecycle::VolumeRefusal::Docker::exit: devlaunch_runner::Exit diff --git a/rust/devlaunch-core/src/flows/kept_copies.rs b/rust/devlaunch-core/src/flows/kept_copies.rs index 45a65e2d..e2a86a04 100644 --- a/rust/devlaunch-core/src/flows/kept_copies.rs +++ b/rust/devlaunch-core/src/flows/kept_copies.rs @@ -188,7 +188,7 @@ impl KeptCopies { /// What one copy says. /// -/// Versioned by "unparseable means absent", which is the whole of the format story: +/// Versioned by "unparsable means absent", which is the whole of the format story: /// a copy a later build writes differently fails to deserialize and reads as no /// copy, costing one workspace's volumes their record rather than costing anybody a /// migration.