mirror of
https://github.com/tiennm99/DocsGPT.git
synced 2026-10-14 08:17:34 +00:00
Keep fixed values when an MCP server's tools are refreshed or re-saved
Parameters that still exist after re-reading the server keep the value the user fixed; parameters the server dropped go with the old schema.
This commit is contained in:
1 parent
f6bdd853d5
commit
54bea5a1d8
4 files changed
+72
-6
No files matched your search
@@ -5,6 +5,7 @@ from urllib.parse import urlencode, urlparse
|
||||
from flask import current_app, jsonify, make_response, redirect, request
|
||||
from flask_restx import Namespace, Resource, fields
|
||||
|
||||
from docsgpt.agents.tool_pins import carry_pins_between
|
||||
from docsgpt.agents.tools.mcp_tool import MCPOAuthManager, MCPTool
|
||||
from docsgpt.api import api
|
||||
from docsgpt.api.user.tools.routes import transform_actions
|
||||
@@ -414,7 +415,8 @@ class MCPServerSave(Resource):
|
||||
"custom_name": display_name,
|
||||
"description": description,
|
||||
"config": storage_config,
|
||||
"actions": transformed_actions,
|
||||
# Fixed values the user set survive a re-save.
|
||||
"actions": carry_pins_between(existing_doc.get("actions"), transformed_actions),
|
||||
"status": status_bool,
|
||||
"connection_id": connection_id,
|
||||
},
|
||||
@@ -442,7 +444,7 @@ class MCPServerSave(Resource):
|
||||
"custom_name": display_name,
|
||||
"description": description,
|
||||
"config": storage_config,
|
||||
"actions": transformed_actions,
|
||||
"actions": carry_pins_between(existing_by_name.get("actions"), transformed_actions),
|
||||
"status": status_bool,
|
||||
"connection_id": connection_id,
|
||||
},
|
||||
|
||||
@@ -2,6 +2,7 @@
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from docsgpt.agents.tool_pins import carry_pins
|
||||
from docsgpt.connectors import catalog, service
|
||||
from docsgpt.connectors.permissions import apply_default_permissions
|
||||
from docsgpt.storage.db.repositories.connector_sessions import ConnectorSessionsRepository
|
||||
@@ -43,9 +44,10 @@ def discover_builtin_actions(user_id: str, connection: dict) -> list[dict]:
|
||||
def refresh_mcp_tools(user_id: str, connection: dict) -> dict:
|
||||
"""Re-read the actions of every MCP tool on a connection.
|
||||
|
||||
Actions that still exist keep the permissions the user chose; new ones
|
||||
get the defaults from their annotations (reads always allowed, writes
|
||||
need approval); removed ones disappear.
|
||||
Actions that still exist keep the permissions the user chose and the
|
||||
fixed values of parameters they still have; new ones get the defaults
|
||||
from their annotations (reads always allowed, writes need approval);
|
||||
removed ones disappear.
|
||||
|
||||
Returns:
|
||||
``{"added": [...], "removed": [...], "tools": [...]}``.
|
||||
@@ -64,7 +66,7 @@ def refresh_mcp_tools(user_id: str, connection: dict) -> dict:
|
||||
old = previous.get(action.get("name"))
|
||||
if old is not None:
|
||||
action = {
|
||||
**action,
|
||||
**carry_pins(old, action),
|
||||
"active": old.get("active", True),
|
||||
"require_approval": bool(old.get("require_approval")),
|
||||
}
|
||||
|
||||
@@ -404,6 +404,47 @@ class TestMCPServerSave:
|
||||
response = MCPServerSave().post()
|
||||
assert response.status_code in (200, 201)
|
||||
|
||||
def test_saving_an_existing_server_keeps_fixed_values(self, app, pg_conn):
|
||||
from docsgpt.api.user.tools.mcp import MCPServerSave
|
||||
from docsgpt.storage.db.repositories.user_tools import UserToolsRepository
|
||||
|
||||
user = "u-mcp-resave"
|
||||
repo = UserToolsRepository(pg_conn)
|
||||
existing = repo.create(
|
||||
user, "mcp_tool",
|
||||
config={"server_url": "https://example.com/mcp", "auth_type": "none"},
|
||||
display_name="My MCP",
|
||||
actions=[{
|
||||
"name": "search",
|
||||
"active": True,
|
||||
"parameters": {"properties": {
|
||||
"q": {"type": "string", "filled_by_llm": True, "value": ""},
|
||||
"team": {"type": "string", "filled_by_llm": False, "value": "ENG"},
|
||||
}},
|
||||
}],
|
||||
)
|
||||
fake_tool = MagicMock()
|
||||
fake_tool.get_actions_metadata.return_value = [{
|
||||
"name": "search",
|
||||
"parameters": {"properties": {"q": {"type": "string"}, "team": {"type": "string"}}},
|
||||
}]
|
||||
with _patch_db(pg_conn), patch(
|
||||
"docsgpt.api.user.tools.mcp.MCPTool", return_value=fake_tool,
|
||||
), app.test_request_context(
|
||||
"/api/mcp_server/save", method="POST",
|
||||
json={
|
||||
"id": str(existing["id"]),
|
||||
"displayName": "My MCP",
|
||||
"config": {"transport_type": "http", "server_url": "https://example.com/mcp", "auth_type": "none"},
|
||||
},
|
||||
):
|
||||
from flask import request
|
||||
request.decoded_token = {"sub": user}
|
||||
response = MCPServerSave().post()
|
||||
assert response.status_code == 200
|
||||
team = repo.get_any(str(existing["id"]), user)["actions"][0]["parameters"]["properties"]["team"]
|
||||
assert team["filled_by_llm"] is False and team["value"] == "ENG"
|
||||
|
||||
|
||||
class TestMCPOAuthCallback:
|
||||
def test_error_param_redirects_error(self, app):
|
||||
|
||||
@@ -81,6 +81,27 @@ class TestRefresh:
|
||||
assert actions["create_issue"]["parameters"]["properties"]["q"]["filled_by_llm"] is True
|
||||
assert [t["id"] for t in result["tools"]] == [tool_id]
|
||||
|
||||
def test_fixed_values_survive_for_parameters_that_still_exist(self, pg_conn):
|
||||
connection = _connection(pg_conn)
|
||||
pinned = _action("search_issues")
|
||||
pinned["parameters"]["properties"] = {
|
||||
"q": {"type": "string", "filled_by_llm": True, "value": ""},
|
||||
"team": {"type": "string", "filled_by_llm": False, "value": "ENG"},
|
||||
"gone": {"type": "string", "filled_by_llm": False, "value": "x"},
|
||||
}
|
||||
tool_id = _tool(pg_conn, connection["id"], [pinned])
|
||||
fresh = _action("search_issues")
|
||||
fresh["parameters"]["properties"] = {"q": {"type": "string"}, "team": {"type": "string"}}
|
||||
with _db(pg_conn), patch.object(mcp, "_discover", return_value=[fresh]):
|
||||
mcp.refresh_mcp_tools("alice", connection)
|
||||
action = pg_conn.execute(
|
||||
text("SELECT actions FROM user_tools WHERE id = CAST(:i AS uuid)"), {"i": tool_id}
|
||||
).scalar()[0]
|
||||
properties = action["parameters"]["properties"]
|
||||
assert properties["team"] == {"type": "string", "filled_by_llm": False, "value": "ENG"}
|
||||
assert properties["q"]["filled_by_llm"] is True
|
||||
assert "gone" not in properties
|
||||
|
||||
def test_other_tools_on_the_connection_are_left_alone(self, pg_conn):
|
||||
connection = _connection(pg_conn)
|
||||
pg_conn.execute(
|
||||
|
||||
Reference in new issue
Block a user