Skip to content

Commit ce8aa9e

Browse files
committed
Fix merge blockers after rebase
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent 30147cc commit ce8aa9e

16 files changed

Lines changed: 131 additions & 121 deletions

go/internal/e2e/permissions_e2e_test.go

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -228,7 +228,7 @@ func TestPermissionsE2E(t *testing.T) {
228228
}
229229
})
230230

231-
t.Run("should accept explicit deny handler when joining an active session", func(t *testing.T) {
231+
t.Run("should accept user-not-available handler when joining an active session", func(t *testing.T) {
232232
ctx.ConfigureForTest(t)
233233

234234
session1, err := client.CreateSession(t.Context(), &copilot.SessionConfig{

go/session.go

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,8 @@ type Session struct {
8282
// eventCh serializes user event handler dispatch. dispatchEvent enqueues;
8383
// a single goroutine (processEvents) dequeues and invokes handlers in FIFO order.
8484
eventCh chan SessionEvent
85-
closeOnce sync.Once // guards eventCh close so Disconnect is safe to call more than once
85+
eventMu sync.RWMutex // coordinates sends with closing eventCh
86+
closeOnce sync.Once // guards eventCh close so Disconnect is safe to call more than once
8687
stateMu sync.Mutex
8788
closed bool
8889
closing chan struct{}
@@ -948,14 +949,17 @@ func fromRPCContent(value rpc.UIElicitationFieldValue) any {
948949
func (s *Session) dispatchEvent(event SessionEvent) {
949950
go s.handleBroadcastEvent(event)
950951

951-
// Send to the event channel in a closure with a recover guard.
952-
// Disconnect closes eventCh, and in Go sending on a closed channel
953-
// panics — there is no non-panicking send primitive. We only want
954-
// to suppress that specific panic; other panics are not expected here.
955-
func() {
956-
defer func() { recover() }()
957-
s.eventCh <- event
958-
}()
952+
s.eventMu.RLock()
953+
defer s.eventMu.RUnlock()
954+
955+
s.stateMu.Lock()
956+
closed := s.closed
957+
s.stateMu.Unlock()
958+
if closed {
959+
return
960+
}
961+
962+
s.eventCh <- event
959963
}
960964

961965
// processEvents is the single consumer goroutine for the event channel.
@@ -1273,7 +1277,9 @@ func (s *Session) markDisconnected() {
12731277
s.closed = true
12741278
s.stateMu.Unlock()
12751279

1280+
s.eventMu.Lock()
12761281
s.closeOnce.Do(func() { close(s.eventCh) })
1282+
s.eventMu.Unlock()
12771283

12781284
s.handlerMutex.Lock()
12791285
s.handlers = nil

nodejs/src/generated/rpc.ts

Lines changed: 20 additions & 53 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

nodejs/src/session.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -502,8 +502,14 @@ export class CopilotSession {
502502
} else {
503503
result = JSON.stringify(rawResult);
504504
}
505+
if (this.disconnected) {
506+
return;
507+
}
505508
await this.rpc.tools.handlePendingToolCall({ requestId, result });
506509
} catch (error) {
510+
if (this.disconnected) {
511+
return;
512+
}
507513
const message = error instanceof Error ? error.message : String(error);
508514
try {
509515
await this.rpc.tools.handlePendingToolCall({ requestId, error: message });
@@ -531,8 +537,14 @@ export class CopilotSession {
531537
if (result.kind === "no-result") {
532538
return;
533539
}
540+
if (this.disconnected) {
541+
return;
542+
}
534543
await this.rpc.permissions.handlePendingPermissionRequest({ requestId, result });
535544
} catch (_error) {
545+
if (this.disconnected) {
546+
return;
547+
}
536548
try {
537549
await this.rpc.permissions.handlePendingPermissionRequest({
538550
requestId,
@@ -576,8 +588,14 @@ export class CopilotSession {
576588

577589
try {
578590
await handler({ sessionId: this.sessionId, command, commandName, args });
591+
if (this.disconnected) {
592+
return;
593+
}
579594
await this.rpc.commands.handlePendingCommand({ requestId });
580595
} catch (error) {
596+
if (this.disconnected) {
597+
return;
598+
}
581599
const message = error instanceof Error ? error.message : String(error);
582600
try {
583601
await this.rpc.commands.handlePendingCommand({ requestId, error: message });

nodejs/test/client.test.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
/* eslint-disable @typescript-eslint/no-explicit-any */
22
import { describe, expect, it, onTestFinished, vi } from "vitest";
33
import { approveAll, CopilotClient, type ModelInfo } from "../src/index.js";
4+
import { createServerRpc } from "../src/generated/rpc.js";
45
import { CopilotSession } from "../src/session.js";
56
import { defaultJoinSessionPermissionHandler } from "../src/types.js";
67

@@ -174,6 +175,21 @@ describe("CopilotClient", () => {
174175
expect(connection.sendRequest).not.toHaveBeenCalled();
175176
});
176177

178+
it("allows generated RPC params with only optional fields to be omitted", async () => {
179+
const connection = {
180+
sendRequest: vi.fn(async (method: string) =>
181+
method === "models.list" ? { models: [] } : { tools: [] }
182+
),
183+
} as any;
184+
const rpc = createServerRpc(connection);
185+
186+
await rpc.models.list();
187+
await rpc.tools.list();
188+
189+
expect(connection.sendRequest).toHaveBeenCalledWith("models.list", {});
190+
expect(connection.sendRequest).toHaveBeenCalledWith("tools.list", {});
191+
});
192+
177193
it("forwards clientName in session.resume request", async () => {
178194
const client = new CopilotClient();
179195
await client.start();

nodejs/test/e2e/commands.e2e.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -83,6 +83,7 @@ describe("Commands", async () => {
8383
it("session with commands resumes successfully", async () => {
8484
const session1 = await client1.createSession({ onPermissionRequest: approveAll });
8585
const sessionId = session1.sessionId;
86+
await session1.disconnect();
8687

8788
const session2 = await client1.resumeSession(sessionId, {
8889
onPermissionRequest: approveAll,

nodejs/test/e2e/mcp_and_agents.e2e.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ describe("MCP Servers and Custom Agents", async () => {
4949
const session1 = await client.createSession({ onPermissionRequest: approveAll });
5050
const sessionId = session1.sessionId;
5151
await session1.sendAndWait({ prompt: "What is 1+1?" });
52+
await session1.disconnect();
5253

5354
// Resume with MCP servers
5455
const mcpServers: Record<string, MCPServerConfig> = {
@@ -160,6 +161,7 @@ describe("MCP Servers and Custom Agents", async () => {
160161
const session1 = await client.createSession({ onPermissionRequest: approveAll });
161162
const sessionId = session1.sessionId;
162163
await session1.sendAndWait({ prompt: "What is 1+1?" });
164+
await session1.disconnect();
163165

164166
// Resume with custom agents
165167
const customAgents: CustomAgentConfig[] = [
@@ -338,6 +340,7 @@ describe("MCP Servers and Custom Agents", async () => {
338340
const session1 = await client.createSession({ onPermissionRequest: approveAll });
339341
const sessionId = session1.sessionId;
340342
await session1.sendAndWait({ prompt: "What is 3+3?" });
343+
await session1.disconnect();
341344

342345
const secretTool = defineTool("secret_tool", {
343346
description: "A secret tool hidden from the default agent",

nodejs/test/e2e/permissions.e2e.test.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,7 @@ describe("Permission callbacks", async () => {
118118
const session1 = await client.createSession({ onPermissionRequest: approveAll });
119119
const sessionId = session1.sessionId;
120120
await session1.sendAndWait({ prompt: "What is 1+1?" });
121+
await session1.disconnect();
121122

122123
const session2 = await client.resumeSession(sessionId, {
123124
onPermissionRequest: () => ({
@@ -182,6 +183,7 @@ describe("Permission callbacks", async () => {
182183
const session1 = await client.createSession({ onPermissionRequest: approveAll });
183184
const sessionId = session1.sessionId;
184185
await session1.sendAndWait({ prompt: "What is 1+1?" });
186+
await session1.disconnect();
185187

186188
// Resume with permission handler
187189
const session2 = await client.resumeSession(sessionId, {

nodejs/test/e2e/session.e2e.test.ts

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ describe("Sessions", async () => {
3131
]);
3232

3333
await session.disconnect();
34-
await expect(() => session.getMessages()).rejects.toThrow(/Session not found/);
34+
await expect(() => session.getMessages()).rejects.toThrow(/Session has been disconnected/);
3535
});
3636

3737
// TODO: Re-enable once test harness CAPI proxy supports this test's session lifecycle
@@ -253,7 +253,7 @@ describe("Sessions", async () => {
253253
// All can be disconnected
254254
await Promise.all([s1.disconnect(), s2.disconnect(), s3.disconnect()]);
255255
for (const s of [s1, s2, s3]) {
256-
await expect(() => s.getMessages()).rejects.toThrow(/Session not found/);
256+
await expect(() => s.getMessages()).rejects.toThrow(/Session has been disconnected/);
257257
}
258258
});
259259

@@ -263,6 +263,7 @@ describe("Sessions", async () => {
263263
const sessionId = session1.sessionId;
264264
const answer = await session1.sendAndWait({ prompt: "What is 1+1?" });
265265
expect(answer?.data.content).toContain("2");
266+
await session1.disconnect();
266267

267268
// Resume using the same client
268269
const session2 = await client.resumeSession(sessionId, { onPermissionRequest: approveAll });
@@ -353,6 +354,7 @@ describe("Sessions", async () => {
353354
it("should resume session with a custom provider", async () => {
354355
const session = await client.createSession({ onPermissionRequest: approveAll });
355356
const sessionId = session.sessionId;
357+
await session.disconnect();
356358

357359
// Resume the session with a provider
358360
const session2 = await client.resumeSession(sessionId, {

0 commit comments

Comments
 (0)