Skip to content

Commit e3d99ed

Browse files
mneudertmneudert
committed
Handle empty API tool parameters more gracefully
1 parent bea47e2 commit e3d99ed

4 files changed

Lines changed: 159 additions & 3 deletions

File tree

‎McpTools/AbstractApiCall.php‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
use Piwik\Plugins\McpServer\Contracts\Ports\Api\ApiCallQueryServiceInterface;
1616
use Piwik\Plugins\McpServer\Contracts\Ports\Api\ApiMethodSummaryQueryServiceInterface;
1717
use Piwik\Plugins\McpServer\Contracts\Records\Api\ApiCallRecord;
18+
use Piwik\Plugins\McpServer\Support\Normalization\ToolDataNormalizer;
1819
use Piwik\Plugins\McpServer\SystemSettings;
1920

2021
/**
@@ -41,6 +42,10 @@ public function call(
4142
?string $action = null,
4243
?array $parameters = null
4344
): array {
45+
$parameters = $parameters === null
46+
? null
47+
: ToolDataNormalizer::requireStringKeyedArrayOrEmptyList($parameters, 'parameters');
48+
4449
$accessMode = $this->systemSettings->getRawApiAccessMode();
4550
$resolvedMethod = $this->apiMethodSummaryQueryService->getApiMethodSummaryBySelector(
4651
$accessMode,

‎Schemas/Api/ApiCallToolInputSchema.php‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,18 @@ final class ApiCallToolInputSchema
3232
'description' => 'Exact Matomo API action name.',
3333
],
3434
'parameters' => [
35-
'type' => 'object',
36-
'additionalProperties' => true,
37-
'description' => 'Optional Matomo API parameters for the selected method.',
35+
'oneOf' => [
36+
[
37+
'type' => 'object',
38+
'additionalProperties' => true,
39+
'description' => 'Optional Matomo API parameters for the selected method.',
40+
],
41+
[
42+
'type' => 'array',
43+
'maxItems' => 0,
44+
'description' => 'Empty array is accepted and normalized as no parameters.',
45+
],
46+
],
3847
],
3948
],
4049
'not' => [

‎tests/Integration/McpTools/ApiCallTest.php‎

Lines changed: 72 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,55 @@ public function testReadModeCallsKnownReadMethodByModuleAndActionSelector(): voi
9191
self::assertIsString($content['result']);
9292
}
9393

94+
public function testReadModeAcceptsEmptyParametersList(): void
95+
{
96+
McpTestHelper::setRawApiAccessMode('read');
97+
98+
$server = McpTestHelper::buildServer();
99+
$sessionId = McpTestHelper::initializeSession($server);
100+
$content = McpTestHelper::callToolAndAssertSuccess(
101+
$server,
102+
$sessionId,
103+
ApiCallRead::TOOL_NAME,
104+
['method' => 'API.getMatomoVersion', 'parameters' => []],
105+
__METHOD__
106+
);
107+
108+
$resolvedMethod = $content['resolvedMethod'] ?? null;
109+
self::assertIsArray($resolvedMethod);
110+
self::assertSame('API.getMatomoVersion', $resolvedMethod['method'] ?? null);
111+
self::assertIsString($content['result'] ?? null);
112+
}
113+
114+
public function testReadModeAcceptsEmptyParametersObject(): void
115+
{
116+
McpTestHelper::setRawApiAccessMode('read');
117+
118+
$server = McpTestHelper::buildServer();
119+
$sessionId = McpTestHelper::initializeSession($server);
120+
$payload = json_encode([
121+
'jsonrpc' => '2.0',
122+
'id' => __METHOD__,
123+
'method' => 'tools/call',
124+
'params' => [
125+
'name' => ApiCallRead::TOOL_NAME,
126+
'arguments' => [
127+
'method' => 'API.getMatomoVersion',
128+
'parameters' => new \stdClass(),
129+
],
130+
],
131+
], JSON_THROW_ON_ERROR);
132+
133+
$response = McpTestHelper::postJson($server, $payload, ['Mcp-Session-Id' => $sessionId]);
134+
$message = McpTestHelper::decodeResponse($response);
135+
$result = McpTestHelper::parseCallTool($message);
136+
$content = McpTestHelper::assertToolSuccess($result);
137+
$resolvedMethod = $content['resolvedMethod'] ?? null;
138+
self::assertIsArray($resolvedMethod);
139+
self::assertSame('API.getMatomoVersion', $resolvedMethod['method'] ?? null);
140+
self::assertIsString($content['result'] ?? null);
141+
}
142+
94143
public function testReadModeRejectsWriteOnlyMethod(): void
95144
{
96145
McpTestHelper::setRawApiAccessMode('read');
@@ -329,6 +378,29 @@ public function testRejectsReservedParameterKeys(): void
329378
);
330379
}
331380

381+
public function testRejectsNonEmptyParametersListAtSchemaLevel(): void
382+
{
383+
McpTestHelper::setRawApiAccessMode('read');
384+
385+
$server = McpTestHelper::buildServer();
386+
$sessionId = McpTestHelper::initializeSession($server);
387+
$message = McpTestHelper::callToolExpectInvalidParams(
388+
$server,
389+
$sessionId,
390+
ApiCallRead::TOOL_NAME,
391+
[
392+
'method' => 'API.getMatomoVersion',
393+
'parameters' => ['flat'],
394+
],
395+
__METHOD__
396+
);
397+
398+
self::assertStringContainsString(
399+
"Invalid parameters for tool '" . ApiCallRead::TOOL_NAME . "':",
400+
$message->message
401+
);
402+
}
403+
332404
public function testRejectsMissingSelectorAtSchemaLevel(): void
333405
{
334406
McpTestHelper::setRawApiAccessMode('full');

‎tests/Unit/McpTools/ApiCallTest.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -160,6 +160,76 @@ public function callApi(
160160
self::assertSame(['userLogin' => 'alice'], $capturedValues['parameters']);
161161
}
162162

163+
public function testCallNormalizesEmptyParameterList(): void
164+
{
165+
$captured = new stdClass();
166+
$captured->values = [];
167+
168+
$record = new ApiMethodSummaryRecord(
169+
'API',
170+
'getMatomoVersion',
171+
'API.getMatomoVersion',
172+
[],
173+
ApiMethodOperationClassifier::CATEGORY_READ,
174+
ApiMethodOperationClassifier::CONFIDENCE_HIGH,
175+
'action-prefix:get'
176+
);
177+
178+
$tool = new ApiCallRead(
179+
new class ($captured) implements ApiCallQueryServiceInterface {
180+
public function __construct(private stdClass $captured)
181+
{
182+
}
183+
184+
public function callApi(
185+
ApiMethodSummaryRecord $resolvedMethod,
186+
?array $parameters = null
187+
): ApiCallRecord {
188+
$this->captured->values = [
189+
'resolvedMethod' => $resolvedMethod,
190+
'parameters' => $parameters,
191+
];
192+
193+
return new ApiCallRecord('6.0.0', $resolvedMethod);
194+
}
195+
},
196+
$this->createMethodSummaryQueryServiceStub($record),
197+
$this->createSystemSettingsStub('read')
198+
);
199+
200+
$actual = $tool->call(method: 'API.getMatomoVersion', parameters: []);
201+
202+
self::assertSame('6.0.0', $actual['result']);
203+
/** @var array<string, mixed> $capturedValues */
204+
$capturedValues = $captured->values;
205+
self::assertSame([], $capturedValues['parameters']);
206+
}
207+
208+
public function testCallRejectsNonEmptyParameterList(): void
209+
{
210+
$record = new ApiMethodSummaryRecord(
211+
'API',
212+
'getMatomoVersion',
213+
'API.getMatomoVersion',
214+
[],
215+
ApiMethodOperationClassifier::CATEGORY_READ,
216+
ApiMethodOperationClassifier::CONFIDENCE_HIGH,
217+
'action-prefix:get'
218+
);
219+
220+
$tool = new ApiCallRead(
221+
$this->createMock(ApiCallQueryServiceInterface::class),
222+
$this->createMethodSummaryQueryServiceStub($record),
223+
$this->createSystemSettingsStub('read')
224+
);
225+
226+
$this->expectException(ToolCallException::class);
227+
$this->expectExceptionMessage('parameters is invalid.');
228+
229+
/** @phpstan-ignore-next-line intentional invalid direct invocation coverage */
230+
$tool->call(method: 'API.getMatomoVersion', parameters: ['flat']);
231+
}
232+
163233
public function testScopedToolRejectsMethodOutsideExpectedOperationCategory(): void
164234
{
165235
$tool = new ApiCallRead(

0 commit comments

Comments
 (0)