Skip to content

Sweep: replace silent timeout-removal on optimistic UI with visible 'not acknowledged + retry' #72

Description

@amirfish1

Context

Commit 0f9fea7 fixed a long-standing class of bug: a pending send echo (.event.user_text.pending) was silently removed by a setTimeout(_PENDING_SEND_ECHO_MAX_MS) if no matching JSONL event arrived. The user's typed text vanished and they assumed delivery — when in fact the agent never received it. Same shape as earlier 0f2e3ff (over-strip swallow).

The fix converts the optimistic bubble to a .not-acknowledged state with a "Re-send" button + dismiss, preserving the typed text so the user can copy or retry. Pattern works well — worth applying everywhere else the same anti-pattern lurks.

The anti-pattern

const optimistic = document.createElement('div');
// ... render optimistic UI
setTimeout(() => removeOptimistic(...), MAX_MS);

If the canonical "real event arrived" handler doesn't fire in time, the optimistic UI vanishes silently. The user has no signal that something failed.

Sweep checklist

Search the codebase for these patterns and decide each case:

  • setTimeout callbacks that remove DOM nodes the user could have typed into or interacted with.
  • Optimistic placeholders that get dropped on _PENDING_*_MAX_MS style constants (_PENDING_SEND_ECHO_MAX_MS, _SENDING_TIMEOUT_MS, etc).
  • Any path where a user action triggers UI that depends on a secondary event (server response, JSONL stream, websocket) to confirm — and the secondary event might not arrive.

Likely candidates

  • Group-chat send (addSessionToGroupChat, group chat reader send path) — does the typed message get a not-acknowledged treatment if the server never registers it?
  • Annotation submission/api/annotations POST; if it 500s or never returns, does the user know?
  • Codex steer requestssendToTerminal('p1', 'steer') → does failure surface?
  • Spawn placeholders (pendingSpawns map) — auto-cleanup at 30s (line ~6601) for Claude placeholders; user might not realize the spawn died.
  • Group-chat createEmptyGroupChat — if /api/coordinate succeeds but the chat never appears in _gcActiveChats, does the toast lie?
  • appendSendFailureNotice — this one DOES show a notice, but verify it surfaces in every error branch and isn't swallowed by another timeout.

Suggested fix shape (already applied to pending send)

setTimeout(() => {
  const div = pending.element;
  if (!div || !div.parentNode) return;
  div.classList.remove('pending');
  div.classList.add('not-acknowledged');
  div.appendChild(buildNotAckNote({ onRetry, onDismiss }));
}, MAX_MS);

Plus matching CSS so the not-ack state reads as a real failure indicator (red border, "× / Re-send" buttons).

Reference

  • The canonical fix: 0f9fea7fix(send): not-acknowledged echo → visible notice + re-send
  • Earlier related: 0f2e3fffix(conv): user_text fallback when cleanIssuePrompt over-strips

Out of scope

  • Don't refactor _pendingSends map shape — it's working.
  • Don't change _PENDING_SEND_ECHO_MAX_MS — 5m15s is the right window.
  • Don't add server-side ack handshakes — the fix is purely UI honesty.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions