Cache OpenSecret clients per API key

[AnthonyRonning]

Closes #32.

Summary:

• Cache OpenSecret client initialization per API key in ProxyState.
• Use async OnceCell so concurrent first requests for the same key share one handshake.
• Add tests for per-key cache cell reuse and separation.

Tests:

• cargo test
• cargo clippy --all-targets -- -D warnings

---

Summary by CodeRabbit

• Refactor
  • Request handling now reuses backend clients per API key with cached entries, TTL-based expiry and capacity-based eviction to reduce repeated initialization.
• Tests
  • Added unit tests covering client reuse, isolation between API keys, eviction and replacement of expired or removed entries.
• Chores
  • Added dashmap dependency and expanded tokio feature set.

[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: 7e274128-7ad7-4ff1-b5f0-be3ba7bef5d4

📥 Commits

Reviewing files that changed from the base of the PR and between 811fa2c and c5e06e8.

📒 Files selected for processing (1)

• src/proxy.rs

---

📝 Walkthrough

Walkthrough

ProxyState adds a DashMap + OnceCell per-API-key cache for OpenSecretClient with TTL and size eviction. Request handlers fetch cached clients via state.client_for_api_key(&api_key). Tests cover reuse, key isolation, eviction, expiration, and removal on failed init.

Changes

Per-API-Key Client Caching

Layer / File(s)	Summary
Client cache infrastructure Cargo.toml, src/proxy.rs	Adds tokio sync feature and dashmap = "6.1"; implements CachedClientEntry (OnceCell + created_at), ProxyState.clients: DashMap, and async helpers for lazy client initialization, TTL-based expiration, and oldest-entry eviction at capacity.
Request handler integration src/proxy.rs	list_models, create_chat_completion, and create_embeddings now obtain backend clients via state.client_for_api_key(&api_key) instead of constructing new clients per request.
Cache behavior tests src/proxy.rs	Unit tests expanded to assert same-key reuse, distinct-key separation, eviction when cache is full, replacement of expired entries, and removal of entries when initialization is removed.

sequenceDiagram
  participant Client as RequestHandler
  participant State as ProxyState
  participant Cell as OnceCell
  participant Backend as OpenSecretClient

  Client->>State: client_for_api_key(api_key)
  State->>Cell: get_or_try_init(init_client)
  Cell->>Backend: initialize OpenSecretClient (attestation)
  Note over State,Cell: cached Arc<OpenSecretClient> stored in DashMap with created_at
  Client->>Backend: send request using cached client

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐰
I stash clients where secrets sleep,
One per key, so handshakes sleep deep.
Old ones tumble, fresh ones bloom,
Fast hops forward — less attestation gloom. 🥕

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title 'Cache OpenSecret clients per API key' directly and clearly summarizes the main change across the codebase: implementing caching of OpenSecretClient instances keyed by API key.
Linked Issues check	✅ Passed	The PR successfully implements all coding requirements from #32: client caching per API key [#32], lazy initialization via OnceCell [#32], per-key reuse [#32], cache size limits and TTL expiration [#32], and removal of entries on failed initialization [#32].
Out of Scope Changes check	✅ Passed	All changes are directly scoped to implementing the client caching mechanism: dependency additions (dashmap, tokio sync) [#32], ProxyState caching infrastructure [#32], lazy client initialization [#32], and cache management with tests [#32].

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

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/cache-opensecret-clients

---

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

[coderabbitai]

🧹 Nitpick comments (1)

src/proxy.rs (1)

32-37: 💤 Low value

Cache has no eviction policy.

The clients map grows unboundedly as new API keys are seen. For typical deployments with a bounded set of API keys this is fine, but if the proxy serves many ephemeral/rotating keys over time, entries will accumulate indefinitely.

Consider whether you need cache eviction (e.g., TTL-based or LRU) in the future. For now, this is likely acceptable if the set of API keys is relatively stable.

🤖 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/proxy.rs` around lines 32 - 37, The current client_cell_for_api_key
creates entries in self.clients with no eviction, causing unbounded growth as
new api_keys appear; replace the backing map with a bounded, thread-safe cache
(e.g., moka::sync::Cache or lru crate wrapped for concurrency) or add TTL-based
cleanup so entries are evicted automatically, and update client_cell_for_api_key
to look up/insert using the cache's API (preserving the
Arc<OnceCell<Arc<OpenSecretClient>>> value type) so concurrent access semantics
remain correct; ensure the chosen cache is safe for Arc values and update any
initialization and imports accordingly.

🤖 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/proxy.rs`:
- Around line 32-37: The current client_cell_for_api_key creates entries in
self.clients with no eviction, causing unbounded growth as new api_keys appear;
replace the backing map with a bounded, thread-safe cache (e.g.,
moka::sync::Cache or lru crate wrapped for concurrency) or add TTL-based cleanup
so entries are evicted automatically, and update client_cell_for_api_key to look
up/insert using the cache's API (preserving the
Arc<OnceCell<Arc<OpenSecretClient>>> value type) so concurrent access semantics
remain correct; ensure the chosen cache is safe for Arc values and update any
initialization and imports accordingly.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6425744a-185e-4305-a94d-2179d81e097a

📥 Commits

Reviewing files that changed from the base of the PR and between 2984f95 and 811fa2c.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• Cargo.toml
• src/proxy.rs

[AnthonyRonning]

Follow-up after validating the bot feedback:

• Cache growth feedback was valid. Updated in c5e06e8: cache entries are capped, expire after one hour, and failed first-use attestation removes the empty cache entry so random invalid API keys do not accumulate.
• The stale-client / no-recovery feedback appears to be a false positive for the OpenSecret SDK path used here. OpenAI calls go through the SDK retry helpers, which rerun perform_attestation_handshake on session/API/decryption/encryption failures and update the same client's SessionManager. The SDK also has tests covering seamless refresh/retry behavior.

Revalidated locally:

• cargo test
• cargo clippy --all-targets -- -D warnings