jerry/claude-agent-acp
1
0
Fork 0
mirror of https://github.com/agentclientprotocol/claude-agent-acp.git synced 2026-10-27 04:30:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

feat: publish the model fallback as a warning advisory (#990)

Browse Source 
## 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.
This commit is contained in:
nikita-ashihmin
2026-08-12 22:16:28 +04:00
committed by GitHub
parent 1f09e9a3ca
commit 35aaddb3d5
2 changed files with 108 additions and 14 deletions
Download Patch File Download Diff File Expand all files Collapse all files

74
src/acp-agent.ts
View File

@@ -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<string, number>();
const sessionFailureEpoch = randomUUID();
const activeSessionFailures = new Map<string, PublishedSessionFailure>();
@@ -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<PublishedSessionFailure | undefined> => {
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,

48
src/tests/acp-agent.test.ts
View File

@@ -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(