diff --git a/README.md b/README.md
index 919a220a..2704accc 100644
--- a/README.md
+++ b/README.md
@@ -129,6 +129,28 @@ Use Space to select the Agent harness you want to install into, then
press Enter to install the skill. Run `bsk install-skill --list` to see
internal variants and install paths.
+To install your own instructions, use `bsk install-skill --harness cursor --source ./SKILL.md`.
+An explicit `--source` stays custom even if its contents match the bundled skill.
+Existing installations are skipped unless you add `--force`.
+
+Daemon startup, `session start`, and `doctor` automatically update managed skills
+only when their contents still match the last installed version. Local edits are
+preserved and automatic updates pause. An older installation without a content
+baseline is enrolled automatically only if it exactly matches the current bundled
+skill; this writes the source marker without rewriting `SKILL.md`. Explicit custom
+installations stay custom even when their contents match.
+
+For differing historical files, local edits, or an unrecognized source marker,
+`doctor` shows `WARN` with the reason and recovery options. These warnings do not
+make the health check fail (`--json` reports `status: "warn"` and `ok: true`).
+A concurrent install or sync is reported as deferred and retried on a later pass.
+
+To keep your current instructions as an explicit customization, run
+`bsk install-skill --harness cursor --source --force`, replacing
+`` with the path to your existing file. To restore the bundled
+skill and resume automatic updates, run `bsk install-skill --harness cursor --force`
+without `--source`. This second command overwrites the existing instructions.
+
Other shell-capable agent harnesses are supported too. Copy
[`skill/SKILL.md`](skill/SKILL.md) into your harness's skills directory as
`browser-skill/SKILL.md` to install the skill manually. DeepSeek Harness uses a
diff --git a/README.zh-CN.md b/README.zh-CN.md
index 2d2bd3c7..fd2664ef 100644
--- a/README.zh-CN.md
+++ b/README.zh-CN.md
@@ -113,6 +113,24 @@ bsk install-skill
用 Space 选择需要安装的 Agent harness，然后按 Enter 安装 skill。运行 `bsk install-skill --list` 可查看 internal 变体及安装路径。
+安装自定义指令可运行 `bsk install-skill --harness cursor --source ./SKILL.md`。
+显式指定 `--source` 的安装始终视为自定义，即使内容与内置 skill 相同。
+已有安装默认跳过，添加 `--force` 才会覆盖。
+
+daemon 启动、`session start` 和 `doctor` 会检查已安装的 skill：只有文件内容仍与
+上次安装或同步时的内容一致，才继续自动更新。检测到本地编辑时会保留文件并暂停更新。
+没有内容基线的历史安装，只有与当前内置 skill 字节级一致时才自动纳入管理；此时只补齐
+来源标记，不重写 `SKILL.md`。明确的自定义安装即使内容相同，也不会被自动纳入管理。
+
+对于内容不同的历史文件、本地编辑或无法识别的来源标记，`doctor` 会显示 `WARN`，
+说明暂停原因及恢复方法。这类警告不会让健康检查失败（`--json` 中为 `status: "warn"`、
+`ok: true`）。其他安装或同步正在进行时，本次同步会推迟到后续再试。
+
+如需将当前指令保留为明确的自定义安装，运行
+`bsk install-skill --harness cursor --source --force`，将
+`` 替换为现有文件路径。如需恢复内置 skill 并重新启用自动更新，运行
+`bsk install-skill --harness cursor --force`，不带 `--source`。后一条命令会覆盖现有指令。
+
其他支持 Shell 的 Agent harness 也可使用 BrowserSkill，但需手动将 [`skill/SKILL.md`](skill/SKILL.md) 复制到对应 skills 目录下的 `browser-skill/SKILL.md`。DeepSeek Harness 走独立插件，见 [DeepSeek Harness 插件](#deepseek-harness-插件)。
diff --git a/crates/bsk-cli/Cargo.toml b/crates/bsk-cli/Cargo.toml
index e24ef887..291901ae 100644
--- a/crates/bsk-cli/Cargo.toml
+++ b/crates/bsk-cli/Cargo.toml
@@ -45,6 +45,7 @@ console = { workspace = true }
dialoguer = { workspace = true }
dirs = { workspace = true }
fs2 = { workspace = true }
+tempfile = { workspace = true }
rand = { workspace = true }
uuid = { workspace = true }
bsk-protocol = { workspace = true }
@@ -68,7 +69,6 @@ windows-sys = { version = "0.59", features = [
] }
[dev-dependencies]
-tempfile = { workspace = true }
serde_json = { workspace = true }
tokio = { workspace = true }
tokio-tungstenite = { workspace = true }
diff --git a/crates/bsk-cli/src/cli/doctor.rs b/crates/bsk-cli/src/cli/doctor.rs
index 23b679e3..fcd0b1e6 100644
--- a/crates/bsk-cli/src/cli/doctor.rs
+++ b/crates/bsk-cli/src/cli/doctor.rs
@@ -24,17 +24,15 @@ const EXTENSION_STORE_URL_EDGE: &str = "https://microsoftedge.microsoft.com/addo
/// Store listings highlighted in repair hints, in the order they appear.
const EXTENSION_STORE_URLS: [&str; 2] = [EXTENSION_STORE_URL, EXTENSION_STORE_URL_EDGE];
-/// Status of a single doctor check. `Ok` / `Fail` are the legacy two
-/// states; `NotApplicable` (review M2) is reported as "N/A" in human
-/// output and as `"status": "na"` in `--json` output, so a check that
-/// has nothing to compare against (e.g. browsers protocol-compat with
-/// zero connected browsers) does not falsely report green.
+/// A warning needs attention but does not fail the overall health check.
#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize)]
#[serde(rename_all = "snake_case")]
pub enum CheckStatus {
#[default]
Ok,
Fail,
+ #[serde(rename = "warn")]
+ Warning,
/// The check could not run because its precondition is absent.
/// Treated as informational and never flips an exit code.
#[serde(rename = "na")]
@@ -44,21 +42,14 @@ pub enum CheckStatus {
#[derive(Debug, Clone, PartialEq, Eq, Serialize)]
pub struct CheckResult {
pub name: String,
- /// Pre-M2 boolean status, retained for backwards-compatible JSON
- /// consumers. `true` means the check passed or was not applicable
- /// (i.e. nothing flips the overall doctor verdict red); `false`
- /// means the check actively failed. New consumers should read the
- /// `status` field below for the tri-state (`ok` / `fail` / `na`)
- /// distinction — the legacy boolean intentionally collapses
- /// `ok` and `na` so a doctor run that includes an N/A check does
- /// not regress for callers that still consult `ok` only.
+ /// Backwards-compatible verdict: only a failure is false. Read `status`
+ /// to distinguish a warning or a check that is not applicable.
pub ok: bool,
- /// Tri-state status (review M2): `ok` / `fail` / `na`.
+ /// `ok`, `fail`, `warn`, or `na`.
#[serde(default)]
pub status: CheckStatus,
pub detail: String,
- /// User-facing repair hint (only meaningful when `status` is
- /// `Fail`).
+ /// Actionable guidance for a failure or warning.
pub hint: Option,
}
@@ -83,6 +74,16 @@ impl CheckResult {
}
}
+ fn warn(name: impl Into, detail: impl Into, hint: impl Into) -> Self {
+ Self {
+ name: name.into(),
+ ok: true,
+ status: CheckStatus::Warning,
+ detail: detail.into(),
+ hint: Some(hint.into()),
+ }
+ }
+
fn na(name: impl Into, detail: impl Into) -> Self {
Self {
name: name.into(),
@@ -111,7 +112,7 @@ pub fn run(output: Output) -> Result> {
}
/// Whether the rendered doctor report contains an active failure.
-/// `NotApplicable` remains informational and must not change the exit code.
+/// Warnings and `NotApplicable` remain informational and do not change the exit code.
pub fn has_failures(checks: &[CheckResult]) -> bool {
checks.iter().any(|check| check.status == CheckStatus::Fail)
}
@@ -247,41 +248,61 @@ fn check_skill_up_to_date() -> CheckResult {
};
let report = crate::skill_install::sync::sync_installed_skills(&home);
- if !report.errors.is_empty() {
- let detail = report
- .errors
- .iter()
- .map(|(h, msg)| format!("{}: {msg}", h.cli_name()))
- .collect::>()
- .join("; ");
- return CheckResult::fail(
- name,
- detail,
- "re-run `bsk install-skill --force --harness ` for the failing harness",
- );
- }
+ skill_check_from_report(&report)
+}
- if !report.updated.is_empty() {
- let names = report
- .updated
- .iter()
- .map(|h| h.cli_name())
- .collect::>()
- .join(", ");
- return CheckResult::ok(
- name,
- format!("synced {} harness(es): {names}", report.updated.len()),
- );
+fn skill_check_from_report(report: &crate::skill_install::sync::SyncReport) -> CheckResult {
+ let name = "agent skill up to date";
+ let mut details = Vec::new();
+ for (label, harnesses) in [
+ ("synced", &report.updated),
+ ("up to date", &report.up_to_date),
+ (
+ "custom skill preserved (automatic updates disabled) in",
+ &report.protected,
+ ),
+ ("busy, sync deferred in", &report.busy),
+ ] {
+ if !harnesses.is_empty() {
+ let names = harnesses
+ .iter()
+ .map(|h| h.cli_name())
+ .collect::>()
+ .join(", ");
+ details.push(format!("{label}: {names}"));
+ }
+ }
+ let mut hints = Vec::new();
+ for (harness, reason) in &report.paused {
+ let id = harness.cli_name();
+ details.push(format!(
+ "automatic updates paused for {id}: {}; content preserved",
+ reason.description()
+ ));
+ hints.push(format!(
+ "{id}: keep your instructions with `bsk install-skill --harness {id} --source --force`, or restore the bundled skill with `bsk install-skill --harness {id} --force` (overwrites existing instructions)"
+ ));
}
+ for (harness, message) in &report.errors {
+ details.push(format!("sync failed for {}: {message}", harness.cli_name()));
+ }
+ let detail = details.join("; ");
- if !report.up_to_date.is_empty() {
- return CheckResult::ok(
- name,
- format!("up to date in {} harness(es)", report.up_to_date.len()),
+ if !report.errors.is_empty() {
+ hints.insert(
+ 0,
+ "check filesystem access for the failing harness, then re-run `bsk doctor`".into(),
);
+ CheckResult::fail(name, detail, hints.join("; "))
+ } else if !report.paused.is_empty() {
+ CheckResult::warn(name, detail, hints.join("; "))
+ } else if !report.updated.is_empty() || !report.up_to_date.is_empty() {
+ CheckResult::ok(name, detail)
+ } else if !details.is_empty() {
+ CheckResult::na(name, detail)
+ } else {
+ CheckResult::na(name, "no agent skill installed")
}
-
- CheckResult::na(name, "no agent skill installed")
}
fn check_daemon_running(state: &DaemonState) -> CheckResult {
@@ -456,10 +477,13 @@ fn render_human(checks: &[CheckResult]) {
let mark = match c.status {
CheckStatus::Ok => "ok ",
CheckStatus::Fail => "FAIL",
+ CheckStatus::Warning => "WARN",
CheckStatus::NotApplicable => "N/A ",
};
let detail = match (&c.hint, c.status) {
- (Some(h), CheckStatus::Fail) => format!("{} — hint: {}", c.detail, style_hint(h)),
+ (Some(h), CheckStatus::Fail | CheckStatus::Warning) => {
+ format!("{} — hint: {}", c.detail, style_hint(h))
+ }
_ => c.detail.clone(),
};
let name = &c.name;
@@ -496,6 +520,138 @@ mod m2_tests {
}
}
+ #[test]
+ fn skill_check_reports_protected_harnesses_with_managed_results() {
+ use crate::skill_install::{HarnessId, sync::SyncReport};
+ for updated in [true, false] {
+ let mut report = SyncReport {
+ protected: vec![HarnessId::Cursor],
+ ..Default::default()
+ };
+ if updated {
+ report.updated.push(HarnessId::ClaudeCode);
+ } else {
+ report.up_to_date.push(HarnessId::ClaudeCode);
+ }
+ let check = skill_check_from_report(&report);
+ assert_eq!(check.status, CheckStatus::Ok);
+ assert!(check.detail.contains("claude-code"));
+ assert!(
+ check
+ .detail
+ .contains("preserved (automatic updates disabled) in: cursor")
+ );
+ let json = serde_json::to_value(&check).unwrap();
+ assert_eq!(json["status"], "ok");
+ assert!(
+ json["detail"]
+ .as_str()
+ .unwrap()
+ .contains("preserved (automatic updates disabled) in: cursor")
+ );
+ }
+ }
+
+ #[test]
+ fn skill_check_keeps_all_outcomes_when_another_harness_fails() {
+ use crate::skill_install::{HarnessId, sync::SyncReport};
+ let check = skill_check_from_report(&SyncReport {
+ updated: vec![HarnessId::ClaudeCode],
+ up_to_date: vec![HarnessId::PiAgent],
+ protected: vec![HarnessId::Cursor],
+ busy: vec![HarnessId::Hermes],
+ errors: vec![(HarnessId::Workbuddy, "permission denied".into())],
+ paused: Vec::new(),
+ });
+ assert_eq!(check.status, CheckStatus::Fail);
+ for text in [
+ "synced: claude-code",
+ "up to date: pi",
+ "preserved (automatic updates disabled) in: cursor",
+ "sync deferred in: hermes",
+ "workbuddy: permission denied",
+ ] {
+ assert!(check.detail.contains(text), "{}", check.detail);
+ }
+ assert!(!check.hint.unwrap().contains("--force"));
+ }
+
+ #[test]
+ fn paused_skills_warn_with_actions_even_alongside_successful_updates() {
+ use crate::skill_install::{
+ HarnessId,
+ sync::{PauseReason, SyncReport},
+ };
+ for reason in [
+ PauseReason::Untracked,
+ PauseReason::MissingBaseline,
+ PauseReason::LocalChanges,
+ PauseReason::InvalidMarker,
+ ] {
+ for updated in [false, true] {
+ let mut report = SyncReport {
+ paused: vec![(HarnessId::Cursor, reason)],
+ ..Default::default()
+ };
+ if updated {
+ report.updated.push(HarnessId::ClaudeCode);
+ }
+ let check = skill_check_from_report(&report);
+ assert_eq!(check.status, CheckStatus::Warning);
+ assert!(check.detail.contains("automatic updates paused for cursor"));
+ assert!(check.detail.contains(reason.description()));
+ if updated {
+ assert!(check.detail.contains("synced: claude-code"));
+ }
+ assert!(!has_failures(std::slice::from_ref(&check)));
+ let json = serde_json::to_value(&check).unwrap();
+ assert_eq!(json["status"], "warn");
+ assert_eq!(json["ok"], true);
+ let hint = json["hint"].as_str().unwrap();
+ assert!(hint.contains("--harness cursor --source --force"));
+ assert!(hint.contains("--harness cursor --force"));
+ assert!(hint.contains("overwrites existing instructions"));
+ // An I/O failure takes precedence without hiding paused installations.
+ report
+ .errors
+ .push((HarnessId::PiAgent, "permission denied".into()));
+ let failed = skill_check_from_report(&report);
+ assert_eq!(failed.status, CheckStatus::Fail);
+ assert!(
+ failed
+ .detail
+ .contains("automatic updates paused for cursor")
+ );
+ assert!(failed.hint.as_ref().unwrap().contains("--harness cursor"));
+ assert!(has_failures(&[failed]));
+ }
+ }
+ }
+
+ #[test]
+ fn protected_or_busy_skills_are_informational() {
+ use crate::skill_install::{HarnessId, sync::SyncReport};
+ for report in [
+ SyncReport {
+ protected: vec![HarnessId::Cursor],
+ ..Default::default()
+ },
+ SyncReport {
+ busy: vec![HarnessId::Cursor],
+ ..Default::default()
+ },
+ ] {
+ let check = skill_check_from_report(&report);
+ assert_eq!(check.status, CheckStatus::NotApplicable);
+ assert!(check.detail.contains("cursor"));
+ assert!(!has_failures(std::slice::from_ref(&check)));
+ assert_eq!(serde_json::to_value(&check).unwrap()["status"], "na");
+ }
+ let empty = skill_check_from_report(&SyncReport::default());
+ assert_eq!(empty.status, CheckStatus::NotApplicable);
+ assert_eq!(empty.detail, "no agent skill installed");
+ }
+
#[test]
fn extension_check_includes_store_url_when_no_browser_connected() {
let status = fake_status(Vec::new(), Vec::new());
diff --git a/crates/bsk-cli/src/cli/install_skill.rs b/crates/bsk-cli/src/cli/install_skill.rs
index 96675a42..9c520fab 100644
--- a/crates/bsk-cli/src/cli/install_skill.rs
+++ b/crates/bsk-cli/src/cli/install_skill.rs
@@ -11,7 +11,7 @@ use serde::Serialize;
use crate::cli::error::CliError;
use crate::cli::status::Output;
use crate::skill_install::{
- InstallOptions, all_harness_reports,
+ InstallOptions, SkillSource, all_harness_reports,
harness::{HarnessId, parse_harness_id},
load_source, print_harness_table, run_interactive_prompt,
};
@@ -43,6 +43,16 @@ pub struct InstallSkillArgs {
pub force: bool,
}
+impl InstallSkillArgs {
+ fn source_kind(&self) -> SkillSource {
+ if self.source.is_some() {
+ SkillSource::Custom
+ } else {
+ SkillSource::Bundled
+ }
+ }
+}
+
#[derive(Debug, Serialize)]
struct ListOutput {
harnesses: Vec,
@@ -61,6 +71,7 @@ pub fn dispatch(args: InstallSkillArgs, output: Output) -> Result<(), CliError>
let install_output = crate::skill_install::install_to_harnesses(&InstallOptions {
harnesses: &harnesses,
source: &source,
+ source_kind: args.source_kind(),
force: args.force,
home: None,
});
@@ -202,6 +213,44 @@ mod tests {
}
}
+ #[test]
+ fn identical_explicit_source_remains_custom_after_install_and_upgrade() {
+ use crate::skill_install::{DEFAULT_SKILL_MD, SOURCE_CUSTOM, SOURCE_MARKER_FILE, sync};
+ let home = tempfile::tempdir().unwrap();
+ let path = home.path().join("my-skill.md");
+ std::fs::write(&path, DEFAULT_SKILL_MD).unwrap();
+ let mut args = args(false, false);
+ assert_eq!(args.source_kind(), SkillSource::Bundled);
+ args.source = Some(path);
+ let source = load_source(args.source.as_deref()).unwrap();
+ let result = crate::skill_install::install_to_harnesses(&InstallOptions {
+ harnesses: &[HarnessId::Cursor],
+ source: &source,
+ source_kind: args.source_kind(),
+ force: false,
+ home: Some(home.path()),
+ });
+ assert!(result.success());
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ assert_eq!(
+ std::fs::read_to_string(dir.join(SOURCE_MARKER_FILE)).unwrap(),
+ SOURCE_CUSTOM
+ );
+ // Matching bytes do not turn a custom installation into an up-to-date managed one.
+ assert_eq!(
+ sync::sync_installed_skills(home.path()).protected,
+ vec![HarnessId::Cursor]
+ );
+ assert_eq!(
+ sync::sync_with_source(home.path(), "next bundled version").protected,
+ vec![HarnessId::Cursor]
+ );
+ assert_eq!(
+ std::fs::read_to_string(dir.join("SKILL.md")).unwrap(),
+ DEFAULT_SKILL_MD
+ );
+ }
+
#[test]
fn all_targets_detected_harnesses_only() {
let reports = vec![
diff --git a/crates/bsk-cli/src/cli/session.rs b/crates/bsk-cli/src/cli/session.rs
index 5365282c..6e552633 100644
--- a/crates/bsk-cli/src/cli/session.rs
+++ b/crates/bsk-cli/src/cli/session.rs
@@ -542,6 +542,13 @@ fn run_skill_sync_for_session_start(format: Format) {
for harness in &report.updated {
eprintln!("≈ skill updated for {}", harness.cli_name());
}
+ for (harness, reason) in &report.paused {
+ eprintln!(
+ "! skill auto-update paused for {}: {}; run `bsk doctor` for options",
+ harness.cli_name(),
+ reason.description()
+ );
+ }
}
for (harness, msg) in &report.errors {
tracing::warn!(harness = harness.cli_name(), error = %msg, "skill sync failed");
diff --git a/crates/bsk-cli/src/daemon/start.rs b/crates/bsk-cli/src/daemon/start.rs
index 017c55d0..4d584575 100644
--- a/crates/bsk-cli/src/daemon/start.rs
+++ b/crates/bsk-cli/src/daemon/start.rs
@@ -279,6 +279,10 @@ pub fn run_foreground(cfg: DaemonConfig) -> Result<()> {
for harness in &report.updated {
info!(harness = harness.cli_name(), "skill synced");
}
+ for (harness, reason) in &report.paused {
+ warn!(harness = harness.cli_name(), reason = reason.description(),
+ "skill auto-update paused; content preserved; run `bsk doctor` for options");
+ }
for (harness, msg) in &report.errors {
warn!(harness = harness.cli_name(), error = %msg, "skill sync failed");
}
diff --git a/crates/bsk-cli/src/skill_install/mod.rs b/crates/bsk-cli/src/skill_install/mod.rs
index b52432e3..456fdc9f 100644
--- a/crates/bsk-cli/src/skill_install/mod.rs
+++ b/crates/bsk-cli/src/skill_install/mod.rs
@@ -1,6 +1,8 @@
-//! Install the bundled browser-skill `SKILL.md` into agent harness skill directories.
+//! Install bundled or custom browser-skill instructions into agent skill directories.
pub mod harness;
+mod provenance;
+mod storage;
pub mod sync;
use std::fs;
@@ -9,12 +11,23 @@ use std::path::{Path, PathBuf};
use anyhow::{Context, Result, bail};
use console::{Style, style};
use dialoguer::{MultiSelect, theme::ColorfulTheme};
-use serde::Serialize;
+use serde::{Deserialize, Serialize};
pub use harness::{HarnessId, HarnessReport, all_harness_reports, parse_harness_id};
pub const SKILL_DIR_NAME: &str = "browser-skill";
pub const DEFAULT_SKILL_MD: &str = include_str!("../../skill/SKILL.md");
+pub const SOURCE_MARKER_FILE: &str = ".bsk-source";
+pub const SOURCE_BUNDLED: &str = "bundled\n";
+pub const SOURCE_CUSTOM: &str = "custom\n";
+
+/// Installation provenance is explicit: even an identical `--source` is custom.
+#[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)]
+#[serde(rename_all = "snake_case")]
+pub enum SkillSource {
+ Bundled,
+ Custom,
+}
#[derive(Debug, Clone, Serialize)]
pub struct InstallResult {
@@ -74,6 +87,7 @@ pub struct InstallError {
pub struct InstallOptions<'a> {
pub harnesses: &'a [HarnessId],
pub source: &'a str,
+ pub source_kind: SkillSource,
pub force: bool,
/// When `Some`, installs under this home instead of the real `$HOME`.
pub home: Option<&'a Path>,
@@ -103,7 +117,7 @@ pub fn install_to_harnesses_at_home(home: &Path, opts: &InstallOptions<'_>) -> I
let mut errors = Vec::new();
for harness in opts.harnesses {
- match install_one_at_home(home, *harness, opts.source, opts.force) {
+ match install_one_at_home(home, *harness, opts.source, opts.source_kind, opts.force) {
Ok((path, status)) => results.push(InstallResult {
harness: harness.cli_name().to_string(),
path,
@@ -111,7 +125,7 @@ pub fn install_to_harnesses_at_home(home: &Path, opts: &InstallOptions<'_>) -> I
}),
Err(err) => errors.push(InstallError {
harness: harness.cli_name().to_string(),
- message: err.to_string(),
+ message: format!("{err:#}"),
}),
}
}
@@ -123,18 +137,40 @@ fn install_one_at_home(
home: &Path,
harness: HarnessId,
source: &str,
+ source_kind: SkillSource,
force: bool,
) -> Result<(PathBuf, InstallStatus)> {
let dest_dir = harness.skill_dest_dir_for_home(home);
let dest_file = dest_dir.join("SKILL.md");
+ // A no-op install needs no write access. Recheck under the lock before writing
+ // so two installers that both observed a missing file cannot overwrite it.
+ if dest_file.exists() && !force {
+ return Ok((dest_file, InstallStatus::Skipped));
+ }
+ fs::create_dir_all(&dest_dir).with_context(|| format!("create {}", dest_dir.display()))?;
+ let _lock = storage::SkillLock::acquire(&dest_dir)
+ .with_context(|| format!("lock {}", dest_dir.display()))?;
+
if dest_file.exists() && !force {
return Ok((dest_file, InstallStatus::Skipped));
}
let existed = dest_file.exists();
- fs::create_dir_all(&dest_dir).with_context(|| format!("create {}", dest_dir.display()))?;
- fs::write(&dest_file, source).with_context(|| format!("write {}", dest_file.display()))?;
+ let marker = dest_dir.join(SOURCE_MARKER_FILE);
+ match source_kind {
+ SkillSource::Custom => {
+ let content = storage::PendingWrite::prepare(&dest_file, source)?;
+ // Protection must be established before any custom content appears.
+ storage::PendingWrite::prepare(&marker, SOURCE_CUSTOM)?.commit()?;
+ content
+ .commit()
+ .context("custom protection recorded, but skill content was not replaced")?;
+ }
+ SkillSource::Bundled => {
+ write_bundled_skill(&dest_file, source)?;
+ }
+ }
let status = if existed {
InstallStatus::Updated
@@ -144,6 +180,22 @@ fn install_one_at_home(
Ok((dest_file, status))
}
+/// Callers hold the skill lock. Publish the baseline only after its content;
+/// a failed marker replacement leaves a conservative, detectable mismatch.
+fn write_bundled_skill(dest: &Path, source: &str) -> Result<()> {
+ let content = storage::PendingWrite::prepare(dest, source)?;
+ let marker = dest
+ .parent()
+ .context("skill destination has no parent")?
+ .join(SOURCE_MARKER_FILE);
+ let metadata = provenance::bundled_marker(source.as_bytes())?;
+ let marker = storage::PendingWrite::prepare(&marker, &metadata)?;
+ content.commit()?;
+ marker
+ .commit()
+ .context("bundled skill content installed, but its source marker was not updated")
+}
+
/// Harnesses visible in the interactive installer (detected on this machine only).
pub fn interactive_candidates(reports: &[HarnessReport]) -> Vec<&HarnessReport> {
reports.iter().filter(|report| report.detected).collect()
@@ -310,6 +362,7 @@ mod tests {
&InstallOptions {
harnesses: &[harness],
source: "# test skill\n",
+ source_kind: SkillSource::Custom,
force: false,
home: Some(&home),
},
@@ -317,6 +370,10 @@ mod tests {
assert!(out.errors.is_empty());
assert_eq!(out.results.len(), 1);
assert!(skills.join(SKILL_DIR_NAME).join("SKILL.md").is_file());
+ assert_eq!(
+ fs::read_to_string(skills.join(SKILL_DIR_NAME).join(SOURCE_MARKER_FILE)).unwrap(),
+ SOURCE_CUSTOM
+ );
}
#[test]
@@ -336,6 +393,7 @@ mod tests {
&InstallOptions {
harnesses: &[harness],
source: "new",
+ source_kind: SkillSource::Custom,
force: false,
home: Some(&home),
},
@@ -361,12 +419,278 @@ mod tests {
&InstallOptions {
harnesses: &[harness],
source: "new",
+ source_kind: SkillSource::Custom,
force: true,
home: Some(&home),
},
);
assert_eq!(out.results[0].status, InstallStatus::Updated);
assert_eq!(fs::read_to_string(&dest).unwrap(), "new");
+ assert_eq!(
+ fs::read_to_string(dest.parent().unwrap().join(SOURCE_MARKER_FILE)).unwrap(),
+ SOURCE_CUSTOM
+ );
+ }
+
+ #[test]
+ fn bundled_install_is_marked_as_managed() {
+ let tmp = TempDir::new().unwrap();
+ let home = tmp.path().to_path_buf();
+ let harness = HarnessId::Cursor;
+
+ let out = install_to_harnesses_at_home(
+ &home,
+ &InstallOptions {
+ harnesses: &[harness],
+ source: DEFAULT_SKILL_MD,
+ source_kind: SkillSource::Bundled,
+ force: false,
+ home: Some(&home),
+ },
+ );
+
+ assert!(out.errors.is_empty());
+ let marker = harness
+ .skill_dest_dir_for_home(&home)
+ .join(SOURCE_MARKER_FILE);
+ assert_eq!(
+ provenance::read(&marker).unwrap(),
+ provenance::Provenance::Bundled {
+ sha256: provenance::digest(DEFAULT_SKILL_MD.as_bytes()),
+ }
+ );
+ }
+
+ #[test]
+ fn custom_install_survives_automatic_bundled_sync() {
+ let tmp = TempDir::new().unwrap();
+ let home = tmp.path().to_path_buf();
+ let harness = HarnessId::Cursor;
+ let dest = harness.skill_dest_dir_for_home(&home).join("SKILL.md");
+
+ let out = install_to_harnesses_at_home(
+ &home,
+ &InstallOptions {
+ harnesses: &[harness],
+ source: "custom instructions",
+ source_kind: SkillSource::Custom,
+ force: false,
+ home: Some(&home),
+ },
+ );
+ assert!(out.errors.is_empty());
+
+ let report = sync::sync_with_source(&home, "new bundled instructions");
+
+ assert_eq!(report.protected, vec![HarnessId::Cursor]);
+ assert_eq!(fs::read_to_string(dest).unwrap(), "custom instructions");
+ }
+
+ #[test]
+ fn skipped_install_does_not_claim_or_change_provenance() {
+ for marker in [None, Some(SOURCE_CUSTOM), Some(SOURCE_BUNDLED)] {
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ fs::create_dir_all(&dir).unwrap();
+ fs::write(dir.join("SKILL.md"), "keep").unwrap();
+ if let Some(marker) = marker {
+ fs::write(dir.join(SOURCE_MARKER_FILE), marker).unwrap();
+ }
+ let (_, status) = install_one_at_home(
+ home.path(),
+ HarnessId::Cursor,
+ DEFAULT_SKILL_MD,
+ SkillSource::Bundled,
+ false,
+ )
+ .unwrap();
+ assert_eq!(status, InstallStatus::Skipped);
+ assert_eq!(fs::read_to_string(dir.join("SKILL.md")).unwrap(), "keep");
+ assert!(!dir.join(".bsk.lock").exists());
+ assert_eq!(
+ fs::read_to_string(dir.join(SOURCE_MARKER_FILE))
+ .ok()
+ .as_deref(),
+ marker
+ );
+ }
+ }
+
+ #[test]
+ fn failed_install_keeps_content_and_provenance_safe() {
+ use storage::test_support::{assert_no_temporary_files, with_replace_hook};
+
+ for kind in [SkillSource::Custom, SkillSource::Bundled] {
+ for failed_file in ["SKILL.md", SOURCE_MARKER_FILE] {
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ let old_kind = if kind == SkillSource::Custom {
+ SkillSource::Bundled
+ } else {
+ SkillSource::Custom
+ };
+ install_one_at_home(
+ home.path(),
+ HarnessId::Cursor,
+ "old content",
+ old_kind,
+ false,
+ )
+ .unwrap();
+ let original_marker = fs::read_to_string(dir.join(SOURCE_MARKER_FILE)).unwrap();
+ let error = with_replace_hook(
+ move |dest| {
+ if dest.file_name().unwrap() == failed_file {
+ Err(std::io::Error::other("injected replacement failure"))
+ } else {
+ Ok(())
+ }
+ },
+ || {
+ install_one_at_home(
+ home.path(),
+ HarnessId::Cursor,
+ "new content",
+ kind,
+ true,
+ )
+ },
+ )
+ .unwrap_err();
+ assert!(format!("{error:#}").contains("injected replacement failure"));
+ let bundled_content_installed =
+ kind == SkillSource::Bundled && failed_file == SOURCE_MARKER_FILE;
+ let expected_content = if bundled_content_installed {
+ "new content"
+ } else {
+ "old content"
+ };
+ assert_eq!(
+ fs::read_to_string(dir.join("SKILL.md")).unwrap(),
+ expected_content
+ );
+ if bundled_content_installed {
+ assert!(
+ error
+ .to_string()
+ .contains("bundled skill content installed")
+ );
+ }
+ let still_bundled =
+ kind == SkillSource::Custom && failed_file == SOURCE_MARKER_FILE;
+ let expected_marker = if still_bundled {
+ original_marker.as_str()
+ } else {
+ SOURCE_CUSTOM
+ };
+ assert_eq!(
+ fs::read_to_string(dir.join(SOURCE_MARKER_FILE)).unwrap(),
+ expected_marker
+ );
+ assert_no_temporary_files(&dir);
+ let report = sync::sync_with_source(home.path(), "next bundled version");
+ if still_bundled {
+ assert_eq!(report.updated, vec![HarnessId::Cursor]);
+ } else {
+ assert_eq!(report.protected, vec![HarnessId::Cursor]);
+ assert_eq!(
+ fs::read_to_string(dir.join("SKILL.md")).unwrap(),
+ expected_content
+ );
+ }
+ }
+ }
+ }
+
+ #[test]
+ fn sync_defers_during_custom_install_then_preserves_it() {
+ use std::sync::mpsc;
+ use std::time::Duration;
+ use storage::test_support::with_replace_hook;
+
+ let home = TempDir::new().unwrap();
+ install_one_at_home(
+ home.path(),
+ HarnessId::Cursor,
+ "old bundled",
+ SkillSource::Bundled,
+ false,
+ )
+ .unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ let worker_home = home.path().to_path_buf();
+ let (ready_tx, ready_rx) = mpsc::channel();
+ let (resume_tx, resume_rx) = mpsc::channel();
+ let worker = std::thread::spawn(move || {
+ with_replace_hook(
+ move |dest| {
+ if dest.file_name().unwrap() == "SKILL.md" {
+ ready_tx.send(()).unwrap();
+ resume_rx.recv_timeout(Duration::from_secs(10)).unwrap();
+ }
+ Ok(())
+ },
+ || {
+ install_one_at_home(
+ &worker_home,
+ HarnessId::Cursor,
+ "custom",
+ SkillSource::Custom,
+ true,
+ )
+ },
+ )
+ });
+ ready_rx.recv_timeout(Duration::from_secs(10)).unwrap();
+ let during = sync::sync_with_source(home.path(), "new bundled");
+ assert_eq!(during.busy, vec![HarnessId::Cursor]);
+ assert!(during.updated.is_empty());
+ assert!(during.errors.is_empty());
+ assert_eq!(
+ fs::read_to_string(dir.join("SKILL.md")).unwrap(),
+ "old bundled"
+ );
+ assert_eq!(
+ fs::read_to_string(dir.join(SOURCE_MARKER_FILE)).unwrap(),
+ SOURCE_CUSTOM
+ );
+ resume_tx.send(()).unwrap();
+ worker.join().unwrap().unwrap();
+ let after = sync::sync_with_source(home.path(), "new bundled");
+ assert_eq!(after.protected, vec![HarnessId::Cursor]);
+ assert_eq!(fs::read_to_string(dir.join("SKILL.md")).unwrap(), "custom");
+ }
+
+ #[test]
+ fn forced_bundled_install_resumes_management() {
+ let home = TempDir::new().unwrap();
+ install_one_at_home(
+ home.path(),
+ HarnessId::Cursor,
+ "custom",
+ SkillSource::Custom,
+ false,
+ )
+ .unwrap();
+ install_one_at_home(
+ home.path(),
+ HarnessId::Cursor,
+ DEFAULT_SKILL_MD,
+ SkillSource::Bundled,
+ true,
+ )
+ .unwrap();
+ let report = sync::sync_with_source(home.path(), "new bundled");
+ assert_eq!(report.updated, vec![HarnessId::Cursor]);
+ assert_eq!(
+ fs::read_to_string(
+ HarnessId::Cursor
+ .skill_dest_dir_for_home(home.path())
+ .join("SKILL.md")
+ )
+ .unwrap(),
+ "new bundled"
+ );
}
#[test]
diff --git a/crates/bsk-cli/src/skill_install/provenance.rs b/crates/bsk-cli/src/skill_install/provenance.rs
new file mode 100644
index 00000000..991e8d5c
--- /dev/null
+++ b/crates/bsk-cli/src/skill_install/provenance.rs
@@ -0,0 +1,76 @@
+//! Read legacy source markers and record the last managed content in the same
+//! atomically replaced marker. A baseline describes what we wrote, not what the
+//! current binary happens to bundle.
+
+use std::path::Path;
+
+use anyhow::{Context, Result};
+use serde::{Deserialize, Serialize};
+use sha2::{Digest, Sha256};
+
+use super::{SOURCE_BUNDLED, SOURCE_CUSTOM, SkillSource};
+
+#[derive(Debug, PartialEq, Eq)]
+pub(super) enum Provenance {
+ Missing,
+ Custom,
+ LegacyBundled,
+ Bundled { sha256: String },
+ Invalid,
+}
+
+#[derive(Deserialize, Serialize)]
+#[serde(deny_unknown_fields)]
+struct Marker {
+ version: u8,
+ source: SkillSource,
+ #[serde(default, skip_serializing_if = "Option::is_none")]
+ sha256: Option,
+}
+
+pub(super) fn digest(content: &[u8]) -> String {
+ Sha256::digest(content)
+ .iter()
+ .map(|byte| format!("{byte:02x}"))
+ .collect()
+}
+
+pub(super) fn bundled_marker(content: &[u8]) -> Result {
+ let marker = Marker {
+ version: 1,
+ source: SkillSource::Bundled,
+ sha256: Some(digest(content)),
+ };
+ Ok(format!("{}\n", serde_json::to_string(&marker)?))
+}
+
+pub(super) fn read(marker: &Path) -> Result {
+ let bytes = match std::fs::read(marker) {
+ Ok(bytes) => bytes,
+ Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(Provenance::Missing),
+ Err(err) => return Err(err).with_context(|| format!("read {}", marker.display())),
+ };
+ if bytes == SOURCE_CUSTOM.as_bytes() {
+ return Ok(Provenance::Custom);
+ }
+ if bytes == SOURCE_BUNDLED.as_bytes() {
+ return Ok(Provenance::LegacyBundled);
+ }
+ match serde_json::from_slice::(&bytes) {
+ Ok(Marker {
+ version: 1,
+ source: SkillSource::Custom,
+ ..
+ }) => Ok(Provenance::Custom),
+ Ok(Marker {
+ version: 1,
+ source: SkillSource::Bundled,
+ sha256: Some(hash),
+ }) if hash.len() == 64 && hash.bytes().all(|byte| byte.is_ascii_hexdigit()) => {
+ Ok(Provenance::Bundled {
+ sha256: hash.to_ascii_lowercase(),
+ })
+ }
+ _ => Ok(Provenance::Invalid),
+ }
+}
diff --git a/crates/bsk-cli/src/skill_install/storage.rs b/crates/bsk-cli/src/skill_install/storage.rs
new file mode 100644
index 00000000..9caba570
--- /dev/null
+++ b/crates/bsk-cli/src/skill_install/storage.rs
@@ -0,0 +1,156 @@
+//! File operations shared by installation and automatic synchronization.
+
+use std::fs::{File, OpenOptions};
+use std::io::{self, Write};
+use std::path::{Path, PathBuf};
+
+use anyhow::{Context, Result};
+use fs2::FileExt;
+use tempfile::NamedTempFile;
+
+/// All readers that may replace a skill hold this lock until their writes finish.
+/// Keep the lock file in place: deleting it could let two processes lock different
+/// files at the same path. Closing the handle releases the OS lock.
+pub(super) struct SkillLock {
+ _file: File,
+}
+
+impl SkillLock {
+ fn open(dir: &Path) -> io::Result {
+ OpenOptions::new()
+ .read(true)
+ .write(true)
+ .create(true)
+ .truncate(false)
+ .open(dir.join(".bsk.lock"))
+ }
+
+ pub(super) fn acquire(dir: &Path) -> io::Result {
+ let file = Self::open(dir)?;
+ file.lock_exclusive()?;
+ Ok(Self { _file: file })
+ }
+
+ /// Automatic sync is best-effort and must not wait on another installer.
+ pub(super) fn try_acquire(dir: &Path) -> io::Result> {
+ let file = Self::open(dir)?;
+ match file.try_lock_exclusive() {
+ Ok(()) => Ok(Some(Self { _file: file })),
+ Err(err) if err.raw_os_error() == fs2::lock_contended_error().raw_os_error() => {
+ Ok(None)
+ }
+ Err(err) => Err(err),
+ }
+ }
+}
+
+/// Prepare complete content before changing the destination. The unique sibling
+/// temporary file is removed on error or drop, including a failed replacement.
+pub(super) struct PendingWrite {
+ file: NamedTempFile,
+ dest: PathBuf,
+}
+
+impl PendingWrite {
+ pub(super) fn prepare(dest: &Path, content: &str) -> Result {
+ let mut file = tempfile::Builder::new()
+ .prefix(".bsk-tmp-")
+ .tempfile_in(dest.parent().context("skill destination has no parent")?)
+ .with_context(|| format!("prepare {}", dest.display()))?;
+ file.write_all(content.as_bytes())
+ .with_context(|| format!("write temporary file for {}", dest.display()))?;
+ Ok(Self {
+ file,
+ dest: dest.to_path_buf(),
+ })
+ }
+
+ pub(super) fn commit(self) -> Result<()> {
+ #[cfg(test)]
+ test_support::before_replace(&self.dest)?;
+ self.file
+ .persist(&self.dest)
+ .map_err(|err| err.error)
+ .with_context(|| format!("replace {}", self.dest.display()))?;
+ Ok(())
+ }
+}
+
+#[cfg(test)]
+pub(super) mod test_support {
+ use super::*;
+ use std::cell::RefCell;
+
+ type ReplaceHook = Box io::Result<()>>;
+ thread_local! {
+ // Thread-local injection keeps failure and interleaving tests independent.
+ static REPLACE_HOOK: RefCell> = RefCell::new(None);
+ }
+
+ pub(super) fn before_replace(dest: &Path) -> io::Result<()> {
+ REPLACE_HOOK.with_borrow(|hook| match hook {
+ Some(hook) => hook(dest),
+ None => Ok(()),
+ })
+ }
+
+ pub(crate) fn with_replace_hook(
+ hook: impl Fn(&Path) -> io::Result<()> + 'static,
+ run: impl FnOnce() -> T,
+ ) -> T {
+ struct Reset(Option);
+ impl Drop for Reset {
+ fn drop(&mut self) {
+ REPLACE_HOOK.set(self.0.take());
+ }
+ }
+ let _reset = Reset(REPLACE_HOOK.replace(Some(Box::new(hook))));
+ run()
+ }
+
+ pub(crate) fn assert_no_temporary_files(dir: &Path) {
+ for entry in std::fs::read_dir(dir).unwrap() {
+ assert!(
+ !entry
+ .unwrap()
+ .file_name()
+ .to_string_lossy()
+ .starts_with(".bsk-tmp-")
+ );
+ }
+ }
+}
+
+#[cfg(test)]
+mod tests {
+ use super::*;
+
+ #[test]
+ fn failed_atomic_replace_preserves_destination_and_cleans_up() {
+ let dir = tempfile::tempdir().unwrap();
+ let dest = dir.path().join("SKILL.md");
+ std::fs::create_dir(&dest).unwrap();
+ std::fs::write(dest.join("sentinel"), "keep").unwrap();
+ assert!(
+ PendingWrite::prepare(&dest, "new")
+ .unwrap()
+ .commit()
+ .is_err()
+ );
+ assert_eq!(
+ std::fs::read_to_string(dest.join("sentinel")).unwrap(),
+ "keep"
+ );
+ test_support::assert_no_temporary_files(dir.path());
+ }
+
+ #[test]
+ fn lock_is_shared_by_separate_handles_and_released_on_drop() {
+ let dir = tempfile::tempdir().unwrap();
+ let lock = SkillLock::acquire(dir.path()).unwrap();
+ assert!(SkillLock::try_acquire(dir.path()).unwrap().is_none());
+ drop(lock);
+ assert!(dir.path().join(".bsk.lock").exists());
+ assert!(SkillLock::try_acquire(dir.path()).unwrap().is_some());
+ }
+}
diff --git a/crates/bsk-cli/src/skill_install/sync.rs b/crates/bsk-cli/src/skill_install/sync.rs
index dbd7e68b..b98140ce 100644
--- a/crates/bsk-cli/src/skill_install/sync.rs
+++ b/crates/bsk-cli/src/skill_install/sync.rs
@@ -3,21 +3,53 @@
use std::path::Path;
-use super::{DEFAULT_SKILL_MD, harness::HarnessId};
+use anyhow::{Context, Result};
+
+use super::{
+ DEFAULT_SKILL_MD, SOURCE_MARKER_FILE,
+ harness::HarnessId,
+ provenance::{self, Provenance},
+ storage::{PendingWrite, SkillLock},
+};
/// Per-harness outcome of a sync pass.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
pub struct SyncReport {
/// Harnesses whose on-disk `SKILL.md` differed and was rewritten.
pub updated: Vec,
- /// Harnesses whose on-disk `SKILL.md` already matched the bundled
+ /// Managed harnesses whose on-disk `SKILL.md` already matched the bundled
/// content; no write happened, mtime preserved.
pub up_to_date: Vec,
+ /// Explicit custom installations that intentionally opt out of updates.
+ pub protected: Vec,
+ /// Content preserved because safe automatic updates need user attention.
+ pub paused: Vec<(HarnessId, PauseReason)>,
+ /// Another install/sync holds the lock; retry on a later sync pass.
+ pub busy: Vec,
/// Harnesses that have an installed `SKILL.md` but the sync attempt
/// failed with an I/O error. The string is a human-readable detail.
pub errors: Vec<(HarnessId, String)>,
}
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
+pub enum PauseReason {
+ Untracked,
+ MissingBaseline,
+ LocalChanges,
+ InvalidMarker,
+}
+
+impl PauseReason {
+ pub fn description(self) -> &'static str {
+ match self {
+ Self::Untracked => "untracked installation differs from the bundled skill",
+ Self::MissingBaseline => "older bundled installation has no content baseline",
+ Self::LocalChanges => "local changes detected",
+ Self::InvalidMarker => "unrecognized or damaged source marker",
+ }
+ }
+}
+
/// Iterates `HarnessId::ALL`, syncing harnesses with an existing
/// `SKILL.md` and leaving the rest untouched.
pub fn sync_installed_skills(home: &Path) -> SyncReport {
@@ -30,10 +62,13 @@ pub(crate) fn sync_with_source(home: &Path, source: &str) -> SyncReport {
for &harness in HarnessId::ALL {
let dest = harness.skill_dest_dir_for_home(home).join("SKILL.md");
match sync_one(&dest, source) {
- SyncOne::Missing => continue,
- SyncOne::UpToDate => report.up_to_date.push(harness),
- SyncOne::Updated => report.updated.push(harness),
- SyncOne::Error(msg) => report.errors.push((harness, msg)),
+ Ok(SyncOne::Missing) => continue,
+ Ok(SyncOne::UpToDate) => report.up_to_date.push(harness),
+ Ok(SyncOne::Updated) => report.updated.push(harness),
+ Ok(SyncOne::Protected) => report.protected.push(harness),
+ Ok(SyncOne::Paused(reason)) => report.paused.push((harness, reason)),
+ Ok(SyncOne::Busy) => report.busy.push(harness),
+ Err(err) => report.errors.push((harness, format!("{err:#}"))),
}
}
report
@@ -43,38 +78,56 @@ enum SyncOne {
Missing,
UpToDate,
Updated,
- Error(String),
+ Protected,
+ Paused(PauseReason),
+ Busy,
}
-fn sync_one(dest: &Path, source: &str) -> SyncOne {
+fn sync_one(dest: &Path, source: &str) -> Result {
+ // Do not create directories or locks for uninstalled harnesses.
if !dest.is_file() {
- return SyncOne::Missing;
+ return Ok(SyncOne::Missing);
}
- let on_disk = match std::fs::read_to_string(dest) {
- Ok(s) => s,
- Err(err) => return SyncOne::Error(format!("read {}: {err}", dest.display())),
+ let dir = dest.parent().context("skill destination has no parent")?;
+ let Some(_lock) =
+ SkillLock::try_acquire(dir).with_context(|| format!("lock {}", dir.display()))?
+ else {
+ return Ok(SyncOne::Busy);
};
- if on_disk == source {
- return SyncOne::UpToDate;
- }
- // Atomic replace: write tmp, rename over. Including pid in the
- // tmp suffix avoids concurrent processes racing on the same path.
- let tmp = dest.with_extension(format!("md.tmp.{}", std::process::id()));
- if let Err(err) = std::fs::write(&tmp, source) {
- // Best-effort cleanup of any partial tmp left by a half-written attempt.
- let _ = std::fs::remove_file(&tmp);
- return SyncOne::Error(format!("write {}: {err}", tmp.display()));
- }
- if let Err(err) = std::fs::rename(&tmp, dest) {
- // Best-effort cleanup of the orphan tmp file.
- let _ = std::fs::remove_file(&tmp);
- return SyncOne::Error(format!(
- "rename {} -> {}: {err}",
- tmp.display(),
- dest.display()
- ));
- }
- SyncOne::Updated
+
+ // Explicit custom intent wins over byte equality; unknown metadata is never
+ // silently claimed. Only missing or recognized legacy markers can migrate.
+ let marker = dir.join(SOURCE_MARKER_FILE);
+ let ownership = provenance::read(&marker)?;
+ match ownership {
+ Provenance::Custom => return Ok(SyncOne::Protected),
+ Provenance::Invalid => return Ok(SyncOne::Paused(PauseReason::InvalidMarker)),
+ _ => {}
+ }
+ let on_disk = match std::fs::read(dest) {
+ Ok(content) => content,
+ Err(err) if err.kind() == std::io::ErrorKind::NotFound => return Ok(SyncOne::Missing),
+ Err(err) => return Err(err).with_context(|| format!("read {}", dest.display())),
+ };
+ let matches_baseline = matches!(&ownership, Provenance::Bundled { sha256 }
+ if *sha256 == provenance::digest(&on_disk));
+ if on_disk == source.as_bytes() {
+ if !matches_baseline {
+ // Also recovers a content update whose final marker write failed.
+ // Neither adoption nor recovery rewrites SKILL.md or its mtime.
+ let metadata = provenance::bundled_marker(&on_disk)?;
+ PendingWrite::prepare(&marker, &metadata)?.commit()?;
+ }
+ return Ok(SyncOne::UpToDate);
+ }
+ match ownership {
+ Provenance::Missing => return Ok(SyncOne::Paused(PauseReason::Untracked)),
+ Provenance::LegacyBundled => return Ok(SyncOne::Paused(PauseReason::MissingBaseline)),
+ Provenance::Bundled { .. } if matches_baseline => {}
+ _ => return Ok(SyncOne::Paused(PauseReason::LocalChanges)),
+ }
+ super::write_bundled_skill(dest, source)?;
+ Ok(SyncOne::Updated)
}
#[cfg(test)]
@@ -82,6 +135,14 @@ mod tests {
use super::*;
use tempfile::TempDir;
+ fn mark_bundled(dest: &Path) {
+ std::fs::write(
+ dest.parent().unwrap().join(SOURCE_MARKER_FILE),
+ provenance::bundled_marker(&std::fs::read(dest).unwrap()).unwrap(),
+ )
+ .unwrap();
+ }
+
#[test]
fn sync_skips_uninstalled_harness() {
let tmp = TempDir::new().unwrap();
@@ -95,8 +156,8 @@ mod tests {
.skill_dest_dir_for_home(tmp.path())
.join("SKILL.md");
assert!(
- !dest.exists(),
- "sync should not create files for uninstalled harnesses"
+ !dest.parent().unwrap().exists(),
+ "sync should not create directories or locks for uninstalled harnesses"
);
}
@@ -108,6 +169,7 @@ mod tests {
std::fs::create_dir_all(&dest_dir).unwrap();
let dest = dest_dir.join("SKILL.md");
std::fs::write(&dest, b"old content").unwrap();
+ mark_bundled(&dest);
let report = sync_with_source(home, "fresh content");
@@ -115,18 +177,7 @@ mod tests {
assert!(report.up_to_date.is_empty());
assert!(report.errors.is_empty());
assert_eq!(std::fs::read_to_string(&dest).unwrap(), "fresh content");
- // Atomicity guard: no orphan tmp files matching SKILL.md.tmp.* should
- // remain after a successful sync (regardless of pid suffix).
- let leftovers: Vec<_> = std::fs::read_dir(&dest_dir)
- .unwrap()
- .filter_map(|entry| entry.ok())
- .map(|entry| entry.file_name())
- .filter(|name| name.to_string_lossy().starts_with("SKILL.md.tmp"))
- .collect();
- assert!(
- leftovers.is_empty(),
- "no SKILL.md.tmp.* files should remain after sync, found: {leftovers:?}"
- );
+ super::super::storage::test_support::assert_no_temporary_files(&dest_dir);
}
#[test]
@@ -137,6 +188,7 @@ mod tests {
std::fs::create_dir_all(&dest_dir).unwrap();
let dest = dest_dir.join("SKILL.md");
std::fs::write(&dest, "frozen content").unwrap();
+ mark_bundled(&dest);
let mtime_before = std::fs::metadata(&dest).unwrap().modified().unwrap();
// Sleep enough that any rewrite would visibly change mtime on
@@ -167,12 +219,14 @@ mod tests {
let cursor_dir = HarnessId::Cursor.skill_dest_dir_for_home(home);
std::fs::create_dir_all(&cursor_dir).unwrap();
std::fs::write(cursor_dir.join("SKILL.md"), "old").unwrap();
+ mark_bundled(&cursor_dir.join("SKILL.md"));
- // Codex: parent dir set to r-x. Reads still succeed, but creating
- // SKILL.md.tmp fails → exercises sync_one's write-tmp error branch.
+ // Codex: a read-only directory prevents lock creation. The other
+ // harness must still update, and the failure must not become Busy.
let codex_dir = HarnessId::Codex.skill_dest_dir_for_home(home);
std::fs::create_dir_all(&codex_dir).unwrap();
std::fs::write(codex_dir.join("SKILL.md"), "old").unwrap();
+ mark_bundled(&codex_dir.join("SKILL.md"));
let mut perms = std::fs::metadata(&codex_dir).unwrap().permissions();
perms.set_mode(0o500); // r-x: blocks tmp creation in this dir
std::fs::set_permissions(&codex_dir, perms).unwrap();
@@ -188,4 +242,344 @@ mod tests {
assert_eq!(report.errors.len(), 1);
assert_eq!(report.errors[0].0, HarnessId::Codex);
}
+
+ #[test]
+ fn sync_preserves_custom_and_untracked_skills() {
+ let tmp = TempDir::new().unwrap();
+ let home = tmp.path();
+
+ let custom_dir = HarnessId::Cursor.skill_dest_dir_for_home(home);
+ std::fs::create_dir_all(&custom_dir).unwrap();
+ std::fs::write(custom_dir.join("SKILL.md"), "custom content").unwrap();
+ std::fs::write(custom_dir.join(SOURCE_MARKER_FILE), "custom\n").unwrap();
+
+ let untracked_dir = HarnessId::Codex.skill_dest_dir_for_home(home);
+ std::fs::create_dir_all(&untracked_dir).unwrap();
+ std::fs::write(untracked_dir.join("SKILL.md"), "historical content").unwrap();
+
+ let report = sync_with_source(home, "new bundled content");
+
+ assert_eq!(report.protected, vec![HarnessId::Cursor]);
+ assert_eq!(
+ report.paused,
+ vec![(HarnessId::Codex, PauseReason::Untracked)]
+ );
+ assert_eq!(
+ std::fs::read_to_string(custom_dir.join("SKILL.md")).unwrap(),
+ "custom content"
+ );
+ assert_eq!(
+ std::fs::read_to_string(untracked_dir.join("SKILL.md")).unwrap(),
+ "historical content"
+ );
+ }
+ #[test]
+ fn unknown_markers_are_not_claimed_even_when_content_matches() {
+ for marker in [
+ "",
+ "unknown\n",
+ r#"{"version":2,"source":"bundled","sha256":"abc"}"#,
+ r#"{"version":1,"source":"bundled","sha256":"invalid"}"#,
+ r#"{"version":1,"source":"bundled"}"#,
+ r#"{"version":1,"source":"custom","unexpected":true}"#,
+ ] {
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(&dir).unwrap();
+ std::fs::write(dir.join("SKILL.md"), "same").unwrap();
+ std::fs::write(dir.join(SOURCE_MARKER_FILE), marker).unwrap();
+ for source in ["same", "new bundled"] {
+ let report = sync_with_source(home.path(), source);
+ assert_eq!(
+ report.paused,
+ vec![(HarnessId::Cursor, PauseReason::InvalidMarker)]
+ );
+ assert!(report.up_to_date.is_empty());
+ assert!(report.errors.is_empty());
+ assert_eq!(
+ std::fs::read_to_string(dir.join("SKILL.md")).unwrap(),
+ "same"
+ );
+ }
+ assert_eq!(
+ std::fs::read_to_string(dir.join(SOURCE_MARKER_FILE)).unwrap(),
+ marker
+ );
+ }
+ }
+
+ #[test]
+ fn marker_read_error_is_reported_without_changing_content() {
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(dir.join(SOURCE_MARKER_FILE)).unwrap();
+ std::fs::write(dir.join("SKILL.md"), "keep").unwrap();
+ let report = sync_with_source(home.path(), "new bundled");
+ assert_eq!(report.errors.len(), 1);
+ assert_eq!(report.errors[0].0, HarnessId::Cursor);
+ assert!(report.errors[0].1.contains(SOURCE_MARKER_FILE));
+ assert_eq!(
+ std::fs::read_to_string(dir.join("SKILL.md")).unwrap(),
+ "keep"
+ );
+ }
+
+ #[test]
+ fn failed_sync_replace_preserves_old_content_and_releases_lock() {
+ use super::super::storage::test_support::{assert_no_temporary_files, with_replace_hook};
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(&dir).unwrap();
+ let dest = dir.join("SKILL.md");
+ std::fs::write(&dest, "old").unwrap();
+ mark_bundled(&dest);
+ let report = with_replace_hook(
+ |_| Err(std::io::Error::other("injected replacement failure")),
+ || sync_with_source(home.path(), "new"),
+ );
+ assert_eq!(report.errors.len(), 1);
+ assert!(report.updated.is_empty());
+ assert_eq!(std::fs::read_to_string(&dest).unwrap(), "old");
+ assert_no_temporary_files(&dir);
+ assert_eq!(
+ sync_with_source(home.path(), "new").updated,
+ vec![HarnessId::Cursor]
+ );
+ }
+
+ #[test]
+ fn sync_holds_lock_until_content_replacement_finishes() {
+ use super::super::storage::test_support::with_replace_hook;
+ use std::sync::mpsc;
+ use std::time::Duration;
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(&dir).unwrap();
+ let dest = dir.join("SKILL.md");
+ std::fs::write(&dest, "old").unwrap();
+ mark_bundled(&dest);
+ let worker_home = home.path().to_path_buf();
+ let (ready_tx, ready_rx) = mpsc::channel();
+ let (resume_tx, resume_rx) = mpsc::channel();
+ let worker = std::thread::spawn(move || {
+ with_replace_hook(
+ move |dest| {
+ if dest.file_name().unwrap() == "SKILL.md" {
+ ready_tx.send(()).unwrap();
+ resume_rx.recv_timeout(Duration::from_secs(10)).unwrap();
+ }
+ Ok(())
+ },
+ || sync_with_source(&worker_home, "first update"),
+ )
+ });
+ ready_rx.recv_timeout(Duration::from_secs(10)).unwrap();
+ let during = sync_with_source(home.path(), "second update");
+ assert_eq!(during.busy, vec![HarnessId::Cursor]);
+ assert!(during.errors.is_empty());
+ assert_eq!(std::fs::read_to_string(&dest).unwrap(), "old");
+ resume_tx.send(()).unwrap();
+ assert_eq!(worker.join().unwrap().updated, vec![HarnessId::Cursor]);
+ assert_eq!(std::fs::read_to_string(&dest).unwrap(), "first update");
+ }
+ #[test]
+ fn matching_legacy_installations_are_adopted_without_rewriting_content() {
+ use std::time::{Duration, SystemTime};
+ for legacy in [None, Some(super::super::SOURCE_BUNDLED)] {
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(&dir).unwrap();
+ let dest = dir.join("SKILL.md");
+ let marker = dir.join(SOURCE_MARKER_FILE);
+ std::fs::write(&dest, "current bundle").unwrap();
+ std::fs::OpenOptions::new()
+ .write(true)
+ .open(&dest)
+ .unwrap()
+ .set_modified(SystemTime::UNIX_EPOCH + Duration::from_secs(1_600_000_000))
+ .unwrap();
+ let mtime = std::fs::metadata(&dest).unwrap().modified().unwrap();
+ if let Some(legacy) = legacy {
+ std::fs::write(&marker, legacy).unwrap();
+ }
+ let report = sync_with_source(home.path(), "current bundle");
+ assert_eq!(report.up_to_date, vec![HarnessId::Cursor]);
+ assert!(report.paused.is_empty());
+ assert_eq!(std::fs::metadata(&dest).unwrap().modified().unwrap(), mtime);
+ assert_eq!(
+ provenance::read(&marker).unwrap(),
+ Provenance::Bundled {
+ sha256: provenance::digest(b"current bundle")
+ }
+ );
+ let marker_mtime = std::fs::metadata(&marker).unwrap().modified().unwrap();
+ assert_eq!(
+ sync_with_source(home.path(), "current bundle").up_to_date,
+ vec![HarnessId::Cursor]
+ );
+ assert_eq!(
+ std::fs::metadata(&marker).unwrap().modified().unwrap(),
+ marker_mtime
+ );
+ assert_eq!(
+ sync_with_source(home.path(), "next bundle").updated,
+ vec![HarnessId::Cursor]
+ );
+ assert_eq!(std::fs::read(&dest).unwrap(), b"next bundle");
+ }
+ }
+
+ #[test]
+ fn differing_legacy_content_is_preserved_with_a_specific_pause_reason() {
+ for (legacy, reason) in [
+ (None, PauseReason::Untracked),
+ (
+ Some(super::super::SOURCE_BUNDLED),
+ PauseReason::MissingBaseline,
+ ),
+ ] {
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(&dir).unwrap();
+ let marker = dir.join(SOURCE_MARKER_FILE);
+ // Even a trailing newline is a byte difference, not an adoption match.
+ std::fs::write(dir.join("SKILL.md"), "bundle\n").unwrap();
+ if let Some(legacy) = legacy {
+ std::fs::write(&marker, legacy).unwrap();
+ }
+ let report = sync_with_source(home.path(), "bundle");
+ assert_eq!(report.paused, vec![(HarnessId::Cursor, reason)]);
+ assert!(report.updated.is_empty());
+ assert!(report.errors.is_empty());
+ assert_eq!(
+ std::fs::read_to_string(dir.join("SKILL.md")).unwrap(),
+ "bundle\n"
+ );
+ assert_eq!(std::fs::read_to_string(&marker).ok().as_deref(), legacy);
+ }
+ }
+
+ #[test]
+ fn failed_adoption_keeps_legacy_content_and_marker_intact() {
+ use super::super::storage::test_support::{assert_no_temporary_files, with_replace_hook};
+ for legacy in [None, Some(super::super::SOURCE_BUNDLED)] {
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(&dir).unwrap();
+ std::fs::write(dir.join("SKILL.md"), "bundle").unwrap();
+ let marker = dir.join(SOURCE_MARKER_FILE);
+ if let Some(legacy) = legacy {
+ std::fs::write(&marker, legacy).unwrap();
+ }
+ let report = with_replace_hook(
+ |_| Err(std::io::Error::other("injected marker failure")),
+ || sync_with_source(home.path(), "bundle"),
+ );
+ assert_eq!(report.errors.len(), 1);
+ assert!(report.up_to_date.is_empty());
+ assert_eq!(std::fs::read_to_string(&marker).ok().as_deref(), legacy);
+ assert_eq!(
+ std::fs::read_to_string(dir.join("SKILL.md")).unwrap(),
+ "bundle"
+ );
+ assert_no_temporary_files(&dir);
+ assert_eq!(
+ sync_with_source(home.path(), "bundle").up_to_date,
+ vec![HarnessId::Cursor]
+ );
+ }
+ }
+
+ #[test]
+ fn local_edits_are_preserved_until_the_recorded_content_is_restored() {
+ for edited in [b"my instructions".as_slice(), b"bundle v1\n", b"\xff"] {
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(&dir).unwrap();
+ let dest = dir.join("SKILL.md");
+ std::fs::write(&dest, "bundle v1").unwrap();
+ mark_bundled(&dest);
+ let original_marker = std::fs::read(dir.join(SOURCE_MARKER_FILE)).unwrap();
+ std::fs::write(&dest, edited).unwrap();
+ for target in ["bundle v1", "bundle v2"] {
+ let report = sync_with_source(home.path(), target);
+ assert_eq!(
+ report.paused,
+ vec![(HarnessId::Cursor, PauseReason::LocalChanges)]
+ );
+ assert!(report.errors.is_empty());
+ assert_eq!(std::fs::read(&dest).unwrap(), edited);
+ assert_eq!(
+ std::fs::read(dir.join(SOURCE_MARKER_FILE)).unwrap(),
+ original_marker
+ );
+ }
+ std::fs::write(&dest, "bundle v1").unwrap();
+ assert_eq!(
+ sync_with_source(home.path(), "bundle v2").updated,
+ vec![HarnessId::Cursor]
+ );
+ assert_eq!(
+ provenance::read(&dir.join(SOURCE_MARKER_FILE)).unwrap(),
+ Provenance::Bundled {
+ sha256: provenance::digest(b"bundle v2")
+ }
+ );
+ // The updated baseline must protect edits made after an upgrade, too.
+ std::fs::write(&dest, "v2 with local edits").unwrap();
+ assert_eq!(
+ sync_with_source(home.path(), "bundle v3").paused,
+ vec![(HarnessId::Cursor, PauseReason::LocalChanges)]
+ );
+ }
+ }
+
+ #[test]
+ fn interrupted_sync_preserves_content_and_repairs_only_a_matching_bundle() {
+ use super::super::storage::test_support::{assert_no_temporary_files, with_replace_hook};
+ let home = TempDir::new().unwrap();
+ let dir = HarnessId::Cursor.skill_dest_dir_for_home(home.path());
+ std::fs::create_dir_all(&dir).unwrap();
+ let dest = dir.join("SKILL.md");
+ std::fs::write(&dest, "old bundle").unwrap();
+ mark_bundled(&dest);
+ let old_marker = std::fs::read(dir.join(SOURCE_MARKER_FILE)).unwrap();
+ let report = with_replace_hook(
+ |dest| {
+ if dest.file_name().unwrap() == SOURCE_MARKER_FILE {
+ Err(std::io::Error::other("injected marker failure"))
+ } else {
+ Ok(())
+ }
+ },
+ || sync_with_source(home.path(), "new bundle"),
+ );
+ assert_eq!(report.errors.len(), 1);
+ assert!(
+ report.errors[0]
+ .1
+ .contains("bundled skill content installed")
+ );
+ assert_eq!(std::fs::read(&dest).unwrap(), b"new bundle");
+ assert_eq!(
+ std::fs::read(dir.join(SOURCE_MARKER_FILE)).unwrap(),
+ old_marker
+ );
+ assert_no_temporary_files(&dir);
+ // A different binary cannot guess the origin of this mismatch.
+ assert_eq!(
+ sync_with_source(home.path(), "another bundle").paused,
+ vec![(HarnessId::Cursor, PauseReason::LocalChanges)]
+ );
+ let mtime = std::fs::metadata(&dest).unwrap().modified().unwrap();
+ assert_eq!(
+ sync_with_source(home.path(), "new bundle").up_to_date,
+ vec![HarnessId::Cursor]
+ );
+ assert_eq!(std::fs::metadata(&dest).unwrap().modified().unwrap(), mtime);
+ assert_eq!(
+ sync_with_source(home.path(), "another bundle").updated,
+ vec![HarnessId::Cursor]
+ );
+ }
}