Add opaque metadata passthrough to SDK tool definitions
#1864
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -43,6 +43,34 @@ public void DefineTool_Omits_Copilot_Metadata_When_Flags_Are_False() | |
| Assert.False(function.AdditionalProperties.ContainsKey("defer")); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DefineTool_Sets_Metadata_In_Additional_Properties() | ||
| { | ||
| var metadata = new Dictionary<string, object> | ||
| { | ||
| ["github.com/copilot:safeForTelemetry"] = new Dictionary<string, object> | ||
| { | ||
| ["name"] = true, | ||
| ["inputsNames"] = false | ||
| } | ||
| }; | ||
|
|
||
| var function = CopilotTool.DefineTool( | ||
| ReturnsOk, | ||
| new CopilotToolOptions { Metadata = metadata }); | ||
|
|
||
| Assert.True(function.AdditionalProperties.TryGetValue("metadata", out var value)); | ||
| Assert.Same(metadata, value); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DefineTool_Omits_Metadata_When_Unset() | ||
| { | ||
| var function = CopilotTool.DefineTool(ReturnsOk); | ||
|
|
||
| Assert.False(function.AdditionalProperties.ContainsKey("metadata")); | ||
| } | ||
|
|
||
| [Fact] | ||
| public void DefineTool_Accepts_Lambda_Handlers_Without_Casts() | ||
| { | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1160,6 +1160,53 @@ func TestToolDefer(t *testing.T) { | |
| }) | ||
| } | ||
|
|
||
| func TestToolMetadata(t *testing.T) { | ||
| t.Run("Metadata is serialized in tool definition", func(t *testing.T) { | ||
| tool := Tool{ | ||
| Name: "my_tool", | ||
| Description: "A custom tool", | ||
| Metadata: map[string]any{ | ||
| "github.com/copilot:safeForTelemetry": map[string]any{"name": true, "inputsNames": false}, | ||
| }, | ||
| Handler: func(_ ToolInvocation) (ToolResult, error) { return ToolResult{}, nil }, | ||
| } | ||
| data, err := json.Marshal(tool) | ||
| if err != nil { | ||
| t.Fatalf("failed to marshal: %v", err) | ||
| } | ||
| var m map[string]any | ||
| if err := json.Unmarshal(data, &m); err != nil { | ||
| t.Fatalf("failed to unmarshal: %v", err) | ||
| } | ||
| meta, ok := m["metadata"].(map[string]any) | ||
| if !ok { | ||
| t.Fatalf("expected metadata object, got %v", m) | ||
| } | ||
| if _, ok := meta["github.com/copilot:safeForTelemetry"]; !ok { | ||
| t.Errorf("expected namespaced key preserved, got %v", meta) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("Metadata omitted when unset", func(t *testing.T) { | ||
| tool := Tool{ | ||
| Name: "custom_tool", | ||
| Description: "A custom tool", | ||
| Handler: func(_ ToolInvocation) (ToolResult, error) { return ToolResult{}, nil }, | ||
| } | ||
| data, err := json.Marshal(tool) | ||
| if err != nil { | ||
| t.Fatalf("failed to marshal: %v", err) | ||
| } | ||
| var m map[string]any | ||
| if err := json.Unmarshal(data, &m); err != nil { | ||
| t.Fatalf("failed to unmarshal: %v", err) | ||
| } | ||
| if _, ok := m["metadata"]; ok { | ||
| t.Errorf("expected metadata to be omitted, got %v", m) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| func TestClient_CreateSession_AllowsMissingPermissionHandler(t *testing.T) { | ||
| t.Run("accepts nil config before connection validation", func(t *testing.T) { | ||
| client := NewClient(&ClientOptions{Connection: StdioConnection{Path: "/__nonexistent_copilot_binary__"}}) | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1269,6 +1269,12 @@ type Tool struct { | |
| // Defer controls whether the tool may be deferred (loaded lazily via tool | ||
| // search) rather than always pre-loaded. When empty, the runtime decides. | ||
| Defer ToolDefer `json:"defer,omitempty"` | ||
| // Metadata is opaque, host-defined metadata associated with the tool | ||
| // definition. Keys are namespaced and not part of the stable public API; | ||
| // the SDK forwards them verbatim to the runtime, which may recognize | ||
| // specific keys to inform host-specific behavior. Unknown keys are | ||
| // preserved and round-tripped untouched. | ||
|
Contributor
There was a problem hiding this comment. Same comment about not mentioning runtime here as if it's a separate thing
Contributor
There was a problem hiding this comment. There are other places in this PR too but I'll not comment on the others - I'm sure you can find them :) |
||
| Metadata map[string]any `json:"metadata,omitempty"` | ||
| // Handler is optional. When nil, the SDK exposes the tool declaration but does | ||
| // not automatically invoke it. | ||
| Handler ToolHandler `json:"-"` | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -75,6 +75,10 @@ | |
| * controls whether the tool may be deferred (loaded lazily via tool | ||
| * search) rather than always pre-loaded; {@code null} lets the | ||
| * runtime decide | ||
| * @param metadata | ||
| * opaque, host-defined metadata forwarded verbatim to the runtime; | ||
| * keys are namespaced and not part of the stable public API; | ||
| * {@code null} when unset | ||
| * @see SessionConfig#setTools(java.util.List) | ||
| * @see ToolHandler | ||
| * @since 1.0.0 | ||
|
|
@@ -83,7 +87,8 @@ | |
| public record ToolDefinition(@JsonProperty("name") String name, @JsonProperty("description") String description, | ||
| @JsonProperty("parameters") Object parameters, @JsonIgnore ToolHandler handler, | ||
| @JsonProperty("overridesBuiltInTool") Boolean overridesBuiltInTool, | ||
| @JsonProperty("skipPermission") Boolean skipPermission, @JsonProperty("defer") ToolDefer defer, | ||
| @JsonProperty("metadata") Map<String, Object> metadata) { | ||
|
Contributor
There was a problem hiding this comment. Seems like this would be a breaking change. @edburns What's the recommended way to handle this in Java? Should this be added purely as a setter and not a constructor parameter? Or should it be on a new constructor overload? |
||
|
|
||
| /** | ||
| * Creates a tool definition with a JSON schema for parameters. | ||
|
|
@@ -103,7 +108,7 @@ public record ToolDefinition(@JsonProperty("name") String name, @JsonProperty("d | |
| */ | ||
| public static ToolDefinition create(String name, String description, Map<String, Object> schema, | ||
| ToolHandler handler) { | ||
| return new ToolDefinition(name, description, schema, handler, null, null, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -127,7 +132,7 @@ public static ToolDefinition create(String name, String description, Map<String, | |
| */ | ||
| public static ToolDefinition createOverride(String name, String description, Map<String, Object> schema, | ||
| ToolHandler handler) { | ||
| return new ToolDefinition(name, description, schema, handler, true, null, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -150,7 +155,7 @@ public static ToolDefinition createOverride(String name, String description, Map | |
| */ | ||
| public static ToolDefinition createSkipPermission(String name, String description, Map<String, Object> schema, | ||
| ToolHandler handler) { | ||
| return new ToolDefinition(name, description, schema, handler, null, true, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -176,7 +181,32 @@ public static ToolDefinition createSkipPermission(String name, String descriptio | |
| */ | ||
| public static ToolDefinition createWithDefer(String name, String description, Map<String, Object> schema, | ||
| ToolHandler handler, ToolDefer defer) { | ||
| return new ToolDefinition(name, description, schema, handler, null, null, defer, null); | ||
| } | ||
|
|
||
| /** | ||
| * Creates a tool definition with opaque, host-defined metadata. | ||
| * <p> | ||
| * Use this factory method to attach namespaced metadata that the SDK forwards | ||
| * verbatim to the runtime. The keys are not part of the stable public API; the | ||
| * runtime may recognize specific keys to inform host-specific behavior. | ||
| * | ||
| * @param name | ||
| * the unique name of the tool | ||
| * @param description | ||
| * a description of what the tool does | ||
| * @param schema | ||
| * the JSON Schema as a {@code Map} | ||
| * @param handler | ||
| * the handler function to execute when invoked | ||
| * @param metadata | ||
| * the opaque metadata map forwarded to the runtime | ||
| * @return a new tool definition with the metadata set | ||
| * @since 1.0.7 | ||
| */ | ||
| public static ToolDefinition createWithMetadata(String name, String description, Map<String, Object> schema, | ||
| ToolHandler handler, Map<String, Object> metadata) { | ||
| return new ToolDefinition(name, description, schema, handler, null, null, null, metadata); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -247,7 +277,7 @@ public static List<ToolDefinition> fromClass(Class<?> clazz) { | |
| */ | ||
| @CopilotExperimental | ||
| public ToolDefinition overridesBuiltInTool(boolean value) { | ||
| return new ToolDefinition(name, description, parameters, handler, value, skipPermission, defer, metadata); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -261,7 +291,7 @@ public ToolDefinition overridesBuiltInTool(boolean value) { | |
| */ | ||
| @CopilotExperimental | ||
| public ToolDefinition skipPermission(boolean value) { | ||
| return new ToolDefinition(name, description, parameters, handler, overridesBuiltInTool, value, defer, metadata); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -275,7 +305,23 @@ public ToolDefinition skipPermission(boolean value) { | |
| */ | ||
| @CopilotExperimental | ||
| public ToolDefinition defer(ToolDefer value) { | ||
| return new ToolDefinition(name, description, parameters, handler, overridesBuiltInTool, skipPermission, value, | ||
| metadata); | ||
| } | ||
|
|
||
| /** | ||
| * Returns a copy with the opaque {@code metadata} bag set. | ||
| * | ||
| * @param value | ||
| * the opaque, host-defined metadata forwarded verbatim to the | ||
| * runtime; keys are namespaced and not part of the stable public API | ||
| * @return a new {@code ToolDefinition} with the metadata applied | ||
| * @since 1.0.7 | ||
| */ | ||
| @CopilotExperimental | ||
| public ToolDefinition metadata(Map<String, Object> value) { | ||
| return new ToolDefinition(name, description, parameters, handler, overridesBuiltInTool, skipPermission, defer, | ||
| value); | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------ | ||
|
|
@@ -319,7 +365,7 @@ public static <R> ToolDefinition from(String name, String description, Supplier< | |
| R result = handler.get(); | ||
| return CompletableFuture.completedFuture(formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -361,7 +407,7 @@ public static <T1, R> ToolDefinition from(String name, String description, Param | |
| R result = handler.apply(arg1); | ||
| return CompletableFuture.completedFuture(formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -409,7 +455,7 @@ public static <T1, T2, R> ToolDefinition from(String name, String description, P | |
| R result = handler.apply(arg1, arg2); | ||
| return CompletableFuture.completedFuture(formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------ | ||
|
|
@@ -458,7 +504,7 @@ public static <R> ToolDefinition fromAsync(String name, String description, | |
| } | ||
| return future.thenApply(result -> formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -506,7 +552,7 @@ public static <T1, R> ToolDefinition fromAsync(String name, String description, | |
| } | ||
| return future.thenApply(result -> formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -551,7 +597,7 @@ public static <T1, T2, R> ToolDefinition fromAsync(String name, String descripti | |
| } | ||
| return future.thenApply(result -> formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------ | ||
|
|
@@ -594,7 +640,7 @@ public static <R> ToolDefinition fromWithToolInvocation(String name, String desc | |
| R result = handler.apply(invocation); | ||
| return CompletableFuture.completedFuture(formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -640,7 +686,7 @@ public static <T1, R> ToolDefinition fromWithToolInvocation(String name, String | |
| R result = handler.apply(arg1, invocation); | ||
| return CompletableFuture.completedFuture(formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------ | ||
|
|
@@ -689,7 +735,7 @@ public static <R> ToolDefinition fromAsyncWithToolInvocation(String name, String | |
| } | ||
| return future.thenApply(result -> formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -741,7 +787,7 @@ public static <T1, R> ToolDefinition fromAsyncWithToolInvocation(String name, St | |
| } | ||
| return future.thenApply(result -> formatResult(result, mapper)); | ||
| }; | ||
| return new ToolDefinition(name, description, schema, toolHandler, null, null, null, null); | ||
| } | ||
|
|
||
| // ------------------------------------------------------------------ | ||
|
|
||
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -276,7 +276,8 @@ private void writeToolDefinition(PrintWriter out, ExecutableElement method) { | |
| out.println(" },"); | ||
| out.println(" " + overridesArg + ","); | ||
| out.println(" " + skipPermArg + ","); | ||
| out.println(" " + deferArg + ","); | ||
| out.println(" null"); | ||
|
Contributor
There was a problem hiding this comment. Should we also be adding "metadata" to CopilotTool and including it in the output here instead of null? Or is there no way to represent arbitrary dictionaries on a Java annotation? |
||
| out.print(" )"); | ||
| } | ||
|
|
||
|
|
||
Oops, something went wrong.
Oops, something went wrong.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm curious if the
[property: JsonPropertyName("metadata")]here makes any difference. I can't see what it would do. Is it possibly redundant?