From df32e6ddcd8dc6785b249aee4d7ba107a2e1655e Mon Sep 17 00:00:00 2001 From: Ivan Malison Date: Tue, 4 Aug 2026 01:29:52 -0700 Subject: [PATCH 1/2] Fix the CI failures on main (#2850) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * fix(app): keep an open workspace menu from unmounting its own trigger The sidebar kebab is revealed by hover, and on a compact layout the menu it opens is a sheet that slides up over the row it came from. The row un-hovers, the trailing overlay unmounts the trigger, and the menu's open state goes with it — nothing is left that can close the surface already presented in the portal, so a full-screen backdrop swallows every click until reload. Lift the menu's open state to the component that decides whether to render the kebab, and keep the kebab rendered while the menu is up. Co-Authored-By: Claude Opus 5 * test(app): follow the sidebar and host badge redesigns in the e2e suite Three specs were still driving surfaces that moved out from under them, so CI has been red on main since the sidebar polish landed. Display preferences became one row per decision with the options a page down, so grouping and the host filter need that page walked first. Five specs had their own copy of the two-line dance; it lives in the sidebar helper now, and the host filter has its own opener next to the menu it belongs to. The host badge is plain secondary text on the meta line and its identity colour lives on the server icon alone, so the helpers stop asserting a tinted label and a pill fill that the redesign removed. Alignment rails are compared across canvas glyph metrics and SVG bounding boxes, so the residual depends on the host's font; a 1px and a 0.5px tolerance both landed on the wrong side of it in CI. Two pixels still catches the whole- pixel offsets these rails encode. The compact sheet spec also read its rails while the panel was still sliding in. Co-Authored-By: Claude Opus 5 * chore(app): let a browser e2e run take a private Metro cache Metro's on-disk cache is shared by every checkout on the machine. Editing one app source file rebuilds that module against everyone else's cached output and the Unistyles styles can come back desynced, which surfaces as alignment specs failing with real-looking pixel numbers that go away when the edit is reverted. E2E_METRO_CACHE_VERSION namespaces a run's cache without deleting anyone else's entries, so the cache can be ruled out before a layout failure is believed. Co-Authored-By: Claude Opus 5 * test(app): stop the chat scroll helper from hanging on a stalled sampler `scrollChatAwayFromBottom` waited on a promise that could only ever resolve from a `requestAnimationFrame` sampler started by a `wheel` listener. rAF does not fire while the page is occluded, which is reachable on a CI runner, so the sampler could stall on its first frame and the wait never returned — the test died on its own 240s timeout with nothing to read. A missed wheel event did the same. Settling the scroll before measuring is an optimization; the assertion after it is what decides the test. Sample on a timer with a deadline so a stall falls through to that assertion instead of consuming the test. Co-Authored-By: Claude Opus 5 * test(app): remove layout geometry e2e coverage Rail and pixel-alignment assertions depend on host rendering details and should not define the browser E2E contract. Preserve the surrounding behavior coverage while dropping the Metro cache escape hatch that only supported diagnosing those assertions. --------- Co-authored-by: Claude Opus 5 Co-authored-by: Mohamed Boudra --- docs/hover.md | 16 + ...alignment.spec.ts => changes-pane.spec.ts} | 349 +----------------- packages/app/e2e/browser/chat-outline.spec.ts | 5 - .../app/e2e/browser/new-workspace.spec.ts | 4 +- .../browser/sidebar-host-filter-multi.spec.ts | 4 +- .../sidebar-mobile-menu-sheets.spec.ts | 30 -- .../app/e2e/browser/sidebar-model-b.spec.ts | 81 +--- .../sidebar-workspace-pin-shortcut.spec.ts | 4 +- .../workspace-model-regressions.spec.ts | 4 +- .../browser/workspace-model-restart.spec.ts | 4 +- .../support/helpers/agent-bottom-anchor.ts | 62 ++-- .../app/e2e/support/helpers/chat-outline.ts | 20 - packages/app/e2e/support/helpers/hosts.ts | 47 +-- .../app/e2e/support/helpers/sheet-layout.ts | 31 +- packages/app/e2e/support/helpers/sidebar.ts | 13 + .../src/components/sidebar-workspace-list.tsx | 5 +- .../sidebar/sidebar-status-list.tsx | 7 +- .../sidebar/sidebar-workspace-menu.tsx | 15 +- .../sidebar/sidebar-workspace-row.tsx | 5 +- .../sidebar/use-open-kebab-menu-visibility.ts | 20 + .../e2e/adaptive-sheet-content-inset.spec.ts | 24 -- 21 files changed, 139 insertions(+), 611 deletions(-) rename packages/app/e2e/browser/{diff-row-alignment.spec.ts => changes-pane.spec.ts} (60%) create mode 100644 packages/app/src/components/sidebar/use-open-kebab-menu-visibility.ts diff --git a/docs/hover.md b/docs/hover.md index 9eaa1f90a..46f16af57 100644 --- a/docs/hover.md +++ b/docs/hover.md @@ -96,6 +96,21 @@ Any gap between A and B (margins between siblings inside the same parent) is par If A and B genuinely can't share a parent — B portals into a different layer, floats above other content — see [Section: real gaps](#real-gaps-with-floating-panels) below. +### Failure mode 4 — A hover-revealed trigger unmounts its own open menu + +A kebab that only exists while its row is hovered opens a menu, and that menu takes the pointer +off the row: a sheet slides up over it, a popover covers it. The row un-hovers, the trailing +overlay unmounts, and the trigger goes with it — taking the menu's open state, which lived +inside the trigger's subtree. The surface itself is already presented in a portal, so nothing +unmounts it and nothing is left that can close it. On a bottom sheet that means a full-screen +backdrop swallowing every click until reload. + +Lift the menu's open state to whatever decides to render the trigger, and keep the trigger +rendered while the menu is up. `useOpenKebabMenuVisibility` +(`packages/app/src/components/sidebar/use-open-kebab-menu-visibility.ts`) is that shape for the +sidebar rows: it owns `open`, hands the menu its controlled props, and ORs `open` into the +row's own `showKebab`. + ## Native fallback Hover doesn't exist on touch devices. Anything you hide behind hover must have a non-hover path on native and compact layouts: @@ -134,5 +149,6 @@ Before opening a PR that touches hover: - [ ] Hovered state does **not** change the trigger's outer geometry (`width`, `height`, `padding`, `borderWidth`, mount/unmount of siblings that shift it). Internal swaps fit inside a fixed `minHeight` / `minWidth`. - [ ] Revealed content inside the trigger uses `opacity` + `pointerEvents`, not conditional rendering, if mounting it would reflow the trigger. - [ ] Visibility on native and compact layouts works without hover (`isHovered || isNative || isCompact`). +- [ ] A menu opened from the revealed trigger keeps the trigger rendered while it is open, so losing hover can't strand it. - [ ] If the revealed content sits in a separate layer (portal, floating panel), `useHoverSafeZone` is wired up. - [ ] You opened the dev server, hovered the trigger, and slowly moved the mouse along **every** revealed element — including any visible gaps — without losing hover state. diff --git a/packages/app/e2e/browser/diff-row-alignment.spec.ts b/packages/app/e2e/browser/changes-pane.spec.ts similarity index 60% rename from packages/app/e2e/browser/diff-row-alignment.spec.ts rename to packages/app/e2e/browser/changes-pane.spec.ts index ac9d4dfe6..174cea357 100644 --- a/packages/app/e2e/browser/diff-row-alignment.spec.ts +++ b/packages/app/e2e/browser/changes-pane.spec.ts @@ -1,6 +1,6 @@ import { unlink, writeFile } from "node:fs/promises"; import path from "node:path"; -import { type Locator, type Page } from "@playwright/test"; +import { type Page } from "@playwright/test"; import { buildHostWorkspaceRoute, buildSettingsSectionRoute } from "../../src/utils/host-routes"; import { test, expect } from "../support/fixtures"; import { getServerId } from "../support/helpers/server-id"; @@ -25,103 +25,6 @@ const cleanupTasks: CleanupTask[] = []; const APP_SETTINGS_KEY = "@paseo:app-settings"; const CHANGES_PREFERENCES_KEY = "@paseo:changes-preferences"; -interface HorizontalInkBounds { - left: number; - right: number; -} - -async function readSvgInkBounds(svgLocator: Locator): Promise { - return svgLocator.evaluate((svg) => { - const graphics = Array.from(svg.querySelectorAll("path, line, polyline")); - const bounds = graphics.map((graphic) => { - const box = graphic.getBBox(); - const matrix = graphic.getScreenCTM(); - if (!matrix) { - throw new Error("SVG glyph has no screen transform"); - } - const strokeInset = Number.parseFloat(getComputedStyle(graphic).strokeWidth) / 2 || 0; - const corners = [ - new DOMPoint(box.x - strokeInset, box.y - strokeInset), - new DOMPoint(box.x + box.width + strokeInset, box.y - strokeInset), - new DOMPoint(box.x - strokeInset, box.y + box.height + strokeInset), - new DOMPoint(box.x + box.width + strokeInset, box.y + box.height + strokeInset), - ].map((point) => point.matrixTransform(matrix)); - return { - left: Math.min(...corners.map((point) => point.x)), - right: Math.max(...corners.map((point) => point.x)), - }; - }); - return { - left: Math.min(...bounds.map((bound) => bound.left)), - right: Math.max(...bounds.map((bound) => bound.right)), - }; - }); -} - -async function readTextInkBounds( - container: Locator, - edge: "first" | "last" = "first", -): Promise { - return container.evaluate((root, requestedEdge) => { - const textElements = [root, ...Array.from(root.querySelectorAll("*"))].filter((element) => - Array.from(element.childNodes).some( - (node) => node.nodeType === Node.TEXT_NODE && Boolean(node.textContent?.trim()), - ), - ); - const element = textElements[requestedEdge === "first" ? 0 : textElements.length - 1]; - const text = Array.from(element.childNodes) - .filter((node) => node.nodeType === Node.TEXT_NODE) - .map((node) => node.textContent ?? "") - .join("") - .trim(); - const style = getComputedStyle(element); - const canvas = document.createElement("canvas"); - const context = canvas.getContext("2d"); - if (!context || !text) { - throw new Error("Text glyph could not be measured"); - } - context.font = style.font; - const metrics = context.measureText(text); - const origin = element.getBoundingClientRect().left; - return { - left: origin - metrics.actualBoundingBoxLeft, - right: origin + metrics.actualBoundingBoxRight, - }; - }, edge); -} - -async function readScrollbarGutter(scrollContainer: Locator): Promise { - return scrollContainer.evaluate((element) => { - const htmlElement = element as HTMLElement; - return htmlElement.offsetWidth - htmlElement.clientWidth; - }); -} - -async function dragOverlayScrollbarDown(page: Page, scrollContainer: Locator): Promise { - const thumb = page.getByTestId("workspace-overlay-scrollbar-grab"); - const thumbBounds = await thumb.boundingBox(); - expect(thumbBounds).not.toBeNull(); - const initialOffset = await scrollContainer.evaluate((element) => element.scrollTop); - await page.mouse.move( - thumbBounds!.x + thumbBounds!.width / 2, - thumbBounds!.y + thumbBounds!.height / 2, - ); - await page.mouse.down(); - await page.mouse.move( - thumbBounds!.x + thumbBounds!.width / 2, - thumbBounds!.y + thumbBounds!.height / 2 + 40, - { steps: 4 }, - ); - await page.mouse.up(); - await expect - .poll(() => scrollContainer.evaluate((element) => element.scrollTop)) - .toBeGreaterThan(initialOffset); -} - -function expectSameRail(actual: number, expected: number): void { - expect(Math.abs(actual - expected)).toBeLessThanOrEqual(1); -} - const BEFORE = `import { useLayoutEffect, useMemo, useRef, useState } from "react"; interface UseMountedTabSetInput { @@ -298,28 +201,6 @@ test.afterEach(async () => { } }); -test("changes diff keeps code rows aligned with the gutter", async ({ page }) => { - const workspace = await createWorkspaceWithMountedTabDiff(); - await useCodeFont(page, 9); - await useUnwrappedDiffLines(page); - await openWorkspaceChanges(page, workspace); - - await expectDiffCodeFontSize(page, 9); - await expectVisibleDiffRowsAligned(page); - await expectDiffCodeTextAlignedWithGutterText(page, [ - { - codeText: "function createInitialMountedTabIds(input: UseMountedTabSetInput)", - lineNumber: "20", - }, - { codeText: "return next;", lineNumber: "55" }, - { codeText: "useLayoutEffect(() => {", lineNumber: "78" }, - ]); - await expectHoverCommentButtonAlignedWithCodeLine(page, { - codeText: "function createInitialMountedTabIds(input: UseMountedTabSetInput)", - lineNumber: "20", - }); -}); - test("changes file actions open below the right-click without a reserved kebab", async ({ page, }) => { @@ -463,138 +344,7 @@ test("changes diff switches between flat and tree file lists", async ({ page }) await expectFlatFileList(page); }); -test("workspace file panes keep their controls on shared alignment rails", async ({ page }) => { - const workspace = await createWorkspaceWithMountedTabDiff(); - await openWorkspaceChanges(page, workspace); - - const diffScroll = page.getByTestId("git-diff-scroll"); - await diffScroll.evaluate((element) => { - element.style.scrollbarGutter = "stable"; - }); - expect(await readScrollbarGutter(diffScroll)).toBe(0); - const overlayScrollbarBounds = await page - .getByTestId("workspace-overlay-scrollbar") - .boundingBox(); - const overlayThumbBounds = await page - .getByTestId("workspace-overlay-scrollbar-thumb") - .boundingBox(); - expect(overlayScrollbarBounds?.width).toBe(8); - expect(overlayThumbBounds?.width).toBe(4); - const flatFileStat = await readTextInkBounds(page.getByTestId("diff-file-0-stat"), "last"); - await page.getByTestId("changes-toggle-view-mode").click(); - await expect(page.getByTestId("diff-folder-src")).toBeVisible(); - - await expect(page.getByTestId("diff-file-0-actions")).toHaveCount(0); - - const folderRow = page.getByTestId("diff-folder-src-toggle"); - const fileRow = page.getByTestId("diff-file-0-toggle"); - await folderRow.hover(); - const folderHoverColor = await folderRow.evaluate((row) => getComputedStyle(row).backgroundColor); - await fileRow.hover(); - const fileHoverColor = await fileRow.evaluate((row) => getComputedStyle(row).backgroundColor); - expect(fileHoverColor).toBe(folderHoverColor); - - const diffFolderName = folderRow.getByText("src", { exact: true }); - const diffFileName = fileRow.getByText("use-mounted-tab-set.ts", { exact: true }); - const diffFolderIconLocator = folderRow.locator("svg").first(); - const diffFileIconLocator = fileRow.locator("svg").first(); - const [diffFolderIcon, diffFileIcon, diffFolderNameBounds, diffFileNameBounds] = - await Promise.all([ - readSvgInkBounds(diffFolderIconLocator), - readSvgInkBounds(diffFileIconLocator), - diffFolderName.boundingBox(), - diffFileName.boundingBox(), - ]); - expect(diffFolderNameBounds).not.toBeNull(); - expect(diffFileNameBounds).not.toBeNull(); - const diffTreeIconNameGap = diffFolderNameBounds!.x - diffFolderIcon.right; - expectSameRail(diffTreeIconNameGap, diffFileNameBounds!.x - diffFileIcon.right); - - const [folderStat, fileStat, optionsChevron, explorerCloseIcon] = await Promise.all([ - readTextInkBounds(page.getByTestId("diff-folder-src-stat"), "last"), - readTextInkBounds(page.getByTestId("diff-file-0-stat"), "last"), - readSvgInkBounds(page.getByTestId("changes-options-menu").locator("svg")), - readSvgInkBounds(page.getByTestId("explorer-close").locator("svg")), - ]); - expectSameRail(folderStat.right, fileStat.right); - expectSameRail(flatFileStat.right, fileStat.right); - expectSameRail(fileStat.right, explorerCloseIcon.right); - expectSameRail(fileStat.right, optionsChevron.right); - expectSameRail(optionsChevron.right, explorerCloseIcon.right); - await folderRow.click(); - await folderRow.click(); - await dragOverlayScrollbarDown(page, diffScroll); - - await page.getByTestId("explorer-tab-files").click(); - await expect(page.getByTestId("file-explorer-row-0")).toBeVisible(); - const filesScroll = page.getByTestId("file-explorer-tree-scroll"); - await filesScroll.evaluate((element) => { - element.style.scrollbarGutter = "stable"; - }); - expect(await readScrollbarGutter(filesScroll)).toBe(0); - - await expect(page.getByTestId("file-explorer-row-0-actions")).toHaveCount(0); - const fileExplorerRow = page.getByTestId("file-explorer-row-0"); - const fileExplorerRowBounds = await fileExplorerRow.boundingBox(); - expect(fileExplorerRowBounds).not.toBeNull(); - await fileExplorerRow.click({ button: "right", position: { x: 80, y: 10 } }); - await expect(page.getByTestId("file-explorer-row-0-context-menu")).toBeVisible(); - const fileMenuBounds = await page.getByTestId("file-explorer-row-0-context-menu").boundingBox(); - expect(fileMenuBounds).not.toBeNull(); - expect(fileMenuBounds!.x).toBeCloseTo(fileExplorerRowBounds!.x + 80, 0); - expect(fileMenuBounds!.y).toBeGreaterThan(fileExplorerRowBounds!.y + 10); - await page.keyboard.press("Escape"); - - const directoryRow = page.getByTestId(/^file-explorer-row-\d+$/).filter({ hasText: "src" }); - const directoryName = directoryRow.getByText("src", { exact: true }); - const [collapsedDirectoryIcon, collapsedDirectoryNameBounds] = await Promise.all([ - readSvgInkBounds(directoryRow.locator("svg").first()), - directoryName.boundingBox(), - ]); - expect(collapsedDirectoryNameBounds).not.toBeNull(); - - await directoryRow.click(); - const nestedFileRow = page - .getByTestId(/^file-explorer-row-\d+$/) - .filter({ hasText: "use-mounted-tab-set.ts" }); - await expect(nestedFileRow).toBeVisible(); - const [expandedDirectoryIcon, expandedDirectoryNameBounds, fileIcon, fileNameBounds] = - await Promise.all([ - readSvgInkBounds(directoryRow.locator("svg").first()), - directoryName.boundingBox(), - readSvgInkBounds(nestedFileRow.locator("svg").first()), - nestedFileRow.getByText("use-mounted-tab-set.ts", { exact: true }).boundingBox(), - ]); - expect(expandedDirectoryNameBounds).not.toBeNull(); - expect(fileNameBounds).not.toBeNull(); - expect(expandedDirectoryNameBounds!.x).toBeCloseTo(collapsedDirectoryNameBounds!.x, 1); - expectSameRail( - collapsedDirectoryNameBounds!.x - collapsedDirectoryIcon.right, - fileNameBounds!.x - fileIcon.right, - ); - expectSameRail( - expandedDirectoryNameBounds!.x - expandedDirectoryIcon.right, - fileNameBounds!.x - fileIcon.right, - ); - expectSameRail(diffTreeIconNameGap, fileNameBounds!.x - fileIcon.right); - - const readmeRow = page.getByTestId(/^file-explorer-row-\d+$/).filter({ hasText: "README.md" }); - const [sortLabel, fileRowIcon, treeBounds, rowBounds] = await Promise.all([ - readTextInkBounds(page.getByTestId("files-sort-label")), - readSvgInkBounds(readmeRow.locator("svg").first()), - page.getByTestId("file-explorer-tree-scroll").boundingBox(), - page.getByTestId("file-explorer-row-0").boundingBox(), - ]); - expect(treeBounds).not.toBeNull(); - expect(rowBounds).not.toBeNull(); - expectSameRail(fileRowIcon.left, sortLabel.left); - expect(rowBounds!.x).toBeCloseTo(treeBounds!.x, 0); - expect(rowBounds!.x + rowBounds!.width).toBeCloseTo(treeBounds!.x + treeBounds!.width, 0); -}); - -test("changes diff keeps unwrapped gutter and code rows aligned after code size changes", async ({ - page, -}) => { +test("changes diff applies code size changes to gutter and code typography", async ({ page }) => { const workspace = await createWorkspaceWithMountedTabDiff(); await useCodeFont(page, 12); await useUnwrappedDiffLines(page); @@ -607,7 +357,6 @@ test("changes diff keeps unwrapped gutter and code rows aligned after code size await expectDiffCodeFontSize(page, 18); await expectVisibleDiffRowsShareTypography(page); - await expectVisibleDiffRowsAligned(page); }); async function useCodeFont(page: Page, codeFontSize: number): Promise { @@ -668,24 +417,17 @@ async function expectDiffCodeFontSize(page: Page, fontSize: number): Promise { - const geometry = await readVisibleDiffRowGeometry(page); - expect(geometry.maxDelta, JSON.stringify(geometry.rows, null, 2)).toBeLessThanOrEqual(1); -} - async function expectVisibleDiffRowsShareTypography(page: Page): Promise { const geometry = await readVisibleDiffRowGeometry(page); expect(geometry.mismatchedTypography, JSON.stringify(geometry, null, 2)).toEqual([]); } async function readVisibleDiffRowGeometry(page: Page): Promise<{ - maxDelta: number; mismatchedTypography: { index: number; gutterLineHeight: number; codeLineHeight: number }[]; rows: { index: number; gutterTop: number; codeTop: number; - delta: number; gutterLineHeight: number; codeLineHeight: number; }[]; @@ -720,7 +462,6 @@ async function readVisibleDiffRowGeometry(page: Page): Promise<{ index: code.index, gutterTop: gutter.top, codeTop: code.top, - delta: Math.abs(code.top - gutter.top), gutterLineHeight: gutter.lineHeight, codeLineHeight: code.lineHeight, }; @@ -728,7 +469,6 @@ async function readVisibleDiffRowGeometry(page: Page): Promise<{ .filter((row) => row.gutterTop >= 0 && row.codeTop >= 0); return { - maxDelta: Math.max(...rows.map((row) => row.delta)), mismatchedTypography: rows .filter((row) => Math.abs(row.gutterLineHeight - row.codeLineHeight) > 0.5) .map((row) => ({ @@ -748,7 +488,7 @@ async function createWorkspaceWithMountedTabDiff( if (options.includeDeletedFile) { files.push({ path: "src/zz-deleted.ts", content: "export const deleted = true;\n" }); } - const repo = await createTempGitRepo("diff-row-alignment-", { files }); + const repo = await createTempGitRepo("changes-pane-", { files }); const client = await connectSeedClient(); cleanupTasks.push({ run: async () => { @@ -837,86 +577,3 @@ async function scrollToLowerUnwrappedDiffRows(page: Page): Promise { await page.getByTestId(`diff-code-row-${lastRowIndex}`).scrollIntoViewIfNeeded(); await expect(page.getByTestId(`diff-code-row-${lastRowIndex}`)).toBeVisible(); } - -async function expectDiffCodeTextAlignedWithGutterText( - page: Page, - lines: { codeText: string; lineNumber: string }[], -): Promise { - const geometries = await readDiffTextGeometry(page, lines); - for (const geometry of geometries) { - expect(geometry.codeTop, geometry.codeText).toBeCloseTo(geometry.gutterTop, 0); - } -} - -async function expectHoverCommentButtonAlignedWithCodeLine( - page: Page, - line: { codeText: string; lineNumber: string }, -): Promise { - const target = await readDiffTextGeometry(page, [line]).then((rows) => rows[0]); - if (!target) { - throw new Error(`Could not find target line ${line.lineNumber}`); - } - await page.getByTestId(`diff-code-row-${target.index}`).hover(); - const geometry = await page - .getByTestId(`diff-gutter-action-${target.index}`) - .evaluate((action, expectedCodeCenterY) => { - const rect = action.getBoundingClientRect(); - return { - actionCenterY: rect.top + rect.height / 2, - codeCenterY: expectedCodeCenterY, - }; - }, target.codeCenterY); - expect(geometry.actionCenterY).toBeCloseTo(geometry.codeCenterY, 0); -} - -async function readDiffTextGeometry( - page: Page, - lines: { codeText: string; lineNumber: string }[], -): Promise< - { index: number; codeText: string; codeTop: number; gutterTop: number; codeCenterY: number }[] -> { - return page.locator("body").evaluate(({ ownerDocument }, targets) => { - const root = ownerDocument.querySelector('[data-testid="explorer-content-area"]'); - if (!root) { - throw new Error("Changes panel is not mounted"); - } - - const readIndexedElements = (prefix: string) => - Array.from(root.querySelectorAll(`[data-testid^="${prefix}"]`)).map( - (element) => { - const testId = element.getAttribute("data-testid") ?? ""; - return { index: Number(testId.slice(prefix.length)), element }; - }, - ); - - const gutterTexts = readIndexedElements("diff-gutter-text-"); - const codeTexts = readIndexedElements("diff-code-text-"); - - return targets.map((target) => { - const gutter = gutterTexts.find( - ({ element }) => (element.textContent ?? "").trim() === target.lineNumber, - ); - if (!gutter) { - throw new Error(`Could not find gutter line ${target.lineNumber}`); - } - const code = codeTexts.find( - ({ index, element }) => - index === gutter.index && (element.textContent ?? "").includes(target.codeText), - ); - if (!code) { - throw new Error(`Could not find code row ${target.codeText}`); - } - - const codeRect = code.element.getBoundingClientRect(); - const gutterRect = gutter.element.getBoundingClientRect(); - - return { - index: gutter.index, - codeText: target.codeText, - codeTop: codeRect.top, - gutterTop: gutterRect.top, - codeCenterY: codeRect.top + codeRect.height / 2, - }; - }); - }, lines); -} diff --git a/packages/app/e2e/browser/chat-outline.spec.ts b/packages/app/e2e/browser/chat-outline.spec.ts index 55e299782..71ffbdbaf 100644 --- a/packages/app/e2e/browser/chat-outline.spec.ts +++ b/packages/app/e2e/browser/chat-outline.spec.ts @@ -6,7 +6,6 @@ import { expectActiveChatOutlinePrompt, expectChatOutlinePreview, expectChatOutlinePrompts, - expectChatOutlineAlignedWithActiveTabGlyph, expectChatOutlinePromptToRemainBare, expectLiveTurnPromptAboveFoldAndActive, expectNoChatOutlinePreviewWhileCrossingToSidebar, @@ -140,10 +139,6 @@ test.describe("desktop chat outline", () => { await expectChatOutlinePromptToRemainBare(page, 4); }); - test("aligns the ticks with the active tab glyph rail", async ({ page }) => { - await expectChatOutlineAlignedWithActiveTabGlyph(page); - }); - test("clamps the newest prompt at maximum scroll while keeping it visible", async ({ page, }) => { diff --git a/packages/app/e2e/browser/new-workspace.spec.ts b/packages/app/e2e/browser/new-workspace.spec.ts index 49062e482..7206c9f52 100644 --- a/packages/app/e2e/browser/new-workspace.spec.ts +++ b/packages/app/e2e/browser/new-workspace.spec.ts @@ -51,6 +51,7 @@ import { type LocalGhPrFixture, } from "../support/helpers/github-fixtures"; import { getServerId } from "../support/helpers/server-id"; +import { selectSidebarStatusGrouping } from "../support/helpers/sidebar"; import { getE2EDaemonPort } from "../support/helpers/daemon-port"; import { chooseAddProjectMethod, expectAddProjectPage } from "../support/helpers/add-project-flow"; import { seedSavedSettingsHosts } from "../support/helpers/settings"; @@ -78,8 +79,7 @@ interface WorkspaceStatusGroupEvent { } async function switchSidebarToStatusGrouping(page: import("@playwright/test").Page) { - await page.getByTestId("sidebar-display-preferences-menu").click(); - await page.getByTestId("sidebar-grouping-status").click(); + await selectSidebarStatusGrouping(page); await expect(page.getByTestId("sidebar-status-group-done")).toBeVisible({ timeout: 30_000 }); } diff --git a/packages/app/e2e/browser/sidebar-host-filter-multi.spec.ts b/packages/app/e2e/browser/sidebar-host-filter-multi.spec.ts index 30d4a4422..7c4781210 100644 --- a/packages/app/e2e/browser/sidebar-host-filter-multi.spec.ts +++ b/packages/app/e2e/browser/sidebar-host-filter-multi.spec.ts @@ -4,7 +4,7 @@ import { gotoAppShell } from "../support/helpers/app"; import { addOfflineHostAndReload, expectHostFilterRow, - openSidebarDisplayPreferences, + openSidebarHostFilter, selectAllHostsFilter, toggleHostFilter, } from "../support/helpers/hosts"; @@ -29,7 +29,7 @@ test.describe("Sidebar host filter (multi-select)", () => { await addOfflineHostAndReload(page, { serverId: SECONDARY_HOST_ID, label: "Secondary Host" }); await expect(workspaceRow).toBeVisible({ timeout: 30_000 }); - await openSidebarDisplayPreferences(page); + await openSidebarHostFilter(page); await expectHostFilterRow(page, serverId); await expectHostFilterRow(page, SECONDARY_HOST_ID); diff --git a/packages/app/e2e/browser/sidebar-mobile-menu-sheets.spec.ts b/packages/app/e2e/browser/sidebar-mobile-menu-sheets.spec.ts index 6cd35902e..cf2f44891 100644 --- a/packages/app/e2e/browser/sidebar-mobile-menu-sheets.spec.ts +++ b/packages/app/e2e/browser/sidebar-mobile-menu-sheets.spec.ts @@ -1,4 +1,3 @@ -import type { Locator } from "@playwright/test"; import { expect, test, type Page } from "../support/fixtures"; import { gotoAppShell } from "../support/helpers/app"; import { projectEquivalenceViewKey } from "../support/helpers/project-view-key"; @@ -8,29 +7,6 @@ import { waitForSidebarHydration } from "../support/helpers/workspace-ui"; test.use({ viewport: { width: 390, height: 844 }, isMobile: true, hasTouch: true }); -async function paintedSvgRight(locator: Locator): Promise { - return locator.evaluateAll((svgs) => { - let right = Number.NEGATIVE_INFINITY; - for (const element of svgs) { - const svg = element as SVGSVGElement; - const svgRect = svg.getBoundingClientRect(); - const viewBox = svg.viewBox.baseVal; - if (svgRect.right <= 0 || svgRect.left >= window.innerWidth || viewBox.width === 0) continue; - const scaleX = svgRect.width / viewBox.width; - for (const child of svg.querySelectorAll( - "path, circle, ellipse, line, polyline, polygon, rect", - )) { - const box = child.getBBox(); - const strokeWidth = Number.parseFloat(getComputedStyle(child).strokeWidth) || 0; - const localRight = box.x + box.width + strokeWidth / 2; - right = Math.max(right, svgRect.left + (localRight - viewBox.x) * scaleX); - } - } - if (!Number.isFinite(right)) throw new Error("SVG has no measurable painted children"); - return right; - }); -} - async function closeMenuSheet(page: Page): Promise { const backdrop = page.getByRole("button", { name: "Bottom sheet backdrop" }).first(); await backdrop.click({ position: { x: 12, y: 12 } }); @@ -45,12 +21,6 @@ test("project and workspace kebabs open action sheets on compact layouts", async await page.getByRole("button", { name: "Open menu", exact: true }).click(); await waitForSidebarHydration(page); - const closeGlyphRight = await paintedSvgRight(page.getByTestId("sidebar-close").locator("svg")); - const trailingRail = await paintedSvgRight( - page.getByTestId("sidebar-display-preferences-menu").locator("svg"), - ); - expect(closeGlyphRight).toBeCloseTo(trailingRail, 0); - await page .getByTestId(`sidebar-project-kebab-${projectEquivalenceViewKey(seeded.projectKey)}`) .click(); diff --git a/packages/app/e2e/browser/sidebar-model-b.spec.ts b/packages/app/e2e/browser/sidebar-model-b.spec.ts index ed724c686..33e6ff8dd 100644 --- a/packages/app/e2e/browser/sidebar-model-b.spec.ts +++ b/packages/app/e2e/browser/sidebar-model-b.spec.ts @@ -1,4 +1,3 @@ -import type { Locator } from "@playwright/test"; import { test, expect, type Page } from "../support/fixtures"; import { gotoAppShell } from "../support/helpers/app"; import { gotoWorkspace, clickNewTerminal } from "../support/helpers/launcher"; @@ -6,6 +5,7 @@ import { seedWorkspace, type SeededWorkspace } from "../support/helpers/seed-cli import { seedMockAgentWorkspace } from "../support/helpers/mock-agent"; import { getServerId } from "../support/helpers/server-id"; import { projectEquivalenceViewKey } from "../support/helpers/project-view-key"; +import { selectSidebarStatusGrouping } from "../support/helpers/sidebar"; import { waitForSidebarHydration } from "../support/helpers/workspace-ui"; import { getVisibleWorkspaceAgentTabIds } from "../support/helpers/workspace-tabs"; @@ -26,35 +26,6 @@ function projectNewWorktreeIcon(page: Page, projectKey: string) { return page.getByTestId(`sidebar-project-new-worktree-${projectEquivalenceViewKey(projectKey)}`); } -async function paintedSvgRight(locator: Locator): Promise { - return locator.evaluate((svg) => { - let right = Number.NEGATIVE_INFINITY; - for (const child of svg.querySelectorAll( - "path, circle, ellipse, line, polyline, polygon, rect", - )) { - const box = child.getBBox(); - const matrix = child.getScreenCTM(); - if (!matrix) continue; - const geometryRight = new DOMPoint(box.x + box.width, box.y).matrixTransform(matrix).x; - const strokeWidth = Number.parseFloat(getComputedStyle(child).strokeWidth) || 0; - const scaleX = Math.hypot(matrix.a, matrix.b); - right = Math.max(right, geometryRight + (strokeWidth * scaleX) / 2); - } - if (!Number.isFinite(right)) throw new Error("SVG has no measurable painted children"); - return right; - }); -} - -async function paintedTextRight(locator: Locator): Promise { - return locator.evaluate((label) => { - const context = document.createElement("canvas").getContext("2d"); - if (!context) throw new Error("Canvas text measurement is unavailable"); - context.font = getComputedStyle(label).font; - const metrics = context.measureText(label.textContent ?? ""); - return label.getBoundingClientRect().left + metrics.actualBoundingBoxRight; - }); -} - async function seedSecondWorkspace(seeded: SeededWorkspace, title: string): Promise { const created = await seeded.client.createWorkspace({ source: { kind: "directory", path: seeded.repoPath, projectId: seeded.projectId }, @@ -116,53 +87,6 @@ test.describe("Model B sidebar shape", () => { } }); - test("sidebar trailing glyphs share the workspace content rail", async ({ page }) => { - const seeded = await seedWorkspace({ repoPrefix: "model-b-trailing-rail-" }); - - try { - await gotoAppShell(page); - await waitForSidebarHydration(page); - - await page.getByTestId("sidebar-display-preferences-menu").click(); - await page.getByTestId("sidebar-display-show").click(); - await page.getByTestId("sidebar-workspace-trailing-timestamp").click(); - await page.keyboard.press("Escape"); - await page.keyboard.press("Escape"); - - const project = projectRow(page, seeded.projectKey); - const workspace = workspaceRow(page, seeded.workspaceId); - const displayPreferencesGlyph = page - .getByTestId("sidebar-display-preferences-menu") - .locator("svg"); - - await project.hover(); - const projectKebabGlyph = page - .getByTestId(`sidebar-project-kebab-${projectEquivalenceViewKey(seeded.projectKey)}`) - .locator("svg"); - await expect(projectKebabGlyph).toBeVisible(); - const projectKebabRight = await paintedSvgRight(projectKebabGlyph); - - await workspace.hover(); - const workspaceKebabGlyph = page - .getByTestId(`sidebar-workspace-kebab-${getServerId()}:${seeded.workspaceId}`) - .locator("svg"); - const timestamp = workspace.getByTestId("sidebar-workspace-timestamp"); - await expect(workspaceKebabGlyph).toBeVisible(); - await expect(timestamp).toBeVisible(); - - const rail = await paintedTextRight(timestamp); - for (const glyphRight of await Promise.all([ - paintedSvgRight(displayPreferencesGlyph), - Promise.resolve(projectKebabRight), - paintedSvgRight(workspaceKebabGlyph), - ])) { - expect(glyphRight).toBeCloseTo(rail, 0); - } - } finally { - await seeded.cleanup(); - } - }); - test("no tab, agent, or terminal ever renders as a sidebar row", async ({ page }) => { const mock = await seedMockAgentWorkspace({ repoPrefix: "model-b-leaf-", @@ -209,8 +133,7 @@ test.describe("Model B sidebar shape", () => { await expect(workspaceRow(page, idleProject.workspaceId)).toBeVisible({ timeout: 30_000 }); // Switch to status grouping. - await page.getByTestId("sidebar-display-preferences-menu").click(); - await page.getByTestId("sidebar-grouping-status").click(); + await selectSidebarStatusGrouping(page); const sidebar = page.getByTestId("sidebar-sessions").filter({ visible: true }).first(); diff --git a/packages/app/e2e/browser/sidebar-workspace-pin-shortcut.spec.ts b/packages/app/e2e/browser/sidebar-workspace-pin-shortcut.spec.ts index 5f5e710a3..4feca297e 100644 --- a/packages/app/e2e/browser/sidebar-workspace-pin-shortcut.spec.ts +++ b/packages/app/e2e/browser/sidebar-workspace-pin-shortcut.spec.ts @@ -3,6 +3,7 @@ import { gotoAppShell } from "../support/helpers/app"; import { daemonWsRoutePattern } from "../support/helpers/daemon-port"; import { seedWorkspace, type SeededWorkspace } from "../support/helpers/seed-client"; import { getServerId } from "../support/helpers/server-id"; +import { selectSidebarStatusGrouping } from "../support/helpers/sidebar"; // The pin shortcut used to be registered by the sidebar row itself, so it silently did nothing // whenever the row was unmounted — a collapsed project section being the common case. It now @@ -39,8 +40,7 @@ async function collapseProjectSection(page: Page, project: SeededWorkspace): Pro } async function switchToStatusGrouping(page: Page): Promise { - await page.getByTestId("sidebar-display-preferences-menu").click(); - await page.getByTestId("sidebar-grouping-status").click(); + await selectSidebarStatusGrouping(page); await expect(page.getByTestId("sidebar-status-list-scroll")).toBeVisible({ timeout: 10_000 }); } diff --git a/packages/app/e2e/browser/workspace-model-regressions.spec.ts b/packages/app/e2e/browser/workspace-model-regressions.spec.ts index 2866e83ab..04cc8bd7d 100644 --- a/packages/app/e2e/browser/workspace-model-regressions.spec.ts +++ b/packages/app/e2e/browser/workspace-model-regressions.spec.ts @@ -16,6 +16,7 @@ import { submitNewWorkspacePrompt, } from "../support/helpers/new-workspace"; import { getServerId } from "../support/helpers/server-id"; +import { selectSidebarStatusGrouping } from "../support/helpers/sidebar"; import { seedWorkspace, type SeededWorkspace } from "../support/helpers/seed-client"; import { expectSubagentRowVisible, @@ -82,8 +83,7 @@ async function fetchAgentStatus(seeded: SeededWorkspace, agentId: string): Promi } async function switchSidebarToStatusGrouping(page: import("@playwright/test").Page) { - await page.getByTestId("sidebar-display-preferences-menu").click(); - await page.getByTestId("sidebar-grouping-status").click(); + await selectSidebarStatusGrouping(page); await expect(page.locator('[data-testid^="sidebar-status-group-"]').first()).toBeVisible({ timeout: 30_000, }); diff --git a/packages/app/e2e/browser/workspace-model-restart.spec.ts b/packages/app/e2e/browser/workspace-model-restart.spec.ts index cecf4f991..14f7cb288 100644 --- a/packages/app/e2e/browser/workspace-model-restart.spec.ts +++ b/packages/app/e2e/browser/workspace-model-restart.spec.ts @@ -21,6 +21,7 @@ import { selectNewWorkspaceProject, submitNewWorkspaceEmpty, } from "../support/helpers/new-workspace"; +import { selectSidebarStatusGrouping } from "../support/helpers/sidebar"; import { waitForSidebarHydration } from "../support/helpers/workspace-ui"; import { getVisibleWorkspaceAgentTabIds } from "../support/helpers/workspace-tabs"; @@ -397,8 +398,7 @@ async function expectWorkspaceRowInStatusBucket( page: Page, input: { serverId: string; workspaceId: string; bucket: string }, ) { - await page.getByTestId("sidebar-display-preferences-menu").click(); - await page.getByTestId("sidebar-grouping-status").click(); + await selectSidebarStatusGrouping(page); await expect( page .getByTestId(`sidebar-status-group-rows-${input.bucket}`) diff --git a/packages/app/e2e/support/helpers/agent-bottom-anchor.ts b/packages/app/e2e/support/helpers/agent-bottom-anchor.ts index ed8951b94..d8c036d63 100644 --- a/packages/app/e2e/support/helpers/agent-bottom-anchor.ts +++ b/packages/app/e2e/support/helpers/agent-bottom-anchor.ts @@ -102,6 +102,8 @@ export async function waitForScrollableChat( .toBeGreaterThan(input.minScrollableDistance); } +const WHEEL_SETTLE_TIMEOUT_MS = 5_000; + export async function scrollChatAwayFromBottom( page: Page, input: { deltaY: number; minDistanceFromBottom: number }, @@ -111,31 +113,41 @@ export async function scrollChatAwayFromBottom( if (!box) { throw new Error("Agent chat scroll container is not visible"); } - const wheelSettled = scroll.evaluate( - (root: Element) => - new Promise((resolve) => { - root.addEventListener( - "wheel", - () => { - const scrollElement = root as HTMLElement; - let previousOffset = scrollElement.scrollTop; - let stableFrames = 0; - const sample = () => { - const currentOffset = scrollElement.scrollTop; - stableFrames = Math.abs(currentOffset - previousOffset) <= 0.5 ? stableFrames + 1 : 0; - previousOffset = currentOffset; - if (stableFrames >= 3) { - resolve(); - return; - } - requestAnimationFrame(sample); - }; - requestAnimationFrame(sample); - }, - { once: true }, - ); - }), - ); + // Letting the scroll animation stop before measuring is an optimization, so it must never be + // the thing that decides the test — the assertion below is. The old wait could not say that: + // it resolved only from a `requestAnimationFrame` sampler started by a `wheel` listener, and + // rAF does not fire at all while the page is occluded, which is reachable on a CI runner and + // stalls the sampler on its first frame. A missed wheel event did the same. Either way the + // test hung until its own timeout with nothing to read. A timed sampler with a deadline + // cannot: worst case it gives up and the poll below reports what the scroll actually did. + const wheelSettled = scroll.evaluate((root: Element, timeoutMs: number) => { + const scrollElement = root as HTMLElement; + return new Promise((resolve) => { + const startedAt = Date.now(); + let sawWheel = false; + let previousOffset = scrollElement.scrollTop; + let stableSamples = 0; + root.addEventListener( + "wheel", + () => { + sawWheel = true; + }, + { once: true }, + ); + const sample = () => { + const currentOffset = scrollElement.scrollTop; + const isStable = sawWheel && Math.abs(currentOffset - previousOffset) <= 0.5; + stableSamples = isStable ? stableSamples + 1 : 0; + previousOffset = currentOffset; + if (stableSamples >= 3 || Date.now() - startedAt >= timeoutMs) { + resolve(); + return; + } + setTimeout(sample, 16); + }; + sample(); + }); + }, WHEEL_SETTLE_TIMEOUT_MS); await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2); await page.mouse.wheel(0, input.deltaY); await wheelSettled; diff --git a/packages/app/e2e/support/helpers/chat-outline.ts b/packages/app/e2e/support/helpers/chat-outline.ts index a70189654..7f903a79a 100644 --- a/packages/app/e2e/support/helpers/chat-outline.ts +++ b/packages/app/e2e/support/helpers/chat-outline.ts @@ -125,26 +125,6 @@ export async function expectChatOutlinePromptToRemainBare( await expect(chatOutlinePrompt(page, position)).toHaveCSS("background-color", "rgba(0, 0, 0, 0)"); } -export async function expectChatOutlineAlignedWithActiveTabGlyph(page: Page): Promise { - const outlinePill = chatOutlinePrompt(page, 1).locator("div").first(); - const activeTabGlyph = page - .getByTestId("workspace-tabs-row") - .filter({ visible: true }) - .locator('[role="button"][aria-selected="true"]') - .locator("svg") - .first(); - - await expect - .poll(async () => { - const [pillBox, glyphBox] = await Promise.all([ - requireBoundingBox(outlinePill), - requireBoundingBox(activeTabGlyph), - ]); - return Math.abs(pillBox.x - glyphBox.x); - }) - .toBeLessThanOrEqual(1); -} - /** Exactly one prompt is marked, without saying which — the reader always has a "you are here". */ export async function expectOneActiveChatOutlinePrompt(page: Page): Promise { await expect(chatOutlineRail(page).getByRole("tab", { selected: true })).toHaveCount(1); diff --git a/packages/app/e2e/support/helpers/hosts.ts b/packages/app/e2e/support/helpers/hosts.ts index 8536eef5b..ca574652c 100644 --- a/packages/app/e2e/support/helpers/hosts.ts +++ b/packages/app/e2e/support/helpers/hosts.ts @@ -130,6 +130,13 @@ export async function openSidebarDisplayPreferences(page: Page): Promise { }); } +// Host filters live a page below the display-preferences root, so the filter rows only exist +// after walking into that branch. +export async function openSidebarHostFilter(page: Page): Promise { + await openSidebarDisplayPreferences(page); + await page.getByTestId("sidebar-display-host-filter").click(); +} + // A host's filter row carries a status dot on the left next to its label. export async function expectHostFilterRow(page: Page, serverId: string): Promise { await expect(page.getByTestId(`sidebar-host-filter-${serverId}`)).toBeVisible(); @@ -144,32 +151,6 @@ export async function selectAllHostsFilter(page: Page): Promise { await page.getByTestId("sidebar-host-filter-all").click(); } -// Playwright reports resolved colors, so the expected value is derived from the same identity -// table the app reads. A hex literal in the test would be a second copy of the palette. -function toRgb(hex: string): string { - const { r, g, b } = parseHex(hex); - return `rgb(${r}, ${g}, ${b})`; -} - -function toRgba(hex: string, alpha: number): string { - const { r, g, b } = parseHex(hex); - return `rgba(${r}, ${g}, ${b}, ${alpha})`; -} - -interface RgbChannels { - r: number; - g: number; - b: number; -} - -function parseHex(hex: string): RgbChannels { - return { - r: Number.parseInt(hex.slice(1, 3), 16), - g: Number.parseInt(hex.slice(3, 5), 16), - b: Number.parseInt(hex.slice(5, 7), 16), - }; -} - export async function openHostAppearanceSettings(page: Page, serverId: string): Promise { await openSettings(page); await selectSettingsHost(page, serverId); @@ -242,17 +223,16 @@ export async function expectNoHostBadge(page: Page, target: HostBadgeTarget): Pr await expect(hostBadge(page, target)).toHaveCount(0, { timeout: 15_000 }); } +// The host sits on the meta line as plain secondary text, so its identity colour lives on the +// server icon alone. The label stays muted like every other item on that line — asserting a +// tinted label or a pill fill would re-assert the design the meta row replaced. export async function expectHostBadgeTinted( page: Page, target: HostBadgeTarget & { color: IdentityColorName }, ): Promise { const badge = hostBadge(page, target); await expect(badge).toBeVisible({ timeout: 15_000 }); - await expect(badge.getByText(target.hostName)).toHaveCSS( - "color", - toRgb(identityColor(target.color)), - ); - await expect(badge).toHaveCSS("background-color", toRgba(identityColor(target.color), 0.1)); + await expect(badge).toHaveText(target.hostName); await expect(badge.locator("svg")).toHaveAttribute("stroke", identityColor(target.color)); } @@ -264,11 +244,6 @@ export async function expectHostAppearancePreview( await expect(preview).toBeVisible(); const badge = preview.getByTestId(`sidebar-host-badge-${input.serverId}`); await expect(badge).toHaveText(input.hostName); - await expect(badge.getByText(input.hostName)).toHaveCSS( - "color", - toRgb(identityColor(input.color)), - ); - await expect(badge).toHaveCSS("background-color", toRgba(identityColor(input.color), 0.1)); await expect(badge.locator("svg")).toHaveAttribute("stroke", identityColor(input.color)); } diff --git a/packages/app/e2e/support/helpers/sheet-layout.ts b/packages/app/e2e/support/helpers/sheet-layout.ts index 08e5bb25c..86b680acc 100644 --- a/packages/app/e2e/support/helpers/sheet-layout.ts +++ b/packages/app/e2e/support/helpers/sheet-layout.ts @@ -1,9 +1,4 @@ -import { expect, type Locator, type Page } from "@playwright/test"; - -export interface Rails { - left: number; - right: number; -} +import { expect, type Locator } from "@playwright/test"; interface Box { x: number; @@ -18,30 +13,6 @@ async function readBox(locator: Locator): Promise { return box as Box; } -/** Leading and trailing edges of an element, in whole pixels. */ -export async function readRails(locator: Locator): Promise { - const box = await readBox(locator); - return { left: Math.round(box.x), right: Math.round(box.x + box.width) }; -} - -/** - * The two rails an AdaptiveModalSheet header indents to: the title's leading - * glyph and the close button's trailing edge. Sheet content sits on the same - * two rails, in every presentation. - */ -export async function readSheetHeaderRails( - page: Page, - sheetTestID: string, - title: string, -): Promise { - const header = page.getByTestId(sheetTestID); - const [titleRails, closeRails] = await Promise.all([ - readRails(header.getByText(title, { exact: true })), - readRails(header.getByRole("button", { name: "Close" })), - ]); - return { left: titleRails.left, right: closeRails.right }; -} - /** Vertical space between the bottom of one element and the top of the next. */ export async function readVerticalGap(upper: Locator, lower: Locator): Promise { const [above, below] = await Promise.all([readBox(upper), readBox(lower)]); diff --git a/packages/app/e2e/support/helpers/sidebar.ts b/packages/app/e2e/support/helpers/sidebar.ts index 0884fc5a3..f0f2ce4b6 100644 --- a/packages/app/e2e/support/helpers/sidebar.ts +++ b/packages/app/e2e/support/helpers/sidebar.ts @@ -166,6 +166,19 @@ export async function expectWorkspaceAbsentFromSidebar( ).toHaveCount(0, { timeout: 30_000 }); } +// The display-preferences menu is one row per decision, and the options sit a page below that +// row. Every caller has to walk the same path, so it lives here: when the menu's shape moves +// again, this is the only place that has to follow. +export async function openSidebarDisplayPage(page: Page, branchTestID: string): Promise { + await page.getByTestId("sidebar-display-preferences-menu").click(); + await page.getByTestId(branchTestID).click(); +} + +export async function selectSidebarStatusGrouping(page: Page): Promise { + await openSidebarDisplayPage(page, "sidebar-display-grouping"); + await page.getByTestId("sidebar-grouping-status").click(); +} + export async function openMobileAgentSidebar(page: Page): Promise { await page.getByRole("button", { name: "Open menu" }).click(); } diff --git a/packages/app/src/components/sidebar-workspace-list.tsx b/packages/app/src/components/sidebar-workspace-list.tsx index 38395dc55..3864544c6 100644 --- a/packages/app/src/components/sidebar-workspace-list.tsx +++ b/packages/app/src/components/sidebar-workspace-list.tsx @@ -111,6 +111,7 @@ import { SidebarWorkspaceTrailingActionOverlay, SidebarWorkspaceTrailingActionSlot, } from "@/components/sidebar/sidebar-workspace-row-content"; +import { useOpenKebabMenuVisibility } from "@/components/sidebar/use-open-kebab-menu-visibility"; import { selectWorkspaceScriptSummary } from "@/components/sidebar/workspace-meta-row"; import { SidebarWorkspaceTrailingContent, @@ -663,6 +664,7 @@ function WorkspaceRowRightGroup({ isTouchPlatform, showShortcut, }); + const kebab = useOpenKebabMenuVisibility(showKebabInSlot); return ( <> @@ -674,9 +676,10 @@ function WorkspaceRowRightGroup({ - + {onArchive ? ( - - {showKebab && onArchive ? ( + + {kebab.showKebab && onArchive ? ( void; openInFileManagerPath?: string | null; + /** + * Lifted so the row that reveals the kebab can keep it mounted while its menu is up. See + * `useOpenKebabMenuVisibility`. + */ + open?: boolean; + onOpenChange?: (open: boolean) => void; } -interface SidebarWorkspaceMenuItemsProps extends Omit { +interface SidebarWorkspaceMenuItemsProps extends Omit< + SidebarWorkspaceMenuProps, + "onArchive" | "open" | "onOpenChange" +> { onArchive?: () => void; } @@ -202,10 +211,12 @@ export function SidebarWorkspaceMenu({ isPinned, onTogglePin, openInFileManagerPath, + open, + onOpenChange, }: SidebarWorkspaceMenuProps) { const { t } = useTranslation(); return ( - + @@ -430,9 +432,10 @@ function WorkspaceRowTrailingActions({ - + {onArchive ? ( void }; +} { + const [open, setOpen] = useState(false); + const menuProps = useMemo(() => ({ open, onOpenChange: setOpen }), [open]); + return { showKebab: showKebab || open, menuProps }; +} diff --git a/packages/desktop/e2e/adaptive-sheet-content-inset.spec.ts b/packages/desktop/e2e/adaptive-sheet-content-inset.spec.ts index 0b1ed4545..58613234d 100644 --- a/packages/desktop/e2e/adaptive-sheet-content-inset.spec.ts +++ b/packages/desktop/e2e/adaptive-sheet-content-inset.spec.ts @@ -1,8 +1,6 @@ import { expect, test, type Page } from "../../app/e2e/support/fixtures"; import { gotoAppShell } from "../../app/e2e/support/helpers/app"; import { - readRails, - readSheetHeaderRails, readVerticalGap, waitForSettledPosition, } from "../../app/e2e/support/helpers/sheet-layout"; @@ -40,10 +38,6 @@ async function openChooseSkillsOnDesktop(page: Page): Promise { await openSkillSelection(page); } -function sheetHeaderRails(page: Page) { - return readSheetHeaderRails(page, "skill-selection-sheet", "Choose skills"); -} - function allSkillsCard(page: Page) { return page.getByTestId("skill-selection-all-card"); } @@ -59,15 +53,6 @@ function bundledSkillsLabel(page: Page) { test.describe("Choose skills sheet on a compact form factor", () => { test.use({ viewport: COMPACT_VIEWPORT }); - test("keeps its cards on the sheet header rails", async ({ page }) => { - await openChooseSkillsOnPhone(page); - - const header = await sheetHeaderRails(page); - - expect(await readRails(allSkillsCard(page))).toEqual(header); - expect(await readRails(skillListCard(page))).toEqual(header); - }); - test("gives the Bundled skills label more room above it than below", async ({ page }) => { await openChooseSkillsOnPhone(page); @@ -81,15 +66,6 @@ test.describe("Choose skills sheet on a compact form factor", () => { test.describe("Choose skills sheet on a desktop form factor", () => { test.use({ viewport: DESKTOP_VIEWPORT }); - test("keeps its cards on the sheet header rails", async ({ page }) => { - await openChooseSkillsOnDesktop(page); - - const header = await sheetHeaderRails(page); - - expect(await readRails(allSkillsCard(page))).toEqual(header); - expect(await readRails(skillListCard(page))).toEqual(header); - }); - test("gives the Bundled skills label more room above it than below", async ({ page }) => { await openChooseSkillsOnDesktop(page); From 7b6965856b8f8940f7ee6da1e625ce13ffeeeb5c Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Tue, 4 Aug 2026 13:09:30 +0200 Subject: [PATCH 2/2] Prevent workspace file watching from stalling the daemon (#2858) * fix(server): pin workspace file watchers to native backends Backend auto-detection could select an IPC watcher whose subscription lock deadlocked the Node worker pool during watcher reconfiguration. Select the host-native backend explicitly so filesystem work, logging, and relay DNS cannot be starved by that path. * test(server): cover native watcher production boundary Route the existing real filesystem observation test through the same subscription function used by the daemon. This proves native watcher selection and reconfiguration with actual file events instead of only testing the platform selector. * test(server): synchronize native watcher readiness Replace fixed startup sleeps with a real tracked-file event barrier. Native backends can resolve their subscription before the initial stream is fully drained, so the integration test now begins assertions only after an event has been delivered and refreshed. * fix(server): measure native workspace watchers --- .../server/scripts/measure-git-observation.ts | 3 +- ...it-service.observation.integration.test.ts | 24 ++++++++++++---- .../workspace-git-service.primitive.test.ts | 15 ++++++++++ .../src/server/workspace-git-service.ts | 28 ++++++++++++++++++- 4 files changed, 62 insertions(+), 8 deletions(-) diff --git a/packages/server/scripts/measure-git-observation.ts b/packages/server/scripts/measure-git-observation.ts index c501a333b..ecd6d07f2 100644 --- a/packages/server/scripts/measure-git-observation.ts +++ b/packages/server/scripts/measure-git-observation.ts @@ -17,6 +17,7 @@ import { } from "../src/utils/run-git-command.js"; import { CheckoutDiffManager } from "../src/server/checkout-diff-manager.js"; import { + subscribeToWorkspaceFileChanges, WorkspaceGitServiceImpl, type WorkspaceGitRuntimeSnapshot, } from "../src/server/workspace-git-service.js"; @@ -94,7 +95,7 @@ function createTrackedSubscriber(failWorktreeRoot: string | null) { if (failWorktreeRoot && path.resolve(args[0]) === failWorktreeRoot) { throw new Error("measurement watcher setup failure"); } - const subscription = await parcelWatcher.subscribe(...args); + const subscription = await subscribeToWorkspaceFileChanges(...args); subscriptions.add(subscription); return { unsubscribe: async () => { diff --git a/packages/server/src/server/workspace-git-service.observation.integration.test.ts b/packages/server/src/server/workspace-git-service.observation.integration.test.ts index 8decb8ca8..70e60fe16 100644 --- a/packages/server/src/server/workspace-git-service.observation.integration.test.ts +++ b/packages/server/src/server/workspace-git-service.observation.integration.test.ts @@ -6,7 +6,10 @@ import type pino from "pino"; import { afterEach, expect, test, vi } from "vitest"; import type { CheckoutSnapshotFacts, CheckoutStatusGit } from "../utils/checkout-git.js"; import { CheckoutDiffManager } from "./checkout-diff-manager.js"; -import { WorkspaceGitServiceImpl } from "./workspace-git-service.js"; +import { + subscribeToWorkspaceFileChanges, + WorkspaceGitServiceImpl, +} from "./workspace-git-service.js"; function createLogger(): pino.Logger { const logger = { @@ -83,7 +86,7 @@ test("native recursive observation updates tracked state and prunes ignored stor events: parcelWatcher.Event[]; }> = []; const subscribe: typeof parcelWatcher.subscribe = async (directory, callback, options) => { - const subscription = await parcelWatcher.subscribe( + const subscription = await subscribeToWorkspaceFileChanges( directory, (error, events) => { deliveredEvents.push({ directory, events }); @@ -194,9 +197,19 @@ test("native recursive observation updates tracked state and prunes ignored stor { timeout: 5_000 }, ); - // Parcel may deliver a startup batch before subscribe resolves. Let its - // already-scheduled consumer debounce finish inside the bootstrap phase. - await new Promise((resolve) => setTimeout(resolve, 250)); + writeFileSync(trackedPath, "base\n"); + await vi.waitFor( + () => { + const events = deliveredEvents.flatMap((batch) => batch.events); + expect(events.map((event) => event.path)).toContain(trackedPath); + expect(getCheckoutWorktreeState).toHaveBeenCalled(); + expect(service.getMetrics()).toMatchObject({ + workspaceRefreshInFlightCount: 0, + workspaceRefreshQueuedCount: 0, + }); + }, + { timeout: 5_000 }, + ); getCheckoutSnapshotFacts.mockClear(); getCheckoutStatus.mockClear(); @@ -206,7 +219,6 @@ test("native recursive observation updates tracked state and prunes ignored stor runGitCommand.mockClear(); deliveredEvents.length = 0; - await new Promise((resolve) => setTimeout(resolve, 250)); expect(runGitCommand).not.toHaveBeenCalled(); expect(getCheckoutWorktreeState).not.toHaveBeenCalled(); expect(getCheckoutDiff, JSON.stringify(deliveredEvents)).not.toHaveBeenCalled(); diff --git a/packages/server/src/server/workspace-git-service.primitive.test.ts b/packages/server/src/server/workspace-git-service.primitive.test.ts index 77380d72b..f0669c2f1 100644 --- a/packages/server/src/server/workspace-git-service.primitive.test.ts +++ b/packages/server/src/server/workspace-git-service.primitive.test.ts @@ -19,6 +19,7 @@ import { } from "../utils/checkout-git.js"; import { runGitCommand as runGitCommandReal } from "../utils/run-git-command.js"; import { + getWorkspaceFileWatcherBackend, getWorkspaceGitSelfHealPhaseMs, WorkspaceGitServiceImpl, type WorkspaceGitRuntimeSnapshot, @@ -891,6 +892,20 @@ describe("WorkspaceGitServiceImpl primitive refresh entrypoint", () => { expect(getWorkspaceGitSelfHealPhaseMs("/tmp/staggered-repo")).toBe(9_817); }); + test.each([ + ["darwin", "fs-events"], + ["linux", "inotify"], + ["win32", "windows"], + ] as const)("uses the native %s file watcher backend", (platform, backend) => { + expect(getWorkspaceFileWatcherBackend(platform)).toBe(backend); + }); + + test("rejects unsupported watcher platforms instead of using backend auto-detection", () => { + expect(() => getWorkspaceFileWatcherBackend("freebsd")).toThrow( + "No native workspace file watcher configured for freebsd", + ); + }); + test("self-heal staggers workspaces and settles into a 120 second cadence", async () => { vi.setSystemTime(0); const otherRepoCwd = resolvePath("/tmp/staggered-repo"); diff --git a/packages/server/src/server/workspace-git-service.ts b/packages/server/src/server/workspace-git-service.ts index f74c9670c..86ea5902b 100644 --- a/packages/server/src/server/workspace-git-service.ts +++ b/packages/server/src/server/workspace-git-service.ts @@ -306,6 +306,32 @@ interface WorkspaceGitServiceOptions { deps?: Partial; } +export function getWorkspaceFileWatcherBackend( + platform: NodeJS.Platform, +): parcelWatcher.BackendType { + switch (platform) { + case "darwin": + return "fs-events"; + case "linux": + return "inotify"; + case "win32": + return "windows"; + default: + throw new Error(`No native workspace file watcher configured for ${platform}`); + } +} + +export function subscribeToWorkspaceFileChanges( + directory: string, + callback: parcelWatcher.SubscribeCallback, + options?: parcelWatcher.Options, +): Promise { + return parcelWatcher.subscribe(directory, callback, { + ...options, + backend: getWorkspaceFileWatcherBackend(process.platform), + }); +} + interface WorkspaceGitTarget { cwd: string; listeners: Set; @@ -375,7 +401,7 @@ interface WorkspaceForgePrStatusPollTarget { function buildDefaultWorkspaceGitServiceDeps(): WorkspaceGitServiceDependencies { return { - subscribe: parcelWatcher.subscribe, + subscribe: subscribeToWorkspaceFileChanges, getCheckoutSnapshotFacts, getCheckoutStatus, getCheckoutShortstat,