Skip to content

Remove the workspace's pixi and dind volumes on delete #325

Closed

Description

@blooop

Work

Remove the two named docker volumes a workspace's devcontainer created, when that
workspace is deleted.

What leaks today

Nothing in devlaunch ever removes a docker volume — grep for a volume command
across the Rust tree returns zero hits, because the only subprocess is devpod.
lifecycle::workspace_delete
is exactly devpod delete <id> plus removing the clone directory, and devpod delete does not remove volumes a devcontainer declared. So every removal path
leaks two volumes per workspace, both named deterministically from the workspace
being deleted:

volume declared by size seen
${localWorkspaceFolderBasename}-pixi our own .devcontainer/devcontainer.json mounts (the .pixi cache) 0.4–2.7 GB
dind-var-lib-docker-${devcontainerId} the docker-in-docker:2 feature up to 4.2 GB

Both names are already knowable at delete time: the basename is the clone
directory devlaunch itself chose, and the devcontainer id is recorded in
~/.devpod/contexts/<ctx>/workspaces/<ws>/workspace_result.json.

Measured on one dev machine, cross-referencing volumes against devpod list and
the workspace result files: 39 orphaned volumes, 37.28 GB, none with a
surviving workspace — 22 pixi volumes (19.5 GB) and 17 dind volumes (17.7 GB).
About 9.5 GB of it carries current name-hash-scheme names, so this is a live leak
and not historical debt. 1.04 GB sat under e2e-* workspace ids, so the e2e suite
leaks too.

Shape of the fix

Once devpod has confirmed it let go of the workspace, remove the volumes whose
names derive from it. Best-effort: a typed notice on failure rather than a failed
delete (the workspace is gone either way, and reporting failure sends the caller
looking for a workspace that is not there — the same reasoning the existing
clone-removal arm uses), and silently skipped where docker is absent, so nothing
added here can fail on a docker-less machine. --autorm and --purge inherit it
by sharing the path.

One judgement to settle while building: whether --purge should also sweep
volumes for workspaces it did not itself delete. Today it promises a clean slate
and leaves every volume standing, which is the gap that let 37 GB accumulate — but
sweeping by name pattern is a different and less certain operation than removing
the volumes of a workspace you just deleted. Prefer the smaller thing unless the
larger one falls out for free; note the call in a breadcrumb.

The sentence that has to change with it

render::DOCKER_BOUNDARY — "devlaunch does
not manage Docker images or volumes" — is printed only through
Cleanup::with_the_boundary, which only --prune and --purge use. rm and
--autorm, the two commands that actually create the orphans, say nothing at all.
Once volumes are removed the sentence needs narrowing to images rather than
deleting: images stay out of scope deliberately (shared, expensive to rebuild,
ownership genuinely ambiguous), and that remains worth saying.

Done when

  • A workspace deleted by rm, --rm, --autorm or --purge leaves neither
    volume behind.
  • Regression test: the removal path issues the removals for both derived
    names. This is the test that would have caught the leak.
  • Regression test: a machine with no docker still deletes the workspace and its
    clone cleanly, with the removal reported as a notice rather than a failure.
  • DOCKER_BOUNDARY and its pinning tests say only what is still true.

Activity

  1. self-assigned this
    on Aug 21, 2026
  2. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: Filed as a single-ticket map and claimed; no build started yet. The
    leak was found while clearing 37.28 GB of orphaned volumes off one dev machine
    (39 volumes, none with a surviving devpod list workspace) — that cleanup is done
    locally and is not part of this ticket. Next move: wf-tdd on this ticket, red
    test first for both derived volume names.

  3. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: wf-auto resumption on an already-claimed ticket: trail read (one breadcrumb, no handoff), no PR and no commits ahead of main, so the stage derives as ready-to-build. Handing to a fresh wf-tdd subagent now.

  4. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: Re-entry on the claimed ticket: trail read (two breadcrumbs, no handoff), no PR, nothing ahead of main. Building the amended spec as written.

    What I will build. Deleting a workspace names the docker volumes its devcontainer created, then removes them once devpod has confirmed it let go. Best-effort throughout: docker absent is silent, docker refusing is a typed notice on a delete that still succeeded.

    Two findings that shape it, both from reading rather than guessing.

    1. The names have to be read before the delete, not after. devpod delete takes devpod's own record of the workspace away with the workspace, and that record is the only place the substituted names live. So the order is: name, delete, remove. Naming after the delete would find nothing, every time, and look like a working fix.
    2. --purge does not share the delete path. The map's note says every removal path goes through workspace_delete; it does not — the purge issues its own captured devpod delete --force per workspace and never calls it. So --purge gets the same three-step wired explicitly rather than inheriting it. rm, --rm and --autorm do all share workspace_delete and do inherit.

    Where the names come from. One source, not two: devpod's create result records what it substituted, so both names derive from the same read — the workspace folder's basename for <basename>-pixi, and the recorded devcontainer id for dind-var-lib-docker-<id>. Confirmed against the pinned devpod binary's own struct tags rather than assumed. Deriving the basename from devlaunch's clone directory instead would be a second answer to a question devpod has already answered, and the two could disagree.

    Removal is one call with both names and --force. Measured against the docker in this container: --force makes an already-absent volume exit 0, so a repo whose devcontainer never declared one of these is a silent no-op rather than a notice on every delete — while a volume still held by a container is still refused, which is a real thing to report.

    Seams I will test at:

    • lifecycle::workspace_delete, argv-exact: after a successful delete, exactly one docker call carrying both derived names. This is the regression test the ticket asks for.
    • the same seam with no docker on the machine: the workspace and its clone still go, the sweep reads as "no docker", and nothing is said.
    • the same seam with docker refusing: the delete still succeeds and the refusal arrives as a typed notice carrying docker's own words.
    • the same seam with no devpod record to read: nothing is named, so docker is not run at all.
    • lifecycle::purge_all_data: the volumes of each workspace it deleted, and none for a workspace devpod refused to delete.
    • the dl binary's pinning tests for the docker-boundary sentence, narrowed to images.
  5. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: the --purge judgement, settled. The ticket left one call open — whether --purge should also sweep volumes for workspaces it did not itself delete. It should not. It sweeps the volumes of every workspace it deletes, and nothing else.

    Three reasons, in the order they decided it.

    1. The blast radius is larger than the command promises, and it cannot be scoped. --purge is ownership-scoped on purpose: it deletes only workspaces whose source is a clone under devlaunch's cache, and it names the survivors before asking. Sweeping the leftovers means matching *-pixi and dind-var-lib-docker-* by pattern, and those patterns belong to the devcontainer spec and to a public feature, not to devlaunch — every repository using the docker-in-docker feature has one, including the workspaces this command has just promised to leave standing. It would also be the one thing in a purge that no XDG_* variable scopes, so a run pointed at a scratch cache — which is how this tool is tested on a real machine — would reach the real volumes anyway. That is the opposite of the property --purge was given when it stopped deleting every workspace devpod list returned.

    2. With the leak fixed at the source, there is nothing left to accumulate. The 37 GB is what the leak produced over its whole life, not a rate. Every workspace removed from here on takes its volumes with it, so a pattern sweep is a one-time cleanup of historical debt — and a one-time cleanup is a thing a person does deliberately, with docker system df -v and docker volume rm, which the README already walks through. Building a command for it would leave a permanent, unscopable capability in the tool to solve a problem that stops existing.

    3. It does not fall out for free, which is the tie-breaker the ticket named. Removing the volumes of a workspace you just deleted needs no new knowledge: devpod recorded the names. Sweeping by pattern needs a docker volume ls, a match against every live workspace to avoid taking one in use, and a decision about volumes belonging to no workspace devlaunch can see. Different operation, less certain, and the ticket said to prefer the smaller thing unless the larger one is free.

    One thing that did not fall out for free and was built anyway: the map's note said every removal path goes through the shared delete flow, so --purge would inherit the fix. It does not — the purge issues its own captured devpod delete --force per workspace and never calls that flow — so it is wired explicitly, with its own test saying so and its own report line, because a refusal during a purge is said as it happens rather than collected.

    Also deliberately out of scope: reading the volume names out of the merged devcontainer config devpod records, rather than deriving them from the two substituted values. That would cover any volume any devcontainer declared, not just these two, and would need no knowledge of either name — genuinely better, if the recorded config turns out to hold post-substitution mount sources. It could not be established from a record on this machine, and getting it wrong means asking docker to remove a volume literally named ${localWorkspaceFolderBasename}-pixi. Left as a follow-up worth measuring rather than guessed at here.

  6. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: Build stage done — PR #331, open against main, Closes #325.

    One slice, in three parts: the delete path names its workspace's two devcontainer volumes from devpod's own substitution record before asking devpod to let go, removes them with one forced docker call afterwards, and reports a refusal as a typed notice on a delete that still succeeded. rm, --rm and --autorm inherit it by sharing the delete flow; --purge was wired explicitly, because it turned out not to share that flow at all. The docker-boundary sentence both cleanups end on is narrowed to images.

    Gates green locally: cargo test --workspace, clippy -D warnings, fmt --check, plus pixi run test, lint and prek. One caveat worth carrying into review: devlaunch-core/public-api.txt was updated by hand, because cargo public-api needs a nightly rustdoc backend the container's pinned toolchain does not have. The entries are right; their ordering is inferred from the file's conventions, so a CI diff there would be a sort nit rather than a surface change.

    Not merged, ticket left open. Next stage is wf-review in fresh context.

  7. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: main moved under the branch while the build was in flight (0.4.0 released), and the conflict was doing real damage rather than being cosmetic: CI never ran on the PR at all. A pull_request workflow tests the merge commit, and a conflicting branch has no merge commit to test — so the PR sat with two vendor checks green and every gate that matters simply absent. Worth knowing next time a PR looks suspiciously quiet.

    Merged main in; the only conflict was the changelog's [Unreleased] heading, where my entry stays put and the released section slots in under it. Every gate re-run on the merged tree, all green.

    One defect found and fixed while waiting, with a test that was red before it: the loop that finds devpod's create result bailed out of the whole search on a context directory whose name is not text devlaunch can spell, instead of stepping over it the way the create-record loop beside it does. A directory that cannot be spelled holds no record anyone could read, so the cost fell entirely on the context that does hold one — a workspace's volumes staying on disk on account of a name nothing was going to look at.

  8. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: CI green on PR #331 — every check: ci, rust, rust-coverage, e2e, packaging, public-api, prek, gate, codecov (patch and all three project flags), GitGuardian. PR is MERGEABLE / CLEAN.

    Two things worth carrying into review rather than leaving in the run log:

    • The hand-written public-api.txt was right, ordering included. cargo public-api needs a nightly rustdoc backend the container's pinned toolchain does not carry, so the snapshot for the two new enums, the two new variants, the new field and the two changed signatures was written by hand against the file's existing conventions. CI's public-api job passes, so it is generated-equivalent and not a guess anybody has to re-check. Two variant names were chosen to make that safe — the sweep's refusing arm is Refused rather than NotRemoved, and the refusal type is VolumeRefusal rather than VolumesRefused, because both of the alternatives sort ambiguously depending on whether the tool's comparison is case-sensitive and I could not run it to find out.
    • The e2e suite passes with the sweep live, which is the one place the removal runs against a real docker daemon rather than a fake. The ticket noted 1.04 GB of orphans under e2e-* ids, so that suite was one of the leak's sources; it now cleans up after itself.

    Build stage is finished. Not merged, ticket left open for wf-review.

  9. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: build → in review. Gate checked from live state: PR #331 open, non-draft, MERGEABLE/CLEAN; all 13 checks green; actions/runs?head_sha=216a256 returns 3 runs, so CI genuinely ran rather than being silently absent. Handing to a fresh wf-review subagent that has not seen the code written.

  10. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: review → needs attention. Two-axis review posted on PR #331 in fresh context. Spec: pass — all four "Done when" bullets met (bullet 3 on the ticket's own "silently skipped where docker is absent" terms rather than the bullet's looser wording), the --purge judgement settled soundly, no scope creep. The devpod JSON keys were the one silent-no-op risk and they check out against the pinned v0.26.1 binary's own struct tags; both docker semantics claims re-measured against docker 29.7.2 and both hold.

    Standards: fail — one blocking finding. lifecycle.rs:1137 substitutions_of re-implements sole_context_holding's contexts walk with a different ambiguity rule (readable workspace_result.json vs existing workspace.json), against an invariant that same file states at :2873. The two disagree, and the disagreement can point the removal at a live workspace's volumes — the exact hazard :1134 claims to avoid. Reproduced with a probe. The fix is a net deletion: sole_workspace_result + fs::read, ~20 lines out for 3 in, retiring :5278 with them.

    Verdict Request changes (posted as a comment — GitHub refuses --request-changes on a same-account PR; the written verdict is the gate). Not merged, ticket left open. Next stage is wf-tdd on the blocking finding.

  11. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: review gate result — two-axis report posted on #331 (#331 (review)). Spec: pass. Standards: fail, one blocking finding, so the node is at needs-attention rather than done. Firing the single retry the protocol allows; a second failure parks the ticket.

  12. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: retry after the review gate — the blocking finding was real, and the fix is a deletion.

    Verified rather than taken on trust. The volume sweep walked devpod's contexts
    with a loop of its own and keyed its ambiguity check on a readable create
    result
    , where the walk every other caller shares keys on the workspace record.
    Those are different questions, so the two genuinely disagreed: one id under two
    contexts with a finished up in only one of them made the shared walk say "I
    cannot say" and made the sweep's loop answer with the context that happened to
    have completed. devpod delete resolves an id against the current context, so
    that is a delete of one workspace and a volume removal aimed at another, living
    one — the exact hazard the sweep's own comment claimed to avoid. The file states
    the no-second-copy invariant on the shared walk itself, so the duplication was
    against a rule written down beside it.

    Red before green. The failing test is the discriminating shape — one id in two
    contexts, a create result in only one — and it named both volumes before the fix.
    The fix is sole_workspace_result plus a read: three lines for twenty-odd, and
    the rule is now identical by construction rather than by comment.

    Two things fell out that are worth knowing.

    • The loop-specific non-UTF-8 skip test retires with the loop, but the behaviour
      it pinned had no other coverage.
      So it moved rather than died: it now asserts
      the same skip on the shared walk, where the skip actually lives. Net test count
      unchanged, coverage moved to where the code is.
    • The binary-level fixture was wrong about devpod. It wrote a create result with
      no workspace record beside it — a shape devpod never leaves on disk, since it
      writes the record on the way in to an up and the result on the way out of
      a finished one. Sharing the walk made the fixture's fiction visible, which is
      the duplication's second maintenance cost inside one ticket.

    Two non-blocking findings addressed, three declined, all answered on the PR.
    Addressed: the changelog now says the frozen api:: surface moved and what an
    out-of-tree caller passes instead (first such change since the snapshot was
    locked), and the refused-volumes sentence is written once instead of once per
    printer. Declined with reasoning: bundling devpod_home onto a struct to retire
    the too_many_arguments allow would change a far more widely constructed public
    constructor to avoid one signature change, and the module's own convention is to
    pass a devpod home explicitly; the docker client's mapping of an unreachable
    timeout is a legal inhabitant of the failure type rather than a sentinel, and
    devpod models it as its own arm because devpod calls actually set timeouts, where
    this one is given none.

    Gates green locally: cargo test --workspace, clippy -D warnings,
    fmt --check, lint, prek. Pushed; not merged, ticket left open.

  13. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: retry finished — PR #331 green again, MERGEABLE/CLEAN, all 13
    checks including public-api (no snapshot change was needed: this round moved no
    public signature) and e2e, which is the one place the sweep runs against a real
    docker daemon.

    main had moved again under the branch (0.4.1 released) and the branch went
    CONFLICTING, which is the same trap as last time: a conflicting PR has no merge
    commit, so every pull_request gate is silently absent rather than failing.
    Worth checking mergeability whenever the check list looks short. The only conflict
    was the changelog's [Unreleased] heading again — my entries stay put and the
    released section slots under them.

    Also corrected the map's Note on #324: it claimed --purge inherits the shared
    delete flow, which it does not. Recorded there so the next reader is not misled by
    the plan.

    Not merged, ticket left open.

  14. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: retry landed 9dbf01d/af3e2f6/ad8125f; head is now ad8125f, 13/13 checks green, actions/runs?head_sha=ad8125f returns 3 real runs. The standing two-axis verdict was against 216a256, so it no longer describes the head and the review→done gate is unproven. Firing one fresh re-review against the current head; a fail there parks the ticket rather than starting a third round.

  15. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: re-review at the moved head — both axes pass. Fresh-context wf-review against ad8125f (the standing verdict was against 216a256): two-axis report posted on PR #331. Standards: pass (no blocking findings). Spec: pass (all four "Done when" bullets met). Verdict Approve — written, since GitHub refuses --approve on a same-account PR.

    Nothing was carried from the previous review. The prior round's single blocking finding — a second contexts walk with a disagreeing ambiguity rule — is genuinely gone: one read_dir over contexts remains, devcontainer_volumes goes through sole_workspace_result, and a probe that reintroduces the divergence now turns one_id_in_two_contexts_names_nothing red. Both behaviours that could have vanished with the deleted loop still have tests where the code now lives.

    Two things re-derived rather than believed, because they are the only ways this fix could fail silently. devpod v0.26.1's own embedded struct tags carry exactly the three JSON keys parse_substitutions reads. And requiring workspace.json beside the create result does not un-name a real workspace: a result-without-record workspace was built under a scratch DEVPOD_HOME and devpod's own list refuses to load it, so no such workspace is ever a deletable target. Docker semantics re-measured on 29.7.2 — both claims hold.

    Two non-blocking items left on the record for a later reader, not this PR: the fabricated OsFailure { kind: TimedOut, errno: None } in clients/docker.rs, and the too_many_arguments allow now that flows::launch::Host already carries a devpod_home.

    Not merged, ticket left open. The review gate is satisfied at this head.

  16. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: re-review gate passed — two-axis report posted against the current head ad8125f (#331 (review)), Standards pass, Spec pass, zero blocking. Verified live: PR open, non-draft, CLEAN, 13/13 checks pass. Stage is now in review, approved.

  17. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    handoff

    Where we are. Parked at in review, approved — the terminal state for an
    unattended run. #331 is open at
    ad8125f, MERGEABLE/CLEAN, 13/13 checks green with real pull_request runs on
    that SHA, and carries an approving two-axis review with zero blocking findings.
    This ticket is deliberately left open: the work is not done until the PR is
    merged, and merging is not something this run does.

    What was built. Both volume names come from one read of devpod's create
    result (SubstitutionContext.LocalWorkspaceFolder → <basename>-pixi,
    .DevContainerID → dind-var-lib-docker-<id>), taken before the delete because
    devpod delete takes that record away with the workspace. Removal is a single
    docker volume rm --force with both names, riding inside DeleteOutcome::Deleted
    as a VolumeSweep so it cannot fail a delete. --purge is wired explicitly
    rather than by inheritance — see the map correction below.

    The open judgement, settled. --purge sweeps only the volumes of workspaces
    it deletes, not by name pattern. Reasoning in full at
    #325 (comment) —
    short version: a pattern sweep reaches volumes belonging to the workspaces
    --purge promises to leave standing and to any repo using the public dind
    feature, it is the one thing in a purge that no XDG_* variable scopes, and it
    does not fall out for free. The 37 GB already on disk is the leak's lifetime
    output, not a rate, so with the source fixed it is one-time debt a person clears
    deliberately.

    One round of changes-requested, resolved. The first review found
    substitutions_of was a second copy of sole_context_holding's contexts walk
    with a divergent ambiguity rule — readable workspace_result.json versus
    existing workspace.json. Because devpod writes the record on the way into an
    up and the result on the way out, an id under two contexts with one finished
    create is where the two rules part, and devpod delete resolving against the
    current context meant dl could delete one workspace and aim docker volume rm at
    a live one. Fixed as a net deletion through the shared helper; the discriminating
    case is now test-enforced rather than comment-enforced.

    The open thread — merge. Nothing else is outstanding. Closes #325 is in the
    PR body, so merging it closes this ticket.

    First move on resume. Re-check that ad8125f is still CLEAN and green
    (main moved twice during this work and a conflicting PR runs no pull_request
    gates at all — the check list just looks short rather than red, which caught this
    build twice). Then squash-merge #331.

    Two non-blocking findings left standing deliberately, both argued on the PR
    and neither worth holding a merge for:

    • clients/docker.rs:76 maps a timeout to a fabricated OsFailure { kind: TimedOut, errno: None } where clients/devpod.rs:87 gives a killed child its
      own arm. Unreachable today — this spawn is given no timeout.
    • flows/lifecycle.rs:971 takes a ninth parameter behind a new
      #[allow(clippy::too_many_arguments)] instead of bundling. Declined because the
      only struct both functions already take is CommandContext, constructed at
      dozens of sites, so bundling trades one frozen-surface change for a broader one.

    Also worth a follow-up someone should measure, noted but not filed: reading
    volume names from the recorded merged devcontainer config instead of deriving
    them from two substituted values. Strictly more general — it would cover any
    volume any devcontainer declares — but it was not established whether that config
    stores mounts after ${devcontainerId} substitution, and getting that wrong means
    asking docker to remove a volume literally named
    ${localWorkspaceFolderBasename}-pixi.

  18. blooop commented on Aug 21, 2026

    @blooop
    OwnerAuthor

    breadcrumb: merged and released. Human granted merge explicitly, lifting the park. #331 merged as e950c18 (merge commit — squash and rebase are both disabled on this repo), which closed this ticket. Release cut on top: 0.5.0, minor rather than patch because this carries the breaking api:: change (workspace_delete and purge_all_data each gained an Option<&Path>), and pre-1.0 semver bumps the minor for that. #334 merged as 207c238; Auto-publish tagged v0.5.0 and published to PyPI, Conda Publish uploaded to prefix.dev. Both green.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions