From 583feff6a481adfdcc9312ab3de23522f5ca9e18 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 10:47:26 -0400 Subject: [PATCH 1/9] refactor: consolidate user-level config directories via btw_user_dirs() Introduces `btw_user_dirs()` as a single source of truth for all user-level btw config locations, returning paths in decreasing priority order (~/.btw, ~/.config/btw, tools::R_user_dir("btw")). Previously, skills, agents, and btw.md each hardcoded their own inconsistent sets of user-level paths. Skills used `tools::R_user_dir("btw", "config")` (wrong `which` argument, shipped in v1.2.0), agents used ~/.btw and ~/.config/btw, and btw.md used the home dir and ~/.config/btw. Now all three subsystems use `btw_user_dirs()`: - `path_find_user()`: gains ~/.btw and tools::R_user_dir("btw") lookups - `find_user_agent_files()`: gains tools::R_user_dir("btw") lookup - `btw_skills_directories()`: uses rev(btw_user_dirs()) for correct increasing-priority insertion; retains tools::R_user_dir("btw","config") as a legacy read-only fallback for skills installed with btw <= 1.2.0 - `resolve_skill_scope("user")`: new resolve_user_skill_dir() picks the first non-empty existing candidate from btw_user_dirs(), defaulting to ~/.btw/skills --- R/tool-skills.R | 44 +++++++++++++++++++++++++++++++------------- R/utils.R | 26 ++++++++++++++++---------- 2 files changed, 47 insertions(+), 23 deletions(-) diff --git a/R/tool-skills.R b/R/tool-skills.R index 9b4ff5e2..b7dcdefd 100644 --- a/R/tool-skills.R +++ b/R/tool-skills.R @@ -23,7 +23,10 @@ NULL #' 1. Skills bundled with the btw package itself #' 2. Skills from currently **attached** R packages — any package with an #' `inst/skills/` directory that is loaded via [library()] or [require()] -#' 3. User-level skills (`tools::R_user_dir("btw", "config")/skills`) +#' 3. User-level skills (`~/.btw/skills`, `~/.config/btw/skills`, +#' `tools::R_user_dir("btw")/skills`). For backwards compatibility, the +#' legacy `tools::R_user_dir("btw", "config")/skills` path used by btw +#' <= 1.2.0 is also included at lower priority. #' 4. Project-level skills (`.btw/skills/` or `.agents/skills/`) #' #' @param name The name of the skill to load, or `""` to list all available @@ -156,13 +159,19 @@ btw_skills_directories <- function(project_dir = getwd()) { # Skills from attached packages dirs <- c(dirs, attached_package_skill_dirs()) - # User-level skills (global installation) - user_skills_dir <- file.path( - tools::R_user_dir("btw", "config"), - "skills" - ) - if (dir.exists(user_skills_dir)) { - dirs <- c(dirs, user_skills_dir) + # Legacy: btw <= 1.2.0 install target — kept for backwards compatibility only, + # never written to by newer versions + legacy_skills_dir <- file.path(tools::R_user_dir("btw", "config"), "skills") + if (dir.exists(legacy_skills_dir)) { + dirs <- c(dirs, legacy_skills_dir) + } + + # User-level skills from btw_user_dirs() in increasing priority order + for (user_dir in rev(btw_user_dirs())) { + user_skills_dir <- file.path(user_dir, "skills") + if (dir.exists(user_skills_dir) && !user_skills_dir %in% dirs) { + dirs <- c(dirs, user_skills_dir) + } } # Project-level skills from multiple conventions @@ -738,11 +747,18 @@ resolve_skill_scope <- function(scope, error_call = caller_env()) { switch( scope, project = resolve_project_skill_dir(error_call = error_call), - user = file.path(tools::R_user_dir("btw", "config"), "skills"), + user = resolve_user_skill_dir(), scope ) } +resolve_user_skill_dir <- function() { + candidates <- file.path(btw_user_dirs(), "skills") + existing <- candidates[dir.exists(candidates)] + non_empty <- existing[map_lgl(existing, function(d) length(list.files(d)) > 0)] + if (length(non_empty) > 0) non_empty[[1]] else candidates[[1]] +} + select_skill_dir <- function( skill_dirs, skill = NULL, @@ -813,8 +829,9 @@ select_skill_dir <- function( #' chosen from `.btw/skills/` or `.agents/skills/` #' in that order. If one already exists, it is used; otherwise #' `.btw/skills/` is created. -#' - `"user"`: Installs to the user-level skills directory -#' (`tools::R_user_dir("btw", "config")/skills`). +#' - `"user"`: Installs to the first of `~/.btw/skills`, +#' `~/.config/btw/skills`, or `tools::R_user_dir("btw")/skills` that +#' already exists, defaulting to `~/.btw/skills` if none do. #' - A directory path: Installs to a custom directory, e.g. #' `scope = ".openhands/skills"`. Use `I("project")` or `I("user")` #' if you need a literal directory with those names. @@ -917,8 +934,9 @@ btw_skill_install_github <- function( #' chosen from `.btw/skills/` or `.agents/skills/` #' in that order. If one already exists, it is used; otherwise #' `.btw/skills/` is created. -#' - `"user"`: Installs to the user-level skills directory -#' (`tools::R_user_dir("btw", "config")/skills`). +#' - `"user"`: Installs to the first of `~/.btw/skills`, +#' `~/.config/btw/skills`, or `tools::R_user_dir("btw")/skills` that +#' already exists, defaulting to `~/.btw/skills` if none do. #' - A directory path: Installs to a custom directory, e.g. #' `scope = ".openhands/skills"`. Use `I("project")` or `I("user")` #' if you need a literal directory with those names. diff --git a/R/utils.R b/R/utils.R index a2299571..e018c37a 100644 --- a/R/utils.R +++ b/R/utils.R @@ -152,17 +152,28 @@ path_find_in_project <- function(filename, dir = getwd()) { path_find_in_project(filename, dirname(dir)) } +# Returns user-level config directories in decreasing priority order. +# Consumers iterate and use the first match (or rev() when building +# an increasing-priority list like btw_skills_directories()). +btw_user_dirs <- function() { + c( + fs::path_home(".btw"), + fs::path_home(".config", "btw"), + tools::R_user_dir("btw") + ) +} + path_find_user <- function(filename) { if (identical(Sys.getenv("TESTTHAT"), "true")) { # In testthat, we don't want to use the home directory return(NULL) } - possibilities <- c( + possibilities <- unique(c( fs::path_home(filename), fs::path_home_r(filename), - fs::path_home(".config", "btw", filename) - ) + file.path(btw_user_dirs(), filename) + )) for (path in possibilities) { if (fs::file_exists(path)) { @@ -191,19 +202,14 @@ find_project_agent_files <- function(dir = getwd()) { as.character(files) } -# Find agent-*.md files in user config directories (~/.btw/, ~/.config/btw/) +# Find agent-*.md files in user config directories find_user_agent_files <- function() { if (identical(Sys.getenv("TESTTHAT"), "true")) { return(character()) } - user_dirs <- c( - fs::path_home(".btw"), - fs::path_home(".config", "btw") - ) - files <- character() - for (dir in user_dirs) { + for (dir in btw_user_dirs()) { if (fs::dir_exists(dir)) { found <- fs::dir_ls(dir, regexp = "agent-.*\\.md$", type = "file") files <- c(files, as.character(found)) From 814cfe97780865fa02f1c441d14581aa315f5aaf Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 11:04:39 -0400 Subject: [PATCH 2/9] docs: note cli message leakage pattern in test conventions --- AGENTS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/AGENTS.md b/AGENTS.md index 3a9445cf..3462ed44 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -302,6 +302,7 @@ Edit files in `inst/prompts/`. These are assembled in `btw_client()` based on co - **All tool wrappers accept `_intent` parameter** (added automatically by `wrap_with_intent()`) - **Snapshot tests for tool outputs** to catch formatting changes - **Use `check_*()` functions** argument validation, described below +- **Suppress cli messages in tests** - `expect_message(expr, pattern)` only absorbs the first *matching* message; non-matching messages (e.g. a `cli_alert_info()` emitted before the one under test) leak to the console even when the test passes. Wrap with `suppressMessages(expect_message(...))` when secondary messages are expected but unimportant, or use `suppressMessages(expr)` alone when the assertion is a side-effect rather than the message content. ### Type Check Functions From 94fe20ced659b5f2805c54b33b0fe1ac541c61d3 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 11:13:38 -0400 Subject: [PATCH 3/9] docs: document btw_user_dirs() in AGENTS.md and fix stale btw.md locations --- AGENTS.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/AGENTS.md b/AGENTS.md index 3462ed44..cc7696a6 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -134,7 +134,7 @@ Follow these style rules: ... btw looks for these files in: 1. Current working directory or its parents (project-specific) -2. `~/.config/btw/btw.md` or `~/btw.md` (global defaults) +2. `~/.btw/btw.md`, `~/.config/btw/btw.md`, or `~/btw.md` (global defaults) Content becomes part of the system prompt. Use `` / `` comments to exclude sections from the prompt. @@ -340,6 +340,7 @@ Edit files in `inst/prompts/`. These are assembled in `btw_client()` based on co - **Git and GitHub commands** - `@git` and `@issue`/`@pr` commands require gert and gh packages respectively, plus appropriate repository access - **MCP server blocks** the R process - run non-interactively - **Test snapshots require frequent updates** as output formatting evolves +- **`btw_user_dirs()` is the single source of truth** for user-level config locations (`~/.btw`, `~/.config/btw`, `tools::R_user_dir("btw")`). Any code that discovers or installs user-level skills, agents, or `btw.md` files must use this helper — never hardcode individual paths ## Resources From bc1dd74a248e06a85f794f65b26a94d48aac7878 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 11:21:08 -0400 Subject: [PATCH 4/9] docs: add NEWS entry for btw_user_dirs() consolidation (#182) --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index b6ffa7c8..c1e4b782 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,8 @@ ## Bug fixes +* User-level config locations are now consistent across skills, agents, and `btw.md` discovery. A new internal `btw_user_dirs()` helper returns `~/.btw`, `~/.config/btw`, and `tools::R_user_dir("btw")` as the canonical search paths. Skills installed with btw <= 1.2.0 (which used `tools::R_user_dir("btw", "config")`) are still discovered for backwards compatibility (#182). + * The `btw` CLI now loads `datasets`, `utils`, `stats`, and `methods` by declaring them in the Rapp `#| launcher:` frontmatter, reducing surprises for users who expect standard R packages to be available (#181). # btw 1.2.0 From c716e9da0701d7c08aa4e6ccd95c1f076dabd312 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 11:24:36 -0400 Subject: [PATCH 5/9] docs: simplify NEWS entry for user-level config fix --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index c1e4b782..f85e4174 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,7 +2,7 @@ ## Bug fixes -* User-level config locations are now consistent across skills, agents, and `btw.md` discovery. A new internal `btw_user_dirs()` helper returns `~/.btw`, `~/.config/btw`, and `tools::R_user_dir("btw")` as the canonical search paths. Skills installed with btw <= 1.2.0 (which used `tools::R_user_dir("btw", "config")`) are still discovered for backwards compatibility (#182). +* User-level config locations are now consistent across skills, agents, and `btw.md` discovery. Skills installed with btw <= 1.2.0 are still discovered for backwards compatibility (#182). * The `btw` CLI now loads `datasets`, `utils`, `stats`, and `methods` by declaring them in the Rapp `#| launcher:` frontmatter, reducing surprises for users who expect standard R packages to be available (#181). From 2ed0ebd051a9b3ad0d04619bc71149a79bc29ebd Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 12:08:46 -0400 Subject: [PATCH 6/9] docs: clarify legacy skills path was only used by v1.2.0 --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index f85e4174..29901c59 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,7 +2,7 @@ ## Bug fixes -* User-level config locations are now consistent across skills, agents, and `btw.md` discovery. Skills installed with btw <= 1.2.0 are still discovered for backwards compatibility (#182). +* User-level config locations are now consistent across skills, agents, and `btw.md` discovery. Skills installed with btw v1.2.0 are still discovered for backwards compatibility (#182). * The `btw` CLI now loads `datasets`, `utils`, `stats`, and `methods` by declaring them in the Rapp `#| launcher:` frontmatter, reducing surprises for users who expect standard R packages to be available (#181). From c35866844af5cc40d084fc6435897b93ebca8e27 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 14:17:03 -0400 Subject: [PATCH 7/9] fix: find_skill() returns highest-priority version of a duplicate skill MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit btw_skills_directories() builds its list in increasing priority order (last = highest). find_skill() was iterating in that same order and returning on first match, meaning it returned the *lowest*-priority version — the opposite of btw_skills_list()'s last-wins semantics. Fix by iterating rev(skill_dirs) in both the fast and slow paths so find_skill() and btw_skills_list() agree on which version wins. Also adds tests for find_skill() priority resolution and resolve_user_skill_dir() selection semantics. --- R/tool-skills.R | 8 +++++--- tests/testthat/test-tool_skills.R | 34 +++++++++++++++++++++++++++++++ 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/R/tool-skills.R b/R/tool-skills.R index b7dcdefd..a80d3f1c 100644 --- a/R/tool-skills.R +++ b/R/tool-skills.R @@ -322,8 +322,11 @@ btw_skills_list <- function() { find_skill <- function(skill_name) { skill_dirs <- btw_skills_directories() + # Search in reverse priority order so higher-priority sources win, consistent + # with btw_skills_list() which uses last-wins semantics. + # Fast path: directory named skill_name - for (dir in skill_dirs) { + for (dir in rev(skill_dirs)) { skill_dir <- file.path(dir, skill_name) skill_md_path <- file.path(skill_dir, "SKILL.md") if (dir.exists(skill_dir) && file.exists(skill_md_path)) { @@ -337,10 +340,9 @@ find_skill <- function(skill_name) { } # Slow path: scan all skills for a matching metadata$name (handles name/dir - # mismatch). We use extract_skill_metadata() first to avoid full validation # on every non-matching skill, then validate only when we find a match. - for (dir in skill_dirs) { + for (dir in rev(skill_dirs)) { subdirs <- list.dirs(dir, full.names = TRUE, recursive = FALSE) for (subdir in subdirs) { skill_md_path <- file.path(subdir, "SKILL.md") diff --git a/tests/testthat/test-tool_skills.R b/tests/testthat/test-tool_skills.R index 6d7f23bc..edb80476 100644 --- a/tests/testthat/test-tool_skills.R +++ b/tests/testthat/test-tool_skills.R @@ -355,6 +355,40 @@ test_that("find_skill() finds a valid skill", { expect_true(file.exists(result$path)) }) +test_that("find_skill() returns the highest-priority version when skill exists in multiple dirs", { + low_dir <- withr::local_tempdir() + high_dir <- withr::local_tempdir() + create_temp_skill(name = "my-skill", description = "Low priority version", dir = low_dir) + create_temp_skill(name = "my-skill", description = "High priority version", dir = high_dir) + # high_dir last = higher priority (consistent with btw_skills_directories() semantics) + local_skill_dirs(c(low_dir, high_dir)) + + result <- find_skill("my-skill") + fm <- frontmatter::read_front_matter(result$path) + expect_equal(fm$data$description, "High priority version") +}) + +# resolve_user_skill_dir() ------------------------------------------------- + +test_that("resolve_user_skill_dir() returns first non-empty existing candidate", { + dir1 <- withr::local_tempdir() + dir2 <- withr::local_tempdir() + skills2 <- file.path(dir2, "skills") + dir.create(skills2) + writeLines("x", file.path(skills2, "placeholder")) + + local_mocked_bindings(btw_user_dirs = function() c(dir1, dir2)) + expect_equal(resolve_user_skill_dir(), skills2) +}) + +test_that("resolve_user_skill_dir() falls back to first candidate when none have content", { + dir1 <- withr::local_tempdir() + dir2 <- withr::local_tempdir() + + local_mocked_bindings(btw_user_dirs = function() c(dir1, dir2)) + expect_equal(resolve_user_skill_dir(), file.path(dir1, "skills")) +}) + # extract_skill_metadata --------------------------------------------------- test_that("extract_skill_metadata() returns parsed frontmatter", { From 6573cb7681a40a54d9cbe4a024845fb00f710074 Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 14:21:14 -0400 Subject: [PATCH 8/9] docs: clarify non-empty check rationale in resolve_user_skill_dir() --- R/tool-skills.R | 3 +++ 1 file changed, 3 insertions(+) diff --git a/R/tool-skills.R b/R/tool-skills.R index a80d3f1c..afc19d0b 100644 --- a/R/tool-skills.R +++ b/R/tool-skills.R @@ -757,6 +757,9 @@ resolve_skill_scope <- function(scope, error_call = caller_env()) { resolve_user_skill_dir <- function() { candidates <- file.path(btw_user_dirs(), "skills") existing <- candidates[dir.exists(candidates)] + # Require content, not just existence: an empty directory (e.g. one left + # behind after removing a skill) should not cause subsequent installs to + # land there instead of the preferred default (~/.btw/skills). non_empty <- existing[map_lgl(existing, function(d) length(list.files(d)) > 0)] if (length(non_empty) > 0) non_empty[[1]] else candidates[[1]] } From de803a5d9f12efb5f78832be64fbc769592381bf Mon Sep 17 00:00:00 2001 From: Garrick Aden-Buie Date: Wed, 18 Mar 2026 19:09:49 -0400 Subject: [PATCH 9/9] docs: tweak docs --- R/tool-skills.R | 8 +++++--- man/btw_skill_install_github.Rd | 5 +++-- man/btw_skill_install_package.Rd | 5 +++-- man/btw_tool_skill.Rd | 5 ++++- 4 files changed, 15 insertions(+), 8 deletions(-) diff --git a/R/tool-skills.R b/R/tool-skills.R index afc19d0b..e7393552 100644 --- a/R/tool-skills.R +++ b/R/tool-skills.R @@ -25,8 +25,8 @@ NULL #' `inst/skills/` directory that is loaded via [library()] or [require()] #' 3. User-level skills (`~/.btw/skills`, `~/.config/btw/skills`, #' `tools::R_user_dir("btw")/skills`). For backwards compatibility, the -#' legacy `tools::R_user_dir("btw", "config")/skills` path used by btw -#' <= 1.2.0 is also included at lower priority. +#' legacy `tools::R_user_dir("btw", "config")/skills` path used by briefly +#' by btw 1.2.0 is also included at lower priority. #' 4. Project-level skills (`.btw/skills/` or `.agents/skills/`) #' #' @param name The name of the skill to load, or `""` to list all available @@ -760,7 +760,9 @@ resolve_user_skill_dir <- function() { # Require content, not just existence: an empty directory (e.g. one left # behind after removing a skill) should not cause subsequent installs to # land there instead of the preferred default (~/.btw/skills). - non_empty <- existing[map_lgl(existing, function(d) length(list.files(d)) > 0)] + non_empty <- existing[map_lgl(existing, function(d) { + length(list.files(d)) > 0 + })] if (length(non_empty) > 0) non_empty[[1]] else candidates[[1]] } diff --git a/man/btw_skill_install_github.Rd b/man/btw_skill_install_github.Rd index 7b246b5f..c22ddf16 100644 --- a/man/btw_skill_install_github.Rd +++ b/man/btw_skill_install_github.Rd @@ -27,8 +27,9 @@ non-interactive sessions).} chosen from \verb{.btw/skills/} or \verb{.agents/skills/} in that order. If one already exists, it is used; otherwise \verb{.btw/skills/} is created. -\item \code{"user"}: Installs to the user-level skills directory -(\code{tools::R_user_dir("btw", "config")/skills}). +\item \code{"user"}: Installs to the first of \verb{~/.btw/skills}, +\verb{~/.config/btw/skills}, or \code{tools::R_user_dir("btw")/skills} that +already exists, defaulting to \verb{~/.btw/skills} if none do. \item A directory path: Installs to a custom directory, e.g. \code{scope = ".openhands/skills"}. Use \code{I("project")} or \code{I("user")} if you need a literal directory with those names. diff --git a/man/btw_skill_install_package.Rd b/man/btw_skill_install_package.Rd index 77310202..94ed132e 100644 --- a/man/btw_skill_install_package.Rd +++ b/man/btw_skill_install_package.Rd @@ -24,8 +24,9 @@ non-interactive sessions).} chosen from \verb{.btw/skills/} or \verb{.agents/skills/} in that order. If one already exists, it is used; otherwise \verb{.btw/skills/} is created. -\item \code{"user"}: Installs to the user-level skills directory -(\code{tools::R_user_dir("btw", "config")/skills}). +\item \code{"user"}: Installs to the first of \verb{~/.btw/skills}, +\verb{~/.config/btw/skills}, or \code{tools::R_user_dir("btw")/skills} that +already exists, defaulting to \verb{~/.btw/skills} if none do. \item A directory path: Installs to a custom directory, e.g. \code{scope = ".openhands/skills"}. Use \code{I("project")} or \code{I("user")} if you need a literal directory with those names. diff --git a/man/btw_tool_skill.Rd b/man/btw_tool_skill.Rd index a5a66190..83753f65 100644 --- a/man/btw_tool_skill.Rd +++ b/man/btw_tool_skill.Rd @@ -38,7 +38,10 @@ priority (later sources override earlier ones when skill names conflict): \item Skills bundled with the btw package itself \item Skills from currently \strong{attached} R packages — any package with an \verb{inst/skills/} directory that is loaded via \code{\link[=library]{library()}} or \code{\link[=require]{require()}} -\item User-level skills (\code{tools::R_user_dir("btw", "config")/skills}) +\item User-level skills (\verb{~/.btw/skills}, \verb{~/.config/btw/skills}, +\code{tools::R_user_dir("btw")/skills}). For backwards compatibility, the +legacy \code{tools::R_user_dir("btw", "config")/skills} path used by briefly +by btw 1.2.0 is also included at lower priority. \item Project-level skills (\verb{.btw/skills/} or \verb{.agents/skills/}) } }