fix(codex): same-millisecond rollout items are no longer dropped (#1288)

docs/plans/2026-09-25-codex-rollout-identity.md

@@ -0,0 +1,20 @@
+# Codex rollout entries need a unique identity (#1288)
+
+## Evidence
+- `rollout.ts:260-262` builds the uuid as `${timestamp}:${payload.id ?? call_id ?? payload.type}`, and `historyLoader.ts` `extractCodexHistoryMarker` duplicates the rule in main.
+- A `function_call` and its `function_call_output` in the same millisecond share `ts:call_id`. Real example: 2026-04-28 rollout, `exec_command df -h .` and its output, both at 16:36:01.649Z.
+- Id-less items (`message`, `event_msg` types) in the same millisecond share `ts:<type>`. Real example: 2026-04-16 14:34:49.436Z, a user message and an assistant message.
+- `committedRecords.ts:100-111` admits by uuid and drops the second one, permanently, on the live, tail and older-page paths.
+- Owner corpus: 270 of 2,212 rollouts have 1,232 colliding keys among visible items, 4 of those rollouts from 2026-09.
+
+## Change
+One shared `codexRolloutIdentity(entry)` in `src/shared/codex/`, used by the renderer uuid, the renderer marker and main's marker, so the three cannot drift:
+- a payload `id` stays `ts:id` (already unique);
+- a `call_id` item stays `ts:call_id`, and its `*_output` becomes `ts:call_id:output`;
+- anything identified only by its type gets `ts:type:<fnv1a of the payload JSON>`, so two different items differ and the same line re-read still dedupes;
+- a missing timestamp is `''`, not `Date.now()`, so the identity is stable across reads.
+
+Markers are never persisted (`historyLoader.ts` header). Offsets anchor older pages, so the format change needs no migration.
+
+## Tests
+Real, minimal, redacted shapes from those two rollouts, driven through the real mapper and `admitMappedEntries`. Both items must be admitted. This is red on main.

src/main/sessions/historyLoader.ts

@@ -1,4 +1,5 @@
 import { mainOperations } from '@main/performance/operations.js'
+import { codexRolloutIdentity } from '@shared/codex/rolloutIdentity.js'
 import type { AgentProviderKind } from '@shared/types/providerKind.js'
 import type { FileHandle } from 'fs/promises'
 import { open, stat } from 'fs/promises'
@@ -121,9 +122,10 @@ function extractClaudeHistoryMarker(entry: Record<string, unknown>): string | nu
     : null
 }
 
-function extractCodexHistoryMarker(entry: Record<string, unknown>): string {
-  const payload = entry.payload as Record<string, unknown> | undefined
-  return `${String(entry.timestamp ?? '')}:${String(payload?.id ?? payload?.call_id ?? payload?.type ?? entry.type)}`
+// The renderer's marker rule, shared so the two cannot drift (#1288).
+// Exported for the parity test that pins it to the renderer's marker.
+export function extractCodexHistoryMarker(entry: Record<string, unknown>): string {
+  return codexRolloutIdentity(entry)
 }
 
 /**

src/providers/codex/renderer/transcript/entries.ts

@@ -142,10 +142,13 @@ export function stripCodexExecWrapper(output: string): string {
   return output.slice(idx + marker.length)
 }
 
-/** True when the output is ONLY the exec wrapper with a trailing
- *  "Process exited with code …" line and nothing else — i.e. no
- *  stdout/stderr worth surfacing. Callers filter these out so the
- *  feed doesn't get cluttered with empty tool-result rows. */
+/** True for ANY exec-wrapped output ("Chunk ID: …" with a "Process exited
+ *  with code …" line), stdout or not. The rollout mapper drops these
+ *  `function_call_output` lines because the correlated `exec_command_end`
+ *  event carries the same result, with exit code and command, and renders
+ *  the card; keeping both would duplicate it. (This comment used to say
+ *  "only the wrapper and nothing else", which the code never did; #1298
+ *  review B.) */
 export function isCodexExecWrapperOutput(output: string): boolean {
   return output.startsWith('Chunk ID:') && output.includes('\nProcess exited with code ')
 }

src/providers/codex/renderer/transcript/rollout.ts

@@ -1,4 +1,5 @@
 import type { Entry } from '@shared/types/transcript'
+import { codexRolloutIdentity } from '@shared/codex/rolloutIdentity'
 import { asRecord } from '@shared/lib/asRecord'
 
 import {
@@ -258,8 +259,10 @@ export function stampCodexTurnId(entry: Entry, turnId: string | null): Entry {
 
 export function mapCodexRolloutToFeedEntries(entry: Record<string, unknown>): Entry[] {
   const payload = asRecord(entry.payload)
-  const uuid =
-    `${String(entry.timestamp ?? Date.now())}:${String(payload?.id ?? payload?.call_id ?? payload?.type ?? entry.type)}`
+  // 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
+  // id-less items, and admission silently dropped the second.
+  const uuid = codexRolloutIdentity(entry)
   const timestamp =
     typeof entry.timestamp === 'string' ? entry.timestamp : undefined
 
@@ -607,11 +610,9 @@ export function mapCodexRolloutToFeedEntries(entry: Record<string, unknown>): En
   return []
 }
 
-/** Build the history marker for a Codex rollout entry. The format is
- *  `<timestamp>:<payload.id|call_id|type|entry.type>` — mirrors the
- *  uuid used by mapCodexRolloutToFeedEntries so the older-history
- *  loader and the dedup path see the same identity. */
+/** The history marker for a Codex rollout entry: the same identity as the
+ *  entry uuid, from the one shared rule (see codexRolloutIdentity), so the
+ *  older-history loader and the dedupe path cannot disagree. */
 export function codexHistoryMarker(entry: Record<string, unknown>): string {
-  const payload = asRecord(entry.payload)
-  return `${String(entry.timestamp ?? '')}:${String(payload?.id ?? payload?.call_id ?? payload?.type ?? entry.type)}`
+  return codexRolloutIdentity(entry)
 }

src/providers/codex/renderer/transcript/rolloutIdentity.test.ts

@@ -0,0 +1,75 @@
+import { readFileSync } from 'node:fs'
+import { join } from 'node:path'
+import { describe, expect, it } from 'vitest'
+
+import { admitMappedEntries, type CommittedSeenLedger } from '@renderer/session-runtime/ingest/committedRecords'
+import { extractCodexHistoryMarker } from '@main/sessions/historyLoader'
+import { createCodexTranscriptEntryMapper } from './mapper'
+import { codexHistoryMarker } from './rollout'
+
+// #1288: the recorded same-millisecond shapes (minimal, redacted). Before the
+// shared identity, the second item of each pair mapped to the first's uuid
+// and admission dropped it for good: a tool card with no result, a message
+// that never rendered.
+const fixture = JSON.parse(readFileSync(join(import.meta.dirname,
+  '../../../../../testing/fixtures/codex-identity/same-millisecond-2026-04.json'), 'utf8')) as {
+  callAndOutput: Record<string, unknown>[]
+  twoOutputs: Record<string, unknown>[]
+  messages: Record<string, unknown>[]
+}
+
+function ledger(): CommittedSeenLedger {
+  return { seen: new Set(), isTrimmed: () => false, releaseTrimmed: () => {} }
+}
+
+function admitAll(lines: Record<string, unknown>[], seen = ledger()) {
+  const mapper = createCodexTranscriptEntryMapper()
+  return lines.flatMap(line => {
+    const { entries, historyMarker } = mapper.map(line)
+    return admitMappedEntries(entries, historyMarker, 'live', seen).admitted
+  })
+}
+
+describe('Codex rollout identity (#1288)', () => {
+  it('admits a tool call and its output that share a millisecond', () => {
+    const admitted = admitAll(fixture.callAndOutput)
+    const blocks = admitted.flatMap(entry => (entry as { message?: { content?: Array<{ type: string }> } }).message?.content ?? [])
+    expect(blocks.map(block => block.type)).toEqual(expect.arrayContaining(['tool_use', 'tool_result']))
+  })
+
+  it('admits two different messages that share a millisecond', () => {
+    expect(admitAll(fixture.messages)).toHaveLength(2)
+  })
+
+  it('still dedupes the same line read twice (a chunk overlap)', () => {
+    const seen = ledger()
+    const first = admitAll(fixture.messages, seen)
+    const again = admitAll(fixture.messages, seen)
+    expect(first).toHaveLength(2)
+    expect(again).toHaveLength(0)
+  })
+
+  it('gives a line without a timestamp the same identity on every read', async () => {
+    const [line] = fixture.messages
+    const { timestamp: _dropped, ...untimed } = line!
+    const uuidOf = () => (createCodexTranscriptEntryMapper().map(untimed).entries[0] as { uuid?: string } | undefined)?.uuid
+    const a = uuidOf()
+    await new Promise(resolve => setTimeout(resolve, 2))
+    expect(a).toBeDefined()
+    expect(uuidOf()).toBe(a)
+  })
+
+  // #1298 review A: one call, two different outputs in the same millisecond.
+  it('admits two different outputs of one call that share a millisecond', () => {
+    expect(admitAll(fixture.twoOutputs)).toHaveLength(2)
+  })
+
+  // #1298 review B: the whole point of the shared rule is that main's marker
+  // and the renderer's cannot drift; a re-inlined old rule broke older-page
+  // anchoring silently. Pin them equal on every recorded shape.
+  it('computes the same marker in main as in the renderer', () => {
+    for (const line of [...fixture.callAndOutput, ...fixture.twoOutputs, ...fixture.messages]) {
+      expect(extractCodexHistoryMarker(line)).toBe(codexHistoryMarker(line))
+    }
+  })
+})

src/shared/codex/rolloutIdentity.ts

@@ -0,0 +1,67 @@
+/**
+ * The identity of one Codex rollout line (#1288).
+ *
+ * WHY this exists, and why it is shared: Codex rollout lines carry no uuid,
+ * so Agent Code synthesizes one. The renderer uses it as the entry uuid that
+ * committed-record admission dedupes on, and as the pagination marker; main's
+ * history loader computes the same marker to find an older page's anchor.
+ * Those three were three copies of `${timestamp}:${id ?? call_id ?? type}`,
+ * and that rule is not unique:
+ *   - a `function_call` and its `*_output` in the same millisecond share
+ *     `ts:call_id`, so the output was dropped as "already seen" and the tool
+ *     card never got its result;
+ *   - items identified only by their type (`message` has no id) collide on
+ *     `ts:message`, so a second user or assistant message in the same
+ *     millisecond never rendered.
+ * The owner's corpus had 1,232 such collisions in 270 of 2,212 rollouts, 4 of
+ * them this month. Admission is the same on the live, tail and older-page
+ * paths, so the loss was permanent and silent.
+ *
+ * The rule, weakest id last:
+ *   - a payload `id` is unique on its own: `ts:id` (unchanged);
+ *   - a `call_id` item keeps `ts:call_id`; its output (`*_output`) gets
+ *     `:output:<hash>` — the pair shares the call id by design, and one call
+ *     can record two DIFFERENT outputs in one millisecond (a real rollout:
+ *     two custom_tool_call_output lines, session_id 88325 then 39977;
+ *     #1298 review A);
+ *   - anything else is identified by type alone, so the payload's content
+ *     decides: `ts:type:<hash>`. Two different items differ; the SAME line
+ *     read twice (chunk overlap, a tail re-read) still maps to the same
+ *     identity, which is what dedupe needs.
+ * A missing timestamp is '' (it was Date.now(), a new identity on every read).
+ *
+ * Markers are never persisted (historyLoader.ts header) and older pages are
+ * anchored by byte offset, so changing the format needs no migration.
+ */
+export function codexRolloutIdentity(entry: Record<string, unknown>): string {
+  const payload = isRecord(entry.payload) ? entry.payload : undefined
+  const timestamp = String(entry.timestamp ?? '')
+  const id = payload?.id
+  if (id !== undefined && id !== null) return `${timestamp}:${String(id)}`
+  const callId = payload?.call_id
+  if (callId !== undefined && callId !== null) {
+    const isOutput = typeof payload?.type === 'string' && payload.type.endsWith('_output')
+    return isOutput
+      ? `${timestamp}:${String(callId)}:output:${fnv1a32(JSON.stringify(payload))}`
+      : `${timestamp}:${String(callId)}`
+  }
+  const type = String(payload?.type ?? entry.type)
+  return `${timestamp}:${type}:${fnv1a32(JSON.stringify(payload ?? entry))}`
+}
+
+function isRecord(value: unknown): value is Record<string, unknown> {
+  return typeof value === 'object' && value !== null && !Array.isArray(value)
+}
+
+/** 32-bit FNV-1a, hex. Synchronous and dependency-free: this runs in the
+ *  renderer per rollout line, where crypto.subtle is async. Collisions only
+ *  matter between items with the same timestamp AND type, a handful at most,
+ *  so 32 bits is ample. */
+function fnv1a32(text: string): string {
+  let hash = 0x811c9dc5
+  for (let i = 0; i < text.length; i++) {
+    hash ^= text.charCodeAt(i)
+    hash = Math.imul(hash, 0x01000193)
+  }
+  return (hash >>> 0).toString(16).padStart(8, '0')
+}

testing/fixtures/codex-identity/same-millisecond-2026-04.json

@@ -0,0 +1,74 @@
+{
+  "source": "owner ~/.codex/sessions rollouts: 2026-04-13 (call + output, same ms), 2026-07-21 (one call, two different outputs, same ms), 2026-04-16 (two messages, same ms). Paths, ids and message text redacted; the kept output text is the recorded error string; wrapper-free shapes as recorded.",
+  "callAndOutput": [
+    {
+      "timestamp": "2026-04-13T09:06:41.517Z",
+      "type": "response_item",
+      "payload": {
+        "type": "function_call",
+        "name": "Edit",
+        "arguments": "{\"replace_all\":false,\"file_path\":\"src/example.ts\",\"old_string\":\"a\",\"new_str\":\"b\"}",
+        "call_id": "call_REDACTED_1"
+      }
+    },
+    {
+      "timestamp": "2026-04-13T09:06:41.517Z",
+      "type": "response_item",
+      "payload": {
+        "type": "function_call_output",
+        "call_id": "call_REDACTED_1",
+        "output": "<tool_use_error>InputValidationError: Edit failed due to the following issues:\nThe required parameter `new_string` is missing\nAn unexpected parameter `new_str` was provided</tool_use_error>"
+      }
+    }
+  ],
+  "twoOutputs": [
+    {
+      "timestamp": "2026-07-21T10:55:08.435Z",
+      "type": "response_item",
+      "payload": {
+        "type": "custom_tool_call_output",
+        "call_id": "call_REDACTED_2",
+        "output": "{\"session_id\":88325}"
+      }
+    },
+    {
+      "timestamp": "2026-07-21T10:55:08.435Z",
+      "type": "response_item",
+      "payload": {
+        "type": "custom_tool_call_output",
+        "call_id": "call_REDACTED_2",
+        "output": "{\"session_id\":39977}"
+      }
+    }
+  ],
+  "messages": [
+    {
+      "timestamp": "2026-04-16T14:34:49.436Z",
+      "type": "response_item",
+      "payload": {
+        "type": "message",
+        "role": "user",
+        "content": [
+          {
+            "type": "input_text",
+            "text": "Continue from the summary."
+          }
+        ]
+      }
+    },
+    {
+      "timestamp": "2026-04-16T14:34:49.436Z",
+      "type": "response_item",
+      "payload": {
+        "type": "message",
+        "role": "assistant",
+        "content": [
+          {
+            "type": "output_text",
+            "text": "Picking up where we left off."
+          }
+        ]
+      }
+    }
+  ]
+}