[YellowSnnowmann]

Summary

• Composio activation survives restart — a durable, user-namespaced client cache seeds the last-known connected toolkits on mount, so a cold start no longer flashes an empty/disconnected Skills grid before the backend round-trip lands. Reconciled by the existing fetch/poll.
• Skill tool-chain latency is now measured against the 60s target — the chat runtime times each turn's tool chain and warns on overrun. Observability only; no timeout behaviour changed.
• Registry browse is cached client-side (in-memory, single-flight) — the ~90k-entry catalog is no longer re-pulled and re-parsed on every Skills-page mount within a session.
• Registry catalog is now paginated — a "Show more" control reveals the next 60 entries (the list was previously truncated to 60 with no way to see the rest). Source filtering now applies to the full list, not just the first page.
• E2E coverage for the headline activate → invoke-from-chat path and for activation-persistence-across-restart.

Problem

Issue #4273 (Skills page — end-to-end completion) called out gaps in the skill lifecycle: activation state not surviving restart, no E2E for the activate→invoke-from-chat path, and no visibility into the 60s latency target. Separately, the registry tab was slow (full ~90k-entry catalog re-fetched + re-parsed on every mount) and the catalog UI dead-ended at 60 entries with a "refine your search" hint but no pagination control.

Solution

• Persistence (AC1): app/src/lib/composio/connectionCache.ts — synchronous localStorage + in-memory mirror (mirrors the sibling catalogCache.ts), namespaced by OPENHUMAN_ACTIVE_USER_ID and inert when no user is identified (read→null, write→no-op) so two unidentified sessions can never share a blob. useComposioIntegrations seeds initial state from it and writes on every successful fetch/poll; evicted on direct-mode key removal and tenant switch. Not persisted as a Redux slice deliberately — coupling the Provider-less hook to the store would have broken its renderHook tests.
• Latency (AC3): app/src/lib/ai/skillToolChainLatency.ts — pure, injectable-clock tracker wired into ChatRuntimeProvider's existing onToolCall/onDone/onError. Non-enforcing: did not clamp the 120s tool timeout to 60s (that would cut off legitimately long chains = breaking).
• Browse cache (perf): skillRegistryApi.browse() gains an in-memory TTL cache + shared in-flight promise; force_refresh and a new invalidateSkillBrowseCache() drop it. Not localStorage — the payload far exceeds the ~5MB quota.
• Pagination (UX): SkillsExplorerTab retains the full fetched list and pages through it client-side via visibleCount (no extra RPC), replacing the contradictory "refine your search" copy with a common.showMore button (no new i18n keys).

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) — unit tests for cache (TTL, user-namespacing, malformed JSON, merge), latency tracker (overrun, error, multi-thread), browse cache (hit, single-flight, force-refresh, invalidate), pagination (60→120→130), hook seeding + reconcile; 2 E2E specs.
• Diff coverage ≥ 80% — run pnpm test:coverage locally to confirm before merge. New TS lines are covered by the added unit tests; the 2 E2E specs do not count toward diff-cover.
• Coverage matrix updated — TODO: add/confirm rows for skill activation persistence + registry pagination in docs/TEST-COVERAGE-MATRIX.md (latency is observability-only → N/A).
• No new external network dependencies introduced — client-only cache; E2E uses the shared mock backend.
• Manual smoke checklist updated if this touches release-cut surfaces — author to confirm: Skills page is user-facing; verify docs/RELEASE-MANUAL-SMOKE.md.
• Linked issue closed via Closes #NNN in the ## Related section

Impact

• Platform: Desktop (Tauri + React). No Rust/core changes in this PR.
• Performance: Repeat Skills-page visits no longer re-transfer/re-parse the full registry catalog; restart no longer flashes a disconnected grid.
• Security: Connection cache is user-namespaced and inert without an identified user — no cross-user leak. No secrets stored (connection status/toolkit slugs only).
• Migration/compat: None. Additive; empty cache → identical to prior behaviour.

Related

• Closes: Skills page — end-to-end completion #4273
• Follow-up PR(s)/TODOs: server-side limit/offset on skill_registry_browse; install-without-full-catalog-refetch; poll-pause-on-tab-hidden; search() result cache; tile virtualization; clear connection cache on logout.

Summary by CodeRabbit

• New Features

  • Registry skills now support on-demand paging with a Show more control and a shown / total count.
  • Connected integrations now restore cached state on load and keep skill activations available after restart-like remounts.

• Bug Fixes

  • Improved reliability of registry and integration data loading across refreshes, reconnects, and temporary backend failures.
  • Skill tool-chain latency is more accurately tracked per request, with warnings when exceeding the 60s target and correct handling on failures.

[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: 066d262a-1408-4c0b-8b18-67acc00cc75e

📥 Commits

Reviewing files that changed from the base of the PR and between a3548ef and ddc4d1d.

📒 Files selected for processing (1)

• app/test/e2e/specs/skill-activation-persistence.spec.ts

🚧 Files skipped from review as they are similar to previous changes (1)

• app/test/e2e/specs/skill-activation-persistence.spec.ts

---

📝 Walkthrough

Walkthrough

Adds durable Composio connection caching, skill browse caching, client-side catalog pagination, per-thread skill tool-chain latency tracking, and E2E coverage for activation persistence and chat invocation.

Changes

Durable Composio Connection Cache

Layer / File(s)	Summary
connectionCache module: interface, read, write, clear app/src/lib/composio/connectionCache.ts	Defines CachedConnectionState, per-user localStorage keying, TTL logic, an in-memory mirror, and exported read/write/clear functions.
connectionCache unit tests app/src/lib/composio/connectionCache.test.ts	Covers null on empty, round-trip write/read, partial merge, clear, TTL expiry, per-user namespacing, malformed JSON, key-prefix purge, expired toolkit resurrection prevention, and simulated cold restart rehydration.
useComposioIntegrations: seed from cache and persist on fetch app/src/lib/composio/hooks.ts	Seeds initial React state from cache, persists connections/toolkits/catalog on successful fetch and polling, clears cache on config-changed and no-API-key paths, and guards stale async results via configGenerationRef.
useComposioIntegrations cache integration tests app/src/lib/composio/hooks.test.ts	Two tests cover immediate non-loading seeded state and stale-connection removal after live fetch.
E2E: activation persistence across restart under backend fault app/test/e2e/specs/skill-activation-persistence.spec.ts	Seeds an ACTIVE connection, populates the durable cache, injects backend fault, simulates remount, and asserts the Gmail connector card still renders from cache.

Skill Registry Browse Cache and Client-Side Pagination

Layer / File(s)	Summary
skillRegistryApi: in-memory browse cache with TTL and in-flight dedup app/src/services/api/skillRegistryApi.ts	Adds browseCache, browseInflight, invalidateSkillBrowseCache(), and rewrites browse() to serve from cache, join in-flight requests, or fetch fresh.
skillRegistryApi browse cache unit tests app/src/services/api/skillRegistryApi.test.ts	Covers cache hit, concurrent dedup, forceRefresh bypass, and explicit invalidation with cold-cache isolation.
SkillsExplorerTab: client-side Show more pagination app/src/components/skills/SkillsExplorerTab.tsx, app/src/components/skills/__tests__/SkillsExplorerTab.test.tsx	Adds visibleCount, stores the full catalog, derives filteredCatalog, slices displayedCatalog, updates the empty state, and renders a Show more control with shown/total count.

Skill Tool-Chain Latency Tracker

Layer / File(s)	Summary
skillToolChainLatency: types, interface, and implementation app/src/lib/ai/skillToolChainLatency.ts	Defines SKILL_TOOL_CHAIN_TARGET_MS, ToolChainMeasurement, SkillToolChainLatencyTracker, and createSkillToolChainLatencyTracker with per-thread Map-based state.
skillToolChainLatency unit tests app/src/lib/ai/skillToolChainLatency.test.ts	Tests null on no tool call, elapsed/count/withinTarget, overrun, boundary, ok:false, per-thread independence, no double-finish, and reset.
ChatRuntimeProvider: latency tracking on onToolCall/onDone/onError app/src/providers/ChatRuntimeProvider.tsx	Creates skillLatencyRef, calls noteToolCall on tool call, finishChain(ok:true) with warning on overrun on done, finishChain(ok:false) on error, and reset on socket disconnect.
ChatRuntimeProvider latency tests app/src/providers/__tests__/ChatRuntimeProvider.test.tsx	Three tests cover warning on >60s chain, no warning within target, and no warning on error path.
E2E: skill activate → invoke from chat → latency assertion app/test/e2e/specs/skill-activate-invoke-chat.spec.ts	Forces a two-turn mock LLM with a composio_execute tool call and a canary reply, then asserts the tool call and final assistant response render within the latency target.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Suggested labels

feature

Suggested reviewers

• oxoxDev

Poem

🐇 I hopped through caches, quick as spring,
"Show more" buttons made the catalog sing.
From chat to skill and back again,
This bunny tracked the chain with zen.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title concisely covers the main additions: persistence, latency instrumentation, and Skills registry browsing improvements.
Linked Issues check	✅ Passed	The PR addresses the linked Skills page goals with persistence, chat execution, latency tracking, browse pagination/cache, and E2E coverage.
Out of Scope Changes check	✅ Passed	The changes stay within Skills discovery, persistence, chat execution, latency, and test coverage, with no unrelated features evident.
Docstring Coverage	✅ Passed	Docstring coverage is 80.00% which is sufficient. The required threshold is 80.00%.

---

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

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e35b767898

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

[coderabbitai]

Caution

Failed to replace (edit) comment. This is likely due to insufficient permissions or the comment being deleted.

Error details

{}

[coderabbitai]

Actionable comments posted: 8

🧹 Nitpick comments (1)

app/src/services/api/skillRegistryApi.test.ts (1)

168-221: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Solid cache coverage; consider two gaps.

The cache-hit, concurrent-dedup, force-refresh and invalidation cases are well covered. Two behaviors documented on the cache remain untested and would be cheap to add with fake timers / a rejected mock:

• TTL expiry: after advancing past BROWSE_CACHE_TTL_MS (use vi.useFakeTimers() + vi.advanceTimersByTime), the next browse() should re-fetch.
• Failure isolation: a rejected callCoreRpc should leave the cache empty (and clear browseInflight) so the next call retries rather than serving a poisoned/empty result.

🤖 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 `@app/src/services/api/skillRegistryApi.test.ts` around lines 168 - 221, The
cache tests in skillRegistryApi.test.ts cover hits, concurrent dedupe, force
refresh, and invalidation, but they still miss TTL expiry and failure recovery.
Add a test around skillRegistryApi.browse using fake timers to advance past
BROWSE_CACHE_TTL_MS and assert the next call re-fetches, and add a rejected
callCoreRpc case to verify browse does not retain a poisoned cache entry and
that browseInflight is cleared so a later browse retries. Use the existing
browse and invalidateSkillBrowseCache helpers and the callCoreRpc mock to locate
the behavior under test.

🤖 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.

Inline comments:
In `@app/src/lib/ai/skillToolChainLatency.ts`:
- Around line 26-27: The `toolCount` field in `SkillToolChainLatency` is
mislabeled because `noteToolCall()` only keys off `threadId`, so it tracks total
calls per thread rather than distinct tools. Update the contract to match the
behavior by either renaming `toolCount` and the related
`noteToolCall()`/consumer usage to reflect “tool call count,” or extend the
tracking to accept a tool identifier and de-duplicate if distinct-tool counting
is intended. Make sure the `SkillToolChainLatency` class and any downstream
logging/telemetry code use the same meaning consistently.

In `@app/src/lib/composio/connectionCache.ts`:
- Around line 100-104: `readConnectionCache()` is caching an expired snapshot in
`memory`, which later lets `writeConnectionCache()` merge against stale
`toolkits`/`catalog` data and revive it. Update the
`readConnectionCache`/`writeConnectionCache` flow in `connectionCache.ts` so
expired disk reads do not become the merge base for subsequent writes; only
populate `memory` when the cached entry is still fresh, or otherwise treat the
cache as empty for merges. Ensure the `useComposioIntegrations()` path cannot
persist partial writes over an expired snapshot.

In `@app/src/lib/composio/hooks.ts`:
- Around line 138-154: The post-invalidation cache writes in the Composio hooks
are racy because in-flight refresh/poll callbacks can still call setConnections
and writeConnectionCache after clearConnectionCache() has run. Add a
config-generation or request-token guard in the relevant hook paths (around the
refresh/poll logic in hooks.ts) so only results from the currently active
Composio client are allowed to commit state. Use the existing refresh/poll
callbacks that write freshConnections/toolkits/catalog to compare against the
latest generation before mutating cache or state, and ignore stale responses
from previous tenants.

In `@app/src/providers/__tests__/ChatRuntimeProvider.test.tsx`:
- Around line 1455-1475: The `console.warn` spy in the
`ChatRuntimeProvider.test.tsx` latency tests is being restored inside each test,
so a failing assertion can leave the spy active for later tests. Move the
restore into the shared cleanup path used by these tests (for example the common
`afterEach`/test teardown alongside `vi.useRealTimers()`), and remove the
per-test `warnSpy.mockRestore()` calls. Keep the existing `vi.spyOn(console,
'warn')` usage in the affected test cases (`renderProvider`, `onToolCall`,
`onDone`) but ensure the original `console.warn` implementation is always
restored even when an expectation throws.

In `@app/src/providers/ChatRuntimeProvider.tsx`:
- Around line 521-523: The latency chain is keyed too broadly in
ChatRuntimeProvider, causing parallel turns that share a thread_id to overwrite
each other’s measurements. Update the tool-chain tracking in the affected turn
flow to use request_id as the primary key instead of event.thread_id, and make
sure both noteToolCall and finishChain operate on the same request-scoped
identifier. Apply the same change consistently in the other affected call sites
so each forked request maintains its own elapsed time and tool count.
- Around line 291-294: The provider-level skill latency tracker persists across
turns, but the disconnect reconciliation path can tear down in-flight turns
without clearing its state, causing stale startedAt/toolCount to leak into the
next tool call. Update the socket-disconnect teardown logic in
ChatRuntimeProvider to reset or finish the tracker whenever turns are abandoned,
using the existing skillLatencyRef and its tracker methods (finishChain/reset)
so each new thread/tool chain starts cleanly.

In `@app/test/e2e/specs/skill-activate-invoke-chat.spec.ts`:
- Around line 123-131: The Composio execute verification in the skill
activate/invoke chat spec is only warning on a missing tool call, so the test
can still pass without the expected routing. Update the check around
waitForToolCallInMockLog and toolHit in skill-activate-invoke-chat.spec.ts to
fail the test immediately when the /agent-integrations/composio/execute call is
not observed, using a hard assertion instead of console.warn so the
activate→invoke path is enforced.

In `@app/test/e2e/specs/skill-activation-persistence.spec.ts`:
- Around line 62-67: The simulated restart in simulateRestart() only remounts
routes within the same renderer, so it never validates the durable cache path.
Update the e2e flow in skill-activation-persistence.spec.ts to force a true
webview reload/relaunch, or explicitly clear the connectionCache.ts module-level
singleton before the second assertion, so readConnectionCache() cannot satisfy
the test from in-memory state populated by writeConnectionCache().

---

Nitpick comments:
In `@app/src/services/api/skillRegistryApi.test.ts`:
- Around line 168-221: The cache tests in skillRegistryApi.test.ts cover hits,
concurrent dedupe, force refresh, and invalidation, but they still miss TTL
expiry and failure recovery. Add a test around skillRegistryApi.browse using
fake timers to advance past BROWSE_CACHE_TTL_MS and assert the next call
re-fetches, and add a rejected callCoreRpc case to verify browse does not retain
a poisoned cache entry and that browseInflight is cleared so a later browse
retries. Use the existing browse and invalidateSkillBrowseCache helpers and the
callCoreRpc mock to locate the behavior under test.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 5dc84bda-6cdb-4dad-b47a-80acd588ec96

📥 Commits

Reviewing files that changed from the base of the PR and between d867c1f and e35b767.

📒 Files selected for processing (14)

• app/src/components/skills/SkillsExplorerTab.tsx
• app/src/components/skills/__tests__/SkillsExplorerTab.test.tsx
• app/src/lib/ai/skillToolChainLatency.test.ts
• app/src/lib/ai/skillToolChainLatency.ts
• app/src/lib/composio/connectionCache.test.ts
• app/src/lib/composio/connectionCache.ts
• app/src/lib/composio/hooks.test.ts
• app/src/lib/composio/hooks.ts
• app/src/providers/ChatRuntimeProvider.tsx
• app/src/providers/__tests__/ChatRuntimeProvider.test.tsx
• app/src/services/api/skillRegistryApi.test.ts
• app/src/services/api/skillRegistryApi.ts
• app/test/e2e/specs/skill-activate-invoke-chat.spec.ts
• app/test/e2e/specs/skill-activation-persistence.spec.ts

[YellowSnnowmann]

Pushed a3548ef addressing all review comments (replied inline on each).

Summary of fixes

• connectionCache: key now user-prefixed ${userId}:composio:connections:v1 so clearAllAppData purges it; isFresh() guard prevents an expired snapshot from seeding the in-memory mirror / write merge base.
• composio hooks: configGenerationRef guard — in-flight refresh/poll can't repopulate the cache after a config-change invalidation.
• ChatRuntimeProvider: latency chain keyed by thread+request (parallel turns no longer collide); tracker reset on socket-disconnect reconcile; toolCount doc clarified.
• tests: shared afterEach spy restore; hard-assert the Composio execute call; durable-write probe + cold-restart read-back unit test.

Re the skillRegistryApi.test.ts nitpick: the browse-cache hit / single-flight / force-refresh / invalidate paths are covered; happy to add an explicit fake-timer TTL-expiry case as a follow-up if useful.

[coderabbitai]

Actionable comments posted: 1

🤖 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.

Inline comments:
In `@app/test/e2e/specs/skill-activation-persistence.spec.ts`:
- Around line 100-105: The persistence check is using a suffix match on
localStorage keys, which can accidentally read a stale connection blob from
another user. Update the e2e test in skill-activation-persistence.spec.ts to
resolve the active user id first and then read and assert against the exact
`${userId}:composio:connections:v1` entry from browser.execute, instead of using
Object.keys(...).find(...) with `endsWith('composio:connections:v1')`.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 41f7c292-c7ef-4391-80e2-a9cb37d1d127

📥 Commits

Reviewing files that changed from the base of the PR and between e35b767 and a3548ef.

📒 Files selected for processing (9)

• app/src/lib/ai/skillToolChainLatency.ts
• app/src/lib/composio/connectionCache.test.ts
• app/src/lib/composio/connectionCache.ts
• app/src/lib/composio/hooks.test.ts
• app/src/lib/composio/hooks.ts
• app/src/providers/ChatRuntimeProvider.tsx
• app/src/providers/__tests__/ChatRuntimeProvider.test.tsx
• app/test/e2e/specs/skill-activate-invoke-chat.spec.ts
• app/test/e2e/specs/skill-activation-persistence.spec.ts

🚧 Files skipped from review as they are similar to previous changes (7)

• app/test/e2e/specs/skill-activate-invoke-chat.spec.ts
• app/src/providers/tests/ChatRuntimeProvider.test.tsx
• app/src/providers/ChatRuntimeProvider.tsx
• app/src/lib/ai/skillToolChainLatency.ts
• app/src/lib/composio/hooks.test.ts
• app/src/lib/composio/hooks.ts
• app/src/lib/composio/connectionCache.ts

[M3gA-Mind]

Reviewed and verified: correct, well-tested, CI green/clearing. Approving. Thanks!