Merge pull request #2880 from lightpanda-io/script_replay_nav_fixes

script: fix two replay crashes on tool-triggered navigation and $LP_* goto URLs

src/browser/Session.zig

@@ -72,7 +72,10 @@ _nav_cursor: usize = 0,
 
 // Set by the agent script Runtime around one tool call so each `Page` handle's
 // tools act on its own frame, not `pages[0]`. Null for all other callers.
-_tool_frame_override: ?*Frame = null,
+// A frame id, not a pointer: a tool-triggered navigation commits a replacement
+// Page mid-call, freeing the old Frame while keeping its frame id (see
+// `commitPendingPage`).
+_tool_frame_override: ?u32 = null,
 
 // Loader IDs are scoped to the Session: each new BrowserContext gets a
 // fresh counter. Frame IDs (`frame_id_gen`) live on `Browser` instead so
@@ -444,8 +447,9 @@ pub fn primaryPage(self: *Session) ?PageHandle {
 
 // DEPRECATED. Exists during our transition to multi-page sessions.
 pub fn currentFrame(self: *Session) ?*Frame {
-    if (self._tool_frame_override) |frame| {
-        return frame;
+    if (self._tool_frame_override) |frame_id| {
+        // No pages[0] fallthrough: the override targets one specific page.
+        return self.findFrameByFrameId(frame_id);
     }
     if (self.pages.items.len == 0) {
         return null;
@@ -458,8 +462,8 @@ pub fn currentFrame(self: *Session) ?*Frame {
 }
 
 /// See `_tool_frame_override`. Pass null to clear.
-pub fn setToolFrameOverride(self: *Session, frame: ?*Frame) void {
-    self._tool_frame_override = frame;
+pub fn setToolFrameOverride(self: *Session, frame_id: ?u32) void {
+    self._tool_frame_override = frame_id;
 }
 
 // Multi-page aware: frame ids are globally unique (monotonic on `Browser`).

src/browser/tools.zig

@@ -1856,7 +1856,8 @@ pub fn startGoto(
     arguments: ?std.json.Value,
     receiver_frame_id: ?u32,
 ) ToolError!StartedGoto {
-    const args = try parseArgs(GotoParams, arena, arguments);
+    // This path bypasses `call`, so it needs its own `$LP_*` substitution pass.
+    const args = try parseArgs(GotoParams, arena, try substituteStringArgs(arena, .goto, arguments));
     if (receiver_frame_id) |fid| {
         if (session.livePage(fid)) |page| {
             registry.resetFrame(arena, &page.frame);

src/script/Runtime.zig

@@ -449,10 +449,10 @@ fn invoke(self: *Runtime, tool: BrowserTool, info: *const v8.FunctionCallbackInf
     // `pages[0]` is not necessarily this handle's page.
     const frame_id = self.receiverFrameId(context, info) orelse
         return self.throwError("page is not navigated or has been closed; call page.goto(url) first");
-    const frame = self.session.findFrameByFrameId(frame_id) orelse
+    if (self.session.findFrameByFrameId(frame_id) == null)
         return self.throwError("page handle is no longer valid; the page was closed");
 
-    self.session.setToolFrameOverride(frame);
+    self.session.setToolFrameOverride(frame_id);
     defer self.session.setToolFrameOverride(null);
 
     const result = self.callTool(arena, tool, args) catch |err| switch (err) {
@@ -1083,6 +1083,51 @@ test "agent script runtime: parallel gotos coexist and route per page" {
     );
 }
 
+test "agent script runtime: goto resolves $LP_* placeholders" {
+    defer testing.reset();
+    defer testing.test_session.closeAllPages();
+
+    var registry = CDPNode.Registry.init(testing.allocator);
+    defer registry.deinit();
+
+    const runtime = try Runtime.init(testing.allocator, testing.test_app, testing.test_session, &registry);
+    defer runtime.deinit();
+
+    _ = setenv(@constCast("LP_RUNTIME_GOTO_BASE"), @constCast("http://localhost:9582"), 1);
+    defer _ = unsetenv(@constCast("LP_RUNTIME_GOTO_BASE"));
+
+    try runTestScript(runtime,
+        \\const page = new Page();
+        \\await page.goto("$LP_RUNTIME_GOTO_BASE/src/browser/tests/mcp_actions.html");
+        \\const { btn } = page.extract({ btn: "#btn" });
+        \\if (btn !== "Click Me") throw new Error("wrong page: " + btn);
+    );
+}
+
+extern fn setenv(name: [*:0]u8, value: [*:0]u8, override: c_int) c_int;
+extern fn unsetenv(name: [*:0]u8) c_int;
+
+test "agent script runtime: a tool-triggered navigation keeps the handle routable" {
+    defer testing.reset();
+    defer testing.test_session.closeAllPages();
+
+    var registry = CDPNode.Registry.init(testing.allocator);
+    defer registry.deinit();
+
+    const runtime = try Runtime.init(testing.allocator, testing.test_app, testing.test_session, &registry);
+    defer runtime.deinit();
+
+    // click commits a replacement Page mid-call; the frame override and the
+    // handle's frame id must both survive the swap.
+    try runTestScript(runtime,
+        \\const page = new Page();
+        \\await page.goto("http://localhost:9582/src/browser/tests/mcp_nav.html");
+        \\page.click("#navlink");
+        \\const href = page.evaluate("location.href");
+        \\if (href !== "about:blank") throw new Error("expected about:blank, got: " + href);
+    );
+}
+
 test "agent script runtime: re-goto on the same page object replaces its page" {
     defer testing.reset();
     defer testing.test_session.closeAllPages();