diff --git a/backend/cli/src/server/routes/settings/skills.ts b/backend/cli/src/server/routes/settings/skills.ts index c9bb23bb9..b7589dfc4 100644 --- a/backend/cli/src/server/routes/settings/skills.ts +++ b/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 /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: diff --git a/backend/cli/src/skill/skill.ts b/backend/cli/src/skill/skill.ts index 825a7df5b..1252ea1ed 100644 --- a/backend/cli/src/skill/skill.ts +++ b/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 + + /** Roots registered for this process only (never written to disk). */ + export const runtimeRoots = new Set() + + 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 { + const config = await Config.getExecution() + const configured = (config.skills?.paths ?? []).map(expandRoot) + const entries = await all() + const buckets = new Map() + 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.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 { + 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 } } + diff --git a/feature-notes/FEATURE-PROPOSAL.md b/feature-notes/FEATURE-PROPOSAL.md new file mode 100644 index 000000000..9b9787630 --- /dev/null +++ b/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": ""` 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 list # inventory with categories +node sync-skills.js diff # server vs local +node sync-skills.js pull # server -> local +node sync-skills.js push # local -> server +node sync-skills.js 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. diff --git a/feature-notes/sync-skills.js b/feature-notes/sync-skills.js new file mode 100644 index 000000000..de47dd92d --- /dev/null +++ b/feature-notes/sync-skills.js @@ -0,0 +1,207 @@ +#!/usr/bin/env node +/** + * OpenScience Local Skills Sync (v2) + * + * Pulls skills from the running server and saves/updates them locally. + * Since GET /skill doesn't return content, we read from the location paths. + * + * Usage: + * node sync-skills.js pull # server → local + * node sync-skills.js push # local → server + * node sync-skills.js diff # compare + * node sync-skills.js status # counts + */ +const http = require("http"); +const fs = require("fs"); +const path = require("path"); + +const API_URL = process.argv[2] || "http://127.0.0.1:4096"; +const LOCAL_DIR = path.join( + process.env.USERPROFILE || process.env.HOME, + ".local", "share", "openscience", "local-skills" +); +const ACTION = process.argv[3] || "pull"; + +// ── HTTP helpers ───────────────────────────────────────────────────────────── + +function httpGet(urlPath) { + return new Promise((resolve, reject) => { + const url = new URL(`${API_URL}${urlPath}`); + http.get({ hostname: url.hostname, port: url.port, path: url.pathname, timeout: 30_000 }, (res) => { + let data = ""; + res.on("data", (c) => (data += c)); + res.on("end", () => { + if (res.statusCode >= 200 && res.statusCode < 300) { + resolve(JSON.parse(data)); + } else { + reject(new Error(`HTTP ${res.statusCode}: ${data.slice(0, 300)}`)); + } + }); + }).on("error", reject); + }); +} + +function httpPut(name, content) { + return new Promise((resolve, reject) => { + const body = JSON.stringify({ content }); + const url = new URL(`${API_URL}/skill/${encodeURIComponent(name)}`); + const req = http.request({ + hostname: url.hostname, port: url.port, path: url.pathname, method: "PUT", + headers: { "Content-Type": "application/json", "Content-Length": Buffer.byteLength(body) }, + timeout: 60_000, + }, (res) => { + let data = ""; + res.on("data", (c) => (data += c)); + res.on("end", () => resolve({ status: res.statusCode, body: data })); + }); + req.on("timeout", () => { req.destroy(); reject(new Error("timeout")); }); + req.on("error", reject); + req.write(body); + req.end(); + }); +} + +// ── file helpers ───────────────────────────────────────────────────────────── + +function readSafe(filePath) { + try { return fs.readFileSync(filePath, "utf8"); } catch { return null; } +} + +function saveLocal(skill) { + const dir = path.join(LOCAL_DIR, skill.category || "unknown", skill.name); + fs.mkdirSync(dir, { recursive: true }); + fs.writeFileSync(path.join(dir, "SKILL.md"), skill.content, "utf8"); +} + +function readLocal(name) { + if (!fs.existsSync(LOCAL_DIR)) return null; + for (const cat of fs.readdirSync(LOCAL_DIR)) { + const file = path.join(LOCAL_DIR, cat, name, "SKILL.md"); + if (fs.existsSync(file)) { + return { file, content: readSafe(file), category: cat }; + } + } + return null; +} + +function listLocalSkills() { + const results = []; + if (!fs.existsSync(LOCAL_DIR)) return results; + for (const cat of fs.readdirSync(LOCAL_DIR)) { + const catDir = path.join(LOCAL_DIR, cat); + if (!fs.statSync(catDir).isDirectory()) continue; + for (const name of fs.readdirSync(catDir)) { + const file = path.join(catDir, name, "SKILL.md"); + if (fs.existsSync(file)) { + results.push({ name, category: cat, file, content: readSafe(file) }); + } + } + } + return results; +} + +// ── actions ────────────────────────────────────────────────────────────────── + +async function pull() { + console.log("Fetching skill list from server..."); + const data = await httpGet("/skill"); + const serverSkills = data.items || data; + console.log(`Server has ${serverSkills.length} skills`); + + let added = 0, updated = 0, unchanged = 0, missing = 0; + for (const sk of serverSkills) { + // Read content from the server's file path + const serverContent = readSafe(sk.location); + if (!serverContent) { + missing++; + continue; + } + + const local = readLocal(sk.name); + if (!local) { + saveLocal({ ...sk, content: serverContent }); + added++; + } else if (local.content !== serverContent) { + saveLocal({ ...sk, content: serverContent }); + updated++; + } else { + unchanged++; + } + process.stdout.write("."); + } + + console.log(`\n\nPull complete:`); + console.log(` Added: ${added}`); + console.log(` Updated: ${updated}`); + console.log(` Unchanged: ${unchanged}`); + console.log(` Missing: ${missing}`); + console.log(` Local dir: ${LOCAL_DIR}`); +} + +async function push() { + console.log("Scanning local skills..."); + const locals = listLocalSkills(); + console.log(`Found ${locals.length} local skills`); + + let pushed = 0, failed = 0; + for (const sk of locals) { + const res = await httpPut(sk.name, sk.content); + if (res.status >= 200 && res.status < 300) { + pushed++; + process.stdout.write("."); + } else { + failed++; + console.log(`\n FAIL ${sk.name}: ${res.status} ${res.body?.slice(0, 100)}`); + } + } + console.log(`\n\nPush complete: ${pushed} pushed, ${failed} failed`); +} + +async function diff() { + console.log("Comparing server vs local...\n"); + const data = await httpGet("/skill"); + const serverSkills = data.items || data; + const serverMap = new Map(serverSkills.map((s) => [s.name, s])); + const locals = listLocalSkills(); + const localMap = new Map(locals.map((s) => [s.name, s])); + + const serverOnly = [...serverMap.keys()].filter((n) => !localMap.has(n)); + const localOnly = [...localMap.keys()].filter((n) => !serverMap.has(n)); + const both = [...serverMap.keys()].filter((n) => localMap.has(n)); + + let different = 0, identical = 0; + for (const n of both) { + const serverContent = readSafe(serverMap.get(n).location); + const localContent = localMap.get(n).content; + if (serverContent && serverContent !== localContent) different++; + else identical++; + } + + console.log(`Server: ${serverSkills.length} | Local: ${locals.length}`); + console.log(`\nServer-only (not local): ${serverOnly.length}`); + serverOnly.forEach((n) => console.log(` + ${n}`)); + console.log(`\nLocal-only (not server): ${localOnly.length}`); + localOnly.forEach((n) => console.log(` - ${n}`)); + console.log(`\nDifferent content: ${different}`); + console.log(`Identical: ${identical}`); +} + +async function status() { + const data = await httpGet("/skill"); + const serverCount = (data.items || data).length; + const localCount = listLocalSkills().length; + console.log(`Server: ${serverCount} skills`); + console.log(`Local: ${localCount} skills`); + console.log(`Source: ${LOCAL_DIR}`); +} + +// ── main ───────────────────────────────────────────────────────────────────── + +const actions = { pull, push, diff, status }; +const fn = actions[ACTION]; +if (!fn) { + console.error(`Unknown action: ${ACTION}`); + console.error("Usage: node sync-skills.js [pull|push|diff|status]"); + process.exit(1); +} +fn().catch((err) => { console.error("Error:", err.message); process.exit(1); });