[Server] Fix wrong parameter order when creating the tool from array #147
+13
−2
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.
Summary
Fix Tool constructor call in ArrayLoader where the
metaparameter was incorrectly passed to theiconsposition due to missingiconsargument. Also addsiconssupport toBuilder::addTool()for consistency with the Tool schema.Motivation and Context
The
Toolclass constructor expects 6 parameters:name,inputSchema,description,annotations,icons, andmeta. However,ArrayLoaderwas only passing 5 arguments, causing themetavalue to be assigned to theiconsparameter instead. This bug would prevent metadata from being properly attached to manually registered tools.Additionally, the
Builder::addTool()method didn't support theiconsparameter, even though the Tool schema includes it. This inconsistency meant users couldn't specify icons when manually registering tools.How Has This Been Tested?
compact()call in Builder includes all necessary keysBreaking Changes
Potentially breaking: The
Builder::addTool()method signature has changed. A newiconsparameter has been added betweeninputSchemaandmeta.Impact: If users were calling
addTool()with positional arguments and passingmeta, they will need to addnullfor theiconsparameter or switch to named arguments.Before:
->addTool($handler, 'name', 'desc', $annotations, $schema, $meta)After (positional):
->addTool($handler, 'name', 'desc', $annotations, $schema, null, $meta)After (named - recommended):
->addTool($handler, name: 'name', meta: $meta)## Types of changes
Checklist
Additional context
Changes made:
ArrayLoader::load()to pass all 6 parameters to Tool constructor using named argumentsannotationsparameter to prevent undefined key warningsiconsparameter toBuilder::addTool()method signatureiconsThe use of named arguments makes the code more maintainable and prevents similar parameter ordering bugs in the future.