fix(app): restore titlebar toggle visual contract after PR #878

packages/app/e2e/session/titlebar-right-rail-contract.spec.ts

@@ -0,0 +1,130 @@
+import { expect, test } from "../fixtures"
+import { openRightPanel } from "../actions"
+import { modKey } from "../utils"
+
+// Contract for the titlebar's right rail — the flex row inside the titlebar's
+// rightmost grid column that hosts:
+//   1. `#pawwork-titlebar-right`  → the right utility panel toggle (or
+//                                   StatusPopover fallback on non-session routes)
+//   2. `#pawwork-titlebar-tabs`   → the right-panel tab strip portal target
+//
+// PR #880 moved the tab strip from absolute overlay to an in-flow flex sibling
+// of the toggle so the two own disjoint geometry (no pointer-events
+// choreography needed). These tests guard the load-bearing properties of that
+// new layout — properties that are easy to silently break by dropping a
+// `self-stretch`, re-introducing absolute positioning on the slot, or
+// forgetting to grow the toggle area when the tab set grows.
+//
+// Sibling specs:
+//   - session-tab-chip-contract.spec.ts — chip visuals & geometry (×, hover,
+//     selection overlay, 4pt grid pinning)
+
+test.describe("titlebar right rail contract", () => {
+  test("right utility toggle stays clickable with the full tab set open", async ({
+    page,
+    gotoSession,
+  }) => {
+    // Regression guard: PR #878 had the titlebar tabs slot absolute-positioned
+    // over `#pawwork-titlebar-right`, relying on `pointer-events-none` /
+    // `-auto` choreography to keep the toggle clickable through the overlay.
+    // That broke at 4+ tabs because the `+` button got pushed into the
+    // toggle's x range and `pointer-events: auto` (required for the `+`
+    // dropdown to open) intercepted clicks meant for the toggle.
+    // Fix: the tabs slot is now an in-flow flex sibling of the toggle inside
+    // the titlebar's right rail (see `Titlebar` comments) — disjoint
+    // geometry, no overlay, no click choreography. This test opens the full
+    // default-reachable tab set so any future regression that reintroduces
+    // overlap (e.g. re-absolutising the slot, growing toggle into the rail)
+    // fails fast.
+    await gotoSession()
+    await openRightPanel(page)
+    // Open the maximum default-reachable tab set so the strip is widest.
+    await page.locator("main").first().click()
+    await page.keyboard.press(`${modKey}+\\`) // fileTree.toggle → Files
+    await page.keyboard.press(`${modKey}+Shift+R`) // review.toggle → Review
+    await page.keyboard.press("Control+`") // terminal.toggle → Terminal (always Ctrl)
+    await page.mouse.move(0, 0)
+
+    const toggle = page.getByRole("button", { name: "Right utility panel" })
+    await expect(toggle).toHaveAttribute("aria-expanded", "true")
+    // If anything in the tabs-portal overlay swallows the click, this
+    // times out with "subtree intercepts pointer events".
+    await toggle.click({ timeout: 3_000 })
+    await expect(toggle).toHaveAttribute("aria-expanded", "false", { timeout: 2_000 })
+  })
+
+  test("tabs slot shrinks to 0 width when viewport drops below the desktop breakpoint", async ({
+    page,
+    gotoSession,
+  }) => {
+    // Regression guard from PR #880 followup review: `SessionSidePanel` gates
+    // its render on `createMediaQuery("(min-width: 768px)")`, but
+    // `--right-panel-width` and the titlebar's `tabsRailActive` only check
+    // `layout.rightPanel.opened()`. Without an explicit viewport gate, opening
+    // the panel at desktop width and then shrinking the viewport below 768px
+    // would leave the titlebar reserving panel-width of empty rail (no portal
+    // mounts under the breakpoint), pushing the right utility toggle off the
+    // viewport edge with nothing visible to justify the gap.
+    await gotoSession()
+    await openRightPanel(page)
+    // Sanity: rail occupies panel-width while we're still desktop.
+    const desktopTabsWidth = await page
+      .locator("#pawwork-titlebar-tabs")
+      .evaluate((el) => Math.round(el.getBoundingClientRect().width))
+    expect(desktopTabsWidth).toBeGreaterThan(0)
+
+    await page.setViewportSize({ width: 600, height: 900 })
+
+    // Poll until layout settles — viewport resize → media query → Solid memo →
+    // DOM update isn't synchronous, and reading geometry on the same tick that
+    // `setViewportSize` resolves can race the transition.
+    await expect
+      .poll(
+        () =>
+          page.evaluate(() => {
+            const tabs = document.getElementById("pawwork-titlebar-tabs") as HTMLElement | null
+            const cs = tabs ? getComputedStyle(tabs) : null
+            return {
+              width: tabs ? Math.round(tabs.getBoundingClientRect().width) : null,
+              // `border-l` should be gone — no stray 1px line in a 0-width slot.
+              borderLeft: cs?.borderLeftWidth ?? null,
+            }
+          }),
+        { timeout: 2_000 },
+      )
+      .toEqual({ width: 0, borderLeft: "0px" })
+  })
+
+  test("tabs slot border-l spans the full titlebar height (no top/bottom seam break)", async ({
+    page,
+    gotoSession,
+  }) => {
+    // Regression guard from PR #880 followup review: with the tabs slot moved
+    // from absolute overlay to an in-flow flex sibling, its `self-stretch` only
+    // matches the parent flex container's content height. The titlebar root
+    // uses `items-center` (grid), which lets each grid cell collapse to its
+    // child's content box unless the cell opts out with `self-stretch` / `h-full`.
+    // If the right rail isn't full-height, the tabs slot's `border-l` also
+    // isn't full-height — it would visibly break above and below the toggle's
+    // 30px row, and stop meeting the right-panel body's `border-l` directly
+    // below the titlebar. This test pins the slot's painted height to the
+    // titlebar's own height so any future regression that drops the
+    // stretch chain fails fast.
+    await gotoSession()
+    await openRightPanel(page)
+    await page.mouse.move(0, 0)
+
+    const heights = await page.evaluate(() => {
+      const titlebar = document.querySelector('[data-component="titlebar-shell"]') as HTMLElement | null
+      const tabs = document.getElementById("pawwork-titlebar-tabs") as HTMLElement | null
+      return {
+        titlebar: titlebar ? Math.round(titlebar.getBoundingClientRect().height) : null,
+        tabs: tabs ? Math.round(tabs.getBoundingClientRect().height) : null,
+      }
+    })
+
+    expect(heights.titlebar).not.toBeNull()
+    expect(heights.tabs).not.toBeNull()
+    expect(heights.tabs).toBe(heights.titlebar)
+  })
+})

packages/app/e2e/snap/right-panel-tabs-hover.snap.ts

@@ -0,0 +1,136 @@
+import { expect, type Page } from "@playwright/test"
+import { openRightPanel, openSidebar } from "../actions"
+import { test } from "../fixtures"
+import { sessionItemSelector } from "../selectors"
+import { applyDarkModeForTests } from "../utils"
+import { composeGrid, snapOutputPath, type Shot } from "./_compose"
+
+// Hover-state probe for the right-panel tab strip. Captures three states of
+// the closable Files tab so we can verify the close-button affordance:
+//
+//   1) rest        — Files tab at rest, leading icon only, no close button
+//   2) hover       — mouse hovering Files tab; expectation: leading icon
+//                    fades to opacity 0, close × fades in at the same 14×14
+//                    cell, no layout shift, no double-icon visible
+//   3) measurements — page-level dump of bounding boxes for the close-button
+//                    slot vs leading icon vs label so we know exactly where
+//                    each piece lands in screen coordinates.
+//
+// Why not extend right-panel-titlebar.snap.ts: that snap intentionally moves
+// the mouse to (0,0) to freeze a clean static frame. Hover state needs its
+// own target so the two contracts don't fight.
+
+test.use({ viewport: { width: 1440, height: 900 }, deviceScaleFactor: 2, reducedMotion: "reduce" })
+
+async function openFilesAndReview(page: Page) {
+  await page.locator("main").first().click()
+  await page.keyboard.press("ControlOrMeta+\\")
+  await page.keyboard.press("ControlOrMeta+Shift+R")
+  await expect.poll(() => page.getByRole("tab").count(), { timeout: 5_000 }).toBe(3)
+  await page.getByRole("tab", { name: "Status" }).click()
+}
+
+async function dumpBoxes(page: Page, label: string) {
+  // Dump the bounding boxes of every interesting element in the tab strip so
+  // we can reason about exact positions vs the CSS-declared 14×14 + left:10
+  // contract in tabs.css.
+  const data = await page.evaluate(() => {
+    const tabs = Array.from(
+      document.querySelectorAll<HTMLElement>('[data-slot="tabs-trigger-wrapper"]'),
+    )
+    return tabs.map((wrap) => {
+      const trig = wrap.querySelector<HTMLElement>('[data-slot="tabs-trigger"]')
+      const icon = wrap.querySelector<HTMLElement>('[data-slot="tab-icon-default"]')
+      const iconSvg = icon?.querySelector<HTMLElement>('[data-component="icon"]')
+      const closeSlot = wrap.querySelector<HTMLElement>('[data-slot="tabs-trigger-close-button"]')
+      const closeBtn = closeSlot?.querySelector<HTMLElement>('[data-component="icon-button"]')
+      const closeBtnIcon = closeBtn?.querySelector<HTMLElement>('[data-component="icon"]')
+      const cs = (el?: HTMLElement | null) => (el ? window.getComputedStyle(el) : null)
+      const box = (el?: HTMLElement | null) => {
+        if (!el) return null
+        const r = el.getBoundingClientRect()
+        return { x: Math.round(r.x), y: Math.round(r.y), w: Math.round(r.width), h: Math.round(r.height) }
+      }
+      return {
+        value: wrap.getAttribute("data-value"),
+        selected: trig?.getAttribute("data-selected") === "" || trig?.hasAttribute("data-selected"),
+        wrapper: box(wrap),
+        trigger: box(trig),
+        leadingSpan: box(icon),
+        leadingIconDiv: box(iconSvg),
+        closeSlot: box(closeSlot),
+        closeBtn: box(closeBtn),
+        closeBtnIcon: box(closeBtnIcon),
+        closeSlotOpacity: cs(closeSlot)?.opacity,
+        leadingSpanOpacity: cs(icon)?.opacity,
+        triggerFontWeight: cs(trig)?.fontWeight,
+      }
+    })
+  })
+  process.stdout.write(`\n[boxes ${label}]\n${JSON.stringify(data, null, 2)}\n`)
+}
+
+// Tab-strip clip: shared by every shot so all states crop identically and
+// diff cleanly in the grid.
+const TAB_STRIP_CLIP = { x: 600, y: 0, width: 700, height: 80 } as const
+
+async function captureShot(page: Page, name: string): Promise<Shot> {
+  await page.waitForTimeout(200)
+  await dumpBoxes(page, name)
+  return {
+    name,
+    buf: await page.screenshot({ clip: TAB_STRIP_CLIP, animations: "disabled" }),
+  }
+}
+
+async function captureStates(
+  page: Page,
+  label: "light" | "dark",
+  sessionID: string,
+): Promise<Shot[]> {
+  await openSidebar(page)
+  await page.locator(sessionItemSelector(sessionID)).click()
+  await openRightPanel(page)
+  await openFilesAndReview(page)
+
+  // Rest state — mouse parked far away so no tab hovers.
+  await page.mouse.move(0, 0)
+  const rest = await captureShot(page, `${label}-rest`)
+
+  // Hover state — hover the Files tab (closable). This should fade the
+  // leading icon and reveal the × close button.
+  await page.getByRole("tab", { name: "Files" }).hover()
+  const hoverFiles = await captureShot(page, `${label}-hover-files`)
+
+  // Hover state on Review (also closable). openFilesAndReview clicks Status,
+  // so Review/Files are open but unselected — same expected swap as Files.
+  await page.getByRole("tab", { name: "Review" }).hover()
+  const hoverReview = await captureShot(page, `${label}-hover-review`)
+
+  return [rest, hoverFiles, hoverReview]
+}
+
+test("right-panel-tabs-hover", async ({ page, project }) => {
+  test.setTimeout(180_000)
+
+  let sessionID: string | undefined
+  await project.open({
+    beforeGoto: async ({ sdk }) => {
+      const session = await sdk.session.create({ title: "snap right panel tabs hover" }).then((res) => res.data)
+      sessionID = session?.id
+      // Track immediately on creation so cleanup runs even if a later
+      // step in project.open() throws before this test completes.
+      if (sessionID) project.trackSession(sessionID)
+    },
+  })
+  if (!sessionID) throw new Error("Session create did not return an id")
+
+  const shots: Shot[] = []
+  shots.push(...(await captureStates(page, "light", sessionID)))
+  await applyDarkModeForTests(page)
+  shots.push(...(await captureStates(page, "dark", sessionID)))
+
+  const out = snapOutputPath("right-panel-tabs-hover")
+  await composeGrid(shots, out)
+  process.stdout.write(`\n[snap] right-panel-tabs-hover grid -> ${out}\n\n`)
+})

packages/app/src/components/session/session-sortable-shell-tab.tsx

@@ -69,9 +69,12 @@ export function ShellTab(props: {
     props.onClose(props.value)
 
     // Wait one tick for Solid to commit the DOM removal, then restore focus.
+    // Re-check `isConnected` inside the rAF: rapid successive closes can
+    // remove the captured sibling before its frame fires, and calling
+    // `.focus()` on a detached node silently moves focus to <body>.
     if (focusTarget) {
       requestAnimationFrame(() => {
-        focusTarget?.focus()
+        if (focusTarget?.isConnected) focusTarget.focus()
       })
     }
   }
@@ -90,15 +93,23 @@ export function ShellTab(props: {
       value={props.value}
       class="shrink-0 h-full"
       classes={{
-        button:
-          "h-7 min-h-7 inline-flex items-center whitespace-nowrap rounded-md text-h3 text-fg-weak gap-1.5 px-2.5",
+        // Spacing (gap, padding-inline, border-radius) lives in tabs.css under
+        // the sidepanel variant: icon↔label gap `--space-sm` (8), chip-edge
+        // padding `--space-xs` (4), corner `--radius-md` (10). Tailwind
+        // utilities are kept out here because the app pins
+        // `html { font-size: 13px }`, which makes the default rem-based
+        // spacing scale drift off the 4pt grid (gap-2 = 0.5rem = 6.5px
+        // instead of 8). Routing through CSS variables in the variant block
+        // keeps the chip exactly on the grid.
+        button: "h-7 min-h-7 inline-flex items-center whitespace-nowrap text-h3 text-fg-weak",
       }}
       onMiddleClick={close}
       aria-label={props.label}
-      // Advertise the Delete key shortcut to assistive technology — only on
-      // closable tabs, since Status's onKeyDown is a no-op and exposing the
-      // shortcut there would be a false promise.
-      aria-keyshortcuts={props.closable ? "Delete" : undefined}
+      // Advertise both close shortcuts to assistive technology — handler
+      // below accepts Delete OR Backspace (the macOS alias). Space-separated
+      // per ARIA spec. Only declared on closable tabs; Status's onKeyDown is
+      // a no-op and exposing the shortcut there would be a false promise.
+      aria-keyshortcuts={props.closable ? "Delete Backspace" : undefined}
       onKeyDown={
         props.closable
           ? (event: KeyboardEvent) => {