Repository navigation
feat(skill-registry): add skill registry module with browse/install UI - #3457
Merged
Merged
Conversation
New top-level domain at src/openhuman/skill_registry/ implementing a skill registry/repository system that supports SKILL.md, Hermes, and OpenClaw skill formats. Module structure follows canonical shape: - types.rs: RegistrySource, CatalogEntry, RegistryKind, default sources - store.rs: JSON cache (~/.openhuman/skill-registry/) with 1hr TTL - ops.rs: browse/search/install logic with flexible JSON parsing - schemas/: RPC wire types, handlers, controller schemas (6 endpoints)
Register 6 RPC endpoints (browse, search, sources, add_source, remove_source, install) and add namespace description.
RPC client for skill_registry endpoints: browse, search, sources, addSource, removeSource, install. Uses callCoreRpc with envelope unwrap.
Add registryTab, installedTab, allFormats, refreshRegistry, registryEmptyTitle, registryEmptyDescription to en.ts and all 13 locale files with proper translations.
Extend SkillsExplorerTab with dual-view (Registry / Installed) toggle, catalog browsing via skillRegistryApi, format filtering dropdown, refresh button, CatalogTile with one-click install, and installed badge for already-present skills.
Wire SkillsExplorerTab into the PillTabBar on the Connections page alongside Composio, Channels, and MCP tabs.
10 tests covering loading, rendering, empty/error states, search filtering, install button, uninstall visibility, version/tags, scope badges, and warnings.
Update tests to account for registry being the default view. Add skillRegistryApi mock, catalog entry tests, switchToInstalled helper, and installed-badge assertion. 12 tests passing.
Contributor
|
Warning Review limit reached
More reviews will be available in 26 minutes and 54 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (18)
📝 WalkthroughWalkthroughThis PR introduces a complete skill registry feature: a Rust backend that fetches and caches catalogs from remote registry sources, a TypeScript RPC client, a React explorer component with two views (installed skills and catalog browsing), integration into the existing Skills page, comprehensive tests, and translated UI strings for 13 languages. ChangesSkill Registry Feature
🎯 4 (Complex) | ⏱️ ~60 minutes
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
coderabbitai
Bot
added
feature
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (4)
app/src/components/skills/__tests__/SkillsExplorerTab.test.tsx (1)
167-176: 💤 Low valueMerge duplicate test cases.
The test at lines 167-176 ("shows install from URL button") and the test at lines 254-266 ("has an install from URL button") both verify the same element. The second test is more comprehensive (checks text content), so the first could be removed or merged.
♻️ Proposed consolidation
Remove the first test and keep only the more complete version:
- it('shows install from URL button', async () => { - const { skillsApi } = await import('../../../services/api/skillsApi'); - vi.mocked(skillsApi.listSkills).mockResolvedValue([]); - - render(<SkillsExplorerTab />); - - await waitFor(() => { - expect(screen.getByTestId('skill-install-from-url-btn')).toBeInTheDocument(); - }); - }); -Also applies to: 254-266
🤖 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/components/skills/__tests__/SkillsExplorerTab.test.tsx` around lines 167 - 176, Remove the duplicate, less-complete test case titled "shows install from URL button" and keep the later, more comprehensive "has an install from URL button" test; ensure the remaining test still imports/mock-sets skillsApi.listSkills and renders <SkillsExplorerTab /> so the assertions (including text content and getByTestId 'skill-install-from-url-btn') still pass and adjust any test setup/teardown if necessary.app/src/components/skills/SkillsExplorerTab.tsx (1)
1-675: ⚖️ Poor tradeoffConsider splitting into smaller modules.
This file is 675 lines, exceeding the ~500-line guideline. The inline badge and tile subcomponents (
SkillFormatBadge,SkillScopeBadge,SourceBadge,SkillTile,CatalogTile) could be extracted to separate co-located files (e.g.,SkillsExplorerTab/tiles.tsx,SkillsExplorerTab/badges.tsx) to improve maintainability.As per coding guidelines, keep frontend file size to ≤ ~500 lines by breaking larger components into smaller, focused modules.
🤖 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/components/skills/SkillsExplorerTab.tsx` around lines 1 - 675, The SkillsExplorerTab component file is too large—extract the smaller subcomponents into co-located modules: move SkillFormatBadge, SkillScopeBadge, and SourceBadge into a new badges module (e.g., SkillsExplorerTab/badges.tsx) and move SkillTile and CatalogTile into a tiles module (e.g., SkillsExplorerTab/tiles.tsx); update SkillsExplorerTab to import { SkillFormatBadge, SkillScopeBadge, SourceBadge } and { SkillTile, CatalogTile } and ensure props/signatures (SkillSummary, CatalogEntry, installingId, handleRegistryInstall, onUninstall) are preserved, export any helper types if needed, and keep all existing prop names and callbacks (fetchSkills, handleRegistryInstall, handleInstalled, handleUninstalled, installingId, installedIds) so behavior is unchanged.Source: Coding guidelines
src/openhuman/skill_registry/store.rs (1)
89-98: ⚡ Quick winConsider logging
create_dir_allfailures for consistency.Line 91 silently ignores directory creation errors, while Line 55-58 in
save_catalog_cache()logs a warning. For consistency and debuggability, consider logging the error here as well, even though saving custom sources is non-critical.🔍 Suggested improvement
pub fn save_custom_sources(sources: &[RegistrySource]) { let Some(dir) = registry_dir() else { return }; - let _ = std::fs::create_dir_all(&dir); + if let Err(e) = std::fs::create_dir_all(&dir) { + tracing::warn!(error = %e, "[skill_registry] failed to create sources dir"); + return; + } let path = dir.join(SOURCES_FILE);🤖 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/skill_registry/store.rs` around lines 89 - 98, In save_custom_sources, the std::fs::create_dir_all(&dir) result is ignored; change it to capture the Result and log a warning on Err so directory creation failures are visible (mirror save_catalog_cache behavior). Specifically, replace the ignored call with let dir_res = std::fs::create_dir_all(&dir); and if let Err(e) = dir_res { tracing::warn!(error = %e, "[skill_registry] failed to create registry dir for SOURCES_FILE"); } ensuring you still proceed with building path and writing the file afterward.src/openhuman/skill_registry/schemas/handlers.rs (1)
89-107: Recheck install cache staleness: expiry triggers refetch; only within-TTL misses can fail
handle_installalways callsops::browse_catalog(false), butbrowse_catalog(false)only uses the cached catalog when it’s still fresh (CACHE_TTL_SECS = 3600); once expired it fetches from enabled sources and re-saves the cache. So there isn’t a “cache expired → entry not found with no recovery” failure mode.Remaining gap: there’s no
force_refresh/retry on the install-side lookup failure against an in-TTL cached catalog (e.g., direct installs/bookmarks for newly published entries during the cache window). Consider retryingbrowse_catalog(true)once when the entry isn’t found.🤖 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/skill_registry/schemas/handlers.rs` around lines 89 - 107, handle_install currently calls ops::browse_catalog(false) which may return a stale in-TTL cache that omits newly published entries; modify handle_install (the function) so that when the initial catalog lookup (using ops::browse_catalog(false)) does not find the entry (the catalog.iter().find(...) result), it retries once by calling ops::browse_catalog(true) to force a refresh and re-running the find before returning the not-found error; keep existing logging and error message behavior but ensure the forced refresh path only runs on that not-found case.
🤖 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/components/skills/SkillsExplorerTab.tsx`:
- Around line 237-253: Replace the hardcoded English labels in SkillsExplorerTab
with i18n keys: import and call useT() from app/src/lib/i18n/I18nContext at the
top of the SkillsExplorerTab component and replace the three literals
("Installed", "Install", "Installing...") with t('skills.installed'),
t('skills.install'), and t('skills.installing') (or similar key names you
choose) where the UI renders based on installed and installing (the button with
data-testid `registry-install-${entry.id}` and the Installed span). Add these
keys with translations to all locale files you added in the PR so the text is
available in every locale.
In `@src/openhuman/skill_registry/ops.rs`:
- Around line 12-21: Add debug-level entry/exit logs to list_sources(): log at
function entry that list_sources() started and report the counts from
default_sources() and store::load_custom_sources(); during the dedupe loop
(where you check sources.iter().any(|s| s.id == c.id)) log when a custom source
is skipped due to duplicate id (include the c.id) and when a custom source is
appended; before returning, log the final total number of RegistrySource items
being returned. Use the project's debug logging facility (e.g., log::debug!) so
the messages show at debug level and reference the functions default_sources(),
store::load_custom_sources(), and the RegistrySource id field in the messages.
- Around line 123-161: In search_catalog, add debug logging at the start to
record the incoming query and filters (use the normalized q, and the raw
format_filter and source_filter Options) and add debug logging before returning
to report the number of matched entries (filtered.len()); use the project
logging macro (e.g., debug! from log or tracing::debug) so calls are no-ops
unless debug enabled, and format the Option values safely (e.g., as {:?}) when
logging; place the first log right after let q = query.to_lowercase() and the
second log right before Ok(filtered).
- Around line 23-52: Add debug/info logging to the entry, key decision points,
and exit of add_source(source: RegistrySource) and remove_source(id: &str): log
on function entry with the incoming source.id or id, log validation failures
(empty id/url) and duplicate/not-found outcomes, log successful persistence
after calling store::save_custom_sources(&custom) and when store::clear_cache()
is invoked, and include contextual messages and identifying symbols (add_source,
remove_source, store::save_custom_sources, store::clear_cache) so callers can
trace state changes and errors.
In `@src/openhuman/skill_registry/schemas/handlers.rs`:
- Around line 25-122: Add inline unit tests (#[cfg(test)] mod tests) exercising
the handler functions handle_browse, handle_search, handle_sources,
handle_add_source, handle_remove_source, and handle_install: create
serde_json::json! param maps and assert deserialize_params behavior for
missing/extra/wrong-typed fields, assert that ops::add_source and
ops::remove_source error paths propagate (use mocks or temporarily replace ops
if needed), test handle_install’s catalog lookup returns the "entry not found"
error for missing entry_id/source_id and a successful install path, and verify
the handlers wrap results into RpcOutcome and to_json produces the expected JSON
shape; use #[tokio::test] for async handlers and reference the exact handler
names above to locate where to add tests.
In `@src/openhuman/skill_registry/types.rs`:
- Around line 57-84: Move the default_sources() function out of types.rs into
src/openhuman/skill_registry/ops.rs so types.rs only contains Serde domain
types; recreate the function in ops.rs with the same signature (pub fn
default_sources() -> Vec<RegistrySource>) returning the existing RegistrySource
entries (using RegistryKind) and delete the implementation from types.rs, then
update any modules that referenced default_sources() to import it from the ops
module (ensure RegistrySource and RegistryKind remain publicly accessible from
types.rs for the new location to compile).
---
Nitpick comments:
In `@app/src/components/skills/__tests__/SkillsExplorerTab.test.tsx`:
- Around line 167-176: Remove the duplicate, less-complete test case titled
"shows install from URL button" and keep the later, more comprehensive "has an
install from URL button" test; ensure the remaining test still imports/mock-sets
skillsApi.listSkills and renders <SkillsExplorerTab /> so the assertions
(including text content and getByTestId 'skill-install-from-url-btn') still pass
and adjust any test setup/teardown if necessary.
In `@app/src/components/skills/SkillsExplorerTab.tsx`:
- Around line 1-675: The SkillsExplorerTab component file is too large—extract
the smaller subcomponents into co-located modules: move SkillFormatBadge,
SkillScopeBadge, and SourceBadge into a new badges module (e.g.,
SkillsExplorerTab/badges.tsx) and move SkillTile and CatalogTile into a tiles
module (e.g., SkillsExplorerTab/tiles.tsx); update SkillsExplorerTab to import {
SkillFormatBadge, SkillScopeBadge, SourceBadge } and { SkillTile, CatalogTile }
and ensure props/signatures (SkillSummary, CatalogEntry, installingId,
handleRegistryInstall, onUninstall) are preserved, export any helper types if
needed, and keep all existing prop names and callbacks (fetchSkills,
handleRegistryInstall, handleInstalled, handleUninstalled, installingId,
installedIds) so behavior is unchanged.
In `@src/openhuman/skill_registry/schemas/handlers.rs`:
- Around line 89-107: handle_install currently calls ops::browse_catalog(false)
which may return a stale in-TTL cache that omits newly published entries; modify
handle_install (the function) so that when the initial catalog lookup (using
ops::browse_catalog(false)) does not find the entry (the
catalog.iter().find(...) result), it retries once by calling
ops::browse_catalog(true) to force a refresh and re-running the find before
returning the not-found error; keep existing logging and error message behavior
but ensure the forced refresh path only runs on that not-found case.
In `@src/openhuman/skill_registry/store.rs`:
- Around line 89-98: In save_custom_sources, the std::fs::create_dir_all(&dir)
result is ignored; change it to capture the Result and log a warning on Err so
directory creation failures are visible (mirror save_catalog_cache behavior).
Specifically, replace the ignored call with let dir_res =
std::fs::create_dir_all(&dir); and if let Err(e) = dir_res {
tracing::warn!(error = %e, "[skill_registry] failed to create registry dir for
SOURCES_FILE"); } ensuring you still proceed with building path and writing the
file afterward.
🪄 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: 5fca26e3-790d-4e0a-8d69-e174b2890653
📒 Files selected for processing (28)
app/src/components/skills/SkillsExplorerTab.tsxapp/src/components/skills/__tests__/SkillsExplorerTab.test.tsxapp/src/lib/i18n/ar.tsapp/src/lib/i18n/bn.tsapp/src/lib/i18n/de.tsapp/src/lib/i18n/en.tsapp/src/lib/i18n/es.tsapp/src/lib/i18n/fr.tsapp/src/lib/i18n/hi.tsapp/src/lib/i18n/id.tsapp/src/lib/i18n/it.tsapp/src/lib/i18n/ko.tsapp/src/lib/i18n/pl.tsapp/src/lib/i18n/pt.tsapp/src/lib/i18n/ru.tsapp/src/lib/i18n/zh-CN.tsapp/src/pages/Skills.tsxapp/src/services/api/skillRegistryApi.tssrc/core/all.rssrc/openhuman/mod.rssrc/openhuman/skill_registry/mod.rssrc/openhuman/skill_registry/ops.rssrc/openhuman/skill_registry/schemas/controller_schemas.rssrc/openhuman/skill_registry/schemas/handlers.rssrc/openhuman/skill_registry/schemas/mod.rssrc/openhuman/skill_registry/schemas/wire_types.rssrc/openhuman/skill_registry/store.rssrc/openhuman/skill_registry/types.rs
Merge both skills and meetings tabs into ConnectionsTab type.
Replace hardcoded "Installed", "Install", "Installing..." strings with t() calls. Add skills.explorer.installed, .install, .installing keys to en.ts and all 13 locale files.
…to ops.rs - Add tracing::debug logging to list_sources, add_source, remove_source, and search_catalog per project coding guidelines. - Move default_sources() from types.rs to ops.rs since types.rs should only contain Serde domain types.
This was referenced Jun 16, 2026
senamakel
added a commit
to nocstah/openhuman
that referenced
this pull request
Sep 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
skill_registrydomain module that aggregates skills from multiple registries (OpenHuman, Hermes, OpenClaw) via JSON index catalogs.skill_registry_browse,_search,_sources,_add_source,_remove_source,_install) for browsing, searching, and one-click installing remote skills.Problem
Solution
skill_registrymodule (src/openhuman/skill_registry/): canonical module shape withtypes.rs,store.rs(JSON cache with 1hr TTL at~/.openhuman/skill-registry/),ops.rs(flexible JSON parsing viaRawIndexEntryto handle varying field names across registries), andschemas/(wire types, handlers, controller schemas).skillRegistryApi.tsRPC client +SkillsExplorerTab.tsxwith Registry/Installed toggle, format filter dropdown, search, refresh button,CatalogTilewith one-click install, andSkillTilefor local skills.install_workflow_from_urlin the workflows module, reusing all SSRF guards and validation.Submission Checklist
Impact
install_workflow_from_url. Catalog fetch uses HTTPS with size limits (5MB) and timeouts (30s).Related
skill_registry_*RPC endpoints, registry source management UIAI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
Validation Run
pnpm --filter openhuman-app format:checkpnpm typecheckpnpm debug unit src/components/skills/__tests__/SkillsExplorerTab.test.tsx(12/12 passing)cargo check --manifest-path Cargo.toml(clean, only pre-existing warnings in slack-backfill)Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
install_workflow_from_urlwith all existing guards.Duplicate / Superseded PR Handling
Summary by CodeRabbit
New Features
Tests