From 90487270ba9d4fd852d50a304ad106ecd4e7977a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Wed, 16 Sep 2026 09:38:42 +0200 Subject: [PATCH] `findElement`: the name filter is one union; a literal is one by grammar A name is a substring or a regex, never both, so the filter says so instead of carrying two optionals the caller has to null against each other. Whether `/.../x` is a literal is now decided by JavaScript's flag alphabet rather than "looks like letters", which stops `/usr/bin` from being read as a pattern with flags. Comments that narrated callers or the type name are gone; the literal parser gets its own test in place of two MCP round-trips. --- src/App.zig | 4 +- src/Regex.zig | 18 +++----- src/browser/interactive.zig | 27 ++++++----- src/browser/tools.zig | 90 +++++++++++++++++++++++-------------- src/mcp/tools.zig | 21 +-------- src/script/skill.zig | 2 +- 6 files changed, 81 insertions(+), 81 deletions(-) diff --git a/src/App.zig b/src/App.zig index 798bd4b43..3d573294b 100644 --- a/src/App.zig +++ b/src/App.zig @@ -44,8 +44,6 @@ allocator: Allocator, arena_pool: ArenaPool, app_dir_path: ?[]const u8, -// Compiles every pattern the process runs: adblock lists, tool arguments. -// Heap-held because PCRE2 keeps its address. regex_context: *Regex.Context, pub fn init(allocator: Allocator, config: *const Config) !*App { @@ -105,7 +103,7 @@ pub fn deinit(self: *App) void { } self.telemetry.deinit(allocator); self.network.deinit(); - // Compiled patterns free through the context, so it goes after them. + // After `network`: its adblock regexes free through this context. self.regex_context.deinit(); self.snapshot.deinit(); self.platform.deinit(); diff --git a/src/Regex.zig b/src/Regex.zig index 96c6d1eca..a5c567f60 100644 --- a/src/Regex.zig +++ b/src/Regex.zig @@ -16,9 +16,8 @@ // You should have received a copy of the GNU Affero General Public License // along with this program. If not, see . -//! A compiled pattern in JavaScript `RegExp` syntax, run by PCRE2. Adblock -//! lists and agent tool arguments both write regexes that way, and PCRE2 -//! reads the syntax as-is, escapes like `\/` included. +//! A pattern in JavaScript `RegExp` syntax, run by PCRE2, which reads that +//! syntax as-is, escapes like `\/` included. //! //! A compiled pattern and its `Context` are never modified after `compile`, //! so one `Regex` can be shared by every thread; the per-call match data is @@ -42,13 +41,12 @@ pub const Options = struct { /// works beyond ASCII. An invalid sequence in the subject fails to match /// rather than erroring. `\b` and `\w` stay ASCII, as in JavaScript. unicode: bool = false, - /// JavaScript's `s`: `.` also matches a newline. + /// JavaScript's `s`. dot_all: bool = false, - /// JavaScript's `m`: `^` and `$` also match around newlines. + /// JavaScript's `m`. multiline: bool = false, }; -/// Why a compile failed, for the caller to log or show. pub const Diagnostic = struct { offset: usize = 0, len: usize = 0, @@ -59,8 +57,7 @@ pub const Diagnostic = struct { } }; -/// What every `Regex` compiled through it shares: the allocator PCRE2 draws -/// from, and the compile and match settings. Outlives the regexes. +/// Shared by every `Regex` compiled through it; outlives them. /// /// PCRE2 would happily use libc's malloc; it is handed the owner's allocator /// so that a compiled pattern nobody freed fails a test the way any other @@ -90,8 +87,7 @@ pub const Context = struct { const compile_context = pcre2.pcre2_compile_context_create_8(general) orelse return error.OutOfMemory; errdefer pcre2.pcre2_compile_context_free_8(compile_context); - // JavaScript without the `u` flag reads an unknown escape as the - // literal character. + // JavaScript reads an unknown escape as the literal character. _ = pcre2.pcre2_set_compile_extra_options_8(compile_context, pcre2.PCRE2_EXTRA_BAD_ESCAPE_IS_LITERAL); const match_context = pcre2.pcre2_match_context_create_8(general) orelse return error.OutOfMemory; @@ -175,8 +171,6 @@ const MATCH_SCRATCH = 24 * 1024; /// Whether the pattern matches anywhere in `text`, as `RegExp.test` would /// answer. A match that hits the backtracking limits counts as no match. pub fn matches(self: Regex, text: []const u8) bool { - // This can run per request or per DOM node; the scratch keeps the common - // case off the heap. var scratch = std.heap.stackFallback(MATCH_SCRATCH, self.context.allocator); var allocator = scratch.get(); const general = pcre2.pcre2_general_context_create_8(Context.cMalloc, Context.cFree, &allocator) orelse return false; diff --git a/src/browser/interactive.zig b/src/browser/interactive.zig index d6ceabf04..a25a3941e 100644 --- a/src/browser/interactive.zig +++ b/src/browser/interactive.zig @@ -150,13 +150,18 @@ pub fn collectInteractiveElements( return walkInteractive(root, arena, frame, .{}); } +pub const Name = union(enum) { + /// Case-insensitive. + substring: []const u8, + /// Unanchored. + regex: Regex, +}; + const FindFilter = struct { /// Exact role match (case-insensitive). When null, role is not filtered. role: ?[]const u8 = null, - /// Accessible-name substring match (case-insensitive). When null, name is not filtered. - name: ?[]const u8 = null, - /// Compiled pattern searched in the accessible name. - name_regex: ?Regex = null, + /// Accessible-name match. When null, name is not filtered. + name: ?Name = null, /// Stop walking once this many matches accumulate. When null, walks the full subtree. max: ?usize = null, }; @@ -230,11 +235,11 @@ fn walkInteractive( if (role == null) try getTextContent(node, arena) else null; if (filter.name) |nf| { const n = name orelse continue; - if (std.ascii.indexOfIgnoreCase(n, nf) == null) continue; - } - if (filter.name_regex) |re| { - const n = name orelse continue; - if (!re.matches(n)) continue; + const hit = switch (nf) { + .substring => |s| std.ascii.indexOfIgnoreCase(n, s) != null, + .regex => |re| re.matches(n), + }; + if (!hit) continue; } const listener_types = getListenerTypes(el.asEventTarget(), listener_targets); @@ -509,14 +514,14 @@ test "browser.interactive: a name regex filters the walk" { const starts_add = try context.compile("^add", options, null); defer starts_add.deinit(); - const found_add = try findInteractiveElements(div.asNode(), frame.call_arena, frame, .{ .name_regex = starts_add }); + const found_add = try findInteractiveElements(div.asNode(), frame.call_arena, frame, .{ .name = .{ .regex = starts_add } }); try testing.expectEqual(2, found_add.len); try testing.expectEqual("Add to cart", found_add[0].name.?); try testing.expectEqual("Add item", found_add[1].name.?); const only_cart = try context.compile("^cart$", options, null); defer only_cart.deinit(); - const found_cart = try findInteractiveElements(div.asNode(), frame.call_arena, frame, .{ .name_regex = only_cart }); + const found_cart = try findInteractiveElements(div.asNode(), frame.call_arena, frame, .{ .name = .{ .regex = only_cart } }); try testing.expectEqual(1, found_cart.len); try testing.expectEqual("Cart", found_cart[0].name.?); } diff --git a/src/browser/tools.zig b/src/browser/tools.zig index eac470790..bed4cbcc2 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -814,8 +814,9 @@ pub fn errorMessage(err: ToolError) []const u8 { /// Outcome of running a tool against the page. Operational failures (OOM, /// missing page, invalid params) come out as Zig errors on the enclosing /// `!ToolResult`; `is_error = true` is the in-band signal for a JS-level -/// failure (V8 caught a throw inside `evaluate`/`extract`) — the LLM consumes -/// `text` either way to self-correct. Non-evaluate tools always set `is_error = +/// failure (V8 caught a throw inside `evaluate`/`extract`) or any failure whose +/// message carries detail the model needs — the LLM consumes `text` either way +/// to self-correct. Non-evaluate tools always set `is_error = /// false` on success. pub const ToolResult = struct { text: []const u8, @@ -2073,33 +2074,40 @@ fn execFindElement(arena: std.mem.Allocator, session: *lp.Session, registry: *No const page = try requireFrame(session); - const literal: ?RegexLiteral = if (args.name) |name| regexLiteral(name) else null; - var diag: lp.Regex.Diagnostic = .{}; - const name_regex: ?lp.Regex = if (literal) |lit| blk: { - var options: lp.Regex.Options = .{ .case_insensitive = true, .unicode = true }; - for (lit.flags) |flag| switch (flag) { - 'i', 'u' => {}, - 's' => options.dot_all = true, - 'm' => options.multiline = true, - else => return .{ - .text = try std.fmt.allocPrint(arena, "findElement: unsupported regex flag '{c}' in '{s}'", .{ flag, args.name.? }), - .is_error = true, - }, - }; - break :blk session.browser.app.regex_context.compile(lit.body, options, &diag) catch |err| switch (err) { - error.OutOfMemory => return error.OutOfMemory, - error.InvalidRegex => return .{ - .text = try std.fmt.allocPrint(arena, "findElement: invalid name regex '{s}': {s} at offset {d}", .{ lit.body, diag.message(), diag.offset }), - .is_error = true, - }, - }; - } else null; - defer if (name_regex) |re| re.deinit(); + var name_filter: ?lp.interactive.Name = null; + defer if (name_filter) |nf| switch (nf) { + .regex => |re| re.deinit(), + .substring => {}, + }; + if (args.name) |name| { + if (regexLiteral(name)) |lit| { + var options: lp.Regex.Options = .{ .case_insensitive = true, .unicode = true }; + for (lit.flags) |flag| switch (flag) { + 'i', 'u' => {}, + 's' => options.dot_all = true, + 'm' => options.multiline = true, + else => return .{ + .text = try std.fmt.allocPrint(arena, "findElement: unsupported regex flag '{c}' in '{s}'", .{ flag, name }), + .is_error = true, + }, + }; + var diag: lp.Regex.Diagnostic = .{}; + const regex = session.browser.app.regex_context.compile(lit.body, options, &diag) catch |err| switch (err) { + error.OutOfMemory => return error.OutOfMemory, + error.InvalidRegex => return .{ + .text = try std.fmt.allocPrint(arena, "findElement: invalid name regex '{s}': {s} at offset {d}", .{ lit.body, diag.message(), diag.offset }), + .is_error = true, + }, + }; + name_filter = .{ .regex = regex }; + } else { + name_filter = .{ .substring = name }; + } + } const matched = lp.interactive.findInteractiveElements(page.document.asNode(), arena, page, .{ .role = args.role, - .name = if (literal == null) args.name else null, - .name_regex = name_regex, + .name = name_filter, }) catch return ToolError.InternalError; lp.interactive.registerNodes(matched, registry) catch @@ -2112,19 +2120,33 @@ const RegexLiteral = struct { flags: []const u8, }; -/// A `/body/flags` literal as JavaScript writes it, and the spelling adblock -/// lists use for a regex too. Anything after the closing slash that is not a -/// letter makes the whole thing plain text again. Matching is unanchored, so -/// a name that really is written as `/foo/` still matches itself. +/// A JavaScript `/body/flags` literal, or null for plain text. A name really +/// written as `/foo/` still matches itself, the search being unanchored. fn regexLiteral(text: []const u8) ?RegexLiteral { - if (text.len < 3 or text[0] != '/') return null; - const close = 1 + (std.mem.lastIndexOfScalar(u8, text[1..], '/') orelse return null); - if (close == 1) return null; + if (text.len == 0 or text[0] != '/') return null; + const close = std.mem.lastIndexOfScalar(u8, text, '/') orelse return null; + if (close < 2) return null; const flags = text[close + 1 ..]; - for (flags) |flag| if (!std.ascii.isLower(flag)) return null; + for (flags) |flag| { + if (std.mem.indexOfScalar(u8, "dgimsuvy", flag) == null) return null; + } return .{ .body = text[1..close], .flags = flags }; } +test "regexLiteral" { + for ([_][]const u8{ "foo", "/", "//", "//i", "/foo", "/foo/ bar", "/usr/bin" }) |text| { + try std.testing.expectEqual(null, regexLiteral(text)); + } + + const plain = regexLiteral("/foo/").?; + try std.testing.expectEqualStrings("foo", plain.body); + try std.testing.expectEqualStrings("", plain.flags); + + const flagged = regexLiteral("/a/b/gi").?; + try std.testing.expectEqualStrings("a/b", flagged.body); + try std.testing.expectEqualStrings("gi", flagged.flags); +} + fn execGetEnv(arena: std.mem.Allocator, arguments: ?std.json.Value) ToolError![]const u8 { const Params = struct { name: ?[]const u8 = null }; const args = try parseArgsOrDefault(Params, arena, arguments); diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index 47caf0aff..4272db40f 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -1414,7 +1414,7 @@ test "MCP - findElement" { { const msg = - \\{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"findElement","arguments":{"name":"/^prevent.*default$/"}}} + \\{"jsonrpc":"2.0","id":5,"method":"tools/call","params":{"name":"findElement","arguments":{"name":"/^PREVENT.*default$/i"}}} ; try router.handleMessage(server, aa, msg); try testing.expect(std.mem.indexOf(u8, out.written(), "Prevent Default") != null); @@ -1432,15 +1432,6 @@ test "MCP - findElement" { out.clearRetainingCapacity(); } - { - const msg = - \\{"jsonrpc":"2.0","id":7,"method":"tools/call","params":{"name":"findElement","arguments":{"name":"/PREVENT.DEFAULT/i"}}} - ; - try router.handleMessage(server, aa, msg); - try testing.expect(std.mem.indexOf(u8, out.written(), "Prevent Default") != null); - out.clearRetainingCapacity(); - } - { const msg = \\{"jsonrpc":"2.0","id":8,"method":"tools/call","params":{"name":"findElement","arguments":{"name":"/prevent/g"}}} @@ -1450,16 +1441,6 @@ test "MCP - findElement" { try testing.expect(std.mem.indexOf(u8, out.written(), "unsupported regex flag 'g' in '/prevent/g'") != null); out.clearRetainingCapacity(); } - - { - // Text after the closing slash that is not a flag is a plain substring. - const msg = - \\{"jsonrpc":"2.0","id":9,"method":"tools/call","params":{"name":"findElement","arguments":{"name":"/prevent/ default"}}} - ; - try router.handleMessage(server, aa, msg); - try testing.expect(std.mem.indexOf(u8, out.written(), "[]") != null); - out.clearRetainingCapacity(); - } } test "MCP - waitForSelector: existing element" { diff --git a/src/script/skill.zig b/src/script/skill.zig index 6aadce0d4..8de0e5329 100644 --- a/src/script/skill.zig +++ b/src/script/skill.zig @@ -203,7 +203,7 @@ fn note(tool: browser_tools.Tool) []const u8 { .screenshot => "`path` is required: writes a PNG of the text layout.", .press => "Selector first! `page.press(\"Enter\")` binds \"Enter\" to `selector` and fails — use `page.press(null, \"Enter\")` or `page.press({ key: \"Enter\" })`.", .click, .fill, .scroll, .hover, .selectOption, .setChecked => "", - .findElement => "`name` is a case-insensitive substring, or a JS regex literal like `/sign (in|up)/i` (case-insensitive even without `i`).", + .findElement => "`name` is a case-insensitive substring, or a JS regex literal like `/sign (in|up)/` (also case-insensitive).", .search, .markdown, .html, .links, .tree, .nodeDetails, .interactiveElements, .structuredData, .detectForms, .consoleLogs, .getUrl, .getCookies, .getEnv => "", }; }