Repository navigation
api::workspace_delete is the delete without the unsaved-work guard #395
Description
Finding
api::workspace_delete — the frozen wf surface — is the delete without the unsaved-work guard.
Removal is three exported functions that the caller must sequence correctly:
| function | site | params |
|---|---|---|
unsaved_work_in |
flows/lifecycle.rs:868 |
6 |
guard_removal |
flows/lifecycle.rs:806 |
3 |
workspace_delete |
flows/lifecycle.rs:972 |
8, #[allow(clippy::too_many_arguments)] |
17 parameters of interface. The sequence — probe for unsaved work, refuse unless --force, then delete — lives in the binary, at dl/src/commands.rs:664-740. dl gets it right. Nothing makes a second caller get it right, and devlaunch-core/src/lib.rs freezes only workspace_delete into api, so a wf that links the promised surface deletes a clone holding uncommitted work without ever asking.
The comment at dl/src/commands.rs:688 calls the guard "the one thing dl refuses on its own account". That is the problem stated as a feature: it is the one thing a caller refuses, and the frozen API has no caller.
Shape
One deep module. remove(context, target, insistence, notices) -> Result<Removal, NotRun>, absorbing target resolution, opening the records once, the unsaved-work probe, the guard, the volume sweep and the delete. Removal::Refused(RemovalRefused) is an arm, so a caller that ignores the guard does not compile.
Two ordering constraints move from prose into the module while this is open:
lifecycle.rs:980-988— volumes must be named beforedevpod delete, because delete destroys the record holding their names. Currently a comment.- The guard must run before the delete, currently a comment plus the binary's control flow.
Cost, deliberately
This changes public-api.api.txt, which CI diffs — a deliberate PR, not a refactor. That is the right weight for a change that decides whether wf can delete unsaved work, and #251 §7 already says an addition to api is a deliberate PR and a removal a breaking change.
Proving it
Red-first, and the red is reachable from the promised surface alone: call api::workspace_delete against a clone with an uncommitted change and watch it go. There is no such test today because the guard is not on that path.
Found by an architecture review.
Activity
blooop commentedon Aug 24, 2026
agent-decided (constructive modeling): shape (i), with the seam one step in from where the ticket drew it. Fold the three exported functions into one
lifecycle::workspace_removewhose outcome sum carries the refusal as an arm;guard_removal,unsaved_work_inand the raw delete becomepub(crate). The seam is the workspace id, not the target word: absorbing target resolution needsColdPathin core, which is #340's job on a human-gated map, and a safety defect on the promised surface must not park behind it.Why (ii), the
Guardedwitness, lostIt is the shape constructive modeling rejects.
Guarded::MayRemovebecomes a witness only if it is unconstructible outside core — and even then it witnesses thatguard_removalran, not that it ran on a real answer.guard_removal(id, Unsaved::NothingToLose, NotInsisted)mints one from a fabricated probe, andUnsavedis a plain data enum that the listing flow's own tests build by hand (flows/listing.rs:2678). The illegal state — delete a clone holding the only copy of someone's work — stays representable, just spelled differently. That is a comment-enforced correlation wearing a type, which is the smell, not the fix. It also keeps three functions and 17 parameters of interface where one function has nine and no sequencing rule at all.Shape (i) makes it a compile error instead: after the fold there is no exported call that deletes without probing, because there is no exported call that deletes.
Why (iii), document the contract, lost
The contract is already documented.
dl/src/commands.rs:688names the guard,flows/lifecycle.rs:980-988names the volume ordering, and the finding is precisely that documentation is the thing the second caller does not read. Free, and fixes nothing.The type
DeleteOutcomeis renamedRemoveOutcomeand grows one arm —Refused(RemovalRefused)besideDevpodRefusedandDeleted. One sum, not two: a freshRemovalalongsideDeleteOutcomewould be two overlapping types for one answer, andRemovalis already taken in this module by purge's tree removal (lifecycle.rs:1363).<Verb>Outcomeis the module's own convention —StopOutcome,PruneOutcome,PurgeOutcome.workspace_remove, not a re-signaturedworkspace_delete, for two reasons. Remove is this domain's verb (dl <ws> rm,RemovalRefused,remove_workspace_by_id); delete is devpod's, and it is one step inside the removal. And the split promise file exists so that a diff in it reads as a contract change: a removed row plus an added row is unmistakable, where a changed signature under the same name reads as routine.What the fold costs, stated honestly
17 parameters across three calls become nine in one, and the
#[allow(clippy::too_many_arguments)]survives.gitgenuinely disappears — inside core it iscontext.git()(flows/listing.rs:108) rather than a parameter — andcache_dirjoins. No parameter-bundle type is invented here, on purpose: #340 moves dl'sColdPath/Recordsinto core, and that bundle is the one that collapsesclones+storage+cache_dirto a single argument. Two overlapping bundles is exactly the thing worth not doing.One behaviour to carry across the fold rather than rediscover: the probe stays conditional on
NotInsisted. dl asks only when the guard could refuse, because the probe is agit statusand agit logper clone (commands.rs:690). Folding makes "probe unconditionally" a one-line accident that slows everyrm -f, so it gets its own test.Does
apichange now — yes, and it is a removalapi::workspace_deletegoes;api::workspace_removearrives. Keeping both would leave the unguarded delete promised, which is the finding. #251 §7 calls a removal fromapia breaking change, and that is the right weight for a change deciding whetherwfcan destroy the only copy of someone's work.apialso re-exports what a caller needs to call it and match the answer —Refresh,WorkspaceCloneManager,MetadataStorage,Insistence,Notices,LifecycleNotice,RemoveOutcome,RemovalRefused. Today 5 ofworkspace_delete's 8 parameter types live outsideapi, so the promised delete is not callable from the promise; that is the same defect #313 already decided forLaunch::new(5 of 7), and the same answer. The red test below is the forcing function: it may name noflows::,domain::orclients::path.Snapshot mechanics:
public-api.api.txtandpublic-api.rest.txtare hand-edited on this branch — cargo-public-api needs nightly and this devcontainer cannot install it (scripts/public-api-snapshots.sh, header). CI's regenerate-and-diff is authoritative, so budget one round-trip to true the rows up rather than trusting the hand edit, and expect the rest file to move too (the canonicalflows::lifecycle::rows and the derives on the renamed type).Ordering against #299
No collision. #313/#340 reshape
Launch,ColdMachinery,ProvisionandColdRefused— disjoint from the remove verb. #314 splitslifecycle.rsalong its banners, and the guard sits above the// deletebanner while the delete sits below it, so the fold should land first: the split then moves one function instead of three across a boundary that no longer separates them. #314 is blocked on #338 in any case. No edge is added into #299's map — this map does not re-decide tickets living there.Proving it, red first
devlaunch_core::apipaths only: a recorded clone holding an uncommitted change,Insistence::NotInsisted→RemoveOutcome::Refused, and the fake runner never saw adevpod delete. Red today, becauseapi::workspace_deletedeletes it.Insistence::Insistedremoves it anyway and runs nogit status/git log— pins the conditional probe against the accident above.lifecycle.rs:980-988becomes an assertion: in the recorded call order, the volume names are read beforedevpod delete.lifecycle.rs:5368-5450) keep running against the now-pub(crate)functions. Nothing there is deleted; the fold changes who may call them, not what they answer.Graduates two builds: the fold and the
apiswap (unblocked, lands before #314), and the parameter collapse onto core's records bundle once #340 lands.blooop commentedon Aug 24, 2026
Resolved above; graduated to #410 (the fold and the api swap) and #411 (the parameter collapse, blocked on #340).