Skip to content

Commit 5377bfe

Browse files
benbrandtbenbrandt
authored
fix: report structured ACP v2 failures (spacering-net#651)
## Summary Report terminal ACP v2 foreground failures through the standard `idle` / `stopReason: "error"` shape, with an `error` object containing a code and user-facing message. This is the next isolated package after spacering-net#649 and spacering-net#650, based on main. - Carry terminal failure independently of AIR negotiation and of the v1-shaped prompt result. Preserve v1 responses, AIR session-failure metadata, quota, and usage. - Cover ordinary prompted turns, local commands, compact/review commands, implementation follow-ups, and agent-initiated turns. - Before insertion, keep the JSON-RPC error response as well as the idle error when foreground work ends. After insertion, keep the successful messageId response and report the work outcome through idle. - Preserve cancellation precedence, including runtime errors raised while aborting. Retried model errors, individual failed tools, unrelated-turn errors, and idle background errors do not become terminal foreground failures. - Close unfinished v2 tool calls before the idle failure instead of clearing their tracker first. - Omit opaque error data from the new standard error object; use safe messages for process loss and unexpected internal exceptions. V2 fallback text for process loss is sanitized too. - Keep resume snapshots free of failure details so reconnecting does not replay a terminal incident as a new completion. The specification checked was `4034a8c`: [v2 failure semantics](https://github.com/agentclientprotocol/agent-client-protocol/blob/4034a8cbc99bc48fd4579f70a3407b07907e0eb6/docs/protocol/v2/prompt-lifecycle.mdx#L801-L839). Initial commands, system prompts, and standard subagent sessions remain separate work packages. Existing AIR subagent behavior is unchanged. ## Validation - `npm run typecheck` — passed, including examples. - `npm test` — 1,815 passed; 36 skipped. - `npm run build` — passed. - `npm run test:integration` — 5 passed against real Codex app-server with a local mock model. - The real-process v2 test now verifies terminal HTTP 401 -> idle/error with code -32000, then clears the mock failure and verifies a successful prompt and all resume modes. The mock holds the error across Codex's authentication retry rather than accidentally returning an unrelated error. - Focused wire/snapshot coverage checks AIR and non-AIR failures, pre-insertion authentication, cancellation races, failed-tool cleanup, implementation failures, unrelated errors, and resume behavior. - `npm run codex-test` — live-model smoke test from an empty scratch directory completed with end_turn (backward-compatibility check). - `git diff --check` — passed. The full live-model E2E suite was not run. Failure scenarios use the local model mock rather than changing real credentials or provoking failures against an external provider.
1 parent 986955a commit 5377bfe

24 files changed

Lines changed: 600 additions & 52 deletions

‎readme-dev.md‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,12 @@ to v2; it reuses the runtime environment variables above.
157157
These are behaviors the AIR (JetBrains) ACP client relies on that aren't obvious from the wire
158158
schema alone:
159159

160+
- Failed foreground work ends with `state: "idle"`, `stopReason: "error"` and a standard `error`
161+
object containing `code` and a user-facing `message`. Before insertion, the prompt also fails
162+
with a JSON-RPC error; after insertion, its successful `messageId` response is not revoked.
163+
Cancellation remains `cancelled`. Retries, individual failed tools, and idle background errors
164+
do not become terminal session failures. Raw process stderr and opaque error data are not sent
165+
in the standard error object.
160166
- On a failed prompt, read `sessionFailure` (`_meta.jetbrains.air.sessionFailure`) and `quota`
161167
(`_meta.quota`) from the idle `state_update` that ends the turn, not from a separate error
162168
channel. The same `_meta` v1 attaches to `PromptResponse` is copied onto the v2 idle update.

‎src/AcpV2Prompt.ts‎

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
import * as acp from "@agentclientprotocol/sdk/unstable";
22
import type * as acpV2 from "@agentclientprotocol/sdk/experimental/v2/unstable";
33
import type {ServerNotification} from "./app-server";
4+
import {CODEX_PROCESS_EXITED_ERROR_CODE} from "./app-server-recovery/AppServerExit";
5+
import {readableServiceErrorMessage} from "./ServiceErrorMessage";
6+
import {ResponseError} from "vscode-jsonrpc";
47

58
/**
69
* Lets a prompt report when Codex records its user message. On v2 that moment, not the turn
@@ -29,6 +32,8 @@ export type UserMessageInsertion = {
2932
* ids. Lets a pending `$/cancel_request` interrupt the right turn before insertion.
3033
*/
3134
onTurnStarted?: (turn: {threadId: string, turnId: string}) => void;
35+
/** Reports terminal work failure without changing the v1 prompt response or depending on AIR. */
36+
onFailure?: (error: acpV2.Error) => void;
3237
};
3338

3439
/**
@@ -64,16 +69,16 @@ export function toV1PromptRequest(params: acpV2.PromptRequest): acp.PromptReques
6469
}
6570

6671
/**
67-
* The `idle` state that ends a v2 turn carries what v1 answers the prompt with: the stop reason,
68-
* the token usage and the `_meta` (quota, and a typed session failure for clients that
69-
* negotiated it).
72+
* Carry v1 usage and metadata into v2, but report terminal failures through the standard error
73+
* stop reason. Cancellation takes precedence over errors raised while aborting work.
7074
*/
71-
export function toV2IdleState(response: acp.PromptResponse): acpV2.StateUpdate {
75+
export function toV2IdleState(response: acp.PromptResponse, failure?: acpV2.Error | null): acpV2.StateUpdate {
7276
const state = {
7377
state: "idle" as const,
7478
...(response.usage != null ? {usage: response.usage} : {}),
7579
...(response._meta != null ? {_meta: response._meta} : {}),
7680
};
81+
if (failure != null && response.stopReason !== "cancelled") return {...state, stopReason: "error", error: failure};
7782
switch (response.stopReason) {
7883
case "end_turn":
7984
return {...state, stopReason: "end_turn"};
@@ -88,6 +93,17 @@ export function toV2IdleState(response: acp.PromptResponse): acpV2.StateUpdate {
8893
}
8994
}
9095

96+
/** Opaque request data and process diagnostics are not user-facing failure details. */
97+
export function toV2Failure(error: unknown): acpV2.Error {
98+
if (error instanceof acp.RequestError || error instanceof ResponseError) {
99+
if (error.code === CODEX_PROCESS_EXITED_ERROR_CODE) {
100+
return {code: error.code, message: "Connection to Codex was lost."};
101+
}
102+
return {code: error.code, message: readableServiceErrorMessage(error.message)};
103+
}
104+
return {code: -32603, message: "Codex encountered an internal error."};
105+
}
106+
91107
/**
92108
* The agent message that tells a v2 client why its prompt failed after it was inserted, when
93109
* there is no longer a request to answer with an error.

‎src/CodexAcpServer.ts‎

Lines changed: 55 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ import {
5656
isInsertedUserMessage,
5757
postInsertionFailureText,
5858
toV1PromptRequest,
59+
toV2Failure,
5960
toV2IdleState,
6061
type UserMessageInsertion,
6162
} from "./AcpV2Prompt";
@@ -70,6 +71,7 @@ import type {
7071
ThreadGoal,
7172
ThreadItem,
7273
ThreadItemEntry,
74+
Turn,
7375
TurnStatus,
7476
UserInput
7577
} from "./app-server/v2";
@@ -345,9 +347,7 @@ function codexRunningTurnId(sessionState: SessionState, turnId: string): string
345347
}
346348

347349
/**
348-
* Simplified `turn/completed` status to v1 stop reason mapping for a turn no `session/prompt`
349-
* owns, where there is no richer terminal-failure handling to consult: only an interruption
350-
* counts as cancelled, a failure still ends the turn normally.
350+
* Legacy stop reason for an unowned runtime turn. Standard v2 failure details are supplied separately.
351351
*/
352352
function stopReasonForUnownedTurn(status: TurnStatus): acp.StopReason {
353353
switch (status) {
@@ -1417,7 +1417,7 @@ export class CodexAcpServer {
14171417
// Codex-started turns carry no `userMessage` item; `handleSessionScopedNotification`
14181418
// already drops that item type, so nothing is synthesized here.
14191419
await baselineEventHandler.handleSessionScopedNotification(event);
1420-
await this.trackCodexTurnCompletion(sessionState, event);
1420+
await this.trackCodexTurnCompletion(sessionState, event, baselineEventHandler);
14211421
},
14221422
DENY_ALL_APPROVALS,
14231423
DENY_ALL_ELICITATIONS,
@@ -1484,16 +1484,24 @@ export class CodexAcpServer {
14841484
* owns, after the notification's own content has already been handled so `idle`
14851485
* stays the last thing sent for the turn.
14861486
*/
1487-
private async trackCodexTurnCompletion(sessionState: SessionState, event: ServerNotification): Promise<void> {
1487+
private async trackCodexTurnCompletion(
1488+
sessionState: SessionState,
1489+
event: ServerNotification,
1490+
eventHandler: CodexEventHandler,
1491+
): Promise<void> {
14881492
if (event.method !== "turn/completed" || event.params.threadId !== sessionState.sessionId) {
14891493
return;
14901494
}
14911495
if (sessionState.codexReportedRunningTurnId === event.params.turn.id) {
14921496
sessionState.codexReportedRunningTurnId = null;
14931497
}
1498+
const failure = eventHandler.getTurnFailure(event.params.turn);
1499+
if (this.protocolVersion === 2 && failure !== null && !this.v2PromptsInFlight.has(sessionState.sessionId)) {
1500+
await this.finishOutstandingToolCalls(sessionState);
1501+
}
14941502
await this.reportCodexTurnState(sessionState.sessionId, toV2IdleState({
14951503
stopReason: stopReasonForUnownedTurn(event.params.turn.status),
1496-
}));
1504+
}, failure));
14971505
}
14981506

14991507
/**
@@ -4099,6 +4107,7 @@ export class CodexAcpServer {
40994107
const promptKind = this.availableCommands.classifyPrompt(request.prompt);
41004108
const messageId = randomUUID();
41014109
const session = new ACPSessionConnection(this.connection, sessionId);
4110+
let failure: acpV2.Error | null = null;
41024111
this.v2PromptsInFlight.add(sessionId);
41034112
const sendState = async (state: acpV2.StateUpdate) => {
41044113
try {
@@ -4114,6 +4123,16 @@ export class CodexAcpServer {
41144123
logger.error(`Failed to report foreground work for session ${sessionId}`, error);
41154124
}
41164125
};
4126+
const sendIdle = async (response: acp.PromptResponse) => {
4127+
if (failure !== null && response.stopReason !== "cancelled") {
4128+
try {
4129+
await this.finishOutstandingToolCalls(sessionState);
4130+
} catch (error) {
4131+
logger.error(`Failed to close tools for failed work in session ${sessionId}`, error);
4132+
}
4133+
}
4134+
await sendState(toV2IdleState(response, failure));
4135+
};
41174136
return await new Promise<acpV2.PromptResponse>((resolve, reject) => {
41184137
let running: Promise<void> | null = null;
41194138
let inserted = false;
@@ -4166,7 +4185,6 @@ export class CodexAcpServer {
41664185
const run = async () => {
41674186
if (promptKind.kind === "localCommand") {
41684187
await onInserted();
4169-
return await this.prompt(request, undefined, undefined, undefined, reservation);
41704188
}
41714189
return await this.prompt(request, undefined, undefined, {
41724190
clientUserMessageId: messageId,
@@ -4188,6 +4206,7 @@ export class CodexAcpServer {
41884206
// An accepted Codex turn can run hooks before inserting user input.
41894207
running = startWork();
41904208
},
4209+
onFailure: (error) => { failure ??= error; },
41914210
}, reservation);
41924211
};
41934212
run().then(
@@ -4200,13 +4219,15 @@ export class CodexAcpServer {
42004219
// never inserted, so it is dropped with `-32800` like a queued prompt.
42014220
reject(response.stopReason === "cancelled"
42024221
? RequestError.requestCancelled(undefined, notInsertedMessage)
4203-
: RequestError.internalError(undefined, notInsertedMessage));
4222+
: failure !== null
4223+
? new RequestError(failure.code, failure.message)
4224+
: RequestError.internalError(undefined, notInsertedMessage));
42044225
// Codex dropped the steered input before the adopted turn ended: that
42054226
// turn's `running` still needs exactly one matching `idle`, and nothing
42064227
// else will send it now that this prompt is no longer in flight.
42074228
if (turnWasAdopted || startedTurn !== null) {
42084229
if (running !== null) await running;
4209-
await sendState(toV2IdleState(response));
4230+
await sendIdle(response);
42104231
}
42114232
return;
42124233
}
@@ -4216,17 +4237,18 @@ export class CodexAcpServer {
42164237
// The session takes the next prompt before `idle` goes out, so a client that
42174238
// prompts again as soon as it sees `idle` is not rejected as overlapping.
42184239
// What v1 would have answered with ends the v2 turn.
4219-
await sendState(toV2IdleState(response));
4240+
await sendIdle(response);
42204241
},
42214242
async (error: unknown) => {
4243+
failure ??= toV2Failure(error);
42224244
if (!inserted) {
42234245
this.v2PromptsInFlight.delete(sessionId);
42244246
reject(error);
42254247
const runtimeLost = this.getExitCode() !== null
42264248
|| this.recovery !== null && !this.recovery.isReady();
42274249
if (turnWasAdopted || startedTurn !== null || runtimeLost) {
42284250
if (running !== null) await running;
4229-
await sendState(toV2IdleState(this.failedPromptResponse(sessionId)));
4251+
await sendIdle(this.failedPromptResponse(sessionId));
42304252
}
42314253
return;
42324254
}
@@ -4239,15 +4261,15 @@ export class CodexAcpServer {
42394261
if (!failureWasShownAsMessage(error)) {
42404262
try {
42414263
await session.update(createAgentTextMessageChunk(postInsertionFailureText(
4242-
error,
4264+
new Error(failure.message),
42434265
promptKind.kind === "localCommand" ? promptKind.name : undefined,
42444266
)));
42454267
} catch (sendError) {
42464268
logger.error(`Failed to send the prompt failure for session ${sessionId}`, sendError);
42474269
}
42484270
}
42494271
this.v2PromptsInFlight.delete(sessionId);
4250-
await sendState(toV2IdleState(this.failedPromptResponse(sessionId)));
4272+
await sendIdle(this.failedPromptResponse(sessionId));
42514273
},
42524274
).finally(() => {
42534275
signal?.removeEventListener("abort", dropPendingRequest);
@@ -4320,6 +4342,10 @@ export class CodexAcpServer {
43204342
};
43214343
const disposePromptRequestCancellation = this.observePromptRequestCancellation(signal, sessionState, activePrompt);
43224344
let eventHandler: CodexEventHandler | null = null;
4345+
const reportTurnFailure = (turn: Turn) => {
4346+
const error = eventHandler?.getTurnFailure(turn);
4347+
if (error) insertion?.onFailure?.(error);
4348+
};
43234349
let promptNotificationsActive = true;
43244350
// The app-server of this prompt. A restart during the prompt must not mix clients, see AppServerRecovery.
43254351
let promptClient = this.codexAcpClient;
@@ -4451,7 +4477,7 @@ export class CodexAcpServer {
44514477
await observeInteraction(event);
44524478
if (!promptNotificationsActive) {
44534479
await promptEventHandler.handleSessionScopedNotification(event);
4454-
await this.trackCodexTurnCompletion(sessionState, event);
4480+
await this.trackCodexTurnCompletion(sessionState, event, promptEventHandler);
44554481
return;
44564482
}
44574483
const completesActiveTurn = event.method === "turn/completed"
@@ -4463,7 +4489,7 @@ export class CodexAcpServer {
44634489
// the causal boundary so a queued late error cannot enter the completed turn's buffer.
44644490
promptNotificationsActive = false;
44654491
}
4466-
await this.trackCodexTurnCompletion(sessionState, event);
4492+
await this.trackCodexTurnCompletion(sessionState, event, promptEventHandler);
44674493
},
44684494
approvalHandler,
44694495
elicitationHandler,
@@ -4532,6 +4558,10 @@ export class CodexAcpServer {
45324558
await eventHandler.flushPendingErrorsAsSessionScoped();
45334559
if (commandResult.turnCompleted) {
45344560
await eventHandler.handleFailedTurn(commandResult.turnCompleted.turn);
4561+
if (insertion && activePrompt.clientCancelled && commandResult.turnCompleted.turn.status === "failed") {
4562+
return cancelledPromptResponse();
4563+
}
4564+
reportTurnFailure(commandResult.turnCompleted.turn);
45354565
}
45364566
if (commandResult.turnCompleted?.turn.status === "interrupted") {
45374567
return cancelledPromptResponse();
@@ -4648,6 +4678,10 @@ export class CodexAcpServer {
46484678
}
46494679
await eventHandler.flushPendingErrors();
46504680
await eventHandler.handleFailedTurn(turnCompleted.turn);
4681+
if (insertion && activePrompt.clientCancelled && turnCompleted.turn.status === "failed") {
4682+
return cancelledPromptResponse();
4683+
}
4684+
reportTurnFailure(turnCompleted.turn);
46514685
promptNotificationsActive = false;
46524686

46534687
if (turnCompleted.turn.status === "interrupted") {
@@ -4764,6 +4798,10 @@ export class CodexAcpServer {
47644798
}
47654799
await eventHandler.flushPendingErrors();
47664800
await eventHandler.handleFailedTurn(turnCompleted.turn);
4801+
if (insertion && activePrompt.clientCancelled && turnCompleted.turn.status === "failed") {
4802+
return cancelledPromptResponse();
4803+
}
4804+
reportTurnFailure(turnCompleted.turn);
47674805
promptNotificationsActive = false;
47684806
if (turnCompleted.turn.status === "interrupted") {
47694807
await eventHandler.flushPendingPlanUpdates();
@@ -4833,7 +4871,7 @@ export class CodexAcpServer {
48334871
const lostAppServer = this.recovery !== null
48344872
&& err instanceof RequestError
48354873
&& err.code === CODEX_PROCESS_EXITED_ERROR_CODE;
4836-
if (activePrompt.signal.aborted || this.sessionIsClosing(params.sessionId)) {
4874+
if (activePrompt.signal.aborted || this.sessionIsClosing(params.sessionId) || insertion && activePrompt.clientCancelled) {
48374875
if (lostAppServer) await this.finishPromptAfterAppServerLoss(sessionState, eventHandler, promptClient, promptElicitations, "cancelled");
48384876
return cancelledPromptResponse();
48394877
}
@@ -4853,6 +4891,7 @@ export class CodexAcpServer {
48534891
return cancelledPromptResponse();
48544892
}
48554893
}
4894+
insertion?.onFailure?.(toV2Failure(err));
48564895
const isUnexpectedFailure = !(err instanceof RequestError);
48574896
if (eventHandler !== null
48584897
&& clientSupportsTypedSessionFailures(this.clientCapabilities)

0 commit comments

Comments
 (0)