imbajin commented on code in PR #382:
URL: https://github.com/apache/hugegraph-ai/pull/382#discussion_r4248415390
##########
hugegraph-llm/src/hugegraph_llm/api/thin_api.py:
##########
@@ -151,10 +151,31 @@ def graph_import_api(req: GraphImportRequest):
@thin_router.post("/vid-embeddings/refresh", status_code=status.HTTP_200_OK,
response_model=ThinAPIResponse)
-def vid_embeddings_refresh_api(_req: VidEmbeddingsRefreshRequest):
+def vid_embeddings_refresh_api(req: VidEmbeddingsRefreshRequest):
if blocked := _thin_write_disabled():
return blocked
- return _wrap_flow_call(FlowName.UPDATE_VID_EMBEDDINGS)
+ # Keep target validation and flow execution atomic with respect to runtime
+ # graph configuration updates. The flow reads the same shared settings.
+ with runtime_config_lock:
+ expected_fields = {"expected_graph", "expected_graphspace"}
+ supplied_fields = req.model_fields_set & expected_fields
+ if supplied_fields and (
+ supplied_fields != expected_fields
+ or req.expected_graph != huge_settings.graph_name
+ or req.expected_graphspace != huge_settings.graph_space
+ ):
+ return _envelope_err(
+ "VALIDATION_ERROR",
+ "Expected graph and graphspace must match the configured
HugeGraph target.",
+ suggestion="Provide both expected_graph and
expected_graphspace matching the AI service target.",
+ )
+ return _wrap_flow_call(FlowName.UPDATE_VID_EMBEDDINGS)
Review Comment:
‼️ Critical. Blocking: yes. Summary: The lock protects the current settings
during validation and dispatch, but `Scheduler.schedule_flow` reuses its
initialized refresh pipeline after the first run. Its fetch client and VID
index capture graph A during `pipeline.init()`; after the UI switches settings
to B, a B request can pass validation and still refresh A. Please invalidate or
rebuild the cached refresh pipeline when graph configuration changes, or fail
closed until it is rebuilt. Evidence: `Scheduler.schedule_flow` stores and
fetches the pipeline; `FetchGraphDataNode.node_init()` binds `get_hg_client()`,
and `BuildSemanticIndex` binds `huge_settings.graph_name` during initialization.
##########
hugegraph-mcp/hugegraph_mcp/tools/refresh_vid_embeddings.py:
##########
@@ -41,7 +42,42 @@ def refresh_vid_embeddings(confirm: bool = False) ->
dict[str, Any]:
suggestion="Pass confirm=True to refresh VID embeddings.",
)
- ai_result = post("/vid-embeddings/refresh", json={})
+ cfg = MCPConfig.from_env()
+ capability = get("/vid-embeddings/refresh-capabilities", cfg=cfg)
+ if not capability.get("ok"):
+ error = capability.get("error") or {}
+ # Only an explicitly absent/unsupported endpoint means the protocol is
+ # unavailable. Keep transient and authentication errors retryable as
+ # reported by the client, without ever dispatching the refresh.
+ status_code = (error.get("details") or {}).get("status_code")
+ if error.get("type") in {
+ "AUTHENTICATION_FAILED",
+ "AUTHORIZATION_FAILED",
+ "FEATURE_DISABLED",
+ } or status_code not in {404, 405}:
+ return capability
+ return envelope_err(
+ ErrorType.FEATURE_DISABLED,
+ "AI refresh target-validation endpoint is unavailable; refresh was
not sent.",
+ suggestion="Check the AI service URL and routing, or upgrade
HugeGraph-AI to support target validation.",
+ details={"status_code": status_code},
+ )
+ support = capability.get("data")
+ if (
+ not isinstance(support, dict)
+ or type(support.get("protocol_version")) is not int
+ or support["protocol_version"] != 1
+ ):
+ return envelope_err(
+ ErrorType.FEATURE_DISABLED,
+ "AI refresh target-validation protocol is unsupported; refresh was
not sent.",
+ suggestion="Upgrade HugeGraph-AI to a compatible target-validation
version.",
+ )
+ ai_result = post(
+ "/vid-embeddings/refresh",
+ cfg=cfg,
+ json={"expected_graph": cfg.graph, "expected_graphspace":
cfg.graphspace},
Review Comment:
⚠️ Important. Blocking: yes. Summary: This request sends only graph and
graphspace, while MCP `url` and AI `GRAPH_URL` are configured independently and
the refresh flow uses the AI service's `GRAPH_URL`. If they point to different
HugeGraph servers that expose the same graphspace/name, this check passes and
the confirmed refresh writes to the wrong instance. Please validate a stable
target identity as part of the capability handshake. Evidence: `MCPConfig` has
separate `url`/`ai_url`; the API compares only
`expected_graph`/`expected_graphspace`; the refresh flow builds its client from
`huge_settings.graph_url`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
