From fe0fc69ad22bf9945374e54767c7d6b004e9d46f Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Sat, 22 Aug 2026 10:43:00 -0700 Subject: [PATCH 1/2] Expect grok-4.6 high cycle to land on xhigh --- src/provider/reasoning-effort.test.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/provider/reasoning-effort.test.ts b/src/provider/reasoning-effort.test.ts index 60b78e278..2e009baff 100644 --- a/src/provider/reasoning-effort.test.ts +++ b/src/provider/reasoning-effort.test.ts @@ -137,7 +137,7 @@ describe("cycleReasoningEffort", () => { expect(cycleReasoningEffort("grok-4.6", undefined)).toBe( cycleReasoningEffort("grok-4.6", "high"), ); - expect(cycleReasoningEffort("grok-4.6", "high")).toBe("low"); + expect(cycleReasoningEffort("grok-4.6", "high")).toBe("xhigh"); }); test("unset gpt-5.1 chat cycles from implicit none to minimal", () => { @@ -152,7 +152,7 @@ describe("cycleReasoningEffort", () => { test("grok leftover minimal cycles the same as unset / high", () => { expect(cycleReasoningEffort("grok-4.6", "minimal")).toBe(cycleReasoningEffort("grok-4.6", undefined)); expect(cycleReasoningEffort("grok-4.6", "minimal")).toBe(cycleReasoningEffort("grok-4.6", "high")); - expect(cycleReasoningEffort("grok-4.6", "minimal")).toBe("low"); + expect(cycleReasoningEffort("grok-4.6", "minimal")).toBe("xhigh"); }); test("unknown models with rungs still start at supported[0] when no default exists", () => { From bb180e5cdf9fed68bce1b5abba5485177a65b910 Mon Sep 17 00:00:00 2001 From: Sawyer Cutler Date: Thu, 13 Aug 2026 22:38:54 -0700 Subject: [PATCH 2/2] Keep concurrent MCP OAuth writes from wiping tokens Auth-store last-writer-wins full snapshots let a second session codeVerifier save erase tokens just written by the first. Serialize read-modify-write per server, use unique temp files, and drop stale DCR clients when the loopback redirect port no longer matches. --- src/mcp/auth-store.test.ts | 60 +++++++++++++++ src/mcp/auth-store.ts | 58 +++++++++++++-- src/mcp/oauth-provider.test.ts | 130 +++++++++++++++++++++++++++++++++ src/mcp/oauth-provider.ts | 68 ++++++++++++++--- 4 files changed, 299 insertions(+), 17 deletions(-) create mode 100644 src/mcp/auth-store.test.ts create mode 100644 src/mcp/oauth-provider.test.ts diff --git a/src/mcp/auth-store.test.ts b/src/mcp/auth-store.test.ts new file mode 100644 index 000000000..1df2223b5 --- /dev/null +++ b/src/mcp/auth-store.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp, readFile } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { loadAuthState, saveAuthState, updateAuthState } from "./auth-store.js"; + +async function tempHome(): Promise { + return mkdtemp(join(tmpdir(), "mcp-auth-")); +} + +describe("mcp auth-store", () => { + test("updateAuthState merges concurrent field writes without losing tokens", async () => { + const home = await tempHome(); + await saveAuthState("linear", { + clientInformation: { + client_id: "c1", + redirect_uris: ["http://127.0.0.1:1/callback"], + client_id_issued_at: 1, + }, + }, home); + + // Reproduce the bug: one writer saves tokens while another overwrites with a + // fresh codeVerifier from a concurrent OAuth start. With full-snapshot + // persists, the verifier write wiped tokens; with updateAuthState, both land. + const writes = await Promise.all([ + updateAuthState("linear", (state) => { + state.tokens = { + access_token: "tok", + token_type: "bearer", + expires_in: 3600, + refresh_token: "ref", + }; + }, home), + updateAuthState("linear", (state) => { + state.codeVerifier = "verifier-from-other-session"; + }, home), + ]); + + const final = await loadAuthState("linear", home); + expect(final.tokens?.access_token).toBe("tok"); + expect(final.codeVerifier).toBe("verifier-from-other-session"); + expect(final.clientInformation?.client_id).toBe("c1"); + expect(writes[0].tokens?.access_token === "tok" || writes[1].tokens?.access_token === "tok").toBe(true); + }); + + test("concurrent saveAuthState calls do not throw ENOENT on temp rename", async () => { + const home = await tempHome(); + await Promise.all( + Array.from({ length: 20 }, (_, i) => + saveAuthState("linear", { codeVerifier: `v${String(i)}` }, home), + ), + ); + const final = await loadAuthState("linear", home); + expect(final.codeVerifier?.startsWith("v")).toBe(true); + // No leftover temp files from failed renames. + const dir = join(home, ".corbits", "mcp-auth"); + const raw = await readFile(join(dir, "linear.json"), "utf8"); + expect(JSON.parse(raw).codeVerifier).toBe(final.codeVerifier); + }); +}); diff --git a/src/mcp/auth-store.ts b/src/mcp/auth-store.ts index 4d49c20d4..a34b9024a 100644 --- a/src/mcp/auth-store.ts +++ b/src/mcp/auth-store.ts @@ -1,6 +1,6 @@ import { mkdir, readFile, rename, writeFile } from "node:fs/promises"; import { homedir } from "node:os"; -import { join } from "node:path"; +import { dirname, join } from "node:path"; import type { OAuthClientInformationFull, OAuthTokens } from "@modelcontextprotocol/sdk/shared/auth.js"; import { SETTINGS_DIR_NAME } from "../branding.js"; @@ -46,11 +46,59 @@ export async function loadAuthState(serverName: string, home: string = homedir() return {}; } +// pid alone is not unique per call — concurrent saves in one process must not +// share a temp path or the second rename hits ENOENT after the first moves it. +let tmpWriteCounter = 0; + +// Serialize read-modify-write per auth file so two OAuth provider instances for +// the same server cannot clobber each other's fields (classic lost-update: one +// session's saveCodeVerifier wiping another's just-written tokens). +const updateChains = new Map>(); + +async function writeAuthFile(path: string, state: MCPAuthState): Promise { + await mkdir(dirname(path), { recursive: true, mode: 0o700 }); + const tmp = `${path}.${process.pid}.${(tmpWriteCounter += 1)}.tmp`; + await writeFile(tmp, JSON.stringify(state, null, 2), { mode: 0o600 }); + await rename(tmp, path); +} + // Tokens are credentials, so the directory and file are restricted to the owner. +// Full replace — prefer updateAuthState when mutating a single field so concurrent +// writers merge instead of last-writer-wins on a stale snapshot. export async function saveAuthState(serverName: string, state: MCPAuthState, home: string = homedir()): Promise { const path = authFilePath(serverName, home); - await mkdir(mcpAuthDir(home), { recursive: true, mode: 0o700 }); - const tmp = `${path}.${process.pid}.tmp`; - await writeFile(tmp, JSON.stringify(state, null, 2), { mode: 0o600 }); - await rename(tmp, path); + const previous = updateChains.get(path) ?? Promise.resolve(); + const write = previous.then( + () => writeAuthFile(path, state), + () => writeAuthFile(path, state), + ); + updateChains.set(path, write.then(() => undefined, () => undefined)); + await write; +} + +// Load → mutate → save under the per-file chain. Mutator receives a mutable +// snapshot of the latest on-disk state; the returned object is what was written. +export async function updateAuthState( + serverName: string, + mutator: (state: MCPAuthState) => void, + home: string = homedir(), +): Promise { + const path = authFilePath(serverName, home); + const previous = updateChains.get(path) ?? Promise.resolve(); + const run = previous.then( + async () => { + const state = await loadAuthState(serverName, home); + mutator(state); + await writeAuthFile(path, state); + return state; + }, + async () => { + const state = await loadAuthState(serverName, home); + mutator(state); + await writeAuthFile(path, state); + return state; + }, + ); + updateChains.set(path, run.then(() => undefined, () => undefined)); + return run; } diff --git a/src/mcp/oauth-provider.test.ts b/src/mcp/oauth-provider.test.ts new file mode 100644 index 000000000..35c9e0584 --- /dev/null +++ b/src/mcp/oauth-provider.test.ts @@ -0,0 +1,130 @@ +import { describe, expect, test } from "bun:test"; +import { mkdtemp } from "node:fs/promises"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import { loadAuthState, saveAuthState } from "./auth-store.js"; +import { createOAuthProvider } from "./oauth-provider.js"; + +async function tempHome(): Promise { + return mkdtemp(join(tmpdir(), "mcp-oauth-")); +} + +const clientInfo = (port: number) => ({ + client_id: `client-on-${String(port)}`, + redirect_uris: [`http://127.0.0.1:${String(port)}/callback`], + client_id_issued_at: 1, + token_endpoint_auth_method: "none" as const, + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + client_name: "interchange-code", +}); + +async function syncValue(value: T | Promise): Promise { + return await value; +} + +describe("createOAuthProvider", () => { + test("drops stale DCR client when redirect port changed and no tokens exist", async () => { + const home = await tempHome(); + await saveAuthState("linear", { + clientInformation: clientInfo(60435), + codeVerifier: "old-verifier", + }, home); + + const provider = await createOAuthProvider({ + serverName: "linear", + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + + expect(await syncValue(provider.clientInformation())).toBeUndefined(); + const disk = await loadAuthState("linear", home); + expect(disk.clientInformation).toBeUndefined(); + expect(disk.codeVerifier).toBeUndefined(); + }); + + test("keeps registered client and tokens when only the loopback port changed", async () => { + const home = await tempHome(); + await saveAuthState("linear", { + clientInformation: clientInfo(60435), + tokens: { + access_token: "live", + token_type: "bearer", + expires_in: 3600, + refresh_token: "refresh", + }, + }, home); + + const provider = await createOAuthProvider({ + serverName: "linear", + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + + expect((await syncValue(provider.clientInformation()))?.client_id).toBe("client-on-60435"); + expect((await syncValue(provider.tokens()))?.access_token).toBe("live"); + }); + + test("concurrent saveTokens and saveCodeVerifier from two providers keep both fields", async () => { + const home = await tempHome(); + await saveAuthState("linear", { clientInformation: clientInfo(1) }, home); + + const a = await createOAuthProvider({ + serverName: "linear", + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + const b = await createOAuthProvider({ + serverName: "linear", + redirectUrl: "http://127.0.0.1:1/callback", + onAuthURL: () => undefined, + home, + }); + + await Promise.all([ + a.saveTokens({ + access_token: "tok-a", + token_type: "bearer", + expires_in: 60, + refresh_token: "ref-a", + }), + b.saveCodeVerifier("verifier-b"), + ]); + + const disk = await loadAuthState("linear", home); + expect(disk.tokens?.access_token).toBe("tok-a"); + expect(disk.codeVerifier).toBe("verifier-b"); + }); + + test("resetAuthorization clears client when redirect no longer matches registration", async () => { + const home = await tempHome(); + await saveAuthState("linear", { + clientInformation: clientInfo(60435), + tokens: { + access_token: "live", + token_type: "bearer", + expires_in: 1, + refresh_token: "r", + }, + codeVerifier: "v", + }, home); + + const provider = await createOAuthProvider({ + serverName: "linear", + redirectUrl: "http://127.0.0.1:62000/callback", + onAuthURL: () => undefined, + home, + }); + + // Tokens present → client kept at create. Reset simulates failed refresh. + await provider.resetAuthorization(); + expect(await syncValue(provider.tokens())).toBeUndefined(); + expect(await syncValue(provider.clientInformation())).toBeUndefined(); + const disk = await loadAuthState("linear", home); + expect(disk.clientInformation).toBeUndefined(); + expect(disk.tokens).toBeUndefined(); + }); +}); diff --git a/src/mcp/oauth-provider.ts b/src/mcp/oauth-provider.ts index ba2f663ac..c748736e3 100644 --- a/src/mcp/oauth-provider.ts +++ b/src/mcp/oauth-provider.ts @@ -1,6 +1,6 @@ import type { OAuthClientProvider } from "@modelcontextprotocol/sdk/client/auth.js"; import type { OAuthClientInformationFull, OAuthClientInformationMixed, OAuthClientMetadata, OAuthTokens } from "@modelcontextprotocol/sdk/shared/auth.js"; -import { loadAuthState, saveAuthState, type MCPAuthState } from "./auth-store.js"; +import { updateAuthState, type MCPAuthState } from "./auth-store.js"; import { MCP_CLIENT_NAME } from "../branding.js"; export type OAuthProviderOptions = { @@ -12,9 +12,45 @@ export type OAuthProviderOptions = { }; export type CorbitsOAuthProvider = OAuthClientProvider & { resetAuthorization(): Promise }; +function redirectUrisInclude(info: OAuthClientInformationFull | undefined, redirectUrl: string): boolean { + const uris = info?.redirect_uris; + if (uris === undefined || uris.length === 0) return true; + return uris.includes(redirectUrl); +} + +// Dynamic client registration bakes in the loopback redirect_uri (ephemeral port). +// A later session that binds a new port cannot reuse that client_id for authorize +// / token exchange — drop the stale registration when we have no refreshable +// tokens and must run the browser flow again. +function dropStaleClientRegistration(state: MCPAuthState, redirectUrl: string): void { + if (state.tokens !== undefined) return; + if (redirectUrisInclude(state.clientInformation, redirectUrl)) return; + delete state.clientInformation; + delete state.codeVerifier; +} + +function replaceStored(stored: MCPAuthState, next: MCPAuthState): void { + delete stored.clientInformation; + delete stored.tokens; + delete stored.codeVerifier; + if (next.clientInformation !== undefined) stored.clientInformation = next.clientInformation; + if (next.tokens !== undefined) stored.tokens = next.tokens; + if (next.codeVerifier !== undefined) stored.codeVerifier = next.codeVerifier; +} + export async function createOAuthProvider(opts: OAuthProviderOptions): Promise { - const stored: MCPAuthState = await loadAuthState(opts.serverName, opts.home); - const persist = (): Promise => saveAuthState(opts.serverName, stored, opts.home); + // Load + scrub stale DCR under the per-file chain so concurrent providers see + // the same cleaned state. Mutations always re-read disk; this in-memory mirror + // only serves the SDK's sync getters (tokens / clientInformation / codeVerifier). + const stored: MCPAuthState = await updateAuthState(opts.serverName, (state) => { + dropStaleClientRegistration(state, opts.redirectUrl); + }, opts.home); + + const apply = async (mutator: (state: MCPAuthState) => void): Promise => { + const next = await updateAuthState(opts.serverName, mutator, opts.home); + replaceStored(stored, next); + }; + let oauthState: string | undefined; return { get redirectUrl(): string { return opts.redirectUrl; }, @@ -27,13 +63,15 @@ export async function createOAuthProvider(opts: OAuthProviderOptions): Promise { - stored.clientInformation = info as OAuthClientInformationFull; - return persist(); + return apply((state) => { + state.clientInformation = info as OAuthClientInformationFull; + }); }, tokens(): OAuthTokens | undefined { return stored.tokens; }, saveTokens(tokens: OAuthTokens): Promise { - stored.tokens = tokens; - return persist(); + return apply((state) => { + state.tokens = tokens; + }); }, redirectToAuthorization(authorizationUrl: URL): void { const state = authorizationUrl.searchParams.get("state"); @@ -41,18 +79,24 @@ export async function createOAuthProvider(opts: OAuthProviderOptions): Promise { - stored.codeVerifier = codeVerifier; - return persist(); + return apply((state) => { + state.codeVerifier = codeVerifier; + }); }, codeVerifier(): string { if (stored.codeVerifier === undefined) throw new Error("No PKCE code verifier saved for this authorization."); return stored.codeVerifier; }, resetAuthorization(): Promise { - delete stored.tokens; - delete stored.codeVerifier; oauthState = undefined; - return persist(); + return apply((state) => { + delete state.tokens; + delete state.codeVerifier; + // Next browser flow needs a client registered for *this* loopback port. + if (!redirectUrisInclude(state.clientInformation, opts.redirectUrl)) { + delete state.clientInformation; + } + }); }, }; }