Skip to content

feat: add skill-manager skill source switch#7

Open
PkuLilBoat wants to merge 1 commit into
O0000-code:mainfrom
PkuLilBoat:feature/skill-manager-source-directory
Open

feat: add skill-manager skill source switch#7
PkuLilBoat wants to merge 1 commit into
O0000-code:mainfrom
PkuLilBoat:feature/skill-manager-source-directory

Conversation

@PkuLilBoat

@PkuLilBoat PkuLilBoat commented May 19, 2026

Copy link
Copy Markdown

Summary\n- change the default skill source directory to the skill-manager canonical library\n- add Settings UI to show the current skill source and switch back to skill-manager\n- keep manual source selection available for custom paths\n\n## Tests\n- npm run build\n- npm run test -- src/stores/tests/settingsStore.test.ts\n- cargo test test_app_settings_default\n- npm run tauri -- build --config '{"bundle":{"macOS":{"signingIdentity":"-","hardenedRuntime":false}}}'

Summary by CodeRabbit

  • New Features
    • Added "Skill Source Directory" setting in Settings to configure skill source location
    • Users can select between skill-manager library, local workshop library, or custom directory
    • Settings page displays current source type and path with quick-access buttons

Review Change Stack

Co-authored-by: Codex <noreply@openai.com>
@coderabbitai

coderabbitai Bot commented May 19, 2026

Copy link
Copy Markdown
📝 Walkthrough

Walkthrough

This PR updates the default skill source directory path from ~/.cc-workshop/skills to /Users/feng/.agents/skill-library across backend and frontend, introduces path constants for consistency, and adds Settings UI controls for users to select between the skill-manager library, local library, or custom directories with automatic skill reload.

Changes

Skill Source Directory Configuration and UI

Layer / File(s) Summary
Skill source constants and default settings
src/stores/settingsStore.ts
LOCAL_SKILL_SOURCE_DIR and SKILL_MANAGER_LIBRARY_DIR constants are exported; defaultSettings.skillSourceDir switches to the skill-manager library path.
Backend Rust types default update
src-tauri/src/types.rs
AppSettings::default() skill_source_dir value changes to the new path, and the unit test is updated to expect the new default.
Settings UI skill source directory selection
src/pages/SettingsPage.tsx
Constants are imported, store bindings extended for skillSourceDir and directory selection. A computed skillSourceKind label reflects the current source type. Two handlers manage setting the skill-manager library and opening a custom directory picker, both triggering skill reload. A new "Skill Source Directory" row displays the source kind and path, with "Use skill-manager" and "Choose..." controls.
Store test alignment with new constants
src/stores/__tests__/settingsStore.test.ts
Test imports SKILL_MANAGER_LIBRARY_DIR and uses it in store reset and initial state assertion instead of a hardcoded path.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Poem

A workshop directory finds a new home,
Constants guide both frontend and Rust foam,
Settings now let users choose their skill source with glee,
Path constants keep paths from going wild and free,
Tests align like ducks in a well-ordered row. 🐇✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 33.33% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: adding a feature to switch skill source directories with a focus on skill-manager integration.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 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-tauri/src/types.rs`:
- Line 482: The default value for skill_source_dir is a hardcoded user-specific
path ("/Users/feng/.agents/skill-library"); change it to a tilde-based default
like "~/.agents/skill-library" and ensure the code that constructs or reads
skill_source_dir uses the path expansion helper (expand_path or expand_tilde
from src-tauri/src/utils/path.rs) so the tilde is resolved to the current user's
home directory at runtime; update the initialization of skill_source_dir to the
tilde form and where skill_source_dir is consumed, pass it through
expand_path()/expand_tilde() (or call the helper when setting the value) to
achieve cross-platform, user-agnostic behavior.
- Line 1825: Update the test assertion that checks settings.skill_source_dir to
expect the tilde form instead of the hardcoded absolute path: change the
assertion comparing settings.skill_source_dir from
"/Users/feng/.agents/skill-library" to "~/.agents/skill-library" so the test is
user-agnostic (refer to the settings.skill_source_dir assertion in the test).

In `@src/stores/settingsStore.ts`:
- Around line 73-74: Replace the hardcoded user-specific path in
SKILL_MANAGER_LIBRARY_DIR with a tilde-based path (e.g.,
'~/.agents/skill-library') so it is not bound to user "feng"; update the
exported constant SKILL_MANAGER_LIBRARY_DIR accordingly (similar to
LOCAL_SKILL_SOURCE_DIR) and ensure any runtime code that uses
SKILL_MANAGER_LIBRARY_DIR expands the tilde to the current user's home directory
when 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: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: ad0c978a-76f6-458a-9793-72e38aa8bb69

📥 Commits

Reviewing files that changed from the base of the PR and between 5381e3f and e978153.

📒 Files selected for processing (4)
  • src-tauri/src/types.rs
  • src/pages/SettingsPage.tsx
  • src/stores/__tests__/settingsStore.test.ts
  • src/stores/settingsStore.ts

Comment thread src-tauri/src/types.rs
fn default() -> Self {
Self {
skill_source_dir: "~/.cc-workshop/skills".to_string(),
skill_source_dir: "/Users/feng/.agents/skill-library".to_string(),

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Replace hardcoded user-specific path with tilde notation.

The default skill_source_dir contains /Users/feng/.agents/skill-library, which is specific to user "feng" and will not work on other machines. Use tilde notation ~/.agents/skill-library instead, which will be expanded by the path utilities at runtime.

🛠️ Proposed fix
         Self {
-            skill_source_dir: "/Users/feng/.agents/skill-library".to_string(),
+            skill_source_dir: "~/.agents/skill-library".to_string(),
             mcp_source_dir: "~/.cc-workshop/mcps".to_string(),

As per coding guidelines, use expand_path() or expand_tilde() in src-tauri/src/utils/path.rs for cross-platform home directory handling.

🤖 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-tauri/src/types.rs` at line 482, The default value for skill_source_dir
is a hardcoded user-specific path ("/Users/feng/.agents/skill-library"); change
it to a tilde-based default like "~/.agents/skill-library" and ensure the code
that constructs or reads skill_source_dir uses the path expansion helper
(expand_path or expand_tilde from src-tauri/src/utils/path.rs) so the tilde is
resolved to the current user's home directory at runtime; update the
initialization of skill_source_dir to the tilde form and where skill_source_dir
is consumed, pass it through expand_path()/expand_tilde() (or call the helper
when setting the value) to achieve cross-platform, user-agnostic behavior.

Comment thread src-tauri/src/types.rs
fn test_app_settings_default() {
let settings = AppSettings::default();
assert_eq!(settings.skill_source_dir, "~/.cc-workshop/skills");
assert_eq!(settings.skill_source_dir, "/Users/feng/.agents/skill-library");

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Update test assertion to use tilde notation.

The test assertion should be updated to expect ~/.agents/skill-library instead of the hardcoded user-specific path.

✅ Proposed fix
         let settings = AppSettings::default();
-        assert_eq!(settings.skill_source_dir, "/Users/feng/.agents/skill-library");
+        assert_eq!(settings.skill_source_dir, "~/.agents/skill-library");
         assert_eq!(settings.mcp_source_dir, "~/.cc-workshop/mcps");
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
assert_eq!(settings.skill_source_dir, "/Users/feng/.agents/skill-library");
let settings = AppSettings::default();
assert_eq!(settings.skill_source_dir, "~/.agents/skill-library");
assert_eq!(settings.mcp_source_dir, "~/.cc-workshop/mcps");
🤖 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-tauri/src/types.rs` at line 1825, Update the test assertion that checks
settings.skill_source_dir to expect the tilde form instead of the hardcoded
absolute path: change the assertion comparing settings.skill_source_dir from
"/Users/feng/.agents/skill-library" to "~/.agents/skill-library" so the test is
user-agnostic (refer to the settings.skill_source_dir assertion in the test).

Comment on lines +73 to +74
export const LOCAL_SKILL_SOURCE_DIR = '~/.cc-workshop/skills';
export const SKILL_MANAGER_LIBRARY_DIR = '/Users/feng/.agents/skill-library';

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical | ⚡ Quick win

Replace hardcoded user-specific path with tilde notation.

SKILL_MANAGER_LIBRARY_DIR contains /Users/feng/.agents/skill-library, which is specific to user "feng" and will not work on other machines. Use tilde notation ~/.agents/skill-library instead so the path expands correctly for any user.

🛠️ Proposed fix
 export const LOCAL_SKILL_SOURCE_DIR = '~/.cc-workshop/skills';
-export const SKILL_MANAGER_LIBRARY_DIR = '/Users/feng/.agents/skill-library';
+export const SKILL_MANAGER_LIBRARY_DIR = '~/.agents/skill-library';
🤖 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/stores/settingsStore.ts` around lines 73 - 74, Replace the hardcoded
user-specific path in SKILL_MANAGER_LIBRARY_DIR with a tilde-based path (e.g.,
'~/.agents/skill-library') so it is not bound to user "feng"; update the
exported constant SKILL_MANAGER_LIBRARY_DIR accordingly (similar to
LOCAL_SKILL_SOURCE_DIR) and ensure any runtime code that uses
SKILL_MANAGER_LIBRARY_DIR expands the tilde to the current user's home directory
when needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants