33 feature add skills#34
Merged
Merged
Conversation
- Added a new skills module to manage lightweight context templates for the agent. - Implemented API endpoints for fetching, adding, deleting, and enabling/disabling skills. - Created a user interface component for managing skills, including browsing and installing from ClawHub. - Documented the skills format and usage in a new markdown file. - Enhanced the agent's prompt with available skills to improve response accuracy.
- Introduced constants for skill hints to improve the clarity of the system prompt for the weather skill. - Updated the skills prompt to include mandatory hints for the weather skill, ensuring proper usage of APIs. - Added a truncation function to manage prompt length while maintaining readability. - Enhanced the weather skill's README to clarify usage instructions and fallback mechanisms for fetching weather data.
Closed
6 tasks
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 38 minutes and 54 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, 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 have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (15)
📝 WalkthroughWalkthroughThis change introduces a comprehensive "skills" system—lightweight markdown-based context templates for the agent—with backend service for management (list, create, delete, enable/disable, ClawHub search/install), frontend API client, and React UI component integrated into the dashboard. Changes
Sequence DiagramsequenceDiagram
actor User as User
participant Client as Frontend
participant Server as Tauri Server
participant Service as Skills Service
participant ClawHub as ClawHub API
participant FS as Filesystem
User->>Client: Click "Install from ClawHub"
Client->>Server: POST /v1/skills/clawhub/search?q=weather
Server->>Service: search_clawhub(query)
Service->>ClawHub: GET https://clawhub.ai/api/search?q=weather
ClawHub-->>Service: ClawHubSearchResponse { results }
Service-->>Server: Vec<ClawHubSkill>
Server-->>Client: { results }
Client->>User: Display search results
User->>Client: Click Install on weather skill
Client->>Server: POST /v1/skills/clawhub/install { slug: "weather" }
Server->>Service: install_clawhub_skill(store_path, slug)
Service->>ClawHub: GET https://clawhub.ai/api/v1/download?slug=weather
ClawHub-->>Service: Zip file (bytes)
Service->>Service: Extract SKILL.md from zip
Service->>FS: Write to $APP_DATA/skills/weather/README.md
FS-->>Service: ✓
Service->>FS: Update .disabled.json (remove if present)
FS-->>Service: ✓
Service-->>Server: Skill { slug, name, enabled, body, ... }
Server-->>Client: { ok: true, skill }
Client->>Client: Reload skills, close modal
Client->>User: Display success, show new skill
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tools/mcp-tools.json (1)
58-81:⚠️ Potential issue | 🟡 MinorConsider the policy implications of enabling
--ignore-robots-txtglobally.Setting
--ignore-robots-txtas a permanent default for the fetch MCP server bypasses robots.txt for every host the agent fetches, not just public APIs like Open-Meteo that don't serve a meaningful robots policy. This may conflict with site terms of service and could expose users to compliance/abuse risks when the agent autonomously crawls arbitrary URLs.A more conservative approach would be to keep robots.txt enforcement on by default and either (a) document an opt-in for users who need it, or (b) restrict the bypass to a known allowlist of API hosts. At minimum, the user-facing description should make the bypass behavior visible in the install/enable UI so users can make an informed choice.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tools/mcp-tools.json` around lines 58 - 81, The config currently enables the flag --ignore-robots-txt in the mcp_server_cmd array which disables robots.txt globally; remove that flag from mcp_server_cmd, update the description field to disclose that robots.txt is enforced by default and provide an opt-in, and add a new explicit config key (e.g., "ignore_robots_by_default": false or "robots_ignore_allowlist": ["open-meteo.com"]) to allow either a user opt-in or a host allowlist; adjust any UI/install text to surface this choice so the agent does not bypass robots.txt for arbitrary hosts by default.
🧹 Nitpick comments (7)
src-tauri/src/modules/skills/types.rs (1)
15-38: Consider a serde default fororigintoo.
SkillderivesDeserialize, andenabledhasdefault_true, butoriginhas no default. Any JSON deserialization path (e.g. cached state, future IPC payload, or a ClawHub response that omits it) will hard-fail here. Sinceoriginis effectively always set byparse_skill/list_skills, either dropDeserializefromSkillor giveorigina sensible default (e.g.Custom) to match the laxness applied to the other fields.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/skills/types.rs` around lines 15 - 38, The Skill struct currently deserializes without a default for the origin field, causing failures when origin is omitted; add a serde default for origin (e.g. #[serde(default = "default_origin")] or #[serde(default)]) and provide a matching default function or implement Default for the SkillOrigin enum that returns SkillOrigin::Custom, then reference that default in the Skill struct's origin field so existing parse_skill/list_skills deserialization paths remain tolerant.src-tauri/src/modules/bot/agent.rs (1)
342-346: Prompt growth — watch the cumulative size.
skills_hintis appended on every turn and, perSKILL_HINT_BODY_CAP = 1600, each enabled skill contributes up to ~1.6 KB plus the weather mandatory block. With several enabled skills this can dominate the system prompt for small local models and silently push user/history context out of the window. Consider either capping total hint size or logging the final system prompt length once for observability.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/bot/agent.rs` around lines 342 - 346, The prompt grows unbounded because skills::skills_prompt_hint(&state.store_path) (skills_hint) is appended every turn and can exceed SKILL_HINT_BODY_CAP cumulatively; modify the code around where skills_hint is produced and used (the skills_hint variable and the format call building the system prompt) to enforce a total hint-cap: compute skills_hint via skills::skills_prompt_hint, then trim or truncate it to a configured MAX_TOTAL_SKILL_HINT_BYTES (or aggregate per-skill entries until the cap), and/or log the final system prompt length once for observability (use processLogger or equivalent) so small local models won’t be pushed out of context. Ensure you reference SKILL_HINT_BODY_CAP when choosing the cap and keep the truncation deterministic (e.g., cut from the end and add an ellipsis) before inserting into the format string.src-tauri/Cargo.toml (1)
37-37:deflate-only feature set is a correct minimization.ClawHub archives only need deflate for
SKILL.mdextraction. Just note: if ClawHub ever ships zips usingdeflate64,bzip2,zstd, or (more likely) stored-but-encrypted entries,extract_skill_mdwill fail with an opaquezip entry N: ...error. Probably fine, but worth surfacing a clearer error message if it happens in practice.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/Cargo.toml` at line 37, Summary: The Cargo.toml minimizes zip features to deflate only but extract_skill_md can fail opaquely for unsupported compression/encryption; update extract_skill_md to surface clearer errors. Modify the extract_skill_md function to catch and map zip read/extraction errors (e.g., unsupported compression methods like deflate64, bzip2, zstd or encrypted/stored entries) to a descriptive error that includes the zip entry name/index and the detected compression/encryption info; specifically wrap errors coming from ZipArchive/ZipFile reads and return a contextual message instead of the opaque "zip entry N: ..." error so callers can distinguish unsupported compression vs corrupted archives. Ensure the new error preserves the original error as an inner source for debugging.src/modules/skills/components/SkillsPanel.tsx (1)
168-175: Minor UX: Cancel doesn't resetaddError/form state.Clicking Cancel only toggles
showAdd; a prioraddError(and any typed slug/markdown) is preserved and reappears next time the form opens. Consider clearing on close:🧹 Suggested tweak
- onClick={() => setShowAdd((v) => !v)} + onClick={() => { + setShowAdd((v) => !v); + setAddError(null); + }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/skills/components/SkillsPanel.tsx` around lines 168 - 175, The Cancel button currently only toggles showAdd and leaves prior addError and form inputs intact; update the onClick handler (or introduce a helper like resetAddForm) so when closing the add form it also clears addError and resets the form state (e.g., call setAddError(undefined/null) and reset slug/markdown state via setSlug('') and setMarkdown('') or the equivalent state setters) before or after calling setShowAdd((v) => !v), so reopening the form is clean.src-tauri/src/modules/skills/service.rs (2)
55-63: Silent fallback on a corrupted.disabled.jsonre-enables every skill.If the JSON file exists but is malformed (partial write, manual edit),
read_disabled_setreturns an empty set, which means every previously-disabled skill silently becomes enabled again — with no log to trace it. Consider logging atwarn!when parsing fails so the failure mode is at least observable.🪵 Suggested fix
- serde_json::from_str::<Vec<String>>(&raw) - .map(|v| v.into_iter().collect()) - .unwrap_or_default() + match serde_json::from_str::<Vec<String>>(&raw) { + Ok(v) => v.into_iter().collect(), + Err(e) => { + log::warn!("corrupt {}: {e}; treating all skills as enabled", path.display()); + HashSet::new() + } + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/skills/service.rs` around lines 55 - 63, read_disabled_set currently swallows JSON parse errors and returns an empty HashSet, silently re-enabling all skills; change the serde_json::from_str::<Vec<String>>(&raw).map(...).unwrap_or_default() path to detect parse failures and log a warn including the parse error and the disabled file path (use disabled_file_path(store_path) or path and the error from serde_json) before returning the default set; keep the same return behavior but emit a warn! so malformed .disabled.json is observable (update code in read_disabled_set to use unwrap_or_else or match on the Result and call warn!).
87-139: Hardcoding a skill-specific hint in the generic service is a code smell.
WEATHER_SKILL_MANDATORY_HINTplus theskills.iter().any(|s| s.slug == "weather")branch (line 135–137) tightly couples the genericskillsmodule to one specific skill. As the comment notes, this exists because the body gets truncated — but that pattern will recur for any richer skill, and repeating this branch per slug won't scale.Consider one of:
- Raising
SKILL_HINT_BODY_CAP(or making it per-skill via frontmatter, e.g.hint_cap: 4096) so the README's own emphasis survives; then drop the override block entirely.- Letting skills declare a
mandatory:frontmatter section that's always appended verbatim regardless of truncation.- At minimum, moving the literal into
tools/skills/weather/README.mdand keying the override generically (e.g. amandatory.mdsibling the service appends when present).This keeps
service.rsagnostic of specific skills and removes the duplication-with-README that the comment already flags.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src-tauri/src/modules/skills/service.rs` around lines 87 - 139, The code currently hardcodes WEATHER_SKILL_MANDATORY_HINT and checks skills.iter().any(|s| s.slug == "weather") inside skills_prompt_hint, coupling the generic skills service to one skill; instead make this generic by allowing each Skill to provide an optional mandatory hint (e.g. a new Skill.mandatory: Option<String> populated by list_skills or by reading a mandatory.md next to the skill README) and change skills_prompt_hint to append that per-skill mandatory text after each skill's truncated body (or always append untruncated mandatory regardless of SKILL_HINT_BODY_CAP); alternatively increase SKILL_HINT_BODY_CAP or make it per-skill via a hint_cap frontmatter, but prefer adding Skill.mandatory and removing WEATHER_SKILL_MANDATORY_HINT and the slug check so service.rs stays skill-agnostic and reads mandatory content from each skill (use existing functions truncate_for_prompt, list_skills, and Skill struct to locate where to add the field).src/modules/skills/types.ts (1)
19-25: Minor:version?: string | nullshould beversion?: stringfor consistency.The Rust
ClawHubSkillusesOption<String>with#[serde(skip_serializing_if = "Option::is_none")], meaning the backend never serializesNoneasnull. The client will never receive anullvalue here. Removing| nullkeeps the type consistent withSkill.versionand accurately reflects the actual backend behavior.♻️ Proposed alignment
- version?: string | null; + version?: string;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/modules/skills/types.ts` around lines 19 - 25, The ClawHubSkill type's version field is incorrectly declared as "version?: string | null"; update the ClawHubSkill type so the version property is optional string only (version?: string) to match backend serialization (Option<String> not null) and align with Skill.version; locate the ClawHubSkill type declaration to make this change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@doc/skills.md`:
- Around line 23-31: The fenced code block in doc/skills.md is missing a
language hint which triggers markdownlint MD040; update the opening fence from
``` to ```text (or ```plaintext) for the code block that contains the directory
listing so the linter is silenced and copy/paste fidelity is preserved.
In `@src-tauri/src/infrastructure/http_server.rs`:
- Around line 1335-1352: handle_skills_set_enabled currently toggles the
disabled set even for slugs that don't exist; before calling
skills_service::set_skill_enabled, call
skills_service::list_skills(&state.store_path) (or an equivalent existence-check
API) and verify the slug is present, returning (StatusCode::NOT_FOUND,
Json(ErrorResponse { error: "skill not found".into() })) if it isn't; only call
skills_service::set_skill_enabled and emit the log when the slug exists, and map
other errors to StatusCode::BAD_REQUEST as before.
- Around line 1297-1307: The handler handle_skills_delete currently maps all
delete_custom_skill errors to BAD_REQUEST; change it to inspect the error
returned by skills_service::delete_custom_skill and return StatusCode::NOT_FOUND
when the error indicates the missing directory (e.g. matches the message "custom
skill '<slug>' not found" or your DeleteError::NotFound variant if you
refactor), otherwise keep returning StatusCode::BAD_REQUEST with
Json(ErrorResponse). Update the error mapping closure where delete_custom_skill
is called so NotFound -> (StatusCode::NOT_FOUND, Json(ErrorResponse{...})) and
other errors -> (StatusCode::BAD_REQUEST, Json(ErrorResponse{...})).
In `@src/modules/skills/api/index.ts`:
- Around line 18-26: The parseApiError function can return empty strings because
it uses the nullish coalescing operator (??) which doesn't treat empty string as
missing; update the try branch in parseApiError to use logical OR (||) so that
when body.error is "" or body is {} it falls back to raw.trim() and then to
`HTTP ${resp.status}`; keep the same behavior for the catch branch and reference
the existing symbols parseApiError, resp, raw, and body.error when making the
change.
---
Outside diff comments:
In `@tools/mcp-tools.json`:
- Around line 58-81: The config currently enables the flag --ignore-robots-txt
in the mcp_server_cmd array which disables robots.txt globally; remove that flag
from mcp_server_cmd, update the description field to disclose that robots.txt is
enforced by default and provide an opt-in, and add a new explicit config key
(e.g., "ignore_robots_by_default": false or "robots_ignore_allowlist":
["open-meteo.com"]) to allow either a user opt-in or a host allowlist; adjust
any UI/install text to surface this choice so the agent does not bypass
robots.txt for arbitrary hosts by default.
---
Nitpick comments:
In `@src-tauri/Cargo.toml`:
- Line 37: Summary: The Cargo.toml minimizes zip features to deflate only but
extract_skill_md can fail opaquely for unsupported compression/encryption;
update extract_skill_md to surface clearer errors. Modify the extract_skill_md
function to catch and map zip read/extraction errors (e.g., unsupported
compression methods like deflate64, bzip2, zstd or encrypted/stored entries) to
a descriptive error that includes the zip entry name/index and the detected
compression/encryption info; specifically wrap errors coming from
ZipArchive/ZipFile reads and return a contextual message instead of the opaque
"zip entry N: ..." error so callers can distinguish unsupported compression vs
corrupted archives. Ensure the new error preserves the original error as an
inner source for debugging.
In `@src-tauri/src/modules/bot/agent.rs`:
- Around line 342-346: The prompt grows unbounded because
skills::skills_prompt_hint(&state.store_path) (skills_hint) is appended every
turn and can exceed SKILL_HINT_BODY_CAP cumulatively; modify the code around
where skills_hint is produced and used (the skills_hint variable and the format
call building the system prompt) to enforce a total hint-cap: compute
skills_hint via skills::skills_prompt_hint, then trim or truncate it to a
configured MAX_TOTAL_SKILL_HINT_BYTES (or aggregate per-skill entries until the
cap), and/or log the final system prompt length once for observability (use
processLogger or equivalent) so small local models won’t be pushed out of
context. Ensure you reference SKILL_HINT_BODY_CAP when choosing the cap and keep
the truncation deterministic (e.g., cut from the end and add an ellipsis) before
inserting into the format string.
In `@src-tauri/src/modules/skills/service.rs`:
- Around line 55-63: read_disabled_set currently swallows JSON parse errors and
returns an empty HashSet, silently re-enabling all skills; change the
serde_json::from_str::<Vec<String>>(&raw).map(...).unwrap_or_default() path to
detect parse failures and log a warn including the parse error and the disabled
file path (use disabled_file_path(store_path) or path and the error from
serde_json) before returning the default set; keep the same return behavior but
emit a warn! so malformed .disabled.json is observable (update code in
read_disabled_set to use unwrap_or_else or match on the Result and call warn!).
- Around line 87-139: The code currently hardcodes WEATHER_SKILL_MANDATORY_HINT
and checks skills.iter().any(|s| s.slug == "weather") inside skills_prompt_hint,
coupling the generic skills service to one skill; instead make this generic by
allowing each Skill to provide an optional mandatory hint (e.g. a new
Skill.mandatory: Option<String> populated by list_skills or by reading a
mandatory.md next to the skill README) and change skills_prompt_hint to append
that per-skill mandatory text after each skill's truncated body (or always
append untruncated mandatory regardless of SKILL_HINT_BODY_CAP); alternatively
increase SKILL_HINT_BODY_CAP or make it per-skill via a hint_cap frontmatter,
but prefer adding Skill.mandatory and removing WEATHER_SKILL_MANDATORY_HINT and
the slug check so service.rs stays skill-agnostic and reads mandatory content
from each skill (use existing functions truncate_for_prompt, list_skills, and
Skill struct to locate where to add the field).
In `@src-tauri/src/modules/skills/types.rs`:
- Around line 15-38: The Skill struct currently deserializes without a default
for the origin field, causing failures when origin is omitted; add a serde
default for origin (e.g. #[serde(default = "default_origin")] or
#[serde(default)]) and provide a matching default function or implement Default
for the SkillOrigin enum that returns SkillOrigin::Custom, then reference that
default in the Skill struct's origin field so existing parse_skill/list_skills
deserialization paths remain tolerant.
In `@src/modules/skills/components/SkillsPanel.tsx`:
- Around line 168-175: The Cancel button currently only toggles showAdd and
leaves prior addError and form inputs intact; update the onClick handler (or
introduce a helper like resetAddForm) so when closing the add form it also
clears addError and resets the form state (e.g., call
setAddError(undefined/null) and reset slug/markdown state via setSlug('') and
setMarkdown('') or the equivalent state setters) before or after calling
setShowAdd((v) => !v), so reopening the form is clean.
In `@src/modules/skills/types.ts`:
- Around line 19-25: The ClawHubSkill type's version field is incorrectly
declared as "version?: string | null"; update the ClawHubSkill type so the
version property is optional string only (version?: string) to match backend
serialization (Option<String> not null) and align with Skill.version; locate the
ClawHubSkill type declaration to make this change.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: aee1c5df-effb-44ac-9532-7d5b0376bdb7
⛔ Files ignored due to path filters (1)
src-tauri/Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (15)
doc/skills.mdsrc-tauri/Cargo.tomlsrc-tauri/src/infrastructure/http_server.rssrc-tauri/src/modules/bot/agent.rssrc-tauri/src/modules/mod.rssrc-tauri/src/modules/skills/mod.rssrc-tauri/src/modules/skills/service.rssrc-tauri/src/modules/skills/types.rssrc/modules/skills/api/index.tssrc/modules/skills/components/SkillsPanel.tsxsrc/modules/skills/index.tssrc/modules/skills/types.tssrc/pages/DashboardPage.tsxtools/mcp-tools.jsontools/skills/weather/README.md
- Added support for `ignore_robots_txt` in the tool catalog, allowing tools to bypass robots.txt restrictions when opted in. - Introduced `robots_ignore_allowlist` for future per-host policy management, currently serving as informational. - Updated the skills management system to include a `mandatory.md` file for skills, providing additional rules and guidelines. - Enhanced error handling in skills API to return more specific error messages for non-existent skills. - Improved the skills prompt to include mandatory hints and manage prompt length effectively.
This was referenced Apr 17, 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
1 participant
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 by CodeRabbit
Release Notes
New Features
Documentation
Improvements