From bf4861dc0ebd09ebbd453f6fc26aecaea44384b4 Mon Sep 17 00:00:00 2001 From: David McKay Date: Mon, 24 Aug 2026 10:54:10 -0700 Subject: [PATCH] Let a package say which skills each coworker gets Knowledge ships as one of three coworkers, described as answering company questions and citing sources, and the skills that would let it do that were seeded attached to nobody. So every clone started with the narrowing they exist for switched off, until somebody opened the Skills page and made the pairing by hand, in each deployment, again after each connector. The package wrote both files and knows which coworker it meant them for. This grants nothing, which is what makes seeding it safe. A skill is an instruction; what a Bot may call is its grants, and the offer each run is the intersection of the two, so a skill naming a tool its Bot does not hold loads nothing. MCP grants reach a person's own account and stay an administrator's decision, untouched. A redeploy takes back only what it gave: package grants carry a mark, and one an administrator made keeps their name and survives. A slug the package does not ship is refused at load rather than dropped, and a slug a person already owns is never granted, because the seed skips it and granting anyway would hand this Bot a stranger's instructions under a name the package expected to be its own. --- CHANGELOG.md | 25 ++++ examples/fintech/agents.yaml | 15 +++ server/src/tenant-package.ts | 105 +++++++++++++++- server/tests/tenant-package.test.ts | 179 +++++++++++++++++++++++++++- 4 files changed, 317 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3ab6e5e2..cbee9e84 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,31 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged. ## Unreleased +### Knowledge searches instead of guessing + +A package can say which of its skills each coworker gets, and the fintech example gives Knowledge the +four document skills it ships. + +Knowledge is one of three coworkers in the box, described as answering company questions and citing +sources. The skills that would let it do that were seeded attached to nobody, so every clone started +with them paired to no Bot: the per-run narrowing that skills exist for was switched off until +somebody opened the Skills page and made the pairing by hand, in each deployment, again after each +new connector. The pairing belongs with the package, which wrote both files and knows which coworker +it meant them for. + +THIS GRANTS NOTHING, which is what makes it safe to seed. A skill is an instruction; what a Bot may +call is its grants, and the offer each run is the intersection of the two. A skill naming a tool its +Bot does not hold loads nothing. Seeding an MCP grant would be the opposite, because those reach a +person's own account, so those stay an administrator's decision and are untouched here. + +A redeploy takes back only what the package gave. Grants it made carry `tenant-package`, and a grant +an administrator made through the Skills page keeps their name and survives, because a deploy quietly +undoing a deliberate decision is the kind of change nobody traces back to the deploy that caused it. +A coworker naming a skill its package does not ship is refused at load rather than dropped, the same +as a channel naming an agent that is not there: a typo that silently attaches nothing looks exactly +like working. + + ### A Bot's computer is no longer on the same network as the database Compose declared no networks, so every service shared one and reached the others by service name. diff --git a/examples/fintech/agents.yaml b/examples/fintech/agents.yaml index 4d58dfcd..292305ab 100644 --- a/examples/fintech/agents.yaml +++ b/examples/fintech/agents.yaml @@ -31,6 +31,21 @@ agents: used. If you have no tool for a source, or a tool tells you it is not connected or reports an error, say that plainly. Never answer from your own memory as though it came from a source, and never claim you lack access to something a tool has just returned. + # The document skills, which is what makes the prompt above reachable rather than aspirational. + # + # THIS GRANTS NOTHING. A skill is an instruction, and what a Bot may call is its grants: the + # offer each run is the intersection of the two, so until somebody connects Drive and grants + # these tools, naming them here loads nothing and this Bot correctly says it has no source. + # The moment they do, it searches instead of guessing, with no second step to remember. + # + # Stated here rather than left to the Skills page because the package wrote both files and knows + # which Bot it meant them for. Left to a screen, every clone starts with the skills attached to + # nobody, which switches off the per-run narrowing they exist for. + skills: + - find-a-document + - check-a-claim + - whats-changed + - who-owns-this # The Bot that ships in the box (agent-bot), addressed exactly the way a customer's own Bot would # be: an AG-UI endpoint in the registry. Names in dollar-brace form are read from the environment, # so the address belongs to the deployment. Replace it with your own service and nothing changes. diff --git a/server/src/tenant-package.ts b/server/src/tenant-package.ts index 94d48cb3..549b51a2 100644 --- a/server/src/tenant-package.ts +++ b/server/src/tenant-package.ts @@ -1,9 +1,9 @@ -import { DEPLOYMENT_ROUTES } from "./computer/deployment-routes"; import { createHash } from "node:crypto"; import { readFile } from "node:fs/promises"; import { join } from "node:path"; -import { desc, eq, inArray, isNull } from "drizzle-orm"; +import { and, desc, eq, inArray, isNull } from "drizzle-orm"; import { parse } from "yaml"; +import { DEPLOYMENT_ROUTES } from "./computer/deployment-routes"; import type { Database } from "./db/client"; import { agentProfiles, @@ -11,8 +11,9 @@ import { channelAgents, channels as channelTable, deploymentPackages, - skillTools, + pluginGrants, skills as skillTable, + skillTools, } from "./db/schema"; const approvedThemeVariables = new Set([ @@ -125,6 +126,14 @@ export type TenantSkill = { tools: string[]; }; +/** + * The mark a grant this package made carries, in `plugin_grants.granted_by`. + * + * Every other value there is the id of the person who pressed the button, so this cannot collide + * with one, and it is what lets a redeploy take back only what the package gave. + */ +const PACKAGE_GRANT = "tenant-package"; + type TenantAgent = { id: string; name: string; @@ -133,6 +142,19 @@ type TenantAgent = { avatarSeed?: string; type: "built_in" | "remote_ag_ui"; configuration: Record; + /** + * The package skills this coworker is given, by slug. + * + * SAFE TO SEED, unlike an MCP grant, and for a reason worth stating rather than assuming. A skill + * is an instruction and confers nothing: what a Bot may call is its grants, and the run-time offer + * is always the intersection of the two, so a skill naming a tool its Bot does not hold loads + * nothing. Seeding an MCP grant would be the opposite, since those reach a person's own account. + * + * Without this a fork boots with the skills its package ships attached to no Bot at all, and the + * per-run narrowing that skills exist for is switched off until somebody opens the Skills page and + * pairs them by hand. The pairing is the package's to state: it wrote both files. + */ + skills: string[]; }; type TenantChannel = { @@ -361,11 +383,35 @@ export function validateTenantPackage(files: PackageFiles): TenantPackage { : { endpoint: requiredString(agent.endpoint, "agent.endpoint"), }, + skills: + agent.skills === undefined || agent.skills === null + ? [] + : stringArray(agent.skills, "agent.skills"), }, ]; }, ); const agentIds = new Set(agents.map((agent) => agent.id)); + const packageSkills = parseTenantSkills(skillsYaml.skills); + const skillSlugs = new Set(packageSkills.map((skill) => skill.slug)); + for (const agent of agents) { + for (const slug of agent.skills) { + /* + * Checked against this package's own skills and nothing else, and refused rather than dropped. + * + * The two files ship together, so a slug matching none of them is a typo, and a typo that + * silently attaches no skill is the kind nobody finds: the Bot simply never narrows and the + * deployment looks like it is working. Deliberately not checked against skills already in the + * deployment, because those include any a person wrote, and a package must not be able to + * hand its Bots somebody else's instructions by naming their slug. + */ + if (!skillSlugs.has(slug)) { + throw new Error( + `agent "${agent.id}" names skill "${slug}", which this package does not ship`, + ); + } + } + } const channels = asList(channelsYaml.channels, "channels.yaml channels").map( (value) => { const channel = asRecord(value, "channel"); @@ -427,7 +473,7 @@ export function validateTenantPackage(files: PackageFiles): TenantPackage { defaultModel: requiredString(model.default_model, "model.default_model"), }, knowledgeSources: sources, - skills: parseTenantSkills(skillsYaml.skills), + skills: packageSkills, themeCss: files.themeCss, }; } @@ -688,6 +734,29 @@ export async function synchronizeTenantPackage( * of shipping it. An unknown ref sits inert until its connector exists, because the run-time * intersection only ever offers what the Bot was granted. */ + /* + * Which coworkers asked for each skill, so the loop below can pair them as it seeds. + * + * Written wholesale under one marker: every grant this package made last time is removed first, + * so a package that stops giving a Bot a skill takes it back. Only its own, though. A grant an + * administrator made through the Skills page carries their id and survives, because retracting + * somebody's deliberate decision is not something a redeploy should do quietly. + */ + const wantedBy = new Map(); + for (const agent of tenantPackage.agents) { + for (const slug of agent.skills) { + wantedBy.set(slug, [...(wantedBy.get(slug) ?? []), agent.id]); + } + } + await transaction + .delete(pluginGrants) + .where( + and( + eq(pluginGrants.kind, "skill"), + eq(pluginGrants.grantedBy, PACKAGE_GRANT), + ), + ); + for (const skill of tenantPackage.skills) { const [seeded] = await transaction .insert(skillTable) @@ -748,6 +817,34 @@ export async function synchronizeTenantPackage( })), ); } + + /* + * Paired only with the skill this package actually owns. + * + * Inside this branch on purpose: the seed above skips a slug a person had already taken, and + * granting there would hand the package's Bots an instruction somebody else wrote under a name + * the package expected to be its own. Skipped, it keeps the existing warning and grants + * nothing, which is the safe half of the same decision. + */ + const agentIds = wantedBy.get(skill.slug) ?? []; + if (agentIds.length > 0) { + await transaction + .insert(pluginGrants) + .values( + agentIds.map((agentId) => ({ + kind: "skill", + ref: seeded.id, + agentId, + grantedBy: PACKAGE_GRANT, + })), + ) + .onConflictDoUpdate({ + target: [pluginGrants.kind, pluginGrants.ref, pluginGrants.agentId], + // An administrator who granted this by hand keeps the credit; the package only ever + // adds what was missing, and the delete above already took back what it owned. + set: { updatedAt: new Date() }, + }); + } } return deploymentPackage; diff --git a/server/tests/tenant-package.test.ts b/server/tests/tenant-package.test.ts index 766fb9a9..07258532 100644 --- a/server/tests/tenant-package.test.ts +++ b/server/tests/tenant-package.test.ts @@ -1,14 +1,14 @@ import { afterAll, afterEach, describe, expect, test } from "bun:test"; import { randomUUID } from "node:crypto"; -import { eq } from "drizzle-orm"; +import { and, eq } from "drizzle-orm"; import { createDatabase } from "../src/db/client"; -import { TEST_POOL } from "./support/database"; import { agentProfiles, agents, deploymentPackages, - skillTools, + pluginGrants, skills as skillsTable, + skillTools, users, } from "../src/db/schema"; import { @@ -20,6 +20,7 @@ import { validateTenantPackage, validateThemeCss, } from "../src/tenant-package"; +import { TEST_POOL } from "./support/database"; const database = createDatabase( process.env.DATABASE_URL ?? @@ -64,6 +65,7 @@ function packageAgent( roleDescription: "Help with everyday work.", type: "built_in", configuration: { systemPrompt: "Be helpful." }, + skills: [], ...overrides, }; } @@ -305,7 +307,18 @@ describe("tenant YAML validation", () => { systemPrompt: "You are a helpful general assistant. Give clear, concise, and accurate answers.", }, + skills: [], }); + // The pairing the shipped package makes, which is the whole reason Knowledge narrows to document + // tools rather than being offered everything its grants hold. + expect( + tenantPackage.agents.find((agent) => agent.id === "knowledge")?.skills, + ).toEqual([ + "find-a-document", + "check-a-claim", + "whats-changed", + "who-owns-this", + ]); expect(tenantPackage.channels).toContainEqual({ id: "general-assistant", name: "General Assistant", @@ -1053,3 +1066,163 @@ describe("seeding the skills a package ships", () => { expect(row?.ownerUserId).toBe(owner); }); }); + +/** + * Pairing a package's coworkers with the skills it ships. + * + * A deployment that seeds skills attached to nobody has switched off the narrowing they exist for, + * so the package states the pairing. The cases below are the ones that decide whether seeding a + * grant is safe: it never confers a capability, it never reaches somebody else's skill, and a + * redeploy can take back its own without touching an administrator's. + */ +describe("pairing a package's coworkers with its skills", () => { + const createdSkillIds: string[] = []; + + afterEach(async () => { + for (const id of createdSkillIds.splice(0)) { + await database.delete(pluginGrants).where(eq(pluginGrants.ref, id)); + await database.delete(skillTools).where(eq(skillTools.skillId, id)); + await database.delete(skillsTable).where(eq(skillsTable.id, id)); + } + }); + + function packageGiving(slugs: string[]) { + const agent = packageAgent({ skills: slugs }); + const loaded = loadedPackage(agent); + const skills = slugs.map((slug) => ({ + slug, + title: "Find a document", + summary: "Search the sources and read what comes back.", + instructions: "Search first, then read the file you found.", + tools: ["google-drive/search_files"], + })); + for (const slug of slugs) createdSkillIds.push(slug); + createdAgentIds.push(agent.id); + return { ...loaded, skills }; + } + + const grantsFor = (agentId: string) => + database + .select() + .from(pluginGrants) + .where( + and(eq(pluginGrants.kind, "skill"), eq(pluginGrants.agentId, agentId)), + ); + + test("a coworker is granted the skills its package named", async () => { + const slug = `pkg-${randomUUID().slice(0, 8)}`; + const loaded = packageGiving([slug]); + const created = await synchronizeTenantPackage(database, loaded); + createdPackageIds.push(created.id); + + const granted = await grantsFor(loaded.agents[0]?.id as string); + expect(granted.map((row) => row.ref)).toEqual([slug]); + // The mark is what lets a later deploy take back its own and nobody else's. + expect(granted[0]?.grantedBy).toBe("tenant-package"); + }); + + test("a skill the package stopped naming is taken back", async () => { + const slug = `pkg-${randomUUID().slice(0, 8)}`; + const loaded = packageGiving([slug]); + createdPackageIds.push( + (await synchronizeTenantPackage(database, loaded)).id, + ); + + const agentId = loaded.agents[0]?.id as string; + expect(await grantsFor(agentId)).toHaveLength(1); + + // Same package, same skill still shipped, but the coworker no longer asks for it. + const withoutIt = { + ...loaded, + agents: [{ ...(loaded.agents[0] as never), skills: [] }], + } as typeof loaded; + createdPackageIds.push( + (await synchronizeTenantPackage(database, withoutIt)).id, + ); + + expect(await grantsFor(agentId)).toHaveLength(0); + }); + + test("a grant an administrator made by hand survives a redeploy", async () => { + const slug = `pkg-${randomUUID().slice(0, 8)}`; + const loaded = packageGiving([slug]); + createdPackageIds.push( + (await synchronizeTenantPackage(database, loaded)).id, + ); + + const agentId = loaded.agents[0]?.id as string; + // Somebody decides this Bot should keep it, through the Skills page. + await database + .update(pluginGrants) + .set({ grantedBy: "an-administrator" }) + .where( + and(eq(pluginGrants.kind, "skill"), eq(pluginGrants.agentId, agentId)), + ); + + const withoutIt = { + ...loaded, + agents: [{ ...(loaded.agents[0] as never), skills: [] }], + } as typeof loaded; + createdPackageIds.push( + (await synchronizeTenantPackage(database, withoutIt)).id, + ); + + /* + * Kept. The package retracts what it gave, not what somebody decided: a redeploy quietly undoing + * a deliberate grant is the kind of thing nobody connects to the deploy that caused it. + */ + const granted = await grantsFor(agentId); + expect(granted.map((row) => row.ref)).toEqual([slug]); + expect(granted[0]?.grantedBy).toBe("an-administrator"); + }); + + test("a slug a person already owns is never granted to the package's Bot", async () => { + /* + * The case that decides whether this is safe at all. The seed skips a slug somebody already + * took, so granting anyway would hand this Bot an instruction a stranger wrote, under a name the + * package believed was its own. + */ + const slug = `pkg-${randomUUID().slice(0, 8)}`; + const owner = await createUser(); + await database.insert(skillsTable).values({ + id: slug, + ownerUserId: owner, + slug, + title: "Mine", + summary: "Mine.", + instructions: "Do it my way.", + origin: "yours", + }); + + const loaded = packageGiving([slug]); + createdPackageIds.push( + (await synchronizeTenantPackage(database, loaded)).id, + ); + + expect(await grantsFor(loaded.agents[0]?.id as string)).toHaveLength(0); + }); + + test("a coworker naming a skill the package does not ship is refused at load", () => { + // A typo here attaches nothing and looks exactly like working, so it is refused rather than + // dropped, the same as a channel naming an agent that is not there. + expect(() => + validateTenantPackage({ + brand: "tenant: { id: fintech, product_name: Ledgerline }", + agents: + "agents: [{ id: knowledge, name: Knowledge, title: Company Knowledge, role_description: Answer., type: built-in, system_prompt: Answer., skills: [no-such-skill] }]", + channels: "channels: []", + model: + "model: { provider: openai, credential_secret_ref: openai-key, default_model: gpt-5.6-terra }", + knowledge: "sources: []", + skills: `skills: + - slug: find-a-document + title: Find a document + summary: Search. + instructions: Search.`, + themeCss: "", + }), + ).toThrow( + 'agent "knowledge" names skill "no-such-skill", which this package does not ship', + ); + }); +});