Skip to content

feat: SessionManager integration for acp sessions #5657

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.

Sign up for GitHub

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

Merged
alexhancock merged 1 commit into from
Nov 11, 2025
Merged

feat: SessionManager integration for acp sessions #5657

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
45 changes: 12 additions & 33 deletions crates/goose-cli/src/commands/acp.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -605,8 +605,12 @@ impl acp::Agent for GooseAcpAgent {
) -> Result<acp::NewSessionResponse, acp::Error> {
info!("ACP: Received new session request {:?}", args);

// Generate a unique session ID
let session_id = uuid::Uuid::new_v4().to_string();
let goose_session = SessionManager::create_session(
std::env::current_dir().unwrap_or_default(),
Comment thread
alexhancock marked this conversation as resolved.
"ACP Session".to_string(), // just an initial name - may be replaced by maybe_update_name
Copy link
Copy Markdown
Collaborator

@DOsinga DOsinga Nov 10, 2025

Choose a reason for hiding this comment

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

hmmm?

Copy link
Copy Markdown
Collaborator

@michaelneale michaelneale Nov 11, 2025

Choose a reason for hiding this comment

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

would goose automatically update this after a few turns (via the LLM session naming)? If so, can probably leave it out

Copy link
Copy Markdown
Collaborator

@codefromthecrypt codefromthecrypt Nov 11, 2025

Choose a reason for hiding this comment

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

I'm not sure it is really helpful to intentionally give no useful initial name, right? there's a race condition and the first inference request will have a worse first name right? If this is an issue of it being too verbose to set the initial name, maybe we can find a better way to rejig it so it isn't so in your face?

Copy link
Copy Markdown
Collaborator

@DOsinga DOsinga Nov 11, 2025

Choose a reason for hiding this comment

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

goose should name this automatically yeah. but also depending on what we decide on visibility here, people won't be seeing this name I think (unless I miss something in the ACP protocol)

SessionType::User,
)
.await?;

let session = GooseAcpSession {
messages: Conversation::new_unvalidated(Vec::new()),
Expand All @@ -615,15 +619,14 @@ impl acp::Agent for GooseAcpAgent {
cancel_token: None,
};

// Store the session
let mut sessions = self.sessions.lock().await;
sessions.insert(session_id.clone(), session);
sessions.insert(goose_session.id.clone(), session);

info!("Created new session with ID: {}", session_id);
info!("Created new ACP/goose session {}", goose_session.id);

Ok(acp::NewSessionResponse {
session_id: acp::SessionId(session_id.into()),
modes: None, // TODO: Implement session modes if needed
session_id: acp::SessionId(goose_session.id.into()),
modes: None,
meta: None,
})
}
Expand All @@ -649,15 +652,9 @@ impl acp::Agent for GooseAcpAgent {
}

async fn prompt(&self, args: acp::PromptRequest) -> Result<acp::PromptResponse, acp::Error> {
info!("ACP: Received prompt request {:?}", args);

// Get the session
let session_id = args.session_id.0.to_string();

// Create and store cancellation token for this prompt
let cancel_token = CancellationToken::new();

// Store the cancel token in the session BEFORE starting the prompt
{
let mut sessions = self.sessions.lock().await;
let session = sessions
Expand All @@ -668,21 +665,13 @@ impl acp::Agent for GooseAcpAgent {

let user_message = self.convert_acp_prompt_to_message(args.prompt);

let session = SessionManager::create_session(
std::env::current_dir().unwrap_or_default(),
"ACP Session".to_string(),
SessionType::Hidden,
)
.await?;

let session_config = SessionConfig {
id: session.id.clone(),
id: session_id.clone(),
schedule_id: None,
max_turns: None,
retry_config: None,
};

// Get agent's reply through the Goose agent
let mut stream = self
.agent
.reply(user_message, session_config, Some(cancel_token.clone()))
Expand All @@ -694,45 +683,36 @@ impl acp::Agent for GooseAcpAgent {

use futures::StreamExt;

// Track if we were cancelled
let mut was_cancelled = false;

// Process the agent's response stream
while let Some(event) = stream.next().await {
// Check if we've been cancelled
if cancel_token.is_cancelled() {
was_cancelled = true;
break;
}

match event {
Ok(goose::agents::AgentEvent::Message(message)) => {
// Re-acquire the lock to add message to conversation
let mut sessions = self.sessions.lock().await;
let session = sessions
.get_mut(&session_id)
.ok_or_else(acp::Error::invalid_params)?;

// Add to conversation
session.messages.push(message.clone());

// Process message content, including tool calls
for content_item in &message.content {
self.handle_message_content(content_item, &args.session_id, session)
.await?;
}
}
Ok(_) => {
// Ignore other events for now
}
Ok(_) => {}
Err(e) => {
error!("Error in agent response stream: {}", e);
return Err(acp::Error::internal_error());
}
}
}

// Clear the cancel token since we're done
let mut sessions = self.sessions.lock().await;
if let Some(session) = sessions.get_mut(&session_id) {
session.cancel_token = None;
Expand All @@ -751,7 +731,6 @@ impl acp::Agent for GooseAcpAgent {
async fn cancel(&self, args: acp::CancelNotification) -> Result<(), acp::Error> {
info!("ACP: Received cancel request {:?}", args);

// Get the session and cancel its active operation
let session_id = args.session_id.0.to_string();
let mut sessions = self.sessions.lock().await;

Expand Down