morluto/rea

Fork 0

mirror of https://github.com/morluto/rea.git synced 2026-11-05 06:45:22 +00:00

[PR #2205] [MERGED] fix(mcp): keep shared optional providers open across discarded probes #5083

New issue

Closed

opened 2026-10-31 01:03:34 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 01:03:34 +00:00

Owner

📋 Pull Request Information

Original PR: https://github.com/morluto/rea/pull/2205
Author: @kaoru0822-kitauji
Created: 10/12/2026
Status: ✅ Merged
Merged: 10/12/2026
Merged by: @kaoru0822-kitauji

Base: main ← Head: fix/issue-2195

📝 Commits (1)

71597db fix(mcp): keep shared optional providers open across discarded probes

📊 Changes

5 files changed (+73 additions, -2 deletions)

View changed files

📝 src/main.ts (+1 -0)
📝 src/main/shutdown.ts (+4 -0)
📝 src/main/transport.ts (+7 -1)
📝 src/server/createServer.ts (+0 -1)
➕ tests/boundary/process/browserScenarioProbeLifecycle.test.ts (+61 -0)

📄 Description

Summary

startMcpTransport loads the optional observation providers once per process and spreads them into every server instance the SDK constructs. createServer's closeProviders also closed options.browserScenarioCapture, a provider the instance did not create. A client can open with server/discover and then fall back to a legacy initialize. The SDK then discards the probe instance, and closing it closed the process-wide browser scenario provider. Every later capture_browser_scenario call on that server process failed within milliseconds with cleanup_incomplete / "Browser scenario provider is closing."

Ownership now follows creation:

createServer no longer closes the caller-supplied browser scenario provider. The per-instance services it creates itself are still closed as before.
startMcpTransport returns closeOptionalProviders, and createShutdown registers it next to closeAndroid, so the shared provider is closed once at process shutdown.

Fixes #2195

Reproduction (raw JSON-RPC over `rea mcp` stdio, scenario with a nonexistent `executable_path` so no browser is needed)

before  legacy-only        call 3,4: execution_failure (launch attempted)
before  probe-then-legacy  call 3,4: cleanup_incomplete "Browser scenario provider is closing."
after   legacy-only        call 3,4: execution_failure (launch attempted)
after   probe-then-legacy  call 3,4: execution_failure (launch attempted), same as legacy-only

Validation

New tests/boundary/process/browserScenarioProbeLifecycle.test.ts mirrors the existing Android discovery-probe test. It constructs a probe server, closes it, constructs the replacement, and asserts that the shared provider's close is not called; transport.closeOptionalProviders() then closes it once. It fails on main (close called twice) and passes with this change.
Existing shutdown, androidAnalysisMcp, browserScenarioMcp, browserScenarioCleanup and optionalProviderIsolation tests pass.
npm run lint passes.
npm test: 9184 passed, 14 failed in 7 files. These are the same environmental files that fail on clean origin/main on this host: Hopper not installed, Command Line Tools only (ibtool), hdiutil, and inspector target-location cases.
Not run: a real browser scenario. This macOS host has no Chrome/Chromium, so the reproduction only checks that the call reaches browser launch again instead of being rejected as closing.

🤖 Generated with Claude Code

_{🔄 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/2205 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/12/2026 **Status:** ✅ Merged **Merged:** 10/12/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `fix/issue-2195` --- ### 📝 Commits (1) - [`71597db`](https://github.com/morluto/rea/commit/71597dbaed9710cf1430a54877caae766ce51a87) fix(mcp): keep shared optional providers open across discarded probes ### 📊 Changes **5 files changed** (+73 additions, -2 deletions) <details> <summary>View changed files</summary> 📝 `src/main.ts` (+1 -0) 📝 `src/main/shutdown.ts` (+4 -0) 📝 `src/main/transport.ts` (+7 -1) 📝 `src/server/createServer.ts` (+0 -1) ➕ `tests/boundary/process/browserScenarioProbeLifecycle.test.ts` (+61 -0) </details> ### 📄 Description ## Summary `startMcpTransport` loads the optional observation providers once per process and spreads them into every server instance the SDK constructs. `createServer`'s `closeProviders` also closed `options.browserScenarioCapture`, a provider the instance did not create. A client can open with `server/discover` and then fall back to a legacy `initialize`. The SDK then discards the probe instance, and closing it closed the process-wide browser scenario provider. Every later `capture_browser_scenario` call on that server process failed within milliseconds with `cleanup_incomplete` / "Browser scenario provider is closing." Ownership now follows creation: - `createServer` no longer closes the caller-supplied browser scenario provider. The per-instance services it creates itself are still closed as before. - `startMcpTransport` returns `closeOptionalProviders`, and `createShutdown` registers it next to `closeAndroid`, so the shared provider is closed once at process shutdown. Fixes #2195 ## Reproduction (raw JSON-RPC over `rea mcp` stdio, scenario with a nonexistent `executable_path` so no browser is needed) ```text before legacy-only call 3,4: execution_failure (launch attempted) before probe-then-legacy call 3,4: cleanup_incomplete "Browser scenario provider is closing." after legacy-only call 3,4: execution_failure (launch attempted) after probe-then-legacy call 3,4: execution_failure (launch attempted), same as legacy-only ``` ## Validation - New `tests/boundary/process/browserScenarioProbeLifecycle.test.ts` mirrors the existing Android discovery-probe test. It constructs a probe server, closes it, constructs the replacement, and asserts that the shared provider's `close` is not called; `transport.closeOptionalProviders()` then closes it once. It fails on main (close called twice) and passes with this change. - Existing `shutdown`, `androidAnalysisMcp`, `browserScenarioMcp`, `browserScenarioCleanup` and `optionalProviderIsolation` tests pass. - `npm run lint` passes. - `npm test`: 9184 passed, 14 failed in 7 files. These are the same environmental files that fail on clean `origin/main` on this host: Hopper not installed, Command Line Tools only (`ibtool`), `hdiutil`, and inspector target-location cases. - Not run: a real browser scenario. This macOS host has no Chrome/Chromium, so the reproduction only checks that the call reaches browser launch again instead of being rejected as closing. 🤖 Generated with [Claude Code](https://claude.com/claude-code) --- <sub>🔄 This issue represents a GitHub Pull Request. It cannot be merged through Gitea due to API limitations.</sub>

bsakaguchi

2026-10-31 01:03:34 +00:00

closed this issue
added the
pull-request
label

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

morluto/rea#5083

No description provided.

Rows
Columns