Skip to content

Commit df45f3a

Browse files
Copilotedburns
andauthored
Fix onPreToolUse hook for sub-agent sessions via fallback dispatch
Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/4ea5c73c-1940-4862-8afb-019a8de0bca2 Co-authored-by: edburns <75821+edburns@users.noreply.github.com>
1 parent 150e7e5 commit df45f3a

3 files changed

Lines changed: 130 additions & 5 deletions

File tree

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

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1219,6 +1219,25 @@ void registerPermissionHandler(PermissionHandler handler) {
12191219
permissionHandler.set(handler);
12201220
}
12211221

1222+
/**
1223+
* Returns whether this session has a permission handler registered.
1224+
*
1225+
* @return {@code true} if a permission handler is set
1226+
*/
1227+
boolean hasPermissionHandler() {
1228+
return permissionHandler.get() != null;
1229+
}
1230+
1231+
/**
1232+
* Returns whether this session has any hooks registered.
1233+
*
1234+
* @return {@code true} if a hooks handler with at least one hook is set
1235+
*/
1236+
boolean hasHooksHandler() {
1237+
SessionHooks hooks = hooksHandler.get();
1238+
return hooks != null && hooks.hasHooks();
1239+
}
1240+
12221241
/**
12231242
* Handles a permission request from the Copilot CLI.
12241243
* <p>

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

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

192192
CopilotSession session = sessions.get(sessionId);
193+
if (session == null) {
194+
// Fall back to any registered session that has a permission handler.
195+
// This handles sub-agent sessions created internally by the CLI whose IDs
196+
// are not in the SDK's session registry.
197+
session = findSessionWithPermissionHandler();
198+
}
193199
if (session == null) {
194200
var result = new PermissionRequestResult()
195201
.setKind(PermissionRequestResultKind.DENIED_COULD_NOT_REQUEST_FROM_USER);
@@ -292,7 +298,14 @@ private void handleHooksInvoke(JsonRpcClient rpc, String requestId, JsonNode par
292298

293299
CopilotSession session = sessions.get(sessionId);
294300
if (session == null) {
295-
rpc.sendErrorResponse(Long.parseLong(requestId), -32602, "Unknown session " + sessionId);
301+
// Fall back to any registered session that has hooks registered.
302+
// This handles sub-agent sessions created internally by the CLI whose IDs
303+
// are not in the SDK's session registry.
304+
session = findSessionWithHooks();
305+
}
306+
if (session == null) {
307+
// No session with hooks — return null output (no-op) rather than an error.
308+
rpc.sendResponse(Long.parseLong(requestId), Collections.singletonMap("output", null));
296309
return;
297310
}
298311

@@ -366,6 +379,34 @@ private void handleSystemMessageTransform(JsonRpcClient rpc, String requestId, J
366379
});
367380
}
368381

382+
/**
383+
* Finds the first registered session that has a hooks handler registered.
384+
*
385+
* @return a session with hooks, or {@code null} if none is found
386+
*/
387+
private CopilotSession findSessionWithHooks() {
388+
for (CopilotSession s : sessions.values()) {
389+
if (s.hasHooksHandler()) {
390+
return s;
391+
}
392+
}
393+
return null;
394+
}
395+
396+
/**
397+
* Finds the first registered session that has a permission handler registered.
398+
*
399+
* @return a session with a permission handler, or {@code null} if none is found
400+
*/
401+
private CopilotSession findSessionWithPermissionHandler() {
402+
for (CopilotSession s : sessions.values()) {
403+
if (s.hasPermissionHandler()) {
404+
return s;
405+
}
406+
}
407+
return null;
408+
}
409+
369410
private void runAsync(Runnable task) {
370411
try {
371412
if (executor != null) {

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

Lines changed: 69 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 registered → denied
299300
ObjectNode params = MAPPER.createObjectNode();
300301
params.put("sessionId", "nonexistent");
301302
params.putObject("permissionRequest");
@@ -307,6 +308,25 @@ 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 permissionRequestWithSubAgentSessionFallsBackToParentWithHandler() throws Exception {
313+
// Register a parent session with a permission handler, invoke with a sub-agent
314+
// session ID
315+
CopilotSession parentSession = createSession("parent-session");
316+
parentSession.registerPermissionHandler((request, invocation) -> CompletableFuture
317+
.completedFuture(new PermissionRequestResult().setKind("allow")));
318+
319+
ObjectNode params = MAPPER.createObjectNode();
320+
params.put("sessionId", "sub-agent-session-id"); // Not in registry
321+
params.putObject("permissionRequest");
322+
323+
invokeHandler("permission.request", "15", params);
324+
325+
JsonNode response = readResponse();
326+
JsonNode result = response.get("result").get("result");
327+
assertEquals("allow", result.get("kind").asText());
328+
}
329+
310330
@Test
311331
void permissionRequestWithHandler() throws Exception {
312332
CopilotSession session = createSession("s1");
@@ -453,7 +473,8 @@ void userInputRequestHandlerFails() throws Exception {
453473
// ===== hooks.invoke tests =====
454474

455475
@Test
456-
void hooksInvokeWithUnknownSession() throws Exception {
476+
void hooksInvokeWithUnknownSessionAndNoFallback() throws Exception {
477+
// No sessions registered at all → returns null output (no-op)
457478
ObjectNode params = MAPPER.createObjectNode();
458479
params.put("sessionId", "nonexistent");
459480
params.put("hookType", "preToolUse");
@@ -462,8 +483,52 @@ void hooksInvokeWithUnknownSession() throws Exception {
462483
invokeHandler("hooks.invoke", "30", params);
463484

464485
JsonNode response = readResponse();
465-
assertNotNull(response.get("error"));
466-
assertEquals(-32602, response.get("error").get("code").asInt());
486+
// No sessions with hooks → null output, not an error
487+
assertNull(response.get("error"), "Should not return an error when no fallback session exists");
488+
JsonNode output = response.get("result").get("output");
489+
assertTrue(output == null || output.isNull(), "Output should be null when no session with hooks is found");
490+
}
491+
492+
@Test
493+
void hooksInvokeWithSubAgentSessionFallsBackToParentWithHooks() throws Exception {
494+
// Register a parent session with hooks, but invoke with a sub-agent session ID
495+
CopilotSession parentSession = createSession("parent-session");
496+
parentSession.registerHooks(new SessionHooks().setOnPreToolUse(
497+
(input, invocation) -> CompletableFuture.completedFuture(PreToolUseHookOutput.allow())));
498+
499+
ObjectNode params = MAPPER.createObjectNode();
500+
params.put("sessionId", "sub-agent-session-id"); // Not in registry
501+
params.put("hookType", "preToolUse");
502+
ObjectNode input = params.putObject("input");
503+
input.put("toolName", "glob");
504+
input.put("toolCallId", "tc-sub-1");
505+
506+
invokeHandler("hooks.invoke", "35", params);
507+
508+
JsonNode response = readResponse();
509+
assertNull(response.get("error"), "Should not return an error when a fallback session with hooks exists");
510+
JsonNode output = response.get("result").get("output");
511+
assertNotNull(output);
512+
assertEquals("allow", output.get("permissionDecision").asText());
513+
}
514+
515+
@Test
516+
void hooksInvokeWithSubAgentSessionAndNoSessionHasHooks() throws Exception {
517+
// Register a parent session WITHOUT hooks, invoke with unknown sub-agent
518+
// session ID
519+
createSession("parent-session-no-hooks");
520+
521+
ObjectNode params = MAPPER.createObjectNode();
522+
params.put("sessionId", "sub-agent-session-id"); // Not in registry
523+
params.put("hookType", "preToolUse");
524+
params.putObject("input");
525+
526+
invokeHandler("hooks.invoke", "36", params);
527+
528+
JsonNode response = readResponse();
529+
assertNull(response.get("error"), "Should not return an error when no fallback session has hooks");
530+
JsonNode output = response.get("result").get("output");
531+
assertTrue(output == null || output.isNull(), "Output should be null when no session with hooks is found");
467532
}
468533

469534
@Test

0 commit comments

Comments
 (0)