-
Notifications
You must be signed in to change notification settings - Fork 2.7k
fix: create new session when goose fails to find existing session #5200
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
Conversation
Signed-off-by: Robert Sturla <[email protected]>
|
thanks for diving on this. this would probably help, but I would love to understand better why this is happening in the first place? if you just have goose session, why does it come up with a session that does not exist? |
| Ok(session) => session.conversation.unwrap_or_default(), | ||
| Err(_) => { | ||
| // If we can't load the session, assume it's new and use empty conversation | ||
| Conversation::new_unvalidated(Vec::new()) |
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 remove the comment here and print something sternly here? like what the session_id was that we thought we were loading? I would like to know how we get us a session_id that doesn't exist; the only way to get in that situation should be that the user entered an invalid one
|
This occurs when a new user attempts to start Goose in the first place. The user has no existing sessions Will action your other comment now, and spend a few minutes diving deeper into the root cause. Edit: |
|
yeah we have: so session_id must be set. so it must come from somewhere. what is the value? |
|
thanks. interesting. so 20251016_1 is the session id of a new session that the session manager made for us, not some random string. which is what you would expect, you start and we create a new session and then try to load that. why did it not get created is then the question? if you could give it a bit more time looking into? |
|
On starting a new session with ( So session_id is being generated here if there is no session. The only time when there is no session_id in the So now the question is: Why does |
|
I think the problem might be that we try to load the session where it crashes in a different context. so the session has been created but might not have made it to the db. since we read it from the other thread, it might not exist in that context yet and we die. |
|
I think you're right. Though I'm not quite sure what the desired fix here would be. |
Summary
Do not panic on startup when no sessions exist.
This fixes an issue where the user’s first session with Goose fails with a panic before it even begins. This is because the previous code unconditionally unwrapped the result of
SessionManager::get_session, causing a panic when no session data was found or when session loading failed for any reason.The new implementation handles this case gracefully by returning an empty, unvalidated
Conversationwhen a session cannot be loaded, allowing Goose to start cleanly even when no prior sessions exist.Type of Change
Testing
This has been tested manually by configuring Goose with GitHub Copilot and running
goosein a clean environment. Previously this caused an error. Now it correctly starts a new session.Related Issues
Relates to #5197
Discussion: LINK (if any)
Screenshots/Demos (for UX changes)
Before:
After:
Email: