Skip to content

fix: frontend-agnostic context overflow message, export recorded errors for recovery - #3920

Merged
trungutt merged 2 commits into
docker:mainfrom
trungutt:fix-context-overflow-error-message
Aug 6, 2026
Merged

fix: frontend-agnostic context overflow message, export recorded errors for recovery#3920
trungutt merged 2 commits into
docker:mainfrom
trungutt:fix-context-overflow-error-message

Conversation

@trungutt

@trungutt trungutt commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

What this PR does

Two fixes motivated by a diagnostics report where a run died with a fatal context overflow:

  1. 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 /compact command. 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.

  2. Include recorded errors in the session recovery export. Recorded error items (persisted since v1.100.0) are filtered out by GetAllMessages(), so ExportSessionForRecovery produced 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 separate errors key (omitted when the session has no recorded errors, so the shape is unchanged for the common case).

@trungutt
trungutt requested a review from a team as a code owner August 6, 2026 07:47

@docker-agent docker-agent left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@aheritier aheritier added area/api For features/issues/fixes related to the usage of the cagent API area/core Core agent runtime, session management area/sessions For features/issues/fixes related to session lifecycle (resume, persistence, export) area/testing Test infrastructure, CI/CD, test runners, evaluation kind/fix PR fixes a bug (maps to fix:). Use on PRs only. labels Aug 6, 2026
@trungutt
trungutt merged commit a2746bb into docker:main Aug 6, 2026
19 of 21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/api For features/issues/fixes related to the usage of the cagent API area/core Core agent runtime, session management area/sessions For features/issues/fixes related to session lifecycle (resume, persistence, export) area/testing Test infrastructure, CI/CD, test runners, evaluation kind/fix PR fixes a bug (maps to fix:). Use on PRs only.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants