fix: frontend-agnostic context overflow message, export recorded errors for recovery - #3920
Conversation
docker-agent
left a comment
There was a problem hiding this comment.
Assessment: 🟢 APPROVE
The two focused fixes in this PR are correct and well-tested:
1. Frontend-agnostic overflow message (): The updated text removes the incorrect claim that "automatic compaction is not enabled" (which was wrong when compaction was enabled but had exhausted its retry budget) and replaces the CLI-specific "/compact" command reference with a generic action description. Clean, correct change.
2. Export recorded errors for recovery (, ): GetAllErrors correctly snapshots items under the read lock via snapshotItems(), iterates error items, and recurses into sub-sessions. The conditional if errs := sess.GetAllErrors(); len(errs) > 0 correctly omits the errors key when empty, preserving the existing export shape for sessions with no recorded errors. The implementation is safe and consistent with the existing GetAllMessages pattern.
Test coverage is thorough: both the error-present and error-absent export cases are exercised, plus the new GetAllErrors unit test validates ordering and sub-session recursion.
What this PR does
Two fixes motivated by a diagnostics report where a run died with a fatal context overflow:
Make the token-overflow message frontend-agnostic. The fatal overflow error told users to "Try running /compact", but this error travels on the runtime event stream and reaches every consumer (API/serve clients, embedding hosts), not all of which expose a
/compactcommand. It also claimed "automatic compaction is not enabled" even when compaction was enabled but its overflow-retry budget was exhausted. The message now names the action ("Compact the conversation…") instead of a command and drops the wrong claim.Include recorded errors in the session recovery export. Recorded error items (persisted since v1.100.0) are filtered out by
GetAllMessages(), soExportSessionForRecoveryproduced a transcript that just stops mid-conversation with no trace of the failure that ended it. Diagnosing the report above required manually reproducing which error the user saw. The export now carries a separateerrorskey (omitted when the session has no recorded errors, so the shape is unchanged for the common case).