Skip to content

fix(skills): load repository skills into conversations and scope skill menus to the conversation workspace#707

Draft
VascoSch92 wants to merge 2 commits into
mainfrom
vasco/fix-repo-skills
Draft

fix(skills): load repository skills into conversations and scope skill menus to the conversation workspace#707
VascoSch92 wants to merge 2 commits into
mainfrom
vasco/fix-repo-skills

Conversation

@VascoSch92
Copy link
Copy Markdown
Member

@VascoSch92 VascoSch92 commented May 21, 2026

  • A human has tested these changes.

Summary

Repository skills under .agents/skills/ (e.g. this repo's custom-codereview-guide.md) were visible on the Skills settings page but never reached conversations: they weren't injected into the agent context, and they didn't appear in the in-conversation / slash-command autocomplete or the skills menu. This wires project skills through both the conversation-start payload and the conversation-scoped skill UIs.

Closes #574.

Issue Number

#574

Root cause

  • Not loaded into the agent. AgentContext only auto-loads user and public skills (load_user_skills / load_public_skills). Verified against the released openhands-sdk v1.23.0 (the version config/defaults.json
    pins): _load_auto_skills() hardcodes include_project=False, and there is no extension_config / wire field to request project skills for a conversation. So project skills had no path into a conversation at all.
  • Not shown in the conversation UIs. The slash-command menu (use-slash-command.ts) and skills modal both call useSkills(), which queried the global getAgentServerWorkingDir() rather than the conversation's own workspace
    — so a conversation attached to a repo never listed that repo's skills.

Changes

  • skills-service.ts: add getProjectSkills(projectDir?) which loads only project skills and rebuilds the SDK Skill wire shape from the flattened SkillInfo (reversing Skill.to_skill_info(), reconstructing KeywordTrigger /
    TaskTrigger). getSkills(projectDir?) gains an optional dir.
  • agent-server-adapter.ts: inject the pre-loaded project skills into agent_context.skills at conversation start (valid on both the OpenHands and ACP paths — skills is acp_compatible).
  • agent-server-conversation-service.api.ts: resolve the skills source as the workspace root (workingDirOverride ?? getAgentServerWorkingDir()), not the per-conversation worktree subdir (which has no .agents/skills/ and
    may not exist yet at request time).
  • use-conversation-skills.ts (new): a useConversationSkills hook that scopes the catalog to the active conversation's selected_workspace. The slash-command hook and skills modal now use it, so the menu lists exactly the
    project skills loaded into that conversation.

Testing

  • npm run typecheck && npm run build pass (the pre-existing unrelated interruptConversation type error on main is untouched).
  • New unit tests: SkillInfoSkill conversion + project-only request, cloud/error fallbacks, injection into the Agent and ACP agent_context, project_dir forwarding, and a regression test that skills load from the workspace
    root (not the per-conversation worktree dir) — verified to fail if the resolution is reverted.

Video/Screenshots

Screen.Recording.2026-05-21.at.14.36.23.mov

Type

  • Bug fix
  • Feature
  • Refactor
  • Breaking change
  • Docs / chore

Notes

  • The client-side injection exists because the pinned SDK has no project-skill wire field. Once an agent-server release ships extension_config, this can be replaced by that top-level field and getProjectSkills dropped.
  • The slash menu only surfaces skills with /-prefixed triggers (or AgentSkills-type), which is existing behavior — a keyword-only knowledge skill still won't appear there by design.

🐳 Docker images for this PR

GHCR package: https://github.com/OpenHands/agent-canvas/pkgs/container/agent-canvas

Component Value
Image ghcr.io/openhands/agent-canvas
Architectures amd64, arm64
Agent Server ghcr.io/openhands/agent-server:1.23.0-python
Automation openhands-automation==1.0.0a3
Commit ff396f72f83682a57cf12312b9b8f6e39a6ba78c

Pull (multi-arch manifest)

# Multi-arch manifest — Docker automatically pulls the correct architecture
docker pull ghcr.io/openhands/agent-canvas:sha-ff396f7

Run

docker run -it --rm \
  -p 8000:8000 \
  ghcr.io/openhands/agent-canvas:sha-ff396f7

All tags pushed for this build

ghcr.io/openhands/agent-canvas:sha-ff396f7-amd64
ghcr.io/openhands/agent-canvas:vasco-fix-repo-skills-amd64
ghcr.io/openhands/agent-canvas:pr-707-amd64
ghcr.io/openhands/agent-canvas:sha-ff396f7-arm64
ghcr.io/openhands/agent-canvas:vasco-fix-repo-skills-arm64
ghcr.io/openhands/agent-canvas:pr-707-arm64
ghcr.io/openhands/agent-canvas:sha-ff396f7
ghcr.io/openhands/agent-canvas:vasco-fix-repo-skills
ghcr.io/openhands/agent-canvas:pr-707

About Multi-Architecture Support

  • Each tag (e.g., sha-ff396f7) is a multi-arch manifest supporting both amd64 and arm64
  • Docker automatically pulls the correct architecture for your platform
  • Individual architecture tags (e.g., sha-ff396f7-amd64) are also available if needed

@vercel
Copy link
Copy Markdown

vercel Bot commented May 21, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agent-canvas Ready Ready Preview, Comment May 21, 2026 12:52pm

Request Review

@VascoSch92 VascoSch92 requested a review from all-hands-bot May 21, 2026 12:47
Copy link
Copy Markdown
Contributor

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

Taste Rating: 🟢 Good taste - Clean, focused bug fix that solves a real problem

This PR properly addresses the repository skills loading gap with a pragmatic approach:

Clean data flow: Skills are fetched from the workspace root (not ephemeral worktree subdirs), converted to SDK wire format, and injected into the agent context

Appropriate scope: The conversation-scoped skill hooks (useConversationSkills) ensure the slash-command menu and skills modal show exactly what's loaded into each conversation

Solid error handling: getProjectSkills() returns an empty array on failure rather than blocking conversation start

Comprehensive testing: Covers the SkillInfo→Skill conversion, workspace root resolution (with regression guard), cloud backend fallback, and both OpenHands/ACP agent paths

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🟢 LOW

Bug fix that restores expected functionality (repository skills loading into conversations). Additive change with good test coverage, error handling prevents blocking conversation start, and the temporary nature of the workaround is documented.

VERDICT:
Worth merging: Solid implementation that fixes the reported issue. Clean separation of concerns, appropriate error handling, and comprehensive test coverage.

KEY INSIGHT:
The workspace root resolution is crucial - project skills must be loaded from the workspace root, not the per-conversation worktree subdirectory, which is ephemeral and has no .agents/skills/ directory.


Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/agent-canvas/actions/runs/26226841601

@VascoSch92 VascoSch92 marked this pull request as ready for review May 21, 2026 12:50
Copy link
Copy Markdown
Contributor

@all-hands-bot all-hands-bot left a comment

Choose a reason for hiding this comment

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

Taste Rating: 🟢 Good taste - Clean, focused bug fix that solves a real problem

This PR properly addresses the repository skills loading gap with a pragmatic approach:

Clean data flow: Skills are fetched from the workspace root (not ephemeral worktree subdirs), converted to SDK wire format, and injected into the agent context

Appropriate scope: The conversation-scoped skill hooks (useConversationSkills) ensure the slash-command menu and skills modal show exactly what's loaded into that conversation

Robust error handling: getProjectSkills() fails gracefully (returns []) so a skills loading hiccup never blocks conversation creation

Comprehensive tests: Covers SkillInfo→Skill conversion, project-only requests, cloud/error fallbacks, injection into both OpenHands and ACP agent contexts, and a regression guard for workspace root loading

Agent Context Change Note: This PR changes what content is available in agent context (project skills from .agents/skills/ are now loaded). This is a bug fix restoring intended functionality - the skills were designed to load but never did due to the missing wiring. Since this adds repository-specific context to prompts, maintainers may want to run lightweight evals before merging if there's concern about benchmark performance impact.

[RISK ASSESSMENT]

  • [Overall PR] ⚠️ Risk Assessment: 🟢 LOW

This is a straightforward bug fix with comprehensive test coverage. The change is additive (doesn't break existing functionality), fails gracefully on errors, and the implementation is clean and well-documented. The only consideration is that it changes agent context content, but it's restoring functionality that should have worked from the start.

VERDICT:
Worth merging: Solid bug fix with clean implementation and thorough testing

KEY INSIGHT:
The workspace root vs. per-conversation worktree distinction (skillsProjectDir vs. workingDir) is critical - the regression test guarding this ensures skills load from the persistent .agents/skills/ directory rather than ephemeral conversation subdirs.


Was this automated review useful? React with 👍 or 👎 to this review to help us measure review quality.
Workflow run: https://github.com/OpenHands/agent-canvas/actions/runs/26227020974

@VascoSch92 VascoSch92 requested a review from hieptl May 21, 2026 12:58
@github-actions
Copy link
Copy Markdown
Contributor

📸 Snapshot Test Report

Warning

Snapshot comparison step crashed (timeout, OOM, or runner error) — diff results below may be incomplete or absent.
Check the CI logs for the full error output (look for the "Run snapshot comparison" step).

Warning

One or more snapshot tests crashed during generation — some snapshots below may be incomplete.
Check the CI logs for the full error output (look for the "Generate current PR snapshots" step).

❌ 1 snapshot differ from the main branch baseline. Add the update-snapshots label to acknowledge intentional changes.

Category Count
🔴 Changed 1
🆕 New 0
✅ Unchanged 72
Total 73

How to resolve:

  • Unintentional diffs — the baselines on main may have moved since this branch was created. Merge the latest main into this branch and re-run CI.
  • Intentional changes — add the update-snapshots label. CI will pass and the new screenshots become the baseline when this PR merges.
🔴 Changed snapshots (1)

backends-extended

backend-after-switch

Expected (main) Actual (PR) Diff
expected actual diff
✅ Unchanged snapshots (72)

archived-conversation

  • conversation-panel-with-archived-badges
  • conversation-view-archived
  • conversation-view-sandbox-error

automations

  • automations-delete-modal
  • automations-list-active-inactive
  • automations-no-automations
  • automations-search-no-results

backends-extended

  • backend-add-blank-disabled
  • backend-add-cloud-advanced-open
  • backend-add-cloud-no-key-disabled
  • backend-add-cloud-with-key-enabled
  • backend-add-form-partially-filled
  • backend-add-invalid-url-disabled
  • backend-add-local-ready
  • backend-add-name-only-disabled
  • backend-add-two-column-layout
  • backend-add-whitespace-host-disabled
  • backend-cancel-nothing-saved
  • backend-dropdown-two-backends
  • backend-edit-prefilled
  • backend-manage-after-removal
  • backend-manage-two-listed
  • backend-remove-cancelled
  • backend-remove-confirmation
  • backend-switch-overlay

backends

  • backend-add-modal
  • backend-manage-modal
  • backend-selector-open

changes-tab

  • changes-deleted-file
  • changes-diff-viewer
  • changes-empty

collapsible-thinking

  • reasoning-content-collapsed
  • reasoning-content-expanded
  • think-action-collapsed
  • think-action-expanded

mcp-page

  • mcp-custom-server-1-editor-open
  • mcp-custom-server-2-url-filled
  • mcp-custom-server-3-all-filled
  • mcp-custom-server-4-installed
  • mcp-custom-server-editor
  • mcp-empty-installed
  • mcp-search-filtered
  • mcp-slack-install-1-marketplace
  • mcp-slack-install-2-modal
  • mcp-slack-install-3-filled
  • mcp-slack-install-4-installed

onboarding

  • onboarding-step-0-choose-agent
  • onboarding-step-1-check-backend
  • onboarding-step-2-setup-llm
  • onboarding-step-3-say-hello

projects-workspace-browser

  • projects-workspace-browser

settings-page

  • add-backend-modal
  • analytics-consent-modal
  • home-screen
  • settings-app-page
  • settings-page

settings-secrets

  • secrets-add-form-filled
  • secrets-add-form
  • secrets-after-save
  • secrets-delete-confirm
  • secrets-list

settings-verification

  • condenser-settings
  • verification-settings-off
  • verification-settings-on

sidebar

  • sidebar-collapsed
  • sidebar-conversation-panel
  • sidebar-filter-menu

skills-page

  • skills-empty
  • skills-loaded
  • skills-no-match
  • skills-search-filtered
  • skills-type-filter

Generated by the Snapshot Tests workflow. This comment was created by an AI agent (OpenHands) on behalf of the repo maintainers.

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.

[Bug] Repository (project) skills are not loaded into conversations in local mode

2 participants