diff --git a/ui/src/components/task-side-panel/TaskComputerPanel.test.tsx b/ui/src/components/task-side-panel/TaskComputerPanel.test.tsx new file mode 100644 index 00000000000..c97a3be6890 --- /dev/null +++ b/ui/src/components/task-side-panel/TaskComputerPanel.test.tsx @@ -0,0 +1,119 @@ +// @vitest-environment jsdom +import { act } from "react"; +import { createRoot, type Root } from "react-dom/client"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { computersApi } from "@/api/computers"; +import { TaskComputerPanel } from "./TaskComputerPanel"; + +vi.mock("@/api/computers", () => ({ computersApi: { + connect: vi.fn(), presence: vi.fn(), disconnect: vi.fn(async () => {}), preview: vi.fn(), +} })); +(globalThis as typeof globalThis & { IS_REACT_ACT_ENVIRONMENT: boolean }).IS_REACT_ACT_ENVIRONMENT = true; +const owner = { computerId: "computer", ownerId: "viewer", generation: 1 }; +const now = new Date("2026-10-10T12:00:00Z"); +const viewer = () => ({ viewerUrl: "https://viewer.example/#private-token", expiresAt: new Date(now.getTime() + 120_000).toISOString(), owner }); +function deferred() { + let resolve!: (value: T) => void; + const promise = new Promise(done => { resolve = done; }); + return { promise, resolve }; +} +let root: Root, host: HTMLDivElement, hidden = false, paneVisible = true, unmounted = false; +async function render(props: { issueId?: string; environmentId?: string; active?: boolean } = {}) { + await act(async () => root.render()); +} +async function connect() { + const button = Array.from(host.querySelectorAll("button")).find(item => item.textContent === "Connect"); + expect(button).toBeTruthy(); + await act(async () => button!.click()); +} +async function tick(ms: number) { await act(async () => { await vi.advanceTimersByTimeAsync(ms); }); } +async function unmount() { await act(async () => root.unmount()); unmounted = true; } +beforeEach(() => { + vi.useFakeTimers(); vi.setSystemTime(now); vi.clearAllMocks(); hidden = false; paneVisible = true; unmounted = false; + vi.mocked(computersApi.connect).mockResolvedValue(viewer()); + vi.mocked(computersApi.presence).mockImplementation(async () => viewer()); + vi.mocked(computersApi.disconnect).mockResolvedValue(undefined); + Object.defineProperty(document, "visibilityState", { configurable: true, get: () => hidden ? "hidden" : "visible" }); + vi.spyOn(HTMLElement.prototype, "getClientRects").mockReturnValue([{}] as unknown as DOMRectList); + Object.defineProperty(Element.prototype, "checkVisibility", { configurable: true, value: () => paneVisible }); + host = document.createElement("div"); document.body.append(host); root = createRoot(host); +}); +afterEach(async () => { if (!unmounted) await unmount(); host.remove(); delete (Element.prototype as { checkVisibility?: unknown }).checkVisibility; vi.restoreAllMocks(); vi.useRealTimers(); }); + +describe("Computer connection lifetime", () => { + it("connects only after the explicit action and keeps credentials in the iframe with no referrer", async () => { + await render(); await tick(60_000); + expect(computersApi.connect).not.toHaveBeenCalled(); + expect(computersApi.presence).not.toHaveBeenCalled(); + expect(host.querySelector("iframe")).toBeNull(); + await connect(); + expect(computersApi.connect).toHaveBeenCalledWith("task-a", "environment-a"); + const iframe = host.querySelector("iframe")!; + expect(iframe.src).toBe(viewer().viewerUrl); + expect(iframe.getAttribute("referrerpolicy")).toBe("no-referrer"); + }); + + it("does not renew while inactive or hidden and expires the stale credential without another connect", async () => { + await render(); await connect(); + const renewals = vi.mocked(computersApi.presence).mock.calls.length; + await render({ active: false }); await tick(30_000); + expect(computersApi.presence).toHaveBeenCalledTimes(renewals); + hidden = true; + await render({ active: true }); await tick(30_000); + expect(computersApi.presence).toHaveBeenCalledTimes(renewals); + await tick(60_000); + expect(host.querySelector("iframe")).toBeNull(); + expect(host.textContent).toContain("connection expired"); + expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner); + expect(computersApi.connect).toHaveBeenCalledTimes(1); + }); + + it("does not keep the computer alive while its selected pane is collapsed", async () => { + await render(); await connect(); + const renewals = vi.mocked(computersApi.presence).mock.calls.length; + paneVisible = false; + await tick(30_000); + expect(computersApi.presence).toHaveBeenCalledTimes(renewals); + paneVisible = true; + await tick(30_000); + expect(computersApi.presence).toHaveBeenCalledTimes(renewals + 1); + }); + + it("revalidates when returning to a visible tab and clears an expired server hold", async () => { + await render(); await connect(); await render({ active: false }); + vi.mocked(computersApi.presence).mockRejectedValueOnce(new Error("Connect again")); + await render({ active: true }); + expect(host.querySelector("iframe")).toBeNull(); + expect(host.textContent).toContain("Connect again"); + expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner); + }); + + it.each(["issue", "environment"])("retires a late connect for an old %s without showing its desktop", async change => { + const pending = deferred>(); + vi.mocked(computersApi.connect).mockReturnValueOnce(pending.promise); + await render(); await connect(); + expect(host.textContent).toContain("Connecting"); + await render(change === "issue" ? { issueId: "task-b" } : { environmentId: "environment-b" }); + expect(host.textContent).not.toContain("Connecting"); + await act(async () => pending.resolve(viewer())); + expect(host.querySelector("iframe")).toBeNull(); + expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner); + }); + + it("retires connections resolved after unmount", async () => { + const pending = deferred>(); + vi.mocked(computersApi.connect).mockReturnValueOnce(pending.promise); + await render(); await connect(); await unmount(); + await act(async () => pending.resolve(viewer())); + expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner); + }); + + it("retires a connected owner on unmount even when its presence response is pending", async () => { + const pending = deferred>(); + vi.mocked(computersApi.presence).mockReturnValueOnce(pending.promise); + await render(); await connect(); await unmount(); + expect(computersApi.disconnect).toHaveBeenCalledWith("task-a", "environment-a", owner); + await act(async () => pending.resolve(viewer())); + expect(host.querySelector("iframe")).toBeNull(); + }); +}); diff --git a/ui/src/components/task-side-panel/TaskComputerPanel.tsx b/ui/src/components/task-side-panel/TaskComputerPanel.tsx index 14df73eae4e..04d5d4c6feb 100644 --- a/ui/src/components/task-side-panel/TaskComputerPanel.tsx +++ b/ui/src/components/task-side-panel/TaskComputerPanel.tsx @@ -5,16 +5,22 @@ import { computersApi } from "@/api/computers"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; -export function TaskComputerPanel({ issueId, environmentId, active = true }: { - issueId: string; - environmentId: string; - active?: boolean; -}) { +type ComputerPanelProps = { issueId: string; environmentId: string; active?: boolean }; + +export function TaskComputerPanel(props: ComputerPanelProps) { + // A connection belongs to one task/environment. A new scope gets new state, + // so a late response can only retire its old owner, never install its viewer. + return ; +} + +function ComputerConnection({ issueId, environmentId, active = true }: ComputerPanelProps) { const [viewer, setViewer] = useState(null); const [connecting, setConnecting] = useState(false); const [error, setError] = useState(null); const current = useRef(null); + const frameRef = useRef(null); const mounted = useRef(true); + const connectPending = useRef(false); const [port, setPort] = useState("5173"); const [previewPending, setPreviewPending] = useState(false); @@ -26,11 +32,18 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: { setError(null); try { const result = await computersApi.preview(issueId, environmentId, Number(port)); - previewWindow.location.replace(result.url); + if (mounted.current) previewWindow.location.replace(result.url); + else previewWindow.close(); } catch (cause) { previewWindow.close(); - setError(cause instanceof Error ? cause.message : "Could not open the preview."); - } finally { setPreviewPending(false); } + if (mounted.current) setError(cause instanceof Error ? cause.message : "Could not open the preview."); + } finally { if (mounted.current) setPreviewPending(false); } + } + + function release(view: ComputerViewer) { + void computersApi.disconnect(issueId, environmentId, view.owner).catch(() => { + // The server's bounded presence deadline also releases lost viewers. + }); } useEffect(() => { @@ -39,20 +52,19 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: { mounted.current = false; const previous = current.current; current.current = null; - if (previous) void computersApi.disconnect(issueId, environmentId, previous.owner).catch(() => { - // The server's bounded presence deadline also releases lost viewers. - }); + if (previous) release(previous); }; }, [issueId, environmentId]); async function connect() { - if (connecting) return; + if (connectPending.current) return; + connectPending.current = true; setConnecting(true); setError(null); try { const result = await computersApi.connect(issueId, environmentId); if (!mounted.current) { - await computersApi.disconnect(issueId, environmentId, result.owner); + release(result); return; } current.current = result; @@ -60,6 +72,7 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: { } catch (cause) { if (mounted.current) setError(cause instanceof Error ? cause.message : "Could not connect to the computer."); } finally { + connectPending.current = false; if (mounted.current) setConnecting(false); } } @@ -69,24 +82,34 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: { let disposed = false; let pending = false; const renew = async () => { - if (pending || !active || document.visibilityState === "hidden") return; + const frame = frameRef.current; + if (pending || !active || document.visibilityState === "hidden" || !frame || !frame.getClientRects().length || + (frame.checkVisibility && !frame.checkVisibility({ checkOpacity: true, checkVisibilityCSS: true }))) return; pending = true; try { const next = await computersApi.presence(issueId, environmentId, viewer.owner); - if (!disposed) { - current.current = next; - setViewer(next); + if (disposed || !mounted.current) { + // It may have renewed the old owner while this pane was closing. + if (current.current?.owner.ownerId !== next.owner.ownerId) release(next); + return; } + current.current = next; + setViewer(next); } catch (cause) { if (!disposed) { + const previous = current.current; current.current = null; setViewer(null); + if (previous) release(previous); setError(cause instanceof Error ? cause.message : "The connection ended. Connect to try again."); } } finally { pending = false; } }; const timer = setInterval(() => void renew(), 30_000); document.addEventListener("visibilitychange", renew); + // A hidden pane may have outlived its server hold. Revalidate immediately + // when it becomes active, before waiting for the next periodic renewal. + void renew(); return () => { disposed = true; clearInterval(timer); @@ -94,7 +117,21 @@ export function TaskComputerPanel({ issueId, environmentId, active = true }: { }; }, [issueId, environmentId, viewer?.owner.ownerId, active]); + useEffect(() => { + if (!viewer) return; + const expiresAt = Date.parse(viewer.expiresAt); + const timer = setTimeout(() => { + if (current.current !== viewer) return; + current.current = null; + setViewer(null); + setError("The connection expired. Connect to try again."); + release(viewer); + }, Math.max(0, Number.isFinite(expiresAt) ? expiresAt - Date.now() : 0)); + return () => clearTimeout(timer); + }, [viewer]); + const desktop = viewer ?