fix(auth-profiles): tolerate legacy kind values on load

[YellowSnnowmann]

Summary

• AuthProfilesStore::load_locked no longer bails the entire load when a single persisted profile has a kind value that the current parser can't interpret (e.g. legacy "OAuth" written before the kebab-case rename, or "api_key" from an older code path).
• Instead the bad row is logged at warn, pushed into the existing dropped_ids set, and purged from auth-profiles.json (along with any stale active_profiles pointer) on the same load — matching the established decrypt-failure recovery pattern in the same function.
• Reduces blast radius from "every provider is broken until the user manually deletes auth-profiles.json" to "one provider needs a fresh login; everything else keeps working."
• Eliminates the per-poll Sentry spam from affected users (≈630 events / 14d across the two issues below) and stops the symptom from re-firing on subsequent loads.
• Added load_drops_profiles_with_unrecognized_kind_instead_of_failing_load reproducing both real-world Sentry payloads ("api_key" and legacy "OAuth") and asserting in-memory drop + on-disk purge + cleared active-pointer.

Problem

• Sentry issue TAURI-RUST-123 — Unsupported auth profile kind: api_key (370 events / 14d).
• Sentry issue TAURI-RUST-2605 — Unsupported auth profile kind: OAuth (258 events / 14d).
• Both surface from the same line in src/openhuman/credentials/profiles.rs:

  let kind = parse_profile_kind(&p.kind)?;

  parse_profile_kind only accepts "oauth" / "token". When any persisted row has a different string (legacy variant-name serialization, older code path writing "api_key"), the ? propagates the error out of load_locked, which means every caller of AuthProfilesStore::load() — set_active_profile, update_profile, app_state_snapshot polls, inference_status polls, every credential read — fails for that user.
• The Tauri shell polls app_state_snapshot / inference_status roughly once per second, so a single bad row produces hundreds of identical Sentry events per session and locks the user out of all their providers, not just the one with the malformed kind. The only user-facing recovery path was "manually delete auth-profiles.json," which also wipes every working profile.

Solution

• Replace the ? with a match that, on Err, logs the unrecognized kind + provider at warn, pushes the id into dropped_ids, and continues — the same shape already used for decrypt failures at lines 287–300 of the same function.
• No new infrastructure: the existing post-loop block at lines 378–395 already removes dropped_ids from persisted.profiles, clears any active_profiles entry that pointed at them, bumps updated_at, and rewrites the file under the lock that's already held. So the bad row is purged on this load and can't keep retriggering on subsequent loads.
• parse_profile_kind itself is unchanged — the public/private API surface, struct shapes, serde reprs, and trait impls are all untouched. The only behavioural delta is that load_locked now degrades gracefully on one malformed row instead of failing closed on all of them.
• Tradeoff: a user whose row was malformed has to re-authenticate that one provider. This is strictly better than the prior "all providers unusable until the file is deleted" failure mode, and the new log::warn! plus a single Sentry breadcrumb tells the user exactly what happened.

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.

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy — added load_drops_profiles_with_unrecognized_kind_instead_of_failing_load covering both real Sentry payloads ("api_key" + legacy "OAuth"); existing store_roundtrip_with_encryption / corrupt_store_is_quarantined_and_reset / load_drops_profiles_whose_decryption_fails_under_rotated_key continue to cover the happy and adjacent-failure paths.
• Diff coverage ≥ 80% — every changed line in profiles.rs (the new match arm) is exercised by the new test, which asserts both the in-memory and on-disk outcome. Verified locally via cargo test --lib openhuman::credentials::profiles (34/34 pass).
• Coverage matrix updated — N/A: behaviour-only fix to an existing code path; no new/removed/renamed feature rows.
• All affected feature IDs from the matrix are listed in the PR description under ## Related — N/A: no matrix rows touched (see above).
• No new external network dependencies introduced (mock backend used per Testing Strategy) — purely local filesystem behaviour; no new crates, no new network calls.
• Manual smoke checklist updated if this touches release-cut surfaces (docs/RELEASE-MANUAL-SMOKE.md) — N/A: internal load-path resilience fix, no UI/release-cut surface changes.
• Linked issue closed via Closes #NNN in the ## Related section — N/A: no GitHub issue filed; links to Sentry issues TAURI-RUST-123 and TAURI-RUST-2605 included instead.

Impact

• Platform: desktop only (Windows / macOS / Linux). The change is in the Rust core (openhuman lib crate) loaded in-process by the Tauri shell; the mobile/web/CLI surfaces don't read this store.
• Runtime: O(1) extra match on the load hot path, no extra I/O for healthy stores; on stores with bad rows, one additional write_persisted_locked per load — already paid by the existing decrypt-failure recovery, so no new write semantics.
• Performance: negligible; the load is a one-shot per session-state poll and the file is small.
• Security: the dropped row's tokens were unusable already (no valid kind to interpret them under) — purging them is strictly safer than leaving them parked on disk. No PII, secrets, or token values are logged; only kind (the unrecognized string) and provider (the slug) appear in the warn line.
• Migration / compatibility: backward-compatible. Reads any pre-existing auth-profiles.json (including malformed legacy rows), writes a clean current-schema version on next save. No schema version bump needed; no manual user action beyond re-authenticating the affected provider.
• Failure modes removed: per-poll error spam to Sentry from affected users; locked-out-of-all-providers state caused by a single malformed row.

Related

• Sentry: TAURI-RUST-1S — Unsupported auth profile kind: api_key (370 events / 14d)
• Sentry: TAURI-RUST-24A — Unsupported auth profile kind: OAuth (258 events / 14d)

Summary by CodeRabbit

• Bug Fixes
  • Improved credential store resilience by gracefully handling unrecognized profile entries. The system now skips invalid profiles instead of failing the entire load operation, allowing other profiles to load successfully. Invalid entries are automatically removed during the load process.

[coderabbitai]

📝 Walkthrough

Walkthrough

This PR improves resilience of auth profile loading by handling malformed profile kind values gracefully. Instead of failing the entire store load when an unrecognized kind is encountered, the code now logs a warning, collects the profile ID for removal, and continues loading valid profiles. A test validates the complete behavior including state cleanup and persistence.

Changes

Profile Kind Parsing Resilience

Layer / File(s)	Summary
Graceful handling of unrecognized profile kinds src/openhuman/credentials/profiles.rs, src/openhuman/credentials/profiles_tests.rs	load_locked treats parse failures as per-profile recoverable errors: invalid kinds are logged, profile IDs are recorded for removal, and load continues. The test seeds invalid profiles (api_key, legacy OAuth) and verifies successful load with purged state and re-persisted JSON.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested labels

working

Suggested reviewers

• senamakel

Poem

🐰 When profiles wear forgotten masks,
The store now bravely logs and asks,
"Shall we drop these ancient ways?"
And onward march through cleaner days,
With warnings kind, no store dismays!

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The PR title 'fix(auth-profiles): tolerate legacy kind values on load' accurately captures the main change: making the auth profiles loader tolerate unrecognized legacy kind values instead of failing entirely.
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.
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.}

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/openhuman/credentials/profiles.rs (1)

329-335: ⚡ Quick win

Use debug/trace level for this recovery-path diagnostic log.

Line 329 logs a recoverable branch with warn; repo policy for Rust diagnostics/error-path tracing requires debug or trace.

♻️ Suggested change

-                    log::warn!(
+                    log::debug!(
                         "[auth] dropping profile with unrecognized kind={:?} provider={}: {e}. \
                          This usually means the profile was written by an older version of \
                          OpenHuman. Re-authenticate to restore the session.",
                         p.kind,
                         p.provider
                     );

As per coding guidelines "Use log / tracing at debug or trace level on RPC entry and exit, error paths, state transitions, and any branch that is hard to infer from tests alone."

🤖 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/openhuman/credentials/profiles.rs` around lines 329 - 335, The log call
that warns about dropping an unrecognized profile should be lowered to a
diagnostic level; replace the log::warn! invocation that references p.kind and
p.provider with log::debug! (or log::trace! if you prefer more verbosity) so
this recoverable branch is logged at debug/trace level; update the logging macro
at the line where the message "dropping profile with unrecognized kind={:?}
provider={}: {e}..." is emitted (the call that interpolates p.kind and
p.provider) to use the chosen debug/trace macro and keep the exact message and
interpolated variables unchanged.

🤖 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/openhuman/credentials/profiles.rs`:
- Around line 329-335: The log call that warns about dropping an unrecognized
profile should be lowered to a diagnostic level; replace the log::warn!
invocation that references p.kind and p.provider with log::debug! (or
log::trace! if you prefer more verbosity) so this recoverable branch is logged
at debug/trace level; update the logging macro at the line where the message
"dropping profile with unrecognized kind={:?} provider={}: {e}..." is emitted
(the call that interpolates p.kind and p.provider) to use the chosen debug/trace
macro and keep the exact message and interpolated variables unchanged.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f5cfce59-1672-4b4b-8667-11cca0bfed7f

📥 Commits

Reviewing files that changed from the base of the PR and between bf6f25e and a4cef67.

📒 Files selected for processing (2)

• src/openhuman/credentials/profiles.rs
• src/openhuman/credentials/profiles_tests.rs

[graycyrus]

Looks good, nice work!