From 35aaddb3d5b14eca316582ab285af8a660150dae Mon Sep 17 00:00:00 2001 From: nikita-ashihmin Date: Wed, 12 Aug 2026 22:16:28 +0400 Subject: [PATCH] feat: publish the model fallback as a warning advisory (#990) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Problem When the API refuses a turn, the SDK retries on the fallback model and makes the swap persistent for the session. We flag that with a transcript line: ```ts text: `**Model fallback:** ${message.original_model} declined this request${category}; retried with ${message.fallback_model}. ${outcome}${explanation}`, ``` It goes out as an `agent_message_chunk`, so the client cannot tell a session-level advisory from something the model actually said — the bold label is the only severity signal available, and it is just characters inside the assistant's own text. The comment above that code already states the stakes: *"Without a notice the user just sees regenerated output."* The notice matters; the channel it uses does not fit it. ## Change Routes it through the AIR typed session-failure extension instead, as a new `advisory` category: - `severity?: "error" | "warning"` on the record. **Optional, and absent means `error`**, so a client that predates this keeps treating every record it receives as a failure. - The advisory carries its own wording (`safeMessage`) rather than the canned per-category message, so the client renders the explanation verbatim in a warning banner. - **Clients that did not negotiate the `sessionFailure` capability keep the exact transcript line they get today.** ### Advisories are not turn failures Two existing behaviours would have mangled them, so both are now guarded: - `createSessionFailure` clears every other active record when a new one is published. An advisory no longer does that, and is no longer cleared by one — otherwise a model-fallback notice would erase an actionable error, or vice versa. - `clearFailuresFromEarlierTurns` clears everything whose `turnId` is not the active turn. Advisories carry no `turnId`, so without the guard every turn boundary would sweep them away. They therefore live under their own `:notice:` id namespace, session-scoped, one slot, newest wins. ## Scoped out `**Fast mode turned off:**` (`syncFastMode…`) is the same pattern and deserves the same treatment, but it sits in a class method while the failure machinery is closure-scoped inside `prompt()`. Lifting that out is a separate change rather than something to smuggle into this one. ## Tests `npm run build`, `npm run check`, and `npm run test:run` (**766 passed**) are clean. The existing `model_refusal_fallback` tests still assert the `**Model fallback:**` transcript line and still pass — they never call `initialize`, so they exercise the no-capability path, which is exactly the backward-compatibility guarantee. One test added for the typed path, asserting that the advisory is published with `severity: "warning"`, that its `safeMessage` carries the real model ids and refusal category, that it uses the `:notice:` id namespace, that it contains no `**` markdown lead-in, and that the transcript line is **not** also emitted. Client side: IJAI-993. --- src/acp-agent.ts | 74 ++++++++++++++++++++++++++++++------- src/tests/acp-agent.test.ts | 48 ++++++++++++++++++++++++ 2 files changed, 108 insertions(+), 14 deletions(-) diff --git a/src/acp-agent.ts b/src/acp-agent.ts index 3064093a..3b70d658 100644 --- a/src/acp-agent.ts +++ b/src/acp-agent.ts @@ -938,6 +938,7 @@ function supportsSubagentTranscript(capabilities?: ClientCapabilities | null): b } type AirSessionFailureCategory = + | "advisory" | "auth_required" | "bad_request" | "budget_exhausted" @@ -950,10 +951,23 @@ type AirSessionFailureCategory = | "transport_lost" | "worker_shutdown"; +/** + * How loudly the client renders a record. Absent on the wire means `error`, so a client that + * predates warning support keeps treating everything it receives as a failure. + */ +type AirSessionFailureSeverity = "error" | "warning"; + const AIR_FAILURE_PRESENTATION: Record< AirSessionFailureCategory, { message: string; retryable: boolean; actions: string[] } > = { + // A non-fatal notice whose wording comes from the adapter or the SDK, so this message is only the + // fallback for an advisory published without one. + advisory: { + message: "Claude reported a notice.", + retryable: false, + actions: [], + }, auth_required: { message: "Sign in to continue using Claude.", retryable: false, @@ -2328,7 +2342,11 @@ export class ClaudeAcpAgent { revision: number; category: AirSessionFailureCategory; turnId?: string; + severity?: AirSessionFailureSeverity; + /** Set when the notice carries its own wording instead of the canned per-category message. */ + safeMessage?: string; }; + const isAdvisory = (failure: PublishedSessionFailure) => failure.severity === "warning"; const failureRevisions = new Map(); const sessionFailureEpoch = randomUUID(); const activeSessionFailures = new Map(); @@ -2347,10 +2365,11 @@ export class ClaudeAcpAgent { phase, category: failure.category, source: "claude", - safeMessage: presentation.message, + safeMessage: failure.safeMessage ?? presentation.message, retryable: presentation.retryable, actions: presentation.actions, ...(failure.turnId ? { turnId: failure.turnId } : {}), + ...(failure.severity ? { severity: failure.severity } : {}), }, }, }, @@ -2403,16 +2422,29 @@ export class ClaudeAcpAgent { const createSessionFailure = async ( category: AirSessionFailureCategory, - options: { turnScoped?: boolean } = {}, + options: { turnScoped?: boolean; safeMessage?: string } = {}, ): Promise => { if (!supportsAirSessionFailures(this.clientCapabilities) || !isCurrentConsumer()) { return undefined; } + // An advisory is not a failure of the turn: it keeps its own id namespace and its own slot, so + // it neither clears an actionable error nor is cleared by one. Only the newest advisory + // matters, so they all share one id and bump its revision. + if (category === "advisory") { + const id = `${params.sessionId}:notice:${sessionFailureEpoch}`; + return { + id, + revision: (failureRevisions.get(id) ?? 0) + 1, + category, + severity: "warning", + ...(options.safeMessage ? { safeMessage: options.safeMessage } : {}), + }; + } const turnId = options.turnScoped === false ? undefined : session.activeTurn?.promptUuid; const id = turnId ? `${turnId}:error` : `${params.sessionId}:session-error:${sessionFailureEpoch}`; - await clearSessionFailure((failure) => failure.id !== id); + await clearSessionFailure((failure) => failure.id !== id && !isAdvisory(failure)); return { id, revision: (failureRevisions.get(id) ?? 0) + 1, @@ -2423,7 +2455,7 @@ export class ClaudeAcpAgent { const publishSessionFailure = async ( category: AirSessionFailureCategory, - options: { turnScoped?: boolean } = {}, + options: { turnScoped?: boolean; safeMessage?: string } = {}, ) => { const failure = await createSessionFailure(category, options); if (!failure) return; @@ -2435,7 +2467,11 @@ export class ClaudeAcpAgent { const clearFailuresFromEarlierTurns = async () => { const activeTurnId = session.activeTurn?.promptUuid; - await clearSessionFailure((failure) => failure.turnId !== activeTurnId); + // Advisories carry no turnId, so without the guard every turn boundary would sweep them away. + // They are session-scoped and stay until superseded or dismissed by the user. + await clearSessionFailure( + (failure) => failure.turnId !== activeTurnId && !isAdvisory(failure), + ); }; const internalErrorForClient = (data: unknown, rawDetail?: string) => @@ -3549,16 +3585,26 @@ export class ClaudeAcpAgent { const outcome = persistent ? `The session will continue on ${message.fallback_model}.` : `The session stays on ${message.original_model}.`; - await sendUpdate({ - sessionId: message.session_id, - update: { - sessionUpdate: "agent_message_chunk", - content: { - type: "text", - text: `**Model fallback:** ${message.original_model} declined this request${category}; retried with ${message.fallback_model}. ${outcome}${explanation}`, + const fallbackNotice = + `${message.original_model} declined this request${category}; ` + + `retried with ${message.fallback_model}. ${outcome}${explanation}`; + // A silent model swap is a session-level advisory, not something the model said. + // Clients that negotiated typed records get it as one; the rest keep the bold-label + // transcript line, which was the only way to flag it before. + if (supportsAirSessionFailures(this.clientCapabilities)) { + await publishSessionFailure("advisory", { safeMessage: fallbackNotice }); + } else { + await sendUpdate({ + sessionId: message.session_id, + update: { + sessionUpdate: "agent_message_chunk", + content: { + type: "text", + text: `**Model fallback:** ${fallbackNotice}`, + }, }, - }, - }); + }); + } if (persistent) { await this.syncModelAfterRefusalFallback( params.sessionId, diff --git a/src/tests/acp-agent.test.ts b/src/tests/acp-agent.test.ts index 5c4d180f..49b59944 100644 --- a/src/tests/acp-agent.test.ts +++ b/src/tests/acp-agent.test.ts @@ -4996,6 +4996,54 @@ describe("model refusal fallback handling", () => { expect(notice.content.text).toContain("This request tripped a safety classifier."); }); + it("publishes the model fallback as a warning advisory for AIR clients", async () => { + const { agent, sessionUpdate } = createCapturingAgent(); + await agent.initialize({ + protocolVersion: 1, + clientCapabilities: { + _meta: { jetbrains: { air: { version: 1, capabilities: ["sessionFailure"] } } }, + }, + }); + injectGeneratorSession( + agent, + makeGenerator([refusalFallbackMessage(), successResult()]), + modelStateOverrides, + ); + + await agent.prompt({ sessionId: "test-session", prompt: [{ type: "text", text: "test" }] }); + + const updates = sessionUpdate.mock.calls.map((c: any[]) => (c[0] as { update: any }).update); + // The advisory replaces the transcript line; it must not be delivered as both. + expect( + updates.find( + (u: any) => + u.sessionUpdate === "agent_message_chunk" && u.content?.text?.includes("Model fallback"), + ), + ).toBeUndefined(); + + const advisory = updates + .map((u: any) => u._meta?.jetbrains?.air?.sessionFailure) + .find((record: any) => record?.category === "advisory"); + expect(advisory).toEqual( + expect.objectContaining({ + category: "advisory", + severity: "warning", + phase: "active", + source: "claude", + retryable: false, + actions: [], + }), + ); + // Its own wording, carried through instead of the canned per-category message. + expect(advisory.safeMessage).toContain("claude-fable-5"); + expect(advisory.safeMessage).toContain("claude-opus-4-8"); + expect(advisory.safeMessage).toContain("(cyber)"); + // No markdown lead-in: the banner supplies the severity, so the text must not fake one. + expect(advisory.safeMessage).not.toContain("**"); + // Its own id namespace, so it never collides with a turn's error record. + expect(advisory.id).toContain(":notice:"); + }); + it("tracks the raw model id when the fallback model is not among the options", async () => { const { agent, sessionUpdate } = createCapturingAgent(); injectGeneratorSession(