fix(core,sdk): stop chat.agent losing messages during recovery - #4907
Conversation
🦋 Changeset detectedLatest commit: 2ece24d The changes in this PR will be included in the next version bump. This PR includes changesets to release 27 packages
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 |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📜 Recent review details⏰ Context from checks skipped due to timeout. (41)
🧰 Additional context used📓 Path-based instructions (7)Always import from `@trigger.dev/sdk`.📄 CodeRabbit inference engine (AGENTS.md) Files:
**Prefer static imports over dynamic imports.**📄 CodeRabbit inference engine (AGENTS.md) Files:
Add crumbs as you write code — not just when debugging.📄 CodeRabbit inference engine (AGENTS.md) Files:
In the Trigger.dev SDK (packages/trigger-sdk), prefer isomorphic code like fetch and ReadableStream instead of Node.js-specific code📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Use function declarations instead of default exports📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
Use types over interfaces for TypeScript Avoid using enums; prefer string unions or const objects instead📄 CodeRabbit inference engine (.github/copilot-instructions.md) Files:
When creating or editing OTEL metrics (counters, histograms, gauges), ensure metric attributes have low cardinality by using only enums, booleans, bounded error codes, or bounded shard IDs Do not use high-cardinality attributes in OTEL metr...📄 CodeRabbit inference engine (.cursor/rules/otel-metrics.mdc) Files:
🔇 Additional comments (12)
WalkthroughThe session router now tracks recovered sequence claims and unsettled claims separately. It drops late live-tail records for claimed sequences and holds the resume floor until each claim settles. Chat recovery passes individual sequence numbers to the router and settles dispatched or undispatched recovered records. Tests cover router floor behavior and prevent duplicate recovered chat messages and turns. Priority: ➖ Normal Severity of issue fixed: Medium Merge Risk: 🔵 Low · up to Recovery now claims and settles individual messages to prevent message loss, but a gap in recovered sequences can still advance a replay cutoff too far in a narrowed recovery path, potentially suppressing a message. 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
Full details: Description checkExplanation The description gives a clear and relevant technical summary of the fix, but it does not follow the repository template. It omits the issue closure line, checklist, Testing section, Changelog section, and Screenshots section. Full details: Docstring CoverageExplanation Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files. (1 skipped: 1 too large.) ✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 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 |
@trigger.dev/build
trigger.dev
@trigger.dev/core
@trigger.dev/python
@trigger.dev/react-hooks
@trigger.dev/redis-worker
@trigger.dev/rsc
@trigger.dev/schema-to-json
@trigger.dev/sdk
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
7d29239 to
9a2cfbb
Compare
52addef to
1268dba
Compare
1268dba to
95d3ab5
Compare
Rebased onto the transcript-storage stack. A continuation boot claims recovered session.in seqNums on the router and holds the resume cursor behind each until the boot settles it, so suppressing the tail's re-answer no longer advances the cursor past an un-answered message. Adds a changeset and router/boot tests.
95d3ab5 to
2ece24d
Compare
1 participant
Summary
When a chat.agent run boots to continue a session (a version handover, or a retry after a crash), it replays the unacknowledged user messages off
session.inand dispatches them itself. A previous change stopped the live tail from re-answering those same messages by folding them into the resume cursor in one step. That cursor is what the next boot reads to know where to resume, and folding in every recovered message at once let it advance past a message the run had not answered yet. So if the run answered the first recovered message, wrote its turn boundary, then crashed before dispatching the rest, the next boot resumed past those messages and they were never answered.Fix
A recovered message is now claimed on the session-stream router instead of folded into the cursor. A claim does two independent things:
The boot settles each claim as it dispatches the message, or right away for a message it folds into the seed chain or deliberately skips, so the cursor only advances over messages that have actually been handled. A claimed record whose route re-read never arrives over the tail degrades to being answered twice on the next boot, never to being dropped.
Covered by router-level unit tests for the claim/settle floor and a chat.agent boot test asserting the cursor published after the first recovered turn stays behind the still-unanswered ones.