Skip to content

Commit af940fd

Browse files
committed
refactor: single source of truth for tool registration
Remove getAvailableTools() - a static duplicate of the dynamic tool registration logic in getToolsForModel(). This eliminates divergence between what tools are listed vs actually registered. Changes: - Delete getAvailableTools() from toolDefinitions.ts - extractToolInstructions() now takes explicit toolNames[] instead of computing them from modelString - aiService registers tools first, then extracts instructions only for registered tools (fixes instruction extraction for unavailable tools) - Tokenizer no longer filters by model-specific availability (tools in messages were always available when called) - Add tests for extractToolInstructions() with new signature The tool registration logic (provider-specific web_search, conditional bash_background_*) remains in getToolsForModel() where it always was.
1 parent 1caa3d4 commit af940fd

File tree

6 files changed

+86
-69
lines changed

6 files changed

+86
-69
lines changed

src/common/utils/tools/toolDefinitions.ts

Lines changed: 0 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -322,45 +322,3 @@ export function getToolSchemas(): Record<string, ToolSchema> {
322322
])
323323
);
324324
}
325-
326-
/**
327-
* Get which tools are available for a given model
328-
* @param modelString The model string (e.g., "anthropic:claude-opus-4-1")
329-
* @returns Array of tool names available for the model
330-
*/
331-
export function getAvailableTools(modelString: string): string[] {
332-
const [provider] = modelString.split(":");
333-
334-
// Base tools available for all models
335-
const baseTools = [
336-
"bash",
337-
"bash_background_read",
338-
"bash_background_list",
339-
"bash_background_terminate",
340-
"file_read",
341-
"file_edit_replace_string",
342-
// "file_edit_replace_lines", // DISABLED: causes models to break repo state
343-
"file_edit_insert",
344-
"propose_plan",
345-
"todo_write",
346-
"todo_read",
347-
"status_set",
348-
"web_fetch",
349-
];
350-
351-
// Add provider-specific tools
352-
switch (provider) {
353-
case "anthropic":
354-
return [...baseTools, "web_search"];
355-
case "openai":
356-
// Only some OpenAI models support web search
357-
if (modelString.includes("gpt-4") || modelString.includes("gpt-5")) {
358-
return [...baseTools, "web_search"];
359-
}
360-
return baseTools;
361-
case "google":
362-
return [...baseTools, "google_search"];
363-
default:
364-
return baseTools;
365-
}
366-
}

src/common/utils/tools/tools.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ export type ToolFactory = (config: ToolConfiguration) => Tool;
5252
* @param additionalInstructions Additional instructions to append to the description
5353
* @returns The same tool instance with the augmented description
5454
*/
55-
function augmentToolDescription(baseTool: Tool, additionalInstructions: string): Tool {
55+
export function augmentToolDescription(baseTool: Tool, additionalInstructions: string): Tool {
5656
// Access the tool as a record to get its properties
5757
// eslint-disable-next-line @typescript-eslint/no-explicit-any
5858
const baseToolRecord = baseTool as any as Record<string, unknown>;

src/node/services/aiService.ts

Lines changed: 22 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import type { Config, ProviderConfig } from "@/node/config";
1616
import { StreamManager } from "./streamManager";
1717
import type { InitStateManager } from "./initStateManager";
1818
import type { SendMessageError } from "@/common/types/errors";
19-
import { getToolsForModel } from "@/common/utils/tools/tools";
19+
import { getToolsForModel, augmentToolDescription } from "@/common/utils/tools/tools";
2020
import { createRuntime } from "@/node/runtime/runtimeFactory";
2121
import { secretsToRecord } from "@/common/types/secrets";
2222
import type { MuxProviderOptions } from "@/common/types/providerOptions";
@@ -752,15 +752,8 @@ export class AIService extends EventEmitter {
752752
const streamToken = this.streamManager.generateStreamToken();
753753
const runtimeTempDir = await this.streamManager.createTempDirForStream(streamToken, runtime);
754754

755-
// Extract tool-specific instructions from AGENTS.md files
756-
const toolInstructions = await readToolInstructions(
757-
metadata,
758-
runtime,
759-
workspacePath,
760-
modelString
761-
);
762-
763755
// Get model-specific tools with workspace path (correct for local or remote)
756+
// Tools are registered first based on runtime capabilities
764757
const allTools = await getToolsForModel(
765758
modelString,
766759
{
@@ -772,10 +765,28 @@ export class AIService extends EventEmitter {
772765
workspaceId,
773766
},
774767
workspaceId,
775-
this.initStateManager,
776-
toolInstructions
768+
this.initStateManager
777769
);
778770

771+
// Extract tool-specific instructions only for registered tools
772+
// This ensures we don't extract instructions for tools that aren't available
773+
// (e.g., background tools in SSH workspaces or CLI paths without a manager)
774+
const registeredToolNames = Object.keys(allTools);
775+
const toolInstructions = await readToolInstructions(
776+
metadata,
777+
runtime,
778+
workspacePath,
779+
registeredToolNames
780+
);
781+
782+
// Apply tool instructions to registered tools
783+
for (const [toolName, instructions] of Object.entries(toolInstructions)) {
784+
const tool = allTools[toolName];
785+
if (tool) {
786+
augmentToolDescription(tool, instructions);
787+
}
788+
}
789+
779790
// Apply tool policy to filter tools (if policy provided)
780791
const tools = applyToolPolicy(allTools, toolPolicy);
781792

src/node/services/systemMessage.test.ts

Lines changed: 55 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import * as fs from "fs/promises";
22
import * as os from "os";
33
import * as path from "path";
4-
import { buildSystemMessage } from "./systemMessage";
4+
import { buildSystemMessage, extractToolInstructions } from "./systemMessage";
55
import type { WorkspaceMetadata } from "@/common/types/workspace";
66
import { DEFAULT_RUNTIME_CONFIG } from "@/common/constants/workspace";
77

@@ -436,3 +436,57 @@ Answer in two sentences max.
436436
}
437437
});
438438
});
439+
440+
describe("extractToolInstructions", () => {
441+
test("extracts instructions only for specified tools", () => {
442+
const globalInstructions = `
443+
# Tool: bash
444+
Custom bash instructions here.
445+
446+
# Tool: file_read
447+
Custom file_read instructions here.
448+
449+
# Tool: nonexistent_tool
450+
This should not be extracted.
451+
`;
452+
const result = extractToolInstructions(globalInstructions, null, ["bash", "file_read"]);
453+
454+
expect(Object.keys(result).sort()).toEqual(["bash", "file_read"]);
455+
expect(result.bash).toContain("Custom bash instructions");
456+
expect(result.file_read).toContain("Custom file_read instructions");
457+
expect(result.nonexistent_tool).toBeUndefined();
458+
});
459+
460+
test("prefers context instructions over global", () => {
461+
const globalInstructions = "# Tool: bash\nGlobal bash instructions";
462+
const contextInstructions = "# Tool: bash\nContext bash instructions";
463+
464+
const result = extractToolInstructions(globalInstructions, contextInstructions, ["bash"]);
465+
466+
expect(result.bash).toContain("Context bash instructions");
467+
expect(result.bash).not.toContain("Global bash instructions");
468+
});
469+
470+
test("falls back to global when context lacks tool section", () => {
471+
const globalInstructions = "# Tool: bash\nGlobal bash instructions";
472+
const contextInstructions = "# Tool: file_read\nContext file_read instructions";
473+
474+
const result = extractToolInstructions(globalInstructions, contextInstructions, [
475+
"bash",
476+
"file_read",
477+
]);
478+
479+
expect(result.bash).toContain("Global bash instructions");
480+
expect(result.file_read).toContain("Context file_read instructions");
481+
});
482+
483+
test("returns empty object when no matching tools", () => {
484+
const result = extractToolInstructions("# Tool: bash\nstuff", null, ["web_search"]);
485+
expect(result).toEqual({});
486+
});
487+
488+
test("returns empty object for empty tool list", () => {
489+
const result = extractToolInstructions("# Tool: bash\nstuff", null, []);
490+
expect(result).toEqual({});
491+
});
492+
});

src/node/services/systemMessage.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import {
1111
} from "@/node/utils/main/markdown";
1212
import type { Runtime } from "@/node/runtime/Runtime";
1313
import { getMuxHome } from "@/common/constants/paths";
14-
import { getAvailableTools } from "@/common/utils/tools/toolDefinitions";
1514

1615
// NOTE: keep this in sync with the docs/models.md file
1716

@@ -85,18 +84,17 @@ function getSystemDirectory(): string {
8584
*
8685
* @param globalInstructions Global instructions from ~/.mux/AGENTS.md
8786
* @param contextInstructions Context instructions from workspace/project AGENTS.md
88-
* @param modelString Active model identifier to determine available tools
87+
* @param toolNames List of registered tool names to extract instructions for
8988
* @returns Map of tool names to their additional instructions
9089
*/
9190
export function extractToolInstructions(
9291
globalInstructions: string | null,
9392
contextInstructions: string | null,
94-
modelString: string
93+
toolNames: string[]
9594
): Record<string, string> {
96-
const availableTools = getAvailableTools(modelString);
9795
const toolInstructions: Record<string, string> = {};
9896

99-
for (const toolName of availableTools) {
97+
for (const toolName of toolNames) {
10098
// Try context instructions first, then global
10199
const content =
102100
(contextInstructions && extractToolSection(contextInstructions, toolName)) ??
@@ -118,22 +116,22 @@ export function extractToolInstructions(
118116
* @param metadata - Workspace metadata (contains projectPath)
119117
* @param runtime - Runtime for reading workspace files (supports SSH)
120118
* @param workspacePath - Workspace directory path
121-
* @param modelString - Active model identifier to determine available tools
119+
* @param toolNames - List of registered tool names to extract instructions for
122120
* @returns Map of tool names to their additional instructions
123121
*/
124122
export async function readToolInstructions(
125123
metadata: WorkspaceMetadata,
126124
runtime: Runtime,
127125
workspacePath: string,
128-
modelString: string
126+
toolNames: string[]
129127
): Promise<Record<string, string>> {
130128
const [globalInstructions, contextInstructions] = await readInstructionSources(
131129
metadata,
132130
runtime,
133131
workspacePath
134132
);
135133

136-
return extractToolInstructions(globalInstructions, contextInstructions, modelString);
134+
return extractToolInstructions(globalInstructions, contextInstructions, toolNames);
137135
}
138136

139137
/**

src/node/utils/main/tokenizer.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import assert from "@/common/utils/assert";
22
import CRC32 from "crc-32";
33
import { LRUCache } from "lru-cache";
4-
import { getAvailableTools, getToolSchemas } from "@/common/utils/tools/toolDefinitions";
4+
import { getToolSchemas } from "@/common/utils/tools/toolDefinitions";
55
import type { CountTokensInput } from "./tokenizer.worker";
66
import { models, type ModelName } from "ai-tokenizer";
77
import { run } from "./workerPool";
@@ -178,14 +178,10 @@ export async function getToolDefinitionTokens(
178178
modelString: string
179179
): Promise<number> {
180180
try {
181-
const availableTools = getAvailableTools(modelString);
182-
if (!availableTools.includes(toolName)) {
183-
return 0;
184-
}
185-
186181
const toolSchemas = getToolSchemas();
187182
const toolSchema = toolSchemas[toolName];
188183
if (!toolSchema) {
184+
// Provider-native tools (web_search, google_search) have no schema in TOOL_DEFINITIONS
189185
return 40;
190186
}
191187

0 commit comments

Comments
 (0)