Fix Dax typecheck on providers that ship Node 20 - #430
thundergolfer wants to merge 1 commit into
Conversation
OpenCode's native-module install needs Node 22+, so replace a stale preinstalled Node instead of skipping the pin and failing at turbo. Co-authored-by: Cursor <cursoragent@cursor.com>
Contributor License AgreementThe following contributors need CLA coverage: |
There was a problem hiding this comment.
Devin Review found 3 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| if [[ "$need_node" -eq 1 ]]; then | ||
| local archive="node-v${NODE_VERSION}-${NODE_ARCH}.tar.gz" | ||
| local prefix="/opt/node-v${NODE_VERSION}-${NODE_ARCH}" | ||
| if ! curl -fsSL "https://nodejs.org/download/release/v${NODE_VERSION}/${archive}" -o "/tmp/${archive}"; then |
There was a problem hiding this comment.
🟡 Alpine upgrades install unusable Node
On musl with Node below 22, need_node downloads the glibc Node archive. The installed binary cannot start, so Alpine benchmarks fail.
Learn more
The script explicitly detects musl because standard Linux binaries can depend on glibc, which Alpine does not provide. The new version gate sends every pre-22 Node installation through the official Node archive. That archive uses glibc, unlike the musl-specific Bun archive selected by BUN_MUSL_SUFFIX. Extraction succeeds, but invoking the installed node fails because its dynamic loader is unavailable.
Example: An x86_64 node:20-alpine sandbox reports Node 20. need_node downloads node-v24.14.1-linux-x64.tar.gz, links it into /usr/local/bin, and preparation appears successful. The later node --version invocation cannot execute the binary, instead of reporting Node 24.
Recommended fix: On musl, install a Node 22+ musl build through Alpine's package repositories or another verified source. If no compatible release exists, emit a specific preparation error rather than installing the glibc archive. Validate the resulting node --version before completing prepare.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if [[ "$need_node" -eq 1 ]]; then | ||
| local archive="node-v${NODE_VERSION}-${NODE_ARCH}.tar.gz" | ||
| local prefix="/opt/node-v${NODE_VERSION}-${NODE_ARCH}" | ||
| if ! curl -fsSL "https://nodejs.org/download/release/v${NODE_VERSION}/${archive}" -o "/tmp/${archive}"; then |
There was a problem hiding this comment.
🟡 Node override bypasses required minimum
With BENCH_NODE_VERSION below 22, need_node installs that release without validation. OpenCode then runs on the unsupported version and can fail typecheck.
Learn more
BENCH_NODE_VERSION controls the archive selected by NODE_VERSION, while the new gate assumes any installed archive satisfies the Node 22 minimum. Nothing checks that assumption after extraction. A caller can therefore request an older release and receive a successful prepare phase even though the workload's stated runtime requirement remains unmet.
Example: With BENCH_NODE_VERSION=20.20.0 and no existing Node, need_node downloads and links Node 20.20.0. command -v node passes, but OpenCode still runs below Node 22 and can fail during dependency installation or typecheck.
Recommended fix: Parse and reject NODE_VERSION values below 22 before downloading, or verify the effective node --version after installation and fail preparation with a specific minimum-version error.
Was this helpful? React with 👍 or 👎 to provide feedback.
| for executable in node npm npx corepack; do | ||
| "${SUDO[@]}" ln -sfn "$prefix/bin/$executable" "/usr/local/bin/$executable" | ||
| done | ||
| hash -r 2>/dev/null || true |
There was a problem hiding this comment.
🟡 PATH precedence defeats Node upgrade
When old Node precedes /usr/local/bin, hash -r resolves the old executable again. The benchmark remains below Node 22 after installing the replacement.
Learn more
Bash's hash -r only clears cached command locations. The next lookup still scans directories in their existing PATH order. If a provider prepends its bundled Node directory, linking the replacement under /usr/local/bin does not change which binary runs.
Example: A sandbox has PATH=/opt/provider/node/bin:/usr/local/bin:/usr/bin and Node 20 at /opt/provider/node/bin/node. Preparation installs Node 24 under /opt and links it into /usr/local/bin. After hash -r, node --version still resolves through /opt/provider/node/bin and reports Node 20.
Recommended fix: Prepend the installed prefix's bin directory to PATH, or invoke and export that absolute toolchain path. Then verify the effective node --version is at least 22 before returning from prepare.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Cannot run turbo.jsonbecause@computesdk/modaldefaults tonode:20and the benchmark skipped installing the pinned Node whenever anynodewas already on PATH.node-gyp/undicithat needs Node 22+ (util.markAsUncloneable). On Node 20,bun installdoes not writenode_modules/.bin, sobun turbo typecheckresolves toturbo.json.Verified on a Modal V2 sandbox with the default
node:20image: Node is upgraded tov24.14.1and all 7 Dax phases complete (bun turbo typechecksucceeds).Test plan
node:20image → typecheck fails onturbo.json,node_modules/.bin/turbomissing)node:24node:20image upgrades tov24.14.1and finishes typechecksandbox-daxrun lists Modal as 7/7Made with Cursor