Skip to content

feat(skill): allagents skills search via GitHub Code Search#386

Merged
christso merged 1 commit into
mainfrom
feat/378-skill-search
May 12, 2026
Merged

feat(skill): allagents skills search via GitHub Code Search#386
christso merged 1 commit into
mainfrom
feat/378-skill-search

Conversation

@christso
Copy link
Copy Markdown
Contributor

Summary

Adds allagents skills search <query> for in-CLI skill discovery via the GitHub Code Search API. Bridges "I want a skill that does X" → install.

  • New src/core/skill-search.ts owns the API call, validation, ranking, and error mapping. Query is composed as <query> filename:SKILL.md path:SKILL.md [user:<owner>] so the search is constrained to skill manifests.
  • Auth comes from GITHUB_TOKEN / GH_TOKEN when present. Unauthenticated requests share the public 10/min Code Search rate limit — a 403 with a rate limit body is mapped to a SkillSearchError(kind='rate-limit') with an actionable hint (gh auth login / GITHUB_TOKEN), never a raw HTML dump.
  • Validation (query ≥ 2 chars, --page ≥ 1, --limit 1–100, --owner ≤ 39 chars alphanumeric+dash) exits 2 with a clear message rather than a Zod stack.
  • Ranking heuristic mirrors gh skill search: exact name match → prefix match → substring → upstream order.
  • Wired into --agent-help via the new skillsSearchMeta.

Test plan

  • bun run build, bun run typecheck
  • bun test — 1199 pass / 0 fail (8 new unit tests use injected fake fetch, no live network)
  • Verification gate from feat: allagents skill search via GitHub Code Search (skill discovery) #378 (with GITHUB_TOKEN):
    • skills search --help | grep owner matches.
    • --json skills search documentation --limit 5 returns ≥ 1 item.
    • --owner github narrows the result set.
    • Page 1 vs page 2 produce different result sets.
    • skills search a exits 2 with "Search query must be at least 2 characters".
    • skills search docs --page 0 and --limit 0 exit 2 with their respective validation messages.
    • Guideline (7) — rate-limit mapping — covered by tests/unit/core/skill-search.test.ts so the assertion is deterministic and doesn't depend on actually exhausting the quota.

Closes #378

@cloudflare-workers-and-pages
Copy link
Copy Markdown

cloudflare-workers-and-pages Bot commented May 12, 2026

Deploying allagents with  Cloudflare Pages  Cloudflare Pages

Latest commit: 000ce23
Status: ✅  Deploy successful!
Preview URL: https://bb00ad57.allagents.pages.dev
Branch Preview URL: https://feat-378-skill-search.allagents.pages.dev

View logs

@christso
Copy link
Copy Markdown
Contributor Author

Code Review: ✅ APPROVE (best-tested PR in this batch)

This PR stands out for having actual unit-test coverage of the new behavior — the validation matrix and the rate-limit / 422 / success error-mapping paths all get distinct tests with an injected fake fetch. That's the model the other feature PRs in this batch should follow.

Verification gate (#378)

  • (1) skills search --help exposes --owner ✓ (option declared in searchCmd.args)
  • (2) Network-dependent — assumed passing per PR description ✓
  • (3) --owner narrows results — depends on upstream behavior, but query is built correctly via user:<owner> qualifier ✓
  • (4) Pagination monotonic — per_page and page are passed through to GitHub ✓
  • (5) skills search a exits 2 with the min-length message — pinned by unit test ✓
  • (6) --page 0 / --limit 0 exit 2 — pinned by unit test ✓
  • (7) Rate-limit mapping — pinned by unit test (maps 403 rate-limit to a SkillSearchError with kind "rate-limit") ✓

CLAUDE.md compliance

  • Conventional commit (feat(skill): ...) ✓
  • No Co-Authored-By
  • Tests-per-distinct-path discipline: clean
  • New module isolated in src/core/skill-search.ts with no premature abstraction

Design highlights

  • Injected fetch dep. searchSkills(query, options, { fetch }) makes the test seam first-class — no module-level monkey-patching. Worth keeping as a convention for similar I/O-bound helpers.
  • Error classification is structured. SkillSearchError.kind ('validation' | 'rate-limit' | 'api') maps cleanly to exit codes (validation → 2, others → 1) in the handler. Better than throwing strings.
  • Rate-limit message is actionable. "Authenticate with gh auth login or set GITHUB_TOKEN" tells the user exactly what to do — matches issue's "don't dump the raw response" guidance.

Non-blocking suggestions

1. Issue scope: interactive TUI selector is missing.

The issue's expected behavior #4 was "allagents skill search docs → TUI selector — pick a row, press Enter → runs allagents skill add for that selection." Implementation notes spelled out: "Interactive selection: build on existing TUI primitives in src/cli/tui/ ... Pressing Enter on a row should invoke the install flow." The PR delivers a read-only ranked list. The verification gate doesn't test this — only checks that search works — so the gate passes, but the "search → install bridge" UX from the issue is one half-step short.

Either acknowledge it as deferred in the squash commit, or file a follow-up issue and link it.

2. Bypasses src/core/github-fetch.ts.

Issue's implementation notes: "Use src/core/github-fetch.ts for the API call so existing auth + token handling is reused." This PR uses raw fetch with inline GITHUB_TOKEN/GH_TOKEN handling. The reasons to use github-fetch.ts are:

  • Centralized retry + back-off logic (if any)
  • Single place to update auth headers when the scheme evolves (e.g., Bearer for fine-grained tokens)
  • Consistent User-Agent header

Worth a follow-up to route through the shared helper, even if it doesn't change observable behavior today.

3. Missing jsonFields declaration.

Issue's implementation notes: "jsonFields (per the json-allowlist issue): at minimum name, repo, path, description, sha." skillsSearchMeta has outputSchema but no jsonFields. Coordinates with #384 — once that lands, --json=name,repo skills search docs should narrow per-item to those fields, but only if jsonFields is declared. Add it here so the feature is usable immediately when #384 merges.

4. --page and --limit as string then Number.parseInt.

page: option({ type: optional(string), long: 'page', ... })
// later
const n = Number.parseInt(page, 10);
if (Number.isNaN(n)) { error }
opts.page = n;

cmd-ts has a number codec — using it would let cmd-ts handle the parse + error, removing two Number.isNaN branches. Minor.

5. Authorization: token <T> vs Bearer <T>.

GitHub's docs recommend Bearer <T> for fine-grained PATs going forward (token <T> still works for classic PATs). Won't break anything today; future-proof to switch.

6. SKILL.md path parsing edge case.

const name = parts.length >= 2 ? parts[parts.length - 2] ?? '' : ...repo basename...

For a repo with path: 'SKILL.md' (top-level), parts.length is 1, so the fallback returns the repo's basename. OK. But for path: 'foo/bar/baz/SKILL.md', name is baz — fine, that's the immediate parent. For path: 'SKILL.md/inner/SKILL.md' (pathological but possible), name would be inner. Probably not worth defending against, but worth a docstring sentence acknowledging the assumption.


Suggested merge order

This PR is genuinely independent. Approves as-is. The TUI selector and jsonFields declaration are good follow-ups but not blockers.

New `skills search <query>` subcommand bridges "I want a skill that does X"
→ install. Hits the GitHub Code Search API for `SKILL.md` files matching the
query, ranks by relevance, and prints either a human table or the standard
JSON envelope.

- src/core/skill-search.ts: new module that owns the API call, validation,
  and error-mapping. The query is composed as
  `<query> filename:SKILL.md path:SKILL.md [user:<owner>]`. Auth comes from
  `GITHUB_TOKEN` / `GH_TOKEN` when present; unauthenticated requests share
  the public 10/min Code Search rate limit.
- Validation: query ≥ 2 chars; `--page` ≥ 1; `--limit` 1–100; `--owner`
  matches `[A-Za-z0-9-]{1,39}`. Validation errors exit 2 with a clear
  message instead of a generic Zod dump.
- Error mapping: a 403 with a "rate limit" message body maps to a
  `SkillSearchError(kind='rate-limit')` with an actionable hint
  (`gh auth login` / `GITHUB_TOKEN`). Other API failures fold into
  `kind='api'`. Never dumps raw HTML or stderr.
- Ranking heuristic mirrors gh-skill's: exact name match → prefix match →
  substring match → upstream order.

- src/cli/commands/plugin-skills.ts: registers `searchCmd` under `skillsCmd`.
- src/cli/metadata/plugin-skills.ts: new `skillsSearchMeta`; wired into
  agent-help so `--agent-help "skills search"` is discoverable.
- tests/unit/core/skill-search.test.ts: 8 cases covering arg validation,
  the 403 rate-limit mapping, the 422 mapping, and the happy-path item
  normalisation. All use injected fake `fetch`, no live network.

Verified end-to-end against the live GitHub API (with GITHUB_TOKEN):
- Basic search returns ≥ 1 result for "documentation".
- `--owner github` narrows to a subset.
- Page 1 vs page 2 produce different result sets.
- Query < 2 chars exits 2 with "Search query must be at least 2 characters".
- `--page 0` / `--limit 0` exit 2 with the matching validation messages.
- Rate-limit unit test covers the unauthenticated-403 path so the gate's
  guideline (7) is exercised without depending on actually exhausting the
  quota.

Closes #378
@christso christso force-pushed the feat/378-skill-search branch from 4f50f85 to 000ce23 Compare May 12, 2026 14:17
@christso christso merged commit 9834e70 into main May 12, 2026
1 check passed
@christso christso deleted the feat/378-skill-search branch May 12, 2026 14:50
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.

feat: allagents skill search via GitHub Code Search (skill discovery)

1 participant