Merge pull request #1905 from code-yeongyu/fix/tmux-split-stability
fix: stabilize tmux split and session readiness handling
This commit is contained in:
@@ -22,9 +22,17 @@ export interface ActionExecutorDeps {
|
||||
enforceMainPaneWidth: typeof enforceMainPaneWidth
|
||||
}
|
||||
|
||||
async function enforceMainPane(windowState: WindowState, deps: ActionExecutorDeps): Promise<void> {
|
||||
async function enforceMainPane(
|
||||
windowState: WindowState,
|
||||
config: TmuxConfig,
|
||||
deps: ActionExecutorDeps,
|
||||
): Promise<void> {
|
||||
if (!windowState.mainPane) return
|
||||
await deps.enforceMainPaneWidth(windowState.mainPane.paneId, windowState.windowWidth)
|
||||
await deps.enforceMainPaneWidth(
|
||||
windowState.mainPane.paneId,
|
||||
windowState.windowWidth,
|
||||
config.main_pane_size,
|
||||
)
|
||||
}
|
||||
|
||||
export async function executeActionWithDeps(
|
||||
@@ -35,7 +43,7 @@ export async function executeActionWithDeps(
|
||||
if (action.type === "close") {
|
||||
const success = await deps.closeTmuxPane(action.paneId)
|
||||
if (success) {
|
||||
await enforceMainPane(ctx.windowState, deps)
|
||||
await enforceMainPane(ctx.windowState, ctx.config, deps)
|
||||
}
|
||||
return { success }
|
||||
}
|
||||
@@ -65,7 +73,7 @@ export async function executeActionWithDeps(
|
||||
|
||||
if (result.success) {
|
||||
await deps.applyLayout(ctx.config.layout, ctx.config.main_pane_size)
|
||||
await enforceMainPane(ctx.windowState, deps)
|
||||
await enforceMainPane(ctx.windowState, ctx.config, deps)
|
||||
}
|
||||
|
||||
return {
|
||||
|
||||
@@ -86,6 +86,7 @@ describe("executeAction", () => {
|
||||
expect(mockApplyLayout).toHaveBeenCalledTimes(1)
|
||||
expect(mockApplyLayout).toHaveBeenCalledWith("main-horizontal", 55)
|
||||
expect(mockEnforceMainPaneWidth).toHaveBeenCalledTimes(1)
|
||||
expect(mockEnforceMainPaneWidth).toHaveBeenCalledWith("%0", 220, 55)
|
||||
})
|
||||
|
||||
test("does not apply layout when spawn fails", async () => {
|
||||
|
||||
@@ -228,7 +228,7 @@ describe("decideSpawnActions", () => {
|
||||
expect(result.actions[0].type).toBe("spawn")
|
||||
})
|
||||
|
||||
it("closes oldest pane when existing panes are too small to split", () => {
|
||||
it("replaces oldest pane when existing panes are too small to split", () => {
|
||||
// given - existing pane is below minimum splittable size
|
||||
const state = createWindowState(220, 30, [
|
||||
{ paneId: "%1", width: 50, height: 15, left: 110, top: 0 },
|
||||
@@ -242,9 +242,8 @@ describe("decideSpawnActions", () => {
|
||||
|
||||
// then
|
||||
expect(result.canSpawn).toBe(true)
|
||||
expect(result.actions.length).toBe(2)
|
||||
expect(result.actions[0].type).toBe("close")
|
||||
expect(result.actions[1].type).toBe("spawn")
|
||||
expect(result.actions.length).toBe(1)
|
||||
expect(result.actions[0].type).toBe("replace")
|
||||
})
|
||||
|
||||
it("can spawn when existing pane is large enough to split", () => {
|
||||
@@ -394,4 +393,27 @@ describe("decideSpawnActions with custom agentPaneWidth", () => {
|
||||
expect(defaultResult.canSpawn).toBe(false)
|
||||
expect(customResult.canSpawn).toBe(true)
|
||||
})
|
||||
|
||||
it("#given custom agentPaneWidth and splittable existing pane #when deciding spawn #then uses spawn without eviction", () => {
|
||||
//#given
|
||||
const customConfig: CapacityConfig = { mainPaneMinWidth: 120, agentPaneWidth: 40 }
|
||||
const state = createWindowState(220, 44, [
|
||||
{ paneId: "%1", width: 90, height: 30, left: 110, top: 0 },
|
||||
])
|
||||
const mappings: SessionMapping[] = [
|
||||
{ sessionId: "old-ses", paneId: "%1", createdAt: new Date("2024-01-01") },
|
||||
]
|
||||
|
||||
//#when
|
||||
const result = decideSpawnActions(state, "ses1", "test", customConfig, mappings)
|
||||
|
||||
//#then
|
||||
expect(result.canSpawn).toBe(true)
|
||||
expect(result.actions.length).toBe(1)
|
||||
expect(result.actions[0].type).toBe("spawn")
|
||||
if (result.actions[0].type === "spawn") {
|
||||
expect(result.actions[0].targetPaneId).toBe("%1")
|
||||
expect(result.actions[0].splitDirection).toBe("-h")
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
@@ -434,6 +434,53 @@ describe('TmuxSessionManager', () => {
|
||||
})
|
||||
|
||||
describe('onSessionDeleted', () => {
|
||||
test('does not track session when readiness timed out', async () => {
|
||||
// given
|
||||
mockIsInsideTmux.mockReturnValue(true)
|
||||
let stateCallCount = 0
|
||||
mockQueryWindowState.mockImplementation(async () => {
|
||||
stateCallCount++
|
||||
if (stateCallCount === 1) {
|
||||
return createWindowState()
|
||||
}
|
||||
return createWindowState({
|
||||
agentPanes: [
|
||||
{
|
||||
paneId: '%mock',
|
||||
width: 40,
|
||||
height: 44,
|
||||
left: 100,
|
||||
top: 0,
|
||||
title: 'omo-subagent-Timeout Task',
|
||||
isActive: false,
|
||||
},
|
||||
],
|
||||
})
|
||||
})
|
||||
|
||||
const { TmuxSessionManager } = await import('./manager')
|
||||
const ctx = createMockContext({ sessionStatusResult: { data: {} } })
|
||||
const config: TmuxConfig = {
|
||||
enabled: true,
|
||||
layout: 'main-vertical',
|
||||
main_pane_size: 60,
|
||||
main_pane_min_width: 80,
|
||||
agent_pane_min_width: 40,
|
||||
}
|
||||
const manager = new TmuxSessionManager(ctx, config, mockTmuxDeps)
|
||||
|
||||
await manager.onSessionCreated(
|
||||
createSessionCreatedEvent('ses_timeout', 'ses_parent', 'Timeout Task')
|
||||
)
|
||||
mockExecuteAction.mockClear()
|
||||
|
||||
// when
|
||||
await manager.onSessionDeleted({ sessionID: 'ses_timeout' })
|
||||
|
||||
// then
|
||||
expect(mockExecuteAction).toHaveBeenCalledTimes(0)
|
||||
})
|
||||
|
||||
test('closes pane when tracked session is deleted', async () => {
|
||||
// given
|
||||
mockIsInsideTmux.mockReturnValue(true)
|
||||
@@ -521,8 +568,13 @@ describe('TmuxSessionManager', () => {
|
||||
mockIsInsideTmux.mockReturnValue(true)
|
||||
|
||||
let callCount = 0
|
||||
mockExecuteActions.mockImplementation(async () => {
|
||||
mockExecuteActions.mockImplementation(async (actions) => {
|
||||
callCount++
|
||||
for (const action of actions) {
|
||||
if (action.type === 'spawn') {
|
||||
trackedSessions.add(action.sessionId)
|
||||
}
|
||||
}
|
||||
return {
|
||||
success: true,
|
||||
spawnedPaneId: `%${callCount}`,
|
||||
|
||||
@@ -213,10 +213,17 @@ export class TmuxSessionManager {
|
||||
const sessionReady = await this.waitForSessionReady(sessionId)
|
||||
|
||||
if (!sessionReady) {
|
||||
log("[tmux-session-manager] session not ready after timeout, tracking anyway", {
|
||||
log("[tmux-session-manager] session not ready after timeout, closing spawned pane", {
|
||||
sessionId,
|
||||
paneId: result.spawnedPaneId,
|
||||
})
|
||||
|
||||
await executeAction(
|
||||
{ type: "close", paneId: result.spawnedPaneId, sessionId },
|
||||
{ config: this.tmuxConfig, serverUrl: this.serverUrl, windowState: state }
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
const now = Date.now()
|
||||
|
||||
@@ -57,11 +57,21 @@ export function canSplitPane(
|
||||
}
|
||||
|
||||
export function canSplitPaneAnyDirection(pane: TmuxPaneInfo): boolean {
|
||||
return pane.width >= MIN_SPLIT_WIDTH || pane.height >= MIN_SPLIT_HEIGHT
|
||||
return canSplitPaneAnyDirectionWithMinWidth(pane, MIN_PANE_WIDTH)
|
||||
}
|
||||
|
||||
export function getBestSplitDirection(pane: TmuxPaneInfo): SplitDirection | null {
|
||||
const canH = pane.width >= MIN_SPLIT_WIDTH
|
||||
export function canSplitPaneAnyDirectionWithMinWidth(
|
||||
pane: TmuxPaneInfo,
|
||||
minPaneWidth: number = MIN_PANE_WIDTH,
|
||||
): boolean {
|
||||
return pane.width >= minSplitWidthFor(minPaneWidth) || pane.height >= MIN_SPLIT_HEIGHT
|
||||
}
|
||||
|
||||
export function getBestSplitDirection(
|
||||
pane: TmuxPaneInfo,
|
||||
minPaneWidth: number = MIN_PANE_WIDTH,
|
||||
): SplitDirection | null {
|
||||
const canH = pane.width >= minSplitWidthFor(minPaneWidth)
|
||||
const canV = pane.height >= MIN_SPLIT_HEIGHT
|
||||
|
||||
if (!canH && !canV) return null
|
||||
|
||||
@@ -135,10 +135,21 @@ export async function handleSessionCreated(
|
||||
|
||||
const sessionReady = await deps.waitForSessionReady(sessionId)
|
||||
if (!sessionReady) {
|
||||
log("[tmux-session-manager] session not ready after timeout, tracking anyway", {
|
||||
log("[tmux-session-manager] session not ready after timeout, closing spawned pane", {
|
||||
sessionId,
|
||||
paneId: result.spawnedPaneId,
|
||||
})
|
||||
|
||||
await executeActions(
|
||||
[{ type: "close", paneId: result.spawnedPaneId, sessionId }],
|
||||
{
|
||||
config: deps.tmuxConfig,
|
||||
serverUrl: deps.serverUrl,
|
||||
windowState: state,
|
||||
},
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
const now = Date.now()
|
||||
|
||||
@@ -129,10 +129,21 @@ export class SessionSpawner {
|
||||
const sessionReady = await this.waitForSessionReady(sessionId)
|
||||
|
||||
if (!sessionReady) {
|
||||
log("[tmux-session-manager] session not ready after timeout, tracking anyway", {
|
||||
log("[tmux-session-manager] session not ready after timeout, closing spawned pane", {
|
||||
sessionId,
|
||||
paneId: result.spawnedPaneId,
|
||||
})
|
||||
|
||||
await executeActions(
|
||||
[{ type: "close", paneId: result.spawnedPaneId, sessionId }],
|
||||
{
|
||||
config: this.tmuxConfig,
|
||||
serverUrl: this.serverUrl,
|
||||
windowState: state,
|
||||
},
|
||||
)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
const now = Date.now()
|
||||
|
||||
@@ -62,7 +62,7 @@ export function decideSpawnActions(
|
||||
}
|
||||
|
||||
if (isSplittableAtCount(agentAreaWidth, currentCount, minPaneWidth)) {
|
||||
const spawnTarget = findSpawnTarget(state)
|
||||
const spawnTarget = findSpawnTarget(state, minPaneWidth)
|
||||
if (spawnTarget) {
|
||||
return {
|
||||
canSpawn: true,
|
||||
@@ -85,19 +85,14 @@ export function decideSpawnActions(
|
||||
canSpawn: true,
|
||||
actions: [
|
||||
{
|
||||
type: "close",
|
||||
type: "replace",
|
||||
paneId: oldestPane.paneId,
|
||||
sessionId: oldestMapping?.sessionId || "",
|
||||
},
|
||||
{
|
||||
type: "spawn",
|
||||
sessionId,
|
||||
oldSessionId: oldestMapping?.sessionId || "",
|
||||
newSessionId: sessionId,
|
||||
description,
|
||||
targetPaneId: state.mainPane.paneId,
|
||||
splitDirection: "-h",
|
||||
},
|
||||
],
|
||||
reason: "closed 1 pane to make room for split",
|
||||
reason: "replaced oldest pane to avoid split churn",
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -2,6 +2,7 @@ import type { SplitDirection, TmuxPaneInfo, WindowState } from "./types"
|
||||
import { MAIN_PANE_RATIO } from "./tmux-grid-constants"
|
||||
import { computeGridPlan, mapPaneToSlot } from "./grid-planning"
|
||||
import { canSplitPane, getBestSplitDirection } from "./pane-split-availability"
|
||||
import { MIN_PANE_WIDTH } from "./types"
|
||||
|
||||
export interface SpawnTarget {
|
||||
targetPaneId: string
|
||||
@@ -37,6 +38,7 @@ function findFirstEmptySlot(
|
||||
|
||||
function findSplittableTarget(
|
||||
state: WindowState,
|
||||
minPaneWidth: number,
|
||||
_preferredDirection?: SplitDirection,
|
||||
): SpawnTarget | null {
|
||||
if (!state.mainPane) return null
|
||||
@@ -44,7 +46,7 @@ function findSplittableTarget(
|
||||
|
||||
if (existingCount === 0) {
|
||||
const virtualMainPane: TmuxPaneInfo = { ...state.mainPane, width: state.windowWidth }
|
||||
if (canSplitPane(virtualMainPane, "-h")) {
|
||||
if (canSplitPane(virtualMainPane, "-h", minPaneWidth)) {
|
||||
return { targetPaneId: state.mainPane.paneId, splitDirection: "-h" }
|
||||
}
|
||||
return null
|
||||
@@ -56,17 +58,17 @@ function findSplittableTarget(
|
||||
const targetSlot = findFirstEmptySlot(occupancy, plan)
|
||||
|
||||
const leftPane = occupancy.get(`${targetSlot.row}:${targetSlot.col - 1}`)
|
||||
if (leftPane && canSplitPane(leftPane, "-h")) {
|
||||
if (leftPane && canSplitPane(leftPane, "-h", minPaneWidth)) {
|
||||
return { targetPaneId: leftPane.paneId, splitDirection: "-h" }
|
||||
}
|
||||
|
||||
const abovePane = occupancy.get(`${targetSlot.row - 1}:${targetSlot.col}`)
|
||||
if (abovePane && canSplitPane(abovePane, "-v")) {
|
||||
if (abovePane && canSplitPane(abovePane, "-v", minPaneWidth)) {
|
||||
return { targetPaneId: abovePane.paneId, splitDirection: "-v" }
|
||||
}
|
||||
|
||||
const splittablePanes = state.agentPanes
|
||||
.map((pane) => ({ pane, direction: getBestSplitDirection(pane) }))
|
||||
.map((pane) => ({ pane, direction: getBestSplitDirection(pane, minPaneWidth) }))
|
||||
.filter(
|
||||
(item): item is { pane: TmuxPaneInfo; direction: SplitDirection } =>
|
||||
item.direction !== null,
|
||||
@@ -81,6 +83,9 @@ function findSplittableTarget(
|
||||
return null
|
||||
}
|
||||
|
||||
export function findSpawnTarget(state: WindowState): SpawnTarget | null {
|
||||
return findSplittableTarget(state)
|
||||
export function findSpawnTarget(
|
||||
state: WindowState,
|
||||
minPaneWidth: number = MIN_PANE_WIDTH,
|
||||
): SpawnTarget | null {
|
||||
return findSplittableTarget(state, minPaneWidth)
|
||||
}
|
||||
|
||||
@@ -29,13 +29,15 @@ export async function applyLayout(
|
||||
export async function enforceMainPaneWidth(
|
||||
mainPaneId: string,
|
||||
windowWidth: number,
|
||||
mainPaneSize: number,
|
||||
): Promise<void> {
|
||||
const { log } = await import("../../logger")
|
||||
const tmux = await getTmuxPath()
|
||||
if (!tmux) return
|
||||
|
||||
const dividerWidth = 1
|
||||
const mainWidth = Math.floor((windowWidth - dividerWidth) / 2)
|
||||
const boundedMainPaneSize = Math.max(20, Math.min(80, mainPaneSize))
|
||||
const mainWidth = Math.floor(((windowWidth - dividerWidth) * boundedMainPaneSize) / 100)
|
||||
|
||||
const proc = spawn([tmux, "resize-pane", "-t", mainPaneId, "-x", String(mainWidth)], {
|
||||
stdout: "ignore",
|
||||
@@ -47,5 +49,6 @@ export async function enforceMainPaneWidth(
|
||||
mainPaneId,
|
||||
mainWidth,
|
||||
windowWidth,
|
||||
mainPaneSize: boundedMainPaneSize,
|
||||
})
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user