fix(web): retain prepared browser popup handle

Author: skulidropek

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()
   }

packages/app/tests/docker-git/actions-browser.test.ts

@@ -67,7 +67,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 +89,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."

packages/app/tests/docker-git/actions-skiller.test.ts

@@ -87,7 +87,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)

packages/app/tests/docker-git/open-url.test.ts

@@ -0,0 +1,47 @@
+import { describe, expect, it } from "@effect/vitest"
+import { afterEach, vi } from "vitest"
+
+import { openUrl, prepareOpenUrl } from "../../src/web/open-url.js"
+import { makeBrowserOpenMockWindow, stubBrowserOpen } from "./browser-open-fixture.js"
+
+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)
+  })
+})