From 4da152e9aaec1a13509647ea7bdfa9a6957b5799 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 15:42:35 +0200 Subject: [PATCH 01/16] agent: resolve the replay selector in the tool layer `--save` drops any tool call that names its element by `backendNodeId` and carries no selector (`Command.ToolCall.isRecorded`), because a registry id means nothing in a later session. That is a silent gap on the chat path: whenever the model addresses a node by id, the action runs and is then quietly omitted from the saved script. The tool layer is the one place that already resolves the node before acting, so it is where the selector can be taken while the node still exists -- a navigation takes it away a moment later. `CallOpts.record` asks for it and `ToolResult.selector` returns it, for every tool, with no per-tool knowledge anywhere. Recording then swaps `backendNodeId` for the selector and keeps every other argument, so `scroll`'s `y` and `waitForState`'s arguments survive. One `save_selectors` entry per call, errors included, so the index lines up with `RunToolsResult.tool_calls_made`. --- src/agent/Agent.zig | 50 ++++++++++++++++++++++++++++++++++++++----- src/browser/tools.zig | 39 ++++++++++++++++++++++++++++++++- 2 files changed, 83 insertions(+), 6 deletions(-) diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 5207605ec..ae870f162 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -167,6 +167,11 @@ cancel_requested: std.atomic.Value(bool) = .init(false), /// mid-request instead of blocking until the model's full response arrives. http_interrupt: zenai.http.Interrupt = .{}, synthetic_tool_call_id: u32 = 0, +/// Per-turn CSS selector for each tool call the model made, in call order, so +/// `--save` can record a call that addressed its element by `backendNodeId`. +/// Only filled while `capturing_for_save`. +save_selectors: std.ArrayListUnmanaged(?[]const u8) = .empty, +capturing_for_save: bool = false, /// Aggregate Anthropic/OpenAI/Gemini token usage across every model call. /// Printed as a structured `$usage ...` line on stderr at the end of `--task` /// (one-shot) mode so wrappers can capture per-task cost. @@ -355,6 +360,7 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent pub fn deinit(self: *Agent) void { self.terminal.uninstallLogSink(); self.save_buffer.deinit(); + self.save_selectors.deinit(self.allocator); if (self.save_path) |p| self.allocator.free(p); self.terminal.deinit(); self.conversation.deinit(); @@ -1328,6 +1334,21 @@ fn logSaveBufferError(self: *Agent, err: anyerror) void { self.terminal.printError("save buffer disabled: {s}", .{@errorName(err)}); } +/// Swap a call's ephemeral `backendNodeId` for the selector the tool layer +/// resolved, so the call can be replayed. Returns `args` untouched when there +/// is nothing to swap. +fn withSelector(arena: std.mem.Allocator, args: ?std.json.Value, selector: ?[]const u8) ?std.json.Value { + const sel = selector orelse return args; + const original = args orelse return args; + if (original != .object) return args; + if (!original.object.contains("backendNodeId")) return args; + + var rewritten = original.object.clone(arena) catch return args; + _ = rewritten.swapRemove("backendNodeId"); + rewritten.put(arena, "selector", .{ .string = sel }) catch return args; + return .{ .object = rewritten }; +} + fn recordSaveCommand(self: *Agent, cmd: Command) void { self.save_buffer.record(cmd) catch |err| self.logSaveBufferError(err); } @@ -1671,6 +1692,10 @@ fn processUserMessage(self: *Agent, input: TurnInput) !?[]const u8 { const provider_client = self.ai_client orelse return error.NoAiClient; self.refreshAuthIfNeeded(); + self.capturing_for_save = input.capture_for_save; + defer self.capturing_for_save = false; + self.save_selectors.clearRetainingCapacity(); + self.terminal.spinner.start(); var result = provider_client.runTools( self.model, @@ -1731,11 +1756,15 @@ fn processUserMessage(self: *Agent, input: TurnInput) !?[]const u8 { const args = browser_tools.normalizeArgKeys(ca, tool, tc.arguments) catch tc.arguments; // Fall back to the navigation a read tool performed, so a // markdown/tree-driven turn isn't lost from `/save`. - const cmd = Command.fromToolCall(tool, args); + // A call that named its element by id is unreplayable as-is; the + // tool layer resolved a selector for it while the node still + // existed. + const replayable = withSelector(ca, args, if (i < self.save_selectors.items.len) self.save_selectors.items[i] else null); + const cmd = Command.fromToolCall(tool, replayable); const to_record = if (cmd.isRecorded()) cmd else - navigationGoto(ca, tool, args) orelse continue; + navigationGoto(ca, tool, replayable) orelse continue; if (!recorded_any) { if (input.record_comment) |c| self.recordSaveComment(c); recorded_any = true; @@ -1888,10 +1917,17 @@ 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 = self.toolOutcome(allocator, tool_name, arguments) catch |err| zenai.provider.Client.ToolHandler.Result{ + var selector: ?[]const u8 = null; + const outcome = self.toolOutcome(allocator, tool_name, arguments, &selector) 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, }; + if (self.capturing_for_save) { + // One entry per call, errors included, so the index lines up with + // `RunToolsResult.tool_calls_made`. + const kept = if (selector) |sel| self.allocator.dupe(u8, sel) catch null else null; + self.save_selectors.append(self.allocator, kept) catch {}; + } 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); @@ -1899,8 +1935,12 @@ 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 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.ts.session, &self.ts.registry, tool_name, arguments, .{ .inline_image = true }); +fn toolOutcome(self: *Agent, allocator: std.mem.Allocator, tool_name: []const u8, arguments: ?std.json.Value, selector: *?[]const u8) browser_tools.ToolError!zenai.provider.Client.ToolHandler.Result { + const result = try browser_tools.call(allocator, self.ts.session, &self.ts.registry, tool_name, arguments, .{ + .inline_image = true, + .record = self.capturing_for_save, + }); + selector.* = result.selector; const content = capToolOutput(allocator, tool_name, result.text); return .{ .content = content, diff --git a/src/browser/tools.zig b/src/browser/tools.zig index 9b8d31e52..9a7bfcf88 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -24,6 +24,8 @@ const NodeRegistry = @import("../NodeRegistry.zig"); const DOMNode = @import("webapi/Node.zig"); const Selector = @import("webapi/selector/Selector.zig"); +const SelectorPath = @import("SelectorPath.zig"); +const Element = @import("webapi/Element.zig"); const log = lp.log; const tavily = zenai.search.tavily; @@ -823,6 +825,10 @@ pub const ToolResult = struct { is_error: bool = false, /// Only set when the caller passed `CallOpts.inline_image`. image: ?lp.screenshot.Prepared = null, + /// Only set when the caller passed `CallOpts.record` and the call named its + /// element by `backendNodeId`. Resolved before the action runs, because a + /// navigation takes the node with it. + selector: ?[]const u8 = null, }; const GotoParams = struct { @@ -854,6 +860,11 @@ const NodeAndPage = struct { node: *DOMNode, page: *lp.Frame, target: ActionTarg pub const CallOpts = struct { /// The caller can hand an image to a model. inline_image: bool = false, + /// The caller is recording for `--save`/`/save`. A call that addresses its + /// element by `backendNodeId` gets `ToolResult.selector` filled in, since + /// a registry id means nothing in a later session and the node may be gone + /// by the time the caller wants to record it. + record: bool = false, }; // An inline screenshot is re-sent on every turn; keep it within what models @@ -886,13 +897,39 @@ pub fn call( }; const substituted = try substituteStringArgs(arena, tool, normalized); - return dispatch(arena, session, registry, tool, substituted, opts) catch |err| { + // Before dispatch: after a navigation the node is gone. + const selector = if (opts.record) selectorForArgs(arena, session, registry, substituted) else null; + + var result = dispatch(arena, session, registry, tool, substituted, opts) catch |err| { if (err == error.NavigationFailed) { if (formatNavigationError(arena, session)) |text| return .{ .text = text, .is_error = true }; } return err; }; + result.selector = selector; + return result; +} + +/// The CSS selector for a call's `backendNodeId`, so the call can be recorded +/// in a form that still resolves in a later session. Null when the arguments +/// name no node, already carry a selector, or the node cannot be named. +fn selectorForArgs( + arena: std.mem.Allocator, + session: *lp.Session, + registry: *NodeRegistry, + arguments: ?std.json.Value, +) ?[]const u8 { + const args = arguments orelse return null; + if (args != .object) return null; + if (args.object.contains("selector")) return null; + const id = args.object.get("backendNodeId") orelse return null; + if (id != .integer) return null; + + const node = registry.lookup_by_id.get(std.math.cast(NodeRegistry.Id, id.integer) orelse return null) orelse return null; + const el = node.dom.is(Element) orelse return null; + const frame = session.currentFrame() orelse return null; + return SelectorPath.init(arena, frame).build(el) catch null; } fn dispatch( From 1e081b5ac8013a286a450ed770f4de9fff2f1545 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 15:44:55 +0200 Subject: [PATCH 02/16] tools: collect search hits once, render them once The four per-engine markdown writers differed only in which response field holds the title, the url and the snippet; the numbering, the newline flattening and the "No results." case were copied four times. Each engine now contributes a `collect` that maps its response onto `[]Hit`, and one `renderResults` turns those into the markdown the model reads. Adding an engine is a field mapping rather than another copy of the renderer, and the intermediate `SearchResults` is a shape a caller can use directly instead of re-parsing markdown. Output is byte-identical -- the existing tests assert the exact strings and are unchanged apart from calling the new pair. --- src/browser/tools.zig | 158 +++++++++++++++++++++--------------------- 1 file changed, 79 insertions(+), 79 deletions(-) diff --git a/src/browser/tools.zig b/src/browser/tools.zig index 9a7bfcf88..68b53c43f 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -1092,7 +1092,7 @@ const api_engines = .{ .init_options = brave.Client.InitOptions{}, // text_decorations=false: no markup in model-read snippets. .options = brave.types.SearchOptions{ .count = 10, .text_decorations = false }, - .format = formatBraveMarkdown, + .collect = collectBrave, }, .{ .tag = SearchEngine.tavily, @@ -1100,7 +1100,7 @@ const api_engines = .{ .Client = tavily.Client, .init_options = tavily.Client.InitOptions{}, .options = tavily.types.SearchOptions{ .max_results = 10 }, - .format = formatTavilyMarkdown, + .collect = collectTavily, }, .{ .tag = SearchEngine.exa, @@ -1110,7 +1110,7 @@ const api_engines = .{ // highlights: Exa returns no snippet text unless contents is requested; // capped at 3 sentences since the default excerpts run long. .options = exa.types.SearchOptions{ .numResults = 10, .contents = .{ .highlights = .{ .numSentences = 3 } } }, - .format = formatExaMarkdown, + .collect = collectExa, }, .{ .tag = SearchEngine.keenable, @@ -1120,7 +1120,7 @@ const api_engines = .{ // snippet_max_length is a hint the API may round up to a word // boundary; 500 keeps ten results within a few KB of context. .options = keenable.types.SearchOptions{ .max_results = 10, .snippet_max_length = 500 }, - .format = formatKeenableMarkdown, + .collect = collectKeenable, }, }; @@ -1261,54 +1261,66 @@ fn apiSearch( }; defer response.deinit(); + return renderResults(arena, try engine.collect(arena, response.value)); +} + +pub const Hit = struct { + title: []const u8, + url: []const u8, + snippet: []const u8, +}; + +pub const SearchResults = struct { + /// Tavily's synthesized answer; empty for the engines that have none. + answer: []const u8 = "", + hits: []const Hit = &.{}, +}; + +fn collectTavily(arena: std.mem.Allocator, resp: tavily.types.SearchResponse) !SearchResults { + const hits = try arena.alloc(Hit, resp.results.len); + for (resp.results, hits) |r, *hit| hit.* = .{ .title = r.title, .url = r.url, .snippet = r.content }; + return .{ .answer = resp.answer orelse "", .hits = hits }; +} + +fn collectBrave(arena: std.mem.Allocator, resp: brave.types.SearchResponse) !SearchResults { + const results: []const brave.types.Result = if (resp.web) |web| web.results else &.{}; + const hits = try arena.alloc(Hit, results.len); + for (results, hits) |r, *hit| hit.* = .{ .title = r.title, .url = r.url, .snippet = r.description }; + return .{ .hits = hits }; +} + +fn collectExa(arena: std.mem.Allocator, resp: exa.types.SearchResponse) !SearchResults { + const hits = try arena.alloc(Hit, resp.results.len); + for (resp.results, hits) |r, *hit| { + const highlights = r.highlights orelse &[_][]const u8{}; + hit.* = .{ + .title = r.title orelse "", + .url = r.url, + .snippet = if (highlights.len > 0) highlights[0] else "", + }; + } + return .{ .hits = hits }; +} + +fn collectKeenable(arena: std.mem.Allocator, resp: keenable.types.SearchResponse) !SearchResults { + const hits = try arena.alloc(Hit, resp.results.len); + // snippet carries the page text (the wire format's always-empty + // `description` is deliberately not even mapped by the client). + for (resp.results, hits) |r, *hit| hit.* = .{ .title = r.title, .url = r.url, .snippet = r.snippet }; + return .{ .hits = hits }; +} + +fn renderResults(arena: std.mem.Allocator, results: SearchResults) ToolError![]const u8 { + if (results.answer.len == 0 and results.hits.len == 0) return "No results."; var aw: std.Io.Writer.Allocating = .init(arena); - try engine.format(&aw.writer, response.value); + const w = &aw.writer; + renderInto(w, results) catch return ToolError.OutOfMemory; return aw.written(); } -fn formatTavilyMarkdown(w: *std.Io.Writer, resp: tavily.types.SearchResponse) !void { - const answer = resp.answer orelse ""; - if (answer.len == 0 and resp.results.len == 0) { - return w.writeAll("No results."); - } - if (answer.len > 0) { - try w.print("**Answer:** {s}\n\n", .{answer}); - } - for (resp.results, 0..) |r, i| { - try writeResultItem(w, i, r.title, r.url, r.content); - } -} - -fn formatBraveMarkdown(w: *std.Io.Writer, resp: brave.types.SearchResponse) !void { - const results: []const brave.types.Result = if (resp.web) |web| web.results else &.{}; - if (results.len == 0) { - return w.writeAll("No results."); - } - for (results, 0..) |r, i| { - try writeResultItem(w, i, r.title, r.url, r.description); - } -} - -fn formatExaMarkdown(w: *std.Io.Writer, resp: exa.types.SearchResponse) !void { - if (resp.results.len == 0) { - return w.writeAll("No results."); - } - for (resp.results, 0..) |r, i| { - const highlights = r.highlights orelse &[_][]const u8{}; - const snippet = if (highlights.len > 0) highlights[0] else ""; - try writeResultItem(w, i, r.title orelse "", r.url, snippet); - } -} - -fn formatKeenableMarkdown(w: *std.Io.Writer, resp: keenable.types.SearchResponse) !void { - if (resp.results.len == 0) { - return w.writeAll("No results."); - } - for (resp.results, 0..) |r, i| { - // snippet carries the page text (the wire format's always-empty - // `description` is deliberately not even mapped by the client). - try writeResultItem(w, i, r.title, r.url, r.snippet); - } +fn renderInto(w: *std.Io.Writer, results: SearchResults) !void { + if (results.answer.len > 0) try w.print("**Answer:** {s}\n\n", .{results.answer}); + for (results.hits, 0..) |hit, i| try writeResultItem(w, i, hit.title, hit.url, hit.snippet); } /// An empty title (providers default it to "") would render as `****`. @@ -2782,7 +2794,7 @@ test "formatLpEnvNames reports empty when no names" { try std.testing.expectEqualStrings("No LP_* environment variables are set.", r); } -test "formatTavilyMarkdown renders answer and results" { +test "collectTavily renders answer and results" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2796,9 +2808,7 @@ test "formatTavilyMarkdown renders answer and results" { }, }; - var aw: std.Io.Writer.Allocating = .init(aa); - try formatTavilyMarkdown(&aw.writer, resp); - const md = aw.written(); + const md = try renderResults(aa, try collectTavily(aa, resp)); try std.testing.expect(std.mem.indexOf(u8, md, "**Answer:** Paris") != null); try std.testing.expect(std.mem.indexOf(u8, md, "1. **Paris - Wikipedia**") != null); try std.testing.expect(std.mem.indexOf(u8, md, "https://en.wikipedia.org/wiki/Paris") != null); @@ -2806,17 +2816,15 @@ test "formatTavilyMarkdown renders answer and results" { try std.testing.expect(std.mem.indexOf(u8, md, "2. **France**") != null); } -test "formatTavilyMarkdown handles empty results" { +test "collectTavily handles empty results" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); - var aw: std.Io.Writer.Allocating = .init(aa); - try formatTavilyMarkdown(&aw.writer, .{}); - try std.testing.expectEqualStrings("No results.", aw.written()); + try std.testing.expectEqualStrings("No results.", try renderResults(aa, try collectTavily(aa, .{}))); } -test "formatBraveMarkdown renders web results" { +test "collectBrave renders web results" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2830,30 +2838,24 @@ test "formatBraveMarkdown renders web results" { }, }; - var aw: std.Io.Writer.Allocating = .init(aa); - try formatBraveMarkdown(&aw.writer, resp); - const md = aw.written(); + const md = try renderResults(aa, try collectBrave(aa, resp)); try std.testing.expect(std.mem.indexOf(u8, md, "1. **Paris - Wikipedia**") != null); try std.testing.expect(std.mem.indexOf(u8, md, "https://en.wikipedia.org/wiki/Paris") != null); try std.testing.expect(std.mem.indexOf(u8, md, "Paris is the capital of France.") != null); try std.testing.expect(std.mem.indexOf(u8, md, "2. **France**") != null); } -test "formatBraveMarkdown handles empty results" { +test "collectBrave handles empty results" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); - var no_web: std.Io.Writer.Allocating = .init(aa); - try formatBraveMarkdown(&no_web.writer, .{}); - try std.testing.expectEqualStrings("No results.", no_web.written()); + try std.testing.expectEqualStrings("No results.", try renderResults(aa, try collectBrave(aa, .{}))); - var empty_web: std.Io.Writer.Allocating = .init(aa); - try formatBraveMarkdown(&empty_web.writer, .{ .web = .{} }); - try std.testing.expectEqualStrings("No results.", empty_web.written()); + try std.testing.expectEqualStrings("No results.", try renderResults(aa, try collectBrave(aa, .{ .web = .{} }))); } -test "formatKeenableMarkdown reads snippet" { +test "collectKeenable reads snippet" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2866,9 +2868,7 @@ test "formatKeenableMarkdown reads snippet" { }, }; - var aw: std.Io.Writer.Allocating = .init(aa); - try formatKeenableMarkdown(&aw.writer, resp); - const md = aw.written(); + const md = try renderResults(aa, try collectKeenable(aa, resp)); try std.testing.expect(std.mem.indexOf(u8, md, "1. **Zig (programming language)**") != null); try std.testing.expect(std.mem.indexOf(u8, md, "Zig is a system programming language.") != null); try std.testing.expect(std.mem.indexOf(u8, md, "2. **Zig guide**") != null); @@ -2883,15 +2883,14 @@ test "writeResultItem uses the URL as title when the title is empty" { try std.testing.expectEqualStrings("1. **https://example.org/x.pdf** — https://example.org/x.pdf\n snippet\n\n", aw.written()); } -test "formatKeenableMarkdown handles empty results" { +test "collectKeenable handles empty results" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); - var aw: std.Io.Writer.Allocating = .init(arena.allocator()); - try formatKeenableMarkdown(&aw.writer, .{}); - try std.testing.expectEqualStrings("No results.", aw.written()); + const aa = arena.allocator(); + try std.testing.expectEqualStrings("No results.", try renderResults(aa, try collectKeenable(aa, .{}))); } -test "formatBraveMarkdown flattens newlines in titles and descriptions" { +test "collectBrave flattens newlines in titles and descriptions" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2904,9 +2903,10 @@ test "formatBraveMarkdown flattens newlines in titles and descriptions" { }, }; - var aw: std.Io.Writer.Allocating = .init(aa); - try formatBraveMarkdown(&aw.writer, resp); - try std.testing.expectEqualStrings("1. **Multi line title** — https://example.org\n line one line two\n\n", aw.written()); + try std.testing.expectEqualStrings( + "1. **Multi line title** — https://example.org\n line one line two\n\n", + try renderResults(aa, try collectBrave(aa, resp)), + ); } test "isPathSafe: relative paths without traversal are accepted" { From 1c9b1b8bc203e94e300331a98b5f694ee2ac962d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 15:47:35 +0200 Subject: [PATCH 03/16] tools: tell the caller why a search failed A failed search reported only the error name: "keenable search failed: ApiError". The status code and the provider's own message were logged once inside `apiSearch` and then died with the client on its deferred deinit, so nothing downstream could tell a rate limit from a rejected key -- and a model reading the tool result had nothing to act on. `Failure` carries the status and body out before the client goes, and the message now names both. A 429 additionally says it is a rate limit and that waiting or reading the page instead are the ways out, because that is the one failure where the right move is not "try another query". Found by a benchmark run where a keyless search engine quietly hit its hourly cap; every search failed for twenty minutes and the run just looked like a bad agent. --- src/browser/tools.zig | 77 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 67 insertions(+), 10 deletions(-) diff --git a/src/browser/tools.zig b/src/browser/tools.zig index 68b53c43f..5b597dfdc 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -1187,19 +1187,26 @@ fn execSearch(arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError!To switch (search_engine) { .auto => { var last_err: ?anyerror = null; + var last_label: []const u8 = "web"; + var last_detail: Failure = .{}; inline for (api_engines) |engine| { if (engineKey(engine)) |api_key| { // Fall through on any failure so one outage doesn't kill // a whole benchmark run. - if (apiSearch(engine, arena, api_key, timeout_ms, args.query)) |markdown_| { + var detail: Failure = .{}; + if (apiSearch(engine, arena, api_key, timeout_ms, args.query, &detail)) |markdown_| { return .{ .text = markdown_ }; } else |err| { last_err = err; + last_label = @tagName(engine.tag); + last_detail = detail; log.warn(.browser, @tagName(engine.tag) ++ " fallback", .{ .err = err }); } } else |_| {} } - return searchFailed(arena, "web", last_err.?); + // The last rung's reason, not a generic one: every engine having + // failed is usually one cause, and the model can act on it. + return searchFailed(arena, last_label, last_err.?, last_detail); }, inline else => |tag| { inline for (api_engines) |engine| { @@ -1217,25 +1224,47 @@ fn searchExplicit(arena: std.mem.Allocator, comptime engine: anytype, timeout_ms .text = "web search engine is set to " ++ label ++ " but " ++ engine.env_var ++ " is not set in the environment", .is_error = true, }; - const markdown_ = apiSearch(engine, arena, api_key, timeout_ms, query) catch |err| - return searchFailed(arena, label, err); + var detail: Failure = .{}; + const markdown_ = apiSearch(engine, arena, api_key, timeout_ms, query, &detail) catch |err| + return searchFailed(arena, label, err, detail); return .{ .text = markdown_ }; } -fn searchFailed(arena: std.mem.Allocator, comptime label: []const u8, err: anyerror) ToolError!ToolResult { - return .{ - .text = try std.fmt.allocPrint(arena, label ++ " search failed: {s}", .{@errorName(err)}), - .is_error = true, - }; +/// What the provider said, duped out of the client before `deinit` takes it. +/// Without this the status and body survive only in a log line, and every +/// failure reaches the model as the word `InternalError`. +const Failure = struct { + status: ?u10 = null, + message: []const u8 = "", +}; + +/// A rate limit and a bad key need different reactions, so the model is told +/// which it hit rather than just that the search failed. +fn searchFailed(arena: std.mem.Allocator, label: []const u8, err: anyerror, detail: Failure) ToolError!ToolResult { + var aw: std.Io.Writer.Allocating = .init(arena); + const w = &aw.writer; + w.print("{s} search failed: {s}", .{ label, @errorName(err) }) catch return ToolError.OutOfMemory; + if (detail.status) |status| w.print(" (HTTP {d})", .{status}) catch return ToolError.OutOfMemory; + if (detail.message.len > 0) { + w.writeAll(": ") catch return ToolError.OutOfMemory; + writeSingleLine(w, detail.message) catch return ToolError.OutOfMemory; + } + if (detail.status == 429) { + w.writeAll(". This engine is rate-limited right now; wait before retrying, or read the answer from a page instead.") catch + return ToolError.OutOfMemory; + } + return .{ .text = aw.written(), .is_error = true }; } -/// `arena` owns the returned slice. +/// `arena` owns the returned slice. `detail` is filled on a non-2xx so the +/// caller can say what actually happened. fn apiSearch( comptime engine: anytype, arena: std.mem.Allocator, api_key: ?[]const u8, timeout_ms: u32, query: []const u8, + detail: *Failure, ) ![]const u8 { var init_options = engine.init_options; // The cascade (or the model) is the retry; honoring a Retry-After (60 s @@ -1256,6 +1285,11 @@ fn apiSearch( .status = status, .body = client.last_error.body, }); + // `client.last_error` dies with the client on the deferred deinit. + detail.* = .{ + .status = status, + .message = if (client.last_error.body) |b| (arena.dupe(u8, b) catch "") else "", + }; } return err; }; @@ -2909,6 +2943,29 @@ test "collectBrave flattens newlines in titles and descriptions" { ); } +test "searchFailed: a rate limit says so, a bare failure stays short" { + var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); + defer arena.deinit(); + const aa = arena.allocator(); + + const limited = try searchFailed(aa, "keenable", error.ApiError, .{ + .status = 429, + .message = "Public API hourly limit reached.\nWait 2 minutes to continue.", + }); + try std.testing.expect(limited.is_error); + // The status and the provider's own words, which previously reached the + // model only as the word "ApiError". + try std.testing.expect(std.mem.indexOf(u8, limited.text, "(HTTP 429)") != null); + try std.testing.expect(std.mem.indexOf(u8, limited.text, "Public API hourly limit reached.") != null); + try std.testing.expect(std.mem.indexOf(u8, limited.text, "rate-limited right now") != null); + // Flattened: a newline would break the numbered-list markdown around it. + try std.testing.expect(std.mem.indexOf(u8, limited.text, "\n") == null); + + const bare = try searchFailed(aa, "web", error.ConnectionRefused, .{}); + try std.testing.expectEqualStrings("web search failed: ConnectionRefused", bare.text); + try std.testing.expect(bare.is_error); +} + test "isPathSafe: relative paths without traversal are accepted" { try std.testing.expect(isPathSafe("foo.txt")); try std.testing.expect(isPathSafe("./foo.txt")); From 92a6b698fce611bfdd5d521edf5b246b32c90cd2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 15:50:15 +0200 Subject: [PATCH 04/16] agent: --search-engine, and say when the engine has no key The search engine could only be set from the REPL's `/searchEngine`, so a one-shot run had no way to pin one -- and a benchmark that wants its results to mean something has to record which API answered. `/searchEngine` also carried the only warning about key state, which is the half that matters more. A keyless engine is not an error and starts fine, so a run that silently falls back to a rate-limited public endpoint looks exactly like a working one until every search begins failing, and then it looks like a bad agent. Both messages now fire wherever the engine is resolved, not just from the command. `resolveSearchEngine`'s doc comment said there was no CLI flag. There is one now. --- src/Config.zig | 2 ++ src/agent/Agent.zig | 10 +++++++++- src/agent/settings.zig | 4 ++-- src/help.zon | 6 ++++++ 4 files changed, 19 insertions(+), 3 deletions(-) diff --git a/src/Config.zig b/src/Config.zig index 708236f5b..493cb8c1a 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -347,6 +347,7 @@ pub const AiProvider = std.meta.Tag(zenai.provider.Client); /// in `Agent.init` (explicit flag > remembered > mode default), so there is /// no Config-level accessor like `agentVerbosity`. pub const Effort = zenai.provider.Effort; +pub const SearchEngine = @import("browser/tools.zig").SearchEngine; /// Controls how chatty `agent` mode is on stderr. pub const AgentVerbosity = enum { @@ -470,6 +471,7 @@ const Commands = cli.Builder(.{ .{ .name = "attach", .short = 'a', .type = []const u8, .multiple = true }, .{ .name = "verbosity", .type = ?AgentVerbosity }, .{ .name = "effort", .type = ?Effort }, + .{ .name = "search_engine", .type = ?SearchEngine }, .{ .name = "list_models", .type = bool }, .{ .name = "no_llm", .type = bool }, }, diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index ae870f162..9f4f0f219 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -289,7 +289,15 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent const effort = settings.resolveEffort(opts, remembered, will_repl, if (resolved) |r| r.credential.provider else null); const verbosity = settings.resolveVerbosity(opts, remembered); const stream_enabled = settings.resolveStream(remembered); - browser_tools.search_engine = settings.resolveSearchEngine(remembered); + browser_tools.search_engine = opts.search_engine orelse settings.resolveSearchEngine(remembered); + // Only the REPL's `/searchEngine` used to say this. A keyless engine that + // has hit its cap fails every search, and without a word here that reads + // as a bad agent rather than a missing key. + if (browser_tools.searchKeyStatus(browser_tools.search_engine)) |key| switch (key.state) { + .set => {}, + .keyless => log.info(.app, "keyless search endpoint", .{ .env_var = key.env_var, .limit = "rate-limited per client IP" }), + .missing => log.warn(.app, "search key missing", .{ .env_var = key.env_var, .engine = @tagName(browser_tools.search_engine) }), + }; if (resolved) |r| { if (r.source == .picked) { diff --git a/src/agent/settings.zig b/src/agent/settings.zig index 1e90b714b..8c6d00b9e 100644 --- a/src/agent/settings.zig +++ b/src/agent/settings.zig @@ -369,8 +369,8 @@ pub fn resolveStream(remembered: ?Remembered) bool { return true; } -/// Precedence: remembered `.lp-agent.zon` value > default (auto). No CLI -/// flag — the REPL `/searchEngine` command sets and persists it. +/// Precedence: `--search-engine` > remembered `.lp-agent.zon` value > +/// default (auto). The REPL `/searchEngine` command also sets and persists it. pub fn resolveSearchEngine(remembered: ?Remembered) lp.tools.SearchEngine { if (remembered) |r| if (r.search_engine) |e| return e; return .auto; diff --git a/src/help.zon b/src/help.zon index fc329aafb..d59303992 100644 --- a/src/help.zon +++ b/src/help.zon @@ -297,6 +297,12 @@ \\ it to PATH, instead of printing the answer. Replay it later with \\ `run PATH` (no LLM calls). Overwrites PATH if it exists. \\ Requires --task. + \\ --search-engine + \\ Which web search API the `search` tool uses. Default: auto, + \\ which takes the first with a key in the environment, in the + \\ order brave, tavily, exa, keenable. `keenable` also works with + \\ no key at all, through a rate-limited public endpoint. + \\ Allowed values: auto, brave, tavily, exa, keenable. \\ --system-prompt \\ Override the default system prompt. \\ --task From 006af21a6d24e674367d6ec42b1b5da534664cd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 15:52:07 +0200 Subject: [PATCH 05/16] agent: --url opens a start page before the first turn A `--task` run that knows where it is going still spent a model turn navigating there, and the REPL had no way to start anywhere but blank. `--url` opens the page first, through `browser_tools.call` rather than around it, so a bad URL fails like any other tool call and the opening navigation is recorded for `--save` -- which is the first line any replayable script needs anyway. --- src/Config.zig | 1 + src/agent/Agent.zig | 26 ++++++++++++++++++++++++++ src/help.zon | 4 ++++ 3 files changed, 31 insertions(+) diff --git a/src/Config.zig b/src/Config.zig index 493cb8c1a..3a564142c 100644 --- a/src/Config.zig +++ b/src/Config.zig @@ -472,6 +472,7 @@ const Commands = cli.Builder(.{ .{ .name = "verbosity", .type = ?AgentVerbosity }, .{ .name = "effort", .type = ?Effort }, .{ .name = "search_engine", .type = ?SearchEngine }, + .{ .name = "url", .type = ?[:0]const u8 }, .{ .name = "list_models", .type = bool }, .{ .name = "no_llm", .type = bool }, }, diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 9f4f0f219..4e7005110 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -159,6 +159,9 @@ model: []u8, /// Per-turn reasoning budget for LLM turns. Mutable at runtime via `/effort`. effort: Config.Effort, script_file: ?[]const u8, +/// `--url`: opened before the first turn, in every mode. A `--task` run that +/// starts on its page does not spend a model turn navigating to it. +start_url: ?[:0]const u8, one_shot_task: ?[]const u8, one_shot_save: ?[]const u8, one_shot_attachments: ?[]const []const u8, @@ -333,6 +336,7 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent .effort = effort, .stream_enabled = stream_enabled, .script_file = opts.script_file, + .start_url = opts.url, .one_shot_task = opts.task, .one_shot_save = opts.save, .one_shot_attachments = if (opts.attach.items.len == 0) null else opts.attach.items, @@ -513,6 +517,12 @@ const TurnInput = struct { /// Returns true on success. pub fn run(self: *Agent) bool { + if (self.start_url) |url| { + if (self.gotoStart(url, self.one_shot_save != null)) |err| { + self.terminal.printError("could not open {s}: {s}", .{ url, browser_tools.errorMessage(err) }); + return false; + } + } if (self.one_shot_task) |task| { const saving = self.one_shot_save != null; const ok = self.runTurn(.{ @@ -540,6 +550,22 @@ pub fn run(self: *Agent) bool { /// `$usage` prefix. Stable key=value format: /// $usage prompt=N completion=N total=N cached=N cache_creation=N /// Fields emit 0 when the provider didn't report them. +/// Goes through the tool layer so a failed start reads like any other tool +/// failure. +fn gotoStart(self: *Agent, url: [:0]const u8, record: bool) ?browser_tools.ToolError { + var arena: std.heap.ArenaAllocator = .init(self.allocator); + defer arena.deinit(); + const a = arena.allocator(); + + var object: std.json.ObjectMap = .empty; + object.put(a, "url", .{ .string = url }) catch return browser_tools.ToolError.OutOfMemory; + const args: std.json.Value = .{ .object = object }; + _ = browser_tools.call(a, self.ts.session, &self.ts.registry, "goto", args, .{}) catch |err| return err; + // The opening navigation is the first line of any replayable script. + if (record) self.recordSaveCommand(Command.fromToolCall(.goto, args)); + return null; +} + fn printUsageSummary(self: *Agent) void { const u = self.total_usage; std.debug.print( diff --git a/src/help.zon b/src/help.zon index d59303992..32667fd1f 100644 --- a/src/help.zon +++ b/src/help.zon @@ -309,6 +309,10 @@ \\ One-shot mode: run a single user turn, print the final answer \\ to stdout, and exit. Conflicts with the positional script. With \\ --save, the answer is suppressed and a script is written instead. + \\ --url + \\ Open this page before the first turn. With --task it saves the + \\ model a turn spent navigating; with --save the opening + \\ navigation is the first line of the script. \\ --verbosity \\ Stderr chatter level. Default: high when --task captures stderr \\ to a pipe or file; low otherwise. low/medium also raise From b012c44fa000ed86886efd5c3864b83c961b1f98 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 15:53:00 +0200 Subject: [PATCH 06/16] SemanticTree: walk once, in visitAll `jsonStringify` and `textStringify` each carried their own copy of the same six-line preamble -- xpath buffer, listener target map, label index, walk, error log -- differing only in the visitor and the wording of the log line. Either one could drift from the other silently, since nothing compares them. `visitAll` takes the visitor and owns the walk; both dumpers are now one line. Their existing tests cover it. --- src/SemanticTree.zig | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/src/SemanticTree.zig b/src/SemanticTree.zig index aab3576ce..01bf01e6a 100644 --- a/src/SemanticTree.zig +++ b/src/SemanticTree.zig @@ -66,8 +66,10 @@ pub fn init(arena: std.mem.Allocator, node: *Node, registry: *NodeRegistry, fram }; } -pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) error{WriteFailed}!void { - var visitor = JsonVisitor{ .jw = jw, .tree = self }; +/// Walk the pruned tree with `visitor`: `visit(*Node, *NodeData) !bool` +/// returns whether to descend into the children, `leave() !void` closes a +/// visited node. +pub fn visitAll(self: @This(), visitor: anytype) error{WriteFailed}!void { var xpath_buffer: std.ArrayList(u8) = .empty; const listener_targets = interactive.buildListenerTargetMap(self.frame, self.arena) catch |err| { log.err(.app, "listener map failed", .{ .err = err }); @@ -79,29 +81,20 @@ pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) error{WriteFailed}! .listener_targets = listener_targets, .label_index = &label_index, }; - self.walk(&ctx, self.dom_node, null, &visitor, 1, 0) catch |err| { - log.err(.app, "semantic tree json dump failed", .{ .err = err }); + self.walk(&ctx, self.dom_node, null, visitor, 1, 0) catch |err| { + log.err(.app, "semantic tree walk failed", .{ .err = err }); return error.WriteFailed; }; } +pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) error{WriteFailed}!void { + var visitor = JsonVisitor{ .jw = jw, .tree = self }; + return self.visitAll(&visitor); +} + pub fn textStringify(self: @This(), writer: *std.Io.Writer) error{WriteFailed}!void { var visitor = TextVisitor{ .writer = writer, .tree = self, .depth = 0 }; - var xpath_buffer: std.ArrayList(u8) = .empty; - const listener_targets = interactive.buildListenerTargetMap(self.frame, self.arena) catch |err| { - log.err(.app, "listener map failed", .{ .err = err }); - return error.WriteFailed; - }; - var label_index: Label.LabelByForIndex = .{}; - var ctx: WalkContext = .{ - .xpath_buffer = &xpath_buffer, - .listener_targets = listener_targets, - .label_index = &label_index, - }; - self.walk(&ctx, self.dom_node, null, &visitor, 1, 0) catch |err| { - log.err(.app, "semantic tree text dump failed", .{ .err = err }); - return error.WriteFailed; - }; + return self.visitAll(&visitor); } const OptionData = struct { From 182ad25157aa34bf041da7d55c1ed6180e5db95e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 15:53:21 +0200 Subject: [PATCH 07/16] NodeRegistry: pin that reset never reuses an id `reset` clears the lookup maps and deliberately leaves `node_id` where it is, so a stale id fails closed with a miss instead of resolving to whatever registers next. Nothing asserted that, and it is the invariant every id-addressed tool call leans on across a navigation. --- src/NodeRegistry.zig | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/NodeRegistry.zig b/src/NodeRegistry.zig index c48213d80..40f5a81fd 100644 --- a/src/NodeRegistry.zig +++ b/src/NodeRegistry.zig @@ -190,3 +190,21 @@ test "NodeRegistry: resetFrame" { try testing.expectEqual(rb, registry.lookup_by_id.get(rb.id).?); try testing.expectEqual(b_node, registry.lookup_by_node.get(b_node).?.dom); } + +test "NodeRegistry: reset never reuses an id" { + var registry = NodeRegistry.init(testing.allocator); + defer registry.deinit(); + + var page = try testing.pageTest("cdp/registry1.html", .{}); + defer page.close(); + + const frame = page.frame().?; + const dom_node = (try frame.window._document.querySelector(.wrap("#a1"), frame)).?.asNode(); + // The pool recycles the `Node` itself, so keep the id, not the pointer. + const first_id = (try registry.register(dom_node)).id; + + registry.reset(); + try testing.expectEqual(null, registry.lookup_by_id.get(first_id)); + // A stale id stays a miss rather than resolving to whatever registers next. + try testing.expect((try registry.register(dom_node)).id != first_id); +} From c839d1fcca93dc1c4d878c96c4613f360f1cf68a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 15:54:16 +0200 Subject: [PATCH 08/16] agent: refuse to save a script the model did not finish `/save` synthesis never looked at `finish_reason`, and `stripCodeFence` accepts a fenced block with no closing fence. A script cut off at `max_tokens` therefore looked exactly like a complete one: it was written to disk, the save path was remembered, the buffer was reset and the terminal said "Saved synthesized script to ...". The truncation only turned up on replay. It now aborts like any other failed synthesis, leaving the previous file and the buffer alone. --- src/agent/Agent.zig | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 4e7005110..eb13cebb9 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -1311,6 +1311,13 @@ fn synthesizeSaveTo(self: *Agent, arena: std.mem.Allocator, path: []const u8, mo return; } + // `stripCodeFence` accepts a block with no closing fence, so a script cut + // off mid-statement is indistinguishable from a complete one once it is on + // disk. Refuse rather than save something that will not replay. + if (result.finish_reason == .max_tokens) { + return self.abortSave(baseline, "the model ran out of output tokens mid-script"); + } + const raw = result.text orelse return self.abortSave(baseline, "the model returned no script"); // `result.text` lives in the conversation arena, freed by the rollback From 2f0d037a53581b9e713b3c970c215e8d6f405d63 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 16:12:43 +0200 Subject: [PATCH 09/16] agent: tighten the backported fixes Review pass over the branch. `selectorForArgs` ran for any call carrying a `backendNodeId`, before `isRecorded` was consulted. `SelectorPath.build` is the expensive part of a tool call -- uncached full-document matches per ancestor -- and the read-only tools that take an id (tree, markdown, html, nodeDetails) are exactly the ones the system prompt steers toward ids, so the common case built a selector and threw it away. `nodeDetails` built it twice. The recorded selectors were duped into `self.allocator` and never freed: `clearRetainingCapacity` drops the only pointers. They belong to the conversation arena, which already owns the args and the `Command` built from them, so the ownership question disappears rather than being patched with a free loop. `searchKeyStatus` returned null for `.auto`, which is the default -- so the keyless-endpoint notice never fired in the one configuration that reaches a keyless endpoint. It now reports the rung the cascade lands on. `gotoStart` was inserted between `printUsageSummary`'s doc comment and the function, leaving the `$usage` wire format -- which wrapper scripts grep -- documenting the wrong function. It also returned `?ToolError` where the rest of the file uses error unions. Plus: the three search collectors differed only in a field name, the failure writer had five `catch return OutOfMemory` in ten lines beside a neighbour doing it with one, `visitAll` was `pub` with no caller outside its file, and the tests named themselves after a function that no longer renders anything. --- src/NodeRegistry.zig | 1 - src/SemanticTree.zig | 2 +- src/agent/Agent.zig | 51 +++++++---------- src/browser/tools.zig | 130 ++++++++++++++++++++++-------------------- 4 files changed, 91 insertions(+), 93 deletions(-) diff --git a/src/NodeRegistry.zig b/src/NodeRegistry.zig index 40f5a81fd..a9347e36b 100644 --- a/src/NodeRegistry.zig +++ b/src/NodeRegistry.zig @@ -205,6 +205,5 @@ test "NodeRegistry: reset never reuses an id" { registry.reset(); try testing.expectEqual(null, registry.lookup_by_id.get(first_id)); - // A stale id stays a miss rather than resolving to whatever registers next. try testing.expect((try registry.register(dom_node)).id != first_id); } diff --git a/src/SemanticTree.zig b/src/SemanticTree.zig index 01bf01e6a..74794c1ff 100644 --- a/src/SemanticTree.zig +++ b/src/SemanticTree.zig @@ -69,7 +69,7 @@ pub fn init(arena: std.mem.Allocator, node: *Node, registry: *NodeRegistry, fram /// Walk the pruned tree with `visitor`: `visit(*Node, *NodeData) !bool` /// returns whether to descend into the children, `leave() !void` closes a /// visited node. -pub fn visitAll(self: @This(), visitor: anytype) error{WriteFailed}!void { +fn visitAll(self: @This(), visitor: anytype) error{WriteFailed}!void { var xpath_buffer: std.ArrayList(u8) = .empty; const listener_targets = interactive.buildListenerTargetMap(self.frame, self.arena) catch |err| { log.err(.app, "listener map failed", .{ .err = err }); diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index eb13cebb9..33cfbc88e 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -159,8 +159,8 @@ model: []u8, /// Per-turn reasoning budget for LLM turns. Mutable at runtime via `/effort`. effort: Config.Effort, script_file: ?[]const u8, -/// `--url`: opened before the first turn, in every mode. A `--task` run that -/// starts on its page does not spend a model turn navigating to it. +/// `--url`: opened before the first turn, so a `--task` run does not spend a +/// model turn navigating to its own start page. start_url: ?[:0]const u8, one_shot_task: ?[]const u8, one_shot_save: ?[]const u8, @@ -172,7 +172,6 @@ http_interrupt: zenai.http.Interrupt = .{}, synthetic_tool_call_id: u32 = 0, /// Per-turn CSS selector for each tool call the model made, in call order, so /// `--save` can record a call that addressed its element by `backendNodeId`. -/// Only filled while `capturing_for_save`. save_selectors: std.ArrayListUnmanaged(?[]const u8) = .empty, capturing_for_save: bool = false, /// Aggregate Anthropic/OpenAI/Gemini token usage across every model call. @@ -293,9 +292,8 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent const verbosity = settings.resolveVerbosity(opts, remembered); const stream_enabled = settings.resolveStream(remembered); browser_tools.search_engine = opts.search_engine orelse settings.resolveSearchEngine(remembered); - // Only the REPL's `/searchEngine` used to say this. A keyless engine that - // has hit its cap fails every search, and without a word here that reads - // as a bad agent rather than a missing key. + // A keyless engine over its cap fails every search; silence reads as a bad + // agent rather than a missing key. if (browser_tools.searchKeyStatus(browser_tools.search_engine)) |key| switch (key.state) { .set => {}, .keyless => log.info(.app, "keyless search endpoint", .{ .env_var = key.env_var, .limit = "rate-limited per client IP" }), @@ -518,10 +516,10 @@ const TurnInput = struct { /// Returns true on success. pub fn run(self: *Agent) bool { if (self.start_url) |url| { - if (self.gotoStart(url, self.one_shot_save != null)) |err| { + self.gotoStart(url, self.one_shot_save != null) catch |err| { self.terminal.printError("could not open {s}: {s}", .{ url, browser_tools.errorMessage(err) }); return false; - } + }; } if (self.one_shot_task) |task| { const saving = self.one_shot_save != null; @@ -545,27 +543,23 @@ pub fn run(self: *Agent) bool { return true; } -/// Print single-line cumulative token usage to stderr, so wrappers driving -/// `lightpanda agent --task ...` can capture per-task cost by `grep`-ing the -/// `$usage` prefix. Stable key=value format: -/// $usage prompt=N completion=N total=N cached=N cache_creation=N -/// Fields emit 0 when the provider didn't report them. -/// Goes through the tool layer so a failed start reads like any other tool -/// failure. -fn gotoStart(self: *Agent, url: [:0]const u8, record: bool) ?browser_tools.ToolError { +fn gotoStart(self: *Agent, url: [:0]const u8, record: bool) browser_tools.ToolError!void { var arena: std.heap.ArenaAllocator = .init(self.allocator); defer arena.deinit(); const a = arena.allocator(); var object: std.json.ObjectMap = .empty; - object.put(a, "url", .{ .string = url }) catch return browser_tools.ToolError.OutOfMemory; + try object.put(a, "url", .{ .string = url }); const args: std.json.Value = .{ .object = object }; - _ = browser_tools.call(a, self.ts.session, &self.ts.registry, "goto", args, .{}) catch |err| return err; - // The opening navigation is the first line of any replayable script. + _ = try browser_tools.call(a, self.ts.session, &self.ts.registry, "goto", args, .{}); if (record) self.recordSaveCommand(Command.fromToolCall(.goto, args)); - return null; } +/// Print single-line cumulative token usage to stderr, so wrappers driving +/// `lightpanda agent --task ...` can capture per-task cost by `grep`-ing the +/// `$usage` prefix. Stable key=value format: +/// $usage prompt=N completion=N total=N cached=N cache_creation=N +/// Fields emit 0 when the provider didn't report them. fn printUsageSummary(self: *Agent) void { const u = self.total_usage; std.debug.print( @@ -1311,9 +1305,8 @@ fn synthesizeSaveTo(self: *Agent, arena: std.mem.Allocator, path: []const u8, mo return; } - // `stripCodeFence` accepts a block with no closing fence, so a script cut - // off mid-statement is indistinguishable from a complete one once it is on - // disk. Refuse rather than save something that will not replay. + // `stripCodeFence` accepts an unclosed block, so a truncated script is + // indistinguishable from a complete one once it is on disk. if (result.finish_reason == .max_tokens) { return self.abortSave(baseline, "the model ran out of output tokens mid-script"); } @@ -1376,8 +1369,7 @@ fn logSaveBufferError(self: *Agent, err: anyerror) void { } /// Swap a call's ephemeral `backendNodeId` for the selector the tool layer -/// resolved, so the call can be replayed. Returns `args` untouched when there -/// is nothing to swap. +/// resolved, so the call can be replayed. fn withSelector(arena: std.mem.Allocator, args: ?std.json.Value, selector: ?[]const u8) ?std.json.Value { const sel = selector orelse return args; const original = args orelse return args; @@ -1797,9 +1789,6 @@ fn processUserMessage(self: *Agent, input: TurnInput) !?[]const u8 { const args = browser_tools.normalizeArgKeys(ca, tool, tc.arguments) catch tc.arguments; // Fall back to the navigation a read tool performed, so a // markdown/tree-driven turn isn't lost from `/save`. - // A call that named its element by id is unreplayable as-is; the - // tool layer resolved a selector for it while the node still - // existed. const replayable = withSelector(ca, args, if (i < self.save_selectors.items.len) self.save_selectors.items[i] else null); const cmd = Command.fromToolCall(tool, replayable); const to_record = if (cmd.isRecorded()) @@ -1965,8 +1954,10 @@ fn handleToolCall(ctx: *anyopaque, allocator: std.mem.Allocator, tool_name: []co }; if (self.capturing_for_save) { // One entry per call, errors included, so the index lines up with - // `RunToolsResult.tool_calls_made`. - const kept = if (selector) |sel| self.allocator.dupe(u8, sel) catch null else null; + // `RunToolsResult.tool_calls_made`. The conversation arena outlives the + // turn that reads them; `allocator` here is zenai's per-call arena. + const ca = self.conversation.arena.allocator(); + const kept = if (selector) |sel| ca.dupe(u8, sel) catch null else null; self.save_selectors.append(self.allocator, kept) catch {}; } diff --git a/src/browser/tools.zig b/src/browser/tools.zig index 5b597dfdc..9ea7f983f 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -825,9 +825,8 @@ pub const ToolResult = struct { is_error: bool = false, /// Only set when the caller passed `CallOpts.inline_image`. image: ?lp.screenshot.Prepared = null, - /// Only set when the caller passed `CallOpts.record` and the call named its - /// element by `backendNodeId`. Resolved before the action runs, because a - /// navigation takes the node with it. + /// Resolved before the action runs, because a navigation takes the node + /// with it. selector: ?[]const u8 = null, }; @@ -860,10 +859,8 @@ const NodeAndPage = struct { node: *DOMNode, page: *lp.Frame, target: ActionTarg pub const CallOpts = struct { /// The caller can hand an image to a model. inline_image: bool = false, - /// The caller is recording for `--save`/`/save`. A call that addresses its - /// element by `backendNodeId` gets `ToolResult.selector` filled in, since - /// a registry id means nothing in a later session and the node may be gone - /// by the time the caller wants to record it. + /// Fill in `ToolResult.selector`: a registry id means nothing in a later + /// session, so `--save` cannot replay a call that used one. record: bool = false, }; @@ -897,8 +894,14 @@ pub fn call( }; const substituted = try substituteStringArgs(arena, tool, normalized); - // Before dispatch: after a navigation the node is gone. - const selector = if (opts.record) selectorForArgs(arena, session, registry, substituted) else null; + // Before dispatch, because a navigation takes the node with it. Gated on + // `isRecorded` because `SelectorPath.build` is the expensive part of a tool + // call and the read-only tools that take a `backendNodeId` -- tree, + // markdown, html, nodeDetails -- would only have it thrown away. + const selector = if (opts.record and tool.isRecorded()) + selectorForArgs(arena, session, registry, substituted) + else + null; var result = dispatch(arena, session, registry, tool, substituted, opts) catch |err| { if (err == error.NavigationFailed) { @@ -912,8 +915,7 @@ pub fn call( } /// The CSS selector for a call's `backendNodeId`, so the call can be recorded -/// in a form that still resolves in a later session. Null when the arguments -/// name no node, already carry a selector, or the node cannot be named. +/// in a form that still resolves in a later session. fn selectorForArgs( arena: std.mem.Allocator, session: *lp.Session, @@ -1168,13 +1170,24 @@ const KeyStatus = struct { state: enum { set, keyless, missing }, }; -/// `null` for `.auto`, which has no key of its own. +fn keyStatusOf(comptime e: anytype) KeyStatus { + return .{ + .env_var = e.env_var, + .state = if (engineKey(e)) |key| (if (key != null) .set else .keyless) else |_| .missing, + }; +} + +/// For `.auto`, the rung the cascade would actually land on -- it has no key of +/// its own, but "which engine is about to serve, and on what terms" is the +/// question worth answering, and `.auto` is the default. pub fn searchKeyStatus(engine: SearchEngine) ?KeyStatus { inline for (api_engines) |e| { - if (engine == e.tag) return .{ - .env_var = e.env_var, - .state = if (engineKey(e)) |key| (if (key != null) .set else .keyless) else |_| .missing, - }; + if (engine == e.tag) return keyStatusOf(e); + } + if (engine != .auto) return null; + inline for (api_engines) |e| { + const status = keyStatusOf(e); + if (status.state != .missing) return status; } return null; } @@ -1204,8 +1217,7 @@ fn execSearch(arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError!To } } else |_| {} } - // The last rung's reason, not a generic one: every engine having - // failed is usually one cause, and the model can act on it. + // The last engine's reason, not a generic one -- the model can act on it. return searchFailed(arena, last_label, last_err.?, last_detail); }, inline else => |tag| { @@ -1230,34 +1242,33 @@ fn searchExplicit(arena: std.mem.Allocator, comptime engine: anytype, timeout_ms return .{ .text = markdown_ }; } -/// What the provider said, duped out of the client before `deinit` takes it. -/// Without this the status and body survive only in a log line, and every -/// failure reaches the model as the word `InternalError`. +/// Duped out of the client before `deinit` takes it; otherwise the model sees +/// only the error name. const Failure = struct { status: ?u10 = null, message: []const u8 = "", }; -/// A rate limit and a bad key need different reactions, so the model is told -/// which it hit rather than just that the search failed. fn searchFailed(arena: std.mem.Allocator, label: []const u8, err: anyerror, detail: Failure) ToolError!ToolResult { var aw: std.Io.Writer.Allocating = .init(arena); - const w = &aw.writer; - w.print("{s} search failed: {s}", .{ label, @errorName(err) }) catch return ToolError.OutOfMemory; - if (detail.status) |status| w.print(" (HTTP {d})", .{status}) catch return ToolError.OutOfMemory; - if (detail.message.len > 0) { - w.writeAll(": ") catch return ToolError.OutOfMemory; - writeSingleLine(w, detail.message) catch return ToolError.OutOfMemory; - } - if (detail.status == 429) { - w.writeAll(". This engine is rate-limited right now; wait before retrying, or read the answer from a page instead.") catch - return ToolError.OutOfMemory; - } + writeFailure(&aw.writer, label, err, detail) catch return ToolError.OutOfMemory; return .{ .text = aw.written(), .is_error = true }; } -/// `arena` owns the returned slice. `detail` is filled on a non-2xx so the -/// caller can say what actually happened. +fn writeFailure(w: *std.Io.Writer, label: []const u8, err: anyerror, detail: Failure) !void { + try w.print("{s} search failed: {s}", .{ label, @errorName(err) }); + if (detail.status) |status| try w.print(" (HTTP {d})", .{status}); + if (detail.message.len > 0) { + try w.writeAll(": "); + try writeSingleLine(w, detail.message); + } + // The one failure where the right move is not "try another query". + if (detail.status == 429) { + try w.writeAll(". This engine is rate-limited right now; wait before retrying, or read the answer from a page instead."); + } +} + +/// `arena` owns the returned slice. fn apiSearch( comptime engine: anytype, arena: std.mem.Allocator, @@ -1285,7 +1296,6 @@ fn apiSearch( .status = status, .body = client.last_error.body, }); - // `client.last_error` dies with the client on the deferred deinit. detail.* = .{ .status = status, .message = if (client.last_error.body) |b| (arena.dupe(u8, b) catch "") else "", @@ -1310,17 +1320,21 @@ pub const SearchResults = struct { hits: []const Hit = &.{}, }; +/// The engines agree on title and url and disagree only on which field holds +/// the snippet. +fn collectHits(arena: std.mem.Allocator, results: anytype, comptime snippet: []const u8) ![]Hit { + const hits = try arena.alloc(Hit, results.len); + for (results, hits) |r, *hit| hit.* = .{ .title = r.title, .url = r.url, .snippet = @field(r, snippet) }; + return hits; +} + fn collectTavily(arena: std.mem.Allocator, resp: tavily.types.SearchResponse) !SearchResults { - const hits = try arena.alloc(Hit, resp.results.len); - for (resp.results, hits) |r, *hit| hit.* = .{ .title = r.title, .url = r.url, .snippet = r.content }; - return .{ .answer = resp.answer orelse "", .hits = hits }; + return .{ .answer = resp.answer orelse "", .hits = try collectHits(arena, resp.results, "content") }; } fn collectBrave(arena: std.mem.Allocator, resp: brave.types.SearchResponse) !SearchResults { const results: []const brave.types.Result = if (resp.web) |web| web.results else &.{}; - const hits = try arena.alloc(Hit, results.len); - for (results, hits) |r, *hit| hit.* = .{ .title = r.title, .url = r.url, .snippet = r.description }; - return .{ .hits = hits }; + return .{ .hits = try collectHits(arena, results, "description") }; } fn collectExa(arena: std.mem.Allocator, resp: exa.types.SearchResponse) !SearchResults { @@ -1337,22 +1351,19 @@ fn collectExa(arena: std.mem.Allocator, resp: exa.types.SearchResponse) !SearchR } fn collectKeenable(arena: std.mem.Allocator, resp: keenable.types.SearchResponse) !SearchResults { - const hits = try arena.alloc(Hit, resp.results.len); - // snippet carries the page text (the wire format's always-empty - // `description` is deliberately not even mapped by the client). - for (resp.results, hits) |r, *hit| hit.* = .{ .title = r.title, .url = r.url, .snippet = r.snippet }; - return .{ .hits = hits }; + // `snippet` carries the page text; the wire format's always-empty + // `description` is deliberately not even mapped by the client. + return .{ .hits = try collectHits(arena, resp.results, "snippet") }; } fn renderResults(arena: std.mem.Allocator, results: SearchResults) ToolError![]const u8 { if (results.answer.len == 0 and results.hits.len == 0) return "No results."; var aw: std.Io.Writer.Allocating = .init(arena); - const w = &aw.writer; - renderInto(w, results) catch return ToolError.OutOfMemory; + writeResults(&aw.writer, results) catch return ToolError.OutOfMemory; return aw.written(); } -fn renderInto(w: *std.Io.Writer, results: SearchResults) !void { +fn writeResults(w: *std.Io.Writer, results: SearchResults) !void { if (results.answer.len > 0) try w.print("**Answer:** {s}\n\n", .{results.answer}); for (results.hits, 0..) |hit, i| try writeResultItem(w, i, hit.title, hit.url, hit.snippet); } @@ -2828,7 +2839,7 @@ test "formatLpEnvNames reports empty when no names" { try std.testing.expectEqualStrings("No LP_* environment variables are set.", r); } -test "collectTavily renders answer and results" { +test "tavily results render as markdown" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2850,7 +2861,7 @@ test "collectTavily renders answer and results" { try std.testing.expect(std.mem.indexOf(u8, md, "2. **France**") != null); } -test "collectTavily handles empty results" { +test "tavily: no results render as a notice" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2858,7 +2869,7 @@ test "collectTavily handles empty results" { try std.testing.expectEqualStrings("No results.", try renderResults(aa, try collectTavily(aa, .{}))); } -test "collectBrave renders web results" { +test "brave results render as markdown" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2879,7 +2890,7 @@ test "collectBrave renders web results" { try std.testing.expect(std.mem.indexOf(u8, md, "2. **France**") != null); } -test "collectBrave handles empty results" { +test "brave: no results render as a notice" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2889,7 +2900,7 @@ test "collectBrave handles empty results" { try std.testing.expectEqualStrings("No results.", try renderResults(aa, try collectBrave(aa, .{ .web = .{} }))); } -test "collectKeenable reads snippet" { +test "keenable results render the snippet as the body" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2917,14 +2928,14 @@ test "writeResultItem uses the URL as title when the title is empty" { try std.testing.expectEqualStrings("1. **https://example.org/x.pdf** — https://example.org/x.pdf\n snippet\n\n", aw.written()); } -test "collectKeenable handles empty results" { +test "keenable: no results render as a notice" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); try std.testing.expectEqualStrings("No results.", try renderResults(aa, try collectKeenable(aa, .{}))); } -test "collectBrave flattens newlines in titles and descriptions" { +test "brave titles and descriptions render on one line" { var arena: std.heap.ArenaAllocator = .init(std.testing.allocator); defer arena.deinit(); const aa = arena.allocator(); @@ -2953,12 +2964,9 @@ test "searchFailed: a rate limit says so, a bare failure stays short" { .message = "Public API hourly limit reached.\nWait 2 minutes to continue.", }); try std.testing.expect(limited.is_error); - // The status and the provider's own words, which previously reached the - // model only as the word "ApiError". try std.testing.expect(std.mem.indexOf(u8, limited.text, "(HTTP 429)") != null); try std.testing.expect(std.mem.indexOf(u8, limited.text, "Public API hourly limit reached.") != null); try std.testing.expect(std.mem.indexOf(u8, limited.text, "rate-limited right now") != null); - // Flattened: a newline would break the numbered-list markdown around it. try std.testing.expect(std.mem.indexOf(u8, limited.text, "\n") == null); const bare = try searchFailed(aa, "web", error.ConnectionRefused, .{}); From 2a9abd880f9d5a73f3317a83df2c22599b21a44f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Tue, 22 Sep 2026 16:14:23 +0200 Subject: [PATCH 10/16] deps: bump zenai for the reasoning-token floor Picks up the per-effort `max_tokens` floor, so a tight budget no longer buys mostly thinking and a truncated answer, and the search clients logging a non-2xx like the model clients already did. The bump also carries zenai's `ErrorDetail.body` -> `.message` rename, which the search failure path reads. --- build.zig.zon | 4 ++-- src/browser/tools.zig | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/build.zig.zon b/build.zig.zon index e78035c35..06793a486 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -36,8 +36,8 @@ .hash = "sqlite3-3.53.2-DMxLWuAOAAA_Px0arJOIOaP4AKEu5prbsQgPMA35W1zz", }, .zenai = .{ - .url = "git+https://github.com/lightpanda-io/zenai.git#15d6e4c37b4508373ba7ae6a2f400717f4ff9d89", - .hash = "zenai-0.0.0-iOY_VLq7BgAZIQzUIZ4w8ikmwpuNaJdyJSvKYCtnVing", + .url = "git+https://github.com/lightpanda-io/zenai.git#93583f32eb09203821539ca16bd45890823f293b", + .hash = "zenai-0.0.0-iOY_VOKsBgDYtAH8xVzBf_1BITeTJ-rZlB4stdJDPiWX", }, .isocline = .{ .url = "git+https://github.com/arrufat/isocline#ec538faf435c616a6b38716f53980b5815c30f8a", diff --git a/src/browser/tools.zig b/src/browser/tools.zig index 9ea7f983f..5c186dc5c 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -1294,11 +1294,11 @@ fn apiSearch( if (client.last_error.status) |status| { log.warn(.browser, @tagName(engine.tag) ++ " non-2xx", .{ .status = status, - .body = client.last_error.body, + .message = client.last_error.message, }); detail.* = .{ .status = status, - .message = if (client.last_error.body) |b| (arena.dupe(u8, b) catch "") else "", + .message = if (client.last_error.message) |m| (arena.dupe(u8, m) catch "") else "", }; } return err; From 3514d8d46b825d404591d48d0bc6f504c64c30ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Wed, 23 Sep 2026 10:59:25 +0200 Subject: [PATCH 11/16] deps: bump isocline for the strdup_from_utf8 overflow isocline copied the submitted line into a buffer one byte short whenever the locale is not UTF-8. A line whose length the allocator serves exactly, such as the 24-byte `/screenshot path=out.png`, corrupted the heap on Enter. --- build.zig.zon | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/build.zig.zon b/build.zig.zon index 06793a486..851080c09 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -40,8 +40,8 @@ .hash = "zenai-0.0.0-iOY_VOKsBgDYtAH8xVzBf_1BITeTJ-rZlB4stdJDPiWX", }, .isocline = .{ - .url = "git+https://github.com/arrufat/isocline#ec538faf435c616a6b38716f53980b5815c30f8a", - .hash = "N-V-__8AAHhtEwBIqx5nOoiGo_FLAG8gpiVC6XzZn1teMKd0", + .url = "git+https://github.com/arrufat/isocline#4a99434bee4a5ed04c1639514224d48cd55e1405", + .hash = "N-V-__8AAHxtEwB16xj2Xz-zx_uklGdTP5C2-GHXDoSzah-8", }, .pcre2 = .{ .url = "https://github.com/PCRE2Project/pcre2/releases/download/pcre2-10.48/pcre2-10.48.tar.gz", From 3ab936ebbbd952fec71676ef4ca5e165d550e5ea Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Wed, 23 Sep 2026 11:00:16 +0200 Subject: [PATCH 12/16] agent: report a --url that fails to load `call` reports a failed navigation in-band, as an is_error result, so the catch in gotoStart never saw it: a start page that could not resolve left the agent on a blank page with no message. --- src/agent/Agent.zig | 22 +++++++++++++++------- 1 file changed, 15 insertions(+), 7 deletions(-) diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 33cfbc88e..90c751893 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -516,10 +516,7 @@ const TurnInput = struct { /// Returns true on success. pub fn run(self: *Agent) bool { if (self.start_url) |url| { - self.gotoStart(url, self.one_shot_save != null) catch |err| { - self.terminal.printError("could not open {s}: {s}", .{ url, browser_tools.errorMessage(err) }); - return false; - }; + if (!self.gotoStart(url, self.one_shot_save != null)) return false; } if (self.one_shot_task) |task| { const saving = self.one_shot_save != null; @@ -543,16 +540,27 @@ pub fn run(self: *Agent) bool { return true; } -fn gotoStart(self: *Agent, url: [:0]const u8, record: bool) browser_tools.ToolError!void { +/// Opens `--url` through the tool layer, so a bad URL fails like any other +/// tool call. +fn gotoStart(self: *Agent, url: [:0]const u8, record: bool) bool { var arena: std.heap.ArenaAllocator = .init(self.allocator); defer arena.deinit(); const a = arena.allocator(); var object: std.json.ObjectMap = .empty; - try object.put(a, "url", .{ .string = url }); + object.put(a, "url", .{ .string = url }) catch return false; const args: std.json.Value = .{ .object = object }; - _ = try browser_tools.call(a, self.ts.session, &self.ts.registry, "goto", args, .{}); + const result = browser_tools.call(a, self.ts.session, &self.ts.registry, "goto", args, .{}) catch |err| { + self.terminal.printError("could not open {s}: {s}", .{ url, browser_tools.errorMessage(err) }); + return false; + }; + // `call` reports a failed navigation in-band, not as an error. + if (result.is_error) { + self.terminal.printError("could not open {s}: {s}", .{ url, result.text }); + return false; + } if (record) self.recordSaveCommand(Command.fromToolCall(.goto, args)); + return true; } /// Print single-line cumulative token usage to stderr, so wrappers driving From 7289281784a54876c52d9be79d2bbdacf55c9581 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Wed, 23 Sep 2026 11:00:36 +0200 Subject: [PATCH 13/16] agent: record the --url navigation for a REPL /save too It was recorded only for a one-shot --save, so a REPL session opened with --url saved a script that started on a blank page. --- src/agent/Agent.zig | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 90c751893..7339b6031 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -516,7 +516,7 @@ const TurnInput = struct { /// Returns true on success. pub fn run(self: *Agent) bool { if (self.start_url) |url| { - if (!self.gotoStart(url, self.one_shot_save != null)) return false; + if (!self.gotoStart(url)) return false; } if (self.one_shot_task) |task| { const saving = self.one_shot_save != null; @@ -541,8 +541,8 @@ pub fn run(self: *Agent) bool { } /// Opens `--url` through the tool layer, so a bad URL fails like any other -/// tool call. -fn gotoStart(self: *Agent, url: [:0]const u8, record: bool) bool { +/// tool call and `/save` replays the opening navigation. +fn gotoStart(self: *Agent, url: [:0]const u8) bool { var arena: std.heap.ArenaAllocator = .init(self.allocator); defer arena.deinit(); const a = arena.allocator(); @@ -559,7 +559,7 @@ fn gotoStart(self: *Agent, url: [:0]const u8, record: bool) bool { self.terminal.printError("could not open {s}: {s}", .{ url, result.text }); return false; } - if (record) self.recordSaveCommand(Command.fromToolCall(.goto, args)); + self.recordSaveCommand(Command.fromToolCall(.goto, args)); return true; } From a099fb580b1c1268fe3f95cba3f524d498aeb9f1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Wed, 23 Sep 2026 11:03:25 +0200 Subject: [PATCH 14/16] tools: let scroll target an element by selector Recording swaps a call's backendNodeId for the selector the tool layer resolved, but scroll had no selector parameter: the replayed scroll({ selector, y }) dropped the field and scrolled the window. Take a selector as click and hover do. --- src/browser/tools.zig | 19 ++++++++++++------- src/mcp/tools.zig | 12 ++++++++++++ 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/browser/tools.zig b/src/browser/tools.zig index 5c186dc5c..0b27768db 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -562,13 +562,14 @@ pub const Tool = enum { ), }, .scroll => .{ - .description = "Scroll the page or a specific element. Returns the scroll position and current page URL and title.", + .description = "Scroll the page or a specific element. Provide a CSS selector (preferred for reproducibility) or a backendNodeId to scroll an element; omit both to scroll the window. Returns the scroll position and current page URL and title.", .summary = "Scroll the page or an element", .input_schema = minify( \\{ \\ "type": "object", \\ "properties": { - \\ "backendNodeId": { "type": "integer", "description": "Optional: The backend node ID of the element to scroll. If the element is not itself a scroll container, its nearest scrollable ancestor is scrolled instead. If omitted (or 0), scrolls the window." }, + \\ "selector": { "type": "string", "description": "Optional: CSS selector of the element to scroll. Preferred over backendNodeId. If the element is not itself a scroll container, its nearest scrollable ancestor is scrolled instead." }, + \\ "backendNodeId": { "type": "integer", "description": "Optional: The backend node ID of the element to scroll. If the element is not itself a scroll container, its nearest scrollable ancestor is scrolled instead. If neither this nor selector is given (or it is 0), scrolls the window." }, \\ "x": { "type": "integer", "description": "Optional: The horizontal scroll offset." }, \\ "y": { "type": "integer", "description": "Optional: The vertical scroll offset." } \\ } @@ -1964,20 +1965,24 @@ fn execFill(arena: std.mem.Allocator, session: *lp.Session, registry: *NodeRegis fn execScroll(arena: std.mem.Allocator, session: *lp.Session, registry: *NodeRegistry, arguments: ?std.json.Value) ToolError![]const u8 { const Params = struct { backendNodeId: ?NodeRegistry.Id = null, + selector: ?[]const u8 = null, x: ?i32 = null, y: ?i32 = null, }; const args = try parseArgsOrDefault(Params, arena, arguments); const scope = beginAction(session); - const page = try requireFrame(session); - const target_node = try resolveOptionalNode(registry, args.backendNodeId); + const resolved: ?NodeAndPage = if (args.selector != null or args.backendNodeId != null) + try resolveTarget(session, registry, args.selector, args.backendNodeId) + else + null; + const page = if (resolved) |r| r.page else try requireFrame(session); - const result = lp.actions.scroll(target_node, args.x, args.y, page) catch |err| return mapActionError(err); + const result = lp.actions.scroll(if (resolved) |r| r.node else null, args.x, args.y, page) catch |err| return mapActionError(err); const body = (switch (result.target) { .window => std.fmt.allocPrint(arena, "Scrolled window to x: {d}, y: {d}", .{ result.x, result.y }), .node => std.fmt.allocPrint(arena, "Scrolled element ({f}) to x: {d}, y: {d}", .{ - ActionTarget{ .backend_node_id = args.backendNodeId.? }, + resolved.?.target, result.x, result.y, }), @@ -1985,7 +1990,7 @@ fn execScroll(arena: std.mem.Allocator, session: *lp.Session, registry: *NodeReg const registered = registry.register(container) catch return ToolError.InternalError; break :blk std.fmt.allocPrint(arena, "Scrolled scroll container ({f}) of element ({f}) to x: {d}, y: {d}", .{ ActionTarget{ .backend_node_id = registered.id }, - ActionTarget{ .backend_node_id = args.backendNodeId.? }, + resolved.?.target, result.x, result.y, }); diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index f965b02f4..c6edfe97e 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -1196,6 +1196,18 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked out.clearRetainingCapacity(); } + // A selector targets the element as a backendNodeId does. + { + const outer = frame.document.getElementById("outerscroll", frame).?.asNode(); + const outer_id = (try server.active_session.registry.register(outer)).id; + try router.handleMessage(server, aa, + \\{"jsonrpc":"2.0","id":41,"method":"tools/call","params":{"name":"scroll","arguments":{"selector":"#innerleaf","y":30}}} + ); + const expected = try std.fmt.allocPrint(aa, "Scrolled scroll container (backendNodeId: {d}) of element (selector: #innerleaf) to x: 0, y: 30", .{outer_id}); + try testing.expect(std.mem.indexOf(u8, out.written(), expected) != null); + out.clearRetainingCapacity(); + } + // The container may be declared in a stylesheet rather than inline. { const leaf = frame.document.getElementById("sheetleaf", frame).?.asNode(); From 1df5838bdb63354cf3c67ad46b60840360807794 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Wed, 23 Sep 2026 11:04:22 +0200 Subject: [PATCH 15/16] agent: record slash commands that address a node by id /save kept a model's backendNodeId call by swapping in the selector the tool layer resolved, but a typed `/scroll backendNodeId=3` never asked for one and was dropped from the script. Ask for it on this path too. --- src/agent/Agent.zig | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 7339b6031..4596c8fd0 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -713,7 +713,8 @@ fn runRepl(self: *Agent) void { self.terminal.endTool(); self.printCommandResult(tc, result); if (!result.is_error) { - self.recordSaveCommand(navigationGoto(aa, tc.tool, tc.args) orelse cmd); + const replayable = Command.fromToolCall(tc.tool, withSelector(aa, tc.args, result.selector)); + self.recordSaveCommand(navigationGoto(aa, tc.tool, tc.args) orelse replayable); } self.recordSlashToolCall(command_text, tc.name(), tc.args, result) catch |err| { self.terminal.printWarning("LLM conversation out of sync (/{s}: {s}); next prompt may not see this action", .{ tc.name(), @errorName(err) }); @@ -1506,7 +1507,7 @@ fn printSlashHelp(self: *Agent, arena: std.mem.Allocator, target: []const u8) vo fn runCommand(self: *Agent, arena: std.mem.Allocator, tc: Command.ToolCall) browser_tools.ToolResult { // The terminal can't show an image, but the conversation can. - return browser_tools.call(arena, self.ts.session, &self.ts.registry, tc.name(), tc.args, .{ .inline_image = self.ai_client != null }) catch |err| .{ + return browser_tools.call(arena, self.ts.session, &self.ts.registry, tc.name(), tc.args, .{ .inline_image = self.ai_client != null, .record = true }) catch |err| .{ .text = switch (err) { error.OutOfMemory => "out of memory", error.FrameNotLoaded => "no page loaded — run /goto first", From 9c583f1fb09a4eeaf4c026c3610ceefebd5af76d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Wed, 23 Sep 2026 11:18:44 +0200 Subject: [PATCH 16/16] js: hand --locale to ICU instead of LC_ALL A BCP 47 tag in LC_ALL is not a POSIX locale, so setlocale(LC_ALL, "") failed for the rest of the process and for any child. In the REPL, isocline took the terminal for non-UTF-8 and dropped every non-ASCII keystroke. Set ICU's default locale through the new v8__V8__SetDefaultLocale binding and leave the C library alone. Pins zig-v8-fork to lightpanda-io/zig-v8-fork#209; CI links once that is tagged and action.yml's zig-v8 is bumped. --- build.zig.zon | 4 ++-- src/browser/js/Platform.zig | 16 +++++++++------- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/build.zig.zon b/build.zig.zon index 851080c09..127a4edb9 100644 --- a/build.zig.zon +++ b/build.zig.zon @@ -5,8 +5,8 @@ .minimum_zig_version = "0.16.0", .dependencies = .{ .v8 = .{ - .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/d3d7b41677a0015fdfa55a8b1caa4f214de6d209.tar.gz", - .hash = "v8-0.0.0-xddH624yAwC5_H_8T303uTxiSAnAu2zrxv6MHLhvLo6t", + .url = "https://github.com/lightpanda-io/zig-v8-fork/archive/0cc0b28d18f021c560d6f84b7e27e5a7ade9f8c8.tar.gz", + .hash = "v8-0.0.0-xddH6_g0AwBnqBwDOD22Mks8ODHFsH6SffwcpFoUSq3X", }, // .v8 = .{ .path = "../zig-v8-fork" }, .brotli = .{ diff --git a/src/browser/js/Platform.zig b/src/browser/js/Platform.zig index ca58de079..5525b848e 100644 --- a/src/browser/js/Platform.zig +++ b/src/browser/js/Platform.zig @@ -30,18 +30,17 @@ pub const Options = struct { timezone: ?[:0]const u8 = null, }; -/// ICU reads LC_ALL and TZ lazily on first use, so the environment must be -/// set here, before InitializeICU and before the platform starts its thread -/// pool (setenv is not safe once other threads may call getenv). ICU -/// canonicalizes a BCP 47 tag itself, script subtag included. +/// ICU reads TZ lazily on first use, so it must be set here, before +/// InitializeICU and before the platform starts its thread pool (setenv is not +/// safe once other threads may call getenv). The locale goes to ICU directly: +/// a BCP 47 tag in LC_ALL is not a POSIX locale, so it broke setlocale for the +/// rest of the process and for every child. ICU canonicalizes the tag itself, +/// script subtag included. pub fn init(opts: Options) !Platform { if (opts.v8_flags) |flags| { v8.v8__V8__SetFlagsFromString(flags.ptr, flags.len); } - if (opts.locale) |tag| { - _ = setenv("LC_ALL", tag, 1); - } if (opts.timezone) |id| { _ = setenv("TZ", id, 1); } @@ -49,6 +48,9 @@ pub fn init(opts: Options) !Platform { if (v8.v8__V8__InitializeICU() == false) { return error.FailedToInitializeICU; } + if (opts.locale) |tag| { + if (!v8.v8__V8__SetDefaultLocale(tag)) return error.InvalidLocale; + } // 0 - threadpool size, 0 == let v8 decide // 1 - idle_task_support, 1 == enabled const handle = v8.v8__Platform__NewDefaultPlatform(0, 1).?;