feat(browser-connection): new dedicated package for noVNC + browser connection (issue #347) #348

coderabbitai · 2026-05-23T10:17:01Z

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Требуется comprehensive TSDoc для класса BrowserError.

Согласно guidelines для **/*.{ts,tsx}, все функции и типы должны включать comprehensive TSDoc с параметрами, типами возвращаемых значений, маркерами @pure, @effect, @invariant, @complexity и другими. Класс BrowserError не содержит документации.

📝 Предлагаемая документация

+/** + * Error type for browser connection failures. + * + * `@remarks` + * PURITY: SHELL - error class for service layer + * INVARIANT: message is non-empty string + * COMPLEXITY: O(1) construction + * + * `@example` + * ```ts + * new BrowserError("Failed to start browser", originalError) + * ``` + */ export class BrowserError { readonly _tag = "BrowserError" as const + /** + * `@param` message - Human-readable error description + * `@param` cause - Optional underlying error that caused this failure + */ constructor(readonly message: string, readonly cause?: unknown) {} }

Согласно coding guidelines для TypeScript: "TypeScript functions must include comprehensive TSDoc with parameters, return types, @pure marker, @effect dependencies, @invariant (mathematical), @precondition, @postcondition, and @complexity O-notation."

🤖 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/browser-connection/src/index.ts` around lines 3 - 6, Add a comprehensive TSDoc block for the BrowserError class: document the class purpose, the readonly _tag, and the constructor parameters (message: string, cause?: unknown) with `@param` entries, include an example showing usage (e.g., new BrowserError("Failed to start browser", originalError)), and add required markers such as `@pure`, `@effect` (if any side-effects/dependencies), `@invariant` (if applicable), `@precondition/`@postcondition for constructor behavior, and `@complexity` O(1); attach the doc block immediately above the export class BrowserError declaration and reference the constructor, _tag, message, and cause symbols in the comments.

coderabbitai · 2026-05-23T10:17:01Z

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Метод parseProxyPath должен возвращать типизированное значение вместо unknown.

Согласно guidelines для **/*.{ts,tsx}: "No any type allowed; all boundary data must be typed, with unknown only at SHELL entry points before decoding". Метод parseProxyPath возвращает Effect.Effect<unknown, never>, что ослабляет type safety. Следует определить конкретный тип возвращаемого значения (например, ProxyPathInfo | null).

🔧 Предлагаемое улучшение типизации

+/** + * Parsed proxy path information + */ +export interface ProxyPathInfo { + readonly projectId: string + readonly path: string +} + export interface BrowserConnection { // ... - readonly parseProxyPath: (pathname: string) => Effect.Effect<unknown, never> + readonly parseProxyPath: (pathname: string) => Effect.Effect<ProxyPathInfo | null, BrowserError>

Согласно coding guidelines: "No any type allowed; all boundary data must be typed, with unknown only at SHELL entry points before decoding."

🤖 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/browser-connection/src/index.ts` at line 13, The parseProxyPath method currently returns Effect.Effect<unknown, never>; change its signature to return a typed effect such as Effect.Effect<ProxyPathInfo | null, never> (or another concrete discriminated type you define) so boundary data is fully typed, update the parseProxyPath implementation to produce that ProxyPathInfo | null shape (including any decoding/validation logic) and update all call sites to expect the new type; ensure you add or reuse a ProxyPathInfo interface/type and adjust imports/exports accordingly so no unknown is leaked from parseProxyPath.

coderabbitai · 2026-05-23T10:17:01Z

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Отсутствует comprehensive TSDoc для методов интерфейса BrowserConnection.

Согласно guidelines для **/*.{ts,tsx}, все функции должны включать TSDoc с @effect dependencies, @invariant, @precondition, @postcondition, @complexity. Интерфейс не содержит документации для методов.

📝 Пример требуемой документации

+/** + * Browser connection service for managing single shared browser sessions. + * + * `@remarks` + * PURITY: SHELL - service interface with side effects + * EFFECT: Depends on Logger, potentially Docker/Process services + * INVARIANT: ∀projectId: getCdpUrl(projectId) and getNoVncUrl(projectId) point to same dg-{projectId}-browser container + */ export interface BrowserConnection { + /** + * Start browser container for the given project. + * + * `@effect` Launches Docker container, creates network resources + * `@complexity` O(1) API call, O(n) container startup time + * `@throws` BrowserError if container fails to start + */ readonly startBrowser: (projectId: string) => Effect.Effect<void, BrowserError> + // ... документация для остальных методов

Согласно coding guidelines: "TypeScript functions must include comprehensive TSDoc with parameters, return types, @pure marker, @effect dependencies, @invariant (mathematical), @precondition, @postcondition, and @complexity O-notation."

🤖 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/browser-connection/src/index.ts` around lines 8 - 15, Add comprehensive TSDoc comments to the BrowserConnection interface methods: startBrowser, getCdpUrl, getNoVncUrl, getVncUrl, parseProxyPath, and rewriteCdpUrl. For each method document parameters and return type, mark `@pure` where applicable, add `@effect` dependencies (e.g., Effect), include `@invariant` (mathematical contract), `@precondition` (input requirements), `@postcondition` (output guarantees), and `@complexity` (Big-O), and ensure tags reflect any thrown BrowserError or never effects; place these comments directly above the respective method signatures in the BrowserConnection interface.

coderabbitai · 2026-05-23T10:17:01Z

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Запрещено использование type assertion as void.

Согласно guidelines для **/*.{ts,tsx}: "Type casting with as is strictly forbidden in regular code; permitted ONLY in a single axiomatic module (brands/constructors/constants)". Строка 26 использует return undefined as void, что нарушает это правило.

🔧 Предлагаемое исправление

startBrowser: (projectId: string) => Effect.gen(function* () { yield* Effect.log(`[browser-connection] starting browser for project ${projectId}`) - return undefined as void }),

Effect.gen автоматически выведет тип Effect.Effect<void, never> если не возвращать значение явно.

Согласно coding guidelines: "Type casting with as is strictly forbidden in regular code; permitted ONLY in a single axiomatic module."

📝 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.

Suggested change

return undefined as void

startBrowser: (projectId: string) =>

Effect.gen(function* () {

yield* Effect.log(`[browser-connection] starting browser for project ${projectId}`)

}),

🤖 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/browser-connection/src/index.ts` at line 26, Replace the forbidden type assertion "return undefined as void" with a non-casted return (either remove the return entirely so Effect.gen infers Effect<void, never>, or use a plain "return" / "return undefined" without "as void"); locate the exact statement "return undefined as void" in packages/browser-connection/src/index.ts and remove the "as void" cast so no type assertion is used.

coderabbitai · 2026-05-23T10:17:01Z

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Несоответствие URL между getCdpUrl и renderCdpUrl.

Метод getCdpUrl (строка 28) возвращает http://localhost:9223?project=${projectId}, в то время как helper renderCdpUrl (строка 42) возвращает http://localhost:9223/json/version?project=${projectId}. Это приведет к ошибкам при использовании разных методов для получения одного и того же URL.

🔧 Предлагаемое исправление

Вариант 1: Использовать helper в реализации Layer:

- getCdpUrl: (projectId: string) => Effect.succeed(`http://localhost:9223?project=${projectId}`), + getCdpUrl: (projectId: string) => Effect.succeed(renderCdpUrl(projectId)),

Вариант 2: Привести helper в соответствие с реализацией Layer:

export const renderCdpUrl = (projectId: string): string => - `http://localhost:9223/json/version?project=${projectId}` + `http://localhost:9223?project=${projectId}`

Рекомендуется вариант 1, чтобы избежать дублирования строковых литералов.

Also applies to: 41-42

🤖 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/browser-connection/src/index.ts` at line 28, getCdpUrl currently returns a different string than the helper renderCdpUrl which will cause inconsistent CDP endpoints; update the Layer implementation of getCdpUrl to call/render via the existing renderCdpUrl helper (or, alternatively, make renderCdpUrl match the literal used by getCdpUrl) so both functions produce the exact same URL format—locate getCdpUrl and renderCdpUrl in packages/browser-connection/src/index.ts and replace the literal in getCdpUrl with a call to renderCdpUrl(projectId) to avoid duplicated string literals.

coderabbitai · 2026-05-23T10:17:01Z

⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy lift

Отсутствуют обязательные functional comments для всех методов Layer.

Согласно guidelines для **/*.{ts,tsx}: "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". В реализации BrowserConnectionLive полностью отсутствуют эти комментарии, что делает невозможной формальную верификацию, заявленную в PR objectives.

📋 Пример требуемых functional comments

+// CHANGE: Implement BrowserConnectionLive Layer with stub methods for single-session browser management +// WHY: Consolidate browser control logic to ensure single shared session invariant +// QUOTE(ТЗ): "When MCP Playwright is installed, it should automatically start noVNC to manage a single shared browser with an agent" (issue `#347`) +// REF: issue `#347`, PR `#348` +// SOURCE: docker-git-session-sync patterns, effect-template style +// FORMAT THEOREM: ∀projectId ∈ String: getCdpUrl(projectId) ∧ getNoVncUrl(projectId) → same container(dg-${projectId}-browser) +// PURITY: SHELL - contains side effects (logging, future Docker operations) +// EFFECT: Effect.Effect<BrowserConnection, never, never> +// INVARIANT: Single browser session per projectId; CDP port 9223, noVNC port 6080 are fixed +// COMPLEXITY: O(1) for all operations (current stub implementation) export const BrowserConnectionLive = Layer.effect( BrowserConnection, Effect.gen(function* () { return { + // CHANGE: Log browser start event + // WHY: Provide observability for browser lifecycle + // PURITY: SHELL (logging side effect) + // EFFECT: Effect.Effect<void, BrowserError> + // COMPLEXITY: O(1) startBrowser: (projectId: string) => Effect.gen(function* () { yield* Effect.log(`[browser-connection] starting browser for project ${projectId}`) }), + // CHANGE: Return deterministic CDP URL for projectId + // PURITY: CORE logic (deterministic string generation) + // EFFECT: Effect.Effect<string, BrowserError> + // INVARIANT: Port 9223 is fixed for CDP endpoint + // COMPLEXITY: O(1) getCdpUrl: (projectId: string) => Effect.succeed(renderCdpUrl(projectId)), // ... аналогично для остальных методов

Согласно coding guidelines: "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." Также см. требование из guidelines: "Ты строгий ревьюер SPEC DRIVEN DEVELOPMENT. Сверь изменения с исходным ТЗ/спекой и обсуждением."

🤖 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/browser-connection/src/index.ts` around lines 19 - 35, Add the required functional comments for the BrowserConnectionLive Layer and each exposed method (BrowserConnectionLive, startBrowser, getCdpUrl, getNoVncUrl, getVncUrl, parseProxyPath, rewriteCdpUrl): for every function provide the header including CHANGE, WHY, QUOTE(ТЗ) or n/a, REF, SOURCE or n/a, FORMAT THEOREM, PURITY (CORE|SHELL) and for SHELL functions include the EFFECT signature, plus INVARIANT and COMPLEXITY. Place the comment block immediately above the Layer implementation and above each method implementation so the formal verification tooling can parse them; ensure the EFFECT signature matches the returned Effect types (e.g., Effect.succeed/Effect.gen) and that each comment references the same function name to uniquely identify the behavior.

coderabbitai · 2026-05-23T10:17:01Z

🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win

Отсутствует документация для pure helper functions.

Согласно guidelines для **/*.{ts,tsx}, все функции должны включать TSDoc с @pure marker, @complexity, @invariant и functional comments (PURITY: CORE для чистых функций). Функции renderNoVncUrl, renderCdpUrl и isSingleBrowserSession не содержат никакой документации.

📝 Пример требуемой документации

-// Pure helpers +// CHANGE: Pure helper functions for URL generation and session validation +// WHY: Separate pure logic (CORE) from effectful service methods (SHELL) +// PURITY: CORE - deterministic, no side effects +// INVARIANT: URL format stability across project lifecycle +/** + * Generate noVNC URL for browser session. + * + * `@pure` true + * `@param` projectId - Project identifier + * `@returns` noVNC URL with autoconnect and websockify path + * `@invariant` Result always includes `/b/${projectId}/vnc.html` + * `@complexity` O(1) time, O(n) space where n = projectId.length + */ export const renderNoVncUrl = (projectId: string): string => `/b/${projectId}/vnc.html?autoconnect=true&resize=remote&path=b/${projectId}/websockify` +/** + * Generate CDP (Chrome DevTools Protocol) URL for browser session. + * + * `@pure` true + * `@param` projectId - Project identifier + * `@returns` CDP endpoint URL on port 9223 + * `@invariant` Port 9223 is fixed for all projects + * `@complexity` O(1) time, O(n) space where n = projectId.length + */ export const renderCdpUrl = (projectId: string): string => `http://localhost:9223/json/version?project=${projectId}` +/** + * Validate that CDP and noVNC URLs belong to single browser session. + * + * `@pure` true + * `@param` cdpUrl - CDP endpoint URL + * `@param` noVncUrl - noVNC web interface URL + * `@returns` true if both URLs point to same browser session + * `@precondition` cdpUrl and noVncUrl are non-empty strings + * `@complexity` O(n) where n = max(cdpUrl.length, noVncUrl.length) + */ export const isSingleBrowserSession = (cdpUrl: string, noVncUrl: string): boolean => cdpUrl.includes("9223") && noVncUrl.includes("/vnc.html")

Согласно coding guidelines: "TypeScript functions must include comprehensive TSDoc with parameters, return types, @pure marker, @effect dependencies, @invariant (mathematical), @precondition, @postcondition, and @complexity O-notation" и "Document all functions with comprehensive TSDoc including: @pure (true/false), @effect (required services), @invariant (mathematical invariants), @precondition, @postcondition, @complexity (time and space)."

🤖 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/browser-connection/src/index.ts` around lines 37 - 45, Add comprehensive TSDoc for the three pure helper functions: renderNoVncUrl, renderCdpUrl, and isSingleBrowserSession; for each function include a short functional description, `@param` and `@returns` tags, a `@pure` marker set to true (or PURE: CORE in the top-line comment), `@effect` (none or required services), `@precondition` (e.g. non-empty projectId or valid URL strings), `@postcondition` (output string contains expected path or query), `@invariant` (mathematical properties like idempotence or substring containment), and `@complexity` with time/space O-notation; place the TSDoc immediately above each function declaration so tools and linters recognize them and ensure the documented invariants mention the exact behavior of renderNoVncUrl (produces /b/{projectId}/vnc.html URL), renderCdpUrl (produces localhost 9223 JSON URL with project query), and isSingleBrowserSession (returns true when cdpUrl includes "9223" and noVncUrl includes "/vnc.html").

coderabbitai · 2026-05-23T10:17:01Z

🧹 Nitpick | 🔵 Trivial | ⚡ Quick win

Слишком упрощенная логика валидации в isSingleBrowserSession.

Функция isSingleBrowserSession использует только проверку подстрок (includes("9223") и includes("/vnc.html")), что не гарантирует, что URL действительно указывают на один и тот же browser container. Согласно PR objectives, должен быть математический инвариант: "getCdpUrl(projectId) and getNoVncUrl(projectId) point to the same dg-*-browser container".

Текущая реализация пропустит любые URL, содержащие эти подстроки, даже если они указывают на разные проекты или контейнеры.

🔧 Предлагаемое улучшение валидации

+/** + * Extract projectId from browser URL. + * `@pure` true + */ +const extractProjectId = (url: string): string | null => { + // For CDP: http://localhost:9223/json/version?project=abc + // For noVNC: /b/abc/vnc.html... + const cdpMatch = url.match(/[?&]project=([^&]+)/) + if (cdpMatch) return cdpMatch[1] + + const vncMatch = url.match(/\/b\/([^\/]+)\/vnc\.html/) + if (vncMatch) return vncMatch[1] + + return null +} + export const isSingleBrowserSession = (cdpUrl: string, noVncUrl: string): boolean => { - cdpUrl.includes("9223") && noVncUrl.includes("/vnc.html") + const cdpProjectId = extractProjectId(cdpUrl) + const vncProjectId = extractProjectId(noVncUrl) + + return cdpProjectId !== null && + vncProjectId !== null && + cdpProjectId === vncProjectId && + cdpUrl.includes("9223") && + noVncUrl.includes("/vnc.html") +}

Это обеспечит математический инвариант: оба URL указывают на один projectId.

🤖 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/browser-connection/src/index.ts` around lines 44 - 45, Функция isSingleBrowserSession слишком примитивно проверяет URL через includes; нужно извлечь и сопоставить реальный projectId/имя контейнера из cdpUrl и noVncUrl вместо простых подстрок. Измените isSingleBrowserSession так, чтобы она парсила переданные URL (например регуляркой) и вытягивала идентификатор dg-*-browser или projectId/namespace из хоста/пути/параметров для обоих URL, затем возвращала true только если оба извлечённых projectId/имени контейнера совпадают и порт/маршрут соответствует ожидаемому (CDP порт 9223 и noVnc путь /vnc.html) — используйте имена функций/констант getCdpUrl/getNoVncUrl и саму isSingleBrowserSession как точки внесения правок.

-Original file line number
+Diff line change
@@ -0,0 +1,44 @@
+    {
+      "name": "@prover-coder-ai/browser-connection",
+      "version": "1.0.0",
+      "description": "Reusable noVNC + browser CDP connection module for docker-git (used by MCP, Hermes tools, and project-browser services)",
+      "main": "dist/index.js",
+      "types": "dist/index.d.ts",
+      "files": ["dist"],
+      "scripts": {
+        "build": "tsc",
+        "check": "bun run typecheck",
+        "prepack": "bun run build",
+        "test": "vitest run --passWithNoTests",
+        "typecheck": "tsc --noEmit -p tsconfig.json"
+      },
+      "repository": {
+        "type": "git",
+        "url": "git+https://github.com/ProverCoderAI/docker-git.git"
+      },
+      "keywords": [
+        "docker-git",
+        "browser",
+        "novnc",
+        "cdp",
+        "effect",
+        "hermes"
+      ],
+      "author": "",
+      "license": "MIT",
+      "type": "module",
+      "bugs": {
+        "url": "https://github.com/ProverCoderAI/docker-git/issues"
+      },
+      "homepage": "https://github.com/ProverCoderAI/docker-git#readme",
+      "packageManager": "bun@1.3.11",
+      "devDependencies": {
+        "@effect/vitest": "^0.29.0",
+        "@types/node": "^25.9.1",
+        "typescript": "^6.0.3",
+        "vitest": "^4.1.7"
+      },
+      "dependencies": {
+        "effect": "^3.12.0"
+      }
+    }

-Original file line number
+Diff line change
@@ -0,0 +1,10 @@
+    {
+    	"extends": "../../tsconfig.base.json",
+    	"compilerOptions": {
+    		"rootDir": ".",
+    		"outDir": "dist",
+    		"types": ["vitest", "node"]
+    	},
+    	"include": ["src/**/*", "tests/**/*"],
+    	"exclude": ["dist", "node_modules"]
+    }

-Original file line number
+Diff line change
@@ -1,5 +1,6 @@
     packages:
       - packages/api
       - packages/app
+      - packages/browser-connection
       - packages/docker-git-session-sync
       - packages/lib

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(browser-connection): new dedicated package for noVNC + browser connection (issue #347) #348

Diff view

Diff view

There are no files selected for viewing

coderabbitai Bot May 23, 2026

Uh oh!

coderabbitai Bot May 23, 2026

Uh oh!

coderabbitai Bot May 23, 2026

Uh oh!

coderabbitai Bot May 23, 2026

Uh oh!

coderabbitai Bot May 23, 2026

Uh oh!

coderabbitai Bot May 23, 2026

Uh oh!

coderabbitai Bot May 23, 2026

Uh oh!

coderabbitai Bot May 23, 2026

Uh oh!

Uh oh!

feat(browser-connection): new dedicated package for noVNC + browser connection (issue #347) #348

Are you sure you want to change the base?

feat(browser-connection): new dedicated package for noVNC + browser connection (issue #347) #348

Uh oh!

Uh oh!

Diff view

Diff view

There are no files selected for viewing

coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

coderabbitai Bot May 23, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!