Skip to content

Commit f6921fd

Browse files
vincentkocsteipete
vincentkoc
authored and
steipete
committed
refactor(auth): break oauth helper import cycle
1 parent 20debfa commit f6921fd

5 files changed

Lines changed: 204 additions & 180 deletions

File tree

‎src/agents/auth-profiles/external-auth.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import {
55
overlayRuntimeExternalOAuthProfiles,
66
shouldPersistRuntimeExternalOAuthProfile,
77
type RuntimeExternalOAuthProfile,
8-
} from "./oauth-manager.js";
8+
} from "./oauth-shared.js";
99
import type { AuthProfileStore, OAuthCredential } from "./types.js";
1010

1111
type ExternalAuthProfileMap = Map<string, ProviderExternalAuthProfile>;

‎src/agents/auth-profiles/external-cli-sync.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
isSafeToOverwriteStoredOAuthIdentity,
99
shouldBootstrapFromExternalCliCredential,
1010
shouldReplaceStoredOAuthCredential,
11-
} from "./oauth-manager.js";
11+
} from "./oauth-shared.js";
1212
import type { AuthProfileStore, OAuthCredential } from "./types.js";
1313

1414
export {
@@ -18,7 +18,7 @@ export {
1818
isSafeToOverwriteStoredOAuthIdentity,
1919
shouldBootstrapFromExternalCliCredential,
2020
shouldReplaceStoredOAuthCredential,
21-
} from "./oauth-manager.js";
21+
} from "./oauth-shared.js";
2222

2323
export type ExternalCliResolvedProfile = {
2424
profileId: string;

‎src/agents/auth-profiles/external-oauth.test.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -124,7 +124,7 @@ describe("auth external oauth helpers", () => {
124124
expect(shouldPersist).toBe(true);
125125
});
126126

127-
it("overlays external CLI OAuth only when the stored credential is no longer usable", () => {
127+
it("does not use Codex CLI OAuth as a runtime overlay source", () => {
128128
readCodexCliCredentialsCachedMock.mockReturnValue(
129129
createCredential({
130130
access: "fresh-cli-access-token",
@@ -146,9 +146,9 @@ describe("auth external oauth helpers", () => {
146146
);
147147

148148
expect(overlaid.profiles["openai-codex:default"]).toMatchObject({
149-
access: "fresh-cli-access-token",
150-
refresh: "fresh-cli-refresh-token",
151-
expires: expect.any(Number),
149+
access: "stale-store-access-token",
150+
refresh: "stale-store-refresh-token",
151+
accountId: "acct-cli",
152152
});
153153
});
154154

‎src/agents/auth-profiles/oauth-manager.ts‎

Lines changed: 21 additions & 173 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,17 @@ import {
66
OAUTH_REFRESH_LOCK_OPTIONS,
77
log,
88
} from "./constants.js";
9-
import { hasUsableOAuthCredential as hasUsableStoredOAuthCredential } from "./credential-state.js";
9+
import {
10+
areOAuthCredentialsEquivalent,
11+
hasUsableOAuthCredential,
12+
isSafeToAdoptBootstrapOAuthIdentity,
13+
isSafeToOverwriteStoredOAuthIdentity,
14+
overlayRuntimeExternalOAuthProfiles,
15+
shouldBootstrapFromExternalCliCredential,
16+
shouldPersistRuntimeExternalOAuthProfile,
17+
shouldReplaceStoredOAuthCredential,
18+
type RuntimeExternalOAuthProfile,
19+
} from "./oauth-shared.js";
1020
import { ensureAuthStoreFile, resolveAuthStorePath, resolveOAuthRefreshLockPath } from "./paths.js";
1121
import {
1222
ensureAuthProfileStore,
@@ -76,179 +86,17 @@ export class OAuthManagerRefreshError extends Error {
7686
}
7787
}
7888

79-
export type RuntimeExternalOAuthProfile = {
80-
profileId: string;
81-
credential: OAuthCredential;
82-
persistence?: "runtime-only" | "persisted";
89+
export {
90+
areOAuthCredentialsEquivalent,
91+
hasUsableOAuthCredential,
92+
isSafeToAdoptBootstrapOAuthIdentity,
93+
isSafeToOverwriteStoredOAuthIdentity,
94+
overlayRuntimeExternalOAuthProfiles,
95+
shouldBootstrapFromExternalCliCredential,
96+
shouldPersistRuntimeExternalOAuthProfile,
97+
shouldReplaceStoredOAuthCredential,
8398
};
84-
85-
export function areOAuthCredentialsEquivalent(
86-
a: OAuthCredential | undefined,
87-
b: OAuthCredential,
88-
): boolean {
89-
if (!a || a.type !== "oauth") {
90-
return false;
91-
}
92-
return (
93-
a.provider === b.provider &&
94-
a.access === b.access &&
95-
a.refresh === b.refresh &&
96-
a.expires === b.expires &&
97-
a.email === b.email &&
98-
a.enterpriseUrl === b.enterpriseUrl &&
99-
a.projectId === b.projectId &&
100-
a.accountId === b.accountId
101-
);
102-
}
103-
104-
function hasNewerStoredOAuthCredential(
105-
existing: OAuthCredential | undefined,
106-
incoming: OAuthCredential,
107-
): boolean {
108-
return Boolean(
109-
existing &&
110-
existing.provider === incoming.provider &&
111-
Number.isFinite(existing.expires) &&
112-
(!Number.isFinite(incoming.expires) || existing.expires > incoming.expires),
113-
);
114-
}
115-
116-
export function shouldReplaceStoredOAuthCredential(
117-
existing: OAuthCredential | undefined,
118-
incoming: OAuthCredential,
119-
): boolean {
120-
if (!existing || existing.type !== "oauth") {
121-
return true;
122-
}
123-
if (areOAuthCredentialsEquivalent(existing, incoming)) {
124-
return false;
125-
}
126-
return !hasNewerStoredOAuthCredential(existing, incoming);
127-
}
128-
129-
export function hasUsableOAuthCredential(
130-
credential: OAuthCredential | undefined,
131-
now = Date.now(),
132-
): boolean {
133-
return hasUsableStoredOAuthCredential(credential, { now });
134-
}
135-
136-
function normalizeAuthIdentityToken(value: string | undefined): string | undefined {
137-
const trimmed = value?.trim();
138-
return trimmed ? trimmed : undefined;
139-
}
140-
141-
function normalizeAuthEmailToken(value: string | undefined): string | undefined {
142-
return normalizeAuthIdentityToken(value)?.toLowerCase();
143-
}
144-
145-
function hasOAuthIdentity(credential: Pick<OAuthCredential, "accountId" | "email">): boolean {
146-
return (
147-
normalizeAuthIdentityToken(credential.accountId) !== undefined ||
148-
normalizeAuthEmailToken(credential.email) !== undefined
149-
);
150-
}
151-
152-
function hasMatchingOAuthIdentity(
153-
existing: Pick<OAuthCredential, "accountId" | "email">,
154-
incoming: Pick<OAuthCredential, "accountId" | "email">,
155-
): boolean {
156-
const existingAccountId = normalizeAuthIdentityToken(existing.accountId);
157-
const incomingAccountId = normalizeAuthIdentityToken(incoming.accountId);
158-
if (existingAccountId !== undefined && incomingAccountId !== undefined) {
159-
return existingAccountId === incomingAccountId;
160-
}
161-
162-
const existingEmail = normalizeAuthEmailToken(existing.email);
163-
const incomingEmail = normalizeAuthEmailToken(incoming.email);
164-
if (existingEmail !== undefined && incomingEmail !== undefined) {
165-
return existingEmail === incomingEmail;
166-
}
167-
168-
return false;
169-
}
170-
171-
export function isSafeToOverwriteStoredOAuthIdentity(
172-
existing: OAuthCredential | undefined,
173-
incoming: OAuthCredential,
174-
): boolean {
175-
if (!existing || existing.type !== "oauth") {
176-
return true;
177-
}
178-
if (existing.provider !== incoming.provider) {
179-
return false;
180-
}
181-
if (areOAuthCredentialsEquivalent(existing, incoming)) {
182-
return true;
183-
}
184-
if (!hasOAuthIdentity(existing)) {
185-
return false;
186-
}
187-
return hasMatchingOAuthIdentity(existing, incoming);
188-
}
189-
190-
export function isSafeToAdoptBootstrapOAuthIdentity(
191-
existing: OAuthCredential | undefined,
192-
incoming: OAuthCredential,
193-
): boolean {
194-
if (!existing || existing.type !== "oauth") {
195-
return true;
196-
}
197-
if (existing.provider !== incoming.provider) {
198-
return false;
199-
}
200-
if (areOAuthCredentialsEquivalent(existing, incoming)) {
201-
return true;
202-
}
203-
if (!hasOAuthIdentity(existing)) {
204-
return true;
205-
}
206-
return hasMatchingOAuthIdentity(existing, incoming);
207-
}
208-
209-
export function shouldBootstrapFromExternalCliCredential(params: {
210-
existing: OAuthCredential | undefined;
211-
imported: OAuthCredential;
212-
now?: number;
213-
}): boolean {
214-
const now = params.now ?? Date.now();
215-
if (hasUsableOAuthCredential(params.existing, now)) {
216-
return false;
217-
}
218-
return hasUsableOAuthCredential(params.imported, now);
219-
}
220-
221-
export function overlayRuntimeExternalOAuthProfiles(
222-
store: AuthProfileStore,
223-
profiles: Iterable<RuntimeExternalOAuthProfile>,
224-
): AuthProfileStore {
225-
const externalProfiles = Array.from(profiles);
226-
if (externalProfiles.length === 0) {
227-
return store;
228-
}
229-
const next = structuredClone(store);
230-
for (const profile of externalProfiles) {
231-
next.profiles[profile.profileId] = profile.credential;
232-
}
233-
return next;
234-
}
235-
236-
export function shouldPersistRuntimeExternalOAuthProfile(params: {
237-
profileId: string;
238-
credential: OAuthCredential;
239-
profiles: Iterable<RuntimeExternalOAuthProfile>;
240-
}): boolean {
241-
for (const profile of params.profiles) {
242-
if (profile.profileId !== params.profileId) {
243-
continue;
244-
}
245-
if (profile.persistence === "persisted") {
246-
return true;
247-
}
248-
return !areOAuthCredentialsEquivalent(profile.credential, params.credential);
249-
}
250-
return true;
251-
}
99+
export type { RuntimeExternalOAuthProfile };
252100

253101
function hasOAuthCredentialChanged(
254102
previous: Pick<OAuthCredential, "access" | "refresh" | "expires">,

0 commit comments

Comments
 (0)