From 62ea5dd6dcdcc5b5d64d048942f0e1a87afc7ac4 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 01:03:12 -0700 Subject: [PATCH 1/4] docs(plans): Codex ghosts matched by provider item id (#1231) Co-Authored-By: Claude Opus 5.5 --- .../2026-09-27-codex-ghost-item-identity.md | 27 +++++++++++++++++++ 1 file changed, 27 insertions(+) create mode 100644 docs/plans/2026-09-27-codex-ghost-item-identity.md diff --git a/docs/plans/2026-09-27-codex-ghost-item-identity.md b/docs/plans/2026-09-27-codex-ghost-item-identity.md new file mode 100644 index 000000000..7fe8b53cf --- /dev/null +++ b/docs/plans/2026-09-27-codex-ghost-item-identity.md @@ -0,0 +1,27 @@ +# Codex text ghosts are never superseded (#1231) + +## Evidence (verified 2026-09-27) +- **The mismatch.** Codex ghosts are keyed by the live semantic turn id, which is the proxy response id (`resp_…`, `ghostsFromSemanticTurn` → `ghostUuid(turn.turnId, blockIndex)`). + - `reconcileUpstream` matches Codex entries on `codexTurnId`, but the mapper stamps that from `turn_context.turn_id`, a rollout UUID (`providers/codex/renderer/transcript/mapper.ts`, `stampCodexTurnId`). It never equals a `resp_…` id, so that branch never matches. + - Text blocks have no tool id for the fallback, so every Codex text ghost orphans after 30 s. It stays hidden only by render rules 3/4 in `selectMergedEntries`. +- **The two ids that do agree.** Every rollout `response_item` carries the provider item id (`msg_…`, `rs_…`, `ctc_…`, `fc_…`), and the live block already has it: `CodexResponsesAdapter` emits `itemId` on `block_started`, and `foldEvent` keeps it on `SemanticLiveBlock.itemId`. + - Checked on 6 real Agent Code 0.157.1 sessions (proxy dumps against their rollouts, 2026-09-27): every `msg_…` id seen on the proxy stream, 45 of 45, appears verbatim in the rollout. +- **The other route and why not.** `token_usage_record` rows carry `{turn_id, response_id}` after each response's items, which would allow a retroactive match by response. The item id is exact per block and needs no lookahead, so it is the route taken (issue comment, route 2). + +## Change +- `mapCodexRolloutToFeedEntries` stamps `codexItemId` (the rollout item's `payload.id`) on every entry it maps from a `response_item`. This is an Agent Code-local field, like `codexTurnId`. +- `ghostContextForBlock` records the block's `itemId` in the ghost's `context`, atp's free-form slot. No package change is needed. +- `reconcileUpstream` supersedes a ghost when `context.itemId === entry.codexItemId`. +- The `codexTurnId === ghost.turnId` branch compares a rollout UUID with a response id, so it can never match. It is removed, and the stale comments that claim the mapper stamps the response id are corrected. + +## Tests +A recorded pair from a real 0.157.1 session: the response's SSE frames for one assistant message (`response.created` id, `output_item.added`/`delta`/`done`) and the rollout `response_item` for the same `msg_…` id. The chain: +1. The frames drive the real `CodexResponsesAdapter` (codex-headless adapter harness) and the real `foldSemanticEvent`. +2. The ghost is minted by `ghostsFromSemanticTurn`. +3. The rollout record goes through the real mapper and `reconcileUpstream`. + +The ghost must be superseded by that entry's uuid. It is red on main. A second rollout message with a different id must NOT supersede it. + +## Out of scope +- Reasoning ghosts get the same match for free, where a mapped entry exists. +- The `token_usage_record` route (parser#38 tracks the type itself). From f8279536344ddb2876a53aed154358d73b04ebb2 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 01:12:45 -0700 Subject: [PATCH 2/4] fix(ghosts): supersede Codex ghosts by provider item id A Codex ghost is keyed by the proxy response id (resp_...), while the committed rollout entry only carried the rollout turn UUID (codexTurnId). reconcileUpstream compared the two, which can never match. Text blocks have no tool id to fall back on, so every Codex text ghost orphaned after 30 s and stayed hidden only by render rules 3 and 4. Both sides do carry the provider item id (msg_/rs_/fc_/ctc_): the live block has it from the proxy adapter, and the rollout has it on every response_item. On six real 0.157.1 sessions, all 45 msg_ ids seen on the proxy were present verbatim in their rollouts. The mapper now stamps codexItemId, the ghost records its block's itemId in its context, and reconcileUpstream matches on it. The dead turn-id branch and the stale comments claiming the mapper stamped the response id are gone. Fail-first: a real recorded message (proxy SSE frames plus the rollout record) driven through the real adapter, fold, mapper and reconciler. Red on main; each half of the fix is pinned by mutation. Fixes #1231 Co-Authored-By: Claude Opus 5.5 --- .../codex/renderer/transcript/rollout.ts | 45 ++- src/renderer/src/features/feed/ui/Feed.tsx | 7 +- .../ghosts.codexItemIdentity.test.ts | 52 +++ src/renderer/src/session-runtime/ghosts.ts | 44 +-- .../ghosts/codex/message-item-identity.json | 325 ++++++++++++++++++ tsconfig.web.json | 3 + 6 files changed, 443 insertions(+), 33 deletions(-) create mode 100644 src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts create mode 100644 testing/fixtures/ghosts/codex/message-item-identity.json diff --git a/src/providers/codex/renderer/transcript/rollout.ts b/src/providers/codex/renderer/transcript/rollout.ts index 15debb708..0bd7ca9e9 100644 --- a/src/providers/codex/renderer/transcript/rollout.ts +++ b/src/providers/codex/renderer/transcript/rollout.ts @@ -44,9 +44,10 @@ type CodexMessageBlock = // // Ordering with `stampCodexTurnId`: Codex rollout doesn't put // `turn_id` on per-item entries — only on `task_started` / -// `turn_started` and `turn_context`. Without tracking it here the -// ghost reconciler has nothing to match Codex assistant-text ghosts -// against (they don't carry `message.id`, don't carry `tool_use_id`). +// `turn_started` and `turn_context`. The feed's committed-text keys +// (renderUnits.ts) use it. Ghost reconciliation does NOT: a ghost is keyed +// by the proxy response id, which is not this turn UUID. Ghosts match on +// the item id instead (`stampCodexItemId`, #1231). // Codex injects two synthetic user messages on the first turn of // every conversation, both meant for the model and not the human: @@ -228,17 +229,17 @@ function codexCompactSummaryEntry( } /** - * Extract the Codex rollout's per-turn response id from a - * `turn_context` side-channel entry. Returns null for any other + * Extract the Codex rollout's per-turn id (a UUID, NOT the proxy response + * id) from a `turn_context` side-channel entry. Returns null for any other * entry type. Call sites that iterate a rollout stream use this to * keep a rolling "current turn id" that subsequent `response_item` * entries get stamped with via `stampCodexTurnId`. * * WHY: Codex rollout doesn't put `turn_id` on per-item entries — - * only on `task_started`/`turn_started` and `turn_context`. Without - * tracking it here the ghost reconciler in `reconcileUpstream` has - * nothing to match Codex assistant-text ghosts against (they don't - * carry `message.id`, don't carry `tool_use_id`). + * only on `task_started`/`turn_started` and `turn_context`. The feed's + * committed-text keys use it. It is NOT the id a Codex ghost is keyed by + * (that is the proxy response id), so `reconcileUpstream` matches ghosts + * by item id instead (#1231). */ export function codexTurnIdFromRollout(entry: Record): string | null { if (entry.type !== 'turn_context') return null @@ -257,7 +258,33 @@ export function stampCodexTurnId(entry: Entry, turnId: string | null): Entry { return { ...entry, codexTurnId: turnId } as Entry } +/** + * Stamp mapped Codex feed entries with the provider ITEM id (`msg_…`, `rs_…`, + * `fc_…`, `ctc_…`) of the rollout `response_item` they came from. + * + * WHY (#1231): this is the one id the live stream and the rollout share. A + * Codex ghost is keyed by the proxy response id (`resp_…`), while the rollout + * knows each item's id and its turn UUID, never the response id. So no + * committed entry could supersede a Codex text ghost, and every one orphaned. + * The proxy adapter already carries the same item id on the live block + * (`SemanticLiveBlock.itemId`); `reconcileUpstream` matches on it. It was + * verified on six real 0.157.1 sessions: every `msg_…` id on the proxy stream + * appears verbatim in the rollout. An Agent Code-local field like + * `codexTurnId`; consumers that don't care ignore it. + */ +export function stampCodexItemId(entry: Entry, itemId: string | null): Entry { + if (itemId === null) return entry + return { ...entry, codexItemId: itemId } as Entry +} + export function mapCodexRolloutToFeedEntries(entry: Record): Entry[] { + const mapped = mapCodexRolloutToFeedEntriesUnstamped(entry) + if (entry.type !== 'response_item' || mapped.length === 0) return mapped + const itemId = stringField(asRecord(entry.payload), 'id') + return itemId === null ? mapped : mapped.map(mappedEntry => stampCodexItemId(mappedEntry, itemId)) +} + +function mapCodexRolloutToFeedEntriesUnstamped(entry: Record): Entry[] { const payload = asRecord(entry.payload) // Shared with the marker below and main's history loader (#1288): the old // `ts:id|call_id|type` rule collided for a call and its output, and for diff --git a/src/renderer/src/features/feed/ui/Feed.tsx b/src/renderer/src/features/feed/ui/Feed.tsx index 384a98360..0be4a571c 100644 --- a/src/renderer/src/features/feed/ui/Feed.tsx +++ b/src/renderer/src/features/feed/ui/Feed.tsx @@ -258,10 +258,9 @@ export type FeedProps = Props // // That duplicate class is now prevented at its source. The ghost // reducer (`reconcileUpstream` in src/renderer/src/session-runtime/ghosts.ts) -// supersedes Codex text ghosts by rollout response id once the -// rollout mapper stamps `codexTurnId` on committed entries -// (src/renderer/src/workspace/workspaceStore.ts::codexTurnIdFromRollout -// + stampCodexTurnId). The live view and the merged feed are split +// supersedes Codex text ghosts by provider item id, which the rollout +// mapper stamps on committed entries as `codexItemId` (#1231; the earlier +// turn-id match could never succeed). The live view and the merged feed are split // by turn ownership (src/renderer/src/session-runtime/mergedEntries.ts), so // there is no longer any path by which the same assistant text can // reach both surfaces at once. diff --git a/src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts b/src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts new file mode 100644 index 000000000..14ca318df --- /dev/null +++ b/src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts @@ -0,0 +1,52 @@ +import { describe, expect, it } from 'vitest' + +import fixture from '../../../../testing/fixtures/ghosts/codex/message-item-identity.json' + +import { ghostsFromSemanticTurn, reconcileUpstream } from '@renderer/session-runtime/ghosts' +import { mountCodexPane } from '@renderer/session-runtime/semantic/testing/proxyPaneDrivers' +import type { Frame } from '@renderer/session-runtime/semantic/testing/proxyPaneDrivers' +import { mapCodexRolloutToFeedEntries } from '@providers/codex/renderer/transcript/rollout' + +// #1231: a Codex text ghost is keyed by the proxy response id (`resp_…`), and +// the committed rollout entry never carried anything that equals it, so no +// Codex text ghost was ever superseded. Both sides do carry the provider ITEM +// id (`msg_…`): the proxy stream on output_item.added, the rollout on the +// response_item. The fixture is one real 0.157.1 assistant message seen from +// both sides (see its `evidence`). It drives the REAL codex proxy adapter and +// the REAL semantic fold, as the desktop hook does, then the REAL rollout +// mapper. +function ghostFromRecordedStream() { + const pane = mountCodexPane() + pane.request('req-1') + pane.frames('req-1', fixture.sseFrames as Frame[]) + const turn = pane.reducer.pane.semantic.currentTurn + if (!turn) throw new Error('recorded frames opened no semantic turn') + const ghosts = ghostsFromSemanticTurn(turn, 'recorded-session', new Map()) + const textGhosts = [...ghosts.values()].filter(ghost => + Array.isArray(ghost.message.content) && + ghost.message.content.some(block => (block as { type?: string }).type === 'text')) + return { turn, ghosts, textGhosts } +} + +describe('Codex text ghosts are superseded by their committed rollout entry (#1231)', () => { + it('supersedes the recorded message ghost when the rollout record for the same item lands', () => { + const { turn, ghosts, textGhosts } = ghostFromRecordedStream() + // The ghost is keyed by the response id; the rollout knows only the item. + expect(turn.turnId).toBe((fixture.sseFrames[0] as { response: { id: string } }).response.id) + expect(textGhosts).toHaveLength(1) + + const [entry] = mapCodexRolloutToFeedEntries(fixture.rolloutRecord as Record) + expect(entry?.uuid).toBeTruthy() + const next = reconcileUpstream(entry!, ghosts) + expect(next.get(textGhosts[0]!.uuid)?._atp.supersededBy).toBe(entry!.uuid) + }) + + it('does not supersede it with a different message', () => { + const { ghosts, textGhosts } = ghostFromRecordedStream() + const other = structuredClone(fixture.rolloutRecord) as { payload: { id: string } } + other.payload.id = other.payload.id.replace(/.$/, c => (c === '0' ? '1' : '0')) + const [entry] = mapCodexRolloutToFeedEntries(other as unknown as Record) + const next = reconcileUpstream(entry!, ghosts) + expect(next.get(textGhosts[0]!.uuid)?._atp.supersededBy).toBeUndefined() + }) +}) diff --git a/src/renderer/src/session-runtime/ghosts.ts b/src/renderer/src/session-runtime/ghosts.ts index 7d057d388..9cf76ac23 100644 --- a/src/renderer/src/session-runtime/ghosts.ts +++ b/src/renderer/src/session-runtime/ghosts.ts @@ -12,7 +12,7 @@ // `ClaudeEntry` records via atp's `createGhost`. When the // authoritative JSONL entry lands (Claude's batched 100 ms drain; // Codex's mpsc flush), `reconcileUpstream` matches by message.id / -// codexTurnId / tool_use_id and supersedes the ghost. If JSONL +// Codex item id / tool_use_id and supersedes the ghost. If JSONL // never matches (Claude Code's auxiliary calls — title gen, // predict-next-prompt, branch-name gen — are not written to the // rollout), `orphanStale` flags the ghost after the TTL. @@ -37,11 +37,14 @@ // so one upstream entry supersedes every ghost block for that // turn at once. // -// Codex: rollout emits one entry per content block, with the -// rollout response_id stamped onto the mapped entry by -// `stampCodexTurnId` in ../codex/rollout.ts. Match is by -// (turnId, blockIndex). When that fails, both providers fall back -// to tool_use_id / call_id pairing for tool blocks. +// Codex: the ghost's turnId is the proxy response id (`resp_…`), which +// the rollout never records, so turn-level matching is impossible +// (#1231). Instead each ghost records its block's provider item id +// (`msg_…`, `rs_…`, `fc_…`) in `context.itemId`, and the rollout mapper +// stamps the same id on each committed entry as `codexItemId` +// (`stampCodexItemId` in providers/codex/renderer/transcript/rollout.ts). +// Match is by that id, one block at a time. Tool blocks can also still +// pair by tool_use_id / call_id. // // ----------------------------------------------------------------------------- // Reference stability @@ -341,6 +344,9 @@ function ghostContextForBlock( if (block.messagePhase) out.messagePhase = block.messagePhase if (block.toolUseId) out.toolUseId = block.toolUseId if (block.callId) out.callId = block.callId + // The Codex provider item id (#1231): the only id the live block and + // its committed rollout entry share. `reconcileUpstream` matches on it. + if (block.itemId) out.itemId = block.itemId return out } @@ -396,15 +402,13 @@ export function reconcileUpstream( typeof messageRecord?.id === 'string' ? messageRecord.id : null - // Codex rollout-sourced entries don't carry message.id (the Codex - // response id lives elsewhere on the rollout payload). Plumbing it - // through the mapper to this matcher is Task 6 of the rendering- - // fixes plan; the field is read defensively here so the match path - // lights up the moment `mapCodexRolloutToFeedEntries` stamps it. + // Codex rollout-sourced entries carry no message.id and never the + // proxy response id a Codex ghost is keyed by. They do carry the + // provider item id, stamped by the rollout mapper (#1231). const codexRecord = asRecord(entry) - const codexTurnId = - typeof codexRecord?.codexTurnId === 'string' - ? codexRecord.codexTurnId + const codexItemId = + typeof codexRecord?.codexItemId === 'string' + ? codexRecord.codexItemId : null // Gather tool_use ids carried by this upstream entry — used for @@ -428,12 +432,12 @@ export function reconcileUpstream( // Claude match by message.id → turnId equality. if (messageId && ghost._atp.turnId === messageId) match = true - // Codex match by response id → turnId equality. Ghosts are minted - // with `turnId = responseId` when the live source is Codex rollout, - // so a committed entry carrying the same responseId supersedes - // every ghost for that turn in one shot — matching the Claude - // message.id contract. - if (!match && codexTurnId && ghost._atp.turnId === codexTurnId) match = true + // Codex match by provider item id (#1231). This replaced a + // `codexTurnId === ghost.turnId` comparison that could never be + // true: the entry's codexTurnId is the rollout turn UUID, and the + // ghost's turnId is the proxy response id. Every Codex text ghost + // therefore orphaned. The item id is exact per block. + if (!match && codexItemId && ghost._atp.context?.itemId === codexItemId) match = true // Shared: tool_use id equality. Works for both providers; wins // over message-id in ambiguous cases (the ghost knows the exact diff --git a/testing/fixtures/ghosts/codex/message-item-identity.json b/testing/fixtures/ghosts/codex/message-item-identity.json new file mode 100644 index 000000000..2d678fa04 --- /dev/null +++ b/testing/fixtures/ghosts/codex/message-item-identity.json @@ -0,0 +1,325 @@ +{ + "evidence": "A real codex-cli 0.157.1 Agent Code session (2026-09-27): the SSE frames of one assistant commentary message as the proxy saw them (response.created id, output_item.added, text deltas, output_item.done), and the rollout response_item for the same msg_ id. Verbatim; the text is a one-line reviewer commentary. The same provider item id is on both sides.", + "sseFrames": [ + { + "type": "response.created", + "response": { + "id": "resp_0026e3aaf911d31d016ab8cd41842887d0ae8dfc0e472b6154" + } + }, + { + "type": "response.output_item.added", + "item": { + "id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "type": "message", + "status": "in_progress", + "content": [], + "internal_chat_message_metadata_passthrough": { + "turn_id": "01a0e1e1-bff7-76c1-bae1-5640298cdbb0" + }, + "phase": "commentary", + "role": "assistant", + "metadata": { + "turn_id": "01a0e1e1-bff7-76c1-bae1-5640298cdbb0" + } + }, + "output_index": 1, + "sequence_number": 4 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": "I", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "s3NGFSUEXsyAqDD", + "output_index": 1, + "sequence_number": 6 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": "’ll", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "JzPKfZkfTDmyp", + "output_index": 1, + "sequence_number": 7 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " read", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "6RIQnoxrKMM", + "output_index": 1, + "sequence_number": 8 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " the", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "q2MRS3wAC5Ox", + "output_index": 1, + "sequence_number": 9 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " review", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "iIit1lmNW", + "output_index": 1, + "sequence_number": 10 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " brief", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "uMOhhOTFsv", + "output_index": 1, + "sequence_number": 11 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " and", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "qRVlraF17yr4", + "output_index": 1, + "sequence_number": 12 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " project", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "zRB0TMCm", + "output_index": 1, + "sequence_number": 13 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " conventions", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "hIB9", + "output_index": 1, + "sequence_number": 14 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": ",", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "yrKplQgvqroXEyC", + "output_index": 1, + "sequence_number": 15 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " inspect", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "KdFpeHqR", + "output_index": 1, + "sequence_number": 16 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " the", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "ofZUo4dJfHzc", + "output_index": 1, + "sequence_number": 17 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " requested", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "AFdKPR", + "output_index": 1, + "sequence_number": 18 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " changes", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "1LONkU7R", + "output_index": 1, + "sequence_number": 19 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": ",", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "LzcPLJEjqbUJ8C1", + "output_index": 1, + "sequence_number": 20 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " and", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "12yRzcEfsL9u", + "output_index": 1, + "sequence_number": 21 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " write", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "etZCPCA3QZ", + "output_index": 1, + "sequence_number": 22 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " the", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "ZD8aHXsKqqtf", + "output_index": 1, + "sequence_number": 23 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " report", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "c7xgWpNbh", + "output_index": 1, + "sequence_number": 24 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " at", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "tuFfhLkkWrK3D", + "output_index": 1, + "sequence_number": 25 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " the", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "ZjzDEPb5P9SM", + "output_index": 1, + "sequence_number": 26 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " specified", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "BWeh47", + "output_index": 1, + "sequence_number": 27 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": " path", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "iOZhG76UfQX", + "output_index": 1, + "sequence_number": 28 + }, + { + "type": "response.output_text.delta", + "content_index": 0, + "delta": ".", + "item_id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "logprobs": [], + "obfuscation": "5q71z1jNW2LcmOY", + "output_index": 1, + "sequence_number": 29 + }, + { + "type": "response.output_item.done", + "item": { + "id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "type": "message", + "status": "completed", + "content": [ + { + "type": "output_text", + "annotations": [], + "logprobs": [], + "text": "I’ll read the review brief and project conventions, inspect the requested changes, and write the report at the specified path." + } + ], + "internal_chat_message_metadata_passthrough": { + "create_time": 1790496065.682551, + "turn_id": "01a0e1e1-bff7-76c1-bae1-5640298cdbb0" + }, + "phase": "commentary", + "role": "assistant", + "metadata": { + "turn_id": "01a0e1e1-bff7-76c1-bae1-5640298cdbb0" + } + }, + "output_index": 1, + "sequence_number": 32 + } + ], + "rolloutRecord": { + "timestamp": "2026-09-27T08:01:06.814Z", + "ordinal": 14, + "type": "response_item", + "payload": { + "type": "message", + "id": "msg_0026e3aaf911d31d016ab8cd426c9487d09f76109642b72ed6", + "role": "assistant", + "content": [ + { + "type": "output_text", + "text": "I’ll read the review brief and project conventions, inspect the requested changes, and write the report at the specified path." + } + ], + "phase": "commentary", + "internal_chat_message_metadata_passthrough": { + "turn_id": "01a0e1e1-bff7-76c1-bae1-5640298cdbb0", + "create_time": 1790496065.682551, + "content_item_kinds": [ + "unknown" + ] + } + }, + "metadata": { + "client_authored": false, + "user_input_order": 1 + } + } +} \ No newline at end of file diff --git a/tsconfig.web.json b/tsconfig.web.json index 264b218b2..14b9f1e38 100644 --- a/tsconfig.web.json +++ b/tsconfig.web.json @@ -98,6 +98,9 @@ // the authoritative typecheck validate those evidence contracts instead // of forcing tests to duplicate captured structures inline. "testing/fixtures/rendering-shapes/**/*.json", + // Recorded Codex ghost-identity pairs (#1231): proxy SSE frames and the + // rollout record for the same provider item, imported by the ghost tests. + "testing/fixtures/ghosts/**/*.json", // The phone client is browser code type-checked here (react-jsx, DOM // lib) even though it builds through its own plain-Vite config — the // tsc gate must cover it or client drift ships silently. From 2609c3acccd46f614318187455ab48d33aec3217 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 01:49:36 -0700 Subject: [PATCH 3/4] docs(ghosts): correct three comments that still described the turn-id match Review a of #1383: rollout.ts, ghosts.ts and useIpcSubscriptions.ts still said Codex ghosts reconcile by (turnId, blockIndex) or by turn id. They reconcile by provider item id now, and codexTurnId's remaining consumer is renderUnits.ts. Refs #1231 Co-Authored-By: Claude Opus 5.5 --- src/providers/codex/renderer/transcript/rollout.ts | 12 +++++++----- src/renderer/src/session-runtime/ghosts.ts | 9 ++++----- .../src/workspace/hook/ipc/useIpcSubscriptions.ts | 4 ++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/src/providers/codex/renderer/transcript/rollout.ts b/src/providers/codex/renderer/transcript/rollout.ts index 0bd7ca9e9..395ce7458 100644 --- a/src/providers/codex/renderer/transcript/rollout.ts +++ b/src/providers/codex/renderer/transcript/rollout.ts @@ -247,11 +247,13 @@ export function codexTurnIdFromRollout(entry: Record): string | } /** - * Stamp a mapped Codex feed entry with the rollout turn id so the - * ghost reconciler can supersede by turn id. The field is added as - * an Agent Code-local extension to the shared `Entry` type via cast — - * consumers that don't care about it ignore it, and - * `reconcileUpstream` reads it defensively. + * Stamp a mapped Codex feed entry with the rollout turn id. Its consumer is + * the feed's committed-text ownership keys (features/feed/ui/semantic/ + * renderUnits.ts). The ghost reconciler does NOT use it: a ghost is keyed by + * the proxy response id, never this turn UUID, so ghosts match on + * `codexItemId` instead (`stampCodexItemId`, #1231). The field is an Agent + * Code-local extension to the shared `Entry` type via cast; consumers that + * don't care about it ignore it. */ export function stampCodexTurnId(entry: Entry, turnId: string | null): Entry { if (turnId === null) return entry diff --git a/src/renderer/src/session-runtime/ghosts.ts b/src/renderer/src/session-runtime/ghosts.ts index 9cf76ac23..d08d0858b 100644 --- a/src/renderer/src/session-runtime/ghosts.ts +++ b/src/renderer/src/session-runtime/ghosts.ts @@ -366,11 +366,10 @@ function ghostContextForBlock( * landing validates all provisional blocks for that turn. * * Codex: rollout emits one entry per content block with its own - * uuid. We match by (turnId, blockIndex). turnId for Codex - * comes from the response id when available; Agent Code's - * Codex ingest path already exposes it on the mapped - * entry. When the mapping is not available we fall back - * to tool_use id pairing for tool blocks. + * uuid, and never the proxy response id a ghost is keyed by. + * We match one block at a time on the provider item id: the + * ghost's `context.itemId` against the entry's `codexItemId` + * (#1231). Tool blocks can also pair by tool_use id. * * Both: if the upstream entry has a tool_use block whose id * matches a ghost's `context.toolUseId`, supersede that diff --git a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts index 7c48f18fa..758b7f375 100644 --- a/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts +++ b/src/renderer/src/workspace/hook/ipc/useIpcSubscriptions.ts @@ -2321,8 +2321,8 @@ export function useIpcSubscriptions( : current.limitHit // Ghost reconciliation — when authoritative entries land, - // supersede any live ghost whose `(turnId, blockIndex)` - // they replace. Runs per appended entry so ghost→real + // supersede any live ghost they replace (Claude: message id; + // Codex: provider item id, #1231; either: tool_use id). Runs per appended entry so ghost→real // handoff is synchronous with the entry becoming visible; // the ghost drops out of the merged view in the same // render as the real entry appears. From b26cc02ea790e7ac5fccf5208bd711b80e7ac508 Mon Sep 17 00:00:00 2001 From: Julius Olsson Date: Sun, 27 Sep 2026 02:15:12 -0700 Subject: [PATCH 4/4] test(ghosts): drive the production mapper; say reasoning ghosts are not matched Review c of #1383: the header listed rs_ ids as matched, but reasoning items map to no feed entry, so reasoning ghosts still orphan as before. The identity test now maps through createCodexTranscriptEntryMapper, the wrapper all three production ingest surfaces use. Refs #1231 Co-Authored-By: Claude Opus 5.5 --- .../ghosts.codexItemIdentity.test.ts | 13 +++++++++---- src/renderer/src/session-runtime/ghosts.ts | 6 ++++-- 2 files changed, 13 insertions(+), 6 deletions(-) diff --git a/src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts b/src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts index 14ca318df..9cf4ee9fe 100644 --- a/src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts +++ b/src/renderer/src/session-runtime/ghosts.codexItemIdentity.test.ts @@ -5,7 +5,7 @@ import fixture from '../../../../testing/fixtures/ghosts/codex/message-item-iden import { ghostsFromSemanticTurn, reconcileUpstream } from '@renderer/session-runtime/ghosts' import { mountCodexPane } from '@renderer/session-runtime/semantic/testing/proxyPaneDrivers' import type { Frame } from '@renderer/session-runtime/semantic/testing/proxyPaneDrivers' -import { mapCodexRolloutToFeedEntries } from '@providers/codex/renderer/transcript/rollout' +import { createCodexTranscriptEntryMapper } from '@providers/codex/renderer/transcript/mapper' // #1231: a Codex text ghost is keyed by the proxy response id (`resp_…`), and // the committed rollout entry never carried anything that equals it, so no @@ -14,7 +14,12 @@ import { mapCodexRolloutToFeedEntries } from '@providers/codex/renderer/transcri // response_item. The fixture is one real 0.157.1 assistant message seen from // both sides (see its `evidence`). It drives the REAL codex proxy adapter and // the REAL semantic fold, as the desktop hook does, then the REAL rollout -// mapper. +// mapper, through the same stateful wrapper all three production ingest +// surfaces use (review c: a bare mapper call would stay green if stamping +// moved into the wrapper). +const mapRollout = (record: unknown) => + createCodexTranscriptEntryMapper().map(record as Record).entries + function ghostFromRecordedStream() { const pane = mountCodexPane() pane.request('req-1') @@ -35,7 +40,7 @@ describe('Codex text ghosts are superseded by their committed rollout entry (#12 expect(turn.turnId).toBe((fixture.sseFrames[0] as { response: { id: string } }).response.id) expect(textGhosts).toHaveLength(1) - const [entry] = mapCodexRolloutToFeedEntries(fixture.rolloutRecord as Record) + const [entry] = mapRollout(fixture.rolloutRecord) expect(entry?.uuid).toBeTruthy() const next = reconcileUpstream(entry!, ghosts) expect(next.get(textGhosts[0]!.uuid)?._atp.supersededBy).toBe(entry!.uuid) @@ -45,7 +50,7 @@ describe('Codex text ghosts are superseded by their committed rollout entry (#12 const { ghosts, textGhosts } = ghostFromRecordedStream() const other = structuredClone(fixture.rolloutRecord) as { payload: { id: string } } other.payload.id = other.payload.id.replace(/.$/, c => (c === '0' ? '1' : '0')) - const [entry] = mapCodexRolloutToFeedEntries(other as unknown as Record) + const [entry] = mapRollout(other) const next = reconcileUpstream(entry!, ghosts) expect(next.get(textGhosts[0]!.uuid)?._atp.supersededBy).toBeUndefined() }) diff --git a/src/renderer/src/session-runtime/ghosts.ts b/src/renderer/src/session-runtime/ghosts.ts index d08d0858b..a5e07659f 100644 --- a/src/renderer/src/session-runtime/ghosts.ts +++ b/src/renderer/src/session-runtime/ghosts.ts @@ -40,10 +40,12 @@ // Codex: the ghost's turnId is the proxy response id (`resp_…`), which // the rollout never records, so turn-level matching is impossible // (#1231). Instead each ghost records its block's provider item id -// (`msg_…`, `rs_…`, `fc_…`) in `context.itemId`, and the rollout mapper +// (`msg_…`, `fc_…`, `ctc_…`) in `context.itemId`, and the rollout mapper // stamps the same id on each committed entry as `codexItemId` // (`stampCodexItemId` in providers/codex/renderer/transcript/rollout.ts). -// Match is by that id, one block at a time. Tool blocks can also still +// Match is by that id, one block at a time. Reasoning ghosts (`rs_…`) are +// NOT matched: a reasoning rollout item maps to no feed entry, so they still +// orphan and stay hidden by the render rules, as before. Tool blocks can also still // pair by tool_use_id / call_id. // // -----------------------------------------------------------------------------