Lead polish 2: full-split drop target, Favorite splits past 12, e2e evaluate race

docs/ARCHITECTURE.md

@@ -112,7 +112,7 @@ Rules: no synchronous disk I/O on hot paths after boot, no full-state broadcasts
 ## 9. Testing and CI
 
 - **Unit** (`vitest`): shared logic (tree moves, URL parsing, accelerators, theme math, persistence normalization), main-process models with fakes. Fast; run on every commit.
-- **E2E** (`@playwright/test` `_electron`): `tests/e2e/helpers.ts` launches the built app with an isolated `userData` and `ORBIT_E2E=1`. That exposes `globalThis.__orbit` test hooks in main (state, view bounds, run command) and serves local fixture pages from a loopback HTTP server, so there's no network dependence. **Every test title starts with the feature IDs it covers**, e.g. `test('[TB-06][TB-07] close and reopen a tab', …)`. `scripts/features.mjs` reads those IDs to mark rows tested in `FEATURES.md`.
+- **E2E** (`@playwright/test` `_electron`): `tests/e2e/helpers.ts` launches the built app with an isolated `userData` and `ORBIT_E2E=1`. That exposes `globalThis.__orbit` test hooks in main (state, view bounds, run command) and serves local fixture pages from a loopback HTTP server. The app runs behind a dead local proxy that loopback bypasses, and `launchOrbit` waits until every profile's session uses it, so a test that searches or follows a link to a real site gets the URL but loads nothing. Playwright reaches main through the inspector, which can interrupt main-process code part-way, even inside a store update. So `launchOrbit` makes each `app.evaluate` start in a task of its own, and the hooks' `run` and `action` defer the same way (D25). **Every test title starts with the feature IDs it covers**, e.g. `test('[TB-06][TB-07] close and reopen a tab', …)`. `scripts/features.mjs` reads those IDs to mark rows tested in `FEATURES.md`.
 - **CI** (`.github/workflows/ci.yml`): typecheck + lint + unit (Linux), e2e under Xvfb (Linux), and build + unit + e2e + unsigned `.dmg`/`.zip` packaging (macOS). Green CI is required before merge.
 - **Feel checks and clips**: `scripts/capture/record.mjs <scenario>` drives the built app through `scripts/capture/scenarios/<scenario>.mjs` while ffmpeg (`ORBIT_FFMPEG`, needs x11grab) records the X display, so native tab views are captured exactly as composited. Clips are named by feature ID and stored in `clips/` in the Orca context directory, screenshots in `screenshots/`. "Feels right" in `FEATURES.md` is a lead sign-off against `feel.md`.
 

src/main/split/index.ts

@@ -1,4 +1,5 @@
 import { clipboard, Menu, type MenuItemConstructorOptions, nativeTheme } from 'electron'
+import { FAVORITES_FULL_MESSAGE } from '../../shared/defaults'
 import { newId } from '../../shared/ids'
 import {
   equalizeLayout,
@@ -32,9 +33,11 @@ import {
   detachTab,
   dropBlankPanes,
   focusInDraft,
+  growsFavoritesPastLimit,
   isBlankPane,
   isFailure,
   leavesOf,
+  overflowsFavorites,
   popOutInDraft,
   separateInDraft,
   sidebarSorted,
@@ -138,6 +141,15 @@ export function registerSplit(ctx: MainContext): void {
       ctx.toast(windowId, SPLIT_FULL_MESSAGE)
       return null
     }
+    // A Favorite split's pane made into a new split leaves its own split as a new tile (FV-02).
+    if (
+      growsFavoritesPastLimit(state, (d) => {
+        createSplitInDraft(d, ordered, { axis: orientation, at: ordered[0] })
+      })
+    ) {
+      ctx.toast(windowId, FAVORITES_FULL_MESSAGE)
+      return null
+    }
     const active = windowId ? activeTabOf(state, windowId) : null
     let result: ReturnType<typeof createSplitInDraft> = 'invalid'
     updateMoving(ordered, (d) => {
@@ -428,18 +440,29 @@ export function registerSplit(ctx: MainContext): void {
     if (rest.length > 0) void commands.run('tabs.close', { tabIds: rest }, { windowId: windowOf(c) })
   })
 
-  commands.register('split.separate', ({ splitId }) => {
+  commands.register('split.separate', ({ splitId }, c) => {
+    // A Favorite split becomes one tile per pane: refused when that passes 12 Favorites (FV-02).
+    const tiles = leavesOf(store.state, splitId).filter((id) => !isPendingPane(store.state, id)).length
+    if (overflowsFavorites(store.state, splitId, tiles)) {
+      ctx.toast(windowOf(c), FAVORITES_FULL_MESSAGE)
+      return
+    }
     for (const id of leavesOf(store.state, splitId)) if (isPendingPane(store.state, id)) dropPending(id)
     store.update((d) => {
       separateInDraft(d, splitId)
     })
   })
 
-  commands.register('split.popOut', ({ tabId }) => {
+  commands.register('split.popOut', ({ tabId }, c) => {
     const state = store.state
     const splitId = splitOfTab(state, tabId)
     const split = splitId ? state.splits[splitId] : undefined
     if (!split) return
+    // The pane becomes a tile of its own beside a Favorite split (FV-02).
+    if (overflowsFavorites(state, split.id, 2)) {
+      ctx.toast(windowOf(c), FAVORITES_FULL_MESSAGE)
+      return
+    }
     const sibling = siblingOf(split.layout, tabId)
     store.update((d) => {
       for (const { windowId, spaceId } of windowsShowing(d, [tabId])) {
@@ -481,11 +504,16 @@ export function registerSplit(ctx: MainContext): void {
     if (other) updateLayout(splitId, (layout) => swapPanes(layout, tabId, other))
   })
 
-  commands.register('split.duplicate', ({ splitId }) => {
+  commands.register('split.duplicate', ({ splitId }, c) => {
     const state = store.state
     const split = state.splits[splitId]
     const loc = locate(state, splitId)
     if (!split || !loc) return null
+    // A Favorite split's copy is one more tile beside it (FV-02).
+    if (overflowsFavorites(state, splitId, 2)) {
+      ctx.toast(windowOf(c), FAVORITES_FULL_MESSAGE)
+      return null
+    }
     const now = Date.now()
     const copies = new Map<TabId, TabId>()
     const tabs = layoutLeaves(split.layout)

src/main/split/model.ts

@@ -4,6 +4,8 @@
  * themselves. Every helper keeps the invariants: 2–4 panes per split, canonical layout,
  * a tab in at most one place.
  */
+import { produce } from 'immer'
+import { MAX_FAVORITES } from '../../shared/defaults'
 import { newId } from '../../shared/ids'
 import {
   anchorOf,
@@ -219,6 +221,33 @@ export function popOutInDraft(d: OrbitState, tabId: TabId, opts: { toToday?: boo
   adoptPlacement(d, [tabId], today)
 }
 
+/**
+ * Whether a Favorite split coming apart into `tiles` tiles would take its profile past the
+ * Favorites limit (FV-02). A split anywhere else never counts.
+ */
+export function overflowsFavorites(d: OrbitState, splitId: SplitId, tiles: number): boolean {
+  const loc = locate(d, splitId)
+  if (loc?.container.kind !== 'favorites') return false
+  const count = d.profiles[loc.container.profileId]?.favorites.length ?? 0
+  return count - 1 + tiles > MAX_FAVORITES
+}
+
+/** Whether `container` is a Favorites grid with no room for one more tile (FV-02). */
+export function favoritesFull(d: OrbitState, container: ContainerRef): boolean {
+  return container.kind === 'favorites' && (d.profiles[container.profileId]?.favorites.length ?? 0) >= MAX_FAVORITES
+}
+
+/**
+ * Whether `recipe` would take a profile's Favorites past the limit (FV-02). It runs on a
+ * throwaway copy of the state; only a grid that grows counts.
+ */
+export function growsFavoritesPastLimit(d: OrbitState, recipe: (draft: OrbitState) => void): boolean {
+  const after = produce(d, recipe)
+  return Object.values(after.profiles).some(
+    (p) => p.favorites.length > MAX_FAVORITES && p.favorites.length > (d.profiles[p.id]?.favorites.length ?? 0),
+  )
+}
+
 /** Replaces a pane's tab with another open tab (which leaves its own place). The old tab goes to Today. */
 export function swapIntoPaneInDraft(d: OrbitState, paneTab: TabId, incoming: TabId): boolean {
   const splitId = splitOfTab(d, paneTab)
@@ -402,8 +431,9 @@ export function restoreUnitInDraft(d: OrbitState, unit: SplitUnit, now = Date.no
   const canonical = normalizeLayout(layout)
   if (!canonical || canonical.type === 'leaf') return undefined
   const first = present[0] as TabId
+  // Its old place, unless that is a full Favorites grid (FV-02): then Today.
   const where =
-    unit.location && containerItems(d, unit.location.container)
+    unit.location && containerItems(d, unit.location.container) && !favoritesFull(d, unit.location.container)
       ? unit.location
       : (() => {
           const today = todayOf(d, first)

src/main/tabs/ops.ts

@@ -3,7 +3,7 @@
  * drag-and-drop and the maintenance timers all go through these so the rules
  * (selection after close, archive, undo, pinned URL bookkeeping) live in one place.
  */
-import { createWindowState, MAX_FAVORITES } from '../../shared/defaults'
+import { createWindowState, FAVORITES_FULL_MESSAGE, MAX_FAVORITES } from '../../shared/defaults'
 import { isFolderId, isTabId, newId } from '../../shared/ids'
 import { isPendingPane, layoutLeaves, paneAfterClose, SPLIT_FULL_MESSAGE } from '../../shared/split'
 import type {
@@ -39,6 +39,7 @@ import {
 import type { MainContext } from '../context'
 import {
   addPaneInDraft,
+  favoritesFull,
   isFailure,
   type PaneAnchor,
   restorePaneInDraft,
@@ -151,7 +152,7 @@ export class TabOps {
     if (!spaceObj) throw new Error(`Unknown space ${space}`)
     const target: ContainerRef = opts.container ?? { kind: 'today', spaceId: space }
     if (target.kind === 'favorites' && this.favoriteCount(target.profileId) >= MAX_FAVORITES) {
-      this.ctx.toast(opts.windowId, `Favorites are full (${MAX_FAVORITES} max)`)
+      this.ctx.toast(opts.windowId, FAVORITES_FULL_MESSAGE)
       throw new Error('Favorites are full')
     }
     const profileId = profileOfContainer(state, target) ?? spaceObj.profileId
@@ -366,7 +367,11 @@ export class TabOps {
         }
         if (!d.spaces[tab.spaceId]) tab.spaceId = d.spaceOrder[0] as string
         d.tabs[tab.id] = tab
-        const where = rec.location && containerItems(d, rec.location.container) ? rec.location : undefined
+        // Its old place, unless that is a full Favorites grid (FV-02): then Today.
+        const where =
+          rec.location && containerItems(d, rec.location.container) && !favoritesFull(d, rec.location.container)
+            ? rec.location
+            : undefined
         let landed: ContainerRef | undefined
         // A pane goes back into its split, in this same update (no frame shows it on its own).
         const intoSplit = rec.pane ? restorePaneInDraft(d, tab.id, rec.pane) : undefined
@@ -774,7 +779,7 @@ export class TabOps {
         return !from || containerKey(from.container) !== target
       }).length
       if (incoming > 0 && this.favoriteCount(to.profileId) + incoming > MAX_FAVORITES) {
-        this.ctx.toast(opts.windowId, `Favorites are full (${MAX_FAVORITES} max)`)
+        this.ctx.toast(opts.windowId, FAVORITES_FULL_MESSAGE)
         return false
       }
     }

src/main/test-hooks.ts

@@ -7,11 +7,16 @@ import type { Persistence } from './persist'
  * ORBIT_E2E=1. Playwright reaches it with electronApp.evaluate().
  */
 export function exposeTestHooks(ctx: MainContext, persistence: Persistence): void {
+  // Playwright's evaluate can run in the middle of other main-process code (the inspector
+  // dispatches it by interrupt), even inside a store update whose result would then drop the
+  // command's change. The e2e helpers defer every evaluate; this covers the perf and capture scripts.
+  const later = <T>(fn: () => T | Promise<T>): Promise<T> => new Promise<void>((r) => setImmediate(r)).then(fn)
   const hooks = {
     ctx,
     state: () => ctx.store.state,
-    run: (name: CommandName, args: unknown, windowId?: string) => ctx.commands.run(name, args as never, { windowId }),
-    action: (id: string, windowId?: string) => ctx.actions.run(id, { windowId }),
+    run: (name: CommandName, args: unknown, windowId?: string) =>
+      later(() => ctx.commands.run(name, args as never, { windowId })),
+    action: (id: string, windowId?: string) => later(() => ctx.actions.run(id, { windowId })),
     flush: () => persistence.flush(),
     marks: () => ctx.perf.marks,
     windowIds: () => ctx.windows.ids(),

src/renderer/src/shell/sidebar/drag.ts

@@ -9,7 +9,7 @@
  */
 import { animate, motionValue } from 'motion/react'
 import { isFolderId, isSplitId, isTabId } from '../../../../shared/ids'
-import { rowDropSide } from '../../../../shared/split'
+import { layoutLeaves, MAX_PANES, rowDropSide } from '../../../../shared/split'
 import type { ItemId, OrbitState, SpaceId } from '../../../../shared/state'
 import { locate, parseContainerKey, visibleTabOrder } from '../../../../shared/tree'
 import { invoke } from '../../api'
@@ -367,6 +367,14 @@ function clearSplitDwell(s: Session): void {
   s.splitDwell = null
 }
 
+/** A split with no room for the tab: it takes no fifth pane, so its row is no split target (SV-25). */
+function isFullFor(key: string, tabId: string): boolean {
+  const layout = isSplitId(key) ? getOrbitState().splits[key]?.layout : undefined
+  if (!layout) return false
+  const leaves = layoutLeaves(layout)
+  return leaves.length >= MAX_PANES && !leaves.includes(tabId)
+}
+
 function splitTarget(s: Session, y: number): DropTarget | null {
   const first = s.ids[0]
   const scroll = ctx.scroll
@@ -380,6 +388,8 @@ function splitTarget(s: Session, y: number): DropTarget | null {
   for (const sl of s.slots) {
     if (sl.kind !== 'item' || sl.folder || s.dragged.has(sl.key) || sl.ancestors.some((a) => s.dragged.has(a))) continue
     if (!isTabId(sl.key) && !isSplitId(sl.key)) continue
+    // A full split shows no target and reorders like any other row, as on the page (SV-25).
+    if (isFullFor(sl.key, first)) continue
     const row = { x: box.left, y: sl.top + (shifts[sl.key] ?? 0), width: box.width, height: sl.height }
     const side = rowDropSide(row, s.lastX, y)
     if (!side) continue

src/shared/defaults.ts

@@ -43,6 +43,9 @@ export const DEFAULT_SETTINGS: Settings = {
 /** Favorites per profile (FV-02). */
 export const MAX_FAVORITES = 12
 
+/** The toast when a change would pass the Favorites limit (FV-02). */
+export const FAVORITES_FULL_MESSAGE = `Favorites are full (${MAX_FAVORITES} max)`
+
 const preset = (...colors: string[]): SpaceTheme => ({ colors, intensity: DEFAULT_INTENSITY, grain: 0 })
 
 /**

tests/e2e/helpers.ts

@@ -22,21 +22,70 @@ export interface Orbit {
   userData: string
 }
 
+/**
+ * Tests stay offline. Orbit runs behind a dead local proxy, which loopback (where the fixture pages
+ * are served) bypasses, so a search or a link to a real site gets its URL but loads nothing.
+ */
+const DEAD_PROXY = '127.0.0.1:9'
+const OFFLINE = `--proxy-server=http://${DEAD_PROXY}`
+
 /** Launches the built app (out/) with an isolated profile directory. */
 export async function launchOrbit(opts: { userData?: string; env?: Record<string, string> } = {}): Promise<Orbit> {
   const userData = opts.userData ?? mkdtempSync(join(tmpdir(), 'orbit-e2e-'))
-  const args = [resolve('out/main/index.js')]
+  const args = [OFFLINE, resolve('out/main/index.js')]
   // Linux CI/dev boxes often lack the setuid sandbox helper; macOS runs fully sandboxed.
   if (process.platform === 'linux') args.unshift('--no-sandbox')
   const app = await electron.launch({
     args,
     env: { ...process.env, ORBIT_E2E: '1', ORBIT_USER_DATA: userData, ...opts.env } as Record<string, string>,
   })
+  evaluateBetweenTasks(app)
   const shell = await viewPage(app, 'shell')
   await shell.getByTestId('sidebar').waitFor()
+  await waitOffline(app)
   return { app, shell, userData }
 }
 
+/**
+ * Waits until every profile's session sends web requests to the dead proxy. A session picks up the
+ * command line's proxy a moment after it's made: here, about 50 ms after the sidebar shows.
+ */
+async function waitOffline(app: ElectronApplication): Promise<void> {
+  const deadline = Date.now() + 10_000
+  for (;;) {
+    const routes = await app.evaluate(async ({ session }) => {
+      const { profiles } = (globalThis as unknown as { __orbit: { state: () => OrbitState } }).__orbit.state()
+      const sessions = [
+        session.defaultSession,
+        ...Object.values(profiles).map((p) => session.fromPartition(p.partition)),
+      ]
+      return Promise.all(sessions.map((s) => s.resolveProxy('https://example.com/')))
+    })
+    if (routes.every((r) => r === `PROXY ${DEAD_PROXY}`)) return
+    if (Date.now() > deadline) throw new Error(`Orbit would reach the network: ${routes.join(', ')}`)
+    await new Promise((r) => setTimeout(r, 20))
+  }
+}
+
+/**
+ * Playwright runs main-process evaluates through the inspector, which can interrupt main-process
+ * code part-way, even inside a store update. A change made there is lost when that update finishes:
+ * a background tab a test had just created vanished. Each evaluate starts in a task of its own instead.
+ */
+function evaluateBetweenTasks(app: ElectronApplication): void {
+  const evaluate = app.evaluate.bind(app)
+  app.evaluate = ((fn: unknown, arg?: unknown) => {
+    if (typeof fn !== 'function') return evaluate(fn as never, arg as never)
+    return evaluate(
+      async (electron, [source, a]) => {
+        await new Promise((resolve) => setImmediate(resolve))
+        return new Function(`return (${source})`)()(electron, a)
+      },
+      [fn.toString(), arg] as const,
+    )
+  }) as ElectronApplication['evaluate']
+}
+
 /** Finds an Orbit UI page (shell/overlay) by its ?view= query. */
 export async function viewPage(app: ElectronApplication, view: 'shell' | 'overlay'): Promise<Page> {
   const match = (p: Page) => p.url().includes(`view=${view}`)
@@ -159,18 +208,28 @@ export async function startFixtures(): Promise<Fixtures> {
     res.writeHead(404, { 'content-type': 'text/plain' })
     res.end('not found')
   })
-  const base = 3300 + (Number(process.env.TEST_PARALLEL_INDEX ?? 0) % 50)
-  await new Promise<void>((resolveListen, reject) => {
+  const port = await listenInRange(server, 3300 + (Number(process.env.TEST_PARALLEL_INDEX ?? 0) % 50))
+  return {
+    url: (path) => `http://localhost:${port}${path}`,
+    close: () => new Promise((r) => server.close(() => r())),
+  }
+}
+
+/**
+ * Listens on loopback at the first free port from `base` up: Orbit keeps to ports 3300–3399 (the
+ * brief's range), and several suites or agents may be using some of them at once.
+ */
+export function listenInRange(server: Server, base = 3300): Promise<number> {
+  return new Promise((resolve, reject) => {
     let port = base
-    server.on('error', (err: NodeJS.ErrnoException) => {
+    const retry = (err: NodeJS.ErrnoException) => {
       if (err.code === 'EADDRINUSE' && port < 3399) server.listen(++port, '127.0.0.1')
       else reject(err)
+    }
+    server.on('error', retry)
+    server.listen(port, '127.0.0.1', () => {
+      server.off('error', retry)
+      resolve(port)
     })
-    server.listen(port, '127.0.0.1', () => resolveListen())
   })
-  const { port } = server.address() as AddressInfo
-  return {
-    url: (path) => `http://localhost:${port}${path}`,
-    close: () => new Promise((r) => server.close(() => r())),
-  }
 }

tests/e2e/peek-helpers.ts

@@ -1,9 +1,8 @@
 import { createServer, type Server } from 'node:http'
-import type { AddressInfo } from 'node:net'
 import type { ElectronApplication, Page } from '@playwright/test'
 import type { MainContext } from '../../src/main/context'
 import type { OrbitState, WindowState } from '../../src/shared/state'
-import { state } from './helpers'
+import { listenInRange, state } from './helpers'
 
 /** The main process's test handle (src/main/test-hooks.ts), for app.evaluate() bodies. */
 export type OrbitGlobal = { __orbit: { ctx: MainContext } }
@@ -96,17 +95,8 @@ export async function startSites(): Promise<Sites> {
       )
     })
   })
-  // Ports 3350-3399 (helpers.ts's fixtures use 3300-3349).
-  const base = 3350 + (Number(process.env.TEST_PARALLEL_INDEX ?? 0) % 50)
-  await new Promise<void>((resolve, reject) => {
-    let p = base
-    server.on('error', (err: NodeJS.ErrnoException) => {
-      if (err.code === 'EADDRINUSE' && p < 3399) server.listen(++p, '127.0.0.1')
-      else reject(err)
-    })
-    server.listen(p, '127.0.0.1', () => resolve())
-  })
-  port = (server.address() as AddressInfo).port
+  // From 3350 up (helpers.ts's fixtures start at 3300).
+  port = await listenInRange(server, 3350 + (Number(process.env.TEST_PARALLEL_INDEX ?? 0) % 50))
   return {
     url: (path) => `http://localhost:${port}${path}`,
     other: (path) => `http://127.0.0.1:${port}${path}`,