Fix page re-navigate

It isn't safe/correct to call `navigate` on the same page multiple times. A page
is meant to have 1 navigate call. The caller should either remove the page
and create a new one, or call Session.replacePage.

This commit removes the *Page from the MCP Server and instead interacts with
the session to create or remove+create the page as needed, and lets the Session
own the *Page.

It also adds a bit of defensiveness around parameter parsing, e.g. calling
{"method": "tools/call"} (without an id) now errors instead of crashing.
This commit is contained in:
Karl Seguin
2026-03-07 10:19:37 +08:00
parent 21313adf9c
commit ae4ad713ec
4 changed files with 42 additions and 27 deletions

View File

@@ -17,7 +17,6 @@ http_client: *HttpClient,
notification: *lp.Notification, notification: *lp.Notification,
browser: lp.Browser, browser: lp.Browser,
session: *lp.Session, session: *lp.Session,
page: *lp.Page,
writer: *std.io.Writer, writer: *std.io.Writer,
mutex: std.Thread.Mutex = .{}, mutex: std.Thread.Mutex = .{},
@@ -45,12 +44,8 @@ pub fn init(allocator: std.mem.Allocator, app: *App, writer: *std.io.Writer) !*S
.http_client = http_client, .http_client = http_client,
.notification = notification, .notification = notification,
.session = undefined, .session = undefined,
.page = undefined,
}; };
self.session = try self.browser.newSession(self.notification); self.session = try self.browser.newSession(self.notification);
self.page = try self.session.createPage();
return self; return self;
} }

View File

@@ -26,6 +26,7 @@ pub const ErrorCode = enum(i64) {
MethodNotFound = -32601, MethodNotFound = -32601,
InvalidParams = -32602, InvalidParams = -32602,
InternalError = -32603, InternalError = -32603,
PageNotLoaded = -32604,
}; };
pub const Notification = struct { pub const Notification = struct {

View File

@@ -37,7 +37,7 @@ const ResourceStreamingResult = struct {
}, },
const StreamingText = struct { const StreamingText = struct {
server: *Server, page: *lp.Page,
format: enum { html, markdown }, format: enum { html, markdown },
pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) !void { pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) !void {
@@ -45,10 +45,10 @@ const ResourceStreamingResult = struct {
try jw.writer.writeByte('"'); try jw.writer.writeByte('"');
var escaped = protocol.JsonEscapingWriter.init(jw.writer); var escaped = protocol.JsonEscapingWriter.init(jw.writer);
switch (self.format) { switch (self.format) {
.html => lp.dump.root(self.server.page.document, .{}, &escaped.writer, self.server.page) catch |err| { .html => lp.dump.root(self.page.document, .{}, &escaped.writer, self.page) catch |err| {
log.err(.mcp, "html dump failed", .{ .err = err }); log.err(.mcp, "html dump failed", .{ .err = err });
}, },
.markdown => lp.markdown.dump(self.server.page.document.asNode(), .{}, &escaped.writer, self.server.page) catch |err| { .markdown => lp.markdown.dump(self.page.document.asNode(), .{}, &escaped.writer, self.page) catch |err| {
log.err(.mcp, "markdown dump failed", .{ .err = err }); log.err(.mcp, "markdown dump failed", .{ .err = err });
}, },
} }
@@ -69,16 +69,21 @@ const resource_map = std.StaticStringMap(ResourceUri).initComptime(.{
}); });
pub fn handleRead(server: *Server, arena: std.mem.Allocator, req: protocol.Request) !void { pub fn handleRead(server: *Server, arena: std.mem.Allocator, req: protocol.Request) !void {
if (req.params == null) { if (req.params == null or req.id == null) {
return server.sendError(req.id.?, .InvalidParams, "Missing params"); return server.sendError(req.id orelse .{ .integer = -1 }, .InvalidParams, "Missing params");
} }
const req_id = req.id.?;
const params = std.json.parseFromValueLeaky(ReadParams, arena, req.params.?, .{ .ignore_unknown_fields = true }) catch { const params = std.json.parseFromValueLeaky(ReadParams, arena, req.params.?, .{ .ignore_unknown_fields = true }) catch {
return server.sendError(req.id.?, .InvalidParams, "Invalid params"); return server.sendError(req_id, .InvalidParams, "Invalid params");
}; };
const uri = resource_map.get(params.uri) orelse { const uri = resource_map.get(params.uri) orelse {
return server.sendError(req.id.?, .InvalidRequest, "Resource not found"); return server.sendError(req_id, .InvalidRequest, "Resource not found");
};
const page = server.session.currentPage() orelse {
return server.sendError(req_id, .PageNotLoaded, "Page not loaded");
}; };
switch (uri) { switch (uri) {
@@ -87,20 +92,20 @@ pub fn handleRead(server: *Server, arena: std.mem.Allocator, req: protocol.Reque
.contents = &.{.{ .contents = &.{.{
.uri = params.uri, .uri = params.uri,
.mimeType = "text/html", .mimeType = "text/html",
.text = .{ .server = server, .format = .html }, .text = .{ .page = page, .format = .html },
}}, }},
}; };
try server.sendResult(req.id.?, result); try server.sendResult(req_id, result);
}, },
.@"mcp://page/markdown" => { .@"mcp://page/markdown" => {
const result: ResourceStreamingResult = .{ const result: ResourceStreamingResult = .{
.contents = &.{.{ .contents = &.{.{
.uri = params.uri, .uri = params.uri,
.mimeType = "text/markdown", .mimeType = "text/markdown",
.text = .{ .server = server, .format = .markdown }, .text = .{ .page = page, .format = .markdown },
}}, }},
}; };
try server.sendResult(req.id.?, result); try server.sendResult(req_id, result);
}, },
} }
} }

View File

@@ -78,7 +78,7 @@ const EvaluateParams = struct {
}; };
const ToolStreamingText = struct { const ToolStreamingText = struct {
server: *Server, page: *lp.Page,
action: enum { markdown, links }, action: enum { markdown, links },
pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) !void { pub fn jsonStringify(self: @This(), jw: *std.json.Stringify) !void {
@@ -87,16 +87,16 @@ const ToolStreamingText = struct {
var escaped = protocol.JsonEscapingWriter.init(jw.writer); var escaped = protocol.JsonEscapingWriter.init(jw.writer);
const w = &escaped.writer; const w = &escaped.writer;
switch (self.action) { switch (self.action) {
.markdown => lp.markdown.dump(self.server.page.document.asNode(), .{}, w, self.server.page) catch |err| { .markdown => lp.markdown.dump(self.page.document.asNode(), .{}, w, self.page) catch |err| {
log.err(.mcp, "markdown dump failed", .{ .err = err }); log.err(.mcp, "markdown dump failed", .{ .err = err });
}, },
.links => { .links => {
if (Selector.querySelectorAll(self.server.page.document.asNode(), "a[href]", self.server.page)) |list| { if (Selector.querySelectorAll(self.page.document.asNode(), "a[href]", self.page)) |list| {
defer list.deinit(self.server.page); defer list.deinit(self.page);
var first = true; var first = true;
for (list._nodes) |node| { for (list._nodes) |node| {
if (node.is(Element.Html.Anchor)) |anchor| { if (node.is(Element.Html.Anchor)) |anchor| {
const href = anchor.getHref(self.server.page) catch |err| { const href = anchor.getHref(self.page) catch |err| {
log.err(.mcp, "resolve href failed", .{ .err = err }); log.err(.mcp, "resolve href failed", .{ .err = err });
continue; continue;
}; };
@@ -135,8 +135,8 @@ const tool_map = std.StaticStringMap(ToolAction).initComptime(.{
}); });
pub fn handleCall(server: *Server, arena: std.mem.Allocator, req: protocol.Request) !void { pub fn handleCall(server: *Server, arena: std.mem.Allocator, req: protocol.Request) !void {
if (req.params == null) { if (req.params == null or req.id == null) {
return server.sendError(req.id.?, .InvalidParams, "Missing params"); return server.sendError(req.id orelse .{ .integer = -1 }, .InvalidParams, "Missing params");
} }
const CallParams = struct { const CallParams = struct {
@@ -179,9 +179,12 @@ fn handleMarkdown(server: *Server, arena: std.mem.Allocator, id: std.json.Value,
} }
} else |_| {} } else |_| {}
} }
const page = server.session.currentPage() orelse {
return server.sendError(id, .PageNotLoaded, "Page not loaded");
};
const content = [_]protocol.TextContent(ToolStreamingText){.{ const content = [_]protocol.TextContent(ToolStreamingText){.{
.text = .{ .server = server, .action = .markdown }, .text = .{ .page = page, .action = .markdown },
}}; }};
try server.sendResult(id, protocol.CallToolResult(ToolStreamingText){ .content = &content }); try server.sendResult(id, protocol.CallToolResult(ToolStreamingText){ .content = &content });
} }
@@ -197,9 +200,12 @@ fn handleLinks(server: *Server, arena: std.mem.Allocator, id: std.json.Value, ar
} }
} else |_| {} } else |_| {}
} }
const page = server.session.currentPage() orelse {
return server.sendError(id, .PageNotLoaded, "Page not loaded");
};
const content = [_]protocol.TextContent(ToolStreamingText){.{ const content = [_]protocol.TextContent(ToolStreamingText){.{
.text = .{ .server = server, .action = .links }, .text = .{ .page = page, .action = .links },
}}; }};
try server.sendResult(id, protocol.CallToolResult(ToolStreamingText){ .content = &content }); try server.sendResult(id, protocol.CallToolResult(ToolStreamingText){ .content = &content });
} }
@@ -210,9 +216,12 @@ fn handleEvaluate(server: *Server, arena: std.mem.Allocator, id: std.json.Value,
if (args.url) |url| { if (args.url) |url| {
try performGoto(server, url, id); try performGoto(server, url, id);
} }
const page = server.session.currentPage() orelse {
return server.sendError(id, .PageNotLoaded, "Page not loaded");
};
var ls: js.Local.Scope = undefined; var ls: js.Local.Scope = undefined;
server.page.js.localScope(&ls); page.js.localScope(&ls);
defer ls.deinit(); defer ls.deinit();
var try_catch: js.TryCatch = undefined; var try_catch: js.TryCatch = undefined;
@@ -247,7 +256,12 @@ fn parseArguments(comptime T: type, arena: std.mem.Allocator, arguments: ?std.js
} }
fn performGoto(server: *Server, url: [:0]const u8, id: std.json.Value) !void { fn performGoto(server: *Server, url: [:0]const u8, id: std.json.Value) !void {
_ = server.page.navigate(url, .{ const session = server.session;
if (session.page != null) {
session.removePage();
}
const page = try session.createPage();
page.navigate(url, .{
.reason = .address_bar, .reason = .address_bar,
.kind = .{ .push = null }, .kind = .{ .push = null },
}) catch { }) catch {