diff --git a/apps/api/src/agent-skills.test.ts b/apps/api/src/agent-skills.test.ts index edc9ee0..873f309 100644 --- a/apps/api/src/agent-skills.test.ts +++ b/apps/api/src/agent-skills.test.ts @@ -26,9 +26,13 @@ function savedSkill(name: string, source = "user") { } function setup(rows: ReturnType[] = []) { - type Where = { id?: string; spaceId: string; userId: string; source?: string }; + type Where = { id?: string; spaceId?: string; userId: string; source?: string; name?: unknown }; const matches = (row: ReturnType, where: Where) => - Object.entries(where).every(([key, value]) => row[key as keyof typeof row] === value); + Object.entries(where).every(([key, value]) => + key === "name" && typeof value === "object" && value && "equals" in value + ? row.name.toLowerCase() === String(value.equals).toLowerCase() + : key === "NOT" || row[key as keyof typeof row] === value, + ); const agentSkill = { findMany: vi.fn(async ({ where }: { where: Where }) => rows.filter((row) => matches(row, where)), @@ -101,7 +105,23 @@ describe("built-in skill precedence in the API", () => { expect(agentSkill.deleteMany).not.toHaveBeenCalled(); }); - it.each([{ spaceId: "other-space" }, { userId: "other-user" }])( + it("lists the user's skills from other spaces too, the current space winning a clash", async () => { + const { service } = setup([ + { ...savedSkill("Launch"), id: "home-1", spaceId: "home" }, + { ...savedSkill("launch"), id: "here-1" }, + { ...savedSkill("Weekly review"), id: "home-2", spaceId: "home" }, + ]); + const own = (await service.list(actor)).filter((skill) => skill.source === "user"); + expect(own.map((skill) => skill.id).sort()).toEqual(["here-1", "home-2"]); + await expect(service.get(actor, { name: "Weekly review" })).resolves.toMatchObject({ + id: "home-2", + }); + await expect( + service.create(actor, { name: "weekly REVIEW", description: "x" }), + ).rejects.toThrow("already exists"); + }); + + it.each([{ userId: "other-user" }])( "does not let foreign skills shadow the builtin: %j", async (foreignOwner) => { const { service } = setup([{ ...savedSkill(" Interrogate "), ...foreignOwner }]); diff --git a/apps/api/src/agent-skills.ts b/apps/api/src/agent-skills.ts index a701ffd..0b552ba 100644 --- a/apps/api/src/agent-skills.ts +++ b/apps/api/src/agent-skills.ts @@ -7,6 +7,7 @@ import { isSkillReadOnly, mergeBuiltinSkills, parseSkillMd, + preferSpaceSkills, type SkillSource, } from "@rakazo/core"; import { IsolationError, type PrismaClient } from "@rakazo/db"; @@ -121,11 +122,7 @@ export function resolveSkillContent(input: { export function createAgentSkillsService(prisma: PrismaClient) { async function owned(actor: Actor, skillId: string) { const row = await prisma.agentSkill.findFirst({ - where: { - id: skillId, - spaceId: actor.spaceId, - userId: actor.userId, - }, + where: { id: skillId, userId: actor.userId }, }); if (!row) throw new IsolationError(); return row; @@ -134,20 +131,24 @@ export function createAgentSkillsService(prisma: PrismaClient) { return { async list(actor: Actor): Promise[]> { const rows = await prisma.agentSkill.findMany({ - where: { spaceId: actor.spaceId, userId: actor.userId }, + where: { userId: actor.userId }, orderBy: [{ name: "asc" }, { id: "asc" }], }); - return mergeBuiltinSkills(builtinCatalog(), rows.map(mapAgentSkill)).map( - ({ content: _content, ...entry }) => entry, - ); + return mergeBuiltinSkills( + builtinCatalog(), + preferSpaceSkills(rows, actor.spaceId).map(mapAgentSkill), + ).map(({ content: _content, ...entry }) => entry); }, async listWithContent(actor: Actor): Promise { const rows = await prisma.agentSkill.findMany({ - where: { spaceId: actor.spaceId, userId: actor.userId }, + where: { userId: actor.userId }, orderBy: [{ name: "asc" }, { id: "asc" }], }); - return mergeBuiltinSkills(builtinCatalog(), rows.map(mapAgentSkill)); + return mergeBuiltinSkills( + builtinCatalog(), + preferSpaceSkills(rows, actor.spaceId).map(mapAgentSkill), + ); }, async get(actor: Actor, input: { skillId?: string; name?: string }): Promise { @@ -161,13 +162,10 @@ export function createAgentSkillsService(prisma: PrismaClient) { } const name = input.name?.trim() ?? ""; const rows = await prisma.agentSkill.findMany({ - where: { - spaceId: actor.spaceId, - userId: actor.userId, - }, + where: { userId: actor.userId }, orderBy: [{ name: "asc" }, { id: "asc" }], }); - const row = findSkillByName(rows, name); + const row = findSkillByName(preferSpaceSkills(rows, actor.spaceId), name); if (row) return mapAgentSkill(row); const builtin = findSkillByName(builtinCatalog(), name); if (!builtin) throw new IsolationError(); @@ -181,7 +179,6 @@ export function createAgentSkillsService(prisma: PrismaClient) { const resolved = resolveSkillContent(input); const clash = await prisma.agentSkill.findFirst({ where: { - spaceId: actor.spaceId, userId: actor.userId, name: { equals: resolved.name, mode: "insensitive" }, }, @@ -234,7 +231,6 @@ export function createAgentSkillsService(prisma: PrismaClient) { if (!findSkillByName([existing], resolved.name)) { const clash = await prisma.agentSkill.findFirst({ where: { - spaceId: actor.spaceId, userId: actor.userId, name: { equals: resolved.name, mode: "insensitive" }, NOT: { id: existing.id }, @@ -250,12 +246,7 @@ export function createAgentSkillsService(prisma: PrismaClient) { // Mutate only owner-scoped user rows (never builtin/plugin), even if source was tampered. try { const updated = await prisma.agentSkill.updateMany({ - where: { - id: existing.id, - spaceId: actor.spaceId, - userId: actor.userId, - source: "user", - }, + where: { id: existing.id, userId: actor.userId, source: "user" }, data: { name: resolved.name, description: resolved.description, @@ -275,11 +266,7 @@ export function createAgentSkillsService(prisma: PrismaClient) { throw error; } const row = await prisma.agentSkill.findFirst({ - where: { - id: existing.id, - spaceId: actor.spaceId, - userId: actor.userId, - }, + where: { id: existing.id, userId: actor.userId }, }); if (!row) throw new IsolationError(); return mapAgentSkill(row); @@ -291,12 +278,7 @@ export function createAgentSkillsService(prisma: PrismaClient) { throw new ORPCError("BAD_REQUEST", { message: "Builtin and plugin skills are read-only." }); } const deleted = await prisma.agentSkill.deleteMany({ - where: { - id: existing.id, - spaceId: actor.spaceId, - userId: actor.userId, - source: "user", - }, + where: { id: existing.id, userId: actor.userId, source: "user" }, }); if (deleted.count !== 1) throw new IsolationError(); return { ok: true }; diff --git a/apps/api/src/router.ts b/apps/api/src/router.ts index 5f75292..d0ea50e 100644 --- a/apps/api/src/router.ts +++ b/apps/api/src/router.ts @@ -3256,8 +3256,9 @@ export function createRouter(deps: RouterDeps) { }, capabilities: { list: authed.capabilities.list.handler(async ({ context }) => { + // Tool sources and plugins follow the user into every space. const rows = await deps.prisma.capabilityInstall.findMany({ - where: { spaceId: context.actor.spaceId, userId: context.actor.userId }, + where: { userId: context.actor.userId }, }); return rows.map((row) => ({ id: row.id, @@ -3416,11 +3417,7 @@ export function createRouter(deps: RouterDeps) { remove: authed.capabilities.remove.handler(async ({ context, input }) => { await deps.prisma.$transaction(async (tx) => { const existing = await tx.capabilityInstall.findFirst({ - where: { - id: input.id, - spaceId: context.actor.spaceId, - userId: context.actor.userId, - }, + where: { id: input.id, userId: context.actor.userId }, }); if (!existing) return; await tx.capabilityInstall.delete({ where: { id: existing.id } }); @@ -3430,11 +3427,7 @@ export function createRouter(deps: RouterDeps) { }); if (shared === 0) { await tx.secret.deleteMany({ - where: { - id: existing.secretId, - spaceId: context.actor.spaceId, - userId: context.actor.userId, - }, + where: { id: existing.secretId, userId: context.actor.userId }, }); } } @@ -3620,11 +3613,7 @@ export function createRouter(deps: RouterDeps) { }); } else if (clearing && existing.secretId) { await tx.secret.deleteMany({ - where: { - id: existing.secretId, - spaceId: context.actor.spaceId, - userId: context.actor.userId, - }, + where: { id: existing.secretId, userId: context.actor.userId }, }); } return updated; diff --git a/packages/adapter-kit/src/types.ts b/packages/adapter-kit/src/types.ts index d2a9bf1..906b8cc 100644 --- a/packages/adapter-kit/src/types.ts +++ b/packages/adapter-kit/src/types.ts @@ -201,6 +201,8 @@ export interface ConnectorRoute { resourceRevision?: string | number; /** Source label for lazy catalog name indexes. Never exposed as a model schema field. */ catalogGroup?: string; + /** Sources a lazy catalog search tool covers, so the run can name what is connected. */ + catalogGroups?: string[]; } export interface ConnectorCall { diff --git a/packages/adapters/src/builtin-tools.ts b/packages/adapters/src/builtin-tools.ts index 9db3929..21ec656 100644 --- a/packages/adapters/src/builtin-tools.ts +++ b/packages/adapters/src/builtin-tools.ts @@ -959,7 +959,7 @@ export const builtinAgentTools: ConnectorTool[] = [ prompt: { type: "string", description: - "Concrete steps for when the schedule fires: name the connected plugin tools to call (e.g. GITHUB_LIST_RELEASES for owner/repo), what to extract, and how to report. Prefer plugin tools over computer browser or web search for app data.", + "Self-contained instructions for when the schedule fires. That run sees none of this conversation, so include every detail it needs: what to check or do, which connected app or skill to use (with exact names such as owner/repo), what to report, and when to say nothing. Prefer connected apps over the computer browser or web search for app data.", }, cron: { type: "string", description: "5-field cron for repeating schedules." }, every: { type: "number", description: "Repeat interval amount for repeating schedules." }, @@ -1007,7 +1007,7 @@ export const builtinAgentTools: ConnectorTool[] = [ { name: "skill_read", description: - "Load a Claude Agent Skill (SKILL.md recipe) by exact name. Call this when a catalog skill matches the user's request, then follow it immediately.", + "Load a skill (SKILL.md recipe) by its exact name from the catalog, then follow it right away.", inputSchema: { type: "object", properties: { @@ -1019,7 +1019,7 @@ export const builtinAgentTools: ConnectorTool[] = [ { name: "skill_create", description: - "Create a reusable Claude Agent Skill (generic how-to SKILL.md) shared across assistants. The Pi runtime already understands this format; we persist and inject them. Use when a multi-step task is worth repeating or the user asks to save a skill. Do not include account names, channels, or inboxes — those belong in a routine.", + "Create one of the user's skills: a reusable how-to (SKILL.md) available in all their projects. Use it when a multi-step task is worth repeating, the user asks to save a skill, or a skill_install skipped one. Keep account names, channels and inboxes out; those belong in a routine.", inputSchema: { type: "object", properties: { @@ -1043,7 +1043,7 @@ export const builtinAgentTools: ConnectorTool[] = [ { name: "skill_install", description: - "Install skills from a public GitHub link the user pasted: a repository, a folder, or one SKILL.md. Every SKILL.md found becomes one of the user's own skills. Only when the user asks to install from that link.", + "Install skills from a public GitHub link the user gave: a repository, a folder, or one SKILL.md. Every SKILL.md found becomes one of the user's own skills in all their projects. Call it once per link; the result lists installed and skipped skills.", inputSchema: { type: "object", properties: { diff --git a/packages/adapters/src/executor-instructions.test.ts b/packages/adapters/src/executor-instructions.test.ts index 4f19ad2..6e934ea 100644 --- a/packages/adapters/src/executor-instructions.test.ts +++ b/packages/adapters/src/executor-instructions.test.ts @@ -348,6 +348,26 @@ describe("run instructions", () => { expect(request.instructions).toContain("\nName: Ada Example\n"); }); + it("tells every agent how to work, naming only tools it has", async () => { + const { request } = await runOnce(); + const { instructions } = request; + expect(instructions.indexOf("How you work:")).toBeGreaterThan( + instructions.indexOf("You are Builder"), + ); + expect(instructions.indexOf("How you work:")).toBeLessThan( + instructions.indexOf("Answer in short sentences."), + ); + expect(instructions).toContain("Say something is done only after a tool confirmed it"); + expect(instructions).toContain("Reply in the language the user writes in."); + expect(instructions).toContain("No apps are connected yet."); + expect(instructions).not.toContain("A schedule started this run"); + }); + + it("tells a routine run that nobody is waiting and history is not shown", async () => { + const { request } = await runOnce({ trigger: "routine" }); + expect(request.instructions).toContain("A schedule started this run"); + }); + it("describes the main bot's role", async () => { const { request } = await runOnce({ bot: { kind: "main", name: "Chief", title: "Chief of Staff", instructions: "" }, diff --git a/packages/adapters/src/executor-prompt.test.ts b/packages/adapters/src/executor-prompt.test.ts index bf2f021..609a102 100644 --- a/packages/adapters/src/executor-prompt.test.ts +++ b/packages/adapters/src/executor-prompt.test.ts @@ -2,10 +2,12 @@ import type { UserContext } from "@rakazo/db"; import { describe, expect, it } from "vitest"; import { formatBotIdentity, + formatConnectionsInstruction, formatMainAgentRole, formatSubagentContext, formatUserContext, } from "./executor-prompt.js"; +import { lazyCatalogTools } from "./lazy-tool-catalog.js"; const user: UserContext = { name: "Ada Example", @@ -178,3 +180,40 @@ describe("formatSubagentContext", () => { ).toBe("You work for Helper.\n\nCurrent date."); }); }); + +describe("formatConnectionsInstruction", () => { + const tool = (name: string, catalogGroup: string) => ({ + name, + description: name, + inputSchema: {}, + route: { connectorId: "installed", resourceId: "install-1", toolName: name, catalogGroup }, + }); + + it("names apps from plugins, direct tools and lazy catalogs, and how to reach the catalog", () => { + const catalog = lazyCatalogTools( + "installed", + "installed", + "API", + Array.from({ length: 30 }, (_, index) => ({ + id: `gh-${index}`, + tool: tool(`gh_${index}`, "GitHub"), + group: "GitHub", + })), + ); + const line = formatConnectionsInstruction({ + apps: ["Gmail"], + tools: [tool("weather_now", "Weather"), ...catalog], + }); + expect(line).toContain("Connected apps: Gmail, Weather, GitHub."); + expect(line).toContain("Never tell the user one of these is not connected."); + expect(line).toContain( + "For GitHub: find the tool with installed_search_tools, load it with installed_load_tool, then run it with installed_execute_tool.", + ); + }); + + it("says plainly when nothing is connected", () => { + expect(formatConnectionsInstruction({ apps: [], tools: [] })).toBe( + "No apps are connected yet. The user connects them under Settings, Integrations.", + ); + }); +}); diff --git a/packages/adapters/src/executor-prompt.ts b/packages/adapters/src/executor-prompt.ts index 4b8c449..5a04cb5 100644 --- a/packages/adapters/src/executor-prompt.ts +++ b/packages/adapters/src/executor-prompt.ts @@ -1,3 +1,4 @@ +import type { ConnectorTool } from "@rakazo/adapter-kit"; import { escapePromptData, knownTimeZone } from "@rakazo/core"; import type { CoachSummary, UserContext } from "@rakazo/db"; import { COACH_TOOL_NAMES } from "./coach-tools.js"; @@ -34,6 +35,70 @@ export function formatBotIdentity(bot: BotIdentity): string { .join("\n"); } +/** + * How every agent works, whatever its role. Local models in particular stop at a plan, claim + * work a tool never confirmed, overbuild, or deny abilities they have, unless told otherwise. + * Lines that need a tool appear only when the run has it. + */ +export function formatWorkingPrinciples(toolNames: ReadonlySet): string { + const canBuildTools = + toolNames.has("shell") && toolNames.has("write_file") && toolNames.has("skill_create"); + return [ + "How you work:", + "- Do the work with your tools instead of describing what you would do, and keep going until the request is done or truly blocked.", + "- Understand before you act: read the request and what it touches. On anything non-trivial, state the assumptions you make.", + "- Prefer the simplest thing that works: reuse what exists, add nothing that was not asked for, and touch only what the task needs.", + "- Say something is done only after a tool confirmed it: read back what you wrote, run what you built, check what you changed. When something fails, find the cause instead of working around it, say what failed, and try another way.", + "- When a request has a real problem, say so and propose the better option, then follow the user's decision.", + toolNames.has("skill_read") + ? "- When a skill fits the task, load it and follow all its steps, checks included." + : undefined, + "- Before saying you cannot do something, check your tools, the connected apps and the skills listed here.", + canBuildTools + ? "- When no tool does what the task needs, build one: write a script in your workspace, test it, and save how to run it as a skill with skill_create so you and the user can reuse it." + : undefined, + "- Ask only when the answer changes the outcome and there is no sensible default; otherwise pick the default and say which.", + "- Reply in the language the user writes in. Lead with the result and keep it short.", + ] + .filter(Boolean) + .join("\n"); +} + +/** A routine run has no conversation and nobody waiting; it reports what it did. */ +export const ROUTINE_RUN_INSTRUCTION = + "A schedule started this run; nobody is waiting in the chat and earlier messages are not shown. Do what the task says with your tools, then post a short result the user can read later. If the task only asks for news and there is none, say so in one line."; + +/** + * What the user connected, named from the tools this run actually received, so the agent + * neither denies a connected app nor claims one whose tools failed to load. Apps reached + * through a lazy catalog come with the search tool that lists their tools. + */ +export function formatConnectionsInstruction(input: { + apps: readonly string[]; + tools: readonly ConnectorTool[]; +}): string { + const names = new Set(input.apps.map((app) => app.trim()).filter(Boolean)); + const catalogs: string[] = []; + for (const tool of input.tools) { + const route = tool.route; + if (route?.catalogGroup) names.add(route.catalogGroup); + if (route?.catalogGroups?.length) { + for (const group of route.catalogGroups) names.add(group); + const prefix = tool.name.replace(/_search_tools$/, ""); + catalogs.push( + `For ${route.catalogGroups.map(escapePromptData).join(", ")}: find the tool with ${tool.name}, load it with ${prefix}_load_tool, then run it with ${prefix}_execute_tool.`, + ); + } + } + if (names.size === 0) { + return "No apps are connected yet. The user connects them under Settings, Integrations."; + } + return [ + `Connected apps: ${[...names].map(escapePromptData).join(", ")}. Use their tools for that app's data and actions (repositories, issues, mail, calendar and similar) before the computer browser or web search. Never tell the user one of these is not connected.`, + ...catalogs, + ].join("\n"); +} + /** * The main bot's role. Tools are named only when this run has them, so the paragraph never * points at a tool the model cannot call. diff --git a/packages/adapters/src/executor.ts b/packages/adapters/src/executor.ts index d116dab..79fb866 100644 --- a/packages/adapters/src/executor.ts +++ b/packages/adapters/src/executor.ts @@ -269,10 +269,13 @@ import { completeEffect, recordEffect, uncertainEffectError } from "./executor-e import { deploymentKeyFor, resolveModelKey, withModelCredentialLock } from "./executor-model.js"; import { formatBotIdentity, + formatConnectionsInstruction, formatMainAgentRole, formatSubagentContext, formatUserContext, + formatWorkingPrinciples, ONBOARDING_OPEN_INSTRUCTION, + ROUTINE_RUN_INSTRUCTION, } from "./executor-prompt.js"; import { isProtectedComputerLifecycleCommandReadingScripts, @@ -3908,10 +3911,10 @@ export function createRunExecutor(deps: ExecutorDeps) { return finish({ error: `unknown tool ${name}` }); }; - const pluginLine = - connectedPlugins.length > 0 - ? `Connected plugins: ${connectedPlugins.map((row) => `${row.displayName} (${row.connectorId}:${row.provider})`).join(", ")}. Prefer those plugin tools over the computer browser or web search when reading app data (repos, releases, mail, calendar, and similar).` - : "No plugins are connected yet."; + const connectionsLine = formatConnectionsInstruction({ + apps: connectedPlugins.map((row) => row.displayName), + tools: exposedConnectorTools, + }); const taughtSkillIndex = savedSkills.slice(0, 20); const taughtSkillsLine = taughtSkillIndex.length > 0 @@ -4078,11 +4081,13 @@ export function createRunExecutor(deps: ExecutorDeps) { prompt, instructions: [ formatBotIdentity(bot), + formatWorkingPrinciples(exposedToolNames), bot.instructions, bot.kind === "main" ? formatMainAgentRole(exposedToolNames, mainAgentProject) : undefined, timeInstruction, + run.trigger === "routine" ? ROUTINE_RUN_INSTRUCTION : undefined, userBlock, personalAgent && userContext.onboardingOpen && @@ -4115,6 +4120,9 @@ export function createRunExecutor(deps: ExecutorDeps) { : undefined, "Never ask for a raw credential in chat or inject it into shell commands.", "Tell the user what you do in plain words; never name your tools to them.", + exposedToolNames.has("ask_user") && exposedToolNames.has("show_card") + ? "When the user has to choose, ask with ask_user; show lists, plans and results with show_card rather than long prose." + : undefined, exposedToolNames.has("remember") ? "Use remember for durable facts." : undefined, exposedToolNames.has("scratchpad_add") ? "Use scratchpad_add / scratchpad_update / scratchpad_complete for open work that should outlive this turn (not reminders — those are schedule_*)." @@ -4125,7 +4133,7 @@ export function createRunExecutor(deps: ExecutorDeps) { "Use destination_write only for connected destination records.", ] .filter(Boolean) - .join(" "), + .join("\n"), productSelfModificationInstruction, workspaceInstruction, agentEnvironmentInstruction, @@ -4151,7 +4159,7 @@ export function createRunExecutor(deps: ExecutorDeps) { exposedToolNames.has("archive_bot") ? "archive_bot safely archives a bot this bot created, and only that bot. Use it when the user asks to remove that bot or when it is finished and unused. The user can restore it or permanently delete it later. confirm_name must exactly match its name." : undefined, - pluginLine, + connectionsLine, agentSkillsLine, taughtSkillsLine, exposedToolNames.has("render_plot") diff --git a/packages/adapters/src/installed-connectors.ts b/packages/adapters/src/installed-connectors.ts index 5607420..7e73d7a 100644 --- a/packages/adapters/src/installed-connectors.ts +++ b/packages/adapters/src/installed-connectors.ts @@ -127,9 +127,9 @@ export class InstalledConnectorProvider implements ConnectorProvider { } private async authorizedTools(context: AdapterContext): Promise { + // A user's tool sources follow them into every space they work in. const installs = await this.prisma.capabilityInstall.findMany({ where: { - spaceId: context.spaceId, userId: context.userId, kind: { in: ["mcp", "api", "graphql"] }, }, @@ -228,7 +228,6 @@ export class InstalledConnectorProvider implements ConnectorProvider { const install = await this.prisma.capabilityInstall.findFirst({ where: { id: installId, - spaceId: context.spaceId, userId: context.userId, kind: { in: ["mcp", "api", "graphql"] }, }, @@ -312,11 +311,7 @@ export class InstalledConnectorProvider implements ConnectorProvider { ): Promise { if (!install.secretId) return undefined; const row = await this.prisma.secret.findFirst({ - where: { - id: install.secretId, - spaceId: context.spaceId, - userId: context.userId, - }, + where: { id: install.secretId, userId: context.userId }, }); return row ? this.secrets.load(row.ciphertext, row.id) : undefined; } diff --git a/packages/adapters/src/lazy-tool-catalog.ts b/packages/adapters/src/lazy-tool-catalog.ts index ed19c2d..abc88ea 100644 --- a/packages/adapters/src/lazy-tool-catalog.ts +++ b/packages/adapters/src/lazy-tool-catalog.ts @@ -74,7 +74,11 @@ export function lazyCatalogTools( }, }, readOnly: true, - route: { connectorId, toolName: CATALOG_SEARCH }, + route: { + connectorId, + toolName: CATALOG_SEARCH, + catalogGroups: [...new Set(entries.map((entry) => entry.group))], + }, }, { name: `${prefix}_load_tool`, diff --git a/packages/adapters/src/product-self-modification.ts b/packages/adapters/src/product-self-modification.ts index 1e96be6..19d3d18 100644 --- a/packages/adapters/src/product-self-modification.ts +++ b/packages/adapters/src/product-self-modification.ts @@ -30,5 +30,5 @@ export function formatProductSelfModificationInstruction( "update_bot only changes this bot's chat profile (name, title, description, look, voice) — not evOo's product UI.", "Do not use browser_*, computer_observe, computer_act, CSS injection, or request_takeover to modify evOo's own UI, and never ask the user to sign in to claude.ai or other third-party services for product changes.", "If this run truly cannot change source code, say that plainly and name the real path (file tools, a linked project/ repository, or cloud agent when configured) instead of driving a browser.", - ].join(" "); + ].join("\n"); } diff --git a/packages/adapters/src/release-watch-guidance.test.ts b/packages/adapters/src/release-watch-guidance.test.ts index 9a98e46..2901762 100644 --- a/packages/adapters/src/release-watch-guidance.test.ts +++ b/packages/adapters/src/release-watch-guidance.test.ts @@ -2,13 +2,13 @@ import { describe, expect, it } from "vitest"; import { builtinAgentTools } from "./builtin-tools.js"; describe("release-watch guidance", () => { - it("tells schedule_create prompts to name plugin tools instead of browsing", () => { + it("tells schedule_create prompts to be self-contained and use connected apps over browsing", () => { const tool = builtinAgentTools.find((entry) => entry.name === "schedule_create"); expect(tool).toBeTruthy(); const prompt = ( tool?.inputSchema as { properties?: { prompt?: { description?: string } } } | undefined )?.properties?.prompt; - expect(prompt?.description ?? "").toContain("GITHUB_LIST_RELEASES"); - expect(prompt?.description ?? "").toMatch(/Prefer plugin tools over computer browser/i); + expect(prompt?.description ?? "").toContain("sees none of this conversation"); + expect(prompt?.description ?? "").toMatch(/Prefer connected apps over the computer browser/i); }); }); diff --git a/packages/adapters/src/skill-tools.test.ts b/packages/adapters/src/skill-tools.test.ts index 90eb395..c233000 100644 --- a/packages/adapters/src/skill-tools.test.ts +++ b/packages/adapters/src/skill-tools.test.ts @@ -54,7 +54,7 @@ function makePrisma(rows: Array> = []) { const index = store.findIndex( (row) => row.id === where.id && - row.spaceId === where.spaceId && + (!where.spaceId || row.spaceId === where.spaceId) && row.userId === where.userId && row.source === where.source, ); @@ -73,7 +73,7 @@ function makePrisma(rows: Array> = []) { const index = store.findIndex( (row) => row.id === where.id && - row.spaceId === where.spaceId && + (!where.spaceId || row.spaceId === where.spaceId) && row.userId === where.userId && row.source === where.source, ); @@ -163,7 +163,7 @@ describe("skill tools", () => { expect(renamed).toMatchObject({ ok: true, name: "Renamed" }); }); - it("scopes list to the owner workspace and injects catalog lines", async () => { + it("keeps other users' skills out and injects catalog lines", async () => { prisma = makePrisma([ { id: "other-1", @@ -351,4 +351,53 @@ describe("skill tools", () => { }), ).toEqual({ error: "Skill name must be at most 80 characters." }); }); + it("follows the user into every space, preferring the current space on a name clash", async () => { + prisma = makePrisma([ + { + id: "home-1", + spaceId: "ws-2", + userId: owner.userId, + name: "Weekly review", + description: "From home", + content: buildSkillMd({ name: "Weekly review", description: "From home", body: "a" }), + source: "user", + }, + { + id: "here-1", + spaceId: owner.spaceId, + userId: owner.userId, + name: "weekly review", + description: "From this project", + content: buildSkillMd({ + name: "weekly review", + description: "From this project", + body: "b", + }), + source: "user", + }, + { + id: "home-2", + spaceId: "ws-2", + userId: owner.userId, + name: "Launch", + description: "Ship it", + content: buildSkillMd({ name: "Launch", description: "Ship it", body: "c" }), + source: "user", + }, + ]); + const skills = await listAgentSkillRecords(prisma as never, owner); + const own = skills.filter((skill) => skill.source === "user"); + expect(own.map((skill) => skill.id).sort()).toEqual(["here-1", "home-2"]); + expect( + await skillCreateFromTool(prisma as never, owner, { name: "Launch", description: "x" }), + ).toEqual({ + error: 'A skill named "Launch" already exists.', + }); + expect( + await skillUpdateFromTool(prisma as never, owner, { + name: "Launch", + description: "Ship it now", + }), + ).toMatchObject({ ok: true }); + }); }); diff --git a/packages/adapters/src/skill-tools.ts b/packages/adapters/src/skill-tools.ts index a7f9b87..a42e28a 100644 --- a/packages/adapters/src/skill-tools.ts +++ b/packages/adapters/src/skill-tools.ts @@ -4,6 +4,7 @@ import { isSkillReadOnly, mergeBuiltinSkills, parseSkillMd, + preferSpaceSkills, type SkillRecord, type SkillSource, } from "@rakazo/core"; @@ -32,6 +33,7 @@ type SkillOwner = { type AgentSkillRow = { id: string; + spaceId: string; name: string; description: string; content: string; @@ -90,10 +92,10 @@ export async function listAgentSkillRecords( owner: SkillOwner, ): Promise> { const rows = await prisma.agentSkill.findMany({ - where: { spaceId: owner.spaceId, userId: owner.userId }, + where: { userId: owner.userId }, orderBy: [{ name: "asc" }, { id: "asc" }], }); - return mergeBuiltinSkills(builtinRecords(), rows.map(toRecord)); + return mergeBuiltinSkills(builtinRecords(), preferSpaceSkills(rows, owner.spaceId).map(toRecord)); } async function findOwnedSkill( @@ -246,12 +248,7 @@ export async function skillUpdateFromTool( try { // Re-scope mutate to owner + user source so a stale id cannot cross tenants. const updated = await prisma.agentSkill.updateMany({ - where: { - id: existing.id, - spaceId: owner.spaceId, - userId: owner.userId, - source: "user", - }, + where: { id: existing.id, userId: owner.userId, source: "user" }, data: { name: nextName, description: nextDescription, @@ -276,12 +273,7 @@ export async function skillDeleteFromTool( return { error: "Builtin and plugin skills are read-only." }; } const deleted = await prisma.agentSkill.deleteMany({ - where: { - id: existing.id, - spaceId: owner.spaceId, - userId: owner.userId, - source: "user", - }, + where: { id: existing.id, userId: owner.userId, source: "user" }, }); if (deleted.count !== 1) return { error: "Could not delete skill." }; return { ok: true, name: existing.name }; diff --git a/packages/core/src/agent-skill.test.ts b/packages/core/src/agent-skill.test.ts index efed618..59bee87 100644 --- a/packages/core/src/agent-skill.test.ts +++ b/packages/core/src/agent-skill.test.ts @@ -286,6 +286,22 @@ describe("skill prompt helpers", () => { ]); expect(line).toContain("- Daily standup: Prepare standup notes"); expect(line).toContain("skill_read"); - expect(line).toContain("Prefer matching skills"); + expect(line).toContain("instead of improvising"); + expect(line).toContain("skill_install with the link"); + }); + + it("clips long descriptions and counts skills past the catalog limit", () => { + const entries = Array.from({ length: 82 }, (_, index) => ({ + name: `Skill ${index}`, + description: `${"word ".repeat(100)}end`, + source: "user" as const, + readOnly: false, + })); + const catalog = formatSkillsCatalogInstruction(entries) ?? ""; + expect(catalog).toContain("- Skill 79: word"); + expect(catalog).not.toContain("- Skill 80:"); + expect(catalog).toContain("- and 2 more the user can start with /Name"); + expect(catalog).not.toContain("end\n"); + expect(catalog.length).toBeLessThan(82 * 260); }); }); diff --git a/packages/core/src/agent-skill.ts b/packages/core/src/agent-skill.ts index 43ef981..5869e6e 100644 --- a/packages/core/src/agent-skill.ts +++ b/packages/core/src/agent-skill.ts @@ -117,20 +117,32 @@ export function buildSkillMd(input: { return `---\n${lines.join("\n")}\n---\n${body.startsWith("\n") ? body : `\n${body}`}`; } +/** Installed skill packs bring long descriptions; the catalog only needs enough to match. */ +const CATALOG_DESCRIPTION_MAX_CHARS = 240; +const CATALOG_MAX_SKILLS = 80; + export function skillCatalogLine(entry: Pick): string { - return `- ${entry.name}: ${entry.description}`; + const description = entry.description.replace(/\s+/g, " ").trim(); + const clipped = + description.length > CATALOG_DESCRIPTION_MAX_CHARS + ? `${description.slice(0, CATALOG_DESCRIPTION_MAX_CHARS - 1).trimEnd()}…` + : description; + return `- ${entry.name}: ${clipped}`; } export function formatSkillsCatalogInstruction(entries: SkillCatalogEntry[]): string | undefined { if (entries.length === 0) return undefined; - const lines = entries.slice(0, 50).map(skillCatalogLine).join("\n"); + const lines = entries.slice(0, CATALOG_MAX_SKILLS).map(skillCatalogLine); + const hidden = entries.length - lines.length; + if (hidden > 0) lines.push(`- and ${hidden} more the user can start with /Name`); return [ - "Available Claude Agent Skills (SKILL.md recipes shared across assistants; generic how-tos, not account-specific routines). The Pi runtime already understands this format; we persist and inject them:", - lines, - "When a skill matches the user's request, call skill_read for that name and follow it immediately. Prefer matching skills over improvising multi-step recipes.", - "Users can force a skill with /Name in the composer. Routines may mention a skill as @Name — that loads the skill at fire time.", - "Create a skill with skill_create when a multi-step task is worth repeating (or when asked). After creating one, mention /Name so the user can open it.", - "Only skill_update / skill_delete user-created skills (not builtin or plugin).", + "Skills are step-by-step recipes (SKILL.md) for recurring kinds of work. Available skills:", + lines.join("\n"), + "When a skill matches the request, load it with skill_read and follow it right away instead of improvising.", + "The user starts a skill with /Name in the composer; a routine can name one as @Name.", + "When the user shares a GitHub link to skills or asks to install skills from one, call skill_install with the link once per link. It installs every SKILL.md behind the link. Then tell the user the new /Names; if some were skipped, fix the cause (for example a clashing name) and create them with skill_create.", + "Save a multi-step task worth repeating as a skill with skill_create when asked or when clearly useful, then mention its /Name.", + "Only change or delete the user's own skills, with skill_update or skill_delete; builtin skills are read-only.", ].join("\n"); } @@ -212,7 +224,15 @@ export function mergeBuiltinSkills( builtins: readonly T[], userSkills: readonly T[], ): T[] { - return [...builtins.filter((skill) => !findSkillByName(userSkills, skill.name)), ...userSkills]; + const unique = userSkills.filter( + (skill, index) => findSkillByName(userSkills, skill.name) === userSkills[index], + ); + return [...builtins.filter((skill) => !findSkillByName(unique, skill.name)), ...unique]; +} + +/** A user's skills follow them into every space; rows from `spaceId` win a name clash. */ +export function preferSpaceSkills(rows: T[], spaceId: string): T[] { + return [...rows].sort((a, b) => Number(b.spaceId === spaceId) - Number(a.spaceId === spaceId)); } export function findSkillByName(