From a5fd70f5dd6b1bc3f458f108aa0a76eb38ffceee Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 10 Apr 2026 14:49:27 +0200 Subject: [PATCH] refactor: improve tool execution and terminal formatting - Standardize `fill` tool to use `value` instead of `text`. - Add formatted output methods (`printInfoFmt`, `printErrorFmt`) to `Terminal`. - Improve error handling in `ToolExecutor` and `browser/tools` by using error unions. - Deduplicate `minify` utility and remove redundant code comments. - Refactor tool dispatch to use `Action` enum tags instead of hardcoded strings. --- src/agent/Agent.zig | 153 +++++++++++----------------------- src/agent/Command.zig | 3 - src/agent/CommandExecutor.zig | 33 ++++---- src/agent/Recorder.zig | 10 +-- src/agent/Terminal.zig | 8 ++ src/agent/ToolExecutor.zig | 20 ++--- src/browser/Page.zig | 4 +- src/browser/tools.zig | 96 +++++++++------------ src/mcp/protocol.zig | 31 +------ src/mcp/tools.zig | 7 +- 10 files changed, 126 insertions(+), 239 deletions(-) diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 05b953a7..5e1da4d1 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -165,15 +165,11 @@ pub fn run(self: *Self) bool { fn runRepl(self: *Self) void { self.terminal.printInfo("Lightpanda Agent (type 'quit' to exit)"); log.debug(.app, "tools loaded", .{ .count = self.tools.len }); - const info = if (self.ai_client) |ai_client| - std.fmt.allocPrint(self.allocator, "Provider: {s}, Model: {s}", .{ - @tagName(std.meta.activeTag(ai_client)), - self.model, - }) catch null - else - null; - self.terminal.printInfo(info orelse "Ready."); - if (info) |i| self.allocator.free(i); + if (self.ai_client) |ai_client| { + self.terminal.printInfoFmt("Provider: {s}, Model: {s}", .{ @tagName(std.meta.activeTag(ai_client)), self.model }); + } else { + self.terminal.printInfo("Ready."); + } while (true) { const line = self.terminal.readLine("> ") orelse break; @@ -196,7 +192,6 @@ fn runRepl(self: *Self) void { }; }, .natural_language => { - // "quit" as a convenience alias if (std.mem.eql(u8, line, "quit")) break; self.processUserMessage(line, line) catch |err| { @@ -214,12 +209,7 @@ fn runRepl(self: *Self) void { } fn printAllocError(self: *Self, comptime fmt: []const u8, args: anytype) void { - const msg = std.fmt.allocPrint(self.allocator, fmt, args) catch { - self.terminal.printError(fmt); - return; - }; - defer self.allocator.free(msg); - self.terminal.printError(msg); + self.terminal.printErrorFmt(fmt, args); } fn runScript(self: *Self, path: []const u8) bool { @@ -235,9 +225,7 @@ fn runScript(self: *Self, path: []const u8) bool { }; defer self.allocator.free(content); - const script_info = std.fmt.allocPrint(self.allocator, "Running script: {s}", .{path}) catch null; - self.terminal.printInfo(script_info orelse "Running script..."); - if (script_info) |i| self.allocator.free(i); + self.terminal.printInfoFmt("Running script: {s}", .{path}); var script_arena = std.heap.ArenaAllocator.init(self.allocator); defer script_arena.deinit(); @@ -252,7 +240,6 @@ fn runScript(self: *Self, path: []const u8) bool { return true; }, .comment => { - // Track # INTENT: comments for self-healing if (std.mem.startsWith(u8, entry.raw_line, "# INTENT:")) { last_intent = std.mem.trim(u8, entry.raw_line["# INTENT:".len..], &std.ascii.whitespace); } @@ -263,7 +250,6 @@ fn runScript(self: *Self, path: []const u8) bool { return false; }, .login, .accept_cookies => { - // High-level commands require LLM if (self.ai_client == null) { self.printAllocError("line {d}: {s} requires an API key for LLM resolution", .{ entry.line_num, @@ -282,11 +268,8 @@ fn runScript(self: *Self, path: []const u8) bool { }; }, else => { - const line_info = std.fmt.allocPrint(self.allocator, "[{d}] {s}", .{ entry.line_num, entry.raw_line }) catch null; - self.terminal.printInfo(line_info orelse entry.raw_line); - if (line_info) |li| self.allocator.free(li); + self.terminal.printInfoFmt("[{d}] {s}", .{ entry.line_num, entry.raw_line }); - // Execute with result checking for self-healing var cmd_arena = std.heap.ArenaAllocator.init(self.allocator); defer cmd_arena.deinit(); @@ -295,7 +278,6 @@ fn runScript(self: *Self, path: []const u8) bool { std.debug.print("\n", .{}); if (result.failed) { - // Attempt self-healing via LLM (opt-in with --self-heal) if (self.self_heal and self.ai_client != null) { self.terminal.printInfo("Command failed, attempting self-healing..."); if (self.attemptSelfHeal(last_intent, entry.raw_line)) { @@ -318,8 +300,6 @@ fn runScript(self: *Self, path: []const u8) bool { const self_heal_max_attempts = 3; -/// Attempt to self-heal a failed command by asking the LLM to resolve it. -/// Retries up to `self_heal_max_attempts` times on transient API errors. fn attemptSelfHeal(self: *Self, intent: ?[]const u8, failed_command: []const u8) bool { var heal_arena = std.heap.ArenaAllocator.init(self.allocator); defer heal_arena.deinit(); @@ -351,7 +331,6 @@ fn attemptSelfHeal(self: *Self, intent: ?[]const u8, failed_command: []const u8) fn processUserMessage(self: *Self, user_input: []const u8, record_comment: []const u8) !void { const ma = self.message_arena.allocator(); - // Add system prompt as first message if this is the first user message if (self.messages.items.len == 0) { try self.messages.append(self.allocator, .{ .role = .system, @@ -359,7 +338,6 @@ fn processUserMessage(self: *Self, user_input: []const u8, record_comment: []con }); } - // Add user message try self.messages.append(self.allocator, .{ .role = .user, .content = try ma.dupe(u8, user_input), @@ -384,7 +362,6 @@ fn processUserMessage(self: *Self, user_input: []const u8, record_comment: []con }; defer result.deinit(); - // Record tool calls as Pandascript (only if they produce commands) var recorded_any = false; for (result.tool_calls_made) |tc| { if (!std.mem.startsWith(u8, tc.result, "Error:")) { @@ -417,90 +394,58 @@ fn handleToolCall(ctx: *anyopaque, allocator: std.mem.Allocator, tool_name: []co return tool_result; } -/// Convert a tool call (name + JSON arguments) into a Pandascript command. fn toolCallToCommand(arena: std.mem.Allocator, tool_name: []const u8, arguments: []const u8) ?Command.Command { + const action = std.meta.stringToEnum(lp.tools.Action, tool_name) orelse return null; const parsed = std.json.parseFromSlice(std.json.Value, arena, arguments, .{}) catch return null; const obj = switch (parsed.value) { .object => |o| o, else => return null, }; - return if (std.mem.eql(u8, tool_name, "goto")) blk: { - break :blk switch (obj.get("url") orelse break :blk null) { - .string => |s| .{ .goto = s }, - else => null, - }; - } else if (std.mem.eql(u8, tool_name, "click")) blk: { - if (obj.get("selector")) |sel_val| { - break :blk switch (sel_val) { - .string => |s| .{ .click = s }, + const getString = struct { + fn f(o: std.json.ObjectMap, key: []const u8) ?[]const u8 { + return switch (o.get(key) orelse return null) { + .string => |s| s, else => null, }; } - // Can't meaningfully record a backendNodeId as Pandascript - break :blk null; - } else if (std.mem.eql(u8, tool_name, "fill")) blk: { - const sel = switch (obj.get("selector") orelse break :blk null) { - .string => |s| s, - else => break :blk null, - }; - const val = switch (obj.get("value") orelse break :blk null) { - .string => |s| s, - else => break :blk null, - }; - break :blk .{ .type_cmd = .{ .selector = sel, .value = val } }; - } else if (std.mem.eql(u8, tool_name, "eval")) blk: { - break :blk switch (obj.get("script") orelse break :blk null) { - .string => |s| .{ .eval_js = s }, - else => null, - }; - } else if (std.mem.eql(u8, tool_name, "waitForSelector")) blk: { - break :blk switch (obj.get("selector") orelse break :blk null) { - .string => |s| .{ .wait = s }, - else => null, - }; - } else if (std.mem.eql(u8, tool_name, "scroll")) blk: { - // Only record window scrolls — element scrolls use ephemeral backendNodeId. - if (obj.get("backendNodeId") != null) break :blk null; - const x: i32 = switch (obj.get("x") orelse std.json.Value{ .integer = 0 }) { - .integer => |i| @intCast(i), - else => 0, - }; - const y: i32 = switch (obj.get("y") orelse std.json.Value{ .integer = 0 }) { - .integer => |i| @intCast(i), - else => 0, - }; - break :blk .{ .scroll = .{ .x = x, .y = y } }; - } else if (std.mem.eql(u8, tool_name, "hover")) blk: { - if (obj.get("selector")) |sel_val| { - break :blk switch (sel_val) { - .string => |s| .{ .hover = s }, - else => null, + }.f; + + return switch (action) { + .goto => .{ .goto = getString(obj, "url") orelse return null }, + .click => .{ .click = getString(obj, "selector") orelse return null }, + .hover => .{ .hover = getString(obj, "selector") orelse return null }, + .eval => .{ .eval_js = getString(obj, "script") orelse return null }, + .waitForSelector => .{ .wait = getString(obj, "selector") orelse return null }, + .fill => .{ .type_cmd = .{ + .selector = getString(obj, "selector") orelse return null, + .value = getString(obj, "value") orelse return null, + } }, + .selectOption => .{ .select = .{ + .selector = getString(obj, "selector") orelse return null, + .value = getString(obj, "value") orelse return null, + } }, + .setChecked => .{ .check = .{ + .selector = getString(obj, "selector") orelse return null, + .checked = switch (obj.get("checked") orelse return null) { + .bool => |b| b, + else => return null, + }, + } }, + .scroll => blk: { + if (obj.get("backendNodeId") != null) break :blk null; + const x: i32 = switch (obj.get("x") orelse std.json.Value{ .integer = 0 }) { + .integer => |i| @intCast(i), + else => 0, }; - } - // backendNodeId-only path stays unrecordable - break :blk null; - } else if (std.mem.eql(u8, tool_name, "selectOption")) blk: { - const sel = switch (obj.get("selector") orelse break :blk null) { - .string => |s| s, - else => break :blk null, - }; - const val = switch (obj.get("value") orelse break :blk null) { - .string => |s| s, - else => break :blk null, - }; - break :blk .{ .select = .{ .selector = sel, .value = val } }; - } else if (std.mem.eql(u8, tool_name, "setChecked")) blk: { - const sel = switch (obj.get("selector") orelse break :blk null) { - .string => |s| s, - else => break :blk null, - }; - const checked = switch (obj.get("checked") orelse break :blk null) { - .bool => |b| b, - else => break :blk null, - }; - break :blk .{ .check = .{ .selector = sel, .checked = checked } }; - } else null; + const y: i32 = switch (obj.get("y") orelse std.json.Value{ .integer = 0 }) { + .integer => |i| @intCast(i), + else => 0, + }; + break :blk .{ .scroll = .{ .x = x, .y = y } }; + }, + else => null, + }; } fn getEnvApiKey(provider_type: Config.AiProvider) ?[:0]const u8 { diff --git a/src/agent/Command.zig b/src/agent/Command.zig index 542a329a..1f5950a3 100644 --- a/src/agent/Command.zig +++ b/src/agent/Command.zig @@ -94,10 +94,8 @@ pub fn parse(line: []const u8) Command { const trimmed = std.mem.trim(u8, line, &std.ascii.whitespace); if (trimmed.len == 0) return .{ .natural_language = trimmed }; - // Skip comment lines if (trimmed[0] == '#') return .{ .comment = {} }; - // Find the command word (first whitespace-delimited token) const cmd_end = std.mem.indexOfAny(u8, trimmed, &std.ascii.whitespace) orelse trimmed.len; const cmd_word = trimmed[0..cmd_end]; const rest = std.mem.trim(u8, trimmed[cmd_end..], &std.ascii.whitespace); @@ -246,7 +244,6 @@ pub const ScriptIterator = struct { const trimmed = std.mem.trim(u8, line, &std.ascii.whitespace); if (trimmed.len == 0) continue; - // Check for EVAL """ multi-line block if (isEvalTripleQuote(trimmed)) { const start_line = self.line_num; if (self.collectEvalBlock()) |js| { diff --git a/src/agent/CommandExecutor.zig b/src/agent/CommandExecutor.zig index b831ee33..ddca45dc 100644 --- a/src/agent/CommandExecutor.zig +++ b/src/agent/CommandExecutor.zig @@ -23,27 +23,27 @@ pub const ExecResult = struct { failed: bool, }; -/// Execute a command and return the result with success/failure status. pub fn executeWithResult(self: *Self, a: std.mem.Allocator, cmd: Command.Command) ExecResult { + const Action = browser_tools.Action; return switch (cmd) { .goto => |url| self.execGoto(a, url), - .click => |sel| self.callTool(a, "click", buildJson(a, .{ .selector = substituteEnvVars(a, sel) })), + .click => |sel| self.callTool(a, @tagName(Action.click), buildJson(a, .{ .selector = substituteEnvVars(a, sel) })), .type_cmd => |args| self.execType(a, args), - .wait => |selector| self.callTool(a, "waitForSelector", buildJson(a, .{ .selector = selector })), - .scroll => |args| self.callTool(a, "scroll", buildJson(a, .{ .x = args.x, .y = args.y })), - .hover => |sel| self.callTool(a, "hover", buildJson(a, .{ .selector = substituteEnvVars(a, sel) })), - .select => |args| self.callTool(a, "selectOption", buildJson(a, .{ + .wait => |selector| self.callTool(a, @tagName(Action.waitForSelector), buildJson(a, .{ .selector = selector })), + .scroll => |args| self.callTool(a, @tagName(Action.scroll), buildJson(a, .{ .x = args.x, .y = args.y })), + .hover => |sel| self.callTool(a, @tagName(Action.hover), buildJson(a, .{ .selector = substituteEnvVars(a, sel) })), + .select => |args| self.callTool(a, @tagName(Action.selectOption), buildJson(a, .{ .selector = substituteEnvVars(a, args.selector), .value = substituteEnvVars(a, args.value), })), - .check => |args| self.callTool(a, "setChecked", buildJson(a, .{ + .check => |args| self.callTool(a, @tagName(Action.setChecked), buildJson(a, .{ .selector = substituteEnvVars(a, args.selector), .checked = args.checked, })), - .tree => self.callTool(a, "semanticTree", ""), - .markdown => self.callTool(a, "markdown", ""), + .tree => self.callTool(a, @tagName(Action.semanticTree), ""), + .markdown => self.callTool(a, @tagName(Action.markdown), ""), .extract => |args| self.execExtract(a, args), - .eval_js => |script| self.callTool(a, "eval", buildJson(a, .{ .script = script })), + .eval_js => |script| self.callTool(a, @tagName(Action.eval), buildJson(a, .{ .script = script })), .exit, .natural_language, .comment, .login, .accept_cookies => unreachable, }; } @@ -67,13 +67,13 @@ fn callTool(self: *Self, arena: std.mem.Allocator, tool_name: []const u8, argume fn execGoto(self: *Self, arena: std.mem.Allocator, raw_url: []const u8) ExecResult { const url = substituteEnvVars(arena, raw_url); - return self.callTool(arena, "goto", buildJson(arena, .{ .url = url })); + return self.callTool(arena, @tagName(browser_tools.Action.goto), buildJson(arena, .{ .url = url })); } fn execType(self: *Self, arena: std.mem.Allocator, args: Command.TypeArgs) ExecResult { const selector = substituteEnvVars(arena, args.selector); const value = substituteEnvVars(arena, args.value); - return self.callTool(arena, "fill", buildJson(arena, .{ .selector = selector, .value = value })); + return self.callTool(arena, @tagName(browser_tools.Action.fill), buildJson(arena, .{ .selector = selector, .value = value })); } fn execExtract(self: *Self, arena: std.mem.Allocator, args: Command.ExtractArgs) ExecResult { @@ -83,8 +83,8 @@ fn execExtract(self: *Self, arena: std.mem.Allocator, args: Command.ExtractArgs) \\JSON.stringify(Array.from(document.querySelectorAll("{s}")).map(el => el.textContent.trim())) , .{selector}) catch return .{ .output = "failed to build extract script", .failed = true }; - const result = self.tool_executor.call(arena, "eval", buildJson(arena, .{ .script = script })) catch - return .{ .output = "extract failed", .failed = true }; + const result = self.tool_executor.call(arena, @tagName(browser_tools.Action.eval), buildJson(arena, .{ .script = script })) catch |err| + return .{ .output = std.fmt.allocPrint(arena, "extract failed: {s}", .{@errorName(err)}) catch "extract failed", .failed = true }; if (args.file) |raw_file| { const file = sanitizePath(raw_file) orelse { @@ -107,9 +107,7 @@ fn execExtract(self: *Self, arena: std.mem.Allocator, args: Command.ExtractArgs) const substituteEnvVars = browser_tools.substituteEnvVars; -/// Escape a string for safe interpolation inside a JS double-quoted string literal. fn escapeJs(arena: std.mem.Allocator, input: []const u8) []const u8 { - // Quick scan: if nothing to escape, return as-is const needs_escape = for (input) |ch| { if (ch == '"' or ch == '\\' or ch == '\n' or ch == '\r' or ch == '\t') break true; } else false; @@ -129,12 +127,9 @@ fn escapeJs(arena: std.mem.Allocator, input: []const u8) []const u8 { return out.toOwnedSlice(arena) catch input; } -/// Validate that a file path is safe: relative, no traversal above cwd. fn sanitizePath(path: []const u8) ?[]const u8 { - // Reject absolute paths if (path.len > 0 and path[0] == '/') return null; - // Reject paths containing ".." components var iter = std.mem.splitScalar(u8, path, '/'); while (iter.next()) |component| { if (std.mem.eql(u8, component, "..")) return null; diff --git a/src/agent/Recorder.zig b/src/agent/Recorder.zig index eb29aac2..c2badb78 100644 --- a/src/agent/Recorder.zig +++ b/src/agent/Recorder.zig @@ -22,8 +22,6 @@ pub fn deinit(self: *Self) void { if (self.file) |f| f.close(); } -/// Record a successfully executed command to the .panda file. -/// Skips read-only commands based on `Command.isRecorded()`. pub fn record(self: *Self, cmd: Command.Command) void { const f = self.file orelse return; if (!cmd.isRecorded()) return; @@ -36,7 +34,6 @@ pub fn record(self: *Self, cmd: Command.Command) void { self.needs_separator = true; } -/// Record a comment line (e.g. user's natural language input). pub fn recordComment(self: *Self, comment: []const u8) void { const f = self.file orelse return; var buf: [4096]u8 = undefined; @@ -50,8 +47,6 @@ pub fn recordComment(self: *Self, comment: []const u8) void { writer.flush() catch return; } -// --- Tests --- - test "record writes state-mutating commands" { var tmp = std.testing.tmpDir(.{}); defer tmp.cleanup(); @@ -63,9 +58,9 @@ test "record writes state-mutating commands" { recorder.record(Command.parse("GOTO https://example.com")); recorder.record(Command.parse("CLICK \"Login\"")); - recorder.record(Command.parse("TREE")); // should be skipped + recorder.record(Command.parse("TREE")); recorder.record(Command.parse("WAIT \".dashboard\"")); - recorder.record(Command.parse("MARKDOWN")); // should be skipped + recorder.record(Command.parse("MARKDOWN")); recorder.record(Command.parse("SCROLL 0 200")); recorder.record(Command.parse("HOVER '#menu'")); recorder.record(Command.parse("SELECT '#country' 'France'")); @@ -90,7 +85,6 @@ test "record writes state-mutating commands" { try std.testing.expect(std.mem.indexOf(u8, content, "CHECK '#newsletter' false\n") != null); try std.testing.expect(std.mem.indexOf(u8, content, "EXTRACT '.title'\n") != null); try std.testing.expect(std.mem.indexOf(u8, content, "\n# LOGIN\n") != null); - // Verify read-only commands are NOT present try std.testing.expect(std.mem.indexOf(u8, content, "TREE") == null); try std.testing.expect(std.mem.indexOf(u8, content, "MARKDOWN") == null); } diff --git a/src/agent/Terminal.zig b/src/agent/Terminal.zig index 78c25271..cd5a225c 100644 --- a/src/agent/Terminal.zig +++ b/src/agent/Terminal.zig @@ -108,6 +108,14 @@ pub fn printError(_: *Self, msg: []const u8) void { std.debug.print("{s}{s}Error: {s}{s}\n", .{ ansi_bold, ansi_red, msg, ansi_reset }); } +pub fn printErrorFmt(_: *Self, comptime fmt: []const u8, args: anytype) void { + std.debug.print("{s}{s}Error: " ++ fmt ++ "{s}\n", .{ ansi_bold, ansi_red } ++ args ++ .{ansi_reset}); +} + pub fn printInfo(_: *Self, msg: []const u8) void { std.debug.print("{s}{s}{s}\n", .{ ansi_dim, msg, ansi_reset }); } + +pub fn printInfoFmt(_: *Self, comptime fmt: []const u8, args: anytype) void { + std.debug.print("{s}" ++ fmt ++ "{s}\n", .{ansi_dim} ++ args ++ .{ansi_reset}); +} diff --git a/src/agent/ToolExecutor.zig b/src/agent/ToolExecutor.zig index a689cfef..c8c21249 100644 --- a/src/agent/ToolExecutor.zig +++ b/src/agent/ToolExecutor.zig @@ -55,7 +55,8 @@ pub fn deinit(self: *Self) void { self.allocator.destroy(self); } -/// Returns the list of tools in zenai provider.Tool format. +pub const CallError = browser_tools.ToolError || error{ InvalidJsonArguments, OutOfMemory }; + pub fn getTools(self: *Self) ![]const zenai.provider.Tool { const arena = self.tool_schema_arena.allocator(); const tools = try arena.alloc(zenai.provider.Tool, browser_tools.tool_defs.len); @@ -75,15 +76,12 @@ pub fn getTools(self: *Self) ![]const zenai.provider.Tool { return tools; } -/// Execute a tool by name with JSON arguments, returning the result as a string. -pub fn call(self: *Self, arena: std.mem.Allocator, tool_name: []const u8, arguments_json: []const u8) ![]const u8 { - const arguments = if (arguments_json.len > 0) - (std.json.parseFromSlice(std.json.Value, arena, arguments_json, .{}) catch - return "Error: invalid JSON arguments").value - else - null; +pub fn call(self: *Self, arena: std.mem.Allocator, tool_name: []const u8, arguments_json: []const u8) CallError![]const u8 { + const arguments = if (arguments_json.len > 0) blk: { + const parsed = std.json.parseFromSlice(std.json.Value, arena, arguments_json, .{}) catch + return error.InvalidJsonArguments; + break :blk parsed.value; + } else null; - return browser_tools.call(self.session, &self.node_registry, arena, tool_name, arguments) catch |err| { - return std.fmt.allocPrint(arena, "Error: {s}", .{@errorName(err)}) catch "Error: tool execution failed"; - }; + return browser_tools.call(self.session, &self.node_registry, arena, tool_name, arguments); } diff --git a/src/browser/Page.zig b/src/browser/Page.zig index 458e13d8..d876710f 100644 --- a/src/browser/Page.zig +++ b/src/browser/Page.zig @@ -241,7 +241,7 @@ _parent_notified: bool = false, _type: enum { root, frame }, // only used for logs right now _req_id: u32 = 0, -console_messages: std.ArrayListUnmanaged(ConsoleMessage) = .{}, +_console_messages: std.ArrayListUnmanaged(ConsoleMessage) = .{}, _navigated_options: ?NavigatedOpts = null, pub fn init(self: *Page, frame_id: u32, session: *Session, parent: ?*Page) !void { @@ -2547,7 +2547,7 @@ pub fn appendConsoleMessage(self: *Page, level: ConsoleMessage.Level, values: [] value.format(&aw.writer) catch return; } const text = aw.written(); - self.console_messages.append(self.arena, .{ .level = level, .text = text }) catch return; + self._console_messages.append(self.arena, .{ .level = level, .text = text }) catch return; } pub fn dupeString(self: *Page, value: []const u8) ![]const u8 { diff --git a/src/browser/tools.zig b/src/browser/tools.zig index d0ec3da6..51515f46 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -11,7 +11,7 @@ pub const ToolDef = struct { input_schema: []const u8, }; -fn minify(comptime json: []const u8) []const u8 { +pub fn minify(comptime json: []const u8) []const u8 { @setEvalBranchQuota(100000); return comptime blk: { var res: []const u8 = ""; @@ -307,9 +307,9 @@ pub const ToolError = error{ NodeNotFound, NavigationFailed, InternalError, + OutOfMemory, }; -/// Result from eval that may represent a JS error (not a tool failure). pub const EvalResult = struct { text: []const u8, is_error: bool = false, @@ -329,9 +329,7 @@ pub const UrlParams = struct { const NodeAndPage = struct { node: *DOMNode, page: *lp.Page }; -// --- Tool dispatch --- - -const Action = enum { +pub const Action = enum { goto, markdown, links, @@ -356,8 +354,6 @@ const Action = enum { getCookies, }; -/// Execute a tool by name. Returns the result text. -/// For `eval`, use `callEval` to distinguish JS errors from tool errors. pub fn call( session: *lp.Session, registry: *CDPNode.Registry, @@ -368,32 +364,15 @@ pub fn call( const action = std.meta.stringToEnum(Action, tool_name) orelse return ToolError.InvalidParams; return switch (action) { - .goto => execGoto(session, registry, arena, arguments), - .markdown => execMarkdown(session, registry, arena, arguments), - .links => execLinks(session, registry, arena, arguments), - .nodeDetails => execNodeDetails(session, registry, arena, arguments), - .interactiveElements => execInteractiveElements(session, registry, arena, arguments), - .structuredData => execStructuredData(session, registry, arena, arguments), - .detectForms => execDetectForms(session, registry, arena, arguments), .eval => execEval(session, registry, arena, arguments).text, - .semanticTree => execSemanticTree(session, registry, arena, arguments), - .click => execClick(session, registry, arena, arguments), - .fill => execFill(session, registry, arena, arguments), - .scroll => execScroll(session, registry, arena, arguments), - .waitForSelector => execWaitForSelector(session, registry, arena, arguments), - .hover => execHover(session, registry, arena, arguments), - .press => execPress(session, registry, arena, arguments), - .selectOption => execSelectOption(session, registry, arena, arguments), - .setChecked => execSetChecked(session, registry, arena, arguments), - .findElement => execFindElement(session, registry, arena, arguments), .getEnv => execGetEnv(arena, arguments), .consoleLogs => execConsoleLogs(session, arena), .getUrl => execGetUrl(session), .getCookies => execGetCookies(session, arena), + inline else => |tag| @field(@This(), "exec" ++ [1]u8{@tagName(tag)[0] - 32} ++ @tagName(tag)[1..])(session, registry, arena, arguments), }; } -/// Like `call`, but for eval returns the full EvalResult with is_error flag. pub fn callEval( session: *lp.Session, registry: *CDPNode.Registry, @@ -403,21 +382,18 @@ pub fn callEval( return execEval(session, registry, arena, arguments); } -/// Check if a tool name is recognized. pub fn isKnownTool(tool_name: []const u8) bool { return std.meta.stringToEnum(Action, tool_name) != null; } -// --- Tool implementations --- - fn execGoto(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { - const args = parseArgsOrErr(GotoParams, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(GotoParams, arena, arguments) orelse return ToolError.InvalidParams; try performGoto(session, registry, args.url, args.timeout, args.waitUntil); return "Navigated successfully."; } fn execMarkdown(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { - const args = parseArgsOrDefault(UrlParams, arena, arguments); + const args = try parseArgsOrDefault(UrlParams, arena, arguments); const page = try ensurePage(session, registry, args.url, args.timeout, args.waitUntil); var aw: std.Io.Writer.Allocating = .init(arena); @@ -427,7 +403,7 @@ fn execMarkdown(session: *lp.Session, registry: *CDPNode.Registry, arena: std.me } fn execLinks(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { - const args = parseArgsOrDefault(UrlParams, arena, arguments); + const args = try parseArgsOrDefault(UrlParams, arena, arguments); const page = try ensurePage(session, registry, args.url, args.timeout, args.waitUntil); const links_list = lp.links.collectLinks(arena, page.document.asNode(), page) catch @@ -449,7 +425,7 @@ fn execSemanticTree(session: *lp.Session, registry: *CDPNode.Registry, arena: st timeout: ?u32 = null, waitUntil: ?lp.Config.WaitUntil = null, }; - const args = parseArgsOrDefault(TreeParams, arena, arguments); + const args = try parseArgsOrDefault(TreeParams, arena, arguments); const page = try ensurePage(session, registry, args.url, args.timeout, args.waitUntil); var root_node = page.document.asNode(); @@ -475,7 +451,7 @@ fn execSemanticTree(session: *lp.Session, registry: *CDPNode.Registry, arena: st fn execNodeDetails(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { const Params = struct { backendNodeId: CDPNode.Id }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; const page = session.currentPage() orelse return ToolError.PageNotLoaded; @@ -490,7 +466,7 @@ fn execNodeDetails(session: *lp.Session, registry: *CDPNode.Registry, arena: std } fn execInteractiveElements(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { - const args = parseArgsOrDefault(UrlParams, arena, arguments); + const args = try parseArgsOrDefault(UrlParams, arena, arguments); const page = try ensurePage(session, registry, args.url, args.timeout, args.waitUntil); const elements = lp.interactive.collectInteractiveElements(page.document.asNode(), arena, page) catch @@ -504,7 +480,7 @@ fn execInteractiveElements(session: *lp.Session, registry: *CDPNode.Registry, ar } fn execStructuredData(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { - const args = parseArgsOrDefault(UrlParams, arena, arguments); + const args = try parseArgsOrDefault(UrlParams, arena, arguments); const page = try ensurePage(session, registry, args.url, args.timeout, args.waitUntil); const data = lp.structured_data.collectStructuredData(page.document.asNode(), arena, page) catch @@ -515,7 +491,7 @@ fn execStructuredData(session: *lp.Session, registry: *CDPNode.Registry, arena: } fn execDetectForms(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { - const args = parseArgsOrDefault(UrlParams, arena, arguments); + const args = try parseArgsOrDefault(UrlParams, arena, arguments); const page = try ensurePage(session, registry, args.url, args.timeout, args.waitUntil); const forms_data = lp.forms.collectForms(arena, page.document.asNode(), page) catch @@ -535,7 +511,8 @@ fn execEval(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Al timeout: ?u32 = null, waitUntil: ?lp.Config.WaitUntil = null, }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return .{ .text = "Error: missing 'script' argument", .is_error = true }; + const args = (parseArgsOrErr(Params, arena, arguments) catch return .{ .text = "Error: out of memory", .is_error = true }) orelse + return .{ .text = "Error: missing 'script' argument", .is_error = true }; const page = ensurePage(session, registry, args.url, args.timeout, args.waitUntil) catch return .{ .text = "Error: page not loaded", .is_error = true }; var ls: lp.js.Local.Scope = undefined; @@ -561,7 +538,7 @@ fn execClick(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.A backendNodeId: ?CDPNode.Id = null, selector: ?[]const u8 = null, }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; const resolved = if (args.selector) |sel| try resolveBySelector(session, sel) else if (args.backendNodeId) |nid| @@ -599,11 +576,11 @@ fn execFill(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.Al const Params = struct { backendNodeId: ?CDPNode.Id = null, selector: ?[]const u8 = null, - text: []const u8 = "", value: []const u8 = "", }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; - const raw_text = if (args.text.len > 0) args.text else if (args.value.len > 0) args.value else return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + if (args.value.len == 0) return ToolError.InvalidParams; + const raw_text = args.value; const text = substituteEnvVars(arena, raw_text); const resolved = if (args.selector) |sel| try resolveBySelector(session, sel) @@ -639,7 +616,7 @@ fn execScroll(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem. x: ?i32 = null, y: ?i32 = null, }; - const args = parseArgsOrDefault(Params, arena, arguments); + const args = try parseArgsOrDefault(Params, arena, arguments); const page = session.currentPage() orelse return ToolError.PageNotLoaded; var target_node: ?*DOMNode = null; @@ -667,7 +644,7 @@ fn execWaitForSelector(session: *lp.Session, registry: *CDPNode.Registry, arena: selector: [:0]const u8, timeout: ?u32 = null, }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; _ = session.currentPage() orelse return ToolError.PageNotLoaded; @@ -687,7 +664,7 @@ fn execHover(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.A backendNodeId: ?CDPNode.Id = null, selector: ?[]const u8 = null, }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; const resolved = if (args.selector) |sel| try resolveBySelector(session, sel) else if (args.backendNodeId) |nid| @@ -718,7 +695,7 @@ fn execPress(session: *lp.Session, registry: *CDPNode.Registry, arena: std.mem.A key: []const u8, backendNodeId: ?CDPNode.Id = null, }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; const page = session.currentPage() orelse return ToolError.PageNotLoaded; @@ -754,7 +731,7 @@ fn execSelectOption(session: *lp.Session, registry: *CDPNode.Registry, arena: st selector: ?[]const u8 = null, value: []const u8, }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; const resolved = if (args.selector) |sel| try resolveBySelector(session, sel) else if (args.backendNodeId) |nid| @@ -787,7 +764,7 @@ fn execSetChecked(session: *lp.Session, registry: *CDPNode.Registry, arena: std. selector: ?[]const u8 = null, checked: bool, }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; const resolved = if (args.selector) |sel| try resolveBySelector(session, sel) else if (args.backendNodeId) |nid| @@ -820,7 +797,7 @@ fn execFindElement(session: *lp.Session, registry: *CDPNode.Registry, arena: std role: ?[]const u8 = null, name: ?[]const u8 = null, }; - const args = parseArgsOrDefault(Params, arena, arguments); + const args = try parseArgsOrDefault(Params, arena, arguments); if (args.role == null and args.name == null) return ToolError.InvalidParams; @@ -853,7 +830,7 @@ fn execFindElement(session: *lp.Session, registry: *CDPNode.Registry, arena: std fn execGetEnv(arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { const Params = struct { name: []const u8 }; - const args = parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; + const args = try parseArgsOrErr(Params, arena, arguments) orelse return ToolError.InvalidParams; const name_z = arena.dupeZ(u8, args.name) catch return ToolError.InternalError; const value = std.posix.getenv(name_z) orelse return std.fmt.allocPrint(arena, "Environment variable '{s}' is not set", .{args.name}) catch ToolError.InternalError; @@ -865,7 +842,7 @@ fn execConsoleLogs( arena: std.mem.Allocator, ) ToolError![]const u8 { const page = session.currentPage() orelse return ToolError.PageNotLoaded; - const messages = page.console_messages.items; + const messages = page._console_messages.items; if (messages.len == 0) return "No console messages."; var aw: std.Io.Writer.Allocating = .init(arena); @@ -873,7 +850,7 @@ fn execConsoleLogs( for (messages) |msg| { writer.print("[{s}] {s}\n", .{ @tagName(msg.level), msg.text }) catch return ToolError.InternalError; } - page.console_messages.clearRetainingCapacity(); + page._console_messages.clearRetainingCapacity(); return aw.written(); } @@ -898,8 +875,6 @@ fn execGetCookies(session: *lp.Session, arena: std.mem.Allocator) ToolError![]co return aw.written(); } -// --- Shared helpers --- - fn ensurePage(session: *lp.Session, registry: *CDPNode.Registry, url: ?[:0]const u8, timeout: ?u32, waitUntil: ?lp.Config.WaitUntil) ToolError!*lp.Page { if (url) |u| { try performGoto(session, registry, u, timeout, waitUntil); @@ -938,17 +913,22 @@ fn resolveBySelector(session: *lp.Session, selector: []const u8) ToolError!NodeA return .{ .node = node, .page = page }; } -fn parseArgsOrDefault(comptime T: type, arena: std.mem.Allocator, arguments: ?std.json.Value) T { +fn parseArgsOrDefault(comptime T: type, arena: std.mem.Allocator, arguments: ?std.json.Value) error{OutOfMemory}!T { const args_raw = arguments orelse return .{}; - return std.json.parseFromValueLeaky(T, arena, args_raw, .{ .ignore_unknown_fields = true }) catch .{}; + return std.json.parseFromValueLeaky(T, arena, args_raw, .{ .ignore_unknown_fields = true }) catch |err| switch (err) { + error.OutOfMemory => error.OutOfMemory, + else => .{}, + }; } -fn parseArgsOrErr(comptime T: type, arena: std.mem.Allocator, arguments: ?std.json.Value) ?T { +fn parseArgsOrErr(comptime T: type, arena: std.mem.Allocator, arguments: ?std.json.Value) error{OutOfMemory}!?T { const args_raw = arguments orelse return null; - return std.json.parseFromValueLeaky(T, arena, args_raw, .{ .ignore_unknown_fields = true }) catch null; + return std.json.parseFromValueLeaky(T, arena, args_raw, .{ .ignore_unknown_fields = true }) catch |err| switch (err) { + error.OutOfMemory => error.OutOfMemory, + else => null, + }; } -/// Substitute $VAR_NAME references with values from the environment. pub fn substituteEnvVars(arena: std.mem.Allocator, input: []const u8) []const u8 { if (std.mem.indexOfScalar(u8, input, '$') == null) return input; diff --git a/src/mcp/protocol.zig b/src/mcp/protocol.zig index 02927278..df435f17 100644 --- a/src/mcp/protocol.zig +++ b/src/mcp/protocol.zig @@ -123,36 +123,7 @@ pub const Tool = struct { } }; -pub fn minify(comptime json: []const u8) []const u8 { - @setEvalBranchQuota(100000); - return comptime blk: { - var res: []const u8 = ""; - var in_string = false; - var escaped = false; - for (json) |c| { - if (in_string) { - res = res ++ [1]u8{c}; - if (escaped) { - escaped = false; - } else if (c == '\\') { - escaped = true; - } else if (c == '"') { - in_string = false; - } - } else { - switch (c) { - ' ', '\n', '\r', '\t' => continue, - '"' => { - in_string = true; - res = res ++ [1]u8{c}; - }, - else => res = res ++ [1]u8{c}, - } - } - } - break :blk res; - }; -} +pub const minify = @import("../browser/tools.zig").minify; pub const Resource = struct { uri: []const u8, diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index 5aed67a2..c7fff3f3 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -57,8 +57,7 @@ pub fn handleCall(server: *Server, arena: std.mem.Allocator, req: protocol.Reque const code: protocol.ErrorCode = switch (err) { error.PageNotLoaded => .PageNotLoaded, error.NodeNotFound, error.InvalidParams => .InvalidParams, - error.NavigationFailed => .InternalError, - error.InternalError => .InternalError, + error.NavigationFailed, error.InternalError, error.OutOfMemory => .InternalError, }; return server.sendError(id, code, @errorName(err)); }; @@ -128,7 +127,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked const inp_id = (try server.node_registry.register(inp)).id; var inp_id_buf: [12]u8 = undefined; const inp_id_str = std.fmt.bufPrint(&inp_id_buf, "{d}", .{inp_id}) catch unreachable; - const fill_msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/call\",\"params\":{\"name\":\"fill\",\"arguments\":{\"backendNodeId\":", inp_id_str, ",\"text\":\"hello\"}}}" }); + const fill_msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/call\",\"params\":{\"name\":\"fill\",\"arguments\":{\"backendNodeId\":", inp_id_str, ",\"value\":\"hello\"}}}" }); try router.handleMessage(server, aa, fill_msg); try testing.expect(std.mem.indexOf(u8, out.written(), "Filled element") != null); try testing.expect(std.mem.indexOf(u8, out.written(), "with \\\"hello\\\"") != null); @@ -141,7 +140,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked const sel_id = (try server.node_registry.register(sel)).id; var sel_id_buf: [12]u8 = undefined; const sel_id_str = std.fmt.bufPrint(&sel_id_buf, "{d}", .{sel_id}) catch unreachable; - const fill_sel_msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":3,\"method\":\"tools/call\",\"params\":{\"name\":\"fill\",\"arguments\":{\"backendNodeId\":", sel_id_str, ",\"text\":\"opt2\"}}}" }); + const fill_sel_msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":3,\"method\":\"tools/call\",\"params\":{\"name\":\"fill\",\"arguments\":{\"backendNodeId\":", sel_id_str, ",\"value\":\"opt2\"}}}" }); try router.handleMessage(server, aa, fill_sel_msg); try testing.expect(std.mem.indexOf(u8, out.written(), "Filled element") != null); try testing.expect(std.mem.indexOf(u8, out.written(), "with \\\"opt2\\\"") != null);