Skip to content

Commit d2db596

Browse files
committed
refactor(mcp): drop cross-process mcp auth file locking
1 parent e0e31cd commit d2db596

4 files changed

Lines changed: 134 additions & 304 deletions

File tree

‎src/mcp/auth-store.test.ts‎

Lines changed: 0 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -73,82 +73,6 @@ describe("mcp auth-store", () => {
7373
).toBe(true);
7474
});
7575

76-
test("overlapping updateAuthState from two processes keeps tokens and client registration", async () => {
77-
const home = await tempHome();
78-
await saveAuthState(
79-
linear,
80-
{
81-
clientInformation: {
82-
client_id: "c1",
83-
redirect_uris: ["http://127.0.0.1:1/callback"],
84-
client_id_issued_at: 1,
85-
},
86-
},
87-
home,
88-
);
89-
90-
const storePath = join(import.meta.dirname, "auth-store.ts");
91-
const barrier = join(home, "start");
92-
const script = `
93-
import { updateAuthState } from ${JSON.stringify(storePath)};
94-
const home = process.argv[1];
95-
const field = process.argv[2];
96-
const barrier = process.argv[3];
97-
const identity = { serverName: "linear", serverURL: "https://mcp.linear.app/mcp" };
98-
while (!(await Bun.file(barrier).exists())) await Bun.sleep(5);
99-
await updateAuthState(
100-
identity,
101-
(state) => {
102-
Bun.sleepSync(150);
103-
if (field === "tokens") {
104-
state.tokens = {
105-
access_token: "tok",
106-
token_type: "bearer",
107-
expires_in: 3600,
108-
refresh_token: "ref",
109-
};
110-
} else {
111-
state.clientInformation = {
112-
client_id: "c1",
113-
redirect_uris: ["http://127.0.0.1:1/callback"],
114-
client_id_issued_at: 1,
115-
client_name: "other-session",
116-
};
117-
}
118-
},
119-
home,
120-
);
121-
`;
122-
const processes = [
123-
Bun.spawn(
124-
[process.execPath, "-e", script, "--", home, "tokens", barrier],
125-
{
126-
stdout: "ignore",
127-
stderr: "pipe",
128-
},
129-
),
130-
Bun.spawn(
131-
[process.execPath, "-e", script, "--", home, "client", barrier],
132-
{
133-
stdout: "ignore",
134-
stderr: "pipe",
135-
},
136-
),
137-
];
138-
await Bun.sleep(50);
139-
await writeFile(barrier, "go");
140-
const exitCodes = await Promise.all(processes.map((child) => child.exited));
141-
const errors = await Promise.all(
142-
processes.map((child) => new Response(child.stderr).text()),
143-
);
144-
expect(exitCodes, errors.join("\n")).toEqual([0, 0]);
145-
146-
const final = await loadAuthState(linear, home);
147-
expect(final.tokens?.access_token).toBe("tok");
148-
expect(final.clientInformation?.client_name).toBe("other-session");
149-
expect(final.clientInformation?.client_id).toBe("c1");
150-
});
151-
15276
test("concurrent saveAuthState calls do not throw ENOENT on temp rename", async () => {
15377
const home = await tempHome();
15478
await Promise.all(

‎src/mcp/auth-store.ts‎

Lines changed: 5 additions & 102 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,8 @@
11
import { createHash } from "node:crypto";
2-
import {
3-
mkdir,
4-
open,
5-
readFile,
6-
rename,
7-
stat,
8-
unlink,
9-
writeFile,
10-
} from "node:fs/promises";
11-
import { readFileSync, writeFileSync } from "node:fs";
2+
import { mkdir, readFile, rename, unlink, writeFile } from "node:fs/promises";
3+
import { writeFileSync } from "node:fs";
124
import { homedir } from "node:os";
135
import { dirname, join } from "node:path";
14-
import { setTimeout as delay } from "node:timers/promises";
156
import { getLogger } from "@intx/log";
167
import type {
178
OAuthClientInformationFull,
@@ -116,15 +107,6 @@ function stateFromRaw(raw: string | undefined, path: string): MCPAuthState {
116107
return parseAuthState(raw, path) ?? {};
117108
}
118109

119-
function readAuthFileSync(path: string): string | undefined {
120-
try {
121-
return readFileSync(path, "utf8");
122-
} catch (err) {
123-
if (isEnoent(err)) return undefined;
124-
throw err;
125-
}
126-
}
127-
128110
async function readAuthFile(path: string): Promise<string | undefined> {
129111
try {
130112
return await readFile(path, "utf8");
@@ -142,97 +124,18 @@ export async function loadAuthState(
142124
return stateFromRaw(await readAuthFile(path), path);
143125
}
144126

145-
// Cache refresh: missing/unreadable/corrupt files return undefined so the
146-
// caller keeps its in-memory mirror. Empty-on-corrupt is loadAuthState's contract.
147-
export function tryLoadAuthStateSync(
148-
identity: MCPAuthIdentity,
149-
home: string = homedir(),
150-
): MCPAuthState | undefined {
151-
const path = authFilePath(identity, home);
152-
let raw: string | undefined;
153-
try {
154-
raw = readAuthFileSync(path);
155-
} catch {
156-
return undefined;
157-
}
158-
if (raw === undefined) return undefined;
159-
return parseAuthState(raw, path);
160-
}
161-
162-
function isEexist(err: unknown): boolean {
163-
return (
164-
typeof err === "object" &&
165-
err !== null &&
166-
"code" in err &&
167-
err.code === "EEXIST"
168-
);
169-
}
170-
171127
// pid alone is not unique per call, so concurrent saves must not share a temp
172128
// path or the second rename hits ENOENT.
173129
let tmpWriteCounter = 0;
174130

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

179-
const LOCK_STALE_MS = 5_000;
180-
const LOCK_RETRY_MS = 25;
181-
182-
async function acquireAuthFileLock(lockPath: string) {
183-
while (true) {
184-
try {
185-
return await open(lockPath, "wx", 0o600);
186-
} catch (err) {
187-
if (!isEexist(err)) throw err;
188-
try {
189-
const info = await stat(lockPath);
190-
if (Date.now() - info.mtimeMs > LOCK_STALE_MS) {
191-
try {
192-
await unlink(lockPath);
193-
} catch (unlinkErr) {
194-
if (!isEnoent(unlinkErr)) throw unlinkErr;
195-
}
196-
continue;
197-
}
198-
} catch (statErr) {
199-
if (isEnoent(statErr)) continue;
200-
throw statErr;
201-
}
202-
await delay(LOCK_RETRY_MS);
203-
}
204-
}
205-
}
206-
207-
async function withAuthFileLock<T>(
208-
path: string,
209-
op: () => Promise<T>,
210-
): Promise<T> {
211-
const lockPath = `${path}.lock`;
212-
await mkdir(dirname(path), { recursive: true, mode: 0o700 });
213-
const lock = await acquireAuthFileLock(lockPath);
214-
try {
215-
return await op();
216-
} finally {
217-
try {
218-
await lock.close();
219-
} catch {
220-
// Close can fail if the handle was already torn down.
221-
}
222-
try {
223-
await unlink(lockPath);
224-
} catch {
225-
// Missing lock is fine; a leftover file is recovered as stale.
226-
}
227-
}
228-
}
229-
230136
function enqueueAuthFileOp<T>(path: string, op: () => Promise<T>): Promise<T> {
231137
const previous = updateChains.get(path) ?? Promise.resolve();
232-
const run = previous.then(
233-
() => withAuthFileLock(path, op),
234-
() => withAuthFileLock(path, op),
235-
);
138+
const run = previous.then(op, op);
236139
updateChains.set(
237140
path,
238141
run.then(

0 commit comments

Comments
 (0)