feat(core): add disabledTools workspace setting (#4175 Wave 4 PR 17)

doudouOUC · doudouOUC · commit c48439e00a85 · 2026-05-18T18:04:13.000+08:00
Introduces a per-workspace skip-registration mechanism for tool names, distinct from `permissions.deny` (which keeps the tool registered and blocks invocation). Tools listed in `disabledTools` are not registered at all and never appear in `/tools`, `getAllTools()`, or function-call discovery — both built-ins and MCP-discovered tools flow through `ToolRegistry.registerTool` / `registerFactory`, so gating there covers every registration path. - `ConfigParameters.disabledTools?: string[]` (frozen into a `ReadonlySet` at Config construction; queried via `Config.getDisabledTools()`) - `ToolRegistry.registerTool` and `ToolRegistry.registerFactory` skip when the tool name is in the disabled set, with a debug log line - New `settings.tools.disabled: string[]` (UNION merge across scopes), wired from `loadCliConfig` into ConfigParameters - Tests pin the contract: skip at register, lazy factory skip, and the "next refresh" semantic (already-registered tools are unaffected by a subsequent toggle — the disabled set is consulted at register time, not at lookup time) Foundation for the `POST /workspace/tools/:name/enable` route in a follow-up commit; the bridge will write the settings file directly, and the next ACP child spawn will pick up the change. 🤖 Generated with [Qwen Code](https://github.com/QwenLM/qwen-code)
diff --git a/packages/cli/src/config/config.ts b/packages/cli/src/config/config.ts
@@ -1419,6 +1419,20 @@ export async function loadCliConfig(
     addDisabled(name);
   }
 
+  // Resolve the per-workspace tool denylist (#4175 Wave 4 PR 17). De-duplicate
+  // while preserving original casing; downstream lookups go through
+  // `Config.getDisabledTools()` which materializes a Set, so the order here
+  // is only meaningful for diagnostic output.
+  const disabledTools: string[] = [];
+  const seenDisabledTools = new Set<string>();
+  for (const raw of settings.tools?.disabled ?? []) {
+    if (typeof raw !== 'string') continue;
+    const trimmed = raw.trim();
+    if (!trimmed || seenDisabledTools.has(trimmed)) continue;
+    seenDisabledTools.add(trimmed);
+    disabledTools.push(trimmed);
+  }
+
   // Helper: check if a tool is explicitly covered by an allow rule OR by the
   // coreTools whitelist. Uses alias matching for coreTools (via isToolEnabled)
   // to preserve the original behaviour where "ShellTool", "Shell", and
@@ -1636,6 +1650,7 @@ export async function loadCliConfig(
     excludeTools: mergedDeny,
     disabledSlashCommands:
       disabledSlashCommands.length > 0 ? disabledSlashCommands : undefined,
+    disabledTools: disabledTools.length > 0 ? disabledTools : undefined,
     // New unified permissions (PermissionManager source of truth).
     permissions: {
       allow: mergedAllow.length > 0 ? mergedAllow : undefined,
diff --git a/packages/cli/src/config/settingsSchema.ts b/packages/cli/src/config/settingsSchema.ts
@@ -1571,6 +1571,17 @@ const SETTINGS_SCHEMA = {
         showInDialog: false,
         mergeStrategy: MergeStrategy.UNION,
       },
+      disabled: {
+        type: 'array',
+        label: 'Disabled Tools',
+        category: 'Tools',
+        requiresRestart: true,
+        default: undefined as string[] | undefined,
+        description:
+          'Tool names hidden from the registry. Differs from permissions.deny: disabled tools are not registered at all, so they never appear in /tools and cannot be discovered by the model. Managed by the daemon mutation route POST /workspace/tools/:name/enable.',
+        showInDialog: false,
+        mergeStrategy: MergeStrategy.UNION,
+      },
       approvalMode: {
         type: 'enum',
         label: 'Tool Approval Mode',
diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts
@@ -470,6 +470,18 @@ export interface ConfigParameters {
    * the `QWEN_DISABLED_SLASH_COMMANDS` environment variable.
    */
   disabledSlashCommands?: string[];
+  /**
+   * Tool names hidden from the registry at construction time. Unlike
+   * `permissions.deny` (which keeps the tool registered and rejects
+   * invocation), tools listed here are not registered at all and never
+   * appear in `/tools`, `getAllTools()`, or function-call discovery.
+   * Sourced from `settings.tools.disabled` and the daemon mutation route
+   * `POST /workspace/tools/:name/enable {enabled:false}` (#4175 Wave 4 PR
+   * 17). Active sessions retain already-registered tools — the disabled
+   * set is consulted at register time, so toggling takes effect on the
+   * next ACP child spawn or `ToolRegistry.refresh()`.
+   */
+  disabledTools?: string[];
   /** Merged permission rules from all sources (settings + CLI args). */
   permissions?: {
     allow?: string[];
@@ -742,6 +754,7 @@ export class Config {
   private readonly allowedTools: string[] | undefined;
   private readonly excludeTools: string[] | undefined;
   private readonly disabledSlashCommands: readonly string[];
+  private readonly disabledTools: ReadonlySet<string>;
   private readonly permissionsAllow: string[];
   private readonly permissionsAsk: string[];
   private readonly permissionsDeny: string[];
@@ -896,6 +909,7 @@ export class Config {
     this.disabledSlashCommands = Object.freeze([
       ...(params.disabledSlashCommands ?? []),
     ]);
+    this.disabledTools = new Set(params.disabledTools ?? []);
     this.permissionsAllow = params.permissions?.allow || [];
     this.permissionsAsk = params.permissions?.ask || [];
     this.permissionsDeny = params.permissions?.deny || [];
@@ -2267,6 +2281,17 @@ export class Config {
     return this.disabledSlashCommands;
   }
 
+  /**
+   * Returns the read-only set of tool names hidden from this Config's
+   * ToolRegistry. Consulted by `ToolRegistry.registerTool` and
+   * `ToolRegistry.registerFactory` to skip registration. Toggling at
+   * runtime requires re-spawning the ACP child (the set is frozen at
+   * construction time). See `disabledTools` in ConfigParameters.
+   */
+  getDisabledTools(): ReadonlySet<string> {
+    return this.disabledTools;
+  }
+
   getToolCallCommand(): string | undefined {
     return this.toolCallCommand;
   }
diff --git a/packages/core/src/tools/tool-registry.test.ts b/packages/core/src/tools/tool-registry.test.ts
@@ -195,6 +195,63 @@ describe('ToolRegistry', () => {
       expect(resolved).not.toBeInstanceOf(DiscoveredMCPTool);
       expect(resolved!.name).toBe('structured_output');
     });
+
+    it('skips tools whose name is in Config.disabledTools (#4175 Wave 4 PR 17)', () => {
+      const disabledConfig = new Config({
+        ...baseConfigParams,
+        disabledTools: ['Bash', 'mcp__github__create_issue'],
+      });
+      const registry = new ToolRegistry(disabledConfig);
+      registry.registerTool(new MockTool({ name: 'Bash' }));
+      registry.registerTool(new MockTool({ name: 'Read' }));
+      registry.registerTool(
+        new MockTool({ name: 'mcp__github__create_issue' }),
+      );
+      expect(registry.getTool('Bash')).toBeUndefined();
+      expect(registry.getTool('Read')).toBeDefined();
+      expect(registry.getTool('mcp__github__create_issue')).toBeUndefined();
+    });
+
+    it('skips lazy factories whose name is in Config.disabledTools', async () => {
+      const disabledConfig = new Config({
+        ...baseConfigParams,
+        disabledTools: ['structured_output'],
+      });
+      const registry = new ToolRegistry(disabledConfig);
+      registry.registerFactory(
+        'structured_output',
+        async () => new MockTool({ name: 'structured_output' }),
+      );
+      registry.registerFactory(
+        'sequential_thinking',
+        async () => new MockTool({ name: 'sequential_thinking' }),
+      );
+      // Disabled factory never materializes.
+      expect(await registry.ensureTool('structured_output')).toBeUndefined();
+      // Non-disabled factory still materializes.
+      const live = await registry.ensureTool('sequential_thinking');
+      expect(live).toBeDefined();
+      expect(live!.name).toBe('sequential_thinking');
+    });
+
+    it('does not retroactively unregister tools registered before toggle (next-refresh semantic)', () => {
+      // Toggle semantics are documented as "effective on next refresh /
+      // ACP child spawn"; a Set lookup at register time cannot un-do a
+      // prior registration. This test pins the contract.
+      const registry = new ToolRegistry(config);
+      registry.registerTool(new MockTool({ name: 'live-tool' }));
+      expect(registry.getTool('live-tool')).toBeDefined();
+      // Simulate a "fresh Config" with the tool now disabled.
+      const reconfigured = new Config({
+        ...baseConfigParams,
+        disabledTools: ['live-tool'],
+      });
+      const next = new ToolRegistry(reconfigured);
+      next.registerTool(new MockTool({ name: 'live-tool' }));
+      // The new registry skips; the old registry is unaffected.
+      expect(next.getTool('live-tool')).toBeUndefined();
+      expect(registry.getTool('live-tool')).toBeDefined();
+    });
   });
 
   describe('getAllTools', () => {
diff --git a/packages/core/src/tools/tool-registry.ts b/packages/core/src/tools/tool-registry.ts
@@ -204,11 +204,29 @@ export class ToolRegistry {
     );
   }
 
+  /**
+   * Returns true when `name` is in the Config's `disabledTools` set, in
+   * which case `registerTool` / `registerFactory` will skip it. This is
+   * the chokepoint for the daemon mutation route at `POST /workspace/
+   * tools/:name/enable {enabled:false}` (#4175 Wave 4 PR 17); both
+   * built-ins and MCP-discovered tools flow through `registerTool`, so
+   * gating here covers every registration path.
+   */
+  private isToolDisabled(name: string): boolean {
+    return this.config.getDisabledTools().has(name);
+  }
+
   /**
    * Registers a tool definition.
    * @param tool - The tool object containing schema and execution logic.
    */
   registerTool(tool: AnyDeclarativeTool): void {
+    if (this.isToolDisabled(tool.name)) {
+      debugLogger.info(
+        `Tool "${tool.name}" skipped: present in disabledTools set.`,
+      );
+      return;
+    }
     // A name collision can happen against either the eager `tools` map
     // (already-instantiated tools) or the lazy `factories` map (registered
     // but not yet constructed — `structured_output` lives here when
@@ -240,6 +258,12 @@ export class ToolRegistry {
    * is not instantiated until {@link ensureTool} or {@link warmAll} is called.
    */
   registerFactory(name: string, factory: ToolFactory): void {
+    if (this.isToolDisabled(name)) {
+      debugLogger.info(
+        `Tool factory "${name}" skipped: present in disabledTools set.`,
+      );
+      return;
+    }
     this.factories.set(name, factory);
   }
 
