## 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>
## Summary
- Added an on-device multilingual PII redactor covering 15 identifier types — Brazilian CPF/CNPJ (mod-11), Argentine CUIT, Mexican RFC, Japanese マイナンバー, US SSN with reserved-range filter, Luhn-validated credit cards, mod-97 IBAN, Verhoeff-validated Aadhaar, Indian PAN, UK NINO, Spanish DNI/NIE, Korean RRN, E.164 and NANP phones — and wired it into the existing `sanitize_text` pipeline that runs on every memory write.
- Added a Unicode-normalization pre-pass that strips zero-width characters and folds fullwidth/Arabic-Indic digits + punctuation, defeating common regex-bypass tricks before matching while preserving non-PII bytes in the original text.
- Added `has_likely_pii()` and wired it into the namespace/key boundary checks in `documents.rs` and `kv.rs` so PII-bearing inputs are rejected outright, mirroring how `has_likely_secret()` is enforced today.
- Extended `SanitizationReport` with a `pii_redactions` counter and surfaced it in the existing `[memory:safety]` audit-log lines across `documents.rs`, `kv.rs`, and `fts5.rs`.
## Problem
- OpenHuman ingests data from 118+ integrations into Memory Tree. The existing `memory::safety` module redacted API keys, tokens, and PEM blocks but had no coverage for personal PII — national IDs, financial identifiers, phone numbers.
- Issue #2017 proposed sending raw content to a third-party HTTP endpoint (`api.trustboost.dev`) for "sanitization". That approach directly contradicts OpenHuman's privacy-first, on-device posture — it would exfiltrate the exact PII it claims to protect to an unaffiliated vendor.
- A naive regex implementation would still leave two real gaps: (a) the multilingual identifier formats that motivated the original issue (LATAM, JP, IN, EU, KR) and (b) trivial bypass via fullwidth-digit or zero-width-character obfuscation, which any motivated attacker (or accidentally-pasted Japanese-locale data) will trigger.
## Solution
- Built `src/openhuman/memory/safety/pii.rs` with 15 PII categories. Where checksums exist (CPF/CNPJ mod-11, CUIT, credit-card Luhn, IBAN mod-97, Aadhaar Verhoeff, Spanish DNI/NIE check-letter, SSN reserved-range), false-positives are rejected at the algorithm level — no LLM, no network. Where checksums don't exist (RFC, PAN-IN, NANP, E.164, RRN), structural format rules carry the discrimination.
- Added a `NormalizedView` that strips U+200B/200C/200D/FEFF/2060/180E and folds fullwidth (`０-９`, `．－／：`) plus Arabic-Indic / Eastern Arabic-Indic digits to ASCII before matching. Match offsets are mapped back to the original byte positions so only PII bytes are replaced — surrounding text (including any intentional fullwidth glyphs) is byte-identical to input.
- Patterns run in priority order (formatted before bare, IBAN before credit-card, etc.) with overlap-deduplication so a single span can't be redacted twice or partially counted as multiple types. A `RegexSet` pre-filter short-circuits PII-free text in one scan instead of ~18 per-pattern scans.
- `has_likely_pii()` mirrors `has_likely_secret()` and is wired into the same boundary checks in `unified/documents.rs` (both `upsert_document` and `upsert_document_metadata_only`) and `unified/kv.rs` (both `kv_set_global` and `kv_set_namespace`).
- Added 37 new tests in `pii.rs` and 2 integration tests in `safety/mod.rs`: positive + negative per pattern, checksum-failing rejection cases, Unicode/zero-width bypass attempts, `has_likely_pii` gating, and an aggressive mixed-language end-to-end test covering 13 PII types in one document. Full safety suite: 53 tests passing. Full memory module: 1007 tests passing, zero regressions.
## 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.
- [x] Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy — 39 new tests; checksum-failing and bypass-attempt negatives included
- [x] Diff coverage ≥ 80% — new file is ~100% line-covered by inline tests; integration sites have direct assertions in `safety/mod.rs::tests`
- [x] All affected feature IDs from the matrix listed in ## Related — N/A no existing matrix IDs cover memory safety redaction; the new row above will be the first
- [x] No new external network dependencies introduced — deliberately on-device; no new crates; `regex` and `once_cell` already in tree
- [x] Manual smoke checklist updated — N/A not a release-cut UI surface
- [x] Linked issue closed via Closes #NNN — see ## Related
## Impact
- Runtime/platform impact: Rust core memory ingestion path only. Desktop app behavior change is two-fold for users — (1) memory writes now produce additional `[REDACTED_PII_*]` tokens in stored content when format-matching PII is present, (2) a new error return (`document/kv namespace/key cannot contain personal identifiers`) on the rare case where a caller tries to use a PII-shaped string as a namespace or key.
- Performance: `RegexSet` pre-filter makes PII-free text a single-scan no-op. On text containing PII, adds one normalized-string allocation plus a handful of regex scans gated by the screen — negligible compared to the embedding/SQLite/markdown-sidecar costs already on the write path. No measurable impact on ingestion latency in local testing.
- Security/migration/compatibility: no schema changes, no new dependencies. The boundary-gate rejection is a behavior change for any caller that previously stored namespace/keys *containing* identifier-shaped strings; expected impact is zero in practice because real namespace/keys are paths like `memory/global/preferences`. Privacy posture is strictly improved — every byte stays on device; no telemetry, no outbound calls.
## Related
- Closes: #2017
<!-- This is an auto-generated comment: release notes by coderabbit.ai -->
## Summary by CodeRabbit
* **New Features**
* Personal information detection and redaction integrated into the memory system
* Write operations on namespace and key fields now validate against personal identifiers
* **Improvements**
* Enhanced sanitization reports with additional metrics on personal identifier redactions
<!-- review_stack_entry_start -->
[](https://app.coderabbit.ai/change-stack/tinyhumansai/openhuman/pull/2310?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: Shanu <shanu@tinyhumans.ai>
Co-authored-by: Steven Enamakel <enamakel@tinyhumans.ai>
