Repository navigation
Codex app-server: envelope transcript source (mapper + forward buffer) - #591
Merged
Merged
Conversation
First self-contained slice of the envelope transcript source. Converts codex app-server cumulative token-usage snapshots into per-event deltas so the additive usage pipeline neither double-counts (cumulative → delta) nor mis-attributes across a model reroute (caller attributes each delta to the model resolved at that instant). A lower/reset cumulative total contributes the whole new total; resume supports an exact baseline (thread/read) and a fallback "baseline on next notification, emit nothing". 6 unit tests. Not yet wired into the runtime — that comes with the envelope mapper + IAcpTranscriptSource implementation (the rest of PR2, cross-repo: Ephemeral/ItemId envelope fields + Plan kind land on both kcap-cli and kcap-server). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…t lane) Per-item accumulation of app-server deltas into the cumulative content-so-far that each ephemeral envelope carries (idempotent replacement at the viewer, no increment reassembly); Complete drops an item's transient state once its canonical snapshot is mapped. 3 unit tests. Composes into the mapper (next). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Additive AcpEventEnvelope fields for the codex app-server envelope transcript: Ephemeral (transient live-lane chunk, no seq, never persisted) and ItemId (the app-server item id keying which transient state a completed item supersedes), plus a new Plan envelope kind for turn/plan/updated full snapshots. All additive/default so ContractVersion stays 1 and an older server is unchanged. Wire-compat test pins the new snake_case names and the Plan constant against the kcap-server mirror.
…§2.4) Adds the additive-billing delta lane distinct from the context-occupancy Usage kind: a token_usage kind and nullable UsageInputTokens/UsageCachedInputTokens/ UsageCacheWriteInputTokens/UsageOutputTokens/UsageReasoningTokens fields (model rides the existing Model field). The server stamps these into $usage metadata so the additive folds count them unchanged. All additive/default so ContractVersion stays 1. Wire-compat pins the snake_case names + the token_usage constant.
The app-server TokenUsageBreakdown carries cacheWriteInputTokens (cache-creation tier, billed separately from cached reads); CodexTokenUsage dropped it. Add it to the record, ParseUsage, and the delta converter's reset/subtraction so no billed bucket is silently lost before the mapper stamps $usage. Converter tests move to the 6-bucket shape.
CodexNotificationMapper translates app-server JSON-RPC notifications into the AcpEventEnvelope vocabulary, composing the ephemeral accumulator + usage delta converter. Canonical lane: item/completed snapshots (agentMessage/reasoning/ plan/userMessage/command result/fileChange diff/mcp result), command+mcp tool opens on item/started, turn/plan/updated full snapshots, and per-event token deltas. Ephemeral lane: agentMessage/reasoning/plan/command/fileChange deltas accumulate into content-so-far; patchUpdated replaces (snapshot). Unknown item types surface as a generic tool call and bump a drift counter. Shapes grounded on codex 0.147.0's generated schema; JSON built with Utf8JsonWriter (AOT-safe). CodexTokenUsage gains FromTotal/IsZero. 15 tests.
CodexForwardBuffer is the bounded §2.4 buffer: canonical envelopes are never dropped (a full buffer blocks the emit → the read loop stops consuming stdout → the app-server blocks, lossless), ephemeral envelopes drop when full, and a canonical stall past forwardStallSeconds fires a one-shot terminal fault. The runtime implements IAcpTranscriptSource (thread id / cwd / resolved model / Envelopes) and feeds every notification through the mapper into the buffer, with model/rerouted updating the model-at-instant for token attribution. Emission is gated OFF by default: feeding the buffer with no attached forwarder would stall it, and draining it without the §2.5 dedup would double-ingest a reviewer session — so the surface ships dormant and §2.5 flips one flag with the factory Transcript wiring + dedup guards. Reviewer path byte-unchanged. Buffer + runtime integration tested (gate on and off); AOT publish clean.
PR Summary by QodoAdd gated Codex app-server envelope transcript pipeline
AI Description
Diagram
High-Level Assessment
Files changed (12)
|
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
…rop) - RenderMcpResult prefers error over result so a failed mcp call carrying both a result and an error renders the error, staying consistent with ToolIsError (Copilot finding 2). Regression test added. - CodexForwardBuffer swallows the shutdown-cancellation OperationCanceledException explicitly instead of propagating it out of the read-loop notification handler; the stall-timeout path is unchanged (finding 3). Declined with rationale: fileChange→ToolCall is the §2.4 spec mapping (lone-ToolCall rendering is a §2.6 server concern, finding 1); IsReset any-component-decrease is correct for monotonic cumulative buckets (finding 4); positional CodexTokenUsage insertion is safe — verified zero positional construction sites (all use FromTotal or named args).
Collaborator
Author
Copilot code-review flow — completeCopilot (context-only, cross-repo) returned 5 findings. Outcome: Addressed (commit 7e52e05):
Declined, with rationale:
Copilot also explicitly confirmed clean: the |
…xtensions) Two real bugs fixed: - Reasoning content/summary are arrays of plain STRINGS (pinned schema), not objects with a text field — RenderReasoning read .text off each element and produced an EMPTY canonical reasoning envelope that wrongly superseded the nonempty ephemeral state. New JoinStrings joins the string elements; JoinTexts stays for userMessage (UserInput objects). Test fixtures corrected to string arrays. - initialize always opted out of the agent-message/reasoning/command/file delta notifications, so flipping emitEnvelopeTranscript alone could never activate the ephemeral lane (the app-server never sent them). The opt-out is now empty when the transcript is on, unchanged (perf) when off. Test asserts the empty opt-out. Rule fixes: - McpArguments/RenderMcpResult/IsMcpError drop raw JsonElement.ValueKind checks for the JsonElementExtensions accessors (.Obj/.Str/.Arr) per the repo checklist. - Drop a wire-compat test comment that restated the asserted defaults.
…esult A completed fileChange emitted a lone ToolCall with no matching ToolResult, an asymmetry both Copilot and Kiro flagged (a consumer expecting ToolCall→ToolResult pairing would see an orphan). It now emits a paired ToolCall (carrying the diff) + ToolResult (the apply status, error on failed/declined), mirroring commandExecution and mcp. Tests cover the pair and the failed-apply error flag. Declined with rationale: the read-loop backpressure block is the intentional §2.4 lossless-stall design (bounded by the configurable watchdog); model/rerouted needs no usage-baseline reset because thread/tokenUsage/updated.total is thread-cumulative (monotonic across reroutes) so the model-at-instant attribution is correct and a reset would drop the spanning interval; JoinTexts already handles userMessage's UserInput objects (distinct from reasoning's string arrays).
Collaborator
Author
Kiro code-review flow — complete (third vendor)Ran a third-vendor Kiro review (context-only) asking it to find what Copilot and qodo missed. 5 findings: Addressed (commit 456c3c8):
Declined, with rationale:
Three vendors (Copilot + qodo + Kiro) have now reviewed; all actionable findings are addressed. |
This was referenced Sep 4, 2026
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
1 participant
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.
Second PR of the interactive-hosted-Codex phase (AI-1762) of the app-server epic (AI-1759). Daemon-only; the shipped reviewer path is byte-unchanged because the new envelope emission is gated OFF (§2.5 activates it).
What
Implements the §2.4 envelope transcript for hosted
codex app-serversessions — the daemon translates the app-server's JSON-RPC notification stream into the existingAcpEventEnvelopevocabulary, so hosted Codex sessions can be ingested from the protocol stream (like the other ACP vendors) instead of the hooks +kcap watchrollout path.Ephemeral/ItemId(the ephemeral live lane + the item key a viewer uses to finalize transient state), aPlankind (turn/plan/updatedfull snapshots), and atoken_usagekind with additive bucket fields. All default soContractVersionstays 1 — an older server ignores them. BothAcpEventEnvelopeWireCompatTestspin each other.CodexNotificationMapper— canonical lane =item/completedsnapshots + command/mcp tool opens onitem/started+turn/plan/updated+ per-event token deltas; ephemeral lane = delta notifications accumulated into content-so-far (patchUpdatedreplaces). Unknown item types surface as a generic tool call and bump a drift counter. Shapes grounded on codex 0.147.0's generated JSON schema; JSON built withUtf8JsonWriter(AOT-safe).CodexUsageDeltaConverter/CodexEphemeralAccumulator— the cumulative→delta usage semantics (attributed to the model-at-instant, correct acrossmodel/rerouted) and the per-item cumulative ephemeral content.CodexTokenUsagegainsCacheWriteInputTokens(a billed cache-creation bucket that was being dropped).CodexForwardBuffer— the bounded §2.4 buffer: canonical envelopes are never dropped (a full buffer blocks the emit → the read loop stops consuming stdout → the app-server blocks, which is lossless); ephemeral envelopes drop when full; a canonical stall pastforwardStallSecondsfires a one-shot terminal fault (never an indefinite wedge, never a silently incomplete transcript).IAcpTranscriptSourceand feeds every notification through the mapper into the buffer.Why gated off
The emission is behind
emitEnvelopeTranscript(default false). Feeding the buffer with no attached forwarder would stall it, and draining it without the §2.5 hooks/watch dedup would double-ingest a reviewer session. So this PR ships the transcript surface dormant; §2.5 flips the one flag together with the factoryTranscriptwiring and the guard-1/2 dedup. A regression test pins the dormant behavior.Tests
Usage converter (6), ephemeral accumulator (3), notification mapper (15), forward buffer (5, incl. backpressure + stall), runtime integration (gate on and off), both wire-compat suites. Daemon AOT publish clean.
Linear: AI-1762