diff --git a/app/src/components/agents/proposed-bot.tsx b/app/src/components/agents/proposed-bot.tsx new file mode 100644 index 000000000..d1a965518 --- /dev/null +++ b/app/src/components/agents/proposed-bot.tsx @@ -0,0 +1,298 @@ +import { Link } from "@tanstack/react-router"; +import { useEffect, useRef, useState } from "react"; +import { Badge, GalleryFrame } from "@/components/gallery/frame"; +import { Button } from "@/components/ui/button"; +import type { AgentFormValues } from "@/lib/agents/form"; +import { + botCardAnswer, + checkProposal, + createdBotIdIn, + type ProposedBot, + wasCreated, +} from "@/lib/agents/proposal"; + +/** + * The coworker a Bot has written, put in front of the person before it exists. + * + * WHY THERE IS A CARD AT ALL. A coworker is not a document: the role description below becomes a + * standing instruction handed to a model on every turn in every channel it is in, and it was written + * by something that was told about the job second-hand. So the same rule the template consent screen + * follows applies here — the whole of that text is shown, unabridged and unclipped, before anything + * is created, because a person cannot agree to instructions they were not shown. + * + * The whole tool is this card. There is no handler behind it: the run suspends here, and nothing is + * written until a button is pressed. + * + * WHAT IT DELIBERATELY CANNOT DO. No address, no connector, no tool, no boundary and no visibility + * change. The coworker arrives private, holding whatever skills were agreed and nothing else, and + * everything it might reach is granted afterwards on its profile by somebody who may. A card that + * could grant capability would make "ask the Bot for it" the fastest route around the screens that + * exist to decide it. + */ + +/** What creating actually did, since creating and granting are two calls and one can fail alone. */ +export type CreatedBot = { + agentId: string; + name: string; + granted: string[]; + failed: string[]; +}; + +export type ProposedBotCardProps = { + /** Partial while the model is still streaming the arguments. */ + args: Partial; + /** + * A coworker already here whose name this one reuses, when there is one. + * + * Drives the wording and nothing else. Names do not have to be unique, so this is not a refusal — + * it is the fact somebody needs in order to notice they are about to end up with two Renewal Desks + * on one roster and no way to tell them apart in a channel list. + */ + clashes?: { title: string; ownership: string }; + /** Answering resumes the Bot. Absent while streaming, and once this card has been answered. */ + respond?: (result: unknown) => Promise; + /** The recorded answer, once there is one. Completed cards show it instead of controls. */ + result?: string; + create: (values: AgentFormValues, skills: string[]) => Promise; +}; + +export function ProposedBotCard({ + args, + clashes, + respond, + result, + create, +}: ProposedBotCardProps) { + const [sending, setSending] = useState<"create" | "decline" | null>(null); + /** + * A refusal from the server, kept on the card rather than answered with. + * + * The run stays suspended, because both things worth doing next need it to be — pressing again + * after whatever the refusal named has been dealt with, or declining. Answering the tool with the + * error would end the turn and leave the person retyping their request. + */ + const [refusal, setRefusal] = useState(null); + + if (result !== undefined) { + /* + * The coworker read back out of the answer rather than out of state. This card is re-rendered + * from the transcript on every reload, when whatever this component remembered is gone; the id + * is the server's, so unlike a skill's slug it is not in the arguments either. See + * `createdBotIdIn`. + */ + const madeId = wasCreated(result) ? createdBotIdIn(result) : null; + return ( + Done} + title={titleFor(args.name, clashes)} + > +

{result}

+ {madeId ? : null} + + ); + } + + if (!respond) { + return ( + +

Writing the coworker…

+ + ); + } + + const checked = checkProposal(args); + if (!checked.ok) { + return ; + } + const { values, skills } = checked; + + const make = async () => { + setSending("create"); + setRefusal(null); + let outcome: CreatedBot; + try { + outcome = await create(values, skills); + } catch (cause) { + // Left on the card with the buttons still live. See `refusal` above. + setSending(null); + setRefusal( + cause instanceof Error + ? cause.message + : "The coworker was not created.", + ); + return; + } + /* + * Pressing again is not offered from here on: the coworker exists, and a second press would make + * a second one. The answer carries its id, which is what the completed card links to. + */ + await answer(respond, botCardAnswer.created(outcome)); + }; + + const decline = async () => { + setSending("decline"); + await answer(respond, botCardAnswer.declined()); + }; + + return ( + Waiting on you} + caption={ + clashes + ? `There is already a ${clashes.title} here, which is ${clashes.ownership}. Names do not have to be unique, but two with the same name are hard to tell apart in a channel list.` + : "Nothing is created until you press the button." + } + title={titleFor(values.name, clashes)} + > +
+
Name
+
{values.name}
+
Job
+
{values.title}
+
Skills
+
+ {skills.length > 0 ? ( + + {skills.map((slug) => `/${slug}`).join(", ")} + + ) : ( + None + )} +
+
+ +

+ Its standing instructions. This text is given to a model on every turn. +

+ {/* + * Scrolled rather than clamped, and the distinction is the whole point of the card. A clamp + * hides the second half of an instruction that will run on somebody's behalf from the one + * person being asked to agree to it; a scroller keeps every character reachable. + */} +

+ {values.roleDescription} +

+ +

+ It arrives private, with no address, no connector and no tool. Anything + it needs to reach is granted on its profile afterwards. +

+ + {refusal ? ( +

+ {refusal} +

+ ) : null} + +
+ + +
+ + ); +} + +function titleFor( + name: string | undefined, + clashes: { title: string } | undefined, +): string { + const called = name?.trim() ? name.trim() : "a new coworker"; + return clashes ? `Create ${called}, again` : `Create ${called}`; +} + +/** + * Where the next step happens. + * + * The profile rather than a channel, because it is the screen that answers both remaining questions + * — what this coworker may reach, and whether anybody else can see it — and it carries the control + * that opens a conversation. + */ +function TalkToIt({ + agentId, + name, +}: { + agentId: string; + name: string | undefined; +}) { + return ( + + Open {name?.trim() || "the coworker"} + + ); +} + +/** + * A proposal that cannot be created as written. + * + * Answered rather than shown as a question, because there is nothing for the person to decide: the + * fields are wrong in a way the model can fix, and the problems name their field so it can. The card + * still appears, so the transcript has no silent gap where a coworker was nearly made. + */ +function Unwritable({ + problems, + respond, +}: { + problems: string[]; + respond: (result: unknown) => Promise; +}) { + /** + * Answered once, guarded by a ref rather than by the dependency list. + * + * `problems` is rebuilt from the arguments on every render and the grant queries behind this card + * poll, so a value-equal array arrives as a new identity every few seconds. Left to the deps, this + * would answer the same tool call again on each of them. + */ + const answered = useRef(false); + useEffect(() => { + if (answered.current) return; + answered.current = true; + void answer(respond, botCardAnswer.unwritable(problems)); + }, [problems, respond]); + + return ( + Not created} + title="Create a coworker" + > +

+ The coworker could not be created as written, so nothing was asked of + you. It is being redrafted. +

+ + ); +} + +/** + * Answering, with a failure to answer swallowed deliberately. + * + * The only way `respond` rejects is a run that has already ended — a reload, a stop, a card answered + * in another tab — and in every one of those the write has already happened or already not happened. + * Throwing here would surface a React error over a decision the person has finished making. + */ +async function answer( + respond: (result: unknown) => Promise, + text: string, +): Promise { + try { + await respond(text); + } catch { + // The run this card belonged to is gone. Nothing further to do. + } +} diff --git a/app/src/lib/agents/answers.ts b/app/src/lib/agents/answers.ts new file mode 100644 index 000000000..175fb1ed3 --- /dev/null +++ b/app/src/lib/agents/answers.ts @@ -0,0 +1,49 @@ +import type { QueryClient } from "@tanstack/react-query"; +import { pluginsPageQueryOptions } from "@/lib/plugins/queries"; +import { describeBot, describeBots, describeGrantableSkills } from "./proposal"; +import { agentListQueryOptions } from "./queries"; + +/** + * What the three reading tools answer with, fetched at the moment they are asked. + * + * THE BUG THIS SHAPE EXISTS TO PREVENT, which the very first conversation with this feature + * produced. The obvious wiring is a `useQuery` in the component and handlers closing over + * `data ?? []`. A run starts the moment somebody sends a message, and the model called + * `list_bot_skills` before that query had come back — so the handler read an empty array and + * answered "No skills exist here yet", which the Bot then told the person as a fact about their + * deployment. Nine skills existed. + * + * An empty list and an unloaded list are different answers and must not share a code path. + * `ensureQueryData` returns what is cached when there is something cached and fetches when there is + * not, so a tool called in the first second of a run waits for the truth rather than inventing a + * tidier one — and a run that never calls these pays for neither. + * + * They live here rather than inline in the hooks so that a test can call them cold, against a query + * client nothing has rendered, which is precisely the state the bug needed. + */ + +export async function answerListBots(client: QueryClient): Promise { + return describeBots(await client.ensureQueryData(agentListQueryOptions())); +} + +export async function answerReadBot( + client: QueryClient, + name: string, +): Promise { + const wanted = name.trim().toLowerCase(); + const known = await client.ensureQueryData(agentListQueryOptions()); + const agent = known.find( + (candidate) => candidate.name.toLowerCase() === wanted, + ); + if (!agent) { + return `There is no coworker called ${name} that this person can see. Call list_bots for the ones there are.`; + } + return describeBot(agent); +} + +export async function answerListBotSkills( + client: QueryClient, +): Promise { + const page = await client.ensureQueryData(pluginsPageQueryOptions()); + return describeGrantableSkills(page.skills); +} diff --git a/app/src/lib/agents/proposal.ts b/app/src/lib/agents/proposal.ts new file mode 100644 index 000000000..cbc4568dd --- /dev/null +++ b/app/src/lib/agents/proposal.ts @@ -0,0 +1,287 @@ +import { z } from "zod"; +import type { PluginSkill } from "@/lib/plugins/queries"; +import { type AgentFormValues, agentFormSchema } from "./form"; +import type { AgentProfile } from "./queries"; + +/** + * A coworker a Bot proposes during a conversation, and the checks it goes through before anybody is + * asked to approve it. + * + * WHY A SEPARATE SHAPE FROM THE FORM'S. `form.ts` describes what a person typed into five fields; a + * model hands over three of them as arguments it may have got wrong in ways a form cannot — a name + * the length of a paragraph, a role description that runs past the limit, `skills` arriving as a + * lone string. So the model-facing schema is permissive about types and the checking is done + * afterwards, against the same `agentFormSchema` the New coworker form uses. A Bot making a coworker + * and a person making one are then held to one rule, with no second parser to drift from the + * server's. + * + * Nothing here decides whether the coworker may be created. That stays the server's: `POST + * /api/agents` already answers who may, records `bot.created`, and refuses in its own words. + */ + +/** + * What the card answers its tool call with, and the facts it reads back out. + * + * WHY THESE ARE HERE AND NOT ON THE CARD. A completed card has only the result string to go on — the + * SDK hands back what the tool answered, not what happened — so whether to offer the link to the new + * coworker is decided by reading the sentence. Written on the card, the sentence and its reader + * would be two literals a reword could silently separate, and the failure is a link that quietly + * stops appearing. + * + * The sentences are addressed to the model, because that is who receives them, and each says what to + * do next: a tool result that only reports state leaves the Bot guessing whether the turn is over. + */ +const CREATED_MARKER = "Created."; + +export const botCardAnswer = { + created: (input: { + agentId: string; + name: string; + granted: readonly string[]; + failed: readonly string[]; + }) => { + const parts = [ + `${CREATED_MARKER} ${input.name} now exists and is private.`, + ]; + if (input.granted.length > 0) { + parts.push(`It holds ${input.granted.map((s) => `/${s}`).join(", ")}.`); + } + /* + * Said out loud rather than rounded up. Creating and granting are two calls, so a grant that + * fails after the coworker exists leaves it with fewer skills than the person agreed to — and a + * Bot that reported the whole set would send somebody away believing in a skill that is not + * there. + */ + if (input.failed.length > 0) { + parts.push( + `${input.failed.map((s) => `/${s}`).join(", ")} could not be put on it; tell the person to add ${input.failed.length === 1 ? "it" : "them"} from the coworker's profile.`, + ); + } + parts.push( + "It was granted no connector, no tool and no address. Tell the person that anything it needs to reach is granted on its profile, that it can be talked to now, and that its profile is at", + `/agents?agent=${input.agentId}.`, + ); + return parts.join(" "); + }, + declined: () => + "The person did not create this coworker. Ask what to change rather than proposing the same one again.", + /** A proposal the fields refuse, answered rather than shown as a question. See the card. */ + unwritable: (problems: readonly string[]) => + `Not created, and the person was not asked, because the coworker is not valid: ${problems.join(" ")} Fix those and propose it again.`, +}; + +/** Whether a completed card's recorded answer is one that made a coworker. */ +export function wasCreated(result: string): boolean { + return result.startsWith(CREATED_MARKER); +} + +/** + * The coworker a completed card made, read back out of its own answer. + * + * WHY IT IS PARSED RATHER THAN REMEMBERED. A card is re-rendered from the transcript on every + * reload, with the arguments and the recorded answer and nothing else — component state does not + * survive it. The id is minted by the server, so unlike a skill's slug it is not in the arguments, + * and a card that kept it in `useState` would show the link once and then quietly stop showing it to + * anybody who refreshed. Putting it in the sentence is also the honest thing for the model, which is + * being told where the coworker it just made can be found. + * + * Paired with {@link botCardAnswer.created} here, in one module, and pinned by a test, because a + * reworded sentence and a reader living apart is exactly how a link stops appearing with nothing to + * say why. + */ +export function createdBotIdIn(result: string): string | null { + return /agent=([A-Za-z0-9_-]+)/.exec(result)?.[1] ?? null; +} + +/** + * The skill whose grant turns a Bot into one you can make coworkers with. + * + * The app offers `save_bot` and the reads beside it only while the active Bot holds this slug, and + * the shipped package is what puts the slug in the deployment and on a Bot. So the two have to + * agree: rename it in one place and making a coworker in a conversation stops working, with nothing + * on screen to say why and a deployment that still looks healthy. `bot-creator-slug.test.ts` is the + * guard, which is why this lives in a module with no React in it rather than beside the hooks. + * + * Not an MCP ref, and deliberately not declared in `skills.yaml`'s `tools:`. A declaration there is + * intersected with the Bot's MCP grants, so naming the app's own tools would name nothing. + */ +export const BOT_CREATOR_SLUG = "bot-creator"; + +/** + * The arguments the model is offered, described for the model rather than for us. + * + * Every `describe` is read by the thing filling the field in, so they say what good looks like + * instead of restating the type. The role description gets the longest one because it is the field + * that decides what the coworker actually is: on a deployment with no Bot of its own it becomes the + * standing instruction the coworker runs on, which is a different job from writing a summary of it. + */ +export const proposedBotSchema = z.object({ + name: z + .string() + .describe( + "What the coworker is called, as a person would say it: `Renewal Desk`, not `renewal_desk_bot`. Up to 80 characters.", + ), + title: z + .string() + .describe( + "The job it does, the way a job title reads: `Accounts Receivable`, `Support Operations`. Up to 120 characters. This is given to the model on every turn, so it is part of what the coworker is rather than a label.", + ), + roleDescription: z + .string() + .describe( + "The coworker's standing instructions, written as directions to it in the imperative, up to 1000 characters. Say what it does, what it must not conclude, and what it says when the evidence is thin. This text is given to a model on every turn in every channel, so write the rules that should always hold rather than a description of the coworker in the third person. Do not address the person talking to it.", + ), + skills: z + .array(z.string()) + .optional() + .describe( + "Slugs of skills that already exist here to put on this coworker, from list_bot_skills, without the slash. This is the only capability the card grants, and it grants nothing else: no connector, no tool and no address. Omit for a coworker that is only its instructions, which is most of them.", + ), +}); + +export type ProposedBot = z.infer; + +/** What checking a proposal answers with: the values to create, or the problems to fix. */ +export type CheckedBot = + | { ok: true; values: AgentFormValues; skills: string[] } + | { ok: false; problems: string[] }; + +/** + * Hold a proposal to the same rule the New coworker form is held to. + * + * Checked here rather than left to the server because the alternative is a person being shown a card + * for a coworker that cannot be created, pressing the button, and reading a validation message as + * though they had done something wrong. A problem the model can fix should reach the model. + * + * The three fields the model does not supply are fixed rather than offered. `visibility` is private + * because a coworker somebody has not read yet has no business on everybody's roster, and making it + * public is one control on its profile. `endpoint` and `authValue` are empty because the card cannot + * point a coworker at a host: with no address the server binds it in-process on the role description + * above, which is the whole reason this interview can end without asking somebody for a URL. + */ +export function checkProposal(args: { + name?: unknown; + title?: unknown; + roleDescription?: unknown; + skills?: unknown; +}): CheckedBot { + const parsed = agentFormSchema.safeParse({ + name: typeof args.name === "string" ? args.name : "", + title: typeof args.title === "string" ? args.title : "", + roleDescription: + typeof args.roleDescription === "string" ? args.roleDescription : "", + visibility: "private", + endpoint: "", + authValue: "", + }); + if (parsed.success) { + return { ok: true, skills: skillSlugsIn(args.skills), values: parsed.data }; + } + return { + ok: false, + problems: parsed.error.issues.map((issue) => { + const field = issue.path.join("."); + return field ? `${field}: ${issue.message}` : issue.message; + }), + }; +} + +/** + * Skill slugs out of whatever the model sent, keeping the strings and a lone string on its own. + * + * Anything else is dropped rather than refused: a model that answers `skills: "find-a-document"` + * meant the one skill, and a refusal here would send it back to redo a whole proposal over a comma. + * A coworker holding no skills is always valid. + */ +export function skillSlugsIn(value: unknown): string[] { + const list = typeof value === "string" ? [value] : value; + if (!Array.isArray(list)) return []; + return list + .filter((slug): slug is string => typeof slug === "string") + .map((slug) => slug.trim().replace(/^\//, "")) + .filter((slug) => slug.length > 0); +} + +/** + * Whose a coworker is, in the words the answer uses. + * + * Ownership rather than permission, and the distinction is the point: who may change a coworker is + * the server's to decide, and what this says is the fact a model needs in order to describe the + * roster accurately. `systemOwned` is checked first because a Bot that ships in the box is the + * deployment's however it looks from here. + */ +export function ownershipOf( + agent: Pick, +): "yours" | "the deployment's" | "somebody else's" { + if (agent.systemOwned) return "the deployment's"; + return agent.mine ? "yours" : "somebody else's"; +} + +/** + * The coworkers that already exist here, as the answer to `list_bots`. + * + * One line each, because the reason to ask is to find out whether the coworker being described is + * already here — a rebuilt duplicate of a Bot somebody else maintains is the most expensive mistake + * this interview can make, and the cheapest to avoid. The role descriptions are deliberately left + * out: a dozen of them is most of a run, and a Bot comparing against one asks for it by name. + */ +export function describeBots(agents: readonly AgentProfile[]): string { + if (agents.length === 0) { + return "No coworkers exist here yet. Anything you propose is the first."; + } + const lines = agents.map((agent) => { + const where = agent.endpoint ? "runs at its own address" : "runs here"; + return `- ${agent.name} — ${agent.title} (${ownershipOf(agent)}, ${where})`; + }); + return [ + `${agents.length} coworker${agents.length === 1 ? "" : "s"} already exist here. If one of them already does the job being described, say so and offer that one instead of making a second.`, + ...lines, + ].join("\n"); +} + +/** + * One coworker in full, including the instructions it runs on, for `read_bot`. + * + * Separate from {@link describeBots} so the expensive field is fetched deliberately, one coworker at + * a time, by a Bot that has been asked to make something like it. Never paraphrase a role + * description that has not been read: it is the only place a coworker's actual rules are written. + */ +export function describeBot(agent: AgentProfile): string { + return [ + `${agent.name} — ${agent.title} (${ownershipOf(agent)})`, + agent.endpoint + ? "Runs at its own address, which a coworker made here cannot be given." + : "Runs on this deployment, on the instructions below.", + "Instructions:", + agent.roleDescription, + ].join("\n"); +} + +/** + * The skills a new coworker could be given, as the answer to `list_bot_skills`. + * + * Every skill the person can see rather than the skills the ASKING Bot holds, and that is deliberate + * rather than loose: what this Bot was granted says nothing about what the coworker being made + * should hold. The person's own skills and the deployment's are both offered, because granting one + * to a coworker is the same act either way. + * + * The instructions are left out for the same reason `list_bots` leaves out role descriptions, and + * because nothing here is being edited: this list exists to pick from. + */ +export function describeGrantableSkills( + skills: readonly PluginSkill[], +): string { + if (skills.length === 0) { + return "No skills exist here yet, so a coworker made now runs on its instructions alone. That is ordinary — most coworkers do."; + } + const lines = skills.map((skill) => { + const parts = [`/${skill.slug} — ${skill.title}`]; + if (skill.summary) parts.push(skill.summary); + if (skill.tools.length > 0) parts.push(`needs ${skill.tools.join(", ")}`); + return `- ${parts.join(" · ")}`; + }); + return [ + "Skills that can be put on a new coworker. A skill is an instruction, not a capability: one naming a tool the coworker was not granted loads nothing, so putting it on is safe and granting the connector is a separate step somebody takes on the profile.", + ...lines, + ].join("\n"); +} diff --git a/app/src/lib/copilot/bot-tools.tsx b/app/src/lib/copilot/bot-tools.tsx new file mode 100644 index 000000000..e54087868 --- /dev/null +++ b/app/src/lib/copilot/bot-tools.tsx @@ -0,0 +1,198 @@ +import { useFrontendTool, useHumanInTheLoop } from "@copilotkit/react-core/v2"; +import { useMutation, useQuery } from "@tanstack/react-query"; +import { useMemo } from "react"; +import { z } from "zod"; +import { + type CreatedBot, + ProposedBotCard, +} from "@/components/agents/proposed-bot"; +import { + answerListBotSkills, + answerListBots, + answerReadBot, +} from "@/lib/agents/answers"; +import type { AgentFormValues } from "@/lib/agents/form"; +import { agentInputFrom } from "@/lib/agents/form"; +import { createAgentMutationOptions } from "@/lib/agents/mutations"; +import { + BOT_CREATOR_SLUG, + ownershipOf, + type ProposedBot, + proposedBotSchema, +} from "@/lib/agents/proposal"; +import { agentListQueryOptions } from "@/lib/agents/queries"; +import { useDeclaredBotId } from "@/lib/copilot/active-bot"; +import { grantPlugin, invalidatePlugins } from "@/lib/plugins/mutations"; +import { agentPluginsQueryOptions } from "@/lib/plugins/queries"; +import { queryClient } from "@/query-client"; + +/** + * Making a coworker from inside a conversation. + * + * The deployment ships a skill called `bot-creator` whose instruction is how to interview somebody + * about the coworker they want. These four tools are what let that interview end in a coworker + * rather than in a wall of text somebody has to retype into a form: three reads so the Bot can see + * what already exists and what it may put on the new one, and one write that suspends the run on a + * card. + * + * WHY THE BROWSER AND NOT THE SERVER. A coworker is created as a person, not as a Bot: it goes on + * their roster under their name, and whether they may make one is a question about them. `POST + * /api/agents` already answers exactly that, and writes the `bot.created` audit row with the actor + * on it, so a tool riding the signed-in session inherits every bit of that unchanged. A server-side + * tool would have to carry an actor into runs that do not always have one — a routine, a Slack + * thread, a schedule — and the first way that goes wrong is a coworker created under the wrong name, + * on a roster nobody chose. Nothing is lost by it living here: making a coworker is an interview, so + * there is nobody to interview where there is no browser. + * + * WHY THEY ARE GATED ON THE GRANT. Four more tools on every run is not free — a model picks the + * right tool reliably out of about ten, which is the whole reason per-run narrowing exists — and a + * Bot for looking up transactions has no business making coworkers. So the gate is the ordinary one: + * the Bot has to hold the `bot-creator` skill, granted the same way everything else is. + */ + +export function BotTools() { + // Grants are only fetched once a surface has declared its Bot; the placeholder is not one the + // server knows, and asking would 404 on every poll. + const declared = useDeclaredBotId(); + // Empty rather than undefined: the factory's own `enabled: agentId.length > 0` is the guard for + // "no Bot declared yet", so the placeholder never reaches the server. + const { data: granted } = useQuery(agentPluginsQueryOptions(declared ?? "")); + const authoring = (granted?.skills ?? []).some( + (skill) => skill.slug === BOT_CREATOR_SLUG, + ); + + /* + * The roster, kept in a query because the card reads it on every render to notice a name that is + * already taken. What the TOOLS answer with is fetched at the moment they are called — see below. + */ + const { data: agents } = useQuery({ + ...agentListQueryOptions(), + enabled: authoring, + }); + const createAgent = useMutation(createAgentMutationOptions(queryClient)); + + const roster = useMemo(() => agents ?? [], [agents]); + + /* + * Every answer is fetched at the moment it is asked for rather than read off a render. See + * `lib/agents/answers.ts`, which is where the reason is written down and where a test can reach + * it. + */ + useFrontendTool({ + name: "list_bots", + description: + "The coworkers that already exist here, one line each, with whose each one is and where it runs. Read this before proposing a coworker: the most expensive mistake this job can make is rebuilding one somebody already maintains.", + parameters: z.object({}), + available: authoring, + handler: async () => answerListBots(queryClient), + }); + + useFrontendTool({ + name: "read_bot", + description: + "One existing coworker in full, including the standing instructions it runs on. Use this when somebody asks for one like an existing coworker — never paraphrase a role description you have not read.", + parameters: z.object({ + name: z.string().describe("The coworker's name, as list_bots spells it."), + }), + available: authoring, + handler: async ({ name }) => answerReadBot(queryClient, String(name)), + }); + + useFrontendTool({ + name: "list_bot_skills", + description: + "The skills that could be put on a new coworker, by slug. Putting a skill on a coworker is the only capability the card grants: a skill is an instruction, so one naming a tool the coworker was not granted simply loads nothing.", + parameters: z.object({}), + available: authoring, + handler: async () => answerListBotSkills(queryClient), + }); + + /** + * Creating, as a card rather than a handler. + * + * Stabilised with `useMemo` for the reason the gallery's decisions are: the render component is + * identity-compared, and a fresh function every render remounts the card, which would reset the + * pressed-Creating state, the refusal the server has just given it, and the id of a coworker that + * by then already exists. + */ + const Card = useMemo( + () => + function ProposedBotRender(props: { + args: Partial; + respond?: (result: unknown) => Promise; + result?: string; + }) { + const proposed = props.args.name?.trim().toLowerCase(); + const existing = proposed + ? roster.find( + (candidate) => candidate.name.toLowerCase() === proposed, + ) + : undefined; + return ( + { + /* + * TWO CALLS, AND THE SECOND ONE IS PER SKILL. `POST /api/agents` makes the coworker + * and `POST /api/plugins/grants` puts a skill on it, which is how the profile screen + * does both — so ownership, refusal and the audit rows are the existing ones rather + * than a third path that has to be kept in step with them. + * + * The cost is that the pair is not atomic: a grant that fails after the coworker + * exists leaves it with fewer skills than were agreed. Rather than roll the coworker + * back — which would delete something the person just watched being made, over a + * skill they can add in two clicks — each failure is collected and reported, and the + * card says which ones did not land. + */ + const agent = await createAgent.mutateAsync( + agentInputFrom(values), + ); + const outcome: CreatedBot = { + agentId: agent.id, + failed: [], + granted: [], + name: agent.name, + }; + for (const slug of chosen) { + try { + await grantPlugin({ + agentId: agent.id, + kind: "skill", + ref: slug, + }); + outcome.granted.push(slug); + } catch { + outcome.failed.push(slug); + } + } + // Once at the end rather than between every pair, matching `grantPlugin`'s own note. + if (outcome.granted.length > 0) invalidatePlugins(queryClient); + return outcome; + }} + respond={props.respond} + result={props.result} + /> + ); + }, + [createAgent, roster], + ); + + useHumanInTheLoop({ + name: "save_bot", + description: + "Put a finished coworker in front of the person to create. They see its name, its job, the whole of its standing instructions and the skills it will hold, and nothing is written unless they press the button. Call this once, at the end, after they have agreed to what the coworker will be told to do — not to check your draft.", + parameters: proposedBotSchema, + available: authoring, + render: Card, + }); + + return null; +} diff --git a/app/src/lib/copilot/provider.tsx b/app/src/lib/copilot/provider.tsx index 46001b609..ea7b52f76 100644 --- a/app/src/lib/copilot/provider.tsx +++ b/app/src/lib/copilot/provider.tsx @@ -3,6 +3,7 @@ import { useQuery } from "@tanstack/react-query"; import type { ReactNode } from "react"; import { deploymentCapabilitiesQueryOptions } from "@/lib/deployment/queries"; import { ActiveBotProvider } from "./active-bot"; +import { BotTools } from "./bot-tools"; import { ComputerTools } from "./computer-tools"; import { EscalationTool } from "./escalation-tool"; import { GalleryTools } from "./gallery-tools"; @@ -62,6 +63,11 @@ export function CopilotProvider({ children }: { children: ReactNode }) { {/* Browser-authored components use the same component grants as the compiled gallery. */} + {/* + Making a coworker from a conversation. Registers nothing unless the declared Bot holds the + `bot-creator` skill, so most runs are not offered these four tools at all. + */} + {children} diff --git a/app/tests/bot-creator-slug.test.ts b/app/tests/bot-creator-slug.test.ts new file mode 100644 index 000000000..eb70c5539 --- /dev/null +++ b/app/tests/bot-creator-slug.test.ts @@ -0,0 +1,61 @@ +import { expect, test } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import { parse } from "yaml"; +import { BOT_CREATOR_SLUG } from "@/lib/agents/proposal"; + +/** + * The coupling that breaks silently, in both directions. + * + * `BOT_CREATOR_SLUG` decides which Bots the app offers `list_bots`, `read_bot`, `list_bot_skills` + * and `save_bot` to. The shipped package decides which slug exists in a deployment and which Bot + * holds it. Nothing connects the two but the string, and every way of breaking it looks like a + * healthy deployment: rename it in the YAML and the tools are never offered; grant it to nobody and + * there is no Bot to make coworkers with. In both cases a person asks their Bot to make a coworker, + * it says it cannot, and there is nothing on any screen that explains why. + * + * So this reads the package the way the loader does and asserts the three facts that have to hold + * together. It is the only thing that makes that failure loud, which is why it must not be deleted + * along with whichever half of the coupling a later change moves. + */ + +const PACKAGE = join(import.meta.dir, "../../examples/fintech"); + +type SkillsFile = { skills: { slug: string; tools?: unknown }[] }; +type AgentsFile = { agents: { id: string; skills?: string[] }[] }; + +function read(file: string): T { + return parse(readFileSync(join(PACKAGE, file), "utf8")) as T; +} + +test("the deployment ships a skill under the slug the app gates on", () => { + const { skills } = read("skills.yaml"); + const shipped = skills.find((skill) => skill.slug === BOT_CREATOR_SLUG); + + expect(shipped).toBeDefined(); +}); + +test("a Bot in the package actually holds it", () => { + const { agents } = read("agents.yaml"); + const holders = agents.filter((agent) => + (agent.skills ?? []).includes(BOT_CREATOR_SLUG), + ); + + // At least one, because a slug granted to nobody offers the tools to nobody. + expect(holders.length).toBeGreaterThan(0); +}); + +/** + * The declaration stays empty, and that is a rule rather than an accident. + * + * `tools:` on a skill is intersected with its Bot's MCP grants to narrow what a run is offered. The + * four tools this skill exists for are the app's own, registered in the browser and gated on the + * grant of the skill itself — they are not MCP refs and can never appear in a grant. Naming them + * here would therefore name nothing, while looking exactly like the thing that makes them work. + */ +test("it declares no tools, because the ones it is for are not MCP refs", () => { + const { skills } = read("skills.yaml"); + const shipped = skills.find((skill) => skill.slug === BOT_CREATOR_SLUG); + + expect(shipped?.tools ?? []).toEqual([]); +}); diff --git a/app/tests/bot-tool-answers.test.ts b/app/tests/bot-tool-answers.test.ts new file mode 100644 index 000000000..df54ac13b --- /dev/null +++ b/app/tests/bot-tool-answers.test.ts @@ -0,0 +1,175 @@ +import { afterAll, expect, test } from "bun:test"; +import { QueryClient } from "@tanstack/react-query"; +import { + answerListBotSkills, + answerListBots, + answerReadBot, +} from "@/lib/agents/answers"; + +/** + * What the three reading tools say, asked cold. + * + * COLD IS THE WHOLE POINT. The regression these guard against is not a wrong sentence, it is a right + * sentence about the wrong state: handlers that closed over a `useQuery` result answered + * "No skills exist here yet" while nine existed, because the model called the tool in the first + * second of a run and the query had not come back. The Bot then told the person that as a fact about + * their deployment. + * + * So every test here uses a query client nothing has rendered against and nothing has warmed, which + * is exactly the state that produced the bug. A handler that reads a snapshot instead of fetching + * fails these; one that fetches passes. + * + * The transport is stubbed at `fetch` and restored afterwards. No module mocks: `mock.module` in bun + * is process-wide and does not come back, so a file that mocked `@/lib/client` here would silently + * change what every other test file in the suite imports. + */ + +const AGENTS = [ + { + id: "general-assistant", + name: "General Assistant", + title: "Everyday Work", + roleDescription: "Help with everyday work.", + avatarSeed: "general-assistant", + visibility: "public", + endpoint: null, + hasAuth: false, + hasCallbackToken: false, + hidden: false, + systemOwned: true, + canManage: true, + mine: false, + }, + { + id: "agent_1", + name: "Renewal Desk", + title: "Accounts Receivable", + roleDescription: "Chase overdue invoices. Never invent a date.", + avatarSeed: "renewal-desk", + visibility: "private", + endpoint: "https://renewals.example.com/ag-ui", + hasAuth: true, + hasCallbackToken: false, + hidden: false, + systemOwned: false, + canManage: true, + mine: true, + }, +]; + +const SKILLS = [ + { + id: "skill_1", + slug: "find-a-document", + ownerUserId: null, + title: "Find a document", + summary: "Search the connected sources and read what it says.", + instructions: "Search first, then read the file you found.", + origin: "package", + installedBy: null, + grantedTo: [], + tools: ["google-drive/search_files"], + }, +]; + +const realFetch = globalThis.fetch; +afterAll(() => { + globalThis.fetch = realFetch; +}); + +/** How many times the transport was reached, so "it fetched" is asserted rather than assumed. */ +let calls: string[] = []; + +globalThis.fetch = (async (input: RequestInfo | URL) => { + const path = typeof input === "string" ? input : input.toString(); + calls.push(path); + if (path.startsWith("/api/agents")) { + return Response.json({ agents: AGENTS }); + } + if (path === "/api/plugins") { + return Response.json({ + catalogue: [], + botsMayCallBack: false, + servers: [], + skills: SKILLS, + redirectUri: "", + }); + } + return Response.json({ error: "not found" }, { status: 404 }); +}) as typeof fetch; + +/** A client nothing has rendered against, which is the state a run's first tool call happens in. */ +function coldClient(): QueryClient { + calls = []; + return new QueryClient({ defaultOptions: { queries: { retry: false } } }); +} + +test("the roster is fetched when asked, not read off a render", async () => { + const answer = await answerListBots(coldClient()); + + expect(calls.some((path) => path.startsWith("/api/agents"))).toBe(true); + expect(answer).toContain("2 coworkers already exist here"); + expect(answer).toContain("General Assistant — Everyday Work"); + // Whose it is and where it runs, which is what stops a duplicate being proposed. + expect(answer).toContain("the deployment's"); + expect(answer).toContain("runs at its own address"); + expect(answer).toContain("Renewal Desk"); + // Role descriptions are deliberately not in the list: a dozen of them is most of a run. + expect(answer).not.toContain("Never invent a date"); +}); + +test("the skills are fetched when asked, so an unloaded list never reads as an empty one", async () => { + const answer = await answerListBotSkills(coldClient()); + + expect(calls).toContain("/api/plugins"); + expect(answer).toContain("/find-a-document — Find a document"); + // The sentence that was wrongly given to a person about a deployment holding nine skills. + expect(answer).not.toContain("No skills exist here yet"); + // The line that stops the interview implying a skill brings capability with it. + expect(answer).toContain("A skill is an instruction, not a capability"); +}); + +test("one coworker comes back in full, including what it runs on", async () => { + const answer = await answerReadBot(coldClient(), "renewal desk"); + + expect(answer).toContain("Renewal Desk — Accounts Receivable (yours)"); + expect(answer).toContain("Chase overdue invoices. Never invent a date."); + expect(answer).toContain("Runs at its own address"); +}); + +test("a coworker that is not here is named rather than guessed at", async () => { + const answer = await answerReadBot(coldClient(), "Nobody"); + + expect(answer).toContain("There is no coworker called Nobody"); + expect(answer).toContain("Call list_bots"); +}); + +/** + * An empty deployment says it is empty, which is only safe because the answer above is fetched. + * + * The two sentences are opposites and the tool has to be able to give both; what must never happen + * is the empty one being given about a deployment that simply had not answered yet. + */ +test("a genuinely empty deployment is described as empty", async () => { + const client = coldClient(); + const previous = globalThis.fetch; + globalThis.fetch = (async (input: RequestInfo | URL) => { + const path = typeof input === "string" ? input : input.toString(); + calls.push(path); + if (path.startsWith("/api/agents")) return Response.json({ agents: [] }); + return Response.json({ + catalogue: [], + botsMayCallBack: false, + servers: [], + skills: [], + redirectUri: "", + }); + }) as typeof fetch; + + expect(await answerListBots(client)).toContain("No coworkers exist here yet"); + expect(await answerListBotSkills(client)).toContain( + "No skills exist here yet", + ); + + globalThis.fetch = previous; +}); diff --git a/app/tests/proposed-bot-card.test.tsx b/app/tests/proposed-bot-card.test.tsx new file mode 100644 index 000000000..c34a6295f --- /dev/null +++ b/app/tests/proposed-bot-card.test.tsx @@ -0,0 +1,313 @@ +import { afterAll, afterEach, beforeAll, expect, test } from "bun:test"; +import { GlobalRegistrator } from "@happy-dom/global-registrator"; +import { + createMemoryHistory, + createRootRoute, + createRoute, + createRouter, + Outlet, + RouterProvider, +} from "@tanstack/react-router"; +import { cleanup, render, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import type { ReactNode } from "react"; +import { ProposedBotCard } from "@/components/agents/proposed-bot"; +import { + botCardAnswer, + createdBotIdIn, + wasCreated, +} from "@/lib/agents/proposal"; + +/** + * The card a Bot's proposed coworker stops on, and every answer it can give. + * + * The card IS the tool: `save_bot` has no handler, so the run suspends here and every way out is a + * render away from a `respond` call. That makes the states worth pinning one at a time — a card that + * answers twice resumes a turn that has already moved on, and one that answers a server refusal ends + * the turn at the moment the person could have fixed it and pressed again. + * + * The property this file exists for above all the others is that the WHOLE role description is on + * screen before anything is created. It becomes a standing instruction handed to a model on every + * turn in every channel, written by something that heard about the job second-hand, and a clamp on + * it would hide the second half of what somebody is agreeing to. CSS does not run in this DOM, so + * `line-clamp` and `truncate` are invisible to `textContent` — the assertion therefore reads the + * class attribute, which is ugly and is the only way to catch the thing that would actually go + * wrong. + * + * THE HARNESS IS THIS REPOSITORY'S, and matching it is load-bearing rather than tidiness. Every DOM + * test here registers Happy DOM in `beforeAll` and unregisters it in `afterAll`, and bun walks every + * file into one process — so a file that instead registered once at module scope and queried the + * global `screen` has its document torn out from under it by whichever neighbour finishes first. + * That failure is invisible in isolation: the file passes alone and every case in it fails in a full + * run, which is exactly what this one did. Queries come off `render()`'s own return for the same + * reason. + * + * NO MODULE MOCKS, and nothing stubbed at `fetch`: creating is a prop, so the card is exercised with + * a plain function and the transport never enters this file. + */ + +beforeAll(() => GlobalRegistrator.register()); +afterEach(cleanup); +afterAll(() => GlobalRegistrator.unregister()); + +/** The instruction under test, long enough that a clamp would visibly cost somebody the end of it. */ +const INSTRUCTIONS = + "Chase overdue invoices and draft the follow-up a person sends. Work only from the ledger you were given, quote the line each amount came from, and never invent a date the record does not carry. Where an invoice is disputed rather than late, say so and stop."; + +/** A proposal that passes the fields, so each test varies only the thing it is about. */ +const DRAFT = { + name: "Renewal Desk", + title: "Accounts Receivable", + roleDescription: INSTRUCTIONS, + skills: ["find-a-document", "check-a-claim"], +}; + +const MADE = { + agentId: "agent_1", + name: "Renewal Desk", + granted: ["find-a-document", "check-a-claim"], + failed: [], +}; + +/** A real router over a memory history, so the completed card's `Link` resolves without a mock. */ +function routed(node: ReactNode) { + const rootRoute = createRootRoute({ component: Outlet }); + const routeTree = rootRoute.addChildren([ + createRoute({ + getParentRoute: () => rootRoute, + path: "/", + component: () => node, + }), + createRoute({ + getParentRoute: () => rootRoute, + path: "/agents", + component: () => null, + }), + ]); + const router = createRouter({ + routeTree, + history: createMemoryHistory({ initialEntries: ["/"] }), + }); + // The app's router is registered globally for typing; this one only has to resolve /agents. + return render(); +} + +test("a proposal still streaming shows nothing to press", async () => { + /* + * `respond` is absent until the arguments are complete. A button drawn before then would create a + * coworker out of half an instruction — and unlike a skill, a coworker cannot be quietly replaced + * by proposing it again. + */ + const view = routed( + MADE} />, + ); + + // Awaited rather than read synchronously: the router mounts its route on a later tick, so nothing + // at all is in the document on the first one. + expect(await view.findByText("Writing the coworker…")).toBeTruthy(); + expect(view.queryByRole("button")).toBeNull(); +}); + +test("the whole instruction is on screen, unclipped, before anything is created", async () => { + let created = 0; + const view = routed( + { + created += 1; + return MADE; + }} + respond={async () => {}} + />, + ); + + const instruction = await view.findByText(INSTRUCTIONS); + // The regression this guards: a clamp or a truncate on the one field somebody is agreeing to. + expect(instruction.className).not.toContain("line-clamp"); + expect(instruction.className).not.toContain("truncate"); + expect(instruction.className).toContain("overflow-y-auto"); + + expect(view.getByText("Renewal Desk")).toBeTruthy(); + expect(view.getByText("Accounts Receivable")).toBeTruthy(); + expect(view.getByText("/find-a-document, /check-a-claim")).toBeTruthy(); + // Rendering the card writes nothing. The press is the write. + expect(created).toBe(0); +}); + +test("pressing creates once and answers with what was made", async () => { + const answers: string[] = []; + let created = 0; + const view = routed( + { + created += 1; + return MADE; + }} + respond={async (result) => { + answers.push(String(result)); + }} + />, + ); + + await userEvent.click(await view.findByText("Create it")); + + await waitFor(() => expect(answers.length).toBe(1)); + expect(created).toBe(1); + expect(answers[0]).toBe(botCardAnswer.created(MADE)); + expect(answers[0]).toContain("Renewal Desk now exists and is private"); + // The line that stops somebody believing the coworker arrived able to reach things. + expect(answers[0]).toContain("granted no connector, no tool and no address"); +}); + +/** + * The half-done outcome, said out loud. + * + * Creating the coworker and putting a skill on it are two calls, so a grant can fail after the + * coworker exists. Rolling back would delete something the person just watched being made over a + * skill they can add in two clicks, so the card keeps the coworker and reports the shortfall — and + * the sentence has to name the skills that did not land, or somebody walks away believing in one. + */ +test("a skill that did not land is named rather than rounded up", async () => { + const answers: string[] = []; + const partial = { + agentId: "agent_2", + name: "Renewal Desk", + granted: ["find-a-document"], + failed: ["check-a-claim"], + }; + const view = routed( + partial} + respond={async (result) => { + answers.push(String(result)); + }} + />, + ); + + await userEvent.click(await view.findByText("Create it")); + + await waitFor(() => expect(answers.length).toBe(1)); + expect(answers[0]).toContain("/check-a-claim could not be put on it"); + expect(answers[0]).toContain("/find-a-document"); +}); + +/** + * A refusal from the server stays on the card, and the run stays suspended. + * + * Answering the tool with the error would end the turn at the exact moment the person could have + * fixed what the refusal named and pressed again, leaving them to retype the whole request. + */ +test("a refusal keeps the run open with the buttons still live", async () => { + const answers: string[] = []; + const view = routed( + { + throw new Error("A coworker called Renewal Desk already exists."); + }} + respond={async (result) => { + answers.push(String(result)); + }} + />, + ); + + await userEvent.click(await view.findByText("Create it")); + + await view.findByText("A coworker called Renewal Desk already exists."); + expect(answers).toEqual([]); + expect((view.getByText("Create it") as HTMLButtonElement).disabled).toBe( + false, + ); +}); + +test("declining answers without creating anything", async () => { + const answers: string[] = []; + let created = 0; + const view = routed( + { + created += 1; + return MADE; + }} + respond={async (result) => { + answers.push(String(result)); + }} + />, + ); + + await userEvent.click(await view.findByText("Don't create")); + + await waitFor(() => expect(answers.length).toBe(1)); + expect(answers[0]).toBe(botCardAnswer.declined()); + expect(created).toBe(0); +}); + +/** + * A proposal the fields refuse is answered, not shown as a question. + * + * There is nothing for the person to decide: the model can fix it, and the problems name their + * field so it can. Asking somebody to press a button on a coworker that cannot be created teaches + * them the button is unreliable. + */ +test("an unwritable proposal is answered without asking anybody", async () => { + const answers: string[] = []; + let created = 0; + const view = routed( + { + created += 1; + return MADE; + }} + respond={async (result) => { + answers.push(String(result)); + }} + />, + ); + + await waitFor(() => expect(answers.length).toBe(1)); + expect(answers[0]).toContain("Not created, and the person was not asked"); + expect(answers[0]).toContain("name:"); + expect(created).toBe(0); + expect(view.queryByText("Create it")).toBeNull(); +}); + +/** + * The completed card, rendered the way the SDK renders one after a reload: the arguments and the + * recorded answer, and nothing this component remembered. + * + * That is the whole reason the id travels in the sentence. A card that kept it in state would draw + * this link once, for the person who pressed the button, and then silently stop drawing it for + * anybody who refreshed the page — which is not a failure anything would report. + */ +test("a completed card links to the coworker, rebuilt from the answer alone", async () => { + const view = routed( + MADE} + result={botCardAnswer.created(MADE)} + />, + ); + + const link = await view.findByText("Open Renewal Desk"); + expect(link.closest("a")?.getAttribute("href")).toContain("agent=agent_1"); +}); + +/** + * The sentence and the reader of the sentence, pinned together. + * + * They are two literals in one module and a reword can separate them, at which point the link stops + * appearing and nothing says why. This is the test that makes that loud. + */ +test("the created answer carries an id the card can read back", () => { + const answer = botCardAnswer.created(MADE); + + expect(wasCreated(answer)).toBe(true); + expect(createdBotIdIn(answer)).toBe("agent_1"); + // A declined card is not a created one, and carries no coworker to link to. + expect(wasCreated(botCardAnswer.declined())).toBe(false); + expect(createdBotIdIn(botCardAnswer.declined())).toBeNull(); +}); diff --git a/examples/fintech/agents.yaml b/examples/fintech/agents.yaml index 292305ab7..4e35696cb 100644 --- a/examples/fintech/agents.yaml +++ b/examples/fintech/agents.yaml @@ -8,6 +8,16 @@ agents: avatar_seed: general-assistant type: built-in system_prompt: You are a helpful general assistant. Give clear, concise, and accurate answers. + # Turns this Bot into the one you make coworkers with, and this line is the whole gate: the app + # offers `list_bots`, `read_bot`, `list_bot_skills` and `save_bot` only to a Bot holding this + # slug. Remove it and making a coworker in a conversation stops working, with nothing on any + # screen to say why. + # + # On this Bot rather than Knowledge because it is the one somebody talks to first and the one + # with no specialism to dilute. Knowledge narrows to document tools per run, and adding a job + # with nothing to do with documents to that list is how the narrowing stops working for both. + skills: + - bot-creator - id: knowledge name: Knowledge title: Company Knowledge diff --git a/examples/fintech/skills.yaml b/examples/fintech/skills.yaml index 24a1cca4c..45efbca05 100644 --- a/examples/fintech/skills.yaml +++ b/examples/fintech/skills.yaml @@ -78,3 +78,44 @@ skills: tools: - google-drive/search_files - google-drive/read_file_content + + # Making a coworker by talking to one, rather than by filling in the New coworker form. + # + # DECLARES NO TOOLS, and that is not an omission. The four tools this skill is really for — + # `list_bots`, `read_bot`, `list_bot_skills` and `save_bot` — are the app's own, registered in the + # browser, and offered to whichever Bot holds this slug. `tools:` here is intersected with a Bot's + # MCP GRANTS, so naming them would name nothing. The grant of this skill is the whole gate. + - slug: bot-creator + title: Make a coworker + summary: Work out what a new coworker should be, then put it in front of you to create. + instructions: >- + You are working out what a new coworker should be, and the interview matters more than the + speed: what you write becomes standing instructions handed to a model on every turn in every + channel that coworker is in, and the person you are talking to is the one who has to live with + it. + + Start by calling list_bots. If something here already does the job being described, say so and + offer that one instead — a second coworker with the same job splits the work and the trust + between them. Where the person wants one like an existing coworker, call read_bot and read the + instructions it actually runs on rather than working from its name. + + Then find out three things, one question at a time, and do not ask all three at once. What + job it does, in the words the person would use. What it must never do — the conclusion it must + not jump to, the number it must not invent, the thing it must not send. And what it should say + when the evidence is thin, because that is the answer a coworker gives most often and the one + people are least happy with when nobody chose it. + + Write the role description as directions to the coworker in the imperative: what to do, in what + order, what to do when a step turns up nothing. Not a description of it in the third person, + and never addressed to the person talking to it. Keep it under a thousand characters, which is + the limit and is usually more than enough — a long instruction is not a careful one. + + Call list_bot_skills and offer only the skills that fit the job. Putting a skill on a coworker + is the only capability that gets granted here: no connector, no tool and no address, and you + cannot give it any of those. Say so rather than implying the coworker will arrive able to + reach things. Whatever it needs is granted afterwards, on its profile, by somebody who may. + + Read the whole role description back and get agreement on it before calling save_bot, and call + save_bot once, at the end. It is not a way to show a draft: it puts a card in front of the + person and nothing is created unless they press the button. + tools: [] diff --git a/server/src/agents/profile-store.ts b/server/src/agents/profile-store.ts index 8d14bdd48..3198a8a8d 100644 --- a/server/src/agents/profile-store.ts +++ b/server/src/agents/profile-store.ts @@ -47,7 +47,23 @@ export type AgentProfileStore = { actor: AgentActor, id: string, ): Promise; - create(actor: AgentActor, input: CreateAgentInput): Promise; + create( + actor: AgentActor, + input: CreateAgentInput & { + /** + * The standing instruction for a coworker with nowhere else to run. + * + * Only consulted when there is neither an endpoint nor a Bot in the box, and it is what makes + * that a `built_in` coworker rather than a refusal. `POST /api/agents` never sends it — the + * form has no such field — so a hand-made Bot on a deployment without a managed agent is + * refused exactly as it is today. + * + * Not on `CreateAgentInput`, so `update` cannot take it: changing an existing Bot's type is a + * different act with different consequences, and this is the creation path. + */ + systemPrompt?: string; + }, + ): Promise; update( actor: AgentActor, id: string, @@ -307,34 +323,58 @@ export function createAgentProfileStore( const endpoint = input.endpoint ? { endpoint: input.endpoint } : managedConfiguration; - if (!endpoint) { + const systemPrompt = input.systemPrompt?.trim(); + if (endpoint) { + await transaction.insert(agents).values({ + id, + name: input.name, + type: "remote_ag_ui", + // Their endpoint if they gave one, ours if they did not. Validated before it reaches + // here; see endpoint.ts for why a stored URL is a security decision and not a text + // field. + // + // The key, if there is one, goes to the vault and only its reference is stored here. See + // auth-header.ts for why a bearer token must not sit next to the endpoint. + configuration: { + ...endpoint, + ...(input.auth && vault + ? { + auth: await storeAgentAuth({ + store: vault.store, + encryptionKey: vault.encryptionKey, + agentId: id, + header: input.auth.header, + value: input.auth.value, + executor: transaction, + }), + } + : {}), + }, + }); + } else if (systemPrompt) { + /* + * Nowhere to send it, so it runs here. + * + * This is the shape General Assistant and Knowledge already have, and the shape + * `registeredAgentFromRow` reads: `built_in` plus a non-empty `configuration.systemPrompt`. + * A key is deliberately not written on this branch — a key authenticates to an address and + * this coworker has none, so storing one would leave a live credential in the vault that + * nothing can ever present. + */ + await transaction.insert(agents).values({ + id, + name: input.name, + type: "built_in", + configuration: { systemPrompt }, + }); + } else { + /* + * No address, no Bot in the box, and no instruction to run on. There is nothing to create: + * a `built_in` row with an empty prompt is a coworker `registeredAgentFromRow` drops on + * the floor, and the Bot would exist on every screen while answering nobody. + */ throw new ManagedAgentUnavailableError(); } - await transaction.insert(agents).values({ - id, - name: input.name, - type: "remote_ag_ui", - // Their endpoint if they gave one, ours if they did not. Validated before it reaches here; - // see endpoint.ts for why a stored URL is a security decision and not a text field. - // - // The key, if there is one, goes to the vault and only its reference is stored here. See - // auth-header.ts for why a bearer token must not sit next to the endpoint. - configuration: { - ...endpoint, - ...(input.auth && vault - ? { - auth: await storeAgentAuth({ - store: vault.store, - encryptionKey: vault.encryptionKey, - agentId: id, - header: input.auth.header, - value: input.auth.value, - executor: transaction, - }), - } - : {}), - }, - }); await transaction.insert(agentProfiles).values({ agentId: id, ownerUserId: actor.id, @@ -369,7 +409,7 @@ export function createAgentProfileStore( * it alone" rather than "remove it". */ const [row] = await transaction - .select({ configuration: agents.configuration }) + .select({ configuration: agents.configuration, type: agents.type }) .from(agents) .where(eq(agents.id, id)) .limit(1); @@ -377,8 +417,29 @@ export function createAgentProfileStore( string, unknown >; + /** + * A coworker that runs here runs on its role description, so editing one has to move both. + * + * `create` writes the role description into `configuration.systemPrompt` for a coworker + * with no address, and that prompt is the ONLY instruction such a coworker ever gets: + * `registeredAgentFromRow` gives a `built_in` agent its `systemPrompt` and no standing + * role message, so `agentProfiles.roleDescription` never reaches it. Left out of this + * merge, an edit wrote the new text to the profile every screen reads and left the Bot + * running on the original — permanently, with nothing anywhere to say so. That is the + * worst shape a failed edit can take, and it is the same one the endpoint comment above + * describes. + * + * Only for `built_in`, and that matters. A remote Bot has no `systemPrompt` and must not + * acquire one — its instruction travels as the standing role message instead — and the + * tenant package's Bots, whose `system_prompt` is deliberately not their + * `role_description`, cannot reach this code at all: `requireManageable` above throws + * `ProtectedAgentError` for anything the package owns. + */ const configuration = { ...previous, + ...(row?.type === "built_in" + ? { systemPrompt: input.roleDescription } + : {}), ...(input.endpoint ? { endpoint: input.endpoint } : {}), ...(input.auth && vault ? { diff --git a/server/src/agents/routes.ts b/server/src/agents/routes.ts index 7a2302284..cca78c487 100644 --- a/server/src/agents/routes.ts +++ b/server/src/agents/routes.ts @@ -354,7 +354,22 @@ export function createAgentRoutes( if (!parsed.ok) return context.json({ error: parsed.error }, 400); try { - const agent = await store.create(context.var.actor, parsed.value); + /* + * A coworker with no address runs here, on the text this form already requires. + * + * "Agent endpoint (optional)" was not optional on the recommended one-container image: with + * nothing to bind to, `create` refused with "This deployment has no managed Bot", so a person + * could not make a coworker at all on the image the README tells them to deploy. The role + * description is what such a coworker runs on — the same field a `built_in` Bot in the tenant + * package carries, for the same purpose — and passing it only when no endpoint was given keeps + * every other path exactly as it was: give an address and it is a remote Bot, as before. + */ + const agent = await store.create(context.var.actor, { + ...parsed.value, + ...(parsed.value.endpoint + ? {} + : { systemPrompt: parsed.value.roleDescription }), + }); /* * The endpoint, because that is where conversation content will be sent, and whether a key was * attached, because "this Bot authenticates" is a fact and the key itself never is. diff --git a/server/tests/agent-profile-store.integration.test.ts b/server/tests/agent-profile-store.integration.test.ts index 01dc052e2..7bbc5cd39 100644 --- a/server/tests/agent-profile-store.integration.test.ts +++ b/server/tests/agent-profile-store.integration.test.ts @@ -221,6 +221,31 @@ async function racePackageAttachment( } } +/** + * The stored row behind a coworker, proven to exist before anything reads it. + * + * Asserted here rather than at each call site because a missing row and a missing field are + * different failures that an optional chain would collapse into the same one — "systemPrompt is + * undefined" reads as a prompt that was not written when it may be a coworker that is not there. + */ +async function agentRow(agentId: string): Promise<{ + type: string; + configuration: { systemPrompt?: string; endpoint?: string }; +}> { + const [row] = await database + .select({ type: agents.type, configuration: agents.configuration }) + .from(agents) + .where(eq(agents.id, agentId)); + if (!row) throw new Error(`no agents row for ${agentId}`); + return { + type: row.type, + configuration: (row.configuration ?? {}) as { + systemPrompt?: string; + endpoint?: string; + }, + }; +} + describe("agent profile store integration", () => { test("refuses to create a coworker with no endpoint when this deployment has no managed Bot", async () => { const owner = await createUser(); @@ -236,6 +261,97 @@ describe("agent profile store integration", () => { ).rejects.toBeInstanceOf(ManagedAgentUnavailableError); }); + /** + * The coworker with nowhere to send it, and the instruction it actually runs on. + * + * `registeredAgentFromRow` gives a `built_in` agent its `configuration.systemPrompt` and NO + * standing role message, so that column is the whole of what such a coworker is ever told — + * `agentProfiles.roleDescription` never reaches it. These two tests exist because the pair can + * drift silently: creating writes both, and an edit that wrote only the profile left every screen + * showing new instructions while the Bot went on following the old ones, for good, with nothing + * anywhere to say so. + */ + test("creates a coworker that runs here when there is nowhere to send it", async () => { + const owner = await createUser(); + const withoutManaged = createAgentProfileStore(database, undefined); + + const created = await withoutManaged.create(owner, { + name: "Runs Here", + title: "Everyday Work", + roleDescription: "Answer from the ledger and quote the line you used.", + visibility: "private", + systemPrompt: "Answer from the ledger and quote the line you used.", + }); + createdAgentIds.push(created.id); + + const row = await agentRow(created.id); + expect(row.type).toBe("built_in"); + expect(row.configuration.systemPrompt).toBe( + "Answer from the ledger and quote the line you used.", + ); + // No address was given and none was invented; that is what makes it built_in rather than remote. + expect(row.configuration.endpoint).toBeUndefined(); + }); + + test("an edit moves the instruction such a coworker actually runs on", async () => { + const owner = await createUser(); + const withoutManaged = createAgentProfileStore(database, undefined); + const created = await withoutManaged.create(owner, { + name: "Runs Here", + title: "Everyday Work", + roleDescription: "The first instruction.", + visibility: "private", + systemPrompt: "The first instruction.", + }); + createdAgentIds.push(created.id); + + await withoutManaged.update(owner, created.id, { + name: "Runs Here", + title: "Everyday Work", + roleDescription: "The second instruction, which must be the live one.", + visibility: "private", + }); + + const row = await agentRow(created.id); + expect(row.type).toBe("built_in"); + expect(row.configuration.systemPrompt).toBe( + "The second instruction, which must be the live one.", + ); + // And the profile every screen reads agrees with it, rather than only the profile moving. + expect((await profileById(owner, created.id)).roleDescription).toBe( + "The second instruction, which must be the live one.", + ); + }); + + /** + * The other half of the same rule: a remote coworker must not acquire a prompt it never had. + * + * Its instruction travels as the standing role message built from the profile, so a `systemPrompt` + * appearing in its configuration would be a second source for the same thing — and the one the + * runtime prefers for a `built_in` row, which is what this coworker would look like if its type + * ever changed. + */ + test("an edit never gives a coworker at its own address a system prompt", async () => { + const owner = await createUser(); + const source = await createProfileFixture({ + owner, + visibility: "private", + configuration: { endpoint: "https://remote.example.test/ag-ui" }, + }); + + await store.update(owner, source.agentId, { + name: "Still Remote", + title: "Elsewhere", + roleDescription: "Edited, and it still runs at its own address.", + visibility: "private", + endpoint: "https://remote.example.test/ag-ui", + }); + + const row = await agentRow(source.agentId); + expect(row.type).toBe("remote_ag_ui"); + expect(row.configuration.systemPrompt).toBeUndefined(); + }); + test("lets an owner and admin get and list a private profile but hides it from another user", async () => { const owner = await createUser(); const other = await createUser(); diff --git a/server/tests/agent-routes.test.ts b/server/tests/agent-routes.test.ts index 6eaab5fb0..500384d06 100644 --- a/server/tests/agent-routes.test.ts +++ b/server/tests/agent-routes.test.ts @@ -304,7 +304,20 @@ describe("agent lifecycle routes", () => { expect(store.calls).toEqual([ ["list", actor, false], ["get", actor, "agent-1"], - ["create", actor, validInput], + /* + * `create` carries a system prompt and `update` does not, and that difference is the point. + * This input names no endpoint, so on a deployment with no Bot in the box the coworker runs + * here on its own role description rather than being refused — the form calls the endpoint + * optional and it now is. `update` is deliberately untouched: changing an existing Bot's type + * is a different act and must not happen through the edit path. + * + * The other create in this file passes an endpoint and correctly gets no prompt. + */ + [ + "create", + actor, + { ...validInput, systemPrompt: validInput.roleDescription }, + ], ["update", actor, "agent-1", validInput], ["duplicate", actor, "agent-1"], ["setHidden", actor, "agent-1", true], diff --git a/server/tests/tenant-package.test.ts b/server/tests/tenant-package.test.ts index 072585328..9521cfb5c 100644 --- a/server/tests/tenant-package.test.ts +++ b/server/tests/tenant-package.test.ts @@ -307,7 +307,14 @@ describe("tenant YAML validation", () => { systemPrompt: "You are a helpful general assistant. Give clear, concise, and accurate answers.", }, - skills: [], + /* + * One skill, and it is a gate rather than a preference. The app offers `list_bots`, + * `read_bot`, `list_bot_skills` and `save_bot` only to a Bot holding `bot-creator`, so this + * line is what makes making a coworker in a conversation possible at all. Dropped from the + * package, the feature stops working with nothing on any screen to say why, which is exactly + * why it is asserted here rather than left to whoever edits the YAML next. + */ + skills: ["bot-creator"], }); // The pairing the shipped package makes, which is the whole reason Knowledge narrows to document // tools rather than being offered everything its grants hold.