Merge pull request #71 from Molecule-AI/fix/issue-68-plugins-union

Merged after 7-gate verification.

Gates: 1 (CI 6/6 + 1 skip) pass, 2 (build/vet) pass, 3 (5 new TestPlugins_* + backward-compat) pass, 4 (security) pass, 5 (design) pass with 1 yellow, 6 (line review) pass, 7 N/A.

Backward-compat verified: molecule-dev/org.yaml re-lists [ecc, molecule-dev, superpowers, browser-automation] in each role; under new UNION+dedupe the merged set is identical to the prior REPLACE result. PR #70's 1 yellow (REPLACE verbosity / re-listing chore) is now closed by this change — orgs can drop the re-listing once confident.

Cross-vendor-review: second-model tooling unavailable in this worktree; Claude-only review applied per standing rule fallback.

Yellow (non-blocking, follow-up): opt-out semantics (`!plugin` / `-plugin`) are documented only in the code comment. Safety plugins like `molecule-careful-bash` can be disabled by an org.yaml using `!molecule-careful-bash` — this is operator-controlled config per I-2 and therefore acceptable, but docs/plugins/ should get an "overriding defaults" page in a follow-up.

noteworthy: plugin-semantics-change
This commit is contained in:
Hongming Wang 2026-04-14 13:42:30 -07:00 committed by GitHub
commit dd61714c55
2 changed files with 100 additions and 5 deletions

View File

@ -340,11 +340,10 @@ func (h *OrgHandler) createWorkspaceTree(ws OrgWorkspace, parentID *string, defa
}
}
// Pre-install plugins: copy from registry into configFiles as plugins/<name>/*
plugins := ws.Plugins
if len(plugins) == 0 {
plugins = defaults.Plugins
}
// Pre-install plugins: copy from registry into configFiles as plugins/<name>/*.
// 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
}

View File

@ -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)
}
}