test(coding-agent/mcp): defend getter-based tool descriptions against signature regression

Built-in tools whose prompt-rendered metadata depends on settings
(`TaskTool`, `SearchToolBm25Tool`, `EditTool`) expose `description`/
`label` via getters that re-evaluate on every access. The skip
optimization in `#applyActiveToolsByName` is correctness-safe for these
because `#computeAppliedToolSignature` reads `tool.description` live
each call, so a settings flip mutates the rendered string and differs
the signature on the next refresh.

This contract was implicit; a future refactor that caches per-tool
description strings would silently break it. Defending it explicitly:

- Added a regression test that wires a getter onto a CustomTool's
  `description`, verifies `refreshMCPTools` skips while the underlying
  state is unchanged, then mutates the state (without changing tool
  object identity) and verifies the rebuild fires.
- Expanded the `#computeAppliedToolSignature` docstring to document
  the getter-based coverage path and the SDK-init-time closure
  constants in `sdk.ts` that genuinely cannot change at runtime
  (`repeatToolDescriptions`, `eagerTasks`, `intentField`,
  `mcpDiscoveryEnabled`, `secretsEnabled`).

Triggered by a review question on whether the skip breaks settings-
based prompt changes. It does not, but the property is non-obvious.

packages/coding-agent/src/session/agent-session.ts

@@ -2309,10 +2309,20 @@ export class AgentSession {
 	 *      embeds these in the appended prompt under "## MCP Server Instructions".
 	 *      A server upgrade can change instructions while keeping tools identical.
 	 *
-	 * Inputs NOT covered: tool input schemas, memory instructions read from disk,
-	 * and any other ambient state the rebuild closure reads. Callers must explicitly
-	 * call `refreshBaseSystemPrompt()` after such side-effecting changes; see e.g.
-	 * the memory hooks and `#syncEditToolModeAfterModelChange`.
+	 * Settings-driven tool metadata is covered automatically: built-in tools that
+	 * depend on settings expose `description`/`label` via getters (see `TaskTool`,
+	 * `SearchToolBm25Tool`, `EditTool`), and the signature reads them live on every
+	 * call - so a settings flip that mutates the rendered string differs the signature
+	 * the next time `#applyActiveToolsByName` runs. Do not refactor `describeTool` to
+	 * cache per-tool strings without preserving this property.
+	 *
+	 * Inputs NOT covered: tool input schemas; memory instructions read from disk;
+	 * and SDK-init-time closure constants in `sdk.ts` (`repeatToolDescriptions`,
+	 * `eagerTasks`, `intentField`, `mcpDiscoveryEnabled`, `secretsEnabled`). The
+	 * closure-captured ones cannot change at runtime regardless of skip behavior.
+	 * For everything else, callers must explicitly call `refreshBaseSystemPrompt()`
+	 * after side-effecting changes; see e.g. the memory hooks and
+	 * `#syncEditToolModeAfterModelChange`.
 	 */
 	#computeAppliedToolSignature(toolNames: string[], tools: AgentTool[]): string {
 		// Order-preserving join: any reorder must produce a different signature so

packages/coding-agent/test/agent-session-tool-rebuild-skip.test.ts

@@ -334,4 +334,56 @@ describe("AgentSession refreshMCPTools rebuild skipping", () => {
 		expect(rebuildCount).toBe(3);
 	});
 
+	it("rebuilds when a tool's getter-based description reflects new settings state", async () => {
+		// Built-in tools whose prompt-rendered metadata depends on settings expose
+		// `description` via getters that re-evaluate on every access (TaskTool reads
+		// task.disabledAgents/maxConcurrency/isolation.mode/simple/async.enabled,
+		// SearchToolBm25Tool reads the discoverable MCP tool count, EditTool resolves
+		// through the current edit-mode definition). The signature reads `tool.description`
+		// live each call, so a settings flip that mutates the rendered string MUST differ
+		// the signature on the next `#applyActiveToolsByName`. Defending this contract
+		// against a future refactor that caches per-tool description strings.
+		let rebuildCount = 0;
+		const { session } = newSession(
+			async toolNames => {
+				rebuildCount++;
+				return `tools:${toolNames.join(",")}`;
+			},
+			// Discovery-on so the dynamic tool participates in the registry segment too,
+			// matching the production pattern where `TaskTool` and friends are always
+			// reachable via the registry.
+			{ mcpDiscoveryEnabled: true },
+		);
+
+		// Reuse the initially-active MCP name so the tool stays in the active list
+		// across refreshes - we want to defend the path where `tool.description` is read
+		// for the active descriptionSegment, not just the registrySegment.
+		const settingState = { disabled: "none" };
+		const dynamicTool = createMcpCustomTool("mcp__nucleus_search", "nucleus", "search", "placeholder");
+		Object.defineProperty(dynamicTool, "description", {
+			get: () => `dynamic disabled=${settingState.disabled}`,
+			enumerable: true,
+			configurable: true,
+		});
+
+		await session.refreshMCPTools([dynamicTool]);
+		const baseline = rebuildCount;
+		expect(baseline).toBeGreaterThanOrEqual(1);
+
+		// Same underlying state, same tool object identity: skip.
+		await session.refreshMCPTools([dynamicTool]);
+		expect(rebuildCount).toBe(baseline);
+
+		// Mutate the settings-backed state. The tool object identity does not change,
+		// but its `description` getter now returns a new string. The signature must
+		// pick this up live (no per-tool caching) and force a rebuild.
+		settingState.disabled = "plan,explore";
+		await session.refreshMCPTools([dynamicTool]);
+		expect(rebuildCount).toBe(baseline + 1);
+
+		// Same state again: skip.
+		await session.refreshMCPTools([dynamicTool]);
+		expect(rebuildCount).toBe(baseline + 1);
+	});
+
 });