Skip to content

Normalize Windows PWD before nvm install#24

Merged
mokagio merged 12 commits into
mainfrom
mokagio/nvm-install-timeout
Jul 2, 2026
Merged

Normalize Windows PWD before nvm install#24
mokagio merged 12 commits into
mainfrom
mokagio/nvm-install-timeout

Conversation

@mokagio

@mokagio mokagio commented Jun 29, 2026

Copy link
Copy Markdown
Contributor

Since switching to Elastic CI Stack 6.67.0, our Windows machines hung when installing Node using the plugin.

I got GPT 5.5 to run a bunch of experiments and it discovered the issue was due to the / vs \ paths and how nvm handles them in the PWD lookup.

Also verified this doesn't break other repos that were not affected, e.g. Studio via Automattic/studio#3999

image

The fix, verified in those pipelines that were hanging and with new tests in CI to avoid regression, was to normalize Windows' PWD to the POSIX form.

AI-generated details

Rationale

nvm install can hang on Windows when Buildkite starts the hook with a Windows-shaped PWD, such as C:\....
In nvm v0.39.4, .nvmrc lookup walks PWD by trimming /; a Windows drive path can collapse to C: and loop forever.

This normalizes Windows PWD to the POSIX form with cygpath -u before invoking nvm, so .nvmrc lookup and activation run from a path nvm can trim safely.

CI coverage

  • Windows explicit versions: v20.19.5 and v24.15.0
  • Windows .nvmrc fallback: 20.19.5
  • Existing non-Windows version and curlrc checks
  • Shell helper and pre-command smoke tests via make test

How to test

  • make test
  • shellcheck hooks/pre-command hooks/lib/nvm-buildkite-plugin.bash tests/nvm-plugin-helpers-test.sh tests/pre-command-smoke-test.sh .buildkite/verify_node.sh
  • ruby -e 'require "yaml"; YAML.load_file(".buildkite/pipeline.yml")'
  • buildkite-agent pipeline upload --dry-run --format yaml --agent-access-token dummy .buildkite/pipeline.yml

Buildkite #102 passed.

Posted by Codex (GPT-5) on behalf of @mokagio with approval.

@mokagio mokagio self-assigned this Jun 29, 2026
@mokagio mokagio changed the title [codex] Bound nvm install hangs Bound nvm install hangs Jun 30, 2026
nvm v0.39.4 can loop forever while resolving `.nvmrc` from a Windows-shaped `PWD` under Git Bash/MSYS/Cygwin.
Convert that working directory to its POSIX form before invoking nvm so `.nvmrc` lookup and activation run from a path nvm can trim safely.

The CI coverage keeps the exact Beeper Node version and a second modern version on Windows, plus a Windows `.nvmrc` fallback check.

---

Generated with the help of OpenAI Codex, https://openai.com/codex
@mokagio mokagio force-pushed the mokagio/nvm-install-timeout branch from 2720f83 to 5ea78ee Compare June 30, 2026 03:22
mokagio added 2 commits June 30, 2026 13:24
The verifier should avoid command-phase `nvm use`, but it does not need to assert the shell executable path.
Using `nvm current` keeps the legacy version matrix working while the hook-level smoke tests and Windows jobs cover the path-normalization fix.

---

Generated with the help of OpenAI Codex, https://openai.com/codex
On Windows, sourcing nvm with `--no-use` leaves `nvm current` reporting `system` even when the hook activated the requested Node executable.
Check `node --version` there, while keeping the legacy `nvm current` check for the non-Windows matrix.

---

Generated with the help of OpenAI Codex, https://openai.com/codex
@mokagio mokagio changed the title Bound nvm install hangs Normalize Windows PWD before nvm install Jun 30, 2026
Generate the `.nvmrc` fallback fixture from a Buildkite repository hook instead of committing it as project configuration.
Also document the Windows verifier branch and why the helper library leaves strict mode to its callers.

---

Generated with the help of OpenAI Codex, https://openai.com/codex
Comment thread .buildkite/hooks/pre-command Outdated
Comment thread .buildkite/hooks/pre-command
mokagio and others added 3 commits June 30, 2026 13:50
Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>
The fallback checks already set the version used to generate `.nvmrc`.
Pass that same value to the verifier instead of repeating the literal in the command.

---

Generated with the help of OpenAI Codex, https://openai.com/codex
Buildkite exposes the fixture env var to repository hooks, but it is not present when the command runs.
Read the generated `.nvmrc` file in the verifier command instead of repeating the version literal.

---

Generated with the help of OpenAI Codex, https://openai.com/codex
Comment thread .buildkite/pipeline.yml Outdated
mokagio added 2 commits June 30, 2026 14:02
Windows agents can reach nvm code while `PWD` still has a drive-letter path.
Normalize before sourcing `nvm.sh`, so every nvm code path sees a POSIX-shaped working directory.

---

Generated with the help of OpenAI Codex, https://openai.com/codex
Comment thread hooks/lib/nvm-buildkite-plugin.bash Outdated
Co-authored-by: Gio Lodi <giovanni.lodi42@gmail.com>
@mokagio mokagio requested a review from Copilot June 30, 2026 04:15

Copilot AI 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.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@mokagio mokagio requested a review from a team June 30, 2026 04:33
@mokagio mokagio marked this pull request as ready for review June 30, 2026 04:33
@mokagio mokagio enabled auto-merge June 30, 2026 04:33
Convert common Windows PWD shapes in Bash before nvm loads.
This removes another external command from the path where build 111 appears to stall before printing the normalization message.

---

Generated with the help of Codex, https://openai.com/codex

Co-Authored-By: Codex GPT-5 <noreply@openai.com>

Copilot AI 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.

Pull request overview

Copilot reviewed 10 out of 10 changed files in this pull request and generated 2 comments.

Comment on lines +57 to +66
local normalized_pwd
if ! normalized_pwd="$(nvm_plugin_windows_pwd_to_posix "$PWD")"; then
echo "Cannot normalize Windows PWD for nvm from ${PWD}"
return 0
fi

if [[ -z "$normalized_pwd" || "$normalized_pwd" == "$PWD" ]]; then
echo "Cannot normalize Windows PWD for nvm from ${PWD}"
return 0
fi
Comment thread Makefile
Comment on lines +1 to 6
test:
bash tests/nvm-plugin-helpers-test.sh
bash tests/pre-command-smoke-test.sh

validate:
docker run -it --rm -v "$(shell pwd):/plugin:ro" buildkite/plugin-linter --id automattic/nvm
return 0
fi

if [[ ! -d "$normalized_pwd" ]]; then

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.

If we detect a Windows PWD that needs normalization but the normalized path does not exist, the helper logs and returns success, then we continue into source "$NVM_DIR/nvm.sh" --no-use. Since we're trying to avoid nvm getting stuck on that bad PWD, should this case fail fast instead? 🤔

@iangmaia iangmaia left a comment

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.

Assuming further testing on currently blocking jobs and that you're going to consider the review comments (not sure if they're blocker though), approving to unblock

The explicit-version Windows jobs hang and get killed (`0xC000013A`): with
no `.nvmrc` in the working directory, nvm v0.39.4's ancestor walk loops
forever under the Windows agent's bash. Setting
`NVM_BUILDKITE_PLUGIN_TEST_NVMRC_VERSION` makes the repo `pre-command` hook
write one before the plugin runs, short-circuiting the walk.

The plugin still installs via the `version:` property; the `.nvmrc` is only
there to satisfy nvm's directory walk, so this is a CI-harness fix with no
change to the plugin.

Proven green by build 116 (PR #29 experiment).

---

Generated with the help of Claude Code, https://claude.com/claude-code

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mokagio mokagio merged commit b1ba730 into main Jul 2, 2026
12 checks passed
@mokagio mokagio deleted the mokagio/nvm-install-timeout branch July 2, 2026 08:15
@mokagio

mokagio commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

🤔 I must have touched the wrong control on GitHub's mobile when looking at this PR because it was not my intention to merge it...

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants