Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions apps/extension/vitest.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@ export default defineConfig({
},
define: {
__BSK_EXT_VERSION__: JSON.stringify(pkg.version),
__BSK_DAEMON_WS_URL__: JSON.stringify(
process.env.BSK_DAEMON_WS_URL ?? "ws://127.0.0.1:52800",
),
__BSK_DAEMON_WS_URL__: JSON.stringify(process.env.BSK_DAEMON_WS_URL ?? "ws://127.0.0.1:52800"),
},
resolve: {
alias: {
Expand Down
13 changes: 13 additions & 0 deletions crates/bsk-cli/src/cli/doctor.rs
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,19 @@ fn check_skill_up_to_date() -> CheckResult {
);
}

if !report.protected.is_empty() {
let names = report
.protected
.iter()
.map(|h| h.cli_name())
.collect::<Vec<_>>()
.join(", ");
return CheckResult::na(
name,
format!("custom or unmanaged skill preserved in: {names}"),
);
}

CheckResult::na(name, "no agent skill installed")
}

Expand Down
65 changes: 65 additions & 0 deletions crates/bsk-cli/src/skill_install/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,9 @@ pub use harness::{HarnessId, HarnessReport, all_harness_reports, parse_harness_i

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";

#[derive(Debug, Clone, Serialize)]
pub struct InstallResult {
Expand Down Expand Up @@ -135,6 +138,13 @@ fn install_one_at_home(
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 source_kind = if source == DEFAULT_SKILL_MD {
SOURCE_BUNDLED
} else {
SOURCE_CUSTOM
};
let marker = dest_dir.join(SOURCE_MARKER_FILE);
fs::write(&marker, source_kind).with_context(|| format!("write {}", marker.display()))?;

let status = if existed {
InstallStatus::Updated
Expand Down Expand Up @@ -317,6 +327,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]
Expand Down Expand Up @@ -367,6 +381,57 @@ mod tests {
);
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,
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!(fs::read_to_string(marker).unwrap(), SOURCE_BUNDLED);
}

#[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",
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]
Expand Down
56 changes: 55 additions & 1 deletion crates/bsk-cli/src/skill_install/sync.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@

use std::path::Path;

use super::{DEFAULT_SKILL_MD, harness::HarnessId};
use super::{DEFAULT_SKILL_MD, SOURCE_BUNDLED, SOURCE_MARKER_FILE, harness::HarnessId};

/// Per-harness outcome of a sync pass.
#[derive(Debug, Clone, Default, PartialEq, Eq)]
Expand All @@ -13,6 +13,9 @@ pub struct SyncReport {
/// Harnesses whose on-disk `SKILL.md` already matched the bundled
/// content; no write happened, mtime preserved.
pub up_to_date: Vec<HarnessId>,
/// Custom or historical untracked installations that must not be
/// overwritten by automatic bundled-skill synchronization.
pub protected: Vec<HarnessId>,
/// 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)>,
Expand All @@ -33,6 +36,7 @@ pub(crate) fn sync_with_source(home: &Path, source: &str) -> SyncReport {
SyncOne::Missing => continue,
SyncOne::UpToDate => report.up_to_date.push(harness),
SyncOne::Updated => report.updated.push(harness),
SyncOne::Protected => report.protected.push(harness),
SyncOne::Error(msg) => report.errors.push((harness, msg)),
}
}
Expand All @@ -43,6 +47,7 @@ enum SyncOne {
Missing,
UpToDate,
Updated,
Protected,
Error(String),
}

Expand All @@ -57,6 +62,16 @@ fn sync_one(dest: &Path, source: &str) -> SyncOne {
if on_disk == source {
return SyncOne::UpToDate;
}
let marker = dest
.parent()
.expect("SKILL.md destination must have a parent")
.join(SOURCE_MARKER_FILE);
match std::fs::read_to_string(&marker) {
Ok(value) if value == SOURCE_BUNDLED => {}
Ok(_) => return SyncOne::Protected,
Err(err) if err.kind() == std::io::ErrorKind::NotFound => return SyncOne::Protected,
Err(err) => return SyncOne::Error(format!("read {}: {err}", marker.display())),
}
// 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()));
Expand All @@ -82,6 +97,14 @@ mod tests {
use super::*;
use tempfile::TempDir;

fn mark_bundled(dest: &Path) {
std::fs::write(
dest.parent().unwrap().join(SOURCE_MARKER_FILE),
SOURCE_BUNDLED,
)
.unwrap();
}

#[test]
fn sync_skips_uninstalled_harness() {
let tmp = TempDir::new().unwrap();
Expand All @@ -108,6 +131,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");

Expand Down Expand Up @@ -137,6 +161,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
Expand Down Expand Up @@ -167,12 +192,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.
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();
Expand All @@ -188,4 +215,31 @@ 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::Codex, HarnessId::Cursor]);
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"
);
}
}
5 changes: 1 addition & 4 deletions crates/bsk-cli/tests/browser_list_ordering.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ use bsk_protocol::system::BrowserStatusEntry;
use bsk_protocol::{ErrorCode, Method};
use rand::Rng;
use serde::Deserialize;
use tokio::net::TcpListener;
use tokio::sync::mpsc;

fn tempfile_path(prefix: &str) -> PathBuf {
Expand All @@ -30,9 +29,7 @@ fn tempfile_path(prefix: &str) -> PathBuf {
}

async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) {
let probe = TcpListener::bind("127.0.0.1:0").await.unwrap();
let port = probe.local_addr().unwrap().port();
drop(probe);
let port = 0;

let config = DaemonConfig::new(port);
let sock = tempfile_path("bsk-test-browser-list");
Expand Down
5 changes: 1 addition & 4 deletions crates/bsk-cli/tests/browser_wait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use bsk_protocol::system::{BrowserListParams, HandshakeParams, HandshakeResult,
use bsk_protocol::{BrowserPeerInfo, Method, RequestFrame, ResponseBody, ResponseFrame};
use futures_util::{SinkExt, StreamExt};
use rand::Rng;
use tokio::net::TcpListener;
use tokio_tungstenite::tungstenite::handshake::client::generate_key;
use tokio_tungstenite::tungstenite::http::Request;
use tokio_tungstenite::tungstenite::protocol::Message;
Expand Down Expand Up @@ -77,9 +76,7 @@ fn tempfile_path(prefix: &str) -> PathBuf {
}

async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) {
let probe = TcpListener::bind("127.0.0.1:0").await.unwrap();
let port = probe.local_addr().unwrap().port();
drop(probe);
let port = 0;

let config = DaemonConfig::new(port);
let sock = tempfile_path("bsk-test-browser-wait");
Expand Down
5 changes: 1 addition & 4 deletions crates/bsk-cli/tests/cancel_forwarding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ use bsk_protocol::{
use futures_util::{SinkExt, StreamExt};
use rand::Rng;
use serde_json::json;
use tokio::net::TcpListener;
use tokio_tungstenite::tungstenite::handshake::client::generate_key;
use tokio_tungstenite::tungstenite::http::Request;
use tokio_tungstenite::tungstenite::protocol::Message;
Expand All @@ -40,9 +39,7 @@ fn tempfile_path(prefix: &str) -> PathBuf {
}

async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) {
let probe = TcpListener::bind("127.0.0.1:0").await.unwrap();
let port = probe.local_addr().unwrap().port();
drop(probe);
let port = 0;

let config = DaemonConfig::new(port);
let sock = tempfile_path("bsk-test-cancel");
Expand Down
5 changes: 1 addition & 4 deletions crates/bsk-cli/tests/handshake_compat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ use bsk_protocol::system::{HandshakeParams, HandshakeResult, StatusResult};
use bsk_protocol::{BrowserPeerInfo, ErrorCode, Method, RequestFrame, ResponseBody, ResponseFrame};
use futures_util::{SinkExt, StreamExt};
use rand::Rng;
use tokio::net::TcpListener;
use tokio_tungstenite::tungstenite::handshake::client::generate_key;
use tokio_tungstenite::tungstenite::http::Request;
use tokio_tungstenite::tungstenite::protocol::Message;
Expand All @@ -28,9 +27,7 @@ fn tempfile_path(prefix: &str) -> PathBuf {
}

async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) {
let probe = TcpListener::bind("127.0.0.1:0").await.unwrap();
let port = probe.local_addr().unwrap().port();
drop(probe);
let port = 0;

let config = DaemonConfig::new(port);
let sock = tempfile_path("bsk-test-handshake");
Expand Down
5 changes: 1 addition & 4 deletions crates/bsk-cli/tests/per_session_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ use futures_util::{SinkExt, StreamExt};
use rand::Rng;
use serde::Deserialize;
use serde_json::json;
use tokio::net::TcpListener;
use tokio::sync::{Mutex, mpsc};
use tokio_tungstenite::tungstenite::handshake::client::generate_key;
use tokio_tungstenite::tungstenite::http::Request;
Expand All @@ -46,9 +45,7 @@ fn tempfile_path(prefix: &str) -> PathBuf {
}

async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) {
let probe = TcpListener::bind("127.0.0.1:0").await.unwrap();
let port = probe.local_addr().unwrap().port();
drop(probe);
let port = 0;
let sock = tempfile_path("bsk-test-queue");
let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone()))
.await
Expand Down
5 changes: 1 addition & 4 deletions crates/bsk-cli/tests/session_user_interrupt.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ use bsk_protocol::{
use futures_util::{SinkExt, StreamExt};
use rand::Rng;
use serde_json::json;
use tokio::net::TcpListener;
use tokio_tungstenite::tungstenite::handshake::client::generate_key;
use tokio_tungstenite::tungstenite::http::Request;
use tokio_tungstenite::tungstenite::protocol::Message;
Expand All @@ -46,9 +45,7 @@ fn tempfile_path(prefix: &str) -> PathBuf {
}

async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) {
let probe = TcpListener::bind("127.0.0.1:0").await.unwrap();
let port = probe.local_addr().unwrap().port();
drop(probe);
let port = 0;

let config = DaemonConfig::new(port);
let sock = tempfile_path("bsk-test-user-interrupt");
Expand Down
5 changes: 1 addition & 4 deletions crates/bsk-cli/tests/sessions_ipc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use bsk_protocol::tools::{SessionStartParams, SessionStartResult, SessionStopPar
use bsk_protocol::{BrowserPeerInfo, Frame, Method, RequestFrame, ResponseBody, ResponseFrame};
use futures_util::{SinkExt, StreamExt};
use rand::Rng;
use tokio::net::TcpListener;
use tokio_tungstenite::tungstenite::handshake::client::generate_key;
use tokio_tungstenite::tungstenite::http::Request;
use tokio_tungstenite::tungstenite::protocol::Message;
Expand All @@ -39,9 +38,7 @@ async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) {
}

async fn spawn_daemon_with_connect_wait(connect_wait: Duration) -> (daemon::DaemonHandle, PathBuf) {
let probe = TcpListener::bind("127.0.0.1:0").await.unwrap();
let port = probe.local_addr().unwrap().port();
drop(probe);
let port = 0;

let config = DaemonConfig::new(port).with_extension_connect_wait(connect_wait);
let sock = tempfile_path("bsk-test-ipc");
Expand Down
Loading