From 5802229fdcbcd329cf092fd9cbf7cee5543ee4c4 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Sat, 17 Oct 2026 17:12:16 +0530 Subject: [PATCH 1/4] feat(plugins): add CLI enable/disable for plugin.json Add zero plugins enable|disable so installed plugins can be toggled without removing them, matching hooks/MCP parity and EXTENDING docs. --- docs/EXTENDING.md | 4 +- internal/cli/extensions.go | 98 ++++++++++++++++++++ internal/cli/extensions_test.go | 121 ++++++++++++++++++++++++ internal/plugins/enabled.go | 114 +++++++++++++++++++++++ internal/plugins/enabled_test.go | 152 +++++++++++++++++++++++++++++++ 5 files changed, 488 insertions(+), 1 deletion(-) create mode 100644 internal/plugins/enabled.go create mode 100644 internal/plugins/enabled_test.go diff --git a/docs/EXTENDING.md b/docs/EXTENDING.md index a2906f39e..38b49303b 100644 --- a/docs/EXTENDING.md +++ b/docs/EXTENDING.md @@ -347,10 +347,12 @@ Install and manage: zero plugins add ./github-pr-review # copy into ~/.config/zero/plugins/ or ./.zero/plugins/ zero plugins list zero plugins info github-pr-review # show manifest path, enabled state, lock metadata +zero plugins disable github-pr-review # sets "enabled": false in plugin.json +zero plugins enable github-pr-review # sets "enabled": true in plugin.json zero plugins remove github-pr-review # alias: rm ``` -A plugin is enabled by being present in the plugins directory and disabled by removing it (or by the user setting `"enabled": false` in its `plugin.json`). Plugins are not enabled or disabled by a CLI subcommand today. +A plugin is active when it is present under a plugins directory and its `plugin.json` has `"enabled": true` (the default when the field is omitted). Use `zero plugins disable ` / `zero plugins enable ` to toggle that field without removing the plugin. Pass `--user` to target only user plugins and ignore `./.zero/plugins`. Plugin commands run with the plugin directory as their working directory. Use relative paths; the loader resolves them at activation time. diff --git a/internal/cli/extensions.go b/internal/cli/extensions.go index c3bb81a4e..c50f55c6f 100644 --- a/internal/cli/extensions.go +++ b/internal/cli/extensions.go @@ -2,6 +2,7 @@ package cli import ( "context" + "errors" "fmt" "io" "strings" @@ -82,11 +83,94 @@ func runPlugins(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) return runPluginInfo(args[1:], stdout, stderr, deps) case "remove", "rm": return runPluginRemove(args[1:], deps.pluginsDir(), stdout, stderr) + case "enable": + return runPluginsToggle(args[1:], stdout, stderr, deps, false) + case "disable": + return runPluginsToggle(args[1:], stdout, stderr, deps, true) default: return writeExecUsageError(stderr, fmt.Sprintf("unknown plugins subcommand %q", args[0])) } } +type pluginToggleOptions struct { + json bool + user bool +} + +func runPluginsToggle(args []string, stdout io.Writer, stderr io.Writer, deps appDeps, disabled bool) int { + commandName := "enable" + if disabled { + commandName = "disable" + } + options, positional, help, err := parsePluginToggleArgs(args, commandName) + if err != nil { + return writeExecUsageError(stderr, err.Error()) + } + if help { + if err := writePluginsToggleHelp(stdout, commandName); err != nil { + return exitCrash + } + return exitSuccess + } + if len(positional) != 1 { + return writeExecUsageError(stderr, fmt.Sprintf("usage: zero plugins %s [--user] [--json]", commandName)) + } + pluginID := positional[0] + + cwd, err := deps.getwd() + if err != nil { + return writeAppError(stderr, "failed to resolve workspace: "+err.Error(), exitCrash) + } + loadOptions := plugins.LoadOptions{Cwd: cwd, ExcludeProject: options.user} + result, err := plugins.SetEnabledByID(loadOptions, pluginID, !disabled) + if err != nil { + if errors.Is(err, plugins.ErrNotInstalled) { + return writeExecUsageError(stderr, err.Error()) + } + return writeAppError(stderr, redaction.ErrorMessage(err, redaction.Options{}), exitCrash) + } + + state := "enabled" + if disabled { + state = "disabled" + } + if options.json { + if err := writePrettyJSON(stdout, redaction.RedactValue(result, redaction.Options{})); err != nil { + return exitCrash + } + return exitSuccess + } + if result.Changed { + if _, err := fmt.Fprintf(stdout, "Plugin %s is now %s in %s.\n", result.ID, state, result.ManifestPath); err != nil { + return exitCrash + } + } else if _, err := fmt.Fprintf(stdout, "Plugin %s was already %s in %s.\n", result.ID, state, result.ManifestPath); err != nil { + return exitCrash + } + return exitSuccess +} + +func parsePluginToggleArgs(args []string, command string) (pluginToggleOptions, []string, bool, error) { + options := pluginToggleOptions{} + positional := []string{} + for _, arg := range args { + switch arg { + case "-h", "--help", "help": + return options, positional, true, nil + case "--json": + options.json = true + case "--user": + options.user = true + default: + if strings.HasPrefix(arg, "-") { + return options, positional, false, execUsageError{fmt.Sprintf("unknown plugins %s flag %q", command, arg)} + } + positional = append(positional, arg) + } + } + return options, positional, false, nil +} + func runHooks(args []string, stdout io.Writer, stderr io.Writer, deps appDeps) int { if len(args) == 0 { return writeExecUsageError(stderr, "hooks subcommand required. Use `zero hooks list`.") @@ -566,10 +650,24 @@ Commands: info Show plugin details and lockfile metadata add Install a plugin (manifest-validated, pinned in plugins.lock) remove Remove an installed plugin and its lockfile entry + enable Enable a plugin via its plugin.json + disable Disable a plugin via its plugin.json `) return err } +func writePluginsToggleHelp(w io.Writer, command string) error { + _, err := fmt.Fprintf(w, `Usage: + zero plugins %s [flags] + +Flags: + --user Target only user plugins (ignore ./.zero/plugins) + --json Print command result as JSON + -h, --help Show this help +`, command) + return err +} + func writePluginsListHelp(w io.Writer) error { _, err := fmt.Fprint(w, `Usage: zero plugins list [flags] diff --git a/internal/cli/extensions_test.go b/internal/cli/extensions_test.go index 8b613fd3e..e6e531b93 100644 --- a/internal/cli/extensions_test.go +++ b/internal/cli/extensions_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "errors" + "os" "path/filepath" "strings" "testing" @@ -333,3 +334,123 @@ func TestRunMCPPermissionsHelpDoesNotOpenStore(t *testing.T) { }) } } + +func TestRunPluginsEnableDisable(t *testing.T) { + cwd := t.TempDir() + pluginDir := filepath.Join(cwd, ".zero", "plugins", "demo") + if err := os.MkdirAll(pluginDir, 0o700); err != nil { + t.Fatal(err) + } + manifestPath := filepath.Join(pluginDir, "plugin.json") + if err := os.WriteFile(manifestPath, []byte(`{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0" +} +`), 0o600); err != nil { + t.Fatal(err) + } + + deps := appDeps{getwd: func() (string, error) { return cwd, nil }} + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"plugins", "disable", "zero.demo", "--json"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("disable exit=%d stderr=%s", exitCode, stderr.String()) + } + var disablePayload plugins.SetEnabledResult + if err := json.Unmarshal(stdout.Bytes(), &disablePayload); err != nil { + t.Fatalf("decode disable JSON: %v\n%s", err, stdout.String()) + } + if disablePayload.ID != "zero.demo" || disablePayload.Enabled || !disablePayload.Changed { + t.Fatalf("unexpected disable payload: %#v", disablePayload) + } + + raw, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(raw), `"enabled": false`) { + t.Fatalf("manifest not disabled: %s", raw) + } + + stdout.Reset() + stderr.Reset() + exitCode = runWithDeps([]string{"plugins", "disable", "zero.demo"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("noop disable exit=%d stderr=%s", exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "was already disabled") { + t.Fatalf("noop disable output: %s", stdout.String()) + } + + stdout.Reset() + stderr.Reset() + exitCode = runWithDeps([]string{"plugins", "enable", "zero.demo"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("enable exit=%d stderr=%s", exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "is now enabled") { + t.Fatalf("enable output: %s", stdout.String()) + } + + stdout.Reset() + stderr.Reset() + exitCode = runWithDeps([]string{"plugins", "enable", "missing-plugin"}, &stdout, &stderr, deps) + if exitCode != exitUsage { + t.Fatalf("missing plugin exit=%d want %d stderr=%s", exitCode, exitUsage, stderr.String()) + } +} + +func TestRunPluginsDisableUserIgnoresProject(t *testing.T) { + home := t.TempDir() + cwd := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", home) + t.Setenv("HOME", home) + + userManifest := filepath.Join(home, "zero", "plugins", "demo", "plugin.json") + projectManifest := filepath.Join(cwd, ".zero", "plugins", "demo", "plugin.json") + for _, path := range []string{userManifest, projectManifest} { + if err := os.MkdirAll(filepath.Dir(path), 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, []byte(`{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0", + "enabled": true +} +`), 0o600); err != nil { + t.Fatal(err) + } + } + + deps := appDeps{getwd: func() (string, error) { return cwd, nil }} + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"plugins", "disable", "zero.demo", "--user"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("disable --user exit=%d stderr=%s", exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "is now disabled") { + t.Fatalf("disable --user output: %s", stdout.String()) + } + + userRaw, err := os.ReadFile(userManifest) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(userRaw), `"enabled": false`) { + t.Fatalf("user manifest not disabled: %s", userRaw) + } + projectRaw, err := os.ReadFile(projectManifest) + if err != nil { + t.Fatal(err) + } + if strings.Contains(string(projectRaw), `"enabled": false`) { + t.Fatalf("project manifest should stay enabled: %s", projectRaw) + } +} diff --git a/internal/plugins/enabled.go b/internal/plugins/enabled.go new file mode 100644 index 000000000..f3b4b357b --- /dev/null +++ b/internal/plugins/enabled.go @@ -0,0 +1,114 @@ +package plugins + +import ( + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "strings" + "time" +) + +// SetEnabledResult describes a successful enable/disable of one plugin manifest. +type SetEnabledResult struct { + ID string `json:"id"` + Enabled bool `json:"enabled"` + Changed bool `json:"changed"` + Source Source `json:"source"` + ManifestPath string `json:"manifestPath"` +} + +// FindByID returns the loaded plugin with the given id after normal discovery +// and precedence (project overrides user). A missing id returns ok=false. +func FindByID(options LoadOptions, id string) (LoadedPlugin, bool, error) { + id = strings.TrimSpace(id) + if id == "" { + return LoadedPlugin{}, false, errors.New("plugin id is required") + } + result, err := Load(options) + if err != nil { + return LoadedPlugin{}, false, err + } + for _, plugin := range result.Plugins { + if plugin.ID == id { + return plugin, true, nil + } + } + return LoadedPlugin{}, false, nil +} + +// SetEnabled updates the "enabled" field in plugin.json, preserving the rest of +// the manifest. It returns whether the file actually changed. +func SetEnabled(manifestPath string, enabled bool) (changed bool, err error) { + manifestPath = strings.TrimSpace(manifestPath) + if manifestPath == "" { + return false, errors.New("plugin manifest path is required") + } + resolved, err := filepath.Abs(manifestPath) + if err != nil { + return false, err + } + + raw, err := os.ReadFile(resolved) + if err != nil { + return false, err + } + var obj map[string]any + if err := json.Unmarshal(raw, &obj); err != nil { + return false, fmt.Errorf("parse plugin manifest: %w", err) + } + if obj == nil { + return false, errors.New("plugin manifest must be a JSON object") + } + + previous, err := optionalBool(obj, "enabled", true) + if err != nil { + return false, err + } + if previous == enabled { + return false, nil + } + obj["enabled"] = enabled + + data, err := json.MarshalIndent(obj, "", " ") + if err != nil { + return false, err + } + mode := os.FileMode(0o644) + if info, statErr := os.Stat(resolved); statErr == nil { + mode = info.Mode().Perm() + } + tempPath := fmt.Sprintf("%s.tmp-%d-%d", resolved, os.Getpid(), time.Now().UnixNano()) + if err := os.WriteFile(tempPath, append(data, '\n'), mode); err != nil { + return false, err + } + if err := os.Rename(tempPath, resolved); err != nil { + _ = os.Remove(tempPath) + return false, err + } + return true, nil +} + +// SetEnabledByID finds a plugin by id (respecting LoadOptions) and toggles its +// manifest enabled field. +func SetEnabledByID(options LoadOptions, id string, enabled bool) (SetEnabledResult, error) { + plugin, ok, err := FindByID(options, id) + if err != nil { + return SetEnabledResult{}, err + } + if !ok { + return SetEnabledResult{}, fmt.Errorf("%w: %q", ErrNotInstalled, id) + } + changed, err := SetEnabled(plugin.ManifestPath, enabled) + if err != nil { + return SetEnabledResult{}, err + } + return SetEnabledResult{ + ID: plugin.ID, + Enabled: enabled, + Changed: changed, + Source: plugin.Source, + ManifestPath: plugin.ManifestPath, + }, nil +} diff --git a/internal/plugins/enabled_test.go b/internal/plugins/enabled_test.go new file mode 100644 index 000000000..340cd549b --- /dev/null +++ b/internal/plugins/enabled_test.go @@ -0,0 +1,152 @@ +package plugins + +import ( + "encoding/json" + "errors" + "os" + "path/filepath" + "runtime" + "testing" +) + +func TestSetEnabledTogglesManifestAndIsIdempotent(t *testing.T) { + root := t.TempDir() + pluginDir := filepath.Join(root, "demo") + writePluginManifest(t, pluginDir, map[string]any{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0", + "description": "keep me", + "tools": []any{}, + }) + manifestPath := filepath.Join(pluginDir, "plugin.json") + + changed, err := SetEnabled(manifestPath, false) + if err != nil { + t.Fatalf("SetEnabled(false): %v", err) + } + if !changed { + t.Fatal("expected first disable to change the manifest") + } + + raw, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + var obj map[string]any + if err := json.Unmarshal(raw, &obj); err != nil { + t.Fatalf("re-read manifest: %v", err) + } + if obj["enabled"] != false { + t.Fatalf("enabled = %#v, want false", obj["enabled"]) + } + if obj["description"] != "keep me" { + t.Fatalf("description was not preserved: %#v", obj["description"]) + } + + changed, err = SetEnabled(manifestPath, false) + if err != nil { + t.Fatalf("SetEnabled(false) again: %v", err) + } + if changed { + t.Fatal("expected second disable to be a no-op") + } + + changed, err = SetEnabled(manifestPath, true) + if err != nil { + t.Fatalf("SetEnabled(true): %v", err) + } + if !changed { + t.Fatal("expected enable to change the manifest") + } +} + +func TestSetEnabledByIDRespectsUserOnlyFilter(t *testing.T) { + dir := t.TempDir() + userRoot := filepath.Join(dir, "user") + projectRoot := filepath.Join(dir, "project") + writePluginManifest(t, filepath.Join(userRoot, "demo"), map[string]any{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "User Demo", + "version": "0.1.0", + "enabled": true, + }) + writePluginManifest(t, filepath.Join(projectRoot, "demo"), map[string]any{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Project Demo", + "version": "0.2.0", + "enabled": true, + }) + + result, err := SetEnabledByID(LoadOptions{ + Roots: []Root{ + {Source: SourceUser, Path: userRoot}, + {Source: SourceProject, Path: projectRoot}, + }, + }, "zero.demo", false) + if err != nil { + t.Fatalf("SetEnabledByID: %v", err) + } + if result.Source != SourceProject { + t.Fatalf("source = %s, want project (precedence)", result.Source) + } + if !result.Changed || result.Enabled { + t.Fatalf("unexpected result: %#v", result) + } + + userOnly, err := SetEnabledByID(LoadOptions{ + Roots: []Root{ + {Source: SourceUser, Path: userRoot}, + {Source: SourceProject, Path: projectRoot}, + }, + ExcludeProject: true, + }, "zero.demo", false) + if err != nil { + t.Fatalf("SetEnabledByID user-only: %v", err) + } + if userOnly.Source != SourceUser { + t.Fatalf("user-only source = %s, want user", userOnly.Source) + } +} + +func TestSetEnabledPreservesManifestMode(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows does not preserve chmod bits in os.Stat mode") + } + root := t.TempDir() + pluginDir := filepath.Join(root, "demo") + writePluginManifest(t, pluginDir, map[string]any{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0", + }) + manifestPath := filepath.Join(pluginDir, "plugin.json") + if err := os.Chmod(manifestPath, 0o644); err != nil { + t.Fatal(err) + } + + if _, err := SetEnabled(manifestPath, false); err != nil { + t.Fatalf("SetEnabled: %v", err) + } + info, err := os.Stat(manifestPath) + if err != nil { + t.Fatal(err) + } + if got := info.Mode().Perm(); got != 0o644 { + t.Fatalf("mode = %04o, want 0644", got) + } +} + +func TestSetEnabledByIDMissingPlugin(t *testing.T) { + _, err := SetEnabledByID(LoadOptions{Roots: []Root{{Source: SourceUser, Path: t.TempDir()}}}, "missing", false) + if err == nil { + t.Fatal("expected missing plugin error") + } + if !errors.Is(err, ErrNotInstalled) { + t.Fatalf("error = %v, want ErrNotInstalled", err) + } +} From c4cf774ef58384d0f239ed48f42e2d0269accba8 Mon Sep 17 00:00:00 2001 From: Ayush7614 Date: Sat, 17 Oct 2026 17:12:23 +0530 Subject: [PATCH 2/4] fix(plugins): serialize toggles with installs and preserve manifest bytes Address review findings on the enable/disable CLI: - Take the same cross-process installtxn lock plugins add/remove take on the plugin's install root before the final lookup and read, and publish plugin.json with installtxn.WriteFileAtomically while holding it. - Re-resolve the plugin under the lock so a concurrent install can never be overwritten with manifest bytes read before it committed, retrying when precedence moves to the other root; a plugin removed under the lock reports ErrNotInstalled instead of being recreated. - Edit the enabled value in place (a targeted token walk) so field order, formatting, and numeric literals survive; the old decode/re-encode dropped precision (9007199254740993 became 9007199254740992) and sorted the keys. - Fail closed on manifest stat errors, refresh a clean plugins.lock hash after a toggle, and leave pre-existing drift in place instead of absorbing it into the lockfile. - Redact the text-mode output, reject an empty plugin id as a usage error, and document project-over-user precedence in the help text. --- docs/EXTENDING.md | 2 +- internal/cli/extensions.go | 18 +- internal/cli/extensions_test.go | 73 ++++ internal/plugins/enabled.go | 355 ++++++++++++++++--- internal/plugins/enabled_test.go | 574 +++++++++++++++++++++++++++++-- internal/plugins/info.go | 3 +- 6 files changed, 939 insertions(+), 86 deletions(-) diff --git a/docs/EXTENDING.md b/docs/EXTENDING.md index 38b49303b..fec923f45 100644 --- a/docs/EXTENDING.md +++ b/docs/EXTENDING.md @@ -352,7 +352,7 @@ zero plugins enable github-pr-review # sets "enabled": true in plugin.json zero plugins remove github-pr-review # alias: rm ``` -A plugin is active when it is present under a plugins directory and its `plugin.json` has `"enabled": true` (the default when the field is omitted). Use `zero plugins disable ` / `zero plugins enable ` to toggle that field without removing the plugin. Pass `--user` to target only user plugins and ignore `./.zero/plugins`. +A plugin is active when it is present under a plugins directory and its `plugin.json` has `"enabled": true` (the default when the field is omitted). Use `zero plugins disable ` / `zero plugins enable ` to toggle that field without removing the plugin. Pass `--user` to target only user plugins and ignore `./.zero/plugins`; without it, a project plugin with the same id takes precedence. The toggle edits the `enabled` value in place — the rest of the manifest keeps its formatting, field order, and numeric literals — and serializes with `plugins add` / `plugins remove` on the same plugins root. Plugin commands run with the plugin directory as their working directory. Use relative paths; the loader resolves them at activation time. diff --git a/internal/cli/extensions.go b/internal/cli/extensions.go index c50f55c6f..fe8da1c53 100644 --- a/internal/cli/extensions.go +++ b/internal/cli/extensions.go @@ -115,7 +115,10 @@ func runPluginsToggle(args []string, stdout io.Writer, stderr io.Writer, deps ap if len(positional) != 1 { return writeExecUsageError(stderr, fmt.Sprintf("usage: zero plugins %s [--user] [--json]", commandName)) } - pluginID := positional[0] + pluginID := strings.TrimSpace(positional[0]) + if pluginID == "" { + return writeExecUsageError(stderr, fmt.Sprintf("usage: zero plugins %s [--user] [--json]", commandName)) + } cwd, err := deps.getwd() if err != nil { @@ -140,11 +143,11 @@ func runPluginsToggle(args []string, stdout io.Writer, stderr io.Writer, deps ap } return exitSuccess } - if result.Changed { - if _, err := fmt.Fprintf(stdout, "Plugin %s is now %s in %s.\n", result.ID, state, result.ManifestPath); err != nil { - return exitCrash - } - } else if _, err := fmt.Fprintf(stdout, "Plugin %s was already %s in %s.\n", result.ID, state, result.ManifestPath); err != nil { + message := fmt.Sprintf("Plugin %s is now %s in %s.\n", result.ID, state, result.ManifestPath) + if !result.Changed { + message = fmt.Sprintf("Plugin %s was already %s in %s.\n", result.ID, state, result.ManifestPath) + } + if _, err := fmt.Fprint(stdout, redaction.RedactString(message, redaction.Options{})); err != nil { return exitCrash } return exitSuccess @@ -664,6 +667,9 @@ Flags: --user Target only user plugins (ignore ./.zero/plugins) --json Print command result as JSON -h, --help Show this help + +Without --user, a project plugin with the same id takes precedence over a user +plugin, matching the loader's discovery order. `, command) return err } diff --git a/internal/cli/extensions_test.go b/internal/cli/extensions_test.go index e6e531b93..b8b7e1703 100644 --- a/internal/cli/extensions_test.go +++ b/internal/cli/extensions_test.go @@ -337,6 +337,9 @@ func TestRunMCPPermissionsHelpDoesNotOpenStore(t *testing.T) { func TestRunPluginsEnableDisable(t *testing.T) { cwd := t.TempDir() + home := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", home) + t.Setenv("HOME", home) pluginDir := filepath.Join(cwd, ".zero", "plugins", "demo") if err := os.MkdirAll(pluginDir, 0o700); err != nil { t.Fatal(err) @@ -404,6 +407,76 @@ func TestRunPluginsEnableDisable(t *testing.T) { } } +// The text-mode toggle output runs through the redaction pipeline, so a +// manifest path that contains something secret-shaped is masked. +func TestRunPluginsToggleTextOutputIsRedacted(t *testing.T) { + cwd := t.TempDir() + home := t.TempDir() + t.Setenv("XDG_CONFIG_HOME", home) + t.Setenv("HOME", home) + secretDir := "sk-proj-abcdefghijklmnopqrstuvwxyz" + pluginDir := filepath.Join(cwd, ".zero", "plugins", secretDir) + if err := os.MkdirAll(pluginDir, 0o700); err != nil { + t.Fatal(err) + } + manifestPath := filepath.Join(pluginDir, "plugin.json") + if err := os.WriteFile(manifestPath, []byte(`{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0" +} +`), 0o600); err != nil { + t.Fatal(err) + } + + deps := appDeps{getwd: func() (string, error) { return cwd, nil }} + + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"plugins", "disable", "zero.demo"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("disable exit=%d stderr=%s", exitCode, stderr.String()) + } + if strings.Contains(stdout.String(), secretDir) { + t.Fatalf("toggle text leaked a secret-shaped manifest path: %q", stdout.String()) + } + if !strings.Contains(stdout.String(), "[REDACTED]") { + t.Fatalf("toggle text was not redacted: %q", stdout.String()) + } + + stdout.Reset() + stderr.Reset() + exitCode = runWithDeps([]string{"plugins", "disable", "zero.demo"}, &stdout, &stderr, deps) + if exitCode != exitSuccess { + t.Fatalf("noop disable exit=%d stderr=%s", exitCode, stderr.String()) + } + if !strings.Contains(stdout.String(), "was already disabled") { + t.Fatalf("noop toggle output: %q", stdout.String()) + } + if strings.Contains(stdout.String(), secretDir) { + t.Fatalf("noop toggle text leaked a secret-shaped manifest path: %q", stdout.String()) + } +} + +func TestRunPluginsToggleRejectsEmptyID(t *testing.T) { + deps := appDeps{getwd: func() (string, error) { return t.TempDir(), nil }} + + for _, subcommand := range []string{"enable", "disable"} { + t.Run(subcommand, func(t *testing.T) { + var stdout bytes.Buffer + var stderr bytes.Buffer + exitCode := runWithDeps([]string{"plugins", subcommand, ""}, &stdout, &stderr, deps) + if exitCode != exitUsage { + t.Fatalf("exit=%d want %d stderr=%s", exitCode, exitUsage, stderr.String()) + } + if !strings.Contains(stderr.String(), "usage: zero plugins "+subcommand) { + t.Fatalf("stderr = %q", stderr.String()) + } + }) + } +} + func TestRunPluginsDisableUserIgnoresProject(t *testing.T) { home := t.TempDir() cwd := t.TempDir() diff --git a/internal/plugins/enabled.go b/internal/plugins/enabled.go index f3b4b357b..38ce5d800 100644 --- a/internal/plugins/enabled.go +++ b/internal/plugins/enabled.go @@ -1,13 +1,15 @@ package plugins import ( + "bytes" "encoding/json" "errors" "fmt" + "io" "os" - "path/filepath" "strings" - "time" + + "github.com/Twigpine/zero/internal/installtxn" ) // SetEnabledResult describes a successful enable/disable of one plugin manifest. @@ -19,6 +21,10 @@ type SetEnabledResult struct { ManifestPath string `json:"manifestPath"` } +// maxEnabledRelockAttempts bounds how many times SetEnabledByID re-resolves the +// plugin after it moves to another install root while waiting for the lock. +const maxEnabledRelockAttempts = 4 + // FindByID returns the loaded plugin with the given id after normal discovery // and precedence (project overrides user). A missing id returns ok=false. func FindByID(options LoadOptions, id string) (LoadedPlugin, bool, error) { @@ -38,77 +44,326 @@ func FindByID(options LoadOptions, id string) (LoadedPlugin, bool, error) { return LoadedPlugin{}, false, nil } -// SetEnabled updates the "enabled" field in plugin.json, preserving the rest of -// the manifest. It returns whether the file actually changed. -func SetEnabled(manifestPath string, enabled bool) (changed bool, err error) { - manifestPath = strings.TrimSpace(manifestPath) - if manifestPath == "" { - return false, errors.New("plugin manifest path is required") - } - resolved, err := filepath.Abs(manifestPath) - if err != nil { - return false, err +// SetEnabledByID finds a plugin by id (respecting LoadOptions) and toggles the +// top-level "enabled" field of its plugin.json. +// +// The read-modify-write is serialized with plugin installs and removals: it +// takes the same cross-process installtxn lock they take on the plugin's +// install root, re-resolves the plugin under that lock, and publishes the +// manifest with an atomic rename while still holding it, so a concurrent +// install can never be interleaved between the read and the write. Only the +// bytes of the "enabled" value change, so field order, formatting, and numeric +// literals in the rest of the manifest survive the toggle. +func SetEnabledByID(options LoadOptions, id string, enabled bool) (SetEnabledResult, error) { + for attempt := 0; ; attempt++ { + plugin, ok, err := FindByID(options, id) + if err != nil { + return SetEnabledResult{}, err + } + if !ok { + return SetEnabledResult{}, fmt.Errorf("%w: %q", ErrNotInstalled, id) + } + // Take the install root lock before the final lookup and read so an + // install or remove cannot swap the tree between them. + unlock, err := installtxn.Lock(plugin.Root) + if err != nil { + return SetEnabledResult{}, err + } + final, ok, err := FindByID(options, id) + if err != nil { + unlock() + return SetEnabledResult{}, err + } + if !ok { + unlock() + return SetEnabledResult{}, fmt.Errorf("%w: %q", ErrNotInstalled, id) + } + if final.Root != plugin.Root { + // Precedence moved to another root while this lock was pending; + // retry against the root that wins now so the edit still serializes + // with that root's installs. + unlock() + if attempt >= maxEnabledRelockAttempts { + return SetEnabledResult{}, fmt.Errorf("plugin %q kept changing roots while waiting for the install lock", id) + } + continue + } + changed, err := setEnabledLocked(final, enabled) + unlock() + if err != nil { + return SetEnabledResult{}, err + } + return SetEnabledResult{ + ID: final.ID, + Enabled: enabled, + Changed: changed, + Source: final.Source, + ManifestPath: final.ManifestPath, + }, nil } +} - raw, err := os.ReadFile(resolved) +// setEnabledLocked performs the manifest read-modify-write while the caller +// holds the installtxn lock for the plugin's install root. +func setEnabledLocked(plugin LoadedPlugin, enabled bool) (bool, error) { + raw, err := os.ReadFile(plugin.ManifestPath) if err != nil { return false, err } - var obj map[string]any - if err := json.Unmarshal(raw, &obj); err != nil { - return false, fmt.Errorf("parse plugin manifest: %w", err) - } - if obj == nil { - return false, errors.New("plugin manifest must be a JSON object") - } - - previous, err := optionalBool(obj, "enabled", true) + data, changed, err := toggleEnabledManifest(raw, enabled) if err != nil { return false, err } - if previous == enabled { + if !changed { return false, nil } - obj["enabled"] = enabled - data, err := json.MarshalIndent(obj, "", " ") + // Read the lockfile before the edit so the refresh below only runs when the + // installed tree still matched its recorded hash; a toggle must never mask + // pre-existing drift. + lock, entry, hashClean := cleanLockEntry(plugin) + + // Fail closed on stat errors instead of widening the permissions. + info, err := os.Stat(plugin.ManifestPath) if err != nil { + return false, fmt.Errorf("inspect plugin manifest: %w", err) + } + if err := installtxn.WriteFileAtomically(plugin.ManifestPath, data, info.Mode().Perm()); err != nil { return false, err } - mode := os.FileMode(0o644) - if info, statErr := os.Stat(resolved); statErr == nil { - mode = info.Mode().Perm() + if !hashClean { + return true, nil } - tempPath := fmt.Sprintf("%s.tmp-%d-%d", resolved, os.Getpid(), time.Now().UnixNano()) - if err := os.WriteFile(tempPath, append(data, '\n'), mode); err != nil { - return false, err + after, err := hashTree(plugin.PluginDir) + if err != nil { + return true, fmt.Errorf("plugin manifest updated but %s hash refresh failed: %w", LockFileName, err) } - if err := os.Rename(tempPath, resolved); err != nil { - _ = os.Remove(tempPath) - return false, err + entry.Hash = after + lock[plugin.ID] = entry + if err := writeLock(plugin.Root, lock); err != nil { + return true, fmt.Errorf("plugin manifest updated but %s hash refresh failed: %w", LockFileName, err) } return true, nil } -// SetEnabledByID finds a plugin by id (respecting LoadOptions) and toggles its -// manifest enabled field. -func SetEnabledByID(options LoadOptions, id string, enabled bool) (SetEnabledResult, error) { - plugin, ok, err := FindByID(options, id) +// cleanLockEntry reports whether the tree on disk still matches the lockfile +// entry recorded for the plugin and returns both for a refresh. +func cleanLockEntry(plugin LoadedPlugin) (map[string]LockEntry, LockEntry, bool) { + lock, err := ReadLock(plugin.Root) if err != nil { - return SetEnabledResult{}, err + return nil, LockEntry{}, false } + entry, ok := lock[plugin.ID] if !ok { - return SetEnabledResult{}, fmt.Errorf("%w: %q", ErrNotInstalled, id) + return nil, LockEntry{}, false } - changed, err := SetEnabled(plugin.ManifestPath, enabled) + current, err := hashTree(plugin.PluginDir) + if err != nil || current != entry.Hash { + return nil, LockEntry{}, false + } + return lock, entry, true +} + +// toggleEnabledManifest returns raw with the top-level "enabled" member set to +// enabled. A missing or null value counts as enabled, matching the loader's +// optionalBool fallback. changed reports whether the result differs from raw. +func toggleEnabledManifest(raw []byte, enabled bool) (out []byte, changed bool, err error) { + dec := json.NewDecoder(bytes.NewReader(raw)) + openTok, err := dec.Token() if err != nil { - return SetEnabledResult{}, err - } - return SetEnabledResult{ - ID: plugin.ID, - Enabled: enabled, - Changed: changed, - Source: plugin.Source, - ManifestPath: plugin.ManifestPath, - }, nil + return nil, false, fmt.Errorf("parse plugin manifest: %w", err) + } + if delim, ok := openTok.(json.Delim); !ok || delim != '{' { + return nil, false, errors.New("plugin manifest must be a JSON object") + } + openEnd := dec.InputOffset() + + current := true + var valueStart, valueEnd int64 = -1, -1 + for dec.More() { + keyTok, err := dec.Token() + if err != nil { + return nil, false, fmt.Errorf("parse plugin manifest: %w", err) + } + key, ok := keyTok.(string) + if !ok { + return nil, false, errors.New("plugin manifest must be a JSON object") + } + keyEnd := dec.InputOffset() + if key != "enabled" { + if err := skipManifestValue(dec); err != nil { + return nil, false, fmt.Errorf("parse plugin manifest: %w", err) + } + continue + } + valueTok, err := dec.Token() + if err != nil { + return nil, false, fmt.Errorf("parse plugin manifest: %w", err) + } + var literal string + switch value := valueTok.(type) { + case bool: + current = value + literal = "false" + if value { + literal = "true" + } + case nil: + current = true + literal = "null" + default: + return nil, false, ManifestError{FieldPath: "enabled", Message: "Expected a boolean."} + } + start, err := enabledValueOffset(raw, keyEnd) + if err != nil { + return nil, false, err + } + if start+int64(len(literal)) > int64(len(raw)) || + !bytes.Equal(raw[start:start+int64(len(literal))], []byte(literal)) { + return nil, false, errors.New("parse plugin manifest: cannot locate the enabled value") + } + // A duplicate key keeps the last occurrence, matching json.Unmarshal. + valueStart, valueEnd = start, start+int64(len(literal)) + } + closingTok, err := dec.Token() + if err != nil { + return nil, false, fmt.Errorf("parse plugin manifest: %w", err) + } + if delim, ok := closingTok.(json.Delim); !ok || delim != '}' { + return nil, false, errors.New("plugin manifest must be a JSON object") + } + closingEnd := dec.InputOffset() + if _, err := dec.Token(); err != io.EOF { + return nil, false, errors.New("parse plugin manifest: unexpected data after the top-level object") + } + + if current == enabled { + return nil, false, nil + } + newLiteral := "false" + if enabled { + newLiteral = "true" + } + if valueStart >= 0 { + out := make([]byte, 0, len(raw)-int(valueEnd-valueStart)+len(newLiteral)) + out = append(out, raw[:valueStart]...) + out = append(out, newLiteral...) + out = append(out, raw[valueEnd:]...) + return out, true, nil + } + return insertEnabledMember(raw, openEnd, closingEnd-1, newLiteral), true, nil +} + +// enabledValueOffset returns the byte offset of the value that follows the +// colon after an "enabled" key token that ended at keyEnd. +func enabledValueOffset(raw []byte, keyEnd int64) (int64, error) { + i := keyEnd + for i < int64(len(raw)) && isJSONSpace(raw[i]) { + i++ + } + if i >= int64(len(raw)) || raw[i] != ':' { + return 0, errors.New("parse plugin manifest: missing colon after key") + } + i++ + for i < int64(len(raw)) && isJSONSpace(raw[i]) { + i++ + } + return i, nil +} + +// insertEnabledMember adds a top-level "enabled" member to a manifest that has +// none, mirroring the document's own whitespace style where it can. openEnd is +// the byte offset just after the opening brace and bracePos is the byte index +// of the closing brace. +func insertEnabledMember(raw []byte, openEnd, bracePos int64, literal string) []byte { + insertPos := bracePos + for insertPos > openEnd && isJSONSpace(raw[insertPos-1]) { + insertPos-- + } + + newline := "\n" + if bytes.Contains(raw, []byte("\r\n")) { + newline = "\r\n" + } + + first := openEnd + for first < bracePos && isJSONSpace(raw[first]) { + first++ + } + empty := first >= bracePos + multiline := bytes.IndexByte(raw[openEnd:bracePos], '\n') >= 0 + + member := `"enabled": ` + literal + var insert string + switch { + case empty && !multiline: + insert = member + case empty && multiline: + insert = newline + closingLineIndent(raw, openEnd, bracePos) + " " + member + case !empty && !multiline: + insert = "," + member + default: + insert = "," + newline + memberLineIndent(raw, openEnd, first) + member + } + + out := make([]byte, 0, len(raw)+len(insert)) + out = append(out, raw[:insertPos]...) + out = append(out, insert...) + out = append(out, raw[insertPos:]...) + return out +} + +// memberLineIndent returns the indentation of the line the first member of a +// JSON object starts on. first is the byte index of that member's first token. +func memberLineIndent(raw []byte, openEnd, first int64) string { + lineStart := first + for lineStart > openEnd && raw[lineStart-1] != '\n' { + lineStart-- + } + end := lineStart + for end < first && (raw[end] == ' ' || raw[end] == '\t') { + end++ + } + return string(raw[lineStart:end]) +} + +// closingLineIndent returns the indentation before an object's closing brace. +func closingLineIndent(raw []byte, openEnd, bracePos int64) string { + start := bracePos + for start > openEnd && (raw[start-1] == ' ' || raw[start-1] == '\t') { + start-- + } + return string(raw[start:bracePos]) +} + +// skipManifestValue consumes the next JSON value from dec. +func skipManifestValue(dec *json.Decoder) error { + tok, err := dec.Token() + if err != nil { + return err + } + delim, ok := tok.(json.Delim) + if !ok || (delim != '{' && delim != '[') { + return nil + } + depth := 1 + for depth > 0 { + tok, err := dec.Token() + if err != nil { + return err + } + if inner, ok := tok.(json.Delim); ok { + switch inner { + case '{', '[': + depth++ + case '}', ']': + depth-- + } + } + } + return nil +} + +func isJSONSpace(b byte) bool { + return b == ' ' || b == '\t' || b == '\n' || b == '\r' } diff --git a/internal/plugins/enabled_test.go b/internal/plugins/enabled_test.go index 340cd549b..5e19a5b52 100644 --- a/internal/plugins/enabled_test.go +++ b/internal/plugins/enabled_test.go @@ -1,18 +1,113 @@ package plugins import ( - "encoding/json" "errors" "os" "path/filepath" "runtime" + "strings" "testing" + "time" + + "github.com/Twigpine/zero/internal/installtxn" ) -func TestSetEnabledTogglesManifestAndIsIdempotent(t *testing.T) { +func enableTestOptions(root string) LoadOptions { + return LoadOptions{Roots: []Root{{Source: SourceUser, Path: root}}} +} + +func writeRawManifest(t *testing.T, root, dir, raw string) string { + t.Helper() + pluginDir := filepath.Join(root, dir) + if err := os.MkdirAll(pluginDir, 0o700); err != nil { + t.Fatal(err) + } + manifestPath := filepath.Join(pluginDir, "plugin.json") + if err := os.WriteFile(manifestPath, []byte(raw), 0o600); err != nil { + t.Fatal(err) + } + return manifestPath +} + +func demoManifest(enabled bool) string { + state := "false" + if enabled { + state = "true" + } + return `{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0", + "enabled": ` + state + ` +} +` +} + +func TestSetEnabledByIDPreservesManifestBytes(t *testing.T) { + root := t.TempDir() + raw := `{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0", + "limits": { + "maxBytes": 9007199254740993 + }, + "description": "keep me", + "enabled": true, + "tools": [] +} +` + manifestPath := writeRawManifest(t, root, "demo", raw) + + result, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false) + if err != nil { + t.Fatalf("SetEnabledByID(false): %v", err) + } + if !result.Changed || result.Enabled { + t.Fatalf("unexpected result: %#v", result) + } + + disabled, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + want := strings.Replace(raw, `"enabled": true`, `"enabled": false`, 1) + if string(disabled) != want { + t.Fatalf("toggle changed more than the enabled value:\n got %q\nwant %q", disabled, want) + } + if !strings.Contains(string(disabled), "9007199254740993") { + t.Fatal("large integer literal was rounded by the toggle") + } + + plugin, ok, err := FindByID(enableTestOptions(root), "zero.demo") + if err != nil || !ok { + t.Fatalf("FindByID after disable: ok=%v err=%v", ok, err) + } + if plugin.Enabled { + t.Fatal("loader still reports the plugin enabled after disable") + } + + result, err = SetEnabledByID(enableTestOptions(root), "zero.demo", true) + if err != nil { + t.Fatalf("SetEnabledByID(true): %v", err) + } + if !result.Changed || !result.Enabled { + t.Fatalf("unexpected enable result: %#v", result) + } + enabled, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + if string(enabled) != raw { + t.Fatalf("round trip did not restore the original bytes:\n got %q\nwant %q", enabled, raw) + } +} + +func TestSetEnabledByIDTogglesManifestAndIsIdempotent(t *testing.T) { root := t.TempDir() - pluginDir := filepath.Join(root, "demo") - writePluginManifest(t, pluginDir, map[string]any{ + writePluginManifest(t, filepath.Join(root, "demo"), map[string]any{ "schemaVersion": 1, "id": "zero.demo", "name": "Demo", @@ -20,48 +115,213 @@ func TestSetEnabledTogglesManifestAndIsIdempotent(t *testing.T) { "description": "keep me", "tools": []any{}, }) - manifestPath := filepath.Join(pluginDir, "plugin.json") + manifestPath := filepath.Join(root, "demo", "plugin.json") - changed, err := SetEnabled(manifestPath, false) + result, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false) if err != nil { - t.Fatalf("SetEnabled(false): %v", err) + t.Fatalf("SetEnabledByID(false): %v", err) } - if !changed { + if !result.Changed { t.Fatal("expected first disable to change the manifest") } - raw, err := os.ReadFile(manifestPath) if err != nil { t.Fatal(err) } - var obj map[string]any - if err := json.Unmarshal(raw, &obj); err != nil { - t.Fatalf("re-read manifest: %v", err) - } - if obj["enabled"] != false { - t.Fatalf("enabled = %#v, want false", obj["enabled"]) + if !strings.Contains(string(raw), `"enabled": false`) { + t.Fatalf("manifest not disabled: %s", raw) } - if obj["description"] != "keep me" { - t.Fatalf("description was not preserved: %#v", obj["description"]) + if !strings.Contains(string(raw), `"keep me"`) { + t.Fatalf("description was not preserved: %s", raw) } - changed, err = SetEnabled(manifestPath, false) + result, err = SetEnabledByID(enableTestOptions(root), "zero.demo", false) if err != nil { - t.Fatalf("SetEnabled(false) again: %v", err) + t.Fatalf("SetEnabledByID(false) again: %v", err) } - if changed { + if result.Changed { t.Fatal("expected second disable to be a no-op") } - changed, err = SetEnabled(manifestPath, true) + result, err = SetEnabledByID(enableTestOptions(root), "zero.demo", true) if err != nil { - t.Fatalf("SetEnabled(true): %v", err) + t.Fatalf("SetEnabledByID(true): %v", err) } - if !changed { + if !result.Changed { t.Fatal("expected enable to change the manifest") } } +func TestSetEnabledByIDInsertsMissingField(t *testing.T) { + t.Run("pretty manifest", func(t *testing.T) { + root := t.TempDir() + raw := `{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0" +} +` + manifestPath := writeRawManifest(t, root, "demo", raw) + + if _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false); err != nil { + t.Fatalf("SetEnabledByID(false): %v", err) + } + disabled, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + wantDisabled := `{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0", + "enabled": false +} +` + if string(disabled) != wantDisabled { + t.Fatalf("after insert:\n got %q\nwant %q", disabled, wantDisabled) + } + + if _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", true); err != nil { + t.Fatalf("SetEnabledByID(true): %v", err) + } + enabled, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + wantEnabled := strings.Replace(wantDisabled, `"enabled": false`, `"enabled": true`, 1) + if string(enabled) != wantEnabled { + t.Fatalf("after replace:\n got %q\nwant %q", enabled, wantEnabled) + } + }) + + t.Run("compact manifest", func(t *testing.T) { + root := t.TempDir() + raw := `{"schemaVersion":1,"id":"zero.demo","name":"Demo","version":"1.0.0"}` + manifestPath := writeRawManifest(t, root, "demo", raw) + + if _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false); err != nil { + t.Fatalf("SetEnabledByID(false): %v", err) + } + disabled, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + want := `{"schemaVersion":1,"id":"zero.demo","name":"Demo","version":"1.0.0","enabled": false}` + if string(disabled) != want { + t.Fatalf("compact insert:\n got %q\nwant %q", disabled, want) + } + }) + + t.Run("null value", func(t *testing.T) { + root := t.TempDir() + raw := `{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "1.0.0", + "enabled": null +} +` + manifestPath := writeRawManifest(t, root, "demo", raw) + + result, err := SetEnabledByID(enableTestOptions(root), "zero.demo", true) + if err != nil { + t.Fatalf("SetEnabledByID(true): %v", err) + } + if result.Changed { + t.Fatal("null counts as enabled, so enabling must be a no-op") + } + unchanged, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + if string(unchanged) != raw { + t.Fatalf("no-op toggle rewrote the manifest: %q", unchanged) + } + + if _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false); err != nil { + t.Fatalf("SetEnabledByID(false): %v", err) + } + disabled, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + want := strings.Replace(raw, `"enabled": null`, `"enabled": false`, 1) + if string(disabled) != want { + t.Fatalf("null replacement:\n got %q\nwant %q", disabled, want) + } + }) +} + +func TestToggleEnabledManifestFormattingEdges(t *testing.T) { + cases := []struct { + name string + in string + enabled bool + want string + changed bool + }{ + {"empty object", `{}`, false, `{"enabled": false}`, true}, + {"empty object with space", `{ }`, false, `{"enabled": false }`, true}, + {"empty multiline", "{\n}", false, "{\n \"enabled\": false\n}", true}, + {"compact members", `{"a":1}`, false, `{"a":1,"enabled": false}`, true}, + {"crlf members", "{\r\n \"a\": 1\r\n}", false, "{\r\n \"a\": 1,\r\n \"enabled\": false\r\n}", true}, + {"nested enabled untouched", `{"nested":{"enabled":true},"a":2}`, false, `{"nested":{"enabled":true},"a":2,"enabled": false}`, true}, + {"replace with true", `{"enabled": false}`, true, `{"enabled": true}`, true}, + {"replace with false", `{"enabled":true}`, false, `{"enabled":false}`, true}, + {"null to false", `{"enabled": null}`, false, `{"enabled": false}`, true}, + {"null to true is a no-op", `{"enabled": null}`, true, `{"enabled": null}`, false}, + {"already enabled is a no-op", `{"enabled": true}`, true, `{"enabled": true}`, false}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + out, changed, err := toggleEnabledManifest([]byte(tc.in), tc.enabled) + if err != nil { + t.Fatalf("toggleEnabledManifest: %v", err) + } + if changed != tc.changed { + t.Fatalf("changed = %v, want %v", changed, tc.changed) + } + if !tc.changed { + if out != nil { + t.Fatalf("no-op returned bytes: %q", out) + } + return + } + if string(out) != tc.want { + t.Fatalf("got %q, want %q", out, tc.want) + } + }) + } +} + +func TestToggleEnabledManifestRejectsMalformedInput(t *testing.T) { + for _, raw := range []string{`{"enabled": "yes"}`, `{"enabled": 1}`, `{"enabled": []}`, `{"enabled": {}}`} { + t.Run("value "+raw, func(t *testing.T) { + _, _, err := toggleEnabledManifest([]byte(raw), false) + if err == nil { + t.Fatal("expected error") + } + var manifestErr ManifestError + if !errors.As(err, &manifestErr) { + t.Fatalf("error = %v, want ManifestError", err) + } + if !strings.Contains(err.Error(), "enabled") { + t.Fatalf("error = %v, want it to name the enabled field", err) + } + }) + } + for _, raw := range []string{``, `{`, `[]`, `null`, `"str"`, `{} {}`, `{}{}`} { + t.Run("document "+raw, func(t *testing.T) { + if _, _, err := toggleEnabledManifest([]byte(raw), false); err == nil { + t.Fatalf("toggleEnabledManifest(%q) succeeded, want error", raw) + } + }) + } +} + func TestSetEnabledByIDRespectsUserOnlyFilter(t *testing.T) { dir := t.TempDir() userRoot := filepath.Join(dir, "user") @@ -117,20 +377,19 @@ func TestSetEnabledPreservesManifestMode(t *testing.T) { t.Skip("Windows does not preserve chmod bits in os.Stat mode") } root := t.TempDir() - pluginDir := filepath.Join(root, "demo") - writePluginManifest(t, pluginDir, map[string]any{ + writePluginManifest(t, filepath.Join(root, "demo"), map[string]any{ "schemaVersion": 1, "id": "zero.demo", "name": "Demo", "version": "1.0.0", }) - manifestPath := filepath.Join(pluginDir, "plugin.json") + manifestPath := filepath.Join(root, "demo", "plugin.json") if err := os.Chmod(manifestPath, 0o644); err != nil { t.Fatal(err) } - if _, err := SetEnabled(manifestPath, false); err != nil { - t.Fatalf("SetEnabled: %v", err) + if _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false); err != nil { + t.Fatalf("SetEnabledByID: %v", err) } info, err := os.Stat(manifestPath) if err != nil { @@ -150,3 +409,262 @@ func TestSetEnabledByIDMissingPlugin(t *testing.T) { t.Fatalf("error = %v, want ErrNotInstalled", err) } } + +func TestSetEnabledPublishFailureLeavesManifestIntact(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("Windows directory permissions do not block writes the way this test needs") + } + if os.Geteuid() == 0 { + t.Skip("root bypasses directory permission checks") + } + root := t.TempDir() + raw := demoManifest(true) + manifestPath := writeRawManifest(t, root, "demo", raw) + pluginDir := filepath.Dir(manifestPath) + if err := os.Chmod(pluginDir, 0o555); err != nil { + t.Fatal(err) + } + defer func() { _ = os.Chmod(pluginDir, 0o755) }() + + if _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false); err == nil { + t.Fatal("expected the publish to fail in a read-only plugin directory") + } + after, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + if string(after) != raw { + t.Fatalf("failed publish changed the manifest:\n got %q\nwant %q", after, raw) + } +} + +// TestSetEnabledByIDWaitsForInstallRootLock proves the toggle takes the same +// cross-process lock Install and Remove take on the plugin's install root: it +// must not complete while an install holds that lock. +func TestSetEnabledByIDWaitsForInstallRootLock(t *testing.T) { + root := t.TempDir() + manifestPath := writeRawManifest(t, root, "demo", demoManifest(true)) + + unlock, err := installtxn.Lock(root) + if err != nil { + t.Fatal(err) + } + + results := make(chan SetEnabledResult, 1) + failures := make(chan error, 1) + go func() { + result, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false) + if err != nil { + failures <- err + return + } + results <- result + }() + + select { + case result := <-results: + unlock() + t.Fatalf("toggle completed while the install lock was held: %#v", result) + case err := <-failures: + unlock() + t.Fatalf("toggle failed while the install lock was held: %v", err) + case <-time.After(200 * time.Millisecond): + } + + unlock() + + select { + case result := <-results: + if !result.Changed { + t.Fatalf("toggle did not report a change: %#v", result) + } + case err := <-failures: + t.Fatalf("toggle failed after the lock was released: %v", err) + case <-time.After(10 * time.Second): + t.Fatal("toggle did not finish after the install lock was released") + } + + raw, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + if !strings.Contains(string(raw), `"enabled": false`) { + t.Fatalf("manifest not disabled: %s", raw) + } +} + +// TestSetEnabledByIDSynchronizesWithStagedInstallCommit pauses an install +// between staging and commit, lets the toggle try to run, then commits the +// install. The toggle must apply to the newly installed manifest instead of +// overwriting it with bytes read before the install, and the lockfile entry +// must describe the tree actually on disk. +func TestSetEnabledByIDSynchronizesWithStagedInstallCommit(t *testing.T) { + root := t.TempDir() + oldRaw := demoManifest(true) + pluginDir := filepath.Dir(writeRawManifest(t, root, "zero.demo", oldRaw)) + + // Build the replacement tree with the same staging primitives Install + // uses and leave it staged but uncommitted. + src := t.TempDir() + newRaw := `{ + "schemaVersion": 1, + "id": "zero.demo", + "name": "Demo", + "version": "2.0.0", + "description": "reinstalled", + "enabled": true +} +` + srcPlugin := filepath.Join(src, "zero.demo") + if err := os.MkdirAll(srcPlugin, 0o700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(srcPlugin, "plugin.json"), []byte(newRaw), 0o600); err != nil { + t.Fatal(err) + } + staged, cleanup, err := installtxn.StageDir(root) + if err != nil { + t.Fatal(err) + } + defer cleanup() + if err := copyTree(srcPlugin, staged); err != nil { + t.Fatal(err) + } + stagedHash, err := hashTree(staged) + if err != nil { + t.Fatal(err) + } + + // An install holds the root lock from before its commit until after it. + unlock, err := installtxn.Lock(root) + if err != nil { + t.Fatal(err) + } + + results := make(chan SetEnabledResult, 1) + failures := make(chan error, 1) + go func() { + result, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false) + if err != nil { + failures <- err + return + } + results <- result + }() + + select { + case result := <-results: + unlock() + t.Fatalf("toggle completed while the install lock was held: %#v", result) + case err := <-failures: + unlock() + t.Fatalf("toggle failed while the install lock was held: %v", err) + case <-time.After(200 * time.Millisecond): + } + + // Commit the staged install exactly like Install does: swap the tree and + // publish the lockfile entry while still holding the lock. + lock, err := ReadLock(root) + if err != nil { + unlock() + t.Fatal(err) + } + lock["zero.demo"] = LockEntry{Source: string(SourceUser), Hash: stagedHash} + if err := installtxn.CommitDir(pluginDir, staged, func() error { return writeLock(root, lock) }); err != nil { + unlock() + t.Fatal(err) + } + unlock() + + var result SetEnabledResult + select { + case result = <-results: + case err := <-failures: + t.Fatalf("toggle failed after the install committed: %v", err) + case <-time.After(10 * time.Second): + t.Fatal("toggle did not finish after the install committed") + } + if !result.Changed { + t.Fatalf("toggle result = %#v, want changed", result) + } + + // The toggle must have applied to the newly installed manifest, not to the + // bytes it first read. + final, err := os.ReadFile(filepath.Join(pluginDir, "plugin.json")) + if err != nil { + t.Fatal(err) + } + want := strings.Replace(newRaw, `"enabled": true`, `"enabled": false`, 1) + if string(final) != want { + t.Fatalf("manifest after concurrent install + disable:\n got %q\nwant %q", final, want) + } + + // The lockfile entry must describe the tree that is actually on disk. + entries, err := ReadLock(root) + if err != nil { + t.Fatal(err) + } + current, err := hashTree(pluginDir) + if err != nil { + t.Fatal(err) + } + if entries["zero.demo"].Hash != current { + t.Fatalf("plugins.lock hash = %q, want current tree hash %q", entries["zero.demo"].Hash, current) + } +} + +func TestSetEnabledByIDRefreshesCleanLockEntry(t *testing.T) { + root := t.TempDir() + manifestPath := writeRawManifest(t, root, "zero.demo", demoManifest(true)) + pluginDir := filepath.Dir(manifestPath) + + current, err := hashTree(pluginDir) + if err != nil { + t.Fatal(err) + } + if err := writeLock(root, map[string]LockEntry{"zero.demo": {Source: string(SourceUser), Hash: current}}); err != nil { + t.Fatal(err) + } + + if _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false); err != nil { + t.Fatalf("SetEnabledByID: %v", err) + } + entries, err := ReadLock(root) + if err != nil { + t.Fatal(err) + } + afterToggle, err := hashTree(pluginDir) + if err != nil { + t.Fatal(err) + } + if entries["zero.demo"].Hash == current { + t.Fatal("lock hash was not refreshed for a clean install") + } + if entries["zero.demo"].Hash != afterToggle { + t.Fatalf("lock hash = %q, want refreshed tree hash %q", entries["zero.demo"].Hash, afterToggle) + } + + info, err := Info(InfoOptions{LoadOptions: enableTestOptions(root)}, "zero.demo") + if err != nil { + t.Fatal(err) + } + if info.HashDrift { + t.Fatal("toggle left hash drift behind for a clean install") + } + + // Pre-existing drift must survive the next toggle instead of being + // absorbed into the lockfile. + if err := os.WriteFile(filepath.Join(pluginDir, "extra.txt"), []byte("edited\n"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", true); err != nil { + t.Fatalf("SetEnabledByID(true): %v", err) + } + entries, err = ReadLock(root) + if err != nil { + t.Fatal(err) + } + if entries["zero.demo"].Hash != afterToggle { + t.Fatalf("toggle absorbed pre-existing drift: hash = %q, want %q", entries["zero.demo"].Hash, afterToggle) + } +} diff --git a/internal/plugins/info.go b/internal/plugins/info.go index 0dac22570..82b64f846 100644 --- a/internal/plugins/info.go +++ b/internal/plugins/info.go @@ -7,7 +7,8 @@ import ( "strings" ) -// ErrNotInstalled is returned when Info cannot find a plugin with the requested id. +// ErrNotInstalled is returned when a plugin id cannot be found under the given +// LoadOptions (for example by Info or SetEnabledByID). var ErrNotInstalled = errors.New("plugin is not installed") // PluginInfo combines a loaded plugin with optional lockfile metadata. From 4d8aaf5a3ebac7d96b0842614889bacf4f4d7936 Mon Sep 17 00:00:00 2001 From: jatmn Date: Sat, 7 Nov 2026 08:01:22 -0800 Subject: [PATCH 3/4] fix(plugins): recover interrupted install transactions before toggling SetEnabledByID takes the plugin root's installtxn lock but went straight to reading and rewriting the tree. installtxn.Recover requires every holder of that lock to resolve interrupted transactions first and to abort on its error, as Install and Remove already do. Without it, a toggle after an install killed between its tree swap and its lockfile publish edited the unpublished tree, and the next install or removal restored the retained backup over it, silently undoing the enable/disable. A transaction recovery could not resolve also failed to stop the toggle. Call installtxn.Recover with the plugins reconciler right after taking the lock, and add regression tests for every interrupted state that leaves the plugin discoverable, the unresolved-transaction abort, and a plugin removed while the toggle waits on the lock. Co-Authored-By: Claude Opus 5.5 --- internal/plugins/enabled.go | 11 ++- internal/plugins/enabled_test.go | 153 +++++++++++++++++++++++++++++++ 2 files changed, 163 insertions(+), 1 deletion(-) diff --git a/internal/plugins/enabled.go b/internal/plugins/enabled.go index 38ce5d800..f2ab4171e 100644 --- a/internal/plugins/enabled.go +++ b/internal/plugins/enabled.go @@ -49,7 +49,8 @@ func FindByID(options LoadOptions, id string) (LoadedPlugin, bool, error) { // // The read-modify-write is serialized with plugin installs and removals: it // takes the same cross-process installtxn lock they take on the plugin's -// install root, re-resolves the plugin under that lock, and publishes the +// install root, runs the same interrupted-transaction recovery they run under +// it, re-resolves the plugin under that lock, and publishes the // manifest with an atomic rename while still holding it, so a concurrent // install can never be interleaved between the read and the write. Only the // bytes of the "enabled" value change, so field order, formatting, and numeric @@ -69,6 +70,14 @@ func SetEnabledByID(options LoadOptions, id string, enabled bool) (SetEnabledRes if err != nil { return SetEnabledResult{}, err } + // Put back anything an earlier install or removal was killed mid-commit, + // and abort if a transaction could not be resolved: editing a tree that is + // still owed a restore loses the edit when the next install or removal + // puts the backup back over it. See installtxn.Recover. + if err := installtxn.Recover(plugin.Root, reconcileInterrupted(plugin.Root)); err != nil { + unlock() + return SetEnabledResult{}, err + } final, ok, err := FindByID(options, id) if err != nil { unlock() diff --git a/internal/plugins/enabled_test.go b/internal/plugins/enabled_test.go index 5e19a5b52..3ef289e84 100644 --- a/internal/plugins/enabled_test.go +++ b/internal/plugins/enabled_test.go @@ -668,3 +668,156 @@ func TestSetEnabledByIDRefreshesCleanLockEntry(t *testing.T) { t.Fatalf("toggle absorbed pre-existing drift: hash = %q, want %q", entries["zero.demo"].Hash, afterToggle) } } + +// TestSetEnabledByIDReportsAPluginRemovedWhileWaiting removes the plugin while +// the toggle is blocked on the install-root lock, exactly as Remove does while +// it holds that lock. The toggle must report the plugin as not installed +// instead of editing, or recreating, a tree that no longer exists. +func TestSetEnabledByIDReportsAPluginRemovedWhileWaiting(t *testing.T) { + root := t.TempDir() + pluginDir := filepath.Dir(writeRawManifest(t, root, "zero.demo", demoManifest(true))) + + unlock, err := installtxn.Lock(root) + if err != nil { + t.Fatal(err) + } + + failures := make(chan error, 1) + go func() { + _, err := SetEnabledByID(enableTestOptions(root), "zero.demo", false) + failures <- err + }() + + select { + case err := <-failures: + unlock() + t.Fatalf("toggle returned while the install lock was held: %v", err) + case <-time.After(200 * time.Millisecond): + } + + if err := installtxn.RemoveDir(pluginDir, func() error { return nil }); err != nil { + unlock() + t.Fatal(err) + } + unlock() + + select { + case err := <-failures: + if !errors.Is(err, ErrNotInstalled) { + t.Fatalf("toggle after a concurrent removal: error = %v, want ErrNotInstalled", err) + } + case <-time.After(10 * time.Second): + t.Fatal("toggle did not finish after the removal released the lock") + } + if _, err := os.Stat(pluginDir); !os.IsNotExist(err) { + t.Fatalf("toggle recreated the removed plugin: %v", err) + } +} + +// TestSetEnabledByIDResolvesInterruptedUpdatesFirst covers every point an +// update can be killed at that leaves the plugin discoverable. The toggle +// holds the install-root lock, so it has to +// run the same recovery pass Install and Remove run before it reads the tree: +// otherwise it edits, or fails to find, a tree that is still owed a restore, +// and the next install or removal puts the backup back over the user's edit. +func TestSetEnabledByIDResolvesInterruptedUpdatesFirst(t *testing.T) { + for _, state := range interruptedStates() { + t.Run(state.name, func(t *testing.T) { + u := seedInterruptedUpdate(t) + workspace := state.plant(t, u) + want := state.want(u) + if _, err := os.Lstat(u.target()); os.IsNotExist(err) { + // No root lists a plugin whose only copy is in a workspace, so the + // toggle reports it not installed without taking any lock, just as + // list and info do; the next install or removal restores it. + t.Skip("nothing is at the target, so the toggle never takes the lock") + } + + result, err := SetEnabledByID(enableTestOptions(u.dir), "zero.demo", false) + if err != nil { + t.Fatalf("toggle after the interrupted update: %v", err) + } + if !result.Changed || result.Enabled { + t.Fatalf("toggle result = %#v, want changed and disabled", result) + } + assertDisabledAfterRecovery(t, u, workspace, want) + + // The next lock holder runs recovery again. The disable has to survive + // it rather than be replaced by a backup the toggle never saw. + if err := installOther(t, u.dir, "zero.other"); err != nil { + t.Fatalf("install after the toggle: %v", err) + } + assertDisabledAfterRecovery(t, u, workspace, want) + }) + } +} + +func assertDisabledAfterRecovery(t *testing.T, u interruptedUpdate, workspace string, want recoveryWant) { + t.Helper() + data, err := os.ReadFile(filepath.Join(u.target(), "generation.txt")) + if err != nil { + t.Fatalf("reading the live tree: %v", err) + } + if string(data) != want.generation { + t.Errorf("the live tree is the %q generation, want %q", data, want.generation) + } + plugin, ok, err := FindByID(enableTestOptions(u.dir), "zero.demo") + if err != nil { + t.Fatal(err) + } + if !ok { + t.Fatal("Load does not see the toggled plugin") + } + if plugin.Version != want.version { + t.Errorf("Load sees version %q, want %q", plugin.Version, want.version) + } + if plugin.Enabled { + t.Errorf("the disable did not stick: Load sees the plugin enabled") + } + _, err = os.Stat(workspace) + if want.workspace && err != nil { + t.Errorf("the workspace should be left alone, got %v", err) + } + if !want.workspace && !os.IsNotExist(err) { + t.Errorf("the resolved workspace should be cleared, got %v", err) + } +} + +// TestSetEnabledByIDAbortsOnAnUnresolvedInterruptedUpdate leaves a workspace +// whose trees match neither lockfile hash. Recovery cannot pick one, so the +// toggle must fail without editing the live manifest or the lockfile. +func TestSetEnabledByIDAbortsOnAnUnresolvedInterruptedUpdate(t *testing.T) { + u := seedInterruptedUpdate(t) + workspace := u.plantWorkspace(t) + u.mustRename(t, u.target(), filepath.Join(workspace, "previous")) + u.mustRename(t, filepath.Join(workspace, "staged"), u.target()) + unmatched := LockEntry{Source: u.oldSource, Hash: "sha256:0000000000000000000000000000000000000000000000000000000000000000"} + lock, err := ReadLock(u.dir) + if err != nil { + t.Fatal(err) + } + lock["zero.demo"] = unmatched + if err := writeLock(u.dir, lock); err != nil { + t.Fatal(err) + } + manifestPath := filepath.Join(u.target(), "plugin.json") + before, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + + if _, err := SetEnabledByID(enableTestOptions(u.dir), "zero.demo", false); err == nil { + t.Fatal("toggle did not report the unresolved transaction") + } + after, err := os.ReadFile(manifestPath) + if err != nil { + t.Fatal(err) + } + if string(after) != string(before) { + t.Errorf("the aborted toggle edited the manifest:\n got %q\nwant %q", after, before) + } + assertUntouched(t, u, "new", unmatched) + if _, err := os.Stat(filepath.Join(workspace, "previous")); err != nil { + t.Errorf("the backup should still be there for a hand rescue, got %v", err) + } +} From 87ecb289ed803169ce63843cdb16c66f71af0175 Mon Sep 17 00:00:00 2001 From: jatmn Date: Sat, 7 Nov 2026 08:12:35 -0800 Subject: [PATCH 4/4] docs(plugins): say when a plugin toggle takes effect Zero reads plugin.json "enabled" only when it activates plugins at startup (runExec and the interactive TUI bootstrap), and nothing reloads them afterwards. A running interactive session therefore keeps the tools, hooks, and skills of a plugin disabled from another terminal. State that in the plugins docs and in the enable/disable help so the documented activation rule matches what the command does. Co-Authored-By: Claude Opus 5.5 --- docs/EXTENDING.md | 2 +- internal/cli/extensions.go | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/EXTENDING.md b/docs/EXTENDING.md index fec923f45..cf9695c16 100644 --- a/docs/EXTENDING.md +++ b/docs/EXTENDING.md @@ -352,7 +352,7 @@ zero plugins enable github-pr-review # sets "enabled": true in plugin.json zero plugins remove github-pr-review # alias: rm ``` -A plugin is active when it is present under a plugins directory and its `plugin.json` has `"enabled": true` (the default when the field is omitted). Use `zero plugins disable ` / `zero plugins enable ` to toggle that field without removing the plugin. Pass `--user` to target only user plugins and ignore `./.zero/plugins`; without it, a project plugin with the same id takes precedence. The toggle edits the `enabled` value in place — the rest of the manifest keeps its formatting, field order, and numeric literals — and serializes with `plugins add` / `plugins remove` on the same plugins root. +A plugin is active when it is present under a plugins directory and its `plugin.json` has `"enabled": true` (the default when the field is omitted). Use `zero plugins disable ` / `zero plugins enable ` to toggle that field without removing the plugin. Pass `--user` to target only user plugins and ignore `./.zero/plugins`; without it, a project plugin with the same id takes precedence. The toggle edits the `enabled` value in place — the rest of the manifest keeps its formatting, field order, and numeric literals — and serializes with `plugins add` / `plugins remove` on the same plugins root. Zero reads plugins when it starts, so the change applies to the next `zero exec` run or interactive session; an interactive session that is already running keeps the plugin tools, hooks, and skills it started with until you restart it. Plugin commands run with the plugin directory as their working directory. Use relative paths; the loader resolves them at activation time. diff --git a/internal/cli/extensions.go b/internal/cli/extensions.go index fe8da1c53..cc9012c3b 100644 --- a/internal/cli/extensions.go +++ b/internal/cli/extensions.go @@ -670,6 +670,10 @@ Flags: Without --user, a project plugin with the same id takes precedence over a user plugin, matching the loader's discovery order. + +Zero reads plugins when it starts, so the change applies to the next zero exec +run or interactive session. A running interactive session keeps the plugins it +started with until you restart it. `, command) return err }