Skip to content

Commit 1890825

Browse files
Address PR review feedback
- Remove unused session2 variable in multi-client test - Document fire-and-forget semantics in _handleBroadcastEvent - Fix null/undefined handling in tool result serialization - Fix stale RPC method names in comment Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent a7e862d commit 1890825

3 files changed

Lines changed: 13 additions & 6 deletions

File tree

nodejs/src/client.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1307,7 +1307,8 @@ export class CopilotClient {
13071307
// External tool calls and permission requests are now handled via broadcast events:
13081308
// the server sends external_tool.requested / permission.requested as session event
13091309
// notifications, and CopilotSession._dispatchEvent handles them internally by
1310-
// executing the handler and responding via session.tools.respond / session.permissions.respond RPC.
1310+
// executing the handler and responding via session.tools.handlePendingToolCall /
1311+
// session.permissions.handlePendingPermissionRequest RPC.
13111312

13121313
this.connection.onRequest(
13131314
"userInput.request",

nodejs/src/session.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -316,6 +316,8 @@ export class CopilotSession {
316316

317317
/**
318318
* Handles broadcast request events by executing local handlers and responding via RPC.
319+
* Handlers are dispatched as fire-and-forget — rejections propagate as unhandled promise
320+
* rejections, consistent with standard EventEmitter / event handler semantics.
319321
* @internal
320322
*/
321323
private _handleBroadcastEvent(event: SessionEvent): void {
@@ -362,10 +364,14 @@ export class CopilotSession {
362364
toolName,
363365
arguments: args,
364366
});
365-
const result =
366-
typeof rawResult === "string"
367-
? rawResult
368-
: JSON.stringify(rawResult ?? "");
367+
let result: string;
368+
if (rawResult == null) {
369+
result = "";
370+
} else if (typeof rawResult === "string") {
371+
result = rawResult;
372+
} else {
373+
result = JSON.stringify(rawResult);
374+
}
369375
await this.rpc.tools.handlePendingToolCall({ requestId, result });
370376
} catch (error) {
371377
const message =

nodejs/test/e2e/multi-client.test.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -255,7 +255,7 @@ describe("Multi-client broadcast", async () => {
255255
});
256256

257257
// Client 2 resumes with ephemeral_tool
258-
const session2 = await client2.resumeSession(session1.sessionId, {
258+
await client2.resumeSession(session1.sessionId, {
259259
onPermissionRequest: approveAll,
260260
tools: [toolB],
261261
});

0 commit comments

Comments
 (0)