Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions crates/goose-cli/src/session/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -210,6 +210,11 @@ pub async fn build_session(session_config: SessionBuilderConfig) -> Session {
// Create the agent
let agent: Agent = Agent::new();

// Reset session state if we're not resuming
if !session_config.resume {
agent.reset_session_state().await;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm confused. We just created the agent on 211, how would we have any state to reset at this point?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tbh I am confused about the whole TODO tool. I dont see where it loads the existing todo if the session is resumed

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@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?

}

if let Some(sub_recipes) = session_config.sub_recipes {
agent.add_sub_recipes(sub_recipes).await;
}
Expand Down
77 changes: 67 additions & 10 deletions crates/goose/src/agents/agent.rs
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,33 @@ impl Agent {
*tool_monitor = Some(ToolMonitor::new(max_repetitions));
}

pub async fn clear_todo_list(&self) {
let mut todo_list = self.todo_list.lock().await;
todo_list.clear();
}

/// Reset all session-specific state while preserving configuration
/// This should be called between sessions to ensure a clean slate
pub async fn reset_session_state(&self) {
self.clear_todo_list().await;

self.reset_retry_attempts().await;

let mut tool_monitor = self.tool_monitor.lock().await;
*tool_monitor = None;

let mut final_output = self.final_output_tool.lock().await;
*final_output = None;

let mut sub_recipe_manager = self.sub_recipe_manager.lock().await;
sub_recipe_manager.clear();

self.tasks_manager.reset().await;

let mut prompt_manager = self.prompt_manager.lock().await;
prompt_manager.clear_session_extras();
}

/// Reset the retry attempts counter to 0
pub async fn reset_retry_attempts(&self) {
self.retry_manager.reset_attempts().await;
Expand Down Expand Up @@ -1519,21 +1546,51 @@ mod tests {
}

#[tokio::test]
async fn test_todo_tools_integration() -> Result<()> {
async fn test_reset_session_state() -> Result<()> {
let agent = Agent::new();

// Test that task planner tools are listed
let tools = agent.list_tools(None).await;
// Add some state to the agent
let response = Response {
json_schema: Some(serde_json::json!({
"type": "object",
"properties": {
"result": {"type": "string"}
}
})),
};
agent.add_final_output_tool(response).await;

// Write to the TODO list
let write_call = mcp_core::tool::ToolCall {
name: TODO_WRITE_TOOL_NAME.to_string(),
arguments: serde_json::json!({
"content": "Test todo content"
}),
};
let (_id, _result) = agent
.dispatch_tool_call(write_call, "test-write".to_string(), None)
.await;

let todo_read = tools.iter().find(|tool| tool.name == TODO_READ_TOOL_NAME);
let todo_write = tools.iter().find(|tool| tool.name == TODO_WRITE_TOOL_NAME);
// Verify state exists
let todo_content = agent.todo_list.lock().await.clone();
assert_eq!(todo_content, "Test todo content");
assert!(agent.final_output_tool.lock().await.is_some());

assert!(todo_read.is_some(), "TODO read tool should be present");
assert!(todo_write.is_some(), "TODO write tool should be present");
// Reset session state
agent.reset_session_state().await;

// Test todo_list initialization
let todo_content = agent.todo_list.lock().await;
assert_eq!(*todo_content, "", "TODO list should be initially empty");
// Verify state is cleared
let todo_content_after = agent.todo_list.lock().await.clone();
assert_eq!(todo_content_after, "", "TODO list should be cleared");
assert!(
agent.final_output_tool.lock().await.is_none(),
"Final output tool should be cleared"
);
assert_eq!(
agent.get_retry_attempts().await,
0,
"Retry attempts should be reset"
);

Ok(())
}
Expand Down
5 changes: 5 additions & 0 deletions crates/goose/src/agents/prompt_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,11 @@ impl PromptManager {
self.system_prompt_extras.push(instruction);
}

/// Clear session-specific prompt extras (preserves override)
pub fn clear_session_extras(&mut self) {
self.system_prompt_extras.clear();
}

/// Override the system prompt with custom text
pub fn set_system_prompt_override(&mut self, template: String) {
self.system_prompt_override = Some(template);
Expand Down
6 changes: 6 additions & 0 deletions crates/goose/src/agents/sub_recipe_manager.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,12 @@ impl SubRecipeManager {
}
}

/// Clear all sub-recipes and their associated tools
pub fn clear(&mut self) {
self.sub_recipe_tools.clear();
self.sub_recipes.clear();
}

pub fn is_sub_recipe_tool(&self, tool_name: &str) -> bool {
self.sub_recipe_tools.contains_key(tool_name)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,12 @@ impl TasksManager {
}
Ok(tasks)
}

/// Reset the tasks manager, clearing all stored tasks
pub async fn reset(&self) {
let mut task_map = self.tasks.write().await;
task_map.clear();
}
}

#[cfg(test)]
Expand Down
Loading