From dc83960adc76f10973f5ff74cf8b10735aa9a560 Mon Sep 17 00:00:00 2001 From: Patrick Nikoletich Date: Thu, 19 Feb 2026 21:03:10 -0800 Subject: [PATCH] feat: auto-exclude built-in tools overridden by user-registered tools When users register tools via SessionConfig with names matching built-in CLI tools, automatically add those names to excludedTools in the RPC payload so the SDK's local handler takes precedence. Implemented consistently across all 4 SDKs (Node.js, Python, Go, .NET) with unit tests and documentation updates. Closes #411 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- dotnet/README.md | 13 ++++ dotnet/src/Client.cs | 12 ++- dotnet/src/GitHub.Copilot.SDK.csproj | 4 + dotnet/test/MergeExcludedToolsTests.cs | 79 +++++++++++++++++++ go/README.md | 11 +++ go/client.go | 27 ++++++- go/client_test.go | 36 +++++++++ nodejs/README.md | 12 +++ nodejs/src/client.ts | 17 ++++- nodejs/test/client.test.ts | 67 ++++++++++++++++ python/README.md | 14 ++++ python/copilot/client.py | 10 ++- python/test_client.py | 102 ++++++++++++++++++++++++- 13 files changed, 395 insertions(+), 9 deletions(-) create mode 100644 dotnet/test/MergeExcludedToolsTests.cs diff --git a/dotnet/README.md b/dotnet/README.md index bda10059d..e9066fb10 100644 --- a/dotnet/README.md +++ b/dotnet/README.md @@ -415,6 +415,19 @@ var session = await client.CreateSessionAsync(new SessionConfig When Copilot invokes `lookup_issue`, the client automatically runs your handler and responds to the CLI. Handlers can return any JSON-serializable value (automatically wrapped), or a `ToolResultAIContent` wrapping a `ToolResultObject` for full control over result metadata. +#### Overriding Built-in Tools + +If you register a tool with the same name as a built-in CLI tool (e.g. `edit_file`, `read_file`), your tool takes precedence. The SDK automatically adds the tool name to `ExcludedTools`, so the built-in is disabled and your handler is called instead. This is useful when you need custom behavior for built-in operations. + +```csharp +AIFunctionFactory.Create( + async ([Description("File path")] string path, [Description("New content")] string content) => { + // your logic + }, + "edit_file", + "Custom file editor with project-specific validation") +``` + ### System Message Customization Control the system prompt using `SystemMessage` in session config: diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 8c70a4a2b..569b6c785 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -382,7 +382,7 @@ public async Task CreateSessionAsync(SessionConfig? config = nul config?.Tools?.Select(ToolDefinition.FromAIFunction).ToList(), config?.SystemMessage, config?.AvailableTools, - config?.ExcludedTools, + MergeExcludedTools(config?.ExcludedTools, config?.Tools), config?.Provider, (bool?)true, config?.OnUserInputRequest != null ? true : null, @@ -467,7 +467,7 @@ public async Task ResumeSessionAsync(string sessionId, ResumeSes config?.Tools?.Select(ToolDefinition.FromAIFunction).ToList(), config?.SystemMessage, config?.AvailableTools, - config?.ExcludedTools, + MergeExcludedTools(config?.ExcludedTools, config?.Tools), config?.Provider, (bool?)true, config?.OnUserInputRequest != null ? true : null, @@ -852,6 +852,14 @@ private void DispatchLifecycleEvent(SessionLifecycleEvent evt) } } + internal static List? MergeExcludedTools(List? excludedTools, ICollection? tools) + { + var toolNames = tools?.Select(t => t.Name).ToList(); + if (toolNames is null or { Count: 0 }) return excludedTools; + if (excludedTools is null or { Count: 0 }) return toolNames; + return excludedTools.Union(toolNames).ToList(); + } + internal static async Task InvokeRpcAsync(JsonRpc rpc, string method, object?[]? args, CancellationToken cancellationToken) { return await InvokeRpcAsync(rpc, method, args, null, cancellationToken); diff --git a/dotnet/src/GitHub.Copilot.SDK.csproj b/dotnet/src/GitHub.Copilot.SDK.csproj index 019788cfa..7a3fdacaf 100644 --- a/dotnet/src/GitHub.Copilot.SDK.csproj +++ b/dotnet/src/GitHub.Copilot.SDK.csproj @@ -17,6 +17,10 @@ true + + + + diff --git a/dotnet/test/MergeExcludedToolsTests.cs b/dotnet/test/MergeExcludedToolsTests.cs new file mode 100644 index 000000000..a5271a4a0 --- /dev/null +++ b/dotnet/test/MergeExcludedToolsTests.cs @@ -0,0 +1,79 @@ +/*--------------------------------------------------------------------------------------------- + * Copyright (c) Microsoft Corporation. All rights reserved. + *--------------------------------------------------------------------------------------------*/ + +using Microsoft.Extensions.AI; +using System.ComponentModel; +using Xunit; + +namespace GitHub.Copilot.SDK.Test; + +public class MergeExcludedToolsTests +{ + [Fact] + public void Tool_Names_Are_Added_To_ExcludedTools() + { + var tools = new List + { + AIFunctionFactory.Create(Noop, "my_tool"), + }; + + var result = CopilotClient.MergeExcludedTools(null, tools); + + Assert.NotNull(result); + Assert.Contains("my_tool", result!); + } + + [Fact] + public void Merges_With_Existing_ExcludedTools_And_Deduplicates() + { + var existing = new List { "view", "my_tool" }; + var tools = new List + { + AIFunctionFactory.Create(Noop, "my_tool"), + AIFunctionFactory.Create(Noop, "another_tool"), + }; + + var result = CopilotClient.MergeExcludedTools(existing, tools); + + Assert.NotNull(result); + Assert.Equal(3, result!.Count); + Assert.Contains("view", result); + Assert.Contains("my_tool", result); + Assert.Contains("another_tool", result); + } + + [Fact] + public void Returns_Null_When_No_Tools_Provided() + { + var result = CopilotClient.MergeExcludedTools(null, null); + Assert.Null(result); + } + + [Fact] + public void Returns_ExcludedTools_Unchanged_When_Tools_Empty() + { + var existing = new List { "view" }; + var result = CopilotClient.MergeExcludedTools(existing, new List()); + + Assert.Same(existing, result); + } + + [Fact] + public void Returns_Tool_Names_When_ExcludedTools_Null() + { + var tools = new List + { + AIFunctionFactory.Create(Noop, "my_tool"), + }; + + var result = CopilotClient.MergeExcludedTools(null, tools); + + Assert.NotNull(result); + Assert.Single(result!); + Assert.Equal("my_tool", result[0]); + } + + [Description("No-op")] + static string Noop() => ""; +} diff --git a/go/README.md b/go/README.md index 37cb7ce07..d4d50cbae 100644 --- a/go/README.md +++ b/go/README.md @@ -266,6 +266,17 @@ session, _ := client.CreateSession(context.Background(), &copilot.SessionConfig{ When the model selects a tool, the SDK automatically runs your handler (in parallel with other calls) and responds to the CLI's `tool.call` with the handler's result. +#### Overriding Built-in Tools + +If you register a tool with the same name as a built-in CLI tool (e.g. `edit_file`, `read_file`), your tool takes precedence. The SDK automatically adds the tool name to `ExcludedTools`, so the built-in is disabled and your handler is called instead. This is useful when you need custom behavior for built-in operations. + +```go +editFile := copilot.DefineTool("edit_file", "Custom file editor with project-specific validation", + func(params EditFileParams, inv copilot.ToolInvocation) (any, error) { + // your logic + }) +``` + ## Streaming Enable streaming to receive assistant response chunks as they're generated: diff --git a/go/client.go b/go/client.go index 68f58d859..7894e3902 100644 --- a/go/client.go +++ b/go/client.go @@ -461,7 +461,7 @@ func (c *Client) CreateSession(ctx context.Context, config *SessionConfig) (*Ses req.Tools = config.Tools req.SystemMessage = config.SystemMessage req.AvailableTools = config.AvailableTools - req.ExcludedTools = config.ExcludedTools + req.ExcludedTools = mergeExcludedTools(config.ExcludedTools, config.Tools) req.Provider = config.Provider req.WorkingDirectory = config.WorkingDirectory req.MCPServers = config.MCPServers @@ -558,7 +558,7 @@ func (c *Client) ResumeSessionWithOptions(ctx context.Context, sessionID string, req.Tools = config.Tools req.Provider = config.Provider req.AvailableTools = config.AvailableTools - req.ExcludedTools = config.ExcludedTools + req.ExcludedTools = mergeExcludedTools(config.ExcludedTools, config.Tools) if config.Streaming { req.Streaming = Bool(true) } @@ -1352,6 +1352,29 @@ func buildFailedToolResult(internalError string) ToolResult { } // buildUnsupportedToolResult creates a failure ToolResult for an unsupported tool. +// mergeExcludedTools returns a deduplicated list combining excludedTools with +// the names of any SDK-registered tools, so the CLI won't handle them. +func mergeExcludedTools(excludedTools []string, tools []Tool) []string { + if len(tools) == 0 { + return excludedTools + } + seen := make(map[string]bool, len(excludedTools)+len(tools)) + merged := make([]string, 0, len(excludedTools)+len(tools)) + for _, name := range excludedTools { + if !seen[name] { + seen[name] = true + merged = append(merged, name) + } + } + for _, t := range tools { + if !seen[t.Name] { + seen[t.Name] = true + merged = append(merged, t.Name) + } + } + return merged +} + func buildUnsupportedToolResult(toolName string) ToolResult { return ToolResult{ TextResultForLLM: fmt.Sprintf("Tool '%s' is not supported by this client instance.", toolName), diff --git a/go/client_test.go b/go/client_test.go index b2e9cdce6..b0d0c709d 100644 --- a/go/client_test.go +++ b/go/client_test.go @@ -444,3 +444,39 @@ func TestResumeSessionRequest_ClientName(t *testing.T) { } }) } + +func TestMergeExcludedTools(t *testing.T) { + t.Run("adds tool names to excluded tools", func(t *testing.T) { + tools := []Tool{{Name: "edit_file"}, {Name: "read_file"}} + got := mergeExcludedTools(nil, tools) + want := []string{"edit_file", "read_file"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } + }) + + t.Run("deduplicates with existing excluded tools", func(t *testing.T) { + excluded := []string{"edit_file", "run_shell"} + tools := []Tool{{Name: "edit_file"}, {Name: "read_file"}} + got := mergeExcludedTools(excluded, tools) + want := []string{"edit_file", "run_shell", "read_file"} + if !reflect.DeepEqual(got, want) { + t.Errorf("got %v, want %v", got, want) + } + }) + + t.Run("returns original list when no tools provided", func(t *testing.T) { + excluded := []string{"edit_file"} + got := mergeExcludedTools(excluded, nil) + if !reflect.DeepEqual(got, excluded) { + t.Errorf("got %v, want %v", got, excluded) + } + }) + + t.Run("returns nil when both inputs are empty", func(t *testing.T) { + got := mergeExcludedTools(nil, nil) + if got != nil { + t.Errorf("got %v, want nil", got) + } + }) +} diff --git a/nodejs/README.md b/nodejs/README.md index 31558b8ab..03ed2f751 100644 --- a/nodejs/README.md +++ b/nodejs/README.md @@ -402,6 +402,18 @@ const session = await client.createSession({ When Copilot invokes `lookup_issue`, the client automatically runs your handler and responds to the CLI. Handlers can return any JSON-serializable value (automatically wrapped), a simple string, or a `ToolResultObject` for full control over result metadata. Raw JSON schemas are also supported if Zod isn't desired. +#### Overriding Built-in Tools + +If you register a tool with the same name as a built-in CLI tool (e.g. `edit_file`, `read_file`), your tool takes precedence. The SDK automatically adds the tool name to `excludedTools`, so the built-in is disabled and your handler is called instead. This is useful when you need custom behavior for built-in operations. + +```ts +defineTool("edit_file", { + description: "Custom file editor with project-specific validation", + parameters: z.object({ path: z.string(), content: z.string() }), + handler: async ({ path, content }) => { /* your logic */ }, +}) +``` + ### System Message Customization Control the system prompt using `systemMessage` in session config: diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 7df64e507..f8a710a4c 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -50,6 +50,19 @@ import type { TypedSessionLifecycleHandler, } from "./types.js"; +/** + * Merge user-provided excludedTools with tool names from config.tools so that + * SDK-registered tools automatically override built-in CLI tools. + */ +function mergeExcludedTools( + excludedTools: string[] | undefined, + tools: Tool[] | undefined +): string[] | undefined { + const toolNames = tools?.map((t) => t.name); + if (!excludedTools?.length && !toolNames?.length) return excludedTools; + return [...new Set([...(excludedTools ?? []), ...(toolNames ?? [])])]; +} + /** * Check if value is a Zod schema (has toJSONSchema method) */ @@ -529,7 +542,7 @@ export class CopilotClient { })), systemMessage: config.systemMessage, availableTools: config.availableTools, - excludedTools: config.excludedTools, + excludedTools: mergeExcludedTools(config.excludedTools, config.tools), provider: config.provider, requestPermission: true, requestUserInput: !!config.onUserInputRequest, @@ -607,7 +620,7 @@ export class CopilotClient { reasoningEffort: config.reasoningEffort, systemMessage: config.systemMessage, availableTools: config.availableTools, - excludedTools: config.excludedTools, + excludedTools: mergeExcludedTools(config.excludedTools, config.tools), tools: config.tools?.map((tool) => ({ name: tool.name, description: tool.description, diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 5d1ed8ac3..4f01c8021 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -243,4 +243,71 @@ describe("CopilotClient", () => { }).toThrow(/githubToken and useLoggedInUser cannot be used with cliUrl/); }); }); + + describe("excludedTools merging with config.tools", () => { + it("adds tool names from config.tools to excludedTools in session.create", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + const spy = vi.spyOn((client as any).connection!, "sendRequest"); + await client.createSession({ + tools: [{ name: "edit_file", description: "edit", handler: async () => "ok" }], + }); + + expect(spy).toHaveBeenCalledWith( + "session.create", + expect.objectContaining({ excludedTools: ["edit_file"] }) + ); + }); + + it("merges and deduplicates with existing excludedTools", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + const spy = vi.spyOn((client as any).connection!, "sendRequest"); + await client.createSession({ + tools: [{ name: "edit_file", description: "edit", handler: async () => "ok" }], + excludedTools: ["edit_file", "run_command"], + }); + + const payload = spy.mock.calls.find((c) => c[0] === "session.create")![1] as any; + expect(payload.excludedTools).toEqual( + expect.arrayContaining(["edit_file", "run_command"]) + ); + expect(payload.excludedTools).toHaveLength(2); + }); + + it("leaves excludedTools unchanged when no tools provided", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + const spy = vi.spyOn((client as any).connection!, "sendRequest"); + await client.createSession({ excludedTools: ["run_command"] }); + + expect(spy).toHaveBeenCalledWith( + "session.create", + expect.objectContaining({ excludedTools: ["run_command"] }) + ); + }); + + it("adds tool names from config.tools to excludedTools in session.resume", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + const session = await client.createSession(); + const spy = vi.spyOn((client as any).connection!, "sendRequest"); + await client.resumeSession(session.sessionId, { + tools: [{ name: "edit_file", description: "edit", handler: async () => "ok" }], + }); + + expect(spy).toHaveBeenCalledWith( + "session.resume", + expect.objectContaining({ excludedTools: ["edit_file"] }) + ); + }); + }); }); diff --git a/python/README.md b/python/README.md index aa82e0c34..09d62ae30 100644 --- a/python/README.md +++ b/python/README.md @@ -210,6 +210,20 @@ session = await client.create_session({ The SDK automatically handles `tool.call`, executes your handler (sync or async), and responds with the final result when the tool completes. +#### Overriding Built-in Tools + +If you register a tool with the same name as a built-in CLI tool (e.g. `edit_file`, `read_file`), your tool takes precedence. The SDK automatically adds the tool name to `excluded_tools`, so the built-in is disabled and your handler is called instead. This is useful when you need custom behavior for built-in operations. + +```python +class EditFileParams(BaseModel): + path: str = Field(description="File path") + content: str = Field(description="New file content") + +@define_tool(name="edit_file", description="Custom file editor with project-specific validation") +async def edit_file(params: EditFileParams) -> str: + # your logic +``` + ## Image Support The SDK supports image attachments via the `attachments` parameter. You can attach images by providing their file path: diff --git a/python/copilot/client.py b/python/copilot/client.py index 90260ffbd..509ad578b 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -483,7 +483,10 @@ async def create_session(self, config: Optional[SessionConfig] = None) -> Copilo available_tools = cfg.get("available_tools") if available_tools is not None: payload["availableTools"] = available_tools - excluded_tools = cfg.get("excluded_tools") + excluded_tools = list(cfg.get("excluded_tools") or []) + if tools: + tool_names = [t.name for t in tools] + excluded_tools = list(dict.fromkeys(excluded_tools + tool_names)) if excluded_tools: payload["excludedTools"] = excluded_tools @@ -655,7 +658,10 @@ async def resume_session( if available_tools is not None: payload["availableTools"] = available_tools - excluded_tools = cfg.get("excluded_tools") + excluded_tools = list(cfg.get("excluded_tools") or []) + if tools: + tool_names = [t.name for t in tools] + excluded_tools = list(dict.fromkeys(excluded_tools + tool_names)) if excluded_tools: payload["excludedTools"] = excluded_tools diff --git a/python/test_client.py b/python/test_client.py index 0bc99ea69..7078cd74f 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -6,7 +6,7 @@ import pytest -from copilot import CopilotClient +from copilot import CopilotClient, define_tool from e2e.testharness import CLI_PATH @@ -149,6 +149,106 @@ def test_use_logged_in_user_with_cli_url_raises(self): ) +class TestExcludedToolsFromRegisteredTools: + @pytest.mark.asyncio + async def test_tools_added_to_excluded_tools(self): + client = CopilotClient({"cli_path": CLI_PATH}) + await client.start() + + try: + captured = {} + original_request = client._client.request + + async def mock_request(method, params): + captured[method] = params + return await original_request(method, params) + + client._client.request = mock_request + + @define_tool(description="Edit a file") + def edit_file(params) -> str: + return "ok" + + await client.create_session({"tools": [edit_file]}) + assert "edit_file" in captured["session.create"]["excludedTools"] + finally: + await client.force_stop() + + @pytest.mark.asyncio + async def test_deduplication_with_existing_excluded_tools(self): + client = CopilotClient({"cli_path": CLI_PATH}) + await client.start() + + try: + captured = {} + original_request = client._client.request + + async def mock_request(method, params): + captured[method] = params + return await original_request(method, params) + + client._client.request = mock_request + + @define_tool(description="Edit a file") + def edit_file(params) -> str: + return "ok" + + await client.create_session({ + "tools": [edit_file], + "excluded_tools": ["edit_file", "other_tool"], + }) + excluded = captured["session.create"]["excludedTools"] + assert excluded.count("edit_file") == 1 + assert "other_tool" in excluded + finally: + await client.force_stop() + + @pytest.mark.asyncio + async def test_no_excluded_tools_when_no_tools(self): + client = CopilotClient({"cli_path": CLI_PATH}) + await client.start() + + try: + captured = {} + original_request = client._client.request + + async def mock_request(method, params): + captured[method] = params + return await original_request(method, params) + + client._client.request = mock_request + await client.create_session() + assert "excludedTools" not in captured["session.create"] + finally: + await client.force_stop() + + @pytest.mark.asyncio + async def test_resume_session_adds_tools_to_excluded(self): + client = CopilotClient({"cli_path": CLI_PATH}) + await client.start() + + try: + session = await client.create_session() + + captured = {} + original_request = client._client.request + + async def mock_request(method, params): + captured[method] = params + return await original_request(method, params) + + client._client.request = mock_request + + @define_tool(description="Edit a file") + def edit_file(params) -> str: + return "ok" + + await client.resume_session(session.session_id, {"tools": [edit_file]}) + assert "edit_file" in captured["session.resume"]["excludedTools"] + finally: + await client.force_stop() + + class TestSessionConfigForwarding: @pytest.mark.asyncio async def test_create_session_forwards_client_name(self):