diff --git a/core/tools/implementations/createNewFile.ts b/core/tools/implementations/createNewFile.ts index b6c626d7380..f08c56564d1 100644 --- a/core/tools/implementations/createNewFile.ts +++ b/core/tools/implementations/createNewFile.ts @@ -3,12 +3,15 @@ import { inferResolvedUriFromRelativePath } from "../../util/ideUtils"; import { ToolImpl } from "."; import { throwIfFileIsSecurityConcern } from "../../indexing/ignore"; import { getCleanUriPath, getUriPathBasename } from "../../util/uri"; -import { getStringArg } from "../parseArgs"; +import { getStringArg, stringifyFileContents } from "../parseArgs"; import { ContinueError, ContinueErrorReason } from "../../util/errors"; export const createNewFileImpl: ToolImpl = async (args, extras) => { const filepath = getStringArg(args, "filepath"); - const contents = getStringArg(args, "contents", true); + const contents = + typeof args?.contents === "object" && args.contents !== null + ? stringifyFileContents(args.contents) + : getStringArg(args, "contents", true); const resolvedFileUri = await inferResolvedUriFromRelativePath( filepath, diff --git a/core/tools/implementations/createNewFile.vitest.ts b/core/tools/implementations/createNewFile.vitest.ts new file mode 100644 index 00000000000..a148d1c5c21 --- /dev/null +++ b/core/tools/implementations/createNewFile.vitest.ts @@ -0,0 +1,46 @@ +import { describe, expect, it, vi } from "vitest"; + +vi.mock("../../util/ideUtils", () => ({ + inferResolvedUriFromRelativePath: async (path: string) => + `file:///workspace/${path}`, +})); + +import { createNewFileImpl } from "./createNewFile"; + +function fakeExtras() { + const written: Record = {}; + const ide = { + fileExists: async () => false, + writeFile: async (uri: string, contents: string) => { + written[uri] = contents; + }, + openFile: async () => {}, + saveFile: async () => {}, + }; + return { written, extras: { ide } as any }; +} + +describe("createNewFileImpl", () => { + it("writes a JSON value sent as contents as indented JSON", async () => { + const { written, extras } = fakeExtras(); + await createNewFileImpl( + { + filepath: "app/package.json", + contents: { name: "app", private: true }, + }, + extras, + ); + expect(written["file:///workspace/app/package.json"]).toBe( + '{\n "name": "app",\n "private": true\n}', + ); + }); + + it("writes string contents unchanged", async () => { + const { written, extras } = fakeExtras(); + await createNewFileImpl( + { filepath: "src/a.ts", contents: "export const a = 1;\n" }, + extras, + ); + expect(written["file:///workspace/src/a.ts"]).toBe("export const a = 1;\n"); + }); +}); diff --git a/core/tools/parseArgs.ts b/core/tools/parseArgs.ts index 64a4f67d09a..673ea1dd099 100644 --- a/core/tools/parseArgs.ts +++ b/core/tools/parseArgs.ts @@ -62,6 +62,26 @@ export function coerceArgsToSchema( return coerced; } +/** + * The contents of a file a tool is about to write, as text. + * Some models send the contents of a JSON file as a JSON value rather than a string, + * so the parsed arguments hold an object or array. Write it as indented JSON, the way + * the file would normally look, rather than on one line. + */ +export function stringifyFileContents(value: unknown): string { + if (typeof value === "string") { + return value; + } + if (value === null || value === undefined) { + return ""; + } + try { + return JSON.stringify(value, null, 2) ?? String(value); + } catch { + return String(value); + } +} + export function getStringArg( args: any, argName: string, diff --git a/core/tools/parseArgs.vitest.ts b/core/tools/parseArgs.vitest.ts index e21254b686d..441d0d49aa2 100644 --- a/core/tools/parseArgs.vitest.ts +++ b/core/tools/parseArgs.vitest.ts @@ -7,6 +7,7 @@ import { getOptionalStringArg, getStringArg, safeParseToolCallArgs, + stringifyFileContents, } from "./parseArgs"; describe("safeParseToolCallArgs", () => { @@ -191,6 +192,20 @@ describe("getStringArg", () => { }); }); +describe("stringifyFileContents", () => { + it("should write a JSON value as indented JSON", () => { + expect(stringifyFileContents({ name: "app", deps: ["a"] })).toBe( + '{\n "name": "app",\n "deps": [\n "a"\n ]\n}', + ); + }); + + it("should keep a string unchanged and turn a missing value into empty text", () => { + expect(stringifyFileContents("const x = 1;\n")).toBe("const x = 1;\n"); + expect(stringifyFileContents(undefined)).toBe(""); + expect(stringifyFileContents(null)).toBe(""); + }); +}); + describe("getOptionalStringArg", () => { it("should return undefined when argument is not provided", () => { const args = { otherArg: "test" }; diff --git a/gui/src/pages/gui/ToolCallDiv/CreateFile.test.tsx b/gui/src/pages/gui/ToolCallDiv/CreateFile.test.tsx new file mode 100644 index 00000000000..5b4dd71604b --- /dev/null +++ b/gui/src/pages/gui/ToolCallDiv/CreateFile.test.tsx @@ -0,0 +1,56 @@ +import { render, screen } from "@testing-library/react"; +import { ToolCallState } from "core"; +import { BuiltInToolNames } from "core/tools/builtIn"; +import { describe, expect, it, vi } from "vitest"; +import FunctionSpecificToolCallDiv from "./FunctionSpecificToolCallDiv"; + +// StyledMarkdownPreview needs Redux; show the Markdown source it is given instead +vi.mock("../../../components/StyledMarkdownPreview", () => ({ + default: ({ source }: { source: string }) => ( +
{source}
+ ), +})); + +function createFileCall(contents: unknown): ToolCallState { + const args = { filepath: "app/package.json", contents }; + return { + toolCallId: "call-1", + toolCall: { + id: "call-1", + type: "function", + function: { + name: BuiltInToolNames.CreateNewFile, + arguments: JSON.stringify(args), + }, + }, + status: "generated", + // The arguments are parsed deeply, so the contents of a .json file arrive as an object + parsedArgs: args, + }; +} + +describe("CreateFile", () => { + it("shows the contents of a JSON file instead of [object Object]", () => { + render( + , + ); + const source = screen.getByTestId("preview").textContent ?? ""; + expect(source).not.toContain("[object Object]"); + expect(source).toContain('{\n "name": "app",\n "version": "1.0.0"\n}'); + }); + + it("shows string contents unchanged", () => { + render( + , + ); + expect(screen.getByTestId("preview").textContent).toContain( + "export const x = 1;", + ); + }); +}); diff --git a/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx b/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx index ed00060a4ad..83f1a5ea9b8 100644 --- a/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx +++ b/gui/src/pages/gui/ToolCallDiv/FunctionSpecificToolCallDiv.tsx @@ -1,6 +1,7 @@ import { ToolCallState } from "core"; import { BuiltInToolNames } from "core/tools/builtIn"; import { EditOperation } from "core/tools/definitions/multiEdit"; +import { stringifyFileContents } from "core/tools/parseArgs"; import { CreateFile } from "./CreateFile"; import { EditFile } from "./EditFile"; import { FindAndReplaceDisplay } from "./FindAndReplace"; @@ -22,7 +23,7 @@ function FunctionSpecificToolCallDiv({ return ( );