Skip to content
Merged
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
15 changes: 9 additions & 6 deletions crates/goose-sdk/src/custom_requests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -652,8 +652,9 @@ impl std::fmt::Display for SourceType {
}
}

/// A source discovered by Goose and backed by an on-disk path. Sources may be
/// either `global` (shared across all projects) or project-specific.
/// A source discovered by Goose. Filesystem sources use an on-disk path;
/// built-in sources use a stable synthetic path. Sources may be either
/// `global` (shared across all projects) or project-specific.
#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema)]
#[serde(rename_all = "camelCase")]
pub struct SourceEntry {
Expand All @@ -663,7 +664,8 @@ pub struct SourceEntry {
pub description: String,
pub content: String,
/// Absolute path to the source on disk. A directory for skills, a file for
/// recipes and agents.
/// recipes and agents. Built-in skills use read-only synthetic
/// `builtin://skills/<name>` paths.
pub directory: String,
/// True when the source lives in the user's global sources directory; false
/// when it lives inside a specific project.
Expand Down Expand Up @@ -710,9 +712,10 @@ pub struct CreateSourceResponse {

/// List discovered sources.
///
/// Today this endpoint only returns skills. If `type` is omitted, it defaults
/// to listing skill sources. Both global and project-scoped skills are included
/// when `project_dir` is set.
/// If `type` is omitted or `skill`, this lists filesystem/plugin skills only.
/// Both global and project-scoped skills are included when `project_dir` is
/// set. If `type` is `builtinSkill`, this lists shipped read-only built-in
/// skills.
#[derive(Debug, Default, Clone, Serialize, Deserialize, JsonSchema, JsonRpcRequest)]
#[request(method = "_goose/sources/list", response = ListSourcesResponse)]
#[serde(rename_all = "camelCase")]
Expand Down
6 changes: 3 additions & 3 deletions crates/goose/acp-schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1572,7 +1572,7 @@
},
"directory": {
"type": "string",
"description": "Absolute path to the source on disk. A directory for skills, a file for\nrecipes and agents."
"description": "Absolute path to the source on disk. A directory for skills, a file for\nrecipes and agents. Built-in skills use read-only synthetic\n`builtin://skills/<name>` paths."
},
"global": {
"type": "boolean",
Expand All @@ -1594,7 +1594,7 @@
"directory",
"global"
],
"description": "A source discovered by Goose and backed by an on-disk path. Sources may be\neither `global` (shared across all projects) or project-specific."
"description": "A source discovered by Goose. Filesystem sources use an on-disk path;\nbuilt-in sources use a stable synthetic path. Sources may be either\n`global` (shared across all projects) or project-specific."
},
"ListSourcesRequest": {
"type": "object",
Expand All @@ -1616,7 +1616,7 @@
]
}
},
"description": "List discovered sources.\n\nToday this endpoint only returns skills. If `type` is omitted, it defaults\nto listing skill sources. Both global and project-scoped skills are included\nwhen `project_dir` is set.",
"description": "List discovered sources.\n\nIf `type` is omitted or `skill`, this lists filesystem/plugin skills only.\nBoth global and project-scoped skills are included when `project_dir` is\nset. If `type` is `builtinSkill`, this lists shipped read-only built-in\nskills.",
"x-side": "agent",
"x-method": "_goose/sources/list"
},
Expand Down
2 changes: 2 additions & 0 deletions crates/goose/src/skills/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -369,8 +369,10 @@ pub fn discover_skills(working_dir: Option<&Path>) -> Vec<SourceEntry> {
if let Some(source) = parse_skill_content(content, &PathBuf::new(), true) {
if !seen.contains(&source.name) {
seen.insert(source.name.clone());
let directory = format!("builtin://skills/{}", source.name);
sources.push(SourceEntry {
source_type: SourceType::BuiltinSkill,
directory,
..source
});
}
Expand Down
125 changes: 119 additions & 6 deletions crates/goose/src/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ fn require_skill_type(source_type: SourceType) -> Result<(), Error> {
Ok(())
}

fn require_listable_type(source_type: Option<SourceType>) -> Result<SourceType, Error> {
match source_type.unwrap_or(SourceType::Skill) {
SourceType::Skill => Ok(SourceType::Skill),
SourceType::BuiltinSkill => Ok(SourceType::BuiltinSkill),
other => Err(Error::invalid_params().data(format!(
"Source type '{}' is not supported. Only 'skill' and 'builtinSkill' are currently supported for listing.",
other
))),
}
}

fn source_entry(
source_type: SourceType,
name: &str,
Expand All @@ -55,6 +66,14 @@ fn source_entry(
}
}

fn builtin_skill_entry(mut source: SourceEntry) -> SourceEntry {
source.source_type = SourceType::BuiltinSkill;
source.directory = format!("builtin://skills/{}", source.name);
source.global = true;
source.supporting_files.clear();
source
}

pub fn create_source(
source_type: SourceType,
name: &str,
Expand Down Expand Up @@ -155,9 +174,7 @@ pub fn list_sources(
source_type: Option<SourceType>,
project_dir: Option<&str>,
) -> Result<Vec<SourceEntry>, Error> {
if let Some(t) = source_type {
require_skill_type(t)?;
}
let listed_type = require_listable_type(source_type)?;

let working_dir = project_dir
.map(str::trim)
Expand All @@ -166,7 +183,14 @@ pub fn list_sources(

let mut sources: Vec<SourceEntry> = discover_skills(working_dir.as_deref())
.into_iter()
.filter(|s| s.source_type == SourceType::Skill)
.filter(|s| s.source_type == listed_type)
.map(|s| {
if listed_type == SourceType::BuiltinSkill {
builtin_skill_entry(s)
} else {
s
}
})
.collect();

sources.sort_by(|a, b| a.name.cmp(&b.name));
Expand Down Expand Up @@ -495,7 +519,64 @@ mod tests {
}

#[test]
fn rejects_non_skill_source_type() {
fn list_sources_lists_builtin_skills() {
let listed = list_sources(Some(SourceType::BuiltinSkill), None).unwrap();
let builtin = listed
.iter()
.find(|source| source.name == "goose-doc-guide")
.expect("expected goose-doc-guide builtin skill");

assert_eq!(builtin.source_type, SourceType::BuiltinSkill);
assert!(builtin.global);
assert_eq!(builtin.directory, "builtin://skills/goose-doc-guide");
assert!(builtin.supporting_files.is_empty());
assert!(!builtin.content.is_empty());
}

#[test]
fn list_skill_excludes_builtin_skills() {
let listed = list_sources(Some(SourceType::Skill), None).unwrap();
assert!(!listed
.iter()
.any(|source| source.source_type == SourceType::BuiltinSkill));
}

#[test]
fn filesystem_skill_suppresses_same_named_builtin() {
let tmp = TempDir::new().unwrap();
let project = tmp.path();
let skill_dir = project
.join(".agents")
.join("skills")
.join("goose-doc-guide");
std::fs::create_dir_all(&skill_dir).unwrap();
std::fs::write(
skill_dir.join("SKILL.md"),
build_skill_md("goose-doc-guide", "project override", "Use project docs"),
)
.unwrap();

let builtins = list_sources(
Some(SourceType::BuiltinSkill),
Some(project.to_str().unwrap()),
)
.unwrap();
assert!(!builtins
.iter()
.any(|source| source.name == "goose-doc-guide"));

let skills =
list_sources(Some(SourceType::Skill), Some(project.to_str().unwrap())).unwrap();
let project_skill = skills
.iter()
.find(|source| source.name == "goose-doc-guide")
.expect("expected project skill");
assert_eq!(project_skill.source_type, SourceType::Skill);
assert_eq!(project_skill.description, "project override");
}

#[test]
fn mutations_reject_non_writable_source_types() {
let tmp = TempDir::new().unwrap();
let project = tmp.path().to_str().unwrap();

Expand All @@ -510,17 +591,49 @@ mod tests {
.unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));

let err = update_source(
SourceType::BuiltinSkill,
"builtin://skills/x",
"x",
"d",
"c",
)
.unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));

let err = update_source(SourceType::Recipe, "x", "x", "d", "c").unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));

let err = delete_source(SourceType::BuiltinSkill, "builtin://skills/x").unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));

let err = delete_source(SourceType::Subrecipe, "x").unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));

let err = list_sources(Some(SourceType::BuiltinSkill), Some(project)).unwrap_err();
let listed = list_sources(Some(SourceType::BuiltinSkill), Some(project)).unwrap();
assert!(listed
.iter()
.any(|source| source.source_type == SourceType::BuiltinSkill));

let err = list_sources(Some(SourceType::Recipe), Some(project)).unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));

let err = export_source(SourceType::BuiltinSkill, "builtin://skills/x").unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));

let err = export_source(SourceType::Recipe, "x").unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));

let payload = serde_json::json!({
"version": 1,
"type": "builtinSkill",
"name": "x",
"description": "d",
"content": "c",
})
.to_string();
let err = import_sources(&payload, false, Some(project)).unwrap_err();
assert!(format!("{:?}", err).contains("not supported"));
}

#[test]
Expand Down
34 changes: 34 additions & 0 deletions crates/goose/tests/acp_custom_requests_test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,40 @@ fn test_custom_get_extensions() {
});
}

#[test]
fn test_custom_list_builtin_skill_sources() {
run_test(async move {
let openai = OpenAiFixture::new(vec![], Arc::new(EnforceSessionId::default())).await;
let conn = AcpServerConnection::new(TestConnectionConfig::default(), openai).await;

let response = send_custom(
conn.cx(),
"_goose/sources/list",
serde_json::json!({ "type": "builtinSkill" }),
)
.await
.expect("builtin skill sources list should succeed");
let sources = response
.get("sources")
.and_then(|value| value.as_array())
.expect("missing sources array");
let builtin = sources
.iter()
.find(|source| source.get("name") == Some(&serde_json::json!("goose-doc-guide")))
.expect("expected goose-doc-guide builtin skill");

assert_eq!(
builtin.get("type"),
Some(&serde_json::json!("builtinSkill"))
);
assert_eq!(builtin.get("global"), Some(&serde_json::json!(true)));
assert_eq!(
builtin.get("directory"),
Some(&serde_json::json!("builtin://skills/goose-doc-guide"))
);
});
}

#[test]
fn test_custom_provider_inventory_includes_metadata() {
run_test(async {
Expand Down
Loading
Loading