fix(scripts): normalise MinGW/MSYS/Cygwin uname for POSIX launcher#161
Open
hypn4 wants to merge 2 commits into
Open
fix(scripts): normalise MinGW/MSYS/Cygwin uname for POSIX launcher#161hypn4 wants to merge 2 commits into
hypn4 wants to merge 2 commits into
Conversation
Git Bash, MSYS2, and Cygwin all report `uname -s` strings like `MINGW64_NT-10.0-26200`, `MSYS_NT-10.0`, or `CYGWIN_NT-10.0`. The POSIX launcher lower-cased that string and used it verbatim as the OS slug, so on Windows it looked for `bin/lumen-mingw64_nt-...-amd64` (skipping the already-installed `bin/lumen-windows-amd64.exe`) and then tried to download `lumen-X.Y.Z-mingw64_nt-...-amd64` from GitHub releases, where no such asset exists. The result was a non-blocking SessionStart hook failure on every Claude Code session start whenever shell:true dispatched through Git Bash instead of cmd.exe (curl exit 22, 404). Map the three Windows POSIX environments to `windows` and append `.exe`, matching scripts/run.cmd and the goreleaser asset names. Add table-driven coverage in scripts/test_run.sh and align the two integration tests' expected binary path with the new normalisation.
Local development uses `make build-local`, which runs `go build -o bin/lumen .`. Go does NOT append `.exe` when `-o` is explicit, so on Windows the dev build is written to `bin/lumen` (no extension). The previous commit only looked for `bin/lumen.exe` and `bin/lumen-windows-amd64.exe`, which would silently fall through to a GitHub download path during `claude --plugin-dir .` development — regressing the dev workflow that the original two-candidate list was designed to preserve. Add `bin/lumen` (without EXT) as the middle candidate so the priority on Windows becomes: .exe dev build → extensionless dev build → downloaded artefact. Linux/macOS retain their existing two-candidate behavior (the duplicate is a no-op there). Add three table-driven tests covering the candidate ordering across windows (.exe), mingw-normalised, and linux configurations.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthroughThe launcher ChangesCross-platform binary resolution with Windows executable suffix handling
🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
scripts/run) useduname -slower-cased verbatim as theOS slug. On Git Bash / MSYS2 / Cygwin that produces
mingw64_nt-10.0-26200,not
windows— so the launcher skipped the already-installedbin/lumen-windows-amd64.exeand tried to download a non-existentlumen-X.Y.Z-mingw64_nt-...-amd64asset from releases.hook_non_blocking_erroron every Claude Code SessionStart whenevershell:truedispatched the hook through Git Bash instead ofcmd.exe(
curl: (22) The requested URL returned error: 404, ~2.7 s per session start).mingw*|msys*|cygwin*→windows+.exe, matching whatscripts/run.cmd,asset_nameintest_run.sh, and the goreleaser assetnames on GitHub already assume. Preserve the extensionless
bin/lumendev-build path so
make build-localstill works on Windows.Root cause
scripts/runhad three places that assumed$OSwas already a valid GitHubasset OS token:
On Git Bash,
uname -sreturnsMINGW64_NT-10.0-26200. Lower-casing leaves thetoken unchanged for our purposes, so the launcher:
bin/lumenandbin/lumen-mingw64_nt-10.0-26200-amd64— neithermatches the installed
bin/lumen-windows-amd64.exe.https://github.com/ory/lumen/releases/download/v0.0.40/lumen-0.0.40-mingw64_nt-10.0-26200-amd64— 404.
filename — 404 again.
22.scripts/run.cmdis correct because it hard-codes-windows-and.exe. Thegap is only on the POSIX path that runs on Git Bash dispatch.
Evidence (from a live Claude Code 2.1.140 session)
{ "type": "hook_non_blocking_error", "hookName": "SessionStart:startup", "stderr": "Failed with non-blocking status code: Downloading lumen v0.0.40 for mingw64_nt-10.0-26200/amd64...\ncurl: (22) The requested URL returned error: 404\n\nVersion v0.0.40 not found, resolving latest release...\nFalling back to v0.0.40...\ncurl: (22) The requested URL returned error: 404", "exitCode": 22, "command": "\"${CLAUDE_PLUGIN_ROOT}/scripts/run\" hook session-start lumen --host claude", "durationMs": 2773 }After this fix the same session emits a normal
Lumen index ready: …line.Changes
scripts/runmingw*|msys*|cygwin*→OS=windows; deriveEXT=.exewhen$OS = windows.${EXT}consistently to the bin candidate list, the post-download$BINARY, and both$ASSETconstructions (manifest and GitHub-API fallback).bin/lumencandidate somake build-local(whichruns
go build -o bin/lumen .— Go does not auto-append.exewhen-oisexplicit) is still discovered on Windows ahead of falling through to a
download.
scripts/test_run.shnormalise_oshelper + 7 table-driven cases covering mingw64/mingw32/msys/cygwin/windows passthrough/linux/darwin.
future change that drops the dev-build path is caught by tests.
_EXPECTED_BINARYwith the newnormalisation (otherwise they pass on Linux but fail on Git Bash).
Test plan
bash scripts/test_run.sh— 39 passed, 2 failed vs. main's29/2. Both pre-existing failures are the candidate-priority
[ -x ]checks on Git Bash (
touch+chmod +xdoesn't produce an exec bitfor extensionless files on MSYS2), unrelated to this PR.
golangci-lint run— 0 issues.go vet ./...— same external-dependency warnings as main (CGOtree-sitter constraint messages, called out as OK in
CLAUDE.md).go build -o bin/lumen .does not append.exeon Windows, confirming the dev-build regression risk the secondcommit guards against.
bash scripts/run hook session-start lumen --host claude→exit 0, validhookSpecificOutputJSON.cache; the
hook_non_blocking_erroris gone and the SessionStartadditional context (
Lumen index ready: …) appears as designed.Notes / out of scope
bin/lumen,bin/lumen-${OS}-${ARCH}) wasdesigned to let
make build-localwin over a downloaded artefact. This PRpreserves that ordering on every platform by introducing
${EXT}ratherthan rewriting it.
index_statushandler bypassesmerkle.IsRootUnindexableand walks the user's home directory anyway,triggering Windows file-lock errors on
AppData\Local\Temp\*.scratch. Thatis unrelated to launcher dispatch and will be filed/sent separately.
Summary by CodeRabbit
Bug Fixes
Tests