Let auth profile locks reach stale reclaim

[aqilaziz]

Summary

• Extends the auth profile lock wait horizon so a fresh leaked lock can cross the stale-lock threshold and be reclaimed.
• Keeps the existing 30s stale threshold unchanged; only the caller-facing timeout moves to STALE_LOCK_AGE_MS + 5s.
• Adds a regression test that locks the timeout/stale-age relationship so the recovery path cannot become unreachable again.

Problem

• Sentry issue TAURI-RUST-B1 reports Timed out waiting for auth profile lock.
• The stale-lock reclaim threshold is 30s, but the lock wait timeout was 10s.
• That meant a just-orphaned lock with a live pid could never age into the existing age-based recovery before callers gave up.

Solution

• Derive LOCK_TIMEOUT_MS from STALE_LOCK_AGE_MS + 5_000.
• Update stale-lock comments to avoid the stale 10s wording.
• Add a focused unit-level guard for the timeout relation.

Submission Checklist

• Tests added or updated (happy path + at least one failure / edge case) per Testing Strategy
• Diff coverage ≥ 80% — changed lines (Vitest + cargo-llvm-cov merged via diff-cover) meet the gate enforced by .github/workflows/coverage.yml. Run pnpm test:coverage and pnpm test:rust locally; PRs below 80% on changed lines will not merge.
• Coverage matrix updated — N/A: internal auth-profile lock recovery constant, no matrix feature row.
• All affected feature IDs from the matrix are listed in the PR description under ## Related
• No new external network dependencies introduced (mock backend used per Testing Strategy)
• Manual smoke checklist updated if this touches release-cut surfaces — N/A: no release-cut surface touched.
• Linked issue closed via Closes #NNN in the ## Related section

Impact

• Auth profile operations now wait long enough for existing stale-lock recovery to handle fresh leaked locks.
• Worst-case wait before reporting a truly live lock increases from 10s to 35s.
• No storage schema or API contract changes.

Related

• Closes: Timed out waiting for auth profile lock #2318
• Follow-up PR(s)/TODOs: N/A
• Coverage matrix feature IDs: N/A: credentials lock recovery internals.

---

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

Linear Issue

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

Commit & Branch

• Branch: codex/2318-auth-profile-lock-timeout
• Commit SHA: c526748

Validation Run

• pnpm --filter openhuman-app format:check — N/A: no frontend files changed.
• pnpm typecheck — N/A: no TypeScript files changed.
• Focused tests: attempted cargo test -p openhuman credentials::profiles --lib.
• Rust fmt/check (if changed): cargo fmt --all --check; git diff --check.
• Tauri fmt/check (if changed): N/A, no app/src-tauri files changed.

Validation Blocked

• command: cargo test -p openhuman credentials::profiles --lib
• error: whisper-rs-sys build could not find clang.dll / libclang.dll; LIBCLANG_PATH is not set in this local Windows environment.
• impact: The targeted Rust test binary did not run locally; CI has the Linux Rust toolchain/image and should execute it.

Behavior Changes

• Intended behavior change: auth profile lock acquisition can wait past the stale-lock threshold and reclaim a fresh leaked lock instead of timing out first.
• User-visible effect: fewer startup/session failures from transient orphaned auth-profiles.lock files.

Parity Contract

• Legacy behavior preserved: stale lock detection logic, pid checks, malformed-lock rules, and guard cleanup are unchanged.
• Guard/fallback/dispatch parity checks: existing stale-lock tests remain in place; new constant guard ensures recovery remains reachable.

Duplicate / Superseded PR Handling

• Duplicate PR(s): N/A
• Canonical PR: N/A
• Resolution (closed/superseded/updated): N/A

Summary by CodeRabbit

• Refactor

  • Improved lock timeout configuration for enhanced maintainability.

• Tests

  • Added test to validate lock timeout behavior and timing constraints.

[coderabbitai]

📝 Walkthrough

Walkthrough

The PR refactors the auth-profile lock timeout constant from a hardcoded 10,000 ms value to a derived computation based on STALE_LOCK_AGE_MS + 5_000. Comments referencing the timeout are updated to use the constant name, and a new test validates the timing relationship between the two constants.

Changes

Lock Timeout Constant Derivation

Layer / File(s)	Summary
Lock timeout constant and documentation alignment src/openhuman/credentials/profiles.rs	LOCK_TIMEOUT_MS is redefined as STALE_LOCK_AGE_MS + 5_000 and positioned after its dependency. Comments in lock acquisition (lines 597–603) and stale-lock reclaim (lines 656–660) paths are updated to reference LOCK_TIMEOUT_MS instead of hardcoded 10s values.
Timing constraint validation test src/openhuman/credentials/profiles_tests.rs	New test lock_timeout_allows_fresh_leaked_locks_to_age_into_stale_reclaim asserts that LOCK_TIMEOUT_MS exceeds STALE_LOCK_AGE_MS by at least 1,000 ms, enforcing the timeout/stale-reclaim timing window.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related issues

• tinyhumansai/openhuman#2318: Directly addresses the lock timeout issue with a more principled constant derivation and validation test.

Possibly related PRs

• tinyhumansai/openhuman#2085: Introduced STALE_LOCK_AGE_MS-based stale-lock reclaim and mid-wait stale probing logic that this PR now integrates with via constant derivation.

Suggested labels

working

Poem

🐰 A timeout's born from constants wise,
Five thousand more than stale's reprise,
The lock's new rhythm, clear and bright,
With tests to prove the timing's right!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 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: extending auth profile lock timeout to allow freshly leaked locks to reach the stale reclaim threshold.
Linked Issues check	✅ Passed	Changes implement the objective from #2318: extending lock wait timeout from 10s to 35s (STALE_LOCK_AGE_MS + 5s) to allow orphaned locks time to age into stale-lock recovery.
Out of Scope Changes check	✅ Passed	All changes are directly related to the lock timeout logic: constant redefinition, comment updates, and regression-prevention test—no unrelated modifications present.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

Warning

Review ran into problems

🔥 Problems

Stopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a @coderabbit review after the pipeline has finished.

---

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/openhuman/credentials/profiles.rs (1)

596-622: ⚡ Quick win

Update the remaining hardcoded 10s wording in this lock-wait comment.

Line 621 still says “the 10s timeout,” which is now stale after deriving LOCK_TIMEOUT_MS from STALE_LOCK_AGE_MS + 5_000. Align it to LOCK_TIMEOUT_MS for consistency with the rest of the block.

🤖 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 596 - 622, The inline
comment referring to "the 10s timeout" is stale; update that wording to
reference the LOCK_TIMEOUT_MS constant (or phrase it as "the LOCK_TIMEOUT_MS
timeout" / "the configured LOCK_TIMEOUT_MS") so the comment in the lock-wait
loop that surrounds clear_lock_if_stale(), cleared_stale, started_at,
next_stale_recheck_ms and the derived STALE_LOCK_AGE_MS + 5_000 logic stays
accurate and consistent with the implementation.

🤖 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 596-622: The inline comment referring to "the 10s timeout" is
stale; update that wording to reference the LOCK_TIMEOUT_MS constant (or phrase
it as "the LOCK_TIMEOUT_MS timeout" / "the configured LOCK_TIMEOUT_MS") so the
comment in the lock-wait loop that surrounds clear_lock_if_stale(),
cleared_stale, started_at, next_stale_recheck_ms and the derived
STALE_LOCK_AGE_MS + 5_000 logic stays accurate and consistent with the
implementation.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: fd0d53be-a47c-40bb-8d4d-e7f540603b8f

📥 Commits

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

📒 Files selected for processing (2)

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

[YOMXXX]

@senamakel #2321 当前 checks 24 success / 3 skipped / 0 pending / 0 failure，CodeRabbit 最新 review approved，review threads 0 unresolved，mergeable。这个是 auth profile lock timeout 的 critical 修复，麻烦再看一下。

[senamakel]

nice one @aqilaziz, letting the wait horizon stretch past the stale threshold so leaked locks can actually get reclaimed is such a clean fix 🙌 love that you locked it in with a regression test too so the recovery path stays reachable. thanks for keeping at it!