Repository navigation
dl --prune: remove clone directories no workspace references #159
Description
Question
<cache>/repos/*/*/ accumulates clone directories forever: measured on the reference host, 52 clone dirs for 17 live devpod workspaces — 37 stale, 4.00 GB, against 7.86 GB live, plus 49 metadata records. Add dl --prune, shaped exactly like --purge (print the plan, name what is left standing and why, confirm, -y/--yes to skip, --force for the guarded arm), that classifies each clone dir with a three-constructor sum — Referenced(workspace_id) (a live workspace's source.localFolder resolves to this path), Orphaned(unsaved: str | None), Disputed(workspace_id) (metadata ties this dir to a workspace devpod still lists but sources elsewhere — issue #88's shape) — and removes only Orphaned(unsaved=None), using remove_tree (dl.py:553) not shutil.rmtree, dropping each removed clone's metadata record plus records whose dir is already gone.
Orphaned with unsaved work is refused by default and named in PR #134's words (13 of the 37 stale clones on the reference host hold unsaved work, two with unpushed commits); Disputed is never removable, not even under --force, and its report cites #88; .bare is never a candidate (0.08 GB for 7 repos, and it makes the next clone fast).
Two traps that must be tested, not just coded:
- Compare
Path.resolve()on both sides — a symlinked cache otherwise makes every clone look unreferenced, which is a total-loss bug. - Hold
repo_manager.lock_path(owner, repo)while classifying and removing a repo's clones, becauseworkspace_clone.py:184/:243populate a clone fully beforedevpod upregisters a workspace for it — an unlocked scan would classify a clone being born as an orphan and delete it.
Import the hardlink-aware size function from #153 for per-orphan and total bytes — do not grow a second one; ship without sizes if #153 has not landed.
Seams: dl.py:510 Refusal, :553 remove_tree, :702 purge_all_data, :1125 is_devlaunch_clone, :1197 workspace_ownership, :1503 list_workspaces, :2247 --purge dispatch; workspace_state.py:144 holds_unsaved_work; worktree/storage.py:365,372.
Tested: scratch XDG_CACHE_HOME only (never XDG_CONFIG_HOME), real git repos with a local bare as remote (PR #134's technique), four clones — referenced / orphaned-clean / orphaned-dirty / disputed — asserting only the clean orphan dies, --force promotes only the dirty one, Disputed survives --force, .bare untouched, idempotent second run, symlinked-cache regression, remove_tree refusal path exits 1 (skip under root, per PR #136's comment), and one devpod list + zero devpod status pinned in test/test_devpod_spawn_counts.py. Red-first with three recorded mutations: guard off, Disputed→Orphaned, classifier absent.
Decided on #155.
Activity
blooop commentedon Aug 9, 2026
breadcrumb (manager): amendments for this ticket's builder, from the re-verdict on PR #165 — the disk-size reporting surface this ticket consumes. Read that review; these are the parts that bind #159 specifically rather than #153.
The containment test is lexical, with no
resolve()._measurable_clonegates onis_devlaunch_clone— the same predicate that producesdevlaunch: trueand thatpurge_all_data→workspace_ownershipdeletes by — so"devlaunch": true⟺disk != nullholds by construction, which is what makes the surface safe to consume. But the reviewer flagged that the containment check is lexical: benign for reporting, and must not be reused as-is for this ticket'sReferencedclassification. A symlinked or non-canonical path could be classified wrongly, and this ticket's arms decide what gets deleted.known_bytes()has no production caller. It was exported for this ticket — "the bytes, whichever arm", for ordering and summing, explicitly not for rendering. Right now only a docstring stops a consumer printing a floor (PartlyUnreadable) as though it were a total. Land it with its caller here, and pin that a floor cannot reach a rendering path.Measured(0)for a hand-removed clone is documented but unpinned — the reviewer's mutation was green. This ticket classifies exactly those directories, so the case that has no test is the case this ticket is about. Pin it.The explanation is duplicated in five places and the figures in three. Whatever this ticket adds, do not make it six.
Standing hazard, since this ticket's seam is classification-then-deletion: three tests on these repos were found this run passing while guarding nothing, one of them on #153 itself — a fixture whose workspace sat outside the patched cache dir, so the guarded function ran zero times. Prove every new guard by mutation, under the full-suite selection, and state the selection. A guard on a deletion path that does not bite is worse than none, because it reads as protection.
blooop commentedon Aug 9, 2026
breadcrumb: claimed, building on a branch off current
main(which now carries both #165's disk surface and #148). The seams I intend to test at, before any of them exists:The classification. Three constructors —
Referenced(workspace_id),Orphaned(unsaved),Disputed(workspace_id, sourced_at)— and exactly one total function turning a status into a decision, with the codebase's existing exhaustiveness sentinel behind it (the same shapeWorkspaceSourceandDiskUsagealready use, so a fourth arm added later is a type-check failure rather than a silent fall-through into "delete"). No boolean beside the arm, so "deletable" is never a field a caller can read without having answered which arm it is.Referenced is decided by path equality after resolving both sides, not by the lexical containment test the reporting surface uses. Taking the manager's first amendment literally: the shipped predicate is safe for reporting because being wrong there costs a size; being wrong here costs somebody's clone. So the reference index is built from resolved source paths and matched by equality against candidate directories that are canonical by construction — enumerated with symlinks not followed, so a symlinked leaf is never a candidate at all.
Disputed is reached from the metadata record, not from a second guess at devpod. A candidate nothing sources, whose record names a workspace devpod still lists, is #88's shape and is never removable — under
--forceeither. That is also what keeps a workspace whose source devlaunch cannot read (an image or container workspace, or a devpod that changed under us) from making its clone look unreferenced.The unsaved probe runs only on the arm that could be deleted — it is the expensive half of the scan and asking it about a referenced clone would be work spent on a directory no answer could affect.
Removal is
remove_treeandRefusal, notrmtree, with #136's exit contract unchanged, and the metadata record dropped for each clone removed plus each record whose directory is definitively gone (onlyFileNotFoundErrorcounts as gone; a directory that cannot be looked at is still there).The repo lock is held across classify and remove, and there are two passes. The report is one pass, the act is another, and the act pass re-classifies under the lock and removes only what it also finds removable — so the set the user approved can shrink between report and confirmation but can never grow. I will say plainly in the docstring which race that closes and which it does not.
known_bytes()gets its production caller here, and it is a sum, not a rendering. The total across the orphans is itself Measured-or-floor: if any single walk came back a floor, the total is a floor and renders as one. That is the manager's second amendment — a floor must not be able to reach a rendering path as though it were a total.Also pinning the manager's third amendment where it bites: a clone directory removed by hand reads
0 B/{"exclusiveBytes": 0}on the existing size surface, which is a measurement of nothing and must stay distinguishable from-/null, which mean "not devlaunch's to measure". Those are exactly the directories this ticket classifies.Cost pinned alongside
--purge's: onedevpod list, zerodevpod status.blooop commentedon Aug 9, 2026
breadcrumb: PR #170.
dl --prunelands: it removes the clone directories under the cache that no live devpod workspace opens, and nothing else.The classification's seams
Three constructors and one total function over them.
decide(status, force)returnsRemove(usage)orKeep(because); it is the only place in the codebase where a directory becomes deletable, it is exhaustive with the_unhandled_*sentinel the source and usage sums already use, and its result is consumed at exactly one site — the scan, which files each directory into the plan'sremovingor itskeeping. There is no boolean beside a status for a later caller to read without having answered which arm it is. A directory whose status could not be determined is unrepresentable as deletable for the same reason: there is no arm for "unknown", and the two places an unknown could enter are closed before classification rather than defaulted through it — a repository directory that cannot be listed yields no candidates, and a live workspace whose source cannot be read as a path stops the whole command with nothing removed.Two fields sit inside
Orphanedrather than beside the classification, and both earn it.unsavedis only actionable on the arm that could be deleted, so "unsaved work on a clone that is staying anyway" cannot be said.usageis only ever wanted for bytes somebody gets back — which is also what keeps the O(files) walk off the other two arms entirely.Removethen carries the usage, so the total reclaimed is a sum over exactly the set that dies.--forcepromotes one arm.ReferencedandDisputedare not refusals to insist past.What the amendments changed
Referencedis equality of canonical paths. Candidates are canonical by construction — root resolved once, real directory names fromscandir, symlinked entries skipped — and workspace sources are resolved individually. Also:is_devlaunch_clonerefuses thegitRepositoryarm, and that answer is wrong here in the direction that matters, because refusing there declines to delete a workspace and refusing here deletes a clone. So this command reads every source arm that can carry a path.known_bytes()has its production caller, and it is a sum rather than a rendering.disk_usage.total_usage()returnsMeasuredorPartlyUnreadable, so one floor among the orphans makes the whole total a floor and it renders with≥.known_bytesalso orders the report, which is the comparative question it was exported for. Nothing prints it.Measured(0)for a hand-removed clone is pinned on the surface it belongs to:0 B/{"exclusiveBytes": 0}, distinguishable from-/null("not devlaunch's to measure"). Those are exactly the directories this command classifies.disk_usage's docstring and README's existing section; the new README section links to it.report_refusalswas extracted frompurge_all_dataso the "a container wrote them as a different user" advice — the only guessed part of that report — has one copy rather than two.Mutation proofs
Selection, for every line below:
pixi run pytest -qin the default environment — the full suite, with-m 'not e2e'coming from pytest's ownaddopts, which is exactly the selectionpixi run ciruns throughcoverage run -m pytest. Each mutation was applied alone tomain-plus-this-branch, the whole suite run, and the mutation reverted. Baseline: 1283 passed, 23 deselected.Disputedcollapses intoOrphaned.barebecomes a candidateexists()instead of the FileNotFoundError-only checkknown_bytes, losing the floor--forcepromotes theDisputedarm tooThe two I found hollow, which is why the exercise was worth running
I deliberately ran mutations I expected to survive. One did, as intended; two more survived that I had expected to be red, and both were my own guards passing while guarding nothing. Both are fixed and both are now red.
A git source naming no path survived its own test. Removing the arm does not expose the clone, because a clone that stops being recognised as opened still carries a metadata record naming a workspace devpod lists — so it lands in
Disputedand survives for the wrong reason. The test asserted the directory was still there, which was true either way. It now asserts the reason on that directory's own report line, and a live clone described to its owner as a records disagreement to go and fix is caught.Following symlinked entries survived too, and for a sharper version of the same mistake: followed, the link becomes a candidate, is weighed through to the real clone, is planned for removal, and is then refused by
remove_tree— so the link survives, the clone survives, and both of the test's assertions held. The only difference was a directory in the plan that was never devlaunch's to name, and exit 1 on a cache with nothing wrong with it. The test now asserts the path never reaches the report, and the exit code.One more thing the mutations showed, which is not a defect
The mutation that turns the classification's unsaved guard off does not kill
test_a_clone_holding_work_saved_nowhere_else_is_kept, because the pre-removal re-probe catches it and withholds the directory. The two guards are genuinely independent layers, each separately proven — the classification by the report tests, the re-probe by the write-while-deciding test — and the outcome test bites only when both are gone. That is the property it should have, but it is worth stating rather than leaving for a reviewer to discover.One earlier defect this exercise also caught, before the mutations: the fixture originally stubbed
dl.subprocess.run, which patches the sharedsubprocessmodule and therefore answeredgit statusandgit logtoo — with a clean exit and empty output, which reads as "this clone holds nothing". The central guard was passing while the clone with an unpushed commit in it was deleted. Non-devpod commands now run for real.And the full-suite selection caught one more thing an isolated run did not: my first "cannot be located" trigger was a symlink loop, which Python 3.10 raises on and 3.13 resolves silently, so the guard existed on one supported version and not the other. It is now triggered by something both versions refuse.
Parked
--prunedoes not refresh the completion cache after removing clones. Nothing asked it to, and adding a backgrounddlspawn to a deletion command is a decision, not a tidy-up.git cloneis still filling. It cannot cover the gap betweenensure_workspacereturning anddevpod upregistering a workspace, because devpod only learns about the clone after the lock is released. Closing that needs the persisted workspace id Build: persist the devpod workspace id instead of re-deriving it, and reconcile the orphans #88 is about.Gate
pixi run cigreen locally end-to-end in both the default environment and py310, as one task each:1283 passed, 23 deselected, pylint 10.00/10 in both. On the PR: every check green,codecov/patchandcodecov/projectincluded, pluse2eand thegatejob.One thing worth passing on to whoever builds here next: the first push failed
ci (py313)on pylint with three warnings in a file that had just been written, while localpixi run ciwas green.pixi run pylintlintsgit ls-files '*.py', which does not list a file that has not beengit added yet — so a new test file is invisible to local lint until it is staged. Add the file, then run CI.blooop commentedon Aug 9, 2026
breadcrumb: review posted on PR #170 — verdict Request changes (both axes independently, plus four executed inputs that put a spec-protected clone into the removal set, incl.
Disputednever firing on #88's measured host state). Node isneeds attention.blooop commentedon Aug 9, 2026
breadcrumb: the review's seven blocking findings are fixed on PR #170, head
0454f7e. On L1 I reached outcome (a): theDisputedarm can be made to fire on devlaunch#88's measured state without adopting any of #88's work, so #155's conclusion stands and its reasoning was wrong. #155's reasoning is corrected below rather than left standing.Selection for every red/green claim on this comment:
pixi run pytest -qin the default environment — the full suite, the same selection the reviewer used. Baseline after these changes:1308 passed, 23 deselected. Each mutation was applied alone, the whole suite run, the mutation reverted, and — this is the part last run got wrong — the failure list checked to confirm the mutation kills the test it is credited to, by node id, not merely that the suite went red.L1 — outcome (a), and why #155's reasoning was wrong
#155 recorded that the dependency on #88 was designed out because "the
Disputedarm makes--prunesafe under exactly the state #88 exists to repair". The reviewer showed the arm as built joined devlaunch's record to devpod's listing by workspace id — and the id is precisely what #88's scheme change broke apart. On that ticket's measured host devpod's id is the old-scheme one while metadata carries the new-scheme one; the lookup misses, the healthy checkout falls toOrphaned, and--prunekeeps the config-only stub while deleting the live workspace's only real checkout. 36 of 39.The arm can fire on that state, because the disagreement is in the data already in hand — it just is not visible through the id. devpod's stale record still names the repository exactly (
<repos_dir>/<owner>/<repo>/<old-leaf>), and #88's own published diagnostic is the discriminator: a recorded folder with no.gitin it. So a live workspace devpod records inside a repository's clone tree, at something that is not a checkout — gone (35 of #88's 36) or a config-only stub (1 of 36) — disputes that repository's clones and no others. Same arm, same report, same pointer at #88; a wider funnel into it.What that needs is one stat per live workspace on data
--prunealready reads. What it does not need is anything #155 placed off this map: nodevpod_workspace_idpersisted at create, no reconciliation rewriting devpod'sworkspace.json, no new state, no schema change, no writes at all. The boundary #155 drew — "adopting it would drag a migration that rewrites devpod's ownworkspace.jsononto a latency-and-disk map" — is untouched.The cost is honest and worth stating: on #88's host all seven affected repositories become undeletable and are named, so
--prunereclaims nothing there until the records are fixed. On #155's own measured host the shape was 0 today, so nothing changes — its 37 stale clones are still prunable. The one-way pointer to #88 stays.Red:
_clone_statusignoring the misplaced index, and separately "every recorded folder counts as a clone", each kill all three of the new #88 tests (3 failed, 1301 passed, 23 deselected).Per finding: the fix, and the mutation that now turns it red
Every row below survived the full suite before this change. Each names the test the failure list actually reported.
_clone_statusignores the misplaced index →TestADevpodRecordThatPointsAtNothing×3,3 failed, 1301 passed.gitis not a clone_is_populated_clonealways true → same three,3 failed, 1301 passedTestAWorkspaceThatAppearedWhileTheQuestionWasOpen×2,2 failed, 1302 passedReferencedis containment, not equality — at or undertest_a_live_workspace_opening_a_subdirectory_still_holds_the_clone,1 failed, 1303 passedlocalFolderdevlaunch cannot read is its own arm and stops the command;image/containeropen no folder here and do nottest_a_local_folder_devlaunch_cannot_read_stops_the_whole_command,1 failed, 1303 passeddecidekeeps ittest_a_repository_that_will_not_answer_is_not_reported_as_holding_nothingandtest_a_clone_whose_repository_will_not_open_is_kept_with_nothing_typed,2 failed, 1302 passed--forceanswered rides on eachReclaimable;PrunePlan.forceis gonetest_it_does_not_turn_off_the_re_probe_for_a_clone_it_promoted_nothing_about,1 failed, 1303 passedtest_the_plan_names_what_forcing_this_one_destroys,1 failed, 1303 passedTestACacheReachedThroughASymlink::test_a_workspace_that_names_its_clone_through_a_symlink_still_holds_it,1 failed, 1303 passedtest_a_repos_dir_reached_through_a_symlink_still_matches_its_clones, on its reason assertion,1 failed, 1303 passed.barepath canonicalised.bareassertion — and.barewas really deleted,1 failed, 1303 passedlocal_path→test_a_record_written_through_a_symlink_still_disputes_its_clone(run under--force, so the unsaved guard cannot be what holds it),1 failed, 1303 passedReferencedtests assert the reasonReferencedarm removed →9 failed, 1295 passed, including the two the reviewer showed it did not killsudo rm -rfadvice names the scan root →test_the_command_to_paste_names_the_obstruction_and_nothing_else,1 failed, 1303 passednothing_to_doignores stale records →test_a_cache_with_only_a_stale_record_left_is_not_nothing_to_prune,1 failed, 1303 passedtest_it_blocks_after_the_answer_while_another_process_holds_the_lock,1 failed, 1303 passedThe reviewer's missing test for the record-resolve mutation is in, in that shape.
Three more hollow guards, found the same way
I ran mutations I expected to survive, aimed at the deletion decision. Three did, all on code that was already correct and held by nothing:
.gitis a file — a linked git worktree. Both places this command asks "is there a repository here" would have said no, and both wrong answers are deletions.All three now have tests, and re-running each mutation turns exactly those tests red (
1 failed,1 failed,2 failedof1308).Two deliberate deviations, stated rather than slipped in
devpod listto one per pass. Reclaim orphaned disk: stale clone directories and left-behind images #155 pinned one. Whether a clone is referenced is the single thing a plan cannot be re-checked against from disk, and the builder's own breadcrumb promised the act pass "re-classifies". The second listing is O(1), is paid only after somebody has said yes to a deletion, and zerodevpod statusis unchanged and still pinned. A run that removes nothing still pays one.XDG_CONFIG_HOME. Ruled: keep the suite scoping both. It is strictly safer — a personalconfig.tomlcan pointrepos_dirback at the real cache — and the new symlinked-repos_dirtest depends on being able to write one.Gate
pixi run cigreen end-to-end as one task in both the default environment and py310:1308 passed, 23 deselected,ruff checkclean,tyclean, pylint 10.00/10 in both.And a real invocation, which nobody had done on this branch: this tree run against a scratch
XDG_CACHE_HOMEwith real git and the host's real devpod. It removed the dead clone and the stray directory, kept the one holding an untracked file, and kept the one whose.githad been broken —holds work git could not be asked about (fatal: not a git repository ...) -- add --force to remove it anyway. That last line is the finding all three review axes found independently, behaving.Still parked, unchanged
#160's Docker boundary line; the completion cache after a prune; and the register window the lock cannot cover, which still needs the persisted workspace id #88 is about — this change narrows what that window can cost, it does not close it.
blooop commentedon Aug 9, 2026
breadcrumb: re-review posted on PR #170 — verdict Request changes (Standards: request changes, Spec: approve, not merged). L1 outcome (a) holds: the
Disputedarm fires on devlaunch#88's measured state, confirmed by execution twice independently, so #155's conclusion stands and does not re-open — no park owed. Six of the seven prior blocking findings are closed; L5/Standards 1/Spec S3 stays open — theCouldNotAskarm has three producers and only one is held (two full-suite-green mutations), and it cannot fire at all when an ancestor directory is a git repository, where a clone holding an unpushed commit and an untracked file was deleted with no--force. Node isneeds attention.blooop commentedon Aug 9, 2026
breadcrumb: re-review of PR #170 on head
0454f7ereturned Request changes (Standards: request changes; Spec: approve — not merged or reranked). That is the second consecutive gate failure for this node, so per the lifecycle rule it parks rather than taking a third retry.The good news first: L1 holds and #155's decision survives — no decision-level park is owed. The prior round's falsification (that
--prune'sDisputedarm never fires on #88's measured state, and would delete 36 of 39 live checkouts) was answered by outcome (a), and the retry's answer was confirmed twice by independent execution: rebuilding #88's published shapes kept the live checkouts even under--force, and taking this host's owndevpod list— which carries #88's shape today, 4 of 5localFolderworkspaces recording a folder with no.git— showed the healthy new-scheme checkout kept by the path-keyedMisplacedarm with no metadata record present at all, so the record-keyed arm cannot have been what saved it.handoff
What blocks the merge: an input still gets a protected directory into the removal set, and it destroys unsaved work.
workspace_state._is_a_repositoryasks "is there a repository here?" of the clone directory, but_gitrunsgit … cwd=clonewith no--git-dirand no ceiling. When.gitis present but unusable, git's repository discovery walks up the parent chain and answers about an ancestor repository instead. It therefore never refuses, so theCouldNotAskarm — the guard that is supposed to catch exactly this — is unreachable on that path.Executed on head
0454f7e, realdl --prune -y, no--force, on a host whose home directory is itself a git repo (dotfiles-in-git, which is common):Isolated at predicate level:
read_clonereturnsCloneState(branch='main', unsaved=NothingToLose())while the clone is actually on branchworkwith an unpushed commit — it is describing the ancestor repo, not the clone.Both axes found this independently from opposite directions. Standards found the guard hollow from the coverage side: two of the three
CouldNotAskproducers (workspace_state.py:242,:253) are uncovered, and mutating each to report "clean" leaves the full suite green — so nothing in CI is holding that arm. Spec's SP3 reproduces the same green.The fix is small; the reason this parks is that it is the third round and the finding is data loss, which should not be rushed. Constrain discovery so it cannot escape the clone —
--git-dir/--work-treeexplicitly, orGIT_CEILING_DIRECTORIES, so an unusable.gitproduces a refusal that reachesCouldNotAskinstead of a confident wrong answer about someone else's repo. Then cover the two uncovered producers with mutation-checked guards, because green currently proves nothing there.This is not confined to unmerged code.
workspace_stateand #134's unsaved guard are already merged and are whatdl --purgestands on, so the same ancestor-walk almost certainly reaches shipped behaviour. Filed separately so it is not gated behind this PR — see the linked issue.Also still open (non-blocking, carried): message chains;
dl.pyis now 3610 lines;Reclaimable.owner/repo; thespawnsfixture shape; S5 (parked → #160); S7.What was verified by execution, so the next agent need not redo it: three independent full-suite baselines (
1308 passed, 23 deselected);pixi run ciand-e py310 ciend to end with pylint 10.00/10, ruff andtyclean in both; 32 mutation rows with stated selections;tyexhaustiveness across all eight sums; #88's diagnostic on this host; two independent end-to-end L1 reproductions. Not verified: the reference-host figures (1017 ms, 4.00 GB, 52/37/49), the migration→classification sequence (read from source only), and./dev.sh(deliberately skipped — it rebinds a host-widedl-nextsymlink that concurrent work shares).blooop commentedon Aug 10, 2026
breadcrumb — a rebase hazard for whoever unparks this. #171's fix (PR #173) replaces
holds_unsaved_work'sOptional[str]— which conflated "nothing to lose" with "could not tell", bothNone, both meaning delete freely — with a total sum whoseCouldNotTellarm refuses the delete, enforced bytyat every consumer.PR #170's branch was read (read-only) during that review. It does consume this reading, at
dl.py:1840and:455— but it carries its own parallelUnsavedWorksum plus anobjection()helper that flattens it back toOptional[str], and its consumers still testif unsaved:(:3464) andobjection(status.unsaved)(:1450,:507).So "it will pick up the new arm when it rebases" is not automatic. Mechanically taking main's side of the conflicts would leave #171's sentinel alive inside
--prune— in the one command on this map whose whole job is deleting clone directories, and on a branch already parked for a data-loss finding of its own. The rebase needs to deleteobjection()and let every--pruneconsumer match the sum arm-by-arm, the wayrmand--ls --jsonnow do.Also relevant to this node: #174 was filed during the same review — the guard reads
record.local_pathwhileremove_workspace_by_idfalls back to the derived path andrmtrees that, so the guard and the delete can name different directories. Reproduced on merged main: exit 0, no--force, unsaved work destroyed.--prunereads the same guard, so it inherits that too.