-
Notifications
You must be signed in to change notification settings - Fork 39
fix: Windows compatibility for npm scripts and cross-platform CI #144
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.
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
Draft
ochafik
wants to merge
16
commits into
main
Choose a base branch
from
ochafik/fix-issue-142
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+734
−109
Conversation
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
Fixes #142 - Windows PowerShell install bug Changes: - Replace single quotes with escaped double quotes in generate:schemas (cmd.exe passes single quotes literally, breaking prettier glob matching) - Simplify prettier scripts: remove redundant --ignore-path flags (prettier auto-loads .gitignore and .prettierignore by default) - Remove $(pwd) from prettier scripts (bash-only syntax) - Add cross-platform CI matrix: Linux x64, Windows x64, macOS ARM64, macOS x64 - Use shell: bash for grep commands in CI (not available in Windows cmd.exe) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
commit: |
The bun npm package fails to install on Windows CI because it can't find @oven/bun-windows-x64. Since we already use oven-sh/setup-bun action in CI which installs bun globally, we don't need the npm package. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The bun npm package tries to install platform-specific binaries which fails on Windows CI. Since bun is installed via oven-sh/setup-bun action in CI (and developers install it globally), we don't need the npm package. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The examples use `INPUT=mcp-app.html vite build` which is bash syntax for setting environment variables. This doesn't work on Windows cmd.exe. Since examples aren't published to npm, we can skip building them on Windows. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Git on Windows auto-converts line endings to CRLF, causing prettier to report formatting issues. Force LF for all text files. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
This verifies that `npm install git+https://...` works on all platforms, which triggers the prepare script and requires bun to be available. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
- Added ubuntu-24.04-arm (Linux ARM64) - Added windows-11-arm (Windows ARM64) Note: WSL is not available on GitHub-hosted runners (requires nested virtualization which isn't supported). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The oven-sh/setup-bun action fails with 404 when trying to download Bun for Windows ARM64. Re-enable when Bun adds proper support. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Uses Vampire/setup-wsl action to install Ubuntu-24.04 in WSL and run the full build/test cycle. This tests the Linux-in-Windows scenario that was reported in issue #142. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The pkg-pr-new publish workflow was missing bun setup, causing "bun: not found" errors during npm ci (which triggers prepare script). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Windows ARM64 can run x64 binaries via emulation. Use bun-download-url to force x64-baseline build since native ARM64 Bun isn't available yet. Reference: oven-sh/bun#9824 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
The macos-13 based runner images are now retired. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Documents that Bun is required for development, with platform-specific install commands in collapsible sections. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
f54014e to
17cdf09
Compare
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Addresses github-advanced-security review comments. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
Fixes #142 - Windows PowerShell install bug
The tradeoff
This PR requires Bun to be installed globally for development (building from source, running examples). In exchange, we get full cross-platform compatibility with CI testing on 6 environments.
For end users: No change -
npm install @modelcontextprotocol/ext-appsworks without Bun (uses pre-built dist from npm).For developers: Install Bun first (see CONTRIBUTING.md for one-liner install commands per platform).
Changes
npm scripts (package.json)
generate:schemas(cmd.exe passes single quotes literally)--ignore-pathflags (prettier auto-loads.gitignoreand.prettierignore)$(pwd)from prettier scripts (bash-only syntax)bunfrom devDependencies (was causing Windows install failures; use global Bun instead)CI (.github/workflows/)
test-git-installjob to verifynpm install git+https://...workssetup-buntopublish.ymlworkflowshell: bashfor grep commands (not available in Windows cmd.exe)examples:buildon Windows (usesINPUT=...bash syntax).gitattributesDocumentation
CI Matrix
Install scenarios
npm install @modelcontextprotocol/ext-appsnpm install git+https://github.com/.../ext-appsTest plan
npm run build)npm test)npm run prettier)🤖 Generated with Claude Code