Repository navigation
dl --purge deletes only the workspaces devlaunch created - #129
Conversation
devpod's workspace namespace is shared. `purge_all_data` iterated everything `devpod list` returned, so a workspace made with a hand-run `devpod up`, by another tool, or by an older devlaunch was deleted along with devlaunch's own -- measured on a real host, `--purge` would have destroyed `pythontemplate`, which devlaunch did not create, does not track and cannot recreate. "Mine" is now: the workspace's source is a clone under devlaunch's own cache directory. Every workspace `dl owner/repo[@Branch]` creates is such a clone, and that directory is exactly what a purge removes -- so the workspaces it deletes are the ones whose source it was going to delete anyway, and everything left standing keeps working afterwards. The split is a value, `WorkspaceOwnership`, not a filter applied at the point of deletion: the count `--purge` asks the user to confirm and the set it deletes read the same object, and the workspaces devlaunch does not own have somewhere to be named from. They are named before the confirmation, while saying no is still an option. Three existing tests were this defect written down as assertions -- their workspaces were sourced from paths outside the cache, which is what someone else's workspace looks like. They now use the shape devlaunch actually creates and keep their original intent. The e2e purge test asserted that `--purge` deleted the workspace `devpod up` had just built for it; it now asserts that workspace is still standing and named.
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideUpdate dl --purge so it only deletes DevPod workspaces that devlaunch created (based on clone location under its cache directory) and reports foreign workspaces before confirmation; add supporting ownership logic, documentation, and tests around this behavior and XDG scratch cache protection. Sequence diagram for updated dl --purge workspace deletion flowsequenceDiagram
actor User
participant CLI as _run_cli
participant LW as list_workspaces
participant WO as workspace_ownership
participant Purge as purge_all_data
participant DevPod as run_devpod
User->>CLI: dl --purge
CLI->>LW: list_workspaces()
CLI->>WO: workspace_ownership(workspaces, cache_dir)
WO-->>CLI: WorkspaceOwnership(mine, foreign)
CLI->>CLI: print mine and foreign workspace counts
CLI->>CLI: print names of foreign workspaces
alt user confirms purge
CLI->>Purge: purge_all_data()
Purge->>LW: list_workspaces()
Purge->>WO: workspace_ownership(workspaces, cache_dir)
WO-->>Purge: WorkspaceOwnership(mine, foreign)
loop ws in owned.mine
Purge->>DevPod: run_devpod(["delete", ws.id, "--force"])
end
Purge->>Purge: remove cache_dir
else user cancels
CLI->>User: no data purged
end
File-Level Changes
Assessment against linked issues
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #129 +/- ##
==========================================
+ Coverage 92.76% 93.21% +0.44%
==========================================
Files 18 18
Lines 2005 2018 +13
==========================================
+ Hits 1860 1881 +21
+ Misses 145 137 -8
🚀 New features to boost your workflow:
|
The e2e job on ubuntu-latest runs `pixi run test-e2e` on every PR, so this test is measured on a real devpod rather than left unverified. It now builds two real workspaces with the same command, differing only in where the source lives -- a clone under devlaunch's scoped cache, and one outside it -- and asserts `--purge -y` deletes the first and names the second as one it did not create. That is the claim the whole predicate rests on: what devlaunch creates is a clone under its own cache, and devpod records that path back as the workspace source. It is the one part of this change a recorded listing cannot prove.
CI's e2e run reported `assert 1 == 0` with the reason truncated out of the repr. `--purge` says on stdout what it deleted, what it left and what it could not remove, so a bare rc comparison is the one thing that cannot be read.
…eleted
Spec, blocking: the e2e test could not pass. `theirs = "e2e-test-purge"` is a
substring of `mine = "e2e-test-purge-mine"`, so `"Deleting ... e2e-test-purge"
not in stdout` failed on *correct* output while `theirs in stdout` passed on no
evidence -- one bug making one assertion impossible and its neighbour vacuous.
The ids no longer contain one another, and both assertions match whole lines, so
a substring cannot stand in for a fact again.
Standards, blocking: the source-type guard had a surviving mutant. Deleting `if
workspace.source_type != "local"` left all 1047 tests green, because the two
tests naming that branch used a git URL and `"{}"` as their sources -- neither is
an absolute path, so containment already rejected them and the guard was never
reached. The test now uses a source *inside* the cache directory with a `git` or
`unknown` type, which is the only shape that puts the guard under test, and it
asserts the same path is recognised when local so the setup cannot rot.
Also from Standards: `_get_cache_dir`, `_get_cache_base` and
`_get_default_metadata_path` were three byte-identical spellings of the same
directory, and this change made them load-bearing -- `--purge` now reads the
first to decide which workspaces are devlaunch's, and the other two are what put
those clones on disk. They now share one answer in `devlaunch.xdg`, beside
`config_home`, which exists for this exact reason. `workspace_ownership` makes
one pass instead of asking the predicate twice per workspace, pinned by a test.
The frozen-dataclass test is gone: `ty` already rejects the assignment, so it
tested CPython.
The e2e purge test no longer requires rc 0. On a runner the container writes into
the bind-mounted clone as uid 1000 while the suite runs as 1001, so `rmtree` hits
EACCES on a directory it cannot chmod -- measured in CI, not inferred. That is a
pre-existing defect in the cache half of the purge, unrelated to ownership and
not fixable from inside the process; the test asserts its own subject and
tolerates only that one failure, by name.
Finally, a guard restored rather than authored: commit 1e72e14 accidentally
captured a review subagent's live mutation of a shared worktree, deleting the
source-type guard from the tree. It is back, and its mutant is dead.
Two independent axes, run as parallel subagents in fresh context against the fixed point StandardsBlocking — surviving mutant on the source-type guard ( Should-fix — three byte-identical copies of the cache directory. Should-fix — the Should-fix — Nits. Passed. The adapted tests in SpecBlocking — the new e2e test could not pass, so the one claim only metal can settle was still unsettled. The rest of the spec is met, checked independently rather than read off the PR.
Should-fix. Nit. README named two survivors where Nit. Containment is lexical, so a symlinked Process defect found during the review, and it is the reviewer'sA Standards subagent ran a mutation experiment inside the shared worktree while the author was committing, and commit VerdictRequest changes — posted as a comment because GitHub refuses Blocking, both now addressed at
Should-fixes taken: the three cache-directory copies now share one answer in Should-fix declined, with the reason: the CHANGELOG entry stays in the PR body. This run is under an explicit batching policy — several changes are in flight and a release PR will lift the prose — so touching Nit declined: the symlinked- Full mutation battery on the corrected code, all five dead:
|
Closes #107.
purge_all_dataiterated everythingdevpod listreturned. devpod's workspacenamespace is shared with every other way a person makes a workspace — a hand-run
devpod up, another tool, an older devlaunch — sodl --purgetreated all of itas devlaunch's to destroy. Measured on the host this was built on, it would have
deleted
pythontemplate, a workspace devlaunch did not create, does not trackand cannot recreate.
What "mine" means
The workspace's source is a clone under devlaunch's own cache directory.
Every workspace
dl owner/repo[@branch]creates is such a clone(
<cache>/repos/<owner>/<repo>/<id>, handed todevpod upas a path, whichdevpod records as the source).
The ticket proposed
metadata.json, which also records these ids. Both werechecked read-only against the same host and they agree exactly on the six
live workspaces, so the choice came down to how each fails:
metadata.jsonlives inside the directory a purge removes, andpurge_all_dataonly warns whendevpod deletefails. One failed delete plusone successful purge would leave a workspace no later purge could recognise.
A source path is read back from devpod every time and outlives the cache it
names.
39 worktree entries for 6 live workspaces.
removes devlaunch's cache, and the workspaces it deletes are the ones backed
by that cache. Everything left standing keeps working afterwards, because
nothing a purge touches backs it.
MetadataStoragein the purge path.Conservative in two places on purpose: a
gitor unrecognised source is neverours (devlaunch always hands devpod a local path), and the comparison is path
containment, not a string prefix —
<cache>-scratchshares six characters with<cache>and is not inside it. A mutation tostr.startswithreds a test.What it does with the rest
Names them, before it asks:
The report goes with the confirmation rather than after the deletions, which is
where a user deciding whether to say yes can read it. No opt-in "delete
everything anyway" flag: that is the footgun this ticket exists to remove, and
dl <workspace> rmalready deletes one workspace on purpose.The split is a value — a frozen
WorkspaceOwnership(mine, foreign)— rather thana filter applied at the point of deletion. The count the user confirms and the
set that dies read the same object, and the unrecognised arm has somewhere to be
named from instead of being inferred from a lookup miss.
Known, deliberate incompleteness
dl ./path,dl <git-url>, and aconfig.tomlthat pointsrepos_diroutsidethe cache all produce workspaces this does not recognise, so they survive a
purge. Closing that needs a new record of "workspaces I created" — a new concept
and a schema change — and the
repos_dircase is already outside what a purgeremoves, so recognising it would delete containers whose clones stayed on disk.
Erring this way costs a user one
dl <workspace> rm; the other kind of mistakecosts work that cannot be recovered. All three are named in the output rather
than silently skipped, and in
is_devlaunch_clone's docstring.Verification
Red first, in two stages. With the type and predicate added but
purge_all_datauntouched: 7 failed, 12 passed — the 12 predicate/value tests green, every
purge test red, one of them printing
Deleting DevPod workspace: pythontemplatein its captured output. Then green.
Three mutations on the landed code, each reverted:
for ws in owned.mine + owned.foreignworkspace.source.startswith(str(cache_dir))source != cache_dirguardVerified on metal, read-only. The real
devpod liston the development host,run through the new code without deleting anything:
That is the ticket's outcome exactly.
devpod listreports the same sixworkspaces before and after, and
~/.ssh/configis unchanged(
0fcb9aeff46aad62551d9d688f2021f7).dl --purgewas never run for real, nodevpod deletewas issued, and the e2e suite was not run — per the ticket, thisis proven by unit tests and fakes.
Full default suite 1047 passed, 22 deselected; ruff,
tyand pylint 10.00/10.Tests that were the defect written down
Four existing tests asserted the old behaviour and now assert the new one with
their original intent intact:
test_dl.py::test_purge_deletes_all_workspacesand::test_purge_handles_workspace_delete_failuresourced their workspaces from/pathand a git URL — which is what someone else's workspace looks like, sothey asserted that
--purgedeleted other people's work. They now use clonesunder the cache.
test_devpod_spawn_counts.py's two purge tests, the same way;DevPodSpawnsgains a
source_rootso it can express both kinds, which is now a realdistinction.
test_purge_deletes_workspacesasserted that--purgedeleted theworkspace
devpod uphad just built for it. It is nowtest_purge_deletes_devlaunchs_workspaces_and_leaves_everyone_elsesand buildstwo real workspaces with the same command, differing only in where the
source lives: a clone under the run's scoped cache, and one outside it. It
asserts the first is deleted and the second is left standing and named.
Metal, by CI
The
e2ejob runspixi run test-e2eonubuntu-latestfor every PR, so the oneclaim a recorded listing cannot settle — devpod records back the clone path
devlaunch handed it — is settled by running it. Green:
8 passed, 14 skipped,5 workspaces created: e2e-test-create, e2e-test-lifecycle, e2e-test-git, e2e-purge-devlaunchs, e2e-purge-hand-made.That run also found a second, separate defect, now filed as
#131 rather than folded in here.
On the runner
dl --purge -yexits 1:The workspace half did exactly what this PR is about, and then
shutil.rmtreehit EACCES: the container writes into the bind-mounted clone as
vscode(uid1000) while the runner is uid 1001, and a non-owner cannot chmod a directory to
empty it. Pre-existing, in the cache half whose code this diff does not touch, and
not fixable from inside the process. It went unnoticed because the e2e purge
test's source used to be outside the cache, so no container had ever written
into the cache during a run. The test therefore asserts its own subject and
tolerates that one failure by name; any other non-zero exit still fails it.
AGENTS.mdgains the one fact this changes for the manual workflow: the scratchXDG_CACHE_HOMErecipe now genuinely protects--purge, where before noXDG_*variable could scope it. That closes the gap #106's
review graduated into this ticket, without needing the second option that review
named.
Proposed CHANGELOG entry
No version bump here — several changes are in flight and a bump publishes a
release immediately. For a later release PR to lift:
Changed
dl --purgenow deletes only the DevPod workspaces devlaunch created — theclones it made under its own cache directory — instead of every workspace
devpod listreturns. devpod's namespace is shared, and a workspace you madewith
devpod up, or that another tool made, was being destroyed along withdevlaunch's own.
dl --purgenames the workspaces it is leaving behind, before it asks forconfirmation, and the count it asks you to approve is now the number it will
actually delete.
Fixed
XDG_CACHE_HOMEat a scratch directory now protectsdl --purge.It never did:
devpod listreads~/.devpod, so a scratch run still saw — anddeleted — every real workspace on the machine.
Note for anyone who used
dl ./pathordl <git-url>: those workspaces opena source
dldid not clone, so--purgecannot tell them from one you made byhand and now leaves them standing. They are listed in the output; remove one with
dl <workspace> rm.🤖 Generated with Claude Code