fix(ci): restore Windows MCP server support and add cross-platform script tests#121
Merged
Conversation
…ript tests Fix the polyglot shebang in scripts/run.cmd that broke Windows delegation (the -S flag in `env -S` is not supported on all platforms and caused the batch path to never be reached). Add a Windows batch test script and a dedicated CI job that runs script tests on both ubuntu-latest and windows-latest. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
hypn4
added a commit
to hypn4/lumen
that referenced
this pull request
May 3, 2026
cmd.exe echoed the polyglot prologue (`#!/bin/sh` and the redirect-comment line) to STDOUT before `@echo off` was set, prepending ~92 bytes of prompt-prefixed text to lumen's output. This broke MCP stdio JSON-RPC framing on Claude Code, Cursor, and other hosts that spawn run.cmd via .cmd file extension association: the parser saw `\nC:\path>#!/bin/sh\n...` as the first bytes and failed the handshake with "Failed to connect". PR ory#121 only verified stderr cleanliness, and the existing `for /f` test in test_run_cmd_windows.bat captured only the last stdout line, so the prompt-echo prefix was masked. Replace the prologue with a label-style line (`:<<"::CMDLITERAL"`) which cmd.exe does not echo even with `@echo on`, while sh treats it as a no-op `:` followed by a quoted heredoc that consumes the cmd-only block. The shebang and `exec run.sh` move to lines 5-6 where they are reached only by sh; on Linux, execvp falls back to /bin/sh on ENOEXEC, so direct execve of the file still routes to sh. Add Test 4 to test_run_cmd_windows.bat that spawns run.cmd via `cmd /c` (matching how MCP hosts launch it) and asserts the first stdout line is the run.bat output. Tighten Test 3 to require empty stderr instead of only checking absence of the historical `-S` error.
hypn4
added a commit
to hypn4/lumen
that referenced
this pull request
May 3, 2026
cmd.exe echoed the polyglot's first two lines (`#!/bin/sh` and `# 2>NUL & @goto batch`) to stdout before `@echo off` took effect, breaking MCP stdio JSON-RPC framing for hosts that spawn run.cmd via .cmd extension association (Claude Code, Cursor). Replace the prologue with `:<<"::CMDLITERAL"`: cmd.exe treats it as a non-echoing label, while sh treats it as a quoted heredoc that consumes the cmd-only block. Add a regression test to test_run_cmd_windows.bat that spawns run.cmd via `cmd /c` and asserts stdout is exactly the run.bat output — the existing `for /f` test only kept the last stdout line, masking any prefix pollution. Refs: ory#121
5 tasks
aeneasr
pushed a commit
that referenced
this pull request
May 5, 2026
cmd.exe echoed the polyglot's first two lines (`#!/bin/sh` and `# 2>NUL & @goto batch`) to stdout before `@echo off` took effect, breaking MCP stdio JSON-RPC framing for hosts that spawn run.cmd via .cmd extension association (Claude Code, Cursor). Replace the prologue with `:<<"::CMDLITERAL"`: cmd.exe treats it as a non-echoing label, while sh treats it as a quoted heredoc that consumes the cmd-only block. Add a regression test to test_run_cmd_windows.bat that spawns run.cmd via `cmd /c` and asserts stdout is exactly the run.bat output — the existing `for /f` test only kept the last stdout line, masking any prefix pollution. Refs: #121
This was referenced May 6, 2026
aeneasr
added a commit
that referenced
this pull request
May 10, 2026
…t workarounds The previous commit (01dcd96) made the Windows launcher e2e test pass by adding /Q to the cmd.exe invocation and a lastJSONObjectLine extraction helper that scanned for the last {...} line. Both masked a real production bug instead of fixing it: Claude Code does not invoke run.cmd with /Q, and its hook reader parses stdout strictly as JSON. The actual bug: cmd.exe echoes the polyglot's first two lines (`#!/bin/sh` and `# 2>/dev/null & @goto batch`) to stdout before `@echo off` fires inside the :batch label, prepending ~92 bytes of prompt-prefixed text to the binary's JSON output. Re-apply PR #145's `:<<"::CMDLITERAL"` heredoc/label trick: cmd.exe treats line 1 as a non-echoing label, sh sees `:` opening a quoted heredoc terminated by ::CMDLITERAL. PR #145 was reverted because byte 0 is now `:` not `#!`, so direct execve (Go's exec.Cmd, naive C programs) fails with ENOEXEC. But Claude Code, Cursor, OpenCode, and other Node.js MCP hosts spawn via libuv, which uses execvp — and execvp falls back to /bin/sh on ENOEXEC. Bash, dash, sh, zsh, ksh all handle ENOEXEC the same way when the file is invoked by name. So this polyglot is correct for every real production spawn path; only Go's bare exec.Cmd is the outlier. Strip the test workarounds: - Drop /Q from cmd.exe; the polyglot must produce clean stdout under a fresh @echo on cmd.exe (matches MCP host launch). - Delete lastJSONObjectLine; parse stdout strictly via json.Unmarshal, the same way Claude Code's hook reader does. - Invoke via /bin/sh on POSIX in the Go test (matches libuv execvp's ENOEXEC fallback). This is not a workaround — it matches Node.js spawn semantics. - Keep mustMkdirTempWithRetryCleanup; Windows briefly holding an exclusive handle on a freshly-exec'd .exe is a documented OS quirk, not a bug we cause. Broaden POSIX shell coverage in test_run_cmd.sh: now exercises bash, dash, sh, zsh, ksh, and direct invocation, so any cross-platform polyglot regression has a specific named test that fails — instead of a vague "broke linux and mac" symptom. Restore PR #145's stricter Windows batch tests: Test 3 asserts zero stderr bytes (cmd.exe must not surface command-not-found errors); Test 4 spawns via `cmd /c` (matching MCP host launch) and asserts stdout is exactly the run.bat output (no prompt-echo pollution). Refs: #121, #145 Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
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.
Fixes #114.
Summary
scripts/run.cmd— the oldenv -Spattern caused an error on some Unix platforms, preventing the batch (Windows) code path from being reached cleanly. The new pattern uses a plain#!/bin/shshebang with the batch redirect comment on the next line.scripts/test_run_cmd_windows.bat— a Windows batch test that verifiesrun.cmdcorrectly delegates torun.batwith the right arguments and produces no unexpected stderr.scriptsCI job that runs on bothubuntu-latestandwindows-latest, coveringtest_run.sh,test_run_cmd.sh(Unix), andtest_run_cmd_windows.bat(Windows).Test plan
bash scripts/test_run.sh— all 34 tests passbash scripts/test_run_cmd.sh— Unix delegation test passesscriptsmatrix job onwindows-latest)🤖 Generated with Claude Code