Skip to content

Commit ea23afa

Browse files
committed
fix: resolve pylint warnings and update tests for A2A protocol refactor
- Remove redundant local `from mcpgateway.config import settings` re-imports in a2a_service.py (W0404/W0621) - Add pylint disable for unused `interaction_type` parameter in build_a2a_jsonrpc_request (W0613) - part of public API - Add missing docstrings for interrogate coverage (8 functions) - Update test patches to target module-level settings binding at mcpgateway.services.a2a_service.settings - Redirect decode_auth/apply_query_param_auth test patches to mcpgateway.services.a2a_protocol where prepare_a2a_invocation now calls them - Update admin test assertion for simplified test_params format Signed-off-by: Jon Spriggs <github@sprig.gs> Signed-off-by: Jonathan Springer <jps@s390x.com>
1 parent 2041cbf commit ea23afa

File tree

7 files changed

+36
-37
lines changed

7 files changed

+36
-37
lines changed

mcpgateway/services/a2a_protocol.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,7 @@ def normalize_a2a_method(method: Optional[str], protocol_version: Optional[str])
126126

127127

128128
def _normalize_role(role: Any, protocol_version: Optional[str]) -> Any:
129+
"""Normalize an A2A message role between v1 and legacy protocol forms."""
129130
value = str(role or "").strip()
130131
if not value:
131132
return role
@@ -135,6 +136,7 @@ def _normalize_role(role: Any, protocol_version: Optional[str]) -> Any:
135136

136137

137138
def _normalize_part(part: Any, protocol_version: Optional[str]) -> Any:
139+
"""Normalize an A2A message part between v1 and legacy protocol forms."""
138140
if not isinstance(part, Mapping):
139141
return part
140142

@@ -159,6 +161,7 @@ def _normalize_part(part: Any, protocol_version: Optional[str]) -> Any:
159161

160162

161163
def _normalize_task_state(state: Any, protocol_version: Optional[str]) -> Any:
164+
"""Normalize an A2A task state between v1 and legacy protocol forms."""
162165
value = str(state or "").strip()
163166
if not value:
164167
return state
@@ -168,6 +171,7 @@ def _normalize_task_state(state: Any, protocol_version: Optional[str]) -> Any:
168171

169172

170173
def _normalize_message(message: Any, protocol_version: Optional[str]) -> Any:
174+
"""Normalize an A2A message object for the target protocol version."""
171175
if not isinstance(message, Mapping):
172176
return message
173177

@@ -184,6 +188,7 @@ def _normalize_message(message: Any, protocol_version: Optional[str]) -> Any:
184188

185189

186190
def _normalize_task_status(status: Any, protocol_version: Optional[str]) -> Any:
191+
"""Normalize an A2A task status for the target protocol version."""
187192
if isinstance(status, str):
188193
return _normalize_task_state(status, protocol_version)
189194
if not isinstance(status, Mapping):
@@ -198,6 +203,7 @@ def _normalize_task_status(status: Any, protocol_version: Optional[str]) -> Any:
198203

199204

200205
def _normalize_task(task: Any, protocol_version: Optional[str]) -> Any:
206+
"""Normalize an A2A task object for the target protocol version."""
201207
if not isinstance(task, Mapping):
202208
return task
203209

@@ -249,6 +255,7 @@ def normalize_a2a_params(params: Any, protocol_version: Optional[str]) -> Any:
249255

250256

251257
def _build_default_message(query: str, protocol_version: Optional[str], message_id: Optional[str] = None) -> Dict[str, Any]:
258+
"""Build a default user message in the appropriate protocol format."""
252259
target_message_id = message_id or f"contextforge-{uuid.uuid4().hex}"
253260
if is_v1_a2a_protocol(protocol_version):
254261
return {
@@ -264,7 +271,7 @@ def _build_default_message(query: str, protocol_version: Optional[str], message_
264271
}
265272

266273

267-
def build_a2a_jsonrpc_request(parameters: Dict[str, Any], protocol_version: Optional[str], *, interaction_type: str = "query") -> Dict[str, Any]:
274+
def build_a2a_jsonrpc_request(parameters: Dict[str, Any], protocol_version: Optional[str], *, interaction_type: str = "query") -> Dict[str, Any]: # pylint: disable=unused-argument
268275
"""Build a JSON-RPC A2A request body for the target protocol version."""
269276
payload = dict(parameters or {})
270277
request_id = payload.pop("id", 1)

mcpgateway/services/a2a_service.py

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -354,9 +354,6 @@ async def register_agent(
354354
# Standard
355355
from urllib.parse import urlparse # pylint: disable=import-outside-toplevel
356356

357-
# First-Party
358-
from mcpgateway.config import settings # pylint: disable=import-outside-toplevel
359-
360357
# Service-layer enforcement: Check feature flag
361358
if not settings.insecure_allow_queryparam_auth:
362359
raise ValueError("Query parameter authentication is disabled. Set INSECURE_ALLOW_QUERYPARAM_AUTH=true to enable.")
@@ -1014,9 +1011,6 @@ async def update_agent(
10141011
# Standard
10151012
from urllib.parse import urlparse # pylint: disable=import-outside-toplevel
10161013

1017-
# First-Party
1018-
from mcpgateway.config import settings # pylint: disable=import-outside-toplevel
1019-
10201014
# Service-layer enforcement: Check feature flag
10211015
if not settings.insecure_allow_queryparam_auth:
10221016
# Grandfather clause: Allow updates to existing query_param agents
@@ -1049,9 +1043,6 @@ async def update_agent(
10491043
is_masked_placeholder = False
10501044
if param_value and hasattr(param_value, "get_secret_value"):
10511045
raw_value = param_value.get_secret_value()
1052-
# First-Party
1053-
from mcpgateway.config import settings # pylint: disable=import-outside-toplevel
1054-
10551046
is_masked_placeholder = raw_value == settings.masked_auth_value
10561047
elif param_value:
10571048
raw_value = str(param_value)

mcpgateway/services/rust_a2a_runtime.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -72,6 +72,7 @@ async def invoke(self, prepared: PreparedA2AInvocation, *, timeout_seconds: Opti
7272
return payload
7373

7474
async def _get_runtime_client(self) -> httpx.AsyncClient:
75+
"""Return the httpx client, lazily creating a UDS transport if configured."""
7576
uds_path = settings.experimental_rust_a2a_runtime_uds
7677
if not uds_path:
7778
return await get_http_client()

tests/unit/mcpgateway/services/test_a2a_query_param_auth.py

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -80,18 +80,18 @@ def _bypass_a2aagentread_validation(monkeypatch):
8080
def mock_all_settings():
8181
"""Mock settings in schemas, a2a_service, and config modules."""
8282
with patch("mcpgateway.schemas.settings") as schema_settings, \
83-
patch("mcpgateway.config.settings") as config_settings:
83+
patch("mcpgateway.services.a2a_service.settings") as svc_settings:
8484
# Configure schema settings
8585
schema_settings.insecure_allow_queryparam_auth = True
8686
schema_settings.insecure_queryparam_auth_allowed_hosts = ["api.tavily.com", "mcp.tavily.com", "api.example.com"]
8787
schema_settings.masked_auth_value = "*****"
8888

89-
# Configure config settings (used by service layer imports)
90-
config_settings.insecure_allow_queryparam_auth = True
91-
config_settings.insecure_queryparam_auth_allowed_hosts = ["api.tavily.com", "mcp.tavily.com", "api.example.com"]
92-
config_settings.masked_auth_value = "*****"
89+
# Configure service-layer settings (module-level import in a2a_service)
90+
svc_settings.insecure_allow_queryparam_auth = True
91+
svc_settings.insecure_queryparam_auth_allowed_hosts = ["api.tavily.com", "mcp.tavily.com", "api.example.com"]
92+
svc_settings.masked_auth_value = "*****"
9393

94-
yield {"schema": schema_settings, "config": config_settings}
94+
yield {"schema": schema_settings, "config": svc_settings}
9595

9696

9797
@pytest.fixture(autouse=True)

tests/unit/mcpgateway/services/test_a2a_service.py

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1339,7 +1339,7 @@ async def test_register_query_param_disabled(self, service, mock_db, monkeypatch
13391339
"""Query param auth disabled raises ValueError."""
13401340
monkeypatch.setattr("mcpgateway.services.a2a_service.get_for_update", lambda *a, **kw: None)
13411341

1342-
with patch("mcpgateway.config.settings") as mock_settings:
1342+
with patch("mcpgateway.services.a2a_service.settings") as mock_settings:
13431343
mock_settings.insecure_allow_queryparam_auth = False
13441344
agent_data = A2AAgentCreate.model_construct(
13451345
name="qp-agent",
@@ -1361,7 +1361,7 @@ async def test_register_query_param_host_not_allowed(self, service, mock_db, mon
13611361
"""Query param auth host not in allowlist raises ValueError."""
13621362
monkeypatch.setattr("mcpgateway.services.a2a_service.get_for_update", lambda *a, **kw: None)
13631363

1364-
with patch("mcpgateway.config.settings") as mock_settings:
1364+
with patch("mcpgateway.services.a2a_service.settings") as mock_settings:
13651365
mock_settings.insecure_allow_queryparam_auth = True
13661366
mock_settings.insecure_queryparam_auth_allowed_hosts = ["safe.host.com"]
13671367
agent_data = A2AAgentCreate.model_construct(
@@ -1397,7 +1397,7 @@ async def test_register_query_param_secretstr_value(self, service, mock_db, monk
13971397
secret_val = MagicMock()
13981398
secret_val.get_secret_value.return_value = "the-secret"
13991399

1400-
with patch("mcpgateway.config.settings") as mock_settings:
1400+
with patch("mcpgateway.services.a2a_service.settings") as mock_settings:
14011401
mock_settings.insecure_allow_queryparam_auth = True
14021402
mock_settings.insecure_queryparam_auth_allowed_hosts = []
14031403

@@ -1437,7 +1437,7 @@ async def test_register_query_param_non_secret_value_uses_str(self, service, moc
14371437
monkeypatch.setattr("mcpgateway.cache.metrics_cache.metrics_cache", SimpleNamespace(invalidate=MagicMock()))
14381438
monkeypatch.setattr("mcpgateway.services.a2a_service.encode_auth", lambda _val: "encrypted")
14391439

1440-
with patch("mcpgateway.config.settings") as mock_settings:
1440+
with patch("mcpgateway.services.a2a_service.settings") as mock_settings:
14411441
mock_settings.insecure_allow_queryparam_auth = True
14421442
mock_settings.insecure_queryparam_auth_allowed_hosts = []
14431443

@@ -1474,7 +1474,7 @@ async def test_register_query_param_missing_key_or_value_skips_encryption(self,
14741474
monkeypatch.setattr("mcpgateway.cache.admin_stats_cache.admin_stats_cache", SimpleNamespace(invalidate_tags=AsyncMock()))
14751475
monkeypatch.setattr("mcpgateway.cache.metrics_cache.metrics_cache", SimpleNamespace(invalidate=MagicMock()))
14761476

1477-
with patch("mcpgateway.config.settings") as mock_settings:
1477+
with patch("mcpgateway.services.a2a_service.settings") as mock_settings:
14781478
mock_settings.insecure_allow_queryparam_auth = True
14791479
mock_settings.insecure_queryparam_auth_allowed_hosts = []
14801480

@@ -1908,7 +1908,7 @@ async def test_queryparam_switching_disabled_grandfather(self, service, mock_db,
19081908
"""Switching to query_param when disabled raises ValueError."""
19091909
agent = self._make_agent(auth_type="bearer")
19101910
with patch("mcpgateway.services.a2a_service.get_for_update", return_value=agent):
1911-
with patch("mcpgateway.config.settings") as mock_settings:
1911+
with patch("mcpgateway.services.a2a_service.settings") as mock_settings:
19121912
mock_settings.insecure_allow_queryparam_auth = False
19131913
mock_settings.insecure_queryparam_auth_allowed_hosts = []
19141914
update = A2AAgentUpdate.model_construct(
@@ -1923,7 +1923,7 @@ async def test_queryparam_host_not_allowed_on_update(self, service, mock_db, mon
19231923
"""Host allowlist is enforced when switching to query_param."""
19241924
agent = self._make_agent(auth_type="bearer", endpoint_url="https://bad.host.com/agent")
19251925
with patch("mcpgateway.services.a2a_service.get_for_update", return_value=agent):
1926-
with patch("mcpgateway.config.settings") as mock_settings:
1926+
with patch("mcpgateway.services.a2a_service.settings") as mock_settings:
19271927
mock_settings.insecure_allow_queryparam_auth = True
19281928
mock_settings.insecure_queryparam_auth_allowed_hosts = ["safe.host.com"]
19291929
update = A2AAgentUpdate.model_construct(

tests/unit/mcpgateway/services/test_tool_service_coverage.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2149,7 +2149,8 @@ async def test_call_a2a_custom_agent(self, tool_service):
21492149
mock_client = AsyncMock()
21502150
mock_client.post.return_value = mock_response
21512151

2152-
with patch("mcpgateway.services.http_client_service.get_http_client", new_callable=AsyncMock, return_value=mock_client):
2152+
with patch("mcpgateway.services.http_client_service.get_http_client", new_callable=AsyncMock, return_value=mock_client), \
2153+
patch("mcpgateway.services.a2a_protocol.decode_auth", return_value={"Authorization": "Bearer my-token"}):
21532154
result = await tool_service._call_a2a_agent(agent, {"query": "test"})
21542155
assert result == {"data": "custom"}
21552156
# Verify bearer auth was added
@@ -2748,24 +2749,21 @@ async def test_jsonrpc_with_query_param(self, tool_service):
27482749

27492750
@pytest.mark.asyncio
27502751
async def test_jsonrpc_request_data_prepare_error_is_logged_and_raised(self, tool_service):
2751-
"""If preparing JSONRPC request data fails, the error is logged and re-raised."""
2752+
"""If preparing request data fails, the error propagates to the caller."""
27522753
agent = self._make_agent(agent_type="jsonrpc")
27532754

27542755
def _info_side_effect(msg, *args, **kwargs):
2755-
# Allow the initial "Calling A2A agent..." log, but force an exception for the JSONRPC request_data log.
2756-
if "JSONRPC request_data prepared" in str(msg):
2756+
# Allow the initial "Calling A2A agent..." log, but force an exception for the request_data log.
2757+
if "invoke tool request_data prepared" in str(msg):
27572758
raise RuntimeError("logger boom")
27582759
return None
27592760

27602761
with patch("mcpgateway.services.tool_service.logger") as mock_logger:
27612762
mock_logger.info.side_effect = _info_side_effect
2762-
mock_logger.error = MagicMock()
27632763

27642764
with pytest.raises(RuntimeError, match="logger boom"):
27652765
await tool_service._call_a2a_agent(agent, {"query": "hello"})
27662766

2767-
mock_logger.error.assert_called_once()
2768-
27692767
@pytest.mark.asyncio
27702768
async def test_custom_agent_format(self, tool_service):
27712769
agent = self._make_agent(agent_type="custom", endpoint_url="http://custom.test")
@@ -2804,7 +2802,8 @@ async def test_bearer_auth(self, tool_service):
28042802
mock_client = AsyncMock()
28052803
mock_client.post.return_value = mock_resp
28062804

2807-
with patch("mcpgateway.services.http_client_service.get_http_client", new_callable=AsyncMock, return_value=mock_client):
2805+
with patch("mcpgateway.services.http_client_service.get_http_client", new_callable=AsyncMock, return_value=mock_client), \
2806+
patch("mcpgateway.services.a2a_protocol.decode_auth", return_value={"Authorization": "Bearer bearer-token"}):
28082807
await tool_service._call_a2a_agent(agent, {"query": "test"})
28092808
headers = mock_client.post.call_args[1]["headers"]
28102809
assert headers["Authorization"] == "Bearer bearer-token"
@@ -2820,9 +2819,9 @@ async def test_query_param_auth(self, tool_service):
28202819

28212820
with (
28222821
patch("mcpgateway.services.http_client_service.get_http_client", new_callable=AsyncMock, return_value=mock_client),
2823-
patch("mcpgateway.services.tool_service.decode_auth", return_value={"api_key": "real-key"}),
2824-
patch("mcpgateway.services.tool_service.apply_query_param_auth", return_value="http://agent.test/api?api_key=real-key"),
2825-
patch("mcpgateway.services.tool_service.sanitize_url_for_logging", return_value="http://agent.test/api?api_key=***"),
2822+
patch("mcpgateway.services.a2a_protocol.decode_auth", return_value={"api_key": "real-key"}),
2823+
patch("mcpgateway.services.a2a_protocol.apply_query_param_auth", return_value="http://agent.test/api?api_key=real-key"),
2824+
patch("mcpgateway.services.a2a_protocol.sanitize_url_for_logging", return_value="http://agent.test/api?api_key=***"),
28262825
):
28272826
await tool_service._call_a2a_agent(agent, {"query": "test"})
28282827
assert mock_client.post.call_args[0][0] == "http://agent.test/api?api_key=real-key"
@@ -6565,8 +6564,9 @@ async def fake_post(url, json=None, headers=None):
65656564
patch("mcpgateway.services.tool_service.current_trace_id") as mock_trace,
65666565
patch("mcpgateway.services.tool_service.create_span") as mock_span_ctx,
65676566
patch("mcpgateway.services.metrics_buffer_service.get_metrics_buffer_service") as mock_mbuf,
6568-
patch("mcpgateway.services.tool_service.decode_auth", return_value={"token": "decrypted_val"}),
6569-
patch("mcpgateway.services.tool_service.apply_query_param_auth", return_value="http://a2a-agent:9000/?token=decrypted_val"),
6567+
patch("mcpgateway.services.a2a_protocol.decode_auth", return_value={"token": "decrypted_val"}),
6568+
patch("mcpgateway.services.a2a_protocol.apply_query_param_auth", return_value="http://a2a-agent:9000/?token=decrypted_val"),
6569+
patch("mcpgateway.services.a2a_protocol.sanitize_url_for_logging", return_value="http://a2a-agent:9000/?token=***"),
65706570
patch("mcpgateway.services.tool_service.compute_passthrough_headers_cached", return_value={}),
65716571
):
65726572
mock_gcc.get_passthrough_headers = MagicMock(return_value=[])

tests/unit/mcpgateway/test_admin.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4898,7 +4898,7 @@ async def test_admin_test_a2a_agent_body_read_exception_uses_default_message(sel
48984898
result = await admin_test_a2a_agent("agent-1", mock_request, mock_db, user={"email": "test-user", "db": mock_db})
48994899
assert result.status_code == 200
49004900
params = service.invoke_agent.call_args.args[2]
4901-
assert "Hello from ContextForge Admin UI test!" in params["params"]["message"]["parts"][0]["text"]
4901+
assert params["query"] == "Hello from ContextForge Admin UI test!"
49024902

49034903
@pytest.mark.asyncio
49044904
async def test_admin_test_a2a_agent_generic_test_params_branch(self, monkeypatch, mock_request, mock_db, allow_permission):

0 commit comments

Comments
 (0)