fix(mcp): stabilize Playwright CDP guard generation#323
Conversation
# Conflicts: # packages/app/src/docker-git/program-auth.ts # packages/lib/src/usecases/auth-grok-oauth.ts
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.
📝 WalkthroughSummary by CodeRabbitЗаметки о выпуске
WalkthroughИзвлечён cdpGuardScript в экспортируемую функцию и новый планируемый файл ChangesCDP Guard Feature Implementation
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 7✅ Passed checks (7 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app/src/lib/core/templates/playwright.ts`:
- Around line 257-259: The fallback function start_cdp_fallback hardcodes ports
(9223 -> 127.0.0.1:9222) which diverges from the guard when custom
MCP_PLAYWRIGHT_* env vars are set; update start_cdp_fallback to read and use the
same MCP_PLAYWRIGHT_CDP_GUARD_PORT (listen port) and
MCP_PLAYWRIGHT_UPSTREAM_HOST / MCP_PLAYWRIGHT_UPSTREAM_PORT (target host:port)
instead of literals, and apply the same change to the other fallback snippet at
the block around lines 271-273 so both guard and fallback use identical
MCP_PLAYWRIGHT_* values.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 48e62b38-5d3c-43cc-9f90-82818cfdf8bb
📒 Files selected for processing (9)
packages/app/src/lib/core/templates.tspackages/app/src/lib/core/templates/dockerfile.tspackages/app/src/lib/core/templates/playwright.tspackages/app/tests/docker-git/core-templates.test.tspackages/lib/src/core/templates.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/src/core/templates/playwright.tspackages/lib/tests/core/templates.test.tspackages/lib/tests/usecases/mcp-playwright.test.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Login context)
- GitHub Check: E2E (OpenCode)
- GitHub Check: E2E (Clone cache)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never useany,unknown,eslint-disable,ts-ignore, orastype assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through.exhaustive()pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects:Effect<Success, Error, Requirements>. Compose effects throughpipe()andEffect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example:Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including:@pure(true/false),@effect(required services),@invariant(mathematical invariants),@precondition,@postcondition,@complexity(time and space),@throwsNever (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...
Files:
packages/lib/src/core/templates.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/playwright.tspackages/app/src/lib/core/templates/playwright.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code:any,eslint-disable,ts-ignore,async/await, raw Promise chains (then/catch),Promise.all,try/catchfor logic control,console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects:Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.
Files:
packages/lib/src/core/templates.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/playwright.tspackages/app/src/lib/core/templates/playwright.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Files:
packages/lib/src/core/templates.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/playwright.tspackages/app/src/lib/core/templates/playwright.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce path traversal or writes outside intended project/container state directories
Files:
packages/lib/src/core/templates.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/playwright.tspackages/app/src/lib/core/templates/playwright.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Files:
packages/lib/src/core/templates.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/playwright.tspackages/app/src/lib/core/templates/playwright.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/lib/src/core/templates.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/playwright.tspackages/app/src/lib/core/templates/playwright.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example:fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example:Effect.provide(MockService), Effect.runPromise.
Files:
packages/app/tests/docker-git/core-templates.test.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.
Files:
packages/app/tests/docker-git/core-templates.test.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.ts
🔇 Additional comments (10)
packages/app/tests/docker-git/core-templates.test.ts (1)
49-51: LGTM!Also applies to: 55-65, 71-71
packages/lib/tests/core/templates.test.ts (1)
755-770: LGTM!Also applies to: 777-777, 780-790
packages/lib/tests/usecases/mcp-playwright.test.ts (1)
146-146: LGTM!Also applies to: 150-150, 154-154, 158-158, 161-167, 170-171
packages/lib/src/core/templates/playwright.ts (2)
256-258: Та же проблема, что и в app-версии fallback.Здесь повторяется жёсткая привязка к
9223 -> 127.0.0.1:9222, из-за чего fallback не следует кастомнымMCP_PLAYWRIGHT_*параметрам.Also applies to: 271-272
9-9: LGTM!Also applies to: 244-244
packages/app/src/lib/core/templates/playwright.ts (1)
10-10: LGTM!Also applies to: 245-245
packages/app/src/lib/core/templates.ts (1)
8-12: LGTM!Also applies to: 59-64
packages/lib/src/core/templates.ts (1)
11-15: LGTM!Also applies to: 71-76
packages/app/src/lib/core/templates/dockerfile.ts (1)
242-243: LGTM!packages/lib/src/core/templates/dockerfile.ts (1)
242-243: LGTM!
Remove CDP endpoint overrides so Playwright MCP always targets the docker-git managed browser exposed on localhost:9223, matching the browser opened from the UI. Keep guard and socat fallback on the same fixed route.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/app/src/lib/core/templates/playwright.ts`:
- Around line 257-272: The startup currently only sleeps 1s then checks the
guard process, which can miss slow startups; modify the guard startup block
around docker-git-cdp-guard (and the start_cdp_fallback call) to perform an
active readiness probe instead of a fixed sleep: after launching
docker-git-cdp-guard and capturing guard_pid, loop with a short sleep retrying a
curl (e.g. http://127.0.0.1:9223/json/version) until it returns a successful
HTTP response or a configurable timeout/retry count is reached, also break early
if kill -0 reports the process died; if the probe fails or the process exited,
log the guard log and call start_cdp_fallback. Keep references to
start_cdp_fallback and docker-git-cdp-guard/guard_pid so the existing fallback
behavior is preserved.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 93b37acf-f13f-4148-8c3d-d61659ebb7e0
📒 Files selected for processing (17)
packages/app/src/docker-git/cli/usage.tspackages/app/src/lib/core/templates-entrypoint/base.tspackages/app/src/lib/core/templates-entrypoint/codex.tspackages/app/src/lib/core/templates/docker-compose.tspackages/app/src/lib/core/templates/dockerfile.tspackages/app/src/lib/core/templates/playwright-browser-runtime.tspackages/app/src/lib/core/templates/playwright.tspackages/app/tests/docker-git/core-templates.test.tspackages/lib/src/core/templates-entrypoint/base.tspackages/lib/src/core/templates-entrypoint/codex.tspackages/lib/src/core/templates/docker-compose.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/src/core/templates/playwright-browser-runtime.tspackages/lib/src/core/templates/playwright.tspackages/lib/tests/core/templates.test.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/usecases/prepare-files.test.ts
💤 Files with no reviewable changes (5)
- packages/lib/src/core/templates-entrypoint/base.ts
- packages/app/src/docker-git/cli/usage.ts
- packages/app/src/lib/core/templates-entrypoint/base.ts
- packages/lib/src/core/templates-entrypoint/codex.ts
- packages/app/src/lib/core/templates-entrypoint/codex.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 900000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Lint
- GitHub Check: E2E (Browser command)
- GitHub Check: E2E (Clone auto-open SSH)
- GitHub Check: E2E (Login context)
- GitHub Check: E2E (Clone cache)
- GitHub Check: E2E (Runtime volumes + SSH)
- GitHub Check: E2E (OpenCode)
🧰 Additional context used
📓 Path-based instructions (8)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.{ts,tsx}: Implement Functional Core, Imperative Shell (FCIS) pattern: CORE layer contains only pure functions with immutable data and mathematical operations; SHELL layer isolates all effects (IO, network, database). Strict dependency direction: SHELL → CORE (never reverse).
Never useany,unknown,eslint-disable,ts-ignore, orastype assertions (except in rigorously justified cases with documentation). Always use exhaustive union type analysis through.exhaustive()pattern matching.
All external dependencies must be wrapped through typed interfaces and injected via Effect-TS Layer pattern. Never call external services directly from CORE functions.
Use monadic composition with Effect-TS for all effects:Effect<Success, Error, Requirements>. Compose effects throughpipe()andEffect.flatMap(). Implement dependency injection via Layer pattern. Handle errors without try/catch blocks.
All functions must be pure in the CORE layer: no side effects (logging, console output, IO operations, mutations). Separate all side effects into the SHELL layer.
Use exhaustive pattern matching with Effect.Match instead of switch statements. Example:Match.value(item).pipe(Match.when(...), Match.exhaustive).
Document all functions with comprehensive TSDoc including:@pure(true/false),@effect(required services),@invariant(mathematical invariants),@precondition,@postcondition,@complexity(time and space),@throwsNever (errors must be typed in Effect).
Use functional comment markers for code clarity: CHANGE (brief description), WHY (mathematical/architectural justification), QUOTE(ТЗ) (requirement citation), REF (RTM or message ID), SOURCE (external source with quote), FORMAT THEOREM (∀x ∈ Domain: P(x) → Q(f(x))), PURITY (CORE|SHELL), EFFECT (Effect type signature), INVARIANT (mathematical invariant), COMPLEXITY (time/space).
Define all external service dependencies as Context.Tag classes with fully typed methods returning Effect types. Example: `class Da...
Files:
packages/lib/tests/usecases/prepare-files.test.tspackages/lib/src/core/templates/docker-compose.tspackages/lib/src/core/templates/playwright-browser-runtime.tspackages/app/src/lib/core/templates/docker-compose.tspackages/app/src/lib/core/templates/playwright-browser-runtime.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/app/src/lib/core/templates/playwright.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/src/core/templates/playwright.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.test.{ts,tsx}: Implement property-based testing using fast-check for mathematical properties and invariants. Example:fc.property(fc.array(messageArbitrary), (messages) => isChronologicallySorted(sortMessagesByTimestamp(messages))).
Mock external dependencies in unit tests using Effect's testing utilities. Run tests without Effect runtime for speed. Example:Effect.provide(MockService), Effect.runPromise.
Files:
packages/lib/tests/usecases/prepare-files.test.tspackages/app/tests/docker-git/core-templates.test.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Forbidden constructs in CORE code:any,eslint-disable,ts-ignore,async/await, raw Promise chains (then/catch),Promise.all,try/catchfor logic control,console.*, switch statements (use Match with .exhaustive() instead)
All functions must use Effect-TS for composing effects:Effect<Success, Error, Requirements>. No direct async/await, Promise chains, or try/catch in product logic.
Functional comments must include: CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL), EFFECT signature for SHELL functions, INVARIANT, and COMPLEXITY.
All data mutations must use immutable patterns (ReadonlyArray, readonly properties, Object.freeze); mutation in SHELL only when absolutely necessary and documented.
Files:
packages/lib/tests/usecases/prepare-files.test.tspackages/lib/src/core/templates/docker-compose.tspackages/lib/src/core/templates/playwright-browser-runtime.tspackages/app/src/lib/core/templates/docker-compose.tspackages/app/src/lib/core/templates/playwright-browser-runtime.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/app/src/lib/core/templates/playwright.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/src/core/templates/playwright.ts
**/*.{test,spec}.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Property-based tests (fast-check) must verify mathematical invariants; unit tests must use Effect test utilities without async/await.
Files:
packages/lib/tests/usecases/prepare-files.test.tspackages/app/tests/docker-git/core-templates.test.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.ts
**/*.{sh,bash,py,js,ts,jsx,tsx,go,java,rb,php}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce command injection or unsafe shell/process execution with user-controlled input
Files:
packages/lib/tests/usecases/prepare-files.test.tspackages/lib/src/core/templates/docker-compose.tspackages/lib/src/core/templates/playwright-browser-runtime.tspackages/app/src/lib/core/templates/docker-compose.tspackages/app/src/lib/core/templates/playwright-browser-runtime.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/app/src/lib/core/templates/playwright.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/src/core/templates/playwright.ts
**/*.{py,js,ts,jsx,tsx,go,java,rb,php,sh,bash,c,cpp}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files introduce path traversal or writes outside intended project/container state directories
Files:
packages/lib/tests/usecases/prepare-files.test.tspackages/lib/src/core/templates/docker-compose.tspackages/lib/src/core/templates/playwright-browser-runtime.tspackages/app/src/lib/core/templates/docker-compose.tspackages/app/src/lib/core/templates/playwright-browser-runtime.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/app/src/lib/core/templates/playwright.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/src/core/templates/playwright.ts
**/*.{js,ts,jsx,tsx,py,java,go,rb,php,sh,bash,yml,yaml,json,env*,toml,cfg,config,dockerfile,dockerignore}
📄 CodeRabbit inference engine (Custom checks)
Fail if changed files expose credentials, tokens, private-keys, or PII in source, generated config, logs, or CI output
Files:
packages/lib/tests/usecases/prepare-files.test.tspackages/lib/src/core/templates/docker-compose.tspackages/lib/src/core/templates/playwright-browser-runtime.tspackages/app/src/lib/core/templates/docker-compose.tspackages/app/src/lib/core/templates/playwright-browser-runtime.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/app/src/lib/core/templates/playwright.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/src/core/templates/playwright.ts
**/*
⚙️ CodeRabbit configuration file
**/*: Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT.Перед выводами изучи README.md, другие *.md файлы, linked issues,
PR description, PR comments/discussion и релевантную кодовую базу.Сверь изменения с исходным ТЗ/спекой и обсуждением. Флагай любой уход
от спеки, недокументированное изменение поведения, отсутствие тестов
для заявленного поведения и security-риск. Если спека не видна,
попроси автора добавить ее в issue или PR description.Проверь решение с точки зрения формальной верификации: какие инварианты,
предусловия и постусловия можно доказать математически, а где доказуемость
слабая. Оцени решение с точки зрения теории игр: устойчивы ли стимулы,
нет ли выгодного обхода правил, и какое решение было бы сильнее.
Files:
packages/lib/tests/usecases/prepare-files.test.tspackages/lib/src/core/templates/docker-compose.tspackages/lib/src/core/templates/playwright-browser-runtime.tspackages/app/src/lib/core/templates/docker-compose.tspackages/app/src/lib/core/templates/playwright-browser-runtime.tspackages/app/tests/docker-git/core-templates.test.tspackages/app/src/lib/core/templates/dockerfile.tspackages/app/src/lib/core/templates/playwright.tspackages/lib/tests/usecases/mcp-playwright.test.tspackages/lib/tests/core/templates.test.tspackages/lib/src/core/templates/dockerfile.tspackages/lib/src/core/templates/playwright.ts
🔇 Additional comments (21)
packages/app/src/lib/core/templates/dockerfile.ts (1)
173-173: LGTM!Also applies to: 180-180, 194-195, 240-241
packages/lib/src/core/templates/dockerfile.ts (1)
173-173: LGTM!Also applies to: 180-180, 194-195, 240-241
packages/app/src/lib/core/templates/playwright-browser-runtime.ts (1)
37-37: LGTM!packages/lib/src/core/templates/playwright-browser-runtime.ts (1)
36-36: LGTM!packages/app/src/lib/core/templates/docker-compose.ts (1)
167-169: LGTM!packages/lib/src/core/templates/docker-compose.ts (1)
166-168: LGTM!packages/app/tests/docker-git/core-templates.test.ts (1)
49-90: LGTM!packages/lib/tests/core/templates.test.ts (4)
239-244: LGTM!
717-717: LGTM!
739-740: LGTM!
759-807: LGTM!packages/lib/tests/usecases/mcp-playwright.test.ts (2)
123-123: LGTM!
146-206: LGTM!packages/lib/tests/usecases/prepare-files.test.ts (1)
290-290: LGTM!packages/lib/src/core/templates/playwright.ts (4)
256-271: Та же логика startup check, что и вpackages/app— применим тот же optional refactor.См. комментарий к
packages/app/src/lib/core/templates/playwright.tsо проверке готовности guard через активный healthcheck вместоsleep 1.
9-10: LGTM!
26-29: LGTM!
244-245: LGTM!packages/app/src/lib/core/templates/playwright.ts (3)
10-11: LGTM!
27-30: LGTM!
245-246: LGTM!
| start_cdp_fallback() { | ||
| socat TCP-LISTEN:9223,fork,reuseaddr TCP:127.0.0.1:9222 >/var/log/socat-9223.log 2>&1 & | ||
| } | ||
|
|
||
| # CDP guard: expose 9223 on the docker network and block browser-level destructive CDP methods | ||
| if [ "\${MCP_PLAYWRIGHT_CDP_GUARD:-1}" = "1" ]; then | ||
| docker-git-cdp-guard >/var/log/docker-git-cdp-guard.log 2>&1 & | ||
| guard_pid="$!" | ||
| sleep 1 | ||
| if ! kill -0 "$guard_pid" 2>/dev/null; then | ||
| echo "docker-git-cdp-guard exited during startup; falling back to socat" >&2 | ||
| sed -n '1,120p' /var/log/docker-git-cdp-guard.log 2>/dev/null >&2 || true | ||
| start_cdp_fallback | ||
| fi | ||
| else | ||
| socat TCP-LISTEN:9223,fork,reuseaddr TCP:127.0.0.1:9222 >/var/log/socat-9223.log 2>&1 & | ||
| start_cdp_fallback |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial | 💤 Low value
Startup check с sleep 1 может пропустить медленный запуск guard.
Если docker-git-cdp-guard стартует дольше 1 секунды (например, при cold start Node.js или медленном диске), kill -0 сработает, но guard может упасть позже. MCP-клиент получит connection refused.
Рекомендация: добавить активную проверку готовности (curl к /json/version на 9223) вместо или в дополнение к sleep 1.
♻️ Альтернатива с активной проверкой готовности
if [ "\${MCP_PLAYWRIGHT_CDP_GUARD:-1}" = "1" ]; then
docker-git-cdp-guard >/var/log/docker-git-cdp-guard.log 2>&1 &
guard_pid="$!"
- sleep 1
- if ! kill -0 "$guard_pid" 2>/dev/null; then
+ # Wait up to 5s for guard to become ready
+ for i in 1 2 3 4 5; do
+ sleep 1
+ if ! kill -0 "$guard_pid" 2>/dev/null; then
+ break
+ fi
+ if curl -sf http://127.0.0.1:9223/json/version >/dev/null 2>&1; then
+ break
+ fi
+ done
+ if ! kill -0 "$guard_pid" 2>/dev/null || ! curl -sf http://127.0.0.1:9223/json/version >/dev/null 2>&1; then
echo "docker-git-cdp-guard exited during startup; falling back to socat" >&2
sed -n '1,120p' /var/log/docker-git-cdp-guard.log 2>/dev/null >&2 || true
start_cdp_fallback
fi📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| start_cdp_fallback() { | |
| socat TCP-LISTEN:9223,fork,reuseaddr TCP:127.0.0.1:9222 >/var/log/socat-9223.log 2>&1 & | |
| } | |
| # CDP guard: expose 9223 on the docker network and block browser-level destructive CDP methods | |
| if [ "\${MCP_PLAYWRIGHT_CDP_GUARD:-1}" = "1" ]; then | |
| docker-git-cdp-guard >/var/log/docker-git-cdp-guard.log 2>&1 & | |
| guard_pid="$!" | |
| sleep 1 | |
| if ! kill -0 "$guard_pid" 2>/dev/null; then | |
| echo "docker-git-cdp-guard exited during startup; falling back to socat" >&2 | |
| sed -n '1,120p' /var/log/docker-git-cdp-guard.log 2>/dev/null >&2 || true | |
| start_cdp_fallback | |
| fi | |
| else | |
| socat TCP-LISTEN:9223,fork,reuseaddr TCP:127.0.0.1:9222 >/var/log/socat-9223.log 2>&1 & | |
| start_cdp_fallback | |
| start_cdp_fallback() { | |
| socat TCP-LISTEN:9223,fork,reuseaddr TCP:127.0.0.1:9222 >/var/log/socat-9223.log 2>&1 & | |
| } | |
| # CDP guard: expose 9223 on the docker network and block browser-level destructive CDP methods | |
| if [ "\${MCP_PLAYWRIGHT_CDP_GUARD:-1}" = "1" ]; then | |
| docker-git-cdp-guard >/var/log/docker-git-cdp-guard.log 2>&1 & | |
| guard_pid="$!" | |
| # Wait up to 5s for guard to become ready | |
| for i in 1 2 3 4 5; do | |
| sleep 1 | |
| if ! kill -0 "$guard_pid" 2>/dev/null; then | |
| break | |
| fi | |
| if curl -sf http://127.0.0.1:9223/json/version >/dev/null 2>&1; then | |
| break | |
| fi | |
| done | |
| if ! kill -0 "$guard_pid" 2>/dev/null || ! curl -sf http://127.0.0.1:9223/json/version >/dev/null 2>&1; then | |
| echo "docker-git-cdp-guard exited during startup; falling back to socat" >&2 | |
| sed -n '1,120p' /var/log/docker-git-cdp-guard.log 2>/dev/null >&2 || true | |
| start_cdp_fallback | |
| fi | |
| else | |
| start_cdp_fallback |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/app/src/lib/core/templates/playwright.ts` around lines 257 - 272,
The startup currently only sleeps 1s then checks the guard process, which can
miss slow startups; modify the guard startup block around docker-git-cdp-guard
(and the start_cdp_fallback call) to perform an active readiness probe instead
of a fixed sleep: after launching docker-git-cdp-guard and capturing guard_pid,
loop with a short sleep retrying a curl (e.g.
http://127.0.0.1:9223/json/version) until it returns a successful HTTP response
or a configurable timeout/retry count is reached, also break early if kill -0
reports the process died; if the probe fails or the process exited, log the
guard log and call start_cdp_fallback. Keep references to start_cdp_fallback and
docker-git-cdp-guard/guard_pid so the existing fallback behavior is preserved.
1 participant
Summary
Verification
Runtime proof