Start iterating on #1682 On branch edburns/1682-java-tool-ergonomics by edburns · Pull Request #1692 · github/copilot-sdk

edburns · 2026-06-16T22:22:19Z

new file: 1682-java-tool-ergonomics-prompts-remove-before-merge/20260615-prompts.md

github-actions

Generated by SDK Consistency Review Agent for issue #1692 · sonnet46 1.6M

github-actions · 2026-06-17T17:08:46Z

+                    Description = "Custom grep override",
+                    AdditionalProperties = new Dictionary<string, object?>
+                    {
+                        ["is_override"] = true,


Cross-SDK consistency: hardcoded internal key "is_override"

This line uses the string "is_override" directly, but that key is declared as internal const string OverridesBuiltInToolKey = "is_override" in CopilotTool.cs — it isn't part of the public API surface.

Every other SDK exposes this intent through a typed, discoverable API:

Node.js: overridesBuiltInTool: true (option in defineTool)

Python: overrides_built_in_tool=True (kwarg in @define_tool)

Go: grepTool.OverridesBuiltInTool = true (struct field)

Java: ToolDefinition.createOverride(...) (factory method)

Rust: .with_overrides_built_in_tool(true) (builder method)

Two options to bring .NET into alignment:

Option A — Use CopilotTool.DefineTool (the idiomatic .NET helper):

CopilotTool.DefineTool( CustomGrep, new CopilotToolOptions { OverridesBuiltInTool = true }, new AIFunctionFactoryOptions { Name = "grep", Description = "Custom grep override" }),

This keeps the AIFunctionArguments-based parameter access (the "low-level" part of the test) while using the typed override flag.

Option B — Make OverridesBuiltInToolKey public so callers who intentionally use raw AIFunctionFactory.Create can reference a typed constant instead of a magic string:

["is_override"] = true, // ← becomes: [CopilotTool.OverridesBuiltInToolKey] = true

Option A is preferred for consistency, since it still demonstrates the low-level AIFunctionArguments pattern without leaking an internal key.

github-actions

Generated by SDK Consistency Review Agent for issue #1692 · sonnet46 2.2M

github-actions · 2026-06-17T17:33:32Z

+                    Description = "Custom grep override",
+                    AdditionalProperties = new Dictionary<string, object?>
+                    {
+                        ["is_override"] = true,


Cross-SDK consistency note: This hardcodes the raw "is_override" string, which is the internal value of CopilotTool.OverridesBuiltInToolKey (currently internal const). In all other SDKs the low-level override path uses a named, public API element:

SDK Approach

Go grepTool.OverridesBuiltInTool = true (exported struct field)

Java ToolDefinition.createOverride(...) (dedicated factory)

Node.js overridesBuiltInTool: true (named option)

Python overrides_built_in_tool=True (named parameter)

Rust .with_overrides_built_in_tool(true) (builder method)

For .NET, callers doing the raw approach must know the magic string "is_override". Consider making CopilotTool.OverridesBuiltInToolKey public so this test (and any other low-level callers) can write:

[CopilotTool.OverridesBuiltInToolKey] = true,

instead of relying on the private string value. Alternatively, the test could use CopilotTool.DefineTool() with CopilotToolOptions { OverridesBuiltInTool = true } for the override tool — even in a "low-level" test, the other two tools still demonstrate the raw AIFunctionArguments path.

github-actions · 2026-06-17T17:33:32Z

+                        60_000).get(90, TimeUnit.SECONDS);
+
+                assertNotNull(response, "Expected a response from the assistant");
+                String content = response.getData().content().toLowerCase();


Minor null-safety concern: content() returns String and can be null if the field is absent in the JSON response. Calling .toLowerCase() directly on it would throw a NullPointerException.

All other SDK tests guard against this:

Node.js: const content = assistantMessage?.data.content ?? "";

Python: content = assistant_message.data.content or ""

Go: explicitly checks if content == "" { t.Fatalf(...) }

Rust: uses a helper assistant_message_content() that handles null

Suggest:

String content = response.getData().content(); assertNotNull(content, "Expected non-null content in assistant response"); assertFalse(content.isEmpty(), "Expected non-empty content"); assertTrue(content.toLowerCase().contains("analyzing"), ...);

github-actions

Generated by SDK Consistency Review Agent for issue #1692 · sonnet46 2.6M

github-actions · 2026-06-17T18:08:13Z

+                    Description = "Custom grep override",
+                    AdditionalProperties = new Dictionary<string, object?>
+                    {
+                        ["is_override"] = true,


Cross-SDK consistency note — .NET "low-level" override key discoverability

The magic string "is_override" here is intentionally testing the low-level AIFunctionFactory.Create path, and that's correct. However, CopilotTool.OverridesBuiltInToolKey (which equals "is_override") is declared internal, so any .NET consumer who wants to use AIFunctionFactory.Create directly (instead of CopilotTool.DefineTool) has no discoverable public constant to reference.

The equivalent APIs in every other SDK are public and discoverable:

Node.js: overridesBuiltInTool: true property on defineTool

Python: overrides_built_in_tool=True parameter on @define_tool

Go: grepTool.OverridesBuiltInTool = true public struct field

Java: ToolDefinition.createOverride() public factory method

Rust: .with_overrides_built_in_tool(true) public builder method

Suggestion: consider either (a) making OverridesBuiltInToolKey a public const to allow consumers to reference it without hardcoding the string, or (b) adding a comment here pointing to CopilotTool.DefineTool with new CopilotToolOptions { OverridesBuiltInTool = true } as the idiomatic alternative when AIFunctionFactory.Create isn't required.

github-actions

Generated by SDK Consistency Review Agent for issue #1692 · sonnet46 2.2M

github-actions · 2026-06-17T23:30:45Z

+                    Description = "Custom grep override",
+                    AdditionalProperties = new Dictionary<string, object?>
+                    {
+                        ["is_override"] = true,


Cross-SDK consistency note: All other SDKs expose overridesBuiltInTool as a typed, public API in their low-level tool definition (e.g., Java's ToolDefinition.createOverride(), Go's grepTool.OverridesBuiltInTool = true, Node.js's overridesBuiltInTool: true, Python's overrides_built_in_tool=True, Rust's .with_overrides_built_in_tool(true)). Here the .NET test sets "is_override" via a raw AdditionalProperties dictionary key, which is an internal SDK constant (CopilotTool.OverridesBuiltInToolKey).

Using CopilotTool.DefineTool() with CopilotToolOptions { OverridesBuiltInTool = true } would be the equivalent public SDK API and would align better with how other languages expose this feature:

CopilotTool.DefineTool(CustomGrep, new CopilotToolOptions { OverridesBuiltInTool = true, }, new AIFunctionFactoryOptions { Name = "grep", Description = "Custom grep override", }),

This is especially relevant since "is_override" is internal const — SDK users reading this test as documentation would copy a magic string that's not part of the documented public surface.

edburns · 2026-06-18T17:44:38Z

PROBLEM STATEMENT: PR 1692 is intractable because of its breadth

I observe that my approach to get this PR merged: #1692 is suboptimal because of its breadth of having changes that span dotnet, go, java, nodejs, python, and rust. Each push to the topic branch of this PR takes over an hour to run all the tests.

I want to break PR 1692 up into several PRs. Work in this session will do this break up.

The topic branch for PR 1692 is checked out at this git worktree:

C:/Users/edburns/workareas/copilot-sdk    150b8417 [edburns/1682-java-tool-ergonomics]

The remainder of this prompt describes how to go about breaking out the language specific elements from that topic branch into 6 new PRs.

❌❌ Do nothing to PR 1692. I will deal with that after our work here is done. ❌❌

SOLUTION APPROACH: BREAK PR 1692 into six other PRs

We have six other worktrees for the other phases of the breakouts, each with its own topic branch on the upstream, as shown next. Each of these has been added as a folder using /add-dir. When this work is done, all the aspects of PR 1692 will have been merged to main, and PR 1692 can be safely closed.

Note: equally important is the parallelism of the PRs. The next section, ORDERING, deals with that aspect.