Merge pull request #3597 from lightpanda-io/agent-fixes

Agent and tool-layer fixes found by a decision-model loop

build.zig.zon

@@ -36,12 +36,12 @@
             .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",
-            .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",

src/Config.zig

@@ -349,6 +349,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 {
@@ -472,6 +473,8 @@ 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 = "url", .type = ?[:0]const u8 },
             .{ .name = "list_models", .type = bool },
             .{ .name = "no_llm", .type = bool },
         },

src/NodeRegistry.zig

@@ -190,3 +190,20 @@ 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));
+    try testing.expect((try registry.register(dom_node)).id != first_id);
+}

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.
+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, &visitor) catch |err| {
-        log.err(.app, "semantic tree json dump failed", .{ .err = err });
+    self.walk(&ctx, visitor) 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, &visitor) catch |err| {
-        log.err(.app, "semantic tree text dump failed", .{ .err = err });
-        return error.WriteFailed;
-    };
+    return self.visitAll(&visitor);
 }
 
 const OptionData = struct {

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, 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,
 one_shot_attachments: ?[]const []const u8,
@@ -167,6 +170,10 @@ 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`.
+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.
@@ -284,7 +291,14 @@ 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);
+    // 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" }),
+        .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) {
@@ -320,6 +334,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,
@@ -355,6 +370,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();
@@ -499,6 +515,9 @@ const TurnInput = struct {
 
 /// Returns true on success.
 pub fn run(self: *Agent) bool {
+    if (self.start_url) |url| {
+        if (!self.gotoStart(url)) return false;
+    }
     if (self.one_shot_task) |task| {
         const saving = self.one_shot_save != null;
         const ok = self.runTurn(.{
@@ -521,6 +540,29 @@ pub fn run(self: *Agent) bool {
     return true;
 }
 
+/// Opens `--url` through the tool layer, so a bad URL fails like any other
+/// 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();
+
+    var object: std.json.ObjectMap = .empty;
+    object.put(a, "url", .{ .string = url }) catch return false;
+    const args: std.json.Value = .{ .object = object };
+    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;
+    }
+    self.recordSaveCommand(Command.fromToolCall(.goto, args));
+    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:
@@ -671,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) });
@@ -1271,6 +1314,12 @@ fn synthesizeSaveTo(self: *Agent, arena: std.mem.Allocator, path: []const u8, mo
         return;
     }
 
+    // `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");
+    }
+
     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
@@ -1328,6 +1377,20 @@ 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.
+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);
 }
@@ -1444,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 <url> first",
@@ -1671,6 +1734,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 +1798,12 @@ 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);
+            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 +1956,19 @@ 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`. 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 {};
+    }
 
     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 +1976,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,

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;

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).?;

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;
@@ -560,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." }
                     \\  }
@@ -823,6 +826,9 @@ pub const ToolResult = struct {
     is_error: bool = false,
     /// Only set when the caller passed `CallOpts.inline_image`.
     image: ?lp.screenshot.Prepared = null,
+    /// Resolved before the action runs, because a navigation takes the node
+    /// with it.
+    selector: ?[]const u8 = null,
 };
 
 const GotoParams = struct {
@@ -854,6 +860,9 @@ 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,
+    /// 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,
 };
 
 // An inline screenshot is re-sent on every turn; keep it within what models
@@ -886,13 +895,44 @@ pub fn call(
     };
     const substituted = try substituteStringArgs(arena, tool, normalized);
 
-    return dispatch(arena, session, registry, tool, substituted, opts) catch |err| {
+    // 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) {
             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.
+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(
@@ -1055,7 +1095,7 @@ const api_engines = .{
         .init_options = brave.Client.InitOptions{},
         // text_decorations=false: no <strong> markup in model-read snippets.
         .options = brave.types.SearchOptions{ .count = 10, .text_decorations = false },
-        .format = formatBraveMarkdown,
+        .collect = collectBrave,
     },
     .{
         .tag = SearchEngine.tavily,
@@ -1063,7 +1103,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,
@@ -1073,7 +1113,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,
@@ -1083,7 +1123,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,
     },
 };
 
@@ -1131,13 +1171,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;
 }
@@ -1150,19 +1201,25 @@ 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 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| {
             inline for (api_engines) |engine| {
@@ -1180,16 +1237,36 @@ 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,
-    };
+/// 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 = "",
+};
+
+fn searchFailed(arena: std.mem.Allocator, label: []const u8, err: anyerror, detail: Failure) ToolError!ToolResult {
+    var aw: std.Io.Writer.Allocating = .init(arena);
+    writeFailure(&aw.writer, label, err, detail) catch return ToolError.OutOfMemory;
+    return .{ .text = aw.written(), .is_error = true };
+}
+
+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.
@@ -1199,6 +1276,7 @@ fn apiSearch(
     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
@@ -1217,61 +1295,78 @@ 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.message) |m| (arena.dupe(u8, m) catch "") else "",
+            };
         }
         return err;
     };
     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 = &.{},
+};
+
+/// 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 {
+    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 &.{};
+    return .{ .hits = try collectHits(arena, results, "description") };
+}
+
+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 {
+    // `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);
-    try engine.format(&aw.writer, response.value);
+    writeResults(&aw.writer, 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 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);
 }
 
 /// An empty title (providers default it to "") would render as `****`.
@@ -1870,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,
         }),
@@ -1891,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,
             });
@@ -2745,7 +2844,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 "tavily results render as markdown" {
     var arena: std.heap.ArenaAllocator = .init(std.testing.allocator);
     defer arena.deinit();
     const aa = arena.allocator();
@@ -2759,9 +2858,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);
@@ -2769,17 +2866,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 "tavily: no results render as a notice" {
     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 "brave results render as markdown" {
     var arena: std.heap.ArenaAllocator = .init(std.testing.allocator);
     defer arena.deinit();
     const aa = arena.allocator();
@@ -2793,30 +2888,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 "brave: no results render as a notice" {
     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 "keenable results render the snippet as the body" {
     var arena: std.heap.ArenaAllocator = .init(std.testing.allocator);
     defer arena.deinit();
     const aa = arena.allocator();
@@ -2829,9 +2918,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);
@@ -2846,15 +2933,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 "keenable: no results render as a notice" {
     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 "brave titles and descriptions render on one line" {
     var arena: std.heap.ArenaAllocator = .init(std.testing.allocator);
     defer arena.deinit();
     const aa = arena.allocator();
@@ -2867,9 +2953,30 @@ 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 "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);
+    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);
+    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" {

src/help.zon

@@ -297,12 +297,22 @@
     \\          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 <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 <STRING>
     \\          Override the default system prompt.
     \\  --task <STRING>
     \\           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 <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 <LEVEL>
     \\          Stderr chatter level. Default: high when --task captures stderr
     \\          to a pipe or file; low otherwise. low/medium also raise

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();