Skip to content

Commit c6fc60b

Browse files
authored
Merge pull request #8973 from mcowger/mcowger/fixJSONcontents
fix: handle JSON contents of create_new_file
2 parents e1ba630 + e04c21f commit c6fc60b

File tree

2 files changed

+70
-3
lines changed

2 files changed

+70
-3
lines changed

core/tools/parseArgs.ts

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,15 +18,38 @@ export function getStringArg(
1818
argName: string,
1919
allowEmpty = false,
2020
): string {
21-
if (!args || !(argName in args) || typeof args[argName] !== "string") {
21+
if (!args || !(argName in args)) {
2222
throw new Error(
2323
`\`${argName}\` argument is required${allowEmpty ? "" : " and must not be empty or whitespace-only"}. (type string)`,
2424
);
2525
}
26-
if (!allowEmpty && !args[argName].trim()) {
26+
27+
let value = args[argName];
28+
29+
// Handle case where JSON was parsed into an object by the tool call parser.
30+
// If the arguments to the tool call are valid JSON (e.g. the model attempts to create a .json file)
31+
// the earlier call to JSON.parse() will have deeply parsed the returned arguments.
32+
// If that has happened, convert back to string.
33+
if (typeof value === "object" && value !== null) {
34+
try {
35+
value = JSON.stringify(value);
36+
return value;
37+
} catch (e) {
38+
//Swallow this, because it might be fine later.
39+
}
40+
}
41+
42+
if (typeof value !== "string") {
43+
throw new Error(
44+
`\`${argName}\` argument is required${allowEmpty ? "" : " and must not be empty or whitespace-only"}. (type string)`,
45+
);
46+
}
47+
48+
if (!allowEmpty && !value.trim()) {
2749
throw new Error(`Argument ${argName} must not be empty or whitespace-only`);
2850
}
29-
return args[argName];
51+
52+
return value;
3053
}
3154

3255
export function getOptionalStringArg(

core/tools/parseArgs.vitest.ts

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,50 @@ describe("getStringArg", () => {
144144
"`name` argument is required and must not be empty or whitespace-only. (type string)",
145145
);
146146
});
147+
148+
it("should convert parsed JSON object to string for contents parameter", () => {
149+
// This simulates the case where JSON.parse() has converted a JSON string into an object
150+
const args = { contents: { key: "value", number: 123 } };
151+
const result = getStringArg(args, "contents");
152+
expect(result).toBe('{"key":"value","number":123}');
153+
});
154+
155+
it("should convert nested JSON object to string for contents parameter", () => {
156+
const args = {
157+
contents: {
158+
user: {
159+
name: "John",
160+
details: {
161+
age: 30,
162+
preferences: ["coding", "reading"],
163+
},
164+
},
165+
},
166+
};
167+
const result = getStringArg(args, "contents");
168+
const expected =
169+
'{"user":{"name":"John","details":{"age":30,"preferences":["coding","reading"]}}}';
170+
expect(result).toBe(expected);
171+
});
172+
173+
it("should convert JSON array to string for contents parameter", () => {
174+
const args = { contents: ["item1", "item2", { key: "value" }] };
175+
const result = getStringArg(args, "contents");
176+
expect(result).toBe('["item1","item2",{"key":"value"}]');
177+
});
178+
179+
it("should handle contents parameter that is already a string", () => {
180+
const args = { contents: "already a string" };
181+
const result = getStringArg(args, "contents");
182+
expect(result).toBe("already a string");
183+
});
184+
185+
it("should handle contents parameter that is null", () => {
186+
const args = { contents: null };
187+
expect(() => getStringArg(args, "contents")).toThrowError(
188+
"`contents` argument is required and must not be empty or whitespace-only. (type string)",
189+
);
190+
});
147191
});
148192

149193
describe("getOptionalStringArg", () => {

0 commit comments

Comments
 (0)