morluto/rea

Fork 0

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

[PR #1691] [MERGED] perf(mcp): keep Playwright and type-only imports off MCP startup #4757

New issue

Closed

opened 2026-10-31 00:54:44 +00:00 by bsakaguchi · 0 comments

bsakaguchi commented

2026-10-31 00:54:44 +00:00

Owner

📋 Pull Request Information

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

Base: main ← Head: perf/mcp-startup-module-resolution

📝 Commits (2)

38659ac fix(build): emit no runtime imports for type-only bindings
c22dc92 perf(mcp): load Playwright when a scenario launches, not at startup

📊 Changes

89 files changed (+197 additions, -160 deletions)

View changed files

📝 .oxlintrc.json (+2 -1)
📝 src/application/AnalysisContextQueries.ts (+1 -1)
📝 src/application/BrowserObservationService.ts (+1 -1)
📝 src/application/BrowserScenarioCaptureService.ts (+1 -1)
📝 src/application/CallPathTracing.ts (+1 -1)
📝 src/application/DirectAnalysis.ts (+1 -1)
📝 src/application/EnhancedLiteralTracing.ts (+1 -1)
📝 src/application/EnhancedToolTypes.ts (+1 -1)
📝 src/application/EnhancedTools.ts (+1 -1)
📝 src/application/EvidenceBundleCommands.ts (+1 -1)
📝 src/application/InventoryProjectionEvidence.ts (+1 -1)
📝 src/application/ReconstructionCoverageService.ts (+1 -1)
📝 src/application/ReconstructionObligationLedgerService.ts (+1 -1)
📝 src/application/ReferenceSourceImport.ts (+3 -3)
📝 src/application/Setup.fixture.ts (+6 -6)
📝 src/application/binary/AnalysisSnapshotCache.ts (+1 -1)
📝 src/application/binary/BinarySession.ts (+1 -1)
📝 src/application/binary/BinarySessionExecution.ts (+1 -1)
📝 src/application/binary/BinarySessionOpen.ts (+2 -2)
📝 src/application/binary/BinarySessionRecords.ts (+2 -2)

...and 69 more files

📄 Description

Closes #1685.

What changed

MCP startup resolved 1,598 modules (6,076 resolutions) before it read the first request. 279 of those modules came from playwright-core, which no-target startup never uses. There were two causes:

Imports used only for types still ran at runtime. With verbatimModuleSyntax, an import whose specifiers all use inline type modifiers compiles to import {} from "…", which still resolves and evaluates the module. 78 compiled modules had one, including import {} from "playwright-core" in PlaywrightScenarioSession. These now use top-level import type, and typescript/no-import-type-side-effects is enabled in .oxlintrc.json so the pattern can't come back.
Playwright launchers were bound when the module loaded. The browser-scenario and Electron-active providers are built before the stdio server starts serving. PlaywrightScenarioBrowser and PlaywrightElectronActiveProvider now load chromium / _electron from playwright-core when a capture opens, the same way NativeModuleResolver already did.

Measurements (macOS arm64, Node 26.5, built package, pinned MCP SDK 2.3.1 client)

Before first request	main	this PR
Unique modules	1,598	1,312
…from `node_modules`	667	388
Resolve calls	6,076	4,902 (−19%)
`playwright-core` loaded	yes	no
`client.connect` (median of 5)	~490 ms	~400 ms

Both builds advertise all 170 tools. The first and repeated tools/list timings did not change (~102 ms / ~20 ms). Modules were counted with a module.registerHooks resolve hook passed through --import.

Not verified: Windows timing. I didn't reproduce the 45 s Windows capture. Playwright is CommonJS internally, so each require there costs extra stat probes, and removing it should cut a bigger share on Windows than the resolve count suggests. The remaining ~4,900 resolutions are mostly REA's own dist/ graph (959 modules), and this PR doesn't change packaging.

Behavior preserved

Provider construction, the tool catalog, and the capability inventory are unchanged. Optional providers still load at startup; only Playwright's engine module is deferred.
If playwright-core itself can't be imported, the error now appears inside the existing launch failure path when a capture runs, instead of as an optional-provider load failure at startup. Every other optional-provider load failure is reported as before.
Module and resource locations, worker and native-helper entrypoints, and shutdown are unchanged. No packaging change.

Validation

New boundary test (tests/boundary/cli/cliStartupImports.test.ts, "MCP startup imports"): spawns scripts/rea.mjs mcp over stdio, initializes, runs a complete tools/list, and asserts that neither playwright-core nor isomorphic-git was resolved. With the two adapter changes reverted, it fails.
The CLI positive probe now runs the real openPlaywrightScenarioBrowser launch path with a missing executable to show that detection still works (previously it only imported the composition module).
npm run check:fast, npm run lint (incl. module boundaries), format:check, knip, docs:check
npm run test:changed: 127 files / 1,387 tests passed
vitest run src/browser tests/composition: 48 files / 454 tests passed
test:focused: browserScenarioMcp, electronMcp, optionalProviderIsolation, compactInputSchemaMcp, scenarioCliInputDiagnostics, electronBlankPathCli, localToolContracts, cliStartupImports all passed

Real-provider scope: a real capture_browser_scenario call with cached Chrome for Testing 1208 reached playwright-core through the deferred import. The launch then failed with TargetClosedError because the local sandbox blocks Chrome; plain Playwright with no REA code fails the same way there, so no successful real Chromium or Electron launch was verified locally.

🤖 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/1691 **Author:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Created:** 10/11/2026 **Status:** ✅ Merged **Merged:** 10/11/2026 **Merged by:** [@kaoru0822-kitauji](https://github.com/kaoru0822-kitauji) **Base:** `main` ← **Head:** `perf/mcp-startup-module-resolution` --- ### 📝 Commits (2) - [`38659ac`](https://github.com/morluto/rea/commit/38659acc6dd1a9901a38e9daffe5747e9594f0bf) fix(build): emit no runtime imports for type-only bindings - [`c22dc92`](https://github.com/morluto/rea/commit/c22dc92fa215b4d02bfe514cfdb640aae48f6677) perf(mcp): load Playwright when a scenario launches, not at startup ### 📊 Changes **89 files changed** (+197 additions, -160 deletions) <details> <summary>View changed files</summary> 📝 `.oxlintrc.json` (+2 -1) 📝 `src/application/AnalysisContextQueries.ts` (+1 -1) 📝 `src/application/BrowserObservationService.ts` (+1 -1) 📝 `src/application/BrowserScenarioCaptureService.ts` (+1 -1) 📝 `src/application/CallPathTracing.ts` (+1 -1) 📝 `src/application/DirectAnalysis.ts` (+1 -1) 📝 `src/application/EnhancedLiteralTracing.ts` (+1 -1) 📝 `src/application/EnhancedToolTypes.ts` (+1 -1) 📝 `src/application/EnhancedTools.ts` (+1 -1) 📝 `src/application/EvidenceBundleCommands.ts` (+1 -1) 📝 `src/application/InventoryProjectionEvidence.ts` (+1 -1) 📝 `src/application/ReconstructionCoverageService.ts` (+1 -1) 📝 `src/application/ReconstructionObligationLedgerService.ts` (+1 -1) 📝 `src/application/ReferenceSourceImport.ts` (+3 -3) 📝 `src/application/Setup.fixture.ts` (+6 -6) 📝 `src/application/binary/AnalysisSnapshotCache.ts` (+1 -1) 📝 `src/application/binary/BinarySession.ts` (+1 -1) 📝 `src/application/binary/BinarySessionExecution.ts` (+1 -1) 📝 `src/application/binary/BinarySessionOpen.ts` (+2 -2) 📝 `src/application/binary/BinarySessionRecords.ts` (+2 -2) _...and 69 more files_ </details> ### 📄 Description Closes #1685. ## What changed MCP startup resolved **1,598 modules (6,076 resolutions)** before it read the first request. 279 of those modules came from `playwright-core`, which no-target startup never uses. There were two causes: 1. **Imports used only for types still ran at runtime.** With `verbatimModuleSyntax`, an import whose specifiers all use inline `type` modifiers compiles to `import {} from "…"`, which still resolves and evaluates the module. 78 compiled modules had one, including `import {} from "playwright-core"` in `PlaywrightScenarioSession`. These now use top-level `import type`, and `typescript/no-import-type-side-effects` is enabled in `.oxlintrc.json` so the pattern can't come back. 2. **Playwright launchers were bound when the module loaded.** The browser-scenario and Electron-active providers are built before the stdio server starts serving. `PlaywrightScenarioBrowser` and `PlaywrightElectronActiveProvider` now load `chromium` / `_electron` from `playwright-core` when a capture opens, the same way `NativeModuleResolver` already did. ## Measurements (macOS arm64, Node 26.5, built package, pinned MCP SDK 2.3.1 client) | Before first request | main | this PR | | --- | ---: | ---: | | Unique modules | 1,598 | 1,312 | | …from `node_modules` | 667 | 388 | | Resolve calls | 6,076 | 4,902 (−19%) | | `playwright-core` loaded | yes | no | | `client.connect` (median of 5) | ~490 ms | ~400 ms | Both builds advertise all 170 tools. The first and repeated `tools/list` timings did not change (~102 ms / ~20 ms). Modules were counted with a `module.registerHooks` resolve hook passed through `--import`. **Not verified:** Windows timing. I didn't reproduce the 45 s Windows capture. Playwright is CommonJS internally, so each `require` there costs extra stat probes, and removing it should cut a bigger share on Windows than the resolve count suggests. The remaining ~4,900 resolutions are mostly REA's own `dist/` graph (959 modules), and this PR doesn't change packaging. ## Behavior preserved - Provider construction, the tool catalog, and the capability inventory are unchanged. Optional providers still load at startup; only Playwright's engine module is deferred. - If `playwright-core` itself can't be imported, the error now appears inside the existing launch failure path when a capture runs, instead of as an optional-provider load failure at startup. Every other optional-provider load failure is reported as before. - Module and resource locations, worker and native-helper entrypoints, and shutdown are unchanged. No packaging change. ## Validation - New boundary test (`tests/boundary/cli/cliStartupImports.test.ts`, "MCP startup imports"): spawns `scripts/rea.mjs mcp` over stdio, initializes, runs a complete `tools/list`, and asserts that neither `playwright-core` nor `isomorphic-git` was resolved. With the two adapter changes reverted, it **fails**. - The CLI positive probe now runs the real `openPlaywrightScenarioBrowser` launch path with a missing executable to show that detection still works (previously it only imported the composition module). - `npm run check:fast`, `npm run lint` (incl. module boundaries), `format:check`, `knip`, `docs:check` - `npm run test:changed`: 127 files / 1,387 tests passed - `vitest run src/browser tests/composition`: 48 files / 454 tests passed - `test:focused`: `browserScenarioMcp`, `electronMcp`, `optionalProviderIsolation`, `compactInputSchemaMcp`, `scenarioCliInputDiagnostics`, `electronBlankPathCli`, `localToolContracts`, `cliStartupImports` all passed **Real-provider scope:** a real `capture_browser_scenario` call with cached Chrome for Testing 1208 reached `playwright-core` through the deferred import. The launch then failed with `TargetClosedError` because the local sandbox blocks Chrome; plain Playwright with no REA code fails the same way there, so no successful real Chromium or Electron launch was verified locally. 🤖 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 00:54:44 +00:00

closed this issue
added the
pull-request
label

bsakaguchi referenced this issue

2026-10-31 01:14:07 +00:00

[PR #4757] [MERGED] fix(javascript): report oversized recovery inputs as file-size limits #6391

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#4757

No description provided.

Rows
Columns