From c7d93ae35fa99630d636234d930d0810b3e06e91 Mon Sep 17 00:00:00 2001 From: Michael Johnston Date: Mon, 9 Nov 2026 19:04:52 -0700 Subject: [PATCH 1/2] fix(tests): synchronize thread-link popups and conversation input Close custom-protocol popup targets only after source input dispatch finishes. Keep keyboard focus coverage inside the visible browser area, wait for interactive rows, and click explicit viewport padding. Carry-Group: incubator Carry-Observation: Feature-specific Browser thread-link and conversation focus regressions stay with their Incubator features. --- apps/server/src/preview/ThreadLinkNavigation.test.ts | 7 ++++++- .../components/chat/MessagesTimeline.browser.test.tsx | 9 +++++++-- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/apps/server/src/preview/ThreadLinkNavigation.test.ts b/apps/server/src/preview/ThreadLinkNavigation.test.ts index 93249af710e84..1efc4448043f2 100644 --- a/apps/server/src/preview/ThreadLinkNavigation.test.ts +++ b/apps/server/src/preview/ThreadLinkNavigation.test.ts @@ -33,13 +33,14 @@ it("keeps clicked thread navigations in the app, including hosted new windows", const browserCdp = await browser.newBrowserCDPSession(); const { targetInfo: sourceTarget } = await cdp.send("Target.getTargetInfo"); let closeProtocolPopup = false; + let created = Promise.withResolvers(); browserCdp.on("Target.targetCreated", ({ targetInfo }) => { if ( closeProtocolPopup && targetInfo.type === "page" && targetInfo.targetId !== sourceTarget.targetId ) - void browserCdp.send("Target.closeTarget", { targetId: targetInfo.targetId }); + created.resolve(targetInfo.targetId); }); await browserCdp.send("Target.setDiscoverTargets", { discover: true }); await context.route(hosted, (route) => @@ -76,6 +77,7 @@ it("keeps clicked thread navigations in the app, including hosted new windows", ["#hostedPopup", hosted], ]) { opened = Promise.withResolvers(); + created = Promise.withResolvers(); // A custom-protocol target has no document to finish initialising. Close // the real Chromium target to exercise its uncommitted, already-closed popup event. closeProtocolPopup = selector === "#popup" || selector === "#customPopup"; @@ -83,6 +85,9 @@ it("keeps clicked thread navigations in the app, including hosted new windows", ? new Promise((resolve) => page.once("popup", resolve)) : undefined; await page.locator(selector!).click(); + // Closing during window.open can strand Chromium's source input dispatch. + if (closeProtocolPopup) + await browserCdp.send("Target.closeTarget", { targetId: await created.promise }); expect(await opened.promise).toBe(expected); if (detached) { const popup = await detached; diff --git a/apps/web/src/components/chat/MessagesTimeline.browser.test.tsx b/apps/web/src/components/chat/MessagesTimeline.browser.test.tsx index 420bfcac41bf3..63c781eb3571f 100644 --- a/apps/web/src/components/chat/MessagesTimeline.browser.test.tsx +++ b/apps/web/src/components/chat/MessagesTimeline.browser.test.tsx @@ -362,6 +362,8 @@ describe("MessagesTimeline messages", () => { ); await using screen = await renderElement(conversation([])); + // Keep the conversation and composer visible so input does not scroll the test document. + screen.container.style.height = "min(600px, 100vh)"; const focus = createWorkspaceFocusController(document); using cleanup = new DisposableStack(); cleanup.defer(focus.dispose); @@ -370,6 +372,9 @@ describe("MessagesTimeline messages", () => { expect(document.activeElement?.getAttribute("data-workspace-area")).toBe("conversation"); await screen.rerender(conversation(entries)); const viewport = props.listRef.current!.getScrollableNode()!; + // Reserve viewport padding that cannot hit a virtual row or one of its controls. + viewport.style.paddingRight = "32px"; + await expect.element(screen.getByText("Turn 39:")).toHaveStyle({ pointerEvents: "auto" }); await vi.waitFor(() => expect(viewport.scrollTop).toBeGreaterThan(0)); const atEnd = viewport.scrollTop; await userEvent.keyboard("{ArrowUp}"); @@ -379,8 +384,8 @@ describe("MessagesTimeline messages", () => { focus.move(-1); expect(document.activeElement).toBe(viewport); screen.button("Send").focus(); - // Click the blank lane between the minimap and the right-aligned message controls. - await userEvent.click(viewport, { position: { x: 100, y: 1 } }); + await userEvent.click(viewport, { position: { x: viewport.clientWidth - 16, y: 1 } }); + await vi.waitFor(() => expect(document.activeElement).toBe(viewport)); await userEvent.keyboard("{Home}"); await vi.waitFor(() => expect(viewport.scrollTop).toBe(0)); await userEvent.keyboard("{ArrowDown}"); From 75a1f862ca679446726dbc0364fcaad5223057a4 Mon Sep 17 00:00:00 2001 From: Michael Johnston Date: Mon, 9 Nov 2026 19:10:02 -0700 Subject: [PATCH 2/2] test(browser): prove interception of an already-closed popup Deliver an uncommitted closed popup through the controlled event fixture and assert its closed state when interception receives it. Carry-Group: incubator --- apps/server/src/preview/ThreadLinkNavigation.test.ts | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/apps/server/src/preview/ThreadLinkNavigation.test.ts b/apps/server/src/preview/ThreadLinkNavigation.test.ts index 1efc4448043f2..49e90590fd831 100644 --- a/apps/server/src/preview/ThreadLinkNavigation.test.ts +++ b/apps/server/src/preview/ThreadLinkNavigation.test.ts @@ -206,7 +206,7 @@ it("preserves HTTP caching and lets typed or agent-loaded hosted documents load" } }); -it("continues dropped opens and expires announcements before a later typed navigation", async () => { +it("handles closed thread popups, dropped opens, and expired announcements", async () => { const events = new NodeEvents.EventEmitter(); const send = vi.fn(async (method: string) => method === "Page.getFrameTree" ? { frameTree: { frame: { id: "main" } } } : {}, @@ -255,6 +255,14 @@ it("continues dropped opens and expires announcements before a later typed navig pause("dropped-open"); expect(navigation.isThreadNavigation(hosted)).toBe(false); open.mockReturnValue(true); + const closedPopup = { url: () => ":", isClosed: () => true, close: vi.fn(async () => {}) }; + blockPopup.mockImplementationOnce((received: Page) => { + expect(received.isClosed()).toBe(true); + }); + events.emit("Page.windowOpen", { url: href, userGesture: true }); + page.emit("popup", closedPopup); + expect(blockPopup).toHaveBeenCalledWith(closedPopup); + expect(closedPopup.close).toHaveBeenCalledOnce(); announce(); expect(navigation.isThreadNavigation(hosted)).toBe(true); gesture("inactive");