Merge pull request #2750 from Molecule-AI/fix/memory-v2-i5-namespace-cleanup

Memory v2 fixup I5: workspace purge cleans up plugin namespace

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:<id>` 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).
 //

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:<id>` 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
 	}

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:<id>` 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.
+}