fix(observability): classify OpenHuman embedding 401 'Invalid token' as SessionExpired (TAURI-RUST-4K5)

[CodeGhost21]

Summary

• Fixes a self-contradicting classification on main: the embedding-401 wire shape Embedding API error (401 …): {"error":"Invalid token"} was asserted to be two different buckets by two merged PRs, leaving the Rust test suite red on main (and therefore on every open PR).
• expected_error_kind now runs is_session_expired_message before is_embedding_backend_auth_failure, so the OpenHuman-backend embedding 401 "Invalid token" envelope classifies as SessionExpired (recoverable re-auth signal) instead of BackendUserError.
• Amends fix(observability): demote expected-error Sentry buckets across embeddings, provider, memory-store, FS, and thinking-mode wire shapes #2830's classifies_embedding_backend_auth_failure test to drop the parenthesised case now owned by the session matcher; the bare-status shape still classifies as BackendUserError.

Problem

PR #2786 (TAURI-RUST-4K5, current main tip 14ff92b2) added an is_session_expired_message arm + the test classifies_embedding_api_invalid_token_401_as_session_expired, asserting that Embedding API error (401 Unauthorized): {"success":false,"error":"Invalid token"} → SessionExpired.

The older PR #2830 (TAURI-RUST-T) had already added is_embedding_backend_auth_failure, which matches the identical string and returns BackendUserError, and runs earlier in expected_error_kind. Its test classifies_embedding_backend_auth_failure asserts BackendUserError for that same string.

The two are mutually exclusive. Because the embedding matcher ran first, #2786's own test fails on main:

core::observability::tests::classifies_embedding_api_invalid_token_401_as_session_expired
  left: Some(BackendUserError)
 right: Some(SessionExpired)

This turns the Rust CI job red on main and on every PR that merges current main.

Solution

• Move the is_session_expired_message check to immediately after is_backend_user_error_message and before is_embedding_backend_auth_failure in expected_error_kind. is_backend_user_error_message only matches the literal "backend returned <status>" substring (which this wire shape lacks), so the new placement is the earliest point that can intercept the embedding 401 envelope.
• The session matcher is narrowly anchored: it requires the parenthesised (401 prefix and the "error":"Invalid token" envelope. Therefore:
  • Parenthesised OpenHuman-backend embedding 401 "Invalid token" → SessionExpired (fixed).
  • Bare-status Embedding API error 401 … → still falls through to is_embedding_backend_auth_failure → BackendUserError (unchanged).
  • BYO-key embedding 401s (invalid_api_key, authentication_error, no "Invalid token" envelope) → still None/actionable in Sentry (unchanged; guarded by does_not_classify_embedding_byo_key_401_as_session_expired).
• Amend fix(observability): demote expected-error Sentry buckets across embeddings, provider, memory-store, FS, and thinking-mode wire shapes #2830's test to assert only the bare-status shape, with a comment documenting the new ownership boundary between the two matchers.

Design decision: SessionExpired is the more specific and more recent intent (#2786 deliberately covers 4P0 + 4K5 + 1EE with BYO-key guards), and a "401 Invalid token" from the OpenHuman backend genuinely is an expired user session that should drive re-auth rather than a generic backend error.

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 — amended classifies_embedding_backend_auth_failure; the SessionExpired happy path (classifies_embedding_api_invalid_token_401_as_session_expired) and the negative guards (does_not_classify_embedding_byo_key_401_as_session_expired, does_not_classify_unrelated_invalid_token_messages) now all pass.
• Diff coverage ≥ 80% — the only non-comment changed lines are the relocated is_session_expired_message branch (exercised by the 4K5 test) and the amended test body; all are covered.
• Coverage matrix updated — N/A: behaviour-only change (re-buckets one already-tracked error classification; no feature row added/removed/renamed).
• All affected feature IDs from the matrix are listed in the PR description under ## Related — N/A: no matrix feature IDs affected.
• No new external network dependencies introduced (mock backend used per Testing Strategy) — pure in-process classifier change.
• Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md) — N/A: observability classification only, no user-facing surface.
• Linked issue closed via Closes #NNN in the ## Related section — N/A: no tracked issue; this fixes a regression introduced by #2786 vs #2830 racing on main.

Impact

• Runtime/platform: desktop core (src/core/observability.rs) only; no UI, mobile, or CLI changes.
• Behavioural: OpenHuman-backend embedding 401 "Invalid token" now reports SessionExpired instead of BackendUserError. Both are Sentry-suppressed "expected" buckets, so Sentry volume is unchanged; the difference is the downstream re-auth signal.
• Restores green Rust CI on main and unblocks all open PRs that were failing on the pre-existing contradiction.

Related

• Closes: N/A
• Follow-up PR(s)/TODOs: surfaces the same root regression noticed while resolving conflicts on fix(observability): classify RPC filesystem path validation errors as expected (Sentry TAURI-RUST-4QH) #2795.

---

AI Authored PR Metadata (required for Codex/Linear PRs)

Keep this section for AI-authored PRs. For human-only PRs, mark each field N/A.

Linear Issue

• Key: N/A
• URL: N/A

Commit & Branch

• Branch: fix/observability-embedding-401-session-precedence
• Commit SHA: ed9c9704

Validation Run

• pnpm --filter openhuman-app format:check — N/A: Rust-only change, no app/ files touched
• pnpm typecheck — N/A: Rust-only change, no TypeScript touched
• Focused tests: cargo test --manifest-path Cargo.toml --lib core::observability → 118 passed, 0 failed
• Rust fmt/check (if changed): cargo fmt --check clean; lib compiles under cargo test
• Tauri fmt/check (if changed): N/A: app/src-tauri not touched

Validation Blocked

• command: pnpm test:coverage / pnpm typecheck
• error: worktree lacks node_modules (prettier: command not found); not installed for this Rust-only fix
• impact: none — change is confined to the Rust core classifier; Rust fmt + focused tests verified

Behavior Changes

• Intended behavior change: re-bucket the OpenHuman-backend embedding 401 "Invalid token" envelope from BackendUserError to SessionExpired.
• User-visible effect: none directly (both buckets skip Sentry); downstream consumers of SessionExpired can now treat the embedding 401 as a recoverable re-auth event.

Parity Contract

• Legacy behavior preserved: bare-status Embedding API error 401 … → BackendUserError; BYO-key embedding 401s → None/actionable; non-embedding session shapes (4P0 OpenHuman, 1EE streaming) unchanged.
• Guard/fallback/dispatch parity checks: does_not_classify_embedding_byo_key_401_as_session_expired and does_not_classify_unrelated_invalid_token_messages still pass.

Duplicate / Superseded PR Handling

• Duplicate PR(s): N/A
• Canonical PR: this PR
• Resolution (closed/superseded/updated): reconciles the conflicting matchers from fix(observability): classify OpenHuman/Embedding/streaming backend 'Invalid token' 401 as SessionExpired (TAURI-RUST-4P0 + 4K5 + 1EE) #2786 (SessionExpired) and fix(observability): demote expected-error Sentry buckets across embeddings, provider, memory-store, FS, and thinking-mode wire shapes #2830 (BackendUserError) in favour of fix(observability): classify OpenHuman/Embedding/streaming backend 'Invalid token' 401 as SessionExpired (TAURI-RUST-4P0 + 4K5 + 1EE) #2786's classification for the parenthesised envelope.

Summary by CodeRabbit

• Bug Fixes
  • Improved session expiration error handling. Session-expired messages are now correctly identified and reported with proper precedence, ensuring more accurate error detection and clearer feedback when sessions expire.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 140ecee8-f0a0-4558-8c23-9f13c45c6eeb

📥 Commits

Reviewing files that changed from the base of the PR and between e83bfd6 and ed9c970.

📒 Files selected for processing (1)

• src/core/observability.rs

---

📝 Walkthrough

Walkthrough

This PR fixes error classification precedence in src/core/observability.rs by moving session-expired checks earlier in the expected_error_kind dispatcher. Embedding 401 "Invalid token" responses now route to SessionExpired instead of the broader embedding-auth failure classifier, with unit tests updated to validate the new behavior.

Changes

Session-Expired Classification Precedence

Layer / File(s)	Summary
Session-expired error classification precedence src/core/observability.rs	The is_session_expired_message check is repositioned before the embedding-backend auth failure matcher, removing the duplicate later check; embedding 401 "Invalid token" envelopes now classify as SessionExpired instead of BackendUserError, validated by updated unit tests.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

• tinyhumansai/openhuman#2830: Added the embedding-backend auth failure matcher (BackendUserError); this PR changes precedence so the same 401 envelope is classified as SessionExpired instead.
• tinyhumansai/openhuman#2786: Expanded is_session_expired_message matcher for the embedding 401 envelope; this PR ensures the session-expired classification wins via precedence reordering.
• tinyhumansai/openhuman#2808: Modified expected_error_kind dispatcher matching logic by reordering and adding match handling for specific error classifications.

Suggested labels

rust-core, sentry-traced-bug, bug

Suggested reviewers

• oxoxDev
• graycyrus

Poem

🐰 A token expired, caught too late,
Now session checks run at the gate,
Before the broad auth matcher's call,
The right error kind gets routed, all!
Precedence fixed, tests smile true. 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and specifically summarizes the main change: fixing the classification of OpenHuman embedding 401 'Invalid token' errors as SessionExpired instead of BackendUserError, with a relevant issue reference.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

[graycyrus]

@CodeGhost21 hey! the code looks good to me, but CI is still pending so i'll hold off on approving until those checks clear.

The fix is correct and well-reasoned. Moving is_session_expired_message before is_embedding_backend_auth_failure is exactly the right call — the parenthesised (401 + "error":"Invalid token" envelope is unambiguously a session expiry signal, and the comment you added documenting the ownership boundary between the two matchers is exactly the kind of thing that prevents this regression from recurring.

The test amendment is accurate too — pulling the parenthesised case out of classifies_embedding_backend_auth_failure and leaving only the bare-status shape there correctly reflects what each matcher now owns.

One thing worth confirming once CI runs: is_session_expired_message uses the original-case message argument while the surrounding checks use &lower. That's carry-over from before the move, so it should be fine, but worth a quick sanity check that the session matcher's string comparisons are case-insensitive or otherwise correctly anchored.

Once the Rust CI jobs go green, i'll come back and approve this.

[M3gA-Mind]

Closing as superseded by #2869, which resolved the same TAURI-RUST-4K5 regression via a different approach: instead of moving is_session_expired_message before is_embedding_backend_auth_failure, #2869 changed is_embedding_backend_auth_failure to return SessionExpired directly for all embedding 401 'Invalid token' payloads. Both approaches restore green Rust CI; #2869 was merged first. Thank you for the thorough analysis and the detailed problem writeup — the PR description is genuinely excellent root-cause documentation.