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
6 changes: 3 additions & 3 deletions crates/goose/src/skills/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ pub fn global_skills_dir() -> Option<PathBuf> {
}

/// Canonical writable location for project-scoped skills:
/// `<project>/.goose/skills`.
/// `<project>/.agents/skills`.
pub fn project_skills_dir(project_dir: &Path) -> PathBuf {
project_dir.join(".goose").join("skills")
project_dir.join(".agents").join("skills")
}

pub(crate) fn skills_dir_global_or_err() -> Result<PathBuf, Error> {
Expand Down Expand Up @@ -196,9 +196,9 @@ pub fn all_skill_dirs(working_dir: Option<&Path>) -> Vec<(PathBuf, bool)> {
let mut dirs: Vec<(PathBuf, bool)> = Vec::new();

if let Some(wd) = working_dir {
dirs.push((wd.join(".agents").join("skills"), false));
dirs.push((wd.join(".goose").join("skills"), false));
dirs.push((wd.join(".claude").join("skills"), false));
dirs.push((wd.join(".agents").join("skills"), false));
}

let home = dirs::home_dir();
Expand Down
65 changes: 63 additions & 2 deletions crates/goose/src/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ mod tests {
)
.unwrap();

let portable_dir = project_a.join(".goose").join("skills").join("portable");
let portable_dir = project_a.join(".agents").join("skills").join("portable");
let (json, filename) =
export_source(SourceType::Skill, portable_dir.to_str().unwrap()).unwrap();
assert_eq!(filename, "portable.skill.json");
Expand Down Expand Up @@ -538,7 +538,7 @@ mod tests {
)
.unwrap();

let skill_dir = tmp.path().join(".goose").join("skills").join("my-dir");
let skill_dir = tmp.path().join(".agents").join("skills").join("my-dir");
let updated = update_source(
SourceType::Skill,
skill_dir.to_str().unwrap(),
Expand All @@ -551,6 +551,67 @@ mod tests {
assert_eq!(updated.name, "my-dir");
}

#[test]
fn list_sources_reads_project_agents_skills() {
let tmp = TempDir::new().unwrap();
let skill_dir = tmp.path().join(".agents").join("skills").join("test-skill");
std::fs::create_dir_all(&skill_dir).unwrap();
std::fs::write(
skill_dir.join("SKILL.md"),
build_skill_md("test-skill", "from agents", "Body"),
)
.unwrap();

let listed =
list_sources(Some(SourceType::Skill), Some(tmp.path().to_str().unwrap())).unwrap();
let skill = listed
.iter()
.find(|source| source.name == "test-skill" && !source.global)
.unwrap();
assert!(skill.directory.contains(".agents/skills"));
assert_eq!(skill.description, "from agents");
}

#[test]
fn project_sources_prefer_agents_directory_over_legacy_goose() {
let tmp = TempDir::new().unwrap();
let agents_skill_dir = tmp
.path()
.join(".agents")
.join("skills")
.join("shared-skill");
let legacy_skill_dir = tmp
.path()
.join(".goose")
.join("skills")
.join("shared-skill");
std::fs::create_dir_all(&agents_skill_dir).unwrap();
std::fs::create_dir_all(&legacy_skill_dir).unwrap();
std::fs::write(
agents_skill_dir.join("SKILL.md"),
build_skill_md("shared-skill", "preferred", "Agents"),
)
.unwrap();
std::fs::write(
legacy_skill_dir.join("SKILL.md"),
build_skill_md("shared-skill", "legacy", "Goose"),
)
.unwrap();

let listed =
list_sources(Some(SourceType::Skill), Some(tmp.path().to_str().unwrap())).unwrap();
let matching: Vec<_> = listed
.iter()
.filter(|source| source.name == "shared-skill" && !source.global)
.collect();
assert_eq!(matching.len(), 1);
assert!(matching[0].directory.contains(".agents/skills"));
assert_eq!(matching[0].description, "preferred");

let exported = export_source(SourceType::Skill, matching[0].directory.as_str()).unwrap();
assert!(exported.0.contains("preferred"));
}

#[test]
fn update_rejects_path_traversal() {
let tmp = TempDir::new().unwrap();
Expand Down
5 changes: 5 additions & 0 deletions ui/goose2/scripts/check-file-sizes.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@ const EXCEPTIONS = {
justification:
"Bubble rendering still owns assistant identity, grouped tool output, attachments, and the inline actions tray pending a later extraction pass.",
},
"src/features/skills/ui/SkillsView.tsx": {
limit: 620,
justification:
"SkillsView currently centralizes list/detail state, project-aware skill hydration, category/source filtering, import/export flows, and detail-page action wiring pending a later decomposition.",
},
"src/features/chat/ui/__tests__/ChatInput.test.tsx": {
limit: 570,
justification:
Expand Down
59 changes: 30 additions & 29 deletions ui/goose2/src/features/agents/ui/PersonaDetails.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import {
AvatarFallback,
AvatarImage,
} from "@/shared/ui/avatar";
import { DetailField } from "@/shared/ui/detail-field";
import { Badge } from "@/shared/ui/badge";
import { MessageResponse } from "@/shared/ui/ai-elements/message";
import { useAvatarSrc } from "@/shared/hooks/useAvatarSrc";
Expand Down Expand Up @@ -46,14 +47,12 @@ export function PersonaDetails({
</AvatarFallback>
</AvatarRoot>
<div className="min-w-0 flex-1 space-y-2">
<div className="space-y-1">
<p className="text-[11px] font-medium uppercase tracking-[0.12em] text-muted-foreground">
{t("editor.displayName")}
</p>
<h2 className="text-base font-semibold tracking-tight">
{displayName}
</h2>
</div>
<DetailField
label={t("editor.displayName")}
contentClassName="text-base font-semibold tracking-tight"
>
{displayName}
</DetailField>
<div className="flex flex-wrap items-center gap-2">
{personaSource === "builtin" ? (
<Badge variant="secondary">
Expand All @@ -69,33 +68,35 @@ export function PersonaDetails({
</section>

<section className="grid gap-3 sm:grid-cols-2">
<div className="space-y-2 rounded-xl border border-border bg-background p-4">
<p className="text-[11px] font-medium uppercase tracking-[0.12em] text-muted-foreground">
{t("editor.provider")}
</p>
<p className="text-sm font-medium text-foreground">
<div className="rounded-xl border border-border bg-background p-4">
<DetailField
label={t("editor.provider")}
contentClassName="font-medium"
>
{providerLabel}
</p>
</DetailField>
</div>
<div className="space-y-2 rounded-xl border border-border bg-background p-4">
<p className="text-[11px] font-medium uppercase tracking-[0.12em] text-muted-foreground">
{t("editor.model")}
</p>
<p className="text-sm font-medium text-foreground">{modelLabel}</p>
<div className="rounded-xl border border-border bg-background p-4">
<DetailField
label={t("editor.model")}
contentClassName="font-medium"
>
{modelLabel}
</DetailField>
</div>
</section>

<section className="space-y-2 rounded-xl border border-border bg-background p-4">
<div className="flex items-center justify-between gap-3">
<p className="text-[11px] font-medium uppercase tracking-[0.12em] text-muted-foreground">
{t("editor.systemPrompt")}
</p>
<span className="text-[10px] text-muted-foreground">
{t("common:labels.characterCount", {
count: systemPrompt.length,
})}
</span>
</div>
<DetailField
label={t("editor.systemPrompt")}
meta={
<span className="text-[10px] text-muted-foreground">
{t("common:labels.characterCount", {
count: systemPrompt.length,
})}
</span>
}
/>
<div className="rounded-lg border border-border bg-muted/20 px-4 py-3">
<MessageResponse className="min-w-0 text-sm leading-6">
{systemPrompt}
Expand Down
159 changes: 159 additions & 0 deletions ui/goose2/src/features/skills/api/skills.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
import { beforeEach, describe, expect, it, vi } from "vitest";

const mockGooseSourcesList = vi.fn();

vi.mock("@/shared/api/acpConnection", () => ({
getClient: async () => ({
goose: {
GooseSourcesList: (...args: unknown[]) => mockGooseSourcesList(...args),
},
}),
}));

describe("listSkills", () => {
beforeEach(() => {
vi.clearAllMocks();
vi.resetModules();
});

it("aggregates project skill listings and recognizes .agents skill paths", async () => {
mockGooseSourcesList
.mockResolvedValueOnce({
sources: [
{
type: "skill",
name: "code-review",
description: "Reviews code",
content: "Review carefully",
directory: "/Users/test/.agents/skills/code-review",
global: true,
},
],
})
.mockResolvedValueOnce({
sources: [
{
type: "skill",
name: "code-review",
description: "Reviews code",
content: "Review carefully",
directory: "/Users/test/.agents/skills/code-review",
global: true,
},
{
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", "/tmp/alpha"]);

expect(mockGooseSourcesList).toHaveBeenNthCalledWith(1, {
type: "skill",
});
expect(mockGooseSourcesList).toHaveBeenNthCalledWith(2, {
type: "skill",
projectDir: "/tmp/alpha",
});
expect(skills.filter((skill) => skill.name === "code-review")).toHaveLength(
1,
);
expect(skills).toEqual(
expect.arrayContaining([
expect.objectContaining({
name: "test-writer",
sourceKind: "project",
sourceLabel: "alpha",
projectLinks: [
{
id: "/tmp/alpha",
name: "alpha",
workingDir: "/tmp/alpha",
},
],
}),
]),
);
});

it("recognizes legacy .goose project skill paths", async () => {
mockGooseSourcesList
.mockResolvedValueOnce({ sources: [] })
.mockResolvedValueOnce({
sources: [
{
type: "skill",
name: "legacy-writer",
description: "Legacy project skill",
content: "Legacy instructions",
directory: "/tmp/beta/.goose/skills/legacy-writer",
global: false,
},
],
});

const { listSkills } = await import("./skills");
const skills = await listSkills(["/tmp/beta"]);

expect(skills).toEqual(
expect.arrayContaining([
expect.objectContaining({
name: "legacy-writer",
sourceKind: "project",
sourceLabel: "beta",
projectLinks: [
{
id: "/tmp/beta",
name: "beta",
workingDir: "/tmp/beta",
},
],
}),
]),
);
});

it("keeps available skills when a project 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("permission denied"))
.mockResolvedValueOnce({
sources: [
{
type: "skill",
name: "test-writer",
description: "Writes tests",
content: "Write tests",
directory: "/tmp/beta/.agents/skills/test-writer",
global: false,
},
],
});

const { listSkills } = await import("./skills");
const skills = await listSkills(["/tmp/alpha", "/tmp/beta"]);

expect(mockGooseSourcesList).toHaveBeenCalledTimes(3);
expect(skills.map((skill) => skill.name)).toEqual([
"code-review",
"test-writer",
]);
});
});
Loading
Loading