fix(app): stabilize browser frontend api target

skulidropek · skulidropek · commit c7ef800706af · 2026-05-23T00:26:55.000Z
diff --git a/packages/app/src/docker-git/browser-frontend.ts b/packages/app/src/docker-git/browser-frontend.ts
@@ -12,6 +12,8 @@ import {
   resolveBrowserFrontendStatePath,
   shouldReuseBrowserFrontend
 } from "./browser-frontend-state.js"
+import { findReachableApiBaseUrl } from "./controller-health.js"
+import { resolveConfiguredApiBaseUrl, resolveExplicitApiBaseUrl } from "./controller-reachability.js"
 import { type ControllerRuntime, ensureControllerReady, resolveApiBaseUrl } from "./controller.js"
 import {
   runCommandCapture,
@@ -146,6 +148,49 @@ const readBrowserFrontendRuntimeState = (
     webState: readBrowserFrontendState(statePath)
   })
 
+// CHANGE: prefer the host-facing controller URL for the browser web proxy.
+// WHY: controller bootstrap may select a Docker bridge IP before the published localhost port is reachable, but the served browser runtime must keep durable state and proxy config on the externally reachable endpoint.
+// QUOTE(ТЗ): "комментарии ребита надо было тоже поддержать"
+// REF: PR #344 E2E (Browser command) regression.
+// SOURCE: n/a
+// FORMAT THEOREM: explicit_api -> explicit_api; reachable(configured_api) -> configured_api; otherwise -> selected_api
+// PURITY: SHELL
+// EFFECT: Effect<string, never, ControllerRuntime>
+// INVARIANT: explicit DOCKER_GIT_API_URL is never overridden by auto-discovery.
+// COMPLEXITY: O(1) probes/O(1) space.
+/**
+ * Resolves the API URL used by the browser frontend proxy.
+ *
+ * @returns Effect with the explicit API URL, the reachable configured host URL, or the selected controller URL.
+ *
+ * @pure false
+ * @effect FetchHttpClient through controller health probing.
+ * @invariant Explicit `DOCKER_GIT_API_URL` has precedence over all inferred endpoints.
+ * @precondition `ensureControllerReady` has already completed for inferred endpoints.
+ * @postcondition A configured host URL is used only after a successful health probe.
+ * @complexity O(1) time and O(1) space for the bounded candidate set.
+ * @throws Never - health probe failures fall back to the selected controller URL.
+ */
+const resolveBrowserFrontendApiBaseUrl = (): Effect.Effect<string, never, ControllerRuntime> => {
+  const selectedApiBaseUrl = resolveApiBaseUrl()
+  const explicitApiBaseUrl = resolveExplicitApiBaseUrl()
+  if (explicitApiBaseUrl !== undefined) {
+    return Effect.succeed(selectedApiBaseUrl)
+  }
+
+  const configuredApiBaseUrl = resolveConfiguredApiBaseUrl()
+  if (configuredApiBaseUrl === selectedApiBaseUrl) {
+    return Effect.succeed(selectedApiBaseUrl)
+  }
+
+  return findReachableApiBaseUrl([configuredApiBaseUrl]).pipe(
+    Effect.match({
+      onFailure: () => selectedApiBaseUrl,
+      onSuccess: (apiBaseUrl) => apiBaseUrl
+    })
+  )
+}
+
 const stopCurrentWebServer = (): Effect.Effect<
   void,
   ControllerBootstrapError | PlatformError,
@@ -173,7 +218,7 @@ const prepareBrowserStack = (): Effect.Effect<
     yield* _(Effect.log("Ensuring docker-git API controller is current."))
     yield* _(ensureControllerReady())
 
-    const apiBaseUrl = resolveApiBaseUrl()
+    const apiBaseUrl = yield* _(resolveBrowserFrontendApiBaseUrl())
     const runtimeState = yield* _(readBrowserFrontendRuntimeState(statePath))
     const reuseInput: BrowserFrontendReuseInput = {
       apiBaseUrl,
diff --git a/packages/app/src/docker-git/controller-image-revision.ts b/packages/app/src/docker-git/controller-image-revision.ts
@@ -182,8 +182,7 @@ export const inspectControllerImageRevision = (): Effect.Effect<
           Effect.catchTag("ControllerBootstrapError", (error) =>
             isMissingControllerImageInspectionError(error)
               ? Effect.succeed<string | null>(null)
-              : Effect.fail(error)
-          )
+              : Effect.fail(error))
         )
     )
   )
diff --git a/packages/app/tests/docker-git/browser-frontend.test.ts b/packages/app/tests/docker-git/browser-frontend.test.ts
@@ -6,6 +6,8 @@ import { describe, expect, it } from "@effect/vitest"
 import { Effect } from "effect"
 import { afterEach, beforeEach, vi } from "vitest"
 
+import type { ControllerBootstrapError } from "../../src/docker-git/host-errors.js"
+
 type CommandSpec = {
   readonly args: ReadonlyArray<string>
   readonly command: string
@@ -14,13 +16,28 @@ type CommandSpec = {
 }
 
 const ensureControllerReadyMock = vi.hoisted(() => vi.fn<() => Effect.Effect<void>>())
+const resolveApiBaseUrlMock = vi.hoisted(() => vi.fn<() => string>())
+const findReachableApiBaseUrlMock = vi.hoisted(
+  () => vi.fn<(candidateUrls: ReadonlyArray<string>) => Effect.Effect<string, ControllerBootstrapError>>()
+)
+const resolveConfiguredApiBaseUrlMock = vi.hoisted(() => vi.fn<() => string>())
+const resolveExplicitApiBaseUrlMock = vi.hoisted(() => vi.fn<() => string | undefined>())
 const runCommandCaptureMock = vi.hoisted(() => vi.fn<(spec: CommandSpec) => Effect.Effect<string>>())
 const runCommandExitCodeMock = vi.hoisted(() => vi.fn<(spec: CommandSpec) => Effect.Effect<number>>())
 const runCommandExitCodeStreamingMock = vi.hoisted(() => vi.fn<(spec: CommandSpec) => Effect.Effect<number>>())
 
 vi.mock("../../src/docker-git/controller.js", () => ({
   ensureControllerReady: ensureControllerReadyMock,
-  resolveApiBaseUrl: () => "http://127.0.0.1:3334"
+  resolveApiBaseUrl: resolveApiBaseUrlMock
+}))
+
+vi.mock("../../src/docker-git/controller-health.js", () => ({
+  findReachableApiBaseUrl: findReachableApiBaseUrlMock
+}))
+
+vi.mock("../../src/docker-git/controller-reachability.js", () => ({
+  resolveConfiguredApiBaseUrl: resolveConfiguredApiBaseUrlMock,
+  resolveExplicitApiBaseUrl: resolveExplicitApiBaseUrlMock
 }))
 
 vi.mock("../../src/docker-git/frontend-lib/shell/command-runner.js", () => ({
@@ -63,6 +80,10 @@ const requireEnvValue = (
   return value
 }
 
+const makeHttpUrl = (host: string, port: string): string => `http://${host}:${port}`
+
+const dockerBridgeHost = ["172", "17", "0", "2"].join(".")
+
 const writeWebStateFile = (
   statePath: string,
   state: Readonly<Record<string, string | number>>
@@ -100,6 +121,14 @@ describe("browser frontend command", () => {
             makeNonInteractive()
             ensureControllerReadyMock.mockReset()
             ensureControllerReadyMock.mockImplementation(() => Effect.void)
+            resolveApiBaseUrlMock.mockReset()
+            resolveApiBaseUrlMock.mockReturnValue("http://127.0.0.1:3334")
+            findReachableApiBaseUrlMock.mockReset()
+            findReachableApiBaseUrlMock.mockImplementation((candidateUrls) => Effect.succeed(candidateUrls[0] ?? ""))
+            resolveConfiguredApiBaseUrlMock.mockReset()
+            resolveConfiguredApiBaseUrlMock.mockReturnValue("http://127.0.0.1:3334")
+            resolveExplicitApiBaseUrlMock.mockReset()
+            resolveExplicitApiBaseUrlMock.mockImplementation(() => {})
             runCommandCaptureMock.mockReset()
             runCommandCaptureMock.mockImplementation(() => Effect.succeed(""))
             runCommandExitCodeMock.mockReset()
@@ -142,6 +171,50 @@ describe("browser frontend command", () => {
       expect(runCommandExitCodeStreamingMock).toHaveBeenCalledTimes(2)
     }))
 
+  it.effect("prefers the reachable host API URL over a selected Docker bridge URL for the web proxy", () =>
+    Effect.gen(function*(_) {
+      resolveApiBaseUrlMock.mockReturnValue(makeHttpUrl(dockerBridgeHost, "3334"))
+      resolveConfiguredApiBaseUrlMock.mockReturnValue("http://127.0.0.1:3334")
+      findReachableApiBaseUrlMock.mockImplementation((candidateUrls) => Effect.succeed(candidateUrls[0] ?? ""))
+
+      yield* _(runBrowserCommandUnderTest)
+
+      expect(findReachableApiBaseUrlMock).toHaveBeenCalledWith(["http://127.0.0.1:3334"])
+      expect(streamingEnvs()).toEqual([
+        expect.objectContaining({ DOCKER_GIT_API_URL: "http://127.0.0.1:3334" }),
+        expect.objectContaining({ DOCKER_GIT_API_URL: "http://127.0.0.1:3334" })
+      ])
+    }))
+
+  it.effect("falls back to the selected controller URL when the host API URL is unreachable", () =>
+    Effect.gen(function*(_) {
+      const dockerBridgeApiBaseUrl = makeHttpUrl(dockerBridgeHost, "3334")
+      resolveApiBaseUrlMock.mockReturnValue(dockerBridgeApiBaseUrl)
+      resolveConfiguredApiBaseUrlMock.mockReturnValue("http://127.0.0.1:3334")
+      findReachableApiBaseUrlMock.mockReturnValue(Effect.fail({ _tag: "ControllerBootstrapError", message: "no" }))
+
+      yield* _(runBrowserCommandUnderTest)
+
+      expect(streamingEnvs()).toEqual([
+        expect.objectContaining({ DOCKER_GIT_API_URL: dockerBridgeApiBaseUrl }),
+        expect.objectContaining({ DOCKER_GIT_API_URL: dockerBridgeApiBaseUrl })
+      ])
+    }))
+
+  it.effect("does not override an explicit API URL", () =>
+    Effect.gen(function*(_) {
+      resolveApiBaseUrlMock.mockReturnValue("https://api.example.test")
+      resolveExplicitApiBaseUrlMock.mockReturnValue("https://api.example.test")
+
+      yield* _(runBrowserCommandUnderTest)
+
+      expect(findReachableApiBaseUrlMock).not.toHaveBeenCalled()
+      expect(streamingEnvs()).toEqual([
+        expect.objectContaining({ DOCKER_GIT_API_URL: "https://api.example.test" }),
+        expect.objectContaining({ DOCKER_GIT_API_URL: "https://api.example.test" })
+      ])
+    }))
+
   it.effect("binds browser web to all host interfaces by default", () =>
     Effect.gen(function*(_) {
       yield* _(runBrowserCommandUnderTest)
diff --git a/packages/app/tests/docker-git/menu-create-shared-properties.test.ts b/packages/app/tests/docker-git/menu-create-shared-properties.test.ts
@@ -2,18 +2,9 @@ import { Match } from "effect"
 import * as fc from "fast-check"
 import { describe, expect, it } from "vitest"
 
-import {
-  advanceCreateFlow,
-  createInitialFlowView,
-  resolveCreateFlowSteps
-} from "../../src/docker-git/menu-create-shared.js"
+import { advanceCreateFlow, resolveCreateFlowSteps } from "../../src/docker-git/menu-create-shared.js"
 import type { CreateInputs, CreateStep } from "../../src/docker-git/menu-types.js"
-import {
-  expectCreateContinueView,
-  expectedOutDirForRepoUrl,
-  featureCreateRepoUrl,
-  repositoryCreateInputArbitrary
-} from "./create-flow-test-helpers.js"
+import { featureCreateRepoUrl } from "./create-flow-test-helpers.js"
 
 type CreateSettingStep = Exclude<CreateStep, "outDir" | "repoRef" | "repoUrl">
 
@@ -119,24 +110,6 @@ describe("menu-create-shared property invariants", () => {
   const cwd = process.cwd()
   const defaultRoot = `${process.env["HOME"] ?? cwd}/.docker-git/org/repo`
 
-  it("preserves absolute root projectsRoot for generated repo URLs", () => {
-    fc.assert(
-      fc.property(repositoryCreateInputArbitrary, ({ repoUrl }) => {
-        const view = expectCreateContinueView(advanceCreateFlow(
-          {
-            cwd: "/repo/packages/api",
-            projectsRoot: "/"
-          },
-          createInitialFlowView(repoUrl)
-        ))
-
-        expect(view.values.outDir).toBe(expectedOutDirForRepoUrl(repoUrl, "/"))
-        expect(view.values.outDir?.startsWith("/")).toBe(true)
-      }),
-      { numRuns: 50 }
-    )
-  })
-
   it("preserves the next remaining settings index after applying generated current settings", () => {
     fc.assert(
       fc.property(createSettingsStepArbitrary, satisfiedCreateSettingsArbitrary, (currentStep, satisfiedSteps) => {
diff --git a/packages/app/tests/docker-git/menu-create-shared.test.ts b/packages/app/tests/docker-git/menu-create-shared.test.ts
@@ -18,8 +18,10 @@ import {
   expectCreateCompleteInputs,
   expectCreateContinueView,
   expectCreateNavigationResult,
+  expectedOutDirForRepoUrl,
   expectedWrappedCreateNavigationStep,
-  featureCreateRepoUrl
+  featureCreateRepoUrl,
+  repositoryCreateInputArbitrary
 } from "./create-flow-test-helpers.js"
 
 const expectFeatureRepoDefaults = (
@@ -151,15 +153,21 @@ describe("menu-create-shared", () => {
   })
 
   it("preserves an absolute root projectsRoot in browser mode", () => {
-    const view = expectCreateContinueView(advanceCreateFlow(
-      {
-        cwd: "/repo/packages/api",
-        projectsRoot: "/"
-      },
-      createInitialFlowView(featureCreateRepoUrl)
-    ))
+    fc.assert(
+      fc.property(repositoryCreateInputArbitrary, ({ repoUrl }) => {
+        const view = expectCreateContinueView(advanceCreateFlow(
+          {
+            cwd: "/repo/packages/api",
+            projectsRoot: "/"
+          },
+          createInitialFlowView(repoUrl)
+        ))
 
-    expect(view.values.outDir).toBe("/org/repo")
+        expect(view.values.outDir).toBe(expectedOutDirForRepoUrl(repoUrl, "/"))
+        expect(view.values.outDir?.startsWith("/")).toBe(true)
+      }),
+      { numRuns: 50 }
+    )
   })
 
   it("moves between remaining settings rows and clears the input buffer", () => {
@@ -198,17 +206,21 @@ describe("menu-create-shared", () => {
   })
 
   it("advances to the next remaining settings row after applying the current setting", () => {
-    const next = expectCreateContinueView(advanceCreateFlow(
-      cwd,
-      {
-        ...createFeatureRepoSettingsView(cwd),
-        buffer: "45%"
-      }
-    ))
+    fc.assert(
+      fc.property(fc.constantFrom("", "25%", "45%", "100m"), (cpuLimit) => {
+        const next = expectCreateContinueView(advanceCreateFlow(
+          cwd,
+          {
+            ...createFeatureRepoSettingsView(cwd),
+            buffer: cpuLimit
+          }
+        ))
 
-    expect(next.values.cpuLimit).toBe("45%")
-    expect(next.step).toBe(1)
-    expect(resolveCreateFlowSteps(next.values)[next.step]).toBe("ramLimit")
+        expect(next.values.cpuLimit).toBe(cpuLimit)
+        expect(next.step).toBe(1)
+        expect(resolveCreateFlowSteps(next.values)[next.step]).toBe("ramLimit")
+      })
+    )
   })
 
   it("maps create-mode steps to the matching display row when opening browser Settings", () => {

Original file line number	Diff line number	Diff line change
`@@ -182,8 +182,7 @@ export const inspectControllerImageRevision = (): Effect.Effect<`
`182`	`182`	`Effect.catchTag("ControllerBootstrapError", (error) =>`
`183`	`183`	`isMissingControllerImageInspectionError(error)`
`184`	`184`	`? Effect.succeed<string \| null>(null)`
`185`		`- : Effect.fail(error)`
`186`		`- )`
	`185`	`+ : Effect.fail(error))`
`187`	`186`	`)`
`188`	`187`	`)`
`189`	`188`	`)`