Skip to content

fix: guard dependency installs on unsupported Node versions - #1346

Merged
Astro-Han merged 4 commits into
devfrom
codex/i1329-electron-repro
Jun 17, 2026
Merged

fix: guard dependency installs on unsupported Node versions#1346
Astro-Han merged 4 commits into
devfrom
codex/i1329-electron-repro

Conversation

@Astro-Han

@Astro-Han Astro-Han commented Jun 17, 2026

Copy link
Copy Markdown
Owner

Summary

Add a small install-time Node guard for PawWork dependency installs and pin the local recommended Node version with .node-version.

Why

Related to #1329.

New git worktrees can silently install an incomplete Electron 40.8.0 binary when bun install runs trusted dependency postinstalls through local Node 26. The broken install leaves Electron's dist as a small stub without path.txt or Electron.app/Contents/Frameworks, and the failure only appears later as Error: Electron uninstall when bun run dev reaches electron-vite dev.

This PR makes the unsupported Node runtime fail loudly during install with a recovery command, instead of letting a bad Electron binary persist.

Related Issue

Fixes #1329.

Human Review Status

Pending

Review Focus

Please check that the guard is scoped tightly to dependency installs and that the recovery message is clear enough for both humans and coding agents.

Risk Notes

The guard blocks Node 25+ for root dependency installs through the root preinstall lifecycle. CI already uses Node 24 for root installs, and site/ installs do not run the root install lifecycle. Developers on Node 25+ need to switch to Node 24 and reinstall dependencies.

How To Verify

Focused test: `bun run test:install-guard` -> 3 passed.
Node 26 guard: `node script/check-node.mjs` with v26.3.0 -> exits 1 with Node 24 reinstall guidance.
Node 24 guard: `/Users/yuhan/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin/node script/check-node.mjs` -> exits 0.
Install hook, Node 26: `bun install --frozen-lockfile` -> exits 1 during root preinstall with the same guidance.
Install hook, Node 24: `PATH=/Users/yuhan/.cache/codex-runtimes/codex-primary-runtime/dependencies/node/bin:$PATH bun install --frozen-lockfile` -> succeeds and then runs `fix-node-pty` from root postinstall.
Electron lifecycle probe: with `electron@40.8.0` trusted, a failing root preinstall blocks Electron's trusted postinstall before `path.txt` or `Electron.app/Contents/Frameworks` exist. Bun still unpacks the package stub before root preinstall.
Fresh worktree repro: on a detached worktree at this commit, Node 26 `bun install --frozen-lockfile` fails loudly at install time; after deleting `node_modules` and reinstalling with Node 24, Electron dist is 275M with `Frameworks` and `path.txt`, and `bun run dev` starts Electron.
Workflow check: `actionlint .github/workflows/ci.yml` -> clean.
Diff check: `git diff --check` -> clean.

Screenshots or Recordings

Not applicable. This changes CLI/install output only; the terminal output was verified in the checks above.

Checklist

How to use this checklist:

  • Tick a box by replacing [ ] with [x]. Do not edit, add, or remove items.
  • The bot-applied label items can only be honestly ticked AFTER the PR is opened and the labeler / priority-triage bots have run — return to the PR description and tick them then.
  • Most items are required. The few that are conditional are explicitly marked (conditional); for those, leave unticked if they truly do not apply and explain why in Risk Notes. All other items must be ticked before requesting human review.
  • Type label — this PR carries exactly one of bug, enhancement, task, documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.
  • Routing labels — this PR carries at least one of app, ui, platform, harness, ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.
  • Priority label — this PR carries exactly one of P0, P1, P2, P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.
  • Human Review Status above is set to Pending, Approved by @<reviewer>, or Not required: <reason> (default is Pending; "not required" is restricted to bot-authored low-risk PRs).
  • I linked the related issue, or stated in Summary why there is no issue.
  • I described the review focus and any meaningful risks.
  • I replaced the example block in How To Verify with the real verification steps and the key result for each.
  • I did not introduce unrelated refactors, dependencies, generated files, or file changes beyond the stated scope.
  • (conditional) I manually checked visible UI or copy changes when needed, with screenshots or recordings. Leave unticked only if no visible UI or copy changed.
  • (conditional) I considered macOS and Windows impact for platform, packaging, updater, signing, paths, shell, or permissions changes. Leave unticked only if no platform/packaging surface was touched.
  • (conditional) I called out docs, release notes, dependencies, permissions, credentials, deletion behavior, generated content, or local file changes when relevant. Leave unticked only if none of those surfaces was touched.
  • I reviewed the final diff for unrelated changes and suspicious dependency changes.
  • I am targeting dev, and my PR title and commit messages use Conventional Commits in English.

@Astro-Han Astro-Han added bug Something isn't working P2 Medium priority platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions labels Jun 17, 2026
@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@Astro-Han, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 17 minutes and 53 seconds. Learn how PR review limits work.

Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file).

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits.

🚦 How do rate limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan refill rate.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, the refill rate gradually slows as usage increases. The highest same-day bursts are limited more strictly.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: 67a9677f-9f71-444b-8164-2eaf8bfa4022

📥 Commits

Reviewing files that changed from the base of the PR and between 2b5f1bc and b67c66d.

📒 Files selected for processing (5)
  • .github/workflows/ci.yml
  • .node-version
  • package.json
  • script/check-node.mjs
  • script/check-node.test.mjs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/i1329-electron-repro

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions github-actions Bot added ci Continuous integration / GitHub Actions and removed platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions labels Jun 17, 2026

@github-actions github-actions 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.

Suggested priority: P2 (includes non-doc, non-test paths outside the low-risk bucket).

P1/P0 are reserved for maintainer confirmation. Please relabel manually if this is a release blocker, security issue, data-loss risk, or updater/runtime failure.

@Astro-Han Astro-Han added the platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions label Jun 17, 2026
@github-actions github-actions Bot removed the platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions label Jun 17, 2026
@Astro-Han Astro-Han added the platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions label Jun 17, 2026

@gemini-code-assist gemini-code-assist 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.

Code Review

This pull request introduces a Node.js version check script (script/check-node.mjs) during the postinstall phase to prevent dependency installation issues on Node 25+. It also adds a .node-version file and corresponding tests. The review feedback suggests making the version parser more robust by handling unparseable or custom Node version strings gracefully instead of throwing an unhandled exception, and adding a test case to verify this behavior.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread script/check-node.mjs Outdated
Comment thread script/check-node.test.mjs
@github-actions github-actions Bot removed the platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions label Jun 17, 2026
@Astro-Han Astro-Han added the platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions label Jun 17, 2026
@github-actions github-actions Bot removed the platform Electron shell, OS integration, packaging, updater, signing, paths, and permissions label Jun 17, 2026
@Astro-Han
Astro-Han merged commit 0549f2b into dev Jun 17, 2026
34 checks passed
@Astro-Han
Astro-Han deleted the codex/i1329-electron-repro branch June 17, 2026 09:54
Astro-Han added a commit that referenced this pull request Jun 17, 2026
Fix Electron's silent installation failure on Node 24.16.0+ and 26.x.

Root cause: extract-zip -> yauzl@2.x -> fd-slicer@1.x has a broken stream
lifecycle on newer Node versions, causing Electron's postinstall to silently
skip binary extraction (path.txt and Frameworks missing, dist is a 244K stub).

Fix: upgrade Electron from 40.8.0 to 40.10.3, which includes the upstream
fix (electron/electron#51886) that replaces extract-zip with
@electron-internal/extract-zip.

Changes:
- Upgrade electron 40.8.0 -> 40.10.3 in packages/desktop-electron
- Remove yauzl override from root package.json (no longer needed)
- Delete script/check-node.mjs and its test (Node 25+ guard removed)
- Remove preinstall hook and test:install-guard from package.json
- Remove install guard unit step from CI workflow
- Add --assert-complete flag to repair-electron-install.mjs
- Add install-matrix CI job covering Node 24 and 26
- Add pre-repair assertion in desktop-smoke workflow

Verification:
- bun install --frozen-lockfile: passes on Node 24 and 26
- install-matrix (24): SUCCESS
- install-matrix (26): SUCCESS
- desktop-smoke: SUCCESS
- Local Node 26 fresh install: path.txt + Frameworks + 275M dist confirmed

Related: electron/electron#51619, electron/electron#51886, #1329, #1346
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working ci Continuous integration / GitHub Actions P2 Medium priority

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[Bug] bun run dev cannot launch Electron inside a git worktree — dist binary is a 244K stub (vs 274M in main checkout)

1 participant