Skip to content

fix(observability): classify SessionExpired at agent layer (OPENHUMAN-TAURI-26)#1763

Merged
senamakel merged 1 commit into
tinyhumansai:mainfrom
CodeGhost21:fix/observability-session-expired-26
May 15, 2026
Merged

fix(observability): classify SessionExpired at agent layer (OPENHUMAN-TAURI-26)#1763
senamakel merged 1 commit into
tinyhumansai:mainfrom
CodeGhost21:fix/observability-session-expired-26

Conversation

@CodeGhost21
Copy link
Copy Markdown
Contributor

@CodeGhost21 CodeGhost21 commented May 14, 2026

Summary

  • agent.run_single and web_channel.run_chat_task call report_error_or_expected before the JSON-RPC dispatch boundary's session-expired filter ever runs, so mid-conversation 401s from the OpenHuman backend ("OpenHuman API error (401 Unauthorized): {…Session expired…}") escaped as Sentry error events from the agent layer — OPENHUMAN-TAURI-26 (15 events on release 0.53.22 before fix(auth): stop 401 cascade after session expiry (OPENHUMAN-TAURI-1T) #1516's cascade dampener reduced volume but did not close the code path).
  • Adds ExpectedErrorKind::SessionExpired and a strict shared classifier is_session_expired_message in core::observability. Anchored on canonical OpenHuman session phrasing ("session expired", "no backend session token", "session JWT required", SESSION_EXPIRED sentinel) so the demote does not silence BYO-key OpenAI / Anthropic 401s — those are actionable misconfigurations and must keep reaching Sentry.
  • Dispatch-site classifier (core::jsonrpc::is_session_expired_error) stays looser on "401 + unauthorized" / "invalid token" so token cleanup and DomainEvent::SessionExpired publish still fire on every 401 propagated to invoke_method. Auto-cleanup teardown unchanged.

Why two classifiers, not one

The dispatch-site predicate needs to be broad enough to clear the local token defensively on any 401 (including a misconfigured BYO-key — clearing stale state is safe). The agent-layer demote must be narrow so a BYO-key 401 still reaches Sentry as an actionable error. Same boundary, different jobs — the docstrings on both helpers call out the relationship explicitly.

Test plan

  • cargo test --lib observability — 45 pass, new tests cover OPENHUMAN-TAURI-26 wire shape (with caller wrapping), the SESSION_EXPIRED sentinel, "no backend session token" / "session JWT required" pre-flight guards, and explicitly assert BYO-key provider 401s (OpenAI / Anthropic) are not silenced.
  • cargo test --lib jsonrpc — 57 pass, existing is_session_expired_error_* suite still green (the looser dispatch-site classifier is unchanged in behavior).
  • cargo fmt + cargo check on core (Cargo.toml) and Tauri shell (app/src-tauri/Cargo.toml).

Closes

Fixes OPENHUMAN-TAURI-26

Summary by CodeRabbit

  • Bug Fixes
    • Improved session expiration error detection and classification. Session-expired errors are now logged at the appropriate level rather than being escalated to error tracking services, reducing noise in error monitoring.
  • Tests
    • Added comprehensive test coverage for session expiration classification scenarios and boundary conditions.

Review Change Stack

…-TAURI-26)

`agent.run_single` and `web_channel.run_chat_task` call `report_error_or_expected`
*before* the JSON-RPC dispatch boundary sees the error string, so the
session-expired classifier living privately in `core::jsonrpc.rs` never ran
against them. Mid-conversation 401s emitted by the OpenHuman backend
("OpenHuman API error (401 Unauthorized): {…Session expired…}") therefore
escaped as Sentry error events from the agent layer — 15 events on
OPENHUMAN-TAURI-26, all from release 0.53.22 before tinyhumansai#1516's cascade
dampener reduced volume but did not close this code path.

Adds `ExpectedErrorKind::SessionExpired` + a strict shared classifier
`is_session_expired_message` in `core::observability`, anchored on
canonical OpenHuman session phrasing ("session expired", "no backend
session token", "session JWT required", and the `SESSION_EXPIRED`
sentinel) so the demote does not silence BYO-key OpenAI / Anthropic 401s
(those are actionable misconfigurations and must reach Sentry).

The dispatch-site classifier (`core::jsonrpc::is_session_expired_error`)
keeps its existing **looser** "401 + unauthorized" / "invalid token"
match — token cleanup and `DomainEvent::SessionExpired` publish still
fire on every 401 propagated to `invoke_method`, so the auto-cleanup
teardown is unchanged.
@CodeGhost21 CodeGhost21 requested a review from a team May 14, 2026 20:07
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 14, 2026

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: a631d48d-2c50-47c2-a0cf-48dd0be40302

📥 Commits

Reviewing files that changed from the base of the PR and between 7d8826f and 48d4ab0.

📒 Files selected for processing (2)
  • src/core/jsonrpc.rs
  • src/core/observability.rs

📝 Walkthrough

Walkthrough

This PR adds a new SessionExpired variant to the observability error classification system. A new is_session_expired_message helper detects session-expired wire shapes via substring matching, which expected_error_kind uses to classify errors. Matching errors are demoted from Sentry reporting to info-level breadcrumbs. Documentation clarifies the distinction between this strict observability classifier and the looser matching in jsonrpc.rs.

Changes

Session-expired error classification

Layer / File(s) Summary
SessionExpired enum variant
src/core/observability.rs
ExpectedErrorKind gains a public SessionExpired variant.
Session-expired detection and wiring
src/core/observability.rs
is_session_expired_message matches OpenHuman session-expired shapes via case-insensitive substrings and a case-sensitive sentinel, and expected_error_kind classifies matching messages as SessionExpired.
Error reporting integration
src/core/observability.rs
report_expected_message match arm for SessionExpired logs an info breadcrumb and suppresses Sentry reporting.
Classification tests
src/core/observability.rs
New unit tests verify correct SessionExpired classification for backend and wrapped strings, and ensure generic 401 messages are not misclassified.
Matching behavior documentation
src/core/jsonrpc.rs
Updated comment clarifies that is_session_expired_error uses intentionally looser matching than the strict observability classifier, including on BYO-key provider failures.

Possibly related PRs

  • tinyhumansai/openhuman#1669: Both PRs extend ExpectedErrorKind and wire new variants into expected_error_kind/report_error_or_expected expected-error classification.
  • tinyhumansai/openhuman#1504: Both PRs extend session-expired classification for the "no backend session token" error shape across the auth boundary matching logic.
  • tinyhumansai/openhuman#1551: Both PRs update session-expired detection, with this PR adding observability classification and the related PR extending is_session_expired_error matching to the same wire shapes.

Suggested labels

working

Suggested reviewers

  • senamakel

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A session expired, the token grew old,
So we classify it, observant and bold,
No Sentry alarms for this boundary so clear—
Just breadcrumbs and logs to hold dear! 🔍✨

🚥 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 describes the main change: adding SessionExpired classification at the agent layer in the observability module, which is the core objective of the PR.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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 coderabbitai Bot added the working A PR that is being worked on by the team. label May 14, 2026
@senamakel senamakel merged commit 1fb0bef into tinyhumansai:main May 15, 2026
25 of 28 checks passed
oxoxDev added a commit to oxoxDev/openhuman that referenced this pull request May 15, 2026
…ore_send wire

Recreates PR tinyhumansai#1719's surface against the current upstream/main (the
prior branch was 20+ commits behind and `SessionExpired` enum + matcher
`is_session_expired_message` + `expected_error_kind` arm + per-kind
`report_expected_message` arm all landed via tinyhumansai#1763, so the original
classifier-extension commits were duplicates).

Three remaining unique deltas land here in one commit:

- `is_session_expired_event(event)` in `src/core/observability.rs` —
  tag-and-body classifier that drops `(domain=llm_provider|backend_api|rpc)
  + status=401 + body matches is_session_expired_message` shapes, plus
  the pre-flight rpc dispatcher path that has no status tag. Composio's
  OAuth-state 401 is intentionally excluded — that's actionable and
  must reach Sentry.

- Bilateral before_send wire in `src/main.rs` (core binary) and
  `app/src-tauri/src/lib.rs` (Tauri shell — since tinyhumansai#1061 it links the
  core in-process, so any session-expired event captured by either
  surface lands in the same Sentry client and must be filtered
  identically). Both filters log only `event.event_id` per CR feedback
  from PR tinyhumansai#1719 — `event.message` carries the raw backend response
  body which CLAUDE.md forbids from local logs.

- Smoke test runtime path in `tests/observability_smoke.rs` — imports
  the new classifier and threads it into the same `before_send` chain
  shape the real binary installs, so the runtime drop is covered end-
  to-end alongside the existing budget / transient / updater filters.

Drops OPENHUMAN-TAURI-25 / -1Q / -27 / -1G (~185 events/day combined).
The original PR's emit-site demotions (compatible.rs / authed_json /
agent run_single) were superseded by tinyhumansai#1763 — leaving them out keeps
this PR scoped to the before_send defense-in-depth that tinyhumansai#1763 didn't
ship.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
AusAgentSmith pushed a commit to AusAgentSmith/openhuman that referenced this pull request May 23, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

working A PR that is being worked on by the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants