Skip to content

Commit 91b31f2

Browse files
committed
feat: session/close support
1 parent f640d4c commit 91b31f2

11 files changed

Lines changed: 2301 additions & 158 deletions

ACP_SPEC_GAP_ANALYSIS.md

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,95 @@
1+
# ACP Spec Gap Analysis for codex-acp
2+
3+
Date: 2026-05-22
4+
5+
## Scope
6+
7+
This compares the current `codex-acp` worktree against:
8+
9+
- Current stable ACP protocol docs: https://agentclientprotocol.com/protocol/schema
10+
- Current TypeScript SDK reference: https://agentclientprotocol.github.io/typescript-sdk/interfaces/Agent.html
11+
- Current ACP RFD status index: https://agentclientprotocol.com/rfds/updates
12+
- Installed adapter SDK: `@agentclientprotocol/sdk` 0.22.1 from `node_modules/@agentclientprotocol/sdk/package.json`
13+
14+
"Missing" means the adapter does not implement or advertise the feature. "Partial" means the adapter has some behavior but not enough to claim full protocol support.
15+
16+
## Current Adapter Surface
17+
18+
The adapter currently advertises these ACP capabilities in `CodexAcpServer.initialize()`:
19+
20+
- `agentInfo`
21+
- `auth.logout`
22+
- `loadSession`
23+
- `promptCapabilities.image`
24+
- `sessionCapabilities.resume`
25+
- `sessionCapabilities.list`
26+
- `sessionCapabilities.close`
27+
- `mcpCapabilities.http = true`
28+
- `mcpCapabilities.sse = false`
29+
30+
Implemented agent methods in the current server:
31+
32+
- Baseline: `initialize`, `authenticate`, `session/new`, `session/prompt`, `session/cancel`
33+
- Stable optional: `session/load`, `session/list`, `session/resume`, `session/close`, `session/set_mode`, `session/set_config_option`
34+
- Unstable optional: `unstable_logout`, `unstable_setSessionModel`
35+
- Extensions: `authentication/status`, `authentication/logout`
36+
37+
Codex app-server methods already wrapped by the adapter include `thread/start`, `thread/resume`, `thread/unsubscribe`, `thread/list`, `thread/loaded/list`, `thread/read`, `turn/start`, `turn/interrupt`, `model/list`, `skills/list`, `account/login/start`, `account/logout`, `account/read`, `config/read`, and `mcpServerStatus/list`.
38+
39+
## Stable Gaps
40+
41+
| Feature | Status | Evidence | Team handoff |
42+
| --- | --- | --- | --- |
43+
| `session_info_update` | Missing stable notification | `session/list` is implemented, but `CodexEventHandler` ignores `thread/name/updated`, `thread/status/changed`, `thread/archived`, `thread/unarchived`, and `thread/closed`. | Map Codex thread metadata notifications to ACP `session/update` with `sessionUpdate: "session_info_update"` where possible. |
44+
| Prompt audio input | Missing optional stable prompt capability | `promptCapabilities.audio` is not advertised and `buildPromptItems()` drops `ContentBlock.audio`. | Only add if Codex app-server can accept audio. Otherwise keep unadvertised. |
45+
| Prompt embedded resources | Partial and unadvertised | `promptCapabilities.embeddedContext` is not advertised. Text resources are converted to text, blob resources are dropped, and resource handling is not standard capability-gated support. | Decide whether to support `embeddedContext`. If yes, advertise it and map both text and binary resource contents deliberately. |
46+
| `session/load` history completeness | Partial for an advertised capability | `loadSession` streams history, but history mapping intentionally drops several Codex item types such as hook prompts and image generation; some local images become text links. | Audit every `ThreadItem` variant and either map it to ACP output or document why it is intentionally invisible. |
47+
| `session/list` filter semantics | Partial compliance | ACP says `cwd` is an absolute-path filter. Current code accepts non-absolute cwd, compares basenames, and may ignore the filter. | Validate `cwd` according to the spec. Prefer `invalid_params` for malformed filters instead of basename fallback. |
48+
| MCP SSE transport | Missing optional stable transport | Adapter advertises `sse: false` and throws if an SSE MCP server is supplied. | Low priority because MCP SSE is deprecated; only implement if a target client still needs it. |
49+
| Client-mediated terminal and filesystem methods | Not used by design | ACP lets agents call client `terminal/*` and `fs/*` methods when the client advertises them. This adapter delegates execution and filesystem access to Codex/app-server instead. | Not a conformance blocker unless the product wants client-mediated execution or client-enforced filesystem access. |
50+
51+
## Unstable, Preview, And Draft Gaps
52+
53+
| Feature | Status | Evidence | Team handoff |
54+
| --- | --- | --- | --- |
55+
| `session/delete` and `sessionCapabilities.delete` | Missing draft feature | Installed SDK exposes `unstable_deleteSession`, but the adapter does not implement or advertise it. App-server types include `thread/archive`, which may be enough for soft delete semantics. | Add `unstable_deleteSession`. Map to Codex `thread/archive` if archived threads are omitted from `thread/list`, and make repeated deletes idempotent. |
56+
| `session/fork` and `sessionCapabilities.fork` | Missing draft feature | Installed SDK has `unstable_forkSession?`; adapter does not implement or advertise it. App-server generated types include `thread/fork`, but `CodexAppServerClient` does not wrap it. | Add `threadFork` wrapper, create a new `SessionState` for the fork, return model/mode/config state, and decide whether forked sessions should stream history. |
57+
| `additionalDirectories` | Missing draft feature | Adapter does not advertise `sessionCapabilities.additionalDirectories` and does not read the standardized `request.additionalDirectories` field. It only reads `_meta.additionalRoots` for skills. | Implement the standard field on `session/new`, `session/load`, `session/resume`, and `session/fork`; validate absolute paths; pass roots to Codex config/skills/sandbox as appropriate; include roots in `session/list` only when authoritative. Resolve current RFD/schema differences around list filtering before coding. |
58+
| Configurable providers (`providers/list`, `providers/set`, `providers/disable`) | Missing draft feature | Adapter uses custom gateway auth and env/config knobs, but does not advertise `providers` or implement ACP provider methods. | Map ACP providers onto Codex model provider configuration. Use `config/read`, model provider capabilities, and config write APIs if app-server can safely support this. |
59+
| Logout method stabilization | Mostly present, needs verification | Adapter advertises `auth.logout` and implements `unstable_logout`; logout RFD is currently Preview. | Re-check the current SDK capability and method shape during SDK update; remove custom `authentication/logout` extension if the standard method fully covers it. |
60+
| Structured elicitation (`elicitation/create`) | Partial, not standard | Adapter handles Codex `mcpServer/elicitation/request` by translating to ACP `session/request_permission`. It does not send ACP `elicitation/create`, does not use client `elicitation` capabilities, and does not return form content. | Implement real ACP elicitation for generic form/url requests. Keep permission-based MCP tool approval separate from structured user input. |
61+
| Next Edit Suggestions (NES) | Missing draft feature | No `agentCapabilities.nes`, no `unstable_startNes`, `unstable_suggestNes`, `unstable_closeNes`, document notifications, accept/reject handling, or `positionEncoding` negotiation. | Requires a Codex backend capability decision first. If Codex has no NES engine, do not advertise it. |
62+
| MCP-over-ACP transport | Missing draft feature | Adapter supports stdio and HTTP MCP server declarations, rejects SSE, and has no `mcpCapabilities.acp` or `mcp/connect`, `mcp/message`, `mcp/disconnect` handling. | Add native ACP MCP transport or a bridge that rewrites ACP MCP servers to a Codex-supported transport. |
63+
| Protocol-level `$/cancel_request` | Missing/unknown | Adapter implements ACP `session/cancel` for active prompt turns only. There is no adapter-level handling for cancelling arbitrary in-flight JSON-RPC requests. | Revisit after SDK update. Add support if the SDK exposes protocol-level cancellation hooks for long-running requests. |
64+
| Session usage/context status | Partial | Adapter sends `usage_update` and returns prompt `usage`, but does not report cost and does not push usage on `session/new`, `session/load`, or `session/resume`. It also sends custom `_meta.quota`. | Align with the RFD fields, add lifecycle-time usage updates when Codex exposes the data, and decide whether custom quota metadata should remain. |
65+
| Deleted-file diff flag | Missing draft field | File deletions are represented with `newText: ""` plus custom `_meta.kind = "delete"`, not a standard `deleted: true` field. | When the schema exposes the draft `Diff.deleted` flag, emit it for deleted files and keep fallback metadata only for older clients if needed. |
66+
| Boolean config options | Missing/partial draft support | Adapter exposes Fast mode as a select option. `setSessionConfigOption()` explicitly rejects boolean-shaped requests. | If boolean config options are adopted, either convert Fast mode to boolean or support both select and boolean shapes. |
67+
| Standard model config category | Missing draft refinement | Fast mode uses `category: "fast-mode"` and model selection is exposed through `unstable_setSessionModel` rather than a config option category. | Revisit once the model config category RFD stabilizes. Use standard categories for model, thought level, and speed tier controls where applicable. |
68+
| Standard auth method variants | Partial | Adapter uses default agent-handled auth methods plus custom `_meta` for API key/gateway. It does not advertise `env_var` or `terminal` auth, even though a `login` CLI path exists. | Optional. Consider standard `env_var` for `OPENAI_API_KEY` and `terminal` auth for the `codex-acp login` flow if target clients support them. |
69+
| Meta propagation conventions | Not audited | Adapter uses `_meta` for `quota`, terminal output, MCP flags, and custom `additionalRoots`, but does not consistently propagate or namespace spec-adjacent metadata. | Audit `_meta` keys, namespace custom keys, and replace `_meta.additionalRoots` with standard `additionalDirectories`. |
70+
| Message IDs | Missing v2 draft field | Agent/user/thought chunks and prompt responses do not include stable message identifiers. | Defer until the field lands in the SDK schema, then preserve client `messageId`, return `userMessageId`, and emit stable IDs on message/thought chunks, especially during `session/load`. |
71+
| Agent telemetry export | Missing draft guidance | No ACP telemetry export behavior is implemented. | Treat as a policy/design task, not a wire-protocol blocker, until the RFD becomes concrete enough for implementation. |
72+
| Proxy chains | Missing draft guidance | No proxy-chain capability or metadata handling is present. | Defer unless this adapter is expected to run behind an ACP proxy that needs explicit chain metadata. |
73+
| Streamable HTTP/WebSocket ACP transport | Missing draft transport | Adapter runs ACP over stdio via the TypeScript SDK stream. | Defer unless distribution needs remote ACP transport. This is transport packaging, not Codex feature mapping. |
74+
| ACP v2 prompt lifecycle | Missing draft protocol version work | Adapter returns the current SDK `PROTOCOL_VERSION` and uses v1 method shapes. | Defer until SDK and spec provide stable v2 interop requirements for agents. |
75+
76+
## Existing Features That Are Not Missing
77+
78+
- Required agent lifecycle: `initialize`, `authenticate`, `session/new`, `session/prompt`, and `session/cancel`.
79+
- Stable session restore basics: `session/load`, `session/list`, `session/resume`, and `session/close`.
80+
- Agent metadata: `initialize` returns `agentInfo` from `package.json`.
81+
- Session modes: three Codex modes are returned and `session/set_mode` mutates local session state.
82+
- Session config options: Fast mode is implemented as a select option and returned from new/load/resume.
83+
- Model state: model list is returned from Codex, and `unstable_setSessionModel` updates local model/effort state.
84+
- Permission requests: command, file-change, and MCP approval requests are translated to ACP `session/request_permission`.
85+
- Tool call streaming: command execution, file changes, MCP tools, dynamic tools, web search, image view, fuzzy file search, plans, warnings, token usage, and terminal-output metadata have ACP mappings.
86+
- MCP stdio and HTTP session configuration are supported; HTTP headers are forwarded.
87+
- Logout is implemented against Codex account logout, pending shape verification for the Preview standard method.
88+
89+
## Suggested Work Order
90+
91+
1. Close stable compliance gaps: `session_info_update`, strict `session/list` validation, and `session/load` history coverage.
92+
2. Keep `@agentclientprotocol/sdk` aligned with the latest generated schema as new RFD surfaces stabilize.
93+
3. Implement the session lifecycle draft cluster together: `session/delete`, `session/fork`, and `additionalDirectories`.
94+
4. Standardize provider/auth configuration: ACP providers, standard auth variants, and removal or namespacing of custom extension methods.
95+
5. Decide product direction for larger protocol surfaces: structured elicitation, MCP-over-ACP, NES, and remote transports.

src/CodexAcpClient.ts

Lines changed: 47 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -378,18 +378,44 @@ export class CodexAcpClient {
378378
this.codexClient.onElicitationRequest(sessionId, elicitationHandler);
379379
}
380380

381+
disposeSession(sessionId: string): void {
382+
this.codexClient.clearSessionHandlers(sessionId);
383+
}
384+
385+
async closeSession(sessionId: string): Promise<void> {
386+
await this.codexClient.threadUnsubscribe({ threadId: sessionId });
387+
}
388+
381389
async sendPrompt(
382390
request: acp.PromptRequest,
383391
agentMode: AgentMode,
384392
modelId: ModelId,
385393
serviceTier: ServiceTier | null,
386394
disableSummary: boolean,
387395
cwd: string,
396+
onTurnStarted?: (turnId: string) => void,
397+
onTurnStartRequested?: () => void,
398+
isCancelled?: () => boolean,
399+
cancellation?: Promise<TurnCompletedNotification>,
388400
): Promise<TurnCompletedNotification> {
389401
const input = buildPromptItems(request.prompt);
390402
const effort = modelId.effort as ReasoningEffort | null; //TODO remove unsafe conversion
391403

392-
await this.refreshSkills(cwd, request._meta);
404+
if (cancellation) {
405+
const refreshResult = await Promise.race([
406+
this.refreshSkills(cwd, request._meta).then(() => null),
407+
cancellation,
408+
]);
409+
if (refreshResult !== null) {
410+
return refreshResult;
411+
}
412+
} else {
413+
await this.refreshSkills(cwd, request._meta);
414+
}
415+
if (isCancelled?.()) {
416+
return createInterruptedTurnCompleted(request.sessionId, "cancelled-before-turn");
417+
}
418+
onTurnStartRequested?.();
393419
return await this.codexClient.runTurn({
394420
threadId: request.sessionId,
395421
input: input,
@@ -399,7 +425,7 @@ export class CodexAcpClient {
399425
effort: effort,
400426
model: modelId.model,
401427
serviceTier: serviceTier,
402-
});
428+
}, onTurnStarted, cancellation);
403429
}
404430

405431
async listSkills(params?: SkillsListParams): Promise<SkillsListResponse> {
@@ -524,6 +550,10 @@ export class CodexAcpClient {
524550
});
525551
}
526552

553+
resolveInterruptedTurn(params: { threadId: string, turnId: string }): void {
554+
this.codexClient.resolveTurnInterrupted(params.threadId, params.turnId);
555+
}
556+
527557
async fetchAvailableModels(): Promise<Model[]> {
528558
const models: Model[] = [];
529559
let cursor: string | null = null;
@@ -575,6 +605,21 @@ export type SessionMetadataWithThread = SessionMetadata & {
575605
thread: Thread,
576606
}
577607

608+
function createInterruptedTurnCompleted(threadId: string, turnId: string): TurnCompletedNotification {
609+
return {
610+
threadId,
611+
turn: {
612+
id: turnId,
613+
items: [],
614+
status: "interrupted",
615+
error: null,
616+
startedAt: null,
617+
completedAt: null,
618+
durationMs: null,
619+
},
620+
};
621+
}
622+
578623
function buildPromptItems(prompt: acp.ContentBlock[]): UserInput[] {
579624
return prompt.map((block): UserInput | null => {
580625
switch (block.type) {

0 commit comments

Comments
 (0)