feat(skills): validation and shadowing signals

packages/core/src/skills/analyzeSkills.test.ts

@@ -0,0 +1,151 @@
+import type { SkillInfo } from "@posthog/shared";
+import { describe, expect, it } from "vitest";
+import { analyzeSkills, OVERSIZED_SKILL_MD_BYTES } from "./analyzeSkills";
+
+function makeSkill(overrides: Partial<SkillInfo>): SkillInfo {
+  return {
+    name: "my-skill",
+    description: "Does a thing",
+    source: "user",
+    path: "/home/.claude/skills/my-skill",
+    editable: true,
+    skillMdBytes: 1024,
+    ...overrides,
+  };
+}
+
+describe("analyzeSkills", () => {
+  it("returns no issues for a healthy skill", () => {
+    expect(analyzeSkills([makeSkill({})])).toEqual({});
+  });
+
+  it.each([
+    ["empty description", ""],
+    ["whitespace description", "   "],
+  ])("flags a missing description: %s", (_label, description) => {
+    const skill = makeSkill({ description });
+
+    const analysis = analyzeSkills([skill]);
+
+    expect(analysis[skill.path]).toEqual([
+      expect.objectContaining({ type: "missing-description" }),
+    ]);
+  });
+
+  it("flags a frontmatter name that differs from the directory name", () => {
+    const skill = makeSkill({
+      name: "Pretty Name",
+      path: "/home/.claude/skills/my-skill",
+    });
+
+    const analysis = analyzeSkills([skill]);
+
+    expect(analysis[skill.path]).toEqual([
+      expect.objectContaining({ type: "name-mismatch" }),
+    ]);
+  });
+
+  it("flags an oversized SKILL.md", () => {
+    const skill = makeSkill({ skillMdBytes: OVERSIZED_SKILL_MD_BYTES + 1 });
+
+    const analysis = analyzeSkills([skill]);
+
+    expect(analysis[skill.path]).toEqual([
+      expect.objectContaining({ type: "oversized-manifest" }),
+    ]);
+  });
+
+  it("does not flag SKILL.md exactly at the size limit", () => {
+    const skill = makeSkill({ skillMdBytes: OVERSIZED_SKILL_MD_BYTES });
+
+    expect(analyzeSkills([skill])).toEqual({});
+  });
+
+  describe("shadowing", () => {
+    it.each([
+      ["repo", "user"],
+      ["repo", "marketplace"],
+      ["repo", "bundled"],
+      ["user", "marketplace"],
+      ["user", "bundled"],
+      ["marketplace", "bundled"],
+    ] as const)("%s shadows %s", (winnerSource, loserSource) => {
+      const winner = makeSkill({
+        source: winnerSource,
+        path: `/roots/${winnerSource}/my-skill`,
+      });
+      const loser = makeSkill({
+        source: loserSource,
+        path: `/roots/${loserSource}/my-skill`,
+      });
+
+      const analysis = analyzeSkills([loser, winner]);
+
+      expect(analysis[winner.path]).toBeUndefined();
+      expect(analysis[loser.path]).toEqual([
+        expect.objectContaining({ type: "shadowed" }),
+      ]);
+    });
+
+    it("does not flag skills with distinct names", () => {
+      const a = makeSkill({ name: "alpha", path: "/roots/user/alpha" });
+      const b = makeSkill({
+        name: "beta",
+        path: "/roots/repo/beta",
+        source: "repo",
+      });
+
+      expect(analyzeSkills([a, b])).toEqual({});
+    });
+
+    it("flags the later repo when two open repos share a skill name", () => {
+      const first = makeSkill({
+        source: "repo",
+        repoName: "repo-a",
+        path: "/roots/repo-a/.claude/skills/my-skill",
+      });
+      const second = makeSkill({
+        source: "repo",
+        repoName: "repo-b",
+        path: "/roots/repo-b/.claude/skills/my-skill",
+      });
+
+      const analysis = analyzeSkills([first, second]);
+
+      expect(analysis[first.path]).toBeUndefined();
+      expect(analysis[second.path]?.[0]?.message).toContain("repo-a");
+    });
+
+    it("marks every loser when three sources collide", () => {
+      const repo = makeSkill({ source: "repo", path: "/roots/repo/my-skill" });
+      const user = makeSkill({ source: "user", path: "/roots/user/my-skill" });
+      const bundled = makeSkill({
+        source: "bundled",
+        path: "/roots/bundled/my-skill",
+        editable: false,
+      });
+
+      const analysis = analyzeSkills([bundled, user, repo]);
+
+      expect(analysis[repo.path]).toBeUndefined();
+      expect(analysis[user.path]?.[0]?.message).toContain("repository");
+      expect(analysis[bundled.path]?.[0]?.message).toContain("repository");
+    });
+  });
+
+  it("accumulates multiple issues on one skill", () => {
+    const skill = makeSkill({
+      name: "Other Name",
+      description: "",
+      skillMdBytes: OVERSIZED_SKILL_MD_BYTES * 2,
+    });
+
+    const analysis = analyzeSkills([skill]);
+
+    expect(analysis[skill.path]?.map((i) => i.type)).toEqual([
+      "missing-description",
+      "name-mismatch",
+      "oversized-manifest",
+    ]);
+  });
+});

packages/core/src/skills/analyzeSkills.ts

@@ -0,0 +1,107 @@
+import type { SkillInfo, SkillSource } from "@posthog/shared";
+
+export type SkillIssueType =
+  | "missing-description"
+  | "name-mismatch"
+  | "oversized-manifest"
+  | "shadowed";
+
+export interface SkillIssue {
+  type: SkillIssueType;
+  message: string;
+}
+
+/** Issues keyed by skill path. Paths without issues are absent. */
+export type SkillAnalysis = Record<string, SkillIssue[]>;
+
+/** SKILL.md is injected into agent context; warn when it gets expensive. */
+export const OVERSIZED_SKILL_MD_BYTES = 32 * 1024;
+
+/**
+ * Precedence when two skills share a name: the most specific source wins.
+ * Repo skills beat user skills beat marketplace plugins beat bundled ones.
+ */
+const SOURCE_PRECEDENCE: SkillSource[] = [
+  "repo",
+  "user",
+  "marketplace",
+  "bundled",
+];
+
+const SOURCE_LABEL: Record<SkillSource, string> = {
+  repo: "repository",
+  user: "user",
+  marketplace: "marketplace",
+  bundled: "bundled",
+};
+
+function precedence(source: SkillSource): number {
+  const index = SOURCE_PRECEDENCE.indexOf(source);
+  return index === -1 ? SOURCE_PRECEDENCE.length : index;
+}
+
+function directoryName(skillPath: string): string {
+  return skillPath.split(/[/\\]/).filter(Boolean).pop() ?? skillPath;
+}
+
+/** Pure health analysis over the discovered skills. No I/O. */
+export function analyzeSkills(skills: SkillInfo[]): SkillAnalysis {
+  const analysis: SkillAnalysis = {};
+  const push = (skill: SkillInfo, issue: SkillIssue) => {
+    const issues = analysis[skill.path] ?? [];
+    issues.push(issue);
+    analysis[skill.path] = issues;
+  };
+
+  for (const skill of skills) {
+    if (!skill.description.trim()) {
+      push(skill, {
+        type: "missing-description",
+        message:
+          "No description — agents rely on it to decide when to use this skill",
+      });
+    }
+
+    const dirName = directoryName(skill.path);
+    if (skill.name !== dirName) {
+      push(skill, {
+        type: "name-mismatch",
+        message: `Frontmatter name "${skill.name}" does not match the directory name "${dirName}"`,
+      });
+    }
+
+    if (skill.skillMdBytes > OVERSIZED_SKILL_MD_BYTES) {
+      push(skill, {
+        type: "oversized-manifest",
+        message: `SKILL.md is ${Math.round(skill.skillMdBytes / 1024)} kB — large manifests add context cost every time the skill is used`,
+      });
+    }
+  }
+
+  const byName = new Map<string, SkillInfo[]>();
+  for (const skill of skills) {
+    const group = byName.get(skill.name);
+    if (group) group.push(skill);
+    else byName.set(skill.name, [skill]);
+  }
+
+  for (const [name, group] of byName) {
+    if (group.length < 2) continue;
+    const sorted = [...group].sort(
+      (a, b) => precedence(a.source) - precedence(b.source),
+    );
+    const winner = sorted[0];
+    if (!winner) continue;
+    const winnerLabel = winner.repoName
+      ? `${SOURCE_LABEL[winner.source]} (${winner.repoName})`
+      : SOURCE_LABEL[winner.source];
+    for (const shadowed of sorted.slice(1)) {
+      push(shadowed, {
+        type: "shadowed",
+        message: `Shadowed by the ${winnerLabel} skill named "${name}"`,
+      });
+    }
+  }
+
+  return analysis;
+}

packages/shared/src/skills.ts

@@ -8,6 +8,8 @@ export interface SkillInfo {
   repoName?: string;
   /** Whether the skill lives in a directory we own on the user's behalf. */
   editable: boolean;
+  /** Size of SKILL.md in bytes (context-cost signal). */
+  skillMdBytes: number;
 }
 
 export interface SkillFileEntry {

packages/ui/src/features/skills/SkillCard.tsx

@@ -1,6 +1,16 @@
-import { Folder, Package, Storefront, User } from "@phosphor-icons/react";
+import {
+  Folder,
+  Package,
+  Storefront,
+  User,
+  Warning,
+} from "@phosphor-icons/react";
+import type {
+  SkillAnalysis,
+  SkillIssue,
+} from "@posthog/core/skills/analyzeSkills";
 import type { SkillInfo, SkillSource } from "@posthog/shared";
-import { Badge, Box, Flex, Text } from "@radix-ui/themes";
+import { Badge, Box, Flex, Text, Tooltip } from "@radix-ui/themes";
 import { useEffect, useRef } from "react";
 
 export const SOURCE_CONFIG: Record<
@@ -28,6 +38,7 @@ interface SkillCardProps {
   /** When true, scroll this card into view once (used for deep-linked skills). */
   scrollIntoView?: boolean;
   onScrolledIntoView?: () => void;
+  issues?: SkillIssue[];
 }
 
 export function SkillCard({
@@ -36,6 +47,7 @@ export function SkillCard({
   onClick,
   scrollIntoView,
   onScrolledIntoView,
+  issues = [],
 }: SkillCardProps) {
   const config = SOURCE_CONFIG[skill.source];
   const Icon = config?.icon ?? Package;
@@ -76,6 +88,22 @@ export function SkillCard({
         )}
       </Flex>
 
+      {issues.length > 0 && (
+        <Tooltip
+          content={
+            <Flex direction="column" gap="1">
+              {issues.map((issue) => (
+                <Text key={issue.message} size="1">
+                  {issue.message}
+                </Text>
+              ))}
+            </Flex>
+          }
+        >
+          <Warning size={14} className="shrink-0 text-amber-11" />
+        </Tooltip>
+      )}
+
       {skill.repoName && (
         <Badge size="1" variant="soft" color="gray" className="shrink-0">
           {skill.repoName}
@@ -92,6 +120,7 @@ interface SkillSectionProps {
   onSelect: (path: string) => void;
   scrollToPath: string | null;
   onScrolledIntoView: () => void;
+  analysis?: SkillAnalysis;
 }
 
 export function SkillSection({
@@ -101,6 +130,7 @@ export function SkillSection({
   onSelect,
   scrollToPath,
   onScrolledIntoView,
+  analysis,
 }: SkillSectionProps) {
   return (
     <Flex direction="column" gap="1">
@@ -116,6 +146,7 @@ export function SkillSection({
             onClick={() => onSelect(skill.path)}
             scrollIntoView={scrollToPath === skill.path}
             onScrolledIntoView={onScrolledIntoView}
+            issues={analysis?.[skill.path]}
           />
         ))}
       </Flex>