feat(java): create @CopilotTool and @Param annotations#1763
Conversation
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Generated by SDK Consistency Review Agent for issue #1763 · sonnet46 1.6M
170d5aa to
2a9bdb4
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Pull request overview
This PR adds a new annotation-based API surface to the Java SDK for defining tools via @CopilotTool (method-level) and @Param (parameter-level), and exports the new com.github.copilot.tool package from the Java module. It also introduces a ToolDefer.NONE enum constant intended as an annotation default, plus unit tests validating the annotations’ retention/targets/defaults via reflection.
Changes:
- Added
@CopilotTooland@Paramannotations undercom.github.copilot.tool(runtime-retained) for future tool metadata discovery. - Exported
com.github.copilot.toolfrommodule-info.javato make the annotations available to consumers. - Added
ToolDefer.NONEas an annotation sentinel and added unit tests for annotation metadata.
Show a summary per file
| File | Description |
|---|---|
| java/src/test/java/com/github/copilot/tool/CopilotToolAnnotationTest.java | Adds reflection-based tests validating retention, targets, defaults, and sample usage of the new annotations. |
| java/src/main/java/module-info.java | Exports the new com.github.copilot.tool package. |
| java/src/main/java/com/github/copilot/tool/Param.java | Introduces @Param annotation for parameter metadata. |
| java/src/main/java/com/github/copilot/tool/CopilotTool.java | Introduces @CopilotTool annotation for tool methods (marked experimental). |
| java/src/main/java/com/github/copilot/rpc/ToolDefer.java | Adds ToolDefer.NONE and changes serialization behavior for the enum. |
Copilot's findings
- Files reviewed: 5/5 changed files
- Comments generated: 5
This comment has been minimized.
This comment has been minimized.
Clarify that @jsonvalue returning null does not cause field omission by @JsonInclude(NON_NULL) — it only changes the leak from "" to null. The primary protection is mapping NONE to a null field reference before constructing ToolDefinition (responsibility of the annotation processor and ToolDefinition.fromObject()).
This comment has been minimized.
This comment has been minimized.
Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This reverts commit a4fe9b2.
Cross-SDK Consistency Review ✅This PR adds Attribute parity — consistent with all SDKs
Parameter metadata — consistent approach
|
* WIP Phase 4.1 * Remove prompts, pre-merge * fix(java): correct ToolDefer.NONE Javadoc on @jsonvalue null semantics Clarify that @jsonvalue returning null does not cause field omission by @JsonInclude(NON_NULL) — it only changes the leak from "" to null. The primary protection is mapping NONE to a null field reference before constructing ToolDefinition (responsibility of the annotation processor and ToolDefinition.fromObject()). * fix(java): address three review comments Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * Revert "Remove prompts, pre-merge" This reverts commit a4fe9b2. --------- Co-authored-by: Ed Burns <edburns@microsoft.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
* WIP Phase 4.1 * Remove prompts, pre-merge * fix(java): correct ToolDefer.NONE Javadoc on @jsonvalue null semantics Clarify that @jsonvalue returning null does not cause field omission by @JsonInclude(NON_NULL) — it only changes the leak from "" to null. The primary protection is mapping NONE to a null field reference before constructing ToolDefinition (responsibility of the annotation processor and ToolDefinition.fromObject()). * fix(java): address three review comments Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * Revert "Remove prompts, pre-merge" This reverts commit a4fe9b2. --------- Co-authored-by: Ed Burns <edburns@microsoft.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
…1777) * Resume 1682 iterating * Phase 03 answer questions * On branch edburns/1682-java-tool-ergonomics Your branch is up to date with 'upstream/edburns/1682-java-tool-ergonomics'. Changes to be committed: (use "git restore --staged <file>..." to unstage) new file: 1682-java-tool-ergonomics-prompts-remove-before-merge/20260618-prompts.md Signed-off-by: Ed Burns <edburns@microsoft.com> * WIP: Phase 3. Question 3.4 * WIP: Phase 3. Question 3.6 * WIP: Phase 3. Question 3.6: Answer * Answer 3.7 * Resolve 3.8 * Initial plan * feat(java): create @copilotTool and @Param annotations with tests - Add NONE constant to ToolDefer enum for annotation default value - Create com.github.copilot.tool.CopilotTool annotation - Create com.github.copilot.tool.Param annotation - Export com.github.copilot.tool package in module-info.java - Add CopilotToolAnnotationTest verifying retention, targets, defaults Closes #1758 * spotless * fix(java): make ToolDefer.NONE serialize as null to prevent wire leak NONE is an annotation-only sentinel for @copilotTool(defer=...) defaults. Its @jsonvalue now returns null so @JsonInclude(NON_NULL) omits it from the JSON-RPC payload, matching the nullable/optional semantics used by all other SDKs (.NET CopilotToolDefer?, Node defer?, Go omitempty, Python | None, Rust Option<DeferMode>). * WIP Phase 4.1 * feat(java): create @copilotTool and @Param annotations (#1763) * WIP Phase 4.1 * Remove prompts, pre-merge * fix(java): correct ToolDefer.NONE Javadoc on @jsonvalue null semantics Clarify that @jsonvalue returning null does not cause field omission by @JsonInclude(NON_NULL) — it only changes the leak from "" to null. The primary protection is mapping NONE to a null field reference before constructing ToolDefinition (responsibility of the annotation processor and ToolDefinition.fromObject()). * fix(java): address three review comments Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * Revert "Remove prompts, pre-merge" This reverts commit a4fe9b2. --------- Co-authored-by: Ed Burns <edburns@microsoft.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * Initial plan * feat(java): add SchemaGenerator compile-time type-to-JSON-Schema utility (#1766) * Initial plan * feat(java): add SchemaGenerator compile-time type-to-JSON-Schema utility Creates SchemaGenerator.java that maps javax.lang.model TypeMirror instances to JSON Schema source code literals (Map.of(...) expressions). Implements all 24 type mappings from the specification including: - Primitives and boxed types (int/Integer, long/Long, etc.) - String, UUID, OffsetDateTime - Collections (List<T>, Collection<T>, Set<T>) - Maps (Map<String, V> with typed values) - Arrays (String[]) - Enums (with constant enumeration) - Records and POJOs (with properties/required) - Optional<T>, OptionalInt, OptionalDouble - Sealed interfaces (oneOf) - JsonNode and Object (any) Also adds SchemaGeneratorTest using compilation-testing approach with javax.tools.JavaCompiler to exercise the generator at compile time. Closes #1759 * fix: address code review - remove unused param, handle all primitive types * fix(java): correct SimpleJavaFileObject override - getCharContent not getContent Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * spotless * Remove .class files generated by test * spotless * fix: use Map.ofEntries for properties to avoid Map.of 10-entry limit Address review comment r3461777483: Map.of() only supports up to 10 key-value pairs. Switch properties maps in SchemaGenerator to use Map.ofEntries(Map.entry(...), ...) so records/POJOs/methods with >10 fields won't cause generated source compilation failures. Update SchemaGeneratorTest expectations to match the new format. * fix: add missing Byte/Short/Character boxed type mappings Address review comment r3461777428: Byte and Short now map to "integer", Character maps to "string", matching their primitive equivalents. Add tests for all three. * fix: add missing OptionalLong mapping in generateDeclaredTypeSchema Address review comment r3461777459: OptionalLong was handled in isOptionalType/unwrapOptional but missing from generateDeclaredTypeSchema, causing it to fall through to POJO introspection when used as a direct return type. Add the mapping and tests for OptionalInt, OptionalLong, and OptionalDouble. * fix: correct misleading @JsonSubTypes comment on sealed interface handling Address review comment r3461777579: the implementation uses getPermittedSubclasses() (Java sealed types), not Jackson annotations. * test: add sealed interface test for oneOf schema generation Address review comment r3461777685: the processor had special handling for TestSealed* types but no test exercised generateSealedSchema(). Add a test with a sealed interface (TestSealedShape) and two record permits (Circle, Rect) verifying the oneOf schema output. * test: add >10-field record test proving Map.ofEntries compiles Address review comment r3461777706: add a test with an 11-component record that verifies the generated Map.ofEntries(...) expression actually compiles, proving the Map.of 10-entry limit fix works end-to-end. --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com> Co-authored-by: Ed Burns <edburns@microsoft.com> * WIP 4.3 * Initial plan * feat(java): Add CopilotToolProcessor annotation processor (task 4.3) Implements JSR 269 annotation processor that finds @CopilotTool-annotated methods and generates $$CopilotToolMeta companion classes containing tool definitions, JSON Schema, and invocation lambdas. Key features: - snake_case tool name conversion from camelCase method names - Access level enforcement (compile error for private methods) - Return type handling (String, void, CompletableFuture<String>, etc.) - Argument deserialization (direct cast for primitives/String, convertValue for complex) - @Param description and defaultValue support in schema - ToolDefer support (NONE maps to null/regular create) - overridesBuiltInTool and skipPermission support Also includes comprehensive test suite using javax.tools.JavaCompiler programmatic compilation. Closes #1760 Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * fix: Address code review feedback - Use fully qualified type names in generated code for type safety - Fix Files.walk() resource leak in test with try-with-resources - Rename exception variables for clarity Co-authored-by: edburns <75821+edburns@users.noreply.github.com> * fix: Fix Spotless formatting and test classpath for JDK 17 - Remove unused Collections import - Reformat boolean expressions: && at start of continuation lines - Reformat ternary: ? at start of continuation line - Reformat .replace() chain with one call per line - Fix hasErrorContaining stream method chain formatting - Fix resolveClasspath() to use System.getProperty("java.class.path") first, ensuring Jackson and all test deps are available when compiling generated $$CopilotToolMeta code * fix: Fix remaining Spotless violations and test classpath resolution - Merge propertyEntries.add() onto one line per formatter requirement - Fix sb.append() chain formatting to match Eclipse formatter output - Revert escapeJava to original line-breaking style (formatter preference) - Fix resolveClasspath() to combine system classpath with CodeSource paths from key classes (SDK, Jackson, RPC types) ensuring all dependencies are available for javac in the annotation processor test * fix: Add jackson-core and jackson-annotations to test classpath The generated 6342CopilotToolMeta code uses ObjectMapper which requires jackson-core (Versioned, JsonFactory) and jackson-annotations at compile time. Add these transitive dependencies to the key classes list so their CodeSource paths are included in the javac classpath. * fix: Fix Spotless formatting for keyClasses array initializer * fix(java): Pass ObjectMapper as parameter in generated $$CopilotToolMeta contract Address PR #1777 review comment (r3463252393): the generated $$CopilotToolMeta class was using `new ObjectMapper()`, which lacks the SDK Jackson configuration (JavaTimeModule, NON_NULL inclusion, lenient unknown-properties). This would break tool argument coercion and return serialization at runtime for java.time.* and other types. Instead of embedding a bare or configured ObjectMapper in the generated code, change the generated `definitions()` method signature from: definitions(MyTools instance) to: definitions(MyTools instance, ObjectMapper mapper) This establishes an internal contract: the caller (the future ToolDefinition.fromObject() in issue #1761) is responsible for supplying a properly configured mapper via reflective invocation. The generated code uses `mapper` for all convertValue() and writeValueAsString() calls. Benefits: - No DRY violation (mapper config stays canonical in JsonRpcClient) - No new public API exposing ObjectMapper - No package-visibility workarounds - Clean separation: generated code declares its needs, caller supplies Issue #1761 description has been updated to document this contract so the implementing agent knows to pass ObjectMapper as the second argument when reflectively invoking definitions(). * fix(java): restrict single-param shortcut to records only Address review comment on PR #1777: the isRecordOrPojo heuristic incorrectly triggered for JDK container types (List, Map, etc.) when used as a single tool parameter. For example, a tool with parameter List<String> would attempt to deserialize the entire arguments object as a List, failing at runtime. Replace the heuristic with a deterministic check: only Java records qualify for the getArgumentsAs() shortcut. Records are immutable data carriers with compiler-guaranteed component lists, making them safe for whole-object deserialization. POJOs and all other class types now fall through to the per-field extraction path, which always works correctly. Removed isSimpleType() helper which was only used by the old heuristic. * fix(java): emit typed default values in JSON Schema Address review comment on PR #1777: @Param(defaultValue=...) was always emitted as a JSON string in the generated schema's 'default' field, making numeric and boolean defaults the wrong type (e.g., "10" instead of 10, "true" instead of true). Changes: - withMeta helper: String defaultValue -> Object defaultValue - buildPropertySchema: reuse generateDefaultLiteral() to emit typed Java literals (int, boolean, etc.) instead of always quoting - Add test emitsTypedDefaultValuesInSchema verifying int -> 10, boolean -> true, String -> "hello" in generated code * fix(java): fix double 61059CopilotToolMeta suffix in test helper Address review comment on PR #1777: getGeneratedSource() fallback search appended 61059CopilotToolMeta to a simpleName that already contained it, producing MyTools$$CopilotToolMeta$$CopilotToolMeta. Simplify to just match on 'class <simpleName>'. * fix(java): use record constructor for independent flag combination Address SDK Consistency Review on PR #1777: the if/else if chain in writeToolDefinition silently dropped combined annotation flags (e.g., overridesBuiltInTool + skipPermission + defer). All other SDKs support combining these flags simultaneously. Replace the factory method dispatch with a direct call to the ToolDefinition record constructor, which accepts all seven fields independently. Each flag is now emitted as its own argument: Boolean.TRUE or null for overridesBuiltInTool/skipPermission, ToolDefer.X or null for defer. Add test generatesCombinedFlags verifying all three flags appear in generated code when set together. --------- Signed-off-by: Ed Burns <edburns@microsoft.com> Co-authored-by: Ed Burns <edburns@microsoft.com> Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
Introduces annotation-based tool definition support for the Java SDK via
@CopilotTooland@Paramin a newcom.github.copilot.toolpackage.Changes
@CopilotToolannotation —RUNTIMEretention,METHODtarget, marked@CopilotExperimental. Attributes:value()(description),name(),overridesBuiltInTool(),skipPermission(),defer().@Paramannotation —RUNTIMEretention,PARAMETERtarget. Attributes:value()(description),name(),required(),defaultValue().ToolDefer.NONE— Added as new enum constant (empty string value) to serve as the annotation default fordefer(), representing "no preference set".exports com.github.copilot.tool;added tomodule-info.java.Usage