Skip to content

Stop every closed terminal leaking a pty, by moving node-pty to 1.2.0-beta.15 - #371

Merged
benfriebe merged 1 commit into
mainfrom
leak-pty
Oct 10, 2026
Merged

benfriebe merged 1 commit into
mainfrom
leak-pty

Conversation

@benfriebe

Copy link
Copy Markdown
Owner

Closes #370

Problem

Every terminal Kelpi ends leaves a /dev/ptmx (a whole pty pair) and a kqueue open in the process that spawned it. macOS has 511 ptys for the whole machine. Since #274 that process is the terminal host, which survives daemon restarts and handoffs, so the leak builds up over weeks until New Pane, and every other terminal app on the Mac, can't open a pty.

Cause

Two bugs in node-pty 1.1.0's macOS native code, both fixed upstream:

1.2.0-beta.15 has both. 1.1.0 is still npm's latest.

Change

  • node-pty is pinned to 1.2.0-beta.15 (exact, so it can't float to a later beta). For Kelpi, nothing else in the bump matters:
    • The native-module lookup and the spawn-helper path are unchanged, and so are the dependencies and install scripts.
    • resize() gains an optional pixel-size argument.
    • Spawn failures give real error messages instead of posix_spawnp failed..
  • New packages/daemon/src/host/pty-release.test.ts:
    • It runs an in-process terminal host on the real spawner, warms it up, then ends 10 shells: 5 hung up (what closing a pane does) and 5 by typing exit.
    • It then checks with lsof that every /dev/ptmx and kqueue fd opened since the baseline has closed.
    • It compares fd numbers, not counts, so a close elsewhere can't hide a leak.
    • It runs on macOS only, in a file of its own so its process holds no other test's terminals.
  • scripts/node-pty-exec-bit.mjs is deleted (A fresh worktree's pnpm install leaves node-pty's spawn-helper non-executable: 22 test failures, and dev-instance terminal panes die on creation #36). The beta ships spawn-helper as 0755, and the script's own header asked to be deleted once that happened. That removes the root postinstall, the --check step in pnpm test, and the pre-flight in dev-instance.mjs. The host runtime copy and stage-payload.mjs still set the bit on their copies; only their comments changed.
  • docs/terminal-host.md: §11 now says that only a full stop or its own idle exit retires a host. §12 lists the new test.

Hosts already running

No code change here, by choice. An in-app update or kelpi daemon restart hands every terminal to the host already running. That host keeps node-pty 1.1.0, and keeps leaking, until one full stop: kelpi daemon stop (or kelpid stop), then reopen. The full stop ends every shell, and it also frees the ptys that host has already leaked. Two ways to do this automatically, neither done here:

  • Bump the host protocol H. The first boot ends the old host, which kills every shell once on update.
  • Drain. New terminals go to a new host while the old one keeps its terminals until they end (§11's future upgrade path).

Testing

  • The new test on node-pty 1.1.0 (origin/main) fails: all 10 ptmx and all 10 kqueue fds stay open. On this branch it passes.
  • A standalone node-pty probe: 10 shells on 1.1.0 left +10 ptmx and +10 kqueue. On the beta: 0 and 0.
  • A fresh install with --ignore-scripts gets spawn-helper as -rwxr-xr-x straight from the tarball.
  • pnpm check: root 643 files / 10291 tests passed, shell 57 files / 1057 tests passed.
  • node scripts/verify.mjs --full: typecheck, root tests, shell tests, bundles, the full audit and the packaged smoke all passed. The packaged smoke loaded node-pty from inside the bundle, ran a real PTY, and kept the shell's pid and scrollback across a SIGUSR2 handoff.
  • The scenario lane failed:
    • plugin-pane-chrome, plugin-pane-search and terminal-link-hover were red in the full lane and green when re-run alone.
    • plugin-settings-presenter was red alone too, on its last check: the cleanup leaves the Settings overlay open ("closed":false). The same check fails the same way on unmodified origin/main, so the red was there before this change.

Reviewer notes

A fresh Claude session and Codex reviewed the branch. Codex found nothing to act on; it also staged the daemon payload and spawned a PTY from it. Claude raised three low-severity points, all fixed:

  • The new doc paragraph said nothing but a full stop retires a host; the host's idle exit was missing.
  • The test compared counts; it now compares fd numbers.
  • The test ran lsof by bare name; it now uses /usr/sbin/lsof.

Not covered

  • No hand test in the app yet.
  • Automatic retirement of hosts that are already running (above).

🤖 Generated with Claude Code

https://claude.ai/code/session_01PzEkyw1X9oNR7zXuTPuCz2

…-beta.15 (#370)

node-pty 1.1.0 left a /dev/ptmx and a kqueue open in the terminal host for every
terminal it ever spawned. The host now outlives daemon restarts, so the leak piled up
until macOS ran out of its 511 ptys and no terminal app on the Mac could open one.
1.2.0-beta.15 carries both upstream fixes (microsoft/node-pty#882 and #931); 1.1.0 is
still the latest stable.

pty-release.test.ts counts the descriptors in the process that owns the ptys after ten
terminals end, half by hang-up and half by exit: 10 and 10 leaked on 1.1.0, 0 and 0 now.

The beta also ships spawn-helper executable, so the install-time repair for #36
(scripts/node-pty-exec-bit.mjs) goes, as its header asked. The runtime copy and the
payload stager still set the bit on their copies.

A host that is already running keeps the node-pty it started with until a full stop.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PzEkyw1X9oNR7zXuTPuCz2
@benfriebe
benfriebe merged commit f0172f8 into main Oct 10, 2026
@benfriebe
benfriebe deleted the leak-pty branch October 10, 2026 21:38
benfriebe added a commit that referenced this pull request Oct 11, 2026
…als now (#374)

Closes #372

## Problem
A terminal host keeps the code it started with, node-pty included, until it exits. The app never stops the daemon, and an update or `kelpi daemon restart` hands every terminal to the host already running. So a fix inside the host, like #371's pty leak, only reaches a machine after a full stop, which ends every shell.

## Change

### Host generations
- `HOST_PROTOCOL_VERSION` goes from 1 to 2. It is now the host's generation, bumped whenever a running host has to be replaced.
- `DRAINABLE_HOST_PROTOCOLS` lists the older generations a daemon drains rather than ends, each with the reason the user is shown. Generation 1 is there because it can leak a pty for every terminal it opens.
- `host-generation.test.ts` fails if node-pty's version changes without `HOST_NODE_PTY_VERSION`, and its message says to bump the generation.

### Draining (`boot/compose.ts`)
- On boot the daemon adopts the current host, then each drainable host it finds a pid record for. It reads the old host's token and says hello with the old protocol, the only one a generation 1 host accepts.
- A draining host keeps its terminals and gets no new ones. When its last terminal ends (`TerminalHostClient.onEmpty`), the daemon shuts it down.
- Panes whose shell exited while no daemon ran are closed only after every host has been read, so a pane living on the old host is never closed by mistake.
- Handoff holds, checkpoints and detaches every host, so a drain carries on across updates and restarts. Stop shuts all of them down. If a draining host dies, its panes are respawned on the current host and their agents resumed, as for the current host.
- A generation that is neither current nor drainable is still ended at boot.

### Restarting old terminals now
`restart-old-terminals` is a new wire verb (explicit chain, owner-only for paired devices). It:
1. reads each old shell's real folder with `lsof` and saves it to the pane;
2. captures each pane's agent session, and mutes agent hook events from the old shells while they end;
3. forgets the old terminals, shuts the old host down and waits for it to exit (SIGKILL after 3 s), so whatever it leaked is free before anything new spawns;
4. starts a fresh shell for each pane on the current host and types `claude --resume <id>` or `codex resume <id>`.

Only one runs at a time (a second request joins the first), a handoff or stop waits for it to finish, and it is refused when the daemon has no current host (terminals running in-process).

### Telling the user
- **Card:** a new WS message, `old-terminal-hosts`, is sent on change and after every snapshot. It drives a card in the toast corner saying how many terminals and agent sessions are on the old host, why it is being retired, and what restarting costs.
  - "Later" hides it for that host, remembered by pid and start time.
  - The palette row "Restart Old Terminals…" brings it back.
  - "Restart terminals" shows "Restarting…" until the restart is done.
- **CLI:** `kelpi daemon status` (and `kelpid status`, `--json` too) lists the current host and each old one. `kelpi daemon restart-old-terminals` says what it restarts and asks first at a terminal; `--force` skips the question.
- **Ping:** gains a `terminal_hosts` block.

### Host version
Every released host reports its version as `0.3.0-dev`: only the daemon gets `KELPID_VERSION`, so the host falls back to the compiled default. New hosts get `KELPID_VERSION` passed through. The card and status leave out an old host's version, since it would be wrong.

### Docs
- `docs/terminal-host.md`: §2 (H = 2), §8 (a draining host dying, forced stop), §10, §11 rewritten (generations, draining, restarting old terminals, telling the user), §12 tests.
- `docs/wire-protocol.md`: the new verb (§6.8.1), the allowlist and the ping block.

## Testing
- **Hand test in the app (owner):** a dev instance on `main` (generation 1) with several terminals and a Claude session, handed over with SIGUSR2 to a dev instance of this branch on the same state. It worked.
- **Against the real v0.2.15 release:**
  - A v0.2.15 daemon and host (node-pty 1.1.0) leaked 10 ptys (ptmx 4 → 14).
  - After a handoff to this branch's daemon, it adopted the terminals from the generation 1 host and drained it.
  - In headless Chrome: the card, Later (still hidden after a reload), the palette row, and Restart terminals. Both panes moved to the current host, a stub Claude resumed `--resume <id>` in its folder, and the old host exited.
  - `kelpid restart-old-terminals` did the same for 3 terminals, and a second run said there was nothing to do.
- **New tests:**
  - `boot/terminal-host.test.ts`, with real host processes and a generation 1 host built by rewriting the constant: draining across two handoffs with a new pane going to the current host; a restart into the live folder with the resume typed once; a handoff in the middle of a restart, and two restarts joining; refusing with no current host; retiring at boot; a draining host crashing; an undrainable generation ended.
  - `host.test.ts`: live counts and `onEmpty`; a mismatched protocol refused.
  - `main-restart-old.test.ts`: the prompt, yes and no, `--force`, scripts, nothing to do, status text and JSON.
  - Also `host-generation.test.ts`, the agent event mute, ping, the bridge, the palette, the card copy and the allowlist count.
- **`pnpm check`:** root 646 files / 10312 tests and shell 1057 tests passed. One run hit `EADDRINUSE` in `tailnet-binding.test.ts`, which passed 3/3 on its own.
- **`node scripts/verify.mjs --full`:** typecheck, root tests, shell tests, bundles, the full audit and the packaged smoke (74/74; ping shows `terminal_hosts` with generation 2) passed.
  - The scenario lane failed only on `plugin-pane-search`: "a daemon restart stands the placement down while disconnected" (`selection null`).
  - It fails the same way on unmodified `origin/main` (33/34), so it was red before this change. It looks like #348's cause.

## Reviewer notes
A fresh Claude session and Codex reviewed the branch. Both flagged a handoff or stop arriving during a restart; both now wait for the restart to finish. Also fixed from the reviews:
- refuse a restart when the daemon has no current host;
- save each recovered folder to its pane;
- capture agent sessions before the old host is ended, on host loss too;
- report an error when a restart is cut short;
- make the verb owner-only for paired devices;
- key "Later" by start time as well as pid;
- run the `lsof` lookups in parallel;
- add the interleaving tests above.

## Not covered
- **No redraw after a restart.** An open window keeps the old screen, and the new shell's output appears below it, as after a host crash today.
- **The resume command shows twice** on screen (the tty echo, then the shell's redraw), but runs once.
- **In-process fallback while a host drains:** if the current host could not start, so new terminals run in the daemon, a handoff keeps the draining host's terminals but ends the in-process ones, and still reports "handed-off".

🤖 Generated with [Claude Code](https://claude.com/claude-code)


Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Every closed terminal leaks a pty on macOS until the machine runs out

1 participant