diff --git a/crates/goose-sdk/src/custom_requests.rs b/crates/goose-sdk/src/custom_requests.rs index d77521bb..9ac41aa7 100644 --- a/crates/goose-sdk/src/custom_requests.rs +++ b/crates/goose-sdk/src/custom_requests.rs @@ -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/` 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")] diff --git a/crates/goose/acp-schema.json b/crates/goose/acp-schema.json index cd25c04c..87c80ff9 100644 --- a/crates/goose/acp-schema.json +++ b/crates/goose/acp-schema.json @@ -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/` 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" }, diff --git a/crates/goose/src/skills/mod.rs b/crates/goose/src/skills/mod.rs index ab45646b..1d1d6ce6 100644 --- a/crates/goose/src/skills/mod.rs +++ b/crates/goose/src/skills/mod.rs @@ -369,8 +369,10 @@ pub fn discover_skills(working_dir: Option<&Path>) -> Vec { 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 }); } diff --git a/crates/goose/src/sources.rs b/crates/goose/src/sources.rs index 9a353803..8b7b21e0 100644 --- a/crates/goose/src/sources.rs +++ b/crates/goose/src/sources.rs @@ -36,6 +36,17 @@ fn require_skill_type(source_type: SourceType) -> Result<(), Error> { Ok(()) } +fn require_listable_type(source_type: Option) -> Result { + 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, project_dir: Option<&str>, ) -> Result, 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 = 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] diff --git a/crates/goose/tests/acp_custom_requests_test.rs b/crates/goose/tests/acp_custom_requests_test.rs index 7e4e8b53..31b65d4f 100644 --- a/crates/goose/tests/acp_custom_requests_test.rs +++ b/crates/goose/tests/acp_custom_requests_test.rs @@ -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 { diff --git a/ui/goose2/src/features/skills/api/skills.test.ts b/ui/goose2/src/features/skills/api/skills.test.ts index eb29b13f..9d20b258 100644 --- a/ui/goose2/src/features/skills/api/skills.test.ts +++ b/ui/goose2/src/features/skills/api/skills.test.ts @@ -30,6 +30,7 @@ describe("listSkills", () => { }, ], }) + .mockResolvedValueOnce({ sources: [] }) .mockResolvedValueOnce({ sources: [ { @@ -58,6 +59,9 @@ describe("listSkills", () => { type: "skill", }); expect(mockGooseSourcesList).toHaveBeenNthCalledWith(2, { + type: "builtinSkill", + }); + expect(mockGooseSourcesList).toHaveBeenNthCalledWith(3, { type: "skill", projectDir: "/tmp/alpha", }); @@ -84,6 +88,7 @@ describe("listSkills", () => { it("recognizes legacy .goose project skill paths", async () => { mockGooseSourcesList + .mockResolvedValueOnce({ sources: [] }) .mockResolvedValueOnce({ sources: [] }) .mockResolvedValueOnce({ sources: [ @@ -133,6 +138,7 @@ describe("listSkills", () => { }, ], }) + .mockResolvedValueOnce({ sources: [] }) .mockRejectedValueOnce(new Error("permission denied")) .mockResolvedValueOnce({ sources: [ @@ -150,10 +156,129 @@ describe("listSkills", () => { const { listSkills } = await import("./skills"); const skills = await listSkills(["/tmp/alpha", "/tmp/beta"]); - expect(mockGooseSourcesList).toHaveBeenCalledTimes(3); + expect(mockGooseSourcesList).toHaveBeenCalledTimes(4); expect(skills.map((skill) => skill.name)).toEqual([ "code-review", "test-writer", ]); }); + + it("keeps filesystem skills when built-in skill listing fails", async () => { + mockGooseSourcesList + .mockResolvedValueOnce({ + sources: [ + { + type: "skill", + name: "code-review", + description: "Reviews code", + content: "Review carefully", + directory: "/Users/test/.agents/skills/code-review", + global: true, + }, + ], + }) + .mockRejectedValueOnce(new Error("unknown source type")) + .mockResolvedValueOnce({ + sources: [ + { + type: "skill", + name: "test-writer", + description: "Writes tests", + content: "Write tests", + directory: "/tmp/alpha/.agents/skills/test-writer", + global: false, + }, + ], + }); + + const { listSkills } = await import("./skills"); + const skills = await listSkills(["/tmp/alpha"]); + + expect(mockGooseSourcesList).toHaveBeenNthCalledWith(1, { + type: "skill", + }); + expect(mockGooseSourcesList).toHaveBeenNthCalledWith(2, { + type: "builtinSkill", + }); + expect(mockGooseSourcesList).toHaveBeenNthCalledWith(3, { + type: "skill", + projectDir: "/tmp/alpha", + }); + expect(skills.map((skill) => skill.name)).toEqual([ + "code-review", + "test-writer", + ]); + }); + + it("fetches and maps built-in skills without filesystem project/global metadata", async () => { + mockGooseSourcesList + .mockResolvedValueOnce({ + sources: [ + { + type: "skill", + name: "personal-review", + description: "Reviews personal code", + content: "Review local changes", + directory: "/Users/test/.agents/skills/personal-review", + global: true, + }, + ], + }) + .mockResolvedValueOnce({ + sources: [ + { + type: "builtinSkill", + name: "goose-doc-guide", + description: "Goose documentation guide", + content: "Use Goose docs", + directory: "builtin://skills/goose-doc-guide", + global: true, + }, + ], + }) + .mockResolvedValueOnce({ + sources: [ + { + type: "skill", + name: "project-helper", + description: "Helps project work", + content: "Use project context", + directory: "/tmp/alpha/.agents/skills/project-helper", + global: false, + }, + ], + }); + + const { listSkills } = await import("./skills"); + const skills = await listSkills(["/tmp/alpha"]); + + expect(mockGooseSourcesList).toHaveBeenNthCalledWith(1, { + type: "skill", + }); + expect(mockGooseSourcesList).toHaveBeenNthCalledWith(2, { + type: "builtinSkill", + }); + expect(mockGooseSourcesList).toHaveBeenNthCalledWith(3, { + type: "skill", + projectDir: "/tmp/alpha", + }); + expect(skills.map((skill) => skill.name)).toEqual([ + "personal-review", + "goose-doc-guide", + "project-helper", + ]); + expect(skills).toEqual( + expect.arrayContaining([ + expect.objectContaining({ + id: "builtin:goose-doc-guide", + name: "goose-doc-guide", + path: "builtin://skills/goose-doc-guide", + fileLocation: "builtin://skills/goose-doc-guide", + sourceKind: "builtin", + sourceLabel: "Built in", + projectLinks: [], + }), + ]), + ); + }); }); diff --git a/ui/goose2/src/features/skills/api/skills.ts b/ui/goose2/src/features/skills/api/skills.ts index c5be0404..ebc488e7 100644 --- a/ui/goose2/src/features/skills/api/skills.ts +++ b/ui/goose2/src/features/skills/api/skills.ts @@ -7,6 +7,7 @@ import { } from "../lib/skillsPath"; const SKILL_SOURCE_TYPE = "skill" as const; +const BUILTIN_SKILL_SOURCE_TYPE = "builtinSkill" as const; export interface SkillProjectLink { id: string; @@ -14,7 +15,7 @@ export interface SkillProjectLink { workingDir: string; } -export type SkillSourceKind = "global" | "project"; +export type SkillSourceKind = "global" | "project" | "builtin"; export interface SkillInfo { id: string; @@ -33,13 +34,42 @@ export type EditingSkill = Pick< "name" | "description" | "instructions" | "path" | "fileLocation" >; -type SkillSourceEntry = SourceEntry & { type: typeof SKILL_SOURCE_TYPE }; +type FilesystemSkillSourceEntry = SourceEntry & { + type: typeof SKILL_SOURCE_TYPE; +}; +type BuiltinSkillSourceEntry = SourceEntry & { + type: typeof BUILTIN_SKILL_SOURCE_TYPE; +}; +type SkillSourceEntry = FilesystemSkillSourceEntry | BuiltinSkillSourceEntry; -function isSkillSource(source: SourceEntry): source is SkillSourceEntry { +function isFilesystemSkillSource( + source: SourceEntry, +): source is FilesystemSkillSourceEntry { return source.type === SKILL_SOURCE_TYPE; } +function isSkillSource(source: SourceEntry): source is SkillSourceEntry { + return ( + source.type === SKILL_SOURCE_TYPE || + source.type === BUILTIN_SKILL_SOURCE_TYPE + ); +} + function toSkillInfo(source: SkillSourceEntry): SkillInfo { + if (source.type === BUILTIN_SKILL_SOURCE_TYPE) { + return { + id: `builtin:${source.name}`, + name: source.name, + description: source.description, + instructions: source.content, + path: source.directory, + fileLocation: source.directory, + sourceKind: "builtin", + sourceLabel: "Built in", + projectLinks: [], + }; + } + const sourceKind: SkillSourceKind = source.global ? "global" : "project"; const projectRoot = source.global ? null @@ -93,20 +123,29 @@ export async function listSkills( projectDirs: string[] = [], ): Promise { const client = await getClient(); - const fetchSources = (projectDir?: string) => + const fetchSources = ( + type: typeof SKILL_SOURCE_TYPE | typeof BUILTIN_SKILL_SOURCE_TYPE, + projectDir?: string, + ) => client.goose.GooseSourcesList({ - type: SKILL_SOURCE_TYPE, + type, ...(projectDir ? { projectDir } : {}), }); - const globalResponse = await fetchSources(); + const [globalResponse, builtinResponse] = await Promise.all([ + fetchSources(SKILL_SOURCE_TYPE), + fetchSources(BUILTIN_SKILL_SOURCE_TYPE).catch(() => null), + ]); const projectResponses = await Promise.allSettled( uniqueProjectDirs(projectDirs).map((projectDir) => - fetchSources(projectDir), + fetchSources(SKILL_SOURCE_TYPE, projectDir), ), ); const responses = [ { response: globalResponse, projectResponse: false }, + ...(builtinResponse + ? [{ response: builtinResponse, projectResponse: false }] + : []), ...projectResponses.flatMap((result) => result.status === "fulfilled" ? [{ response: result.value, projectResponse: true }] @@ -122,7 +161,10 @@ export async function listSkills( continue; } - const key = `${source.global ? "global" : "project"}:${source.directory}`; + const key = + source.type === BUILTIN_SKILL_SOURCE_TYPE + ? `builtin:${source.name}` + : `${source.global ? "global" : "project"}:${source.directory}`; if (seen.has(key)) { continue; } @@ -158,7 +200,7 @@ export async function updateSkill( content: instructions, }); - if (!isSkillSource(response.source)) { + if (!isFilesystemSkillSource(response.source)) { throw new Error(`Unexpected source type returned: ${response.source.type}`); } @@ -191,5 +233,5 @@ export async function importSkills( global: true, }); - return response.sources.filter(isSkillSource).map(toSkillInfo); + return response.sources.filter(isFilesystemSkillSource).map(toSkillInfo); } diff --git a/ui/goose2/src/features/skills/hooks/useSkillImportExport.ts b/ui/goose2/src/features/skills/hooks/useSkillImportExport.ts index 1bd2bd93..cc24968a 100644 --- a/ui/goose2/src/features/skills/hooks/useSkillImportExport.ts +++ b/ui/goose2/src/features/skills/hooks/useSkillImportExport.ts @@ -8,6 +8,10 @@ export function useSkillImportExport(onAfterImport: () => Promise) { const { t } = useTranslation(["skills"]); const handleExport = async (skill: SkillInfo) => { + if (skill.sourceKind === "builtin") { + return; + } + try { const result = await exportSkill(skill.path); downloadExport(result.json, result.filename); diff --git a/ui/goose2/src/features/skills/lib/skillsHelpers.ts b/ui/goose2/src/features/skills/lib/skillsHelpers.ts index 8bafa393..89a5627c 100644 --- a/ui/goose2/src/features/skills/lib/skillsHelpers.ts +++ b/ui/goose2/src/features/skills/lib/skillsHelpers.ts @@ -1,6 +1,6 @@ import type { SkillInfo } from "../api/skills"; -export type SkillsFilter = "all" | "global" | `project:${string}`; +export type SkillsFilter = "all" | "global" | "builtin" | `project:${string}`; export interface SkillsSection { id: string; @@ -77,9 +77,11 @@ export function filterSkills( ? true : filters.activeFilter === "global" ? skill.sourceKind === "global" - : skill.projectLinks.some( - (project) => `project:${project.id}` === filters.activeFilter, - ); + : filters.activeFilter === "builtin" + ? skill.sourceKind === "builtin" + : skill.projectLinks.some( + (project) => `project:${project.id}` === filters.activeFilter, + ); return matchesSearch && matchesFilter; }); @@ -89,7 +91,11 @@ export function groupSkills( filteredSkills: SkillInfo[], activeFilter: SkillsFilter, projectFilters: { id: string; name: string }[], - labels: { personalTitle: string; projectsFallback: string }, + labels: { + personalTitle: string; + builtinTitle: string; + projectsFallback: string; + }, ): SkillsSection[] { if (activeFilter === "global") { return [ @@ -101,6 +107,16 @@ export function groupSkills( ]; } + if (activeFilter === "builtin") { + return [ + { + id: "builtin", + title: labels.builtinTitle, + skills: [...filteredSkills].sort(compareSkillsByName), + }, + ]; + } + if (activeFilter.startsWith("project:")) { const projectId = activeFilter.slice("project:".length); const projectName = @@ -120,6 +136,10 @@ export function groupSkills( .filter((skill) => skill.sourceKind === "global") .sort(compareSkillsByName); + const builtinSkills = filteredSkills + .filter((skill) => skill.sourceKind === "builtin") + .sort(compareSkillsByName); + const projectSections = projectFilters .map((project) => ({ id: `project:${project.id}`, @@ -142,6 +162,15 @@ export function groupSkills( }, ] : []), + ...(builtinSkills.length > 0 + ? [ + { + id: "builtin", + title: labels.builtinTitle, + skills: builtinSkills, + }, + ] + : []), ...projectSections, ]; } diff --git a/ui/goose2/src/features/skills/ui/SkillDetailPage.tsx b/ui/goose2/src/features/skills/ui/SkillDetailPage.tsx index f657b574..bb048f1d 100644 --- a/ui/goose2/src/features/skills/ui/SkillDetailPage.tsx +++ b/ui/goose2/src/features/skills/ui/SkillDetailPage.tsx @@ -97,6 +97,61 @@ export function SkillDetailPage({ const editLabel = t("common:actions.edit"); const revealLabel = t("view.reveal"); const moreLabel = t("view.more"); + const isBuiltin = skill.sourceKind === "builtin"; + const actions = ( + <> + {onStartChat ? ( + } + tooltipSide="top" + onClick={() => onStartChat(skill)} + /> + ) : null} + {!isBuiltin ? ( + <> + } + tooltipSide="top" + onClick={() => onEdit(skill)} + /> + } + tooltipSide="top" + onClick={() => onReveal(skill)} + /> + + + + + + onShare(skill)}> + + {t("view.share")} + + onDelete(skill)} + > + + {t("common:actions.delete")} + + + + + ) : null} + + ); return ( @@ -117,111 +172,71 @@ export function SkillDetailPage({ description={skill.description} actionsPlacement="below" descriptionClassName="max-w-3xl leading-relaxed" - actions={ - <> - {onStartChat ? ( - } - tooltipSide="top" - onClick={() => onStartChat(skill)} - /> - ) : null} - } - tooltipSide="top" - onClick={() => onEdit(skill)} - /> - } - tooltipSide="top" - onClick={() => onReveal(skill)} - /> - - - - - - onShare(skill)}> - - {t("view.share")} - - onDelete(skill)} - > - - {t("common:actions.delete")} - - - - - } + actions={actions} actionsClassName="gap-2" /> - -
- - {sourceLabels.map((label) => ( -

{label}

- ))} - - - {skill.projectLinks.length > 0 ? ( - - {skill.projectLinks.map((project) => ( -
-

{project.name}

-

- {project.workingDir} -

-
- ))} - - ) : null} - - - {skill.fileLocation} - -
- - } - > + {isBuiltin ? (
{skill.instructions || " "}
- + ) : ( + +
+ + {sourceLabels.map((label) => ( +

{label}

+ ))} + + + {skill.projectLinks.length > 0 ? ( + + {skill.projectLinks.map((project) => ( +
+

{project.name}

+

+ {project.workingDir} +

+
+ ))} + + ) : null} + + + {skill.fileLocation} + +
+ + } + > +
+ + + {skill.instructions || " "} + +
+ + )} ); } diff --git a/ui/goose2/src/features/skills/ui/SkillsToolbar.tsx b/ui/goose2/src/features/skills/ui/SkillsToolbar.tsx index a0077761..819dee0a 100644 --- a/ui/goose2/src/features/skills/ui/SkillsToolbar.tsx +++ b/ui/goose2/src/features/skills/ui/SkillsToolbar.tsx @@ -10,6 +10,7 @@ interface SkillsToolbarProps { onSearchChange: (value: string) => void; activeFilter: SkillsFilter; onActiveFilterChange: (filter: SkillsFilter) => void; + hasBuiltinSkills: boolean; projectFilters: { id: string; name: string }[]; dropHandlers?: React.HTMLAttributes; isDragOver?: boolean; @@ -41,6 +42,7 @@ export function SkillsToolbar({ onSearchChange, activeFilter, onActiveFilterChange, + hasBuiltinSkills, projectFilters, dropHandlers, isDragOver, @@ -74,6 +76,14 @@ export function SkillsToolbar({ > {t("view.filtersGlobal")} + {hasBuiltinSkills ? ( + onActiveFilterChange("builtin")} + > + {t("view.filtersBuiltin")} + + ) : null} {projectFilters.map((project) => { const filterValue = `project:${project.id}` as const; return ( diff --git a/ui/goose2/src/features/skills/ui/SkillsView.tsx b/ui/goose2/src/features/skills/ui/SkillsView.tsx index 3fe51aae..846d5910 100644 --- a/ui/goose2/src/features/skills/ui/SkillsView.tsx +++ b/ui/goose2/src/features/skills/ui/SkillsView.tsx @@ -78,6 +78,10 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { }, [loadSkills]); const projectFilters = useMemo(() => uniqueProjectFilters(skills), [skills]); + const hasBuiltinSkills = useMemo( + () => skills.some((skill) => skill.sourceKind === "builtin"), + [skills], + ); useEffect(() => { if (!activeFilter.startsWith("project:")) { @@ -90,6 +94,12 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { } }, [activeFilter, projectFilters]); + useEffect(() => { + if (activeFilter === "builtin" && !hasBuiltinSkills) { + setActiveFilter("all"); + } + }, [activeFilter, hasBuiltinSkills]); + const filteredSkills = useMemo( () => filterSkills(skills, { search, activeFilter }), [skills, search, activeFilter], @@ -99,6 +109,7 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { () => groupSkills(filteredSkills, activeFilter, projectFilters, { personalTitle: t("view.filtersGlobal"), + builtinTitle: t("view.filtersBuiltin"), projectsFallback: t("view.projects"), }), [filteredSkills, activeFilter, projectFilters, t], @@ -117,11 +128,18 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { skills.find((skill) => skill.id === activeSkillId) ?? null; const handleDelete = (skill: SkillInfo) => { + if (skill.sourceKind === "builtin") { + return; + } setDeletingSkill(skill); }; const handleConfirmDeleteSkill = async () => { if (!deletingSkill) return; + if (deletingSkill.sourceKind === "builtin") { + setDeletingSkill(null); + return; + } try { await deleteSkill(deletingSkill.path); await loadSkills(); @@ -136,6 +154,9 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { }; const handleEdit = (skill: SkillInfo) => { + if (skill.sourceKind === "builtin") { + return; + } setEditingSkill({ name: skill.name, description: skill.description, @@ -147,6 +168,9 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { }; const handleReveal = useCallback((skill: SkillInfo) => { + if (skill.sourceKind === "builtin") { + return; + } void revealInFileManager(skill.path); }, []); @@ -193,6 +217,16 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { handleExport, } = useSkillImportExport(refreshSkills); + const handleShare = useCallback( + (skill: SkillInfo) => { + if (skill.sourceKind === "builtin") { + return; + } + void handleExport(skill); + }, + [handleExport], + ); + const handleSelectSkill = (skill: SkillInfo) => { setActiveSkillId(skill.id); }; @@ -217,7 +251,7 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { onBack={() => setActiveSkillId(null)} onEdit={handleEdit} onReveal={handleReveal} - onShare={handleExport} + onShare={handleShare} onStartChat={onStartChatWithSkill ? handleStartChat : undefined} onDelete={handleDelete} /> @@ -261,6 +295,7 @@ export function SkillsView({ onStartChatWithSkill }: SkillsViewProps) { onSearchChange={setSearch} activeFilter={activeFilter} onActiveFilterChange={setActiveFilter} + hasBuiltinSkills={hasBuiltinSkills} projectFilters={projectFilters} dropHandlers={dropHandlers} isDragOver={isDragOver} diff --git a/ui/goose2/src/features/skills/ui/__tests__/SkillsView.test.tsx b/ui/goose2/src/features/skills/ui/__tests__/SkillsView.test.tsx index 60b33eed..5be538aa 100644 --- a/ui/goose2/src/features/skills/ui/__tests__/SkillsView.test.tsx +++ b/ui/goose2/src/features/skills/ui/__tests__/SkillsView.test.tsx @@ -60,6 +60,18 @@ const mockSkills: SkillInfo[] = [ }, ]; +const builtinSkill: SkillInfo = { + id: "builtin:goose-doc-guide", + name: "goose-doc-guide", + description: "Reference Goose documentation", + instructions: "Fetch Goose docs before answering.", + path: "builtin://skills/goose-doc-guide", + fileLocation: "builtin://skills/goose-doc-guide", + sourceKind: "builtin" as const, + sourceLabel: "Built in", + projectLinks: [], +}; + vi.mock("../../api/skills", () => ({ listSkills: vi.fn().mockResolvedValue([]), createSkill: vi.fn().mockResolvedValue(undefined), @@ -87,12 +99,13 @@ vi.mock("@/features/projects/stores/projectStore", () => ({ ) => selector({ projects: mockProjects }), })); -const { listSkills, deleteSkill, updateSkill } = (await import( +const { listSkills, deleteSkill, updateSkill, exportSkill } = (await import( "../../api/skills" )) as unknown as { listSkills: ReturnType; deleteSkill: ReturnType; updateSkill: ReturnType; + exportSkill: ReturnType; }; beforeEach(() => { @@ -377,6 +390,31 @@ describe("SkillsView", () => { expect(screen.getByText("test-writer")).toBeInTheDocument(); }); + it("groups and filters built-in skills separately", async () => { + listSkills.mockResolvedValue([...mockSkills, builtinSkill]); + const user = userEvent.setup(); + + render(); + await screen.findByText("goose-doc-guide"); + + expect( + screen.getByRole("button", { name: "Built in" }), + ).toBeInTheDocument(); + const sectionButtons = screen + .getAllByRole("button") + .filter((button) => /\d skill/.test(button.textContent ?? "")); + expect(sectionButtons.map((button) => button.textContent)).toEqual([ + expect.stringContaining("Personal"), + expect.stringContaining("Built in"), + expect.stringContaining("alpha"), + ]); + + await user.click(screen.getByRole("button", { name: "Built in" })); + + expect(screen.getByText("goose-doc-guide")).toBeInTheDocument(); + expect(screen.queryByText("code-review")).not.toBeInTheDocument(); + expect(screen.queryByText("test-writer")).not.toBeInTheDocument(); + }); it("shows a delete confirmation from the detail panel", async () => { listSkills.mockResolvedValue(mockSkills); const user = userEvent.setup(); @@ -403,6 +441,46 @@ describe("SkillsView", () => { }); }); + it("shows built-in details without filesystem actions and still starts chat", async () => { + listSkills.mockResolvedValue([...mockSkills, builtinSkill]); + const onStartChatWithSkill = vi.fn(); + const user = userEvent.setup(); + + render(); + await screen.findByText("goose-doc-guide"); + + await user.click( + screen.getByRole("button", { name: "Open goose-doc-guide details" }), + ); + + expect( + screen.getByText("Fetch Goose docs before answering."), + ).toBeInTheDocument(); + expect(screen.queryByText("Location")).not.toBeInTheDocument(); + expect( + screen.queryByText("builtin://skills/goose-doc-guide"), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Edit" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "Show in folder" }), + ).not.toBeInTheDocument(); + expect( + screen.queryByRole("button", { name: "More" }), + ).not.toBeInTheDocument(); + + await user.click(screen.getByRole("button", { name: "Start chat" })); + + expect(onStartChatWithSkill).toHaveBeenCalledWith( + expect.objectContaining({ name: "goose-doc-guide" }), + null, + ); + expect(updateSkill).not.toHaveBeenCalled(); + expect(deleteSkill).not.toHaveBeenCalled(); + expect(exportSkill).not.toHaveBeenCalled(); + }); + it("passes saved project working directories into listSkills", async () => { mockProjects = [ { diff --git a/ui/goose2/src/shared/i18n/locales/en/skills.json b/ui/goose2/src/shared/i18n/locales/en/skills.json index a8bb948f..55a65b59 100644 --- a/ui/goose2/src/shared/i18n/locales/en/skills.json +++ b/ui/goose2/src/shared/i18n/locales/en/skills.json @@ -30,6 +30,7 @@ "exportedTo": "Exported to {{filename}}", "filePath": "File path", "filtersAllSources": "All", + "filtersBuiltin": "Built in", "filtersGlobal": "Personal", "importError": "Failed to import skill", "importSuccess": "Skill imported", diff --git a/ui/goose2/src/shared/i18n/locales/es/skills.json b/ui/goose2/src/shared/i18n/locales/es/skills.json index 95ca63f2..354cf9fe 100644 --- a/ui/goose2/src/shared/i18n/locales/es/skills.json +++ b/ui/goose2/src/shared/i18n/locales/es/skills.json @@ -30,6 +30,7 @@ "exportedTo": "Exportado a {{filename}}", "filePath": "Ruta del archivo", "filtersAllSources": "Todas", + "filtersBuiltin": "Integradas", "filtersGlobal": "Personal", "importError": "Error al importar la skill", "importSuccess": "Skill importada", diff --git a/ui/sdk/src/generated/types.gen.ts b/ui/sdk/src/generated/types.gen.ts index 5fd9f3cd..f6816093 100644 --- a/ui/sdk/src/generated/types.gen.ts +++ b/ui/sdk/src/generated/types.gen.ts @@ -764,8 +764,9 @@ export type CreateSourceResponse = { }; /** - * 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. */ export type SourceEntry = { type: SourceType; @@ -774,7 +775,8 @@ export type SourceEntry = { 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/` paths. */ directory: string; /** @@ -792,9 +794,10 @@ export type SourceEntry = { /** * 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. */ export type ListSourcesRequest = { type?: SourceType | null; diff --git a/ui/sdk/src/generated/zod.gen.ts b/ui/sdk/src/generated/zod.gen.ts index c0549114..7ad4e56b 100644 --- a/ui/sdk/src/generated/zod.gen.ts +++ b/ui/sdk/src/generated/zod.gen.ts @@ -774,8 +774,9 @@ export const zCreateSourceRequest = z.object({ }); /** - * 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. */ export const zSourceEntry = z.object({ type: zSourceType, @@ -794,9 +795,10 @@ export const zCreateSourceResponse = z.object({ /** * 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. */ export const zListSourcesRequest = z.object({ type: z.union([