From 071eca1f3febbd88fc574801f8fc80a4aefb788b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Thu, 27 Aug 2026 16:27:27 +0200 Subject: [PATCH 1/5] mcp: fold Server.Session onto lp.ToolSession Extract the Browser+Session+Notification+node Registry quartet, with its ordering-sensitive init/teardown and isolate enter/exit, into src/ToolSession.zig. mcp/Server.zig's sessions map now holds *lp.ToolSession directly; the session id lives only as the map key. Claude-Session: https://claude.ai/code/session_01M6WGk8wZSE28efFQkYT9SK --- src/ToolSession.zig | 70 +++++++++++++++++++++++++++++ src/lightpanda.zig | 1 + src/mcp/Server.zig | 105 +++++++++++++++----------------------------- src/mcp/tools.zig | 36 +++++++-------- 4 files changed, 125 insertions(+), 87 deletions(-) create mode 100644 src/ToolSession.zig diff --git a/src/ToolSession.zig b/src/ToolSession.zig new file mode 100644 index 000000000..bc4d490cd --- /dev/null +++ b/src/ToolSession.zig @@ -0,0 +1,70 @@ +// Copyright (C) 2023-2025 Lightpanda (Selecy SAS) +// +// Francis Bouvier +// Pierre Tachoire +// +// This program is free software: you can redistribute it and/or modify +// it under the terms of the GNU Affero General Public License as +// published by the Free Software Foundation, either version 3 of the +// License, or (at your option) any later version. +// +// This program is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU Affero General Public License for more details. +// +// You should have received a copy of the GNU Affero General Public License +// along with this program. If not, see . + +//! An isolated browsing context driven through `lp.tools`: its own Browser +//! (hence V8 isolate), Session, Notification and node Registry. `self` must +//! not move after `init` — Browser registers self-pointers. + +const App = @import("App.zig"); +const Browser = @import("browser/Browser.zig"); +const Session = @import("browser/Session.zig"); +const Notification = @import("Notification.zig"); +const CDPNode = @import("cdp/Node.zig"); + +const ToolSession = @This(); + +browser: Browser, +session: *Session, +notification: *Notification, +registry: CDPNode.Registry, + +/// Leaves the browser's isolate entered, like `Browser.init`; callers +/// sharing one thread between several isolates park it with +/// `exitIsolate` afterwards. +pub fn init(self: *ToolSession, app: *App) !void { + self.notification = try .init(app.allocator); + errdefer self.notification.deinit(); + + self.registry = .init(app.allocator); + errdefer self.registry.deinit(); + + try self.browser.init(app, .{}, null); + errdefer self.browser.deinit(); + + self.session = try self.browser.newSession(self.notification); + try self.session.enableConsoleCapture(); +} + +/// The isolate must be current (`enterIsolate` if parked): Browser.deinit's +/// Env.deinit exit has to balance against this context's isolate. +pub fn deinit(self: *ToolSession) void { + self.registry.deinit(); + self.browser.deinit(); + self.notification.deinit(); +} + +/// V8's "current isolate" is a per-thread stack: when several contexts +/// share a thread, bracket any use of the Browser/Session with +/// enterIsolate/exitIsolate and leave it un-entered otherwise. +pub fn enterIsolate(self: *ToolSession) void { + self.browser.env.isolate.enter(); +} + +pub fn exitIsolate(self: *ToolSession) void { + self.browser.env.isolate.exit(); +} diff --git a/src/lightpanda.zig b/src/lightpanda.zig index 1ce1e85ff..14da3226c 100644 --- a/src/lightpanda.zig +++ b/src/lightpanda.zig @@ -34,6 +34,7 @@ pub const Page = @import("browser/Page.zig"); pub const Frame = @import("browser/Frame.zig"); pub const Browser = @import("browser/Browser.zig"); pub const Session = @import("browser/Session.zig"); +pub const ToolSession = @import("ToolSession.zig"); pub const js = @import("browser/js/js.zig"); pub const dump = @import("browser/dump.zig"); diff --git a/src/mcp/Server.zig b/src/mcp/Server.zig index 74a698b19..bc101aa2b 100644 --- a/src/mcp/Server.zig +++ b/src/mcp/Server.zig @@ -9,7 +9,6 @@ const resources = @import("resources.zig"); const router = @import("router.zig"); const tools = @import("tools.zig"); const Transport = @import("Transport.zig"); -const CDPNode = @import("../cdp/Node.zig"); const Self = @This(); @@ -18,39 +17,22 @@ const Self = @This(); /// `Mcp-Session-Id`. pub const default_session_id = "default"; -/// One isolated browsing context. Each owns its own V8 isolate (via -/// `Browser`), so two agents driving different sessions never touch the same -/// page. Heap-allocated and never moved after `init`: `Browser` registers -/// self-pointers (watchdog, http_client) that must stay stable. -pub const Session = struct { - id: []const u8, - browser: lp.Browser, - session: *lp.Session, - notification: *lp.Notification, - node_registry: CDPNode.Registry, - - fn isDefault(self: *const Session) bool { - return std.mem.eql(u8, self.id, default_session_id); - } -}; - allocator: std.mem.Allocator, app: *App, -sessions: std.StringHashMapUnmanaged(*Session) = .empty, +sessions: std.StringHashMapUnmanaged(*lp.ToolSession) = .empty, /// Monotonic counter backing auto-generated session ids (`s1`, `s2`, …). session_seq: u32 = 0, -/// When several sessions (each its own V8 isolate) share one thread, V8's -/// "current isolate" is a per-thread stack, so an isolate must be *entered* -/// around any use of it and left un-entered otherwise. The HTTP transport -/// sets this; stdio (one isolate, permanently entered by `Env`) leaves it -/// false and keeps its historical behavior. See `enterIsolate`/`exitIsolate`. +/// Whether sessions park their isolates between uses (see +/// `ToolSession.enterIsolate`). The HTTP transport sets this; stdio (one +/// isolate, permanently entered by `Env`) leaves it false and keeps its +/// historical behavior. park_isolates: bool = false, /// The session the request currently being handled targets. Safe as a single /// field because every request is dispatched on one thread, one at a time; /// the transport sets it (via `useSession`) before each dispatch. Tools and /// resources read it rather than threading a session through every call. -active_session: *Session = undefined, +active_session: *lp.ToolSession = undefined, transport: Transport, pub fn init(allocator: std.mem.Allocator, app: *App, writer: *std.Io.Writer) !*Self { @@ -69,8 +51,8 @@ pub fn init(allocator: std.mem.Allocator, app: *App, writer: *std.Io.Writer) !*S } pub fn deinit(self: *Self) void { - var it = self.sessions.valueIterator(); - while (it.next()) |entry| self.destroySession(entry.*); + var it = self.sessions.iterator(); + while (it.next()) |kv| self.destroySession(kv.key_ptr.*, kv.value_ptr.*); self.sessions.deinit(self.allocator); self.transport.deinit(); @@ -78,48 +60,37 @@ pub fn deinit(self: *Self) void { } /// Create the session named `id`, or return the existing one. The `id` is -/// duped, so the caller keeps ownership of its slice. -pub fn createSession(self: *Self, id: []const u8) !*Session { +/// duped, so the caller keeps ownership of its slice. Sessions are +/// heap-allocated and never moved: `Browser` registers self-pointers. +pub fn createSession(self: *Self, id: []const u8) !*lp.ToolSession { if (self.sessions.get(id)) |existing| return existing; const owned_id = try self.allocator.dupe(u8, id); errdefer self.allocator.free(owned_id); - const entry = try self.allocator.create(Session); + const entry = try self.allocator.create(lp.ToolSession); errdefer self.allocator.destroy(entry); - const notification = try lp.Notification.init(self.allocator); - errdefer notification.deinit(); - - entry.* = .{ - .id = owned_id, - .browser = undefined, - .session = undefined, - .notification = notification, - .node_registry = CDPNode.Registry.init(self.allocator), - }; - errdefer entry.node_registry.deinit(); - - try entry.browser.init(self.app, .{}, null); - errdefer entry.browser.deinit(); - - entry.session = try entry.browser.newSession(notification); - try entry.session.enableConsoleCapture(); + try entry.init(self.app); + errdefer entry.deinit(); // Only the default session is backed by the on-disk cookie file; named // sessions start clean so agents stay isolated by default. - if (entry.isDefault()) { + if (isDefault(id)) { if (self.app.config.cookieFile()) |cookie_path| { lp.cookies.loadFromFile(entry.session, cookie_path); } } try self.sessions.put(self.allocator, owned_id, entry); - // Browser.init left the isolate entered; park it (see park_isolates). self.exitIsolate(entry); return entry; } +fn isDefault(id: []const u8) bool { + return std.mem.eql(u8, id, default_session_id); +} + /// Switch to the multi-isolate discipline: park the default (which `Server.init` /// left entered) and require every use to bracket with `enterIsolate`. The HTTP /// transport calls this on its worker thread before serving anyone. @@ -128,52 +99,48 @@ pub fn enableIsolateParking(self: *Self) void { self.exitIsolate(self.defaultSession()); } -/// Make `entry`'s isolate the current one for this thread. Must bracket any -/// use of its Browser/Session (dispatch, idle pumping, teardown). No-op under -/// stdio, where the single isolate is permanently current. -pub fn enterIsolate(self: *Self, entry: *Session) void { - if (self.park_isolates) entry.browser.env.isolate.enter(); +/// `ToolSession.enterIsolate`, unless stdio's single isolate is permanently +/// current. Must bracket any use of the session (dispatch, idle pumping, +/// teardown). +pub fn enterIsolate(self: *Self, entry: *lp.ToolSession) void { + if (self.park_isolates) entry.enterIsolate(); } -pub fn exitIsolate(self: *Self, entry: *Session) void { - if (self.park_isolates) entry.browser.env.isolate.exit(); +pub fn exitIsolate(self: *Self, entry: *lp.ToolSession) void { + if (self.park_isolates) entry.exitIsolate(); } /// Tear down the session named `id`. Returns false if no such session, or if /// it is the default (which lives for the whole process). pub fn closeSession(self: *Self, id: []const u8) bool { - if (std.mem.eql(u8, id, default_session_id)) return false; - const entry = self.sessions.fetchRemove(id) orelse return false; - if (self.active_session == entry.value) self.active_session = self.defaultSession(); - self.destroySession(entry.value); + if (isDefault(id)) return false; + const kv = self.sessions.fetchRemove(id) orelse return false; + if (self.active_session == kv.value) self.active_session = self.defaultSession(); + self.destroySession(kv.key, kv.value); return true; } -fn destroySession(self: *Self, entry: *Session) void { - if (entry.isDefault()) { +fn destroySession(self: *Self, id: []const u8, entry: *lp.ToolSession) void { + if (isDefault(id)) { if (self.app.config.cookieJarFile()) |cookie_jar_path| { lp.cookies.saveToFile(&entry.session.cookie_jar, cookie_jar_path); } } - // Re-enter so `Browser.deinit`'s `Env.deinit` exit stays balanced against - // a parked isolate (and operates on the current one). self.enterIsolate(entry); - entry.node_registry.deinit(); - entry.browser.deinit(); - entry.notification.deinit(); - self.allocator.free(entry.id); + entry.deinit(); + self.allocator.free(id); self.allocator.destroy(entry); } /// The session an un-scoped (stdio, or header-less HTTP) request targets. -pub fn defaultSession(self: *Self) *Session { +pub fn defaultSession(self: *Self) *lp.ToolSession { return self.sessions.get(default_session_id).?; } /// Point subsequent tool/resource dispatch at the session named `id`, creating /// it on first use. A null or empty `id` selects the default. -pub fn useSession(self: *Self, id: ?[]const u8) !*Session { +pub fn useSession(self: *Self, id: ?[]const u8) !*lp.ToolSession { const wanted = id orelse ""; self.active_session = if (wanted.len == 0) self.defaultSession() else try self.createSession(wanted); return self.active_session; diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index 0eb10ee0d..f2f001e81 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -154,7 +154,7 @@ fn dispatchBrowserTool( }; const active = server.active_session; - const result = browser_tools.call(arena, active.session, &active.node_registry, name, arguments) catch |err| { + const result = browser_tools.call(arena, active.session, &active.registry, name, arguments) catch |err| { // evaluate/extract surface failures in-band so the LLM can self-correct; // other tools' operational failures are protocol-level. if (surfacesErrorInBand(tool)) { @@ -239,10 +239,10 @@ fn handleSessionList(server: *Server, arena: std.mem.Allocator, id: std.json.Val const Entry = struct { id: []const u8, url: ?[]const u8 }; var list: std.ArrayList(Entry) = .empty; - var it = server.sessions.valueIterator(); - while (it.next()) |entry| { - const url: ?[]const u8 = if (entry.*.session.currentFrame()) |frame| frame.url else null; - list.append(arena, .{ .id = entry.*.id, .url = url }) catch + var it = server.sessions.iterator(); + while (it.next()) |kv| { + const url: ?[]const u8 = if (kv.value_ptr.*.session.currentFrame()) |frame| frame.url else null; + list.append(arena, .{ .id = kv.key_ptr.*, .url = url }) catch return sendErrorContent(server, id, "out of memory"); } @@ -263,7 +263,7 @@ fn handleSessionClose(server: *Server, arena: std.mem.Allocator, id: std.json.Va } // Closing the session serving this very call would tear down the isolate // mid-dispatch; require the client to be elsewhere first. - if (std.mem.eql(u8, args.id, server.active_session.id)) { + if (server.sessions.get(args.id) == server.active_session) { return sendErrorContent(server, id, "cannot close the session you are attached to"); } if (!server.closeSession(args.id)) { @@ -1041,7 +1041,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const btn = frame.document.getElementById("btn", frame).?.asNode(); - const btn_id = (try server.active_session.node_registry.register(btn)).id; + const btn_id = (try server.active_session.registry.register(btn)).id; var btn_id_buf: [12]u8 = undefined; const btn_id_str = std.fmt.bufPrint(&btn_id_buf, "{d}", .{btn_id}) catch unreachable; const click_msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":1,\"method\":\"tools/call\",\"params\":{\"name\":\"click\",\"arguments\":{\"backendNodeId\":", btn_id_str, "}}}" }); @@ -1053,7 +1053,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const inp = frame.document.getElementById("inp", frame).?.asNode(); - const inp_id = (try server.active_session.node_registry.register(inp)).id; + const inp_id = (try server.active_session.registry.register(inp)).id; var inp_id_buf: [12]u8 = undefined; const inp_id_str = std.fmt.bufPrint(&inp_id_buf, "{d}", .{inp_id}) catch unreachable; const fill_msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":2,\"method\":\"tools/call\",\"params\":{\"name\":\"fill\",\"arguments\":{\"backendNodeId\":", inp_id_str, ",\"value\":\"hello\"}}}" }); @@ -1065,7 +1065,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const sel = frame.document.getElementById("sel", frame).?.asNode(); - const sel_id = (try server.active_session.node_registry.register(sel)).id; + const sel_id = (try server.active_session.registry.register(sel)).id; var sel_id_buf: [12]u8 = undefined; const sel_id_str = std.fmt.bufPrint(&sel_id_buf, "{d}", .{sel_id}) catch unreachable; const fill_sel_msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":3,\"method\":\"tools/call\",\"params\":{\"name\":\"fill\",\"arguments\":{\"backendNodeId\":", sel_id_str, ",\"value\":\"opt2\"}}}" }); @@ -1077,7 +1077,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const scrollbox = frame.document.getElementById("scrollbox", frame).?.asNode(); - const scrollbox_id = (try server.active_session.node_registry.register(scrollbox)).id; + const scrollbox_id = (try server.active_session.registry.register(scrollbox)).id; var scroll_id_buf: [12]u8 = undefined; const scroll_id_str = std.fmt.bufPrint(&scroll_id_buf, "{d}", .{scrollbox_id}) catch unreachable; const scroll_msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":4,\"method\":\"tools/call\",\"params\":{\"name\":\"scroll\",\"arguments\":{\"backendNodeId\":", scroll_id_str, ",\"y\":50}}}" }); @@ -1088,7 +1088,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const el = frame.document.getElementById("hoverTarget", frame).?.asNode(); - const el_id = (try server.active_session.node_registry.register(el)).id; + const el_id = (try server.active_session.registry.register(el)).id; var id_buf: [12]u8 = undefined; const id_str = std.fmt.bufPrint(&id_buf, "{d}", .{el_id}) catch unreachable; const msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":5,\"method\":\"tools/call\",\"params\":{\"name\":\"hover\",\"arguments\":{\"backendNodeId\":", id_str, "}}}" }); @@ -1099,7 +1099,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const el = frame.document.getElementById("keyTarget", frame).?.asNode(); - const el_id = (try server.active_session.node_registry.register(el)).id; + const el_id = (try server.active_session.registry.register(el)).id; var id_buf: [12]u8 = undefined; const id_str = std.fmt.bufPrint(&id_buf, "{d}", .{el_id}) catch unreachable; const msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":6,\"method\":\"tools/call\",\"params\":{\"name\":\"press\",\"arguments\":{\"key\":\"Enter\",\"backendNodeId\":", id_str, "}}}" }); @@ -1110,7 +1110,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const el = frame.document.getElementById("sel2", frame).?.asNode(); - const el_id = (try server.active_session.node_registry.register(el)).id; + const el_id = (try server.active_session.registry.register(el)).id; var id_buf: [12]u8 = undefined; const id_str = std.fmt.bufPrint(&id_buf, "{d}", .{el_id}) catch unreachable; const msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":7,\"method\":\"tools/call\",\"params\":{\"name\":\"selectOption\",\"arguments\":{\"backendNodeId\":", id_str, ",\"value\":\"b\"}}}" }); @@ -1121,7 +1121,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const el = frame.document.getElementById("chk", frame).?.asNode(); - const el_id = (try server.active_session.node_registry.register(el)).id; + const el_id = (try server.active_session.registry.register(el)).id; var id_buf: [12]u8 = undefined; const id_str = std.fmt.bufPrint(&id_buf, "{d}", .{el_id}) catch unreachable; const msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":8,\"method\":\"tools/call\",\"params\":{\"name\":\"setChecked\",\"arguments\":{\"backendNodeId\":", id_str, ",\"checked\":true}}}" }); @@ -1132,7 +1132,7 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked { const el = frame.document.getElementById("rad", frame).?.asNode(); - const el_id = (try server.active_session.node_registry.register(el)).id; + const el_id = (try server.active_session.registry.register(el)).id; var id_buf: [12]u8 = undefined; const id_str = std.fmt.bufPrint(&id_buf, "{d}", .{el_id}) catch unreachable; const msg = try std.mem.concat(aa, u8, &.{ "{\"jsonrpc\":\"2.0\",\"id\":9,\"method\":\"tools/call\",\"params\":{\"name\":\"setChecked\",\"arguments\":{\"backendNodeId\":", id_str, ",\"checked\":true}}}" }); @@ -1176,8 +1176,8 @@ test "MCP - click that navigates clears node registry" { const before_frame = server.active_session.session.currentFrame().?; const link = before_frame.document.getElementById("navlink", before_frame).?.asNode(); - const link_id = (try server.active_session.node_registry.register(link)).id; - try testing.expect(server.active_session.node_registry.lookup_by_id.contains(link_id)); + const link_id = (try server.active_session.registry.register(link)).id; + try testing.expect(server.active_session.registry.lookup_by_id.contains(link_id)); var id_buf: [12]u8 = undefined; const id_str = std.fmt.bufPrint(&id_buf, "{d}", .{link_id}) catch unreachable; @@ -1189,7 +1189,7 @@ test "MCP - click that navigates clears node registry" { try router.handleMessage(server, aa, click_msg); try testing.expect(server.active_session.session.currentFrame().? != before_frame); - try testing.expect(!server.active_session.node_registry.lookup_by_id.contains(link_id)); + try testing.expect(!server.active_session.registry.lookup_by_id.contains(link_id)); } test "MCP - Actions by selector: hover, selectOption, setChecked" { From d8893f95d75aad0923962840ea7ef23928202ed5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Thu, 27 Aug 2026 16:30:49 +0200 Subject: [PATCH 2/5] mcp: bracket every isolate use, drop park_isolates stdio kept the isolate permanently entered while HTTP parked it between requests, so a handler touching page JS without enterIsolate passed the stdio-mode tests and only failed under --port. Bracket unconditionally: stdio is the one-isolate case of the same discipline. The flag survives only as `multi_session`, which is what it always gated: the session tools need a transport that routes by id. Claude-Session: https://claude.ai/code/session_01M6WGk8wZSE28efFQkYT9SK --- src/mcp/HttpServer.zig | 2 +- src/mcp/Server.zig | 44 ++++++++++++------------------------------ src/mcp/tools.zig | 32 +++++++++++++++++++++--------- 3 files changed, 36 insertions(+), 42 deletions(-) diff --git a/src/mcp/HttpServer.zig b/src/mcp/HttpServer.zig index e1394cd4a..517dd7d19 100644 --- a/src/mcp/HttpServer.zig +++ b/src/mcp/HttpServer.zig @@ -282,7 +282,7 @@ fn worker(self: *HttpServer) void { }; defer server.deinit(); - server.enableIsolateParking(); + server.multi_session = true; self.worker_ok = true; self.worker_ready.set(lp.io); diff --git a/src/mcp/Server.zig b/src/mcp/Server.zig index bc101aa2b..76084450d 100644 --- a/src/mcp/Server.zig +++ b/src/mcp/Server.zig @@ -23,11 +23,10 @@ app: *App, sessions: std.StringHashMapUnmanaged(*lp.ToolSession) = .empty, /// Monotonic counter backing auto-generated session ids (`s1`, `s2`, …). session_seq: u32 = 0, -/// Whether sessions park their isolates between uses (see -/// `ToolSession.enterIsolate`). The HTTP transport sets this; stdio (one -/// isolate, permanently entered by `Env`) leaves it false and keeps its -/// historical behavior. -park_isolates: bool = false, +/// Whether the transport can route a request to a named session. HTTP does +/// (`Mcp-Session-Id`); over stdio the session tools are refused, since a +/// session created there could never be addressed. +multi_session: bool = false, /// The session the request currently being handled targets. Safe as a single /// field because every request is dispatched on one thread, one at a time; /// the transport sets it (via `useSession`) before each dispatch. Tools and @@ -83,7 +82,7 @@ pub fn createSession(self: *Self, id: []const u8) !*lp.ToolSession { } try self.sessions.put(self.allocator, owned_id, entry); - self.exitIsolate(entry); + entry.exitIsolate(); return entry; } @@ -91,25 +90,6 @@ fn isDefault(id: []const u8) bool { return std.mem.eql(u8, id, default_session_id); } -/// Switch to the multi-isolate discipline: park the default (which `Server.init` -/// left entered) and require every use to bracket with `enterIsolate`. The HTTP -/// transport calls this on its worker thread before serving anyone. -pub fn enableIsolateParking(self: *Self) void { - self.park_isolates = true; - self.exitIsolate(self.defaultSession()); -} - -/// `ToolSession.enterIsolate`, unless stdio's single isolate is permanently -/// current. Must bracket any use of the session (dispatch, idle pumping, -/// teardown). -pub fn enterIsolate(self: *Self, entry: *lp.ToolSession) void { - if (self.park_isolates) entry.enterIsolate(); -} - -pub fn exitIsolate(self: *Self, entry: *lp.ToolSession) void { - if (self.park_isolates) entry.exitIsolate(); -} - /// Tear down the session named `id`. Returns false if no such session, or if /// it is the default (which lives for the whole process). pub fn closeSession(self: *Self, id: []const u8) bool { @@ -127,7 +107,7 @@ fn destroySession(self: *Self, id: []const u8, entry: *lp.ToolSession) void { } } - self.enterIsolate(entry); + entry.enterIsolate(); entry.deinit(); self.allocator.free(id); self.allocator.destroy(entry); @@ -163,9 +143,9 @@ pub fn idle(self: *Self) u31 { while (it.next()) |entry| { // Pumping may resume JS (e.g. a completed script fetch), so it needs // the session's isolate current. - self.enterIsolate(entry.*); + entry.*.enterIsolate(); wait = @min(wait, entry.*.session.idleSlice()); - self.exitIsolate(entry.*); + entry.*.exitIsolate(); } return wait; } @@ -198,8 +178,8 @@ pub fn handleToolList(self: *Self, arena: std.mem.Allocator, req: protocol.Reque pub fn handleToolCall(self: *Self, arena: std.mem.Allocator, req: protocol.Request) !void { // Dispatch runs page JS, so enter the target isolate around it. const entry = self.active_session; - self.enterIsolate(entry); - defer self.exitIsolate(entry); + entry.enterIsolate(); + defer entry.exitIsolate(); return tools.handleCall(self, arena, req); } @@ -209,8 +189,8 @@ pub fn handleResourceList(self: *Self, req: protocol.Request) !void { pub fn handleResourceRead(self: *Self, arena: std.mem.Allocator, req: protocol.Request) !void { const entry = self.active_session; - self.enterIsolate(entry); - defer self.exitIsolate(entry); + entry.enterIsolate(); + defer entry.exitIsolate(); return resources.handleRead(self, arena, req); } diff --git a/src/mcp/tools.zig b/src/mcp/tools.zig index f2f001e81..31444d7f7 100644 --- a/src/mcp/tools.zig +++ b/src/mcp/tools.zig @@ -207,12 +207,11 @@ fn handleSave(server: *Server, arena: std.mem.Allocator, id: std.json.Value, arg try sendToolResultText(server, id, msg, false); } -/// The session tools require the HTTP transport's parked-isolate discipline: -/// a second session means a second V8 isolate, only safe when isolates are -/// entered around use. Over stdio (one permanently-entered isolate) they are -/// all unsupported, kept uniform so clients see one consistent rule. +/// The session tools need a transport that routes by session id (HTTP's +/// `Mcp-Session-Id`). Over stdio they are all unsupported, kept uniform so +/// clients see one consistent rule. fn requireMultiSession(server: *Server, id: std.json.Value) !bool { - if (server.park_isolates) return true; + if (server.multi_session) return true; try sendToolResultText(server, id, "multiple sessions require the HTTP transport (start with --port)", true); return false; } @@ -1036,6 +1035,10 @@ test "MCP - Actions: click, fill, scroll, hover, press, selectOption, setChecked var out: std.Io.Writer.Allocating = .init(aa); const server = try testLoadPage("http://localhost:9582/src/browser/tests/mcp_actions.html", &out.writer); defer server.deinit(); + // Poking the page directly (node registration, JS) needs its isolate + // entered, as a tool dispatch would. + server.active_session.enterIsolate(); + defer server.active_session.exitIsolate(); const frame = server.active_session.session.currentFrame().?; @@ -1173,6 +1176,10 @@ test "MCP - click that navigates clears node registry" { var out: std.Io.Writer.Allocating = .init(aa); const server = try testLoadPage("http://localhost:9582/src/browser/tests/mcp_nav.html", &out.writer); defer server.deinit(); + // Poking the page directly (node registration, JS) needs its isolate + // entered, as a tool dispatch would. + server.active_session.enterIsolate(); + defer server.active_session.exitIsolate(); const before_frame = server.active_session.session.currentFrame().?; const link = before_frame.document.getElementById("navlink", before_frame).?.asNode(); @@ -1198,6 +1205,10 @@ test "MCP - Actions by selector: hover, selectOption, setChecked" { var out: std.Io.Writer.Allocating = .init(aa); const server = try testLoadPage("http://localhost:9582/src/browser/tests/mcp_actions.html", &out.writer); defer server.deinit(); + // Poking the page directly (node registration, JS) needs its isolate + // entered, as a tool dispatch would. + server.active_session.enterIsolate(); + defer server.active_session.exitIsolate(); // Single-page test: reach straight into the live page. const page = server.active_session.session.pages.items[0]; @@ -1580,8 +1591,7 @@ test "MCP - sessions: new, list, attach isolation, close" { var out: std.Io.Writer.Allocating = .init(aa); var server = try Server.init(testing.allocator, testing.test_app, &out.writer); defer server.deinit(); - // Session tools require the HTTP transport's parked-isolate discipline. - server.enableIsolateParking(); + server.multi_session = true; try router.handleMessage(server, aa, \\{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"session_new","arguments":{"name":"a"}}} @@ -1628,10 +1638,14 @@ fn testLoadPage(url: [:0]const u8, writer: *std.Io.Writer) !*Server { var server = try Server.init(testing.allocator, testing.test_app, writer); errdefer server.deinit(); - const page = try server.active_session.session.createPage(); + const session = server.active_session; + session.enterIsolate(); + defer session.exitIsolate(); + + const page = try session.session.createPage(); try page.navigate(url, .{}); - var runner = server.active_session.session.runner(.{}); + var runner = session.session.runner(.{}); try runner.waitForFrame(page.frame_id, 2000, .{ .until = .done }); return server; } From dadbae121c513f7d40a46c461e2a36ab43dea33c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Thu, 27 Aug 2026 16:35:11 +0200 Subject: [PATCH 3/5] ToolSession: test the shared-thread contract Two sessions on one thread, each entered only around its own use, with JS state proving the isolates are separate and a balanced park/enter/ deinit teardown. Pins what the C API relies on without Server in the loop. Claude-Session: https://claude.ai/code/session_01M6WGk8wZSE28efFQkYT9SK --- src/ToolSession.zig | 40 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 40 insertions(+) diff --git a/src/ToolSession.zig b/src/ToolSession.zig index bc4d490cd..28902b670 100644 --- a/src/ToolSession.zig +++ b/src/ToolSession.zig @@ -68,3 +68,43 @@ pub fn enterIsolate(self: *ToolSession) void { pub fn exitIsolate(self: *ToolSession) void { self.browser.env.isolate.exit(); } + +const std = @import("std"); +const tools = @import("browser/tools.zig"); +const testing = @import("testing.zig"); + +test "ToolSession: isolates interleave on one thread and tear down balanced" { + const arena = testing.arena_allocator; + + var a: ToolSession = undefined; + try a.init(testing.test_app); + try loadBlank(&a); + a.exitIsolate(); + + var b: ToolSession = undefined; + try b.init(testing.test_app); + try loadBlank(&b); + b.exitIsolate(); + + a.enterIsolate(); + try std.testing.expectEqualStrings("a", (try tools.evalScript(arena, a.session, &a.registry, "globalThis.tag = 'a'")).text); + a.exitIsolate(); + + b.enterIsolate(); + try std.testing.expectEqualStrings("undefined", (try tools.evalScript(arena, b.session, &b.registry, "String(globalThis.tag)")).text); + b.exitIsolate(); + + a.enterIsolate(); + try std.testing.expectEqualStrings("a", (try tools.evalScript(arena, a.session, &a.registry, "globalThis.tag")).text); + a.deinit(); + + b.enterIsolate(); + b.deinit(); +} + +fn loadBlank(ts: *ToolSession) !void { + const page = try ts.session.createPage(); + try page.navigate("about:blank", .{}); + var runner = ts.session.runner(.{}); + try runner.waitForFrame(page.frame_id, 2000, .{ .until = .done }); +} From 0865f4e337c4f5e28cd500cdcf13713653cd5fc4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Thu, 27 Aug 2026 16:40:53 +0200 Subject: [PATCH 4/5] agent: fold the browser quartet onto lp.ToolSession Agent held the same Browser/Session/Notification/registry fields and teardown order as mcp/Server.zig, plus its own "enableConsoleCapture after every newSession" for /reset. ToolSession.restartSession() owns that now; init is written in terms of it. Claude-Session: https://claude.ai/code/session_01M6WGk8wZSE28efFQkYT9SK --- src/ToolSession.zig | 6 +++++ src/agent/Agent.zig | 61 +++++++++++++++++---------------------------- 2 files changed, 29 insertions(+), 38 deletions(-) diff --git a/src/ToolSession.zig b/src/ToolSession.zig index 28902b670..9dcf7324d 100644 --- a/src/ToolSession.zig +++ b/src/ToolSession.zig @@ -46,6 +46,12 @@ pub fn init(self: *ToolSession, app: *App) !void { try self.browser.init(app, .{}, null); errdefer self.browser.deinit(); + try self.restartSession(); +} + +/// Replace the session with a fresh one on the same browser: page, cookies, +/// storage and history gone. The old `session` pointer is invalid afterwards. +pub fn restartSession(self: *ToolSession) !void { self.session = try self.browser.newSession(self.notification); try self.session.enableConsoleCapture(); } diff --git a/src/agent/Agent.zig b/src/agent/Agent.zig index 2895737b9..e68f770e9 100644 --- a/src/agent/Agent.zig +++ b/src/agent/Agent.zig @@ -32,7 +32,6 @@ const ScriptRuntime = lp.Runtime; const Candidate = zenai.provider.Candidate; const App = @import("../App.zig"); -const CDPNode = @import("../cdp/Node.zig"); const Conversation = @import("Conversation.zig"); const Terminal = @import("Terminal.zig"); const SlashCommand = @import("SlashCommand.zig"); @@ -149,10 +148,7 @@ model_base_url: ?[:0]const u8, /// `model_completion_arena`; invalidated on `/provider` switch. model_completions: ?ModelCompletions, model_completion_arena: std.heap.ArenaAllocator, -notification: *lp.Notification, -browser: lp.Browser, -session: *lp.Session, -node_registry: CDPNode.Registry, +ts: lp.ToolSession, terminal: Terminal, save_buffer: Recorder, save_path: ?[]u8, @@ -308,9 +304,6 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent std.debug.print("\n", .{}); } - const notification: *lp.Notification = try .init(allocator); - errdefer notification.deinit(); - const self = try allocator.create(Agent); errdefer allocator.destroy(self); @@ -328,10 +321,7 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent .model_base_url = opts.base_url, .model_completions = null, .model_completion_arena = .init(allocator), - .notification = notification, - .browser = undefined, - .session = undefined, - .node_registry = .init(allocator), + .ts = undefined, .terminal = .init(allocator, history_paths, verbosity, will_repl), .save_buffer = .init(allocator), .save_path = null, @@ -345,16 +335,14 @@ pub fn init(allocator: std.mem.Allocator, app: *App, opts: Config.Agent) !*Agent .one_shot_attachments = if (opts.attach.items.len == 0) null else opts.attach.items, .available_providers = available_providers, }; - errdefer self.node_registry.deinit(); errdefer self.terminal.deinit(); errdefer self.conversation.deinit(); self.terminal.installLogSink(); errdefer self.terminal.uninstallLogSink(); - try self.browser.init(app, .{}, null); - errdefer self.browser.deinit(); - - try self.startSession(); + try self.ts.init(app); + errdefer self.ts.deinit(); + self.installCancelHook(); self.ai_client = if (self.credential) |*c| try zenai.provider.Client.init(lp.io, allocator, c.provider, c.keySlice(), .{ .base_url = opts.base_url, .retry_policy = .long_running, .bill_to = hfBillTo(c.provider), .environ = lp.environ(), .account_id = c.accountId() }) else null; errdefer if (self.ai_client) |c| c.deinit(allocator); @@ -381,9 +369,7 @@ pub fn deinit(self: *Agent) void { self.terminal.deinit(); self.conversation.deinit(); self.model_completion_arena.deinit(); - self.node_registry.deinit(); - self.browser.deinit(); - self.notification.deinit(); + self.ts.deinit(); if (self.ai_client) |ai_client| ai_client.deinit(self.allocator); if (self.credential) |*c| c.deinit(self.allocator); self.allocator.free(self.model); @@ -395,15 +381,13 @@ pub fn deinit(self: *Agent) void { /// isocline idle hook; returns the delay in ms before the next invocation. fn idlePump(arg: ?*anyopaque) callconv(.c) c_long { const self: *Agent = @ptrCast(@alignCast(arg.?)); - return self.session.idleSlice(); + return self.ts.session.idleSlice(); } -/// Create a fresh browser session and wire its cancel hook back to this agent -/// so Ctrl-C aborts in-flight page work. Startup and `/reset`. -fn startSession(self: *Agent) !void { - self.session = try self.browser.newSession(self.notification); - self.session.cancel_hook = .{ .context = @ptrCast(self), .check = checkCancel }; - try self.session.enableConsoleCapture(); +/// Wire the session's cancel hook back to this agent so Ctrl-C aborts +/// in-flight page work. Startup and `/reset`. +fn installCancelHook(self: *Agent) void { + self.ts.session.cancel_hook = .{ .context = @ptrCast(self), .check = checkCancel }; } // Compile-time constant; projected once per process to avoid rebuilding per call. @@ -435,7 +419,7 @@ pub fn requestCancel(self: *Agent) void { runtime.terminate(); } } - self.browser.env.terminate(); + self.ts.browser.env.terminate(); } /// Lives in main's stack so it can be registered with the sighandler before the @@ -477,7 +461,7 @@ fn drainCancellation(self: *Agent, baseline: usize) error{UserCancelled} { fn resetAfterCancel(self: *Agent, baseline: usize) void { self.endStreamedText(); self.conversation.rollback(baseline); - self.browser.env.cancelTerminate(); + self.ts.browser.env.cancelTerminate(); self.cancel_requested.store(false, .release); self.http_interrupt.reset(); } @@ -612,7 +596,7 @@ fn runRepl(self: *Agent) void { // Slash commands and idle Ctrl-C set the cancel flag without clearing // V8's terminate state; drain both before the next turn. if (self.cancel_requested.swap(false, .acq_rel)) { - self.browser.env.cancelTerminate(); + self.ts.browser.env.cancelTerminate(); } const trimmed = std.mem.trim(u8, line, &std.ascii.whitespace); @@ -630,7 +614,7 @@ fn runRepl(self: *Agent) void { // `line` keeps the `$LP_*` placeholder so the secret never reaches // the recorder; only the evaluated copy is expanded. const script = browser_tools.substituteEnvVars(aa, line) catch line; - const result = browser_tools.evalScript(aa, self.session, &self.node_registry, script) catch |err| { + const result = browser_tools.evalScript(aa, self.ts.session, &self.ts.registry, script) catch |err| { self.terminal.printError("{s}", .{switch (err) { error.OutOfMemory => "out of memory", error.FrameNotLoaded => "no page loaded — run /goto first (Esc exits JS mode)", @@ -640,7 +624,7 @@ fn runRepl(self: *Agent) void { }; // Surface console output: slash commands (and thus /consoleLogs) // are unreachable in JS mode, so a console must echo logs itself. - const logs = std.mem.trimEnd(u8, self.session.drainConsoleMessages(), "\n"); + const logs = std.mem.trimEnd(u8, self.ts.session.drainConsoleMessages(), "\n"); if (logs.len > 0) self.printData(logs); if (result.is_error) { self.terminal.printError("{s}", .{result.text}); @@ -807,7 +791,7 @@ fn clearConversation(self: *Agent) void { if (self.save_path) |p| self.allocator.free(p); self.save_path = null; self.total_usage = .{}; - self.node_registry.reset(); + self.ts.registry.reset(); } /// Forget the conversation while leaving the browser session live — loaded page @@ -820,10 +804,11 @@ fn handleClear(self: *Agent) void { /// Full clean slate: everything `/clear` drops, plus a fresh browser session, /// so the loaded page, cookies, storage, and history are gone too. fn handleReset(self: *Agent) void { - self.startSession() catch |err| { + self.ts.restartSession() catch |err| { self.terminal.printError("reset failed: {s}", .{@errorName(err)}); return; }; + self.installCancelHook(); self.clearConversation(); self.terminal.printInfo("Reset conversation and browser session. Page, cookies, and storage cleared.", .{}); } @@ -1488,7 +1473,7 @@ fn runCommand(self: *Agent, arena: std.mem.Allocator, cmd: Command) browser_tool .tool_call => |t| t, else => return .{ .text = "internal: command has no tool mapping", .is_error = true }, }; - return browser_tools.call(arena, self.session, &self.node_registry, tc.name(), tc.args) catch |err| .{ + return browser_tools.call(arena, self.ts.session, &self.ts.registry, tc.name(), tc.args) catch |err| .{ .text = switch (err) { error.OutOfMemory => "out of memory", error.FrameNotLoaded => "no page loaded — run /goto first", @@ -1548,7 +1533,7 @@ fn runScript(self: *Agent, path: []const u8) bool { return false; }; - const runtime = ScriptRuntime.init(self.allocator, self.browser.app, self.session, &self.node_registry) catch |err| { + const runtime = ScriptRuntime.init(self.allocator, self.ts.browser.app, self.ts.session, &self.ts.registry) catch |err| { self.terminal.printError("Failed to initialize script runtime: {s}", .{@errorName(err)}); return false; }; @@ -1561,7 +1546,7 @@ fn runScript(self: *Agent, path: []const u8) bool { self.active_script_runtime = null; self.script_runtime_mutex.unlock(lp.io); runtime.cancelTerminate(); - self.browser.env.cancelTerminate(); + self.ts.browser.env.cancelTerminate(); self.cancel_requested.store(false, .release); } @@ -1922,7 +1907,7 @@ 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: zenai.provider.Client.ToolHandler.Result = if (browser_tools.call(allocator, self.session, &self.node_registry, tool_name, arguments)) |result| + const outcome: zenai.provider.Client.ToolHandler.Result = if (browser_tools.call(allocator, self.ts.session, &self.ts.registry, tool_name, arguments)) |result| .{ .content = capToolOutput(allocator, tool_name, result.text), .is_error = result.is_error } else |err| .{ .content = std.fmt.allocPrint(allocator, "Error: {s}", .{browser_tools.errorMessage(err)}) catch "Error: tool execution failed", .is_error = true }; From 33fcf99d0852624c9528e1e78669bbd797e61479 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A0=20Arrufat?= Date: Fri, 28 Aug 2026 15:16:58 +0200 Subject: [PATCH 5/5] test: use testing.expectEqual helper --- src/ToolSession.zig | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/ToolSession.zig b/src/ToolSession.zig index 9dcf7324d..eca52abc2 100644 --- a/src/ToolSession.zig +++ b/src/ToolSession.zig @@ -75,7 +75,6 @@ pub fn exitIsolate(self: *ToolSession) void { self.browser.env.isolate.exit(); } -const std = @import("std"); const tools = @import("browser/tools.zig"); const testing = @import("testing.zig"); @@ -93,15 +92,15 @@ test "ToolSession: isolates interleave on one thread and tear down balanced" { b.exitIsolate(); a.enterIsolate(); - try std.testing.expectEqualStrings("a", (try tools.evalScript(arena, a.session, &a.registry, "globalThis.tag = 'a'")).text); + try testing.expectEqual("a", (try tools.evalScript(arena, a.session, &a.registry, "globalThis.tag = 'a'")).text); a.exitIsolate(); b.enterIsolate(); - try std.testing.expectEqualStrings("undefined", (try tools.evalScript(arena, b.session, &b.registry, "String(globalThis.tag)")).text); + try testing.expectEqual("undefined", (try tools.evalScript(arena, b.session, &b.registry, "String(globalThis.tag)")).text); b.exitIsolate(); a.enterIsolate(); - try std.testing.expectEqualStrings("a", (try tools.evalScript(arena, a.session, &a.registry, "globalThis.tag")).text); + try testing.expectEqual("a", (try tools.evalScript(arena, a.session, &a.registry, "globalThis.tag")).text); a.deinit(); b.enterIsolate();