From e8172f501bc1e035e692a51d99e7b943a5eb5442 Mon Sep 17 00:00:00 2001 From: Steven Enamakel Date: Fri, 3 Jul 2026 23:59:49 +0000 Subject: [PATCH] fix(agent-harness): route the no-progress nudge through InjectMessage so interactive turns don't crash (#4089) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The tinyagents 1.5 migration (#4473) moved the no-progress escalation ladder into the crate (`no_progress::NoProgressTracker`), which feeds a structured "no progress since step X" corrective back into the loop as a `NoProgress::Nudge` — giving the model one chance to change strategy before the same-strategy retry cap. OpenHuman's `RepeatedToolFailureMiddleware` delivered that nudge via `SteeringCommand::Redirect`. But `Redirect` is Background (sub-agent) only: the interactive-turn steering policy deliberately permits just `InjectMessage`/`Pause` so the user's live turn can't be redirected out from under them by a rogue steer (`orchestration.rs`). The middleware runs on *every* turn, so every interactive turn that hit the nudge aborted with `steering command redirect is not permitted by the run policy` — a #4473 regression that red-lines the raw-tool-loop / monitor E2E coverage suites. Deliver the nudge as a system message via the `InjectMessage` lane instead. The corrective is trusted, system-generated advisory text, so `InjectMessage` is both permitted (on interactive *and* background) and semantically correct — the change-strategy steering #4089 asks for now actually reaches the model on interactive chat instead of crashing it. Tests: the nudge fires once (as an InjectMessage carrying the "no progress" signal + the failing call name) on the 2nd identical failure, before the halt; and a regression assertion that the interactive steering policy permits the `InjectMessage` lane the nudge now uses (and still refuses `Redirect`). Closes #4089. Claude-Session: https://claude.ai/code/session_01KcmdqJVpjmnH31HqTHRLwG --- src/openhuman/tinyagents/middleware.rs | 92 ++++++++++++++++++- .../config_auth_app_state_connectivity_e2e.rs | 2 + 2 files changed, 91 insertions(+), 3 deletions(-) diff --git a/src/openhuman/tinyagents/middleware.rs b/src/openhuman/tinyagents/middleware.rs index 7ec1bd65e6..23421c80fd 100644 --- a/src/openhuman/tinyagents/middleware.rs +++ b/src/openhuman/tinyagents/middleware.rs @@ -1640,9 +1640,19 @@ impl Middleware<()> for RepeatedToolFailureMiddleware { hard_reject, "[tinyagents::mw] no-progress nudge — steering the model to change strategy before the retry cap" ); - // Inject the crate's structured corrective into the working - // transcript (advisory system text; bypasses no security gate). - self.handle.send(SteeringCommand::Redirect { instruction }); + // Inject the crate's structured corrective as a system message via + // the `InjectMessage` steering lane. This runs on *every* turn, + // including the user's live interactive turn, whose steering policy + // permits only `InjectMessage`/`Pause` — `Redirect` is Background + // (sub-agent) only, so sending it here aborted every interactive + // turn that hit the nudge with `steering command redirect is not + // permitted by the run policy` (a #4473 migration regression). The + // corrective is trusted, system-generated advisory text, so the + // `InjectMessage` lane is both permitted and semantically correct. + self.handle + .send(SteeringCommand::InjectMessage(TaMessage::system( + instruction, + ))); } NoProgress::Halt(summary) => { tracing::warn!( @@ -2093,6 +2103,82 @@ mod tests { ); } + /// Collect the nudge system-message texts drained from `handle`. The nudge + /// rides the `InjectMessage` lane (not `Redirect`) so it is permitted on the + /// user's interactive turn — see the test below. + fn drain_nudge_messages(handle: &SteeringHandle) -> Vec { + handle + .drain() + .into_iter() + .filter_map(|c| match c { + SteeringCommand::InjectMessage(message) => Some(message.text()), + _ => None, + }) + .collect() + } + + #[tokio::test] + async fn repeated_tool_failure_nudges_change_of_strategy_before_the_halt() { + use crate::openhuman::tinyagents::orchestration::{ + openhuman_steering_handle, SteeringRunClass, + }; + use tinyagents::harness::steering::SteeringCommandKind; + + // #4089: before the same-strategy retry cap, the breaker must feed a + // structured "no progress since step X" corrective back into the loop so + // the model changes approach rather than retrying the identical failing + // call — and it must do so *without* pausing yet. + let handle = SteeringHandle::allow_all(); + let mw = RepeatedToolFailureMiddleware::new( + handle.clone(), + 3, + std::sync::Arc::new(std::sync::Mutex::new(None)), + ); + // First identical failure: not a loop yet — no steering. + let mut r = failing_result("read_file", "file not found"); + mw.after_tool(&mut ctx(), &(), &mut r).await.unwrap(); + assert!( + handle.drain().is_empty(), + "a single failure is never a loop" + ); + // Second identical failure: the nudge fires, still no halt. + let mut r = failing_result("read_file", "file not found"); + mw.after_tool(&mut ctx(), &(), &mut r).await.unwrap(); + let nudges = drain_nudge_messages(&handle); + assert_eq!( + nudges.len(), + 1, + "the repeat should steer the model to change strategy before the retry cap" + ); + let nudge = &nudges[0]; + assert!( + nudge.contains("no progress"), + "the nudge carries the structured no-progress signal: {nudge}" + ); + assert!( + nudge.to_lowercase().contains("read_file"), + "the nudge names the failing call so the model knows what not to repeat: {nudge}" + ); + + // Regression for the #4473 crash: the nudge must ride a steering lane the + // user's *interactive* turn permits. `Redirect` is Background-only, so a + // Redirect nudge aborted interactive turns; `InjectMessage` is permitted + // on both classes. Assert the interactive policy accepts the lane we use. + let interactive = openhuman_steering_handle(SteeringRunClass::Interactive); + assert!( + interactive + .policy() + .is_allowed(SteeringCommandKind::InjectMessage), + "the no-progress nudge must use a lane the interactive turn permits" + ); + assert!( + !interactive + .policy() + .is_allowed(SteeringCommandKind::Redirect), + "sanity: interactive still refuses Redirect (the lane that crashed it)" + ); + } + // ── ApprovalSecurityMiddleware ────────────────────────────────────────── #[test] diff --git a/tests/config_auth_app_state_connectivity_e2e.rs b/tests/config_auth_app_state_connectivity_e2e.rs index 266d46584c..802855f2fe 100644 --- a/tests/config_auth_app_state_connectivity_e2e.rs +++ b/tests/config_auth_app_state_connectivity_e2e.rs @@ -2802,6 +2802,7 @@ async fn worker_a_controller_schemas_are_fully_exposed() { "openhuman.config_get_meet_settings", "openhuman.config_get_memory_sync_settings", "openhuman.config_get_onboarding_completed", + "openhuman.config_get_privacy_mode", "openhuman.config_get_runtime_flags", "openhuman.config_get_sandbox_settings", "openhuman.config_get_search_settings", @@ -2811,6 +2812,7 @@ async fn worker_a_controller_schemas_are_fully_exposed() { "openhuman.config_resolve_api_url", "openhuman.config_set_browser_allow_all", "openhuman.config_set_onboarding_completed", + "openhuman.config_set_privacy_mode", "openhuman.config_set_super_context_enabled", "openhuman.config_update_activity_level_settings", "openhuman.config_update_agent_paths",