WIP: fix(ai-chat) prevent duplicate assistant persistence on history round-trips

[whoiskatrin]

Summary

WIP fix for #957.

This PR addresses duplicate assistant rows being persisted when the client resends full chat history with local assistant IDs that differ from server-persisted IDs.

Root cause

persistMessages() previously upserted by message.id.

For plain-text assistant messages (no toolCallId), when a client round-tripped history using a different local assistant ID, the server treated it as a new message and inserted a duplicate row.

Tool messages were already protected by _resolveMessageForToolMerge, but plain assistant text messages were not.

What changed

1) Repro test added

• File: packages/ai-chat/src/tests/merge-server-state.test.ts
• Added test:
  • reuses server assistant IDs for plain text messages to avoid duplicates
• The test reproduces the issue:
  1. Persist user + assistant(server ID)
  2. Persist full history again with same assistant content but different client ID
  3. Assert no duplicate assistant row and server ID is reused

2) ID reconciliation for non-tool assistant messages

• File: packages/ai-chat/src/index.ts

• _mergeIncomingWithServerState() now:

  1. keeps existing tool-output merge behavior
  2. runs assistant ID reconciliation against current server history

• Added helpers:

  • _reconcileAssistantIdsWithServerState(...)
  • _hasToolCallPart(...)
  • _assistantMessageContentKey(...)

Reconciliation behavior

For assistant messages without tool parts:

• if incoming ID already exists on server: unchanged
• else, if content (sanitized.parts) matches a server assistant message in sequence: replace incoming ID with server ID
• else: unchanged

This preserves current tool-flow logic and prevents duplicate inserts for plain assistant text round-trips.

Why this approach

• Keeps server as source of truth for persisted assistant IDs.
• Fixes duplication without requiring client transport protocol changes.
• Avoids touching tool-call merge paths that already have dedicated logic.

Reviewer guide

Suggested review order

1. Test first
   • packages/ai-chat/src/tests/merge-server-state.test.ts
   • Verify the new test reproduces AIChatAgent: assistant messages duplicated in SQLite on every saveMessages() round-trip #957 semantics.
2. Fix implementation
   • packages/ai-chat/src/index.ts
   • Start at _mergeIncomingWithServerState() and then inspect new helper methods.
3. Regression considerations
   • Confirm tool-call paths remain handled by existing _resolveMessageForToolMerge behavior.

What to validate

• No duplicate assistant rows when same assistant content is round-tripped with different IDs.
• Existing tool merge tests still pass.
• Existing non-tool persistence behavior remains stable when IDs already match.

Commands used locally

cd packages/ai-chat
npm run test:workers -- merge-server-state.test.ts
npm run test:workers -- merge-server-state.test.ts client-tool-duplicate-message.test.ts

Tests

• check e2e scenarios in all existing examples

[changeset-bot]

🦋 Changeset detected

Latest commit: 10b657a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@cloudflare/ai-chat	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[pkg-pr-new]

Open in StackBlitz

npm i https://pkg.pr.new/cloudflare/agents@958

npm i https://pkg.pr.new/cloudflare/agents/@cloudflare/ai-chat@958

npm i https://pkg.pr.new/cloudflare/agents/@cloudflare/codemode@958

npm i https://pkg.pr.new/cloudflare/agents/hono-agents@958

commit: 10b657a

[whoiskatrin]

/bonk review this PR

[whoiskatrin]

/bonk re-review this PR

[ask-bonk]

Re-review: fix(ai-chat) prevent duplicate assistant persistence on history round-trips

Test results

All 174 tests pass across 22 test files on this branch (workers project), including:

• 4 tests in merge-server-state.test.ts (2 existing + 2 new)
• 21 tests in client-tool-duplicate-message.test.ts (no regressions)

File-by-file analysis

.changeset/calm-pens-juggle.md — Correct package (@cloudflare/ai-chat), correct semver (patch), clear description. Good.

examples/playground/testing.md — Escapes a markdown glob (X-Agent-* → X-Agent-\*). Unrelated to the fix. Should be a separate commit.

package-lock.json — Peer dependency flag reshuffling. These "peer": true additions/removals look like lockfile regeneration noise from npm install. Not concerning, but worth confirming this wasn't an unintended dependency change.

Implementation review (packages/ai-chat/src/index.ts)

The refactor of _mergeIncomingWithServerState cleanly separates two concerns:

1. Tool output merging (existing logic, now in withMergedToolOutputs)
2. Assistant ID reconciliation (new _reconcileAssistantIdsWithServerState)

This is a good structural improvement.

Reconciliation logic (lines 1042-1103)

The serverCursor approach is sound — it enforces positional ordering and prevents a single server message from matching multiple incoming messages with identical content. The algorithm:

1. Fast path (line 1059): exact ID match advances cursor for any role
2. Skip non-candidates (line 1068): non-assistant or tool-bearing messages fall through
3. Content match (line 1091): scans forward from cursor for matching sanitized content
4. Fallback (line 1101): unmatched messages pass through unchanged

Concern: content-match scan doesn't advance cursor on non-matches

In the content-matching loop (lines 1082-1099), when a server message is assistant + non-tool but its content doesn't match, the cursor is NOT advanced past it. This means a later incoming message could still match that skipped server message. This is intentional and correct — you don't want to burn a server message that might match a later incoming message. But it does mean the algorithm is O(n*m) in the worst case (n incoming × m server assistant messages). For typical chat histories this is fine, but worth noting.

Concern: double reconciliation for tool-bearing assistant messages

The flow is:

1. _mergeIncomingWithServerState → merges tool outputs → calls _reconcileAssistantIdsWithServerState
2. persistMessages loop → calls _resolveMessageForToolMerge per message

Tool-bearing assistant messages are excluded from step 1's reconciliation (line 1070), so they hit _resolveMessageForToolMerge in step 2 as before. This is correct — no double processing. Good.

Nit: _assistantMessageContentKey calls _sanitizeMessageForPersistence redundantly

At line 1116, _assistantMessageContentKey sanitizes the message to compute a content key. But in persistMessages (line 936), the message is sanitized again for actual persistence. For the reconciliation path, the incoming message is sanitized once for key computation and once for storage — two sanitization passes. This is functionally correct (sanitization is idempotent) but costs extra allocations. Not a blocker, but could be optimized if this path becomes hot.

Test review (merge-server-state.test.ts)

Test: "reuses server assistant IDs for plain text messages to avoid duplicates" (lines 107-156) — Correctly reproduces #957: persists with server ID, re-persists same content with different client ID, asserts server ID is reused and no duplicate. Good.

Test: "matches repeated assistant text messages in order" (lines 158-219) — Tests the serverCursor ordering logic with two assistant messages having identical content. Verifies each maps to the correct server ID by position. This directly addresses the previous review's suggestion. Good.

Suggestions (non-blocking)

1. Move the testing.md change to a separate commit — keeps the fix atomic.

2. Add a brief inline comment explaining why the content-match loop doesn't skip non-assistant server messages — the continue on line 1088 skips non-assistant and tool-bearing server messages but doesn't advance the cursor past them. A reader might wonder if non-assistant messages should also advance the cursor to maintain strict ordering. A comment like // Don't advance cursor: non-assistant messages don't participate in content matching would help.

3. Consider whether _reconcileAssistantIdsWithServerState should also handle the edge case where the client removes messages from history — if a client sends a shorter history than what's on the server, the cursor approach still works (unmatched server messages are simply not reused), but the orphaned server rows remain. This is existing behavior, not introduced by this PR, but worth documenting as a known limitation if it matters for #957.

Verdict

The fix is correct, well-tested, and well-scoped. The serverCursor approach for positional content matching is the right design choice. All existing tests pass — no regressions in tool merge paths or other persistence behavior. The second test (duplicate content ordering) was a good addition from the previous review's feedback.

Approve with minor suggestions — the optional feedback above can be addressed in follow-up or as part of removing the WIP status.

[ask-bonk]

Re-review posted. Here's the summary:

All 174 tests pass across 22 test files — no regressions.

The fix is correct and well-tested. The _reconcileAssistantIdsWithServerState method uses a serverCursor to match incoming assistant messages to server messages by content and position, preventing duplicate rows when clients resend history with different local IDs. Tool-bearing messages are correctly excluded, deferring to the existing _resolveMessageForToolMerge path.

Key observations:

• The refactor cleanly separates tool output merging from ID reconciliation
• The new "matches repeated assistant text messages in order" test addresses the previous review's feedback about identical-content edge cases
• Minor non-blocking suggestions: separate the testing.md change, add a clarifying comment in the content-match loop, and note the redundant sanitization pass in _assistantMessageContentKey

Verdict: Approve with minor suggestions — ready to merge after removing WIP status.

github run

[whoiskatrin]

examples/playground/testing.md is annoying, but for some reason fails the CI