From d437edfa68b3b30ee13b4ac01b03a38cf74e7201 Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Fri, 14 Aug 2026 07:27:03 +0100 Subject: [PATCH 1/2] Remove the clone directories nothing opens any more `dl --purge` is all-or-nothing, so the only way to reclaim the 4.00 GB of dead clone directories measured under one cache was to destroy the 7.86 GB of live ones and every bare cache with them. `dl --prune` removes exactly the directories no live devpod workspace opens, in `--purge`'s own shape: print the plan, name what is left standing and why, confirm, `-y` to skip. Three constructors and one total function over them. `decide` returns Remove or Keep, it is the only place a directory becomes deletable, and its result is consumed once -- so a directory cannot be planned for removal without a reason for keeping it having been ruled out, and a status nobody handled stops the build rather than falling through into a deletion. `unsaved` and `usage` live inside the Orphaned arm, which is what makes "unsaved work on a clone that is staying" unsayable and keeps the unbounded walk off the two arms whose bytes nobody gets back. Referenced is containment, not equality: a workspace opened on a subdirectory of a clone needs the clone. It is decided by canonical paths rather than by the lexical containment `is_devlaunch_clone` uses -- refusing there declines to delete a workspace, and refusing here deletes a clone. A live workspace whose source cannot be read as a path stops the command outright, because while one exists no directory can honestly be called unreferenced. The pass that acts classifies again, per directory, under the lock, against a fresh listing -- a launch that completes while the report is on screen registers a workspace for a directory already in the plan. What `--force` answered rides on each Reclaimable rather than over the whole plan, so it cannot switch the re-check off for clones it promoted nothing about. devlaunch#88's state is joined by path, not by workspace id. The id is what that ticket's scheme change broke, so a record naming a workspace devpod still lists could never match on the host the Disputed arm was written for. known_bytes gets its first production caller, as a sum rather than a rendering: total_usage returns Measured or PartlyUnreadable, so one floor among the orphans makes the whole total a floor and it prints as one. Rebased onto devlaunch#171/#173/#174, which landed while this was parked and which solved two of this branch's own problems better: - the unsaved-work sum this branch added to `workspace_state` is dropped entirely in favour of the one on `main`. `main`'s is the same three arms under different names (`CouldNotTell` for `CouldNotAsk`) plus the half this branch did not have: `_git` names its repository with `--git-dir` and `--work-tree`, so git's discovery cannot walk up to an ancestor repository and answer about that one instead. That was the blocking finding on the last review of this branch, executed as a deletion of an unpushed commit and an untracked file with no `--force` typed. - `--prune` keeps its own two-answers-to-one function, `_objection`, but it is now total over `main`'s arms with `unhandled_unsaved` behind it. One behaviour follows from `main`'s sum and is a change from this branch's earlier shape: a directory git cannot read as a repository at all -- stray notes somebody left in the cache -- is `CouldNotTell` and is therefore *kept* and named, with `--force` as the way past it. This branch used to tell that case apart with a `.git` probe of its own and remove it. Deleting the second opinion is the point: `dl rm` already refuses on that arm, and the probe that separated them answered "empty" for a directory dl never looked inside. --- AGENTS.md | 6 + README.md | 87 +- devlaunch/disk_usage.py | 33 +- devlaunch/dl.py | 995 ++++++++++++++++++- pyproject.toml | 1 + test/test_devpod_spawn_counts.py | 44 + test/test_workspace_state.py | 28 + test/unit/test_disk_usage.py | 33 + test/unit/test_prune_orphaned_clones.py | 1213 +++++++++++++++++++++++ test/unit/test_workspace_source.py | 37 +- 10 files changed, 2450 insertions(+), 27 deletions(-) create mode 100644 test/unit/test_prune_orphaned_clones.py diff --git a/AGENTS.md b/AGENTS.md index 7788c2f4..37b5fcd8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -72,6 +72,12 @@ Two things to know about it: `~/.devpod`. `DEVPOD_HOME` is still what scopes devpod itself, and is what `test/conftest.py` sets for the suite. + **`dl --prune`** is scoped by the same variable and more narrowly: it removes + clone directories under `repos_dir` and never touches a devpod workspace at + all, so a scratch cache leaves it with nothing to find. It prints its plan and + asks before removing anything, and `-y` is what skips the question — so a + scratch run of it is a read-only way to see the classification. + ### Inside the devcontainer: one build, and it is `pixi run dl` Everything above is about the host. This repo's devcontainer already installs the diff --git a/README.md b/README.md index e986228f..d98b6a1b 100644 --- a/README.md +++ b/README.md @@ -321,8 +321,8 @@ existed — picks the tools up on its next `dl restart`. | `dl --ls --json` | The same list as JSON, with each workspace's repo, branch, state and [unsaved work](#cleaning-up-workspaces) — for tools that decide what to clean up | | `dl --ls --size` | Add [what deleting each workspace would free](#how-much-disk-a-workspace-costs). Opt-in: it walks every file in the clone | | `dl --install` | Install shell completions | +| `dl --prune [-y] [--force]` | Remove [the clone directories no workspace opens any more](#pruning-the-clones-nothing-opens) — and nothing else | | `dl --purge [-y]` | Remove all devlaunch data — [the workspaces devlaunch created](#what-purge-deletes), and its caches | -| `dl --prune-worktrees [days]` | Remove unused worktrees (default: 30 days) | | `dl --refresh` | Refresh completion cache | | `dl --help, -h` | Show this help | | `dl --version` | Show version (an editable install also names the tree it runs from) | @@ -419,6 +419,91 @@ directory yourself if you meant to: following the link would empty a directory you never named, and removing just the link would report a clean sweep while your clones sat on the other volume. +### Pruning the clones nothing opens + +A workspace per branch means clone directories accumulate under the cache, and +until now nothing removed them: measured on one host, **52 clone directories for +17 live devpod workspaces — 37 of them attached to nothing, 4.00 GB, against +7.86 GB still in use.** `--purge` is the wrong tool for that, being +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 +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: + +- **a live workspace opens it** — kept, and named with the workspace that has + it. "Opens" means at *or under*: a workspace opened on a subdirectory of a + clone still needs the clone; +- **nothing opens it** — removed, unless it holds work that exists nowhere else, + or `git` would not say what it holds. A clone a container wrote as another + user is unreadable rather than empty, and "cannot tell" is kept, not removed; +- **`dl`'s records and devpod's disagree about it** — kept, always. This is + [#88](https://github.com/blooop/devlaunch/issues/88)'s shape. On that ticket's + host, 36 devpod workspaces out of 39 recorded a source folder that was gone or + was a config-only stub, while the real checkout sat beside it under a newer + naming scheme — so a perfectly healthy clone was opened by nobody, and the + stub was the only thing anything pointed at. `--prune` will not guess which + clone such a workspace needs: it keeps every clone of that repository and + names the record to go and fix. `--force` does not move any of them. + +Note that *every* directory two levels under `/devlaunch/repos` is a +candidate — a stray directory somebody left there is an orphan like any other. +The cache is `dl`'s to manage; things that are not clones do not belong in it. + +``` +$ dl --prune +Clone directories under /home/you/.cache/devlaunch/repos: + +Removing 2 that nothing references -- 1.4 GiB: + - /home/you/.cache/devlaunch/repos/blooop/bencher/bencher-test1-pipagito (1.1 GiB) + - /home/you/.cache/devlaunch/repos/blooop/devlaunch/devlaunch-t1-vebilote (317.0 MiB) + +Leaving 3: + - /home/you/.cache/devlaunch/repos/blooop/devlaunch/devlaunch-main-zovomobo: workspace devlaunch-main-zovomobo still opens it + - /home/you/.cache/devlaunch/repos/blooop/wayfinder/wayfinder-devlaunch-kilarabo: holds 2 unpushed commit(s) -- add --force to remove it anyway + - /home/you/.cache/devlaunch/repos/blooop/rockerc/rockerc-main-ludomane: devpod lists workspace rockerc-main-ludomane and sources it at /home/you/.cache/devlaunch/repos/blooop/rockerc/main; see devlaunch#88 + +Dropping 12 record(s) of directories already gone. + +Are you sure? [y/N] +``` + +`-y` skips the question. **A clone holding uncommitted or unpushed work is kept +and named**, in the same words [`dl rm`](#cleaning-up-workspaces) +refuses in — 13 of those 37 stale clones did, two of them with real unpushed +commits, so this is load-bearing rather than a formality. `--force` promotes +that one case and nothing else. Erring this way costs you a flag; erring the +other way costs work that cannot be recovered. + +The sizes are the same *exclusive* bytes `dl --ls --size` reports, and they mean +[the same thing](#how-much-disk-a-workspace-costs): what removing that directory +would actually free, not what `du` would print. Where a walk could not read +something the figure reads `≥` and so does the total, because a floor printed as +a total is a cleanup tool telling you a directory is small when it is not. + +Directories that will not come away are named the same way [a purge names +them](#when-part-of-the-cache-will-not-go), the rest still go, and the exit +status is `1`. + +**Nothing here runs on its own.** A full scan measured 1017 ms on that host — +about two warm launches — and it gets slower exactly as the cache gets fuller, +so it is never on a launch path and never folded into `dl --ls`. Answering `n` +*is* the read-only view; there is no separate flag for it. It costs one +`devpod list` to build the plan and no `devpod status` at all, because whether a +workspace is running has no bearing on whether a directory is opened by one. A +run you say yes to pays a second `devpod list` before it removes anything, and +classifies every directory again: a launch that finishes while the report is on +screen registers a workspace for one of the directories in the plan, and that is +the one thing the plan cannot be re-checked against from disk. The set you +approved can shrink between the report and the act. It can never grow. + +It also drops the `metadata.json` records of directories that are already gone. +That file was append-only in practice — 49 records for 17 live workspaces on the +same host — and this is the first thing that prunes it. + ### Cleaning up workspaces One workspace per branch means workspaces accumulate, and `--purge` is the wrong diff --git a/devlaunch/disk_usage.py b/devlaunch/disk_usage.py index 9a369b47..d9292008 100644 --- a/devlaunch/disk_usage.py +++ b/devlaunch/disk_usage.py @@ -58,7 +58,7 @@ import stat from dataclasses import dataclass from pathlib import Path -from typing import Any, Dict, List, NoReturn, Tuple, Union +from typing import Any, Dict, Iterable, List, NoReturn, Tuple, Union # st_blocks is counted in 512-byte units by POSIX, whatever the filesystem's own # block size is. @@ -230,6 +230,37 @@ def known_bytes(usage: DiskUsage) -> int: _unhandled_usage(usage) +def _unreadable_in(usage: DiskUsage) -> Tuple[Path, ...]: + """The doors this usage could not open -- none, for a complete walk.""" + if isinstance(usage, Measured): + return () + if isinstance(usage, PartlyUnreadable): + return usage.unreadable + _unhandled_usage(usage) + + +def total_usage(usages: Iterable[DiskUsage]) -> DiskUsage: + """What removing all of *usages*' trees together would free. + + A sum with one floor in it is a floor, and this returns the arm that says + so. That is the whole reason a total lives here rather than in the caller: + adding :func:`known_bytes` up gives an integer that has lost which kind of + answer it is, and an integer printed as a size is a floor read as a total -- + the one mistake every other function in this module is shaped to prevent. + + Which bytes these are, and why they do not add up to the disk a cache holds, + is the module docstring's business and is not restated here. + """ + known = 0 + unreadable: List[Path] = [] + for usage in usages: + known += known_bytes(usage) + unreadable.extend(_unreadable_in(usage)) + if unreadable: + return PartlyUnreadable(known, tuple(unreadable)) + return Measured(known) + + def usage_as_json(usage: DiskUsage) -> Dict[str, Any]: """How a usage reads to a tool: one key, and the key says which kind it is. diff --git a/devlaunch/dl.py b/devlaunch/dl.py index 991cd159..f57e459e 100644 --- a/devlaunch/dl.py +++ b/devlaunch/dl.py @@ -42,6 +42,7 @@ from .worktree.config import get_worktree_config from .worktree.locks import hold_lock from .worktree.migration import migrate_cache +from .worktree.models import WorktreeInfo from .worktree.workspace_clone import WorkspaceCloneManager from .xdg import devlaunch_cache @@ -859,21 +860,38 @@ def purge_all_data() -> int: # either -- an exit code cannot distinguish "removed most of it" from # "removed none of it", and the difference is the whole news, so the report # carries it and the exit code only says the job is unfinished. - print(f"Removed what was permitted under {cache_dir}. These refused:") + report_refusals( + refused, + f"Removed what was permitted under {cache_dir}. These refused:", + (cache_dir,), + ) + return 1 + + +def report_refusals( + refused: Sequence[Refusal], headline: str, remove_by_hand: Sequence[pathlib.Path] +) -> None: + """Print what would not come away, and the one thing that usually clears it. + + Shared by the two commands that remove directories, because a second copy of + this advice is a second copy to keep true -- and the advice is the part most + likely to change, being the only part that is a guess. + + Hedged, because the cause is not knowable from here. A container writing as + another user is the common one, but a read-only mount, `chattr +i` and a busy + mountpoint all land in the same report -- and for the last two this command + does not help either. Saying so flatly would be wrong more often than the + errno beside each path is. + """ + print(headline) for refusal in refused: print(f" - {refusal.path}: {refusal.reason}") print() - # Hedged, because the cause is not knowable from here. A container writing - # as another user is the common one, but a read-only mount, `chattr +i` and - # a busy mountpoint all land in the same report -- and for the last two this - # command does not help either. Saying so flatly would be wrong more often - # than the errno above is. print("Usually this means a container wrote them as a different user, and:") - # Quoted: cache_dir comes from $XDG_CACHE_HOME or $HOME, and a space in it - # turns a pasted `sudo rm -rf` into two targets, the first of them wrong. - print(f" sudo rm -rf {shlex.quote(str(cache_dir))}") + # Quoted: these paths descend from $XDG_CACHE_HOME or $HOME, and a space in + # one turns a pasted `sudo rm -rf` into two targets, the first of them wrong. + print(f" sudo rm -rf {' '.join(shlex.quote(str(path)) for path in remove_by_hand)}") print("clears them. Check the reasons above first -- it does not fix all of them.") - return 1 # Regex to match owner/repo[@branch] format (not a path, not already a URL) @@ -1112,7 +1130,7 @@ class GitRepository: @dataclass(frozen=True) class UnrecognisedSource: - """A `devpod list` source devlaunch has no reading for, kept verbatim. + """A `devpod list` source that opens no directory on this machine. Reachable rather than defensive: devpod's workspace source also carries `image` and `container`, so `devpod up ubuntu:24.04` lands here. Holding the @@ -1120,14 +1138,36 @@ class UnrecognisedSource: replaced was typed as a path and filled with `str(the dict)`, so the one thing a caller could not do with it was read what devpod had said. - It deliberately has no path and no URL. There is nothing to make it say - where the source lives, because devlaunch does not know. + It deliberately has no path and no URL, and that is a *fact about the + workspace* rather than a gap in devlaunch's reading: an image workspace + mounts no folder here, so no clone directory can be at risk from it. A + source that does name a folder and could not be read is the opposite answer + to that same question and is :class:`UnreadableLocalFolder`. """ payload: Mapping[str, Any] -WorkspaceSource = LocalFolder | GitRepository | UnrecognisedSource +@dataclass(frozen=True) +class UnreadableLocalFolder: + """devpod says this workspace opens a folder, and devlaunch cannot say which. + + Split from UnrecognisedSource because the two are opposite answers to the + one question a deletion has to ask, and sharing an arm made the dangerous + one silent: `--prune` reads "no path" as "contributes no path and no alarm", + which is right for an image workspace and wrong for this -- a live workspace + really is opening a directory, and while devlaunch cannot say which, there + is no clone it can honestly call unreferenced. + + Reached by a `localFolder` devpod filled with something that is not a + non-empty string: an object, a number, a list. The payload is kept whole so + the report can show what devpod actually said. + """ + + payload: Mapping[str, Any] + + +WorkspaceSource = LocalFolder | GitRepository | UnrecognisedSource | UnreadableLocalFolder def _unhandled_source(source: NoReturn) -> NoReturn: @@ -1174,7 +1214,12 @@ def parse_workspace_source(source: Mapping[str, Any]) -> WorkspaceSource: Neither is an unreadable *listing*, unlike a `source` that is not an object at all: the object is right here and can be kept whole. It is a source dl - cannot read, which is an arm. + cannot read, which is an arm -- and *which* arm turns on whether devpod + claimed a folder here at all. A `localFolder` holding an object or a number + is a workspace that opens a directory devlaunch cannot name; a source with + no `localFolder` key, or an empty one, opens no directory here, which is + what an `image` or `container` workspace is. Only the first can put a clone + at risk, so only the first is an alarm. Note this is the one link in the chain the type checker cannot stand in for a test. A *reader* that misses an arm is a build failure, but this is a @@ -1184,6 +1229,9 @@ def parse_workspace_source(source: Mapping[str, Any]) -> WorkspaceSource: path = _readable_text(source, "localFolder") if path is not None: return LocalFolder(path) + claimed = source.get("localFolder") + if claimed is not None and claimed != "": + return UnreadableLocalFolder(dict(source)) url = _readable_text(source, "gitRepository") if url is not None: return GitRepository(url) @@ -1205,7 +1253,7 @@ def describe_source(source: WorkspaceSource) -> Tuple[str, str]: return "local", source.path if isinstance(source, GitRepository): return "git", source.url - if isinstance(source, UnrecognisedSource): + if isinstance(source, (UnrecognisedSource, UnreadableLocalFolder)): return "unknown", json.dumps(source.payload) _unhandled_source(source) @@ -1284,7 +1332,7 @@ def is_devlaunch_clone(workspace: Workspace, cache_dir: pathlib.Path) -> bool: # Purely lexical, so a clone whose directory has already been removed is # still recognisable from the source devpod kept. return path != pathlib.PurePath(cache_dir) and path.is_relative_to(cache_dir) - if isinstance(source, (GitRepository, UnrecognisedSource)): + if isinstance(source, (GitRepository, UnrecognisedSource, UnreadableLocalFolder)): return False _unhandled_source(source) @@ -1350,6 +1398,907 @@ def workspace_ownership( return WorkspaceOwnership(mine=tuple(mine), foreign=tuple(foreign)) +@dataclass(frozen=True) +class Referenced: + """A live devpod workspace opens this exact clone directory.""" + + workspace_id: str + + +@dataclass(frozen=True) +class Orphaned: + """Nothing opens this directory and no record ties it to a live workspace. + + *unsaved* is what deleting it would destroy, in the words `dl rm` + already refuses in -- and it is a three-armed answer rather than a + description-or-None, because "git would not say" is not "nothing to say". + It sits inside this arm rather than beside the classification because it is + only ever actionable here: "unsaved work on a clone that is staying anyway" + is a sentence this type cannot say. + + *usage* is here for the same reason, and it earns its place twice over. The + walk behind it is O(files) with no ceiling, and this is the only arm whose + bytes anybody is going to get back -- so putting it here is what keeps the + other two arms from being walked at all. + """ + + unsaved: workspace_state.Unsaved + usage: disk_usage.DiskUsage + + +@dataclass(frozen=True) +class Disputed: + """devpod lists the workspace this directory's record names, elsewhere. + + devlaunch#88's shape, and the reason `--prune` does not wait on #88: 36 of + 39 devpod records on that ticket's host pointed at folders that no longer + existed, and under that state a healthy clone at the *new* path is sourced + by nobody. Read as an orphan it would be deleted; read as referenced it + would silently hide disk. It is neither -- it is two records disagreeing, + and the answer to a disagreement is to keep the directory and say so. + """ + + workspace_id: str + sourced_at: str + + +CloneStatus = Referenced | Orphaned | Disputed + + +def _unhandled_status(status: NoReturn) -> NoReturn: + """Reject a clone status nobody handled -- at type-check time, not at runtime. + + The same device as _unhandled_source, and it matters more here: the function + it guards is the only place a directory becomes deletable, so an arm that + fell through it would fall through into a deletion. + """ + raise AssertionError(f"Unhandled clone status: {status!r}") + + +@dataclass(frozen=True) +class Unopposed: + """Nothing objected to removing this directory.""" + + +@dataclass(frozen=True) +class Insisted: + """`--force` carried this directory past an objection, named in *despite*. + + Carried on the decision rather than read from the plan's `--force` flag, and + that difference is a deletion. A plan-wide boolean says "the user insisted" + about every directory in the plan, including the ones nothing objected to -- + so the later re-probe, which exists to catch work written while the user was + reading the report, was skipped for clones `--force` had not promoted at + all. A promotion belongs to the directory it promoted. + """ + + despite: str + + +Promotion = Unopposed | Insisted + + +def _unhandled_promotion(promotion: NoReturn) -> NoReturn: + """Reject a promotion nobody handled -- at type-check time, not at runtime.""" + raise AssertionError(f"Unhandled promotion: {promotion!r}") + + +@dataclass(frozen=True) +class Remove: + """This directory goes, this is what it gives back, and this is what was + insisted past to get here. + + The bytes travel with the decision rather than beside it, so "what this run + reclaims" is a total over the things it is actually removing and cannot be + assembled from a different set than the one that dies. The promotion travels + with it for the same reason one layer down: it is the only record that + `--force` answered *this* directory's objection, and the report and the + second pass both need to know which. + """ + + usage: disk_usage.DiskUsage + promotion: Promotion + + +@dataclass(frozen=True) +class Keep: + """This directory stays, and *because* says why, for a person to read.""" + + because: str + + +Decision = Remove | Keep + + +def _objection(unsaved: workspace_state.Unsaved) -> Optional[str]: + """What deleting a clone in this state would destroy or risk, or ``None``. + + The one place `--prune` turns devlaunch#171's three answers into the two + :func:`decide` acts on: something to say, or nothing. ``None`` is safe here + in a way it was not on the raw probe, because "could not tell" arrives as + words rather than as an absence -- and it arrives as words that *object*, + so the clone is kept for the same reason unpushed work keeps one. + + Total over the arms, with :func:`workspace_state.unhandled_unsaved` behind + it: a fourth answer stops the build rather than falling through into a + deletion. The clause reads after "holds", so ``--prune``'s report builds its + own sentence around it -- the same clause ``dl rm``'s refusal renders + from the arms directly. + """ + if isinstance(unsaved, workspace_state.NothingToLose): + return None + if isinstance(unsaved, workspace_state.WouldLose): + return unsaved.description + if isinstance(unsaved, workspace_state.CouldNotTell): + return f"work git could not be asked about ({unsaved.reason})" + workspace_state.unhandled_unsaved(unsaved) + + +def decide(status: CloneStatus, force: bool) -> Decision: + """What `--prune` does about one clone directory. The only such place. + + Total over the arms, and deliberately the single point at which anything + becomes deletable: there is no boolean beside a status that a later caller + could read without having answered which arm it is, and no path from a + directory devlaunch could not classify to one it would remove. A fourth arm + added to CloneStatus stops this build rather than defaulting to a deletion. + + `--force` promotes exactly one arm. It is not a general override: + Referenced and Disputed are not "refusals to be insisted past", they are + devlaunch saying the directory is still in use or that its own records + disagree, and there is nothing for a user to mean by insisting. + """ + if isinstance(status, Referenced): + return Keep(f"workspace {status.workspace_id} still opens it") + if isinstance(status, Orphaned): + objection = _objection(status.unsaved) + if objection is None: + return Remove(status.usage, Unopposed()) + if force: + return Remove(status.usage, Insisted(f"holds {objection}")) + return Keep(f"holds {objection} -- add --force to remove it anyway") + if isinstance(status, Disputed): + return Keep( + f"devpod lists workspace {status.workspace_id} and sources it at " + f"{status.sourced_at}; see devlaunch#88" + ) + _unhandled_status(status) + + +@dataclass(frozen=True) +class Reclaimable: + """One clone directory this run will remove, what it frees, and why it may. + + *promotion* is how the second pass knows whether `--force` was answering + *this* directory. Without it the flag is plan-wide and turns off the + re-probe for every directory in the plan. + """ + + path: pathlib.Path + owner: str + repo: str + usage: disk_usage.DiskUsage + promotion: Promotion + + +@dataclass(frozen=True) +class Kept: + """One clone directory this run will leave standing, and why.""" + + path: pathlib.Path + because: str + + +@dataclass(frozen=True) +class PrunePlan: + """Everything one `dl --prune` will do, settled before anything is asked. + + A value, for the reason WorkspaceOwnership is one: the report a user answers + and the set that actually dies must come from the same object, and here the + difference between them is somebody's directory. The two tuples are built by + one pass over one `decide` call each, so a directory cannot be in both and + cannot be in neither. + + There is deliberately no `force` here. It was a field, and a plan-wide + boolean is exactly the shape `decide` refuses to have beside a status: the + pass that acts read it and skipped its safety re-check for every directory, + including the ones `--force` had promoted nothing about. What `--force` + answered rides on each `Reclaimable` instead. + """ + + root: pathlib.Path + removing: Tuple[Reclaimable, ...] + keeping: Tuple[Kept, ...] + stale_records: Tuple[WorktreeInfo, ...] + + @property + def nothing_to_do(self) -> bool: + """Whether this run would change nothing at all.""" + return not self.removing and not self.stale_records + + +def _canonical(path: str) -> Optional[pathlib.Path]: + """*path* with every symlink resolved, or None if it could not be followed. + + None means "cannot tell", never "somewhere else": every caller here is + deciding whether a directory is referenced, and answering that from a lookup + that failed is how a live clone becomes an orphan. + + A path that is not *there* is not a failure -- resolving canonicalises as + much of it as exists and leaves the rest, which is the right answer for a + workspace whose source has been deleted, and there are hosts where that is + most of them (devlaunch#88). What lands here is a path that could not be + read as one at all: text devpod's JSON carried that no filesystem call will + accept (ValueError), and, on Python 3.10 only, a symlink loop, which later + versions resolve as far as they can instead of raising. All three are + caught, because the package supports both versions and the difference + between them must not decide what gets deleted. + """ + try: + return pathlib.Path(path).resolve() + except (OSError, RuntimeError, ValueError): + return None + + +@dataclass(frozen=True) +class Placeable: + """Every place on this machine a source could name -- possibly none. + + Empty is a real answer and not a shrug: an image or container workspace + opens no directory on this disk, so there is nothing to compare and no clone + it could be holding. + """ + + paths: Tuple[str, ...] + + +@dataclass(frozen=True) +class Unplaceable: + """The source opens a folder here and devlaunch cannot say which one.""" + + detail: str + + +SourcePlaces = Placeable | Unplaceable + + +def source_places(source: WorkspaceSource) -> SourcePlaces: + """Where on this machine *source* could be. Total over the arms. + + A `gitRepository` counts, even though devlaunch only ever hands devpod a + local path, and the reason is which way the mistake runs. `devpod up + ` records that arm with a path in it, and a path this + function does not return is a directory `--prune` will call unreferenced. + is_devlaunch_clone refuses the same arm on purpose -- but refusing there + means declining to delete somebody else's *workspace*, which is the opposite + direction, so its answer must not be reused here. That is what the review of + the disk-size surface meant by "not as-is": the predicate is right for + reporting and for purging, and this question is neither. + + The two answers that carry no path are kept apart, because reading them + alike is how a live workspace contributed no path *and* no alarm while the + command printed that it stops for exactly that. An image or container + workspace is `Placeable(())` -- nothing here, nothing at risk. A `localFolder` + devpod filled with something unreadable is `Unplaceable`, and it stops the + command the same way a source that will not resolve does. + """ + if isinstance(source, LocalFolder): + return Placeable((source.path,)) + if isinstance(source, GitRepository): + return Placeable((source.url,)) + if isinstance(source, UnrecognisedSource): + return Placeable(()) + if isinstance(source, UnreadableLocalFolder): + return Unplaceable(json.dumps(source.payload)) + _unhandled_source(source) + + +@dataclass(frozen=True) +class Misplaced: + """A live workspace devpod records inside a repository's clone tree, at + something that is not a clone. + + devlaunch#88's measured shape, and the reason it needs a name of its own. + On that ticket's host 36 of 39 devpod records named a folder that was gone + (35) or a config-only stub devpod itself wrote from cache (1), while the + real checkout sat beside it under the new id scheme. The two records cannot + be joined by workspace id -- the id is exactly what changed -- so the join + is made from the path instead: devpod points into `///` + at a directory that holds no `.git`, and which of that repository's clones + the workspace actually needs is unanswerable. + """ + + workspace_id: str + sourced_at: str + + +@dataclass(frozen=True) +class WorkspaceLocations: + """Where devpod's workspaces are on this disk, and which ones are unknown. + + *unlocatable* is not an empty result with a note on it. A live workspace + whose source cannot be followed is a directory that might be *any* of the + candidates, so while one exists there is no honest answer to "is this clone + referenced" -- and `--prune` says so instead of guessing. + + *misplaced* is the same refusal made narrow. A workspace devpod records at a + non-clone *inside one repository's clone tree* can only be confused with + that repository's clones, so it disputes those and leaves every other + repository prunable -- which is what keeps this command usable on the host + devlaunch#88 describes rather than merely safe on it. + """ + + by_path: Mapping[pathlib.Path, str] + unlocatable: Tuple[str, ...] + misplaced: Mapping[Tuple[str, str], Misplaced] + + def holder(self, candidate: pathlib.Path) -> Optional[str]: + """The live workspace *candidate* holds the checkout for, if any. + + At **or under**, not equal to, and the direction matters in the only way + this command's mistakes matter. `devpod up /subproject` records + the subdirectory, and a clone whose subdirectory a live workspace opens + is a clone that live workspace needs -- deleting it takes the workspace + with it. Equality answered no and deleted the parent. + + The containment is between two canonical paths, which is what keeps it + from being the lexical prefix test the reporting surface uses: + `-scratch` is not under ``, and a symlinked source has + already been resolved before it gets here. + """ + workspace_id = self.by_path.get(candidate) + if workspace_id is not None: + return workspace_id + for source, held_by in self.by_path.items(): + if candidate in source.parents: + return held_by + return None + + +@dataclass(frozen=True) +class Outside: + """The source is not in devlaunch's clone tree, so no clone answers for it.""" + + +@dataclass(frozen=True) +class InAClone: + """The source is at or under *clone*, a directory that holds a checkout.""" + + clone: pathlib.Path + + +@dataclass(frozen=True) +class InARepositoryOnly: + """The source is in *(owner, repo)*'s clone tree but at no clone of it.""" + + owner: str + repo: str + + +@dataclass(frozen=True) +class TooShallow: + """The source is in the clone tree above any repository, so it names none.""" + + +SourceSite = Outside | InAClone | InARepositoryOnly | TooShallow + + +def _site_of(source: pathlib.Path, root: pathlib.Path) -> SourceSite: + """Where a resolved source sits with respect to devlaunch's clone tree. + + Read off the path rather than derived from an id, because on devlaunch#88's + host the id is what went wrong and the path is what survived: devpod's stale + record still says `/blooop/devlaunch/`, which names the + repository exactly even though the leaf and the workspace id match nothing + any more. + + The clone is the *third* component under the root and the source may be + deeper -- `devpod up /subproject` is a live workspace whose source is + inside a clone, and the clone is what answers for it. + """ + try: + parts = source.relative_to(root).parts + except ValueError: + return Outside() + if len(parts) < 3: + return TooShallow() if len(parts) < 2 else InARepositoryOnly(parts[0], parts[1]) + clone = root / parts[0] / parts[1] / parts[2] + if _is_populated_clone(clone): + return InAClone(clone) + return InARepositoryOnly(parts[0], parts[1]) + + +def workspace_locations(workspaces: Sequence[Workspace], root: pathlib.Path) -> WorkspaceLocations: + """Resolve every live workspace's source to a real directory on this disk. + + Both sides of the comparison this feeds are canonical, and that is the whole + point rather than tidiness. A cache reached through a symlink -- somebody + moved theirs, or `/tmp` is a link on their machine -- makes a lexical + comparison say that *no* clone is referenced, which is a total-loss bug in + the one direction that cannot be undone. The candidates are canonical by + construction (see prune_plan); this canonicalises the other side. + + Three ways a workspace fails to place itself, and they are not one thing: + a source devlaunch cannot read at all, and a source that named a folder no + filesystem call will accept, both mean the workspace could be opening *any* + candidate and stop the command; a source that lands inside a repository's + clone tree on something with no `.git` in it means the workspace could be + opening any of *that repository's* clones, and disputes only those. + """ + by_path: Dict[pathlib.Path, str] = {} + unlocatable: List[str] = [] + misplaced: Dict[Tuple[str, str], Misplaced] = {} + for workspace in workspaces: + places = source_places(workspace.source) + if isinstance(places, Unplaceable): + unlocatable.append(f"{workspace.id}: {places.detail}") + continue + for source in places.paths: + resolved = _canonical(source) + if resolved is None: + unlocatable.append(f"{workspace.id}: {source}") + continue + site = _site_of(resolved, root) + if isinstance(site, (Outside, InAClone)): + by_path[resolved] = workspace.id + elif isinstance(site, InARepositoryOnly): + misplaced[(site.owner, site.repo)] = Misplaced(workspace.id, str(resolved)) + elif isinstance(site, TooShallow): + unlocatable.append(f"{workspace.id}: {source}") + else: + _unhandled_site(site) + return WorkspaceLocations(by_path=by_path, unlocatable=tuple(unlocatable), misplaced=misplaced) + + +def _unhandled_site(site: NoReturn) -> NoReturn: + """Reject a source site nobody handled -- at type-check time, not at runtime.""" + raise AssertionError(f"Unhandled source site: {site!r}") + + +def _is_populated_clone(path: pathlib.Path) -> bool: + """Whether *path* is a checkout rather than a place one used to be. + + The same question `workspace_exists` asks, and devlaunch#88's own published + diagnostic (`[ -d "$p/.git" ] || echo BROKEN`). It is what separates a + devpod record that still describes something from one the id-scheme change + left behind -- a folder that is gone, or the config-only stub devpod + reconstitutes from its cache, neither of which any clone can be matched to. + """ + try: + return (path / ".git").exists() + except OSError: + return False + + +def _subdirectories(path: pathlib.Path) -> List[pathlib.Path]: + """The real directories directly under *path*, sorted, symlinks not followed. + + A symlinked entry is skipped rather than followed. Following one would put a + candidate outside the cache entirely, and unlinking the link instead would + report a clone as reclaimed while it sat on another volume -- the same two + wrong answers remove_tree already refuses for a symlinked root. Skipping is + that refusal, one step earlier, and it is also what keeps every candidate's + path canonical without a resolve() that could fail. + + A directory that cannot be listed yields nothing: there is no such thing as + a clone this process can delete but not see, so the safe reading of a closed + door is that there is nothing behind it to remove. + """ + try: + with os.scandir(path) as entries: + found = [entry for entry in entries if entry.is_dir(follow_symlinks=False)] + except OSError: + return [] + return sorted(pathlib.Path(entry.path) for entry in found) + + +def _clone_status( + clone: pathlib.Path, + owner: str, + repo: str, + locations: WorkspaceLocations, + record_for: Mapping[pathlib.Path, WorktreeInfo], + listed_at: Mapping[str, str], +) -> CloneStatus: + """Which arm *clone* is, asked in the order that fails towards keeping it. + + devpod's own listing is consulted first, and by containment rather than + containment-in-the-cache: the question is whether any live workspace's + source is at or under *this* directory, which the lexical predicate the + reporting surface uses cannot answer at all. + + Then the two ways devpod's records and devlaunch's can disagree, and both + are devlaunch#88's shape: + + - devpod has a live workspace somewhere in *this repository's* clone tree + that is not a clone -- a folder that is gone, or the config-only stub + devpod rebuilds from cache. 36 of 39 workspaces on #88's host. Which of + this repository's clones it needs cannot be answered, so none of them is + unreferenced. + - this directory's own record names a workspace devpod still lists and + sources elsewhere. The narrower shape, and the one that survives when the + ids still line up. + + A record naming a workspace devpod has forgotten is not a disagreement, it + is the ordinary stale clone this command exists for -- 34 of the reference + host's 37. + + The unsaved probe and the disk walk run last and only on the arm that could + be removed. Together they are the expensive half of a scan (593 ms of git + over 37 clones on the reference host, plus a walk with no ceiling), and + asking them about a directory no answer could affect is time spent to learn + nothing. + """ + workspace_id = locations.holder(clone) + if workspace_id is not None: + return Referenced(workspace_id) + misplaced = locations.misplaced.get((owner, repo)) + if misplaced is not None: + return Disputed(misplaced.workspace_id, misplaced.sourced_at) + record = record_for.get(clone) + if record is not None: + elsewhere = listed_at.get(record.workspace_id) + if elsewhere is not None: + return Disputed(record.workspace_id, elsewhere) + return Orphaned( + workspace_state.holds_unsaved_work(clone), + disk_usage.exclusive_usage(clone), + ) + + +def _records_by_directory( + clone_mgr: WorkspaceCloneManager, +) -> Dict[pathlib.Path, WorktreeInfo]: + """metadata.json's worktree records, keyed by the directory each names. + + Which directory a record names is :meth:`resolve_clone_path`'s question and + not this function's, and asking it here instead was the shape devlaunch#174 + was: `local_path` read raw is one of *two* answers a record can give, and the + other one is what the delete acts on. The consequence here is the dangerous + direction rather than the merely inconsistent one -- a record that missed + its clone leaves that clone with no record at all, which drops it out of + :class:`Disputed` and into :class:`Orphaned`, which is a deletion. + + An empty recorded path is the case that makes this concrete: `Path("")` is + `Path(".")`, which is truthy and which exists, so it canonicalised to + whatever directory dl happened to be run from and the record was filed under + that. :meth:`resolve_clone_path` refuses anything not absolute and derives + the real directory from the record's own owner/repo/branch instead. + + A record dl cannot name a directory for at all is left out. It cannot be + matched to a candidate by definition, and there is no path it could be filed + under that would not be a guess. + """ + records: Dict[pathlib.Path, WorktreeInfo] = {} + for record in clone_mgr.storage.list_worktrees(): + directory = clone_mgr.resolve_clone_path(record) + if directory is None: + continue + resolved = _canonical(str(directory)) + if resolved is not None: + records[resolved] = record + return records + + +def _repo_lock(clone_mgr: WorkspaceCloneManager, owner: str, repo: str): + """The lock a launch of *owner/repo* holds while it fills a clone. + + `--prune` takes the same one, and for the same span it looks at and removes + that repository's clones: workspace_clone populates a clone fully before it + returns, so without this a scan can weigh -- or delete -- a directory that + `git clone` is still writing into. + + It closes that window and not a wider one, and the difference is worth being + plain about: devpod only learns about a 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. Closing that would + take a record devlaunch does not keep (devlaunch#88's ticket is where the + id it would need is meant to start being written down). + """ + return hold_lock( + clone_mgr.repo_manager.lock_path(owner, repo), + waiting_note=f"another dl run preparing {owner}/{repo}", + ) + + +def clone_root(clone_mgr: WorkspaceCloneManager) -> pathlib.Path: + """The directory `--prune` scans, canonicalised once. + + `repos_dir` as the clone manager reports it. Taking it from there rather + than rebuilding `/repos` is what keeps the directories scanned, the + locks taken and the workspace sources compared answering to the same + configuration: a `config.toml` that moves `repos_dir` moves all three, and + they cannot drift into scanning one tree while serialising against another + or comparing against a third. + + Resolved directly rather than through _canonical: a repos_dir this could + fail on is one RepositoryManager's own mkdir has already refused, so a clone + manager existing at all is evidence the path is usable. Absent is not a + failure -- a fresh install has no repos directory yet and resolving one that + is not there is what says so. + """ + return pathlib.Path(clone_mgr.repo_manager.repos_dir).resolve() + + +def prune_plan( + clone_mgr: WorkspaceCloneManager, + workspaces: Sequence[Workspace], + locations: WorkspaceLocations, + root: pathlib.Path, + force: bool, +) -> PrunePlan: + """Classify every clone directory under the cache, one repository at a time. + + Every candidate path is canonical without ever being resolved individually + -- a resolved root (see :func:`clone_root`) plus real directory names, + symlinks skipped. + """ + removing: List[Reclaimable] = [] + keeping: List[Kept] = [] + record_for = _records_by_directory(clone_mgr) + listed_at = {ws.id: describe_source(ws.source)[1] for ws in workspaces} + for owner_dir in _subdirectories(root): + for repo_dir in _subdirectories(owner_dir): + owner, repo = owner_dir.name, repo_dir.name + bare = _canonical(str(clone_mgr.repo_manager.get_bare_path(owner, repo))) + with _repo_lock(clone_mgr, owner, repo): + for clone in _subdirectories(repo_dir): + if clone == bare: + # Never a candidate and never reported. Nothing sources + # it and no record names it, so every rule above would + # call it an orphan -- and it is the copy every clone of + # this repo hardlinks its git objects out of, 0.08 GB + # for all seven repos on the reference host, and the + # reason the next clone is fast. + continue + status = _clone_status(clone, owner, repo, locations, record_for, listed_at) + decision = decide(status, force) + if isinstance(decision, Remove): + removing.append( + Reclaimable(clone, owner, repo, decision.usage, decision.promotion) + ) + else: + keeping.append(Kept(clone, decision.because)) + return PrunePlan( + root=root, + # Biggest first: the report's job is to be acted on, and "which of these + # is worth reclaiming" is the comparative question known_bytes exists + # for. Path breaks ties so two runs over an unchanged cache read alike. + removing=tuple(sorted(removing, key=lambda r: (-disk_usage.known_bytes(r.usage), r.path))), + keeping=tuple(keeping), + stale_records=_records_for_absent_directories(clone_mgr), + ) + + +def _records_for_absent_directories( + clone_mgr: WorkspaceCloneManager, +) -> Tuple[WorktreeInfo, ...]: + """The worktree records whose directory is definitively not there any more. + + metadata.json is append-mostly and nothing has ever pruned it: 49 records + for 17 live workspaces on the reference host. These are the ones that + describe nothing at all. + + "Definitively" is _present's distinction and it is load-bearing here too: a + directory this process is not allowed to look at is still a directory, and + dropping its record would lose the only note of where a clone lives. + + Which directory the record describes is asked of + :meth:`resolve_clone_path`, the same resolver the classification, the + listing and the delete all go through (devlaunch#174). Reading `local_path` + raw asks about a directory that may not be the one the clone is in: a record + written before the current id scheme, or one whose recorded path is empty -- + `Path("")` is `Path(".")`, present by construction -- gets an answer about + somewhere else, and a record dropped for a clone that is still there is the + only note of where that clone lives, gone. + + A record dl cannot name a directory for is not dropped. "dl could not work + out where this is" is not "this is not there", and only the second is a + reason to forget it. + """ + stale: List[WorktreeInfo] = [] + for record in clone_mgr.storage.list_worktrees(): + directory = clone_mgr.resolve_clone_path(record) + if directory is not None and not _present(directory): + stale.append(record) + return tuple(stale) + + +def print_prune_plan(plan: PrunePlan) -> None: + """Say what is going, what is staying and why, before anything is asked. + + Printed whether or not there is anything to do, and in that order, because + the reason a directory is *staying* is the half a person cannot get anywhere + else -- `dl --ls` lists workspaces, and a clone with no workspace has no row + there to appear in. + """ + print(f"Clone directories under {plan.root}:") + print() + if plan.removing: + freed = disk_usage.describe_usage(disk_usage.total_usage(r.usage for r in plan.removing)) + print(f"Removing {len(plan.removing)} that nothing references -- {freed}:") + for reclaimable in plan.removing: + line = f" - {reclaimable.path} ({disk_usage.describe_usage(reclaimable.usage)})" + promotion = reclaimable.promotion + if isinstance(promotion, Insisted): + # What --force is answering, on the line of the directory it + # answers for. Without it the plan reads the same for a clone + # holding an afternoon's uncommitted work as for an empty one, + # and the confirmation cannot say what it costs. + line = f"{line} -- {promotion.despite}, removing anyway" + elif not isinstance(promotion, Unopposed): + _unhandled_promotion(promotion) + print(line) + print() + if plan.keeping: + print(f"Leaving {len(plan.keeping)}:") + for kept in plan.keeping: + print(f" - {kept.path}: {kept.because}") + print() + if plan.stale_records: + print(f"Dropping {len(plan.stale_records)} record(s) of directories already gone.") + print() + if plan.nothing_to_do: + print("Nothing to prune.") + + +def report_unlocatable(locations: WorkspaceLocations) -> None: + """Say which live workspaces could not be placed, and that nothing went. + + Not a warning above a report. A workspace whose source cannot be followed -- + text no filesystem call will accept, or a `localFolder` devpod filled with + something that is not a path -- could be opening any of the candidates, so + while one exists there is no directory this command can honestly call + unreferenced. Printed by both passes, because both have to be able to stop. + """ + print("dl --prune cannot follow these live workspaces' sources:") + for source in locations.unlocatable: + print(f" - {source}") + print() + print("Nothing was removed: no clone is unreferenced while a workspace is unaccounted for.") + + +def prune_clones(clone_mgr: WorkspaceCloneManager, plan: PrunePlan, root: pathlib.Path) -> int: + """Carry out *plan*: remove the directories, then forget them. + + **Every directory is classified again, under the lock, immediately before it + goes**, and only what this pass *also* finds removable is removed. The report + a user answered was taken before they answered it, and everything it rests on + can have moved in between: a container writes into a clone, or a launch that + was mid-clone when the plan was printed finishes and registers a workspace + for one of these exact directories -- the clone path for `(owner, repo, + branch)` is deterministic, so a concurrent launch reuses the very directory + in the plan. Re-asking only "has it grown unsaved work" caught the first and + not the second, and the difference was somebody's running workspace. + + That is why this pass pays a second `devpod list`. It is the one question + whose answer cannot be re-derived from disk, it is O(1) rather than per + workspace, and it is paid only after a user has said yes to a deletion. + + `--force` is re-applied per directory, from the promotion the plan recorded + for that directory rather than from a flag over the whole run, so insisting + past one clone's unsaved work does not turn the re-probe off for the others. + Referenced and Disputed are not promotable on either pass, so a directory + that became either since the plan is kept whatever was typed. + + The approved set can therefore shrink between the report and the act, and can + never grow -- the direction that costs a command rather than a morning's work. + + Classifying again means walking again, since the disk figure lives inside + the arm that could be removed. That is one extra walk of each directory this + run is about to delete, and none of the others -- and the bytes reported + stay the plan's, so what a person is told they got back is what they said + yes to. + """ + removed: List[Reclaimable] = [] + refused: List[Refusal] = [] + withheld: List[Tuple[pathlib.Path, str]] = [] + workspaces = list_workspaces(refresh=True) + locations = workspace_locations(workspaces, root) + if locations.unlocatable: + report_unlocatable(locations) + return 1 + listed_at = {ws.id: describe_source(ws.source)[1] for ws in workspaces} + record_for = _records_by_directory(clone_mgr) + by_repo: Dict[Tuple[str, str], List[Reclaimable]] = {} + for reclaimable in plan.removing: + by_repo.setdefault((reclaimable.owner, reclaimable.repo), []).append(reclaimable) + for (owner, repo), reclaimables in sorted(by_repo.items()): + with _repo_lock(clone_mgr, owner, repo): + for reclaimable in reclaimables: + decision = decide( + _clone_status(reclaimable.path, owner, repo, locations, record_for, listed_at), + isinstance(reclaimable.promotion, Insisted), + ) + if isinstance(decision, Keep): + withheld.append((reclaimable.path, decision.because)) + continue + refusals = remove_tree(reclaimable.path) + if refusals: + refused.extend(refusals) + continue + removed.append(reclaimable) + _forget_clone(clone_mgr, record_for.get(reclaimable.path)) + for record in plan.stale_records: + _forget_clone(clone_mgr, record) + freed = disk_usage.describe_usage(disk_usage.total_usage(r.usage for r in removed)) + print(f"Removed {len(removed)} clone director(ies) -- {freed}.") + for path, because in withheld: + print(f"Left {path}: {because}. That was not so when the plan above was printed.") + if not refused: + return 0 + # Not 0: a directory the user was told would go is still on disk. The + # clones that did go are still gone, which is why this is a report and not + # an abort. + report_refusals( + refused, + "Some directories would not come away. These refused:", + tuple(refusal.path for refusal in refused), + ) + return 1 + + +def _forget_clone(clone_mgr: WorkspaceCloneManager, record: Optional[WorktreeInfo]) -> None: + """Drop one worktree record, if there was one to drop. + + Removing a clone without this is what left metadata.json describing + workspaces that stopped existing years ago; a record kept for a directory + that is gone is not a safety margin, it is the thing that made the file + unreadable as a description of anything. + """ + if record is None: + return + try: + clone_mgr.storage.remove_worktree(record.owner, record.repo, record.branch) + except OSError as e: + logging.warning(f"Could not drop the record for {record.local_path}: {e}") + + +_PRUNE_FLAGS = ("-y", "--yes", "--force") + + +def prune_command(flags: Sequence[str]) -> int: + """`dl --prune`: report the clone directories nothing references, then act. + + Shaped like `--purge` on purpose -- print the plan, name what is left + standing and why, confirm, `-y` to skip -- because a user who has run one + has already learned this one. What it does is the opposite of `--purge`'s + all-or-nothing: it removes only the clone directories no live workspace + opens, leaves every bare cache alone, and never touches a devpod workspace, + a container, an image or a volume. + + Nothing here runs on its own or on the way to anything else. A full scan + measured 1017 ms on the reference host -- about two warm launches -- and it + gets slower exactly as the problem it is for gets worse, so it is a command + somebody runs, and `dl --prune` answered `n` is how you read the report. + """ + unknown = [flag for flag in flags if flag not in _PRUNE_FLAGS] + if unknown: + # Refused rather than ignored. An ignored option fails safe here by + # luck rather than by design -- but `dl --prune --dry-run -y` reads as a + # rehearsal and would have been a deletion, and that is not a mistake to + # find out about afterwards. + logging.error(f"Unknown option(s) for --prune: {' '.join(unknown)}") + return 1 + force = "--force" in flags + clone_mgr = _get_clone_manager() + root = clone_root(clone_mgr) + workspaces = list_workspaces() + locations = workspace_locations(workspaces, root) + if locations.unlocatable: + report_unlocatable(locations) + return 1 + plan = prune_plan(clone_mgr, workspaces, locations, root, force) + print_prune_plan(plan) + if plan.nothing_to_do: + return 0 + if not any(flag in ("-y", "--yes") for flag in flags): + if input("Are you sure? [y/N] ").strip().lower() not in ("y", "yes"): + print("Aborted.") + return 0 + return prune_clones(clone_mgr, plan, root) + + # Regex patterns for parsing git URLs GIT_URL_PATTERNS = [ # git@github.com:owner/repo.git @@ -1441,7 +2390,7 @@ def discover_repos_from_workspaces(workspaces: List[Workspace]) -> Dict[str, Lis if remote_url: owner_repo = parse_owner_repo_from_url(remote_url) - elif isinstance(source, UnrecognisedSource): + elif isinstance(source, (UnrecognisedSource, UnreadableLocalFolder)): logging.warning( f"Not looking for a repo in workspace '{ws.id}': " f"devpod describes its source as {json.dumps(source.payload)}, " @@ -2251,6 +3200,13 @@ def print_help(): --json, where the field is `disk`. dl --install Install shell completions dl --refresh Refresh completion cache + dl --prune [-y] [--force] Remove the clone directories no workspace + opens any more, and forget the records of + directories already gone. Bare caches, live + clones and every devpod workspace are left + alone; a clone holding uncommitted or + unpushed work is named and kept unless + --force. Prints the plan and asks first. dl --purge [-y] Remove devlaunch's workspaces and caches dl --help, -h Show this help dl --version Show version (editable installs name their tree) @@ -2463,6 +3419,9 @@ def _run_cli(argv: Optional[List[str]] = None) -> int: update_completion_cache() return install_completions(rc_path) + if args[0] == "--prune": + return prune_command(args[1:]) + if args[0] == "--purge": # Check for -y flag to skip confirmation skip_confirm = len(args) > 1 and args[1] in ("-y", "--yes") diff --git a/pyproject.toml b/pyproject.toml index 476981b6..763473c7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -162,6 +162,7 @@ exclude_also = [ # is reachable, so in a build that type-checks these lines cannot be executed # -- the same reason `raise AssertionError` is already on this list. "_unhandled_source\\(", + "_unhandled_status\\(", "_unhandled_usage\\(", "if __name__ == .__main__.:", "pass", diff --git a/test/test_devpod_spawn_counts.py b/test/test_devpod_spawn_counts.py index 2bb62715..f155784c 100644 --- a/test/test_devpod_spawn_counts.py +++ b/test/test_devpod_spawn_counts.py @@ -208,6 +208,50 @@ def test_purge_reads_the_workspace_list_exactly_once(self, spawns, tmp_path): ["delete", "ws2", "--force"], ] + def test_prune_lists_workspaces_once_per_pass_and_asks_no_status(self, spawns): + """`dl --prune` costs one `devpod list` per pass and nothing else. + + A workspace's *state* has no bearing on whether a clone directory is + referenced -- a stopped workspace still opens the one it was made from + -- so a `devpod status` per workspace here would be half a second each + to learn something the answer does not depend on. That is the cost this + pins, and the reason this command is never on a launch path and never on + `--ls`: the whole scan was measured at 1017 ms on the reference host, + most of it the listing and the git probes, and it gets slower exactly as + the cache it is for gets fuller. + + Two listings, not one, and the second is not an oversight. Everything a + plan rests on can move while the user is reading it, and whether a clone + became *referenced* is the one thing that cannot be re-derived from + disk: a launch that completes in that window registers a workspace for + the very directory in the plan, because the clone path for + `(owner, repo, branch)` is deterministic. One extra O(1) listing, paid + only after somebody has said yes to a deletion, buys the re-check. + + The clone goes under the cache dl actually resolves, and the assertion + that it was removed comes first: a fixture whose directories sit + somewhere the command never looks would leave these counts describing a + scan that classified nothing at all. + """ + from devlaunch.xdg import devlaunch_cache + + stale = devlaunch_cache() / "repos" / "o" / "r" / "r-main-abcdefgh" + stale.mkdir(parents=True) + (stale / "payload.bin").write_bytes(b"\0" * 1024) + assert _run_dl("--prune", "-y") == 0 + assert not stale.exists() + assert spawns.devpod_commands == [ + ["list", "--output", "json"], + ["list", "--output", "json"], + ] + + def test_prune_lists_nothing_twice_when_there_is_nothing_to_do(self, spawns): + """The second listing belongs to the pass that acts, so a run that does + not act does not pay for it -- `dl --prune` answered `n`, or a cache + with nothing in it, is one listing.""" + assert _run_dl("--prune", "-y") == 0 + assert spawns.devpod_commands == [["list", "--output", "json"]] + def test_attaching_to_a_running_workspace(self, spawns): """The interactive attach chain, pinned call by call. diff --git a/test/test_workspace_state.py b/test/test_workspace_state.py index b75fda91..817177eb 100644 --- a/test/test_workspace_state.py +++ b/test/test_workspace_state.py @@ -762,6 +762,7 @@ def _run( unreadable=False, only_foreign=False, recorded=True, + hand_removed=False, ): cache = tmp_path / "cache" / "devlaunch" clone = cache / "repos" / "blooop" / "r" / "r-feature-aaa" @@ -770,6 +771,8 @@ def _run( if unreadable: (clone / "locked").mkdir(exist_ok=True) (clone / "locked").chmod(0o000) + if hand_removed: + shutil.rmtree(clone) entries = [_entry("someone-elses", tmp_path / "projects" / "other")] if not only_foreign: entries.insert(0, _entry("r-feature-aaa", clone)) @@ -823,6 +826,31 @@ def test_a_workspace_devlaunch_did_not_make_is_not_walked(self, tmp_path, capsys assert foreign["devlaunch"] is False assert foreign["disk"] is None + def test_a_clone_removed_by_hand_measures_nothing_rather_than_going_unmeasured( + self, tmp_path, capsys + ): + """Zero and "not measured" are different answers, and the difference is + what a cleanup tool acts on. + + `-` and `null` mean "there is no clone of `dl`'s here to measure" -- a + workspace opened from somebody's own project directory. A clone `dl` + made and somebody then deleted by hand is `dl`'s, and what removing it + would free is nothing, which is a measurement. A reader that saw `null` + for it would leave the workspace alone as somebody else's; a reader that + sees zero knows it is `dl`'s and costs nothing to clean up. + + These are precisely the directories `dl --prune` is about, and until now + the case had no test on either rendering. + """ + report = self._json(tmp_path, ["--ls", "--json", "--size"], capsys, hand_removed=True) + ours = next(row for row in report if row["id"] == "r-feature-aaa") + assert ours["devlaunch"] is True + assert ours["disk"] == {"exclusiveBytes": 0} + + _code, out = self._run(tmp_path, ["--ls", "--size"], capsys, hand_removed=True) + row = next(line for line in out.splitlines() if line.startswith("r-feature-aaa")) + assert "0 B" in row + def test_what_could_not_be_read_is_reported_as_a_floor_not_a_total(self, tmp_path, capsys): if os.geteuid() == 0: pytest.skip("root is refused by nothing, so the closed door would open") diff --git a/test/unit/test_disk_usage.py b/test/unit/test_disk_usage.py index 10b8210d..da32369f 100644 --- a/test/unit/test_disk_usage.py +++ b/test/unit/test_disk_usage.py @@ -26,6 +26,7 @@ describe_usage, exclusive_usage, known_bytes, + total_usage, usage_as_json, ) @@ -233,6 +234,38 @@ def test_it_is_not_how_a_usage_is_reported(self): assert describe_usage(floor) != describe_usage(Measured(1536)) +class TestAddingUsagesUp: + """A total is an answer of the same two kinds, and says which it is. + + The caller is `dl --prune`, which prints "removing N directories -- X". + Adding known_bytes up gives an integer that has forgotten whether any part + of it was a floor, and an integer rendered as a size is a floor read as a + total -- a cleanup tool telling somebody 4 GB when the true figure is more. + """ + + def test_complete_walks_add_up_to_a_complete_total(self): + assert total_usage([Measured(1024), Measured(512)]) == Measured(1536) + + def test_nothing_at_all_adds_up_to_nothing_measured(self): + assert total_usage([]) == Measured(0) + + def test_one_floor_among_them_makes_the_total_a_floor(self): + total = total_usage([Measured(1024), PartlyUnreadable(512, (Path("/x"),))]) + assert total == PartlyUnreadable(1536, (Path("/x"),)) + assert describe_usage(total) == "≥1.5 KiB" + + def test_every_closed_door_is_carried_into_the_total(self): + """The paths are what makes the caveat actionable rather than a warning: + a person told which directories were not counted can go and look.""" + total = total_usage( + [ + PartlyUnreadable(512, (Path("/x"),)), + PartlyUnreadable(512, (Path("/y"),)), + ] + ) + assert total == PartlyUnreadable(1024, (Path("/x"), Path("/y"))) + + class TestHowAUsageReads: @pytest.mark.parametrize( "measured,expected", diff --git a/test/unit/test_prune_orphaned_clones.py b/test/unit/test_prune_orphaned_clones.py new file mode 100644 index 00000000..ae300ff0 --- /dev/null +++ b/test/unit/test_prune_orphaned_clones.py @@ -0,0 +1,1213 @@ +# pylint: disable=redefined-outer-name +"""`dl --prune`: remove the clone directories no workspace references any more. + +A workspace per branch means clone directories accumulate under the cache and +nothing ever removes them -- measured on the reference host, 52 clone directories +for 17 live devpod workspaces, 4.00 GB of them attached to nothing. `--purge` is +the wrong tool for that: it is all-or-nothing and takes the live clones and the +bare caches with it. + +**These tests guard a deletion path, and the two ways of being wrong are not +equal.** A clone wrongly called referenced leaves garbage on disk; a clone +wrongly called orphaned destroys work that exists nowhere else. So every case +below that keeps a directory is the load-bearing one, and the fixture is built +so that a guard which stopped biting would show up as a *deletion* rather than +as a survivor. + +The clones are real git repositories with a local bare standing in for GitHub, +which is PR #134's technique: a local path is a real git remote, so pushes, +fetches and remote-tracking refs behave exactly as they do over ssh with no +network, and the one bug that shipped in this area (an argument order that +reported every clone as safe to delete) was invisible to everything except a +real `git log`. Everything lives under the suite's scratch `XDG_CACHE_HOME`, so +`_get_cache_dir`, the metadata file and `repos_dir` all agree without being +patched into agreement -- a fixture whose clones sit outside the directory under +test is how a guard comes to run zero times. +""" + +import json +import os +import shlex +import shutil +import subprocess +import sys +import threading +import time +from pathlib import Path +from typing import Callable, Dict, List, Optional +from unittest.mock import patch + +import pytest + +from devlaunch.dl import main +from devlaunch.worktree.models import WorktreeInfo +from devlaunch.worktree.storage import MetadataStorage +from devlaunch.xdg import devlaunch_cache + +# root is refused by nothing, so under root a permission test would pass with +# the behaviour it guards fully reverted. Same reasoning as PR #136's. +needs_an_unprivileged_user = pytest.mark.skipif( + os.geteuid() == 0, reason="root can empty any directory, so nothing here can refuse" +) + + +def git(*args: str, cwd: Path) -> str: + """Run git for real, failing the test with git's own words if it refuses.""" + result = subprocess.run(["git", *args], cwd=cwd, capture_output=True, text=True, check=False) + assert result.returncode == 0, f"git {' '.join(args)}: {result.stderr}" + return result.stdout.strip() + + +def commit(work: Path, message: str) -> None: + git("add", "-A", cwd=work) + git("-c", "user.email=t@t", "-c", "user.name=t", "commit", "-m", message, cwd=work) + + +class World: + """The cache `dl --prune` is pointed at, and the devpod that answers about it. + + One clone of each kind the classification has an arm for, plus the bare cache + every one of them was made from. + """ + + def __init__(self, cache: Path): + self.cache = cache + self.repo_dir = cache / "repos" / "o" / "r" + self.bare = self.repo_dir / ".bare" + self.clones: Dict[str, Path] = {} + self.listed: List[dict] = [] + self.records: Dict[str, WorktreeInfo] = {} + + @property + def storage(self) -> MetadataStorage: + return MetadataStorage(self.cache / "metadata.json") + + def record_branches(self) -> List[str]: + """The branches metadata.json still holds a worktree record for.""" + return sorted(record.branch for record in self.storage.list_worktrees()) + + +def _entry(workspace_id: str, source) -> dict: + """One `devpod list --output json` element.""" + return { + "id": workspace_id, + "source": {"localFolder": str(source)}, + "provider": {"name": "docker"}, + "ide": {"name": "none"}, + "lastUsed": "2026-08-08T11:43:27Z", + } + + +# Captured before anything is patched, because patching `devlaunch.dl.subprocess` +# patches the subprocess *module* -- `dl` imports it rather than a name out of +# it, so there is only one `subprocess.run` in the process and a stub put there +# answers every module's calls. +_real_run = subprocess.run + + +class RecordedDevpod: + """Answers `devpod` from the listing it is asked for, and lets everything + else run. + + The listing is a callable rather than a captured string, because the two + passes of this command are meant to be able to see different worlds. A + launch that finishes while the report is on screen registers a workspace, + and the pass that acts has to be able to learn about it -- a fixture that + answered both listings from one snapshot could not tell whether it did. + + Letting git through is not a convenience. The unsaved-work guard is a real + `git status` and a real `git log --not --remotes`, and a stub that answered + every command would hand it a clean exit with empty output -- which reads as + "this clone holds nothing", which is the answer that deletes it. Written the + other way round, this file's central guard passed while guarding nothing, + and the clone with two unpushed commits in it was removed. + """ + + def __init__(self, listing: Callable[[], str]): + self.listing = listing + self.commands: List[List[str]] = [] + + def __call__(self, cmd, *args, **kwargs): + argv = list(cmd) + if argv[:1] != ["devpod"]: + # pylint: disable=subprocess-run-check # the caller's own kwargs carry it + return _real_run(cmd, *args, **kwargs) + self.commands.append(argv) + if argv[1:2] == ["list"]: + return subprocess.CompletedProcess(argv, 0, self.listing(), "") + return subprocess.CompletedProcess(argv, 0, "", "") + + @property + def devpod_commands(self) -> List[List[str]]: + """Every devpod invocation, without the leading `devpod`, in order.""" + return [argv[1:] for argv in self.commands] + + +@pytest.fixture +def world(tmp_path) -> World: + """A cache holding one clone directory of every kind, all of them real repos. + + `referenced` is sourced by a live workspace. `orphan-clean` is sourced by + nobody and holds nothing unpushed. `orphan-dirty` is sourced by nobody and + holds both an unpushed commit and an uncommitted file -- 13 of the reference + host's 37 stale clones were in that state, two of them with real work in + them. `disputed` has a metadata record naming a workspace devpod still lists + but sources somewhere else entirely, which is devlaunch#88's shape. + """ + world = World(devlaunch_cache()) + world.repo_dir.mkdir(parents=True) + + origin = tmp_path / "origin.git" + seed = tmp_path / "seed" + git("init", "-b", "main", str(seed), cwd=tmp_path) + (seed / "README.md").write_text("seed\n") + commit(seed, "seed") + git("clone", "--bare", str(seed), str(origin), cwd=tmp_path) + git("clone", "--bare", str(origin), str(world.bare), cwd=tmp_path) + + storage = world.storage + for leaf, branch in ( + ("referenced", "ref"), + ("orphan-clean", "clean"), + ("orphan-dirty", "dirty"), + ("disputed", "disp"), + ): + clone = world.repo_dir / leaf + # Cloned from the bare, exactly as dl does it, so the git objects are + # hardlinked out of the bare and the reported sizes are the real + # exclusive ones rather than a double count. + git("clone", str(world.bare), str(clone), cwd=tmp_path) + git("remote", "set-url", "origin", str(origin), cwd=clone) + git("checkout", "-b", branch, cwd=clone) + (clone / f"{branch}.txt").write_text("work\n") + commit(clone, branch) + git("push", "-u", "origin", branch, cwd=clone) + world.clones[leaf] = clone + record = WorktreeInfo( + owner="o", repo="r", branch=branch, local_path=clone, workspace_id=leaf + ) + storage.add_worktree(record) + world.records[leaf] = record + + # Two megabytes of payload nothing else links to, so the reclaimed figure is + # a number a test can name rather than whatever git happened to write. + (world.clones["orphan-clean"] / "payload.bin").write_bytes(b"\0" * 2 * 1024 * 1024) + git("update-index", "--skip-worktree", "--", "README.md", cwd=world.clones["orphan-clean"]) + (world.clones["orphan-clean"] / ".git" / "info" / "exclude").write_text("payload.bin\n") + + dirty = world.clones["orphan-dirty"] + (dirty / "later.txt").write_text("later\n") + commit(dirty, "later") # committed, never pushed + (dirty / "scratch.md").write_text("an agent's notes\n") # never even added + + world.listed = [ + _entry("referenced", world.clones["referenced"]), + _entry("disputed", tmp_path / "somewhere" / "else"), + ] + return world + + +def reported(out: str, path: Path) -> str: + """The one report line that names *path*, or a failure saying it is absent. + + Every assertion in this file about a directory *surviving* goes through + here rather than through `path.exists()`, because "it is still there" is + true of a clone kept for the right reason and of one kept by a guard that + was never asked. The line says which. + """ + lines = [row for row in out.splitlines() if str(path) in row] + assert len(lines) == 1, f"expected one report line for {path}, got {lines}" + return lines[0] + + +def run_prune(world: World, *flags: str, answer: Optional[str] = None): + """Run `dl --prune` against *world*, with devpod answering its listing.""" + devpod = RecordedDevpod(lambda: json.dumps(world.listed)) + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + if answer is None: + code = main(["--prune", *flags]) + else: + with patch("builtins.input", return_value=answer): + code = main(["--prune", *flags]) + return code, devpod + + +class TestWhichCloneDirectoriesAreRemoved: + """The three arms, decided from the disk and from devpod's own listing.""" + + def test_a_clone_nothing_sources_is_removed(self, world, capsys): + code, _devpod = run_prune(world, "-y") + capsys.readouterr() + assert code == 0 + assert not world.clones["orphan-clean"].exists() + + def test_a_clone_a_live_workspace_sources_is_kept(self, world, capsys): + """The failure this one guards costs somebody a running workspace. + + Asserted on the *reason* rather than on the directory surviving. Written + the other way round it passed with the Referenced arm deleted outright: + every clone in this fixture also carries a metadata record, so one that + stops being recognised as opened lands in Disputed and survives for the + wrong reason -- a live workspace described to its owner as a records + disagreement to go and fix. + """ + run_prune(world, "-y") + out = capsys.readouterr().out + assert world.clones["referenced"].exists() + assert reported(out, world.clones["referenced"]).endswith( + "workspace referenced still opens it" + ) + + def test_a_clone_holding_work_saved_nowhere_else_is_kept(self, world, capsys): + """Nothing sources it, and deleting it would still destroy an unpushed + commit and an untracked file. 13 of the reference host's 37 stale clones + were exactly this.""" + run_prune(world, "-y") + capsys.readouterr() + assert world.clones["orphan-dirty"].exists() + + def test_a_clone_devpod_sources_somewhere_else_is_kept(self, world, capsys): + """devlaunch#88's shape: devpod still lists the workspace this directory + was made for, and records it at a path that is not this one. That is a + disagreement between two records, not evidence that nothing needs the + clone, and the safe reading of a disagreement is to keep the disk.""" + run_prune(world, "-y") + capsys.readouterr() + assert world.clones["disputed"].exists() + + def test_the_bare_cache_is_never_a_candidate(self, world, capsys): + """No workspace ever sources `.bare` and no record names it, so every + rule above would call it an orphan. It is what makes the next clone of + the repo fast, and all seven on the reference host came to 0.08 GB.""" + run_prune(world, "-y") + out = capsys.readouterr().out + assert world.bare.exists() + assert ".bare" not in out + + +class TestEveryWayAWorkspaceCanNameADirectory: + """A source arm this command does not read is a directory it will delete.""" + + def test_a_workspace_devpod_records_as_a_git_source_still_holds_its_clone(self, world, capsys): + """`devpod up ` is recorded as a `gitRepository`, and + nothing stops that repo living in the cache -- which is why the source + here is a path inside it rather than a URL, the same shape the purge + tests use. `--purge`'s predicate refuses this arm on purpose, and that + answer must not be reused: refusing there means declining to delete + somebody's *workspace*, and refusing here means deleting their clone. + + Asserted on the *reason*, not on the directory surviving. Written the + other way round this passed with the arm deleted: a clone that stops + being recognised as opened still carries a metadata record naming a + workspace devpod lists, so it lands in Disputed and survives for the + wrong reason -- a records disagreement reported where there is none, and + a live clone described to its owner as something to go and fix. + """ + world.listed[0] = { + **_entry("referenced", "unused"), + "source": {"gitRepository": str(world.clones["referenced"])}, + } + code, _devpod = run_prune(world, answer="n") + out = capsys.readouterr().out + assert code == 0 + line = next(row for row in out.splitlines() if str(world.clones["referenced"]) in row) + assert line.endswith("workspace referenced still opens it"), line + + def test_a_symlink_standing_where_a_clone_would_be_is_left_alone(self, world, capsys): + """Following it would put this command outside the cache entirely, and + unlinking it would report a clone as reclaimed while it sat on another + volume. remove_tree refuses a symlinked root for exactly those two + reasons; not making one a candidate is that refusal a step earlier. + + Asserted on it never reaching the report, because "the link is still + there" is true either way and so is "the clone is still there". 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 both + survive, and the only difference is a directory in the plan that was + never devlaunch's to name and an exit code of 1 on a cache with nothing + wrong with it. That is what this assertion had to be changed to catch. + """ + elsewhere = world.repo_dir / "points-away" + elsewhere.symlink_to(world.clones["referenced"]) + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 0 + assert str(elsewhere) not in out + assert elsewhere.is_symlink() + assert world.clones["referenced"].exists() + + +class TestADevpodThatCouldNotBeRead: + def test_a_listing_that_failed_is_not_read_as_a_machine_with_no_workspaces(self, world, capsys): + """The worst thing devpod can do to this command is answer badly, and + the worst answer is an empty one: every clone in the cache would be + opened by nobody at once. So a listing that could not be read is not an + answer, and `--prune` never gets as far as looking at a directory.""" + + def broken(cmd, *args, **kwargs): + argv = list(cmd) + if argv[:1] != ["devpod"]: + # pylint: disable=subprocess-run-check # the caller's own kwargs carry it + return _real_run(cmd, *args, **kwargs) + return subprocess.CompletedProcess(argv, 1, "", "devpod: provider is not configured") + + with patch("devlaunch.dl.subprocess.run", side_effect=broken): + code = main(["--prune", "-y"]) + assert code != 0 + assert all(clone.exists() for clone in world.clones.values()) + assert "Removing" not in capsys.readouterr().out + + +class TestAnEmptyCache: + def test_a_machine_with_no_clone_directories_has_nothing_to_prune(self, capsys): + """The first run on a fresh install, where `repos_dir` does not exist.""" + devpod = RecordedDevpod(lambda: "[]") + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + code = main(["--prune", "-y"]) + assert code == 0 + assert "Nothing to prune." in capsys.readouterr().out + + +class TestWhatForcePromotesAndWhatItDoesNot: + """`--force` is not a general override. It answers exactly one refusal.""" + + def test_it_removes_the_clone_that_holds_unsaved_work(self, world, capsys): + code, _devpod = run_prune(world, "-y", "--force") + capsys.readouterr() + assert code == 0 + assert not world.clones["orphan-dirty"].exists() + + def test_it_does_not_touch_the_clone_a_live_workspace_opens(self, world, capsys): + """There is nothing for a user to mean by insisting past this one: the + workspace is still there and still opens this directory. + + On the reason, for the same cause as its sibling above: under `--force` + the unsaved guard is not what is holding this directory, but the + Disputed arm still would be, and that is not what is being tested. + """ + run_prune(world, "-y", "--force") + out = capsys.readouterr().out + assert world.clones["referenced"].exists() + assert reported(out, world.clones["referenced"]).endswith( + "workspace referenced still opens it" + ) + + def test_it_does_not_touch_the_clone_devpod_sources_elsewhere(self, world, capsys): + """Disputed is never removable, `--force` included. It is not devlaunch + refusing to act on a user's behalf, it is devlaunch's records and + devpod's disagreeing -- and forcing a deletion is not an answer to a + disagreement, it is a way of losing the argument permanently.""" + run_prune(world, "-y", "--force") + capsys.readouterr() + assert world.clones["disputed"].exists() + + def test_the_bare_cache_survives_force_too(self, world, capsys): + run_prune(world, "-y", "--force") + capsys.readouterr() + assert world.bare.exists() + + +class TestTheReportComesBeforeTheQuestion: + """Everything a person needs to answer `y` is on screen before they do.""" + + def test_declining_removes_nothing(self, world, capsys): + code, _devpod = run_prune(world, answer="n") + capsys.readouterr() + assert code == 0 + assert all(clone.exists() for clone in world.clones.values()) + + def test_every_survivor_is_named_with_the_reason_it_survived(self, world, capsys): + """Silence about a survivor is the surprise: a person who asked for a + tidy-up and gets 4 GB back has no way to learn from `dl --ls` that a + clone was left behind, because a clone with no workspace has no row + there to appear in.""" + run_prune(world, answer="n") + out = capsys.readouterr().out + assert "workspace referenced still opens it" in out + assert "unpushed commit(s)" in out and "--force" in out + assert "devlaunch#88" in out + assert str(world.clones["disputed"]) in out + + def test_the_plan_is_printed_before_the_question_is_asked(self, world, capsys): + """`--purge`'s shape: the report is what the question is about, so it + cannot arrive after the answer.""" + seen = {} + + def answering(_prompt=""): + seen["out"] = capsys.readouterr().out + return "n" + + devpod = RecordedDevpod(lambda: json.dumps(world.listed)) + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + with patch("builtins.input", side_effect=answering): + assert main(["--prune"]) == 0 + assert str(world.clones["orphan-clean"]) in seen["out"] + + def test_work_written_while_the_question_was_open_is_not_destroyed(self, world, capsys): + """The plan was made before the user answered it, and a container + writing into a clone in between is the ordinary case here rather than + the exotic one. So each directory is asked once more what it holds, + under the repository lock, immediately before it goes -- the approved + set may shrink and may never grow.""" + + def answering(_prompt=""): + (world.clones["orphan-clean"] / "notes.md").write_text("written while deciding\n") + return "y" + + devpod = RecordedDevpod(lambda: json.dumps(world.listed)) + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + with patch("builtins.input", side_effect=answering): + code = main(["--prune"]) + out = capsys.readouterr().out + assert code == 0 + assert world.clones["orphan-clean"].exists() + assert "notes.md" in out + + def test_an_option_it_does_not_know_is_refused_rather_than_ignored(self, world, capsys): + """`dl --prune --dry-run -y` reads as a rehearsal. Ignoring the flag it + does not know would make it a deletion instead, and the exit code would + be 0.""" + code, _devpod = run_prune(world, "--dry-run", "-y") + capsys.readouterr() + assert code == 1 + assert all(clone.exists() for clone in world.clones.values()) + + +class TestRunningItAgain: + def test_a_second_run_finds_nothing_left_to_do(self, world, capsys): + assert run_prune(world, "-y")[0] == 0 + capsys.readouterr() + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 0 + assert "Nothing to prune." in out + + +class TestTheRecordsFollowTheDirectories: + """metadata.json is append-mostly, and this is where that stops.""" + + def test_the_removed_clone_loses_its_record_and_the_others_keep_theirs(self, world, capsys): + assert world.record_branches() == ["clean", "dirty", "disp", "ref"] + run_prune(world, "-y") + capsys.readouterr() + assert world.record_branches() == ["dirty", "disp", "ref"] + + def test_a_record_for_a_directory_that_is_already_gone_is_dropped(self, world, capsys): + """49 records for 17 live workspaces on the reference host. These ones + describe nothing at all, and nothing else was ever going to drop them.""" + world.storage.add_worktree( + WorktreeInfo( + owner="o", + repo="r", + branch="long-gone", + local_path=world.repo_dir / "long-gone", + workspace_id="long-gone", + ) + ) + assert "long-gone" in world.record_branches() + run_prune(world, "-y") + capsys.readouterr() + assert "long-gone" not in world.record_branches() + + @needs_an_unprivileged_user + def test_a_record_whose_directory_cannot_be_looked_at_is_kept(self, world, capsys): + """ "Gone" is only ever FileNotFoundError. A directory behind a door this + process cannot open is still a directory, and dropping its record would + throw away the only note of where a clone lives -- on a machine where + the container ran as another user, which is the machine this whole + command is for.""" + sealed = world.cache / "sealed" + (sealed / "clone").mkdir(parents=True) + world.storage.add_worktree( + WorktreeInfo( + owner="o", + repo="r", + branch="behind-a-door", + local_path=sealed / "clone", + workspace_id="behind-a-door", + ) + ) + sealed.chmod(0o000) + try: + run_prune(world, "-y") + capsys.readouterr() + assert "behind-a-door" in world.record_branches() + finally: + sealed.chmod(0o700) + + +class TestACacheReachedThroughASymlink: + """The total-loss regression, and the reason it gets a test of its own.""" + + def test_a_workspace_that_names_its_clone_through_a_symlink_still_holds_it( + self, world, tmp_path, capsys + ): + """devpod records whatever path it was handed. Somebody who moved their + cache and left a symlink behind -- or whose `/tmp` is one -- has records + naming a path that is not the canonical one, and a comparison that + stopped resolving both sides would find that *no* clone is referenced + and delete every one of them. There is no undo for that. + + Asserted on the reason, which is the whole difference between this test + and the one it replaced. "The clone is still there" is true with the + resolve taken off too: unresolved, the source matches nothing, the + directory falls through to its own metadata record, and the record names + a workspace devpod lists -- so it survives on the Disputed arm, reported + to its owner as a records disagreement, and the test that named this + regression passed anyway. + """ + through_a_link = tmp_path / "link-to-cache" + through_a_link.symlink_to(world.cache) + world.listed[0] = _entry("referenced", through_a_link / "repos" / "o" / "r" / "referenced") + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 0 + assert reported(out, world.clones["referenced"]).endswith( + "workspace referenced still opens it" + ) + # And the run was a real one, not a scan that found nothing to do. + assert not world.clones["orphan-clean"].exists() + + +class TestWhenAWorkspaceCannotBeLocated: + def test_a_source_that_cannot_be_read_as_a_path_stops_the_whole_command(self, world, capsys): + """A live workspace whose source will not resolve could be opening any + of the candidates. While one exists there is no directory this command + can honestly call unreferenced, so it says so and removes nothing -- + rather than quietly dropping that workspace out of the comparison, + which is a deletion decided by a lookup that failed. + + The source here is text devpod's JSON can carry and no filesystem call + will accept. A source that merely does not *exist* deliberately does not + land here -- that is most of the workspaces on devlaunch#88's host, and + stopping on it would make this command unrunnable exactly where it is + most needed. + """ + world.listed.append(_entry("unreadable-source", "/tmp/not\0a/path")) + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 1 + assert "unreadable-source" in out + assert all(clone.exists() for clone in world.clones.values()) + + def test_a_source_that_is_simply_gone_does_not(self, world, capsys): + """The other half, and the reason the two are told apart at all.""" + world.listed.append(_entry("source-deleted", world.cache / "went" / "away")) + code, _devpod = run_prune(world, "-y") + capsys.readouterr() + assert code == 0 + assert not world.clones["orphan-clean"].exists() + + +class TestWhenADirectoryWillNotComeAway: + """A container writes into its clone as its own user, so this is ordinary.""" + + @staticmethod + def _seal(world: World) -> Path: + """A second orphan with a door in it this process cannot open.""" + leftover = world.repo_dir / "leftover" + (leftover / "locked").mkdir(parents=True) + (leftover / "locked" / "payload.bin").write_bytes(b"\0" * 512 * 1024) + (leftover / "locked").chmod(0o000) + return leftover + + @needs_an_unprivileged_user + def test_what_could_not_be_measured_is_reported_as_a_floor_not_a_total(self, world, capsys): + """The one number this command must never print is a floor with the + `≥` taken off. It is a cleanup tool telling somebody a directory is + small when it is not -- and the total across the plan is where that + would happen, because a sum of integers has lost which kind of answer + each of them was.""" + leftover = self._seal(world) + try: + run_prune(world, answer="n") + out = capsys.readouterr().out + finally: + (leftover / "locked").chmod(0o700) + line = next(row for row in out.splitlines() if str(leftover) in row) + assert "≥" in line, line + headline = next(row for row in out.splitlines() if row.startswith("Removing ")) + assert "≥" in headline, headline + + @needs_an_unprivileged_user + def test_a_directory_that_refuses_is_named_and_its_siblings_still_go(self, world, capsys): + """PR #136's contract, unchanged: what would not come away is named + with what the system said about it, everything else still goes, and the + exit code says the job is unfinished rather than untried.""" + leftover = self._seal(world) + try: + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + finally: + (leftover / "locked").chmod(0o700) + assert code == 1 + assert f"{leftover / 'locked'}:" in out + assert not world.clones["orphan-clean"].exists() + assert (leftover / "locked").exists() + + @needs_an_unprivileged_user + def test_the_command_to_paste_names_the_obstruction_and_nothing_else(self, world, capsys): + """The one line here a person runs as root, so it is the one line that + must not be widened by accident. Aimed at `plan.root`: pointed there + instead of at the refusals, this prints an instruction to `rm -rf` every + clone in the cache, live ones included, and every other assertion in + this file still holds.""" + leftover = self._seal(world) + try: + run_prune(world, "-y") + out = capsys.readouterr().out + finally: + (leftover / "locked").chmod(0o700) + advice = next(row for row in out.splitlines() if "sudo rm -rf" in row) + assert shlex.split(advice)[3:] == [str(leftover / "locked")], advice + + +_LOCK_HOLDER = """ +import fcntl, os, sys, time +from pathlib import Path + +lock_path, held, release = sys.argv[1:4] +Path(lock_path).parent.mkdir(parents=True, exist_ok=True) +fd = os.open(lock_path, os.O_RDWR | os.O_CREAT, 0o600) +fcntl.flock(fd, fcntl.LOCK_EX) +Path(held).touch() +while not Path(release).exists(): + time.sleep(0.01) +os.close(fd) +""" + + +class TestItWaitsForALaunchThatIsStillCloning: + """The window a clone is born in, and why `--prune` may not look into it.""" + + def test_it_blocks_while_another_process_holds_the_repository_lock( + self, world, tmp_path, capsys + ): + """A cold launch fills a clone directory completely before it returns, + under this exact lock. A scan that walked past it would weigh -- and + then delete -- a directory `git clone` was still writing into, and the + launch would fail with an error about a repository that was there a + moment ago. + + Deterministic, in the shape test_concurrent_launches.py already uses: a + separate process takes the lock and sits on it, and the prune must not + get past the scan until it lets go. Unserialized, a scan of four small + clones is finished in well under the grace period. + """ + script = tmp_path / "holder.py" + script.write_text(_LOCK_HOLDER) + held, release = tmp_path / "held", tmp_path / "release" + lock_path = world.repo_dir / ".lock" + # Not a `with`: the holder has to outlive the block that starts it and be + # let go from the `finally` below, which is what makes the release + # deterministic rather than tied to a scope exit. + holder = subprocess.Popen( # pylint: disable=consider-using-with + [sys.executable, str(script), str(lock_path), str(held), str(release)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + try: + deadline = time.monotonic() + 60 + while not held.exists(): + assert time.monotonic() < deadline, "the lock holder never started" + time.sleep(0.01) + + done = threading.Event() + + def prune(): + try: + run_prune(world, "-y") + finally: + done.set() + + worker = threading.Thread(target=prune, daemon=True) + worker.start() + still_blocked = not done.wait(timeout=3) + still_there = world.clones["orphan-clean"].exists() + release.touch() + worker.join(timeout=60) + capsys.readouterr() + assert still_blocked, "the scan ran while another process held the repository lock" + assert still_there + assert not world.clones["orphan-clean"].exists() + finally: + release.touch() + holder.communicate(timeout=60) + + +class TestADevpodRecordThatPointsAtNothing: + """devlaunch#88's measured shape, and the reason `--prune` may ship ahead of it. + + On that ticket's host, **36 of 39** devpod workspaces recorded a source + folder that was missing (35) or a config-only stub devpod itself rebuilt + from cache (1), while the real checkout sat beside it under the new id + scheme. The two sides cannot be joined by workspace id -- the id scheme + change is what broke them apart -- so the join is made from the path devpod + still records, which names the repository exactly. + + Without this, `--prune` on such a host keeps the stub, because a live + workspace really does open it, and deletes the live workspace's only real + checkout, because nothing sources it and its record names an id devpod no + longer lists. That is the total loss this whole command is written to avoid, + on the host class the decision to build it cited. + """ + + @staticmethod + def _stub(world: World, leaf: str) -> Path: + """The config-only folder devpod reconstitutes from `workspace_result.json`. + + A real directory with a `devcontainer.json` in it and no `.git` -- one + file, which is what devlaunch#88 measured. + """ + stub = world.repo_dir / leaf + (stub / ".devcontainer").mkdir(parents=True) + (stub / ".devcontainer" / "devcontainer.json").write_text('{"name": "r"}\n') + return stub + + def test_a_workspace_recorded_at_a_stub_disputes_that_repositorys_clones(self, world, capsys): + """The one of #88's 36 whose recorded folder still exists. devpod opens + it, so it survives on its own account -- and the real checkout beside it + must not be read as unreferenced just because the workspace id moved.""" + stub = self._stub(world, "r-main-oldscheme") + world.listed[0] = _entry("r-main", stub) + code, _devpod = run_prune(world, "-y", "--force") + out = capsys.readouterr().out + assert code == 0 + assert world.clones["referenced"].exists() + assert "devlaunch#88" in reported(out, world.clones["referenced"]) + assert stub.exists() + + def test_a_workspace_recorded_at_a_folder_that_is_gone_disputes_them_too(self, world, capsys): + """35 of #88's 36. The recorded folder is not there at all, so there is + nothing to open and nothing to match -- and which of this repository's + clones the workspace needs is exactly the question #88 exists to + answer.""" + world.listed[0] = _entry("r-main", world.repo_dir / "main") + code, _devpod = run_prune(world, "-y", "--force") + out = capsys.readouterr().out + assert code == 0 + assert world.clones["referenced"].exists() + assert world.clones["orphan-clean"].exists() + assert "devlaunch#88" in reported(out, world.clones["orphan-clean"]) + + def test_a_workspace_recorded_at_the_repository_directory_itself_disputes_it( + self, world, capsys + ): + """The shallowest way a record can land in a repository's tree and name + no clone of it. It is still a live workspace somewhere in there, and + which clone it wants is still unanswerable -- read as "not in the tree" + instead, every clone of the repository becomes prunable while the + workspace is open.""" + world.listed[0] = _entry("r-main", world.repo_dir) + code, _devpod = run_prune(world, "-y", "--force") + out = capsys.readouterr().out + assert code == 0 + assert world.clones["orphan-clean"].exists() + assert "devlaunch#88" in reported(out, world.clones["orphan-clean"]) + + def test_only_that_repositorys_clones_are_disputed(self, world, capsys): + """Scoped to the repository whose tree the broken record points into, + which is what keeps this command useful on #88's host rather than merely + safe on it: seven repositories were affected there and the rest of the + cache is still somebody's dead disk.""" + elsewhere = world.cache / "repos" / "other" / "project" / "stale" + elsewhere.mkdir(parents=True) + (elsewhere / "payload.bin").write_bytes(b"\0" * 1024) + world.listed[0] = _entry("r-main", world.repo_dir / "main") + code, _devpod = run_prune(world, "-y") + capsys.readouterr() + assert code == 0 + assert not elsewhere.exists() + assert world.clones["referenced"].exists() + + +class TestAWorkspaceThatAppearedWhileTheQuestionWasOpen: + """The plan is a photograph; the act happens later, and the world moved. + + A cold launch fills its clone under the repository lock and only registers a + devpod workspace after releasing it, so a launch that was mid-clone when the + report printed can be a *live workspace* by the time the user says yes -- and + the clone path for `(owner, repo, branch)` is deterministic, so it is one of + the directories in the plan, not a new one. + """ + + def test_a_clone_a_launch_registered_since_the_plan_is_not_removed(self, world, capsys): + def answering(_prompt=""): + world.listed.append(_entry("o-r-clean", world.clones["orphan-clean"])) + return "y" + + devpod = RecordedDevpod(lambda: json.dumps(world.listed)) + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + with patch("builtins.input", side_effect=answering): + code = main(["--prune"]) + out = capsys.readouterr().out + assert code == 0 + assert world.clones["orphan-clean"].exists() + assert "o-r-clean" in out + + def test_it_is_not_removed_under_force_either(self, world, capsys): + """`--force` promotes unsaved work on an orphan. A directory a live + workspace opens is not that arm at all, on either pass.""" + + def answering(_prompt=""): + world.listed.append(_entry("o-r-clean", world.clones["orphan-clean"])) + return "y" + + devpod = RecordedDevpod(lambda: json.dumps(world.listed)) + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + with patch("builtins.input", side_effect=answering): + code = main(["--prune", "--force"]) + capsys.readouterr() + assert code == 0 + assert world.clones["orphan-clean"].exists() + + +class TestAWorkspaceThatOpensPartOfAClone: + def test_a_live_workspace_opening_a_subdirectory_still_holds_the_clone(self, world, capsys): + """`devpod up /subproject` is an ordinary thing to do with a + monorepo, and it records the subdirectory. Compared by equality the + clone itself is opened by nobody -- and deleting it takes the running + workspace's checkout with it. + + Run under `--force` and with the subdirectory committed and pushed, so + the unsaved-work guard has nothing to say and cannot be what saves it. + """ + clone = world.clones["referenced"] + sub = clone / "subproject" + sub.mkdir() + (sub / "app.py").write_text("print('hi')\n") + commit(clone, "subproject") + git("push", cwd=clone) + world.listed[0] = _entry("referenced", sub) + code, _devpod = run_prune(world, "-y", "--force") + out = capsys.readouterr().out + assert code == 0 + assert clone.exists() + assert reported(out, clone).endswith("workspace referenced still opens it") + + +class TestASourceThatNamesAFolderItCannotRead: + """`UnrecognisedSource` used to answer two opposite questions with one arm.""" + + def test_a_local_folder_devlaunch_cannot_read_stops_the_whole_command(self, world, capsys): + """devpod says this workspace opens a directory here. While devlaunch + cannot say which, no clone is unreferenced -- and the command already + prints exactly that for a source that will not resolve. It used to print + nothing and carry on, because this arm contributed no path *and* no + alarm.""" + world.listed.append( + {**_entry("folder-i-cannot-read", "unused"), "source": {"localFolder": {"a": "b"}}} + ) + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 1 + assert "folder-i-cannot-read" in out + assert all(clone.exists() for clone in world.clones.values()) + + def test_one_that_appeared_since_the_plan_stops_the_removal_too(self, world, capsys): + """Both passes have to be able to stop, and for the same reason. A + workspace devlaunch cannot place could be opening any of the directories + the user just approved -- the fact that it was placeable when the report + was printed is not an answer about the world the removal happens in.""" + + def answering(_prompt=""): + world.listed.append( + { + **_entry("folder-i-cannot-read", "unused"), + "source": {"localFolder": {"a": "b"}}, + } + ) + return "y" + + devpod = RecordedDevpod(lambda: json.dumps(world.listed)) + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + with patch("builtins.input", side_effect=answering): + code = main(["--prune"]) + out = capsys.readouterr().out + assert code == 1 + assert "folder-i-cannot-read" in out + assert all(clone.exists() for clone in world.clones.values()) + + def test_a_workspace_that_opens_no_folder_here_does_not(self, world, capsys): + """The other half, and the reason the arm had to be split rather than + made to alarm. `devpod up ubuntu:24.04` mounts no directory on this + machine, so no clone can be at risk from it -- and stopping on one would + make `--prune` unrunnable for anybody who has ever run devpod by hand.""" + world.listed.append( + {**_entry("an-image", "unused"), "source": {"container": "abc123def456"}} + ) + code, _devpod = run_prune(world, "-y") + capsys.readouterr() + assert code == 0 + assert not world.clones["orphan-clean"].exists() + + +class TestACloneGitWillNotAnswerAbout: + """ "Could not ask" is not "nothing to lose", and this is where it costs. + + Clones are precisely the directories a container wrote as uid 1000, so a + repository this process cannot read is the normal case on the machine + devlaunch exists for -- not the exotic one. Read as clean, such a clone is + removed with nothing typed. + """ + + @staticmethod + def _unreadable(clone: Path) -> None: + (clone / ".git" / "HEAD").unlink() + + def test_a_clone_whose_repository_will_not_open_is_kept_with_nothing_typed(self, world, capsys): + clone = world.clones["orphan-clean"] + (clone / "scratch.md").write_text("an agent's notes\n") + self._unreadable(clone) + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 0 + assert clone.exists() + assert (clone / "scratch.md").exists() + line = reported(out, clone) + assert "could not" in line and "--force" in line, line + + def test_force_is_what_removes_it(self, world, capsys): + """Kept is not unremovable. A clone whose git is broken is exactly what + somebody runs this command to clear away, and `--force` is the sentence + that says they meant it.""" + clone = world.clones["orphan-clean"] + self._unreadable(clone) + code, _devpod = run_prune(world, "-y", "--force") + capsys.readouterr() + assert code == 0 + assert not clone.exists() + + def test_a_directory_that_was_never_a_repository_is_still_removed(self, world, capsys): + """The distinction the arm rests on. Left-behind notes under the cache + really do hold nothing git could lose; a repository that will not open + does not, and the two must not be told apart by asking git, which + refuses identically for both.""" + stray = world.repo_dir / "notes-i-left-here" + stray.mkdir() + (stray / "plan.md").write_text("half a plan\n") + code, _devpod = run_prune(world, "-y") + capsys.readouterr() + assert code == 0 + assert not stray.exists() + + +class TestWhatForceIsAnsweringAndForWhich: + """`--force` answers one directory's objection, not the whole plan's.""" + + def test_the_plan_names_what_forcing_this_one_destroys(self, world, capsys): + """Under `--force` the dirty clone used to read exactly like the empty + one -- a path and a size -- so the confirmation could not say what it + cost, and there is no later chance to.""" + run_prune(world, "--force", answer="n") + out = capsys.readouterr().out + dirty = reported(out, world.clones["orphan-dirty"]) + assert "unpushed commit(s)" in dirty and "removing anyway" in dirty, dirty + assert "removing anyway" not in reported(out, world.clones["orphan-clean"]) + + def test_it_does_not_turn_off_the_re_probe_for_a_clone_it_promoted_nothing_about( + self, world, capsys + ): + """Read from the plan as one boolean, `--force` skipped the pre-removal + re-check for every directory in the plan -- including the ones it had + promoted nothing about. Work written into a clean orphan while the user + was reading the report was then destroyed.""" + + def answering(_prompt=""): + (world.clones["orphan-clean"] / "notes.md").write_text("written while deciding\n") + return "y" + + devpod = RecordedDevpod(lambda: json.dumps(world.listed)) + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + with patch("builtins.input", side_effect=answering): + code = main(["--prune", "--force"]) + out = capsys.readouterr().out + assert code == 0 + assert world.clones["orphan-clean"].exists() + assert "notes.md" in out + # And the one it *did* promote still goes: this is not --force undone. + assert not world.clones["orphan-dirty"].exists() + + +class TestEveryPathThisCommandComparesIsCanonical: + """Three more places a symlink decides what gets deleted. + + The workspace source has a test of its own above. These are the other side + of each comparison, and all three survived mutation under the full suite + while the command was correct in every one of them -- correct and unheld. + """ + + def test_a_repos_dir_reached_through_a_symlink_still_matches_its_clones(self, world, capsys): + """`config.toml` may point `repos_dir` anywhere, including through a + link. Unresolved, every candidate path is the link's and every workspace + source is the target's, so no clone is referenced and the bare cache is + not recognised either -- the whole repository goes.""" + link = world.cache / "repos-through-a-link" + link.symlink_to(world.cache / "repos") + config = Path(os.environ["XDG_CONFIG_HOME"]) / "devlaunch" / "config.toml" + config.parent.mkdir(parents=True, exist_ok=True) + config.write_text(f'[worktree]\nrepos_dir = "{link}"\n') + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 0 + assert reported(out, world.clones["referenced"]).endswith( + "workspace referenced still opens it" + ) + assert world.bare.exists() + assert ".bare" not in out + # A real run rather than a scan that found nothing to look at. + assert not world.clones["orphan-clean"].exists() + + def test_a_record_written_through_a_symlink_still_disputes_its_clone( + self, world, tmp_path, capsys + ): + """The metadata side. devlaunch writes `local_path` from whatever path + it was handed, so a cache reached through a link puts a non-canonical + path in the record -- and a record that does not match its directory + drops that clone from Disputed to Orphaned, which is a deletion. + + Under `--force`, so the unsaved-work guard cannot be what holds it. + """ + through_a_link = tmp_path / "link-to-cache" + through_a_link.symlink_to(world.cache) + storage = world.storage + storage.remove_worktree("o", "r", "disp") + storage.add_worktree( + WorktreeInfo( + owner="o", + repo="r", + branch="disp", + local_path=through_a_link / "repos" / "o" / "r" / "disputed", + workspace_id="disputed", + ) + ) + code, _devpod = run_prune(world, "-y", "--force") + out = capsys.readouterr().out + assert code == 0 + assert world.clones["disputed"].exists() + assert "devlaunch#88" in reported(out, world.clones["disputed"]) + + +class TestARunWhoseOnlyWorkIsTheRecords: + def test_a_cache_with_only_a_stale_record_left_is_not_nothing_to_prune(self, world, capsys): + """metadata.json is the other half of what this command tidies, and a + run that would drop a record and say "Nothing to prune." does nothing + and reports that it was right to. + + The cache here has no removable *directory* at all -- the clean orphan + was cleared away by hand, which is how its record came to describe + nothing -- so the record is the whole of the work. + """ + shutil.rmtree(world.clones["orphan-clean"]) + assert "clean" in world.record_branches() + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 0 + assert "Nothing to prune." not in out + assert "clean" not in world.record_branches() + + +class TestTheLockIsHeldWhileItRemoves: + """The scan takes the repository lock. So must the pass that deletes. + + The concurrency test above proves the *plan* pass waits. The window that + actually costs a clone is later: a launch that starts while the user is + reading the report holds this lock while it fills a directory, and a removal + that walked past it deletes what `git clone` is writing into. + """ + + def test_it_blocks_after_the_answer_while_another_process_holds_the_lock( + self, world, tmp_path, capsys + ): + script = tmp_path / "holder.py" + script.write_text(_LOCK_HOLDER) + held, release = tmp_path / "held", tmp_path / "release" + lock_path = world.repo_dir / ".lock" + answered = threading.Event() + may_answer = threading.Event() + done = threading.Event() + + def answering(_prompt=""): + answered.set() + assert may_answer.wait(timeout=60), "the lock holder never took the lock" + return "y" + + def prune(): + try: + devpod = RecordedDevpod(lambda: json.dumps(world.listed)) + with patch("devlaunch.dl.subprocess.run", side_effect=devpod): + with patch("builtins.input", side_effect=answering): + main(["--prune"]) + finally: + done.set() + + worker = threading.Thread(target=prune, daemon=True) + worker.start() + holder = None + try: + assert answered.wait(timeout=60), "the plan was never printed" + # Started only now: the plan pass holds this same lock, so a holder + # taken earlier would prove nothing about the pass that removes. + holder = subprocess.Popen( # pylint: disable=consider-using-with + [sys.executable, str(script), str(lock_path), str(held), str(release)], + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + ) + deadline = time.monotonic() + 60 + while not held.exists(): + assert time.monotonic() < deadline, "the lock holder never started" + time.sleep(0.01) + may_answer.set() + still_blocked = not done.wait(timeout=3) + still_there = world.clones["orphan-clean"].exists() + release.touch() + worker.join(timeout=60) + capsys.readouterr() + assert still_blocked, "it removed a clone while another process held the lock" + assert still_there + assert not world.clones["orphan-clean"].exists() + finally: + release.touch() + may_answer.set() + if holder is not None: + holder.communicate(timeout=60) + + +class TestACloneWhoseGitIsAFile: + """A linked git worktree keeps `.git` as a *file*, not a directory. + + devlaunch clones today, so every clone in this cache has a `.git` directory + -- which is exactly why this needs saying out loud. Both places this command + asks "is there a repository at this path" would answer no for a worktree, + and both wrong answers are deletions: the checkout a live workspace opens + stops counting as a clone and its whole repository is disputed instead, and + an orphan git could perfectly well have been asked about is read as holding + nothing and removed. + """ + + @staticmethod + def _worktree(world: World, leaf: str, branch: str) -> Path: + path = world.repo_dir / leaf + git("worktree", "add", "-b", branch, str(path), cwd=world.bare) + assert (path / ".git").is_file(), "a linked worktree's .git is a file" + return path + + def test_a_live_workspace_opening_one_still_holds_it(self, world, capsys): + worktree = self._worktree(world, "worktree-referenced", "wt-ref") + world.listed[0] = _entry("wt-ref", worktree) + code, _devpod = run_prune(world, "-y", "--force") + out = capsys.readouterr().out + assert code == 0 + assert worktree.exists() + assert reported(out, worktree).endswith("workspace wt-ref still opens it") + + def test_an_orphaned_one_holding_work_is_still_kept(self, world, capsys): + worktree = self._worktree(world, "worktree-orphan", "wt-orphan") + (worktree / "scratch.md").write_text("an agent's notes\n") + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 0 + assert worktree.exists() + assert "uncommitted" in reported(out, worktree) diff --git a/test/unit/test_workspace_source.py b/test/unit/test_workspace_source.py index 1f277a93..17c10d7e 100644 --- a/test/unit/test_workspace_source.py +++ b/test/unit/test_workspace_source.py @@ -29,6 +29,7 @@ from devlaunch.dl import ( GitRepository, LocalFolder, + UnreadableLocalFolder, UnreadableWorkspaceList, UnrecognisedSource, Workspace, @@ -96,16 +97,38 @@ def test_a_key_devpod_left_empty_is_not_a_readable_source(self, source): assert _listed(source).source == UnrecognisedSource(source) @pytest.mark.parametrize("value", [{"str_of_a_dict": "is back"}, 7, ["/home/dev"], None]) - @pytest.mark.parametrize("key", ["localFolder", "gitRepository"]) - def test_a_key_whose_value_is_not_text_is_not_a_readable_source(self, key, value): + def test_a_git_url_that_is_not_text_is_not_a_readable_source(self, value): """Otherwise the sentinel comes back one level in. - `{"localFolder": {...}}` would build a `LocalFolder` whose `path` is a - dict -- the same dict-where-a-path-belongs this whole change removed -- - and the two readers that treat it as a path raise `TypeError` on it. - An arm's field is only honest if the value going into it is checked. + `{"gitRepository": {...}}` would build a `GitRepository` whose `url` is + a dict -- the same dict-where-a-path-belongs this whole change removed + -- and the readers that treat it as text raise `TypeError` on it. An + arm's field is only honest if the value going into it is checked. """ - assert _listed({key: value}).source == UnrecognisedSource({key: value}) + assert _listed({"gitRepository": value}).source == UnrecognisedSource( + {"gitRepository": value} + ) + + @pytest.mark.parametrize("value", [{"str_of_a_dict": "is back"}, 7, ["/home/dev"]]) + def test_a_folder_devpod_named_and_devlaunch_cannot_read_is_its_own_arm(self, value): + """The same rejection, and *not* the same answer. + + A `localFolder` this unreadable is still devpod saying the workspace + opens a directory on this machine. Sharing an arm with `image` and + `container` -- which open none -- made the two indistinguishable to the + one reader where the difference is a deletion: `dl --prune` treats "no + folder here" as nothing to compare and proceeds, which is right for an + image workspace and wrong for a folder it merely could not read. + """ + assert _listed({"localFolder": value}).source == UnreadableLocalFolder( + {"localFolder": value} + ) + + def test_a_folder_key_devpod_left_null_claims_no_folder_at_all(self): + """`null` is devpod saying there is no local folder, which is what an + image or container workspace has -- not a folder that could not be + read. The empty string is pinned the same way above.""" + assert _listed({"localFolder": None}).source == UnrecognisedSource({"localFolder": None}) def test_a_source_carrying_both_keys_is_read_as_a_folder(self): """Key precedence, pinned. Nothing obligates it: a reader that misses an From 67c576ed8ae9c9da9e1512b2581cbda319b1aa9e Mon Sep 17 00:00:00 2001 From: Austin Gregg-Smith Date: Fri, 14 Aug 2026 07:45:25 +0100 Subject: [PATCH 2/2] Ask the one resolver which directory a record names `--prune` reads metadata.json in two places -- to decide which directory a record disputes, and to decide which record describes nothing -- and both read `record.local_path` raw. A record gives *two* answers about where its clone is, the path written in it and the one derived from owner/repo/branch, and devlaunch#174 was those two answers disagreeing between the guard and the delete. `resolve_clone_path` is now the single place that settles it, and these are the last two callers that were not going through it. Here the consequence is the dangerous direction rather than the merely inconsistent one. A record that misses its clone leaves that clone with no record at all, which drops it out of `Disputed` and into `Orphaned` -- a deletion, with no `--force` and nothing said. The empty recorded path makes it concrete: `Path("")` is `Path(".")`, truthy and existing, so it canonicalised to whatever directory `dl` was run from and the record was filed under that. On the other reader a record pointing somewhere stale described "nothing" and was dropped, taking the only note of where a clone that is still on disk lives. Both were unheld -- the review's mutation table has the first as a surviving green -- and both now have a test that fails under exactly the mutation that reintroduces the fault, one asserting the clone is deleted and one asserting the record is dropped. `_is_populated_clone` stops asking `Path.exists()`. It is the third arm of the same question and it had no test: `exists()` swallows ENOENT, ENOTDIR, EBADF and ELOOP and re-raises the rest on 3.10-3.13 while 3.14 returns False, so one expression is two behaviours across the `ci` matrix. `os.stat` asks it once, a closed door reads as "not a populated clone" -- which disputes the whole repository and keeps its clones, the direction a refusal has actually established -- and the test for that arm is red under the mutation that reads it the other way and green under nothing else. README says out loud that clearing non-clone junk out of the cache now takes `--force`, which is what `main`'s unsaved-work sum makes true. --- README.md | 7 +- devlaunch/dl.py | 18 ++- test/unit/test_prune_orphaned_clones.py | 198 ++++++++++++++++++++++-- 3 files changed, 206 insertions(+), 17 deletions(-) diff --git a/README.md b/README.md index d98b6a1b..2bc133d6 100644 --- a/README.md +++ b/README.md @@ -450,8 +450,13 @@ the next clone of a repo fast), and never looks outside names the record to go and fix. `--force` does not move any of them. Note that *every* directory two levels under `/devlaunch/repos` is a -candidate — a stray directory somebody left there is an orphan like any other. +candidate — a stray directory somebody left there is looked at like any other. The cache is `dl`'s to manage; things that are not clones do not belong in it. +But `git` cannot say what a directory that is not a repository holds, and +"cannot say" is kept rather than removed, so clearing junk out of the cache +takes `--force`. That is the same refusal a clone with a half-written `.git` +gets, and deliberately so: telling the two apart would mean `--prune` forming +its own opinion about a directory `dl rm` already refuses on. ``` $ dl --prune diff --git a/devlaunch/dl.py b/devlaunch/dl.py index f57e459e..913c28db 100644 --- a/devlaunch/dl.py +++ b/devlaunch/dl.py @@ -1863,11 +1863,25 @@ def _is_populated_clone(path: pathlib.Path) -> bool: devpod record that still describes something from one the id-scheme change left behind -- a folder that is gone, or the config-only stub devpod reconstitutes from its cache, neither of which any clone can be matched to. + + `os.stat` rather than `Path.exists()`, for the reason + :func:`devlaunch.worktree.workspace_clone._on_disk` gives at length: + `exists()` swallows ENOENT, ENOTDIR, EBADF and ELOOP and re-raises the rest + on Python 3.10-3.13 while 3.14 returns False, so an expression that looks + like one question is two behaviours across the versions in the `ci` matrix. + + A door this process cannot open reads as **not** a populated clone, and that + is the safe direction rather than the tidy one. Answering "yes" would say + devpod's workspace is at *this* clone and nowhere else, which leaves the + repository's other clones prunable; answering "no" says which clone of the + repository the workspace wants cannot be established, which disputes all of + them and keeps them. The second is what a refusal has actually established. """ try: - return (path / ".git").exists() - except OSError: + os.stat(path / ".git") + except (OSError, ValueError): return False + return True def _subdirectories(path: pathlib.Path) -> List[pathlib.Path]: diff --git a/test/unit/test_prune_orphaned_clones.py b/test/unit/test_prune_orphaned_clones.py index ae300ff0..a0f1ffca 100644 --- a/test/unit/test_prune_orphaned_clones.py +++ b/test/unit/test_prune_orphaned_clones.py @@ -40,6 +40,7 @@ import pytest from devlaunch.dl import main +from devlaunch.workspace_id import WorkspaceId from devlaunch.worktree.models import WorktreeInfo from devlaunch.worktree.storage import MetadataStorage from devlaunch.xdg import devlaunch_cache @@ -70,8 +71,10 @@ class World: every one of them was made from. """ - def __init__(self, cache: Path): + def __init__(self, cache: Path, tmp_path: Path): self.cache = cache + self.tmp_path = tmp_path + self.origin = tmp_path / "origin.git" self.repo_dir = cache / "repos" / "o" / "r" self.bare = self.repo_dir / ".bare" self.clones: Dict[str, Path] = {} @@ -86,6 +89,31 @@ def record_branches(self) -> List[str]: """The branches metadata.json still holds a worktree record for.""" return sorted(record.branch for record in self.storage.list_worktrees()) + def spare_clone(self, path: Path, branch: str) -> Path: + """One more real clone, fully pushed, that `--prune` may remove. + + A test that needs *something removable* has to build a repository git + can answer about, and cannot use a bare directory with a file in it as a + stand-in. Since devlaunch#171 a directory git cannot read as a + repository is a :class:`CouldNotTell` rather than "holds nothing", so a + plain directory is now the arm that *refuses* -- which is the right + answer for it and the wrong fixture for a deletion test. Written the + other way round, three of the tests below asserted a refusal path that + nothing ever reached. + + Made the way the fixture's own four clones are, from the same bare and + pushed to the same origin, so the git objects hardlink out of the bare + and `git log --not --remotes` really has nothing to report. + """ + path.parent.mkdir(parents=True, exist_ok=True) + git("clone", str(self.bare), str(path), cwd=self.tmp_path) + git("remote", "set-url", "origin", str(self.origin), cwd=path) + git("checkout", "-b", branch, cwd=path) + (path / f"{branch}.txt").write_text("work\n") + commit(path, branch) + git("push", "-u", "origin", branch, cwd=path) + return path + def _entry(workspace_id: str, source) -> dict: """One `devpod list --output json` element.""" @@ -154,10 +182,10 @@ def world(tmp_path) -> World: them. `disputed` has a metadata record naming a workspace devpod still lists but sources somewhere else entirely, which is devlaunch#88's shape. """ - world = World(devlaunch_cache()) + world = World(devlaunch_cache(), tmp_path) world.repo_dir.mkdir(parents=True) - origin = tmp_path / "origin.git" + origin = world.origin seed = tmp_path / "seed" git("init", "-b", "main", str(seed), cwd=tmp_path) (seed / "README.md").write_text("seed\n") @@ -536,6 +564,83 @@ def test_a_record_whose_directory_cannot_be_looked_at_is_kept(self, world, capsy sealed.chmod(0o700) +class TestWhichDirectoryARecordNames: + """One resolver answers it, and asking a second way is a deletion. + + A record gives *two* answers about where its clone is -- the path written in + it, and the one derived from its owner/repo/branch -- and devlaunch#174 was + those two answers disagreeing between the guard and the delete. + :meth:`WorkspaceCloneManager.resolve_clone_path` is the single place that + settles it, and `--prune` reads records in two places: to decide which + directory a record disputes, and to decide which record describes nothing. + Both go through it. + """ + + @staticmethod + def _derived(world: World, branch: str) -> Path: + """Where dl would put `o/r@branch`'s clone if no path were recorded.""" + return world.repo_dir / WorkspaceId("o", "r", branch).value + + def test_a_disputed_clone_whose_record_lost_its_path_is_still_disputed(self, world, capsys): + """The unheld guard, held. A record whose `local_path` is empty -- + hand-edited, or truncated on a metadata write -- still names its clone + perfectly well through owner/repo/branch, and the clone it names is one + devpod disputes. + + Read raw, `Path("")` is `Path(".")`: truthy, existing, and canonicalising + to whatever directory `dl` was run from. The record is filed under that, + the real clone is left with no record at all, and a clone that should be + `Disputed` falls through to `Orphaned` -- which is a deletion, with no + `--force` and nothing said. + """ + clone = world.spare_clone(self._derived(world, "lost"), "lost") + world.storage.add_worktree( + WorktreeInfo( + owner="o", + repo="r", + branch="lost", + local_path=Path(""), + workspace_id="a-live-one", + ) + ) + # devpod still lists that workspace, and sources it somewhere else. + world.listed.append(_entry("a-live-one", world.tmp_path / "somewhere" / "else")) + code, _devpod = run_prune(world, "-y", "--force") + out = capsys.readouterr().out + assert code == 0 + assert clone.exists() + assert "devlaunch#88" in reported(out, clone) + + def test_a_record_whose_clone_is_at_the_derived_path_is_not_called_gone(self, world, capsys): + """The same resolver, on the other reader. + + A record left pointing somewhere stale is devlaunch#174's own shape -- + the recorded path is not on disk and the clone is at the derived one, + which is exactly the case `resolve_clone_path` exists to settle. Read + raw, the record describes nothing and gets dropped, and the note of + where a clone that is *right there* lives goes with it. `--prune` is the + only thing that has ever removed a record, so there is nothing to put it + back. + """ + derived = self._derived(world, "lost") + derived.mkdir() + (derived / "work.txt").write_text("still here\n") + world.storage.add_worktree( + WorktreeInfo( + owner="o", + repo="r", + branch="lost", + # Where the clone used to be, before an id scheme moved it. + local_path=world.repo_dir / "r-lost-oldscheme", + workspace_id="a-live-one", + ) + ) + run_prune(world, "-y") + capsys.readouterr() + assert derived.exists() + assert "lost" in world.record_branches() + + class TestACacheReachedThroughASymlink: """The total-loss regression, and the reason it gets a test of its own.""" @@ -604,10 +709,20 @@ class TestWhenADirectoryWillNotComeAway: @staticmethod def _seal(world: World) -> Path: - """A second orphan with a door in it this process cannot open.""" - leftover = world.repo_dir / "leftover" - (leftover / "locked").mkdir(parents=True) + """A second orphan with a door in it this process cannot open. + + A real clone rather than a bare directory, and the difference is whether + these tests reach the refusal path at all: git answers "nothing to lose" + about a repository whose subdirectory it cannot open (a warning on + stderr, rc 0, no output -- executed against git 2.55.0), so the clone is + classified removable and `remove_tree` gets to refuse. A plain directory + is a :class:`CouldNotTell` since devlaunch#171 and is *kept*, so nothing + would ever be removed and there would be no refusal to report. + """ + leftover = world.spare_clone(world.repo_dir / "leftover", "leftover") + (leftover / "locked").mkdir() (leftover / "locked" / "payload.bin").write_bytes(b"\0" * 512 * 1024) + (leftover / ".git" / "info" / "exclude").write_text("locked/\n") (leftover / "locked").chmod(0o000) return leftover @@ -811,9 +926,7 @@ def test_only_that_repositorys_clones_are_disputed(self, world, capsys): which is what keeps this command useful on #88's host rather than merely safe on it: seven repositories were affected there and the rest of the cache is still somebody's dead disk.""" - elsewhere = world.cache / "repos" / "other" / "project" / "stale" - elsewhere.mkdir(parents=True) - (elsewhere / "payload.bin").write_bytes(b"\0" * 1024) + elsewhere = world.spare_clone(world.cache / "repos" / "other" / "project" / "stale", "old") world.listed[0] = _entry("r-main", world.repo_dir / "main") code, _devpod = run_prune(world, "-y") capsys.readouterr() @@ -821,6 +934,35 @@ def test_only_that_repositorys_clones_are_disputed(self, world, capsys): assert not elsewhere.exists() assert world.clones["referenced"].exists() + @needs_an_unprivileged_user + def test_a_source_directory_dl_may_not_look_into_disputes_the_repository(self, world, capsys): + """A closed door is not an answer, and the arm that says so had no test. + + `_is_populated_clone` asks whether devpod's recorded source is a real + checkout, and it can be refused rather than answered -- a clone a + container made mode 000, which is the machine this command exists for. + Reading the refusal as "yes, a populated clone" says the live workspace + is at this one directory and nowhere else, which leaves the + repository's *other* clones prunable while a workspace dl could not + inspect is open on the repository. Reading it as "no" disputes the + repository, which is what a refusal has actually established. + + Asserted on the reason rather than on survival: every clone here also + carries a record, so one that stopped being disputed for this reason + would still be kept for another and the assertion would not notice. + """ + sealed = world.clones["referenced"] + world.listed[0] = _entry("r-main", sealed) + sealed.chmod(0o000) + try: + code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + finally: + sealed.chmod(0o755) + assert code == 0 + assert world.clones["orphan-clean"].exists() + assert "devlaunch#88" in reported(out, world.clones["orphan-clean"]) + class TestAWorkspaceThatAppearedWhileTheQuestionWasOpen: """The plan is a photograph; the act happens later, and the world moved. @@ -979,15 +1121,43 @@ def test_force_is_what_removes_it(self, world, capsys): assert code == 0 assert not clone.exists() - def test_a_directory_that_was_never_a_repository_is_still_removed(self, world, capsys): - """The distinction the arm rests on. Left-behind notes under the cache - really do hold nothing git could lose; a repository that will not open - does not, and the two must not be told apart by asking git, which - refuses identically for both.""" + def test_a_directory_that_was_never_a_repository_is_kept_and_named(self, world, capsys): + """A directory git cannot read as a repository gets the same answer as a + repository that will not open, and this file used to assert the + opposite. + + The earlier reading was that left-behind notes under the cache "really + do hold nothing git could lose", and told the two apart with a `.git` + probe of `--prune`'s own. devlaunch#171 settled it the other way and + settled it in the resolver every caller shares: nothing has established + that `plan.md` exists anywhere else, and a probe that answers "empty" + for a directory dl never looked inside is the same sentinel one layer + out. So the stray directory is kept, it is named with why, and `--force` + is the sentence that removes it -- which is the arm `dl rm` already + refuses on and the answer this command should not have had a second + opinion about. + + The cost is real and is the right way round: a cache with junk in it + needs `--force` to clear the junk. The alternative costs an hour of + somebody's work on a clone whose `.git` a container half-wrote. + """ stray = world.repo_dir / "notes-i-left-here" stray.mkdir() (stray / "plan.md").write_text("half a plan\n") code, _devpod = run_prune(world, "-y") + out = capsys.readouterr().out + assert code == 0 + assert (stray / "plan.md").exists() + line = reported(out, stray) + assert "could not" in line and "--force" in line, line + + def test_force_removes_a_directory_that_was_never_a_repository_too(self, world, capsys): + """The other half: kept is not unremovable, and a cache full of stray + directories is still clearable in one command.""" + stray = world.repo_dir / "notes-i-left-here" + stray.mkdir() + (stray / "plan.md").write_text("half a plan\n") + code, _devpod = run_prune(world, "-y", "--force") capsys.readouterr() assert code == 0 assert not stray.exists()