Sanskar Singh
cffc94a978
ConPTY's ClosePseudoConsole can deadlock when it tries to flush output to a pipe that nobody is reading (microsoft/terminal#1810). This caused the PTY Promise to never resolve on Windows, making bash commands with pty:true hang indefinitely. Root cause: portable-pty's drop(master) calls ClosePseudoConsole synchronously. If ConPTY's internal render thread is blocked writing to a full/undrained output pipe, ClosePseudoConsole waits forever. Fix (three parts): 1. Rust (pty.rs): Reordered teardown to follow Microsoft's recommended shutdown sequence: - Drop writer first (close ConPTY input pipe) - Drain reader thread with 500ms timeout (consume output pipe) - Drop master in a background thread with recv_timeout(2s): * Clean case: ClosePseudoConsole completes, thread reclaimed * Hung case: timeout expires, main thread returns anyway - Replace child.wait() with try_wait() polling on Windows (WaitForSingleObject can also hang in ConPTY) 2. TypeScript (bash-pty-selection.ts): Remove the Windows blanket disable that prevented PTY from ever being used on Windows. 3. Tests: Updated to verify PTY works on Windows with UI context.
65 lines
1.7 KiB
TypeScript
65 lines
1.7 KiB
TypeScript
import { afterEach, describe, expect, it } from "bun:test";
|
|
import { canUseInteractiveBashPty } from "@oh-my-pi/pi-coding-agent/tools/bash-pty-selection";
|
|
|
|
const originalPlatform = process.platform;
|
|
const originalNoPty = Bun.env.PI_NO_PTY;
|
|
|
|
function setPlatform(platform: NodeJS.Platform): void {
|
|
Object.defineProperty(process, "platform", {
|
|
value: platform,
|
|
configurable: true,
|
|
writable: true,
|
|
});
|
|
}
|
|
|
|
function restorePlatform(): void {
|
|
Object.defineProperty(process, "platform", {
|
|
value: originalPlatform,
|
|
configurable: true,
|
|
writable: true,
|
|
});
|
|
}
|
|
|
|
function setNoPty(value: string | undefined): void {
|
|
if (value === undefined) {
|
|
delete Bun.env.PI_NO_PTY;
|
|
return;
|
|
}
|
|
Bun.env.PI_NO_PTY = value;
|
|
}
|
|
|
|
function interactiveContext() {
|
|
return { hasUI: true, ui: {} };
|
|
}
|
|
|
|
describe("bash PTY selection", () => {
|
|
afterEach(() => {
|
|
restorePlatform();
|
|
setNoPty(originalNoPty);
|
|
});
|
|
|
|
it("allows interactive PTY on Windows when requested with UI", () => {
|
|
setPlatform("win32");
|
|
setNoPty(undefined);
|
|
|
|
expect(canUseInteractiveBashPty(true, interactiveContext())).toBe(true);
|
|
});
|
|
|
|
it("allows interactive PTY on non-Windows when requested with UI and not disabled", () => {
|
|
setPlatform("linux");
|
|
setNoPty(undefined);
|
|
|
|
expect(canUseInteractiveBashPty(true, interactiveContext())).toBe(true);
|
|
expect(canUseInteractiveBashPty(false, interactiveContext())).toBe(false);
|
|
expect(canUseInteractiveBashPty(true, undefined)).toBe(false);
|
|
|
|
setNoPty("1");
|
|
expect(canUseInteractiveBashPty(true, interactiveContext())).toBe(false);
|
|
});
|
|
|
|
it("disables interactive PTY when pty is false", () => {
|
|
setPlatform("win32");
|
|
expect(canUseInteractiveBashPty(false, interactiveContext())).toBe(false);
|
|
});
|
|
});
|