huskies: merge 1213 story Chat "stop" command that immediately aborts the in-flight LLM turn
This commit is contained in:
@@ -3,7 +3,7 @@
|
||||
|
||||
use crate::chat::ChatTransport;
|
||||
use crate::chat::util::drain_complete_paragraphs;
|
||||
use crate::llm::providers::claude_code::{ClaudeCodeProvider, ClaudeCodeResult};
|
||||
use crate::llm::providers::claude_code::{CANCELLED, ClaudeCodeProvider, ClaudeCodeResult};
|
||||
use crate::slog;
|
||||
use matrix_sdk::ruma::OwnedRoomId;
|
||||
use std::sync::Arc;
|
||||
@@ -184,6 +184,7 @@ pub(in crate::chat::transport::matrix::bot) async fn handle_message(
|
||||
let remaining = buffer.lock().unwrap().trim().to_string();
|
||||
let did_send_any = sent_any_chunk.load(Ordering::Relaxed);
|
||||
|
||||
let mut was_cancelled = false;
|
||||
let (assistant_reply, new_session_id, turn_usage) = match result {
|
||||
Ok(ClaudeCodeResult {
|
||||
messages,
|
||||
@@ -213,6 +214,15 @@ pub(in crate::chat::transport::matrix::bot) async fn handle_message(
|
||||
slog!("[matrix-bot] session_id from chat_stream: {:?}", session_id);
|
||||
(reply, session_id, usage)
|
||||
}
|
||||
Err(e) if e == CANCELLED => {
|
||||
// A user-initiated "stop" — not a crash. The stop handler already
|
||||
// sent a confirmation, so don't post a second message here, and
|
||||
// don't run the crash-recovery path below (which would otherwise
|
||||
// clear the room's session_id as if the session were poisoned).
|
||||
slog!("[matrix-bot] LLM turn cancelled via stop for room {room_id}");
|
||||
was_cancelled = true;
|
||||
(String::new(), None, None)
|
||||
}
|
||||
Err(e) => {
|
||||
slog!("[matrix-bot] LLM error: {e}");
|
||||
let err_msg = if let Some(url) = crate::llm::oauth::extract_login_url_from_error(&e) {
|
||||
@@ -230,6 +240,10 @@ pub(in crate::chat::transport::matrix::bot) async fn handle_message(
|
||||
drop(msg_tx);
|
||||
let _ = post_task.await;
|
||||
|
||||
if was_cancelled {
|
||||
return;
|
||||
}
|
||||
|
||||
// Record this exchange in the per-room conversation history and persist
|
||||
// the session ID so the next turn resumes with structured API messages.
|
||||
let mut compact_suggestion: Option<String> = None;
|
||||
|
||||
@@ -295,6 +295,47 @@ async fn try_handle_compact_command(
|
||||
true
|
||||
}
|
||||
|
||||
/// Attempt to handle an addressed message as a bare `stop`/`halt`/`abort`
|
||||
/// command.
|
||||
///
|
||||
/// Returns `true` when `user_message` was recognised as a stop command and
|
||||
/// handled: the in-flight turn (if any) was cancelled via
|
||||
/// [`crate::chat::dispatcher::ChatDispatcher::stop`] and a confirmation was
|
||||
/// sent via `ctx.transport`. Returns `false` so the caller falls through to
|
||||
/// the dispatcher submit path — this must run before `chat_dispatcher.submit`
|
||||
/// so a bare "stop" is never itself queued as a prompt.
|
||||
async fn try_handle_stop_command(
|
||||
ctx: &BotContext,
|
||||
sender: &str,
|
||||
user_message: &str,
|
||||
room_id_str: &str,
|
||||
) -> bool {
|
||||
if super::super::super::stop::extract_stop_command(
|
||||
user_message,
|
||||
&ctx.services.bot_name,
|
||||
ctx.matrix_user_id.as_str(),
|
||||
)
|
||||
.is_none()
|
||||
{
|
||||
return false;
|
||||
}
|
||||
slog!("[matrix-bot] stop command from {sender} for session {room_id_str}");
|
||||
let response = match ctx.services.chat_dispatcher.stop(room_id_str) {
|
||||
crate::chat::dispatcher::StopOutcome::Cancelled => "Stopped.",
|
||||
crate::chat::dispatcher::StopOutcome::NothingRunning => "Nothing is currently running.",
|
||||
};
|
||||
let html = markdown_to_html(response);
|
||||
if let Ok(msg_id) = ctx
|
||||
.transport
|
||||
.send_message(room_id_str, response, &html)
|
||||
.await
|
||||
&& let Ok(event_id) = msg_id.parse()
|
||||
{
|
||||
ctx.bot_sent_event_ids.lock().await.insert(event_id);
|
||||
}
|
||||
true
|
||||
}
|
||||
|
||||
pub(in crate::chat::transport::matrix::bot) async fn on_room_message(
|
||||
ev: OriginalSyncRoomMessageEvent,
|
||||
room: Room,
|
||||
@@ -490,6 +531,13 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message(
|
||||
// entry silently swallows the command (bug: compact no-op in
|
||||
// gateway mode, distinct from the 1192/1205 registry-ordering bug).
|
||||
"compact",
|
||||
// `stop` is a gateway-local session command (sibling of `compact`):
|
||||
// it cancels THIS gateway's in-flight LLM turn via the local
|
||||
// `chat_dispatcher`. It must be listed here for the same reason
|
||||
// as `compact` above — once registered in `commands()` (so `help`
|
||||
// lists it), the gateway proxy would otherwise forward it to the
|
||||
// active project's sled instead of executing it locally.
|
||||
"stop",
|
||||
"switch",
|
||||
"all_status",
|
||||
"new",
|
||||
@@ -1403,27 +1451,10 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message(
|
||||
return;
|
||||
}
|
||||
|
||||
// "stop" — cancel the running LLM turn for this session and clear pending queue.
|
||||
{
|
||||
let stripped = crate::chat::util::strip_bot_mention(
|
||||
&user_message,
|
||||
&ctx.services.bot_name,
|
||||
ctx.matrix_user_id.as_str(),
|
||||
)
|
||||
.trim()
|
||||
.to_ascii_lowercase();
|
||||
if stripped == "stop" {
|
||||
slog!("[matrix-bot] stop command from {sender} for session {room_id_str}");
|
||||
ctx.services.chat_dispatcher.stop(&room_id_str);
|
||||
let msg = "Stopped.";
|
||||
let html = markdown_to_html(msg);
|
||||
if let Ok(msg_id) = ctx.transport.send_message(&room_id_str, msg, &html).await
|
||||
&& let Ok(event_id) = msg_id.parse()
|
||||
{
|
||||
ctx.bot_sent_event_ids.lock().await.insert(event_id);
|
||||
}
|
||||
return;
|
||||
}
|
||||
// "stop"/"halt"/"abort" — cancel the running LLM turn for this session
|
||||
// and clear its pending queue.
|
||||
if try_handle_stop_command(&ctx, &sender, &user_message, &room_id_str).await {
|
||||
return;
|
||||
}
|
||||
|
||||
// Hand the message to the protocol-agnostic dispatcher instead of spawning
|
||||
@@ -1465,7 +1496,7 @@ pub(in crate::chat::transport::matrix::bot) async fn on_room_message(
|
||||
mod tests {
|
||||
use super::{
|
||||
eval_gateway_overview_command, eval_gateway_status_command, eval_switch_command,
|
||||
try_handle_compact_command,
|
||||
try_handle_compact_command, try_handle_stop_command,
|
||||
};
|
||||
use crate::chat::{ChatTransport, MessageId};
|
||||
use crate::service::gateway::config::ProjectEntry;
|
||||
@@ -1987,4 +2018,97 @@ mod tests {
|
||||
"no reply should be sent for a message that isn't compact"
|
||||
);
|
||||
}
|
||||
|
||||
// ── stop (story 1213) ──────────────────────────────────────────────
|
||||
|
||||
/// AC1: a bare "stop" cancels an active run and replies "Stopped.".
|
||||
#[tokio::test]
|
||||
async fn stop_command_cancels_active_run_and_confirms() {
|
||||
use std::sync::Arc;
|
||||
|
||||
let room_id: matrix_sdk::ruma::OwnedRoomId = "!test:example.com".parse().unwrap();
|
||||
let project_root_dir = tempfile::tempdir().unwrap();
|
||||
let services = crate::services::Services::new_test(
|
||||
project_root_dir.path().to_path_buf(),
|
||||
"Huskies".to_string(),
|
||||
);
|
||||
let transport = Arc::new(CapturingTransport::new());
|
||||
let ctx = make_test_ctx(services, transport.clone());
|
||||
|
||||
// Start a long-running "turn" on the dispatcher for this room.
|
||||
let factory: crate::chat::dispatcher::SpawnFn = Arc::new(|_prompt, _cancel_rx| {
|
||||
Box::pin(async move {
|
||||
tokio::time::sleep(std::time::Duration::from_millis(500)).await;
|
||||
})
|
||||
});
|
||||
ctx.services.chat_dispatcher.submit(
|
||||
room_id.to_string(),
|
||||
"long-running".to_string(),
|
||||
factory,
|
||||
);
|
||||
// Let the coalesce window fire so the run is actually active.
|
||||
tokio::time::sleep(std::time::Duration::from_millis(50)).await;
|
||||
|
||||
let handled =
|
||||
try_handle_stop_command(&ctx, "@alice:example.com", "stop", room_id.as_str()).await;
|
||||
|
||||
assert!(handled, "bare stop must be recognized and handled");
|
||||
let sent = transport.sent.lock().unwrap().clone();
|
||||
assert_eq!(sent.len(), 1, "exactly one reply must be sent");
|
||||
assert_eq!(sent[0].1, "Stopped.");
|
||||
}
|
||||
|
||||
/// AC6: stop with nothing running replies that nothing is running,
|
||||
/// instead of silently no-opping.
|
||||
#[tokio::test]
|
||||
async fn stop_command_with_nothing_running_reports_nothing_running() {
|
||||
use std::sync::Arc;
|
||||
|
||||
let room_id: matrix_sdk::ruma::OwnedRoomId = "!test:example.com".parse().unwrap();
|
||||
let project_root_dir = tempfile::tempdir().unwrap();
|
||||
let services = crate::services::Services::new_test(
|
||||
project_root_dir.path().to_path_buf(),
|
||||
"Huskies".to_string(),
|
||||
);
|
||||
let transport = Arc::new(CapturingTransport::new());
|
||||
let ctx = make_test_ctx(services, transport.clone());
|
||||
|
||||
let handled =
|
||||
try_handle_stop_command(&ctx, "@alice:example.com", "stop", room_id.as_str()).await;
|
||||
|
||||
assert!(handled, "bare stop must be recognized and handled");
|
||||
let sent = transport.sent.lock().unwrap().clone();
|
||||
assert_eq!(sent.len(), 1, "exactly one reply must be sent");
|
||||
assert_eq!(sent[0].1, "Nothing is currently running.");
|
||||
}
|
||||
|
||||
/// AC4: a non-bare message that merely contains the word "stop" must
|
||||
/// fall through untouched, never treated as an abort.
|
||||
#[tokio::test]
|
||||
async fn stop_with_trailing_text_falls_through() {
|
||||
use std::sync::Arc;
|
||||
|
||||
let room_id: matrix_sdk::ruma::OwnedRoomId = "!test:example.com".parse().unwrap();
|
||||
let project_root_dir = tempfile::tempdir().unwrap();
|
||||
let services = crate::services::Services::new_test(
|
||||
project_root_dir.path().to_path_buf(),
|
||||
"Huskies".to_string(),
|
||||
);
|
||||
let transport = Arc::new(CapturingTransport::new());
|
||||
let ctx = make_test_ctx(services, transport.clone());
|
||||
|
||||
let handled = try_handle_stop_command(
|
||||
&ctx,
|
||||
"@alice:example.com",
|
||||
"stop the deployment and redeploy",
|
||||
room_id.as_str(),
|
||||
)
|
||||
.await;
|
||||
|
||||
assert!(!handled, "non-bare 'stop' text must fall through");
|
||||
assert!(
|
||||
transport.sent.lock().unwrap().is_empty(),
|
||||
"no reply should be sent for a message that isn't a bare stop"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user