diff --git a/CHANGELOG.md b/CHANGELOG.md index 233826fed..dd90db074 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - [EE] Added a `list_branches` tool to the MCP server and Ask Sourcebot for discovering repository branches and whether they are indexed. [#1609](https://github.com/sourcebot-dev/sourcebot/pull/1609) +- [EE] Added `create_skill`, `update_skill`, and `list_skills` tools to the MCP server and Ask Sourcebot for managing skills. [#1612](https://github.com/sourcebot-dev/sourcebot/pull/1612) ### Changed - Migrated connection syncing, repository indexing, permission syncing, and background pruning from in-process managers and pollers to BullMQ workloads with retries and per-resource execution locking. [#1427](https://github.com/sourcebot-dev/sourcebot/pull/1427) diff --git a/docs/docs/features/mcp-server.mdx b/docs/docs/features/mcp-server.mdx index 2b307bfc4..890175eb6 100644 --- a/docs/docs/features/mcp-server.mdx +++ b/docs/docs/features/mcp-server.mdx @@ -452,3 +452,44 @@ Parameters: | `repos` | no | The repositories that are accessible to the agent during the chat. If not provided, all repositories are accessible. | | `languageModel` | no | The language model to use for answering the question. Object with `provider` and `model`. If not provided, defaults to the first model in the config. Use `list_language_models` to see available options. | | `visibility` | no | The visibility of the chat session (`'PRIVATE'` or `'PUBLIC'`). Defaults to `PRIVATE` for authenticated users and `PUBLIC` for anonymous users. Set to `PUBLIC` to make the chat viewable by anyone with the link (useful in shared environments like Slack). | + +### `create_skill` + +You can create an agent skill: a reusable set of instructions you invoke in Ask Sourcebot as a `/` slash command, or that the agent loads automatically when your request matches its description. The skill is personal to you and enabled immediately. The result includes a link to the skill in **Settings → Skills**. + +You need an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions and repository-scoped access tokens cannot use this tool. + +Parameters: +| Name | Required | Description | +|:---------------|:---------|:------------| +| `name` | yes | Display name for the skill (1-80 characters). | +| `slug` | yes | Slash command for the skill, without the leading `/`. Lowercase letters, numbers, and hyphens; at most 64 characters. Must be unique among the user's skills. | +| `description` | yes | When to use the skill (1-500 characters). Used by the agent to auto-load the skill. | +| `instructions` | yes | Markdown instructions the agent follows when the skill is invoked (1-20,000 characters). | + +### `update_skill` + +You can edit an existing skill in place. Fields you omit keep their current values. You can edit your personal skills, and shared skills only if you created them and they are enabled. Skills synced from a repository file are rejected; edit those in **Settings → Skills**. This tool never enables or disables a skill, and never moves it between the personal and shared catalogs. + +You need an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions and repository-scoped access tokens cannot use this tool. + +Parameters: +| Name | Required | Description | +|:---------------|:---------|:------------| +| `slug` | yes | Current slash command of the skill to update, without the leading `/`. | +| `scope` | yes | Which catalog the skill lives in: `personal` or `shared`. | +| `name` | no | New display name (1-80 characters). | +| `newSlug` | no | New slash command, without the leading `/`. Lowercase letters, numbers, and hyphens; at most 64 characters. | +| `description` | no | New description of when to use the skill (1-500 characters). | +| `instructions` | no | New markdown instructions (1-20,000 characters). | + +### `list_skills` + +You can list the skills visible to you: your personal skills plus your organization's shared catalog. Each row includes `slug` and `scope` (the pair `update_skill` needs), `enabled`, `isSynced` (linked to a repository file), `canEdit` (whether `update_skill` can edit it), and, on shared rows, `adopted`. Skill instructions are never included. + +You need an authenticated user (API key or OAuth) and the Ask Sourcebot feature. Anonymous sessions and repository-scoped access tokens cannot use this tool. + +Parameters: +| Name | Required | Description | +|:--------|:---------|:------------| +| `scope` | no | Filter to one catalog: `personal` or `shared`. Omit to list both. | diff --git a/packages/web/src/app/api/(server)/ee/mcp/route.ts b/packages/web/src/app/api/(server)/ee/mcp/route.ts index b9388ff6a..d82be950a 100644 --- a/packages/web/src/app/api/(server)/ee/mcp/route.ts +++ b/packages/web/src/app/api/(server)/ee/mcp/route.ts @@ -75,7 +75,7 @@ export const POST = apiHandler(async (request: NextRequest) => { } const response = await sew(() => - withOptionalAuth(async ({ user }) => { + withOptionalAuth(async ({ user, principal }) => { if (env.EXPERIMENT_ASK_GH_ENABLED === 'true' && !user) { return notAuthenticated(); } @@ -111,7 +111,13 @@ export const POST = apiHandler(async (request: NextRequest) => { }, }); - const mcpServer = await createMcpServer(); + // Repository-scoped access tokens never get the skill management + // tools: their documented authorization boundary is the selected + // repositories only. The tools also reject scoped principals + // per-request, since a session is keyed by owner, not principal. + const mcpServer = await createMcpServer({ + canManageSkills: ownerId !== null && principal?.source !== 'scoped_access_token', + }); await mcpServer.connect(transport); return transport.handleRequest(request); diff --git a/packages/web/src/ee/features/chat/agent.test.ts b/packages/web/src/ee/features/chat/agent.test.ts index 8cfa87c3b..fa2c23763 100644 --- a/packages/web/src/ee/features/chat/agent.test.ts +++ b/packages/web/src/ee/features/chat/agent.test.ts @@ -89,6 +89,9 @@ vi.mock('@/features/tools', () => { listReposDefinition: createToolDefinition('list_repos'), listTreeDefinition: createToolDefinition('list_tree'), readFileDefinition: createToolDefinition('read_file'), + createSkillDefinition: createToolDefinition('create_skill'), + updateSkillDefinition: createToolDefinition('update_skill'), + listSkillsDefinition: createToolDefinition('list_skills'), toVercelAITool: vi.fn((definition: { name: string }) => ({ name: definition.name, })), @@ -273,6 +276,29 @@ describe('createMessageStream built-in tools', () => { expect(tools).toHaveProperty('list_branches'); expect(activeTools).toContain('list_branches'); }); + + test('makes the skill management tools available to authenticated, interactive requesters', async () => { + const { tools, activeTools } = await runCreateMessageStream([createUserMessage()], { + userId: 'user-1', + orgId: 1, + }); + + for (const toolName of ['create_skill', 'update_skill', 'list_skills']) { + expect(tools).toHaveProperty(toolName); + expect(activeTools).toContain(toolName); + } + }); + + test('omits the skill management tools when the requester is anonymous or programmatic', async () => { + const { tools, activeTools } = await runCreateMessageStream([createUserMessage()]); + + for (const toolName of ['create_skill', 'update_skill', 'list_skills']) { + expect(tools).not.toHaveProperty(toolName); + expect(activeTools).not.toContain(toolName); + } + // The other built-ins are unaffected by the gate. + expect(tools).toHaveProperty('list_branches'); + }); }); describe('createMessageStream approval continuation', () => { diff --git a/packages/web/src/ee/features/chat/agent.ts b/packages/web/src/ee/features/chat/agent.ts index eccdf389e..e69f368cf 100644 --- a/packages/web/src/ee/features/chat/agent.ts +++ b/packages/web/src/ee/features/chat/agent.ts @@ -24,6 +24,7 @@ import { ANSWER_TAG, FILE_REFERENCE_PREFIX } from "@/features/chat/constants"; import { Source } from "@/features/chat/types"; import { addLineNumbers, fileReferenceToString, formatAttachmentsForPrompt, getAnswerPartFromAssistantMessage, getTurnProgressState, getUserMessageAttachments, getUserMessageText } from "@/features/chat/utils"; import { createTools } from "./tools"; +import { createSkillDefinition, listSkillsDefinition, updateSkillDefinition } from "@/features/tools"; import { getConnectedMcpClients } from "@/ee/features/chat/mcp/mcpClientFactory"; import { getMcpTools, McpToolsResult } from "@/ee/features/chat/mcp/mcpToolSets"; import { @@ -692,7 +693,22 @@ const createAgentStream = async ({ skillRegistry, }); - const builtinTools = createTools({ source: 'sourcebot-ask-agent', selectedRepos: sortedRepos }); + const allBuiltinTools = createTools({ source: 'sourcebot-ask-agent', selectedRepos: sortedRepos }); + + // The skill management tools require an interactive, authenticated + // requester: same gate as load_skill. This also excludes programmatic runs + // (askCodebase, /api/chat/blocking) where nobody can answer an approval + // request. + const skillManagementToolNames: string[] = [ + createSkillDefinition.name, + updateSkillDefinition.name, + listSkillsDefinition.name, + ]; + const builtinTools: Record = (userId !== undefined && orgId !== undefined) + ? allBuiltinTools + : Object.fromEntries( + Object.entries(allBuiltinTools).filter(([name]) => !skillManagementToolNames.includes(name)), + ); const builtinToolNames = Object.keys(builtinTools); const allTools: Record = { ...builtinTools, diff --git a/packages/web/src/ee/features/chat/askCommandsContext.tsx b/packages/web/src/ee/features/chat/askCommandsContext.tsx new file mode 100644 index 000000000..5535c21fd --- /dev/null +++ b/packages/web/src/ee/features/chat/askCommandsContext.tsx @@ -0,0 +1,11 @@ +'use client'; + +import { createContext, useContext } from 'react'; +import type { AskCommandDefinition } from '@/features/chat/commands/types'; + +// The chat page's server-rendered slash-command catalog, made available to +// nested chat components (e.g. the tool approval banner resolves a skill's +// display name from its slug + scope). +export const AskCommandsContext = createContext([]); + +export const useAskCommands = () => useContext(AskCommandsContext); diff --git a/packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx b/packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx index f740f0eff..08bce6bc7 100644 --- a/packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx +++ b/packages/web/src/ee/features/chat/components/chatThread/chatThread.tsx @@ -33,6 +33,7 @@ import { McpReconnectContext } from '../../mcpReconnectContext'; import { McpAuthRequiredData, McpServerLoadFailureData, useMcpReconnectController } from './useMcpReconnectController'; import { McpReconnectBanner } from './mcpReconnectBanner'; import { ToolApprovalProvider } from '../../toolApprovalContext'; +import { AskCommandsContext } from '../../askCommandsContext'; import useCaptureEvent from '@/hooks/useCaptureEvent'; import { SignInPromptBanner } from './signInPromptBanner'; import { DuplicateChatDialog } from '@/app/(app)/chat/components/duplicateChatDialog'; @@ -459,6 +460,7 @@ export const ChatThread = ({ return ( + @@ -615,6 +617,7 @@ export const ChatThread = ({ + ); } diff --git a/packages/web/src/ee/features/chat/components/chatThread/detailsCard.test.tsx b/packages/web/src/ee/features/chat/components/chatThread/detailsCard.test.tsx index 69b67460a..ce76d1e06 100644 --- a/packages/web/src/ee/features/chat/components/chatThread/detailsCard.test.tsx +++ b/packages/web/src/ee/features/chat/components/chatThread/detailsCard.test.tsx @@ -194,6 +194,190 @@ describe('DetailsCard', () => { expect(screen.queryByText('Look for correctness issues first.')).toBeNull(); }); + test('renders a create_skill success summary with the name and command', () => { + const createSkillPart = { + type: 'tool-create_skill', + toolCallId: 'tool-call-create-skill', + state: 'output-available', + input: { + name: 'Review PR', + slug: 'review-pr', + description: 'Review a pull request.', + instructions: 'Look for correctness issues first.', + }, + output: { + output: '{}', + metadata: { + id: 'skill-1', + slug: 'review-pr', + name: 'Review PR', + url: 'https://sourcebot.example.com/settings/skills?skill=skill-1', + }, + }, + } satisfies SBChatMessagePart; + + const { container } = render( + + + + ); + + expect(container.textContent).toContain('Created skill'); + expect(container.textContent).toContain('Review PR'); + expect(container.textContent).toContain('/review-pr'); + expect(screen.queryByText('Creating skill...')).toBeNull(); + }); + + test('renders an update_skill success summary with the name and command', () => { + const updateSkillPart = { + type: 'tool-update_skill', + toolCallId: 'tool-call-update-skill', + state: 'output-available', + input: { + slug: 'review-pr', + scope: 'shared', + name: 'Review PR v2', + }, + output: { + output: '{}', + metadata: { + id: 'skill-1', + slug: 'review-pr', + name: 'Review PR v2', + scope: 'shared', + url: 'https://sourcebot.example.com/settings/skills?skill=skill-1', + }, + }, + } satisfies SBChatMessagePart; + + const { container } = render( + + + + ); + + expect(container.textContent).toContain('Updated shared skill'); + expect(container.textContent).toContain('Review PR v2'); + expect(container.textContent).toContain('/review-pr'); + expect(screen.queryByText('Updating skill...')).toBeNull(); + }); + + test('renders a list_skills count summary', () => { + const listSkillsPart = { + type: 'tool-list_skills', + toolCallId: 'tool-call-list-skills', + state: 'output-available', + input: {}, + output: { + output: '{"skills":[]}', + metadata: { count: 3 }, + }, + } satisfies SBChatMessagePart; + + render( + + + + ); + + expect(screen.queryByText('Listed skills')).toBeTruthy(); + expect(screen.queryByText('3 skills')).toBeTruthy(); + }); + + test('shows a non-pulsing waiting state for an approval-requested tool call', () => { + const approvalRequestedPart = { + type: 'tool-create_skill', + toolCallId: 'tool-call-approval', + state: 'approval-requested', + input: { + name: 'Review PR', + slug: 'review-pr', + description: 'Review a pull request.', + instructions: 'Look for correctness issues first.', + }, + approval: { id: 'approval-1' }, + } satisfies SBChatMessagePart; + + render( + + + + ); + + expect(screen.queryByText('Waiting for approval')).toBeTruthy(); + expect(screen.queryByText('Creating skill...')).toBeNull(); + }); + + test('shows a denied label for an output-denied tool call', () => { + const deniedPart = { + type: 'tool-create_skill', + toolCallId: 'tool-call-denied', + title: 'Create skill', + state: 'output-denied', + input: { + name: 'Review PR', + slug: 'review-pr', + description: 'Review a pull request.', + instructions: 'Look for correctness issues first.', + }, + approval: { id: 'approval-1', approved: false, reason: 'User denied' }, + } satisfies SBChatMessagePart; + + render( + + + + ); + + expect(screen.queryByText('Create skill denied')).toBeTruthy(); + expect(screen.queryByText('Creating skill...')).toBeNull(); + }); + test('renders an unavailable skill load instead of a silent no-op', () => { const unavailableSkillPart = { type: 'tool-load_skill', diff --git a/packages/web/src/ee/features/chat/components/chatThread/detailsCard.tsx b/packages/web/src/ee/features/chat/components/chatThread/detailsCard.tsx index 4cfc17633..cbd2803ec 100644 --- a/packages/web/src/ee/features/chat/components/chatThread/detailsCard.tsx +++ b/packages/web/src/ee/features/chat/components/chatThread/detailsCard.tsx @@ -29,6 +29,9 @@ import { ToolOutputGuard } from './tools/toolOutputGuard'; import { McpToolComponent } from './tools/mcpToolComponent'; import { ToolSearchToolComponent } from './tools/toolSearchToolComponent'; import { LoadSkillToolComponent } from './tools/loadSkillToolComponent'; +import { CreateSkillToolComponent } from './tools/createSkillToolComponent'; +import { UpdateSkillToolComponent } from './tools/updateSkillToolComponent'; +import { ListSkillsToolComponent } from './tools/listSkillsToolComponent'; // A UI-visible step: the parts of one LLM invocation, tagged with the @@ -463,7 +466,10 @@ type GuardedToolType = | 'tool-list_branches' | 'tool-list_commits' | 'tool-get_diff' - | 'tool-list_tree'; + | 'tool-list_tree' + | 'tool-create_skill' + | 'tool-update_skill' + | 'tool-list_skills'; type GuardedToolPart = Extract; @@ -481,6 +487,9 @@ const TOOL_GUARD_CONFIG = { 'tool-list_commits': { loadingText: 'Listing commits...', render: (output) => }, 'tool-get_diff': { loadingText: 'Comparing revisions...', render: (output) => }, 'tool-list_tree': { loadingText: 'Listing tree...', render: (output) => }, + 'tool-create_skill': { loadingText: 'Creating skill...', render: (output) => }, + 'tool-update_skill': { loadingText: 'Updating skill...', render: (output) => }, + 'tool-list_skills': { loadingText: 'Listing skills...', render: (output) => }, } satisfies { [K in GuardedToolType]: { loadingText: string; @@ -509,7 +518,10 @@ export const StepPartRenderer = ({ part, toolTokenUsageMap }: { part: SBChatMess case 'tool-list_branches': case 'tool-list_commits': case 'tool-get_diff': - case 'tool-list_tree': { + case 'tool-list_tree': + case 'tool-create_skill': + case 'tool-update_skill': + case 'tool-list_skills': { const { loadingText, render } = TOOL_GUARD_CONFIG[part.type]; return ( { + cleanup(); +}); + +const askCommands: AskCommandDefinition[] = [ + { + id: 'skill-1', + sourceId: ASK_COMMAND_SOURCE_PERSONAL_SKILL, + sourceLabel: 'Personal', + slug: 'review-pr', + name: 'Review PR', + description: 'Review a pull request.', + isSynced: false, + }, + { + id: 'skill-2', + sourceId: ASK_COMMAND_SOURCE_SHARED_SKILL, + sourceLabel: 'Shared', + slug: 'audit', + name: 'Audit Billing', + description: 'Audit billing issues.', + isSynced: false, + }, +]; + +const renderBanner = (parts: ApprovalRequestedToolPart[]) => + render( + + + + + + ); + +describe('ToolApprovalBanner', () => { + test('headlines the skill name from the input for create_skill', () => { + const { container } = renderBanner([ + { + type: 'tool-create_skill', + toolCallId: 'tool-call-1', + state: 'approval-requested', + input: { + name: 'Release Checklist', + slug: 'release-checklist', + description: 'Run the release checklist.', + instructions: 'Check the changelog first.', + }, + approval: { id: 'approval-1' }, + }, + ]); + + expect(container.textContent).toContain('Agent wants to create skill'); + expect(container.textContent).toContain('Release Checklist'); + expect(container.textContent).not.toContain('Agent wants to use'); + }); + + test('resolves the update_skill display name from askCommands', () => { + const { container } = renderBanner([ + { + type: 'tool-update_skill', + toolCallId: 'tool-call-2', + state: 'approval-requested', + input: { slug: 'review-pr', scope: 'personal', name: 'Renamed' }, + approval: { id: 'approval-2' }, + }, + ]); + + expect(container.textContent).toContain('Agent wants to update your skill'); + expect(container.textContent).toContain('Review PR'); + }); + + test('labels shared skills and falls back to /slug when no command matches', () => { + const { container } = renderBanner([ + { + type: 'tool-update_skill', + toolCallId: 'tool-call-3', + state: 'approval-requested', + input: { slug: 'unadopted', scope: 'shared', description: 'New description.' }, + approval: { id: 'approval-3' }, + }, + ]); + + expect(container.textContent).toContain('Agent wants to update shared skill'); + expect(container.textContent).toContain('/unadopted'); + }); + + test('keeps the generic line for tools without a summary renderer', () => { + const { container } = renderBanner([ + { + type: 'dynamic-tool', + toolName: 'mcp_linear__save_issue', + toolCallId: 'tool-call-4', + state: 'approval-requested', + input: { title: 'Issue' }, + approval: { id: 'approval-4' }, + }, + ]); + + expect(container.textContent).toContain('Agent wants to use'); + }); +}); diff --git a/packages/web/src/ee/features/chat/components/chatThread/toolApprovalBanner.tsx b/packages/web/src/ee/features/chat/components/chatThread/toolApprovalBanner.tsx index 792ba3fe9..0f1b5a027 100644 --- a/packages/web/src/ee/features/chat/components/chatThread/toolApprovalBanner.tsx +++ b/packages/web/src/ee/features/chat/components/chatThread/toolApprovalBanner.tsx @@ -1,14 +1,16 @@ 'use client'; import { Button } from "@/components/ui/button"; +import { useAskCommands } from "@/ee/features/chat/askCommandsContext"; import { McpFavicon } from "@/ee/features/chat/mcp/components/mcpFavicon"; import { McpToolNameMap, useMcpServerIconMap, useMcpToolNameMap } from "@/ee/features/chat/mcpDisplayMetadataContext"; import { useToolApproval } from "@/ee/features/chat/toolApprovalContext"; +import { ASK_COMMAND_SOURCE_PERSONAL_SKILL, ASK_COMMAND_SOURCE_SHARED_SKILL, type AskCommandDefinition } from "@/features/chat/commands/types"; import { SBChatToolPart } from "@/features/chat/utils"; import { cn } from "@/lib/utils"; import { getToolName } from "ai"; import { ChevronRight } from "lucide-react"; -import { useCallback, useState } from "react"; +import { ReactNode, useCallback, useState } from "react"; import { getMcpToolDisplayParts } from "./tools/mcpToolComponent"; import { JsonHighlighter } from "./tools/jsonHighlighter"; @@ -24,6 +26,7 @@ export const ToolApprovalBanner = ({ parts }: ToolApprovalBannerProps) => { const addToolApprovalResponse = useToolApproval(); const iconMap = useMcpServerIconMap(); const rawToolNames = useMcpToolNameMap(); + const askCommands = useAskCommands(); if (parts.length === 0) { return null; @@ -38,27 +41,64 @@ export const ToolApprovalBanner = ({ parts }: ToolApprovalBannerProps) => { addToolApprovalResponse={addToolApprovalResponse} iconMap={iconMap} rawToolNames={rawToolNames} + askCommands={askCommands} /> ))} ); }; +// Per-tool approval summaries: built-in tools whose approval line should read +// as an action over the tool's input rather than the generic "wants to use +// {tool}" line. Tools without an entry fall back to the generic line. +const getBuiltinApprovalSummary = ( + part: ApprovalRequestedToolPart, + askCommands: AskCommandDefinition[], +): ReactNode | undefined => { + switch (part.type) { + case 'tool-create_skill': + return ( + <> + Agent wants to create skill {part.input.name} + + ); + case 'tool-update_skill': { + const { slug, scope } = part.input; + // Resolve the skill's display name from the chat page's command + // catalog. Falls back to /slug when no match exists (e.g. a + // creator-owned shared skill the user has not adopted). + const sourceId = scope === 'shared' ? ASK_COMMAND_SOURCE_SHARED_SKILL : ASK_COMMAND_SOURCE_PERSONAL_SKILL; + const command = askCommands.find((candidate) => candidate.sourceId === sourceId && candidate.slug === slug); + return ( + <> + Agent wants to update {scope === 'shared' ? 'shared' : 'your'} skill{' '} + {command?.name ?? `/${slug}`} + + ); + } + default: + return undefined; + } +}; + const ToolApprovalItem = ({ part, addToolApprovalResponse, iconMap, rawToolNames, + askCommands, }: { part: ApprovalRequestedToolPart; addToolApprovalResponse: ReturnType; iconMap: Record; rawToolNames: McpToolNameMap; + askCommands: AskCommandDefinition[]; }) => { const [isExpanded, setIsExpanded] = useState(false); const partToolName = getToolName(part); const display = getMcpToolDisplayParts(partToolName, rawToolNames); const faviconUrl = display.serverName ? iconMap[display.serverName] : undefined; + const builtinSummary = getBuiltinApprovalSummary(part, askCommands); const requestText = JSON.stringify(part.input, null, 2); @@ -85,7 +125,9 @@ const ToolApprovalItem = ({ > - {display.serverName ? ( + {builtinSummary ? ( + builtinSummary + ) : display.serverName ? ( <> Agent wants to use {display.toolName} from {display.serverName} diff --git a/packages/web/src/ee/features/chat/components/chatThread/tools/createSkillToolComponent.tsx b/packages/web/src/ee/features/chat/components/chatThread/tools/createSkillToolComponent.tsx new file mode 100644 index 000000000..d4b332f15 --- /dev/null +++ b/packages/web/src/ee/features/chat/components/chatThread/tools/createSkillToolComponent.tsx @@ -0,0 +1,23 @@ +'use client'; + +import Link from 'next/link'; +import { Sparkles } from 'lucide-react'; +import { CreateSkillMetadata, ToolResult } from '@/features/tools'; + +export const CreateSkillToolComponent = ({ metadata }: ToolResult) => { + return ( +
+ + Created skill + + {metadata.name} + + /{metadata.slug} + +
+ ); +}; diff --git a/packages/web/src/ee/features/chat/components/chatThread/tools/listSkillsToolComponent.tsx b/packages/web/src/ee/features/chat/components/chatThread/tools/listSkillsToolComponent.tsx new file mode 100644 index 000000000..28455f66c --- /dev/null +++ b/packages/web/src/ee/features/chat/components/chatThread/tools/listSkillsToolComponent.tsx @@ -0,0 +1,17 @@ +'use client'; + +import { Separator } from '@/components/ui/separator'; +import { ListSkillsMetadata, ToolResult } from '@/features/tools'; + +export const ListSkillsToolComponent = ({ metadata }: ToolResult) => { + const label = `${metadata.count} ${metadata.count === 1 ? 'skill' : 'skills'}`; + + return ( +
+ Listed skills + + {label} + +
+ ); +}; diff --git a/packages/web/src/ee/features/chat/components/chatThread/tools/toolOutputGuard.tsx b/packages/web/src/ee/features/chat/components/chatThread/tools/toolOutputGuard.tsx index ec7d8c175..3e7d1667f 100644 --- a/packages/web/src/ee/features/chat/components/chatThread/tools/toolOutputGuard.tsx +++ b/packages/web/src/ee/features/chat/components/chatThread/tools/toolOutputGuard.tsx @@ -62,6 +62,14 @@ export const ToolOutputGuard = {part.title!} failed with error: {part.errorText} + ) : part.state === 'approval-requested' ? ( + + Waiting for approval + + ) : part.state === 'output-denied' ? ( + + {part.title ?? 'Tool call'} denied + ) : part.state !== 'output-available' ? ( {loadingText} diff --git a/packages/web/src/ee/features/chat/components/chatThread/tools/updateSkillToolComponent.tsx b/packages/web/src/ee/features/chat/components/chatThread/tools/updateSkillToolComponent.tsx new file mode 100644 index 000000000..bd17f0619 --- /dev/null +++ b/packages/web/src/ee/features/chat/components/chatThread/tools/updateSkillToolComponent.tsx @@ -0,0 +1,23 @@ +'use client'; + +import Link from 'next/link'; +import { Sparkles } from 'lucide-react'; +import { ToolResult, UpdateSkillMetadata } from '@/features/tools'; + +export const UpdateSkillToolComponent = ({ metadata }: ToolResult) => { + return ( +
+ + Updated {metadata.scope === 'shared' ? 'shared skill' : 'skill'} + + {metadata.name} + + /{metadata.slug} + +
+ ); +}; diff --git a/packages/web/src/ee/features/chat/skills/actions.ts b/packages/web/src/ee/features/chat/skills/actions.ts index 8d7563291..2cc15004a 100644 --- a/packages/web/src/ee/features/chat/skills/actions.ts +++ b/packages/web/src/ee/features/chat/skills/actions.ts @@ -7,10 +7,8 @@ import { ErrorCode } from "@/lib/errorCodes"; import { captureEvent } from "@/lib/posthog"; import type { AskSkillActorRelationship, - AskSkillChangedField, AskSkillCreationMethod, AskSkillEntryPoint, - PosthogEventMap, } from "@/lib/posthogEvents"; import { isRecordNotFoundError, isUniqueConstraintError } from "@/lib/prismaErrors"; import { requestBodySchemaValidationError, unexpectedError, ServiceError } from "@/lib/serviceError"; @@ -49,24 +47,16 @@ import { } from "./commandCatalog"; import { canAccessSkillSource, filterSkillsBySourceRepoAccess } from "./sourceRepoAccess"; import { hashSkillId, normalizeSkillAnalyticsEntryPoint } from "./skillAnalytics"; - -const skillAlreadyExists = (slug: string): ServiceError => ({ - statusCode: StatusCodes.CONFLICT, - errorCode: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, - message: `A skill with command /${slug} already exists.`, -}); - -const skillNotFound = (): ServiceError => ({ - statusCode: StatusCodes.NOT_FOUND, - errorCode: ErrorCode.AGENT_SKILL_NOT_FOUND, - message: "Skill not found.", -}); - -const insufficientSkillPermissions = (): ServiceError => ({ - statusCode: StatusCodes.FORBIDDEN, - errorCode: ErrorCode.INSUFFICIENT_PERMISSIONS, - message: "You do not have sufficient permissions to manage this skill.", -}); +import { + canManageSharedSkill, + createPersonalAgentSkillForContext, + emitSkillEvent, + insufficientSkillPermissions, + skillAlreadyExists, + skillNotFound, + updateAgentSkillForContext, + type SkillEventBase, +} from "./skillCreation"; const skillNotSynced = (): ServiceError => ({ statusCode: StatusCodes.BAD_REQUEST, @@ -93,28 +83,6 @@ type SkillAnalyticsContext = { const SKILL_ANALYTICS_SOURCE = 'sourcebot-web-client' as const; -type SkillOutcomeEventName = { - [EventName in keyof PosthogEventMap]: PosthogEventMap[EventName] extends { success: boolean } - ? EventName - : never; -}[keyof PosthogEventMap]; -type SkillEventBase = - Omit; -type SkillEventOutcome = - | { success: true } - | { success: false; failureReason: string }; - -const emitSkillEvent = ( - eventName: EventName, - base: SkillEventBase, - outcome: SkillEventOutcome, -) => { - void captureEvent(eventName, { - ...base, - ...outcome, - } as PosthogEventMap[EventName]); -}; - const getSkillAnalyticsEntryPoint = (analytics?: SkillAnalyticsContext): AskSkillEntryPoint => normalizeSkillAnalyticsEntryPoint(analytics?.entryPoint); @@ -137,26 +105,6 @@ const getSharedSkillActorRelationship = ( return 'member'; }; -const getChangedFieldTypes = ( - before: Pick, - after: AgentSkillInput, -): AskSkillChangedField[] => { - const changedFields: AskSkillChangedField[] = []; - if (before.name !== after.name) { - changedFields.push('name'); - } - if (before.slug !== after.slug) { - changedFields.push('command'); - } - if (before.description !== after.description) { - changedFields.push('description'); - } - if (before.instructions !== after.instructions) { - changedFields.push('instructions'); - } - return changedFields; -}; - const sharedCatalogSkillSelect = (userId: string, orgId: number) => ({ id: true, visibility: true, @@ -240,12 +188,6 @@ const sourceColumnsCarryOver = (skill: AgentSkillSourceColumns): AgentSkillSourc sourceImportedAt: skill.sourceImportedAt, }); -const canManageSharedSkill = ( - skill: { createdById: string }, - userId: string, - role: OrgRole, -) => skill.createdById === userId || role === OrgRole.OWNER; - async function requireManageableSharedSkill( params: RequireManageableSharedSkillParams & { includeUpdateSnapshot: true }, ): Promise; @@ -515,62 +457,23 @@ export const createPersonalAgentSkill = async ( return askError; } - const scope = personalAgentSkillScope(user.id, org.id); const { source } = parsed.data; - const entryPoint = getSkillAnalyticsEntryPoint(analytics); - const creationMethod = analytics?.creationMethod ?? (source ? 'repository' : 'manual'); - const eventBase: SkillEventBase<'ask_skill_created'> = { - source: SKILL_ANALYTICS_SOURCE, - entryPoint, - scope: 'personal', - creationMethod, - isSynced: source !== undefined, - }; - - try { - const skill = await prisma.agentSkill.create({ - data: { - ...scope, - slug: parsed.data.slug, - name: parsed.data.name, - description: parsed.data.description, - instructions: parsed.data.instructions, - createdById: user.id, - updatedById: user.id, - // When imported from a repository file, record provenance so the - // skill can be synced against the indexed file. sourceBlobSha is - // the comparison key. - ...(source ? { - sourceRepoName: source.repoName, - sourceFilePath: source.filePath, - sourceRevision: source.revision, - sourceBlobSha: source.blobSha, - sourceImportedAt: new Date(), - } : {}), - }, - }); + const result = await createPersonalAgentSkillForContext({ + prisma, + userId: user.id, + orgId: org.id, + input: parsed.data, + analytics: { + source: SKILL_ANALYTICS_SOURCE, + entryPoint: getSkillAnalyticsEntryPoint(analytics), + creationMethod: analytics?.creationMethod ?? (source ? 'repository' : 'manual'), + }, + }); + if (!isServiceError(result)) { refreshSkillSettingsViews(); - emitSkillEvent('ask_skill_created', { - ...eventBase, - skillIdHash: hashSkillId(skill.id), - }, { success: true }); - return toAgentSkillListItem(skill); - } catch (error) { - if (isUniqueConstraintError(error)) { - emitSkillEvent('ask_skill_created', eventBase, { - success: false, - failureReason: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, - }); - return skillAlreadyExists(parsed.data.slug); - } - - emitSkillEvent('ask_skill_created', eventBase, { - success: false, - failureReason: ErrorCode.UNEXPECTED_ERROR, - }); - throw error; } + return result; })); }; @@ -590,71 +493,26 @@ export const updatePersonalAgentSkill = async ( return askError; } - const scope = personalAgentSkillAuthScope(user.id, org.id); - const existingSkill = await prisma.agentSkill.findFirst({ - where: { - id: parsed.data.id, - ...scope, - }, - select: { - id: true, - name: true, - slug: true, - description: true, - instructions: true, - sourceRepoName: true, - }, - }); - - if (!existingSkill) { - return skillNotFound(); - } - // Synced skills stay editable: local edits to description/instructions // persist until the user updates the skill from its source file, which // replaces them with the file's content. - const isSynced = existingSkill.sourceRepoName !== null; - const entryPoint = getSkillAnalyticsEntryPoint(analytics); - const changedFieldTypes = getChangedFieldTypes(existingSkill, parsed.data); - const eventBase: SkillEventBase<'ask_skill_updated'> = { - source: SKILL_ANALYTICS_SOURCE, - entryPoint, - scope: 'personal', - isSynced, - skillIdHash: hashSkillId(existingSkill.id), - changedFieldTypes, - }; - - try { - const skill = await prisma.agentSkill.update({ - where: { id: existingSkill.id }, - data: { - slug: parsed.data.slug, - name: parsed.data.name, - description: parsed.data.description, - instructions: parsed.data.instructions, - updatedById: user.id, - }, - }); + const result = await updateAgentSkillForContext({ + prisma, + userId: user.id, + orgId: org.id, + target: { scope: 'personal', id: parsed.data.id }, + fields: parsed.data, + policy: { sharedManageableBy: 'creator-or-owner', allowSynced: true }, + analytics: { + source: SKILL_ANALYTICS_SOURCE, + entryPoint: getSkillAnalyticsEntryPoint(analytics), + }, + }); + if (!isServiceError(result)) { refreshSkillSettingsViews(); - emitSkillEvent('ask_skill_updated', eventBase, { success: true }); - return toAgentSkillListItem(skill); - } catch (error) { - if (isUniqueConstraintError(error)) { - emitSkillEvent('ask_skill_updated', eventBase, { - success: false, - failureReason: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, - }); - return skillAlreadyExists(parsed.data.slug); - } - - emitSkillEvent('ask_skill_updated', eventBase, { - success: false, - failureReason: ErrorCode.UNEXPECTED_ERROR, - }); - throw error; } + return result; })); }; @@ -1458,64 +1316,26 @@ export const updateSharedAgentSkill = async ( return askError; } - const existingSkill = await requireManageableSharedSkill({ + // As with personal skills, a synced shared skill stays editable; local + // edits persist until an update from source replaces them. + const result = await updateAgentSkillForContext({ prisma, - orgId: org.id, userId: user.id, + orgId: org.id, role, - skillId: parsed.data.id, - requireEnabled: true, - includeUpdateSnapshot: true, + target: { scope: 'shared', id: parsed.data.id }, + fields: parsed.data, + policy: { sharedManageableBy: 'creator-or-owner', allowSynced: true }, + analytics: { + source: SKILL_ANALYTICS_SOURCE, + entryPoint: getSkillAnalyticsEntryPoint(analytics), + }, }); - if ("errorCode" in existingSkill) { - return existingSkill; - } - - // As with personal skills, a synced shared skill stays editable; local - // edits persist until an update from source replaces them. - const isSynced = existingSkill.sourceRepoName !== null; - const entryPoint = getSkillAnalyticsEntryPoint(analytics); - const changedFieldTypes = getChangedFieldTypes(existingSkill, parsed.data); - const eventBase: SkillEventBase<'ask_skill_updated'> = { - source: SKILL_ANALYTICS_SOURCE, - entryPoint, - scope: 'shared', - isSynced, - skillIdHash: hashSkillId(existingSkill.id), - changedFieldTypes, - }; - - try { - const skill = await prisma.agentSkill.update({ - where: { id: existingSkill.id }, - data: { - slug: parsed.data.slug, - name: parsed.data.name, - description: parsed.data.description, - instructions: parsed.data.instructions, - updatedById: user.id, - }, - }); - + if (!isServiceError(result)) { refreshSkillSettingsViews(); - emitSkillEvent('ask_skill_updated', eventBase, { success: true }); - return toAgentSkillListItem(skill); - } catch (error) { - if (isUniqueConstraintError(error)) { - emitSkillEvent('ask_skill_updated', eventBase, { - success: false, - failureReason: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, - }); - return skillAlreadyExists(parsed.data.slug); - } - - emitSkillEvent('ask_skill_updated', eventBase, { - success: false, - failureReason: ErrorCode.UNEXPECTED_ERROR, - }); - throw error; } + return result; })); }; diff --git a/packages/web/src/ee/features/chat/skills/skillAnalytics.ts b/packages/web/src/ee/features/chat/skills/skillAnalytics.ts index 0ac519271..b09bf3f7c 100644 --- a/packages/web/src/ee/features/chat/skills/skillAnalytics.ts +++ b/packages/web/src/ee/features/chat/skills/skillAnalytics.ts @@ -14,6 +14,7 @@ const VALID_SKILL_ENTRY_POINTS = new Set([ 'account_ask_agent_settings', 'workspace_ask_agent_settings', 'chat_box', + 'agent_tool', 'unknown', ]); diff --git a/packages/web/src/ee/features/chat/skills/skillCreation.test.ts b/packages/web/src/ee/features/chat/skills/skillCreation.test.ts new file mode 100644 index 000000000..6d8172432 --- /dev/null +++ b/packages/web/src/ee/features/chat/skills/skillCreation.test.ts @@ -0,0 +1,367 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { OrgRole, Prisma } from "@sourcebot/db"; +import { ErrorCode } from "@/lib/errorCodes"; +import { StatusCodes } from "http-status-codes"; + +const mocks = vi.hoisted(() => ({ + captureEvent: vi.fn(), +})); + +vi.mock("@/lib/posthog", () => ({ + captureEvent: mocks.captureEvent, +})); + +const { createPersonalAgentSkillForContext, updateAgentSkillForContext } = await import("./skillCreation"); + +function createPrismaMock() { + return { + agentSkill: { + create: vi.fn(), + findFirst: vi.fn(), + update: vi.fn(), + }, + }; +} + +const uniqueConstraintError = () => + new Prisma.PrismaClientKnownRequestError("Unique constraint failed", { + code: "P2002", + clientVersion: "0", + }); + +const validInput = { + slug: "review", + name: "Review", + description: "Review risky changes.", + instructions: "Review the change.", +}; + +const createdRow = { + id: "skill-1", + visibility: "PERSONAL" as const, + slug: "review", + name: "Review", + description: "Review risky changes.", + instructions: "Review the change.", + enabled: true, + sourceRepoName: null, + sourceFilePath: null, + sourceRevision: null, + createdAt: new Date("2026-01-01T00:00:00.000Z"), + updatedAt: new Date("2026-01-02T00:00:00.000Z"), +}; + +const analytics = { + source: "sourcebot-ask-agent" as const, + entryPoint: "agent_tool" as const, +}; + +beforeEach(() => { + vi.clearAllMocks(); +}); + +describe("createPersonalAgentSkillForContext", () => { + test("creates the skill in the personal scope with creator/updater set", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.create.mockResolvedValue(createdRow); + + const result = await createPersonalAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "user-1", + orgId: 1, + input: validInput, + analytics: { ...analytics, creationMethod: "manual" }, + }); + + expect(prisma.agentSkill.create).toHaveBeenCalledWith({ + data: { + visibility: "PERSONAL", + scopeId: "user-1", + orgId: 1, + slug: "review", + name: "Review", + description: "Review risky changes.", + instructions: "Review the change.", + createdById: "user-1", + updatedById: "user-1", + }, + }); + expect(result).toMatchObject({ id: "skill-1", slug: "review", enabled: true }); + expect(mocks.captureEvent).toHaveBeenCalledWith("ask_skill_created", expect.objectContaining({ + source: "sourcebot-ask-agent", + entryPoint: "agent_tool", + scope: "personal", + creationMethod: "manual", + isSynced: false, + success: true, + })); + }); + + test("maps a unique-constraint error to AGENT_SKILL_ALREADY_EXISTS with failure analytics", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.create.mockRejectedValue(uniqueConstraintError()); + + const result = await createPersonalAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "user-1", + orgId: 1, + input: validInput, + analytics: { ...analytics, creationMethod: "manual" }, + }); + + expect(result).toEqual({ + statusCode: StatusCodes.CONFLICT, + errorCode: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, + message: "A skill with command /review already exists.", + }); + expect(mocks.captureEvent).toHaveBeenCalledWith("ask_skill_created", expect.objectContaining({ + success: false, + failureReason: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, + })); + }); + + test("emits failure analytics and rethrows on unexpected errors", async () => { + const prisma = createPrismaMock(); + const unexpected = new Error("db down"); + prisma.agentSkill.create.mockRejectedValue(unexpected); + + await expect(createPersonalAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "user-1", + orgId: 1, + input: validInput, + analytics: { ...analytics, creationMethod: "manual" }, + })).rejects.toThrow("db down"); + + expect(mocks.captureEvent).toHaveBeenCalledWith("ask_skill_created", expect.objectContaining({ + success: false, + failureReason: ErrorCode.UNEXPECTED_ERROR, + })); + }); +}); + +const existingPersonalRow = { + id: "skill-1", + name: "Review", + slug: "review", + description: "Review risky changes.", + instructions: "Review the change.", + sourceRepoName: null, + createdById: "user-1", +}; + +const updatedRow = { + ...createdRow, + name: "Renamed", + slug: "renamed", +}; + +describe("updateAgentSkillForContext", () => { + test("resolves a personal skill by slug and merges partial fields over the existing values", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.findFirst.mockResolvedValue(existingPersonalRow); + prisma.agentSkill.update.mockResolvedValue(updatedRow); + + const result = await updateAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "user-1", + orgId: 1, + target: { scope: "personal", slug: "review" }, + fields: { name: "Renamed" }, + policy: { sharedManageableBy: "creator", allowSynced: false }, + analytics, + }); + + expect(prisma.agentSkill.findFirst).toHaveBeenCalledWith(expect.objectContaining({ + where: { + slug: "review", + visibility: "PERSONAL", + scopeId: "user-1", + orgId: 1, + createdById: "user-1", + }, + })); + expect(prisma.agentSkill.update).toHaveBeenCalledWith({ + where: { id: "skill-1" }, + data: { + slug: "review", + name: "Renamed", + description: "Review risky changes.", + instructions: "Review the change.", + updatedById: "user-1", + }, + }); + expect(result).toMatchObject({ id: "skill-1" }); + expect(mocks.captureEvent).toHaveBeenCalledWith("ask_skill_updated", expect.objectContaining({ + source: "sourcebot-ask-agent", + entryPoint: "agent_tool", + scope: "personal", + changedFieldTypes: ["name"], + success: true, + })); + }); + + test("the creator-only policy rejects an org owner who is not the creator", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.findFirst.mockResolvedValue({ + ...existingPersonalRow, + createdById: "author-1", + }); + + const result = await updateAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "owner-1", + orgId: 1, + role: OrgRole.OWNER, + target: { scope: "shared", slug: "review" }, + fields: { name: "Renamed" }, + policy: { sharedManageableBy: "creator", allowSynced: false }, + analytics, + }); + + expect(result).toEqual({ + statusCode: StatusCodes.FORBIDDEN, + errorCode: ErrorCode.INSUFFICIENT_PERMISSIONS, + message: "You do not have sufficient permissions to manage this skill.", + }); + expect(prisma.agentSkill.update).not.toHaveBeenCalled(); + }); + + test("the creator-or-owner policy lets an org owner edit another author's shared skill", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.findFirst.mockResolvedValue({ + ...existingPersonalRow, + createdById: "author-1", + }); + prisma.agentSkill.update.mockResolvedValue({ ...updatedRow, visibility: "SHARED" as const }); + + const result = await updateAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "owner-1", + orgId: 1, + role: OrgRole.OWNER, + target: { scope: "shared", id: "skill-1" }, + fields: { name: "Renamed" }, + policy: { sharedManageableBy: "creator-or-owner", allowSynced: true }, + analytics, + }); + + expect(prisma.agentSkill.findFirst).toHaveBeenCalledWith(expect.objectContaining({ + where: { + id: "skill-1", + visibility: "SHARED", + scopeId: "1", + orgId: 1, + enabled: true, + }, + })); + expect(result).toMatchObject({ id: "skill-1" }); + }); + + test("rejects a synced skill with the source-naming error and no write", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.findFirst.mockResolvedValue({ + ...existingPersonalRow, + sourceRepoName: "github.com/acme/widgets", + }); + + const result = await updateAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "user-1", + orgId: 1, + target: { scope: "personal", slug: "review" }, + fields: { name: "Renamed" }, + policy: { sharedManageableBy: "creator", allowSynced: false }, + analytics, + }); + + expect(result).toMatchObject({ + errorCode: ErrorCode.INVALID_REQUEST_BODY, + message: expect.stringContaining("github.com/acme/widgets"), + }); + expect(result).toMatchObject({ + message: expect.stringContaining("Settings → Skills"), + }); + expect(prisma.agentSkill.update).not.toHaveBeenCalled(); + }); + + test("returns skillNotFound for a disabled shared skill (the enabled filter excludes it)", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.findFirst.mockResolvedValue(null); + + const result = await updateAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "user-1", + orgId: 1, + target: { scope: "shared", slug: "review" }, + fields: { name: "Renamed" }, + policy: { sharedManageableBy: "creator", allowSynced: false }, + analytics, + }); + + expect(result).toEqual({ + statusCode: StatusCodes.NOT_FOUND, + errorCode: ErrorCode.AGENT_SKILL_NOT_FOUND, + message: "Skill not found.", + }); + expect(prisma.agentSkill.findFirst).toHaveBeenCalledWith(expect.objectContaining({ + where: expect.objectContaining({ enabled: true }), + })); + }); + + test("maps a slug conflict on rename to AGENT_SKILL_ALREADY_EXISTS with failure analytics", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.findFirst.mockResolvedValue(existingPersonalRow); + prisma.agentSkill.update.mockRejectedValue(uniqueConstraintError()); + + const result = await updateAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "user-1", + orgId: 1, + target: { scope: "personal", slug: "review" }, + fields: { slug: "taken" }, + policy: { sharedManageableBy: "creator", allowSynced: false }, + analytics, + }); + + expect(result).toEqual({ + statusCode: StatusCodes.CONFLICT, + errorCode: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, + message: "A skill with command /taken already exists.", + }); + expect(mocks.captureEvent).toHaveBeenCalledWith("ask_skill_updated", expect.objectContaining({ + source: "sourcebot-ask-agent", + entryPoint: "agent_tool", + success: false, + failureReason: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, + })); + }); + + test("validates the merged result with the schema messages", async () => { + const prisma = createPrismaMock(); + prisma.agentSkill.findFirst.mockResolvedValue(existingPersonalRow); + + const result = await updateAgentSkillForContext({ + // eslint-disable-next-line @typescript-eslint/no-explicit-any + prisma: prisma as any, + userId: "user-1", + orgId: 1, + target: { scope: "personal", slug: "review" }, + fields: { name: " " }, + policy: { sharedManageableBy: "creator", allowSynced: false }, + analytics, + }); + + expect(result).toMatchObject({ errorCode: ErrorCode.INVALID_REQUEST_BODY }); + expect(prisma.agentSkill.update).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/web/src/ee/features/chat/skills/skillCreation.ts b/packages/web/src/ee/features/chat/skills/skillCreation.ts new file mode 100644 index 000000000..12395ae01 --- /dev/null +++ b/packages/web/src/ee/features/chat/skills/skillCreation.ts @@ -0,0 +1,312 @@ +import { ErrorCode } from "@/lib/errorCodes"; +import { captureEvent } from "@/lib/posthog"; +import type { + AskSkillAnalyticsSource, + AskSkillChangedField, + AskSkillCreationMethod, + AskSkillEntryPoint, + AskSkillScope, + PosthogEventMap, +} from "@/lib/posthogEvents"; +import { isUniqueConstraintError } from "@/lib/prismaErrors"; +import { requestBodySchemaValidationError, ServiceError } from "@/lib/serviceError"; +import { OrgRole, personalAgentSkillAuthScope, personalAgentSkillScope, sharedAgentSkillAuthScope, type AgentSkill, type PrismaClient } from "@sourcebot/db"; +import { StatusCodes } from "http-status-codes"; +import { + agentSkillInputSchema, + toAgentSkillListItem, + type AgentSkillInput, + type AgentSkillListItem, + type CreatePersonalAgentSkillInput, +} from "./types"; +import { hashSkillId } from "./skillAnalytics"; + +// The creation, update, and analytics cores shared by the skill server actions +// and the agent-facing skill tools. Everything here runs inside the caller's +// auth context: no auth, no entitlement checks, and no `next/cache` calls +// (route handlers cannot call `refresh()`; only the server actions do). + +export const skillAlreadyExists = (slug: string): ServiceError => ({ + statusCode: StatusCodes.CONFLICT, + errorCode: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, + message: `A skill with command /${slug} already exists.`, +}); + +export const skillNotFound = (): ServiceError => ({ + statusCode: StatusCodes.NOT_FOUND, + errorCode: ErrorCode.AGENT_SKILL_NOT_FOUND, + message: "Skill not found.", +}); + +export const insufficientSkillPermissions = (): ServiceError => ({ + statusCode: StatusCodes.FORBIDDEN, + errorCode: ErrorCode.INSUFFICIENT_PERMISSIONS, + message: "You do not have sufficient permissions to manage this skill.", +}); + +export const syncedSkillNotEditable = (repoName: string): ServiceError => ({ + statusCode: StatusCodes.BAD_REQUEST, + errorCode: ErrorCode.INVALID_REQUEST_BODY, + message: `This skill is synced from ${repoName} and cannot be edited here. It can be edited in Settings → Skills, where it will stay linked to its source file.`, +}); + +export type SkillOutcomeEventName = { + [EventName in keyof PosthogEventMap]: PosthogEventMap[EventName] extends { success: boolean } + ? EventName + : never; +}[keyof PosthogEventMap]; +export type SkillEventBase = + Omit; +export type SkillEventOutcome = + | { success: true } + | { success: false; failureReason: string }; + +export const emitSkillEvent = ( + eventName: EventName, + base: SkillEventBase, + outcome: SkillEventOutcome, +) => { + void captureEvent(eventName, { + ...base, + ...outcome, + } as PosthogEventMap[EventName]); +}; + +export const canManageSharedSkill = ( + skill: { createdById: string }, + userId: string, + role: OrgRole, +) => skill.createdById === userId || role === OrgRole.OWNER; + +export const getChangedFieldTypes = ( + before: Pick, + after: AgentSkillInput, +): AskSkillChangedField[] => { + const changedFields: AskSkillChangedField[] = []; + if (before.name !== after.name) { + changedFields.push('name'); + } + if (before.slug !== after.slug) { + changedFields.push('command'); + } + if (before.description !== after.description) { + changedFields.push('description'); + } + if (before.instructions !== after.instructions) { + changedFields.push('instructions'); + } + return changedFields; +}; + +export type SkillMutationAnalytics = { + source: AskSkillAnalyticsSource; + entryPoint: AskSkillEntryPoint; +}; + +/** + * Creates an enabled personal skill for the given user in the given org, emits + * `ask_skill_created`, and maps unique-constraint violations to the + * already-exists error. `input` must already be parsed (slug normalized). + */ +export const createPersonalAgentSkillForContext = async ({ + prisma, + userId, + orgId, + input, + analytics, +}: { + prisma: PrismaClient; + userId: string; + orgId: number; + input: CreatePersonalAgentSkillInput; + analytics: SkillMutationAnalytics & { creationMethod: AskSkillCreationMethod }; +}): Promise => { + const { source } = input; + const eventBase: SkillEventBase<'ask_skill_created'> = { + source: analytics.source, + entryPoint: analytics.entryPoint, + scope: 'personal', + creationMethod: analytics.creationMethod, + isSynced: source !== undefined, + }; + + try { + const skill = await prisma.agentSkill.create({ + data: { + ...personalAgentSkillScope(userId, orgId), + slug: input.slug, + name: input.name, + description: input.description, + instructions: input.instructions, + createdById: userId, + updatedById: userId, + // When imported from a repository file, record provenance so the + // skill can be synced against the indexed file. sourceBlobSha is + // the comparison key. + ...(source ? { + sourceRepoName: source.repoName, + sourceFilePath: source.filePath, + sourceRevision: source.revision, + sourceBlobSha: source.blobSha, + sourceImportedAt: new Date(), + } : {}), + }, + }); + + emitSkillEvent('ask_skill_created', { + ...eventBase, + skillIdHash: hashSkillId(skill.id), + }, { success: true }); + return toAgentSkillListItem(skill); + } catch (error) { + if (isUniqueConstraintError(error)) { + emitSkillEvent('ask_skill_created', eventBase, { + success: false, + failureReason: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, + }); + return skillAlreadyExists(input.slug); + } + + emitSkillEvent('ask_skill_created', eventBase, { + success: false, + failureReason: ErrorCode.UNEXPECTED_ERROR, + }); + throw error; + } +}; + +// The skill row an update resolves against, identified either by id (the +// settings actions) or by its current slug (the agent tool). +export type UpdateAgentSkillTarget = { + scope: AskSkillScope; +} & ({ id: string; slug?: undefined } | { slug: string; id?: undefined }); + +export type UpdateAgentSkillPolicy = { + // Who may edit a shared skill. The settings actions allow the creator or an + // org owner (requires `role`); the agent tool restricts to the creator. + sharedManageableBy: 'creator' | 'creator-or-owner'; + // Whether repo-synced skills may be edited. The settings actions allow it + // (local edits persist until a sync); the agent tool rejects it so synced + // skills stay synced unless a human intervenes in Settings → Skills. + allowSynced: boolean; +}; + +/** + * The scope-aware update core behind `updatePersonalAgentSkill`, + * `updateSharedAgentSkill`, and the `update_skill` tool. Resolves the target + * row, enforces the caller's policy, merges `fields` over the existing values, + * validates the merged result, performs the update, and emits + * `ask_skill_updated`. Never changes `enabled` or the skill's scope. + */ +export const updateAgentSkillForContext = async ({ + prisma, + userId, + orgId, + role, + target, + fields, + policy, + analytics, +}: { + prisma: PrismaClient; + userId: string; + orgId: number; + // Required when policy.sharedManageableBy is 'creator-or-owner'. + role?: OrgRole; + target: UpdateAgentSkillTarget; + fields: Partial; + policy: UpdateAgentSkillPolicy; + analytics: SkillMutationAnalytics; +}): Promise => { + // Shared skills are only manageable while enabled (mirrors the + // requireManageableSharedSkill rule); a disabled shared skill resolves to + // not-found rather than revealing its state. + const scopeWhere = target.scope === 'personal' + ? personalAgentSkillAuthScope(userId, orgId) + : { ...sharedAgentSkillAuthScope(orgId), enabled: true }; + + const existingSkill = await prisma.agentSkill.findFirst({ + where: { + ...(target.id !== undefined ? { id: target.id } : { slug: target.slug }), + ...scopeWhere, + }, + select: { + id: true, + name: true, + slug: true, + description: true, + instructions: true, + sourceRepoName: true, + createdById: true, + }, + }); + + if (!existingSkill) { + return skillNotFound(); + } + + if (target.scope === 'shared') { + const isManageable = policy.sharedManageableBy === 'creator' + ? existingSkill.createdById === userId + : canManageSharedSkill(existingSkill, userId, role ?? OrgRole.MEMBER); + if (!isManageable) { + return insufficientSkillPermissions(); + } + } + + if (!policy.allowSynced && existingSkill.sourceRepoName !== null) { + return syncedSkillNotEditable(existingSkill.sourceRepoName); + } + + const merged = agentSkillInputSchema.safeParse({ + name: fields.name ?? existingSkill.name, + slug: fields.slug ?? existingSkill.slug, + description: fields.description ?? existingSkill.description, + instructions: fields.instructions ?? existingSkill.instructions, + }); + + if (!merged.success) { + return requestBodySchemaValidationError(merged.error); + } + + const isSynced = existingSkill.sourceRepoName !== null; + const changedFieldTypes = getChangedFieldTypes(existingSkill, merged.data); + const eventBase: SkillEventBase<'ask_skill_updated'> = { + source: analytics.source, + entryPoint: analytics.entryPoint, + scope: target.scope, + isSynced, + skillIdHash: hashSkillId(existingSkill.id), + changedFieldTypes, + }; + + try { + const skill = await prisma.agentSkill.update({ + where: { id: existingSkill.id }, + data: { + slug: merged.data.slug, + name: merged.data.name, + description: merged.data.description, + instructions: merged.data.instructions, + updatedById: userId, + }, + }); + + emitSkillEvent('ask_skill_updated', eventBase, { success: true }); + return toAgentSkillListItem(skill); + } catch (error) { + if (isUniqueConstraintError(error)) { + emitSkillEvent('ask_skill_updated', eventBase, { + success: false, + failureReason: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, + }); + return skillAlreadyExists(merged.data.slug); + } + + emitSkillEvent('ask_skill_updated', eventBase, { + success: false, + failureReason: ErrorCode.UNEXPECTED_ERROR, + }); + throw error; + } +}; diff --git a/packages/web/src/ee/features/chat/skills/skillListing.ts b/packages/web/src/ee/features/chat/skills/skillListing.ts new file mode 100644 index 000000000..28cff3659 --- /dev/null +++ b/packages/web/src/ee/features/chat/skills/skillListing.ts @@ -0,0 +1,119 @@ +import type { AskSkillScope } from "@/lib/posthogEvents"; +import { personalAgentSkillAuthScope, sharedAgentSkillAuthScope, type PrismaClient } from "@sourcebot/db"; +import { agentSkillOrderBy } from "./types"; +import { filterSkillsBySourceRepoAccess } from "./sourceRepoAccess"; + +// The read core behind the `list_skills` tool. Instructions are never included: +// `load_skill` and the `` catalog carry those. Same rules as the +// mutation cores: no auth, no entitlement, no `next/cache`. + +export type AgentSkillToolListItem = { + id: string; + slug: string; + name: string; + description: string; + scope: AskSkillScope; + enabled: boolean; + // Shared rows only: whether the skill is active for this user (adopted or + // auto-enrolled, and not removed). + adopted?: boolean; + isSynced: boolean; + canEdit: boolean; +}; + +const listSkillSelect = { + id: true, + slug: true, + name: true, + description: true, + enabled: true, + autoEnrolled: true, + createdById: true, + sourceRepoName: true, +} as const; + +type ListedSkillRow = { + id: string; + slug: string; + name: string; + description: string; + enabled: boolean; + createdById: string; + sourceRepoName: string | null; +}; + +const toToolListItem = ( + skill: ListedSkillRow, + scope: AskSkillScope, + userId: string, + adopted?: boolean, +): AgentSkillToolListItem => { + const isSynced = skill.sourceRepoName !== null; + return { + id: skill.id, + slug: skill.slug, + name: skill.name, + description: skill.description, + scope, + enabled: skill.enabled, + ...(adopted !== undefined ? { adopted } : {}), + isSynced, + canEdit: (scope === 'personal' || skill.createdById === userId) && !isSynced && skill.enabled, + }; +}; + +export const listAgentSkillsForContext = async ({ + prisma, + userId, + orgId, + scope, +}: { + prisma: PrismaClient; + userId: string; + orgId: number; + scope?: AskSkillScope; +}): Promise => { + const personalSkills = scope === 'shared' ? [] : await prisma.agentSkill.findMany({ + where: { + ...personalAgentSkillAuthScope(userId, orgId), + }, + orderBy: agentSkillOrderBy, + select: listSkillSelect, + }); + + // The entire shared catalog, mirroring listSharedAgentSkillCatalog: enabled + // skills only, hiding skills synced from a repo the user cannot access. + const sharedSkills = scope === 'personal' ? [] : await (async () => { + const skills = await prisma.agentSkill.findMany({ + where: { + ...sharedAgentSkillAuthScope(orgId), + enabled: true, + }, + orderBy: agentSkillOrderBy, + select: { + ...listSkillSelect, + adoptions: { + where: { + userId, + orgId, + }, + select: { + removedAt: true, + }, + }, + }, + }); + + return filterSkillsBySourceRepoAccess(skills, { prisma, orgId }); + })(); + + return [ + ...personalSkills.map((skill) => toToolListItem(skill, 'personal', userId)), + ...sharedSkills.map((skill) => { + const isAdopted = skill.adoptions.some((adoption) => adoption.removedAt === null); + const isRemoved = skill.adoptions.some((adoption) => adoption.removedAt !== null); + const adopted = (skill.autoEnrolled || isAdopted) && !isRemoved; + return toToolListItem(skill, 'shared', userId, adopted); + }), + ]; +}; diff --git a/packages/web/src/ee/features/chat/tools/index.ts b/packages/web/src/ee/features/chat/tools/index.ts index 442b7d7c2..d6776a93b 100644 --- a/packages/web/src/ee/features/chat/tools/index.ts +++ b/packages/web/src/ee/features/chat/tools/index.ts @@ -10,6 +10,9 @@ import { findSymbolReferencesDefinition, findSymbolDefinitionsDefinition, listTreeDefinition, + createSkillDefinition, + updateSkillDefinition, + listSkillsDefinition, } from "@/features/tools"; import type { ToolContext } from "@/features/tools/types"; import type { ToolUIPart } from "ai"; @@ -26,6 +29,9 @@ export const createTools = (context: ToolContext) => ({ [findSymbolReferencesDefinition.name]: toVercelAITool(findSymbolReferencesDefinition, context), [findSymbolDefinitionsDefinition.name]: toVercelAITool(findSymbolDefinitionsDefinition, context), [listTreeDefinition.name]: toVercelAITool(listTreeDefinition, context), + [createSkillDefinition.name]: toVercelAITool(createSkillDefinition, context), + [updateSkillDefinition.name]: toVercelAITool(updateSkillDefinition, context), + [listSkillsDefinition.name]: toVercelAITool(listSkillsDefinition, context), }); export type ReadFileToolUIPart = ToolUIPart<{ read_file: SBChatMessageToolTypes['read_file'] }>; @@ -38,3 +44,6 @@ export type GlobToolUIPart = ToolUIPart<{ glob: SBChatMessageToolTypes['glob'] } export type FindSymbolReferencesToolUIPart = ToolUIPart<{ find_symbol_references: SBChatMessageToolTypes['find_symbol_references'] }>; export type FindSymbolDefinitionsToolUIPart = ToolUIPart<{ find_symbol_definitions: SBChatMessageToolTypes['find_symbol_definitions'] }>; export type ListTreeToolUIPart = ToolUIPart<{ list_tree: SBChatMessageToolTypes['list_tree'] }>; +export type CreateSkillToolUIPart = ToolUIPart<{ create_skill: SBChatMessageToolTypes['create_skill'] }>; +export type UpdateSkillToolUIPart = ToolUIPart<{ update_skill: SBChatMessageToolTypes['update_skill'] }>; +export type ListSkillsToolUIPart = ToolUIPart<{ list_skills: SBChatMessageToolTypes['list_skills'] }>; diff --git a/packages/web/src/ee/features/mcp/server.ts b/packages/web/src/ee/features/mcp/server.ts index 03ae96b9a..3f08f2fa9 100644 --- a/packages/web/src/ee/features/mcp/server.ts +++ b/packages/web/src/ee/features/mcp/server.ts @@ -12,23 +12,26 @@ import _dedent from 'dedent'; import { z } from 'zod'; import { getConfiguredLanguageModelsInfo } from "@/features/chat/utils.server"; import { + createSkillDefinition, findSymbolDefinitionsDefinition, findSymbolReferencesDefinition, getDiffDefinition, listBranchesDefinition, listCommitsDefinition, listReposDefinition, + listSkillsDefinition, listTreeDefinition, readFileDefinition, registerMcpTool, grepDefinition, ToolContext, globDefinition, + updateSkillDefinition, } from '@/features/tools'; const dedent = _dedent.withOptions({ alignValues: true }); -export async function createMcpServer(): Promise { +export async function createMcpServer({ canManageSkills }: { canManageSkills: boolean }): Promise { // Defense-in-depth: the MCP server is a paid feature. The /api/ee/mcp route // gates on the `mcp` entitlement before calling this; this assertion // backstops that contract so the server can't be constructed on a @@ -60,6 +63,19 @@ export async function createMcpServer(): Promise { registerMcpTool(server, findSymbolDefinitionsDefinition, toolContext); registerMcpTool(server, findSymbolReferencesDefinition, toolContext); + // The skill management tools require an authenticated user (skills are + // per-user) whose credential is not repository-scoped (scoped access + // tokens grant repo access only, never account-level skill management), + // plus the Ask feature. Registration is best-effort UX: sessions are keyed + // by owner, not principal, so the per-request checks inside each tool's + // execute (withAuth + the scoped-token rejection) remain the real + // enforcement. + if (canManageSkills && await hasEntitlement('ask')) { + registerMcpTool(server, createSkillDefinition, toolContext); + registerMcpTool(server, updateSkillDefinition, toolContext); + registerMcpTool(server, listSkillsDefinition, toolContext); + } + server.registerTool( "list_language_models", { diff --git a/packages/web/src/features/tools/adapters.test.ts b/packages/web/src/features/tools/adapters.test.ts new file mode 100644 index 000000000..a0e6f9aed --- /dev/null +++ b/packages/web/src/features/tools/adapters.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, test, vi } from "vitest"; +import { z } from "zod"; +import type { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; +import type { ToolDefinition } from "./types"; + +vi.mock("@/lib/posthog", () => ({ + captureEvent: vi.fn(), +})); + +const { registerMcpTool, toVercelAITool } = await import("./adapters"); + +const emptyShape = {}; + +const makeDefinition = (overrides: Partial>): ToolDefinition => ({ + name: "fake_tool", + title: "Fake tool", + description: "A fake tool.", + inputSchema: z.object(emptyShape), + isReadOnly: true, + isIdempotent: true, + execute: vi.fn(async () => ({ output: "", metadata: {} })), + ...overrides, +}); + +describe("toVercelAITool", () => { + test("requires approval for non-read-only tools", () => { + const tool = toVercelAITool(makeDefinition({ isReadOnly: false }), {}); + expect(tool.needsApproval).toBe(true); + }); + + test("does not require approval for read-only tools", () => { + const tool = toVercelAITool(makeDefinition({ isReadOnly: true }), {}); + expect(tool.needsApproval).toBeFalsy(); + }); +}); + +describe("registerMcpTool", () => { + const registerOn = (def: ToolDefinition) => { + const registerTool = vi.fn(); + registerMcpTool({ registerTool } as unknown as McpServer, def, {}); + return registerTool.mock.calls[0][1].annotations as Record; + }; + + test("emits destructiveHint: false for an additive write", () => { + const annotations = registerOn(makeDefinition({ isReadOnly: false, isIdempotent: false, isDestructive: false })); + expect(annotations).toEqual({ + readOnlyHint: false, + idempotentHint: false, + destructiveHint: false, + }); + }); + + test("emits destructiveHint: true for a destructive write", () => { + const annotations = registerOn(makeDefinition({ isReadOnly: false, isDestructive: true })); + expect(annotations).toMatchObject({ destructiveHint: true }); + }); + + test("omits destructiveHint when isDestructive is undefined", () => { + const annotations = registerOn(makeDefinition({})); + expect(annotations).toEqual({ + readOnlyHint: true, + idempotentHint: true, + }); + }); +}); diff --git a/packages/web/src/features/tools/adapters.ts b/packages/web/src/features/tools/adapters.ts index 2d3a4142e..2cb68985c 100644 --- a/packages/web/src/features/tools/adapters.ts +++ b/packages/web/src/features/tools/adapters.ts @@ -12,6 +12,8 @@ export function toVercelAITool { let success = true; try { @@ -50,6 +52,7 @@ export function registerMcpTool { diff --git a/packages/web/src/features/tools/createSkill.test.ts b/packages/web/src/features/tools/createSkill.test.ts new file mode 100644 index 000000000..bd794c890 --- /dev/null +++ b/packages/web/src/features/tools/createSkill.test.ts @@ -0,0 +1,169 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { ErrorCode } from "@/lib/errorCodes"; +import { StatusCodes } from "http-status-codes"; + +const mocks = vi.hoisted(() => ({ + authContext: undefined as unknown, + checkAskEntitlement: vi.fn(), + withAuth: vi.fn(), + createPersonalAgentSkillForContext: vi.fn(), +})); + +vi.mock("@sourcebot/shared", () => ({ + createLogger: () => ({ error: vi.fn(), info: vi.fn(), warn: vi.fn(), debug: vi.fn() }), + env: { AUTH_URL: "https://sourcebot.example.com" }, +})); + +vi.mock("@/features/chat/utils.server", () => ({ + checkAskEntitlement: mocks.checkAskEntitlement, +})); + +vi.mock("@/middleware/withAuth", () => ({ + withAuth: mocks.withAuth, +})); + +vi.mock("@/ee/features/chat/skills/skillCreation", () => ({ + createPersonalAgentSkillForContext: mocks.createPersonalAgentSkillForContext, +})); + +const { createSkillDefinition } = await import("./createSkill"); + +const validInput = { + name: "Review PR", + slug: "Review PR", + description: "Review a pull request.", + instructions: "Look for correctness issues first.", +}; + +const createdSkill = { + id: "skill-1", + scope: "PERSONAL", + slug: "review-pr", + name: "Review PR", + description: "Review a pull request.", + instructions: "Look for correctness issues first.", + enabled: true, + source: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-01T00:00:00.000Z", +}; + +beforeEach(() => { + vi.clearAllMocks(); + mocks.authContext = { + org: { id: 1 }, + user: { id: "user-1" }, + prisma: {}, + principal: { source: "api_key" }, + }; + mocks.withAuth.mockImplementation(async (callback: (context: unknown) => unknown) => callback(mocks.authContext)); + mocks.checkAskEntitlement.mockResolvedValue(null); + mocks.createPersonalAgentSkillForContext.mockResolvedValue(createdSkill); +}); + +describe("createSkillDefinition", () => { + test("has the expected definition flags", () => { + expect(createSkillDefinition.name).toBe("create_skill"); + expect(createSkillDefinition.isReadOnly).toBe(false); + expect(createSkillDefinition.isIdempotent).toBe(false); + expect(createSkillDefinition.isDestructive).toBe(false); + }); + + test("normalizes the slug before creating", async () => { + await createSkillDefinition.execute(validInput, { source: "sourcebot-ask-agent" }); + + expect(mocks.createPersonalAgentSkillForContext).toHaveBeenCalledWith(expect.objectContaining({ + userId: "user-1", + orgId: 1, + input: expect.objectContaining({ slug: "review-pr" }), + analytics: { + source: "sourcebot-ask-agent", + entryPoint: "agent_tool", + creationMethod: "manual", + }, + })); + }); + + test("throws the schema message on invalid input without calling withAuth", async () => { + await expect(createSkillDefinition.execute( + { ...validInput, name: " " }, + { source: "sourcebot-ask-agent" }, + )).rejects.toThrow("Name is required."); + + expect(mocks.withAuth).not.toHaveBeenCalled(); + expect(mocks.createPersonalAgentSkillForContext).not.toHaveBeenCalled(); + }); + + test("throws when the requester is not authenticated", async () => { + mocks.withAuth.mockResolvedValue({ + statusCode: StatusCodes.UNAUTHORIZED, + errorCode: ErrorCode.NOT_AUTHENTICATED, + message: "Not authenticated", + }); + + await expect(createSkillDefinition.execute(validInput, { source: "sourcebot-ask-agent" })) + .rejects.toThrow("Authentication is required to create skills."); + }); + + test("rejects a repository-scoped access token without creating", async () => { + mocks.authContext = { + org: { id: 1 }, + user: { id: "user-1" }, + prisma: {}, + principal: { source: "scoped_access_token" }, + }; + + await expect(createSkillDefinition.execute(validInput, { source: "sourcebot-mcp-server" })) + .rejects.toThrow("Repository-scoped access tokens cannot manage skills."); + expect(mocks.createPersonalAgentSkillForContext).not.toHaveBeenCalled(); + }); + + test("throws the entitlement message when Ask is not available", async () => { + mocks.checkAskEntitlement.mockResolvedValue({ + statusCode: StatusCodes.FORBIDDEN, + errorCode: ErrorCode.INSUFFICIENT_PERMISSIONS, + message: "Ask Sourcebot is not available in your current plan", + }); + + await expect(createSkillDefinition.execute(validInput, { source: "sourcebot-ask-agent" })) + .rejects.toThrow("Ask Sourcebot is not available in your current plan"); + expect(mocks.createPersonalAgentSkillForContext).not.toHaveBeenCalled(); + }); + + test("surfaces a slug conflict with the actionable message", async () => { + mocks.createPersonalAgentSkillForContext.mockResolvedValue({ + statusCode: StatusCodes.CONFLICT, + errorCode: ErrorCode.AGENT_SKILL_ALREADY_EXISTS, + message: "A skill with command /review-pr already exists.", + }); + + await expect(createSkillDefinition.execute(validInput, { source: "sourcebot-ask-agent" })) + .rejects.toThrow("A skill with command /review-pr already exists."); + }); + + test("returns the created skill as JSON output plus UI metadata with a settings deep link", async () => { + const result = await createSkillDefinition.execute(validInput, { source: "sourcebot-mcp-server" }); + + const url = "https://sourcebot.example.com/settings/skills?skill=skill-1"; + expect(JSON.parse(result.output)).toEqual({ + id: "skill-1", + slug: "review-pr", + name: "Review PR", + description: "Review a pull request.", + enabled: true, + createdAt: "2026-01-01T00:00:00.000Z", + url, + }); + expect(result.metadata).toEqual({ + id: "skill-1", + slug: "review-pr", + name: "Review PR", + url, + }); + // Instructions are never echoed back. + expect(result.output).not.toContain("Look for correctness issues first."); + expect(mocks.createPersonalAgentSkillForContext).toHaveBeenCalledWith(expect.objectContaining({ + analytics: expect.objectContaining({ source: "sourcebot-mcp-server" }), + })); + }); +}); diff --git a/packages/web/src/features/tools/createSkill.ts b/packages/web/src/features/tools/createSkill.ts new file mode 100644 index 000000000..6f76e8f14 --- /dev/null +++ b/packages/web/src/features/tools/createSkill.ts @@ -0,0 +1,98 @@ +import { z } from "zod"; +import { checkAskEntitlement } from "@/features/chat/utils.server"; +import { agentSkillInputSchema } from "@/ee/features/chat/skills/types"; +import { createPersonalAgentSkillForContext } from "@/ee/features/chat/skills/skillCreation"; +import { isServiceError } from "@/lib/utils"; +import { sew } from "@/middleware/sew"; +import { withAuth } from "@/middleware/withAuth"; +import { ToolDefinition } from "./types"; +import { logger } from "./logger"; +import { scopedTokensCannotManageSkills, skillSettingsUrl, toSkillToolError, toSkillAnalyticsSource } from "./skillToolShared"; +import description from "./createSkill.txt"; + +// Plain described strings rather than the piped slug schema: the AI SDK +// converts piped zod schemas to `allOf` JSON schemas in the model-facing tool +// definition, so validation runs in `execute` via agentSkillInputSchema instead +// (same rules and messages as the settings UI). +const createSkillShape = { + name: z.string().describe("Display name for the skill, 1-80 characters."), + slug: z.string().describe("Slash command for the skill, without the leading '/'. Lowercase letters, numbers, and hyphens; at most 64 characters (e.g. 'review-pr')."), + description: z.string().describe("When to use the skill, 1-500 characters. Shown in the skill catalog the agent uses to auto-load skills."), + instructions: z.string().describe("Markdown instructions the agent follows when the skill is invoked, 1-20,000 characters."), +}; + +export type CreateSkillMetadata = { + id: string; + slug: string; + name: string; + url: string; +}; + +export const createSkillDefinition: ToolDefinition<"create_skill", typeof createSkillShape, CreateSkillMetadata> = { + name: "create_skill", + title: "Create skill", + isReadOnly: false, + isIdempotent: false, + isDestructive: false, + description, + inputSchema: z.object(createSkillShape), + execute: async (input, context) => { + logger.debug('create_skill', { slug: input.slug }); + + const parsed = agentSkillInputSchema.safeParse(input); + if (!parsed.success) { + throw new Error(parsed.error.issues.map((issue) => issue.message).join(' ')); + } + + const result = await sew(() => + withAuth(async ({ org, user, prisma, principal }) => { + if (principal.source === 'scoped_access_token') { + return scopedTokensCannotManageSkills(); + } + + const askError = await checkAskEntitlement(); + if (askError) { + return askError; + } + + return createPersonalAgentSkillForContext({ + prisma, + userId: user.id, + orgId: org.id, + input: parsed.data, + analytics: { + source: toSkillAnalyticsSource(context.source), + entryPoint: 'agent_tool', + creationMethod: 'manual', + }, + }); + })); + + if (isServiceError(result)) { + logger.error('create_skill failed', { serviceError: result }); + throw toSkillToolError(result, { + notAuthenticatedMessage: 'Authentication is required to create skills.', + fallbackMessage: 'Failed to create skill.', + }); + } + + const url = skillSettingsUrl(result.id); + return { + output: JSON.stringify({ + id: result.id, + slug: result.slug, + name: result.name, + description: result.description, + enabled: result.enabled, + createdAt: result.createdAt, + url, + }), + metadata: { + id: result.id, + slug: result.slug, + name: result.name, + url, + }, + }; + }, +}; diff --git a/packages/web/src/features/tools/createSkill.txt b/packages/web/src/features/tools/createSkill.txt new file mode 100644 index 000000000..20b192f1a --- /dev/null +++ b/packages/web/src/features/tools/createSkill.txt @@ -0,0 +1,9 @@ +Creates a new agent skill: a reusable, named instruction set the user can invoke later. + +Usage: +- ONLY call this tool when the user explicitly asks to create or save a skill. NEVER invent skills from conversation context on your own initiative. +- The skill is personal to the requesting user and enabled immediately. In later messages it can be invoked manually as `/` or loaded automatically when a request matches its description. +- `slug` becomes the skill's slash command (without the leading `/`). It must be unique among the user's skills; if it already exists, the tool fails and you can retry with a different slug. Use `list_skills` to check which slugs are taken. +- `description` should say when the skill applies — it is what the agent matches against to auto-load the skill. +- `instructions` are the markdown instructions the agent follows when the skill is invoked. +- The result includes a `url` to the skill in Settings → Skills, where the user can edit, share, or delete it. To change a skill later, use `update_skill`. diff --git a/packages/web/src/features/tools/index.ts b/packages/web/src/features/tools/index.ts index 99d27041a..a339ef64b 100644 --- a/packages/web/src/features/tools/index.ts +++ b/packages/web/src/features/tools/index.ts @@ -8,5 +8,8 @@ export * from './getDiff'; export * from './findSymbolReferences'; export * from './findSymbolDefinitions'; export * from './listTree'; +export * from './createSkill'; +export * from './updateSkill'; +export * from './listSkills'; export * from './adapters'; export * from './types'; diff --git a/packages/web/src/features/tools/listSkills.test.ts b/packages/web/src/features/tools/listSkills.test.ts new file mode 100644 index 000000000..d13ca1053 --- /dev/null +++ b/packages/web/src/features/tools/listSkills.test.ts @@ -0,0 +1,147 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; + +const mocks = vi.hoisted(() => ({ + authContext: undefined as unknown, + checkAskEntitlement: vi.fn(), + withAuth: vi.fn(), +})); + +vi.mock("@sourcebot/shared", () => ({ + createLogger: () => ({ error: vi.fn(), info: vi.fn(), warn: vi.fn(), debug: vi.fn() }), + env: { AUTH_URL: "https://sourcebot.example.com" }, +})); + +vi.mock("@/features/chat/utils.server", () => ({ + checkAskEntitlement: mocks.checkAskEntitlement, +})); + +vi.mock("@/middleware/withAuth", () => ({ + withAuth: mocks.withAuth, +})); + +// The real listing core runs against a mocked prisma so canEdit / isSynced / +// adopted derivation is exercised end-to-end through the tool. +const { listSkillsDefinition } = await import("./listSkills"); + +function createPrismaMock() { + return { + agentSkill: { + findMany: vi.fn().mockResolvedValue([]), + }, + repo: { + findMany: vi.fn().mockResolvedValue([]), + }, + }; +} + +const personalRow = (overrides: Record = {}) => ({ + id: "personal-1", + slug: "review", + name: "Review", + description: "Personal review.", + enabled: true, + autoEnrolled: false, + createdById: "user-1", + sourceRepoName: null, + ...overrides, +}); + +const sharedRow = (overrides: Record = {}) => ({ + id: "shared-1", + slug: "audit", + name: "Audit", + description: "Shared audit.", + enabled: true, + autoEnrolled: false, + createdById: "user-1", + sourceRepoName: null, + adoptions: [], + ...overrides, +}); + +let prisma: ReturnType; + +beforeEach(() => { + vi.clearAllMocks(); + prisma = createPrismaMock(); + mocks.authContext = { + org: { id: 1 }, + user: { id: "user-1" }, + prisma, + principal: { source: "api_key" }, + }; + mocks.withAuth.mockImplementation(async (callback: (context: unknown) => unknown) => callback(mocks.authContext)); + mocks.checkAskEntitlement.mockResolvedValue(null); +}); + +describe("listSkillsDefinition", () => { + test("has the expected definition flags", () => { + expect(listSkillsDefinition.name).toBe("list_skills"); + expect(listSkillsDefinition.isReadOnly).toBe(true); + expect(listSkillsDefinition.isIdempotent).toBe(true); + expect(listSkillsDefinition.isDestructive).toBe(false); + }); + + test("derives canEdit / isSynced / adopted across personal and shared rows and never returns instructions", async () => { + prisma.agentSkill.findMany + .mockResolvedValueOnce([ + personalRow(), + personalRow({ id: "personal-2", slug: "disabled", name: "Disabled", enabled: false }), + personalRow({ id: "personal-3", slug: "synced", name: "Synced", sourceRepoName: "github.com/acme/widgets" }), + ]) + .mockResolvedValueOnce([ + sharedRow({ adoptions: [{ removedAt: null }] }), + sharedRow({ id: "shared-2", slug: "foreign", name: "Foreign", createdById: "author-2", autoEnrolled: true }), + ]); + prisma.repo.findMany.mockResolvedValue([{ name: "github.com/acme/widgets" }]); + + const result = await listSkillsDefinition.execute({}, { source: "sourcebot-ask-agent" }); + + expect(result.metadata).toEqual({ count: 5 }); + const { skills } = JSON.parse(result.output); + expect(skills).toEqual([ + { id: "personal-1", slug: "review", name: "Review", description: "Personal review.", scope: "personal", enabled: true, isSynced: false, canEdit: true }, + { id: "personal-2", slug: "disabled", name: "Disabled", description: "Personal review.", scope: "personal", enabled: false, isSynced: false, canEdit: false }, + { id: "personal-3", slug: "synced", name: "Synced", description: "Personal review.", scope: "personal", enabled: true, isSynced: true, canEdit: false }, + { id: "shared-1", slug: "audit", name: "Audit", description: "Shared audit.", scope: "shared", enabled: true, adopted: true, isSynced: false, canEdit: true }, + { id: "shared-2", slug: "foreign", name: "Foreign", description: "Shared audit.", scope: "shared", enabled: true, adopted: true, isSynced: false, canEdit: false }, + ]); + }); + + test("hides shared skills synced from a repo the user cannot access", async () => { + prisma.agentSkill.findMany + .mockResolvedValueOnce([]) + .mockResolvedValueOnce([ + sharedRow({ sourceRepoName: "github.com/acme/secret" }), + ]); + prisma.repo.findMany.mockResolvedValue([]); + + const result = await listSkillsDefinition.execute({}, { source: "sourcebot-ask-agent" }); + + expect(result.metadata).toEqual({ count: 0 }); + }); + + test("rejects a repository-scoped access token without listing", async () => { + mocks.authContext = { + org: { id: 1 }, + user: { id: "user-1" }, + prisma, + principal: { source: "scoped_access_token" }, + }; + + await expect(listSkillsDefinition.execute({}, { source: "sourcebot-mcp-server" })) + .rejects.toThrow("Repository-scoped access tokens cannot manage skills."); + expect(prisma.agentSkill.findMany).not.toHaveBeenCalled(); + }); + + test("the scope filter limits the query to one catalog", async () => { + prisma.agentSkill.findMany.mockResolvedValueOnce([personalRow()]); + + const result = await listSkillsDefinition.execute({ scope: "personal" }, { source: "sourcebot-ask-agent" }); + + expect(prisma.agentSkill.findMany).toHaveBeenCalledTimes(1); + expect(result.metadata).toEqual({ count: 1 }); + const { skills } = JSON.parse(result.output); + expect(skills[0].scope).toBe("personal"); + }); +}); diff --git a/packages/web/src/features/tools/listSkills.ts b/packages/web/src/features/tools/listSkills.ts new file mode 100644 index 000000000..5eca43fd0 --- /dev/null +++ b/packages/web/src/features/tools/listSkills.ts @@ -0,0 +1,65 @@ +import { z } from "zod"; +import { checkAskEntitlement } from "@/features/chat/utils.server"; +import { listAgentSkillsForContext } from "@/ee/features/chat/skills/skillListing"; +import { isServiceError } from "@/lib/utils"; +import { sew } from "@/middleware/sew"; +import { withAuth } from "@/middleware/withAuth"; +import { ToolDefinition } from "./types"; +import { logger } from "./logger"; +import { scopedTokensCannotManageSkills, toSkillToolError } from "./skillToolShared"; +import description from "./listSkills.txt"; + +const listSkillsShape = { + scope: z.enum(['personal', 'shared']).optional().describe("Filter to one catalog: 'personal' or 'shared'. Omit to list both."), +}; + +export type ListSkillsMetadata = { + count: number; +}; + +export const listSkillsDefinition: ToolDefinition<"list_skills", typeof listSkillsShape, ListSkillsMetadata> = { + name: "list_skills", + title: "List skills", + isReadOnly: true, + isIdempotent: true, + isDestructive: false, + description, + inputSchema: z.object(listSkillsShape), + execute: async (input, _context) => { + logger.debug('list_skills', input); + + const result = await sew(() => + withAuth(async ({ org, user, prisma, principal }) => { + if (principal.source === 'scoped_access_token') { + return scopedTokensCannotManageSkills(); + } + + const askError = await checkAskEntitlement(); + if (askError) { + return askError; + } + + return listAgentSkillsForContext({ + prisma, + userId: user.id, + orgId: org.id, + scope: input.scope, + }); + })); + + if (isServiceError(result)) { + logger.error('list_skills failed', { serviceError: result }); + throw toSkillToolError(result, { + notAuthenticatedMessage: 'Authentication is required to list skills.', + fallbackMessage: 'Failed to list skills.', + }); + } + + return { + output: JSON.stringify({ skills: result }), + metadata: { + count: result.length, + }, + }; + }, +}; diff --git a/packages/web/src/features/tools/listSkills.txt b/packages/web/src/features/tools/listSkills.txt new file mode 100644 index 000000000..20adba2b1 --- /dev/null +++ b/packages/web/src/features/tools/listSkills.txt @@ -0,0 +1,7 @@ +Lists the agent skills visible to the requesting user: their personal skills plus the organization's shared skill catalog (including shared skills they have not adopted or authored). + +Usage: +- Use `scope` to list only `personal` or only `shared` skills; omit it to list both. +- Each row carries `slug` and `scope`, the identifier pair `update_skill` needs, plus `enabled`, `isSynced` (linked to a repository file), `canEdit` (whether `update_skill` can edit it), and, on shared rows, `adopted` (whether the skill is active for this user). +- Skill instructions are never included in the results. +- Use this before `create_skill` to avoid slug conflicts, and before `update_skill` to locate the target skill and check whether it is editable. diff --git a/packages/web/src/features/tools/skillToolShared.ts b/packages/web/src/features/tools/skillToolShared.ts new file mode 100644 index 000000000..5eb312cce --- /dev/null +++ b/packages/web/src/features/tools/skillToolShared.ts @@ -0,0 +1,48 @@ +import { env } from "@sourcebot/shared"; +import { ErrorCode } from "@/lib/errorCodes"; +import type { AskSkillAnalyticsSource } from "@/lib/posthogEvents"; +import type { ServiceError } from "@/lib/serviceError"; +import { StatusCodes } from "http-status-codes"; + +// Helpers shared by the skill management tools (create_skill, update_skill, +// list_skills). + +export const toSkillAnalyticsSource = (source: string | undefined): AskSkillAnalyticsSource => + source === 'sourcebot-mcp-server' || source === 'sourcebot-web-client' + ? source + : 'sourcebot-ask-agent'; + +// Repository-scoped access tokens grant access to selected repositories only; +// account-level skill management is outside their documented authorization +// boundary. Every skill tool rejects them in its handler regardless of +// registration-time gating: an MCP session is keyed by its owner, not its +// principal, so a session created with a full credential can later be driven +// by a scoped token for the same user. +export const scopedTokensCannotManageSkills = (): ServiceError => ({ + statusCode: StatusCodes.FORBIDDEN, + errorCode: ErrorCode.INSUFFICIENT_PERMISSIONS, + message: "Repository-scoped access tokens cannot manage skills.", +}); + +// The settings page supports ?skill= deep links that open the given skill. +export const skillSettingsUrl = (skillId: string): string => + `${env.AUTH_URL.replace(/\/$/, '')}/settings/skills?skill=${encodeURIComponent(skillId)}`; + +/** + * Maps a ServiceError to the tool error both adapters surface to the model. + * Validation, conflict, permission, and entitlement errors keep their + * actionable messages; unexpected failures collapse to a generic message + * (details are already logged server-side by `sew`). + */ +export const toSkillToolError = ( + error: ServiceError, + { notAuthenticatedMessage, fallbackMessage }: { notAuthenticatedMessage: string; fallbackMessage: string }, +): Error => { + if (error.errorCode === ErrorCode.NOT_AUTHENTICATED) { + return new Error(notAuthenticatedMessage); + } + if (error.errorCode === ErrorCode.UNEXPECTED_ERROR) { + return new Error(fallbackMessage); + } + return new Error(error.message); +}; diff --git a/packages/web/src/features/tools/types.ts b/packages/web/src/features/tools/types.ts index 9c221a409..4be6e4a87 100644 --- a/packages/web/src/features/tools/types.ts +++ b/packages/web/src/features/tools/types.ts @@ -30,6 +30,10 @@ export interface ToolDefinition< inputSchema: z.ZodObject; isReadOnly: boolean; isIdempotent: boolean; + // Whether the tool can destroy or overwrite existing data. Emitted as the + // MCP destructiveHint annotation (which MCP clients default to true for + // non-read-only tools, so additive writes should set this to false). + isDestructive?: boolean; execute: (input: z.infer>, context: ToolContext) => Promise>; } diff --git a/packages/web/src/features/tools/updateSkill.test.ts b/packages/web/src/features/tools/updateSkill.test.ts new file mode 100644 index 000000000..62173d548 --- /dev/null +++ b/packages/web/src/features/tools/updateSkill.test.ts @@ -0,0 +1,165 @@ +import { beforeEach, describe, expect, test, vi } from "vitest"; +import { ErrorCode } from "@/lib/errorCodes"; +import { StatusCodes } from "http-status-codes"; + +const mocks = vi.hoisted(() => ({ + authContext: undefined as unknown, + checkAskEntitlement: vi.fn(), + withAuth: vi.fn(), + updateAgentSkillForContext: vi.fn(), +})); + +vi.mock("@sourcebot/shared", () => ({ + createLogger: () => ({ error: vi.fn(), info: vi.fn(), warn: vi.fn(), debug: vi.fn() }), + env: { AUTH_URL: "https://sourcebot.example.com" }, +})); + +vi.mock("@/features/chat/utils.server", () => ({ + checkAskEntitlement: mocks.checkAskEntitlement, +})); + +vi.mock("@/middleware/withAuth", () => ({ + withAuth: mocks.withAuth, +})); + +vi.mock("@/ee/features/chat/skills/skillCreation", () => ({ + updateAgentSkillForContext: mocks.updateAgentSkillForContext, +})); + +const { updateSkillDefinition } = await import("./updateSkill"); + +const updatedSkill = { + id: "skill-1", + scope: "PERSONAL", + slug: "review-pr", + name: "Review PR", + description: "Review a pull request.", + instructions: "Look for correctness issues first.", + enabled: true, + source: null, + createdAt: "2026-01-01T00:00:00.000Z", + updatedAt: "2026-01-02T00:00:00.000Z", +}; + +beforeEach(() => { + vi.clearAllMocks(); + mocks.authContext = { + org: { id: 1 }, + user: { id: "user-1" }, + prisma: {}, + principal: { source: "api_key" }, + }; + mocks.withAuth.mockImplementation(async (callback: (context: unknown) => unknown) => callback(mocks.authContext)); + mocks.checkAskEntitlement.mockResolvedValue(null); + mocks.updateAgentSkillForContext.mockResolvedValue(updatedSkill); +}); + +describe("updateSkillDefinition", () => { + test("has the expected definition flags", () => { + expect(updateSkillDefinition.name).toBe("update_skill"); + expect(updateSkillDefinition.isReadOnly).toBe(false); + expect(updateSkillDefinition.isIdempotent).toBe(true); + expect(updateSkillDefinition.isDestructive).toBe(true); + }); + + test("passes only the provided fields so unspecified fields keep their current values", async () => { + await updateSkillDefinition.execute( + { slug: "Review PR", scope: "personal", name: "Renamed" }, + { source: "sourcebot-ask-agent" }, + ); + + expect(mocks.updateAgentSkillForContext).toHaveBeenCalledWith({ + prisma: {}, + userId: "user-1", + orgId: 1, + // The lookup slug is normalized for forgiveness. + target: { scope: "personal", slug: "review-pr" }, + fields: { name: "Renamed" }, + policy: { sharedManageableBy: "creator", allowSynced: false }, + analytics: { + source: "sourcebot-ask-agent", + entryPoint: "agent_tool", + }, + }); + }); + + test("maps newSlug to the slug field", async () => { + await updateSkillDefinition.execute( + { slug: "review-pr", scope: "shared", newSlug: "review-pr-v2", instructions: "New instructions." }, + { source: "sourcebot-mcp-server" }, + ); + + expect(mocks.updateAgentSkillForContext).toHaveBeenCalledWith(expect.objectContaining({ + target: { scope: "shared", slug: "review-pr" }, + fields: { slug: "review-pr-v2", instructions: "New instructions." }, + analytics: expect.objectContaining({ source: "sourcebot-mcp-server" }), + })); + }); + + test("throws when no skill matches the slug + scope", async () => { + mocks.updateAgentSkillForContext.mockResolvedValue({ + statusCode: StatusCodes.NOT_FOUND, + errorCode: ErrorCode.AGENT_SKILL_NOT_FOUND, + message: "Skill not found.", + }); + + await expect(updateSkillDefinition.execute( + { slug: "ghost", scope: "personal", name: "Renamed" }, + { source: "sourcebot-ask-agent" }, + )).rejects.toThrow("Skill not found."); + }); + + test("throws when the requester is not authenticated", async () => { + mocks.withAuth.mockResolvedValue({ + statusCode: StatusCodes.UNAUTHORIZED, + errorCode: ErrorCode.NOT_AUTHENTICATED, + message: "Not authenticated", + }); + + await expect(updateSkillDefinition.execute( + { slug: "review-pr", scope: "personal", name: "Renamed" }, + { source: "sourcebot-ask-agent" }, + )).rejects.toThrow("Authentication is required to update skills."); + }); + + test("rejects a repository-scoped access token without updating", async () => { + mocks.authContext = { + org: { id: 1 }, + user: { id: "user-1" }, + prisma: {}, + principal: { source: "scoped_access_token" }, + }; + + await expect(updateSkillDefinition.execute( + { slug: "review-pr", scope: "personal", name: "Renamed" }, + { source: "sourcebot-mcp-server" }, + )).rejects.toThrow("Repository-scoped access tokens cannot manage skills."); + expect(mocks.updateAgentSkillForContext).not.toHaveBeenCalled(); + }); + + test("returns the updated skill as JSON output plus UI metadata with the tool's scope", async () => { + const result = await updateSkillDefinition.execute( + { slug: "review-pr", scope: "personal", name: "Review PR" }, + { source: "sourcebot-ask-agent" }, + ); + + const url = "https://sourcebot.example.com/settings/skills?skill=skill-1"; + expect(JSON.parse(result.output)).toEqual({ + id: "skill-1", + slug: "review-pr", + name: "Review PR", + description: "Review a pull request.", + scope: "personal", + enabled: true, + updatedAt: "2026-01-02T00:00:00.000Z", + url, + }); + expect(result.metadata).toEqual({ + id: "skill-1", + slug: "review-pr", + name: "Review PR", + scope: "personal", + url, + }); + }); +}); diff --git a/packages/web/src/features/tools/updateSkill.ts b/packages/web/src/features/tools/updateSkill.ts new file mode 100644 index 000000000..312e0ce77 --- /dev/null +++ b/packages/web/src/features/tools/updateSkill.ts @@ -0,0 +1,104 @@ +import { z } from "zod"; +import { checkAskEntitlement } from "@/features/chat/utils.server"; +import { normalizeAgentSkillSlug } from "@/ee/features/chat/skills/types"; +import { updateAgentSkillForContext } from "@/ee/features/chat/skills/skillCreation"; +import { isServiceError } from "@/lib/utils"; +import { sew } from "@/middleware/sew"; +import { withAuth } from "@/middleware/withAuth"; +import { ToolDefinition } from "./types"; +import { logger } from "./logger"; +import { scopedTokensCannotManageSkills, skillSettingsUrl, toSkillToolError, toSkillAnalyticsSource } from "./skillToolShared"; +import description from "./updateSkill.txt"; + +// Same plain-string style as create_skill: the merged result is validated in +// the update core via agentSkillInputSchema. +const updateSkillShape = { + slug: z.string().describe("Current slash command of the skill to update, without the leading '/'."), + scope: z.enum(['personal', 'shared']).describe("Which catalog the skill lives in: 'personal' (your skills) or 'shared' (the organization catalog)."), + name: z.string().optional().describe("New display name, 1-80 characters. Omit to keep the current name."), + newSlug: z.string().optional().describe("New slash command, without the leading '/'. Lowercase letters, numbers, and hyphens; at most 64 characters. Omit to keep the current command."), + description: z.string().optional().describe("New description of when to use the skill, 1-500 characters. Omit to keep the current description."), + instructions: z.string().optional().describe("New markdown instructions, 1-20,000 characters. Omit to keep the current instructions."), +}; + +export type UpdateSkillMetadata = { + id: string; + slug: string; + name: string; + scope: 'personal' | 'shared'; + url: string; +}; + +export const updateSkillDefinition: ToolDefinition<"update_skill", typeof updateSkillShape, UpdateSkillMetadata> = { + name: "update_skill", + title: "Update skill", + isReadOnly: false, + isIdempotent: true, + isDestructive: true, + description, + inputSchema: z.object(updateSkillShape), + execute: async (input, context) => { + logger.debug('update_skill', { slug: input.slug, scope: input.scope }); + + const { slug, scope, name, newSlug, description: newDescription, instructions } = input; + + const result = await sew(() => + withAuth(async ({ org, user, prisma, principal }) => { + if (principal.source === 'scoped_access_token') { + return scopedTokensCannotManageSkills(); + } + + const askError = await checkAskEntitlement(); + if (askError) { + return askError; + } + + return updateAgentSkillForContext({ + prisma, + userId: user.id, + orgId: org.id, + target: { scope, slug: normalizeAgentSkillSlug(slug) }, + fields: { + ...(name !== undefined ? { name } : {}), + ...(newSlug !== undefined ? { slug: newSlug } : {}), + ...(newDescription !== undefined ? { description: newDescription } : {}), + ...(instructions !== undefined ? { instructions } : {}), + }, + policy: { sharedManageableBy: 'creator', allowSynced: false }, + analytics: { + source: toSkillAnalyticsSource(context.source), + entryPoint: 'agent_tool', + }, + }); + })); + + if (isServiceError(result)) { + logger.error('update_skill failed', { serviceError: result }); + throw toSkillToolError(result, { + notAuthenticatedMessage: 'Authentication is required to update skills.', + fallbackMessage: 'Failed to update skill.', + }); + } + + const url = skillSettingsUrl(result.id); + return { + output: JSON.stringify({ + id: result.id, + slug: result.slug, + name: result.name, + description: result.description, + scope, + enabled: result.enabled, + updatedAt: result.updatedAt, + url, + }), + metadata: { + id: result.id, + slug: result.slug, + name: result.name, + scope, + url, + }, + }; + }, +}; diff --git a/packages/web/src/features/tools/updateSkill.txt b/packages/web/src/features/tools/updateSkill.txt new file mode 100644 index 000000000..8fc650e95 --- /dev/null +++ b/packages/web/src/features/tools/updateSkill.txt @@ -0,0 +1,10 @@ +Updates an existing agent skill in place. + +Usage: +- ONLY call this tool when the user explicitly asks to change a skill. NEVER rewrite a skill on your own initiative. +- Identify the skill by its current `slug` (without the leading `/`) and `scope` (`personal` or `shared`). Use `list_skills` to discover both. +- All content fields (`name`, `newSlug`, `description`, `instructions`) are optional; omitted fields keep their current values. `newSlug` renames the skill's slash command. +- Personal skills are editable by their owner. Shared skills are editable only by the user who created them, and only while enabled. +- Skills synced from a repository file cannot be edited here; they must be edited in Settings → Skills so they stay linked to their source file. +- This tool never enables/disables a skill and never moves it between the personal and shared catalogs — those actions belong to Settings → Skills. +- Editing a shared skill changes it immediately for everyone in the organization who has adopted it. diff --git a/packages/web/src/lib/posthogEvents.ts b/packages/web/src/lib/posthogEvents.ts index 152a293e4..bf777c2af 100644 --- a/packages/web/src/lib/posthogEvents.ts +++ b/packages/web/src/lib/posthogEvents.ts @@ -18,11 +18,13 @@ export type AskMcpAnalyticsSource = SourcebotWebClientSource | 'sourcebot-ask-ag export type McpConnectorEntryPoint = 'chat' | 'account_settings' | 'workspace_settings' | 'unknown'; export type McpConnectorAuthMode = 'dynamic' | 'static'; export type AskSkillScope = 'personal' | 'shared'; +export type AskSkillAnalyticsSource = SourcebotWebClientSource | 'sourcebot-ask-agent' | 'sourcebot-mcp-server'; export type AskSkillEntryPoint = 'skills_settings' | 'account_ask_agent_settings' | 'workspace_ask_agent_settings' | 'chat_box' | + 'agent_tool' | 'unknown'; export type AskSkillCreationMethod = 'manual' | 'local_markdown' | 'repository'; export type AskSkillChangedField = 'name' | 'command' | 'description' | 'instructions'; @@ -281,7 +283,7 @@ export type PosthogEventMap = { durationMs: number, }, ask_skill_created: { - source: SourcebotWebClientSource, + source: AskSkillAnalyticsSource, entryPoint: AskSkillEntryPoint, scope: AskSkillScope, creationMethod: AskSkillCreationMethod, @@ -291,7 +293,7 @@ export type PosthogEventMap = { failureReason?: string, }, ask_skill_updated: { - source: SourcebotWebClientSource, + source: AskSkillAnalyticsSource, entryPoint: AskSkillEntryPoint, scope: AskSkillScope, isSynced: boolean,