fix(credentials): bound keyring calls by time so a dead backend cannot hang (#1181)

[frankbria]

Closes #1181

The bug

credentials.py implemented "keyring unavailable" as raises, or is the fail backend:

kr = keyring.get_keyring()
if "fail" in kr.__class__.__name__.lower():
    return False
return True

A backend that is installed and selected but unresponsive raises nothing — it blocks. On a headless box (WSL2, a container, an SSH session with no session D-Bus) the SecretService backend is importable and gets selected, and the D-Bus call waits with no timeout. Detection by exception therefore never fires, the encrypted-file fallback never engages, and the caller hangs forever.

That is not just a test annoyance: GET /api/v2/settings/keys reaches this through _build_status, so an unresponsive keyring hangs a server request thread indefinitely, and the same blocking read sits under every CLI/agent credential lookup.

The fix

Bound it by time, not only by exception. Every keyring call now runs through _keyring_call, which joins a daemon worker with a timeout and raises KeyringTimeoutError — deliberately a KeyringError subclass, so the "keyring failed, use the encrypted file" handling already present at each call site does the right thing with no new branching.

A blocked call cannot be cancelled (it is parked inside libdbus), so the worker is abandoned. That is bounded: the timeout verdict is sticky per process and short-circuits the wrapper itself, so a store built before the backend died stops waiting too — at most one abandoned thread per process, and the timeout is paid once rather than once per request.

delete() re-raised on any keyring failure, which would have turned a timeout into a failed delete. A timeout there now degrades to the encrypted-file cleanup, which is the part that matters.

New knobs, both documented in CLAUDE.md:

Variable	Effect
CODEFRAME_DISABLE_KEYRING=1	Skip the keyring outright — the explicit opt-out, and avoids even the first timeout
CODEFRAME_KEYRING_TIMEOUT	Seconds per call, default 2.0, read at call time rather than import (#963)

PYTHON_KEYRING_BACKEND=keyring.backends.fail.Keyring remains the keyring-native equivalent and is now documented next to them.

Acceptance criteria

• A blocking backend does not hang CredentialManager; it degrades to the encrypted file store within a bounded time
• GET /api/v2/settings/keys returns rather than hanging when the keyring is unresponsive
• A test pins the timeout path with a deliberately blocking fake backend
• The local workaround is documented

Evidence

The gate the issue reported as wedging:

$ env -u PYTHON_KEYRING_BACKEND uv run pytest tests/ --ignore=tests/e2e -m "not lifecycle" -q
31 failed, 6485 passed, 19 skipped, 2 deselected in 445.36s (0:07:25)

It completes, on the same WSL2 box where it previously sat at 92% for over an hour. All 31 failures reproduce identically on main (verified in a clean worktree, 31 failed in 7.06s) — they are ANSI/terminal-width assertions in CLI output tests, an ambient property of this machine, untouched by this diff.

Affected suites on the final code: 263 passed. ruff check clean.

Tests

tests/core/test_keyring_timeout_1181.py — 12 tests across two deliberately different failure shapes: a backend that is selected instantly but blocks on every credential op, and one where keyring.get_keyring() itself never returns (backend selection probes each candidate's priority, and SecretService's probe talks to D-Bus, so selection is a blocking call in its own right). Covers construction, store/retrieve/delete round-trip through the file fallback, get_credential, the sticky verdict on both new and pre-existing store instances, delete degrading instead of raising, the escape hatch, the endpoint, and that a healthy backend is still used in preference to the file.

Two changes to existing tests, both deliberate:

• test_keyring_availability_check asserted "importable implies available" — precisely the assumption that caused this bug. It now asserts a verdict comes back, and quickly.
• The root conftest.py resets the sticky verdict per test, the same treatment _MIGRATION_COMPLETE gets. Without it a single real timeout silently pushes every later keyring test onto the file fallback, which is an order-dependent failure.

Review

codex review (cross-family, pre-PR) raised two P2s, both fixed in this branch before opening:

1. The sticky flag was only honored in _check_keyring, so a store built before the timeout — and store()'s own delete_password retry after a timed-out set_password — would start another stuck worker and wait again. _keyring_call now short-circuits.
2. The fixture claimed to block backend lookup while get_keyring returned instantly, so the tests did not actually prove the selection-time hang was bounded. Split into two fixtures, each with its own test.

Follow-up fixes from review (all in this branch)

Finding	Source	Status
Sticky flag honored only in _check_keyring, so a pre-existing store — and store()'s own cleanup delete_password — started another blocked worker	codex review, pre-PR	Fixed: _keyring_call short-circuits
Fixture claimed to block backend lookup while get_keyring returned instantly, so the selection-time hang was never actually proven bounded	codex review, pre-PR	Fixed: split into two fixtures with their own tests
Concurrent first callers each pass the flag check and each leak a worker, so the sticky verdict bounds nothing under a burst	codex review, post-PR	Fixed: _KEYRING_CALL_LOCK; mutation-checked at "leaked 8 blocked keyring threads"
list_key_status is async def and called _build_status on the event loop, so the bounded-but-blocking join stalled every in-flight request	claude-review	Fixed: credential calls in async handlers go through run_in_threadpool; mutation-checked at "event loop was blocked (only 0 ticks)"
Unparseable CODEFRAME_KEYRING_TIMEOUT fell back silently	claude-review	Fixed: logs the value it ignored
Timeout env var read twice on the timeout path	claude-review	Fixed: read once

Known limitations

• The abandoned worker thread is unavoidable: there is no way to interrupt a call parked in libdbus. Bounded to one per process by the sticky verdict.
• The verdict is sticky for the life of the process: a keyring that comes back (D-Bus started after the fact) is not re-probed until restart. Deliberate — re-probing would reintroduce a per-call timeout on the common bad case, and the file fallback is correct in the meantime.
• Sticky state is per process, not per worker, so a multi-worker server pays the first timeout once per worker.
• The lock changes contention shape: a healthy backend sitting on a real unlock prompt now queues other credential accesses behind it, where before each caller raced independently. Deliberate — independent racing is exactly what leaked a thread per request, and either outcome (prompt resolves, or the timeout trips the sticky verdict) is bounded, which the old behavior was not.
• Under a burst against a hung backend each request occupies a Starlette threadpool slot while queueing on the lock. Bounded by the threadpool size rather than unbounded; inherent to moving blocking work off the loop, and it matters only if this is ever deployed multi-tenant rather than single-operator.

https://claude.ai/code/session_011pR6Wqk15b3ESqXDiheNMZ

[github-actions]

Precision bug-hunting review in progress

Reviewing #1184 (keyring timeout bounding) for concrete defects only — logic errors, hangs/races, data loss, security.

• Fetch PR diff and context
• Read codeframe/core/credentials.py in full
• Read new/changed tests and tests/conftest.py
• Check all keyring call sites inside and outside credentials.py for unwrapped blocking paths
• Verify env-var parsing, sticky-verdict, and fallback semantics against real callers
• Post inline findings + summary

View job run

[claude]

Reviewed the diff (codeframe/core/credentials.py, tests/core/test_keyring_timeout_1181.py, tests/conftest.py, tests/core/test_credentials_integration.py, CLAUDE.md). No prior substantive review on this PR to reconcile with.

Overall

Solid fix for a real class of bug: detecting keyring unavailability by exception alone genuinely can't catch a backend that's selected but blocks forever on D-Bus. Time-boxing every call via a daemon worker + join(timeout), treating the timeout as a KeyringError subclass so existing call sites need no new branching, and making the verdict process-sticky (with the short-circuit at the top of _keyring_call, not just in _check_keyring) is the right shape for something that can't be cancelled once it's parked in libdbus. delete() degrading on KeyringTimeoutError instead of raising is correct — a timeout isn't evidence the entry doesn't exist, it's evidence the backend is dead. Docs in CLAUDE.md are clear and the two new env vars are read at call time, consistent with the #963 lesson already documented elsewhere in this file.

Test coverage is thorough: both failure shapes (backend selection itself hanging vs. a selected backend hanging per-call), the sticky-verdict short-circuit (including the "stale store built before the timeout" case, which is the subtlest part of this fix), the escape hatch, the endpoint, and a regression check that a healthy backend is still preferred. The test_keyring_availability_check rewrite is the right call — the old assertion (store._keyring_available or not KEYRING_AVAILABLE) was literally the "importable implies available" assumption that caused #1181. The conftest.py autouse reset avoids the order-dependent leak this would otherwise cause across the suite.

Findings (non-blocking)

1. GET /api/v2/settings/keys can still block the actual asyncio event loop once per process, not just a worker thread. get_credential_manager_readonly is a plain def dependency, so FastAPI runs construction (and thus the first _check_keyring timeout) in a threadpool. But list_key_status is async def and calls _build_status → manager.get_credential_source/get_credential → store.retrieve directly in the route body, not through another threadpooled dependency. If backend selection succeeds fast but get_password itself hangs (plausible — SecretService can select instantly and only block on the actual session/read), the first provider's retrieve() call spends up to CODEFRAME_KEYRING_TIMEOUT (default 2s) blocking the event loop directly, before the sticky flag kicks in for the remaining providers in the same request. Bounded and one-time per process (much better than the infinite hang this fixes), but on a single-worker deployment that's up to 2s where every concurrent request stalls, not just the one hitting the dead keyring. Worth a follow-up to wrap the retrieve loop in asyncio.to_thread, or at least worth knowing this isn't fully non-blocking yet.

2. Minor doc-accuracy nit: the comment/PR description claim of "at most one abandoned thread per process" assumes the sticky flag is set before a second caller starts its own _keyring_call. Under genuine concurrency (e.g. two requests arriving before the first timeout resolves), both can start their own blocking worker and each pay the full timeout before the flag propagates — so it's "at most one per already-observed-timeout caller," not a hard global cap. Not a bug, just slightly overstated.

3. Minor: an unparseable CODEFRAME_KEYRING_TIMEOUT silently falls back to the 2.0s default with no logged warning, unlike other config parsing in this module. Low priority, but a typo'd env var currently fails silently.

None of these block the fix — the core problem (unbounded hang) is solved, and the design correctly treats timeout as a first-class KeyringError case throughout store/retrieve/delete.

[frankbria]

Third-party review (post-PR) — codex review

opencode/GLM is the documented primary but has a standing record on this repo of mutating the tree and timing out at zero bytes, so codex review was used for both the pre-PR and post-PR passes.

Post-PR finding — [P2] Serialize first keyring timeout attempt (credentials.py):

When multiple request threads hit credentials before any timeout verdict is set, they can all pass the _KEYRING_TIMED_OUT check and each start an unkillable daemon keyring worker before the first one flips the sticky flag. This means a burst of concurrent GET /api/v2/settings/keys calls on a headless SecretService host still pays the timeout per in-flight request and can leak one blocked thread per request, so the sticky flag does not actually bound the failure to one abandoned keyring call per process.

Correct, and the realistic shape for that endpoint. Fixed in 10a03a1 with a process-wide lock around the wrapper: the losers wait on it and then see the verdict the winner set. A healthy keyring call is milliseconds and credential reads are rare, so the contention cost is nil.

Pinned by test_that_concurrent_first_callers_start_only_one_worker. Mutation-checked — remove the lock and it fails with AssertionError: leaked 8 blocked keyring threads; restore it and it passes. Eight concurrent callers now share one timeout (0.30s at the test's 0.3s bound), not eight.

The two pre-PR P2s were fixed before the PR opened and are described in the PR body.

---

Demo — Phase 11, outcome evidence

Not a fake: demo/blocking_backend.py subclasses keyring.backend.KeyringBackend with priority = 99 and time.sleep(86400) in every method, then keyring.set_keyring() selects it — the same shape as SecretService parked on a dead D-Bus. Real CredentialManager, real router, no mocks in the credential path.

Before — the same script on main:

$ cd <worktree of main> && timeout 45 uv run python demo/ac1_cli.py
Terminated
EXIT=143   # SIGTERM at 45s; still parked in the D-Bus call

AC1 — degrades to the encrypted file store within a bounded time:

$ timeout 90 uv run python demo/ac1_cli.py
WARNING  Keyring backend did not respond within 2.0s; falling back to the encrypted
         credential file for the rest of this process. Set CODEFRAME_DISABLE_KEYRING=1
         to skip the keyring entirely.
selected backend: BlockingBackend
write+read round trip: 2.06s
value round-tripped:   'sk-ant-demo-value'
landed in:             ['credentials.encrypted', 'salt']
AC1 PASS

The outcome is not "it returned" — the credential was written, persisted to credentials.encrypted, and read back byte-identical while the selected backend was still asleep. 2.06s for the round trip, i.e. one timeout total, not one per operation.

AC2 — GET /api/v2/settings/keys returns rather than hanging:

$ timeout 120 uv run python demo/ac2_endpoint.py
WARNING  Keyring backend did not respond within 2.0s; falling back ...
HTTP 200 in 2.01s
body: [{'provider': 'LLM_ANTHROPIC', 'stored': False, 'source': 'none', 'last_four': None},
       {'provider': 'LLM_OPENAI', ...}, {'provider': 'GIT_GITHUB', ...}]
second request: HTTP 200 in 0.00s   (sticky verdict — paid once per process, not per request)
AC2 PASS

A correct 200 with a correctly-populated body, then 0.00s on the second request. That second line is the sticky verdict doing its job: this endpoint used to hang a request thread indefinitely.

AC3 — a test pins the timeout path with a deliberately blocking fake backend:
tests/core/test_keyring_timeout_1181.py, 13 tests, two distinct failure shapes (blocking credential ops vs. blocking backend selection). 207 passed across every credential-touching suite.

AC4 — the workaround is documented: CLAUDE.md env block now carries CODEFRAME_DISABLE_KEYRING, CODEFRAME_KEYRING_TIMEOUT, and the PYTHON_KEYRING_BACKEND equivalent, with the cause written down so the next person does not lose an hour to it.

The gate the issue reported as wedging:

$ env -u PYTHON_KEYRING_BACKEND uv run pytest tests/ --ignore=tests/e2e -m "not lifecycle" -q
31 failed, 6485 passed, 19 skipped, 2 deselected in 445.36s (0:07:25)

It completes. All 31 failures reproduce identically on main in a clean worktree (31 failed in 7.06s) — ANSI/terminal-width assertions in CLI output tests, an ambient property of this machine, untouched by this diff.

https://claude.ai/code/session_011pR6Wqk15b3ESqXDiheNMZ

[github-actions]

Precision bug-hunting review in progress

Reviewing the updated #1184 diff (commit 30cd200, the new serialization lock) for concrete defects only — logic errors, hangs/races/deadlocks, data loss, security.

• Fetch PR diff and new-commit changes
• Read codeframe/core/credentials.py in full
• Read new/changed tests and tests/conftest.py
• Check all keyring call sites inside and outside credentials.py for unwrapped blocking paths
• Verify the new lock for deadlock/re-entrancy against real call sequences (store() retry, concurrent callers)
• Verify env-var parsing, sticky-verdict, and fallback semantics against real callers
• Post inline findings + summary

View job run

[claude]

Follow-up review after the concurrency fix (10a03a1/30cd200f) and the reply to the codex review P2.

Recap of prior review

I reviewed this PR earlier in this thread. Status of those findings on the current diff:

1. Event loop blocking in GET /api/v2/settings/keys — still open, unaddressed, non-blocking. settings_v2.list_key_status (codeframe/ui/routers/settings_v2.py:290-295) is async def and calls _build_status() directly in the route body — not through a threadpooled dependency. _build_status → manager.get_credential_source/get_credential → store.retrieve() → _keyring_call(), which does a blocking worker.join(_keyring_timeout()) on the calling thread. get_credential_manager_readonly (the Depends) is a plain def, so FastAPI runs construction in a threadpool — but the subsequent retrieve() calls inside the async handler run straight on the event loop thread. So the first request against a backend that selects fast but hangs on get_password still blocks the whole event loop for up to CODEFRAME_KEYRING_TIMEOUT (2s default), stalling every other concurrent request on a single-worker deployment — not just the caller. This is exactly the class of bug the PR fixes for thread-level hangs, just one layer up. Worth a fast follow-up: wrap the _build_status loop (or the route) in asyncio.to_thread.

2. Concurrent-first-caller race — fixed. The _KEYRING_CALL_LOCK added in 30cd200f correctly serializes the first-timeout race the codex review P2 flagged, and test_that_concurrent_first_callers_start_only_one_worker mutation-checks it (confirmed by the PR author: removing the lock fails the assertion, restoring it passes). No concerns with the fix itself — a process-wide lock around keyring calls is a reasonable and cheap trade-off since the fast path is milliseconds and reads are rare. One second-order consequence worth being aware of (not a bug): if a healthy backend blocks on a real unlock prompt, every other credential access in the process now queues behind that prompt until it resolves or the timeout trips it into the sticky-dead state — previously unrelated callers could at least race independently. Given local single-operator usage is the primary deployment target here, this is an acceptable trade-off, just flagging it as a behavior change from "each caller pays its own timeout" to "one caller's slow prompt stalls everyone."

3. Silent fallback on unparseable CODEFRAME_KEYRING_TIMEOUT — still present, low priority. _keyring_timeout() catches ValueError and falls back to the 2.0s default with no log line, unlike the pattern of loud config-validation elsewhere in this file. Not worth blocking on.

New check on this pass

• Confirmed no other call sites touch the keyring module directly outside codeframe/core/credentials.py (grep -rn "keyring\." across codeframe/), so the wrapper's coverage is complete — there's no bypass route left unwrapped.
• Verified _keyring_call_locked's sticky short-circuit and the lock don't introduce a reentrancy/deadlock risk — _keyring_call is never called recursively from within itself, and the worker thread never touches the lock.
• The two-commit history (30cd200f serialize, f2a5b6a9 bound-by-time) matches the PR description's account of the pre/post-PR codex review passes; nothing outside that scope has landed since.

Nothing here blocks merge. Item 1 is the only one I'd actually want tracked as a fast follow-up (a small, mechanical asyncio.to_thread wrap), since it's the same underlying failure mode this PR exists to fix, just surfacing at the event-loop level instead of the thread level.

[frankbria]

Review triage — both remaining claude-review findings fixed

1. GET /api/v2/settings/keys blocked the event loop, not just a worker thread — FIXED.

Correct and worth fixing here rather than deferring: it is the same failure this PR exists to bound, one layer up. list_key_status is async def and called _build_status directly in the route body, so worker.join(timeout) ran on the loop thread — a backend that selects fast but hangs on get_password would stall every in-flight request for 2s, not just the caller.

Every credential call in an async handler now goes through run_in_threadpool, already this file's pattern for the blocking provider-verification calls: _build_status in both the list and store routes, set_credential, delete_credential, and the get_credential in the verify route.

test_that_the_event_loop_stays_responsive_during_the_timeout drives the real route through an ASGI client with a heartbeat task ticking every 20ms. Mutation-checked:

# with the run_in_threadpool wrap removed
AssertionError: event loop was blocked (only 0 ticks)
# restored
1 passed

2. Silent fallback on an unparseable CODEFRAME_KEYRING_TIMEOUT — FIXED. It now logs the value it ignored and the default it used.

On the noted behavior change (not a finding): yes — with the lock, a healthy backend sitting on a real unlock prompt now queues other credential accesses behind it rather than letting them race independently. That is the deliberate trade: unbounded independent racing is exactly what leaked a thread per request. If the prompt resolves, everyone proceeds; if it does not, the timeout trips it into the sticky-dead state and everyone proceeds anyway. Bounded either way, which the previous behavior was not. Noted in Known Limitations.

232 passed across every credential-touching suite; ruff check . clean.

[github-actions]

Precision bug-hunting review in progress

Reviewing the updated #1184 diff (commit eadb80e — event-loop offload in settings_v2 + timeout-parse logging) for concrete defects only: logic errors, hangs/races/deadlocks, data loss, security.

• Fetch PR diff and new-commit changes
• Read codeframe/core/credentials.py in full
• Read codeframe/ui/routers/settings_v2.py and audit the run_in_threadpool wraps
• Audit every other credential touchpoint (async routes outside settings_v2, CLI, agent paths) for unwrapped blocking paths
• Verify lock / sticky-verdict / fallback semantics against real callers
• Post inline findings + summary

View job run

[claude]

Review of the current HEAD (eadb80e)

This is a re-review of the final state after the two follow-up commits. I reviewed this PR earlier in the thread and flagged three items; status on the current diff:

1. Event-loop blocking in GET /api/v2/settings/keys, PUT/DELETE /keys/{provider}, POST /verify-key — fixed. Every credential call reachable from an async def route now goes through run_in_threadpool (settings_v2.py:299-301, 327, 338-340, 357, 466). Confirmed the two dependencies that construct CredentialManager (get_credential_manager, get_credential_manager_readonly) are plain def, so FastAPI already threadpools construction — the gap was specifically the _build_status/set_credential/delete_credential/get_credential calls made directly in async route bodies, and that gap is now closed. Also verified via grep -rn "keyring\." that codeframe/core/credentials.py is the only module touching the keyring package directly, so there's no other unwrapped call site.
2. Concurrent-first-caller race (the codex review P2) — fixed and mutation-tested. _KEYRING_CALL_LOCK correctly bounds a burst to one paid timeout; _keyring_call_locked's sticky short-circuit happens inside the lock so there's no TOCTOU window between the check and the flag write. No reentrancy risk — the worker thread never touches the lock, and _keyring_call isn't called recursively.
3. Silent fallback on an unparseable CODEFRAME_KEYRING_TIMEOUT — fixed. _keyring_timeout() now logs a warning naming the bad value and the default used, consistent with the loud-validation pattern elsewhere in this file.

New checks on this pass

• store()'s cleanup path (set_password times out → cleanup delete_password) correctly avoids paying a second timeout: once _KEYRING_TIMED_OUT is set, the cleanup call short-circuits inside the lock instead of spawning another blocked worker.
• KeyringTimeoutError subclassing KeyringError is exercised correctly at every call site's exception handling — retrieve()/delete() special-case it to flip _keyring_available = False rather than falling into the generic except Exception, which matters because delete()'s generic handler re-raises while the KeyringTimeoutError handler deliberately degrades.
• pytest.ini has asyncio_mode = auto, so test_that_the_event_loop_stays_responsive_during_the_timeout (using a bare @pytest.mark.asyncio) runs correctly.
• The behavior-change note the author added (a real backend blocked on an unlock prompt now stalls other credential accesses behind the lock, not just independent per-caller timeouts) is accurately described as a deliberate, documented trade-off rather than a regression — bounded-but-shared beats unbounded-but-independent.

Non-blocking observations

• _keyring_call_locked calls _keyring_timeout() twice on the timeout path (once for worker.join(), once for the warning log). Environment variables essentially never change mid-call, so this isn't a real race, just a redundant read — not worth a follow-up on its own.
• The threadpool trade-off: under a burst of concurrent GET /keys requests against a genuinely hung backend, each request now occupies a Starlette threadpool worker while it queues on _KEYRING_CALL_LOCK (only one actually runs the blocking join; the rest wait on the lock from within their own threadpool slot). That's bounded by the default threadpool size rather than unbounded, and is an inherent consequence of moving blocking work off the loop — not something to fix here, just worth knowing if this is ever deployed multi-tenant instead of single-operator.

Nothing here blocks merge. Test coverage (13 tests across both failure shapes, the sticky-verdict edge cases, the escape hatch, and the endpoint/event-loop tests) is thorough and each new assertion is mutation-checked per the PR description. Good fix for a real, previously-unbounded hang.

[github-actions]

Precision bug-hunting review in progress

Reviewing the updated #1184 diff (commit 0e3d018 — timeout now read once per call) for concrete defects only: logic errors, hangs/races/deadlocks, data loss, security.

• Fetch PR diff and the new-commit delta
• Read codeframe/core/credentials.py in full
• Audit the once-per-call timeout read for behavior changes (sticky path, warning log, env parsing)
• Re-check lock/sticky/fallback semantics and the test fixtures against real callers
• Check remaining credential touchpoints (async routes outside settings_v2) for unwrapped blocking paths
• Post inline findings + summary

View job run

[claude]

Reviewed the final state (HEAD 0e3d018). This PR has already been through several thorough automated review rounds in this thread, and every substantive finding raised (event-loop blocking in settings_v2 routes, the concurrent-first-caller thread leak, silent fallback on a bad CODEFRAME_KEYRING_TIMEOUT, and the double-read of the timeout env var) has been fixed and verified in later commits. I don't have new findings to add on top of that history — this comment is a confirmation pass, not a re-review from scratch.

Specific things I checked independently:

• get_credential_manager / get_credential_manager_readonly are plain def dependencies (settings_v2.py:84, :114), so FastAPI already threadpools CredentialManager.__init__ — including _check_keyring() and, on the write path, _migrate_machine_wide_entries(), which loops retrieve() over every CredentialProvider. Combined with the run_in_threadpool wraps added around _build_status/set_credential/delete_credential/get_credential in the route bodies, there's no remaining path from an async def route to a blocking keyring call on the event loop thread.
• store()'s cleanup path is safe from a double timeout. When set_password times out, _KEYRING_TIMED_OUT is already flipped (inside the lock) before KeyringTimeoutError propagates, so the subsequent cleanup delete_password call short-circuits immediately in _keyring_call_locked instead of spawning a second blocked worker.
• KeyringTimeoutError subclassing KeyringError is handled correctly at each call site — retrieve()/delete() special-case it to flip _keyring_available = False and degrade, rather than falling into a generic handler that (in delete()'s case) re-raises.
• grep -rn "keyring\." across codeframe/ confirms credentials.py is the only module touching the keyring package directly, so the wrapper's coverage is complete.

Code quality: the module docstring, CLAUDE.md env-var block, and inline comments on _keyring_call/_keyring_call_locked clearly explain why (unkillable libdbus call, sticky-per-process verdict, lock needed for the verdict to mean anything under concurrency), which matches this repo's convention of comments justifying non-obvious behavior rather than restating code.

Test coverage is strong: both failure shapes (selection-time hang vs. per-call hang), the sticky-verdict short-circuit including the "stale store built before the timeout" case, the concurrent-burst leak (mutation-checked per the PR description), the escape hatch, and the event-loop-responsiveness test via a real ASGI client with a heartbeat task. The test_keyring_availability_check rewrite correctly removes the "importable implies available" assumption that caused the original bug.

No blocking issues. This is a solid, well-tested fix for a real unbounded-hang bug, and the known limitations section in the PR body honestly documents the remaining trade-offs (per-process sticky verdict, lock changes contention shape for a healthy-but-slow backend) rather than hiding them.