Skip to content

Commit 7031911

Browse files
skulidropekskulidropek
authored
fix(web): retain prepared browser popup handle (#360)
* fix(web): retain prepared browser popup handle * fix(browser): wait for current controller and runtime * fix(browser): apply controller review feedback * fix(browser): satisfy controller review checks * fix(browser): deduplicate health probe options
1 parent 7e20051 commit 7031911

11 files changed

Lines changed: 543 additions & 47 deletions

‎packages/api/src/services/project-browser.ts‎

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import type { PlatformError } from "@effect/platform/Error"
1010
import * as HttpServerError from "@effect/platform/HttpServerError"
1111
import * as HttpServerRequest from "@effect/platform/HttpServerRequest"
1212
import * as HttpServerResponse from "@effect/platform/HttpServerResponse"
13-
import { Effect } from "effect"
13+
import { Duration, Effect, pipe, Schedule } from "effect"
1414
import * as Stream from "effect/Stream"
1515
import type { IncomingMessage, Server as HttpServer } from "node:http"
1616
import { createConnection, type Socket } from "node:net"
@@ -96,6 +96,7 @@ const cdpHostHeader = "127.0.0.1:9222"
9696
const browserActivityWriteIntervalMs = 30_000
9797
const browserActivityWrites = new Map<string, number>()
9898
const browserWebSocketCounts = new Map<string, number>()
99+
const browserRuntimeReadySchedule = Schedule.addDelay(Schedule.recurs(40), () => Duration.millis(250))
99100

100101
const hopByHopRequestHeaders = new Set([
101102
"connection",
@@ -248,6 +249,32 @@ const startBrowserContainer = (
248249
)
249250
)
250251

252+
const renderBrowserRuntimeReadyProbeScript = (): string => [
253+
"set -eu",
254+
`for port in ${browserNoVncPort} ${browserVncPort} ${browserCdpPort}; do`,
255+
" timeout 1 bash -lc \"</dev/tcp/127.0.0.1/${port}\"",
256+
"done"
257+
].join("\n")
258+
259+
const waitForBrowserRuntimeReady = (
260+
cwd: string,
261+
projectContainerName: string
262+
) =>
263+
pipe(
264+
dockerCapture(
265+
cwd,
266+
["exec", projectContainerName, "bash", "-lc", renderBrowserRuntimeReadyProbeScript()],
267+
"docker exec browser runtime ready probe"
268+
),
269+
Effect.asVoid,
270+
Effect.retry(browserRuntimeReadySchedule),
271+
Effect.mapError(() =>
272+
new ApiConflictError({
273+
message: `Browser runtime did not become ready for ${projectContainerName}.`
274+
})
275+
)
276+
)
277+
251278
const parseContainerNetworkEntries = (output: string): ReadonlyArray<ContainerNetworkEntry> =>
252279
output
253280
.trim()
@@ -439,6 +466,7 @@ export const startProjectBrowserSession = (
439466
const project = yield* _(getProjectItemById(projectId))
440467
const containerName = browserContainerName(project.containerName)
441468
yield* _(startBrowserContainer(project.projectDir, project.containerName))
469+
yield* _(waitForBrowserRuntimeReady(project.projectDir, project.containerName))
442470
const state = yield* _(inspectBrowserContainerState(project.projectDir, containerName))
443471
return browserSessionFromState(projectId, containerName, state, externalOrigin)
444472
})

‎packages/app/src/docker-git/controller-health.ts‎

Lines changed: 74 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,9 @@ const parseHealthRevision = (text: string): string | null =>
3131
}
3232
})
3333

34-
const probeHealth = (apiBaseUrl: string): Effect.Effect<HealthProbeResult, ControllerBootstrapError> =>
34+
const probeHealth = (
35+
apiBaseUrl: string
36+
): Effect.Effect<HealthProbeResult, ControllerBootstrapError, HttpClient.HttpClient> =>
3537
Effect.gen(function*(_) {
3638
const client = yield* _(HttpClient.HttpClient)
3739
const response = yield* _(client.get(`${apiBaseUrl}/health`, { headers: { accept: "application/json" } }))
@@ -52,7 +54,6 @@ const probeHealth = (apiBaseUrl: string): Effect.Effect<HealthProbeResult, Contr
5254
)
5355
)
5456
}).pipe(
55-
Effect.provide(FetchHttpClient.layer),
5657
Effect.mapError((error): ControllerBootstrapError =>
5758
error._tag === "ControllerBootstrapError"
5859
? error
@@ -64,45 +65,98 @@ const probeHealth = (apiBaseUrl: string): Effect.Effect<HealthProbeResult, Contr
6465
)
6566

6667
const findReachableHealthProbe = (
67-
candidateUrls: ReadonlyArray<string>
68-
): Effect.Effect<HealthProbeResult, ControllerBootstrapError> =>
68+
candidateUrls: ReadonlyArray<string>,
69+
expectedRevision?: string
70+
): Effect.Effect<HealthProbeResult, ControllerBootstrapError, HttpClient.HttpClient> =>
6971
Effect.gen(function*(_) {
7072
if (candidateUrls.length === 0) {
7173
return yield* _(
7274
Effect.fail(controllerBootstrapError("No docker-git controller endpoint candidates were generated."))
7375
)
7476
}
7577

78+
const mismatches: Array<string> = []
7679
for (const candidateUrl of candidateUrls) {
7780
const healthy = yield* _(probeHealth(candidateUrl).pipe(Effect.either))
78-
if (Either.isRight(healthy)) {
81+
if (Either.isLeft(healthy)) {
82+
continue
83+
}
84+
if (matchesExpectedRevision(healthy.right, expectedRevision)) {
7985
return healthy.right
8086
}
87+
mismatches.push(describeRevisionMismatch(healthy.right))
8188
}
8289

83-
return yield* _(Effect.fail(controllerBootstrapError("No docker-git controller endpoint responded to /health.")))
90+
return yield* _(Effect.fail(noMatchingHealthProbeError(expectedRevision, mismatches)))
8491
})
8592

8693
const findReachableHealthProbeOrNull = (
87-
candidateUrls: ReadonlyArray<string>
88-
): Effect.Effect<HealthProbeResult | null> =>
89-
findReachableHealthProbe(candidateUrls).pipe(
94+
candidateUrls: ReadonlyArray<string>,
95+
expectedRevision?: string
96+
): Effect.Effect<HealthProbeResult | null, never, HttpClient.HttpClient> =>
97+
findReachableHealthProbe(candidateUrls, expectedRevision).pipe(
9098
Effect.match({
9199
onFailure: () => null,
92100
onSuccess: (probe) => probe
93101
})
94102
)
95103

104+
const matchesExpectedRevision = (
105+
probe: HealthProbeResult,
106+
expectedRevision: string | undefined
107+
): boolean => expectedRevision === undefined || probe.revision === expectedRevision
108+
109+
const describeRevisionMismatch = (probe: HealthProbeResult): string =>
110+
`${probe.apiBaseUrl} revision ${probe.revision ?? "unknown"}`
111+
112+
const noMatchingHealthProbeError = (
113+
expectedRevision: string | undefined,
114+
mismatches: ReadonlyArray<string>
115+
): ControllerBootstrapError =>
116+
expectedRevision !== undefined && mismatches.length > 0
117+
? controllerBootstrapError(
118+
`No docker-git controller endpoint with revision ${expectedRevision} responded. ` +
119+
`Reachable mismatched controllers: ${mismatches.join(", ")}.`
120+
)
121+
: controllerBootstrapError("No docker-git controller endpoint responded to /health.")
122+
123+
export const findReachableApiBaseUrlWithHttpClient = (
124+
candidateUrls: ReadonlyArray<string>,
125+
expectedRevision?: string
126+
): Effect.Effect<string, ControllerBootstrapError, HttpClient.HttpClient> =>
127+
findReachableHealthProbe(candidateUrls, expectedRevision).pipe(Effect.map(({ apiBaseUrl }) => apiBaseUrl))
128+
96129
export const findReachableApiBaseUrl = (
97-
candidateUrls: ReadonlyArray<string>
130+
candidateUrls: ReadonlyArray<string>,
131+
expectedRevision?: string
98132
): Effect.Effect<string, ControllerBootstrapError> =>
99-
findReachableHealthProbe(candidateUrls).pipe(Effect.map(({ apiBaseUrl }) => apiBaseUrl))
133+
findReachableApiBaseUrlWithHttpClient(candidateUrls, expectedRevision).pipe(Effect.provide(FetchHttpClient.layer))
100134

101-
export const findReachableDirectHealthProbe = (options: {
135+
// CHANGE: select only controller endpoints that prove the expected source revision.
136+
// WHY: containerized hosts can see stale controllers through host.docker.internal before the current local controller is reachable.
137+
// QUOTE(ТЗ): "проверь сам что Open Browser кнопка работает"
138+
// REF: user-message-2026-05-29-open-browser-e2e
139+
// SOURCE: n/a
140+
// FORMAT THEOREM: selected(endpoint) -> health(endpoint).revision = expectedRevision
141+
// PURITY: SHELL
142+
// EFFECT: FetchHttpClient health probes.
143+
// INVARIANT: mismatched reachable controllers are rejected rather than reused.
144+
// COMPLEXITY: O(n) health probes where n = |candidateUrls|.
145+
export const findReachableApiBaseUrlMatchingRevision = (
146+
candidateUrls: ReadonlyArray<string>,
147+
expectedRevision: string
148+
): Effect.Effect<string, ControllerBootstrapError> => findReachableApiBaseUrl(candidateUrls, expectedRevision)
149+
150+
type DirectHealthProbeOptions = {
102151
readonly explicitApiBaseUrl: string | undefined
103152
readonly defaultLocalApiBaseUrl: string | undefined
104153
readonly cachedApiBaseUrl: string | undefined
105-
}): Effect.Effect<HealthProbeResult | null> =>
154+
readonly expectedRevision?: string | undefined
155+
}
156+
157+
export const findReachableDirectHealthProbeWithHttpClient = (
158+
options: DirectHealthProbeOptions
159+
): Effect.Effect<HealthProbeResult | null, never, HttpClient.HttpClient> =>
106160
findReachableHealthProbeOrNull(
107161
buildApiBaseUrlCandidates({
108162
explicitApiBaseUrl: options.explicitApiBaseUrl,
@@ -112,5 +166,11 @@ export const findReachableDirectHealthProbe = (options: {
112166
currentContainerNetworks: {},
113167
controllerNetworks: {},
114168
port: resolveApiPort()
115-
})
169+
}),
170+
options.expectedRevision
116171
)
172+
173+
export const findReachableDirectHealthProbe = (
174+
options: DirectHealthProbeOptions
175+
): Effect.Effect<HealthProbeResult | null> =>
176+
findReachableDirectHealthProbeWithHttpClient(options).pipe(Effect.provide(FetchHttpClient.layer))

‎packages/app/src/docker-git/controller-reachability-diagnostics.ts‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { Effect } from "effect"
2+
3+
import * as ControllerDocker from "./controller-docker.js"
4+
import { type DockerNetworkIps, formatNetworkIps } from "./controller-reachability.js"
5+
6+
// CHANGE: document controller reachability diagnostics as a SHELL effect.
7+
// WHY: diagnostics inspect published controller ports and must make their runtime dependency explicit.
8+
// QUOTE(ТЗ): n/a
9+
// REF: PR-360-coderabbit-reachability-diagnostics-contract
10+
// SOURCE: n/a
11+
// FORMAT THEOREM: forall candidates C: diagnostics(C) returns a string containing attempted endpoints and runtime network state.
12+
// PURITY: SHELL
13+
// EFFECT: Effect<string, never, ControllerDocker.ControllerRuntime>
14+
// INVARIANT: every returned diagnostic string includes candidate URLs, published ports, current runtime networks, and controller networks.
15+
// COMPLEXITY: O(n) where n = |candidateUrls|.
16+
/**
17+
* Collects host/controller reachability diagnostics for failed bootstrap probes.
18+
*
19+
* @param candidateUrls - API base URL candidates attempted by the bootstrap path.
20+
* @param currentContainerNetworks - network IPs visible from the current runtime container.
21+
* @param controllerNetworks - network IPs attached to the docker-git controller container.
22+
* @returns Effect.Effect<string, never, ControllerDocker.ControllerRuntime>
23+
*
24+
* @pure false
25+
* @effect Requires ControllerDocker.inspectControllerPublishedPorts through ControllerDocker.ControllerRuntime.
26+
* @invariant The result describes controller runtime endpoints, published ports, and network visibility.
27+
* @complexity O(n) where n is candidateUrls.length.
28+
*/
29+
export const collectReachabilityDiagnostics = (
30+
candidateUrls: ReadonlyArray<string>,
31+
currentContainerNetworks: DockerNetworkIps,
32+
controllerNetworks: DockerNetworkIps
33+
): Effect.Effect<string, never, ControllerDocker.ControllerRuntime> =>
34+
Effect.gen(function*(_) {
35+
const publishedPorts = yield* _(ControllerDocker.inspectControllerPublishedPorts())
36+
37+
return [
38+
"Tried endpoints:",
39+
...candidateUrls.map((candidateUrl) => `- ${candidateUrl}`),
40+
`Published ports: ${publishedPorts.length > 0 ? publishedPorts : "unavailable"}`,
41+
`Current runtime networks: ${formatNetworkIps(currentContainerNetworks)}`,
42+
`Controller networks: ${formatNetworkIps(controllerNetworks)}`
43+
].join("\n")
44+
})

‎packages/app/src/docker-git/controller.ts‎

Lines changed: 17 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@ import { resolveControllerComposeUpArgs, shouldBuildControllerImage } from "./co
44
import * as ControllerDocker from "./controller-docker.js"
55
import { findReachableApiBaseUrl, findReachableDirectHealthProbe } from "./controller-health.js"
66
import { inspectControllerImageRevision } from "./controller-image-revision.js"
7+
import { collectReachabilityDiagnostics } from "./controller-reachability-diagnostics.js"
78
import {
89
buildApiBaseUrlCandidates,
910
type DockerNetworkIps,
10-
formatNetworkIps,
1111
resolveApiPort,
1212
resolveConfiguredApiBaseUrl,
1313
resolveDefaultLocalApiBaseUrl,
@@ -42,30 +42,14 @@ const rememberSelectedApiBaseUrl = (value: string): void => {
4242
export const resolveApiBaseUrl = (): string =>
4343
resolveExplicitApiBaseUrl() ?? selectedApiBaseUrl ?? resolveConfiguredApiBaseUrl()
4444

45-
const collectReachabilityDiagnostics = (
46-
candidateUrls: ReadonlyArray<string>,
47-
currentContainerNetworks: DockerNetworkIps,
48-
controllerNetworks: DockerNetworkIps
49-
): Effect.Effect<string, never, ControllerDocker.ControllerRuntime> =>
50-
Effect.gen(function*(_) {
51-
const publishedPorts = yield* _(ControllerDocker.inspectControllerPublishedPorts())
52-
53-
return [
54-
"Tried endpoints:",
55-
...candidateUrls.map((candidateUrl) => `- ${candidateUrl}`),
56-
`Published ports: ${publishedPorts.length > 0 ? publishedPorts : "unavailable"}`,
57-
`Current runtime networks: ${formatNetworkIps(currentContainerNetworks)}`,
58-
`Controller networks: ${formatNetworkIps(controllerNetworks)}`
59-
].join("\n")
60-
})
61-
6245
const waitForReachableApiBaseUrl = (
6346
candidateUrls: ReadonlyArray<string>,
6447
currentContainerNetworks: DockerNetworkIps,
65-
controllerNetworks: DockerNetworkIps
48+
controllerNetworks: DockerNetworkIps,
49+
expectedRevision: string | undefined
6650
): ControllerEffect<string> =>
6751
pipe(
68-
findReachableApiBaseUrl(candidateUrls),
52+
findReachableApiBaseUrl(candidateUrls, expectedRevision),
6953
Effect.retry(
7054
Schedule.addDelay(Schedule.recurs(30), () => Duration.seconds(2))
7155
),
@@ -118,9 +102,10 @@ const failIfRemoteDockerWithoutApiUrl = (
118102
}
119103

120104
const findReachableApiBaseUrlOrNull = (
121-
candidateUrls: ReadonlyArray<string>
105+
candidateUrls: ReadonlyArray<string>,
106+
expectedRevision: string | undefined
122107
): Effect.Effect<string | null> =>
123-
findReachableApiBaseUrl(candidateUrls).pipe(
108+
findReachableApiBaseUrl(candidateUrls, expectedRevision).pipe(
124109
Effect.match({
125110
onFailure: () => null,
126111
onSuccess: (apiBaseUrl) => apiBaseUrl
@@ -209,7 +194,8 @@ const reuseReachableControllerIfPossible = (
209194
context.explicitApiBaseUrl,
210195
context.currentContainerNetworks,
211196
context.initialControllerNetworks
212-
)
197+
),
198+
context.explicitApiBaseUrl === undefined ? context.localControllerRevision : undefined
213199
).pipe(
214200
Effect.map((reachableApiBaseUrl) => {
215201
if (reachableApiBaseUrl === null || context.forceRecreateController) {
@@ -252,7 +238,12 @@ const startAndRememberController = (
252238
controllerNetworks
253239
)
254240
const reachableApiBaseUrl = yield* _(
255-
waitForReachableApiBaseUrl(candidateUrls, context.currentContainerNetworks, controllerNetworks)
241+
waitForReachableApiBaseUrl(
242+
candidateUrls,
243+
context.currentContainerNetworks,
244+
controllerNetworks,
245+
context.explicitApiBaseUrl === undefined ? context.localControllerRevision : undefined
246+
)
256247
)
257248
rememberSelectedApiBaseUrl(reachableApiBaseUrl)
258249
})
@@ -292,7 +283,8 @@ export const ensureControllerReady = (): ControllerEffect<void> =>
292283
findReachableDirectHealthProbe({
293284
cachedApiBaseUrl: selectedApiBaseUrl,
294285
defaultLocalApiBaseUrl,
295-
explicitApiBaseUrl
286+
explicitApiBaseUrl,
287+
expectedRevision: localControllerRevision
296288
})
297289
)
298290
if (

‎packages/app/src/web/open-url.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export const prepareOpenUrl = (): PreparedOpenUrl => {
2020
if (typeof globalThis.open !== "function") {
2121
return blockedPreparedOpenUrl()
2222
}
23-
const openedWindow = globalThis.open("about:blank", "_blank", "noopener")
23+
const openedWindow = globalThis.open("about:blank", "_blank")
2424
if (openedWindow === null) {
2525
return blockedPreparedOpenUrl()
2626
}

0 commit comments

Comments
 (0)