Skip to content

Fold the removal guard into one workspace_remove and swap the api row #410

Description

@blooop

Slice

Fold flows::lifecycle's three exported removal functions into one, so the promised surface cannot delete a clone holding the only copy of someone's work. Decided on #395.

Today unsaved_work_in (lifecycle.rs:868, 6 params), guard_removal (:806, 3) and workspace_delete (:972, 8) are three pub calls the caller must sequence correctly, the sequence lives in the binary (dl/src/commands.rs:664-740), and api freezes only the last one.

  • lifecycle::workspace_remove(context, refresh, clones, storage, cache_dir, devpod_home, workspace_id, insistence, notices) -> Result<RemoveOutcome, NotRun> — probe, guard, volume-name, delete, clone removal, in that order. git is not a parameter: inside core it is context.git() (flows/listing.rs:108).
  • DeleteOutcome is renamed RemoveOutcome and grows Refused(RemovalRefused) beside DevpodRefused and Deleted. One sum, not two — Removal is already purge's tree-removal type (lifecycle.rs:1363), and <Verb>Outcome is this module's convention.
  • guard_removal, unsaved_work_in and the raw delete body become pub(crate). Their unit tests (lifecycle.rs:5368-5450) keep running unchanged.
  • The probe stays conditional on Insistence::NotInsisted — it is a git status and a git log per clone, and dl skips it under --force today (commands.rs:690). Probing unconditionally is the one-line accident this fold invites.
  • dl's render_remove collapses to one call plus rendering: match RemoveOutcome::Refused → render::removal_refusal + Ending::Refused, the other two arms as today.

api changes, and it is a removal

api::workspace_delete out, api::workspace_remove in — keeping both leaves the unguarded delete promised, which is the finding. #251 §7 calls that a breaking change; it is the right weight here.

api also re-exports what a caller needs to call it and match the answer: Refresh, WorkspaceCloneManager, MetadataStorage, Insistence, Notices, LifecycleNotice, RemoveOutcome, RemovalRefused. Today 5 of workspace_delete's 8 parameter types live outside api, so the promised delete is not callable from the promise — the same defect #313 decided for Launch::new, same answer.

Snapshots are hand-edited. cargo-public-api needs nightly and this devcontainer cannot install it (scripts/public-api-snapshots.sh header), so edit public-api.api.txt and public-api.rest.txt by hand and let CI's regenerate-and-diff be authoritative. Budget one round-trip. The rest file moves too: the canonical flows::lifecycle:: rows and the derives on the renamed type.

Red first

  1. The red the finding names, written with devlaunch_core::api paths only — no flows::, domain:: or clients:: path in the test file, which is what proves the promise is self-sufficient. A recorded clone holding an uncommitted change, Insistence::NotInsisted → RemoveOutcome::Refused, and the fake runner never saw devpod delete. Red today: api::workspace_delete deletes it.
  2. Insistence::Insisted removes it anyway and runs no git status/git log — pins the conditional probe.
  3. The ordering comment at lifecycle.rs:980-988 becomes an assertion: in the recorded call order, the volume names are read before devpod delete.

The World/clones_for scaffolding at lifecycle.rs:3527-3660 is what tests 1 and 2 build on.

Ordering

Unblocked. Should land before #314 (the lifecycle.rs banner split): the guard sits above the // delete banner and the delete below it, so folding first means the split moves one function rather than three across a boundary that no longer separates them. #314 is blocked on #338 anyway. No edge is added into #299's map.

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