feat: add client side skills (#1320)

Co-authored-by: Letta Code <noreply@letta.com>
This commit is contained in:
Sarah Wooders
2026-03-10 13:18:14 -07:00
committed by GitHub
parent 87312720d5
commit e82a2d33f8
25 changed files with 377 additions and 151 deletions

View File

@@ -0,0 +1,88 @@
import { describe, expect, test } from "bun:test";
import type { Skill, SkillDiscoveryResult } from "../../agent/skills";
const baseSkill: Skill = {
id: "base",
name: "Base",
description: "Base skill",
path: "/tmp/base/SKILL.md",
source: "project",
};
describe("buildClientSkillsPayload", () => {
test("returns deterministically sorted client skills and path map", async () => {
const { buildClientSkillsPayload } = await import(
"../../agent/clientSkills"
);
const discoverSkillsFn = async (): Promise<SkillDiscoveryResult> => ({
skills: [
{
...baseSkill,
id: "z-skill",
description: "z",
path: "/tmp/z/SKILL.md",
source: "project",
},
{
...baseSkill,
id: "a-skill",
description: "a",
path: "/tmp/a/SKILL.md",
source: "bundled",
},
],
errors: [],
});
const result = await buildClientSkillsPayload({
agentId: "agent-1",
skillsDirectory: "/tmp/.skills",
skillSources: ["project", "bundled"],
discoverSkillsFn,
});
expect(result.clientSkills).toEqual([
{
name: "a-skill",
description: "a",
location: "/tmp/a/SKILL.md",
},
{
name: "z-skill",
description: "z",
location: "/tmp/z/SKILL.md",
},
]);
expect(result.skillPathById).toEqual({
"a-skill": "/tmp/a/SKILL.md",
"z-skill": "/tmp/z/SKILL.md",
});
expect(result.errors).toEqual([]);
});
test("fails open with empty client_skills when discovery throws", async () => {
const { buildClientSkillsPayload } = await import(
"../../agent/clientSkills"
);
const discoverSkillsFn = async (): Promise<SkillDiscoveryResult> => {
throw new Error("boom");
};
const logs: string[] = [];
const result = await buildClientSkillsPayload({
skillsDirectory: "/tmp/.skills",
discoverSkillsFn,
logger: (m) => logs.push(m),
});
expect(result.clientSkills).toEqual([]);
expect(result.skillPathById).toEqual({});
expect(result.errors).toHaveLength(1);
expect(result.errors[0]?.path).toBe("/tmp/.skills");
expect(
logs.some((m) => m.includes("Failed to build client_skills payload")),
).toBe(true);
});
});

View File

@@ -0,0 +1,35 @@
import { describe, expect, test } from "bun:test";
import { buildConversationMessagesCreateRequestBody } from "../../agent/message";
describe("buildConversationMessagesCreateRequestBody client_skills", () => {
test("includes client_skills alongside client_tools", () => {
const body = buildConversationMessagesCreateRequestBody(
"default",
[{ type: "message", role: "user", content: "hello" }],
{ agentId: "agent-1", streamTokens: true, background: true },
[
{
name: "ShellCommand",
description: "Run shell command",
parameters: { type: "object", properties: {} },
},
],
[
{
name: "debugging",
description: "Debugging checklist",
location: "/tmp/.skills/debugging/SKILL.md",
},
],
);
expect(body.client_tools).toHaveLength(1);
expect(body.client_skills).toEqual([
{
name: "debugging",
description: "Debugging checklist",
location: "/tmp/.skills/debugging/SKILL.md",
},
]);
});
});

View File

@@ -107,5 +107,24 @@ describe.skipIf(process.platform === "win32")(
result.errors.some((error) => error.path.includes("broken-link")),
).toBe(true);
});
test("returns discovered skills in deterministic sorted order", async () => {
mkdirSync(projectSkillsDir, { recursive: true });
writeSkill(join(projectSkillsDir, "z-skill"), "Z Skill");
writeSkill(join(projectSkillsDir, "a-skill"), "A Skill");
writeSkill(join(projectSkillsDir, "m-skill"), "M Skill");
const result = await discoverSkills(projectSkillsDir, undefined, {
skipBundled: true,
sources: ["project"],
});
expect(result.errors).toHaveLength(0);
expect(result.skills.map((skill) => skill.id)).toEqual([
"a-skill",
"m-skill",
"z-skill",
]);
});
},
);

View File

@@ -66,4 +66,53 @@ describe("Skills formatting (system reminder)", () => {
expect(result).toContain("project-skill (project)");
expect(result).toContain("global-skill (global)");
});
test("sorts skills deterministically before formatting", () => {
const skills: Skill[] = [
{
id: "z-skill",
name: "Z Skill",
description: "Last by id",
path: "/test/.skills/z-skill/SKILL.md",
source: "project",
},
{
id: "a-skill",
name: "A Skill",
description: "First by id",
path: "/test/.skills/a-skill/SKILL.md",
source: "project",
},
{
id: "same-id",
name: "Same Id Global",
description: "Global variant",
path: "/global/.skills/same-id/SKILL.md",
source: "global",
},
{
id: "same-id",
name: "Same Id Project",
description: "Project variant",
path: "/project/.skills/same-id/SKILL.md",
source: "project",
},
];
const result = formatSkillsAsSystemReminder(skills);
const aSkillIndex = result.indexOf("- a-skill (project): First by id");
const sameIdGlobalIndex = result.indexOf(
"- same-id (global): Global variant",
);
const sameIdProjectIndex = result.indexOf(
"- same-id (project): Project variant",
);
const zSkillIndex = result.indexOf("- z-skill (project): Last by id");
expect(aSkillIndex).toBeGreaterThan(-1);
expect(sameIdGlobalIndex).toBeGreaterThan(aSkillIndex);
expect(sameIdProjectIndex).toBeGreaterThan(sameIdGlobalIndex);
expect(zSkillIndex).toBeGreaterThan(sameIdProjectIndex);
});
});

View File

@@ -102,7 +102,6 @@ describe("accumulator usage statistics", () => {
);
expect(tracker.pendingCompaction).toBe(true);
expect(tracker.pendingSkillsReinject).toBe(true);
expect(tracker.pendingReflectionTrigger).toBe(true);
});
@@ -126,7 +125,6 @@ describe("accumulator usage statistics", () => {
);
expect(tracker.pendingCompaction).toBe(true);
expect(tracker.pendingSkillsReinject).toBe(true);
expect(tracker.pendingReflectionTrigger).toBe(true);
});

View File

@@ -12,7 +12,6 @@ describe("contextTracker", () => {
{ timestamp: 1, tokens: 111, turnId: 1, compacted: true },
];
tracker.pendingCompaction = true;
tracker.pendingSkillsReinject = true;
tracker.pendingReflectionTrigger = true;
tracker.currentTurnId = 9;
@@ -21,7 +20,6 @@ describe("contextTracker", () => {
expect(tracker.lastContextTokens).toBe(0);
expect(tracker.contextTokensHistory).toEqual([]);
expect(tracker.pendingCompaction).toBe(false);
expect(tracker.pendingSkillsReinject).toBe(false);
expect(tracker.pendingReflectionTrigger).toBe(false);
expect(tracker.currentTurnId).toBe(9);
});

View File

@@ -0,0 +1,19 @@
import { describe, expect, test } from "bun:test";
import { readFileSync } from "node:fs";
import { fileURLToPath } from "node:url";
describe("headless client skills wiring", () => {
test("pre-load-skills resolves skill paths from client-skills helper", () => {
const headlessPath = fileURLToPath(
new URL("../../headless.ts", import.meta.url),
);
const source = readFileSync(headlessPath, "utf-8");
expect(source).toContain("buildClientSkillsPayload({");
expect(source).toContain(
"const { skillPathById } = await buildClientSkillsPayload",
);
expect(source).toContain("const skillPath = skillPathById[skillId]");
expect(source).not.toContain("sharedReminderState.skillPathById");
});
});

View File

@@ -1,54 +0,0 @@
import { describe, expect, test } from "bun:test";
import type { SharedReminderContext } from "../../reminders/engine";
import { sharedReminderProviders } from "../../reminders/engine";
import { createSharedReminderState } from "../../reminders/state";
function buildContext(): SharedReminderContext {
return {
mode: "interactive",
agent: {
id: "agent-1",
name: "Agent 1",
description: null,
lastRunAt: null,
},
state: createSharedReminderState(),
sessionContextReminderEnabled: true,
reflectionSettings: {
trigger: "off",
behavior: "reminder",
stepCount: 25,
},
skillSources: ["bundled"],
resolvePlanModeReminder: () => "",
};
}
describe("shared skills reminder", () => {
test("recovers from discovery failure and reinjects after next successful discovery", async () => {
const provider = sharedReminderProviders.skills;
const context = buildContext();
const mutableProcess = process as typeof process & { cwd: () => string };
const originalCwd = mutableProcess.cwd;
try {
mutableProcess.cwd = () => {
throw new Error("cwd unavailable for test");
};
const first = await provider(context);
expect(first).toBeNull();
expect(context.state.hasInjectedSkillsReminder).toBe(true);
expect(context.state.cachedSkillsReminder).toBe("");
} finally {
mutableProcess.cwd = originalCwd;
}
const second = await provider(context);
expect(second).not.toBeNull();
expect(context.state.pendingSkillsReinject).toBe(false);
if (second) {
expect(second).toContain("<system-reminder>");
}
});
});