Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
WalkthroughAdds domain-aware sub-module support for MCP servers: new module resolution/loading, runtime tool→module mapping, two opentiny modules, docs, and small typings/entrypoint tweaks; sidepanel URL resolution and MCP server creation now consider current page URL and module mappings. Changes
Sequence Diagram(s)sequenceDiagram
actor Client as Client/Page
participant Loader as Module Loader
participant Registry as Tool Registry
participant Resolver as URL Resolver
participant Server as MCP Server
Client->>Loader: request MCP tools for domain
Loader->>Loader: resolveModuleEntry(domain, meta.modules)
Loader->>Loader: loadSubModuleTools(domain, meta)
Note over Loader: dynamic import of module entry files
Loader->>Registry: register main tool
Loader->>Registry: register sub-module tools (merged)
Registry-->>Server: updated tool registry
Client->>Resolver: resolveTargetUrl(toolName, currentUrl)
Resolver->>Registry: getModuleInfoByToolName(toolName)
alt module found & domain/url match
Resolver-->>Client: return module.moduleUrl
else check jump links
Resolver-->>Client: return toolsJumpLinks[toolName] or meta.url
end
Client->>Server: invoke resolved tool
Server-->>Client: tool result
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Tip Try Coding Plans. Let us write the prompt for your AI agent so you can ship faster (with fewer bugs). 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 |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/next-wxt/entrypoints/sidepanel/mcpServer.ts (1)
66-68:⚠️ Potential issue | 🟠 Major子模块工具会被错误复用“最后一个 tab”。
当前分支只要没有
toolsJumpLinks就直接返回最后一个 tab,这会绕过targetUrl匹配;而子模块工具通常也没有toolsJumpLinks,会导致在错误页面执行工具。🔧 Proposed fix
- if (!meta.toolsJumpLinks?.[toolName]) { + const canReuseLastTab = !meta.toolsJumpLinks?.[toolName] && targetUrl === meta.url + if (canReuseLastTab) { return tabIds[tabIds.length - 1] }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/next-wxt/entrypoints/sidepanel/mcpServer.ts` around lines 66 - 68, The current early return when meta.toolsJumpLinks?.[toolName] is falsy causes submodule tools to always use tabIds[tabIds.length - 1]; instead, update the logic in mcpServer.ts so that when meta.toolsJumpLinks?.[toolName] is missing you do not immediately return the last tab but instead fallback to matching tabs by targetUrl (use the existing targetUrl variable and tabIds list to find the correct tab) and only default to tabIds[tabIds.length - 1] if no tab matches the targetUrl; adjust the branch that references meta.toolsJumpLinks, toolName, tabIds and targetUrl accordingly.
🤖 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/next-wxt/mcp-servers/index.ts`:
- Around line 10-11: toolModuleMap currently uses only toolName as the key which
allows same-named tools from different domains to clobber each other; change the
map to use domain-aware keys by either (A) switching toolModuleMap to a nested
map Map<domain, Map<toolName, ModuleInfo>> or (B) keep a single Map but use a
composite key like `${domain}::${toolName}`; update every place that
sets/gets/deletes from toolModuleMap (the original declaration toolModuleMap and
all uses where modules are registered/looked-up) to pass both domain and
toolName and compute/lookup the composite key (or index into the nested map)
consistently so reads/writes no longer collide across domains.
- Around line 112-113: The current submodule matching only compares origin and
pathname (currentUrlObj.origin and currentUrlObj.pathname vs moduleUrlObj) which
causes incorrect matches for routes that differ by query string or hash; update
the comparison to also include search and hash (e.g., compare
currentUrlObj.search and currentUrlObj.hash against
moduleUrlObj.search/moduleUrlObj.hash or compare the full serialized URL such as
currentUrlObj.href/moduleUrlObj.href) so that modules configured for more
specific URLs (hash/query) are not overmatched; locate the comparison using the
variables currentUrlObj and moduleUrlObj in the matching logic and add the
additional equality checks.
- Around line 29-30: Replace broad any usage by introducing precise type
aliases: declare a ToolRegistrationFunction type for the wrapper functions
returned/used in loadSubModuleTools and any create*Tool wrappers (reference:
loadSubModuleTools and the wrapper functions created around import.meta.glob
results), and replace Array<any> with Array<ToolRegistrationFunction> in
loadSubModuleTools signature and variable. Define a Result type for
getAllMcpServersByIsAlwaysEnabled describing the object shape (e.g., { id:
string; isAlwaysEnabled: boolean; ... }) and use it in that function's return
type instead of inline any. Also add a typed Params interface for the wrapper
functions' params parameter and update all occurrences (lines flagged) to use
these aliases so only the import.meta.glob unknowns remain untyped.
In `@packages/next-wxt/mcp-servers/MODULE_ROUTING.md`:
- Line 206: The fenced code block in MODULE_ROUTING.md is missing a language
identifier (causing markdownlint MD040); update the opening fence from ``` to
include a language tag (for example change the opening fence to ```text) so the
block is properly annotated and markdownlint no longer complains about MD040.
In `@packages/next-wxt/mcp-servers/opentiny.design/tiny-robot/index.ts`:
- Around line 31-37: Change each untyped catch to use catch(error: unknown) and
narrow to an Error before accessing .message; e.g., in the blocks that return
the response object containing the template string like `获取 TinyRobot
信息失败:${error.message}`, replace direct access with: const message = error
instanceof Error ? error.message : String(error) and use `${message}` in the
returned content. Apply the same change to the other two similar catch blocks
that build error text responses (the blocks returning the "更新 TinyRobot 信息失败…"
and "注册 TinyRobot 信息失败…" messages) so all three catch handlers use the narrowed
message variable.
In `@packages/next-wxt/mcp-servers/opentiny.design/tiny-vue/index.ts`:
- Around line 34-40: The catch blocks in index.ts access error.message directly;
change the catch parameters to type unknown and narrow before using .message
(e.g., in the first catch around the "获取组件文档失败" return and the second catch at
the later block), by checking if error is an instance of Error and using
error.message, otherwise convert to a safe string (String(error) or a default
message); update both catch blocks that currently reference error.message so
TypeScript strict mode is satisfied.
---
Outside diff comments:
In `@packages/next-wxt/entrypoints/sidepanel/mcpServer.ts`:
- Around line 66-68: The current early return when
meta.toolsJumpLinks?.[toolName] is falsy causes submodule tools to always use
tabIds[tabIds.length - 1]; instead, update the logic in mcpServer.ts so that
when meta.toolsJumpLinks?.[toolName] is missing you do not immediately return
the last tab but instead fallback to matching tabs by targetUrl (use the
existing targetUrl variable and tabIds list to find the correct tab) and only
default to tabIds[tabIds.length - 1] if no tab matches the targetUrl; adjust the
branch that references meta.toolsJumpLinks, toolName, tabIds and targetUrl
accordingly.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
packages/next-wxt/components.d.tspackages/next-wxt/entrypoints/sidepanel/mcpServer.tspackages/next-wxt/mcp-servers/MODULE_ROUTING.mdpackages/next-wxt/mcp-servers/index.tspackages/next-wxt/mcp-servers/opentiny.design/meta.tspackages/next-wxt/mcp-servers/opentiny.design/tiny-robot/index.tspackages/next-wxt/mcp-servers/opentiny.design/tiny-vue/index.tspackages/next-wxt/mcp-servers/types.d.tspackages/next-wxt/utils/createMcpServer.ts
💤 Files with no reviewable changes (1)
- packages/next-wxt/components.d.ts
| // 工具名称到模块信息的映射表(用于路由拦截) | ||
| const toolModuleMap = new Map<string, ModuleInfo>() |
There was a problem hiding this comment.
toolModuleMap 仅用 toolName 作为 key 会发生跨域覆盖。
不同域名如果出现同名工具,后注册的映射会覆盖先注册的映射,导致路由解析退回默认 URL 或跳错模块页。
🔧 Suggested direction
-const toolModuleMap = new Map<string, ModuleInfo>()
+const toolModuleMap = new Map<string, Map<string, ModuleInfo>>() // toolName -> (domain -> info)并把写入/读取改为 (toolName, domain) 双键语义,避免跨域同名冲突。
Also applies to: 54-58, 127-131, 179-188
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/next-wxt/mcp-servers/index.ts` around lines 10 - 11, toolModuleMap
currently uses only toolName as the key which allows same-named tools from
different domains to clobber each other; change the map to use domain-aware keys
by either (A) switching toolModuleMap to a nested map Map<domain, Map<toolName,
ModuleInfo>> or (B) keep a single Map but use a composite key like
`${domain}::${toolName}`; update every place that sets/gets/deletes from
toolModuleMap (the original declaration toolModuleMap and all uses where modules
are registered/looked-up) to pass both domain and toolName and compute/lookup
the composite key (or index into the nested map) consistently so reads/writes no
longer collide across domains.
| function loadSubModuleTools(domain: string, meta: MetaConfig): Array<any> { | ||
| const subModuleTools: Array<any> = [] |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check TypeScript configuration
find . -name "tsconfig.json" -type f | head -5 | xargs catRepository: opentiny/next-sdk
Length of output: 1921
🏁 Script executed:
#!/bin/bash
# Examine the specific file and line ranges mentioned
wc -l packages/next-wxt/mcp-servers/index.tsRepository: opentiny/next-sdk
Length of output: 103
🏁 Script executed:
#!/bin/bash
# View the entire file to understand context
cat -n packages/next-wxt/mcp-servers/index.ts | head -220Repository: opentiny/next-sdk
Length of output: 8565
🏁 Script executed:
#!/bin/bash
# View the rest of the file from line 200 onward
cat -n packages/next-wxt/mcp-servers/index.ts | tail -60Repository: opentiny/next-sdk
Length of output: 2001
🏁 Script executed:
#!/bin/bash
# Get the types.ts to understand the type definitions available
cat packages/next-wxt/mcp-servers/types.tsRepository: opentiny/next-sdk
Length of output: 131
🏁 Script executed:
#!/bin/bash
# Find the types file
find packages/next-wxt/mcp-servers -name "types*" -o -name "*.d.ts"Repository: opentiny/next-sdk
Length of output: 101
🏁 Script executed:
#!/bin/bash
# Check what's in the mcp-servers directory
ls -la packages/next-wxt/mcp-servers/Repository: opentiny/next-sdk
Length of output: 493
🏁 Script executed:
#!/bin/bash
# Try to find where types are defined
rg "export.*MetaConfig|export.*ModuleInfo" packages/next-wxtRepository: opentiny/next-sdk
Length of output: 424
🏁 Script executed:
#!/bin/bash
# Read the types.d.ts file
cat -n packages/next-wxt/mcp-servers/types.d.tsRepository: opentiny/next-sdk
Length of output: 1733
Reduce any type usage with type aliases for tool registration functions to maintain strict typing.
The code uses any extensively (lines 29, 43, 46, 52, 93, 102, 119, 122, 126, 156, 206-207, 209, 217, 224), which violates the coding guideline requiring strict TypeScript typing. While some any usage is unavoidable due to Vite's import.meta.glob() returning untyped records, define explicit type aliases to improve type safety:
- Create a
ToolRegistrationFunctiontype for the wrapper functions instead of using rawany - Define a type for the result structure in
getAllMcpServersByIsAlwaysEnabledinstead of using inline object withanyproperties - Type the
paramsparameter in wrapper functions more precisely where possible
This will narrow the scope of untyped code and provide better IDE support and compile-time safety.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/next-wxt/mcp-servers/index.ts` around lines 29 - 30, Replace broad
any usage by introducing precise type aliases: declare a
ToolRegistrationFunction type for the wrapper functions returned/used in
loadSubModuleTools and any create*Tool wrappers (reference: loadSubModuleTools
and the wrapper functions created around import.meta.glob results), and replace
Array<any> with Array<ToolRegistrationFunction> in loadSubModuleTools signature
and variable. Define a Result type for getAllMcpServersByIsAlwaysEnabled
describing the object shape (e.g., { id: string; isAlwaysEnabled: boolean; ...
}) and use it in that function's return type instead of inline any. Also add a
typed Params interface for the wrapper functions' params parameter and update
all occurrences (lines flagged) to use these aliases so only the
import.meta.glob unknowns remain untyped.
| if (currentUrlObj.origin === moduleUrlObj.origin && | ||
| currentUrlObj.pathname === moduleUrlObj.pathname) { |
There was a problem hiding this comment.
子模块匹配仅比较 origin + pathname,会误匹配 hash/search 路由。
当前逻辑对 hash/query 路由无法区分。配置了更具体 URL(例如 hash 路由)的模块会被过度匹配。
🔧 Proposed fix
- if (currentUrlObj.origin === moduleUrlObj.origin &&
- currentUrlObj.pathname === moduleUrlObj.pathname) {
+ const sameBase =
+ currentUrlObj.origin === moduleUrlObj.origin &&
+ currentUrlObj.pathname === moduleUrlObj.pathname
+ const sameHash = !moduleUrlObj.hash || currentUrlObj.hash === moduleUrlObj.hash
+ const sameSearch = !moduleUrlObj.search || currentUrlObj.search === moduleUrlObj.search
+
+ if (sameBase && sameHash && sameSearch) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/next-wxt/mcp-servers/index.ts` around lines 112 - 113, The current
submodule matching only compares origin and pathname (currentUrlObj.origin and
currentUrlObj.pathname vs moduleUrlObj) which causes incorrect matches for
routes that differ by query string or hash; update the comparison to also
include search and hash (e.g., compare currentUrlObj.search and
currentUrlObj.hash against moduleUrlObj.search/moduleUrlObj.hash or compare the
full serialized URL such as currentUrlObj.href/moduleUrlObj.href) so that
modules configured for more specific URLs (hash/query) are not overmatched;
locate the comparison using the variables currentUrlObj and moduleUrlObj in the
matching logic and add the additional equality checks.
|
|
||
| ## 目录结构 | ||
|
|
||
| ``` |
There was a problem hiding this comment.
补充 fenced code block 的语言标识。
该代码块缺少语言标识,会触发 markdownlint MD040。
🔧 Proposed fix
-```
+```text
packages/next-wxt/mcp-servers/
...
-```
+```🧰 Tools
🪛 markdownlint-cli2 (0.21.0)
[warning] 206-206: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/next-wxt/mcp-servers/MODULE_ROUTING.md` at line 206, The fenced code
block in MODULE_ROUTING.md is missing a language identifier (causing
markdownlint MD040); update the opening fence from ``` to include a language tag
(for example change the opening fence to ```text) so the block is properly
annotated and markdownlint no longer complains about MD040.
| } catch (error) { | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: `获取 TinyRobot 信息失败:${error.message}` | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat packages/next-wxt/mcp-servers/opentiny.design/tiny-robot/index.tsRepository: opentiny/next-sdk
Length of output: 3776
🏁 Script executed:
fd tsconfig.json | head -20 | xargs -I {} sh -c 'echo "=== {} ===" && cat {}'Repository: opentiny/next-sdk
Length of output: 2298
🏁 Script executed:
find packages/next-wxt -name "tsconfig.json" -o -name ".wxt"Repository: opentiny/next-sdk
Length of output: 92
🏁 Script executed:
find . -path "*/.wxt/tsconfig.json" -o -path "*/.wxt*" -type d | head -10Repository: opentiny/next-sdk
Length of output: 43
🏁 Script executed:
# Check root level and common build directories for wxt config
ls -la packages/next-wxt/ | grep -E "(wxt|tsconfig|dist|build)"Repository: opentiny/next-sdk
Length of output: 188
🏁 Script executed:
cat packages/next-wxt/wxt.config.tsRepository: opentiny/next-sdk
Length of output: 2359
🏁 Script executed:
cat tsconfig.json 2>/dev/null || echo "No root tsconfig.json"Repository: opentiny/next-sdk
Length of output: 82
🏁 Script executed:
# Check if there's a tsconfig in the wxt directory or if it's generated elsewhere
find packages/next-wxt -type f -name "*.json" | grep -i config | head -20Repository: opentiny/next-sdk
Length of output: 92
🏁 Script executed:
# Check wxt version and configuration
cat packages/next-wxt/package.json | grep -A 20 '"dependencies"'Repository: opentiny/next-sdk
Length of output: 726
🏁 Script executed:
# Check if there are any other TypeScript files in next-wxt to see the pattern
rg "catch\s*\(" packages/next-wxt/src --type ts -A 3 | head -40Repository: opentiny/next-sdk
Length of output: 122
🏁 Script executed:
# Search for all catch blocks in next-wxt to see patterns
find packages/next-wxt -type f \( -name "*.ts" -o -name "*.tsx" \) | xargs rg "catch\s*\(" -A 3 | head -60Repository: opentiny/next-sdk
Length of output: 3913
🏁 Script executed:
# Check if there's a root or workspace tsconfig that defines compiler options
find . -name "tsconfig.base.json" -o -name "tsconfig.*.json" | head -10 | xargs cat 2>/dev/nullRepository: opentiny/next-sdk
Length of output: 2585
🏁 Script executed:
# Check WXT's actual TypeScript configuration by looking at node_modules (if available) or defaults
rg "noImplicitAny|strict" packages/next-wxt/ --type jsonRepository: opentiny/next-sdk
Length of output: 43
🏁 Script executed:
# Check if WXT has any TypeScript configuration defaults
npm list wxt 2>/dev/null | head -5Repository: opentiny/next-sdk
Length of output: 107
🏁 Script executed:
# Try to understand what TypeScript settings WXT uses by checking common patterns
# Look at how the file is actually compiled/checked
rg "noImplicitAny" packages/ --type json -A 1 -B 1Repository: opentiny/next-sdk
Length of output: 43
🏁 Script executed:
# Check if there's any tsconfig that applies to the mcp-servers directory
find packages/next-wxt/mcp-servers -name "tsconfig.json" -o -name "*.json" | xargs cat 2>/dev/nullRepository: opentiny/next-sdk
Length of output: 43
Untyped catch blocks accessing error.message violate strict typing requirements.
Under TypeScript strict mode (as required by the coding guidelines "All code must be TypeScript with strict typing"), catch blocks receive unknown type and cannot safely access .message without type narrowing. Apply the proposed type narrowing pattern across all three instances to ensure type safety:
catch (error: unknown) {
const message = error instanceof Error ? error.message : String(error)Affects lines 31-37, 85-91, and 127-133.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/next-wxt/mcp-servers/opentiny.design/tiny-robot/index.ts` around
lines 31 - 37, Change each untyped catch to use catch(error: unknown) and narrow
to an Error before accessing .message; e.g., in the blocks that return the
response object containing the template string like `获取 TinyRobot
信息失败:${error.message}`, replace direct access with: const message = error
instanceof Error ? error.message : String(error) and use `${message}` in the
returned content. Apply the same change to the other two similar catch blocks
that build error text responses (the blocks returning the "更新 TinyRobot 信息失败…"
and "注册 TinyRobot 信息失败…" messages) so all three catch handlers use the narrowed
message variable.
| } catch (error) { | ||
| return { | ||
| content: [ | ||
| { | ||
| type: 'text', | ||
| text: `获取组件文档失败:${error.message}` | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1. Find and examine the specific file
echo "=== Checking file existence and content ==="
fd -a "index.ts" packages/next-wxt/mcp-servers/opentiny.design/tiny-vue/ 2>/dev/null
# 2. Check tsconfig.json for strict settings
echo -e "\n=== Checking tsconfig.json strict settings ==="
fd tsconfig.json -x cat {}
# 3. Read the file and check lines around 34-40 and 90-96
echo -e "\n=== File content around lines 30-45 ==="
cat -n "packages/next-wxt/mcp-servers/opentiny.design/tiny-vue/index.ts" | sed -n '30,45p'
echo -e "\n=== File content around lines 85-100 ==="
cat -n "packages/next-wxt/mcp-servers/opentiny.design/tiny-vue/index.ts" | sed -n '85,100p'
# 4. Search all catch blocks in the file
echo -e "\n=== All catch blocks in the file ==="
rg -n 'catch\s*\([^)]*\)' "packages/next-wxt/mcp-servers/opentiny.design/tiny-vue/index.ts" -A 3Repository: opentiny/next-sdk
Length of output: 3416
Narrow catch error types before accessing .message property.
The code directly accesses error.message in two catch blocks (lines 34 and 90) without type narrowing. Since TypeScript strict mode is enabled, catch blocks must explicitly type the error parameter as unknown and perform proper type narrowing.
🔧 Proposed fix
- } catch (error) {
+ } catch (error: unknown) {
+ const message = error instanceof Error ? error.message : String(error)
return {
content: [
{
type: 'text',
- text: `获取组件文档失败:${error.message}`
+ text: `获取组件文档失败:${message}`
}
]
}
}Apply the same pattern to the second catch block at line 90.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/next-wxt/mcp-servers/opentiny.design/tiny-vue/index.ts` around lines
34 - 40, The catch blocks in index.ts access error.message directly; change the
catch parameters to type unknown and narrow before using .message (e.g., in the
first catch around the "获取组件文档失败" return and the second catch at the later
block), by checking if error is an instance of Error and using error.message,
otherwise convert to a safe string (String(error) or a default message); update
both catch blocks that currently reference error.message so TypeScript strict
mode is satisfied.
Pull Request (OpenTiny NEXT-SDKs)
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Documentation
Chores