Skip to content

Commit b1efa7b

Browse files
Copilotbrunoborges
andauthored
Fix onPreToolUse hook not firing for sub-agent tool calls
When the CLI creates sub-agent sessions internally, their session IDs are not in the SDK's session registry. The hooks.invoke and permission.request handlers now fall back to any registered session that has the appropriate handler, enabling hooks and permission checks to fire for sub-agent tool calls. 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>
1 parent 2faaaca commit b1efa7b

File tree

3 files changed

+139
-5
lines changed

3 files changed

+139
-5
lines changed

src/main/java/com/github/copilot/sdk/CopilotSession.java

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1298,6 +1298,31 @@ void registerHooks(SessionHooks hooks) {
12981298
hooksHandler.set(hooks);
12991299
}
13001300

1301+
/**
1302+
* Returns whether this session has hooks registered.
1303+
* <p>
1304+
* Used internally to find a session that can handle hooks invocations for
1305+
* sub-agent sessions whose IDs are not directly tracked by the SDK.
1306+
*
1307+
* @return {@code true} if hooks are registered and at least one handler is set
1308+
*/
1309+
boolean hasHooks() {
1310+
SessionHooks hooks = hooksHandler.get();
1311+
return hooks != null && hooks.hasHooks();
1312+
}
1313+
1314+
/**
1315+
* Returns whether this session has a permission handler registered.
1316+
* <p>
1317+
* Used internally to find a session that can handle permission requests for
1318+
* sub-agent sessions whose IDs are not directly tracked by the SDK.
1319+
*
1320+
* @return {@code true} if a permission handler is registered
1321+
*/
1322+
boolean hasPermissionHandler() {
1323+
return permissionHandler.get() != null;
1324+
}
1325+
13011326
/**
13021327
* Registers transform callbacks for system message sections.
13031328
* <p>

src/main/java/com/github/copilot/sdk/RpcHandlerDispatcher.java

Lines changed: 49 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,12 @@ private void handlePermissionRequest(JsonRpcClient rpc, String requestId, JsonNo
191191
JsonNode permissionRequest = params.get("permissionRequest");
192192

193193
CopilotSession session = sessions.get(sessionId);
194+
if (session == null) {
195+
// The CLI may send permission requests for sub-agent sessions
196+
// whose IDs are not tracked by the SDK. Fall back to any
197+
// registered session that has a permission handler.
198+
session = findSessionWithPermissionHandler();
199+
}
194200
if (session == null) {
195201
var result = new PermissionRequestResult()
196202
.setKind(PermissionRequestResultKind.DENIED_COULD_NOT_REQUEST_FROM_USER);
@@ -293,7 +299,14 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par
293299

294300
CopilotSession session = sessions.get(sessionId);
295301
if (session == null) {
296-
rpc.sendErrorResponse(Long.parseLong(requestId), -32602, "Unknown session " + sessionId);
302+
// The CLI may send hooks.invoke for sub-agent sessions whose IDs
303+
// are not tracked by the SDK. Fall back to any registered session
304+
// that has hooks so that application-level hooks (e.g. onPreToolUse)
305+
// also fire for sub-agent tool calls.
306+
session = findSessionWithHooks();
307+
}
308+
if (session == null) {
309+
rpc.sendResponse(Long.parseLong(requestId), Collections.singletonMap("output", null));
297310
return;
298311
}
299312

@@ -318,6 +331,41 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par
318331
});
319332
}
320333

334+
/**
335+
* Finds a session that has hooks registered.
336+
* <p>
337+
* When the CLI creates sub-agent sessions internally, their session IDs are not
338+
* in the SDK's session map. This method searches all registered sessions to
339+
* find one with hooks, enabling hook handlers to fire for sub-agent tool calls.
340+
*
341+
* @return a session with hooks, or {@code null} if none found
342+
*/
343+
private CopilotSession findSessionWithHooks() {
344+
for (CopilotSession s : sessions.values()) {
345+
if (s.hasHooks()) {
346+
return s;
347+
}
348+
}
349+
return null;
350+
}
351+
352+
/**
353+
* Finds a session that has a permission handler registered.
354+
* <p>
355+
* Similar to {@link #findSessionWithHooks()}, this enables permission handlers
356+
* to fire for sub-agent sessions whose IDs are not tracked by the SDK.
357+
*
358+
* @return a session with a permission handler, or {@code null} if none found
359+
*/
360+
private CopilotSession findSessionWithPermissionHandler() {
361+
for (CopilotSession s : sessions.values()) {
362+
if (s.hasPermissionHandler()) {
363+
return s;
364+
}
365+
}
366+
return null;
367+
}
368+
321369
/**
322370
* Functional interface for dispatching lifecycle events.
323371
*/

src/test/java/com/github/copilot/sdk/RpcHandlerDispatcherTest.java

Lines changed: 65 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,8 @@ void toolCallHandlerFails() throws Exception {
295295
// ===== permission.request tests =====
296296

297297
@Test
298-
void permissionRequestWithUnknownSession() throws Exception {
298+
void permissionRequestWithUnknownSessionAndNoFallback() throws Exception {
299+
// No sessions at all — returns denied
299300
ObjectNode params = MAPPER.createObjectNode();
300301
params.put("sessionId", "nonexistent");
301302
params.putObject("permissionRequest");
@@ -307,6 +308,24 @@ void permissionRequestWithUnknownSession() throws Exception {
307308
assertEquals("denied-no-approval-rule-and-could-not-request-from-user", result.get("kind").asText());
308309
}
309310

311+
@Test
312+
void permissionRequestFallsBackToSessionWithHandlerForSubAgent() throws Exception {
313+
// Parent session has permission handler; sub-agent session ID not in map.
314+
CopilotSession parent = createSession("parent-session");
315+
parent.registerPermissionHandler((request, invocation) -> CompletableFuture
316+
.completedFuture(new PermissionRequestResult().setKind("allow")));
317+
318+
ObjectNode params = MAPPER.createObjectNode();
319+
params.put("sessionId", "subagent-session-id");
320+
params.putObject("permissionRequest");
321+
322+
invokeHandler("permission.request", "15", params);
323+
324+
JsonNode response = readResponse();
325+
JsonNode result = response.get("result").get("result");
326+
assertEquals("allow", result.get("kind").asText());
327+
}
328+
310329
@Test
311330
void permissionRequestWithHandler() throws Exception {
312331
CopilotSession session = createSession("s1");
@@ -453,7 +472,8 @@ void userInputRequestHandlerFails() throws Exception {
453472
// ===== hooks.invoke tests =====
454473

455474
@Test
456-
void hooksInvokeWithUnknownSession() throws Exception {
475+
void hooksInvokeWithUnknownSessionAndNoFallback() throws Exception {
476+
// No sessions at all — returns null output (no-op)
457477
ObjectNode params = MAPPER.createObjectNode();
458478
params.put("sessionId", "nonexistent");
459479
params.put("hookType", "preToolUse");
@@ -462,8 +482,49 @@ void hooksInvokeWithUnknownSession() throws Exception {
462482
invokeHandler("hooks.invoke", "30", params);
463483

464484
JsonNode response = readResponse();
465-
assertNotNull(response.get("error"));
466-
assertEquals(-32602, response.get("error").get("code").asInt());
485+
JsonNode output = response.get("result").get("output");
486+
assertTrue(output == null || output.isNull(),
487+
"Output should be null when no sessions with hooks are registered");
488+
}
489+
490+
@Test
491+
void hooksInvokeFallsBackToSessionWithHooksForSubAgent() throws Exception {
492+
// Parent session has hooks; sub-agent session ID is not in the map.
493+
// The dispatcher should fall back to the parent session's hooks.
494+
CopilotSession parent = createSession("parent-session");
495+
parent.registerHooks(new SessionHooks().setOnPreToolUse(
496+
(input, invocation) -> CompletableFuture.completedFuture(PreToolUseHookOutput.allow())));
497+
498+
ObjectNode params = MAPPER.createObjectNode();
499+
params.put("sessionId", "subagent-session-id");
500+
params.put("hookType", "preToolUse");
501+
ObjectNode input = params.putObject("input");
502+
input.put("toolName", "glob");
503+
input.put("toolCallId", "tc-sub");
504+
505+
invokeHandler("hooks.invoke", "35", params);
506+
507+
JsonNode response = readResponse();
508+
JsonNode output = response.get("result").get("output");
509+
assertNotNull(output, "Hooks should fire for sub-agent tool calls via fallback");
510+
assertEquals("allow", output.get("permissionDecision").asText());
511+
}
512+
513+
@Test
514+
void hooksInvokeFallbackSkipsSessionWithoutHooks() throws Exception {
515+
// Session exists but has no hooks — should not be used as fallback
516+
createSession("no-hooks-session");
517+
518+
ObjectNode params = MAPPER.createObjectNode();
519+
params.put("sessionId", "subagent-session-id");
520+
params.put("hookType", "preToolUse");
521+
params.putObject("input");
522+
523+
invokeHandler("hooks.invoke", "36", params);
524+
525+
JsonNode response = readResponse();
526+
JsonNode output = response.get("result").get("output");
527+
assertTrue(output == null || output.isNull(), "Should return null when no session with hooks is found");
467528
}
468529

469530
@Test

0 commit comments

Comments
 (0)