Conversation
|
@codecov-ai-reviewer review |
|
Important Review skippedToo many files! This PR contains 128 files, which is 78 over the limit of 50. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (128)
You can disable this status message by setting the Use the checkbox below for a quick retry:
Walkthrough新增 HugeGraph MCP 服务及受控写入链路，扩展 HugeGraph LLM 薄 API，修复 Python 客户端行为，并加入 CI、集成测试、workspace 配置、文档和 Agent skills。 ChangesHugeGraph MCP 服务
HugeGraph LLM 薄 API
Python 客户端与项目基础设施
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant MCPServer
participant ManageGraphData
participant ConfirmationStore
participant HugeGraph
Client->>MCPServer: dry_run graph change
MCPServer->>ManageGraphData: validate and preview
ManageGraphData->>HugeGraph: read schema and match targets
HugeGraph-->>ManageGraphData: schema and match counts
ManageGraphData-->>Client: plan_hash, nonce, expires_at
Client->>MCPServer: confirm plan
MCPServer->>ConfirmationStore: consume nonce
ConfirmationStore-->>MCPServer: accepted or already used
MCPServer->>HugeGraph: execute validated writes
HugeGraph-->>MCPServer: write result and post-read state
MCPServer-->>Client: unified response envelope
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ 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. Comment |
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
Comment on lines
+43
to
+92
| runs-on: ubuntu-latest | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| python-version: ["3.10", "3.11", "3.12"] | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| persist-credentials: false | ||
|
|
||
| - name: Set up Python ${{ matrix.python-version }} | ||
| uses: actions/setup-python@v5 | ||
| with: | ||
| python-version: ${{ matrix.python-version }} | ||
|
|
||
| - name: Install uv | ||
| run: | | ||
| curl -LsSf https://astral.sh/uv/install.sh | sh | ||
| echo "$HOME/.cargo/bin" >> $GITHUB_PATH | ||
|
|
||
| - name: Cache dependencies | ||
| uses: actions/cache@v4 | ||
| with: | ||
| path: | | ||
| ~/.cache/uv | ||
| key: ${{ runner.os }}-mcp-uv-${{ matrix.python-version }}-${{ hashFiles('**/pyproject.toml', 'uv.lock') }} | ||
| restore-keys: | | ||
| ${{ runner.os }}-mcp-uv-${{ matrix.python-version }}- | ||
|
|
||
| - name: Install MCP dependencies | ||
| run: | | ||
| uv sync --extra mcp --extra dev | ||
|
|
||
| - name: Check MCP formatting | ||
| working-directory: hugegraph-mcp | ||
| run: | | ||
| uv run ruff format --check hugegraph_mcp tests | ||
|
|
||
| - name: Lint MCP | ||
| working-directory: hugegraph-mcp | ||
| run: | | ||
| uv run ruff check hugegraph_mcp tests | ||
|
|
||
| - name: Run MCP tests | ||
| working-directory: hugegraph-mcp | ||
| run: | | ||
| uv run pytest -m "not live and not integration and not llm" | ||
|
|
||
| real-hugegraph-write-path: |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces the hugegraph-mcp package, implementing a Model Context Protocol (MCP) server for HugeGraph with standardized envelopes, Gremlin safety policies, capability guards, and a secure write safety chain. It also adds a thin API router to hugegraph-llm and updates hugegraph-python-client for graphspace compatibility and logging robustness. The review feedback identifies several critical issues: gremlin_policy.py should include common TinkerPop tokens in _ALLOWED_ARG_TOKENS to avoid false positives; dry_run_graph_change_plan must restrict planned_operations to preceding operations to prevent out-of-order reference bugs; edge endpoint mapping needs to handle backend IDs correctly when primary keys are missing; and log.py should guard os.makedirs against empty directory paths to prevent relative path failures.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 17
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🟡 Minor comments (11)
hugegraph-python-client/src/tests/api/test_auth_routing.py-180-181 (1)
180-181: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win这里最好按 JSON 结构断言，不要依赖序列化字符串格式。
当前断言对空格和编码细节敏感，后续只要 JSON 序列化方式变化，测试就会误报失败，即使请求载荷仍然正确。直接
json.loads(request["data"])后校验字段会更稳。建议修复
+ import json + request = sess.requests[-1] assert request["path"] == "schema/edgelabels" assert request["method"] == "POST" - assert '"parent_label": "knows"' in request["data"] - assert '"edgelabel_type": "SUB"' in request["data"] + payload = json.loads(request["data"]) + assert payload["parent_label"] == "knows" + assert payload["edgelabel_type"] == "SUB"🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-python-client/src/tests/api/test_auth_routing.py` around lines 180 - 181, The assertions in test_auth_routing should not depend on the exact serialized JSON string in request["data"]; instead, parse the payload with json.loads in the relevant test and assert on the resulting fields. Update the checks around the request payload in the test to verify parent_label and edgelabel_type through the parsed object so the test stays stable even if JSON formatting changes.hugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.py-233-240 (1)
233-240: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win把其它非
dict的 schema 返回值也补进来。Line 77 现在拦的是“所有非
dict”载荷，但这里实际上只锁住了None。像[]或"oops"这类返回值同样会走这个分支，补一个列表或字符串用例更能覆盖这次改动。As per coding guidelines, "Any code change inhugegraph-llmmust add or update tests that exercise the changed behavior, regression risk, or failure path."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.py` around lines 233 - 240, The current test only covers the None failure path in test_run_with_none_schema, but SchemaManager.run now rejects any non-dict schema payload. Update this test module to add coverage for other invalid return values from mock_schema.getSchema, such as a list or string, and assert the same ValueError from SchemaManager.run while reusing the existing schema_manager and getSchema symbols to keep the regression coverage aligned with the new validation behavior.Source: Coding guidelines
hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py-183-183 (1)
183-183: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win给
create_app()补一个路由挂载回归测试。当前测试只把
thin_router直接挂到临时FastAPI()上，没有覆盖 Line 183 这次新增的真实挂载点；如果应用里漏挂、挂到错误路由层，或认证依赖没有一起生效，这组测试仍然会通过。As per coding guidelines, "Any code change inhugegraph-llmmust add or update tests that exercise the changed behavior, regression risk, or failure path."🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py` at line 183, The current tests do not cover the real router mount path added through create_app(), so they can miss a bad or missing api_auth.include_router(thin_router) integration. Add or update a regression test that instantiates create_app() and verifies the thin_router endpoints are actually reachable through the app and still protected/affected by the auth dependency setup, using create_app() and api_auth.include_router as the key symbols to target.Source: Coding guidelines
hugegraph-mcp/hugegraph_mcp/gremlin_tools.py-169-171 (1)
169-171: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win不要把所有
ValueError都当成 Gremlin 语法错误。这里的
try同时包住了客户端构造和exec()。如果配置或客户端初始化抛出ValueError，现在会被错误包装成QUERY_SYNTAX_ERROR，排障方向会被带偏。🔧 建议修改
except ValueError as e: + if actual_client is None: + return { + "success": False, + "error_type": "unknown_error", + "message": f"Failed to initialize Gremlin client: {e!s}", + "suggestions": [ + "Check HugeGraph connection settings", + "Verify HUGEGRAPH_URL / graph / graphspace configuration", + ], + "duration_ms": (time.perf_counter() - start) * 1000.0, + "operation_type": operation_type, + } return { "success": False, "error_type": "query_syntax_error", "message": f"Gremlin query syntax error: {e!s}",Also applies to: 289-301
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/gremlin_tools.py` around lines 169 - 171, The try block in Gremlin execution is catching too much in `query()`/the related helper around `actual_client = client() if callable(client) else client` and `exec()`, so `ValueError` from client/config initialization is being misreported as `QUERY_SYNTAX_ERROR`. Narrow the exception handling so only `exec()` failures are classified as query syntax errors, and let client creation/config errors propagate or be mapped to a separate error type; apply the same split in the duplicated logic referenced by the same Gremlin query path.hugegraph-mcp/hugegraph_mcp/gremlin_tools.py-244-256 (1)
244-256: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win500 分支需要回退读取非 JSON 响应体。
response.json()失败时这里直接丢掉了正文。服务端把NoIndexException放在纯文本响应体里时，会退化成通用SERVER_ERROR，走不到专门的NO_INDEX提示路径。🔧 建议修改
try: if hasattr(e, "response") and e.response is not None: - error_json = e.response.json() - detail_message = error_json.get("exception") or "" - if not detail_message: - detail_message = ( - error_json.get("message") - or error_json.get("detail") - or error_json.get("error") - or str(error_json) - ) + try: + error_json = e.response.json() + except ValueError: + error_json = None + if isinstance(error_json, dict): + detail_message = error_json.get("exception") or "" + if not detail_message: + detail_message = ( + error_json.get("message") + or error_json.get("detail") + or error_json.get("error") + or str(error_json) + ) + if not detail_message: + detail_message = getattr(e.response, "text", "") or "" except Exception: passAlso applies to: 258-264
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/gremlin_tools.py` around lines 244 - 256, The 500-handling block in the exception parsing logic currently drops the response body when `response.json()` fails, so plain-text errors like `NoIndexException` never reach the specific `NO_INDEX` path. Update the error extraction in the `gremlin_tools` exception handling around `error_json`/`detail_message` to fall back to reading `e.response.text` (or equivalent raw body) inside the `except` path, then use that text when no JSON fields are available so `SERVER_ERROR` is not returned unnecessarily.hugegraph-mcp/tests/test_plan_hash.py-26-95 (1)
26-95: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win这些环境变量测试缺少缓存失效。
这里的断言依赖
monkeypatch.setenv()立刻影响build_plan_context()/verify_plan_hash()，但当前层说明里MCPConfig.from_env()是带缓存的。这样第二次调用很可能仍然复用旧配置，导致测试结果依赖缓存状态或执行顺序，而不是 plan-hash 逻辑本身。建议在每次改环境后显式清缓存，或直接 monkeypatchMCPConfig.from_env的返回值。Also applies to: 183-359
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/tests/test_plan_hash.py` around lines 26 - 95, The plan-hash environment-variable tests are currently relying on MCPConfig.from_env() to re-read changed env vars, but that method is cached, so the second call can reuse stale config. Update the tests around build_plan_context() and verify_plan_hash() to explicitly clear the cache after each monkeypatch.setenv() change, or mock MCPConfig.from_env() so each assertion uses a fresh config. Keep the cache reset/mocking scoped to each test so the hash assertions validate plan-hash behavior only, not prior test state.hugegraph-mcp/hugegraph_mcp/tools/generate_gremlin.py-122-127 (1)
122-127: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win成功执行后的 warnings 也要继续透传。
execute_gremlin_read()成功时，这里只保留了data/meta，但没有把执行阶段的warnings带到最终响应里。这样调用方通过generate_gremlin(execute=True)会丢掉读路径原本要暴露的代价/兼容性告警，和统一 envelope 契约不一致。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/tools/generate_gremlin.py` around lines 122 - 127, `execute_gremlin_read()` 成功分支在组装 `data` 时丢失了执行阶段的 warnings，导致 `generate_gremlin(execute=True)` 的最终响应不完整。请在该成功分支里继续把 `execution_result` 中的 `warnings` 透传到返回的 envelope，和 `data/meta` 一起保留；重点检查 `generate_gremlin()` 里处理 `execution_result` 的逻辑，并确保最终通过 `envelope_ok` 返回时包含这些告警信息。hugegraph-mcp/hugegraph_mcp/tools/extract_graph_data.py-125-145 (1)
125-145: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win解包
graph_data时不要丢掉外层元数据。当 AI 返回
{ "graph_data": {...}, "warnings": [...], "schema_warnings": [...] }这类结构时，这里先把parsed替换成parsed["graph_data"]，后面再读取warnings/raw_summary/schema_warnings就只会看内层对象，外层元数据会被静默吞掉。这样extract_graph_data()返回的 envelope 会比上游响应少信息。可参考的修复方式
def _extract_graph_data(data: Any) -> dict[str, Any] | None: parsed = _parse_json_if_needed(data) + outer = parsed if isinstance(parsed, dict) else None if isinstance(parsed, dict) and "graph_data" in parsed: parsed = _parse_json_if_needed(parsed.get("graph_data")) if not isinstance(parsed, dict): return None @@ return { "vertices": vertices, "edges": edges, - "warnings": parsed.get("warnings", []), - "raw": parsed.get("raw"), - "raw_summary": parsed.get("raw_summary"), - "schema_warnings": parsed.get("schema_warnings", []), + "warnings": parsed.get("warnings", outer.get("warnings", [])) if outer else parsed.get("warnings", []), + "raw": parsed.get("raw", outer.get("raw")) if outer else parsed.get("raw"), + "raw_summary": parsed.get("raw_summary", outer.get("raw_summary")) if outer else parsed.get("raw_summary"), + "schema_warnings": parsed.get("schema_warnings", outer.get("schema_warnings", [])) if outer else parsed.get("schema_warnings", []), }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/tools/extract_graph_data.py` around lines 125 - 145, _extract_graph_data currently replaces parsed with parsed["graph_data"], which drops outer metadata like warnings and schema_warnings from the AI response. Update _extract_graph_data in extract_graph_data.py so it unwraps graph_data only for vertices/edges validation while preserving the original envelope, then merge or prefer outer-level warnings, raw_summary, raw, and schema_warnings when building the return dict. Keep the checks around _parse_json_if_needed, vertices, and edges, but ensure the final envelope includes metadata from both the outer object and the inner graph_data payload instead of silently discarding the outer fields.hugegraph-mcp/hugegraph_mcp/tools/refresh_vid_embeddings.py-79-87 (1)
79-87: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win字符串结果解析过于依赖固定语序。
当前正则只覆盖
add 3/removed 2这类“动词在前”的格式。上游一旦返回3 added、added: 3或removed=2，这里就会静默回落到0，对外暴露错误统计。建议同时兼容“数字在前”和常见分隔符格式，并补一个对应回归用例。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/tools/refresh_vid_embeddings.py` around lines 79 - 87, The parsing in _parse_numbers_from_string is too dependent on a single word order, so update it to recognize both “verb before number” and “number before verb” formats, plus common separators like colon or equals for added/removed values. Adjust the regex logic in _parse_numbers_from_string so inputs like “3 added”, “added: 3”, and “removed=2” are handled instead of falling back to zero, and add a regression test covering these variants.hugegraph-mcp/hugegraph_mcp/tools/inspect_graph.py-60-89 (1)
60-89: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win不要把已知的索引非就绪状态折叠成
unknown。这里的辅助函数只回答“是否 ready”，所以
vid_embedding_status现在只能是"available"或"unknown"。如果上游明确返回"building"、"failed"、"disabled"之类的状态，结果会被静默改写成"unknown"，把真实状态丢掉了。对于一个状态巡检工具，这会直接误导调用方。建议把这里改成“解析并透传状态字符串”，而不是布尔判断。Also applies to: 151-153
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/tools/inspect_graph.py` around lines 60 - 89, The state handling in _has_graph_index_info and _is_ready_index_value is too lossy: it only answers “ready” and collapses non-ready index states into unknown. Update the logic in inspect_graph.py so graph_index_info parsing preserves and returns the actual status strings from fields like vid_embedding_status, vid_index_status, and embedding_index_status instead of converting them through readiness checks; keep the ready check separate from status normalization so values such as building, failed, and disabled are passed through unchanged..github/workflows/hugegraph-mcp.yml-20-22 (1)
20-22: 🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win显式收紧
GITHUB_TOKEN权限。这个工作流只需要读取仓库；当前未声明
permissions，会继承默认令牌权限。建议在工作流级别加上contents: read，避免暴露不必要的写权限。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/hugegraph-mcp.yml around lines 20 - 22, The HugeGraph-MCP CI workflow is missing an explicit token permission scope, so add a workflow-level permissions block to restrict the default GITHUB_TOKEN to read-only repository access. Update the workflow near the top-level configuration so the permissions for contents are set to read, keeping the rest of the job definitions unchanged.Source: Linters/SAST tools
🧹 Nitpick comments (5)
hugegraph-mcp/README.md (1)
92-92: 📐 Maintainability & Code Quality | 🔵 Trivial旧工具引用需澄清来源。
第 92 行提到
query_graph_tool、manage_schema_tool和manage_graph_data_tool不再对外暴露，但前文未说明这些工具是否来自 MCP V0 或内部实现。建议在首次出现时补充说明（如 "V0 遗留工具" 或 "内部工具"），避免新用户困惑。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/README.md` at line 92, The README text currently mentions query_graph_tool, manage_schema_tool, and manage_graph_data_tool as no longer exposed, but it does not clarify their origin. Update the documentation near the first mention of these tool names to explicitly label them as V0 legacy tools or internal tools, and keep the current guidance that new integrations should use the stable tools listed above.hugegraph-mcp/README.zh-CN.md (1)
96-96: 📐 Maintainability & Code Quality | 🔵 Trivial旧工具引用需澄清来源。
与英文版第 92 行一致，此处提到旧工具不再对外暴露，但未说明其来源（V0 遗留或内部实现）。建议补充说明，如"V0 遗留工具"或"内部工具"。
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/README.zh-CN.md` at line 96, The README note about the deprecated tools is ambiguous about where they came from. Update the affected Chinese documentation text to explicitly label the old tools using a clear source term such as “V0 遗留工具” or “内部工具,” matching the intent of the corresponding English wording and making the origin clear to readers.skills/hugegraph-query-analyst/SKILL.md (1)
13-16: 📐 Maintainability & Code Quality | 🔵 Trivial生成工具的路由条目可优化。
第 13 行与第 16 行均使用
generate_gremlin_tool(query, execute=false)，目标分别为"自然语言生成 Gremlin"和"仅生成不执行"。建议合并为单一条目并说明execute=false为默认行为，与 README 中"默认只生成，不执行"的表述保持一致。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/hugegraph-query-analyst/SKILL.md` around lines 13 - 16, The routing table in SKILL.md repeats generate_gremlin_tool(query, execute=false) for both “generate Gremlin from natural language” and “generate only, without execution”; consolidate these into one entry and make it explicit that execute=false is the default behavior. Update the nearby “Answer a natural-language graph question” row only if needed so the route remains generate_gremlin_tool(query, execute=false) followed by execute_gremlin_read_tool(gremlin_query), keeping the wording aligned with the README’s “default generate only, do not execute” guidance.skills/hugegraph-operator/SKILL.md (1)
12-15: 📐 Maintainability & Code Quality | 🔵 Trivial工具路由表存在冗余条目。
第 12、14、15 行均映射到相同的工具调用
inspect_graph_tool(include_raw_schema=false)，只是目标描述不同。虽然这有助于 AI Agent 理解同一工具的多场景用途，但建议合并为单一条目并列举多个目标，或明确各场景的差异参数（如第 19 行已展示generate_gremlin_tool的可选参数用法）。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@skills/hugegraph-operator/SKILL.md` around lines 12 - 15, There is redundant routing in the SKILL.md tool table because multiple entries point to the same inspect_graph_tool(include_raw_schema=false) call with only the description changed. Consolidate the repeated rows into a single inspect_graph_tool entry that lists all applicable use cases, or split them only if different parameters or behaviors are introduced; keep the distinct inspect_graph_tool and execute_gremlin_read_tool symbols easy to find and align the table with the parameter-style examples used elsewhere.hugegraph-mcp/tests/test_v1_stable_tools.py (1)
17-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win把
os.makedirs的恢复也纳入断言。当前只校验了
RotatingFileHandler被恢复，但server.py在导入期同样全局替换了os.makedirs。如果将来漏掉finally里的恢复，这条测试抓不住，影响会扩散到进程内其他目录创建。🧪 建议补断言
+import os import warnings @@ def test_server_import_restores_logging_globals(): assert logging.handlers.RotatingFileHandler is server._OriginalRotatingFileHandler + assert os.makedirs is server._original_makedirs assert logging.root.manager.disable < logging.CRITICALAlso applies to: 73-75
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/tests/test_v1_stable_tools.py` around lines 17 - 19, The test setup currently only asserts that RotatingFileHandler is restored, but server.py also monkey-patches os.makedirs at import time, so the cleanup coverage is incomplete. Update the relevant test in test_v1_stable_tools.py to also verify that os.makedirs is restored in the finally/teardown path, using the existing import-time patching flow around server.py as the locator for the behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/hugegraph-mcp.yml:
- Around line 27-39: The workflow trigger is too broad because
hugegraph-python-client changes currently run only the hugegraph-mcp checks,
which can miss client regressions. Update the hugegraph-mcp workflow’s paths
filter or, if keeping hugegraph-python-client/** in the trigger, extend the job
steps to run the relevant hugegraph-python-client tests plus any hugegraph-llm
caller-compatibility tests. Use the workflow definition and its
paths/pull_request conditions as the place to adjust this cross-module coverage.
In `@hugegraph-llm/src/hugegraph_llm/api/thin_api.py`:
- Around line 87-94: The Thin API error handling in thin_api.py is still logging
the raw exception text and traceback, which can leak sensitive downstream
details. Update the exception path in the flow execution handler to generate a
request_id once, include that same request_id in the returned envelope, and
change the log call in the thin API flow so it records only the request_id and
exception type rather than exc or full traceback content. Keep the user-facing
envelope unchanged except for reusing the request_id for correlation.
In `@hugegraph-mcp/hugegraph_mcp/config.py`:
- Around line 102-107: The readonly parsing in Config is treating invalid input
as false, which can accidentally disable protection. Update the configuration
parsing around _parse_bool and the Config fields so HUGEGRAPH_MCP_READONLY falls
back to its safe default when the value is malformed, while still allowing
explicit false values; apply the same pattern where needed in the related config
parsing paths (including the other boolean/env parsing in Config). If the input
is invalid, log a warning and preserve the default instead of switching modes
silently.
- Around line 43-58: The cached MCPConfig instance is mutable, so callers can
accidentally turn readonly/admin flags into shared process-wide state. Make
MCPConfig immutable (for example by freezing the dataclass) or change
MCPConfig.from_env() to return a fresh copy on each call/cache hit instead of
the stored object. Update the config-loading path and any related helpers that
read or cache MCPConfig so mutations from one caller cannot affect later reads.
In `@hugegraph-mcp/hugegraph_mcp/envelope.py`:
- Around line 56-81: The meta-building logic in build_meta currently lets
extra_meta and kwargs overwrite reserved envelope fields like request_id, graph,
graphspace, and readonly. Update build_meta so extension fields are merged first
and then the reserved keys are written last, or explicitly filter those reserved
names before update() to keep the envelope context authoritative.
In `@hugegraph-mcp/hugegraph_mcp/hugegraph_ai_client.py`:
- Around line 47-67: The URL construction in hugegraph_ai_client.py currently
lets callers pass absolute http(s) URLs through path, which can bypass
cfg.ai_url and still send Basic Auth from the request flow. Update the
URL-building logic around _build_url and the request call in the AI client so
only relative paths are accepted, rejecting or sanitizing any absolute URL input
before requests.request is invoked. Keep all outbound targets anchored to
cfg.ai_url, and apply the same validation/fix to the other affected call site
noted in the review.
In `@hugegraph-mcp/hugegraph_mcp/plan_hash.py`:
- Around line 55-65: The current plan hash in compute_plan_hash is an unkeyed
digest over PlanContext, so callers can recompute it after changing payload
fields and bypass the dry_run-only constraint. Update compute_plan_hash to use a
server-side secret (for example an HMAC or server-held plan token) so
submitted_hash cannot be forged from public context alone, and adjust the
confirm flow that uses the returned hash accordingly. In the confirm-related
code around the plan hash response, stop echoing any server-derived signature
back to the caller once the secret-backed scheme is in place.
In `@hugegraph-mcp/hugegraph_mcp/schema_tools.py`:
- Around line 89-125: `design_schema()` is dropping important caller context and
only returning step counters, so update the function to preserve and return the
input metadata it already accepts (`thought`, `is_revision`, `revision_of`)
alongside the existing fields. Also expand the return payload to include the
actual schema-design guidance content that the tool is meant to provide, so
callers can render and chain the step output. Use the `design_schema` function
and its current return dict as the place to fix this.
In `@hugegraph-mcp/hugegraph_mcp/tools/graph_data_execute.py`:
- Around line 189-198: The dry-run path in graph data execution currently
validates only against the live graph, so duplicate create_vertex requests in
the same batch can pass preview and fail later with partial writes. Update the
create_vertex handling in the graph_data_execute flow to track planned
duplicates by (label, id) and by (label, primary_key values) across the batch,
and reject them during dry-run with an error before previewing. Add a regression
test covering duplicate vertex IDs and duplicate primary-key values within the
same batch to verify the new validation.
- Around line 174-182: dry-run 统计 planned_count 时把后续才创建的顶点也算进来了，导致与顺序执行不一致；请在
graph_data_execute.py 的执行流程中，针对 create_edge 调用 _append_edge_endpoint_counts
时只传入当前操作之前的计划操作，或先显式重排并为 create_edge / _append_edge_endpoint_counts 增加测试，确保
create_vertex(A) -> create_edge(A,B) -> create_vertex(B) 这种顺序在 dry-run
和实际执行的契约一致。
In `@hugegraph-mcp/hugegraph_mcp/tools/ingest_graph_data.py`:
- Around line 650-655: The edge validation in `_validate_graph_data` is
incorrectly treating entries with both endpoints missing as valid by continuing
when `source is None and target is None`. Update this check so edges in that
state are rejected and an error is appended, alongside the existing
`source`/`target` missing cases, to prevent payloads with only labels from
passing `dry_run` and reaching `_prepare_graph_import_data()`/`/graph-import`
without real endpoints.
- Around line 204-210: `_vertex_sort_key()` is only using `primary_keys[0]` for
vertices with composite keys, which makes ordering depend on input sequence and
can change `plan_hash`; update the identity calculation to use all fields in
`schema_primary_keys` for the label (for example, derive a stable composite sort
key from every primary key value in `ingest_graph_data.py`). Keep the existing
fallback for non-keyed properties, but ensure the vertex identity used by
`sorted()` is deterministic for multi-field primary keys so dry-run/confirm
outputs stay stable.
In `@hugegraph-mcp/hugegraph_mcp/tools/manage_graph_data.py`:
- Around line 264-269: The graph write result handling is marking all
non-success outcomes as retryable via the `envelope_err` path, which can cause
unsafe replays after partial mutations. Update the retryable decision in
`manage_graph_data` so `partial` is treated as non-retryable by default, and
only return `retryable=True` when the executor can prove no side effects
occurred or explicit idempotency guarantees exist. Apply the same fix in the
other duplicated error-return block that uses `normalized`/`warnings` and
`retryable=bool(normalized.get("retryable"))`.
In `@hugegraph-mcp/hugegraph_mcp/tools/manage_schema.py`:
- Around line 297-303: `create_vertex_label` 的引用校验只覆盖了 `properties`，导致
`primary_keys` 可能引用未定义字段并仍被判定为有效；请在 `manage_schema.py` 的相关校验流程中补充对
`primary_keys` 的检查。修改 `_validate_property_references` 的调用/实现，确保 `primary_keys`
也必须引用已存在或同批创建的 property key，并在 `properties` 被提供时额外验证 `primary_keys` 是其子集，保持
`validate_schema_operation` 的 `valid` 判定与 `dry_run` 结果一致。
In `@hugegraph-mcp/tests/test_extract_graph_data.py`:
- Around line 63-80: This test is order-dependent because MCPConfig.from_env()
caches environment-derived config, so changing HUGEGRAPH_GRAPH_PATH alone may
not take effect. Update test_extract_graph_data_uses_graph_schema_by_default to
clear the MCPConfig cache/instance state before calling extract_graph_data(),
ensuring the test uses the freshly set environment and avoids stale
configuration from earlier tests.
In `@hugegraph-mcp/tests/test_manage_graph_data.py`:
- Around line 175-1652: Add regression coverage in manage_graph_data tests for
dry_run_graph_change_plan and execute_graph_change_plan around same-batch
ordering and duplicate identity handling. Specifically, extend the existing
create_vertex/create_edge scenarios to assert that a create_edge cannot
reference a vertex that is only created later in the same plan, and that
duplicate create_vertex ids or primary-key matches in one batch are rejected
during dry-run. Use the existing helpers like _create_edge_query,
_create_vertex_query, graph_data_to_change_plan, and dry_run_graph_change_plan
to keep the new cases aligned with current behavior.
In `@hugegraph-python-client/src/pyhugegraph/utils/log.py`:
- Around line 120-132: The log setup in the logging utility is treating a bare
filename like a directory-creation failure, which disables file logging for
valid inputs such as client.log. Update the directory creation logic in the log
initialization flow to only call os.makedirs when the directory part from
os.path.dirname(log_filename) is non-empty, and keep the existing
stdout_logging/null-handler fallback only for real OSError cases. Use the
log_instance setup block and the log_filename handling in the same function to
locate the fix.
---
Minor comments:
In @.github/workflows/hugegraph-mcp.yml:
- Around line 20-22: The HugeGraph-MCP CI workflow is missing an explicit token
permission scope, so add a workflow-level permissions block to restrict the
default GITHUB_TOKEN to read-only repository access. Update the workflow near
the top-level configuration so the permissions for contents are set to read,
keeping the rest of the job definitions unchanged.
In `@hugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.py`:
- Line 183: The current tests do not cover the real router mount path added
through create_app(), so they can miss a bad or missing
api_auth.include_router(thin_router) integration. Add or update a regression
test that instantiates create_app() and verifies the thin_router endpoints are
actually reachable through the app and still protected/affected by the auth
dependency setup, using create_app() and api_auth.include_router as the key
symbols to target.
In `@hugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.py`:
- Around line 233-240: The current test only covers the None failure path in
test_run_with_none_schema, but SchemaManager.run now rejects any non-dict schema
payload. Update this test module to add coverage for other invalid return values
from mock_schema.getSchema, such as a list or string, and assert the same
ValueError from SchemaManager.run while reusing the existing schema_manager and
getSchema symbols to keep the regression coverage aligned with the new
validation behavior.
In `@hugegraph-mcp/hugegraph_mcp/gremlin_tools.py`:
- Around line 169-171: The try block in Gremlin execution is catching too much
in `query()`/the related helper around `actual_client = client() if
callable(client) else client` and `exec()`, so `ValueError` from client/config
initialization is being misreported as `QUERY_SYNTAX_ERROR`. Narrow the
exception handling so only `exec()` failures are classified as query syntax
errors, and let client creation/config errors propagate or be mapped to a
separate error type; apply the same split in the duplicated logic referenced by
the same Gremlin query path.
- Around line 244-256: The 500-handling block in the exception parsing logic
currently drops the response body when `response.json()` fails, so plain-text
errors like `NoIndexException` never reach the specific `NO_INDEX` path. Update
the error extraction in the `gremlin_tools` exception handling around
`error_json`/`detail_message` to fall back to reading `e.response.text` (or
equivalent raw body) inside the `except` path, then use that text when no JSON
fields are available so `SERVER_ERROR` is not returned unnecessarily.
In `@hugegraph-mcp/hugegraph_mcp/tools/extract_graph_data.py`:
- Around line 125-145: _extract_graph_data currently replaces parsed with
parsed["graph_data"], which drops outer metadata like warnings and
schema_warnings from the AI response. Update _extract_graph_data in
extract_graph_data.py so it unwraps graph_data only for vertices/edges
validation while preserving the original envelope, then merge or prefer
outer-level warnings, raw_summary, raw, and schema_warnings when building the
return dict. Keep the checks around _parse_json_if_needed, vertices, and edges,
but ensure the final envelope includes metadata from both the outer object and
the inner graph_data payload instead of silently discarding the outer fields.
In `@hugegraph-mcp/hugegraph_mcp/tools/generate_gremlin.py`:
- Around line 122-127: `execute_gremlin_read()` 成功分支在组装 `data` 时丢失了执行阶段的
warnings，导致 `generate_gremlin(execute=True)` 的最终响应不完整。请在该成功分支里继续把
`execution_result` 中的 `warnings` 透传到返回的 envelope，和 `data/meta` 一起保留；重点检查
`generate_gremlin()` 里处理 `execution_result` 的逻辑，并确保最终通过 `envelope_ok`
返回时包含这些告警信息。
In `@hugegraph-mcp/hugegraph_mcp/tools/inspect_graph.py`:
- Around line 60-89: The state handling in _has_graph_index_info and
_is_ready_index_value is too lossy: it only answers “ready” and collapses
non-ready index states into unknown. Update the logic in inspect_graph.py so
graph_index_info parsing preserves and returns the actual status strings from
fields like vid_embedding_status, vid_index_status, and embedding_index_status
instead of converting them through readiness checks; keep the ready check
separate from status normalization so values such as building, failed, and
disabled are passed through unchanged.
In `@hugegraph-mcp/hugegraph_mcp/tools/refresh_vid_embeddings.py`:
- Around line 79-87: The parsing in _parse_numbers_from_string is too dependent
on a single word order, so update it to recognize both “verb before number” and
“number before verb” formats, plus common separators like colon or equals for
added/removed values. Adjust the regex logic in _parse_numbers_from_string so
inputs like “3 added”, “added: 3”, and “removed=2” are handled instead of
falling back to zero, and add a regression test covering these variants.
In `@hugegraph-mcp/tests/test_plan_hash.py`:
- Around line 26-95: The plan-hash environment-variable tests are currently
relying on MCPConfig.from_env() to re-read changed env vars, but that method is
cached, so the second call can reuse stale config. Update the tests around
build_plan_context() and verify_plan_hash() to explicitly clear the cache after
each monkeypatch.setenv() change, or mock MCPConfig.from_env() so each assertion
uses a fresh config. Keep the cache reset/mocking scoped to each test so the
hash assertions validate plan-hash behavior only, not prior test state.
In `@hugegraph-python-client/src/tests/api/test_auth_routing.py`:
- Around line 180-181: The assertions in test_auth_routing should not depend on
the exact serialized JSON string in request["data"]; instead, parse the payload
with json.loads in the relevant test and assert on the resulting fields. Update
the checks around the request payload in the test to verify parent_label and
edgelabel_type through the parsed object so the test stays stable even if JSON
formatting changes.
---
Nitpick comments:
In `@hugegraph-mcp/README.md`:
- Line 92: The README text currently mentions query_graph_tool,
manage_schema_tool, and manage_graph_data_tool as no longer exposed, but it does
not clarify their origin. Update the documentation near the first mention of
these tool names to explicitly label them as V0 legacy tools or internal tools,
and keep the current guidance that new integrations should use the stable tools
listed above.
In `@hugegraph-mcp/README.zh-CN.md`:
- Line 96: The README note about the deprecated tools is ambiguous about where
they came from. Update the affected Chinese documentation text to explicitly
label the old tools using a clear source term such as “V0 遗留工具” or “内部工具,”
matching the intent of the corresponding English wording and making the origin
clear to readers.
In `@hugegraph-mcp/tests/test_v1_stable_tools.py`:
- Around line 17-19: The test setup currently only asserts that
RotatingFileHandler is restored, but server.py also monkey-patches os.makedirs
at import time, so the cleanup coverage is incomplete. Update the relevant test
in test_v1_stable_tools.py to also verify that os.makedirs is restored in the
finally/teardown path, using the existing import-time patching flow around
server.py as the locator for the behavior.
In `@skills/hugegraph-operator/SKILL.md`:
- Around line 12-15: There is redundant routing in the SKILL.md tool table
because multiple entries point to the same
inspect_graph_tool(include_raw_schema=false) call with only the description
changed. Consolidate the repeated rows into a single inspect_graph_tool entry
that lists all applicable use cases, or split them only if different parameters
or behaviors are introduced; keep the distinct inspect_graph_tool and
execute_gremlin_read_tool symbols easy to find and align the table with the
parameter-style examples used elsewhere.
In `@skills/hugegraph-query-analyst/SKILL.md`:
- Around line 13-16: The routing table in SKILL.md repeats
generate_gremlin_tool(query, execute=false) for both “generate Gremlin from
natural language” and “generate only, without execution”; consolidate these into
one entry and make it explicit that execute=false is the default behavior.
Update the nearby “Answer a natural-language graph question” row only if needed
so the route remains generate_gremlin_tool(query, execute=false) followed by
execute_gremlin_read_tool(gremlin_query), keeping the wording aligned with the
README’s “default generate only, do not execute” guidance.
🪄 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: 6795b753-a9df-4d83-ad62-ab750ae9bec4
📒 Files selected for processing (83)
.github/workflows/hugegraph-mcp.yml.gitignore.spec/hugegraph-llm/fixed_flow/design.md.spec/hugegraph-llm/fixed_flow/requirements.md.spec/hugegraph-llm/fixed_flow/tasks.md.spec/hugegraph-mcp/graph_mcp/requirements.mdhugegraph-llm/src/hugegraph_llm/api/models/rag_requests.pyhugegraph-llm/src/hugegraph_llm/api/models/rag_response.pyhugegraph-llm/src/hugegraph_llm/api/thin_api.pyhugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.pyhugegraph-llm/src/hugegraph_llm/flows/import_graph_data.pyhugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.pyhugegraph-llm/src/tests/api/test_thin_api.pyhugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.pyhugegraph-mcp/README.mdhugegraph-mcp/README.zh-CN.mdhugegraph-mcp/docs/ingest_graph_data_validation_fix_plan.mdhugegraph-mcp/docs/mcp-v1-prune-prd.mdhugegraph-mcp/hugegraph_mcp/__init__.pyhugegraph-mcp/hugegraph_mcp/config.pyhugegraph-mcp/hugegraph_mcp/envelope.pyhugegraph-mcp/hugegraph_mcp/gremlin_policy.pyhugegraph-mcp/hugegraph_mcp/gremlin_safety.pyhugegraph-mcp/hugegraph_mcp/gremlin_tools.pyhugegraph-mcp/hugegraph_mcp/guard.pyhugegraph-mcp/hugegraph_mcp/hugegraph_ai_client.pyhugegraph-mcp/hugegraph_mcp/hugegraph_client.pyhugegraph-mcp/hugegraph_mcp/plan_hash.pyhugegraph-mcp/hugegraph_mcp/schema_tools.pyhugegraph-mcp/hugegraph_mcp/server.pyhugegraph-mcp/hugegraph_mcp/tools/__init__.pyhugegraph-mcp/hugegraph_mcp/tools/extract_graph_data.pyhugegraph-mcp/hugegraph_mcp/tools/generate_gremlin.pyhugegraph-mcp/hugegraph_mcp/tools/graph_data_execute.pyhugegraph-mcp/hugegraph_mcp/tools/graph_data_gremlin.pyhugegraph-mcp/hugegraph_mcp/tools/graph_data_mapping.pyhugegraph-mcp/hugegraph_mcp/tools/graph_data_validate.pyhugegraph-mcp/hugegraph_mcp/tools/ingest_graph_data.pyhugegraph-mcp/hugegraph_mcp/tools/inspect_graph.pyhugegraph-mcp/hugegraph_mcp/tools/live_schema.pyhugegraph-mcp/hugegraph_mcp/tools/manage_graph_data.pyhugegraph-mcp/hugegraph_mcp/tools/manage_schema.pyhugegraph-mcp/hugegraph_mcp/tools/refresh_vid_embeddings.pyhugegraph-mcp/hugegraph_mcp/tools/schema_utils.pyhugegraph-mcp/pyproject.tomlhugegraph-mcp/tests/integration/test_real_write_path.pyhugegraph-mcp/tests/test_config.pyhugegraph-mcp/tests/test_envelope.pyhugegraph-mcp/tests/test_error_handling.pyhugegraph-mcp/tests/test_execute_gremlin_read.pyhugegraph-mcp/tests/test_execute_gremlin_write.pyhugegraph-mcp/tests/test_extract_graph_data.pyhugegraph-mcp/tests/test_generate_gremlin.pyhugegraph-mcp/tests/test_get_live_schema.pyhugegraph-mcp/tests/test_gremlin_policy.pyhugegraph-mcp/tests/test_gremlin_safety.pyhugegraph-mcp/tests/test_guard.pyhugegraph-mcp/tests/test_hugegraph_ai_client.pyhugegraph-mcp/tests/test_import_graph_data_tool.pyhugegraph-mcp/tests/test_ingest_graph_data.pyhugegraph-mcp/tests/test_inspect_graph.pyhugegraph-mcp/tests/test_manage_graph_data.pyhugegraph-mcp/tests/test_manage_schema.pyhugegraph-mcp/tests/test_plan_hash.pyhugegraph-mcp/tests/test_readonly_mode.pyhugegraph-mcp/tests/test_refresh_vid_embeddings.pyhugegraph-mcp/tests/test_schema_utils.pyhugegraph-mcp/tests/test_v1_stable_tools.pyhugegraph-python-client/src/pyhugegraph/utils/huge_config.pyhugegraph-python-client/src/pyhugegraph/utils/log.pyhugegraph-python-client/src/tests/api/test_auth.pyhugegraph-python-client/src/tests/api/test_auth_routing.pypyproject.tomlskills/hugegraph-data-importer/SKILL.mdskills/hugegraph-data-importer/agents/openai.yamlskills/hugegraph-operator/SKILL.mdskills/hugegraph-operator/agents/openai.yamlskills/hugegraph-query-analyst/SKILL.mdskills/hugegraph-query-analyst/agents/openai.yamlskills/hugegraph-regression-tester/SKILL.mdskills/hugegraph-regression-tester/agents/openai.yamlskills/hugegraph-schema-designer/SKILL.mdskills/hugegraph-schema-designer/agents/openai.yaml
UIengF
force-pushed
the
graph-mcp
branch
2 times, most recently
from
July 11, 2026 05:15
fac98cf to
611eef6
Compare
added 3 commits
July 11, 2026 13:19
Change-Id: Iaec68dbf134040f94b6fd13cd257d66542d1ae9d
Change-Id: I110718665ecc71910b699baef9d06de104dc17e6
Change-Id: Ia85907918f09080d00f1546efe45040aede06ee2
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
hugegraph-mcp/tests/test_extract_graph_data.py (1)
63-80: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
MCPConfig.from_env()缓存导致测试顺序依赖 — 前一轮已标记，仍未修复。Line 64 设置了
HUGEGRAPH_GRAPH_PATH，但未清空from_env()的缓存/单例。若前序测试已初始化配置，本用例可能读到旧值导致断言偶发失败。此问题在前一轮复核中已标记。🛡️ 建议在调用前清空配置缓存
def test_extract_graph_data_uses_graph_schema_by_default(monkeypatch): monkeypatch.setenv("HUGEGRAPH_GRAPH_PATH", "DEFAULT/hugegraph") + MCPConfig.reset_cache() # 或等价的缓存清理方式 graph_data = {"vertices": [], "edges": []}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/tests/test_extract_graph_data.py` around lines 63 - 80, 修复测试中的 MCPConfig.from_env() 缓存导致的顺序依赖。定位 test_extract_graph_data_uses_graph_schema_by_default，在设置 HUGEGRAPH_GRAPH_PATH 后、调用 extract_graph_data_module.extract_graph_data 前清空 MCPConfig 的缓存或单例状态，确保测试始终读取当前环境变量配置；如项目已有统一的缓存重置方式，请复用该方式。
🧹 Nitpick comments (6)
hugegraph-mcp/hugegraph_mcp/hugegraph_ai_client.py (1)
181-183: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
health_check末尾or get(...)为死代码。循环遍历
("/graph-index-info", "/openapi.json")，每次迭代都会执行last_result = result（Line 173）。循环结束后last_result必非None，or get("/openapi.json", cfg=cfg)永远不会执行。移除可避免误导。♻️ 建议清理死代码
if last_result is not None and attempts: last_result["warnings"] = [*last_result.get("warnings", []), *attempts] - return last_result or get("/openapi.json", cfg=cfg) + return last_result🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/hugegraph_ai_client.py` around lines 181 - 183, Remove the dead fallback call `or get("/openapi.json", cfg=cfg)` from the return statement at the end of `health_check`; return `last_result` directly after preserving the existing warnings aggregation.hugegraph-mcp/hugegraph_mcp/tools/extract_graph_data.py (1)
55-81: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
MCPConfig.from_env()在单次调用中被重复获取。
_schema_message（Line 104）、post（内部request）和 Line 81 各自独立调用from_env()。建议在函数入口获取一次cfg，传入_schema_message和post，保证一致性并减少重复开销。♻️ 建议统一获取配置
def extract_graph_data( text: str, schema: dict[str, Any] | str | None = None, example_prompt: str | None = None, ) -> dict[str, Any]: + cfg = MCPConfig.from_env() schema_message = _schema_message(schema, cfg) prompt_message = _example_prompt_message(example_prompt) ai_result = post( "/graph-extract", + cfg=cfg, json={ "text": text, "schema": schema_message, "example_prompt": prompt_message, "language": "zh", }, ) @@ - cfg = MCPConfig.from_env() return envelope_ok( def _schema_message(schema: Any, cfg: MCPConfig | None = None) -> str: if schema is None: - return MCPConfig.from_env().graph + return (cfg or MCPConfig.from_env()).graph🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/tools/extract_graph_data.py` around lines 55 - 81, 在图数据提取函数入口只调用一次 MCPConfig.from_env()，避免同一请求中重复读取配置。将该 cfg 传入 _schema_message 以及 post（及其内部 request），并移除末尾再次调用 MCPConfig.from_env() 的逻辑，确保整次调用使用同一份配置。hugegraph-mcp/hugegraph_mcp/tools/inspect_schema.py (1)
84-89: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
all_summary在include_index_labels=False时仍包含 index labels。当
include_index_labels=False且filter_kind is None时，data["filtered"]["index_labels"]来自all_summary（始终包含 index labels），而data["index_labels"]为空列表。这种不一致可能使 API 消费者困惑。如果是有意设计，建议在文档中注明filtered始终返回完整 schema 视图。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/tools/inspect_schema.py` around lines 84 - 89, 修正 inspect_schema 中 all_summary 与返回结果的不一致：当 include_index_labels=False 时，确保传入 _filter_schema 的摘要不包含 index labels，使 data["filtered"]["index_labels"] 与 data["index_labels"] 保持一致；检查 _build_summary、_filter_schema 及其调用逻辑，并补充相关测试覆盖 filter_kind=None 的场景。hugegraph-mcp/tests/test_v1_stable_tools.py (1)
26-30: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value避免依赖 fastmcp 的私有列举方法
_mcp_list_tools/_list_tools仍然是内部实现细节，后续 fastmcp 改名时这类测试会直接失效。若server.mcp有公开的list_tools/list_tools_mcp，优先改用公共接口。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/tests/test_v1_stable_tools.py` around lines 26 - 30, 更新 _list_mcp_tools，优先调用 server.mcp 提供的公开 list_tools 或 list_tools_mcp 接口，并仅在公共接口不可用时保留兼容处理；移除对 _mcp_list_tools 和 _list_tools 私有方法的依赖。hugegraph-mcp/hugegraph_mcp/tools/generate_gremlin.py (1)
69-69: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
if gremlin条件冗余Line 59 已对
not gremlin做了提前返回，此处gremlin必为真值，if gremlin else None的else分支永远不会执行。可以简化为decision = check_gremlin_read(gremlin)。♻️ 建议简化
- decision = check_gremlin_read(gremlin) if gremlin else None + decision = check_gremlin_read(gremlin)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/tools/generate_gremlin.py` at line 69, Remove the redundant conditional in the Gremlin generation flow and assign decision directly from check_gremlin_read(gremlin), since the earlier guard already returns when gremlin is falsy.hugegraph-mcp/hugegraph_mcp/gremlin_tools.py (1)
42-72: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value每次查询创建新客户端实例
_get_read_client和_get_write_client每次调用都会创建新的GremlinExecutor和PyHugeClient，意味着每次 Gremlin 查询都会建立新的 HTTP 连接。对于 MCP 服务器的典型工作负载来说可以接受，但如果未来需要支持高频调用，可以考虑引入客户端复用或连接池。🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@hugegraph-mcp/hugegraph_mcp/gremlin_tools.py` around lines 42 - 72, 每次调用_get_read_client和_get_write_client都会重新创建GremlinExecutor及PyHugeClient。请在GremlinExecutor或模块级别增加客户端复用机制（必要时使用连接池），让后续查询复用已创建的Gremlin客户端，同时保持读写客户端配置和线程安全。
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hugegraph-mcp/docs/p0a-integration-checklist.md`:
- Around line 14-16:
将文档中的硬编码本地路径“/Users/uleng/Code/hugegraph-ai”替换为通用占位符“<YOUR_PROJECT_PATH>”或适用的相对路径；同步更新相关
sed 命令及其他出现位置，确保用户可按自身项目路径执行。
In `@hugegraph-mcp/hugegraph_mcp/hugegraph_ai_client.py`:
- Around line 186-189: 修复 `_build_url` 以拒绝绝对 URL，避免请求绕过配置的 `cfg.ai_url`
并向任意主机发送凭据；当 `path` 以 `http://` 或 `https://`
开头时应抛出明确异常，而不是直接返回该地址，同时保留相对路径的安全拼接逻辑。
In `@hugegraph-mcp/hugegraph_mcp/tools/query_graph_data.py`:
- Around line 83-84: 将 get_by_id/get_by_ids 等路径中的 limit 转换纳入统一的 try/except 处理，避免
int(limit) 在异常处理范围外抛出 ValueError。调整 query_graph_data 中 bounded_limit
的计算位置或提前安全解析，并确保无效 limit 通过现有结构化错误响应返回；同时保留 _validate_limit 对 page 和 condition
的业务校验。
---
Duplicate comments:
In `@hugegraph-mcp/tests/test_extract_graph_data.py`:
- Around line 63-80: 修复测试中的 MCPConfig.from_env() 缓存导致的顺序依赖。定位
test_extract_graph_data_uses_graph_schema_by_default，在设置 HUGEGRAPH_GRAPH_PATH
后、调用 extract_graph_data_module.extract_graph_data 前清空 MCPConfig
的缓存或单例状态，确保测试始终读取当前环境变量配置；如项目已有统一的缓存重置方式，请复用该方式。
---
Nitpick comments:
In `@hugegraph-mcp/hugegraph_mcp/gremlin_tools.py`:
- Around line 42-72:
每次调用_get_read_client和_get_write_client都会重新创建GremlinExecutor及PyHugeClient。请在GremlinExecutor或模块级别增加客户端复用机制（必要时使用连接池），让后续查询复用已创建的Gremlin客户端，同时保持读写客户端配置和线程安全。
In `@hugegraph-mcp/hugegraph_mcp/hugegraph_ai_client.py`:
- Around line 181-183: Remove the dead fallback call `or get("/openapi.json",
cfg=cfg)` from the return statement at the end of `health_check`; return
`last_result` directly after preserving the existing warnings aggregation.
In `@hugegraph-mcp/hugegraph_mcp/tools/extract_graph_data.py`:
- Around line 55-81: 在图数据提取函数入口只调用一次 MCPConfig.from_env()，避免同一请求中重复读取配置。将该 cfg
传入 _schema_message 以及 post（及其内部 request），并移除末尾再次调用 MCPConfig.from_env()
的逻辑，确保整次调用使用同一份配置。
In `@hugegraph-mcp/hugegraph_mcp/tools/generate_gremlin.py`:
- Line 69: Remove the redundant conditional in the Gremlin generation flow and
assign decision directly from check_gremlin_read(gremlin), since the earlier
guard already returns when gremlin is falsy.
In `@hugegraph-mcp/hugegraph_mcp/tools/inspect_schema.py`:
- Around line 84-89: 修正 inspect_schema 中 all_summary 与返回结果的不一致：当
include_index_labels=False 时，确保传入 _filter_schema 的摘要不包含 index labels，使
data["filtered"]["index_labels"] 与 data["index_labels"] 保持一致；检查
_build_summary、_filter_schema 及其调用逻辑，并补充相关测试覆盖 filter_kind=None 的场景。
In `@hugegraph-mcp/tests/test_v1_stable_tools.py`:
- Around line 26-30: 更新 _list_mcp_tools，优先调用 server.mcp 提供的公开 list_tools 或
list_tools_mcp 接口，并仅在公共接口不可用时保留兼容处理；移除对 _mcp_list_tools 和 _list_tools 私有方法的依赖。
🪄 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: 2295ff8c-7473-4c61-b95e-f11616f1995c
📒 Files selected for processing (112)
.github/workflows/hugegraph-mcp.yml.gitignore.spec/hugegraph-llm/fixed_flow/design.md.spec/hugegraph-llm/fixed_flow/requirements.md.spec/hugegraph-llm/fixed_flow/tasks.md.spec/hugegraph-mcp/graph_mcp/requirements.mdREADME.mddocker/docker-compose-llm.ymldocker/docker-compose-network.ymlhugegraph-llm/README.mdhugegraph-llm/src/hugegraph_llm/api/models/rag_requests.pyhugegraph-llm/src/hugegraph_llm/api/models/rag_response.pyhugegraph-llm/src/hugegraph_llm/api/thin_api.pyhugegraph-llm/src/hugegraph_llm/demo/rag_demo/app.pyhugegraph-llm/src/hugegraph_llm/flows/import_graph_data.pyhugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.pyhugegraph-llm/src/tests/api/test_thin_api.pyhugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.pyhugegraph-llm/src/tests/test_rag_demo_cli.pyhugegraph-mcp/README.mdhugegraph-mcp/README.zh-CN.mdhugegraph-mcp/docs/graph_mcp_post_review_fix_plan.mdhugegraph-mcp/docs/graph_mcp_remaining_defect_fix_plan.mdhugegraph-mcp/docs/graph_mcp_review_findings_fixplan.mdhugegraph-mcp/docs/graph_mcp_review_fix_plan.mdhugegraph-mcp/docs/ingest_graph_data_validation_fix_plan.mdhugegraph-mcp/docs/mcp-v1-prune-prd.mdhugegraph-mcp/docs/p0a-integration-checklist.mdhugegraph-mcp/hugegraph_mcp/__init__.pyhugegraph-mcp/hugegraph_mcp/config.pyhugegraph-mcp/hugegraph_mcp/confirmable_workflow.pyhugegraph-mcp/hugegraph_mcp/confirmation_store.pyhugegraph-mcp/hugegraph_mcp/envelope.pyhugegraph-mcp/hugegraph_mcp/error_mapping.pyhugegraph-mcp/hugegraph_mcp/gremlin_policy.pyhugegraph-mcp/hugegraph_mcp/gremlin_safety.pyhugegraph-mcp/hugegraph_mcp/gremlin_tools.pyhugegraph-mcp/hugegraph_mcp/guard.pyhugegraph-mcp/hugegraph_mcp/hugegraph_ai_client.pyhugegraph-mcp/hugegraph_mcp/hugegraph_client.pyhugegraph-mcp/hugegraph_mcp/plan_hash.pyhugegraph-mcp/hugegraph_mcp/schema_tools.pyhugegraph-mcp/hugegraph_mcp/server.pyhugegraph-mcp/hugegraph_mcp/tools/__init__.pyhugegraph-mcp/hugegraph_mcp/tools/extract_graph_data.pyhugegraph-mcp/hugegraph_mcp/tools/generate_gremlin.pyhugegraph-mcp/hugegraph_mcp/tools/graph_data_execute.pyhugegraph-mcp/hugegraph_mcp/tools/graph_data_gremlin.pyhugegraph-mcp/hugegraph_mcp/tools/graph_data_mapping.pyhugegraph-mcp/hugegraph_mcp/tools/graph_data_validate.pyhugegraph-mcp/hugegraph_mcp/tools/ingest_graph_data.pyhugegraph-mcp/hugegraph_mcp/tools/inspect_graph.pyhugegraph-mcp/hugegraph_mcp/tools/inspect_schema.pyhugegraph-mcp/hugegraph_mcp/tools/live_schema.pyhugegraph-mcp/hugegraph_mcp/tools/manage_graph_data.pyhugegraph-mcp/hugegraph_mcp/tools/manage_schema.pyhugegraph-mcp/hugegraph_mcp/tools/mutate_graph_properties.pyhugegraph-mcp/hugegraph_mcp/tools/query_graph_data.pyhugegraph-mcp/hugegraph_mcp/tools/refresh_vid_embeddings.pyhugegraph-mcp/hugegraph_mcp/tools/schema_utils.pyhugegraph-mcp/pyproject.tomlhugegraph-mcp/tests/conftest.pyhugegraph-mcp/tests/integration/test_real_write_path.pyhugegraph-mcp/tests/test_config.pyhugegraph-mcp/tests/test_envelope.pyhugegraph-mcp/tests/test_error_handling.pyhugegraph-mcp/tests/test_execute_gremlin_read.pyhugegraph-mcp/tests/test_execute_gremlin_write.pyhugegraph-mcp/tests/test_extract_graph_data.pyhugegraph-mcp/tests/test_generate_gremlin.pyhugegraph-mcp/tests/test_get_live_schema.pyhugegraph-mcp/tests/test_gremlin_policy.pyhugegraph-mcp/tests/test_gremlin_safety.pyhugegraph-mcp/tests/test_guard.pyhugegraph-mcp/tests/test_hugegraph_ai_client.pyhugegraph-mcp/tests/test_import_graph_data_tool.pyhugegraph-mcp/tests/test_ingest_graph_data.pyhugegraph-mcp/tests/test_inspect_graph.pyhugegraph-mcp/tests/test_inspect_schema_tool.pyhugegraph-mcp/tests/test_manage_graph_data.pyhugegraph-mcp/tests/test_manage_schema.pyhugegraph-mcp/tests/test_mutate_graph_properties_tool.pyhugegraph-mcp/tests/test_plan_hash.pyhugegraph-mcp/tests/test_query_graph_data_tool.pyhugegraph-mcp/tests/test_readonly_mode.pyhugegraph-mcp/tests/test_refresh_vid_embeddings.pyhugegraph-mcp/tests/test_schema_utils.pyhugegraph-mcp/tests/test_v1_stable_tools.pyhugegraph-python-client/src/pyhugegraph/api/graph.pyhugegraph-python-client/src/pyhugegraph/api/schema_manage/index_label.pyhugegraph-python-client/src/pyhugegraph/api/schema_manage/property_key.pyhugegraph-python-client/src/pyhugegraph/utils/huge_config.pyhugegraph-python-client/src/pyhugegraph/utils/id_format.pyhugegraph-python-client/src/pyhugegraph/utils/log.pyhugegraph-python-client/src/tests/api/test_auth.pyhugegraph-python-client/src/tests/api/test_auth_routing.pyhugegraph-python-client/src/tests/api/test_graph.pyhugegraph-python-client/src/tests/api/test_schema.pyhugegraph-python-client/src/tests/api/test_schema_contract.pyhugegraph-python-client/src/tests/api/test_traverser.pyhugegraph-python-client/src/tests/api/test_vertex_id_format.pypyproject.tomlskills/hugegraph-data-importer/SKILL.mdskills/hugegraph-data-importer/agents/openai.yamlskills/hugegraph-operator/SKILL.mdskills/hugegraph-operator/agents/openai.yamlskills/hugegraph-query-analyst/SKILL.mdskills/hugegraph-query-analyst/agents/openai.yamlskills/hugegraph-regression-tester/SKILL.mdskills/hugegraph-regression-tester/agents/openai.yamlskills/hugegraph-schema-designer/SKILL.mdskills/hugegraph-schema-designer/agents/openai.yaml
💤 Files with no reviewable changes (1)
- hugegraph-python-client/src/tests/api/test_schema.py
✅ Files skipped from review due to trivial changes (20)
- hugegraph-mcp/hugegraph_mcp/tools/init.py
- hugegraph-mcp/hugegraph_mcp/init.py
- skills/hugegraph-query-analyst/agents/openai.yaml
- hugegraph-mcp/hugegraph_mcp/error_mapping.py
- .spec/hugegraph-llm/fixed_flow/tasks.md
- skills/hugegraph-schema-designer/agents/openai.yaml
- hugegraph-mcp/docs/ingest_graph_data_validation_fix_plan.md
- hugegraph-llm/src/tests/operators/hugegraph_op/test_schema_manager.py
- skills/hugegraph-operator/SKILL.md
- hugegraph-mcp/hugegraph_mcp/gremlin_safety.py
- skills/hugegraph-data-importer/SKILL.md
- skills/hugegraph-query-analyst/SKILL.md
- .gitignore
- skills/hugegraph-schema-designer/SKILL.md
- hugegraph-mcp/tests/test_refresh_vid_embeddings.py
- README.md
- hugegraph-llm/README.md
- skills/hugegraph-regression-tester/SKILL.md
- .spec/hugegraph-llm/fixed_flow/design.md
- hugegraph-mcp/README.zh-CN.md
🚧 Files skipped from review as they are similar to previous changes (34)
- hugegraph-mcp/tests/test_envelope.py
- skills/hugegraph-regression-tester/agents/openai.yaml
- skills/hugegraph-data-importer/agents/openai.yaml
- hugegraph-mcp/hugegraph_mcp/hugegraph_client.py
- hugegraph-python-client/src/tests/api/test_auth.py
- skills/hugegraph-operator/agents/openai.yaml
- hugegraph-python-client/src/pyhugegraph/utils/log.py
- hugegraph-python-client/src/pyhugegraph/utils/huge_config.py
- hugegraph-mcp/tests/test_execute_gremlin_write.py
- hugegraph-llm/src/hugegraph_llm/operators/hugegraph_op/schema_manager.py
- hugegraph-mcp/tests/test_gremlin_safety.py
- hugegraph-mcp/tests/test_get_live_schema.py
- hugegraph-mcp/pyproject.toml
- hugegraph-mcp/hugegraph_mcp/guard.py
- hugegraph-llm/src/hugegraph_llm/api/models/rag_requests.py
- hugegraph-mcp/hugegraph_mcp/schema_tools.py
- hugegraph-mcp/hugegraph_mcp/tools/refresh_vid_embeddings.py
- hugegraph-mcp/hugegraph_mcp/tools/live_schema.py
- hugegraph-mcp/tests/test_execute_gremlin_read.py
- hugegraph-python-client/src/tests/api/test_auth_routing.py
- hugegraph-llm/src/hugegraph_llm/api/thin_api.py
- hugegraph-mcp/tests/test_gremlin_policy.py
- hugegraph-mcp/tests/test_guard.py
- hugegraph-mcp/tests/test_import_graph_data_tool.py
- hugegraph-mcp/tests/test_readonly_mode.py
- hugegraph-mcp/hugegraph_mcp/tools/schema_utils.py
- hugegraph-llm/src/hugegraph_llm/api/models/rag_response.py
- hugegraph-mcp/hugegraph_mcp/tools/graph_data_validate.py
- hugegraph-mcp/hugegraph_mcp/tools/inspect_graph.py
- hugegraph-llm/src/tests/api/test_thin_api.py
- hugegraph-mcp/hugegraph_mcp/config.py
- hugegraph-mcp/hugegraph_mcp/tools/graph_data_mapping.py
- hugegraph-mcp/hugegraph_mcp/tools/manage_graph_data.py
- hugegraph-mcp/tests/test_manage_graph_data.py
UIengF
force-pushed
the
graph-mcp
branch
2 times, most recently
from
July 11, 2026 06:06
1833b36 to
acd7490
Compare
Change-Id: I39f965b763bf635927d5b00bf8ad6b6295ae1a2d
imbajin
requested changes
Aug 2, 2026
imbajin
left a comment
imbajin
left a comment
There was a problem hiding this comment.
Review 结论：Request changes
综合评分：5.9 / 10，当前不建议合并。
本轮按最新 head 8dac401a2ae88ed6212cce6839ba112705341a37 检查了方案设计、执行逻辑、代码质量、测试有效性和用户易用性，并核对了已有评论，避免重复提交已知问题。
整体方向是合理的：MCP 保持薄适配层，写操作采用 dry_run → 服务端签发计划 → confirm → 原子消费，Live Schema 校验、精确查询、真实 HugeGraph 写路径测试也具备较好的工程基础。尤其是最新版本已经修复早期“客户端可自行构造 plan_hash”和“nonce 仅存内存”的问题，这部分不建议重新设计。
但当前仍有一个直接破坏安全边界的 P0，以及三个需要在合并前处理的 P1。
❗ P0：只读 Gremlin 分类器可被 Groovy quoted identifier 绕过
当前分类器只识别 .method(，但 HugeGraph 执行的是 Gremlin-Groovy，Groovy 允许 quoted method identifier：
g.V().'drop'().'iterate'()
g.V()."property"('flag', true)."iterate"()字符串清理会清空 drop/property/iterate，随后方法提取只能看到 V，这些写操作可能被判为 safe。只读执行又使用实际 HugeGraph 凭据，因此可以绕过 readonly、admin、dry-run、plan hash 和 confirm。
这属于明确的合并阻断问题。V1 不必引入完整 Groovy parser，建议采用保守的 fail-closed 策略：检测点号后的 quoted/dynamic member 时统一返回 uncertain，并补攻击回归测试。防御纵深上建议允许 read client 使用独立的只读数据库 principal。
⚠️ P1：抽取阶段可能读取错误实例/graphspace 的 Schema
当 schema=None 时，MCP 只将 graph 名称传给 HugeGraph-AI；AI 端会使用自身全局连接配置读取同名图，但 MCP 最终仍把结果标记为当前 MCP 的 graph/graphspace。
当两个服务指向不同 URL、graphspace 或租户时，抽取会基于图 B 的 Schema 生成数据，却声明来源是图 A。建议由 MCP 使用自己的连接读取 live schema，将规范化 Schema JSON 直接发送给 AI 服务，并在 schema_ref 中带上 schema fingerprint。
⚠️ P1：合法 UUID 属性会被提前判定为 unsupported
_value_matches_type() 已经支持 UUID，但 _SUPPORTED_DATA_TYPES 没有 UUID，所以 UUID 属性尚未进入值校验就会失败。请补齐 UUID，并增加 SINGLE/LIST/SET 测试。最好让支持列表和 validator 共用同一个映射，避免两套定义再次漂移。
⚠️ P1：计划已消费后，partial/unknown 结果仍可能返回 retryable=true
此问题已有历史评论，本轮不重复开行级线程，但仍未解决。
当前执行顺序是：
verify_and_consume_plan
↓
计划永久消费
↓
执行多项写入
↓
partial / degraded / timeout / unknown
一旦计划被消费，只要无法明确证明“零副作用”，就不应提示 Agent 直接重试。partial、结果未知、请求可能已经送达的超时均应返回 retryable=false，并指导调用者先检查当前图状态，再生成只包含剩余操作的新 dry-run。
其他需要收口的问题
这些问题不必扩大架构，但建议在本 PR 中一并做最小修正：
HUGEGRAPH_MCP_TOOLSET当前只有精确v1才进入 v1，拼写错误或非法值会扩大到v2_core。配置错误应 fail-closed，非法值应启动失败。- admin/debug Gremlin write 工具即使关闭 admin mode 仍会出现在
tools/list。建议关闭时不注册，或拆分独立 admin entrypoint，减少 Agent 误调用和 prompt injection 面积。 - 公共 Gremlin 读默认仅对无界查询
warn，而非阻断；同时底层查询没有统一超时。Agent 场景建议默认reject_unbounded。 auto_append直接在字符串末尾追加.limit(100)，无法确认前序步骤仍是 traversal，可能把合法查询改写为无效 Gremlin。V1 更适合删除自动改写，只返回明确修正建议。table_data、mapping等参数已暴露在 MCP JSON Schema 中，但对应模式明确禁用且参数未生效。建议暂时移除，减少工具表面和模型误传参数。- PR 范围说明写“不包含 Schema 真实 apply”，但 README/代码中的
v2_core存在有限 create apply，文档与实际安全边界需要统一。 - 多份 review/fix-plan 临时文档进入正式模块，建议合并成一份长期 architecture/decision 文档，删除过程性记录，避免维护噪声。
测试结论
当前 CI 覆盖 Python 3.10/3.11/3.12、Ruff、pytest、wheel 安装以及 HugeGraph 1.7.0 真实写路径，这部分值得肯定。但真实测试主要直接调用 Python tool function，尚未覆盖完整 MCP 协议链：
installed wheel
→ 启动 STDIO server
→ initialize
→ tools/list
→ tools/call
→ HugeGraph
因此当前不能证明 CLI/STDIO JSON-RPC、工具 Schema、条件注册以及异常序列化在真实 MCP Client 下都正常。另有需求文档提出核心覆盖率目标，但 CI 没有覆盖率统计或门禁。
合并前最低建议补充：
- quoted/dynamic Groovy member 攻击回归；
- UUID SINGLE/LIST/SET；
- MCP 与 AI 配置不一致时的 Schema 来源测试；
- partial/unknown result 的 non-retryable 契约；
- installed wheel + STDIO MCP initialize/tools/list/tools/call smoke；
- 至少统计 MCP 核心模块覆盖率。
分项评分
| 维度 | 得分 | 说明 |
|---|---|---|
| 方案设计 | 7.2 / 10 | 薄适配层、能力门禁、确认账本方向正确，但只读安全模型仍依赖脆弱字符串分类 |
| 逻辑执行 | 4.8 / 10 | 存在只读写入绕过、Schema 目标错配、一次性计划与 retryable 语义冲突 |
| 代码质量 | 6.4 / 10 | 模块边界基本清楚，但配置 fail-open、无效接口和过程文档较多 |
| 测试有效性 | 6.3 / 10 | 函数级和真实写路径较好，缺 MCP 协议链、安全攻击语料和覆盖率门禁 |
| 用户易用性 | 5.6 / 10 | 响应信封较清晰，但工具表面偏大，危险工具可见，文档范围存在矛盾 |
完成 P0、三个 P1 和必要协议级测试后，预计可以达到 8.0～8.4 / 10。不需要推倒重构，也不建议为此引入完整 Groovy AST 或新的复杂服务层。
|
|
||
| GremlinClassification = Literal["safe", "unsafe", "uncertain"] | ||
| GremlinSafety = GremlinClassification # 兼容别名 | ||
|
|
There was a problem hiding this comment.
❗️ P0：Groovy quoted identifier 可以绕过只读 Gremlin 分类器
当前正则只提取 .method( 形式的方法名，但 HugeGraph 执行的是 Gremlin-Groovy，Groovy 允许点号后的方法名使用 quoted identifier，例如：
g.V().'drop'().'iterate'()
g.V()."property"('flag', true)."iterate"()_strip_string_literals() 会清空 drop/property/iterate 的内容，随后 _extract_method_names() 只能提取到 V，以上语句可能被判为 safe，并通过只读工具使用实际 HugeGraph 凭据执行。
这会绕过 readonly、admin、dry-run、plan_hash 和 confirm，属于合并阻断问题。
建议 V1 先采用 fail-closed：检测点号后的 quoted 或 dynamic member（如 .'...'、."..."、.(...)）时统一返回 uncertain，并增加上述攻击回归测试。防御纵深上建议允许 read client 配置独立的只读 HugeGraph principal。
|
|
||
| def _example_prompt_message(example_prompt: str | None) -> str: | ||
| if example_prompt is None: | ||
| return DEFAULT_GRAPH_EXTRACT_PROMPT_ZH |
There was a problem hiding this comment.
schema=None 时，实际抽取使用的 Schema 与返回的 schema_ref 可能不是同一个图
这里只把 MCP 当前 graph 名称发送给 HugeGraph-AI，没有携带 URL、graphspace 或 request-scoped connection。AI 端收到图名后会使用自己的全局 HugeGraph 配置读取同名图；但本函数最终仍把结果标记为 MCP 当前 graph/graphspace。
当 MCP 与 HugeGraph-AI 指向不同实例、graphspace 或租户时，候选数据会基于错误 Schema 抽取，同时携带错误 provenance。
建议由 MCP 使用自己的连接读取并规范化 live schema，然后将 Schema JSON 直接发送给 AI 服务；返回的 schema_ref 同时携带 schema fingerprint。这样不需要跨服务传递数据库凭据，也能彻底消除同名 graph 的目标歧义。
| replayed_plan_error, | ||
| verify_and_consume_plan, | ||
| ) | ||
| from hugegraph_mcp.envelope import ErrorType, envelope_err, envelope_ok |
There was a problem hiding this comment.
_value_matches_type() 已经显式支持 UUID，但 _SUPPORTED_DATA_TYPES 中没有 UUID，因此 live schema 中的 UUID property 会先被判为 unsupported，永远到不了 UUID 值匹配逻辑。
请补充 UUID，并增加 UUID + SINGLE/LIST/SET 的回归测试。更建议让“是否支持”和对应 validator 共用同一个映射，避免两套定义以后再次漂移。
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.
概述
本 PR 新增 HugeGraph MCP V1，并在基础工具集之上补充 Schema 检查、图数据查询和属性变更能力，同时加强写入确认、Schema 校验、配置安全和真实 HugeGraph 写入行为。
HugeGraph MCP 定位为安全、受控的薄适配层，为 MCP Client 和 Agent 提供图查询、Gremlin 生成、数据抽取、受控导入/删除以及 Schema 设计与校验能力。
主要改动
MCP V1 工具
写入安全
dry_runplan_hashconfirmplan_hash绑定目标图、Schema、Payload、权限状态和过期时间。PLAN_ALREADY_USED，不会重复执行写入。HUGEGRAPH_MCP_STATE_DIR配置。Schema 与属性校验
SINGLE、LIST和SET属性基数。nullbool和int。INTEGER -> INT、BOOL -> BOOLEAN。查询与属性变更
配置与部署安全
true / 1 / yes / onfalse / 0 / no / off测试
已完成以下验证：
git diff --check通过447 passed, 11 deselected11 passed真实环境验证了：
import_graph_data_tool(mode="ingest")成功写入 2 个顶点和 1 条边。["a", "b", "b"]，顺序和重复项得到保留。["a", "b"]，重复项被去除。SCHEMA_MISMATCH。提交结构
本 PR 已整理为 4 个职责明确的提交：
范围说明
本 PR 不包含以下能力：
暂不支持的入口继续返回
FEATURE_DISABLED，后续可通过独立 PR 实现。HugeGraph Server 要求版本为
1.7.0