From 07c52ea03c75ae1f7f2ee6430b29bf1a37b6db51 Mon Sep 17 00:00:00 2001 From: Ben Hoverter <32376575+benhoverter@users.noreply.github.com> Date: Wed, 27 May 2026 19:38:50 -0700 Subject: [PATCH] runtime: gate phantom-action guard on text-reply-is-delivery MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The phantom-action guard in `run_agent_loop` re-prompts the LLM when it emits action-shaped text (e.g. "I sent the message to the channel.") without calling any tool. In channel-reply paths (Discord, Telegram, etc.), however, the bridge delivers the agent's text response verbatim back to the originating channel — the text IS the delivery, not a hallucinated tool call. The guard misfires on these legitimate turns and either re-prompts the model or surfaces a "claimed action but did not call any tools" system reminder that the next turn tries to argue with. Fix: thread a `text_reply_is_delivery: bool` from each kernel entry point through `execute_llm_agent` into `run_agent_loop`, and gate the phantom detector on `!text_reply_is_delivery`. Channel adapters (`KernelBridgeAdapter`) opt in via new wrappers `send_message_channel_reply{,_with_blocks}`; cron, peer-to-peer agent_send, and API direct paths keep the default `false` and the detector behaves unchanged for them. The detector logic itself is unchanged — only the gating condition becomes more conservative (never more aggressive). The streaming variant (`run_agent_loop_streaming`) does not reference the phantom detector, so no change there. Plumbing: * `agent_loop::run_agent_loop`: new last param `text_reply_is_delivery` * `kernel::send_message_with_handle_and_blocks`: new last param * `kernel::execute_llm_agent`: new last param, forwarded * `kernel::send_message_channel_reply{,_with_blocks}`: new wrappers that pass `true` * `KernelBridgeAdapter::send_message{,_with_blocks}`: switched to the new channel-reply wrappers * `kernel::send_message{,_with_blocks,_with_handle}`: pass `false` * `routes::send_message` API direct call: passes `false` * Cron path (`send_message_with_handle` at delivery): passes `false` Tests: * `phantom_guard_fires_when_text_reply_is_delivery_false` — confirms unchanged behavior for non-channel callers. * `phantom_guard_suppressed_when_text_reply_is_delivery_true` — confirms the bug is fixed for channel-reply callers. Both use a `PhantomShapedDriver` that returns action-shaped text on iteration 0 and a distinct marker on iteration 1, so the test asserts on which turn's output is delivered. Verified: * `cargo test -p openfang-runtime --lib` → 995/995 pass * `cargo test -p openfang-kernel --lib` → 289/289 pass * `cargo test -p openfang-api --lib` → 92/92 pass * `cargo clippy -p openfang-{runtime,kernel,api} --all-targets -- -D warnings` → clean --- crates/openfang-api/src/channel_bridge.rs | 4 +- crates/openfang-api/src/routes.rs | 1 + crates/openfang-kernel/src/kernel.rs | 50 ++++ crates/openfang-runtime/src/agent_loop.rs | 280 ++++++++++++++++++---- 4 files changed, 284 insertions(+), 51 deletions(-) diff --git a/crates/openfang-api/src/channel_bridge.rs b/crates/openfang-api/src/channel_bridge.rs index 37ad72921f..6f7c42b7aa 100644 --- a/crates/openfang-api/src/channel_bridge.rs +++ b/crates/openfang-api/src/channel_bridge.rs @@ -74,7 +74,7 @@ impl ChannelBridgeHandle for KernelBridgeAdapter { async fn send_message(&self, agent_id: AgentId, message: &str) -> Result { let result = self .kernel - .send_message(agent_id, message) + .send_message_channel_reply(agent_id, message) .await .map_err(|e| format!("{e}"))?; // Silent/NO_REPLY responses should not be forwarded to channels @@ -105,7 +105,7 @@ impl ChannelBridgeHandle for KernelBridgeAdapter { }; let result = self .kernel - .send_message_with_blocks(agent_id, &text, blocks) + .send_message_channel_reply_with_blocks(agent_id, &text, blocks) .await .map_err(|e| format!("{e}"))?; Ok(result.response) diff --git a/crates/openfang-api/src/routes.rs b/crates/openfang-api/src/routes.rs index cebb1f599a..d383a2e56d 100644 --- a/crates/openfang-api/src/routes.rs +++ b/crates/openfang-api/src/routes.rs @@ -393,6 +393,7 @@ pub async fn send_message( content_blocks, req.sender_id, req.sender_name, + false, ) .await { diff --git a/crates/openfang-kernel/src/kernel.rs b/crates/openfang-kernel/src/kernel.rs index 8f59414c97..bff27c6b25 100644 --- a/crates/openfang-kernel/src/kernel.rs +++ b/crates/openfang-kernel/src/kernel.rs @@ -1853,6 +1853,50 @@ impl OpenFangKernel { Some(blocks), None, None, + false, + ) + .await + } + + /// Send a message in a channel-reply context: the caller (a channel + /// bridge) will deliver the agent's text response back to the originating + /// user-visible channel verbatim. Sets `text_reply_is_delivery = true` + /// so the phantom-action detector does not misfire on legitimate + /// text-only replies that describe channel actions. + pub async fn send_message_channel_reply( + &self, + agent_id: AgentId, + message: &str, + ) -> KernelResult { + let handle: Option> = self + .self_handle + .get() + .and_then(|w| w.upgrade()) + .map(|arc| arc as Arc); + self.send_message_with_handle_and_blocks(agent_id, message, handle, None, None, None, true) + .await + } + + /// Multimodal channel-reply variant; see [`Self::send_message_channel_reply`]. + pub async fn send_message_channel_reply_with_blocks( + &self, + agent_id: AgentId, + message: &str, + blocks: Vec, + ) -> KernelResult { + let handle: Option> = self + .self_handle + .get() + .and_then(|w| w.upgrade()) + .map(|arc| arc as Arc); + self.send_message_with_handle_and_blocks( + agent_id, + message, + handle, + Some(blocks), + None, + None, + true, ) .await } @@ -1873,6 +1917,7 @@ impl OpenFangKernel { None, sender_id, sender_name, + false, ) .await } @@ -1886,6 +1931,7 @@ impl OpenFangKernel { /// Per-agent locking ensures that concurrent messages for the same agent /// are serialized (preventing session corruption), while messages for /// different agents run in parallel. + #[allow(clippy::too_many_arguments)] pub async fn send_message_with_handle_and_blocks( &self, agent_id: AgentId, @@ -1894,6 +1940,7 @@ impl OpenFangKernel { content_blocks: Option>, sender_id: Option, sender_name: Option, + text_reply_is_delivery: bool, ) -> KernelResult { // Acquire per-agent lock to serialize concurrent messages for the same agent. // This prevents session corruption when multiple messages arrive in quick @@ -1931,6 +1978,7 @@ impl OpenFangKernel { content_blocks, sender_id, sender_name, + text_reply_is_delivery, ) .await }; @@ -2637,6 +2685,7 @@ impl OpenFangKernel { content_blocks: Option>, sender_id: Option, sender_name: Option, + text_reply_is_delivery: bool, ) -> KernelResult { // Check metering quota before starting self.metering @@ -2969,6 +3018,7 @@ impl OpenFangKernel { ctx_window, Some(&self.process_manager), content_blocks, + text_reply_is_delivery, ) .await .map_err(KernelError::OpenFang)?; diff --git a/crates/openfang-runtime/src/agent_loop.rs b/crates/openfang-runtime/src/agent_loop.rs index 615fbfdb73..be5c0157e0 100644 --- a/crates/openfang-runtime/src/agent_loop.rs +++ b/crates/openfang-runtime/src/agent_loop.rs @@ -312,8 +312,21 @@ pub async fn run_agent_loop( context_window_tokens: Option, process_manager: Option<&crate::process_manager::ProcessManager>, user_content_blocks: Option>, + // When true, the calling context treats the agent's final text response + // as the user-visible delivery (e.g. Discord/Telegram channel-reply path, + // where the bridge sends the reply text back to the originating channel + // without the agent needing to call `channel_send`). In that case the + // `phantom_action_detected` guard must be suppressed: text-only replies + // containing words like "sent" or "channel" are legitimately the action, + // not a fabricated tool call. Default `false` for cron, agent_send, + // API direct, and other callers where text alone is NOT delivery. + text_reply_is_delivery: bool, ) -> OpenFangResult { - info!(agent = %manifest.name, "Starting agent loop"); + info!( + agent = %manifest.name, + text_reply_is_delivery, + "Starting agent loop" + ); // Extract hand-allowed env vars from manifest metadata (set by kernel for hand settings) let hand_allowed_env: Vec = manifest @@ -687,7 +700,15 @@ pub async fn run_agent_loop( // channel action (send, post, email, etc.) but never actually // called the corresponding tool, re-prompt once to force real // tool usage instead of hallucinated completion. - let text = if !any_tools_executed + // + // Suppressed when `text_reply_is_delivery` is true: in + // channel-reply paths the bridge delivers the agent's text + // verbatim back to the originating channel, so phrases like + // "I sent the message" are factually true descriptions of + // what's about to happen, not a hallucinated tool call. + // + let text = if !text_reply_is_delivery + && !any_tools_executed && iteration == 0 && phantom_action_detected(&text) { @@ -3813,14 +3834,15 @@ mod tests { None, None, None, - None, // on_phase - None, // media_engine - None, // tts_engine - None, // docker_config - None, // hooks - None, // context_window_tokens - None, // process_manager - None, // user_content_blocks + None, // on_phase + None, // media_engine + None, // tts_engine + None, // docker_config + None, // hooks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + false, // text_reply_is_delivery ) .await .expect("Loop should complete without error"); @@ -3866,14 +3888,15 @@ mod tests { None, None, None, - None, // on_phase - None, // media_engine - None, // tts_engine - None, // docker_config - None, // hooks - None, // context_window_tokens - None, // process_manager - None, // user_content_blocks + None, // on_phase + None, // media_engine + None, // tts_engine + None, // docker_config + None, // hooks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + false, // text_reply_is_delivery ) .await .expect("Loop should complete without error"); @@ -3921,14 +3944,15 @@ mod tests { None, None, None, - None, // on_phase - None, // media_engine - None, // tts_engine - None, // docker_config - None, // hooks - None, // context_window_tokens - None, // process_manager - None, // user_content_blocks + None, // on_phase + None, // media_engine + None, // tts_engine + None, // docker_config + None, // hooks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + false, // text_reply_is_delivery ) .await .expect("Loop should complete without error"); @@ -3974,14 +3998,15 @@ mod tests { None, None, None, - None, // on_phase - None, // media_engine - None, // tts_engine - None, // docker_config - None, // hooks - None, // context_window_tokens - None, // process_manager - None, // user_content_blocks + None, // on_phase + None, // media_engine + None, // tts_engine + None, // docker_config + None, // hooks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + false, // text_reply_is_delivery ) .await .expect("Loop should complete without error"); @@ -4149,9 +4174,10 @@ mod tests { None, None, None, - None, // context_window_tokens - None, // process_manager - None, // user_content_blocks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + false, // text_reply_is_delivery ) .await .expect("Loop should recover via retry"); @@ -4196,9 +4222,10 @@ mod tests { None, None, None, - None, // context_window_tokens - None, // process_manager - None, // user_content_blocks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + false, // text_reply_is_delivery ) .await .expect("Loop should complete with fallback"); @@ -5222,14 +5249,15 @@ mod tests { None, None, None, - None, // on_phase - None, // media_engine - None, // tts_engine - None, // docker_config - None, // hooks - None, // context_window_tokens - None, // process_manager - None, // user_content_blocks + None, // on_phase + None, // media_engine + None, // tts_engine + None, // docker_config + None, // hooks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + false, // text_reply_is_delivery ) .await .expect("Agent loop should complete"); @@ -5300,6 +5328,7 @@ mod tests { None, None, None, + false, ) .await .expect("Agent loop should recover nested XML tool calls"); @@ -5371,7 +5400,8 @@ mod tests { None, None, None, - None, // user_content_blocks + None, // user_content_blocks + false, // text_reply_is_delivery ) .await .expect("Normal loop should complete"); @@ -5490,4 +5520,156 @@ mod tests { assert!(!is_silent_token("SILENT")); assert!(!is_silent_token("")); } + + // === phantom_action_detected gating by text_reply_is_delivery === + + /// Mock driver that, on iteration 0, returns phantom-action-shaped text + /// (matches `phantom_action_detected`) with no tool calls. On iteration 1 + /// (only reached if the phantom guard re-prompts) it returns a distinct + /// marker text so callers can tell whether a re-prompt occurred. + struct PhantomShapedDriver { + call_count: AtomicU32, + } + + impl PhantomShapedDriver { + fn new() -> Self { + Self { + call_count: AtomicU32::new(0), + } + } + } + + #[async_trait] + impl LlmDriver for PhantomShapedDriver { + async fn complete( + &self, + _request: CompletionRequest, + ) -> Result { + let call = self.call_count.fetch_add(1, Ordering::Relaxed); + let text = if call == 0 { + // Matches phantom_action_detected: action verb + channel ref, + // and is exactly the legitimate "text-IS-delivery" shape we + // want to stop misfiring on in channel-reply contexts. + "I sent the message to the channel." + } else { + "REPROMPTED-FALLBACK" + }; + Ok(CompletionResponse { + content: vec![ContentBlock::Text { + text: text.to_string(), + provider_metadata: None, + }], + stop_reason: StopReason::EndTurn, + tool_calls: vec![], + usage: TokenUsage { + input_tokens: 10, + output_tokens: 8, + }, + }) + } + } + + /// When `text_reply_is_delivery == false` (cron / agent_send / API direct), + /// the phantom-action guard must still re-prompt on phantom-shaped text. + #[tokio::test] + async fn phantom_guard_fires_when_text_reply_is_delivery_false() { + let memory = openfang_memory::MemorySubstrate::open_in_memory(0.01).unwrap(); + let agent_id = openfang_types::agent::AgentId::new(); + let mut session = openfang_memory::session::Session { + id: openfang_types::agent::SessionId::new(), + agent_id, + messages: Vec::new(), + context_window_tokens: 0, + label: None, + }; + let manifest = test_manifest(); + let driver: Arc = Arc::new(PhantomShapedDriver::new()); + + let result = run_agent_loop( + &manifest, + "Send a message to #system-code", + &mut session, + &memory, + driver, + &[], + None, + None, + None, + None, + None, + None, + None, + None, // on_phase + None, // media_engine + None, // tts_engine + None, // docker_config + None, // hooks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + false, // text_reply_is_delivery + ) + .await + .expect("loop should complete"); + + assert_eq!( + result.response.trim(), + "REPROMPTED-FALLBACK", + "phantom guard must re-prompt when text_reply_is_delivery=false; got {:?}", + result.response + ); + } + + /// When `text_reply_is_delivery == true` (Discord/Telegram channel-reply + /// path), the same phantom-shaped text is the legitimate delivery and + /// must pass through unmodified — the guard is suppressed. This is the + /// behavior change this patch introduces. + #[tokio::test] + async fn phantom_guard_suppressed_when_text_reply_is_delivery_true() { + let memory = openfang_memory::MemorySubstrate::open_in_memory(0.01).unwrap(); + let agent_id = openfang_types::agent::AgentId::new(); + let mut session = openfang_memory::session::Session { + id: openfang_types::agent::SessionId::new(), + agent_id, + messages: Vec::new(), + context_window_tokens: 0, + label: None, + }; + let manifest = test_manifest(); + let driver: Arc = Arc::new(PhantomShapedDriver::new()); + + let result = run_agent_loop( + &manifest, + "Send a message to #system-code", + &mut session, + &memory, + driver, + &[], + None, + None, + None, + None, + None, + None, + None, + None, // on_phase + None, // media_engine + None, // tts_engine + None, // docker_config + None, // hooks + None, // context_window_tokens + None, // process_manager + None, // user_content_blocks + true, // text_reply_is_delivery — suppress phantom guard + ) + .await + .expect("loop should complete"); + + assert_eq!( + result.response.trim(), + "I sent the message to the channel.", + "phantom guard must be suppressed when text_reply_is_delivery=true; got {:?}", + result.response + ); + } }