From db14d0be1b1d83dcd7f22f26fcc75a7507732415 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 22 May 2026 18:47:46 +0200 Subject: [PATCH] tools: map waitForSelector timeout to NodeNotFound When a selector times out without matching, the outcome is the same as /hover or /click against a missing node. Returning InternalError instead gave inconsistent feedback to the agent. Map error.Timeout to NodeNotFound and leave the catch-all log + InternalError for genuinely unexpected errors. Tighten the existing MCP - waitForSelector: timeout test to pin the specific error name. --- src/browser/tools.zig | 12 +++++++++--- src/mcp/tools.zig | 5 +++-- 2 files changed, 12 insertions(+), 5 deletions(-) diff --git a/src/browser/tools.zig b/src/browser/tools.zig index 9a3d8d6c..1154ab46 100644 --- a/src/browser/tools.zig +++ b/src/browser/tools.zig @@ -961,9 +961,15 @@ fn execWaitForSelector(arena: std.mem.Allocator, session: *lp.Session, registry: const timeout_ms = args.timeout orelse 5000; - const node = lp.actions.waitForSelector(args.selector, timeout_ms, session) catch |err| { - if (err == error.InvalidSelector) return ToolError.InvalidParams; - return ToolError.InternalError; + const node = lp.actions.waitForSelector(args.selector, timeout_ms, session) catch |err| switch (err) { + error.InvalidSelector => return ToolError.InvalidParams, + // Timeout w/o a match: same outcome as `/hover selector=…` on a missing + // node — surface `NodeNotFound` so the LLM sees a consistent signal. + error.Timeout => return ToolError.NodeNotFound, + else => { + log.debug(.browser, "waitForSelector error", .{ .err = @errorName(err) }); + return ToolError.InternalError; + }, }; const registered = registry.register(node) catch return ToolError.InternalError; diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index 93a92684..6fc3c9f3 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -886,14 +886,15 @@ test "MCP - waitForSelector: timeout" { ); defer server.deinit(); - // waitForSelector with a short timeout on a non-existent element should error + // Missing element after the timeout surfaces as NodeNotFound, matching + // the error /hover, /click, etc. produce when their selector misses. const msg = \\{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"waitForSelector","arguments":{"selector":"#nonexistent","timeout":100}}} ; try router.handleMessage(server, testing.arena_allocator, msg); try testing.expectJson(.{ .id = 1, - .@"error" = struct {}{}, + .@"error" = .{ .message = "NodeNotFound" }, }, out.written()); }