Skip to content

fix(mcp-server): resolve Windows native host collision and CLI detection#16

Merged
iota31 merged 2 commits intoonllm-dev:mainfrom
PavelA85:fix/windows-native-host-and-cli-detection
Apr 28, 2026
Merged

fix(mcp-server): resolve Windows native host collision and CLI detection#16
iota31 merged 2 commits intoonllm-dev:mainfrom
PavelA85:fix/windows-native-host-and-cli-detection

Conversation

@PavelA85
Copy link
Copy Markdown
Contributor

This PR fixes two issues on Windows:

  1. Native Host Manifest Collision: Each browser (Chrome, Edge, Firefox) now has its own subdirectory for the native host manifest, preventing them from overwriting each other.
  2. CLI Detection: Added shell: true to spawnSync calls for claude and codex CLIs to ensure they are correctly found on Windows (where they often exist as .cmd wrappers).

- Ensure each browser has a unique manifest directory on Windows
- Use shell: true in spawnSync to correctly find .cmd CLIs on Windows
Copilot AI review requested due to automatic review settings April 21, 2026 16:26
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR addresses Windows-specific setup reliability issues in @onui/mcp-server by preventing native host manifest overwrites between browsers and improving detection/execution of .cmd-wrapped CLIs.

Changes:

  • Make Windows native host manifest directories browser-specific to avoid collisions.
  • Use spawnSync(..., { shell: process.platform === 'win32' }) for claude/codex CLI detection and MCP registration operations.
  • Apply the same Windows shell behavior in doctor checks for claude/codex MCP registration.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/mcp-server/src/store/path.ts Adds per-browser subdirectory for native host manifests on Windows (and fallback default).
packages/mcp-server/src/setup/configure-codex.ts Ensures codex is discoverable on Windows by enabling shell execution.
packages/mcp-server/src/setup/configure-claude.ts Ensures claude is discoverable on Windows by enabling shell execution.
packages/mcp-server/src/doctor/checks/codex-mcp.ts Uses shell execution on Windows to reliably check codex availability/registration.
packages/mcp-server/src/doctor/checks/claude-mcp.ts Uses shell execution on Windows to reliably check claude availability/registration.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread packages/mcp-server/src/store/path.ts
@youllook
Copy link
Copy Markdown

+1 — 確認這個 PR 在 Windows 11 真實安裝環境修好了 bug。

我今天踩到這 bug,用的是 Chrome Web Store 擴充 + installer v2.2.3。花了大約 3 小時才追到根因,因為 Chrome 的錯誤訊息很誤導 —— 看起來像 JSON 格式問題,但其實是這 PR 修的問題:Windows 上三個 browser 共用同一個 manifest 檔,Firefox 寫最後(只有 allowed_extensions)覆蓋掉 Chrome 的 manifest(裡面有 allowed_origins)。

我把這 PR cherry-pick 到 fork(附連結)、重 build、替換安裝。結果:每個 browser 各自獨立子目錄,allowlist 正確,Local bridge 立刻從 unavailable 變 ok。

額外好處:shell: true 那個修法讓 Windows 上的 Codex MCP 註冊也修好了 —— setup 輸出從「找不到 codex CLI」變成「Codex MCP 設定完成」。

希望能 merge。需要進一步測試我可以幫忙。

@iota31
Copy link
Copy Markdown
Collaborator

iota31 commented Apr 28, 2026

Hi, @PavelA85 small comment,

add a win32 test case to packages/mcp-server/src/store/path.test.ts asserting Chrome/Edge/Firefox resolve to distinct manifest paths. The collision fix is the headline change in this PR and it currently has zero test coverage. Without it, the bug can silently regress.

added the tests myself.

Asserts Chrome, Edge, and Firefox resolve to distinct manifest
paths on Windows, locking in the collision fix from this PR.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@iota31 iota31 merged commit d1a2d29 into onllm-dev:main Apr 28, 2026
3 checks passed
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.

4 participants