feat: bundle.json mcp server#309
Open
msslulu wants to merge 12 commits intoopentiny:developfrom
Open
Conversation
WalkthroughAdds MCP server support and Spring AI tooling: expands MyBatis mapper scan, enables MCP server in dev config, adds a ToolCallbackProvider, introduces GitFileReaderService to import bundle.json (HTTP or git), provides URL validation utility, and adds spring-ai and JGit dependencies. Changes
Sequence DiagramsequenceDiagram
participant Client as MCP Client
participant MCP as MCP Server
participant Tool as GitFileReaderService
participant Source as HTTP/Git Source
participant Parser as JSON Parser
participant DB as Database Mappers
Client->>MCP: invoke bundle_create(url)
MCP->>Tool: call readFileFromRepo(url)
Tool->>Source: fetch bytes (HTTP GET or JGit clone)
Source-->>Tool: file bytes
Tool->>Tool: removeBOM / isValidJson
Tool->>Parser: parse BundleDto -> components/packages
Parser-->>Tool: ComponentList, PackageList
Tool->>DB: upsert ComponentLibrary entries
DB-->>Tool: upsert results
Tool->>DB: bulkCreate components / insert/update material/history records
DB-->>Tool: persistence results
Tool-->>MCP: FileResult (counts / status)
MCP-->>Client: response
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 0/1 reviews remaining, refill in 60 minutes.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pom.xml (1)
43-76:⚠️ Potential issue | 🟠 MajorSpring AI 1.1.4 requires Spring Boot 3.4.x or 3.5.x minimum; current baseline is 3.1.9.
Spring AI 1.1.4 is incompatible with Spring Boot 3.1.9. The official documentation explicitly states support only for Spring Boot 3.4.x and 3.5.x; release notes confirm upgrades to Spring Boot 3.5.x versions. Adding
spring-ai-starter-mcp-server-webmvcwithout upgrading the Spring Boot parent version will cause dependency resolution and auto-configuration failures. Upgrade the Spring Boot parent to at least 3.4.x (preferably 3.5.x) before merging this dependency.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@pom.xml` around lines 43 - 76, The pom currently adds the dependency artifactId spring-ai-starter-mcp-server-webmvc using property spring-ai.version (1.1.4) which requires Spring Boot 3.4.x+; update the project’s Spring Boot parent/version to at least 3.4.x (preferably 3.5.x) before adding this dependency so dependency resolution and auto-configuration succeed—modify the parent or spring-boot.version property to a 3.5.x release, adjust any incompatible plugin or dependency versions if the build flags them, and then refresh dependencies (e.g., mvn -U) and run the test/build to verify compatibility.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java`:
- Around line 52-60: The readFileFromRepo method currently passes unvalidated
caller URLs into fetchBytes, creating an SSRF risk; before calling
fetchBytes(url) validate the URL's host against your approved-hosts/allowlist
(reuse or add a helper like isHostAllowed(String host) or AllowedHosts class),
parse the URL using new URL(url) to extract the host, reject or throw if the
host is not allowed, and apply the same validation to the other code path
referenced (the block around lines 149-156) so both usages of fetchBytes enforce
the allowlist.
- Around line 91-98: Check the Result returned by parseBundle(BundleDto) before
dereferencing its getData(): if bundleResultDtoResult.isFailure() or
bundleResultDtoResult.getData() is null, log/return a deterministic validation
error and avoid calling getComponentList()/getPackageList(); additionally
validate BundleDto.getMaterials() (and other required fields) for null/invalid
structure before calling parseBundle or make parseBundle return a clear
validation Result when materials is null so callers (GitFileReaderService) can
handle it safely; update code around the use of BundleDto, parseBundle,
BundleResultDto, getData, getComponentList, getPackageList, and getMaterials
accordingly.
- Around line 365-372: The code currently hard-codes materialId=1 and
materialHistoryId=1 when creating MaterialComponent and
MaterialHistoryComponent; instead, query or create the correct material and
history IDs and use those values. Before calling
baseMapper.createMaterialComponent and
baseMapper.createMaterialHistoryComponent, call or add appropriate baseMapper
methods (e.g., getOrCreateMaterial(...), findMaterialByName(...),
createMaterial(...) and corresponding getOrCreateMaterialHistory(...)) to obtain
the real materialId and materialHistoryId, then call
materialComponent.setMaterialId(obtainedMaterialId) and
materialHistoryComponent.setMaterialHistoryId(obtainedMaterialHistoryId)
(keeping component.getId() as-is).
- Around line 321-324: The loop in GitFileReaderService currently overwrites
each ComponentLibrary's framework with the hardcoded string "Vue" (see the
variable componentLibrary and the packages/map loop), causing framework metadata
to be incorrect for non-Vue bundles; change the assignment to read the framework
from the source data or the owning bundle (e.g., use library.get("framework") if
present, otherwise fallback to bundleDto.getFramework()) and set that value on
componentLibrary instead of the constant "Vue" so framework remains consistent
across related records.
- Around line 105-135: The current import routine in GitFileReaderService
swallows all Exceptions, returns an error string, and thus prevents
`@Transactional` rollbacks leading to partially applied data; update the method
handling the component/library import to run within a transactional boundary
(add `@Transactional` with appropriate rollbackFor if needed) and stop converting
all Exceptions to a String—log the error (using log.error) but then rethrow the
exception (or throw a RuntimeException) so the transaction can roll back;
specifically modify the method that calls
componentLibraryMapper.createComponentLibrary/updateComponentLibraryById and
bulkCreate (and the catch block shown) to either remove the catch or rethrow
after logging, and apply the same fix to the other affected block (lines
referenced ~332-389) so database changes are not left half-committed.
- Around line 69-76: The JSON validation runs against the raw bytes
(jsonContent) before stripping a possible UTF-8 BOM, causing valid BOM-prefixed
files to be rejected; call removeBOM(...) first and assign to jsonString, then
call isValidJson(jsonString) (use jsonString for the validation and in the error
log), so change the order around the removeBOM and isValidJson calls and
reference jsonString in the log/return path (methods: removeBOM, isValidJson;
variables: jsonContent, jsonString).
- Around line 361-374: The code increments addNum regardless of whether the DB
writes succeed; change the logic in the block that handles
queryComponent.isEmpty() so that addNum is incremented only after
baseMapper.createComponent(component) returns success and the subsequent
relation writes (baseMapper.createMaterialComponent(...) and
baseMapper.createMaterialHistoryComponent(...)) also succeed; use the return
values of createComponent, createMaterialComponent and
createMaterialHistoryComponent (or throw/catch exceptions) to verify all three
writes completed before incrementing addNum and handle/rollback or log failures
when any of the relation writes fail instead of counting the insert as
successful.
---
Outside diff comments:
In `@pom.xml`:
- Around line 43-76: The pom currently adds the dependency artifactId
spring-ai-starter-mcp-server-webmvc using property spring-ai.version (1.1.4)
which requires Spring Boot 3.4.x+; update the project’s Spring Boot
parent/version to at least 3.4.x (preferably 3.5.x) before adding this
dependency so dependency resolution and auto-configuration succeed—modify the
parent or spring-boot.version property to a 3.5.x release, adjust any
incompatible plugin or dependency versions if the build flags them, and then
refresh dependencies (e.g., mvn -U) and run the test/build to verify
compatibility.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 614b97a1-b2f7-41c4-a788-6013f0849d7c
📒 Files selected for processing (8)
app/src/main/java/com/tinyengine/it/TinyEngineApplication.javaapp/src/main/resources/application-dev.ymlapp/src/main/resources/sql/h2/init_data_for_test_v1.0.0.sqlapp/src/main/resources/sql/mysql/init_data_for_test_v1.0.0.sqlbase/src/main/java/com/tinyengine/it/mcp/config/McpToolsConfig.javabase/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.javadocker-deploy-data/mysql/init/init_data_for_test_v1.0.0.sqlpom.xml
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (4)
base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java (4)
344-348:⚠️ Potential issue | 🟠 Major | ⚡ Quick winStop forcing package framework to
"Vue".Line 347 overwrites framework metadata and can make package records inconsistent with bundle/component framework values.
Suggested fix
- componentLibrary.setFramework("Vue"); + Object framework = library.get("framework"); + componentLibrary.setFramework( + framework != null ? String.valueOf(framework) : bundleDto.getFramework() + );🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java` around lines 344 - 348, The code in GitFileReaderService is overwriting the package framework by calling componentLibrary.setFramework("Vue");; instead, preserve the framework from the source data or the mapped bean: remove the hard-coded setFramework("Vue") call and, if needed, read framework from the library map (e.g., library.get("framework")) or from the mapped ComponentLibrary instance (from BeanUtil.mapToBean) and only overwrite if the source value is absent. Ensure setPackageName(String.valueOf(library.get("package"))) remains but do not force a static framework value.
121-157:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftDo not swallow exceptions around multi-table writes.
Line 153 returns
"Error: ..."after broad catch while this method executes library/component writes. This can leave partial data without a reliable rollback boundary.Suggested fix direction
+ `@Transactional`(rollbackFor = Exception.class) public String readFileFromRepo(String url) { try { ... } catch (Exception e) { - e.printStackTrace(); - log.error("从Raw GitHub URL {} 中读取文件 '{}' 失败: {}", url, "bundle.json", e.getMessage()); - return "Error: " + e.getMessage(); + log.error("从Raw GitHub URL {} 中读取文件 '{}' 失败", url, "bundle.json", e); + throw e; } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java` around lines 121 - 157, The catch block in GitFileReaderService around the multi-table writes (calls to componentLibraryMapper.createComponentLibrary/updateComponentLibraryById and bulkCreate) currently logs and returns "Error: ..." which swallows exceptions and can leave partially committed data; change this to either wrap the method in a transactional boundary (e.g., annotate the service method with `@Transactional` or begin/rollback a transaction) so any exception (from createComponentLibrary, updateComponentLibraryById, or bulkCreate) triggers a rollback, or rethrow the exception after logging so callers can handle it; ensure you remove the swallow-return behavior and consistently log the exception (use log.error with the exception object) and propagate or roll back to avoid partial commits.
393-398:⚠️ Potential issue | 🟠 Major | ⚡ Quick winValidate relation insert results before counting success.
Line 398 increments
addNumwithout checking whethercreateMaterialComponentandcreateMaterialHistoryComponentactually succeeded.Suggested fix
MaterialComponent materialComponent = new MaterialComponent(); materialComponent.setMaterialId(1); materialComponent.setComponentId(component.getId()); - baseMapper.createMaterialComponent(materialComponent); + Integer mc = baseMapper.createMaterialComponent(materialComponent); MaterialHistoryComponent materialHistoryComponent = new MaterialHistoryComponent(); materialHistoryComponent.setComponentId(component.getId()); materialHistoryComponent.setMaterialHistoryId(1); - baseMapper.createMaterialHistoryComponent(materialHistoryComponent); - addNum = addNum + 1; + Integer mhc = baseMapper.createMaterialHistoryComponent(materialHistoryComponent); + if (mc != 1 || mhc != 1) { + throw new IllegalStateException("create relation rows failed: " + mc + ", " + mhc); + } + addNum++;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java` around lines 393 - 398, The code unconditionally increments addNum after calling baseMapper.createMaterialComponent(...) and baseMapper.createMaterialHistoryComponent(...); instead capture and check their results (e.g., int rowsAffected = baseMapper.createMaterialComponent(materialComponent); int histRows = baseMapper.createMaterialHistoryComponent(materialHistoryComponent); or catch exceptions) and only increment addNum when both calls indicate success (rowsAffected > 0 && histRows > 0); otherwise log an error or handle the failure path for materialComponent/materialHistoryComponent so you don't count failed inserts as successes.
390-397:⚠️ Potential issue | 🟠 Major | 🏗️ Heavy liftReplace fixed material/history IDs with resolved IDs.
Lines 391 and 396 hardcode
materialId=1andmaterialHistoryId=1, which can silently bind imported components to the wrong material context.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java` around lines 390 - 397, The code currently hardcodes materialId=1 and materialHistoryId=1 when creating MaterialComponent and MaterialHistoryComponent, which must be replaced by resolved IDs: lookup or create the correct material ID and its history ID (via the appropriate baseMapper or repository methods) based on the component/context, assign those resolved IDs to materialComponent.setMaterialId(...) and materialHistoryComponent.setMaterialHistoryId(...), then call baseMapper.createMaterialComponent(...) and baseMapper.createMaterialHistoryComponent(...); keep using component.getId() for componentId. Ensure you handle the case where the material or history does not exist by creating it and using the returned/generated ID.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java`:
- Around line 170-183: The fetchBytes method currently allows HttpURLConnection
to auto-follow redirects which can bypass earlier
urlValidateUtil.validateFinalUrl checks; update fetchBytes to disable automatic
redirects on the HttpURLConnection (call setInstanceFollowRedirects(false) or
equivalent on the connection) and explicitly treat any 3xx response codes as
failures by rejecting responses with responseCode / 100 == 3 (i.e., throw an
IOException with a clear message) before reading the response stream; ensure
this change is applied inside the fetchBytes method around where connection is
created and responseCode is checked so redirects cannot lead to internal
endpoints.
In `@base/src/main/java/com/tinyengine/it/mcp/utils/UrlValidateUtil.java`:
- Around line 54-59: The current early return when isLoopback true in
UrlValidateUtil bypasses further checks and reopens SSRF risk; remove the
short-circuit so that enforceHttpsAndIpCheck(uri, host) always runs regardless
of isLoopback (i.e., delete the "if (isLoopback) { return; }" branch or invert
logic to call enforceHttpsAndIpCheck before any return), ensuring
enforceHttpsAndIpCheck still executes for loopback hosts while preserving any
prior allowlist logic.
- Around line 21-27: The validateFinalUrl method in UrlValidateUtil currently
calls new URI(finalUrl) without guarding against null or blank input, which
causes an NPE instead of the intended ServiceException; add an explicit check at
the start of validateFinalUrl (e.g., if finalUrl is null or
finalUrl.trim().isEmpty()) and throw the same ServiceException("400", "Invalid
baseUrl format") for null/blank inputs before attempting new URI(finalUrl), then
keep the existing try/catch for URISyntaxException.
---
Duplicate comments:
In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java`:
- Around line 344-348: The code in GitFileReaderService is overwriting the
package framework by calling componentLibrary.setFramework("Vue");; instead,
preserve the framework from the source data or the mapped bean: remove the
hard-coded setFramework("Vue") call and, if needed, read framework from the
library map (e.g., library.get("framework")) or from the mapped ComponentLibrary
instance (from BeanUtil.mapToBean) and only overwrite if the source value is
absent. Ensure setPackageName(String.valueOf(library.get("package"))) remains
but do not force a static framework value.
- Around line 121-157: The catch block in GitFileReaderService around the
multi-table writes (calls to
componentLibraryMapper.createComponentLibrary/updateComponentLibraryById and
bulkCreate) currently logs and returns "Error: ..." which swallows exceptions
and can leave partially committed data; change this to either wrap the method in
a transactional boundary (e.g., annotate the service method with `@Transactional`
or begin/rollback a transaction) so any exception (from createComponentLibrary,
updateComponentLibraryById, or bulkCreate) triggers a rollback, or rethrow the
exception after logging so callers can handle it; ensure you remove the
swallow-return behavior and consistently log the exception (use log.error with
the exception object) and propagate or roll back to avoid partial commits.
- Around line 393-398: The code unconditionally increments addNum after calling
baseMapper.createMaterialComponent(...) and
baseMapper.createMaterialHistoryComponent(...); instead capture and check their
results (e.g., int rowsAffected =
baseMapper.createMaterialComponent(materialComponent); int histRows =
baseMapper.createMaterialHistoryComponent(materialHistoryComponent); or catch
exceptions) and only increment addNum when both calls indicate success
(rowsAffected > 0 && histRows > 0); otherwise log an error or handle the failure
path for materialComponent/materialHistoryComponent so you don't count failed
inserts as successes.
- Around line 390-397: The code currently hardcodes materialId=1 and
materialHistoryId=1 when creating MaterialComponent and
MaterialHistoryComponent, which must be replaced by resolved IDs: lookup or
create the correct material ID and its history ID (via the appropriate
baseMapper or repository methods) based on the component/context, assign those
resolved IDs to materialComponent.setMaterialId(...) and
materialHistoryComponent.setMaterialHistoryId(...), then call
baseMapper.createMaterialComponent(...) and
baseMapper.createMaterialHistoryComponent(...); keep using component.getId() for
componentId. Ensure you handle the case where the material or history does not
exist by creating it and using the returned/generated ID.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1a1adbb0-582d-49e1-b038-286804969dcf
📒 Files selected for processing (3)
app/src/main/java/com/tinyengine/it/TinyEngineApplication.javabase/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.javabase/src/main/java/com/tinyengine/it/mcp/utils/UrlValidateUtil.java
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/java/com/tinyengine/it/TinyEngineApplication.java
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java`:
- Around line 67-73: readFileFromRepo currently calls
urlValidateUtil.validateFinalUrl(url) and then fetchBytes(url), causing two
separate DNS resolutions and a DNS-rebind risk; change the flow so
readFileFromRepo performs DNS resolution once (e.g., use
InetAddress.getAllByName or equivalent), run urlValidateUtil.isBlockedAddress()
(or the validation logic) against the resolved IP(s), then open the
HttpURLConnection bound to that validated IP (or connect via the resolved
InetSocketAddress and set the original Host header) so fetchBytes no longer
triggers a second resolution—update or replace fetchBytes to accept a resolved
address/socket or accept a prevalidated URL/address and use the same resolved
endpoint used for validation to establish the HTTP connection.
- Around line 440-443: The update branch currently increments updateNum
unconditionally after calling baseMapper.updateComponentById(component), which
can report a successful update even when 0 rows were affected; change the logic
in the update path that uses
component.setId(...)/component.setLastUpdatedTime(...) and
baseMapper.updateComponentById(component) to capture the returned
affectedRowCount, only increment updateNum when affectedRowCount == 1, and throw
or handle an error (same behavior as the insert path) when the returned count is
not 1 so the import summary remains accurate.
- Around line 131-136: The lookup currently only sets the display name on the
temporary ComponentLibrary (variable library) before calling
componentLibraryMapper.queryComponentLibraryByCondition(library), which can
match the wrong row if names collide; update the probe to also set the package
identifier on that object (e.g., call
library.setPackageName(componentLibrary.getPackageName()) or the parsed package
key property) so the mapper matches by packageName plus name (while leaving
version overwrite behavior unchanged).
In `@base/src/main/java/com/tinyengine/it/mcp/utils/UrlValidateUtil.java`:
- Around line 53-55: In UrlValidateUtil (where the validation checks use the
matched variable and throw new ServiceException("400", ...)), stop returning the
full allowedHosts list to callers: replace the exception message with a generic
denial like "Host not allowed" while logging the detailed information (host and
allowedHosts) to server logs via the class logger (e.g., LOGGER.warn or
LOGGER.debug) before throwing; ensure you still throw ServiceException("400",
"Host not allowed") and do not include allowedHosts in the exception text.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0e3976b9-5082-4a4c-91c5-14e99fa5a98e
📒 Files selected for processing (3)
app/src/main/resources/application-dev.ymlbase/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.javabase/src/main/java/com/tinyengine/it/mcp/utils/UrlValidateUtil.java
Comment on lines
+131
to
+136
| ComponentLibrary library = new ComponentLibrary(); | ||
| library.setName(componentLibrary.getName()); | ||
| // 默认覆盖更新 其他业务需求多版本组件可放开 library.setVersion(componentLibrary.getVersion()); | ||
| // 查询是否存在组件库 | ||
| List<ComponentLibrary> componentLibraryList = componentLibraryMapper.queryComponentLibraryByCondition( | ||
| library); |
There was a problem hiding this comment.
Look up existing libraries by packageName, not just display name.
This probe only sets name, but the mapper can also match on packageName. If two libraries share the same visible name, this update path can hit the wrong row. Reuse the parsed package key here, even if version-overwrite stays intentionally loose.
🎯 Narrow the lookup key
ComponentLibrary library = new ComponentLibrary();
library.setName(componentLibrary.getName());
+ library.setPackageName(componentLibrary.getPackageName());
// 默认覆盖更新 其他业务需求多版本组件可放开 library.setVersion(componentLibrary.getVersion());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java`
around lines 131 - 136, The lookup currently only sets the display name on the
temporary ComponentLibrary (variable library) before calling
componentLibraryMapper.queryComponentLibraryByCondition(library), which can
match the wrong row if names collide; update the probe to also set the package
identifier on that object (e.g., call
library.setPackageName(componentLibrary.getPackageName()) or the parsed package
key property) so the mapper matches by packageName plus name (while leaving
version overwrite behavior unchanged).
Comment on lines
+53
to
+55
| if (!matched) { | ||
| throw new ServiceException("400", | ||
| "Host not allowed: " + host + ". Allowed hosts: " + allowedHosts); |
There was a problem hiding this comment.
Do not return the full allowlist to callers.
This 400 leaks your outbound security policy and makes host probing easier. Keep the allowlist detail in server logs only, and return a generic denial here.
🔒 Tighten the client-facing error
if (!matched) {
- throw new ServiceException("400",
- "Host not allowed: " + host + ". Allowed hosts: " + allowedHosts);
+ throw new ServiceException("400", "Host not allowed");
}📝 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
| if (!matched) { | |
| throw new ServiceException("400", | |
| "Host not allowed: " + host + ". Allowed hosts: " + allowedHosts); | |
| if (!matched) { | |
| throw new ServiceException("400", "Host not allowed"); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@base/src/main/java/com/tinyengine/it/mcp/utils/UrlValidateUtil.java` around
lines 53 - 55, In UrlValidateUtil (where the validation checks use the matched
variable and throw new ServiceException("400", ...)), stop returning the full
allowedHosts list to callers: replace the exception message with a generic
denial like "Host not allowed" while logging the detailed information (host and
allowedHosts) to server logs via the class logger (e.g., LOGGER.warn or
LOGGER.debug) before throwing; ensure you still throw ServiceException("400",
"Host not allowed") and do not include allowedHosts in the exception text.
chilingling
reviewed
Apr 30, 2026
| * @param url The URL of the file to be read. | ||
| * @return A JSON string representing the result of the operation, or an error message if the process fails. | ||
| */ | ||
| @Tool(name="bundle_create",description = "给定的 文件URL 读取内容.") |
Member
There was a problem hiding this comment.
这个 tool 注解最终会得到什么？
参数会解出来嘛？
description 不对。
AI 很难看懂这个工具怎么用
chilingling
reviewed
Apr 30, 2026
| * @param url The URL of the file to be read. | ||
| * @return A JSON string representing the result of the operation, or an error message if the process fails. | ||
| */ | ||
| @Tool(name="bundle_create",description = "给定的 文件URL 读取内容.") |
Member
There was a problem hiding this comment.
当调用 mcp list tool 工具的时候，期望能够看到：
- 工具名称
- description
- 参数、参数描述
- 参数是否必填
当 AI 调用工具的时候，成功应该返回成功，以及相关的描述，错误应该返回错误以及相关的描述。
MCP 协议：https://modelcontextprotocol.io/specification/2025-11-25/server/tools
请检查当前的实现能否满足最低要求。
Contributor
Author
There was a problem hiding this comment.
{
"jsonrpc": "2.0",
"id": 3,
"result": {
"tools": [
{
"name": "bundle_create",
"description": "通过给定的bundle.json文件URL同步物料库",
"inputSchema": {
"type": "object",
"properties": {
"url": {
"type": "string",
"description": "bundle.json文件地址，必须是可以在地址栏请求到的"
}
},
"required": [
"url"
],
"additionalProperties": false
}
}
]
}
}
hexqi
reviewed
Apr 30, 2026
| @SpringBootApplication | ||
| @EnableAspectJAutoProxy | ||
| @MapperScan({"com.tinyengine.it.mapper","com.tinyengine.it.dynamic.dao"}) | ||
| @MapperScan({"com.tinyengine.it.mapper","com.tinyengine.it.dynamic.dao","com.tinyengine.it.mcp"}) |
Collaborator
There was a problem hiding this comment.
-
高风险:
pom.xml引入spring-ai-starter-mcp-server-webmvc:1.1.4，但项目父 POM 仍是 Spring Boot3.1.9，这是官方未支持的版本组合，存在直接编译/启动失败风险。
文件:pom.xml（新增spring-ai.version和 MCP starter 的 hunk）
影响: Spring AI 1.1.x 官方文档写明支持的是 Spring Boot3.4.x和3.5.x。当前 PR 没有同步升级 Boot，也没有引入 Spring AI BOM 做版本对齐，部署时很可能在依赖解析、自动配置或运行期出现不兼容。
依据: https://docs.spring.io/spring-ai/reference/getting-started.html -
高风险: 导入流程实际上没有事务边界，失败后会留下部分已写入的数据。
文件:base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java:58,:111,:321,:352
影响:readFileFromRepo()先更新/创建组件库，再在同类内部调用parseBundle()和bulkCreate()。后两者虽然标了@Transactional，但属于 Spring 自调用，事务代理不会生效；入口方法本身也没有事务。只要中途某次createComponent/createMaterialComponent/createMaterialHistoryComponent失败，就会留下半导入状态，后续需要人工清库修复。 -
中风险: 新导入的组件被硬编码关联到
material_id=1和material_history_id=1，会把任意 bundle 的结果写进默认物料记录。
文件:base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java:386-396
影响: 这会把工具的行为绑定到固定主键，既不跟随导入上下文，也不校验目标物料是否存在。只要环境里不是“永远只有默认物料 1”，就会产生错误关联，污染既有数据。
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java (3)
133-138:⚠️ Potential issue | 🟠 Major | ⚡ Quick winScope the library upsert lookup by
packageNametoo.This probe only sets the display name before
queryComponentLibraryByCondition(...). If two libraries share the same visible name, this branch can update the wrong row and then attach imported components to the wrong library.🎯 Narrow the lookup key
ComponentLibrary library = new ComponentLibrary(); library.setName(componentLibrary.getName()); + library.setPackageName(componentLibrary.getPackageName()); // 默认覆盖更新 其他业务需求多版本组件可放开 library.setVersion(componentLibrary.getVersion());🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java` around lines 133 - 138, The upsert lookup currently only sets the display name (library.setName(...)) before calling queryComponentLibraryByCondition(ComponentLibrary), which can match the wrong row when names collide; update the probe to also set the package identifier on the lookup object (e.g., call library.setPackageName(componentLibrary.getPackageName())) so queryComponentLibraryByCondition(...) scopes by packageName as well, and ensure the mapper/SQL uses packageName in its WHERE clause so the correct ComponentLibrary row is returned for updates and imports.
67-69:⚠️ Potential issue | 🟠 Major | ⚡ Quick winMark the transaction rollback-only before returning the error string.
Because
readFileFromRepo()catches every exception inside the outer@Transactionalmethod, Spring sees a normal return and can commit whatever library/component rows were written before the failure. If this tool must return a string on failure, mark the transaction rollback-only here; otherwise rethrow after logging. Also dropprintStackTrace()and log the throwable directly.💡 Minimal fix
+import org.springframework.transaction.interceptor.TransactionAspectSupport; ... } catch (Exception e) { - e.printStackTrace(); - log.error("从 URL {} 中读取文件 '{}' 失败: {}", url, "bundle.json", e.getMessage()); + log.error("从 URL {} 中读取文件 '{}' 失败", url, "bundle.json", e); + TransactionAspectSupport.currentTransactionStatus().setRollbackOnly(); return "Error: " + e.getMessage(); }Also applies to: 156-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java` around lines 67 - 69, The readFileFromRepo method currently swallows exceptions inside an `@Transactional` method causing commits despite failures; in the catch blocks call TransactionAspectSupport.currentTransactionStatus().setRollbackOnly() to mark the transaction for rollback (or rethrow the exception if preferred), and replace any printStackTrace() calls with proper logger.error("context message", throwable) to log the throwable. Apply the same change to the other transactional handler mentioned (the block around lines 156-159) so both methods mark rollback-only on error and log exceptions rather than printing stack traces.
81-82: 🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick winReturn one response shape from all branches.
This tool currently returns
"", a serializedFileResult, or plain"Error: ..."text. That gives callers three incompatible payload shapes for the same method, so they have to infer success from the content instead of a stable contract.Also applies to: 124-125, 156-159
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java` around lines 81 - 82, The method in GitFileReaderService returns mixed shapes (empty string, serialized FileResult, or plain "Error: ..." text); make all branches return the same FileResult shape: when path is null/empty return a FileResult instance indicating failure or empty content (e.g., set success=false and an error/message field), on exceptions populate and return a FileResult with the error message, and on success populate and return a FileResult with content/metadata; update all branches referenced (the null/empty branch, the exception branches around lines 124-125 and 156-159) to construct and return FileResult consistently rather than raw strings.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java`:
- Around line 395-415: Resolve and set the component's libraryId before querying
for existing components: retrieve the ComponentLibrary via
componentLibraryMapper (as you already do to compute componentLibraryId and call
component.setLibraryId(componentLibraryId)), then include that libraryId on the
Component parameter used for search
(componentParam.setLibraryId(componentLibraryId)) so that
queryComponentByCondition(componentParam) is scoped to the resolved library;
apply the same change to the second similar block around lines 418-447 where
components are checked/updated.
---
Duplicate comments:
In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java`:
- Around line 133-138: The upsert lookup currently only sets the display name
(library.setName(...)) before calling
queryComponentLibraryByCondition(ComponentLibrary), which can match the wrong
row when names collide; update the probe to also set the package identifier on
the lookup object (e.g., call
library.setPackageName(componentLibrary.getPackageName())) so
queryComponentLibraryByCondition(...) scopes by packageName as well, and ensure
the mapper/SQL uses packageName in its WHERE clause so the correct
ComponentLibrary row is returned for updates and imports.
- Around line 67-69: The readFileFromRepo method currently swallows exceptions
inside an `@Transactional` method causing commits despite failures; in the catch
blocks call
TransactionAspectSupport.currentTransactionStatus().setRollbackOnly() to mark
the transaction for rollback (or rethrow the exception if preferred), and
replace any printStackTrace() calls with proper logger.error("context message",
throwable) to log the throwable. Apply the same change to the other
transactional handler mentioned (the block around lines 156-159) so both methods
mark rollback-only on error and log exceptions rather than printing stack
traces.
- Around line 81-82: The method in GitFileReaderService returns mixed shapes
(empty string, serialized FileResult, or plain "Error: ..." text); make all
branches return the same FileResult shape: when path is null/empty return a
FileResult instance indicating failure or empty content (e.g., set success=false
and an error/message field), on exceptions populate and return a FileResult with
the error message, and on success populate and return a FileResult with
content/metadata; update all branches referenced (the null/empty branch, the
exception branches around lines 124-125 and 156-159) to construct and return
FileResult consistently rather than raw strings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 9cfcce0d-aad2-4e55-9118-7c74093ba760
📒 Files selected for processing (2)
base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.javabase/src/main/java/com/tinyengine/it/mcp/utils/UrlValidateUtil.java
🚧 Files skipped from review as they are similar to previous changes (1)
- base/src/main/java/com/tinyengine/it/mcp/utils/UrlValidateUtil.java
Comment on lines
+395
to
+415
| Component componentParam = new Component(); | ||
| componentParam.setComponent(component.getComponent()); | ||
| componentParam.setName(component.getName()); | ||
| componentParam.setVersion(component.getVersion()); | ||
| List<Component> queryComponent = baseMapper.queryComponentByCondition(componentParam); | ||
| // 查询组件库id | ||
| String packageName = null; | ||
| if (component.getNpm() != null && component.getNpm().get("package") != null) { | ||
| packageName = String.valueOf(component.getNpm().get("package")); | ||
| } | ||
| if (packageName != null && !packageName.isEmpty()) { | ||
| ComponentLibrary componentLibrary = new ComponentLibrary(); | ||
| componentLibrary.setPackageName(String.valueOf(component.getNpm().get("package"))); | ||
| componentLibrary.setVersion(component.getVersion()); | ||
| List<ComponentLibrary> componentLibraryList = componentLibraryMapper.queryComponentLibraryByCondition( | ||
| componentLibrary); | ||
| Integer componentLibraryId = null; | ||
| if (!componentLibraryList.isEmpty()) { | ||
| componentLibraryId = componentLibraryList.get(0).getId(); | ||
| } | ||
| component.setLibraryId(componentLibraryId); |
There was a problem hiding this comment.
Resolve libraryId before checking whether the component already exists.
The existence check runs before this code figures out which npm package/library the component belongs to. That means two libraries with the same component/name/version can collide, and the update branch will overwrite whichever row queryComponentByCondition(...) returns first.
🔧 Scope the component lookup to the resolved library
- Component componentParam = new Component();
- componentParam.setComponent(component.getComponent());
- componentParam.setName(component.getName());
- componentParam.setVersion(component.getVersion());
- List<Component> queryComponent = baseMapper.queryComponentByCondition(componentParam);
- // 查询组件库id
+ Component componentParam = new Component();
+ componentParam.setComponent(component.getComponent());
+ componentParam.setName(component.getName());
+ componentParam.setVersion(component.getVersion());
+ // 先解析组件库id，再按 libraryId 查询组件
String packageName = null;
if (component.getNpm() != null && component.getNpm().get("package") != null) {
packageName = String.valueOf(component.getNpm().get("package"));
}
+ Integer componentLibraryId = null;
if (packageName != null && !packageName.isEmpty()) {
ComponentLibrary componentLibrary = new ComponentLibrary();
componentLibrary.setPackageName(String.valueOf(component.getNpm().get("package")));
componentLibrary.setVersion(component.getVersion());
List<ComponentLibrary> componentLibraryList = componentLibraryMapper.queryComponentLibraryByCondition(
componentLibrary);
- Integer componentLibraryId = null;
if (!componentLibraryList.isEmpty()) {
componentLibraryId = componentLibraryList.get(0).getId();
}
component.setLibraryId(componentLibraryId);
}
+ componentParam.setLibraryId(componentLibraryId);
+ List<Component> queryComponent = baseMapper.queryComponentByCondition(componentParam);Also applies to: 418-447
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@base/src/main/java/com/tinyengine/it/mcp/tools/GitFileReaderService.java`
around lines 395 - 415, Resolve and set the component's libraryId before
querying for existing components: retrieve the ComponentLibrary via
componentLibraryMapper (as you already do to compute componentLibraryId and call
component.setLibraryId(componentLibraryId)), then include that libraryId on the
Component parameter used for search
(componentParam.setLibraryId(componentLibraryId)) so that
queryComponentByCondition(componentParam) is scoped to the resolved library;
apply the same change to the second similar block around lines 418-447 where
components are checked/updated.
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
3 participants
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.
feat: bundle.json mcp server
Summary by CodeRabbit
New Features
Chores