Conversation
There was a problem hiding this comment.
I went ahead and ported the "real" skill-creator skill (from Anthropic) by default. I think it's probably worth porting the .py files to .R so that the agent can just source(file-path).
| user_skills_dir <- file.path( | ||
| tools::R_user_dir("btw", "config"), | ||
| "skills" | ||
| ) |
There was a problem hiding this comment.
Some existing tools have tools::R_user_dir("btw", which = "cache"). Is this a reasonable directory to look in? I've used ~/.config/{pkg}/ in a few packages, but don't know if this is a Simon-ism. Whatever directory we land on, it should ofc be documented. :)
There was a problem hiding this comment.
There's a helper that wraps tools::R_user_dir() -- path_btw_cache(). My take is: this is for cached files used and written by btw. I'm currently only using it for some code related to Shiny bookmarks, but I have plans to have btw_app() remember conversation history and we'd use it there.
Separately, there's path_find_user() that considers these locations:
possibilities <- c(
fs::path_home(filename),
fs::path_home_r(filename),
fs::path_home(".config", "btw", filename)
)So ~/.config/btw/skills would be a natural place to put this that fits in with currently places we look for btw.md. (This is only briefly mentioned in ?btw_client and ?edit_btw_md.)
On the other hand, I could also see ~/.btw/skills making sense.
What do you think about inheriting installed Claude skills too from ~/.claude/skills?
Also: I like using inst/skills just like inst/prompts! I wonder how hard it is to look through installed packages to auto-discover these skills. Or maybe we'd need an option that helps bring in skills from packages.
There was a problem hiding this comment.
Apparently also .github/skills now too https://code.visualstudio.com/docs/copilot/customization/agent-skills
There was a problem hiding this comment.
Actually, .agents/skills is now emerging as the standard. This list of locations is helpful: https://github.com/vercel-labs/skills#supported-agents
| Skills may include bundled resources: | ||
| - **Scripts**: Executable code (R, Python, bash) for automated tasks | ||
| - **References**: Additional documentation to consult as needed | ||
| - **Assets**: Templates and files for use in outputs |
There was a problem hiding this comment.
This is auto-injected into btw_client() (and thus btw_app())'s context, along with a btw_list_skills() summary so that the agent knows what it has access to via a fetch_skill() tool call. This is Claude Code's design.
There was a problem hiding this comment.
I would be open to some sort of preferential treatment given to https://github.com/posit-dev/skills as well
I really like this, but I wonder if we should have it work a little bit differently in btw where we would have a tool for fetching a skill and a separate tool for fetching resources and another tool for running skill scripts. (Maybe the fetching skill tool could also get resources with an optional argument.) There are generally two things that I want to enable. We don't currently have a way to run general system commands, i.e. we don't have an analogous tool to Claude's Bash tool. I'm a little hesitant to create one but much less hesitant about a tool with reduced scope that specifically could run skill scripts. The other thought is that Also, dynamically choosing tools means we might not want to inject the available tools into the system prompt. We could instead put them in the tool description. I'm not sure if that makes them more or less effective, but it does mean that we can know we're not adding false directions to the system prompt accidentally. |
Linking to #148. I guess we would probably still want |
…d parsing Use `frontmatter::read_front_matter()` (already in Imports) instead of manual YAML extraction with `yaml::yaml.load()`. This aligns with the pattern used in btw_client.R and simplifies both `extract_skill_metadata()` and `btw_tool_fetch_skill_impl()`. Removes `yaml` from Suggests since it's no longer used anywhere.
- Add `validate_skill()` internal function implementing the Agent Skills spec validation rules: name format (lowercase, hyphens, max 64 chars, must match directory name), description constraints (max 1024 chars), compatibility (max 500 chars), unexpected field detection - Integrate validation into `btw_list_skills()` — invalid skills are skipped with a warning instead of silently loaded - Surface `compatibility` and `allowed-tools` fields in system prompt XML - Fix `list_files_in_subdir()` to use recursive listing for nested resource directories (e.g., assets/hello-world/) - Add skills group icon mapping in `tool_group_icon()`
…_install() User-facing R functions for skill management: - btw_skill_create(): Initialize a new skill directory with SKILL.md template and optional resource directories (scripts/, references/, assets/). Supports project-level and user-level scopes. - btw_skill_validate(): Validate a skill directory against the Agent Skills spec, reporting issues with name format, description, and frontmatter structure. - btw_skill_install(): Install a skill from a .skill file (ZIP archive) or directory into project or user skill locations. Validates before installing.
…d management 77 tests covering: - validate_skill(): name format rules (uppercase, hyphens, length, directory mismatch), description constraints, optional fields, unexpected fields - btw_list_skills(): discovery, invalid skill warnings, metadata extraction - find_skill() and extract_skill_metadata() helpers - list_skill_resources(): recursive file listing, nested directories - btw_tool_fetch_skill_impl(): content fetching, resource listing, errors - btw_skills_system_prompt(): empty case, metadata including compatibility - btw_skill_create(): valid creation, resource directories, name validation - btw_skill_validate(): valid/invalid reporting, error handling - btw_skill_install(): directory and .skill file installation, validation
Scan three project-level directories for skills: 1. .btw/skills/ (preferred) 2. .agents/skills/ 3. .claude/skills/ When installing or creating skills with scope="project": - If no project skill dirs exist, defaults to .btw/skills/ - If exactly one exists, uses it - If multiple exist and session is interactive, prompts the user - If multiple exist and non-interactive, uses first by priority order Users can always pass a custom path via the scope parameter.
Add the skill's SKILL.md absolute path as a <location> field in the system prompt XML, following the Agent Skills integration guide recommendation for filesystem-based agents. This lets agents with file access (e.g. Claude Code) read SKILL.md directly without needing the fetch_skill tool. Exclude the skills tool group from btw_mcp_server() by default via a new btw_mcp_tools() helper. The skill system prompt with <available_skills> metadata is injected by btw_client(), not by MCP, so the fetch_skill tool would appear without context for the model to know what skills exist.
Update the skills system prompt to accurately describe that btw cannot directly execute skill scripts. Scripts are listed with paths but the agent should read them for reference or adapt their logic into R code for use with the R code execution tool. This is intentional: executing arbitrary bundled scripts requires a tool approval system that btw does not yet have.
…rces Add btw_skill_install_github() and btw_skill_install_package() as the new public API for installing skills. The original btw_skill_install() is refactored into an internal install_skill_from_dir() helper that both new functions delegate to. The .skill ZIP archive code path is removed. A shared select_skill_dir() helper extracts the duplicated skill selection logic (match by name, auto-select single, interactive menu).
- Add validate_skill() call in find_skill() so invalid skills return NULL - Add xml_escape() helper and apply to system prompt skill metadata - Extract validate_skill_name() shared helper, used by both validate_skill() and btw_skill_create() - Flag non-character compatibility field in validate_skill() - Warn on long description (>1024 chars) in btw_skill_create() - Warn on frontmatter parse failure in extract_skill_metadata() - Add overwrite parameter to install_skill_from_dir(), btw_skill_install_github(), and btw_skill_install_package()
- Add validate_skill() call in find_skill() to reject invalid skills - Add xml_escape() helper, apply to system prompt interpolation - Extract validate_skill_name() shared by validate_skill() and btw_skill_create() - Flag non-character compatibility field in validate_skill() - Warn on long description in btw_skill_create() - Warn on parse failure in extract_skill_metadata() - Add overwrite parameter to install functions - Add resolve_skill_scope() supporting "project", "user", custom paths, and I() escape hatch for literal directory names - Document scope resolution order and user-level path in roxygen
Remove the separate `ref` argument and instead parse the Git reference inline from the `repo` string, following the convention used by pak::pak() and remotes::install_github(). Extract parse_github_repo() into R/utils.R for reuse, with targeted tests in test-utils.R.
…ardening - Split validate_skill() into hard errors (missing SKILL.md, unparseable frontmatter, missing name/description) and soft warnings (name format, extra fields, length limits). Skills with only warnings are still loaded. - Eliminate <<- in tryCatch and fix double-message on parse failure - find_skill() now returns validation info instead of NULL for invalid skills - btw_tool_fetch_skill_impl() surfaces "exists but has validation errors" instead of generic "not found" when a skill is on disk but invalid - btw_skill_create() uses frontmatter::write_front_matter() instead of paste0, preventing malformed YAML from special characters in descriptions - btw_list_skills() emits informational message when a skill is overridden by a higher-priority directory - Fix cli_alert_success → cli_alert_info in resolve_project_skill_dir() - Add project_dir param to btw_skill_directories() (default getwd()) - Optimize btw_can_register with lightweight any_skills_exist() check - Extract validate_skill_name_format() for reuse - Add skill_validation() constructor for consistent return structure - New/updated tests: override ordering, parse failure warning, find_skill validation info, two-tier assertion updates (145 total)
- find_skill(): collapse dead duplicate return branches; add slow-path metadata$name scan so fetch works when frontmatter name differs from dir name - skill_validation(): carry parsed metadata through; validate_skill() passes it at final return; btw_list_skills() reads validation$metadata directly, eliminating the double-parse of SKILL.md - btw_list_skills(): use metadata$name as authoritative skill identity instead of basename(subdir); remove unreachable description fallback - resolve_project_skill_dir(): accept error_call; resolve_skill_scope() passes it through so cli_abort() reports the right call site - any_skills_exist(): remove redundant dir.exists() check (already gated by btw_skill_directories()) - validate_skill(): improve name/dir mismatch warning to recommend renaming - format_resources_listing(): normalize Scripts to \n\n spacing like References and Assets
Three tests compared paths using == after file.path(), which produces mixed separators on Windows (withr::local_tempdir() normalizes to backslashes, but file.path() appends with forward slashes). Wrap both sides of the affected expect_equal() calls with normalizePath() so the comparison is separator-agnostic.
Change `overwrite` default from `FALSE` to `NULL` in `btw_skill_install_github()`, `btw_skill_install_package()`, and `install_skill_from_dir()`. When `NULL` and a skill already exists, shows a `utils::menu()` prompt in interactive sessions; in non-interactive sessions silently defaults to `FALSE` (error on conflict). Explicit `TRUE`/`FALSE` behave as before.
In btw_client(), emit a cli::cli_warn() after setting tools if btw_tool_fetch_skill is present but btw_tool_files_read is not. In btw_app(), track the mismatch state with a shiny::reactive() and use shiny::observeEvent() to show a toast notification (via notifier()) only when transitioning into the mismatched state. Extracts notifier() from btw_status_bar_server to module scope so it can be reused for consistent toast styling.
Align with naming conventions used by other agent frameworks (Claude
Code's Skill, OpenCode's skill). Renames parameter skill_name to name,
updates tool description to instruct the model to proactively load
matching skills and handle /{name} syntax, and adds guidance to reuse
previously loaded skills.
These exported functions add maintenance surface area without proportional user value. Internal validation via validate_skill() remains intact for skill loading and installation.
Exposes the bundled skill-creator skill as an interactive task function, following the same pattern as btw_task_create_readme() and btw_task_create_btw_md(). Supports app, console, client, and tool modes with optional skill name parameter.
# Conflicts: # NAMESPACE # man/btw_task_create_btw_md.Rd # man/btw_task_create_readme.Rd
2 participants
This PR ports the skills system from side with a few changes. As in Claude Code, skills are discovered + briefly described at startup (name + description added to system prompt) but only fully loaded when the model calls
btw_tool_skills_fetch, keeping the base context lightweight. side had a multi-tool approach to interface with them, but I aligned more closely with Claude Code here where btw uses a single fetch tool that returns the SKILL.md content plus a listing of bundled resources—the model then uses existing file tools to access scripts and references as needed. Skills can be provided by the package (inst/skills/), user config directory, or project-local.btw/skills/folder.skills.mov
Opening as draft as there are still some rough edges I'd like to sand off; I'll leave comments on a couple of them now.