diff --git a/crates/tui/src/commands/skills.rs b/crates/tui/src/commands/skills.rs index 3e85b06a..bf024556 100644 --- a/crates/tui/src/commands/skills.rs +++ b/crates/tui/src/commands/skills.rs @@ -13,6 +13,10 @@ use crate::tui::history::HistoryCell; use super::CommandResult; +fn discover_visible_skills(app: &App) -> SkillRegistry { + crate::skills::discover_for_workspace_and_dir(&app.workspace, &app.skills_dir) +} + fn render_skill_warnings(registry: &SkillRegistry) -> String { if registry.warnings().is_empty() { return String::new(); @@ -44,7 +48,7 @@ pub fn list_skills(app: &mut App, arg: Option<&str>) -> CommandResult { } } let skills_dir = app.skills_dir.clone(); - let registry = SkillRegistry::discover(&skills_dir); + let registry = discover_visible_skills(app); let warnings = render_skill_warnings(®istry); if registry.is_empty() { @@ -86,8 +90,7 @@ pub fn list_skills(app: &mut App, arg: Option<&str>) -> CommandResult { /// Try to run a skill by exact name (used for unified slash-command namespace, #435). /// Returns None when no skill with that name exists, so the caller can try other sources. pub fn run_skill_by_name(app: &mut App, name: &str, _arg: Option<&str>) -> Option { - let skills_dir = app.skills_dir.clone(); - let registry = crate::skills::SkillRegistry::discover(&skills_dir); + let registry = discover_visible_skills(app); if registry.get(name).is_some() { Some(activate_skill(app, name)) } else { @@ -125,8 +128,7 @@ fn activate_skill(app: &mut App, name: &str) -> CommandResult { // `/skill new` is a friendly alias for `/skill skill-creator`. let name = if name == "new" { "skill-creator" } else { name }; - let skills_dir = app.skills_dir.clone(); - let registry = SkillRegistry::discover(&skills_dir); + let registry = discover_visible_skills(app); if let Some(skill) = registry.get(name) { let instruction = format!( @@ -508,6 +510,34 @@ mod tests { assert!(msg.contains("/test-skill")); } + #[test] + fn test_list_skills_merges_workspace_and_configured_dirs() { + let tmpdir = TempDir::new().unwrap(); + let workspace_skill_dir = tmpdir + .path() + .join(".agents") + .join("skills") + .join("workspace-skill"); + std::fs::create_dir_all(&workspace_skill_dir).unwrap(); + std::fs::write( + workspace_skill_dir.join("SKILL.md"), + "---\nname: workspace-skill\ndescription: Workspace skill\n---\nDo workspace work", + ) + .unwrap(); + create_skill_dir( + &tmpdir, + "configured-skill", + "---\nname: configured-skill\ndescription: Configured skill\n---\nDo configured work", + ); + + let mut app = create_test_app_with_tmpdir(&tmpdir); + let result = list_skills(&mut app, None); + let msg = result.message.unwrap(); + + assert!(msg.contains("/workspace-skill"), "got: {msg}"); + assert!(msg.contains("/configured-skill"), "got: {msg}"); + } + #[test] fn test_skill_subcommand_dispatch_install_usage() { let tmpdir = TempDir::new().unwrap(); diff --git a/crates/tui/src/prompts.rs b/crates/tui/src/prompts.rs index d4a5c494..bb67c35c 100644 --- a/crates/tui/src/prompts.rs +++ b/crates/tui/src/prompts.rs @@ -618,8 +618,8 @@ mod tests { fn package_version_is_current_hotfix_release() { assert_eq!( env!("CARGO_PKG_VERSION"), - "0.8.16", - "0.8.16 hotfix branch must report the release version before publishing" + "0.8.17", + "0.8.17 hotfix branch must report the release version before publishing" ); } diff --git a/crates/tui/src/skills/mod.rs b/crates/tui/src/skills/mod.rs index ff69fc4b..2750a784 100644 --- a/crates/tui/src/skills/mod.rs +++ b/crates/tui/src/skills/mod.rs @@ -436,6 +436,31 @@ pub fn discover_in_workspace(workspace: &Path) -> SkillRegistry { merged } +/// 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. +#[must_use] +pub fn discover_for_workspace_and_dir(workspace: &Path, skills_dir: &Path) -> SkillRegistry { + let mut dirs = skills_directories(workspace); + if skills_dir.is_dir() && !dirs.iter().any(|p| p == skills_dir) { + dirs.push(skills_dir.to_path_buf()); + } + + let mut merged = SkillRegistry::default(); + for dir in dirs { + let registry = SkillRegistry::discover(&dir); + for skill in registry.skills { + if !merged.skills.iter().any(|s| s.name == skill.name) { + merged.skills.push(skill); + } + } + for warning in registry.warnings { + merged.warnings.push(warning); + } + } + merged +} + /// Render the system-prompt skills block from every workspace /// candidate directory plus the global default (#432). Wraps /// [`discover_in_workspace`] for callers (e.g. `prompts.rs`) that diff --git a/crates/tui/tests/qa_pty.rs b/crates/tui/tests/qa_pty.rs index 348e5944..6654d51e 100644 --- a/crates/tui/tests/qa_pty.rs +++ b/crates/tui/tests/qa_pty.rs @@ -45,6 +45,16 @@ fn boot_minimal() -> anyhow::Result<(qa_harness::harness::SealedWorkspace, Harne Ok((ws, h)) } +fn write_skill(root: std::path::PathBuf, name: &str, description: &str) -> anyhow::Result<()> { + let dir = root.join(name); + std::fs::create_dir_all(&dir)?; + std::fs::write( + dir.join("SKILL.md"), + format!("---\nname: {name}\ndescription: {description}\n---\nUse {name}.\n"), + )?; + Ok(()) +} + /// Smoke: the binary boots into an alt-screen, paints a composer, and the /// header shows the project label. If this fails, the harness itself is /// broken before we worry about any scenario. @@ -81,6 +91,54 @@ fn smoke_keystroke_reaches_composer() -> anyhow::Result<()> { Ok(()) } +/// Regression: `/skills` should reflect the same merged discovery set as the +/// slash menu and model-visible skills block, not just the first selected +/// skills directory. +#[test] +fn skills_menu_shows_local_and_global_skills() -> anyhow::Result<()> { + let ws = make_sealed_workspace()?; + write_skill(ws.user_skills_dir(), "global-alpha", "Global alpha skill")?; + write_skill( + ws.workspace().join(".agents").join("skills"), + "workspace-beta", + "Workspace beta skill", + )?; + + let mut h = Harness::builder(Harness::cargo_bin("deepseek-tui")) + .cwd(ws.workspace()) + .seal_home(ws.home()) + .env("DEEPSEEK_API_KEY", "ci-test-key-not-real") + .env("DEEPSEEK_BASE_URL", "http://127.0.0.1:1") + .env("RUST_LOG", "warn") + .args([ + "--workspace", + ws.workspace().to_str().expect("utf-8 workspace path"), + "--no-project-config", + "--skip-onboarding", + ]) + .size(40, 140) + .spawn()?; + + h.wait_for_text("Composer", BOOT_TIMEOUT)?; + h.send(keys::key::text("/skills"))?; + h.wait_for_idle(Duration::from_millis(300), Duration::from_secs(2))?; + h.send(keys::key::enter())?; + h.wait_for_text("Available skills", KEY_TIMEOUT)?; + h.wait_for_text("global-alpha", KEY_TIMEOUT)?; + h.wait_for_text("workspace-beta", KEY_TIMEOUT)?; + + let f = h.frame(); + let dump = f.debug_dump(); + assert!(f.contains("global-alpha"), "global skill missing:\n{dump}"); + assert!( + f.contains("workspace-beta"), + "workspace skill missing:\n{dump}" + ); + + let _ = h.shutdown(); + Ok(()) +} + // =========================================================================== // #1073 — pasting multi-line text with a trailing newline must NOT auto-submit // =========================================================================== diff --git a/crates/tui/tests/support/qa_harness/harness.rs b/crates/tui/tests/support/qa_harness/harness.rs index 4c4b5676..4344973a 100644 --- a/crates/tui/tests/support/qa_harness/harness.rs +++ b/crates/tui/tests/support/qa_harness/harness.rs @@ -215,13 +215,18 @@ impl Harness { /// Resolve a binary by Cargo bin-name (uses `CARGO_BIN_EXE_`). /// Tests should call this rather than hard-coding paths. pub fn cargo_bin(name: &str) -> PathBuf { - // CARGO_BIN_EXE_ is set by Cargo for binaries declared in the - // same crate as the integration test. For deepseek-tui the binary - // name is `deepseek-tui`. + // Newer Cargo exposes CARGO_BIN_EXE_* at runtime; older supported + // Cargo versions expose it to the integration test at compile time. let key = format!("CARGO_BIN_EXE_{name}"); - std::env::var_os(&key) - .map(PathBuf::from) - .unwrap_or_else(|| panic!("env {key} not set; is the binary declared in this crate?")) + if let Some(path) = std::env::var_os(&key) { + return PathBuf::from(path); + } + if name == "deepseek-tui" + && let Some(path) = option_env!("CARGO_BIN_EXE_deepseek-tui") + { + return PathBuf::from(path); + } + panic!("env {key} not set; is the binary declared in this crate?") } /// Best-effort cooperative shutdown. diff --git a/crates/tui/tests/support/qa_harness/pty.rs b/crates/tui/tests/support/qa_harness/pty.rs index 55c90004..04bd72a1 100644 --- a/crates/tui/tests/support/qa_harness/pty.rs +++ b/crates/tui/tests/support/qa_harness/pty.rs @@ -209,12 +209,17 @@ impl PtySession { } /// Send SIGTERM-equivalent and wait briefly. Returns the exit status if - /// the child reaped within `grace`, or `None` otherwise (in which case - /// `kill_hard` is called as a last resort). + /// the child reaped within `grace`, or `None` otherwise. pub fn shutdown(mut self, grace: Duration) -> Option { + self.kill_and_join_reader(grace) + } + + fn kill_and_join_reader(&mut self, grace: Duration) -> Option { let _ = self.child.kill(); let exit = self.wait_until(Instant::now() + grace); - if let Some(handle) = self.reader_handle.take() { + if exit.is_some() + && let Some(handle) = self.reader_handle.take() + { // Don't block on the reader thread forever — it exits on EOF. let _ = handle.join(); } @@ -224,6 +229,6 @@ impl PtySession { impl Drop for PtySession { fn drop(&mut self) { - let _ = self.child.kill(); + let _ = self.kill_and_join_reader(Duration::from_secs(2)); } }