Repository navigation
[Server] Log the tool name at info for tools/call - #548
aton-of-data wants to merge 1 commit into
Conversation
| if ($request instanceof CallToolRequest) { | ||
| $context['name'] = $request->name; | ||
| } |
There was a problem hiding this comment.
Sorry, let's not start to bring in method specific data on this level - we need a different solution or let that topic go
chr-hertel
added
Server
|
Understood, Protocol should stay method-agnostic. A different place for it: |
Since modelcontextprotocol#525 the info-level "Handling request." record in Protocol carries only the method and id, and CallToolHandler logs the called tool at debug level only, so a server logging at INFO no longer shows which tool ran. CallToolHandler now logs the tool name at info level. The arguments stay in the existing debug record, and Protocol stays method-agnostic.
b7ce7b3 to
cd86d80
Compare
|
Reworked: the |
This branch has not been deployed
2 participants
Follow-up to #525, as @mglaman suggested in his review. Reworked after review:
Protocolstays method-agnostic, and the change now lives in the handler that knows about tools.After #525 the info-level "Handling request." record carries only the method and id.
CallToolHandlerlogs the tool name at debug only, so a server logging at INFO has no record of which tool ran.Change
CallToolHandler::handle()now logs'Calling tool'at info level, with only['name' => $toolName]as context. The existing debug'Executing tool'record, which carries the arguments, is unchanged, so arguments still never reach info (#524).Protocol.phpis not touched. There is also a CHANGELOG line.Tests
New test:
CallToolHandlerTest::testToolNameIsLoggedAtInfoLevelAndArgumentsOnlyAtDebugLevel. It asserts there is exactly one info record, with context['name' => 'login'], and that every record carryingargumentsis at debug level.CallToolHandlerTest.php:316withFailed asserting that two arrays are identical, because there is no info record. With the change:OK (1 test, 3 assertions).mainatb35b52d: 1800 tests before, 1801 after, OK, 4 skipped.vendor/bin/phpstan --memory-limit=-1:[OK] No errors.vendor/bin/php-cs-fixer fix --dry-run --diff: 0 of 607 files fixable.Not run: the integration, interop and conformance suites. Everything ran on PHP 8.3.6 only.
Branch base: this branch sits on #525's merge commit (
eee5836), not on currentmain, because my fork token can't push the workflow changes from #538 and #540. The trial merge intob35b52dis clean, and the checks above ran on that merged tree.