From db7cd09090740afd6c5ad25fd7acec1fcbe224c4 Mon Sep 17 00:00:00 2001 From: hotragn Date: Sat, 22 Aug 2026 16:09:22 -0400 Subject: [PATCH 1/2] Pick the tools a skill needs where the skill is written MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit only be made by calling the API, so on a deployment set up through the product every skill declared nothing, every Bot fell to the "nothing was declared" branch, and the narrowing had nothing to narrow on. The release note for it says as much: nothing changes for a deployment that has not declared tools on any skill. Writing or editing a skill now lists the tools of every connected server, grouped by server, with writes marked. `SkillRecord.tools` and the `tools` field on the save endpoint already existed; this is the screen for them, so the change is entirely under app/src. Part of the form, unlike the Agents toggles below it. What a skill needs is the author's draft until they press save, so unticking one and closing the panel changes nothing — where granting saves the instant a button is pressed. The two deliberately do not look alike. The field is always sent, including empty. The server replaces the set when the field is present and leaves it alone when it is absent, so a form that had its last tool unticked has to send `[]` rather than omit the key. Refs are not validated in the browser. The server refuses one it has never seen with a sentence naming it, and it is the only side that knows which tools exist; a list rebuilt here would go stale the moment a server's tools were refreshed. Picking a tool is not granting it, and the hint says so beside the choice. The offer is still intersected with the grants, which is what keeps the surface anybody signed in may write on from being a way around the one that is an administrator's. --- CHANGELOG.md | 13 +++ app/src/components/skills/edit-skill.tsx | 3 + app/src/components/skills/skill-fields.tsx | 9 ++ app/src/components/skills/skill-tools.tsx | 122 +++++++++++++++++++++ app/src/lib/plugins/mutations.ts | 8 ++ app/src/lib/plugins/queries.ts | 9 ++ app/src/lib/skills/form.ts | 12 ++ app/tests/skill-form.test.ts | 60 ++++++++++ 8 files changed, 236 insertions(+) create mode 100644 app/src/components/skills/skill-tools.tsx create mode 100644 app/tests/skill-form.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 4f7713ec..41b0919f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -32,6 +32,19 @@ refusing to boot over. The example package ships four: `/find-a-document`, `/whats-changed`, `/who-owns-this` and `/check-a-claim`. +### The tools a skill needs can be picked where the skill is written + +A package's skills arrive with their tools declared. A skill somebody writes here could not: the +`tools` field existed on the save endpoint and on no screen, so a skill written in the product declared +nothing, and the only way to change that was to call the API by hand. Writing or editing a skill now +lists the tools of every connected server, grouped by server, with the ones that change something +marked. + +Picking a tool here is not granting it. The offer is still intersected with what the Bot was granted, +so a skill naming a tool its Bot does not hold selects the skill and loads nothing — which is why +anybody may write a skill while connecting a server stays an administrator's decision. The screen says +so, next to the choice. + ## 0.0.3 ### A Bot is offered the tools its message needs, not every tool it holds diff --git a/app/src/components/skills/edit-skill.tsx b/app/src/components/skills/edit-skill.tsx index f4b4cd04..6e2cdf44 100644 --- a/app/src/components/skills/edit-skill.tsx +++ b/app/src/components/skills/edit-skill.tsx @@ -62,6 +62,9 @@ export function EditSkill({ slug }: { slug: string }) { title: skill.title, summary: skill.summary ?? "", instructions: skill.instructions, + // Defaulted, so a skill written before this field existed opens with nothing ticked rather + // than with an undefined the form would refuse to submit. + tools: skill.tools ?? [], }} error={saveSkill.error} /* diff --git a/app/src/components/skills/skill-fields.tsx b/app/src/components/skills/skill-fields.tsx index f2c15597..8f6fd8ef 100644 --- a/app/src/components/skills/skill-fields.tsx +++ b/app/src/components/skills/skill-fields.tsx @@ -1,4 +1,5 @@ import { useForm } from "@tanstack/react-form"; +import { SkillTools } from "@/components/skills/skill-tools"; import { Button } from "@/components/ui/button"; import { Field, @@ -188,6 +189,14 @@ export function SkillFields({ ); }} + + {(field) => ( + + )} + {/* diff --git a/app/src/components/skills/skill-tools.tsx b/app/src/components/skills/skill-tools.tsx new file mode 100644 index 00000000..c4992842 --- /dev/null +++ b/app/src/components/skills/skill-tools.tsx @@ -0,0 +1,122 @@ +import { useQuery } from "@tanstack/react-query"; +import { Button } from "@/components/ui/button"; +import { Field, FieldLabel } from "@/components/ui/field"; +import { pluginsPageQueryOptions } from "@/lib/plugins/queries"; + +/** + * Which tools this skill says it needs. + * + * WHY IT MATTERS THAT THIS EXISTS AT ALL. Selection is over skills: before a run the deployment asks + * its own model which skills the message needs, and the Bot is built with those skills' tools plus + * every granted tool no skill claims. A deployment where no skill declares anything has nothing to + * select on, so every Bot is offered its whole catalogue — which is the case the narrowing was built + * to fix. Until this screen existed the only way to declare a tool was to call the API by hand. + * + * A DECLARATION IS NOT A GRANT, and this is the one thing the screen has to get across. Anybody + * signed in may write a skill; adding an MCP server is an administrator's decision. If naming a tool + * here made it callable, the first surface would be a way around the second. It does not: the offer + * is intersected with what the Bot was granted, so a skill naming a tool its Bot does not hold + * selects the skill and loads nothing. Said in the hint rather than left for somebody to discover, + * because the failure it prevents is somebody ticking boxes expecting access. + * + * PART OF THE FORM, unlike the Agents toggles below it. What a skill needs is the author's draft + * until they press save, so unticking one and closing the panel changes nothing. Granting is the + * opposite and saves on press, which is why the two look different on purpose. + */ +export function SkillTools({ + selected, + onChange, +}: { + selected: string[]; + onChange: (refs: string[]) => void; +}) { + const plugins = useQuery(pluginsPageQueryOptions()); + const held = new Set(selected); + + const toggle = (ref: string) => { + /* + * Rebuilt rather than mutated, and filtered rather than spliced, so the array handed to the form + * is a new one. React Form compares by reference; mutating in place leaves the field's value + * looking unchanged and the submit button disabled on a form the person has edited. + */ + onChange( + held.has(ref) + ? selected.filter((each) => each !== ref) + : [...selected, ref], + ); + }; + + /* + * Only servers that actually offer something. A server whose tools have never been refreshed has + * an empty list, and a heading with nothing under it reads as a tool that failed to draw. + */ + const servers = (plugins.data?.servers ?? []).filter( + (server) => server.tools.length > 0, + ); + + return ( + + Tools it needs + + {plugins.isPending ? null : plugins.error ? ( +

+ Could not load the tools this deployment has. +

+ ) : servers.length === 0 ? ( +

+ No connected server offers a tool yet. A skill can still be written — + most are instructions rather than tool use. +

+ ) : ( +
+ {servers.map((server) => ( +
+

{server.title}

+
+ {server.tools.map((tool) => { + const on = held.has(tool.ref); + return ( + + ); + })} +
+
+ ))} +
+ )} + +

+ Picking this skill is what loads these tools for a turn. It does not + grant them — a Bot still only calls what it was granted, so naming a + tool here gives nobody access to it. +

+ + ); +} diff --git a/app/src/lib/plugins/mutations.ts b/app/src/lib/plugins/mutations.ts index e2e2228c..7ead8df7 100644 --- a/app/src/lib/plugins/mutations.ts +++ b/app/src/lib/plugins/mutations.ts @@ -17,6 +17,14 @@ export type SkillInput = { summary?: string; instructions: string; global?: boolean; + /** + * The tools this skill says it needs, as `/` refs. + * + * Sent on every save, including empty, because the server replaces the set rather than merging + * into it: omitting the field to mean "leave them alone" and sending `[]` to mean "clear them" + * would be the same request from a form that just had its last one unticked. + */ + tools?: string[]; }; /** A curated server from the catalogue, which supplies the URL. */ diff --git a/app/src/lib/plugins/queries.ts b/app/src/lib/plugins/queries.ts index 9c7241f5..27accab6 100644 --- a/app/src/lib/plugins/queries.ts +++ b/app/src/lib/plugins/queries.ts @@ -41,6 +41,15 @@ export type PluginSkill = { origin: string; installedBy: string | null; grantedTo: string[]; + /** + * The tools this skill says it needs, as `/` refs. + * + * A declaration, not a grant, and the difference is the whole reason anybody may write a skill: + * naming a tool here cannot make it callable. Selection intersects this with what the Bot was + * granted, so a skill naming a tool its Bot does not hold selects the skill and loads nothing. + * `grantedTo` on the tool side is what decides. + */ + tools: string[]; }; export type CatalogueItem = { diff --git a/app/src/lib/skills/form.ts b/app/src/lib/skills/form.ts index 596243b1..37998e30 100644 --- a/app/src/lib/skills/form.ts +++ b/app/src/lib/skills/form.ts @@ -34,6 +34,17 @@ export const skillFormSchema = z.object({ .string() .trim() .min(1, "Instructions are required — this is what the Bot follows."), + /** + * The tools this skill says it needs, as `/` refs. + * + * No minimum, and no validation of the refs themselves. A skill needing no tool is ordinary — most + * are prose — and the server already refuses a ref it has never seen with a sentence naming it, + * which is a better answer than anything this file could reconstruct about which tools exist. + * + * Part of the form rather than saved on press, unlike granting. What a skill needs is the author's + * draft until they save it, so unticking one and closing the panel has to change nothing. + */ + tools: z.array(z.string()), }); export type SkillFormValues = z.infer; @@ -43,4 +54,5 @@ export const emptySkillForm: SkillFormValues = { title: "", summary: "", instructions: "", + tools: [], }; diff --git a/app/tests/skill-form.test.ts b/app/tests/skill-form.test.ts new file mode 100644 index 00000000..372bfefc --- /dev/null +++ b/app/tests/skill-form.test.ts @@ -0,0 +1,60 @@ +import { describe, expect, test } from "bun:test"; +import { emptySkillForm, skillFormSchema } from "@/lib/skills/form"; + +/** + * The declared tools, as the form carries them. + * + * A skill needing no tool is the ordinary case, and a skill that had its last one unticked has to + * submit as an empty array rather than as an absent field — the server replaces the set when the + * field is present and leaves it alone when it is not, so those two are different requests. + */ + +const valid = { + slug: "standup", + title: "Standup", + summary: "", + instructions: "Summarise yesterday.", +}; + +describe("declaring the tools a skill needs", () => { + test("a new skill starts with none declared", () => { + expect(emptySkillForm.tools).toEqual([]); + // And the empty form is a shape the schema accepts, apart from the fields a person has to fill. + expect( + skillFormSchema.safeParse({ ...emptySkillForm, ...valid }).success, + ).toBeTrue(); + }); + + test("no tools is valid, and stays an array", () => { + const parsed = skillFormSchema.safeParse({ ...valid, tools: [] }); + expect(parsed.success).toBeTrue(); + // Present rather than stripped: this is what clears the set on a skill that used to declare one. + expect(parsed.success && parsed.data.tools).toEqual([]); + }); + + test("refs are carried through as written", () => { + // `/`, the same key a grant is written against. The form does not reformat + // them, because the server compares them to grant refs character for character. + const tools = ["acme-docs/find_document", "acme-chat/search_messages"]; + const parsed = skillFormSchema.safeParse({ ...valid, tools }); + expect(parsed.success && parsed.data.tools).toEqual(tools); + }); + + test("the field is required, so a save always says what the set is now", () => { + // Omitting it would submit a save the server reads as "leave the tools alone", which is not what + // a form with nothing ticked means. + expect(skillFormSchema.safeParse(valid).success).toBeFalse(); + }); + + test("a ref this deployment has never seen is left to the server", () => { + /* + * Not validated here on purpose. The server refuses an unknown ref with a sentence naming it, and + * it is the only side that knows which tools exist — a list reconstructed in the browser would go + * stale the moment a server's tools were refreshed. + */ + expect( + skillFormSchema.safeParse({ ...valid, tools: ["nope/not_a_tool"] }) + .success, + ).toBeTrue(); + }); +}); From f32a9555fb145b6fa703eff1f07fbd94ae1d296b Mon Sep 17 00:00:00 2001 From: David McKay Date: Sat, 22 Aug 2026 13:50:10 -0700 Subject: [PATCH 2/2] Say which of a skill's tools no connected server offers The picker draws the tools of the servers this deployment has connected, and a declaration is not confined to those. A package ships skills naming tools for connectors nobody has added yet, and a person's skill outlives the server it was written against. So a skill declaring two tools drew one, and the second was invisible to the person governing it: the screen stated part of the declaration as though it were the whole of it, and editing kept a tool the author had just been shown they did not have. The leftovers now appear under their own heading with the whole ref, since the server id is the part that says which connector is missing. They are not an error and the wording says so: a ref for a connector that is not here loads nothing, because the offer is intersected with the Bot's grants. Computed only once the server list has loaded, so the declared set does not flash into the group and out again while the query is pending. --- CHANGELOG.md | 5 ++ app/src/components/skills/skill-tools.tsx | 51 ++++++++++++++++++++ app/src/lib/skills/form.ts | 22 +++++++++ app/tests/skill-form.test.ts | 57 ++++++++++++++++++++++- 4 files changed, 134 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 41b0919f..b78285b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,11 @@ so a skill naming a tool its Bot does not hold selects the skill and loads nothi anybody may write a skill while connecting a server stays an administrator's decision. The screen says so, next to the choice. +A tool the skill names that no connected server offers is shown too, under its own heading, rather +than left out. A package ships skills declaring tools for connectors nobody has added yet, and a +skill outlives the server it was written against, so a screen that drew only what matched was +stating part of the declaration as though it were all of it. + ## 0.0.3 ### A Bot is offered the tools its message needs, not every tool it holds diff --git a/app/src/components/skills/skill-tools.tsx b/app/src/components/skills/skill-tools.tsx index c4992842..90633864 100644 --- a/app/src/components/skills/skill-tools.tsx +++ b/app/src/components/skills/skill-tools.tsx @@ -2,6 +2,7 @@ import { useQuery } from "@tanstack/react-query"; import { Button } from "@/components/ui/button"; import { Field, FieldLabel } from "@/components/ui/field"; import { pluginsPageQueryOptions } from "@/lib/plugins/queries"; +import { undeclaredElsewhere } from "@/lib/skills/form"; /** * Which tools this skill says it needs. @@ -54,6 +55,26 @@ export function SkillTools({ (server) => server.tools.length > 0, ); + /* + * Declared, and offered by nothing this deployment has connected. + * + * Shown rather than dropped, because the alternative is a screen that states part of the + * declaration as though it were all of it. A package ships skills declaring tools for connectors + * nobody has added yet, and a person's own skill outlives the server it was written against, so + * this is the ordinary case rather than a corner of one. Left out, a skill needing two tools drew + * one and the second was invisible to the person governing it. + * + * Only computed once the list has actually loaded: while the query is pending every ref looks + * unmatched, and flashing the whole declared set into this group and out again would be worse + * than showing nothing for a moment. + */ + const elsewhere = plugins.data + ? undeclaredElsewhere( + selected, + servers.flatMap((server) => server.tools.map((tool) => tool.ref)), + ) + : []; + return ( Tools it needs @@ -112,6 +133,36 @@ export function SkillTools({ )} + {elsewhere.length > 0 ? ( +
+

Not connected here

+
+ {elsewhere.map((ref) => ( + + ))} +
+

+ This skill names these, and no server connected here offers them — + because the connector has not been added, or was removed. They cost + nothing and load nothing until it exists. Click one to stop naming + it. +

+
+ ) : null} +

Picking this skill is what loads these tools for a turn. It does not grant them — a Bot still only calls what it was granted, so naming a diff --git a/app/src/lib/skills/form.ts b/app/src/lib/skills/form.ts index 37998e30..74baed8d 100644 --- a/app/src/lib/skills/form.ts +++ b/app/src/lib/skills/form.ts @@ -56,3 +56,25 @@ export const emptySkillForm: SkillFormValues = { instructions: "", tools: [], }; + +/** + * The declared refs no connected server offers. + * + * WHY THIS EXISTS. The picker draws the tools of the servers this deployment has connected, and a + * skill's declared set is not confined to those: a package ships skills declaring tools for + * connectors nobody has added yet, and a person's own skill outlives the server it was written + * against. Rendering only what matched meant the screen showed a subset of the declaration and + * presented it as the whole thing — a skill needing two tools drew one, and nothing said the other + * was there. Editing it silently kept a tool the author had just been shown they did not have. + * + * A wrong number on a screen somebody governs with is worse than no number, so the leftovers are + * named rather than dropped. They are not an error: a ref for a connector that does not exist here + * loads nothing, because the offer is intersected with the Bot's grants. + */ +export function undeclaredElsewhere( + selected: readonly string[], + offered: readonly string[], +): string[] { + const known = new Set(offered); + return selected.filter((ref) => !known.has(ref)); +} diff --git a/app/tests/skill-form.test.ts b/app/tests/skill-form.test.ts index 372bfefc..0d35729a 100644 --- a/app/tests/skill-form.test.ts +++ b/app/tests/skill-form.test.ts @@ -1,5 +1,9 @@ import { describe, expect, test } from "bun:test"; -import { emptySkillForm, skillFormSchema } from "@/lib/skills/form"; +import { + emptySkillForm, + skillFormSchema, + undeclaredElsewhere, +} from "@/lib/skills/form"; /** * The declared tools, as the form carries them. @@ -58,3 +62,54 @@ describe("declaring the tools a skill needs", () => { ).toBeTrue(); }); }); + +/** + * Declared refs the picker cannot draw as a tool. + * + * The picker lists the tools of the servers this deployment has connected, and a declaration is not + * confined to those: a package ships skills naming tools for connectors nobody has added yet, and a + * person's own skill outlives the server it was written against. Anything left over has to be shown, + * or the screen states part of the declaration as though it were the whole of it. + */ +describe("declared tools no connected server offers", () => { + const offered = [ + "google-drive/search_files", + "google-drive/read_file_content", + ]; + + test("a ref for a connector nobody has added is surfaced", () => { + expect( + undeclaredElsewhere( + ["google-drive/search_files", "jira/search_issues"], + offered, + ), + ).toEqual(["jira/search_issues"]); + }); + + test("a fully matched declaration leaves nothing over", () => { + expect(undeclaredElsewhere(offered, offered)).toEqual([]); + }); + + test("declaring nothing leaves nothing over", () => { + expect(undeclaredElsewhere([], offered)).toEqual([]); + }); + + test("with no server connected, every declared ref is left over", () => { + // The case a fresh clone is in: the package ships skills declaring Drive tools and Drive has not + // been connected. Every one of them has to be visible, or the skill reads as declaring nothing. + expect( + undeclaredElsewhere( + ["google-drive/search_files", "google-drive/read_file_content"], + [], + ), + ).toEqual(["google-drive/search_files", "google-drive/read_file_content"]); + }); + + test("order is the declaration's, so the list does not reshuffle as servers connect", () => { + expect(undeclaredElsewhere(["z/one", "a/two", "m/three"], [])).toEqual([ + "z/one", + "a/two", + "m/three", + ]); + }); +});