Closes#2813 (team-collapse) and #2814 (workspace delete).
Two leaks, one class. Both call sites had the same shape pre-fix:
if h.provisioner != nil {
h.provisioner.Stop(ctx, wsID)
}
On SaaS where h.provisioner (Docker) is nil and h.cpProv is set, that
gate evaluates false and the EC2 keeps running. Workspace gets marked
removed in DB; EC2 lives on until the orphan sweeper catches it.
Same drift class as PR #2811's org-import provision bug — a Docker-
only check on what should be a both-backend operation. Confirmed in
production: PR #2811's verification step deleted a test workspace and
the EC2 stayed running until I terminated it manually.
Fix: WorkspaceHandler.StopWorkspaceAuto(ctx, wsID) — symmetric mirror
of provisionWorkspaceAuto. CP first, Docker second, no-op when neither
is wired (a workspace nobody is running can't be stopped — that's a
no-op, not a failure, distinct from provision's mark-failed contract).
Three call-site changes:
- team.go:208 (Collapse) → h.wh.StopWorkspaceAuto(ctx, childID)
- workspace_crud.go:432 (stopAndRemove) → h.StopWorkspaceAuto(...);
RemoveVolume stays Docker-only behind an explicit gate since
CP-managed workspaces have no host-bind volumes
- TeamHandler.provisioner field + NewTeamHandler's *Provisioner param
removed as dead code (Stop was the only call site)
Volume cleanup separation is intentional: the abstraction is "stop
the running workload," not "tear down all state." Callers that need
volume cleanup keep their `if h.provisioner != nil { RemoveVolume }`
gate AFTER the Stop call.
Tests:
- TestStopWorkspaceAuto_RoutesToCPWhenSet — SaaS path
- TestStopWorkspaceAuto_RoutesToDockerWhenOnlyDocker — self-hosted
- TestStopWorkspaceAuto_NoBackendIsNoOp — pins the contract distinction
from provisionWorkspaceAuto's mark-failed
- TestNoCallSiteCallsBareStop — source-level pin against
`.provisioner.Stop(` / `.cpProv.Stop(` outside the dispatcher,
per-backend bodies, restart helper, and the Docker-daemon-direct
short-lived-container path. Strips Go comments before substring
match so archaeology in code comments doesn't trip the gate.
- Verified: pin FAILS against the buggy shape (workspace_crud.go
reversion); team.go reversion compile-fails because the field is
gone — even stronger than the test.
Out of scope (tracked under #2799):
- workspace_restart.go's manual if-cpProv-else dispatch with retry
semantics tuned for the restart hot path. Functionally equivalent
+ wraps cpStopWithRetry, so it's not the bug class this PR closes.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
11c9ed2a46