Skip to content

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

Merged
jdx merged 2 commits into
mainfrom
feat/skills-loose-file
Jul 27, 2026
Merged

feat: also read a single skills/SKILL.md, and tolerate skill.md casing#9
jdx merged 2 commits into
mainfrom
feat/skills-loose-file

Conversation

@jdx

@jdx jdx commented Jul 27, 2026

Copy link
Copy Markdown
Owner

#8 only matched the spec layout, skills/<name>/SKILL.md. Checking what repos actually do:

32  skills/skill.md          ← loose file
10  skills/<name>/SKILL.md   ← what #8 detected
 4  skills/SKILL.md
 3  other (x_skill.md, tdd.skill.md, SKILLS/Skill.md)

Most of the convention in the wild was invisible. This reads a loose file as one unnamed skill, and accepts skill.md inside a directory when SKILL.md is absent.

Cost

The loose file's casing comes from the listing already being fetched, so it costs no extra request. The in-directory fallback costs one, and only for a directory that did not use the documented spelling.

Skill.dir becomes Skill.path — the real repo-relative path — so the page links to a file that exists in both layouts instead of reconstructing one that might not.

This is a smaller win than the path counts suggest

Worth stating plainly, because I predicted 36 of 49 when I proposed it and that was wrong: I had counted paths, not contents.

Sampling ten of the loose-file repos, only five have YAML frontmatter at all. The rest are plain markdown in a folder named skills — not Agent Skills, and still rejected. That includes ahmadawais/excalidraw-cli and ahmadawais/ramadan-cli, the two clearest end-user CLI skills I found and the ones that motivated this change. Both open with an H1 and no frontmatter:

# Excalidraw CLI — Agent Skill

Create hand-drawn diagrams from JSON using `excalidraw-cli`.

Reading those would mean inventing the description — the field that tells an agent when a skill applies. I would rather show nothing than guess that. So they stay out, and that is the right call even though they are exactly the content this feature wants.

Realistically this takes detection from 10 of 49 to roughly 26.

On the question that prompted it

Worth recording the answer, since it decides whether this feature makes sense at all: skills in a project's repo are for end users of the tool, not contributors working on it. Every real example checked reads that way — excalidraw-cli "Create hand-drawn diagrams... using excalidraw-cli", ramadan-cli "Run, validate, and debug ramadan-cli", antvis/L7 "Use when users need to...".

That is not luck: contributor guidance has its own conventions (CLAUDE.md, AGENTS.md, CONTRIBUTING.md) and skills/ came out of distribution — TanStack Intent, skills-npm — where the point is shipping knowledge to consumers. The two do not compete for the directory.

Nothing enforces it, though, and usage.sh cannot tell a misfiled skill from a real one. The mitigation is the one already in #8: the page attributes a skill to the repo rather than asserting it, so a misfiled one reads as "this is what they published".

npm test 28/28 · typecheck 0 errors · build clean. Verified against antvis/L7 and crazyguitar/pysheeet (spec layout, both parse) and the two CLI repos above (correctly rejected).

This PR was generated by an AI coding assistant.


Note

Low Risk
Scoped to optional skill listing and parsing; no auth or data-store changes, with behavior guarded by existing frontmatter requirements and fetch caps.

Overview
GitHub skill discovery now covers layouts that were invisible before: a single skills/SKILL.md (or skill.md at the root of skills/, casing from the directory listing) and per-directory files when SKILL.md is missing but skill.md exists. Fetches share a small fetchText helper; the 25-skill cap still applies, with the loose file counted first.

Skill.dir is replaced by Skill.path — the literal repo-relative path of the file that was read. parseSkill takes (path, fallbackName, source) so display names can fall back to the directory name or the repo name for a loose file. Repo pages link to blob/HEAD/${skill.path} instead of reconstructing skills/<dir>/SKILL.md.

Tests were updated and extended for loose-file naming, accurate paths after casing fallback, and Unicode paths without percent-encoding.

Reviewed by Cursor Bugbot for commit c6dd024. Bugbot is set up for automated code reviews on this repo. Configure here.

Detection only matched `skills/<name>/SKILL.md`. Of 49 repos using a
`skills/` directory, 32 instead put a single loose file at `skills/skill.md`
and 4 at `skills/SKILL.md`, so most of the convention in the wild was
invisible here.

A loose file is now read as one unnamed skill, taking its name from the repo
since there is no directory to take it from, and `skill.md` is accepted
inside a directory as a fallback when `SKILL.md` is absent. The loose file's
casing comes from the listing rather than being guessed, so it costs no
extra request; the in-directory fallback costs one, and only for a directory
that did not use the documented spelling.

`Skill.dir` becomes `Skill.path`, the real repo-relative path, so the page
links to the file that actually exists in both layouts rather than
reconstructing one that might not.

This is a smaller win than the path counts suggest, and worth recording:
sampling ten of the loose-file repos, only five had YAML frontmatter at all.
The rest are plain markdown in a folder named `skills`, so they are not
Agent Skills and are still rejected — including `excalidraw-cli` and
`ramadan-cli`, the two clearest end-user CLI skills, whose files open with
an H1 and no frontmatter. Reading them would mean inventing the description
that tells an agent when the skill applies. Roughly 26 of 49 rather than 10.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown

Warning

Review limit reached

@jdx, you've reached your PR review limit, so we couldn't start this review.

Next review available in: 58 minutes

Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available.
You're only billed for reviews past your plan's rate limits ($0.25/file).

How can I continue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews.

How do review limits work?

CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability.

For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window.

Please refer docs for additional details.

Review details
⚙️ Run configuration

Configuration used: Central YAML (base), Organization UI (inherited)

Review profile: CHILL

Plan: Pro Plus

Run ID: 1118a3d2-bea6-469f-8c79-7f7a81a9ade5

📥 Commits

Reviewing files that changed from the base of the PR and between 9f18171 and c6dd024.

📒 Files selected for processing (4)
  • src/forges/github.ts
  • src/pages/gh/[owner]/[repo].astro
  • src/skills.test.ts
  • src/skills.ts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b7c42d7. Configure here.

Comment thread src/forges/github.ts Outdated
@greptile-apps

greptile-apps Bot commented Jul 27, 2026

Copy link
Copy Markdown

Greptile Summary

This PR expands GitHub skill discovery while preserving accurate source paths and the existing fetch cap.

  • Detects a loose skills/SKILL.md or case-insensitive equivalent.
  • Falls back from SKILL.md to skill.md inside skill directories.
  • Stores literal repository-relative paths and uses them for displayed source links.
  • Counts loose and directory-based skills together under MAX_SKILLS.
  • Updates parsing and tests for the new path and fallback-name contract.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains; the current code fixes the lowercase fallback path, enforces the aggregate skill cap, and keeps encoded request paths separate from literal display paths.

Important Files Changed

Filename Overview
src/forges/github.ts Adds loose-file and lowercase fallback discovery while correctly preserving literal paths and applying the shared fetch cap; all three previously reported issues are fixed.
src/pages/gh/[owner]/[repo].astro Uses each skill's actual repository-relative path for both link targets and labels.
src/skills.ts Replaces directory metadata with the actual source path and supports an explicit fallback name.
src/skills.test.ts Updates parser tests and covers lowercase paths, loose skills, literal Unicode paths, and fallback naming.

Reviews (2): Last reviewed commit: "fix: record the file actually read, hono..." | Re-trigger Greptile

Comment thread src/forges/github.ts Outdated
Comment thread src/forges/github.ts
Comment thread src/forges/github.ts Outdated
Three problems in the detection block, all found in review:

The `skill.md` fallback stored `skills/<dir>/SKILL.md` regardless of which
file it read, so the page linked to something that is not in the repo. That
is exactly what renaming `dir` to `path` was meant to prevent, and I
hardcoded the wrong filename one line after doing it.

The loose file was appended after the directory jobs had been capped, so a
repo with 25 skill directories plus a loose one fetched 26. It is counted
against the cap first now, since it costs a request like any other.

`Skill.path` was built from a percent-encoded directory name, so a repo with
Unicode in a skill directory would have rendered `%E6%97%A5...` on the page.
The model holds the literal path for display and linking; only the request
URL is encoded.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jdx

jdx commented Jul 27, 2026

Copy link
Copy Markdown
Owner Author

Three real bugs, all in the same block, all fixed.

Wrong path for the skill.md fallback (cursor and greptile both) — and an embarrassing one: renaming dir to path was specifically so links point at files that exist, and I then hardcoded SKILL.md in the fallback branch one line later. The loop now returns the path it actually read.

Loose file bypassed the cap (greptile) — it was pushed after the directories had been sliced, so 25 directories plus a loose file fetched 26. Counted against the cap first now, since it costs a request like any other.

Percent-encoded path in the model (greptile) — right, and the fix is the one suggested: keep the literal path for display and linking, encode only when building the request URL. A repo with Unicode in a skill directory would have rendered %E6%97%A5... on the page.

Two regression tests added for the first and third; the cap one is structural.

30/30 tests, typecheck and build clean.

This comment was generated by an AI coding assistant.

@jdx
jdx merged commit 5314ae3 into main Jul 27, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant