fix(goal): start a turn when /goal or /grind is set (#9801)
Signed-off-by: Michael Neale <michael.neale@gmail.com>
This commit is contained in:
@@ -1516,6 +1516,8 @@ impl Agent {
|
||||
.execute_command(&message_text, &session_config.id)
|
||||
.await;
|
||||
|
||||
let mut command_preamble: Vec<AgentEvent> = Vec::new();
|
||||
|
||||
match command_result {
|
||||
Err(e) => {
|
||||
let error_message = Message::assistant()
|
||||
@@ -1525,6 +1527,44 @@ impl Agent {
|
||||
Ok(AgentEvent::Message(error_message))
|
||||
})));
|
||||
}
|
||||
Ok(Some(response))
|
||||
if response.role == rmcp::model::Role::Assistant
|
||||
&& crate::agents::execute_commands::command_starts_turn(&message_text) =>
|
||||
{
|
||||
// Setting a goal/grind should immediately start a turn so the
|
||||
// agent begins pursuing it, rather than waiting for the next
|
||||
// user prompt. Record the command and its confirmation as
|
||||
// user-visible only, then inject an agent-visible kickoff and
|
||||
// fall through into the reply loop.
|
||||
session_manager
|
||||
.add_message(
|
||||
&session_config.id,
|
||||
&user_message.clone().with_visibility(true, false),
|
||||
)
|
||||
.await?;
|
||||
session_manager
|
||||
.add_message(
|
||||
&session_config.id,
|
||||
&response.clone().with_visibility(true, false),
|
||||
)
|
||||
.await?;
|
||||
let goal_text = crate::agents::execute_commands::parse_slash_command(&message_text)
|
||||
.map(|parsed| parsed.params_str.to_string())
|
||||
.unwrap_or_default();
|
||||
let kickoff = Message::user()
|
||||
.with_text(format!(
|
||||
"Start working toward this goal now:\n\n**Goal:** {goal_text}"
|
||||
))
|
||||
.with_visibility(false, true);
|
||||
session_manager
|
||||
.add_message(&session_config.id, &kickoff)
|
||||
.await?;
|
||||
|
||||
command_preamble = vec![
|
||||
AgentEvent::Message(user_message.clone()),
|
||||
AgentEvent::Message(response.clone()),
|
||||
];
|
||||
}
|
||||
Ok(Some(response)) if response.role == rmcp::model::Role::Assistant => {
|
||||
session_manager
|
||||
.add_message(
|
||||
@@ -1599,6 +1639,10 @@ impl Agent {
|
||||
let conversation_to_compact = conversation.clone();
|
||||
|
||||
Ok(Box::pin(async_stream::try_stream! {
|
||||
for event in command_preamble {
|
||||
yield event;
|
||||
}
|
||||
|
||||
let final_conversation = if !needs_auto_compact {
|
||||
conversation
|
||||
} else {
|
||||
|
||||
@@ -84,6 +84,22 @@ pub fn list_commands() -> &'static [CommandDef] {
|
||||
COMMANDS
|
||||
}
|
||||
|
||||
fn is_clear_goal_param(params_str: &str) -> bool {
|
||||
matches!(params_str, "off" | "clear" | "none")
|
||||
}
|
||||
|
||||
/// Whether a slash command should kick off an agent turn instead of just
|
||||
/// returning a confirmation. Setting a `/goal` or `/grind` (with a description,
|
||||
/// not the query or `off` forms) makes the agent start pursuing it immediately.
|
||||
pub fn command_starts_turn(message_text: &str) -> bool {
|
||||
let Some(parsed) = parse_slash_command(message_text) else {
|
||||
return false;
|
||||
};
|
||||
matches!(parsed.command, "goal" | "grind")
|
||||
&& !parsed.params_str.is_empty()
|
||||
&& !is_clear_goal_param(parsed.params_str)
|
||||
}
|
||||
|
||||
impl Agent {
|
||||
pub async fn execute_command(
|
||||
&self,
|
||||
@@ -380,7 +396,7 @@ impl Agent {
|
||||
return Ok(Some(Message::assistant().with_text(text)));
|
||||
}
|
||||
|
||||
if params_str == "off" || params_str == "clear" || params_str == "none" {
|
||||
if is_clear_goal_param(params_str) {
|
||||
self.set_goal(None).await;
|
||||
return Ok(Some(
|
||||
Message::assistant().with_text("Goal cleared. The agent will finish normally."),
|
||||
@@ -404,7 +420,7 @@ impl Agent {
|
||||
return Ok(Some(Message::assistant().with_text(text)));
|
||||
}
|
||||
|
||||
if params_str == "off" || params_str == "clear" {
|
||||
if is_clear_goal_param(params_str) {
|
||||
self.set_grind(None).await;
|
||||
return Ok(Some(
|
||||
Message::assistant().with_text("Grind cleared. The agent will finish normally."),
|
||||
@@ -447,6 +463,24 @@ mod tests {
|
||||
assert_eq!(parsed.params_str, "");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn command_starts_turn_only_for_goal_and_grind_with_description() {
|
||||
assert!(command_starts_turn("/goal make all tests pass"));
|
||||
assert!(command_starts_turn("/grind keep refactoring"));
|
||||
|
||||
// Query and clear forms must not start a turn.
|
||||
assert!(!command_starts_turn("/goal"));
|
||||
assert!(!command_starts_turn("/goal off"));
|
||||
assert!(!command_starts_turn("/goal clear"));
|
||||
assert!(!command_starts_turn("/goal none"));
|
||||
assert!(!command_starts_turn("/grind"));
|
||||
assert!(!command_starts_turn("/grind off"));
|
||||
|
||||
// Other commands and plain prompts never start a turn here.
|
||||
assert!(!command_starts_turn("/compact"));
|
||||
assert!(!command_starts_turn("just a normal message"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn user_only_assistant_text_is_durable_text_not_system_notification() {
|
||||
let message = user_only_assistant_text("Conversation cleared");
|
||||
|
||||
@@ -1387,6 +1387,105 @@ mod tests {
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_setting_goal_via_reply_starts_a_turn() -> Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
let session_manager = Arc::new(SessionManager::new(temp_dir.path().to_path_buf()));
|
||||
let agent = create_agent_with_session_naming_disabled(session_manager.clone());
|
||||
let provider = Arc::new(GoalTextProvider::new());
|
||||
|
||||
let session = session_manager
|
||||
.create_session(
|
||||
PathBuf::default(),
|
||||
"goal-start-turn".to_string(),
|
||||
SessionType::Hidden,
|
||||
GooseMode::default(),
|
||||
)
|
||||
.await?;
|
||||
agent.update_provider(provider.clone(), &session.id).await?;
|
||||
|
||||
let session_config = SessionConfig {
|
||||
id: session.id.clone(),
|
||||
schedule_id: None,
|
||||
max_turns: Some(10),
|
||||
retry_config: None,
|
||||
};
|
||||
|
||||
let reply_stream = agent
|
||||
.reply(
|
||||
Message::user().with_text("/goal make all tests pass"),
|
||||
session_config,
|
||||
None,
|
||||
)
|
||||
.await?;
|
||||
tokio::pin!(reply_stream);
|
||||
|
||||
let mut messages = Vec::new();
|
||||
while let Some(event) = reply_stream.next().await {
|
||||
if let Ok(AgentEvent::Message(msg)) = event {
|
||||
messages.push(msg);
|
||||
}
|
||||
}
|
||||
|
||||
// The provider must be invoked: setting a goal kicks off a turn
|
||||
// (the goal-checking loop then runs and clears the goal once met).
|
||||
assert!(
|
||||
provider.call_count.load(Ordering::SeqCst) >= 1,
|
||||
"Setting a goal should start an agent turn"
|
||||
);
|
||||
|
||||
// The user still sees the confirmation.
|
||||
assert!(
|
||||
messages
|
||||
.iter()
|
||||
.any(|m| m.as_concat_text().contains("Goal set")),
|
||||
"Goal confirmation should be surfaced to the user"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn test_querying_goal_via_reply_does_not_start_a_turn() -> Result<()> {
|
||||
let temp_dir = TempDir::new()?;
|
||||
let session_manager = Arc::new(SessionManager::new(temp_dir.path().to_path_buf()));
|
||||
let agent = create_agent_with_session_naming_disabled(session_manager.clone());
|
||||
let provider = Arc::new(GoalTextProvider::new());
|
||||
|
||||
let session = session_manager
|
||||
.create_session(
|
||||
PathBuf::default(),
|
||||
"goal-query-no-turn".to_string(),
|
||||
SessionType::Hidden,
|
||||
GooseMode::default(),
|
||||
)
|
||||
.await?;
|
||||
agent.update_provider(provider.clone(), &session.id).await?;
|
||||
|
||||
let session_config = SessionConfig {
|
||||
id: session.id.clone(),
|
||||
schedule_id: None,
|
||||
max_turns: Some(10),
|
||||
retry_config: None,
|
||||
};
|
||||
|
||||
let reply_stream = agent
|
||||
.reply(Message::user().with_text("/goal"), session_config, None)
|
||||
.await?;
|
||||
tokio::pin!(reply_stream);
|
||||
while let Some(event) = reply_stream.next().await {
|
||||
let _ = event?;
|
||||
}
|
||||
|
||||
assert_eq!(
|
||||
provider.call_count.load(Ordering::SeqCst),
|
||||
0,
|
||||
"Querying the goal should not start an agent turn"
|
||||
);
|
||||
|
||||
Ok(())
|
||||
}
|
||||
}
|
||||
|
||||
mod cumulative_token_tests {
|
||||
|
||||
Reference in New Issue
Block a user