fix(runner): fail the turn instead of answering with a lost conversation - #5493
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
Two silent failure modes met in the same place. `fetchSessionRecords` returned null on any error and the caller treated that as "keep the inbound history"; once the client sends only its last message, that history is one message, so an unreadable log produced an agent that answered as if the user had just arrived, with no error anywhere on the wire. Separately, `takePersistFailures` had no caller outside the tests, so a dropped record was counted into a map nothing read, and the next turn reconstructed from a log with a hole in it. Consume the drop count at the turn-end drain, where the docstring already said it happened, and mark the session. Reconstruction now fails the turn for an unreadable log or a session known to have lost a record, so the caller sees an error rather than a confident wrong answer. Requests that still send their own history are unaffected: they never reach this path. Claude-Session: https://claude.ai/code/session_01KM69J7uHafgciiN5zfG7qR
3286a79 to
5a00886
Compare
8f19a97 to
f621400
Compare
Railway Preview Environment
|
The problem
Two silent failure modes that both end with the agent answering as though the conversation never happened.
An unreadable record log looked like success.
fetchSessionRecordscatches every error, logs one line to stderr, and returnsnull. The caller treatednullas "keep the inbound history", which was safe while the client still sent the whole conversation. Once the client sends only its last message, the inbound history is one message. So a failed query produced a model turn with no prior context at all, and answered confidently. Nothing on the wire said anything went wrong: no error frame, no retry, no signal to the caller.A dropped record was counted and then ignored.
takePersistFailureshad no caller outside the unit tests. Its own docstring says the turn-end drain uses it to decide "whether the durable history is complete enough to reconstruct model context from", and nothing did that. So when durable ingest exhausted its retries, the count went into a map nothing read, the client still discarded its history on the next turn, and reconstruction rebuilt a conversation with a hole in it.Reported by Codex; confirmed by reading the call graph.
The fix
Consume the drop count at the turn-end drain, which is where the docstring already claimed it happened, and mark the session's log as incomplete.
Then make reconstruction refuse rather than guess. When the client sent only its last message there is no history to fall back on, so an unreadable log or a session known to have lost a record fails the turn and the caller sees an error instead of a wrong answer delivered with confidence.
Requests that still send their own history never reach this path and are unaffected.
Before / after
Testing
Two new cases assert the turn rejects on an unreadable log and on a session marked incomplete, and that the second case never issues a query. Full runner unit suite green. Context: QA results.