Repository navigation
feat: stdio MCP client and managed Playwright tier (browser tiers phase 1, PR 2) - #123
Open
chaodu-agent wants to merge 1 commit into
Open
chaodu-agent wants to merge 1 commit into
chaodu-agent wants to merge 1 commit into
Conversation
…se 1, PR 2) Adds StdioMCPClient (newline-delimited JSON-RPC, 8 MiB line cap, per-request timeouts), ManagedProcess (posix_spawn, own process group, allow-list env, 64 KiB in-memory stderr ring that is only ever classified, never logged, quit-time group termination), ManagedPlaywrightTransport (lazy start, backoff 1/2/4..30 s, latch after 5 failures in 2 min, Retry) and the managed mode of PlaywrightBackend. The install location is configuration (--playwright-root, default under Application Support); with nothing installed the router falls back and sys_info/menu say so. No listener. Reporting: sys_info gains a browser block and one text line; the menu's Browser tools row shows Playwright ready / not installed from the last known state, never starting the child to draw a menu. Review follow-ups from #122 in this PR: PortOwnerProbe takes the validated loopback endpoint (family and address) and is tested with simultaneous IPv4 and IPv6 listeners; healthCheck pings the live session with a 2 s timeout instead of re-running initialize, and DELETEs a session it replaces; port range check; repeated --upstream browser= rejected; lazy router status calls; tool list and owner check caching; pruned guard map; bracketed IPv6 Host header. Tested on macmini (macOS 15.7) in /private/tmp: swift build --build-tests, swift test (583 tests, 0 failures, 2 pre-existing skips), the new suites re-run alone (68 tests, 0 failures), swift build -c release, flag checks (exit 64 for repeated browser upstream, port 70000, relative root), scripts/check-no-private-identifiers.sh, test-pw-mcp-check.sh, test-name-consistency.sh.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 1, PR 2 of the browser tiers design (ADR, requirements). Follows #121.
What was built
StdioMCPClient: MCP stdio framing (newline-delimited UTF-8 JSON, CRLF tolerated, 8 MiB line cap, non-JSON lines counted and dropped), per-request timeouts (initialize 15 s, health ping 2 s, calls 90 s,notifications/cancelledon timeout). A closed channel fails every pending request withplaywright_exited. All errors are fixed classes.ManagedProcess:posix_spawnchild that leads its own process group, allow-list environment (HOME,TMPDIR,LANG,PATH), only fds 0/1/2 inherited, default signal dispositions. Stderr goes to a 64 KiB in-memory ring with no accessor; it is only classified into app-defined classes (profile_locked,executable_not_found, ...). Nothing from stderr is logged, persisted or returned. Termination: close stdin, 1 s, SIGTERM the group, 3 s, SIGKILL, plus a group sweep when the leader exits. A registry stops all groups onatexitand on SIGTERM (menu Quit ends inexit).ManagedPlaywrightTransport(tier P): starts on demand, restarts lazily (no timer, so Chromium is never launched unasked) with backoff 1, 2, 4 ... 30 s, latchesplaywright_unavailableafter 5 failures in 2 min untilretry(). Exit lines arebrowser: playwright_exit class=<class> status=<n>only.--playwright-root <dir>(absolute), default~/Library/Application Support/oab-computer/playwright.install.json(version,chromiumExecutable) plusnode/bin/nodeandpkg/node_modules/@playwright/mcp/cli.jsunder<root>/<version>/; paths are checked to stay inside that directory. Launch vector is exactly R7.4 with no--port,--hostor--allowed-hosts. This PR never writesinstall.json; there is no download or installer.auto; with nothing installed the backend reportsneedsSetup("playwright_not_installed"),tools/listhas nobrowser_*, and a call says "Playwright is not installed." External listener rules are unchanged (--upstream browser=still requires the operator flag and replaces the managed backend).Reporting (R1.6, R5.9)
Done in this PR (#121 deferred it).
sys_infogainsstructuredContent.browser(preference,active, per backendstate,class,modemanaged/external, owner name and uid) and one text line such asbrowser tools: playwright not installed. No URLs, titles or page text. The menu's Browser tools row readsBrowser tools: Playwright ready/Playwright not installed; with an external upstream the existing address row is kept and· Playwright external (<owner>)is appended. Both readpeekStatus()(last known state), so drawing a menu or runningsys_infonever starts the child. Backends implementpeekStatus,mode,ownerSummary.#122 items included (issue left untouched)
PortOwnerProbe.owner(of:)takes the validated endpoint (family + loopback address). The libproc match checks the socket's local address and family (v4 loopback/wildcard, v6 loopback/wildcard, dual-stack wildcard). Tested with real simultaneous v4 and v6 listeners: same owner, only v4 open, and different owning processes.healthCheckpings a live session with a 2 s timeout instead of re-runninginitialize; a rejected session (400) isDELETEd before the newinitialize; health and the DELETE use 2 s even with a 90 s client timeout.UInt16(...)trap above 65535); repeated--upstream browser=exits 64; router status calls are lazy (pinned preference asks one backend); owner check success cached 2 s, tool list cached 30 s and the last list kept on a transient error;lastmap pruned (30 min, 1024 cap); IPv6 Host header is[::1]. I kept it without a port, as the IPv4 form sends today (the allowed-hosts list compares verbatim); Browser router phase 1 follow-ups from PR #121 review #122 suggested[::1]:<port>, and that part is still open pending V8. The case-insensitive tailscale scan was already case-insensitive (it lowercases the text).Not in this PR
Wizard page, on-demand download/install, the menu items Set up / Install / Retry / Remove / Prefer, Apple Events tier, Linux (#119). V12 (no listener from the pinned
@playwright/mcpand Chromium) is still a release blocker and was not run here since there is no real install to test.Tested
On macmini (macOS 15.7), in a temp dir under
/private/tmp(removed afterwards):swift build --build-tests, fullswift test(583 tests, 0 failures, 2 skips that were already there), the new suites repeated alone (68 tests, 0 failures),swift build -c release, flag checks (repeated browser upstream, port 70000 and a relative root all exit 64),scripts/check-no-private-identifiers.sh,scripts/test-pw-mcp-check.sh,scripts/test-name-consistency.sh. The managed child is exercised with a fake MCP stdio server (Tests/FakeMCPStdioServer, test target only) covering chunked and noisy output, EOF exit, SIGTERM-ignoring child, grandchild in the group, stderr canary never reaching logs/results/disk, crash mid-call. Not run:scripts/test-packaging.sh(known failure on macOS 27), a real@playwright/mcp, lz-macmini.