-
Notifications
You must be signed in to change notification settings - Fork 38
feat: Add automatic loading of user skills from home directory #950
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
[Automatic Post]: I have assigned @xingyaoww as a reviewer based on git blame information. Thanks in advance for the help! |
|
@OpenHands make sure pre-commit run --all-files pass on this branch. You are on a fork branch "whywaita:feat/implement-user-skills", please update your remote URL before you push y our changes. |
|
I'm on it! xingyaoww can track my progress at all-hands.dev |
Co-authored-by: openhands <[email protected]>
SummaryI've successfully made the pre-commit checks pass on the What was done:✅ Checked out the branch: Successfully fetched and checked out ✅ Ran pre-commit checks: Identified formatting issues in ✅ Fixed formatting issues: The ruff formatter reformatted 3 lines in the test file to split long ✅ Committed changes: Created commit with message "style: Format code with ruff" and proper co-author attribution ✅ Verified all checks pass: All pre-commit hooks now pass successfully:
✅ Pushed to fork: Successfully pushed the changes to Changes made:
The PR #950 has been updated with the formatting fix and all pre-commit checks now pass on the branch. |
| context = AgentContext(skills=[python_agent, testing_agent]) | ||
| context = AgentContext( | ||
| load_user_skills=False, skills=[python_agent, testing_agent] | ||
| ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@OpenHands you are on a fork branch whywaita:feat/implement-user-skills (pls reset the remote url before you push) - load_user_skills should be optional arg, can you remove the changes we added in test_agent_context.py in this PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm on it! xingyaoww can track my progress at all-hands.dev
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
I've successfully completed your request to clean up the test files in PR #950. Here's what was done:
Changes Made (2 commits):
Commit 1: 9bdec4c8 - "Remove explicit load_user_skills=False from test_agent_context.py"
- ✅ Removed all explicit
load_user_skills=Falseparameters fromtest_agent_context.py - This eliminated ~16 redundant parameter assignments since
Falseis already the default value
Commit 2: f402f638 - "Fix tests: explicitly set load_user_skills=True where needed"
- ✅ Fixed 3 tests in
test_load_user_skills.pythat were failing after the default value change - These tests specifically verify the auto-loading functionality, so they need
load_user_skills=True - Updated tests:
test_agent_context_loads_user_skills_by_defaulttest_agent_context_merges_explicit_and_user_skillstest_agent_context_explicit_skill_takes_precedence
Test Results:
- ✅ All 111 tests in
tests/sdk/context/pass - ✅ Pre-commit hooks pass (Ruff format, Ruff lint, pycodestyle, pyright)
PR Status:
- Branch:
feat/implement-user-skillson forkwhywaita/agent-sdk - PR feat: Add automatic loading of user skills from home directory #950 has been updated with both commits
- Current HEAD:
f402f6387605f8866c53e2bb6f927ceb36326179
The PR is now cleaner with redundant parameters removed while maintaining full test coverage and functionality.
Since load_user_skills defaults to False in AgentContext, there's no need to explicitly set it in tests. This makes the tests cleaner and relies on the default behavior. Co-authored-by: openhands <[email protected]>
Since load_user_skills defaults to False, tests that expect automatic loading of user skills need to explicitly enable it. Updated tests: - test_agent_context_loads_user_skills_by_default - test_agent_context_merges_explicit_and_user_skills - test_agent_context_explicit_skill_takes_precedence Co-authored-by: openhands <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
|
@OpenHands Post an issue on OpenHands/OpenHands, tag it as CLI, for the CLI to
|
|
I'm on it! enyst can track my progress at all-hands.dev |
|
Summary of work completed
What’s in the issue
Checks
If you want any edits to the issue’s title, scope, or acceptance criteria, I can update it. |
Summary
This PR implements automatic loading of user skills from the user's home directory, allowing users to define custom skills that are automatically available across all agents without explicit configuration.
Changes
Core Implementation
load_user_skills()function that loads skills from:~/.openhands/skills/(primary location)~/.openhands/microagents/(legacy support for backward compatibility)load_user_skillsfield toAgentContext(default:True)~/.openhands/skills/take precedence over~/.openhands/microagents/API Updates
load_user_skillsfunction fromopenhands.sdk.contextmoduleAgentContextfor automatic user skills loadingTesting
tests/sdk/context/skill/test_load_user_skills.py)AgentContextintegrationBehavior
When enabled (default)
When disabled (
load_user_skills=False)Testing
Related
fixes: OpenHands/OpenHands#11483