kaizen: remove unused deprecated type aliases#171
Open
gabrypavanello wants to merge 6 commits intomainfrom
Open
kaizen: remove unused deprecated type aliases#171gabrypavanello wants to merge 6 commits intomainfrom
gabrypavanello wants to merge 6 commits intomainfrom
Conversation
added 6 commits
February 10, 2026 09:03
…SK-037-02) Replace all raw console.log/warn/error calls across 17 inspector/src files with scoped logger instances using createLogger(). Each module gets a descriptive source name for log filtering. - 17 files migrated to structured logger - Stale eslint-disable no-console comments removed - Tests updated to match new console.info output format - bin/mcp-inspector.ts and widget-server-templates.ts untouched - proxy-resources.ts skipped (console call is in client-side template)
…d IconConfig These deprecated aliases (FileBasedGlobalConfig → GlobalConfig, IconConfig → Icon) had zero consumers across the codebase. Removing them eliminates 1 eslint-disable suppression and 2 deprecated type exports.
Contributor
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughThis PR removes deprecated type aliases from the codegen package and introduces a centralized, structured logger throughout the inspector, testing, and UI packages. The logger supports level-based filtering (DEBUG, INFO, WARN, ERROR), timestamps, and respects the MCP_APPS_LOG_LEVEL environment variable. All console.log/warn/error calls are replaced with corresponding logger method calls. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~35 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Review: PR #171 - Kaizen: Remove Unused Deprecated Type AliasesSummaryThis PR successfully removes two deprecated type aliases (FileBasedGlobalConfig and IconConfig) from @mcp-apps-kit/codegen while introducing a comprehensive structured logging system across the inspector package. Strengths1. Clean Deprecated Code Removal
2. Well-Designed Logger Module
3. Consistent Migration Pattern
Issues & Concerns1. Code Duplication (Medium Priority)
Recommendation: Extract the logger into a shared package to avoid drift. 2. Browser Compatibility (Low-Medium) 3. Missing Documentation (Low) Security & Performance
RecommendationsMust Fix: None - safe to merge as-is Should Fix (Follow-Up):
Alignment with Repository Standards
Final VerdictAPPROVE with recommendations for follow-up improvements. Risk Assessment: 🟢 Low Risk
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/inspector/src/widget-session-manager.ts (1)
401-445:⚠️ Potential issue | 🟠 MajorDon’t use the Node logger inside
page.evaluate(browser context).
loggeris undefined in the page, so these evaluate calls can throw and break globals sync/event delivery. Useconsolein the page or inject a logger explicitly.🛠️ Suggested fix
- logger.info("[MCP Host] Sent ui/notifications/host-context-changed", ctx); + console.info("[MCP Host] Sent ui/notifications/host-context-changed", ctx);- logger.info("[OpenAI Host] Sent globals sync:", message.data); + console.info("[OpenAI Host] Sent globals sync:", message.data);- logger.info("[MCP Host] Stored hostContext update for ui/initialize:", p); + console.info("[MCP Host] Stored hostContext update for ui/initialize:", p);- logger.info("[MCP Host] Sent synced event:", m, p); + console.info("[MCP Host] Sent synced event:", m, p);- logger.info("[OpenAI Host] Sent synced event:", message.syncType, message.data); + console.info("[OpenAI Host] Sent synced event:", message.syncType, message.data);Also applies to: 733-766, 792-799
packages/inspector/src/session/session-renderer.ts (1)
206-218:⚠️ Potential issue | 🔴 CriticalFix:
loggerusage insidepage.evaluatewill throw ReferenceError in browser context.
page.evaluateexecutes code in the browser, whereloggeris undefined. The calls at lines 216, 255, 285, 290, 295, and 303 will reject the evaluate promise, breaking globals sync and tool response delivery. Replacelogger.info()withconsole.info()inside evaluate blocks and update the ESLint disable comment to includeno-console.Proposed fixes
- /* eslint-disable no-undef */ + /* eslint-disable no-undef, no-console */ await page.evaluate((ctx: typeof hostContext) => { const iframe = document.getElementById("widget-frame") as HTMLIFrameElement | null; if (iframe?.contentWindow) { const message = { jsonrpc: "2.0", method: "ui/notifications/host-context-changed", params: { hostContext: ctx }, }; iframe.contentWindow.postMessage(message, "*"); - logger.info("[MCP Host] Sent ui/notifications/host-context-changed", ctx); + console.info("[MCP Host] Sent ui/notifications/host-context-changed", ctx); } }, hostContext); - /* eslint-enable no-undef */ + /* eslint-enable no-undef, no-console */- /* eslint-disable no-undef */ + /* eslint-disable no-undef, no-console */ await page.evaluate((message: typeof syncMessage) => { const iframe = document.getElementById("widget-frame") as HTMLIFrameElement | null; if (iframe?.contentWindow) { iframe.contentWindow.postMessage(message, "*"); - logger.info("[OpenAI Host] Sent globals sync:", message.data); + console.info("[OpenAI Host] Sent globals sync:", message.data); } }, syncMessage); - /* eslint-enable no-undef */ + /* eslint-enable no-undef, no-console */- /* eslint-disable no-undef */ + /* eslint-disable no-undef, no-console */ await page.evaluate((responseData: unknown) => { // Validate responseData is a non-null object before accessing properties if (typeof responseData !== "object" || responseData === null) { - logger.info("[MCP Host] Tool response is not a valid object:", responseData); + console.info("[MCP Host] Tool response is not a valid object:", responseData); return; } const d = responseData as Record<string, unknown>; const nameValue = d.name; const toolNameValue = d.toolName; // Validate that name or toolName exists and is a string const toolName = typeof nameValue === "string" && nameValue ? nameValue : typeof toolNameValue === "string" && toolNameValue ? toolNameValue : null; if (!toolName) { - logger.info("[MCP Host] Tool response missing valid name/toolName string:", responseData); + console.info("[MCP Host] Tool response missing valid name/toolName string:", responseData); return; } type PendingCall = { messageId: number | string; args: unknown; timestamp: number }; const w = window as Window & { __pendingToolCalls?: Record<string, PendingCall[]> }; const pending = w.__pendingToolCalls?.[toolName]; if (!pending || pending.length === 0) { - logger.info("[MCP Host] No pending calls for tool:", toolName); + console.info("[MCP Host] No pending calls for tool:", toolName); return; } // Get the oldest pending call (FIFO) const call = pending.shift(); if (!call) return; // Send response to widget iframe const iframe = document.getElementById("widget-frame") as HTMLIFrameElement | null; if (iframe?.contentWindow) { iframe.contentWindow.postMessage( { jsonrpc: "2.0", id: call.messageId, result: d.result ?? { content: [{ type: "text", text: JSON.stringify(d) }] }, }, "*" ); - logger.info("[MCP Host] Delivered synced tool response:", toolName, call.messageId); + console.info("[MCP Host] Delivered synced tool response:", toolName, call.messageId); } }, data); - /* eslint-enable no-undef */ + /* eslint-enable no-undef, no-console */
🤖 Fix all issues with AI agents
In `@packages/inspector/src/hosts/mcp-host.ts`:
- Around line 222-223: The Playwright page-init script is calling logger (e.g.,
logger.info('[MCP Host Emulator] Received ui/initialize, responding...')), which
is undefined in the browser context and will throw; change those in-page logging
calls to use console (console.info/console.warn/console.error) or pass an
injected logger object into the page context and call that instead. Locate all
occurrences of logger.* used inside the Playwright init script (the in-page
handler that sends the MCP emulator responses) and either replace them with
console.* or wire a serialized logger via page.exposeFunction /
page.evaluateOnNewDocument and call that exposed function from the page code to
avoid runtime errors.
In `@packages/inspector/src/hosts/openai-host.ts`:
- Around line 248-249: The init script injected into the browser context uses an
undefined logger variable (logger.info(...)) which will throw inside the
Playwright page; replace that call with console (e.g., console.info('[OpenAI
Host] requestDisplayMode:', mode, 'sizing:', sizing)) or inject a page-scoped
logger before use. Locate the snippet in openai-host.ts where the init script
emits "requestDisplayMode" (the logger.info call) and change logger to console
or wire an injected logger object into the page context so the code no longer
references an undefined symbol.
Comment on lines
+222
to
223
| logger.info('[MCP Host Emulator] Received ui/initialize, responding...'); | ||
| // Respond with initialization success |
Contributor
There was a problem hiding this comment.
Avoid logger calls inside the Playwright init script (browser context).
logger isn’t defined in the page, so these will throw and can break the script. Use console or inject a logger into the page context.
🛠️ Suggested fix
- logger.info('[MCP Host Emulator] Received ui/initialize, responding...');
+ console.info('[MCP Host Emulator] Received ui/initialize, responding...');- logger.info('[MCP Host Emulator] Sent ui/initialize response');
+ console.info('[MCP Host Emulator] Sent ui/initialize response');- logger.info('[MCP Host Emulator] Sending ui/notifications/tool-result...');
+ console.info('[MCP Host Emulator] Sending ui/notifications/tool-result...');- logger.info('[MCP Host Emulator] Display mode changed to:', requestedMode, 'sizing:', sizing);
+ console.info('[MCP Host Emulator] Display mode changed to:', requestedMode, 'sizing:', sizing);Also applies to: 247-252, 280-281
🤖 Prompt for AI Agents
In `@packages/inspector/src/hosts/mcp-host.ts` around lines 222 - 223, The
Playwright page-init script is calling logger (e.g., logger.info('[MCP Host
Emulator] Received ui/initialize, responding...')), which is undefined in the
browser context and will throw; change those in-page logging calls to use
console (console.info/console.warn/console.error) or pass an injected logger
object into the page context and call that instead. Locate all occurrences of
logger.* used inside the Playwright init script (the in-page handler that sends
the MCP emulator responses) and either replace them with console.* or wire a
serialized logger via page.exposeFunction / page.evaluateOnNewDocument and call
that exposed function from the page code to avoid runtime errors.
Comment on lines
+248
to
249
| logger.info('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing); | ||
| return { mode: mode }; |
Contributor
There was a problem hiding this comment.
Avoid logger calls inside the Playwright init script (browser context).
logger is not defined in the page context, so this will throw and can break the init script. Use console (or inject a logger into the page) instead.
🛠️ Suggested fix
- logger.info('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing);
+ console.info('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| logger.info('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing); | |
| return { mode: mode }; | |
| console.info('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing); | |
| return { mode: mode }; |
🤖 Prompt for AI Agents
In `@packages/inspector/src/hosts/openai-host.ts` around lines 248 - 249, The init
script injected into the browser context uses an undefined logger variable
(logger.info(...)) which will throw inside the Playwright page; replace that
call with console (e.g., console.info('[OpenAI Host] requestDisplayMode:', mode,
'sizing:', sizing)) or inject a page-scoped logger before use. Locate the
snippet in openai-host.ts where the init script emits "requestDisplayMode" (the
logger.info call) and change logger to console or wire an injected logger object
into the page context so the code no longer references an undefined symbol.
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.
Removes
FileBasedGlobalConfigandIconConfigdeprecated type aliases from@mcp-apps-kit/codegen.What: These were deprecated aliases (
FileBasedGlobalConfig → GlobalConfig,IconConfig → Icon) with zero consumers across the codebase.Why: Eliminates 1
eslint-disable @typescript-eslint/no-deprecatedsuppression and 2 dead type exports.Risk: None — no consumers found in any package.