refactor(testing): remove unnecessary this-alias in mock constructors#176
Open
gabrypavanello wants to merge 6 commits intomainfrom
Open
refactor(testing): remove unnecessary this-alias in mock constructors#176gabrypavanello 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)
Arrow function callbacks capture `this` lexically, so the `const self = this` alias was dead code. Removing it also drops two eslint-disable comments for @typescript-eslint/no-this-alias. No behavior change.
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughIntroduces a centralized structured logging system across multiple packages with environment-based level gating. Creates a new logger module providing ISO-timestamped, formatted output "[timestamp] [LEVEL] [source] message" controlled by MCP_APPS_LOG_LEVEL environment variable. Replaces console logging calls throughout inspector, codegen, testing, and UI packages with this structured logger while maintaining existing functionality. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
Contributor
Code Review
Critical Bug —
|
| Severity | Issue |
|---|---|
| 🔴 Critical | logger referenced inside page.evaluate() — ReferenceError in browser context |
| 🟠 Major | Logger IIFE duplicated across 3 packages; should be a shared utility |
| 🟡 Minor | const logger between import blocks (ESLint/style) |
| 🟡 Minor | Log level frozen at module init — worth a JSDoc note |
The this-alias portion of the PR is straightforwardly correct. The logging refactor needs the page.evaluate bug fixed before merge.
![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: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (6)
packages/inspector/src/session/session-renderer.ts (3)
206-220:⚠️ Potential issue | 🔴 Critical
loggeris not available insidepage.evaluate()- this will throw a runtime error.The
loggerobject is defined at module scope in Node.js, butpage.evaluate()serializes the callback and runs it in the browser context. Theloggervariable doesn't exist in the browser, so line 216 will throwReferenceError: logger is not defined.The
/* eslint-disable no-undef */comment suppresses the ESLint warning but doesn't make the Node.js module available in the browser.🐛 Proposed fix - remove browser-context logging or use console directly
/* eslint-disable no-undef */ 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); + // Debug logging removed - logger not available in browser context } }, hostContext); /* eslint-enable no-undef */Alternatively, if debug logging is needed in the browser, use
console.logdirectly:- logger.info("[MCP Host] Sent ui/notifications/host-context-changed", ctx); + console.log("[MCP Host] Sent ui/notifications/host-context-changed", ctx);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/session/session-renderer.ts` around lines 206 - 220, The browser callback passed to page.evaluate uses the Node-scoped logger (logger.info) which does not exist in the page context and will throw; update the code in the page.evaluate callback (the anonymous function passed to page.evaluate in session-renderer.ts) to remove or replace logger calls with browser-safe logging (e.g., use console.log/console.info) or move the logger.info call out of the page.evaluate and log from the Node side after evaluate resolves; ensure you only reference hostContext inside the evaluated function and keep any Node-only symbols (logger) outside.
250-258:⚠️ Potential issue | 🔴 CriticalSame issue:
loggeris not available insidepage.evaluate().Line 255 will throw
ReferenceError: logger is not definedat runtime.🐛 Proposed fix
/* eslint-disable no-undef */ 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); + // Debug logging removed - logger not available in browser context } }, syncMessage); /* eslint-enable no-undef */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/session/session-renderer.ts` around lines 250 - 258, Inside the page.evaluate call you reference logger (line with logger.info), which is not defined in the page context and will throw ReferenceError; remove the logger usage from the function passed to page.evaluate (keeping only DOM/postMessage logic that references iframe/widget-frame and message), and instead call logger.info after the await page.evaluate completes using the syncMessage (or syncMessage.data) so the log runs in node context rather than inside page.evaluate.
282-334:⚠️ Potential issue | 🔴 CriticalSame issue: Multiple
loggercalls insidepage.evaluate()will fail.Lines 286, 303, 312, and 331 all use
logger.infoinside the browser context where it's undefined.🐛 Proposed fix - use console.log for browser-context logging
/* eslint-disable no-undef */ 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.log("[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.log("[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.log("[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.log("[MCP Host] Delivered synced tool response:", toolName, call.messageId); } }, data); /* eslint-enable no-undef */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/session/session-renderer.ts` around lines 282 - 334, Inside the page.evaluate(...) callback in session-renderer.ts the code calls logger.info (e.g., at the checks of responseData, missing toolName, no pending calls, and after delivering the response) but logger is not available in the browser context; replace those logger.info calls with console.log (or window.console.log) so logging occurs in the page context; update the four references inside the evaluate callback that mention logger.info (around responseData validation, toolName missing, no pending calls, and Delivered synced tool response) and keep the same message strings and variables (responseData, toolName, call.messageId) when calling console.log.packages/inspector/src/widget-session-manager.ts (3)
401-414:⚠️ Potential issue | 🔴 CriticalCritical:
loggeris undefined in browser context insidepage.evaluate().The
logger.info()call on line 412 runs insidepage.evaluate(), which executes in the browser context (Playwright page), not Node.js. Theloggervariable from the Node.js module scope is not accessible in the browser, causing aReferenceError: logger is not definedat runtime.🐛 Proposed fix - revert to console.log for browser context
await page.evaluate((ctx: typeof hostContext) => { const iframe = document.getElementById("widget-frame") as HTMLIFrameElement | null; if (iframe?.contentWindow) { // Use the correct MCP Apps protocol method name // Wrap context in hostContext to match ext-apps SDK format 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); + // eslint-disable-next-line no-console + console.info("[MCP Host] Sent ui/notifications/host-context-changed", ctx); } }, hostContext);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/widget-session-manager.ts` around lines 401 - 414, Inside the page.evaluate callback (used in the widget session manager), the Node.js module-scoped logger is not available and causes a ReferenceError; replace the in-browser logger.info call with a browser-safe console.log (or remove it) inside the function that constructs and posts the message to iframe.contentWindow, or alternatively move logging about sending "ui/notifications/host-context-changed" outside of page.evaluate and call the module logger (logger.info) after the await page.evaluate(...) completes using the same hostContext for context; target the page.evaluate callback, the iframe variable, and the sent message with method "ui/notifications/host-context-changed".
437-445:⚠️ Potential issue | 🔴 CriticalCritical: Same browser context issue with
loggerin OpenAI protocol path.This is the same bug as above -
logger.info()on line 442 runs in browser context viapage.evaluate()where the Node.jsloggeris not defined.🐛 Proposed fix - revert to console.log for browser context
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); + // eslint-disable-next-line no-console + console.info("[OpenAI Host] Sent globals sync:", message.data); } }, syncMessage);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/widget-session-manager.ts` around lines 437 - 445, The logger.info call is being executed inside page.evaluate (browser context) where the Node.js logger is undefined; change the code so page.evaluate only posts the message and uses console.log inside the browser if needed, and perform any Node-side logging with the Node logger outside of page.evaluate (e.g., call logger.info after the await page.evaluate(...) using syncMessage.data or the returned acknowledgment). Update the page.evaluate invocation in widget-session-manager.ts (the function call using page.evaluate, syncMessage and the iframe) to remove logger usage from the evaluated function and move Node logger calls to run after the await completes.
733-765:⚠️ Potential issue | 🔴 CriticalCritical: Three more instances of
loggerused insidepage.evaluate()browser context.Lines 747, 760, and 796 all use
logger.info()insidepage.evaluate()callbacks, which will fail at runtime becauseloggeris a Node.js module-scope variable not available in the browser context.🐛 Proposed fix for all three locations
Line 747:
- logger.info("[MCP Host] Stored hostContext update for ui/initialize:", p); + // eslint-disable-next-line no-console + console.info("[MCP Host] Stored hostContext update for ui/initialize:", p);Line 760:
- logger.info("[MCP Host] Sent synced event:", m, p); + // eslint-disable-next-line no-console + console.info("[MCP Host] Sent synced event:", m, p);Line 796:
- logger.info("[OpenAI Host] Sent synced event:", message.syncType, message.data); + // eslint-disable-next-line no-console + console.info("[OpenAI Host] Sent synced event:", message.syncType, message.data);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/widget-session-manager.ts` around lines 733 - 765, The page.evaluate callback erroneously references the Node-scoped logger (logger.info) which isn't available in the browser context; update the session.page.evaluate call in widget-session-manager.ts (the block using method, params, storeOnHost/isHostContextUpdate, iframe, and __mcpHostContextUpdates) to remove any logger usage inside the browser function and instead return a serializable result object (e.g., { stored: boolean, sent: boolean, method: string, params: any }) from the evaluate call; then, after the await completes in Node, inspect that returned object and call logger.info there to record stored/sent events and details.
🧹 Nitpick comments (4)
packages/inspector/src/widget-server.ts (1)
432-436: Minor: Redundant prefix in log message.The
logger.infoalready prefixes output with[widget-server]from the source identifier, so the[WidgetServer]in the message creates duplication like[widget-server] [WidgetServer] Server started.... Consider removing the[WidgetServer]prefix from the message string.♻️ Suggested fix
private log(message: string): void { if (this.options.debug) { - logger.info(`[WidgetServer] ${message}`); + logger.info(message); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/widget-server.ts` around lines 432 - 436, The log method in WidgetServer duplicates the source prefix because logger already uses "[widget-server]"; update the private log(message: string) method to remove the manual "[WidgetServer]" prefix and just pass the message string to logger.info when this.options.debug is true (i.e., change the behavior in the log function so it calls logger.info(message) instead of logger.info(`[WidgetServer] ${message}`) to avoid duplicate prefixes).packages/inspector/src/oauth/wellknown-proxy.ts (1)
81-85: Minor: Redundant prefix in log message.Similar to widget-server.ts, the
[oauth:wellknown-proxy]prefix in the message is redundant with the logger's[wellknown-proxy]source identifier.♻️ Suggested fix
function log(message: string): void { if (debug) { - logger.info(`[oauth:wellknown-proxy] ${message}`); + logger.info(message); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/oauth/wellknown-proxy.ts` around lines 81 - 85, The log function in wellknown-proxy.ts emits messages with a redundant “[oauth:wellknown-proxy]” prefix; update the log(message: string) helper to call logger.info(message) (or logger.info with a concise message) when debug is true so messages rely on the logger's existing source identifier rather than duplicating the prefix; keep the same debug guard and function signature.packages/inspector/src/oauth/provider.ts (1)
632-636: Minor: Redundant prefix in log message.The
[oauth:provider]prefix in the message is redundant with the logger's[oauth-provider]source identifier.♻️ Suggested fix
private log(message: string): void { if (this._debug) { - logger.info(`[oauth:provider] ${message}`); + logger.info(message); } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/oauth/provider.ts` around lines 632 - 636, The log method is prepending a redundant "[oauth:provider]" prefix; update the private log(message: string) method to rely on the logger's own source identifier instead of adding the prefix—i.e., when this._debug is true call logger.info with the raw message (or a minimally adjusted message) rather than `[oauth:provider] ${message}` so logging isn't duplicated (refer to the log method, this._debug and logger.info to locate the change).packages/inspector/src/session/session-store.ts (1)
82-84: Minor: Redundant prefix pattern across all log calls.The
[SessionStore]prefix in messages is redundant with the logger's[session-store]source identifier. This applies to all logger.info calls in this file (lines 83, 264, 291, 320, 337).♻️ Suggested fix for one example
if (this.debug) { - logger.info(`[SessionStore] Created session ${options.sessionId}`); + logger.info(`Created session ${options.sessionId}`); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/inspector/src/session/session-store.ts` around lines 82 - 84, Remove the redundant "[SessionStore]" prefix from log messages in this file: find all logger.info calls that include the literal "[SessionStore]" (e.g., the creation log under the this.debug check and the other info calls later in the SessionStore class) and change them to concise messages without the prefix so the existing logger source identifier (session-store) provides that context; update each affected call site to keep the rest of the message text unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/inspector/src/hosts/mcp-host.ts`:
- Line 222: The Playwright init script returned by getPlaywrightInitScript()
embeds Node-only logger calls (e.g., logger.info(...) inside the returned
string) which will throw in the browser; update the script to use console.log
(or remove the debug calls) instead of the Node-scoped logger so the code runs
in page context—search for logger usage inside the string returned by
getPlaywrightInitScript() and replace those occurrences (e.g., the logger.info
calls for ui/initialize and other events) with console.log or delete them.
In `@packages/inspector/src/hosts/openai-host.ts`:
- Line 248: The Playwright-injected script returned by getPlaywrightInitScript()
calls logger.info inside the browser context (in requestDisplayMode), but logger
is a Node-only variable and will cause a ReferenceError at runtime; replace that
call with console.log (to match other init script logs) or remove the logging
line entirely so requestDisplayMode no longer references the undefined logger
variable. Ensure you update the string returned by getPlaywrightInitScript()
where requestDisplayMode is defined to use console.log or omit logging.
In `@packages/ui-react/src/hooks.ts`:
- Around line 27-31: The eslint disable comment currently only applies to the
next physical line and doesn't cover the multi-line declaration of the envLevel
constant; update the comment to a block-style directive (e.g., /* eslint-disable
no-undef, `@typescript-eslint/no-unnecessary-condition` */) placed immediately
above the multi-line expression that defines envLevel (the const envLevel = ...
statement) and re-enable rules after the block if desired, so the usage of
process in the envLevel computation is properly suppressed.
---
Outside diff comments:
In `@packages/inspector/src/session/session-renderer.ts`:
- Around line 206-220: The browser callback passed to page.evaluate uses the
Node-scoped logger (logger.info) which does not exist in the page context and
will throw; update the code in the page.evaluate callback (the anonymous
function passed to page.evaluate in session-renderer.ts) to remove or replace
logger calls with browser-safe logging (e.g., use console.log/console.info) or
move the logger.info call out of the page.evaluate and log from the Node side
after evaluate resolves; ensure you only reference hostContext inside the
evaluated function and keep any Node-only symbols (logger) outside.
- Around line 250-258: Inside the page.evaluate call you reference logger (line
with logger.info), which is not defined in the page context and will throw
ReferenceError; remove the logger usage from the function passed to
page.evaluate (keeping only DOM/postMessage logic that references
iframe/widget-frame and message), and instead call logger.info after the await
page.evaluate completes using the syncMessage (or syncMessage.data) so the log
runs in node context rather than inside page.evaluate.
- Around line 282-334: Inside the page.evaluate(...) callback in
session-renderer.ts the code calls logger.info (e.g., at the checks of
responseData, missing toolName, no pending calls, and after delivering the
response) but logger is not available in the browser context; replace those
logger.info calls with console.log (or window.console.log) so logging occurs in
the page context; update the four references inside the evaluate callback that
mention logger.info (around responseData validation, toolName missing, no
pending calls, and Delivered synced tool response) and keep the same message
strings and variables (responseData, toolName, call.messageId) when calling
console.log.
In `@packages/inspector/src/widget-session-manager.ts`:
- Around line 401-414: Inside the page.evaluate callback (used in the widget
session manager), the Node.js module-scoped logger is not available and causes a
ReferenceError; replace the in-browser logger.info call with a browser-safe
console.log (or remove it) inside the function that constructs and posts the
message to iframe.contentWindow, or alternatively move logging about sending
"ui/notifications/host-context-changed" outside of page.evaluate and call the
module logger (logger.info) after the await page.evaluate(...) completes using
the same hostContext for context; target the page.evaluate callback, the iframe
variable, and the sent message with method
"ui/notifications/host-context-changed".
- Around line 437-445: The logger.info call is being executed inside
page.evaluate (browser context) where the Node.js logger is undefined; change
the code so page.evaluate only posts the message and uses console.log inside the
browser if needed, and perform any Node-side logging with the Node logger
outside of page.evaluate (e.g., call logger.info after the await
page.evaluate(...) using syncMessage.data or the returned acknowledgment).
Update the page.evaluate invocation in widget-session-manager.ts (the function
call using page.evaluate, syncMessage and the iframe) to remove logger usage
from the evaluated function and move Node logger calls to run after the await
completes.
- Around line 733-765: The page.evaluate callback erroneously references the
Node-scoped logger (logger.info) which isn't available in the browser context;
update the session.page.evaluate call in widget-session-manager.ts (the block
using method, params, storeOnHost/isHostContextUpdate, iframe, and
__mcpHostContextUpdates) to remove any logger usage inside the browser function
and instead return a serializable result object (e.g., { stored: boolean, sent:
boolean, method: string, params: any }) from the evaluate call; then, after the
await completes in Node, inspect that returned object and call logger.info there
to record stored/sent events and details.
---
Nitpick comments:
In `@packages/inspector/src/oauth/provider.ts`:
- Around line 632-636: The log method is prepending a redundant
"[oauth:provider]" prefix; update the private log(message: string) method to
rely on the logger's own source identifier instead of adding the prefix—i.e.,
when this._debug is true call logger.info with the raw message (or a minimally
adjusted message) rather than `[oauth:provider] ${message}` so logging isn't
duplicated (refer to the log method, this._debug and logger.info to locate the
change).
In `@packages/inspector/src/oauth/wellknown-proxy.ts`:
- Around line 81-85: The log function in wellknown-proxy.ts emits messages with
a redundant “[oauth:wellknown-proxy]” prefix; update the log(message: string)
helper to call logger.info(message) (or logger.info with a concise message) when
debug is true so messages rely on the logger's existing source identifier rather
than duplicating the prefix; keep the same debug guard and function signature.
In `@packages/inspector/src/session/session-store.ts`:
- Around line 82-84: Remove the redundant "[SessionStore]" prefix from log
messages in this file: find all logger.info calls that include the literal
"[SessionStore]" (e.g., the creation log under the this.debug check and the
other info calls later in the SessionStore class) and change them to concise
messages without the prefix so the existing logger source identifier
(session-store) provides that context; update each affected call site to keep
the rest of the message text unchanged.
In `@packages/inspector/src/widget-server.ts`:
- Around line 432-436: The log method in WidgetServer duplicates the source
prefix because logger already uses "[widget-server]"; update the private
log(message: string) method to remove the manual "[WidgetServer]" prefix and
just pass the message string to logger.info when this.options.debug is true
(i.e., change the behavior in the log function so it calls logger.info(message)
instead of logger.info(`[WidgetServer] ${message}`) to avoid duplicate
prefixes).
| // Handle ui/initialize request | ||
| if (message && message.jsonrpc === '2.0' && message.method === 'ui/initialize') { | ||
| console.log('[MCP Host Emulator] Received ui/initialize, responding...'); | ||
| logger.info('[MCP Host Emulator] Received ui/initialize, responding...'); |
Contributor
There was a problem hiding this comment.
logger is undefined in browser context — these lines will throw at runtime.
Lines 222, 247, 251, and 280 are inside the string returned by getPlaywrightInitScript(). This code executes in the browser (Playwright page context), not Node.js. The logger object exists only in the Node.js module scope and will cause ReferenceError: logger is not defined when these paths execute.
Use console.log directly in the Playwright init script string, or remove these debug statements.
🐛 Proposed fix
// Handle ui/initialize request
if (message && message.jsonrpc === '2.0' && message.method === 'ui/initialize') {
- logger.info('[MCP Host Emulator] Received ui/initialize, responding...');
+ console.log('[MCP Host Emulator] Received ui/initialize, responding...');
// Respond with initialization success source: window,
}));
- logger.info('[MCP Host Emulator] Sent ui/initialize response');
+ console.log('[MCP Host Emulator] Sent ui/initialize response');
// Then send the tool result after a longer delay to ensure widget is ready
setTimeout(function() {
- logger.info('[MCP Host Emulator] Sending ui/notifications/tool-result...');
+ console.log('[MCP Host Emulator] Sending ui/notifications/tool-result...');
var resultMessage = { emu.displayMode = requestedMode;
emu.viewport = { width: sizing.width, height: sizing.height };
- logger.info('[MCP Host Emulator] Display mode changed to:', requestedMode, 'sizing:', sizing);
+ console.log('[MCP Host Emulator] Display mode changed to:', requestedMode, 'sizing:', sizing);
// Send responseAlso applies to: 247-247, 251-251, 280-280
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/inspector/src/hosts/mcp-host.ts` at line 222, The Playwright init
script returned by getPlaywrightInitScript() embeds Node-only logger calls
(e.g., logger.info(...) inside the returned string) which will throw in the
browser; update the script to use console.log (or remove the debug calls)
instead of the Node-scoped logger so the code runs in page context—search for
logger usage inside the string returned by getPlaywrightInitScript() and replace
those occurrences (e.g., the logger.info calls for ui/initialize and other
events) with console.log or delete them.
| })); | ||
|
|
||
| console.log('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing); | ||
| logger.info('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing); |
Contributor
There was a problem hiding this comment.
logger is undefined in browser context — this will throw at runtime.
Line 248 is inside the string returned by getPlaywrightInitScript(). This code executes in the browser (Playwright page context), not Node.js. The logger object exists only in the Node.js module scope and is not injected into the browser, so this will cause a ReferenceError: logger is not defined when requestDisplayMode is called in the widget.
Either remove this log line from the Playwright script, or use console.log directly (matching the pattern elsewhere in the init script).
🐛 Proposed fix
- logger.info('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing);
+ console.log('[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); | |
| console.log('[OpenAI Host] requestDisplayMode:', mode, 'sizing:', sizing); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/inspector/src/hosts/openai-host.ts` at line 248, The
Playwright-injected script returned by getPlaywrightInitScript() calls
logger.info inside the browser context (in requestDisplayMode), but logger is a
Node-only variable and will cause a ReferenceError at runtime; replace that call
with console.log (to match other init script logs) or remove the logging line
entirely so requestDisplayMode no longer references the undefined logger
variable. Ensure you update the string returned by getPlaywrightInitScript()
where requestDisplayMode is defined to use console.log or omit logging.
Comment on lines
+27
to
+31
| // eslint-disable-next-line no-undef, @typescript-eslint/no-unnecessary-condition | ||
| const envLevel = | ||
| typeof process !== "undefined" | ||
| ? ((process.env?.MCP_APPS_LOG_LEVEL as string | undefined) ?? "").toLowerCase() | ||
| : ""; |
Contributor
There was a problem hiding this comment.
ESLint disable comment does not cover process usage on line 30.
eslint-disable-next-line only suppresses rules for the immediate next physical line (line 28). The process reference on line 30 remains uncovered, causing the no-undef error flagged by static analysis.
Use block-style comments to cover the entire multi-line statement:
🔧 Proposed fix
- // eslint-disable-next-line no-undef, `@typescript-eslint/no-unnecessary-condition`
+ /* eslint-disable no-undef, `@typescript-eslint/no-unnecessary-condition` */
const envLevel =
typeof process !== "undefined"
? ((process.env?.MCP_APPS_LOG_LEVEL as string | undefined) ?? "").toLowerCase()
: "";
+ /* eslint-enable no-undef, `@typescript-eslint/no-unnecessary-condition` */🧰 Tools
🪛 ESLint
[error] 30-30: 'process' is not defined.
(no-undef)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/ui-react/src/hooks.ts` around lines 27 - 31, The eslint disable
comment currently only applies to the next physical line and doesn't cover the
multi-line declaration of the envLevel constant; update the comment to a
block-style directive (e.g., /* eslint-disable no-undef,
`@typescript-eslint/no-unnecessary-condition` */) placed immediately above the
multi-line expression that defines envLevel (the const envLevel = ... statement)
and re-enable rules after the block if desired, so the usage of process in the
envLevel computation is properly suppressed.
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.
Kaizen — 2026-02-19
What: Removed two
const self = thisaliases and their// eslint-disable-next-line @typescript-eslint/no-this-aliascomments from the mock transport constructors inpackages/testing/tests/unit/server/test-client.test.ts.Why: The
setTimeoutcallbacks were already arrow functions, which capturethislexically from the enclosing constructor. Theselfalias was therefore dead code — it added noise and required suppressing a lint rule unnecessarily.Change: 4 lines removed (2 disable comments + 2 variable declarations). Swapped
self.onclose→this.onclosein the arrow function bodies.Tests: All 210 tests in
@mcp-apps-kit/testingpass. Build green (cached).