Repository navigation
[Server] Log the tool name at info for tools/call - #548
Open
aton-of-data wants to merge 1 commit into
Open
aton-of-data wants to merge 1 commit into
aton-of-data wants to merge 1 commit into
Conversation
Since payloads moved to debug level, nothing at info said which tool a tools/call request ran: CallToolHandler only logs the name at debug. A tool name is server-defined, not user content, so the "Handling request." info record now carries it under the same "name" key CallToolHandler uses, while the arguments stay at debug. Refs modelcontextprotocol#524
aton-of-data
requested review from
CodeWithKyrian,
Nyholm,
chr-hertel and
soyuka
as code owners
October 7, 2026 21:40
chr-hertel
requested changes
Oct 7, 2026
Comment on lines
+263
to
+265
| if ($request instanceof CallToolRequest) { | ||
| $context['name'] = $request->name; | ||
| } |
Member
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
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #525, as @mglaman suggested in his review.
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. A tool name is defined by the server, not supplied by the user, so it is safe to log at info. The arguments stay at debug.Change
src/Server/Protocol.php:261(handleRequest()): when the request is aCallToolRequest, the info context also gets'name' => $request->name, the same keyCallToolHandleruses. Other requests log the same context as before. CHANGELOG entry added, placed next to the otherProtocoland event entries so it merges into the current0.9.0list without a conflict.Tests
New test:
ProtocolTest::testToolCallRequestIsLoggedWithToolNameAtInfoLevel. It sends atools/callwith a secret argument and checks the info records.Protocol.phpchange it fails:Failed asserting that '[[],{"method":"tools/call","request_id":1},{"response_id":1}]' [ASCII](length: 61) contains ""method":"tools/call","request_id":1,"name":"login"" [ASCII](length: 51).OK (1 test, 2 assertions)Tests: 1649, Assertions: 4300, Skipped: 4. With the change onae72b2c:Tests: 1650, Assertions: 4302, Skipped: 4.vendor/bin/phpstan --memory-limit=-1(2.2.17):[OK] No errorsvendor/bin/php-cs-fixer fix --diff --verbose --dry-run(3.95.27):Found 0 of 602 files that can be fixedNot run: the integration and interop suites, and the conformance tests. In my sandbox the integration HTTP tests time out on
initializeon main as well, so they could not tell me anything. I ran everything on PHP 8.3.6.Branch base: this branch sits on #525's merge commit (
eee5836), not on currentmain. My fork token can't push the workflow change from #538, so I can't bring the fork up to date. A local trial merge intomainatae72b2capplies cleanly, and the mergedsrc/andtests/diff is byte-identical to the one I tested onae72b2cabove.Built with Claude (AI) — code, test and this description; I verified everything listed above by running it.