fix(skills): merge configured and workspace skill dirs

Harvested from PR #2737 by @h3c-hexin.

Co-authored-by: h3c-hexin <13790929+h3c-hexin@users.noreply.github.com>
This commit is contained in:
Hunter B
2026-06-03 21:39:15 -07:00
parent 7ac8063b6b
commit 9719b45cd3
4 changed files with 198 additions and 20 deletions
+5 -1
View File
@@ -54,6 +54,9 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Moved the TUI Starlark execpolicy parser and PTY support behind non-OHOS
target dependencies so published OpenHarmony builds no longer pull `nix` 0.28
through `rustyline` or `portable-pty`.
- Explicit `skills_dir` configuration is now unioned with workspace skill
discovery instead of being shadowed by workspace-local skills, and configured
skills take precedence over global defaults when prompt space is constrained.
### Community
@@ -64,7 +67,8 @@ prefix-cache stability work (#2517), **@xyuai** for canonical CodeWhale
settings-path migration work (#2730), **@gaord** for the runtime thread
workspace update API (#2640), **@shenjackyuanjie** for the
HarmonyOS/OpenHarmony port and MatePad Edge validation trail (#2634), and
**@idling11** for the PlanArtifact direction in Plan mode (#2733).
**@idling11** for the PlanArtifact direction in Plan mode (#2733), and
**@h3c-hexin** for the configured `skills_dir` merge fix (#2737).
## [0.8.53] - 2026-06-03
+80 -7
View File
@@ -1068,13 +1068,16 @@ pub fn system_prompt_for_mode_with_context_skills_session_and_approval(
// skills directory (`.agents/skills`, `skills`,
// `.opencode/skills`, `.claude/skills`, `.cursor/skills`) plus global
// `~/.agents/skills` / `~/.deepseek/skills` so skills installed for any
// AI-tool convention show up in the catalogue. The legacy
// single-`skills_dir` path is
// honoured as a fallback for callers that don't supply a
// workspace-aware view; it falls through to the same merged
// registry when available.
let skills_block = crate::skills::render_available_skills_context_for_workspace(workspace)
.or_else(|| skills_dir.and_then(crate::skills::render_available_skills_context));
// AI-tool convention show up in the catalogue. When an explicit
// `skills_dir` is configured, union it with the workspace view instead of
// treating it as a fallback; the workspace view often returns Some and
// would otherwise shadow the configured directory entirely.
let skills_block = match skills_dir {
Some(dir) => {
crate::skills::render_available_skills_context_for_workspace_and_dir(workspace, dir)
}
None => crate::skills::render_available_skills_context_for_workspace(workspace),
};
if let Some(block) = skills_block {
full_prompt = format!("{full_prompt}\n\n{block}");
}
@@ -1483,6 +1486,76 @@ mod tests {
);
}
#[test]
fn system_prompt_merges_workspace_and_configured_skills_dir() {
let _env_guard = crate::test_support::lock_test_env();
let tmp = tempdir().expect("tempdir");
let _home = ScopedHome::set(tmp.path().join("home"));
let workspace = tmp.path().join("workspace");
let configured_dir = tmp.path().join("configured-skills");
write_test_skill(
&workspace.join(".claude").join("skills"),
"workspace-skill",
"workspace skill",
);
write_test_skill(&configured_dir, "configured-skill", "configured skill");
let text = match system_prompt_for_mode_with_context_and_skills(
AppMode::Plan,
&workspace,
None,
Some(&configured_dir),
None,
None,
) {
SystemPrompt::Text(text) => text,
SystemPrompt::Blocks(_) => panic!("expected text system prompt"),
};
assert!(text.contains("workspace-skill"));
assert!(text.contains("configured-skill"));
}
struct ScopedHome {
previous: Option<std::ffi::OsString>,
}
impl ScopedHome {
fn set(path: std::path::PathBuf) -> Self {
let previous = std::env::var_os("HOME");
// Safety: this test serializes environment access with
// lock_test_env and restores HOME in Drop.
unsafe {
std::env::set_var("HOME", path);
}
Self { previous }
}
}
impl Drop for ScopedHome {
fn drop(&mut self) {
// Safety: this test serializes environment access with
// lock_test_env and restores HOME in Drop.
unsafe {
if let Some(previous) = self.previous.take() {
std::env::set_var("HOME", previous);
} else {
std::env::remove_var("HOME");
}
}
}
}
fn write_test_skill(root: &std::path::Path, name: &str, description: &str) {
let dir = root.join(name);
std::fs::create_dir_all(&dir).expect("skill dir");
std::fs::write(
dir.join("SKILL.md"),
format!("---\nname: {name}\ndescription: {description}\n---\n\n# {name}\n"),
)
.expect("skill file");
}
#[test]
fn calm_personality_declares_tier_8_subordination() {
assert!(
+108 -11
View File
@@ -567,20 +567,41 @@ pub fn discover_in_workspace(workspace: &Path) -> SkillRegistry {
}
/// Discover skills from the workspace search set plus the configured install
/// directory. Workspace/global directories keep their normal precedence; a
/// custom configured directory is appended when it is outside that set.
/// directory. Workspace-local directories keep their normal precedence; a
/// custom configured directory is inserted before global defaults when it is
/// outside that set so explicit configuration cannot be buried by large global
/// libraries.
#[must_use]
pub fn discover_for_workspace_and_dir(workspace: &Path, skills_dir: &Path) -> SkillRegistry {
let dirs = skills_directories(workspace);
discover_for_workspace_dirs_and_dir(dirs, skills_dir)
let mut dirs = skills_directories(workspace);
insert_configured_skills_dir(&mut dirs, workspace, skills_dir);
discover_from_directories(dirs)
}
fn discover_for_workspace_dirs_and_dir(mut dirs: Vec<PathBuf>, skills_dir: &Path) -> SkillRegistry {
if skills_dir.is_dir() && !dirs.iter().any(|p| p == skills_dir) {
dirs.push(skills_dir.to_path_buf());
fn insert_configured_skills_dir(dirs: &mut Vec<PathBuf>, workspace: &Path, skills_dir: &Path) {
if !skills_dir.is_dir() || dirs.iter().any(|p| paths_refer_to_same_dir(p, skills_dir)) {
return;
}
discover_from_directories(dirs)
let workspace_root = fs::canonicalize(workspace).ok();
let insert_at = workspace_root
.as_ref()
.and_then(|root| {
dirs.iter()
.position(|dir| fs::canonicalize(dir).map_or(true, |dir| !dir.starts_with(root)))
})
.unwrap_or(dirs.len());
dirs.insert(insert_at, skills_dir.to_path_buf());
}
fn paths_refer_to_same_dir(left: &Path, right: &Path) -> bool {
if left == right {
return true;
}
match (fs::canonicalize(left), fs::canonicalize(right)) {
(Ok(left), Ok(right)) => left == right,
_ => false,
}
}
pub(crate) fn discover_from_directories(dirs: impl IntoIterator<Item = PathBuf>) -> SkillRegistry {
@@ -605,8 +626,9 @@ fn discover_for_workspace_and_dir_with_home(
skills_dir: &Path,
home_dir: Option<&Path>,
) -> SkillRegistry {
let dirs = skills_directories_with_home(workspace, home_dir);
discover_for_workspace_dirs_and_dir(dirs, skills_dir)
let mut dirs = skills_directories_with_home(workspace, home_dir);
insert_configured_skills_dir(&mut dirs, workspace, skills_dir);
discover_from_directories(dirs)
}
/// Render the system-prompt skills block from every workspace
@@ -626,12 +648,24 @@ pub fn render_available_skills_context_for_workspace(workspace: &Path) -> Option
/// Single-directory variant — use
/// [`render_available_skills_context_for_workspace`] when scanning
/// a workspace for cross-tool skill folders (#432).
#[cfg(test)]
#[must_use]
pub fn render_available_skills_context(skills_dir: &Path) -> Option<String> {
fn render_available_skills_context(skills_dir: &Path) -> Option<String> {
let registry = SkillRegistry::discover(skills_dir);
render_skills_block(&registry)
}
/// Union variant: merge skills discovered in the `workspace` (cross-tool skill
/// folders) and an explicitly-configured `skills_dir`.
#[must_use]
pub fn render_available_skills_context_for_workspace_and_dir(
workspace: &Path,
skills_dir: &Path,
) -> Option<String> {
let registry = discover_for_workspace_and_dir(workspace, skills_dir);
render_skills_block(&registry)
}
fn render_skills_block(registry: &SkillRegistry) -> Option<String> {
if registry.is_empty() {
return None;
@@ -1197,6 +1231,69 @@ mod tests {
assert!(rendered.contains("from-claude"));
}
#[test]
fn discover_for_workspace_and_dir_merges_workspace_and_configured_sources() {
let tmpdir = TempDir::new().unwrap();
let workspace = tmpdir.path().join("workspace");
let home = tmpdir.path().join("home");
let configured_dir = tmpdir.path().join("configured-skills");
std::fs::create_dir_all(&workspace).unwrap();
write_skill(
&workspace.join(".claude").join("skills"),
"workspace-skill",
"workspace visible skill",
"body",
);
write_skill(
&configured_dir,
"configured-skill",
"configured visible skill",
"body",
);
let registry = super::discover_for_workspace_and_dir_with_home(
&workspace,
&configured_dir,
Some(&home),
);
let names: Vec<&str> = registry.list().iter().map(|s| s.name.as_str()).collect();
assert!(names.contains(&"workspace-skill"));
assert!(names.contains(&"configured-skill"));
}
#[test]
fn explicit_configured_skills_dir_precedes_global_defaults() {
let tmpdir = TempDir::new().unwrap();
let workspace = tmpdir.path().join("workspace");
let home = tmpdir.path().join("home");
let configured_dir = tmpdir.path().join("configured-skills");
std::fs::create_dir_all(&workspace).unwrap();
write_skill(
&home.join(".agents").join("skills"),
"shared-skill",
"global skill",
"global body",
);
write_skill(
&configured_dir,
"shared-skill",
"configured skill",
"configured body",
);
let registry = super::discover_for_workspace_and_dir_with_home(
&workspace,
&configured_dir,
Some(&home),
);
let skill = registry
.get("shared-skill")
.expect("shared skill discovered");
assert_eq!(skill.description, "configured skill");
}
/// Regression for the GitHub issue where users organize skills under
/// vendor / category subdirectories (e.g. cloned skill repos that
/// bundle several skills together). The old single-level `read_dir`
+5 -1
View File
@@ -46,6 +46,7 @@ harvest/stewardship commits:
| Contributor credit plumbing | Added locally after the co-author audit. | Normalized unpushed harvest author/trailer emails to numeric GitHub noreply identities, added `.github/AUTHOR_MAP`, and wired `scripts/check-coauthor-trailers.py` into CI so future `Harvested from PR #N by @handle` commits require machine-readable credit. |
| #2640 workspace field on UpdateThreadRequest | Harvested with the stale-engine fix restored. | Added `workspace` to `PATCH /v1/threads/{id}`, rejects empty paths, rejects workspace changes during active turns, and evicts idle cached engines so the next turn uses the new workspace. `cargo test -p codewhale-tui --bin codewhale-tui --locked update_thread_workspace -- --nocapture` and `cargo clippy -p codewhale-tui --locked -- -D warnings` passed. |
| #2733 PlanArtifact for Plan mode | Locally harvested as a broader continuity-artifact slice. | Added rich `update_plan` fields for objective, context, sources, files, constraints, verification, risks, and handoff notes; renders them in the transcript card and Plan confirmation prompt; preserves them through `/relay`, fork-state, and saved-session replay. `cargo test -p codewhale-tui --bin codewhale-tui --locked plan_ -- --nocapture`, `cargo test -p codewhale-tui --bin codewhale-tui --locked relay_slash_command_routes_to_session_relay_instruction -- --nocapture`, and `cargo clippy -p codewhale-tui --locked -- -D warnings` passed. |
| #2737 configured `skills_dir` discovery | Locally harvested with explicit-config precedence. | The system prompt now unions workspace-discovered skills and configured `skills_dir` skills instead of treating the configured directory as a fallback. Explicit configured skills are inserted before global defaults so they are not lost behind a large global skill library. Credit @h3c-hexin; comment/close the original after the integration branch is public. |
| #2636 project-context mtime cache | Defer direct merge; harvest only after cache key/signature is widened. | Must include constitution changes, auto-generated context deletion, canonical path equivalence, and overwrite detection before landing. |
| #2634 HarmonyOS port | Locally harvested with additional Nix-chain clearance; keep credited and do not close until the integration branch is public. | User-supplied MatePad Edge demo (`https://bilibili.com/video/av116689597368905`) confirms real-device interest. Added env-driven OpenHarmony SDK setup, OHOS platform guards/fallbacks, self-update disablement, and OHOS target gating for Starlark execpolicy parsing plus PTY support so published OHOS builds do not pull `nix` 0.28 through `rustyline` or `portable-pty`. `cargo check --workspace --all-features --locked`, focused PTY/clipboard tests, and `cargo tree --locked -p codewhale-tui --target aarch64-unknown-linux-ohos -i nix@0.28.0` passed; full OHOS target check is blocked on this host because `OHOS_NATIVE_SDK`/target CC/sysroot are not configured and `ring` cannot find `assert.h`. |
| #2687 append-only mode/approval prompt | Defer direct merge; draft has compile failures and Plan-mode prompt correctness risks. | Any future harvest must keep stable `message[0]` genuinely mode-agnostic, preserve mode/approval suffixes after capacity replans, and distinguish external overrides from persisted generated prompts. |
@@ -115,6 +116,9 @@ harvest/stewardship commits:
| #2730 canonical codewhale settings path | Mergeable | Already harvested as `9e15805f6`; follow-up reviewer assertion added locally. Comment/close original after integration branch is public, crediting @xyuai and issue #2664. |
| #2732 pausable command lifecycle | Draft/mergeable | Defer; review flagged behavior changes. |
| #2733 PlanArtifact UI | Mergeable | Locally harvested with richer schema, rendering, relay/fork-state propagation, and replay tests. Comment/close original after integration branch is public, crediting @idling11 and issue #2691; keep #2691 open only if additional PlanReview product work remains. |
| #2736 sub-agent model inheritance | Mergeable | Harvest next with tighter tests: `ToolAgent` should inherit the parent runtime model instead of hard-coding `deepseek-v4-flash`, while preserving the current reasoning-effort behavior unless provider-specific request shaping proves unsafe. Credit @h3c-hexin. |
| #2737 configured `skills_dir` discovery | Mergeable | Locally harvested with extra configured-before-global precedence tests. Comment/close original after the integration branch is public, crediting @h3c-hexin. |
| #2738 dense tool-call transcript collapse | Mergeable | Do not merge as-is. The compaction idea matches the `/relay` direction, but the PR currently bypasses normal rendering, lacks expansion wiring, defaults to expanded mode, and has cache-key/index maintenance risks. Harvest only after completing those behaviors. |
## Issue Reduction Strategy
@@ -135,6 +139,6 @@ Issue count should drop through evidence-backed consolidation, not bulk closing.
## Immediate Next Actions
1. Prepare public comments for #2708, #2502, #2513, #2530, #2576, #2581, #2627,
#2634, #2636, #2687, and already-harvested performance PRs.
#2634, #2636, #2687, #2737, and already-harvested performance PRs.
2. Start file decomposition Phase 1 only after the PR harvest table has no
unknown high-priority provider/prompt/cache branches.