mirror of
https://github.com/vercel-labs/fx.git
synced 2026-10-28 10:15:17 +08:00
Reject wrong-type grep_files and glob_files arguments
Both tools replaced a wrong-type value with its default and ran anyway. A string "true" for case_insensitive searched case-sensitively, a numeric path searched the whole workspace, and an unknown mode fell back to the default, while the model believed it got what it asked for. They now answer with a message that names the field, as the other tools do. A missing field or null still means the field was not given.
This commit is contained in:
2 files changed
+56
-25
No files matched your search
@@ -47,16 +47,19 @@ pub fn decode(ctx: tool_dispatch.DispatchContext, args_json: []const u8) tool_di
|
||||
return .{ .failure = try ctx.allocator.dupe(u8, "glob_files field \"pattern\" must be a string") };
|
||||
}
|
||||
|
||||
const path_value = parsed.value.object.get("path");
|
||||
const path_string: ?[]const u8 = if (path_value) |value| switch (value) {
|
||||
// A missing field and JSON null both read as not given.
|
||||
const path_string: ?[]const u8 = if (parsed.value.object.get("path")) |value| switch (value) {
|
||||
.null => null,
|
||||
.string => |path| path,
|
||||
else => null,
|
||||
else => return .{ .failure = try ctx.allocator.dupe(u8, "glob_files field \"path\" must be a string") },
|
||||
} else null;
|
||||
|
||||
const mode_value = parsed.value.object.get("mode");
|
||||
const mode: OutputMode = if (mode_value) |value| switch (value) {
|
||||
.string => |raw| if (std.mem.eql(u8, raw, "count")) .count else .matches,
|
||||
else => .matches,
|
||||
const mode: OutputMode = if (parsed.value.object.get("mode")) |value| switch (value) {
|
||||
.null => .matches,
|
||||
.string => |raw| std.meta.stringToEnum(OutputMode, raw) orelse {
|
||||
return .{ .failure = try ctx.allocator.dupe(u8, "glob_files field \"mode\" must be one of matches, count") };
|
||||
},
|
||||
else => return .{ .failure = try ctx.allocator.dupe(u8, "glob_files field \"mode\" must be a string") },
|
||||
} else .matches;
|
||||
|
||||
const owned_pattern = try ctx.allocator.dupe(u8, pattern_value.string);
|
||||
@@ -474,13 +477,16 @@ test "glob_files decodes invalid argument shapes as failures" {
|
||||
try expectDecodeFailure("[]", "glob_files arguments must be an object");
|
||||
try expectDecodeFailure("{\"path\":\".\"}", "glob_files requires string field \"pattern\"");
|
||||
try expectDecodeFailure("{\"pattern\":1}", "glob_files field \"pattern\" must be a string");
|
||||
try expectDecodeFailure("{\"pattern\":\"*.zig\",\"path\":1}", "glob_files field \"path\" must be a string");
|
||||
try expectDecodeFailure("{\"pattern\":\"*.zig\",\"mode\":\"files\"}", "glob_files field \"mode\" must be one of matches, count");
|
||||
try expectDecodeFailure("{\"pattern\":\"*.zig\",\"mode\":true}", "glob_files field \"mode\" must be a string");
|
||||
}
|
||||
|
||||
test "glob_files decodes omitted path and valid input" {
|
||||
try expectDecodeInput("{\"pattern\":\"*.zig\"}", "*.zig", ".");
|
||||
try expectDecodeInput("{\"pattern\":\"*.zig\",\"path\":\"\"}", "*.zig", ".");
|
||||
try expectDecodeInput("{\"pattern\":\"*.zig\",\"path\":\"src\"}", "*.zig", "src");
|
||||
try expectDecodeInput("{\"pattern\":\"*.zig\",\"path\":1}", "*.zig", ".");
|
||||
try expectDecodeInput("{\"pattern\":\"*.zig\",\"path\":null,\"mode\":null}", "*.zig", ".");
|
||||
}
|
||||
|
||||
test "glob_files validate preserves active raw pattern and path values" {
|
||||
|
||||
@@ -68,13 +68,28 @@ pub fn decode(ctx: tool_dispatch.DispatchContext, args_json: []const u8) tool_di
|
||||
return .{ .failure = try ctx.allocator.dupe(u8, "grep_files field \"pattern\" must be a string") };
|
||||
}
|
||||
|
||||
const path_string = optionalStringField(parsed.value.object, "path");
|
||||
const include_string = optionalStringField(parsed.value.object, "include");
|
||||
const path_string = switch (try parseOptionalString(ctx.allocator, parsed.value.object, "path")) {
|
||||
.missing => null,
|
||||
.value => |value| value,
|
||||
.failure => |body| return .{ .failure = body },
|
||||
};
|
||||
const include_string = switch (try parseOptionalString(ctx.allocator, parsed.value.object, "include")) {
|
||||
.missing => null,
|
||||
.value => |value| value,
|
||||
.failure => |body| return .{ .failure = body },
|
||||
};
|
||||
const case_insensitive = if (parsed.value.object.get("case_insensitive")) |value| switch (value) {
|
||||
.null => false,
|
||||
.bool => |enabled| enabled,
|
||||
else => false,
|
||||
else => return .{ .failure = try ctx.allocator.dupe(u8, "grep_files field \"case_insensitive\" must be a boolean") },
|
||||
} else false;
|
||||
const mode = parseOutputMode(parsed.value.object);
|
||||
const mode: OutputMode = switch (try parseOptionalString(ctx.allocator, parsed.value.object, "mode")) {
|
||||
.missing => .matches,
|
||||
.value => |raw| std.meta.stringToEnum(OutputMode, raw) orelse {
|
||||
return .{ .failure = try ctx.allocator.dupe(u8, "grep_files field \"mode\" must be one of matches, files_with_matches, count") };
|
||||
},
|
||||
.failure => |body| return .{ .failure = body },
|
||||
};
|
||||
|
||||
const head_limit = switch (try parseOptionalInteger(ctx.allocator, parsed.value.object, "head_limit", 1, "positive")) {
|
||||
.missing => output_cap,
|
||||
@@ -125,19 +140,24 @@ fn inputDeinit(ptr: *anyopaque, alloc: Allocator) void {
|
||||
alloc.destroy(input);
|
||||
}
|
||||
|
||||
fn optionalStringField(object: std.json.ObjectMap, key: []const u8) ?[]const u8 {
|
||||
const value = object.get(key) orelse return null;
|
||||
return switch (value) {
|
||||
.string => |text| text,
|
||||
else => null,
|
||||
};
|
||||
}
|
||||
const OptionalStringParse = union(enum) {
|
||||
missing,
|
||||
value: []const u8,
|
||||
failure: []u8,
|
||||
};
|
||||
|
||||
fn parseOutputMode(object: std.json.ObjectMap) OutputMode {
|
||||
const raw = optionalStringField(object, "mode") orelse return .matches;
|
||||
if (std.mem.eql(u8, raw, "count")) return .count;
|
||||
if (std.mem.eql(u8, raw, "files_with_matches")) return .files_with_matches;
|
||||
return .matches;
|
||||
/// A missing field and JSON null both read as not given.
|
||||
fn parseOptionalString(
|
||||
alloc: Allocator,
|
||||
object: std.json.ObjectMap,
|
||||
key: []const u8,
|
||||
) tool_dispatch.DispatchError!OptionalStringParse {
|
||||
const value = object.get(key) orelse return .missing;
|
||||
return switch (value) {
|
||||
.null => .missing,
|
||||
.string => |text| .{ .value = text },
|
||||
else => .{ .failure = try alloc.print("grep_files field \"{s}\" must be a string", .{key}) },
|
||||
};
|
||||
}
|
||||
|
||||
const OptionalIntegerParse = union(enum) {
|
||||
@@ -857,6 +877,11 @@ test "grep_files decodes invalid argument shapes as failures" {
|
||||
try expectDecodeFailure("[]", "grep_files arguments must be an object");
|
||||
try expectDecodeFailure("{\"path\":\".\"}", "grep_files requires string field \"pattern\"");
|
||||
try expectDecodeFailure("{\"pattern\":1}", "grep_files field \"pattern\" must be a string");
|
||||
try expectDecodeFailure("{\"pattern\":\"needle\",\"path\":1}", "grep_files field \"path\" must be a string");
|
||||
try expectDecodeFailure("{\"pattern\":\"needle\",\"include\":[\"*.zig\"]}", "grep_files field \"include\" must be a string");
|
||||
try expectDecodeFailure("{\"pattern\":\"needle\",\"case_insensitive\":\"true\"}", "grep_files field \"case_insensitive\" must be a boolean");
|
||||
try expectDecodeFailure("{\"pattern\":\"needle\",\"mode\":\"files\"}", "grep_files field \"mode\" must be one of matches, files_with_matches, count");
|
||||
try expectDecodeFailure("{\"pattern\":\"needle\",\"mode\":2}", "grep_files field \"mode\" must be a string");
|
||||
}
|
||||
|
||||
test "grep_files validate preserves active raw pattern and path values" {
|
||||
@@ -887,7 +912,7 @@ test "grep_files decodes defaults and all fields" {
|
||||
5,
|
||||
2,
|
||||
);
|
||||
try expectDecodeInput("{\"pattern\":\"needle\",\"path\":1,\"include\":1,\"case_insensitive\":\"yes\"}", "needle", ".", null, false, output_cap, 0);
|
||||
try expectDecodeInput("{\"pattern\":\"needle\",\"path\":null,\"include\":null,\"case_insensitive\":null,\"mode\":null}", "needle", ".", null, false, output_cap, 0);
|
||||
}
|
||||
|
||||
test "grep_files decodes context_lines above cap as bounded" {
|
||||
|
||||
Reference in new issue
Block a user