diff --git a/workspace-server/internal/handlers/workspace.go b/workspace-server/internal/handlers/workspace.go index 2f640d77..f6fef476 100644 --- a/workspace-server/internal/handlers/workspace.go +++ b/workspace-server/internal/handlers/workspace.go @@ -66,6 +66,12 @@ type WorkspaceHandler struct { // template manifests (#2054 phase 2). Lazy-init on first scan; see // runtime_provision_timeouts.go for the loader contract. provisionTimeouts runtimeProvisionTimeoutsCache + // namespaceCleanupFn is the I5 (RFC #2728) hook called best-effort + // during purge to delete the workspace's plugin-side namespace. + // nil = no-op (default for operators who haven't wired the v2 + // memory plugin). main.go sets this to plugin.DeleteNamespace + // when MEMORY_PLUGIN_URL is configured. + namespaceCleanupFn func(ctx context.Context, workspaceID string) } func NewWorkspaceHandler(b events.EventEmitter, p *provisioner.Provisioner, platformURL, configsDir string) *WorkspaceHandler { @@ -87,6 +93,16 @@ func NewWorkspaceHandler(b events.EventEmitter, p *provisioner.Provisioner, plat return h } +// WithNamespaceCleanup wires the I5 hook (RFC #2728) so workspace +// purge can drop the plugin's `workspace:` namespace. main.go +// passes a closure over plugin.DeleteNamespace; tests pass a stub. +// Nil-safe: omitting this leaves namespaceCleanupFn nil, which the +// purge path treats as a no-op. +func (h *WorkspaceHandler) WithNamespaceCleanup(fn func(ctx context.Context, workspaceID string)) *WorkspaceHandler { + h.namespaceCleanupFn = fn + return h +} + // SetCPProvisioner wires the control plane provisioner for SaaS tenants. // Auto-activated when MOLECULE_ORG_ID is set (no manual config needed). // diff --git a/workspace-server/internal/handlers/workspace_crud.go b/workspace-server/internal/handlers/workspace_crud.go index d3e5354a..f254ea86 100644 --- a/workspace-server/internal/handlers/workspace_crud.go +++ b/workspace-server/internal/handlers/workspace_crud.go @@ -507,6 +507,22 @@ func (h *WorkspaceHandler) Delete(c *gin.Context) { c.JSON(http.StatusInternalServerError, gin.H{"error": "purge failed"}) return } + + // I5 (RFC #2728): best-effort plugin namespace cleanup. If + // MEMORY_V2 is wired, ask the plugin to drop each purged + // workspace's `workspace:` namespace so stale namespaces + // don't accumulate. We deliberately do NOT clean up team:* / + // org:* namespaces — those may still be referenced by other + // workspaces under the same root. + // + // Failures are logged but don't fail the purge (which has + // already succeeded against the workspaces table). + if h.namespaceCleanupFn != nil { + for _, id := range allIDs { + h.namespaceCleanupFn(ctx, id) + } + } + c.JSON(http.StatusOK, gin.H{"status": "purged", "cascade_deleted": len(descendantIDs)}) return } diff --git a/workspace-server/internal/handlers/workspace_namespace_cleanup_test.go b/workspace-server/internal/handlers/workspace_namespace_cleanup_test.go new file mode 100644 index 00000000..18abd149 --- /dev/null +++ b/workspace-server/internal/handlers/workspace_namespace_cleanup_test.go @@ -0,0 +1,92 @@ +package handlers + +// Pins the I5 fix (RFC #2728): workspace purge MUST call the plugin's +// DeleteNamespace for each affected workspace so the plugin's +// `workspace:` namespace doesn't leak. + +import ( + "context" + "sync" + "testing" +) + +// captureCleanupHook records every workspace id passed to the hook. +type captureCleanupHook struct { + mu sync.Mutex + calls []string +} + +func (c *captureCleanupHook) fn(_ context.Context, workspaceID string) { + c.mu.Lock() + defer c.mu.Unlock() + c.calls = append(c.calls, workspaceID) +} + +func TestWithNamespaceCleanup_DefaultIsNil(t *testing.T) { + h := &WorkspaceHandler{} + if h.namespaceCleanupFn != nil { + t.Errorf("default namespaceCleanupFn must be nil") + } +} + +func TestWithNamespaceCleanup_NilStaysNil(t *testing.T) { + out := (&WorkspaceHandler{}).WithNamespaceCleanup(nil) + if out.namespaceCleanupFn != nil { + t.Errorf("explicit nil must remain nil (no-op default preserved)") + } +} + +func TestWithNamespaceCleanup_AttachesFn(t *testing.T) { + called := false + h := (&WorkspaceHandler{}).WithNamespaceCleanup(func(_ context.Context, _ string) { + called = true + }) + if h.namespaceCleanupFn == nil { + t.Fatal("WithNamespaceCleanup must attach the fn") + } + h.namespaceCleanupFn(context.Background(), "ws-1") + if !called { + t.Errorf("hook not invoked") + } +} + +// TestPurge_CallsCleanupHookPerID covers the per-id loop the purge +// path uses. We exercise the loop directly here because a full +// end-to-end Delete-handler test requires mocking broadcaster + +// provisioner + descendant-query SQL — too much surface for the +// scope of this fixup. The integration coverage lives in PR-11's +// E2E swap test (which exercises the full handler chain against a +// stub plugin). +func TestPurge_CallsCleanupHookPerID(t *testing.T) { + hook := &captureCleanupHook{} + h := (&WorkspaceHandler{}).WithNamespaceCleanup(hook.fn) + + // Mirror the loop body in workspace_crud.go's purge branch. + allIDs := []string{"ws-root", "ws-child-1", "ws-child-2"} + if h.namespaceCleanupFn != nil { + for _, id := range allIDs { + h.namespaceCleanupFn(context.Background(), id) + } + } + if len(hook.calls) != 3 { + t.Fatalf("expected 3 cleanup calls, got %d (%v)", len(hook.calls), hook.calls) + } + for i, want := range allIDs { + if hook.calls[i] != want { + t.Errorf("call %d: got %q, want %q", i, hook.calls[i], want) + } + } +} + +func TestPurge_NilHookIsSkipped(t *testing.T) { + h := &WorkspaceHandler{} // hook never set + allIDs := []string{"ws-1", "ws-2"} + // Mirrors the actual purge body's nil guard. If this panics, the + // production guard is wrong. + if h.namespaceCleanupFn != nil { + for _, id := range allIDs { + h.namespaceCleanupFn(context.Background(), id) + } + } + // Reaches here without panicking — that's the assertion. +}