feat: use cache key to persist klavis client

[Dani Akash (DaniAkash)]

This pull request adds caching and request deduplication to the createStrata method in the KlavisClient class to improve efficiency and prevent redundant network calls. Now, repeated or concurrent requests for the same user and server combination within a 5-minute window will return cached results or share the same in-flight request.

Caching and deduplication improvements:

• Added a strataCache with a 5-minute TTL to store StrataCreateResponse objects, preventing unnecessary API calls for the same (userId, servers) combination. [1] [2]
• Implemented a pendingRequests map to deduplicate concurrent requests for the same key, ensuring only one network request is made and shared among callers. [1] [2]
• Introduced a cache key generation method (buildStrataCacheKey) that uniquely identifies requests by userId and a sorted list of servers.
• Updated the createStrata method to use the new caching and deduplication logic, and to clean up pending requests after completion or error.

[greptile-apps]

Greptile Summary

This PR adds TTL-based response caching (5-minute window) and concurrent-request deduplication to KlavisClient.createStrata, reducing redundant network calls for the same (userId, servers) combination. The buildStrataCacheKey helper correctly normalises the key using JSON.stringify([userId, ...[...servers].sort()]), addressing both the separator-collision risk and the in-place mutation of the callers's array that were flagged in earlier review threads.

Two concerns remain:

• Incomplete cache invalidation in removeServer — only the exact single-server cache key is cleared. Any cached multi-server strata entry that includes the removed server is left untouched and will be served stale until its TTL expires.
• No active cache eviction — expired strataCache entries are never swept; they accumulate in memory for the lifetime of the process, which can be a concern in long-running server environments with many unique user/server combinations.

Confidence Score: 3/5

• Safe to merge with low risk, but the incomplete cache invalidation in removeServer may surface stale data in multi-server strata scenarios.
• The caching and deduplication logic is well-structured and the previously flagged issues (key collision, array mutation) are correctly resolved. The remaining concerns — partial cache invalidation on removal and unbounded cache growth — are real but bounded in impact: the first is limited to multi-server strata combinations and the second is a memory pressure concern on long-running instances rather than a correctness bug in most common flows.
• packages/browseros-agent/apps/server/src/lib/clients/klavis/klavis-client.ts — specifically the removeServer cache invalidation and the absence of a cache eviction strategy.

Important Files Changed

Filename	Overview
packages/browseros-agent/apps/server/src/lib/clients/klavis/klavis-client.ts	Adds TTL-based caching and concurrent-request deduplication to createStrata. The buildStrataCacheKey correctly avoids separator collisions (JSON.stringify) and mutation (copies array before sorting). Two issues remain: removeServer only invalidates the single-server cache key rather than all entries containing the removed server, and expired cache entries are never actively evicted from memory.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant createStrata
    participant strataCache
    participant pendingRequests
    participant KlavisAPI

    Caller->>createStrata: createStrata(userId, servers)
    createStrata->>createStrata: buildStrataCacheKey(userId, servers)
    createStrata->>strataCache: get(cacheKey)

    alt Cache hit (not expired)
        strataCache-->>createStrata: CachedStrata
        createStrata-->>Caller: return cached response
    else Cache miss
        createStrata->>pendingRequests: get(cacheKey)
        alt In-flight request exists
            pendingRequests-->>createStrata: existing Promise
            createStrata-->>Caller: return shared Promise
        else No pending request
            createStrata->>KlavisAPI: POST /mcp-server/strata/create
            createStrata->>pendingRequests: set(cacheKey, promise)
            KlavisAPI-->>createStrata: StrataCreateResponse
            createStrata->>strataCache: set(cacheKey, {response, expiresAt})
            createStrata->>pendingRequests: delete(cacheKey)
            createStrata-->>Caller: return response
        end
    end

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/lib/clients/klavis/klavis-client.ts
Line: 197

Comment:
**Cache invalidation only clears the single-server entry**

`removeServer` invalidates only the exact `(userId, [serverName])` cache key. However, if `createStrata` was previously called with a multi-server combination that includes `serverName` (e.g., `createStrata(userId, ['serverA', 'serverB'])`), that cache entry retains the old `StrataCreateResponse` — including the now-stale `addedServers` list and OAuth/API-key URLs.

Depending on whether the Klavis API considers each server-combination a separate strata object or shares one physical strata per user, the multi-server cache entry may reflect an inconsistent server state after `removeServer` is called. At a minimum, any entry whose server list contains `serverName` should be invalidated:

```typescript
// Instead of a single targeted delete, clear all entries for this user
// that contain the removed server
for (const [key, _] of this.strataCache) {
  const parsed: [string, ...string[]] = JSON.parse(key)
  if (parsed[0] === userId && parsed.slice(1).includes(serverName)) {
    this.strataCache.delete(key)
  }
}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: packages/browseros-agent/apps/server/src/lib/clients/klavis/klavis-client.ts
Line: 38

Comment:
**No active eviction — expired entries accumulate indefinitely**

`strataCache` is a plain `Map` with no background sweep. Expired entries (past `expiresAt`) are only displaced when `createStrata` is called again for the exact same key. In a long-running server process with many unique `(userId, servers)` combinations, stale entries will accumulate in memory until the process restarts.

Consider a lightweight eviction approach: either schedule a periodic cleanup, or evict inline on every `createStrata` call:

```typescript
private evictExpiredStrataCache(): void {
  const now = Date.now()
  for (const [key, entry] of this.strataCache) {
    if (entry.expiresAt <= now) {
      this.strataCache.delete(key)
    }
  }
}
```

Call `this.evictExpiredStrataCache()` at the top of `createStrata` before the cache lookup to keep memory bounded without requiring a timer.

How can I resolve this? If you propose a fix, please make it concise.

_{Last reviewed commit: "fix: code reviews"}

[Dani Akash (DaniAkash)]

All three findings addressed:

1. Stale cache after removeServer — Fixed. Cache entry is now invalidated after a successful DELETE.
2. Cache key collision — Fixed. Cache key now uses JSON.stringify([userId, ...servers.sort()]) instead of string concatenation with separators.
3. Unbounded cache growth — Not applicable. This is a single-user desktop app with one userId and a handful of server combinations. The cache will have at most a few entries.

[Dani Akash (DaniAkash)]

Claude (@claude) Greptile (@greptileai)

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[Dani Akash (DaniAkash)]

Claude (@claude) Greptile (@greptileai)

[claude]

Claude encountered an error —— View job

---

I'll analyze this and get back to you.

[Dani Akash (DaniAkash)]

The caching layer didn't add significant performance gains - so closing the ticket