tools: add argument diagnostics

Validate tool arguments like `waitUntil` before parsing to provide
clearer error messages with expected enum values.
This commit is contained in:
Adrià Arrufat
2026-05-22 19:30:20 +02:00
parent 2669e3223f
commit 0219ee66b7
3 changed files with 47 additions and 4 deletions

View File

@@ -663,9 +663,9 @@ pub fn printAssistant(_: *Terminal, text: []const u8) void {
const max_result_display_len = 2000;
/// Tool-outcome line shared by REPL slash commands and LLM tool calls.
/// REPL: green ● on success, red ● on error (`name` is already on the
/// preceding `[tool: …]` line). Non-REPL gates on `medium+` and prefixes
/// `[result: name]`; same green/red coloring.
/// REPL: green ● on success, red ● on error. Non-REPL prefixes `[result:
/// name]`; success gates on `medium+`, errors bypass the gate so a
/// failing script still surfaces *why* at the default verbosity.
pub fn printToolOutcome(self: *Terminal, name: []const u8, text: []const u8, is_error: bool) void {
if (self.repl_arena) |*a| {
defer _ = a.reset(.retain_capacity);
@@ -674,7 +674,7 @@ pub fn printToolOutcome(self: *Terminal, name: []const u8, text: []const u8, is_
_ = std.posix.write(std.posix.STDERR_FILENO, bytes) catch {};
return;
}
if (!atLeast(self.verbosity, .medium)) return;
if (!is_error and !atLeast(self.verbosity, .medium)) return;
const truncated = text[0..@min(text.len, max_result_display_len)];
const ellipsis: []const u8 = if (text.len > max_result_display_len) "..." else "";
const color: []const u8 = if (is_error) ansi.red else ansi.green;

View File

@@ -507,6 +507,8 @@ pub fn call(
arguments: ?std.json.Value,
) ToolError!ToolResult {
const tool = std.meta.stringToEnum(Tool, tool_name) orelse return ToolError.InvalidParams;
if (diagnoseArgs(arena, arguments)) |msg|
return .{ .text = msg, .is_error = true };
const substituted = try substituteStringArgs(arena, tool, arguments);
return switch (tool) {
@@ -1213,6 +1215,31 @@ fn resolveBySelector(session: *lp.Session, selector: []const u8) ToolError!NodeA
pub const ParseArgsError = error{ OutOfMemory, InvalidParams };
/// Surface field/value context for known typed args — `std.json`'s parse
/// errors only carry the tag (`InvalidEnumTag`, …), not which field failed.
fn diagnoseArgs(arena: std.mem.Allocator, arguments: ?std.json.Value) ?[]const u8 {
const args = arguments orelse return null;
if (args != .object) return null;
if (args.object.get("waitUntil")) |v| switch (v) {
.string => |s| if (std.meta.stringToEnum(lp.Config.WaitUntil, s) == null)
return formatEnumError(arena, "waitUntil", s, lp.Config.WaitUntil),
else => return std.fmt.allocPrint(arena, "waitUntil must be a string", .{}) catch null,
};
return null;
}
fn formatEnumError(arena: std.mem.Allocator, field: []const u8, got: []const u8, comptime E: type) ?[]const u8 {
var aw: std.Io.Writer.Allocating = .init(arena);
aw.writer.print("invalid {s} '{s}'. Expected one of: ", .{ field, got }) catch return null;
inline for (std.meta.fields(E), 0..) |f, i| {
if (i > 0) aw.writer.writeAll(", ") catch return null;
aw.writer.writeAll(f.name) catch return null;
}
return aw.written();
}
pub fn parseValue(comptime T: type, arena: std.mem.Allocator, value: std.json.Value) ParseArgsError!T {
return std.json.parseFromValueLeaky(T, arena, value, .{ .ignore_unknown_fields = true }) catch |err| switch (err) {
error.OutOfMemory => error.OutOfMemory,

View File

@@ -898,6 +898,22 @@ test "MCP - waitForSelector: timeout" {
}, out.written());
}
test "MCP - goto with bad waitUntil surfaces rich error" {
defer testing.reset();
var out: std.io.Writer.Allocating = .init(testing.arena_allocator);
const server = try testLoadPage("about:blank", &out.writer);
defer server.deinit();
const msg =
\\{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"goto","arguments":{"url":"about:blank","waitUntil":"x"}}}
;
try router.handleMessage(server, testing.arena_allocator, msg);
const written = out.written();
try testing.expect(std.mem.indexOf(u8, written, "invalid waitUntil 'x'") != null);
try testing.expect(std.mem.indexOf(u8, written, "load") != null);
try testing.expect(std.mem.indexOf(u8, written, "isError\":true") != null);
}
fn testLoadPage(url: [:0]const u8, writer: *std.Io.Writer) !*Server {
var server = try Server.init(testing.allocator, testing.test_app, writer);
errdefer server.deinit();