feat!: AIR tool call contract, exact diff patches, and fixes for every client - #530
Open
nikita-ashihmin wants to merge 41 commits into
Open
nikita-ashihmin wants to merge 41 commits into
nikita-ashihmin wants to merge 41 commits into
Conversation
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
One critical and six moderate correctness issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (2)
What changed in this PR
Implements the AIR tool-call contract, sparse field reporting, exact Git patches, and improved tool/subagent lifecycle handling while preserving non-AIR compatibility.
Changes:
- Adds capability-aware tool reporters and AIR-specific metadata/rendering.
- Adds exact Git patch and plan-delta support.
- Expands protocol, lifecycle, replay, buffering, and scenario coverage.
Unresolved findings:
- Critical (1 vote):
src/CodexAcpServer.ts:2297must force terminal status when replaying history. - Moderate (2 votes):
src/GitPatch.ts:103accepts malformed no-newline markers. - Moderate (1 vote):
src/ToolCallReportingConnection.ts:49does not restore full permission-call fields after cancellation. - Moderate (1 vote):
src/ToolCallReportingConnection.ts:41repeats unchanged permission-call fields. - Moderate (1 vote):
src/subagents/PendingNotificationBuffer.ts:34undercounts JSON-escaped buffered bytes. - Moderate (1 vote):
src/tool-calls/reporters/FileChangeReporter.ts:110drops pure renames with empty patches. - Moderate (1 vote):
src/tool-calls/reporters/ImageGenerationReporter.ts:61duplicates images for non-AIR clients. - Nit (1 vote):
README.md:17incorrectly presents the AIR-only shape as universal.
| File | Reviewed change |
|---|---|
src/ToolCallReportingConnection.ts |
Filters changed fields; two permission-flow issues remain. |
src/tool-calls/ToolFacts.ts |
Defines normalized tool facts. |
src/tool-calls/reporters/WebSearchReporter.ts |
Reports web searches. |
src/tool-calls/reporters/ToolStatus.ts |
Maps tool statuses. |
src/tool-calls/reporters/SubagentActivityReporter.ts |
Reports subagent activity. |
src/tool-calls/reporters/SandboxPermissionReporter.ts |
Reports sandbox permissions. |
src/tool-calls/reporters/PlanReviewReporter.ts |
Reports plan reviews. |
src/tool-calls/reporters/McpToolReporter.ts |
Reports MCP tools. |
src/tool-calls/reporters/McpStartupReporter.ts |
Reports MCP startup. |
src/tool-calls/reporters/ImageViewReporter.ts |
Reports viewed images. |
src/tool-calls/reporters/ImageGenerationReporter.ts |
Reports generated images; non-AIR duplication remains. |
src/tool-calls/reporters/GuardianReporter.ts |
Reports guardian reviews. |
src/tool-calls/reporters/FuzzySearchReporter.ts |
Reports fuzzy searches. |
src/tool-calls/reporters/ElicitationReporter.ts |
Reports elicitation requests. |
src/tool-calls/reporters/DynamicToolReporter.ts |
Reports dynamic tools. |
src/tool-calls/reporters/CompactionReporter.ts |
Reports context compaction. |
src/tool-calls/reporters/CollabAgentReporter.ts |
Reports collaborative agents. |
src/tool-calls/ClientCapabilities.ts |
Centralizes client capabilities. |
src/TerminalOutputMode.ts |
Removes superseded output capability handling. |
src/subagents/PendingNotificationBuffer.ts |
Buffers notifications; encoded-size accounting remains incorrect. |
src/subagents/CodexSubagentEventRouter.ts |
Routes and cleans up subagent events. |
src/PlanCapabilities.ts |
Removes superseded plan capabilities. |
src/permissions/presentation.ts |
Updates permission presentation. |
src/permissions/plan-review.ts |
Updates plan-review permissions. |
src/permissions/metadata.ts |
Namespaces permission metadata. |
src/permissions/mcp.ts |
Updates MCP permissions. |
src/permissions/lifecycle.ts |
Updates permission lifecycle handling. |
src/permissions/CodexApprovalHandler.ts |
Integrates approval reporting. |
src/DiffStats.ts |
Removes replaced diff statistics. |
src/ContextCompactionMeta.ts |
Reshapes compaction metadata. |
src/ContentChunks.ts |
Gates AIR content metadata. |
src/CodexPlanStream.ts |
Streams capability-aware plan deltas. |
src/CodexElicitationHandler.ts |
Integrates elicitation reporting. |
src/CodexCommands.ts |
Namespaces AIR command actions. |
src/AirExtension.ts |
Defines AIR negotiation and metadata. |
src/AgentMode.ts |
Gates AIR mode metadata. |
src/__tests__/ToolCallReportingConnection.test.ts |
Tests filtering and cancellation. |
src/__tests__/TerminalOutputMode.test.ts |
Updates terminal-output tests. |
src/__tests__/scenarios/data/zed/mcp-elicitation.json |
Updates Zed elicitation snapshot. |
src/__tests__/scenarios/data/zed/goal-update.json |
Updates Zed goal snapshot. |
src/__tests__/scenarios/data/plain/plan-stream.json |
Updates plain plan snapshot. |
src/__tests__/scenarios/data/plain/mcp-elicitation.json |
Updates plain elicitation snapshot. |
src/__tests__/scenarios/data/plain/goal-update.json |
Updates plain goal snapshot. |
src/__tests__/scenarios/acp-schema.ts |
Validates outbound ACP messages. |
src/__tests__/PendingNotificationBuffer.test.ts |
Tests notification buffering. |
src/__tests__/GitPatch.test.ts |
Tests Git patch generation. |
src/__tests__/DiffStats.test.ts |
Updates diff-related tests. |
src/__tests__/CodexPlanStream.test.ts |
Tests plan streaming profiles. |
src/__tests__/CodexACPAgent/turn-diff-events.test.ts |
Tests turn diff events. |
src/__tests__/CodexACPAgent/thread-goal-events.test.ts |
Tests goal events. |
src/__tests__/CodexACPAgent/terminal-output-events.test.ts |
Tests terminal output events. |
src/__tests__/CodexACPAgent/session-compaction.test.ts |
Tests session compaction. |
src/__tests__/CodexACPAgent/response-item-history-fallback.test.ts |
Tests history fallback. |
src/__tests__/CodexACPAgent/plan-review-events.test.ts |
Tests plan-review events. |
src/__tests__/CodexACPAgent/plan-events.test.ts |
Tests plan events. |
src/__tests__/CodexACPAgent/load-session.test.ts |
Tests session loading. |
src/__tests__/CodexACPAgent/initialize.test.ts |
Tests capability initialization. |
src/__tests__/CodexACPAgent/fuzzy-file-search-events.test.ts |
Tests fuzzy-search events. |
src/__tests__/CodexACPAgent/elicitation-events.test.ts |
Tests elicitation events. |
src/__tests__/CodexACPAgent/data/web-search-start-and-complete.json |
Covers web-search lifecycle. |
src/__tests__/CodexACPAgent/data/web-search-action-titles.json |
Covers web-search titles. |
src/__tests__/CodexACPAgent/data/view-image-flow.json |
Covers image viewing. |
src/__tests__/CodexACPAgent/data/tool-call-dynamic-names.json |
Covers dynamic tool names. |
src/__tests__/CodexACPAgent/data/tool-call-completed-name.json |
Covers completed tool names. |
src/__tests__/CodexACPAgent/data/tool-call-command-names.json |
Covers command names. |
src/__tests__/CodexACPAgent/data/thread-goal-updated.json |
Covers goal updates. |
src/__tests__/CodexACPAgent/data/thread-goal-updated-multiline.json |
Covers multiline goals. |
src/__tests__/CodexACPAgent/data/thread-goal-cleared.json |
Covers cleared goals. |
src/__tests__/CodexACPAgent/data/terminal-output-parsed-command-legacy-delta.json |
Covers legacy terminal deltas. |
src/__tests__/CodexACPAgent/data/terminal-interaction-stdin.json |
Covers terminal input. |
src/__tests__/CodexACPAgent/data/terminal-command-failed.json |
Covers failed commands. |
src/__tests__/CodexACPAgent/data/terminal-command-completed.json |
Covers completed commands. |
src/__tests__/CodexACPAgent/data/session-notices-replay.json |
Covers notice replay. |
src/__tests__/CodexACPAgent/data/session-compaction-legacy.json |
Covers legacy compaction. |
src/__tests__/CodexACPAgent/data/response-item-history-tool-names.json |
Covers history tool names. |
src/__tests__/CodexACPAgent/data/plan-deltas.json |
Covers plan deltas. |
src/__tests__/CodexACPAgent/data/plan-delta-fallback.json |
Covers plan-delta fallback. |
src/__tests__/CodexACPAgent/data/plan-completed-fallback.json |
Covers completed-plan fallback. |
src/__tests__/CodexACPAgent/data/mcp-tool-repeated-progress.json |
Covers repeated MCP progress. |
src/__tests__/CodexACPAgent/data/mcp-tool-completed-with-logs.json |
Covers MCP completion logs. |
src/__tests__/CodexACPAgent/data/load-session-response-item-history-fallback.json |
Covers loaded history fallback. |
src/__tests__/CodexACPAgent/data/load-session-history.json |
Covers loaded session history. |
src/__tests__/CodexACPAgent/data/image-generation-flow.json |
Covers image-generation lifecycle. |
src/__tests__/CodexACPAgent/data/image-generation-completed-only.json |
Covers completion-only image generation. |
src/__tests__/CodexACPAgent/data/guardian-approval-review-flow.json |
Covers guardian review flow. |
src/__tests__/CodexACPAgent/data/guardian-approval-review-completed-without-start.json |
Covers completion-only guardian review. |
src/__tests__/CodexACPAgent/data/file-change-delete-raw-content.json |
Covers deletion from raw content. |
src/__tests__/CodexACPAgent/data/file-change-delete-file.json |
Covers file deletion. |
src/__tests__/CodexACPAgent/data/file-change-add-raw-content.json |
Covers addition from raw content. |
src/__tests__/CodexACPAgent/data/file-change-add-new-file.json |
Covers new files. |
src/__tests__/CodexACPAgent/data/file-change-add-multiple-files.json |
Covers multiple file additions. |
src/__tests__/CodexACPAgent/data/elicitation-url-accept.json |
Covers accepted URL elicitation. |
src/__tests__/CodexACPAgent/data/elicitation-tool-approval-session-only.json |
Covers session-only approval. |
src/__tests__/CodexACPAgent/data/elicitation-tool-approval-no-persist.json |
Covers non-persisted approval. |
src/__tests__/CodexACPAgent/data/elicitation-tool-approval-all-persist.json |
Covers persisted approval. |
src/__tests__/CodexACPAgent/data/dynamic-tool-in-progress.json |
Covers active dynamic tools. |
src/__tests__/CodexACPAgent/data/dynamic-tool-completed.json |
Covers completed dynamic tools. |
src/__tests__/CodexACPAgent/data/context-compaction-lifecycle.json |
Covers compaction lifecycle. |
src/__tests__/CodexACPAgent/data/command-search-with-query-only.json |
Covers query-only search. |
src/__tests__/CodexACPAgent/data/command-search-with-query-and-path.json |
Covers scoped query search. |
src/__tests__/CodexACPAgent/data/command-search-with-path-only.json |
Covers path-only search. |
src/__tests__/CodexACPAgent/data/command-search-no-query-no-path.json |
Covers empty search parameters. |
src/__tests__/CodexACPAgent/data/command-read-file-with-path.json |
Covers file reads. |
src/__tests__/CodexACPAgent/data/command-list-files-without-path.json |
Covers unscoped file listing. |
src/__tests__/CodexACPAgent/data/command-list-files-with-path.json |
Covers scoped file listing. |
src/__tests__/CodexACPAgent/data/available-commands-skills.json |
Covers skill commands. |
src/__tests__/CodexACPAgent/data/available-commands-build-in.json |
Covers built-in commands. |
src/__tests__/CodexACPAgent/data/agent-message-phases.json |
Covers message phases. |
src/__tests__/CodexACPAgent/collab-agent-events.test.ts |
Tests collaborative-agent events. |
src/__tests__/CodexACPAgent/CodexAcpClient.test.ts |
Tests ACP client behavior. |
src/__tests__/CodexACPAgent/auth-status.test.ts |
Tests authentication status. |
src/__tests__/CodexACPAgent/auth-error-events.test.ts |
Tests authentication errors. |
src/__tests__/CodexACPAgent/approval-events.test.ts |
Tests approval events. |
src/__tests__/CodexACPAgent/agent-file-change-report.test.ts |
Tests file-change reports. |
src/__tests__/AirExtension.test.ts |
Tests AIR negotiation. |
src/__tests__/acp-test-utils.ts |
Updates shared test utilities. |
README.md |
Documents AIR features; wording overstates their scope. |
readme-dev.md |
Removes obsolete diff-statistics guidance. |
package.json |
Adds schema-validation support. |
package-lock.json |
Locks dependency changes. |
docs/subagent-sessions.md |
Updates subagent documentation. |
docs/session-compaction.md |
Updates compaction documentation. |
docs/recommended-config-values-extension.md |
Removes superseded documentation. |
docs/goal-extension.md |
Removes superseded documentation. |
docs/diff-statistics-extension.md |
Removes replaced documentation. |
docs/async-tasks.md |
Removes superseded documentation. |
docs/agent-file-change-report.md |
Removes superseded documentation. |
CHANGELOG.md |
Removes the obsolete diff-statistics entry. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| return [renderer.render(ImageViewReporter.viewed(item))]; | ||
| case "imageGeneration": | ||
| return [createImageGenerationUpdate(item)]; | ||
| return [renderer.render(ImageGenerationReporter.whole(item))]; |
Send compact Git patches when both the client and the adapter advertise diffPatch. Keep the standard diff text fields for a client without the capability. The patch builders: - strip the leading slash of an absolute path, so a header reads a/workspace/App.ts and not a//workspace/App.ts; - quote a path that Git quotes; - add rename from and rename to headers and use the target path; - replace the file headers that Codex supplied, so that all headers agree; - add new file mode and deleted file mode headers; - keep carriage returns and mark a missing final newline. A patch is built only when it has at least one valid hunk, holds no binary content and is at most 1 MiB. Otherwise the adapter sends the standard diff with the file texts. This covers an empty file, a pure rename and malformed Codex hunks. Before, the adapter dropped the change when the diff did not parse. The legacy already-patched branch now reports the move target path. Stop sending the ACP diff statistics. A client can derive the line counts from a negotiated patch or from the standard diff texts. The diff statistics contract and its documentation are removed. docs/diff-patch-extension.md describes the negotiation, the patch metadata, the placeholder text fields, the header paths and quoting, the file mode and rename headers, the kept bytes, the final newline marker and every fallback case.
nikita-ashihmin
force-pushed
the
nikita.ashikhmin/acp-patch-content
branch
from
September 23, 2026 11:48
21b4f35 to
34a3d5c
Compare
The hunk validator accepted every line that starts with a backslash. A hunk with such a line then went to the client as a valid patch. Now the validator accepts only the exact "\ No newline at end of file" line that Git writes. Any other backslash line makes the hunks invalid. The adapter then sends the standard ACP diff.
…r every client
Report tool calls through reporters and one renderer. The tool call
mapping moves to src/tool-calls/. A ToolReporter per tool kind reads
the Codex item once and returns tool facts. One AcpToolCallRenderer
puts each fact into one ACP field. It reads the capability choices
from one ClientCapabilities object. CodexToolCallMapper,
TerminalOutputMode, PlanCapabilities and permissions/presentation.ts
are removed.
Only AIR gets the AIR shape. A client is AIR when it declares
clientCapabilities._meta.jetbrains.air. A client that is not AIR, for
example Zed or a plain ACP client, gets the fields that origin/main
sent. ToolFacts.standard holds these fields where they differ.
Fixes for every client:
- Send only changed tool call fields. A new ToolCallReportingConnection
wraps the ACP client connection and runs every session update through
the ToolCallReports filter: live events, history replay, MCP startup,
permission and elicitation updates, plan review and async tasks. The
permission request tool call counts as a report. Appended output
chunks are never compared. ACP defines no merge for the keys of
_meta, so a client that is not AIR gets the whole _meta of each
report.
- Release the reported fields when a turn ends, when a native child
session ends, and after a cancelled or failed permission request.
Keep the small fields of a finished tool call, so a replayed tool
call does not repeat its status.
- Drop terminal and MCP output deltas that arrive after the tool call
finished.
- Send replayed command output once, and generated images once.
- Make the MCP startup tool call ids unique.
- Forward the result of a dynamic tool in content.
- Keep the shown title, status and kind of a started tool call in its
permission request.
- Send empty locations when a fuzzy search finds no file.
- Keep every notification of a pending subagent. The buffer merges
adjacent text deltas and is bounded by bytes, not by count. It stores
a copy of each notification.
- Send terminal_output_delta chunks to a client that declares no
terminal channel. Zed gets terminal_output for a command with a
terminal.
The AIR profile:
- Move the AIR metadata keys to _meta.jetbrains.air: the message phase,
the goal, the mode kind, the command action, the permission
presentation and the compaction record. AIR is not released, so the
adapter sends only the new keys.
- One fact goes into one field. A command sends its output in
terminal_output_delta, the raw stdin in _meta.terminal_input and its
end in terminal_exit, without rawOutput. Read, search and list output
is a result in content.
- rawInputRendering: AIR gets no display copy of readable input.
- planContentDelta: a streamed plan goes out as plan_update snapshots
and _meta.jetbrains.air.contentDelta appends (CodexPlanStream).
- The MCP result goes to rawOutput = {result, error}. AIR gets no MCP
progress. Other clients get the trimmed progress text.
- rawInput keeps the collaboration keys. Only spawnAgent gets
_meta.jetbrains.air.subagent.
- The plan review sends the plan text in rawInput.plan.
…air-extensions.md Merge the AIR extension docs into one file: the diff patch, the agent file change report, async tasks, goals, permissions, recommended config values, and the tool call contract. The old files are removed, and the README and the other docs link to the new sections. The compatibility rule limits the AIR extensions to AIR. A client that is not AIR gets the fields that the adapter sent before these extensions, with only the listed differences. The docs state the title and commandTitle exceptions of the contract, the terminal channel and the _meta filter of each client, and the Zed conventions. The plan of codex-acp is always streamed text. AIR also accepts a plan as a path to a file that it follows, but Codex keeps the plan only as text, so this adapter does not declare the planFile capability.
…call The tool call contract sent only the status in the permission request of a started MCP tool call. Before, the request also had kind execute. A client that reads the kind of the request, such as the MCP approval e2e test, no longer found it. Now the request has kind execute again for every client. The compatibility rule in docs/air-extensions.md now says that a permission request omits no field that it had before.
A scenario harness drives the adapter with scripted app-server traffic for every tool kind: commands with output, stdin, and failure; read, search, and list; file changes; MCP tools, approvals, and elicitations; dynamic tools; web search; images; subagents with native, nested and late child sessions and the collaboration controls; fuzzy search; guardian reviews; compaction; plans and plan review; goals; permission requests; MCP startup failures; background terminals; messages and reasoning; and a history replay. The harness records every outbound ACP message for a plain ACP client, Zed, and AIR. The tests: - validate each message against the ACP JSON schema of @agentclientprotocol/sdk; - keep the AIR messages as golden snapshots, one message per line, with the keys in sorted order; - compare the plain and Zed messages with the baseline, the recorded messages of origin/main at 1cc6223. The comparison applies only the allowed differences of the compatibility rule in docs/air-extensions.md; - check that no client gets a tool call field twice with the same value, the Zed conventions, the plain client output, the subagent paths, and the AIR keys. The SDK does not export its zod schemas, and zod cannot read the ACP schema, so Ajv validates it. Ajv is a new dev dependency. Each of these regressions was injected into the adapter and then reverted, to prove that the baseline comparison catches it: - no is_mcp_tool_call for a client that is not AIR: 6 comparisons fail; - Zed gets the command output in content, not in rawOutput: 6 fail; - the plain client loses its first terminal_output_delta chunk: 1 fails; - a client that is not AIR gets every unchanged field again: the repeated-field check fails for plain and Zed. To record the baseline again, see src/__tests__/scenarios/baseline.ts.
The baseline comparison merged the tool call of a permission request into the stored tool call. A request that lost a field, such as the kind, then still equaled the baseline. Now the comparison keeps the tool call of a permission request as it is. Only a tool_call_update is merged. The fields of the request still go into the stored tool call. A new test checks that a request without the kind fails the comparison. The AIR recording of mcp-tool-approval now has kind execute in the request, after the fix of the approval request in the tool call contract.
…nses The scenario harness removed models from the session/new and session/load responses. A change of the model list then passed every scenario test, although the tests claimed to record every outbound message. Now the harness keeps models. The baseline of the plain client and of Zed was recorded again at 1cc6223, and the AIR snapshots were updated. Each recording changes only in the session/new or session/load line, which now has models. The model list of the current adapter equals the baseline. A new test checks the model list of each profile. docs/air-extensions.md now names the one message that the harness does not record: the _auth/status_update notification.
…ording The harness replaced every UUID-shaped string with <uuid>, every string under a version key with <version>, and every large number under a key that ends with At, AtMs, Time, or timestamp with <time>. A wrong id, version, or time of the same shape then passed the comparison. Now the harness replaces only these exact values: - the temporary workspace path, as before; - the package version in initialize.agentInfo.version; - the random id of each MCP startup tool call, wherever that id occurs. The time rule is removed. The only times in the recordings are the goal times, and the scenario sets them. The AIR goal-update snapshot and the plain and Zed goal-update baselines, recorded again at 1cc6223, now show createdAt 1710000000000 and updatedAt 1710000001000. With the old rules, a wrong goal time conversion or a wrong agent version passed every scenario test. Now the first fails the AIR goal-update snapshot, and the second fails 99 recordings. New tests check that normalization keeps every other id, version, path, and time.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Multiple moderate patch-validation and pure-rename handling issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (3)
Resolved since last review (1)
Comment on lines
+882
to
+885
| const patch = parseSinglePatch(unifiedDiff); | ||
| if (patch === null) { | ||
| logger.log("Skipped a file change whose diff has no single valid patch", {path: change.path}); | ||
| return null; |
Comment on lines
+83
to
+86
| const header = HUNK_HEADER.exec(hunks[index]!); | ||
| if (header === null) return null; | ||
| let oldLines = header[1] === undefined ? 1 : Number(header[1]); | ||
| let newLines = header[2] === undefined ? 1 : Number(header[2]); |
…enario recordings The step-3 commit said that the scenario tests record every outbound ACP message. The harness still dropped two kinds of message: - the _auth/status_update notification, by a filter; - every message before the first turn, because it cleared the connection dump after session/new. These were the first _auth/status_update and the available_commands_update of the session. Now the harness drops no message. After initialize, it waits for the first _auth/status_update, because the adapter sends it from a later event loop phase. After session/new, it waits for the available_commands_update. Both waits keep the order of the recording stable. The ACP schema defines no _auth/status_update, so the tests validate it against the payload of AuthStatusUpdateNotification in src/AuthStatusMeta.ts. An extension notification without a schema fails the validation. The recordings changed only by added lines: - each of the 99 recordings has the _auth/status_update line after the initialize response; - the 93 recordings of a scenario with a prompt turn have the available_commands_update line after the session/new response. The history and MCP startup scenarios recorded it already. The plain and Zed baseline was recorded again at 1cc6223 with the new harness, as baseline.ts describes. The same procedure with the old harness gives the old baseline byte for byte, so the baseline changes only by the new lines. The AIR snapshots were updated. A new test checks that each recording starts with the initialize response and the auth status, and has one available_commands_update. The timeline helper and one guardian review test now skip the messages of the session start.
…hema coverage docs/air-extensions.md said that the scenario tests validate each message against the ACP schema. The ACP schema does not define the AIR session updates of subagents and async tasks: subagent_spawned, subagent_state_update, async_task_spawned, and async_task_state_update. The tests skipped these updates, so an update without a session id passed. Now the tests check the SessionNotification envelope of each AIR session update: a valid ACP sessionId, an update with an AIR kind, and _meta as an object or null. The payload of the update stays unchecked, because no schema defines it. The docs now say that only the standard ACP messages are validated against the ACP schema, and name the checks for the other messages. A new test checks that the envelope check rejects an AIR session update without a sessionId, with a sessionId that is not a string, and with a _meta that is not an object. The recordings do not change. This answers the Copilot review comment on docs/air-extensions.md in PR #545.
A history item is finished. The history replay rendered an image generation with ImageGenerationReporter.whole(item), which mapped a provider status such as generating to in_progress. The terminalStatus option was set only for a live item/completed. A persisted item that Codex saved while it was generating then stayed an active tool call that never completed. origin/main had the same bug, so the fix applies to every client. Now whole() always maps the status to completed or failed, because both callers report a finished item. The terminalStatus option and the non-terminal status mapping are removed. docs/air-extensions.md lists the fix with the other bug fixes of the compatibility rule. A new test replays a generating image generation for each client profile and expects one report with status completed. The scenario recordings do not change, because the history-replay scenario has only a completed image generation. This answers the Copilot review comment on src/CodexAcpServer.ts in PR #530.
…built The hunk validator read only the line counts of a hunk header and ignored the start lines. Hunks out of order, overlapping hunks, such as two identical "@@ -1 +1 @@" hunks, and a hunk with a wrong new start line then went to the client as a valid patch. A start or a count that is larger than a safe integer was also accepted. Now the validator reads the start and the count of both sides as safe integers. Start 0 is valid only for an empty side. An empty side starts after the line that the header names, as Git writes it. Each hunk must start at or after the end of the previous hunk in the old file. Its new start must equal its old start plus the line count change of the hunks before it. Otherwise the adapter sends the standard ACP diff. New tests check valid hunks with an insertion and a deletion, and reject hunks out of order, overlapping hunks, wrong new start lines, start 0 with lines, and numbers that are not safe integers. This answers the Copilot review comment on src/GitPatch.ts ~86 in PR #530.
…ffer cap The pending notification buffer of a subagent stores serialized JSON notifications and is bounded by bytes. For a new notification, it reserved the size of the serialized notification. For a text delta that it merged into the previous notification, it reserved only the UTF-8 size of the raw delta. A quote, a backslash, a newline, or a control character takes more bytes in JSON, so the stored notifications could exceed maxBytes. Now a merged delta reserves the growth of the serialized notification: the delta as a JSON string without its two quotes. A surrogate pair that a stream splits between two deltas counts with its escapes, so the count can be too large but never too small. A new test fills the buffer to 10 bytes under the cap and pushes a delta of three quotes and three newlines. The delta is 6 bytes of text and 12 bytes in JSON. Before the fix, the buffer kept it and exceeded the cap by 2 bytes. Now it drops the delta and keeps the next delta that fits. This answers the Copilot review comment on src/subagents/PendingNotificationBuffer.ts in PR #544.
The reader joined each 64 KB chunk to the partial line and searched the whole partial line for a line break again. One line of M bytes therefore took O(M^2 / 64 KB) time. A 20 MB line took about 6 s, and a 40 MB line took 21 s. Codex sends lines of that size, for example an item/completed with the aggregated command output or a turn/diff/updated. The reader also decoded each chunk alone. A UTF-8 character on a chunk boundary became two replacement characters. readline decodes UTF-8 across the chunks and searches only the new chunk. A 40 MB line now takes about 20 ms. The reader still adds the missing jsonrpc field, and it still skips blank and malformed lines.
session/load for a client without subagent sessions also read the whole rollout file and rebuilt tool calls from its raw function_call records. It then merged them into the thread history. The rollout parse is O(file size) on each load. The merge scanned the fallback list from the start for each thread update, which is O(U * F): 12,000 updates on each side took 16.5 s, all synchronous. The fallback was added when thread/read returned only the text of a session. The app-server now returns the tool items as well. A check of 532 local sessions with thread/read found no command, file change or MCP call that only the fallback recovered. The fallback added only calls that the live session did not show as tool calls: 1167 send_message, 339 followup_task, 302 spawn_agent with an encrypted message, 283 sleep that duplicate the sleep items, 180 list_agents, 124 wait, 41 write_stdin and 30 request_user_input. The adapter now replays only the thread items and sends each update when it is built. A session from an old app-server that has no tool items replays its text only.
session/load read every turn of the thread into one array, and only then sent the history. For native subagents it also read the whole thread of each child and kept it in a cache until the load ended, although the replay used only one turn of the child. A local session of 171 turns kept 178 MB of heap during the load. A session of 4 turns with 7457 items kept 160 MB, because one turn can hold thousands of items. The adapter now reads the history with thread/items/list, oldest first, 100 items per page, and sends each page before it reads the next one. The pages end at the itemsBackwardsCursor of thread/resume, so an item that arrives after the resume is not replayed twice. A child replay reads only the items of its turn. The same two sessions now keep 10 MB and 6 MB. A legacy store still reads the whole history in one request. A full read for a fork still reads every turn.
For AIR, the adapter collected the output chunks of each read, search and list command in memory and sent the output once in content at completion. After 1 MiB, each chunk copied the last 1 MiB of the collected text, so 50 MB of output in 1 KB chunks cost about 5 s of CPU. The collected text was used only when item/completed had no aggregatedOutput, and none of 14,812 local completed commands lacked it. AIR also drops the result text of a read: it does not need the file text. AIR now gets no output of a file read. The output of a search or a list streams to _meta.terminal_output_delta, and output that did not stream goes there once at completion. The adapter collects nothing. The other clients keep the output as on main.
The standard ACP diff of an updated file held the whole file before and after the change. To build it, the adapter read the file from disk, applied the Codex hunks, and on a failure applied the reversed hunks. For a 50 MB file that took about 1 to 2 s of synchronous work and about 350 MB at the peak. The history replay did the same for each file change of the session, against today's file, so the patch usually failed. The adapter now builds the standard diff from the Codex diff alone. An update gets one diff block per hunk, with the context lines and the changed lines of the hunk. ACP asks for the original and the new content of the change, not of the whole file. The adapter reads no file, for a live change and for a replay. A pure rename gets one block without changed lines. An added or a deleted file keeps its whole text. A Codex diff larger than 1 MiB gets no block, for every client. The Git patch builder now checks the size of its source text before it copies it: a 50 MB added file took 356 ms and 151 MB before it returned null.
The output of a command could reach a client that is not AIR twice: in the
chunks, and again in rawOutput.formatted_output at the end. Zed and plain
clients got both for a terminal command, and a replayed command carried
both for every client. Zed also got read, search and list chunks in
terminal_output_delta, which it does not read, and then the output in
rawOutput, which it shows only as a JSON fallback when content is empty.
The rule is now one channel per command and client:
- With a chunk channel, the output goes only to the chunks, and output
that did not stream goes in one chunk at the end, also on a replay.
- Without a chunk channel, the output goes once to content as text. Zed has
no chunk channel for a read, search or list command.
- A command without a terminal keeps rawOutput = {exit_code}.
AIR gets the output of a read, search or list command once, as rawOutput
text at the end, from aggregatedOutput. AIR stores it as an output stream
and shows it on the generic card. The file read is not dropped any more.
Stdin no longer marks a command as streamed, so a command that got stdin
and no output chunk still sends its output at the end.
The baseline comparison accepts the output once, in any channel, as an
allowed difference.
nikita-ashihmin
force-pushed
the
nikita.ashikhmin/acp-patch-content
branch
from
September 24, 2026 11:41
7aab80c to
2168cb0
Compare
Before each turn/start, the prompt waited for
skills/list {forceReload: true} and dropped the answer. That was one more
request and a scan of the skill directories on each turn, before the model
could start.
A check with Codex CLI 0.156.1 showed that the reload is not needed: a
skill file that appears during a session is in skills/list without
forceReload, and the model of the next turn sees it without any request.
Codex sent no skills/changed notification in that check.
The prompt now only sets the extra skill roots when they change. After a
turn, the adapter lists the skills in the background and sends a new
available_commands_update when the commands changed. A skill that appears
during a session now becomes a slash command after the next turn. Before,
it did not appear at all.
A spawned child is announced to the client only when Codex reports its subAgentActivity. Until then, the router buffers the updates of the child, up to 32 MiB. A child that ended first, for example a fast child, a failed spawn or a child of an interrupted root turn, moved to terminalPendingSpawns together with its buffer. That map lives until the session closes, so the buffer stayed in memory for the whole session, although nothing read it again. The ended spawn now keeps only its parent and its task, which a reopen needs. Its buffered updates are dropped, as the client never saw the child. The replay of a materialized child also pushed the whole buffer with a spread, push(...buffered). About 150,000 updates exceed the argument limit and throw a RangeError after the buffer is already taken, so the updates were lost and the approvals that wait for the child session hung. The replay now uses a loop. A test replays 200,000 updates.
…ended tasks Codex sends no event when a command moves to the background or its background process exits, so the adapter lists the background terminals of a thread. It sent thread/backgroundTerminals/list on every item/started of any type, also for a message or a reasoning item, and it waited for one more list at turn/completed inside the notification queue of the session. A task also stayed in memory for the whole session. The adapter remembers every command with a process id, and finish() only changed the state, so a normal command that never went to the background stayed as well. Each sync walked all tasks of the session. Now: - The adapter lists the terminals on item/started only while a command of the thread runs, and at the turn end without waiting. A background task outlives the turn, so the prompt response does not depend on the list. - A task leaves the map when it ends: at once when the client never saw it, and after its terminal state reached the client otherwise. - A small map keeps the ended tasks that a list can still name, for example a list that started before the end. A task leaves it when a list of its thread no longer names it, so an ended task never comes back.
Since the history replay reads pages lazily, a history request runs after the session is installed. A failed page request failed session/load, but the session state and the thread subscription stayed: the client got part of the history, an error, and a half-open session. Before, the history was read inside the open, and a failure closed the session. A failed read of a subagent turn now also stays local to that child, as before: the child pages are read lazily too, so the error came from the nested replay and failed the whole load. - A failed root history read now closes the session and rethrows. - A failed child history read marks the child as unreadable, logs, and the replay goes on.
0b1437c read the Codex stdout with readline. readline also ends a line at U+2028 and U+2029, and JSON allows both unescaped in a string. Codex writes them so. A message that held one was split into parts that are not JSON, and the reader dropped them in silence. A lost response left its request waiting forever: session/load of a real session hung at the page whose text held U+2028. A lost notification dropped a message chunk or a command output chunk. The reader now splits at \n only. It decodes UTF-8 across chunks with a StringDecoder and searches only the new text, so a long line still costs linear time. A final line without a line break is read at the end of the stream. A line that is not JSON is now logged, so a lost message is visible. The session that hung now loads its 1753 items in 88 ms.
A history page request had no timeout, so a lost response made session/load wait forever. A page takes milliseconds. A page request now fails after 60 s with an error that names the method and the thread. The load then fails and closes the session, instead of hanging.
session/list called thread/list without useStateDbOnly, so Codex scanned and repaired every rollout file on each call. On a local store of 92 sessions that took 3.0 to 3.7 s for 4 pages. From the state DB, the same list takes 9 to 11 ms. A comparison of the two answers found the same titles, previews and update times. The state DB also returned 3 VS Code sessions that the scan dropped, and it returns the real path of a cwd behind a symbolic link, for example /private/var instead of /var. An empty page also ran three more thread/list calls for diagnostics, each with a full scan. They only logged counts, so they are removed.
A close during session/load did not stop the replay. The adapter read every remaining history page, also of the subagents, and sent updates for the closed session. For a session of 10k items that is about 100 extra thread/items/list requests. The replay now checks the session generation at each page and at each item. A close stops the read and the load fails with "is closing". The load does not close the session a second time.
…closed thread settings The adapter kept the settings of each thread from thread/settings/updated and never removed them. An entry holds the developer instructions of the mode, about 10 KB for Plan mode, and subagent threads add entries too. A resume or a load read the mode from this map, but Codex sends no thread/settings/updated on thread/resume (checked live on Codex 0.156.1). After a restart of the adapter, a loaded Plan session showed the default mode. The resume response holds the mode, so the adapter now reads it there. The close of a thread now removes its settings.
Only tests called it. The tests now call threadReadWithHistory of the app-server client directly.
The title generation sent the whole first message to the title model. A pasted log of 200 KB cost about 50k tokens, and a message larger than the context of the model failed the generation without a title. The adapter now sends the first 4000 characters. It does not split a surrogate pair.
The README said that every client gets the tool call contract and that the goal extension is provider-neutral. Only AIR gets both. Other clients keep the earlier tool call fields. The subagent document said that the adapter always advertises nativeSubagentSessions. It advertises the key to AIR only. The comment of DIFF_PATCH_MAX_BYTES said that a larger change uses the standard ACP diff. The file change reporter sends no diff for it. The comment of commandActionFacts named the deleted history fallback. The history page constants now sit above the class comment, so the comment documents the class again.
CommandEnd.replay was set but never read. FileChangeReporter.started is synchronous, so the callers do not await it. renderFacts accepted null, but no caller passes null, so the handler calls the renderer directly. The Git patch builders no longer check the source size. The file change reporter drops a Codex diff over the limit before it calls a builder, and the builder still checks the size of the patch with its headers. The patch test no longer measures time. The MCP progress comment said that the empty report stops the progress for AIR. The event handler stops it, so the comment now says so. Nine exports had no importer outside their module. They are now module-private.
The guardian reporter built the verdict text in two functions that differ only in the Action line. One function now builds both texts. The event handler and the server built the goal session_info_update in the same way. goalSessionInfoUpdate in ThreadGoalSnapshot.ts now builds it for both. Four reporters mapped a finished item status to completed or failed in place. toTerminalToolStatus in ToolStatus.ts now does it. Four AIR key constants lived in the modules that use them. They now live in AirExtension.ts with the other AIR keys. ToolCallReports now imports isRecord instead of a copy.
ClientCapabilities.with was called only from tests. The tests now build the capabilities with ClientCapabilities.from, as the server does. The approval handler, the elicitation handler, the event handler and the subagent router had default parameters that only tests used. The server passes every argument, so the defaults are removed. The tests now pass the arguments. createTestEventHandler builds an event handler for a test.
Three loops read a cursor-paged history: threadItemPages, the turn pages of threadReadWithHistory and readSessionTurnItems. Only the first had a timeout. A lost response made the other two wait forever, so a session/load or a subagent replay could hang. historyPages now holds the loop for all three. It applies the page timeout and the repeated-cursor check. threadTurnPages exposes it for thread/turns/list. The first request of threadReadWithHistory also has the timeout now.
Two history fallback fixtures lost their last reader with the deleted history fallback. The MCP log fixture named a home directory of a developer. It now names /workspace.
Six test files of this branch had their own copy of deferred. They now import one copy from acp-test-utils.
A child that ends before Codex reports its activity drops its buffered updates. No test checked this. The new test ends such a child, reopens it, and checks that the ended spawn keeps no buffer and that nothing is replayed. The process exit test starts a real process. In a full serial run it once took more than the default 5 s. It now has 15 s.
AIR shows a command that reads one file as the viewed file. That view does not show the text, and AIR stored the rawOutput in the session log in full. The completion for AIR now leaves the output of a file read out. A search or a list has no path and keeps its output. Other clients keep the output.
The plain and the Zed baselines of 27 of the 33 scenarios were the same file. The Zed baseline now exists only where its messages differ, and the test reads the plain baseline otherwise. The baseline writer applies the same rule. .gitattributes marks the scenario recordings as generated, so a pull request diff folds them.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.


Summary
One PR with separate commits (the earlier stack #544 and #545 is folded back in).
Diff patch
diffPatchAIR capability and send a file change as one compact Git patch only to a client that declares it.src/GitPatch.tsbuilds valid patches: paths and quoting, rename headers, file modes, CRLF, the exact\ No newline at end of filemarker, and hunks whose starts and counts are in order without overlap. Anything else falls back to the standard ACP diff.For every client
Clients that do not declare
_meta.jetbrains.air(Zed, plain ACP clients) get the same information as onmain.tool_call_updateomits top-level fields that did not change since the last report of the same tool call (ACP merge semantics). The_metaof each report stays complete.kindof a started tool call, an empty fuzzy search clears old locations, the subagent buffer keeps every notification and counts its cap in serialized bytes.For AIR only
rawInput, the file text of an edit only in the diff, command output only throughterminal_output_delta, stdin interminal_input, no display copies of the input (rawInputRendering), no MCP progress lines.plan_updatewith_meta.jetbrains.air.contentDelta(planContentDelta)._meta.jetbrains.air.*and are no longer sent to other clients (hencefeat!; only AIR reads them).Structure and docs
src/tool-calls/: one reporter per tool kind, oneClientCapabilities, oneAcpToolCallRenderer, the changed-field filter.docs/air-extensions.mdreplaces the per-extension AIR docs.Tests
npm run typecheck,npm test: 976 passed._auth/status_update); standard ACP messages are validated against the ACP JSON schema, AIR extension updates get an envelope check; plain and Zed are compared with a recordedmainbaseline in code; AIR keeps compact golden recordings.