update the client file , for error classes are now in ./errors/index.js#592
update the client file , for error classes are now in ./errors/index.js#592hamzaMissewi wants to merge 9 commits into
Conversation
fix(mcp): handle optional @ant/claude-for-chrome-mcp dependency Provides clear runtime error message if feature is used without the optional dependency present.
kevincodex1
requested review from
Vasanthdev2004,
anandh8x,
auriti,
gnanam1990 and
kevincodex1
gnanam1990
left a comment
gnanam1990
left a comment
There was a problem hiding this comment.
Thanks for the PR. I pulled the current head locally and I can’t call this merge-ready yet.
The main issue is that the PR description and the actual change scope do not match. The description frames this as a narrow fix to make @ant/claude-for-chrome-mcp optional so open-source builds can compile cleanly, but the current diff is much broader than that. In addition to the optional-dependency handling in src/services/mcp/client.ts, it also introduces new MCP error-class files, new transport factory files, changes in src/components/agents/ModelSelector.tsx, and unrelated command test additions. That makes it harder to review and increases the risk surface beyond the stated goal.
I also hit a concrete current-head failure locally:
- bun test ./src/commands/mcp/mcp.test.tsx ./src/commands/auto-fix.test.ts ./src/commands/version.test.ts
Current result:
- SyntaxError: Export named 'McpToolCallError_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS' not found in module '/tmp/openclaude-pr592/src/services/mcp/client.ts'
From the current branch, the error classes were moved into src/services/mcp/errors/index.ts, but there are still consumers importing McpToolCallError_I_VERIFIED_THIS_IS_NOT_CODE_OR_FILEPATHS from src/services/mcp/client.ts (for example in the MCP/tool execution path). So the branch is not clean on current head yet.
What I’d want before re-review:
- Fix the broken import/export path so the current head passes focused MCP-related tests cleanly.
- Either narrow this PR back down to the optional dependency fix, or update the title/description and split out the unrelated MCP refactor pieces into a separate PR.
- Add focused verification for the optional-dependency path itself, since that is the stated user-facing goal.
Right now, with the current-head import break and the scope mismatch, I’d keep this as needs changes.
|
bro please fix broken tests |
…rminal Add intelligent provider routing system that automatically selects optimal providers based on latency, cost, and health metrics. New features: - scripts/smart-router.ts: Launcher for Python router subprocess - src/utils/claudeInChrome/smartRouterBridge.ts: TypeScript bridge with health checks, routing API, and provider selection - package.json: Add dev:router script for easy startup Agent summary enhancements: - Add AgentSummaryConfig interface for customizable intervals - Implement timeout handling (10s default) for summary generation - Add duplicate detection to skip unchanged summaries - Add failure tracking with exponential backoff (5 max failures) - Validate empty/whitespace summaries Fixes: - Make @ant/claude-for-chrome-mcp optional dependency with graceful error handling in StdioTransportFactory.ts Usage: bun run dev:router # Start smart router bun run dev:router --strategy=latency --port=8080
…rminal Add intelligent provider routing system that automatically selects optimal providers based on latency, cost, and health metrics. New features: - scripts/smart-router.ts: Launcher for Python router subprocess - src/utils/claudeInChrome/smartRouterBridge.ts: TypeScript bridge with health checks, routing API, and provider selection - package.json: Add dev:router script for easy startup Agent summary enhancements: - Add AgentSummaryConfig interface for customizable intervals - Implement timeout handling (10s default) for summary generation - Add duplicate detection to skip unchanged summaries - Add failure tracking with exponential backoff (5 max failures) - Validate empty/whitespace summaries Fixes: - Make @ant/claude-for-chrome-mcp optional dependency with graceful error handling in StdioTransportFactory.ts Usage: bun run dev:router # Start smart router bun run dev:router --strategy=latency --port=8080
|
Hello Thank you for your replies and remarks @gnanam1990 and kevin, |
| @@ -1,5 +1,5 @@ | |||
| // Stub — AssistantSessionChooser not included in source snapshot | |||
| import React from 'react' | |||
… types for open-source builds - Add try-catch around mcpSkills.js require for graceful fallback when module unavailable - Re-export MCP error classes from client.ts for backward compatibility - Create src/types/message.ts with AssistantMessage, UserMessage, Message types - Add tests for error class exports in client.test.ts - Fix import path in StreamingToolExecutor.ts from mailbox.js to message.js"
3 participants
Problem
TypeScript compilation fails with "Cannot find module '@ant/claude-for-chrome-mcp'" because this internal Anthropic dependency is not available in open-source builds.
Provides clear runtime error message if feature is used without the optional dependency present.
Solution
Make the Chrome MCP server dependency optional by wrapping its import in try-catch. This allows compilation to succeed while providing a clear error message if the feature is used without the dependency.
Summary
Impact
Testing
bun run buildbun run smokeNotes