feat: also read a single skills/SKILL.md, and tolerate skill.md casing

src/forges/github.ts

@@ -115,25 +115,58 @@ export const github: Forge = {
     const entries = (await res.json()) as Array<{ name: string; type: string }>;
     if (!Array.isArray(entries)) return null;
 
-    const dirs = entries
+    const fetchText = async (path: string): Promise<string | null> => {
+      const r = await fetch(`${RAW}/${owner}/${repo}/HEAD/${path}`, {
+        cf: { cacheTtl: 600, cacheEverything: true },
+      }).catch(() => null);
+      return r?.ok ? await r.text() : null;
+    };
+
+    const jobs: Array<Promise<Skill | null>> = [];
+
+    // A single unnamed skill at `skills/SKILL.md`. Not the spec layout, but by
+    // far the most common shape in the wild. Taken from the listing, so the
+    // casing is whatever the repo used and nothing is guessed. Counted against
+    // the cap first, since it costs a fetch like any other.
+    const loose = entries.find(
+      (e) => e.type === "file" && /^skill\.md$/i.test(e.name),
+    );
+    if (loose) {
+      const path = `skills/${loose.name}`;
+      jobs.push(
+        (async () => {
+          const source = await fetchText(path);
+          // No directory to name it after, so fall back to the repo.
+          return source ? parseSkill(path, repo, source) : null;
+        })(),
+      );
+    }
+
+    // The spec layout. `SKILL.md` is the spelling it defines; some repos use
+    // `skill.md`, so fall back rather than miss them — one extra request, and
+    // only for a directory that did not have the documented name.
+    for (const dir of entries
       .filter((e) => e.type === "dir")
       .map((e) => e.name)
       .sort()
-      .slice(0, MAX_SKILLS);
-
-    const loaded = await Promise.all(
-      dirs.map(async (dir) => {
-        const body = await fetch(
-          `${RAW}/${owner}/${repo}/HEAD/skills/${encodeURIComponent(dir)}/SKILL.md`,
-          { cf: { cacheTtl: 600, cacheEverything: true } },
-        ).catch(() => null);
-        if (!body?.ok) return null;
-        // A skill whose frontmatter cannot be read is skipped rather than
-        // shown half-parsed; it is someone else's file, not ours to guess at.
-        return parseSkill(dir, await body.text());
-      }),
-    );
-
+      .slice(0, MAX_SKILLS - jobs.length)) {
+      jobs.push(
+        (async () => {
+          for (const file of ["SKILL.md", "skill.md"]) {
+            // `path` is the literal repo path, for display and for linking;
+            // only the request URL is encoded.
+            const path = `skills/${dir}/${file}`;
+            const source = await fetchText(
+              `skills/${encodeURIComponent(dir)}/${file}`,
+            );
+            if (source) return parseSkill(path, dir, source);
+          }
+          return null;
+        })(),
+      );
+    }
+
+    const loaded = await Promise.all(jobs);
     return loaded.filter((s): s is Skill => s !== null);
   },
 

src/pages/gh/[owner]/[repo].astro

@@ -211,10 +211,10 @@ const fmtMs = (n: number) => `${n.toFixed(2)}ms`;
                   <p class="m small">{skill.compatibility}</p>
                 )}
                 <a
-                  href={`${data.url}/blob/HEAD/skills/${skill.dir}/SKILL.md`}
+                  href={`${data.url}/blob/HEAD/${skill.path}`}
                   rel="noopener"
                 >
-                  skills/{skill.dir}/SKILL.md
+                  {skill.path}
                 </a>
               </div>
             ))}

src/skills.test.ts

@@ -10,6 +10,7 @@ import { parseSkill } from "./skills.ts";
 
 test("reads the required fields", () => {
   const s = parseSkill(
+    "skills/pdf-processing/SKILL.md",
     "pdf-processing",
     `---
 name: pdf-processing
@@ -21,14 +22,15 @@ description: Extract PDF text and fill forms. Use when handling PDFs.
 Body text.
 `,
   )!;
-  assert.equal(s.dir, "pdf-processing");
+  assert.equal(s.path, "skills/pdf-processing/SKILL.md");
   assert.equal(s.name, "pdf-processing");
   assert.match(s.description, /Extract PDF text/);
   assert.match(s.body, /^# PDF processing/);
 });
 
 test("reads the optional fields the spec defines", () => {
   const s = parseSkill(
+    "skills/x/SKILL.md",
     "x",
     `---
 name: x
@@ -47,6 +49,7 @@ body
 
 test("handles quoted values", () => {
   const s = parseSkill(
+    "skills/x/SKILL.md",
     "x",
     `---
 name: "x"
@@ -60,6 +63,7 @@ description: 'Uses: colons, and commas'
 
 test("handles a folded description", () => {
   const s = parseSkill(
+    "skills/x/SKILL.md",
     "x",
     `---
 name: x
@@ -77,6 +81,7 @@ description: >
 
 test("handles a literal block, keeping its newlines", () => {
   const s = parseSkill(
+    "skills/x/SKILL.md",
     "x",
     `---
 name: x
@@ -92,6 +97,7 @@ description: |
 test("skips nested structures it does not model", () => {
   // `metadata` is an arbitrary map; its keys must not leak into the fields.
   const s = parseSkill(
+    "skills/x/SKILL.md",
     "x",
     `---
 name: x
@@ -106,32 +112,81 @@ description: after the nested block
   assert.equal(s.name, "x");
 });
 
-test("falls back to the directory name when name is absent", () => {
-  const s = parseSkill("from-dir", `---\ndescription: d\n---\n`)!;
+test("falls back to the given name when the field is absent", () => {
+  const s = parseSkill(
+    "skills/from-dir/SKILL.md",
+    "from-dir", `---\ndescription: d\n---\n`)!;
   assert.equal(s.name, "from-dir");
 });
 
 test("rejects a file with no description", () => {
   // The spec requires it, and without one there is nothing worth listing.
-  assert.equal(parseSkill("x", `---\nname: x\n---\nbody`), null);
+  assert.equal(parseSkill(
+    "skills/x/SKILL.md",
+    "x", `---\nname: x\n---\nbody`), null);
+});
+
+test("a loose skills/SKILL.md falls back to the repo name", () => {
+  // Not the spec layout, but the most common shape in the wild; there is no
+  // directory to take a name from, so the repo stands in.
+  const s = parseSkill(
+    "skills/skill.md",
+    "excalidraw-cli",
+    `---\ndescription: Create diagrams from JSON\n---\nbody`,
+  )!;
+  assert.equal(s.name, "excalidraw-cli");
+  assert.equal(s.path, "skills/skill.md");
+});
+
+test("the path records the file that was actually read", () => {
+  // The fallback to `skill.md` must not claim `SKILL.md`, or the page links
+  // to a file that is not in the repo.
+  const s = parseSkill(
+    "skills/ui/skill.md",
+    "ui",
+    `---\nname: ui\ndescription: d\n---\nbody`,
+  )!;
+  assert.equal(s.path, "skills/ui/skill.md");
+});
+
+test("the path is literal, not percent-encoded", () => {
+  // Encoding belongs in the request URL; the model is what gets displayed
+  // and linked, so a Unicode directory name must survive intact.
+  const s = parseSkill(
+    "skills/日本語/SKILL.md",
+    "日本語",
+    `---\ndescription: d\n---\nbody`,
+  )!;
+  assert.equal(s.path, "skills/日本語/SKILL.md");
+  assert.equal(s.name, "日本語");
 });
 
 test("rejects a whitespace-only description", () => {
   // The spec requires a non-empty description; quotes made this look present.
-  assert.equal(parseSkill("x", `---\nname: x\ndescription: "   "\n---\nbody`), null);
-  assert.equal(parseSkill("x", `---\nname: x\ndescription: |\n   \n---\nbody`), null);
+  assert.equal(parseSkill(
+    "skills/x/SKILL.md",
+    "x", `---\nname: x\ndescription: "   "\n---\nbody`), null);
+  assert.equal(parseSkill(
+    "skills/x/SKILL.md",
+    "x", `---\nname: x\ndescription: |\n   \n---\nbody`), null);
 });
 
 test("rejects a file with no frontmatter", () => {
-  assert.equal(parseSkill("x", "# Just markdown\n"), null);
+  assert.equal(parseSkill(
+    "skills/x/SKILL.md",
+    "x", "# Just markdown\n"), null);
 });
 
 test("rejects an unterminated frontmatter block", () => {
-  assert.equal(parseSkill("x", "---\nname: x\ndescription: d\n"), null);
+  assert.equal(parseSkill(
+    "skills/x/SKILL.md",
+    "x", "---\nname: x\ndescription: d\n"), null);
 });
 
 test("tolerates CRLF line endings", () => {
-  const s = parseSkill("x", "---\r\nname: x\r\ndescription: d\r\n---\r\nbody\r\n")!;
+  const s = parseSkill(
+    "skills/x/SKILL.md",
+    "x", "---\r\nname: x\r\ndescription: d\r\n---\r\nbody\r\n")!;
   assert.equal(s.description, "d");
   assert.equal(s.body, "body");
 });

src/skills.ts

@@ -13,8 +13,12 @@
 
 /** The frontmatter fields the spec defines. `metadata` is deliberately not read. */
 export interface Skill {
-  /** Directory name, which the spec requires to match the `name` field. */
-  dir: string;
+  /**
+   * Repo-relative path to the file, so the page can link to the real thing.
+   * `skills/<name>/SKILL.md` for the spec layout, `skills/SKILL.md` for a
+   * repo that publishes a single unnamed skill.
+   */
+  path: string;
   name: string;
   description: string;
   license?: string;
@@ -88,17 +92,22 @@ function parseFrontmatter(src: string): {
 /**
  * Build a Skill from one `SKILL.md`, or null when it is not usable.
  *
- * `name` and `description` are the spec's only required fields, and without a
- * description there is nothing worth listing, so both are required here too.
+ * `description` is required: the spec says so, and without one there is
+ * nothing worth listing. `name` falls back to `fallbackName`, which is the
+ * containing directory for the spec layout and the repo for a loose file.
  */
-export function parseSkill(dir: string, source: string): Skill | null {
+export function parseSkill(
+  path: string,
+  fallbackName: string,
+  source: string,
+): Skill | null {
   const { fields, body } = parseFrontmatter(source);
-  const name = fields.name ?? dir;
+  const name = fields.name ?? fallbackName;
   const description = fields.description;
   if (!description) return null;
 
   return {
-    dir,
+    path,
     name,
     description,
     license: fields.license,