From 90b86d0bf659cec3d8561bece03ce2243e12e7e4 Mon Sep 17 00:00:00 2001 From: Anton Date: Wed, 10 Jun 2026 23:37:40 +0200 Subject: [PATCH 1/2] fix: harden JSON stores, session restore and canvas placement fixes - quarantine corrupt JSON (userData stores, granted paths, workspace files) via shared quarantineCorruptFile instead of silently overwriting - validate workspace.json container shapes and fall through to .bak when the primary is parseable but structurally broken - reject terminal ids with path separators before joining log paths - recover dock window snapshots whose zones reference no panels by laying the orphaned panels into a fresh tab stack instead of dropping them - land keyboard-created panels on the active canvas, not the primary one - capture live terminal cwds for every activated workspace on session save - remount dock tab content when switching between same-type tabs so a reused instance never drives the previous tab's store - agent panel: drop stale model picks from disconnected providers once auth status arrives, unify composer disabled/placeholder states - add regression suites: dock/window flush races, placement, canvas and panel lifecycle, terminal lifecycle and logger, session serialize round-trip, panel transfer, plus a multi-canvas e2e spec --- e2e/multi-canvas.spec.ts | 185 +++++ src/agent/renderer/AgentPanel.tsx | 56 +- src/agent/renderer/agentModelPrefs.ts | 6 + src/main/dockWindowFlush.races.test.ts | 179 +++++ src/main/grantedPathStore.ts | 23 +- src/main/ipc/terminal.ts | 26 + src/main/ipc/terminalLogger.test.ts | 325 ++++++++ src/main/jsonFileStore.ts | 18 +- src/main/jsonStateFile.ts | 15 +- src/main/projectWorkspaceStore.ts | 69 +- src/main/quarantineCorruptFile.ts | 22 + src/main/windowPanels.races.test.ts | 223 ++++++ src/renderer/canvas/placement.test.ts | 332 ++++++++ src/renderer/docking/DockTabStack.tsx | 11 +- src/renderer/drag/commit.test.ts | 167 ++++ .../useCanvasInteraction.gesture.test.tsx | 703 +++++++++++++++++ src/renderer/lib/activePanel.test.ts | 9 +- .../lib/panelTransfer.receive.test.ts | 599 ++++++++++++++ .../lib/terminal/terminalLifecycle.test.ts | 593 ++++++++++++++ .../workspace/canvasAccess.placement.test.tsx | 69 ++ src/renderer/lib/workspace/canvasAccess.ts | 9 +- .../session.restoreDockWindow.test.ts | 82 +- src/renderer/lib/workspace/sessionSave.ts | 11 +- .../sessionSerialize.roundTrip.test.ts | 259 ++++++ src/renderer/lib/workspace/sessionStartup.ts | 45 +- .../appStore/closePanel.lifecycle.test.ts | 514 ++++++++++++ src/renderer/stores/appStore/helpers.ts | 13 +- .../stores/canvas/placementSlice.test.ts | 397 ++++++++++ .../stores/canvasStore.lifecycle.test.ts | 546 +++++++++++++ src/renderer/stores/dockStore.test.ts | 738 ++++++++++++++++++ 30 files changed, 6162 insertions(+), 82 deletions(-) create mode 100644 e2e/multi-canvas.spec.ts create mode 100644 src/main/dockWindowFlush.races.test.ts create mode 100644 src/main/ipc/terminalLogger.test.ts create mode 100644 src/main/quarantineCorruptFile.ts create mode 100644 src/main/windowPanels.races.test.ts create mode 100644 src/renderer/canvas/placement.test.ts create mode 100644 src/renderer/hooks/useCanvasInteraction.gesture.test.tsx create mode 100644 src/renderer/lib/panelTransfer.receive.test.ts create mode 100644 src/renderer/lib/terminal/terminalLifecycle.test.ts create mode 100644 src/renderer/lib/workspace/canvasAccess.placement.test.tsx create mode 100644 src/renderer/lib/workspace/sessionSerialize.roundTrip.test.ts create mode 100644 src/renderer/stores/appStore/closePanel.lifecycle.test.ts create mode 100644 src/renderer/stores/canvas/placementSlice.test.ts create mode 100644 src/renderer/stores/canvasStore.lifecycle.test.ts create mode 100644 src/renderer/stores/dockStore.test.ts diff --git a/e2e/multi-canvas.spec.ts b/e2e/multi-canvas.spec.ts new file mode 100644 index 00000000..ca81f6d8 --- /dev/null +++ b/e2e/multi-canvas.spec.ts @@ -0,0 +1,185 @@ +// Multi-canvas regressions — several canvas tabs in the center dock zone. +// +// Bug 1 (remount): DockTabStack rendered the active tab's content without a +// React key, so switching between two canvas tabs REUSED the same mounted +// Canvas instance with a swapped panelId prop. Canvas wires its world-transform +// subscription, wheel handling, and observers in mount-only effects, so the +// visible canvas kept rendering/zooming through the PREVIOUS canvas's store: +// zoom appeared dead, panels created on the hidden store never appeared, and +// the world transform showed another canvas's viewport. +// +// Bug 2 (placement routing): an unpinned panel create (keyboard shortcut, +// programmatic create) routed to the workspace's PRIMARY canvas (first canvas +// tab) instead of the ACTIVE one, so with a secondary canvas tab active the new +// panel landed on a hidden canvas. +import { test, expect } from '@playwright/test' +import { launchApp, closeApp } from './fixtures/electron-app' +import type { ElectronApplication, Page } from 'playwright' + +let app: ElectronApplication +let page: Page + +const EXTRA_CANVASES = 6 + +test.beforeEach(async () => { + ;({ electronApp: app, mainWindow: page } = await launchApp()) + await page.evaluate(() => window.__cateE2E!.setActiveLeftSidebarView(null)) + // Seed: 6 extra canvas tabs beside the default one. Each create activates + // the new tab, so the LAST canvas ends up active/mounted. + for (let i = 0; i < EXTRA_CANVASES; i++) { + await page.evaluate(() => void window.__cateE2E!.createCanvasPanel({ x: 100, y: 100 })) + } + await page.waitForTimeout(200) +}) +test.afterEach(async () => closeApp(app)) + +/** The single mounted canvas: its panel id, its world div's CSS transform, and + * its store zoom (the harness resolves the store by the mounted DOM id). */ +function mountedCanvas(p: Page) { + return p.evaluate(() => { + const el = document.querySelector('[data-canvas-panel-id]') as HTMLElement | null + const world = el?.querySelector('div[style*="transform-origin"]') as HTMLElement | null + return { + id: el?.getAttribute('data-canvas-panel-id') ?? null, + transform: world?.style.transform ?? null, + zoom: window.__cateE2E!.zoom(), + } + }) +} + +/** Ids of all canvas tabs in the center tab strip, in order. */ +function canvasTabIds(p: Page) { + return p.evaluate(() => + Array.from(document.querySelectorAll('.dock-tab-bar [data-tab-panel-id]')).map( + (el) => el.getAttribute('data-tab-panel-id')!, + ), + ) +} + +test('exactly one canvas is mounted and it is the last-created tab', async () => { + const tabs = await canvasTabIds(page) + expect(tabs.length).toBe(EXTRA_CANVASES + 1) + + const mountedIds = await page.evaluate(() => + Array.from(document.querySelectorAll('[data-canvas-panel-id]')).map((el) => + el.getAttribute('data-canvas-panel-id'), + ), + ) + expect(mountedIds).toEqual([tabs[tabs.length - 1]]) +}) + +test('unpinned create lands on the ACTIVE canvas, not the hidden primary one', async () => { + const result = await page.evaluate(async () => { + const mountedId = document + .querySelector('[data-canvas-panel-id]')! + .getAttribute('data-canvas-panel-id')! + const nodeId = window.__cateE2E!.createTerminal({ x: 300, y: 300 }) + await new Promise((r) => setTimeout(r, 200)) + return { + mountedId, + nodeId, + nodesOnMounted: window.__cateE2E!.nodes().length, + nodeInDom: !!document.querySelector(`[data-node-id="${nodeId}"]`), + } + }) + // The node must exist on the canvas the user is looking at AND be rendered. + expect(result.nodesOnMounted).toBe(1) + expect(result.nodeInDom).toBe(true) +}) + +test('zoom drives the mounted canvas: store and world transform stay in lock-step', async () => { + // Store-level zoom (toolbar/shortcut path) must move THIS canvas's world div. + const probe = await page.evaluate(async () => { + window.__cateE2E!.setZoom(2) + await new Promise((r) => setTimeout(r, 200)) + const world = document.querySelector( + '[data-canvas-panel-id] div[style*="transform-origin"]', + ) as HTMLElement + return { zoom: window.__cateE2E!.zoom(), transform: world.style.transform } + }) + expect(probe.zoom).toBe(2) + expect(probe.transform).toContain('scale(2)') +}) + +test('ctrl+wheel zooms the mounted canvas store (not a previous tab)', async () => { + const probe = await page.evaluate(async () => { + const el = document.querySelector('[data-canvas-panel-id]') as HTMLElement + const r = el.getBoundingClientRect() + const before = window.__cateE2E!.zoom() + el.dispatchEvent( + new WheelEvent('wheel', { + deltaY: -120, + clientX: r.left + r.width / 2, + clientY: r.top + r.height / 2, + ctrlKey: true, + bubbles: true, + cancelable: true, + }), + ) + // Smooth zoom is rAF-driven; give it time to settle. + await new Promise((res) => setTimeout(res, 800)) + return { before, after: window.__cateE2E!.zoom() } + }) + expect(probe.before).toBe(1) + expect(probe.after).toBeGreaterThan(1) +}) + +test('switching canvas tabs remounts: each canvas shows its OWN viewport', async () => { + const tabs = await canvasTabIds(page) + const last = tabs[tabs.length - 1] + const secondToLast = tabs[tabs.length - 2] + + // Zoom the active (last) canvas to 3. + await page.evaluate(() => window.__cateE2E!.setZoom(3)) + await page.waitForTimeout(100) + expect((await mountedCanvas(page)).transform).toContain('scale(3)') + + // Switch to the second-to-last canvas tab. + await page.click(`.dock-tab-bar [data-tab-panel-id="${secondToLast}"]`) + await page.waitForTimeout(200) + + let mounted = await mountedCanvas(page) + expect(mounted.id).toBe(secondToLast) + // Its viewport is its own (zoom 1) — NOT the previous tab's scale(3). + expect(mounted.zoom).toBe(1) + expect(mounted.transform).toContain('scale(1)') + + // Zooming THIS canvas updates THIS canvas's world div. + await page.evaluate(() => window.__cateE2E!.setZoom(2)) + await page.waitForTimeout(100) + mounted = await mountedCanvas(page) + expect(mounted.zoom).toBe(2) + expect(mounted.transform).toContain('scale(2)') + + // Switch back: the last canvas still has its own zoom 3. + await page.click(`.dock-tab-bar [data-tab-panel-id="${last}"]`) + await page.waitForTimeout(200) + mounted = await mountedCanvas(page) + expect(mounted.id).toBe(last) + expect(mounted.zoom).toBe(3) + expect(mounted.transform).toContain('scale(3)') +}) + +test('panels created across several canvas tabs land on their own canvas', async () => { + const tabs = await canvasTabIds(page) + // On each of the last three canvas tabs: activate, create, assert isolation. + for (const tabId of tabs.slice(-3)) { + await page.click(`.dock-tab-bar [data-tab-panel-id="${tabId}"]`) + await page.waitForTimeout(150) + const res = await page.evaluate(async (expected) => { + const mountedId = document + .querySelector('[data-canvas-panel-id]')! + .getAttribute('data-canvas-panel-id')! + const nodeId = window.__cateE2E!.createTerminal({ x: 200, y: 200 }) + await new Promise((r) => setTimeout(r, 150)) + return { + mountedOk: mountedId === expected, + nodes: window.__cateE2E!.nodes().length, + nodeInDom: !!document.querySelector(`[data-node-id="${nodeId}"]`), + } + }, tabId) + expect(res.mountedOk).toBe(true) + expect(res.nodes).toBe(1) // exactly its own node — no bleed from other tabs + expect(res.nodeInDom).toBe(true) + } +}) diff --git a/src/agent/renderer/AgentPanel.tsx b/src/agent/renderer/AgentPanel.tsx index 83b4310a..5ebc90ea 100644 --- a/src/agent/renderer/AgentPanel.tsx +++ b/src/agent/renderer/AgentPanel.tsx @@ -62,7 +62,7 @@ import type { AuthProviderStatus, } from '../../shared/types' import type { AgentMessage as StoreMessage } from './agentStore' -import { loadDefaultModel } from './agentModelPrefs' +import { loadDefaultModel, clearModelPrefsForProvider } from './agentModelPrefs' // ----------------------------------------------------------------------------- // Component @@ -137,6 +137,9 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { const currentUiRequest = uiRequests[0] const [providerStatuses, setProviderStatuses] = useState([]) + /** False until the first authStatus() round-trip — gates the stale-model + * reset below so an empty initial status list never wipes a valid pick. */ + const [authLoaded, setAuthLoaded] = useState(false) const [availableModels, setAvailableModels] = useState< Array<{ provider: string; model: string; label?: string }> >([]) @@ -211,6 +214,7 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { try { const statuses = await window.electronAPI.authStatus() setProviderStatuses(statuses) + setAuthLoaded(true) } catch (err) { log.warn('[AgentPanel] refreshAuth failed', err) } @@ -435,6 +439,7 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { // Read the draft straight from the active slice so we never send a stale // closure value mid-stream. const cur = useAgentStore.getState().panels[activeAgentKey] + if (!cur?.model) return const text = (cur?.draft ?? '').trim() const images = (cur?.draftImages ?? []).slice() if (!text && images.length === 0) return @@ -563,6 +568,27 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { return !!s?.connected }, [selectedModel, providerStatuses]) + // A model remembered from a provider the user has since cleared (saved + // default, or a resumed session's lastModel) should reset, not prompt a + // reconnect. Once real auth state is in, drop the stale pick — the auto-pick + // effect above then selects from whatever providers remain, or the "no + // model" hint shows when none do. + useEffect(() => { + if (!authLoaded || !activeAgentKey || !selectedModel) return + if (selectedProviderConnected) return + useAgentStore.getState().setModel(activeAgentKey, null) + clearModelPrefsForProvider(selectedModel.provider) + }, [authLoaded, activeAgentKey, selectedModel, selectedProviderConnected]) + + // With no model the composer is disabled and the placeholder doubles as the + // hint explaining why. The disconnected-provider term only matters in the + // window before the first authStatus() resolves — once it does, the reset + // effect above nulls the model and the no-model state takes over. + const composerDisabled = !selectedModel || !selectedProviderConnected + const composerPlaceholder = !selectedModel + ? 'No model selected or no provider is set up yet' + : undefined + const filteredChats = useMemo(() => { if (!chatSearch.trim()) return chats const q = chatSearch.trim().toLowerCase() @@ -1011,19 +1037,26 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { /> ) : (
- {selectedModel && !selectedProviderConnected && ( + {!selectedModel ? (
- Connect {selectedModel.provider} to start. + No model selected or no provider is set up yet.
- )} + ) : null} {/* Retry status is now shown inline in the chat thread */} @@ -1044,7 +1077,7 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { onChange={setDraft} onSubmit={handleSend} onStop={handleInterrupt} - disabled={!!selectedModel && !selectedProviderConnected} + disabled={composerDisabled} running={running} textareaRef={textareaRef} commands={commands} @@ -1063,11 +1096,7 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { planModeActive={planModeActive} onTogglePlanMode={handleTogglePlanMode} onSlashOpen={handleSlashOpen} - placeholder={ - !selectedModel ? 'Pick a model to start…' - : !selectedProviderConnected ? `Connect ${selectedModel.provider} to start…` - : 'Ask the agent anything about this workspace…' - } + placeholder={composerPlaceholder ?? 'Ask the agent anything about this workspace…'} />
@@ -1101,7 +1130,7 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { onChange={setDraft} onSubmit={handleSend} onStop={handleInterrupt} - disabled={!!selectedModel && !selectedProviderConnected} + disabled={composerDisabled} running={running} textareaRef={textareaRef} commands={commands} @@ -1120,6 +1149,7 @@ export default function AgentPanel({ panelId, workspaceId }: PanelProps) { planModeActive={planModeActive} onTogglePlanMode={handleTogglePlanMode} onSlashOpen={handleSlashOpen} + placeholder={composerPlaceholder} /> )} diff --git a/src/agent/renderer/agentModelPrefs.ts b/src/agent/renderer/agentModelPrefs.ts index ffc5c8b9..5977f156 100644 --- a/src/agent/renderer/agentModelPrefs.ts +++ b/src/agent/renderer/agentModelPrefs.ts @@ -18,3 +18,9 @@ export function loadDefaultModel(): AgentModelRef | null { export function saveDefaultModel(model: AgentModelRef | null): void { useSettingsStore.getState().setSetting('agentDefaultModel', model) } + +/** Drop every saved model preference that points at a provider the user just + * disconnected, so a stale pick doesn't resurface as a "reconnect" prompt. */ +export function clearModelPrefsForProvider(providerId: string): void { + if (loadDefaultModel()?.provider === providerId) saveDefaultModel(null) +} diff --git a/src/main/dockWindowFlush.races.test.ts b/src/main/dockWindowFlush.races.test.ts new file mode 100644 index 00000000..e8fcaa5a --- /dev/null +++ b/src/main/dockWindowFlush.races.test.ts @@ -0,0 +1,179 @@ +// ============================================================================= +// ORDERING RACES — flushDockWindowsBeforeQuit. +// +// dockWindowFlush.test.ts covers the steady-state contract; this file covers +// the interleavings around overlapping flushes and windows dying mid-flight: +// - a second flush starting while the first still awaits ACKs, with a window +// closing between the request and its ACK +// - a window closing after the request was sent (ACK never arrives) +// - an ACK fired synchronously from inside requestSync (subscription-first) +// - a tardy ACK arriving after the flush settled +// - a stale ACK from a PREVIOUS flush generation satisfying a NEW flush +// +// The flush is pure of Electron: "window closed" manifests as requestSync +// throwing (webContents gone) and/or the ACK never arriving. +// ============================================================================= + +import { describe, expect, it, afterEach, vi } from 'vitest' +import { flushDockWindowsBeforeQuit } from './dockWindowFlush' + +/** A tiny manual ack bus standing in for the IPC ack subscription. */ +function makeAckBus() { + const handlers = new Set<(id: number) => void>() + return { + subscribe: (h: (id: number) => void) => { + handlers.add(h) + return () => handlers.delete(h) + }, + ack: (id: number) => { + for (const h of handlers) h(id) + }, + handlerCount: () => handlers.size, + } +} + +afterEach(() => { + vi.useRealTimers() +}) + +describe('ordering races — flushDockWindowsBeforeQuit', () => { + it('overlapping flushes with a window closing between request and ACK: both settle cleanly, no hang, no per-flush double-send', async () => { + vi.useFakeTimers() + const bus = makeAckBus() + const closed = new Set() + const sendsA: number[] = [] + const sendsB: number[] = [] + const requestInto = (sends: number[]) => (id: number) => { + if (closed.has(id)) throw new Error('Object has been destroyed') // dead webContents + sends.push(id) + } + + // Flush A requests syncs from windows 1 and 2… + const pA = flushDockWindowsBeforeQuit({ + windowIds: [1, 2], + requestSync: requestInto(sendsA), + subscribeAck: bus.subscribe, + timeoutMs: 500, + }) + expect(sendsA).toEqual([1, 2]) + + // …then window 2 closes BEFORE acking, and a second flush starts while the + // first is still pending. + closed.add(2) + const pB = flushDockWindowsBeforeQuit({ + windowIds: [1, 2], + requestSync: requestInto(sendsB), + subscribeAck: bus.subscribe, + timeoutMs: 500, + }) + // B's request to the dead window threw and was swallowed; only 1 was reached. + expect(sendsB).toEqual([1]) + + // Window 1 acks ONCE. Both in-flight flushes observe the same ack — acks + // carry no flush generation, so a single ack satisfies every pending flush. + bus.ack(1) + + // Window 2 never acks; both flushes must resolve at their timeout. + vi.advanceTimersByTime(500) + const [ackedA, ackedB] = await Promise.all([pA, pB]) + expect([...ackedA]).toEqual([1]) + expect([...ackedB]).toEqual([1]) + + // No re-sends happened while waiting (exactly one request per window per + // flush), and both subscriptions were cleaned up — nothing hangs or leaks. + expect(sendsA).toEqual([1, 2]) + expect(sendsB).toEqual([1]) + expect(bus.handlerCount()).toBe(0) + }) + + it('window closes after the request was sent: ACK never arrives, flush resolves empty at the timeout', async () => { + vi.useFakeTimers() + const bus = makeAckBus() + const requestSync = vi.fn() + + const p = flushDockWindowsBeforeQuit({ + windowIds: [7], + requestSync, + subscribeAck: bus.subscribe, + timeoutMs: 250, + }) + expect(requestSync).toHaveBeenCalledTimes(1) + + // The window closes here; its ACK will never come. Just before the + // deadline nothing has been re-sent and the promise is still pending. + vi.advanceTimersByTime(249) + expect(requestSync).toHaveBeenCalledTimes(1) + + vi.advanceTimersByTime(1) + expect([...(await p)]).toEqual([]) + expect(bus.handlerCount()).toBe(0) + }) + + it('an ACK fired synchronously from inside requestSync is counted (subscription is live before any send)', async () => { + const bus = makeAckBus() + // A pathologically fast renderer: the ack arrives re-entrantly, during the + // request loop itself. + const p = flushDockWindowsBeforeQuit({ + windowIds: [1, 2], + requestSync: (id) => bus.ack(id), + subscribeAck: bus.subscribe, + timeoutMs: 10_000, + }) + expect([...(await p)].sort()).toEqual([1, 2]) + expect(bus.handlerCount()).toBe(0) + }) + + it('a tardy ACK after the flush settled is ignored and cannot mutate the resolved set', async () => { + vi.useFakeTimers() + const bus = makeAckBus() + const p = flushDockWindowsBeforeQuit({ + windowIds: [1, 2], + requestSync: () => {}, + subscribeAck: bus.subscribe, + timeoutMs: 100, + }) + + bus.ack(1) + vi.advanceTimersByTime(100) // settle with only window 1 + const acked = await p + expect([...acked]).toEqual([1]) + + // Window 2's ack finally limps in after resolution: the flush already + // unsubscribed, so this must neither throw nor grow the resolved set. + expect(() => bus.ack(2)).not.toThrow() + expect([...acked]).toEqual([1]) + }) + + it('a stale ACK from a previous flush generation satisfies a NEW flush (documents current behavior)', async () => { + vi.useFakeTimers() + const bus = makeAckBus() + + // Flush A's window is too slow: A times out without the ack. + const pA = flushDockWindowsBeforeQuit({ + windowIds: [1], + requestSync: () => {}, + subscribeAck: bus.subscribe, + timeoutMs: 100, + }) + vi.advanceTimersByTime(100) + expect([...(await pA)]).toEqual([]) + + // Flush B starts; the delayed ack — the renderer answering flush A's + // request — only now arrives. + const pB = flushDockWindowsBeforeQuit({ + windowIds: [1], + requestSync: () => {}, + subscribeAck: bus.subscribe, + timeoutMs: 100, + }) + bus.ack(1) + + // BUG?: ACKs carry no request generation/nonce, so flush B counts A's late + // ack as its own and resolves immediately — the state main reads may have + // been synced BEFORE B's request went out. Benign for the quit flow (the + // renderer did sync moments earlier, and a fresh sync request is also in + // flight) but it is a real ordering hole; a nonce would close it. + expect([...(await pB)]).toEqual([1]) + expect(bus.handlerCount()).toBe(0) + }) +}) diff --git a/src/main/grantedPathStore.ts b/src/main/grantedPathStore.ts index 582568db..328e0603 100644 --- a/src/main/grantedPathStore.ts +++ b/src/main/grantedPathStore.ts @@ -16,6 +16,7 @@ import { app } from 'electron' import fs from 'fs/promises' import log from './logger' import { writeJsonAtomic } from './writeJsonAtomic' +import { quarantineCorruptFile } from './quarantineCorruptFile' const STORE_FILENAME = 'granted-paths.json' @@ -27,16 +28,26 @@ let cache: Set | null = null async function load(): Promise> { if (cache) return cache + let raw: string | null = null try { - const raw = await fs.readFile(storePath(), 'utf-8') - const parsed = JSON.parse(raw) - if (Array.isArray(parsed)) { - cache = new Set(parsed.filter((x): x is string => typeof x === 'string')) - return cache - } + raw = await fs.readFile(storePath(), 'utf-8') } catch { // File missing or unreadable — treat as empty. } + if (raw !== null) { + try { + const parsed = JSON.parse(raw) + if (Array.isArray(parsed)) { + cache = new Set(parsed.filter((x): x is string => typeof x === 'string')) + return cache + } + } catch { + // Unparseable: quarantine before the next flush overwrites it, so the + // user's approved-path list stays recoverable. + const backup = quarantineCorruptFile(storePath()) + log.warn('[grantedPathStore] %s is corrupt%s; starting empty', STORE_FILENAME, backup ? `, backed up to ${backup}` : '') + } + } cache = new Set() return cache } diff --git a/src/main/ipc/terminal.ts b/src/main/ipc/terminal.ts index 18e2b5d2..1ad645e3 100644 --- a/src/main/ipc/terminal.ts +++ b/src/main/ipc/terminal.ts @@ -27,6 +27,7 @@ import { TERMINAL_SET_VISIBILITY, } from '../../shared/ipc-channels' import { getOrCreateLogger, removeLogger, flushAll as flushAllLoggers, disposeAll as disposeAllLoggers } from './terminalLogger' +import log from '../logger' import { sendToWindow, windowFromEvent, onWindowClosed } from '../windowRegistry' import { countTerminalData } from '../perf/perfMonitor' import { parseLocator, type CompanionId } from '../companion/locator' @@ -318,7 +319,28 @@ export function registerHandlers(): void { return companion.process.getCwd(ptyId) }) + // Scrollback/log file names are derived from ids supplied by the renderer + // (and, on restore, from hand-editable session.json) and joined into log-dir + // paths. Accept only a plain single-segment file name so a crafted id cannot + // escape the log directory via path separators or dot-dot. + function isSafeLogFileId(id: unknown): id is string { + return ( + typeof id === 'string' && + id.length > 0 && + id.length <= 256 && + !id.includes('/') && + !id.includes('\\') && + !id.includes('\0') && + id !== '.' && + id !== '..' + ) + } + ipcMain.handle(TERMINAL_LOG_READ, async (_event, terminalId: string): Promise => { + if (!isSafeLogFileId(terminalId)) { + log.warn('[terminal] rejected unsafe terminal id for log read: %s', String(terminalId)) + return null + } const { TerminalLogger } = await import('./terminalLogger') const logDir = TerminalLogger.getLogDir() const scrollbackPath = path.join(logDir, `${terminalId}.scrollback`) @@ -336,6 +358,10 @@ export function registerHandlers(): void { }) ipcMain.handle(TERMINAL_SCROLLBACK_SAVE, async (_event, ptyId: string, content: string): Promise => { + if (!isSafeLogFileId(ptyId)) { + log.warn('[terminal] rejected unsafe terminal id for scrollback save: %s', String(ptyId)) + return + } const { TerminalLogger } = await import('./terminalLogger') const logDir = TerminalLogger.getLogDir() await fs.mkdir(logDir, { recursive: true }) diff --git a/src/main/ipc/terminalLogger.test.ts b/src/main/ipc/terminalLogger.test.ts new file mode 100644 index 00000000..1faf2899 --- /dev/null +++ b/src/main/ipc/terminalLogger.test.ts @@ -0,0 +1,325 @@ +// Behavioral tests for the terminal scrollback logger — the thing that makes +// terminal session restore credible. Tests run against a REAL temp userData +// dir (mkdtemp), no fs mocks, following the projectTodosStore.test.ts pattern. + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' +import fs from 'fs' +import path from 'path' +import { tmpdir } from 'os' + +// terminalLogger resolves its directory via electron's app.getPath('userData'); +// point it at a per-test temp dir. vi.hoisted so the holder exists before the +// hoisted vi.mock factory references it. +const h = vi.hoisted(() => ({ userDataDir: '' })) +vi.mock('electron', () => ({ + app: { getPath: () => h.userDataDir }, +})) + +import { + TerminalLogger, + getOrCreateLogger, + removeLogger, + flushAll, + disposeAll, +} from './terminalLogger' + +const logDir = () => path.join(h.userDataDir, 'TerminalLogs') +const logPath = (id: string) => path.join(logDir(), `${id}.log`) +const prevLogPath = (id: string) => path.join(logDir(), `${id}.prev.log`) + +async function waitForFileContent(file: string, expected: string): Promise { + await vi.waitFor(() => { + expect(fs.existsSync(file)).toBe(true) + expect(fs.readFileSync(file, 'utf-8')).toBe(expected) + }, { timeout: 3000, interval: 20 }) +} + +beforeEach(() => { + h.userDataDir = fs.mkdtempSync(path.join(tmpdir(), 'cate-termlog-')) +}) + +afterEach(() => { + // Tear down loggers FIRST (stops timers, closes streams) before removing the + // directory they write into. + disposeAll() + fs.rmSync(h.userDataDir, { recursive: true, force: true }) +}) + +// =========================================================================== +// Round-trip +// =========================================================================== +describe('write → read round-trip', () => { + it('round-trips bytes exactly, including ANSI escapes and multibyte text', () => { + const logger = getOrCreateLogger('t-roundtrip') + const chunks = [ + '\x1b[32m$ npm test\x1b[0m\r\n', + 'héllo wörld — ünïcode ✓ 漢字\r\n', + '\x1b]0;title\x07partial line without newline', + ] + for (const c of chunks) logger.append(c) + + // readAll() drains the in-memory buffer synchronously first, so the bytes + // are observable immediately — no flush timer involved. + expect(logger.readAll()).toBe(chunks.join('')) + // And the same bytes are actually on disk. + expect(fs.readFileSync(logPath('t-roundtrip'), 'utf-8')).toBe(chunks.join('')) + }) + + it('preserves replay order across many appends and repeated sync flushes', () => { + const logger = getOrCreateLogger('t-order') + const chunks: string[] = [] + for (let i = 0; i < 200; i++) { + const chunk = `chunk-${String(i).padStart(3, '0')};` + chunks.push(chunk) + logger.append(chunk) + if (i % 7 === 0) logger.flushSync() // flush at irregular boundaries + } + expect(logger.readAll()).toBe(chunks.join('')) + }) + + it('appending an empty string is a no-op that creates no file', () => { + const logger = getOrCreateLogger('t-empty') + logger.append('') + logger.flushSync() + expect(fs.existsSync(logPath('t-empty'))).toBe(false) + expect(logger.readAll()).toBe('') + }) +}) + +// =========================================================================== +// Restore unhappy paths +// =========================================================================== +describe('restore edge cases', () => { + it('returns "" (no throw) when no scrollback file was ever written', () => { + const logger = getOrCreateLogger('t-missing') + expect(() => logger.readAll()).not.toThrow() + expect(logger.readAll()).toBe('') + }) + + it('returns "" (no throw) when even the log directory is missing', () => { + const logger = getOrCreateLogger('t-nodir') + fs.rmSync(logDir(), { recursive: true, force: true }) + expect(logger.readAll()).toBe('') + }) + + it('degrades gracefully on a truncated/corrupt scrollback file (invalid UTF-8)', () => { + fs.mkdirSync(logDir(), { recursive: true }) + // 'héllo' truncated mid-codepoint + raw garbage bytes: not valid UTF-8. + const corrupt = Buffer.concat([ + Buffer.from('h\xc3\xa9llo', 'binary').subarray(0, 3), // cut inside é + Buffer.from([0xff, 0xfe, 0x80, 0x81]), + ]) + fs.writeFileSync(logPath('t-corrupt'), corrupt) + + const logger = getOrCreateLogger('t-corrupt') + let result = '' + expect(() => { result = logger.readAll() }).not.toThrow() + // Documented behavior: Node's utf-8 decoder substitutes U+FFFD for the + // invalid sequences — the replay string is lossy but safe to write into + // xterm; no exception ever reaches the IPC handler. + expect(result).toContain('h') + expect(result).toContain('�') + }) + + it('still reads the prev rotation file when the current file is unreadable garbage', () => { + fs.mkdirSync(logDir(), { recursive: true }) + fs.writeFileSync(prevLogPath('t-half'), 'older history;') + fs.writeFileSync(logPath('t-half'), Buffer.from([0xc3])) // truncated lead byte + const logger = getOrCreateLogger('t-half') + const result = logger.readAll() + expect(result.startsWith('older history;')).toBe(true) // prev replayed first + expect(result).toContain('�') + }) +}) + +// =========================================================================== +// Rotation / size capping +// =========================================================================== +describe('rotation at the 1MB cap', () => { + const KB600 = 600 * 1024 + + it('rotates current → prev once the byte counter crosses 1MB, losing nothing', () => { + const logger = getOrCreateLogger('t-rotate') + const a = 'A'.repeat(KB600) + const b = 'B'.repeat(KB600) + + logger.append(a) + logger.flushSync() // 600KB — under the cap, no rotation + expect(fs.existsSync(prevLogPath('t-rotate'))).toBe(false) + + logger.append(b) + logger.flushSync() // counter was 600KB (< 1MB) before writing → still one file + expect(fs.existsSync(prevLogPath('t-rotate'))).toBe(false) + expect(fs.statSync(logPath('t-rotate')).size).toBe(2 * KB600) + + logger.append('C-after-rotate') + logger.flushSync() // counter now 1.2MB ≥ 1MB → rotate BEFORE writing + + // current(A+B) became prev; the new chunk starts a fresh current file. + expect(fs.readFileSync(prevLogPath('t-rotate'), 'utf-8')).toBe(a + b) + expect(fs.readFileSync(logPath('t-rotate'), 'utf-8')).toBe('C-after-rotate') + // Replay = prev + current, order preserved across the rotation boundary. + expect(logger.readAll()).toBe(a + b + 'C-after-rotate') + }) + + it('a second rotation drops the oldest window (two-file cap, by design)', () => { + const logger = getOrCreateLogger('t-rotate2') + const a = 'A'.repeat(KB600) + const b = 'B'.repeat(KB600) + const c = 'C'.repeat(KB600) + const d = 'D'.repeat(KB600) + + logger.append(a); logger.flushSync() + logger.append(b); logger.flushSync() + logger.append(c); logger.flushSync() // rotation #1: prev=A+B, current=C + logger.append(d); logger.flushSync() // current C+D (1.2MB), no rotate yet + logger.append('E'); logger.flushSync() // rotation #2: prev=C+D, current=E + + const replay = logger.readAll() + expect(replay).toBe(c + d + 'E') + expect(replay).not.toContain('A') // oldest 1.2MB window is gone — documented cap + }) + + it('seeds the byte counter from the on-disk size when reattaching to an existing log', () => { + // First logger writes 700KB, then is removed (terminal exited; logs kept). + const first = getOrCreateLogger('t-reattach') + const a = 'A'.repeat(700 * 1024) + first.append(a) + removeLogger('t-reattach') + expect(fs.statSync(logPath('t-reattach')).size).toBe(700 * 1024) + + // A NEW logger instance for the same id must count the existing 700KB — + // otherwise rotation would only trigger after a full fresh 1MB. + const second = getOrCreateLogger('t-reattach') + expect(second).not.toBe(first) + const b = 'B'.repeat(400 * 1024) + second.append(b) + second.flushSync() // 700KB on disk < 1MB → appends, file now 1.1MB + expect(fs.statSync(logPath('t-reattach')).size).toBe(1100 * 1024) + + second.append('tip') + second.flushSync() // 1.1MB ≥ 1MB → rotates + expect(fs.readFileSync(prevLogPath('t-reattach'), 'utf-8')).toBe(a + b) + expect(fs.readFileSync(logPath('t-reattach'), 'utf-8')).toBe('tip') + }) +}) + +// =========================================================================== +// Same-id writers / interleaving +// =========================================================================== +describe('concurrent writers for one terminal id', () => { + it('getOrCreateLogger returns the same instance for the same id — single writer, no interleaving', () => { + const a = getOrCreateLogger('t-same') + const b = getOrCreateLogger('t-same') + expect(b).toBe(a) + + // Two call sites appending through "different" handles serialize through + // the one in-memory buffer. + a.append('from-a;') + b.append('from-b;') + a.append('from-a-again;') + expect(a.readAll()).toBe('from-a;from-b;from-a-again;') + }) + + it('async stream flush and sync flush both land on disk, but readAll can miss/reorder in-flight stream data', async () => { + const logger = getOrCreateLogger('t-mixed') + + logger.append('first;') + logger.flush() // hot path: queued on the (asynchronously opening) write stream + logger.append('second;') + + // readAll only drains the in-memory buffer synchronously; the stream chunk + // is still in flight inside the just-created WriteStream. + const replay = logger.readAll() + + // BUG?: a readAll() that races a hot-path flush() returns the sync-flushed + // bytes but NOT the stream-buffered ones, and the file ends up with the + // chunks transposed ("second;first;"). In production this window is the + // ~250ms flush cadence: a session save/restore read issued right after a + // timer flush can produce a replay missing (and a log file reordering) the + // newest output. Pinning current behavior: + expect(replay).toBe('second;') + await waitForFileContent(logPath('t-mixed'), 'second;first;') + }) + + it('flushAll drains every logger to disk without tearing them down', async () => { + const l1 = getOrCreateLogger('t-fa-1') + const l2 = getOrCreateLogger('t-fa-2') + l1.append('one') + l2.append('two') + + flushAll() + + expect(fs.readFileSync(logPath('t-fa-1'), 'utf-8')).toBe('one') + expect(fs.readFileSync(logPath('t-fa-2'), 'utf-8')).toBe('two') + + // Loggers stay live: appends after the quit-path flush still work. + l1.append('-more') + expect(l1.readAll()).toBe('one-more') + }) +}) + +// =========================================================================== +// Cleanup / deletion paths +// =========================================================================== +describe('cleanup and deletion', () => { + it('delete() removes both rotation files; the replay afterwards is empty', () => { + const logger = getOrCreateLogger('t-del') + // Force a rotation so both files exist (sub-cap chunks + sync flushes so + // the size-cap stream path never kicks in). + logger.append('X'.repeat(600 * 1024)); logger.flushSync() + logger.append('X'.repeat(600 * 1024)); logger.flushSync() + logger.append('Y'); logger.flushSync() + expect(fs.existsSync(prevLogPath('t-del'))).toBe(true) + expect(fs.existsSync(logPath('t-del'))).toBe(true) + + logger.delete() + + expect(fs.existsSync(prevLogPath('t-del'))).toBe(false) + expect(fs.existsSync(logPath('t-del'))).toBe(false) + expect(logger.readAll()).toBe('') + }) + + it('removeLogger drains buffered output to disk and drops the instance, keeping the file', () => { + const logger = getOrCreateLogger('t-remove') + logger.append('buffered tail') + removeLogger('t-remove') + + // Nothing buffered was lost; the file survives for session restore. + expect(fs.readFileSync(logPath('t-remove'), 'utf-8')).toBe('buffered tail') + // The map slot was freed: same id now yields a fresh instance. + expect(getOrCreateLogger('t-remove')).not.toBe(logger) + }) + + it('disposeAll drains every buffer and clears the registry', () => { + const l1 = getOrCreateLogger('t-da-1') + const l2 = getOrCreateLogger('t-da-2') + l1.append('alpha') + l2.append('beta') + + disposeAll() + + expect(fs.readFileSync(logPath('t-da-1'), 'utf-8')).toBe('alpha') + expect(fs.readFileSync(logPath('t-da-2'), 'utf-8')).toBe('beta') + expect(getOrCreateLogger('t-da-1')).not.toBe(l1) + }) + + it('pruneOrphaned removes only files whose terminalId is not active', () => { + const live = getOrCreateLogger('t-live') + live.append('keep me') + live.flushSync() + fs.writeFileSync(logPath('t-dead'), 'stale') + fs.writeFileSync(prevLogPath('t-dead'), 'staler') + + TerminalLogger.pruneOrphaned(new Set(['t-live'])) + + expect(fs.existsSync(logPath('t-live'))).toBe(true) + expect(fs.existsSync(logPath('t-dead'))).toBe(false) + expect(fs.existsSync(prevLogPath('t-dead'))).toBe(false) + }) + + it('pruneOrphaned is a no-op when the log directory does not exist', () => { + fs.rmSync(logDir(), { recursive: true, force: true }) + expect(() => TerminalLogger.pruneOrphaned(new Set())).not.toThrow() + }) +}) diff --git a/src/main/jsonFileStore.ts b/src/main/jsonFileStore.ts index c266d33f..b5753b2d 100644 --- a/src/main/jsonFileStore.ts +++ b/src/main/jsonFileStore.ts @@ -10,22 +10,32 @@ import path from 'path' import { app } from 'electron' import log from './logger' import { writeJsonAtomicSync, writeTextAtomicSync } from './writeJsonAtomic' +import { quarantineCorruptFile } from './quarantineCorruptFile' function fullPath(filename: string): string { return path.join(app.getPath('userData'), filename) } -/** Read and JSON.parse a file under userData/. Returns the fallback on any failure. */ +/** Read and JSON.parse a file under userData/. Returns the fallback on any + * failure; an unparseable file is quarantined (copied aside as + * `.corrupt-`) first so the broken content stays recoverable. */ export function readJsonFile(filename: string, fallback: T): T { const p = fullPath(filename) + let raw: string try { if (!fs.existsSync(p)) return fallback - const raw = fs.readFileSync(p, 'utf-8') + raw = fs.readFileSync(p, 'utf-8') + } catch (err) { + log.warn('[jsonFileStore] read %s failed: %s', filename, err instanceof Error ? err.message : String(err)) + return fallback + } + try { const parsed = JSON.parse(raw) if (parsed && typeof parsed === 'object') return parsed as T return fallback - } catch (err) { - log.warn('[jsonFileStore] read %s failed: %s', filename, err instanceof Error ? err.message : String(err)) + } catch { + const backup = quarantineCorruptFile(p) + log.warn('[jsonFileStore] %s is corrupt%s; using fallback', filename, backup ? `, backed up to ${backup}` : '') return fallback } } diff --git a/src/main/jsonStateFile.ts b/src/main/jsonStateFile.ts index f7b970e3..f6421ad8 100644 --- a/src/main/jsonStateFile.ts +++ b/src/main/jsonStateFile.ts @@ -26,6 +26,7 @@ import path from 'path' import { watch, type FSWatcher } from 'chokidar' import log from './logger' import { writeJsonAtomic, writeJsonAtomicSync } from './writeJsonAtomic' +import { quarantineCorruptFile } from './quarantineCorruptFile' export interface JsonStateFileOptions { /** File name under `app.getPath('userData')`. */ @@ -87,14 +88,12 @@ export function createJsonStateFile(options: JsonStateFileOptions): JsonSt /** Copy an unparseable file aside so a corrupt hand-edit / crash-mid-write is * preserved for recovery instead of silently overwritten with defaults. */ - function quarantineCorruptFile(): void { - try { - const p = filePath() - const backup = `${p}.corrupt-${Date.now()}` - fsSync.copyFileSync(p, backup) + function quarantineCorrupt(): void { + const backup = quarantineCorruptFile(filePath()) + if (backup) { log.error('[jsonStateFile] %s is corrupt; backed up to %s and using defaults', filename, backup) - } catch (err) { - log.warn('[jsonStateFile] corrupt backup for %s failed: %O', filename, err) + } else { + log.warn('[jsonStateFile] corrupt backup for %s failed', filename) } } @@ -112,7 +111,7 @@ export function createJsonStateFile(options: JsonStateFileOptions): JsonSt current = normalize(parsed, defaults) lastWrittenContent = raw } catch { - quarantineCorruptFile() + quarantineCorrupt() current = defaults } } diff --git a/src/main/projectWorkspaceStore.ts b/src/main/projectWorkspaceStore.ts index 15626059..72d122d7 100644 --- a/src/main/projectWorkspaceStore.ts +++ b/src/main/projectWorkspaceStore.ts @@ -11,6 +11,8 @@ import { WORKSPACE_EXTERNAL_EDIT_DISMISS, } from '../shared/ipc-channels' import { holdsProjectLock, acquireProjectLock } from './projectLock' +import { isPlainObject } from './jsonUtils' +import { quarantineCorruptFile } from './quarantineCorruptFile' import type { ProjectWorkspaceFile, ProjectSessionFile } from '../shared/types' import { toRelativePath } from '../shared/pathUtils' import { broadcastToAll } from './windowRegistry' @@ -126,10 +128,19 @@ function atomicWriteSync(filePath: string, json: string): void { } async function tryReadJson(filePath: string): Promise { + let data: string + try { + data = await fs.readFile(filePath, 'utf-8') + } catch { + return null + } try { - const data = await fs.readFile(filePath, 'utf-8') return JSON.parse(data) as T } catch { + // The file exists but is unparseable: quarantine it so the broken content + // survives for recovery (the .bak tier handles the actual load fallback). + const backup = quarantineCorruptFile(filePath) + log.warn('Corrupt JSON at %s%s; ignoring', filePath, backup ? `, backed up to ${backup}` : '') return null } } @@ -184,10 +195,14 @@ function wouldEmptyOverwriteWorkspaceSync(rootPath: string, incomingNodeCount: n // Recovery tiers are primary then .bak. The writers (atomicWrite/atomicWriteSync) // no longer leave a fixed `.tmp` behind — they uniquify each tmp as // `...tmp` — so reading that stale name only ever found nothing. -async function tryReadWithFallback(filePath: string): Promise { +// When a validator is given, a parseable-but-invalid primary also falls through +// to the .bak tier instead of masking a still-good backup. +async function tryReadWithFallback(filePath: string, isValid?: (v: unknown) => boolean): Promise { const result = await tryReadJson(filePath) - if (result) return result - return tryReadJson(filePath + '.bak') + if (result && (!isValid || isValid(result))) return result + const bak = await tryReadJson(filePath + '.bak') + if (bak && (!isValid || isValid(bak))) return bak + return null } // Sweep orphaned `...tmp` files next to `filePath`. A crash @@ -231,17 +246,47 @@ async function readWorkspaceWithFallback(filePath: string): Promise + if (!isPlainObject(data)) return false // workspace.json carries the shareable name/color; session.json does not — // that's what tells the two version-1 files apart. - return obj.version === 1 && typeof obj.name === 'string' && typeof obj.color === 'string' + if (data.version !== 1 || typeof data.name !== 'string' || typeof data.color !== 'string') return false + // workspace.json is committable and hand-editable, so also check the container + // shapes the restore code dereferences. A structurally broken file degrades to + // the .bak tier instead of flowing malformed entries into the renderer (or + // crashing workspaceNodeCount on e.g. a null canvases entry). + if (data.dockState !== undefined) { + if (!isPlainObject(data.dockState) || !isPlainObject(data.dockState.zones)) return false + } + if (data.panels !== undefined) { + if (!isPlainObject(data.panels)) return false + for (const ref of Object.values(data.panels)) { + if (!isPlainObject(ref) || typeof ref.type !== 'string') return false + } + } + if (data.canvases !== undefined) { + if (!isPlainObject(data.canvases)) return false + for (const canvas of Object.values(data.canvases)) { + if (!isPlainObject(canvas)) return false + if (canvas.canvasNodes !== undefined && !isPlainObject(canvas.canvasNodes)) return false + } + } + return true } function isValidSession(data: unknown): data is ProjectSessionFile { - if (!data || typeof data !== 'object') return false - const obj = data as Record - return obj.version === 1 && obj.panels != null + if (!isPlainObject(data)) return false + if (data.version !== 1 || !isPlainObject(data.panels)) return false + for (const panel of Object.values(data.panels)) { + if (!isPlainObject(panel)) return false + } + if (data.dockWindows !== undefined) { + if (!Array.isArray(data.dockWindows)) return false + for (const dw of data.dockWindows) { + if (!isPlainObject(dw) || !isPlainObject(dw.panels)) return false + } + } + if (data.worktrees !== undefined && !Array.isArray(data.worktrees)) return false + return true } // Core local save: serializes the per-root write and applies both disk-boundary @@ -296,7 +341,7 @@ export async function loadProjectState(rootPath: string): Promise<{ .readFile(workspacePath(rootPath), 'utf-8') .then((raw) => rememberWorkspaceContent(rootPath, raw)) .catch(() => {}) - const sess = await tryReadWithFallback(sessionPath(rootPath)) + const sess = await tryReadWithFallback(sessionPath(rootPath), isValidSession) // Sweep any orphaned tmp files a crashed write may have left behind. await Promise.all([ cleanOrphanedTmpFiles(workspacePath(rootPath)), @@ -304,7 +349,7 @@ export async function loadProjectState(rootPath: string): Promise<{ ]) return { workspace: ws, - session: sess && isValidSession(sess) ? sess : null, + session: sess, } } diff --git a/src/main/quarantineCorruptFile.ts b/src/main/quarantineCorruptFile.ts new file mode 100644 index 00000000..becedd26 --- /dev/null +++ b/src/main/quarantineCorruptFile.ts @@ -0,0 +1,22 @@ +// ============================================================================= +// quarantineCorruptFile — copy an unparseable state file aside as +// `.corrupt-` before falling back to defaults, so a corrupt +// hand-edit or crash-mid-write is preserved for recovery instead of silently +// ignored or overwritten. Shared by every JSON-backed store (jsonStateFile, +// jsonFileStore, grantedPathStore, projectWorkspaceStore) so corrupt-file +// handling is consistent across them. +// ============================================================================= + +import fs from 'fs' + +/** Copy `filePath` aside as `.corrupt-`. Returns the backup path, + * or null when the copy failed (e.g. the file vanished). Never throws. */ +export function quarantineCorruptFile(filePath: string): string | null { + try { + const backup = `${filePath}.corrupt-${Date.now()}` + fs.copyFileSync(filePath, backup) + return backup + } catch { + return null + } +} diff --git a/src/main/windowPanels.races.test.ts b/src/main/windowPanels.races.test.ts new file mode 100644 index 00000000..8c88e2af --- /dev/null +++ b/src/main/windowPanels.races.test.ts @@ -0,0 +1,223 @@ +// ============================================================================= +// ORDERING RACES — cross-window panel discovery (main process). +// +// windowPanels.test.ts covers the steady-state contract; this file covers the +// orderings where IPC and window lifecycle interleave badly: +// - a panels report landing AFTER its window closed (late in-flight IPC) +// - a window destroyed WITHOUT a clean 'closed' event (crash-ish teardown) +// - register → immediate destroy → report +// - two windows closing back-to-back with reports still in flight +// - panel migration overlap (new owner reports before old owner closes) +// - the closing window itself being destroyed before 'closed' fires +// +// Harness replicated from windowPanels.test.ts (importing a test file would +// register its tests here), extended with a mutable `destroyed` flag so +// isDestroyed() can flip independently of the 'closed' event. +// ============================================================================= + +import { describe, it, expect, afterEach, vi } from 'vitest' + +// windowRegistry only uses BrowserWindow.fromWebContents (not exercised here); +// a stub class satisfies the import. +vi.mock('electron', () => ({ BrowserWindow: class {} })) +vi.mock('./perf/perfMonitor', () => ({ PERF_ENABLED: false, countIpc: vi.fn() })) + +import { registerWindow } from './windowRegistry' +import { setWindowPanels, getWindowPanels, revealWindowPanel } from './windowPanels' +import { WINDOW_PANELS_CHANGED } from '../shared/ipc-channels' +import type { WindowPanelInfo, WindowPanelReport, PanelState } from '../shared/types' + +// ----------------------------------------------------------------------------- +// Fake BrowserWindow harness (see windowPanels.test.ts) + `destroyed` flag. +// ----------------------------------------------------------------------------- + +interface FakeWin { + id: number + sent: Array<{ channel: string; args: unknown[] }> + focus: ReturnType + /** Flip to true to simulate destruction WITHOUT firing 'closed'. */ + destroyed: boolean + fireClosed: () => void + win: never +} + +const liveWindows = new Set() + +function makeWin(id: number): FakeWin { + const handlers: Record void> = {} + const sent: FakeWin['sent'] = [] + const fake: FakeWin = { + id, + sent, + focus: vi.fn(), + destroyed: false, + fireClosed: () => handlers['closed']?.(), + win: { + id, + isDestroyed: () => fake.destroyed, + isMinimized: () => false, + restore: vi.fn(), + focus: () => fake.focus(), + isAlwaysOnTop: () => false, + setAlwaysOnTop: vi.fn(), + getBounds: () => ({ x: 0, y: 0, width: 800, height: 600 }), + on: (event: string, cb: () => void) => { handlers[event] = cb }, + webContents: { send: (channel: string, ...args: unknown[]) => sent.push({ channel, args }) }, + } as never, + } + liveWindows.add(fake) + return fake +} + +function open(id: number, type: 'main' | 'dock' | 'panel', workspaceId?: string): FakeWin { + const fake = makeWin(id) + registerWindow(fake.win, type, workspaceId) + return fake +} + +const report = (panelId: string, type: PanelState['type'], title: string): WindowPanelReport => + ({ panelId, type, title, workspaceId: 'ws-A' }) + +function broadcastsTo(win: FakeWin): WindowPanelInfo[][] { + return win.sent + .filter((m) => m.channel === WINDOW_PANELS_CHANGED) + .map((m) => m.args[0] as WindowPanelInfo[]) +} + +afterEach(() => { + // Fire 'closed' on EVERY window (including destroyed-without-closed zombies) + // so windowRegistry's module-level maps and the broadcast signature reset. + for (const w of [...liveWindows]) w.fireClosed() + liveWindows.clear() +}) + +describe('ordering races — late reports vs window close', () => { + it('a panels report arriving AFTER the window closed is dropped: no resurrection, no throw, no rebroadcast', () => { + const main = open(1, 'main', 'ws-A') + const dock = open(201, 'dock', 'ws-A') + setWindowPanels(201, [report('t1', 'terminal', 'Term')]) + expect(getWindowPanels()).toHaveLength(1) + + // RACE: the renderer sent a fresh report, then the window closed before + // main processed it — the report is delivered after 'closed'. + dock.fireClosed() + const broadcastsAfterClose = broadcastsTo(main).length + expect(broadcastsTo(main).at(-1)).toEqual([]) + + expect(() => + setWindowPanels(201, [report('t1', 'terminal', 'Term'), report('t2', 'terminal', 'Term 2')]), + ).not.toThrow() + + // The closed window's entry must NOT be resurrected, and the empty union + // must not be rebroadcast for a dropped report. + expect(getWindowPanels()).toHaveLength(0) + expect(broadcastsTo(main).length).toBe(broadcastsAfterClose) + expect(revealWindowPanel('t1')).toBe(false) + }) + + it('two windows closing in quick succession with reports in flight converge to a clean, ordered union', () => { + const main = open(1, 'main', 'ws-A') + const a = open(210, 'dock', 'ws-A') + const b = open(211, 'dock', 'ws-A') + setWindowPanels(210, [report('a1', 'terminal', 'A1')]) + setWindowPanels(211, [report('b1', 'terminal', 'B1')]) + + // RACE: A closes; its in-flight report lands late; then B closes; B's + // late (even retitled) report lands after that. + a.fireClosed() + setWindowPanels(210, [report('a1', 'terminal', 'A1')]) + b.fireClosed() + setWindowPanels(211, [report('b1', 'terminal', 'B1 renamed')]) + + expect(getWindowPanels()).toEqual([]) + // Exact broadcast progression observed by the main window — the late + // reports must not have produced any extra or out-of-order broadcasts. + const seq = broadcastsTo(main).map((bc) => bc.map((p) => p.panelId).sort().join(',')) + expect(seq).toEqual(['a1', 'a1,b1', 'b1', '']) + }) + + it('panel migration overlap: replacement window reports the panel before the old owner closes', () => { + open(1, 'main', 'ws-A') + const oldWin = open(220, 'dock', 'ws-A') + setWindowPanels(220, [report('t1', 'terminal', 'Term')]) + + // RACE: during a drag-migration the new window reports the panel while the + // old window is still open — the union briefly holds BOTH owners. + const newWin = open(221, 'dock', 'ws-A') + setWindowPanels(221, [report('t1', 'terminal', 'Term')]) + expect(getWindowPanels().filter((p) => p.panelId === 't1')).toHaveLength(2) + + oldWin.fireClosed() + const final = getWindowPanels() + expect(final).toHaveLength(1) + expect(final[0]).toMatchObject({ panelId: 't1', ownerWindowId: 221 }) + + // Reveal targets the surviving owner, never the closed one. + expect(revealWindowPanel('t1')).toBe(true) + expect(newWin.focus).toHaveBeenCalled() + expect(oldWin.focus).not.toHaveBeenCalled() + }) +}) + +describe('ordering races — destroyed without a clean closed event', () => { + it('getWindowPanels hides a destroyed-but-not-closed window without throwing; reveal degrades to false', () => { + open(1, 'main', 'ws-A') + const dock = open(202, 'dock', 'ws-A') + setWindowPanels(202, [report('t1', 'terminal', 'Term')]) + expect(getWindowPanels()).toHaveLength(1) + + // RACE: the window is destroyed but its 'closed' event was never observed + // (e.g. teardown path that skips the handler). The registry entry is stale. + dock.destroyed = true + + expect(() => getWindowPanels()).not.toThrow() + // The getWindow() isDestroyed guard masks the stale entry from the union. + expect(getWindowPanels()).toHaveLength(0) + // Reveal must not focus or message a destroyed window. + expect(revealWindowPanel('t1')).toBe(false) + expect(dock.focus).not.toHaveBeenCalled() + }) + + it('register → immediate destroy → report: report is silently retained but invisible (no broadcast, no throw)', () => { + const main = open(1, 'main', 'ws-A') + const dock = open(203, 'dock', 'ws-A') + dock.destroyed = true // destroyed before its first report arrives + + const before = broadcastsTo(main).length + expect(() => setWindowPanels(203, [report('z1', 'terminal', 'Zombie')])).not.toThrow() + + // The union excludes it, and since the visible union didn't change, the + // signature guard suppresses any broadcast. + expect(getWindowPanels()).toHaveLength(0) + expect(broadcastsTo(main).length).toBe(before) + + // Internal-retention probe (NOT a real Electron sequence — window ids are + // never reused; this only proves the entry was stored, not dropped): + // because getWindowType() still answers for the stale registry entry, the + // zombie's report IS kept in the per-window map and only the getWindow() + // isDestroyed guard hides it. + dock.destroyed = false + expect(getWindowPanels().map((p) => p.panelId)).toEqual(['z1']) + dock.destroyed = true + // BUG?: a window that dies without ever emitting 'closed' leaks its report + // (plus its windowRegistry entries) forever — every consumer is shielded by + // the isDestroyed guard, so it is a bounded memory leak rather than a + // correctness bug, and real Electron does emit 'closed' on destroy(). + }) + + it('the closing window itself is skipped by the removal rebroadcast when destroyed before closed fires', () => { + const main = open(1, 'main', 'ws-A') + const dock = open(230, 'dock', 'ws-A') + setWindowPanels(230, [report('t1', 'terminal', 'T')]) + const dockSeen = broadcastsTo(dock).length + + // Real Electron ordering: the window is already destroyed by the time + // 'closed' fires, so the removal rebroadcast must not try to send to it. + dock.destroyed = true + expect(() => dock.fireClosed()).not.toThrow() + + expect(broadcastsTo(dock).length).toBe(dockSeen) // nothing sent to the dead webContents + expect(broadcastsTo(main).at(-1)).toEqual([]) // survivors still learn of the removal + expect(getWindowPanels()).toEqual([]) + }) +}) diff --git a/src/renderer/canvas/placement.test.ts b/src/renderer/canvas/placement.test.ts new file mode 100644 index 00000000..af1925f4 --- /dev/null +++ b/src/renderer/canvas/placement.test.ts @@ -0,0 +1,332 @@ +// ============================================================================= +// Placement engine — behavioral tests for the pure recommendation algorithm: +// recommendPlacements (the numbered-ghost candidates), findFreePosition (the +// non-interactive auto-placement), and nudgeToFree (the click-anywhere escape +// hatch). Contracts under test: candidates never overlap existing nodes or each +// other, keep their clearance gap, sit on the grid, are ranked best-first +// toward the anchor/focused node, and the function always returns >= 1 spot — +// deterministically — even for degenerate inputs. +// ============================================================================= + +import { describe, it, expect } from 'vitest' +import { recommendPlacements, findFreePosition, nudgeToFree } from './placement' +import { CANVAS_GRID_SIZE, rectsOverlap } from './layoutEngine' +import { PANEL_DEFAULT_SIZES } from '../../shared/types' +import type { CanvasNodeState, Point, Rect, Size } from '../../shared/types' + +const TERMINAL = PANEL_DEFAULT_SIZES.terminal // 640x400 + +function node( + id: string, + x: number, + y: number, + w = TERMINAL.width, + h = TERMINAL.height, + creationIndex = 0, +): CanvasNodeState { + return { + id, + panelId: `panel-${id}`, + origin: { x, y }, + size: { width: w, height: h }, + zOrder: creationIndex, + creationIndex, + } +} + +function nodeMap(...list: CanvasNodeState[]): Record { + return Object.fromEntries(list.map((n) => [n.id, n])) +} + +const VIEWPORT = { offset: { x: 0, y: 0 }, zoom: 1, containerSize: { width: 1200, height: 800 } } + +function rectOf(c: { point: Point; size: Size }): Rect { + return { origin: c.point, size: c.size } +} + +/** Inflate a rect by m on every side — used to assert the clearance gap. */ +function inflate(r: Rect, m: number): Rect { + return { + origin: { x: r.origin.x - m, y: r.origin.y - m }, + size: { width: r.size.width + m * 2, height: r.size.height + m * 2 }, + } +} + +function expectGridAligned(p: Point) { + // Normalize -0 from the % of negative coordinates. + expect(Math.abs(p.x % CANVAS_GRID_SIZE)).toBe(0) + expect(Math.abs(p.y % CANVAS_GRID_SIZE)).toBe(0) +} + +describe('recommendPlacements — empty canvas', () => { + it('returns a small set of standard-size spots centred on the anchor, best first', () => { + const anchor = { x: 600, y: 400 } + const out = recommendPlacements({}, null, 'terminal', VIEWPORT, anchor) + + // The exact contract for the blank-canvas case: the centred spot first, + // then below, then to the right (nearer first), all standard-sized. + expect(out).toEqual([ + { point: { x: 280, y: 200 }, size: TERMINAL }, + { point: { x: 280, y: 640 }, size: TERMINAL }, + { point: { x: 960, y: 200 }, size: TERMINAL }, + ]) + }) + + it('falls back to the viewport centre without an anchor, and to a fixed origin without a viewport', () => { + const fromViewCenter = recommendPlacements({}, null, 'terminal', VIEWPORT, null) + // View centre (600,400) → same first spot as anchoring there explicitly. + expect(fromViewCenter[0].point).toEqual({ x: 280, y: 200 }) + + const unmeasured = recommendPlacements( + {}, + null, + 'terminal', + { offset: { x: 0, y: 0 }, zoom: 1, containerSize: { width: 0, height: 0 } }, + null, + ) + expect(unmeasured.length).toBeGreaterThanOrEqual(1) + expect(unmeasured[0].point).toEqual({ x: 100, y: 100 }) + }) +}) + +describe('recommendPlacements — around existing nodes', () => { + it('packs candidates around the focused node: on grid, gap-clear of the node and each other, nearest first', () => { + const focused = node('a', 0, 0) + // Big viewport centred on the node so every candidate is on screen and the + // ranking is purely by distance. + const viewport = { + offset: { x: 1500, y: 1200 }, + zoom: 1, + containerSize: { width: 4000, height: 3000 }, + } + const out = recommendPlacements(nodeMap(focused), 'a', 'terminal', viewport, null) + + expect(out.length).toBeGreaterThanOrEqual(3) + expect(out.length).toBeLessThanOrEqual(6) + const nodeRect: Rect = { origin: focused.origin, size: focused.size } + for (const c of out) { + expectGridAligned(c.point) + // Clearance: even inflated by gap-1 (39px) the candidate must not touch + // the existing node or any other candidate. + expect(rectsOverlap(inflate(rectOf(c), 39), nodeRect)).toBe(false) + } + for (let i = 0; i < out.length; i++) { + for (let j = i + 1; j < out.length; j++) { + expect( + rectsOverlap(inflate(rectOf(out[i]), 39), rectOf(out[j])), + `candidates ${i} and ${j} violate the gap`, + ).toBe(false) + } + } + // Best-first: distances from the focused node's centre never decrease. + const center = { x: 320, y: 200 } + const dists = out.map((c) => + Math.hypot(c.point.x + c.size.width / 2 - center.x, c.point.y + c.size.height / 2 - center.y), + ) + for (let i = 1; i < dists.length; i++) { + expect(dists[i]).toBeGreaterThanOrEqual(dists[i - 1]) + } + }) + + it('with nothing focused, ranks from the cursor anchor — the best ghost fills the hole under the cursor', () => { + // Four nodes around a central hole; the anchor sits in the hole's middle. + const nodes = nodeMap( + node('a', 0, 0, 640, 400, 0), + node('b', 1360, 0, 640, 400, 1), + node('c', 0, 880, 640, 400, 2), + node('d', 1360, 880, 640, 400, 3), + ) + const viewport = { offset: { x: 0, y: 0 }, zoom: 1, containerSize: { width: 2200, height: 1400 } } + const anchor = { x: 1000, y: 640 } + + const out = recommendPlacements(nodes, null, 'terminal', viewport, anchor) + + expect(out.length).toBeGreaterThanOrEqual(1) + // The best candidate is centred on the anchor, inside the hole. + const best = out[0] + expect(best.point.x + best.size.width / 2).toBeCloseTo(anchor.x, 0) + expect(best.point.y + best.size.height / 2).toBeCloseTo(anchor.y, 0) + for (const c of out) { + for (const n of Object.values(nodes)) { + expect(rectsOverlap(rectOf(c), { origin: n.origin, size: n.size })).toBe(false) + } + } + }) + + it('crowded viewport with no qualifying hole still returns one non-overlapping fallback spot', () => { + // 3x2 tiling with 80px gaps — every hole is smaller than the minimum + // recommendation size, so the packer finds nothing and the fallback fires. + const list: CanvasNodeState[] = [] + let ci = 0 + for (const y of [0, 480]) { + for (const x of [0, 720, 1440]) list.push(node(`n${ci}`, x, y, 640, 400, ci++)) + } + const nodes = nodeMap(...list) + const viewport = { offset: { x: 0, y: 0 }, zoom: 1, containerSize: { width: 2200, height: 1000 } } + + const out = recommendPlacements(nodes, null, 'terminal', viewport, { x: 1100, y: 500 }) + + expect(out).toHaveLength(1) + for (const n of list) { + expect(rectsOverlap(rectOf(out[0]), { origin: n.origin, size: n.size })).toBe(false) + } + expectGridAligned(out[0].point) + }) + + it('panned away from every node, recommends where the camera is looking', () => { + const nodes = nodeMap(node('a', 0, 0)) + // Offset shifts the view far right: visible canvas x in [10000, 11200]. + const viewport = { offset: { x: -10000, y: 0 }, zoom: 1, containerSize: { width: 1200, height: 800 } } + + const out = recommendPlacements(nodes, 'a', 'terminal', viewport, null) + + // Centred on the view centre (10600, 400) → standard blank-space spots. + expect(out[0].point).toEqual({ x: 10280, y: 200 }) + for (const c of out) { + expect(rectsOverlap(rectOf(c), { origin: { x: 0, y: 0 }, size: TERMINAL })).toBe(false) + } + }) + + it('respects the max parameter', () => { + const out = recommendPlacements(nodeMap(node('a', 0, 0)), 'a', 'terminal', VIEWPORT, null, 2) + expect(out.length).toBeLessThanOrEqual(2) + expect(out.length).toBeGreaterThanOrEqual(1) + }) + + it('uses the size override for full-size candidates', () => { + const size = { width: 400, height: 300 } + const out = recommendPlacements(nodeMap(node('a', 0, 0)), 'a', 'terminal', VIEWPORT, null, 6, size) + expect(out.length).toBeGreaterThanOrEqual(1) + // In the open space around a single node, the best candidates carry the + // requested size (tight gaps may shrink later ones, never the first). + expect(out[0].size).toEqual(size) + }) + + it('is deterministic — identical inputs produce identical candidate lists', () => { + const nodes = nodeMap(node('a', 0, 0, 640, 400, 0), node('b', 900, 300, 300, 250, 1)) + const run = () => recommendPlacements(nodes, 'a', 'terminal', VIEWPORT, { x: 777, y: 333 }) + expect(run()).toEqual(run()) + }) +}) + +describe('recommendPlacements — degenerate inputs', () => { + it('zero-size panel: still returns at least one spot and never produces negative sizes', () => { + const zero = { width: 0, height: 0 } + const empty = recommendPlacements({}, null, 'terminal', VIEWPORT, { x: 600, y: 400 }, 6, zero) + expect(empty.length).toBeGreaterThanOrEqual(1) + + const around = recommendPlacements( + nodeMap(node('a', 0, 0)), + 'a', + 'terminal', + VIEWPORT, + null, + 6, + zero, + ) + expect(around.length).toBeGreaterThanOrEqual(1) + for (const c of [...empty, ...around]) { + expect(c.size.width).toBeGreaterThanOrEqual(0) + expect(c.size.height).toBeGreaterThanOrEqual(0) + expect(Number.isFinite(c.point.x)).toBe(true) + expect(Number.isFinite(c.point.y)).toBe(true) + } + }) + + it('extreme zoom-out (huge visible area) completes and keeps candidates clear of nodes', () => { + const nodes = nodeMap(node('a', 0, 0)) + // zoom 0.001 is floored to 0.01 internally → visible canvas 120000x80000. + const viewport = { offset: { x: 400, y: 300 }, zoom: 0.001, containerSize: { width: 1200, height: 800 } } + + const out = recommendPlacements(nodes, 'a', 'terminal', viewport, null) + + expect(out.length).toBeGreaterThanOrEqual(1) + expect(out.length).toBeLessThanOrEqual(6) + for (const c of out) { + expect(rectsOverlap(rectOf(c), { origin: { x: 0, y: 0 }, size: TERMINAL })).toBe(false) + } + }) + + it('extreme zoom-in (tiny visible area) still returns a spot', () => { + const nodes = nodeMap(node('a', 0, 0)) + const viewport = { offset: { x: -100, y: -100 }, zoom: 3, containerSize: { width: 1200, height: 800 } } + + const out = recommendPlacements(nodes, 'a', 'terminal', viewport, null) + + expect(out.length).toBeGreaterThanOrEqual(1) + for (const c of out) { + expect(rectsOverlap(rectOf(c), { origin: { x: 0, y: 0 }, size: TERMINAL })).toBe(false) + } + }) +}) + +describe('findFreePosition', () => { + it('empty canvas: returns the preferred point or the fixed default', () => { + expect(findFreePosition({}, null, TERMINAL)).toEqual({ x: 100, y: 100 }) + // The empty-canvas path returns the preferred point VERBATIM — grid + // snapping only kicks in once there are nodes to collide with. + expect(findFreePosition({}, null, TERMINAL, { x: 333, y: 287 })).toEqual({ x: 333, y: 287 }) + // With any node present, the same preferred point gets snapped. + const nodes = nodeMap(node('far', 9000, 9000)) + expect(findFreePosition(nodes, null, TERMINAL, { x: 333, y: 287 })).toEqual({ x: 340, y: 280 }) + }) + + it('honors a free preferred point but searches outward when it collides', () => { + const nodes = nodeMap(node('a', 0, 0)) + // Free preferred spot → returned as-is (already grid-aligned). + expect(findFreePosition(nodes, 'a', TERMINAL, { x: 1000, y: 1000 })).toEqual({ x: 1000, y: 1000 }) + // Colliding preferred spot → some other, non-overlapping slot. + const p = findFreePosition(nodes, 'a', TERMINAL, { x: 100, y: 100 }) + expect(rectsOverlap({ origin: p, size: TERMINAL }, { origin: { x: 0, y: 0 }, size: TERMINAL })).toBe(false) + }) + + it('picks the cardinal slot whose centre is nearest the reference', () => { + const nodes = nodeMap(node('a', 0, 0)) // 640x400 → vertical slots are nearer + const p = findFreePosition(nodes, 'a', TERMINAL) + expect(p).toEqual({ x: 0, y: 440 }) // below, one gap away + + // Block "below" → the next-nearest (above) wins over the farther sides. + const blocked = nodeMap(node('a', 0, 0, 640, 400, 0), node('b', 0, 440, 640, 400, 1)) + expect(findFreePosition(blocked, 'a', TERMINAL)).toEqual({ x: 0, y: -440 }) + }) + + it('without a focused node, searches from the most recently created node', () => { + const nodes = nodeMap(node('a', 0, 0, 640, 400, 0), node('b', 5000, 5000, 640, 400, 7)) + const p = findFreePosition(nodes, null, TERMINAL) + // Adjacent to b (the highest creationIndex), not to a. + expect(Math.hypot(p.x - 5000, p.y - 5000)).toBeLessThan(1000) + for (const n of Object.values(nodes)) { + expect(rectsOverlap({ origin: p, size: TERMINAL }, { origin: n.origin, size: n.size })).toBe(false) + } + }) +}) + +describe('nudgeToFree', () => { + it('returns the snapped desired point when it is already free', () => { + const nodes = nodeMap(node('a', 0, 0)) + expect(nudgeToFree(nodes, { width: 200, height: 200 }, { x: 1007, y: 993 })).toEqual({ x: 1000, y: 1000 }) + }) + + it('spirals off an occupied spot to a grid-aligned free position', () => { + const nodes = nodeMap(node('a', 0, 0)) + const size = { width: 200, height: 200 } + + const p = nudgeToFree(nodes, size, { x: 100, y: 100 }) // dead centre of the node + + expectGridAligned(p) + expect(rectsOverlap({ origin: p, size }, { origin: { x: 0, y: 0 }, size: TERMINAL })).toBe(false) + }) + + it('gives up and allows the overlap when everything within the search ring is covered', () => { + // One node blankets the entire spiral search radius (25 rings x 40px = 1000px). + const nodes = nodeMap(node('blanket', -2000, -2000, 4000, 4000)) + const size = { width: 100, height: 100 } + + const p = nudgeToFree(nodes, size, { x: 0, y: 0 }) + + // Documented give-up contract: the start point is returned even though it + // overlaps, rather than refusing the placement. + expect(p).toEqual({ x: 0, y: 0 }) + expect(rectsOverlap({ origin: p, size }, { origin: { x: -2000, y: -2000 }, size: { width: 4000, height: 4000 } })).toBe(true) + }) +}) diff --git a/src/renderer/docking/DockTabStack.tsx b/src/renderer/docking/DockTabStack.tsx index 27aa2a57..6690195e 100644 --- a/src/renderer/docking/DockTabStack.tsx +++ b/src/renderer/docking/DockTabStack.tsx @@ -325,9 +325,16 @@ export default function DockTabStack({ stack, zone: zoneProp, renderPanel, getPa )} - {/* Active panel content */} + {/* Active panel content. Keyed by panel id: switching between two tabs of + the SAME component type (canvas↔canvas, terminal↔terminal) must + remount the content, not reuse the instance with a swapped panelId — + panels wire store subscriptions in mount-only effects, so a reused + instance keeps driving the previous panel's store (visible canvas + transformed by the hidden canvas's zoom/offset). */}
- {activePanelId ? renderPanel(activePanelId) : ( + {activePanelId ? ( + {renderPanel(activePanelId)} + ) : (
No panel
diff --git a/src/renderer/drag/commit.test.ts b/src/renderer/drag/commit.test.ts index a1dc4008..7f2fc7a1 100644 --- a/src/renderer/drag/commit.test.ts +++ b/src/renderer/drag/commit.test.ts @@ -496,3 +496,170 @@ describe('commitDrop — terminal PTY preservation on same-window drags', () => expect(ctx.onRemovedFromCanvas).toHaveBeenCalledWith('p-term', 'terminal') }) }) + +// ----------------------------------------------------------------------------- +// Edge cases: stale state mid-drag — sources/targets that vanished or broke +// between resolve and commit must never lose the panel or leak a pending-detach. +// ----------------------------------------------------------------------------- + +describe('commitDrop — canvas-on-canvas refusal', () => { + it('refuses the drop without touching the source (would silently delete a canvas tab)', async () => { + const dock = createMockDockStore() + const tgt = createMockCanvasStore() + const source: DragSource = { + panelId: 'cv-1', + origin: { kind: 'dock-tab', dockStoreApi: dock.store, zone: 'center' as never, stackId: 'stack-1' }, + } + const target: DropTarget = { + kind: 'canvas-add', + canvasStoreApi: tgt.store, + origin: { x: 0, y: 0 }, + size: { width: 400, height: 300 }, + } + await commitDrop(source, target, { id: 'cv-1', type: 'canvas', title: 'Board' }, defaultCtx()) + expect(dock.state.undockPanel).not.toHaveBeenCalled() + expect(tgt.state.addNode).not.toHaveBeenCalled() + }) +}) + +describe('commitDrop — panel-window source onto dock targets', () => { + it('dock-tab and dock-split targets are no-ops (own-window dock is impossible)', async () => { + const tgtDock = createMockDockStore() + const source: DragSource = { panelId: 'panel-1', origin: { kind: 'panel-window' } } + + await commitDrop( + source, + { kind: 'dock-tab', dockStoreApi: tgtDock.store, stackId: 'stack-T' }, + panel, + defaultCtx(), + ) + await commitDrop( + source, + { kind: 'dock-split', dockStoreApi: tgtDock.store, stackId: 'stack-T', edge: 'right' }, + panel, + defaultCtx(), + ) + + expect(tgtDock.state.dockPanel).not.toHaveBeenCalled() + }) +}) + +describe('commitDrop — source vanished mid-drag', () => { + it('source dock unmounted (undockPanel throws): swallowed, panel still docks into the target', async () => { + const srcDock = createMockDockStore() + srcDock.state.undockPanel = vi.fn(() => { + throw new Error('source dock unmounted') + }) + const tgtDock = createMockDockStore() + const source: DragSource = { + panelId: 'panel-1', + origin: { kind: 'dock-tab', dockStoreApi: srcDock.store, zone: 'left' as never, stackId: 'stack-S' }, + } + const target: DropTarget = { + kind: 'dock-zone', + dockStoreApi: tgtDock.store, + zone: 'bottom' as never, + } + + await commitDrop(source, target, panel, defaultCtx()) + + expect(tgtDock.state.dockPanel).toHaveBeenCalledWith('panel-1', 'bottom') + }) + + it('source canvas store released (reconcile yields null): no crash, target placement still lands', async () => { + findCanvasStoreForNodeMock.mockReturnValue(null) + const tgtCanvas = createMockCanvasStore() + const source: DragSource = { + panelId: 'panel-1', + // The store was released mid-drag; the session has no store for the node + // and the caller-provided handle is gone too. + origin: { kind: 'canvas-node', canvasStoreApi: null as never, nodeId: 'node-gone' }, + } + const target: DropTarget = { + kind: 'canvas-add', + canvasStoreApi: tgtCanvas.store, + origin: { x: 0, y: 0 }, + size: { width: 320, height: 200 }, + } + + await commitDrop(source, target, panel, defaultCtx()) + + expect(tgtCanvas.state.addNode).toHaveBeenCalledWith( + 'panel-1', + 'editor', + { x: 0, y: 0 }, + { width: 320, height: 200 }, + ) + }) +}) + +describe('commitDrop — pending-detach pairing under IPC failure', () => { + function detachSetup() { + const srcCanvas = createMockCanvasStore() + findCanvasStoreForNodeMock.mockReturnValue(srcCanvas.store) + const source: DragSource = { + panelId: 'panel-1', + origin: { kind: 'canvas-node', canvasStoreApi: srcCanvas.store, nodeId: 'node-S' }, + } + const target: DropTarget = { kind: 'detach', screen: { x: 999, y: 100 } } + const beginPendingDetach = vi.fn() + const endPendingDetach = vi.fn() + return { srcCanvas, source, target, beginPendingDetach, endPendingDetach } + } + + it('crossWindowResolve rejects: endPendingDetach still runs, source untouched', async () => { + const { srcCanvas, source, target, beginPendingDetach, endPendingDetach } = detachSetup() + const ctx = defaultCtx({ + crossWindowResolve: vi.fn(async () => { + throw new Error('ipc dead') + }), + beginPendingDetach, + endPendingDetach, + }) + + await expect(commitDrop(source, target, panel, ctx)).rejects.toThrow('ipc dead') + + expect(beginPendingDetach).toHaveBeenCalledWith('panel-1', 'node-S') + expect(endPendingDetach).toHaveBeenCalledWith('panel-1') + expect(srcCanvas.state.finalizeRemoveNode).not.toHaveBeenCalled() + }) + + it('dragDetach rejects: endPendingDetach still runs, source untouched', async () => { + const { srcCanvas, source, target, beginPendingDetach, endPendingDetach } = detachSetup() + const ctx = defaultCtx({ + crossWindowResolve: vi.fn(async () => ({ claimed: false })), + dragDetach: vi.fn(async () => { + throw new Error('window spawn failed') + }), + beginPendingDetach, + endPendingDetach, + }) + + await expect(commitDrop(source, target, panel, ctx)).rejects.toThrow('window spawn failed') + + expect(endPendingDetach).toHaveBeenCalledWith('panel-1') + expect(srcCanvas.state.finalizeRemoveNode).not.toHaveBeenCalled() + }) + + it('dock-tab source passes a null nodeId to beginPendingDetach', async () => { + const srcDock = createMockDockStore() + const source: DragSource = { + panelId: 'panel-1', + origin: { kind: 'dock-tab', dockStoreApi: srcDock.store, zone: 'left' as never, stackId: 'stack-S' }, + } + const target: DropTarget = { kind: 'detach', screen: { x: 999, y: 100 } } + const beginPendingDetach = vi.fn() + const endPendingDetach = vi.fn() + const ctx = defaultCtx({ + crossWindowResolve: vi.fn(async () => ({ claimed: true })), + beginPendingDetach, + endPendingDetach, + }) + + await commitDrop(source, target, panel, ctx) + + expect(beginPendingDetach).toHaveBeenCalledWith('panel-1', null) + expect(endPendingDetach).toHaveBeenCalledWith('panel-1') + expect(srcDock.state.undockPanel).toHaveBeenCalledWith('panel-1') + }) +}) diff --git a/src/renderer/hooks/useCanvasInteraction.gesture.test.tsx b/src/renderer/hooks/useCanvasInteraction.gesture.test.tsx new file mode 100644 index 00000000..2b2f81d1 --- /dev/null +++ b/src/renderer/hooks/useCanvasInteraction.gesture.test.tsx @@ -0,0 +1,703 @@ +// ============================================================================= +// useCanvasInteraction — gesture behavior tests. +// +// Pins the canvas pan/zoom feel end-to-end through real DOM events: +// - wheel-intent disambiguation (Cmd/Ctrl+scroll & pinch → zoom, trackpad +// two-finger scroll → pan, physical mouse wheel → zoom under the select +// tool / pan under the hand tool); +// - cursor-anchored zoom (the canvas point under the cursor must not move in +// view-space) and ZOOM_MIN/ZOOM_MAX clamping; +// - right-click drag panning, the 4-px context-menu threshold, and the +// context-menu canvas-coordinate math under non-default zoom/offset; +// - momentum coasting after a fast right-drag (decays to a stop; killed +// instantly by a new drag or a zoom); +// - rAF lifecycle (wheel-pan throttling, unmount mid-animation). +// +// Driven like useNodeResize.gesture.test.tsx: a real React tree, native events +// dispatched on the rendered canvas div (wheel via a passive:false native +// listener, mirroring Canvas.tsx's wiring), a manually pumped +// requestAnimationFrame stub, and a fake performance.now clock so the +// velocity-sample windows and inertia decay are deterministic. +// ============================================================================= + +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest' + +// Heavy renderer modules whose import-time side effects explode under jsdom — +// pulled in transitively via the canvas store. Mirrors the sibling gesture tests. +vi.mock('../lib/terminal/terminalRegistry', () => ({ + terminalRegistry: { release: vi.fn() }, +})) +vi.mock('../lib/logger', () => ({ + default: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), log: vi.fn() }, +})) + +import * as React from 'react' +import { createRoot, type Root } from 'react-dom/client' +import { act } from 'react-dom/test-utils' +import type { StoreApi } from 'zustand' +import { useCanvasInteraction } from './useCanvasInteraction' +import { + getOrCreateCanvasStoreForPanel, + releaseCanvasStoreForPanel, + type CanvasStore, +} from '../stores/canvasStore' +import { useSettingsStore } from '../stores/settingsStore' +import { useUIStore } from '../stores/uiStore' +import { useDragStore } from '../drag' +import { viewToCanvas } from '../lib/canvas/coordinates' +import { ZOOM_MIN, ZOOM_MAX } from '../../shared/types' +import type { Point } from '../../shared/types' + +// createRoot + act needs this flag, or React logs "environment is not +// configured to support act(...)" to console.error — which the unmount test +// asserts stays clean. +;(globalThis as Record).IS_REACT_ACT_ENVIRONMENT = true + +// ----------------------------------------------------------------------------- +// rAF pump — multi-slot (the hook can have a zoom loop, a wheel-pan throttle +// commit, and an inertia loop alive in the same test). cancelAnimationFrame +// must only kill the handle it's given, otherwise "kill momentum on zoom" +// couldn't be told apart from "kill everything". +// ----------------------------------------------------------------------------- + +let rafCallbacks = new Map() +let nextRafId = 1 + +/** Fake clock driving performance.now(); advanced by pumpFrame. */ +let fakeNow = 1000 + +function pendingRafCount() { + return rafCallbacks.size +} + +/** Advance the clock and run every callback that was pending BEFORE the pump + * (callbacks scheduled during the pump wait for the next frame, like real rAF). */ +function pumpFrame(advanceMs = 16) { + fakeNow += advanceMs + const cbs = [...rafCallbacks.values()] + rafCallbacks.clear() + for (const cb of cbs) { + act(() => cb(fakeNow)) + } +} + +function pumpUntilIdle(maxFrames = 400) { + let n = 0 + while (rafCallbacks.size > 0 && n < maxFrames) { + pumpFrame() + n++ + } + if (rafCallbacks.size > 0) throw new Error(`rAF did not settle within ${maxFrames} frames`) + return n +} + +// ----------------------------------------------------------------------------- +// Scene +// ----------------------------------------------------------------------------- + +let container: HTMLDivElement +let root: Root +let rootUnmounted = false +let sceneCounter = 0 +const releaseStore: Array<() => void> = [] + +type Handlers = ReturnType + +function CanvasProbe({ + store, + expose, +}: { + store: StoreApi + expose: { current: Handlers | null } +}) { + const ref = React.useRef(null) + const handlers = useCanvasInteraction(ref, store) + expose.current = handlers + + // Mirror Canvas.tsx: handleWheel is wired through a NATIVE wheel listener + // (capture, passive:false) so preventDefault works, with the event cast to + // React's type. + const wheelRef = React.useRef(handlers.handleWheel) + wheelRef.current = handlers.handleWheel + React.useEffect(() => { + const el = ref.current + if (!el) return + const onWheel = (e: WheelEvent) => + wheelRef.current(e as unknown as React.WheelEvent) + el.addEventListener('wheel', onWheel, { capture: true, passive: false }) + return () => el.removeEventListener('wheel', onWheel, { capture: true }) + }, []) + + return ( +
+ {/* A fake node — right-clicks targeted here must NOT open the canvas menu. */} +
+
+ ) +} + +function setupScene(init?: { zoom?: number; offset?: Point }) { + const panelId = `canvas-interaction-test-${sceneCounter++}` + const store = getOrCreateCanvasStoreForPanel(panelId) + releaseStore.push(() => releaseCanvasStoreForPanel(panelId)) + act(() => { + store.getState().setZoomAndOffset(init?.zoom ?? 1, init?.offset ?? { x: 0, y: 0 }) + }) + + const expose: { current: Handlers | null } = { current: null } + act(() => { + root.render() + }) + const el = container.querySelector('[data-testid="canvas"]') + if (!el) throw new Error('canvas not rendered') + return { store, el, expose } +} + +// ----------------------------------------------------------------------------- +// Event drivers +// ----------------------------------------------------------------------------- + +interface WheelOpts { + deltaX?: number + deltaY?: number + metaKey?: boolean + ctrlKey?: boolean + clientX?: number + clientY?: number + /** Chromium's non-standard physical-wheel marker (multiples of 120). Leave + * undefined to simulate a trackpad. */ + wheelDeltaY?: number +} + +function wheel(el: HTMLElement, opts: WheelOpts): WheelEvent { + const e = new WheelEvent('wheel', { + deltaX: opts.deltaX ?? 0, + deltaY: opts.deltaY ?? 0, + deltaMode: 0, + metaKey: opts.metaKey ?? false, + ctrlKey: opts.ctrlKey ?? false, + clientX: opts.clientX ?? 0, + clientY: opts.clientY ?? 0, + bubbles: true, + cancelable: true, + }) + if (opts.wheelDeltaY !== undefined) { + Object.defineProperty(e, 'wheelDeltaY', { value: opts.wheelDeltaY }) + } + act(() => { + el.dispatchEvent(e) + }) + return e +} + +function mouse(el: HTMLElement, type: string, button: number, clientX: number, clientY: number) { + act(() => { + el.dispatchEvent(new MouseEvent(type, { button, clientX, clientY, bubbles: true })) + }) +} + +const rightDown = (el: HTMLElement, x: number, y: number) => mouse(el, 'mousedown', 2, x, y) +const moveTo = (el: HTMLElement, x: number, y: number) => mouse(el, 'mousemove', 2, x, y) +const rightUp = (el: HTMLElement, x: number, y: number) => mouse(el, 'mouseup', 2, x, y) + +/** Expected anchored offset for a zoom to `targetZoom` around view point `v`, + * given the state at the moment the gesture began. */ +function anchoredOffset(v: Point, zoom: number, offset: Point, targetZoom: number): Point { + const c = viewToCanvas(v, zoom, offset) + return { x: v.x - c.x * targetZoom, y: v.y - c.y * targetZoom } +} + +// ----------------------------------------------------------------------------- + +beforeEach(() => { + rafCallbacks = new Map() + nextRafId = 1 + fakeNow = 1000 + vi.stubGlobal('requestAnimationFrame', (cb: FrameRequestCallback) => { + const id = nextRafId++ + rafCallbacks.set(id, cb) + return id + }) + vi.stubGlobal('cancelAnimationFrame', (id: number) => { + rafCallbacks.delete(id) + }) + vi.spyOn(performance, 'now').mockImplementation(() => fakeNow) + + container = document.createElement('div') + document.body.appendChild(container) + rootUnmounted = false + act(() => { + root = createRoot(container) + }) + + useSettingsStore.setState({ zoomSpeed: 1.0 }) + useUIStore.setState({ activeTool: 'select', marquee: null }) + useDragStore.setState({ isDragging: false }) +}) + +afterEach(() => { + if (!rootUnmounted) act(() => root.unmount()) + container.remove() + rafCallbacks.clear() + vi.useRealTimers() + vi.unstubAllGlobals() + vi.restoreAllMocks() + document.body.classList.remove('canvas-interacting', 'canvas-dragging') + useUIStore.setState({ activeTool: 'select', marquee: null }) + useDragStore.setState({ isDragging: false }) + while (releaseStore.length) releaseStore.pop()!() +}) + +// ============================================================================= +// 1. Wheel intent — zoom vs pan +// ============================================================================= + +describe('wheel intent', () => { + it('Cmd+trackpad-scroll zooms about the cursor: zoom changes, the canvas point under the cursor stays fixed', () => { + const { store, el } = setupScene() + const cursor = { x: 400, y: 300 } + const before = store.getState() + + // deltaY -100 → target = 1 + 100*0.01*zoomSpeed = 2.0 + const e = wheel(el, { deltaY: -100, metaKey: true, clientX: cursor.x, clientY: cursor.y }) + expect(e.defaultPrevented).toBe(true) + + // Smooth zoom is rAF-driven; nothing applied synchronously beyond frame 0. + pumpUntilIdle() + + const after = store.getState() + expect(after.zoomLevel).toBeCloseTo(2.0, 6) + // Anchor invariance: the canvas point under the cursor before == after. + const canvasBefore = viewToCanvas(cursor, before.zoomLevel, before.viewportOffset) + const canvasAfter = viewToCanvas(cursor, after.zoomLevel, after.viewportOffset) + expect(canvasAfter.x).toBeCloseTo(canvasBefore.x, 6) + expect(canvasAfter.y).toBeCloseTo(canvasBefore.y, 6) + // Exact anchored offset: (400,300) at zoom 1/offset 0 → offset (-400,-300). + expect(after.viewportOffset.x).toBeCloseTo(-400, 6) + expect(after.viewportOffset.y).toBeCloseTo(-300, 6) + }) + + it('trackpad pinch (ctrlKey synthesized by Chromium) zooms about the cursor', () => { + const { store, el } = setupScene() + const cursor = { x: 100, y: 50 } + + wheel(el, { deltaY: -50, ctrlKey: true, clientX: cursor.x, clientY: cursor.y }) + pumpUntilIdle() + + const after = store.getState() + expect(after.zoomLevel).toBeCloseTo(1.5, 6) + const expected = anchoredOffset(cursor, 1, { x: 0, y: 0 }, 1.5) + expect(after.viewportOffset.x).toBeCloseTo(expected.x, 6) + expect(after.viewportOffset.y).toBeCloseTo(expected.y, 6) + }) + + it('clamps zoom at ZOOM_MAX (huge zoom-in) and ZOOM_MIN (huge zoom-out)', () => { + const { store, el } = setupScene() + + // 1 + 1000*0.01 = 11 → clamped to ZOOM_MAX. + wheel(el, { deltaY: -1000, metaKey: true, clientX: 200, clientY: 200 }) + pumpUntilIdle() + expect(store.getState().zoomLevel).toBeCloseTo(ZOOM_MAX, 6) + + // From 3.0: 3 - 1000*0.01 = -7 → clamped to ZOOM_MIN. + wheel(el, { deltaY: 1000, metaKey: true, clientX: 200, clientY: 200 }) + pumpUntilIdle() + expect(store.getState().zoomLevel).toBeCloseTo(ZOOM_MIN, 6) + }) + + it('rapid zoom wheels accumulate from the in-flight target, not the live zoom', () => { + const { store, el } = setupScene() + + // Two -50 deltas before any frame runs: 1.0 → target 1.5 → target 2.0. + wheel(el, { deltaY: -50, metaKey: true, clientX: 300, clientY: 300 }) + wheel(el, { deltaY: -50, metaKey: true, clientX: 300, clientY: 300 }) + pumpUntilIdle() + + expect(store.getState().zoomLevel).toBeCloseTo(2.0, 6) + }) + + it('plain trackpad two-finger scroll pans (rAF-throttled, deltas coalesced into one commit)', () => { + const { store, el } = setupScene() + + // No modifiers, no wheelDeltaY, deltaMode 0 → classified as trackpad → pan. + const e1 = wheel(el, { deltaX: 30, deltaY: 50 }) + expect(e1.defaultPrevented).toBe(true) + expect(document.body.classList.contains('canvas-interacting')).toBe(true) + + // Second wheel before the frame fires — must coalesce into ONE rAF commit. + wheel(el, { deltaX: -10, deltaY: 20 }) + expect(pendingRafCount()).toBe(1) + // Nothing applied until the frame runs. + expect(store.getState().viewportOffset).toEqual({ x: 0, y: 0 }) + + pumpFrame() + // offset -= summed deltas: (30-10, 50+20) = (20, 70). + expect(store.getState().viewportOffset).toEqual({ x: -20, y: -70 }) + expect(store.getState().zoomLevel).toBe(1) + + // Throttle is drained — extra frames change nothing. + pumpFrame() + expect(store.getState().viewportOffset).toEqual({ x: -20, y: -70 }) + }) + + it('releases the canvas-interacting body class ~150ms after the wheel goes quiet', () => { + vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] }) + const { el } = setupScene() + + wheel(el, { deltaX: 0, deltaY: 40 }) + pumpFrame() + expect(document.body.classList.contains('canvas-interacting')).toBe(true) + + // Another wheel within the quiet window re-arms the timer. + act(() => vi.advanceTimersByTime(100)) + wheel(el, { deltaX: 0, deltaY: 10 }) + act(() => vi.advanceTimersByTime(100)) + expect(document.body.classList.contains('canvas-interacting')).toBe(true) + + act(() => vi.advanceTimersByTime(60)) // past 150ms since the last wheel + expect(document.body.classList.contains('canvas-interacting')).toBe(false) + }) + + it('physical mouse wheel zooms proportionally under the select tool', () => { + const { store, el } = setupScene() + const cursor = { x: 200, y: 150 } + + // wheelDeltaY ±120 + deltaX 0 → physical mouse notch → zoom, not pan. + wheel(el, { deltaY: -100, wheelDeltaY: 120, clientX: cursor.x, clientY: cursor.y }) + pumpUntilIdle() + + const after = store.getState() + // One notch: 1 * (1 + 0.15 * zoomSpeed) = 1.15, anchored at the cursor. + expect(after.zoomLevel).toBeCloseTo(1.15, 6) + const expected = anchoredOffset(cursor, 1, { x: 0, y: 0 }, 1.15) + expect(after.viewportOffset.x).toBeCloseTo(expected.x, 6) + expect(after.viewportOffset.y).toBeCloseTo(expected.y, 6) + }) + + it('physical mouse wheel PANS under the hand tool (no zoom)', () => { + useUIStore.setState({ activeTool: 'hand' }) + const { store, el } = setupScene() + + wheel(el, { deltaY: 100, wheelDeltaY: -120 }) + pumpFrame() + + expect(store.getState().zoomLevel).toBe(1) + expect(store.getState().viewportOffset).toEqual({ x: 0, y: -100 }) + }) + + it('swallows wheel input while a marquee selection is in progress', () => { + const { store, el } = setupScene() + useUIStore.setState({ marquee: { startX: 0, startY: 0, currentX: 10, currentY: 10 } }) + + const e = wheel(el, { deltaY: -100, metaKey: true, clientX: 100, clientY: 100 }) + + expect(e.defaultPrevented).toBe(true) + expect(pendingRafCount()).toBe(0) + expect(store.getState().zoomLevel).toBe(1) + expect(store.getState().viewportOffset).toEqual({ x: 0, y: 0 }) + }) + + it('swallows wheel input while a dock-drag is active', () => { + const { store, el } = setupScene() + act(() => { + useDragStore.setState({ isDragging: true }) + }) + + const zoomWheel = wheel(el, { deltaY: -100, metaKey: true, clientX: 100, clientY: 100 }) + const panWheel = wheel(el, { deltaX: 20, deltaY: 30 }) + + expect(zoomWheel.defaultPrevented).toBe(true) + expect(panWheel.defaultPrevented).toBe(true) + expect(pendingRafCount()).toBe(0) + expect(store.getState().zoomLevel).toBe(1) + expect(store.getState().viewportOffset).toEqual({ x: 0, y: 0 }) + }) +}) + +// ============================================================================= +// 2. Right-click drag panning +// ============================================================================= + +describe('right-drag panning', () => { + it('pans the viewport by the cursor delta past the threshold; release ends the pan', () => { + const { store, el } = setupScene() + + rightDown(el, 100, 100) + moveTo(el, 150, 130) // 58px — far past the 4px threshold + expect(store.getState().viewportOffset).toEqual({ x: 50, y: 30 }) + + moveTo(el, 160, 120) // incremental: +10, -10 + expect(store.getState().viewportOffset).toEqual({ x: 60, y: 20 }) + + rightUp(el, 160, 120) + // Dragged → no context menu, no coast (only 2 slow samples is fine, but + // assert the pan state itself is torn down): + const after = { ...store.getState().viewportOffset } + moveTo(el, 400, 400) // mousemove after release must NOT pan + expect(store.getState().viewportOffset).toEqual(after) + }) + + it('sub-threshold movement still opens the context menu on release (viewport tracks the tiny delta 1:1)', () => { + const { store, el, expose } = setupScene() + + rightDown(el, 100, 100) + moveTo(el, 102, 101) // √5 ≈ 2.24px — below the 4px drag threshold + // BUG?: the threshold only gates the context menu, not the pan itself — + // the viewport already moved by the sub-threshold delta. Expected: a + // movement that's too small to count as a "drag" arguably shouldn't pan at + // all; actual: setViewportOffset runs on every mousemove while panning. + // Documented as current behavior (the menu math below compensates exactly). + expect(store.getState().viewportOffset).toEqual({ x: 2, y: 1 }) + + rightUp(el, 102, 101) + + // Not a drag → the canvas context menu opens at the release point... + const menu = expose.current!.canvasContextMenu + expect(menu).not.toBeNull() + expect(menu!.x).toBe(102) + expect(menu!.y).toBe(101) + // ...and the canvasPoint accounts for the (slightly moved) offset: + // ((102-2)/1, (101-1)/1) = the original press point. + expect(menu!.canvasPoint).toEqual({ x: 100, y: 100 }) + // The pan itself is torn down. + expect(document.body.classList.contains('canvas-interacting')).toBe(false) + }) + + it('does not open the context menu after a real drag', () => { + const { el, expose } = setupScene() + + rightDown(el, 100, 100) + moveTo(el, 200, 100) + rightUp(el, 200, 100) + + expect(expose.current!.canvasContextMenu).toBeNull() + }) +}) + +// ============================================================================= +// 3. Momentum / inertia +// ============================================================================= + +/** Fast right-drag along +x: three 30px samples 16ms apart. Leaves the store + * offset at (90, 0) and a coast rAF armed. */ +function flickRight(el: HTMLElement) { + rightDown(el, 100, 100) + fakeNow += 16 + moveTo(el, 130, 100) + fakeNow += 16 + moveTo(el, 160, 100) + fakeNow += 16 + moveTo(el, 190, 100) + fakeNow += 2 + rightUp(el, 190, 100) +} + +describe('momentum / inertia', () => { + it('coasts after a fast right-drag release and decays to a stop', () => { + const { store, el } = setupScene() + + flickRight(el) + // Release itself doesn't move the viewport; the coast loop is armed. + expect(store.getState().viewportOffset.x).toBe(90) + expect(pendingRafCount()).toBe(1) + + pumpFrame() + const afterOneFrame = store.getState().viewportOffset.x + expect(afterOneFrame).toBeGreaterThan(90) // still moving in the fling direction + expect(store.getState().viewportOffset.y).toBe(0) + + const frames = pumpUntilIdle() + const settled = store.getState().viewportOffset.x + expect(settled).toBeGreaterThan(afterOneFrame) // kept coasting... + expect(frames).toBeGreaterThan(2) // ...over multiple frames + expect(pendingRafCount()).toBe(0) // ...and stopped on its own + + // Dead after settling: more frames change nothing. + pumpFrame() + pumpFrame() + expect(store.getState().viewportOffset.x).toBe(settled) + }) + + it('a release after a pause (stale velocity samples) does NOT coast', () => { + const { store, el } = setupScene() + + rightDown(el, 100, 100) + fakeNow += 16 + moveTo(el, 130, 100) + fakeNow += 16 + moveTo(el, 160, 100) + fakeNow += 200 // hold still — samples are now older than the 100ms window + rightUp(el, 160, 100) + + expect(pendingRafCount()).toBe(0) + expect(store.getState().viewportOffset).toEqual({ x: 60, y: 0 }) + }) + + it('a slow / single-sample drag does NOT coast', () => { + const { el } = setupScene() + + rightDown(el, 100, 100) + fakeNow += 16 + moveTo(el, 150, 100) // one sample only + rightUp(el, 150, 100) + + expect(pendingRafCount()).toBe(0) + }) + + it('starting a NEW right-drag mid-coast kills the momentum immediately', () => { + const { store, el } = setupScene() + + flickRight(el) + pumpFrame() + pumpFrame() + expect(pendingRafCount()).toBe(1) // coast still alive + + rightDown(el, 500, 500) // new pan begins + expect(pendingRafCount()).toBe(0) // inertia rAF cancelled on the spot + + const frozen = { ...store.getState().viewportOffset } + pumpFrame() + pumpFrame() + expect(store.getState().viewportOffset).toEqual(frozen) // no fighting + + // The new drag still pans normally. + moveTo(el, 510, 505) + expect(store.getState().viewportOffset).toEqual({ x: frozen.x + 10, y: frozen.y + 5 }) + rightUp(el, 510, 505) + }) + + it('a zoom wheel mid-coast kills the momentum: the final state is purely zoom-anchored', () => { + const { store, el } = setupScene() + + flickRight(el) + pumpFrame() + pumpFrame() + expect(pendingRafCount()).toBe(1) + + // Capture the state at the moment the zoom starts; if inertia kept + // running, the final offset would deviate from the pure zoom anchor. + const atWheel = store.getState() + const cursor = { x: 400, y: 300 } + wheel(el, { deltaY: -100, metaKey: true, clientX: cursor.x, clientY: cursor.y }) + pumpUntilIdle() + + const after = store.getState() + expect(after.zoomLevel).toBeCloseTo(2.0, 6) + const expected = anchoredOffset(cursor, atWheel.zoomLevel, atWheel.viewportOffset, 2.0) + expect(after.viewportOffset.x).toBeCloseTo(expected.x, 6) + expect(after.viewportOffset.y).toBeCloseTo(expected.y, 6) + }) +}) + +// ============================================================================= +// 4. Lifecycle — unmount mid-animation +// ============================================================================= + +describe('lifecycle', () => { + it('unmounting mid-coast cancels the rAF: no further viewport updates, no errors', () => { + const errorSpy = vi.spyOn(console, 'error') + const { store, el } = setupScene() + + flickRight(el) + pumpFrame() + expect(pendingRafCount()).toBe(1) + + act(() => root.unmount()) + rootUnmounted = true + + expect(pendingRafCount()).toBe(0) // coast handle cancelled by the unmount cleanup + const frozen = { ...store.getState().viewportOffset } + pumpFrame() + pumpFrame() + expect(store.getState().viewportOffset).toEqual(frozen) + // No act()/unmounted-update warnings (ignore unrelated one-time deprecation + // notices, which would otherwise make this order-dependent). + const offending = errorSpy.mock.calls.filter( + (args) => String(args[0]).includes('act(') || String(args[0]).includes('unmounted'), + ) + expect(offending).toEqual([]) + }) + + it('unmounting mid smooth-zoom cancels the rAF and freezes the zoom', () => { + const { store, el } = setupScene() + + wheel(el, { deltaY: -100, metaKey: true, clientX: 400, clientY: 300 }) + pumpFrame() + pumpFrame() + const midZoom = store.getState().zoomLevel + expect(midZoom).toBeGreaterThan(1) + expect(midZoom).toBeLessThan(2) + expect(pendingRafCount()).toBe(1) + + act(() => root.unmount()) + rootUnmounted = true + + expect(pendingRafCount()).toBe(0) + pumpFrame() + expect(store.getState().zoomLevel).toBe(midZoom) + }) +}) + +// ============================================================================= +// 5. Context menu +// ============================================================================= + +describe('context menu', () => { + it('right-click without drag converts the click point to canvas space under non-default zoom/offset', () => { + const { el, expose } = setupScene({ zoom: 2, offset: { x: 50, y: -20 } }) + + rightDown(el, 300, 200) + rightUp(el, 300, 200) // no movement + + const menu = expose.current!.canvasContextMenu + expect(menu).not.toBeNull() + // Screen coords pass through untouched. + expect(menu!.x).toBe(300) + expect(menu!.y).toBe(200) + // canvasPoint = (view - offset) / zoom = ((300-50)/2, (200+20)/2). + expect(menu!.canvasPoint).toEqual({ x: 125, y: 110 }) + + act(() => expose.current!.closeCanvasContextMenu()) + expect(expose.current!.canvasContextMenu).toBeNull() + }) + + it('right-click on a node does NOT open the canvas background menu', () => { + const { expose } = setupScene() + const node = container.querySelector('[data-testid="node"]')! + + rightDown(node, 50, 50) + rightUp(node, 50, 50) + + expect(expose.current!.canvasContextMenu).toBeNull() + }) + + it('suppresses the native contextmenu event', () => { + const { el } = setupScene() + let prevented = false + act(() => { + const e = new MouseEvent('contextmenu', { bubbles: true, cancelable: true }) + el.dispatchEvent(e) + prevented = e.defaultPrevented + }) + expect(prevented).toBe(true) + }) +}) + +// ----------------------------------------------------------------------------- +// Sanity for the harness itself: the menu state must be a fresh snapshot, so a +// stale `expose.current` would invalidate the assertions above. +// ----------------------------------------------------------------------------- + +it('expose ref tracks re-renders (harness sanity)', () => { + const { expose } = setupScene() + expect(expose.current).not.toBeNull() + expect(typeof expose.current!.handleWheel).toBe('function') + expect(expose.current!.canvasContextMenu).toBeNull() +}) diff --git a/src/renderer/lib/activePanel.test.ts b/src/renderer/lib/activePanel.test.ts index 52302956..7c3c43d5 100644 --- a/src/renderer/lib/activePanel.test.ts +++ b/src/renderer/lib/activePanel.test.ts @@ -86,10 +86,13 @@ describe('placementForActivePanel', () => { expect(placementForActivePanel()).toBeUndefined() }) - it('uses the default canvas placement when a CANVAS is active, despite its center-zone dock location', () => { + it('pins the placement to the active CANVAS, despite its center-zone dock location', () => { // The canvas panel is itself docked in the center zone, so it has a dock // location — but a create while it's active must land ON the canvas, not as - // a sibling tab. The canvas ops registry distinguishes it. + // a sibling tab. The canvas ops registry distinguishes it. The placement is + // pinned to THAT canvas explicitly: the unpinned default would route to the + // workspace's primary canvas, which is the wrong (hidden) one whenever a + // secondary canvas tab is the active one. const canvasId = 'canvas-1' const dock = getOrCreateWorkspaceDockStore(wsId) dock.getState().dockPanel(canvasId, 'center') @@ -97,7 +100,7 @@ describe('placementForActivePanel', () => { registerCanvasOps(canvasId, createCanvasOps(getOrCreateCanvasStoreForPanel(canvasId))) setActivePanel(canvasId) - expect(placementForActivePanel()).toBeUndefined() + expect(placementForActivePanel()).toEqual({ target: 'canvas', canvasPanelId: canvasId }) unregisterCanvasOps(canvasId) releaseCanvasStoreForPanel(canvasId) diff --git a/src/renderer/lib/panelTransfer.receive.test.ts b/src/renderer/lib/panelTransfer.receive.test.ts new file mode 100644 index 00000000..e052f8f6 --- /dev/null +++ b/src/renderer/lib/panelTransfer.receive.test.ts @@ -0,0 +1,599 @@ +// Receive-side tests for panelTransfer: hydrateReceivedPanel / +// depositPanelTerminalTransfer / hydrateCanvasState driven through the REAL +// terminalRegistry (xterm/IPC collaborators stubbed, same harness shape as +// terminalRegistry.test.ts). Each test simulates the real sender→receiver +// sequence the window shells run: +// +// sender: terminalRegistry.getOrCreate (live PTY) → createTransferSnapshot +// → terminalRegistry.release (panelTeardown — PTY stays alive) +// receiver: hydrateReceivedPanel + ensurePanelsInAppStore (the PANEL_RECEIVE +// preamble in DockWindowShell/App) → terminalRegistry.getOrCreate +// (the TerminalPanel mount) → attach (reconnect finalization) +// +// NOTE: sender and receiver share one process here (one registry / one canvas +// store map), so the test releases the sender entry BEFORE depositing the +// receiver hand-off — exactly the order the two real processes produce, and +// required because release() clears pendingTransfers for the panel id. + +// @vitest-environment jsdom + +import { describe, it, expect, vi, beforeEach } from 'vitest' + +// Hoisted so the vi.mock factories (which run while the import graph loads, +// before this module's body) can safely reference the shared harness state. +const { + events, + settingsState, + appState, + replayTerminalLog, + getNodeDockLayout, + terminalCreate, + panelTransferAck, + terminalKill, +} = vi.hoisted(() => ({ + // Shared event log for ordering-sensitive assertions during reconnect+attach. + events: [] as string[], + settingsState: { + terminalFontFamily: '', + terminalFontSize: 0, + terminalScrollback: 2000, + terminalCursorBlink: false, + terminalScrollSpeed: 1.0, + terminalContrast: 4.5, + terminalOptionIsMeta: true, + }, + // Functional appStore fake state: real mutable state behind a zustand-style + // setState so the REAL ensurePanelsInAppStore / applyCanvasChildPanels run + // unmodified and we can assert what actually landed in the receiving store. + appState: { workspaces: [] as any[], selectedWorkspaceId: '' }, + replayTerminalLog: vi.fn(async (_panelId: string) => undefined), + // Controllable node mini-dock layout (null = node has only its seed panel). + getNodeDockLayout: vi.fn<() => unknown>(() => null), + terminalCreate: vi.fn(async () => 'pty-fresh'), + panelTransferAck: vi.fn(async (_id: string) => undefined as undefined), + terminalKill: vi.fn(async (_id: string) => undefined), +})) + +vi.mock('@xterm/xterm', () => { + class FakeTerminal { + public writes: string[] = [] + public options: Record + public buffer = { active: { baseY: 0, cursorY: 0, viewportY: 0, getLine: () => undefined } } + public element: HTMLElement | undefined + public cols = 80 + public rows = 24 + constructor(options: Record = {}) { + this.options = options + } + // Activate addons so the fake SerializeAddon below can capture this + // terminal's writes (the real registry serializes via the addon). + loadAddon(addon: { activate?: (t: unknown) => void }): void { + addon.activate?.(this) + } + open(container: HTMLElement): void { + this.element = document.createElement('div') + const viewport = document.createElement('div') + viewport.className = 'xterm-viewport' + this.element.appendChild(viewport) + container.appendChild(this.element) + events.push('open') + } + write(s: string): void { + this.writes.push(s) + events.push(`write:${s.slice(0, 24)}`) + } + onData(): { dispose: () => void } { return { dispose: () => {} } } + onResize(): { dispose: () => void } { return { dispose: () => {} } } + onTitleChange(): { dispose: () => void } { return { dispose: () => {} } } + hasSelection(): boolean { return false } + attachCustomKeyEventHandler(): void { /* no-op */ } + registerLinkProvider(): { dispose: () => void } { return { dispose: () => {} } } + refresh(): void { /* no-op */ } + focus(): void { /* no-op */ } + scrollToLine(line: number): void { + this.buffer.active.viewportY = Math.max(0, Math.min(line, this.buffer.active.baseY)) + } + scrollToBottom(): void { this.buffer.active.viewportY = this.buffer.active.baseY } + resize(c: number, r: number): void { this.cols = c; this.rows = r } + dispose(): void { /* no-op */ } + } + return { Terminal: FakeTerminal } +}) + +vi.mock('@xterm/addon-fit', () => ({ + FitAddon: class { proposeDimensions() { return { cols: 80, rows: 24 } } fit() {} dispose() {} }, +})) +vi.mock('@xterm/addon-webgl', () => ({ + WebglAddon: class { onContextLoss() {} dispose() {} }, +})) +vi.mock('@xterm/addon-search', () => ({ + SearchAddon: class { findNext() { return false } findPrevious() { return false } clearDecorations() {} }, +})) +// Deterministic serialize: a terminal's "scrollback" is everything written to it. +vi.mock('@xterm/addon-serialize', () => ({ + SerializeAddon: class { + private t: { writes: string[] } | undefined + activate(t: unknown): void { this.t = t as { writes: string[] } } + serialize(): string { return this.t ? this.t.writes.join('') : '' } + dispose(): void { /* no-op */ } + }, +})) + +vi.mock('../stores/statusStore', () => ({ + useStatusStore: { getState: () => ({ registerTerminal: vi.fn(), unregisterTerminal: vi.fn() }) }, + setTerminalWorkspaceResolver: vi.fn(), +})) +vi.mock('../stores/settingsStore', () => ({ + useSettingsStore: { + getState: () => settingsState, + subscribe: () => () => {}, + }, +})) +vi.mock('../stores/appStore', () => ({ + awaitWorkspaceSync: async () => {}, + useAppStore: { + getState: () => appState, + setState: (updater: unknown) => { + const partial = + typeof updater === 'function' + ? (updater as (s: typeof appState) => Partial)(appState) + : (updater as Partial) + Object.assign(appState, partial) + }, + }, +})) + +vi.mock('./workspace/session', () => ({ + replayTerminalLog: (...a: unknown[]) => replayTerminalLog(...(a as [string])), +})) +vi.mock('./terminal/terminalUrlOpen', () => ({ openTerminalUrl: () => {} })) +vi.mock('./themeManager', () => ({ + getActiveTheme: () => ({ terminal: {} }), + subscribeTheme: () => () => {}, +})) +vi.mock('./logger', () => ({ default: { warn: () => {}, info: () => {}, error: () => {}, debug: () => {} } })) + +vi.mock('./workspace/canvasAccess', () => ({ + getNodeDockLayout: () => getNodeDockLayout(), +})) + +import { + createTransferSnapshot, + depositPanelTerminalTransfer, + hydrateReceivedPanel, +} from './panelTransfer' +import { terminalRegistry } from './terminal/terminalRegistry' +import { terminalRestoreData } from './terminal/terminalRestoreData' +import { getOrCreateCanvasStoreForPanel } from '../stores/canvasStore' +import { ensurePanelsInAppStore } from './canvas/applyCanvasChildPanels' +import type { PanelState, PanelTransferSnapshot, PanelLocation } from '../../shared/types' + +beforeEach(() => { + events.length = 0 + appState.workspaces = [] + appState.selectedWorkspaceId = '' + terminalRestoreData.clear() + terminalCreate.mockClear() + terminalCreate.mockImplementation(async () => 'pty-fresh') + panelTransferAck.mockClear() + terminalKill.mockClear() + replayTerminalLog.mockClear() + getNodeDockLayout.mockReturnValue(null) + Object.defineProperty(window, 'electronAPI', { + configurable: true, + writable: true, + value: { + terminalCreate, + terminalWrite: vi.fn(), + terminalResize: vi.fn(), + terminalKill, + onTerminalData: vi.fn(() => () => {}), + onTerminalExit: vi.fn(() => () => {}), + shellRegisterTerminal: vi.fn(async () => undefined), + shellUnregisterTerminal: vi.fn(async () => undefined), + settingsGet: vi.fn(async () => ''), + panelTransferAck, + }, + }) +}) + +const GEOM = { origin: { x: 0, y: 0 }, size: { width: 800, height: 600 } } +const DOCK_LOC: PanelLocation = { type: 'dock', zone: 'center', stackId: 's-1' } as PanelLocation +const CANVAS_LOC: PanelLocation = { type: 'canvas', canvasId: 'c-root', canvasNodeId: 'n-root' } as PanelLocation + +function termPanel(id: string): PanelState { + return { id, type: 'terminal', title: 'zsh', isDirty: false } as PanelState +} + +/** Sender side: spawn a live terminal entry and put content in its buffer. */ +async function spawnSenderTerminal(panelId: string, ptyId: string, content: string) { + terminalCreate.mockResolvedValueOnce(ptyId) + const entry = await terminalRegistry.getOrCreate(panelId, { workspaceId: 'ws-src' }) + entry.terminal.write(content) // simulate PTY output that landed in the buffer + return entry +} + +/** Receiver side: mount the terminal panel's DOM and let attach() finalize. */ +async function attachAndSettle(panelId: string): Promise { + const container = document.createElement('div') + Object.defineProperty(container, 'offsetWidth', { value: 800, configurable: true }) + Object.defineProperty(container, 'offsetHeight', { value: 600, configurable: true }) + document.body.appendChild(container) + terminalRegistry.attach(panelId, container) + await new Promise((resolve) => requestAnimationFrame(() => resolve())) + await new Promise((resolve) => requestAnimationFrame(() => resolve())) + return container +} + +function receiverPanels(wsId: string): Record | undefined { + return appState.workspaces.find((w) => w.id === wsId)?.panels +} + +// =========================================================================== +// 1. hydrateReceivedPanel — happy paths (full sender → receiver round trip) +// =========================================================================== +describe('hydrateReceivedPanel — happy path round trips', () => { + it('terminal: receiver reconnects to the live PTY and replays the captured scrollback', async () => { + const panel = termPanel('hrp-term') + await spawnSenderTerminal('hrp-term', 'pty-live-1', 'PROMPT$ npm run dev\n') + + const snapshot = createTransferSnapshot(panel, DOCK_LOC, GEOM, { workspaceRootPath: '/repo' }) + expect(snapshot.terminalPtyId).toBe('pty-live-1') + expect(snapshot.terminalScrollback).toBe('PROMPT$ npm run dev\n') + + // Sender teardown (panelTeardown.ts): release WITHOUT killing the PTY. + terminalRegistry.release('hrp-term') + expect(terminalKill).not.toHaveBeenCalled() + + // Receiver PANEL_RECEIVE preamble (DockWindowShell.onPanelReceive order). + hydrateReceivedPanel('ws-recv', snapshot) + ensurePanelsInAppStore('ws-recv', { [snapshot.panel.id]: snapshot.panel }, snapshot.rootPath, snapshot.worktrees) + + // TerminalPanel mounts: must take the reconnect path, NOT spawn fresh. + terminalCreate.mockClear() + const entry = await terminalRegistry.getOrCreate('hrp-term', { workspaceId: 'ws-recv' }) + expect(entry.ptyId).toBe('pty-live-1') + expect(terminalCreate).not.toHaveBeenCalled() + + // attach() finalizes the reconnect: scrollback replayed, transfer ACKed. + await attachAndSettle('hrp-term') + const writes = (entry.terminal as unknown as { writes: string[] }).writes.join('') + expect(writes).toContain('PROMPT$ npm run dev') + expect(panelTransferAck).toHaveBeenCalledWith('pty-live-1') + + // And the receiving window's appStore stub holds the panel + rootPath. + expect(receiverPanels('ws-recv')?.['hrp-term']).toMatchObject({ id: 'hrp-term', type: 'terminal' }) + expect(appState.workspaces.find((w) => w.id === 'ws-recv')?.rootPath).toBe('/repo') + + terminalRegistry.dispose('hrp-term') + }) + + it('editor: unsaved content rides the snapshot into the receiving appStore; no terminal state is armed', () => { + const panel: PanelState = { + id: 'hrp-editor', + type: 'editor', + title: 'Untitled', + isDirty: true, + unsavedContent: 'const x = 1', + } as PanelState + + const snapshot = createTransferSnapshot(panel, DOCK_LOC, GEOM) + expect(snapshot.editorState?.unsavedContent).toBe('const x = 1') + + expect(() => hydrateReceivedPanel('ws-recv', snapshot)).not.toThrow() + ensurePanelsInAppStore('ws-recv', { [snapshot.panel.id]: snapshot.panel }) + + // The unsaved buffer is observable on the receiver. + expect(receiverPanels('ws-recv')?.['hrp-editor']).toMatchObject({ + isDirty: true, + unsavedContent: 'const x = 1', + }) + // The preamble must not touch terminal machinery for a non-terminal panel. + expect(terminalRestoreData.size).toBe(0) + }) + + it('canvas: children, viewport and live child PTYs hydrate before the canvas mounts', async () => { + const canvasPanel: PanelState = { id: 'hrp-canvas', type: 'canvas', title: 'C', isDirty: false } as PanelState + const store = getOrCreateCanvasStoreForPanel('hrp-canvas') + const nodeId = store + .getState() + .addNode('hrp-canvas-child', 'terminal', { x: 40, y: 20 }, { width: 320, height: 240 }) + store.setState({ zoomLevel: 1.25, viewportOffset: { x: 7, y: 9 } }) + await spawnSenderTerminal('hrp-canvas-child', 'pty-child-7', 'child output\n') + + const childPanel = termPanel('hrp-canvas-child') + const snapshot = createTransferSnapshot(canvasPanel, CANVAS_LOC, GEOM, { + resolveChildPanel: (id) => (id === 'hrp-canvas-child' ? childPanel : undefined), + }) + expect(snapshot.canvasState?.childTerminals?.['hrp-canvas-child']).toEqual({ + ptyId: 'pty-child-7', + scrollback: 'child output\n', + }) + + // Sender teardown: child released (PTY lives on), then simulate the fresh + // receiving window's empty per-panel store (same process shares the map). + terminalRegistry.release('hrp-canvas-child') + store.setState({ nodes: {}, zoomLevel: 1, viewportOffset: { x: 0, y: 0 } }) + + hydrateReceivedPanel('ws-recv', snapshot) + ensurePanelsInAppStore('ws-recv', { [snapshot.panel.id]: snapshot.panel }) + + // Layout + viewport restored into the receiver's canvas store. + const hydrated = getOrCreateCanvasStoreForPanel('hrp-canvas').getState() + expect(hydrated.nodes[nodeId]?.panelId).toBe('hrp-canvas-child') + expect(hydrated.zoomLevel).toBe(1.25) + expect(hydrated.viewportOffset).toEqual({ x: 7, y: 9 }) + + // Child PanelState seeded so the node renders a real terminal, not a stub. + expect(receiverPanels('ws-recv')?.['hrp-canvas-child']).toEqual(childPanel) + + // Child terminal mount reconnects to the live PTY instead of spawning. + terminalCreate.mockClear() + const entry = await terminalRegistry.getOrCreate('hrp-canvas-child', { workspaceId: 'ws-recv' }) + expect(entry.ptyId).toBe('pty-child-7') + expect(terminalCreate).not.toHaveBeenCalled() + + terminalRegistry.dispose('hrp-canvas-child') + }) +}) + +// =========================================================================== +// 2. depositPanelTerminalTransfer — the PTY hand-off +// =========================================================================== +describe('depositPanelTerminalTransfer — PTY hand-off', () => { + it('arms a live transfer so the receiving terminal mount reconnects, not respawns', async () => { + const snapshot: PanelTransferSnapshot = { + panel: termPanel('dep-live'), + geometry: GEOM, + sourceLocation: DOCK_LOC, + terminalPtyId: 'pty-armed', + terminalScrollback: 'history', + } + + depositPanelTerminalTransfer(snapshot) + // Live transfer must not be confused with cold session restore. + expect(terminalRestoreData.has('dep-live')).toBe(false) + + const entry = await terminalRegistry.getOrCreate('dep-live', { workspaceId: 'ws-recv' }) + expect(entry.ptyId).toBe('pty-armed') + expect(terminalCreate).not.toHaveBeenCalled() + + await attachAndSettle('dep-live') + expect((entry.terminal as unknown as { writes: string[] }).writes.join('')).toContain('history') + expect(panelTransferAck).toHaveBeenCalledWith('pty-armed') + + terminalRegistry.dispose('dep-live') + }) + + it('session restore (no live PTY): arms scrollback replay by the stable panelId and spawns fresh', async () => { + const snapshot: PanelTransferSnapshot = { + panel: termPanel('dep-replay'), + geometry: GEOM, + sourceLocation: DOCK_LOC, + terminalReplayPtyId: 'pty-dead-old', + } + + depositPanelTerminalTransfer(snapshot) + expect(terminalRestoreData.get('dep-replay')).toEqual({ replayFromId: 'dep-replay' }) + + terminalCreate.mockResolvedValueOnce('pty-respawned') + const entry = await terminalRegistry.getOrCreate('dep-replay', { workspaceId: 'ws-recv' }) + expect(entry.ptyId).toBe('pty-respawned') // fresh spawn, not a reconnect + expect(replayTerminalLog).toHaveBeenCalledWith('dep-replay') + + terminalRegistry.dispose('dep-replay') + }) + + it('ignores a replay hint on a non-terminal panel', () => { + const snapshot: PanelTransferSnapshot = { + panel: { id: 'dep-not-term', type: 'editor', title: 'E', isDirty: false } as PanelState, + geometry: GEOM, + sourceLocation: DOCK_LOC, + terminalReplayPtyId: 'pty-dead-old', + } + depositPanelTerminalTransfer(snapshot) + expect(terminalRestoreData.has('dep-not-term')).toBe(false) + }) +}) + +// =========================================================================== +// 3. Edge: registry entry released mid-drag (snapshot finds no live terminal) +// =========================================================================== +describe('snapshot of a terminal whose registry entry was released mid-drag', () => { + it('carries no PTY; the receiver hydrates without throwing and spawns fresh on mount', async () => { + const panel = termPanel('edge-released') + await spawnSenderTerminal('edge-released', 'pty-vanishing', 'gone\n') + // The entry is released BEFORE the snapshot is taken (drag raced teardown). + terminalRegistry.release('edge-released') + + const snapshot = createTransferSnapshot(panel, DOCK_LOC, GEOM) + expect(snapshot.terminalPtyId).toBeUndefined() + expect(snapshot.terminalScrollback).toBeUndefined() + + expect(() => hydrateReceivedPanel('ws-recv', snapshot)).not.toThrow() + // No hand-off and no replay were armed. + expect(terminalRestoreData.size).toBe(0) + + // Mount: a FRESH PTY is spawned — the fresh-spawn signal is a terminalCreate + // IPC call and a brand-new ptyId (not the stale 'pty-vanishing'). + terminalCreate.mockClear() + terminalCreate.mockResolvedValueOnce('pty-fresh-9') + const entry = await terminalRegistry.getOrCreate('edge-released', { workspaceId: 'ws-recv' }) + expect(terminalCreate).toHaveBeenCalledTimes(1) + expect(entry.ptyId).toBe('pty-fresh-9') + // Nothing tries to ACK a transfer that never existed. + expect(panelTransferAck).not.toHaveBeenCalled() + + terminalRegistry.dispose('edge-released') + }) +}) + +// =========================================================================== +// 4. Edge: canvas child panel record vanished between snapshot and receive +// =========================================================================== +describe('canvas snapshot with a vanished child panel record', () => { + it('hydrates without throwing and KEEPS the orphan node (renders as a generic stub)', () => { + const canvasPanel: PanelState = { id: 'edge-ghost-canvas', type: 'canvas', title: 'C', isDirty: false } as PanelState + const store = getOrCreateCanvasStoreForPanel('edge-ghost-canvas') + const nodeId = store + .getState() + .addNode('ghost-child', 'terminal', { x: 0, y: 0 }, { width: 300, height: 200 }) + + // The child's PanelState vanished before capture: resolveChildPanel finds + // nothing and there is no registry entry either. + const snapshot = createTransferSnapshot(canvasPanel, CANVAS_LOC, GEOM, { + resolveChildPanel: () => undefined, + }) + expect(snapshot.canvasState?.childPanels).toEqual({}) + expect(snapshot.canvasState?.nodes[nodeId]?.panelId).toBe('ghost-child') + + store.setState({ nodes: {} }) // fresh receiver store + expect(() => hydrateReceivedPanel('ws-recv', snapshot)).not.toThrow() + + // Documented behavior: the orphan node is KEPT (loadWorkspaceCanvas applies + // the captured nodes verbatim; nothing prunes nodes whose panelId has no + // PanelState). The receiver renders it via resolvePanel's generic "Panel" + // stub fallback — it is NOT dropped. + const hydrated = getOrCreateCanvasStoreForPanel('edge-ghost-canvas').getState() + expect(hydrated.nodes[nodeId]?.panelId).toBe('ghost-child') + // ...while the appStore receives no record for the ghost child. + expect(receiverPanels('ws-recv')?.['ghost-child']).toBeUndefined() + // And no terminal hand-off was armed for it. + expect(terminalRestoreData.size).toBe(0) + }) +}) + +// =========================================================================== +// 5. Edge: second PANEL_RECEIVE deposit before the first panel mounts +// =========================================================================== +describe('two deposits race before the panel mounts', () => { + it('same panel id: the SECOND deposit clobbers the first; the first PTY is orphaned', async () => { + const first: PanelTransferSnapshot = { + panel: termPanel('race-panel'), + geometry: GEOM, + sourceLocation: DOCK_LOC, + terminalPtyId: 'pty-first', + terminalScrollback: 'FIRST', + } + const second: PanelTransferSnapshot = { + panel: termPanel('race-panel'), + geometry: GEOM, + sourceLocation: DOCK_LOC, + terminalPtyId: 'pty-second', + terminalScrollback: 'SECOND', + } + + hydrateReceivedPanel('ws-recv', first) + hydrateReceivedPanel('ws-recv', second) // arrives before the panel mounts + + const entry = await terminalRegistry.getOrCreate('race-panel', { workspaceId: 'ws-recv' }) + await attachAndSettle('race-panel') + + // Actual behavior: pendingTransfers is keyed by panelId, so the second + // set() overwrites the first — the mount reconnects to pty-second only. + expect(entry.ptyId).toBe('pty-second') + expect((entry.terminal as unknown as { writes: string[] }).writes.join('')).toContain('SECOND') + expect(panelTransferAck).toHaveBeenCalledTimes(1) + expect(panelTransferAck).toHaveBeenCalledWith('pty-second') + + // BUG?: the first hand-off is silently lost. 'pty-first' is never + // reconnected, never ACKed (main keeps holding its buffered output for a + // flush that never comes) and never killed — the first transfer's PTY + // process leaks in the main process until app shutdown. + expect(panelTransferAck).not.toHaveBeenCalledWith('pty-first') + expect(terminalKill).not.toHaveBeenCalled() + const writes = (entry.terminal as unknown as { writes: string[] }).writes.join('') + expect(writes).not.toContain('FIRST') + + terminalRegistry.dispose('race-panel') + }) + + it('different panel ids: deposits do not interfere — each panel reconnects to its own PTY', async () => { + hydrateReceivedPanel('ws-recv', { + panel: termPanel('race-a'), + geometry: GEOM, + sourceLocation: DOCK_LOC, + terminalPtyId: 'pty-a', + }) + hydrateReceivedPanel('ws-recv', { + panel: termPanel('race-b'), + geometry: GEOM, + sourceLocation: DOCK_LOC, + terminalPtyId: 'pty-b', + }) + + const a = await terminalRegistry.getOrCreate('race-a', { workspaceId: 'ws-recv' }) + const b = await terminalRegistry.getOrCreate('race-b', { workspaceId: 'ws-recv' }) + expect(a.ptyId).toBe('pty-a') + expect(b.ptyId).toBe('pty-b') + expect(terminalCreate).not.toHaveBeenCalled() + + terminalRegistry.dispose('race-a') + terminalRegistry.dispose('race-b') + }) +}) + +// =========================================================================== +// 6. Unhappy: malformed / partial snapshots +// =========================================================================== +describe('malformed or partial snapshots hydrate without throwing', () => { + it('editor snapshot missing editorState', () => { + const snapshot: PanelTransferSnapshot = { + panel: { id: 'mal-editor', type: 'editor', title: 'E', isDirty: false } as PanelState, + geometry: GEOM, + sourceLocation: DOCK_LOC, + // no editorState at all + } + expect(() => hydrateReceivedPanel('ws-recv', snapshot)).not.toThrow() + }) + + it('terminal snapshot with neither a live ptyId nor a replay hint arms nothing', () => { + const snapshot: PanelTransferSnapshot = { + panel: termPanel('mal-term'), + geometry: GEOM, + sourceLocation: DOCK_LOC, + } + expect(() => hydrateReceivedPanel('ws-recv', snapshot)).not.toThrow() + expect(terminalRestoreData.size).toBe(0) + }) + + it('canvas snapshot with undefined childPanels and childTerminals still loads the layout', () => { + const snapshot: PanelTransferSnapshot = { + panel: { id: 'mal-canvas', type: 'canvas', title: 'C', isDirty: false } as PanelState, + geometry: GEOM, + sourceLocation: CANVAS_LOC, + canvasState: { + nodes: {}, + viewportOffset: { x: 3, y: 4 }, + zoomLevel: 2, + // A stale sender (or hand-edited session file) can omit these despite + // the type marking childPanels required. + childPanels: undefined, + childTerminals: undefined, + } as unknown as NonNullable, + } + + expect(() => hydrateReceivedPanel('ws-recv', snapshot)).not.toThrow() + const hydrated = getOrCreateCanvasStoreForPanel('mal-canvas').getState() + expect(hydrated.viewportOffset).toEqual({ x: 3, y: 4 }) + expect(hydrated.zoomLevel).toBe(2) + // Nothing seeded, nothing armed. + expect(receiverPanels('ws-recv')).toBeUndefined() + expect(terminalRestoreData.size).toBe(0) + }) + + it('canvas panel WITHOUT canvasState skips hydration entirely (no throw, store untouched)', () => { + const store = getOrCreateCanvasStoreForPanel('mal-canvas-empty') + store.getState().addNode('pre-existing', 'terminal', { x: 0, y: 0 }, { width: 100, height: 100 }) + const before = store.getState().nodes + + const snapshot: PanelTransferSnapshot = { + panel: { id: 'mal-canvas-empty', type: 'canvas', title: 'C', isDirty: false } as PanelState, + geometry: GEOM, + sourceLocation: CANVAS_LOC, + // canvasState missing — e.g. a sender that crashed mid-capture. + } + expect(() => hydrateReceivedPanel('ws-recv', snapshot)).not.toThrow() + expect(store.getState().nodes).toBe(before) + }) +}) diff --git a/src/renderer/lib/terminal/terminalLifecycle.test.ts b/src/renderer/lib/terminal/terminalLifecycle.test.ts new file mode 100644 index 00000000..57569094 --- /dev/null +++ b/src/renderer/lib/terminal/terminalLifecycle.test.ts @@ -0,0 +1,593 @@ +// Behavioral tests for terminalLifecycle — the renderer-side spawn / adopt / +// release / dispose state machine behind every terminal panel. These drive the +// real module (real registryState maps, real terminalSettings/terminalInput) +// through its public functions, with the heavy collaborators (xterm, IPC +// bridge, zustand stores, session module) stubbed the same way +// terminalRegistry.test.ts does. + +// @vitest-environment jsdom + +import { describe, it, expect, vi, beforeAll, beforeEach } from 'vitest' + +// --------------------------------------------------------------------------- +// Fakes / shared spies. Mock factories run lazily (on the first dynamic import +// in beforeAll), so referencing these module-level consts is safe. +// --------------------------------------------------------------------------- + +interface FakeTerminalShape { + writes: string[] + disposeCount: number + options: Record + cols: number + rows: number +} +const terminalInstances: FakeTerminalShape[] = [] + +vi.mock('@xterm/xterm', () => { + class FakeTerminal { + public writes: string[] = [] + public disposeCount = 0 + public options: Record + public buffer = { active: { baseY: 0, cursorY: 0, viewportY: 0, getLine: () => undefined } } + public element: HTMLElement | undefined + public cols = 80 + public rows = 24 + constructor(options: Record = {}) { + this.options = options + terminalInstances.push(this as unknown as FakeTerminalShape) + } + loadAddon(): void {} + open(container: HTMLElement): void { + this.element = document.createElement('div') + container.appendChild(this.element) + } + write(s: string): void { this.writes.push(s) } + onData(): { dispose: () => void } { return { dispose: () => {} } } + onResize(): { dispose: () => void } { return { dispose: () => {} } } + onTitleChange(): { dispose: () => void } { return { dispose: () => {} } } + hasSelection(): boolean { return false } + attachCustomKeyEventHandler(): void {} + registerLinkProvider(): { dispose: () => void } { return { dispose: () => {} } } + refresh(): void {} + focus(): void {} + scrollToBottom(): void {} + resize(c: number, r: number): void { this.cols = c; this.rows = r } + dispose(): void { this.disposeCount++ } + } + return { Terminal: FakeTerminal } +}) + +vi.mock('@xterm/addon-fit', () => ({ + FitAddon: class { proposeDimensions() { return { cols: 80, rows: 24 } } fit() {} dispose() {} }, +})) +vi.mock('@xterm/addon-webgl', () => ({ + WebglAddon: class { onContextLoss() {} dispose() {} }, +})) +vi.mock('@xterm/addon-search', () => ({ + SearchAddon: class { findNext() { return false } findPrevious() { return false } clearDecorations() {} }, +})) +vi.mock('@xterm/addon-serialize', () => ({ + SerializeAddon: class { serialize() { return '' } dispose() {} }, +})) +vi.mock('@xterm/addon-web-links', () => ({ + WebLinksAddon: class { dispose() {} }, +})) + +const statusRegisterTerminal = vi.fn() +const statusUnregisterTerminal = vi.fn() +vi.mock('../../stores/statusStore', () => ({ + useStatusStore: { + getState: () => ({ + registerTerminal: statusRegisterTerminal, + unregisterTerminal: statusUnregisterTerminal, + workspaces: {}, + }), + }, + setTerminalWorkspaceResolver: vi.fn(), +})) +vi.mock('../../stores/settingsStore', () => ({ + useSettingsStore: { + getState: () => ({ + terminalFontFamily: '', + terminalFontSize: 0, + terminalScrollback: 2000, + terminalCursorBlink: false, + terminalScrollSpeed: 1.0, + terminalContrast: 4.5, + terminalOptionIsMeta: true, + }), + subscribe: () => () => {}, + }, +})) +vi.mock('../../stores/appStore', () => ({ + awaitWorkspaceSync: async () => {}, + useAppStore: { + getState: () => ({ workspaces: [], updatePanelTitleFromAgent: vi.fn() }), + }, +})) +const replayTerminalLog = vi.fn(async () => {}) +vi.mock('../workspace/session', () => ({ + get replayTerminalLog() { return replayTerminalLog }, + terminalRestoreData: new Map(), +})) +vi.mock('./terminalUrlOpen', () => ({ openTerminalUrl: () => {} })) +vi.mock('./terminalFileLinkProvider', () => ({ + createFileLinkProvider: () => ({ provideLinks: (_y: number, cb: (l?: unknown[]) => void) => cb(undefined) }), + resolveLinkRoot: () => undefined, +})) +vi.mock('../agent/agentScreenDetector', () => ({ + noteAgentTitle: vi.fn(), + noteAgentSpinnerByte: vi.fn(), + noteAgentPresence: vi.fn(), + forgetAgentTracker: vi.fn(), +})) +vi.mock('../themeManager', () => ({ + getActiveTheme: () => ({ terminal: {} }), + subscribeTheme: () => () => {}, +})) +vi.mock('../logger', () => ({ + default: { warn: () => {}, info: () => {}, error: () => {}, debug: () => {} }, +})) + +// --------------------------------------------------------------------------- +// IPC bridge spies. Data/exit listeners are captured so tests can inject +// synthetic PTY traffic; the returned disposers are spies so teardown can be +// asserted. +// --------------------------------------------------------------------------- + +let ptyCounter = 0 +const terminalCreate = vi.fn(async () => `pty-${++ptyCounter}`) +const terminalWrite = vi.fn() +const terminalResize = vi.fn() +const terminalKill = vi.fn(async () => undefined) +const shellRegisterTerminal = vi.fn(async () => undefined) +const shellUnregisterTerminal = vi.fn(async () => undefined) +const panelTransferAck = vi.fn(async () => undefined) +const settingsGet = vi.fn(async () => '') + +const dataListeners: Array<(id: string, data: string) => void> = [] +const exitListeners: Array<(id: string, code: number) => void> = [] +const dataDisposers: Array> = [] +const captureDataListener = (cb: (id: string, data: string) => void) => { + dataListeners.push(cb) + const disposer = vi.fn() + dataDisposers.push(disposer) + return disposer +} +const captureExitListener = (cb: (id: string, code: number) => void) => { + exitListeners.push(cb) + return () => {} +} +const onTerminalData = vi.fn(captureDataListener) +const onTerminalExit = vi.fn(captureExitListener) + +function fireData(ptyId: string, data: string): void { + for (const cb of [...dataListeners]) cb(ptyId, data) +} +function fireExit(ptyId: string, code: number): void { + for (const cb of [...exitListeners]) cb(ptyId, code) +} + +// --------------------------------------------------------------------------- +// Module handles — loaded dynamically AFTER mocks and consts exist. +// --------------------------------------------------------------------------- + +let LC: typeof import('./terminalLifecycle') +let RS: typeof import('./registryState') +let restoreData: Map + +beforeAll(async () => { + LC = await import('./terminalLifecycle') + RS = await import('./registryState') + restoreData = (await import('./terminalRestoreData')).terminalRestoreData +}) + +beforeEach(() => { + Object.defineProperty(window, 'electronAPI', { + configurable: true, + writable: true, + value: { + terminalCreate, terminalWrite, terminalResize, terminalKill, + onTerminalData, onTerminalExit, + shellRegisterTerminal, shellUnregisterTerminal, + settingsGet, panelTransferAck, + }, + }) + // Drain module-singleton state left by a prior test. + for (const panelId of [...RS.registry.keys()]) LC.dispose(panelId) + RS.pendingTransfers.clear() + RS.failures.clear() + restoreData.clear() + terminalInstances.length = 0 + dataListeners.length = 0 + exitListeners.length = 0 + dataDisposers.length = 0 + terminalCreate.mockClear() + terminalCreate.mockImplementation(async () => `pty-${++ptyCounter}`) + terminalWrite.mockClear() + terminalResize.mockClear() + terminalKill.mockClear() + terminalKill.mockImplementation(async () => undefined) + shellRegisterTerminal.mockClear() + shellRegisterTerminal.mockImplementation(async () => undefined) + shellUnregisterTerminal.mockClear() + shellUnregisterTerminal.mockImplementation(async () => undefined) + panelTransferAck.mockClear() + panelTransferAck.mockImplementation(async () => undefined) + settingsGet.mockClear() + settingsGet.mockImplementation(async () => '') + // restoreMocks: true wipes vi.fn implementations after each test — reinstall + // the listener-capturing implementations. + onTerminalData.mockClear() + onTerminalData.mockImplementation(captureDataListener) + onTerminalExit.mockClear() + onTerminalExit.mockImplementation(captureExitListener) + statusRegisterTerminal.mockClear() + statusUnregisterTerminal.mockClear() + replayTerminalLog.mockClear() + replayTerminalLog.mockImplementation(async () => {}) +}) + +// =========================================================================== +// Spawn happy path +// =========================================================================== +describe('spawn → wire → dispose happy path', () => { + it('spawns one PTY, registers identity, routes data, and dispose tears it all down exactly once', async () => { + terminalCreate.mockResolvedValueOnce('pty-happy') + + const entry = await LC.getOrCreate('panel-happy', { workspaceId: 'ws-1' }) + + // One PTY spawn, standard 80x24 defaults (real fit happens on attach). + expect(terminalCreate).toHaveBeenCalledTimes(1) + expect(terminalCreate).toHaveBeenCalledWith( + expect.objectContaining({ cols: 80, rows: 24, workspaceId: 'ws-1' }), + ) + + // Registry + identity bimap populated, entry alive. + expect(RS.has('panel-happy')).toBe(true) + expect(entry.ptyId).toBe('pty-happy') + expect(RS.ptyIdForPanel('panel-happy')).toBe('pty-happy') + expect(RS.panelIdForPty('pty-happy')).toBe('panel-happy') + expect(entry.alive).toBe(true) + + // Listener wiring: shell monitor + status store registered. + expect(shellRegisterTerminal).toHaveBeenCalledWith('pty-happy') + expect(statusRegisterTerminal).toHaveBeenCalledWith('pty-happy', 'ws-1') + + // PTY data is routed into THIS xterm only when the ptyId matches. + const fake = terminalInstances[0] + fireData('pty-happy', 'hello from pty') + fireData('pty-other', 'not for us') + expect(fake.writes).toContain('hello from pty') + expect(fake.writes).not.toContain('not for us') + + // Dispose: kills the PTY, unregisters everywhere, removes the entry, + // disposes the xterm exactly once, and removes the IPC data listener. + LC.dispose('panel-happy') + expect(terminalKill).toHaveBeenCalledTimes(1) + expect(terminalKill).toHaveBeenCalledWith('pty-happy') + expect(shellUnregisterTerminal).toHaveBeenCalledWith('pty-happy') + expect(statusUnregisterTerminal).toHaveBeenCalledWith('pty-happy') + expect(RS.has('panel-happy')).toBe(false) + expect(RS.panelIdForPty('pty-happy')).toBeNull() + expect(fake.disposeCount).toBe(1) + expect(dataDisposers[0]).toHaveBeenCalledTimes(1) + }) + + it('returns the same in-flight entry for concurrent getOrCreate calls and spawns once', async () => { + const [a, b] = await Promise.all([ + LC.getOrCreate('panel-race', { workspaceId: 'ws-1' }), + LC.getOrCreate('panel-race', { workspaceId: 'ws-1' }), + ]) + expect(a).toBe(b) + expect(terminalCreate).toHaveBeenCalledTimes(1) + LC.dispose('panel-race') + }) + + it('writes initialInput into the terminal after spawn', async () => { + await LC.getOrCreate('panel-input', { workspaceId: 'ws-1', initialInput: 'npm test\r' }) + expect(terminalInstances[0].writes).toContain('npm test\r') + LC.dispose('panel-input') + }) + + it('uses the session-restore cwd and replays the scrollback log for restored terminals', async () => { + restoreData.set('panel-restored', { cwd: '/tmp/restored-project', replayFromId: 'old-pty' }) + + await LC.getOrCreate('panel-restored', { workspaceId: 'ws-1' }) + + expect(terminalCreate).toHaveBeenCalledWith( + expect.objectContaining({ cwd: '/tmp/restored-project' }), + ) + expect(replayTerminalLog).toHaveBeenCalledWith('panel-restored') + LC.dispose('panel-restored') + }) + + it('marks the entry dead (not removed) and prints the exit line when the PTY exits on its own', async () => { + terminalCreate.mockResolvedValueOnce('pty-exits') + const entry = await LC.getOrCreate('panel-exits', { workspaceId: 'ws-1' }) + + fireExit('pty-exits', 137) + + // Membership != liveness: the entry lingers so the buffer stays readable. + expect(RS.has('panel-exits')).toBe(true) + expect(entry.alive).toBe(false) + const exitLine = terminalInstances[0].writes.find((w) => w.includes('Process exited with code 137')) + expect(exitLine).toBeTruthy() + LC.dispose('panel-exits') + }) +}) + +// =========================================================================== +// Adopt path (cross-window transfer): reconnect to an existing PTY +// =========================================================================== +describe('adopt path — pending transfer reconnects without spawning', () => { + it('adopts the existing ptyId, wires listeners, and defers scrollback + ack', async () => { + LC.setPendingTransfer('panel-adopt', 'pty-transferred', 'CAPTURED-SCROLLBACK') + + const entry = await LC.getOrCreate('panel-adopt', { workspaceId: 'ws-2' }) + + // No new PTY — the live one is adopted. + expect(terminalCreate).not.toHaveBeenCalled() + expect(entry.ptyId).toBe('pty-transferred') + expect(RS.panelIdForPty('pty-transferred')).toBe('panel-adopt') + expect(RS.pendingTransfers.has('panel-adopt')).toBe(false) // consumed + + // Listeners are wired to the EXISTING pty immediately. + expect(shellRegisterTerminal).toHaveBeenCalledWith('pty-transferred') + fireData('pty-transferred', 'live output') + expect(terminalInstances[0].writes).toContain('live output') + + // Scrollback + ack are deferred until attach()/finalizeReconnect — writing + // them now would land in an unopened 80x24 buffer. + expect(entry.pendingReconnect).toEqual({ ptyId: 'pty-transferred', scrollback: 'CAPTURED-SCROLLBACK' }) + expect(terminalInstances[0].writes).not.toContain('CAPTURED-SCROLLBACK') + expect(panelTransferAck).not.toHaveBeenCalled() + + LC.dispose('panel-adopt') + }) + + it('finalizeReconnect nudges winsize, writes scrollback, acks once, then settles size', async () => { + vi.useFakeTimers() + try { + LC.setPendingTransfer('panel-fin', 'pty-fin', 'SB') + const entry = await LC.getOrCreate('panel-fin', { workspaceId: 'ws-2' }) + + LC.finalizeReconnect('panel-fin') + + // SIGWINCH nudge: one row short of the fitted size (80x24 fake default). + expect(terminalResize).toHaveBeenCalledWith('pty-fin', 80, 23) + expect(terminalInstances[0].writes).toContain('SB') + expect(panelTransferAck).toHaveBeenCalledTimes(1) + expect(panelTransferAck).toHaveBeenCalledWith('pty-fin') + expect(entry.pendingReconnect).toBeUndefined() + + // 150ms later it settles to the real fitted size. + vi.advanceTimersByTime(150) + expect(terminalResize).toHaveBeenLastCalledWith('pty-fin', 80, 24) + + // Idempotent: a second finalize is a no-op (no double ack / rewrite). + LC.finalizeReconnect('panel-fin') + expect(panelTransferAck).toHaveBeenCalledTimes(1) + expect(terminalInstances[0].writes.filter((w) => w === 'SB')).toHaveLength(1) + + LC.dispose('panel-fin') + } finally { + vi.useRealTimers() + } + }) + + it('skips the settle resize if the panel was disposed during the 150ms window', async () => { + vi.useFakeTimers() + try { + LC.setPendingTransfer('panel-gone', 'pty-gone', undefined) + await LC.getOrCreate('panel-gone', { workspaceId: 'ws-2' }) + LC.finalizeReconnect('panel-gone') + terminalResize.mockClear() + + LC.dispose('panel-gone') + vi.advanceTimersByTime(200) + + expect(terminalResize).not.toHaveBeenCalled() + } finally { + vi.useRealTimers() + } + }) +}) + +// =========================================================================== +// Release (transfer source) + double-release +// =========================================================================== +describe('release — transfer source lets go without killing the PTY', () => { + it('removes the entry and identity but never kills the PTY; xterm disposed exactly once', async () => { + terminalCreate.mockResolvedValueOnce('pty-keep') + await LC.getOrCreate('panel-rel', { workspaceId: 'ws-1' }) + + LC.release('panel-rel') + + expect(RS.has('panel-rel')).toBe(false) + expect(RS.panelIdForPty('pty-keep')).toBeNull() + expect(terminalKill).not.toHaveBeenCalled() // PTY lives on in main + expect(shellUnregisterTerminal).not.toHaveBeenCalled() + expect(terminalInstances[0].disposeCount).toBe(1) // local xterm torn down + expect(dataDisposers[0]).toHaveBeenCalledTimes(1) // IPC listener removed + }) + + it('double-release is a safe no-op (no double-dispose, no throw)', async () => { + await LC.getOrCreate('panel-rel2', { workspaceId: 'ws-1' }) + + LC.release('panel-rel2') + expect(() => LC.release('panel-rel2')).not.toThrow() + + expect(terminalInstances[0].disposeCount).toBe(1) + expect(dataDisposers[0]).toHaveBeenCalledTimes(1) + }) + + it('double-dispose is a safe no-op (PTY killed exactly once)', async () => { + terminalCreate.mockResolvedValueOnce('pty-d2') + await LC.getOrCreate('panel-d2', { workspaceId: 'ws-1' }) + + LC.dispose('panel-d2') + expect(() => LC.dispose('panel-d2')).not.toThrow() + + expect(terminalKill).toHaveBeenCalledTimes(1) + expect(terminalInstances[0].disposeCount).toBe(1) + }) + + it('release clears a pending transfer so it cannot hijack a later fresh mount', async () => { + await LC.getOrCreate('panel-hijack', { workspaceId: 'ws-1' }) + LC.setPendingTransfer('panel-hijack', 'pty-stale', 'old-sb') + + LC.release('panel-hijack') + expect(RS.pendingTransfers.has('panel-hijack')).toBe(false) + + // A later fresh mount spawns a NEW pty instead of adopting the stale one. + terminalCreate.mockResolvedValueOnce('pty-fresh-again') + const entry = await LC.getOrCreate('panel-hijack', { workspaceId: 'ws-1' }) + expect(entry.ptyId).toBe('pty-fresh-again') + LC.dispose('panel-hijack') + }) +}) + +// =========================================================================== +// Remount armed but the panel never remounts +// =========================================================================== +describe('remount armed but never completed', () => { + it('an unconsumed pending transfer stays deposited indefinitely', async () => { + // Arm: source window released, transfer deposited in this window... and + // then the receiving panel never mounts (e.g. drop aborted mid-flight). + LC.setPendingTransfer('panel-never', 'pty-orphan', 'sb') + + // Nothing in the lifecycle module ever expires this entry. + expect(RS.pendingTransfers.get('panel-never')).toEqual({ ptyId: 'pty-orphan', scrollback: 'sb' }) + + // BUG?: pendingTransfers has no TTL/cancel path. If the receiving panel + // never mounts, the PTY in main stays un-acked (main keeps buffering its + // output) and a MUCH later mount of the same panelId would silently adopt + // the stale ptyId instead of spawning fresh. Worse: dispose() and release() + // both early-return when there is no registry entry — BEFORE their + // pendingTransfers.delete line — so for a never-mounted panel there is NO + // cleanup path at all except actually mounting it (getOrCreate consumes it). + LC.dispose('panel-never') + LC.release('panel-never') + expect(RS.pendingTransfers.has('panel-never')).toBe(true) // still armed! + + // Mounting is the only consumer — and it adopts the (possibly stale) pty. + const entry = await LC.getOrCreate('panel-never', { workspaceId: 'ws-1' }) + expect(entry.ptyId).toBe('pty-orphan') + expect(RS.pendingTransfers.has('panel-never')).toBe(false) + LC.dispose('panel-never') + }) + + it('an adopted-but-never-attached terminal is fully reclaimed by dispose, though the transfer is never acked', async () => { + LC.setPendingTransfer('panel-noattach', 'pty-noattach', 'sb') + const entry = await LC.getOrCreate('panel-noattach', { workspaceId: 'ws-2' }) + expect(entry.pendingReconnect).toBeTruthy() + + // The panel unmounts before attach() ever ran finalizeReconnect. + LC.dispose('panel-noattach') + + // dispose() kills the adopted PTY, so main-side state is reclaimed; the + // ack is never sent, which is moot since the PTY is dead. No registry leak. + expect(terminalKill).toHaveBeenCalledWith('pty-noattach') + expect(panelTransferAck).not.toHaveBeenCalled() + expect(RS.has('panel-noattach')).toBe(false) + expect(RS.panelIdForPty('pty-noattach')).toBeNull() + }) +}) + +// =========================================================================== +// Spawn failure / IPC error path +// =========================================================================== +describe('spawn failure', () => { + it('records the failure, notifies, and removes the zombie entry so retry can rebuild', async () => { + const notified: string[] = [] + const unsub = RS.subscribeFailure((id) => notified.push(id)) + terminalCreate.mockRejectedValueOnce(new Error('spawn failed: no shell')) + + await LC.getOrCreate('panel-fail', { workspaceId: 'ws-1' }) + + // No zombie: the half-built entry is gone, the xterm disposed. + expect(RS.has('panel-fail')).toBe(false) + expect(terminalInstances[0].disposeCount).toBe(1) + // Failure recorded + observers notified. + expect(RS.getFailure('panel-fail')).toMatch(/spawn failed/i) + expect(notified).toContain('panel-fail') + + // Retry: a fresh getOrCreate clears the failure and spawns for real. + notified.length = 0 + terminalCreate.mockResolvedValueOnce('pty-retry-ok') + const entry = await LC.getOrCreate('panel-fail', { workspaceId: 'ws-1' }) + expect(entry.ptyId).toBe('pty-retry-ok') + expect(RS.getFailure('panel-fail')).toBeNull() + expect(notified).toContain('panel-fail') // re-render notification on retry + expect(RS.has('panel-fail')).toBe(true) + + LC.dispose('panel-fail') + unsub() + }) + + it('kills a PTY that finishes spawning after the panel was already disposed', async () => { + let resolveSpawn: (id: string) => void = () => {} + terminalCreate.mockImplementationOnce( + () => new Promise((resolve) => { resolveSpawn = resolve }), + ) + + const pending = LC.getOrCreate('panel-late', { workspaceId: 'ws-1' }) + expect(RS.has('panel-late')).toBe(true) // entry registered synchronously + + // Panel torn down while the IPC spawn is still in flight. ptyId is still '' + // so dispose cannot kill anything yet. + LC.dispose('panel-late') + expect(terminalKill).not.toHaveBeenCalled() + + // Wait until getOrCreate has progressed past its earlier awaits and the + // (deferred) spawn IPC is actually in flight, then let it land — the + // orphan PTY must be killed, not leaked. + await vi.waitFor(() => expect(terminalCreate).toHaveBeenCalled()) + resolveSpawn('pty-late') + await pending + + expect(terminalKill).toHaveBeenCalledWith('pty-late') + expect(RS.has('panel-late')).toBe(false) + expect(RS.panelIdForPty('pty-late')).toBeNull() + }) +}) + +// =========================================================================== +// Workspace boundaries +// =========================================================================== +describe('workspace boundaries', () => { + it('remounting a panelId under a different workspace kills the old PTY and spawns fresh', async () => { + terminalCreate.mockResolvedValueOnce('pty-ws-a') + await LC.getOrCreate('panel-move', { workspaceId: 'ws-A' }) + + terminalCreate.mockResolvedValueOnce('pty-ws-b') + const entry = await LC.getOrCreate('panel-move', { workspaceId: 'ws-B' }) + + expect(terminalKill).toHaveBeenCalledWith('pty-ws-a') // old one torn down + expect(entry.ptyId).toBe('pty-ws-b') + expect(RS.workspaceIdForPanel('panel-move')).toBe('ws-B') + expect(RS.panelIdForPty('pty-ws-a')).toBeNull() + LC.dispose('panel-move') + }) + + it('disposeWorkspace tears down only that workspace\'s terminals', async () => { + terminalCreate.mockResolvedValueOnce('pty-w1a') + await LC.getOrCreate('panel-w1a', { workspaceId: 'ws-1' }) + terminalCreate.mockResolvedValueOnce('pty-w1b') + await LC.getOrCreate('panel-w1b', { workspaceId: 'ws-1' }) + terminalCreate.mockResolvedValueOnce('pty-w2') + await LC.getOrCreate('panel-w2', { workspaceId: 'ws-2' }) + + LC.disposeWorkspace('ws-1') + + expect(RS.has('panel-w1a')).toBe(false) + expect(RS.has('panel-w1b')).toBe(false) + expect(RS.has('panel-w2')).toBe(true) + expect(terminalKill).toHaveBeenCalledWith('pty-w1a') + expect(terminalKill).toHaveBeenCalledWith('pty-w1b') + expect(terminalKill).not.toHaveBeenCalledWith('pty-w2') + + LC.dispose('panel-w2') + }) +}) diff --git a/src/renderer/lib/workspace/canvasAccess.placement.test.tsx b/src/renderer/lib/workspace/canvasAccess.placement.test.tsx new file mode 100644 index 00000000..db79ec56 --- /dev/null +++ b/src/renderer/lib/workspace/canvasAccess.placement.test.tsx @@ -0,0 +1,69 @@ +// ============================================================================= +// placementForActivePanel — multi-canvas routing regression. +// +// Bug: when the active panel was a canvas, placementForActivePanel returned +// `undefined` ("default canvas placement"), and the default routes to the +// workspace's PRIMARY canvas (the first canvas tab in the center zone). With a +// SECONDARY canvas tab active, every keyboard-created panel therefore landed on +// a hidden canvas — to the user, panel creation silently stopped working. +// The fix pins the placement to the active canvas explicitly. +// ============================================================================= + +import { describe, it, expect, afterEach } from 'vitest' +import { + registerCanvasOps, + unregisterCanvasOps, + placementForActivePanel, + getActiveCanvasPanelId, +} from './canvasAccess' +import type { CanvasOperations } from '../canvas/canvasBridge' +import { setActivePanel } from '../activePanel' + +// placementForActivePanel only consults the ops REGISTRY for the canvas-active +// branch, so a stub is sufficient — no real canvas store needed. +const stubOps = {} as CanvasOperations + +const PRIMARY = 'canvas-primary' +const SECONDARY = 'canvas-secondary' + +afterEach(() => { + unregisterCanvasOps(PRIMARY) + unregisterCanvasOps(SECONDARY) + setActivePanel(null) +}) + +describe('placementForActivePanel with multiple canvases', () => { + it('pins the placement to the ACTIVE canvas, not the primary one', () => { + registerCanvasOps(PRIMARY, stubOps) + registerCanvasOps(SECONDARY, stubOps) + setActivePanel(SECONDARY) + + expect(placementForActivePanel()).toEqual({ + target: 'canvas', + canvasPanelId: SECONDARY, + }) + }) + + it('pins to the primary canvas when that one is active', () => { + registerCanvasOps(PRIMARY, stubOps) + registerCanvasOps(SECONDARY, stubOps) + setActivePanel(PRIMARY) + + expect(placementForActivePanel()).toEqual({ + target: 'canvas', + canvasPanelId: PRIMARY, + }) + }) + + it('returns undefined when nothing is active', () => { + expect(placementForActivePanel()).toBeUndefined() + }) + + it('getActiveCanvasPanelId resolves the active secondary canvas', () => { + registerCanvasOps(PRIMARY, stubOps) + registerCanvasOps(SECONDARY, stubOps) + setActivePanel(SECONDARY) + + expect(getActiveCanvasPanelId()).toBe(SECONDARY) + }) +}) diff --git a/src/renderer/lib/workspace/canvasAccess.ts b/src/renderer/lib/workspace/canvasAccess.ts index 6041ea1d..4a6753fc 100644 --- a/src/renderer/lib/workspace/canvasAccess.ts +++ b/src/renderer/lib/workspace/canvasAccess.ts @@ -112,15 +112,18 @@ export function getActiveCanvasOps(): CanvasOperations | null { /** Placement for a keyboard-created panel (Cmd+T / Cmd+N / …) based on the * canonical active panel. A docked active panel → tab into its exact stack (so * a split lands in the focused pane, not the zone's first stack). A canvas - * active panel (or none) → undefined, the default canvas placement. */ + * active panel → pinned to THAT canvas; none → undefined, the default + * (primary) canvas placement. */ export function placementForActivePanel(): PanelPlacement | undefined { const activeId = getActivePanelId() if (!activeId) return undefined // A canvas is itself a center-zone dock tab, so it HAS a dock location — but a // create while a canvas is active must land ON the canvas, not as a sibling // tab beside it. Canvas panels register ops, so the registry distinguishes - // them; treat them as the default (canvas) placement. - if (canvasOpsRegistry.has(activeId)) return undefined + // them. Pin to the active canvas explicitly: the unpinned default routes to + // the workspace's PRIMARY canvas, which is the wrong (hidden) one whenever a + // secondary canvas tab is active. + if (canvasOpsRegistry.has(activeId)) return { target: 'canvas', canvasPanelId: activeId } const workspaceId = useAppStore.getState().selectedWorkspaceId const location = getWorkspaceDockStore(workspaceId)?.getState().getPanelLocation(activeId) if (location?.type === 'dock') { diff --git a/src/renderer/lib/workspace/session.restoreDockWindow.test.ts b/src/renderer/lib/workspace/session.restoreDockWindow.test.ts index b7595c5f..0d7c853a 100644 --- a/src/renderer/lib/workspace/session.restoreDockWindow.test.ts +++ b/src/renderer/lib/workspace/session.restoreDockWindow.test.ts @@ -146,28 +146,44 @@ describe('buildDockWindowRestoreInit', () => { expect(Object.keys(cs!.childPanels).sort()).toEqual(['canvas-child-editor', 'canvas-child-term']) }) - it('does NOT throw when dockState is missing entirely (legacy/malformed snapshot)', () => { + it('recovers panels into a tab stack when dockState is missing entirely (legacy/malformed snapshot)', () => { // FIX: buildDockWindowRestoreInit used to read dw.dockState.zones blindly and // threw "Cannot read properties of undefined (reading 'zones')" for snapshots - // produced before dockState was synced. It must degrade to an empty window. + // produced before dockState was synced. It then degraded to an empty window, + // which silently dropped every panel record. Now the panels are recovered as + // tabs in a synthesized center stack so the window (and its panels) survive. const dw = makeMultiTabSnapshot() delete (dw as Partial).dockState const { topLevelPanelIds, initPayload } = buildDockWindowRestoreInit(dw) - expect(topLevelPanelIds).toEqual([]) + expect(topLevelPanelIds.sort()).toEqual([ + 'canvas-child-editor', + 'canvas-child-term', + 'top-canvas', + 'top-editor', + 'top-term', + ]) expect(initPayload.workspaceId).toBe('ws-9') // Panel records still pass through so the receiver can resolve types/titles. expect(initPayload.panels).toBe(dw.panels) - expect(initPayload.dockState).toBeDefined() + const center = initPayload.dockState.center.layout + expect(center?.type).toBe('tabs') + expect(center && 'panelIds' in center ? center.panelIds.sort() : []).toEqual([ + 'canvas-child-editor', + 'canvas-child-term', + 'top-canvas', + 'top-editor', + 'top-term', + ]) }) - it('does NOT throw when dockState exists but its zones are missing', () => { + it('recovers panels when dockState exists but its zones are missing', () => { const dw = makeMultiTabSnapshot() dw.dockState = {} as DockStateSnapshot const { topLevelPanelIds } = buildDockWindowRestoreInit(dw) - expect(topLevelPanelIds).toEqual([]) + expect(topLevelPanelIds).toHaveLength(5) }) - it('returns no top-level ids for an empty dock layout', () => { + it('recovers panels orphaned by an empty dock layout', () => { const dw = makeMultiTabSnapshot() dw.dockState = { zones: { @@ -178,6 +194,58 @@ describe('buildDockWindowRestoreInit', () => { }, locations: {}, } + const { topLevelPanelIds } = buildDockWindowRestoreInit(dw) + expect(topLevelPanelIds).toHaveLength(5) + }) + + it('tolerates zones referencing a panel with no record (stale layout after an incomplete flush)', () => { + const dw = makeMultiTabSnapshot() + dw.dockState = multiTabDockState(['top-term', 'ghost-panel', 'top-canvas']) + + const { topLevelPanelIds, initPayload } = buildDockWindowRestoreInit(dw) + + // No throw, and no canvas state is fabricated for the missing record. + expect(initPayload.canvasStates?.['ghost-panel']).toBeUndefined() + expect(initPayload.canvasStates?.['top-canvas']).toBeDefined() + expect(topLevelPanelIds).toContain('top-term') + }) + + // Asserts the DESIRED behavior and is marked `.fails`: a zone entry whose + // panel record is gone currently rides through into topLevelPanelIds and the + // restored layout, so the shell renders a tab backed by nothing. It should be + // pruned at restore. Remove the marker when fixed. + it.fails('prunes zone entries whose panel record is missing', () => { + const dw = makeMultiTabSnapshot() + dw.dockState = multiTabDockState(['top-term', 'ghost-panel', 'top-editor', 'top-canvas']) + + const { topLevelPanelIds, initPayload } = buildDockWindowRestoreInit(dw) + + expect(topLevelPanelIds).not.toContain('ghost-panel') + const center = initPayload.dockState.center.layout + expect(center && 'panelIds' in center ? center.panelIds : []).not.toContain('ghost-panel') + }) + + it('normalizes an empty terminalCwds map to undefined', () => { + const dw = makeMultiTabSnapshot() + dw.terminalCwds = {} + const { initPayload } = buildDockWindowRestoreInit(dw) + expect(initPayload.terminalCwds).toBeUndefined() + }) + + it('omits canvasStates entirely when no top-level tab is a canvas', () => { + const dw = makeMultiTabSnapshot() + dw.dockState = multiTabDockState(['top-term', 'top-editor']) + + const { topLevelPanelIds, initPayload } = buildDockWindowRestoreInit(dw) + + expect(topLevelPanelIds).toEqual(['top-term', 'top-editor']) + expect(initPayload.canvasStates).toBeUndefined() + }) + + it('returns no top-level ids for a genuinely empty window (no panels)', () => { + const dw = makeMultiTabSnapshot() + delete (dw as Partial).dockState + dw.panels = {} const { topLevelPanelIds, initPayload } = buildDockWindowRestoreInit(dw) expect(topLevelPanelIds).toEqual([]) expect(initPayload.canvasStates).toBeUndefined() diff --git a/src/renderer/lib/workspace/sessionSave.ts b/src/renderer/lib/workspace/sessionSave.ts index febb8100..2eef7a4b 100644 --- a/src/renderer/lib/workspace/sessionSave.ts +++ b/src/renderer/lib/workspace/sessionSave.ts @@ -57,8 +57,6 @@ export async function saveSession(): Promise { continue } - const isSelected = workspace.id === updatedState.selectedWorkspaceId - // Dock layout from the workspace's OWN dock store if activated, else its // last-saved snapshot. The center-zone canvas panel is the primary canvas. const dockSnapshot = getWorkspaceDockSnapshot(workspace.id) @@ -116,10 +114,13 @@ export async function saveSession(): Promise { await Promise.all(scrollbackPromises) } - // Live working directory for each terminal in the SELECTED workspace, keyed - // by panel id, so a restored terminal respawns where it was. Batched. + // Live working directory for every running terminal, keyed by panel id, so a + // restored terminal respawns where it was. Batched. PTYs keep running in + // non-selected (but previously activated) workspaces, so capture is NOT + // limited to the selected one; never-activated workspaces took the deferred- + // snapshot path above and keep their saved cwds. const terminalCwds: Record = {} - if (isSelected && panels) { + if (panels) { const cwdPromises: { id: string; promise: Promise }[] = [] for (const panel of Object.values(panels)) { if (panel.type !== 'terminal') continue diff --git a/src/renderer/lib/workspace/sessionSerialize.roundTrip.test.ts b/src/renderer/lib/workspace/sessionSerialize.roundTrip.test.ts new file mode 100644 index 00000000..24895c57 --- /dev/null +++ b/src/renderer/lib/workspace/sessionSerialize.roundTrip.test.ts @@ -0,0 +1,259 @@ +// ============================================================================= +// Session persistence round-trip — SessionSnapshot → on-disk project files +// (.cate/workspace.json + .cate/session.json, through real JSON) → snapshot. +// Builds the dock layout and canvas geometry with the REAL dockStore / +// canvasStore so this exercises the same shapes saveSession persists, and +// asserts the shareable / machine-local split: workspace.json carries no +// absolute paths, scratch content, or worktree tags; session.json carries +// exactly those and nothing for panels that have none. +// ============================================================================= + +import { describe, it, expect } from 'vitest' +import { + buildWorkspaceFile, + buildSessionFile, + projectFilesToSnapshot, + collectPanelIdsFromDockState, +} from './sessionSerialize' +import { createDockStore } from '../../stores/dockStore' +import { createCanvasStore } from '../../stores/canvasStore' +import type { + SessionSnapshot, + PanelState, + DetachedDockWindowSnapshot, + CanvasSnapshot, +} from '../../../shared/types' + +const ROOT = '/Users/dev/my-repo' +const WORKTREE_PATH = `${ROOT}/.cate/worktrees/fix-login` + +function panel(p: Partial & Pick): PanelState { + return { title: p.id, isDirty: false, ...p } +} + +/** Build a realistic snapshot: a center canvas holding two editors, a bottom + * dock with a browser | terminal split, plus worktree + remote metadata. */ +function buildSnapshot(): { snapshot: SessionSnapshot; canvasSnapshot: CanvasSnapshot } { + // Dock layout via the real dock store. + const dock = createDockStore() + dock.getState().dockPanel('canvas-1', 'center') + dock.getState().dockPanel('web-1', 'bottom') + const webStack = dock.getState().getPanelLocation('web-1')! + dock.getState().dockPanel('term-1', 'bottom', { + type: 'split', + stackId: webStack.type === 'dock' ? webStack.stackId : '', + edge: 'right', + }) + const dockState = dock.getState().getSnapshot() + + // Canvas geometry via the real canvas store. + const canvas = createCanvasStore() + canvas.getState().addNode('ed-1', 'editor', { x: 100, y: 80 }, { width: 600, height: 400 }) + canvas.getState().addNode('ed-scratch', 'editor', { x: 900, y: 80 }, { width: 500, height: 300 }) + canvas.getState().setZoomAndOffset(0.8, { x: -120, y: 40 }) + const canvasSnapshot: CanvasSnapshot = { + id: 'canvas-1', + canvasNodes: canvas.getState().nodes, + zoomLevel: canvas.getState().zoomLevel, + viewportOffset: canvas.getState().viewportOffset, + } + + const snapshot: SessionSnapshot = { + workspaceId: 'ws-uuid-1', + workspaceName: 'My Repo', + rootPath: ROOT, + dockState, + panels: { + 'canvas-1': panel({ id: 'canvas-1', type: 'canvas' }), + 'ed-1': panel({ id: 'ed-1', type: 'editor', filePath: `${ROOT}/src/app.ts` }), + 'ed-scratch': panel({ id: 'ed-scratch', type: 'editor', unsavedContent: 'SCRATCH-CONTENT' }), + 'term-1': panel({ id: 'term-1', type: 'terminal', worktreeId: 'wt-1' }), + 'web-1': panel({ + id: 'web-1', + type: 'browser', + url: 'http://localhost:3000', + proxyUrl: 'http://user:pass@proxy:8080', + }), + }, + canvases: { 'canvas-1': canvasSnapshot }, + terminalCwds: { 'term-1': WORKTREE_PATH }, + worktrees: [{ id: 'wt-1', path: WORKTREE_PATH, color: '#ff5555', label: 'fix-login' }], + connection: { + kind: 'server', + companionId: 'comp-1', + host: 'devbox', + user: 'anton', + remotePath: '/srv/my-repo', + }, + } + return { snapshot, canvasSnapshot } +} + +/** Simulate the disk: serialize to JSON text and parse back, like a real save/load. */ +function throughDisk(value: T): T { + return JSON.parse(JSON.stringify(value)) +} + +describe('workspace.json + session.json round-trip', () => { + it('restores an equivalent snapshot through real JSON serialization', () => { + const { snapshot, canvasSnapshot } = buildSnapshot() + + const wsFile = throughDisk(buildWorkspaceFile(snapshot, ROOT, '#00ff00')) + const sessFile = throughDisk(buildSessionFile(snapshot)) + const restored = projectFilesToSnapshot(wsFile, sessFile, ROOT) + + // Identity + reconnect metadata. + expect(restored.workspaceId).toBe('ws-uuid-1') + expect(restored.workspaceName).toBe('My Repo') + expect(restored.rootPath).toBe(ROOT) + expect(restored.connection).toEqual(snapshot.connection) + expect(restored.worktrees).toEqual(snapshot.worktrees) + + // Every placed panel comes back, with machine-local facts re-attached. + expect(Object.keys(restored.panels!).sort()).toEqual( + Object.keys(snapshot.panels!).sort(), + ) + expect(restored.panels!['ed-1'].filePath).toBe(`${ROOT}/src/app.ts`) + expect(restored.panels!['ed-scratch'].unsavedContent).toBe('SCRATCH-CONTENT') + expect(restored.panels!['term-1'].worktreeId).toBe('wt-1') + expect(restored.panels!['web-1'].url).toBe('http://localhost:3000') + expect(restored.panels!['web-1'].proxyUrl).toBe('http://user:pass@proxy:8080') + expect(restored.terminalCwds).toEqual({ 'term-1': WORKTREE_PATH }) + + // Dock layout and canvas geometry survive byte-for-byte. + expect(restored.dockState).toEqual(throughDisk(snapshot.dockState)) + expect(restored.canvases).toEqual(throughDisk({ 'canvas-1': canvasSnapshot })) + expect(collectPanelIdsFromDockState(restored.dockState!.zones).sort()).toEqual([ + 'canvas-1', + 'term-1', + 'web-1', + ]) + }) + + it('keeps machine-local facts OUT of the committed workspace.json', () => { + const { snapshot } = buildSnapshot() + + const wsText = JSON.stringify(buildWorkspaceFile(snapshot, ROOT, '')) + + // No absolute paths — the editor's file is stored relative to the root. + expect(wsText).not.toContain(ROOT) + expect(wsText).toContain('"src/app.ts"') + // No scratch buffers, worktree tags, or terminal cwds. + expect(wsText).not.toContain('SCRATCH-CONTENT') + expect(wsText).not.toContain('wt-1') + expect(wsText).not.toContain('workingDirectory') + }) + + it('keeps shareable metadata OUT of session.json and skips panels with no machine-local facts', () => { + const { snapshot } = buildSnapshot() + + const sessFile = buildSessionFile(snapshot) + + // Only the terminal (cwd + worktree) and the scratch editor have session facts. + expect(Object.keys(sessFile.panels).sort()).toEqual(['ed-scratch', 'term-1']) + expect(sessFile.panels['term-1']).toEqual({ + panelId: 'term-1', + workingDirectory: WORKTREE_PATH, + unsavedContent: undefined, + worktreeId: 'wt-1', + }) + const sessText = JSON.stringify(sessFile) + expect(sessText).not.toContain('src/app.ts') + expect(sessText).not.toContain('localhost:3000') + }) + + it('a file outside the workspace root keeps its absolute path through the round trip', () => { + const { snapshot } = buildSnapshot() + snapshot.panels!['ed-out'] = panel({ + id: 'ed-out', + type: 'editor', + filePath: '/etc/hosts', + }) + + const wsFile = throughDisk(buildWorkspaceFile(snapshot, ROOT)) + const restored = projectFilesToSnapshot(wsFile, null, ROOT) + + expect(restored.panels!['ed-out'].filePath).toBe('/etc/hosts') + }) + + it('restores from workspace.json alone (no session.json) without machine-local facts', () => { + const { snapshot } = buildSnapshot() + + const wsFile = throughDisk(buildWorkspaceFile(snapshot, ROOT)) + const restored = projectFilesToSnapshot(wsFile, null, ROOT) + + expect(restored.workspaceId).toBeUndefined() + expect(restored.connection).toBeUndefined() + expect(restored.worktrees).toBeUndefined() + expect(restored.terminalCwds).toBeUndefined() + expect(restored.panels!['term-1'].worktreeId).toBeUndefined() + // Shareable structure still restores fully. + expect(restored.panels!['ed-1'].filePath).toBe(`${ROOT}/src/app.ts`) + expect(restored.dockState).toEqual(throughDisk(snapshot.dockState)) + expect(restored.canvases).toEqual(throughDisk(snapshot.canvases)) + }) + + it('includes detached dock windows in session.json only when there are any', () => { + const { snapshot } = buildSnapshot() + expect(buildSessionFile(snapshot, []).dockWindows).toBeUndefined() + + const detachedDock = createDockStore() + detachedDock.getState().dockPanel('term-2', 'center') + const dw: DetachedDockWindowSnapshot = { + dockState: detachedDock.getState().getSnapshot(), + panels: { 'term-2': panel({ id: 'term-2', type: 'terminal' }) }, + bounds: { x: 50, y: 60, width: 800, height: 600 }, + workspaceId: 'ws-uuid-1', + terminalCwds: { 'term-2': ROOT }, + } + + const sessFile = throughDisk(buildSessionFile(snapshot, [dw])) + expect(sessFile.dockWindows).toHaveLength(1) + expect(sessFile.dockWindows![0].bounds).toEqual({ x: 50, y: 60, width: 800, height: 600 }) + expect(collectPanelIdsFromDockState(sessFile.dockWindows![0].dockState.zones)).toEqual([ + 'term-2', + ]) + }) + + it('restores a bare workspace.json with no panels, canvases, or dock state', () => { + const restored = projectFilesToSnapshot({ version: 1, name: 'Bare', color: '' }, null, ROOT) + + expect(restored.workspaceName).toBe('Bare') + expect(restored.rootPath).toBe(ROOT) + expect(restored.panels).toBeUndefined() + expect(restored.canvases).toBeUndefined() + expect(restored.dockState).toBeUndefined() + expect(restored.terminalCwds).toBeUndefined() + }) + + it('ignores session.json entries for panels missing from workspace.json', () => { + const { snapshot } = buildSnapshot() + const wsFile = throughDisk(buildWorkspaceFile(snapshot, ROOT)) + // A stale session.json referencing a panel that was since closed. + const staleSess = throughDisk(buildSessionFile(snapshot)) + staleSess.panels['ghost'] = { + panelId: 'ghost', + workingDirectory: '/tmp/elsewhere', + worktreeId: 'wt-stale', + } + + const restored = projectFilesToSnapshot(wsFile, staleSess, ROOT) + + expect(restored.panels!['ghost']).toBeUndefined() + expect(restored.terminalCwds).toEqual({ 'term-1': WORKTREE_PATH }) + }) + + it('a Windows-style root round-trips editor paths with native separators', () => { + const winRoot = 'C:\\Users\\dev\\repo' + const { snapshot } = buildSnapshot() + snapshot.panels = { + 'ed-1': panel({ id: 'ed-1', type: 'editor', filePath: 'C:\\Users\\dev\\repo\\src\\app.ts' }), + } + + const wsFile = throughDisk(buildWorkspaceFile(snapshot, winRoot)) + expect(wsFile.panels!['ed-1'].filePath).toBe('src/app.ts') + + const restored = projectFilesToSnapshot(wsFile, null, winRoot) + expect(restored.panels!['ed-1'].filePath).toBe('C:\\Users\\dev\\repo\\src\\app.ts') + }) +}) diff --git a/src/renderer/lib/workspace/sessionStartup.ts b/src/renderer/lib/workspace/sessionStartup.ts index bd464fa6..cd39c77a 100644 --- a/src/renderer/lib/workspace/sessionStartup.ts +++ b/src/renderer/lib/workspace/sessionStartup.ts @@ -204,23 +204,36 @@ async function recreateDockWindow(dw: DetachedDockWindowSnapshot): Promise export function buildDockWindowRestoreInit( dw: DetachedDockWindowSnapshot, ): { topLevelPanelIds: string[]; initPayload: DockWindowInitPayload } { - // A legacy/malformed snapshot may lack dockState.zones entirely — treat it as - // an empty window so the caller skips it cleanly rather than throwing. - const zones = dw.dockState?.zones - if (!zones) { - return { - topLevelPanelIds: [], - initPayload: { - panels: dw.panels, - dockState: createDefaultDockState(), - workspaceId: dw.workspaceId, - restore: true, - terminalCwds: dw.terminalCwds, - }, + // A legacy/malformed snapshot may lack dockState.zones entirely, or carry + // zones that reference no panels. A window with no panels at all is genuinely + // empty: return no top-level ids so the caller skips it. But when panel + // records DO exist, dropping the window would silently lose them (the next + // autosave only persists live windows) — recover by laying every panel out as + // a tab in a fresh center stack instead. + let zones = dw.dockState?.zones + let topLevelIds = zones ? collectPanelIdsFromDockState(zones) : [] + if (topLevelIds.length === 0) { + const orphanIds = Object.keys(dw.panels ?? {}) + if (orphanIds.length === 0) { + return { + topLevelPanelIds: [], + initPayload: { + panels: dw.panels, + dockState: zones ?? createDefaultDockState(), + workspaceId: dw.workspaceId, + restore: true, + terminalCwds: dw.terminalCwds, + }, + } } + log.warn( + `[session] dock window snapshot has ${orphanIds.length} panels but no dock zones referencing them; recovering panels into a single tab stack`, + ) + zones = createDefaultDockState() + zones.center.layout = { type: 'tabs', id: 'restored-orphan-tabs', panelIds: orphanIds, activeIndex: 0 } + topLevelIds = orphanIds } - const topLevelIds = collectPanelIdsFromDockState(zones) const topLevelSet = new Set(topLevelIds) const canvasStates: Record = {} @@ -238,7 +251,9 @@ export function buildDockWindowRestoreInit( // Send EVERY persisted panel record (top-level tabs AND canvas children) so // the receiving shell can resolve types/titles AND arm replay for all of them. panels: dw.panels, - dockState: zones, + // Past the guard above, zones is either the snapshot's own layout (with at + // least one referenced panel) or the synthesized recovery stack. + dockState: zones!, workspaceId: dw.workspaceId, // Cold restore: the shell replays every terminal panel by its panelId. restore: true, diff --git a/src/renderer/stores/appStore/closePanel.lifecycle.test.ts b/src/renderer/stores/appStore/closePanel.lifecycle.test.ts new file mode 100644 index 00000000..609cb8e7 --- /dev/null +++ b/src/renderer/stores/appStore/closePanel.lifecycle.test.ts @@ -0,0 +1,514 @@ +// ============================================================================= +// closePanel lifecycle — behavioral tests for THE single disposal path for +// panel records and PTYs (appStore/panelSlice.ts closePanel, plus the helpers +// it rides on). selectionSlice.delete.test.ts mocks closePanel; this suite +// exercises the real thing end-to-end through real store actions. +// +// Faked boundary: lib/terminal/terminalRegistry. The fake reproduces the real +// registry's semantics (terminalLifecycle.ts): a window-global entry map keyed +// by panelId; dispose() deletes the entry FIRST (so re-entrant calls are +// no-ops) and only then kills the PTY; release() drops the entry WITHOUT +// killing the PTY (cross-window transfer). `ptyKill` is the spy at the actual +// IPC boundary — a call means a real shell process would have died. +// ============================================================================= + +import { describe, it, expect, beforeEach, vi } from 'vitest' + +// --------------------------------------------------------------------------- +// Hoisted fakes (vi.mock factories run before imports) +// --------------------------------------------------------------------------- + +const h = vi.hoisted(() => { + interface FakeEntry { + ptyId: string + workspaceId: string + } + const entries = new Map() + const ptyKill = vi.fn((_ptyId: string) => {}) + const disposeSpy = vi.fn() + const releaseSpy = vi.fn() + return { entries, ptyKill, disposeSpy, releaseSpy } +}) + +vi.mock('../../lib/logger', () => ({ + default: { debug: vi.fn(), info: vi.fn(), warn: vi.fn(), error: vi.fn(), log: vi.fn() }, +})) + +// Stateful fake mirroring terminalLifecycle.ts dispose/release/disposeWorkspace. +vi.mock('../../lib/terminal/terminalRegistry', () => ({ + terminalRegistry: { + dispose: (panelId: string) => { + h.disposeSpy(panelId) + const entry = h.entries.get(panelId) + if (!entry) return // real dispose() no-ops on a missing entry + h.entries.delete(panelId) // removed from registry BEFORE the kill, like the real one + h.ptyKill(entry.ptyId) + }, + release: (panelId: string) => { + h.releaseSpy(panelId) + h.entries.delete(panelId) // PTY keeps running — no kill + }, + disposeWorkspace: (workspaceId: string) => { + for (const [panelId, entry] of [...h.entries]) { + if (entry.workspaceId === workspaceId) { + h.entries.delete(panelId) + h.ptyKill(entry.ptyId) + } + } + }, + has: (panelId: string) => h.entries.has(panelId), + getEntry: (panelId: string) => h.entries.get(panelId), + }, +})) + +// Agent pi sessions are out of scope here (they have no PTY); stub the module +// so importing the appStore graph doesn't pull in the agent store. +vi.mock('../../../agent/renderer/agentSessionRegistry', () => ({ + disposeAgentPanel: vi.fn(), + getAgentPanelSession: vi.fn(), + saveAgentPanelSession: vi.fn(), +})) + +// Minimal electronAPI so the fire-and-forget workspace sync calls resolve. +beforeEach(() => { + const g = globalThis as unknown as { window?: { electronAPI?: unknown } } + g.window = g.window ?? {} + g.window.electronAPI = { + workspaceCreate: vi.fn(async (input: { id?: string; name?: string; rootPath?: string }) => ({ + ok: true, + workspace: { + id: input.id ?? 'gen', + name: input.name ?? 'Workspace', + color: '', + rootPath: input.rootPath ?? '', + }, + })), + workspaceUpdate: vi.fn(async () => ({ ok: true, workspace: {} })), + workspaceRemove: vi.fn(async () => ({ ok: true })), + recentProjectsAdd: vi.fn(), + recentProjectsRemove: vi.fn(async () => undefined), + agentDispose: vi.fn(async () => undefined), + } +}) + +import { useAppStore } from './index' +import { + getOrCreateCanvasStoreForPanel, + peekCanvasStoreForPanel, +} from '../canvasStore' +import { removePanelFromWindow } from '../../lib/panels/removePanelFromWindow' +import { setActivePanel, getActivePanelId } from '../../lib/activePanel' +import type { DockLayoutNode, PanelState } from '../../../shared/types' + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +let ptySeq = 0 + +/** Simulate what TerminalPanel's mount does: register a live PTY-backed entry + * for the panel in the (faked) terminal registry. */ +function spawnPty(panelId: string, workspaceId: string): string { + const ptyId = `pty-${++ptySeq}` + h.entries.set(panelId, { ptyId, workspaceId }) + return ptyId +} + +function panelsOf(workspaceId: string): Record { + return useAppStore.getState().workspaces.find((w) => w.id === workspaceId)?.panels ?? {} +} + +function tabs(panelIds: string[]): DockLayoutNode { + return { type: 'tabs', id: `stack-${panelIds.join('-')}`, panelIds, activeIndex: 0 } +} + +/** Fresh workspace with its center canvas, as the shell would set it up. */ +function makeWorkspace(suffix: string): { wsId: string; canvasId: string } { + const wsId = useAppStore.getState().addWorkspace(`WS-${suffix}`, `/tmp/${suffix}`, `ws-${suffix}`) + const canvasId = useAppStore.getState().createCanvas(wsId) + return { wsId, canvasId } +} + +let testSeq = 0 + +beforeEach(() => { + // Tear down workspaces from the previous test (removeWorkspace mints one + // fresh empty replacement when the last one goes — that's by design). + for (const w of [...useAppStore.getState().workspaces]) { + useAppStore.getState().removeWorkspace(w.id) + } + h.entries.clear() + h.ptyKill.mockClear() + h.ptyKill.mockImplementation(() => {}) + h.disposeSpy.mockClear() + h.releaseSpy.mockClear() + setActivePanel(null) + testSeq += 1 +}) + +// --------------------------------------------------------------------------- +// 1. Happy path +// --------------------------------------------------------------------------- + +describe('closePanel — happy path', () => { + it('closing a terminal removes its record, kills its PTY exactly once, and removes its (emptied) canvas node', () => { + const { wsId, canvasId } = makeWorkspace(`hp-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 10, y: 10 }) + const ptyId = spawnPty(termId, wsId) + + // Sanity: record exists and the panel landed as a node on the canvas. + expect(panelsOf(wsId)[termId]?.type).toBe('terminal') + const canvasStore = getOrCreateCanvasStoreForPanel(canvasId) + const nodeId = canvasStore.getState().nodeForPanel(termId) + expect(nodeId).toBeTruthy() + // The interactive close path (DockTabStack tab close) first undocks the + // panel from the node's mini-dock; emulate that emptied state here (no + // live node dock store is mounted headlessly, so clear the projection). + canvasStore.getState().setNodeDockLayout(nodeId!, null) + setActivePanel(termId) + + useAppStore.getState().closePanel(wsId, termId) + + // Record gone, PTY killed exactly once at the boundary. + expect(panelsOf(wsId)[termId]).toBeUndefined() + expect(h.ptyKill).toHaveBeenCalledTimes(1) + expect(h.ptyKill).toHaveBeenCalledWith(ptyId) + expect(h.entries.has(termId)).toBe(false) + + // The canvas node is on its way out (removeNode marks it 'exiting'; the UI + // finalizes after the animation) and no longer resolves for the panel after + // finalize. Either way it must not be a live idle node anymore. + const node = nodeId ? canvasStore.getState().nodes[nodeId] : undefined + expect(node === undefined || node.animationState === 'exiting').toBe(true) + + // Closed panel no longer reads as the active panel. + expect(getActivePanelId()).toBeNull() + + // The canvas panel itself is untouched. + expect(panelsOf(wsId)[canvasId]?.type).toBe('canvas') + }) + + it('headless close (petTools/runAction path) leaves the canvas node behind with a stale panelId (BUG?)', () => { + const { wsId, canvasId } = makeWorkspace(`ghost-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 10, y: 10 }) + const ptyId = spawnPty(termId, wsId) + const canvasStore = getOrCreateCanvasStoreForPanel(canvasId) + const nodeId = canvasStore.getState().nodeForPanel(termId)! + + // Call closePanel directly, the way petTools close_terminal and the + // runAction 'closePanel' shortcut do — with the node's seeded mini-dock + // layout (every addNode seeds dockLayout = [its own panel]) untouched. + useAppStore.getState().closePanel(wsId, termId) + + // Disposal and record removal still happen correctly... + expect(panelsOf(wsId)[termId]).toBeUndefined() + expect(h.ptyKill).toHaveBeenCalledTimes(1) + expect(h.ptyKill).toHaveBeenCalledWith(ptyId) + + // BUG?: ...but removeNodeForPanel early-returns whenever the node's dock + // layout still lists ANY panel — and the seeded layout always lists the + // panel being closed. So unless a UI layer emptied the node's mini-dock + // first (the DockTabStack close path) or removes the node itself + // (deleteSelection, CanvasNode close button), the node survives as a ghost + // pointing at a deleted panel record. Headless callers (petTools + // close_terminal, runAction closePanel on a culled/unmounted node) hit + // exactly this. + const node = canvasStore.getState().nodes[nodeId] + expect(node).toBeDefined() // ghost node + expect(node.animationState).not.toBe('exiting') + expect(node.panelId).toBe(termId) // stale reference to a deleted record + }) + + it('closing an editor removes its record without touching any PTY', () => { + const { wsId } = makeWorkspace(`hp2-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 0, y: 0 }) + spawnPty(termId, wsId) + const editorId = useAppStore.getState().createEditor(wsId, '/tmp/file.ts', { x: 300, y: 0 }) + + useAppStore.getState().closePanel(wsId, editorId) + + expect(panelsOf(wsId)[editorId]).toBeUndefined() + // The terminal's PTY is untouched by the editor close. + expect(h.ptyKill).not.toHaveBeenCalled() + expect(h.entries.has(termId)).toBe(true) + expect(panelsOf(wsId)[termId]).toBeDefined() + }) +}) + +// --------------------------------------------------------------------------- +// 2. Closing a canvas panel with children +// --------------------------------------------------------------------------- + +describe('closePanel — canvas with children', () => { + it('recursively disposes child node panels AND mini-dock tab panels, then releases the canvas store', () => { + const { wsId } = makeWorkspace(`cv-${testSeq}`) + const canvas2 = useAppStore.getState().createCanvas(wsId) + + // Two children placed on the secondary canvas via the pinned-canvas path. + const childTerm = useAppStore.getState().createTerminal( + wsId, undefined, undefined, + { target: 'canvas', canvasPanelId: canvas2, position: { x: 0, y: 0 } }, + ) + const childEditor = useAppStore.getState().createEditor( + wsId, '/tmp/x.ts', undefined, + { target: 'canvas', canvasPanelId: canvas2, position: { x: 400, y: 0 } }, + ) + const childPty = spawnPty(childTerm, wsId) + + // A second terminal living only as a TAB in the child node's mini-dock + // (no canvas node of its own) — closePanel must find it via the node's + // dock layout. + const dockChild = `dock-child-${testSeq}` + useAppStore.getState().addPanel(wsId, { id: dockChild, type: 'terminal', title: 'T-tab', isDirty: false }) + const dockChildPty = spawnPty(dockChild, wsId) + const canvas2Store = getOrCreateCanvasStoreForPanel(canvas2) + const childNodeId = canvas2Store.getState().nodeForPanel(childTerm)! + canvas2Store.getState().setNodeDockLayout(childNodeId, tabs([childTerm, dockChild])) + + useAppStore.getState().closePanel(wsId, canvas2) + + // All child records gone, both terminal PTYs killed exactly once each. + expect(panelsOf(wsId)[canvas2]).toBeUndefined() + expect(panelsOf(wsId)[childTerm]).toBeUndefined() + expect(panelsOf(wsId)[childEditor]).toBeUndefined() + expect(panelsOf(wsId)[dockChild]).toBeUndefined() + expect(h.ptyKill).toHaveBeenCalledTimes(2) + expect(h.ptyKill).toHaveBeenCalledWith(childPty) + expect(h.ptyKill).toHaveBeenCalledWith(dockChildPty) + + // The per-panel canvas store was released. + expect(peekCanvasStoreForPanel(canvas2)).toBeUndefined() + }) + + it('canvas-on-canvas is refused at the data layer, so one-level child recursion is exhaustive', () => { + const { wsId } = makeWorkspace(`nest-${testSeq}`) + const parentCanvas = useAppStore.getState().createCanvas(wsId) + + // closePanel recurses exactly ONE level into a closing canvas's children. + // That is sufficient only because a canvas node can never host another + // canvas: addNode refuses panelType 'canvas' outright (returns '' and adds + // nothing), so a grandchild canvas — the case that would need deeper + // recursion — is structurally impossible. + const parentStore = getOrCreateCanvasStoreForPanel(parentCanvas) + const refused = parentStore.getState() + .addNode(`nested-canvas-${testSeq}`, 'canvas', { x: 0, y: 0 }, { width: 600, height: 400 }) + expect(refused).toBe('') + expect(Object.keys(parentStore.getState().nodes)).toEqual([]) + + // And a closing canvas with a normal child still cleans up that child. + const childTerm = useAppStore.getState().createTerminal( + wsId, undefined, undefined, + { target: 'canvas', canvasPanelId: parentCanvas, position: { x: 0, y: 0 } }, + ) + const childPty = spawnPty(childTerm, wsId) + + useAppStore.getState().closePanel(wsId, parentCanvas) + + expect(panelsOf(wsId)[parentCanvas]).toBeUndefined() + expect(panelsOf(wsId)[childTerm]).toBeUndefined() + expect(h.ptyKill).toHaveBeenCalledTimes(1) + expect(h.ptyKill).toHaveBeenCalledWith(childPty) + }) +}) + +// --------------------------------------------------------------------------- +// 3. Panels owned by a detached window +// --------------------------------------------------------------------------- + +describe('closePanel — detached-window interactions', () => { + it('a transfer (detach) releases the xterm but never kills the PTY, and drops the record', () => { + const { wsId } = makeWorkspace(`det-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 0, y: 0 }) + spawnPty(termId, wsId) + + // What the detach/cross-window-drop path runs in the source window. + removePanelFromWindow(wsId, termId, 'terminal', 'transfer') + + expect(h.releaseSpy).toHaveBeenCalledWith(termId) + expect(h.ptyKill).not.toHaveBeenCalled() // PTY survives the move + expect(panelsOf(wsId)[termId]).toBeUndefined() // record now lives in the other window + }) + + it('closePanel on an already-transferred panel is inert: it cannot reach the moved PTY', () => { + const { wsId } = makeWorkspace(`det2-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 0, y: 0 }) + spawnPty(termId, wsId) + removePanelFromWindow(wsId, termId, 'terminal', 'transfer') + h.disposeSpy.mockClear() + + // E.g. a stale UI action referencing the now-detached panel. Note the + // close-vs-transfer decision is encapsulated in teardownPanelContent and + // the registry entry was already released, so dispose() finds nothing — + // the detached window's live PTY cannot be killed from here. Closing a + // detached panel for real routes through that window's own close handler + // (DockWindowShell → removePanelFromWindow 'close'). + expect(() => useAppStore.getState().closePanel(wsId, termId)).not.toThrow() + expect(h.disposeSpy).toHaveBeenCalledWith(termId) // probe happens... + expect(h.ptyKill).not.toHaveBeenCalled() // ...but nothing to kill + }) + + it("closes a record-only panel (placement 'none', e.g. a mini-dock owner placed it privately)", () => { + const { wsId } = makeWorkspace(`none-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, undefined, { target: 'none' }) + const ptyId = spawnPty(termId, wsId) + expect(panelsOf(wsId)[termId]).toBeDefined() + + // Not in the dock tree, not on any canvas — resolvePanelLocation finds + // nothing, but disposal and record removal must still happen. + useAppStore.getState().closePanel(wsId, termId) + + expect(panelsOf(wsId)[termId]).toBeUndefined() + expect(h.ptyKill).toHaveBeenCalledTimes(1) + expect(h.ptyKill).toHaveBeenCalledWith(ptyId) + }) +}) + +// --------------------------------------------------------------------------- +// 4. Cross-workspace isolation +// --------------------------------------------------------------------------- + +describe('closePanel — workspace isolation', () => { + it("closing a panel in workspace A never touches workspace B's panels or PTYs", () => { + const a = makeWorkspace(`iso-a-${testSeq}`) + const b = makeWorkspace(`iso-b-${testSeq}`) + const termA = useAppStore.getState().createTerminal(a.wsId, undefined, { x: 0, y: 0 }) + const termB = useAppStore.getState().createTerminal(b.wsId, undefined, { x: 0, y: 0 }) + const ptyA = spawnPty(termA, a.wsId) + const ptyB = spawnPty(termB, b.wsId) + + useAppStore.getState().closePanel(a.wsId, termA) + + expect(h.ptyKill).toHaveBeenCalledTimes(1) + expect(h.ptyKill).toHaveBeenCalledWith(ptyA) + expect(h.ptyKill).not.toHaveBeenCalledWith(ptyB) + expect(panelsOf(b.wsId)[termB]).toBeDefined() + expect(h.entries.has(termB)).toBe(true) + // B's canvas node is untouched. + expect(getOrCreateCanvasStoreForPanel(b.canvasId).getState().nodeForPanel(termB)).toBeTruthy() + }) + + it("duplicate panel ids across workspaces: the record removal is scoped to A, but the registry is window-global", () => { + const a = makeWorkspace(`dup-a-${testSeq}`) + const b = makeWorkspace(`dup-b-${testSeq}`) + const dupId = `dup-term-${testSeq}` + useAppStore.getState().addPanel(a.wsId, { id: dupId, type: 'terminal', title: 'Dup A', isDirty: false }) + useAppStore.getState().addPanel(b.wsId, { id: dupId, type: 'terminal', title: 'Dup B', isDirty: false }) + // The registry is keyed by panelId per WINDOW, so two same-id panels can + // only ever own ONE entry — ids are assumed globally unique (generateId). + // Register the entry as B's live terminal to expose the seam. + const ptyB = spawnPty(dupId, b.wsId) + + useAppStore.getState().closePanel(a.wsId, dupId) + + // The store-level removal is correctly scoped: only A's record is gone. + expect(panelsOf(a.wsId)[dupId]).toBeUndefined() + expect(panelsOf(b.wsId)[dupId]).toBeDefined() + + // BUG?: the registry teardown is NOT scoped — dispose(panelId) is keyed by + // panel id alone, so closing A's copy killed the PTY registered for B's + // same-id panel. Harmless as long as generateId keeps ids unique per + // window, but any id collision (hand-edited session.json, forced restore + // ids) silently kills the other workspace's terminal. + expect(h.ptyKill).toHaveBeenCalledWith(ptyB) + }) +}) + +// --------------------------------------------------------------------------- +// 5. Unhappy / edge cases +// --------------------------------------------------------------------------- + +describe('closePanel — unhappy paths', () => { + it('unknown panel id: no throw, no state damage, no PTY killed', () => { + const { wsId } = makeWorkspace(`unk-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 0, y: 0 }) + spawnPty(termId, wsId) + const before = JSON.parse(JSON.stringify(useAppStore.getState().workspaces)) + + expect(() => useAppStore.getState().closePanel(wsId, 'no-such-panel')).not.toThrow() + + expect(JSON.parse(JSON.stringify(useAppStore.getState().workspaces))).toEqual(before) + expect(h.ptyKill).not.toHaveBeenCalled() + expect(h.entries.has(termId)).toBe(true) + }) + + it('double close: the second call is a no-op and the PTY is killed exactly once', () => { + const { wsId } = makeWorkspace(`dbl-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 0, y: 0 }) + spawnPty(termId, wsId) + + useAppStore.getState().closePanel(wsId, termId) + expect(() => useAppStore.getState().closePanel(wsId, termId)).not.toThrow() + + expect(h.ptyKill).toHaveBeenCalledTimes(1) + expect(panelsOf(wsId)[termId]).toBeUndefined() + }) + + it('closing the last panel leaves the workspace coherent and still able to create panels', () => { + const { wsId, canvasId } = makeWorkspace(`last-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 0, y: 0 }) + spawnPty(termId, wsId) + + useAppStore.getState().closePanel(wsId, termId) + useAppStore.getState().closePanel(wsId, canvasId) // close the center canvas too + + const ws = useAppStore.getState().workspaces.find((w) => w.id === wsId) + expect(ws).toBeDefined() // workspace itself survives + expect(Object.keys(ws!.panels)).toEqual([]) // truly empty, no zombie records + expect(useAppStore.getState().selectedWorkspaceId).toBeTruthy() + + // The store still functions: a new create lands somewhere (with no canvas + // left, placePanel falls back to the center dock zone) and is recorded. + const newTerm = useAppStore.getState().createTerminal(wsId, undefined, undefined) + expect(newTerm).toBeTruthy() + expect(panelsOf(wsId)[newTerm]?.type).toBe('terminal') + }) +}) + +// --------------------------------------------------------------------------- +// 6. Disposal failure +// --------------------------------------------------------------------------- + +describe('closePanel — disposal failure', () => { + it('a synchronous teardown throw aborts the close and leaves a zombie panel record (BUG?)', () => { + const { wsId } = makeWorkspace(`fail-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 0, y: 0 }) + spawnPty(termId, wsId) + + // The real dispose() catches PTY-kill IPC rejections (.catch), but its + // xterm teardown (cleanupListeners loop, terminal.dispose) is NOT guarded + // — a synchronous throw there escapes teardownPanelContent. + h.ptyKill.mockImplementationOnce(() => { + throw new Error('xterm teardown exploded') + }) + + // BUG?: closePanel runs teardownPanelContent FIRST and without try/catch, + // so a sync teardown failure aborts the whole close: the panel record + // stays in workspace.panels and the dock/canvas placement is never + // removed. The user sees a panel that refused to close; session.json + // keeps persisting it. (The registry entry itself is already gone — real + // dispose deletes it before tearing down — so a RETRY close then succeeds + // in dropping the record, which is the only recovery path.) + expect(() => useAppStore.getState().closePanel(wsId, termId)).toThrow('xterm teardown exploded') + expect(panelsOf(wsId)[termId]).toBeDefined() // zombie record + + // Retry: entry already consumed, teardown no-ops, record finally removed. + expect(() => useAppStore.getState().closePanel(wsId, termId)).not.toThrow() + expect(panelsOf(wsId)[termId]).toBeUndefined() + }) + + it('an async PTY-kill IPC rejection does not block the close (real dispose swallows it)', () => { + const { wsId } = makeWorkspace(`rej-${testSeq}`) + const termId = useAppStore.getState().createTerminal(wsId, undefined, { x: 0, y: 0 }) + spawnPty(termId, wsId) + + // Model the real path: terminalKill() rejects, dispose catches and logs — + // from the caller's perspective the kill "succeeded" synchronously. + h.ptyKill.mockImplementationOnce((ptyId: string) => { + void Promise.reject(new Error(`kill ${ptyId} failed`)).catch(() => { /* logged */ }) + }) + + expect(() => useAppStore.getState().closePanel(wsId, termId)).not.toThrow() + expect(panelsOf(wsId)[termId]).toBeUndefined() + expect(h.entries.has(termId)).toBe(false) + }) +}) diff --git a/src/renderer/stores/appStore/helpers.ts b/src/renderer/stores/appStore/helpers.ts index 2c52f7cb..125f3a66 100644 --- a/src/renderer/stores/appStore/helpers.ts +++ b/src/renderer/stores/appStore/helpers.ts @@ -23,6 +23,7 @@ import { getOrCreateWorkspaceDockStore } from '../../lib/workspace/dockRegistry' import { createDefaultDockState } from '../dockStore' import { ensureCanvasOpsForPanel, + getActiveCanvasOps, getWorkspaceCanvasOps, } from '../../lib/workspace/canvasAccess' import { useWindowPanelStore } from '../windowPanelStore' @@ -270,11 +271,15 @@ function placePanel( // Default: place on a canvas (target === 'canvas'/'auto'/undefined). // Prefer the explicit originating canvas when the caller pinned one (an // interactive create from a specific canvas's toolbar/menu/drop), so the node - // lands on the canvas the user aimed at — not just the primary one. Otherwise - // route by workspace id, which lands on the workspace's own primary canvas and - // works for a background restore into an inactive workspace. + // lands on the canvas the user aimed at — not just the primary one. An + // unpinned create on the ACTIVE workspace lands on the canvas the user is + // looking at (the active one) — the workspace's primary canvas is the wrong + // (hidden) target whenever a secondary canvas tab is active. Background + // restores into an inactive workspace keep the primary-canvas routing. const pinnedCanvasId = placement?.target === 'canvas' ? placement.canvasPanelId : undefined - const ops = pinnedCanvasId ? ensureCanvasOpsForPanel(pinnedCanvasId) : getWorkspaceCanvasOps(workspaceId) + const ops = pinnedCanvasId + ? ensureCanvasOpsForPanel(pinnedCanvasId) + : (isActiveWorkspace ? getActiveCanvasOps() : null) ?? getWorkspaceCanvasOps(workspaceId) if (!ops) { // No canvas to place onto — e.g. a detached dock window (center zone only) // where nothing was focused, so placementForActivePanel couldn't tab into a diff --git a/src/renderer/stores/canvas/placementSlice.test.ts b/src/renderer/stores/canvas/placementSlice.test.ts new file mode 100644 index 00000000..d53d18d7 --- /dev/null +++ b/src/renderer/stores/canvas/placementSlice.test.ts @@ -0,0 +1,397 @@ +// ============================================================================= +// Placement slice — behavioral tests for the interactive ghost-placement +// transaction: beginPlacement snapshots the viewport and zooms out; +// commit/cancel must resolve the transaction exactly once (place the node OR +// restore the viewport + roll back the orphan panel record via onCancelled); +// free "click-anywhere" mode and hover are transaction-scoped sub-state. +// Driven end-to-end through the real composed canvas store — no mocks. +// ============================================================================= + +import { describe, it, expect, vi } from 'vitest' +import { createCanvasStore } from '../canvasStore' +import { ZOOM_MIN, PANEL_DEFAULT_SIZES } from '../../../shared/types' +import { rectsOverlap } from '../../canvas/layoutEngine' + +const SEED_SIZE = { width: 640, height: 400 } +const CONTAINER = { width: 1200, height: 800 } + +/** Store with one focused 640x400 node at the origin — the canonical + * "place a second panel" scenario that triggers the interactive picker. */ +function storeWithSeed(zoom = 1, offset = { x: 0, y: 0 }) { + const store = createCanvasStore() + store.getState().setContainerSize(CONTAINER) + const seed = store.getState().addNode('seed-panel', 'terminal', { x: 0, y: 0 }, SEED_SIZE) + store.getState().focusNode(seed) + store.getState().setZoomAndOffset(zoom, offset) + return { store, seed } +} + +function nodeCount(store: ReturnType) { + return Object.keys(store.getState().nodes).length +} + +describe('beginPlacement', () => { + it('on an empty canvas skips the picker and drops the panel at the viewport centre', () => { + const store = createCanvasStore() + store.getState().setContainerSize(CONTAINER) + const onCancelled = vi.fn() + + const shown = store.getState().beginPlacement('p1', 'terminal', onCancelled) + + expect(shown).toBe(true) + expect(store.getState().pendingPlacement).toBeNull() // no transaction opened + const nodes = Object.values(store.getState().nodes) + expect(nodes).toHaveLength(1) + // 640x400 terminal centred on the view centre (600,400) → top-left (280,200). + expect(nodes[0].origin).toEqual({ x: 280, y: 200 }) + expect(nodes[0].size).toEqual(PANEL_DEFAULT_SIZES.terminal) + expect(store.getState().focusedNodeId).toBe(nodes[0].id) + expect(onCancelled).not.toHaveBeenCalled() + }) + + it('on an empty canvas with an unmeasured container falls back to the default origin', () => { + const store = createCanvasStore() // containerSize stays 0x0 + + const shown = store.getState().beginPlacement('p1', 'editor') + + expect(shown).toBe(true) + const nodes = Object.values(store.getState().nodes) + expect(nodes).toHaveLength(1) + expect(nodes[0].origin).toEqual({ x: 100, y: 100 }) // findFreePosition default + }) + + it('snapshots zoom/offset, zooms out to fit the ghosts, and populates candidate state', () => { + const { store } = storeWithSeed(1.5, { x: 120, y: -40 }) + + const shown = store.getState().beginPlacement('p2', 'terminal') + + expect(shown).toBe(true) + const pending = store.getState().pendingPlacement + expect(pending).not.toBeNull() + // Snapshot is the viewport at begin time, byte-exact. + expect(pending!.prevZoom).toBe(1.5) + expect(pending!.prevOffset).toEqual({ x: 120, y: -40 }) + // The ghosts around a 640x400 node can't fit at 1.5x in a 1200x800 view → + // the camera zoomed OUT (never below the floor) and recentred. + expect(store.getState().zoomLevel).toBeLessThan(1.5) + expect(store.getState().zoomLevel).toBeGreaterThanOrEqual(ZOOM_MIN) + expect(store.getState().viewportOffset).not.toEqual({ x: 120, y: -40 }) + // Fresh transaction sub-state. + expect(pending!.candidates.length).toBeGreaterThanOrEqual(1) + expect(pending!.candidates.length).toBeLessThanOrEqual(6) + expect(pending!.hoveredIndex).toBeNull() + expect(pending!.freeArmed).toBe(false) + expect(pending!.freeGhost).toBeNull() + expect(pending!.size).toEqual(PANEL_DEFAULT_SIZES.terminal) + // The node itself is NOT created yet — only the ghost transaction. + expect(nodeCount(store)).toBe(1) + }) + + it('only ever zooms OUT — already zoomed out further than the fit, zoom is untouched', () => { + const { store } = storeWithSeed(ZOOM_MIN, { x: 0, y: 0 }) + + store.getState().beginPlacement('p2', 'terminal') + + expect(store.getState().pendingPlacement).not.toBeNull() + expect(store.getState().zoomLevel).toBe(ZOOM_MIN) + }) + + it('honors an explicit size override for the ghosts and the placed node', () => { + const { store } = storeWithSeed() + const size = { width: 400, height: 300 } + + store.getState().beginPlacement('p2', 'terminal', undefined, size) + + const pending = store.getState().pendingPlacement! + expect(pending.size).toEqual(size) + const id = store.getState().commitFreePlacement({ x: 5000, y: 5000 })! + expect(store.getState().nodes[id].size).toEqual(size) + }) + + it('anchors recommendations to the last pointer position when nothing is focused', () => { + const make = (pointer: { x: number; y: number }) => { + const store = createCanvasStore() + store.getState().setContainerSize(CONTAINER) + store.getState().addNode('seed', 'terminal', { x: 0, y: 0 }, SEED_SIZE) // not focused + store.getState().setPlacementPointer(pointer) + store.getState().beginPlacement('p2', 'terminal') + return store.getState().pendingPlacement!.candidates[0] + } + + const towardRight = make({ x: 1100, y: 200 }) + const towardBottom = make({ x: 300, y: 700 }) + + // Best ghost lands on the pointer's side of the seed node. + expect(towardRight.point.x).toBeGreaterThanOrEqual(640) + expect(towardBottom.point.y).toBeGreaterThanOrEqual(400) + expect(towardRight.point).not.toEqual(towardBottom.point) + }) +}) + +describe('commitPlacement', () => { + it('places the node exactly once at the chosen candidate, restores zoom, and recentres on the node', () => { + const { store } = storeWithSeed(1.5, { x: 120, y: -40 }) + const onCancelled = vi.fn() + store.getState().beginPlacement('p2', 'terminal', onCancelled) + const candidate = store.getState().pendingPlacement!.candidates[0] + + const nodeId = store.getState().commitPlacement(0) + + expect(nodeId).not.toBeNull() + const s = store.getState() + expect(s.pendingPlacement).toBeNull() + // Exactly one new node, exactly at the candidate rect. + expect(nodeCount(store)).toBe(2) + expect(s.nodes[nodeId!].origin).toEqual(candidate.point) + expect(s.nodes[nodeId!].size).toEqual(candidate.size) + expect(s.nodes[nodeId!].panelId).toBe('p2') + // Zoom is restored to the snapshot; the offset is intentionally NOT the + // snapshot — commit recentres the camera on the freshly placed node. + expect(s.zoomLevel).toBe(1.5) + const cx = candidate.point.x + candidate.size.width / 2 + const cy = candidate.point.y + candidate.size.height / 2 + expect(s.viewportOffset.x).toBeCloseTo(CONTAINER.width / 2 - cx * 1.5) + expect(s.viewportOffset.y).toBeCloseTo(CONTAINER.height / 2 - cy * 1.5) + expect(s.focusedNodeId).toBe(nodeId) + // Commit is the success path — the rollback callback must never fire. + expect(onCancelled).not.toHaveBeenCalled() + }) + + it('rejects an out-of-range candidate index and keeps the transaction pending', () => { + const { store } = storeWithSeed() + store.getState().beginPlacement('p2', 'terminal') + const pendingBefore = store.getState().pendingPlacement! + const zoomBefore = store.getState().zoomLevel + + expect(store.getState().commitPlacement(99)).toBeNull() + expect(store.getState().commitPlacement(-1)).toBeNull() + + // Nothing placed, nothing restored, transaction object untouched. + expect(nodeCount(store)).toBe(1) + expect(store.getState().pendingPlacement).toBe(pendingBefore) + expect(store.getState().zoomLevel).toBe(zoomBefore) + // A valid commit still succeeds afterwards. + expect(store.getState().commitPlacement(0)).not.toBeNull() + expect(nodeCount(store)).toBe(2) + }) + + it('called twice only places once — the second commit is a rejected no-op', () => { + const { store } = storeWithSeed() + store.getState().beginPlacement('p2', 'terminal') + + const first = store.getState().commitPlacement(0) + const second = store.getState().commitPlacement(0) + + expect(first).not.toBeNull() + expect(second).toBeNull() + expect(nodeCount(store)).toBe(2) + }) + + it('is a no-op when no placement is pending', () => { + const { store } = storeWithSeed() + expect(store.getState().commitPlacement(0)).toBeNull() + expect(nodeCount(store)).toBe(1) + }) +}) + +describe('cancelPlacement', () => { + it('restores the snapshotted zoom/offset exactly and fires onCancelled with the panel id', () => { + const { store } = storeWithSeed(1.5, { x: 120, y: -40 }) + const onCancelled = vi.fn() + store.getState().beginPlacement('p2', 'terminal', onCancelled) + expect(store.getState().zoomLevel).not.toBe(1.5) // sanity: we actually zoomed out + + store.getState().cancelPlacement() + + const s = store.getState() + expect(s.pendingPlacement).toBeNull() + expect(s.zoomLevel).toBe(1.5) + expect(s.viewportOffset).toEqual({ x: 120, y: -40 }) + expect(nodeCount(store)).toBe(1) // no node was placed + expect(onCancelled).toHaveBeenCalledTimes(1) + expect(onCancelled).toHaveBeenCalledWith('p2') + }) + + it('cancel mid-placement with the free ghost armed still restores and rolls back cleanly', () => { + const { store } = storeWithSeed(1.2, { x: 7, y: 13 }) + const onCancelled = vi.fn() + store.getState().beginPlacement('p2', 'terminal', onCancelled) + store.getState().setFreeArmed(true) + store.getState().updatePlacementCursor({ x: 2000, y: 2000 }) + expect(store.getState().pendingPlacement!.freeGhost).not.toBeNull() + + store.getState().cancelPlacement() // Escape + + expect(store.getState().pendingPlacement).toBeNull() + expect(store.getState().zoomLevel).toBe(1.2) + expect(store.getState().viewportOffset).toEqual({ x: 7, y: 13 }) + expect(onCancelled).toHaveBeenCalledTimes(1) + }) + + it('after a commit is a no-op — no double restore, no spurious rollback', () => { + const { store } = storeWithSeed(1.5, { x: 120, y: -40 }) + const onCancelled = vi.fn() + store.getState().beginPlacement('p2', 'terminal', onCancelled) + store.getState().commitPlacement(0) + const zoomAfterCommit = store.getState().zoomLevel + const offsetAfterCommit = store.getState().viewportOffset + + store.getState().cancelPlacement() + + expect(store.getState().zoomLevel).toBe(zoomAfterCommit) + expect(store.getState().viewportOffset).toBe(offsetAfterCommit) + expect(nodeCount(store)).toBe(2) // placed node survives + expect(onCancelled).not.toHaveBeenCalled() + }) + + it('double cancel only restores and rolls back once', () => { + const { store } = storeWithSeed(1.5, { x: 0, y: 0 }) + const onCancelled = vi.fn() + store.getState().beginPlacement('p2', 'terminal', onCancelled) + + store.getState().cancelPlacement() + store.getState().setZoom(0.8) // user moves on + store.getState().cancelPlacement() // stray second Escape + + expect(store.getState().zoomLevel).toBe(0.8) // not yanked back to 1.5 + expect(onCancelled).toHaveBeenCalledTimes(1) + }) +}) + +describe('free "place anywhere" mode', () => { + it('arming, previewing, and disarming are transaction-scoped', () => { + const { store } = storeWithSeed() + store.getState().beginPlacement('p2', 'terminal') + + store.getState().setFreeArmed(true) + expect(store.getState().pendingPlacement!.freeArmed).toBe(true) + + // Cursor centred on the seed node → ghost is nudged to a free, non- + // overlapping spot rather than previewing an overlap. + store.getState().updatePlacementCursor({ x: 320, y: 200 }) + const ghost = store.getState().pendingPlacement!.freeGhost + expect(ghost).not.toBeNull() + expect( + rectsOverlap( + { origin: ghost!.point, size: ghost!.size }, + { origin: { x: 0, y: 0 }, size: SEED_SIZE }, + ), + ).toBe(false) + + // Re-sending the same cursor position must not churn state objects. + const before = store.getState().pendingPlacement + store.getState().updatePlacementCursor({ x: 320, y: 200 }) + expect(store.getState().pendingPlacement).toBe(before) + + // Disarming clears the ghost preview. + store.getState().setFreeArmed(false) + expect(store.getState().pendingPlacement!.freeArmed).toBe(false) + expect(store.getState().pendingPlacement!.freeGhost).toBeNull() + }) + + it('commitFreePlacement places once at the nudged spot, restores zoom, and ends the transaction', () => { + const { store } = storeWithSeed(1.5, { x: 0, y: 0 }) + const onCancelled = vi.fn() + store.getState().beginPlacement('p2', 'terminal', onCancelled) + + // Click directly on top of the seed node — the drop must be nudged free. + const nodeId = store.getState().commitFreePlacement({ x: 320, y: 200 }) + + expect(nodeId).not.toBeNull() + const s = store.getState() + expect(s.pendingPlacement).toBeNull() + expect(nodeCount(store)).toBe(2) + const placed = s.nodes[nodeId!] + expect( + rectsOverlap( + { origin: placed.origin, size: placed.size }, + { origin: { x: 0, y: 0 }, size: SEED_SIZE }, + ), + ).toBe(false) + expect(s.zoomLevel).toBe(1.5) + expect(s.focusedNodeId).toBe(nodeId) + expect(onCancelled).not.toHaveBeenCalled() + // Transaction is closed: a second free commit does nothing. + expect(store.getState().commitFreePlacement({ x: 9000, y: 9000 })).toBeNull() + expect(nodeCount(store)).toBe(2) + }) + + it('free-mode actions are no-ops while no placement is pending', () => { + const { store } = storeWithSeed() + const before = store.getState() + + store.getState().setFreeArmed(true) + store.getState().updatePlacementCursor({ x: 50, y: 50 }) + store.getState().setPlacementHover(1) + + expect(store.getState().pendingPlacement).toBeNull() + expect(store.getState().nodes).toBe(before.nodes) + expect(store.getState().commitFreePlacement({ x: 50, y: 50 })).toBeNull() + }) +}) + +describe('setPlacementHover', () => { + it('sets and clears the hovered candidate without disturbing the rest of the transaction', () => { + const { store } = storeWithSeed() + store.getState().beginPlacement('p2', 'terminal') + const candidates = store.getState().pendingPlacement!.candidates + + store.getState().setPlacementHover(1) + expect(store.getState().pendingPlacement!.hoveredIndex).toBe(1) + expect(store.getState().pendingPlacement!.candidates).toBe(candidates) + + // Same value → no state churn. + const before = store.getState().pendingPlacement + store.getState().setPlacementHover(1) + expect(store.getState().pendingPlacement).toBe(before) + + store.getState().setPlacementHover(null) + expect(store.getState().pendingPlacement!.hoveredIndex).toBeNull() + }) +}) + +describe('re-entrant beginPlacement (placement pending → begin again)', () => { + it('rolls back the previous pending panel and replaces the transaction (latest wins)', () => { + const { store } = storeWithSeed(1.5, { x: 120, y: -40 }) + const firstCancelled = vi.fn() + const secondCancelled = vi.fn() + store.getState().beginPlacement('p2', 'terminal', firstCancelled) + const zoomDuringFirst = store.getState().zoomLevel + + store.getState().beginPlacement('p3', 'editor', secondCancelled) + + // First transaction's orphan panel was rolled back immediately. + expect(firstCancelled).toHaveBeenCalledTimes(1) + expect(firstCancelled).toHaveBeenCalledWith('p2') + expect(secondCancelled).not.toHaveBeenCalled() + const pending = store.getState().pendingPlacement! + expect(pending.panelId).toBe('p3') + expect(pending.panelType).toBe('editor') + + // BUG?: the replacement transaction snapshots the CURRENT (already + // zoomed-out) viewport rather than inheriting the first transaction's + // snapshot, so the user's original viewport (zoom 1.5, offset 120/-40) + // leaks: cancelling now restores the first placement's zoomed-out camera, + // not where the user actually was before any placement started. + expect(pending.prevZoom).toBe(zoomDuringFirst) + expect(pending.prevZoom).not.toBe(1.5) + store.getState().cancelPlacement() + expect(store.getState().zoomLevel).toBe(zoomDuringFirst) + expect(store.getState().zoomLevel).not.toBe(1.5) + expect(secondCancelled).toHaveBeenCalledTimes(1) + }) + + it('re-triggering with the SAME panel id does not roll the panel back', () => { + const { store } = storeWithSeed() + const onCancelled = vi.fn() + store.getState().beginPlacement('p2', 'terminal', onCancelled) + + store.getState().beginPlacement('p2', 'terminal', onCancelled) + + expect(onCancelled).not.toHaveBeenCalled() + expect(store.getState().pendingPlacement!.panelId).toBe('p2') + // The panel is still placeable exactly once. + expect(store.getState().commitPlacement(0)).not.toBeNull() + expect(nodeCount(store)).toBe(2) + }) +}) diff --git a/src/renderer/stores/canvasStore.lifecycle.test.ts b/src/renderer/stores/canvasStore.lifecycle.test.ts new file mode 100644 index 00000000..dda4e706 --- /dev/null +++ b/src/renderer/stores/canvasStore.lifecycle.test.ts @@ -0,0 +1,546 @@ +// ============================================================================= +// Canvas store — end-to-end lifecycle tests across the composed slices: node +// CRUD + focus/z-order, viewport math (zoom clamping, anchor-preserving zoom, +// fit/selection zoom), undo/redo through a bulk delete, spatial keyboard +// navigation, selection arrangement (stack/tidy/auto-layout), and the +// loadWorkspaceCanvas session round-trip. These intentionally drive several +// slices in one scenario — the bugs this guards against live at the seams. +// ============================================================================= + +import { describe, it, expect, vi } from 'vitest' + +// deleteSelection routes panel closure through a lazily-imported appStore. +const closePanel = vi.fn() +vi.mock('./appStore', () => ({ + useAppStore: { + getState: () => ({ selectedWorkspaceId: 'ws-1', closePanel }), + }, +})) + +import { createCanvasStore } from './canvasStore' +import { ZOOM_MIN, ZOOM_MAX } from '../../shared/types' +import type { CanvasNodeState } from '../../shared/types' + +const SIZE = { width: 200, height: 200 } + +// Positions are grid-multiples (CANVAS_GRID_SIZE = 20) and far apart, so +// findFreePosition returns them verbatim instead of nudging. +function addThree(store: ReturnType) { + const a = store.getState().addNode('panel-a', 'terminal', { x: 0, y: 0 }, SIZE) + const b = store.getState().addNode('panel-b', 'editor', { x: 1000, y: 0 }, SIZE) + const c = store.getState().addNode('panel-c', 'terminal', { x: 2000, y: 0 }, SIZE) + return { a, b, c } +} + +function nodesOverlap(a: CanvasNodeState, b: CanvasNodeState): boolean { + return ( + a.origin.x < b.origin.x + b.size.width && + b.origin.x < a.origin.x + a.size.width && + a.origin.y < b.origin.y + b.size.height && + b.origin.y < a.origin.y + a.size.height + ) +} + +async function flushMicrotasks() { + await new Promise((resolve) => setTimeout(resolve, 0)) +} + +describe('node lifecycle', () => { + it('creates nodes at the requested origins with monotonic creation/z counters', () => { + const store = createCanvasStore() + const { a, b, c } = addThree(store) + + const nodes = store.getState().nodes + expect(nodes[a].origin).toEqual({ x: 0, y: 0 }) + expect(nodes[b].origin).toEqual({ x: 1000, y: 0 }) + expect(nodes[c].origin).toEqual({ x: 2000, y: 0 }) + expect([nodes[a], nodes[b], nodes[c]].map((n) => n.creationIndex)).toEqual([0, 1, 2]) + expect(nodes[b].zOrder).toBeGreaterThan(nodes[a].zOrder) + expect(nodes[c].zOrder).toBeGreaterThan(nodes[b].zOrder) + // Each node is seeded with a single-tab mini-dock for its panel. + expect(nodes[a].dockLayout).toMatchObject({ type: 'tabs', panelIds: ['panel-a'], activeIndex: 0 }) + }) + + it('refuses canvas-on-canvas at the data layer', () => { + const store = createCanvasStore() + const id = store.getState().addNode('nested-canvas', 'canvas', { x: 0, y: 0 }, SIZE) + expect(id).toBe('') + expect(Object.keys(store.getState().nodes)).toHaveLength(0) + }) + + it('focusNode raises the node, bumps focusEpoch, and ends keyboard-nav mode', () => { + const store = createCanvasStore() + const { a, b } = addThree(store) + store.getState().panViewport('down') // sets suppressAutoFocus + expect(store.getState().suppressAutoFocus).toBe(true) + const epochBefore = store.getState().focusEpoch + const topBefore = store.getState().nodes[b].zOrder + + store.getState().focusNode(a) + + const s = store.getState() + expect(s.focusedNodeId).toBe(a) + expect(s.focusEpoch).toBe(epochBefore + 1) + expect(s.nodes[a].zOrder).toBeGreaterThan(topBefore) + expect(s.suppressAutoFocus).toBe(false) + + // Re-focusing the same node still bumps the epoch (panels re-run side effects). + store.getState().focusNode(a) + expect(store.getState().focusEpoch).toBe(epochBefore + 2) + }) + + it('removeNode marks exiting + drops focus; finalizeRemoveNode deletes', () => { + const store = createCanvasStore() + const { a } = addThree(store) + store.getState().focusNode(a) + + store.getState().removeNode(a) + expect(store.getState().nodes[a].animationState).toBe('exiting') + expect(store.getState().focusedNodeId).toBeNull() + + store.getState().finalizeRemoveNode(a) + expect(store.getState().nodes[a]).toBeUndefined() + }) + + it('moveToBack places a node strictly below the current minimum', () => { + const store = createCanvasStore() + const { a, b, c } = addThree(store) + + store.getState().moveToBack(c) + + const nodes = store.getState().nodes + expect(nodes[c].zOrder).toBeLessThan(nodes[a].zOrder) + expect(nodes[c].zOrder).toBeLessThan(nodes[b].zOrder) + + store.getState().moveToFront(c) + expect(nodes[a].zOrder).toBeLessThan(store.getState().nodes[c].zOrder) + }) + + it('cycles nextNode/previousNode in creation order with wrap-around', () => { + const store = createCanvasStore() + const { a, b, c } = addThree(store) + + expect(store.getState().nextNode()).toBe(a) // nothing focused → first + store.getState().focusNode(a) + expect(store.getState().nextNode()).toBe(b) + store.getState().focusNode(c) + expect(store.getState().nextNode()).toBe(a) // wraps + expect(store.getState().previousNode()).toBe(b) + store.getState().focusNode(a) + expect(store.getState().previousNode()).toBe(c) // wraps backwards + }) +}) + +describe('viewport math', () => { + it('canvasToView/viewToCanvas are exact inverses under the live zoom/offset', () => { + const store = createCanvasStore() + store.getState().setZoomAndOffset(2, { x: 100, y: 50 }) + + expect(store.getState().canvasToView({ x: 10, y: 10 })).toEqual({ x: 120, y: 70 }) + const roundTrip = store.getState().viewToCanvas(store.getState().canvasToView({ x: 37, y: -12 })) + expect(roundTrip.x).toBeCloseTo(37) + expect(roundTrip.y).toBeCloseTo(-12) + }) + + it('clamps zoom everywhere it can be set', () => { + const store = createCanvasStore() + store.getState().setZoom(100) + expect(store.getState().zoomLevel).toBe(ZOOM_MAX) + store.getState().setZoom(0.0001) + expect(store.getState().zoomLevel).toBe(ZOOM_MIN) + store.getState().setZoomAndOffset(50, { x: 0, y: 0 }) + expect(store.getState().zoomLevel).toBe(ZOOM_MAX) + store.getState().setContainerSize({ width: 1200, height: 800 }) + store.getState().zoomAroundCenter(0.0001) + expect(store.getState().zoomLevel).toBe(ZOOM_MIN) + }) + + it('zoomAroundCenter keeps the canvas point under the view center fixed', () => { + const store = createCanvasStore() + store.getState().setContainerSize({ width: 1200, height: 800 }) + store.getState().setZoomAndOffset(0.8, { x: -340, y: 220 }) + const anchorBefore = store.getState().viewToCanvas({ x: 600, y: 400 }) + + store.getState().zoomAroundCenter(2) + + expect(store.getState().zoomLevel).toBe(2) + const anchorAfter = store.getState().viewToCanvas({ x: 600, y: 400 }) + expect(anchorAfter.x).toBeCloseTo(anchorBefore.x) + expect(anchorAfter.y).toBeCloseTo(anchorBefore.y) + }) + + it('viewFrame reports the node rect scaled into view space', () => { + const store = createCanvasStore() + const id = store.getState().addNode('p', 'editor', { x: 100, y: 40 }, { width: 300, height: 200 }) + store.getState().setZoomAndOffset(2, { x: 10, y: 20 }) + + expect(store.getState().viewFrame(id)).toEqual({ + origin: { x: 210, y: 100 }, + size: { width: 600, height: 400 }, + }) + expect(store.getState().viewFrame('missing')).toBeNull() + }) + + it('zoomToFit brings every node fully on screen', () => { + const store = createCanvasStore() + store.getState().addNode('p1', 'editor', { x: 0, y: 0 }, { width: 400, height: 300 }) + store.getState().addNode('p2', 'editor', { x: 1000, y: 800 }, { width: 400, height: 300 }) + store.getState().setContainerSize({ width: 1200, height: 800 }) + + store.getState().zoomToFit() + + const topLeft = store.getState().canvasToView({ x: 0, y: 0 }) + const bottomRight = store.getState().canvasToView({ x: 1400, y: 1100 }) + expect(topLeft.x).toBeGreaterThanOrEqual(0) + expect(topLeft.y).toBeGreaterThanOrEqual(0) + expect(bottomRight.x).toBeLessThanOrEqual(1200) + expect(bottomRight.y).toBeLessThanOrEqual(800) + }) + + it('zoomToSelection caps a single-node target at 1.5x', () => { + const store = createCanvasStore() + const id = store.getState().addNode('p1', 'editor', { x: 0, y: 0 }, { width: 200, height: 100 }) + store.getState().setContainerSize({ width: 1200, height: 800 }) + store.getState().selectNodes([id]) + + store.getState().zoomToSelection() + + expect(store.getState().zoomLevel).toBe(1.5) + }) + + it('toggleMaximize fills the visible canvas and restores exactly on toggle back', () => { + const store = createCanvasStore() + const id = store.getState().addNode('p', 'editor', { x: 100, y: 100 }, { width: 300, height: 200 }) + store.getState().setContainerSize({ width: 1200, height: 800 }) + store.getState().setZoomAndOffset(1, { x: 0, y: 0 }) + + store.getState().toggleMaximize(id, { width: 1200, height: 800 }) + const maxed = store.getState().nodes[id] + expect(maxed.origin).toEqual({ x: 20, y: 20 }) + expect(maxed.size).toEqual({ width: 1160, height: 760 }) + expect(maxed.preMaximizeOrigin).toEqual({ x: 100, y: 100 }) + expect(store.getState().focusedNodeId).toBe(id) + + store.getState().toggleMaximize(id, { width: 1200, height: 800 }) + const restored = store.getState().nodes[id] + expect(restored.origin).toEqual({ x: 100, y: 100 }) + expect(restored.size).toEqual({ width: 300, height: 200 }) + expect(restored.preMaximizeOrigin).toBeUndefined() + expect(restored.preMaximizeSize).toBeUndefined() + }) +}) + +describe('undo/redo across a bulk delete', () => { + it('undo restores deleted nodes + selection; redo reapplies the delete', async () => { + const store = createCanvasStore() + const { a, b } = addThree(store) + store.getState().selectNodes([a, b]) + + store.getState().deleteSelection() + await flushMicrotasks() + expect(closePanel).toHaveBeenCalledWith('ws-1', 'panel-a') + expect(closePanel).toHaveBeenCalledWith('ws-1', 'panel-b') + expect(store.getState().nodes[a].animationState).toBe('exiting') + expect(store.getState().nodes[b].animationState).toBe('exiting') + expect(store.getState().selectedNodeIds.size).toBe(0) + + // deleteSelection pushes once + once per removeNode → two undos rewind it. + store.getState().undo() + store.getState().undo() + expect(store.getState().nodes[a].animationState).not.toBe('exiting') + expect(store.getState().nodes[b].animationState).not.toBe('exiting') + expect([...store.getState().selectedNodeIds].sort()).toEqual([a, b].sort()) + + store.getState().redo() + store.getState().redo() + expect(store.getState().nodes[a].animationState).toBe('exiting') + expect(store.getState().nodes[b].animationState).toBe('exiting') + expect(store.getState().selectedNodeIds.size).toBe(0) + }) + + it('a new mutation clears the redo stack', () => { + const store = createCanvasStore() + const { a } = addThree(store) + store.getState().removeNode(a) + store.getState().undo() + expect(store.getState().future.length).toBeGreaterThan(0) + + store.getState().addNode('panel-d', 'terminal', { x: 3000, y: 0 }, SIZE) + + expect(store.getState().future).toHaveLength(0) + store.getState().redo() // no-op + expect(store.getState().nodes[a]).toBeDefined() + }) + + it('selection restored by undo never references nodes absent from that snapshot', () => { + const store = createCanvasStore() + const { a } = addThree(store) + // History entry captured BEFORE d exists, while d is selected later. + const d = store.getState().addNode('panel-d', 'terminal', { x: 3000, y: 0 }, SIZE) + store.getState().selectNodes([a, d]) + store.getState().removeNode(a) + + store.getState().undo() // restores pre-remove snapshot + store.getState().undo() // restores pre-add-d snapshot (selection had nothing yet) + + const s = store.getState() + for (const id of s.selectedNodeIds) { + expect(s.nodes[id], `selected id ${id} must exist`).toBeDefined() + } + expect(s.nodes[d]).toBeUndefined() + }) +}) + +describe('spatial keyboard navigation', () => { + function setupRow(store: ReturnType) { + const ids = addThree(store) // x = 0 / 1000 / 2000, all 200x200 at y=0 + store.getState().setContainerSize({ width: 1200, height: 800 }) + store.getState().setZoomAndOffset(1, { x: 0, y: 0 }) + return ids + } + + it('navigateDirection focuses the nearest node in the cone and centers it', () => { + const store = createCanvasStore() + const { a, b, c } = setupRow(store) + store.getState().focusNode(a) + + store.getState().navigateDirection('right') + expect(store.getState().focusedNodeId).toBe(b) + // focusAndCenter: node center (1100, 100) maps to the container center. + expect(store.getState().viewportOffset).toEqual({ x: 600 - 1100, y: 400 - 100 }) + + store.getState().navigateDirection('right') + expect(store.getState().focusedNodeId).toBe(c) + store.getState().navigateDirection('left') + expect(store.getState().focusedNodeId).toBe(b) + }) + + it('ignores candidates outside the directional cone', () => { + const store = createCanvasStore() + const a = store.getState().addNode('panel-a', 'terminal', { x: 0, y: 0 }, SIZE) + // Mostly below-right: |dy| > |dx| from a's center, so not "right" of it. + store.getState().addNode('panel-d', 'terminal', { x: 400, y: 1200 }, SIZE) + store.getState().setContainerSize({ width: 1200, height: 800 }) + store.getState().focusNode(a) + + store.getState().navigateDirection('right') + + expect(store.getState().focusedNodeId).toBe(a) + }) + + it('navigateSelect moves the selection cursor without grabbing focus', () => { + const store = createCanvasStore() + const { a, b } = setupRow(store) + store.getState().focusNode(a) + + store.getState().navigateSelect('right') + + const s = store.getState() + expect([...s.selectedNodeIds]).toEqual([b]) + expect(s.focusedNodeId).toBeNull() + expect(s.suppressAutoFocus).toBe(true) + // No rAF in this environment → the viewport tween applies instantly. + expect(s.viewportOffset).toEqual({ x: 600 - 1100, y: 400 - 100 }) + }) + + it('panViewport steps the camera and a manual pan re-enables auto-focus', () => { + const store = createCanvasStore() + store.getState().setContainerSize({ width: 1200, height: 800 }) + + store.getState().panViewport('down') + expect(store.getState().viewportOffset).toEqual({ x: 0, y: -120 }) + expect(store.getState().suppressAutoFocus).toBe(true) + + store.getState().panViewport('right') + expect(store.getState().viewportOffset).toEqual({ x: -120, y: -120 }) + + store.getState().setViewportOffset({ x: 5, y: 5 }) + expect(store.getState().suppressAutoFocus).toBe(false) + }) +}) + +describe('selection arrangement', () => { + it('stackSelected(row) lines nodes up left-to-right with the gap, anchored at the selection origin', () => { + const store = createCanvasStore() + const n1 = store.getState().addNode('p1', 'editor', { x: 0, y: 0 }, { width: 200, height: 100 }) + const n2 = store.getState().addNode('p2', 'editor', { x: 500, y: 40 }, { width: 300, height: 100 }) + const n3 = store.getState().addNode('p3', 'editor', { x: 1000, y: 80 }, { width: 250, height: 100 }) + store.getState().selectNodes([n1, n2, n3]) + + store.getState().stackSelected('row', 16) + + const nodes = store.getState().nodes + expect(nodes[n1].origin).toEqual({ x: 0, y: 0 }) + expect(nodes[n2].origin).toEqual({ x: 216, y: 0 }) + expect(nodes[n3].origin).toEqual({ x: 532, y: 0 }) + }) + + it('tidyGridSelected packs mixed-size nodes into a non-overlapping grid', () => { + const store = createCanvasStore() + const ids = [ + store.getState().addNode('p1', 'editor', { x: 0, y: 0 }, { width: 200, height: 100 }), + store.getState().addNode('p2', 'editor', { x: 600, y: 0 }, { width: 300, height: 150 }), + store.getState().addNode('p3', 'editor', { x: 0, y: 600 }, { width: 250, height: 120 }), + store.getState().addNode('p4', 'editor', { x: 600, y: 600 }, { width: 180, height: 90 }), + ] + store.getState().selectNodes(ids) + + store.getState().tidyGridSelected(16) + + const list = ids.map((id) => store.getState().nodes[id]) + for (let i = 0; i < list.length; i++) { + for (let j = i + 1; j < list.length; j++) { + expect(nodesOverlap(list[i], list[j]), `nodes ${i} and ${j} overlap`).toBe(false) + } + } + // 2-column grid anchored at the selection's top-left, cells = max w/h. + expect(store.getState().nodes[ids[0]].origin).toEqual({ x: 0, y: 0 }) + expect(store.getState().nodes[ids[1]].origin).toEqual({ x: 316, y: 0 }) + }) + + it('autoLayout produces a non-overlapping grid for the whole canvas', () => { + const store = createCanvasStore() + for (let i = 0; i < 5; i++) { + store.getState().addNode(`p${i}`, 'editor', { x: i * 40, y: i * 40 }, { width: 600, height: 400 }) + } + store.getState().setContainerSize({ width: 1600, height: 1000 }) + + store.getState().autoLayout() + + const list = Object.values(store.getState().nodes) + for (let i = 0; i < list.length; i++) { + for (let j = i + 1; j < list.length; j++) { + expect(nodesOverlap(list[i], list[j]), `nodes ${i} and ${j} overlap`).toBe(false) + } + } + // autoLayout is undoable. + store.getState().undo() + expect(store.getState().nodes[list[0].id]).toBeDefined() + }) +}) + +describe('degenerate inputs and limits', () => { + it('mutations on unknown node ids are no-ops and do not pollute history or focus', () => { + const store = createCanvasStore() + store.getState().addNode('panel-a', 'terminal', { x: 0, y: 0 }, SIZE) + const nodesBefore = store.getState().nodes + const historyLen = store.getState().history.length + const epochBefore = store.getState().focusEpoch + + store.getState().moveNode('ghost', { x: 9, y: 9 }) + store.getState().resizeNode('ghost', { width: 9, height: 9 }) + store.getState().focusNode('ghost') + store.getState().removeNode('ghost') + store.getState().togglePin('ghost') + store.getState().moveToFront('ghost') + store.getState().setNodeAnimationState('ghost', 'idle') + store.getState().toggleMaximize('ghost', { width: 100, height: 100 }) + + expect(store.getState().nodes).toEqual(nodesBefore) + expect(store.getState().history).toHaveLength(historyLen) + expect(store.getState().focusedNodeId).toBeNull() + expect(store.getState().focusEpoch).toBe(epochBefore) + }) + + it('undo/redo on empty stacks are no-ops', () => { + const store = createCanvasStore() + const { a } = addThree(store) + + store.getState().redo() // future empty + expect(store.getState().nodes[a]).toBeDefined() + + // Drain history past the bottom — extra undos must not throw or corrupt. + for (let i = 0; i < 10; i++) store.getState().undo() + expect(Object.keys(store.getState().nodes)).toHaveLength(0) // back to empty canvas + expect(store.getState().history).toHaveLength(0) + }) + + it('zoomToFit / zoomToSelection without nodes or a measured container are no-ops', () => { + const store = createCanvasStore() + store.getState().setContainerSize({ width: 1200, height: 800 }) + store.getState().zoomToFit() // no nodes + store.getState().zoomToSelection() // no nodes, no selection + expect(store.getState().zoomLevel).toBe(1) + expect(store.getState().viewportOffset).toEqual({ x: 0, y: 0 }) + + const unmeasured = createCanvasStore() // containerSize still 0x0 + unmeasured.getState().addNode('p', 'editor', { x: 0, y: 0 }, SIZE) + unmeasured.getState().zoomToFit() + expect(unmeasured.getState().zoomLevel).toBe(1) + }) + + it('caps undo history at 100 entries', () => { + const store = createCanvasStore() + store.getState().addNode('panel-a', 'terminal', { x: 0, y: 0 }, SIZE) + + for (let i = 0; i < 150; i++) store.getState().pushHistory() + + expect(store.getState().history).toHaveLength(100) + for (let i = 0; i < 110; i++) store.getState().undo() // drains without error + expect(store.getState().history).toHaveLength(0) + }) + + it('nextNode/previousNode fall back to the ends when the focused id is stale', () => { + const store = createCanvasStore() + const { a, b, c } = addThree(store) + store.getState().focusNode(a) + // finalizeRemoveNode without removeNode leaves focusedNodeId dangling — the + // queries must still answer from the live node set. + store.getState().finalizeRemoveNode(a) + + expect(store.getState().nextNode()).toBe(b) + expect(store.getState().previousNode()).toBe(c) + }) +}) + +describe('loadWorkspaceCanvas session round-trip', () => { + it('restores geometry exactly through JSON serialization, resets transients, clamps zoom', () => { + const source = createCanvasStore() + const { a, b, c } = addThree(source) + source.getState().moveNode(b, { x: 1500, y: 300 }) + source.getState().resizeNode(c, { width: 640, height: 480 }) + source.getState().focusNode(a) + source.getState().selectNodes([a, b]) + source.getState().setNodeDockLayout(a, { + type: 'tabs', + id: 'stack-1', + panelIds: ['panel-a', 'panel-extra'], + activeIndex: 1, + }) + + // What sessionSave persists: the nodes record + viewport, through JSON. + const persisted = JSON.parse( + JSON.stringify({ + nodes: source.getState().nodes, + viewportOffset: { x: -250, y: 80 }, + zoomLevel: 99, // corrupt/out-of-range value on disk + }), + ) + + const restored = createCanvasStore() + restored.getState().loadWorkspaceCanvas(persisted.nodes, persisted.viewportOffset, persisted.zoomLevel) + + const s = restored.getState() + expect(s.zoomLevel).toBe(ZOOM_MAX) + expect(s.viewportOffset).toEqual({ x: -250, y: 80 }) + // Geometry and per-node dock layout survive byte-for-byte. + expect(s.nodes[b].origin).toEqual({ x: 1500, y: 300 }) + expect(s.nodes[c].size).toEqual({ width: 640, height: 480 }) + expect(s.nodes[a].dockLayout).toEqual({ + type: 'tabs', + id: 'stack-1', + panelIds: ['panel-a', 'panel-extra'], + activeIndex: 1, + }) + // Transients reset: no focus/selection/history, nothing animates on restore. + expect(s.focusedNodeId).toBeNull() + expect(s.selectedNodeIds.size).toBe(0) + expect(s.history).toHaveLength(0) + expect(Object.values(s.nodes).every((n) => n.animationState === 'idle')).toBe(true) + // Counters resume past the loaded maxima — new nodes stack on top. + const d = restored.getState().addNode('panel-d', 'terminal', { x: 4000, y: 0 }, SIZE) + const maxLoadedZ = Math.max(...[a, b, c].map((id) => s.nodes[id].zOrder)) + expect(restored.getState().nodes[d].zOrder).toBeGreaterThan(maxLoadedZ) + expect(restored.getState().nodes[d].creationIndex).toBe(3) + }) +}) diff --git a/src/renderer/stores/dockStore.test.ts b/src/renderer/stores/dockStore.test.ts new file mode 100644 index 00000000..d229d5d5 --- /dev/null +++ b/src/renderer/stores/dockStore.test.ts @@ -0,0 +1,738 @@ +// ============================================================================= +// Dock store — end-to-end tests for the dock-zone layout tree: panel placement +// (append / tab-insert / edge splits), flat 3-way splits, the duplicate guard, +// undocking with split collapse + ratio re-normalization, tab moves within and +// across zones, split resizing, stack collapse, derived panel locations, and +// snapshot round-trips. Structural invariants (no duplicate panels, ratios sum +// to 1, valid active tab indices) are re-checked after every mutation. +// ============================================================================= + +import { describe, it, expect, beforeEach } from 'vitest' +import { + createDockStore, + createDefaultDockState, + DEFAULT_SIDE_ZONE_SIZE, + DEFAULT_BOTTOM_ZONE_SIZE, +} from './dockStore' +import { setActivePanel, getActivePanelId } from '../lib/activePanel' +import type { + DockLayoutNode, + DockSplitNode, + DockTabStack, + DockZonePosition, + WindowDockState, +} from '../../shared/types' +import { ALL_ZONES } from '../../shared/types' + +type Store = ReturnType + +// --- tree helpers ----------------------------------------------------------- + +function collectStacks(node: DockLayoutNode | null, out: DockTabStack[] = []): DockTabStack[] { + if (!node) return out + if (node.type === 'tabs') { + out.push(node) + return out + } + for (const child of node.children) collectStacks(child, out) + return out +} + +function collectSplits(node: DockLayoutNode | null, out: DockSplitNode[] = []): DockSplitNode[] { + if (!node || node.type === 'tabs') return out + out.push(node) + for (const child of node.children) collectSplits(child, out) + return out +} + +/** Structural invariants every dock tree must satisfy at all times. */ +function expectTreeInvariants(zones: WindowDockState) { + const seen = new Set() + for (const pos of ALL_ZONES) { + const layout = zones[pos].layout + for (const stack of collectStacks(layout)) { + expect(stack.panelIds.length).toBeGreaterThan(0) + expect(stack.activeIndex).toBeGreaterThanOrEqual(0) + expect(stack.activeIndex).toBeLessThan(stack.panelIds.length) + for (const id of stack.panelIds) { + expect(seen.has(id), `panel ${id} appears twice in the dock tree`).toBe(false) + seen.add(id) + } + } + for (const split of collectSplits(layout)) { + expect(split.children.length).toBeGreaterThanOrEqual(2) + expect(split.ratios).toHaveLength(split.children.length) + expect(split.ratios.reduce((a, b) => a + b, 0)).toBeCloseTo(1, 6) + } + } +} + +function zoneLayout(store: Store, zone: DockZonePosition): DockLayoutNode | null { + return store.getState().zones[zone].layout +} + +function rootStack(store: Store, zone: DockZonePosition): DockTabStack { + const layout = zoneLayout(store, zone) + if (!layout || layout.type !== 'tabs') throw new Error(`zone ${zone} root is not a tab stack`) + return layout +} + +function rootSplit(store: Store, zone: DockZonePosition): DockSplitNode { + const layout = zoneLayout(store, zone) + if (!layout || layout.type !== 'split') throw new Error(`zone ${zone} root is not a split`) + return layout +} + +function stackContaining(store: Store, panelId: string): DockTabStack { + for (const pos of ALL_ZONES) { + const found = collectStacks(zoneLayout(store, pos)).find((s) => s.panelIds.includes(panelId)) + if (found) return found + } + throw new Error(`no stack contains ${panelId}`) +} + +// --- tests -------------------------------------------------------------------- + +describe('createDefaultDockState', () => { + it('creates hidden side zones with default sizes and a visible center', () => { + const zones = createDefaultDockState() + expect(zones.left.visible).toBe(false) + expect(zones.right.visible).toBe(false) + expect(zones.bottom.visible).toBe(false) + expect(zones.center.visible).toBe(true) + expect(zones.left.size).toBe(DEFAULT_SIDE_ZONE_SIZE) + expect(zones.right.size).toBe(DEFAULT_SIDE_ZONE_SIZE) + expect(zones.bottom.size).toBe(DEFAULT_BOTTOM_ZONE_SIZE) + expect(zones.left.layout).toBeNull() + expect(zones.center.layout).toBeNull() + }) +}) + +describe('zone visibility and sizing', () => { + it('toggleZone flips visibility', () => { + const store = createDockStore() + store.getState().toggleZone('bottom') + expect(store.getState().zones.bottom.visible).toBe(true) + store.getState().toggleZone('bottom') + expect(store.getState().zones.bottom.visible).toBe(false) + }) + + it('setZoneSize clamps to the minimum zone size', () => { + const store = createDockStore() + store.getState().setZoneSize('left', 500) + expect(store.getState().zones.left.size).toBe(500) + store.getState().setZoneSize('left', 10) + expect(store.getState().zones.left.size).toBe(120) + }) +}) + +describe('dockPanel — default placement', () => { + it('creates a root tab stack in an empty zone and auto-shows the zone', () => { + const store = createDockStore() + store.getState().dockPanel('term-1', 'bottom') + + const stack = rootStack(store, 'bottom') + expect(stack.panelIds).toEqual(['term-1']) + expect(stack.activeIndex).toBe(0) + expect(store.getState().zones.bottom.visible).toBe(true) + expectTreeInvariants(store.getState().zones) + }) + + it('appends subsequent panels to the root stack and activates the new tab', () => { + const store = createDockStore() + store.getState().dockPanel('term-1', 'bottom') + store.getState().dockPanel('term-2', 'bottom') + store.getState().dockPanel('term-3', 'bottom') + + const stack = rootStack(store, 'bottom') + expect(stack.panelIds).toEqual(['term-1', 'term-2', 'term-3']) + expect(stack.activeIndex).toBe(2) + expectTreeInvariants(store.getState().zones) + }) + + it('re-docking a panel already in the zone moves it instead of duplicating it', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + store.getState().dockPanel('a', 'bottom') // again, no target + + const stack = rootStack(store, 'bottom') + expect(stack.panelIds).toEqual(['b', 'a']) + expectTreeInvariants(store.getState().zones) + }) + + it('when the root is a split, default placement appends to the first stack', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + + store.getState().dockPanel('c', 'bottom') // no target — root is now a split + + const first = collectStacks(zoneLayout(store, 'bottom'))[0] + expect(first.panelIds).toEqual(['a', 'c']) + expectTreeInvariants(store.getState().zones) + }) +}) + +describe('dockPanel — tab targets', () => { + it('inserts at the requested index and activates it', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + const stack = rootStack(store, 'bottom') + + store.getState().dockPanel('c', 'bottom', { type: 'tab', stackId: stack.id, index: 1 }) + + const updated = rootStack(store, 'bottom') + expect(updated.panelIds).toEqual(['a', 'c', 'b']) + expect(updated.activeIndex).toBe(1) + expectTreeInvariants(store.getState().zones) + }) + + it('a tab target whose stack no longer exists falls back to zone append, not dropping the panel', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + + store.getState().dockPanel('b', 'bottom', { type: 'tab', stackId: 'stack-gone', index: 0 }) + + const stack = rootStack(store, 'bottom') + expect(stack.panelIds).toEqual(['a', 'b']) + expectTreeInvariants(store.getState().zones) + }) +}) + +describe('dockPanel — split targets', () => { + it('edge:right creates a horizontal split [existing, new] at 50/50', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + + const split = rootSplit(store, 'bottom') + expect(split.direction).toBe('horizontal') + expect(split.ratios).toEqual([0.5, 0.5]) + const [left, right] = split.children as DockTabStack[] + expect(left.panelIds).toEqual(['a']) + expect(right.panelIds).toEqual(['b']) + expectTreeInvariants(store.getState().zones) + }) + + it('edge:left places the new stack before the existing one', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'left' }) + + const split = rootSplit(store, 'bottom') + const [first, second] = split.children as DockTabStack[] + expect(first.panelIds).toEqual(['b']) + expect(second.panelIds).toEqual(['a']) + expectTreeInvariants(store.getState().zones) + }) + + it('edge:bottom creates a vertical split', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'left') + const stackA = rootStack(store, 'left') + + store.getState().dockPanel('b', 'left', { type: 'split', stackId: stackA.id, edge: 'bottom' }) + + const split = rootSplit(store, 'left') + expect(split.direction).toBe('vertical') + const [top, bottom] = split.children as DockTabStack[] + expect(top.panelIds).toEqual(['a']) + expect(bottom.panelIds).toEqual(['b']) + expectTreeInvariants(store.getState().zones) + }) + + it('splitting again in the SAME direction stays flat (3-way split, no nesting)', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + const stackB = stackContaining(store, 'b') + + store.getState().dockPanel('c', 'bottom', { type: 'split', stackId: stackB.id, edge: 'right' }) + + const split = rootSplit(store, 'bottom') + expect(split.children).toHaveLength(3) + expect(split.children.every((c) => c.type === 'tabs')).toBe(true) + expect((split.children as DockTabStack[]).map((s) => s.panelIds[0])).toEqual(['a', 'b', 'c']) + // The new stack takes half of the sibling it split from. + expect(split.ratios[0]).toBeCloseTo(0.5) + expect(split.ratios[1]).toBeCloseTo(0.25) + expect(split.ratios[2]).toBeCloseTo(0.25) + expectTreeInvariants(store.getState().zones) + }) + + it('splitting in the PERPENDICULAR direction nests a split', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + const stackB = stackContaining(store, 'b') + + store.getState().dockPanel('c', 'bottom', { type: 'split', stackId: stackB.id, edge: 'bottom' }) + + const root = rootSplit(store, 'bottom') + expect(root.direction).toBe('horizontal') + expect(root.children).toHaveLength(2) + const nested = root.children[1] as DockSplitNode + expect(nested.type).toBe('split') + expect(nested.direction).toBe('vertical') + expect((nested.children as DockTabStack[]).map((s) => s.panelIds[0])).toEqual(['b', 'c']) + expectTreeInvariants(store.getState().zones) + }) +}) + +describe('undockPanel', () => { + it('removing the last panel empties the layout and auto-hides a side zone', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().undockPanel('a') + + expect(zoneLayout(store, 'bottom')).toBeNull() + expect(store.getState().zones.bottom.visible).toBe(false) + }) + + it('never hides the center zone even when emptied', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'center') + store.getState().undockPanel('a') + + expect(zoneLayout(store, 'center')).toBeNull() + expect(store.getState().zones.center.visible).toBe(true) + }) + + it('collapses a 2-way split back to a single tab stack', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + + store.getState().undockPanel('b') + + const layout = zoneLayout(store, 'bottom')! + expect(layout.type).toBe('tabs') + expect((layout as DockTabStack).panelIds).toEqual(['a']) + expectTreeInvariants(store.getState().zones) + }) + + it('removing one child of a 3-way split re-normalizes the remaining ratios', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + const stackB = stackContaining(store, 'b') + store.getState().dockPanel('c', 'bottom', { type: 'split', stackId: stackB.id, edge: 'right' }) + // ratios are now [0.5, 0.25, 0.25] + + store.getState().undockPanel('b') + + const split = rootSplit(store, 'bottom') + expect(split.children).toHaveLength(2) + expect(split.ratios[0]).toBeCloseTo(0.5 / 0.75) + expect(split.ratios[1]).toBeCloseTo(0.25 / 0.75) + expectTreeInvariants(store.getState().zones) + }) + + it('clamps the active tab when the last tab is removed', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + store.getState().dockPanel('c', 'bottom') + expect(rootStack(store, 'bottom').activeIndex).toBe(2) + + store.getState().undockPanel('c') + + const stack = rootStack(store, 'bottom') + expect(stack.panelIds).toEqual(['a', 'b']) + expect(stack.activeIndex).toBe(1) + expectTreeInvariants(store.getState().zones) + }) + + it('is a no-op for a panel that is not docked anywhere', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const before = store.getState().zones + + store.getState().undockPanel('ghost') + + expect(store.getState().zones).toBe(before) + }) +}) + +describe('moveTab', () => { + function setupTwoStacks(store: Store): { from: DockTabStack; to: DockTabStack } { + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + const stackA = rootStack(store, 'bottom') + store.getState().dockPanel('c', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + return { from: stackContaining(store, 'a'), to: stackContaining(store, 'c') } + } + + it('moves a panel between stacks and activates it at the insert index', () => { + const store = createDockStore() + const { from, to } = setupTwoStacks(store) + + store.getState().moveTab('a', from.id, to.id, 0) + + expect(stackContaining(store, 'a').id).toBe(to.id) + const target = stackContaining(store, 'c') + expect(target.panelIds).toEqual(['a', 'c']) + expect(target.activeIndex).toBe(0) + const source = stackContaining(store, 'b') + expect(source.panelIds).toEqual(['b']) + expectTreeInvariants(store.getState().zones) + }) + + it('emptying the source stack removes it and collapses the split', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + const from = stackContaining(store, 'b') + const to = stackContaining(store, 'a') + + store.getState().moveTab('b', from.id, to.id) + + const layout = zoneLayout(store, 'bottom')! + expect(layout.type).toBe('tabs') + expect((layout as DockTabStack).panelIds).toEqual(['a', 'b']) + expectTreeInvariants(store.getState().zones) + }) + + it('moves a panel across zones', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + store.getState().dockPanel('x', 'left') + const from = rootStack(store, 'bottom') + const to = rootStack(store, 'left') + + store.getState().moveTab('a', from.id, to.id) + + expect(rootStack(store, 'bottom').panelIds).toEqual(['b']) + expect(rootStack(store, 'left').panelIds).toEqual(['x', 'a']) + expect(store.getState().getPanelLocation('a')).toEqual({ + type: 'dock', + zone: 'left', + stackId: to.id, + }) + expectTreeInvariants(store.getState().zones) + }) + + it('reorders within the same stack', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + store.getState().dockPanel('c', 'bottom') + const stack = rootStack(store, 'bottom') + + store.getState().moveTab('a', stack.id, stack.id, 2) + + expect(rootStack(store, 'bottom').panelIds).toEqual(['b', 'c', 'a']) + expectTreeInvariants(store.getState().zones) + }) +}) + +describe('setActiveTab', () => { + it('activates a valid index and ignores out-of-range ones', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + const stack = rootStack(store, 'bottom') + + store.getState().setActiveTab(stack.id, 0) + expect(rootStack(store, 'bottom').activeIndex).toBe(0) + + store.getState().setActiveTab(stack.id, 5) + expect(rootStack(store, 'bottom').activeIndex).toBe(0) + store.getState().setActiveTab(stack.id, -1) + expect(rootStack(store, 'bottom').activeIndex).toBe(0) + }) +}) + +describe('setSplitRatio', () => { + it('updates the ratios of the addressed split only', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stackA = rootStack(store, 'bottom') + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: stackA.id, edge: 'right' }) + const stackB = stackContaining(store, 'b') + store.getState().dockPanel('c', 'bottom', { type: 'split', stackId: stackB.id, edge: 'bottom' }) + const root = rootSplit(store, 'bottom') + const nested = root.children[1] as DockSplitNode + + store.getState().setSplitRatio(nested.id, [0.7, 0.3]) + + const after = rootSplit(store, 'bottom') + expect((after.children[1] as DockSplitNode).ratios).toEqual([0.7, 0.3]) + expect(after.ratios).toEqual([0.5, 0.5]) // untouched + expectTreeInvariants(store.getState().zones) + }) +}) + +describe('collapseStack', () => { + it('removes the whole stack, promotes the sibling, and clears a matching active panel', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + const stackAB = rootStack(store, 'bottom') + store.getState().dockPanel('c', 'bottom', { type: 'split', stackId: stackAB.id, edge: 'right' }) + + setActivePanel('b') + store.getState().collapseStack(stackAB.id) + + expect(getActivePanelId()).toBeNull() + const layout = zoneLayout(store, 'bottom')! + expect(layout.type).toBe('tabs') + expect((layout as DockTabStack).panelIds).toEqual(['c']) + expectTreeInvariants(store.getState().zones) + }) + + it('leaves an unrelated active panel alone and hides the zone when emptied', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'left') + const stack = rootStack(store, 'left') + + setActivePanel('other-panel') + store.getState().collapseStack(stack.id) + + expect(getActivePanelId()).toBe('other-panel') + expect(zoneLayout(store, 'left')).toBeNull() + expect(store.getState().zones.left.visible).toBe(false) + setActivePanel(null) + }) +}) + +describe('getPanelLocation', () => { + it('derives zone + stack from the tree and returns undefined for unknown panels', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('x', 'right') + + const locA = store.getState().getPanelLocation('a') + expect(locA).toEqual({ type: 'dock', zone: 'bottom', stackId: rootStack(store, 'bottom').id }) + const locX = store.getState().getPanelLocation('x') + expect(locX).toEqual({ type: 'dock', zone: 'right', stackId: rootStack(store, 'right').id }) + expect(store.getState().getPanelLocation('nope')).toBeUndefined() + }) +}) + +describe('snapshot round-trip', () => { + function buildComplexLayout(store: Store) { + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + const stackAB = rootStack(store, 'bottom') + store.getState().dockPanel('c', 'bottom', { type: 'split', stackId: stackAB.id, edge: 'right' }) + const stackC = stackContaining(store, 'c') + store.getState().dockPanel('d', 'bottom', { type: 'split', stackId: stackC.id, edge: 'bottom' }) + store.getState().dockPanel('e', 'left') + store.getState().setZoneSize('left', 320) + } + + it('getSnapshot derives one location per docked panel', () => { + const store = createDockStore() + buildComplexLayout(store) + + const snapshot = store.getState().getSnapshot() + + expect(Object.keys(snapshot.locations).sort()).toEqual(['a', 'b', 'c', 'd', 'e']) + for (const id of ['a', 'b', 'c', 'd', 'e']) { + expect(snapshot.locations[id]).toEqual(store.getState().getPanelLocation(id)) + } + }) + + it('restoreSnapshot reproduces the zones tree exactly in a fresh store', () => { + const store = createDockStore() + buildComplexLayout(store) + const snapshot = store.getState().getSnapshot() + + const restored = createDockStore() + restored.getState().restoreSnapshot(JSON.parse(JSON.stringify(snapshot))) + + expect(restored.getState().zones).toEqual(store.getState().zones) + expectTreeInvariants(restored.getState().zones) + // Derived lookups keep working on the restored tree. + expect(restored.getState().getPanelLocation('d')).toEqual(store.getState().getPanelLocation('d')) + }) + + it('createDockStore(initialState) seeds from a snapshot', () => { + const store = createDockStore() + buildComplexLayout(store) + const snapshot = store.getState().getSnapshot() + + const seeded = createDockStore(JSON.parse(JSON.stringify(snapshot))) + + expect(seeded.getState().zones).toEqual(store.getState().zones) + expect(seeded.getState().zones.left.size).toBe(320) + }) +}) + +describe('degenerate inputs', () => { + it('moveTab between nonexistent stacks is a no-op', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const before = store.getState().zones + + store.getState().moveTab('a', 'no-such-from', 'no-such-to') + + expect(store.getState().zones).toEqual(before) + expectTreeInvariants(store.getState().zones) + }) + + it('collapseStack with an unknown stack id is a no-op', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const before = store.getState().zones + + store.getState().collapseStack('no-such-stack') + + expect(store.getState().zones).toEqual(before) + }) + + it('setActiveTab with an unknown stack id is a no-op', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const before = store.getState().zones + + store.getState().setActiveTab('no-such-stack', 0) + + expect(store.getState().zones).toEqual(before) + }) +}) + +describe('cross-zone moves', () => { + it('undockPanel + dockPanel (the contract drag commit relies on) leaves a single location', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'left') + store.getState().dockPanel('b', 'left') + + // commit.ts always removes from the source before docking into the target. + store.getState().undockPanel('a') + store.getState().dockPanel('a', 'bottom') + + expectTreeInvariants(store.getState().zones) + expect(store.getState().getPanelLocation('a')).toMatchObject({ zone: 'bottom' }) + expect(rootStack(store, 'left').panelIds).toEqual(['b']) + }) + + it('undockPanel finds the panel in whatever zone it lives in (no zone hint needed)', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'right') + store.getState().dockPanel('b', 'center') + + store.getState().undockPanel('a') + store.getState().undockPanel('b') + + expect(zoneLayout(store, 'right')).toBeNull() + expect(zoneLayout(store, 'center')).toBeNull() + }) +}) + +// Each test below asserts the DESIRED behavior and is marked `.fails` because +// the current implementation silently loses or corrupts state instead. When the +// underlying hole is fixed, the test starts passing and vitest flags it — +// remove the `.fails` marker then. +describe('known holes (documented as expected failures)', () => { + it.fails('dockPanel into a different zone must not duplicate the panel across zones', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'left') + + // The duplicate guard in dockPanel only cleans the TARGET zone's tree. + // Docking a panel that lives in another zone leaves it in both. Today the + // drag commit always undocks first, but any caller that skips that (or a + // refactor that reorders it) corrupts the layout silently. + store.getState().dockPanel('a', 'bottom') + + expectTreeInvariants(store.getState().zones) // 'a' appears twice today + expect(store.getState().getPanelLocation('a')).toMatchObject({ zone: 'bottom' }) + expect(collectStacks(zoneLayout(store, 'left')).some((s) => s.panelIds.includes('a'))).toBe(false) + }) + + it.fails('a split target whose stack is gone falls back to zone append instead of dropping the panel', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + + // The stack was closed since the user last interacted with it — same + // situation the tab-target path already guards against. + store.getState().dockPanel('b', 'bottom', { type: 'split', stackId: 'stack-gone', edge: 'right' }) + + expect(store.getState().getPanelLocation('b')).toBeDefined() + }) + + it.fails('moveTab to a stack that no longer exists does not lose the panel', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + store.getState().dockPanel('b', 'bottom') + const stack = rootStack(store, 'bottom') + + store.getState().moveTab('a', stack.id, 'stack-gone') + + expect(store.getState().getPanelLocation('a')).toBeDefined() + }) + + it.fails('a tab insert index beyond the stack length keeps activeIndex in range', () => { + const store = createDockStore() + store.getState().dockPanel('a', 'bottom') + const stack = rootStack(store, 'bottom') + + store.getState().dockPanel('b', 'bottom', { type: 'tab', stackId: stack.id, index: 5 }) + + const updated = rootStack(store, 'bottom') + expect(updated.panelIds).toEqual(['a', 'b']) + expect(updated.activeIndex).toBeLessThan(updated.panelIds.length) + }) +}) + +describe('end-to-end rearrangement scenario', () => { + it('keeps the tree consistent through a long realistic op sequence', () => { + const store = createDockStore() + const s = store.getState + + // Build: bottom = [term1, term2] | agent split right, left = files + s().dockPanel('term1', 'bottom') + s().dockPanel('term2', 'bottom') + expectTreeInvariants(s().zones) + const termStack = rootStack(store, 'bottom') + s().dockPanel('agent', 'bottom', { type: 'split', stackId: termStack.id, edge: 'right' }) + expectTreeInvariants(s().zones) + s().dockPanel('files', 'left') + expectTreeInvariants(s().zones) + + // Tab the agent into the terminal stack, collapsing its split. + const agentStack = stackContaining(store, 'agent') + s().moveTab('agent', agentStack.id, termStack.id, 1) + expectTreeInvariants(s().zones) + expect(rootStack(store, 'bottom').panelIds).toEqual(['term1', 'agent', 'term2']) + + // Split term2 to the bottom of the (now single) stack, then move files over. + const stack = rootStack(store, 'bottom') + s().undockPanel('term2') + expectTreeInvariants(s().zones) + s().dockPanel('term2', 'bottom', { type: 'split', stackId: stack.id, edge: 'bottom' }) + expectTreeInvariants(s().zones) + s().moveTab('files', rootStack(store, 'left').id, stackContaining(store, 'term2').id) + expectTreeInvariants(s().zones) + + // Left zone emptied by the move; bottom holds everything, each panel locatable. + expect(zoneLayout(store, 'left')).toBeNull() + for (const id of ['term1', 'agent', 'term2', 'files']) { + const loc = s().getPanelLocation(id) + expect(loc?.type).toBe('dock') + expect(loc && 'zone' in loc && loc.zone).toBe('bottom') + } + + // Tear everything down — zone hides itself at the end. + for (const id of ['term1', 'agent', 'term2', 'files']) { + s().undockPanel(id) + expectTreeInvariants(s().zones) + } + expect(zoneLayout(store, 'bottom')).toBeNull() + expect(s().zones.bottom.visible).toBe(false) + }) +}) From 2c85756e060337a8d6fb6d95f5a8f1037a077be5 Mon Sep 17 00:00:00 2001 From: Anton Date: Wed, 10 Jun 2026 23:49:50 +0200 Subject: [PATCH 2/2] fix: retry temp dir cleanup in terminalLogger test for Windows stream.end() releases the fd asynchronously; on Windows the open handle made the afterEach rmSync fail with ENOTEMPTY. Retry the removal until the close lands. --- src/main/ipc/terminalLogger.test.ts | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/main/ipc/terminalLogger.test.ts b/src/main/ipc/terminalLogger.test.ts index 1faf2899..5b3f60af 100644 --- a/src/main/ipc/terminalLogger.test.ts +++ b/src/main/ipc/terminalLogger.test.ts @@ -38,11 +38,17 @@ beforeEach(() => { h.userDataDir = fs.mkdtempSync(path.join(tmpdir(), 'cate-termlog-')) }) -afterEach(() => { +afterEach(async () => { // Tear down loggers FIRST (stops timers, closes streams) before removing the // directory they write into. disposeAll() - fs.rmSync(h.userDataDir, { recursive: true, force: true }) + // closeStream() ends the write stream without awaiting the fd release; on + // Windows the still-open handle makes rmSync fail (ENOTEMPTY/EBUSY), so + // retry until the close has actually landed. + await vi.waitFor(() => fs.rmSync(h.userDataDir, { recursive: true, force: true }), { + timeout: 3000, + interval: 20, + }) }) // ===========================================================================