Address browser runtime review feedback

Author: skulidropek

packages/lib/src/core/templates.ts

@@ -1,7 +1,11 @@
 import type { TemplateConfig } from "./domain.js"
 import type { ResolvedComposeResourceLimits } from "./resource-limits.js"
 import { renderEntrypoint } from "./templates-entrypoint.js"
-import { type ComposeResourceLimits, renderDockerCompose } from "./templates/docker-compose.js"
+import {
+  type ComposeResourceLimits,
+  type DockerComposeRenderOptions,
+  renderDockerCompose
+} from "./templates/docker-compose.js"
 import { renderDockerfile } from "./templates/dockerfile.js"
 import { renderPlaywrightBrowserRuntime } from "./templates/playwright-browser-runtime.js"
 import { renderPlaywrightBrowserDockerfile, renderPlaywrightStartExtra } from "./templates/playwright.js"
@@ -10,6 +14,14 @@ export type FileSpec =
   | { readonly _tag: "File"; readonly relativePath: string; readonly contents: string; readonly mode?: number }
   | { readonly _tag: "Dir"; readonly relativePath: string }
 
+export type TemplateRenderOptions = {
+  readonly compose: DockerComposeRenderOptions
+}
+
+const defaultTemplateRenderOptions: TemplateRenderOptions = {
+  compose: { enableLocalDockerSocket: false }
+}
+
 const renderGitignore = (): string =>
   `# docker-git project files
 # NOTE: bootstrap secrets stay local-only and should not be committed.
@@ -46,7 +58,8 @@ const renderConfigJson = (config: TemplateConfig): string =>
 
 export const planFiles = (
   config: TemplateConfig,
-  composeResourceLimits?: ResolvedComposeResourceLimits | ComposeResourceLimits
+  composeResourceLimits?: ResolvedComposeResourceLimits | ComposeResourceLimits,
+  options: TemplateRenderOptions = defaultTemplateRenderOptions
 ): ReadonlyArray<FileSpec> => {
   const maybePlaywrightFiles = config.enableMcpPlaywright
     ? ([
@@ -72,7 +85,7 @@ export const planFiles = (
     {
       _tag: "File",
       relativePath: "docker-compose.yml",
-      contents: renderDockerCompose(config, composeResourceLimits)
+      contents: renderDockerCompose(config, composeResourceLimits, options.compose)
     },
     { _tag: "File", relativePath: ".dockerignore", contents: renderDockerignore() },
     { _tag: "File", relativePath: "docker-git.json", contents: renderConfigJson(config) },

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

@@ -30,11 +30,16 @@ type PlaywrightFragments = Pick<
   "maybeDependsOn" | "maybeDockerSocketMount" | "maybePlaywrightEnv" | "maybeBrowserVolume"
 >
 
+export type DockerComposeRenderOptions = {
+  readonly enableLocalDockerSocket: boolean
+}
+
 export type ComposeResourceLimits = {
   readonly main: ResolvedComposeResourceLimits | undefined
   readonly playwright: ResolvedComposeResourceLimits | undefined
 }
 
+const defaultDockerComposeRenderOptions: DockerComposeRenderOptions = { enableLocalDockerSocket: false }
 const sharedCodexVolumeKey = "docker_git_shared_codex"
 const sharedCacheVolumeKey = "docker_git_shared_cache"
 const bootstrapVolumeKey = "docker_git_bootstrap"
@@ -81,7 +86,10 @@ const renderGpu = (gpu: TemplateConfig["gpu"]): string =>
 
 const renderBootstrapMounts = (): string => `      - ${bootstrapVolumeKey}:/opt/docker-git/bootstrap/source:ro`
 
-const renderOptionalDockerSocketMount = (): string => `      - /var/run/docker.sock:/var/run/docker.sock`
+const renderOptionalDockerSocketMount = (enableLocalDockerSocket: boolean): string =>
+  enableLocalDockerSocket
+    ? `      - /var/run/docker.sock:/var/run/docker.sock`
+    : ""
 
 const renderEnvFiles = (config: TemplateConfig): string =>
   `    env_file:\n      - ${config.envGlobalPath}\n      - ${config.envProjectPath}\n`
@@ -93,7 +101,8 @@ const renderBrowserLimitEnv = (
 
 const buildPlaywrightFragments = (
   config: TemplateConfig,
-  resourceLimits: ResolvedComposeResourceLimits | undefined
+  resourceLimits: ResolvedComposeResourceLimits | undefined,
+  options: DockerComposeRenderOptions
 ): PlaywrightFragments => {
   if (!config.enableMcpPlaywright) {
     return {
@@ -110,7 +119,7 @@ const buildPlaywrightFragments = (
 
   return {
     maybeDependsOn: "",
-    maybeDockerSocketMount: renderOptionalDockerSocketMount(),
+    maybeDockerSocketMount: renderOptionalDockerSocketMount(options.enableLocalDockerSocket),
     maybePlaywrightEnv:
       `      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${
         renderBrowserLimitEnv("DOCKER_GIT_BROWSER_CPU_LIMIT", resourceLimits?.cpuLimit)
@@ -137,7 +146,8 @@ const normalizeComposeResourceLimits = (
 
 const buildComposeFragments = (
   config: TemplateConfig,
-  resourceLimits: ComposeResourceLimits
+  resourceLimits: ComposeResourceLimits,
+  options: DockerComposeRenderOptions
 ): ComposeFragments => {
   const networkMode = config.dockerNetworkMode
   const networkName = resolveComposeNetworkName(config)
@@ -151,7 +161,7 @@ const buildComposeFragments = (
   const maybeClaudeAuthLabelEnv = renderClaudeAuthLabelEnv(claudeAuthLabel)
   const maybeAgentModeEnv = renderAgentModeEnv(config.agentMode)
   const maybeAgentAutoEnv = renderAgentAutoEnv(config.agentAuto)
-  const playwright = buildPlaywrightFragments(config, resourceLimits.playwright)
+  const playwright = buildPlaywrightFragments(config, resourceLimits.playwright, options)
 
   return {
     networkMode,
@@ -243,10 +253,11 @@ const renderComposeVolumes = (config: TemplateConfig, maybeBrowserVolume: string
 
 export const renderDockerCompose = (
   config: TemplateConfig,
-  resourceLimits?: ResolvedComposeResourceLimits | ComposeResourceLimits
+  resourceLimits?: ResolvedComposeResourceLimits | ComposeResourceLimits,
+  options: DockerComposeRenderOptions = defaultDockerComposeRenderOptions
 ): string => {
   const limits = normalizeComposeResourceLimits(resourceLimits)
-  const fragments = buildComposeFragments(config, limits)
+  const fragments = buildComposeFragments(config, limits, options)
   return [
     `name: ${resolveComposeProjectName(config)}`,
     renderComposeServices(config, fragments, limits),

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

@@ -24,6 +24,37 @@ docker_git_stop_playwright_browser() {
   docker rm -f "$container_name" >/dev/null 2>&1 || true
 }
 
+docker_git_cleanup_orphaned_playwright_browsers() {
+  if ! docker_git_browser_has_docker; then
+    return 0
+  fi
+
+  local browser_id
+  while IFS= read -r browser_id; do
+    if [[ -z "$browser_id" ]]; then
+      continue
+    fi
+
+    local project_container
+    project_container="$(docker inspect --format '{{ index .Config.Labels "docker-git.project-container" }}' "$browser_id" 2>/dev/null || true)"
+
+    local project_running
+    project_running="false"
+    if [[ -n "$project_container" && "$project_container" != "<no value>" ]]; then
+      project_running="$(docker inspect --format '{{ .State.Running }}' "$project_container" 2>/dev/null || true)"
+    fi
+
+    if [[ "$project_running" == "true" ]]; then
+      continue
+    fi
+
+    local browser_name
+    browser_name="$(docker inspect --format '{{ .Name }}' "$browser_id" 2>/dev/null | sed 's#^/##' || true)"
+    docker_git_browser_log "removing orphaned browser container \${browser_name:-$browser_id}"
+    docker rm -f "$browser_id" >/dev/null 2>&1 || true
+  done < <(docker ps -a -q --filter "label=docker-git.browser=1" --filter "label=docker-git.project-container")
+}
+
 docker_git_start_playwright_browser() {
   if [[ "\${MCP_PLAYWRIGHT_ENABLE:-0}" != "1" ]]; then
     docker_git_stop_playwright_browser || true
@@ -50,13 +81,15 @@ docker_git_start_playwright_browser() {
     return 0
   fi
 
+  docker_git_stop_playwright_browser || true
+  docker_git_cleanup_orphaned_playwright_browsers || true
+
   docker_git_browser_log "building $image_name"
   docker build -t "$image_name" -f "$context_dir/Dockerfile.browser" "$context_dir" >/var/log/docker-git-browser-build.log 2>&1 || {
     docker_git_browser_log "browser image build failed; see /var/log/docker-git-browser-build.log"
     return 0
   }
 
-  docker_git_stop_playwright_browser || true
   docker volume create "$volume_name" >/dev/null
 
   local args=(
@@ -96,6 +129,6 @@ docker_git_start_playwright_browser() {
 // FORMAT THEOREM: start(main) -> running(browser) with network(browser) = container:main OR logged_warning
 // PURITY: SHELL
 // EFFECT: shell commands executed by generated entrypoint
-// INVARIANT: browser data volume is preserved; runtime cleanup removes only the browser container
-// COMPLEXITY: O(build + docker-run)/O(1)
+// INVARIANT: browser data volume is preserved; runtime cleanup removes only browser-labeled containers
+// COMPLEXITY: O(b + build + docker-run)/O(1), where b = browser-labeled containers
 export const renderPlaywrightBrowserRuntime = (): string => playwrightBrowserRuntimeScript

packages/lib/src/shell/files.ts

@@ -11,6 +11,7 @@ import {
   withDefaultResourceLimitIntent
 } from "../core/resource-limits.js"
 import { type FileSpec, planFiles } from "../core/templates.js"
+import { resolveDockerEnvValue } from "./docker-auth.js"
 import { FileExistsError } from "./errors.js"
 import { resolveBaseDir } from "./paths.js"
 import { resolveWorkspaceRoot } from "./workspace-root.js"
@@ -45,6 +46,8 @@ const loadHostResources = (): Effect.Effect<
 
 const isFileSpec = (spec: FileSpec): spec is Extract<FileSpec, { readonly _tag: "File" }> => spec._tag === "File"
 
+const shouldMountLocalDockerSocket = (): boolean => resolveDockerEnvValue("DOCKER_GIT_PROJECT_DOCKER_HOST") === null
+
 const resolveSpecPath = (
   path: Path.Path,
   baseDir: string,
@@ -242,7 +245,9 @@ export const writeProjectFiles = (
       main: resolveComposeResourceLimits(normalizedConfig, hostResources),
       playwright: resolvePlaywrightComposeResourceLimits(normalizedConfig, hostResources)
     }
-    const specs = planFiles(normalizedConfig, composeResourceLimits)
+    const specs = planFiles(normalizedConfig, composeResourceLimits, {
+      compose: { enableLocalDockerSocket: shouldMountLocalDockerSocket() }
+    })
     const created: Array<string> = []
     const existingFilePaths = force ? [] : yield* _(collectExistingFilePaths(fs, path, baseDir, specs))
     const existingSet = new Set(existingFilePaths)

packages/lib/tests/core/templates.test.ts

@@ -601,14 +601,30 @@ describe("renderDockerCompose", () => {
     expect(compose).toContain('DOCKER_GIT_BROWSER_VOLUME_NAME: "dg-test-home-browser"')
     expect(compose).toContain('DOCKER_GIT_BROWSER_CPU_LIMIT: "${DOCKER_GIT_BROWSER_CPU_LIMIT:-1.5}"')
     expect(compose).toContain('DOCKER_GIT_BROWSER_RAM_LIMIT: "${DOCKER_GIT_BROWSER_RAM_LIMIT:-2g}"')
-    expect(compose).toContain("      - /var/run/docker.sock:/var/run/docker.sock")
+    expect(compose).not.toContain("      - /var/run/docker.sock:/var/run/docker.sock")
     expect(compose).toContain("  dg-test-home-browser:")
     expect(compose).not.toContain("\n  dg-test-browser:\n")
     expect(compose).not.toContain("dg-test-browser:\n    build:")
     expect(compose).not.toContain("restart:")
     expect((compose.match(/\n    dns:\n/g) ?? []).length).toBe(1)
   })
 
+  it("renders local Docker socket mount only when explicitly enabled", () => {
+    const compose = renderDockerCompose(
+      makeTemplateConfig({
+        enableMcpPlaywright: true,
+        gpu: "none",
+      }),
+      {
+        cpuLimit: 1.5,
+        ramLimit: "2g"
+      },
+      { enableLocalDockerSocket: true }
+    )
+
+    expect(compose).toContain("      - /var/run/docker.sock:/var/run/docker.sock")
+  })
+
   it("applies separate resource limits for the nested browser runtime when provided", () => {
     const compose = renderDockerCompose(
       makeTemplateConfig({

packages/lib/tests/usecases/mcp-playwright.test.ts

@@ -159,6 +159,9 @@ describe("enableMcpPlaywrightProjectFiles", () => {
         expect(startExtra).toContain("docker-git-cdp-guard")
         expect(startExtra).toContain("socat TCP-LISTEN:9223")
         expect(browserRuntime).toContain('--network "container:$main_container"')
+        expect(browserRuntime).toContain("docker_git_cleanup_orphaned_playwright_browsers")
+        expect(browserRuntime).toContain('--filter "label=docker-git.browser=1" --filter "label=docker-git.project-container"')
+        expect(browserRuntime).toContain('docker inspect --format \'{{ .State.Running }}\' "$project_container"')
         expect(browserRuntime).toContain('args+=(--cpus "$DOCKER_GIT_BROWSER_CPU_LIMIT")')
         expect(browserRuntime).toContain('args+=(--memory "$DOCKER_GIT_BROWSER_RAM_LIMIT" --memory-swap "$DOCKER_GIT_BROWSER_RAM_LIMIT")')
 

packages/lib/tests/usecases/prepare-files.test.ts

@@ -306,6 +306,31 @@ describe("prepareProjectFiles", () => {
       })
     ).pipe(Effect.provide(NodeContext.layer)))
 
+  it.effect("omits local Docker socket mount when project Docker host is injected", () =>
+    withTempDir((root) =>
+      withPatchedEnv(
+        { DOCKER_GIT_PROJECT_DOCKER_HOST: "tcp://docker-controller:2375" },
+        Effect.gen(function*(_) {
+          const fs = yield* _(FileSystem.FileSystem)
+          const path = yield* _(Path.Path)
+          const outDir = path.join(root, "project-docker-host")
+          const globalConfig = makeGlobalConfig(root, path)
+          const projectConfig = makeProjectConfig(outDir, true, path)
+
+          yield* _(
+            prepareProjectFiles(outDir, root, globalConfig, projectConfig, {
+              force: false,
+              forceEnv: false
+            })
+          )
+
+          const compose = yield* _(fs.readFileString(path.join(outDir, "docker-compose.yml")))
+          expect(compose).toContain('DOCKER_GIT_PROJECT_DOCKER_HOST: "${DOCKER_GIT_PROJECT_DOCKER_HOST:-}"')
+          expect(compose).not.toContain("      - /var/run/docker.sock:/var/run/docker.sock")
+        })
+      )
+    ).pipe(Effect.provide(NodeContext.layer)))
+
   it.effect("renders project-scoped network when dockerNetworkMode=project", () =>
     withTempDir((root) =>
       Effect.gen(function*(_) {