From 7aa61af96f4ea339d2940fb8b736e55581ca0d18 Mon Sep 17 00:00:00 2001 From: pentaxis93 <13393192+pentaxis93@users.noreply.github.com> Date: Tue, 10 Mar 2026 07:53:24 +0000 Subject: [PATCH] fix(cli): prune stale groundwork skills without lock history Detect groundwork-managed dependencies by repo+path when lock history is missing so stale skills/* entries are pruned during reconcile. Add no-lock regression tests, keep non-skills groundwork paths untouched, and align shipped-manifest order assertion with the current skill list. Refs #113 --- CHANGELOG.md | 1 + crates/groundwork-cli/src/main.rs | 82 ++++++++++++++++++++++++++++--- 2 files changed, 77 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 68d6087..9e462c9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -57,3 +57,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). - Corrected dependency ordering across pipeline documentation - Unified skills table by pipeline stage instead of by source - Hardened CLI tool bootstrap security: `gh-issue-sync` auto-install now uses pinned release assets with SHA-256 verification, and install lock writing now fails if tool version capture is missing/empty instead of silently recording unknown provenance +- Fixed stale groundwork-managed dependency pruning when no install lock exists: `init/update` now removes old `gh = "pentaxis93/groundwork"` entries under `skills/*` even without prior lock history, while preserving non-groundwork dependencies. diff --git a/crates/groundwork-cli/src/main.rs b/crates/groundwork-cli/src/main.rs index 70deb20..b9d210e 100644 --- a/crates/groundwork-cli/src/main.rs +++ b/crates/groundwork-cli/src/main.rs @@ -960,8 +960,10 @@ fn prune_stale_managed_dependencies( ) -> usize { let to_remove: Vec = deps .iter() - .filter_map(|(k, _)| { - if previously_managed.contains(k) && !desired_aliases.contains(k) { + .filter_map(|(k, item)| { + let owns_entry = + previously_managed.contains(k) || is_groundwork_managed_dependency(item); + if owns_entry && !desired_aliases.contains(k) { Some(k.to_string()) } else { None @@ -976,6 +978,32 @@ fn prune_stale_managed_dependencies( to_remove.len() } +fn is_groundwork_managed_dependency(item: &Item) -> bool { + let Some(repo) = dependency_string_field(item, "gh") else { + return false; + }; + let Some(path) = dependency_string_field(item, "path") else { + return false; + }; + + repo == GROUNDWORK_REPO && path.starts_with("skills/") +} + +fn dependency_string_field(item: &Item, key: &str) -> Option { + match item { + Item::Value(Value::InlineTable(inline)) => inline + .get(key) + .and_then(|v| v.as_str()) + .map(|v| v.to_string()), + Item::Table(table) => table + .get(key) + .and_then(Item::as_value) + .and_then(Value::as_str) + .map(|v| v.to_string()), + _ => None, + } +} + fn upsert_dependency(deps: &mut Table, spec: &ManagedDependencySpec) -> bool { let mut inline = InlineTable::new(); inline.insert(spec.source_key.as_str(), Value::from(spec.repo.as_str())); @@ -1948,6 +1976,7 @@ mod tests { "skills/issue-craft", "skills/begin", "skills/test-first", + "skills/systematic-debugging", "skills/third-force", "skills/documentation", "skills/verification-before-completion", @@ -2020,10 +2049,7 @@ claude-code = true .find(|e| e.alias == alias) .expect("lock entry exists"); - assert_eq!( - lock_entry.path.as_deref(), - Some("skills/test-first") - ); + assert_eq!(lock_entry.path.as_deref(), Some("skills/test-first")); } #[test] @@ -2261,6 +2287,50 @@ unrelated = { gh = "org/repo", path = "y" } assert!(deps.contains_key("unrelated")); } + #[test] + fn prune_removes_stale_groundwork_skill_without_lock_history() { + let mut doc = parse_doc( + r#"[agents] +claude-code = true + +[dependencies] +next_issue = { gh = "pentaxis93/groundwork", path = "skills/next-issue" } +external_dep = { gh = "org/repo", path = "y" } +"#, + ); + + let previously_managed = HashSet::new(); + let desired: HashSet = ["begin"].iter().map(|s| s.to_string()).collect(); + + let deps = doc["dependencies"].as_table_mut().expect("deps table"); + let pruned = prune_stale_managed_dependencies(deps, &desired, &previously_managed); + + assert_eq!(pruned, 1); + assert!(!deps.contains_key("next_issue")); + assert!(deps.contains_key("external_dep")); + } + + #[test] + fn prune_does_not_remove_groundwork_entries_outside_skills_path_without_lock() { + let mut doc = parse_doc( + r#"[agents] +claude-code = true + +[dependencies] +groundwork_meta = { gh = "pentaxis93/groundwork", path = "docs/guide" } +"#, + ); + + let previously_managed = HashSet::new(); + let desired: HashSet = HashSet::new(); + + let deps = doc["dependencies"].as_table_mut().expect("deps table"); + let pruned = prune_stale_managed_dependencies(deps, &desired, &previously_managed); + + assert_eq!(pruned, 0); + assert!(deps.contains_key("groundwork_meta")); + } + #[test] fn lock_round_trips_with_issue_sync() { let lock = InstallLock {