Skip to content

test(observability): pin SESSION_EXPIRED wire-shape regression tests (#SG)#2188

Merged
senamakel merged 2 commits into
tinyhumansai:mainfrom
oxoxDev:fix/sentry-session-expired-leak-sg
May 19, 2026
Merged

test(observability): pin SESSION_EXPIRED wire-shape regression tests (#SG)#2188
senamakel merged 2 commits into
tinyhumansai:mainfrom
oxoxDev:fix/sentry-session-expired-leak-sg

Conversation

@oxoxDev
Copy link
Copy Markdown
Contributor

@oxoxDev oxoxDev commented May 19, 2026

Summary

Problem

OPENHUMAN-TAURI-SG (33 events, escalating, all on release 0.53.43+2b64ea8…) leaked the SESSION_EXPIRED: backend session not active — sign in to resume LLM work sentinel — raised at src/openhuman/inference/provider/openhuman_backend.rs:55 (resolve_bearer) — through agent.run_single (src/openhuman/agent/harness/session/runtime.rs:528) before PR #1763 wired the ExpectedErrorKind::SessionExpired arm in src/core/observability.rs. Lane C's KICKOFF Step 1 diagnostic test confirmed the post-#1763 classifier already matches the SG byte string verbatim; the existing classifies_session_expired_messages test (src/core/observability.rs:1594) covers the same string but does not anchor it to the Sentry-event payload as a pinned regression case. Lane C Step 2A grep across agent/, cron/, scheduler_gate/, inference/ found no direct report_error! / capture_message bypass — every SESSION_EXPIRED emitter routes through report_error_or_expected on main. The 33 events are pre-fix-release noise on 0.53.43 (does not contain 1fb0bef5) and will phase out as staging/production binaries roll past v0.53.48-staging+ (which does contain the fix).

Two sibling sentinel sites at src/openhuman/inference/provider/factory.rs:247 and :266 (verify_session_active) emit related but distinct SESSION_EXPIRED: messages. They share the is_session_expired_message msg.contains("SESSION_EXPIRED") branch with the SG sentinel — a single matcher tweak (e.g. tightening from contains to a prefix/suffix anchor) would silently re-leak all three. None of the three sibling strings had its own pinned test before this PR.

Solution

Two new tests in src/core/observability::tests, kept tight and family-scoped:

  • session_expired_sg_wire_shape_matches — single-case test asserting the exact byte string lifted from the OPENHUMAN-TAURI-SG Sentry-event payload classifies as ExpectedErrorKind::SessionExpired. Anchored to the Sentry payload so a future matcher tweak can't regress this specific wire form without a red test (and an explicit message linking the regression cost: "re-leaks 33+ events/cycle to Sentry").
  • session_expired_sibling_family_factory_strings_match — pins both factory.rs:247 and factory.rs:266 variants. Same arm, different suffix bytes — any matcher narrowing that catches one but misses another would surface here.

No change to is_session_expired_message, expected_error_kind, or any caller. Scope deliberately narrow per KICKOFF: do not generalize the matcher beyond what is needed (per #1719 CR — broadening risks BYO-key 401 false-positives).

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.

Impact

  • Runtime/platform: none — #[cfg(test)] only, no binary diff in openhuman-core or app/src-tauri.
  • Performance: none.
  • Security / migration / compatibility: none. The matcher rule itself is unchanged.
  • Pre-existing repo clippy state (e.g. clippy::useless_vec, clippy::field_reassign_with_default, clippy::duplicate_mod) is unrelated to this PR and predates the branch tip — flagged on upstream/main directly.

Related

Closes OPENHUMAN-TAURI-SG

Sentry-Issue: OPENHUMAN-TAURI-SG

Context PRs:


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 — Sentry-only PR, no Linear issue.
  • URL: N/A

Commit & Branch

  • Branch: fix/sentry-session-expired-leak-sg
  • Commit SHA: 5aed9756 (head)

Validation Run

  • N/A: pnpm --filter openhuman-app format:check — Rust-only change, no app/ files touched.
  • N/A: pnpm typecheck — Rust-only change.
  • Focused tests: cargo test --lib core::observability::tests (57 passed, 0 failed) and cargo test --lib core::observability::tests::session_expired (both new tests green).
  • Rust fmt/check (if changed): cargo fmt --check clean; cargo check clean.
  • N/A: Tauri fmt/check (if changed) — no app/src-tauri/ change.

Validation Blocked

  • command: cargo clippy --lib --tests -- -D warnings
  • error: 501 pre-existing clippy errors across unrelated files (e.g. src/openhuman/tokenjuice/text/process.rs clippy::useless_vec, src/openhuman/inference/provider/compatible_dump.rs clippy::duplicate_mod, src/core/observability.rs:2193,2199 clippy::field_reassign_with_default). None introduced by this branch; all reproduce on upstream/main (7741c58).
  • impact: none — added test code is clippy-clean.

Behavior Changes

  • Intended behavior change: none — test-only PR.
  • User-visible effect: none.

Parity Contract

  • Legacy behavior preserved: yes — no production code touched.
  • Guard/fallback/dispatch parity checks: existing positive and negative SessionExpired tests continue to pass (BYO-key 401 still classified None, unrelated 401 / 500 still classified None).

Summary by CodeRabbit

  • Tests
    • Extended test coverage for session expiration message classification, adding tests to verify proper handling of session-expired message variants.

Review Change Stack

oxoxDev added 2 commits May 19, 2026 14:19
…wire shape

OPENHUMAN-TAURI-SG (33 events, escalating, release 0.53.43) was a
pre-tinyhumansai#1763 leak of `providers::openhuman_backend::resolve_bearer`'s
SESSION_EXPIRED sentinel through `agent.run_single`. PR tinyhumansai#1763 (1fb0bef)
wired the `SessionExpired` arm and the classifier now matches verbatim.

Add a single-case test that pins the exact byte string lifted from the
Sentry-event payload so a future tweak to `is_session_expired_message`
(or its callers) cannot regress this specific wire form without a red
test.

Related: OPENHUMAN-TAURI-SG
…47,266)

`providers::factory::verify_session_active` emits two more SESSION_EXPIRED
bail variants alongside the run_single sentinel that anchored SG:

- "SESSION_EXPIRED: backend session not active — sign in to use custom providers"
  (scheduler_gate signed-out fast-path)
- "SESSION_EXPIRED: no backend session — sign in to use OpenHuman"
  (empty auth-profile JWT pre-flight)

All three currently classify via the `msg.contains("SESSION_EXPIRED")`
branch in `is_session_expired_message`. Pin both sibling strings with
their own test so a future matcher tweak (e.g. moving from `contains`
to a stricter prefix/suffix match) is caught for the whole family, not
just the SG instance — preventing leak #3 / #4 down the line.

Related: OPENHUMAN-TAURI-SG
@oxoxDev oxoxDev requested a review from a team May 19, 2026 08:52
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 19, 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: 993c86f3-821e-43df-9980-b94e4367a508

📥 Commits

Reviewing files that changed from the base of the PR and between f285b80 and 5aed975.

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

📝 Walkthrough

Walkthrough

Two regression unit tests were added to src/core/observability.rs to verify that expected_error_kind correctly classifies specific SESSION_EXPIRED: sentinel wire strings and their variants as ExpectedErrorKind::SessionExpired.

Changes

Session Expired Classification Tests

Layer / File(s) Summary
Session Expired Classification Tests
src/core/observability.rs
Two regression tests pin exact SESSION_EXPIRED: sentinel wire strings (SG variant and providers::factory::verify_session_active custom-providers and no-backend-session siblings) and verify they all classify as ExpectedErrorKind::SessionExpired.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

  • tinyhumansai/openhuman#1763: Added unit tests directly extend the SESSION_EXPIRED and session-expired message classifier changes introduced in this PR.
  • tinyhumansai/openhuman#1719: Added unit tests validate the session-expired classifier behavior introduced in this related PR.

Suggested labels

working

Suggested reviewers

  • senamakel

Poem

🐰 A rabbit hops with tests so keen,
Session's expiration caught between,
The wire strings twine, the sentries shine,
Classification marked with care divine—
Coverage blooming, code pristine!

🚥 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 PR title clearly summarizes the main change: adding regression tests to pin SESSION_EXPIRED wire-shape strings, preventing future matcher tweaks from reopening a previously fixed Sentry leak.
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 19, 2026
@senamakel senamakel merged commit 3281f17 into tinyhumansai:main May 19, 2026
28 of 31 checks passed
mtkik pushed a commit to mtkik/openhuman-meet that referenced this pull request May 21, 2026
CodeGhost21 pushed a commit to CodeGhost21/openhuman that referenced this pull request May 22, 2026
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