-
Notifications
You must be signed in to change notification settings - Fork 66
Add session manager #48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…variables - Update mocking patterns to properly isolate tests from environment variables - Mock os.getenv, os.getcwd, and Path to ensure consistent test behavior - Fixes tests that were failing due to environment variable interference
**Configuration:** | ||
- Use `--session-path <path>` to specify where sessions are stored | ||
- Or set `STRANDS_SESSION_PATH` environment variable | ||
- Sessions are only enabled when a path is provided |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I dont think we need a path defined, it uses the temp dir automatically. Can we make this parameter optional?
# Create session manager | ||
session_manager = create_session_manager(resolved_session_id, session_base_path) | ||
if session_manager: | ||
resolved_session_id = session_manager.session_id |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why do you keep setting resolved_session_id? Is it different then the one passed in?
) -> Optional[FileSessionManager]: | ||
"""Create a FileSessionManager with the given or generated session ID. Returns None if no base_path.""" | ||
if not base_path or not validate_session_path(base_path): | ||
return None |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we raise here instead?
return session_manager, resolved_session_id, is_resuming | ||
|
||
|
||
def handle_session_commands(command: str, session_id: Optional[str], session_base_path: Optional[str]) -> bool: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you document what the expected failure cases are? If we are going to rely on session persistence, then we should probably fail if its not working.
console.print(f"[bold cyan]Total messages:[/bold cyan] {info['total_messages']}") | ||
return True | ||
|
||
elif command == "session list" and session_base_path: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer if the keyword command handling was all done in the strands.py file, and we just have a list_sessions function in this file. I think that would be a bit more readable. That way we can remove all (or most) of the console printing logic from this file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually have more changes on top of it. I am extracting commands to commands_util.py
and i want to use prompt toolkit for autocomplete. I think it'll be useful later when we bring mcp prompt support too
Description
This PR introduces comprehensive session management functionality to the Strands Agent Builder, enabling users to persist and resume conversation sessions across CLI invocations. The implementation provides a secure, robust foundation for maintaining conversation context between terminal sessions.
Key Features Added:
🔄 Session Persistence & Management
🖥️ Enhanced CLI Interface
--session-path <path>
- Specify session storage directory--session-id <id>
- Resume specific session or create with custom ID--list-sessions
- Display all available sessions with detailsSTRANDS_SESSION_PATH
for default session storage location!session info
- Show current session details!session list
- List all available sessions!session
- Display help for session commands🎨 Rich User Experience
Implementation Details:
Core Architecture
session_utils.py
: Complete session management utility module with 15+ functionsvalidate_session_id()
andvalidate_session_path()
for securityRelated Issues
Adding a way to store/continue conversations #31
Documentation PR
[No associated documentation PR at this time]
Type of Change
Testing
Automated Testing
hatch fmt --linter
- All linting checks pass ✅hatch fmt --formatter
- Code formatting is consistent ✅hatch test
- All existing and new tests pass ✅Checklist
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.