diff --git a/apps/web/src/pages/create-skill-dialog.tsx b/apps/web/src/pages/create-skill-dialog.tsx index 24f407fcd..224cb8ae4 100644 --- a/apps/web/src/pages/create-skill-dialog.tsx +++ b/apps/web/src/pages/create-skill-dialog.tsx @@ -1,17 +1,22 @@ -// The create-skill form: identity (name, description) and the skill -// body itself (the instructions/tools text). Submitting hands the -// values to `onSubmit`, which the Skills page turns directly into a +// The create-skill dialog: paste a skill's fields directly, or upload a +// `SKILL.md` file and let the registry parse them out. Submitting hands +// the result to `onSubmit`, which the Skills page turns directly into a // native `kind:"skill"` asset write in the workbench's registry -// (`../skills-api.ts`) — there is no intermediate draft. A rejection -// from that call (a name conflict, or SKILL.md frontmatter the registry -// refuses) surfaces inline here rather than closing the dialog, so the -// person never loses what they typed. +// (`../skills-api.ts`) — there is no intermediate draft, and both modes +// converge on the same registry `create`. A rejection from that call (a +// name conflict, or SKILL.md frontmatter the registry refuses) surfaces +// inline here rather than closing the dialog, so the person never loses +// what they typed or which file they picked. // // The name field is bound by the registry's own rule — lowercase // letters, digits, and hyphens — because that is what a SKILL.md's // frontmatter must carry. Rejecting it here beats a server error after // the person has typed a whole skill body. // +// A skill is one `SKILL.md`, not a bundle: the registry stores a single +// file per skill asset, so upload is deliberately single-file — no +// folders, no zips, no attachments to half-support. +// // Creation only. Editing an existing skill happens on its own page // (`skill-detail-page.tsx`, CL-6416), where a save is reviewed as a diff // before it publishes a new version — this dialog has no edit mode to @@ -26,17 +31,29 @@ import { DialogFooter, DialogHeader, DialogTitle, + FileInput, IntakeForm, + Tabs, intakeFieldsComplete, } from "@corbits/react-ui"; import type { IntakeField } from "@corbits/react-ui"; import { useState } from "react"; -export type SkillCreateInput = { - readonly name: string; - readonly description: string; - readonly body: string; -}; +export type SkillCreateInput = + | { + readonly kind: "fields"; + readonly name: string; + readonly description: string; + readonly body: string; + } + | { + readonly kind: "file"; + /** The uploaded file's raw text — the registry parses it with the + * same `parseSkillMd` it reads a skill back with. */ + readonly source: string; + }; + +type Mode = "paste" | "upload"; /** Mirrors `@corbits/skills`' `skillNameSchema` — kebab-case, `<=64`. */ const SKILL_NAME_PATTERN = /^[a-z0-9]+(-[a-z0-9]+)*$/; @@ -114,13 +131,17 @@ export function CreateSkillDialog({ * inline and the form is left as typed. */ readonly onSubmit: (input: SkillCreateInput) => Promise; }) { + const [mode, setMode] = useState("paste"); const [values, setValues] = useState(EMPTY_VALUES); + const [file, setFile] = useState(null); const [showIssues, setShowIssues] = useState(false); const [serverError, setServerError] = useState(null); const [submitting, setSubmitting] = useState(false); function reset() { + setMode("paste"); setValues(EMPTY_VALUES); + setFile(null); setShowIssues(false); setServerError(null); } @@ -130,6 +151,12 @@ export function CreateSkillDialog({ onOpenChange(next); } + function handleModeChange(next: Mode) { + setMode(next); + setShowIssues(false); + setServerError(null); + } + function handleFormChange(next: Record) { setValues({ name: typeof next.name === "string" ? next.name : values.name, @@ -141,18 +168,27 @@ export function CreateSkillDialog({ const issues = validationIssues(values); async function handleSubmit() { - if (issues.length > 0) { + if (mode === "paste" && issues.length > 0) { + setShowIssues(true); + return; + } + if (mode === "upload" && file === null) { setShowIssues(true); return; } setServerError(null); setSubmitting(true); try { - await onSubmit({ - name: values.name.trim(), - description: values.description.trim(), - body: values.body.trim(), - }); + if (mode === "upload" && file !== null) { + await onSubmit({ kind: "file", source: await file.text() }); + } else { + await onSubmit({ + kind: "fields", + name: values.name.trim(), + description: values.description.trim(), + body: values.body.trim(), + }); + } reset(); } catch (cause) { setServerError(cause instanceof Error ? cause.message : String(cause)); @@ -172,27 +208,64 @@ export function CreateSkillDialog({ - {showIssues && issues.length > 0 && ( -
    - {issues.map((issue) => ( -
  • {issue}
    • - ))} -
- )} {serverError !== null && (

{serverError}

)} - + + {(active) => + active === "paste" ? ( + <> + {showIssues && issues.length > 0 && ( +
    + {issues.map((issue) => ( +
  • {issue}
    • + ))} +
+ )} + + + ) : ( + <> + {showIssues && file === null && ( +

+ Choose a SKILL.md file to upload. +

+ )} + setFile(files[0] ?? null)} + /> + {file !== null && ( +

+ {file.name} selected +

+ )} + + ) + } + diff --git a/apps/web/src/pages/plugins-page.tsx b/apps/web/src/pages/plugins-page.tsx index 795e6ca99..6fdb775fa 100644 --- a/apps/web/src/pages/plugins-page.tsx +++ b/apps/web/src/pages/plugins-page.tsx @@ -37,7 +37,12 @@ import { usePendingConnectProvider, } from "../shell/provider-health-context"; import { StageTopBar } from "../shell/stage-top-bar"; -import { createSkill, listSkills, type SkillSummary } from "../skills-api"; +import { + createSkill, + createSkillFromFile, + listSkills, + type SkillSummary, +} from "../skills-api"; import { CreateSkillDialog, type SkillCreateInput, @@ -145,7 +150,14 @@ export function PluginsRoute({ async function handleCreateSkill(input: SkillCreateInput) { if (selectedTenantId === null) return; - const skill = await createSkill(selectedTenantId, input); + const skill = + input.kind === "file" + ? await createSkillFromFile(selectedTenantId, input.source) + : await createSkill(selectedTenantId, { + name: input.name, + description: input.description, + body: input.body, + }); setCreateSkillOpen(false); reloadSkills(); setOpenSkillName(skill.name); diff --git a/apps/web/src/pages/skills-page.tsx b/apps/web/src/pages/skills-page.tsx index 2a9ce96f1..a55b59a16 100644 --- a/apps/web/src/pages/skills-page.tsx +++ b/apps/web/src/pages/skills-page.tsx @@ -35,7 +35,12 @@ import { useCallback, useEffect, useState, type ReactNode } from "react"; import { rowActivationProps } from "../activatable-row"; import { consumePendingNewSkill } from "../command-palette-actions"; -import { createSkill, listSkills, type SkillSummary } from "../skills-api"; +import { + createSkill, + createSkillFromFile, + listSkills, + type SkillSummary, +} from "../skills-api"; import { CreateSkillDialog, type SkillCreateInput, @@ -107,7 +112,14 @@ export function SkillsPage({ async function handleCreate(input: SkillCreateInput) { if (tenantId === null) return; - const skill = await createSkill(tenantId, input); + const skill = + input.kind === "file" + ? await createSkillFromFile(tenantId, input.source) + : await createSkill(tenantId, { + name: input.name, + description: input.description, + body: input.body, + }); setCreateOpen(false); await reload(); open(skill.name); diff --git a/apps/web/src/skills-api.ts b/apps/web/src/skills-api.ts index 18e2c90ef..f5e25a018 100644 --- a/apps/web/src/skills-api.ts +++ b/apps/web/src/skills-api.ts @@ -182,6 +182,22 @@ export function createSkill( }).then((page) => page.skill); } +/** + * Creates a skill from an uploaded `SKILL.md`'s raw contents. The registry + * parses `source` with the same `parseSkillMd` it reads a skill back with — + * name, description, and body come from the file, never from a client-side + * guess — and rejects a malformed file without creating anything. + */ +export function createSkillFromFile( + tenantId: string, + source: string, +): Promise { + return request(base(tenantId), SkillResponse, { + method: "POST", + body: { source, scope: DEFAULT_CREATE_SCOPE }, + }).then((page) => page.skill); +} + /** * Republishes a skill's description/body as a new version on its same * asset — scope untouched, name untouched (only the author may rename by diff --git a/apps/web/test/skills-page.test.tsx b/apps/web/test/skills-page.test.tsx index 4c61433a6..9bc62315d 100644 --- a/apps/web/test/skills-page.test.tsx +++ b/apps/web/test/skills-page.test.tsx @@ -192,6 +192,146 @@ describe("SkillsPage", () => { expect(navigated).toContain("/skills/summarize"); }); + test("Upload SKILL.md posts its parsed source and opens the new skill's page", async () => { + stubRoutes({ + ...EMPTY_REGISTRY, + [`POST /api/tenants/${TENANT}/skills`]: { + skill: { ...TRIAGE, name: "summarize" }, + }, + }); + const navigated: string[] = []; + const el = await mount({ navigate: (to) => navigated.push(to) }); + + const newSkill = Array.from(el.querySelectorAll("button")).find((button) => + button.textContent?.includes("New skill"), + ); + await act(async () => { + newSkill?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + const uploadTab = Array.from( + document.body.querySelectorAll("[role='tab']"), + ).find((tab) => tab.textContent === "Upload"); + await act(async () => { + uploadTab?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + const source = [ + "---", + "name: summarize", + "description: 'Condenses.'", + "---", + "", + "Do it.", + "", + ].join("\n"); + const fileInput = document.body.querySelector( + 'input[type="file"]', + ) as HTMLInputElement; + expect(fileInput).not.toBeNull(); + const file = new File([source], "SKILL.md", { type: "text/markdown" }); + const transfer = new DataTransfer(); + transfer.items.add(file); + await act(async () => { + Object.defineProperty(fileInput, "files", { + value: transfer.files, + configurable: true, + }); + fileInput.dispatchEvent(new Event("change", { bubbles: true })); + }); + + const create = Array.from(document.body.querySelectorAll("button")).find( + (button) => button.textContent === "Create skill", + ); + expect(create?.hasAttribute("disabled")).toBe(false); + await act(async () => { + create?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await act(async () => { + await Promise.resolve(); + }); + + const call = requested.find( + (entry) => entry.method === "POST" && entry.path.endsWith("/skills"), + ); + expect(call?.body).toEqual({ source, scope: "private" }); + expect(navigated).toContain("/skills/summarize"); + }); + + test("uploading a malformed SKILL.md surfaces the parse error and creates nothing", async () => { + globalThis.fetch = (async (input: unknown, init?: RequestInit) => { + const path = String(input); + const method = init?.method ?? "GET"; + requested.push({ + method, + path, + body: + init?.body === undefined ? undefined : JSON.parse(String(init.body)), + }); + if (method === "POST" && path.endsWith("/skills")) { + return new Response( + JSON.stringify({ + error: { + message: "SKILL.md is missing its YAML frontmatter delimiter", + }, + }), + { status: 400 }, + ); + } + return new Response(JSON.stringify({ skills: [] }), { status: 200 }); + }) as unknown as typeof fetch; + + const el = await mount(); + const newSkill = Array.from(el.querySelectorAll("button")).find((button) => + button.textContent?.includes("New skill"), + ); + await act(async () => { + newSkill?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + const uploadTab = Array.from( + document.body.querySelectorAll("[role='tab']"), + ).find((tab) => tab.textContent === "Upload"); + await act(async () => { + uploadTab?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + + const fileInput = document.body.querySelector( + 'input[type="file"]', + ) as HTMLInputElement; + const file = new File(["not a skill file"], "SKILL.md", { + type: "text/markdown", + }); + const transfer = new DataTransfer(); + transfer.items.add(file); + await act(async () => { + Object.defineProperty(fileInput, "files", { + value: transfer.files, + configurable: true, + }); + fileInput.dispatchEvent(new Event("change", { bubbles: true })); + }); + + const create = Array.from(document.body.querySelectorAll("button")).find( + (button) => button.textContent === "Create skill", + ); + await act(async () => { + create?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await act(async () => { + await Promise.resolve(); + }); + + expect(document.body.textContent).toContain( + "SKILL.md is missing its YAML frontmatter delimiter", + ); + expect( + requested.some( + (entry) => entry.method === "POST" && entry.path.endsWith("/skills"), + ), + ).toBe(true); + }); + test("a rejected create surfaces the registry's error inline in the dialog and creates nothing", async () => { // The dialog's own client-side validationIssues() never checks the // description for HTML, so an author typing markup only gets caught diff --git a/packages/skills/src/routes.ts b/packages/skills/src/routes.ts index 211608e7b..4fc483873 100644 --- a/packages/skills/src/routes.ts +++ b/packages/skills/src/routes.ts @@ -14,6 +14,7 @@ import { type SkillRegistry, type SkillRegistryErrorReason, } from "./registry"; +import { parseSkillMd, SkillContentError } from "./skill-md"; /** Which workflow definitions pin a given skill. */ export type PinnedByResolver = { @@ -25,13 +26,25 @@ export type PinnedByResolver = { >; }; -const CreateSkillBody = type({ +/** The paste path: identity and body already split into fields. */ +const CreateSkillFieldsBody = type({ name: "string", description: "string", body: "string", scope: skillAccessScopeSchema, }); +/** 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: skillAccessScopeSchema, +}); + +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 @@ -112,10 +125,27 @@ export function createSkillRoutes({ 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(errorEnvelope("bad_request", cause.message), 400); + } + throw cause; + } + } else { + fields = { + name: body.name, + description: body.description, + body: body.body, + }; + } const skill = await registry.create(caller(c), { - name: body.name, - description: body.description, - body: body.body, + name: fields.name, + description: fields.description, + body: fields.body, scope: body.scope, }); return c.json({ skill }, 201); diff --git a/packages/skills/test/routes.test.ts b/packages/skills/test/routes.test.ts index 59e382f57..3430425a9 100644 --- a/packages/skills/test/routes.test.ts +++ b/packages/skills/test/routes.test.ts @@ -56,6 +56,57 @@ async function createSkill(app: Hono): Promise { 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: { message: string } }; + expect(payload.error.message).toContain("frontmatter delimiter"); + + 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);