Skip to content

Commit 2f185fe

Browse files
committed
Strengthen the auth-store queue regression and clarify comments
Replace the dual-save case that stayed green without the queue with a 50-way same-process burst that needs fresh lock deadlines, and document the queue plus defensive temp-counter in the store header.
1 parent bb3e210 commit 2f185fe

2 files changed

Lines changed: 39 additions & 37 deletions

File tree

‎src/auth/store.test.ts‎

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -114,49 +114,48 @@ describe("createAuthStore", () => {
114114
}
115115
});
116116

117-
test("queues same-process profile writes so neither save is lost", async () => {
118-
const home = await mkdtemp(join(tmpdir(), "oauth-store-same-process-"));
117+
test("keeps a same-process burst of profile saves without shared-deadline loss", async () => {
118+
const home = await mkdtemp(join(tmpdir(), "oauth-store-burst-"));
119119
try {
120120
const store = createAuthStore<TestTokens>({
121121
filename: "test-auth.json",
122122
settingsDirName: TEST_SETTINGS_DIR,
123123
isTokens: isTestTokens,
124124
});
125125

126-
await Promise.all([
127-
store.saveProfile(
128-
{
129-
name: "personal",
130-
tokens: { access: "p", refresh: "pr", expiresAt: 1 },
131-
createdAt: 1,
132-
},
133-
home,
134-
),
135-
store.saveProfile(
136-
{
137-
name: "work",
138-
tokens: { access: "w", refresh: "wr", expiresAt: 2 },
139-
createdAt: 2,
140-
},
141-
home,
126+
// Without the per-path queue, a large same-process burst shares one lock
127+
// deadline from invoke time and some waiters time out. With the queue,
128+
// each save gets its own window and all land.
129+
const names = Array.from(
130+
{ length: 50 },
131+
(_, index) => `profile-${String(index)}`,
132+
);
133+
const results = await Promise.allSettled(
134+
names.map((name) =>
135+
store.saveProfile(
136+
{
137+
name,
138+
tokens: {
139+
access: `access-${name}`,
140+
refresh: `refresh-${name}`,
141+
expiresAt: 1,
142+
},
143+
createdAt: 1,
144+
},
145+
home,
146+
),
142147
),
143-
]);
148+
);
144149

145-
const profiles = await store.listProfiles(home);
146-
expect(profiles.map((profile) => profile.name)).toEqual([
147-
"personal",
148-
"work",
149-
]);
150-
expect(profiles.find((profile) => profile.name === "personal")).toEqual({
151-
name: "personal",
152-
tokens: { access: "p", refresh: "pr", expiresAt: 1 },
153-
createdAt: 1,
154-
});
155-
expect(profiles.find((profile) => profile.name === "work")).toEqual({
156-
name: "work",
157-
tokens: { access: "w", refresh: "wr", expiresAt: 2 },
158-
createdAt: 2,
159-
});
150+
const failures = results.flatMap((result, index) =>
151+
result.status === "rejected"
152+
? [`${names[index]}: ${String(result.reason)}`]
153+
: [],
154+
);
155+
expect(failures).toEqual([]);
156+
expect((await store.listProfiles(home)).map((profile) => profile.name)).toEqual(
157+
[...names].sort(),
158+
);
160159
} finally {
161160
await rm(home, { recursive: true, force: true });
162161
}
@@ -181,6 +180,8 @@ describe("createAuthStore", () => {
181180

182181
// Hold the lock until the head of the same-process queue times out; the
183182
// queued write must still get its own lock window after we release.
183+
// `second` may already be polling when `first` rejects — release must land
184+
// inside LOCK_TIMEOUT_MS of that handoff.
184185
const lockPath = `${store.authPath(home)}.lock`;
185186
await writeFile(lockPath, "foreign", { mode: 0o600 });
186187

‎src/auth/store.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ export type { AuthProfile, BaseTokens };
1818
// for the same provider, so credentials are keyed by a user-chosen profile name
1919
// within a single file. Tokens are credentials, so the file is owner-only (0o600)
2020
// and the directory 0o700. Writes go through a temp file + rename so a concurrent
21-
// reader never observes a torn file.
21+
// reader never observes a torn file. Same-process writers also queue per auth path
22+
// so each lock wait starts its own deadline.
2223

2324
export interface AuthStore<TTokens extends BaseTokens> {
2425
authPath: (home?: string) => string;
@@ -48,8 +49,8 @@ interface AuthFile<TTokens extends BaseTokens> {
4849
const LOCK_RETRY_MS = 25;
4950
const LOCK_TIMEOUT_MS = 1_000;
5051

51-
// pid alone is not unique per call — concurrent saves in one process must not
52-
// share a temp path or the second rename hits ENOENT after the first moves it.
52+
// Per-call unique temp (pid + counter). Matches mcp/auth-store — pid alone is not
53+
// unique per call if writeAuthFile ever overlaps in-process.
5354
let tmpWriteCounter = 0;
5455

5556
// Same-process ops on one auth file queue here so a caller's lock deadline

0 commit comments

Comments
 (0)