You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
dl <ws> rm can ask "is there unsaved work?" about one directory and then destroy a different one. Found during the two-axis review of PR #173 and reproduced by execution against the real clone manager. Pre-existing on merged main, not introduced by that PR.
The mechanism
The guard reads record.local_path (devlaunch/dl.py:463). The delete does not: remove_workspace_by_id (devlaunch/worktree/workspace_clone.py:448-468) falls back to the derived clone path when local_path is not on disk, and shutil.rmtrees that instead.
So when a record's local_path points somewhere that no longer exists — a moved cache, a half-finished migration, a hand-edited record, exactly the states #88 documents — the guard truthfully reports "nothing to lose" about the absent path, and the delete then removes the live derived clone.
Reproduction (executed, real clone manager and storage)
record local_path: /…/moved-away (absent) -> guard saw NothingToLose()
dl r-feature-aaa rm -> EXIT 0
Removed workspace clone: /…/repos/blooop/r/r-feature-piheniho
derived clone still on disk: False
work file still on disk: False
Unsaved work destroyed, exit 0, no --force, no refusal.
#171 is the same class — the guard answers about a directory other than the one that dies — but a different mechanism. There, git's discovery walked up to an ancestor repository; the fix constrains discovery so it cannot escape the clone. Here, discovery is fine: the guard and the delete are simply given different paths, and both answer correctly about the path they were given.
PR #173 does not introduce this and does not make it worse. It does, however, re-affirm and pin the NothingToLose-for-an-absent-directory arm that this defect leans on — correctly, because "a directory that is not there holds nothing" is true, and the same arm is what lets rm clear away a clone someone removed by hand without --force. The defect is not that arm; it is that the guard and the delete do not agree on which directory is at stake.
What the fix has to achieve
One path, decided once. The guard and the delete must be handed the same resolved directory, so a disagreement is impossible rather than merely unlikely. A caller that resolves the path and passes it to both is the obvious shape; deriving it twice is what created this.
Do not regress the hand-removed case.dl <ws> rm must still clear a workspace whose clone is genuinely gone, exit 0, without --force. That is pinned by test/test_workspace_state.py:660 and by Ask git about the clone, not whichever repo it wandered into (#171) #173's over-refusal mutation; keep both green.
Test-first
Guards must be shown red under a mutation that reintroduces the fault, not merely green — this repo has shipped hollow guards repeatedly (#145's leaked module memo made a pinned regression permanently green; review of #170 found two CouldNotAsk producers that stayed green when mutated to report "clean"). Run the full CI selection, never a filtered -k run.
At minimum: the reproduction above, asserting the derived clone and its unsaved file survive and the command refuses; the hand-removed case still exits 0 without --force; and whatever new arm the fix introduces is exhaustively matched (ty) at every consumer, as #173 established for Unsaved.
Related: #171 (same class, different mechanism), #88 (the record/reality divergence that produces the triggering state), #159 (--prune, parked, reads the same guard).
Type: build
Map: #139
dl <ws> rmcan ask "is there unsaved work?" about one directory and then destroy a different one. Found during the two-axis review of PR #173 and reproduced by execution against the real clone manager. Pre-existing on mergedmain, not introduced by that PR.The mechanism
The guard reads
record.local_path(devlaunch/dl.py:463). The delete does not:remove_workspace_by_id(devlaunch/worktree/workspace_clone.py:448-468) falls back to the derived clone path whenlocal_pathis not on disk, andshutil.rmtrees that instead.So when a record's
local_pathpoints somewhere that no longer exists — a moved cache, a half-finished migration, a hand-edited record, exactly the states #88 documents — the guard truthfully reports "nothing to lose" about the absent path, and the delete then removes the live derived clone.Reproduction (executed, real clone manager and storage)
Unsaved work destroyed, exit 0, no
--force, no refusal.Why this is not #171
#171 is the same class — the guard answers about a directory other than the one that dies — but a different mechanism. There, git's discovery walked up to an ancestor repository; the fix constrains discovery so it cannot escape the clone. Here, discovery is fine: the guard and the delete are simply given different paths, and both answer correctly about the path they were given.
PR #173 does not introduce this and does not make it worse. It does, however, re-affirm and pin the
NothingToLose-for-an-absent-directory arm that this defect leans on — correctly, because "a directory that is not there holds nothing" is true, and the same arm is what letsrmclear away a clone someone removed by hand without--force. The defect is not that arm; it is that the guard and the delete do not agree on which directory is at stake.What the fix has to achieve
local_pathand the derived path can both exist and differ, that is a "cannot tell which" and must fail closed the way holds_unsaved_work can answer about an ancestor repository and report a dirty clone as safe to delete #171'sCouldNotTelldoes — never a silent choice of one.dl <ws> rmmust still clear a workspace whose clone is genuinely gone, exit 0, without--force. That is pinned bytest/test_workspace_state.py:660and by Ask git about the clone, not whichever repo it wandered into (#171) #173's over-refusal mutation; keep both green.Test-first
Guards must be shown red under a mutation that reintroduces the fault, not merely green — this repo has shipped hollow guards repeatedly (#145's leaked module memo made a pinned regression permanently green; review of #170 found two
CouldNotAskproducers that stayed green when mutated to report "clean"). Run the full CI selection, never a filtered-krun.At minimum: the reproduction above, asserting the derived clone and its unsaved file survive and the command refuses; the hand-removed case still exits 0 without
--force; and whatever new arm the fix introduces is exhaustively matched (ty) at every consumer, as #173 established forUnsaved.Related: #171 (same class, different mechanism), #88 (the record/reality divergence that produces the triggering state), #159 (
--prune, parked, reads the same guard).