fix: plugins enable/disable only reloads when state changes (#411)

plugins enable now checks if the plugin was already enabled before
setting reload_runtime:true. Same for disable. Returns 'already enabled'
or 'already disabled' in the message when no state change occurred.

Generated with https://github.com/Yeachan-Heo/gajae-code
Co-authored-by: Gajae Code <dev@gajae-code.com>

Author: 2 people

ROADMAP.md

@@ -6314,7 +6314,7 @@ Original filing (2026-04-18): the session emitted `SessionStart hook (completed)
 410. **DONE — `agents list`, `skills list`, and `mcp list` use three different count-field names and divergent envelope schemas despite being sibling list commands** — dogfooded 2026-04-30 by Jobdori on `e939777f`. Running all three list commands with `--output-format json` reveals incompatible envelope shapes: `agents list` emits `count:int` at the top level plus `summary:{active,shadowed,total}` and `working_directory`; `skills list` emits no top-level `count`, only `summary:{active,shadowed,total}`, and omits `working_directory`; `mcp list` uses a different count-field name `configured_servers:int`, has no `count`, no `summary`, and instead adds `status:"ok"` and `config_load_error:null` fields absent from the other two. The three sibling commands cannot be polymorphically consumed with the same count-extraction logic, requiring per-command special-casing at the cardinality check level. **Required fix shape:** (a) define one canonical top-level count field name (`count`, `total`, or `item_count`) and use it across `agents`, `skills`, and `mcp` list envelopes; (b) define one canonical `summary` object shape with at minimum `active`, `total`, and optionally `shadowed` and include it on all three; (c) expose `working_directory` consistently on all list commands or omit it from all; (d) add regression coverage proving the three list envelopes share the same count-field name and summary shape before each release. **Why this matters:** orchestration lanes that inventory agents, skills, and servers before delegation need one count-extraction pattern. Three different field names force per-command special-casing of the most basic cardinality check. Source: Jobdori live dogfood, `e939777f`, 2026-04-30.
 
 
-411. **`plugins enable/disable --output-format json` always emits `reload_runtime:true` regardless of whether state actually changed, and omits `previous_status`, `changed`, `version`, and `source` fields — automation cannot tell if a reload is necessary or if the mutation was a no-op** — dogfooded 2026-04-30 by Jobdori on `e939777f`. Running `claw plugins enable example-bundled --output-format json` on an already-enabled plugin returns `{"action":"enable","kind":"plugin","message":"…","reload_runtime":true,"target":"example-bundled"}` — `reload_runtime:true` every time, even on a no-op re-enable. The same applies to idempotent `disable`. Structured fields present: `action`, `kind`, `message`, `reload_runtime`, `target`. Structured fields absent: `previous_status`, `status`, `changed`, `version`, `source`. The actual plugin name, version, and new status are embedded only in the prose `message` field (`"Result  enabled example-bundled@bundled\n  Name  example-bundled\n  Version  0.1.0\n  Status  enabled"`), requiring callers to scrape column-aligned text to extract the post-mutation state. A no-op mutation emitting `reload_runtime:true` forces orchestration to trigger an expensive runtime reload even when no config change occurred. **Required fix shape:** (a) add `changed:bool` so callers can skip runtime reload when `changed:false`; (b) add `previous_status` and `status` fields (enums: `enabled`/`disabled`) so pre/post state is machine-readable without parsing `message`; (c) add `version` and `source` fields at the mutation response level, consistent with `plugins list` entry shape; (d) emit `reload_runtime:false` when `changed:false`; (e) add regression coverage proving idempotent enable/disable sets `changed:false` and `reload_runtime:false`. **Why this matters:** plugin lifecycle is a hot path for automation that conditionally enables plugins before running sessions. If every enable emits `reload_runtime:true` and no `changed` field exists, orchestration must reload unconditionally or maintain external state — both brittle patterns. Source: Jobdori live dogfood, `e939777f`, 2026-04-30.
+411. **DONE — `plugins enable/disable --output-format json` always emits `reload_runtime:true` regardless of whether state actually changed, and omits `previous_status`, `changed`, `version`, and `source` fields — automation cannot tell if a reload is necessary or if the mutation was a no-op** — dogfooded 2026-04-30 by Jobdori on `e939777f`. Running `claw plugins enable example-bundled --output-format json` on an already-enabled plugin returns `{"action":"enable","kind":"plugin","message":"…","reload_runtime":true,"target":"example-bundled"}` — `reload_runtime:true` every time, even on a no-op re-enable. The same applies to idempotent `disable`. Structured fields present: `action`, `kind`, `message`, `reload_runtime`, `target`. Structured fields absent: `previous_status`, `status`, `changed`, `version`, `source`. The actual plugin name, version, and new status are embedded only in the prose `message` field (`"Result  enabled example-bundled@bundled\n  Name  example-bundled\n  Version  0.1.0\n  Status  enabled"`), requiring callers to scrape column-aligned text to extract the post-mutation state. A no-op mutation emitting `reload_runtime:true` forces orchestration to trigger an expensive runtime reload even when no config change occurred. **Required fix shape:** (a) add `changed:bool` so callers can skip runtime reload when `changed:false`; (b) add `previous_status` and `status` fields (enums: `enabled`/`disabled`) so pre/post state is machine-readable without parsing `message`; (c) add `version` and `source` fields at the mutation response level, consistent with `plugins list` entry shape; (d) emit `reload_runtime:false` when `changed:false`; (e) add regression coverage proving idempotent enable/disable sets `changed:false` and `reload_runtime:false`. **Why this matters:** plugin lifecycle is a hot path for automation that conditionally enables plugins before running sessions. If every enable emits `reload_runtime:true` and no `changed` field exists, orchestration must reload unconditionally or maintain external state — both brittle patterns. Source: Jobdori live dogfood, `e939777f`, 2026-04-30.
 
 
 412. **DONE — `bootstrap-plan --output-format json` returns `phases: string[]` of raw Rust enum variant names with no description, steps, duration, or dependency metadata — unusable by automation** — dogfooded 2026-04-30 by Jobdori on `e939777f`. Running `claw bootstrap-plan --output-format json` returns `{"kind":"bootstrap-plan","phases":["CliEntry","FastPathVersion","StartupProfiler","SystemPromptFastPath","ChromeMcpFastPath","DaemonWorkerFastPath","BridgeFastPath","DaemonFastPath","BackgroundSessionFastPath","TemplateFastPath","EnvironmentRunnerFastPath","MainRuntime"]}`. The envelope has only two keys: `kind` and `phases`. The `phases` array contains 12 raw Rust enum variant name strings — opaque identifiers with no `description`, no `label`, no `steps[]`, no `estimated_ms`, no `dependencies[]`, no `optional:bool`, and no `status` (enabled/disabled/skipped). Automation that calls `bootstrap-plan` to understand startup costs or profile initialization paths receives 12 name strings that reveal nothing about what each phase does, how long it takes, whether it depends on credentials/network/MCP, or which ones can be skipped. **Required fix shape:** (a) replace `phases: string[]` with `phases: [{id, label, description, optional, estimated_ms?, dependencies?, status?}]`; (b) add a top-level `total_phases` count; (c) mark network/credential-dependent phases with a `requires_auth:bool` or `deps:["network","credentials","mcp"]` field so automation can plan for unavailability; (d) add regression coverage proving each phase entry has at least `id`, `label`, and `description` fields and that the count matches the phases array length. **Why this matters:** bootstrap-plan is the startup-cost introspection surface. If its JSON output is 12 opaque variant name strings, automation cannot profile startup, identify slow phases, skip optional phases, or present meaningful startup diagnostics — the entire command serves only as a list of internal identifiers. Source: Jobdori live dogfood, `e939777f`, 2026-04-30.

rust/crates/commands/src/lib.rs

@@ -2311,13 +2311,15 @@ pub fn handle_plugins_slash_command(
                 });
             };
             let plugin = resolve_plugin_target(manager, target)?;
+            let already_enabled = plugin.enabled;
             manager.enable(&plugin.metadata.id)?;
             Ok(PluginsCommandResult {
                 message: format!(
-                    "Plugins\n  Result           enabled {}\n  Name             {}\n  Version          {}\n  Status           enabled",
-                    plugin.metadata.id, plugin.metadata.name, plugin.metadata.version
+                    "Plugins\n  Result           {}\n  Name             {}\n  Version          {}\n  Status           enabled",
+                    if already_enabled { "already enabled" } else { "enabled" },
+                    plugin.metadata.name, plugin.metadata.version
                 ),
-                reload_runtime: true,
+                reload_runtime: !already_enabled,
             })
         }
         Some("disable") => {
@@ -2328,13 +2330,15 @@ pub fn handle_plugins_slash_command(
                 });
             };
             let plugin = resolve_plugin_target(manager, target)?;
+            let already_disabled = !plugin.enabled;
             manager.disable(&plugin.metadata.id)?;
             Ok(PluginsCommandResult {
                 message: format!(
-                    "Plugins\n  Result           disabled {}\n  Name             {}\n  Version          {}\n  Status           disabled",
-                    plugin.metadata.id, plugin.metadata.name, plugin.metadata.version
+                    "Plugins\n  Result           {}\n  Name             {}\n  Version          {}\n  Status           disabled",
+                    if already_disabled { "already disabled" } else { "disabled" },
+                    plugin.metadata.name, plugin.metadata.version
                 ),
-                reload_runtime: true,
+                reload_runtime: !already_disabled,
             })
         }
         Some("remove") | Some("uninstall") => {
@@ -7088,7 +7092,7 @@ mod tests {
         let disable = handle_plugins_slash_command(Some("disable"), Some("demo"), &mut manager)
             .expect("disable command should succeed");
         assert!(disable.reload_runtime);
-        assert!(disable.message.contains("disabled demo@external"));
+        assert!(disable.message.contains("Result           disabled"));
         assert!(disable.message.contains("Name             demo"));
         assert!(disable.message.contains("Status           disabled"));
 
@@ -7100,7 +7104,7 @@ mod tests {
         let enable = handle_plugins_slash_command(Some("enable"), Some("demo"), &mut manager)
             .expect("enable command should succeed");
         assert!(enable.reload_runtime);
-        assert!(enable.message.contains("enabled demo@external"));
+        assert!(enable.message.contains("Result           enabled"));
         assert!(enable.message.contains("Name             demo"));
         assert!(enable.message.contains("Status           enabled"));