[Node] Set requestPermission False If Using Default Permission Handler (#1056)

* [Node] Set requestPermission False If Using Default Permission Handler

* Add joinSession permission tests

* Format joinSession changes

Author: MRayermannMSFT

nodejs/src/client.ts

@@ -55,6 +55,7 @@ import type {
     TraceContextProvider,
     TypedSessionLifecycleHandler,
 } from "./types.js";
+import { defaultJoinSessionPermissionHandler } from "./types.js";
 
 /**
  * Minimum protocol version this SDK can communicate with.
@@ -868,7 +869,8 @@ export class CopilotClient {
                 })),
                 provider: config.provider,
                 modelCapabilities: config.modelCapabilities,
-                requestPermission: true,
+                requestPermission:
+                    config.onPermissionRequest !== defaultJoinSessionPermissionHandler,
                 requestUserInput: !!config.onUserInputRequest,
                 requestElicitation: !!config.onElicitationRequest,
                 hooks: !!(config.hooks && Object.values(config.hooks).some(Boolean)),

nodejs/src/extension.ts

@@ -4,11 +4,11 @@
 
 import { CopilotClient } from "./client.js";
 import type { CopilotSession } from "./session.js";
-import type { PermissionHandler, PermissionRequestResult, ResumeSessionConfig } from "./types.js";
-
-const defaultJoinSessionPermissionHandler: PermissionHandler = (): PermissionRequestResult => ({
-    kind: "no-result",
-});
+import {
+    defaultJoinSessionPermissionHandler,
+    type PermissionHandler,
+    type ResumeSessionConfig,
+} from "./types.js";
 
 export type JoinSessionConfig = Omit<ResumeSessionConfig, "onPermissionRequest"> & {
     onPermissionRequest?: PermissionHandler;

nodejs/src/types.ts

@@ -757,6 +757,11 @@ export type PermissionHandler = (
 
 export const approveAll: PermissionHandler = () => ({ kind: "approved" });
 
+export const defaultJoinSessionPermissionHandler: PermissionHandler =
+    (): PermissionRequestResult => ({
+        kind: "no-result",
+    });
+
 // ============================================================================
 // User Input Request Types
 // ============================================================================

nodejs/test/client.test.ts

@@ -1,6 +1,7 @@
 /* eslint-disable @typescript-eslint/no-explicit-any */
 import { describe, expect, it, onTestFinished, vi } from "vitest";
 import { approveAll, CopilotClient, type ModelInfo } from "../src/index.js";
+import { defaultJoinSessionPermissionHandler } from "../src/types.js";
 
 // This file is for unit tests. Where relevant, prefer to add e2e tests in e2e/*.test.ts instead
 
@@ -97,6 +98,60 @@ describe("CopilotClient", () => {
         spy.mockRestore();
     });
 
+    it("does not request permissions on session.resume when using the default joinSession handler", async () => {
+        const client = new CopilotClient();
+        await client.start();
+        onTestFinished(() => client.forceStop());
+
+        const session = await client.createSession({ onPermissionRequest: approveAll });
+        const spy = vi
+            .spyOn((client as any).connection!, "sendRequest")
+            .mockImplementation(async (method: string, params: any) => {
+                if (method === "session.resume") return { sessionId: params.sessionId };
+                throw new Error(`Unexpected method: ${method}`);
+            });
+
+        await client.resumeSession(session.sessionId, {
+            onPermissionRequest: defaultJoinSessionPermissionHandler,
+        });
+
+        expect(spy).toHaveBeenCalledWith(
+            "session.resume",
+            expect.objectContaining({
+                sessionId: session.sessionId,
+                requestPermission: false,
+            })
+        );
+        spy.mockRestore();
+    });
+
+    it("requests permissions on session.resume when using an explicit handler", async () => {
+        const client = new CopilotClient();
+        await client.start();
+        onTestFinished(() => client.forceStop());
+
+        const session = await client.createSession({ onPermissionRequest: approveAll });
+        const spy = vi
+            .spyOn((client as any).connection!, "sendRequest")
+            .mockImplementation(async (method: string, params: any) => {
+                if (method === "session.resume") return { sessionId: params.sessionId };
+                throw new Error(`Unexpected method: ${method}`);
+            });
+
+        await client.resumeSession(session.sessionId, {
+            onPermissionRequest: approveAll,
+        });
+
+        expect(spy).toHaveBeenCalledWith(
+            "session.resume",
+            expect.objectContaining({
+                sessionId: session.sessionId,
+                requestPermission: true,
+            })
+        );
+        spy.mockRestore();
+    });
+
     it("sends session.model.switchTo RPC with correct params", async () => {
         const client = new CopilotClient();
         await client.start();

nodejs/test/extension.test.ts

@@ -2,6 +2,7 @@ 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";
+import { defaultJoinSessionPermissionHandler } from "../src/types.js";
 
 describe("joinSession", () => {
     const originalSessionId = process.env.SESSION_ID;
@@ -25,6 +26,7 @@ describe("joinSession", () => {
 
         const [, config] = resumeSession.mock.calls[0]!;
         expect(config.onPermissionRequest).toBeDefined();
+        expect(config.onPermissionRequest).toBe(defaultJoinSessionPermissionHandler);
         const result = await Promise.resolve(
             config.onPermissionRequest!({ kind: "write" }, { sessionId: "session-123" })
         );