fix: refine agent chat ui

[centdix]

Summary

Refines the in-app agent chat UI and Codex conversation streaming model. The PR keeps the UI simpler, preserves Codex live ordering, and changes Codex history loading so the HTTP snapshot comes from the persisted JSONL transcript while WebSocket/app-server notifications are used only as the live tail.

This avoids reconstructing historical tool calls from live stream events or depending on the app-server thread/read item model for the rendered transcript. The app server still resolves the Codex thread, path, running state, and control actions; the JSONL file is the source of truth for rendered snapshot messages.

Changes

• Remove the compact Ready/agent label header above the chat transcript.
• Replace the separate textarea footer controls with a simpler rounded composer and integrated send/stop icon button.
• Show the processing indicator during pending sends and keep it tied to the same state that shows the composer stop button.
• Render tool calls as quieter whole-row expandable blocks with a two-line input preview and inline expanded output.
• Add JSONL-backed Codex snapshot parsing for visible user/assistant messages, reasoning summaries, tool calls, and tool outputs.
• Use app-server thread/list / thread/read for thread resolution and status, then prefer JSONL session messages for the conversation snapshot when available.
• Keep Codex WebSocket notifications as the live tail: idle snapshots do not open a stream, running snapshots do, and sends force a stream before posting the new turn.
• Handle terminal thread/status/changed notifications so live Codex sessions clear running even when no turn/completed notification arrives.
• Keep non-Codex conversation sockets as quiet no-ops for existing CLI Claude flows.
• Restore tolerant app-server item parsing, including unknown item fallback and nullable agent-message phases, so a single unfamiliar item does not break history resolution.
• Normalize old Linear webmux: 1 attachments that lack order or kind, preserving --from-linear compatibility with previously exported conversations.

Test plan

• bun run --cwd backend check
• bun run --cwd frontend check
• bun test backend/src/tests/codex-session-log-service.test.ts backend/src/tests/worktree-conversation-service.test.ts backend/src/tests/agents-ui-stream-service.test.ts backend/src/tests/codex-app-server.test.ts backend/src/tests/conversation-export-service.test.ts
• bun run --cwd frontend test src/lib/MobileChatSurface.test.ts
• bun run --cwd backend test
• bun run --cwd frontend test
• git diff --check

---

Generated with Claude Code

[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	b0fcca7	Commit Preview URL Branch Preview URL	May 29 2026, 08:23 PM

[claude]

Claude finished @centdix's task in 3m 29s —— View job

---

Code Review: PR #260 — fix: simplify agent chat composer

• Read changed files and diff
• Review composer/header changes in WorktreeConversationPanel.svelte
• Review loading-state logic
• Review test changes
• Post review feedback

Overall this is a clean, well-scoped change. The composer simplification is tidy, it follows project conventions (runes, {#snippet}, Tailwind theme vars, aria-labels on the icon-only buttons), and the new tests cover the behavior changes well. The key fix — bridging the loading indicator across the send round-trip — is sound:

• showTurnPending = isSending || running (WorktreeConversationPanel.svelte:46) correctly closes the gap. In MobileChatSurface.svelte:212-233, isSending is true during the POST, then markConversationTurnStarted flips running to true before isSending resets in finally, so the indicator never flickers off mid-handoff. Nice.

No correctness bugs found. A few minor observations below.

---

1. Empty in-progress tool start now renders both a tool card and the processing indicator (confirm intended)

This is the behavior the new test at WorktreeConversationPanel.test.ts:243 encodes. An in-progress toolUse with empty text is still a visible transcript message (isVisibleTranscriptMessage returns true for any toolUse, line 105-111), so the transcript renders a "Running shell" card with an empty body + "running" label. At the same time showProcessingIndicator is true, because isVisibleProgressMessage returns false for an empty-text toolUse (line 113-120). Result: the user sees an empty "Running shell" card and "Codex is processing" directly below it.

Previously (old isVisibleTranscriptMessage-based check) the indicator was suppressed once any in-progress visible message existed, so this is a deliberate change. It's defensible — "Running shell" is informative even with an empty command — but the stacked card + spinner can read as redundant. Worth a quick confirm that this is the intended UX rather than suppressing one of the two while the command text is still empty.

2. The two visibility predicates now diverge subtly — a one-line WHY comment would help

isVisibleTranscriptMessage (line 105) and isVisibleProgressMessage (line 113) encode related-but-different notions: the former gates what renders in the transcript, the latter gates whether we still consider the turn "waiting for visible output." The divergence for empty toolUse (visible in transcript, not visible as progress) is exactly what drives finding #1, and it's non-obvious on a cold read. Per the repo's commenting guidance (comment the non-obvious WHY), a short note on isVisibleProgressMessage explaining why it intentionally differs from the transcript predicate would save a future reader.

3. Nit — brief window with no interrupt control during a pending send

While isSending is true but running is not yet set (the POST is in flight), showInterrupt is false, so the UI shows a disabled Send icon with "…is processing" visible and no stop button. It's a short window and interrupting an unacknowledged request isn't very meaningful, so low priority — just noting the transient state.

4. Nit — sendIcon is a reply/return-arrow glyph, not a send/paper-plane

The paths at WorktreeConversationPanel.svelte:192-193 draw a "return/reply" arrow rather than a conventional send icon. Purely stylistic and may well be intentional with the rounded composer aesthetic — flag only if a paper-plane was the intent.

5. Nit — queryByText("Claude") assertion is a little coupled

In WorktreeConversationPanel.test.ts:106, asserting queryByText("Claude") is absent passes today only because the default conversation isn't running (so no "Claude is processing" bubble). It's fine as-is, but a query targeting the removed header specifically (the old "Turn in progress"/"Ready" toggle) would be less sensitive to unrelated changes.

---

None of these block merge. #1 is the only one I'd want a deliberate answer on; the rest are nits.

[centdix]

@claude review this PR again, make sure code is clean and simple, no duplication, no complex logic

[claude]

Claude finished @centdix's task in 3m 51s —— View job

---

Code Review: PR #260 — fix: refine agent chat ui

• Read CLAUDE.md / AGENTS.md
• Diff all changed files
• Review frontend changes (WorktreeConversationPanel, MobileChatSurface, worktree-conversation)
• Review backend changes (agents-ui, agents-ui-stream-service, worktree-conversation-service, codex-app-server)
• Review shared types and API contract
• Post review findings

---

Overall this is a solid simplification. The removal of the heuristic text-overlap message matching, the snapshot-refresh machinery, and the type duplication between agents-ui.ts and api-contract are all genuine cleanups. A few concrete issues below, ordered by importance.

---

1. docs/codex-streaming-architecture-audit.md should not be committed

docs/codex-streaming-architecture-audit.md is an analysis/audit document. Per AGENTS.md: "Do not create planning, decision, or analysis documents unless the user asks for them — work from conversation context, not intermediate files." This file belongs in the conversation, not the repo. Fix this →

---

2. worktree-conversation.ts — 4× unnecessary intermediate variable before return

applyConversationMessageDelta, applyConversationMessageUpsert, mergeConversationSnapshot, and markConversationTurnStarted all create a nextConversation variable only to immediately return it. Every other function in the file (and project) returns the literal directly. These are inconsistent and add noise without aiding readability.

// current — worktree-conversation.ts:80-84
const nextConversation = {
  ...conversation,
  running: true,
  activeTurnId: event.turnId,
  messages: orderConversationMessages(messages),
};
return nextConversation;

// should be
return {
  ...conversation,
  running: true,
  activeTurnId: event.turnId,
  messages: orderConversationMessages(messages),
};

Fix this →

---

3. buildConversationProgressSignature — redundant sort

buildConversationProgressSignature calls orderConversationMessages(conversation.messages) before reading the last element. But every path that mutates conversation.messages already calls orderConversationMessages before storing, so the array is always sorted when it arrives here. The re-sort is dead work. Fix this →

---

4. agents-ui.ts — AgentsSendMessageRequest renamed on import

// backend/src/domain/agents-ui.ts
export type {
  AgentsSendMessageRequest as AgentsUiSendMessageRequest,
  ...

The type is AgentsSendMessageRequest in api-contract but exported as AgentsUiSendMessageRequest from the domain barrel. Either rename it in api-contract to AgentsUiSendMessageRequest (so the canonical name is consistent) or import it by its real name throughout. A rename-on-reexport hides the actual source type name. Fix this →

---

5. orderSpanForItem — invisible coupling to builder output

orderSpanForItem in agents-ui-stream-service.ts hardcodes return 2 for commandExecution, fileChange, mcpToolCall, dynamicToolCall. This must match the number of messages buildCodexItemConversationMessages returns for those types. When a tool has no result (empty diff, no content items), the builder returns 1 message but orderSpanForItem still reserves 2 slots — creating a silent gap in order numbers between the live stream and the snapshot.

This isn't a correctness bug today (gaps don't break ordering), but the span values are undocumented and will silently diverge if a new builder returns a different count. The dependency should either be documented with a comment on orderSpanForItem, or the span should be computed from buildCodexItemConversationMessages output rather than hardcoded.

---

Things done well

• Removed heuristic text-overlap matching (isSameLogicalConversationMessage, textOverlaps) — ID-based matching is far simpler and harder to get wrong.
• Deleted the snapshot-refresh machinery (queueSnapshotRefresh, refreshInFlight, liveMessages map) — pure event streaming is much simpler.
• agents-ui.ts is now a thin re-export barrel — eliminates the duplicate type definitions that existed between domain/agents-ui.ts and api-contract.
• commandExecutionStatus and isActiveTurnStatus are tightened now that the status types are enums rather than free strings.
• buildCodexItemConversationMessages dispatch is now a clean if-chain to typed builders, replacing the old merged-messages approach.

Issues #1 (doc file) and #2/#3 (intermediate variable / redundant sort) are the most clear-cut fixes. #4 and #5 are lower priority.
| Branch preview

[centdix]

@claude review again

[claude]

Claude finished @centdix's task in 4m 31s —— View job

---

Code Review: PR #260 — fix: refine agent chat ui

• Read AGENTS.md / CLAUDE.md
• Get current diff vs origin/main
• Review backend changes
• Review frontend changes
• Review types and API contract
• Post findings

---

None of the issues from the previous review were fixed. They are all still present in the current diff. Restating them here along with one new issue found.

---

1. docs/codex-streaming-architecture-audit.md must be removed — and is now also inaccurate

The file is still committed. Per AGENTS.md: "Do not create planning, decision, or analysis documents unless the user asks for them."

On top of that, the document now contradicts the actual implementation. It claims:

The backend emits deltas/upserts by exact item id. It refreshes snapshots on turn status changes and on completion of user, assistant, and command-execution items.

But queueSnapshotRefresh, runSnapshotRefresh, sendSnapshot, and loadSnapshot were all deleted in this PR. The backend never sends a snapshot event anymore — it was removed from AgentsUiConversationEventSchema. The doc describes the old architecture and should not be in the repo.

Fix this →

---

2. worktree-conversation.ts — 4 functions still use const nextConversation = …; return nextConversation

applyConversationMessageDelta (line 82), applyConversationMessageUpsert (line 115), mergeConversationSnapshot (line 164), and markConversationTurnStarted (line 182–188) all create an intermediate variable only to immediately return it. Every other function in the file returns the literal directly. This pattern adds noise without aiding readability.

// current — worktree-conversation.ts:182-188
const nextConversation = {
  ...conversation,
  running: true,
  activeTurnId: turnId,
  messages: orderConversationMessages(nextMessages),
};
return nextConversation;

// should be
return {
  ...conversation,
  running: true,
  activeTurnId: turnId,
  messages: orderConversationMessages(nextMessages),
};

Fix this →

---

3. buildConversationProgressSignature — still re-sorts when invariant guarantees order

buildConversationProgressSignature (line 194) calls orderConversationMessages(conversation.messages) before reading the last element, but every path that writes to conversation.messages already calls orderConversationMessages before storing. The array is always sorted on arrival. The re-sort is dead work.

Fix this →

---

4. agents-ui.ts — AgentsSendMessageRequest still renamed on re-export

// backend/src/domain/agents-ui.ts
export type {
  AgentsSendMessageRequest as AgentsUiSendMessageRequest,

The canonical name in api-contract is AgentsSendMessageRequest. Rename-on-reexport hides the source name. Either rename the type in api-contract to AgentsUiSendMessageRequest or reexport it under its real name. Fix this →

---

5. NEW — CodexAppServerTurnSchema turn-status narrowing breaks on unexpected values

The old schema used status: z.string() for turns. This PR narrows it to:

status: z.enum(["completed", "interrupted", "failed", "inProgress"]),

This is correct for the known values, but z.array(CodexAppServerTurnSchema) fails the entire array parse if a single turn carries an unknown status. The Codex app server previously sent statuses like "active", "queued", "running" (all handled by the old isActiveTurnStatus heuristic). If the app server sends any of those for a live turn, the entire thread read silently returns null and the conversation fails to load.

Item-level schemas are safe because CodexAppServerGenericItemSchema catches unknown item types as a fallback. Turn-level has no such fallback. Either make the turn status z.union([z.enum([...]), z.string()]) to preserve forward compatibility, or use .catch() to degrade gracefully rather than fail completely.

---

6. orderSpanForItem — span vs builder output coupling (unchanged from previous review)

orderSpanForItem hardcodes return 2 for fileChange, mcpToolCall, and dynamicToolCall, but buildFileChangeMessages, buildMcpToolCallMessages, and buildDynamicToolCallMessages each return only 1 message when the result text is empty. The span reservation is correct (needed for the later result), but the dependency is undocumented — a future change to a builder that always returns 1 would silently misalign order numbers. A one-line comment on orderSpanForItem explaining that span must match the builder's maximum possible output count would prevent this from diverging.

---

Summary

Issues #1–#3 are the clearest fixes: the doc file should be deleted (it's prohibited by AGENTS.md and now inaccurate), and the two code style issues should be straightforward. Issue #5 is the only new correctness concern.

#	Issue	Status
1	docs/ audit file	Not fixed, now also inaccurate
2	Intermediate nextConversation vars	Not fixed
3	Redundant sort in progress signature	Not fixed
4	Type rename on re-export	Not fixed
5	Turn-status enum breaks on unknown values	New
6	orderSpanForItem coupling undocumented	Not fixed
Branch preview

[claude]

Claude finished @centdix's task in 4m 33s —— View job

---

Code Review: PR #260 — fix: refine agent chat ui

• Read AGENTS.md / CLAUDE.md
• Get current state (PR merged → HEAD reflects merged code)
• Review frontend changes
• Review backend changes
• Check status of previously-raised issues
• Post findings

---

Good progress since the last round — most of the prior issues are now fixed. Verified the current state of main (the PR is merged, so I reviewed the merged code directly).

#	Prior issue	Status
1	docs/codex-streaming-architecture-audit.md committed	✅ Fixed — file deleted
2	4× intermediate nextConversation vars	✅ Fixed — all 4 return literals directly
3	Redundant sort in buildConversationProgressSignature	✅ Fixed — uses conversation.messages directly (worktree-conversation.ts:190)
4	AgentsSendMessageRequest renamed on re-export	⚠️ Still present (see below)
5	CodexAppServerTurnSchema enum breaks on unknown status	✅ Fixed — turn status is z.string() (codex-app-server.ts:434)
6	orderSpanForItem coupling undocumented	✅ Fixed — comment added (agents-ui-stream-service.ts:63)

---

Remaining + new findings (clean/simple/no-duplication focus)

A. WorktreeConversationPanel.svelte — the scroll $effect duplicates the shared signature helper and uses a void dance

WorktreeConversationPanel.svelte:147-158 reads conversationId / messageCount / lastMessageId / lastMessageTextLength inline, then voids the last four:

$effect(() => {
  const conversationId = conversation?.conversationId ?? null;
  const messageCount = conversation?.messages.length ?? 0;
  const lastMessageId = messageCount > 0 ? conversation?.messages[messageCount - 1]?.id ?? null : null;
  const lastMessageTextLength = messageCount > 0 ? conversation?.messages[messageCount - 1]?.text.length ?? 0 : 0;
  if (!conversationId || !transcriptViewport) return;
  void scrollTranscriptToBottom();
  void conversationId;       // ← these four add nothing
  void messageCount;
  void lastMessageId;
  void lastMessageTextLength;
});

Two things here:

• The void conversationId/... statements don't establish reactive dependencies — those are already captured by the property reads on lines 148-151 (which run before the early return). The voids only exist to silence "unused local" for lastMessageId/lastMessageTextLength. It reads like dependency-tracking but isn't.
• This inline computation duplicates buildConversationProgressSignature (worktree-conversation.ts:187), which already packages exactly these fields and is the helper MobileChatSurface.svelte:170,188 uses for the same "did the conversation make progress" question.

Simpler and DRY:

const progressSignature = $derived(buildConversationProgressSignature(conversation));

$effect(() => {
  void progressSignature;          // re-run on any conversation progress
  if (!conversation || !transcriptViewport) return;
  void scrollTranscriptToBottom();
});

Fix this →

B. Issue #4 — the renamed re-export is also dead code in the backend

backend/src/domain/agents-ui.ts:2 re-exports AgentsSendMessageRequest as AgentsUiSendMessageRequest, but nothing in backend/ ever imports AgentsUiSendMessageRequest (the frontend has its own separate alias in types.ts:73). So this line is both the naming inconsistency flagged before and unused. Simplest fix: drop the line entirely. (If you want the AgentsUi-prefixed name to be canonical, rename AgentsSendMessageRequest in packages/api-contract/src/schemas.ts instead of aliasing — but removing the dead re-export is the minimal change.)

Fix this →

C. worktree-conversation-service.ts — 4 tool-call builders share a duplicated result-append tail

buildCommandExecutionMessages (:280), buildFileChangeMessages (:328), buildMcpToolCallMessages (:368), and buildDynamicToolCallMessages (:411) each end with the identical pattern:

const resultText = ...;
if (resultText.length === 0) return [toolUse];
return [toolUse, { id: `${item.id}:result`, order: order + 1, role: "user", kind: "toolResult", toolName, toolCallId: item.id, text: resultText, status, createdAt, /* +durationMs/exitCode */ }];

The :result message construction is repeated 4× with only the extra fields (command/cwd/exitCode vs durationMs) differing. Since you asked specifically about duplication, this is the clearest spot: a small withToolResult(toolUse, resultText, extras) helper would collapse the four tails into one. The toolUse objects differ enough that I'd leave those inline — only the result tail is worth extracting. Judgment call, not a blocker.

Fix this →

D. Carried-over UX nuance (minor, confirm intended)

isVisibleTranscriptMessage (:106) treats any toolUse as visible, while showProcessingIndicator (:47) stays true during a running turn. So an in-progress tool with empty input text renders an empty "Running …" card and the "{agent} is processing" indicator stacked below it. Defensible, but reads as slightly redundant — flagging only to confirm it's the intended look.

---

Net: clean PR. Issues #1–#3, #5, #6 from prior rounds are resolved. The above are the leftover simplification/DRY opportunities (A and C most relevant to your "no duplication / no complex logic" ask). None are correctness bugs.
· Branch preview