[CodeGhost21]

Summary

• Extends the SI server's start_session error classifier with a new sibling, is_expected_session_already_active_failure, plus a union helper is_expected_benign_start_session_failure.
• Both SiServer::run (start_session error arm) and start_if_enabled's spawned future now consume the union helper, so duplicate start_session calls log at info! instead of error! — sentry-tracing records at most a breadcrumb.

Fixes OPENHUMAN-TAURI-5H (16 macOS occurrences in 4 days on openhuman@0.53.22).

Why this is benign, not a failure

engine::start_session returns "session already active" (engine.rs:107) when the engine already has a live session. This happens at the embedded server entry point when something else got there first:

• screen_intelligence.enable invoked over RPC (UI/onboarding flow) before the embedded run() reached its own start_session, or
• A prior run() cycle left the engine session in place across a server restart.

The session is running. The embedded server's start_session is a duplicate, not a real failure — there's no remediable Sentry signal.

Why introduce a union helper instead of inlining

The classifier sits at two call sites today (SiServer::run, start_if_enabled). A union helper keeps the log-level decision in lockstep across both — same pattern as PR #1542 (is_expected_macos_only_failure).

Test plan

• cargo test --lib openhuman::screen_intelligence::server — 9 tests pass.
• is_expected_session_already_active_failure_classifies_known_signal covers the bare wire shape, a wrapped/prefixed form, and pins unrelated session-domain failures (permission errors, IO errors) so they still surface as Sentry events.
• is_expected_benign_start_session_failure_is_union_of_classifiers pins the union helper's contract against both known-benign shapes.
• cargo check --lib — no new warnings.
• cargo fmt.

Summary by CodeRabbit

• Bug Fixes

  • Screen intelligence server now treats duplicate session start attempts (a known benign condition) as non-fatal and logs them at info level instead of failing.
  • Error logging improved to distinguish benign start-session failures from critical errors.

• Tests

  • Added unit tests covering duplicate session detection and the combined error-classification behavior.

[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: 4bfc0069-a6e3-4836-9239-c187ad908428

📥 Commits

Reviewing files that changed from the base of the PR and between f8e2aa9 and 6f46203.

📒 Files selected for processing (1)

• src/openhuman/screen_intelligence/server.rs

🚧 Files skipped from review as they are similar to previous changes (1)

• src/openhuman/screen_intelligence/server.rs

---

📝 Walkthrough

Walkthrough

Refines screen-intelligence session-start error handling: adds detection for engine "session already active", unions it with the existing macOS classifier, applies the union at SiServer::run and embedded autostart, and adds unit tests.

Changes

Session Start Failure Classification

Layer / File(s)	Summary
Error classifier helpers src/openhuman/screen_intelligence/server.rs	New private helpers detect the engine's "session already active" failure message and a union classifier combining this with the existing macOS-only classifier.
Benign session start integration at call sites src/openhuman/screen_intelligence/server.rs	SiServer::run and the embedded auto-start spawned future now use the unified benign classifier to log and handle start-session failures consistently as non-fatal when they match either known benign shape.
Classifier unit tests src/openhuman/screen_intelligence/server.rs	Tests verify the "session already active" detector recognizes engine error strings, rejects unrelated failures, and confirm the union helper correctly identifies both benign shapes and rejects genuine errors.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

• tinyhumansai/openhuman#1542: Both PRs modify src/openhuman/screen_intelligence/server.rs to classify start_session failures and adjust info! vs error! logging/behavior across SiServer::run and the embedded start_if_enabled autostart path.
• tinyhumansai/openhuman#382: Introduced the standalone SiServer runtime that starts an AccessibilityEngine session, which this PR then refines by treating "session already active" as benign at the SiServer::run and autostart call sites.

Suggested reviewers

• senamakel

Poem

🐰 I hopped into code with a curious grin,
Found sessions already started—no need for din.
One helper to spot the duplicate start,
Two call sites now log it gentle, not smart—
Tests snug like carrots, the change is a win.

🚥 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 clearly and specifically describes the main change: introducing benign error handling for 'session already active' in start_session with Sentry skip logic, matching the PR's core objective.
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.}

[graycyrus]

Walkthrough

Adds a new is_expected_session_already_active_failure classifier for the benign "session already active" error from engine::start_session, unions it with the existing is_expected_macos_only_failure via is_expected_benign_start_session_failure, and swaps both call sites to the union helper. Net effect: duplicate start_session calls log at info! instead of error!, silencing the OPENHUMAN-TAURI-5H noise in Sentry.

Change Summary

File	Change	Description
src/openhuman/screen_intelligence/server.rs	Modified	Added two new classifier fns, replaced call sites, added 2 unit tests

Analysis

Clean, focused observability fix. Follows the established pattern from is_expected_macos_only_failure exactly — private classifier, union helper, both call sites updated in lockstep. Tests cover canonical wire shape, substring matching (wrapped/prefixed), negative cases for unrelated failures, and the union contract. Good negative-case discipline (permission errors, IO errors pinned as non-benign).

One CI-blocking issue found (see inline comment).