fix(jsonrpc): keep scoped 401s from expiring session

[aqilaziz]

Summary

• Narrows JSON-RPC session-expiry detection to confirmed OpenHuman app-session shapes.
• Keeps generic downstream/provider 401 Unauthorized and invalid token errors from publishing SessionExpired.
• Adds diagnostic logging that includes the RPC method and sanitized reason when an auth-like error is not treated as session expiry.
• Updates classifier tests for true session expiry, scoped 401s, and generic invalid-token text.

Problem

• The previous JSON-RPC classifier treated any 401 + unauthorized text as a global app session expiry.
• That could clear the stored session and bounce users to sign-in for unrelated provider, integration, or channel errors.
• This is the root pattern behind the Discord card logout report.

Solution

• Reuse the strict observability::is_session_expired_message classifier at the JSON-RPC dispatch boundary.
• Preserve SessionExpired publication for explicit OpenHuman auth states: session expired, SESSION_EXPIRED, no backend session token, and session JWT required.
• Add a diagnostics-only predicate for generic auth-looking errors so they are logged with method context but do not clear the session.
• Update comments in observability to match the stricter dispatch behavior.

Submission Checklist

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
• Diff coverage ≥ 80% — local coverage run is blocked by missing libclang; CI coverage gate will verify changed lines.
• Coverage matrix updated — N/A: behavior-only core classifier hardening, 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 ID changed.
• 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) — N/A: no release smoke checklist surface changed.
• Linked issue closed via Closes #NNN in the ## Related section

Impact

• Runtime: core JSON-RPC auth/error handling.
• User-visible: scoped provider/integration 401s should remain recoverable errors instead of logging the user out.
• Security/privacy: session-expiry reasons are sanitized before logging/publishing.
• Compatibility: explicit OpenHuman session-expired sentinels still clear the session as before.

Related

• Closes SessionExpired clears the session for unrelated backend 401s #2286
• Refs Discord channel card click logs the user out #2285
• Follow-up PR(s)/TODOs: identify the exact Discord card-click RPC from user logs if the UI still needs a provider-specific error state.

---

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

Linear Issue

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

Commit & Branch

• Branch: codex/2286-session-expired-narrowing
• Commit SHA: 6f98f1e28a113afd0ea47908d0a397ecfe681560

Validation Run

• pnpm --filter openhuman-app format:check — N/A: no frontend changes.
• pnpm typecheck — N/A: no TypeScript changes.
• Focused tests: blocked locally; see Validation Blocked.
• Rust fmt/check (if changed): cargo fmt --check passed.
• Tauri fmt/check (if changed): N/A: no Tauri shell changes.

Validation Blocked

• command: cargo test --lib is_session_expired_error --manifest-path Cargo.toml
• error: whisper-rs-sys build script could not find clang.dll / libclang.dll; LIBCLANG_PATH is unset in this Windows environment.
• impact: focused Rust tests could not run locally, but the changed classifier tests are included for CI.

Behavior Changes

• Intended behavior change: generic provider/integration/channel 401 Unauthorized text no longer publishes DomainEvent::SessionExpired.
• User-visible effect: users should not be logged out by a scoped downstream 401, including the suspected Discord card-click path.

Parity Contract

• Legacy behavior preserved: explicit OpenHuman session expiry, uppercase SESSION_EXPIRED, missing backend session token, and missing session JWT still clear the app session.
• Guard/fallback/dispatch parity checks: classifier tests cover true session expiry, generic 401, invalid token, and local missing-session cases.

Duplicate / Superseded PR Handling

• Duplicate PR(s): none found for SessionExpired clears the session for unrelated backend 401s #2286/Discord channel card click logs the user out #2285 at PR creation time.
• Canonical PR: this PR.
• Resolution (closed/superseded/updated): N/A

[coderabbitai]

📝 Walkthrough

Walkthrough

JSON-RPC session auto-cleanup now publishes DomainEvent::SessionExpired only for confirmed OpenHuman session-auth failures, using a strict predicate from core::observability. Unconfirmed authorization errors log warnings and preserve the stored session, keeping downstream/provider 401s recoverable. A new is_unconfirmed_unauthorized_error helper distinguishes non-session-expired auth-looking responses. Tests verify the narrower classification.

Changes

Session-Expiry Classification and Dispatch

Layer / File(s)	Summary
Stricter session-expired classification rules and helpers src/core/observability.rs, src/core/jsonrpc.rs	Observability documentation clarifies that the strict "OpenHuman session expired" body-match predicate controls JSON-RPC dispatch-boundary behavior, preventing unrelated downstream/provider 401s from triggering session cleanup. New is_unconfirmed_unauthorized_error helper detects authorization-looking responses that do not match the strict predicate.
invoke_method session auto-cleanup refactor src/core/jsonrpc.rs	invoke_method publishes DomainEvent::SessionExpired only when an error strictly matches the session-expired predicate delegated from core::observability. Authorization-looking errors that do not match trigger a warning log and preserve the stored session.
Session-expiry classification test coverage src/core/jsonrpc_tests.rs	Tests verify that generic "401 Unauthorized" and "Invalid Token" variants no longer classify as session-expired, that is_unconfirmed_unauthorized_error matches those cases for diagnostics, and that only the OpenHuman "Session expired" 401 body matches the strict predicate.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tinyhumansai/openhuman#1763: Introduced the strict is_session_expired_message and ExpectedErrorKind::SessionExpired classification that this PR's JSON-RPC dispatch now delegates to.
• tinyhumansai/openhuman#2188: Adds regression test pins for the exact SESSION_EXPIRED: wire strings that the stricter predicate in src/core/observability.rs must recognize.
• tinyhumansai/openhuman#2200: Wires the same crate::core::observability::is_session_expired_message predicate into ReliableProvider retry logic, ensuring consistent session-expiry classification across dispatch boundaries.

Suggested labels

working

Suggested reviewers

• senamakel

Poem

🐰 A session's fate must be quite clear,
Not every 401 means the end is near.
Now strict and wise, we check with care,
Only true expiry clears the air.
Downstream 401s? Those live to fight another day, ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly summarizes the main change: narrowing session expiry detection to prevent scoped 401s from clearing the session.
Linked Issues check	✅ Passed	The PR addresses all coding-related requirements from #2286: narrows SessionExpired to confirmed OpenHuman sentinels, prevents generic 401s from clearing session, adds diagnostic logging, and includes regression-preventing tests.
Out of Scope Changes check	✅ Passed	All changes in jsonrpc.rs, jsonrpc_tests.rs, and observability.rs are directly scoped to session-expiry classification narrowing and testing per #2286 requirements.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ 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.}

[coderabbitai]

🧹 Nitpick comments (1)

src/core/jsonrpc.rs (1)

184-188: ⚡ Quick win

Use debug-level structured logs for diagnostics-only auth classification.

Line 184 and Line 199 are diagnostics branches; emitting them at warn can create avoidable noise. Prefer debug/trace with structured fields.

Proposed adjustment

-            log::warn!(
-                "[jsonrpc] confirmed session expiry for method '{}' — publishing SessionExpired: {}",
-                method,
-                sanitized_reason
-            );
+            log::debug!(
+                method = %method,
+                reason = %sanitized_reason,
+                "[jsonrpc] confirmed session expiry — publishing SessionExpired"
+            );

...
-            log::warn!(
-                "[jsonrpc] unauthorized error for method '{}' did not match OpenHuman session expiry — leaving session intact: {}",
-                method,
-                sanitized_reason
-            );
+            log::debug!(
+                method = %method,
+                reason = %sanitized_reason,
+                "[jsonrpc] unauthorized error did not match OpenHuman session expiry — leaving session intact"
+            );

As per coding guidelines: src/**/*.rs: Use log or tracing crate at debug or trace level for Rust diagnostic logs.

Also applies to: 199-203

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@src/core/jsonrpc.rs` around lines 184 - 188, Replace the two diagnostic
log::warn! calls (the "confirmed session expiry" branch that logs method and
sanitized_reason and the subsequent diagnostic branch around lines 199–203) with
debug- or trace-level logs and emit structured fields rather than interpolated
text; specifically change those log::warn! invocations to
log::debug!/log::trace! and include method and sanitized_reason as named fields
(e.g., method=..., reason=...) so diagnostics are only visible at debug/trace
level and are machine-parsable.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@src/core/jsonrpc.rs`:
- Around line 184-188: Replace the two diagnostic log::warn! calls (the
"confirmed session expiry" branch that logs method and sanitized_reason and the
subsequent diagnostic branch around lines 199–203) with debug- or trace-level
logs and emit structured fields rather than interpolated text; specifically
change those log::warn! invocations to log::debug!/log::trace! and include
method and sanitized_reason as named fields (e.g., method=..., reason=...) so
diagnostics are only visible at debug/trace level and are machine-parsable.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 4179fdf4-04a9-41c4-b1d7-1308e4cc509f

📥 Commits

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

📒 Files selected for processing (3)

• src/core/jsonrpc.rs
• src/core/jsonrpc_tests.rs
• src/core/observability.rs

[graycyrus]

Clean — moving to approval queue.

Well-targeted fix. The core change — delegating is_session_expired_error to the strict observability::is_session_expired_message classifier — is the right call. Generic downstream 401s (BYO-key, Composio, channel providers) no longer nuke the app session, while all four canonical OpenHuman session-expired wire shapes still trigger cleanup as before.

Good details:

• is_unconfirmed_unauthorized_error keeps diagnostic visibility for scoped 401s without feeding the SessionExpired publish path.
• sanitize_api_error is called once before branching — no duplication, no PII in logs.
• Tests cover the full classification matrix: true session expiry, generic 401, invalid token, and the OpenHuman API error body shape.
• All CI green, coverage gate passes.

Re issue alignment: #2286 acceptance criteria are met. The follow-up to identify the exact Discord card-click RPC (#2285) is correctly scoped as separate work — this PR eliminates the root cause regardless of which specific RPC triggers the downstream 401.

[senamakel]

Looks good, nice work!

[senamakel]

nice one @aqilaziz, love how this narrows the session-expiry check to actual openhuman shapes so random downstream 401s stop nuking sessions 🙌 the sanitized diagnostic logging is a really thoughtful touch too. thanks for keeping pushing on this stuff 🔥