From d0a7efb69f984b05904f22bbdfc54431d1eda997 Mon Sep 17 00:00:00 2001 From: "sergey.shkredov" Date: Fri, 25 Sep 2026 14:11:23 +0200 Subject: [PATCH] fix: report authentication failures through ACP login flow --- src/CodexEventHandler.ts | 17 +++- .../CodexACPAgent/auth-error-events.test.ts | 99 +++++++++++-------- 2 files changed, 71 insertions(+), 45 deletions(-) diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 62abb4863..29f4866af 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -312,6 +312,7 @@ export class CodexEventHandler { return; } await this.finishCompactionsForNotification(notification); + if (this.isAuthenticationRequiredError(notification.params.error.codexErrorInfo)) return; if (notification.params.willRetry) { await this.session.update(this.createSessionFailureUpdate(this.recordRetryWarning(notification.params, false))); return; @@ -364,6 +365,10 @@ export class CodexEventHandler { } async handleFailedTurn(turn: Turn): Promise { + if (turn.status === "failed" && this.isAuthenticationRequiredError(turn.error?.codexErrorInfo ?? null)) { + this.failure = RequestError.authRequired(); + return; + } const activeFailure = this.sessionState.sessionFailure; if (!this.supportsTypedSessionFailures || turn.status !== "failed" @@ -1193,6 +1198,14 @@ export class CodexEventHandler { }); return null; } + // ACP authRequired starts the client login flow. A second access update + // or chat message would show the same refusal as a false session error. + if (this.isAuthenticationRequiredError(error)) { + if (!params.willRetry && params.turnId === this.sessionState.currentTurnId) { + this.failure = RequestError.authRequired(); + } + return null; + } if (params.turnId !== this.sessionState.currentTurnId) { if (this.supportsTypedSessionFailures) { const failure = params.willRetry @@ -1231,10 +1244,6 @@ export class CodexEventHandler { this.failure = RequestError.internalError( this.createTurnErrorData(params.error), ); - } else if (this.isAuthenticationRequiredError(error)) { - this.failure = this.sessionState.authConfigured - ? RequestError.internalError(this.createTurnErrorData(params.error)) - : RequestError.authRequired(this.createTurnErrorData(params.error), params.error.message); } return createAgentTextMessageChunk(`${params.error.message}\n\n`); } diff --git a/src/__tests__/CodexACPAgent/auth-error-events.test.ts b/src/__tests__/CodexACPAgent/auth-error-events.test.ts index 860740cc4..a4145f9a0 100644 --- a/src/__tests__/CodexACPAgent/auth-error-events.test.ts +++ b/src/__tests__/CodexACPAgent/auth-error-events.test.ts @@ -247,7 +247,7 @@ describe("CodexEventHandler - auth error events", () => { }]); }); - it("returns a typed auth failure without forwarding provider details", async () => { + it("uses the ACP login error without forwarding provider details", async () => { const {result, updates} = await runPromptWithError(createTestSessionState({ sessionId: "typed-auth-session", account: null, @@ -259,11 +259,55 @@ describe("CodexEventHandler - auth error events", () => { misalignment: null, }, false, typedFailureCapabilities); - expect(result).toMatchObject({ - stopReason: "end_turn", - _meta: {jetbrains: {air: {sessionFailure: {category: "access"}}}}, - }); + expect(result).toMatchObject({code: -32000, message: "Authentication required"}); expect(JSON.stringify(result)).not.toContain("secret authentication details"); + expect(JSON.stringify(result)).not.toContain("sessionFailure"); + expect(updates).toEqual([]); + }); + + it("does not forward an auth error from another turn", async () => { + const {result, updates} = await runPromptWithError(createTestSessionState({ + sessionId: "foreign-auth-session", + account: {type: "apiKey"}, + }), { + message: "Sign in to continue", + codexErrorInfo: "unauthorized", + additionalDetails: null, + misalignment: null, + }, false, typedFailureCapabilities, "foreign-turn"); + + expect(result).toMatchObject({stopReason: "end_turn"}); + expect(updates).toEqual([]); + }); + + it("uses the ACP login error when auth arrives before the turn id", async () => { + const {result, updates} = await runPromptWithError(createTestSessionState({ + sessionId: "early-auth-session", + account: {type: "apiKey"}, + }), { + message: "Sign in to continue", + codexErrorInfo: "unauthorized", + additionalDetails: null, + misalignment: null, + }, false, typedFailureCapabilities, "turn-id", true); + + expect(result).toMatchObject({code: -32000, message: "Authentication required"}); + expect(updates).toEqual([]); + }); + + it("uses the ACP login error when only the failed turn reports auth", async () => { + const {result, updates} = await runPromptWithCompletedTurn( + createTestSessionState({sessionId: "completion-auth-session", account: {type: "apiKey"}}), + typedFailureCapabilities, + createTurn("failed", "turn-id", { + message: "Sign in to continue", + codexErrorInfo: "unauthorized", + additionalDetails: null, + misalignment: null, + }), + ); + + expect(result).toMatchObject({code: -32000, message: "Authentication required"}); expect(updates).toEqual([]); }); @@ -303,7 +347,6 @@ describe("CodexEventHandler - auth error events", () => { it.each([ ["connection", {responseStreamDisconnected: {httpStatusCode: 503}}], - ["access", "unauthorized"], ["limit", {responseStreamDisconnected: {httpStatusCode: 429}}], ["limit", "usageLimitExceeded"], ["service", "serverOverloaded"], @@ -758,25 +801,7 @@ describe("CodexEventHandler - auth error events", () => { expect(response).toMatchObject({ stopReason: "end_turn", }); - expect(updates).toEqual([{ - sessionUpdate: "session_info_update", - _meta: { - codex: { - error: { - message: "Reconnecting after provider returned 401", - codexErrorInfo: { - responseStreamDisconnected: { - httpStatusCode: 401, - }, - }, - additionalDetails: "HTTP status 401", - misalignment: null, - turnId: "turn-id", - willRetry: true, - }, - }, - }, - }]); + expect(updates).toEqual([]); }); it("returns AuthRequired for auth errors when no auth is configured", async () => { @@ -791,14 +816,8 @@ describe("CodexEventHandler - auth error events", () => { misalignment: null, }); - expect(error).toMatchObject({ - code: -32000, - message: "Authentication required: Authentication is required", - data: { - message: "Authentication is required", - codexErrorInfo: "unauthorized", - }, - }); + expect(error).toMatchObject({code: -32000, message: "Authentication required"}); + expect(JSON.stringify(error)).not.toContain("Authentication is required"); }); it.each(configuredAuthFailureCases)( @@ -810,14 +829,12 @@ describe("CodexEventHandler - auth error events", () => { ...sessionOverrides, }), turnError); - expect(error).toMatchObject({ - code: -32603, - message: "Internal error", - data: expectedData, - }); - expect(error).not.toMatchObject({ - code: -32000, - }); + if (turnError.codexErrorInfo === "usageLimitExceeded") { + expect(error).toMatchObject({code: -32603, message: "Internal error", data: expectedData}); + } else { + expect(error).toMatchObject({code: -32000, message: "Authentication required"}); + expect(JSON.stringify(error)).not.toContain(turnError.message); + } }, ); });