fix: validate individual edit entries in parseEditFilesArgs (#24301)

site/src/pages/AgentsPage/components/ChatElements/runtimeTypeUtils.ts

@@ -1,3 +1,5 @@
+import type { Schema } from "yup";
+
 export const asRecord = (value: unknown): Record<string, unknown> | null => {
 	if (!value || typeof value !== "object" || Array.isArray(value)) {
 		return null;
@@ -8,6 +10,15 @@ export const asRecord = (value: unknown): Record<string, unknown> | null => {
 export const asString = (value: unknown): string =>
 	typeof value === "string" ? value : "";
 
+/**
+ * Type-narrowing wrapper around a Yup schema. Returns `true`
+ * (and narrows `value` to `T`) when `value` satisfies the
+ * schema. Strict mode is always enabled to prevent silent
+ * type coercion.
+ */
+export const isValid = <T>(schema: Schema<T>, value: unknown): value is T =>
+	schema.isValidSync(value, { strict: true });
+
 export const asNumber = (
 	value: unknown,
 	options?: { readonly parseString?: boolean },

site/src/pages/AgentsPage/components/ChatElements/tools/utils.test.ts

@@ -495,6 +495,7 @@ describe("parseEditFilesArgs", () => {
 				{ path: "a.ts", edits: [{ search: "x", replace: "y" }] },
 				{ path: 42, edits: [] }, // invalid: path not string
 				null, // invalid: null
+				undefined, // invalid: undefined
 				{ path: "b.ts" }, // invalid: no edits array
 				{ path: "c.ts", edits: [{ search: "a", replace: "b" }] },
 			],
@@ -513,6 +514,131 @@ describe("parseEditFilesArgs", () => {
 		expect(result).toHaveLength(1);
 		expect(result[0].path).toBe("test.ts");
 	});
+
+	it("filters out edits with non-string search or replace", () => {
+		const args = {
+			files: [
+				{
+					path: "a.ts",
+					edits: [
+						{ search: "x", replace: "y" },
+						{ search: "a" }, // missing replace
+						{ replace: "b" }, // missing search
+						{ search: 42, replace: "c" }, // non-string search
+						{ search: "d", replace: null }, // non-string replace
+						null, // null edit
+					],
+				},
+			],
+		};
+		const result = parseEditFilesArgs(args);
+		expect(result).toHaveLength(1);
+		expect(result[0].edits).toHaveLength(1);
+		expect(result[0].edits[0]).toEqual({ search: "x", replace: "y" });
+	});
+
+	// Yup.object() is optional by default, so undefined passes
+	// isValidSync in strict mode. Without .required() on the
+	// schemas, undefined entries survive the filter and crash
+	// the subsequent .map() accessing f.path.
+	it("rejects undefined file entries and edits", () => {
+		const args = {
+			files: [
+				undefined,
+				{
+					path: "a.ts",
+					edits: [undefined, { search: "x", replace: "y" }],
+				},
+			],
+		};
+		const result = parseEditFilesArgs(args);
+		expect(result).toHaveLength(1);
+		expect(result[0].path).toBe("a.ts");
+		expect(result[0].edits).toHaveLength(1);
+		expect(result[0].edits[0]).toEqual({ search: "x", replace: "y" });
+	});
+
+	// Regression: a partial edit with a missing replace field caused
+	// Diff.createPatch to crash inside its tokenize method with
+	// "Cannot read properties of undefined (reading 'split')".
+	// This reproduces the exact call path from Tool.tsx:
+	// parseEditFilesArgs(args) -> buildEditDiff(file.path, file.edits).
+	it("does not crash buildEditDiff when edits have missing replace", () => {
+		const args = {
+			files: [
+				{
+					path: "src/app.ts",
+					edits: [
+						{ search: "const x = 1;", replace: "const x = 2;" },
+						{ search: "const y = 3;" }, // streamed edit, replace not yet present
+					],
+				},
+			],
+		};
+		const parsed = parseEditFilesArgs(args);
+		expect(parsed).toHaveLength(1);
+		// The incomplete edit should be filtered out, leaving only
+		// the valid one so buildEditDiff never sees undefined.
+		expect(parsed[0].edits).toHaveLength(1);
+		const diff = buildEditDiff(parsed[0].path, parsed[0].edits);
+		expect(diff).not.toBeNull();
+	});
+
+	// search uses required() (rejects "") while replace uses
+	// defined() (allows ""). This asymmetry is intentional:
+	// empty search is meaningless, empty replace is a deletion.
+	it("rejects edits with empty-string search", () => {
+		const args = {
+			files: [
+				{
+					path: "a.ts",
+					edits: [
+						{ search: "", replace: "new" },
+						{ search: "valid", replace: "also valid" },
+					],
+				},
+			],
+		};
+		const result = parseEditFilesArgs(args);
+		expect(result).toHaveLength(1);
+		expect(result[0].edits).toHaveLength(1);
+		expect(result[0].edits[0].search).toBe("valid");
+	});
+
+	it("preserves edits with empty-string replace (deletion)", () => {
+		const args = {
+			files: [
+				{
+					path: "src/app.ts",
+					edits: [{ search: "const old = 1;", replace: "" }],
+				},
+			],
+		};
+		const parsed = parseEditFilesArgs(args);
+		expect(parsed).toHaveLength(1);
+		expect(parsed[0].edits).toHaveLength(1);
+		expect(parsed[0].edits[0].replace).toBe("");
+	});
+
+	// During streaming the model may emit a file entry before any
+	// edit is complete. Every edit has a missing replace, so all are
+	// filtered out. The file entry survives with an empty edits
+	// array and buildEditDiff returns null.
+	it("returns file entry with empty edits when all edits are invalid", () => {
+		const args = {
+			files: [
+				{
+					path: "src/app.ts",
+					edits: [{ search: "const x = 1;" }, { search: "const y = 2;" }],
+				},
+			],
+		};
+		const parsed = parseEditFilesArgs(args);
+		expect(parsed).toHaveLength(1);
+		expect(parsed[0].path).toBe("src/app.ts");
+		expect(parsed[0].edits).toHaveLength(0);
+		expect(buildEditDiff(parsed[0].path, parsed[0].edits)).toBeNull();
+	});
 });
 
 describe("buildEditDiff", () => {

site/src/pages/AgentsPage/components/ChatElements/tools/utils.ts

@@ -2,7 +2,8 @@ import type { FileDiffMetadata } from "@pierre/diffs";
 import { parsePatchFiles } from "@pierre/diffs";
 import * as Diff from "diff";
 import type React from "react";
-import { asRecord, asString } from "../runtimeTypeUtils";
+import * as Yup from "yup";
+import { asRecord, asString, isValid } from "../runtimeTypeUtils";
 
 export type ToolStatus = "completed" | "error" | "running";
 
@@ -11,6 +12,20 @@ export interface EditFilesFileEntry {
 	edits: Array<{ search: string; replace: string }>;
 }
 
+const searchReplaceSchema = Yup.object({
+	search: Yup.string().required(),
+	replace: Yup.string().defined(),
+}).required();
+
+type SearchReplace = Yup.InferType<typeof searchReplaceSchema>;
+
+const fileEntrySchema = Yup.object({
+	path: Yup.string().required(),
+	edits: Yup.array().defined(),
+}).required();
+
+type FileEntry = Yup.InferType<typeof fileEntrySchema>;
+
 export const toProviderLabel = (
 	providerDisplayName: string,
 	providerID: string,
@@ -515,13 +530,14 @@ export const parseEditFilesArgs = (args: unknown): EditFilesFileEntry[] => {
 	if (!parsed) return [];
 	const files = parsed.files;
 	if (!Array.isArray(files)) return [];
-	return files.filter(
-		(f): f is EditFilesFileEntry =>
-			f !== null &&
-			typeof f === "object" &&
-			typeof (f as Record<string, unknown>).path === "string" &&
-			Array.isArray((f as Record<string, unknown>).edits),
-	);
+	return files
+		.filter((f): f is FileEntry => isValid(fileEntrySchema, f))
+		.map((f) => ({
+			path: f.path,
+			edits: f.edits.filter((e): e is SearchReplace =>
+				isValid(searchReplaceSchema, e),
+			),
+		}));
 };
 
 /**