diff --git a/server/src/matrix/bot.rs b/server/src/matrix/bot.rs index e71a61c..e441b47 100644 --- a/server/src/matrix/bot.rs +++ b/server/src/matrix/bot.rs @@ -817,7 +817,6 @@ async fn on_room_message( agents: &ctx.agents, ambient_rooms: &ctx.ambient_rooms, room_id: &room_id_str, - is_addressed, }; if let Some(response) = super::commands::try_handle_command(&dispatch, &user_message) { slog!("[matrix-bot] Handled bot command from {sender}"); diff --git a/server/src/matrix/commands/ambient.rs b/server/src/matrix/commands/ambient.rs index e1c854a..e73b977 100644 --- a/server/src/matrix/commands/ambient.rs +++ b/server/src/matrix/commands/ambient.rs @@ -5,12 +5,11 @@ use crate::matrix::config::save_ambient_rooms; /// Toggle ambient mode for this room. /// -/// Only acts when the message directly addressed the bot (`is_addressed=true`) -/// to prevent accidental toggling via ambient-mode traffic. +/// Works whether or not the message directly addressed the bot — the user can +/// say "timmy ambient on", "@timmy ambient on", or just "ambient on" in an +/// ambient-mode room. The command is specific enough (must be the first word +/// after any bot-mention prefix) that accidental triggering is very unlikely. pub(super) fn handle_ambient(ctx: &CommandContext) -> Option { - if !ctx.is_addressed { - return None; - } let enable = match ctx.args { "on" => true, "off" => false, @@ -50,8 +49,12 @@ mod tests { Arc::new(AgentPool::new_test(3000)) } + // Bug 352: ambient commands were being forwarded to LLM after refactors + // 328/330 because handle_ambient required is_addressed=true, but + // mentions_bot() only matches @-prefixed mentions, not bare bot names. + // "timmy ambient off" sets is_addressed=false even though it names the bot. #[test] - fn ambient_on_requires_addressed() { + fn ambient_on_works_when_unaddressed() { let ambient_rooms = test_ambient_rooms(); let room_id = "!myroom:example.com".to_string(); let agents = test_agents(); @@ -62,14 +65,42 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: false, // not addressed }; - let result = try_handle_command(&dispatch, "@timmy ambient on"); - // Should fall through to LLM when not addressed - assert!(result.is_none(), "ambient should not fire in non-addressed mode"); + // "timmy ambient on" — bot name mentioned but not @-prefixed, so + // is_addressed is false; strip_bot_mention still strips "timmy ". + let result = try_handle_command(&dispatch, "timmy ambient on"); + assert!(result.is_some(), "ambient on should fire even when is_addressed=false"); + assert!( + ambient_rooms.lock().unwrap().contains(&room_id), + "room should be in ambient_rooms after ambient on" + ); + } + + #[test] + fn ambient_off_works_bare_in_ambient_room() { + let ambient_rooms = test_ambient_rooms(); + let room_id = "!myroom:example.com".to_string(); + ambient_rooms.lock().unwrap().insert(room_id.clone()); + let agents = test_agents(); + let dispatch = CommandDispatch { + bot_name: "Timmy", + bot_user_id: "@timmy:homeserver.local", + project_root: std::path::Path::new("/tmp"), + agents: &agents, + ambient_rooms: &ambient_rooms, + room_id: &room_id, + }; + // Bare "ambient off" in an ambient room (is_addressed=false). + let result = try_handle_command(&dispatch, "ambient off"); + assert!(result.is_some(), "bare ambient off should be handled without LLM"); + let output = result.unwrap(); + assert!( + output.contains("Ambient mode off"), + "response should confirm ambient off: {output}" + ); assert!( !ambient_rooms.lock().unwrap().contains(&room_id), - "ambient_rooms should not be modified when not addressed" + "room should be removed from ambient_rooms after ambient off" ); } @@ -85,7 +116,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; let result = try_handle_command(&dispatch, "@timmy ambient on"); assert!(result.is_some(), "ambient on should produce a response"); @@ -115,7 +145,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; let result = try_handle_command(&dispatch, "@timmy ambient off"); assert!(result.is_some(), "ambient off should produce a response"); diff --git a/server/src/matrix/commands/cost.rs b/server/src/matrix/commands/cost.rs index 511c9cc..1eee04b 100644 --- a/server/src/matrix/commands/cost.rs +++ b/server/src/matrix/commands/cost.rs @@ -144,7 +144,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; try_handle_command(&dispatch, "@timmy cost") } diff --git a/server/src/matrix/commands/git.rs b/server/src/matrix/commands/git.rs index d757e5d..f8b6d53 100644 --- a/server/src/matrix/commands/git.rs +++ b/server/src/matrix/commands/git.rs @@ -121,7 +121,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; let result = try_handle_command(&dispatch, "@timmy git"); assert!(result.is_some(), "git command should always return Some"); @@ -142,7 +141,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; let output = try_handle_command(&dispatch, "@timmy git").unwrap(); assert!( @@ -166,7 +164,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; let output = try_handle_command(&dispatch, "@timmy git").unwrap(); assert!( @@ -190,7 +187,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; let output = try_handle_command(&dispatch, "@timmy git").unwrap(); assert!( diff --git a/server/src/matrix/commands/mod.rs b/server/src/matrix/commands/mod.rs index 3d67d18..e0c0c04 100644 --- a/server/src/matrix/commands/mod.rs +++ b/server/src/matrix/commands/mod.rs @@ -52,9 +52,6 @@ pub struct CommandDispatch<'a> { pub ambient_rooms: &'a Arc>>, /// The room this message came from (needed by ambient). pub room_id: &'a str, - /// Whether the message directly addressed the bot (mention/reply). - /// Some commands (e.g. ambient) only operate when directly addressed. - pub is_addressed: bool, } /// Context passed to individual command handlers. @@ -71,9 +68,6 @@ pub struct CommandContext<'a> { pub ambient_rooms: &'a Arc>>, /// The room this message came from (needed by ambient). pub room_id: &'a str, - /// Whether the message directly addressed the bot (mention/reply). - /// Some commands (e.g. ambient) only operate when directly addressed. - pub is_addressed: bool, } /// Returns the full list of registered bot commands. @@ -171,7 +165,6 @@ pub fn try_handle_command(dispatch: &CommandDispatch<'_>, message: &str) -> Opti agents: dispatch.agents, ambient_rooms: dispatch.ambient_rooms, room_id: dispatch.room_id, - is_addressed: dispatch.is_addressed, }; commands() @@ -291,7 +284,6 @@ pub(crate) mod tests { bot_user_id: &str, message: &str, ambient_rooms: &Arc>>, - is_addressed: bool, ) -> Option { let agents = test_agents(); let room_id = "!test:example.com".to_string(); @@ -302,13 +294,12 @@ pub(crate) mod tests { agents: &agents, ambient_rooms, room_id: &room_id, - is_addressed, }; try_handle_command(&dispatch, message) } pub fn try_cmd_addressed(bot_name: &str, bot_user_id: &str, message: &str) -> Option { - try_cmd(bot_name, bot_user_id, message, &test_ambient_rooms(), true) + try_cmd(bot_name, bot_user_id, message, &test_ambient_rooms()) } // Re-export commands() for submodule tests diff --git a/server/src/matrix/commands/move_story.rs b/server/src/matrix/commands/move_story.rs index 9c786d5..7439c1e 100644 --- a/server/src/matrix/commands/move_story.rs +++ b/server/src/matrix/commands/move_story.rs @@ -138,7 +138,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; try_handle_command(&dispatch, &format!("@timmy move {args}")) } diff --git a/server/src/matrix/commands/overview.rs b/server/src/matrix/commands/overview.rs index fbdd29c..35827f6 100644 --- a/server/src/matrix/commands/overview.rs +++ b/server/src/matrix/commands/overview.rs @@ -238,7 +238,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; try_handle_command(&dispatch, &format!("@timmy overview {args}")) } diff --git a/server/src/matrix/commands/show.rs b/server/src/matrix/commands/show.rs index 43a9e84..0b25c00 100644 --- a/server/src/matrix/commands/show.rs +++ b/server/src/matrix/commands/show.rs @@ -87,7 +87,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; try_handle_command(&dispatch, &format!("@timmy show {args}")) } diff --git a/server/src/slack.rs b/server/src/slack.rs index 955ce9f..15d1fce 100644 --- a/server/src/slack.rs +++ b/server/src/slack.rs @@ -678,7 +678,6 @@ pub async fn slash_command_receive( agents: &ctx.agents, ambient_rooms: &ctx.ambient_rooms, room_id: &payload.channel_id, - is_addressed: true, }; let response_text = try_handle_command(&dispatch, &synthetic_message) @@ -711,8 +710,6 @@ async fn handle_incoming_message( agents: &ctx.agents, ambient_rooms: &ctx.ambient_rooms, room_id: channel, - // Slack messages in configured channels are always "addressed" to the bot. - is_addressed: true, }; if let Some(response) = try_handle_command(&dispatch, message) { @@ -1429,7 +1426,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; let result = try_handle_command(&dispatch, &synthetic); @@ -1457,7 +1453,6 @@ mod tests { agents: &agents, ambient_rooms: &ambient_rooms, room_id: &room_id, - is_addressed: true, }; let result = try_handle_command(&dispatch, &synthetic); diff --git a/server/src/whatsapp.rs b/server/src/whatsapp.rs index e4fb273..b9f36ca 100644 --- a/server/src/whatsapp.rs +++ b/server/src/whatsapp.rs @@ -722,8 +722,6 @@ async fn handle_incoming_message( agents: &ctx.agents, ambient_rooms: &ctx.ambient_rooms, room_id: sender, - // WhatsApp messages are always "addressed" to the bot (1:1 or bot-specific). - is_addressed: true, }; if let Some(response) = try_handle_command(&dispatch, message) {