From eab4ba245dc69479f0629668d66fc5ac238b37c5 Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 16 Sep 2026 22:14:30 -0700 Subject: [PATCH 1/7] refactor(skills): remove workbench registry HTTP surface (CL-8086) Skills are native kind:skill hub assets served by the stock asset routes. Delete the registry, its routes, the git-history version store, and the per-principal scope layer from @corbits/skills, drop the hub mount, and repoint agent-directory, onboarding, workflows, and the skill tool bundles at asset metadata. --- .../agent-directory/src/agent-workflow.ts | 2 +- .../agent-directory/src/definition-history.ts | 58 +- packages/agent-directory/src/index.ts | 1 + packages/agent-directory/src/routes.ts | 17 +- packages/agent-directory/test/routes.test.ts | 10 +- packages/onboarding/src/desired-state.ts | 17 +- packages/plugins-ui/README.md | 4 +- packages/plugins-ui/src/plugins-gallery.tsx | 36 +- packages/plugins-ui/src/skill-card.tsx | 20 +- .../plugins-ui/test/plugins-gallery.test.tsx | 9 +- packages/skills-tools/package.json | 2 +- packages/skills-tools/src/client.test.ts | 163 +----- packages/skills-tools/src/client.ts | 149 ++--- packages/skills-tools/src/tool.test.ts | 172 +----- packages/skills/package.json | 14 +- packages/skills/src/asset-history.ts | 55 -- packages/skills/src/asset-store.ts | 66 --- packages/skills/src/hub-asset-store.ts | 180 ------ packages/skills/src/index.ts | 34 -- packages/skills/src/registry.ts | 549 ------------------ packages/skills/src/routes.ts | 257 -------- packages/skills/src/workflow-routes.ts | 204 ------- packages/skills/test/asset-history.test.ts | 77 --- packages/skills/test/fakes.ts | 179 ------ packages/skills/test/registry.test.ts | 412 ------------- packages/skills/test/routes.test.ts | 301 ---------- packages/skills/test/workflow-routes.test.ts | 253 -------- packages/tools-skills/package.json | 2 +- packages/tools-skills/src/client.test.ts | 137 +---- packages/tools-skills/src/client.ts | 144 +---- packages/tools-skills/src/index.ts | 1 - packages/tools-skills/src/tool.test.ts | 66 +-- workflows/assistant/src/index.ts | 2 +- 33 files changed, 257 insertions(+), 3336 deletions(-) delete mode 100644 packages/skills/src/asset-history.ts delete mode 100644 packages/skills/src/asset-store.ts delete mode 100644 packages/skills/src/hub-asset-store.ts delete mode 100644 packages/skills/src/registry.ts delete mode 100644 packages/skills/src/routes.ts delete mode 100644 packages/skills/src/workflow-routes.ts delete mode 100644 packages/skills/test/asset-history.test.ts delete mode 100644 packages/skills/test/fakes.ts delete mode 100644 packages/skills/test/registry.test.ts delete mode 100644 packages/skills/test/routes.test.ts delete mode 100644 packages/skills/test/workflow-routes.test.ts diff --git a/packages/agent-directory/src/agent-workflow.ts b/packages/agent-directory/src/agent-workflow.ts index 2d9fec1916..947efa52e7 100644 --- a/packages/agent-directory/src/agent-workflow.ts +++ b/packages/agent-directory/src/agent-workflow.ts @@ -46,7 +46,7 @@ export const AGENT_DEFINITION_STEP_ID = "agent"; */ export const SKILLS_TOOL_PACKAGE_PIN = { name: "@corbits/tools-skills", - version: "0.0.2", + version: "0.0.3", } as const; /** diff --git a/packages/agent-directory/src/definition-history.ts b/packages/agent-directory/src/definition-history.ts index a61e27c6f7..36dd48f329 100644 --- a/packages/agent-directory/src/definition-history.ts +++ b/packages/agent-directory/src/definition-history.ts @@ -1,17 +1,59 @@ -// A definition's `workflow`-kind asset carries its own git history, read -// through `@corbits/skills`' shared `readAssetCommitHistory` — the same -// commit-log walk `createHubSkillAssetStore` uses for `kind:"skill"` -// assets. The git history IS the version store here too: no separate -// versions table, and restoring a version re-commits an older commit's -// blobs onto the default ref rather than rewriting history. +// A definition's `workflow`-kind asset carries its own git history. The +// git history IS the version store here: no separate versions table, and +// restoring a version re-commits an older commit's blobs onto the default +// ref rather than rewriting history. import fs from "node:fs"; import git from "isomorphic-git"; -import { readAssetCommitHistory } from "@corbits/skills"; -import { DEFAULT_ASSET_REF, type RepoStore } from "@intx/hub-sessions"; +import { + DEFAULT_ASSET_REF, + type RepoId, + type RepoStore, +} from "@intx/hub-sessions"; const AGENT_DEFINITION_ASSET_KIND = "workflow"; +// The hub genesis commit message — substrate scaffolding from +// `createAsset` → `initRepo`, never a person-saved version, so it never +// surfaces in the version list a saver reads back. +const ASSET_GENESIS_COMMIT_MESSAGE = "Initialize repository"; + +function isAssetGenesisCommit(message: string): boolean { + return message.trim() === ASSET_GENESIS_COMMIT_MESSAGE; +} + +async function readAssetCommitHistory(input: { + readonly repoStore: RepoStore; + readonly kind: RepoId["kind"]; + readonly assetId: string; + readonly ref: string; +}): Promise { + const dir = await input.repoStore.getRepoDir({ + kind: input.kind, + id: input.assetId, + }); + let entries: Awaited>; + try { + entries = await git.log({ fs, dir, ref: input.ref }); + } catch { + return []; + } + const commits: DefinitionCommit[] = []; + for (const entry of entries) { + const message = entry.commit.message.trim(); + if (isAssetGenesisCommit(message)) continue; + commits.push({ + commitSha: entry.oid, + message, + author: entry.commit.author.name, + committedAtIso: new Date( + entry.commit.author.timestamp * 1000, + ).toISOString(), + }); + } + return commits; +} + /** One commit on a definition's asset default ref. */ export type DefinitionCommit = { readonly commitSha: string; diff --git a/packages/agent-directory/src/index.ts b/packages/agent-directory/src/index.ts index 08abd0a119..7c219dc473 100644 --- a/packages/agent-directory/src/index.ts +++ b/packages/agent-directory/src/index.ts @@ -52,6 +52,7 @@ export { createAgentDefinitionRoutes, type CreateAgentDefinitionRoutesDeps, type PinnedSkillIndexResolver, + SkillIndexResolutionError, } from "./routes"; export { createDefinitionAssetHistory, diff --git a/packages/agent-directory/src/routes.ts b/packages/agent-directory/src/routes.ts index 7a315b75ae..e50cf5bbd6 100644 --- a/packages/agent-directory/src/routes.ts +++ b/packages/agent-directory/src/routes.ts @@ -25,10 +25,7 @@ import { idResource } from "@intx/hub-api"; import { DEFAULT_ASSET_REF } from "@intx/hub-sessions"; import type { AssetService } from "@intx/hub-sessions"; -import { - SkillRegistryError, - type PinnedSkillIndexEntry, -} from "@corbits/skills"; +import { type PinnedSkillIndexEntry } from "@corbits/skills"; import { isWorkbenchHostDefinitionName } from "@corbits/chat/workbench-host-naming"; import { @@ -88,6 +85,16 @@ export type PinnedSkillIndexResolver = { ): Promise; }; +/** Thrown by a `PinnedSkillIndexResolver` when a pinned name can't be + * resolved against the tenant's native skill assets — a bad request from + * the person editing the agent, not a server fault. */ +export class SkillIndexResolutionError extends Error { + constructor(message: string) { + super(message); + this.name = "SkillIndexResolutionError"; + } +} + export type CreateAgentDefinitionRoutesDeps = { db: DB["db"]; assetService: AssetService; @@ -149,7 +156,7 @@ export function createAgentDefinitionRoutes({ // the person editing the agent, not a server fault — surface either as // one rather than letting it read as a 500. app.onError((err, c) => { - if (err instanceof SkillRegistryError) { + if (err instanceof SkillIndexResolutionError) { return c.json( makeErrorEnvelope({ code: "bad_request", userMessage: err.message }), 400, diff --git a/packages/agent-directory/test/routes.test.ts b/packages/agent-directory/test/routes.test.ts index 45e2de706b..35987aa207 100644 --- a/packages/agent-directory/test/routes.test.ts +++ b/packages/agent-directory/test/routes.test.ts @@ -15,7 +15,6 @@ import { AssetServiceError } from "@intx/hub-sessions"; import type { AssetService } from "@intx/hub-sessions"; import type { DB } from "@intx/db"; -import { SkillRegistryError } from "@corbits/skills"; import { CORBITS_TOOLS_REGISTRY } from "@corbits/tool-registry-publish"; import { @@ -30,7 +29,10 @@ import { readAgentDefinitionWorkflowJson, RetiredWorkflowEnvelopeError, } from "../src/definition-asset"; -import { createAgentDefinitionRoutes } from "../src/routes"; +import { + createAgentDefinitionRoutes, + SkillIndexResolutionError, +} from "../src/routes"; import type { PinnedSkillIndexResolver } from "../src/routes"; import { createWorkflowSkillPinRoutes, @@ -1554,7 +1556,7 @@ test("a create request indexes its pinned skills into the stored system prompt", // The prompt tells the model to call `skills_load`, so the bundle that // provides it must be pinned on the same push. expect(pinsFrom(workflowJson)).toEqual([ - { name: "@corbits/tools-skills", version: "0.0.2" }, + { name: "@corbits/tools-skills", version: "0.0.3" }, ]); }); @@ -1565,7 +1567,7 @@ test("pinning a skill the registry cannot resolve is a 400, not a 500", async () skillIndex: { resolve: () => Promise.reject( - new SkillRegistryError("not_found", 'cannot pin skill "ghost"'), + new SkillIndexResolutionError('cannot pin skill "ghost"'), ), }, history: fakeHistory(), diff --git a/packages/onboarding/src/desired-state.ts b/packages/onboarding/src/desired-state.ts index 822048bac8..752b6e6b2c 100644 --- a/packages/onboarding/src/desired-state.ts +++ b/packages/onboarding/src/desired-state.ts @@ -190,16 +190,21 @@ async function readSkillState( skill: SkillPin, ): Promise { try { - const existing = await api( + const response = await api( "GET", - `/api/tenants/${tenantId}/skills/${encodeURIComponent(skill.name)}`, + `/api/tenants/${tenantId}/assets?kind=skill&inherited=false`, undefined, cookies, ); - if (existing.status === 200) return "present"; - // A 502-class response (or a thrown sidecar-unavailable error) is - // the sidecar-unavailable class: blocked, not pending. - return existing.status >= 500 ? "blocked" : "pending"; + if (response.status >= 500) return "blocked"; + const assets = parseAs( + AssetWithOriginResponse.array(), + response.data, + "assets response", + ); + return assets.some((asset) => asset.name === skill.name) + ? "present" + : "pending"; } catch (cause) { if (isSidecarUnavailableError(cause)) return "blocked"; throw cause; diff --git a/packages/plugins-ui/README.md b/packages/plugins-ui/README.md index a678caa0b8..e7ab869941 100644 --- a/packages/plugins-ui/README.md +++ b/packages/plugins-ui/README.md @@ -28,8 +28,8 @@ owns fetching data and passes it down. installed strip, and a Featured-then-category card grid. - `plugin-card.tsx` — one plugin's icon, name, outcome sentence, and a single action that reads honestly off the connector's real status. -- `skill-card.tsx` — a skill given the same card treatment, with a - "shared"/"private" scope badge. +- `skill-card.tsx` — a skill given the same card treatment (CL-8086: no + scope badge — the stock asset routes carry no scope/visibility flag). - `installed-strip.tsx` — the at-a-glance row of already-connected plugins above the search/category grid. - `plugin-connect-panel.tsx` — the right-docked connect flow for every diff --git a/packages/plugins-ui/src/plugins-gallery.tsx b/packages/plugins-ui/src/plugins-gallery.tsx index 9dff2f4257..6e8c6ec57c 100644 --- a/packages/plugins-ui/src/plugins-gallery.tsx +++ b/packages/plugins-ui/src/plugins-gallery.tsx @@ -234,34 +234,16 @@ function SkillsTabPanel({ ); } - const groups = [ - { - label: "Shared with everyone", - skills: filtered.filter((skill) => skill.scope === "tenant"), - }, - { - label: "Just you", - skills: filtered.filter((skill) => skill.scope === "private"), - }, - ].filter((group) => group.skills.length > 0); - + // CL-8086: no scope groups — the stock asset routes carry no + // scope/visibility flag, so every visible skill renders in one flat list. return ( -
- {groups.map((group) => ( -
-

- {group.label} -

-
- {group.skills.map((skill) => ( - onOpen(skill)} - /> - ))} -
-
+
+ {filtered.map((skill) => ( + onOpen(skill)} + /> ))}
); diff --git a/packages/plugins-ui/src/skill-card.tsx b/packages/plugins-ui/src/skill-card.tsx index 56ac47b9ae..f850bf7227 100644 --- a/packages/plugins-ui/src/skill-card.tsx +++ b/packages/plugins-ui/src/skill-card.tsx @@ -1,10 +1,11 @@ // A skill, given the same dense row treatment as a plugin (owner ruling, // CL-6090; density CL-6272.1): icon, name, its one-line description -// truncated, a scope caption in the same plain words the plugin -// provenance captions use — "shared" reads as the tenant-wide set an -// agent can pin, mirroring a plugin connected at the parent tenant; -// "private" is yours alone, same shape as a plugin this workbench -// connected for itself — and a quiet view affordance. +// truncated, and a quiet view affordance. +// +// CL-8086: skills are native `kind:"skill"` hub assets now and the stock +// asset routes carry no scope/visibility flag, so the "Shared with +// everyone" / "Just you" caption the card used to mirror from the plugin +// provenance captions is gone — one flat list, no scope groups. import { Button } from "@corbits/react-ui"; import { Lightning } from "@corbits/icons"; @@ -13,12 +14,6 @@ export type SkillCardData = { readonly assetId: string; readonly name: string; readonly description: string; - readonly scope: "private" | "tenant"; -}; - -const SCOPE_LABEL: Record = { - tenant: "Shared with everyone", - private: "Just you", }; export function SkillCard({ @@ -53,9 +48,6 @@ export function SkillCard({ {skill.description}
- - {SCOPE_LABEL[skill.scope]} - diff --git a/packages/plugins-ui/test/plugins-gallery.test.tsx b/packages/plugins-ui/test/plugins-gallery.test.tsx index 1996b44cb9..5eefc3dda9 100644 --- a/packages/plugins-ui/test/plugins-gallery.test.tsx +++ b/packages/plugins-ui/test/plugins-gallery.test.tsx @@ -78,13 +78,11 @@ const SKILLS: readonly SkillCardData[] = [ assetId: "skill_1", name: "Weekly digest", description: "Summarizes the week's channel activity.", - scope: "tenant", }, { assetId: "skill_2", name: "Draft replies", description: "Drafts a reply in my voice.", - scope: "private", }, ]; @@ -389,9 +387,12 @@ describe("PluginsGallery", () => { skillsTab?.click(); }); + // CL-8086: one flat list — the stock asset routes carry no + // scope/visibility flag, so there are no scope group headings. expect(container.textContent).toContain("Weekly digest"); - expect(container.textContent).toContain("Shared with everyone"); - expect(container.textContent).toContain("Just you"); + expect(container.textContent).toContain("Draft replies"); + expect(container.textContent).not.toContain("Shared with everyone"); + expect(container.textContent).not.toContain("Just you"); }); test("every preset returned by the route reaches a fresh catalog", async () => { diff --git a/packages/skills-tools/package.json b/packages/skills-tools/package.json index 0085472be4..c4d70131d3 100644 --- a/packages/skills-tools/package.json +++ b/packages/skills-tools/package.json @@ -2,7 +2,7 @@ "name": "@corbits/skills-tools", "private": true, "description": "The list_skills/create_skill/update_skill/pin_skill @intx/agent tool bundle: Myra's in-chat way to capture repeatable know-how as a workbench skill and pin an existing one onto an agent definition, over @corbits/skills' asset store and @corbits/agent-directory's definition_skills store", - "version": "0.0.6", + "version": "0.0.7", "license": "LGPL-2.1-or-later", "type": "module", "exports": { diff --git a/packages/skills-tools/src/client.test.ts b/packages/skills-tools/src/client.test.ts index 2a926030bc..a255a51dda 100644 --- a/packages/skills-tools/src/client.test.ts +++ b/packages/skills-tools/src/client.test.ts @@ -19,159 +19,40 @@ function testConfig(fetchImpl: typeof fetch): SkillsToolClientConfig { }; } -test("listSkills gets the workflow-skills index with sidecar auth", async () => { - let seenUrl: string | undefined; - let seenHeaders: Record | undefined; - const fetchImpl = (async (url: string | URL, init?: RequestInit) => { - seenUrl = String(url); - seenHeaders = init?.headers as Record; - return new Response( - JSON.stringify({ - data: [{ name: "triage", description: "Sorts inbound issues." }], - }), - ); - }) as unknown as typeof fetch; - - const skills = await listSkills(testConfig(fetchImpl)); - - expect(seenUrl).toBe("https://hub.example.com/api/workflow-skills/list"); - expect(seenHeaders?.["authorization"]).toBe("Bearer sc-token"); - expect(seenHeaders?.["x-workflow-run-address"]).toBe("run_1@workflow"); - expect(skills).toEqual([ - { name: "triage", description: "Sorts inbound issues." }, - ]); -}); - -test("listSkills throws an honest error on a non-ok response", async () => { - const fetchImpl = (async () => - new Response("", { - status: 500, - statusText: "Internal Server Error", - })) as unknown as typeof fetch; - - await expect(listSkills(testConfig(fetchImpl))).rejects.toThrow(/500/); -}); - -test("loadSkill posts the name to /load and returns the skill with its full body", async () => { - let seenUrl: string | undefined; - let seenBody: unknown; - const fetchImpl = (async (url: string | URL, init?: RequestInit) => { - seenUrl = String(url); - seenBody = JSON.parse(String(init?.body)); - return new Response( - JSON.stringify({ - data: { - name: "triage", - description: "Sorts inbound issues.", - body: "## Steps\nAlways label severity first.", - }, - }), - ); - }) as unknown as typeof fetch; - - const skill = await loadSkill(testConfig(fetchImpl), "triage"); - - expect(seenUrl).toBe("https://hub.example.com/api/workflow-skills/load"); - expect(seenBody).toEqual({ name: "triage" }); - expect(skill.body).toContain("Always label severity first."); +// CL-8086: the workflow-skills HTTP surface these four calls used to +// reach was deleted, and no stock Interchange route yet serves skill +// content, so each fails closed with an explicit error instead of +// reaching a dead route. +const unreachableFetch = (() => { + throw new Error("must not be called: no stock route exists to call"); +}) as unknown as typeof fetch; + +test("listSkills fails closed naming the missing stock route", async () => { + await expect(listSkills(testConfig(unreachableFetch))).rejects.toThrow( + /no stock Interchange HTTP route/, + ); }); -test("createSkill posts to /create and parses back the created summary", async () => { - let seenUrl: string | undefined; - let seenBody: unknown; - const fetchImpl = (async (url: string | URL, init?: RequestInit) => { - seenUrl = String(url); - seenBody = JSON.parse(String(init?.body)); - return new Response( - JSON.stringify({ - data: { - assetId: "asset_1", - name: "triage", - description: "Sorts inbound issues.", - scope: "tenant", - creatorPrincipalId: "prn_1", - updatedAtIso: "2026-01-01T00:00:00.000Z", - }, - }), - ); - }) as unknown as typeof fetch; - - const skill = await createSkill(testConfig(fetchImpl), { - name: "triage", - description: "Sorts inbound issues.", - body: "Read the report.", - }); - - expect(seenUrl).toBe("https://hub.example.com/api/workflow-skills/create"); - expect(seenBody).toEqual({ - name: "triage", - description: "Sorts inbound issues.", - body: "Read the report.", - }); - expect(skill.name).toBe("triage"); - expect(skill.scope).toBe("tenant"); +test("loadSkill fails closed naming the missing stock route", async () => { + await expect( + loadSkill(testConfig(unreachableFetch), "triage"), + ).rejects.toThrow(/no stock Interchange HTTP route/); }); -test("createSkill throws on a non-ok response, never fabricating a summary", async () => { - const fetchImpl = (async () => - new Response( - JSON.stringify({ - error: { code: "conflict", message: "already exists" }, - }), - { status: 409 }, - )) as unknown as typeof fetch; - +test("createSkill fails closed naming the missing stock route", async () => { await expect( - createSkill(testConfig(fetchImpl), { + createSkill(testConfig(unreachableFetch), { name: "triage", description: "d", body: "b", }), - ).rejects.toThrow(/409/); -}); - -test("updateSkill posts to /update, omitting description when not given", async () => { - let seenBody: unknown; - const fetchImpl = (async (_url: string | URL, init?: RequestInit) => { - seenBody = JSON.parse(String(init?.body)); - return new Response( - JSON.stringify({ - data: { - assetId: "asset_1", - name: "triage", - description: "Sorts inbound issues.", - scope: "tenant", - creatorPrincipalId: "prn_1", - updatedAtIso: "2026-01-02T00:00:00.000Z", - }, - }), - ); - }) as unknown as typeof fetch; - - const skill = await updateSkill(testConfig(fetchImpl), { - name: "triage", - body: "Read the report. Pick one label.", - }); - - expect(seenBody).toEqual({ - name: "triage", - body: "Read the report. Pick one label.", - }); - expect(skill.description).toBe("Sorts inbound issues."); + ).rejects.toThrow(/no stock Interchange HTTP route/); }); -test("updateSkill throws a 404 as a plain error when the skill doesn't exist", async () => { - const fetchImpl = (async () => - new Response( - JSON.stringify({ - error: { code: "not_found", message: "no such skill" }, - }), - { status: 404 }, - )) as unknown as typeof fetch; - +test("updateSkill fails closed naming the missing stock route", async () => { await expect( - updateSkill(testConfig(fetchImpl), { name: "ghost", body: "b" }), - ).rejects.toThrow(/404/); + updateSkill(testConfig(unreachableFetch), { name: "triage", body: "b" }), + ).rejects.toThrow(/no stock Interchange HTTP route/); }); test("pinSkill posts to the agent-directory workflow-skill-pins surface", async () => { diff --git a/packages/skills-tools/src/client.ts b/packages/skills-tools/src/client.ts index 43e2481e61..45b8d5456f 100644 --- a/packages/skills-tools/src/client.ts +++ b/packages/skills-tools/src/client.ts @@ -1,15 +1,20 @@ -// A minimal client for the two workflow-run-authenticated surfaces -// `@corbits/skills-tools`' tools call: `@corbits/skills`' -// `createWorkflowSkillRoutes` (mounted in `apps/hub` at -// `/api/workflow-skills`, already serving read-only `list`/`search`/ -// `load` — this client adds the two write endpoints, -// `POST /create`/`POST /update`, that same route file now also serves) -// and `@corbits/agent-directory`'s `createWorkflowSkillPinRoutes` -// (mounted at `/api/workflow-skill-pins`, `POST /pin`). +// A minimal client for the workflow-run-authenticated surfaces +// `@corbits/skills-tools`' tools call. // -// Same auth-header/error-handling/arktype-parsing shape as -// `@corbits/capability-tools`' `client.ts`: a sidecar bearer token plus -// the run's own address, never a model-supplied identity. +// CL-8086: `@corbits/skills`' `createWorkflowSkillRoutes` (formerly +// mounted at `/api/workflow-skills`, serving `list`/`search`/`load` plus +// the `create`/`update` writes) was deleted — skills are native +// `kind:"skill"` hub assets now, and no stock Interchange route yet lets +// a workflow-run bearer identity read or write a skill's content +// (`@intx/hub-api`'s `routes/assets.ts` covers asset metadata and +// package-registry tarballs, not skill content). `listSkills`, +// `loadSkill`, `createSkill`, and `updateSkill` below fail closed with an +// explicit error naming that gap rather than reaching a dead route or +// inventing a result. +// +// `pinSkill` is unaffected: it calls `@corbits/agent-directory`'s +// `createWorkflowSkillPinRoutes` (mounted at `/api/workflow-skill-pins`), +// a live, separate surface this ticket does not touch. import { type } from "arktype"; export interface SkillsToolClientConfig { @@ -44,21 +49,6 @@ export type SkillIndexEntry = { readonly description: string; }; -const SkillSummaryResponse = type({ - data: { - assetId: "string", - name: "string", - description: "string", - scope: "'private' | 'tenant'", - creatorPrincipalId: "string", - updatedAtIso: "string", - }, -}); - -const SkillIndexResponse = type({ - data: type({ name: "string", description: "string" }).array(), -}); - const PinResponse = type({ skills: "string[]" }); export type SkillDetail = { @@ -67,10 +57,6 @@ export type SkillDetail = { readonly body: string; }; -const SkillDetailResponse = type({ - data: { name: "string", description: "string", body: "string" }, -}); - function authHeaders(config: SkillsToolClientConfig): Record { return { authorization: `Bearer ${config.sidecarToken}`, @@ -100,10 +86,6 @@ async function postJson( return response.json(); } -function skillsEndpoint(config: SkillsToolClientConfig, path: string): string { - return `${config.hubSkillsUrl}/api/workflow-skills${path}`; -} - function skillPinEndpoint( config: SkillsToolClientConfig, path: string, @@ -111,92 +93,57 @@ function skillPinEndpoint( return `${config.hubAgentDirectoryUrl}/api/workflow-skill-pins${path}`; } +const NO_STOCK_SKILL_CONTENT_ROUTE = + "Skill content has no stock Interchange HTTP route (CL-8086): the " + + "workbench-specific skills registry that used to serve it was removed, " + + "and no replacement has been added to @intx/hub-api yet."; + /** Every skill this run can see, index-only (name + description, no - * body) — mirrors `GET /list`'s own shape. */ -export async function listSkills( - config: SkillsToolClientConfig, + * body). No stock Interchange route serves this yet (CL-8086); fails + * closed rather than reaching a dead route or reading as an empty + * registry. */ +export function listSkills( + _config: SkillsToolClientConfig, ): Promise { - const doFetch = config.fetchImpl ?? fetch; - const response = await doFetch(skillsEndpoint(config, "/list"), { - headers: authHeaders(config), - }); - if (!response.ok) { - throw new Error( - `Listing skills failed: ${response.status} ${response.statusText}`, - ); - } - const body: unknown = await response.json(); - const parsed = SkillIndexResponse(body); - if (parsed instanceof type.errors) { - throw new Error( - `Skill list response did not match the expected shape: ${parsed.summary}`, - ); - } - return parsed.data; + return Promise.reject(new Error(NO_STOCK_SKILL_CONTENT_ROUTE)); } -/** Loads one skill's full body by name — the on-demand read behind - * `read_skill`, over the same `/load` route the pinned-skill - * `load_skill` tool uses. Throws on any transport, HTTP, or shape - * failure — never fabricates content. */ -export async function loadSkill( - config: SkillsToolClientConfig, - name: string, +/** Loads one skill's full body by name. No stock Interchange route + * serves this yet (CL-8086); fails closed rather than fabricating + * content. */ +export function loadSkill( + _config: SkillsToolClientConfig, + _name: string, ): Promise { - const body = await postJson(config, skillsEndpoint(config, "/load"), { - name, - }); - const parsed = SkillDetailResponse(body); - if (parsed instanceof type.errors) { - throw new Error( - `Load-skill response did not match the expected shape: ${parsed.summary}`, - ); - } - return parsed.data; + return Promise.reject(new Error(NO_STOCK_SKILL_CONTENT_ROUTE)); } -/** Creates a new, always tenant-scoped skill. Throws on any transport, - * HTTP, or shape failure — never fabricates success. */ -export async function createSkill( - config: SkillsToolClientConfig, - input: { +/** Creates a new, always tenant-scoped skill. No stock Interchange route + * accepts skill content yet (CL-8086); fails closed rather than + * fabricating success. */ +export function createSkill( + _config: SkillsToolClientConfig, + _input: { readonly name: string; readonly description: string; readonly body: string; }, ): Promise { - const body = await postJson(config, skillsEndpoint(config, "/create"), input); - const parsed = SkillSummaryResponse(body); - if (parsed instanceof type.errors) { - throw new Error( - `Create-skill response did not match the expected shape: ${parsed.summary}`, - ); - } - return parsed.data; + return Promise.reject(new Error(NO_STOCK_SKILL_CONTENT_ROUTE)); } -/** Republishes an existing skill's body, optionally its description — - * an omitted `description` leaves the skill's current one untouched - * (the route's own `/update` fills it in from the skill it loads - * first). Throws 404 the same way a bare fetch failure throws: as a - * plain `Error`, on any skill name this run cannot see or that does - * not exist. */ -export async function updateSkill( - config: SkillsToolClientConfig, - input: { +/** Republishes an existing skill's body. No stock Interchange route + * accepts skill content yet (CL-8086); fails closed rather than + * fabricating success. */ +export function updateSkill( + _config: SkillsToolClientConfig, + _input: { readonly name: string; readonly body: string; readonly description?: string; }, ): Promise { - const body = await postJson(config, skillsEndpoint(config, "/update"), input); - const parsed = SkillSummaryResponse(body); - if (parsed instanceof type.errors) { - throw new Error( - `Update-skill response did not match the expected shape: ${parsed.summary}`, - ); - } - return parsed.data; + return Promise.reject(new Error(NO_STOCK_SKILL_CONTENT_ROUTE)); } /** Pins a skill name onto any definition in this run's own tenant. diff --git a/packages/skills-tools/src/tool.test.ts b/packages/skills-tools/src/tool.test.ts index c8b50d2845..07b3118982 100644 --- a/packages/skills-tools/src/tool.test.ts +++ b/packages/skills-tools/src/tool.test.ts @@ -67,29 +67,6 @@ test('the two reads and the two grant-free writes (create_skill, update_skill) c ]); }); -test("read_skill returns the skill's full body", async () => { - const bundle = skillsTools(testEnv()); - const result = await withFetch( - (async () => - new Response( - JSON.stringify({ - data: { - name: "triage", - description: "Sorts inbound issues.", - body: "## Steps\nAlways label severity first.", - }, - }), - )) as unknown as typeof fetch, - () => - bundle.run( - callFor(READ_SKILL_TOOL, { name: "triage" }), - new AbortController().signal, - ), - ); - expect(result.isError).toBeFalsy(); - expect(result.content).toContain("Always label severity first."); -}); - test("read_skill rejects a call missing the name without calling out", async () => { const bundle = skillsTools(testEnv()); const result = await bundle.run( @@ -100,50 +77,28 @@ test("read_skill rejects a call missing the name without calling out", async () expect(result.content).toMatch(/invalid input/); }); -test("read_skill surfaces a 404 as an honest tool error", async () => { +// CL-8086: the workflow-skills HTTP surface these tools used to call was +// deleted, and no stock Interchange route yet serves skill content, so +// each fails closed with an explicit error instead of a fabricated +// result. +test("read_skill surfaces the missing stock route as an honest tool error", async () => { const bundle = skillsTools(testEnv()); - const result = await withFetch( - (async () => - new Response("", { - status: 404, - statusText: "Not Found", - })) as unknown as typeof fetch, - () => - bundle.run( - callFor(READ_SKILL_TOOL, { name: "ghost" }), - new AbortController().signal, - ), + const result = await bundle.run( + callFor(READ_SKILL_TOOL, { name: "triage" }), + new AbortController().signal, ); expect(result.isError).toBe(true); - expect(result.content).toMatch(/404/); + expect(result.content).toMatch(/no stock Interchange HTTP route/); }); -test("list_skills reports each skill's name and description", async () => { +test("list_skills surfaces the missing stock route as an honest tool error, never an empty list", async () => { const bundle = skillsTools(testEnv()); - const result = await withFetch( - (async () => - new Response( - JSON.stringify({ - data: [{ name: "triage", description: "Sorts inbound issues." }], - }), - )) as unknown as typeof fetch, - () => - bundle.run(callFor(LIST_SKILLS_TOOL, {}), new AbortController().signal), - ); - expect(result.isError).toBeFalsy(); - expect(result.content).toContain("triage: Sorts inbound issues."); -}); - -test("list_skills reports plainly when the workbench has no skills yet", async () => { - const bundle = skillsTools(testEnv()); - const result = await withFetch( - (async () => - new Response(JSON.stringify({ data: [] }))) as unknown as typeof fetch, - () => - bundle.run(callFor(LIST_SKILLS_TOOL, {}), new AbortController().signal), + const result = await bundle.run( + callFor(LIST_SKILLS_TOOL, {}), + new AbortController().signal, ); - expect(result.isError).toBeFalsy(); - expect(result.content).toMatch(/no skills/i); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/no stock Interchange HTTP route/); }); test("create_skill rejects a call missing a required field without calling out", async () => { @@ -156,95 +111,28 @@ test("create_skill rejects a call missing a required field without calling out", expect(result.content).toMatch(/invalid input/); }); -test("create_skill posts the exact input and reports the created skill's name", async () => { +test("create_skill surfaces the missing stock route as an honest tool error, never a fabricated success", async () => { const bundle = skillsTools(testEnv()); - let seenUrl: string | undefined; - let seenBody: unknown; - const result = await withFetch( - (async (url: string | URL, init?: RequestInit) => { - seenUrl = String(url); - seenBody = JSON.parse(String(init?.body)); - return new Response( - JSON.stringify({ - data: { - assetId: "asset_1", - name: "triage", - description: "Sorts inbound issues.", - scope: "tenant", - creatorPrincipalId: "prn_1", - updatedAtIso: "2026-01-01T00:00:00.000Z", - }, - }), - ); - }) as unknown as typeof fetch, - () => - bundle.run( - callFor(CREATE_SKILL_TOOL, { - name: "triage", - description: "Sorts inbound issues.", - body: "Read the report.", - }), - new AbortController().signal, - ), - ); - expect(seenUrl).toBe("https://hub.example.com/api/workflow-skills/create"); - expect(seenBody).toEqual({ - name: "triage", - description: "Sorts inbound issues.", - body: "Read the report.", - }); - expect(result.isError).toBeFalsy(); - expect(result.content).toBe('Created the "triage" skill.'); -}); - -test("update_skill omits description from the request body when not supplied", async () => { - const bundle = skillsTools(testEnv()); - let seenBody: unknown; - const result = await withFetch( - (async (_url: string | URL, init?: RequestInit) => { - seenBody = JSON.parse(String(init?.body)); - return new Response( - JSON.stringify({ - data: { - assetId: "asset_1", - name: "triage", - description: "Sorts inbound issues.", - scope: "tenant", - creatorPrincipalId: "prn_1", - updatedAtIso: "2026-01-02T00:00:00.000Z", - }, - }), - ); - }) as unknown as typeof fetch, - () => - bundle.run( - callFor(UPDATE_SKILL_TOOL, { name: "triage", body: "New body." }), - new AbortController().signal, - ), + const result = await bundle.run( + callFor(CREATE_SKILL_TOOL, { + name: "triage", + description: "Sorts inbound issues.", + body: "Read the report.", + }), + new AbortController().signal, ); - expect(seenBody).toEqual({ name: "triage", body: "New body." }); - expect(result.isError).toBeFalsy(); - expect(result.content).toBe('Updated the "triage" skill.'); + expect(result.isError).toBe(true); + expect(result.content).toMatch(/no stock Interchange HTTP route/); }); -test("update_skill surfaces a 404 from the route as an honest tool error, never a fabricated success", async () => { +test("update_skill surfaces the missing stock route as an honest tool error, never a fabricated success", async () => { const bundle = skillsTools(testEnv()); - const result = await withFetch( - (async () => - new Response( - JSON.stringify({ - error: { code: "not_found", message: "no such skill" }, - }), - { status: 404 }, - )) as unknown as typeof fetch, - () => - bundle.run( - callFor(UPDATE_SKILL_TOOL, { name: "ghost", body: "b" }), - new AbortController().signal, - ), + const result = await bundle.run( + callFor(UPDATE_SKILL_TOOL, { name: "triage", body: "New body." }), + new AbortController().signal, ); expect(result.isError).toBe(true); - expect(result.content).toMatch(/404/); + expect(result.content).toMatch(/no stock Interchange HTTP route/); }); test("pin_skill reports the definition's full pinned-skill list after the pin", async () => { diff --git a/packages/skills/package.json b/packages/skills/package.json index 4b549a3e00..b24b199a94 100644 --- a/packages/skills/package.json +++ b/packages/skills/package.json @@ -1,8 +1,8 @@ { "name": "@corbits/skills", "private": true, - "description": "The skill registry over native kind:\"skill\" hub assets: SKILL.md authoring, direct create, git-history versions, per-principal access scoping, and the pinned-skills prompt index", - "version": "0.0.1", + "description": "SKILL.md authoring (parse/build against native kind:\"skill\" hub assets) and the pinned-skills prompt index. Storage, listing, and access to skill assets themselves are native Interchange asset routes, not this package.", + "version": "0.0.2", "license": "LGPL-2.1-or-later", "type": "module", "exports": { @@ -13,15 +13,7 @@ "test": "bun test" }, "dependencies": { - "@corbits/error-sink": "workspace:*", - "@intx/db": "workspace:*", - "@intx/hub-api": "workspace:*", - "@intx/hub-sessions": "workspace:*", - "arktype": "catalog:", - "drizzle-orm": "catalog:", - "hono": "catalog:", - "isomorphic-git": "catalog:", - "postgres": "catalog:" + "arktype": "catalog:" }, "devDependencies": { "@types/bun": "catalog:", diff --git a/packages/skills/src/asset-history.ts b/packages/skills/src/asset-history.ts deleted file mode 100644 index 47f38021b5..0000000000 --- a/packages/skills/src/asset-history.ts +++ /dev/null @@ -1,55 +0,0 @@ -// The commit-log walk shared by every asset kind whose version history IS -// its git history: `isomorphic-git`'s `git.log` over the repo `RepoStore` -// resolves for a given `{ kind, id }`. Both `@corbits/skills`' skill assets -// and `@corbits/agent-directory`'s workflow-kind definition assets read -// their history through this one walk. -// -// Consumer history omits the hub's genesis commit — every `createAsset` → -// `initRepo` path writes an "Initialize repository" scaffolding commit -// authored as interchange-hub. That is substrate, not a person-saved -// version, so it never surfaces in the version list a saver reads back. -import fs from "node:fs"; -import git from "isomorphic-git"; - -import type { RepoId, RepoStore } from "@intx/hub-sessions"; - -import type { SkillCommit } from "./asset-store"; - -/** The hub genesis commit message — substrate scaffolding, not a save. */ -export const ASSET_GENESIS_COMMIT_MESSAGE = "Initialize repository"; - -export function isAssetGenesisCommit(message: string): boolean { - return message.trim() === ASSET_GENESIS_COMMIT_MESSAGE; -} - -export async function readAssetCommitHistory(input: { - readonly repoStore: RepoStore; - readonly kind: RepoId["kind"]; - readonly assetId: string; - readonly ref: string; -}): Promise { - const dir = await input.repoStore.getRepoDir({ - kind: input.kind, - id: input.assetId, - }); - let entries: Awaited>; - try { - entries = await git.log({ fs, dir, ref: input.ref }); - } catch { - return []; - } - const commits: SkillCommit[] = []; - for (const entry of entries) { - const message = entry.commit.message.trim(); - if (isAssetGenesisCommit(message)) continue; - commits.push({ - commitSha: entry.oid, - message, - author: entry.commit.author.name, - committedAtIso: new Date( - entry.commit.author.timestamp * 1000, - ).toISOString(), - }); - } - return commits; -} diff --git a/packages/skills/src/asset-store.ts b/packages/skills/src/asset-store.ts deleted file mode 100644 index 2068c9f3d9..0000000000 --- a/packages/skills/src/asset-store.ts +++ /dev/null @@ -1,66 +0,0 @@ -// The narrow port the registry needs over native `kind:"skill"` hub -// assets. Everything the registry stores lives behind it: the asset row, -// the SKILL.md commit, and the asset repo's git history — which IS the -// version store. There is no versions table anywhere in this package. -export type SkillAssetRow = { - readonly id: string; - readonly tenantId: string; - readonly name: string; - readonly displayName: string | null; - readonly creatorPrincipalId: string | null; - readonly createdAt: Date; - readonly updatedAt: Date; -}; - -/** One commit on the skill asset's default ref. */ -export type SkillCommit = { - readonly commitSha: string; - readonly message: string; - readonly author: string; - readonly committedAtIso: string; -}; - -export type SkillAssetStore = { - create(input: { - readonly tenantId: string; - readonly name: string; - readonly displayName: string; - readonly creatorPrincipalId: string; - }): Promise; - /** - * Resolves a skill by name the way every native asset resolves: walking - * the tenant's ancestor chain and returning the first match, so a - * tenant's own skill shadows a same-named one inherited from a parent. - */ - findByName(tenantId: string, name: string): Promise; - /** - * The flat, non-inheriting lookup: only an asset declared directly on - * `tenantId`. Used for the one case that must never see an ancestor's - * asset — deciding whether `create` is naming a fresh skill or resuming - * this tenant's own half-finished one. - */ - findOwnByName(tenantId: string, name: string): Promise; - /** - * Every skill visible to `tenantId`, including those inherited from - * ancestors, with a descendant's own asset shadowing a same-named - * ancestor asset. - */ - listForTenant(tenantId: string): Promise; - writeSkillMd(input: { - readonly assetId: string; - readonly skillName: string; - readonly contents: string; - readonly message: string; - }): Promise<{ readonly commitSha: string }>; - readSkillMd(input: { - readonly assetId: string; - readonly skillName: string; - readonly commitSha?: string; - }): Promise; - history(assetId: string): Promise; -}; - -/** Path of a skill's SKILL.md inside its own asset tree. */ -export function skillMdPath(skillName: string): string { - return `${skillName}/SKILL.md`; -} diff --git a/packages/skills/src/hub-asset-store.ts b/packages/skills/src/hub-asset-store.ts deleted file mode 100644 index e410de9bac..0000000000 --- a/packages/skills/src/hub-asset-store.ts +++ /dev/null @@ -1,180 +0,0 @@ -// The production `SkillAssetStore`: native `kind:"skill"` assets driven -// through the platform's own `AssetService` and `RepoStore`, exactly the -// seam `@corbits/agent-directory` already uses to materialize a -// `workflow`-kind asset in-process. No git subprocess and no second -// content store. -// -// Version history reads the asset repo's commit log directly -// (`isomorphic-git` over `repoStore.getRepoDir`, the same handle the -// platform's own `readAssetBlob` resolves through). The git history IS -// the version store: restoring a version re-commits an older commit's -// SKILL.md onto the default ref, so the rewind is itself a new commit -// and nothing about the history is rewritten. -import fs from "node:fs"; -import { and, eq } from "drizzle-orm"; -import git from "isomorphic-git"; - -import type { DB } from "@intx/db"; -import { listAssetsForTenant, resolveAssetByName } from "@intx/db"; -import { asset as assetTable } from "@intx/db/schema"; -import { - AssetServiceError, - DEFAULT_ASSET_REF, - type AssetService, - type RepoStore, -} from "@intx/hub-sessions"; - -import { readAssetCommitHistory } from "./asset-history"; -import { - skillMdPath, - type SkillAssetRow, - type SkillAssetStore, -} from "./asset-store"; - -const SKILL_ASSET_KIND = "skill"; - -function rowToSkillAsset(row: typeof assetTable.$inferSelect): SkillAssetRow { - return { - id: row.id, - tenantId: row.tenantId, - name: row.name, - displayName: row.displayName, - creatorPrincipalId: row.creatorPrincipalId, - createdAt: row.createdAt, - updatedAt: row.updatedAt, - }; -} - -export type CreateHubSkillAssetStoreDeps = { - db: DB["db"]; - assetService: AssetService; - repoStore: RepoStore; -}; - -export function createHubSkillAssetStore( - deps: CreateHubSkillAssetStoreDeps, -): SkillAssetStore { - const { db, assetService, repoStore } = deps; - - async function repoDirFor(assetId: string): Promise { - return repoStore.getRepoDir({ kind: SKILL_ASSET_KIND, id: assetId }); - } - - return { - async create(input) { - const created = await assetService.createAsset({ - tenantId: input.tenantId, - kind: SKILL_ASSET_KIND, - name: input.name, - displayName: input.displayName, - creatorPrincipalId: input.creatorPrincipalId, - }); - return { - id: created.id, - tenantId: created.tenantId, - name: created.name, - displayName: created.displayName, - creatorPrincipalId: created.creatorPrincipalId, - createdAt: created.createdAt, - updatedAt: created.updatedAt, - }; - }, - - async findByName(tenantId, name) { - const row = await resolveAssetByName( - db, - tenantId, - SKILL_ASSET_KIND, - name, - ); - return row === null ? null : rowToSkillAsset(row); - }, - - async findOwnByName(tenantId, name) { - const row = await db.query.asset.findFirst({ - where: and( - eq(assetTable.tenantId, tenantId), - eq(assetTable.kind, SKILL_ASSET_KIND), - eq(assetTable.name, name), - ), - }); - return row === undefined ? null : rowToSkillAsset(row); - }, - - async listForTenant(tenantId) { - const rows = await listAssetsForTenant(db, tenantId, SKILL_ASSET_KIND); - return rows.map(rowToSkillAsset); - }, - - async writeSkillMd(input) { - const { commitSha } = await assetService.populateAsset({ - assetId: input.assetId, - ref: DEFAULT_ASSET_REF, - principal: { kind: "hub" }, - tree: { - files: { [skillMdPath(input.skillName)]: input.contents }, - message: input.message, - }, - }); - return { commitSha }; - }, - - async readSkillMd(input) { - const path = skillMdPath(input.skillName); - if (input.commitSha === undefined) { - try { - const bytes = await assetService.readAssetBlob({ - assetId: input.assetId, - path, - }); - return new TextDecoder().decode(bytes); - } catch (cause) { - if ( - cause instanceof AssetServiceError && - cause.reason === "not_found" - ) { - return null; - } - throw cause; - } - } - const dir = await repoDirFor(input.assetId); - try { - const { commit } = await git.readCommit({ - fs, - dir, - oid: input.commitSha, - }); - const { blob } = await git.readBlob({ - fs, - dir, - oid: commit.tree, - filepath: path, - }); - return new TextDecoder().decode(blob); - } catch { - return null; - } - }, - - async history(assetId) { - // RepoStore always commits as `interchange-hub` (fixed git identity). - // Only the skill's author may save, so attribute every consumer-visible - // version to that principal — never the machine account. - const commits = await readAssetCommitHistory({ - repoStore, - kind: SKILL_ASSET_KIND, - assetId, - ref: DEFAULT_ASSET_REF, - }); - const row = await db.query.asset.findFirst({ - where: eq(assetTable.id, assetId), - }); - const author = row?.creatorPrincipalId; - if (author === undefined || author === null || author === "") { - return commits; - } - return commits.map((commit) => ({ ...commit, author })); - }, - }; -} diff --git a/packages/skills/src/index.ts b/packages/skills/src/index.ts index bf087856f2..77385917df 100644 --- a/packages/skills/src/index.ts +++ b/packages/skills/src/index.ts @@ -1,25 +1,3 @@ -export { - SkillRegistryError, - createSkillRegistry, - type CreateSkillRegistryDeps, - type SkillCaller, - type SkillDetail, - type SkillRegistry, - type SkillRegistryErrorReason, - type SkillSummary, - type SkillVersion, -} from "./registry"; -export { readAssetCommitHistory, isAssetGenesisCommit } from "./asset-history"; -export { - skillMdPath, - type SkillAssetRow, - type SkillAssetStore, - type SkillCommit, -} from "./asset-store"; -export { - createHubSkillAssetStore, - type CreateHubSkillAssetStoreDeps, -} from "./hub-asset-store"; export { AVAILABLE_SKILLS_CLOSE_TAG, AVAILABLE_SKILLS_OPEN_TAG, @@ -29,11 +7,6 @@ export { withAvailableSkills, type PinnedSkillIndexEntry, } from "./prompt"; -export { - createSkillRoutes, - type CreateSkillRoutesDeps, - type PinnedByResolver, -} from "./routes"; export { SKILL_MD_FILENAME, SkillContentError, @@ -48,10 +21,3 @@ export { type SkillFrontmatter, type SkillScope, } from "./skill-md"; -export { - createWorkflowSkillRoutes, - type CreateWorkflowSkillRoutesDeps, - type WorkflowRunAuthenticator, - type WorkflowRunScope, - type WorkflowSkillsEnv, -} from "./workflow-routes"; diff --git a/packages/skills/src/registry.ts b/packages/skills/src/registry.ts deleted file mode 100644 index ae7eb3c4ce..0000000000 --- a/packages/skills/src/registry.ts +++ /dev/null @@ -1,549 +0,0 @@ -// The skill registry. One surface, one store: every skill is a native -// `kind:"skill"` hub asset carrying a single `/SKILL.md`, and every -// version of it is a commit on that asset's default ref. Visibility lives -// in the SKILL.md frontmatter — the asset's own bytes — so there is no -// side table to drift from the content it gates. Creating a skill creates -// that asset directly — there is no intermediate pending state. -import type { - SkillAssetRow, - SkillAssetStore, - SkillCommit, -} from "./asset-store"; -import { - buildSkillMd, - parseSkillMd, - skillDescriptionSchema, - skillNameSchema, - skillScopeSchema, - SkillContentError, - type ParsedSkillMd, - type SkillScope, -} from "./skill-md"; -import { type } from "arktype"; - -export type SkillRegistryErrorReason = - "not_found" | "forbidden" | "conflict" | "invalid"; - -export class SkillRegistryError extends Error { - readonly reason: SkillRegistryErrorReason; - /** The validator's raw diagnostic (an arktype summary can quote a regex - * literal) — for logs and debugging, never for the message a person - * reads, which stays plain language. */ - readonly details?: string | undefined; - constructor( - reason: SkillRegistryErrorReason, - message: string, - details?: string | undefined, - ) { - super(message); - this.name = "SkillRegistryError"; - this.reason = reason; - this.details = details; - } -} - -export type SkillCaller = { - readonly tenantId: string; - readonly principalId: string; -}; - -/** A skill as the registry resolved it: the parsed tip SKILL.md plus the - * asset it was read from. Scope and authorship both come from these two — - * nothing else is consulted. */ -type ResolvedSkill = { - readonly asset: SkillAssetRow; - readonly parsed: ParsedSkillMd; - readonly updatedAtIso: string; -}; - -export type SkillSummary = { - readonly assetId: string; - readonly name: string; - readonly description: string; - readonly scope: SkillScope; - readonly creatorPrincipalId: string; - readonly updatedAtIso: string; -}; - -export type SkillDetail = SkillSummary & { readonly body: string }; - -export type SkillVersion = SkillCommit & { readonly current: boolean }; - -export type SkillRegistry = { - list(caller: SkillCaller): Promise; - search(caller: SkillCaller, query: string): Promise; - load(caller: SkillCaller, name: string): Promise; - versions(caller: SkillCaller, name: string): Promise; - /** The skill exactly as it stood at one commit — what a diff against the - * current version is computed from. Read-only: nothing is written and no - * version is cut. */ - versionContent( - caller: SkillCaller, - name: string, - commitSha: string, - ): Promise; - restore( - caller: SkillCaller, - name: string, - commitSha: string, - ): Promise; - /** Republishes the skill's current content under a new scope as its own - * commit, so the previous scope stays restorable like any other version. - * Only the skill's own creator may rescope it. */ - setScope( - caller: SkillCaller, - name: string, - scope: SkillScope, - ): Promise; - create( - caller: SkillCaller, - input: { - readonly name: string; - readonly description: string; - readonly body: string; - readonly scope: SkillScope; - }, - ): Promise; - /** Republishes an existing skill's body/description as a new commit on - * its same asset, scope untouched — only the skill's own creator may - * `update` it. Distinct from `create`, which always 409s on a name a - * fully-formed skill already owns (see `create`'s own conflict - * handling). */ - update( - caller: SkillCaller, - name: string, - input: { - readonly description: string; - readonly body: string; - /** The commit the editor's content was read from. When given and no - * longer the current commit, the write is refused as a conflict - * rather than silently burying whoever saved in between. */ - readonly expectedHeadSha?: string | undefined; - }, - ): Promise; -}; - -export type CreateSkillRegistryDeps = { - assets: SkillAssetStore; -}; - -/** True when an arktype failure includes a regex `pattern` check — the - * one failure mode whose default summary quotes the raw regex literal, - * which is implementation detail no end user should see. */ -function isPatternFailure(errors: type.errors): boolean { - return errors.some((error) => error.code === "pattern"); -} - -function assertSkillName(raw: string): string { - const parsed = skillNameSchema(raw); - if (parsed instanceof type.errors) { - throw new SkillRegistryError( - "invalid", - isPatternFailure(parsed) - ? "Name must be lowercase letters, digits, and hyphens." - : `skill name ${JSON.stringify(raw)} is invalid: ${parsed.summary}`, - parsed.summary, - ); - } - return parsed; -} - -function assertDescription(raw: string): string { - const parsed = skillDescriptionSchema(raw); - if (parsed instanceof type.errors) { - throw new SkillRegistryError( - "invalid", - isPatternFailure(parsed) - ? "Description can't contain HTML tags." - : `skill description is invalid: ${parsed.summary}`, - parsed.summary, - ); - } - return parsed; -} - -function assertScope(raw: string): SkillScope { - const parsed = skillScopeSchema(raw); - if (parsed instanceof type.errors) { - throw new SkillRegistryError( - "invalid", - `skill scope ${JSON.stringify(raw)} is invalid: ${parsed.summary}`, - ); - } - return parsed; -} - -function contentErrorToRegistryError(cause: unknown): never { - if (cause instanceof SkillContentError) { - throw new SkillRegistryError("invalid", cause.message); - } - throw cause; -} - -/** - * A `tenant`-scoped skill is visible to every principal in the tenant - * that can already reach the asset; a `private` one only to the principal - * who created it, tenant notwithstanding — a private skill inherited from - * a parent stays invisible to everyone in the child but its author. - * - * This predicate only ever runs on assets `SkillAssetStore.findByName` - * and `SkillAssetStore.listForTenant` already resolved, both of which - * bound their results to the caller's own tenant plus its ancestors (the - * same chain-walk the native asset resolver uses) — so the tenant - * boundary is enforced once, at the resolution layer, not duplicated - * here. This function's whole job is the scope check. - */ -function isSkillVisibleTo(skill: ResolvedSkill, caller: SkillCaller): boolean { - if (skill.parsed.scope === "tenant") return true; - return skill.asset.creatorPrincipalId === caller.principalId; -} - -/** - * Only the creating principal, calling from the tenant that owns the - * skill, may republish, restore, or change its scope. An asset inherited - * from an ancestor tenant is never administerable from a descendant — - * even by its own author — so writes never touch an ancestor's asset; - * the registry refuses those explicitly rather than silently forking a - * copy (see `requireOwnTenant`). - */ -function canAdministerSkill( - skill: ResolvedSkill, - caller: SkillCaller, -): boolean { - return ( - skill.asset.tenantId === caller.tenantId && - skill.asset.creatorPrincipalId === caller.principalId - ); -} - -/** - * Guards every write path (`update`, `restore`, `setScope`). A skill - * inherited from an ancestor tenant is refused loudly and specifically — - * never silently forked into a same-named copy in the caller's own - * tenant — before falling through to the ordinary "only the author" - * check for skills the caller's own tenant does own. - */ -function requireOwnTenant( - skill: ResolvedSkill, - caller: SkillCaller, - name: string, - action: string, -): void { - if (skill.asset.tenantId !== caller.tenantId) { - throw new SkillRegistryError( - "forbidden", - `"${name}" is inherited from a parent workbench — ${action} it from the workbench that owns it, not from a child.`, - ); - } - if (!canAdministerSkill(skill, caller)) { - throw new SkillRegistryError( - "forbidden", - `only the author of "${name}" may ${action} it`, - ); - } -} - -function summarize(skill: ResolvedSkill): SkillSummary { - return { - assetId: skill.asset.id, - name: skill.parsed.name, - description: skill.parsed.description, - scope: skill.parsed.scope, - creatorPrincipalId: skill.asset.creatorPrincipalId ?? "unknown", - updatedAtIso: skill.updatedAtIso, - }; -} - -function detailOf(skill: ResolvedSkill): SkillDetail { - return { ...summarize(skill), body: skill.parsed.body }; -} - -export function createSkillRegistry( - deps: CreateSkillRegistryDeps, -): SkillRegistry { - const { assets } = deps; - - /** Reads a skill's tip SKILL.md and parses it. Returns null when the - * asset carries no SKILL.md yet — the half-written state a crashed - * `create` leaves behind — so callers treat it as invisible rather - * than as a skill with empty content. */ - async function readTip(asset: SkillAssetRow): Promise { - const contents = await assets.readSkillMd({ - assetId: asset.id, - skillName: asset.name, - }); - if (contents === null) return null; - return { - asset, - parsed: parseSkillMd(contents), - updatedAtIso: asset.updatedAt.toISOString(), - }; - } - - async function resolveVisible( - caller: SkillCaller, - name: string, - ): Promise { - const asset = await assets.findByName(caller.tenantId, name); - if (asset === null) { - throw new SkillRegistryError("not_found", `no skill named "${name}"`); - } - const skill = await readTip(asset); - if (skill === null || !isSkillVisibleTo(skill, caller)) { - throw new SkillRegistryError("not_found", `no skill named "${name}"`); - } - return skill; - } - - async function visibleSkills( - caller: SkillCaller, - ): Promise { - const assetRows = await assets.listForTenant(caller.tenantId); - const out: ResolvedSkill[] = []; - for (const asset of assetRows) { - const skill = await readTip(asset); - if (skill === null) continue; - if (!isSkillVisibleTo(skill, caller)) continue; - out.push(skill); - } - return out.sort((a, b) => a.parsed.name.localeCompare(b.parsed.name)); - } - - return { - async list(caller) { - return (await visibleSkills(caller)).map(summarize); - }, - - async search(caller, query) { - const needle = query.trim().toLowerCase(); - const skills = await visibleSkills(caller); - if (needle === "") return skills.map(summarize); - return skills - .filter( - (skill) => - skill.parsed.name.toLowerCase().includes(needle) || - skill.parsed.description.toLowerCase().includes(needle), - ) - .map(summarize); - }, - - async load(caller, name) { - return detailOf(await resolveVisible(caller, name)); - }, - - async versions(caller, name) { - const skill = await resolveVisible(caller, name); - const commits = await assets.history(skill.asset.id); - return commits.map((commit, index) => ({ - ...commit, - current: index === 0, - })); - }, - - async versionContent(caller, name, commitSha) { - const skill = await resolveVisible(caller, name); - const contents = await assets.readSkillMd({ - assetId: skill.asset.id, - skillName: skill.asset.name, - commitSha, - }); - if (contents === null) { - throw new SkillRegistryError( - "not_found", - `skill "${name}" has no SKILL.md at commit ${commitSha}`, - ); - } - const commit = (await assets.history(skill.asset.id)).find( - (entry) => entry.commitSha === commitSha, - ); - if (commit === undefined) { - throw new SkillRegistryError( - "not_found", - `commit ${commitSha} is not in "${name}"'s history`, - ); - } - const parsed = parseSkillMd(contents); - return { - assetId: skill.asset.id, - name: parsed.name, - description: parsed.description, - body: parsed.body, - scope: parsed.scope, - creatorPrincipalId: skill.asset.creatorPrincipalId ?? "unknown", - updatedAtIso: commit.committedAtIso, - }; - }, - - async restore(caller, name, commitSha) { - const skill = await resolveVisible(caller, name); - requireOwnTenant(skill, caller, name, "restore"); - const contents = await assets.readSkillMd({ - assetId: skill.asset.id, - skillName: skill.asset.name, - commitSha, - }); - if (contents === null) { - throw new SkillRegistryError( - "not_found", - `skill "${name}" has no SKILL.md at commit ${commitSha}`, - ); - } - await assets.writeSkillMd({ - assetId: skill.asset.id, - skillName: skill.asset.name, - contents, - message: `Restore ${name} to ${commitSha.slice(0, 8)}`, - }); - const restored = await readTip(skill.asset); - if (restored === null) { - throw new SkillRegistryError( - "not_found", - `skill "${name}" has no SKILL.md on its default ref`, - ); - } - return detailOf(restored); - }, - - async setScope(caller, name, scope) { - const parsedScope = assertScope(scope); - const skill = await resolveVisible(caller, name); - requireOwnTenant(skill, caller, name, "change who can see"); - const rescoped = buildSkillMd({ - name: skill.parsed.name, - description: skill.parsed.description, - scope: parsedScope, - body: skill.parsed.body, - }); - await assets.writeSkillMd({ - assetId: skill.asset.id, - skillName: skill.asset.name, - contents: rescoped, - message: `Set scope of ${name} to ${parsedScope}`, - }); - const updated = await readTip(skill.asset); - if (updated === null) { - throw new SkillRegistryError( - "not_found", - `skill "${name}" has no SKILL.md on its default ref`, - ); - } - return summarize(updated); - }, - - async create(caller, input) { - const name = assertSkillName(input.name); - const description = assertDescription(input.description); - const parsedScope = assertScope(input.scope); - let contents: string; - try { - contents = buildSkillMd({ - name, - description, - scope: parsedScope, - body: input.body, - }); - } catch (cause) { - contentErrorToRegistryError(cause); - } - - async function finish(asset: SkillAssetRow): Promise { - const skill = await readTip(asset); - if (skill === null) { - throw new SkillRegistryError( - "not_found", - `created skill "${name}" is not readable back`, - ); - } - return summarize(skill); - } - - // Own-tenant only, deliberately not inheritance-aware: a name - // already claimed by an ancestor's skill must not block this - // tenant from creating its own — that's the shadowing contract, - // not a conflict. - const existing = await assets.findOwnByName(caller.tenantId, name); - if (existing !== null) { - const existingSkill = await readTip(existing); - if ( - existingSkill !== null || - existing.creatorPrincipalId !== caller.principalId - ) { - // Either a fully-formed skill already owns this name, or the - // orphaned asset was started by someone else — neither is this - // caller's to complete. - throw new SkillRegistryError( - "conflict", - `a skill named "${name}" already exists in this workbench`, - ); - } - // The asset exists but carries no SKILL.md yet: a prior create - // for this exact name got as far as `assets.create` and then - // failed or timed out before the SKILL.md commit landed. Finish - // it — write the commit, never re-create the asset — rather than - // 409ing on a name this same caller can never use again. - await assets.writeSkillMd({ - assetId: existing.id, - skillName: name, - contents, - message: `Create ${name}`, - }); - return finish(existing); - } - - const created = await assets.create({ - tenantId: caller.tenantId, - name, - displayName: name, - creatorPrincipalId: caller.principalId, - }); - await assets.writeSkillMd({ - assetId: created.id, - skillName: name, - contents, - message: `Create ${name}`, - }); - return finish(created); - }, - - async update(caller, name, input) { - const parsedName = assertSkillName(name); - const description = assertDescription(input.description); - const skill = await resolveVisible(caller, parsedName); - requireOwnTenant(skill, caller, parsedName, "update"); - if (input.expectedHeadSha !== undefined) { - const head = (await assets.history(skill.asset.id))[0]; - if (head?.commitSha !== input.expectedHeadSha) { - throw new SkillRegistryError( - "conflict", - `"${parsedName}" changed since this edit started — review the current version before saving.`, - ); - } - } - let contents: string; - try { - contents = buildSkillMd({ - name: parsedName, - description, - scope: skill.parsed.scope, - body: input.body, - }); - } catch (cause) { - contentErrorToRegistryError(cause); - } - await assets.writeSkillMd({ - assetId: skill.asset.id, - skillName: parsedName, - contents, - message: `Update ${parsedName}`, - }); - const updated = await readTip(skill.asset); - if (updated === null) { - throw new SkillRegistryError( - "not_found", - `updated skill "${parsedName}" is not readable back`, - ); - } - return summarize(updated); - }, - }; -} diff --git a/packages/skills/src/routes.ts b/packages/skills/src/routes.ts deleted file mode 100644 index 9740d3710c..0000000000 --- a/packages/skills/src/routes.ts +++ /dev/null @@ -1,257 +0,0 @@ -// The tenant-scoped registry surface the Skills settings section talks -// to. Mounted under the hub's tenant prefix, so `tenant` and `principal` -// are already resolved by the session middleware; the caller identity -// the registry scopes against comes from that context and never from a -// request body. -import { type } from "arktype"; -import { Hono } from "hono"; - -import type { TenantEnv, RequireGrant } from "@intx/hub-api"; - -import { - SkillRegistryError, - type SkillRegistry, - type SkillRegistryErrorReason, -} from "./registry"; -import { parseSkillMd, skillScopeSchema, SkillContentError } from "./skill-md"; -import { makeErrorEnvelope } from "@corbits/error-sink"; - -/** Which workflow definitions pin a given skill. */ -export type PinnedByResolver = { - resolve( - tenantId: string, - skillName: string, - ): Promise< - readonly { readonly definitionId: string; readonly name: string }[] - >; -}; - -/** The paste path: identity and body already split into fields. */ -const CreateSkillFieldsBody = type({ - name: "string", - description: "string", - body: "string", - scope: skillScopeSchema, -}); - -/** The upload path: a whole SKILL.md, parsed at this boundary with the - * exact same `parseSkillMd` the registry uses to read a skill back — so a - * round trip through upload lands on the same fields a paste of the same - * content would have. */ -const CreateSkillFileBody = type({ - source: "string", - scope: skillScopeSchema, -}); - -const CreateSkillBody = CreateSkillFieldsBody.or(CreateSkillFileBody); - -/** A commit id, as the asset store hands them out: one bounded opaque - * token, never a path or free text. The store owns the exact alphabet, so - * this parses the shape the boundary needs — short, no separators, nothing - * that could read as a path — and leaves "is this a real commit" to the - * registry's own history lookup. */ -const commitShaSchema = type("string <= 64").narrow( - (value, ctx) => /^[A-Za-z0-9._-]+$/.test(value) || ctx.mustBe("a version id"), -); - -const UpdateSkillBody = type({ - description: "string", - body: "string", - "expectedHeadSha?": commitShaSchema, -}); - -const RestoreBody = type({ commitSha: commitShaSchema }); - -const ScopeBody = type({ scope: skillScopeSchema }); - -const STATUS_BY_REASON: Record< - SkillRegistryErrorReason, - 400 | 403 | 404 | 409 -> = { - invalid: 400, - forbidden: 403, - not_found: 404, - conflict: 409, -}; - -export type CreateSkillRoutesDeps = { - registry: SkillRegistry; - pinnedBy: PinnedByResolver; - requireGrant: RequireGrant; -}; - -export function createSkillRoutes({ - registry, - pinnedBy, - requireGrant, -}: CreateSkillRoutesDeps): Hono { - const app = new Hono(); - - app.onError((err, c) => { - if (err instanceof SkillRegistryError) { - return c.json( - makeErrorEnvelope({ code: err.reason, userMessage: err.message }), - STATUS_BY_REASON[err.reason], - ); - } - throw err; - }); - - function caller(c: { get: (key: "tenant" | "principal") => { id: string } }) { - return { - tenantId: c.get("tenant").id, - principalId: c.get("principal").id, - }; - } - - app.get("/", requireGrant("asset:*", "read"), async (c) => { - const query = c.req.query("q"); - const skills = - query === undefined || query.trim() === "" - ? await registry.list(caller(c)) - : await registry.search(caller(c), query); - return c.json({ skills }); - }); - - app.post("/", requireGrant("asset:*", "create"), async (c) => { - const body = CreateSkillBody(await c.req.json().catch(() => undefined)); - if (body instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: `invalid skill: ${body.summary}`, - }), - 400, - ); - } - let fields: { name: string; description: string; body: string }; - if ("source" in body) { - try { - fields = parseSkillMd(body.source); - } catch (cause) { - if (cause instanceof SkillContentError) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: cause.message, - }), - 400, - ); - } - throw cause; - } - } else { - fields = { - name: body.name, - description: body.description, - body: body.body, - }; - } - const skill = await registry.create(caller(c), { - name: fields.name, - description: fields.description, - body: fields.body, - scope: body.scope, - }); - return c.json({ skill }, 201); - }); - - app.get("/:name", requireGrant("asset:*", "read"), async (c) => { - const name = c.req.param("name"); - const scope = caller(c); - const skill = await registry.load(scope, name); - return c.json({ - skill, - pinnedBy: await pinnedBy.resolve(scope.tenantId, name), - }); - }); - - app.put("/:name", requireGrant("asset:*", "create"), async (c) => { - const body = UpdateSkillBody(await c.req.json().catch(() => undefined)); - if (body instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: `invalid update: ${body.summary}`, - }), - 400, - ); - } - const skill = await registry.update(caller(c), c.req.param("name"), { - description: body.description, - body: body.body, - expectedHeadSha: body.expectedHeadSha, - }); - return c.json({ skill }); - }); - - app.get("/:name/versions", requireGrant("asset:*", "read"), async (c) => { - return c.json({ - versions: await registry.versions(caller(c), c.req.param("name")), - }); - }); - - app.get( - "/:name/versions/:commitSha", - requireGrant("asset:*", "read"), - async (c) => { - const commitSha = commitShaSchema(c.req.param("commitSha")); - if (commitSha instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: `invalid version: ${commitSha.summary}`, - }), - 400, - ); - } - return c.json({ - skill: await registry.versionContent( - caller(c), - c.req.param("name"), - commitSha, - ), - }); - }, - ); - - app.post("/:name/restore", requireGrant("asset:*", "create"), async (c) => { - const body = RestoreBody(await c.req.json().catch(() => undefined)); - if (body instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: `invalid restore: ${body.summary}`, - }), - 400, - ); - } - const skill = await registry.restore( - caller(c), - c.req.param("name"), - body.commitSha, - ); - return c.json({ skill }); - }); - - app.put("/:name/scope", requireGrant("asset:*", "create"), async (c) => { - const body = ScopeBody(await c.req.json().catch(() => undefined)); - if (body instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: `invalid scope: ${body.summary}`, - }), - 400, - ); - } - const skill = await registry.setScope( - caller(c), - c.req.param("name"), - body.scope, - ); - return c.json({ skill }); - }); - - return app; -} diff --git a/packages/skills/src/workflow-routes.ts b/packages/skills/src/workflow-routes.ts deleted file mode 100644 index 0ee6892e7a..0000000000 --- a/packages/skills/src/workflow-routes.ts +++ /dev/null @@ -1,204 +0,0 @@ -// The sanctioned path for a workflow-process child to reach the skill -// registry, mirroring `@corbits/memory-hub`'s workflow-memory surface: a -// workflow child has no browser session, only its own sidecar bearer -// token and the run's address, so it authenticates through -// `@corbits/artifacts-hub`'s `WorkflowRunAuthenticator` rather than the -// tenant-session pipeline the routes in `./routes.ts` use. -// -// Mounted OUTSIDE the tenant prefix for that reason. Identity NEVER -// rides in a request body: the tenant and principal every read is scoped -// to come from the authenticated run alone, so a model's tool arguments -// can never name a different tenant or a different author's private -// skill. -import { type } from "arktype"; -import { Hono } from "hono"; -import { makeErrorEnvelope } from "@corbits/error-sink"; - -import { SkillRegistryError, type SkillRegistry } from "./registry"; - -/** - * The tenant + principal a presented sidecar token and run address - * resolve to. Declared structurally rather than imported so this package - * carries no dependency on the artifacts plane; `apps/hub` supplies - * `@corbits/artifacts-hub`'s `createWorkflowRunAuthenticator`, which is - * generic over "a sidecar-provisioned caller acting as one resolved - * run's tenant + principal" and satisfies this shape exactly. - */ -export type WorkflowRunScope = { - readonly tenantId: string; - readonly principalId: string; -}; - -export type WorkflowRunAuthenticator = { - resolve(token: string, runAddress: string): Promise; -}; - -export type WorkflowSkillsEnv = { - Variables: { workflowRunScope: WorkflowRunScope }; -}; - -const SearchBody = type({ query: "string" }); - -const LoadBody = type({ name: "string > 0" }); - -const CreateBody = type({ - name: "string", - description: "string", - body: "string", -}); - -const UpdateBody = type({ - name: "string", - body: "string", - "description?": "string", -}); - -export type CreateWorkflowSkillRoutesDeps = { - authenticator: WorkflowRunAuthenticator; - registry: SkillRegistry; -}; - -export function createWorkflowSkillRoutes( - deps: CreateWorkflowSkillRoutesDeps, -): Hono { - const app = new Hono(); - - app.onError((err, c) => { - if (err instanceof SkillRegistryError) { - return c.json( - makeErrorEnvelope({ - code: err.reason, - userMessage: err.message, - }), - err.reason === "not_found" ? 404 : 400, - ); - } - throw err; - }); - - app.use("*", async (c, next) => { - const authHeader = c.req.header("authorization") ?? ""; - const token = authHeader.startsWith("Bearer ") - ? authHeader.slice("Bearer ".length) - : ""; - const address = c.req.header("x-workflow-run-address") ?? ""; - const scope = await deps.authenticator.resolve(token, address); - if (scope === null) { - return c.json( - makeErrorEnvelope({ - code: "unauthorized", - userMessage: - "Missing or unrecognized sidecar bearer token / run address", - }), - 401, - ); - } - c.set("workflowRunScope", scope); - await next(); - }); - - app.get("/list", async (c) => { - const scope = c.get("workflowRunScope"); - const skills = await deps.registry.list(scope); - return c.json({ - data: skills.map((skill) => ({ - name: skill.name, - description: skill.description, - })), - }); - }); - - app.post("/search", async (c) => { - const body = SearchBody(await c.req.json().catch(() => undefined)); - if (body instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: body.summary, - }), - 400, - ); - } - const scope = c.get("workflowRunScope"); - const skills = await deps.registry.search(scope, body.query); - return c.json({ - data: skills.map((skill) => ({ - name: skill.name, - description: skill.description, - })), - }); - }); - - app.post("/load", async (c) => { - const body = LoadBody(await c.req.json().catch(() => undefined)); - if (body instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: body.summary, - }), - 400, - ); - } - const scope = c.get("workflowRunScope"); - const skill = await deps.registry.load(scope, body.name); - return c.json({ - data: { - name: skill.name, - description: skill.description, - body: skill.body, - }, - }); - }); - - // Always `scope: "tenant"` — a skill Myra captures from a workbench - // conversation is durable know-how for the whole workbench, never a - // private note only this run's principal can see again. - app.post("/create", async (c) => { - const body = CreateBody(await c.req.json().catch(() => undefined)); - if (body instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: body.summary, - }), - 400, - ); - } - const scope = c.get("workflowRunScope"); - const skill = await deps.registry.create(scope, { - name: body.name, - description: body.description, - body: body.body, - scope: "tenant", - }); - return c.json({ data: skill }); - }); - - // `registry.load` first (404s if the name is unknown) so an update - // that only wants to change the body can leave the skill's current - // description exactly as it was, rather than a client having to - // resend fields it never touched. `registry.update` itself leaves - // scope untouched — republishing content never changes who can see it. - app.post("/update", async (c) => { - const body = UpdateBody(await c.req.json().catch(() => undefined)); - if (body instanceof type.errors) { - return c.json( - makeErrorEnvelope({ - code: "bad_request", - userMessage: body.summary, - }), - 400, - ); - } - const scope = c.get("workflowRunScope"); - const existing = await deps.registry.load(scope, body.name); - const skill = await deps.registry.update(scope, body.name, { - description: body.description ?? existing.description, - body: body.body, - }); - return c.json({ data: skill }); - }); - - return app; -} diff --git a/packages/skills/test/asset-history.test.ts b/packages/skills/test/asset-history.test.ts deleted file mode 100644 index 5296019561..0000000000 --- a/packages/skills/test/asset-history.test.ts +++ /dev/null @@ -1,77 +0,0 @@ -// Consumer history must hide the hub's genesis commit — a person who just -// created a skill should see their "Create …" version, never the substrate's -// "Initialize repository" scaffolding authored as interchange-hub. -import fs from "node:fs"; -import os from "node:os"; -import path from "node:path"; - -import { afterEach, describe, expect, test } from "bun:test"; -import git from "isomorphic-git"; - -import { readAssetCommitHistory } from "../src/asset-history"; -import type { RepoId, RepoStore } from "@intx/hub-sessions"; - -const dirs: string[] = []; - -afterEach(() => { - for (const dir of dirs.splice(0)) { - fs.rmSync(dir, { recursive: true, force: true }); - } -}); - -function tempDir(): string { - const dir = fs.mkdtempSync(path.join(os.tmpdir(), "skill-history-")); - dirs.push(dir); - return dir; -} - -function repoStoreFor(dir: string): RepoStore { - return { - getRepoDir: async (_id: RepoId) => dir, - } as unknown as RepoStore; -} - -describe("readAssetCommitHistory", () => { - test("hides the Initialize repository genesis from consumer history", async () => { - const dir = tempDir(); - await git.init({ fs, dir, defaultBranch: "main" }); - await fs.promises.writeFile(path.join(dir, ".gitignore"), "keys/\n"); - await git.add({ fs, dir, filepath: ".gitignore" }); - await git.commit({ - fs, - dir, - message: "Initialize repository", - author: { - name: "interchange-hub", - email: "hub@interchange.local", - }, - }); - await fs.promises.mkdir(path.join(dir, "triage"), { recursive: true }); - await fs.promises.writeFile( - path.join(dir, "triage", "SKILL.md"), - "---\nname: triage\ndescription: Sort issues.\n---\n\nBody.\n", - ); - await git.add({ fs, dir, filepath: "triage/SKILL.md" }); - await git.commit({ - fs, - dir, - message: "Create triage", - author: { - name: "interchange-hub", - email: "hub@interchange.local", - }, - }); - - const history = await readAssetCommitHistory({ - repoStore: repoStoreFor(dir), - kind: "skill", - assetId: "asset_1", - ref: "main", - }); - - expect(history.map((entry) => entry.message)).toEqual(["Create triage"]); - expect( - history.some((entry) => entry.message === "Initialize repository"), - ).toBe(false); - }); -}); diff --git a/packages/skills/test/fakes.ts b/packages/skills/test/fakes.ts deleted file mode 100644 index bb8790f695..0000000000 --- a/packages/skills/test/fakes.ts +++ /dev/null @@ -1,179 +0,0 @@ -// In-memory double for the one port the registry stands on. It keeps the -// registry suite honest without a live hub or a database: the asset -// double keeps a real per-asset commit list so version history and -// restore are exercised against an append-only log, exactly the shape -// the git-backed store serves. -import type { - SkillAssetRow, - SkillAssetStore, - SkillCommit, -} from "../src/asset-store"; -import { isAssetGenesisCommit } from "../src/asset-history"; -import { skillMdPath } from "../src/asset-store"; - -type StoredCommit = { - readonly commit: SkillCommit; - readonly files: ReadonlyMap; -}; - -export type FakeSkillAssets = SkillAssetStore & { - readonly assets: Map; - /** Makes the next writeSkillMd call throw, simulating a crash between - * the asset-row write and the SKILL.md write. */ - failNextWriteSkillMd: () => void; -}; - -/** A tenant's own id, then its parent, then its grandparent, and so on to - * the root — the same shape `getAncestorChain` returns for real tenants, - * built here from a flat `tenantId -> parentId` map so tests can lay out - * a hierarchy without a database. */ -export type TenantParents = Readonly>; - -function ancestorChain(parents: TenantParents, tenantId: string): string[] { - const chain: string[] = []; - let current: string | undefined = tenantId; - while (current !== undefined && !chain.includes(current)) { - chain.push(current); - current = parents[current]; - } - return chain; -} - -export function createFakeSkillAssets( - options: { - clock?: { now: () => Date }; - tenantParents?: TenantParents; - } = {}, -): FakeSkillAssets { - const clock = options.clock ?? { - now: () => new Date("2026-01-01T00:00:00Z"), - }; - const parents = options.tenantParents ?? {}; - const assets = new Map(); - const commits = new Map(); - let nextId = 1; - let nextSha = 1; - let failNextWrite = false; - - function tip(assetId: string): StoredCommit | undefined { - const log = commits.get(assetId); - return log === undefined ? undefined : log[log.length - 1]; - } - - function ownByName(tenantId: string, name: string): SkillAssetRow | null { - for (const row of assets.values()) { - if (row.tenantId === tenantId && row.name === name) return row; - } - return null; - } - - return { - assets, - failNextWriteSkillMd() { - failNextWrite = true; - }, - async create(input) { - const key = `${input.tenantId}${input.name}`; - for (const existing of assets.values()) { - if (`${existing.tenantId}${existing.name}` === key) { - throw new Error(`duplicate asset ${input.name}`); - } - } - const now = clock.now(); - const row: SkillAssetRow = { - id: `asset_${String(nextId++)}`, - tenantId: input.tenantId, - name: input.name, - displayName: input.displayName, - creatorPrincipalId: input.creatorPrincipalId, - createdAt: now, - updatedAt: now, - }; - assets.set(row.id, row); - // Mirror the hub's genesis commit so consumer-history filtering is - // exercised the same way a real `createAsset` → `initRepo` path is. - commits.set(row.id, [ - { - commit: { - commitSha: `commit${String(nextSha++).padStart(4, "0")}`, - message: "Initialize repository", - author: "interchange-hub", - committedAtIso: now.toISOString(), - }, - files: new Map(), - }, - ]); - return row; - }, - async findByName(tenantId, name) { - for (const tid of ancestorChain(parents, tenantId)) { - const row = ownByName(tid, name); - if (row !== null) return row; - } - return null; - }, - async findOwnByName(tenantId, name) { - return ownByName(tenantId, name); - }, - async listForTenant(tenantId) { - const byName = new Map(); - for (const tid of ancestorChain(parents, tenantId)) { - for (const row of assets.values()) { - if (row.tenantId !== tid) continue; - if (byName.has(row.name)) continue; - byName.set(row.name, row); - } - } - return [...byName.values()]; - }, - async writeSkillMd(input) { - if (failNextWrite) { - failNextWrite = false; - throw new Error("simulated SKILL.md write failure"); - } - const log = commits.get(input.assetId); - if (log === undefined) { - throw new Error(`no such asset ${input.assetId}`); - } - const files = new Map(tip(input.assetId)?.files ?? []); - files.set(skillMdPath(input.skillName), input.contents); - const commitSha = `commit${String(nextSha++).padStart(4, "0")}`; - const row = assets.get(input.assetId); - // Only the skill's author may write (registry enforces this); attribute - // the save to that principal rather than a machine account. - const author = row?.creatorPrincipalId ?? "workbench"; - log.push({ - commit: { - commitSha, - message: input.message, - author, - committedAtIso: clock.now().toISOString(), - }, - files, - }); - if (row !== undefined) { - assets.set(input.assetId, { ...row, updatedAt: clock.now() }); - } - return { commitSha }; - }, - async readSkillMd(input) { - const log = commits.get(input.assetId); - if (log === undefined) return null; - const path = skillMdPath(input.skillName); - if (input.commitSha === undefined) { - return tip(input.assetId)?.files.get(path) ?? null; - } - const found = log.find( - (entry) => entry.commit.commitSha === input.commitSha, - ); - return found?.files.get(path) ?? null; - }, - async history(assetId) { - const log = commits.get(assetId) ?? []; - return [...log] - .reverse() - .map((entry) => entry.commit) - .filter((commit) => !isAssetGenesisCommit(commit.message)); - }, - }; -} diff --git a/packages/skills/test/registry.test.ts b/packages/skills/test/registry.test.ts deleted file mode 100644 index 76c99e0fec..0000000000 --- a/packages/skills/test/registry.test.ts +++ /dev/null @@ -1,412 +0,0 @@ -import { beforeEach, describe, expect, test } from "bun:test"; - -import { - createSkillRegistry, - SkillRegistryError, - type SkillRegistry, -} from "../src/registry"; -import { createFakeSkillAssets } from "./fakes"; -import type { FakeSkillAssets } from "./fakes"; - -const AUTHOR = { tenantId: "tenant_1", principalId: "principal_author" }; -const TEAMMATE = { tenantId: "tenant_1", principalId: "principal_teammate" }; -const OUTSIDER = { tenantId: "tenant_2", principalId: "principal_author" }; - -const CREATE_INPUT = { - name: "triage", - description: "Sorts inbound issues into bug, question, or feature.", - body: "Read the report. Pick exactly one label. Explain the pick.", -}; - -let assets: FakeSkillAssets; -let registry: SkillRegistry; - -beforeEach(() => { - assets = createFakeSkillAssets(); - registry = createSkillRegistry({ assets }); -}); - -async function publish(scope: "private" | "tenant") { - return registry.create(AUTHOR, { ...CREATE_INPUT, scope }); -} - -describe("create", () => { - test("creates a real skill immediately, with no pending state in between", async () => { - const skill = await publish("tenant"); - expect(skill.name).toBe("triage"); - const names = [...assets.assets.values()].map((row) => row.name); - expect(names).toEqual(["triage"]); - expect((await registry.list(AUTHOR)).map((s) => s.name)).toEqual([ - "triage", - ]); - }); - - test("the created skill keeps the authored description and body", async () => { - await publish("tenant"); - const loaded = await registry.load(AUTHOR, "triage"); - expect(loaded.description).toBe(CREATE_INPUT.description); - expect(loaded.body).toBe(CREATE_INPUT.body); - }); - - test("a second skill of the same name is a conflict, not a silent overwrite", async () => { - await publish("private"); - expect( - registry.create(AUTHOR, { ...CREATE_INPUT, scope: "private" }), - ).rejects.toThrow(/already exists/); - }); - - test("an invalid skill body is refused before any asset is created", async () => { - expect( - registry.create(AUTHOR, { - ...CREATE_INPUT, - body: " ", - scope: "private", - }), - ).rejects.toThrow(SkillRegistryError); - expect(assets.assets.size).toBe(0); - }); - - test("invalid SKILL.md frontmatter (a name the schema rejects) is refused before any asset is created", async () => { - expect( - registry.create(AUTHOR, { - ...CREATE_INPUT, - name: "Not A Valid Name", - scope: "private", - }), - ).rejects.toThrow(SkillRegistryError); - expect(assets.assets.size).toBe(0); - }); - - test("a name that fails the kebab-case pattern gets a plain-language message, never the raw regex", async () => { - await expect( - registry.create(AUTHOR, { - ...CREATE_INPUT, - name: "Not A Valid Name", - scope: "private", - }), - ).rejects.toThrow("Name must be lowercase letters, digits, and hyphens."); - }); - - test("a description with an HTML tag gets a plain-language message, never the raw regex", async () => { - await expect( - registry.create(AUTHOR, { - ...CREATE_INPUT, - description: "", - scope: "private", - }), - ).rejects.toThrow("Description can't contain HTML tags."); - }); - - test("retrying create after a crash between the asset write and the SKILL.md write completes it, rather than 409ing forever", async () => { - // Simulates the exact crash window `create` cannot make transactional: - // the asset row exists, but its SKILL.md commit never landed. - assets.failNextWriteSkillMd(); - - await expect( - registry.create(AUTHOR, { ...CREATE_INPUT, scope: "private" }), - ).rejects.toThrow(/simulated SKILL.md write failure/); - - // The asset was created and left behind, but is invisible: it carries - // no SKILL.md yet. - expect(assets.assets.size).toBe(1); - expect(await registry.list(AUTHOR)).toHaveLength(0); - - // Retrying the exact same create — the natural recovery a user or - // client would attempt — finishes the interrupted write instead of - // 409ing on a name this same caller can never use again. - const completed = await registry.create(AUTHOR, { - ...CREATE_INPUT, - scope: "private", - }); - expect(completed.name).toBe("triage"); - expect(assets.assets.size).toBe(1); - expect((await registry.list(AUTHOR)).map((s) => s.name)).toEqual([ - "triage", - ]); - - // A third attempt now hits a fully-formed skill and is a genuine - // conflict. - await expect( - registry.create(AUTHOR, { ...CREATE_INPUT, scope: "private" }), - ).rejects.toThrow(/already exists/); - }); - - test("a caller can never complete another principal's half-written create", async () => { - assets.failNextWriteSkillMd(); - await expect( - registry.create(AUTHOR, { ...CREATE_INPUT, scope: "private" }), - ).rejects.toThrow(/simulated SKILL.md write failure/); - - // A different principal retrying the same name hits a conflict, not - // a takeover of the first caller's orphaned asset. - await expect( - registry.create(TEAMMATE, { ...CREATE_INPUT, scope: "private" }), - ).rejects.toThrow(/already exists/); - expect(await registry.list(TEAMMATE)).toHaveLength(0); - }); -}); - -describe("access scoping", () => { - test("a private skill is listed for its author only", async () => { - await publish("private"); - expect((await registry.list(AUTHOR)).map((s) => s.name)).toEqual([ - "triage", - ]); - expect(await registry.list(TEAMMATE)).toHaveLength(0); - }); - - test("a tenant skill is listed for every principal in the tenant", async () => { - await publish("tenant"); - expect((await registry.list(TEAMMATE)).map((s) => s.name)).toEqual([ - "triage", - ]); - }); - - test("no skill is ever visible from another tenant", async () => { - await publish("tenant"); - expect(await registry.list(OUTSIDER)).toHaveLength(0); - expect(registry.load(OUTSIDER, "triage")).rejects.toThrow( - SkillRegistryError, - ); - }); - - test("search never reaches past the caller's own visibility", async () => { - await publish("private"); - expect(await registry.search(TEAMMATE, "triage")).toHaveLength(0); - expect( - (await registry.search(AUTHOR, "triage")).map((s) => s.name), - ).toEqual(["triage"]); - }); - - test("search matches on the description as well as the name", async () => { - await publish("tenant"); - expect(await registry.search(AUTHOR, "inbound issues")).toHaveLength(1); - expect(await registry.search(AUTHOR, "unrelated")).toHaveLength(0); - }); - - test("setScope shares a private skill with the tenant", async () => { - await publish("private"); - await registry.setScope(AUTHOR, "triage", "tenant"); - expect(await registry.list(TEAMMATE)).toHaveLength(1); - }); - - test("setScope re-commits the same content under the new scope", async () => { - await publish("private"); - const before = await registry.versions(AUTHOR, "triage"); - expect(before).toHaveLength(1); - - const shared = await registry.setScope(AUTHOR, "triage", "tenant"); - expect(shared.scope).toBe("tenant"); - - // The rescope is itself a commit: the content is unchanged, but the - // history grew, so the previous scope stays restorable. - const loaded = await registry.load(AUTHOR, "triage"); - expect(loaded.body).toBe(CREATE_INPUT.body); - expect(loaded.description).toBe(CREATE_INPUT.description); - const after = await registry.versions(AUTHOR, "triage"); - expect(after).toHaveLength(2); - expect(after[0]?.current).toBe(true); - expect(await registry.list(TEAMMATE)).toHaveLength(1); - }); - - test("setScope unshares a tenant skill back to its author", async () => { - await publish("tenant"); - await registry.setScope(AUTHOR, "triage", "private"); - expect(await registry.list(TEAMMATE)).toHaveLength(0); - }); - - test("a teammate who can read a shared skill still cannot rescope it", async () => { - await publish("tenant"); - expect(registry.setScope(TEAMMATE, "triage", "private")).rejects.toThrow( - /only the author/, - ); - }); -}); - -describe("versions", () => { - test("history comes from the asset's commits, newest first", async () => { - await publish("tenant"); - const versions = await registry.versions(AUTHOR, "triage"); - expect(versions).toHaveLength(1); - expect(versions[0]?.current).toBe(true); - expect(versions[0]?.message).toBe("Create triage"); - }); - - test("after create, consumer history is one real version authored by the saver", async () => { - await publish("tenant"); - const versions = await registry.versions(AUTHOR, "triage"); - expect(versions).toHaveLength(1); - expect(versions[0]?.message).toBe("Create triage"); - expect(versions[0]?.author).toBe(AUTHOR.principalId); - expect( - versions.some((version) => version.message === "Initialize repository"), - ).toBe(false); - }); - - test("restore re-commits an older version and marks it current", async () => { - await publish("tenant"); - const first = (await registry.versions(AUTHOR, "triage"))[0]; - expect(first).toBeDefined(); - - // Author a second version by restoring, then confirm the rewind is - // itself a new commit rather than a rewrite of the history. - const restored = await registry.restore( - AUTHOR, - "triage", - first?.commitSha ?? "", - ); - expect(restored.body).toBe(CREATE_INPUT.body); - const versions = await registry.versions(AUTHOR, "triage"); - expect(versions).toHaveLength(2); - expect(versions[0]?.current).toBe(true); - expect(versions[1]?.commitSha).toBe(first?.commitSha); - }); - - test("restoring a commit that carries no SKILL.md is a not-found", async () => { - await publish("tenant"); - expect(registry.restore(AUTHOR, "triage", "commit9999")).rejects.toThrow( - /no SKILL.md at commit/, - ); - }); - - test("a teammate cannot restore a shared skill they did not author", async () => { - await publish("tenant"); - const first = (await registry.versions(TEAMMATE, "triage"))[0]; - expect( - registry.restore(TEAMMATE, "triage", first?.commitSha ?? ""), - ).rejects.toThrow(/only the author/); - }); -}); - -describe("tenant inheritance", () => { - const PARENT_TENANT = "tenant_parent"; - const CHILD_TENANT = "tenant_child"; - const PARENT_HIERARCHY = { [CHILD_TENANT]: PARENT_TENANT }; - - const PARENT_AUTHOR = { - tenantId: PARENT_TENANT, - principalId: "principal_parent_author", - }; - const CHILD_MEMBER = { - tenantId: CHILD_TENANT, - principalId: "principal_child_member", - }; - - function createInheritingRegistry(): { - registry: SkillRegistry; - assets: FakeSkillAssets; - } { - const inheritingAssets = createFakeSkillAssets({ - tenantParents: PARENT_HIERARCHY, - }); - return { - assets: inheritingAssets, - registry: createSkillRegistry({ - assets: inheritingAssets, - }), - }; - } - - test("a tenant-scoped skill authored on a parent is visible and loadable from a child tenant", async () => { - const { registry: inheriting } = createInheritingRegistry(); - await inheriting.create(PARENT_AUTHOR, { - ...CREATE_INPUT, - scope: "tenant", - }); - - expect((await inheriting.list(CHILD_MEMBER)).map((s) => s.name)).toEqual([ - "triage", - ]); - const loaded = await inheriting.load(CHILD_MEMBER, "triage"); - expect(loaded.body).toBe(CREATE_INPUT.body); - }); - - test("a child's own skill shadows a same-named skill inherited from its parent", async () => { - const { registry: inheriting } = createInheritingRegistry(); - await inheriting.create(PARENT_AUTHOR, { - ...CREATE_INPUT, - description: "The parent's version.", - body: "Parent body.", - scope: "tenant", - }); - await inheriting.create(CHILD_MEMBER, { - ...CREATE_INPUT, - description: "The child's own version.", - body: "Child body.", - scope: "tenant", - }); - - const loaded = await inheriting.load(CHILD_MEMBER, "triage"); - expect(loaded.body).toBe("Child body."); - expect((await inheriting.list(CHILD_MEMBER)).map((s) => s.name)).toEqual([ - "triage", - ]); - }); - - test("a private skill authored on the parent is invisible from the child, except to its own creator", async () => { - const { registry: inheriting } = createInheritingRegistry(); - await inheriting.create(PARENT_AUTHOR, { - ...CREATE_INPUT, - scope: "private", - }); - - expect(await inheriting.list(CHILD_MEMBER)).toHaveLength(0); - expect(inheriting.load(CHILD_MEMBER, "triage")).rejects.toThrow( - SkillRegistryError, - ); - - const parentAuthorInChild = { - tenantId: CHILD_TENANT, - principalId: PARENT_AUTHOR.principalId, - }; - const loaded = await inheriting.load(parentAuthorInChild, "triage"); - expect(loaded.body).toBe(CREATE_INPUT.body); - }); - - test("updating an inherited skill from the child is refused loudly, never forked into a child-owned copy", async () => { - const { registry: inheriting, assets } = createInheritingRegistry(); - await inheriting.create(PARENT_AUTHOR, { - ...CREATE_INPUT, - scope: "tenant", - }); - - const parentAuthorInChild = { - tenantId: CHILD_TENANT, - principalId: PARENT_AUTHOR.principalId, - }; - await expect( - inheriting.update(parentAuthorInChild, "triage", { - description: CREATE_INPUT.description, - body: "an attempted child-side edit", - }), - ).rejects.toThrow(/inherited from a parent workbench/); - - // Confirm nothing was forked into the child tenant. - const childOwnedNames = (await assets.listForTenant(CHILD_TENANT)).filter( - (a) => a.tenantId === CHILD_TENANT, - ); - expect(childOwnedNames).toHaveLength(0); - const loaded = await inheriting.load(CHILD_MEMBER, "triage"); - expect(loaded.body).toBe(CREATE_INPUT.body); - }); - - test("restore and setScope on an inherited skill are refused the same way", async () => { - const { registry: inheriting } = createInheritingRegistry(); - await inheriting.create(PARENT_AUTHOR, { - ...CREATE_INPUT, - scope: "tenant", - }); - const parentAuthorInChild = { - tenantId: CHILD_TENANT, - principalId: PARENT_AUTHOR.principalId, - }; - const first = (await inheriting.versions(CHILD_MEMBER, "triage"))[0]; - - await expect( - inheriting.restore(parentAuthorInChild, "triage", first?.commitSha ?? ""), - ).rejects.toThrow(/inherited from a parent workbench/); - await expect( - inheriting.setScope(parentAuthorInChild, "triage", "private"), - ).rejects.toThrow(/inherited from a parent workbench/); - }); -}); diff --git a/packages/skills/test/routes.test.ts b/packages/skills/test/routes.test.ts deleted file mode 100644 index f3b01a9c8a..0000000000 --- a/packages/skills/test/routes.test.ts +++ /dev/null @@ -1,301 +0,0 @@ -// Route-level tests for the tenant-scoped skill registry surface — -// request parsing, grant gating, and the PUT /:name update round trip -// (CL-6355): a new version is created and the version it replaced stays -// restorable. Registry semantics themselves are covered in registry.test.ts. -import { expect, test } from "bun:test"; -import { Hono } from "hono"; -import type { MiddlewareHandler } from "hono"; - -import type { TenantEnv } from "@intx/hub-api"; - -import { createSkillRegistry } from "../src/registry"; -import { createSkillRoutes } from "../src/routes"; -import { createFakeSkillAssets } from "./fakes"; - -const TENANT = { id: "tnt_1" }; -const PRINCIPAL = { id: "prn_author" }; - -function buildApp(): Hono { - const registry = createSkillRegistry({ - assets: createFakeSkillAssets(), - }); - const routes = createSkillRoutes({ - registry, - pinnedBy: { - async resolve() { - return []; - }, - }, - requireGrant: () => async (_c, next) => { - await next(); - }, - }); - const asTenant: MiddlewareHandler = async (c, next) => { - c.set("tenant", TENANT as never); - c.set("principal", PRINCIPAL as never); - await next(); - }; - const app = new Hono(); - app.use("*", asTenant); - app.route("/", routes); - return app; -} - -async function createSkill(app: Hono): Promise { - const response = await app.request("/", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - name: "triage", - description: "Sorts inbound issues.", - body: "Read the report. Pick one label.", - scope: "private", - }), - }); - expect(response.status).toBe(201); -} - -test("POST / with a source SKILL.md creates a skill whose fields match the file", async () => { - const app = buildApp(); - const response = await app.request("/", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - source: [ - "---", - "name: triage", - "description: 'Sorts inbound issues.'", - "---", - "", - "Read the report. Pick one label.", - "", - ].join("\n"), - scope: "private", - }), - }); - expect(response.status).toBe(201); - const created = (await response.json()) as { - skill: { name: string; description: string }; - }; - expect(created.skill.name).toBe("triage"); - expect(created.skill.description).toBe("Sorts inbound issues."); - - const loaded = (await (await app.request("/triage")).json()) as { - skill: { body: string }; - }; - expect(loaded.skill.body).toBe("Read the report. Pick one label."); -}); - -test("POST / with a malformed source SKILL.md is a 400 and creates nothing", async () => { - const app = buildApp(); - const response = await app.request("/", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - source: "not a skill file at all", - scope: "private", - }), - }); - expect(response.status).toBe(400); - const payload = (await response.json()) as { - error: { userMessage: string; refId: string }; - }; - expect(payload.error.userMessage).toContain("frontmatter delimiter"); - expect(typeof payload.error.refId).toBe("string"); - expect(payload.error.refId.length).toBeGreaterThan(0); - - const list = (await (await app.request("/")).json()) as { - skills: unknown[]; - }; - expect(list.skills).toHaveLength(0); -}); - -test("PUT /:name creates a new version and leaves the prior one restorable", async () => { - const app = buildApp(); - await createSkill(app); - - const updateResponse = await app.request("/triage", { - method: "PUT", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - description: "Sorts inbound issues by severity.", - body: "Read the report. Pick a severity label.", - }), - }); - expect(updateResponse.status).toBe(200); - const updated = (await updateResponse.json()) as { - skill: { description: string }; - }; - expect(updated.skill.description).toBe("Sorts inbound issues by severity."); - - const loadResponse = await app.request("/triage"); - const loaded = (await loadResponse.json()) as { - skill: { body: string }; - }; - expect(loaded.skill.body).toBe("Read the report. Pick a severity label."); - - const versionsResponse = await app.request("/triage/versions"); - const versions = (await versionsResponse.json()) as { - versions: { commitSha: string; current: boolean }[]; - }; - expect(versions.versions).toHaveLength(2); - const priorVersion = versions.versions[1]; - expect(priorVersion?.current).toBe(false); - - const restoreResponse = await app.request("/triage/restore", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ commitSha: priorVersion?.commitSha }), - }); - expect(restoreResponse.status).toBe(200); - - const restoredLoad = await app.request("/triage"); - const restored = (await restoredLoad.json()) as { - skill: { body: string }; - }; - expect(restored.skill.body).toBe("Read the report. Pick one label."); -}); - -test("GET /:name/versions/:commitSha reads a prior version without cutting one", async () => { - const app = buildApp(); - await createSkill(app); - await app.request("/triage", { - method: "PUT", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - description: "Sorts inbound issues.", - body: "Read the report. Pick a severity label.", - }), - }); - - const versions = (await (await app.request("/triage/versions")).json()) as { - versions: { commitSha: string }[]; - }; - const prior = versions.versions[1]?.commitSha ?? ""; - - const response = await app.request(`/triage/versions/${prior}`); - expect(response.status).toBe(200); - const payload = (await response.json()) as { skill: { body: string } }; - expect(payload.skill.body).toBe("Read the report. Pick one label."); - - const after = (await (await app.request("/triage/versions")).json()) as { - versions: { commitSha: string }[]; - }; - expect(after.versions).toHaveLength(2); - const current = (await (await app.request("/triage")).json()) as { - skill: { body: string }; - }; - expect(current.skill.body).toBe("Read the report. Pick a severity label."); -}); - -test("GET /:name/versions/:commitSha for an unknown commit is a 404", async () => { - const app = buildApp(); - await createSkill(app); - const response = await app.request("/triage/versions/deadbeef"); - expect(response.status).toBe(404); -}); - -async function headSha(app: Hono): Promise { - const versions = (await (await app.request("/triage/versions")).json()) as { - versions: { commitSha: string }[]; - }; - return versions.versions[0]?.commitSha ?? ""; -} - -test("PUT /:name with the current head as expectedHeadSha saves", async () => { - const app = buildApp(); - await createSkill(app); - const response = await app.request("/triage", { - method: "PUT", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - description: "Sorts inbound issues.", - body: "Read the report. Pick a severity label.", - expectedHeadSha: await headSha(app), - }), - }); - expect(response.status).toBe(200); -}); - -test("PUT /:name is a 409 when the skill moved on since the edit started", async () => { - const app = buildApp(); - await createSkill(app); - const staleSha = await headSha(app); - - // Somebody else saves first. - await app.request("/triage", { - method: "PUT", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - description: "Sorts inbound issues.", - body: "Their version.", - }), - }); - - const response = await app.request("/triage", { - method: "PUT", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - description: "Sorts inbound issues.", - body: "My version, written against the old head.", - expectedHeadSha: staleSha, - }), - }); - expect(response.status).toBe(409); - - // The other person's version is still the current one — nothing buried. - const current = (await (await app.request("/triage")).json()) as { - skill: { body: string }; - }; - expect(current.skill.body).toBe("Their version."); -}); - -test("PUT /:name with a malformed expectedHeadSha is a 400", async () => { - const app = buildApp(); - await createSkill(app); - const response = await app.request("/triage", { - method: "PUT", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - description: "Sorts inbound issues.", - body: "x", - expectedHeadSha: "not a sha", - }), - }); - expect(response.status).toBe(400); -}); - -test("GET /:name/versions/:commitSha refuses a version id that isn't one", async () => { - const app = buildApp(); - await createSkill(app); - // Path-shaped and over-long ids are refused at the boundary; a - // well-shaped id that simply isn't in this skill's history is a 404. - expect( - (await app.request("/triage/versions/..%2F..%2Fetc%2Fpasswd")).status, - ).toBe(400); - expect((await app.request(`/triage/versions/${"a".repeat(65)}`)).status).toBe( - 400, - ); - expect((await app.request("/triage/versions/commit9999")).status).toBe(404); -}); - -test("PUT /:name for an unknown skill is a 404", async () => { - const app = buildApp(); - const response = await app.request("/does-not-exist", { - method: "PUT", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ description: "x", body: "y" }), - }); - expect(response.status).toBe(404); -}); - -test("PUT /:name with a malformed body is a 400", async () => { - const app = buildApp(); - await createSkill(app); - const response = await app.request("/triage", { - method: "PUT", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ description: "x" }), - }); - expect(response.status).toBe(400); -}); diff --git a/packages/skills/test/workflow-routes.test.ts b/packages/skills/test/workflow-routes.test.ts deleted file mode 100644 index 1dd8aedfd6..0000000000 --- a/packages/skills/test/workflow-routes.test.ts +++ /dev/null @@ -1,253 +0,0 @@ -import { beforeEach, describe, expect, test } from "bun:test"; -import type { Hono } from "hono"; - -import { createSkillRegistry } from "../src/registry"; -import { - createWorkflowSkillRoutes, - type WorkflowRunAuthenticator, - type WorkflowSkillsEnv, -} from "../src/workflow-routes"; -import { createFakeSkillAssets } from "./fakes"; - -const AUTHOR_TOKEN = "sidecar-token"; -const AUTHOR_ADDRESS = "run-author@runs.example"; -const TEAMMATE_ADDRESS = "run-teammate@runs.example"; - -const SCOPES: Record = { - [AUTHOR_ADDRESS]: { - tenantId: "tenant_1", - principalId: "principal_author", - }, - [TEAMMATE_ADDRESS]: { - tenantId: "tenant_1", - principalId: "principal_teammate", - }, -}; - -const authenticator: WorkflowRunAuthenticator = { - async resolve(token, runAddress) { - if (token !== AUTHOR_TOKEN) return null; - return SCOPES[runAddress] ?? null; - }, -}; - -let app: Hono; - -async function request( - path: string, - init: { - method?: string; - body?: unknown; - address?: string; - token?: string; - } = {}, -): Promise { - const headers: Record = { - authorization: `Bearer ${init.token ?? AUTHOR_TOKEN}`, - "x-workflow-run-address": init.address ?? AUTHOR_ADDRESS, - "content-type": "application/json", - }; - return await app.request(path, { - method: init.method ?? "GET", - headers, - body: init.body === undefined ? undefined : JSON.stringify(init.body), - }); -} - -beforeEach(async () => { - const registry = createSkillRegistry({ - assets: createFakeSkillAssets(), - }); - const author = { tenantId: "tenant_1", principalId: "principal_author" }; - await registry.create(author, { - name: "triage", - description: "Sorts inbound issues.", - body: "Read the report. Pick one label.", - scope: "private", - }); - await registry.create(author, { - name: "summarize", - description: "Condenses a long thread.", - body: "Collect decisions and owners.", - scope: "tenant", - }); - app = createWorkflowSkillRoutes({ authenticator, registry }); -}); - -describe("authentication", () => { - test("a request with no bearer token is rejected", async () => { - const response = await app.request("/list"); - expect(response.status).toBe(401); - }); - - test("an unrecognized sidecar token is rejected", async () => { - expect((await request("/list", { token: "wrong" })).status).toBe(401); - }); - - test("a token with no matching run address is rejected", async () => { - expect( - (await request("/list", { address: "unknown@runs.example" })).status, - ).toBe(401); - }); -}); - -describe("list", () => { - test("serves the calling run's own visible skills, index-only", async () => { - const response = await request("/list"); - expect(response.status).toBe(200); - const body = (await response.json()) as { - data: { name: string; description: string; body?: string }[]; - }; - expect(body.data.map((s) => s.name).sort()).toEqual([ - "summarize", - "triage", - ]); - expect(body.data[0]).not.toHaveProperty("body"); - }); - - test("a different run's principal sees only the tenant-scoped skill", async () => { - const response = await request("/list", { address: TEAMMATE_ADDRESS }); - const body = (await response.json()) as { data: { name: string }[] }; - expect(body.data.map((s) => s.name)).toEqual(["summarize"]); - }); -}); - -describe("search", () => { - test("matches within the caller's own visibility", async () => { - const response = await request("/search", { - method: "POST", - body: { query: "inbound" }, - }); - const body = (await response.json()) as { data: { name: string }[] }; - expect(body.data.map((s) => s.name)).toEqual(["triage"]); - }); - - test("a teammate's search cannot reach the author's private skill", async () => { - const response = await request("/search", { - method: "POST", - body: { query: "inbound" }, - address: TEAMMATE_ADDRESS, - }); - const body = (await response.json()) as { data: unknown[] }; - expect(body.data).toHaveLength(0); - }); - - test("a malformed body is a 400, never an unfiltered listing", async () => { - const response = await request("/search", { - method: "POST", - body: { queries: "inbound" }, - }); - expect(response.status).toBe(400); - }); -}); - -describe("load", () => { - test("returns the full skill body", async () => { - const response = await request("/load", { - method: "POST", - body: { name: "triage" }, - }); - const body = (await response.json()) as { data: { body: string } }; - expect(body.data.body).toBe("Read the report. Pick one label."); - }); - - test("loading a skill the caller cannot see is a 404, never an empty body", async () => { - const response = await request("/load", { - method: "POST", - body: { name: "triage" }, - address: TEAMMATE_ADDRESS, - }); - expect(response.status).toBe(404); - }); -}); - -describe("create", () => { - test("a request with no bearer token is rejected", async () => { - const response = await app.request("/create", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ - name: "escalate", - description: "Escalates blockers.", - body: "Ping the on-call.", - }), - }); - expect(response.status).toBe(401); - }); - - test("creates a tenant-scoped skill regardless of the caller's own scope choice", async () => { - const response = await request("/create", { - method: "POST", - body: { - name: "escalate", - description: "Escalates blockers.", - body: "Ping the on-call.", - }, - }); - expect(response.status).toBe(200); - const body = (await response.json()) as { - data: { name: string; scope: string }; - }; - expect(body.data.name).toBe("escalate"); - expect(body.data.scope).toBe("tenant"); - - const loaded = await request("/load", { - method: "POST", - body: { name: "escalate" }, - address: TEAMMATE_ADDRESS, - }); - expect(loaded.status).toBe(200); - }); -}); - -describe("update", () => { - test("a request with no bearer token is rejected", async () => { - const response = await app.request("/update", { - method: "POST", - headers: { "content-type": "application/json" }, - body: JSON.stringify({ name: "summarize", body: "New body." }), - }); - expect(response.status).toBe(401); - }); - - test("updating an unknown skill is a 404", async () => { - const response = await request("/update", { - method: "POST", - body: { name: "does-not-exist", body: "New body." }, - }); - expect(response.status).toBe(404); - }); - - test("omitting description preserves the skill's current description", async () => { - const response = await request("/update", { - method: "POST", - body: { name: "summarize", body: "Collect decisions, owners, dates." }, - }); - expect(response.status).toBe(200); - const body = (await response.json()) as { - data: { description: string; body?: string }; - }; - expect(body.data.description).toBe("Condenses a long thread."); - - const loaded = await request("/load", { - method: "POST", - body: { name: "summarize" }, - }); - const loadedBody = (await loaded.json()) as { data: { body: string } }; - expect(loadedBody.data.body).toBe("Collect decisions, owners, dates."); - }); - - test("an explicit description replaces the skill's current one", async () => { - const response = await request("/update", { - method: "POST", - body: { - name: "summarize", - body: "Collect decisions.", - description: "Summarizes any thread.", - }, - }); - expect(response.status).toBe(200); - const body = (await response.json()) as { data: { description: string } }; - expect(body.data.description).toBe("Summarizes any thread."); - }); -}); diff --git a/packages/tools-skills/package.json b/packages/tools-skills/package.json index e2f42ba370..ef7a239dc2 100644 --- a/packages/tools-skills/package.json +++ b/packages/tools-skills/package.json @@ -2,7 +2,7 @@ "name": "@corbits/tools-skills", "private": true, "description": "skills_list / skills_search / skills_load as an @intx/agent tool bundle: calls the sanctioned workflow-skills HTTP surface (@corbits/skills) with the sidecar's own bearer token and the run's own address, and fails closed rather than returning an empty registry", - "version": "0.0.2", + "version": "0.0.3", "license": "LGPL-2.1-or-later", "type": "module", "exports": { diff --git a/packages/tools-skills/src/client.test.ts b/packages/tools-skills/src/client.test.ts index 9fbce59e0a..bbcf012ed5 100644 --- a/packages/tools-skills/src/client.test.ts +++ b/packages/tools-skills/src/client.test.ts @@ -8,139 +8,30 @@ const CONFIG = { runAddress: "run@runs.example", }; -function jsonResponse(body: unknown, status = 200): Response { - return new Response(JSON.stringify(body), { - status, - headers: { "content-type": "application/json" }, - }); -} - +// CL-8086: the workflow-skills HTTP surface these calls used to reach was +// deleted, and no stock Interchange route yet serves skill content. Every +// call fails closed with an explicit error rather than degrading to an +// empty registry. describe("listSkills", () => { - test("calls the workflow-skills surface with the run's own credentials", async () => { - let seen: { url: string; headers: Headers } | null = null; - const skills = await listSkills({ - ...CONFIG, - fetchImpl: async (input, init) => { - seen = { - url: String(input), - headers: new Headers(init?.headers), - }; - return jsonResponse({ - data: [{ name: "triage", description: "Sorts issues." }], - }); - }, - }); - expect(skills).toEqual([{ name: "triage", description: "Sorts issues." }]); - const call = seen as unknown as { url: string; headers: Headers }; - expect(call.url).toBe("https://hub.example/api/workflow-skills/list"); - expect(call.headers.get("authorization")).toBe("Bearer sidecar-token"); - expect(call.headers.get("x-workflow-run-address")).toBe("run@runs.example"); - }); - - test("throws on a non-OK response instead of reading as an empty registry", async () => { - expect( - listSkills({ - ...CONFIG, - fetchImpl: async () => jsonResponse({ error: "nope" }, 503), - }), - ).rejects.toThrow(/Skill list failed: 503/); - }); - - test("throws when the response shape does not match", async () => { - expect( - listSkills({ - ...CONFIG, - fetchImpl: async () => jsonResponse({ data: [{ name: "triage" }] }), - }), - ).rejects.toThrow(/did not match the expected shape/); - }); - - test("throws when the transport itself fails", async () => { - expect( - listSkills({ - ...CONFIG, - fetchImpl: async () => { - throw new Error("connection refused"); - }, - }), - ).rejects.toThrow("connection refused"); - }); - - test("refuses to call at all with no sidecar token", async () => { - expect( - listSkills({ - ...CONFIG, - sidecarToken: "", - fetchImpl: async () => jsonResponse({ data: [] }), - }), - ).rejects.toThrow(/no sidecar token/); - }); - - test("refuses to call at all with no run address", async () => { - expect( - listSkills({ - ...CONFIG, - runAddress: "", - fetchImpl: async () => jsonResponse({ data: [] }), - }), - ).rejects.toThrow(/no run address/); + test("fails closed naming the missing stock route", () => { + expect(listSkills(CONFIG)).rejects.toThrow( + /no stock Interchange HTTP route/, + ); }); }); describe("searchSkills", () => { - test("posts the query and returns the index", async () => { - let body = ""; - const skills = await searchSkills( - { - ...CONFIG, - fetchImpl: async (_input, init) => { - body = String(init?.body); - return jsonResponse({ - data: [{ name: "triage", description: "Sorts issues." }], - }); - }, - }, - "issues", + test("fails closed naming the missing stock route", () => { + expect(searchSkills(CONFIG, "issues")).rejects.toThrow( + /no stock Interchange HTTP route/, ); - expect(JSON.parse(body)).toEqual({ query: "issues" }); - expect(skills).toHaveLength(1); - }); - - test("throws on a 401 rather than returning nothing", async () => { - expect( - searchSkills( - { ...CONFIG, fetchImpl: async () => jsonResponse({}, 401) }, - "issues", - ), - ).rejects.toThrow(/Skill search failed: 401/); }); }); describe("loadSkill", () => { - test("returns the skill body", async () => { - const skill = await loadSkill( - { - ...CONFIG, - fetchImpl: async () => - jsonResponse({ - data: { - name: "triage", - description: "Sorts issues.", - body: "Pick one label.", - }, - }), - }, - "triage", + test("fails closed naming the missing stock route", () => { + expect(loadSkill(CONFIG, "triage")).rejects.toThrow( + /no stock Interchange HTTP route/, ); - expect(skill.body).toBe("Pick one label."); - }); - - test("throws when the skill is not visible to the run", async () => { - expect( - loadSkill( - { ...CONFIG, fetchImpl: async () => jsonResponse({}, 404) }, - "triage", - ), - ).rejects.toThrow(/Skill load failed: 404/); }); }); diff --git a/packages/tools-skills/src/client.ts b/packages/tools-skills/src/client.ts index d58dcabf4e..0c5427d30b 100644 --- a/packages/tools-skills/src/client.ts +++ b/packages/tools-skills/src/client.ts @@ -1,32 +1,18 @@ -// A minimal client for the sanctioned workflow-skills HTTP surface -// (`@corbits/skills`' `createWorkflowSkillRoutes`): list, search, and -// load against the tenant's skill registry. Authenticates with the -// sidecar's own bearer token plus the run's own mailbox address — both -// already reach a workflow-process child's tool env — never a database -// handle, and never a model-supplied tenant or principal. -// -// Every function throws on any transport, HTTP, or shape failure. There -// is deliberately no degraded path: a registry that cannot be reached -// must never read as "this workbench has no skills", because an agent -// told its skills are absent will confidently proceed without them. -import { type } from "arktype"; - -/** - * The subset of `fetch` this client uses. Narrower than `typeof fetch` - * so a test double is a plain function rather than something that has to - * reimplement the runtime's extra members. - */ -export type FetchLike = ( - input: string, - init?: RequestInit, -) => Promise; - +// CL-8086: the Workbench skill registry (`@corbits/skills`' HTTP surface +// this client used to call, mounted at `/api/workflow-skills`) was +// deleted — skills are native `kind:"skill"` hub assets now, and there is +// no stock Interchange route yet that lets a workflow-run bearer identity +// list a skill's index or read its `SKILL.md` body (the stock asset +// routes in `@intx/hub-api`'s `routes/assets.ts` cover asset metadata — +// id/name/displayName — and package-registry tarballs, but no skill +// content). Rather than vendor a replacement surface or silently degrade +// to "no skills", every call here fails closed with an explicit error +// naming the gap, exactly as this package's own design already commits +// to doing for any registry it cannot reach. export interface WorkflowSkillsClientConfig { readonly hubSkillsUrl: string; readonly sidecarToken: string; readonly runAddress: string; - /** Override for tests; defaults to the global `fetch`. */ - readonly fetchImpl?: FetchLike; } export type SkillIndexItem = { @@ -36,108 +22,30 @@ export type SkillIndexItem = { export type LoadedSkill = SkillIndexItem & { readonly body: string }; -const SkillIndexResponse = type({ - data: type({ name: "string", description: "string" }).array(), -}); - -const LoadedSkillResponse = type({ - data: { name: "string", description: "string", body: "string" }, -}); - -function authHeaders( - config: WorkflowSkillsClientConfig, -): Record { - return { - authorization: `Bearer ${config.sidecarToken}`, - "x-workflow-run-address": config.runAddress, - }; -} - -function endpoint(config: WorkflowSkillsClientConfig, path: string): string { - return `${config.hubSkillsUrl}/api/workflow-skills${path}`; -} - -function assertCredentials(config: WorkflowSkillsClientConfig): void { - if (config.sidecarToken === "") { - throw new Error("Skill registry call has no sidecar token to present"); - } - if (config.runAddress === "") { - throw new Error("Skill registry call has no run address to present"); - } - if (config.hubSkillsUrl === "") { - throw new Error("Skill registry call has no hub URL to reach"); - } -} - -async function parseIndex( - response: Response, - label: string, -): Promise { - if (!response.ok) { - throw new Error( - `${label} failed: ${String(response.status)} ${response.statusText}`, - ); - } - const body: unknown = await response.json(); - const parsed = SkillIndexResponse(body); - if (parsed instanceof type.errors) { - throw new Error( - `${label} response did not match the expected shape: ${parsed.summary}`, - ); - } - return parsed.data; -} +const NO_STOCK_SKILL_CONTENT_ROUTE = + "Skill content has no stock Interchange HTTP route (CL-8086): the " + + "workbench-specific skills registry that used to serve it was removed, " + + "and no replacement has been added to @intx/hub-api yet."; /** Lists every skill the run's principal can see. */ -export async function listSkills( - config: WorkflowSkillsClientConfig, +export function listSkills( + _config: WorkflowSkillsClientConfig, ): Promise { - assertCredentials(config); - const doFetch = config.fetchImpl ?? fetch; - const response = await doFetch(endpoint(config, "/list"), { - headers: authHeaders(config), - }); - return parseIndex(response, "Skill list"); + return Promise.reject(new Error(NO_STOCK_SKILL_CONTENT_ROUTE)); } /** Searches the visible skills by name and description. */ -export async function searchSkills( - config: WorkflowSkillsClientConfig, - query: string, +export function searchSkills( + _config: WorkflowSkillsClientConfig, + _query: string, ): Promise { - assertCredentials(config); - const doFetch = config.fetchImpl ?? fetch; - const response = await doFetch(endpoint(config, "/search"), { - method: "POST", - headers: { ...authHeaders(config), "content-type": "application/json" }, - body: JSON.stringify({ query }), - }); - return parseIndex(response, "Skill search"); + return Promise.reject(new Error(NO_STOCK_SKILL_CONTENT_ROUTE)); } /** Reads one skill's full instructions. */ -export async function loadSkill( - config: WorkflowSkillsClientConfig, - name: string, +export function loadSkill( + _config: WorkflowSkillsClientConfig, + _name: string, ): Promise { - assertCredentials(config); - const doFetch = config.fetchImpl ?? fetch; - const response = await doFetch(endpoint(config, "/load"), { - method: "POST", - headers: { ...authHeaders(config), "content-type": "application/json" }, - body: JSON.stringify({ name }), - }); - if (!response.ok) { - throw new Error( - `Skill load failed: ${String(response.status)} ${response.statusText}`, - ); - } - const body: unknown = await response.json(); - const parsed = LoadedSkillResponse(body); - if (parsed instanceof type.errors) { - throw new Error( - `Skill load response did not match the expected shape: ${parsed.summary}`, - ); - } - return parsed.data; + return Promise.reject(new Error(NO_STOCK_SKILL_CONTENT_ROUTE)); } diff --git a/packages/tools-skills/src/index.ts b/packages/tools-skills/src/index.ts index b10bb2827e..749aad8563 100644 --- a/packages/tools-skills/src/index.ts +++ b/packages/tools-skills/src/index.ts @@ -2,7 +2,6 @@ export { listSkills, loadSkill, searchSkills, - type FetchLike, type LoadedSkill, type SkillIndexItem, type WorkflowSkillsClientConfig, diff --git a/packages/tools-skills/src/tool.test.ts b/packages/tools-skills/src/tool.test.ts index 9c15ec756d..ccd4d8c589 100644 --- a/packages/tools-skills/src/tool.test.ts +++ b/packages/tools-skills/src/tool.test.ts @@ -1,4 +1,4 @@ -import { afterEach, expect, test } from "bun:test"; +import { expect, test } from "bun:test"; import type { ToolCall } from "@intx/types/runtime"; import { @@ -9,12 +9,6 @@ import { } from "./tool"; import type { WorkflowSkillsToolEnv } from "./tool"; -const originalFetch = globalThis.fetch; - -afterEach(() => { - globalThis.fetch = originalFetch; -}); - function testEnv(): WorkflowSkillsToolEnv { return { hubSkillsUrl: "https://hub.example.com", @@ -27,10 +21,6 @@ function callFor(name: string, args: Record = {}): ToolCall { return { id: "call_1", name, arguments: args }; } -function stubFetch(respond: () => Response | Promise): void { - globalThis.fetch = (async () => respond()) as unknown as typeof fetch; -} - test("declares skills_list, skills_search, and skills_load", () => { const bundle = skillsTools(testEnv()); expect(bundle.definitions.map((d) => d.name)).toEqual([ @@ -61,34 +51,16 @@ test("no tool's input schema accepts a tenant or principal argument", () => { } }); -test("skills_list returns the registry index on a successful call", async () => { - stubFetch( - () => - new Response( - JSON.stringify({ - data: [{ name: "triage", description: "Sorts issues." }], - }), - ), - ); - const result = await skillsTools(testEnv()).run( - callFor(SKILLS_LIST_TOOL), - new AbortController().signal, - ); - expect(result.isError).toBeFalsy(); - const parsed = JSON.parse(String(result.content)) as { - skills: { name: string }[]; - }; - expect(parsed.skills.map((s) => s.name)).toEqual(["triage"]); -}); - -test("skills_list surfaces an unreachable registry as an error, never an empty list", async () => { - stubFetch(() => new Response("nope", { status: 503 })); +// CL-8086: the registry HTTP surface this client used to call was +// deleted, and no stock Interchange route yet serves skill content, so +// every call fails closed rather than reaching a live registry. +test("skills_list surfaces the missing stock route as an error, never an empty list", async () => { const result = await skillsTools(testEnv()).run( callFor(SKILLS_LIST_TOOL), new AbortController().signal, ); expect(result.isError).toBe(true); - expect(String(result.content)).toContain("503"); + expect(String(result.content)).toContain("no stock Interchange HTTP route"); }); test("skills_search requires a query", async () => { @@ -100,35 +72,13 @@ test("skills_search requires a query", async () => { expect(String(result.content)).toContain("requires a query"); }); -test("skills_load returns the skill body", async () => { - stubFetch( - () => - new Response( - JSON.stringify({ - data: { - name: "triage", - description: "Sorts issues.", - body: "Pick one label.", - }, - }), - ), - ); - const result = await skillsTools(testEnv()).run( - callFor(SKILLS_LOAD_TOOL, { name: "triage" }), - new AbortController().signal, - ); - expect(result.isError).toBeFalsy(); - expect(String(result.content)).toContain("Pick one label."); -}); - -test("skills_load surfaces a missing skill as an error rather than inventing one", async () => { - stubFetch(() => new Response("nope", { status: 404 })); +test("skills_load surfaces the missing stock route as an error rather than inventing one", async () => { const result = await skillsTools(testEnv()).run( callFor(SKILLS_LOAD_TOOL, { name: "triage" }), new AbortController().signal, ); expect(result.isError).toBe(true); - expect(String(result.content)).toContain("404"); + expect(String(result.content)).toContain("no stock Interchange HTTP route"); }); test("skills_load requires a name", async () => { diff --git a/workflows/assistant/src/index.ts b/workflows/assistant/src/index.ts index d0f5250108..aefafa0277 100644 --- a/workflows/assistant/src/index.ts +++ b/workflows/assistant/src/index.ts @@ -52,7 +52,7 @@ export const ASSISTANT_TOOL_PACKAGE_PINS: readonly ToolPackagePin[] = [ { name: "@corbits/agent-directory-tools", version: "0.0.6" }, { name: "@corbits/connections-tools", version: "0.0.8" }, { name: "@corbits/catalog-tools", version: "0.0.5" }, - { name: "@corbits/skills-tools", version: "0.0.6" }, + { name: "@corbits/skills-tools", version: "0.0.7" }, { name: "@corbits/mcp-tools", version: "0.0.11" }, { name: "@corbits/interaction-tools", version: "0.0.8" }, { name: "@corbits/manus-tools", version: "0.0.11" }, From 148a18e02962081b331a9e69cf52888fb108026f Mon Sep 17 00:00:00 2001 From: Sawyer Date: Wed, 16 Sep 2026 22:14:32 -0700 Subject: [PATCH 2/7] feat(web): read skills from stock kind:skill asset routes (CL-8086) The roster lists asset metadata and creates bare skill assets; the detail page renders the display title, updated note, and an honest placeholder where registry-only content (body, versions, pins, scope) lived until a stock content route exists. --- apps/web/src/pages/agent-skills-picker.tsx | 8 +- apps/web/src/pages/create-skill-dialog.tsx | 230 +++------ apps/web/src/pages/plugins-page.tsx | 20 +- apps/web/src/pages/skill-detail-page.tsx | 573 +++------------------ apps/web/src/pages/skills-page.tsx | 29 +- apps/web/src/skill-display-name.test.ts | 9 + apps/web/src/skill-display-name.ts | 7 +- apps/web/src/skills-api.ts | 246 +++------ 8 files changed, 219 insertions(+), 903 deletions(-) diff --git a/apps/web/src/pages/agent-skills-picker.tsx b/apps/web/src/pages/agent-skills-picker.tsx index acaa774861..46699ae6a5 100644 --- a/apps/web/src/pages/agent-skills-picker.tsx +++ b/apps/web/src/pages/agent-skills-picker.tsx @@ -73,9 +73,8 @@ export function AgentSkillsPicker({ } const visibleNames = new Set(state.skills.map((skill) => skill.name)); - // A pin can outlive its skill's visibility — the author made it - // private, it was renamed, or it was discarded — leaving a name in - // `selected` with no matching registry entry. Render those as + // A pin can outlive its skill — it was renamed or discarded — leaving + // a name in `selected` with no matching registry entry. Render those as // removable rows rather than silently dropping them, so the picker // can always reach a saveable state. const staleNames = selected.filter((name) => !visibleNames.has(name)); @@ -97,8 +96,7 @@ export function AgentSkillsPicker({ {name} - No longer available — its author may have made it private, - renamed it, or discarded it. + No longer available — it may have been renamed or discarded. diff --git a/apps/web/src/pages/plugins-page.tsx b/apps/web/src/pages/plugins-page.tsx index e618ce5831..9f1deea4b8 100644 --- a/apps/web/src/pages/plugins-page.tsx +++ b/apps/web/src/pages/plugins-page.tsx @@ -39,12 +39,7 @@ import { useRequestPluginsConnect, } from "../shell/provider-health-context"; import { StageTopBar } from "../shell/stage-top-bar"; -import { - createSkill, - createSkillFromFile, - listSkills, - type SkillSummary, -} from "../skills-api"; +import { createSkill, listSkills, type SkillSummary } from "../skills-api"; import { CreateSkillDialog, type SkillCreateInput, @@ -261,14 +256,10 @@ export function PluginsRoute({ async function handleCreateSkill(input: SkillCreateInput) { if (selectedTenantId === null) return; - const skill = - input.kind === "file" - ? await createSkillFromFile(selectedTenantId, input.source) - : await createSkill(selectedTenantId, { - name: input.name, - description: input.description, - body: input.body, - }); + const skill = await createSkill(selectedTenantId, { + name: input.name, + ...(input.displayName !== "" ? { displayName: input.displayName } : {}), + }); setCreateSkillOpen(false); reloadSkills(); openSkill(skill.name); @@ -330,7 +321,6 @@ export function PluginsRoute({ assetId: skill.assetId, name: skill.name, description: skill.description, - scope: skill.scope, })); return ( diff --git a/apps/web/src/pages/skill-detail-page.tsx b/apps/web/src/pages/skill-detail-page.tsx index c5e077d692..ea9fc56353 100644 --- a/apps/web/src/pages/skill-detail-page.tsx +++ b/apps/web/src/pages/skill-detail-page.tsx @@ -1,111 +1,45 @@ -// The skill detail page at `/skills/` (CL-6416). One full page per -// skill, replacing the placeholder that stood at this route and absorbing -// the version-history panel the Skills roster used to carry inline — there -// is one skill editor now, and it lives here. +// The skill detail page at `/skills/` (CL-6416). // -// Versioning is git: every save is a commit on the skill's own SKILL.md -// (`@corbits/skills`), so "the version list" is that commit history and -// "restore" is a new commit carrying an older content. Nothing on this -// page stores a version itself. -// -// A save is never silent, and never blind: -// -// * "Review" opens a review step showing the diff between the published -// version and what is in the editor; the commit happens only on -// confirm. -// * The save carries the version the editor was seeded from, so if -// somebody else saved in the meantime the registry refuses it (409) -// and the review re-opens against what is actually published now, -// rather than burying their work. -// -// The editor buffer is newline-normalized, so the bytes reviewed in the -// diff are exactly the bytes the confirm writes. -// -// The page reads top to bottom in one column: who can see the skill, the -// description agents match on, the body, then the full version history. -// History was a narrow side column, which left every one of its columns -// truncated to a stub — a table nobody can read teaches nothing, so it now -// gets the full width it needs. - +// CL-8086: this used to be a full editor over `@corbits/skills`' own +// registry — description/body editing with diff review, restore-by- +// version off that asset's git history, a "pinned by" list, and a +// private/shared visibility toggle. That registry (and the workflow +// routes it served) was deleted: skills are native `kind:"skill"` hub +// assets now, and the stock asset routes (`@intx/hub-api`'s +// `routes/assets.ts`) carry only metadata — id, name, displayName, +// creator, timestamps. There is no stock route yet to read a skill's +// SKILL.md content, its version history, who has it pinned, or a +// scope/visibility flag, so none of that can be rendered here without +// vendoring a replacement surface. This page is scoped down to what the +// stock routes actually carry until one exists — see the CL-8086 PR body +// for the gap. import { - Badge, - Button, - Dialog, - DialogBody, - DialogContent, - DialogDescription, - DialogFooter, - DialogHeader, - DialogTitle, PageShell, RichEmptyState, Section, - Table, - TableBody, - TableCell, - TableHead, - TableHeader, - TableRow, - Textarea, formatRelativeTime, } from "@corbits/react-ui"; -import { GitDiff, Lightning } from "@corbits/icons"; +import { Lightning } from "@corbits/icons"; import { WorkbenchLoadingState } from "@corbits/chat-ui"; import { ApiQueryError, describeApiError } from "@corbits/api-query"; -import { normalizeNewlines } from "@corbits/text-diff"; import { useCallback, useEffect, useState, type ReactNode } from "react"; import { useBench } from "../bench-context"; import { SKILLS_PATH_PREFIX, skillIdFromPath } from "../path-ids"; import { skillDisplayName } from "../skill-display-name"; -import { skillVersionSavedBy } from "../skill-version-author"; import { StageTopBar } from "../shell/stage-top-bar"; -import { - listSkillVersions, - loadSkill, - loadSkillAtVersion, - restoreSkillVersion, - setSkillScope, - updateSkill, - type PinnedByEntry, - type SkillDetail, - type SkillVersion, -} from "../skills-api"; -import { DiffHeading, DiffView } from "./diff-view"; - -type Draft = { readonly description: string; readonly body: string }; - -type Loaded = { - readonly skill: SkillDetail; - readonly pinnedBy: readonly PinnedByEntry[]; - readonly versions: readonly SkillVersion[]; -}; +import { loadSkill, type SkillSummary } from "../skills-api"; type PageState = | { readonly status: "loading" } - | ({ readonly status: "ready" } & Loaded) + | { readonly status: "ready"; readonly skill: SkillSummary } | { readonly status: "missing" } | { readonly status: "error"; readonly message: string }; -type Comparison = { readonly version: SkillVersion; readonly body: string }; - function statusOf(cause: unknown): number | undefined { return cause instanceof ApiQueryError ? cause.status : undefined; } -/** "Version 3 of 7" numbering: history is newest-first, so a row's number - * counts up from the oldest commit. */ -function versionLabel(total: number, index: number): string { - return `Version ${String(total - index)}`; -} - -function draftOf(skill: SkillDetail): Draft { - return { - description: normalizeNewlines(skill.description), - body: normalizeNewlines(skill.body), - }; -} - export function SkillDetailPage({ tenantId, name, @@ -116,59 +50,27 @@ export function SkillDetailPage({ readonly now?: number; }) { const [state, setState] = useState({ status: "loading" }); - const [draft, setDraft] = useState(null); - const [confirming, setConfirming] = useState(false); - const [saveError, setSaveError] = useState(null); - const [staleNotice, setStaleNotice] = useState(null); - const [actionError, setActionError] = useState(null); - const [busy, setBusy] = useState(false); - const [comparison, setComparison] = useState(null); - /** Reads the skill and its history. `keepDraft` protects unsaved edits: - * a background re-read must never quietly discard what someone typed. */ - const read = useCallback( - async (options: { - readonly keepDraft: boolean; - }): Promise => { - if (tenantId === null) return null; - if (!options.keepDraft) setState({ status: "loading" }); - try { - const [detail, versions] = await Promise.all([ - loadSkill(tenantId, name), - listSkillVersions(tenantId, name), - ]); - const loaded: Loaded = { - skill: detail.skill, - pinnedBy: detail.pinnedBy, - versions, - }; - setState({ status: "ready", ...loaded }); - if (!options.keepDraft) { - setDraft(draftOf(detail.skill)); - setComparison(null); - } - return loaded; - } catch (cause) { - if (options.keepDraft) { - setActionError(describeApiError(cause, "re-reading this skill")); - return null; - } - setState( - statusOf(cause) === 404 - ? { status: "missing" } - : { - status: "error", - message: describeApiError(cause, "loading this skill"), - }, - ); - return null; - } - }, - [tenantId, name], - ); + const read = useCallback(async (): Promise => { + if (tenantId === null) return; + setState({ status: "loading" }); + try { + const { skill } = await loadSkill(tenantId, name); + setState({ status: "ready", skill }); + } catch (cause) { + setState( + statusOf(cause) === 404 + ? { status: "missing" } + : { + status: "error", + message: describeApiError(cause, "loading this skill"), + }, + ); + } + }, [tenantId, name]); useEffect(() => { - void read({ keepDraft: false }); + void read(); }, [read]); const crumbs = [ @@ -176,10 +78,10 @@ export function SkillDetailPage({ { label: name }, ]; - function frame(actions: ReactNode, body: ReactNode) { + function frame(body: ReactNode) { return (
- +
{body} @@ -191,7 +93,6 @@ export function SkillDetailPage({ if (tenantId === null) { return frame( - null,

Pick a workbench to see this skill.

, @@ -200,7 +101,6 @@ export function SkillDetailPage({ if (state.status === "missing") { return frame( - null, } title={`No skill named “${name}”`} @@ -212,400 +112,43 @@ export function SkillDetailPage({ if (state.status === "error") { return frame( - null, } title="Couldn't load this skill" description={state.message} - actions={[ - { label: "Retry", onClick: () => void read({ keepDraft: false }) }, - ]} + actions={[{ label: "Retry", onClick: () => void read() }]} />, ); } - if (state.status === "loading" || draft === null) { - return frame(null, ); + if (state.status === "loading") { + return frame(); } - const registryTenantId: string = tenantId; - const { skill, pinnedBy, versions } = state; - const published = draftOf(skill); - const headSha = - versions.find((version) => version.current)?.commitSha ?? null; - const shared = skill.scope === "tenant"; - const edited = - draft.body !== published.body || - draft.description !== published.description; - - /** A side action (visibility, restore, compare): its failure belongs next - * to the action, never in place of the whole page, and its success never - * throws away a dirty draft. */ - async function runSideAction(action: () => Promise) { - setActionError(null); - setBusy(true); - try { - await action(); - await read({ keepDraft: edited }); - } catch (cause) { - setActionError(describeApiError(cause, "saving that change")); - } finally { - setBusy(false); - } - } - - async function confirmSave() { - if (draft === null) return; - setSaveError(null); - setBusy(true); - try { - await updateSkill(registryTenantId, skill.name, { - description: draft.description, - body: draft.body, - expectedHeadSha: headSha, - }); - setConfirming(false); - setStaleNotice(null); - await read({ keepDraft: false }); - } catch (cause) { - if (statusOf(cause) === 409) { - // Somebody else published while this edit was open. Re-read, keep - // the typed edit, and leave the review open — now diffing against - // what is actually published. - const reread = await read({ keepDraft: true }); - setStaleNotice( - reread === null - ? "Someone else saved this skill while you were editing. Reload to see their version." - : "Someone else saved this skill while you were editing. The diff below now compares your edit with their version — confirm to save on top of it.", - ); - } else { - setSaveError(describeApiError(cause, "saving this skill")); - } - } finally { - setBusy(false); - } - } - - async function compare(version: SkillVersion) { - if (comparison?.version.commitSha === version.commitSha) { - setComparison(null); - return; - } - setActionError(null); - try { - const at = await loadSkillAtVersion( - registryTenantId, - skill.name, - version.commitSha, - ); - setComparison({ version, body: at.body }); - } catch (cause) { - setActionError(describeApiError(cause, "reading that version")); - } - } - - const saveAction = ( - - ); + const { skill } = state; return frame( - saveAction,
-
-
-

- {skillDisplayName(skill)} -

-

- Updated {formatRelativeTime(skill.updatedAtIso, now)} -

-
-
-

- {shared - ? "Everyone in this workbench can use this skill." - : "Only you can use this skill."} -

- -
-
- - {actionError === null ? null : ( -

- {actionError} +

+

+ {skillDisplayName(skill)} +

+

+ Updated {formatRelativeTime(skill.updatedAtIso, now)}

- )} - -
-
-