Skip to content
4 changes: 2 additions & 2 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,8 @@ A configured remote (or local) source of skills, managed by `repos`. Types: `git
_Avoid_: **source**, **registry** — both are deprecated command aliases now folded into `repos`; do not reintroduce them as concepts.

**Origin**:
Where a single installed skill came from — both the *input* to an install and the *provenance* recorded on the installed skill (one type, one truth). Variants: `git` (url + ref + subdir), `local` (path, editable), `zip-url`, and `repository` (a *reference into* a configured **Repository**: `{repo, skill, version?}`). The `repository` variant is the only one **Version constraint** / ADR-0004 governs; `git`/`local`/`zip-url` are ref-based and versionless. `Origin` is the single canonical model — it replaces the former `SkillSource` (two colliding types), `SourceType`, and the manifest `"source"` serde tag.
_Avoid_: **source** (banned, see above); do not blur `Origin::repository` (a reference) with **Repository** (the configured place itself).
Where a single installed skill came from — the install **intent** (what the user asked for), recorded as provenance on the installed skill. Variants: `git` (url + ref + subdir), `local` (a filesystem path — directory *or* `.zip` — plus `editable`, dir-only), `zip-url` (a remote zip), and `repository` (a *reference into* a configured **Repository**: `{repo, skill, version?}`). The `repository` variant is the only one **Version constraint** / ADR-0004 governs; `git`/`local`/`zip-url` are ref-based and versionless. `Origin` is intent only: the **resolved** facts (exact commit, resolved version, checksum, timestamps) live in the **Lock**, not in `Origin`. It is the single canonical model — replacing the former `SkillSource` (two colliding types), `SourceType`, `SourceSpecificFields`, and the flat `source_*` fields on the manifest/lock/skill records.
_Avoid_: **source** (banned, see above); do not blur `Origin::repository` (a reference; always names a concrete Repository) with **Repository** (the configured place itself).

### Serving surfaces

Expand Down
18 changes: 3 additions & 15 deletions crates/fastskill-cli/src/commands/add/install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -88,26 +88,14 @@ pub(super) async fn finish_skill_install(
) -> CliResult<()> {
use chrono::Utc;
skill_def.skill_file = storage_dir.join("SKILL.md");
skill_def.source_url = meta.source_url;
skill_def.source_type = meta.source_type;
skill_def.source_branch = meta.source_branch;
skill_def.source_tag = meta.source_tag;
skill_def.source_subdir = meta.source_subdir;
skill_def.installed_from = meta.installed_from;
skill_def.origin = meta.origin;
skill_def.fetched_at = Some(Utc::now());
skill_def.editable = ctx.editable;

super::register_skill_once(ctx, &skill_def).await?;

let update = fastskill_core::core::skill_manager::SkillUpdate {
source_url: skill_def.source_url.clone(),
source_type: skill_def.source_type.clone(),
source_branch: skill_def.source_branch.clone(),
source_tag: skill_def.source_tag.clone(),
source_subdir: skill_def.source_subdir.clone(),
installed_from: skill_def.installed_from.clone(),
origin: Some(skill_def.origin.clone()),
fetched_at: skill_def.fetched_at,
editable: Some(skill_def.editable),
..Default::default()
};
ctx.service
Expand All @@ -133,7 +121,7 @@ pub(super) async fn finish_skill_install(
crate::utils::messages::ok("Updated global-skills.lock")
);
} else {
super::update_project_files(&skill_def, ctx.groups.clone(), ctx.editable)?;
super::update_project_files(&skill_def, ctx.groups.clone())?;
println!(
"Successfully added skill: {} (v{})",
skill_def.name, version_display
Expand Down
22 changes: 8 additions & 14 deletions crates/fastskill-cli/src/commands/add/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ use cli_framework::command::{FromArgValueMap, IntoCommandSpec};
use cli_framework::spec::arg_spec::{ArgKind, ArgSpec, ArgValueType, Cardinality};
use cli_framework::spec::command_tree::CommandSpec;
use cli_framework::spec::value::ArgValue;
use fastskill_core::core::origin::Origin;
use fastskill_core::core::project::resolve_project_file;
use fastskill_core::core::skill_manager::SourceType;
use fastskill_core::{FastSkillService, SkillDefinition};
pub use install::copy_dir_recursive;
pub use skill_def::create_skill_from_path;
Expand All @@ -31,14 +31,12 @@ struct AddContext<'a> {
global: bool,
}

/// Source metadata to record after installing a skill
/// Source metadata to record after installing a skill.
///
/// `Origin` (install intent) is now the single source of truth for provenance,
/// so this is a thin wrapper rather than a flat bag of source_* fields.
struct SourceMeta {
source_url: Option<String>,
source_type: Option<SourceType>,
source_branch: Option<String>,
source_tag: Option<String>,
source_subdir: Option<PathBuf>,
installed_from: Option<String>,
origin: Origin,
}

/// Target for install: where to copy and what to record
Expand All @@ -48,13 +46,9 @@ struct InstallTarget {
version_display: String,
}

fn update_project_files(
skill_def: &SkillDefinition,
groups: Vec<String>,
editable: bool,
) -> CliResult<()> {
fn update_project_files(skill_def: &SkillDefinition, groups: Vec<String>) -> CliResult<()> {
use crate::utils::manifest_utils;
manifest_utils::add_skill_to_project_toml(skill_def, groups.clone(), editable)
manifest_utils::add_skill_to_project_toml(skill_def, groups.clone())
.map_err(|e| CliError::Config(format!("Failed to update skill-project.toml: {}", e)))?;

let current_dir = env::current_dir()
Expand Down
34 changes: 28 additions & 6 deletions crates/fastskill-cli/src/commands/add/skill_def.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! Skill definition helpers for the add command.

use crate::error::{CliError, CliResult, CliWarning};
use fastskill_core::core::origin::Origin;
use fastskill_core::SkillDefinition;
use std::fs;
use std::path::Path;
Expand Down Expand Up @@ -86,8 +87,12 @@ fn generate_fallback_skill_id(skill_path: &Path) -> CliResult<String> {

/// Create a skill definition from a path containing SKILL.md.
/// The skill ID is read from skill-project.toml if present, otherwise from SKILL.md frontmatter.
///
/// `origin` is the caller-constructed provenance (install intent) for this skill;
/// `source_type` is only a cosmetic label used for the missing-toml warning display.
pub fn create_skill_from_path(
skill_path: &Path,
origin: Origin,
source_type: &str,
editable: bool,
) -> CliResult<SkillDefinition> {
Expand Down Expand Up @@ -129,8 +134,13 @@ pub fn create_skill_from_path(
"1.0.0".to_string()
};

let mut skill =
SkillDefinition::new(skill_id, frontmatter.name, frontmatter.description, version);
let mut skill = SkillDefinition::new(
skill_id,
frontmatter.name,
frontmatter.description,
version,
origin,
);

skill.skill_file = skill_file.clone();
skill.author = frontmatter.author;
Expand All @@ -145,6 +155,15 @@ mod tests {
use std::fs;
use tempfile::TempDir;

/// A throwaway `Origin` for tests that only care about the resulting `SkillDefinition`
/// fields derived from SKILL.md / skill-project.toml, not the origin itself.
fn test_origin(path: &Path) -> Origin {
Origin::Local {
path: path.to_path_buf(),
editable: false,
}
}

#[test]
fn test_read_project_metadata_missing_file() {
let tmp = TempDir::new().unwrap();
Expand Down Expand Up @@ -181,7 +200,8 @@ Test skill content
"#;
fs::write(tmp.path().join("SKILL.md"), skill_md).unwrap();

let skill = create_skill_from_path(tmp.path(), "local", false).unwrap();
let skill =
create_skill_from_path(tmp.path(), test_origin(tmp.path()), "local", false).unwrap();
assert_eq!(skill.id.as_str(), "test-skill-no-toml");
}

Expand All @@ -204,7 +224,8 @@ version = "1.5.0"
"#;
fs::write(tmp.path().join("skill-project.toml"), project_toml).unwrap();

let skill = create_skill_from_path(tmp.path(), "local", false).unwrap();
let skill =
create_skill_from_path(tmp.path(), test_origin(tmp.path()), "local", false).unwrap();
assert_eq!(skill.id.as_str(), "from-toml");
assert_eq!(skill.version, "1.5.0");
}
Expand All @@ -220,7 +241,8 @@ Test content
"#;
fs::write(tmp.path().join("SKILL.md"), skill_md).unwrap();

let skill = create_skill_from_path(tmp.path(), "local", false).unwrap();
let skill =
create_skill_from_path(tmp.path(), test_origin(tmp.path()), "local", false).unwrap();
assert_eq!(skill.id.as_str(), "fallback-name-skill");
}

Expand All @@ -235,7 +257,7 @@ Test content
"#;
fs::write(tmp.path().join("SKILL.md"), skill_md).unwrap();

let result = create_skill_from_path(tmp.path(), "local", false);
let result = create_skill_from_path(tmp.path(), test_origin(tmp.path()), "local", false);
assert!(result.is_err(), "Should fail due to spaces in name");
}
}
Loading
Loading