Skip to content

Commit adc873c

Browse files
Fix MCP OAuth resume order in Node.js SDK
resumeSession issued the session-scoped session.eventLog.registerInterest RPC for mcp.oauth_required BEFORE session.resume. The runtime only registers the session in its routing table while handling session.resume, so a fresh process resuming a persisted session rejected the pre-resume registerInterest with 'Session not found for sessionId: <id>'. VS Code always passes onMcpAuthRequest, so every resume threw. Move the registerInterest block to after the session.resume response is processed, mirroring the already-correct createSession path. Add a unit test asserting resume is sent before registerInterest, and an e2e test that reproduces the runtime error end-to-end via a fresh-client resume. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
1 parent cc66206 commit adc873c

4 files changed

Lines changed: 67 additions & 14 deletions

File tree

nodejs/src/client.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1575,12 +1575,6 @@ export class CopilotClient {
15751575
}
15761576
this.sessions.set(sessionId, session);
15771577
this.setupSessionFs(session, config);
1578-
if (config.onMcpAuthRequest) {
1579-
await this.connection!.sendRequest("session.eventLog.registerInterest", {
1580-
sessionId,
1581-
eventType: "mcp.oauth_required",
1582-
});
1583-
}
15841578

15851579
const toolFilterOptions = this.resolveToolFilterOptions(config);
15861580

@@ -1671,6 +1665,12 @@ export class CopilotClient {
16711665
session["_workspacePath"] = workspacePath;
16721666
session.setCapabilities(capabilities);
16731667
session.setOpenCanvases(openCanvases ?? []);
1668+
if (config.onMcpAuthRequest) {
1669+
await this.connection!.sendRequest("session.eventLog.registerInterest", {
1670+
sessionId,
1671+
eventType: "mcp.oauth_required",
1672+
});
1673+
}
16741674

16751675
await this.updateSessionOptionsForMode(session, config);
16761676
} catch (e) {

nodejs/test/client.test.ts

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -220,7 +220,7 @@ describe("CopilotClient", () => {
220220
]);
221221
});
222222

223-
it("registers MCP OAuth interest before resuming only when an auth handler is configured", async () => {
223+
it("registers MCP OAuth interest after resuming only when an auth handler is configured", async () => {
224224
const client = new CopilotClient();
225225
await client.start();
226226
onTestFinished(() => client.forceStop());
@@ -240,12 +240,23 @@ describe("CopilotClient", () => {
240240
onMcpAuthRequest: () => ({ kind: "cancelled" }),
241241
});
242242

243-
expect(spy.mock.calls[0]).toEqual([
244-
"session.eventLog.registerInterest",
245-
{ sessionId: "session-with-auth", eventType: "mcp.oauth_required" },
246-
]);
247-
expect(spy.mock.calls[1][0]).toBe("session.resume");
248-
expect(spy.mock.calls[1][1]).toEqual(expect.objectContaining({ requestPermission: true }));
243+
// `session.eventLog.registerInterest` is session-scoped: the runtime only
244+
// registers the session id while handling `session.resume`, so resume must
245+
// be sent BEFORE registering interest.
246+
const resumeIndex = spy.mock.calls.findIndex(([method]) => method === "session.resume");
247+
const interestIndex = spy.mock.calls.findIndex(
248+
([method]) => method === "session.eventLog.registerInterest"
249+
);
250+
expect(resumeIndex).toBeGreaterThanOrEqual(0);
251+
expect(interestIndex).toBeGreaterThanOrEqual(0);
252+
expect(resumeIndex).toBeLessThan(interestIndex);
253+
expect(spy.mock.calls[resumeIndex][1]).toEqual(
254+
expect.objectContaining({ sessionId: "session-with-auth", requestPermission: true })
255+
);
256+
expect(spy.mock.calls[interestIndex][1]).toEqual({
257+
sessionId: "session-with-auth",
258+
eventType: "mcp.oauth_required",
259+
});
249260

250261
spy.mockClear();
251262
await client.resumeSession("session-without-auth", {

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

Lines changed: 33 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { rm } from "fs/promises";
22
import { describe, expect, it, onTestFinished, vi } from "vitest";
33
import { ParsedHttpExchange } from "../../../test/harness/replayingCapiProxy.js";
44
import { CopilotClient, approveAll, defineTool, RuntimeConnection } from "../../src/index.js";
5-
import { createSdkTestContext, isCI } from "./harness/sdkTestContext.js";
5+
import { createSdkTestContext, DEFAULT_GITHUB_TOKEN, isCI } from "./harness/sdkTestContext.js";
66
import { getFinalAssistantMessage, getNextEventOfType, retry } from "./harness/sdkTestHelper.js";
77

88
describe("Sessions", async () => {
@@ -464,6 +464,38 @@ describe("Sessions", async () => {
464464
expect(session2.sessionId).toBe(sessionId);
465465
});
466466

467+
it("resumes a persisted session from a new client when an MCP OAuth handler is configured", async () => {
468+
// Take a turn so the session is persisted to the store and can be
469+
// loaded by a different CLI process.
470+
const session1 = await client.createSession({
471+
onPermissionRequest: approveAll,
472+
onMcpAuthRequest: () => ({ kind: "cancelled" }),
473+
});
474+
const sessionId = session1.sessionId;
475+
const answer = await session1.sendAndWait({ prompt: "What is 1+1?" });
476+
expect(answer?.data.content).toContain("2");
477+
478+
// Resume from a fresh client (new CLI process). Its routing table does
479+
// not know the session until it handles `session.resume`. Because an MCP
480+
// OAuth handler is configured, the SDK issues a session-scoped
481+
// `session.eventLog.registerInterest` for `mcp.oauth_required`; that must
482+
// be sent AFTER `session.resume`, otherwise the runtime rejects it with
483+
// "Session not found: <id>".
484+
const newClient = new CopilotClient({
485+
env,
486+
gitHubToken: DEFAULT_GITHUB_TOKEN,
487+
});
488+
onTestFinished(() => newClient.forceStop());
489+
490+
const session2 = await newClient.resumeSession(sessionId, {
491+
onPermissionRequest: approveAll,
492+
onMcpAuthRequest: () => ({ kind: "cancelled" }),
493+
});
494+
495+
expect(session2.sessionId).toBe(sessionId);
496+
await session2.disconnect();
497+
});
498+
467499
it("should abort a session", async () => {
468500
const session = await client.createSession({ onPermissionRequest: approveAll });
469501

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
models:
2+
- claude-sonnet-4.5
3+
conversations:
4+
- messages:
5+
- role: system
6+
content: ${system}
7+
- role: user
8+
content: What is 1+1?
9+
- role: assistant
10+
content: 1 + 1 = 2

0 commit comments

Comments
 (0)