diff --git a/config/electron-builder.config.cjs b/config/electron-builder.config.cjs index 5c36dfd1486..434f5fc0ee4 100644 --- a/config/electron-builder.config.cjs +++ b/config/electron-builder.config.cjs @@ -54,6 +54,7 @@ const { orcadTemplateMacSignIgnore } = require('./scripts/packaged-orcad-template.cjs') const { verifySkillsCliRuntime } = require('./scripts/verify-skills-cli-runtime.cjs') +const { assertNotLayoutOwnerBuild } = require('./layout-owner-build.cjs') const { verifyStaticAppImagePackage } = require('./scripts/static-appimage-package-contract.cjs') const { appImageNodeRuntimeFpmArgs, @@ -396,6 +397,7 @@ module.exports = { mobileWebBundleDir = MOBILE_WEB_BUNDLE_DIR, ensureWaylandClipboard = ensureBundledWaylandClipboard ) => { + assertNotLayoutOwnerBuild([join(__dirname, '..', 'out', 'main')]) assertPackagedNativeVariantsInstalled(context.electronPlatformName, context.arch) assertBundledRipgrepInstalled() assertProcessHostOutputBuilt() diff --git a/config/layout-owner-build.cjs b/config/layout-owner-build.cjs new file mode 100644 index 00000000000..4a71f5c8b83 --- /dev/null +++ b/config/layout-owner-build.cjs @@ -0,0 +1,39 @@ +const { existsSync } = require('node:fs') +const { join } = require('node:path') + +// The dark layout-owner switch (src/shared/layout-owner-switch.ts): a build compiles it in only +// when ORCA_LAYOUT_OWNER=main asks. Such a main build leaves this marker in out/main, and packaging +// refuses it; packaging and the orcad template also refuse the variable itself. +const LAYOUT_OWNER_MARKER_FILENAME = 'ORCA_LAYOUT_OWNER_MAIN' +const LAYOUT_OWNER_MARKER_TEXT = 'Built with ORCA_LAYOUT_OWNER=main, for dark testing only.\n' + +function isLayoutOwnerBuild(env = process.env) { + return env.ORCA_LAYOUT_OWNER === 'main' +} + +/** The `ORCA_LAYOUT_OWNER` define: 'main' only when asked, so other builds fold the branches away. */ +function layoutOwnerDefine(env = process.env) { + return JSON.stringify(isLayoutOwnerBuild(env) ? 'main' : 'window') +} + +function assertNotLayoutOwnerBuild(outputDirs, env = process.env) { + if (isLayoutOwnerBuild(env)) { + throw new Error( + 'ORCA_LAYOUT_OWNER=main is for dark test builds; unset it to package a release.' + ) + } + const marked = outputDirs.find((dir) => existsSync(join(dir, LAYOUT_OWNER_MARKER_FILENAME))) + if (marked) { + throw new Error( + `${marked} holds an ORCA_LAYOUT_OWNER=main build; rebuild it without that variable first.` + ) + } +} + +module.exports = { + LAYOUT_OWNER_MARKER_FILENAME, + LAYOUT_OWNER_MARKER_TEXT, + assertNotLayoutOwnerBuild, + isLayoutOwnerBuild, + layoutOwnerDefine +} diff --git a/config/scripts/build-orcad-template.mjs b/config/scripts/build-orcad-template.mjs index 83b6665107a..a69e023dcc9 100644 --- a/config/scripts/build-orcad-template.mjs +++ b/config/scripts/build-orcad-template.mjs @@ -37,6 +37,7 @@ import { } from './orcad-prebuild-slot-contents.mjs' import { runProcessSync } from '@orca/process-host' import { verifyPackagedOrcadTemplate } from './verify-packaged-orcad-template.cjs' +import { assertNotLayoutOwnerBuild } from '../layout-owner-build.cjs' const root = resolve(import.meta.dirname, '../..') const outputDir = join(root, 'out', 'orcad-template') @@ -179,6 +180,8 @@ function sameBytes(left, right) { } async function main() { + // The template is what releases ship as the server; a dark build must never become one. + assertNotLayoutOwnerBuild([]) const templateTargets = requestedTemplateTargets() rmSync(buildDir, { recursive: true, force: true }) const packages = Object.fromEntries( diff --git a/config/scripts/electron-builder-config.test.mjs b/config/scripts/electron-builder-config.test.mjs index c50da1d9585..bc0c6dfd27d 100644 --- a/config/scripts/electron-builder-config.test.mjs +++ b/config/scripts/electron-builder-config.test.mjs @@ -593,6 +593,20 @@ describe('arch-aware packaging guard', () => { expect(() => packHost(HOST_ARCH)).not.toThrow() }) + it('refuses to package a dark layout-owner build', () => { + const previous = process.env.ORCA_LAYOUT_OWNER + process.env.ORCA_LAYOUT_OWNER = 'main' + try { + expect(() => packHost(HOST_ARCH)).toThrow('ORCA_LAYOUT_OWNER=main') + } finally { + if (previous === undefined) { + delete process.env.ORCA_LAYOUT_OWNER + } else { + process.env.ORCA_LAYOUT_OWNER = previous + } + } + }) + it('requires the other architecture natives to be installed', () => { const otherSherpaInstalled = existsSync( join(REPO_ROOT, 'node_modules', otherSherpa, 'package.json') diff --git a/config/scripts/headless-detector-compiler-cache.test.mjs b/config/scripts/headless-detector-compiler-cache.test.mjs index 0f8ad3c4e3b..ffec74d6df0 100644 --- a/config/scripts/headless-detector-compiler-cache.test.mjs +++ b/config/scripts/headless-detector-compiler-cache.test.mjs @@ -84,6 +84,10 @@ it('activates the compiler and workspace-policy parser and preserves import grap join(root, 'config', 'scripts', `${name}.mjs`) ) } + cpSync( + new URL('../layout-owner-build.cjs', import.meta.url), + join(root, 'config', 'layout-owner-build.cjs') + ) mkdirSync(join(root, 'src', 'shared'), { recursive: true }) cpSync( new URL('../../src/shared/orcad-artifacts.ts', import.meta.url), diff --git a/config/scripts/layout-owner-build.test.mjs b/config/scripts/layout-owner-build.test.mjs new file mode 100644 index 00000000000..035ac617ef0 --- /dev/null +++ b/config/scripts/layout-owner-build.test.mjs @@ -0,0 +1,34 @@ +import { mkdtemp, rm, writeFile } from 'node:fs/promises' +import { createRequire } from 'node:module' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, describe, expect, it } from 'vitest' + +const { LAYOUT_OWNER_MARKER_FILENAME, assertNotLayoutOwnerBuild, layoutOwnerDefine } = + createRequire(import.meta.url)('../layout-owner-build.cjs') + +const scratch = [] +afterEach(async () => { + await Promise.all(scratch.splice(0).map((dir) => rm(dir, { recursive: true, force: true }))) +}) + +describe('layout-owner build switch', () => { + it('compiles the dark path in only when ORCA_LAYOUT_OWNER=main asks', () => { + expect(layoutOwnerDefine({ ORCA_LAYOUT_OWNER: 'main' })).toBe('"main"') + expect(layoutOwnerDefine({ ORCA_LAYOUT_OWNER: 'window' })).toBe('"window"') + expect(layoutOwnerDefine({})).toBe('"window"') + }) + + it('refuses the switch at packaging time, and any output a dark build marked', async () => { + const clean = await mkdtemp(join(tmpdir(), 'orca-layout-owner-clean-')) + const dark = await mkdtemp(join(tmpdir(), 'orca-layout-owner-dark-')) + scratch.push(clean, dark) + await writeFile(join(dark, LAYOUT_OWNER_MARKER_FILENAME), 'dark\n') + + expect(() => assertNotLayoutOwnerBuild([clean], {})).not.toThrow() + expect(() => assertNotLayoutOwnerBuild([clean], { ORCA_LAYOUT_OWNER: 'main' })).toThrow( + 'ORCA_LAYOUT_OWNER=main' + ) + expect(() => assertNotLayoutOwnerBuild([clean, dark], {})).toThrow(dark) + }) +}) diff --git a/config/scripts/orcad-entry-build.mjs b/config/scripts/orcad-entry-build.mjs index dc65783e8af..1830b98ea40 100644 --- a/config/scripts/orcad-entry-build.mjs +++ b/config/scripts/orcad-entry-build.mjs @@ -1,10 +1,14 @@ import { build } from 'esbuild' import { createHash } from 'node:crypto' import { readFileSync } from 'node:fs' +import { createRequire } from 'node:module' import { basename, dirname, join } from 'node:path' import { ORCAD_SERVER_ENTRY_FILENAME } from '../../src/shared/orcad-artifacts.ts' const root = join(import.meta.dirname, '..', '..') +const { isLayoutOwnerBuild, layoutOwnerDefine } = createRequire(import.meta.url)( + '../layout-owner-build.cjs' +) export const ORCAD_ENTRY_POINT = 'src/main/orcad/main.ts' export const ORCAD_LAUNCHER_ENTRY_POINT = 'src/main/orcad/launcher.ts' @@ -33,7 +37,11 @@ export const ORCAD_COMMONJS_MODULE_OPTIONS = { banner: { js: 'var __orcadModuleUrl = require("node:url").pathToFileURL(__filename).href;' }, - define: { 'import.meta.url': '__orcadModuleUrl' }, + define: { + 'import.meta.url': '__orcadModuleUrl', + // Why only when on: an undeclared switch already reads as off, so other builds keep their bytes. + ...(isLayoutOwnerBuild() ? { ORCA_LAYOUT_OWNER: layoutOwnerDefine() } : {}) + }, logOverride: { 'empty-import-meta': 'error' } } diff --git a/electron.vite.config.ts b/electron.vite.config.ts index 2df9e448c46..ee8d6bf95b2 100644 --- a/electron.vite.config.ts +++ b/electron.vite.config.ts @@ -1,7 +1,8 @@ import { markdownParserAliases } from './config/build-plugins/markdown-parser-exports' -import { isBuiltin } from 'node:module' +import { createRequire, isBuiltin } from 'node:module' import { resolve } from 'node:path' import { defineConfig, type UserConfig } from 'electron-vite' +import type { Plugin } from 'vite' import react from '@vitejs/plugin-react' import tailwindcss from '@tailwindcss/vite' import { createBootstrapFatalExitBanner } from './config/build-plugins/bootstrap-fatal-exit-banner' @@ -66,11 +67,27 @@ const ORCA_DIAGNOSTICS_TOKEN_URL_LITERAL = ? JSON.stringify(orcaDiagnosticsTokenUrl) : 'null' -// Why: the dark layout-owner path (src/shared/layout-owner-switch.ts) is compiled in only for a -// build that asks for it; every other build substitutes 'window', so its branches fold away. -const ORCA_LAYOUT_OWNER_LITERAL = JSON.stringify( - process.env.ORCA_LAYOUT_OWNER === 'main' ? 'main' : 'window' -) +const layoutOwnerBuild: { + LAYOUT_OWNER_MARKER_FILENAME: string + LAYOUT_OWNER_MARKER_TEXT: string + isLayoutOwnerBuild: () => boolean + layoutOwnerDefine: () => string +} = createRequire(import.meta.url)('./config/layout-owner-build.cjs') +const ORCA_LAYOUT_OWNER_LITERAL = layoutOwnerBuild.layoutOwnerDefine() + +/** Marks a dark layout-owner main build so packaging refuses it. */ +function createLayoutOwnerMarkerPlugin(): Plugin { + return { + name: 'orca-layout-owner-marker', + generateBundle() { + this.emitFile({ + type: 'asset', + fileName: layoutOwnerBuild.LAYOUT_OWNER_MARKER_FILENAME, + source: layoutOwnerBuild.LAYOUT_OWNER_MARKER_TEXT + }) + } + } +} // Why: official skill installs pin to the commit the build came from, so they match the // bundled skills on every channel. Main runs the installer; the renderer prints the command. @@ -249,7 +266,8 @@ export const electronViteConfig: UserConfig = { plugins: [ createMainBootstrapPlugin(), createPlainNodeEntryGuardPlugin(), - createProcessHostDevRebuildPlugin() + createProcessHostDevRebuildPlugin(), + ...(layoutOwnerBuild.isLayoutOwnerBuild() ? [createLayoutOwnerMarkerPlugin()] : []) ] } }, diff --git a/src/renderer/src/components/terminal-pane/layout-owner-last-pane-refusal.ts b/src/renderer/src/components/terminal-pane/layout-owner-last-pane-refusal.ts new file mode 100644 index 00000000000..a664ea69c74 --- /dev/null +++ b/src/renderer/src/components/terminal-pane/layout-owner-last-pane-refusal.ts @@ -0,0 +1,20 @@ +import type { PaneManager } from '@/lib/pane-manager/pane-manager' + +/** + * Main refused a pane close as its last pane. An earlier close in flight took the sibling: once + * it lands here too, the tab closes. More panes here than main has: main's copy lags, so close as + * today. An unmounted tab is left alone. + */ +export function closeAfterLastPaneRefusal( + manager: PaneManager | null, + closeTab: () => void, + closePaneAsToday: (paneId: number) => void, + paneId: number +): void { + const panes = manager?.getPanes().length + if (panes === 1) { + closeTab() + } else if (panes) { + closePaneAsToday(paneId) + } +} diff --git a/src/renderer/src/components/terminal-pane/layout-owner-pane-close.test.ts b/src/renderer/src/components/terminal-pane/layout-owner-pane-close.test.ts index 06626042c93..40aedc591d0 100644 --- a/src/renderer/src/components/terminal-pane/layout-owner-pane-close.test.ts +++ b/src/renderer/src/components/terminal-pane/layout-owner-pane-close.test.ts @@ -110,3 +110,15 @@ it('a layout-owner build leaves an unmounted tab alone when main refuses its las expect(p.controller.onCloseTab).not.toHaveBeenCalled() }) + +it('a layout-owner build closes as today when main calls it the last pane but the window has more', async () => { + mocks.layoutOwnerIsMain = true + const p = await preparePendingSplitClose() + p.actions.executeClosePane(1) + + mocks.sendPaneCommandToLayoutOwner.mock.calls[0]?.[2].refused?.('last_pane') + + expect(p.controller.onCloseTab).not.toHaveBeenCalled() + expect(mocks.commitTerminalSurfaceClose).toHaveBeenCalledOnce() + expect(p.transports.has(1)).toBe(false) +}) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.test.ts index b3f1d98cc12..75648c6cb5d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.test.ts @@ -61,11 +61,8 @@ describe('sendPaneCommandToLayoutOwner', () => { expect(on.asToday).not.toHaveBeenCalled() }) - it.each([ - ['a refusal', () => mocks.callRuntimeRpc.mockResolvedValue({ ok: false, code: 'last_pane' })], - ['a failed call', () => mocks.callRuntimeRpc.mockRejectedValue(new Error('rpc_failed'))] - ])('changes nothing after %s', async (_case, arrange) => { - arrange() + it('changes nothing itself after a refusal', async () => { + mocks.callRuntimeRpc.mockResolvedValue({ ok: false, code: 'last_pane' }) const on = handlers() expect(sendPaneCommandToLayoutOwner('worktree-1', SPLIT, on)).toBe(true) @@ -75,6 +72,43 @@ describe('sendPaneCommandToLayoutOwner', () => { expect(on.asToday).not.toHaveBeenCalled() }) + it.each([ + [ + 'main lacks the pane', + () => mocks.callRuntimeRpc.mockResolvedValue({ ok: false, code: 'pane_not_found' }) + ], + [ + 'main lacks the tab', + () => mocks.callRuntimeRpc.mockResolvedValue({ ok: false, code: 'tab_not_found' }) + ], + [ + 'main lacks the workspace', + () => mocks.callRuntimeRpc.mockResolvedValue({ ok: false, code: 'workspace_not_found' }) + ], + ['the call fails', () => mocks.callRuntimeRpc.mockRejectedValue(new Error('rpc_failed'))] + ])('acts as today when %s', async (_case, arrange) => { + arrange() + const on = handlers() + + sendPaneCommandToLayoutOwner('worktree-1', SPLIT, on) + + await vi.waitFor(() => expect(on.asToday).toHaveBeenCalledOnce()) + expect(on.refused).not.toHaveBeenCalled() + }) + + it('does not run the fallback when the caller’s own handler throws', async () => { + mocks.callRuntimeRpc.mockResolvedValue({ ok: true, result: { leafId: 'leaf-minted' } }) + const on = handlers() + on.apply.mockImplementation(() => { + throw new Error('mount failed') + }) + + sendPaneCommandToLayoutOwner('worktree-1', SPLIT, on) + + await vi.waitFor(() => expect(console.warn).toHaveBeenCalled()) + expect(on.asToday).not.toHaveBeenCalled() + }) + it('hands any other refusal to the caller', async () => { mocks.callRuntimeRpc.mockResolvedValue({ ok: false, code: 'last_pane' }) const on = handlers() diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.ts index bd56b4b017a..7cdf1dff933 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-commands.ts @@ -22,8 +22,9 @@ export type PaneLayoutCommand = /** * A pane command in a layout-owner build: main applies it first, then `apply` shows main's result. - * `asToday` runs instead when main says a paired server holds the tab (#18202); `refused` on any - * other refusal. False when a paired server owns the workspace's layout; the caller acts as today. + * `asToday` runs instead when main cannot act on it: a paired server holds the tab (#18202), main's + * copy lacks the workspace, tab or pane, or the call threw. `refused` on any other refusal. False + * when a paired server owns the workspace's layout; the caller acts as today. */ export function sendPaneCommandToLayoutOwner( worktreeId: string, @@ -42,23 +43,40 @@ export function sendPaneCommandToLayoutOwner( void callRuntimeRpc({ kind: 'local' }, 'layout.apply', { command: { ...command, workspace: worktreeId } }) - .then((reply) => { - if (reply.ok) { - handlers.apply(reply.result) - } else if (reply.code === 'host_not_owned') { + .then( + (reply) => { + if (reply.ok) { + handlers.apply(reply.result) + } else if (reply.code === 'host_not_owned' || mainLacksTarget(reply.code)) { + // Why lacking: main's copy lags the window's here (the window still saves its own). + handlers.asToday() + } else { + console.warn(`[layout-owner] ${command.type} refused:`, reply.code) + handlers.refused?.(reply.code) + } + }, + (error: unknown) => { + // Why as today: a thrown write is rolled back. An indeterminate disk write is not, and may + // leave a pane main minted; accepted until main is the only writer. + console.warn(`[layout-owner] ${command.type} failed:`, errorMessage(error)) handlers.asToday() - } else { - console.warn(`[layout-owner] ${command.type} refused:`, reply.code) - handlers.refused?.(reply.code) } - }) + ) + // A throwing handler is not a failed call; it must not run the fallback too. .catch((error: unknown) => { - const message = error instanceof Error ? error.message : error - console.warn(`[layout-owner] ${command.type} failed:`, message) + console.warn(`[layout-owner] ${command.type} handler threw:`, errorMessage(error)) }) return true } +function mainLacksTarget(code: LayoutRefusalCode): boolean { + return code === 'workspace_not_found' || code === 'tab_not_found' || code === 'pane_not_found' +} + +function errorMessage(error: unknown): unknown { + return error instanceof Error ? error.message : error +} + /** Equalizes a tab's panes, then `then` runs (focus); in a layout-owner build main equalizes first. */ export function equalizeTerminalPanes( pane: { worktreeId: string; tabId: string; getManager: () => PaneManager | null }, diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.test.ts index 76fa1ff2acf..e6807f473a6 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.test.ts @@ -52,9 +52,72 @@ describe('layoutOwnerPaneGestures', () => { expect(mocks.send).toHaveBeenCalledWith( 'worktree-1', { type: 'movePane', tabId: 'tab-1', leafId: 'leaf-2', targetLeafId: 'leaf-1', side: 'left' }, - { apply: move, asToday: move } + { + apply: expect.any(Function), + asToday: expect.any(Function), + refused: expect.any(Function) + } ) expect(move).not.toHaveBeenCalled() + + mocks.send.mock.calls[0]?.[2].apply({}) + expect(move).toHaveBeenCalledOnce() + }) + + it.each(['asToday', 'refused'] as const)( + 'moves the pane as today when main answers %s', + (outcome) => { + const move = vi.fn() + gestures().takePaneDrop?.({ sourcePaneId: 2, targetPaneId: 1, zone: 'left' }, move) + const handlers = mocks.send.mock.calls[0]?.[2] + + if (outcome === 'refused') { + handlers?.refused?.('same_pane') + } else { + handlers?.asToday() + } + + expect(move).toHaveBeenCalledOnce() + } + ) + + it('holds divider ratios until every move in flight has settled, even one that threw', () => { + const view = gestures() + const failingMove = vi.fn(() => { + throw new Error('pane gone') + }) + view.takePaneDrop?.({ sourcePaneId: 2, targetPaneId: 1, zone: 'left' }, failingMove) + view.takePaneDrop?.({ sourcePaneId: 1, targetPaneId: 2, zone: 'top' }, vi.fn()) + view.onDividerRatiosCommitted?.() + const [first, second] = mocks.send.mock.calls.map(([, , on]) => on) + + expect(() => first?.refused?.('pane_structure_changed')).toThrow('pane gone') + expect(mocks.send).toHaveBeenCalledTimes(2) + + second?.apply({}) + expect(mocks.send).toHaveBeenCalledTimes(3) + expect(mocks.send).toHaveBeenLastCalledWith( + 'worktree-1', + { type: 'setPaneRatios', tabId: 'tab-1', root: mocks.root }, + { apply: expect.any(Function), asToday: expect.any(Function) } + ) + }) + + it('holds divider ratios sent during a move until the window has moved too', () => { + const view = gestures() + const move = vi.fn() + view.takePaneDrop?.({ sourcePaneId: 2, targetPaneId: 1, zone: 'left' }, move) + + view.onDividerRatiosCommitted?.() + expect(mocks.send).toHaveBeenCalledOnce() + + mocks.send.mock.calls[0]?.[2].apply({}) + expect(move).toHaveBeenCalledOnce() + expect(mocks.send).toHaveBeenLastCalledWith( + 'worktree-1', + { type: 'setPaneRatios', tabId: 'tab-1', root: mocks.root }, + { apply: expect.any(Function), asToday: expect.any(Function) } + ) }) it('leaves the drop to the window when main cannot take it', () => { diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.ts b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.ts index 7f8dc7e1ff0..167f751821d 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-layout-owner-gestures.ts @@ -3,8 +3,9 @@ import { useAppStore } from '@/store' import { sendPaneCommandToLayoutOwner } from './terminal-pane-layout-owner-commands' /** - * A layout-owner build's pane gestures: an in-tab drop moves the pane once main has; a divider's - * new ratios, already drawn while dragging, are saved by main when the drag ends. + * A layout-owner build's pane gestures: an in-tab drop moves the pane once main has answered, as + * today when main refuses; a divider's new ratios, already drawn while dragging, are saved by main + * when the drag ends. */ export function layoutOwnerPaneGestures(pane: { worktreeId: string @@ -14,6 +15,27 @@ export function layoutOwnerPaneGestures(pane: { shouldPersistLayout: () => boolean }): Pick { const { worktreeId, tabId } = pane + // Why: main holds the moved tree before this window does, so ratios sent meanwhile are refused; + // they wait for the window's move and go then. + let movesInFlight = 0 + let ratiosWaiting = false + const saveRatios = (): void => { + if (movesInFlight > 0) { + ratiosWaiting = true + return + } + ratiosWaiting = false + // Why the store: the gesture's own persist has just written the window's tree there. + const root = useAppStore.getState().terminalLayoutsByTabId[tabId]?.root + if (root && pane.shouldPersistLayout()) { + const keep = (): void => {} + sendPaneCommandToLayoutOwner( + worktreeId, + { type: 'setPaneRatios', tabId, root }, + { apply: keep, asToday: keep } + ) + } + } return { takePaneDrop: ({ sourcePaneId, targetPaneId, zone }, move) => { const manager = pane.managerRef.current @@ -22,23 +44,26 @@ export function layoutOwnerPaneGestures(pane: { if (!leafId || !targetLeafId) { return false } - return sendPaneCommandToLayoutOwner( + const settle = (): void => { + try { + move() + } finally { + movesInFlight -= 1 + if (movesInFlight === 0 && ratiosWaiting) { + saveRatios() + } + } + } + const taken = sendPaneCommandToLayoutOwner( worktreeId, { type: 'movePane', tabId, leafId, targetLeafId, side: zone }, - { apply: move, asToday: move } + { apply: settle, asToday: settle, refused: settle } ) - }, - onDividerRatiosCommitted: () => { - // Why the store: the gesture's own persist has just written the window's tree there. - const root = useAppStore.getState().terminalLayoutsByTabId[tabId]?.root - if (root && pane.shouldPersistLayout()) { - const keep = (): void => {} - sendPaneCommandToLayoutOwner( - worktreeId, - { type: 'setPaneRatios', tabId, root }, - { apply: keep, asToday: keep } - ) + if (taken) { + movesInFlight += 1 } - } + return taken + }, + onDividerRatiosCommitted: saveRatios } } diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts index a2a0b21b876..cabbf81921e 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.test.ts @@ -4,19 +4,13 @@ import type { PtyTransport } from './pty-transport-types' import type { PaneCwdMap } from './resolve-split-cwd' import { splitTerminalPaneWithInheritedCwd } from './terminal-pane-split-with-inherited-cwd' import { createDeferred } from './pty-connection-test-async' +import type { sendPaneCommandToLayoutOwner } from './terminal-pane-layout-owner-commands' const mocks = vi.hoisted(() => ({ layoutOwnerIsMain: false, recordCreatedTerminalPaneSplit: vi.fn(), resolveSplitCwd: vi.fn(), - sendPaneCommandToLayoutOwner: - vi.fn< - ( - worktreeId: string, - command: unknown, - handlers: { apply: (result: { leafId?: string }) => void; asToday: () => void } - ) => boolean - >(), + sendPaneCommandToLayoutOwner: vi.fn(), splitWebRuntimeTerminal: vi.fn() })) @@ -43,7 +37,15 @@ vi.mock('./terminal-pane-split-completion', () => ({ })) function makeManager(splitPane: ReturnType): PaneManager { - return { splitPane } as unknown as PaneManager + // A pane counts as mounted once a split that placed it returned a pane. + const getNumericIdForLeaf = (leafId: string): number | null => + splitPane.mock.calls.some( + (call, index) => call[2]?.leafId === leafId && splitPane.mock.results[index]?.value + ) + ? 2 + : null + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the split reads only splitPane and getNumericIdForLeaf. + return { splitPane, getNumericIdForLeaf } as unknown as PaneManager } describe('splitTerminalPaneWithInheritedCwd', () => { @@ -271,7 +273,10 @@ describe('splitTerminalPaneWithInheritedCwd', () => { expect(mocks.sendPaneCommandToLayoutOwner).toHaveBeenCalledWith( 'worktree-1', { type: 'splitPane', tabId: 'tab-1', leafId: 'leaf-1', direction: 'vertical' }, - { apply: expect.any(Function), asToday: expect.any(Function) } + { + apply: expect.any(Function), + asToday: expect.any(Function) + } ) expect(splitPane).not.toHaveBeenCalled() @@ -285,6 +290,7 @@ describe('splitTerminalPaneWithInheritedCwd', () => { source: 'keyboard', direction: 'vertical' }) + expect(mocks.sendPaneCommandToLayoutOwner).toHaveBeenCalledOnce() }) it('in a layout-owner build, splits as today where a paired server owns the layout', () => { @@ -306,4 +312,36 @@ describe('splitTerminalPaneWithInheritedCwd', () => { expect(splitPane).toHaveBeenCalledWith(1, 'horizontal', { cwd: '/cached' }) }) + + function splitInLayoutOwnerBuild(manager: PaneManager | null): void { + mocks.layoutOwnerIsMain = true + mocks.sendPaneCommandToLayoutOwner.mockReturnValue(true) + splitTerminalPaneWithInheritedCwd({ + worktreeId: 'worktree-1', + tabId: 'tab-1', + manager: manager ?? makeManager(vi.fn()), + getManager: () => manager, + paneTransports: new Map(), + paneCwdMap: new Map([[1, { cwd: '/cached', confirmed: true }]]), + fallbackCwd: '/fallback', + pane: { id: 1, leafId: 'leaf-1' } as ManagedPane, + direction: 'vertical', + source: 'keyboard' + }) + } + + it.each([ + ['the manager is gone', null], + ['the source pane is gone', makeManager(vi.fn(() => null))] + ])('withdraws the pane main saved when %s by the time main answers', (_case, manager) => { + splitInLayoutOwnerBuild(manager) + + mocks.sendPaneCommandToLayoutOwner.mock.calls[0]?.[2].apply({ leafId: 'leaf-minted' }) + + expect(mocks.sendPaneCommandToLayoutOwner).toHaveBeenLastCalledWith( + 'worktree-1', + { type: 'closePane', tabId: 'tab-1', leafId: 'leaf-minted' }, + { apply: expect.any(Function), asToday: expect.any(Function) } + ) + }) }) diff --git a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts index 198b115dc66..3939e6a108f 100644 --- a/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts +++ b/src/renderer/src/components/terminal-pane/terminal-pane-split-with-inherited-cwd.ts @@ -38,7 +38,7 @@ export function splitTerminalPaneWithInheritedCwd(args: SplitArgs): void { args.worktreeId, { type: 'splitPane', tabId: args.tabId, leafId: args.pane.leafId, direction: args.direction }, { - apply: ({ leafId }) => leafId && splitLocally(args, ptyId, leafId), + apply: ({ leafId }) => leafId && mountMainSplit(args, ptyId, leafId), asToday: () => splitLocally(args, ptyId) } ) @@ -48,6 +48,20 @@ export function splitTerminalPaneWithInheritedCwd(args: SplitArgs): void { splitLocally(args, ptyId) } +/** Mounts main's new pane, or withdraws it when its source pane or manager went meanwhile. */ +function mountMainSplit(args: SplitArgs, ptyId: string | null, leafId: string): void { + splitLocally(args, ptyId, leafId) + const manager = args.getManager ? args.getManager() : args.manager + if (manager?.getNumericIdForLeaf(leafId) == null) { + const keep = (): void => {} + sendPaneCommandToLayoutOwner( + args.worktreeId, + { type: 'closePane', tabId: args.tabId, leafId }, + { apply: keep, asToday: keep } + ) + } +} + function splitLocally(args: SplitArgs, ptyId: string | null, leafId?: string): void { const manager = args.getManager ? args.getManager() : args.manager if (!manager) { diff --git a/src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts b/src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts index bc0a493a1e9..7efdd623872 100644 --- a/src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts +++ b/src/renderer/src/components/terminal-pane/use-terminal-pane-close-actions.ts @@ -21,6 +21,7 @@ import { commitTerminalSurfaceClose } from '@/store/terminals/terminal-surface-c import { noteAgentLaunchPaneClosedByUser } from '@/lib/agent-launch-pane-closes' import { ORCA_LAYOUT_OWNER_IS_MAIN } from '../../../../shared/layout-owner-switch' import { sendPaneCommandToLayoutOwner } from './terminal-pane-layout-owner-commands' +import { closeAfterLastPaneRefusal } from './layout-owner-last-pane-refusal' export function useTerminalPaneCloseActions(controller: TerminalPaneBindingController) { const confirmedCloseRef = useRef<(() => void) | null>(null) @@ -124,9 +125,9 @@ export function useTerminalPaneCloseActions(controller: TerminalPaneBindingContr { apply: () => closePaneLocally(paneId, leafId), asToday: () => closePaneLocally(paneId), - // An earlier close in flight took the sibling: once it lands here too, the tab closes. refused: (code) => - code === 'last_pane' && managerRef.current?.getPanes().length === 1 && onCloseTab() + code === 'last_pane' && + closeAfterLastPaneRefusal(managerRef.current, onCloseTab, closePaneLocally, paneId) } ) ) { diff --git a/src/shared/workspace-layout/workspace-layout-record-patch.ts b/src/shared/workspace-layout/workspace-layout-record-patch.ts index 61934a7f50d..1340122f2a6 100644 --- a/src/shared/workspace-layout/workspace-layout-record-patch.ts +++ b/src/shared/workspace-layout/workspace-layout-record-patch.ts @@ -16,8 +16,8 @@ const LIST_FIELDS: Record unknown> = { browserTabsByWorktree: (record) => record.id } -// Why 2: a session map (layouts by tab, sleeping records by pane) holds records; a changed record -// field is replaced whole, as diffStoredSession reports it. +// Session maps hold records (layouts by tab, sleeping records by pane): patch down to each record's +// fields and replace a changed field whole, as diffStoredSession reports it. const MAP_RECORD_DEPTH = 2 /** @@ -111,7 +111,7 @@ function patchList(stored: Json[], before: Json[], after: Json[], idOf: (record: const queue = after .map((record) => result.find((candidate) => idOf(candidate) === idOf(record))) .filter((record): record is Json => record !== undefined) - return result.map((record) => (placed.has(idOf(record)) ? queue.shift()! : record)) + return result.map((record) => (placed.has(idOf(record)) ? (queue.shift() ?? record) : record)) } /** After the nearest record that precedes it in `after` and is already in the list. */ @@ -121,8 +121,8 @@ function insertionIndex( position: number, idOf: (record: Json) => unknown ): number { - for (let previous = position - 1; previous >= 0; previous--) { - const index = result.findIndex((record) => idOf(record) === idOf(after[previous]!)) + for (const preceding of after.slice(0, position).toReversed()) { + const index = result.findIndex((record) => idOf(record) === idOf(preceding)) if (index !== -1) { return index + 1 }