This commit is contained in:
Aiden Cline
2026-05-09 13:37:34 -05:00
parent 59795b7543
commit 32935ef3cf
8 changed files with 190 additions and 43 deletions

View File

@@ -2,6 +2,7 @@ import { DialogSelect, type DialogSelectOption } from "@tui/ui/dialog-select"
import { createResource, createMemo } from "solid-js"
import { useDialog } from "@tui/ui/dialog"
import { useSDK } from "@tui/context/sdk"
import path from "path"
export type DialogSkillProps = {
onSelect: (skill: string) => void
@@ -13,23 +14,36 @@ export function DialogSkill(props: DialogSkillProps) {
dialog.setSize("large")
const [skills] = createResource(async () => {
const result = await sdk.client.app.skills()
return result.data ?? []
const result = await sdk.client.app.skills({ include: "invalid" })
return Array.isArray(result.data) ? { skills: result.data, invalid: [] } : (result.data ?? { skills: [], invalid: [] })
})
const options = createMemo<DialogSelectOption<string>[]>(() => {
const list = skills() ?? []
const maxWidth = Math.max(0, ...list.map((s) => s.name.length))
return list.map((skill) => ({
title: skill.name.padEnd(maxWidth),
description: skill.description?.replace(/\s+/g, " ").trim(),
value: skill.name,
category: "Skills",
onSelect: () => {
props.onSelect(skill.name)
dialog.clear()
},
}))
const list = skills() ?? { skills: [], invalid: [] }
const maxWidth = Math.max(
0,
...list.skills.map((s) => s.name.length),
...list.invalid.map((s) => path.basename(path.dirname(s.path)).length),
)
return [
...list.skills.map((skill) => ({
title: skill.name.padEnd(maxWidth),
description: skill.description?.replace(/\s+/g, " ").trim(),
value: skill.name,
category: "Skills",
onSelect: () => {
props.onSelect(skill.name)
dialog.clear()
},
})),
...list.invalid.map((skill) => ({
title: path.basename(path.dirname(skill.path)).padEnd(maxWidth),
description: `${skill.reason}: ${skill.message}`,
value: skill.path,
category: "Invalid skills",
disabled: true,
})),
]
})
return <DialogSelect title="Skills" placeholder="Search skills..." options={options()} />

View File

@@ -23,6 +23,10 @@ export const VcsDiffQuery = Schema.Struct({
mode: Vcs.Mode,
})
export const SkillQuery = Schema.Struct({
include: Schema.optional(Schema.Literals(["invalid"])),
})
export class ApiVcsApplyError extends Schema.ErrorClass<ApiVcsApplyError>("VcsApplyError")(
{
name: Schema.Literal("VcsApplyError"),
@@ -143,7 +147,8 @@ export const InstanceApi = HttpApi.make("instance")
}),
),
HttpApiEndpoint.get("skill", InstancePaths.skill, {
success: described(Schema.Array(Skill.Info), "List of skills"),
query: SkillQuery,
success: described(Schema.Union([Schema.Array(Skill.Info), Skill.ListWithInvalid]), "List of skills"),
}).annotateMerge(
OpenApi.annotations({
identifier: "app.skills",

View File

@@ -9,7 +9,7 @@ import { Skill } from "@/skill"
import { Effect } from "effect"
import { HttpApiBuilder } from "effect/unstable/httpapi"
import { InstanceHttpApi } from "../api"
import { ApiVcsApplyError } from "../groups/instance"
import { ApiVcsApplyError, SkillQuery } from "../groups/instance"
import { markInstanceForDisposal } from "../lifecycle"
export const instanceHandlers = HttpApiBuilder.group(InstanceHttpApi, "instance", (handlers) =>
@@ -77,8 +77,15 @@ export const instanceHandlers = HttpApiBuilder.group(InstanceHttpApi, "instance"
return yield* agent.list()
})
const getSkill = Effect.fn("InstanceHttpApi.skill")(function* () {
return yield* skill.all()
const getSkill = Effect.fn("InstanceHttpApi.skill")(function* (ctx: { query: typeof SkillQuery.Type }) {
const skills = yield* skill.all()
if (ctx.query.include === "invalid") {
return {
skills,
invalid: yield* skill.invalid(),
}
}
return skills
})
const getLsp = Effect.fn("InstanceHttpApi.lsp")(function* () {

View File

@@ -33,6 +33,19 @@ export const Info = Schema.Struct({
}).pipe(withStatics((s) => ({ zod: zod(s) })))
export type Info = Schema.Schema.Type<typeof Info>
export const Invalid = Schema.Struct({
path: Schema.String,
reason: Schema.Literals(["parse", "frontmatter", "schema", "duplicate"]),
message: Schema.String,
}).pipe(withStatics((s) => ({ zod: zod(s) })))
export type Invalid = Schema.Schema.Type<typeof Invalid>
export const ListWithInvalid = Schema.Struct({
skills: Schema.Array(Info),
invalid: Schema.Array(Invalid),
}).pipe(withStatics((s) => ({ zod: zod(s) })))
export type ListWithInvalid = Schema.Schema.Type<typeof ListWithInvalid>
export const InvalidError = NamedError.create(
"SkillInvalidError",
z.object({
@@ -53,6 +66,7 @@ export const NameMismatchError = NamedError.create(
type State = {
skills: Record<string, Info>
invalid: Invalid[]
dirs: Set<string>
}
@@ -69,6 +83,7 @@ type ScanState = {
export interface Interface {
readonly get: (name: string) => Effect.Effect<Info | undefined>
readonly all: () => Effect.Effect<Info[]>
readonly invalid: () => Effect.Effect<Invalid[]>
readonly dirs: () => Effect.Effect<string[]>
readonly available: (agent?: Agent.Info) => Effect.Effect<Info[]>
}
@@ -80,9 +95,15 @@ const add = Effect.fnUntraced(function* (state: State, match: string, bus: Bus.I
}).pipe(
Effect.catch(
Effect.fnUntraced(function* (err) {
const message = ConfigMarkdown.FrontmatterError.isInstance(err)
const isFrontmatter = ConfigMarkdown.FrontmatterError.isInstance(err)
const message = isFrontmatter
? err.data.message
: `Failed to parse skill ${match}`
state.invalid.push({
path: match,
reason: isFrontmatter ? "frontmatter" : "parse",
message,
})
const { Session } = yield* Effect.promise(() => import("@/session/session"))
yield* bus.publish(Session.Event.Error, { error: new NamedError.Unknown({ message }).toObject() })
log.error("failed to load skill", { skill: match, err })
@@ -93,15 +114,50 @@ const add = Effect.fnUntraced(function* (state: State, match: string, bus: Bus.I
if (!md) return
const parsed = z.object({ name: z.string(), description: z.string().optional() }).safeParse(md.data)
if (!parsed.success) return
const parsed = z
.object({
name: z
.string()
.min(1, "Skill name is required")
.max(64, "Skill name must be at most 64 characters")
.regex(/^[a-z0-9-]+$/, "Skill name must contain only lowercase letters, numbers, and hyphens")
.regex(/^[^-].*[^-]$|^[^-]$/, "Skill name must not start or end with a hyphen")
.refine((value) => !value.includes("--"), "Skill name must not contain consecutive hyphens")
.refine((value) => !/(<[^>]*>)/.test(value), "Skill name must not contain XML tags")
.refine(
(value) => !value.includes("anthropic") && !value.includes("claude"),
'Skill name must not contain reserved words "anthropic" or "claude"',
)
.refine((value) => value === path.basename(path.dirname(match)), "Skill name must match parent directory name"),
description: z
.string({ error: "Skill description is required" })
.trim()
.min(1, "Skill description is required")
.max(1024, "Skill description must be at most 1024 characters")
.refine((value) => !/(<[^>]*>)/.test(value), "Skill description must not contain XML tags"),
})
.safeParse(md.data)
if (!parsed.success) {
state.invalid.push({
path: match,
reason: "schema",
message: parsed.error.issues.map((issue) => issue.message).join(", "),
})
return
}
if (state.skills[parsed.data.name]) {
state.invalid.push({
path: match,
reason: "duplicate",
message: `Duplicate skill name "${parsed.data.name}" already loaded from ${state.skills[parsed.data.name].location}`,
})
log.warn("duplicate skill name", {
name: parsed.data.name,
existing: state.skills[parsed.data.name].location,
duplicate: match,
})
return
}
state.dirs.add(path.dirname(match))
@@ -229,7 +285,7 @@ export const layer = Layer.effect(
)
const state = yield* InstanceState.make(
Effect.fn("Skill.state")(function* () {
const s: State = { skills: {}, dirs: new Set() }
const s: State = { skills: {}, invalid: [], dirs: new Set() }
yield* loadSkills(s, yield* InstanceState.get(discovered), bus)
return s
}),
@@ -245,6 +301,10 @@ export const layer = Layer.effect(
return Object.values(s.skills)
})
const invalid = Effect.fn("Skill.invalid")(function* () {
return (yield* InstanceState.get(state)).invalid
})
const dirs = Effect.fn("Skill.dirs")(function* () {
return (yield* InstanceState.get(discovered)).dirs
})
@@ -256,7 +316,7 @@ export const layer = Layer.effect(
return list.filter((skill) => Permission.evaluate("skill", skill.name, agent.permission).action !== "deny")
})
return Service.of({ get, all, dirs, available })
return Service.of({ get, all, invalid, dirs, available })
}),
)

View File

@@ -48,6 +48,7 @@ const it = testEffect(
Skill.Service.of({
get: (name) => Effect.succeed(skills.find((skill) => skill.name === name)),
all: () => Effect.succeed(skills),
invalid: () => Effect.succeed([]),
dirs: () => Effect.succeed([]),
available: () => Effect.succeed(skills),
}),

View File

@@ -163,7 +163,7 @@ Just some content without YAML frontmatter.
),
)
it.live("discovers skills without descriptions", () =>
it.live("tracks skills without descriptions as invalid", () =>
provideTmpdirInstance(
(dir) =>
Effect.gen(function* () {
@@ -183,10 +183,11 @@ Instructions here.
const skill = yield* Skill.Service
const list = yield* skill.all()
expect(list.length).toBe(1)
const item = list.find((x) => x.name === "manual-skill")
expect(item).toBeDefined()
expect(item!.description).toBeUndefined()
const invalid = yield* skill.invalid()
expect(list.length).toBe(0)
expect(invalid.length).toBe(1)
expect(invalid[0].reason).toBe("schema")
expect(invalid[0].message).toContain("Skill description is required")
expect(Skill.fmt(list, { verbose: false })).toBe("No skills are currently available.")
expect(Skill.fmt(list, { verbose: true })).toBe("No skills are currently available.")
}),
@@ -200,13 +201,13 @@ Instructions here.
Effect.gen(function* () {
yield* Effect.promise(() =>
Bun.write(
path.join(dir, ".claude", "skills", "claude-skill", "SKILL.md"),
path.join(dir, ".claude", "skills", "external-skill", "SKILL.md"),
`---
name: claude-skill
name: external-skill
description: A skill in the .claude/skills directory.
---
# Claude Skill
# External Skill
`,
),
)
@@ -214,9 +215,9 @@ description: A skill in the .claude/skills directory.
const skill = yield* Skill.Service
const list = yield* skill.all()
expect(list.length).toBe(1)
const item = list.find((x) => x.name === "claude-skill")
const item = list.find((x) => x.name === "external-skill")
expect(item).toBeDefined()
expect(item!.location).toContain(path.join(".claude", "skills", "claude-skill", "SKILL.md"))
expect(item!.location).toContain(path.join(".claude", "skills", "external-skill", "SKILL.md"))
}),
{ git: true },
),
@@ -332,13 +333,13 @@ This skill is loaded from the global home directory.
yield* Effect.promise(() =>
Promise.all([
Bun.write(
path.join(dir, ".claude", "skills", "claude-skill", "SKILL.md"),
path.join(dir, ".claude", "skills", "external-skill", "SKILL.md"),
`---
name: claude-skill
name: external-skill
description: A skill in the .claude/skills directory.
---
# Claude Skill
# External Skill
`,
),
Bun.write(
@@ -357,13 +358,55 @@ description: A skill in the .agents/skills directory.
const skill = yield* Skill.Service
const list = yield* skill.all()
expect(list.length).toBe(2)
expect(list.find((x) => x.name === "claude-skill")).toBeDefined()
expect(list.find((x) => x.name === "external-skill")).toBeDefined()
expect(list.find((x) => x.name === "agent-skill")).toBeDefined()
}),
{ git: true },
),
)
it.live("tracks duplicate skill names as invalid diagnostics", () =>
provideTmpdirInstance(
(dir) =>
Effect.gen(function* () {
yield* Effect.promise(() =>
Promise.all([
Bun.write(
path.join(dir, ".agents", "skills", "duplicate-skill", "SKILL.md"),
`---
name: duplicate-skill
description: A duplicate skill in the .agents directory.
---
# Duplicate Skill
`,
),
Bun.write(
path.join(dir, ".claude", "skills", "duplicate-skill", "SKILL.md"),
`---
name: duplicate-skill
description: A duplicate skill in the .claude directory.
---
# Duplicate Skill
`,
),
]),
)
const skill = yield* Skill.Service
const list = yield* skill.all()
const invalid = yield* skill.invalid()
expect(list.length).toBe(1)
expect(list[0].name).toBe("duplicate-skill")
expect(invalid.length).toBe(1)
expect(invalid[0].reason).toBe("duplicate")
expect(invalid[0].message).toContain("Duplicate skill name")
}),
{ git: true },
),
)
it.live("properly resolves directories that skills live in", () =>
provideTmpdirInstance(
(dir) =>

View File

@@ -407,6 +407,7 @@ export class App extends HeyApiClient {
parameters?: {
directory?: string
workspace?: string
include?: "invalid"
},
options?: Options<never, ThrowOnError>,
) {
@@ -417,6 +418,7 @@ export class App extends HeyApiClient {
args: [
{ in: "query", key: "directory" },
{ in: "query", key: "workspace" },
{ in: "query", key: "include" },
],
},
],

View File

@@ -4233,6 +4233,7 @@ export type AppSkillsData = {
query?: {
directory?: string
workspace?: string
include?: "invalid"
}
url: "/skill"
}
@@ -4241,12 +4242,26 @@ export type AppSkillsResponses = {
/**
* List of skills
*/
200: Array<{
name: string
description?: string
location: string
content: string
}>
200:
| Array<{
name: string
description?: string
location: string
content: string
}>
| {
skills: Array<{
name: string
description?: string
location: string
content: string
}>
invalid: Array<{
path: string
reason: "parse" | "frontmatter" | "schema" | "duplicate"
message: string
}>
}
}
export type AppSkillsResponse = AppSkillsResponses[keyof AppSkillsResponses]