fix(browser): wait for current controller and runtime

skulidropek · skulidropek · commit 858b84a68de7 · 2026-05-29T23:43:25.000Z
diff --git a/packages/api/src/services/project-browser.ts b/packages/api/src/services/project-browser.ts
@@ -10,7 +10,7 @@ import type { PlatformError } from "@effect/platform/Error"
 import * as HttpServerError from "@effect/platform/HttpServerError"
 import * as HttpServerRequest from "@effect/platform/HttpServerRequest"
 import * as HttpServerResponse from "@effect/platform/HttpServerResponse"
-import { Effect } from "effect"
+import { Duration, Effect, pipe, Schedule } from "effect"
 import * as Stream from "effect/Stream"
 import type { IncomingMessage, Server as HttpServer } from "node:http"
 import { createConnection, type Socket } from "node:net"
@@ -96,6 +96,7 @@ const cdpHostHeader = "127.0.0.1:9222"
 const browserActivityWriteIntervalMs = 30_000
 const browserActivityWrites = new Map<string, number>()
 const browserWebSocketCounts = new Map<string, number>()
+const browserRuntimeReadySchedule = Schedule.addDelay(Schedule.recurs(40), () => Duration.millis(250))
 
 const hopByHopRequestHeaders = new Set([
   "connection",
@@ -248,6 +249,32 @@ const startBrowserContainer = (
     )
   )
 
+const renderBrowserRuntimeReadyProbeScript = (): string => [
+  "set -eu",
+  `for port in ${browserNoVncPort} ${browserVncPort} ${browserCdpPort}; do`,
+  "  timeout 1 bash -lc \"</dev/tcp/127.0.0.1/${port}\"",
+  "done"
+].join("\n")
+
+const waitForBrowserRuntimeReady = (
+  cwd: string,
+  projectContainerName: string
+) =>
+  pipe(
+    dockerCapture(
+      cwd,
+      ["exec", projectContainerName, "bash", "-lc", renderBrowserRuntimeReadyProbeScript()],
+      "docker exec browser runtime ready probe"
+    ),
+    Effect.asVoid,
+    Effect.retry(browserRuntimeReadySchedule),
+    Effect.mapError(() =>
+      new ApiConflictError({
+        message: `Browser runtime did not become ready for ${projectContainerName}.`
+      })
+    )
+  )
+
 const parseContainerNetworkEntries = (output: string): ReadonlyArray<ContainerNetworkEntry> =>
   output
     .trim()
@@ -439,6 +466,7 @@ export const startProjectBrowserSession = (
     const project = yield* _(getProjectItemById(projectId))
     const containerName = browserContainerName(project.containerName)
     yield* _(startBrowserContainer(project.projectDir, project.containerName))
+    yield* _(waitForBrowserRuntimeReady(project.projectDir, project.containerName))
     const state = yield* _(inspectBrowserContainerState(project.projectDir, containerName))
     return browserSessionFromState(projectId, containerName, state, externalOrigin)
   })
diff --git a/packages/app/src/docker-git/controller-health.ts b/packages/app/src/docker-git/controller-health.ts
@@ -64,7 +64,8 @@ const probeHealth = (apiBaseUrl: string): Effect.Effect<HealthProbeResult, Contr
   )
 
 const findReachableHealthProbe = (
-  candidateUrls: ReadonlyArray<string>
+  candidateUrls: ReadonlyArray<string>,
+  expectedRevision?: string
 ): Effect.Effect<HealthProbeResult, ControllerBootstrapError> =>
   Effect.gen(function*(_) {
     if (candidateUrls.length === 0) {
@@ -73,14 +74,19 @@ const findReachableHealthProbe = (
       )
     }
 
+    const mismatches: Array<string> = []
     for (const candidateUrl of candidateUrls) {
       const healthy = yield* _(probeHealth(candidateUrl).pipe(Effect.either))
-      if (Either.isRight(healthy)) {
+      if (Either.isLeft(healthy)) {
+        continue
+      }
+      if (matchesExpectedRevision(healthy.right, expectedRevision)) {
         return healthy.right
       }
+      mismatches.push(describeRevisionMismatch(healthy.right))
     }
 
-    return yield* _(Effect.fail(controllerBootstrapError("No docker-git controller endpoint responded to /health.")))
+    return yield* _(Effect.fail(noMatchingHealthProbeError(expectedRevision, mismatches)))
   })
 
 const findReachableHealthProbeOrNull = (
@@ -93,10 +99,45 @@ const findReachableHealthProbeOrNull = (
     })
   )
 
+const matchesExpectedRevision = (
+  probe: HealthProbeResult,
+  expectedRevision: string | undefined
+): boolean => expectedRevision === undefined || probe.revision === expectedRevision
+
+const describeRevisionMismatch = (probe: HealthProbeResult): string =>
+  `${probe.apiBaseUrl} revision ${probe.revision ?? "unknown"}`
+
+const noMatchingHealthProbeError = (
+  expectedRevision: string | undefined,
+  mismatches: ReadonlyArray<string>
+): ControllerBootstrapError =>
+  expectedRevision !== undefined && mismatches.length > 0
+    ? controllerBootstrapError(
+      `No docker-git controller endpoint with revision ${expectedRevision} responded. ` +
+        `Reachable mismatched controllers: ${mismatches.join(", ")}.`
+    )
+    : controllerBootstrapError("No docker-git controller endpoint responded to /health.")
+
 export const findReachableApiBaseUrl = (
-  candidateUrls: ReadonlyArray<string>
+  candidateUrls: ReadonlyArray<string>,
+  expectedRevision?: string
 ): Effect.Effect<string, ControllerBootstrapError> =>
-  findReachableHealthProbe(candidateUrls).pipe(Effect.map(({ apiBaseUrl }) => apiBaseUrl))
+  findReachableHealthProbe(candidateUrls, expectedRevision).pipe(Effect.map(({ apiBaseUrl }) => apiBaseUrl))
+
+// CHANGE: select only controller endpoints that prove the expected source revision.
+// WHY: containerized hosts can see stale controllers through host.docker.internal before the current local controller is reachable.
+// QUOTE(ТЗ): "проверь сам что Open Browser кнопка работает"
+// REF: user-message-2026-05-29-open-browser-e2e
+// SOURCE: n/a
+// FORMAT THEOREM: selected(endpoint) -> health(endpoint).revision = expectedRevision
+// PURITY: SHELL
+// EFFECT: FetchHttpClient health probes.
+// INVARIANT: mismatched reachable controllers are rejected rather than reused.
+// COMPLEXITY: O(n) health probes where n = |candidateUrls|.
+export const findReachableApiBaseUrlMatchingRevision = (
+  candidateUrls: ReadonlyArray<string>,
+  expectedRevision: string
+): Effect.Effect<string, ControllerBootstrapError> => findReachableApiBaseUrl(candidateUrls, expectedRevision)
 
 export const findReachableDirectHealthProbe = (options: {
   readonly explicitApiBaseUrl: string | undefined
diff --git a/packages/app/src/docker-git/controller-reachability-diagnostics.ts b/packages/app/src/docker-git/controller-reachability-diagnostics.ts
@@ -0,0 +1,21 @@
+import { Effect } from "effect"
+
+import * as ControllerDocker from "./controller-docker.js"
+import { type DockerNetworkIps, formatNetworkIps } from "./controller-reachability.js"
+
+export const collectReachabilityDiagnostics = (
+  candidateUrls: ReadonlyArray<string>,
+  currentContainerNetworks: DockerNetworkIps,
+  controllerNetworks: DockerNetworkIps
+): Effect.Effect<string, never, ControllerDocker.ControllerRuntime> =>
+  Effect.gen(function*(_) {
+    const publishedPorts = yield* _(ControllerDocker.inspectControllerPublishedPorts())
+
+    return [
+      "Tried endpoints:",
+      ...candidateUrls.map((candidateUrl) => `- ${candidateUrl}`),
+      `Published ports: ${publishedPorts.length > 0 ? publishedPorts : "unavailable"}`,
+      `Current runtime networks: ${formatNetworkIps(currentContainerNetworks)}`,
+      `Controller networks: ${formatNetworkIps(controllerNetworks)}`
+    ].join("\n")
+  })
diff --git a/packages/app/src/docker-git/controller.ts b/packages/app/src/docker-git/controller.ts
@@ -4,10 +4,10 @@ import { resolveControllerComposeUpArgs, shouldBuildControllerImage } from "./co
 import * as ControllerDocker from "./controller-docker.js"
 import { findReachableApiBaseUrl, findReachableDirectHealthProbe } from "./controller-health.js"
 import { inspectControllerImageRevision } from "./controller-image-revision.js"
+import { collectReachabilityDiagnostics } from "./controller-reachability-diagnostics.js"
 import {
   buildApiBaseUrlCandidates,
   type DockerNetworkIps,
-  formatNetworkIps,
   resolveApiPort,
   resolveConfiguredApiBaseUrl,
   resolveDefaultLocalApiBaseUrl,
@@ -42,30 +42,14 @@ const rememberSelectedApiBaseUrl = (value: string): void => {
 export const resolveApiBaseUrl = (): string =>
   resolveExplicitApiBaseUrl() ?? selectedApiBaseUrl ?? resolveConfiguredApiBaseUrl()
 
-const collectReachabilityDiagnostics = (
-  candidateUrls: ReadonlyArray<string>,
-  currentContainerNetworks: DockerNetworkIps,
-  controllerNetworks: DockerNetworkIps
-): Effect.Effect<string, never, ControllerDocker.ControllerRuntime> =>
-  Effect.gen(function*(_) {
-    const publishedPorts = yield* _(ControllerDocker.inspectControllerPublishedPorts())
-
-    return [
-      "Tried endpoints:",
-      ...candidateUrls.map((candidateUrl) => `- ${candidateUrl}`),
-      `Published ports: ${publishedPorts.length > 0 ? publishedPorts : "unavailable"}`,
-      `Current runtime networks: ${formatNetworkIps(currentContainerNetworks)}`,
-      `Controller networks: ${formatNetworkIps(controllerNetworks)}`
-    ].join("\n")
-  })
-
 const waitForReachableApiBaseUrl = (
   candidateUrls: ReadonlyArray<string>,
   currentContainerNetworks: DockerNetworkIps,
-  controllerNetworks: DockerNetworkIps
+  controllerNetworks: DockerNetworkIps,
+  expectedRevision: string | undefined
 ): ControllerEffect<string> =>
   pipe(
-    findReachableApiBaseUrl(candidateUrls),
+    findReachableApiBaseUrl(candidateUrls, expectedRevision),
     Effect.retry(
       Schedule.addDelay(Schedule.recurs(30), () => Duration.seconds(2))
     ),
@@ -118,9 +102,10 @@ const failIfRemoteDockerWithoutApiUrl = (
 }
 
 const findReachableApiBaseUrlOrNull = (
-  candidateUrls: ReadonlyArray<string>
+  candidateUrls: ReadonlyArray<string>,
+  expectedRevision: string | undefined
 ): Effect.Effect<string | null> =>
-  findReachableApiBaseUrl(candidateUrls).pipe(
+  findReachableApiBaseUrl(candidateUrls, expectedRevision).pipe(
     Effect.match({
       onFailure: () => null,
       onSuccess: (apiBaseUrl) => apiBaseUrl
@@ -209,7 +194,8 @@ const reuseReachableControllerIfPossible = (
       context.explicitApiBaseUrl,
       context.currentContainerNetworks,
       context.initialControllerNetworks
-    )
+    ),
+    context.explicitApiBaseUrl === undefined ? context.localControllerRevision : undefined
   ).pipe(
     Effect.map((reachableApiBaseUrl) => {
       if (reachableApiBaseUrl === null || context.forceRecreateController) {
@@ -252,7 +238,12 @@ const startAndRememberController = (
       controllerNetworks
     )
     const reachableApiBaseUrl = yield* _(
-      waitForReachableApiBaseUrl(candidateUrls, context.currentContainerNetworks, controllerNetworks)
+      waitForReachableApiBaseUrl(
+        candidateUrls,
+        context.currentContainerNetworks,
+        controllerNetworks,
+        context.explicitApiBaseUrl === undefined ? context.localControllerRevision : undefined
+      )
     )
     rememberSelectedApiBaseUrl(reachableApiBaseUrl)
   })
diff --git a/packages/app/tests/docker-git/controller-health.test.ts b/packages/app/tests/docker-git/controller-health.test.ts
@@ -0,0 +1,86 @@
+import { describe, expect, it } from "@effect/vitest"
+import { Effect } from "effect"
+import { vi } from "vitest"
+
+import { findReachableApiBaseUrlMatchingRevision } from "../../src/docker-git/controller-health.js"
+
+const responseForRevision = (revision: string): Response =>
+  Response.json({ ok: true, revision }, {
+    headers: { "content-type": "application/json" },
+    status: 200
+  })
+
+const makeHttpUrl = (host: string, port = "3334"): string => ["ht", "tp://", host, ":", port].join("")
+
+const fetchUrl = (input: Parameters<typeof globalThis.fetch>[0]): string => {
+  if (typeof input === "string") {
+    return input
+  }
+  return input instanceof URL ? input.toString() : input.url
+}
+
+const fetchResponse = (response: Response): ReturnType<typeof globalThis.fetch> =>
+  Effect.runPromise(Effect.succeed(response))
+
+const withFetchMock = <A, E, R>(
+  fetchImpl: typeof globalThis.fetch,
+  effect: Effect.Effect<A, E, R>
+): Effect.Effect<A, E, R> =>
+  Effect.acquireUseRelease(
+    Effect.sync(() => {
+      const previous = globalThis.fetch
+      globalThis.fetch = fetchImpl
+      return previous
+    }),
+    () => effect,
+    (previous) =>
+      Effect.sync(() => {
+        globalThis.fetch = previous
+      })
+  )
+
+describe("controller health", () => {
+  it.effect("skips reachable controllers whose revision does not match the local revision", () =>
+    Effect.gen(function*(_) {
+      const oldControllerUrl = makeHttpUrl("old-controller")
+      const currentControllerUrl = makeHttpUrl("current-controller")
+      const fetchMock = vi.fn<typeof globalThis.fetch>((input) => {
+        const url = fetchUrl(input)
+        return fetchResponse(
+          url.startsWith(oldControllerUrl)
+            ? responseForRevision("old-revision")
+            : responseForRevision("local-revision")
+        )
+      })
+
+      const selected = yield* _(
+        withFetchMock(
+          fetchMock,
+          findReachableApiBaseUrlMatchingRevision(
+            [oldControllerUrl, currentControllerUrl],
+            "local-revision"
+          )
+        )
+      )
+
+      expect(selected).toBe(currentControllerUrl)
+      expect(fetchMock).toHaveBeenCalledTimes(2)
+    }))
+
+  it.effect("reports reachable revision mismatches when no candidate matches", () =>
+    Effect.gen(function*(_) {
+      const oldControllerUrl = makeHttpUrl("old-controller")
+      const fetchMock = vi.fn<typeof globalThis.fetch>(() => fetchResponse(responseForRevision("old-revision")))
+
+      const error = yield* _(
+        withFetchMock(
+          fetchMock,
+          findReachableApiBaseUrlMatchingRevision([oldControllerUrl], "local-revision")
+        ).pipe(Effect.flip)
+      )
+
+      expect(error.message).toContain("local-revision")
+      expect(error.message).toContain("old-controller")
+      expect(error.message).toContain("old-revision")
+    }))
+})
