fix: paginate thread history when forking and loading sessions - #481
Merged
nikita-ashihmin merged 4 commits intoSep 6, 2026
Merged
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Cursor-cycle detection is required to prevent malformed pagination responses from causing an infinite loop.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Adds cursor-based thread-history pagination while preserving replay and fork-point matching.
Changes:
- Loads full turns chronologically via paginated
thread/turns/list. - Excludes turns from resume, fork, and audit-fork responses.
- Adds pagination, failure, and fingerprint-matching tests.
File summaries
| File | Description |
|---|---|
src/SessionFork.ts |
Resolves fork points from paginated history. |
src/CodexAppServerClient.ts |
Implements pagination, but repeated or cyclic cursors can cause an infinite request loop. |
src/CodexAcpClient.ts |
Uses metadata-only resume and fork responses. |
src/__tests__/CodexACPAgent/thread-history.test.ts |
Tests pagination and failure handling. |
src/__tests__/CodexACPAgent/load-session.test.ts |
Updates session-loading mocks and assertions. |
src/__tests__/CodexACPAgent/data/paginated-thread-history.json |
Provides expected paginated-history data. |
src/__tests__/CodexACPAgent/CodexAcpClient.test.ts |
Tests resume, fork, and fingerprint behavior. |
src/__tests__/CodexACPAgent/agent-file-change-report.test.ts |
Verifies metadata-only audit forks. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+587
to
+599
| const turns: ThreadReadResponse["thread"]["turns"] = []; | ||
| let cursor: string | null = null; | ||
| do { | ||
| const page = await this.threadTurnsList({ | ||
| threadId, | ||
| cursor, | ||
| limit: 50, | ||
| sortDirection: "asc", | ||
| itemsView: "full", | ||
| }); | ||
| turns.push(...page.data); | ||
| cursor = page.nextCursor; | ||
| } while (cursor !== null); |
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate issues prevent history pagination from providing a consistent snapshot.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 2
- Review effort level: Balanced
| threadId: response.thread.id, | ||
| includeTurns: true, | ||
| }); | ||
| const historyResponse = await this.codexClient.threadReadWithHistory(response.thread.id); |
| threadId, | ||
| cursor, | ||
| limit: 50, | ||
| sortDirection: "asc", |
There was a problem hiding this comment.
🟡 Changes recommended
Legacy session history must retain its required full-read path before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- Review effort level: Balanced
Comment on lines
+585
to
+589
| async threadReadWithHistory(threadId: string): Promise<ThreadReadResponse> { | ||
| const response = await this.threadRead({threadId}); | ||
| const turns = await this.threadReadHistory(threadId); | ||
| return {...response, thread: {...response.thread, turns}}; | ||
| } |
nikita-ashihmin
deleted the
nikita.ashikhmin/fix-fork-history-pagination
branch
September 6, 2026 09:17
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Forking and loading paginated threads emitted repeated “Full-history hydration is deprecated” warnings because the adapter requested complete history through
thread/read,thread/fork, andthread/resume.Load history through descending
thread/turns/listpages withitemsView: "full", then reverse the turns for chronological replay. Paginated session loads use the resume response'sturnsBackwardsCursoras the upper boundary; a null resume cursor means empty durable history. Standalone reads and AIR fork-point resolution start at the newest page so newly appended turns do not enter subsequent pages. Legacy histories usethread/read(includeTurns: true)instead of pagination, avoiding repeated rollout reconstruction and dependence on the paging API. Repeated or cyclic cursors fail explicitly.Use
excludeTurns: truefor session resume, session fork, and temporary audit forks. Preserve complete items, their order within each turn, and AIR fork-point matching across pages.Validation:
npm run typecheckandnpm run buildpassed.npm test: 552 passed, 26 skipped.