Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions app/src-tauri/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2358,6 +2358,7 @@ pub fn run() {
if openhuman_core::core::observability::is_transient_backend_api_failure(&event)
|| openhuman_core::core::observability::is_transient_integrations_failure(&event)
|| openhuman_core::core::observability::is_updater_transient_event(&event)
|| openhuman_core::core::observability::is_skill_install_user_fetch_failure(&event)
{
return None;
}
Expand Down
2 changes: 1 addition & 1 deletion docs/agent-workflows/cursor-cloud-agents.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# Cursor Cloud Agents — parallel workflow

Operator playbook for running 15–20 [Cursor Cloud Agents](https://docs.cursor.com/agents/cloud) in parallel against OpenHuman. Companion to [`codex-pr-checklist.md`](codex-pr-checklist.md); the same merge gates apply.
Operator playbook for running 15–20 [Cursor Cloud Agents](https://cursor.com/docs/cloud-agent) in parallel against OpenHuman. Companion to [`codex-pr-checklist.md`](codex-pr-checklist.md); the same merge gates apply.

This doc closes [`tinyhumansai/openhuman#1480`](https://github.com/tinyhumansai/openhuman/issues/1480).

Expand Down
82 changes: 82 additions & 0 deletions src/core/observability.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2665,6 +2665,29 @@ pub fn is_transient_backend_api_failure(event: &sentry::protocol::Event<'_>) ->
is_transient_domain_failure(event, "backend_api")
}

/// Defense-in-depth `before_send` filter for skill-install fetch 4xx statuses.
///
/// A user/catalog-supplied `SKILL.md` URL returning 4xx means the remote skill
/// path is missing, private, or otherwise unavailable to that user. The install
/// RPC still returns the error so the UI can surface it, but Sentry should keep
/// reporting server-side and transport failures only.
pub fn is_skill_install_user_fetch_failure(event: &sentry::protocol::Event<'_>) -> bool {
let tags = &event.tags;
if tags.get("domain").map(String::as_str) != Some("skills") {
return false;
}
if tags.get("operation").map(String::as_str) != Some("install_fetch") {
return false;
}
if tags.get("failure").map(String::as_str) != Some("non_2xx") {
return false;
}

tags.get("status")
.and_then(|status| status.parse::<u16>().ok())
.is_some_and(|status| (400..500).contains(&status))
}

/// Transient integrations / Composio failures (timeout, connection reset,
/// gateway hiccups).
///
Expand Down Expand Up @@ -6104,6 +6127,65 @@ mod tests {
);
}

#[test]
fn skills_install_fetch_filter_drops_client_error_statuses() {
for status in ["400", "401", "403", "404", "410", "499"] {
let event = event_with_tags(&[
("domain", "skills"),
("operation", "install_fetch"),
("failure", "non_2xx"),
("status", status),
]);
assert!(
is_skill_install_user_fetch_failure(&event),
"skills install_fetch status {status} must be treated as user/catalog state"
);
}
}

#[test]
fn skills_install_fetch_filter_keeps_server_and_wrong_shape_failures() {
for status in ["500", "502", "503"] {
let event = event_with_tags(&[
("domain", "skills"),
("operation", "install_fetch"),
("failure", "non_2xx"),
("status", status),
]);
assert!(
!is_skill_install_user_fetch_failure(&event),
"skills install_fetch status {status} must remain reportable"
);
}

for tags in [
[
("domain", "skills"),
("operation", "install_fetch"),
("failure", "transport"),
("status", "404"),
],
[
("domain", "skills"),
("operation", "run"),
("failure", "non_2xx"),
("status", "404"),
],
[
("domain", "backend_api"),
("operation", "install_fetch"),
("failure", "non_2xx"),
("status", "404"),
],
] {
let event = event_with_tags(&tags);
assert!(
!is_skill_install_user_fetch_failure(&event),
"only skills.install_fetch non_2xx 4xx events may be filtered: {tags:?}"
);
}
}

#[test]
fn integrations_filter_drops_transient_statuses() {
for status in TRANSIENT_HTTP_STATUSES {
Expand Down
1 change: 1 addition & 0 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ fn main() {
if openhuman_core::core::observability::is_transient_backend_api_failure(&event)
|| openhuman_core::core::observability::is_transient_integrations_failure(&event)
|| openhuman_core::core::observability::is_updater_transient_event(&event)
|| openhuman_core::core::observability::is_skill_install_user_fetch_failure(&event)
{
return None;
}
Expand Down
1 change: 1 addition & 0 deletions src/openhuman/workflows/ops.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub(crate) use super::ops_discover::discover_workflows_inner;
#[cfg(test)]
pub(crate) use super::ops_install::{
derive_install_slug, install_workflow_from_url_with_home, normalize_install_url,
should_report_install_fetch_status,
};
#[cfg(test)]
pub(crate) use super::ops_types::{
Expand Down
25 changes: 20 additions & 5 deletions src/openhuman/workflows/ops_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,6 +122,10 @@ pub async fn install_workflow_from_url(
install_workflow_from_url_with_home(workspace_dir, params, home.as_deref()).await
}

pub(crate) fn should_report_install_fetch_status(status: reqwest::StatusCode) -> bool {
!status.is_success() && !status.is_client_error()
}

pub(crate) async fn install_workflow_from_url_with_home(
workspace_dir: &Path,
params: InstallWorkflowFromUrlParams,
Expand Down Expand Up @@ -196,18 +200,23 @@ pub(crate) async fn install_workflow_from_url_with_home(

let status = response.status();
if !status.is_success() {
let code = status.as_u16();
let msg = format!("fetch failed: {fetch_url} returned status {code}");
// A 4xx (esp. 404/410) means the requested SKILL.md is gone or the URL
// is wrong — expected user/catalog input state, surfaced to the UI as
// "skill not found". Don't page Sentry for it (TAURI-RUST-CGE: ~1,446
// events / 72 users on `openhuman@0.57.53`, almost all 404). Keep
// reporting 5xx — a genuine remote failure is still Sentry-actionable.
// The `Err(msg)` return is unchanged in both cases so the UI always
// surfaces the failure.
if !status.is_client_error() {
let status_str = code.to_string();
let report_msg = format!("fetch failed: {redacted_fetch_url} returned status {code}");
let status_str = status.as_u16().to_string();
let msg = format!(
"fetch failed: {fetch_url} returned status {}",
status.as_u16()
);
let report_msg = format!(
"fetch failed: {redacted_fetch_url} returned status {}",
status.as_u16()
);
if should_report_install_fetch_status(status) {
crate::core::observability::report_error(
report_msg.as_str(),
"skills",
Expand All @@ -218,6 +227,12 @@ pub(crate) async fn install_workflow_from_url_with_home(
("failure", "non_2xx"),
],
);
} else {
tracing::debug!(
fetch_url = %redacted_fetch_url,
status = status.as_u16(),
"[skills] install_workflow_from_url: skipped Sentry report for user/catalog fetch status"
);
}
return Err(msg);
}
Expand Down
17 changes: 17 additions & 0 deletions src/openhuman/workflows/ops_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1244,6 +1244,23 @@ async fn install_workflow_from_url_is_idempotent_when_skill_already_exists() {
assert!(second.stdout.contains("already installed"), "{second:?}");
}

#[test]
fn install_fetch_status_reporting_suppresses_client_errors_only() {
assert!(!should_report_install_fetch_status(reqwest::StatusCode::OK));
assert!(!should_report_install_fetch_status(
reqwest::StatusCode::NOT_FOUND
));
assert!(!should_report_install_fetch_status(
reqwest::StatusCode::GONE
));
assert!(should_report_install_fetch_status(
reqwest::StatusCode::INTERNAL_SERVER_ERROR
));
assert!(should_report_install_fetch_status(
reqwest::StatusCode::BAD_GATEWAY
));
}

/// Happy path: install a SKILL.md under a synthetic user home, verify
/// discovery sees it, uninstall, verify discovery no longer sees it and
/// the on-disk dir is gone.
Expand Down
36 changes: 34 additions & 2 deletions tests/observability_smoke.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,9 @@

use openhuman_core::core::observability::{
is_all_transient_provider_exhaustion_event, is_budget_event, is_session_expired_event,
is_transient_backend_api_failure, is_transient_integrations_failure,
is_transient_provider_http_failure, is_updater_transient_event,
is_skill_install_user_fetch_failure, is_transient_backend_api_failure,
is_transient_integrations_failure, is_transient_provider_http_failure,
is_updater_transient_event,
};
use sentry::protocol::Event;
use std::collections::BTreeMap;
Expand Down Expand Up @@ -63,6 +64,7 @@ fn count_captured(events: Vec<Event<'static>>) -> usize {
|| is_transient_integrations_failure(&event)
|| is_budget_event(&event)
|| is_updater_transient_event(&event)
|| is_skill_install_user_fetch_failure(&event)
|| is_session_expired_event(&event)
{
None
Expand Down Expand Up @@ -119,6 +121,36 @@ fn drops_backend_api_transient_statuses() {
);
}

#[test]
fn drops_skills_install_fetch_404() {
let event = event_with_tags(&[
("domain", "skills"),
("operation", "install_fetch"),
("failure", "non_2xx"),
("status", "404"),
]);
assert_eq!(
count_captured(vec![event]),
0,
"user/catalog skill install 4xx failures must be filtered in before_send"
);
}

#[test]
fn keeps_skills_install_fetch_500() {
let event = event_with_tags(&[
("domain", "skills"),
("operation", "install_fetch"),
("failure", "non_2xx"),
("status", "500"),
]);
assert_eq!(
count_captured(vec![event]),
1,
"skill install server failures must still reach Sentry"
);
}

#[test]
fn drops_integrations_transient_transport_timeout() {
let event = event_with_tags_and_message(
Expand Down
Loading