diff --git a/CHANGELOG.md b/CHANGELOG.md index 9b552328..cb088d30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/crates/tui/src/prompts.rs b/crates/tui/src/prompts.rs index 00584cd3..f505338f 100644 --- a/crates/tui/src/prompts.rs +++ b/crates/tui/src/prompts.rs @@ -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, + } + + 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!( diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index d2c2f6ad..783d4586 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -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, 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, 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) -> 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 { +fn render_available_skills_context(skills_dir: &Path) -> Option { let registry = SkillRegistry::discover(skills_dir); render_skills_block(®istry) } +/// 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 { + let registry = discover_for_workspace_and_dir(workspace, skills_dir); + render_skills_block(®istry) +} + fn render_skills_block(registry: &SkillRegistry) -> Option { 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` diff --git a/docs/V0_9_0_EXECUTION_MAP.md b/docs/V0_9_0_EXECUTION_MAP.md index 312c6bc6..04f4b96c 100644 --- a/docs/V0_9_0_EXECUTION_MAP.md +++ b/docs/V0_9_0_EXECUTION_MAP.md @@ -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.