fix(controller): defer MCP base middleware to first request#5995
fix(controller): defer MCP base middleware to first request#5995elrrrrrrr wants to merge 1 commit into
Conversation
The MCP route-setup methods acquire the base middleware eagerly during
controller registration:
let mw = app.middleware.teggCtxLifecycleMiddleware();
mw = self.composeGlobalMiddleware(mw);
But \`app.middleware.teggCtxLifecycleMiddleware\` is loaded by egg's
loadMiddleware, which runs *after* controller registration (the tegg
load-unit init / postCreate). Calling it at registration time throws
\`teggCtxLifecycleMiddleware is not a function\` when booting an app that
registers MCP controllers.
This is the same class of bug as #5994 (eager middleware access at
registration), one layer deeper: that PR deferred the configured global
middlewares; this one defers the base middleware too. \`composeGlobalMiddleware\`
now takes a thunk that resolves the base middleware on the first request,
alongside the global middlewares.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
📝 WalkthroughWalkthrough
ChangesLazy MCP middleware resolution
Sequence Diagram(s)sequenceDiagram
participant Request
participant MCPControllerRegister
participant composeGlobalMiddleware
participant teggCtxLifecycleMiddleware
participant register.globalMiddlewares
Request->>MCPControllerRegister: first request enters MCP route
MCPControllerRegister->>composeGlobalMiddleware: call with mwFactory thunk
composeGlobalMiddleware->>teggCtxLifecycleMiddleware: invoke mwFactory()
composeGlobalMiddleware->>register.globalMiddlewares: resolve configured global middleware
composeGlobalMiddleware->>composeGlobalMiddleware: cache composed middleware
Request->>MCPControllerRegister: later request enters MCP route
MCPControllerRegister->>composeGlobalMiddleware: reuse cached middleware
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint install failed: private package registry requires authentication. Disable ESLint in CodeRabbit settings or use public packages. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying egg with
|
| Latest commit: |
834660a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://4eb0de05.egg-cci.pages.dev |
| Branch Preview URL: | https://fix-mcp-base-middleware-lazy.egg-cci.pages.dev |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## next #5995 +/- ##
==========================================
+ Coverage 84.89% 85.32% +0.43%
==========================================
Files 669 658 -11
Lines 19946 19678 -268
Branches 3964 3904 -60
==========================================
- Hits 16933 16791 -142
+ Misses 2588 2482 -106
+ Partials 425 405 -20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Deploying egg-v3 with
|
| Latest commit: |
834660a
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fc714dc0.egg-v3.pages.dev |
| Branch Preview URL: | https://fix-mcp-base-middleware-lazy.egg-v3.pages.dev |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request defers the resolution of the base middleware (teggCtxLifecycleMiddleware) to the first request by passing it as a factory function (thunk) to composeGlobalMiddleware. This prevents errors during controller registration when the middleware is not yet loaded. The tests have been updated and expanded to verify this deferred behavior. The review feedback suggests introducing a cached helper method, getMcpMiddleware(), to eliminate the duplicated middleware creation logic across several registration methods.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const self = this; | ||
| let mw = (this.app.middleware as any).teggCtxLifecycleMiddleware(); | ||
| mw = self.composeGlobalMiddleware(mw); | ||
| const mw = self.composeGlobalMiddleware(() => (this.app.middleware as any).teggCtxLifecycleMiddleware()); |
There was a problem hiding this comment.
Use the cached getMcpMiddleware() helper to avoid duplicate middleware creation logic and prevent recreating the middleware closure per SSE connection.
| const mw = self.composeGlobalMiddleware(() => (this.app.middleware as any).teggCtxLifecycleMiddleware()); | |
| const mw = self.getMcpMiddleware(); |
There was a problem hiding this comment.
Pull request overview
This PR is a follow-up fix for MCP controller registration timing: it prevents MCP route setup from eagerly calling app.middleware.teggCtxLifecycleMiddleware() during controller registration (which happens before Egg’s loadMiddleware), by deferring acquisition of the base middleware to the first request.
Changes:
- Change
composeGlobalMiddlewareto accept a base-middleware factory thunk and resolve it lazily on first request. - Update all MCP route setup call sites to pass
() => app.middleware.teggCtxLifecycleMiddleware()instead of invoking it during registration. - Expand tests to assert the base middleware factory is not invoked at registration and is invoked exactly once on first request.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| tegg/plugin/controller/src/lib/impl/mcp/MCPControllerRegister.ts | Defer base middleware acquisition via thunk in MCP route setup; update composeGlobalMiddleware to resolve base + globals lazily. |
| tegg/plugin/controller/test/lib/MCPGlobalMiddleware.test.ts | Add/adjust tests to validate base thunk deferral + caching and ensure registration doesn’t touch app.middlewares or the base factory. |
| // request. | ||
| let resolved = false; | ||
| let composed: compose.Middleware<EggContext> = mw; | ||
| let composed: compose.Middleware<EggContext>; |
Follow-up to #5994.
Problem
The MCP route-setup methods acquire the base middleware eagerly during controller registration:
app.middleware.teggCtxLifecycleMiddlewareis loaded by egg’sloadMiddleware, which runs after controller registration (the tegg load-unit init /postCreate). Calling it at registration time throws:when booting an app that registers MCP controllers.
This is the same class of bug as #5994 (eager middleware access at registration), one layer deeper: #5994 deferred the configured global middlewares; this defers the base middleware too.
Fix
composeGlobalMiddlewarenow takes a thunk that resolves the base middleware on the first request, alongside the global middlewares. All four call sites (mcpStatelessStreamServerInit,mcpStreamServerInit,sseCtxStorageRun,mcpServerRegister) pass() => app.middleware.teggCtxLifecycleMiddleware()instead of the already-resolved middleware.Tests
tegg/plugin/controller/test/lib/MCPGlobalMiddleware.test.ts(8 tests, all green):teggCtxLifecycleMiddlewareis not invoked at registration.Verified end-to-end by patching the published
@eggjs/controller-plugin@4.0.2-beta.14dist into a downstream tegg app: theteggCtxLifecycleMiddleware is not a functionboot error is gone.🤖 Generated with Claude Code
Summary by CodeRabbit