Merge pull request #741 from corbitsdev/cl-7296-hide-commands-for-disabled-skills

Enforce live plugin command activation

Author: TheGreatAxios

src/plugins/register.ts

@@ -39,19 +39,20 @@ export function enablePluginConfig(
   return { ...config, [id]: { ...prev, enabled: true } };
 }
 
-export function isEnabledCommandPlugin(
-  mod: PluginModule,
-  config: Record<string, PluginConfig>,
-): boolean {
+function isCommandPluginModule(mod: PluginModule): boolean {
   if (mod.commandPlugin === undefined) return false;
   const kind = mod.manifest?.kind;
   // command/workflow plugins own their slash commands; agent plugins may also
   // contribute commands (e.g. a Claude marketplace plugin's tagged skills), so
   // commands wire as an added surface without changing the plugin's primary kind.
-  return (
-    (kind === "command" || kind === "workflow" || kind === "agent") &&
-    isPluginModuleEnabled(mod, config)
-  );
+  return kind === "command" || kind === "workflow" || kind === "agent";
+}
+
+export function isEnabledCommandPlugin(
+  mod: PluginModule,
+  config: Record<string, PluginConfig>,
+): boolean {
+  return isCommandPluginModule(mod) && isPluginModuleEnabled(mod, config);
 }
 
 export function isEnabledWorkflowPlugin(
@@ -65,15 +66,27 @@ export function isEnabledWorkflowPlugin(
   );
 }
 
+export function registerCommandPluginModule(
+  mod: PluginModule,
+  getConfig: () => Record<string, PluginConfig>,
+): boolean {
+  if (!isCommandPluginModule(mod)) return false;
+  const commandPlugin = mod.commandPlugin;
+  if (commandPlugin === undefined) return false;
+  registerCommandPlugin(commandPlugin, () => isPluginModuleEnabled(mod, getConfig()));
+  return true;
+}
+
 export function registerCommandPlugins(
   modules: PluginModule[],
-  config: Record<string, PluginConfig>,
+  config: Record<string, PluginConfig> | (() => Record<string, PluginConfig>),
 ): string[] {
+  const getConfig = typeof config === "function" ? config : () => config;
   const registered: string[] = [];
   for (const mod of modules) {
-    if (!isEnabledCommandPlugin(mod, config)) continue;
-    registerCommandPlugin(mod.commandPlugin!);
-    registered.push(mod.manifest!.id);
+    const id = mod.manifest?.id;
+    if (id === undefined || !registerCommandPluginModule(mod, getConfig)) continue;
+    if (isPluginModuleEnabled(mod, getConfig())) registered.push(id);
   }
   return registered;
 }

src/tui/command-catalog.ts

@@ -4,7 +4,7 @@
  * Pure: host injects `listCommands()` results (or fixtures). No registry import
  * here — avoids circular / heavy deps from `src/tui/commands`.
  *
- *   setPaletteCatalog(shell, commandItemsFromRegistry(listCommands()))
+ *   setPaletteCatalog(shell, () => commandItemsFromRegistry(listCommands()))
  */
 
 import { sliceToWidth, stringWidth } from "./view/height.js";

src/tui/command-registry-setup.test.ts

@@ -1,6 +1,8 @@
 import { describe, test, expect } from "bun:test";
 import { setUpCommandRegistry } from "./runner.js";
 import { getCommand, listCommands } from "./commands/registry.js";
+import type { PluginConfig } from "../config/settings.js";
+import type { PluginModule } from "../plugins/loader.js";
 
 // Built-in registration once rode on an import side effect; deleting its only
 // importer emptied the registry with no type error and no failing test.
@@ -21,4 +23,60 @@ describe("session command registry setup", () => {
     expect(listCommands().map((c) => c.name)).not.toContain("help");
     expect(getCommand("help")).toBeDefined();
   });
+
+  test("resolves plugin command candidates against live canonical config", () => {
+    const plugin = (
+      id: string,
+      description: string,
+      name = "live-config-command",
+    ): PluginModule => ({
+      manifest: { id, name: id, kind: "command" },
+      origin: "user",
+      commandPlugin: {
+        commands: [
+          {
+            name,
+            description,
+            handler: () => ({ type: "message", text: description }),
+          },
+        ],
+      },
+    });
+    let config: Record<string, PluginConfig> = {
+      "disabled-command-plugin": { enabled: false },
+      "enabled-command-plugin": { enabled: true },
+      "help-collision-plugin": { enabled: true },
+    };
+
+    setUpCommandRegistry(
+      { providers: {}, plugins: config },
+      [
+        plugin("disabled-command-plugin", "disabled"),
+        plugin("enabled-command-plugin", "enabled"),
+        plugin("help-collision-plugin", "plugin help", "help"),
+      ],
+      () => config,
+    );
+
+    expect(getCommand("live-config-command")?.description).toBe("enabled");
+    expect(getCommand("live-config-command")?.handler("", { signalClear: () => {} })).toEqual({
+      type: "message",
+      text: "enabled",
+    });
+    expect(getCommand("help")?.description).not.toBe("plugin help");
+
+    config = {
+      ...config,
+      "disabled-command-plugin": { enabled: true },
+      "enabled-command-plugin": { enabled: false },
+    };
+    expect(getCommand("live-config-command")?.description).toBe("disabled");
+
+    config = {
+      ...config,
+      "disabled-command-plugin": { enabled: false },
+    };
+    expect(getCommand("live-config-command")).toBeUndefined();
+    expect(listCommands().map((command) => command.name)).not.toContain("live-config-command");
+  });
 });

src/tui/commands/registry.test.ts

@@ -69,6 +69,18 @@ describe("command registry", () => {
     expect(def?.handler("", ctx)).toEqual({ type: "message", text: "built-in" });
   });
 
+  it("keeps command-specific availability visibility-only", () => {
+    registerCommand({
+      name: "unavailable-but-callable",
+      description: "visibility gated",
+      available: () => false,
+      handler: () => ({ type: "noop" }),
+    });
+
+    expect(listCommands().map((command) => command.name)).not.toContain("unavailable-but-callable");
+    expect(getCommand("unavailable-but-callable")).toBeDefined();
+  });
+
   it("invokes handler with args and context", () => {
     let receivedArgs = "";
     let clearCalled = false;
@@ -102,6 +114,141 @@ describe("registerCommandPlugin", () => {
     expect(getCommand("plugin-cmd-a")).toBeDefined();
     expect(getCommand("plugin-cmd-b")).toBeDefined();
   });
+
+  it("re-resolves activation for discovery and execution", () => {
+    let active = true;
+    registerCommandPlugin(
+      {
+        commands: [
+          {
+            name: "live-plugin-cmd",
+            description: "live plugin",
+            handler: () => ({ type: "message", text: "ran" }),
+          },
+        ],
+      },
+      () => active,
+    );
+
+    expect(listCommands().map((command) => command.name)).toContain("live-plugin-cmd");
+    expect(getCommand("live-plugin-cmd")?.handler("", ctx)).toEqual({
+      type: "message",
+      text: "ran",
+    });
+
+    active = false;
+    expect(listCommands().map((command) => command.name)).not.toContain("live-plugin-cmd");
+    expect(getCommand("live-plugin-cmd")).toBeUndefined();
+
+    active = true;
+    expect(listCommands().map((command) => command.name)).toContain("live-plugin-cmd");
+    expect(getCommand("live-plugin-cmd")).toBeDefined();
+  });
+
+  it("serves the next candidate when the first plugin deactivates", () => {
+    let firstActive = true;
+    registerCommandPlugin(
+      {
+        commands: [
+          {
+            name: "plugin-live-fallback",
+            description: "first",
+            handler: () => ({ type: "noop" }),
+          },
+        ],
+      },
+      () => firstActive,
+    );
+    registerCommandPlugin({
+      commands: [
+        {
+          name: "plugin-live-fallback",
+          description: "second",
+          handler: () => ({ type: "noop" }),
+        },
+      ],
+    });
+
+    expect(getCommand("plugin-live-fallback")?.description).toBe("first");
+    expect(
+      listCommands().find((command) => command.name === "plugin-live-fallback")?.description,
+    ).toBe("first");
+
+    firstActive = false;
+    expect(getCommand("plugin-live-fallback")?.description).toBe("second");
+    expect(
+      listCommands().find((command) => command.name === "plugin-live-fallback")?.description,
+    ).toBe("second");
+  });
+
+  it("does not execute a typed slash name after the plugin deactivates", () => {
+    let active = true;
+    registerCommandPlugin(
+      {
+        commands: [
+          {
+            name: "typed-after-disable",
+            description: "typed",
+            handler: () => ({ type: "message", text: "ran" }),
+          },
+        ],
+      },
+      () => active,
+    );
+
+    const stalePaletteRow = "typed-after-disable";
+    expect(getCommand(stalePaletteRow)).toBeDefined();
+    active = false;
+    expect(getCommand(stalePaletteRow)).toBeUndefined();
+  });
+
+  it("lets an enabled plugin claim a name ahead of a disabled candidate", () => {
+    registerCommandPlugin(
+      {
+        commands: [
+          {
+            name: "plugin-candidate-collision",
+            description: "disabled candidate",
+            handler: () => ({ type: "noop" }),
+          },
+        ],
+      },
+      () => false,
+    );
+    registerCommandPlugin(
+      {
+        commands: [
+          {
+            name: "plugin-candidate-collision",
+            description: "enabled candidate",
+            handler: () => ({ type: "noop" }),
+          },
+        ],
+      },
+      () => true,
+    );
+
+    expect(getCommand("plugin-candidate-collision")?.description).toBe("enabled candidate");
+  });
+
+  it("never lets a plugin collision replace a built-in command", () => {
+    registerCommand({
+      name: "built-in-plugin-collision",
+      description: "built-in",
+      handler: () => ({ type: "noop" }),
+    });
+    registerCommandPlugin({
+      commands: [
+        {
+          name: "built-in-plugin-collision",
+          description: "plugin",
+          handler: () => ({ type: "noop" }),
+        },
+      ],
+    });
+
+    expect(getCommand("built-in-plugin-collision")?.description).toBe("built-in");
+  });
 });
 
 describe("setHiddenCommands", () => {

src/tui/commands/registry.ts

@@ -65,7 +65,13 @@ export interface CommandPlugin {
   commands: CommandDefinition[];
 }
 
+interface PluginCommandCandidate {
+  command: CommandDefinition;
+  isActive: () => boolean;
+}
+
 const registry = new Map<string, CommandDefinition>();
+const pluginCandidates = new Map<string, PluginCommandCandidate[]>();
 const hidden = new Set<string>();
 
 export function registerCommand(def: CommandDefinition): void {
@@ -75,9 +81,14 @@ export function registerCommand(def: CommandDefinition): void {
   registry.set(def.name, def);
 }
 
-export function registerCommandPlugin(plugin: CommandPlugin): void {
+export function registerCommandPlugin(
+  plugin: CommandPlugin,
+  isActive: () => boolean = () => true,
+): void {
   for (const cmd of plugin.commands) {
-    registerCommand(cmd);
+    const candidates = pluginCandidates.get(cmd.name) ?? [];
+    candidates.push({ command: cmd, isActive });
+    pluginCandidates.set(cmd.name, candidates);
   }
 }
 
@@ -87,11 +98,19 @@ export function setHiddenCommands(names: string[]): void {
 }
 
 export function getCommand(name: string): CommandDefinition | undefined {
-  return registry.get(name);
+  const registered = registry.get(name);
+  if (registered !== undefined) return registered;
+  return pluginCandidates.get(name)?.find((candidate) => candidate.isActive())?.command;
 }
 
 export function listCommands(): CommandDefinition[] {
-  return [...registry.values()]
+  const commands = [...registry.values()];
+  for (const name of pluginCandidates.keys()) {
+    if (registry.has(name)) continue;
+    const command = getCommand(name);
+    if (command !== undefined) commands.push(command);
+  }
+  return commands
     .filter((c) => !hidden.has(c.name) && (c.available === undefined || c.available()))
     .sort((a, b) => a.name.localeCompare(b.name));
 }

src/tui/product-host.ts

@@ -149,7 +149,7 @@ export interface ProductHostConfig {
    */
   readonly addProviderChoices?: () => readonly ProductHostAddProviderChoice[];
   /** Command palette catalog (registry-backed). */
-  readonly commands?: readonly PaletteCommand[];
+  readonly commands?: readonly PaletteCommand[] | (() => readonly PaletteCommand[]);
   readonly onCommand?: (name: string) => void;
   /** Optional initial chrome snapshot. */
   readonly chrome?: ChromeLiveState | null;
@@ -326,7 +326,7 @@ export async function mountProductHost(config: ProductHostConfig): Promise<Produ
   // while still opting this host into the quota-retry / stall timers.
   const bridge = attachSessionBridge(shell, port, config.turnMonitor ?? {});
 
-  if (config.commands !== undefined && config.commands.length > 0) {
+  if (config.commands !== undefined) {
     setPaletteCatalog(shell, config.commands);
   }
   if (config.onCommand) {