Skip to content

Commit b224aa8

Browse files
committed
fix(auth): coordinate OAuth refresh across concurrent codebase processes
Users running multiple codebase instances simultaneously (5-10 at once isn't unusual when juggling several agents) hit a real failure mode: when two processes detect token expiry at the same moment, each fires its own refresh request with the shared refresh_token. Most OAuth backends rotate refresh tokens on each use, so the second call returns "refresh token already used" and that process gets logged out — even though everything looked fine on its end. Wrap the refresh path in a proper-lockfile-coordinated critical section on the credentials directory: at most one process holds the lock at a time, and after acquiring it we re-read the credentials file before deciding to refresh. The first process refreshes + saves; the others acquire the lock, see fresh tokens on disk, and skip the refresh entirely. In-memory single-flight remains for within-process collapse. Also bumps the refresh-skew buffer from 60s to 5 min so the request never travels under a near-expired token under clock skew + slow refresh latency.
1 parent 0f2a4a2 commit b224aa8

4 files changed

Lines changed: 184 additions & 31 deletions

File tree

package-lock.json

Lines changed: 39 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "codebase-cli",
3-
"version": "2.0.0-pre.40",
3+
"version": "2.0.0-pre.41",
44
"description": "Codebase CLI — a TypeScript coding agent on the pi-mono runtime. OAuth-aware, any LLM provider, single install.",
55
"keywords": [
66
"ai",
@@ -62,10 +62,12 @@
6262
"@earendil-works/pi-agent-core": "0.74.0",
6363
"@earendil-works/pi-ai": "0.74.0",
6464
"@types/diff": "^7.0.2",
65+
"@types/proper-lockfile": "^4.1.4",
6566
"diff": "^9.0.0",
6667
"glob": "^13.0.1",
6768
"ignore": "^7.0.0",
6869
"ink": "^5.2.1",
70+
"proper-lockfile": "^4.1.2",
6971
"react": "^18.3.1",
7072
"typebox": "^1.1.24"
7173
},

src/auth/token-manager.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,3 +138,62 @@ describe("TokenManager", () => {
138138
expect(fetchSpy).not.toHaveBeenCalled();
139139
});
140140
});
141+
142+
describe("TokenManager — multi-process coordination", () => {
143+
let dataRoot: string;
144+
145+
beforeEach(() => {
146+
dataRoot = mkdtempSync(join(tmpdir(), "tm-mp-"));
147+
});
148+
149+
afterEach(() => {
150+
rmSync(dataRoot, { recursive: true, force: true });
151+
vi.restoreAllMocks();
152+
});
153+
154+
it("two TokenManagers refreshing concurrently produce only one refresh round-trip", async () => {
155+
const storeA = new CredentialsStore({ dataRoot });
156+
const storeB = new CredentialsStore({ dataRoot });
157+
storeA.save(freshCreds({ expiresAt: Date.now() + 5_000 }));
158+
const fetchSpy = vi.spyOn(globalThis, "fetch").mockImplementation(
159+
() =>
160+
new Promise((resolve) => {
161+
setTimeout(
162+
() =>
163+
resolve(
164+
new Response(JSON.stringify({ access_token: "access-NEW", expires_in: 3600 }), {
165+
status: 200,
166+
headers: { "Content-Type": "application/json" },
167+
}),
168+
),
169+
50,
170+
);
171+
}),
172+
);
173+
const tmA = new TokenManager({ store: storeA, oauthConfig: OAUTH });
174+
const tmB = new TokenManager({ store: storeB, oauthConfig: OAUTH });
175+
const [a, b] = await Promise.all([tmA.getAccessToken(), tmB.getAccessToken()]);
176+
expect(a).toBe("access-NEW");
177+
expect(b).toBe("access-NEW");
178+
expect(fetchSpy).toHaveBeenCalledTimes(1);
179+
});
180+
181+
it("a TokenManager started after another process refreshed reads the rotated token without refreshing", async () => {
182+
const storeA = new CredentialsStore({ dataRoot });
183+
storeA.save(freshCreds({ expiresAt: Date.now() + 5_000 }));
184+
vi.spyOn(globalThis, "fetch").mockResolvedValueOnce(
185+
new Response(JSON.stringify({ access_token: "access-NEW", expires_in: 3600 }), {
186+
status: 200,
187+
headers: { "Content-Type": "application/json" },
188+
}),
189+
);
190+
const tmA = new TokenManager({ store: storeA, oauthConfig: OAUTH });
191+
await tmA.getAccessToken();
192+
// Now another process starts up with its own TokenManager.
193+
const fetchSpyLate = vi.spyOn(globalThis, "fetch");
194+
const storeB = new CredentialsStore({ dataRoot });
195+
const tmB = new TokenManager({ store: storeB, oauthConfig: OAUTH });
196+
await expect(tmB.getAccessToken()).resolves.toBe("access-NEW");
197+
expect(fetchSpyLate).not.toHaveBeenCalled();
198+
});
199+
});

src/auth/token-manager.ts

Lines changed: 83 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,6 @@
1+
import { mkdirSync } from "node:fs";
2+
import { dirname } from "node:path";
3+
import * as lockfile from "proper-lockfile";
14
import type { Credentials, CredentialsStore } from "./credentials.js";
25
import { type OAuthConfig, refreshAccessToken } from "./flow.js";
36

@@ -6,32 +9,48 @@ export interface TokenManagerOptions {
69
oauthConfig: OAuthConfig;
710
/**
811
* Refresh when the access token's remaining lifetime falls below this
9-
* many milliseconds. Default 60s — generous enough to ride out clock
10-
* skew + a slow refresh round-trip without ever sending an expired
11-
* token over the wire.
12+
* many milliseconds. Default 5 minutes — wide enough to absorb clock
13+
* skew, slow refresh round-trips, and a request that starts just under
14+
* the wire so it never travels with an already-expired token.
1215
*/
1316
refreshSkewMs?: number;
17+
/**
18+
* Max time to wait acquiring the cross-process refresh lock before
19+
* giving up. If another `codebase` process is mid-refresh we wait;
20+
* if the lock is wedged (stale, crashed peer) we bail rather than
21+
* hang the user's request.
22+
*/
23+
lockTimeoutMs?: number;
1424
}
1525

1626
/**
1727
* Read-through, refresh-aware accessor for the OAuth access token.
1828
*
19-
* Pi-mono's `getApiKey` runs on every API call, so we have to be fast in
20-
* the common case (token still valid). Slow path: refresh + persist before
21-
* returning the new token. A single in-flight promise (`pending`) prevents
22-
* a burst of concurrent calls from firing parallel refreshes at the same
23-
* moment — they all await the one refresh that's already running.
29+
* Two coordination layers:
30+
* 1. In-memory single-flight (`pending`) — multiple awaits within ONE
31+
* process collapse into one refresh round-trip.
32+
* 2. Filesystem lockfile on the credentials directory — multiple
33+
* `codebase` processes on the same machine coordinate so only one
34+
* refreshes at a time. The others wait, re-read the rotated token,
35+
* and skip their own refresh. Necessary because refresh tokens are
36+
* often one-time-use: two parallel refreshes burn the shared refresh
37+
* token and one process gets logged out.
38+
*
39+
* Pi-mono's `getApiKey` runs on every API call, so the cached-token fast
40+
* path stays branch-free; the slow path only fires near expiry.
2441
*/
2542
export class TokenManager {
2643
private readonly store: CredentialsStore;
2744
private readonly oauthConfig: OAuthConfig;
2845
private readonly refreshSkewMs: number;
46+
private readonly lockTimeoutMs: number;
2947
private pending: Promise<string> | null = null;
3048

3149
constructor(options: TokenManagerOptions) {
3250
this.store = options.store;
3351
this.oauthConfig = options.oauthConfig;
34-
this.refreshSkewMs = options.refreshSkewMs ?? 60_000;
52+
this.refreshSkewMs = options.refreshSkewMs ?? 5 * 60_000;
53+
this.lockTimeoutMs = options.lockTimeoutMs ?? 30_000;
3554
}
3655

3756
/**
@@ -47,39 +66,75 @@ export class TokenManager {
4766
if (!creds.refreshToken) {
4867
throw new Error("access token expired and no refresh token saved — run `codebase auth login`");
4968
}
50-
return this.refresh(creds.refreshToken);
69+
return this.refresh();
5170
}
5271

5372
private needsRefresh(creds: Credentials): boolean {
5473
if (!creds.expiresAt) return false;
5574
return creds.expiresAt - this.refreshSkewMs <= Date.now();
5675
}
5776

58-
private refresh(refreshToken: string): Promise<string> {
77+
private refresh(): Promise<string> {
5978
if (this.pending) return this.pending;
6079
this.pending = (async () => {
6180
try {
62-
const next = await refreshAccessToken(this.oauthConfig, refreshToken);
63-
// Preserve fields the refresh response doesn't echo back (source,
64-
// email, userId) so they survive every rotation. The refresh
65-
// response is authoritative for tokens + expiry; we layer the
66-
// stable metadata back on top.
67-
const existing = this.store.load();
68-
this.store.save({
69-
accessToken: next.accessToken,
70-
refreshToken: next.refreshToken ?? existing?.refreshToken ?? refreshToken,
71-
expiresAt: next.expiresAt,
72-
scopes: next.scopes,
73-
source: existing?.source ?? next.source,
74-
userId: existing?.userId ?? next.userId,
75-
email: existing?.email ?? next.email,
76-
provider: existing?.provider ?? next.provider,
77-
});
78-
return next.accessToken;
81+
return await this.refreshWithLock();
7982
} finally {
8083
this.pending = null;
8184
}
8285
})();
8386
return this.pending;
8487
}
88+
89+
/**
90+
* Acquire a cross-process lock, then re-check (another process may have
91+
* already refreshed by the time we get the lock), then refresh + save.
92+
* The double-check is the whole point of taking the lock — it converts
93+
* N parallel refreshes into 1 refresh + N-1 reads of the fresh token.
94+
*/
95+
private async refreshWithLock(): Promise<string> {
96+
const lockDir = dirname(this.store.filePath);
97+
mkdirSync(lockDir, { recursive: true });
98+
const release = await lockfile.lock(lockDir, {
99+
retries: {
100+
retries: Math.max(1, Math.ceil(this.lockTimeoutMs / 500)),
101+
minTimeout: 250,
102+
maxTimeout: 1000,
103+
factor: 1.5,
104+
randomize: true,
105+
},
106+
// 30s — stale-lock window. Survives a normal refresh; releases
107+
// quickly enough that a crashed peer doesn't wedge us for long.
108+
stale: 30_000,
109+
});
110+
try {
111+
const reread = this.store.load();
112+
if (reread && !this.needsRefresh(reread)) return reread.accessToken;
113+
if (!reread?.refreshToken) {
114+
throw new Error("access token expired and no refresh token saved — run `codebase auth login`");
115+
}
116+
const next = await refreshAccessToken(this.oauthConfig, reread.refreshToken);
117+
// Preserve fields the refresh response doesn't echo back (source,
118+
// email, userId) so they survive every rotation. The refresh
119+
// response is authoritative for tokens + expiry; we layer the
120+
// stable metadata back on top.
121+
this.store.save({
122+
accessToken: next.accessToken,
123+
refreshToken: next.refreshToken ?? reread.refreshToken,
124+
expiresAt: next.expiresAt,
125+
scopes: next.scopes,
126+
source: reread.source ?? next.source,
127+
userId: reread.userId ?? next.userId,
128+
email: reread.email ?? next.email,
129+
provider: reread.provider ?? next.provider,
130+
});
131+
return next.accessToken;
132+
} finally {
133+
await release().catch(() => {
134+
// Release can fail if the lock was stale-released by another
135+
// process while we held it. The credentials are already saved
136+
// at that point — there's nothing useful to do here.
137+
});
138+
}
139+
}
85140
}

0 commit comments

Comments
 (0)