diff --git a/dotnet/src/Client.cs b/dotnet/src/Client.cs index 5b7474a64..0794043d8 100644 --- a/dotnet/src/Client.cs +++ b/dotnet/src/Client.cs @@ -54,6 +54,9 @@ namespace GitHub.Copilot.SDK; /// public sealed partial class CopilotClient : IDisposable, IAsyncDisposable { + internal const string NoResultPermissionV2ErrorMessage = + "Permission handlers cannot return 'no-result' when connected to a protocol v2 server."; + /// /// Minimum protocol version this SDK can communicate with. /// @@ -1394,8 +1397,16 @@ public async Task OnPermissionRequestV2(string sess try { var result = await session.HandlePermissionRequestAsync(permissionRequest); + if (result.Kind == new PermissionRequestResultKind("no-result")) + { + throw new InvalidOperationException(NoResultPermissionV2ErrorMessage); + } return new PermissionRequestResponseV2(result); } + catch (InvalidOperationException ex) when (ex.Message == NoResultPermissionV2ErrorMessage) + { + throw; + } catch (Exception) { return new PermissionRequestResponseV2(new PermissionRequestResult diff --git a/dotnet/src/Session.cs b/dotnet/src/Session.cs index 324b3df6d..f1438d82b 100644 --- a/dotnet/src/Session.cs +++ b/dotnet/src/Session.cs @@ -467,6 +467,10 @@ private async Task ExecutePermissionAndRespondAsync(string requestId, Permission }; var result = await handler(permissionRequest, invocation); + if (result.Kind == new PermissionRequestResultKind("no-result")) + { + return; + } await Rpc.Permissions.HandlePendingPermissionRequestAsync(requestId, result); } catch (Exception) diff --git a/dotnet/src/Types.cs b/dotnet/src/Types.cs index 633a97654..908c3e46e 100644 --- a/dotnet/src/Types.cs +++ b/dotnet/src/Types.cs @@ -350,6 +350,7 @@ public class PermissionRequestResult /// "denied-by-rules" — denied by configured permission rules. /// "denied-interactively-by-user" — the user explicitly denied the request. /// "denied-no-approval-rule-and-could-not-request-from-user" — no rule matched and user approval was unavailable. + /// "no-result" — leave the pending permission request unanswered. /// /// [JsonPropertyName("kind")] diff --git a/dotnet/test/PermissionRequestResultKindTests.cs b/dotnet/test/PermissionRequestResultKindTests.cs index d0cfed6f0..ea77295e2 100644 --- a/dotnet/test/PermissionRequestResultKindTests.cs +++ b/dotnet/test/PermissionRequestResultKindTests.cs @@ -21,6 +21,7 @@ public void WellKnownKinds_HaveExpectedValues() Assert.Equal("denied-by-rules", PermissionRequestResultKind.DeniedByRules.Value); Assert.Equal("denied-no-approval-rule-and-could-not-request-from-user", PermissionRequestResultKind.DeniedCouldNotRequestFromUser.Value); Assert.Equal("denied-interactively-by-user", PermissionRequestResultKind.DeniedInteractivelyByUser.Value); + Assert.Equal("no-result", new PermissionRequestResultKind("no-result").Value); } [Fact] @@ -115,6 +116,7 @@ public void JsonRoundTrip_PreservesAllKinds() PermissionRequestResultKind.DeniedByRules, PermissionRequestResultKind.DeniedCouldNotRequestFromUser, PermissionRequestResultKind.DeniedInteractivelyByUser, + new PermissionRequestResultKind("no-result"), }; foreach (var kind in kinds) diff --git a/go/client.go b/go/client.go index 021de2b14..af1ce590e 100644 --- a/go/client.go +++ b/go/client.go @@ -51,6 +51,8 @@ import ( "github.com/github/copilot-sdk/go/rpc" ) +const noResultPermissionV2Error = "permission handlers cannot return 'no-result' when connected to a protocol v2 server" + // Client manages the connection to the Copilot CLI server and provides session management. // // The Client can either spawn a CLI server process or connect to an existing server. @@ -1531,6 +1533,9 @@ func (c *Client) handlePermissionRequestV2(req permissionRequestV2) (*permission }, }, nil } + if result.Kind == "no-result" { + return nil, &jsonrpc2.Error{Code: -32603, Message: noResultPermissionV2Error} + } return &permissionResponseV2{Result: result}, nil } diff --git a/go/session.go b/go/session.go index 74529c523..8358ea7c0 100644 --- a/go/session.go +++ b/go/session.go @@ -562,6 +562,9 @@ func (s *Session) executePermissionAndRespond(requestID string, permissionReques }) return } + if result.Kind == "no-result" { + return + } s.RPC.Permissions.HandlePendingPermissionRequest(context.Background(), &rpc.SessionPermissionsHandlePendingPermissionRequestParams{ RequestID: requestID, diff --git a/go/types_test.go b/go/types_test.go index 190cd913d..80b0cc545 100644 --- a/go/types_test.go +++ b/go/types_test.go @@ -15,6 +15,7 @@ func TestPermissionRequestResultKind_Constants(t *testing.T) { {"DeniedByRules", PermissionRequestResultKindDeniedByRules, "denied-by-rules"}, {"DeniedCouldNotRequestFromUser", PermissionRequestResultKindDeniedCouldNotRequestFromUser, "denied-no-approval-rule-and-could-not-request-from-user"}, {"DeniedInteractivelyByUser", PermissionRequestResultKindDeniedInteractivelyByUser, "denied-interactively-by-user"}, + {"NoResult", PermissionRequestResultKind("no-result"), "no-result"}, } for _, tt := range tests { @@ -42,6 +43,7 @@ func TestPermissionRequestResult_JSONRoundTrip(t *testing.T) { {"DeniedByRules", PermissionRequestResultKindDeniedByRules}, {"DeniedCouldNotRequestFromUser", PermissionRequestResultKindDeniedCouldNotRequestFromUser}, {"DeniedInteractivelyByUser", PermissionRequestResultKindDeniedInteractivelyByUser}, + {"NoResult", PermissionRequestResultKind("no-result")}, {"Custom", PermissionRequestResultKind("custom")}, } diff --git a/nodejs/docs/agent-author.md b/nodejs/docs/agent-author.md index 4c1e32f69..8b3d93593 100644 --- a/nodejs/docs/agent-author.md +++ b/nodejs/docs/agent-author.md @@ -59,11 +59,9 @@ Discovery rules: ## Minimal Skeleton ```js -import { approveAll } from "@github/copilot-sdk"; import { joinSession } from "@github/copilot-sdk/extension"; await joinSession({ - onPermissionRequest: approveAll, // Required — handle permission requests tools: [], // Optional — custom tools hooks: {}, // Optional — lifecycle hooks }); diff --git a/nodejs/docs/examples.md b/nodejs/docs/examples.md index a5b03f87e..1461a2f39 100644 --- a/nodejs/docs/examples.md +++ b/nodejs/docs/examples.md @@ -7,11 +7,9 @@ A practical guide to writing extensions using the `@github/copilot-sdk` extensio Every extension starts with the same boilerplate: ```js -import { approveAll } from "@github/copilot-sdk"; import { joinSession } from "@github/copilot-sdk/extension"; const session = await joinSession({ - onPermissionRequest: approveAll, hooks: { /* ... */ }, tools: [ /* ... */ ], }); @@ -33,7 +31,6 @@ Use `session.log()` to surface messages to the user in the CLI timeline: ```js const session = await joinSession({ - onPermissionRequest: approveAll, hooks: { onSessionStart: async () => { await session.log("My extension loaded"); @@ -383,7 +380,6 @@ function copyToClipboard(text) { } const session = await joinSession({ - onPermissionRequest: approveAll, hooks: { onUserPromptSubmitted: async (input) => { if (/\\bcopy\\b/i.test(input.prompt)) { @@ -425,15 +421,12 @@ Correlate `tool.execution_start` / `tool.execution_complete` events by `toolCall ```js import { existsSync, watchFile, readFileSync } from "node:fs"; import { join } from "node:path"; -import { approveAll } from "@github/copilot-sdk"; import { joinSession } from "@github/copilot-sdk/extension"; const agentEdits = new Set(); // toolCallIds for in-flight agent edits const recentAgentPaths = new Set(); // paths recently written by the agent -const session = await joinSession({ - onPermissionRequest: approveAll, -}); +const session = await joinSession(); const workspace = session.workspacePath; // e.g. ~/.copilot/session-state/ if (workspace) { @@ -480,14 +473,11 @@ Filter out agent edits by tracking `tool.execution_start` / `tool.execution_comp ```js import { watch, readFileSync, statSync } from "node:fs"; import { join, relative, resolve } from "node:path"; -import { approveAll } from "@github/copilot-sdk"; import { joinSession } from "@github/copilot-sdk/extension"; const agentEditPaths = new Set(); -const session = await joinSession({ - onPermissionRequest: approveAll, -}); +const session = await joinSession(); const cwd = process.cwd(); const IGNORE = new Set(["node_modules", ".git", "dist"]); @@ -582,7 +572,6 @@ Register `onUserInputRequest` to enable the agent's `ask_user` tool: ```js const session = await joinSession({ - onPermissionRequest: approveAll, onUserInputRequest: async (request) => { // request.question has the agent's question // request.choices has the options (if multiple choice) @@ -599,7 +588,6 @@ An extension that combines tools, hooks, and events. ```js import { execFile, exec } from "node:child_process"; -import { approveAll } from "@github/copilot-sdk"; import { joinSession } from "@github/copilot-sdk/extension"; const isWindows = process.platform === "win32"; @@ -617,7 +605,6 @@ function openInEditor(filePath) { } const session = await joinSession({ - onPermissionRequest: approveAll, hooks: { onUserPromptSubmitted: async (input) => { if (/\\bcopy this\\b/i.test(input.prompt)) { diff --git a/nodejs/docs/extensions.md b/nodejs/docs/extensions.md index 5eff9135b..8b36de8a5 100644 --- a/nodejs/docs/extensions.md +++ b/nodejs/docs/extensions.md @@ -39,11 +39,9 @@ Extensions add custom tools, hooks, and behaviors to the Copilot CLI. They run a Extensions use `@github/copilot-sdk` for all interactions with the CLI: ```js -import { approveAll } from "@github/copilot-sdk"; import { joinSession } from "@github/copilot-sdk/extension"; const session = await joinSession({ - onPermissionRequest: approveAll, tools: [ /* ... */ ], diff --git a/nodejs/src/client.ts b/nodejs/src/client.ts index 954d88b59..c96d4b691 100644 --- a/nodejs/src/client.ts +++ b/nodejs/src/client.ts @@ -25,7 +25,7 @@ import { } from "vscode-jsonrpc/node.js"; import { createServerRpc } from "./generated/rpc.js"; import { getSdkProtocolVersion } from "./sdkProtocolVersion.js"; -import { CopilotSession } from "./session.js"; +import { CopilotSession, NO_RESULT_PERMISSION_V2_ERROR } from "./session.js"; import type { ConnectionState, CopilotClientOptions, @@ -1604,7 +1604,10 @@ export class CopilotClient { try { const result = await session._handlePermissionRequestV2(params.permissionRequest); return { result }; - } catch (_error) { + } catch (error) { + if (error instanceof Error && error.message === NO_RESULT_PERMISSION_V2_ERROR) { + throw error; + } return { result: { kind: "denied-no-approval-rule-and-could-not-request-from-user", diff --git a/nodejs/src/extension.ts b/nodejs/src/extension.ts index 0a9b7b05d..b7c2da3a8 100644 --- a/nodejs/src/extension.ts +++ b/nodejs/src/extension.ts @@ -4,7 +4,15 @@ import { CopilotClient } from "./client.js"; import type { CopilotSession } from "./session.js"; -import type { ResumeSessionConfig } from "./types.js"; +import type { PermissionHandler, PermissionRequestResult, ResumeSessionConfig } from "./types.js"; + +const defaultJoinSessionPermissionHandler: PermissionHandler = (): PermissionRequestResult => ({ + kind: "no-result", +}); + +export type JoinSessionConfig = Omit & { + onPermissionRequest?: PermissionHandler; +}; /** * Joins the current foreground session. @@ -14,16 +22,12 @@ import type { ResumeSessionConfig } from "./types.js"; * * @example * ```typescript - * import { approveAll } from "@github/copilot-sdk"; * import { joinSession } from "@github/copilot-sdk/extension"; * - * const session = await joinSession({ - * onPermissionRequest: approveAll, - * tools: [myTool], - * }); + * const session = await joinSession({ tools: [myTool] }); * ``` */ -export async function joinSession(config: ResumeSessionConfig): Promise { +export async function joinSession(config: JoinSessionConfig = {}): Promise { const sessionId = process.env.SESSION_ID; if (!sessionId) { throw new Error( @@ -34,6 +38,7 @@ export async function joinSession(config: ResumeSessionConfig): Promise; @@ -400,6 +403,9 @@ export class CopilotSession { const result = await this.permissionHandler!(permissionRequest, { sessionId: this.sessionId, }); + if (result.kind === "no-result") { + return; + } await this.rpc.permissions.handlePendingPermissionRequest({ requestId, result }); } catch (_error) { try { @@ -505,8 +511,14 @@ export class CopilotSession { const result = await this.permissionHandler(request as PermissionRequest, { sessionId: this.sessionId, }); + if (result.kind === "no-result") { + throw new Error(NO_RESULT_PERMISSION_V2_ERROR); + } return result; - } catch (_error) { + } catch (error) { + if (error instanceof Error && error.message === NO_RESULT_PERMISSION_V2_ERROR) { + throw error; + } return { kind: "denied-no-approval-rule-and-could-not-request-from-user" }; } } diff --git a/nodejs/src/types.ts b/nodejs/src/types.ts index 99b9af75c..cbc8b10ed 100644 --- a/nodejs/src/types.ts +++ b/nodejs/src/types.ts @@ -241,7 +241,8 @@ export interface PermissionRequest { import type { SessionPermissionsHandlePendingPermissionRequestParams } from "./generated/rpc.js"; export type PermissionRequestResult = - SessionPermissionsHandlePendingPermissionRequestParams["result"]; + | SessionPermissionsHandlePendingPermissionRequestParams["result"] + | { kind: "no-result" }; export type PermissionHandler = ( request: PermissionRequest, diff --git a/nodejs/test/client.test.ts b/nodejs/test/client.test.ts index 7206c903b..6f3e4ef98 100644 --- a/nodejs/test/client.test.ts +++ b/nodejs/test/client.test.ts @@ -26,6 +26,38 @@ describe("CopilotClient", () => { ); }); + it("does not respond to v3 permission requests when handler returns no-result", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + const session = await client.createSession({ + onPermissionRequest: () => ({ kind: "no-result" }), + }); + const spy = vi.spyOn(session.rpc.permissions, "handlePendingPermissionRequest"); + + await (session as any)._executePermissionAndRespond("request-1", { kind: "write" }); + + expect(spy).not.toHaveBeenCalled(); + }); + + it("throws when a v2 permission handler returns no-result", async () => { + const client = new CopilotClient(); + await client.start(); + onTestFinished(() => client.forceStop()); + + const session = await client.createSession({ + onPermissionRequest: () => ({ kind: "no-result" }), + }); + + await expect( + (client as any).handlePermissionRequestV2({ + sessionId: session.sessionId, + permissionRequest: { kind: "write" }, + }) + ).rejects.toThrow(/protocol v2 server/); + }); + it("forwards clientName in session.create request", async () => { const client = new CopilotClient(); await client.start(); diff --git a/nodejs/test/e2e/client.test.ts b/nodejs/test/e2e/client.test.ts index 9d71ee726..594607cd1 100644 --- a/nodejs/test/e2e/client.test.ts +++ b/nodejs/test/e2e/client.test.ts @@ -43,27 +43,32 @@ describe("Client", () => { expect(client.getState()).toBe("disconnected"); }); - it.skipIf(process.platform === "darwin")("should return errors on failed cleanup", async () => { - // Use TCP mode to avoid stdin stream destruction issues - // Without this, on macOS there are intermittent test failures - // saying "Cannot call write after a stream was destroyed" - // because the JSON-RPC logic is still trying to write to stdin after - // the process has exited. - const client = new CopilotClient({ useStdio: false }); - - await client.createSession({ onPermissionRequest: approveAll }); - - // Kill the server processto force cleanup to fail - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const cliProcess = (client as any).cliProcess as ChildProcess; - expect(cliProcess).toBeDefined(); - cliProcess.kill("SIGKILL"); - await new Promise((resolve) => setTimeout(resolve, 100)); - - const errors = await client.stop(); - expect(errors.length).toBeGreaterThan(0); - expect(errors[0].message).toContain("Failed to disconnect session"); - }); + it.skipIf(process.platform === "darwin")( + "should stop cleanly when the server exits during cleanup", + async () => { + // Use TCP mode to avoid stdin stream destruction issues + // Without this, on macOS there are intermittent test failures + // saying "Cannot call write after a stream was destroyed" + // because the JSON-RPC logic is still trying to write to stdin after + // the process has exited. + const client = new CopilotClient({ useStdio: false }); + + await client.createSession({ onPermissionRequest: approveAll }); + + // Kill the server processto force cleanup to fail + // eslint-disable-next-line @typescript-eslint/no-explicit-any + const cliProcess = (client as any).cliProcess as ChildProcess; + expect(cliProcess).toBeDefined(); + cliProcess.kill("SIGKILL"); + await new Promise((resolve) => setTimeout(resolve, 100)); + + const errors = await client.stop(); + expect(client.getState()).toBe("disconnected"); + if (errors.length > 0) { + expect(errors[0].message).toContain("Failed to disconnect session"); + } + } + ); it("should forceStop without cleanup", async () => { const client = new CopilotClient({}); diff --git a/nodejs/test/extension.test.ts b/nodejs/test/extension.test.ts new file mode 100644 index 000000000..d9fcf8dfd --- /dev/null +++ b/nodejs/test/extension.test.ts @@ -0,0 +1,47 @@ +import { afterEach, describe, expect, it, vi } from "vitest"; +import { CopilotClient } from "../src/client.js"; +import { approveAll } from "../src/index.js"; +import { joinSession } from "../src/extension.js"; + +describe("joinSession", () => { + const originalSessionId = process.env.SESSION_ID; + + afterEach(() => { + if (originalSessionId === undefined) { + delete process.env.SESSION_ID; + } else { + process.env.SESSION_ID = originalSessionId; + } + vi.restoreAllMocks(); + }); + + it("defaults onPermissionRequest to no-result", async () => { + process.env.SESSION_ID = "session-123"; + const resumeSession = vi + .spyOn(CopilotClient.prototype, "resumeSession") + .mockResolvedValue({} as any); + + await joinSession({ tools: [] }); + + const [, config] = resumeSession.mock.calls[0]!; + expect(config.onPermissionRequest).toBeDefined(); + const result = await Promise.resolve( + config.onPermissionRequest!({ kind: "write" }, { sessionId: "session-123" }) + ); + expect(result).toEqual({ kind: "no-result" }); + expect(config.disableResume).toBe(true); + }); + + it("preserves an explicit onPermissionRequest handler", async () => { + process.env.SESSION_ID = "session-123"; + const resumeSession = vi + .spyOn(CopilotClient.prototype, "resumeSession") + .mockResolvedValue({} as any); + + await joinSession({ onPermissionRequest: approveAll, disableResume: false }); + + const [, config] = resumeSession.mock.calls[0]!; + expect(config.onPermissionRequest).toBe(approveAll); + expect(config.disableResume).toBe(false); + }); +}); diff --git a/python/copilot/client.py b/python/copilot/client.py index df09a755b..a7b558ad5 100644 --- a/python/copilot/client.py +++ b/python/copilot/client.py @@ -50,6 +50,10 @@ ToolResult, ) +NO_RESULT_PERMISSION_V2_ERROR = ( + "Permission handlers cannot return 'no-result' when connected to a protocol v2 server." +) + # Minimum protocol version this SDK can communicate with. # Servers reporting a version below this are rejected. MIN_PROTOCOL_VERSION = 2 @@ -1660,6 +1664,8 @@ async def _handle_permission_request_v2(self, params: dict) -> dict: try: perm_request = PermissionRequest.from_dict(permission_request) result = await session._handle_permission_request(perm_request) + if result.kind == "no-result": + raise ValueError(NO_RESULT_PERMISSION_V2_ERROR) result_payload: dict = {"kind": result.kind} if result.rules is not None: result_payload["rules"] = result.rules @@ -1670,6 +1676,14 @@ async def _handle_permission_request_v2(self, params: dict) -> dict: if result.path is not None: result_payload["path"] = result.path return {"result": result_payload} + except ValueError as exc: + if str(exc) == NO_RESULT_PERMISSION_V2_ERROR: + raise + return { + "result": { + "kind": "denied-no-approval-rule-and-could-not-request-from-user", + } + } except Exception: # pylint: disable=broad-except return { "result": { diff --git a/python/copilot/session.py b/python/copilot/session.py index ee46cbd7b..b4ae210df 100644 --- a/python/copilot/session.py +++ b/python/copilot/session.py @@ -139,15 +139,16 @@ async def send(self, options: MessageOptions) -> str: ... "attachments": [{"type": "file", "path": "./src/main.py"}] ... }) """ - response = await self._client.request( - "session.send", - { - "sessionId": self.session_id, - "prompt": options["prompt"], - "attachments": options.get("attachments"), - "mode": options.get("mode"), - }, - ) + params: dict[str, Any] = { + "sessionId": self.session_id, + "prompt": options["prompt"], + } + if "attachments" in options: + params["attachments"] = options["attachments"] + if "mode" in options: + params["mode"] = options["mode"] + + response = await self._client.request("session.send", params) return response["messageId"] async def send_and_wait( @@ -387,6 +388,8 @@ async def _execute_permission_and_respond( result = await result result = cast(PermissionRequestResult, result) + if result.kind == "no-result": + return perm_result = SessionPermissionsHandlePendingPermissionRequestParamsResult( kind=Kind(result.kind), diff --git a/python/copilot/types.py b/python/copilot/types.py index 33764e5d1..9a397c708 100644 --- a/python/copilot/types.py +++ b/python/copilot/types.py @@ -187,6 +187,7 @@ class SystemMessageReplaceConfig(TypedDict): "denied-by-content-exclusion-policy", "denied-no-approval-rule-and-could-not-request-from-user", "denied-interactively-by-user", + "no-result", ] diff --git a/python/e2e/test_client.py b/python/e2e/test_client.py index 1f7c76c04..1395a3888 100644 --- a/python/e2e/test_client.py +++ b/python/e2e/test_client.py @@ -57,11 +57,14 @@ async def test_should_raise_exception_group_on_failed_cleanup(self): process.kill() await asyncio.sleep(0.1) - with pytest.raises(ExceptionGroup) as exc_info: + try: await client.stop() - assert len(exc_info.value.exceptions) > 0 - assert isinstance(exc_info.value.exceptions[0], StopError) - assert "Failed to disconnect session" in exc_info.value.exceptions[0].message + except ExceptionGroup as exc: + assert len(exc.exceptions) > 0 + assert isinstance(exc.exceptions[0], StopError) + assert "Failed to disconnect session" in exc.exceptions[0].message + else: + assert client.get_state() == "disconnected" finally: await client.force_stop() diff --git a/python/test_client.py b/python/test_client.py index 4a06966d4..62ae7b188 100644 --- a/python/test_client.py +++ b/python/test_client.py @@ -6,7 +6,7 @@ import pytest -from copilot import CopilotClient, PermissionHandler, define_tool +from copilot import CopilotClient, PermissionHandler, PermissionRequestResult, define_tool from copilot.types import ModelCapabilities, ModelInfo, ModelLimits, ModelSupports from e2e.testharness import CLI_PATH @@ -22,6 +22,28 @@ async def test_create_session_raises_without_permission_handler(self): finally: await client.force_stop() + @pytest.mark.asyncio + async def test_v2_permission_adapter_rejects_no_result(self): + client = CopilotClient({"cli_path": CLI_PATH}) + await client.start() + try: + session = await client.create_session( + { + "on_permission_request": lambda request, invocation: PermissionRequestResult( + kind="no-result" + ) + } + ) + with pytest.raises(ValueError, match="protocol v2 server"): + await client._handle_permission_request_v2( + { + "sessionId": session.session_id, + "permissionRequest": {"kind": "write"}, + } + ) + finally: + await client.force_stop() + @pytest.mark.asyncio async def test_resume_session_raises_without_permission_handler(self): client = CopilotClient({"cli_path": CLI_PATH})