Address review feedback: use generated ConnectParams/ConnectResult, allow null copilotHome, strengthen permission test assertion, add instructionDirectories unit tests

Agent-Logs-Url: https://github.com/github/copilot-sdk-java/sessions/bf21fd78-4e30-40e7-8e6b-536294818f16

Co-authored-by: edburns <75821+edburns@users.noreply.github.com>

Author: Copilot

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

@@ -21,6 +21,7 @@
 
 import com.github.copilot.sdk.json.CopilotClientOptions;
 import com.github.copilot.sdk.json.CreateSessionResponse;
+import com.github.copilot.sdk.generated.rpc.ConnectParams;
 import com.github.copilot.sdk.generated.rpc.ServerRpc;
 import com.github.copilot.sdk.json.DeleteSessionResponse;
 import com.github.copilot.sdk.json.GetAuthStatusResponse;
@@ -230,11 +231,13 @@ private void verifyProtocolVersion(Connection connection) throws Exception {
 
         try {
             // Try the new 'connect' RPC which supports connection tokens
-            var connectParams = new HashMap<String, Object>();
-            connectParams.put("token", effectiveConnectionToken);
-            var connectResponse = connection.rpc.invoke("connect", connectParams, ConnectResult.class).get(30,
-                    TimeUnit.SECONDS);
-            serverVersion = connectResponse.protocolVersion();
+            var connectParams = new ConnectParams(effectiveConnectionToken);
+            var connectResponse = connection.rpc
+                    .invoke("connect", connectParams, com.github.copilot.sdk.generated.rpc.ConnectResult.class)
+                    .get(30, TimeUnit.SECONDS);
+            serverVersion = connectResponse.protocolVersion() != null
+                    ? connectResponse.protocolVersion().intValue()
+                    : null;
         } catch (Exception e) {
             // Unwrap CompletionException/ExecutionException to check inner cause
             Throwable cause = e;
@@ -267,12 +270,6 @@ private void verifyProtocolVersion(Connection connection) throws Exception {
         }
     }
 
-    /**
-     * Internal record for the 'connect' RPC response.
-     */
-    record ConnectResult(Integer protocolVersion) {
-    }
-
     /**
      * Disconnects from the Copilot server and closes all active sessions.
      * <p>

src/main/java/com/github/copilot/sdk/json/CopilotClientOptions.java

@@ -207,17 +207,20 @@ public String getCopilotHome() {
      * Sets the base directory for Copilot data (session state, config, etc.).
      * <p>
      * Sets the {@code COPILOT_HOME} environment variable on the spawned CLI
-     * process. When {@code null}, the CLI defaults to {@code ~/.copilot}.
+     * process. When {@code null}, the {@code COPILOT_HOME} env var is not set on
+     * the spawned process, so the CLI falls back to its default
+     * ({@code ~/.copilot}).
      * <p>
      * This option is only used when the SDK spawns the CLI process; it is ignored
      * when connecting to an external server via {@link #setCliUrl(String)}.
      *
      * @param copilotHome
-     *            the Copilot home directory path (must not be {@code null})
+     *            the Copilot home directory path, or {@code null} to use the CLI
+     *            default
      * @return this options instance for method chaining
      */
     public CopilotClientOptions setCopilotHome(String copilotHome) {
-        this.copilotHome = Objects.requireNonNull(copilotHome, "copilotHome must not be null");
+        this.copilotHome = copilotHome;
         return this;
     }
 

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

@@ -2276,6 +2276,9 @@ void testParsePermissionCompletedEvent() throws Exception {
         assertEquals("permission.completed", event.getType());
         assertEquals("perm-req-456", event.getData().requestId());
         assertNotNull(event.getData().result());
+        @SuppressWarnings("unchecked")
+        var result = (java.util.Map<String, Object>) event.getData().result();
+        assertEquals("approved", result.get("kind"));
     }
 
     @Test

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

@@ -441,4 +441,28 @@ void testBuildResumeRequestWithGitHubToken() {
 
         assertEquals("ghp_per_session_token", request.getGitHubToken());
     }
+
+    // =========================================================================
+    // instructionDirectories propagation
+    // =========================================================================
+
+    @Test
+    void testBuildCreateRequestPropagatesInstructionDirectories() {
+        var dirs = List.of("/path/to/instructions", "/another/path");
+        var config = new SessionConfig().setInstructionDirectories(dirs);
+
+        CreateSessionRequest request = SessionRequestBuilder.buildCreateRequest(config);
+
+        assertEquals(dirs, request.getInstructionDirectories());
+    }
+
+    @Test
+    void testBuildResumeRequestPropagatesInstructionDirectories() {
+        var dirs = List.of("/resume/instructions", "/other/dir");
+        var config = new ResumeSessionConfig().setInstructionDirectories(dirs);
+
+        ResumeSessionRequest request = SessionRequestBuilder.buildResumeRequest("sid-inst", config);
+
+        assertEquals(dirs, request.getInstructionDirectories());
+    }
 }