Skip to content

Commit 9934322

Browse files
committed
fix(pty): fix race condition in PTY read loop
Add temporary monkey patch to bun-pty _startReadLoop to prevent race condition until upstream PR is merged. Update echo test to use echo command for simpler testing. Add spawn repeat test.
1 parent 151d989 commit 9934322

3 files changed

Lines changed: 95 additions & 16 deletions

File tree

src/plugin/pty/manager.ts

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,15 @@ import { OutputManager } from './OutputManager.ts'
55
import { NotificationManager } from './NotificationManager.ts'
66
import { withSession } from './utils.ts'
77

8+
// Monkey-patch bun-pty to fix race condition in _startReadLoop
9+
// Temporary workaround until https://github.com/sursaone/bun-pty/pull/37 is merged
10+
import { Terminal } from 'bun-pty'
11+
const originalStartReadLoop = (Terminal.prototype as any)._startReadLoop
12+
;(Terminal.prototype as any)._startReadLoop = async function (...args: any[]) {
13+
await new Promise((resolve) => setTimeout(resolve, 0))
14+
return originalStartReadLoop.apply(this, args)
15+
}
16+
817
let onSessionUpdate: (() => void) | undefined
918

1019
export function setOnSessionUpdate(callback: () => void) {

test/pty-echo.test.ts

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -22,35 +22,33 @@ describe('PTY Echo Behavior', () => {
2222
it('should echo input characters in interactive bash session', async () => {
2323
const receivedOutputs: string[] = []
2424

25-
// Subscribe to raw output events
26-
onRawOutput((_sessionId, rawData) => {
27-
receivedOutputs.push(rawData)
28-
})
25+
const promise = new Promise<void>((resolve) => {
26+
// Subscribe to raw output events
27+
onRawOutput((_sessionId, rawData) => {
28+
receivedOutputs.push(rawData)
29+
if (receivedOutputs.join('').includes('Hello World')) {
30+
resolve()
31+
}
32+
})
33+
setTimeout(resolve, 1000)
34+
}).catch((e) => { console.error(e) })
2935

3036
// Spawn interactive bash session
3137
const session = manager.spawn({
32-
command: 'bash',
33-
args: ['-i'],
38+
command: 'echo',
39+
args: ['Hello World'],
3440
description: 'Echo test session',
3541
parentSessionId: 'test',
3642
})
3743

38-
// Wait for PTY to initialize and show prompt
39-
await new Promise((resolve) => setTimeout(resolve, 200))
40-
41-
// Send test input
42-
const success = manager.write(session.id, 'a')
43-
expect(success).toBe(true)
44-
45-
// Wait for echo to be processed
46-
await new Promise((resolve) => setTimeout(resolve, 200))
44+
await promise
4745

4846
// Clean up
4947
manager.kill(session.id, true)
5048

5149
// Verify echo occurred
5250
const allOutput = receivedOutputs.join('')
53-
expect(allOutput).toContain('a')
51+
expect(allOutput).toContain('Hello World')
5452

5553
// Should have received some output (prompt + echo)
5654
expect(receivedOutputs.length).toBeGreaterThan(0)

test/spawn-repeat.test.ts

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,72 @@
1+
import { describe, it, expect, beforeEach, afterEach, } from 'bun:test'
2+
import { initManager, manager, onRawOutput } from '../src/plugin/pty/manager.ts'
3+
4+
describe('PTY Echo Behavior', () => {
5+
const fakeClient = {
6+
app: {
7+
log: async (_opts: any) => {
8+
// Mock logger
9+
},
10+
},
11+
} as any
12+
13+
beforeEach(() => {
14+
initManager(fakeClient)
15+
})
16+
17+
afterEach(() => {
18+
// Clean up any sessions
19+
manager.clearAllSessions()
20+
})
21+
22+
it("should receive initial data reproducibly", async () => {
23+
const start = Date.now();
24+
const maxRuntime = 4000;
25+
let runnings = 1;
26+
while (Date.now() - start < maxRuntime) {
27+
console.log(`[TEST] Iteration ${runnings++}`);
28+
const { success, stderr } = Bun.spawnSync({
29+
cmd: ["bun", "test", "--test-name-pattern", "should receive initial data once"],
30+
stdout: "pipe",
31+
stderr: "pipe",
32+
env: { ...process.env, SYNC_TESTS: "1" },
33+
});
34+
expect(success, `stderr: ${stderr}`).toBe(true);
35+
}
36+
});
37+
38+
it.skipIf(!process.env.SYNC_TESTS)("should receive initial data once", async () => {
39+
const receivedOutputs: string[] = []
40+
41+
const promise = new Promise<void>((resolve) => {
42+
// Subscribe to raw output events
43+
onRawOutput((_sessionId, rawData) => {
44+
receivedOutputs.push(rawData)
45+
if (receivedOutputs.join('').includes('Hello World')) {
46+
resolve()
47+
}
48+
})
49+
setTimeout(resolve, 1000)
50+
}).catch((e) => { console.error(e) })
51+
52+
// Spawn interactive bash session
53+
const session = manager.spawn({
54+
command: 'echo',
55+
args: ['Hello World'],
56+
description: 'Echo test session',
57+
parentSessionId: 'test',
58+
})
59+
60+
await promise
61+
62+
// Clean up
63+
manager.kill(session.id, true)
64+
65+
// Verify echo occurred
66+
const allOutput = receivedOutputs.join('')
67+
expect(allOutput).toContain('Hello World')
68+
69+
// Should have received some output (prompt + echo)
70+
expect(receivedOutputs.length).toBeGreaterThan(0)
71+
});
72+
})

0 commit comments

Comments
 (0)