Found reviewing #37's bun-pty patch. The patch fixed the loss that was happening; the contract it leaves behind is invisible and one yield* away from bringing it back.
The contract
bun-pty's emitter delivers to whoever is subscribed at that instant and keeps nothing for anyone else (interfaces.ts, fire walks the current listener list). patches/bun-pty@0.4.8.patch defers the read loop's first read to a microtask, so nothing fires before spawn returns. That makes a subscription taken in the same JS turn as the spawn early enough — and nothing later is.
Pty.create satisfies it today, by accident of how it is written (packages/core/src/pty.ts:186 spawns inside Effect.sync, :223-243 subscribes in the generator code that follows). A review chased whether Effect's fairness yield could land in that gap and established that it cannot: sync invokes its continuation inside the same runLoop iteration, and the yield check is a separate step behind setImmediate, measured at 0 hits in 20000 iterations and 0 with MaxOpsBeforeYield forced down to 4.
So it works. What makes it fragile is that nothing enforces or states it. Insert any yield* of an async effect between the spawn and the session.listeners.push(...) — a config read, a plugin hook, a log that awaits — and every terminal starts losing its first output and short-lived ones their exit again, at the rate #29 records. No type, no test and no comment stands in the way.
Why the test cannot cover it
packages/core/test/pty/pty-spawn.test.ts is a batch of 30 short-lived sessions, and the loss is load-dependent: measured unpatched on four cores, idle is 1 lost exit in 150 while three busy cores give 8 in 150. So a regression of the call-site timing passes that test on an idle machine — a review reverted the deferral and watched it pass 6 runs in 10 locally. The patch itself is now guarded deterministically (patched-dependencies.test.ts asserts the added line is present exactly once), but that says nothing about the caller.
The fix that removes the class
Buffer in the shim. packages/core/src/pty/pty.bun.ts constructs the Terminal and can subscribe to it immediately, in the same turn, before returning its Proc. Events that arrive before the caller subscribes go into a queue; each of the caller's subscriptions flushes what is waiting for it and then streams live. After that, when the caller subscribes stops mattering, and the test can be deterministic: spawn, await a macrotask, subscribe, assert nothing was lost — which fails without the buffer on any machine, idle or loaded.
Points to settle rather than guess at:
- Ordering between the two kinds. Flushing at subscription time keeps data before exit as long as the caller registers
onData first, which Pty.create does. A caller that registers only onExit would see the exit before the buffered output. Either document that or keep one ordered queue and flush it when the first subscription of either kind arrives.
- A bound. A
Proc nobody ever subscribes to would accumulate its child's whole output. Pty caps its own buffer at 2 MiB; the shim's exists only to cover a gap of one turn, so a much smaller cap is enough, and it needs to be a cap rather than nothing.
- Whether the node shim needs it.
pty.node.ts uses @lydell/node-pty, which delivers through libuv asynchronously, so it has no equivalent window. Adding the buffer only where the hazard is keeps the two shims honestly different; adding it to both makes Proc's contract uniform. The second is probably worth more than it costs.
Found reviewing #37's bun-pty patch. The patch fixed the loss that was happening; the contract it leaves behind is invisible and one
yield*away from bringing it back.The contract
bun-pty's emitter delivers to whoever is subscribed at that instant and keeps nothing for anyone else (
interfaces.ts,firewalks the current listener list).patches/bun-pty@0.4.8.patchdefers the read loop's first read to a microtask, so nothing fires beforespawnreturns. That makes a subscription taken in the same JS turn as the spawn early enough — and nothing later is.Pty.createsatisfies it today, by accident of how it is written (packages/core/src/pty.ts:186spawns insideEffect.sync,:223-243subscribes in the generator code that follows). A review chased whether Effect's fairness yield could land in that gap and established that it cannot:syncinvokes its continuation inside the same runLoop iteration, and the yield check is a separate step behindsetImmediate, measured at 0 hits in 20000 iterations and 0 withMaxOpsBeforeYieldforced down to 4.So it works. What makes it fragile is that nothing enforces or states it. Insert any
yield*of an async effect between the spawn and thesession.listeners.push(...)— a config read, a plugin hook, a log that awaits — and every terminal starts losing its first output and short-lived ones their exit again, at the rate #29 records. No type, no test and no comment stands in the way.Why the test cannot cover it
packages/core/test/pty/pty-spawn.test.tsis a batch of 30 short-lived sessions, and the loss is load-dependent: measured unpatched on four cores, idle is 1 lost exit in 150 while three busy cores give 8 in 150. So a regression of the call-site timing passes that test on an idle machine — a review reverted the deferral and watched it pass 6 runs in 10 locally. The patch itself is now guarded deterministically (patched-dependencies.test.tsasserts the added line is present exactly once), but that says nothing about the caller.The fix that removes the class
Buffer in the shim.
packages/core/src/pty/pty.bun.tsconstructs theTerminaland can subscribe to it immediately, in the same turn, before returning itsProc. Events that arrive before the caller subscribes go into a queue; each of the caller's subscriptions flushes what is waiting for it and then streams live. After that, when the caller subscribes stops mattering, and the test can be deterministic: spawn,awaita macrotask, subscribe, assert nothing was lost — which fails without the buffer on any machine, idle or loaded.Points to settle rather than guess at:
onDatafirst, whichPty.createdoes. A caller that registers onlyonExitwould see the exit before the buffered output. Either document that or keep one ordered queue and flush it when the first subscription of either kind arrives.Procnobody ever subscribes to would accumulate its child's whole output.Ptycaps its own buffer at 2 MiB; the shim's exists only to cover a gap of one turn, so a much smaller cap is enough, and it needs to be a cap rather than nothing.pty.node.tsuses@lydell/node-pty, which delivers through libuv asynchronously, so it has no equivalent window. Adding the buffer only where the hazard is keeps the two shims honestly different; adding it to both makesProc's contract uniform. The second is probably worth more than it costs.