feat(skills): add Hermes-style skill explorer#3388
Merged
senamakel merged 12 commits intoJun 5, 2026
Conversation
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR extends skill metadata across Rust and TypeScript layers, refactors discovery to support new platform compatibility and related-skills fields, normalizes API responses, updates UI rendering, fixes test navigation for tab isolation, and adds comprehensive i18n support across 13+ languages. ChangesSkill Metadata & Explorer Feature
Monitor Test Improvement
🎯 4 (Complex) | ⏱️ ~60 minutes Possibly Related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsStopped waiting for pipeline failures after 30000ms. One of your pipelines takes longer than our 30000ms fetch window to run, so review may not consider pipeline-failure results for inline comments if any failures occurred after the fetch window. Increase the timeout if you want to wait longer or run a 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: 1
🧹 Nitpick comments (1)
app/src/pages/Skills.tsx (1)
1-1257: ⚖️ Poor tradeoffConsider splitting this file in a future refactor.
The file is 1257 lines, which exceeds the 500-line guideline. This is a pre-existing issue (the file was already large before this PR), and the ~60 lines added for the explorer tab are well-structured. Consider extracting tab-specific logic into separate components (e.g.,
SkillsExplorerTab,SkillsComposioTab,SkillsChannelsTab) in a follow-up refactor.As per coding guidelines: Keep TypeScript/React files at or below ~500 lines; split growing modules into smaller files.
🤖 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/pages/Skills.tsx` around lines 1 - 1257, This file is too large; extract tab-specific UI and logic into smaller components to stay under ~500 lines: create components like SkillsExplorerTab, SkillsComposioTab, and SkillsChannelsTab and move the relevant JSX + local state/handlers into them (e.g., explorer: searchQuery, selectedCategory, discoveredSkills, refreshDiscoveredSkills, groupedItems, renderGroup, SkillSearchBar usage; composio: composioGridEntries/composioSortedEntries, composioModalToolkit, ComposioConnectorTile, composio-related useEffect and openhumanComposioGetMode handling; channels: channelDefs, channelConnections, handleSetDefaultChannel, ChannelTile and default channel UI). Keep shared utilities and hooks (bestChannelStatus, channelStatusLabel/color, composioStatusLabel/color/sort, BUILT_IN_SKILLS, SkillItem type) in the original file or a new shared module and pass needed props (e.g., composioConnectionByToolkit, agentReadyComposioToolkits, refreshComposio, setComposioModalToolkit, setChannelModalDef, setSelectedSkill, toasts handlers) into the extracted components to preserve behavior. Ensure exported components accept the minimal props and update imports/usages in Skills to render the new subcomponents and to keep modal state (ChannelSetupModal, ComposioConnectModal, CreateSkillModal, InstallSkillDialog, SkillDetailDrawer, UninstallSkillConfirmDialog, ToastContainer) either lifted or passed callbacks as needed.
🤖 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 `@src/openhuman/skills/ops_discover.rs`:
- Around line 327-328: The current .find(|s| s.name == skill_id || s.dir_name ==
skill_id) can pick the wrong skill when one skill's name equals another's
dir_name; replace it with disambiguation: first search for an exact name match
(s.name == skill_id), if none then search for a dir_name match (s.dir_name ==
skill_id), and if both a name-match and a different dir_name-match exist return
an explicit error about the ambiguity instead of silently picking the first;
update the code that uses skill_id, the closure using s.name/s.dir_name, and the
enclosing function in ops_discover.rs to implement this deterministic
resolution.
---
Nitpick comments:
In `@app/src/pages/Skills.tsx`:
- Around line 1-1257: This file is too large; extract tab-specific UI and logic
into smaller components to stay under ~500 lines: create components like
SkillsExplorerTab, SkillsComposioTab, and SkillsChannelsTab and move the
relevant JSX + local state/handlers into them (e.g., explorer: searchQuery,
selectedCategory, discoveredSkills, refreshDiscoveredSkills, groupedItems,
renderGroup, SkillSearchBar usage; composio:
composioGridEntries/composioSortedEntries, composioModalToolkit,
ComposioConnectorTile, composio-related useEffect and openhumanComposioGetMode
handling; channels: channelDefs, channelConnections, handleSetDefaultChannel,
ChannelTile and default channel UI). Keep shared utilities and hooks
(bestChannelStatus, channelStatusLabel/color, composioStatusLabel/color/sort,
BUILT_IN_SKILLS, SkillItem type) in the original file or a new shared module and
pass needed props (e.g., composioConnectionByToolkit,
agentReadyComposioToolkits, refreshComposio, setComposioModalToolkit,
setChannelModalDef, setSelectedSkill, toasts handlers) into the extracted
components to preserve behavior. Ensure exported components accept the minimal
props and update imports/usages in Skills to render the new subcomponents and to
keep modal state (ChannelSetupModal, ComposioConnectModal, CreateSkillModal,
InstallSkillDialog, SkillDetailDrawer, UninstallSkillConfirmDialog,
ToastContainer) either lifted or passed callbacks as needed.
🪄 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: 3fe13494-8796-48ae-948c-9c1bc80fcffd
📒 Files selected for processing (29)
app/src/components/skills/SkillDetailDrawer.tsxapp/src/components/skills/SkillResourceTree.tsxapp/src/components/skills/__tests__/CreateSkillModal.test.tsxapp/src/components/skills/__tests__/SkillDetailDrawer.test.tsxapp/src/components/skills/__tests__/UninstallSkillConfirmDialog.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/pages/__tests__/Skills.discovered-skills.test.tsxapp/src/services/api/skillsApi.tssrc/openhuman/context/channels_prompt.rssrc/openhuman/skills/inject.rssrc/openhuman/skills/ops_discover.rssrc/openhuman/skills/ops_parse.rssrc/openhuman/skills/ops_tests.rssrc/openhuman/skills/ops_types.rssrc/openhuman/skills/schemas.rs
Contributor
|
Hey, This is a great feature, Thank-You😀 |
Contributor
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/openhuman/monitor/ops.rs (1)
49-58: ⚡ Quick winRemove duplicate gated-command validation in
start().Line 50-53 and Line 55-58 both call
security.check_gated_command(command). This double-check is redundant and can drift if the policy check ever becomes stateful/time-sensitive. Keep a single evaluation and branch logging around that result.Proposed simplification
- if gate_decision == GateDecision::Block { - security.check_gated_command(command).map_err(|reason| { - tracing::debug!(class = ?class, reason = %reason, "[monitor] ops:start blocked"); - reason.to_string() - })?; - } - security.check_gated_command(command).map_err(|reason| { - tracing::debug!(class = ?class, reason = %reason, "[monitor] ops:start denied"); - reason.to_string() - })?; + if let Err(reason) = security.check_gated_command(command) { + let msg = if gate_decision == GateDecision::Block { + "[monitor] ops:start blocked" + } else { + "[monitor] ops:start denied" + }; + tracing::debug!(class = ?class, reason = %reason, "{msg}"); + return Err(reason.to_string()); + }🤖 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/monitor/ops.rs` around lines 49 - 58, There are two back-to-back calls to security.check_gated_command(command) in start(), causing a redundant double validation; replace them with a single call and branch the logging inside its error handler based on GateDecision (use gate_decision == GateDecision::Block to choose between the "[monitor] ops:start blocked" and "[monitor] ops:start denied" debug messages). Concretely: compute the result once (e.g., security.check_gated_command(command).map_err(|reason| { if gate_decision == GateDecision::Block { tracing::debug!(..., "[monitor] ops:start blocked"); } else { tracing::debug!(..., "[monitor] ops:start denied"); } reason.to_string() })? ) so the check is only executed once and behavior matches the original logs.
🤖 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/openhuman/monitor/ops.rs`:
- Around line 49-58: There are two back-to-back calls to
security.check_gated_command(command) in start(), causing a redundant double
validation; replace them with a single call and branch the logging inside its
error handler based on GateDecision (use gate_decision == GateDecision::Block to
choose between the "[monitor] ops:start blocked" and "[monitor] ops:start
denied" debug messages). Concretely: compute the result once (e.g.,
security.check_gated_command(command).map_err(|reason| { if gate_decision ==
GateDecision::Block { tracing::debug!(..., "[monitor] ops:start blocked"); }
else { tracing::debug!(..., "[monitor] ops:start denied"); } reason.to_string()
})? ) so the check is only executed once and behavior matches the original logs.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 7ea0e8cb-5986-42da-9d6a-de57e1c3dda2
📒 Files selected for processing (25)
app/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/pages/__tests__/Skills.channels-grid.test.tsxapp/src/pages/__tests__/Skills.composio-catalog.test.tsxapp/src/pages/__tests__/Skills.third-party-gmail-sync.test.tsxapp/src/pages/__tests__/Skills.third-party-notion-debug-tools.test.tsxapp/test/playwright/specs/composio-triggers-flow.spec.tsapp/test/playwright/specs/connector-gmail-composio.spec.tsapp/test/playwright/specs/connector-session-guard-matrix.spec.tsapp/test/playwright/specs/gmail-flow.spec.tsapp/test/playwright/specs/skills-registry.spec.tssrc/openhuman/monitor/ops.rs
✅ Files skipped from review due to trivial changes (11)
- app/src/lib/i18n/bn.ts
- app/src/lib/i18n/ru.ts
- app/src/lib/i18n/en.ts
- app/src/lib/i18n/es.ts
- app/src/lib/i18n/ko.ts
- app/src/lib/i18n/ar.ts
- app/src/lib/i18n/hi.ts
- app/src/lib/i18n/zh-CN.ts
- app/src/lib/i18n/de.ts
- app/src/lib/i18n/it.ts
- app/src/lib/i18n/pl.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- app/src/lib/i18n/id.ts
- app/src/pages/Skills.tsx
- app/src/lib/i18n/pt.ts
- app/src/lib/i18n/fr.ts
…explorer # Conflicts: # app/src/components/skills/SkillDetailDrawer.tsx # app/src/components/skills/__tests__/SkillDetailDrawer.test.tsx # app/src/pages/Skills.tsx # app/src/pages/__tests__/Skills.discovered-skills.test.tsx # src/openhuman/monitor/ops.rs # src/openhuman/workflows/ops_discover.rs # src/openhuman/workflows/ops_types.rs
Contributor
There was a problem hiding this comment.
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/playwright/specs/skills-registry.spec.ts`:
- Around line 37-39: Scope the assertion to the Composio panel to avoid false
positives: before calling page.getByText(...).first(), click/open the Composio
tab (e.g., page.getByRole('tab', { name: /Composio/i }).click()) or create a
scoped locator for the Composio container (e.g., const composio =
page.locator('selector-for-composio-panel')) and then use
composio.getByText(/Gmail|Notion|Telegram|GitHub|Google Drive/, { exact: false
}).first().toBeVisible() so the matcher only searches inside the Composio
content.
🪄 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: 29716407-0407-4960-840f-ea7f8431e76c
📒 Files selected for processing (2)
app/src/services/api/__tests__/skillsApi.test.tsapp/test/playwright/specs/skills-registry.spec.ts
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
2 participants
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
Problem
Solution
Submission Checklist
diff-cover) meet the gate enforced by.github/workflows/pr-ci.yml. Focused local tests cover changed Rust/React paths; CI diff-cover remains the authoritative merged coverage gate.## Related— N/A: no coverage matrix feature ID changed.docs/RELEASE-MANUAL-SMOKE.md) — N/A: no release manual smoke surface changed.Closes #NNNin the## Relatedsection — N/A: no GitHub issue was provided for this request.Impact
Related
AI Authored PR Metadata (required for Codex/Linear PRs)
Linear Issue
Commit & Branch
feat/hermes-skills-explorere6cdcd2bfValidation Run
pnpm --filter openhuman-app format:check— passed via pre-push hook.pnpm typecheck—pnpm compilepassed locally; pre-push hook also ran compile.pnpm debug unit src/pages/__tests__/Skills.discovered-skills.test.tsx src/components/skills/__tests__/SkillDetailDrawer.test.tsx --verbose— passed, 11 tests.pnpm debug rust openhuman::skills --verbose— passed, 173 skills tests.pnpm exec tsx scripts/i18n-coverage.ts --no-unused --locale zh-CN,hi,es,ar,fr,bn,pt,de,ru,id,it,ko,pl— passed, 0 missing / 0 extra keys.cargo fmt --manifest-path Cargo.toml;pnpm --filter openhuman-app rust:checkpassed via pre-push hook.cargo fmt --manifest-path app/src-tauri/Cargo.toml --all --checkandcargo check --manifest-path app/src-tauri/Cargo.tomlpassed via pre-push hook.Validation Blocked
command:N/Aerror:N/Aimpact:N/ABehavior Changes
Parity Contract
Duplicate / Superseded PR Handling
senamakel:feat/hermes-skills-explorer.Summary by CodeRabbit
New Features
Documentation