Skip to content

refactor(mcp): drop cross-process mcp auth file locking #1458

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
TheGreatAxios merged 1 commit into from
Oct 16, 2026
Merged

refactor(mcp): drop cross-process mcp auth file locking #1458

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
76 changes: 0 additions & 76 deletions src/mcp/auth-store.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,82 +73,6 @@ describe("mcp auth-store", () => {
).toBe(true);
});

test("overlapping updateAuthState from two processes keeps tokens and client registration", 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,
);

const storePath = join(import.meta.dirname, "auth-store.ts");
const barrier = join(home, "start");
const script = `
import { updateAuthState } from ${JSON.stringify(storePath)};
const home = process.argv[1];
const field = process.argv[2];
const barrier = process.argv[3];
const identity = { serverName: "linear", serverURL: "https://mcp.linear.app/mcp" };
while (!(await Bun.file(barrier).exists())) await Bun.sleep(5);
await updateAuthState(
identity,
(state) => {
Bun.sleepSync(150);
if (field === "tokens") {
state.tokens = {
access_token: "tok",
token_type: "bearer",
expires_in: 3600,
refresh_token: "ref",
};
} else {
state.clientInformation = {
client_id: "c1",
redirect_uris: ["http://127.0.0.1:1/callback"],
client_id_issued_at: 1,
client_name: "other-session",
};
}
},
home,
);
`;
const processes = [
Bun.spawn(
[process.execPath, "-e", script, "--", home, "tokens", barrier],
{
stdout: "ignore",
stderr: "pipe",
},
),
Bun.spawn(
[process.execPath, "-e", script, "--", home, "client", barrier],
{
stdout: "ignore",
stderr: "pipe",
},
),
];
await Bun.sleep(50);
await writeFile(barrier, "go");
const exitCodes = await Promise.all(processes.map((child) => child.exited));
const errors = await Promise.all(
processes.map((child) => new Response(child.stderr).text()),
);
expect(exitCodes, errors.join("\n")).toEqual([0, 0]);

const final = await loadAuthState(linear, home);
expect(final.tokens?.access_token).toBe("tok");
expect(final.clientInformation?.client_name).toBe("other-session");
expect(final.clientInformation?.client_id).toBe("c1");
});

test("concurrent saveAuthState calls do not throw ENOENT on temp rename", async () => {
const home = await tempHome();
await Promise.all(
Expand Down
107 changes: 5 additions & 102 deletions src/mcp/auth-store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,8 @@
import { createHash } from "node:crypto";
import {
mkdir,
open,
readFile,
rename,
stat,
unlink,
writeFile,
} from "node:fs/promises";
import { readFileSync, writeFileSync } from "node:fs";
import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises";
import { writeFileSync } from "node:fs";
import { homedir } from "node:os";
import { dirname, join } from "node:path";
import { setTimeout as delay } from "node:timers/promises";
import { getLogger } from "@intx/log";
import type {
OAuthClientInformationFull,
Expand Down Expand Up @@ -116,15 +107,6 @@ function stateFromRaw(raw: string | undefined, path: string): MCPAuthState {
return parseAuthState(raw, path) ?? {};
}

function readAuthFileSync(path: string): string | undefined {
try {
return readFileSync(path, "utf8");
} catch (err) {
if (isEnoent(err)) return undefined;
throw err;
}
}

async function readAuthFile(path: string): Promise<string | undefined> {
try {
return await readFile(path, "utf8");
Expand All @@ -142,97 +124,18 @@ export async function loadAuthState(
return stateFromRaw(await readAuthFile(path), path);
}

// Cache refresh: missing/unreadable/corrupt files return undefined so the
// caller keeps its in-memory mirror. Empty-on-corrupt is loadAuthState's contract.
export function tryLoadAuthStateSync(
identity: MCPAuthIdentity,
home: string = homedir(),
): MCPAuthState | undefined {
const path = authFilePath(identity, home);
let raw: string | undefined;
try {
raw = readAuthFileSync(path);
} catch {
return undefined;
}
if (raw === undefined) return undefined;
return parseAuthState(raw, path);
}

function isEexist(err: unknown): boolean {
return (
typeof err === "object" &&
err !== null &&
"code" in err &&
err.code === "EEXIST"
);
}

// pid alone is not unique per call, so concurrent saves must not share a temp
// path or the second rename hits ENOENT.
let tmpWriteCounter = 0;

// Serialize read-modify-write per auth file so two providers for the same
// server cannot clobber each other's fields (lost-update).
// server cannot clobber each other's fields (lost-update). Same-process only:
// the chain runs op directly, with no cross-process file lock.
const updateChains = new Map<string, Promise<unknown>>();

const LOCK_STALE_MS = 5_000;
const LOCK_RETRY_MS = 25;

async function acquireAuthFileLock(lockPath: string) {
while (true) {
try {
return await open(lockPath, "wx", 0o600);
} catch (err) {
if (!isEexist(err)) throw err;
try {
const info = await stat(lockPath);
if (Date.now() - info.mtimeMs > LOCK_STALE_MS) {
try {
await unlink(lockPath);
} catch (unlinkErr) {
if (!isEnoent(unlinkErr)) throw unlinkErr;
}
continue;
}
} catch (statErr) {
if (isEnoent(statErr)) continue;
throw statErr;
}
await delay(LOCK_RETRY_MS);
}
}
}

async function withAuthFileLock<T>(
path: string,
op: () => Promise<T>,
): Promise<T> {
const lockPath = `${path}.lock`;
await mkdir(dirname(path), { recursive: true, mode: 0o700 });
const lock = await acquireAuthFileLock(lockPath);
try {
return await op();
} finally {
try {
await lock.close();
} catch {
// Close can fail if the handle was already torn down.
}
try {
await unlink(lockPath);
} catch {
// Missing lock is fine; a leftover file is recovered as stale.
}
}
}

function enqueueAuthFileOp<T>(path: string, op: () => Promise<T>): Promise<T> {
const previous = updateChains.get(path) ?? Promise.resolve();
const run = previous.then(
() => withAuthFileLock(path, op),
() => withAuthFileLock(path, op),
);
const run = previous.then(op, op);
updateChains.set(
path,
run.then(
Expand Down
Loading
Loading