From 50fec5630e3582991405b809d5329b42998de982 Mon Sep 17 00:00:00 2001 From: Brennan Benson <79079362+brennanb2025@users.noreply.github.com> Date: Sun, 16 Aug 2026 15:31:53 -0700 Subject: [PATCH] fix(native-chat): keep structured rows put and stop raw opcode rows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three seams behind "the compaction and notification rows moved above the message I'd already sent". - The reducer pinned `sequence` across revisions but let `observedAt` advance, and clients sort the timeline by that timestamp. Codex echoing a send revises the submission row, so the user's own bubble jumped below every row that landed while the turn was in flight. Pin `observedAt` with `sequence`: a revision refreshes content, never position. - `item:contextCompaction` reached the timeline as a raw `codex · item:` row because the frame catalog is keyed by notification METHOD and never matches item-form kinds. Codex item types now get their own dispositions; compaction is chrome, matching the `thread/compacted` notification it duplicates. Undispositioned item types still fall through visibly. - The generic fallback row named the opcode and nothing else, so a real warning read as `codex · notification:warning`. It now leads with the provider's own sentence when the payload carries one, with the raw frame still behind the row's disclosure. --- .../journal-reducer.test.ts | 16 +++++++++ .../agent-session-journal/journal-reducer.ts | 6 +++- .../provider-frame-disposition.test.ts | 11 ++++++ .../provider-frame-disposition.ts | 17 +++++++++ .../unhandled-provider-frame.test.ts | 21 +++++++++++ .../unhandled-provider-frame.ts | 36 ++++++++++++++++++- 6 files changed, 105 insertions(+), 2 deletions(-) diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.test.ts b/src/main/native-chat/agent-session-journal/journal-reducer.test.ts index 1be6f256406..d09f4fab2d7 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.test.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.test.ts @@ -123,6 +123,22 @@ describe('ordering', () => { expect(items[1]?.recovered).toBe(true) }) + it('pins observedAt to creation so a revision cannot relocate the row', () => { + // Clients sort the timeline by observedAt. The provider echoing a send revises + // the submission row; if that advanced the timestamp the user's own bubble + // would sort below rows that landed while the turn was in flight. + const state = fold([ + { kind: 'item', itemId: 'send', revision: 0, body: userText('ok thanks'), ...base(1) }, + { kind: 'item', itemId: 'frame', revision: 1, body: text('warning'), ...base(2) }, + { kind: 'item', itemId: 'send', revision: 1, body: userText('ok thanks'), ...base(3) } + ]) + const items = renderJournalState(state).items + expect(items.map((item) => item.itemId)).toEqual(['send', 'frame']) + expect(items.map((item) => item.observedAt)).toEqual([base(1).ts, base(2).ts]) + // The revision still lands — only its ordering keys are ignored. + expect(items[0]?.revision).toBe(1) + }) + it('never collapses two items that carry identical text', () => { const state = fold([ { kind: 'item', itemId: 'a', revision: 1, body: text('run the tests'), ...base(1) }, diff --git a/src/main/native-chat/agent-session-journal/journal-reducer.ts b/src/main/native-chat/agent-session-journal/journal-reducer.ts index c311d003a2b..c1d7f39bfec 100644 --- a/src/main/native-chat/agent-session-journal/journal-reducer.ts +++ b/src/main/native-chat/agent-session-journal/journal-reducer.ts @@ -146,7 +146,11 @@ function upsertItem( return } // Creation sequence is the ordering key; a revision refreshes content only. - state.items.set(itemId, { ...next, sequence: existing.sequence }) + // `observedAt` is pinned with it: clients sort the timeline by that timestamp, + // so letting a revision advance it makes the row jump past everything that + // landed in between — the provider's own echo of a send revises the submission + // row, which relocated the user's bubble below later rows. + state.items.set(itemId, { ...next, sequence: existing.sequence, observedAt: existing.observedAt }) state.tombstones.delete(itemId) } diff --git a/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts index 3d7eeaead62..ad9ca66c52a 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.test.ts @@ -78,4 +78,15 @@ describe('provider frame classification catalog', () => { ) expect(classifyProviderFrame('claude', 'message:future_delta', {})).toBe('stream-into-item') }) + + it('dispositions codex item-form frames, which the method catalog never matches', () => { + // `thread/compacted` is already chrome; its item form is the same event and + // must not leak `codex · item:contextCompaction` into the transcript. + expect(classifyProviderFrame('codex', 'item:contextCompaction', {})).toBe('status-chrome') + expect(classifyProviderFrame('codex', 'notification:thread/compacted', {})).toBe( + 'status-chrome' + ) + // An item type nobody has dispositioned still falls through visibly. + expect(classifyProviderFrame('codex', 'item:futureThing', {})).toBe('timeline-substantive') + }) }) diff --git a/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts b/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts index cf06598f7e4..8496c973a95 100644 --- a/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts +++ b/src/main/native-chat/agent-session-wire/provider-frame-disposition.ts @@ -180,10 +180,23 @@ function hasProviderError(payload: unknown): boolean { return false } +/** Codex thread-item types with no typed renderer, dispositioned by hand so a + * new item type cannot leak `codex · item:` into the transcript. The + * notification catalog above is keyed by METHOD and never matches these. */ +const CODEX_ITEM_CLASSIFICATIONS: Record = { + // The `thread/compacted` notification is already chrome; its item form is the + // same event and must not read as a mysterious opcode row. + contextCompaction: 'status-chrome' +} + function notificationKind(kind: string): string { return kind.startsWith('notification:') ? kind.slice('notification:'.length) : kind } +function itemKind(kind: string): string | null { + return kind.startsWith('item:') ? kind.slice('item:'.length) : null +} + export function isDeltaShapedProviderFrameKind(kind: string): boolean { return notificationKind(kind).toLowerCase().endsWith('delta') } @@ -193,6 +206,10 @@ function catalogClassification( kind: string ): ProviderFrameClassification | undefined { if (provider === 'codex') { + const item = itemKind(kind) + if (item !== null) { + return CODEX_ITEM_CLASSIFICATIONS[item] + } return PROVIDER_FRAME_CLASSIFICATIONS.codex[ notificationKind(kind) as CodexAppServerNotificationMethod ] diff --git a/src/main/native-chat/agent-session-wire/unhandled-provider-frame.test.ts b/src/main/native-chat/agent-session-wire/unhandled-provider-frame.test.ts index ed3dea75c21..0a279eea23a 100644 --- a/src/main/native-chat/agent-session-wire/unhandled-provider-frame.test.ts +++ b/src/main/native-chat/agent-session-wire/unhandled-provider-frame.test.ts @@ -167,4 +167,25 @@ describe('unhandled provider frame journal fallback', () => { ).not.toBeNull() expect(unhandledProviderFrameJournalItem('claude', 'message:future/event', {})).not.toBeNull() }) + + it('leads with the provider sentence instead of naming the opcode', () => { + const row = unhandledProviderFrameJournalItem('codex', 'notification:warning', { + message: 'Your plan limit resets in 2 hours.' + }) + expect(row?.body.text).toBe('Your plan limit resets in 2 hours.') + // The raw frame stays available behind the row's disclosure. + expect(row?.body.providerFrame?.kind).toBe('notification:warning') + }) + + it('unwraps a nested sentence and falls back to the opcode when there is none', () => { + expect( + unhandledProviderFrameJournalItem('codex', 'notification:warning', { + warning: { text: 'Sandbox is degraded.' } + })?.body.text + ).toBe('Sandbox is degraded.') + expect( + unhandledProviderFrameJournalItem('codex', 'notification:future/event', { count: 3 })?.body + .text + ).toBe('codex \u00b7 notification:future/event') + }) }) diff --git a/src/main/native-chat/agent-session-wire/unhandled-provider-frame.ts b/src/main/native-chat/agent-session-wire/unhandled-provider-frame.ts index d72ebe0d69d..bfcbcff2919 100644 --- a/src/main/native-chat/agent-session-wire/unhandled-provider-frame.ts +++ b/src/main/native-chat/agent-session-wire/unhandled-provider-frame.ts @@ -20,6 +20,36 @@ function serializeProviderPayload(payload: unknown): string { } } +/** Fields providers use for the human-facing sentence on a frame, most specific + * first. Nested one level because warnings arrive wrapped as often as not. */ +const MESSAGE_KEYS = ['message', 'text', 'warning', 'detail', 'description', 'reason'] as const + +function readableMessage(payload: unknown): string | null { + if (typeof payload === 'string') { + return payload.trim() || null + } + if (typeof payload !== 'object' || payload === null || Array.isArray(payload)) { + return null + } + const record = payload as Record + for (const key of MESSAGE_KEYS) { + const value = record[key] + if (typeof value === 'string' && value.trim().length > 0) { + return value.trim() + } + } + for (const key of MESSAGE_KEYS) { + const nested = record[key] + if (typeof nested === 'object' && nested !== null && !Array.isArray(nested)) { + const inner = readableMessage(nested) + if (inner) { + return inner + } + } + } + return null +} + /** Substantive adapter fallbacks become visible, bounded journal rows. */ export function unhandledProviderFrameJournalItem( provider: string, @@ -37,10 +67,14 @@ export function unhandledProviderFrameJournalItem( } const serialized = serializeProviderPayload(payload) const bounded = boundPayload(serialized, limits) + // Why: the opcode alone ("codex · notification:warning") tells the user nothing + // and reads as protocol noise. Lead with the provider's own sentence when it has + // one; the raw frame stays behind the row's disclosure either way. + const message = readableMessage(payload) return { body: { kind: 'status', - text: `${provider} · ${kind}`, + text: message ?? `${provider} · ${kind}`, providerFrame: { provider, kind, payload: bounded } }, blobs: bounded.truncated ? [{ digest: bounded.digest, payload: serialized }] : []