Skip to content

fix(pty-daemon): raise daemon RLIMIT_NOFILE and surface real spawn errno #5067

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
saddlepaddle merged 1 commit into from
Jun 3, 2026
+41 −12
Merged

fix(pty-daemon): raise daemon RLIMIT_NOFILE and surface real spawn errno #5067

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 22 additions & 10 deletions packages/host-service/src/daemon/DaemonSupervisor.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -942,16 +942,28 @@ export class DaemonSupervisor {
// Prod: detached so PTYs survive host-service restarts via socket
// adoption. Dev: attached as defense-in-depth in case serve.ts's
// dev shutdown doesn't fire (e.g. host-service crash).
child = childProcess.spawn(
process.execPath,
[this.opts.scriptPath, `--socket=${socketPath}`],
{
detached: !isDev,
stdio,
env: childEnv,
windowsHide: true,
},
);
// Raise RLIMIT_NOFILE before exec: macOS's 256 soft default starves a
// daemon hosting many worktrees' PTYs and surfaces as node-pty
// "posix_spawnp failed" (EMFILE). The raised limit is inherited by
// handoff successors the daemon spawns from itself.
const isWindows = process.platform === "win32";
const command = isWindows ? process.execPath : "/bin/sh";
const commandArgs = isWindows
? [this.opts.scriptPath, `--socket=${socketPath}`]
: [
"-c",
'ulimit -n 1048576 2>/dev/null || ulimit -n "$(ulimit -Hn)" 2>/dev/null || true; exec "$@"',
"sh",
process.execPath,
this.opts.scriptPath,
`--socket=${socketPath}`,
];
child = childProcess.spawn(command, commandArgs, {
detached: !isDev,
stdio,
env: childEnv,
windowsHide: true,
});
} finally {
if (logFd >= 0) {
try {
Expand Down
21 changes: 19 additions & 2 deletions packages/pty-daemon/src/Pty/Pty.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,21 @@ function validateDims(cols: number, rows: number): void {
}
}

function reprobeErrno(meta: SessionMeta): string {
try {
const probe = childProcess.spawnSync(meta.shell, ["-c", ":"], {
cwd: meta.cwd,

@cubic-dev-ai cubic-dev-ai Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2: Errno reprobe does not use the same environment as the failing spawn, so reported errno can be incorrect/misleading.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/pty-daemon/src/Pty/Pty.ts, line 148:

<comment>Errno reprobe does not use the same environment as the failing spawn, so reported errno can be incorrect/misleading.</comment>

<file context>
@@ -142,6 +142,21 @@ function validateDims(cols: number, rows: number): void {
+function reprobeErrno(meta: SessionMeta): string {
+	try {
+		const probe = childProcess.spawnSync(meta.shell, ["-c", ":"], {
+			cwd: meta.cwd,
+			timeout: 1000,
+			stdio: "ignore",
</file context>
Suggested change
cwd: meta.cwd,
cwd: meta.cwd,
env: meta.env,

timeout: 1000,
stdio: "ignore",
});
Comment on lines +147 to +151

@coderabbitai coderabbitai Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify parity between nodePty.spawn and reprobeErrno spawnSync options in Pty.ts
fd -i 'Pty.ts' | xargs -I{} sh -c '
  echo "== {} ==";
  rg -n -C3 "nodePty\\.spawn\\(|spawnSync\\(" "{}";
  rg -n -C2 "env:\\s*meta\\.env" "{}";
'

Repository: superset-sh/superset

Length of output: 958

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Matched files by fd -i 'Pty.ts':"
fd -i 'Pty\.ts$' .

echo
echo "---- reprobeErrno block ----"
sed -n '120,220p' packages/pty-daemon/src/Pty/Pty.ts | cat -n

echo
echo "---- Search for spawnSync options containing env: meta.env ----"
rg -n --context 2 "spawnSync\\(" packages/pty-daemon/src/Pty/Pty.ts
rg -n --context 2 "env:\\s*meta\\.env" packages/pty-daemon/src/Pty/Pty.ts

Repository: superset-sh/superset

Length of output: 4552

🏁 Script executed:

#!/bin/bash
set -euo pipefail

rg -n "type SessionMeta|interface SessionMeta|SessionMeta" packages/pty-daemon/src/Pty/Pty.ts packages/pty-daemon/src -S

Repository: superset-sh/superset

Length of output: 1959

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,120p' packages/pty-daemon/src/protocol/messages.ts | nl -ba

Repository: superset-sh/superset

Length of output: 107

🏁 Script executed:

#!/bin/bash
set -euo pipefail

sed -n '1,120p' packages/pty-daemon/src/protocol/messages.ts | cat -n

Repository: superset-sh/superset

Length of output: 3608

Preserve spawn parity in reprobeErrno by passing meta.env to spawnSync.

reprobeErrno probes with cwd only, while the real nodePty.spawn uses env: meta.env; dropping env can misreport the errno for environment-driven failures.

Suggested patch 
 function reprobeErrno(meta: SessionMeta): string {
 	try {
 		const probe = childProcess.spawnSync(meta.shell, ["-c", ":"], {
 			cwd: meta.cwd,
+			env: meta.env,
 			timeout: 1000,
 			stdio: "ignore",
 		});
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const probe = childProcess.spawnSync(meta.shell, ["-c", ":"], {
cwd: meta.cwd,
timeout: 1000,
stdio: "ignore",
});
const probe = childProcess.spawnSync(meta.shell, ["-c", ":"], {
cwd: meta.cwd,
env: meta.env,
timeout: 1000,
stdio: "ignore",
});
🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/pty-daemon/src/Pty/Pty.ts` around lines 147 - 151, The reprobeErrno
probe currently calls childProcess.spawnSync(meta.shell, ["-c", ":"], { cwd:
meta.cwd, timeout: 1000, stdio: "ignore" }) without passing environment, so
update the options to include env: meta.env to match nodePty.spawn behavior;
specifically modify the spawnSync call inside reprobeErrno to pass the meta.env
property (env: meta.env) alongside cwd, timeout, and stdio so environment-driven
errno results are reproduced correctly.

if (!probe.error) return "ok";

@greptile-apps greptile-apps Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 "ok" in error message is confusing

When the re-probe succeeds (the EMFILE condition resolved between node-pty's failure and spawnSync), the thrown error message reads errno=ok, which is contradictory — the caller sees an exception but with a result string that implies success. A label like "non-sticky" or "resolved" would make it immediately clear that the condition was transient rather than suggesting the probe found no error.

Prompt To Fix With AI 
This is a comment left during a code review.
Path: packages/pty-daemon/src/Pty/Pty.ts
Line: 152

Comment:
**`"ok"` in error message is confusing**

When the re-probe succeeds (the EMFILE condition resolved between node-pty's failure and `spawnSync`), the thrown error message reads `errno=ok`, which is contradictory — the caller sees an exception but with a result string that implies success. A label like `"non-sticky"` or `"resolved"` would make it immediately clear that the condition was transient rather than suggesting the probe found no error.

How can I resolve this? If you propose a fix, please make it concise.

@cubic-dev-ai cubic-dev-ai Bot Jun 3, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P3: Returning "ok" here produces a confusing error message like errno=ok in the thrown exception — the caller sees an error but the embedded errno label suggests success. Use a label like "transient" or "non-reproducible" to clarify that the condition was not replicable at reprobe time.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At packages/pty-daemon/src/Pty/Pty.ts, line 152:

<comment>Returning `"ok"` here produces a confusing error message like `errno=ok` in the thrown exception — the caller sees an error but the embedded errno label suggests success. Use a label like `"transient"` or `"non-reproducible"` to clarify that the condition was not replicable at reprobe time.</comment>

<file context>
@@ -142,6 +142,21 @@ function validateDims(cols: number, rows: number): void {
+			timeout: 1000,
+			stdio: "ignore",
+		});
+		if (!probe.error) return "ok";
+		const e = probe.error as NodeJS.ErrnoException;
+		return e.code ?? e.message;
</file context>
Suggested change
if (!probe.error) return "ok";
if (!probe.error) return "non-reproducible";

const e = probe.error as NodeJS.ErrnoException;
return e.code ?? e.message;
} catch (e) {
return `reprobe-failed:${(e as Error).message}`;
}
}

export function spawn({ meta }: SpawnOptions): Pty {
validateDims(meta.cols, meta.rows);
// Pre-flight: node-pty's "posix_spawnp failed" message swallows errno
Expand Down Expand Up @@ -178,9 +193,11 @@ export function spawn({ meta }: SpawnOptions): Pty {
encoding: null,
});
} catch (err) {
// Annotate with shell + cwd so the wire-error reply is actionable.
// node-pty's native "posix_spawnp failed." drops the errno, so re-probe
// the same shell+cwd with spawnSync to surface the real code (e.g.
// EMFILE/EAGAIN/ENOENT).
throw new Error(
`spawn failed (shell=${meta.shell} cwd=${meta.cwd ?? "(none)"}): ${(err as Error).message}`,
`spawn failed (shell=${meta.shell} cwd=${meta.cwd ?? "(none)"} errno=${reprobeErrno(meta)}): ${(err as Error).message}`,
);
}
const adapter = new NodePtyAdapter(term, meta);
Expand Down
Loading