Skip to content

mcp: fold Server.Session onto lp.ToolSession, bracket every isolate use - #3312

Merged
karlseguin merged 7 commits into
mainfrom
tool-session
Aug 28, 2026
Merged

mcp: fold Server.Session onto lp.ToolSession, bracket every isolate use#3312
karlseguin merged 7 commits into
mainfrom
tool-session

Conversation

@arrufat

@arrufat arrufat commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Follow-up deferred from #3096: the c-api branch defines a ToolSession inline in lightpanda.zig for the Browser/Session/Notification/node Registry quartet, and mcp/Server.zig carries its own copy of the same init/teardown sequence. Landing the shared type on main first, so the c-api branch only drops its inline copy when it merges (same init/deinit/enterIsolate/exitIsolate API).

  • src/ToolSession.zig: the quartet with its ordering-sensitive init/teardown and isolate enter/exit, re-exported as lp.ToolSession.
  • mcp/Server.zig: no more Session wrapper. sessions maps straight to *lp.ToolSession, the id is only the map key.
  • Every isolate use is bracketed now, stdio included. Before, stdio kept its isolate permanently entered and only HTTP parked between requests, so a handler touching page JS without enterIsolate passed the stdio-mode tests (56 of 57) and only failed under --port. park_isolates survives as multi_session, which is what it actually gated: the session tools need a transport that routes by Mcp-Session-Id.
  • Three tests that reach into the page directly (registry.register, frame.js) bracket the isolate; register creates V8 handles.
  • A direct ToolSession test: two sessions on one thread, entered only around their own use, separate JS globals, balanced teardown. That is what c_api.zig relies on without Server in the loop.

Not in this PR: Agent.zig, see #3313 (stacked on this one).

Verified: full suite (1282/1282); lightpanda mcp over stdio with goto, evaluate, a refused session_new, clean exit.

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
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
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
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
agent: fold the browser quartet onto lp.ToolSession

@karlseguin karlseguin left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

needs rebase

Comment thread src/ToolSession.zig Outdated
@karlseguin
karlseguin merged commit eded004 into main Aug 28, 2026
26 checks passed
@karlseguin
karlseguin deleted the tool-session branch August 28, 2026 23:02
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 28, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants