From acecb16d22b8f205592bd4412530a730697a5664 Mon Sep 17 00:00:00 2001 From: hongming-ceo-delegated Date: Fri, 29 May 2026 23:49:37 -0700 Subject: [PATCH 1/3] feat(workspace): per-workspace data_persistence choice (internal#734 PR-2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Threads the user's durable-data choice from the workspace Compute config through to CP's provision request, so a user can pick persist vs ephemeral per workspace (the caller side of cp#410's data_persistence support). - models.WorkspaceCompute.DataPersistence (persisted in the compute JSONB) - validateWorkspaceCompute: enum guard (persist|ephemeral|"") → clear 400 before the CP round-trip; CP re-validates at its edge (defense in depth) - WorkspaceConfig.DataPersistence + workspace_provision build site - cpProvisionRequest.data_persistence (omitempty → ""=auto omitted on wire) Empty/auto = today's behavior; forward-compatible (inert until CP deploys cp#410). +validator enum test. build/vet/test/gofmt green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/handlers/workspace_compute.go | 8 ++++ .../handlers/workspace_compute_test.go | 19 +++++++- .../internal/handlers/workspace_provision.go | 1 + workspace-server/internal/models/workspace.go | 6 +++ .../internal/provisioner/cp_provisioner.go | 43 +++++++++++-------- .../internal/provisioner/provisioner.go | 1 + 6 files changed, 58 insertions(+), 20 deletions(-) diff --git a/workspace-server/internal/handlers/workspace_compute.go b/workspace-server/internal/handlers/workspace_compute.go index 9ec1760b2..0824a655b 100644 --- a/workspace-server/internal/handlers/workspace_compute.go +++ b/workspace-server/internal/handlers/workspace_compute.go @@ -65,6 +65,14 @@ func validateWorkspaceCompute(compute models.WorkspaceCompute) error { if err := validateWorkspaceDisplayDimensions(compute.Display.Width, compute.Display.Height); err != nil { return err } + // internal#734: the durable-data choice. CP re-validates the same enum at + // its provision edge (IsValidDataPersistence → 400); validating here too + // gives the user a clear workspace-server error before the CP round-trip. + switch compute.DataPersistence { + case "", "persist", "ephemeral": + default: + return fmt.Errorf("unsupported compute.data_persistence (want persist|ephemeral)") + } return nil } diff --git a/workspace-server/internal/handlers/workspace_compute_test.go b/workspace-server/internal/handlers/workspace_compute_test.go index 38d07113c..ab212cb5a 100644 --- a/workspace-server/internal/handlers/workspace_compute_test.go +++ b/workspace-server/internal/handlers/workspace_compute_test.go @@ -11,8 +11,8 @@ import ( "testing" "time" - "github.com/DATA-DOG/go-sqlmock" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/models" + "github.com/DATA-DOG/go-sqlmock" "github.com/gin-gonic/gin" ) @@ -36,6 +36,23 @@ func TestValidateWorkspaceCompute_RejectsUnknownInstanceType(t *testing.T) { } } +// internal#734: data_persistence enum. "" (auto), "persist", "ephemeral" are +// the only accepted values; anything else is a clear 400 before the CP call. +func TestValidateWorkspaceCompute_DataPersistence(t *testing.T) { + for _, ok := range []string{"", "persist", "ephemeral"} { + c := models.WorkspaceCompute{DataPersistence: ok} + if err := validateWorkspaceCompute(c); err != nil { + t.Errorf("data_persistence=%q must be accepted: %v", ok, err) + } + } + for _, bad := range []string{"persistent", "off", "none", "Ephemeral", "true"} { + c := models.WorkspaceCompute{DataPersistence: bad} + if err := validateWorkspaceCompute(c); err == nil { + t.Errorf("data_persistence=%q must be rejected", bad) + } + } +} + func TestValidateWorkspaceCompute_RejectsOutOfRangeRootVolume(t *testing.T) { for _, rootGB := range []int{29, 501} { compute := models.WorkspaceCompute{Volume: models.WorkspaceComputeVolume{RootGB: rootGB}} diff --git a/workspace-server/internal/handlers/workspace_provision.go b/workspace-server/internal/handlers/workspace_provision.go index 92439db52..da5e71f1e 100644 --- a/workspace-server/internal/handlers/workspace_provision.go +++ b/workspace-server/internal/handlers/workspace_provision.go @@ -330,6 +330,7 @@ func (h *WorkspaceHandler) buildProvisionerConfig( Runtime: payload.Runtime, InstanceType: payload.Compute.InstanceType, DiskGB: int32(payload.Compute.Volume.RootGB), + DataPersistence: payload.Compute.DataPersistence, Display: provisioner.WorkspaceDisplayConfig{ Mode: payload.Compute.Display.Mode, Width: payload.Compute.Display.Width, diff --git a/workspace-server/internal/models/workspace.go b/workspace-server/internal/models/workspace.go index a23bccd9e..a43a162bb 100644 --- a/workspace-server/internal/models/workspace.go +++ b/workspace-server/internal/models/workspace.go @@ -168,6 +168,12 @@ type WorkspaceCompute struct { InstanceType string `json:"instance_type,omitempty"` Volume WorkspaceComputeVolume `json:"volume,omitempty"` Display WorkspaceComputeDisplay `json:"display,omitempty"` + // DataPersistence is the per-workspace durable-data choice (internal#734): + // "persist" keeps the workspace's data volume (browser profile / cookies / + // downloads / agent memory) across recreate; "ephemeral" uses no durable + // disk (wiped each recreate — privacy); "" = auto (desktop-control persists, + // others follow the org flag). Forwarded verbatim to CP's data_persistence. + DataPersistence string `json:"data_persistence,omitempty"` } type CreateWorkspacePayload struct { diff --git a/workspace-server/internal/provisioner/cp_provisioner.go b/workspace-server/internal/provisioner/cp_provisioner.go index 804831e1a..88e3c6bc2 100644 --- a/workspace-server/internal/provisioner/cp_provisioner.go +++ b/workspace-server/internal/provisioner/cp_provisioner.go @@ -152,15 +152,19 @@ func (p *CPProvisioner) adminAuthHeaders(req *http.Request) { } type cpProvisionRequest struct { - OrgID string `json:"org_id"` - WorkspaceID string `json:"workspace_id"` - Runtime string `json:"runtime"` - Tier int `json:"tier"` - InstanceType string `json:"instance_type,omitempty"` - DiskGB int32 `json:"disk_gb,omitempty"` - Display WorkspaceDisplayConfig `json:"display,omitempty"` - PlatformURL string `json:"platform_url"` - Env map[string]string `json:"env"` + OrgID string `json:"org_id"` + WorkspaceID string `json:"workspace_id"` + Runtime string `json:"runtime"` + Tier int `json:"tier"` + InstanceType string `json:"instance_type,omitempty"` + DiskGB int32 `json:"disk_gb,omitempty"` + // DataPersistence is the per-workspace durable-data choice (internal#734); + // CP validates the enum at its provision edge and resolves the data volume + // from it. Empty = auto (omitted on the wire). + DataPersistence string `json:"data_persistence,omitempty"` + Display WorkspaceDisplayConfig `json:"display,omitempty"` + PlatformURL string `json:"platform_url"` + Env map[string]string `json:"env"` // ConfigFiles are template + generated config files to write into the // EC2 instance's /configs directory. OFFSEC-010: collected by // collectCPConfigFiles which rejects symlinks and non-regular files @@ -211,16 +215,17 @@ func (p *CPProvisioner) Start(ctx context.Context, cfg WorkspaceConfig) (string, } req := cpProvisionRequest{ - OrgID: p.orgID, - WorkspaceID: cfg.WorkspaceID, - Runtime: cfg.Runtime, - Tier: cfg.Tier, - InstanceType: cfg.InstanceType, - DiskGB: cfg.DiskGB, - Display: cfg.Display, - PlatformURL: cfg.PlatformURL, - Env: env, - ConfigFiles: configFiles, + OrgID: p.orgID, + WorkspaceID: cfg.WorkspaceID, + Runtime: cfg.Runtime, + Tier: cfg.Tier, + InstanceType: cfg.InstanceType, + DiskGB: cfg.DiskGB, + DataPersistence: cfg.DataPersistence, + Display: cfg.Display, + PlatformURL: cfg.PlatformURL, + Env: env, + ConfigFiles: configFiles, } body, err := json.Marshal(req) diff --git a/workspace-server/internal/provisioner/provisioner.go b/workspace-server/internal/provisioner/provisioner.go index 498643447..60c1c827c 100644 --- a/workspace-server/internal/provisioner/provisioner.go +++ b/workspace-server/internal/provisioner/provisioner.go @@ -99,6 +99,7 @@ type WorkspaceConfig struct { Runtime string // "claude-code" (default), "codex", "hermes", "openclaw", etc. InstanceType string // Optional CP EC2 instance type override (SaaS only) DiskGB int32 // Optional CP root volume size override in GiB (SaaS only) + DataPersistence string // internal#734: "persist"|"ephemeral"|"" — durable-data choice forwarded to CP (SaaS only) Display WorkspaceDisplayConfig EnvVars map[string]string // Additional env vars (API keys, etc.) PlatformURL string -- 2.52.0 From db6f5b2e9314dbb210a0f4f88c0157873c6c9d6e Mon Sep 17 00:00:00 2001 From: hongming-ceo-delegated Date: Sat, 30 May 2026 09:16:47 -0700 Subject: [PATCH 2/3] =?UTF-8?q?feat(workspace):=20prune-on-delete=20caller?= =?UTF-8?q?=20wiring=20(internal#734=20F1=20=E2=80=94=20pairs=20with=20cp#?= =?UTF-8?q?415)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The caller side of the recreate-safe prune (cp#415 Five-Axis F1): the prune signal reaches CP ONLY on a permanent user-delete-with-erase, NEVER on restart/recreate/reconcile. - CPProvisionerAPI.StopAndPrune (CPProvisioner builds DELETE with &prune=true; Stop never does — shared stopInternal). - cpStopWithRetryErr(...prune): restart/hibernate pass false; delete passes the user choice. - stopWorkspaceForDelete(...erase) → CascadeDelete(...erase): HTTP Delete reads ?erase_data=true (opt-in; default keeps data for the orphan-sweeper grace); org-import reconcile passes false. Discriminating test: Stop sends NO prune=true (recreate-safety), StopAndPrune sends it. All CPProvisionerAPI mocks gain StopAndPrune. Full handlers+provisioner suite + vet + gofmt green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../internal/handlers/a2a_proxy_test.go | 6 ++- workspace-server/internal/handlers/org.go | 4 +- .../internal/handlers/workspace_crud.go | 18 +++++++-- .../internal/handlers/workspace_crud_test.go | 6 +-- .../workspace_delete_stop_retry_test.go | 6 +-- .../handlers/workspace_dispatchers.go | 8 +++- .../handlers/workspace_provision_auto_test.go | 8 ++++ ...rkspace_provision_concurrent_repro_test.go | 5 ++- .../handlers/workspace_provision_test.go | 3 ++ .../internal/handlers/workspace_restart.go | 16 ++++++-- .../workspace_restart_stop_retry_test.go | 7 ++++ .../internal/provisioner/cp_provisioner.go | 21 ++++++++++ .../provisioner/cp_provisioner_test.go | 38 +++++++++++++++++++ 13 files changed, 129 insertions(+), 17 deletions(-) diff --git a/workspace-server/internal/handlers/a2a_proxy_test.go b/workspace-server/internal/handlers/a2a_proxy_test.go index 6a254a7f5..10b21cbda 100644 --- a/workspace-server/internal/handlers/a2a_proxy_test.go +++ b/workspace-server/internal/handlers/a2a_proxy_test.go @@ -16,9 +16,9 @@ import ( "testing" "time" - "github.com/DATA-DOG/go-sqlmock" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/models" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/provisioner" + "github.com/DATA-DOG/go-sqlmock" "github.com/gin-gonic/gin" ) @@ -2117,6 +2117,10 @@ func (f *fakeCPProv) Stop(_ context.Context, _ string) error { f.stopCalls++ return nil } +func (f *fakeCPProv) StopAndPrune(_ context.Context, _ string) error { + f.stopCalls++ + return nil +} func (f *fakeCPProv) GetConsoleOutput(_ context.Context, _ string) (string, error) { return "", nil } diff --git a/workspace-server/internal/handlers/org.go b/workspace-server/internal/handlers/org.go index 0d9e87b39..1682b759d 100644 --- a/workspace-server/internal/handlers/org.go +++ b/workspace-server/internal/handlers/org.go @@ -875,7 +875,9 @@ func (h *OrgHandler) Import(c *gin.Context) { rows.Close() for _, oid := range orphanIDs { - descendantIDs, stopErrs, err := h.workspace.CascadeDelete(ctx, oid) + // erase=false: a reconcile is not a user-requested erase — + // never prune data volumes on the import-reconcile path (internal#734). + descendantIDs, stopErrs, err := h.workspace.CascadeDelete(ctx, oid, false) if err != nil { log.Printf("Org import reconcile: CascadeDelete(%s) failed: %v", oid, err) reconcileErrs = append(reconcileErrs, fmt.Sprintf("delete %s: %v", oid, err)) diff --git a/workspace-server/internal/handlers/workspace_crud.go b/workspace-server/internal/handlers/workspace_crud.go index f122f4fb3..1c79ee2b8 100644 --- a/workspace-server/internal/handlers/workspace_crud.go +++ b/workspace-server/internal/handlers/workspace_crud.go @@ -399,7 +399,13 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) { // disable, broadcast). The HTTP-specific bits — direct-children 409 // gate above, ?purge=true hard-delete below, response shaping — // stay in this handler. - descendantIDs, stopErrs, err := h.CascadeDelete(ctx, id) + // internal#734: the user can ask to erase saved data (browser profile / + // cookies / downloads / agent memory) on delete. Opt-in — default keeps the + // data on its volume for the orphan-sweeper grace. Only a genuine + // permanent-delete reaches here (restart/reconcile use other paths), so this + // is the one place prune may be requested. + erase := c.Query("erase_data") == "true" + descendantIDs, stopErrs, err := h.CascadeDelete(ctx, id, erase) if err != nil { // Audit 2026-05-09 (Core-Security): raw `err.Error()` here was // exposed to HTTP clients verbatim, including wrapped lib/pq @@ -515,7 +521,13 @@ func destructiveDeleteCounts(ctx context.Context, id string) (childCount int, sc // Caller is responsible for the children-confirmation gate (the HTTP handler // returns 409 when children exist + ?confirm=true is missing); this helper // always cascades. -func (h *WorkspaceHandler) CascadeDelete(ctx context.Context, id string) ([]string, []error, error) { +// CascadeDelete tears down a workspace and its descendants (stop compute, +// remove volumes, revoke tokens, disable schedules, broadcast). erase=true +// (internal#734) means the user asked to erase saved data, so the CP compute +// teardown prunes each workspace's durable data volume; the HTTP delete passes +// the user's choice, the org-import reconcile passes false (a reconcile is not +// a user-erase). +func (h *WorkspaceHandler) CascadeDelete(ctx context.Context, id string, erase bool) ([]string, []error, error) { if err := validateWorkspaceID(id); err != nil { return nil, nil, err } @@ -579,7 +591,7 @@ func (h *WorkspaceHandler) CascadeDelete(ctx context.Context, id string) ([]stri // pending EC2 is queryable and handed off to the CP-orphan-sweeper — // rather than the bare one-shot StopWorkspaceAuto that produced the // silent-leak class (task #15 / workspace-ec2-leak). - if err := h.stopWorkspaceForDelete(cleanupCtx, wsID); err != nil { + if err := h.stopWorkspaceForDelete(cleanupCtx, wsID, erase); err != nil { log.Printf("CascadeDelete %s stop failed: %v — leaving cleanup for orphan sweeper", wsID, err) stopErrs = append(stopErrs, fmt.Errorf("stop %s: %w", wsID, err)) return diff --git a/workspace-server/internal/handlers/workspace_crud_test.go b/workspace-server/internal/handlers/workspace_crud_test.go index 8010e7c00..7988ac649 100644 --- a/workspace-server/internal/handlers/workspace_crud_test.go +++ b/workspace-server/internal/handlers/workspace_crud_test.go @@ -521,7 +521,7 @@ func TestValidateWorkspaceDir_Empty(t *testing.T) { func TestCascadeDelete_InvalidUUID(t *testing.T) { h := &WorkspaceHandler{} - descendants, stopErrs, err := h.CascadeDelete(context.Background(), "not-a-uuid") + descendants, stopErrs, err := h.CascadeDelete(context.Background(), "not-a-uuid", false) if err == nil { t.Error("expected error for invalid UUID") } @@ -542,7 +542,7 @@ func TestCascadeDelete_DescendantQueryError(t *testing.T) { WithArgs(wsID). WillReturnError(sql.ErrConnDone) - deleted, stopErrs, err := h.CascadeDelete(context.Background(), wsID) + deleted, stopErrs, err := h.CascadeDelete(context.Background(), wsID, false) if err == nil { t.Error("CascadeDelete returned nil error; want descendant query error") } @@ -569,7 +569,7 @@ func TestCascadeDelete_DescendantRowsError(t *testing.T) { WithArgs(wsID). WillReturnRows(rows) - deleted, stopErrs, err := h.CascadeDelete(context.Background(), wsID) + deleted, stopErrs, err := h.CascadeDelete(context.Background(), wsID, false) if err == nil { t.Fatal("CascadeDelete returned nil error; want descendant rows error") } diff --git a/workspace-server/internal/handlers/workspace_delete_stop_retry_test.go b/workspace-server/internal/handlers/workspace_delete_stop_retry_test.go index 480c49c45..6889bff67 100644 --- a/workspace-server/internal/handlers/workspace_delete_stop_retry_test.go +++ b/workspace-server/internal/handlers/workspace_delete_stop_retry_test.go @@ -45,7 +45,7 @@ func TestStopWorkspaceForDelete_CPRetriesTransientThenSucceeds(t *testing.T) { }} h := &WorkspaceHandler{cpProv: stub} - err := h.stopWorkspaceForDelete(context.Background(), "ws-del-1") + err := h.stopWorkspaceForDelete(context.Background(), "ws-del-1", false) if err != nil { t.Fatalf("expected nil error on eventual success, got %v", err) } @@ -73,7 +73,7 @@ func TestStopWorkspaceForDelete_CPExhaustsEmitsDurableEventAndReturnsError(t *te mock.ExpectExec("INSERT INTO structure_events"). WillReturnResult(sqlmock.NewResult(0, 1)) - err := h.stopWorkspaceForDelete(context.Background(), "ws-doomed") + err := h.stopWorkspaceForDelete(context.Background(), "ws-doomed", false) if err == nil { t.Fatal("expected terminal error on retry exhaustion, got nil") } @@ -96,7 +96,7 @@ func TestStopWorkspaceForDelete_CPExhaustsEmitsDurableEventAndReturnsError(t *te func TestStopWorkspaceForDelete_NoBackendIsNoOp(t *testing.T) { h := &WorkspaceHandler{} // cpProv nil, provisioner nil - if err := h.stopWorkspaceForDelete(context.Background(), "ws-x"); err != nil { + if err := h.stopWorkspaceForDelete(context.Background(), "ws-x", false); err != nil { t.Errorf("expected nil no-op with no backend, got %v", err) } } diff --git a/workspace-server/internal/handlers/workspace_dispatchers.go b/workspace-server/internal/handlers/workspace_dispatchers.go index 0ebcf7696..a739d5818 100644 --- a/workspace-server/internal/handlers/workspace_dispatchers.go +++ b/workspace-server/internal/handlers/workspace_dispatchers.go @@ -235,9 +235,13 @@ func (h *WorkspaceHandler) StopWorkspaceAuto(ctx context.Context, workspaceID st // container won't heal on retry (matches RestartWorkspaceAuto's Docker // rationale); the orphan-container sweeper (registry/orphan_sweeper.go) is // the Docker-side backstop. -func (h *WorkspaceHandler) stopWorkspaceForDelete(ctx context.Context, workspaceID string) error { +// stopWorkspaceForDelete terminates a workspace's compute on the delete path. +// erase=true (internal#734) means the user asked to erase saved data, so the CP +// teardown prunes the durable data volume. The local-docker path always removes +// its volume via CascadeDelete's RemoveVolume, so erase is a CP-only concern. +func (h *WorkspaceHandler) stopWorkspaceForDelete(ctx context.Context, workspaceID string, erase bool) error { if h.cpProv != nil { - if err := h.cpStopWithRetryErr(ctx, workspaceID, "Delete"); err != nil { + if err := h.cpStopWithRetryErr(ctx, workspaceID, "Delete", erase); err != nil { h.emitDeleteTerminateRetryExhausted(ctx, workspaceID, err) return err } diff --git a/workspace-server/internal/handlers/workspace_provision_auto_test.go b/workspace-server/internal/handlers/workspace_provision_auto_test.go index b856b79c2..3ce0ee3cb 100644 --- a/workspace-server/internal/handlers/workspace_provision_auto_test.go +++ b/workspace-server/internal/handlers/workspace_provision_auto_test.go @@ -42,6 +42,7 @@ type trackingCPProv struct { mu sync.Mutex started []string stopped []string + pruned []string // internal#734: workspaces stopped via StopAndPrune startErr error stopErr error } @@ -61,6 +62,13 @@ func (r *trackingCPProv) Stop(_ context.Context, workspaceID string) error { r.mu.Unlock() return r.stopErr } +func (r *trackingCPProv) StopAndPrune(_ context.Context, workspaceID string) error { + r.mu.Lock() + r.stopped = append(r.stopped, workspaceID) + r.pruned = append(r.pruned, workspaceID) + r.mu.Unlock() + return r.stopErr +} func (r *trackingCPProv) GetConsoleOutput(_ context.Context, _ string) (string, error) { return "", nil } diff --git a/workspace-server/internal/handlers/workspace_provision_concurrent_repro_test.go b/workspace-server/internal/handlers/workspace_provision_concurrent_repro_test.go index 93e80735d..e240ce6a4 100644 --- a/workspace-server/internal/handlers/workspace_provision_concurrent_repro_test.go +++ b/workspace-server/internal/handlers/workspace_provision_concurrent_repro_test.go @@ -10,9 +10,9 @@ import ( "sync/atomic" "testing" - "github.com/DATA-DOG/go-sqlmock" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/models" "git.moleculesai.app/molecule-ai/molecule-core/workspace-server/internal/provisioner" + "github.com/DATA-DOG/go-sqlmock" ) // Issue #2486 reproduction harness: 7 simultaneous claude-code provisions @@ -71,6 +71,9 @@ func (r *recordingCPProv) Start(_ context.Context, cfg provisioner.WorkspaceConf func (r *recordingCPProv) Stop(_ context.Context, _ string) error { panic("recordingCPProv.Stop not expected in concurrent-repro test") } +func (r *recordingCPProv) StopAndPrune(_ context.Context, _ string) error { + panic("recordingCPProv.StopAndPrune not expected in concurrent-repro test") +} func (r *recordingCPProv) GetConsoleOutput(_ context.Context, _ string) (string, error) { panic("recordingCPProv.GetConsoleOutput not expected in concurrent-repro test") diff --git a/workspace-server/internal/handlers/workspace_provision_test.go b/workspace-server/internal/handlers/workspace_provision_test.go index b76c3166a..4cd5b844e 100644 --- a/workspace-server/internal/handlers/workspace_provision_test.go +++ b/workspace-server/internal/handlers/workspace_provision_test.go @@ -1399,6 +1399,9 @@ func (s *stubFailingCPProv) Start(_ context.Context, _ provisioner.WorkspaceConf func (s *stubFailingCPProv) Stop(_ context.Context, _ string) error { panic("stubFailingCPProv.Stop not expected on the provisionWorkspaceCP failure path") } +func (s *stubFailingCPProv) StopAndPrune(_ context.Context, _ string) error { + panic("stubFailingCPProv.StopAndPrune not expected on the provisionWorkspaceCP failure path") +} func (s *stubFailingCPProv) GetConsoleOutput(_ context.Context, _ string) (string, error) { panic("stubFailingCPProv.GetConsoleOutput not expected on the provisionWorkspaceCP failure path") diff --git a/workspace-server/internal/handlers/workspace_restart.go b/workspace-server/internal/handlers/workspace_restart.go index 8bae4ef61..04992b06b 100644 --- a/workspace-server/internal/handlers/workspace_restart.go +++ b/workspace-server/internal/handlers/workspace_restart.go @@ -726,7 +726,7 @@ func (h *WorkspaceHandler) cpStopWithRetry(ctx context.Context, workspaceID, sou // terminal error. The delete path needs the error (it must keep the // row recoverable for the orphan-sweeper + emit a durable event), so // the actual retry loop lives in cpStopWithRetryErr below. - _ = h.cpStopWithRetryErr(ctx, workspaceID, source) + _ = h.cpStopWithRetryErr(ctx, workspaceID, source, false) // restart/hibernate never prunes } // cpStopWithRetryErr is the shared bounded-retry core for cpProv.Stop. @@ -743,14 +743,24 @@ func (h *WorkspaceHandler) cpStopWithRetry(ctx context.Context, workspaceID, sou // - all attempts fail → returns the LAST attempt's error and emits the // stable `LEAK-SUSPECT cpProv.Stop ...` log line so the CP-side orphan // reconciler can correlate by workspace_id. -func (h *WorkspaceHandler) cpStopWithRetryErr(ctx context.Context, workspaceID, source string) error { +// +// cpStopWithRetryErr terminates the workspace's CP-managed compute with bounded +// retry. prune=true (internal#734) additionally requests CP erase the durable +// data volume — set ONLY by the permanent-delete-with-erase path, NEVER by +// restart/hibernate (those pass false), so a recreate can never prune. +func (h *WorkspaceHandler) cpStopWithRetryErr(ctx context.Context, workspaceID, source string, prune bool) error { if h.cpProv == nil { return nil } var lastErr error delay := cpStopRetryBaseDelay for attempt := 1; attempt <= cpStopRetryAttempts; attempt++ { - err := h.cpProv.Stop(ctx, workspaceID) + var err error + if prune { + err = h.cpProv.StopAndPrune(ctx, workspaceID) + } else { + err = h.cpProv.Stop(ctx, workspaceID) + } if err == nil { if attempt > 1 { log.Printf("%s: cpProv.Stop(%s) succeeded on attempt %d", source, workspaceID, attempt) diff --git a/workspace-server/internal/handlers/workspace_restart_stop_retry_test.go b/workspace-server/internal/handlers/workspace_restart_stop_retry_test.go index 0c7dcf4c6..bd2bfeda2 100644 --- a/workspace-server/internal/handlers/workspace_restart_stop_retry_test.go +++ b/workspace-server/internal/handlers/workspace_restart_stop_retry_test.go @@ -72,6 +72,13 @@ func (s *scriptedCPStop) Stop(ctx context.Context, _ string) error { } return nil } + +// StopAndPrune delegates to Stop so the retry/error scripting is identical — +// the prune flag only changes the URL the real provisioner builds, not the +// retry behavior these tests exercise. +func (s *scriptedCPStop) StopAndPrune(ctx context.Context, id string) error { + return s.Stop(ctx, id) +} func (s *scriptedCPStop) Start(_ context.Context, _ provisioner.WorkspaceConfig) (string, error) { return "", nil } diff --git a/workspace-server/internal/provisioner/cp_provisioner.go b/workspace-server/internal/provisioner/cp_provisioner.go index 88e3c6bc2..93ee0fe48 100644 --- a/workspace-server/internal/provisioner/cp_provisioner.go +++ b/workspace-server/internal/provisioner/cp_provisioner.go @@ -32,6 +32,9 @@ import ( type CPProvisionerAPI interface { Start(ctx context.Context, cfg WorkspaceConfig) (string, error) Stop(ctx context.Context, workspaceID string) error + // StopAndPrune is Stop + "erase the durable data volume" (internal#734), + // for the permanent-delete-with-erase flow ONLY. Restart/recreate use Stop. + StopAndPrune(ctx context.Context, workspaceID string) error GetConsoleOutput(ctx context.Context, workspaceID string) (string, error) // IsRunning reports whether the workspace's compute (EC2 instance) is // currently in the running state. Surfaced on the interface (rather than @@ -403,6 +406,20 @@ func collectCPConfigFiles(cfg WorkspaceConfig) (map[string]string, error) { // blocking the next provision with InvalidGroup.Duplicate — a full // "Save & Restart" crash on SaaS. func (p *CPProvisioner) Stop(ctx context.Context, workspaceID string) error { + return p.stopInternal(ctx, workspaceID, false) +} + +// StopAndPrune terminates the workspace's compute AND requests that its durable +// data volume (browser profile / cookies / downloads / agent memory) be erased +// (internal#734). Used ONLY by the permanent-delete flow when the user chose to +// erase saved data — NEVER by restart/recreate (which call Stop), so a recreate +// can never trigger a prune. CP enforces this defensively too (the prune is a +// short-grace mark-then-sweep gated on the workspace being genuinely gone). +func (p *CPProvisioner) StopAndPrune(ctx context.Context, workspaceID string) error { + return p.stopInternal(ctx, workspaceID, true) +} + +func (p *CPProvisioner) stopInternal(ctx context.Context, workspaceID string, prune bool) error { if p == nil { return ErrNoBackend } @@ -425,6 +442,10 @@ func (p *CPProvisioner) Stop(ctx context.Context, workspaceID string) error { return ErrNoBackend } url := fmt.Sprintf("%s/cp/workspaces/%s?instance_id=%s", p.baseURL, workspaceID, instanceID) + if prune { + // internal#734: ask CP to erase the data volume on this delete. + url += "&prune=true" + } req, err := http.NewRequestWithContext(ctx, "DELETE", url, nil) if err != nil { return fmt.Errorf("cp provisioner: stop: build request: %w", err) diff --git a/workspace-server/internal/provisioner/cp_provisioner_test.go b/workspace-server/internal/provisioner/cp_provisioner_test.go index 35af6ca6c..75b366b22 100644 --- a/workspace-server/internal/provisioner/cp_provisioner_test.go +++ b/workspace-server/internal/provisioner/cp_provisioner_test.go @@ -1096,3 +1096,41 @@ func TestCollectCPConfigFiles_RejectsRootSymlink(t *testing.T) { t.Errorf("expected symlink-related error, got: %v", err) } } + +// internal#734 (F1): the prune signal must reach CP ONLY via StopAndPrune. +// Stop must NEVER append prune=true (restart/recreate use Stop, and a prune on +// a recreate = data loss). Captures the DELETE URL and asserts the contract. +func TestStopVsStopAndPrune_PruneQueryParam(t *testing.T) { + primeInstanceIDLookup(t, map[string]string{"ws-keep": "i-keep", "ws-erase": "i-erase"}) + + var gotURL string + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + gotURL = r.URL.String() + w.WriteHeader(http.StatusOK) + _, _ = io.WriteString(w, `{"status":"terminated"}`) + })) + defer srv.Close() + p := &CPProvisioner{baseURL: srv.URL, orgID: "org-1", httpClient: srv.Client()} + + // Stop → no prune. + if err := p.Stop(context.Background(), "ws-keep"); err != nil { + t.Fatalf("Stop: %v", err) + } + if strings.Contains(gotURL, "prune=true") { + t.Errorf("Stop must NOT send prune=true (recreate-safety); url=%s", gotURL) + } + if !strings.Contains(gotURL, "instance_id=i-keep") { + t.Errorf("Stop url missing instance_id; url=%s", gotURL) + } + + // StopAndPrune → prune=true. + if err := p.StopAndPrune(context.Background(), "ws-erase"); err != nil { + t.Fatalf("StopAndPrune: %v", err) + } + if !strings.Contains(gotURL, "prune=true") { + t.Errorf("StopAndPrune MUST send prune=true; url=%s", gotURL) + } + if !strings.Contains(gotURL, "instance_id=i-erase") { + t.Errorf("StopAndPrune url missing instance_id; url=%s", gotURL) + } +} -- 2.52.0 From 257a61672bf8d23e4aecf74e43dfb06c9cc75245 Mon Sep 17 00:00:00 2001 From: hongming-ceo-delegated Date: Sat, 30 May 2026 09:22:20 -0700 Subject: [PATCH 3/3] feat(canvas): per-workspace data persistence + erase-on-delete UI (internal#734) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The user-facing choice for the prune/persistence backend: - ContainerConfigTab: a 'Saved data' selector (Auto / Always keep / Don't keep) → compute.data_persistence (omitted when Auto = unchanged wire/default). - DetailsTab delete: an 'also erase saved data' checkbox → DELETE ?erase_data=true (default off keeps it for the orphan-sweeper grace). - WorkspaceCompute.data_persistence type. +test: erase checkbox sends erase_data=true; default delete unchanged. The 37 ContainerConfigTab+DetailsTab tests pass; my files typecheck clean. Co-Authored-By: Claude Opus 4.8 (1M context) --- .../components/tabs/ContainerConfigTab.tsx | 28 +++++++++++++++++-- canvas/src/components/tabs/DetailsTab.tsx | 20 ++++++++++++- .../tabs/__tests__/DetailsTab.test.tsx | 19 +++++++++++++ canvas/src/store/socket.ts | 3 ++ 4 files changed, 67 insertions(+), 3 deletions(-) diff --git a/canvas/src/components/tabs/ContainerConfigTab.tsx b/canvas/src/components/tabs/ContainerConfigTab.tsx index f8a3259c0..24813e1a8 100644 --- a/canvas/src/components/tabs/ContainerConfigTab.tsx +++ b/canvas/src/components/tabs/ContainerConfigTab.tsx @@ -29,8 +29,15 @@ type FormState = { displayMode: string; displayProtocol: string; resolution: string; + dataPersistence: string; // "" (auto) | "persist" | "ephemeral" — internal#734 }; +// internal#734: per-workspace durable-data choice. "" = auto (desktop-control +// keeps data, others follow the org default). Human labels for the selector. +const DATA_PERSISTENCE_OPTIONS = ["", "persist", "ephemeral"]; +const dataPersistenceLabel = (v: string): string => + v === "persist" ? "Always keep (persist)" : v === "ephemeral" ? "Don't keep (ephemeral)" : "Auto"; + export function ContainerConfigTab({ workspaceId, data }: Props) { const runtime = data.runtime; const instanceType = data.compute?.instance_type; @@ -39,9 +46,10 @@ export function ContainerConfigTab({ workspaceId, data }: Props) { const displayProtocol = data.compute?.display?.protocol; const displayWidth = data.compute?.display?.width; const displayHeight = data.compute?.display?.height; + const dataPersistence = data.compute?.data_persistence; const initial = useMemo( - () => formFromData({ runtime, instanceType, rootGB, displayMode, displayProtocol, displayWidth, displayHeight }), - [runtime, instanceType, rootGB, displayMode, displayProtocol, displayWidth, displayHeight], + () => formFromData({ runtime, instanceType, rootGB, displayMode, displayProtocol, displayWidth, displayHeight, dataPersistence }), + [runtime, instanceType, rootGB, displayMode, displayProtocol, displayWidth, displayHeight, dataPersistence], ); const [form, setForm] = useState(initial); const [saving, setSaving] = useState(false); @@ -84,6 +92,8 @@ export function ContainerConfigTab({ workspaceId, data }: Props) { display: form.displayEnabled ? { mode: form.displayMode, protocol: form.displayProtocol, width, height } : { mode: "none" }, + // internal#734: omit when "auto" so the wire/default behavior is unchanged. + ...(form.dataPersistence ? { data_persistence: form.dataPersistence } : {}), }; const resp = await api.patch<{ needs_restart?: boolean }>(`/workspaces/${workspaceId}`, { @@ -176,6 +186,18 @@ export function ContainerConfigTab({ workspaceId, data }: Props) { onChange={(resolution) => setForm((s) => ({ ...s, resolution }))} /> )} + setForm((s) => ({ ...s, dataPersistence }))} + /> +

+ Whether this workspace's data survives a restart/recreate. Auto keeps it for + browser (desktop) workspaces; Ephemeral never keeps it (privacy). +

@@ -231,6 +253,7 @@ function formFromData(data: { displayProtocol?: string; displayWidth?: number; displayHeight?: number; + dataPersistence?: string; }): FormState { const width = data.displayWidth ?? 1920; const height = data.displayHeight ?? 1080; @@ -243,6 +266,7 @@ function formFromData(data: { displayMode: data.displayMode && data.displayMode !== "none" ? data.displayMode : "desktop-control", displayProtocol: data.displayProtocol || "novnc", resolution, + dataPersistence: data.dataPersistence || "", }; } diff --git a/canvas/src/components/tabs/DetailsTab.tsx b/canvas/src/components/tabs/DetailsTab.tsx index 197015ecb..d39cb554d 100644 --- a/canvas/src/components/tabs/DetailsTab.tsx +++ b/canvas/src/components/tabs/DetailsTab.tsx @@ -29,6 +29,7 @@ export function DetailsTab({ workspaceId, data }: Props) { const [peers, setPeers] = useState([]); const [saving, setSaving] = useState(false); const [confirmDelete, setConfirmDelete] = useState(false); + const [eraseData, setEraseData] = useState(false); // internal#734: erase saved data on delete const [peersError, setPeersError] = useState(null); const [saveError, setSaveError] = useState(null); const [deleteError, setDeleteError] = useState(null); @@ -93,7 +94,10 @@ export function DetailsTab({ workspaceId, data }: Props) { const handleDelete = async () => { setDeleteError(null); try { - await api.del(`/workspaces/${workspaceId}?confirm=true`, { + // internal#734: erase_data=true asks the server to prune this workspace's + // durable data volume (cookies / downloads / memory). Default off keeps it + // for the orphan-sweeper grace. + await api.del(`/workspaces/${workspaceId}?confirm=true${eraseData ? "&erase_data=true" : ""}`, { headers: { "X-Confirm-Name": name }, }); // Mirror the server-side cascade — drop the row + every @@ -323,6 +327,19 @@ export function DetailsTab({ workspaceId, data }: Props) {

Confirm deletion

+