fix(pty-daemon): release macOS PTY master descriptors

Merged
KI
4mo ago
+628-63

Summary

  • add idempotent master-fd disposal for native node-pty adapters and inherited/adopted PTYs
  • dispose owned descriptors on natural exit, explicit close, failed or partial open/adoption, and normal daemon shutdown
  • preserve successful handoff ownership by skipping predecessor signaling and disposal after the successor acknowledges adoption
  • keep TreeKiller escalation and socket backpressure behavior intact

Root cause

The standalone pty-daemon removed exited sessions from its store but did not have an explicit descriptor-ownership teardown contract. The host-service reaper cannot close fds held by the detached daemon process, so master descriptors could accumulate across long-lived, high-churn workloads until macOS exhausted kern.tty.ptmx_max.

Implementation

Pty.dispose() now releases the process-owned master descriptor exactly once. Native PTYs destroy node-pty's read/write streams, while adopted PTYs destroy their inherited tty.ReadStream. Close paths signal through the existing TreeKiller before disposal, and its escalation chain continues independently so detached/background descendants are still reaped.

Failed native construction and later partial-open setup kill and dispose the newly created PTY. Partial successor adoption closes only the successor's inherited copies without signaling shells that remain owned by the predecessor. Explicitly closed sessions are immediately excluded from fd handoff while their eventual exit notification still completes normal store cleanup.

Successful handoff still calls close({ killSessions: false }), which neither kills nor disposes predecessor adapters.

The package README now documents the ownership rule and lifecycle coverage.

Validation

  • bun run test — 66 passed
  • bun run build:daemon
  • bun run test:integration — 56 passed, including control-plane, handoff, flow-control, TreeKiller, signal recovery, and new real-fd lifecycle suites
  • 128-cycle server churn with real OS descriptors
  • 128-cycle real node-pty master-fd churn plus native/adopted idempotency checks
  • bun run typecheck — repo-wide, 35 tasks passed
  • bun run lint:fix
  • bun run lint

Closes #5305

Open in Stage

Summary by cubic

Fixes PTY master-fd leaks by disposing descriptors on teardown across pty-daemon. Also prevents hidden macOS /dev/ptmx leaks by pinning node-pty to a fixed version.

  • Bug Fixes

    • Added dispose() to Pty (idempotent) for native node-pty and adopted PTYs.
    • Dispose on natural exit, explicit close (kill then dispose), failed/partial spawn or adoption, and daemon shutdown; skip after successful handoff.
    • Hardened open/adoption rollback: close only the successor’s inherited copies on failure.
    • Mark sessions exited on close and ignore stale exit events so recycled ids aren’t deleted.

  • Dependencies

    • Pin node-pty to 1.2.0-beta.14 to fix hidden macOS /dev/ptmx descriptor leaks; applied in apps/desktop, packages/host-service, and packages/pty-daemon.

Summary by CodeRabbit

  • New Features
    • Added an idempotent dispose() API for PTY instances (optionally preserving exit polling).
  • Bug Fixes
    • Improved PTY/session cleanup on open/adopt failures, close, shutdown, and natural exits.
    • Ensured stale exit events won’t affect newer sessions using the same id.
    • Prevented fd/PTY leaks during partial adoption and exit callback ordering.
  • Tests
    • Expanded coverage for native/adopted master fd lifecycle, churn, forced termination, and handoff behavior.
    • Strengthened integration and handler tests with dispose-aware fakes.
  • Documentation
    • Updated PTY daemon README with clearer fd lifecycle and testing guidance.