Skip to content

Commit 40f4e29

Browse files
authored
fix(mcp): stabilize Playwright CDP guard generation (#323)
* fix(mcp): keep Playwright MCP handshake nonblocking * fix(app): restore auth terminal checks * fix(lib): restore grok device auth command * fix(mcp): address coderabbit review comments * fix(mcp): stabilize Playwright CDP guard generation Copy the Playwright CDP guard as a generated build-context file instead of relying on a Dockerfile heredoc, and fall back to socat when the guard exits during browser startup.
1 parent 1aee809 commit 40f4e29

19 files changed

Lines changed: 162 additions & 58 deletions

File tree

packages/app/src/docker-git/cli/usage.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,6 @@ Container runtime env (set via .orch/env/project.env):
102102
MCP_PLAYWRIGHT_ISOLATED=1|0 Isolated browser contexts; default 0 shares the VNC session
103103
MCP_PLAYWRIGHT_CDP_GUARD=1|0 Guard CDP so MCP cannot close/crash shared Chromium (default: 1)
104104
MCP_PLAYWRIGHT_BLOCK_BROWSER_CLOSE=1|0 Block destructive Browser.close/crash CDP methods (default: 1)
105-
MCP_PLAYWRIGHT_CDP_ENDPOINT=http://... Override CDP endpoint (default: http://127.0.0.1:9223)
106105
MCP_PLAYWRIGHT_CDP_TIMEOUT=<ms> CDP connect timeout passed to Playwright MCP (default: 60000)
107106
MCP_PLAYWRIGHT_READY_ATTEMPTS=<n> Startup readiness attempts before disabling broken MCP (default: 60)
108107
MCP_PLAYWRIGHT_READY_DELAY=<seconds> Delay between startup readiness attempts (default: 1)

packages/app/src/lib/core/templates-entrypoint/base.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ CODEX_AUTO_UPDATE="\${CODEX_AUTO_UPDATE:-1}"
4040
AGENT_MODE="\${AGENT_MODE:-}"
4141
AGENT_AUTO="\${AGENT_AUTO:-}"
4242
MCP_PLAYWRIGHT_ENABLE="\${MCP_PLAYWRIGHT_ENABLE:-${config.enableMcpPlaywright ? "1" : "0"}}"
43-
MCP_PLAYWRIGHT_CDP_ENDPOINT="\${MCP_PLAYWRIGHT_CDP_ENDPOINT:-}"
4443
MCP_PLAYWRIGHT_ISOLATED="\${MCP_PLAYWRIGHT_ISOLATED:-0}"
4544
MCP_PLAYWRIGHT_CDP_GUARD="\${MCP_PLAYWRIGHT_CDP_GUARD:-1}"
4645
MCP_PLAYWRIGHT_BLOCK_BROWSER_CLOSE="\${MCP_PLAYWRIGHT_BLOCK_BROWSER_CLOSE:-1}"

packages/app/src/lib/core/templates-entrypoint/codex.ts

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -98,10 +98,6 @@ EOF
9898
chown 1000:1000 "$CODEX_CONFIG_FILE" || true
9999
fi
100100
101-
if [[ -z "$MCP_PLAYWRIGHT_CDP_ENDPOINT" ]]; then
102-
MCP_PLAYWRIGHT_CDP_ENDPOINT="http://127.0.0.1:9223"
103-
fi
104-
105101
# Replace the docker-git Playwright block to allow upgrades via --force without manual edits.
106102
if grep -q "^\[mcp_servers\.playwright" "$CODEX_CONFIG_FILE" 2>/dev/null; then
107103
awk '

packages/app/src/lib/core/templates.ts

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,11 @@ import { renderEntrypoint } from "./templates-entrypoint.js"
55
import { type ComposeResourceLimits, renderDockerCompose } from "./templates/docker-compose.js"
66
import { renderDockerfile } from "./templates/dockerfile.js"
77
import { renderPlaywrightBrowserRuntime } from "./templates/playwright-browser-runtime.js"
8-
import { renderPlaywrightBrowserDockerfile, renderPlaywrightStartExtra } from "./templates/playwright.js"
8+
import {
9+
renderPlaywrightBrowserDockerfile,
10+
renderPlaywrightCdpGuard,
11+
renderPlaywrightStartExtra
12+
} from "./templates/playwright.js"
913

1014
export type FileSpec =
1115
| { readonly _tag: "File"; readonly relativePath: string; readonly contents: string; readonly mode?: number }
@@ -52,6 +56,12 @@ export const planFiles = (
5256
const maybePlaywrightFiles = config.enableMcpPlaywright
5357
? ([
5458
{ _tag: "File", relativePath: "Dockerfile.browser", contents: renderPlaywrightBrowserDockerfile() },
59+
{
60+
_tag: "File",
61+
relativePath: "docker-git-cdp-guard",
62+
contents: renderPlaywrightCdpGuard(),
63+
mode: 0o755
64+
},
5565
{
5666
_tag: "File",
5767
relativePath: "mcp-playwright-start-extra.sh",

packages/app/src/lib/core/templates/docker-compose.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -164,7 +164,7 @@ const buildPlaywrightFragments = (
164164
maybeDependsOn: "",
165165
maybeDockerSocketMount: renderOptionalDockerSocketMount(options.enableLocalDockerSocket),
166166
maybePlaywrightEnv:
167-
` MCP_PLAYWRIGHT_ENABLE: "1"\n MCP_PLAYWRIGHT_CDP_ENDPOINT: "http://127.0.0.1:9223"\n DOCKER_GIT_PROJECT_CONTAINER_NAME: "${config.containerName}"\n DOCKER_GIT_BROWSER_CONTAINER_NAME: "${browserContainerName}"\n DOCKER_GIT_BROWSER_IMAGE_NAME: "${browserImageName}"\n DOCKER_GIT_BROWSER_VOLUME_NAME: "${browserVolumeName}"\n${
167+
` MCP_PLAYWRIGHT_ENABLE: "1"\n DOCKER_GIT_PROJECT_CONTAINER_NAME: "${config.containerName}"\n DOCKER_GIT_BROWSER_CONTAINER_NAME: "${browserContainerName}"\n DOCKER_GIT_BROWSER_IMAGE_NAME: "${browserImageName}"\n DOCKER_GIT_BROWSER_VOLUME_NAME: "${browserVolumeName}"\n${
168168
renderBrowserLimitEnv("DOCKER_GIT_BROWSER_CPU_LIMIT", resourceLimits?.cpuLimit)
169169
}${renderBrowserLimitEnv("DOCKER_GIT_BROWSER_RAM_LIMIT", resourceLimits?.ramLimit)}`,
170170
maybeBrowserVolume: ` ${browserVolumeName}:`

packages/app/src/lib/core/templates/dockerfile.ts

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -170,17 +170,14 @@ for arg in "$@"; do
170170
esac
171171
done
172172
173-
CDP_ENDPOINT="\${MCP_PLAYWRIGHT_CDP_ENDPOINT:-}"
174-
if [[ -z "$CDP_ENDPOINT" ]]; then
175-
CDP_ENDPOINT="http://127.0.0.1:9223"
176-
fi
173+
CDP_ENDPOINT="http://127.0.0.1:9223"
177174
178175
# CHANGE: keep MCP initialize independent from nested browser readiness
179176
# WHY: Codex starts MCP servers during boot; blocking here closes stdio before initialize when CDP is slow.
180177
# QUOTE(issue-319): "handshaking with MCP server failed: connection closed: initialize response"
181178
# REF: issue-319
182179
# SOURCE: https://playwright.dev/mcp/configuration/options
183-
# FORMAT THEOREM: guarded_cdp(endpoint) -> mcp_stdio_ready_before_browser_connection
180+
# FORMAT THEOREM: guarded_cdp(fixed_nested_browser_endpoint) -> mcp_stdio_ready_before_browser_connection
184181
# PURITY: SHELL
185182
# INVARIANT: guarded mode never exits before handing stdio to playwright-mcp
186183
# COMPLEXITY: O(1)
@@ -194,7 +191,8 @@ if [[ "\${MCP_PLAYWRIGHT_ISOLATED:-0}" == "1" ]]; then
194191
EXTRA_ARGS+=(--isolated)
195192
fi
196193
197-
# Guarded endpoints are stable HTTP CDP endpoints. Passing the HTTP URL lets Playwright MCP
194+
# The guarded endpoint is the nested browser opened by docker-git Open browser.
195+
# Passing the fixed HTTP URL lets Playwright MCP
198196
# re-resolve /json/version instead of pinning itself to one stale /devtools/browser/<id>.
199197
if [[ "$MCP_PLAYWRIGHT_CDP_GUARD" == "1" ]]; then
200198
exec playwright-mcp --cdp-endpoint "$CDP_ENDPOINT" --cdp-timeout "$MCP_PLAYWRIGHT_CDP_TIMEOUT" "\${EXTRA_ARGS[@]}" "$@"
@@ -239,8 +237,8 @@ RUN chmod +x /usr/local/bin/docker-git-playwright-mcp`
239237
const renderDockerfilePlaywrightRuntime = (config: TemplateConfig): string =>
240238
config.enableMcpPlaywright
241239
? `# docker-git nested Playwright browser runtime context
242-
COPY Dockerfile.browser mcp-playwright-start-extra.sh docker-git-browser-runtime.sh /opt/docker-git/browser/
243-
RUN chmod +x /opt/docker-git/browser/mcp-playwright-start-extra.sh /opt/docker-git/browser/docker-git-browser-runtime.sh`
240+
COPY Dockerfile.browser docker-git-cdp-guard mcp-playwright-start-extra.sh docker-git-browser-runtime.sh /opt/docker-git/browser/
241+
RUN chmod +x /opt/docker-git/browser/docker-git-cdp-guard /opt/docker-git/browser/mcp-playwright-start-extra.sh /opt/docker-git/browser/docker-git-browser-runtime.sh`
244242
: ""
245243

246244
/**

packages/app/src/lib/core/templates/playwright-browser-runtime.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ docker_git_disable_playwright_mcp() {
3434
}
3535
3636
docker_git_playwright_cdp_endpoint() {
37-
printf '%s\\n' "\${MCP_PLAYWRIGHT_CDP_ENDPOINT:-http://127.0.0.1:9223}"
37+
printf '%s\\n' "http://127.0.0.1:9223"
3838
}
3939
4040
docker_git_fetch_playwright_cdp_version() {

packages/app/src/lib/core/templates/playwright.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,7 @@ export const renderPlaywrightBrowserDockerfile = (): string =>
77
RUN apk add --no-cache bash procps socat nodejs npm python3 net-tools
88
RUN npm install --omit=dev --prefix /opt/docker-git-cdp-guard ws@8.18.3
99
10-
RUN cat <<'EOF' > /usr/local/bin/docker-git-cdp-guard
11-
${cdpGuardScript}
12-
EOF
10+
COPY docker-git-cdp-guard /usr/local/bin/docker-git-cdp-guard
1311
RUN chmod +x /usr/local/bin/docker-git-cdp-guard
1412
1513
COPY mcp-playwright-start-extra.sh /usr/local/bin/mcp-playwright-start-extra.sh
@@ -26,10 +24,10 @@ const http = require("node:http");
2624
const { URL } = require("node:url");
2725
const { WebSocket, WebSocketServer } = require("/opt/docker-git-cdp-guard/node_modules/ws");
2826
29-
const upstreamHost = process.env.MCP_PLAYWRIGHT_UPSTREAM_CDP_HOST || "127.0.0.1";
30-
const upstreamPort = Number.parseInt(process.env.MCP_PLAYWRIGHT_UPSTREAM_CDP_PORT || "9222", 10);
31-
const listenHost = process.env.MCP_PLAYWRIGHT_CDP_GUARD_HOST || "0.0.0.0";
32-
const listenPort = Number.parseInt(process.env.MCP_PLAYWRIGHT_CDP_GUARD_PORT || "9223", 10);
27+
const upstreamHost = "127.0.0.1";
28+
const upstreamPort = 9222;
29+
const listenHost = "0.0.0.0";
30+
const listenPort = 9223;
3331
const blockedMethods = new Set(["Browser.close", "Browser.crash", "Browser.crashGpuProcess"]);
3432
3533
const log = (message) => process.stderr.write("[docker-git-cdp-guard] " + message + "\n");
@@ -244,6 +242,8 @@ server.listen(listenPort, listenHost, () => {
244242
});
245243
`
246244

245+
export const renderPlaywrightCdpGuard = (): string => cdpGuardScript
246+
247247
export const renderPlaywrightStartExtra = (): string =>
248248
`#!/bin/sh
249249
set -eu
@@ -254,11 +254,22 @@ rm -f /data/SingletonLock /data/SingletonCookie /data/SingletonSocket || true
254254
# Wait for chromium/x11vnc/noVNC to come up
255255
sleep 2
256256
257+
start_cdp_fallback() {
258+
socat TCP-LISTEN:9223,fork,reuseaddr TCP:127.0.0.1:9222 >/var/log/socat-9223.log 2>&1 &
259+
}
260+
257261
# CDP guard: expose 9223 on the docker network and block browser-level destructive CDP methods
258262
if [ "\${MCP_PLAYWRIGHT_CDP_GUARD:-1}" = "1" ]; then
259263
docker-git-cdp-guard >/var/log/docker-git-cdp-guard.log 2>&1 &
264+
guard_pid="$!"
265+
sleep 1
266+
if ! kill -0 "$guard_pid" 2>/dev/null; then
267+
echo "docker-git-cdp-guard exited during startup; falling back to socat" >&2
268+
sed -n '1,120p' /var/log/docker-git-cdp-guard.log 2>/dev/null >&2 || true
269+
start_cdp_fallback
270+
fi
260271
else
261-
socat TCP-LISTEN:9223,fork,reuseaddr TCP:127.0.0.1:9222 >/var/log/socat-9223.log 2>&1 &
272+
start_cdp_fallback
262273
fi
263274
264275
# Optional VNC password disabling (useful if you publish VNC/noVNC ports)

packages/app/tests/docker-git/core-templates.test.ts

Lines changed: 23 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,21 +46,43 @@ describe("app planFiles", () => {
4646
const files = planFiles(makeTemplateConfig({ enableMcpPlaywright: true }))
4747
const filePaths = getGeneratedFilePaths(files)
4848
const runtime = getGeneratedFile(files, "docker-git-browser-runtime.sh")
49+
const cdpGuard = getGeneratedFile(files, "docker-git-cdp-guard")
50+
const browserDockerfile = getGeneratedFile(files, "Dockerfile.browser")
51+
const startExtra = getGeneratedFile(files, "mcp-playwright-start-extra.sh")
4952
const dockerfile = getGeneratedFile(files, "Dockerfile")
5053

5154
expect(filePaths).toContain("Dockerfile.browser")
55+
expect(filePaths).toContain("docker-git-cdp-guard")
5256
expect(filePaths).toContain("mcp-playwright-start-extra.sh")
5357
expect(filePaths).toContain("docker-git-browser-runtime.sh")
58+
expect(cdpGuard.mode).toBe(0o755)
59+
expect(cdpGuard.contents).toContain("#!/usr/bin/env node")
60+
expect(cdpGuard.contents).toContain("const upstreamHost = \"127.0.0.1\";")
61+
expect(cdpGuard.contents).toContain("const upstreamPort = 9222;")
62+
expect(cdpGuard.contents).toContain("const listenHost = \"0.0.0.0\";")
63+
expect(cdpGuard.contents).toContain("const listenPort = 9223;")
64+
expect(cdpGuard.contents).not.toContain("MCP_PLAYWRIGHT_UPSTREAM_CDP_HOST")
65+
expect(cdpGuard.contents).not.toContain("MCP_PLAYWRIGHT_CDP_GUARD_PORT")
66+
expect(cdpGuard.contents).toContain("Browser.close")
67+
expect(browserDockerfile.contents).toContain("COPY docker-git-cdp-guard /usr/local/bin/docker-git-cdp-guard")
68+
expect(browserDockerfile.contents).not.toContain("RUN cat <<'EOF' > /usr/local/bin/docker-git-cdp-guard")
69+
expect(startExtra.contents).toContain("guard_pid=\"$!\"")
70+
expect(startExtra.contents).toContain("falling back to socat")
71+
expect(startExtra.contents).toContain("socat TCP-LISTEN:9223,fork,reuseaddr TCP:127.0.0.1:9222")
5472
expect(runtime.mode).toBe(0o755)
5573
expect(runtime.contents).toContain("if [[ \"${MCP_PLAYWRIGHT_ENABLE:-0}\" != \"1\" ]]; then")
74+
expect(runtime.contents).toContain(String.raw`printf '%s\n' "http://127.0.0.1:9223"`)
75+
expect(runtime.contents).not.toContain("printf '%s\\n' \"${MCP_PLAYWRIGHT_CDP_ENDPOINT:-http://127.0.0.1:9223}\"")
5676
expect(runtime.contents).toContain("docker_git_wait_for_playwright_cdp()")
5777
expect(runtime.contents).toContain("MCP_PLAYWRIGHT_ENABLE=0")
5878
expect(runtime.contents).not.toContain("\\${MCP_PLAYWRIGHT_ENABLE:-0}")
5979
expect(dockerfile.contents).toContain(
60-
"COPY Dockerfile.browser mcp-playwright-start-extra.sh docker-git-browser-runtime.sh /opt/docker-git/browser/"
80+
"COPY Dockerfile.browser docker-git-cdp-guard mcp-playwright-start-extra.sh docker-git-browser-runtime.sh /opt/docker-git/browser/"
6181
)
6282
expect(dockerfile.contents).toContain("ARG PLAYWRIGHT_MCP_VERSION=0.0.75")
6383
expect(dockerfile.contents).toContain("RUN npm install -g \"@playwright/mcp@${PLAYWRIGHT_MCP_VERSION}\"")
84+
expect(dockerfile.contents).toContain("CDP_ENDPOINT=\"http://127.0.0.1:9223\"")
85+
expect(dockerfile.contents).not.toContain("CDP_ENDPOINT=\"${MCP_PLAYWRIGHT_CDP_ENDPOINT:-}\"")
6486
expect(dockerfile.contents).toContain("MCP_PLAYWRIGHT_CDP_TIMEOUT=\"${MCP_PLAYWRIGHT_CDP_TIMEOUT:-60000}\"")
6587
expect(runtime.contents).toContain("invalid MCP_PLAYWRIGHT_READY_ATTEMPTS")
6688
expect(runtime.contents).toContain("while (( attempt <= attempts )); do")

packages/lib/src/core/templates-entrypoint/base.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ CODEX_AUTO_UPDATE="\${CODEX_AUTO_UPDATE:-1}"
4040
AGENT_MODE="\${AGENT_MODE:-}"
4141
AGENT_AUTO="\${AGENT_AUTO:-}"
4242
MCP_PLAYWRIGHT_ENABLE="\${MCP_PLAYWRIGHT_ENABLE:-${config.enableMcpPlaywright ? "1" : "0"}}"
43-
MCP_PLAYWRIGHT_CDP_ENDPOINT="\${MCP_PLAYWRIGHT_CDP_ENDPOINT:-}"
4443
MCP_PLAYWRIGHT_ISOLATED="\${MCP_PLAYWRIGHT_ISOLATED:-0}"
4544
MCP_PLAYWRIGHT_CDP_GUARD="\${MCP_PLAYWRIGHT_CDP_GUARD:-1}"
4645
MCP_PLAYWRIGHT_BLOCK_BROWSER_CLOSE="\${MCP_PLAYWRIGHT_BLOCK_BROWSER_CLOSE:-1}"

0 commit comments

Comments
 (0)