Clean up agent state between sessions #4126
Conversation
|
|
||
| // Reset session state if we're not resuming | ||
| if !session_config.resume { | ||
| agent.reset_session_state().await; |
There was a problem hiding this comment.
We should be careful how we reset and mutate things right? If the issue is that more state than intended is shared, could any of these changes clear state relevant to one session when starting another?
There was a problem hiding this comment.
I'm confused. We just created the agent on 211, how would we have any state to reset at this point?
There was a problem hiding this comment.
tbh I am confused about the whole TODO tool. I dont see where it loads the existing todo if the session is resumed
There was a problem hiding this comment.
I thought todo was in memory and if there is any x-pollution between sessions it is by mistake? and yes especially in CLI this seems really confusing
There was a problem hiding this comment.
Hey! Yes. It is in memory right now and agent scoped, which is what we discussed before.
But, yeah, this todo can easily be moved to being session scoped and persistent. Let me put up a PR for that instead of just patching the clearing functionality here
There was a problem hiding this comment.
@tlongwell-block How involved a change will it be? To get a fix out to include in #4154 would it be simpler just to disable the tools for now, then re-enable once a comprehensive fix is in?
|
what do you mean with "between sessions"? in general a session's properties should be stored in its session object and that is all the state it needs. |
|
Right now TODO Tool stores it's state inside the agent object. That's the major issue and we should just move it completely to the session? Would be great to persist it there too. The issue now is if you start a new session in the desktop it remembers you're old TODO state from fetching it from the agent. The agent is persisted across the entire process (between session). Maybe that's intended as are each of these subfields; in that case can nuke PR and just move TODO state into the session. |
we should talk about how the TODO thing is supposed to work, since it is all in memory, if you do resume a session, it will lose its TODO list I think. but to the point in case here, I don't think the agent persists between sessions since we have one agent per goosed and one goosed per session (or one agent per CLI session), so the leakage must occur elsewhere? |
|
if the session jsonl structure supports it the todo could be persisted along with session messages if you wanted it 1:1 mapping? (just it is treated differently to other messages/content?) |
|
Can we just disable the TODOs functionality until this can be properly figured out? |
sgtm #4158. Let's disable until we fix this. |
|
Here is the session scoped implementation for reference #4157 |
Main issue is TODO list is being persisted between sessions, but seems like in general a lot of this other state shouldn't be persisted as well?