diff --git a/docs/AGENTS.md b/docs/AGENTS.md index b0f0726e34..c2cde2697d 100644 --- a/docs/AGENTS.md +++ b/docs/AGENTS.md @@ -87,6 +87,13 @@ description: Agent instructions for AI assistants working on the Mux codebase - **Avoid `setTimeout` for component coordination** - racy and fragile; use callbacks or effects. - **Keyboard event propagation** - React's `e.stopPropagation()` only stops synthetic event bubbling; native `window` listeners still fire. Use `stopKeyboardPropagation(e)` from `@/browser/utils/events` to stop both React and native propagation when blocking global handlers (like stream interrupt on Escape). + +## Tool Schema Conventions + +- **Use `.nullish()` for optional tool input parameters** — never `.optional()` alone. OpenAI's Responses API normalizes tool schemas into strict mode, which forces every field into `required` and expects optional fields to accept `null`. `.nullish()` (= `.optional().nullable()`) satisfies strict-mode providers (OpenAI) while remaining compatible with non-strict providers (Anthropic, Google). See the module doc comment in `src/common/utils/tools/toolDefinitions.ts` for details. +- Implementation handlers should use `!= null` (loose equality) instead of `!== undefined` to treat both `null` and `undefined` as "not provided". +- This applies only to tool **input** schemas (parameters the model provides), not to tool **output/result** schemas (constructed by our backend). + ## Component State & Storage - Prefer **self-contained components** over utility functions + hook proliferation. A component that takes `workspaceId` and computes everything internally is better than one that requires 10 props drilled from parent hooks. diff --git a/docs/config/notifications.mdx b/docs/config/notifications.mdx index daefd3db5b..4fe62b021a 100644 --- a/docs/config/notifications.mdx +++ b/docs/config/notifications.mdx @@ -105,7 +105,7 @@ notify: { message: z .string() .max(200) - .optional() + .nullish() .describe( "Optional notification body with more details (max 200 chars). " + "Keep it brief - users may only see a preview." diff --git a/src/browser/components/tools/AgentSkillReadFileToolCall.tsx b/src/browser/components/tools/AgentSkillReadFileToolCall.tsx index 3f3b0e3840..898e1e99fe 100644 --- a/src/browser/components/tools/AgentSkillReadFileToolCall.tsx +++ b/src/browser/components/tools/AgentSkillReadFileToolCall.tsx @@ -139,13 +139,13 @@ export const AgentSkillReadFileToolCall: React.FCFile: {args.filePath} - {args.offset !== undefined && ( + {args.offset != null && (
Offset: line {args.offset}
)} - {args.limit !== undefined && ( + {args.limit != null && (
Limit: {args.limit} lines diff --git a/src/browser/components/tools/FileReadToolCall.tsx b/src/browser/components/tools/FileReadToolCall.tsx index 571dfe2cd4..31219bbf7c 100644 --- a/src/browser/components/tools/FileReadToolCall.tsx +++ b/src/browser/components/tools/FileReadToolCall.tsx @@ -104,13 +104,13 @@ export const FileReadToolCall: React.FC = ({ Path: {filePath}
- {args.offset !== undefined && ( + {args.offset != null && (
Offset: line {args.offset}
)} - {args.limit !== undefined && ( + {args.limit != null && (
Limit: {args.limit} lines diff --git a/src/browser/components/tools/TaskToolCall.tsx b/src/browser/components/tools/TaskToolCall.tsx index 4362ccee8b..0565d8667f 100644 --- a/src/browser/components/tools/TaskToolCall.tsx +++ b/src/browser/components/tools/TaskToolCall.tsx @@ -467,10 +467,7 @@ export const TaskAwaitToolCall: React.FC = ({ const suppressReportInAwaitTaskIds = taskReportLinking?.suppressReportInAwaitTaskIds; const showConfigInfo = - taskIds !== undefined || - timeoutSecs !== undefined || - args.filter !== undefined || - args.filter_exclude === true; + taskIds != null || timeoutSecs != null || args.filter != null || args.filter_exclude === true; // Summary for header const completedCount = results.filter((r) => r.status === "completed").length; @@ -562,9 +559,9 @@ export const TaskAwaitToolCall: React.FC = ({ {/* Config info */} {showConfigInfo && (
- {taskIds !== undefined && Waiting for: {taskIds.length} task(s)} - {timeoutSecs !== undefined && Timeout: {timeoutSecs}s} - {args.filter !== undefined && Filter: {args.filter}} + {taskIds != null && Waiting for: {taskIds.length} task(s)} + {timeoutSecs != null && Timeout: {timeoutSecs}s} + {args.filter != null && Filter: {args.filter}} {args.filter_exclude === true && Exclude: true}
)} diff --git a/src/cli/toolFormatters.ts b/src/cli/toolFormatters.ts index 0d55e65fd3..1a0b4ee2ae 100644 --- a/src/cli/toolFormatters.ts +++ b/src/cli/toolFormatters.ts @@ -120,10 +120,10 @@ function formatFileReadStart(_toolName: string, args: unknown): string | null { if (!readArgs?.file_path) return null; let suffix = ""; - if (readArgs.offset !== undefined || readArgs.limit !== undefined) { + if (readArgs.offset != null || readArgs.limit != null) { const parts: string[] = []; - if (readArgs.offset !== undefined) parts.push(`L${readArgs.offset}`); - if (readArgs.limit !== undefined) parts.push(`+${readArgs.limit}`); + if (readArgs.offset != null) parts.push(`L${readArgs.offset}`); + if (readArgs.limit != null) parts.push(`+${readArgs.limit}`); suffix = chalk.dim(` (${parts.join(", ")})`); } diff --git a/src/common/types/tools.ts b/src/common/types/tools.ts index 8a4752f65b..3a76e6bf1b 100644 --- a/src/common/types/tools.ts +++ b/src/common/types/tools.ts @@ -29,23 +29,14 @@ import type { WebFetchToolResultSchema, } from "@/common/utils/tools/toolDefinitions"; -// Bash Tool Types -export interface BashToolArgs { - script: string; - timeout_secs: number; // Required - defaults should be applied by producers - run_in_background?: boolean; // Run without blocking (for long-running processes) - display_name: string; // Required - used as process identifier if sent to background -} +// Bash Tool Types — derived from schema (avoid drift) +export type BashToolArgs = z.infer; // BashToolResult derived from Zod schema (single source of truth) export type BashToolResult = z.infer; -// File Read Tool Types -export interface FileReadToolArgs { - file_path: string; - offset?: number; // 1-based starting line number (optional) - limit?: number; // number of lines to return from offset (optional) -} +// File Read Tool Types — derived from schema (avoid drift) +export type FileReadToolArgs = z.infer; // Agent Skill Tool Types // Args derived from schema (avoid drift) @@ -110,35 +101,24 @@ export interface FileEditErrorResult extends ToolOutputUiOnlyFields { note?: string; // Agent-only message (not displayed in UI) } -export interface FileEditInsertToolArgs { - file_path: string; - /** Anchor text to insert before. Content will be placed immediately before this substring. */ - insert_before?: string; - /** Anchor text to insert after. Content will be placed immediately after this substring. */ - insert_after?: string; - content: string; -} +// FileEditInsertToolArgs derived from schema (avoid drift) +export type FileEditInsertToolArgs = z.infer; // FileEditInsertToolResult derived from Zod schema (single source of truth) export type FileEditInsertToolResult = z.infer; -export interface FileEditReplaceStringToolArgs { - file_path: string; - old_string: string; - new_string: string; - replace_count?: number; -} +// FileEditReplaceStringToolArgs derived from schema (avoid drift) +export type FileEditReplaceStringToolArgs = z.infer< + typeof TOOL_DEFINITIONS.file_edit_replace_string.schema +>; // FileEditReplaceStringToolResult derived from Zod schema (single source of truth) export type FileEditReplaceStringToolResult = z.infer; -export interface FileEditReplaceLinesToolArgs { - file_path: string; - start_line: number; - end_line: number; - new_lines: string[]; - expected_lines?: string[]; -} +// FileEditReplaceLinesToolArgs derived from schema (avoid drift) +export type FileEditReplaceLinesToolArgs = z.infer< + typeof TOOL_DEFINITIONS.file_edit_replace_lines.schema +>; export type FileEditReplaceLinesToolResult = | (FileEditDiffSuccessBase & { @@ -288,43 +268,28 @@ export interface LegacyProposePlanToolResult { message: string; } -// Todo Tool Types -export interface TodoItem { - content: string; - status: "pending" | "in_progress" | "completed"; -} - -export interface TodoWriteToolArgs { - todos: TodoItem[]; -} +// Todo Tool Types — derived from schema (avoid drift) +export type TodoWriteToolArgs = z.infer; +export type TodoItem = TodoWriteToolArgs["todos"][number]; export interface TodoWriteToolResult { success: true; count: number; } -// Status Set Tool Types -export interface StatusSetToolArgs { - emoji: string; - message: string; - url?: string; -} +// Status Set Tool Types — derived from schema (avoid drift) +export type StatusSetToolArgs = z.infer; -// Bash Output Tool Types (read incremental output from background processes) -export interface BashOutputToolArgs { - process_id: string; - filter?: string; - filter_exclude?: boolean; - timeout_secs: number; -} +// Bash Output Tool Types — derived from schema (avoid drift) +export type BashOutputToolArgs = z.infer; // BashOutputToolResult derived from Zod schema (single source of truth) export type BashOutputToolResult = z.infer; -// Bash Background Tool Types -export interface BashBackgroundTerminateArgs { - process_id: string; -} +// Bash Background Tool Types — derived from schema (avoid drift) +export type BashBackgroundTerminateArgs = z.infer< + typeof TOOL_DEFINITIONS.bash_background_terminate.schema +>; // BashBackgroundTerminateResult derived from Zod schema (single source of truth) export type BashBackgroundTerminateResult = z.infer; @@ -353,10 +318,8 @@ export type StatusSetToolResult = error: string; }; -// Web Fetch Tool Types -export interface WebFetchToolArgs { - url: string; -} +// Web Fetch Tool Types — derived from schema (avoid drift) +export type WebFetchToolArgs = z.infer; // WebFetchToolResult derived from Zod schema (single source of truth) export type WebFetchToolResult = z.infer; diff --git a/src/common/utils/tools/toolDefinitions.ts b/src/common/utils/tools/toolDefinitions.ts index 16e6a78e70..d83f7790f5 100644 --- a/src/common/utils/tools/toolDefinitions.ts +++ b/src/common/utils/tools/toolDefinitions.ts @@ -3,6 +3,27 @@ * * Single source of truth for all tool definitions. * Zod schemas are defined here and JSON schemas are auto-generated. + * + * ## Schema convention: `.nullish()` for optional tool parameters + * + * All optional fields in **tool input schemas** (i.e. parameters the model + * provides) MUST use `.nullish()` instead of `.optional()`. + * + * Why: OpenAI's Responses API normalizes tool schemas into strict mode, which + * forces every field into `required` and expects optional fields to accept + * `null` (via `"type": ["string", "null"]`). Using `.optional()` alone + * produces a schema without a null type, so the model is forced to hallucinate + * values for fields it would normally skip. `.nullish()` (= `.optional().nullable()`) + * emits both `null` in the type union AND keeps the field out of `required`, + * which satisfies strict-mode providers (OpenAI) while remaining compatible + * with non-strict providers (Anthropic, Google). + * + * Implementation handlers that consume these values should use `!= null` + * (loose equality) instead of `!== undefined` to correctly treat both + * `null` and `undefined` as "not provided". + * + * This does NOT apply to tool **output/result** schemas — those are constructed + * by our own backend code and always use `undefined` for absent fields. */ import { z } from "zod"; @@ -87,7 +108,7 @@ export const AskUserQuestionToolArgsSchema = z .object({ questions: z.array(AskUserQuestionQuestionSchema).min(1).max(4), // Optional prefilled answers (Claude Code supports this, though Mux typically won't use it) - answers: z.record(z.string(), z.string()).optional(), + answers: z.record(z.string(), z.string()).nullish(), }) .strict() .superRefine((args, ctx) => { @@ -137,8 +158,8 @@ const TaskAgentIdSchema = z.preprocess( const TaskToolAgentArgsSchema = z .object({ // Prefer agentId. subagent_type is a deprecated alias for backwards compatibility. - agentId: TaskAgentIdSchema.optional(), - subagent_type: SubagentTypeSchema.optional(), + agentId: TaskAgentIdSchema.nullish(), + subagent_type: SubagentTypeSchema.nullish(), prompt: z.string().min(1), title: z.string().min(1), run_in_background: z.boolean().default(false), @@ -206,13 +227,13 @@ export const TaskAwaitToolArgsSchema = z .object({ task_ids: z .array(z.string().min(1)) - .optional() + .nullish() .describe( "List of task IDs to await. When omitted, waits for all active descendant tasks of the current workspace." ), filter: z .string() - .optional() + .nullish() .describe( "Optional regex to filter bash task output lines. By default, only matching lines are returned. " + "When filter_exclude is true, matching lines are excluded instead. " + @@ -220,7 +241,7 @@ export const TaskAwaitToolArgsSchema = z ), filter_exclude: z .boolean() - .optional() + .nullish() .describe( "When true, lines matching 'filter' are excluded instead of kept. " + "Requires 'filter' to be set." @@ -228,7 +249,7 @@ export const TaskAwaitToolArgsSchema = z timeout_secs: z .number() .min(0) - .optional() + .nullish() .default(600) .describe( "Maximum time to wait in seconds for each task. " + @@ -349,14 +370,14 @@ export const TaskApplyGitPatchToolArgsSchema = z task_id: z.string().min(1).describe("Child task ID whose patch artifact should be applied"), dry_run: z .boolean() - .optional() + .nullish() .describe( "When true, attempt to apply the patch in a temporary git worktree and then discard it (does not modify the current workspace)." ), - three_way: z.boolean().optional().default(true).describe("When true, run git am with --3way"), + three_way: z.boolean().nullish().default(true).describe("When true, run git am with --3way"), force: z .boolean() - .optional() + .nullish() .describe( "When true, allow apply even if the patch was previously applied (and skip clean-tree checks)." ), @@ -466,7 +487,7 @@ export const TaskListToolArgsSchema = z .object({ statuses: z .array(TaskListStatusSchema) - .optional() + .nullish() .describe( "Task statuses to include. Defaults to active tasks: queued, running, awaiting_report." ), @@ -501,7 +522,7 @@ export const TaskListToolResultSchema = z export const AgentReportToolArgsSchema = z .object({ reportMarkdown: z.string().min(1), - title: z.string().optional(), + title: z.string().nullish(), }) .strict(); @@ -520,6 +541,30 @@ interface ToolSchema { }; } +/** + * Schema for a single keep-range item in the system1_keep_ranges tool. + * Extracted as a named export so internal code can derive the type via z.infer<> + * instead of maintaining a hand-written interface. + * + * Note: the tool schema applies .passthrough() on top of this to tolerate extra + * keys from models, but the inferred type is the strict shape. + */ +export const System1KeepRangeSchema = z.object({ + start: z.coerce + .number() + .finite() + .min(1) + .describe("1-based start line (inclusive) in the numbered output"), + end: z.coerce + .number() + .finite() + .min(1) + .describe("1-based end line (inclusive) in the numbered output"), + // .nullish() accepts both null and undefined, so the preprocess + // hack that mapped null→undefined is no longer needed. + reason: z.string().nullish().describe("Optional short reason for keeping this range"), +}); + /** * Tool definitions: single source of truth * Key = tool name, Value = { description, schema } @@ -594,13 +639,13 @@ export const TOOL_DEFINITIONS = { .number() .int() .positive() - .optional() + .nullish() .describe("1-based starting line number (optional, defaults to 1)"), limit: z .number() .int() .positive() - .optional() + .nullish() .describe("Number of lines to return from offset (optional, returns all if not specified)"), }), }, @@ -650,13 +695,13 @@ export const TOOL_DEFINITIONS = { .number() .int() .positive() - .optional() + .nullish() .describe("1-based starting line number (optional, defaults to 1)"), limit: z .number() .int() .positive() - .optional() + .nullish() .describe( "Number of lines to return from offset (optional, returns all if not specified)" ), @@ -679,7 +724,7 @@ export const TOOL_DEFINITIONS = { replace_count: z .number() .int() - .optional() + .nullish() .describe( "Number of occurrences to replace (default: 1). Use -1 to replace all occurrences. If 1, old_string must be unique in the file." ), @@ -698,7 +743,7 @@ export const TOOL_DEFINITIONS = { .describe("Replacement lines. Provide an empty array to delete the specified range."), expected_lines: z .array(z.string()) - .optional() + .nullish() .describe( "Optional safety check. When provided, the current lines in the specified range must match exactly." ), @@ -718,20 +763,20 @@ export const TOOL_DEFINITIONS = { insert_before: z .string() .min(1) - .optional() + .nullish() .describe( "Anchor text to insert before. Content will be placed immediately before this substring." ), insert_after: z .string() .min(1) - .optional() + .nullish() .describe( "Anchor text to insert after. Content will be placed immediately after this substring." ), content: z.string().describe("The content to insert"), }) - .refine((data) => !(data.insert_before !== undefined && data.insert_after !== undefined), { + .refine((data) => !(data.insert_before != null && data.insert_after != null), { message: "Provide only one of insert_before or insert_after (not both).", path: ["insert_before"], }), @@ -814,25 +859,7 @@ export const TOOL_DEFINITIONS = { .object({ keep_ranges: z .array( - z - .object({ - start: z.coerce - .number() - .finite() - .min(1) - .describe("1-based start line (inclusive) in the numbered output"), - end: z.coerce - .number() - .finite() - .min(1) - .describe("1-based end line (inclusive) in the numbered output"), - reason: z - .preprocess( - (value) => (value === null ? undefined : value), - z.string().optional() - ) - .describe("Optional short reason for keeping this range"), - }) + System1KeepRangeSchema // Providers/models sometimes include extra keys in tool arguments; be permissive and // ignore them rather than failing the whole compaction call. .passthrough() @@ -909,7 +936,7 @@ export const TOOL_DEFINITIONS = { url: z .string() .url() - .optional() + .nullish() .describe( "Optional URL to external resource with more details (e.g., Pull Request URL). The URL persists and is displayed to the user for easy access." ), @@ -930,7 +957,7 @@ export const TOOL_DEFINITIONS = { process_id: z.string().describe("The ID of the background process to retrieve output from"), filter: z .string() - .optional() + .nullish() .describe( "Optional regex to filter output lines. By default, only matching lines are returned. " + "When filter_exclude is true, matching lines are excluded instead. " + @@ -938,7 +965,7 @@ export const TOOL_DEFINITIONS = { ), filter_exclude: z .boolean() - .optional() + .nullish() .describe( "When true, lines matching 'filter' are excluded instead of kept. " + "Key behavior: excluded lines do NOT cause early return from timeout - " + @@ -1011,7 +1038,7 @@ export const TOOL_DEFINITIONS = { message: z .string() .max(200) - .optional() + .nullish() .describe( "Optional notification body with more details (max 200 chars). " + "Keep it brief - users may only see a preview." diff --git a/src/node/services/system1/bashOutputFiltering.ts b/src/node/services/system1/bashOutputFiltering.ts index f781826bef..1a0d47cf88 100644 --- a/src/node/services/system1/bashOutputFiltering.ts +++ b/src/node/services/system1/bashOutputFiltering.ts @@ -1,10 +1,9 @@ import assert from "@/common/utils/assert"; +import type { z } from "zod"; +import type { System1KeepRangeSchema } from "@/common/utils/tools/toolDefinitions"; -export interface System1KeepRange { - start: number; - end: number; - reason?: string; -} +// Derived from the Zod schema (single source of truth) to avoid drift. +export type System1KeepRange = z.infer; export interface ApplySystem1KeepRangesResult { filteredOutput: string; diff --git a/src/node/services/taskService.ts b/src/node/services/taskService.ts index 3d7a336fd3..eadac8afc1 100644 --- a/src/node/services/taskService.ts +++ b/src/node/services/taskService.ts @@ -2448,7 +2448,9 @@ export class TaskService { if (!isSuccessfulToolResult(part.output)) continue; const parsed = AgentReportToolArgsSchema.safeParse(part.input); if (!parsed.success) continue; - return parsed.data; + // Normalize null → undefined at the schema boundary so downstream + // code that expects `title?: string` doesn't need to handle null. + return { reportMarkdown: parsed.data.reportMarkdown, title: parsed.data.title ?? undefined }; } return null; } diff --git a/src/node/services/tools/agent_skill_read_file.ts b/src/node/services/tools/agent_skill_read_file.ts index afbc2bbbb6..8c81a93cc1 100644 --- a/src/node/services/tools/agent_skill_read_file.ts +++ b/src/node/services/tools/agent_skill_read_file.ts @@ -20,8 +20,8 @@ function readContentWithFileReadLimits(input: { fullContent: string; fileSize: number; modifiedTime: string; - offset?: number; - limit?: number; + offset?: number | null; + limit?: number | null; }): AgentSkillReadFileToolResult { if (input.fileSize > MAX_FILE_SIZE) { const sizeMB = (input.fileSize / (1024 * 1024)).toFixed(2); @@ -34,7 +34,7 @@ function readContentWithFileReadLimits(input: { const lines = input.fullContent === "" ? [] : input.fullContent.split("\n"); - if (input.offset !== undefined && input.offset > lines.length) { + if (input.offset != null && input.offset > lines.length) { return { success: false, error: `Offset ${input.offset} is beyond file length`, @@ -43,7 +43,7 @@ function readContentWithFileReadLimits(input: { const startLineNumber = input.offset ?? 1; const startIdx = startLineNumber - 1; - const endIdx = input.limit !== undefined ? startIdx + input.limit : lines.length; + const endIdx = input.limit != null ? startIdx + input.limit : lines.length; const numberedLines: string[] = []; let totalBytesAccumulated = 0; @@ -122,7 +122,7 @@ export const createAgentSkillReadFileTool: ToolFactory = (config: ToolConfigurat } try { - if (offset !== undefined && offset < 1) { + if (offset != null && offset < 1) { return { success: false, error: `Offset must be positive (got ${offset})`, diff --git a/src/node/services/tools/bash.test.ts b/src/node/services/tools/bash.test.ts index ae67dabf0d..bf1d13f0d6 100644 --- a/src/node/services/tools/bash.test.ts +++ b/src/node/services/tools/bash.test.ts @@ -1587,6 +1587,7 @@ describe("muxEnv environment variables", () => { const args: BashToolArgs = { script: 'echo "PROJECT:$MUX_PROJECT_PATH RUNTIME:$MUX_RUNTIME WORKSPACE:$MUX_WORKSPACE_NAME"', timeout_secs: 5, + run_in_background: false, display_name: "test", }; @@ -1616,6 +1617,7 @@ describe("muxEnv environment variables", () => { const args: BashToolArgs = { script: 'echo "MUX:$MUX_PROJECT_PATH CUSTOM:$CUSTOM_VAR"', timeout_secs: 5, + run_in_background: false, display_name: "test", }; diff --git a/src/node/services/tools/bash_output.ts b/src/node/services/tools/bash_output.ts index 94a5f15a53..12dcd93309 100644 --- a/src/node/services/tools/bash_output.ts +++ b/src/node/services/tools/bash_output.ts @@ -42,8 +42,8 @@ export const createBashOutputTool: ToolFactory = (config: ToolConfiguration) => // Pass workspaceId so getOutput can check for queued messages return await config.backgroundProcessManager.getOutput( process_id, - filter, - filter_exclude, + filter ?? undefined, + filter_exclude ?? undefined, timeout_secs, abortSignal, config.workspaceId diff --git a/src/node/services/tools/code_execution.ts b/src/node/services/tools/code_execution.ts index 7bf632ae15..569f1739ae 100644 --- a/src/node/services/tools/code_execution.ts +++ b/src/node/services/tools/code_execution.ts @@ -91,7 +91,7 @@ ${muxTypes} .number() .int() .positive() - .optional() + .nullish() .describe( "Execution timeout in seconds (default: 300, max: 3600). " + "Increase when spawning subagents that may take 5-15+ minutes." diff --git a/src/node/services/tools/file_edit_insert.ts b/src/node/services/tools/file_edit_insert.ts index f6c301bc22..292a4babcb 100644 --- a/src/node/services/tools/file_edit_insert.ts +++ b/src/node/services/tools/file_edit_insert.ts @@ -28,10 +28,7 @@ interface InsertOperationFailure { note?: string; } -interface InsertContentOptions { - insert_before?: string; - insert_after?: string; -} +type InsertContentOptions = Pick; interface GuardResolutionSuccess { success: true; @@ -147,11 +144,11 @@ function insertContent( ): InsertOperationSuccess | InsertOperationFailure { const { insert_before, insert_after } = options; - if (insert_before !== undefined && insert_after !== undefined) { + if (insert_before != null && insert_after != null) { return guardFailure("Provide only one of insert_before or insert_after (not both)."); } - if (insert_before === undefined && insert_after === undefined) { + if (insert_before == null && insert_after == null) { return guardFailure( "Provide either insert_before or insert_after guard when editing existing files." ); @@ -215,7 +212,7 @@ function resolveGuardAnchor( const fileEol = detectFileEol(originalContent); // insert_after: content goes after this anchor, so insertion point is at end of anchor - if (insert_after !== undefined) { + if (insert_after != null) { const exactResult = findUniqueSubstringIndex(originalContent, insert_after, "insert_after"); if (exactResult.success) { return { success: true, index: exactResult.index + insert_after.length }; @@ -241,7 +238,7 @@ function resolveGuardAnchor( } // insert_before: content goes before this anchor, so insertion point is at start of anchor - if (insert_before !== undefined) { + if (insert_before != null) { const exactResult = findUniqueSubstringIndex(originalContent, insert_before, "insert_before"); if (exactResult.success) { return { success: true, index: exactResult.index }; diff --git a/src/node/services/tools/file_edit_replace_shared.ts b/src/node/services/tools/file_edit_replace_shared.ts index 76876510c0..129196697b 100644 --- a/src/node/services/tools/file_edit_replace_shared.ts +++ b/src/node/services/tools/file_edit_replace_shared.ts @@ -10,6 +10,8 @@ import { NOTE_READ_FILE_FIRST_RETRY, NOTE_READ_FILE_RETRY, NOTE_READ_FILE_AGAIN_RETRY, + type FileEditReplaceStringToolArgs, + type FileEditReplaceLinesToolArgs, } from "@/common/types/tools"; import { convertNewlines, detectFileEol, normalizeNewlinesToLF } from "./eol"; @@ -34,20 +36,10 @@ export interface OperationError { export type OperationOutcome = OperationResult | OperationError; -export interface StringReplaceArgs { - file_path: string; - old_string: string; - new_string: string; - replace_count?: number; -} - -export interface LineReplaceArgs { - file_path: string; - start_line: number; - end_line: number; - new_lines: string[]; - expected_lines?: string[]; -} +// Re-export schema-derived types for backward compatibility. +// Local code previously imported StringReplaceArgs / LineReplaceArgs from this module. +export type StringReplaceArgs = FileEditReplaceStringToolArgs; +export type LineReplaceArgs = FileEditReplaceLinesToolArgs; /** * Handle string-based replacement diff --git a/src/node/services/tools/file_read.ts b/src/node/services/tools/file_read.ts index 837830981e..463ddabac5 100644 --- a/src/node/services/tools/file_read.ts +++ b/src/node/services/tools/file_read.ts @@ -81,7 +81,7 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { const startLineNumber = offset ?? 1; // Validate offset - if (offset !== undefined && offset < 1) { + if (offset != null && offset < 1) { return { success: false, error: `Offset must be positive (got ${offset})`, @@ -93,7 +93,7 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { const lines = fullContent === "" ? [] : fullContent.split("\n"); // Validate offset - if (offset !== undefined && offset > lines.length) { + if (offset != null && offset > lines.length) { return { success: false, error: `Offset ${offset} is beyond file length`, @@ -108,7 +108,7 @@ export const createFileReadTool: ToolFactory = (config: ToolConfiguration) => { // Process lines with offset and limit const startIdx = startLineNumber - 1; // Convert to 0-based index - const endIdx = limit !== undefined ? startIdx + limit : lines.length; + const endIdx = limit != null ? startIdx + limit : lines.length; for (let i = startIdx; i < Math.min(endIdx, lines.length); i++) { const line = lines[i]; diff --git a/src/node/services/tools/system1_keep_ranges.ts b/src/node/services/tools/system1_keep_ranges.ts index 42f6bfe80b..c726c5177c 100644 --- a/src/node/services/tools/system1_keep_ranges.ts +++ b/src/node/services/tools/system1_keep_ranges.ts @@ -1,14 +1,14 @@ import { tool } from "ai"; import type { Tool } from "ai"; +import type { z } from "zod"; import type { ToolConfiguration } from "@/common/utils/tools/tools"; import { TOOL_DEFINITIONS } from "@/common/utils/tools/toolDefinitions"; import type { System1KeepRange } from "@/node/services/system1/bashOutputFiltering"; -export interface System1KeepRangesToolArgs { - keep_ranges: System1KeepRange[]; -} +// Derived from the Zod schema (single source of truth) to avoid drift. +export type System1KeepRangesToolArgs = z.infer; export type System1KeepRangesToolResult = | { diff --git a/src/node/services/tools/task_await.ts b/src/node/services/tools/task_await.ts index 927a6f63c5..852c502c39 100644 --- a/src/node/services/tools/task_await.ts +++ b/src/node/services/tools/task_await.ts @@ -29,7 +29,9 @@ export const createTaskAwaitTool: ToolFactory = (config: ToolConfiguration) => { const taskService = requireTaskService(config, "task_await"); const timeoutMs = coerceTimeoutMs(args.timeout_secs); - const timeoutSecsForBash = args.timeout_secs; + // Preserve the documented 600s default when the model sends null + // (Zod .default() only replaces undefined, not null). + const timeoutSecsForBash = args.timeout_secs ?? 600; const requestedIds: string[] | null = args.task_ids && args.task_ids.length > 0 ? args.task_ids : null; @@ -115,8 +117,8 @@ export const createTaskAwaitTool: ToolFactory = (config: ToolConfiguration) => { const outputResult = await config.backgroundProcessManager.getOutput( maybeProcessId, - args.filter, - args.filter_exclude, + args.filter ?? undefined, + args.filter_exclude ?? undefined, timeoutSecsForBash, abortSignal, workspaceId,