Skip to content

Commit a3eaabc

Browse files
committed
fix(skill): preserve custom installed instructions
1 parent 4549ed0 commit a3eaabc

5 files changed

Lines changed: 229 additions & 3 deletions

File tree

crates/bsk-cli/src/cli/doctor.rs

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -275,6 +275,19 @@ fn check_skill_up_to_date() -> CheckResult {
275275
);
276276
}
277277

278+
if !report.preserved.is_empty() {
279+
let names = report
280+
.preserved
281+
.iter()
282+
.map(|h| h.cli_name())
283+
.collect::<Vec<_>>()
284+
.join(", ");
285+
return CheckResult::na(
286+
name,
287+
format!("custom or edited skill preserved in: {names}"),
288+
);
289+
}
290+
278291
CheckResult::na(name, "no agent skill installed")
279292
}
280293

crates/bsk-cli/src/cli/install_skill.rs

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ use serde::Serialize;
1111
use crate::cli::error::CliError;
1212
use crate::cli::status::Output;
1313
use crate::skill_install::{
14-
InstallOptions, all_harness_reports,
14+
InstallOptions, InstallSourceKind, all_harness_reports,
1515
harness::{HarnessId, parse_harness_id},
1616
load_source, print_harness_table, run_interactive_prompt,
1717
};
@@ -56,11 +56,17 @@ pub fn dispatch(args: InstallSkillArgs, output: Output) -> Result<(), CliError>
5656
}
5757

5858
let harnesses = resolve_targets(&args, &reports).map_err(CliError::Local)?;
59+
let source_kind = if args.source.is_some() {
60+
InstallSourceKind::Custom
61+
} else {
62+
InstallSourceKind::Bundled
63+
};
5964
let source = load_source(args.source.as_deref()).map_err(CliError::Local)?;
6065

6166
let install_output = crate::skill_install::install_to_harnesses(&InstallOptions {
6267
harnesses: &harnesses,
6368
source: &source,
69+
source_kind,
6470
force: args.force,
6571
home: None,
6672
});

crates/bsk-cli/src/daemon/start.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -282,6 +282,12 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> {
282282
if !report.up_to_date.is_empty() {
283283
debug!(count = report.up_to_date.len(), "skill already up to date");
284284
}
285+
if !report.preserved.is_empty() {
286+
debug!(
287+
count = report.preserved.len(),
288+
"custom or edited skill preserved"
289+
);
290+
}
285291
}
286292
Ok(None) => { /* home_dir() already warned inside the closure */ }
287293
Err(join_err) => {

crates/bsk-cli/src/skill_install/mod.rs

Lines changed: 96 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,11 +10,30 @@ use anyhow::{Context, Result, bail};
1010
use console::{Style, style};
1111
use dialoguer::{MultiSelect, theme::ColorfulTheme};
1212
use serde::Serialize;
13+
use sha2::{Digest, Sha256};
1314

1415
pub use harness::{HarnessId, HarnessReport, all_harness_reports, parse_harness_id};
1516

1617
pub const SKILL_DIR_NAME: &str = "browser-skill";
1718
pub const DEFAULT_SKILL_MD: &str = include_str!("../../skill/SKILL.md");
19+
pub const SOURCE_MARKER_FILE: &str = ".bsk-source";
20+
pub const SOURCE_CUSTOM: &str = "custom\n";
21+
22+
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
23+
pub enum InstallSourceKind {
24+
Bundled,
25+
Custom,
26+
}
27+
28+
pub(crate) fn bundled_source_marker(source: &str) -> String {
29+
let digest = Sha256::digest(source.as_bytes());
30+
let mut hex = String::with_capacity(digest.len() * 2);
31+
for byte in digest {
32+
use std::fmt::Write as _;
33+
let _ = write!(hex, "{byte:02x}");
34+
}
35+
format!("bundled:{hex}\n")
36+
}
1837

1938
#[derive(Debug, Clone, Serialize)]
2039
pub struct InstallResult {
@@ -74,6 +93,7 @@ pub struct InstallError {
7493
pub struct InstallOptions<'a> {
7594
pub harnesses: &'a [HarnessId],
7695
pub source: &'a str,
96+
pub source_kind: InstallSourceKind,
7797
pub force: bool,
7898
/// When `Some`, installs under this home instead of the real `$HOME`.
7999
pub home: Option<&'a Path>,
@@ -103,7 +123,7 @@ pub fn install_to_harnesses_at_home(home: &Path, opts: &InstallOptions<'_>) -> I
103123
let mut errors = Vec::new();
104124

105125
for harness in opts.harnesses {
106-
match install_one_at_home(home, *harness, opts.source, opts.force) {
126+
match install_one_at_home(home, *harness, opts.source, opts.source_kind, opts.force) {
107127
Ok((path, status)) => results.push(InstallResult {
108128
harness: harness.cli_name().to_string(),
109129
path,
@@ -123,6 +143,7 @@ fn install_one_at_home(
123143
home: &Path,
124144
harness: HarnessId,
125145
source: &str,
146+
source_kind: InstallSourceKind,
126147
force: bool,
127148
) -> Result<(PathBuf, InstallStatus)> {
128149
let dest_dir = harness.skill_dest_dir_for_home(home);
@@ -134,7 +155,18 @@ fn install_one_at_home(
134155

135156
let existed = dest_file.exists();
136157
fs::create_dir_all(&dest_dir).with_context(|| format!("create {}", dest_dir.display()))?;
158+
let marker = dest_dir.join(SOURCE_MARKER_FILE);
159+
// Mark custom ownership before replacing the skill. If the process
160+
// stops between these writes, automatic sync fails safe and preserves
161+
// the previous file instead of treating custom content as managed.
162+
if source_kind == InstallSourceKind::Custom {
163+
fs::write(&marker, SOURCE_CUSTOM).with_context(|| format!("write {}", marker.display()))?;
164+
}
137165
fs::write(&dest_file, source).with_context(|| format!("write {}", dest_file.display()))?;
166+
if source_kind == InstallSourceKind::Bundled {
167+
fs::write(&marker, bundled_source_marker(source))
168+
.with_context(|| format!("write {}", marker.display()))?;
169+
}
138170

139171
let status = if existed {
140172
InstallStatus::Updated
@@ -310,13 +342,18 @@ mod tests {
310342
&InstallOptions {
311343
harnesses: &[harness],
312344
source: "# test skill\n",
345+
source_kind: InstallSourceKind::Custom,
313346
force: false,
314347
home: Some(&home),
315348
},
316349
);
317350
assert!(out.errors.is_empty());
318351
assert_eq!(out.results.len(), 1);
319352
assert!(skills.join(SKILL_DIR_NAME).join("SKILL.md").is_file());
353+
assert_eq!(
354+
fs::read_to_string(skills.join(SKILL_DIR_NAME).join(SOURCE_MARKER_FILE)).unwrap(),
355+
SOURCE_CUSTOM
356+
);
320357
}
321358

322359
#[test]
@@ -336,6 +373,7 @@ mod tests {
336373
&InstallOptions {
337374
harnesses: &[harness],
338375
source: "new",
376+
source_kind: InstallSourceKind::Custom,
339377
force: false,
340378
home: Some(&home),
341379
},
@@ -361,12 +399,69 @@ mod tests {
361399
&InstallOptions {
362400
harnesses: &[harness],
363401
source: "new",
402+
source_kind: InstallSourceKind::Custom,
364403
force: true,
365404
home: Some(&home),
366405
},
367406
);
368407
assert_eq!(out.results[0].status, InstallStatus::Updated);
369408
assert_eq!(fs::read_to_string(&dest).unwrap(), "new");
409+
assert_eq!(
410+
fs::read_to_string(dest.parent().unwrap().join(SOURCE_MARKER_FILE)).unwrap(),
411+
SOURCE_CUSTOM
412+
);
413+
}
414+
415+
#[test]
416+
fn bundled_install_records_the_written_content_hash() {
417+
let tmp = TempDir::new().unwrap();
418+
let home = tmp.path().to_path_buf();
419+
let harness = HarnessId::Cursor;
420+
421+
let out = install_to_harnesses_at_home(
422+
&home,
423+
&InstallOptions {
424+
harnesses: &[harness],
425+
source: DEFAULT_SKILL_MD,
426+
source_kind: InstallSourceKind::Bundled,
427+
force: false,
428+
home: Some(&home),
429+
},
430+
);
431+
432+
assert!(out.errors.is_empty());
433+
let marker = harness
434+
.skill_dest_dir_for_home(&home)
435+
.join(SOURCE_MARKER_FILE);
436+
assert_eq!(
437+
fs::read_to_string(marker).unwrap(),
438+
bundled_source_marker(DEFAULT_SKILL_MD)
439+
);
440+
}
441+
442+
#[test]
443+
fn custom_install_survives_automatic_bundled_sync() {
444+
let tmp = TempDir::new().unwrap();
445+
let home = tmp.path().to_path_buf();
446+
let harness = HarnessId::Cursor;
447+
let dest = harness.skill_dest_dir_for_home(&home).join("SKILL.md");
448+
449+
let out = install_to_harnesses_at_home(
450+
&home,
451+
&InstallOptions {
452+
harnesses: &[harness],
453+
source: "custom instructions",
454+
source_kind: InstallSourceKind::Custom,
455+
force: false,
456+
home: Some(&home),
457+
},
458+
);
459+
assert!(out.errors.is_empty());
460+
461+
let report = sync::sync_with_source(&home, "new bundled instructions");
462+
463+
assert_eq!(report.preserved, vec![HarnessId::Cursor]);
464+
assert_eq!(fs::read_to_string(dest).unwrap(), "custom instructions");
370465
}
371466

372467
#[test]

0 commit comments

Comments
 (0)