diff --git a/crates/tui/src/prompts.rs b/crates/tui/src/prompts.rs index 00584cd38..6a9023e57 100644 --- a/crates/tui/src/prompts.rs +++ b/crates/tui/src/prompts.rs @@ -1068,13 +1068,15 @@ 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 rather than treating it as an `or_else` fallback: the + // workspace view almost always returns Some (any cross-tool skill folder), + // which would otherwise shadow the configured `skills_dir` entirely. + let skills_block = skills_dir.map_or_else( + || crate::skills::render_available_skills_context_for_workspace(workspace), + |dir| crate::skills::render_available_skills_context_for_workspace_and_dir(workspace, dir), + ); if let Some(block) = skills_block { full_prompt = format!("{full_prompt}\n\n{block}"); } diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index d2c2f6add..d34d05dc3 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -619,19 +619,25 @@ pub fn render_available_skills_context_for_workspace(workspace: &Path) -> Option render_skills_block(®istry) } -/// Codex's progressive-disclosure contract: the model sees skill names, -/// descriptions, and paths up front, then opens the specific `SKILL.md` only -/// when a skill is relevant. -/// -/// Single-directory variant — use -/// [`render_available_skills_context_for_workspace`] when scanning -/// a workspace for cross-tool skill folders (#432). +/// Union variant: merge skills discovered in the `workspace` (cross-tool skill +/// folders) **and** an explicitly-configured `skills_dir`. Use this when an +/// embedder injects a dedicated `skills_dir` that must stay visible even when +/// the workspace already contains skills of its own — a plain `or_else` between +/// the workspace view and the dir view shadows the configured `skills_dir` +/// whenever `discover_in_workspace` returns any skill, so embedder-provided +/// skills silently disappear. #[must_use] -pub fn render_available_skills_context(skills_dir: &Path) -> Option { - let registry = SkillRegistry::discover(skills_dir); +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) } +/// Codex's progressive-disclosure contract: the model sees skill names, +/// descriptions, and paths up front, then opens the specific `SKILL.md` only +/// when a skill is relevant. fn render_skills_block(registry: &SkillRegistry) -> Option { if registry.is_empty() { return None; @@ -760,6 +766,14 @@ mod tests { std::fs::write(skill_dir.join("SKILL.md"), skill_content).unwrap(); } + /// Single-directory render used by the rendering-behaviour tests below. + /// Mirrors the retired `render_available_skills_context` entry point — + /// prompts.rs now goes through the workspace/union variants, which share + /// this same discover + render path. + fn render_dir_context(skills_dir: &std::path::Path) -> Option { + super::render_skills_block(&super::SkillRegistry::discover(skills_dir)) + } + #[test] fn render_available_skills_context_lists_paths_and_usage() { let tmpdir = TempDir::new().unwrap(); @@ -769,9 +783,7 @@ mod tests { "---\nname: test-skill\ndescription: A test skill\n---\nDo something special", ); - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); let expected_path = tmpdir .path() @@ -804,9 +816,7 @@ mod tests { "---\nname: friendly-name\ndescription: drift case\n---\nbody", ); - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); let real_path = tmpdir .path() @@ -838,10 +848,10 @@ mod tests { let tmpdir = TempDir::new().unwrap(); let empty = tmpdir.path().join("skills"); std::fs::create_dir_all(&empty).unwrap(); - assert!(crate::skills::render_available_skills_context(&empty).is_none()); + assert!(render_dir_context(&empty).is_none()); let missing = tmpdir.path().join("does-not-exist"); - assert!(crate::skills::render_available_skills_context(&missing).is_none()); + assert!(render_dir_context(&missing).is_none()); } #[test] @@ -851,9 +861,7 @@ mod tests { let body = format!("---\nname: bigdesc\ndescription: {long_desc}\n---\nbody"); create_skill_dir(&tmpdir, "bigdesc", &body); - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); let max = super::MAX_SKILL_DESCRIPTION_CHARS; assert!(rendered.contains('…'), "expected truncation marker"); @@ -872,9 +880,7 @@ mod tests { "---\nname: spaced-skill\ndescription: alpha \t beta gamma\n---\nbody", ); - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); let line = rendered .lines() @@ -892,9 +898,7 @@ mod tests { create_skill_dir(&tmpdir, &format!("skill-{i:03}"), &body); } - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); assert!( rendered.contains("additional skills omitted from this prompt budget"), @@ -1435,9 +1439,7 @@ mod tests { "folded-skill", "---\nname: folded-skill\ndescription: >\n line one chinese\n line two chinese\n---\nbody", ); - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); assert!( rendered.contains("line one chinese line two chinese"), "folded block scalar should join lines with space, got:\n{rendered}" @@ -1454,9 +1456,7 @@ mod tests { "literal-skill", "---\nname: literal-skill\ndescription: |\n line one\n line two\n---\nbody", ); - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); // `truncate_for_prompt` collapses whitespace, so the newlines // become spaces. The key assertion is that the content is // captured (not just `|`). @@ -1476,9 +1476,7 @@ mod tests { "strip-skill", "---\nname: strip-skill\ndescription: >-\n alpha\n beta\n\n---\nbody", ); - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); assert!( rendered.contains("alpha beta"), "strip-chomped folded block should join lines, got:\n{rendered}" @@ -1495,9 +1493,7 @@ mod tests { "plain-skill", "---\nname: plain-skill\ndescription: A simple description\n---\nbody", ); - let rendered = - crate::skills::render_available_skills_context(&tmpdir.path().join("skills")) - .expect("skill context"); + let rendered = render_dir_context(&tmpdir.path().join("skills")).expect("skill context"); assert!( rendered.contains("- plain-skill: A simple description"), "single-line description should still work, got:\n{rendered}"