feat: improve codex web chat feedback

[centdix]

Summary

Improve Codex web chat feedback and make the streaming path reliable enough for live use. This adds richer streamed conversation events, preserves correct ordering with revisions, and fixes duplicate/stale message behavior seen during WebUI testing.

Changes

• Add richer Codex conversation messages for commentary, thinking, tool calls, tool results, failures, and live messageUpsert updates.
• Add stream event revisions, backend stream session state, notification buffering, refresh coalescing, and stale-event filtering on frontend and CLI.
• Fix Codex thread resolution so saved thread metadata wins over unrelated cwd discovery.
• Fix app-server stdout decoding across split UTF-8 chunks and final lines without trailing newlines.
• Prevent duplicate user/assistant messages by merging logical live/snapshot messages and replacing optimistic user bubbles with real server messages.
• Remove the typing label under plain assistant messages while keeping internal in-progress state.

Test plan

• bun run --cwd backend check
• bun run --cwd frontend check
• bun run test
• Manual WebUI smoke test with live Codex tool calls and duplicate-message checks

---

Generated with Claude Code

[claude]

Claude finished @centdix's task in 5m 39s —— View job

---

Code Review — PR #258: improve codex web chat feedback

Thorough review done. The streaming architecture (server-assigned monotonic revisions, notification buffering until the first snapshot, and refresh coalescing) is well thought out and the test coverage across backend, frontend, and CLI is strong. Parity is maintained — messageUpsert + revision filtering are wired into the CLI (oneshot.ts) and the web surface (MobileChatSurface), and WorktreeConversationPanel stays purely presentational. Nice work. A few correctness concerns below (3 posted inline).

Findings

1. Duplicate tool bubbles if app-server item id ≠ rollout call_id (needs confirmation — inline on worktree-conversation-service.ts:252)
Snapshot merging dedupes tool calls only by exact message.id, and the live merge matches by toolCallId. commandExecution items are keyed by the app-server item id while session-log entries are keyed by the model call_id. If those differ for the same command, you get two bubbles (one shell, one exec_command) plus a liveMessages entry that is never evicted. The PR's manual smoke test may have hidden this if the ids happen to coincide — please confirm they're guaranteed equal, otherwise dedupe across sources by a stable key.

2. Optimistic user bubble vanishes on duplicate text (inline on worktree-conversation.ts:87)
isSameServerUserMessage's text fallback matches the pending message against any prior user message with equal text. Re-sending an identical prompt makes mergeConversationSnapshot collapse the new optimistic bubble onto the older server message, so the just-sent prompt disappears until the real server message arrives.

3. running can stick on (inline on agents-ui-stream-service.ts:188)
A live inProgress message that never receives a terminal item/completed (aborted turn / dropped notification) is never evicted by shouldKeepLiveMessage, so every later snapshot re-asserts running: true indefinitely.

Minor / nits

• frontend/src/lib/worktree-conversation.ts — mergeConversationMessage(_existing, incoming) => incoming is an identity replace. Combined with no length guard, an out-of-order messageUpsert with shorter text would regress visible text. Monotonic revisions make this unlikely in practice, but the backend version guards with a max-length merge while the frontend doesn't — worth aligning or adding a brief note on why replace-wins is intentional here.
• buildAgentsUiMessageUpsertEvents uses includeEmptyText: true, so an item/started agentMessage with empty text emits an empty assistant bubble that renders as a blank padded box until deltas arrive. Minor visual flash.
• codex-app-server.ts — stdoutDecoder is a reused instance; if the stdout reader throws before the final decode() flush, a partial multibyte sequence carries into the next spawned process. Low risk, but resetting/recreating the decoder per process would be safer.
• mergeConversationMessages sort: compareMessagesByTimestamp returns 0 when either createdAt is null and ties fall back to array index (thread messages before session-log messages). Fine given session-log entries always carry timestamps, but it's an implicit invariant worth a one-line comment.

Things that look correct

• Revision init/reset is consistent across server (nextRevision starts at 1), CLI, and frontend (lastStreamRevision = 0 on (re)connect).

• Notification buffering replay is race-free (synchronous), and handleNotification re-filters by thread id so cross-thread buffered events are safely ignored.

• Thread-resolution change (saved metadata wins over cwd discovery) matches the stated intent and is covered by the rewritten test.

• UTF-8 split-chunk + trailing-line decoding is correct and well-tested.

• Read PR diff and changed files

• Review backend stream service & app-server adapter

• Review session-log service & conversation service

• Review frontend conversation logic & components

• Review API contract / schema changes

• Post review feedback (3 inline + summary)

[cloudflare-workers-and-pages]

Deploying with    Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status	Name	Latest Commit	Preview URL	Updated (UTC)
✅ Deployment successful! View logs	webmux	26ae08b	Commit Preview URL Branch Preview URL	May 28 2026, 04:01 PM

[claude]

Optimistic bubble can vanish when re-sending identical text. The text fallback matches the pending message against any prior user message with equal text, not just the one for the current turn. If a user sends a message identical to an earlier one, mergeConversationSnapshot sees the older server message satisfy isSameServerUserMessage, drops the freshly-added optimistic bubble (whose real server message hasn't landed yet), and the just-sent prompt disappears until the next snapshot includes it.

Consider scoping the text fallback to messages that aren't already present in the prior conversation (e.g. only match incoming user messages whose id wasn't already in the current state), so a duplicate prompt doesn't collapse onto a stale one.

Fix this →

[claude]

Cross-source tool-call dedup relies on identical ids. This merge dedupes only by exact message.id. buildConversationMessages now emits commandExecution tool messages keyed by the app-server item id (item.id / ${item.id}:result), while readCodexSessionMessages emits tool messages keyed by the rollout call_id (call_id / ${call_id}:result). If a persisted thread ever contains a commandExecution item for the same command that also appears in the session log, and item.id !== call_id, the same shell command renders twice.

The same assumption underpins the live path: isSameLogicalConversationMessage matches tool calls by toolCallId, so a live commandExecution (toolCallId = item.id, toolName shell) and the session-log entry (toolCallId = call_id, toolName exec_command) won't be recognized as the same call — leaving a duplicate bubble and a never-evicted entry in liveMessages.

Could you confirm commandExecution.id and the rollout call_id are guaranteed equal? If not, dedupe tool calls across sources by a stable key rather than the raw id.

[claude]

running can get stuck on. If a live message goes inProgress (via a delta or item/started) but never receives a terminal item/completed notification — e.g. the turn is aborted or the completion is dropped — shouldKeepLiveMessage keeps it (no matching completed snapshot message), so every subsequent snapshot re-asserts running: true indefinitely and the UI shows the conversation as perpetually running. Worth a guard that evicts/clears live in-progress messages once the snapshot reports the turn is no longer active.

[claude]

Claude finished @centdix's task in 5m 48s —— View job

---

Code Review — PR #258: improve codex web chat feedback

Reviewed the full diff against origin/main (the PR is merged; I diffed 8adbdef^..8adbdef). The streaming redesign is solid: server‑assigned monotonic revisions, notification buffering until the first snapshot, synchronous buffered‑replay, refresh coalescing, and stale‑event filtering on both the web (MobileChatSurface) and CLI (oneshot.ts) surfaces. Parity is maintained and test coverage is genuinely good (live‑merge, revision filtering, UTF‑8 split chunks, thread resolution).

This is a re‑review, so below I confirm/refine the three earlier findings (with corrected mechanics) and add a few observations the first pass didn't surface.

---

Confirmed findings (refined)

1. Cross‑source tool dedup is fragile — but the real hinge is turnId/callId, not raw item.id. (worktree-conversation-service.ts:200 isSameLogicalMessage, agents-ui-stream-service.ts:150 isSameLogicalConversationMessage)
The snapshot merge does not dedupe tool calls purely by message.id. For toolUse/toolResult it falls through to a turnId + normalized‑text (+ cwd) comparison, so an item.id ≠ call_id mismatch alone won't duplicate. The genuine fragility is:

• Snapshot path requires the app‑server turn.id to equal the session‑log task_started.turn_id. If those differ, isSameLogicalMessage returns false at the turnId guard and the same shell command renders twice (shell from the thread item + exec_command from the log).
• Live path matches by toolCallId first (agents-ui-stream-service.ts:155), so a live commandExecution (toolCallId = item.id) and the log entry (toolCallId = call_id) won't be recognized as the same call if those ids differ — leaving a duplicate bubble and a never‑evicted liveMessages entry.

Worth confirming whether app‑server and rollout share turn/call identifiers. If not, dedupe across sources by a stable key. (Related: thread agentMessage items with phase === "analysis" become kind: "thinking", while the log emits reasoning as thinking too — isSameLogicalMessage returns false for thinking, so if the thread ever carries reasoning items you'd get duplicate Thinking blocks.)

2. Optimistic user bubble can vanish — but only on rapid/concurrent identical sends, not simple resends. (frontend/src/lib/worktree-conversation.ts:87 isSameServerUserMessage)
The newlyArrivedUserMessages scoping in mergeConversationSnapshot already protects the sequential resend case — covered by the test at worktree-conversation.test.ts:255. The hole the text fallback leaves open is two in‑flight optimistic bubbles with identical text:

1. User sends "Ship it" → optimistic pending-user:turn-1.
2. Before the first snapshot lands, sends "Ship it" again → pending-user:turn-2.
3. Snapshot arrives with only the first server message user-1 ("Ship it") → it's in newlyArrivedUserMessages.
4. Both optimistic bubbles match user-1 via the text fallback, so both are dropped — pending-user:turn-2 disappears until its own server message arrives.

The suggested fix (scope the fallback to genuinely‑new server messages / match by reconciled turn id rather than raw text) is right, and a test for the double‑identical‑send would lock it in. Fix this →

3. running stuck‑on is narrower than first stated — self‑heal already exists. (agents-ui-stream-service.ts:188 shouldKeepLiveMessage)
When a snapshot reports running: false, an unmatched in‑progress live message is evicted (return snapshot.running || liveMessage.status !== "inProgress" → false), covered by agents-ui-stream-service.test.ts:465. The true stuck case requires all of: an orphan in‑progress live message with no terminal item/completed, and snapshot.running staying true (other activity keeps the thread active), and no turn/completed / thread/status idle|completed|interrupted notification ever arriving for that turn (otherwise completeLiveMessages clears it). Narrow, but real — a turn‑id‑scoped TTL or evicting in‑progress live messages whose turnId is absent from the snapshot's active turn would close it.

---

Minor / nits

• frontend/src/lib/worktree-conversation.ts:62 — mergeConversationMessage(_existing, incoming) => incoming is an identity replace, while the backend's mergeConversationMessage does a max‑length text merge. Monotonic revisions make a regressing out‑of‑order messageUpsert unlikely, but the asymmetry is worth a one‑line note or alignment. (Note: mergeConversationUpsertMessage does guard length — it's only the snapshot‑path mergeConversationMessage that replaces.)
• agents-ui-stream-service.ts buildAgentsUiMessageUpsertEvents uses includeEmptyText: true, so an item/started agentMessage with empty text emits an empty assistant bubble until deltas arrive — a brief blank padded box. WorktreeConversationPanel.isVisibleTranscriptMessage filters empty text/thinking from the transcript, which mitigates it on the web side, but the CLI printNewMessages may still print it.
• backend/src/adapters/codex-app-server.ts — the new per‑process TextDecoder in startStdoutLoop is the right fix. The startStderrLoop uses decode(value, { stream: true }) but never flushes on done; harmless for debug logging, but a trailing multibyte stderr line could be dropped.
• worktree-conversation-service.ts compareMessagesByTimestamp returns 0 when either createdAt is null, falling back to array index. Fine because session‑log entries always carry timestamps and thread‑derived messages sort before them, but that's an implicit invariant worth a comment.

Looks correct

• Revision init/reset is consistent: server nextRevision() starts at 1, CLI and frontend reset lastStreamRevision = 0 on (re)connect, and the non‑codex path emits a one‑off revision: 1.
• Buffered‑notification replay is race‑free (synchronous loop, single‑threaded), bufferingNotifications flips before replay, and handleNotification re‑filters by thread id so cross‑thread buffered events are ignored.
• Thread‑resolution change (saved metadata wins over cwd discovery) matches intent and is covered by the rewritten test.
• UTF‑8 split‑chunk + trailing‑line decoding (readCodexAppServerStdoutLines) is correct and well‑tested.
• mergeConversationSnapshotWithLiveMessages iterates a copied liveMessages array, so the delete/set during sendSnapshot is safe.

Overall: well‑structured, well‑tested change. Findings #1 and #2 are the ones I'd prioritize chasing down; #3 is a narrow edge with existing self‑heal.

• Read PR diff against origin/main
• Review backend stream service & app-server adapter
• Review session-log & conversation service
• Review frontend conversation logic & components
• Review API contract / CLI parity
• Post review feedback