From cadae27bb009812248096af3597cb9521696bd19 Mon Sep 17 00:00:00 2001 From: Alex Hancock Date: Tue, 5 May 2026 14:47:25 -0400 Subject: [PATCH] fix: 8531 - elicitation fixes (#8999) Signed-off-by: Alex Hancock --- crates/goose-cli/src/session/elicitation.rs | 22 +++++++++-- .../goose-server/src/routes/session_events.rs | 35 ++++++++++++++++-- crates/goose/src/agents/agent.rs | 21 ++++++----- .../src/components/ElicitationRequest.tsx | 37 ++++++++++++++++--- ui/desktop/src/components/GooseMessage.tsx | 7 +++- .../src/components/ProgressiveMessageList.tsx | 7 +++- .../src/components/ui/JsonSchemaForm.tsx | 3 ++ ui/desktop/src/hooks/useChatStream.ts | 23 +++++++++--- ui/desktop/src/i18n/messages/en.json | 3 ++ 9 files changed, 128 insertions(+), 30 deletions(-) diff --git a/crates/goose-cli/src/session/elicitation.rs b/crates/goose-cli/src/session/elicitation.rs index ad4b9fcb..ef4930d7 100644 --- a/crates/goose-cli/src/session/elicitation.rs +++ b/crates/goose-cli/src/session/elicitation.rs @@ -11,9 +11,25 @@ pub fn collect_elicitation_input( println!("\n{}", style(message).cyan()); } - let properties = match schema.get("properties").and_then(|p| p.as_object()) { - Some(props) => props, - None => return Ok(Some(HashMap::new())), + let properties = schema.get("properties").and_then(|p| p.as_object()); + + // Schema-less (or empty-schema) elicitations are pure approval prompts — + // offer an explicit Y/N confirmation instead of silently auto-accepting. + let properties = match properties { + Some(props) if !props.is_empty() => props, + _ => { + let prompt = if message.is_empty() { + "Approve this action?" + } else { + "Approve?" + }; + return match cliclack::confirm(prompt).initial_value(true).interact() { + Ok(true) => Ok(Some(HashMap::new())), + Ok(false) => Ok(None), + Err(e) if e.kind() == io::ErrorKind::Interrupted => Ok(None), + Err(e) => Err(e), + }; + } }; let required: Vec<&str> = schema diff --git a/crates/goose-server/src/routes/session_events.rs b/crates/goose-server/src/routes/session_events.rs index a0492642..dc5d3d54 100644 --- a/crates/goose-server/src/routes/session_events.rs +++ b/crates/goose-server/src/routes/session_events.rs @@ -313,6 +313,38 @@ pub async fn session_reply( } } + let user_message = request.user_message; + let override_conversation = request.override_conversation; + + // An elicitation response unblocks an in-flight tool call that is already + // streaming on another request_id — don't register a new active request or + // open a new SSE stream; route it to the agent's short-circuit path. + let is_elicitation_response = user_message.content.iter().any(|c| { + matches!( + c, + goose::conversation::message::MessageContent::ActionRequired(ar) + if matches!( + ar.data, + goose::conversation::message::ActionRequiredData::ElicitationResponse { .. } + ) + ) + }); + + if is_elicitation_response { + let agent = state.get_agent_for_route(session_id.clone()).await?; + let session_config = goose::agents::types::SessionConfig { + id: session_id.clone(), + schedule_id: session_data.schedule_id.clone(), + max_turns: None, + retry_config: None, + }; + let _ = agent + .reply(user_message, session_config, None) + .await + .map_err(|e| ErrorResponse::internal(e.to_string()))?; + return Ok(Json(SessionReplyResponse { request_id })); + } + let bus = state.get_or_create_event_bus(&session_id).await; let cancel_token = bus @@ -322,9 +354,6 @@ pub async fn session_reply( ErrorResponse::bad_request("Session already has an active request. Cancel it first.") })?; - let user_message = request.user_message; - let override_conversation = request.override_conversation; - let task_state = state.clone(); let task_session_id = session_id.clone(); let task_request_id = request_id.clone(); diff --git a/crates/goose/src/agents/agent.rs b/crates/goose/src/agents/agent.rs index ccb1fff3..9a435953 100644 --- a/crates/goose/src/agents/agent.rs +++ b/crates/goose/src/agents/agent.rs @@ -1042,18 +1042,19 @@ impl Agent { if let ActionRequiredData::ElicitationResponse { id, user_data } = &action_required.data { - if let Err(e) = ActionRequiredManager::global() + // Surface stale/cancelled/timed-out elicitations as a hard + // error so callers (e.g. the HTTP handler) can propagate + // failure to the client instead of silently reporting + // success while the blocked tool call stays unblocked. + // The success path returns an empty stream; an Err here + // makes the contract: Ok(empty) on accept, Err on reject. + ActionRequiredManager::global() .submit_response(id.clone(), user_data.clone()) .await - { - let error_text = format!("Failed to submit elicitation response: {}", e); - error!(error_text); - return Ok(Box::pin(stream::once(async { - Ok(AgentEvent::Message( - Message::assistant().with_text(error_text), - )) - }))); - } + .map_err(|e| { + error!("Failed to submit elicitation response: {}", e); + anyhow!("Failed to submit elicitation response: {}", e) + })?; session_manager .add_message(&session_config.id, &user_message) .await?; diff --git a/ui/desktop/src/components/ElicitationRequest.tsx b/ui/desktop/src/components/ElicitationRequest.tsx index 195e6679..4eefffd9 100644 --- a/ui/desktop/src/components/ElicitationRequest.tsx +++ b/ui/desktop/src/components/ElicitationRequest.tsx @@ -1,6 +1,7 @@ import { useState, useEffect, useRef } from 'react'; import { ActionRequired } from '../api'; import { defineMessages, useIntl } from '../i18n'; +import { Button } from './ui/button'; import JsonSchemaForm from './ui/JsonSchemaForm'; import type { JsonSchema } from './ui/JsonSchemaForm'; @@ -25,6 +26,10 @@ const i18n = defineMessages({ id: 'elicitationRequest.submit', defaultMessage: 'Submit', }, + accept: { + id: 'elicitationRequest.accept', + defaultMessage: 'Accept', + }, waitingForResponse: { id: 'elicitationRequest.waitingForResponse', defaultMessage: 'Waiting for your response ({timeRemaining} remaining)', @@ -79,11 +84,19 @@ export default function ElicitationRequest({ const { id: elicitationId, message, requested_schema } = actionRequiredContent.data; + const schema = (requested_schema ?? {}) as JsonSchema; + const hasSchemaFields = Boolean(schema.properties && Object.keys(schema.properties).length > 0); + const handleSubmit = (formData: Record) => { setSubmitted(true); onSubmit(elicitationId, formData); }; + const handleAccept = () => { + setSubmitted(true); + onSubmit(elicitationId, {}); + }; + if (isCancelledMessage) { return (
@@ -147,11 +160,19 @@ export default function ElicitationRequest({
- + {hasSchemaFields ? ( + + ) : ( +
+ +
+ )}
@@ -169,7 +190,11 @@ export default function ElicitationRequest({ d="M12 8v4l3 3m6-3a9 9 0 11-18 0 9 9 0 0118 0z" /> - {intl.formatMessage(i18n.waitingForResponse, { timeRemaining: formatTime(timeRemaining) })} + + {intl.formatMessage(i18n.waitingForResponse, { + timeRemaining: formatTime(timeRemaining), + })} +
diff --git a/ui/desktop/src/components/GooseMessage.tsx b/ui/desktop/src/components/GooseMessage.tsx index c609e92b..ee0a433a 100644 --- a/ui/desktop/src/components/GooseMessage.tsx +++ b/ui/desktop/src/components/GooseMessage.tsx @@ -117,7 +117,12 @@ export default function GooseMessage({ {thinkingContent && ( )} diff --git a/ui/desktop/src/components/ProgressiveMessageList.tsx b/ui/desktop/src/components/ProgressiveMessageList.tsx index ef904b6c..6a492efd 100644 --- a/ui/desktop/src/components/ProgressiveMessageList.tsx +++ b/ui/desktop/src/components/ProgressiveMessageList.tsx @@ -283,7 +283,12 @@ export default function ProgressiveMessageList({ {/* Loading indicator when progressively rendering */} {isLoading && (
- +
{intl.formatMessage(i18n.searchHint)}
diff --git a/ui/desktop/src/components/ui/JsonSchemaForm.tsx b/ui/desktop/src/components/ui/JsonSchemaForm.tsx index 0076d7ee..73889cfe 100644 --- a/ui/desktop/src/components/ui/JsonSchemaForm.tsx +++ b/ui/desktop/src/components/ui/JsonSchemaForm.tsx @@ -82,6 +82,7 @@ export default function JsonSchemaForm({ const [formData, setFormData] = useState>(() => { const initial: Record = {}; if (schema.properties) { + const isRequired = (key: string) => schema.required?.includes(key) ?? false; for (const [key, prop] of Object.entries(schema.properties)) { if (prop.default !== undefined) { initial[key] = prop.default; @@ -89,6 +90,8 @@ export default function JsonSchemaForm({ initial[key] = false; } else if (prop.type === 'number' || prop.type === 'integer') { initial[key] = prop.minimum ?? 0; + } else if (prop.enum && prop.enum.length > 0 && isRequired(key)) { + initial[key] = prop.enum[0]; } else { initial[key] = ''; } diff --git a/ui/desktop/src/hooks/useChatStream.ts b/ui/desktop/src/hooks/useChatStream.ts index 7d84b625..8895aff3 100644 --- a/ui/desktop/src/hooks/useChatStream.ts +++ b/ui/desktop/src/hooks/useChatStream.ts @@ -897,15 +897,26 @@ export function useChatStream({ return; } + // An elicitation response unblocks an in-flight tool call on the original + // request's SSE stream — don't start a new stream or flip chat state. const responseMessage = createElicitationResponseMessage(elicitationId, userData); - const currentMessages = [...currentState.messages, responseMessage]; + const nextMessages = [...currentState.messages, responseMessage]; + dispatch({ type: 'SET_MESSAGES', payload: nextMessages }); - dispatch({ type: 'SET_MESSAGES', payload: currentMessages }); - dispatch({ type: 'START_STREAMING' }); - - await submitToSession(sessionId, responseMessage, currentMessages); + try { + await sessionReply({ + path: { id: sessionId }, + body: { + request_id: uuidv7(), + user_message: responseMessage, + }, + throwOnError: true, + }); + } catch (error) { + onFinish('Submit error: ' + errorMessage(error)); + } }, - [sessionId, submitToSession] + [sessionId, onFinish] ); const setRecipeUserParams = useCallback( diff --git a/ui/desktop/src/i18n/messages/en.json b/ui/desktop/src/i18n/messages/en.json index 6c99f411..bc4378d8 100644 --- a/ui/desktop/src/i18n/messages/en.json +++ b/ui/desktop/src/i18n/messages/en.json @@ -899,6 +899,9 @@ "dirSwitcher.failedToUpdateWorkingDir": { "defaultMessage": "Failed to update working directory" }, + "elicitationRequest.accept": { + "defaultMessage": "Accept" + }, "elicitationRequest.cancelled": { "defaultMessage": "Information request was cancelled." },