wip

packages/opencode/src/cli/cmd/tui/component/dialog-skill.tsx

@@ -1,7 +1,14 @@
-import { DialogSelect, type DialogSelectOption } from "@tui/ui/dialog-select"
-import { createResource, createMemo } from "solid-js"
+import { createResource, createMemo, For } from "solid-js"
 import { useDialog } from "@tui/ui/dialog"
 import { useSDK } from "@tui/context/sdk"
+import { useTheme } from "@tui/context/theme"
+import { TextAttributes } from "@opentui/core"
+import { useKeyboard } from "@opentui/solid"
+import { createStore } from "solid-js/store"
+import { Locale } from "@/util/locale"
+import { getScrollAcceleration } from "../util/scroll"
+import { useTuiConfig } from "../context/tui-config"
+import path from "path"
 
 export type DialogSkillProps = {
   onSelect: (skill: string) => void
@@ -10,27 +17,226 @@ export type DialogSkillProps = {
 export function DialogSkill(props: DialogSkillProps) {
   const dialog = useDialog()
   const sdk = useSDK()
+  const { theme } = useTheme()
+  const tuiConfig = useTuiConfig()
+  const scrollAcceleration = createMemo(() => getScrollAcceleration(tuiConfig))
+  const [store, setStore] = createStore({ selected: 0 })
   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 rows = createMemo(() => {
+    const list = skills() ?? { skills: [], invalid: [] }
+    const maxWidth = Math.max(
+      16,
+      ...list.skills.map((s) => s.name.length),
+      ...list.invalid.map((s) => path.basename(path.dirname(s.path)).length),
+    )
+    const groups = Map.groupBy(
+      list.skills.toSorted((a, b) => sourceSort(a.location).localeCompare(sourceSort(b.location)) || a.name.localeCompare(b.name)),
+      (skill) => sourceRoot(skill.location),
+    )
+
+    return [
+      ...Array.from(groups).flatMap(([root, group]) =>
+        [
+          { type: "header" as const, id: root, root, count: group.length },
+          ...group.map((skill) => ({
+            type: "skill" as const,
+            id: skill.name,
+            name: skill.name.padEnd(maxWidth),
+            rawName: skill.name,
+            description: skill.description?.replace(/\s+/g, " ").trim() ?? "No description",
+          })),
+        ],
+      ),
+      ...(list.invalid.length > 0
+        ? [
+            { type: "error-header" as const, id: "errors", count: list.invalid.length },
+            ...list.invalid.map((skill) => ({
+              type: "error" as const,
+              id: skill.path,
+              name: path.basename(path.dirname(skill.path)).padEnd(maxWidth),
+              reason: skill.reason,
+              message: skill.message,
+              location: compactLocation(skill.path),
+            })),
+          ]
+        : []),
+    ]
   })
 
-  return <DialogSelect title="Skills" placeholder="Search skills..." options={options()} />
+  const selectable = createMemo(() => rows().filter((row) => row.type === "skill" || row.type === "error"))
+  const height = createMemo(() =>
+    Math.min(
+      18,
+      rows().reduce((total, row) => total + (row.type === "error" ? 2 : 1), 0),
+    ),
+  )
+
+  function move(offset: number) {
+    if (selectable().length === 0) return
+    const next = store.selected + offset
+    setStore("selected", next < 0 ? selectable().length - 1 : next >= selectable().length ? 0 : next)
+  }
+
+  function select() {
+    const row = selectable()[store.selected]
+    if (!row || row.type !== "skill") return
+    props.onSelect(row.rawName)
+    dialog.clear()
+  }
+
+  useKeyboard((evt) => {
+    if (evt.name === "up") {
+      evt.preventDefault()
+      evt.stopPropagation()
+      move(-1)
+      return
+    }
+    if (evt.name === "down") {
+      evt.preventDefault()
+      evt.stopPropagation()
+      move(1)
+      return
+    }
+    if (evt.name === "return") {
+      evt.preventDefault()
+      evt.stopPropagation()
+      select()
+    }
+  })
+
+  const title = createMemo(() => {
+    const list = skills() ?? { skills: [], invalid: [] }
+    return list.invalid.length > 0 ? `Skills (${list.skills.length})  ${list.invalid.length} skipped` : `Skills (${list.skills.length})`
+  })
+
+  return (
+    <box border={true} borderColor={theme.accent} paddingTop={1} paddingBottom={1} gap={1}>
+      <box flexDirection="row" justifyContent="space-between" paddingLeft={3} paddingRight={3}>
+        <text fg={theme.accent} attributes={TextAttributes.BOLD}>{title()}</text>
+        <text>
+          <span style={{ fg: theme.textMuted }}>(</span>
+          <span style={{ fg: theme.accent }}>enter</span>
+          <span style={{ fg: theme.textMuted }}>) invoke  Escape to close</span>
+        </text>
+      </box>
+      <scrollbox
+        height={height()}
+        paddingLeft={2}
+        paddingRight={2}
+        scrollbarOptions={{ visible: false }}
+        scrollAcceleration={scrollAcceleration()}
+      >
+        <For each={rows()}>
+          {(row) => {
+            if (row.type === "header") return <SourceRow root={row.root} count={row.count} />
+            if (row.type === "error-header") return <ErrorHeader count={row.count} />
+            const selected = createMemo(() => selectable()[store.selected]?.id === row.id)
+            if (row.type === "error") {
+              return (
+                <box
+                  flexDirection="column"
+                  paddingLeft={selected() ? 2 : 3}
+                  paddingRight={2}
+                  backgroundColor={selected() ? theme.backgroundElement : undefined}
+                  onMouseUp={() => setStore("selected", selectable().findIndex((item) => item.id === row.id))}
+                >
+                  <text>
+                    <span style={{ fg: theme.error }}>! </span>
+                    <span style={{ fg: selected() ? theme.text : theme.accent, attributes: selected() ? TextAttributes.BOLD : undefined }}>
+                      {row.name}
+                    </span>
+                    <span style={{ fg: theme.textMuted }}> {row.reason}: </span>
+                    <span style={{ fg: theme.text }}>{Locale.truncate(row.message, 46)}</span>
+                  </text>
+                  <text fg={theme.textMuted}>   {Locale.truncateLeft(row.location, 66)}</text>
+                </box>
+              )
+            }
+            return (
+              <box
+                flexDirection="row"
+                paddingLeft={3}
+                paddingRight={2}
+                backgroundColor={selected() ? theme.backgroundElement : undefined}
+                onMouseUp={() => {
+                  setStore("selected", selectable().findIndex((item) => item.id === row.id))
+                  props.onSelect(row.rawName)
+                  dialog.clear()
+                }}
+              >
+                <text flexGrow={1} wrapMode="none">
+                  <span style={{ fg: selected() ? theme.selectedListItemText : theme.warning, attributes: TextAttributes.BOLD }}>
+                    {row.name}
+                  </span>
+                  <span style={{ fg: selected() ? theme.text : theme.textMuted }}>
+                    {Locale.truncate(row.description, 46)}
+                  </span>
+                </text>
+              </box>
+            )
+          }}
+        </For>
+      </scrollbox>
+    </box>
+  )
+}
+
+function compactLocation(location: string) {
+  const home = process.env.HOME
+  if (!home) return location
+  return location.startsWith(home) ? `~${location.slice(home.length)}` : location
+}
+
+function sourceSort(location: string) {
+  const label = sourceLabel(location)
+  const rank = label === "Global" ? 0 : label === "Project" ? 1 : label === "User" ? 2 : label === "Registry" ? 3 : 4
+  return `${rank}:${sourceRoot(location)}`
+}
+
+function sourceLabel(location: string) {
+  return sourceRootLabel(sourceRoot(location))
+}
+
+function sourceRootLabel(root: string) {
+  const compact = compactLocation(root)
+  if (compact.startsWith("~/.agents/") || compact.startsWith("~/.claude/")) return "Global"
+  if (compact.includes("/.opencode/cache/skills")) return "Registry"
+  if (root.startsWith(process.cwd())) return "Project"
+  if (compact.startsWith("~/")) return "User"
+  return "Project"
+}
+
+function sourceRoot(location: string) {
+  return path.dirname(path.dirname(location))
+}
+
+function SourceRow(props: { root: string; count: number }) {
+  const { theme } = useTheme()
+  return (
+    <box paddingLeft={2}>
+      <text>
+        <span style={{ fg: theme.accent, attributes: TextAttributes.BOLD }}>{sourceRootLabel(props.root)} </span>
+        <span style={{ fg: theme.textMuted }}>{compactLocation(props.root)}/</span>
+        <span style={{ fg: theme.textMuted }}> ({props.count})</span>
+      </text>
+    </box>
+  )
+}
+
+function ErrorHeader(props: { count: number }) {
+  const { theme } = useTheme()
+  return (
+    <box paddingLeft={2}>
+      <text>
+        <span style={{ fg: theme.error, attributes: TextAttributes.BOLD }}>Skipped skills with errors </span>
+        <span style={{ fg: theme.textMuted }}>({props.count})</span>
+      </text>
+    </box>
+  )
 }

packages/opencode/src/server/routes/instance/httpapi/groups/instance.ts

@@ -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",

packages/opencode/src/server/routes/instance/httpapi/handlers/instance.ts

@@ -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* () {

packages/opencode/src/skill/index.ts

@@ -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 })
   }),
 )
 

packages/opencode/test/session/system.test.ts

@@ -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),
         }),

packages/opencode/test/skill/skill.test.ts

@@ -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) =>

packages/sdk/js/src/v2/gen/sdk.gen.ts

@@ -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" },
           ],
         },
       ],

packages/sdk/js/src/v2/gen/types.gen.ts

@@ -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]