Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions __tests__/index-lifecycle.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const mocks = vi.hoisted(() => ({
updateStatusBar: vi.fn(),
flushMetadataCache: vi.fn(),
initializeOAuth: vi.fn().mockResolvedValue(undefined),
shutdownOAuth: vi.fn().mockResolvedValue(undefined),
loadMcpConfig: vi.fn(() => ({ mcpServers: {} })),
loadMetadataCache: vi.fn(() => null),
buildProxyDescription: vi.fn(() => "MCP gateway"),
Expand Down Expand Up @@ -34,6 +35,7 @@ vi.mock("../init.js", () => ({

vi.mock("../mcp-auth-flow.js", () => ({
initializeOAuth: mocks.initializeOAuth,
shutdownOAuth: mocks.shutdownOAuth,
}));

vi.mock("../config.js", () => ({
Expand Down Expand Up @@ -122,6 +124,7 @@ describe("mcpAdapter session lifecycle", () => {
}

mocks.initializeOAuth.mockResolvedValue(undefined);
mocks.shutdownOAuth.mockResolvedValue(undefined);
mocks.loadMcpConfig.mockReturnValue({ mcpServers: {} });
mocks.loadMetadataCache.mockReturnValue(null);
mocks.buildProxyDescription.mockReturnValue("MCP gateway");
Expand Down Expand Up @@ -169,6 +172,37 @@ describe("mcpAdapter session lifecycle", () => {
expect(staleState.lifecycle.gracefulShutdown).toHaveBeenCalledTimes(1);
});

/**
* Regression: OAuth changes introduced initializeOAuth() on session_start but
* session_shutdown did not call shutdownOAuth(). The callback server was left running,
* keeping the event loop alive and preventing sub-agent processes from exiting.
*
* Fix: session_shutdown must call shutdownOAuth() so the callback server is closed.
*/
it("calls shutdownOAuth on session_shutdown so the callback server closes and sub-agents can exit", async () => {
const state = createState();
mocks.initializeMcp.mockResolvedValue(state);

const { default: mcpAdapter } = await import("../index.ts");
const { api, handlers } = createPi();
mcpAdapter(api);

// Establish a live session
const sessionStart = handlers.get("session_start");
await sessionStart?.({}, {});
await Promise.resolve();
await Promise.resolve();

mocks.shutdownOAuth.mockClear();

// Trigger shutdown (e.g. sub-agent process completing its task)
const sessionShutdown = handlers.get("session_shutdown");
expect(sessionShutdown).toBeTypeOf("function");
await sessionShutdown?.();

expect(mocks.shutdownOAuth).toHaveBeenCalledTimes(1);
});

it("logs initialization errors when updateStatusBar throws", async () => {
const state = createState();
mocks.initializeMcp.mockResolvedValue(state);
Expand Down
91 changes: 91 additions & 0 deletions __tests__/mcp-callback-server-unref.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* Regression: OAuth callback server must call server.unref() after binding so that
* sub-agent processes (which have no other active work) can exit naturally when done.
*
* Without unref(), the HTTP server keeps the Node.js event loop alive and the sub-agent
* process hangs indefinitely after completing its task.
*/

import { beforeEach, describe, expect, it, vi } from "vitest";

const mocks = vi.hoisted(() => {
const mockServer = {
listen: vi.fn(),
once: vi.fn(),
close: vi.fn(),
unref: vi.fn(),
};
return {
mockServer,
createServer: vi.fn(() => mockServer),
getConfiguredOAuthCallbackPort: vi.fn(() => 4337),
getOAuthCallbackPort: vi.fn(() => 4337),
setOAuthCallbackPort: vi.fn(),
};
});

vi.mock("http", () => ({
createServer: mocks.createServer,
}));

vi.mock("../mcp-oauth-provider.js", () => ({
OAUTH_CALLBACK_PATH: "/mcp/oauth/callback",
getConfiguredOAuthCallbackPort: mocks.getConfiguredOAuthCallbackPort,
getOAuthCallbackPort: mocks.getOAuthCallbackPort,
setOAuthCallbackPort: mocks.setOAuthCallbackPort,
}));

describe("ensureCallbackServer: sub-agent process exit", () => {
beforeEach(() => {
vi.resetModules();

mocks.mockServer.listen.mockReset();
mocks.mockServer.once.mockReset().mockReturnValue(mocks.mockServer);
mocks.mockServer.unref.mockReset();
mocks.mockServer.close.mockReset().mockImplementation((cb?: () => void) => cb?.());
mocks.createServer.mockReset().mockReturnValue(mocks.mockServer);
mocks.getConfiguredOAuthCallbackPort.mockReset().mockReturnValue(4337);
mocks.getOAuthCallbackPort.mockReset().mockReturnValue(4337);
mocks.setOAuthCallbackPort.mockReset();

// Simulate a successful port bind: resolve the listen promise immediately
mocks.mockServer.listen.mockImplementation(
(_port: number, _host: string, cb: () => void) => {
cb();
}
);
});

it("calls server.unref() after binding so the OAuth server does not prevent sub-agent exit", async () => {
const { ensureCallbackServer } = await import("../mcp-callback-server.ts");

await ensureCallbackServer();

expect(mocks.mockServer.unref).toHaveBeenCalledTimes(1);
});

it("does not call unref() on a failed bind (port in use)", async () => {
// Simulate EADDRINUSE on every port in the scan range
mocks.mockServer.listen.mockImplementation(
(_port: number, _host: string, _cb: () => void) => {
// Do nothing — the "error" handler will fire instead
}
);
mocks.mockServer.once.mockImplementation(
(event: string, handler: (err: NodeJS.ErrnoException) => void) => {
if (event === "error") {
const err = Object.assign(new Error("EADDRINUSE"), { code: "EADDRINUSE" });
// Defer to simulate async error
Promise.resolve().then(() => handler(err));
}
return mocks.mockServer;
}
);

const { ensureCallbackServer } = await import("../mcp-callback-server.ts");

await expect(ensureCallbackServer({ strictPort: true })).rejects.toThrow();

expect(mocks.mockServer.unref).not.toHaveBeenCalled();
});
});
7 changes: 5 additions & 2 deletions index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { flushMetadataCache, initializeMcp, updateStatusBar } from "./init.js";
import { loadMetadataCache } from "./metadata-cache.js";
import { executeCall, executeConnect, executeDescribe, executeList, executeSearch, executeStatus, executeUiMessages } from "./proxy-modes.js";
import { getConfigPathFromArgv, truncateAtWord } from "./utils.js";
import { initializeOAuth } from "./mcp-auth-flow.js";
import { initializeOAuth, shutdownOAuth } from "./mcp-auth-flow.js";

export default function mcpAdapter(pi: ExtensionAPI) {
let state: McpExtensionState | null = null;
Expand Down Expand Up @@ -133,7 +133,10 @@ export default function mcpAdapter(pi: ExtensionAPI) {
initPromise = null;

try {
await shutdownState(currentState, "session_shutdown");
await Promise.all([
shutdownState(currentState, "session_shutdown"),
shutdownOAuth(),
]);
} catch (error) {
console.error("MCP: session shutdown cleanup failed", error);
}
Expand Down
1 change: 1 addition & 0 deletions mcp-callback-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ export async function ensureCallbackServer(options: EnsureCallbackServerOptions
})

server = candidateServer
server.unref()
setOAuthCallbackPort(candidatePort)
return
} catch (error) {
Expand Down