Skip to content

Fix Dax typecheck on providers that ship Node 20 #430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
thundergolfer wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix Dax typecheck on providers that ship Node 20 #430

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 17 additions & 4 deletions benchmarks/scripts/dax-benchmark.sh
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,10 +157,22 @@ prepare() {
# setuptools is optional - only needed for node-gyp native module compilation,
# not for the clone/install/typecheck benchmark. Skip the check entirely.

# Install Node.js only if it's not already available. Some sandboxes (e.g.
# Vercel) ship Node.js pre-installed; respect that rather than trying to
# override it (the symlink may not take precedence in PATH).
if ! command -v node >/dev/null 2>&1; then
# Install the pinned Node.js if it is missing or older than 22. Some
# sandboxes (e.g. Vercel) ship a current Node.js; keep that rather than
# fighting PATH precedence. OpenCode's native-module install scripts
# require Node 22+ (recent node-gyp pulls an undici that needs
# util.markAsUncloneable).
local need_node=1
if command -v node >/dev/null 2>&1; then
local current_major
current_major="$(node -v 2>/dev/null || true)"
current_major="${current_major#v}"
current_major="${current_major%%.*}"
if [[ "$current_major" =~ ^[0-9]+$ ]] && (( current_major >= 22 )); then
need_node=0
fi
fi
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
Comment on lines +175 to 178

@devin-ai-integration devin-ai-integration Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Comment on lines +175 to 178

@devin-ai-integration devin-ai-integration Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Expand All @@ -176,6 +188,7 @@ prepare() {
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

@devin-ai-integration devin-ai-integration Bot Sep 14, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 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.

Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

fi
if ! command -v node >/dev/null 2>&1; then
printf 'BENCH_ERROR\tprepare\tnode_not_found\n' >&2
Expand Down