From 4f2f679559e3bfeccce9575f118ffcadf6c429a3 Mon Sep 17 00:00:00 2001
From: Aleksander Grygier <aleksander.grygier@gmail.com>
Date: Thu, 27 Aug 2026 06:44:24 +0200
Subject: [PATCH] ui: clean up tool key helpers and store docs

Documents getEnabledToolsForLLM properly, unstacks the JSDoc at
isEntryEnabled, makes setToolEnabled persist like setCategoryEnabled
(toggleTool now delegates to it), and routes the serverId-less MCP
branch of toolKey through getMcpServerToolsKey so both key formats
come from one place. Preferences banner comments become plain
comments so they no longer read as class member docs.

Assisted-by: pi
---
 .../conversations/preferences.svelte.ts       | 26 +++------------
 tools/ui/src/lib/stores/tools.svelte.ts       | 33 ++++++++-----------
 2 files changed, 18 insertions(+), 41 deletions(-)

diff --git a/tools/ui/src/lib/stores/conversations/preferences.svelte.ts b/tools/ui/src/lib/stores/conversations/preferences.svelte.ts
index 5e75f1d7c6..fdbf5da99f 100644
--- a/tools/ui/src/lib/stores/conversations/preferences.svelte.ts
+++ b/tools/ui/src/lib/stores/conversations/preferences.svelte.ts
@@ -86,6 +86,8 @@ export class ConversationPreferences {
 		return buildDisabledToolCategories(this.host.activeConversation);
 	}
 
+	// Tool Policy
+
 	// getters, not $derived fields: lazy evaluation keeps them off the class
 	// field initialization order (host is assigned by the constructor), and
 	// reads of the underlying $state stay tracked in reactive contexts
@@ -93,14 +95,6 @@ export class ConversationPreferences {
 		return buildDisabledTools(this.host.activeConversation);
 	}
 
-	/**
-	 *
-	 *
-	 * Tool Policy
-	 *
-	 *
-	 */
-
 	constructor(private host: ConversationsPreferencesHost) {}
 
 	/** Effective disabled tool categories for the current context, captured at flow start. */
@@ -192,13 +186,7 @@ export class ConversationPreferences {
 		this.pendingCwd = null;
 	}
 
-	/**
-	 *
-	 *
-	 * Working Directory
-	 *
-	 *
-	 */
+	// Working Directory
 
 	/**
 	 * Sets the working directory for the active conversation. Pass `null` or
@@ -233,13 +221,7 @@ export class ConversationPreferences {
 		this.pendingCwd = null;
 	}
 
-	/**
-	 *
-	 *
-	 * Reasoning Effort
-	 *
-	 *
-	 */
+	// Reasoning Effort
 
 	/**
 	 * Sets the reasoning effort for the active conversation.
diff --git a/tools/ui/src/lib/stores/tools.svelte.ts b/tools/ui/src/lib/stores/tools.svelte.ts
index 7ce9c7b95f..db05e3cd55 100644
--- a/tools/ui/src/lib/stores/tools.svelte.ts
+++ b/tools/ui/src/lib/stores/tools.svelte.ts
@@ -263,6 +263,14 @@ class ToolsStore {
 		}
 	}
 
+	/**
+	 * Enabled tool definitions for sending to the LLM. Callers pass an
+	 * explicit policy (the active conversation's, resolved with global
+	 * defaults when absent); without arguments the store defaults apply.
+	 * MCP tool schemas are normalized here so the wire payload is consistent
+	 * across all four sources (server, browser/sandbox, MCP, custom JSON).
+	 * The API identifies tools by name, so a name is sent at most once.
+	 */
 	getEnabledToolsForLLM(
 		disabledTools: ReadonlySet<string> = this._disabledTools,
 		disabledCategories: ReadonlySet<ToolSource> = this._disabledToolCategories
@@ -396,17 +404,6 @@ class ToolsStore {
 		return !this._disabledToolCategories.has(source);
 	}
 
-	/**
-	 * Enabled tool definitions for sending to the LLM.
-	 * MCP tool schemas are normalized here so the wire payload is consistent
-	 * across all four sources (server, browser/sandbox, MCP, custom JSON).
-	 * The API identifies tools by name, so a name is sent at most once.
-	 */
-	/**
-	 * The single enable rule for a tool entry: its category must be on, its
-	 * own key must not be disabled, and for MCP tools the server-scoped group
-	 * key must not be disabled either.
-	 */
 	isEntryEnabled(
 		entry: ToolEntry,
 		disabledTools: ReadonlySet<string>,
@@ -469,6 +466,8 @@ class ToolsStore {
 		} else {
 			this._disabledTools.add(key);
 		}
+
+		this.persistDisabledTools();
 	}
 
 	toggleCategory(source: ToolSource): void {
@@ -476,13 +475,7 @@ class ToolsStore {
 	}
 
 	toggleTool(key: string): void {
-		if (this._disabledTools.has(key)) {
-			this._disabledTools.delete(key);
-		} else {
-			this._disabledTools.add(key);
-		}
-
-		this.persistDisabledTools();
+		this.setToolEnabled(key, !this.isToolEnabled(key));
 	}
 
 	/** First canonical entry matching a tool name, runtime tool calls resolve by name */
@@ -710,7 +703,9 @@ class ToolsStore {
 	private toolKey(source: ToolSource, name: string, serverId?: string): string {
 		switch (source) {
 			case ToolSource.MCP:
-				return serverId ? `mcp-${serverId}:${name}` : `mcp:${name}`;
+				// with a serverId this is a per-tool key; without one it hits the
+				// server group key shape, which no MCP entry ever does
+				return serverId ? `mcp-${serverId}:${name}` : this.getMcpServerToolsKey(name);
 			case ToolSource.CUSTOM:
 				return `custom:${name}`;
 			case ToolSource.BROWSER: