Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 18 additions & 12 deletions extensions/cli/src/permissions/permissionChecker.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -601,13 +601,16 @@ describe("Permission Checker", () => {
describe("Hybrid Permission Model with Dynamic Evaluation", () => {
// Mock the runTerminalCommand tool with evaluateToolCallPolicy
const mockBashTool = {
name: "Bash",
displayName: "Bash",
description: "Execute bash commands",
parameters: {
type: "object" as const,
properties: {},
type: "function" as const,
function: {
name: "Bash",
description: "Execute bash commands",
parameters: {
type: "object" as const,
properties: {},
},
},
displayName: "Bash",
isBuiltIn: true,
evaluateToolCallPolicy: vi.fn(),
run: vi.fn(),
Expand Down Expand Up @@ -882,13 +885,16 @@ describe("Permission Checker", () => {
it("should handle tools without dynamic evaluation", () => {
// Mock a Read tool without evaluateToolCallPolicy
const mockReadTool = {
name: "Read",
displayName: "Read",
description: "Read files",
parameters: {
type: "object" as const,
properties: {},
type: "function" as const,
function: {
name: "Read",
description: "Read files",
parameters: {
type: "object" as const,
properties: {},
},
},
displayName: "Read",
isBuiltIn: true,
run: vi.fn(),
// No evaluateToolCallPolicy
Expand Down
2 changes: 1 addition & 1 deletion extensions/cli/src/permissions/permissionChecker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,7 @@ export function checkToolPermission(

// Check if tool has dynamic policy evaluation
const builtinTools = getAllBuiltinTools();
const tool = builtinTools.find((t) => t.name === toolCall.name);
const tool = builtinTools.find((t) => t.function.name === toolCall.name);

if (tool?.evaluateToolCallPolicy) {
// Convert CLI permission to core policy
Expand Down
12 changes: 6 additions & 6 deletions extensions/cli/src/stream/handleToolCalls.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ export async function handleToolCalls(
export async function getAllTools() {
// Get all available tool names
const allBuiltinTools = getAllBuiltinTools();
const builtinToolNames = allBuiltinTools.map((tool) => tool.name);
const builtinToolNames = allBuiltinTools.map((tool) => tool.function.name);

let mcpTools: MCPTool[] = [];
let mcpToolNames: string[] = [];
Expand Down Expand Up @@ -223,18 +223,18 @@ export async function getAllTools() {

// Filter builtin tools
const allowedBuiltinTools = allBuiltinTools.filter((tool) =>
allowedToolNamesSet.has(tool.name),
allowedToolNamesSet.has(tool.function.name),
);

const allTools: ChatCompletionTool[] = allowedBuiltinTools.map((tool) => ({
type: "function" as const,
function: {
name: tool.name,
description: tool.description,
name: tool.function.name,
description: tool.function.description,
parameters: {
type: "object",
required: tool.parameters.required,
properties: tool.parameters.properties,
required: tool.function.parameters.required,
properties: tool.function.parameters.properties,
},
},
}));
Expand Down
15 changes: 9 additions & 6 deletions extensions/cli/src/stream/streamChatResponse.helpers.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@ describe("streamChatResponse.helpers", () => {
argumentsStr: '{"filepath":"test.txt","content":"hello"}',
startNotified: false,
tool: {
name: "Write",
displayName: "Write",
description: "Write to a file",
parameters: {
type: "object",
properties: {},
type: "function",
function: {
name: "Write",
description: "Write to a file",
parameters: {
type: "object",
properties: {},
},
},
displayName: "Write",
run: vi.fn(),
isBuiltIn: true,
},
Expand Down
4 changes: 2 additions & 2 deletions extensions/cli/src/stream/streamChatResponse.helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export async function handleHeadlessPermission(
toolCall: PreprocessedToolCall,
): Promise<never> {
const allBuiltinTools = getAllBuiltinTools();
const tool = allBuiltinTools.find((t) => t.name === toolCall.name);
const tool = allBuiltinTools.find((t) => t.function.name === toolCall.name);
const toolName = tool?.displayName || toolCall.name;

// Import safeStderr to bypass console blocking in headless mode
Expand Down Expand Up @@ -334,7 +334,7 @@ export async function preprocessStreamedToolCalls(
for (const toolCall of toolCalls) {
const startTime = Date.now();
try {
const tool = availableTools.find((t) => t.name === toolCall.name);
const tool = availableTools.find((t) => t.function.name === toolCall.name);
if (!tool) {
throw new Error(`Tool ${toolCall.name} not found`);
}
Expand Down
65 changes: 34 additions & 31 deletions extensions/cli/src/tools/edit.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function validateAndResolveFilePath(args: any): {
if (!readFilesSet.has(resolvedPath)) {
throw new ContinueError(
ContinueErrorReason.EditToolFileNotRead,
`You must use the ${readFileTool.name} tool to read ${file_path} before editing it.`,
`You must use the ${readFileTool.function.name} tool to read ${file_path} before editing it.`,
);
}

Expand All @@ -62,47 +62,50 @@ export interface EditArgs extends EditOperation {
}

export const editTool: Tool = {
name: "Edit",
displayName: "Edit",
readonly: false,
isBuiltIn: true,
description: `Performs exact string replacements in a file.
type: "function",
function: {
name: "Edit",
description: `Performs exact string replacements in a file.

USAGE:
- ALWAYS use the \`${readFileTool.name}\` tool just before making edits, to understand the file's up-to-date contents and context.
- When editing text from ${readFileTool.name} tool output, ensure you preserve exact whitespace/indentation.
- ALWAYS use the \`${readFileTool.function.name}\` tool just before making edits, to understand the file's up-to-date contents and context.
- When editing text from ${readFileTool.function.name} tool output, ensure you preserve exact whitespace/indentation.
- Always prefer editing existing files in the codebase. NEVER write new files unless explicitly required.
- Only use emojis if the user explicitly requests it. Avoid adding emojis to files unless asked.
- Use \`replace_all\` for replacing and renaming strings across the file. This parameter is useful if you want to rename a variable, for instance.

WARNINGS:
- When not using \`replace_all\`, the edit will FAIL if \`old_string\` is not unique in the file. Either provide a larger string with more surrounding context to make it unique or use \`replace_all\` to change every instance of \`old_string\`.
- The edit will FAIL if you have not recently used the \`${readFileTool.name}\` tool to view up-to-date file contents.`,
parameters: {
type: "object",
required: ["file_path", "old_string", "new_string"],
properties: {
file_path: {
type: "string",
description:
"Absolute or relative path to the file to modify. Absolute preferred",
},
old_string: {
type: "string",
description:
"The text to replace - must be exact including whitespace/indentation",
},
new_string: {
type: "string",
description:
"The text to replace it with (MUST be different from old_string)",
},
replace_all: {
type: "boolean",
description: "Replace all occurrences of old_string (default false)",
- The edit will FAIL if you have not recently used the \`${readFileTool.function.name}\` tool to view up-to-date file contents.`,
parameters: {
type: "object",
required: ["file_path", "old_string", "new_string"],
properties: {
file_path: {
type: "string",
description:
"Absolute or relative path to the file to modify. Absolute preferred",
},
old_string: {
type: "string",
description:
"The text to replace - must be exact including whitespace/indentation",
},
new_string: {
type: "string",
description:
"The text to replace it with (MUST be different from old_string)",
},
replace_all: {
type: "boolean",
description: "Replace all occurrences of old_string (default false)",
},
},
},
},
displayName: "Edit",
readonly: false,
isBuiltIn: true,
preprocess: async (args) => {
const { old_string, new_string, replace_all } = args as EditArgs;

Expand Down
17 changes: 10 additions & 7 deletions extensions/cli/src/tools/exit.ts
Original file line number Diff line number Diff line change
@@ -1,14 +1,17 @@
import { Tool } from "./types.js";

export const exitTool: Tool = {
name: "Exit",
displayName: "Exit",
description:
"Exit the current process with status code 1, indicating a failure or error",
parameters: {
type: "object",
properties: {},
type: "function",
function: {
name: "Exit",
description:
"Exit the current process with status code 1, indicating a failure or error",
parameters: {
type: "object",
properties: {},
},
},
displayName: "Exit",
readonly: false,
isBuiltIn: true,
run: async (): Promise<string> => {
Expand Down
7 changes: 4 additions & 3 deletions extensions/cli/src/tools/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,14 +136,15 @@ describe("fetchTool", () => {
});

it("should have correct tool metadata", () => {
expect(fetchTool.name).toBe("Fetch");
expect(fetchTool.type).toBe("function");
expect(fetchTool.function.name).toBe("Fetch");
expect(fetchTool.displayName).toBe("Fetch");
expect(fetchTool.description).toBe(
expect(fetchTool.function.description).toBe(
"Fetches content from a URL, converts to markdown, and handles long content with truncation",
);
expect(fetchTool.readonly).toBe(true);
expect(fetchTool.isBuiltIn).toBe(true);
expect(fetchTool.parameters).toEqual({
expect(fetchTool.function.parameters).toEqual({
type: "object",
required: ["url"],
properties: {
Expand Down
25 changes: 14 additions & 11 deletions extensions/cli/src/tools/fetch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,20 +4,23 @@ import { fetchUrlContentImpl } from "core/tools/implementations/fetchUrlContent.
import { Tool } from "./types.js";

export const fetchTool: Tool = {
name: "Fetch",
displayName: "Fetch",
description:
"Fetches content from a URL, converts to markdown, and handles long content with truncation",
parameters: {
type: "object",
required: ["url"],
properties: {
url: {
type: "string",
description: "The URL to fetch content from",
type: "function",
function: {
name: "Fetch",
description:
"Fetches content from a URL, converts to markdown, and handles long content with truncation",
parameters: {
type: "object",
required: ["url"],
properties: {
url: {
type: "string",
description: "The URL to fetch content from",
},
},
},
},
displayName: "Fetch",
readonly: true,
isBuiltIn: true,
preprocess: async (args) => {
Expand Down
31 changes: 17 additions & 14 deletions extensions/cli/src/tools/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export function getAllBuiltinTools(): Tool[] {
// Apply capability-based filtering for edit tools
// If model is capable, exclude editTool in favor of multiEditTool
if (shouldExcludeEditTool()) {
builtinTools = builtinTools.filter((tool) => tool.name !== editTool.name);
builtinTools = builtinTools.filter((tool) => tool.function.name !== editTool.function.name);
logger.debug(
"Excluded Edit tool for capable model - MultiEdit will be used instead",
);
Expand All @@ -121,7 +121,7 @@ export function getAllBuiltinTools(): Tool[] {

export function getToolDisplayName(toolName: string): string {
const allTools = getAllBuiltinTools();
const tool = allTools.find((t) => t.name === toolName);
const tool = allTools.find((t) => t.function.name === toolName);
return tool?.displayName || toolName;
}

Expand Down Expand Up @@ -158,17 +158,20 @@ export async function getAvailableTools() {
const tools = mcpState.tools ?? [];
const mcpTools: Tool[] =
tools.map((t) => ({
name: t.name,
displayName: t.name.replace("mcp__", "").replace("ide__", ""),
description: t.description ?? "",
parameters: {
type: "object",
properties: (t.inputSchema.properties ?? {}) as Record<
string,
ParameterSchema
>,
required: t.inputSchema.required,
type: "function",
function: {
name: t.name,
description: t.description ?? "",
parameters: {
type: "object",
properties: (t.inputSchema.properties ?? {}) as Record<
string,
ParameterSchema
>,
required: t.inputSchema.required,
},
},
displayName: t.name.replace("mcp__", "").replace("ide__", ""),
readonly: undefined, // MCP tools don't have readonly property
isBuiltIn: false,
run: async (args: any) => {
Expand Down Expand Up @@ -246,8 +249,8 @@ export async function executeToolCall(

// Only checks top-level required
export function validateToolCallArgsPresent(toolCall: ToolCall, tool: Tool) {
const requiredParams = tool.parameters.required ?? [];
for (const [paramName] of Object.entries(tool.parameters)) {
const requiredParams = tool.function.parameters.required ?? [];
for (const [paramName] of Object.entries(tool.function.parameters)) {
Copy link
Contributor

@cubic-dev-ai cubic-dev-ai bot Oct 21, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Iterating over the schema object means you only ever see keys like "type" and "properties", so required argument names are never checked. Please iterate over tool.function.parameters.properties instead.

Prompt for AI agents
Address the following comment on extensions/cli/src/tools/index.tsx at line 253:

<comment>Iterating over the schema object means you only ever see keys like &quot;type&quot; and &quot;properties&quot;, so required argument names are never checked. Please iterate over `tool.function.parameters.properties` instead.</comment>

<file context>
@@ -246,8 +249,8 @@ export async function executeToolCall(
-  const requiredParams = tool.parameters.required ?? [];
-  for (const [paramName] of Object.entries(tool.parameters)) {
+  const requiredParams = tool.function.parameters.required ?? [];
+  for (const [paramName] of Object.entries(tool.function.parameters)) {
     if (
       requiredParams.includes(paramName) &amp;&amp;
</file context>
Fix with Cubic

if (
requiredParams.includes(paramName) &&
(toolCall.arguments[paramName] === undefined ||
Expand Down
23 changes: 13 additions & 10 deletions extensions/cli/src/tools/listFiles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,19 +6,22 @@ import { Tool } from "./types.js";

// List files in a directory
export const listFilesTool: Tool = {
name: "List",
displayName: "List",
description: "List files in a directory",
parameters: {
type: "object",
required: ["dirpath"],
properties: {
dirpath: {
type: "string",
description: "The path to the directory to list",
type: "function",
function: {
name: "List",
description: "List files in a directory",
parameters: {
type: "object",
required: ["dirpath"],
properties: {
dirpath: {
type: "string",
description: "The path to the directory to list",
},
},
},
},
displayName: "List",
readonly: true,
isBuiltIn: true,
preprocess: async (args) => {
Expand Down
Loading
Loading