feat(acp): expose built-in skills through sources list acp calls (#9045)
Signed-off-by: Kalvin Chau <kalvin@block.xyz>
This commit is contained in:
@@ -831,8 +831,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 {
|
||||
@@ -842,7 +843,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.
|
||||
@@ -889,9 +891,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")]
|
||||
|
||||
@@ -1938,7 +1938,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",
|
||||
@@ -1960,7 +1960,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",
|
||||
@@ -1982,7 +1982,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"
|
||||
},
|
||||
|
||||
@@ -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
|
||||
});
|
||||
}
|
||||
|
||||
+119
-6
@@ -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,
|
||||
@@ -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,
|
||||
@@ -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)
|
||||
@@ -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));
|
||||
@@ -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();
|
||||
|
||||
@@ -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]
|
||||
|
||||
@@ -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 {
|
||||
|
||||
Reference in New Issue
Block a user