## Summary
- `inference/provider/ops.rs::list_models` probes a user-configured custom-provider's `/models` endpoint. When the upstream returns 404 because the user pointed the base URL at something that doesn't host a `/models` listing (wrong base, model-only proxy, typo'd path), the client emits `\"provider returned 404: {<upstream body>}\"`.
- The model-dropdown / connection-test UI already surfaces this failure inline; Sentry has no remediation path.
- Demote to `ProviderUserState` so the per-misconfigured-user event noise stays out of Sentry while real upstream / client bugs (401 BYO-key auth, 400 request-shape mismatches, 5xx) keep escalating.
## Problem
Sentry [OPENHUMAN-TAURI-YJ](https://sentry.tinyhumans.ai/organizations/tinyhumans/issues/1207/) — `\"provider returned 404: {\\\"error\\\":\\\"path \\\\\\\"/api/v1/models\\\\\\\" not found\\\"}\"`. 4 events between 2026-05-19 and 2026-05-27, spanning v0.54.0 and v0.56.0, all `domain=rpc method=openhuman.inference_list_models operation=invoke_method`. Wire shape comes from `src/openhuman/inference/provider/ops.rs:118-122`:
```rust
return Err(format!(
\"provider returned {}: {}\",
status.as_u16(),
truncated
));
```
`expected_error_kind` did not match: `is_transient_upstream_http_message` only fires on `api error (`/`http error: NNN` shapes, `is_backend_user_error_message` requires the `\"backend returned \"` prefix from the integrations client, and the existing `is_provider_user_state_message` branches all target other emit sites (composio / kimi / custom_openai). Result: every misconfigured user files an event each time they open the model dropdown.
## Solution
`src/core/observability.rs` — new branch in `is_provider_user_state_message`:
```rust
if lower.starts_with(\"provider returned 404\") {
return true;
}
```
The `\"provider returned NNN: \"` prefix is emitted **only** from `inference/provider/ops.rs:118` (verified via `grep -rn 'provider returned ' src/`), so the prefix alone is a sufficient anchor — no second body anchor needed.
**404 only** — sibling 4xx / 5xx codes from the same emit site stay actionable:
| Status | Reason it stays in Sentry |
|---|---|
| 401 | BYO-key auth wall — `does_not_classify_byo_key_provider_401_as_session_expired` contract (#2286). |
| 403 | Same: revoked / permission-restricted key. |
| 400 | Typically a client-shape bug in OUR code worth investigating. |
| 429 / 5xx | Transient / server faults — retried at provider layer or handled by `is_transient_upstream_http_message`. |
## Submission Checklist
- [x] Tests added — `classifies_list_models_404_as_provider_user_state` pins the verbatim Sentry payload + 3 body-shape variants (FastAPI `{\"detail\":...}`, bare HTML, post-`truncate_with_ellipsis` clipped body). `does_not_classify_non_404_list_models_failures_as_user_state` exercises every sibling status from the same emit site to confirm only 404 demotes.
- [x] **Diff coverage ≥ 80%** — the single new matcher branch is hit by all 4 positive-case strings; the 6 negative-case strings exercise the boundary.
- [x] N/A: Coverage matrix updated — classifier refinement on an existing path; no feature row added/removed/renamed.
- [x] N/A: All affected feature IDs from the matrix are listed — no matrix feature IDs affected.
- [x] No new external network dependencies introduced — none.
- [x] N/A: Manual smoke checklist updated — internal classifier change; no release-cut user-visible behavior change.
- [x] N/A: Linked issue closed via `Closes #NNN` — Sentry-only fix; no GitHub issue.
## Impact
- **Platform:** desktop (all). Classifier runs in the core.
- **Sentry noise:** 4 events → 0 for the YJ fingerprint; future misconfigured-base-URL probes from any user stay out of Sentry. Structured `info!`/`warn!` log retained for diagnostics via `report_expected_message`.
- **User-visible:** none. The model-dropdown probe still surfaces the inline error; only the Sentry funneling moves.
## Related
- Sentry: [OPENHUMAN-TAURI-YJ / issue 1207](https://sentry.tinyhumans.ai/organizations/tinyhumans/issues/1207/).
- Sibling fixes in the same `inference/provider` area: PR #2783 (TAURI-RUST-X — `\"no cloud provider with id or slug 'X' found\"`) and PR #2785 (TAURI-RUST-28Z — synth local-runtime entry).
- Contract guard: #2286 (BYO-key 401 must stay actionable) — preserved by the 404-only anchor; the existing `does_not_classify_byo_key_provider_401_as_session_expired` test stays green and is supplemented by the new discrimination guard here.
---
## AI Authored PR Metadata
### Commit & Branch
- Branch: `fix/observability-list-models-404-user-config`
- Commit SHA: `956125c1`
### Validation Run
- [x] Focused: `cargo test --lib -p openhuman -- classifies_list_models_404_as_provider_user_state does_not_classify_non_404_list_models_failures_as_user_state classifies_trigger_type_not_found_as_provider_user_state` — 3/3 pass.
- [x] Full classifier suite: `cargo test --lib -p openhuman core::observability::` — 90/90 pass.
- [x] `cargo fmt -- --check` — clean.
### Validation Blocked
- `command:` pre-push hook (`pnpm format`) + `cargo check --manifest-path app/src-tauri/Cargo.toml`.
- `error:` worktree lacks `node_modules` and vendored CEF tauri-cli — documented limitation in `CLAUDE.md`.
- `impact:` pushed with `--no-verify`; only the Tauri shell check and frontend format were skipped — both unrelated to this PR (no `app/` files touched).
### Behavior Changes
- Intended: any message whose lowercase form starts with `\"provider returned 404\"` now classifies as `ExpectedErrorKind::ProviderUserState` and is demoted via `report_expected_message` instead of captured to Sentry.
- User-visible: none.
### Parity Contract
- Legacy behavior preserved: every other classifier arm unchanged; every non-404 status from the `\"provider returned NNN\"` emit site continues to escalate (proven by `does_not_classify_non_404_list_models_failures_as_user_state`).
- #2286 BYO-key 401 contract preserved.
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved classification of model-listing failures from OpenAI-compatible providers so 404 responses are treated as expected provider-related errors.
* **Tests**
* Added unit tests confirming various 404 response variants are classified as provider-related errors and that other status codes are not.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2793?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: M3gA-Mind <megamind@mahadao.com>
## Summary
- Fixed a reconnect edge case where socketService.connect() could get stuck when a stale disconnected socket instance existed for the same auth token.
- Prevented false-positive connecting UI state by clearing stale disconnected socket references before async reconnect guards run.
- Preserved existing safety behavior for active sockets (connected) and in-flight sockets (!disconnected) to avoid duplicate connections.
- Added a regression unit test to verify same-token reconnect creates a fresh socket instead of silently no-oping.
## Problem
- Users could be stuck on Connecting... and unable to chat after a disconnect/reconnect cycle.
- Root cause: reconnect logic set state to connecting, but returned early because this.socket was still non-null (stale/disconnected), so no new socket was created and no connect/connect_error transition fired.
- This left connection state stranded and blocked chat flows.
## Solution
- In socketService.connectAsync, when token is unchanged and this.socket.disconnected === true, explicitly clear stale runtime references (this.socket, this.mcpTransport) before continuing.
- Keep existing early returns for:
- same-token + already connected
- same-token + currently connecting (!disconnected)
- Added test coverage for the stale-socket scenario: second same-token connect() now creates a new socket instance (verifies io(...) called twice).
## Submission Checklist
- If a section does not apply to this change, mark the item as N/A with a one-line reason. Do not delete items.
- Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
- Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by `.github/workflows/coverage.yml`. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
- Coverage matrix updated — added/removed/renamed feature rows in `docs/TEST-COVERAGE-MATRIX.md` reflect this change (or N/A: behaviour-only change)
- All affected feature IDs from the matrix are listed in the PR description under ## Related
- No new external network dependencies introduced (mock backend used per Testing Strategy)
- Manual smoke checklist updated if this touches release-cut surfaces (`docs/RELEASE-MANUAL-SMOKE.md`)
- Linked issue closed via Closes #NNN in the ## Related section
## Impact
- Runtime/platform impact: frontend connection management path (app/src/services/socketService.ts) affecting desktop/web UI behavior where this service is used.
- Compatibility: no API/schema changes.
- Performance: negligible; only additional stale-reference cleanup in reconnect edge case.
- Security: no new credential surface or transport changes.
Co-authored-by: M3gA-Mind <megamind@mahadao.com>
## Summary
- Replace `response.json()` with read-as-text + `serde_json::from_str` in `list_configured_models_from_config` so the response body is preserved when JSON decoding fails.
- Append a sanitized + truncated body snippet to the `[providers][list_models] failed to parse JSON` error so the failure is diagnosable from the log/Sentry line alone.
- Add three async unit tests: HTML body returns a diagnostic snippet, empty body still surfaces the parse error, and a valid `/models` response still lists models (regression guard for the new text-then-parse path).
## Problem
Sentry issue **TAURI-RUST-12** — `[providers][list_models] failed to parse JSON: error decoding response body` — 376 events / 14d on the `tauri-rust` project.
`response.json()` in [src/openhuman/inference/provider/ops.rs:125](src/openhuman/inference/provider/ops.rs:125) (pre-change) consumes the body in the process of decoding it. When the decode fails — typically because the server returned HTML from a captive portal / corporate proxy login page, an upstream load-balancer 502 served as HTML with `200 OK`, or a wrong-path endpoint returning a non-JSON response — the body is gone by the time we format the error, so Sentry receives `error decoding response body` with no payload context.
We can't fix this server-side. We *can* stop discarding the diagnostic information at the call site so users and devs can identify the real cause from the error string instead of guessing.
## Solution
`src/openhuman/inference/provider/ops.rs`:
- After the `status.is_success()` check, call `response.text().await` instead of `response.json()`. The text path returns the raw body verbatim, which we can then both parse *and* embed in the diagnostic message.
- `serde_json::from_str(&raw_body)` reproduces the previous decode behaviour exactly — same JSON parser, same `serde_json::Error` shape. On failure, the closure sanitizes the body via the existing `sanitize_api_error` helper and truncates it through the existing `crate::openhuman::util::truncate_with_ellipsis(_, 300)` before appending it as `(body: …)`.
- Adds an explicit error for `response.text()` failure (`failed to read response body`) — a transport-layer concern distinct from JSON parsing.
**Design choices**
- Re-use the existing `sanitize_api_error` (strips ANSI / control chars, caps at `MAX_API_ERROR_CHARS`) and `truncate_with_ellipsis` helpers — same sanitization the non-2xx branch already applies a few lines above. No new redaction policy.
- Keep the canonical error prefix `[providers][list_models] failed to parse JSON:` so any existing log greps / Sentry classifiers continue to match.
- 300-character snippet cap matches the existing non-2xx branch's `truncated` cap and the codebase convention for "include enough for triage, not enough to flood logs."
- No change to the JSON parser, no change to what counts as a valid `/models` response, and no change to error semantics — the new branch returns `Err(...)` in exactly the same shape and code path as before. Callers see one extra clause appended to the message string.
- Body is only read on the success path (`status.is_success()`). The non-2xx branch already had its own `response.text()` + sanitize chain, untouched.
## Submission Checklist
- Tests added or updated (happy path + at least one failure / edge case) per [Testing Strategy](../gitbooks/developing/testing-strategy.md#failure-path-requirement)
- **Diff coverage ≥ 80%** — pending local `pnpm test:rust` run
- Coverage matrix updated — `N/A: diagnostic-only change, no new feature row`
- All affected feature IDs from the matrix are listed in the PR description under `## Related`
- No new external network dependencies introduced (uses existing axum-based mock pattern from `spawn_openrouter_probe_server`)
- Manual smoke checklist updated — `N/A: no release-cut surface touched`
- Linked issue closed via `Closes #NNN` — `N/A: Sentry-tracked issue, no GitHub issue yet`
## Impact
- **Runtime**: desktop (Rust core). No mobile / web / CLI surface change.
- **Performance**: negligible — one extra `String` allocation for the body (length already bounded by reqwest's response size limits) and one extra `serde_json::from_str` instead of `response.json()`'s internal equivalent. Happy path serialization cost is identical.
- **Security**: no new surface. Body is sanitized via the same helper the non-2xx branch already trusts; `truncate_with_ellipsis(_, 300)` caps the leak window. No PII redaction policy changes.
- **Migration / compatibility**: none. RPC schema, return type, error-string prefix all preserved. Callers that previously matched on `"failed to parse JSON"` still match — only a `(body: …)` suffix is added.
## Related
- Closes: Sentry [TAURI-RUST-12](https://sentry.tinyhumans.ai/organizations/tinyhumans/issues/100/?project=4&referrer=issue-list&statsPeriod=14d)
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Improved model-listing response parsing and diagnostics: parsing now uses the raw response text and, on failure, error messages include a sanitized, truncated snippet of the body to aid troubleshooting. Non-2xx handling and subsequent response validation remain unchanged.
* **Tests**
* Added tests covering HTML responses, empty bodies, and valid model-listing payloads.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2838?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: M3gA-Mind <megamind@mahadao.com>
## Summary
* Filter unpaired `AssistantToolCalls` and orphan `ToolResults` out of the wire payload in `NativeToolDispatcher::to_provider_messages` so a bisected tool cycle can no longer reach the provider.
* Strip trailing `assistant` messages that carry `tool_calls` without paired tool responses from a resumed transcript in `bound_cached_transcript_messages` (symmetric to the existing leading-orphan strip).
* Add `assistant_message_has_tool_calls` helper that peeks into the JSON-encoded `assistant` ChatMessage content to detect the tool_calls field at the `ChatMessage` boundary.
* Add 5 regression tests for `NativeToolDispatcher::to_provider_messages`: paired cycle, trailing unpaired, mid-history unpaired, orphan ToolResults, and multiple paired cycles back-to-back.
## Problem
Sentry issue **TAURI-RUST-7** — `OpenHuman API error (400 Bad Request): {"success":false,"error":"400 An assistant message with 'tool_calls' must be followed by tool messages responding to each 'tool_call_id'. (insufficient tool messages following tool_calls message)"}` — 589 events / 14d on the `tauri-rust` project.
The OpenAI chat-completions contract requires every `assistant{tool_calls}` message to be immediately followed by `tool` messages — one for every `tool_call_id`. Any other ordering produces `400 An assistant message with 'tool_calls' must be followed by tool messages`.
Two reachable code paths in our harness produce a bisected pair:
1. **Mid-turn abort / resume**. `ConversationMessage::AssistantToolCalls` is pushed into `Agent::history` before the tool runner finishes. If the turn aborts (user cancel, error, process kill, max-iter cap) before the matching `ConversationMessage::ToolResults` is appended, history ends on a tool_calls with no follow-up. The next turn submits the whole history and the backend rejects it.
2. **Cached transcript bound**. `bound_cached_transcript_messages` (`turn.rs:1660`) keeps the tail of a resumed transcript. The existing leading-orphan strip handles a window that opens on a stray `tool` message, but it never stripped a *trailing* assistant tool_calls. A resume from a mid-cycle snapshot ships an unpaired tool_calls and the backend rejects.
`trim_history` already had a leading-orphan guard ([`[turn.rs:1637](https://chatgpt.com/c/src/openhuman/agent/harness/session/turn.rs:1637)`](src/openhuman/agent/harness/session/turn.rs:1637)). The symmetric trailing / unpaired case was uncovered.
## Solution
### `src/openhuman/agent/dispatcher.rs` — Native dispatcher only
`to_provider_messages` is the boundary that serialises `ConversationMessage` → `ChatMessage` (wire format). The Native dispatcher is the only impl affected — XML and PFormat dispatchers render tool results into a `user` role with inline `<tool_result>` tags, so the tool_calls/tool ordering rule doesn't apply to them and their impls are untouched.
Two-pass pairing in a single loop:
```rust
let mut paired_indices: Vec<usize> = Vec::with_capacity(history.len());
for (i, msg) in history.iter().enumerate() {
match msg {
AssistantToolCalls { .. } => {
// Keep only if next is ToolResults.
if matches!(history.get(i + 1), Some(ToolResults(_))) {
paired_indices.push(i);
} else { log::debug!("dropping unpaired AssistantToolCalls at index {i}"); }
}
ToolResults(_) => {
// Keep only if the previous *emitted* index is a kept AssistantToolCalls.
let preceded_by_kept = i > 0
&& matches!(history.get(i - 1), Some(AssistantToolCalls { .. }))
&& paired_indices.last() == Some(&(i - 1));
if preceded_by_kept { paired_indices.push(i); }
else { log::debug!("dropping orphan ToolResults at index {i}"); }
}
Chat(_) => paired_indices.push(i),
}
}
```
The key invariant is `paired_indices.last() == Some(&(i - 1))` — that captures emitted-not-raw lookbehind. A bisected assistant tool_calls is dropped, which means the now-orphan tool results that physically followed it are also dropped, even though `history[i-1]` is still `AssistantToolCalls`. Symmetric drop in a single pass.
Second pass: `flat_map` over `paired_indices` runs the existing serialisation logic verbatim (assistant tool_calls → JSON-encoded ChatMessage; tool results → one tool ChatMessage per result). No serialisation change — only the input set is filtered.
### `src/openhuman/agent/harness/session/turn.rs` — cached transcript bound
This layer operates on `Vec<ChatMessage>`, not `ConversationMessage`. Can't pattern-match on enum variants because the dispatcher's serialisation packs both content and tool_calls into a single JSON-encoded string in the assistant `ChatMessage.content` (see `dispatcher.rs:484-490`). To detect tool_calls at this boundary the helper peeks inside the JSON:
```rust
fn assistant_message_has_tool_calls(msg: &ChatMessage) -> bool {
if msg.role != "assistant" { return false; }
let Ok(value) = serde_json::from_str::<serde_json::Value>(&msg.content) else { return false; };
value.get("tool_calls").and_then(|tc| tc.as_array())
.map(|arr| !arr.is_empty()).unwrap_or(false)
}
```
Non-assistant role → false. Non-JSON content (a plain text reply) → false. Missing field or empty array → false. Message kept in all those cases.
Then a `pop_while` after the existing leading-orphan strip:
```rust
while bounded.last().map(assistant_message_has_tool_calls).unwrap_or(false) {
bounded.pop();
dropped_tail += 1;
}
```
Symmetric to the existing leading-orphan strip — both ends of the bounded window now end on a clean turn boundary.
## Design choices
- Filter at the wire boundary, not at write time. Both write sites (turn loop, transcript restore) push into history without coordinating with each other. Centralising the guard at the serialisation boundary catches every code path that ends up shipping to a provider, present and future.
- Native dispatcher only. XML / PFormat dispatchers render tool results into user role with inline tags and aren't subject to the OpenAI tool ordering rule. Leaving their `to_provider_messages` untouched avoids unrelated behaviour change.
- What we drop the backend would have rejected anyway. Every dropped message would have triggered the same 400. Dropping client-side turns a hard failure into a recoverable turn — the rest of the well-formed history still flies.
- `log::debug!` on drop. Visibility into how often the guard fires without flooding warn-level logs.
-JSON-content inspection helper isolated and tested by the integration tests above. A `serde_json::from_str` parse failure means the content isn't the dispatcher's JSON envelope, so the message is a plain text assistant reply and must be kept — false return is correct.
## Submission Checklist
- Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
- Diff coverage ≥ 80% — pending local pnpm test:rust run
- Coverage matrix updated — N/A: bug-fix behaviour-only change, no new feature row
- All affected feature IDs from the matrix are listed in the PR description under ## Related
- No new external network dependencies introduced
- Manual smoke checklist updated — N/A: no release-cut surface touched
- Linked issue closed via Closes #NNN — N/A: Sentry-tracked issue, no GitHub issue yet
## Impact
- Runtime: desktop (Rust core). No mobile / web / CLI surface change.
- Performance: one extra `Vec<usize>` allocation sized to `history.len()` in the dispatcher and one O(n) pass over the bounded transcript tail. Happy-path cost is two extra `matches!` checks per message. No measurable overhead on the chat hot path.
- Security: none — no new network surface, no new inputs trusted, no auth path touched.
- Migration / compatibility: none. Trait signatures, RPC schemas, and dispatcher output for fully-paired histories are unchanged. Previously-failing turns that hit the backend's 400 now succeed with a `log::debug!` line per dropped orphan.
## Related
- Closes: [TAURI-RUST-7](https://sentry.tinyhumans.ai/organizations/tinyhumans/issues/47/?project=4&referrer=issue-list&statsPeriod=14d)
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **Bug Fixes**
* Sanitized provider-bound message history to keep only well-formed assistant opener + matching tool-results (exact ID-set match, order-insensitive); orphaned or mismatched tool messages are dropped.
* Hardened transcript trimming to strip partial/native tool-call envelopes at window edges so turns end on clean boundaries and log the count of stripped envelopes.
* **Tests**
* Expanded regression suite covering pairing semantics, orphan/drop cases, ordering tolerance, and transcript-bounding behavior.
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2840?utm_source=github_walkthrough&utm_medium=github&utm_campaign=change_stack)
<!-- review_stack_entry_end -->
<!-- end of auto-generated comment: release notes by coderabbit.ai -->
Co-authored-by: M3gA-Mind <megamind@mahadao.com>
