fix(jsonrpc): narrow SessionExpired to backend-boundary signal (#2286)

[obchain]

Summary

• Tag real OpenHuman backend 401s at the API boundary in src/api/rest.rs with a SESSION_EXPIRED: sentinel prefix.
• Narrow is_session_expired_error in src/core/jsonrpc.rs to match only the sentinel and the two literal local guards ("no backend session token", "session jwt required").
• Drop the generic "401 + unauthorized" / "invalid token" match that signed users out on downstream / integration / BYO-key 401s (Discord card-open, BYO-key mis-config, Lark channel 401, MCP server 401, …).
• Annotate the SessionExpired publish log line with a match marker so future debugging tells which boundary fired.
• 5 new Rust tests across jsonrpc_tests.rs (BYO-key, integration, channel, sentinel, generic-401 negative) and rest_tests.rs (401 sentinel emission + non-401 negative).

Problem

is_session_expired_error (src/core/jsonrpc.rs:230) used .contains("401") && .contains("unauthorized") plus .contains("invalid token"). Any RPC path that bubbled a 401 from a downstream provider or integration matched and published DomainEvent::SessionExpired; the credentials subscriber then cleared the local OpenHuman token and bounced the user back to Welcome / Google sign-in. A single mis-configured BYO API key, a Discord card-open status fetch, or an MCP server token refresh would nuke the entire app session. asura.zhou (May 16) and Claudy (Discord card click) both hit this cascade.

Solution

Two-step:

1. Tag at the boundary. src/api/rest.rs is the single place every legitimate OpenHuman backend call routes through (effective_backend_api_url → authed_json). When the response status is 401, bail with "SESSION_EXPIRED: {method} {path} failed (401): {body}" and skip the Sentry report (the SessionExpired subscriber owns the teardown — a signed-out user is an expected state). Other 401-emitting sites (inference/provider/ops.rs, mcp_client/client.rs, channels/providers/lark.rs, http_host/auth.rs) keep their existing error shapes; they no longer match the classifier.
2. Narrow the classifier. is_session_expired_error now matches only:
   • SESSION_EXPIRED — authoritative boundary marker (also already emitted by openhuman/inference/provider/openhuman_backend.rs:50 for the no-session inference path).
   • "no backend session token" — local guard for the missing-profile case.
   • "session jwt required" — local guard for the no-JWT-in-store case.

Also added a matched=<marker> field to the SessionExpired publish log so future debugging can tell at a glance whether the signal came from the backend sentinel or one of the local guards.

Submission Checklist

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy — 5 new cases covering BYO-key, integration / channel, sentinel, generic-401 negative, and rest-side 401 sentinel emission + non-401 negative.
• Diff coverage ≥ 80% — every changed branch in the narrowed classifier and the rest-side 401 handler is exercised by a dedicated test.
• Coverage matrix updated — N/A: behaviour-only fix; no new feature row needed.
• All affected feature IDs from the matrix are listed in the PR description under ## Related — N/A: no matrix row affected.
• No new external network dependencies introduced — Rust-only change behind the existing backend client.
• Manual smoke checklist updated if this touches release-cut surfaces — N/A: not on the release-cut surface list.
• Linked issue closed via Closes #NNN in the ## Related section.

Impact

• Runtime/platform: desktop core (mac/win/linux) + standalone CLI. No frontend changes.
• Performance: zero — classifier is now strictly cheaper (no compound .contains() chain).
• Security: tighter authorisation handling — sign-out signal is now restricted to a single boundary, fewer paths can clear the local session.
• Migration / compatibility: behaviour change — a 401 from a non-backend source (BYO-key, integration, channel status, MCP server) no longer logs the user out. Real OpenHuman backend 401s still trigger the full sign-out cascade via the sentinel. The SESSION_EXPIRED sentinel in error strings is internal to the dispatch classifier and not part of any public API.

Related

• Closes: SessionExpired clears the session for unrelated backend 401s #2286
• Follow-up PR(s)/TODOs: When the Discord card-open repro from Discord channel card click logs the user out #2285 is verified against this change, that issue can be closed as a duplicate / fixed-by-this.

---

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

Linear Issue

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

Commit & Branch

• Branch: fix/2286-narrow-session-expired
• Commit SHA: b0728d5

Validation Run

• pnpm --filter openhuman-app format:check — N/A: no frontend changes.
• pnpm typecheck — N/A: no TypeScript changes.
• Focused tests: cargo test --lib is_session_expired (9/9 pass); cargo test --lib authed_json (4/4 pass); cargo test --lib -- jsonrpc rest (113/114, 1 ignored — pre-existing).
• Rust fmt/check (if changed): cargo fmt --all --check clean; cargo check clean.
• Tauri fmt/check (if changed): N/A — no Tauri changes.

Validation Blocked

• command: N/A
• error: N/A
• impact: N/A

Behavior Changes

• Intended behavior change: only real OpenHuman backend 401s (and the two literal local auth-boundary guards) clear the user session. Downstream provider / integration / channel / MCP 401s no longer sign the user out.
• User-visible effect: BYO-key mis-config, Discord card-open 401, Lark channel auth refresh, MCP server token expiry, integration auth failures all stay in-app as recoverable errors instead of bouncing the user to sign-in.

Parity Contract

• Legacy behavior preserved: real backend session expiry still publishes SessionExpired and clears the local token (via the new sentinel path). The openhuman_backend.rs no-session inference path continues to match (already used the sentinel pre-SessionExpired clears the session for unrelated backend 401s #2286).
• Guard/fallback/dispatch parity checks: the SessionExpired subscriber's teardown contract is unchanged; only the upstream classifier is narrowed.

Duplicate / Superseded PR Handling

• Duplicate PR(s): None.
• Canonical PR: This PR.
• Resolution (closed/superseded/updated): N/A.

Summary by CodeRabbit

Release Notes

• Bug Fixes

  • Refined session expiration detection to accurately distinguish between platform session timeouts and external authentication failures, reducing false positive session cleanups.
  • Enhanced error messages and logging for authentication issues to provide better diagnostics and observability.

• Tests

  • Added integration tests verifying correct session expiration detection and error classification across various authentication failure scenarios.

[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: 9e6e50c4-ffc8-40f1-b2ac-ea22b7912f63

📥 Commits

Reviewing files that changed from the base of the PR and between 65d92bf and b0728d5.

📒 Files selected for processing (4)

• src/api/rest.rs
• src/api/rest_tests.rs
• src/core/jsonrpc.rs
• src/core/jsonrpc_tests.rs

---

📝 Walkthrough

Walkthrough

The PR narrows SessionExpired classification to prevent downstream provider and integration 401 responses from clearing the user's OpenHuman session. Backend 401s at the OAuth boundary are now explicitly marked with a SESSION_EXPIRED: sentinel, and the detector is tightened to recognize only that sentinel and specific local authentication guards.

Changes

Session-expiration narrowing

Layer / File(s)	Summary
Backend 401 sentinel marking src/api/rest.rs, src/api/rest_tests.rs	BackendOAuthClient::authed_json logs a dedicated "session_expired" event for 401 responses and throws errors prefixed with SESSION_EXPIRED:. Tests verify that 401s receive the sentinel and non-401s do not.
Session-expiry detector narrowing src/core/jsonrpc.rs, src/core/jsonrpc_tests.rs	is_session_expired_error is tightened to match only SESSION_EXPIRED: sentinel, no backend session token, and session jwt required messages—removing generic 401/unauthorized patterns. Observability is enhanced with finer-grained matched labels. Tests verify that generic 401 variants (BYO-key provider, integration, channel failures) do not trigger session expiry, while only the sentinel and specific local guards do.

Sequence Diagram

sequenceDiagram
  participant User as User / RPC Client
  participant JSONRPCHandler as invoke_method
  participant BackendClient as authed_json
  participant BackendAPI as Backend API
  
  User->>JSONRPCHandler: call RPC method
  JSONRPCHandler->>BackendClient: invoke authed_json()
  BackendClient->>BackendAPI: HTTP request
  alt True OpenHuman session expired
    BackendAPI-->>BackendClient: 401 Unauthorized
    BackendClient->>BackendClient: log session_expired event
    BackendClient-->>JSONRPCHandler: error with SESSION_EXPIRED: prefix
    JSONRPCHandler->>JSONRPCHandler: is_session_expired_error matches sentinel
    JSONRPCHandler->>JSONRPCHandler: log method + matched=backend_sentinel
    JSONRPCHandler->>User: publish SessionExpired event
  else Downstream provider 401
    BackendAPI-->>BackendClient: 401 Unauthorized (provider auth)
    BackendClient-->>JSONRPCHandler: error without SESSION_EXPIRED
    JSONRPCHandler->>JSONRPCHandler: is_session_expired_error returns false
    JSONRPCHandler-->>User: return recoverable provider error
  end

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

• tinyhumansai/openhuman#2188: Adds observability classification tests that pin the exact SESSION_EXPIRED: sentinel string introduced in this PR.
• tinyhumansai/openhuman#2200: Updates ReliableProvider retry logic to treat SESSION_EXPIRED: errors as non-retryable, depending on this PR's session-expired signaling.
• tinyhumansai/openhuman#1719: Implements the observability classifier and Sentry before_send filter that detects the SESSION_EXPIRED: sentinel emitted by this PR.

Suggested labels

working

Suggested reviewers

• senamakel
• graycyrus

Poem

🐰 A session once too eager to depart,
Now checks the sentinel before it breaks the heart.
Backend says "I'm done," with SESSION_EXPIRED's call,
While distant providers' 401s need not end it all! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title directly references the main change: narrowing SessionExpired to only backend-boundary signals, which matches the PR's core objective to fix overly aggressive session expiry behavior.
Linked Issues check	✅ Passed	The PR comprehensively addresses all coding requirements from issue #2286: adds SESSION_EXPIRED sentinel at backend boundary [rest.rs], tightens is_session_expired_error classifier [jsonrpc.rs], adds matched= logging context, covers downstream 401 scenarios with tests [rest_tests.rs, jsonrpc_tests.rs], and prevents unrelated 401s from clearing the session.
Out of Scope Changes check	✅ Passed	All changes are scoped to the session-expiry narrowing objective: backend 401 tagging, classifier tightening, logging enhancement, and corresponding test coverage directly support issue #2286 requirements without unrelated modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

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

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

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

[obchain]

Superseded by #2356 which merged first and closes the same #2286. Different approach (HTTP-method-prefix heuristic vs. explicit SESSION_EXPIRED: sentinel from the API boundary) but equivalent coverage — Discord / BYO-key / Composio 401s no longer trigger session clear. Closing this one out.