Fold the removal guard into one workspace_remove and swap the api row - #520
Merged
Conversation
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Reviewer's GuideThis PR makes workspace removal safe by construction: core now owns one guarded Sequence diagram for guarded workspace removalsequenceDiagram
participant Caller
participant Core as workspace_remove
participant Git
participant Guard
participant Devpod
participant Clone
Caller->>Core: workspace_remove(..., removal, ...)
alt Removal::Insisted
Core->>Devpod: workspace_delete
else Removal::Guarded or Removal::Wedged
Core->>Git: unsaved_work_in
Git-->>Core: unsaved work finding
Core->>Guard: guard_removal
alt Removal::Guarded and work would be lost
Guard-->>Core: RemoveOutcome::Refused
Core-->>Caller: Refused(RemovalRefused)
else Removal::Wedged and work would be lost
Core-->>Caller: RemovingOverWork notice
Core->>Devpod: workspace_delete
else No refusal
Core->>Devpod: workspace_delete
end
end
Core->>Devpod: name volumes
Core->>Devpod: devpod delete
Devpod-->>Core: delete result
Core->>Clone: remove clone
Core-->>Caller: RemoveOutcome
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
`devlaunch_core::api` promised `workspace_delete`, which is the delete without the unsaved-work guard. The probe and the guard that make it safe were two further exported functions a caller had to run first, in the right order, and the sequence lived in the `dl` binary: nothing outside `dl` could reuse it and nothing held `dl` to it. A consumer following the promise exactly deletes a clone holding the only copy of somebody's work. `lifecycle::workspace_remove` is the one exported removal now -- probe, guard, name the volumes, delete, remove the clone. `guard_removal`, `unsaved_work_in`, `workspace_delete`, `Guarded` and `Persistence` are `pub(crate)`; `workspace_delete` keeps its signature, so its unit tests run unchanged. `DeleteOutcome` is `RemoveOutcome` with a `Refused` arm. Which of the three removals is being asked for is one `Removal` value moved out of `dl` rather than four flags a caller assembles: `Insistence`, `Persistence` and the probe are total functions of it. That is what keeps `kill`'s report of the work it destroys, which the ticket's `insistence`-only sketch would have dropped. `repo_manager`'s `pub(crate)` tree-removal result is `TreeSweep`, after the function that produces it. Two `LifecycleNotice` arms carry what `dl` printed between the calls, and `dl` passes the streaming sink so the lines keep their order and timing. `api::workspace_delete` is out and `api::workspace_remove` is in, with the parameter and answer types a caller needs to reach either. Closes #410.
blooop
force-pushed
the
wayfinder/devlaunch-410
branch
from
August 29, 2026 19:40
cd6c3fe to
387c0f8
Compare
`workspace_remove` now takes the volume-copy store #516 added to the delete and passes it straight through, so the fold keeps the reclamation rather than dropping it: a removal that came back removed still forgets the copy that named the volumes docker has just taken away. `api` re-exports `KeptCopies` with it, because a parameter type outside the promise is a promised call nobody outside can make. Also: the repo_manager import list is the union of both sides, `main`'s new `FetchRepoError` beside this branch's `TreeSweep`; one unit test from #516 follows `DeleteOutcome` to its new name; and the CHANGELOG entry sits under the current `[Unreleased]`, not inside the 0.25.0 section the release cut renamed underneath it.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
No nightly toolchain on this host, so the generator could not be run here: CI's public-api job regenerates and diffs, and these are the two rows it named. `api::LifecycleNotice::VolumesNotRemoved::occasion` is a merge casualty rather than a stale file. #516 added the field while this branch was moving the whole notice vocabulary from the rest file into the promise, so the row was added to a block that no longer existed and lost on the way across. The second is the re-export's own cost: promising `KeptCopies` makes the generator render its inherent and derived impls a second time, at the canonical path, beside the other promised types' impls.
blooop
added a commit
that referenced
this pull request
Aug 29, 2026
A diff in public-api.api.txt is a change to the tier an external consumer may depend on, so this is the one to read rather than skim. `RemovalRefused` stops being a two-arm enum and becomes a struct carrying the whole standing, which is #446's decision reaching the guard: a clone can hold work *and* have a question that could not be put, and a refusal that named one arm would be telling half the truth. It reaches the promised tier only because main promoted `RemovalRefused` into `api` in #520 while this branch was in flight. The consequence is that `agent_worktrees::Standing` is now named at that tier too, through the `standing` field, and its readers -- would_lose, could_not_tell, describe -- are what a consumer reads the refusal with.
This was referenced Aug 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Stacked on #514 (base
wayfinder/devlaunch-340); GitHub retargets this tomainwhen that merges. Read the diff over #514's head, not over main.Implements #410, decided on #395.
The problem, stated as a test
devlaunch_core::apipromisedworkspace_delete, and that is the delete without the unsaved-work guard. The probe and the guard that make it safe were two further exported functions a caller had to run first, in the right order, with the right arguments, and the sequence lived in thedlbinary. So the promise was the unguarded delete, nothing outsidedlcould reuse the safe sequence, and nothing helddlto it. A second consumer following the promise exactly deletes a clone holding the only copy of somebody's afternoon.The red is
rust/devlaunch-core/tests/api_removal_is_self_sufficient.rs, written withapipaths only, over a real cache directory with a real git repository in it:a_guarded_removal_refuses_over_unsaved_work_and_never_asks_devpod— a recorded clone with an uncommitted change,Removal::Guarded, and the answer isRemoveOutcome::Refused(RemovalRefused::WouldLose { losses, .. }). Thelossesare bound and read, not just matched, and the runner never sawdevpod delete. Against the old surface it does not compile: there is noapi::workspace_remove, and five ofworkspace_delete's parameter types were not inapieither.an_insisted_removal_deletes_the_same_clone_and_runs_no_probe—rm --forceremoves it and runs nogit statusand nogit log. Pins the conditional probe: probing unconditionally is the one-line accident this fold invites, and it is invisible except in the bill. Verified red by mutatingRemoval::Insisted => Probe::SkiptoProbe::Look(..).the_volumes_are_named_before_devpod_is_asked_to_delete— the ordering the delete's own comment claims. The fake devpod removes its own record when asked to delete, the way the real one does, so a sweep that read the names afterwards would have nothing to ask docker about. Verified red by moving the naming below the delete; both orders otherwise remove the workspace and exit zero.What folded
lifecycle::workspace_removeis the one exported removal:Probe, guard, name the volumes, delete, remove the clone, in that order.
guard_removal,unsaved_work_inandworkspace_deletearepub(crate), and so areGuardedandPersistence; their unit tests run unchanged, becauseworkspace_deletekeeps the signature it had.gitis not a parameter: inside core it iscontext.git().DeleteOutcomeisRemoveOutcome, withRefused(RemovalRefused)besideDeletedandDevpodRefused. One sum, not two: a refusal is an end of the removal, and having it beside the delete's answer is what let a caller hold the first and go on to the second.Two deviations from the ticket's sketch, both about
killThe ticket writes the signature with
insistence: Insistenceand "the probe stays conditional onInsistence::NotInsisted". Applied literally that silently dropsdl <ws> kill's report:killisInsistence::Insisted, andRemoval::Wedged's own doc says the guard still looks and reports rather than stopping, whichdl/tests/lifecycle.rs::a_kill_names_the_work_it_is_about_to_destroy_and_destroys_itholds it to. The ticket's list also has no room forpersistenceor thestalledcallback, whichkillneeds for devpod's--forceand the deadline.So the parameter is
Removal, which isdl's own three-way value moved into core:Guarded,Insisted,Wedged. It is a deeper fold than one flag, not a wider one —Insistence,Persistenceand the probe are all total functions of it now, so the four flags a caller used to assemble are down to one value that names a command line.Removal::insistence()stays public because a rendering of the answer turns on it.The name was taken by
repo_manager's tree-removal result, which ispub(crate), in no snapshot, and now calledTreeSweepafter the function that produces it (remove_tree_as_far_as_it_goes).RemoveOutcomestill follows the module's<Verb>Outcomeconvention.Two
LifecycleNoticearms carry whatdlused to print between the calls:Removing { workspace_id }andRemovingOverWork { refusal }. Their timing is the whole of their value — one is a warning before a wait rather than a receipt after it — sodlpasses the streamingrender::Sayingsink instead of collecting into a vector. Same lines, same order, same words.api, and it is a removalapi::workspace_deleteis out andapi::workspace_removeis in. #251 §7 calls dropping a promised function a breaking change; it is the right weight here, because keeping both leaves the unguarded delete promised, which is the finding. An unguarded delete is now unrepresentable rather than documented.apialso gained what a caller needs to call it and read its answer:DevpodHome,MetadataStorage,WorkspaceCloneManager,Records,DeleteStalled,Insistence,LifecycleNotice,Removal,RemovalRefused,RemoveOutcome. Same defect and same answer as #313's forLaunch::new.Recordsand the two it holds are what #411 collapses into one parameter; the list is left explicit rather than bundled a second time, so that fold has something to build on.DevpodHomeis also #427's finding, arriving as a deletion: that PR re-typedworkspace_delete's fifth parameter toOption<&DevpodHome>without re-exporting the type. The row it left uncallable is gone, and the type is inapifor the call that replaced it.Snapshots regenerated with the pinned generator, not hand-edited.
Both of the things the last rebase owed
Merged with
main, and neither of these was a text merge.Keep devlaunch's own copy of the volume names, and reclaim from it #516's
KeptCopiesis carried. That PR widened the delete with thevolume-copy store so a removal reclaims the volumes devpod substituted, and it
landed while this branch was based on Move ColdPath and ToolProvisioning into core; complete api's re-exports #514.
workspace_removetakescopies: &KeptCopiesbesidedevpod_homenow and passes it straight through toworkspace_delete, whose signature is Keep devlaunch's own copy of the volume names, and reclaim from it #516's and unchanged. Resolving thesnapshot conflict by keeping this side would have kept a
workspace_removewithno store to drop a copy into, which compiles against nothing and quietly loses
the whole of Keep devlaunch's own copy of the volume names, and reclaim from it #456: the workspace still goes, the exit is still zero, and the next
--prunereports reclaiming volumes that left with the workspace.So there is a fourth test in
api_removal_is_self_sufficient.rs,the_kept_copy_of_the_volumes_goes_with_the_workspace, which writes the copy acompleted
upleaves and asserts the removal takes it away. Verified red bypointing
workspace_remove's pass-through at a different directory: every othertest stays green.
apigainedKeptCopieswith it. Keep devlaunch's own copy of the volume names, and reclaim from it #516 put the type in the promised row'ssignature and re-exported nothing, which is devpod's on-disk layout gets the module it was missing #427's finding again and the same
answer: a parameter type outside the promise is a promised call nobody outside
can make.
The CHANGELOG entry is under the current
[Unreleased], above## [0.25.0] - 2026-08-28, merged intomain's own lists rather than replacingthem. The trap is that the heading a rebase anchors against was renamed by the
release cut, so the clean resolution puts the entry inside a shipped release.
One thing that is not a merge artefact:
repo_manager's import list inlifecycle.rsis the union of both sides,main's newFetchRepoErrorbesidethis branch's
TreeSweep, and one unit test #516 added followsDeleteOutcometoits new name.
🤖 Generated with Claude Code
Summary by Sourcery
Replace the exported unguarded workspace delete with a single self-contained removal operation that enforces safety checks and cleanup for every removal mode.
New Features:
workspace_removeAPI that performs guarded, forced, and wedged workspace removals through a single removal mode.Bug Fixes:
Enhancements:
TreeSweepand consolidate deletion results underRemoveOutcome.Documentation:
workspace_deletesurface in the changelog.Tests: