feat: playwright self-validation and general browser capability#53
Merged
feat: playwright self-validation and general browser capability#53
Conversation
Adds two new MCP tool servers that share one Chromium instance and one BrowserContext per query: - phantom_preview_page: one-call self-validation for /ui/<path> pages. Navigates headless Chromium, captures a full-page PNG, and bundles HTTP status, title, console messages, and failed network requests alongside the screenshot. Stubs window.EventSource in an init script so Phantom's live-reload SSE wiring cannot hold the page open past 'load'. - phantom-browser: embeds the full 21-tool @playwright/mcp surface in process via createConnection with a contextGetter, giving the agent browser_navigate, browser_click, browser_snapshot, browser_run_code, and the rest out of the box. Both share one Browser singleton launched lazily on first use and one BrowserContext minted per query with a short-lived phantom_session cookie (10 minute TTL, domain: localhost). The browser and context are torn down on SIGTERM via the existing graceful-shutdown registry. The contextGetter path is load-bearing: the naive createConnection hangs under Bun on Linux amd64 (traced across five probes in research 01b). browser.isolated: false is mandatory because SimpleBrowser.newContext() throws; the embed defers to our shared context instead. mcpServerFactories now accepts async factories. The per-query resolution path in AgentRuntime uses Promise.all so sync factories pass through as resolved promises without behavior change. Agent prompt gains two new guidance blocks: always validate after phantom_create_page, and prefer phantom_preview_page for self-validation while using browser_* directly for multi-step browsing. Dockerfile installs the --only-shell chromium build (~75 MiB saved over the full binary) with PLAYWRIGHT_BROWSERS_PATH set to a phantom-owned cache directory. playwright@1.59.1 and @playwright/mcp@0.0.70 are pinned exactly because contextGetter and the capability surface have shifted between point releases already. Research: local/2026-04-12-phantom-ui-chapter/research/01-playwright-self-validation.md Research: local/2026-04-12-phantom-ui-chapter/research/01b-playwright-expanded/implementation-plan.md Research: local/2026-04-12-phantom-ui-chapter/research/01b-playwright-expanded/findings.md Test count: 931 -> 940 pass (+9) plus 6 opt-in integration tests skipped by default. Zero regressions. lint and typecheck clean. Docker image delta: +996 MiB (914 MiB -> 1.91 GiB).
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 41369cfda9
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
src/ui/preview.ts
Outdated
Comment on lines
+59
to
+60
| if (currentContext) return currentContext; | ||
| if (currentContextPromise) return currentContextPromise; |
There was a problem hiding this comment.
This function always returns the same BrowserContext, but that context only receives a preview session cookie once (minted by createPreviewSession, which now expires after 10 minutes). After the token expires, /ui/* requests become unauthenticated and phantom_preview_page/browser_* calls against local UI pages will start landing on login/401 responses until process restart. Rotating the context (or re-minting/re-injecting the cookie) per query is needed to keep the new preview capability working beyond the first 10 minutes.
Useful? React with 👍 / 👎.
src/ui/preview.ts
Outdated
Comment on lines
+46
to
+48
| browserPromise = (async () => { | ||
| const b = await chromium.launch({ | ||
| headless: true, |
There was a problem hiding this comment.
If chromium.launch(...) throws once (e.g., transient resource error), browserPromise remains set to that rejected promise, so every later call immediately fails and the preview/browser tools cannot recover without restarting the process. The cached promise should be cleared on failure so a subsequent call can retry launching Chromium.
Useful? React with 👍 / 👎.
Surgical fix pass on top of 41369cf in response to the internal review at local/2026-04-12-phantom-ui-chapter/review/01-playwright-review.md and Codex PR comments on #53. Fourteen acceptance criteria met. Preview correctness (src/ui/preview.ts): - F5: cookie path reverted from / to /ui, matching the magic-link posture in src/ui/serve.ts (buildPreviewCookie helper extracted). - F6 / Codex P1: rotate the preview session cookie on every warm getOrCreatePreviewContext call once Date.now() is inside the 2 minute safety margin before the 10 minute TTL. Uses addCookies in place so the cached BrowserContext stays the same instance. - F7: shuttingDown module flag set in closePreviewResources and guarded at the top of both getOrCreateBrowser and getOrCreatePreviewContext. Concurrent tool calls during SIGTERM now throw cleanly instead of spawning a resurrected Chromium. - F8: null currentContext in the page.close() catch block so the next call recreates a fresh context instead of handing back a dead one. - F9: replace window.EventSource = undefined with Reflect.deleteProperty(globalThis, "EventSource"). Covers the 'in' operator and drops a TypeScript widening. - Codex P2: try/finally pattern around browserPromise and currentContextPromise so a transient chromium.launch error no longer permanently poisons the cache. Subsequent calls retry cleanly. Browser MCP (src/ui/browser-mcp.ts): - F17: header note 3 rewritten to reflect the actual on-disk layout (top-level hoisted playwright-core@1.60.0-alpha vs. nested playwright/node_modules/playwright-core@1.59.1; no nested playwright-core under @playwright/mcp/node_modules). Cites the integration test that verifies the cross-version BrowserContext boundary end to end. Tests: - F1: preview.integration.test.ts rewritten to drive the real phantom_preview_page handler through a Client + InMemoryTransport pair. Asserts the bundled {image, text} result shape, base64 round-trip, status, title, console capture, and requestfailed capture. - F2 + F4: browser-mcp.integration.test.ts extended with a real Client.listTools() call asserting exactly 21 tools by name and a real browser_navigate round-trip across the cross-version BrowserContext boundary. - New preview-correctness.test.ts with 8 unit tests covering launch-failure recovery, shuttingDown, cookie rotation threshold, cookie path, and the F8 invariant. Uses spyOn(chromium, "launch") plus a fake Browser / BrowserContext so no real Chromium is spawned. Docs: - F3 + F11: Dockerfile image cost breakdown comment expanded to record the 996 MiB delta split: ~327 MB headless shell, ~91 MB fonts, ~500+ MB X11/GTK shared libs. Documents why --only-shell cannot trim more. Test count: 940 pass + 6 skip baseline -> 948 pass + 10 skip after (strictly greater, zero regressions). bun run lint and bun run typecheck both clean. PHANTOM_INTEGRATION=1 integration run on macOS arm64 passes 14/14 with real Chromium. Deferred per fix brief anti-patterns: F10 (v1.1 error-path screenshot), F11 (afterEach scope narrowing, pre-existing), F12 (graceful.ts tasks.reverse, out of scope).
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
1 participant
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.
Summary
Closes both UI gaps in one change: the agent can now self-validate pages it generates via
phantom_create_page, and it has the full first-party@playwright/mcp21-tool browsing surface always available. Both tools share one Chromium Browser and one per-query BrowserContext.What changed
phantom_preview_page(src/ui/preview.ts): custom in-process MCP tool. Navigates headless Chromium to a/ui/<path>, captures a full-page PNG, bundles HTTP status, title, console messages, and failed network requests into one multimodal result. Stubswindow.EventSourcein an init script so Phantom's SSE live-reload wiring cannot hold the page open pastload.phantom-browser(src/ui/browser-mcp.ts): embeds@playwright/mcpviacreateConnection(config, contextGetter). Shares the sameBrowserContextasphantom_preview_pageviagetOrCreatePreviewContext()so the agent can mix tools within a single query.browser.isolated: falseis mandatory for the contextGetter path; documented inline.src/agent/runtime.ts):mcpServerFactoriesnow accepts() => McpServerConfig | Promise<McpServerConfig>. Resolution wrapped inPromise.all. Existing sync factories pass through unchanged.src/index.ts):closePreviewResources()wired into the existingonShutdown(...)registry next to Scheduler and HTTP server.src/agent/prompt-assembler.ts): two new guidance blocks (self-validate every UI page, general browser capability). Explicit warning againstbrowser_run_codeon external origins.bunx playwright install --with-deps --only-shell chromiumadded after the node_modules copy, withPLAYWRIGHT_BROWSERS_PATH=/home/phantom/.cache/ms-playwrightand ownership chown tophantom:phantom.package.json: exact-pinnedplaywright@1.59.1and@playwright/mcp@0.0.70. No caret ranges because the contextGetter API has moved between point releases.Research references
local/2026-04-12-phantom-ui-chapter/research/01-playwright-self-validation.md(auth model, SSE workaround, Docker plan, bundled-result design)local/2026-04-12-phantom-ui-chapter/research/01b-playwright-expanded/implementation-plan.md(file-by-file changes, Dockerfile diff, prompt blocks, test plan, acceptance criteria)local/2026-04-12-phantom-ui-chapter/research/01b-playwright-expanded/findings.md(createConnection contextGetter workaround, five-probe trace, 01b benchmark)Test count
PHANTOM_INTEGRATION=1: 3 pass in 530ms with real Chromium on macOS arm64bun run lintclean,bun run typecheckclean, zero regressions.Docker image delta
--with-deps)The 8 GiB container RAM cap from PR #52 comfortably absorbs the larger image.
Test plan
bun installinstalls pinned versionsbun run typecheckzero errorsbun run lintzero warningsbun test940 pass, 0 fail, 6 opt-in skipPHANTOM_INTEGRATION=1 bun test src/ui/__tests__/*.integration.test.tspasses with real Chromiumdocker build -f Dockerfile -t phantom-playwright:v1 .succeeds/home/phantom/.cache/ms-playwright/chromium_headless_shell-*owned byphantom:phantomphantom_preview_pagein a Slack flow and the screenshot looks rightHandoff
Full handoff report with decisions and open questions:
local/2026-04-12-phantom-ui-chapter/handoffs/01-playwright-handoff.md