Skip to content

fix(sandbox): copy complete bundled skill directories#655

Open
hreiten wants to merge 1 commit into
ColeMurray:mainfrom
watchdog-no:fix/copy-bundled-skill-directories
Open

fix(sandbox): copy complete bundled skill directories#655
hreiten wants to merge 1 commit into
ColeMurray:mainfrom
watchdog-no:fix/copy-bundled-skill-directories

Conversation

@hreiten
Copy link
Copy Markdown
Contributor

@hreiten hreiten commented May 18, 2026

Summary

  • copy each valid bundled skill directory into .opencode/skills, instead of copying only its SKILL.md
  • keep the existing rule that only directories containing SKILL.md are installed
  • skip generated local artifacts such as __pycache__, *.pyc, and .DS_Store
  • expand the installer test to cover companion files, fresh installs, merge behavior, and ignored non-skill entries

Why this is useful

Bundled skills are a platform extension point, but the current installer only copies the top-level SKILL.md. That means any skill which needs companion files, such as helper scripts, references, templates, or assets, is silently incomplete inside the sandbox even though those files are present in the runtime image.

Copying the full validated skill directory makes bundled skills self-contained and lets future skills grow beyond a single markdown file without each one needing custom installer logic. The change keeps the existing safety boundary: loose files in the skills root and directories without SKILL.md are still ignored.

Prior upstream work checked

  • Refreshed upstream/main before creating this branch.
  • Checked upstream/main for _install_skills, .opencode/skills, shutil.copy(skill_file, and copytree(skill_dir.
  • Reviewed related upstream PRs found by searching for _install_skills, .opencode/skills, bundled skills, and copytree. The matching PRs added or documented skills, but did not change the installer to copy complete skill directories.

Test plan

  • cd packages/sandbox-runtime && ./.venv/bin/ruff check .
  • cd packages/sandbox-runtime && ./.venv/bin/ruff format --check .
  • cd packages/sandbox-runtime && ./.venv/bin/pytest tests/test_tool_installation.py -q
  • cd packages/sandbox-runtime && ./.venv/bin/pytest tests/ -q
  • git diff --check upstream/main...HEAD

The full sandbox-runtime test suite passes with the existing RuntimeWarnings in tests/test_bridge_git_identity.py.

Summary by CodeRabbit

  • Bug Fixes
    • Enhanced skills installation to copy complete skill directories with improved file handling, properly excluding cache and system files while preserving symlinks for more reliable installations.

Review Change Stack

@coderabbitai
Copy link
Copy Markdown

coderabbitai Bot commented May 18, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 4b78761f-4e31-45ec-ad36-d2a5d423aea2

📥 Commits

Reviewing files that changed from the base of the PR and between a8a14c4 and 77ed993.

📒 Files selected for processing (2)
  • packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py
  • packages/sandbox-runtime/tests/test_tool_installation.py

📝 Walkthrough

Walkthrough

The PR enhances the skill installation process to copy complete skill directory trees instead of only the SKILL.md file. The implementation uses shutil.copytree with symlink preservation and intelligent filtering of Python cache and system artifacts. Tests are expanded to validate the behavior across multiple skills, nested directories, cache exclusion, existing file preservation, and non-skill entry filtering.

Changes

Skill Directory Installation Enhancement

Layer / File(s) Summary
Full-tree skill directory copying with comprehensive testing
packages/sandbox-runtime/src/sandbox_runtime/entrypoint.py, packages/sandbox-runtime/tests/test_tool_installation.py
_install_skills now uses shutil.copytree with symlink preservation and cache-file exclusion (__pycache__, *.pyc, .DS_Store) to deploy entire skill directories. Tests construct multiple skills with nested content, verify __pycache__ exclusion, confirm existing file preservation, validate companion file copying, and confirm non-skill entry rejection.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 Skill directories now travel whole and complete,
No more lonely SKILL.md feet!
Cache files filtered, symlinks preserved with care,
Multiple skills installed everywhere. ✨

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'fix(sandbox): copy complete bundled skill directories' directly and clearly summarizes the main change: updating the installer to copy entire skill directories instead of just SKILL.md files.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
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.

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.

1 participant