From 2a99a524ea99ac3eba5717da3bd0e4c5b8bc53fe Mon Sep 17 00:00:00 2001 From: YeonGyu-Kim Date: Sat, 18 Apr 2026 19:31:04 +0900 Subject: [PATCH] fix(tmux): drain kill-pane stdout to prevent pipe backpressure hang closeTmuxPane spawned kill-pane with stdout: "pipe" but never drained the stream, which could leave the subprocess hanging indefinitely when tmux wrote anything to stdout (for example under --force-close race conditions). - send-keys now uses stdout: "ignore" so there is no pipe to drain - kill-pane keeps the pipe but drains stdout/stderr alongside proc.exited - switch imports to the new spawn-process helper so the behavior is covered by hermetic tests that mock the spawn boundary --- src/shared/tmux/tmux-utils/pane-close.test.ts | 197 ++++++++++++++++++ src/shared/tmux/tmux-utils/pane-close.ts | 28 ++- src/shared/tmux/tmux-utils/spawn-process.ts | 1 + 3 files changed, 216 insertions(+), 10 deletions(-) create mode 100644 src/shared/tmux/tmux-utils/pane-close.test.ts create mode 100644 src/shared/tmux/tmux-utils/spawn-process.ts diff --git a/src/shared/tmux/tmux-utils/pane-close.test.ts b/src/shared/tmux/tmux-utils/pane-close.test.ts new file mode 100644 index 000000000..ca5d74684 --- /dev/null +++ b/src/shared/tmux/tmux-utils/pane-close.test.ts @@ -0,0 +1,197 @@ +import { beforeEach, describe, expect, it, mock } from "bun:test" + +type CloseTmuxPane = typeof import("./pane-close").closeTmuxPane + +type SpawnCall = { + command: string[] + options: { + stdout?: string + stderr?: string + } +} + +type FakeSubprocess = { + exited: Promise + stdout: ReadableStream + stderr: ReadableStream +} + +const TIMEOUT = Symbol("timeout") +const spawnCalls: SpawnCall[] = [] +const queuedProcesses: FakeSubprocess[] = [] + +function createClosedStream(): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.close() + }, + }) +} + +type DrainSignal = { onPull: () => void } + +function createDrainSensitiveStream(byteLength: number, signal: DrainSignal): ReadableStream { + let remainingBytes = byteLength + const chunk = new TextEncoder().encode("x".repeat(16 * 1024)) + + return new ReadableStream({ + pull(controller) { + signal.onPull() + + if (remainingBytes <= 0) { + controller.close() + return + } + + const nextChunkSize = Math.min(remainingBytes, chunk.byteLength) + controller.enqueue(chunk.subarray(0, nextChunkSize)) + remainingBytes -= nextChunkSize + }, + }) +} + +function createProcess(exitCode: number): FakeSubprocess { + return { + exited: Promise.resolve(exitCode), + stdout: createClosedStream(), + stderr: createClosedStream(), + } +} + +function createStdoutSensitiveProcess(exitCode: number, stdoutBytes: number): FakeSubprocess { + let resolveDrained: () => void = () => undefined + const drained = new Promise((resolve) => { + resolveDrained = resolve + }) + const stdout = createDrainSensitiveStream(stdoutBytes, { onPull: () => resolveDrained() }) + + return { + exited: drained.then(() => exitCode), + stdout, + stderr: createClosedStream(), + } +} + +const spawnMock = mock((command: string[], options: { stdout?: string; stderr?: string } = {}): FakeSubprocess => { + spawnCalls.push({ command, options }) + + const process = queuedProcesses.shift() + if (!process) { + throw new Error(`No fake subprocess configured for ${command.join(" ")}`) + } + + return process +}) + +const isInsideTmuxMock = mock((): boolean => true) +const getTmuxPathMock = mock(async (): Promise => "tmux") +const logMock = mock(() => undefined) + +const paneCloseSpecifier = import.meta.resolve("./pane-close") +const environmentSpecifier = import.meta.resolve("./environment") +const loggerSpecifier = import.meta.resolve("../../logger") +const spawnProcessSpecifier = import.meta.resolve("./spawn-process") +const tmuxPathResolverSpecifier = import.meta.resolve("../../../tools/interactive-bash/tmux-path-resolver") + +async function loadCloseTmuxPane(): Promise { + const module = await import(`${paneCloseSpecifier}?test=${crypto.randomUUID()}`) + return module.closeTmuxPane +} + +function registerModuleMocks(): void { + mock.module(spawnProcessSpecifier, () => ({ spawn: spawnMock })) + mock.module(environmentSpecifier, () => ({ isInsideTmux: isInsideTmuxMock })) + mock.module(tmuxPathResolverSpecifier, () => ({ getTmuxPath: getTmuxPathMock })) + mock.module(loggerSpecifier, () => ({ log: logMock })) +} + +function resolveWithin(promise: Promise, milliseconds: number): Promise { + return Promise.race([ + promise, + new Promise((resolve) => { + setTimeout(() => resolve(TIMEOUT), milliseconds) + }), + ]) +} + +describe("closeTmuxPane", () => { + beforeEach(() => { + registerModuleMocks() + spawnCalls.length = 0 + queuedProcesses.length = 0 + spawnMock.mockClear() + isInsideTmuxMock.mockClear() + getTmuxPathMock.mockClear() + logMock.mockClear() + + isInsideTmuxMock.mockImplementation((): boolean => true) + getTmuxPathMock.mockImplementation(async (): Promise => "tmux") + }) + + it("#given pane exists #when closeTmuxPane called #then returns true and invokes send-keys + kill-pane in order", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + queuedProcesses.push(createProcess(0), createProcess(0)) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(true) + expect(spawnCalls).toEqual([ + { command: ["tmux", "send-keys", "-t", "%42", "C-c"], options: { stdout: "ignore", stderr: "ignore" } }, + { command: ["tmux", "kill-pane", "-t", "%42"], options: { stdout: "pipe", stderr: "pipe" } }, + ]) + }) + + it("#given not inside tmux #when closeTmuxPane called #then returns false without spawn", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + isInsideTmuxMock.mockImplementation((): boolean => false) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(false) + expect(spawnCalls).toHaveLength(0) + }) + + it("#given tmux not found #when closeTmuxPane called #then returns false without spawn", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + getTmuxPathMock.mockImplementation(async (): Promise => undefined) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(false) + expect(spawnCalls).toHaveLength(0) + }) + + it("#given kill-pane fails #when closeTmuxPane called #then returns false", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + queuedProcesses.push(createProcess(0), createProcess(1)) + + // when + const result = await closeTmuxPane("%42") + + // then + expect(result).toBe(false) + }) + + it("#given kill-pane stdout stream waits for drain #when closeTmuxPane called #then returns true once drainer consumes stdout", async () => { + // given + const closeTmuxPane = await loadCloseTmuxPane() + queuedProcesses.push(createProcess(0), createStdoutSensitiveProcess(0, 16 * 1024)) + + // when + const result = await resolveWithin(closeTmuxPane("%42"), 2000) + + // then + expect(result).not.toBe(TIMEOUT) + expect(result).toBe(true) + }) +}) diff --git a/src/shared/tmux/tmux-utils/pane-close.ts b/src/shared/tmux/tmux-utils/pane-close.ts index cc6f4b6c4..76d9dd11b 100644 --- a/src/shared/tmux/tmux-utils/pane-close.ts +++ b/src/shared/tmux/tmux-utils/pane-close.ts @@ -1,13 +1,18 @@ -import { spawn } from "bun" -import { getTmuxPath } from "../../../tools/interactive-bash/tmux-path-resolver" -import { isInsideTmux } from "./environment" - function delay(milliseconds: number): Promise { return new Promise((resolve) => setTimeout(resolve, milliseconds)) } +async function readStream(stream: ReadableStream | null | undefined): Promise { + return stream ? new Response(stream).text() : "" +} + export async function closeTmuxPane(paneId: string): Promise { - const { log } = await import("../../logger") + const [{ log }, { isInsideTmux }, { getTmuxPath }, { spawn }] = await Promise.all([ + import("../../logger"), + import("./environment"), + import("../../../tools/interactive-bash/tmux-path-resolver"), + import("./spawn-process"), + ]) if (!isInsideTmux()) { log("[closeTmuxPane] SKIP: not inside tmux") @@ -22,8 +27,8 @@ export async function closeTmuxPane(paneId: string): Promise { log("[closeTmuxPane] sending Ctrl+C for graceful shutdown", { paneId }) const ctrlCProc = spawn([tmux, "send-keys", "-t", paneId, "C-c"], { - stdout: "pipe", - stderr: "pipe", + stdout: "ignore", + stderr: "ignore", }) await ctrlCProc.exited @@ -31,12 +36,15 @@ export async function closeTmuxPane(paneId: string): Promise { log("[closeTmuxPane] killing pane", { paneId }) - const proc = spawn([tmux, "kill-pane", "-t", paneId], { + const killPaneProc = spawn([tmux, "kill-pane", "-t", paneId], { stdout: "pipe", stderr: "pipe", }) - const exitCode = await proc.exited - const stderr = await new Response(proc.stderr).text() + const [, stderr, exitCode] = await Promise.all([ + readStream(killPaneProc.stdout), + readStream(killPaneProc.stderr), + killPaneProc.exited, + ]) if (exitCode !== 0) { log("[closeTmuxPane] FAILED", { paneId, exitCode, stderr: stderr.trim() }) diff --git a/src/shared/tmux/tmux-utils/spawn-process.ts b/src/shared/tmux/tmux-utils/spawn-process.ts new file mode 100644 index 000000000..c75826cab --- /dev/null +++ b/src/shared/tmux/tmux-utils/spawn-process.ts @@ -0,0 +1 @@ +export { spawn } from "bun"