fix(copilot): make user-message persistence idempotent on message_id#4811
fix(copilot): make user-message persistence idempotent on message_id#4811waleedlatif1 wants to merge 1 commit into
Conversation
persistUserMessage appended to copilot_chats.messages unconditionally
(messages || [userMsg]::jsonb) with no check that the id already exists.
A repeated POST for the same userMessageId — network retry, double
submit, React double-effect, or a bypassed/expired send lock (Redis-down
no-op) — therefore tacked on a duplicate array entry. The assistant
append in terminal-state.ts is already guarded; this brings the user
append to parity. (Evidence: 100% of duplicated message_ids in prod are
role=user.)
Guard the append with a containment check so it only concatenates when
the array doesn't already hold a message with this id:
CASE WHEN messages @> [{id}] THEN messages
ELSE messages || [userMsg] END
The client mints a fresh id per distinct send (verified across all hook
revisions), so a repeated id is always the same message — skipping is
safe and never drops a distinct message. conversationId/updatedAt stay
unconditional so the stream marker still advances on a retry. The
dual-write to copilot_messages is already idempotent (ON CONFLICT).
Verified on Postgres: same id -> stays 1 element; different id -> 2.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryLow Risk Overview
Reviewed by Cursor Bugbot for commit a863c99. Configure here. |
Greptile SummaryThis PR fixes a duplicate-entry bug in
Confidence Score: 4/5The SQL guard is correct and the change is intentionally minimal — safe to merge with one observability gap to consider. The JSONB containment check works correctly in Postgres, apps/sim/lib/copilot/chat/post.ts — the trace outcome on the THEN branch of the CASE expression. Important Files Changed
Sequence DiagramsequenceDiagram
participant Client
participant Server
participant DB as copilotChats
participant DW as copilotMessages
Client->>Server: POST userMessageId + message
Server->>DB: UPDATE CASE WHEN messages contains id THEN keep ELSE append END
DB-->>Server: updated row
Server->>DW: appendCopilotChatMessages ON CONFLICT DO UPDATE
DW-->>Server: ok
Note over Client,Server: Retry with same userMessageId
Client->>Server: POST same userMessageId
Server->>DB: UPDATE CASE WHEN detects existing id THEN messages unchanged
DB-->>Server: updated row stream marker advanced
Server->>DW: appendCopilotChatMessages idempotent no-op
DW-->>Server: ok
|
What
persistUserMessage(apps/sim/lib/copilot/chat/post.ts) appended the user message tocopilot_chats.messagesunconditionally —messages || [userMsg]::jsonb— with no check that the id already exists. This change guards the append so it only concatenates when the array doesn't already contain a message with that id.Why (root cause, verified)
A repeated POST for the same
userMessageId(network retry, double-submit, React double-effect, or the send lock being bypassed when Redis is down / its 60s TTL expires) tacked on a duplicate array entry. The assistant append interminal-state.tsis already guarded by afindIndexcheck; the user append was not — so duplicates only ever accrue on user messages.Evidence: across prod, 100% of duplicated
message_ids arerole:"user"(zero assistant) — exactly what this asymmetry predicts. A deep trace of the client hook across all revisions confirmed the client mints a fresh id per distinct send (queued sends, queue-edit, retry, recovery, replay all verified), so a repeated id reaching the server is always the same message. The unconditional server append was the sole mechanism creating the duplicates.The fix
conversationId/updatedAtstay unconditional, so the active-stream marker still advances on a retry.copilot_messagesis already idempotent (ON CONFLICT DO UPDATE), so no change needed there.Scope
Deliberately tiny — one SQL expression in
persistUserMessage. No client change (the client is already correct). No schema change.Verification
[{id:A}], appending idA→ stays 1 element; appending idB→ 2 elements.bun vitest run lib/copilot/chat— 68 tests pass; type-check clean; biome clean;check:api-validationpasses.Note
This stops new duplicates at the source. Historical duplicates (67 chats, mostly Sept–Oct 2025) are a separate, optional cleanup; they still live intact in the JSONB column.
🤖 Generated with Claude Code