Skip to content

Commit 4192829

Browse files
committed
refactor(auth): drop duplicate extraction tests and narration
The shared lock and session helpers stay. Remove the second copy of cases already pinned by the store and refresh-lock suites, unused file-lock exports, and comments that only restated the extraction.
1 parent c68a273 commit 4192829

13 files changed

Lines changed: 26 additions & 476 deletions

‎src/auth/bearer-headers.ts‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ export interface BearerToken {
22
readonly access: string;
33
}
44

5-
// The one header every provider carries on an inference request; provider
6-
// extras (account ids, client identifiers) ride alongside via the spread.
75
export function bearerHeaders(
86
token: BearerToken,
97
extra: Record<string, string> = {},

‎src/auth/codex/login.ts‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@ export async function startCodexLogin(
2727
return startProviderLogin<CodexTokens>(loginOpts, {
2828
oauthConfig: codexOAuthConfig,
2929
startCallbackServer: (state) => startCodexCallbackServer(state, copy),
30-
// The mapper leaves expiresAt unset when the response omits expires_in.
3130
exchangeCode: async (code, verifier, now) =>
3231
withDefaultCodexExpiry(await exchangeCodexCode(code, verifier, now), now),
3332
saveProfile: (profile) => saveCodexProfile(profile, home),

‎src/auth/codex/refresh-lock.test.ts‎

Lines changed: 2 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -13,12 +13,10 @@ import { CodexRefreshLockTimeoutError } from "./refresh-lock.js";
1313

1414
import { withMockedModule } from "../../../testkit/mock-module.js";
1515

16-
// Non-ENOENT lock-file failures must surface raw instead of being swallowed
17-
// (the pre-unification codex lock caught every fs error). Flags gate each
18-
// failure to its own test; undefined keeps every call on the real module.
16+
// Non-ENOENT read and stat failures must surface raw instead of being swallowed.
17+
// Flags gate each failure to its own test; undefined keeps every call on the real module.
1918
let failingReadPath: string | undefined;
2019
let failingStatPath: string | undefined;
21-
let failingUnlinkPath: string | undefined;
2220
await withMockedModule(
2321
import.meta.resolve("node:fs/promises"),
2422
(real: typeof import("node:fs/promises")) => ({
@@ -31,10 +29,6 @@ await withMockedModule(
3129
if (args[0] === failingStatPath) throw new Error("lock stat failed");
3230
return real.stat(...args);
3331
},
34-
unlink: async (...args: Parameters<typeof real.unlink>) => {
35-
if (args[0] === failingUnlinkPath) throw new Error("lock release failed");
36-
return real.unlink(...args);
37-
},
3832
}),
3933
);
4034

@@ -286,31 +280,4 @@ describe("codex refresh lock", () => {
286280
await rm(dir, { recursive: true, force: true });
287281
}
288282
});
289-
290-
test("a non-ENOENT release failure surfaces after the work value", async () => {
291-
const dir = await tempDir();
292-
try {
293-
const lock = join(dir, "refresh.lock");
294-
failingUnlinkPath = lock;
295-
try {
296-
// The refresh succeeded, but a release that fails for a real reason
297-
// (not a steal's ENOENT) must not vanish: the caller has to know the
298-
// lock file is still on disk.
299-
await expect(
300-
withCodexRefreshLock(lock, async () => "done", {
301-
timeoutMs: 1_000,
302-
retryMs: 10,
303-
}),
304-
).rejects.toThrow("lock release failed");
305-
// The failed release leaves the claim file in place for recovery.
306-
await expect(readFile(lock, "utf8")).resolves.toContain(
307-
String(process.pid),
308-
);
309-
} finally {
310-
failingUnlinkPath = undefined;
311-
}
312-
} finally {
313-
await rm(dir, { recursive: true, force: true });
314-
}
315-
});
316283
});

‎src/auth/codex/refresh-lock.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,7 @@ import { withFileLock } from "../file-lock.js";
1414

1515
// Stale horizon for legacy locks that carry no holder PID (foreign writers,
1616
// pre-tagging locks). Kept far above any legitimate hold and contender
17-
// timeout so a live holder is never declared stale mid-wait. The provider
18-
// auth store's own lock horizon is deliberately smaller (5s, short auth-file
19-
// mutations); these two horizons are pinned invariants and must not unify.
17+
// timeout so a live holder is never declared stale mid-wait.
2018
const DEFAULT_STALE_MS = 120_000;
2119

2220
export interface CodexRefreshLockOptions {

‎src/auth/file-lock.test.ts‎

Lines changed: 0 additions & 221 deletions
This file was deleted.

‎src/auth/file-lock.ts‎

Lines changed: 6 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,6 @@ import {
99
import { dirname } from "node:path";
1010
import { setTimeout as delay } from "node:timers/promises";
1111

12-
// File-lock primitives shared by every credential lock in this repo. The
13-
// invariants that make them safe: per-waiter unique claims name the holder,
14-
// kill(pid, 0) liveness takes over a crashed holder's lock, an untagged lock
15-
// older than the stale horizon is presumed orphaned, a steal re-reads before
16-
// unlinking so a concurrent winner's fresh lock is never removed, and both a
17-
// post-create content check and a fenced release keep a loser from deleting
18-
// or releasing under a live winner.
19-
2012
export function isErrnoCode(err: unknown, code: string): boolean {
2113
return (
2214
typeof err === "object" &&
@@ -26,27 +18,27 @@ export function isErrnoCode(err: unknown, code: string): boolean {
2618
);
2719
}
2820

29-
export function isEnoent(err: unknown): boolean {
21+
function isEnoent(err: unknown): boolean {
3022
return isErrnoCode(err, "ENOENT");
3123
}
3224

3325
// Per-waiter unique claim (pid + counter): pid alone is not unique per call,
3426
// so the counter leg tells our claim from a concurrent winner's.
3527
let lockClaimCounter = 0;
3628

37-
export function newLockClaim(): string {
29+
function newLockClaim(): string {
3830
return `${String(process.pid)}:${String((lockClaimCounter += 1))}`;
3931
}
4032

41-
export function holderPid(content: string): number | null {
33+
function holderPid(content: string): number | null {
4234
const pid = Number(content.split(":")[0]?.trim());
4335
return Number.isInteger(pid) && pid > 0 ? pid : null;
4436
}
4537

4638
// kill(pid, 0) liveness: success or EPERM means alive; ESRCH/EINVAL mean
4739
// dead. Any other failure reads as alive — never steal a live holder's lock
4840
// on a confused signal check; the waiter times out with a recovery hint.
49-
export function isPidAlive(pid: number): boolean {
41+
function isPidAlive(pid: number): boolean {
5042
try {
5143
process.kill(pid, 0);
5244
return true;
@@ -57,9 +49,7 @@ export function isPidAlive(pid: number): boolean {
5749

5850
// Null when the lock vanished under the waiter (a release raced the read);
5951
// anything but ENOENT propagates — permission and disk errors must surface.
60-
export async function readLockContent(
61-
lockPath: string,
62-
): Promise<string | null> {
52+
async function readLockContent(lockPath: string): Promise<string | null> {
6353
try {
6454
return await readFile(lockPath, "utf8");
6555
} catch (err) {
@@ -71,7 +61,7 @@ export async function readLockContent(
7161
// True when the lock is safe to take over: a tagged holder with a dead PID
7262
// (crashed), or a legacy untagged lock older than the stale horizon. A
7363
// vanished lock reads as not stale; the acquire loop retries the create.
74-
export async function isLockStale(
64+
async function isLockStale(
7565
lockPath: string,
7666
staleMs: number,
7767
content: string,
@@ -114,9 +104,6 @@ export interface FileLockOptions {
114104
timeoutError: FileLockTimeoutError;
115105
}
116106

117-
// Credential locks in this repo serialize on exactly this acquire/release
118-
// pairing: exclusive-create with a holder tag, stale takeover, and a fenced
119-
// claim-matched release. Do not fork a variant that drops any of the three.
120107
export async function withFileLock<TResult>(
121108
lockPath: string,
122109
work: () => Promise<TResult>,

‎src/auth/oauth-scope-check.ts‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,9 +106,6 @@ function classifyStatus(
106106
return unavailable(providerLabel, `provider HTTP ${status}`);
107107
}
108108

109-
// One probe shape per provider: refresh staged tokens into the request
110-
// headers, hit the provider's catalog/list endpoint, and classify the
111-
// response. Only the headers, endpoint, and timeout differ.
112109
async function probeOAuthScope(
113110
providerLabel: string,
114111
authorizeHeaders: () => Promise<Record<string, string>>,

0 commit comments

Comments
 (0)