holds_unsaved_work can answer about an ancestor repository and report a dirty clone as safe to delete

Type: build
Map: #139

holds_unsaved_work — the one judgement dl makes before destroying a directory — can answer about a different repository and report "nothing would be lost" while the clone holds unsaved work. This is live in merged main (39d4d2a), not only in the unmerged --prune work.

The mechanism

workspace_state._git runs git … cwd=repo with no --git-dir, no --work-tree, and no ceiling. Git's repository discovery therefore walks up the parent chain. When the clone directory exists but its .git is unusable — broken, truncated, half-removed by an interrupted delete — git does not refuse. It finds an ancestor repository and confidently answers about that one.

read_clone's docstring says "a directory that is not there, or is not a repository, holds nothing", but the code never asks whether it is a repository; it asks git a question and trusts the reply. So the "cannot tell" path that _git's docstring carefully describes — "a refusal is 'cannot tell', never an answer … must never be reported as safe to delete on the strength of a failed command" — is unreachable here, because the command succeeds.

Reproduction (executed against `origin/main:devlaunch/workspace_state.py`)

An ancestor repo that is clean and fully pushed (the dotfiles-in-$HOME case, which is common), with a workspace clone nested under it whose .git is garbage and which holds an untracked scratch file:

ancestor repo: clean and fully pushed  (git status --porcelain and
                                        git log main --not --remotes both empty)

read_clone         -> CloneState(branch='main', unsaved=None)
holds_unsaved_work -> None

TRUTH: clone dir holds ['.git', 'scratch.md']   <- scratch.md would be destroyed

branch='main' is the ancestor's branch, not the clone's. With unsaved=None, dl <ws> rm does not refuse and does not require --force.

Note the near-miss that hides this: if the ancestor has any unpushed commit or dirty file, the guard fires — for entirely the wrong reason, describing the wrong repository — and the delete is refused. So the failure only shows itself on a tidy host, which is the host most likely to be trusted.

Independently found from the coverage side during review of PR #170: two of the three CouldNotAsk producers in the --prune work (workspace_state.py:242, :253) are uncovered, and mutating each to report "clean" leaves the full suite green.

What the fix has to achieve

Discovery cannot escape the clone. Ask git explicitly about this directory — --git-dir/--work-tree, or GIT_CEILING_DIRECTORIES — so an unusable .git produces a refusal rather than an answer about somebody else's repository.
A refusal must be representable and must fail safe. Today Optional[str] conflates "nothing to lose" with "could not tell": both are None, and holds_unsaved_work returning None means delete freely. That is the sentinel principle 3 forbids, and it is load-bearing for a destructive operation. A total sum — nothing-to-lose / would-lose(description) / could-not-tell — makes the dangerous conflation unrepresentable, and could-not-tell must refuse the delete exactly as would-lose does.
Guards proven by mutation, not by green. The existing tests pass with the bug present. Each new guard must be shown red under a mutation that reintroduces the fault.

Blast radius

holds_unsaved_work is the guard for dl <ws> rm; --purge and the unmerged --prune (#159) stand on the same reading. #159 is parked on this finding — but this ticket is deliberately not gated behind that PR, because the bug ships today.

Discovered by review of PR #170; parked handoff on #159.

breadcrumb: built test-first in PR #173.

Reproduced the ancestor walk against origin/main first (clean, fully pushed ancestor → read_clone -> CloneState(branch='main', unsaved=None) while the clone held scratch.md), then wrote the failing test, then fixed it.

Discovery cannot escape the clone: every git call now passes --git-dir=<clone>/.git --work-tree=<clone>, which switches discovery off rather than bounding it. Verified against real git 2.55.0 on five broken-clone shapes (garbage .git, empty .git, HEAD-only .git, real clone with .git/objects deleted, truncated gitfile) — all four of the first answered about the ancestor before and refuse now; a healthy clone and a linked worktree (gitfile .git) still answer normally. GIT_CEILING_DIRECTORIES was tried and rejected: it fails open back to the ancestor when its path does not match what git resolved, where --git-dir fails closed.
The refusal is representable: Unsaved = NothingToLose | WouldLose(what) | CouldNotTell(why), every caller names its arm, and could-not-tell refuses rm exactly as would-lose does. The same sentinel one layer down went too — if status: was reading a refused git status as a clean tree, so _git now returns GitSaid/GitRefused.
Callers: dl <ws> rm, dl --ls --json (breaking: unsaved is now an object with one key naming the arm), and _unsaved_work_in, where an unreadable record was the same conflation one level up. --purge was checked and does not consult this reading — purge_all_data deletes on the ownership predicate after confirmation. The docstring claim that it does is inherited and wrong; dl --prune: remove clone directories no workspace references #159's --prune does read it and gets the new arm when it rebases.
7 mutations, each re-running the full CI selection (no -k; baseline 1265 passed): discovery restored → 4 failed; refused status read as clean → 7 failed; refused git log swallowed → 1 failed; guard drops the CouldNotTell arm → 2 failed; unreadable record → 1 failed; JSON renders could-not-tell as nothing-to-lose → 2 failed; absent directory over-refused → 3 failed. All red.

workspace_state.py is at 100% statement coverage — the two uncovered CouldNotTell producers were the shape flagged in review of #170, so they were covered rather than left. pixi run ci and pixi run -e py310 ci both green (pylint 10.00/10, ty clean). ./dev.sh was not run: it rebinds a host-wide dl-next symlink that concurrent work shares.

breadcrumb — correction to this ticket. Its Blast-radius paragraph says the bug ships in dl <ws> rm and --purge. The second half is wrong, and I wrote it. Verified by execution on both merged main and the fix branch: instrumenting holds_unsaved_work and running main(["--purge","-y"]) produces zero calls — purge_all_data uses workspace_ownership + remove_tree only. --purge is unguarded against unsaved work by design (#131: one y/N and the cache goes), so this is a documentation error rather than a second hole. The real consumers are dl <ws> rm, dl --ls --json, and the unmerged --prune (#159).

I inherited "almost certainly" from a reviewer and passed it on as fact without checking. The bug itself was reproduced by execution; its reach was not.

Say null once, pin the work tree, and stop overstating the evidence

breadcrumb — two-axis review of b92c67b addressed in PR #173, head 88c1b4c. Both axes said the fix itself is sound; nothing in it was rewritten. Four bounded items, each shown red under a mutation re-running the full CI selection (pixi run ci, never -k).

S1 — null still meant two things at the JSON surface. dl --ls --json gated unsaved on the metadata record while its neighbour disk gated on _measurable_clone — the clone directory. A clone under the cache with no record printed devlaunch: true with a measured disk beside unsaved: null, and null is documented as no clone of dl's here to inspect. That is this ticket's own sentinel surviving one layer out, and the same divergence PR #165 closed for disk. Fixed rather than documented away: unsaved, path and checkedOut fall back to _measurable_clone when there is no record, so unsaved: null and devlaunch: false are the same set by construction (is_devlaunch_clone true ⇒ LocalFolder source ⇒ _measurable_clone returns a path). The record is still preferred when there is one, because that is the directory dl <ws> rm's guard reads and the listing must not name a third. README and CHANGELOG said "null only where repo and branch are"; both now say "null exactly where devlaunch is false".

--work-tree was load-bearing and unguarded. Dropping it while keeping --git-dir left the whole suite green — reviewer's finding, reproduced here from the other side as MB below failing exactly one test, the new one. It is not redundant: with core.worktree set to another directory in the clone's own config, real git 2.55.0 gives git --git-dir=<clone>/.git status --porcelain → empty at rc 0 on a clone holding an untracked file, i.e. "nothing to lose" about a directory nobody asked about. Its neighbour core.bare = true fails closed (fatal: this operation must be run in a work tree), which is why only the core.worktree shape demonstrates the flag matters. Both executed here; a test now builds the core.worktree shape and goes red without the flag.

Two docstrings overstated their own evidence, and both are corrected. _git said "all five refuse here; all five answered about the ancestor under plain cwd=" — the second clause is false. Re-executed on six truncated/invalid gitfile variants: all six rc=128 under plain cwd= as well, because git treats an unreadable gitfile as a hard error rather than continuing discovery upward. That shape was never part of the bug. The PR body's table always said this; the prose and the docstring did not, and now both say four of five. CloneState asserted branch is None "in every case where unsaved is a CouldNotTell" — falsified by executing the shape its own new test builds: CloneState(branch='feature', unsaved=CouldNotTell('… bad object origin/bogus')). The behaviour is right; the invariant was written in the same commit as the test disproving it, and is gone.

S4 (pre-existing) fixed, and it was worse than reported. read_clone asked Path.is_dir(), which cannot express "I was not allowed to look" — and gives a different wrong answer per Python, both confirmed by execution on this machine: 3.10.20 raises PermissionError (so dl <ws> rm fails closed by crashing and dl --ls --json becomes a traceback for the whole listing because of one workspace), 3.14.6 returns False, which reads as "not there, so nothing to lose" — a clone that may be full of work, reported as free to delete. Those are the two Pythons CI runs. The errno is now read directly: ENOENT/ENOTDIR mean no clone there, everything else is a CouldNotTell.

S3 left, with a reason rather than by omission. An empty leftover directory still needs --force. Changing it would widen the delete-freely set inside the fix for a fail-open bug, on a shape that fails safe today and whose message names --force. That deserves its own change with its own red test.

S2 not widened — the guard and the delete naming different directories is pre-existing, a different mechanism, and is #174.

The --purge correction re-verified on the new head, not taken on trust. Instrumented holds_unsaved_work and ran main(["--purge","-y"]) against a cache holding a broken clone with an untracked file: 0 calls, exit 0, clone gone. purge_all_data deletes on the ownership predicate after one y/N, by design (#131). This ticket and map #139 were already corrected; nothing in the PR changes --purge.

Mutation evidence, each re-running the full pixi run ci (baseline on the new head: 1271 passed / 23 deselected, exit 0):

#	Mutation	Result
MA	`--ls --json`'s `unsaved`/`path`/`checkedOut` gate on the metadata record again	2 failed, 1269 passed — exit 1
MB	`_git` drops `--work-tree`, keeps `--git-dir`	1 failed, 1270 passed — exit 1
MC	`read_clone` goes back to `Path.is_dir()`	2 failed, 1269 passed — exit 1

MB failing exactly one test — the new one — is the reviewer's "survives the full suite green" reproduced from the other side.

pixi run ci and pixi run -e py310 ci both exit 0 on the new head. ./dev.sh was not run, nothing touched the real ~/.cache/devlaunch, and no devpod workspace, container or clone on this machine was deleted.

breadcrumb: PR #173, head 395e527 (was 88c1b4c). All four Standards items closed; no change to what the fix does.

1. The boundary is 3.14, not 3.13 — and the leg it lives on is now in CI. Re-executed independently on this repo's own environments, Path.is_dir() on a clone behind a mode-000 parent: 3.10.20, 3.11.15, 3.12.13, 3.13.14 raise PermissionError; 3.14.6 returns False. Fixed in workspace_state.py, test_workspace_state.py and CHANGELOG. Patch levels between those were not run and the text says so.

.github/workflows/ci.yml now runs ci on default (3.14.6) as well as py310–py313. default rather than a new py314 feature: no lockfile change, and it tracks whatever the newest solvable Python is. Executed proof the leg is load-bearing — with present = clone.is_dir() put back inside the existing try: on 3.14 3 failed / 1273 passed, on 3.10 1 failed / 1275 passed. Both exit 1. What differs is which tests: the two closed-door tests are red on 3.14 and green on 3.10, because on 3.10 is_dir() re-raises into the except (OSError, ValueError) arm — while 3.10's one failure is the NUL-path test added in this same push, since CPython 3.10's Path.is_dir() catches ValueError and returns False. So ci (py310) would have been red and that mutation would not have merged.

Corrected 2026-08-10 in dc9c9c1. This paragraph originally read "green on 3.10 … that mutation would have merged", which its own numbers contradict; both review axes measured py310 exit 1. The narrow claim above — closed-door tests red on 3.14, green on 3.10 — is what was verified.

2. The --work-tree test now pins the safety property. Re-executed at git 2.55.0 on the exact fixture: with the other work tree empty, --git-dir alone prints " D README.md\n D feature.txt" at rc 0 — a WouldLose about the wrong files, a refusal. The fail-open needs the other tree to mirror HEAD, and then it prints nothing at rc 0: NothingToLose() on a clone holding an hour of work. The test builds the mirrored shape, so the mutation now fails on isinstance(..., WouldLose) (AssertionError: NothingToLose()); a second test pins that a clean clone is not reported dirty by the other tree's absences. Both go red: 2 failed / 1274 passed. Both docstrings corrected to state the two outcomes separately.

3. "Same set by construction" is now actually by construction, and the mutation is red. clone_path = _measurable_clone(...), mine = clone_path is not None, record overwrites only when there is one. The input that broke the two-gate form has a test — a record that outlives the reason its workspace counts as dl's (repos_dir moved outside the cache). Dropping if mine else None: 1 failed / 1275 passed, and it is that test.

4. README.md corrected. A dl-owned clone with no record: the listing reports wouldLose, rm exits 0 and orphans the clone. README now says that instead of "refuses". Pinned by a test, and the test double's remove_workspace_by_id now teaches both halves of the real one instead of returning a truthy Mock — reverting it to the MagicMock default is 1 failed / 1275 passed. Not fixed in code: the message would also fire for every dl ./path workspace, and the choice belongs with #174.

NITs: except (OSError, ValueError) taken — a NUL byte in a recorded path is a ValueError before the syscall and took the whole listing down for one bad record; test red on 3.10 and 3.14 alike (1 failed / 1275 passed). Path("") not taken: remove_workspace_by_id already falls back to the derived path for an empty local_path, so the guard would read dl's cwd while the delete removed a third directory — that is #174's family, and fixing half of it here would put the refusal and the listing on two different directories.

Green: pixi run ci exit 0, 1276 passed / 23 deselected, on all five environments the matrix now runs (py310, py311, py312, py313, default 3.14.6). pixi run prek exit 0. workspace_state.py 100% on 3.10 and 3.14.

One process note, because it nearly cost a row. Restores between mutations were sha256-verified snapshot copies, never git checkout. This scratchpad is shared with other sessions, and one of them overwrote a generically-named restore.sh mid-run — so one mutation was executed with the previous mutation still applied. The checksum check caught it, that row was discarded and re-run from a verified snapshot, and the script and snapshot now carry unique names with the snapshot verified before every restore. Worth knowing for anyone else running a mutation harness out of this directory.

Not widened to #174 (guard and delete naming different directories) or #159's rebase hazard.

breadcrumb: PR #173, head dc9c9c1 (was 395e527). Text only — no behaviour changed, and the review that asked for these said so explicitly ("No code defect found").

1. Deleted the causal claim that the missing 3.14 leg is why the boundary was wrong. It is false, and it was falsified by execution: Standards checked out 88c1b4c — the revision carrying the "3.13+" text — and ran pixi run ci on 3.14.6: 1271 passed, exit 0, green. A ci (default) leg would have been green on the wrong boundary, because these tests assert version-independent behaviour and none of them calls is_dir(). What caught it was a reviewer asking for it to be run. Removed from workspace_state.py, test_workspace_state.py, ci.yml and the CHANGELOG paragraph that chained the same "so". The leg stays — it catches the reversion, which is the reason ci.yml already gave a paragraph earlier.

2. Corrected the MD mutation row. It read "green on 3.10 … so that mutation would have merged". Both axes measured py310: exit 1, 1 failed — test_a_recorded_path_that_is_not_a_path_at_all_is_could_not_tell, the NUL-byte test added in that same push, because CPython 3.10's Path.is_dir() catches ValueError and returns False. ci (py310) would have been red; it would not have merged. True of 88c1b4c, false of 395e527. The narrow claim now stated everywhere — PR body, table, and the corrected comment above: the two closed-door tests are red on 3.14 and green on 3.10.

3. read_clone's ValueError paragraph had its referents inverted — "total over the first and not over the second" said the opposite of the truth and of its own next clause. It now names OSError and ValueError instead of counting.

4. ci.yml cited a CONTRIBUTING file that does not exist. There is no CONTRIBUTING* in the repo; the instruction is in README's Development section, which is what it now cites.

The surviving green mutation now has a test. Skipping read_clone for a clone directory that is genuinely absent — a plausible "don't shell out to git for a directory that isn't there" — was green and produced {"devlaunch": true, "path": "<dir>", "unsaved": null}. TestTheJsonListing gained a row: an absent directory is NothingToLose(), which is what lets rm clear away a workspace whose clone was removed by hand, so the listing has to say so too. Under that mutation, executed on this head: 1 failed / 1276 passed, exit 1, and the failing line is the unsaved assertion (assert None == {'nothingToLose': True}), not the path one.

Other NITs taken: README's rm lead now says the recorded clone, which is the directory the guard reads (the paragraph 18 lines below already corrected it); _measurable_clone's docstring describes all four fields it now answers rather than only --size; the orphaned line-wrap in ci.yml is closed; and ci.yml records that default is the only unpinned leg, so pixi update moves it unreviewed and the matrix loses 3.14 the day 3.15 solves — frozen: true keeps each commit reproducible, so it is a lockfile-review note, not a hole.

Gate on this head, both ends of the boundary: pixi run ci (default, 3.14.6) → exit 0, 1277 passed / 23 deselected; pixi run -e py310 ci (3.10.20) → exit 0, 1277 passed / 23 deselected. ruff / pylint 10.00 / ty clean on each. workspace_state.py 100% on both.

On GitHub: all eleven checks pass on dc9c9c1, including all five ci legs and gate.

Not executed locally on this head, and said so: py311/py312/py313 (green on GitHub for this head; last local run was 395e527); Standards' green run at 88c1b4c, attributed to them above and not reproduced here; every mutation row except the new one. Snapshot restores were sha256-verified and uniquely named, per the note above. ./dev.sh was not run and no devpod workspace, container or clone was deleted.

The pattern is worth naming, since this is the fourth round of it. Items 1 and 2 were both introduced by the round that fixed the previous instance — a sentence written one step past what was executed, each time in a commit about not doing that. This round the check applied to every "because", "only", "would have" and "cannot" was: was the thing it asserts actually run, and by whom. Where it was not, the narrower claim is written, and the attributions above are part of that.

Merge pull request #173 from blooop/fix/171-clone-guard-ancestor

Navigation Menu

Repository navigation

holds_unsaved_work can answer about an ancestor repository and report a dirty clone as safe to delete #171

The mechanism

Reproduction (executed against `origin/main:devlaunch/workspace_state.py`)

What the fix has to achieve

Blast radius

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

holds_unsaved_work can answer about an ancestor repository and report a dirty clone as safe to delete #171

Description

The mechanism

Reproduction (executed against origin/main:devlaunch/workspace_state.py)

What the fix has to achieve

Blast radius

Activity

blooop commented on Aug 10, 2026

blooop commented on Aug 10, 2026

blooop commented on Aug 10, 2026

blooop commented on Aug 10, 2026

blooop commented on Aug 10, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

Reproduction (executed against `origin/main:devlaunch/workspace_state.py`)