mirror of
https://github.com/tinyhumansai/openhuman.git
synced 2026-07-27 21:08:00 +00:00
fix(triage): defer instead of error on prompt-guard rejection (OPENHUMAN-TAURI-X) (#1678)
This commit is contained in:
@@ -210,6 +210,12 @@ where
|
||||
// eventual Deferred reason explains *why* we're sitting idle rather
|
||||
// than the generic "both arms failed" copy.
|
||||
let mut cloud_budget_exhausted: Option<anyhow::Error> = None;
|
||||
// Track whether the cloud arm bailed because the prompt-guard
|
||||
// flagged the content (OPENHUMAN-TAURI-X). The guard runs before
|
||||
// dispatch and is shared across arms, so a flag on cloud will
|
||||
// repeat on local — surface the verdict in the Deferred reason
|
||||
// for operator-facing telemetry.
|
||||
let mut cloud_safety_flagged: Option<anyhow::Error> = None;
|
||||
|
||||
// ── Cloud arm ──────────────────────────────────────────────────
|
||||
match try_arm(&cloud, envelope, TriageResolutionPath::Cloud).await {
|
||||
@@ -227,6 +233,18 @@ where
|
||||
);
|
||||
cloud_budget_exhausted = Some(err);
|
||||
}
|
||||
Err(ArmError::SafetyFlagged(err)) => {
|
||||
tracing::warn!(
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
path = TriageResolutionPath::Cloud.as_str(),
|
||||
error = %err,
|
||||
"[triage::evaluator] cloud rejected by prompt-guard; \
|
||||
skipping retry and falling back to local arm"
|
||||
);
|
||||
cloud_safety_flagged = Some(err);
|
||||
}
|
||||
Err(ArmError::Retryable { retry_after_ms, .. }) => {
|
||||
// Sleep before the cloud retry. Honour Retry-After when
|
||||
// present; otherwise use a short backoff so the second
|
||||
@@ -257,6 +275,18 @@ where
|
||||
);
|
||||
cloud_budget_exhausted = Some(err);
|
||||
}
|
||||
Err(ArmError::SafetyFlagged(err)) => {
|
||||
tracing::warn!(
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
path = TriageResolutionPath::CloudAfterRetry.as_str(),
|
||||
error = %err,
|
||||
"[triage::evaluator] cloud rejected by prompt-guard on retry; \
|
||||
falling back to local arm"
|
||||
);
|
||||
cloud_safety_flagged = Some(err);
|
||||
}
|
||||
Err(ArmError::Retryable { .. }) => {
|
||||
// Exhausted cloud budget — fall through to local.
|
||||
tracing::warn!(
|
||||
@@ -278,19 +308,28 @@ where
|
||||
// forwarded into telemetry / UI, so it must stay a stable,
|
||||
// scrubbed string. Raw upstream error text goes to the debug
|
||||
// log instead, where it is operator-visible but not surfaced.
|
||||
let reason = match &cloud_budget_exhausted {
|
||||
Some(err) => {
|
||||
tracing::debug!(
|
||||
target: "[triage::evaluator]",
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
error = %err,
|
||||
"cloud budget exhausted; no local arm — full upstream error"
|
||||
);
|
||||
"cloud budget exhausted; local arm unavailable".to_string()
|
||||
}
|
||||
None => "cloud retry exhausted; local arm unavailable".to_string(),
|
||||
let reason = if let Some(err) = cloud_safety_flagged.as_ref() {
|
||||
tracing::debug!(
|
||||
target: "[triage::evaluator]",
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
error = %err,
|
||||
"prompt-guard rejected on cloud; no local arm — full guard verdict"
|
||||
);
|
||||
"prompt-guard rejection; local arm unavailable".to_string()
|
||||
} else if let Some(err) = cloud_budget_exhausted.as_ref() {
|
||||
tracing::debug!(
|
||||
target: "[triage::evaluator]",
|
||||
source = %envelope.source.slug(),
|
||||
label = %envelope.display_label,
|
||||
external_id = %envelope.external_id,
|
||||
error = %err,
|
||||
"cloud budget exhausted; no local arm — full upstream error"
|
||||
);
|
||||
"cloud budget exhausted; local arm unavailable".to_string()
|
||||
} else {
|
||||
"cloud retry exhausted; local arm unavailable".to_string()
|
||||
};
|
||||
return Ok(TriageOutcome::Deferred {
|
||||
defer_until_ms: now_ms().saturating_add(DEFER_WAKEUP_MS),
|
||||
@@ -306,6 +345,7 @@ where
|
||||
Ok(run) => Ok(TriageOutcome::Decision(run)),
|
||||
Err(ArmError::Fatal(err))
|
||||
| Err(ArmError::BudgetExhausted(err))
|
||||
| Err(ArmError::SafetyFlagged(err))
|
||||
| Err(ArmError::Retryable { source: err, .. }) => {
|
||||
// Local also failed — defer rather than surface a hard
|
||||
// error. Today's "hard fail" is the wrong default for a
|
||||
@@ -315,9 +355,12 @@ where
|
||||
// flow into telemetry / UI, so keep it scrubbed. Raw error
|
||||
// text from cloud + local lives in the structured warn
|
||||
// fields below — visible to operators, not callers.
|
||||
let reason = match cloud_budget_exhausted.as_ref() {
|
||||
Some(_) => "cloud budget exhausted; local arm also failed".to_string(),
|
||||
None => "cloud retry exhausted; local arm also failed".to_string(),
|
||||
let reason = if cloud_safety_flagged.is_some() {
|
||||
"prompt-guard rejection; local arm also failed".to_string()
|
||||
} else if cloud_budget_exhausted.is_some() {
|
||||
"cloud budget exhausted; local arm also failed".to_string()
|
||||
} else {
|
||||
"cloud retry exhausted; local arm also failed".to_string()
|
||||
};
|
||||
tracing::warn!(
|
||||
target: "[triage::evaluator]",
|
||||
@@ -327,6 +370,7 @@ where
|
||||
local_error = %err,
|
||||
cloud_error = cloud_budget_exhausted
|
||||
.as_ref()
|
||||
.or(cloud_safety_flagged.as_ref())
|
||||
.map(|e| e.to_string())
|
||||
.unwrap_or_default(),
|
||||
defer_ms = DEFER_WAKEUP_MS,
|
||||
@@ -361,6 +405,17 @@ enum ArmError {
|
||||
/// This is **not** a fatal error: the user takes an explicit
|
||||
/// action (top up) to fix it, so it must not page Sentry.
|
||||
BudgetExhausted(anyhow::Error),
|
||||
/// Our prompt-injection guard (`agent::bus`'s `enforce_prompt_input`,
|
||||
/// also used by `agent::harness::session::runtime`) flagged the
|
||||
/// incoming content as adversarial / unsafe and refused to dispatch
|
||||
/// the turn. The guard runs *before* either model is contacted, so
|
||||
/// trying the same prompt again — on cloud or local — produces the
|
||||
/// same verdict. This is **not** a fatal error: the guard is doing
|
||||
/// its job (OPENHUMAN-TAURI-X regression: an adversarial Gmail
|
||||
/// message reliably trips the guard, and every fire paged Sentry).
|
||||
/// Route the same way as `BudgetExhausted` so the local-arm fallthrough
|
||||
/// lands in `TriageOutcome::Deferred` rather than `Err(_)`.
|
||||
SafetyFlagged(anyhow::Error),
|
||||
}
|
||||
|
||||
/// Run a single arm: dispatch the agent turn through the native bus
|
||||
@@ -519,9 +574,41 @@ fn classify_error(message: String) -> ArmError {
|
||||
if is_inference_budget_exceeded(&message) {
|
||||
return ArmError::BudgetExhausted(err);
|
||||
}
|
||||
// Prompt-guard rejection (`agent::bus::enforce_prompt_input` →
|
||||
// `ReviewBlocked` / `Blocked`). The guard fires *before* either
|
||||
// arm contacts a model, so the verdict is identical on cloud and
|
||||
// local — no point retrying. Treat as Deferred-eligible so the
|
||||
// chain ends in `TriageOutcome::Deferred` rather than Fatal,
|
||||
// which was paging Sentry for adversarial-email triage attempts
|
||||
// (OPENHUMAN-TAURI-X regression).
|
||||
if is_prompt_guard_rejection(&message) {
|
||||
return ArmError::SafetyFlagged(err);
|
||||
}
|
||||
ArmError::Fatal(err)
|
||||
}
|
||||
|
||||
/// Returns `true` when `message` is the verbatim string our prompt-injection
|
||||
/// guard returns when it rejects a turn before dispatch.
|
||||
///
|
||||
/// Canonical sources:
|
||||
/// - `src/openhuman/agent/bus.rs` — `Blocked` / `ReviewBlocked` arms of the
|
||||
/// `enforce_prompt_input` decision (the path the triage evaluator hits via
|
||||
/// `agent.run_turn`).
|
||||
/// - `src/openhuman/agent/harness/session/runtime.rs` — same strings in the
|
||||
/// tool-call loop, kept identical so this classifier covers both.
|
||||
/// - `src/openhuman/local_ai/ops.rs` — user-facing variants with the
|
||||
/// `"Please rephrase clearly."` suffix; we match the leading phrase so
|
||||
/// either form classifies.
|
||||
///
|
||||
/// Kept narrow on purpose: the guard's full output strings are private to
|
||||
/// our code, so a substring match against the leading phrase will not collide
|
||||
/// with anything coming back from upstream providers.
|
||||
fn is_prompt_guard_rejection(message: &str) -> bool {
|
||||
let lower = message.to_ascii_lowercase();
|
||||
lower.contains("prompt flagged for security review")
|
||||
|| lower.contains("prompt blocked by security policy")
|
||||
}
|
||||
|
||||
/// Returns `true` when `message` signals that the upstream rejected the
|
||||
/// call because the user's inference budget or credit balance is empty —
|
||||
/// meaning a retry would hit the same wall.
|
||||
|
||||
@@ -163,6 +163,7 @@ fn classify_string_recognises_top_up_and_out_of_credits_as_budget_exhausted() {
|
||||
ArmError::Retryable { .. } => "Retryable",
|
||||
ArmError::Fatal(_) => "Fatal",
|
||||
ArmError::BudgetExhausted(_) => "BudgetExhausted",
|
||||
ArmError::SafetyFlagged(_) => "SafetyFlagged",
|
||||
};
|
||||
assert!(
|
||||
matches!(&err, ArmError::BudgetExhausted(_)),
|
||||
@@ -171,6 +172,60 @@ fn classify_string_recognises_top_up_and_out_of_credits_as_budget_exhausted() {
|
||||
}
|
||||
}
|
||||
|
||||
fn arm_error_label(err: &ArmError) -> &'static str {
|
||||
match err {
|
||||
ArmError::Retryable { .. } => "Retryable",
|
||||
ArmError::Fatal(_) => "Fatal",
|
||||
ArmError::BudgetExhausted(_) => "BudgetExhausted",
|
||||
ArmError::SafetyFlagged(_) => "SafetyFlagged",
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_string_recognises_prompt_guard_rejection_as_safety_flagged() {
|
||||
// OPENHUMAN-TAURI-X regression: the exact phrase our prompt-injection
|
||||
// guard emits from `agent::bus::enforce_prompt_input` /
|
||||
// `agent::harness::session::runtime` when it refuses to dispatch a
|
||||
// turn. Was previously classified as Fatal, which paged Sentry for
|
||||
// every adversarial Gmail message the triage agent saw.
|
||||
for raw in [
|
||||
"Prompt flagged for security review and was not processed.",
|
||||
"Prompt flagged for security review and was not processed. Please rephrase clearly.",
|
||||
"Prompt blocked by security policy.",
|
||||
// Wrapped in caller context (the agent bus surfaces the
|
||||
// verdict via `NativeRequestError::HandlerFailed` whose
|
||||
// message may carry additional prefix) — substring match
|
||||
// must still classify.
|
||||
"[agent.run_turn dispatch] HandlerFailed: Prompt flagged for security review and was not processed.",
|
||||
] {
|
||||
let err = classify_error(raw.to_string());
|
||||
let label = arm_error_label(&err);
|
||||
assert!(
|
||||
matches!(&err, ArmError::SafetyFlagged(_)),
|
||||
"expected SafetyFlagged for {raw:?}, got {label}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn classify_string_does_not_misclassify_unrelated_security_phrases() {
|
||||
// Conservative: only the verbatim guard phrases match. A doc-string
|
||||
// mentioning "security review" generically must NOT classify, or
|
||||
// we'd silently hide real issues from Sentry.
|
||||
for raw in [
|
||||
"scheduling a quarterly security review",
|
||||
"the runbook covers security policy violations",
|
||||
"ai security report attached",
|
||||
] {
|
||||
let err = classify_error(raw.to_string());
|
||||
let label = arm_error_label(&err);
|
||||
assert!(
|
||||
matches!(&err, ArmError::Fatal(_)),
|
||||
"expected Fatal for {raw:?}, got {label}"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ── Tiered fallback integration tests ───────────────────────────
|
||||
//
|
||||
// These drive `run_triage_with_arms` end-to-end through the agent
|
||||
@@ -557,6 +612,128 @@ async fn cloud_budget_exhausted_without_local_returns_deferred_not_err() {
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_safety_flagged_then_local_flagged_defers_not_errs() {
|
||||
// Regression for OPENHUMAN-TAURI-X (regressed): the prompt-injection
|
||||
// guard fires the same verdict on cloud and local arms (the guard
|
||||
// runs in `agent::bus::run_turn` before either model is contacted),
|
||||
// so the realistic path is cloud-flagged → local-flagged → defer.
|
||||
// Previously this paged Sentry every time an adversarial Gmail
|
||||
// triage attempt fired (118 hits in 6 days).
|
||||
AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");
|
||||
let counter = StdArc::new(AtomicUsize::new(0));
|
||||
let counter_for_stub = StdArc::clone(&counter);
|
||||
|
||||
let _guard = mock_agent_run_turn(move |_req| {
|
||||
let counter = StdArc::clone(&counter_for_stub);
|
||||
async move {
|
||||
counter.fetch_add(1, Ordering::SeqCst);
|
||||
// Verbatim string our prompt-injection guard emits.
|
||||
Err("Prompt flagged for security review and was not processed.".to_string())
|
||||
}
|
||||
})
|
||||
.await;
|
||||
|
||||
let outcome = run_triage_with_arms_for_test(cloud_arm(), Some(local_arm()), &envelope())
|
||||
.await
|
||||
.expect("safety-flagged must not surface as Err — must Defer");
|
||||
|
||||
match outcome {
|
||||
TriageOutcome::Deferred { reason, .. } => {
|
||||
assert!(
|
||||
reason.to_lowercase().contains("prompt-guard"),
|
||||
"deferral reason should name the prompt-guard cause: {reason}"
|
||||
);
|
||||
}
|
||||
TriageOutcome::Decision(_) => panic!("expected Deferred, got Decision"),
|
||||
}
|
||||
assert_eq!(
|
||||
counter.load(Ordering::SeqCst),
|
||||
2,
|
||||
"1 cloud (safety-flagged, no retry) + 1 local (same verdict) = 2"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_safety_flagged_then_local_recovers_decides_on_local() {
|
||||
// Defense in depth: while in practice cloud + local share the
|
||||
// guard verdict, the chain must still cleanly dispatch to local
|
||||
// if cloud is the only arm that flagged. Locks in the
|
||||
// skip-retry-and-fall-through semantics independently of whether
|
||||
// local also flags.
|
||||
AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");
|
||||
let counter = StdArc::new(AtomicUsize::new(0));
|
||||
let counter_for_stub = StdArc::clone(&counter);
|
||||
|
||||
let _guard = mock_agent_run_turn(move |req| {
|
||||
let counter = StdArc::clone(&counter_for_stub);
|
||||
async move {
|
||||
let n = counter.fetch_add(1, Ordering::SeqCst);
|
||||
if n == 0 {
|
||||
assert_eq!(req.provider_name, "stub-cloud", "first call must hit cloud");
|
||||
Err("Prompt flagged for security review and was not processed.".to_string())
|
||||
} else {
|
||||
assert_eq!(
|
||||
req.provider_name, "stub-local",
|
||||
"safety-flagged on cloud must skip cloud retry and dispatch to local"
|
||||
);
|
||||
Ok(AgentTurnResponse {
|
||||
text: VALID_JSON_REPLY.to_string(),
|
||||
})
|
||||
}
|
||||
}
|
||||
})
|
||||
.await;
|
||||
|
||||
let outcome = run_triage_with_arms_for_test(cloud_arm(), Some(local_arm()), &envelope())
|
||||
.await
|
||||
.expect("safety-flagged must not surface as Err");
|
||||
|
||||
let run = outcome.into_decision().expect("decision");
|
||||
assert_eq!(run.resolution_path, TriageResolutionPath::LocalFallback);
|
||||
assert!(run.used_local);
|
||||
assert_eq!(
|
||||
counter.load(Ordering::SeqCst),
|
||||
2,
|
||||
"1 cloud (safety-flagged) + 1 local = 2 (no cloud retry)"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn cloud_safety_flagged_without_local_returns_deferred_not_err() {
|
||||
AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");
|
||||
let counter = StdArc::new(AtomicUsize::new(0));
|
||||
let counter_for_stub = StdArc::clone(&counter);
|
||||
|
||||
let _guard = mock_agent_run_turn(move |_req| {
|
||||
let counter = StdArc::clone(&counter_for_stub);
|
||||
async move {
|
||||
counter.fetch_add(1, Ordering::SeqCst);
|
||||
Err("Prompt flagged for security review and was not processed.".to_string())
|
||||
}
|
||||
})
|
||||
.await;
|
||||
|
||||
let outcome = run_triage_with_arms_for_test(cloud_arm(), None, &envelope())
|
||||
.await
|
||||
.expect("safety-flagged with no local must Defer, not Err");
|
||||
|
||||
match outcome {
|
||||
TriageOutcome::Deferred { reason, .. } => {
|
||||
assert!(
|
||||
reason.to_lowercase().contains("prompt-guard"),
|
||||
"deferral reason should name the prompt-guard cause: {reason}"
|
||||
);
|
||||
}
|
||||
TriageOutcome::Decision(_) => panic!("expected Deferred"),
|
||||
}
|
||||
assert_eq!(
|
||||
counter.load(Ordering::SeqCst),
|
||||
1,
|
||||
"no retry — guard would block the second cloud call too"
|
||||
);
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn no_local_arm_returns_deferred_after_cloud_exhaustion() {
|
||||
AgentDefinitionRegistry::init_global_builtins().expect("init_global_builtins");
|
||||
|
||||
Reference in New Issue
Block a user