Skip to content

fix(install): recover frontend build from npm optional-deps bug (npm/cli#4828) - #4095

Open
cykhoo0108 wants to merge 1 commit into
kirodotdev:mainfrom
cykhoo0108:fix/cloud-frontend-native-binding
Open

fix(install): recover frontend build from npm optional-deps bug (npm/cli#4828)#4095
cykhoo0108 wants to merge 1 commit into
kirodotdev:mainfrom
cykhoo0108:fix/cloud-frontend-native-binding

Conversation

@cykhoo0108

@cykhoo0108 cykhoo0108 commented Aug 17, 2026

Copy link
Copy Markdown

Problem / Motivation

On a fresh cloud launch (and any first-run/dev build) the frontend build can abort with:

Cannot find native binding. npm has a bug related to optional dependencies (https://github.com/npm/cli/issues/4828).
... Cannot find module '@rolldown/binding-linux-arm64-gnu'

install.sh builds the website with npm ci when a package-lock.json is present. npm ci strictly 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, no website/dist is 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 build aborts with "Cannot find native binding".

  • Root cause: npm ci + [BUG] Platform-specific optional dependencies not being included in package-lock.json when reinstalling with node_modules present npm/cli#4828 → the @rolldown/binding-<os>-<arch> optional dep for the current platform is missing from node_modules.

  • Change: keep the fast npm ci path when it works, but recover on build failure in two escalating steps:

    1. Re-resolve dependencies from scratch (rm -rf node_modules package-lock.json && npm install) so npm re-evaluates optional deps for this platform, then rebuild.
    2. If the binding is still missing, install the exact @rolldown/binding-<os>-<arch>[-abi] matching the resolved rolldown version (os/arch from uname, -gnu/-musl detected via ldd), then rebuild.

    Each step rebuilds; the first that produces a bundle wins. The common (working) npm ci path 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.sh passes.

Manual verification

This exact two-step recovery is what resolved the failure on a live linux-arm64 AL2023 EC2 instance during a cloud launch: a clean npm install alone did not restore the arm64 binding, and installing @rolldown/binding-linux-arm64-gnu@<rolldown version> explicitly did — after which npm run build produced website/dist and 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

  • Single commit with a Conventional Commits title (fix: ...)
  • Existing tests pass and new tests added for new functionality (no new functionality; installer tests unaffected)
  • Self-review completed; code follows project style guidelines
  • Documentation updated (if applicable) — N/A
  • No secrets, credentials, or internal references in the diff

@cykhoo0108
cykhoo0108 requested a review from a team as a code owner August 17, 2026 08:25
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention labels Aug 17, 2026
@iamwhatever

Copy link
Copy Markdown
Collaborator

👋 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:

  • ## Problem / Motivation
  • ## What changed

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.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Aug 17, 2026
@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 87dbd0564216a53bd7a7c0bf7c51eb585fd360e2 via the fork AI-review pipeline; updated in place on each push.

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
rm -rf node_modules package-lock.json
Build failure -> fallback deletion -> failed npm install exits under set -e -> checkout loses its tracked lockfile.
Anchor: residual/crash-data-loss-corruption
Fix: preserve and restore package-lock.json on every recovery exit path.
[BLOCK-MERGE] 87dbd05
[GPT-REVIEWED] 87dbd05

Adjudication (Opus 4.8) — is blocking on each finding proportionate?

Fenced finding F1 — the deletion at rm -rf node_modules package-lock.json fires whenever the first _fe_build fails (patch line 59, if ! _fe_build), which is not an extreme condition: it is the designed trigger of this recovery block and covers exactly the common OOM / npm-cli#4828 native-binding failures this PR exists to handle. Because the triggering condition is common rather than extreme, I cannot complete a rarity argument, and the tracked website/package-lock.json is deleted from the user's clone before any restore. No condition combination here is so contradictory or writer-impossible that a human would plausibly accept the residual risk, so the FLAG bar is not met.

[ADJUDICATION] 87dbd05 total=0 uphold=0 downgrade=0
[GPT-ADJUDICATED] 87dbd05
[ADJUDICATION-FENCED] 87dbd05 fenced=1 flagged=0
UPHOLD-FENCED F1 install.sh:466 -- deletion fires on any first-build failure (the designed, common trigger), so no rarity argument holds and the residual risk of losing the tracked lockfile is not one a human should silently accept.
[GPT-ADJUDICATED-FENCED] 87dbd05

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 87dbd0564216a53bd7a7c0bf7c51eb585fd360e2 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

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

  • Step 1 runs rm -rf node_modules package-lock.json on a tracked file in the clone: the recovery leaves the working tree dirty, so the advertised update path (git pull && bash install.sh, printed at install.sh:722) starts failing or silently pins an unreviewed local re-resolution on exactly the machines that needed recovery. Per the PR's own manual verification ("a clean npm install alone did not restore the arm64 binding"), this destructive step also isn't the one that fixed anything — the lockfile-preserving --no-save binding install (step 2) was. Reorder step 2 first and/or git checkout -- package-lock.json after recovery.
    Clears when: the recovery either preserves/restores package-lock.json or tries the --no-save binding install before deleting it.
  • Recovery fires on any build failure, not the missing-binding signature: an unrelated deterministic build error (real compile break, OOM) now costs two additional full installs+builds, discards pinned dependency versions, and pushes the true error further down the log the failure path tails. $_fe_log already captures stderr — grep it for "Cannot find native binding" before escalating.
    Clears when: the escalation is gated on the missing-native-binding error signature in the build log.

Suggestions

  • The committed lockfile already pins every @rolldown/binding-* platform (including linux-arm64-gnu), so the targeted binding install is a complete remedy on its own; the full re-resolve step may be droppable entirely rather than hardened.

[DESIGN-REVIEWED] 87dbd05

@github-actions

github-actions Bot commented Aug 17, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 87dbd0564216a53bd7a7c0bf7c51eb585fd360e2 via the fork AI-review pipeline; updated in place on each push.

Review details

No findings that block; one advisory below.

FINDING — install.sh:466 — rm -rf node_modules package-lock.json runs on any first-build failure (TS error, disk-full, transient crash — not just the npm/cli#4828 case), deleting the git-tracked website/package-lock.json and regenerating it via the following npm install; since install.sh always runs against the clone (KIROCREW_APP_DIR="$(cd "$(dirname "$0")" && pwd)"), the user's checkout is left with a modified/possibly-differently-resolved lockfile and npm ci reproducibility is lost, with no benefit when the failure was unrelated to the binding → Fix: back up and restore package-lock.json around the re-resolve (cp before, restore on the way out), or drop only node_modules and defer the lockfile removal into the _fe_install_binding recovery path, rather than deleting the tracked file unconditionally.

[OPUS-REVIEWED] 87dbd05

@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention and removed readiness: checking Automated validation is still running labels Aug 17, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator

Hi @cykhoo0108, a maintainer nudge on this one: it is carrying the readiness: action required label and has had no activity for about 5 days, so it is not moving toward merge.

Current state:

  • Base: 438 commits behind main
  • Merge state: mergeable, but the readiness gate is still red
  • Red signals:
    • GPT 5.6 Review
    • PR Readiness

Could you rebase onto the latest main and push a fix?

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 work

The 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 test

A good part of this branch's redness is likely stale rather than a real defect: it predates a lot of what is now on main, and several of these gates (the Coverage Gate, the Windows shards, the AI review lanes) have changed since the last run here. A rebase alone often clears them. If something still fails afterwards and you believe it is a false positive, say so in a comment and we will take a look. If the change is no longer needed, feel free to close the PR.

@bolichen97
bolichen97 enabled auto-merge (squash) August 24, 2026 06:59
@iamwhatever iamwhatever added the needs-pr-triage PR scanner: awaiting automated triage label Aug 26, 2026
@bolichen97 bolichen97 added drive-to-green PR claimed by drive-to-green pipeline and removed needs-pr-triage PR scanner: awaiting automated triage labels Aug 26, 2026
@bolichen97

Copy link
Copy Markdown
Collaborator

🤖 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 rm -rf node_modules package-lock.json destroys uncommitted lockfile edits and dirties the tracked lockfile). Fix plan: make recovery non-destructive (remove only node_modules, keep the lockfile intact / restore it after success), gate the recovery on the missing-native-binding signature so unrelated build failures fail fast, and complete the missing PR-template sections. Design Review's advisory suggestions (invert the escalation ladder to try the targeted --no-save binding install first) align with the same fix and will be addressed together.

If you'd prefer I don't touch this PR, add the pr-no-autofix label.

@bolichen97

Copy link
Copy Markdown
Collaborator

Open PR relationship audit

This 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

  • PR #2188 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #2188: CONTINUE_DEVELOPMENT. Both edit the same ~15 lines of install.sh, so whichever lands second takes a textual conflict there; the two changes should be sequenced deliberately, and 4095's recovery ladder should sit inside the elif arm 2188 creates rather than be re-derived. Files: install.sh.

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.
@bolichen97
bolichen97 force-pushed the fix/cloud-frontend-native-binding branch from f267521 to 87dbd05 Compare September 8, 2026 14:31
@bolichen97

Copy link
Copy Markdown
Collaborator

Rebased onto main 53987e75 by a maintainer as part of the 2026-09-08 open-PR audit. Old head f2675214 -> new head 87dbd056.

Clean rebase: no conflicts, and the diff is unchanged (still install.sh only, +48/-5).

Gates run locally: bash -n install.sh and sh -n install.sh (both pass). No Python or frontend files are touched, so the Python and TypeScript gates do not apply.

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 install.sh frontend block (adding a prebuilt-dist skip branch and turning that if into an elif). The goals are complementary, but whichever lands second will need to re-apply its hunk. Reply if anything looks wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

drive-to-green PR claimed by drive-to-green pipeline fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants