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

@@ -0,0 +1,70 @@
+// Copyright (C) 2023-2025  Lightpanda (Selecy SAS)
+//
+// Francis Bouvier <francis@lightpanda.io>
+// Pierre Tachoire <pierre@lightpanda.io>
+//
+// 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 <https://www.gnu.org/licenses/>.
+
+//! 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();
+}

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

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;

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" {