feat: skills catalog REST API — list, detail, search, Fuse.js, error handler (tasks 5.1-5.3, 5.5-5.7)

[johnmcollier]

Summary

Implements the Skills Catalog read API in src/server/routes/skills.ts and src/server/search/FuseSearchProvider.ts.

• GET /api/v1/skills — paginated listing; page, per_page (1–100), sort (name|updated_at); response {data, meta:{page, per_page, total}}; 400 on invalid params
• GET /api/v1/skills/search?q= — registered before /:source/:slug to avoid path conflict; Fuse.js fuzzy search over name, description, sourceSlug; ranked results with score; 400 on missing q
• GET /api/v1/skills/:source/:slug — full detail with files:[{path, contents}]; 404 SKILL_NOT_FOUND
• FuseSearchProvider — implements SearchProvider; threshold 0.4 (typo-tolerant); index rebuilt from full catalog at startup and after each sync
• Global error handler — all 4xx/5xx return {error:{code, message}}; Fastify validation errors → 400

Notes

Fuse.js score is a distance value (0 = perfect match, 1 = no match) — results are ordered best-first (correct per spec), but the raw score value increases for worse matches. Worth being aware of for future UI consumers.

Test plan

• npm run build:server — zero TypeScript errors
• npm test — 74/74 passing; 17 new skills route tests

Related

• OpenSpec tasks: 5.1, 5.2, 5.3, 5.5, 5.6, 5.7
• Jira: RHIDP-14977
• Followed by: feat/epic5-well-known (tasks 5.4 — well-known discovery + artifact serving)

Made with Cursor

[qodo-code-review]

PR Summary by Qodo

Add skills catalog read API with Fuse.js search and Fastify error handling
✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

Description

• Add read-only Skills Catalog API: list, detail, and fuzzy search endpoints.
• Introduce Fuse.js-backed search provider with index build at startup and after sync.
• Standardize API error responses and add route-level test coverage.

Diagram

graph TD
  A[API client] --> B[Fastify server]
  B --> C["skills routes"] --> D[(SQLite DB)]
  B --> E["sources routes"] --> D
  B --> F["Fuse search"] --> C
  E --> G["ingestSource()"] --> H[Git repo]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use Fastify schema validation for query/body params

• ➕ Consistent 400 responses without manual checks
• ➕ Enables automatic OpenAPI/Swagger generation later
• ➕ Centralizes validation rules and error messages
• ➖ More verbose route definitions
• ➖ Would require aligning custom error codes/messages with Fastify’s validation output

2. Use SQLite FTS5 (or similar) instead of Fuse.js

• ➕ Search stays consistent with DB state without explicit index rebuilds
• ➕ Scales better for large catalogs
• ➕ Enables filtering/sorting directly in SQL
• ➖ More complex schema/migrations and query tuning
• ➖ FTS fuzzy behavior differs from Fuse.js; may not match current spec expectations

3. Incremental index updates on skill upserts instead of full rebuild

• ➕ Faster syncs for large catalogs
• ➕ Avoids scanning the full DB each time
• ➖ More bookkeeping and higher risk of index/DB divergence
• ➖ Harder to implement correctly across deletes and source removal

Recommendation: Current approach (Fuse.js in-memory index rebuilt at startup and after sync/delete) is a good fit for the current scope/spec: it’s simple, deterministic, and keeps the REST API implementation straightforward. If the catalog size grows significantly or startup time becomes an issue, consider moving to FTS5 or incremental index updates.

Files changed (9) +777 / -16

Enhancement (6) +357 / -6

SqliteSkillRepository.tsAdd updated_at sorting support for skill listings +2/-1

Add updated_at sorting support for skill listings

• Extends the repository sort mapping to include updatedAt (updated_at DESC) and expands the findAll() sort union type accordingly.

src/server/db/SqliteSkillRepository.ts

types.tsExtend SkillRepository findAll sort options +1/-1

Extend SkillRepository findAll sort options

• Updates the SkillRepository interface so findAll() can accept an updatedAt sort option in addition to name and createdAt.

src/server/db/types.ts

index.tsWire skills/sources routes, Fuse search index, and global error handler +50/-4

Wire skills/sources routes, Fuse search index, and global error handler

• Adds a global Fastify error handler returning a consistent {error:{code,message}} shape, builds a Fuse.js search index from the current catalog, registers the new skills and sources route plugins, and changes buildServer() to return both app and searchProvider. Also guards the top-level server startup so index.ts can be imported safely in tests.

src/server/index.ts

skills.tsImplement skills list/detail/search REST endpoints +135/-0

Implement skills list/detail/search REST endpoints

• Introduces a Fastify plugin for /api/v1/skills with paginated listing (page/per_page/sort), fuzzy search at /search (registered before the dynamic route), and a skill detail endpoint returning SKILL.md content. Includes explicit parameter validation and error codes (e.g., INVALID_PARAMS, MISSING_QUERY, SKILL_NOT_FOUND).

src/server/routes/skills.ts

sources.tsImplement source management create/delete/sync endpoints with index rebuild +142/-0

Implement source management create/delete/sync endpoints with index rebuild

• Adds a Fastify plugin for /api/v1/sources implementing create (with slug/url validation, conflict handling, and clone failure cleanup), delete (with FK cascade), and sync (with concurrent sync guard and status updates). Rebuilds the search index after successful ingestion and after deletions when a SearchProvider is provided.

src/server/routes/sources.ts

FuseSearchProvider.tsAdd Fuse.js SearchProvider implementation +27/-0

Add Fuse.js SearchProvider implementation

• Implements SearchProvider using Fuse.js with tuned options (includeScore, threshold 0.4) and a replaceable index built from catalog items. Exposes search results including score for ranking/diagnostics.

src/server/search/FuseSearchProvider.ts

Tests (2) +410 / -0

skills.test.tsAdd skills API tests for pagination, detail, and search routing +212/-0

Add skills API tests for pagination, detail, and search routing

• Adds Vitest coverage for /api/v1/skills listing (defaults, pagination bounds, sorting), /:source/:slug detail responses including files[], and /search behavior including missing/empty q and typo-tolerant matching. Also verifies the /search route is not shadowed by the dynamic route.

test/server/routes/skills.test.ts

sources.test.tsAdd source management API tests for validation, conflicts, sync, and delete +198/-0

Add source management API tests for validation, conflicts, sync, and delete

• Introduces Vitest coverage for sources endpoints: slug validation, duplicate slug conflict, clone failure cleanup (no orphan source), delete behavior including FK cascade, invalid id handling, unknown source handling, and concurrent sync rejection.

test/server/routes/sources.test.ts

Documentation (1) +10 / -10

tasks.mdMark Skills Catalog and Source Management API tasks complete +10/-10

Mark Skills Catalog and Source Management API tasks complete

• Updates the OpenSpec task checklist to mark tasks 5.1–5.3, 5.5–5.7 and 6.1–6.4 as completed.

openspec/changes/rhess-enterprise-skills-server/tasks.md

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Fuse index capped at 100 ✓ Resolved 🐞 Bug ≡ Correctness

Description

The Fuse.js index build uses skills.findAll({ perPage: 10000 }), but SqliteSkillRepository.findAll()
hard-caps perPage to 100, so the search index (startup and rebuilds) will silently omit skills
beyond the first 100 and searches will miss them.

Code

src/server/index.ts[R76-86]

+  // 5.5 — Build Fuse.js search index from the current catalog
+  const searchProvider = new FuseSearchProvider();
+  const allSkills = db.skills.findAll({ perPage: 10000 });
+  searchProvider.buildIndex(
+    allSkills.map((s) => ({
+      sourceSlug: s.sourceSlug,
+      slug: s.slug,
+      name: s.name,
+      description: s.description,
+    }))
+  );

Evidence

The server asks for 10,000 skills when building the Fuse index, but the SQLite repository
implementation clamps perPage to 100, so the query can never return more than 100 rows per call; the
same pattern is used when rebuilding the index after a source is created/synced/deleted.

src/server/index.ts[76-86]
src/server/routes/sources.ts[11-22]
src/server/db/SqliteSkillRepository.ts[86-98]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The Fuse index is built/rebuilt using `findAll({ perPage: 10000 })`, but `SqliteSkillRepository.findAll()` clamps `perPage` to `<= 100`, so only the first 100 rows are ever indexed.

## Issue Context
This affects both startup index build and post-sync rebuilds, leading to incomplete search results once the catalog exceeds 100 skills.

## Fix Focus Areas
- src/server/index.ts[76-86]
- src/server/routes/sources.ts[11-22]
- src/server/db/SqliteSkillRepository.ts[86-98]

## Implementation direction
- Add a repository method that fetches all skills without the public pagination cap (e.g., `findAllUnpaged()` / `findAllForIndex()` using a SQL statement without `LIMIT`), and use it for index builds.
 - OR: loop pages using the existing API (`page=1..n`, `perPage=100`) until a page returns fewer than 100 items, concatenating all results before calling `buildIndex()`.
- Ensure both startup indexing and `rebuildSearchIndex()` use the same corrected full-scan logic.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

2. Detail returns wrong file tree ✓ Resolved 🐞 Bug ≡ Correctness

Description

GET /api/v1/skills/:source/:slug always returns files=[{path:'SKILL.md', contents: skill.content}]
and ignores supporting files, so it cannot return the full file tree; additionally, for archive
skills the ingestion stores a base64 tar.gz artifact in skill.content, which will be mislabeled as
SKILL.md contents.

Code

src/server/routes/skills.ts[R117-119]

+      const files: Array<{ path: string; contents: string }> = [
+        { path: "SKILL.md", contents: skill.content },
+      ];

Evidence

The route always emits a single SKILL.md file whose contents is skill.content. However, the
ingestion pipeline stores bundleResult.artifact into content, and bundling explicitly defines
that for archive the artifact is a base64 tar.gz string, so the endpoint will return non-markdown
archive payloads as if they were SKILL.md contents and never includes supporting files.

src/server/routes/skills.ts[117-130]
src/server/ingestion/bundle.ts[7-13]
src/server/ingestion/bundle.ts[32-57]
src/server/ingestion/ingest.ts[45-72]
src/server/db/types.ts[18-25]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The skill detail endpoint always responds with a single `SKILL.md` entry whose `contents` is `skill.content`. This is incorrect for `artifactType='archive'` (where `content` is a base64 tar.gz artifact), and it never returns supporting files.

## Issue Context
- `bundleSkill()` returns `artifact` as raw SKILL.md for `skill-md`, but as **base64 tar.gz** for `archive`.
- `ingestFromClonedPath()` stores `content: bundleResult.artifact` for both types.
- The route currently assumes `skill.content` is always SKILL.md text.

## Fix Focus Areas
- src/server/routes/skills.ts[117-130]
- src/server/ingestion/bundle.ts[7-13]
- src/server/ingestion/bundle.ts[32-57]
- src/server/ingestion/ingest.ts[45-72]
- src/server/db/types.ts[18-25]

## Implementation direction
Choose one coherent contract and implement it end-to-end:
1) **If the endpoint must return `files[{path, contents}]`:**
  - For `skill-md`: keep returning `SKILL.md` with raw markdown.
  - For `archive`: decode + untar the stored base64 artifact on demand and return the file entries (including SKILL.md and supporting files) with textual contents (define behavior for binary files).

2) **If archives should be served as an artifact instead of expanded files:**
  - Change the response shape to return either `files` for `skill-md` or `{artifactType:'archive', artifactBase64: ..., digest: ...}` for archives.
  - Update the OpenAPI/spec and tests accordingly.

Additionally:
- Include supporting file paths from `skill.supportingFiles` in the response (either as entries in `files` or as metadata) so the endpoint matches the “full file tree” requirement.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---