feat: add skill-manager skill source switch#7
Conversation
Co-authored-by: Codex <noreply@openai.com>
📝 WalkthroughWalkthroughThis PR updates the default skill source directory path from ChangesSkill Source Directory Configuration and UI
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Poem
🚥 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. ✨ 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 |
There was a problem hiding this comment.
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
📒 Files selected for processing (4)
src-tauri/src/types.rssrc/pages/SettingsPage.tsxsrc/stores/__tests__/settingsStore.test.tssrc/stores/settingsStore.ts
| fn default() -> Self { | ||
| Self { | ||
| skill_source_dir: "~/.cc-workshop/skills".to_string(), | ||
| skill_source_dir: "/Users/feng/.agents/skill-library".to_string(), |
There was a problem hiding this comment.
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.
| 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"); |
There was a problem hiding this comment.
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.
| 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).
| export const LOCAL_SKILL_SOURCE_DIR = '~/.cc-workshop/skills'; | ||
| export const SKILL_MANAGER_LIBRARY_DIR = '/Users/feng/.agents/skill-library'; |
There was a problem hiding this comment.
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.
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