From eea64f06ec2d381ec67ab6f5d487927aab42a42c Mon Sep 17 00:00:00 2001 From: Hongming Wang Date: Tue, 14 Apr 2026 13:21:23 -0700 Subject: [PATCH] fix(org): per-workspace plugins UNION with defaults; '!' prefix opts out (#68) Per-workspace `plugins:` now UNIONS with `defaults.plugins` instead of replacing. A leading `!` or `-` on a per-workspace entry opts a default out. Backward-compatible: re-listing defaults still dedupes to the same list. Refactored the inline REPLACE logic into a pure helper `mergePlugins` in org.go so it's unit-testable. Five TestPlugins_* cases added. Co-Authored-By: Claude Opus 4.6 (1M context) --- platform/internal/handlers/org.go | 51 +++++++++++++++++++++--- platform/internal/handlers/org_test.go | 54 ++++++++++++++++++++++++++ 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/platform/internal/handlers/org.go b/platform/internal/handlers/org.go index 98ae26b7..77973edb 100644 --- a/platform/internal/handlers/org.go +++ b/platform/internal/handlers/org.go @@ -340,11 +340,10 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, defa } } - // Pre-install plugins: copy from registry into configFiles as plugins//* - plugins := ws.Plugins - if len(plugins) == 0 { - plugins = defaults.Plugins - } + // Pre-install plugins: copy from registry into configFiles as plugins//*. + // Per-workspace plugins UNION with defaults.plugins (issue #68). + // A leading "!" or "-" on a per-workspace entry opts that plugin out. + plugins := mergePlugins(defaults.Plugins, ws.Plugins) if len(plugins) > 0 { if configFiles == nil { configFiles = map[string][]byte{} @@ -642,3 +641,45 @@ func parseEnvFile(path string, out map[string]string) { } } } + +// mergePlugins returns the union of defaults and per-workspace plugin lists +// (deduplicated, defaults first). A per-workspace entry starting with "!" or +// "-" opts that plugin OUT of the union. See issue #68. +func mergePlugins(defaultPlugins, wsPlugins []string) []string { + seen := map[string]bool{} + out := make([]string, 0, len(defaultPlugins)+len(wsPlugins)) + for _, p := range defaultPlugins { + if p == "" || seen[p] { + continue + } + seen[p] = true + out = append(out, p) + } + for _, p := range wsPlugins { + if p == "" { + continue + } + if strings.HasPrefix(p, "!") || strings.HasPrefix(p, "-") { + target := strings.TrimLeft(p, "!-") + if target == "" { + continue + } + if seen[target] { + delete(seen, target) + filtered := out[:0] + for _, existing := range out { + if existing != target { + filtered = append(filtered, existing) + } + } + out = filtered + } + continue + } + if !seen[p] { + seen[p] = true + out = append(out, p) + } + } + return out +} diff --git a/platform/internal/handlers/org_test.go b/platform/internal/handlers/org_test.go index 111f57d7..df9dd295 100644 --- a/platform/internal/handlers/org_test.go +++ b/platform/internal/handlers/org_test.go @@ -383,3 +383,57 @@ func TestHasUnresolvedVarRef_DollarVarSyntax(t *testing.T) { t.Error("$VAR syntax should be detected as ref when unresolved") } } + +func eqStringSlice(a, b []string) bool { + if len(a) != len(b) { + return false + } + for i := range a { + if a[i] != b[i] { + return false + } + } + return true +} + +func TestPlugins_UnionWithDefaults(t *testing.T) { + got := mergePlugins([]string{"a", "b"}, []string{"c"}) + want := []string{"a", "b", "c"} + if !eqStringSlice(got, want) { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestPlugins_DedupesDuplicates(t *testing.T) { + got := mergePlugins([]string{"a", "b"}, []string{"b", "c"}) + want := []string{"a", "b", "c"} + if !eqStringSlice(got, want) { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestPlugins_OptOutWithBang(t *testing.T) { + got := mergePlugins([]string{"a", "b", "c"}, []string{"!b", "d"}) + want := []string{"a", "c", "d"} + if !eqStringSlice(got, want) { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestPlugins_OptOutWithDash(t *testing.T) { + got := mergePlugins([]string{"a", "b"}, []string{"-a"}) + want := []string{"b"} + if !eqStringSlice(got, want) { + t.Fatalf("got %v, want %v", got, want) + } +} + +func TestPlugins_BackwardCompat(t *testing.T) { + // Re-listing defaults in per-workspace plugins still yields the same list + // (dedupe keeps behavior stable for existing org.yaml files). + got := mergePlugins([]string{"a", "b"}, []string{"a", "b", "c"}) + want := []string{"a", "b", "c"} + if !eqStringSlice(got, want) { + t.Fatalf("got %v, want %v", got, want) + } +}