fix(install): recover frontend build from npm optional-deps bug (npm/cli#4828) - #4095
fix(install): recover frontend build from npm optional-deps bug (npm/cli#4828)#4095cykhoo0108 wants to merge 1 commit into
Conversation
|
👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated. Missing sections:
Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle. |
GPT 5.6 Review (fork) — 🔴 changes requested (blocking)Reviewed 1 of 1 blocking finding(s) are security-class and were withheld from adjudication, so the blocking verdict stands. BLOCKING -- install.sh:466 -- failed recovery deletes the tracked lockfile Adjudication (Opus 4.8) — is blocking on each finding proportionate?Fenced finding F1 — the deletion at [ADJUDICATION] 87dbd05 total=0 uphold=0 downgrade=0 |
Design Review (Fable 5, fork) — 🟡 CONCERNSDesign-level review of Design-Verdict: CONCERNS Real, live-verified failure; but the recovery's step 1 deletes a tracked lockfile — the one step the author's own repro showed doesn't work. Watch
Suggestions
[DESIGN-REVIEWED] 87dbd05 |
Opus 4.8 Review (fork) — ✅ no blocking findingsReviewed Review detailsNo findings that block; one advisory below. FINDING — install.sh:466 — [OPUS-REVIEWED] 87dbd05 |
|
Hi @cykhoo0108, a maintainer nudge on this one: it is carrying the Current state:
Could you rebase onto the latest git fetch upstream # or: git fetch origin, if this branch lives here
git rebase upstream/main
# resolve any conflicts, run the local gates below, then update the branch with
# a force-with-lease so the rebase lands without clobbering anyone else's workThe local gates to run before updating the branch: black src/kiro_crew test && isort src/kiro_crew test
flake8 src/kiro_crew test && mypy src/kiro_crew
python -m pytest
cd website && npm run build && npm run testA good part of this branch's redness is likely stale rather than a real defect: it predates a lot of what is now on |
|
🤖 Kiro Crew [operator: bolichen97#bb3ad1ca]: This PR has been inactive for 7+ days with failing CI. I've assessed the blockers and they appear resolvable — I'll push fixes directly to this branch as a co-author. Assessment: The only red is the GPT 5.6 BLOCKING finding (install.sh:416 — the recovery path If you'd prefer I don't touch this PR, add the |
Open PR relationship auditThis is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion. Relationship findings
No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit. |
…cli#4828) npm ci replays a lockfile resolved on another OS/arch and can skip the platform-specific native binding the bundler needs (e.g. @rolldown/binding-<os>-<arch>), so the cloud/first-run frontend build aborts with 'Cannot find native binding' and the dashboard serves 'Dashboard HTML not found'. Recover in two escalating steps when the build fails: re-resolve dependencies from scratch (re-evaluates optional deps for the current platform), then, if that still omits the binding, install the exact @rolldown/binding-<os>-<arch>[-abi] for the resolved rolldown version. Each step rebuilds; the first that produces a bundle wins. The fast npm ci path is unchanged when it works.
f267521 to
87dbd05
Compare
|
Rebased onto main Clean rebase: no conflicts, and the diff is unchanged (still Gates run locally: Please review the rebased branch. Note that a maintainer push makes the maintainer the last pusher, so under this repo's last-push rule a second approver is needed before merge. Heads up on overlap: open PR #2188 rewrites the entry to the same |
Problem / Motivation
On a fresh cloud launch (and any first-run/dev build) the frontend build can abort with:
install.shbuilds the website withnpm ciwhen apackage-lock.jsonis present.npm cistrictly replays the committed lockfile (resolved on a different OS/arch), so npm's long-standing optional-dependencies bug (npm/cli#4828) drops the platform-specific native binding the bundler (rolldown/vite) needs. The build fails, nowebsite/distis produced, and the gateway serves the "Dashboard HTML not found" page.Why it matters
For a cloud crew the dashboard is the product. The failure is silent from the user's side: the launch "succeeds" (the gateway answers on :5476) but every dashboard load shows "Dashboard HTML not found" with no obvious cause. It reproduces reliably on
linux-arm64(Amazon Linux 2023 arm64 EC2), a common cloud target.What changed (motivation → approach → change)
Symptom:
npm run buildaborts with "Cannot find native binding".Root cause:
npm ci+ [BUG] Platform-specific optional dependencies not being included inpackage-lock.jsonwhen reinstalling withnode_modulespresent npm/cli#4828 → the@rolldown/binding-<os>-<arch>optional dep for the current platform is missing fromnode_modules.Change: keep the fast
npm cipath when it works, but recover on build failure in two escalating steps:rm -rf node_modules package-lock.json && npm install) so npm re-evaluates optional deps for this platform, then rebuild.@rolldown/binding-<os>-<arch>[-abi]matching the resolvedrolldownversion (os/arch fromuname,-gnu/-musldetected vialdd), then rebuild.Each step rebuilds; the first that produces a bundle wins. The common (working)
npm cipath is untouched — the recovery only runs when the build would otherwise fail.Tests
No automated test change. The existing installer tests (
test/test_cloud_install.py,test/test_installer_shim_and_cleanup.py) do not assert on the frontend-build block, so they are unaffected.bash -n install.shpasses.Manual verification
This exact two-step recovery is what resolved the failure on a live
linux-arm64AL2023 EC2 instance during a cloud launch: a cleannpm installalone did not restore the arm64 binding, and installing@rolldown/binding-linux-arm64-gnu@<rolldown version>explicitly did — after whichnpm run buildproducedwebsite/distand the dashboard rendered. The script change mirrors those manual steps. I was not able to run the live arm64 cloud bootstrap from CI/locally (macOS host), so the fix is verified by shell-syntax check + reproduction of the manual remedy rather than an automated arm64 build.Related Issues
N/A
Checklist
fix: ...)