feat: expose warning-severity advisories for AIR - #393
Open
nikita-ashihmin wants to merge 2 commits into
Open
Conversation
Codex reports non-fatal advisories as dedicated app-server notifications, but codex-acp flattened them into assistant text: `warning` became `Warning: <message>` and `configWarning` became `Config warning: <summary>`, 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 agentclientprotocol#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.
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
Codex reports non-fatal advisories as dedicated app-server notifications, but we flatten them into assistant text:
Called with neither
messageIdnormeta, so the client sees an ordinaryagent_message_chunkand cannot tell it from the model's own output. In practice a compaction hint renders as if the agent had said it:The structure exists upstream — codex emits
warning { message }fromcodex_core::compact— and is lost here, at the adapter.Change
Extends the typed session-failure extension from #383 instead of adding a parallel notice concept:
severity?: "error" | "warning"on the record. Optional, and absent meanserror, so a client that predates this change keeps treating every record it receives as a failure.advisorycategory, whose wording comes from the app-server rather than our canned presentation table.sessionNoticeslot under a:noticeid, so they never disturb the revision bookkeeping of an in-flight terminal failure. The two coexist and the client decides which to surface.Three notifications now route through it, all gated on the client having negotiated the
sessionFailurecapability:warningWarning: <message>textconfigWarningConfig warning: <summary>textdeprecationNoticedeprecationNoticecarries the same{summary, details}shape asconfigWarning, so the two now share one formatting rule (joinSummaryAndDetails). It had no legacy rendering to preserve, so it stays invisible to clients that did not negotiate typed records rather than newly appearing as text.Scoped out deliberately
thread/compactedkeeps its italic transcript line — informational rather than a warning, and promoting it to a banner would be louder than the current UX.guardianWarningstays dropped: the guardian review it refers to is already rendered as a tool call, so a banner may duplicate it. Worth revisiting against real traffic.windows/worldWritableWarningstays dropped: it carries a structured path list, so it needs its own rendering rather than a text advisory.Tests
Six cases added to
typed-session-failure-wire.test.ts, all exercised end to end over the real ACP transport:warningarrives as anadvisoryrecord withseverity: "warning"— and asserts the payload contains neitheragent_message_chunknor theWarning:prefixconfigWarningfoldsdetailsinto the advisory messagedeprecationNoticesurfaces as an advisorywarningtext is byte-identical to today, anddeprecationNoticestill produces nothingseveritynpx vitest run: 406 passed, 28 skipped.npm run typecheckclean.Client side: IJAI-993.