Expose foreground process group PID on Unix#918
Open
srid wants to merge 1 commit into
Open
Conversation
Add a `foregroundPid` getter that returns `tcgetpgrp(fd)` on the pty master fd, exposing the pid of the foreground process group attached to the pty (or `undefined` on failure). This is the same syscall the existing `pty_getproc` already invokes internally to resolve the foreground process *name* — it's just returning the pid instead of throwing it away. Useful for callers that need to identify which process is currently in the foreground of a terminal (e.g. to detect that an agent like claude-code is running, where the process name resolves to "node"). Cross-platform: works on both Linux and macOS via tcgetpgrp(3). Not exposed on Windows (no equivalent ConPTY concept).
srid
force-pushed
the
feat/foreground-pid-upstream
branch
from
f510252 to
cd071ee
Compare
Author
|
Hello! Just checking on this PR. Is there any interest in merging this feature? This is the only reason for https://kolu.dev/ to continue to maintain a node-pty fork. |
srid
added a commit
to juspay/kolu
that referenced
this pull request
Jun 2, 2026
**Long-lived Kolu sessions on Linux leak a `/dev/ptmx` master fd into every shell spawned after the first**, and those leaked masters ride along into each shell's children — the suspected cause of the [frozen split-terminal after a raw-mode TUI exits (#1076)](#1076), plus a steady fd leak that pressures `EMFILE` in busy sessions. The root cause was a **stale node-pty fork**: our pin (`fix/darwin-pty-fd-leak`) branched 2025-12-21 and sat **127 commits behind upstream**, predating the Linux fix for exactly this. This repoints node-pty to a refreshed fork branch and drops a fork patch that has since landed upstream. ### What changed | | Before (`fix/darwin-pty-fd-leak`) | After (`feat/foreground-pid-dist`) | | --- | --- | --- | | Base | upstream @ 2025-12-21 | upstream `main` (2026-05-18) | | Linux fd-close fix ([`52417d516`](microsoft/node-pty@52417d516)) | ❌ missing | ✅ included | | `foregroundPid` accessor | ✅ (fork patch) | ✅ (same patch, now also [PR #918](microsoft/node-pty#918)) | | macOS `/dev/ptmx` fix (#882) | fork patch | dropped — now upstream | | built `lib/` for git install | ✅ | ✅ | Upstream `52417d516` closes inherited fds in the `forkpty()` child before `exec` (`close_range(2)` → `/proc/self/fd` fallback). macOS was never affected (`POSIX_SPAWN_CLOEXEC_DEFAULT`), which is why this bug is **Linux-only**. ### Verification Measured on this branch with the rebuilt addon: - **Leak gone:** a 2nd shell spawned after the first now reports **0** leaked `/dev/ptmx` fds (it was **1** before — confirmed in a real Kolu split sub-terminal, whose shell held `fd 29 -> /dev/ptmx`, the main terminal's master). - **`foregroundPid` intact:** resolves to the foreground pgid (`== pid`), so `pty-host`'s spawn-time sanity check still passes — agent/Dock foreground detection unaffected. - `just check` clean; native `pty.node` compiles under nix (`node-gyp rebuild`). ### Fork housekeeping The upstreaming of `foregroundPid` is [microsoft/node-pty#918](microsoft/node-pty#918) (rebased here, mergeable, awaiting maintainer review). Once it merges, the fork shrinks to just the committed-`lib/` packaging shim — and could be retired entirely if we build node-pty from source in the nix derivation. Refs #1076. _Generated by Claude Code (model `claude-opus-4-8`)._
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Unix terminals can now report the foreground process group PID directly through a new
foregroundPidaccessor.processalready callstcgetpgrp(3)internally to resolve the foreground process name; this surfaces the underlying pgid so callers can identify the active foreground program without reverse-mapping from a process title.The accessor returns
number | undefinedon Unix —undefinedwhen the pty has exited ortcgetpgrpfails — and is optional on the publicIPtytype, since Windows/ConPTY has no equivalent concept. The native binding reuses the same syscall path the existingPtyGetProcalready relies on, so there's no new platform surface.Motivating use case: identifying which process owns a terminal's foreground when the process name alone is ambiguous — e.g. an interpreter-shimmed CLI whose kernel process name resolves to
node.Verification
npm run build(tsc) cleannpm test -- --grep "foreground process group"passes on macOS arm64main, no conflicts — a single additive commitGenerated by Claude Code (model
claude-opus-4-8).