feat(skills): allow registering local skill directories at runtime

backend/cli/src/server/routes/settings/skills.ts

@@ -15,8 +15,75 @@ import { lazy } from "@synsci/util/lazy"
 // runs the full local-first fetch + multi-layer security review pipeline
 // (`Skill.Install.add`). This route exposes exactly that.
 export const SettingsSkillsRoutes = lazy(() =>
-  new Hono().post(
-    "/install",
+  new Hono()
+    // ---- custom skill roots -------------------------------------------
+    .get(
+      "/paths",
+      describeRoute({
+        summary: "List skill roots",
+        description:
+          "Every directory currently contributing skills, with its origin (builtin / user / custom) and skill count. `custom` covers roots declared in `skills.paths` and roots registered at runtime.",
+        operationId: "settings.skills.paths.list",
+        responses: { 200: { description: "Skill roots", content: { "application/json": { schema: resolver(z.object({ paths: z.array(z.any()) })) } } } },
+      }),
+      async (c) => c.json({ paths: await Skill.roots() }),
+    )
+    .post(
+      "/paths",
+      describeRoute({
+        summary: "Register a skill directory",
+        description:
+          "Add a local directory as a skill root. The directory is scanned immediately; no restart is needed. With persist=true the path is also written to skills.paths so it survives a restart. A missing directory is rejected with 400.",
+        operationId: "settings.skills.paths.add",
+        responses: { 201: { description: "Registered" }, ...errors(400) },
+      }),
+      validator(
+        "json",
+        z.object({
+          path: z.string().min(1).describe("Directory containing one or more <name>/SKILL.md skills"),
+          persist: z.boolean().optional().describe("Also write the path to skills.paths"),
+        }),
+      ),
+      async (c) => {
+        const { path, persist } = c.req.valid("json")
+        try {
+          return c.json(await Skill.addPath(path, persist ?? false), 201)
+        } catch (e) {
+          if (e instanceof Skill.InvalidRootError) return c.json({ error: e.data }, 400)
+          throw e
+        }
+      },
+    )
+    .delete(
+      "/paths",
+      describeRoute({
+        summary: "Unregister a skill directory",
+        description: "Remove a runtime-registered skill root, and with persist=true also drop it from skills.paths.",
+        operationId: "settings.skills.paths.remove",
+        responses: { 200: { description: "Removed" } },
+      }),
+      validator("query", z.object({ path: z.string().min(1), persist: z.coerce.boolean().optional() })),
+      async (c) => {
+        const { path, persist } = c.req.valid("query")
+        await Skill.removePath(path, persist ?? false)
+        return c.json({ ok: true })
+      },
+    )
+    .post(
+      "/reload",
+      describeRoute({
+        summary: "Rescan skill directories",
+        description: "Invalidate the skill cache so directories changed out-of-band are picked up without a restart.",
+        operationId: "settings.skills.reload",
+        responses: { 200: { description: "Rescanned", content: { "application/json": { schema: resolver(z.object({ skills: z.number() })) } } } },
+      }),
+      async (c) => {
+        await Skill.invalidate()
+        return c.json({ skills: (await Skill.all()).length })
+      },
+    )
+    .post(
+      "/install",
     describeRoute({
       summary: "Install skill from git",
       description:

backend/cli/src/skill/skill.ts

@@ -501,9 +501,11 @@ export namespace Skill {
       }
     }
 
-    // Scan additional skill paths from config
+    // Scan additional skill paths from config, plus any roots registered at
+    // runtime through Skill.addPath() (which do not need a restart).
     const config = await Config.getExecution()
-    for (const skillPath of config.skills?.paths ?? []) {
+    const extraRoots = [...(config.skills?.paths ?? []), ...Skill.runtimeRoots]
+    for (const skillPath of extraRoots) {
       const expanded = skillPath.startsWith("~/") ? path.join(os.homedir(), skillPath.slice(2)) : skillPath
       const resolved = path.isAbsolute(expanded) ? expanded : path.join(Instance.directory, expanded)
       if (!(await Filesystem.isDir(resolved))) {
@@ -656,6 +658,104 @@ export namespace Skill {
     return value
   }
 
+  // ---------------------------------------------------------------------
+  // Custom skill roots
+  //
+  // `skills.paths` in openscience.json has always been able to load extra
+  // directories, but only at boot and with no way to see what is active. These
+  // helpers make skill roots first-class: they can be listed, added and removed
+  // while the server is running, and every root reports where it came from so a
+  // shadowed skill is visible instead of silently losing to a same-named one.
+  // ---------------------------------------------------------------------
+
+  export const RootInfo = z.object({
+    path: z.string(),
+    kind: z.enum(["builtin", "user", "custom"]),
+    skills: z.number(),
+  })
+  export type RootInfo = z.infer<typeof RootInfo>
+
+  /** Roots registered for this process only (never written to disk). */
+  export const runtimeRoots = new Set<string>()
+
+  function expandRoot(raw: string) {
+    const expanded = raw.startsWith("~/") ? path.join(os.homedir(), raw.slice(2)) : raw
+    return path.isAbsolute(expanded) ? path.normalize(expanded) : path.join(Instance.directory, expanded)
+  }
+
+  /** Distinct skill roots currently contributing to the catalog. */
+  export async function roots(): Promise<RootInfo[]> {
+    const config = await Config.getExecution()
+    const configured = (config.skills?.paths ?? []).map(expandRoot)
+    const entries = await all()
+    const buckets = new Map<string, { kind: RootInfo["kind"]; skills: number }>()
+    for (const root of configured) buckets.set(root, { kind: "custom", skills: 0 })
+    const runtime = [...runtimeRoots]
+    for (const info of entries) {
+      const dir = path.dirname(path.dirname(info.location)) // …/<skill>/SKILL.md -> …
+      // a root is "custom" when it came from config or from Skill.addPath();
+      // everything else is a built-in or user root
+      const hit = configured.find((r) => info.location.startsWith(r + path.sep))
+                    ?? runtime.find((r) => info.location.startsWith(r + path.sep))
+      const key = hit ?? dir
+      const kind: RootInfo["kind"] = hit
+        ? "custom"
+        : info.origin === "user"
+          ? "user"
+          : "builtin"
+      const prev = buckets.get(key)
+      buckets.set(key, { kind: prev?.kind ?? kind, skills: (prev?.skills ?? 0) + 1 })
+    }
+    return [...buckets.entries()]
+      .filter(([dir]) => dir)
+      .map(([dir, v]) => ({ path: dir, kind: v.kind, skills: v.skills }))
+      .sort((a, b) => a.path.localeCompare(b.path))
+  }
+
+  /**
+   * Register a directory as a skill root. Validation is strict on purpose: a
+   * missing directory is an error rather than a silent no-op, because the
+   * failure mode people hit today is a typo'd path that simply does nothing.
+   * With `persist` the path is appended to `skills.paths` so it survives a
+   * restart; without it the root lives only in this process.
+   */
+  export async function addPath(raw: string, persist = false): Promise<RootInfo> {
+    const resolved = expandRoot(raw)
+    if (!(await Filesystem.isDir(resolved))) throw new InvalidRootError({ path: resolved, reason: "not a directory" })
+    if (persist) {
+      const global = await Config.getGlobal()
+      const current = global.skills?.paths ?? []
+      if (!current.includes(raw)) {
+        await Config.updateGlobal({ ...global, skills: { ...global.skills, paths: [...current, raw] } })
+      }
+    }
+    runtimeRoots.add(resolved)
+    await invalidate()
+    log.info("skill root added", { path: resolved, persist })
+    const found = (await roots()).find((r) => r.path === resolved)
+    return found ?? { path: resolved, kind: "custom", skills: 0 }
+  }
+
+  /** Drop a runtime-registered root (or remove it from the config list). */
+  export async function removePath(raw: string, persist = false) {
+    const resolved = expandRoot(raw)
+    runtimeRoots.delete(resolved)
+    if (persist) {
+      const global = await Config.getGlobal()
+      const current = global.skills?.paths ?? []
+      const next = current.filter((x) => expandRoot(x) !== resolved)
+      if (next.length !== current.length) {
+        await Config.updateGlobal({ ...global, skills: { ...global.skills, paths: next } })
+      }
+    }
+    await invalidate()
+    log.info("skill root removed", { path: resolved, persist })
+  }
+
+  export const InvalidRootError = NamedError.create(
+    "SkillInvalidRootError",
+    z.object({ path: z.string(), reason: z.string() }),
+  )
   /** Build the single permission-annotated catalog consumed by every skill
    * discovery surface. Skill contents stay lazy; this snapshot contains only
    * the already-indexed frontmatter metadata. */
@@ -684,3 +784,4 @@ export namespace Skill {
     return snapshot
   }
 }
+

feature-notes/FEATURE-PROPOSAL.md

@@ -0,0 +1,123 @@
+# Feature proposal: plug arbitrary local skill directories into the skill library
+
+**Status:** proposal + reference tooling · **Target:** synthetic-sciences/openscience
+
+## Problem
+
+`skills.paths` already exists in `openscience.json`, so skills can be loaded from
+extra directories — but only **statically, at boot**. There is no way for a user
+or a tool to
+
+* ask the running server **which** skill roots are active and where each skill
+  came from,
+* **register or remove** a local skill directory without editing a config file
+  and restarting,
+* **sync / diff / vendor** a local skill collection against the server.
+
+As a result anyone who keeps their own skill library (a private team pack, a
+vendor pack, an air-gapped mirror) has to fork the project and patch the source,
+which is exactly the situation this proposal removes.
+
+### Current API surface (v2.0.93, measured)
+
+| endpoint | state |
+|:--|:--|
+| `GET /skill` | lists `name, description, location, origin, permission_action, recommended, enabled` — **no content** |
+| `PUT /skill/{name}` | write a skill (body: `content`) |
+| `DELETE /skill/{name}` | remove a skill |
+| `POST /settings/skills/install` | install from a `url` |
+| `GET/POST/DELETE /skill/paths` | **absent (404)** |
+| `POST /skill/reload` | **absent (404)** |
+| `GET /skill?withContent=1` | parameter ignored |
+
+## Proposal
+
+### 1. Make skill roots first-class and introspectable
+
+```
+GET /skill/paths
+-> { "paths": [
+       { "path": "…/backend/cli/skills",        "kind": "builtin", "skills": 312 },
+       { "path": "…/user-skills",               "kind": "user",    "skills": 295 },
+       { "path": "D:/my-team-skills",           "kind": "custom",  "skills": 49 }
+     ], "revision": 17 }
+```
+
+`kind` is `builtin | user | custom`, derived from how the root was registered.
+`revision` increments on any change so clients can cache.
+
+### 2. Register / unregister roots at runtime (no restart)
+
+```
+POST   /skill/paths   { "path": "D:/my-team-skills", "persist": true }
+                      -> 201 { "revision": 18, "skills": 49 }
+DELETE /skill/paths?path=D:/my-team-skills
+                      -> 200 { "revision": 19 }
+```
+
+* the directory is **scanned immediately** and its skills become usable without
+  a restart (hot reload); a `POST /skill/reload` endpoint is also useful for
+  re-scanning after an out-of-band file change;
+* `persist: true` appends the path to `skills.paths` in `openscience.json`, so it
+  survives a restart (this is the only behaviour that needs a config write);
+* reject paths that do not exist or are not directories (400), and paths already
+  registered (409) rather than silently duplicating skills.
+
+### 3. Deterministic conflict resolution
+
+Scanning must be recursive — a real skill library nests skills under category
+folders (`ml-training/unsloth-fine-tuning/SKILL.md`). When two roots expose the
+same skill name:
+
+```
+priority: custom  >  user  >  builtin
+```
+
+(later-registered custom roots win over earlier ones), and the shadowed entry is
+still reported by `GET /skill` with `"shadowed_by": "<path>"` so a user can see
+why their edit had no effect. This is the single most confusing failure mode
+today: a locally edited skill silently loses to a same-named builtin one.
+
+### 4. Let clients sync without filesystem access
+
+```
+GET /skill?withContent=1     # include content in each entry
+GET /skill/{name}/content    # or a dedicated sub-resource
+```
+
+Today the only way to read a skill's text is to follow `location` on the local
+filesystem, which breaks for a remote or containerised server. Either form is
+enough to make a dumb client able to vendor a whole library.
+
+## Reference tooling (included here)
+
+`sync-skills.js` — a dependency-free Node CLI that works against an **unmodified**
+server today by reading each entry's `location`:
+
+```
+node sync-skills.js <url> list     # inventory with categories
+node sync-skills.js <url> diff     # server vs local
+node sync-skills.js <url> pull     # server -> local
+node sync-skills.js <url> push     # local  -> server
+node sync-skills.js <url> prune    # drop local skills removed upstream
+```
+
+Run it as `.cjs` (or outside the repo) when the surrounding `package.json` sets
+`"type": "module"`. Once the endpoints above exist the same CLI can drop its
+filesystem dependency and work against a remote server.
+
+## Acceptance criteria
+
+- [ ] `GET /skill/paths` reports every active root with `kind` and skill count
+- [ ] `POST /skill/paths` makes a new directory's skills usable with **no restart**
+- [ ] `DELETE /skill/paths` removes only that root's skills
+- [ ] duplicate names resolve by the documented priority and are reported as shadowed
+- [ ] `persist: true` survives a restart; the default does not touch the config file
+- [ ] an empty / missing directory is a 400, not a silent no-op
+- [ ] `withContent` returns the text so a remote client can vendor the library
+
+## Backwards compatibility
+
+`skills.paths` keeps its current meaning (a boot-time `custom` root list), so
+existing configurations behave exactly as before; `GET /skill` without
+`withContent` keeps its present shape.