Skip to content

Commit 5585391

Browse files
committed
fix(skills): stop repeat auto-downloads (parse SKILL.md, stale tarball guard)
- Fix parse_version_from_skill_md: CRLF/BOM opening lines, optional v prefix, and do not use ? on non-version lines (was returning None on first line). - After auto-update download, if skills still do not match CLI, suppress further auto attempts for this CLI version until skills install or CLI changes. - Clear suppression when synced or when running skills install.
1 parent c90bbf4 commit 5585391

1 file changed

Lines changed: 106 additions & 4 deletions

File tree

src/skill.rs

Lines changed: 106 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -61,10 +61,19 @@ fn detected_agent_skill_paths(skill_name: &str) -> Vec<(String, PathBuf)> {
6161
}
6262

6363
fn parse_version_from_skill_md(content: &str) -> Option<Version> {
64-
let inner = content.strip_prefix("---\n")?.split("\n---").next()?;
65-
for line in inner.lines() {
66-
if let Some(v) = line.strip_prefix("version:") {
67-
return Version::parse(v.trim()).ok();
64+
let content = content.strip_prefix('\u{FEFF}').unwrap_or(content);
65+
let rest = content
66+
.strip_prefix("---\n")
67+
.or_else(|| content.strip_prefix("---\r\n"))?;
68+
let inner = rest.split("\n---").next()?;
69+
for raw_line in inner.lines() {
70+
let line = raw_line.trim();
71+
let Some(ver_raw) = line.strip_prefix("version:") else {
72+
continue;
73+
};
74+
let ver_str = ver_raw.trim().trim_start_matches('v').trim();
75+
if let Ok(v) = Version::parse(ver_str) {
76+
return Some(v);
6877
}
6978
}
7079
None
@@ -81,6 +90,32 @@ fn all_skill_stores_present() -> bool {
8190
.all(|name| skill_store_path(name).exists())
8291
}
8392

93+
fn skill_auto_update_suppress_path() -> PathBuf {
94+
home_dir()
95+
.join(".hotdata")
96+
.join("skills")
97+
.join(".skill_auto_update_suppressed_for_cli")
98+
}
99+
100+
fn skill_auto_update_suppressed_for_this_cli() -> bool {
101+
let Ok(s) = fs::read_to_string(skill_auto_update_suppress_path()) else {
102+
return false;
103+
};
104+
s.trim() == CURRENT_VERSION
105+
}
106+
107+
fn suppress_skill_auto_update_for_this_cli() {
108+
let path = skill_auto_update_suppress_path();
109+
if let Some(parent) = path.parent() {
110+
let _ = fs::create_dir_all(parent);
111+
}
112+
let _ = fs::write(path, format!("{CURRENT_VERSION}\n"));
113+
}
114+
115+
fn clear_skill_auto_update_suppression() {
116+
let _ = fs::remove_file(skill_auto_update_suppress_path());
117+
}
118+
84119
/// If the user has previously installed agent skills (`~/.hotdata/skills/hotdata` exists) but the on-disk
85120
/// bundle is older than this CLI or incomplete, download the matching release tarball and refresh symlinks.
86121
/// Does nothing when skills were never installed or when [`is_managed_by_skills_agent`] is true.
@@ -99,6 +134,11 @@ pub fn maybe_auto_update_after_cli_upgrade() {
99134
_ => true,
100135
};
101136
if !needs_refresh {
137+
clear_skill_auto_update_suppression();
138+
return;
139+
}
140+
141+
if skill_auto_update_suppressed_for_this_cli() {
102142
return;
103143
}
104144

@@ -112,6 +152,25 @@ pub fn maybe_auto_update_after_cli_upgrade() {
112152

113153
let _symlinks = ensure_symlinks();
114154

155+
let still_needed = match read_installed_version() {
156+
Some(v) if v >= current && all_skill_stores_present() => false,
157+
_ => true,
158+
};
159+
160+
if still_needed {
161+
suppress_skill_auto_update_for_this_cli();
162+
eprintln!(
163+
"{}",
164+
format!(
165+
"warning: agent skills still do not match this CLI after download (release tarball may lag the binary). Automatic refresh is suppressed for CLI v{CURRENT_VERSION}; remove {} to retry, or run `hotdata skills install`.",
166+
skill_auto_update_suppress_path().display()
167+
)
168+
.yellow()
169+
);
170+
return;
171+
}
172+
173+
clear_skill_auto_update_suppression();
115174
eprintln!(
116175
"{}",
117176
format!("Agent skills updated to v{current}.").green()
@@ -257,6 +316,7 @@ fn ensure_symlinks() -> Vec<(String, PathBuf, Result<bool, String>)> {
257316
}
258317

259318
pub fn install_project() {
319+
clear_skill_auto_update_suppression();
260320
let current = Version::parse(CURRENT_VERSION).expect("invalid package version");
261321

262322
// Ensure skill files exist locally first
@@ -366,6 +426,7 @@ pub fn install_project() {
366426
}
367427

368428
pub fn install() {
429+
clear_skill_auto_update_suppression();
369430
let current = Version::parse(CURRENT_VERSION).expect("invalid package version");
370431

371432
let needs_download = if is_managed_by_skills_agent() {
@@ -534,3 +595,44 @@ pub fn status() {
534595
println!("\nRun 'hotdata skills install' to update.");
535596
}
536597
}
598+
599+
#[cfg(test)]
600+
mod tests {
601+
use super::*;
602+
603+
#[test]
604+
fn parse_skill_md_accepts_lf_frontmatter() {
605+
let s = "---\nname: hotdata\nversion: 0.1.14\n---\n";
606+
assert_eq!(
607+
parse_version_from_skill_md(s),
608+
Some(Version::parse("0.1.14").unwrap())
609+
);
610+
}
611+
612+
#[test]
613+
fn parse_skill_md_accepts_crlf_opening() {
614+
let s = "---\r\nname: hotdata\r\nversion: 0.1.14\r\n---\r\n";
615+
assert_eq!(
616+
parse_version_from_skill_md(s),
617+
Some(Version::parse("0.1.14").unwrap())
618+
);
619+
}
620+
621+
#[test]
622+
fn parse_skill_md_accepts_bom() {
623+
let s = "\u{FEFF}---\nname: hotdata\nversion: 0.1.14\n---\n";
624+
assert_eq!(
625+
parse_version_from_skill_md(s),
626+
Some(Version::parse("0.1.14").unwrap())
627+
);
628+
}
629+
630+
#[test]
631+
fn parse_skill_md_accepts_v_prefix() {
632+
let s = "---\nname: hotdata\nversion: v0.1.14\n---\n";
633+
assert_eq!(
634+
parse_version_from_skill_md(s),
635+
Some(Version::parse("0.1.14").unwrap())
636+
);
637+
}
638+
}

0 commit comments

Comments
 (0)