From 2c513b2d10d96b336cbf25f1c28f99f226846167 Mon Sep 17 00:00:00 2001 From: Jinwoo-H Date: Sun, 18 Oct 2026 03:12:35 -0400 Subject: [PATCH] fix(layout): a layout command main cannot apply is a logged refusal, not the window's own change (dark, layout-owner build only) other_partition, a thrown call, and host_not_owned for a workspace that is not a paired server's are refused and logged: a window save no longer carries the change, so drawing it would last only until the next frame. A twin main may route to a paired server keeps today's path, as does the first terminal's host_not_owned fallback. --- .../src/runtime/layout-apply-send.test.ts | 50 ++++++++++++++++--- src/renderer/src/runtime/layout-apply-send.ts | 43 +++++++++++----- .../src/runtime/layout-owner-command.test.ts | 36 +++++++------ ...erminal-tab-create-through-layout-owner.ts | 9 ++-- 4 files changed, 100 insertions(+), 38 deletions(-) diff --git a/src/renderer/src/runtime/layout-apply-send.test.ts b/src/renderer/src/runtime/layout-apply-send.test.ts index 5291b071009..9a7400a54f0 100644 --- a/src/renderer/src/runtime/layout-apply-send.test.ts +++ b/src/renderer/src/runtime/layout-apply-send.test.ts @@ -2,12 +2,22 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' import type { LayoutApplyReply } from '../../../shared/workspace-layout/workspace-layout-command-types' const mocks = vi.hoisted(() => { - const sent: { reply: LayoutApplyReply } = { reply: { ok: true, result: {} } } - return { sent } + const sent: { reply: LayoutApplyReply | Error } = { reply: { ok: true, result: {} } } + const owner: { kind: 'resolved' | 'ambiguous' } = { kind: 'resolved' } + return { sent, owner } }) -vi.mock('@/lib/resolve-owner', () => ({ runtimeEnvironmentIdUnlessTwin: () => null })) +vi.mock('@/lib/resolve-owner', () => ({ + runtimeEnvironmentIdUnlessTwin: () => null, + runtimeEnvironmentIdForWorkspaceOwner: () => + mocks.owner.kind === 'resolved' ? { kind: 'resolved', environmentId: null } : mocks.owner +})) vi.mock('@/runtime/runtime-rpc-client', () => ({ - callRuntimeRpc: vi.fn(async () => mocks.sent.reply) + callRuntimeRpc: vi.fn(async () => { + if (mocks.sent.reply instanceof Error) { + throw mocks.sent.reply + } + return mocks.sent.reply + }) })) vi.mock('@/runtime/layout-frame-position', () => ({ afterLayoutFrame: (_seq: number | undefined, run: () => void) => run() @@ -18,7 +28,7 @@ import { sendLayoutApply } from './layout-apply-send' const RENAME = { type: 'renamePane', tabId: 'tab-1', leafId: 'leaf-1', title: 'Pane' } as const -async function send(reply: LayoutApplyReply) { +async function send(reply: LayoutApplyReply | Error) { mocks.sent.reply = reply const handlers = { apply: vi.fn(), asToday: vi.fn(), refused: vi.fn(), unapplied: vi.fn() } // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the owner lookup is mocked and never reads the state. @@ -34,7 +44,35 @@ async function send(reply: LayoutApplyReply) { } describe('sendLayoutApply', () => { - beforeEach(() => vi.spyOn(console, 'warn').mockImplementation(() => {})) + beforeEach(() => { + mocks.owner.kind = 'resolved' + vi.spyOn(console, 'warn').mockImplementation(() => {}) + }) + + // A window save no longer carries the change, so doing it here would last only until the next frame. + it.each([ + { reply: { ok: false, code: 'other_partition' } as const, code: 'other_partition' }, + { reply: { ok: false, code: 'host_not_owned' } as const, code: 'host_not_owned' }, + { reply: new Error('disk refused'), code: 'call_failed' } + ])('logs a $code refusal and draws nothing of its own', async ({ reply, code }) => { + const handlers = await send(reply) + + expect(handlers.refused).toHaveBeenCalledWith(code) + expect(handlers.asToday).not.toHaveBeenCalled() + expect(handlers.unapplied).not.toHaveBeenCalled() + expect(console.warn).toHaveBeenCalledWith( + expect.stringContaining(`[layout-owner] renamePane refused`), + ...(reply instanceof Error ? ['disk refused'] : [code]) + ) + }) + + it('acts as today on a twin main routed to a paired server', async () => { + mocks.owner.kind = 'ambiguous' + const handlers = await send({ ok: false, code: 'host_not_owned' }) + + expect(handlers.asToday).toHaveBeenCalledOnce() + expect(handlers.refused).not.toHaveBeenCalled() + }) it('refuses a change on a workspace a WSL move has paused, drawing nothing', async () => { const handlers = await send({ ok: false, code: 'wsl_move_in_progress' }) diff --git a/src/renderer/src/runtime/layout-apply-send.ts b/src/renderer/src/runtime/layout-apply-send.ts index b6800e321de..9760457d2d6 100644 --- a/src/renderer/src/runtime/layout-apply-send.ts +++ b/src/renderer/src/runtime/layout-apply-send.ts @@ -1,5 +1,8 @@ // Store-free so store slices can send; `layout-owner-command` reads the store for other callers. -import { runtimeEnvironmentIdUnlessTwin } from '@/lib/resolve-owner' +import { + runtimeEnvironmentIdForWorkspaceOwner, + runtimeEnvironmentIdUnlessTwin +} from '@/lib/resolve-owner' import { callRuntimeRpc } from '@/runtime/runtime-rpc-client' import { afterLayoutFrame } from '@/runtime/layout-frame-position' import { holdTabPaneTree } from '@/runtime/layout-owner-pane-intents' @@ -131,13 +134,14 @@ export type WindowLayoutCommand = | { type: 'wake'; tabId: string; paneKeys: string[]; recordOnly: true } | { type: 'setDefaultTabsApplied' } -type WindowRefusalCode = LayoutRefusalCode | 'wsl_move_in_progress' +/** `call_failed`: the call threw, and a thrown write is rolled back. */ +type WindowRefusalCode = LayoutRefusalCode | 'wsl_move_in_progress' | 'call_failed' export type LayoutApplyHandlers = { apply: (result: LayoutCommandResult) => void asToday: () => void refused?: (code: WindowRefusalCode) => void - /** Main's copy lacks the target, or the call threw; `asToday` when absent. A pane split or move + /** Main's copy lacks the target; `asToday` when absent. A pane split or move * passes one: main keeps its pane trees over the window's saves, so that change would not last. * A close keeps `asToday`: a pane main lacks has nothing to keep it. */ unapplied?: () => void @@ -157,10 +161,11 @@ const PANE_COMMANDS: ReadonlySet = new Set([ /** * A layout command in a layout-owner build: main applies it first, then, once the window has the - * frame with main's change, `apply` shows main's result. `asToday` runs instead when main cannot - * act on it: a paired server holds the tab (#18202), a move's workspaces are in two partitions, - * main's copy lacks the workspace, tab, pane or group, or the call threw. `refused` on any other - * refusal. False when a paired server owns the workspace's layout; the caller acts as today. + * frame with main's change, `apply` shows main's result. `asToday` runs instead only when a paired + * server may hold the tab (#18202), or main's copy lacks the workspace, tab, pane or group. Any + * other refusal, and a call that threw, is logged and `refused`: main's frame stays what is drawn, + * since a window save no longer carries the change. False when a paired server owns the + * workspace's layout; the caller acts as today. */ export function sendLayoutApply( state: Parameters[0], @@ -190,17 +195,17 @@ export function sendLayoutApply( (reply) => afterLayoutFrame(reply.frameSeq, () => { try { - handleReply(command, reply, handlers) + handleReply(state, worktreeId, command, reply, handlers) } finally { release() } }), (error: unknown) => { // Why: 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)) + // pane main minted, which its frame then draws. + console.warn(`[layout-owner] ${command.type} refused: call_failed`, errorMessage(error)) try { - unapplied(handlers) + handlers.refused?.('call_failed') } finally { release() } @@ -214,16 +219,18 @@ export function sendLayoutApply( } function handleReply( + state: Parameters[0], + worktreeId: string, command: WindowLayoutCommand, reply: LayoutApplyReply, handlers: LayoutApplyHandlers ): void { if (reply.ok) { handlers.apply(reply.result) - } else if (reply.code === 'host_not_owned' || reply.code === 'other_partition') { + } else if (reply.code === 'host_not_owned' && mayBePairedServers(state, worktreeId)) { handlers.asToday() } else if (mainLacksTarget(reply.code)) { - // Why: main's copy lags the window's here (the window still saves its tabs). + // Why: main's copy can lag the window's: a tab the window drew before main's create answered. console.warn(`[layout-owner] ${command.type} found no target:`, reply.code) unapplied(handlers) } else { @@ -232,6 +239,16 @@ function handleReply( } } +/** A twin: main may route it to a paired server, whose layout the window still mirrors. */ +function mayBePairedServers( + state: Parameters[0], + worktreeId: string +): boolean { + return ( + runtimeEnvironmentIdForWorkspaceOwner(state, { workspaceId: worktreeId }).kind !== 'resolved' + ) +} + function unapplied(handlers: LayoutApplyHandlers): void { if (handlers.unapplied) { handlers.unapplied() diff --git a/src/renderer/src/runtime/layout-owner-command.test.ts b/src/renderer/src/runtime/layout-owner-command.test.ts index 02929b32bc8..0d3191eed39 100644 --- a/src/renderer/src/runtime/layout-owner-command.test.ts +++ b/src/renderer/src/runtime/layout-owner-command.test.ts @@ -19,7 +19,11 @@ const mocks = vi.hoisted( vi.mock('@/runtime/runtime-rpc-client', () => ({ callRuntimeRpc: mocks.callRuntimeRpc })) vi.mock('@/store', () => ({ useAppStore: { getState: () => ({}) } })) vi.mock('@/lib/resolve-owner', () => ({ - runtimeEnvironmentIdUnlessTwin: () => mocks.ownerEnvironmentId + runtimeEnvironmentIdUnlessTwin: () => mocks.ownerEnvironmentId, + runtimeEnvironmentIdForWorkspaceOwner: () => ({ + kind: 'resolved', + environmentId: mocks.ownerEnvironmentId + }) })) const SPLIT = { @@ -100,8 +104,7 @@ describe('sendLayoutCommandToOwner', () => { [ '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() @@ -112,15 +115,19 @@ describe('sendLayoutCommandToOwner', () => { expect(on.refused).not.toHaveBeenCalled() }) - it.each([ - ['main lacks the pane', { ok: false, code: 'pane_not_found' }], - ['the call fails', new Error('rpc_failed')] - ])('draws nothing for a pane command passing `unapplied` when %s', async (_case, answer) => { - if (answer instanceof Error) { - mocks.callRuntimeRpc.mockRejectedValue(answer) - } else { - mocks.callRuntimeRpc.mockResolvedValue(answer) - } + it('refuses, drawing nothing, when the call fails', async () => { + mocks.callRuntimeRpc.mockRejectedValue(new Error('rpc_failed')) + const on = { ...handlers(), unapplied: vi.fn() } + + sendLayoutCommandToOwner('worktree-1', SPLIT, on) + + await vi.waitFor(() => expect(on.refused).toHaveBeenCalledWith('call_failed')) + expect(on.asToday).not.toHaveBeenCalled() + expect(on.unapplied).not.toHaveBeenCalled() + }) + + it('draws nothing for a pane command passing `unapplied` when main lacks the pane', async () => { + mocks.callRuntimeRpc.mockResolvedValue({ ok: false, code: 'pane_not_found' }) const on = { ...handlers(), unapplied: vi.fn() } sendLayoutCommandToOwner('worktree-1', SPLIT, on) @@ -188,13 +195,14 @@ describe('sendLayoutCommandToOwner', () => { await vi.waitFor(() => expect(on.refused).toHaveBeenCalledWith('last_pane')) }) - it('acts as today when main says a paired server holds the tab', async () => { + it('refuses a workspace main routes to no host of its own, drawing nothing', async () => { mocks.callRuntimeRpc.mockResolvedValue({ ok: false, code: 'host_not_owned' }) const on = handlers() expect(sendLayoutCommandToOwner('worktree-1', SPLIT, on)).toBe(true) - await vi.waitFor(() => expect(on.asToday).toHaveBeenCalled()) + await vi.waitFor(() => expect(on.refused).toHaveBeenCalledWith('host_not_owned')) + expect(on.asToday).not.toHaveBeenCalled() expect(on.apply).not.toHaveBeenCalled() }) diff --git a/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.ts b/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.ts index 55dfb303650..a2ec52f584b 100644 --- a/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.ts +++ b/src/renderer/src/store/terminals/terminal-tab-create-through-layout-owner.ts @@ -86,9 +86,8 @@ export function terminalTabCreateCommand( } /** - * Sends a tab the window just drew to main. Nothing to undo on any answer: main holding the same - * pane already, or a paired server or no partition holding the workspace, leaves the window's own - * session save to carry the tab, as today. + * Sends a tab the window just drew to main. Nothing to undo on any answer: main's frame is what + * stays drawn, since the window's session save no longer carries the tab. A refusal is logged. */ export function recordCreatedTerminalTabInLayoutOwner( state: AppState, @@ -105,13 +104,13 @@ export function recordCreatedTerminalTabInLayoutOwner( } void callRuntimeRpc({ kind: 'local' }, 'layout.apply', { command }).then( (reply) => { - if (!reply.ok && reply.code !== 'host_not_owned') { + if (!reply.ok) { console.warn('[layout-owner] createTerminalTab refused:', reply.code) } }, (error: unknown) => { console.warn( - '[layout-owner] createTerminalTab failed:', + '[layout-owner] createTerminalTab refused: call_failed', error instanceof Error ? error.message : error ) }