GH-2588 Add name attribite to @ToolParam

[Grandys]

Fixes #2588
Adds explicit parameter name support for @ToolParam and @McpToolParam, allowing stable tool schema property names and argument binding even when Java parameter metadata is unavailable.

Also adds duplicate effective-name validation for tool method parameters, keeps schema generation and invocation lookup aligned, updates documentation, and covers core and MCP paths with tests.

[kuntal1461]

Code Review — GH-2588 Add name attribute to @ToolParam

The core feature is implemented correctly and the approach is sound. Schema generation and invocation both use the same getParameterName() call so the LLM-visible name and the binding key stay aligned. Backward compatibility is preserved via the "" default. A few issues worth addressing before merge:

---

Critical

1. name() is silently ignored when @ToolParam is used on a field or annotation type

@ToolParam targets PARAMETER, FIELD, and ANNOTATION_TYPE, but ToolMethodParameterUtils.getParameterName() only reads name() for method parameters. A developer who writes @ToolParam(name = "foo") on a record field gets no rename and no error — silent misbehaviour.

The name() Javadoc should be explicit:

/**
 * The name of the tool argument. Only effective on method parameters.
 * For fields or record components, use {@link com.fasterxml.jackson.annotation.JsonProperty}
 * or {@code @Schema} to customize schema property names.
 */
String name() default "";

Same change needed in McpToolParam.

---

2. Duplicate validation fires twice in the normal tool-registration flow

MethodToolCallbackProvider.getToolCallbacks() calls ToolDefinitions.from(method) → JsonSchemaGenerator.generateForMethodInput() which runs validateUniqueParameterNames() (call #1). Then MethodToolCallback is built and its constructor runs validateUniqueParameterNames() again on the same method (call #2). The same duplication exists in the MCP path (AbstractMcpToolMethodCallback constructor + McpJsonSchemaGenerator).

Not a correctness bug but a design smell — same check twice with no benefit. Please pick one location: either the constructor (fail-fast at object construction) or schema gen (covers standalone usage too), not both.

---

Moderate

3. validateUniqueParameterNames body is copy-pasted between the two utility classes

ToolMethodParameterUtils and McpToolMethodParameterUtils have byte-for-byte identical loop implementations. Since mcp-annotations already depends on spring-ai-model, McpToolMethodParameterUtils should delegate:

public static void validateUniqueParameterNames(Method method,
        Predicate<Parameter> infrastructureParameterPredicate) {
    ToolMethodParameterUtils.validateUniqueParameterNames(method, infrastructureParameterPredicate);
}

---

4. McpToolMethodParameterUtils is not covered by @NullMarked

The tool/support package has @NullMarked in its package-info.java so ToolMethodParameterUtils is protected. The mcp/annotation/method/tool package-info does not have @NullMarked, leaving McpToolMethodParameterUtils unguarded under JSpecify — inconsistent with the rest of the project.

---

5. Documentation examples use name redundantly where Java names already match

In both MCP adoc files the examples now show:

@McpToolParam(name = "a", description = "First number", required = true) int a,
@McpToolParam(name = "b", description = "Second number", required = true) int b

The parameter is already named a/b in Java — name = "a" adds nothing and implies name is always required. These examples should either drop the name attribute (it is not needed here) or use a meaningful override (e.g., name = "first_number" on a parameter called a) to demonstrate when the attribute is actually useful.

---

6. New public utility classes are missing @since tags

ToolMethodParameterUtils and McpToolMethodParameterUtils have @author but no @since 1.0.0, unlike other public classes in the project.

---

Minor

7. McpToolMethodParameterUtils.isInfrastructureParameter() silently excludes server exchange types

The method name implies completeness but it deliberately does not cover McpSyncServerExchange/McpAsyncServerExchange (callers handle those). The class Javadoc explains this at the class level but not on the method itself. A one-line Javadoc note on the method would prevent a future maintainer from trusting it as a complete check and silently introducing a bug.

---

8. No invocation test for a simple named String parameter

The only invocation test that exercises @ToolParam(name) binding uses Map<String, Integer> (named_map). A simple @ToolParam(name = "customer_id") String customerId invocation test directly matches the issue reporter's scenario and would round out coverage.

---

What is done well

• Blank-name fallback via StringUtils.hasText() is correct — "" and " " both fall back to the reflective name
• Fail-fast duplicate detection at construction time is the right place to catch programmer errors
• name placed first in the annotation — consistent with @Tool and @McpTool attribute ordering
• Both spring-ai-model and mcp-annotations paths updated symmetrically
• All new tests use AssertJ — matches project standard
• Fully backward compatible

[kuntal1461]

Inline file-by-file review comments for GH-2588 / PR #6326. Each finding is pinned to the relevant line.

[kuntal1461]

[Critical #1] name() is silently ignored on fields and annotation-type uses

@McpToolParam targets PARAMETER, FIELD, and ANNOTATION_TYPE. But McpToolMethodParameterUtils.getParameterName() only reads name() for method parameters. A developer who writes @McpToolParam(name = "foo") on a field gets no rename and no error — silent misbehaviour.

Please update the Javadoc:

/**
 * The name of the tool argument. Only effective on method parameters.
 * For fields or record components, use {@literal @}JsonProperty or {@literal @}Schema instead.
 */
String name() default "";

[kuntal1461]

[Critical #1] Same issue as in McpToolParam — name() is silently ignored on fields

@ToolParam also targets FIELD and ANNOTATION_TYPE, but ToolMethodParameterUtils.getParameterName() only reads name() for method parameters. Using @ToolParam(name = "foo") on a record field does nothing — no rename, no warning.

Please update the Javadoc to say it is only effective on method parameters, and direct field/record users to @JsonProperty or @Schema.

[kuntal1461]

[Critical #2] Duplicate validation — same check fires twice in normal tool-registration flow

In the normal path MethodToolCallbackProvider.getToolCallbacks() → ToolDefinitions.from(method) → JsonSchemaGenerator.generateForMethodInput() already calls validateUniqueParameterNames(). Then this constructor fires it again on the same method.

Please pick one location — either the constructor (fail-fast at object construction) or generateForMethodInput() (covers standalone schema gen too) — not both. Having both with no rationale is a maintenance smell.

[kuntal1461]

[Critical #2 continued] Second call-site for the duplicated validation

This is the other half of the double-validation issue flagged at MethodToolCallback line 84. When the full tool-registration path runs, this call and the constructor call both execute on the same method. Choose one authoritative location.

[kuntal1461]

Moderate and minor issues — continued from the critical issues review above.

[kuntal1461]

[Moderate #3] validateUniqueParameterNames is copy-pasted from ToolMethodParameterUtils

The loop body here is byte-for-byte identical to ToolMethodParameterUtils.validateUniqueParameterNames(). Since mcp-annotations already depends on spring-ai-model, this method should simply delegate instead of reimplementing:

public static void validateUniqueParameterNames(Method method,
        Predicate<Parameter> infrastructureParameterPredicate) {
    ToolMethodParameterUtils.validateUniqueParameterNames(method, infrastructureParameterPredicate);
}

[kuntal1461]

[Moderate #4 & #6] Missing @NullMarked coverage and missing @since tag

Two issues here:

1. The tool/support package has @NullMarked in its package-info.java, so ToolMethodParameterUtils is protected. The mcp/annotation/method/tool package-info.java does not have @NullMarked, leaving this class unguarded under JSpecify. Please add @NullMarked to the package-info or to this class directly.
2. Other public classes in the project carry @since 1.0.0. Please add it here.

[kuntal1461]

[Minor #7] Method name isInfrastructureParameter overpromises — silently excludes server exchange types

This method deliberately does not cover McpSyncServerExchange or McpAsyncServerExchange (those are handled by callers). The class-level Javadoc explains the design, but this method has no documentation. A future maintainer will assume the method is complete and may introduce a subtle bug by relying on it exclusively.

Please add a brief note:

/**
 * Returns true for common MCP infrastructure parameters (request contexts, progress tokens,
 * McpMeta, CallToolRequest, McpTransportContext).
 * Does NOT include server exchange types (McpSyncServerExchange, McpAsyncServerExchange);
 * callers must handle those separately.
 */

[kuntal1461]

[Critical #2 — MCP path] Same double-validation issue as in MethodToolCallback

McpJsonSchemaGenerator.internalGenerateFromMethodArguments() already calls validateUniqueParameterNames() during schema generation. This constructor call duplicates that check on the same method. Pick one authoritative location.

[kuntal1461]

Remaining issues: missing @SInCE, redundant name in docs, and missing invocation test.

[kuntal1461]

[Moderate #6] Missing @since tag

Other public classes in the project carry @since 1.0.0. Please add it to match project convention:

 * @author Michał Grandys
 * @since 1.0.0

[kuntal1461]

[Moderate #5] Redundant name attribute in documentation examples

The Java parameters are already named a, b, x, y — adding name = "a" when the name already matches adds nothing and misleads readers into thinking name is always required.

The whole point of this feature is to override names when they differ (e.g., a parameter named a in Java that should appear as first_number in the schema). Please either:

• Remove name from these examples (it is not needed here), or
• Use a realistic override — e.g., rename int a → @McpToolParam(name = "first_number") int a — to demonstrate the actual value of the attribute.

[kuntal1461]

[Moderate #5 continued] Same redundant name issue in server adoc

Same feedback as mcp-annotations-overview.adoc: name = "a" and name = "b" are redundant because the Java parameter names already match. This example teaches the wrong lesson. Show a case where the override is actually needed.

[kuntal1461]

[Minor #8] No invocation test for a simple named String parameter

This is the only invocation test that exercises @ToolParam(name) argument binding, and it uses a Map<String, Integer> (a generic type). While this works, the canonical use case from the issue is a simple type like String. A companion test with @ToolParam(name = "customer_id") String customerId called via {"customer_id": "abc"} would directly cover the reporter's scenario and round out the test suite.