Skip to content

Commit b7c78dd

Browse files
authored
fix: Track configured auth for turn errors (#245)
* fix: Track configured auth for turn errors Closes #244 * Clear session auth state on logout * Refresh session auth state after logout * Refresh session auth state after login * Treat custom providers as auth configured * Track auth state by model provider * Normalize configured auth error responses * Test provider auth state for restored sessions
1 parent 54dcc04 commit b7c78dd

8 files changed

Lines changed: 581 additions & 17 deletions

src/CodexAcpClient.ts

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -217,6 +217,10 @@ export class CodexAcpClient {
217217
return response.requiresOpenaiAuth && !response.account;
218218
}
219219

220+
hasGatewayAuth(): boolean {
221+
return this.gatewayConfig !== null;
222+
}
223+
220224
async getAccount(): Promise<GetAccountResponse> {
221225
return this.codexClient.accountRead({refreshToken: false});
222226
}
@@ -238,6 +242,7 @@ export class CodexAcpClient {
238242
sessionId: request.sessionId,
239243
currentModelId: currentModelId,
240244
models: codexModels,
245+
modelProvider: response.modelProvider,
241246
currentServiceTier: response.serviceTier as ServiceTier ?? null,
242247
additionalDirectories,
243248
}
@@ -264,6 +269,7 @@ export class CodexAcpClient {
264269
sessionId: request.sessionId,
265270
currentModelId: currentModelId,
266271
models: codexModels,
272+
modelProvider: response.modelProvider,
267273
currentServiceTier: response.serviceTier as ServiceTier ?? null,
268274
thread: historyResponse.thread,
269275
additionalDirectories,
@@ -289,6 +295,7 @@ export class CodexAcpClient {
289295
sessionId: response.thread.id,
290296
currentModelId: currentModelId,
291297
models: codexModels,
298+
modelProvider: response.modelProvider,
292299
currentServiceTier: response.serviceTier as ServiceTier ?? null,
293300
additionalDirectories,
294301
};
@@ -698,6 +705,7 @@ export type SessionMetadata = {
698705
sessionId: string,
699706
currentModelId: string,
700707
models: Model[],
708+
modelProvider?: string | null,
701709
currentServiceTier?: ServiceTier | null,
702710
additionalDirectories: string[],
703711
}

src/CodexAcpServer.ts

Lines changed: 67 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import {RequestError, type SessionId, type SessionModeState} from "@agentclientp
33
import {CodexEventHandler} from "./CodexEventHandler";
44
import {CodexApprovalHandler} from "./CodexApprovalHandler";
55
import {CodexElicitationHandler} from "./CodexElicitationHandler";
6-
import {type CodexAuthRequest, getCodexAuthMethods} from "./CodexAuthMethod";
6+
import {type CodexAuthRequest, getCodexAuthMethods, isCodexAuthRequest} from "./CodexAuthMethod";
77
import {CodexAcpClient, type SessionMetadata, type SessionMetadataWithThread} from "./CodexAcpClient";
88
import type {McpStartupResult} from "./CodexAppServerClient";
99
import {ACPSessionConnection, type AcpClientConnection, type UpdateSessionEvent} from "./ACPSessionConnection";
@@ -79,6 +79,8 @@ export interface SessionState {
7979
modelContextWindow: number | null;
8080
rateLimits: RateLimitsMap | null;
8181
account: Account | null;
82+
authConfigured: boolean;
83+
authProvider: string | null;
8284
cwd: string;
8385
additionalDirectories: string[];
8486
fastModeEnabled: boolean;
@@ -87,6 +89,11 @@ export interface SessionState {
8789
terminalOutputMode: TerminalOutputMode;
8890
}
8991

92+
interface ActiveAuthState {
93+
account: Account | null;
94+
authConfigured: boolean;
95+
}
96+
9097
interface PendingMcpStartupSession {
9198
requestedServers: Set<string>;
9299
afterVersion: number;
@@ -156,7 +163,8 @@ export class CodexAcpServer {
156163
this.availableCommands = new CodexCommands(
157164
connection,
158165
codexAcpClient,
159-
(operation) => this.runWithProcessCheck(operation)
166+
(operation) => this.runWithProcessCheck(operation),
167+
() => this.refreshSessionsAuthState(null)
160168
);
161169
}
162170

@@ -247,6 +255,7 @@ export class CodexAcpServer {
247255
async handleError(e: Error){
248256
if (e.message.includes("log out") || e.message.includes("cloud requirements")) {
249257
await this.runWithProcessCheck(() => this.codexAcpClient.logout());
258+
await this.refreshSessionsAuthState(null);
250259
throw RequestError.internalError(`${(e.message)}\n\nYou have been logged out. Please try again.`);
251260
}
252261
}
@@ -346,9 +355,10 @@ export class CodexAcpServer {
346355
}
347356

348357
const {sessionId, currentModelId, models} = sessionMetadata;
349-
let account: Account | null;
358+
const authProvider = sessionMetadata.modelProvider ?? this.codexAcpClient.getModelProvider();
359+
let authState: ActiveAuthState;
350360
try {
351-
account = await this.getActiveAccount();
361+
authState = await this.getAuthStateForProvider(authProvider);
352362
} catch (err) {
353363
if (resumeSubscribed && requestedSessionGeneration !== null) {
354364
await this.cleanupStaleSessionOpen(sessionId, requestedSessionGeneration);
@@ -375,7 +385,9 @@ export class CodexAcpServer {
375385
totalTokenUsage: null,
376386
modelContextWindow: null,
377387
rateLimits: null,
378-
account: account,
388+
account: authState.account,
389+
authConfigured: authState.authConfigured,
390+
authProvider: authProvider,
379391
cwd: request.cwd,
380392
additionalDirectories: sessionMetadata.additionalDirectories,
381393
fastModeEnabled: sessionMetadata.currentServiceTier === "fast",
@@ -401,12 +413,36 @@ export class CodexAcpServer {
401413
return [sessionId, sessionModelState, sessionModeState];
402414
}
403415

404-
private async getActiveAccount(){
405-
if (this.codexAcpClient.getModelProvider()) {
406-
return null
416+
private async getAuthStateForProvider(authProvider: string | null): Promise<ActiveAuthState> {
417+
if (!this.authProviderUsesOpenAiAccount(authProvider)) {
418+
return {
419+
account: null,
420+
authConfigured: true,
421+
};
407422
}
408423
const accountResponse = await this.runWithProcessCheck(() => this.codexAcpClient.getAccount());
409-
return accountResponse.account;
424+
return {
425+
account: accountResponse.account,
426+
authConfigured: accountResponse.account !== null || !accountResponse.requiresOpenaiAuth,
427+
};
428+
}
429+
430+
private authProviderUsesOpenAiAccount(authProvider: string | null): boolean {
431+
return authProvider === null || authProvider === "openai";
432+
}
433+
434+
private authProvidersMatch(a: string | null, b: string | null): boolean {
435+
if (this.authProviderUsesOpenAiAccount(a) && this.authProviderUsesOpenAiAccount(b)) {
436+
return true;
437+
}
438+
return a === b;
439+
}
440+
441+
private getAuthProviderForAuthenticateRequest(request: acp.AuthenticateRequest): string | null {
442+
if (isCodexAuthRequest(request) && request.methodId === "gateway") {
443+
return "custom-gateway";
444+
}
445+
return null;
410446
}
411447

412448
async loadSession(params: acp.LoadSessionRequest): Promise<LegacyLoadSessionResponse> {
@@ -565,16 +601,32 @@ export class CodexAcpServer {
565601
logger.log("Authenticate request failed");
566602
throw RequestError.invalidParams();
567603
}
604+
await this.refreshSessionsAuthState(this.getAuthProviderForAuthenticateRequest(_params));
568605
logger.log("Authenticate request completed");
569606
return { };
570607
}
571608

572609
async logout(_params: acp.LogoutRequest): Promise<void> {
573610
logger.log("Logout request received");
574611
await this.runWithProcessCheck(() => this.codexAcpClient.logout());
612+
await this.refreshSessionsAuthState(null);
575613
logger.log("Logout request completed");
576614
}
577615

616+
private async refreshSessionsAuthState(authProvider: string | null): Promise<void> {
617+
if (this.sessions.size === 0) return;
618+
619+
const sessionsToRefresh = [...this.sessions.values()]
620+
.filter(sessionState => this.authProvidersMatch(sessionState.authProvider, authProvider));
621+
if (sessionsToRefresh.length === 0) return;
622+
623+
const authState = await this.getAuthStateForProvider(authProvider);
624+
for (const sessionState of sessionsToRefresh) {
625+
sessionState.account = authState.account;
626+
sessionState.authConfigured = authState.authConfigured;
627+
}
628+
}
629+
578630
async setSessionMode(
579631
_params: acp.SetSessionModeRequest,
580632
): Promise<acp.SetSessionModeResponse> {
@@ -798,9 +850,10 @@ export class CodexAcpServer {
798850
}
799851

800852
const {sessionId, currentModelId, models, thread} = sessionMetadata;
801-
let account: Account | null;
853+
const authProvider = sessionMetadata.modelProvider ?? this.codexAcpClient.getModelProvider();
854+
let authState: ActiveAuthState;
802855
try {
803-
account = await this.getActiveAccount();
856+
authState = await this.getAuthStateForProvider(authProvider);
804857
} catch (err) {
805858
if (subscribed) {
806859
await this.cleanupStaleSessionOpen(request.sessionId, requestedSessionGeneration);
@@ -826,7 +879,9 @@ export class CodexAcpServer {
826879
totalTokenUsage: null,
827880
modelContextWindow: null,
828881
rateLimits: null,
829-
account: account,
882+
account: authState.account,
883+
authConfigured: authState.authConfigured,
884+
authProvider: authProvider,
830885
cwd: request.cwd,
831886
additionalDirectories: sessionMetadata.additionalDirectories,
832887
fastModeEnabled: sessionMetadata.currentServiceTier === "fast",

src/CodexCommands.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,24 @@ export type CommandHandleOptions = {
2121
onTurnStarted?: (turnId: string, threadId: string) => void;
2222
};
2323

24+
export type LogoutHandler = () => void | Promise<void>;
25+
2426
export class CodexCommands {
2527
private readonly connection: AcpClientConnection;
2628
private readonly codexAcpClient: CodexAcpClient;
2729
private readonly runWithProcessCheck: <T>(operation: () => Promise<T>) => Promise<T>;
30+
private readonly onLogout: LogoutHandler;
2831

2932
constructor(
3033
connection: AcpClientConnection,
3134
codexAcpClient: CodexAcpClient,
32-
runWithProcessCheck: <T>(operation: () => Promise<T>) => Promise<T>
35+
runWithProcessCheck: <T>(operation: () => Promise<T>) => Promise<T>,
36+
onLogout: LogoutHandler = () => {}
3337
) {
3438
this.connection = connection;
3539
this.codexAcpClient = codexAcpClient;
3640
this.runWithProcessCheck = runWithProcessCheck;
41+
this.onLogout = onLogout;
3742
}
3843

3944
async publish(sessionId: string): Promise<void> {
@@ -198,6 +203,7 @@ export class CodexCommands {
198203
}
199204
case "logout": {
200205
await this.runWithProcessCheck(() => this.codexAcpClient.logout());
206+
await this.onLogout();
201207
const session = new ACPSessionConnection(this.connection, sessionId);
202208
await session.update({
203209
sessionUpdate: "agent_message_chunk",

src/CodexEventHandler.ts

Lines changed: 34 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -589,9 +589,15 @@ export class CodexEventHandler {
589589
}
590590

591591
private async createErrorEvent(params: ErrorNotification): Promise<UpdateSessionEvent> {
592-
const error = params.error.codexErrorInfo
593-
if (error == "unauthorized" || error == "usageLimitExceeded" || this.getHttpStatusCode(error) == 401) {
594-
this.failure = RequestError.authRequired();
592+
const error = params.error.codexErrorInfo;
593+
if (error === "usageLimitExceeded") {
594+
this.failure = RequestError.internalError(
595+
this.createTurnErrorData(params.error),
596+
);
597+
} else if (this.isAuthenticationRequiredError(error)) {
598+
this.failure = this.sessionState.authConfigured
599+
? RequestError.internalError(this.createTurnErrorData(params.error))
600+
: RequestError.authRequired(this.createTurnErrorData(params.error), params.error.message);
595601
}
596602
return {
597603
sessionUpdate: "agent_message_chunk",
@@ -602,6 +608,10 @@ export class CodexEventHandler {
602608
}
603609
}
604610

611+
private isAuthenticationRequiredError(error: CodexErrorInfo | null): boolean {
612+
return error === "unauthorized" || this.getHttpStatusCode(error) === 401;
613+
}
614+
605615
private getHttpStatusCode(error: CodexErrorInfo | null): number | null {
606616
if (error !== null && typeof error === "object") {
607617
if ("httpConnectionFailed" in error) {
@@ -617,6 +627,27 @@ export class CodexEventHandler {
617627
return null;
618628
}
619629

630+
private createTurnErrorData(error: ErrorNotification["error"]): {
631+
message: string;
632+
codexErrorInfo?: CodexErrorInfo;
633+
additionalDetails?: string;
634+
} {
635+
const data: {
636+
message: string;
637+
codexErrorInfo?: CodexErrorInfo;
638+
additionalDetails?: string;
639+
} = {
640+
message: error.additionalDetails ?? error.message,
641+
};
642+
if (error.codexErrorInfo !== null) {
643+
data.codexErrorInfo = error.codexErrorInfo;
644+
}
645+
if (error.additionalDetails !== null) {
646+
data.additionalDetails = error.additionalDetails;
647+
}
648+
return data;
649+
}
650+
620651
private handleTokenUsageUpdated(params: ThreadTokenUsageUpdatedNotification): void {
621652
this.sessionState.lastTokenUsage = toTokenCount(params.tokenUsage.last);
622653
this.sessionState.totalTokenUsage = toTokenCount(params.tokenUsage.total);

0 commit comments

Comments
 (0)