From 4347e579c1725e4f9450f96fd022177c9019a62e Mon Sep 17 00:00:00 2001 From: oxoxDev <164490987+oxoxDev@users.noreply.github.com> Date: Mon, 22 Jun 2026 23:38:55 +0530 Subject: [PATCH] fix(inference): classify list_models 404 at source + actionable hint (TAURI-RUST-8X3) (#3769) Co-authored-by: Steven Enamakel --- .../settings/panels/TaskSourcesPanel.test.tsx | 8 +- src/core/observability.rs | 82 ++++++++++++++++++- src/openhuman/inference/ops.rs | 26 ++++++ .../inference/provider/ops/models.rs | 15 ++++ 4 files changed, 129 insertions(+), 2 deletions(-) diff --git a/app/src/components/settings/panels/TaskSourcesPanel.test.tsx b/app/src/components/settings/panels/TaskSourcesPanel.test.tsx index c8cf81a50..b924f7349 100644 --- a/app/src/components/settings/panels/TaskSourcesPanel.test.tsx +++ b/app/src/components/settings/panels/TaskSourcesPanel.test.tsx @@ -289,9 +289,15 @@ describe('', () => { it('clicking Refresh calls load() (line 586)', async () => { renderPanel(); await screen.findByTestId('task-source-s-1'); + // The Refresh button early-returns inside load() while a load is in flight + // (and is disabled in the UI for the same reason). Wait for the mount load + // to finish — i.e. the button to become enabled — before clicking, mirroring + // what a real user can do and removing the mount-vs-click race. + const refreshButton = screen.getByRole('button', { name: /Refresh/i }); + await waitFor(() => expect(refreshButton).toBeEnabled()); // The first listMock call happens on mount; clicking Refresh triggers another const beforeCount = listMock.mock.calls.length; - fireEvent.click(screen.getByRole('button', { name: /Refresh/i })); + fireEvent.click(refreshButton); await waitFor(() => { expect(listMock.mock.calls.length).toBeGreaterThan(beforeCount); }); diff --git a/src/core/observability.rs b/src/core/observability.rs index 7214d00b6..0758f48d0 100644 --- a/src/core/observability.rs +++ b/src/core/observability.rs @@ -1265,7 +1265,28 @@ fn is_provider_user_state_message(lower: &str) -> bool { // No `inference/provider/ops.rs::list_models` other than this site emits // the `provider returned NNN` prefix (verified via grep), so the prefix // alone is a sufficient anchor. - if lower.starts_with("provider returned 404") { + // + // TAURI-RUST-8X3: anchor to the position where `provider returned 404` is + // the formatted *error prefix* — never to any occurrence in the response + // body. The primary fix classifies the *raw* error at the source + // (`inference/ops.rs::inference_list_models`) before any log prefix is + // applied, so the raw shape always starts with `provider returned 404:`. + // The one historically-observed prefixed re-report path is the + // `inference/ops.rs` `error!("[inference::ops] list_models:error: {err}")` + // log line — handled below as an explicit prefixed shape. + // + // A bare `contains` would mis-fire: a genuine 400/500 list-models failure + // formats as `provider returned 500: `, and if `` merely + // relays an upstream phrase like `upstream provider returned 404 ...`, the + // loose substring would demote that real 4xx/5xx defect out of Sentry — + // exactly the failures the discrimination guard + // (`does_not_classify_non_404_list_models_failures_as_user_state`) says + // must still escalate. So we require the anchor to be the prefix, not buried + // text. (Mirrors the parenthesised `(401` anchoring in + // `is_session_expired_message`.) + if lower.starts_with("provider returned 404") + || lower.contains("list_models:error: provider returned 404") + { return true; } @@ -4390,6 +4411,55 @@ mod tests { } } + #[test] + fn couples_list_models_404_source_shape_to_classifier() { + // TAURI-RUST-8X3 coupling guard. Ties the TYPED SOURCE error shape + // emitted by `inference/provider/ops/models.rs` (the + // `provider returned 404: ` format) to the classifier, so a + // wording / prefix drift fails CI instead of silently leaking events. + // + // The wild Sentry message carried the `inference/ops.rs` + // `error!("[inference::ops] list_models:error: {err}")` PREFIX. The + // primary fix classifies the raw `err` at the source before that + // prefix is applied, but the `contains` widening above must ALSO + // catch the prefixed variant for any future prefixed re-report path. + // Assert BOTH the raw source shape and the prefixed log-line shape. + + // (a) Raw source shape — exactly what `models.rs` returns for a Go + // default-handler 404 (`404 page not found`). + let raw_source = "provider returned 404: 404 page not found"; + assert_eq!( + expected_error_kind(raw_source), + Some(ExpectedErrorKind::ProviderUserState), + "raw list_models 404 source shape must classify as ProviderUserState" + ); + + // (b) Raw source shape WITH the actionable hint appended by + // `models.rs` for the 404 case — the prefix anchor must survive + // the suffix. + let raw_with_hint = "provider returned 404: 404 page not found — the configured base URL does not expose a `/models` endpoint; check the provider's base URL (it usually ends in `/v1`)"; + assert_eq!( + expected_error_kind(raw_with_hint), + Some(ExpectedErrorKind::ProviderUserState), + "list_models 404 + actionable hint must still classify as ProviderUserState" + ); + + // (c) Prefixed log-line shape — the exact pattern from + // `inference/ops.rs::inference_list_models` `error!`. The explicit + // `list_models:error: provider returned 404` anchor must catch this + // even though it does not start with `provider returned 404`. The + // anchor is the formatted prefix, not a bare `404` substring, so it + // does NOT mis-fire on a 500 whose body merely relays an upstream + // 404 (see `does_not_classify_non_404_list_models_failures_as_user_state`). + let prefixed = + "[inference::ops] list_models:error: provider returned 404: 404 page not found"; + assert_eq!( + expected_error_kind(prefixed), + Some(ExpectedErrorKind::ProviderUserState), + "prefixed list_models 404 log line must still classify as ProviderUserState (anchored prefix)" + ); + } + #[test] fn does_not_classify_non_404_list_models_failures_as_user_state() { // Discrimination guard: only the 404 prefix demotes. Sibling 4xx / @@ -4411,6 +4481,16 @@ mod tests { r#"provider returned 503: upstream temporarily unavailable"#, // 500 — a real upstream bug; must reach Sentry. r#"provider returned 500: {"error":"internal_server_error"}"#, + // TAURI-RUST-8X3 false-negative guard: a genuine 4xx/5xx whose + // *body* merely relays an upstream 404 phrase. The actual status + // is 500/400, so this is a real failure that MUST reach Sentry — + // the classifier anchors on the `provider returned 404:` prefix, + // not on any `404` occurrence in the body, so these must NOT demote. + r#"provider returned 500: {"error":"upstream provider returned 404 page not found"}"#, + r#"provider returned 400: gateway error — upstream provider returned 404"#, + // Prefixed log-line variant of the same trap: the genuine status is + // 500, the buried `... 404` is body text. + "[inference::ops] list_models:error: provider returned 500: upstream provider returned 404 not found", ] { assert_ne!( expected_error_kind(raw), diff --git a/src/openhuman/inference/ops.rs b/src/openhuman/inference/ops.rs index 6e99a3669..58cdcfb74 100644 --- a/src/openhuman/inference/ops.rs +++ b/src/openhuman/inference/ops.rs @@ -295,6 +295,32 @@ pub async fn inference_list_models(provider_id: &str) -> Result/models` probe means the + // configured base URL does not host an OpenAI-compatible `/models` + // listing (wrong base, a model-only proxy, or a missing `/v1` + // suffix). Append an actionable hint so the model-dropdown probe + // surfaces *recovery guidance* inline instead of the bare Go-style + // `404 page not found`. The `provider returned 404` prefix is kept + // verbatim so the `is_provider_user_state_message` classifier anchor + // (which demotes this preventable user-state case out of Sentry) + // still matches — see `src/core/observability.rs`. + if status.as_u16() == 404 { + return Err(format!( + "provider returned 404: {} — the configured base URL does not expose a `/models` endpoint; check the provider's base URL (it usually ends in `/v1`)", + truncated + )); + } return Err(format!( "provider returned {}: {}", status.as_u16(),