From e03cf22ec31a5f76c103e4d655ab6cfb1b128d63 Mon Sep 17 00:00:00 2001 From: Nikita Ashikhmin Date: Wed, 12 Aug 2026 21:10:38 +0400 Subject: [PATCH 1/2] feat: expose warning-severity advisories for AIR (IJAI-993) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Codex reports non-fatal advisories as dedicated app-server notifications, but codex-acp flattened them into assistant text: `warning` became `Warning: ` and `configWarning` became `Config warning: `, both as an untagged `agent_message_chunk`. The client could not tell them from the model's own words, so a compaction hint rendered as if the agent had said it. Extend the typed session-failure extension from #383 rather than adding a parallel notice concept: a record now carries an optional `severity` of `error` or `warning`, plus an `advisory` category whose wording comes from the app-server. Clients that negotiated the `sessionFailure` capability receive the structured record; everyone else keeps the existing text, unchanged. Advisories live in their own `sessionNotice` slot under a `:notice` id, so they never disturb the revision bookkeeping of an in-flight terminal failure — the two records coexist and the client decides which to show. `severity` is optional on the wire and absent means `error`, so an AIR build that predates this change keeps treating every record it receives as a failure. Scoped out deliberately: `thread/compacted` keeps its italic transcript line (informational, not a warning), and `guardianWarning` / `deprecationNotice` remain dropped as before. --- src/CodexAcpServer.ts | 14 +- src/CodexEventHandler.ts | 47 +++++- .../typed-session-failure-wire.test.ts | 136 ++++++++++++++++++ 3 files changed, 195 insertions(+), 2 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index dc21a60a..28b26b60 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -131,15 +131,26 @@ export interface SessionState { sessionTitle: string | null; sessionTitleSource: "unset" | "fallback" | "explicit" | "unknown"; sessionFailure?: SessionFailure; + /** + * Warning-severity advisories, tracked separately from {@link SessionState.sessionFailure} so a + * non-fatal notice never overwrites the revision bookkeeping of an in-flight terminal failure. + */ + sessionNotice?: SessionFailure; } export type SessionFailureCategory = | "transport_lost" | "auth_required" | "rate_limited" | "quota_exhausted" | "overloaded" | "context_exhausted" | "budget_exhausted" | "policy_denied" | "bad_request" - | "provider_error" | "internal_error"; + | "provider_error" | "internal_error" | "advisory"; export type SessionFailureAction = "retry" | "reconnect" | "login" | "new_turn" | "new_session"; +/** + * How loudly the client should render the record. Absent on the wire means `error`, so an AIR build + * that predates warning support keeps treating every record it receives as a failure. + */ +export type SessionFailureSeverity = "error" | "warning"; + export interface SessionFailure { id: string; revision: number; @@ -150,6 +161,7 @@ export interface SessionFailure { retryable: boolean; actions: SessionFailureAction[]; turnId?: string; + severity?: SessionFailureSeverity; } const CODEX_PROCESS_EXITED_ERROR_CODE = 1001; diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index df2ebf95..d2bb40df 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -4,6 +4,7 @@ import type { ServerNotification } from "./app-server"; import type { + SessionFailure, SessionFailureAction, SessionFailureCategory, SessionState, @@ -103,8 +104,18 @@ const SESSION_FAILURE_PRESENTATION: Record; type StructuredCodexErrorInfo = Exclude; type KeysOfUnion = T extends unknown ? keyof T : never; @@ -488,10 +499,18 @@ export class CodexEventHandler { private async createConfigWarningEvent(event: ConfigWarningNotification): Promise { const detailsText = event.details ? `\n\n${event.details}` : ""; + if (this.supportsTypedSessionFailures) { + return this.createSessionFailureUpdate( + this.recordSessionNotice(`${event.summary}${detailsText}`), + ); + } return createAgentTextMessageChunk(`Config warning: ${event.summary}${detailsText}\n\n`); } private createWarningEvent(event: WarningNotification): UpdateSessionEvent { + if (this.supportsTypedSessionFailures) { + return this.createSessionFailureUpdate(this.recordSessionNotice(event.message)); + } return createAgentTextMessageChunk(`Warning: ${event.message}\n\n`); } @@ -975,7 +994,7 @@ export class CodexEventHandler { : `${turnId}:error`; const failure: NonNullable = { id, - revision: previous?.id === id ? previous.revision + 1 : 1, + revision: nextSessionFailureRevision(previous, id), phase: "active" as const, category, source: "codex", @@ -988,6 +1007,32 @@ export class CodexEventHandler { return failure; } + /** + * Records a warning-severity advisory in its own slot and under its own id namespace, so it + * shares the wire contract with terminal failures without competing for their revision counter. + * + * The advisory replaces whichever advisory preceded it: the app-server sends these as standalone + * hints, so only the newest one is worth a banner. + */ + private recordSessionNotice(safeMessage: string): NonNullable { + const presentation = SESSION_FAILURE_PRESENTATION.advisory; + const previous = this.sessionState.sessionNotice; + const id = `${this.sessionState.sessionId}:notice:${this.sessionFailureEpoch}`; + const notice: NonNullable = { + id, + revision: nextSessionFailureRevision(previous, id), + phase: "active" as const, + category: "advisory", + source: "codex", + safeMessage, + retryable: presentation.retryable, + actions: presentation.actions, + severity: "warning", + }; + this.sessionState.sessionNotice = notice; + return notice; + } + private createSessionFailureMeta( failure: NonNullable, ): Record { diff --git a/src/__tests__/CodexACPAgent/typed-session-failure-wire.test.ts b/src/__tests__/CodexACPAgent/typed-session-failure-wire.test.ts index 824d0142..4930b486 100644 --- a/src/__tests__/CodexACPAgent/typed-session-failure-wire.test.ts +++ b/src/__tests__/CodexACPAgent/typed-session-failure-wire.test.ts @@ -134,8 +134,144 @@ describe("typed session failures over ACP transport", () => { expect(JSON.stringify(fixture.updates)).not.toContain("raw idle provider detail"); expect(JSON.stringify(fixture.updates)).not.toContain("secret idle detail"); }); + + it("delivers an app-server warning as a typed advisory instead of assistant text", async () => { + const fixture = await createIdleFixture("wire-warning"); + + fixture.sendServerNotification({ + method: "warning", + params: { + threadId: fixture.sessionId, + message: "Heads up: Long threads and multiple compactions can cause the model to be less accurate.", + }, + }); + await fixture.codexClient.waitForSessionNotifications(fixture.sessionId); + await vi.waitFor(() => expect(fixture.updates).toHaveLength(1)); + + expect(fixture.updates[0]).toMatchObject({ + update: { + sessionUpdate: "session_info_update", + _meta: { + jetbrains: { + air: { + sessionFailure: { + id: expect.stringMatching(/^wire-warning:notice:[0-9a-f-]+$/), + category: "advisory", + severity: "warning", + phase: "active", + revision: 1, + retryable: false, + actions: ["new_session"], + safeMessage: + "Heads up: Long threads and multiple compactions can cause the model to be less accurate.", + }, + }, + }, + }, + }, + }); + // The whole point of the change: it must not arrive as an agent message chunk. + expect(JSON.stringify(fixture.updates)).not.toContain("agent_message_chunk"); + expect(JSON.stringify(fixture.updates)).not.toContain("Warning: "); + }); + + it("folds a config warning's details into the advisory message", async () => { + const fixture = await createIdleFixture("wire-config-warning"); + + fixture.sendServerNotification({ + method: "configWarning", + params: {summary: "Unknown key `foo`", details: "in ~/.codex/config.toml"}, + }); + await fixture.codexClient.waitForSessionNotifications(fixture.sessionId); + await vi.waitFor(() => expect(fixture.updates).toHaveLength(1)); + + expect(fixture.updates[0]).toMatchObject({ + update: { + _meta: { + jetbrains: { + air: { + sessionFailure: { + category: "advisory", + severity: "warning", + safeMessage: "Unknown key `foo`\n\nin ~/.codex/config.toml", + }, + }, + }, + }, + }, + }); + }); + + it("keeps warnings as assistant text when the capability is absent", async () => { + const fixture = await createIdleFixture("wire-legacy-warning", {}); + + fixture.sendServerNotification({ + method: "warning", + params: {threadId: fixture.sessionId, message: "legacy advisory"}, + }); + await fixture.codexClient.waitForSessionNotifications(fixture.sessionId); + await vi.waitFor(() => expect(fixture.updates).toHaveLength(1)); + + expect(fixture.updates[0]!.update).toMatchObject({ + sessionUpdate: "agent_message_chunk", + content: {type: "text", text: "Warning: legacy advisory\n\n"}, + }); + }); + + it("keeps an advisory in its own id namespace so it never bumps an active failure's revision", async () => { + const fixture = await createIdleFixture("wire-mixed"); + + fixture.sendServerNotification({ + method: "error", + params: { + threadId: fixture.sessionId, + turnId: "turn-id", + willRetry: false, + error: {message: "provider blew up", codexErrorInfo: "serverOverloaded", additionalDetails: null}, + }, + }); + fixture.sendServerNotification({ + method: "warning", + params: {threadId: fixture.sessionId, message: "unrelated advisory"}, + }); + await fixture.codexClient.waitForSessionNotifications(fixture.sessionId); + await vi.waitFor(() => expect(fixture.updates).toHaveLength(2)); + + const records = fixture.updates.map(update => (update.update._meta as { + jetbrains: {air: {sessionFailure: {id: string; revision: number; severity?: string}}}; + }).jetbrains.air.sessionFailure); + // Distinct ids, each starting its own revision sequence at 1. + expect(records[0]).toMatchObject({revision: 1, category: "overloaded"}); + expect(records[1]).toMatchObject({revision: 1, category: "advisory", severity: "warning"}); + expect(records[0]!.id).not.toEqual(records[1]!.id); + expect(records[0]).not.toHaveProperty("severity"); + }); }); +/** A fixture whose session already completed a turn, so notifications route to a live event handler. */ +async function createIdleFixture( + sessionId: string, + clientCapabilities: acp.ClientCapabilities = typedFailureCapabilities, +) { + const fixture = createWireFixture(); + await fixture.initialize(clientCapabilities); + const sessionState = createTestSessionState({sessionId, account: {type: "apiKey"}}); + vi.spyOn(fixture.server, "getSessionState").mockReturnValue(sessionState); + vi.spyOn(fixture.appServer, "turnStart").mockResolvedValue({turn: createTurn("inProgress")}); + vi.spyOn(fixture.appServer, "awaitTurnCompleted").mockResolvedValue({ + threadId: sessionId, + turn: createTurn("completed"), + }); + + await fixture.client.prompt({ + sessionId, + prompt: [{type: "text", text: "settle the session"}], + }); + fixture.updates.splice(0); + + return {...fixture, sessionId}; +} + function createWireFixture(options: {exitCode?: number | null; stderr?: string} = {}) { const mockConnections = createMockConnections(); const appServer = new CodexAppServerClient(mockConnections.mockCodexConnection); From eab146fbafa03c8aade14cb870a71fa0a2783e3a Mon Sep 17 00:00:00 2001 From: Nikita Ashikhmin Date: Wed, 12 Aug 2026 21:18:00 +0400 Subject: [PATCH 2/2] feat: route deprecationNotice through the advisory banner too deprecationNotice carries the same {summary, details} shape as configWarning, so treating the two differently was inertia, not design: it was simply already being dropped. Unlike warning and configWarning it has no legacy text rendering to preserve, so it is emitted only to clients that negotiated typed records. A client that did not must keep seeing exactly what it sees today, which is nothing. Extracts joinSummaryAndDetails so the two notifications share one formatting rule rather than repeating it. --- src/CodexEventHandler.ts | 29 ++++++++++--- .../typed-session-failure-wire.test.ts | 42 +++++++++++++++++++ 2 files changed, 65 insertions(+), 6 deletions(-) diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index d2bb40df..cb5f3114 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -17,6 +17,7 @@ import type { CodexErrorInfo, CommandExecutionOutputDeltaNotification, ConfigWarningNotification, + DeprecationNoticeNotification, ErrorNotification, ItemGuardianApprovalReviewCompletedNotification, ItemGuardianApprovalReviewStartedNotification, @@ -109,6 +110,11 @@ const SESSION_FAILURE_PRESENTATION: Record { - const detailsText = event.details ? `\n\n${event.details}` : ""; + const text = joinSummaryAndDetails(event.summary, event.details); if (this.supportsTypedSessionFailures) { - return this.createSessionFailureUpdate( - this.recordSessionNotice(`${event.summary}${detailsText}`), - ); + return this.createSessionFailureUpdate(this.recordSessionNotice(text)); } - return createAgentTextMessageChunk(`Config warning: ${event.summary}${detailsText}\n\n`); + return createAgentTextMessageChunk(`Config warning: ${text}\n\n`); + } + + /** + * Unlike `warning` and `configWarning`, this notification was dropped outright, so there is no + * legacy rendering to preserve. It is surfaced only to clients that negotiated typed records; + * every other client keeps seeing exactly what it sees today, which is nothing. + */ + private createDeprecationNoticeEvent(event: DeprecationNoticeNotification): UpdateSessionEvent | null { + if (!this.supportsTypedSessionFailures) return null; + return this.createSessionFailureUpdate( + this.recordSessionNotice(joinSummaryAndDetails(event.summary, event.details)), + ); } private createWarningEvent(event: WarningNotification): UpdateSessionEvent { diff --git a/src/__tests__/CodexACPAgent/typed-session-failure-wire.test.ts b/src/__tests__/CodexACPAgent/typed-session-failure-wire.test.ts index 4930b486..9dc6b816 100644 --- a/src/__tests__/CodexACPAgent/typed-session-failure-wire.test.ts +++ b/src/__tests__/CodexACPAgent/typed-session-failure-wire.test.ts @@ -202,6 +202,48 @@ describe("typed session failures over ACP transport", () => { }); }); + it("surfaces a deprecation notice that used to be dropped outright", async () => { + const fixture = await createIdleFixture("wire-deprecation"); + + fixture.sendServerNotification({ + method: "deprecationNotice", + params: {summary: "`--legacy-flag` is deprecated", details: "Use `--flag` instead."}, + }); + await fixture.codexClient.waitForSessionNotifications(fixture.sessionId); + await vi.waitFor(() => expect(fixture.updates).toHaveLength(1)); + + expect(fixture.updates[0]).toMatchObject({ + update: { + sessionUpdate: "session_info_update", + _meta: { + jetbrains: { + air: { + sessionFailure: { + category: "advisory", + severity: "warning", + safeMessage: "`--legacy-flag` is deprecated\n\nUse `--flag` instead.", + }, + }, + }, + }, + }, + }); + }); + + it("still drops a deprecation notice when the capability is absent", async () => { + const fixture = await createIdleFixture("wire-legacy-deprecation", {}); + + fixture.sendServerNotification({ + method: "deprecationNotice", + params: {summary: "`--legacy-flag` is deprecated", details: null}, + }); + await fixture.codexClient.waitForSessionNotifications(fixture.sessionId); + + // This notification produced nothing before typed records existed; a client that did not + // negotiate them must not suddenly start seeing it. + expect(fixture.updates).toEqual([]); + }); + it("keeps warnings as assistant text when the capability is absent", async () => { const fixture = await createIdleFixture("wire-legacy-warning", {});