fix: add postinstall script to fix node-pty spawn-helper permissions on macOS#294
fix: add postinstall script to fix node-pty spawn-helper permissions on macOS#294amacsmith wants to merge 5 commits intositeboon:mainfrom
Conversation
…on macOS The node-pty package's spawn-helper binary may be shipped without execute permissions in npm tarballs, causing "posix_spawnp failed" errors on macOS when trying to start terminal sessions. This commit: - Adds a postinstall script that automatically fixes spawn-helper permissions - Adds troubleshooting documentation to README.md explaining the issue - Updates node-pty to stable v1.0.0 Fixes siteboon#284 Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdded detailed troubleshooting subsections to README.md (macOS posix_spawnp, expanded "No Claude projects found", and "File Explorer Issues") and modified package.json to add a Changes
Sequence Diagram(s)(omitted — changes are documentation and a small package.json update without new multi-component control flow) Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @README.md:
- Around line 324-326: Update the fenced code block that contains the line
"Error: posix_spawnp failed." in README.md to include a language specifier
(e.g., use ```text or ```plaintext) so the block becomes a labeled code fence;
locate the exact fenced block in the file and prepend the language token to
satisfy linting and ensure consistent rendering.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
README.mdpackage.json
🧰 Additional context used
🪛 markdownlint-cli2 (0.18.1)
README.md
324-324: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Agent
🔇 Additional comments (4)
package.json (3)
33-33: Postinstall script looks correct for the stated purpose.The inline script correctly constructs the platform-specific path and applies chmod 755. The try/catch gracefully handles cases where the file doesn't exist (e.g., Windows, or different architectures).
Minor observations:
- The script runs on all platforms but only macOS requires this fix. Consider adding a platform check (
process.platform === 'darwin') to skip unnecessary work on Linux/Windows.- If users have multiple node-pty versions or nested node_modules, the fix only applies to the top-level install.
These are minor optimizations and the current implementation is functional.
93-93: Verify: Unrelated dependency addition?The
task-master-aidependency appears unrelated to the PR's stated objective of fixing the node-pty spawn-helper permissions issue (#284). Consider splitting this into a separate PR for cleaner change tracking, or clarify in the PR description if this addition is intentional.
81-81: Good move to stable version.Downgrading from beta (
^1.1.0-beta34) to stable (^1.0.0) improves reliability. This aligns with the PR's goal of fixing terminal issues.README.md (1)
322-344: Well-documented troubleshooting section.The troubleshooting documentation is comprehensive, covering the problem, cause, solution, and permanent fix. The inclusion of architecture-specific paths (Intel vs Apple Silicon) and verification commands is helpful for users.
There was a problem hiding this comment.
Pull request overview
This PR addresses the posix_spawnp failed error on macOS by adding a postinstall script to automatically fix spawn-helper permissions and updating the node-pty dependency.
Changes:
- Adds postinstall script to chmod the node-pty spawn-helper binary to 755
- Updates node-pty from ^1.1.0-beta34 to ^1.0.0
- Adds comprehensive troubleshooting documentation for the macOS spawn-helper permissions issue
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| package.json | Adds postinstall script for fixing spawn-helper permissions, updates node-pty version, and adds task-master-ai dependency |
| README.md | Adds detailed troubleshooting section explaining the posix_spawnp error and solutions |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- Add `text` language specifier to error code block in README.md - Remove unrelated `task-master-ai` dependency - Improve postinstall error handling: only silence ENOENT errors, log actual permission failures for debugging Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
Hey @viper151 @blackmammoth 👋 This PR fixes issue #284 - the Summary of changes:
I've addressed the review feedback from CodeRabbit and Copilot:
Would appreciate a review when you get a chance. Thanks! 🙏 |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| **Cause**: The `node-pty` package's `spawn-helper` binary may be missing execute permissions after installation. This commonly happens on macOS when: | ||
| - Installing via `npx` or `pnpm` | ||
| - The npm tarball doesn't preserve the execute bit |
There was a problem hiding this comment.
The troubleshooting section describes the issue as "commonly happens on macOS when installing via npx or pnpm" but the postinstall script will run for all package managers (npm, yarn, pnpm, etc.) on all platforms. Consider clarifying in the documentation that while the issue commonly occurs with certain package managers on macOS, the postinstall fix applies universally. Alternatively, if the issue is truly specific to certain installation methods, the documentation should explain why the postinstall script may not always be sufficient.
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
There was a problem hiding this comment.
@copilot open a new pull request to apply changes based on this feedback
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
2 participants
Summary
Fixes the
posix_spawnp failederror that occurs on macOS when trying to start terminal sessions.postinstallscript topackage.jsonthat automatically fixesspawn-helperpermissionsnode-ptyto stable v1.0.0Problem
The
node-ptypackage'sspawn-helperbinary may be shipped without execute permissions in npm tarballs, causing errors like:This commonly happens on macOS when:
npxorpnpmSolution
The postinstall script runs
chmod 755on the spawn-helper binary after each install:Test plan
npm installproperly sets permissionsFixes #284
🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores
✏️ Tip: You can customize this high-level summary in your review settings.