Skip to content

Commit 986955a

Browse files
authored
fix: synchronize ACP v2 foreground state (spacering-net#650)
## Summary Synchronize ACP v2 foreground state independently of prompt acceptance. This PR is based directly on main after spacering-net#649 merged. - Send a current `state_update` after any requested history replay and before every successful `session/resume` response, including omitted/null `replayFrom`. - Restore active runtime state on resume and serialize snapshots with live state transitions so backpressure cannot send a stale snapshot after newer state. Idle snapshots repeat neither completion usage nor failure metadata. - Report `running` when Codex accepts or reports foreground work, including pre-insertion hooks. Prompt success still waits for the user-message echo. Work that starts but never inserts input still ends with an idle boundary, including transport loss before the turn-start acknowledgment. - Replace per-request requires_action/running brackets with session-owned pending interactions and runnable items. Parallel work can remain running; answering one wait does not blindly resume a still-blocked workflow. - Use actual runtime starts, streamed progress, and active status with no wait flags as positive progress evidence. Nonempty Codex wait flags count requests and are not by themselves evidence that the whole workflow is blocked. - Do not resume withdrawn interactions, and invalidate old waits at idle/close so late answers cannot revive an ended turn or alter a newer turn. - Update the developer contract, snapshots, and focused regression coverage. Add a real-process v2 integration test for early running, prompt insertion, and all three resume modes. ACP v1 behavior and the existing AIR subagent wire contract are preserved. Standard structured failure reporting, initial commands, system prompts, and standard subagent sessions remain separate work packages; failure stop reasons are intentionally unchanged here. ## Specification and runtime evidence Checked ACP specification revision `4034a8c`: - [Current-state synchronization](https://github.com/agentclientprotocol/agent-client-protocol/blob/4034a8cbc99bc48fd4579f70a3407b07907e0eb6/docs/protocol/v2/session-setup.mdx#L313-L329) - [Foreground state and user-action semantics](https://github.com/agentclientprotocol/agent-client-protocol/blob/4034a8cbc99bc48fd4579f70a3407b07907e0eb6/docs/protocol/v2/prompt-lifecycle.mdx#L843-L938) Codex runtime status semantics checked against bundled runtime `0.160.1` (`codex-rs/app-server/src/thread_status.rs`): wait flags count pending interactions; active status with no wait flags implies the runtime is running. ## Validation - `npm run typecheck` — passed, including examples. - `npm test` — 1,802 passed; 36 skipped. - `npm run build` — passed. - `npm run test:integration` — 5 passed, including the new real-process ACP v2 test with a local mock model. - `npm run codex-test` — live-model smoke test from an empty scratch directory completed with `end_turn`. - `git diff --check` — passed. The full live-model E2E suite was not run.
1 parent c8fd166 commit 986955a

44 files changed

Lines changed: 942 additions & 261 deletions

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

‎readme-dev.md‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -161,12 +161,17 @@ schema alone:
161161
(`_meta.quota`) from the idle `state_update` that ends the turn, not from a separate error
162162
channel. The same `_meta` v1 attaches to `PromptResponse` is copied onto the v2 idle update.
163163
- Custom session updates are renamed on v2: `subagent_spawned`/`subagent_state_update` become a
164-
single `_subagent_update` (gated on the AIR `nativeSubagentSessions` capability; it will rename to
165-
`subagent_update` once the subagent RFD, spec PR #1992, lands), and
164+
single `_subagent_update` (gated on the AIR `nativeSubagentSessions` capability; this contract is
165+
retained until the adapter migrates to the standard subagent model), and
166166
`async_task_spawned`/`async_task_state_update` become `_async_task_spawned`/`_async_task_state_update`
167167
(gated on the AIR `asyncTasks` capability). Payload shapes are unchanged.
168-
- The SDK client treats any `idle` state update as the stop of whichever prompt is pending, not just
169-
one paired 1:1 with a `session/prompt` call.
168+
- Foreground state is independent of prompt acceptance. `running` can precede the user-message
169+
echo, including while Codex runs hooks. A successful prompt response still waits for insertion.
170+
- Every successful `session/resume` sends a current-state snapshot after any replay and before its
171+
response. An idle snapshot is not another completed turn and repeats no usage or failure metadata.
172+
- Permission and elicitation requests affect aggregate foreground state: another runnable item keeps
173+
the session running, and answering one wait does not resume work that remains blocked. Withdrawing
174+
a request on cancellation does not resume the workflow; late answers cannot revive an ended turn.
170175
- A steer (`_session/steering`) shows up as a `user_message` once it actually lands in the turn.
171176
- On cancel, any permission or elicitation request still waiting on a client response is withdrawn
172177
with `$/cancel_request`, not just the underlying Codex turn.

‎src/ACPSessionConnection.ts‎

Lines changed: 57 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import {
88
import {toV2SessionUpdates} from "./AcpV2SessionUpdate";
99
import {toV1RequestPermissionResponse, toV2RequestPermissionRequest} from "./AcpV2Permissions";
1010
import {elicitationSessionId, toV1CreateElicitationResponse, toV2CreateElicitationRequest} from "./AcpV2Elicitation";
11-
import {logger} from "./Logger";
11+
import {AcpV2ForegroundState} from "./AcpV2ForegroundState";
1212

1313
export type AcpClientConnection = Pick<acp.AgentContext, "notify" | "request">;
1414

@@ -20,44 +20,27 @@ export type AcpV2ClientConnection = Pick<acpV2.AgentContext, "notify" | "request
2020
export class AcpV2Connection {
2121
readonly client: AcpV2ClientConnection;
2222
private isTurnRunning: (sessionId: string) => boolean = () => true;
23+
private readonly foreground = new Map<string, AcpV2ForegroundState>();
2324

2425
constructor(client: AcpV2ClientConnection) {
2526
this.client = client;
2627
}
2728

28-
/**
29-
* Lets the owning server report whether a turn is still running for a session, so the
30-
* `requires_action`/`running` bracket below is only applied around requests that actually
31-
* interrupt foreground work: a request sent while idle must not send `requires_action` (there
32-
* is no foreground work to block), and a request that outlives its turn must not resurrect
33-
* `running` after the turn already went `idle` on its own.
34-
* Defaults to always running, so callers that never wire this up keep the old behavior.
35-
*/
29+
/** Identifies foreground interactions; background requests do not affect session state. */
3630
setTurnRunningCheck(check: (sessionId: string) => boolean): void {
3731
this.isTurnRunning = check;
3832
}
3933

40-
/**
41-
* Runs `sendRequest`, bracketed with `requires_action`/`running` `state_update`s only if a
42-
* turn is running for `sessionId` at the moment the request is about to be sent. If idle,
43-
* `sendRequest` runs unbracketed: sending `requires_action` while there is no foreground work
44-
* to block would contradict the state's definition, and the client is already correctly idle.
45-
* If busy, `requires_action` is sent first, and `running` follows in `finally` only if a turn
46-
* is still running by then -- a turn that went `idle` on its own while the request was pending
47-
* must not have that `idle` undone.
48-
*/
49-
private async withStateBracket<T>(sessionId: string, sendRequest: () => Promise<T>): Promise<T> {
34+
private async withUserAction<T>(
35+
sessionId: string,
36+
itemId: string | undefined,
37+
sendRequest: () => Promise<T>,
38+
signal?: AbortSignal,
39+
): Promise<T> {
5040
if (!this.isTurnRunning(sessionId)) {
5141
return sendRequest();
5242
}
53-
await this.sendState(sessionId, {state: "requires_action"});
54-
try {
55-
return await sendRequest();
56-
} finally {
57-
if (this.isTurnRunning(sessionId)) {
58-
await this.sendState(sessionId, {state: "running"});
59-
}
60-
}
43+
return this.foregroundFor(sessionId).withUserAction(itemId, sendRequest, signal);
6144
}
6245

6346
/**
@@ -72,10 +55,47 @@ export class AcpV2Connection {
7255

7356
/** Sends a `state_update`, which only exists on v2 and so needs no rendering. */
7457
async updateState(sessionId: string, state: acpV2.StateUpdate): Promise<void> {
75-
await this.client.notify(acpV2.methods.client.session.update, {
76-
sessionId,
77-
update: {sessionUpdate: "state_update", ...state},
78-
});
58+
await this.foregroundFor(sessionId).update(state);
59+
}
60+
61+
startForegroundWork(sessionId: string): Promise<void> {
62+
return this.foregroundFor(sessionId).start();
63+
}
64+
65+
snapshotForegroundState(sessionId: string): Promise<void> {
66+
return this.foregroundFor(sessionId).snapshot();
67+
}
68+
69+
startForegroundItem(sessionId: string, itemId: string): Promise<void> {
70+
return this.foregroundFor(sessionId).startItem(itemId);
71+
}
72+
73+
completeForegroundItem(sessionId: string, itemId: string): Promise<void> {
74+
return this.foregroundFor(sessionId).completeItem(itemId);
75+
}
76+
77+
restoreForegroundState(sessionId: string, running: boolean): void {
78+
this.foregroundFor(sessionId, running);
79+
}
80+
81+
forgetSession(sessionId: string): void {
82+
this.foreground.get(sessionId)?.dispose();
83+
this.foreground.delete(sessionId);
84+
}
85+
86+
private foregroundFor(sessionId: string, running = false): AcpV2ForegroundState {
87+
let foreground = this.foreground.get(sessionId);
88+
if (!foreground) {
89+
foreground = new AcpV2ForegroundState(
90+
state => this.client.notify(acpV2.methods.client.session.update, {
91+
sessionId,
92+
update: {sessionUpdate: "state_update", ...state},
93+
}),
94+
running,
95+
);
96+
this.foreground.set(sessionId, foreground);
97+
}
98+
return foreground;
7999
}
80100

81101
/**
@@ -103,43 +123,22 @@ export class AcpV2Connection {
103123
});
104124
}
105125

106-
/** `updateState`, with send failures caught and logged instead of propagated. */
107-
private async sendState(sessionId: string, state: acpV2.StateUpdate): Promise<void> {
108-
try {
109-
await this.updateState(sessionId, state);
110-
} catch (error) {
111-
logger.error(`Failed to send the '${state.state}' state for session ${sessionId}`, error);
112-
}
113-
}
114-
115-
/**
116-
* Sends `session/request_permission`, rendered in the v2 wire shape by
117-
* `toV2RequestPermissionRequest`. Per the spec, brackets the request with `requires_action`/
118-
* `running` `state_update`s -- but only while a turn is actually running (see
119-
* `withStateBracket`); a permission request while idle (e.g. an out-of-turn MCP elicitation's
120-
* fallback) is sent with no state update at all.
121-
*/
126+
/** Sends permission requests without mistaking one pending request for a blocked session. */
122127
async requestPermission(
123128
request: acp.RequestPermissionRequest,
124129
options?: acp.SendRequestOptions,
125130
): Promise<acp.RequestPermissionResponse> {
126-
return this.withStateBracket(request.sessionId, async () => {
131+
return this.withUserAction(request.sessionId, request.toolCall.toolCallId, async () => {
127132
const response = await this.client.request(
128133
acpV2.methods.client.session.requestPermission,
129134
toV2RequestPermissionRequest(request),
130135
options,
131136
);
132137
return toV1RequestPermissionResponse(response);
133-
});
138+
}, options?.cancellationSignal);
134139
}
135140

136-
/**
137-
* Sends `elicitation/create` (wire-identical request/response between versions, per
138-
* `AcpV2Elicitation`). Brackets it with `requires_action`/`running` `state_update`s like
139-
* `requestPermission()` (see `withStateBracket`), but only when the elicitation is
140-
* session-scoped: a request-scoped elicitation (e.g. device-code login, sent before any
141-
* session exists) has no session to attach a state update to, so it is sent unbracketed.
142-
*/
141+
/** Request-scoped login elicitations and idle background elicitations never change state. */
143142
async createElicitation(
144143
request: acp.CreateElicitationRequest,
145144
options?: acp.SendRequestOptions,
@@ -156,7 +155,8 @@ export class AcpV2Connection {
156155
if (sessionId === undefined) {
157156
return sendRequest();
158157
}
159-
return this.withStateBracket(sessionId, sendRequest);
158+
const toolCallId = "toolCallId" in request && typeof request.toolCallId === "string" ? request.toolCallId : undefined;
159+
return this.withUserAction(sessionId, toolCallId, sendRequest, options?.cancellationSignal);
160160
}
161161

162162
/** Sends `elicitation/complete`, wire-identical between versions. */

‎src/AcpV2ForegroundState.ts‎

Lines changed: 104 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,104 @@
1+
import type * as acpV2 from "@agentclientprotocol/sdk/experimental/v2/unstable";
2+
import {logger} from "./Logger";
3+
4+
type Activity = "idle" | "running" | "requires_action";
5+
6+
/** Session state and its wire order are independent of prompt acceptance and individual requests. */
7+
export class AcpV2ForegroundState {
8+
private activity: Activity;
9+
private readonly items = new Set<string>();
10+
private readonly actions = new Map<symbol, string | undefined>();
11+
private sending: Promise<void> = Promise.resolve();
12+
13+
constructor(
14+
private readonly notify: (state: acpV2.StateUpdate) => Promise<void>,
15+
running = false,
16+
) {
17+
this.activity = running ? "running" : "idle";
18+
}
19+
20+
async update(state: acpV2.StateUpdate): Promise<void> {
21+
if (state.state === "idle") {
22+
this.items.clear();
23+
this.actions.clear();
24+
if (this.activity === "idle") return this.sending;
25+
this.activity = "idle";
26+
} else if (state.state === "running" || state.state === "requires_action") {
27+
if (this.activity === state.state) return this.sending;
28+
this.activity = state.state;
29+
}
30+
await this.publish(state);
31+
}
32+
33+
/** A newly accepted runtime turn or local command is evidence of foreground work. */
34+
async start(): Promise<void> {
35+
if (this.activity === "idle" || this.actions.size === 0) await this.update({state: "running"});
36+
}
37+
38+
/** Snapshots never repeat completion usage or failure metadata. */
39+
snapshot(): Promise<void> {
40+
switch (this.activity) {
41+
case "idle":
42+
return this.publish({state: "idle"});
43+
case "running":
44+
return this.publish({state: "running"});
45+
case "requires_action":
46+
return this.publish({state: "requires_action"});
47+
}
48+
}
49+
50+
async startItem(itemId: string): Promise<void> {
51+
if (this.activity === "idle") return;
52+
this.items.add(itemId);
53+
await this.reportActionState();
54+
}
55+
56+
async completeItem(itemId: string): Promise<void> {
57+
this.items.delete(itemId);
58+
if (this.activity !== "idle" && this.actions.size > 0) await this.reportActionState();
59+
}
60+
61+
async withUserAction<T>(itemId: string | undefined, request: () => Promise<T>, signal?: AbortSignal): Promise<T> {
62+
if (signal?.aborted) return request();
63+
const action = Symbol();
64+
this.actions.set(action, itemId);
65+
// The caller only registers actions that belong to active foreground work.
66+
if (this.activity === "idle") this.activity = "running";
67+
try {
68+
await this.reportActionStateSafely();
69+
return await request();
70+
} finally {
71+
// An idle boundary invalidates old waits, even if a new turn has already started.
72+
if (this.actions.delete(action) && !signal?.aborted) await this.reportActionStateSafely();
73+
}
74+
}
75+
76+
dispose(): void {
77+
this.activity = "idle";
78+
this.items.clear();
79+
this.actions.clear();
80+
}
81+
82+
private reportActionState(): Promise<void> {
83+
const blockedItems = new Set(this.actions.values());
84+
const runnable = [...this.items].some(itemId => !blockedItems.has(itemId));
85+
if (runnable) return this.update({state: "running"});
86+
if (this.actions.size > 0) return this.update({state: "requires_action"});
87+
// An answer alone does not establish that foreground progress has resumed.
88+
return this.sending;
89+
}
90+
91+
private async reportActionStateSafely(): Promise<void> {
92+
try {
93+
await this.reportActionState();
94+
} catch (error) {
95+
logger.error("Failed to report foreground user-action state", error);
96+
}
97+
}
98+
99+
private publish(state: acpV2.StateUpdate): Promise<void> {
100+
const sent = this.sending.then(() => this.notify(state));
101+
this.sending = sent.catch(() => {});
102+
return sent;
103+
}
104+
}

‎src/AcpV2SessionUpdate.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -116,10 +116,8 @@ export function toV2SessionUpdate(update: AcpSessionUpdate): acpV2.SessionUpdate
116116
...(content !== undefined ? {content: content && content.map(toV2ToolCallContent)} : {}),
117117
};
118118
}
119-
// Subagent RFD (spec PR #1992, unmerged) uses a single upsert-style update. Sent under a
120-
// `_`-prefixed tag because the RFD's `subagent_update` isn't in the v2 unstable schema yet;
121-
// rename to `subagent_update` once it lands there. The AIR `nativeSubagentSessions` gate
122-
// already ran upstream (whatever created this update), so no gate check is needed here.
119+
// Preserve AIR's existing subagent contract. The standard schema uses different fields
120+
// and work states; that migration is separate. The AIR capability gate already ran upstream.
123121
case "subagent_spawned": {
124122
const {subagentSessionId, name, task, _meta} = update;
125123
return {

‎src/CodexAcpClient.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -701,6 +701,7 @@ export class CodexAcpClient {
701701
models: codexModels,
702702
// Codex sends no thread/settings/updated on a resume. The response holds the mode.
703703
collaborationMode: response.collaborationMode ?? this.getCollaborationMode(response.thread.id),
704+
threadStatus: response.thread.status,
704705
modelProvider: response.modelProvider,
705706
currentServiceTier: response.serviceTier as ServiceTier ?? null,
706707
additionalDirectories,

0 commit comments

Comments
 (0)