chore(deps): bump node-pty to 1.2.0-beta.12 (ptmx leak fix)#967
chore(deps): bump node-pty to 1.2.0-beta.12 (ptmx leak fix)#967jSydorowicz21 wants to merge 3 commits intoRunMaestro:rcfrom
Conversation
Stage 2 of the package-upgrade plan: take the beta to pick up the ptmx leak fix. Pinned exact (no caret) since beta channels don't follow semver in the usual way; we don't want a stealth jump to a new beta. - node-pty ^1.1.0 -> 1.2.0-beta.12 Verification: - npm install + electron-rebuild against Electron 28 succeeded - npm run lint passed (3 tsconfigs) - npm run lint:eslint passed - npm run format:check (src/) passed - npm run build succeeded (warnings pre-existing) - Targeted vitest: 131 PTY/process-manager tests passed - npm ls node-pty -> 1.2.0-beta.12
Add a "node-pty version pinning" subsection to PROCESS-SYSTEM.md so the deliberate exact-version pin (no caret) is discoverable in-tree, not just in the Stage 2 commit message. Future contributors running `npm outdated` get a clear answer for why this dep can't auto-bump through the beta channel and what criterion moves it back to a caret range.
|
ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
💤 Files with no reviewable changes (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughWalkthroughThis PR pins the Changesnode-pty Version Pin
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested labels
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Greptile SummaryThis PR pins
Confidence Score: 5/5Safe to merge — targeted dependency bump with no logic changes in application code. The only modified files are package.json, the lockfile, a deleted patch whose removal is required for patch-package to work correctly with the new version, and documentation. The postinstall script chains patch-package then electron-rebuild; with the patch gone and the version bumped, both steps proceed cleanly. No files require special attention. Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A[npm install] --> B[postinstall script]
B --> C[patch-package]
C --> D{patches/ dir}
D -- "Before PR\nnode-pty+1.1.0.patch" --> E[Apply local C++ patch\nto node-pty 1.1.0]
D -- "After PR\nempty" --> F[No patches to apply\ncontinue cleanly]
E --> G[electron-rebuild\n-f -w node-pty,better-sqlite3]
F --> G
G --> H[Native module rebuilt\nagainst Electron 28]
H --> I[node-pty 1.2.0-beta.12\nwith upstream ptmx FD-leak fix]
style E fill:#f99,stroke:#c33
style F fill:#9f9,stroke:#3c3
style I fill:#9cf,stroke:#39c
Reviews (2): Last reviewed commit: "chore(deps): remove orphaned node-pty pa..." | Re-trigger Greptile |
![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
🤖 Prompt for all review comments with 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.
Inline comments:
In `@package.json`:
- Line 284: The package.json dependency "node-pty": "1.2.0-beta.12" cannot be
installed from npm; replace that entry with a valid installable reference by
either pinning to the latest published version "node-pty": "1.1.0", switching to
the GitHub reference "node-pty": "github:microsoft/node-pty#v1.2.0-beta.12", or
switching to the scoped fork "@lydell/node-pty": "1.2.0-beta.12" so npm install
will succeed; update the dependency line for the symbol node-pty accordingly and
run npm install to verify.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 7a0a4d30-c8a8-4f21-94b3-ac77bdd6a9ce
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (2)
docs/agent-guides/PROCESS-SYSTEM.mdpackage.json
| "marked": "^17.0.1", | ||
| "mermaid": "^11.12.1", | ||
| "node-pty": "^1.1.0", | ||
| "node-pty": "1.2.0-beta.12", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Is node-pty version 1.2.0-beta.12 published on npm?
💡 Result:
No, node-pty version 1.2.0-beta.12 is not published on npm as a stable or latest version for the main "node-pty" package. The latest stable version on npm is 1.1.0, published on 2025-12-22 [1][2]. Beta versions like 1.2.0-beta.12 are available as GitHub prereleases (published 2026-03-12 [3]) and listed under the "beta" tag on npmx.dev [2], but not installable via standard npm for the main package's latest. A scoped fork "@lydell/node-pty" has published 1.2.0-beta.12 as its "latest" (15 days ago [4]), which is a repackaging of the GitHub release [5]. npmx.dev shows 1.2.0-beta.12 under "beta" tag for node-pty [6][2], suggesting it may be tagged but not latest.
Citations:
- 1: https://www.npmjs.com/package/node-pty
- 2: https://npmx.dev/package/node-pty/v/latest
- 3: https://github.com/microsoft/node-pty/releases/tag/v1.2.0-beta.12
- 4: https://www.npmjs.com/package/@lydell/node-pty?activeTab=versions
- 5: https://github.com/lydell/node-pty/releases/tag/v1.2.0-beta.3
- 6: https://npmx.dev/package/node-pty/v/1.2.0-beta.12
🏁 Script executed:
#!/bin/bash
# Confirm package-lock.json exists and captures the pinned node-pty version
fd -e json -g 'package-lock.json' --max-depth 2 | head -5
rg -n '"node-pty"' package-lock.json 2>/dev/null | head -10Repository: RunMaestro/Maestro
Length of output: 116
Cannot install node-pty@1.2.0-beta.12 with standard npm. This version is not published to the npm registry under the main node-pty package. It exists only as a GitHub prerelease and in a scoped fork (@lydell/node-pty). Running npm install will fail to resolve this dependency.
Either pin to the latest published version (1.1.0), switch to a GitHub reference (github:microsoft/node-pty#v1.2.0-beta.12), or use the scoped fork (@lydell/node-pty@1.2.0-beta.12).
🤖 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 `@package.json` at line 284, The package.json dependency "node-pty":
"1.2.0-beta.12" cannot be installed from npm; replace that entry with a valid
installable reference by either pinning to the latest published version
"node-pty": "1.1.0", switching to the GitHub reference "node-pty":
"github:microsoft/node-pty#v1.2.0-beta.12", or switching to the scoped fork
"@lydell/node-pty": "1.2.0-beta.12" so npm install will succeed; update the
dependency line for the symbol node-pty accordingly and run npm install to
verify.
There was a problem hiding this comment.
Beta.12 is pinned in npm and installs fine
There was a problem hiding this comment.
@jSydorowicz21, you're correct, and I apologize for the false alarm. Libraries.io confirms node-pty 1.2.0-beta.12 is present on the npm registry. An exact version specifier like "1.2.0-beta.12" in package.json resolves the pinned version directly from the registry without relying on the latest dist-tag, so npm install works fine. My original concern was mistaken.
✏️ Learnings added
Learnt from: jSydorowicz21
Repo: RunMaestro/Maestro PR: 967
File: package.json:284-284
Timestamp: 2026-05-08T01:04:44.558Z
Learning: In package.json, an exact (non-caret) pre-release version pin such as `"node-pty": "1.2.0-beta.12"` is perfectly valid and installable via npm as long as that exact version exists in the registry under any dist-tag (e.g. `beta`). npm resolves exact version specifiers directly without going through the `latest` dist-tag. Do not flag exact pre-release version pins in package.json as unresolvable unless the version is confirmed absent from the registry entirely.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
The local fork in patches/node-pty+1.1.0.patch (rich error reporting via std::string*, FD close on error paths, corrected close-low-fds loop bound) is now upstream in 1.2.0-beta.12. patch-package was failing on the stale patch; removing it is the actual fix, not switching to a github ref.
|
Update: deleted Verified locally:
|
|
@greptile rereview please |
1 participant
Summary
Pins
node-ptyto exact1.2.0-beta.12(no caret) to pick up the upstream ptmx file-descriptor leak fix that's not in the1.1.0stable line. Pin is exact deliberately — beta channels don't follow semver predictably, and a stealth jump to the next beta could breakelectron-rebuild. Move back to a caret range once the leak fix lands in a non-beta release.The pin rationale and exit criteria are documented in-tree at
docs/agent-guides/PROCESS-SYSTEM.md(new "node-pty version pinning" subsection) so future contributors runningnpm outdatedfind the answer.Verification
npm install+electron-rebuildagainst Electron 28 succeedednpm run lint/lint:eslint/format:checkcleannpm run buildclean/tmpext4 worktree: 775 files pass, 0 fail, 27,356 tests pass (identical torcbaseline)Test plan
rcbaselineSummary by CodeRabbit
Documentation
Chores
Bug Fixes