fix(backend/copilot): prevent title update from overwriting session messages - #12302
Conversation
…essages The background title generation task in update_session_title() performs a read-modify-write on the Redis session cache (read session → set title → write back). This was not synchronized with upsert_chat_session() which persists messages during streaming. Race condition: if the title task reads the cache between streaming turns (e.g., when the session has 34 messages), then writes it back after streaming completes (when the session has 101 messages), the write overwrites the 101-message version with the stale 34-message snapshot. When the next user message lands on a different pod, it loads the stale session from Redis and the model "forgets" everything that happened. Fix: acquire the same _get_session_lock used by upsert_chat_session before the read-modify-write in update_session_title. Since the title background task always runs on the same pod as the streaming function, the in-process asyncio lock serializes the two operations correctly.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📜 Recent review details⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
🧰 Additional context used📓 Path-based instructions (4)autogpt_platform/backend/**/*.py📄 CodeRabbit inference engine (.github/copilot-instructions.md)
Files:
autogpt_platform/backend/**/*.{py,txt}📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Files:
autogpt_platform/backend/backend/**/*.py📄 CodeRabbit inference engine (autogpt_platform/backend/CLAUDE.md)
Files:
autogpt_platform/**/*.py📄 CodeRabbit inference engine (AGENTS.md)
Files:
🧠 Learnings (4)📓 Common learnings📚 Learning: 2026-02-26T17:02:22.448ZApplied to files:
📚 Learning: 2026-03-04T08:04:35.881ZApplied to files:
📚 Learning: 2026-03-05T15:42:08.207ZApplied to files:
🔇 Additional comments (1)
WalkthroughReplaced an in-place cached-session update in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔍 PR Overlap DetectionThis check compares your PR against all other open PRs targeting the same branch to detect potential merge conflicts early. 🔴 Merge Conflicts DetectedThe following PRs have been tested and will have merge conflicts if merged after this PR. Consider coordinating with the authors.
Summary: 1 conflict(s), 0 medium risk, 0 low risk (out of 1 PRs with file overlap) Auto-generated on push. Ignores: |
Replace the lock + read + mutate + write pattern with a simple cache invalidation. This eliminates the race condition more robustly — no stale snapshot can ever overwrite fresh data.
…essages (#12302) ### Changes 🏗️ Fixes a race condition in `update_session_title()` where the background title generation task could overwrite the Redis session cache with a stale snapshot, causing the copilot to "forget" its previous turns. **Root cause:** `update_session_title()` performs a read-modify-write on the Redis cache (read full session → set title → write back). Meanwhile, `upsert_chat_session()` writes a newer version with more messages during streaming. If the title task reads early (e.g., 34 messages) and writes late (after streaming persisted 101 messages), the stale 34-message version overwrites the 101-message version. When the next message lands on a different pod, it loads the stale session from Redis. **Fix:** Replace the read-modify-write with a simple cache invalidation (`invalidate_session_cache`). The title is already updated in the DB; the next access just reloads from DB with the correct title and messages. No locks, no deserialization of the full session blob, no risk of stale overwrites. **Evidence from prod logs (session `41a3814c`):** - Pod `tm2jb` persisted session with 101 messages - Pod `phflm` loaded session from Redis cache with only 35 messages (66 messages lost) - The title background task ran between these events, overwriting the cache ### Checklist 📋 #### For code changes: - [x] I have clearly listed my changes in the PR description - [x] I have made a test plan - [x] I have tested my changes according to the test plan: - [x] `poetry run pytest backend/copilot/model_test.py` — 15/15 pass - [x] All pre-commit hooks pass (ruff, black, isort, pyright) - [ ] After deploy: verify long sessions no longer lose context on multi-pod setups
Changes 🏗️
Fixes a race condition in
update_session_title()where the background title generation task could overwrite the Redis session cache with a stale snapshot, causing the copilot to "forget" its previous turns.Root cause:
update_session_title()performs a read-modify-write on the Redis cache (read full session → set title → write back). Meanwhile,upsert_chat_session()writes a newer version with more messages during streaming. If the title task reads early (e.g., 34 messages) and writes late (after streaming persisted 101 messages), the stale 34-message version overwrites the 101-message version. When the next message lands on a different pod, it loads the stale session from Redis.Fix: Replace the read-modify-write with a simple cache invalidation (
invalidate_session_cache). The title is already updated in the DB; the next access just reloads from DB with the correct title and messages. No locks, no deserialization of the full session blob, no risk of stale overwrites.Evidence from prod logs (session
41a3814c):tm2jbpersisted session with 101 messagesphflmloaded session from Redis cache with only 35 messages (66 messages lost)Checklist 📋
For code changes:
poetry run pytest backend/copilot/model_test.py— 15/15 pass