Skip to content

The unsaved-work guard and the delete can name different directories #174

Description

@blooop

Type: build
Map: #139

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.

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 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

  1. 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.
  2. A disagreement, if it can still occur, must refuse. If local_path and 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's CouldNotTell does — never a silent choice of one.
  3. 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).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions