Skip to content

Commit 975b5aa

Browse files
committed
fix(skills): run retired-skill cleanup on the out-of-band upgrade path
Address review on #244. - `maybe_auto_update_after_cli_upgrade()` (the auto-refresh path hit by Homebrew/curl/package-manager upgraders) refreshed symlinks but never called `remove_retired_skills_global()`, and its version gate means it never runs again — so stale top-level `hotdata-search`/`-analytics`/`-geospatial` entries would persist permanently. Call cleanup right after `ensure_symlinks()`. - Add a unit test for `remove_retired_skills_project` (real dir + symlink → both removed, `hotdata` survives). - Give sub-skill cross-references relative paths (e.g. `../search/SKILL.md`) so a sub-skill loaded on demand can reach its siblings without the parent's name→path mapping in context.
1 parent 8125cd3 commit 975b5aa

5 files changed

Lines changed: 45 additions & 6 deletions

File tree

‎skills/hotdata/subskills/analytics/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ version: 0.21.0
1010

1111
**Prerequisites:** Authenticate, workspace, and catalog discovery via the **`hotdata`** skill (`ingest` datasources, `tables`, `databases`).
1212

13-
**Related skills:** **`hotdata-search`** (BM25, vector, retrieval indexes), **`hotdata-geospatial`** (spatial SQL).
13+
**Related sub-skills** (bundled alongside this one — `Read` on demand): **`hotdata-search`** ([`../search/SKILL.md`](../search/SKILL.md) — BM25, vector, retrieval indexes), **`hotdata-geospatial`** ([`../geospatial/SKILL.md`](../geospatial/SKILL.md) — spatial SQL).
1414

1515
---
1616

‎skills/hotdata/subskills/analytics/references/WORKFLOWS.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
OLAP-style SQL, **History** (query runs and stored results), and **Chain** (materialized follow-ups). Requires **`hotdata`** for auth, workspaces, and catalog commands.
44

5-
**Related:** **`hotdata-search`** for BM25/vector indexes and `hotdata search`; **`hotdata`** [WORKFLOWS.md](../../../references/WORKFLOWS.md) for managed databases.
5+
**Related:** **`hotdata-search`** ([`../../search/SKILL.md`](../../search/SKILL.md)) for BM25/vector indexes and `hotdata search`; **`hotdata`** [WORKFLOWS.md](../../../references/WORKFLOWS.md) for managed databases.
66

77
---
88

‎skills/hotdata/subskills/geospatial/SKILL.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ version: 0.21.0
88

99
Hotdata supports a subset of PostGIS-style functions in **PostgreSQL-dialect SQL**. This skill is data-agnostic — apply it to any table with geometry columns.
1010

11-
**Requires the core `hotdata` skill** for auth, workspace, and table discovery. **Related:** **`hotdata-analytics`** (OLAP SQL), **`hotdata-search`** (BM25/vector).
11+
**Requires the core `hotdata` skill** for auth, workspace, and table discovery. **Related sub-skills** (bundled alongside this one — `Read` on demand): **`hotdata-analytics`** ([`../analytics/SKILL.md`](../analytics/SKILL.md) — OLAP SQL), **`hotdata-search`** ([`../search/SKILL.md`](../search/SKILL.md) — BM25/vector).
1212

1313
## Running these queries
1414

‎skills/hotdata/subskills/search/SKILL.md‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ Retrieval workloads in Hotdata: **BM25 full-text**, **vector similarity**, and t
1010

1111
**Prerequisites:** Authenticate, set a workspace, and set an active database (`hotdata databases set <id>`) — see the **`hotdata`** skill. Use fully qualified table names: `<catalog>.<schema>.<table>`.
1212

13-
**Related skills:** **`hotdata-analytics`** (OLAP SQL, query history, materialized chains), **`hotdata-geospatial`** (PostGIS-style functions).
13+
**Related sub-skills** (bundled alongside this one — `Read` on demand): **`hotdata-analytics`** ([`../analytics/SKILL.md`](../analytics/SKILL.md) — OLAP SQL, query history, materialized chains), **`hotdata-geospatial`** ([`../geospatial/SKILL.md`](../geospatial/SKILL.md) — PostGIS-style functions).
1414

1515
---
1616

@@ -62,8 +62,8 @@ hotdata indexes create --catalog <alias> --schema <schema> --table <table> \
6262
hotdata indexes delete --catalog <alias> --schema <schema> --table <table> --name <name>
6363
```
6464

65-
- **`--type` is required** on create: `bm25` (one or more text columns, comma-separated in `--column`) or `vector` (exactly one column; often embeddings or auto-embedded text). (`sorted` is also a valid `--type`, covered in **`hotdata-analytics`**.)
66-
- **`sorted`** indexes (range/equality for OLAP filters) are documented in **`hotdata-analytics`** — this skill focuses on retrieval types.
65+
- **`--type` is required** on create: `bm25` (one or more text columns, comma-separated in `--column`) or `vector` (exactly one column; often embeddings or auto-embedded text). (`sorted` is also a valid `--type`, covered in **`hotdata-analytics`** — [`../analytics/SKILL.md`](../analytics/SKILL.md).)
66+
- **`sorted`** indexes (range/equality for OLAP filters) are documented in **`hotdata-analytics`** ([`../analytics/SKILL.md`](../analytics/SKILL.md)) — this skill focuses on retrieval types.
6767
- **`--async`:** poll with `hotdata jobs <job_id>` (see **`hotdata`** skill **Jobs**).
6868
- **Auto-embedding:** `--type vector` on a **text** column generates embeddings server-side. Optional `--embedding-provider-id`; default output column `{column}_embedding` (override with `--output-column`).
6969

‎src/commands/skill.rs‎

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -168,6 +168,7 @@ pub fn maybe_auto_update_after_cli_upgrade() {
168168
}
169169

170170
let _symlinks = ensure_symlinks();
171+
remove_retired_skills_global();
171172

172173
let still_needed = match read_installed_version() {
173174
Some(v) if v >= current && all_skill_stores_present() => false,
@@ -719,4 +720,42 @@ mod tests {
719720
Some(Version::parse("0.1.14").unwrap())
720721
);
721722
}
723+
724+
#[test]
725+
fn remove_retired_skills_project_clears_stale_top_level_dirs() {
726+
let tmp = tempfile::tempdir().unwrap();
727+
let cwd = tmp.path();
728+
let project_skills_root = cwd.join(".agents").join("skills");
729+
730+
// A retired skill present as a real dir in the `.agents` layer and as a
731+
// symlink in the `.claude` root (exercises both branches of
732+
// `remove_skill_path`), plus the surviving `hotdata` skill.
733+
let agents_retired = project_skills_root.join("hotdata-search");
734+
fs::create_dir_all(&agents_retired).unwrap();
735+
let agents_hotdata = project_skills_root.join("hotdata");
736+
fs::create_dir_all(&agents_hotdata).unwrap();
737+
738+
let claude_skills = cwd.join(".claude").join("skills");
739+
fs::create_dir_all(&claude_skills).unwrap();
740+
let claude_retired = claude_skills.join("hotdata-search");
741+
#[cfg(unix)]
742+
std::os::unix::fs::symlink(&agents_retired, &claude_retired).unwrap();
743+
#[cfg(not(unix))]
744+
fs::create_dir_all(&claude_retired).unwrap();
745+
746+
remove_retired_skills_project(cwd, &project_skills_root);
747+
748+
assert!(
749+
!agents_retired.exists(),
750+
"retired skill in .agents should be removed"
751+
);
752+
assert!(
753+
claude_retired.symlink_metadata().is_err(),
754+
"retired skill symlink in .claude should be removed"
755+
);
756+
assert!(
757+
agents_hotdata.exists(),
758+
"the surviving hotdata skill must be left in place"
759+
);
760+
}
722761
}

0 commit comments

Comments
 (0)