From cff32f6c047a8d2936dfdb7ec348d82901c93fa6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Thu, 27 Aug 2026 10:37:56 +0200 Subject: [PATCH] agent: let the model see a REPL /screenshot; fold the tool-result adapters The slash path's result has two consumers: the terminal, which can't show an image, and the conversation, which can. Opt in when a model is attached and forward the image through the same adapter the model-driven path uses, and stop mapping a failed adapter to a text-only success. MCP's ImageContent and CallToolResult take their payload type like TextContent does; resolveScope reuses resolveTarget; needsLocator folds into replayRequires. --- src/agent/Agent.zig | 34 +++++++++++++++++------------- src/browser/screenshot.zig | 25 ++++------------------- src/browser/tools.zig | 42 +++++++++++++++----------------------- src/mcp/protocol.zig | 20 ++++++++++-------- src/mcp/tools.zig | 7 ++++--- src/script/skill.zig | 4 +++- 6 files changed, 59 insertions(+), 73 deletions(-) diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index dd2d568a2..f033575a1 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -1488,7 +1488,8 @@ fn runCommand(self: *Agent, arena: std.mem.Allocator, cmd: Command) browser_tool .tool_call => |t| t, else => return .{ .text = "internal: command has no tool mapping", .is_error = true }, }; - return browser_tools.call(arena, self.session, &self.node_registry, tc.name(), tc.args, .{}) catch |err| .{ + // The terminal can't show an image, but the conversation can. + return browser_tools.call(arena, self.session, &self.node_registry, tc.name(), tc.args, .{ .inline_image = self.ai_client != null }) catch |err| .{ .text = switch (err) { error.OutOfMemory => "out of memory", error.FrameNotLoaded => "no page loaded — run /goto first", @@ -1620,6 +1621,7 @@ fn recordSlashToolCall( .id = try ma.dupe(u8, tool_calls[0].id), .name = try ma.dupe(u8, tool_calls[0].name), .content = content, + .parts = if (result.image) |image| try imageParts(ma, content, &image) else null, .is_error = result.is_error, }; @@ -1922,14 +1924,10 @@ fn handleToolCall(ctx: *anyopaque, allocator: std.mem.Allocator, tool_name: []co self.terminal.spinner.setTool(tool_name, args_str); defer self.terminal.spinner.setThinking(); - const outcome: zenai.provider.Client.ToolHandler.Result = if (browser_tools.call(allocator, self.session, &self.node_registry, tool_name, arguments, .{ .inline_image = true })) |result| blk: { - const content = capToolOutput(allocator, tool_name, result.text); - break :blk .{ - .content = content, - .is_error = result.is_error, - .parts = if (result.image) |image| imageParts(allocator, content, &image) catch null else null, - }; - } else |err| .{ .content = std.fmt.allocPrint(allocator, "Error: {s}", .{browser_tools.errorMessage(err)}) catch "Error: tool execution failed", .is_error = true }; + const outcome = self.toolOutcome(allocator, tool_name, arguments) catch |err| zenai.provider.Client.ToolHandler.Result{ + .content = std.fmt.allocPrint(allocator, "Error: {s}", .{browser_tools.errorMessage(err)}) catch "Error: tool execution failed", + .is_error = true, + }; self.terminal.agentToolDone(tool_name, args_str, !outcome.is_error); if (self.terminal.verbosity == .high) self.terminal.printToolOutcome(tool_name, outcome.content, outcome.is_error); @@ -1937,11 +1935,19 @@ fn handleToolCall(ctx: *anyopaque, allocator: std.mem.Allocator, tool_name: []co } /// The text plus the rendered PNG, for backends that can show the model an image. -fn imageParts(allocator: std.mem.Allocator, text: []const u8, image: *const lp.screenshot.Prepared) ![]const zenai.provider.ContentPart { - const parts = try allocator.alloc(zenai.provider.ContentPart, 2); - parts[0] = .{ .text = text }; - parts[1] = .{ .image = .{ .data = try image.base64Alloc(allocator), .mime_type = "image/png" } }; - return parts; +fn toolOutcome(self: *Agent, allocator: std.mem.Allocator, tool_name: []const u8, arguments: ?std.json.Value) browser_tools.ToolError!zenai.provider.Client.ToolHandler.Result { + const result = try browser_tools.call(allocator, self.session, &self.node_registry, tool_name, arguments, .{ .inline_image = true }); + const content = capToolOutput(allocator, tool_name, result.text); + return .{ + .content = content, + .is_error = result.is_error, + .parts = if (result.image) |image| try imageParts(allocator, content, &image) else null, + }; +} + +fn imageParts(arena: std.mem.Allocator, text: []const u8, image: *const lp.screenshot.Prepared) browser_tools.ToolError![]const zenai.provider.ContentPart { + const data = image.base64Alloc(arena) catch return error.InternalError; + return try arena.dupe(zenai.provider.ContentPart, &.{ .{ .text = text }, .{ .image = .{ .data = data, .mime_type = "image/png" } } }); } /// One-shot for `--list-models`: resolve provider+key, fetch chat-capable model diff --git a/src/browser/screenshot.zig b/src/browser/screenshot.zig index 9643c3950..ee50b0522 100644 --- a/src/browser/screenshot.zig +++ b/src/browser/screenshot.zig @@ -136,11 +136,10 @@ pub const Prepared = struct { } /// The PNG as base64, for APIs that want it as one string. - pub fn base64Alloc(self: *const Prepared, allocator: Allocator) ![]const u8 { - var aw: std.Io.Writer.Allocating = .init(allocator); - errdefer aw.deinit(); + pub fn base64Alloc(self: *const Prepared, arena: Allocator) ![]const u8 { + var aw: std.Io.Writer.Allocating = .init(arena); try self.writeBase64(&aw.writer); - return aw.toOwnedSlice(); + return aw.written(); } fn writeBase64(self: *const Prepared, writer: *std.Io.Writer) std.Io.Writer.Error!void { @@ -676,23 +675,6 @@ test "browser.screenshot: png signature and dimensions" { try testing.expectEqual(true, height > 60 and height < 200); } -test "browser.screenshot: base64Alloc is the png, base64 encoded" { - defer testing.test_session.closeAllPages(); - const frame = try testing.createFrame(); - frame.url = "http://localhost/"; - const div = try frame.window._document.createElement("div", null, frame); - try Frame.parse.htmlAsChildren(frame, div.asNode(), "

Hello

"); - - const prepared = try prepare(testing.arena_allocator, div.asNode(), .{ .width = 320 }, frame); - const b64 = try prepared.base64Alloc(testing.arena_allocator); - try testing.expect(std.mem.startsWith(u8, b64, "iVBORw0KGgo")); - - const decoder = std.base64.standard.Decoder; - const bytes = try testing.arena_allocator.alloc(u8, try decoder.calcSizeForSlice(b64)); - try decoder.decode(bytes, b64); - try testing.expectEqual("\x89PNG\r\n\x1a\n", bytes[0..8]); -} - test "browser.screenshot: fixed height, clip and scale" { defer testing.test_session.closeAllPages(); const frame = try testing.createFrame(); @@ -810,6 +792,7 @@ test "browser.screenshot: json streams base64" { _ = enc.encode(expected[9 .. expected.len - 2], raw.written()); @memcpy(expected[expected.len - 2 ..], "\"}"); try testing.expectString(expected, json.written()); + try testing.expectString(expected[9 .. expected.len - 2], try prepared.base64Alloc(testing.arena_allocator)); } test "browser.screenshot: block extraction" { diff --git a/src/browser/tools.zig b/src/browser/tools.zig index ee2832e73..d8a315ace 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -40,8 +40,7 @@ const Selector = @import("webapi/selector/Selector.zig"); pub const driver_guidance = \\You are driving Lightpanda, a headless browser, through text tools: \\you reason over pages as a semantic tree, markdown or HTML. `screenshot` - \\renders that text layout as a PNG (no images, fonts or CSS) — use it - \\for spatial layout, not as a primary read. + \\renders that text layout as a PNG: for spatial layout, not a primary read. \\ \\Reading pages (cheap → expensive — prefer cheaper): \\- `tree` → semantic overview (role, name, value, backendNodeId per @@ -215,14 +214,6 @@ pub const save_script_rules = /// `..` segment. Operator-controlled symlinks already inside CWD are out /// of scope — the threat we close here is "client supplies an arbitrary /// path string". -pub const unsafe_path_message = "path must be relative and must not contain '..' segments"; - -/// The cwd is the server's, not one the user picked, so report where a file -/// really went. -pub fn absolutePath(arena: std.mem.Allocator, path: []const u8) []const u8 { - return std.Io.Dir.cwd().realPathFileAlloc(lp.io, path, arena) catch path; -} - pub fn isPathSafe(path: []const u8) bool { if (path.len == 0) return false; if (std.fs.path.isAbsolute(path)) return false; @@ -233,6 +224,14 @@ pub fn isPathSafe(path: []const u8) bool { return true; } +pub const unsafe_path_message = "path must be relative and must not contain '..' segments"; + +/// The cwd is the server's, not one the user picked, so report where a file +/// really went. +pub fn absolutePath(arena: std.mem.Allocator, path: []const u8) []const u8 { + return std.Io.Dir.cwd().realPathFileAlloc(lp.io, path, arena) catch path; +} + /// Hand-written so per-tool semantics (record/heal/locator/data) and /// LLM-facing metadata (`definition`) live as exhaustive switches on the /// tag — adding a new tool is a compile error until each predicate AND @@ -320,13 +319,6 @@ pub const Tool = enum { }; } - pub fn needsLocator(self: Tool) bool { - for (self.replayRequires()) |field| { - if (std.mem.eql(u8, field, "selector")) return true; - } - return false; - } - /// Result is data the caller probably wants on stdout (extracted JSON, /// markdown, evaluate return value) rather than a status line on stderr. pub fn producesData(self: Tool) bool { @@ -1322,9 +1314,8 @@ fn execMarkdown(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNo /// The node a read tool works on: the selector match, the registry node, or /// the whole document. All three live in the current frame. fn resolveScope(session: *lp.Session, registry: *CDPNode.Registry, page: *lp.Frame, selector: ?[]const u8, node_id: ?CDPNode.Id) ToolError!*DOMNode { - if (selector) |sel| return (try resolveBySelector(session, sel)).node; - if (node_id) |nid| return (try resolveNodeAndPage(session, registry, nid)).node; - return page.document.asNode(); + if (selector == null and node_id == null) return page.document.asNode(); + return (try resolveTarget(session, registry, selector, node_id)).node; } const HtmlParams = struct { @@ -1345,7 +1336,7 @@ fn execHtml(arena: std.mem.Allocator, session: *lp.Session, registry: *CDPNode.R if (args.selector == null and args.backendNodeId == null) { lp.dump.root(page.document, opts, &aw.writer, page) catch return ToolError.InternalError; } else { - const node = try resolveScope(session, registry, page, args.selector, args.backendNodeId); + const node = (try resolveTarget(session, registry, args.selector, args.backendNodeId)).node; lp.dump.deep(node, opts, &aw.writer, page) catch return ToolError.InternalError; } return aw.written(); @@ -1368,19 +1359,18 @@ fn execScreenshot(arena: std.mem.Allocator, session: *lp.Session, registry: *CDP } const page = try ensurePage(session, registry, args.url, args.timeout); const node = try resolveScope(session, registry, page, args.selector, args.backendNodeId); - const prepared = lp.screenshot.prepare(arena, node, .fromViewport(page._page.getViewport(), args.fullPage), page) catch - return ToolError.InternalError; - const width = prepared.opts.width; + const opts: lp.screenshot.Opts = .fromViewport(page._page.getViewport(), args.fullPage); + const prepared = lp.screenshot.prepare(arena, node, opts, page) catch return ToolError.InternalError; if (args.path) |path| { const height = writePng(&prepared, path) catch |err| return .{ .text = std.fmt.allocPrint(arena, "could not write {s}: {s}", .{ path, @errorName(err) }) catch return ToolError.OutOfMemory, .is_error = true, }; - return .{ .text = std.fmt.allocPrint(arena, "Saved {d}x{d} PNG to {s}", .{ width, height, absolutePath(arena, path) }) catch return ToolError.OutOfMemory }; + return .{ .text = std.fmt.allocPrint(arena, "Saved {d}x{d} PNG to {s}", .{ opts.width, height, absolutePath(arena, path) }) catch return ToolError.OutOfMemory }; } return .{ - .text = std.fmt.allocPrint(arena, "PNG, {d}px wide", .{width}) catch return ToolError.OutOfMemory, + .text = std.fmt.allocPrint(arena, "PNG, {d}px wide", .{opts.width}) catch return ToolError.OutOfMemory, .image = prepared, }; } diff --git a/src/mcp/protocol.zig b/src/mcp/protocol.zig index 8b657bd4b..f4a6f4a46 100644 --- a/src/mcp/protocol.zig +++ b/src/mcp/protocol.zig @@ -167,12 +167,6 @@ pub const CallParams = struct { arguments: ?std.json.Value = null, }; -pub const ImageContent = struct { - type: []const u8 = "image", - data: @import("../browser/screenshot.zig").Prepared, - mimeType: []const u8 = "image/png", -}; - pub fn TextContent(comptime T: type) type { return struct { type: []const u8 = "text", @@ -180,9 +174,19 @@ pub fn TextContent(comptime T: type) type { }; } -pub fn CallToolResult(comptime T: type) type { +/// `T` serializes as the base64 payload. +pub fn ImageContent(comptime T: type) type { return struct { - content: []const TextContent(T), + type: []const u8 = "image", + data: T, + mimeType: []const u8, + }; +} + +/// `Content` is the content array: a slice or tuple of `TextContent`/`ImageContent`. +pub fn CallToolResult(comptime Content: type) type { + return struct { + content: Content, isError: bool = false, }; } diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index 259e1c497..66f86dbe3 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -173,8 +173,9 @@ fn dispatchBrowserTool( }; if (result.image) |image| { - return server.sendResult(id, .{ - .content = .{ protocol.ImageContent{ .data = image }, protocol.TextContent([]const u8){ .text = result.text } }, + const Content = struct { protocol.ImageContent(lp.screenshot.Prepared), protocol.TextContent([]const u8) }; + return server.sendResult(id, protocol.CallToolResult(Content){ + .content = .{ .{ .data = image, .mimeType = "image/png" }, .{ .text = result.text } }, .isError = result.is_error, }); } @@ -289,7 +290,7 @@ fn writeScript(path: []const u8, content: []const u8) !void { fn sendToolResultText(server: *Server, id: std.json.Value, msg: []const u8, is_error: bool) !void { const content = [_]protocol.TextContent([]const u8){.{ .text = msg }}; - try server.sendResult(id, protocol.CallToolResult([]const u8){ .content = &content, .isError = is_error }); + try server.sendResult(id, protocol.CallToolResult([]const protocol.TextContent([]const u8)){ .content = &content, .isError = is_error }); } fn sendErrorContent(server: *Server, id: std.json.Value, msg: []const u8) !void { diff --git a/src/script/skill.zig b/src/script/skill.zig index 3cbf47b7a..7ad4306c9 100644 --- a/src/script/skill.zig +++ b/src/script/skill.zig @@ -180,7 +180,9 @@ fn positionalOptional(s: *const Schema, p: []const u8) bool { if (s.findField(p)) |f| { if (f.default_true) return true; } - if (std.mem.eql(u8, p, "selector") and s.tool.needsLocator()) return false; + for (s.tool.replayRequires()) |r| { + if (std.mem.eql(u8, r, p)) return false; + } for (s.required) |r| { if (std.mem.eql(u8, r, p)) return false; }