+ * Used internally to find a session that can handle hooks invocations for + * sub-agent sessions whose IDs are not directly tracked by the SDK. + * + * @return {@code true} if hooks are registered and at least one handler is set + */ + boolean hasHooks() { + SessionHooks hooks = hooksHandler.get(); + return hooks != null && hooks.hasHooks(); + } + + /** + * Returns whether this session has a permission handler registered. + *
+ * Used internally to find a session that can handle permission requests for + * sub-agent sessions whose IDs are not directly tracked by the SDK. + * + * @return {@code true} if a permission handler is registered + */ + boolean hasPermissionHandler() { + return permissionHandler.get() != null; + } + /** * Registers transform callbacks for system message sections. *
diff --git a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java index f1d488105..030643240 100644 --- a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java +++ b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java @@ -191,6 +191,12 @@ private void handlePermissionRequest(JsonRpcClient rpc, String requestId, JsonNo JsonNode permissionRequest = params.get("permissionRequest"); CopilotSession session = sessions.get(sessionId); + if (session == null) { + // The CLI may send permission requests for sub-agent sessions + // whose IDs are not tracked by the SDK. Fall back to any + // registered session that has a permission handler. + session = findSessionWithPermissionHandler(); + } if (session == null) { var result = new PermissionRequestResult() .setKind(PermissionRequestResultKind.DENIED_COULD_NOT_REQUEST_FROM_USER); @@ -293,7 +299,14 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par CopilotSession session = sessions.get(sessionId); if (session == null) { - rpc.sendErrorResponse(Long.parseLong(requestId), -32602, "Unknown session " + sessionId); + // The CLI may send hooks.invoke for sub-agent sessions whose IDs + // are not tracked by the SDK. Fall back to any registered session + // that has hooks so that application-level hooks (e.g. onPreToolUse) + // also fire for sub-agent tool calls. + session = findSessionWithHooks(); + } + if (session == null) { + rpc.sendResponse(Long.parseLong(requestId), Collections.singletonMap("output", null)); return; } @@ -318,6 +331,41 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par }); } + /** + * Finds a session that has hooks registered. + *
+ * When the CLI creates sub-agent sessions internally, their session IDs are not + * in the SDK's session map. This method searches all registered sessions to + * find one with hooks, enabling hook handlers to fire for sub-agent tool calls. + * + * @return a session with hooks, or {@code null} if none found + */ + private CopilotSession findSessionWithHooks() { + for (CopilotSession s : sessions.values()) { + if (s.hasHooks()) { + return s; + } + } + return null; + } + + /** + * Finds a session that has a permission handler registered. + *
+ * Similar to {@link #findSessionWithHooks()}, this enables permission handlers + * to fire for sub-agent sessions whose IDs are not tracked by the SDK. + * + * @return a session with a permission handler, or {@code null} if none found + */ + private CopilotSession findSessionWithPermissionHandler() { + for (CopilotSession s : sessions.values()) { + if (s.hasPermissionHandler()) { + return s; + } + } + return null; + } + /** * Functional interface for dispatching lifecycle events. */ diff --git a/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java b/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java index 315a38b90..186546f0c 100644 --- a/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java +++ b/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java @@ -295,7 +295,8 @@ void toolCallHandlerFails() throws Exception { // ===== permission.request tests ===== @Test - void permissionRequestWithUnknownSession() throws Exception { + void permissionRequestWithUnknownSessionAndNoFallback() throws Exception { + // No sessions at all — returns denied ObjectNode params = MAPPER.createObjectNode(); params.put("sessionId", "nonexistent"); params.putObject("permissionRequest"); @@ -307,6 +308,24 @@ void permissionRequestWithUnknownSession() throws Exception { assertEquals("denied-no-approval-rule-and-could-not-request-from-user", result.get("kind").asText()); } + @Test + void permissionRequestFallsBackToSessionWithHandlerForSubAgent() throws Exception { + // Parent session has permission handler; sub-agent session ID not in map. + CopilotSession parent = createSession("parent-session"); + parent.registerPermissionHandler((request, invocation) -> CompletableFuture + .completedFuture(new PermissionRequestResult().setKind("allow"))); + + ObjectNode params = MAPPER.createObjectNode(); + params.put("sessionId", "subagent-session-id"); + params.putObject("permissionRequest"); + + invokeHandler("permission.request", "15", params); + + JsonNode response = readResponse(); + JsonNode result = response.get("result").get("result"); + assertEquals("allow", result.get("kind").asText()); + } + @Test void permissionRequestWithHandler() throws Exception { CopilotSession session = createSession("s1"); @@ -453,7 +472,8 @@ void userInputRequestHandlerFails() throws Exception { // ===== hooks.invoke tests ===== @Test - void hooksInvokeWithUnknownSession() throws Exception { + void hooksInvokeWithUnknownSessionAndNoFallback() throws Exception { + // No sessions at all — returns null output (no-op) ObjectNode params = MAPPER.createObjectNode(); params.put("sessionId", "nonexistent"); params.put("hookType", "preToolUse"); @@ -462,8 +482,49 @@ void hooksInvokeWithUnknownSession() throws Exception { invokeHandler("hooks.invoke", "30", params); JsonNode response = readResponse(); - assertNotNull(response.get("error")); - assertEquals(-32602, response.get("error").get("code").asInt()); + JsonNode output = response.get("result").get("output"); + assertTrue(output == null || output.isNull(), + "Output should be null when no sessions with hooks are registered"); + } + + @Test + void hooksInvokeFallsBackToSessionWithHooksForSubAgent() throws Exception { + // Parent session has hooks; sub-agent session ID is not in the map. + // The dispatcher should fall back to the parent session's hooks. + CopilotSession parent = createSession("parent-session"); + parent.registerHooks(new SessionHooks().setOnPreToolUse( + (input, invocation) -> CompletableFuture.completedFuture(PreToolUseHookOutput.allow()))); + + ObjectNode params = MAPPER.createObjectNode(); + params.put("sessionId", "subagent-session-id"); + params.put("hookType", "preToolUse"); + ObjectNode input = params.putObject("input"); + input.put("toolName", "glob"); + input.put("toolCallId", "tc-sub"); + + invokeHandler("hooks.invoke", "35", params); + + JsonNode response = readResponse(); + JsonNode output = response.get("result").get("output"); + assertNotNull(output, "Hooks should fire for sub-agent tool calls via fallback"); + assertEquals("allow", output.get("permissionDecision").asText()); + } + + @Test + void hooksInvokeFallbackSkipsSessionWithoutHooks() throws Exception { + // Session exists but has no hooks — should not be used as fallback + createSession("no-hooks-session"); + + ObjectNode params = MAPPER.createObjectNode(); + params.put("sessionId", "subagent-session-id"); + params.put("hookType", "preToolUse"); + params.putObject("input"); + + invokeHandler("hooks.invoke", "36", params); + + JsonNode response = readResponse(); + JsonNode output = response.get("result").get("output"); + assertTrue(output == null || output.isNull(), "Should return null when no session with hooks is found"); } @Test From 5a8a2b4a3cd343ddc05821de4dc336feef3ca989 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 11 Apr 2026 23:31:17 +0000 Subject: [PATCH 3/4] Improve Javadoc for findSessionWithHooks and findSessionWithPermissionHandler Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/868c6fed-c57d-4d9f-806c-eca509096672 Co-authored-by: brunoborges <129743+brunoborges@users.noreply.github.com> --- .../java/com/github/copilot/sdk/RpcHandlerDispatcher.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java index 030643240..b0b319a0c 100644 --- a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java +++ b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java @@ -337,6 +337,10 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par * When the CLI creates sub-agent sessions internally, their session IDs are not * in the SDK's session map. This method searches all registered sessions to * find one with hooks, enabling hook handlers to fire for sub-agent tool calls. + *
+ * If multiple sessions have hooks registered, the first one found is returned. + * In practice, SDK users typically register hooks on a single session that + * covers all sub-agent activity. * * @return a session with hooks, or {@code null} if none found */ @@ -354,6 +358,10 @@ private CopilotSession findSessionWithHooks() { *
* Similar to {@link #findSessionWithHooks()}, this enables permission handlers * to fire for sub-agent sessions whose IDs are not tracked by the SDK. + *
+ * If multiple sessions have permission handlers, the first one found is + * returned. In practice, SDK users typically register a single permission + * handler that covers all sub-agent activity. * * @return a session with a permission handler, or {@code null} if none found */ From 1d0bf62dc54edbccd61e0dd9ebfa86ca76695127 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sun, 12 Apr 2026 00:29:20 +0000 Subject: [PATCH 4/4] Revert SDK-level fallback to match upstream .NET/Node.js/Go behavior All three upstream SDKs (C#, TypeScript, Go) throw/error for unknown session IDs in hooks.invoke and permission.request handlers with no fallback logic. The sub-agent hooks gap exists in all SDK implementations and should be addressed at the CLI/protocol level rather than with SDK heuristics. Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/50bf8907-712c-4792-aeea-219889b1ffa9 Co-authored-by: brunoborges <129743+brunoborges@users.noreply.github.com> --- .../github/copilot/sdk/CopilotSession.java | 25 ------- .../copilot/sdk/RpcHandlerDispatcher.java | 58 +--------------- .../copilot/sdk/RpcHandlerDispatcherTest.java | 69 ++----------------- 3 files changed, 5 insertions(+), 147 deletions(-) diff --git a/src/main/java/com/github/copilot/sdk/CopilotSession.java b/src/main/java/com/github/copilot/sdk/CopilotSession.java index e219edbfd..23b1b5368 100644 --- a/src/main/java/com/github/copilot/sdk/CopilotSession.java +++ b/src/main/java/com/github/copilot/sdk/CopilotSession.java @@ -1298,31 +1298,6 @@ void registerHooks(SessionHooks hooks) { hooksHandler.set(hooks); } - /** - * Returns whether this session has hooks registered. - *
- * Used internally to find a session that can handle hooks invocations for - * sub-agent sessions whose IDs are not directly tracked by the SDK. - * - * @return {@code true} if hooks are registered and at least one handler is set - */ - boolean hasHooks() { - SessionHooks hooks = hooksHandler.get(); - return hooks != null && hooks.hasHooks(); - } - - /** - * Returns whether this session has a permission handler registered. - *
- * Used internally to find a session that can handle permission requests for - * sub-agent sessions whose IDs are not directly tracked by the SDK. - * - * @return {@code true} if a permission handler is registered - */ - boolean hasPermissionHandler() { - return permissionHandler.get() != null; - } - /** * Registers transform callbacks for system message sections. *
diff --git a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java index b0b319a0c..f1d488105 100644 --- a/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java +++ b/src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java @@ -191,12 +191,6 @@ private void handlePermissionRequest(JsonRpcClient rpc, String requestId, JsonNo JsonNode permissionRequest = params.get("permissionRequest"); CopilotSession session = sessions.get(sessionId); - if (session == null) { - // The CLI may send permission requests for sub-agent sessions - // whose IDs are not tracked by the SDK. Fall back to any - // registered session that has a permission handler. - session = findSessionWithPermissionHandler(); - } if (session == null) { var result = new PermissionRequestResult() .setKind(PermissionRequestResultKind.DENIED_COULD_NOT_REQUEST_FROM_USER); @@ -299,14 +293,7 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par CopilotSession session = sessions.get(sessionId); if (session == null) { - // The CLI may send hooks.invoke for sub-agent sessions whose IDs - // are not tracked by the SDK. Fall back to any registered session - // that has hooks so that application-level hooks (e.g. onPreToolUse) - // also fire for sub-agent tool calls. - session = findSessionWithHooks(); - } - if (session == null) { - rpc.sendResponse(Long.parseLong(requestId), Collections.singletonMap("output", null)); + rpc.sendErrorResponse(Long.parseLong(requestId), -32602, "Unknown session " + sessionId); return; } @@ -331,49 +318,6 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par }); } - /** - * Finds a session that has hooks registered. - *
- * When the CLI creates sub-agent sessions internally, their session IDs are not - * in the SDK's session map. This method searches all registered sessions to - * find one with hooks, enabling hook handlers to fire for sub-agent tool calls. - *
- * If multiple sessions have hooks registered, the first one found is returned. - * In practice, SDK users typically register hooks on a single session that - * covers all sub-agent activity. - * - * @return a session with hooks, or {@code null} if none found - */ - private CopilotSession findSessionWithHooks() { - for (CopilotSession s : sessions.values()) { - if (s.hasHooks()) { - return s; - } - } - return null; - } - - /** - * Finds a session that has a permission handler registered. - *
- * Similar to {@link #findSessionWithHooks()}, this enables permission handlers - * to fire for sub-agent sessions whose IDs are not tracked by the SDK. - *
- * If multiple sessions have permission handlers, the first one found is - * returned. In practice, SDK users typically register a single permission - * handler that covers all sub-agent activity. - * - * @return a session with a permission handler, or {@code null} if none found - */ - private CopilotSession findSessionWithPermissionHandler() { - for (CopilotSession s : sessions.values()) { - if (s.hasPermissionHandler()) { - return s; - } - } - return null; - } - /** * Functional interface for dispatching lifecycle events. */ diff --git a/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java b/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java index 186546f0c..315a38b90 100644 --- a/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java +++ b/src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java @@ -295,8 +295,7 @@ void toolCallHandlerFails() throws Exception { // ===== permission.request tests ===== @Test - void permissionRequestWithUnknownSessionAndNoFallback() throws Exception { - // No sessions at all — returns denied + void permissionRequestWithUnknownSession() throws Exception { ObjectNode params = MAPPER.createObjectNode(); params.put("sessionId", "nonexistent"); params.putObject("permissionRequest"); @@ -308,24 +307,6 @@ void permissionRequestWithUnknownSessionAndNoFallback() throws Exception { assertEquals("denied-no-approval-rule-and-could-not-request-from-user", result.get("kind").asText()); } - @Test - void permissionRequestFallsBackToSessionWithHandlerForSubAgent() throws Exception { - // Parent session has permission handler; sub-agent session ID not in map. - CopilotSession parent = createSession("parent-session"); - parent.registerPermissionHandler((request, invocation) -> CompletableFuture - .completedFuture(new PermissionRequestResult().setKind("allow"))); - - ObjectNode params = MAPPER.createObjectNode(); - params.put("sessionId", "subagent-session-id"); - params.putObject("permissionRequest"); - - invokeHandler("permission.request", "15", params); - - JsonNode response = readResponse(); - JsonNode result = response.get("result").get("result"); - assertEquals("allow", result.get("kind").asText()); - } - @Test void permissionRequestWithHandler() throws Exception { CopilotSession session = createSession("s1"); @@ -472,8 +453,7 @@ void userInputRequestHandlerFails() throws Exception { // ===== hooks.invoke tests ===== @Test - void hooksInvokeWithUnknownSessionAndNoFallback() throws Exception { - // No sessions at all — returns null output (no-op) + void hooksInvokeWithUnknownSession() throws Exception { ObjectNode params = MAPPER.createObjectNode(); params.put("sessionId", "nonexistent"); params.put("hookType", "preToolUse"); @@ -482,49 +462,8 @@ void hooksInvokeWithUnknownSessionAndNoFallback() throws Exception { invokeHandler("hooks.invoke", "30", params); JsonNode response = readResponse(); - JsonNode output = response.get("result").get("output"); - assertTrue(output == null || output.isNull(), - "Output should be null when no sessions with hooks are registered"); - } - - @Test - void hooksInvokeFallsBackToSessionWithHooksForSubAgent() throws Exception { - // Parent session has hooks; sub-agent session ID is not in the map. - // The dispatcher should fall back to the parent session's hooks. - CopilotSession parent = createSession("parent-session"); - parent.registerHooks(new SessionHooks().setOnPreToolUse( - (input, invocation) -> CompletableFuture.completedFuture(PreToolUseHookOutput.allow()))); - - ObjectNode params = MAPPER.createObjectNode(); - params.put("sessionId", "subagent-session-id"); - params.put("hookType", "preToolUse"); - ObjectNode input = params.putObject("input"); - input.put("toolName", "glob"); - input.put("toolCallId", "tc-sub"); - - invokeHandler("hooks.invoke", "35", params); - - JsonNode response = readResponse(); - JsonNode output = response.get("result").get("output"); - assertNotNull(output, "Hooks should fire for sub-agent tool calls via fallback"); - assertEquals("allow", output.get("permissionDecision").asText()); - } - - @Test - void hooksInvokeFallbackSkipsSessionWithoutHooks() throws Exception { - // Session exists but has no hooks — should not be used as fallback - createSession("no-hooks-session"); - - ObjectNode params = MAPPER.createObjectNode(); - params.put("sessionId", "subagent-session-id"); - params.put("hookType", "preToolUse"); - params.putObject("input"); - - invokeHandler("hooks.invoke", "36", params); - - JsonNode response = readResponse(); - JsonNode output = response.get("result").get("output"); - assertTrue(output == null || output.isNull(), "Should return null when no session with hooks is found"); + assertNotNull(response.get("error")); + assertEquals(-32602, response.get("error").get("code").asInt()); } @Test