diff --git a/packages/api/src/services/project-browser.ts b/packages/api/src/services/project-browser.ts index 4517c3bf..0f12ab4d 100644 --- 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() const browserWebSocketCounts = new Map() +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 \" + 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 => 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 index d5574be7..24a7911e 100644 --- a/packages/app/src/docker-git/controller-health.ts +++ b/packages/app/src/docker-git/controller-health.ts @@ -31,7 +31,9 @@ const parseHealthRevision = (text: string): string | null => } }) -const probeHealth = (apiBaseUrl: string): Effect.Effect => +const probeHealth = ( + apiBaseUrl: string +): Effect.Effect => Effect.gen(function*(_) { const client = yield* _(HttpClient.HttpClient) const response = yield* _(client.get(`${apiBaseUrl}/health`, { headers: { accept: "application/json" } })) @@ -52,7 +54,6 @@ const probeHealth = (apiBaseUrl: string): Effect.Effect error._tag === "ControllerBootstrapError" ? error @@ -64,8 +65,9 @@ const probeHealth = (apiBaseUrl: string): Effect.Effect -): Effect.Effect => + candidateUrls: ReadonlyArray, + expectedRevision?: string +): Effect.Effect => Effect.gen(function*(_) { if (candidateUrls.length === 0) { return yield* _( @@ -73,36 +75,88 @@ const findReachableHealthProbe = ( ) } + const mismatches: Array = [] 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 = ( - candidateUrls: ReadonlyArray -): Effect.Effect => - findReachableHealthProbe(candidateUrls).pipe( + candidateUrls: ReadonlyArray, + expectedRevision?: string +): Effect.Effect => + findReachableHealthProbe(candidateUrls, expectedRevision).pipe( Effect.match({ onFailure: () => null, onSuccess: (probe) => probe }) ) +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 +): 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 findReachableApiBaseUrlWithHttpClient = ( + candidateUrls: ReadonlyArray, + expectedRevision?: string +): Effect.Effect => + findReachableHealthProbe(candidateUrls, expectedRevision).pipe(Effect.map(({ apiBaseUrl }) => apiBaseUrl)) + export const findReachableApiBaseUrl = ( - candidateUrls: ReadonlyArray + candidateUrls: ReadonlyArray, + expectedRevision?: string ): Effect.Effect => - findReachableHealthProbe(candidateUrls).pipe(Effect.map(({ apiBaseUrl }) => apiBaseUrl)) + findReachableApiBaseUrlWithHttpClient(candidateUrls, expectedRevision).pipe(Effect.provide(FetchHttpClient.layer)) -export const findReachableDirectHealthProbe = (options: { +// 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, + expectedRevision: string +): Effect.Effect => findReachableApiBaseUrl(candidateUrls, expectedRevision) + +type DirectHealthProbeOptions = { readonly explicitApiBaseUrl: string | undefined readonly defaultLocalApiBaseUrl: string | undefined readonly cachedApiBaseUrl: string | undefined -}): Effect.Effect => + readonly expectedRevision?: string | undefined +} + +export const findReachableDirectHealthProbeWithHttpClient = ( + options: DirectHealthProbeOptions +): Effect.Effect => findReachableHealthProbeOrNull( buildApiBaseUrlCandidates({ explicitApiBaseUrl: options.explicitApiBaseUrl, @@ -112,5 +166,11 @@ export const findReachableDirectHealthProbe = (options: { currentContainerNetworks: {}, controllerNetworks: {}, port: resolveApiPort() - }) + }), + options.expectedRevision ) + +export const findReachableDirectHealthProbe = ( + options: DirectHealthProbeOptions +): Effect.Effect => + findReachableDirectHealthProbeWithHttpClient(options).pipe(Effect.provide(FetchHttpClient.layer)) diff --git a/packages/app/src/docker-git/controller-reachability-diagnostics.ts b/packages/app/src/docker-git/controller-reachability-diagnostics.ts new file mode 100644 index 00000000..fce05aa3 --- /dev/null +++ b/packages/app/src/docker-git/controller-reachability-diagnostics.ts @@ -0,0 +1,44 @@ +import { Effect } from "effect" + +import * as ControllerDocker from "./controller-docker.js" +import { type DockerNetworkIps, formatNetworkIps } from "./controller-reachability.js" + +// CHANGE: document controller reachability diagnostics as a SHELL effect. +// WHY: diagnostics inspect published controller ports and must make their runtime dependency explicit. +// QUOTE(ТЗ): n/a +// REF: PR-360-coderabbit-reachability-diagnostics-contract +// SOURCE: n/a +// FORMAT THEOREM: forall candidates C: diagnostics(C) returns a string containing attempted endpoints and runtime network state. +// PURITY: SHELL +// EFFECT: Effect +// INVARIANT: every returned diagnostic string includes candidate URLs, published ports, current runtime networks, and controller networks. +// COMPLEXITY: O(n) where n = |candidateUrls|. +/** + * Collects host/controller reachability diagnostics for failed bootstrap probes. + * + * @param candidateUrls - API base URL candidates attempted by the bootstrap path. + * @param currentContainerNetworks - network IPs visible from the current runtime container. + * @param controllerNetworks - network IPs attached to the docker-git controller container. + * @returns Effect.Effect + * + * @pure false + * @effect Requires ControllerDocker.inspectControllerPublishedPorts through ControllerDocker.ControllerRuntime. + * @invariant The result describes controller runtime endpoints, published ports, and network visibility. + * @complexity O(n) where n is candidateUrls.length. + */ +export const collectReachabilityDiagnostics = ( + candidateUrls: ReadonlyArray, + currentContainerNetworks: DockerNetworkIps, + controllerNetworks: DockerNetworkIps +): Effect.Effect => + 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 index 9eba5957..824243a4 100644 --- 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, - currentContainerNetworks: DockerNetworkIps, - controllerNetworks: DockerNetworkIps -): Effect.Effect => - 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, currentContainerNetworks: DockerNetworkIps, - controllerNetworks: DockerNetworkIps + controllerNetworks: DockerNetworkIps, + expectedRevision: string | undefined ): ControllerEffect => 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 + candidateUrls: ReadonlyArray, + expectedRevision: string | undefined ): Effect.Effect => - 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) }) @@ -292,7 +283,8 @@ export const ensureControllerReady = (): ControllerEffect => findReachableDirectHealthProbe({ cachedApiBaseUrl: selectedApiBaseUrl, defaultLocalApiBaseUrl, - explicitApiBaseUrl + explicitApiBaseUrl, + expectedRevision: localControllerRevision }) ) if ( diff --git a/packages/app/src/web/open-url.ts b/packages/app/src/web/open-url.ts index 78e967b4..d4ab5582 100644 --- a/packages/app/src/web/open-url.ts +++ b/packages/app/src/web/open-url.ts @@ -20,7 +20,7 @@ export const prepareOpenUrl = (): PreparedOpenUrl => { if (typeof globalThis.open !== "function") { return blockedPreparedOpenUrl() } - const openedWindow = globalThis.open("about:blank", "_blank", "noopener") + const openedWindow = globalThis.open("about:blank", "_blank") if (openedWindow === null) { return blockedPreparedOpenUrl() } diff --git a/packages/app/tests/docker-git/actions-browser.test.ts b/packages/app/tests/docker-git/actions-browser.test.ts index 783c266f..d7815b73 100644 --- a/packages/app/tests/docker-git/actions-browser.test.ts +++ b/packages/app/tests/docker-git/actions-browser.test.ts @@ -1,5 +1,7 @@ +/* jscpd:ignore-start */ import { describe, expect, it } from "@effect/vitest" import { Effect } from "effect" +import * as fc from "fast-check" import { afterEach, beforeEach, vi } from "vitest" import { openProjectBrowserById, openSelectedProjectBrowser } from "../../src/web/actions-browser.js" @@ -37,6 +39,15 @@ const missingBrowser: ProjectBrowserSession = { status: "missing" } +const pathSegmentChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" +const pathSegmentCharArbitrary = fc + .integer({ max: pathSegmentChars.length - 1, min: 0 }) + .map((index) => pathSegmentChars.charAt(index)) +const projectBrowserPathArbitrary = (suffix: string) => + fc + .array(pathSegmentCharArbitrary, { minLength: 1, maxLength: 24 }) + .map((chars) => `/api/projects/${chars.join("")}/browser/${suffix}`) + describe("web browser actions", () => { let openedWindow: BrowserOpenMockWindow = makeBrowserOpenMockWindow() @@ -67,7 +78,7 @@ describe("web browser actions", () => { expect(setProjectBrowser).toHaveBeenCalledWith(runningBrowser) })) - expect(openMock).toHaveBeenCalledWith("about:blank", "_blank", "noopener") + expect(openMock).toHaveBeenCalledWith("about:blank", "_blank") expect(openedWindow.location.href).toBe("/api/projects/project-1/browser/novnc") expect(setMessage).toHaveBeenLastCalledWith("Browser opened. CDP endpoint: /api/projects/project-1/browser/cdp.") })) @@ -89,7 +100,7 @@ describe("web browser actions", () => { })) expect(startProjectBrowserMock).toHaveBeenCalledWith("project-1") - expect(openMock).toHaveBeenCalledWith("about:blank", "_blank", "noopener") + expect(openMock).toHaveBeenCalledWith("about:blank", "_blank") expect(openedWindow.close).toHaveBeenCalledOnce() expect(setMessage).toHaveBeenLastCalledWith( "Browser runtime is missing. Enable Playwright MCP and start the project first." @@ -104,4 +115,73 @@ describe("web browser actions", () => { expect(startProjectBrowserMock).not.toHaveBeenCalled() expect(setMessage).toHaveBeenLastCalledWith("No project selected.") }) + + it.effect("keeps popup and browser state messages consistent for generated browser URLs", () => + Effect.tryPromise({ + catch: (error) => error, + try: () => + fc.assert( + fc.asyncProperty( + projectBrowserPathArbitrary("novnc"), + projectBrowserPathArbitrary("cdp"), + fc.boolean(), + fc.boolean(), + (noVncPath, cdpPath, popupAllowed, useSelectedProject) => + Effect.runPromise( + Effect.gen(function*(_) { + vi.unstubAllGlobals() + startProjectBrowserMock.mockReset() + loadProjectBrowserMock.mockReset() + openedWindow = makeBrowserOpenMockWindow() + + if (popupAllowed) { + stubBrowserOpen(openedWindow) + } else { + vi.stubGlobal("open", null) + } + + const browser = { + ...runningBrowser, + cdpPath, + noVncPath, + projectId: "project-1" + } + startProjectBrowserMock.mockImplementation(() => Effect.succeed(browser)) + const { context, setMessage, setProjectBrowser } = makeBrowserActionContext({ + selectedProjectId: "project-1", + selectedProjectName: "octocat/hello-world" + }) + + if (useSelectedProject) { + openSelectedProjectBrowser(context) + } else { + openProjectBrowserById("project-1", context) + } + + yield* _(waitForAssertion(() => { + expect(setProjectBrowser).toHaveBeenCalledWith(browser) + })) + + expect(startProjectBrowserMock).toHaveBeenCalledWith("project-1") + expect(setMessage).toHaveBeenLastCalledWith( + popupAllowed + ? `Browser opened. CDP endpoint: ${cdpPath}.` + : `Browser popup was blocked. Open ${noVncPath} manually. CDP endpoint: ${cdpPath}.` + ) + + if (popupAllowed) { + expect(openedWindow.location.href).toBe(noVncPath) + expect(openedWindow.focus).toHaveBeenCalledOnce() + } + }).pipe( + Effect.ensuring(Effect.sync(() => { + vi.unstubAllGlobals() + })) + ) + ) + ), + { numRuns: 20 } + ) + })) }) +/* jscpd:ignore-end */ diff --git a/packages/app/tests/docker-git/actions-skiller.test.ts b/packages/app/tests/docker-git/actions-skiller.test.ts index ba23515b..c62011c4 100644 --- a/packages/app/tests/docker-git/actions-skiller.test.ts +++ b/packages/app/tests/docker-git/actions-skiller.test.ts @@ -1,5 +1,7 @@ +/* jscpd:ignore-start */ import { describe, expect, it } from "@effect/vitest" import { Effect } from "effect" +import * as fc from "fast-check" import { afterEach, beforeEach, vi } from "vitest" import { openSkillerApp } from "../../src/web/actions-skiller.js" @@ -62,6 +64,17 @@ const mockScopedSkillerLaunch = (): void => { ) } +const skillerPathChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_" +const skillerPathCharArbitrary = fc + .integer({ max: skillerPathChars.length - 1, min: 0 }) + .map((index) => skillerPathChars.charAt(index)) +const skillerPathArbitrary = fc + .array(skillerPathCharArbitrary, { minLength: 1, maxLength: 24 }) + .map((chars) => `/api/skiller/app/${chars.join("")}/`) +const projectKeyArbitrary = fc + .array(skillerPathCharArbitrary, { minLength: 1, maxLength: 18 }) + .map((chars) => chars.join("")) + vi.mock("../../src/web/api.js", () => ({ openSkiller: openSkillerMock })) @@ -87,7 +100,7 @@ describe("web Skiller actions", () => { openSkillerApp(context) - expect(browserOpenMock).toHaveBeenCalledWith("about:blank", "_blank", "noopener") + expect(browserOpenMock).toHaveBeenCalledWith("about:blank", "_blank") expect(setMessage).toHaveBeenCalledWith("Opening Skiller...") yield* _(waitForAssertion(() => { expect(openSkillerMock).toHaveBeenCalledWith(undefined, undefined) @@ -169,4 +182,53 @@ describe("web Skiller actions", () => { expect(setMessage).toHaveBeenCalledWith("Skiller failed") })) })) + + it.effect("keeps Skiller popup URL and launch message consistent for generated app paths", () => + Effect.tryPromise({ + catch: (error) => error, + try: () => + fc.assert( + fc.asyncProperty(skillerPathArbitrary, projectKeyArbitrary, (appPath, projectKey) => + Effect.runPromise( + Effect.gen(function*(_) { + vi.unstubAllGlobals() + vi.clearAllMocks() + openedWindow = makeBrowserOpenMockWindow() + browserOpenMock = stubBrowserOpen(openedWindow) + const scopedLaunch = skillerLaunch({ + appPath, + scope: { + ...proofScope, + projectKey + } + }) + openSkillerMock.mockImplementation(() => Effect.succeed(scopedLaunch)) + const { context, setMessage } = makeBrowserActionContext({ selectedProjectKey: projectKey }) + + openSkillerApp(context) + + expect(browserOpenMock).toHaveBeenCalledWith("about:blank", "_blank") + yield* _(waitForAssertion(() => { + expect(openSkillerMock).toHaveBeenCalledWith(projectKey, undefined) + })) + yield* _(waitForAssertion(() => { + expect(openedWindow.location.href).toBe(appPath) + expect(openedWindow.focus).toHaveBeenCalledOnce() + expect(setMessage).toHaveBeenCalledWith( + `Skiller launch started (pid 1234). Log: /home/dev/.docker-git/logs/skiller.log. ` + + `Container FS: dg-project:/home/dev/app. Opened ${appPath}.` + ) + })) + expect(context.setBusyLabel).toHaveBeenCalledWith("Opening Skiller") + expect(context.setBusyLabel).toHaveBeenLastCalledWith(null) + }).pipe( + Effect.ensuring(Effect.sync(() => { + vi.unstubAllGlobals() + })) + ) + )), + { numRuns: 20 } + ) + })) }) +/* jscpd:ignore-end */ diff --git a/packages/app/tests/docker-git/controller-health.test.ts b/packages/app/tests/docker-git/controller-health.test.ts new file mode 100644 index 00000000..f3bd375e --- /dev/null +++ b/packages/app/tests/docker-git/controller-health.test.ts @@ -0,0 +1,123 @@ +/* jscpd:ignore-start */ +import { HttpClient, HttpClientResponse } from "@effect/platform" +import { describe, expect, it } from "@effect/vitest" +import { Effect } from "effect" +import * as fc from "fast-check" + +import { + findReachableApiBaseUrlWithHttpClient, + findReachableDirectHealthProbeWithHttpClient +} from "../../src/docker-git/controller-health.js" + +const expectedRevision = "local-revision" + +const mismatchRevisionArbitrary = fc + .integer({ min: 1, max: 10_000 }) + .map((index) => `old-revision-${index}`) + +const candidateUrl = (baseUrl: string, index: number): string => `${baseUrl}/candidate-${index}` + +const toCandidateUrls = (baseUrl: string, revisions: ReadonlyArray): ReadonlyArray => + revisions.map((_, index) => candidateUrl(baseUrl, index)) + +const parseCandidateIndex = (pathname: string): number | null => { + const match = /^\/candidate-(\d+)\/health$/u.exec(pathname) + if (match === null) { + return null + } + const parsed = Number(match[1]) + return Number.isSafeInteger(parsed) ? parsed : null +} + +const healthResponse = (revisions: ReadonlyArray, pathname: string): Response => { + const index = parseCandidateIndex(pathname) + const revision = index === null ? undefined : revisions[index] + if (revision === undefined) { + return Response.json({ ok: false }, { status: 404 }) + } + + return Response.json({ ok: true, revision }) +} + +const makeHealthClient = (revisions: ReadonlyArray): HttpClient.HttpClient => + HttpClient.make((request, url) => + Effect.succeed(HttpClientResponse.fromWeb(request, healthResponse(revisions, url.pathname))) + ) + +const withHealthClient = ( + revisions: ReadonlyArray, + effect: (baseUrl: string) => Effect.Effect +): Effect.Effect => + effect("http://controller.test").pipe(Effect.provideService(HttpClient.HttpClient, makeHealthClient(revisions))) + +describe("controller health", () => { + it.effect("selects the first reachable candidate whose revision matches the expected revision", () => + Effect.tryPromise({ + catch: (error) => error, + try: () => + fc.assert( + fc.asyncProperty( + fc.array(mismatchRevisionArbitrary, { maxLength: 4 }), + fc.array(mismatchRevisionArbitrary, { maxLength: 4 }), + (before, after) => + Effect.runPromise( + withHealthClient([...before, expectedRevision, ...after], (baseUrl) => + Effect.gen(function*(_) { + const candidates = toCandidateUrls(baseUrl, [...before, expectedRevision, ...after]) + const selected = yield* _( + findReachableApiBaseUrlWithHttpClient(candidates, expectedRevision) + ) + + expect(selected).toBe(candidateUrl(baseUrl, before.length)) + })) + ) + ), + { numRuns: 20 } + ) + })) + + it.effect("reports every reachable mismatched revision when no candidate matches", () => + Effect.tryPromise({ + catch: (error) => error, + try: () => + fc.assert( + fc.asyncProperty( + fc.array(mismatchRevisionArbitrary, { minLength: 1, maxLength: 5 }), + (revisions) => + Effect.runPromise( + withHealthClient(revisions, (baseUrl) => + Effect.gen(function*(_) { + const candidates = toCandidateUrls(baseUrl, revisions) + const error = yield* _( + findReachableApiBaseUrlWithHttpClient(candidates, expectedRevision).pipe(Effect.flip) + ) + + expect(error.message).toContain(expectedRevision) + for (const [index, revision] of revisions.entries()) { + expect(error.message).toContain(candidateUrl(baseUrl, index)) + expect(error.message).toContain(revision) + } + })) + ) + ), + { numRuns: 20 } + ) + })) + + it.effect("filters direct health probes by expected revision before bootstrap", () => + withHealthClient(["old-revision", expectedRevision], (baseUrl) => + Effect.gen(function*(_) { + const probe = yield* _( + findReachableDirectHealthProbeWithHttpClient({ + cachedApiBaseUrl: candidateUrl(baseUrl, 1), + defaultLocalApiBaseUrl: candidateUrl(baseUrl, 0), + explicitApiBaseUrl: undefined, + expectedRevision + }) + ) + + expect(probe?.apiBaseUrl).toBe(candidateUrl(baseUrl, 1)) + expect(probe?.revision).toBe(expectedRevision) + }))) +}) +/* jscpd:ignore-end */ diff --git a/packages/app/tests/docker-git/controller-ready.test.ts b/packages/app/tests/docker-git/controller-ready.test.ts index 13040002..d6d431a7 100644 --- a/packages/app/tests/docker-git/controller-ready.test.ts +++ b/packages/app/tests/docker-git/controller-ready.test.ts @@ -133,7 +133,8 @@ describe("controller readiness bootstrap", () => { expect(findReachableDirectHealthProbeMock).toHaveBeenCalledWith({ cachedApiBaseUrl: undefined, defaultLocalApiBaseUrl: "http://127.0.0.1:3334", - explicitApiBaseUrl: undefined + explicitApiBaseUrl: undefined, + expectedRevision: "local-revision" }) expect(prepareLocalControllerRevisionMock).toHaveBeenCalled() expect(prepareControllerResourceLimitEnvMock).toHaveBeenCalledTimes(1) diff --git a/packages/app/tests/docker-git/core-templates.test.ts b/packages/app/tests/docker-git/core-templates.test.ts index c8f7fbc0..904b036f 100644 --- a/packages/app/tests/docker-git/core-templates.test.ts +++ b/packages/app/tests/docker-git/core-templates.test.ts @@ -72,7 +72,9 @@ describe("app planFiles", () => { expect(entrypoint.contents).toContain("docker_git_stop_playwright_browser()") expect(entrypoint.contents).toContain("docker-git-browser-connection") expect(entrypoint.contents).toContain("stop --project \"$project_container\"") - expect(entrypoint.contents).toContain('command = "browser-connection"') - expect(entrypoint.contents).toContain('args = ["--project", "$DOCKER_GIT_BROWSER_PROJECT", "--network", "$DOCKER_GIT_BROWSER_NETWORK"]') + expect(entrypoint.contents).toContain("command = \"browser-connection\"") + expect(entrypoint.contents).toContain( + "args = [\"--project\", \"$DOCKER_GIT_BROWSER_PROJECT\", \"--network\", \"$DOCKER_GIT_BROWSER_NETWORK\"]" + ) }) }) diff --git a/packages/app/tests/docker-git/open-url.test.ts b/packages/app/tests/docker-git/open-url.test.ts new file mode 100644 index 00000000..e808a5ca --- /dev/null +++ b/packages/app/tests/docker-git/open-url.test.ts @@ -0,0 +1,104 @@ +/* jscpd:ignore-start */ +import { describe, expect, it } from "@effect/vitest" +import * as fc from "fast-check" +import { afterEach, vi } from "vitest" + +import { openUrl, prepareOpenUrl } from "../../src/web/open-url.js" +import { makeBrowserOpenMockWindow, stubBrowserOpen } from "./browser-open-fixture.js" + +const urlChars = "abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789-_/.:?=&" +const urlCharArbitrary = fc.integer({ max: urlChars.length - 1, min: 0 }).map((index) => urlChars.charAt(index)) +const urlArbitrary = fc.array(urlCharArbitrary, { minLength: 1, maxLength: 80 }).map((chars) => chars.join("")) + +describe("open-url helpers", () => { + afterEach(() => { + vi.unstubAllGlobals() + }) + + it("opens prepared async popups without noopener so the caller keeps the window handle", () => { + const openedWindow = makeBrowserOpenMockWindow() + const openMock = stubBrowserOpen(openedWindow) + + const prepared = prepareOpenUrl() + + expect(openMock).toHaveBeenCalledWith("about:blank", "_blank") + expect(openedWindow.opener).toBeNull() + expect(prepared.navigate("/api/projects/project-1/browser/novnc")).toBe(true) + expect(openedWindow.location.href).toBe("/api/projects/project-1/browser/novnc") + expect(openedWindow.focus).toHaveBeenCalledOnce() + + prepared.close() + + expect(openedWindow.close).toHaveBeenCalledOnce() + }) + + it("falls back to direct noopener open when the prepared popup is blocked", () => { + const openedWindow = makeBrowserOpenMockWindow() + const openMock = vi.fn((url: string) => url === "about:blank" ? null : openedWindow) + vi.stubGlobal("open", openMock) + + const prepared = prepareOpenUrl() + + expect(prepared.navigate("/api/projects/project-1/browser/novnc")).toBe(true) + expect(openMock).toHaveBeenNthCalledWith(1, "about:blank", "_blank") + expect(openMock).toHaveBeenNthCalledWith(2, "/api/projects/project-1/browser/novnc", "_blank", "noopener") + }) + + it("reports direct opens as blocked when no browser open function exists", () => { + vi.stubGlobal("open", null) + + expect(openUrl("/api/projects/project-1/browser/novnc")).toBe(false) + expect(prepareOpenUrl().navigate("/api/projects/project-1/browser/novnc")).toBe(false) + }) + + it("reports unavailable browser open for every target URL", () => { + fc.assert( + fc.property(urlArbitrary, (url) => { + vi.stubGlobal("open", null) + + expect(openUrl(url)).toBe(false) + expect(prepareOpenUrl().navigate(url)).toBe(false) + }), + { numRuns: 30 } + ) + }) + + it("falls back to noopener direct open for every target URL when the prepared popup is blocked", () => { + fc.assert( + fc.property(urlArbitrary, (url) => { + const openedWindow = makeBrowserOpenMockWindow() + const openMock = vi.fn((targetUrl: string) => targetUrl === "about:blank" ? null : openedWindow) + vi.stubGlobal("open", openMock) + + const prepared = prepareOpenUrl() + + expect(prepared.navigate(url)).toBe(true) + expect(openMock).toHaveBeenNthCalledWith(1, "about:blank", "_blank") + expect(openMock).toHaveBeenNthCalledWith(2, url, "_blank", "noopener") + }), + { numRuns: 30 } + ) + }) + + it("navigates prepared popups for every target URL without losing the window handle", () => { + fc.assert( + fc.property(urlArbitrary, (url) => { + const openedWindow = makeBrowserOpenMockWindow() + const openMock = stubBrowserOpen(openedWindow) + + const prepared = prepareOpenUrl() + + expect(openMock).toHaveBeenCalledWith("about:blank", "_blank") + expect(prepared.navigate(url)).toBe(true) + expect(openedWindow.location.href).toBe(url) + expect(openedWindow.focus).toHaveBeenCalledOnce() + + prepared.close() + + expect(openedWindow.close).toHaveBeenCalledOnce() + }), + { numRuns: 30 } + ) + }) +}) +/* jscpd:ignore-end */