Session store cleanup #1668
Open
Session store cleanup #1668
+50
−234
Conversation
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
ResolveSessionID now uses the existing GetSessionSummaries method, which already returns root sessions sorted by created_at DESC. Also fixes InMemorySessionStore.GetSessionSummaries to filter out sub-sessions and sort by CreatedAt descending, matching the SQLite implementation. Assisted-By: cagent Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
Export buildBranchedSession as session.BranchSession so callers compose it with GetSession and AddSession directly, rather than having the store own branching logic. Also removes the now-unused collectSessionIDs helper. Assisted-By: cagent Signed-off-by: Djordje Lukic <djordje.lukic@docker.com>
![docker-agent[bot]](https://avatars.githubusercontent.com/in/2787757?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Summary
I've completed a thorough review of this PR. The refactoring successfully removes the BranchSession and GetSessionByOffset methods from the Store interface and moves them to more appropriate locations:
BranchSessionis now an exported function in the session packageGetSessionByOffsetlogic is inlined intoResolveSessionID
No bugs found in the changed code. All call sites have been correctly updated:
- Tests now call
BranchSessiondirectly and manually save viaAddSession - TUI handler follows the same pattern: load parent → branch → save
ResolveSessionIDcorrectly inlines the offset logic with proper bounds checkingGetSessionSummariesnow filters out subsessions and sorts by creation time
The refactoring improves the architecture by:
- Making the Store interface simpler and more focused
- Moving branching logic to the domain layer where it belongs
- Reducing unnecessary coupling between the store and session branching
Looks good! ✅
krissetto
approved these changes
Feb 9, 2026
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Remove too-specific methods from the session store
These are not really needed in the store interface