huskies: merge 1192 bug compact command is swallowed by the registry placeholder before its real handler runs
This commit is contained in:
@@ -425,10 +425,15 @@ fn handle_reset_fallback(_ctx: &CommandContext) -> Option<String> {
|
||||
None
|
||||
}
|
||||
|
||||
/// Fallback handler for the `compact` command when it is not intercepted by
|
||||
/// the async handler in `on_room_message`. In practice this is never called —
|
||||
/// compact is detected and handled before `try_handle_command` is invoked.
|
||||
/// The entry exists in the registry only so `help` lists it.
|
||||
/// Fallback handler for the `compact` command.
|
||||
///
|
||||
/// This IS called on every `compact` dispatch — `try_handle_command` always
|
||||
/// invokes the matched handler, so this runs before the async `compact`
|
||||
/// check each transport performs afterward (`on_room_message` for Matrix; an
|
||||
/// inline check in `handle_incoming_message` for Discord/Slack/WhatsApp). It
|
||||
/// deliberately always returns `None` so that check gets a chance to run the
|
||||
/// real handler instead of the LLM. The entry exists in the registry so
|
||||
/// `help` lists it.
|
||||
///
|
||||
/// Returns `None` to prevent the LLM from receiving "compact" as a prompt.
|
||||
fn handle_compact_fallback(_ctx: &CommandContext) -> Option<String> {
|
||||
|
||||
@@ -12,9 +12,11 @@ pub mod digest;
|
||||
/// Resolves a Claude Code session transcript's path on disk from its id.
|
||||
pub(crate) mod transcript;
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::fs;
|
||||
use std::path::{Path, PathBuf};
|
||||
|
||||
use crate::chat::transport::matrix::RoomConversation;
|
||||
use crate::chat::util::strip_bot_mention;
|
||||
|
||||
/// A parsed `compact` command.
|
||||
@@ -118,6 +120,50 @@ pub fn compact_session(
|
||||
})
|
||||
}
|
||||
|
||||
/// Handle a `compact` command for any transport that keys its conversation
|
||||
/// history by a plain string (Discord channel id, Slack channel id, WhatsApp
|
||||
/// phone number, …), rather than Matrix's `OwnedRoomId`.
|
||||
///
|
||||
/// Mirrors [`crate::chat::transport::matrix::compact::handle_compact`]: looks
|
||||
/// up the room's session, distills its transcript into a digest, writes the
|
||||
/// digest as a seed, clears the session id and entries, sets `pending_seed`,
|
||||
/// then calls `save` to persist the updated history. Returns an actionable
|
||||
/// message and changes nothing when there is no active session, or when the
|
||||
/// session transcript cannot be read.
|
||||
pub async fn handle_compact_for_key(
|
||||
room_key: &str,
|
||||
history: &tokio::sync::Mutex<HashMap<String, RoomConversation>>,
|
||||
project_root: &Path,
|
||||
max_bytes: usize,
|
||||
save: impl FnOnce(&Path, &HashMap<String, RoomConversation>),
|
||||
) -> String {
|
||||
let mut guard = history.lock().await;
|
||||
let conv = guard.entry(room_key.to_string()).or_default();
|
||||
|
||||
let Some(session_id) = conv.session_id.clone() else {
|
||||
return "No active session to compact. Send a message first to start one.".to_string();
|
||||
};
|
||||
|
||||
let transcript_path = transcript::transcript_path(project_root, &session_id);
|
||||
|
||||
match compact_session(project_root, &transcript_path, room_key, max_bytes) {
|
||||
Ok(outcome) => {
|
||||
conv.session_id = None;
|
||||
conv.entries.clear();
|
||||
conv.pending_seed = Some(outcome.digest);
|
||||
save(project_root, &guard);
|
||||
format!(
|
||||
"Compacted session context: {} → {} bytes. Starting fresh — the distilled \
|
||||
summary will be included as background in your next message.",
|
||||
outcome.before_bytes, outcome.after_bytes
|
||||
)
|
||||
}
|
||||
Err(CompactError::TranscriptUnreadable(e)) => format!(
|
||||
"Could not compact: session transcript is unreadable ({e}). Nothing was changed."
|
||||
),
|
||||
}
|
||||
}
|
||||
|
||||
/// Path to the seed file for a given room, under
|
||||
/// `project_root/.huskies/chat_seeds/`.
|
||||
fn seed_file_path(project_root: &Path, room_key: &str) -> PathBuf {
|
||||
|
||||
@@ -216,6 +216,27 @@ pub(super) async fn handle_incoming_message(
|
||||
return;
|
||||
}
|
||||
|
||||
if crate::chat::compact::extract_compact_command(
|
||||
message,
|
||||
&ctx.services.bot_name,
|
||||
&ctx.services.bot_user_id,
|
||||
)
|
||||
.is_some()
|
||||
{
|
||||
slog!("[discord] Handling compact command from {user} in {channel}");
|
||||
let response = crate::chat::compact::handle_compact_for_key(
|
||||
channel,
|
||||
&ctx.history,
|
||||
&ctx.services.project_root,
|
||||
8_000,
|
||||
save_discord_history,
|
||||
)
|
||||
.await;
|
||||
let response = markdown_to_discord(&response);
|
||||
let _ = ctx.transport.send_message(channel, &response, "").await;
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(start_cmd) = crate::chat::transport::matrix::start::extract_start_command(
|
||||
message,
|
||||
&ctx.services.bot_name,
|
||||
@@ -630,4 +651,104 @@ mod tests {
|
||||
"assembled prompt must contain user message; got: {prompt}"
|
||||
);
|
||||
}
|
||||
|
||||
/// Regression test for story 1192: `compact` must not be swallowed by the
|
||||
/// registry placeholder. Drives the real entrypoint
|
||||
/// (`handle_incoming_message`, not `handle_compact_for_key` directly) so a
|
||||
/// future regression in the dispatch order is caught, and asserts the
|
||||
/// real handler ran: reply mentions the byte-size confirmation, and the
|
||||
/// history's session_id/entries were cleared with pending_seed set.
|
||||
#[tokio::test]
|
||||
async fn compact_command_runs_through_full_dispatch_and_clears_session() {
|
||||
use crate::chat::transport::matrix::{
|
||||
ConversationEntry, ConversationRole, RoomConversation,
|
||||
};
|
||||
use std::collections::HashSet;
|
||||
use std::sync::Arc;
|
||||
|
||||
let channel = "555444333";
|
||||
let session_id = "sess-discord-compact";
|
||||
|
||||
let tmp = tempfile::tempdir().unwrap();
|
||||
let project_root = tmp.path().join("project");
|
||||
std::fs::create_dir_all(&project_root).unwrap();
|
||||
let home = tempfile::tempdir().unwrap();
|
||||
// SAFETY: this test owns HOME for its duration; no other test in this
|
||||
// process reads HOME concurrently with this call.
|
||||
unsafe {
|
||||
std::env::set_var("HOME", home.path());
|
||||
}
|
||||
let transcript_dir =
|
||||
crate::chat::compact::transcript::transcript_path(&project_root, session_id)
|
||||
.parent()
|
||||
.unwrap()
|
||||
.to_path_buf();
|
||||
std::fs::create_dir_all(&transcript_dir).unwrap();
|
||||
let jsonl = r#"{"type":"user","message":{"role":"user","content":"hello"}}
|
||||
{"type":"assistant","message":{"role":"assistant","content":[{"type":"text","text":"hi there"}]}}"#;
|
||||
std::fs::write(transcript_dir.join(format!("{session_id}.jsonl")), jsonl).unwrap();
|
||||
|
||||
let history: DiscordConversationHistory = Arc::new(TokioMutex::new({
|
||||
let mut m = HashMap::new();
|
||||
m.insert(
|
||||
channel.to_string(),
|
||||
RoomConversation {
|
||||
session_id: Some(session_id.to_string()),
|
||||
entries: vec![ConversationEntry {
|
||||
role: ConversationRole::User,
|
||||
sender: "user123".to_string(),
|
||||
content: "hi".to_string(),
|
||||
}],
|
||||
pending_seed: None,
|
||||
last_compact_suggested_at_ms: None,
|
||||
},
|
||||
);
|
||||
m
|
||||
}));
|
||||
|
||||
let mut server = mockito::Server::new_async().await;
|
||||
let mock = server
|
||||
.mock("POST", format!("/channels/{channel}/messages").as_str())
|
||||
.match_body(mockito::Matcher::Regex(
|
||||
"Compacted session context".to_string(),
|
||||
))
|
||||
.with_body(r#"{"id": "1"}"#)
|
||||
.create_async()
|
||||
.await;
|
||||
|
||||
let services =
|
||||
crate::services::Services::new_test(project_root.clone(), "Huskies".to_string());
|
||||
let ctx = DiscordContext {
|
||||
services,
|
||||
bot_token: "test-token".to_string(),
|
||||
transport: Arc::new(DiscordTransport::with_api_base(
|
||||
"test-token".to_string(),
|
||||
server.url(),
|
||||
)),
|
||||
history: history.clone(),
|
||||
history_size: 20,
|
||||
channel_ids: HashSet::new(),
|
||||
allowed_users: HashSet::new(),
|
||||
};
|
||||
|
||||
handle_incoming_message(&ctx, channel, "user123", "compact").await;
|
||||
|
||||
mock.assert_async().await;
|
||||
|
||||
let guard = history.lock().await;
|
||||
let conv = guard.get(channel).unwrap();
|
||||
assert!(
|
||||
conv.session_id.is_none(),
|
||||
"session_id must be cleared after compact"
|
||||
);
|
||||
assert!(
|
||||
conv.entries.is_empty(),
|
||||
"entries must be cleared after compact"
|
||||
);
|
||||
assert_eq!(
|
||||
conv.pending_seed.as_deref(),
|
||||
Some("User: hello\nAssistant: hi there"),
|
||||
"pending_seed must hold the distilled digest"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -34,8 +34,10 @@ impl DiscordTransport {
|
||||
}
|
||||
}
|
||||
|
||||
/// Creates a `DiscordTransport` pointed at a custom API base URL, for
|
||||
/// tests that mock the Discord API instead of hitting discord.com.
|
||||
#[cfg(test)]
|
||||
fn with_api_base(bot_token: String, api_base: String) -> Self {
|
||||
pub(crate) fn with_api_base(bot_token: String, api_base: String) -> Self {
|
||||
Self {
|
||||
bot_token,
|
||||
client: reqwest::Client::new(),
|
||||
|
||||
@@ -263,6 +263,27 @@ pub(super) async fn handle_incoming_message(
|
||||
return;
|
||||
}
|
||||
|
||||
if crate::chat::compact::extract_compact_command(
|
||||
message,
|
||||
&ctx.services.bot_name,
|
||||
&ctx.services.bot_user_id,
|
||||
)
|
||||
.is_some()
|
||||
{
|
||||
slog!("[slack] Handling compact command from {user} in {channel}");
|
||||
let response = crate::chat::compact::handle_compact_for_key(
|
||||
channel,
|
||||
&ctx.history,
|
||||
&ctx.services.project_root,
|
||||
8_000,
|
||||
save_slack_history,
|
||||
)
|
||||
.await;
|
||||
let response = markdown_to_slack(&response);
|
||||
let _ = ctx.transport.send_message(channel, &response, "").await;
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(start_cmd) = crate::chat::transport::matrix::start::extract_start_command(
|
||||
message,
|
||||
&ctx.services.bot_name,
|
||||
|
||||
@@ -174,6 +174,27 @@ pub(super) async fn handle_incoming_message(
|
||||
return;
|
||||
}
|
||||
|
||||
if crate::chat::compact::extract_compact_command(
|
||||
message,
|
||||
&ctx.services.bot_name,
|
||||
&ctx.services.bot_user_id,
|
||||
)
|
||||
.is_some()
|
||||
{
|
||||
slog!("[whatsapp] Handling compact command from {sender}");
|
||||
let response = crate::chat::compact::handle_compact_for_key(
|
||||
sender,
|
||||
&ctx.history,
|
||||
&ctx.services.project_root,
|
||||
8_000,
|
||||
save_whatsapp_history,
|
||||
)
|
||||
.await;
|
||||
let formatted = markdown_to_whatsapp(&response);
|
||||
let _ = ctx.transport.send_message(sender, &formatted, "").await;
|
||||
return;
|
||||
}
|
||||
|
||||
if let Some(start_cmd) = crate::chat::transport::matrix::start::extract_start_command(
|
||||
message,
|
||||
&ctx.services.bot_name,
|
||||
|
||||
Reference in New Issue
Block a user