[PR #2312] [MERGED] test(mcp): fix three MCP boundary suites on native Windows #5157

New issue

Closed

opened 2026-10-31 01:04:25 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:04:25 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2312
Author: @pablodiazjorge
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/12/2026
Merged by: @morluto

Base: main ← Head: test/windows-mcp-boundary

📝 Commits (2)

285805d test(mcp): fix three MCP boundary suites on native Windows
1f6cb44 test(mcp): report unavailable root symlink coverage as a skip

📊 Changes

3 files changed (+67 additions, -55 deletions)

View changed files

📝 tests/boundary/mcp/dylibResolutionMcp.test.ts (+53 -48)
📝 tests/boundary/mcp/electronMcp.test.ts (+11 -6)
📝 tests/boundary/mcp/toolErrorSchemaDelivery.test.ts (+3 -1)

📄 Description

Summary

Five tests in three MCP boundary suites fail on native Windows because of POSIX assumptions in the tests themselves; the product behaves correctly. No issue is linked: I found them while running the MCP boundary project on Windows for #2148, and the fix touches only these test files, like #2113.

Problem and expected behavior

Smallest trigger, on Windows: npx vitest run tests/boundary/mcp/dylibResolutionMcp.test.ts tests/boundary/mcp/electronMcp.test.ts tests/boundary/mcp/toolErrorSchemaDelivery.test.ts gives 5 failed and 18 passed on 0043d9b13. Expected: each test reaches its assertions and passes, or is skipped where its mechanism does not exist on Windows.

Test	Failure on Windows	Cause
`dylibResolutionMcp` › keeps a permission denied while canonicalizing the opened bundle	`expected undefined to be true`	`chmod(parent, 0o000)` does not deny access on Windows
`dylibResolutionMcp` › reports caller-selected roots that cannot be traced as invalid input	`EPERM: operation not permitted, symlink 'C:\bin\ls' -> '<tmp>\…\Contents\MacOS\outside'`	File symlinks need Developer Mode or an elevated token
`electronMcp` › exposes endpoint-scoped Electron discovery and inspection as Evidence	`file_path: "C:\\…\\index.html"` does not match `/\/index\.html$/u`	REA returns native paths; the patterns expect `/`
`electronMcp` › runs active Electron scenarios with selected paths and inferred working directory	`ENOENT: no such file or directory, symlink '<tmp>\rea-electron-active-mcp-…' -> '<tmp>\C:\…\rea-electron-active-mcp-…-alias'`	`root.split("/")` returns the whole path, so the alias path is doubled
`toolErrorSchemaDelivery` › delivers typed tool failures without violating the advertised success schema over stdio	`expected '{"error":{"code":"target_unavailable"…' to contain 'C:\…\not-present.bin'`	The raw path is searched inside `JSON.stringify(...)`, where backslashes are escaped

Change and scope

dylibResolutionMcp.test.ts:
- The chmod-based permission test is skipped on Windows with the condition other chmod tests already use: it.skipIf(process.platform === "win32" || process.getuid?.() === 0).
- In the caller-selected roots test, only the outside-root case needs the /bin/ls symlink. The three other roots now also run on Windows; the symlink and its case are added where the platform is not Windows.
electronMcp.test.ts:
- The three file_path patterns accept either separator (/[\\/]index\.html$/u, and the same for app.js and worker.js).
- The alias name uses basename(root), and the alias is a junction on Windows, which needs no symlink privilege. It stays a dir symlink elsewhere.
toolErrorSchemaDelivery.test.ts: the diagnostic is searched for the JSON-encoded path, JSON.stringify(missing).slice(1, -1). On POSIX this is the same string as before.

No product code changes. On Linux and macOS every changed line behaves as before: [\\/] matches /, basename returns the last / segment, the symlink type and the outside-root case are unchanged, and JSON encoding leaves a temporary path with no backslashes or quotes as it is.

Contribution scope: not applicable (test-only fix within existing boundaries).

Contract and boundary impact

Semantic owner and earliest changed stage: test files only.
Contribution scope fit and linked maintainer decision: not applicable.
CLI and MCP/tool-catalog contract: none.
Provider, bridge, target-format, or platform compatibility: none; the tests now run on native Windows.
Evidence, artifact, provenance, or reconstruction contract: none.
Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable.
Generated metadata (docs/public/product-catalog.json), package, or installation impact: none.

Evidence and regression coverage

Tests added or updated: the three files above. On Windows, one test is skipped (the chmod denial) and the outside-root case of another does not run; everything else now runs there.
Base reproduction or other evidence: on 0043d9b13 the command above gives 5 failed, 18 passed on Windows; the errors are in the table.
User-visible CLI/MCP output (if applicable): not applicable.
Remaining proof gaps: Linux and macOS were not run locally; CI covers them.

Validation performed

Windows 11 26H2 x64, non-elevated, Node 22.22.3, with a locally built Windows native addon in native/windows/build (the JavaScript analysis tests in electronMcp need the Windows native controls).

npx vitest run tests/boundary/mcp/dylibResolutionMcp.test.ts tests/boundary/mcp/electronMcp.test.ts tests/boundary/mcp/toolErrorSchemaDelivery.test.ts on 0043d9b13: 5 failed, 18 passed.
The same command with this change: 22 passed, 1 skipped.
npx vitest run --project mcp-boundary with this change: 98 files passed, 6 skipped; 420 tests passed, 13 skipped, 0 failed.
npm run check:fast: passed.
npx oxfmt --check on the three files: passed.

Compatibility, safety, and release

Breaking changes or migration steps: none.
Real Hopper/Ghidra, browser, or OS coverage: native Windows for these suites; no provider involved.
Package or release metadata impact: none.
Security, privacy, process, or containment review: none; tests only.

Before merge

This PR addresses a concrete problem or an agreed enhancement.
Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff.
The PR has one focused outcome and the title follows type(scope): outcome.
Related issue is linked, or the reason for not linking one is stated above.
Tests cover changed observable behavior and meaningful failure paths.
Owning docs, contracts, and generated metadata are updated where needed.
User-visible CLI/MCP changes include representative output.
I checked the final diff for secrets, unrelated cleanup, and unsupported claims.

_{🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.}

## 📋 Pull Request Information **Original PR:** https://github.com/morluto/rea/pull/2312 **Author:** [@pablodiazjorge](https://github.com/pablodiazjorge) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@morluto](https://github.com/morluto) **Base:** `main` ← **Head:** `test/windows-mcp-boundary` --- ### 📝 Commits (2) - [`285805d`](https://github.com/morluto/rea/commit/285805db9ec4e97a479c1e9c360889b3734c32da) test(mcp): fix three MCP boundary suites on native Windows - [`1f6cb44`](https://github.com/morluto/rea/commit/1f6cb44def4c31919b9e1185b9994dd85e95c97d) test(mcp): report unavailable root symlink coverage as a skip ### 📊 Changes **3 files changed** (+67 additions, -55 deletions) <details> <summary>View changed files</summary> 📝 `tests/boundary/mcp/dylibResolutionMcp.test.ts` (+53 -48) 📝 `tests/boundary/mcp/electronMcp.test.ts` (+11 -6) 📝 `tests/boundary/mcp/toolErrorSchemaDelivery.test.ts` (+3 -1) </details> ### 📄 Description ## Summary Five tests in three MCP boundary suites fail on native Windows because of POSIX assumptions in the tests themselves; the product behaves correctly. No issue is linked: I found them while running the MCP boundary project on Windows for #2148, and the fix touches only these test files, like #2113. ## Problem and expected behavior Smallest trigger, on Windows: `npx vitest run tests/boundary/mcp/dylibResolutionMcp.test.ts tests/boundary/mcp/electronMcp.test.ts tests/boundary/mcp/toolErrorSchemaDelivery.test.ts` gives 5 failed and 18 passed on `0043d9b13`. Expected: each test reaches its assertions and passes, or is skipped where its mechanism does not exist on Windows. | Test | Failure on Windows | Cause | |---|---|---| | `dylibResolutionMcp` › keeps a permission denied while canonicalizing the opened bundle | `expected undefined to be true` | `chmod(parent, 0o000)` does not deny access on Windows | | `dylibResolutionMcp` › reports caller-selected roots that cannot be traced as invalid input | `EPERM: operation not permitted, symlink 'C:\bin\ls' -> '<tmp>\…\Contents\MacOS\outside'` | File symlinks need Developer Mode or an elevated token | | `electronMcp` › exposes endpoint-scoped Electron discovery and inspection as Evidence | `file_path: "C:\\…\\index.html"` does not match `/\/index\.html$/u` | REA returns native paths; the patterns expect `/` | | `electronMcp` › runs active Electron scenarios with selected paths and inferred working directory | `ENOENT: no such file or directory, symlink '<tmp>\rea-electron-active-mcp-…' -> '<tmp>\C:\…\rea-electron-active-mcp-…-alias'` | `root.split("/")` returns the whole path, so the alias path is doubled | | `toolErrorSchemaDelivery` › delivers typed tool failures without violating the advertised success schema over stdio | `expected '{"error":{"code":"target_unavailable"…' to contain 'C:\…\not-present.bin'` | The raw path is searched inside `JSON.stringify(...)`, where backslashes are escaped | ## Change and scope - `dylibResolutionMcp.test.ts`: - The `chmod`-based permission test is skipped on Windows with the condition other `chmod` tests already use: `it.skipIf(process.platform === "win32" || process.getuid?.() === 0)`. - In the caller-selected roots test, only the outside-root case needs the `/bin/ls` symlink. The three other roots now also run on Windows; the symlink and its case are added where the platform is not Windows. - `electronMcp.test.ts`: - The three `file_path` patterns accept either separator (`/[\\/]index\.html$/u`, and the same for `app.js` and `worker.js`). - The alias name uses `basename(root)`, and the alias is a junction on Windows, which needs no symlink privilege. It stays a `dir` symlink elsewhere. - `toolErrorSchemaDelivery.test.ts`: the diagnostic is searched for the JSON-encoded path, `JSON.stringify(missing).slice(1, -1)`. On POSIX this is the same string as before. No product code changes. On Linux and macOS every changed line behaves as before: `[\\/]` matches `/`, `basename` returns the last `/` segment, the symlink type and the outside-root case are unchanged, and JSON encoding leaves a temporary path with no backslashes or quotes as it is. Contribution scope: not applicable (test-only fix within existing boundaries). ## Contract and boundary impact - Semantic owner and earliest changed stage: test files only. - Contribution scope fit and linked maintainer decision: not applicable. - CLI and MCP/tool-catalog contract: none. - Provider, bridge, target-format, or platform compatibility: none; the tests now run on native Windows. - Evidence, artifact, provenance, or reconstruction contract: none. - Target selection, authority, upstream behavior, and cleanup details (if applicable): not applicable. - Generated metadata (`docs/public/product-catalog.json`), package, or installation impact: none. ## Evidence and regression coverage - Tests added or updated: the three files above. On Windows, one test is skipped (the `chmod` denial) and the outside-root case of another does not run; everything else now runs there. - Base reproduction or other evidence: on `0043d9b13` the command above gives 5 failed, 18 passed on Windows; the errors are in the table. - User-visible CLI/MCP output (if applicable): not applicable. - Remaining proof gaps: Linux and macOS were not run locally; CI covers them. ## Validation performed Windows 11 26H2 x64, non-elevated, Node 22.22.3, with a locally built Windows native addon in `native/windows/build` (the JavaScript analysis tests in `electronMcp` need the Windows native controls). - `npx vitest run tests/boundary/mcp/dylibResolutionMcp.test.ts tests/boundary/mcp/electronMcp.test.ts tests/boundary/mcp/toolErrorSchemaDelivery.test.ts` on `0043d9b13`: 5 failed, 18 passed. - The same command with this change: 22 passed, 1 skipped. - `npx vitest run --project mcp-boundary` with this change: 98 files passed, 6 skipped; 420 tests passed, 13 skipped, 0 failed. - `npm run check:fast`: passed. - `npx oxfmt --check` on the three files: passed. ## Compatibility, safety, and release - Breaking changes or migration steps: none. - Real Hopper/Ghidra, browser, or OS coverage: native Windows for these suites; no provider involved. - Package or release metadata impact: none. - Security, privacy, process, or containment review: none; tests only. ## Before merge - [x] This PR addresses a concrete problem or an agreed enhancement. - [x] Material scope and authority questions are resolved, and any required maintainer scope decision covers the final diff. - [x] The PR has one focused outcome and the title follows `type(scope): outcome`. - [x] Related issue is linked, or the reason for not linking one is stated above. - [x] Tests cover changed observable behavior and meaningful failure paths. - [x] Owning docs, contracts, and generated metadata are updated where needed. - [x] User-visible CLI/MCP changes include representative output. - [x] I checked the final diff for secrets, unrelated cleanup, and unsupported claims. --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:04:25 +00:00