Skip to content

fix(terminal): resolve 'posix_spawnp failed.' on macOS terminal creation - #20

Merged
sambitcreate merged 15 commits into
feature/playwright-e2e-lmstudio-attachmentsfrom
fix/terminal-posix-spawnp-mac
Aug 11, 2026
Merged

fix(terminal): resolve 'posix_spawnp failed.' on macOS terminal creation#20
sambitcreate merged 15 commits into
feature/playwright-e2e-lmstudio-attachmentsfrom
fix/terminal-posix-spawnp-mac

Conversation

@sambitcreate

Copy link
Copy Markdown
Owner

Problem

Opening the terminal drawer fails with an opaque error on macOS:

Error invoking remote method 'terminal:create'
Error: posix_spawnp failed.

Root cause (reproduced)

The error string posix_spawnp failed. comes verbatim from node-pty's native C++ binding (src/unix/pty.cc), thrown when posix_spawn() of node-pty's own spawn-helper binary returns non-zero. It does not indicate a bad shell.

On macOS node-pty spawns spawn-helper first, and spawn-helper then execvps the shell. node-pty 1.1.0's npm prebuilt tarball restores spawn-helper with mode 0644 (no execute bit), so posix_spawn of a non-executable file fails.

Reproduced deterministically:

  • chmod 644 spawn-helperposix_spawnp failed.
  • chmod 755 spawn-helper → spawn succeeds

The existing runtime fix (ensureSpawnHelperExecutable) chmod\ed the helper but swallowed every failure silently (catch {}), so users got the opaque error with no clue why.

Fix (two layers — works on any Mac)

Layer 1 — runtime guard (main/services/terminal.ts):

  • Resolves every prebuilds/* helper (not just one guessed arch), including the packaged app.asar.unpacked copy
  • chmod only when the execute bit is missing (never needlessly rewrites an already-good file)
  • Verifies after and throws a descriptive, path-bearing error (Run "chmod 755 <path>"…) instead of swallowing — the load-bearing change that makes this whole class of bug self-diagnosing
  • Shell resolution now verifies the candidate is executable, with a fallback chain: $SHELL/bin/zsh/bin/bash/bin/sh — so a stale $SHELL can no longer break terminal creation

Layer 2 — build-time hardening (scripts/configure-electron-fuses.mjs, the afterPack hook):

  • Walks app.asar.unpacked/.../node-pty/prebuilds/*/spawn-helper in the packaged .app, chmod each to 0755
  • Throws in CI if any is missing or still non-executable — so a broken package fails at build time, not at the user's first terminal open

Why this works on any Mac

  • Packaged builds: Layer 2 guarantees the execute bit before the .app ships, regardless of how npm extracted the tarball
  • Dev / reinstalls: Layer 1 verifies and fixes (or fails loudly) at first terminal open
  • Wrong-arch / Rosetta: Layer 1 scans all prebuilds/* dirs
  • Exotic shells: Layer 1 verifies $SHELL and falls back to system shells
  • No more silent failures: every guard now verify-and-throws with a remediation hint

Verification

  • Reproduced → fixed end-to-end (restored the bug with chmod 644, ran the new logic → SPAWN OK)
  • tsc --noEmit clean
  • 15/15 tests pass (10 terminal + 5 fuse), including new coverage for chmod+verify, missing-helper no-op, already-executable no-op, shell fallback, and the descriptive-error paths

Files changed

  • main/services/terminal.ts — robust helper chmod+verify, async shell resolution with fallback
  • main/services/terminal.test.ts — 4 new tests
  • scripts/configure-electron-fuses.mjsmakeSpawnHelpersExecutable in afterPack
  • scripts/configure-electron-fuses.test.mjs — 3 new tests

Windows is unaffected (uses ConPTY, no spawn-helper).

…fore spawn (macOS posix_spawnp fix)

node-pty 1.1.0's npm prebuilt tarball restores spawn-helper without its
execute bit (0644). posix_spawn of a non-executable file is exactly what
surfaces to users as 'posix_spawnp failed.' the first time they open the
terminal drawer.

TerminalService.ensureSpawnHelperExecutable now resolves every prebuilds/*
helper (including app.asar.unpacked), chmods only when the execute bit is
missing, and verifies afterward — throwing a path-bearing remediation error
instead of silently swallowing (the prior catch {}). Shell resolution now
verifies the candidate is executable with a fallback chain (/bin/zsh ->
/bin/zsh -> /bin/bash -> /bin/sh) so a stale SHELL can no longer break
terminal creation.
Packaged builds must never ship spawn-helper without its execute bit. The
afterPack hook now walks app.asar.unpacked/.../node-pty/prebuilds/*,
chmods each spawn-helper to 0755, and throws in CI if any is missing or
still non-executable — so a broken package fails at build time rather than
at the user's first terminal open.

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

  • Terminal shell resolution with fallbackresolveShell checks candidates for executability via accessSync(X_OK), falling back from $SHELL through /bin/zsh, /bin/bash, to /bin/sh, with a descriptive error when no candidate is viable.
  • Runtime spawn-helper chmod + verifydefaultSpawnHelperPaths enumerates every prebuilds/*/spawn-helper (including app.asar.unpacked mirrors), ensureHelperExecutable chmods to 0755 only when needed and verifies after — throwing descriptive, path-bearing errors instead of the old silent catch {}.
  • Build-time afterPack hardeningmakeSpawnHelpersExecutable walks the packaged app.asar.unpacked tree in the afterPack hook, chmods every spawn-helper, and fails the build if none is found or any remains non-executable.
  • Tests — 8 new tests (5 terminal-service, 3 fuse-script) covering shell fallback, shell rejection, chmod+verify, missing-helper no-op, already-executable no-op, and the three fuse-script scenarios.

Pullfrog  | View workflow run | Using DeepSeek Pro𝕏

…ation

Adds TerminalHistoryStore: a debounced, line-capped, per-workspace log of
terminal output rooted at <userData>/terminal-history. Output is sanitized
before persisting so a replayed snapshot cannot trigger fresh shell replies
— CSI cursor-position reports, device-attributes/status queries, DECRQM/PM,
XTVERSION, Kitty keyboard, DCS DECRQSS/XTGETTCAP, and OSC color queries are
stripped while benign SGR/cursor sequences survive. Partial sequences split
across chunks are carried via a pending prefix.

Ported from t3code's sanitizeTerminalHistoryChunk (Manager.ts:953).
Shell fallback: the spawn path now walks an executable candidate list
($SHELL -> /bin/zsh -> /bin/bash -> /bin/sh) and retries the next on a
retryable failure (posix_spawnp failed, ENOENT, not found). A broken $SHELL
self-heals instead of throwing. Non-retryable errors (EINVAL, out of fds)
surface immediately. The session result gains resolvedShell and
preferredShellSkipped so the renderer can tell the user which shell launched.

History wiring: TerminalService now accepts an optional historyStore. On open
the prior sanitized output seeds the buffer (the renderer re-hydrates xterm
from snapshot, so no renderer change is needed for the seed); each PTY data
event appends to the store; terminate/exit flush the final chunk.
…hell refactor

The phase3 contract test asserts the terminal.ts source orders revalidate →
abort-check → spawn → abort-check. The spawn call changed shape (single
spawn → trySpawnShell destructure) in the shell-fallback PR; update the
assertion to match while preserving the ordering invariant it protects.
@sambitcreate
sambitcreate changed the base branch from main to feature/playwright-e2e-lmstudio-attachments August 11, 2026 15:34
…history

feat(terminal): shell fallback retry + persisted sanitized history

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Important

Three new shell-retry tests fail on systems where /bin/zsh is not installed (did on this CI runner). The isExecutable filter at main/services/terminal.ts:250 runs a real accessSync(X_OK) before the spawn mock has a chance to throw, so tests that supply ["/bin/zsh", "/bin/sh"] as candidates silently test a different code path when /bin/zsh is absent on the host.

Reviewed changes

These commits since the prior Pullfrog review (350dea4) add shell retry-loop logic, a per-workspace terminal history store with escape-sequence sanitization, and the IPC/renderer surface for surfacing fallback-shell use to the user.

  • Shell-candidate retry loopdefaultShellCandidates() returns $SHELL then /bin/zsh/bin/bash/bin/sh with dedup; trySpawnShell() tries each verified-executable candidate, retrying on posix_spawnp failed/ENOENT but rethrowing non-retryable errors immediately.
  • Per-workspace history store with sanitization — New terminal-history.ts module with TerminalHistoryStore (debounced disk writes), sanitizeTerminalHistoryChunk (strips CSI/DCS/OSC device queries from replayed output), and capHistory (5000-line bound).
  • Fallback-shell surfaceTerminalSessionInfo now carries resolvedShell and preferredShellSkipped; the renderer shows a one-time toast when the preferred shell was unavailable.
  • Test registration — Terminal and terminal-history test files registered in package.json test and test:coverage scripts; subagent contract test updated for the refactored spawn line.

ℹ️ Nitpicks

  • TerminalHistoryStoreOptions.now is declared but never usedmain/services/terminal-history.ts:31: the now?: () => number field is documented as a test seam but is never read in the constructor or anywhere in the implementation. Remove it or wire it to the schedule fallback so it isn't dead API surface.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow run | Using DeepSeek Pro𝕏

Comment thread main/services/terminal.ts Outdated

@pullfrog pullfrog Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

✅ No new issues found.

Reviewed changes

These commits since the prior Pullfrog review (5718840) add the shellIsExecutable injectable test seam, remove dead code, and update tests for cross-host portability.

  • Added shellIsExecutable test seam to TerminalServiceOptionscreate() now resolves this.options.shellIsExecutable ?? isExecutable instead of calling the module-level isExecutable directly, so shell-retry tests control the executability filter alongside the spawn mock.
  • Removed dead now field from TerminalHistoryStoreOptions — the unused now?: () => number field has been dropped.
  • Updated shell-retry tests for portability — four retry-loop tests now inject shellIsExecutable: () => true, making them independent of the host's filesystem.
  • Adjusted subagent contract test — the source-reading assertion updated to match the refactored const { pty, destructuring line.

Pullfrog  | View workflow run | Using DeepSeek Pro𝕏

@sambitcreate
sambitcreate merged commit 3a66cc9 into feature/playwright-e2e-lmstudio-attachments Aug 11, 2026
5 of 6 checks passed
@sambitcreate
sambitcreate deleted the fix/terminal-posix-spawnp-mac branch August 12, 2026 04:49
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.

1 participant