From a27664cda76bd5f4b2aa4bd5e5c6d06d5ae59542 Mon Sep 17 00:00:00 2001 From: "helpcode.ai" <340500827+helpcodeai@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:20:07 +0200 Subject: [PATCH 1/8] Add per-member skill learning control and fix skill provenance (#1145) Generating skills deleted every pending skill in its scope, including Agent Skills Finder suggestions awaiting approval and manual pending ones. Skills now record their source (manual, llm, finder) and generation only replaces its own pending llm suggestions. activeSkillsText also filters by organization, and the skill endpoints validate their bodies with DTOs. Intent capture was one switch for the whole workspace. Admins can now set each member to inherit, on or off (also in bulk, optionally erasing the intents already captured), and members can opt themselves out. Only contributing callers get the _intent parameter; callers without a member identity never contribute. Changes are logged as LEARNING_CHANGED. --- docs/knowledge-graph.md | 36 ++- .../migration.sql | 19 ++ packages/backend/prisma/schema.prisma | 11 + .../src/audit/security-event.service.ts | 5 + .../kg-learning.service.spec.ts | 163 ++++++++++++++ .../knowledge-graph/kg-learning.service.ts | 211 ++++++++++++++++++ .../knowledge-graph/kg-skill.service.spec.ts | 119 ++++++++++ .../src/knowledge-graph/kg-skill.service.ts | 17 +- .../src/knowledge-graph/kg.controller.ts | 178 +++++++++++++-- .../backend/src/knowledge-graph/kg.module.ts | 5 +- .../backend/src/knowledge-graph/kg.service.ts | 11 +- .../mcp-endpoint.controller.spec.ts | 19 +- .../src/mcp-server/mcp-endpoint.controller.ts | 9 +- .../mcp-servers.instructions.spec.ts | 41 +++- .../src/mcp-servers/mcp-servers.service.ts | 34 ++- packages/backend/src/users/users.service.ts | 2 + .../src/app/settings/organization/page.tsx | 2 +- packages/frontend/src/app/settings/page.tsx | 74 +++++- .../frontend/src/app/settings/users/page.tsx | 124 +++++++++- packages/frontend/src/lib/api.ts | 29 +++ 20 files changed, 1061 insertions(+), 48 deletions(-) create mode 100644 packages/backend/prisma/migrations/20261019100000_skill_source_member_learning/migration.sql create mode 100644 packages/backend/src/knowledge-graph/kg-learning.service.spec.ts create mode 100644 packages/backend/src/knowledge-graph/kg-learning.service.ts diff --git a/docs/knowledge-graph.md b/docs/knowledge-graph.md index 959365580..1135f7e22 100644 --- a/docs/knowledge-graph.md +++ b/docs/knowledge-graph.md @@ -86,12 +86,16 @@ whole MCP server. **The loop:** -1. **Capture (optional).** With the per-workspace *intent capture* switch on, - every MCP tool gains an optional `_intent` parameter; the agent records *why* - it's calling the tool. The value is stripped before the upstream call and - stored for analysis (never sent to the API). +1. **Capture (optional).** For users who contribute to learning (see + [Who contributes](#who-contributes-to-skill-learning)), every MCP tool gains + an optional `_intent` parameter; the agent records *why* it's calling the + tool. The value is stripped before the upstream call and stored for analysis + (never sent to the API). Other users' tools do not get the parameter. 2. **Suggest.** An AI pass over the captured intents proposes skills (title, - *when to use*, *instruction*), scoped per connector or per server. + *when to use*, *instruction*), scoped per connector or per server. Generating + again replaces only the previous AI suggestions that are still pending: + skills you created by hand and skills filed by the Agent Skills Finder are + kept. 3. **Review.** On the **Skills** page each suggestion has **Apply / Edit / Dismiss / Delete**. The list has status tabs with counts (All / Suggested / Active / Dismissed), a search box and pagination, so it stays usable with @@ -109,6 +113,26 @@ whole MCP server. target selector as Generate). Pending and dismissed skills are left untouched, and a bad/empty model reply never wipes live skills. +### Who contributes to skill learning + +Whether a user's requests are captured is decided per user and per workspace: + +- **Workspace default:** the *Intent capture* switch (Settings → Organization → + Features). Users set to *Default* follow it. +- **Per user (admins):** Settings → Users → *Skill learning* sets a user to + *Default*, *On* or *Off*. Select several users to change them at once; when + turning learning off you can also erase the requests already captured from + them. +- **Opt-out (each user):** Settings → Profile → *Skill learning*. A user can + always opt out and erase their own captured requests. Removing the opt-out + does not turn learning on: the admin's setting applies again. + +A caller with no user identity (an instance-level static API key or bearer, a +client-credentials token) never contributes. Every change is recorded in the +security event log as `LEARNING_CHANGED`. Settings belong to the workspace +membership, so the same person can have different settings in two workspaces. +Cloud and self-hosted behave the same. + ### Skills are instructions, not extra tools > **Important:** applied skills do **not** add tool calls to your MCP surface. @@ -167,7 +191,7 @@ tiny built-in client (no SDK). Two ways to run them: |---|---| | Knowledge Graph | Master switch — gates build, serve, cron and the MCP tool. | | AI enrichment | Allows the LLM graph pass for this workspace. | -| Intent capture | Adds the optional `_intent` param to this workspace's tools. | +| Intent capture | Default for users set to *Default*: adds the optional `_intent` param to their tools. Override per user under Settings → Users. | | Scheduled AI extension | Lets the cloud cron extend graph + skills for this workspace. | **Instance env** (see `.env.example` for the full list): diff --git a/packages/backend/prisma/migrations/20261019100000_skill_source_member_learning/migration.sql b/packages/backend/prisma/migrations/20261019100000_skill_source_member_learning/migration.sql new file mode 100644 index 000000000..589a347e1 --- /dev/null +++ b/packages/backend/prisma/migrations/20261019100000_skill_source_member_learning/migration.sql @@ -0,0 +1,19 @@ +-- Where a workspace skill came from, so skill generation only replaces its +-- own pending suggestions (it used to delete skills filed by the Agent Skills +-- Finder and manual pending ones too). +ALTER TABLE "kg_skill_suggestions" ADD COLUMN "source" TEXT NOT NULL DEFAULT 'manual'; + +-- Backfill. Finder skills end with the "Source: (licence: …)" line that +-- skills_save_to_workspace appends; manual skills are created with +-- confidence 1, generated and consolidated ones carry the model's score. +UPDATE "kg_skill_suggestions" +SET "source" = 'finder' +WHERE "instruction" ~ E'\n\nSource: https?://\\S+ \\(licence: [^)]*\\)$'; + +UPDATE "kg_skill_suggestions" +SET "source" = 'llm' +WHERE "source" = 'manual' AND "confidence" < 1; + +-- Per-member control over skill learning (intent capture). +ALTER TABLE "organization_members" ADD COLUMN "learning_mode" TEXT NOT NULL DEFAULT 'inherit'; +ALTER TABLE "organization_members" ADD COLUMN "learning_opt_out" BOOLEAN NOT NULL DEFAULT false; diff --git a/packages/backend/prisma/schema.prisma b/packages/backend/prisma/schema.prisma index 1ef0abe06..4e24d5ff9 100644 --- a/packages/backend/prisma/schema.prisma +++ b/packages/backend/prisma/schema.prisma @@ -66,6 +66,13 @@ model OrganizationMember { /// reactivation, and so the member list can say "deactivated" instead of /// making a leaver indistinguishable from someone who was never here. deactivatedAt DateTime? @map("deactivated_at") + /// Whether this member's requests may feed skill learning (intent capture): + /// `inherit` follows the workspace switch, `on` / `off` override it. Set by + /// an admin. Per membership, so one person can differ between workspaces. + learningMode String @default("inherit") @map("learning_mode") + /// The member's own opt-out. Wins over `learningMode`; a member can opt + /// out but never opt in against the admin. + learningOptOut Boolean @default(false) @map("learning_opt_out") @@unique([userId, organizationId]) @@index([userId]) @@ -1368,6 +1375,10 @@ model KgSkillSuggestion { confidence Float @default(0) evidenceCount Int @default(0) @map("evidence_count") status String @default("pending") // pending | applied | dismissed + // Where the skill came from: manual (UI/API) | llm (generated or + // consolidated) | finder (skills_save_to_workspace). Generation only + // replaces its own pending `llm` rows. + source String @default("manual") createdAt DateTime @default(now()) @map("created_at") updatedAt DateTime @updatedAt @map("updated_at") diff --git a/packages/backend/src/audit/security-event.service.ts b/packages/backend/src/audit/security-event.service.ts index 3b65b5053..18920a9b4 100644 --- a/packages/backend/src/audit/security-event.service.ts +++ b/packages/backend/src/audit/security-event.service.ts @@ -74,6 +74,11 @@ export const SecurityEvents = { */ WORKSPACE_SESSIONS_REVOKED: 'WORKSPACE_SESSIONS_REVOKED', API_KEY_DEACTIVATED: 'API_KEY_DEACTIVATED', + /** + * Whether a member's requests may feed skill learning changed: an admin set + * their mode, or the member opted out / back in. One row per member. + */ + LEARNING_CHANGED: 'LEARNING_CHANGED', // ── Consent plane: which AI client was authorized, by whom ────────────── CONSENT_GRANTED: 'CONSENT_GRANTED', diff --git a/packages/backend/src/knowledge-graph/kg-learning.service.spec.ts b/packages/backend/src/knowledge-graph/kg-learning.service.spec.ts new file mode 100644 index 000000000..9b0205317 --- /dev/null +++ b/packages/backend/src/knowledge-graph/kg-learning.service.spec.ts @@ -0,0 +1,163 @@ +import { NotFoundException } from '@nestjs/common'; +import { KgLearningService, effectiveLearning, memberContributes } from './kg-learning.service'; + +const ORG = 'org-A'; + +describe('effectiveLearning', () => { + it.each([ + // mode, optOut, workspaceDefault, expected + ['inherit', false, true, true], + ['inherit', false, false, false], + ['on', false, false, true], + ['off', false, true, false], + ['on', true, true, false], + ['inherit', true, true, false], + ['garbage', false, true, true], + ])('%s / optOut=%s / default=%s → %s', (mode, optOut, def, expected) => { + expect(effectiveLearning(mode as string, optOut as boolean, def as boolean)).toBe(expected); + }); +}); + +describe('memberContributes', () => { + function deps(member: unknown, workspaceDefault = true) { + const prisma = { + organizationMember: { findUnique: jest.fn().mockResolvedValue(member) }, + }; + const staticSvc = { getFlag: jest.fn().mockResolvedValue(workspaceDefault) }; + return { prisma, staticSvc }; + } + + it('is false without a user (static instance credentials)', async () => { + const { prisma, staticSvc } = deps({ learningMode: 'on', learningOptOut: false, deactivatedAt: null }); + expect(await memberContributes(prisma as any, staticSvc as any, ORG, undefined)).toBe(false); + expect(prisma.organizationMember.findUnique).not.toHaveBeenCalled(); + }); + + it('is false for a caller who is not a member of the workspace', async () => { + const { prisma, staticSvc } = deps(null); + expect(await memberContributes(prisma as any, staticSvc as any, ORG, 'client:abc')).toBe(false); + }); + + it('is false for a deactivated member', async () => { + const { prisma, staticSvc } = deps({ + learningMode: 'on', + learningOptOut: false, + deactivatedAt: new Date(), + }); + expect(await memberContributes(prisma as any, staticSvc as any, ORG, 'u1')).toBe(false); + }); + + it('follows the workspace switch for inherit and looks the member up in this workspace', async () => { + const { prisma, staticSvc } = deps({ learningMode: 'inherit', learningOptOut: false, deactivatedAt: null }); + expect(await memberContributes(prisma as any, staticSvc as any, ORG, 'u1')).toBe(true); + expect(prisma.organizationMember.findUnique.mock.calls[0][0].where).toEqual({ + userId_organizationId: { userId: 'u1', organizationId: ORG }, + }); + expect(staticSvc.getFlag).toHaveBeenCalledWith(ORG, 'kg_capture_intent', false); + }); + + it('honours an admin off even when the workspace switch is on', async () => { + const { prisma, staticSvc } = deps({ learningMode: 'off', learningOptOut: false, deactivatedAt: null }); + expect(await memberContributes(prisma as any, staticSvc as any, ORG, 'u1')).toBe(false); + }); +}); + +describe('KgLearningService', () => { + const ctx = { actorUserId: 'admin', ip: '1.2.3.4', userAgent: 'jest' }; + + function make(members: Array<{ userId: string; learningMode: string; learningOptOut: boolean }>, workspaceDefault = false) { + const prisma = { + organizationMember: { + findMany: jest.fn(async ({ where }: any) => + members.filter((m) => where.userId.in.includes(m.userId)), + ), + findUnique: jest.fn(async ({ where }: any) => + members.find((m) => m.userId === where.userId_organizationId.userId) ?? null, + ), + updateMany: jest.fn().mockResolvedValue({ count: 0 }), + update: jest.fn().mockResolvedValue({}), + }, + toolInvocation: { updateMany: jest.fn().mockResolvedValue({ count: 3 }) }, + }; + const staticSvc = { getFlag: jest.fn().mockResolvedValue(workspaceDefault) }; + const securityEvents = { log: jest.fn().mockResolvedValue(undefined) }; + const svc = new KgLearningService(prisma as any, staticSvc as any, securityEvents as any); + return { svc, prisma, securityEvents }; + } + + it('sets the mode of members of this workspace only and logs one event per changed member', async () => { + const { svc, prisma, securityEvents } = make([ + { userId: 'u1', learningMode: 'inherit', learningOptOut: false }, + { userId: 'u2', learningMode: 'on', learningOptOut: false }, + ]); + const res = await svc.setMode(ORG, ['u1', 'u2', 'stranger'], 'on', ctx); + + expect(res).toEqual({ updated: 1, notFound: ['stranger'], intentsCleared: 0 }); + expect(prisma.organizationMember.findMany.mock.calls[0][0].where.organizationId).toBe(ORG); + expect(prisma.organizationMember.updateMany).toHaveBeenCalledWith({ + where: { organizationId: ORG, userId: { in: ['u1'] } }, + data: { learningMode: 'on' }, + }); + expect(securityEvents.log).toHaveBeenCalledTimes(1); + expect(securityEvents.log.mock.calls[0][0]).toEqual( + expect.objectContaining({ + event: 'LEARNING_CHANGED', + organizationId: ORG, + actorUserId: 'admin', + targetUserId: 'u1', + metadata: { from: 'inherit', to: 'on', via: 'admin' }, + }), + ); + }); + + it('clears captured intents only for members whose learning ends up off, in this workspace', async () => { + const { svc, prisma } = make([ + { userId: 'u1', learningMode: 'on', learningOptOut: false }, + { userId: 'u2', learningMode: 'on', learningOptOut: false }, + ]); + const res = await svc.setMode(ORG, ['u1', 'u2'], 'off', ctx, { clearIntents: true }); + + expect(res.intentsCleared).toBe(3); + expect(prisma.toolInvocation.updateMany).toHaveBeenCalledWith({ + where: { organizationId: ORG, userId: { in: ['u1', 'u2'] }, intent: { not: null } }, + data: { intent: null }, + }); + }); + + it('does not clear intents when the members stay on', async () => { + const { svc, prisma } = make([{ userId: 'u1', learningMode: 'off', learningOptOut: false }]); + await svc.setMode(ORG, ['u1'], 'on', ctx, { clearIntents: true }); + expect(prisma.toolInvocation.updateMany).not.toHaveBeenCalled(); + }); + + it('lets a member opt out, but opting back in only restores the admin setting', async () => { + const members = [{ userId: 'u1', learningMode: 'off', learningOptOut: true }]; + const { svc, prisma } = make(members, true); + const res = await svc.setOptOut(ORG, 'u1', false, { actorUserId: 'u1' }); + expect(prisma.organizationMember.update).toHaveBeenCalledWith({ + where: { userId_organizationId: { userId: 'u1', organizationId: ORG } }, + data: { learningOptOut: false }, + }); + // The mock row is not mutated, but the admin's "off" decides either way. + expect(res.effective).toBe(false); + }); + + it('erases the member\'s own intents when opting out with clearIntents', async () => { + const { svc, prisma } = make([{ userId: 'u1', learningMode: 'on', learningOptOut: false }]); + const res = await svc.setOptOut(ORG, 'u1', true, { actorUserId: 'u1' }, { clearIntents: true }); + expect(res.intentsCleared).toBe(3); + expect(prisma.toolInvocation.updateMany.mock.calls[0][0].where).toEqual({ + organizationId: ORG, + userId: { in: ['u1'] }, + intent: { not: null }, + }); + }); + + it('refuses a user who is not a member of the workspace', async () => { + const { svc } = make([]); + await expect(svc.getForMember(ORG, 'nobody')).rejects.toBeInstanceOf(NotFoundException); + await expect(svc.setOptOut(ORG, 'nobody', true, { actorUserId: 'nobody' })).rejects.toBeInstanceOf( + NotFoundException, + ); + }); +}); diff --git a/packages/backend/src/knowledge-graph/kg-learning.service.ts b/packages/backend/src/knowledge-graph/kg-learning.service.ts new file mode 100644 index 000000000..906b012f7 --- /dev/null +++ b/packages/backend/src/knowledge-graph/kg-learning.service.ts @@ -0,0 +1,211 @@ +import { Injectable, NotFoundException } from '@nestjs/common'; +import { PrismaService } from '../common/prisma.service'; +import { SecurityEventService, SecurityEvents } from '../audit/security-event.service'; +import { KgStaticService } from './kg-static.service'; + +export const LEARNING_MODES = ['inherit', 'on', 'off'] as const; +export type LearningMode = (typeof LEARNING_MODES)[number]; + +/** Workspace switch that members set to `inherit` follow. */ +export const LEARNING_DEFAULT_FLAG = 'kg_capture_intent'; + +/** Most members one bulk change may touch. */ +export const LEARNING_BULK_MAX = 500; + +export interface LearningActionContext { + actorUserId: string; + ip?: string; + userAgent?: string; +} + +export interface MemberLearning { + /** The workspace switch (`Capture user intent`). */ + workspaceDefault: boolean; + mode: LearningMode; + optOut: boolean; + /** Whether this member's requests are used for learning right now. */ + effective: boolean; +} + +/** + * The member's own opt-out wins; otherwise an admin's `on`/`off`, otherwise + * the workspace switch. + */ +export function effectiveLearning( + mode: string, + optOut: boolean, + workspaceDefault: boolean, +): boolean { + if (optOut) return false; + if (mode === 'on') return true; + if (mode === 'off') return false; + return workspaceDefault; +} + +/** + * Whether a caller may contribute to skill learning in a workspace: an active + * member whose effective setting is on. No user, a non-member (e.g. a + * `client:` token) or a deactivated member never contributes. + */ +export async function memberContributes( + prisma: PrismaService, + staticSvc: KgStaticService, + organizationId: string, + userId?: string | null, +): Promise { + if (!organizationId || !userId) return false; + const [workspaceDefault, member] = await Promise.all([ + staticSvc.getFlag(organizationId, LEARNING_DEFAULT_FLAG, false), + prisma.organizationMember.findUnique({ + where: { userId_organizationId: { userId, organizationId } }, + select: { learningMode: true, learningOptOut: true, deactivatedAt: true }, + }), + ]); + if (!member || member.deactivatedAt) return false; + return effectiveLearning(member.learningMode, member.learningOptOut, workspaceDefault); +} + +/** + * Who may contribute to skill learning. A contributing caller's tools carry + * the optional `_intent` parameter and their intents are stored; for anyone + * else the parameter is not offered and nothing is captured. + * + * Settings live on the membership, so the same person can differ between + * workspaces (Cloud) and a caller without a member identity (static instance + * credentials, client-credentials tokens) never contributes. + */ +@Injectable() +export class KgLearningService { + constructor( + private readonly prisma: PrismaService, + private readonly staticSvc: KgStaticService, + private readonly securityEvents: SecurityEventService, + ) {} + + workspaceDefault(organizationId: string): Promise { + return this.staticSvc.getFlag(organizationId, LEARNING_DEFAULT_FLAG, false); + } + + /** Whether this caller's tool calls may capture intent. Fails closed. */ + canContribute(organizationId: string, userId?: string | null): Promise { + return memberContributes(this.prisma, this.staticSvc, organizationId, userId); + } + + async getForMember(organizationId: string, userId: string): Promise { + const [workspaceDefault, member] = await Promise.all([ + this.workspaceDefault(organizationId), + this.prisma.organizationMember.findUnique({ + where: { userId_organizationId: { userId, organizationId } }, + select: { learningMode: true, learningOptOut: true }, + }), + ]); + if (!member) throw new NotFoundException('Member not found.'); + const mode = toMode(member.learningMode); + return { + workspaceDefault, + mode, + optOut: member.learningOptOut, + effective: effectiveLearning(mode, member.learningOptOut, workspaceDefault), + }; + } + + /** + * Admin: set the mode of one or more members of this workspace. Ids that + * are not members here are ignored (reported as `notFound`), never touched. + * `clearIntents` also erases the intents already captured from members + * whose learning ends up off. + */ + async setMode( + organizationId: string, + userIds: string[], + mode: LearningMode, + ctx: LearningActionContext, + opts?: { clearIntents?: boolean }, + ): Promise<{ updated: number; notFound: string[]; intentsCleared: number }> { + const ids = [...new Set(userIds)].slice(0, LEARNING_BULK_MAX); + const members = await this.prisma.organizationMember.findMany({ + where: { organizationId, userId: { in: ids } }, + select: { userId: true, learningMode: true, learningOptOut: true }, + }); + const found = new Set(members.map((m) => m.userId)); + const notFound = ids.filter((id) => !found.has(id)); + const changed = members.filter((m) => m.learningMode !== mode); + + if (changed.length > 0) { + await this.prisma.organizationMember.updateMany({ + where: { organizationId, userId: { in: changed.map((m) => m.userId) } }, + data: { learningMode: mode }, + }); + } + for (const m of changed) { + await this.securityEvents.log({ + event: SecurityEvents.LEARNING_CHANGED, + actorType: 'USER', + organizationId, + actorUserId: ctx.actorUserId, + targetUserId: m.userId, + metadata: { from: m.learningMode, to: mode, via: 'admin' }, + ip: ctx.ip, + userAgent: ctx.userAgent, + }); + } + + let intentsCleared = 0; + if (opts?.clearIntents) { + const workspaceDefault = await this.workspaceDefault(organizationId); + const off = members + .filter((m) => !effectiveLearning(mode, m.learningOptOut, workspaceDefault)) + .map((m) => m.userId); + intentsCleared = await this.clearIntents(organizationId, off); + } + return { updated: changed.length, notFound, intentsCleared }; + } + + /** Member: opt out of (or back into) learning in this workspace. */ + async setOptOut( + organizationId: string, + userId: string, + optOut: boolean, + ctx: LearningActionContext, + opts?: { clearIntents?: boolean }, + ): Promise { + const member = await this.prisma.organizationMember.findUnique({ + where: { userId_organizationId: { userId, organizationId } }, + select: { learningOptOut: true }, + }); + if (!member) throw new NotFoundException('Member not found.'); + if (member.learningOptOut !== optOut) { + await this.prisma.organizationMember.update({ + where: { userId_organizationId: { userId, organizationId } }, + data: { learningOptOut: optOut }, + }); + await this.securityEvents.log({ + event: SecurityEvents.LEARNING_CHANGED, + actorType: 'USER', + organizationId, + actorUserId: ctx.actorUserId, + targetUserId: userId, + metadata: { optOut, via: 'self' }, + ip: ctx.ip, + userAgent: ctx.userAgent, + }); + } + const intentsCleared = + optOut && opts?.clearIntents ? await this.clearIntents(organizationId, [userId]) : 0; + return { ...(await this.getForMember(organizationId, userId)), intentsCleared }; + } + + /** Erase captured intents of these members in this workspace only. */ + private async clearIntents(organizationId: string, userIds: string[]): Promise { + if (userIds.length === 0) return 0; + const res = await this.prisma.toolInvocation.updateMany({ + where: { organizationId, userId: { in: userIds }, intent: { not: null } }, + data: { intent: null }, + }); + return res.count; + } +} + +function toMode(value: string): LearningMode { + return (LEARNING_MODES as readonly string[]).includes(value) ? (value as LearningMode) : 'inherit'; +} diff --git a/packages/backend/src/knowledge-graph/kg-skill.service.spec.ts b/packages/backend/src/knowledge-graph/kg-skill.service.spec.ts index 02b2bdd1d..3e5dc0921 100644 --- a/packages/backend/src/knowledge-graph/kg-skill.service.spec.ts +++ b/packages/backend/src/knowledge-graph/kg-skill.service.spec.ts @@ -162,3 +162,122 @@ describe('KgSkillService custom skip', () => { expect(prisma.kgSkillSuggestion.deleteMany).not.toHaveBeenCalled(); }); }); + +/** + * Generation used to delete every pending skill in its scope, including + * Agent Skills Finder suggestions awaiting approval and manual pending ones. + * It now replaces only its own previous output (`source: 'llm'`). + */ +describe('KgSkillService provenance', () => { + const ORG = 'org-A'; + const realFetch = global.fetch; + + beforeEach(() => { + process.env.KG_LLM_BASE_URL = 'http://localhost:11434/v1'; + process.env.KG_LLM_MODEL = 'qwen2.5'; + delete process.env.KG_LLM_API_KEY; + global.fetch = jest.fn().mockResolvedValue( + okJson({ + choices: [ + { + message: { + content: JSON.stringify({ skills: [{ title: 'T', instruction: 'I', confidence: 0.5 }] }), + }, + }, + ], + }), + ); + }); + afterEach(() => { + global.fetch = realFetch; + delete process.env.KG_LLM_BASE_URL; + delete process.env.KG_LLM_MODEL; + }); + + it('server generation deletes only its own pending llm rows and tags new ones llm', async () => { + const prisma = { + mcpServerConfig: { + findUnique: jest.fn().mockResolvedValue({ id: 'srv', name: 'S', organizationId: ORG }), + }, + mcpServerConnector: { findMany: jest.fn().mockResolvedValue([]) }, + kgNode: { findMany: jest.fn().mockResolvedValue([]) }, + toolInvocation: { findMany: jest.fn().mockResolvedValue([]) }, + orgSettings: { findUnique: jest.fn().mockResolvedValue(null) }, + kgSkillSuggestion: { deleteMany: jest.fn(), create: jest.fn() }, + }; + const svc = new KgSkillService(prisma as any, { isEnabled: async () => true } as any); + await svc.generate(ORG, { mcpServerId: 'srv' }); + + expect(prisma.kgSkillSuggestion.deleteMany).toHaveBeenCalledWith({ + where: { organizationId: ORG, status: 'pending', source: 'llm', mcpServerId: 'srv' }, + }); + expect(prisma.kgSkillSuggestion.create.mock.calls[0][0].data).toEqual( + expect.objectContaining({ source: 'llm', status: 'pending', mcpServerId: 'srv' }), + ); + }); + + it('connector generation deletes only its own pending llm rows', async () => { + const prisma = { + toolInvocation: { + findMany: jest.fn().mockResolvedValue([ + { intent: 'x', status: 'SUCCESS', tool: { name: 't', connector: { name: 'C' } } }, + ]), + }, + connector: { findMany: jest.fn().mockResolvedValue([]) }, + orgSettings: { findUnique: jest.fn().mockResolvedValue(null) }, + kgSkillSuggestion: { deleteMany: jest.fn(), create: jest.fn() }, + }; + const svc = new KgSkillService(prisma as any, { isEnabled: async () => true } as any); + await svc.generate(ORG); + + expect(prisma.kgSkillSuggestion.deleteMany).toHaveBeenCalledWith({ + where: { organizationId: ORG, status: 'pending', source: 'llm', mcpServerId: null }, + }); + }); +}); + +describe('KgSkillService.create source', () => { + const ORG = 'org-A'; + + function prismaFor() { + return { + mcpServerConfig: { findUnique: jest.fn().mockResolvedValue({ organizationId: ORG }) }, + kgSkillSuggestion: { create: jest.fn().mockResolvedValue({ id: 's1' }) }, + }; + } + + it('records manual by default', async () => { + const prisma = prismaFor(); + await new KgSkillService(prisma as any, {} as any).create(ORG, { + title: 'T', + instruction: 'I', + mcpServerId: 'srv', + }); + expect(prisma.kgSkillSuggestion.create.mock.calls[0][0].data.source).toBe('manual'); + }); + + it('records finder when the Skills Finder files it', async () => { + const prisma = prismaFor(); + await new KgSkillService(prisma as any, {} as any).create(ORG, { + title: 'T', + instruction: 'I', + mcpServerId: 'srv', + status: 'pending', + source: 'finder', + }); + expect(prisma.kgSkillSuggestion.create.mock.calls[0][0].data).toEqual( + expect.objectContaining({ source: 'finder', status: 'pending' }), + ); + }); +}); + +describe('KgSkillService.activeSkillsText', () => { + it('filters by organization as well as by the given ids', async () => { + const findMany = jest.fn().mockResolvedValue([]); + const svc = new KgSkillService({ kgSkillSuggestion: { findMany } } as any, {} as any); + await svc.activeSkillsText('org-A', 'srv-1', ['c-1']); + expect(findMany.mock.calls[0][0].where).toEqual( + expect.objectContaining({ organizationId: 'org-A', status: 'applied' }), + ); + }); +}); diff --git a/packages/backend/src/knowledge-graph/kg-skill.service.ts b/packages/backend/src/knowledge-graph/kg-skill.service.ts index 6acc867f1..6430cc4ef 100644 --- a/packages/backend/src/knowledge-graph/kg-skill.service.ts +++ b/packages/backend/src/knowledge-graph/kg-skill.service.ts @@ -6,6 +6,9 @@ import { maybeRedactIntent } from './redact'; const MAX_INTENTS = 200; +/** Where a skill came from; generation only ever replaces its own `llm` rows. */ +export type SkillSource = 'manual' | 'llm' | 'finder'; + /** Auto-apply a generated skill only when at/above this confidence. */ export const SKILL_AUTO_APPLY_MIN = 0.9; @@ -129,8 +132,10 @@ export class KgSkillService { const idByName = new Map(connectors.map((c) => [c.name.toLowerCase(), c.id])); const autoApply = await this.autoApplyEnabled(organizationId); + // Replace only the previous generation's pending output: manual pending + // skills and Finder suggestions awaiting approval are not ours to drop. await this.prisma.kgSkillSuggestion.deleteMany({ - where: { organizationId, status: 'pending', mcpServerId: null }, + where: { organizationId, status: 'pending', source: 'llm', mcpServerId: null }, }); let created = 0; @@ -196,7 +201,7 @@ export class KgSkillService { const skills: any[] = Array.isArray(json?.skills) ? json.skills : []; await this.prisma.kgSkillSuggestion.deleteMany({ - where: { organizationId, status: 'pending', mcpServerId }, + where: { organizationId, status: 'pending', source: 'llm', mcpServerId }, }); const autoApply = await this.autoApplyEnabled(organizationId); @@ -232,6 +237,7 @@ export class KgSkillService { confidence, evidenceCount: Math.max(0, Math.min(9999, parseInt(s?.evidenceCount, 10) || 0)), status, + source: 'llm', }, }); return true; @@ -373,6 +379,7 @@ export class KgSkillService { confidence: Math.max(0, Math.min(1, Number(s?.confidence) || 0.8)), evidenceCount: 0, status: 'applied', + source: 'llm', }, }); n++; @@ -386,12 +393,16 @@ export class KgSkillService { /** Active skills composed into an MCP server's instructions at serve time. */ async activeSkillsText( + organizationId: string, serverId: string | string[], connectorIds: string[], ): Promise { const serverIds = Array.isArray(serverId) ? serverId : [serverId]; const skills = await this.prisma.kgSkillSuggestion.findMany({ where: { + // Re-stated rather than trusted from the ids, so a stray id can never + // pull another workspace's skills into these instructions. + organizationId, status: 'applied', OR: [ { mcpServerId: { in: serverIds.length ? serverIds : ['__none__'] } }, @@ -422,6 +433,7 @@ export class KgSkillService { connectorId?: string | null; mcpServerId?: string | null; status?: string; + source?: SkillSource; }, ) { const title = (body.title || '').trim().slice(0, 160); @@ -466,6 +478,7 @@ export class KgSkillService { confidence: 1, evidenceCount: 0, status, + source: body.source === 'finder' ? 'finder' : 'manual', }, }); } diff --git a/packages/backend/src/knowledge-graph/kg.controller.ts b/packages/backend/src/knowledge-graph/kg.controller.ts index 21bec3f78..7537305d6 100644 --- a/packages/backend/src/knowledge-graph/kg.controller.ts +++ b/packages/backend/src/knowledge-graph/kg.controller.ts @@ -12,10 +12,124 @@ import { UseGuards, } from '@nestjs/common'; import { AuthGuard } from '@nestjs/passport'; -import { ApiBearerAuth, ApiOperation, ApiTags } from '@nestjs/swagger'; +import { ApiBearerAuth, ApiOperation, ApiProperty, ApiPropertyOptional, ApiTags } from '@nestjs/swagger'; +import { + ArrayMaxSize, + ArrayNotEmpty, + IsArray, + IsBoolean, + IsIn, + IsOptional, + IsString, +} from 'class-validator'; import { Roles, RolesGuard } from '../auth/roles.guard'; import { KgService } from './kg.service'; import { KgSkillService } from './kg-skill.service'; +import { + KgLearningService, + LEARNING_BULK_MAX, + LEARNING_MODES, + type LearningMode, +} from './kg-learning.service'; + +const SKILL_STATUSES = ['pending', 'applied', 'dismissed']; + +class CreateSkillDto { + @ApiProperty({ description: 'Short name of the skill (stored up to 160 characters).' }) + @IsString() + title: string; + + @ApiPropertyOptional({ description: 'When the model should apply it.' }) + @IsOptional() + @IsString() + whenToUse?: string; + + @ApiProperty({ description: 'The rule or procedure (stored up to 2000 characters).' }) + @IsString() + instruction: string; + + @ApiPropertyOptional({ description: 'Scope the skill to one connector of this workspace.' }) + @IsOptional() + @IsString() + connectorId?: string | null; + + @ApiPropertyOptional({ description: 'Scope the skill to one MCP server of this workspace (wins over connectorId).' }) + @IsOptional() + @IsString() + mcpServerId?: string | null; + + @ApiPropertyOptional({ enum: SKILL_STATUSES, description: 'Defaults to applied.' }) + @IsOptional() + @IsIn(SKILL_STATUSES) + status?: string; +} + +class UpdateSkillDto { + @ApiPropertyOptional() + @IsOptional() + @IsString() + title?: string; + + @ApiPropertyOptional() + @IsOptional() + @IsString() + whenToUse?: string; + + @ApiPropertyOptional() + @IsOptional() + @IsString() + instruction?: string; + + @ApiPropertyOptional({ enum: SKILL_STATUSES }) + @IsOptional() + @IsIn(SKILL_STATUSES) + status?: string; +} + +class SkillScopeDto { + @ApiPropertyOptional({ description: 'Work on one MCP server; omit for the connector-scoped skills.' }) + @IsOptional() + @IsString() + mcpServerId?: string; +} + +class SetMembersLearningDto { + @ApiProperty({ type: [String], description: `Member user ids of this workspace (max ${LEARNING_BULK_MAX}).` }) + @IsArray() + @ArrayNotEmpty() + @ArrayMaxSize(LEARNING_BULK_MAX) + @IsString({ each: true }) + userIds: string[]; + + @ApiProperty({ + enum: LEARNING_MODES, + description: 'inherit = follow the workspace switch; on / off = override it for these members.', + }) + @IsIn(LEARNING_MODES as unknown as string[]) + mode: LearningMode; + + @ApiPropertyOptional({ + description: 'Also erase the intents already captured from members whose learning ends up off.', + default: false, + }) + @IsOptional() + @IsBoolean() + clearIntents?: boolean; +} + +class SetMyLearningDto { + @ApiProperty({ description: 'true = do not use my requests for skill learning in this workspace.' }) + @IsBoolean() + optOut: boolean; + + @ApiPropertyOptional({ + description: 'When opting out, also erase the intents already captured from me.', + default: false, + }) + @IsOptional() + @IsBoolean() + clearIntents?: boolean; +} @ApiTags('Knowledge Graph') @ApiBearerAuth() @@ -25,6 +139,7 @@ export class KgController { constructor( private readonly kg: KgService, private readonly skills: KgSkillService, + private readonly learning: KgLearningService, ) {} @Get() @@ -96,17 +211,9 @@ export class KgController { @ApiOperation({ summary: 'Create a skill manually (live for MCP by default)' }) async createSkill( @Req() req: any, - @Body() - body: { - title: string; - whenToUse?: string; - instruction: string; - connectorId?: string | null; - mcpServerId?: string | null; - status?: string; - }, + @Body() body: CreateSkillDto, ) { - return this.skills.create(req.user.organizationId, body); + return this.skills.create(req.user.organizationId, { ...body, source: 'manual' }); } @Post('skills/generate') @@ -115,7 +222,7 @@ export class KgController { summary: 'Generate skill suggestions from captured intents — per-connector, or server-wide when mcpServerId is given', }) - async generateSkills(@Req() req: any, @Body() body: { mcpServerId?: string }) { + async generateSkills(@Req() req: any, @Body() body: SkillScopeDto) { return this.skills.generate(req.user.organizationId, { mcpServerId: body?.mcpServerId }); } @@ -124,7 +231,7 @@ export class KgController { @ApiOperation({ summary: 'Merge the active skills in a scope into the fewest non-redundant ones (AI)', }) - async consolidateSkills(@Req() req: any, @Body() body: { mcpServerId?: string }) { + async consolidateSkills(@Req() req: any, @Body() body: SkillScopeDto) { return this.skills.consolidate(req.user.organizationId, { mcpServerId: body?.mcpServerId }); } @@ -148,11 +255,46 @@ export class KgController { async updateSkill( @Req() req: any, @Param('id') id: string, - @Body() body: { title?: string; whenToUse?: string; instruction?: string; status?: string }, + @Body() body: UpdateSkillDto, ) { return this.skills.update(req.user.organizationId, id, body); } + // ── Who contributes to skill learning ──────────────────────────────────── + + @Get('learning/me') + @ApiOperation({ summary: 'Whether my requests are used for skill learning in this workspace' }) + async getMyLearning(@Req() req: any) { + return this.learning.getForMember(req.user.organizationId, req.user.sub); + } + + @Put('learning/me') + @ApiOperation({ + summary: 'Opt out of (or back into) skill learning in this workspace. Cannot override an admin\'s "off".', + }) + async setMyLearning(@Req() req: any, @Body() body: SetMyLearningDto) { + return this.learning.setOptOut( + req.user.organizationId, + req.user.sub, + body.optOut, + this.actionContext(req), + { clearIntents: body.clearIntents }, + ); + } + + @Put('learning/members') + @Roles('ADMIN') + @ApiOperation({ summary: 'Set whether these members\' requests are used for skill learning (ADMIN)' }) + async setMembersLearning(@Req() req: any, @Body() body: SetMembersLearningDto) { + return this.learning.setMode( + req.user.organizationId, + body.userIds, + body.mode, + this.actionContext(req), + { clearIntents: body.clearIntents }, + ); + } + @Delete('skills/:id') @Roles('ADMIN') @ApiOperation({ summary: 'Delete a skill' }) @@ -229,4 +371,12 @@ export class KgController { async deleteNode(@Req() req: any, @Param('id') id: string) { return this.kg.deleteNode(req.user.organizationId, id); } + + private actionContext(req: any) { + return { + actorUserId: req.user.sub as string, + ip: req.ip as string | undefined, + userAgent: req.headers?.['user-agent'] as string | undefined, + }; + } } diff --git a/packages/backend/src/knowledge-graph/kg.module.ts b/packages/backend/src/knowledge-graph/kg.module.ts index c3933a704..5773f7d81 100644 --- a/packages/backend/src/knowledge-graph/kg.module.ts +++ b/packages/backend/src/knowledge-graph/kg.module.ts @@ -5,6 +5,7 @@ import { KgStaticService } from './kg-static.service'; import { KgObservationalService } from './kg-observational.service'; import { KgLlmService } from './kg-llm.service'; import { KgSkillService } from './kg-skill.service'; +import { KgLearningService } from './kg-learning.service'; /** * Knowledge Graph module. PrismaService, RedisService, DeploymentService and @@ -14,7 +15,7 @@ import { KgSkillService } from './kg-skill.service'; @Global() @Module({ controllers: [KgController], - providers: [KgService, KgStaticService, KgObservationalService, KgLlmService, KgSkillService], - exports: [KgService, KgStaticService, KgObservationalService, KgLlmService, KgSkillService], + providers: [KgService, KgStaticService, KgObservationalService, KgLlmService, KgSkillService, KgLearningService], + exports: [KgService, KgStaticService, KgObservationalService, KgLlmService, KgSkillService, KgLearningService], }) export class KgModule {} diff --git a/packages/backend/src/knowledge-graph/kg.service.ts b/packages/backend/src/knowledge-graph/kg.service.ts index 2ac20b6e8..9a37590b4 100644 --- a/packages/backend/src/knowledge-graph/kg.service.ts +++ b/packages/backend/src/knowledge-graph/kg.service.ts @@ -9,6 +9,7 @@ import { PrismaService } from '../common/prisma.service'; import { RedisService } from '../common/redis.service'; import { RolesService } from '../roles/roles.service'; import { KgStaticService } from './kg-static.service'; +import { memberContributes } from './kg-learning.service'; import { KgObservationalService } from './kg-observational.service'; import { KgLlmService } from './kg-llm.service'; @@ -128,9 +129,13 @@ export class KgService { } } - /** Whether intent capture is on for this org (used by the MCP path). */ - captureIntentEnabled(organizationId: string): Promise { - return this.staticSvc.getFlag(organizationId, 'kg_capture_intent', false); + /** + * Whether this caller's tool calls capture intent (used by the MCP path): + * the workspace switch, overridden per member by an admin and by the + * member's own opt-out. A caller without a member identity never does. + */ + captureIntentEnabled(organizationId: string, userId?: string | null): Promise { + return memberContributes(this.prisma, this.staticSvc, organizationId, userId); } /** diff --git a/packages/backend/src/mcp-server/mcp-endpoint.controller.spec.ts b/packages/backend/src/mcp-server/mcp-endpoint.controller.spec.ts index 9270503e5..12fbe59d8 100644 --- a/packages/backend/src/mcp-server/mcp-endpoint.controller.spec.ts +++ b/packages/backend/src/mcp-server/mcp-endpoint.controller.spec.ts @@ -747,6 +747,7 @@ describe('McpEndpointController — skills_save_to_workspace', () => { 'Source: https://github.com/bytedance/deer-flow/tree/8bb14fa/skills/public/code-documentation (licence: MIT)', mcpServerId: 'srv-A', status: 'pending', + source: 'finder', }); expect(JSON.parse(result.content[0].text)).toMatchObject({ saved: true, id: 'sk-1', status: 'pending' }); }); @@ -931,7 +932,7 @@ describe('McpEndpointController — resources and instructions follow the tool p ]; const kgSkills = { // A connector-scoped skill on the Fibu connector. - activeSkillsText: jest.fn(async (_srv: string, ids: string[]) => + activeSkillsText: jest.fn(async (_org: string, _srv: string, ids: string[]) => ids.includes('c-fibu') ? '## Workspace skills\n- FIBU-SKILL' : null, ), }; @@ -1047,6 +1048,22 @@ describe('McpEndpointController — resources and instructions follow the tool p await client.close(); }); + it('offers _intent only to a caller who contributes to learning, checked per member', async () => { + kgService.captureIntentEnabled.mockResolvedValue(false); + const off = await connect(null); + const offTools = (await off.listTools()).tools; + expect(offTools.length).toBeGreaterThan(0); + for (const t of offTools) expect(t.inputSchema.properties ?? {}).not.toHaveProperty('_intent'); + expect(kgService.captureIntentEnabled).toHaveBeenCalledWith(ORG_A, 'u-1'); + await off.close(); + + kgService.captureIntentEnabled.mockResolvedValue(true); + const on = await connect(null); + const onTools = (await on.listTools()).tools.filter((t) => t.name === 'crm_get'); + expect(onTools[0].inputSchema.properties).toHaveProperty('_intent'); + await on.close(); + }); + it('a caller who sees everything keeps the full instructions', async () => { const client = await connect(null); const instructions = client.getInstructions() ?? ''; diff --git a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts index cbdfff499..6b06f5947 100644 --- a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts +++ b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts @@ -1094,9 +1094,13 @@ export class McpEndpointController { // Optional: ask the calling agent to pass the user's originating request on // every tool call, so we capture the intent/context behind it (used later to - // optimize the graph and suggest skills). Per-workspace switch, default off. + // optimize the graph and suggest skills). Workspace switch, default off, + // overridden per member; only callers who contribute get `_intent`. const captureIntent = invocationContext.organizationId - ? await this.kgService.captureIntentEnabled(invocationContext.organizationId) + ? await this.kgService.captureIntentEnabled( + invocationContext.organizationId, + invocationContext.userId, + ) : false; const kgEnabled = process.env.KG_MCP_TOOL !== 'off' && !!invocationContext.organizationId @@ -1748,6 +1752,7 @@ export class McpEndpointController { instruction: `${args.instruction.trim()}\n\nSource: ${args.sourceUrl} (licence: ${args.license || 'unknown'})`, mcpServerId: serverId, status: 'pending', + source: 'finder', }); const result = { saved: true, diff --git a/packages/backend/src/mcp-servers/mcp-servers.instructions.spec.ts b/packages/backend/src/mcp-servers/mcp-servers.instructions.spec.ts index 657bc9681..b23ddf44e 100644 --- a/packages/backend/src/mcp-servers/mcp-servers.instructions.spec.ts +++ b/packages/backend/src/mcp-servers/mcp-servers.instructions.spec.ts @@ -24,7 +24,7 @@ describe('McpServersService — caller-scoped instructions and resources', () => mcpResource: { findMany: jest.fn().mockResolvedValue(resourceRows) }, }; const kgSkills = { - activeSkillsText: jest.fn(async (_s: string, ids: string[]) => + activeSkillsText: jest.fn(async (_o: string, _s: string, ids: string[]) => ids.length ? `## Workspace skills\n- for ${ids.join(',')}` : '## Workspace skills\n- server-wide', ), }; @@ -46,7 +46,7 @@ describe('McpServersService — caller-scoped instructions and resources', () => expect(text).toContain('Use the CRM.'); expect(text).not.toContain('Ledger rules.'); expect(text).not.toContain('Fibu'); - expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('srv-1', ['c-crm']); + expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', 'srv-1', ['c-crm']); }); it('scopes the assignment query to the visible ids AND the server organization', async () => { @@ -64,7 +64,7 @@ describe('McpServersService — caller-scoped instructions and resources', () => const text = await service.getComposedInstructions('srv-1', []); expect(text).toBe('SERVER-GUIDANCE\n\n## Workspace skills\n- server-wide'); expect(prisma.mcpServerConnector.findMany).not.toHaveBeenCalled(); - expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('srv-1', []); + expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', 'srv-1', []); }); it('getVisibleContent returns the visible connectors with instructions and their stored rows', async () => { @@ -110,3 +110,38 @@ describe('McpServersService — caller-scoped instructions and resources', () => expect(prisma.mcpResource.findMany).not.toHaveBeenCalled(); }); }); + +/** + * A shared /mcp connection can reach connectors of several workspaces. Each + * workspace's skills are read with its own organization and only its own + * server and connector ids. + */ +describe('McpServersService — shared guide skills per workspace', () => { + it('reads each workspace\'s skills with its own organization and ids', async () => { + const prisma = { + mcpServerConfig: { + findMany: jest.fn().mockResolvedValue([ + { id: 'srv-a', name: 'A', instructions: null, organizationId: 'org-A' }, + ]), + }, + connector: { + findMany: jest.fn().mockResolvedValue([ + { id: 'c-a', name: 'CRM', instructions: null, organizationId: 'org-A' }, + { id: 'c-b', name: 'Fibu', instructions: null, organizationId: 'org-B' }, + ]), + }, + }; + const kgSkills = { + activeSkillsText: jest.fn(async (org: string) => `## Workspace skills\n- ${org}`), + }; + const service = new McpServersService(prisma as any, kgSkills as any, {} as any); + + const text = await service.getSharedGuide({ connectorIds: ['c-a', 'c-b'], serverIds: ['srv-a'] }); + + expect(kgSkills.activeSkillsText).toHaveBeenCalledTimes(2); + expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', ['srv-a'], ['c-a']); + expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-B', [], ['c-b']); + expect(text).toContain('- org-A'); + expect(text).toContain('- org-B'); + }); +}); diff --git a/packages/backend/src/mcp-servers/mcp-servers.service.ts b/packages/backend/src/mcp-servers/mcp-servers.service.ts index bd280b22a..eb21cbaee 100644 --- a/packages/backend/src/mcp-servers/mcp-servers.service.ts +++ b/packages/backend/src/mcp-servers/mcp-servers.service.ts @@ -366,13 +366,15 @@ export class McpServersService { serverIds.length ? this.prisma.mcpServerConfig.findMany({ where: { id: { in: serverIds } }, - select: { name: true, instructions: true }, + select: { id: true, name: true, instructions: true, organizationId: true }, orderBy: { createdAt: 'asc' }, }) - : Promise.resolve([]), + : Promise.resolve( + [] as Array<{ id: string; name: string; instructions: string | null; organizationId: string }>, + ), this.prisma.connector.findMany({ where: { id: { in: connectorIds } }, - select: { name: true, instructions: true }, + select: { id: true, name: true, instructions: true, organizationId: true }, orderBy: { name: 'asc' }, }), ]); @@ -384,8 +386,17 @@ export class McpServersService { for (const c of connectors) { if (c.instructions) parts.push(`## ${c.name}\n${c.instructions}`); } - const skillsText = await this.kgSkills.activeSkillsText(serverIds, connectorIds); - if (skillsText) parts.push(skillsText); + // A shared connection can reach several workspaces: read each one's + // skills with its own organization and only its own server/connector ids. + const orgIds = [...new Set(connectors.map((c) => c.organizationId))]; + for (const orgId of orgIds) { + const skillsText = await this.kgSkills.activeSkillsText( + orgId, + servers.filter((s) => s.organizationId === orgId).map((s) => s.id), + connectors.filter((c) => c.organizationId === orgId).map((c) => c.id), + ); + if (skillsText) parts.push(skillsText); + } return parts.length > 0 ? parts.join('\n\n') : undefined; } @@ -500,7 +511,7 @@ export class McpServersService { private async composeInstructions( serverId: string, - server: { instructions: string | null } | null, + server: { instructions: string | null; organizationId: string } | null, serverConnectors: Array<{ connectorId: string; connector: { name: string; instructions: string | null }; @@ -521,10 +532,13 @@ export class McpServersService { // Compose applied skills (server-scoped + the visible connectors' // connector-scoped ones) dynamically, so editing/deleting a skill takes // effect immediately. - const skillsText = await this.kgSkills.activeSkillsText( - serverId, - serverConnectors.map((sc) => sc.connectorId), - ); + const skillsText = server + ? await this.kgSkills.activeSkillsText( + server.organizationId, + serverId, + serverConnectors.map((sc) => sc.connectorId), + ) + : null; if (skillsText) parts.push(skillsText); return parts.length > 0 ? parts.join('\n\n') : undefined; diff --git a/packages/backend/src/users/users.service.ts b/packages/backend/src/users/users.service.ts index 62dc1d636..4fd75ccf6 100644 --- a/packages/backend/src/users/users.service.ts +++ b/packages/backend/src/users/users.service.ts @@ -79,6 +79,8 @@ export class UsersService { joinedAt: m.joinedAt, deactivatedAt: m.deactivatedAt, active: m.deactivatedAt === null, + learningMode: m.learningMode, + learningOptOut: m.learningOptOut, })); } diff --git a/packages/frontend/src/app/settings/organization/page.tsx b/packages/frontend/src/app/settings/organization/page.tsx index cb64d75dc..6a7496c64 100644 --- a/packages/frontend/src/app/settings/organization/page.tsx +++ b/packages/frontend/src/app/settings/organization/page.tsx @@ -268,7 +268,7 @@ export default function OrganizationSettingsPage() { (null); const [marketingSaving, setMarketingSaving] = useState(false); + const [learning, setLearning] = useState(null); + const [learningSaving, setLearningSaving] = useState(false); // Delete account const [deleteOpen, setDeleteOpen] = useState(false); @@ -65,6 +67,35 @@ export default function SettingsPage() { .catch(() => setMarketingOptOut(null)); }, [isCloud, token]); + useEffect(() => { + if (!token) return; + knowledgeGraph.learning + .me(token) + .then(setLearning) + .catch(() => setLearning(null)); + }, [token]); + + const handleLearningOptOut = async (optOut: boolean, clearIntents = false) => { + if (!token) return; + setLearningSaving(true); + try { + const res = await knowledgeGraph.learning.setMine(token, { optOut, clearIntents }); + setLearning(res); + toast.show({ + tone: 'success', + title: clearIntents + ? `${res.intentsCleared} captured request(s) erased` + : optOut + ? 'Your requests are no longer used for skill learning' + : 'Your choice was removed; your admin\'s setting applies', + }); + } catch (err: any) { + toast.show({ tone: 'error', title: 'Could not save', description: err?.message }); + } finally { + setLearningSaving(false); + } + }; + const handleMarketingToggle = async (wantsEmails: boolean) => { if (!token) return; setMarketingSaving(true); @@ -272,6 +303,47 @@ export default function SettingsPage() { )} + {/* Skill learning: shown once it can concern this user (workspace switch + on, an admin turned it on for them, or they already opted out). */} + {learning && (learning.workspaceDefault || learning.mode === 'on' || learning.optOut) && ( + +

Skill learning

+

+ {learning.effective + ? 'Your requests to AI clients connected to this workspace are recorded with the tool calls they lead to, so admins can turn recurring needs into workspace skills.' + : learning.optOut + ? 'You opted out: your requests are not recorded for skill learning in this workspace.' + : 'Your admin has turned skill learning off for you in this workspace.'} +

+ + {learning.optOut && ( + + )} +
+ )} + {/* Change Password */}

Change Password

diff --git a/packages/frontend/src/app/settings/users/page.tsx b/packages/frontend/src/app/settings/users/page.tsx index dcb6fac27..c07e26466 100644 --- a/packages/frontend/src/app/settings/users/page.tsx +++ b/packages/frontend/src/app/settings/users/page.tsx @@ -4,7 +4,7 @@ import Link from 'next/link'; import { useEffect, useState } from 'react'; import * as Dialog from '@radix-ui/react-dialog'; import { useAuth } from '@/lib/auth-context'; -import { users, auth, roles } from '@/lib/api'; +import { users, auth, roles, knowledgeGraph, type LearningMode } from '@/lib/api'; import { AppSelect } from '@/components/ui/select'; import { Button } from '@/components/ui/button'; import { Card } from '@/components/ui/card'; @@ -14,6 +14,14 @@ import { EnterpriseNotice } from '@/components/enterprise-notice'; const ROLES = ['ADMIN', 'EDITOR', 'VIEWER'] as const; +function learningOptions(workspaceDefault: boolean) { + return [ + { value: 'inherit', label: `Default (${workspaceDefault ? 'on' : 'off'})` }, + { value: 'on', label: 'On' }, + { value: 'off', label: 'Off' }, + ]; +} + function getInvitationStatus(invite: any): 'pending' | 'expired' { return new Date(invite.expiresAt) < new Date() ? 'expired' : 'pending'; } @@ -44,15 +52,23 @@ export default function SettingsUsersPage() { const [revoking, setRevoking] = useState(false); const [revokeError, setRevokeError] = useState(null); + // Skill learning: whose requests may feed AI skill suggestions. + const [learningDefault, setLearningDefault] = useState(false); + const [selected, setSelected] = useState>(new Set()); + const [eraseIntents, setEraseIntents] = useState(false); + const [learningSaving, setLearningSaving] = useState(false); + const loadData = async () => { if (!token) return; try { - const [userData, roleData, invitationData] = await Promise.all([ + const [userData, roleData, invitationData, kgSettings] = await Promise.all([ users.list(token), roles.list(token).catch(() => []), users.invitations(token).catch(() => []), + knowledgeGraph.getSettings(token).catch(() => null), ]); setUserList(userData); + setLearningDefault(!!kgSettings?.captureIntent); setRoleList(roleData); setInvitationList(invitationData); } catch (err: any) { @@ -79,6 +95,34 @@ export default function SettingsUsersPage() { } }; + const handleLearningChange = async (userIds: string[], mode: LearningMode) => { + if (!token || userIds.length === 0) return; + setLearningSaving(true); + try { + const clearIntents = mode !== 'on' && eraseIntents; + const result = await knowledgeGraph.learning.setMembers(token, { userIds, mode, clearIntents }); + setUserList((prev) => + prev.map((u) => (userIds.includes(u.id) ? { ...u, learningMode: mode } : u)), + ); + const parts = [`Skill learning updated for ${userIds.length} user${userIds.length !== 1 ? 's' : ''}.`]; + if (clearIntents) parts.push(`${result.intentsCleared} captured request(s) erased.`); + setMsg(parts.join(' ')); + } catch (err: any) { + setMsg(`Error: ${err.message}`); + } finally { + setLearningSaving(false); + } + }; + + const toggleSelected = (userId: string) => { + setSelected((prev) => { + const next = new Set(prev); + if (next.has(userId)) next.delete(userId); + else next.add(userId); + return next; + }); + }; + const handleDeactivate = async (userId: string, email: string) => { if ( !token || @@ -362,18 +406,64 @@ export default function SettingsUsersPage() {
)} + {selected.size > 0 && ( + + + {selected.size} selected · Skill learning: + + {learningOptions(learningDefault).map((o) => ( + + ))} + + + + )} + {loading ? (

Loading...

) : ( - +
+ + @@ -381,6 +471,14 @@ export default function SettingsUsersPage() { {userList.map((u) => ( + + @@ -459,6 +573,7 @@ export default function SettingsUsersPage() { const status = getInvitationStatus(inv); return ( + @@ -476,6 +591,9 @@ export default function SettingsUsersPage() { +
+ 0 && selected.size === userList.length} + onChange={(e) => + setSelected(e.target.checked ? new Set(userList.map((u) => u.id)) : new Set()) + } + /> + Email Name Status Role MCP Role + Skill learning + Date Actions
+ toggleSelected(u.id)} + /> + {u.email} {u.id === currentUser?.id && ( @@ -422,6 +520,22 @@ export default function SettingsUsersPage() { )} +
+ handleLearningChange([u.id], v as LearningMode)} + className="h-8 whitespace-nowrap rounded-[9px] border border-[var(--border)] px-2 text-xs bg-[var(--surface)] text-[var(--text)]" + options={learningOptions(learningDefault)} + /> + {u.learningOptOut && ( + + opted out + + )} +
+
{new Date(u.createdAt).toLocaleDateString()}
{inv.email} — + Default + {new Date(inv.createdAt).toLocaleDateString()} diff --git a/packages/frontend/src/lib/api.ts b/packages/frontend/src/lib/api.ts index bc2a5ba6e..570a671a1 100644 --- a/packages/frontend/src/lib/api.ts +++ b/packages/frontend/src/lib/api.ts @@ -980,6 +980,17 @@ export interface KgSettings { edgeAutoApply: boolean; } +export type LearningMode = 'inherit' | 'on' | 'off'; + +export interface MemberLearning { + /** The workspace switch ("Capture user intent"). */ + workspaceDefault: boolean; + mode: LearningMode; + optOut: boolean; + /** Whether this member's requests are used for skill learning right now. */ + effective: boolean; +} + export interface KgSkillList { items: KgSkill[]; total: number; @@ -996,6 +1007,24 @@ export const knowledgeGraph = { ), getSettings: (token: string) => request('/api/knowledge-graph/settings', { token }), + learning: { + me: (token: string) => + request('/api/knowledge-graph/learning/me', { token }), + setMine: (token: string, body: { optOut: boolean; clearIntents?: boolean }) => + request('/api/knowledge-graph/learning/me', { + token, + method: 'PUT', + body, + }), + setMembers: ( + token: string, + body: { userIds: string[]; mode: LearningMode; clearIntents?: boolean }, + ) => + request<{ updated: number; notFound: string[]; intentsCleared: number }>( + '/api/knowledge-graph/learning/members', + { token, method: 'PUT', body }, + ), + }, updateSettings: ( token: string, body: { From 332638baf3e64a80f4c8c2cf88a7e9828d2d15f2 Mon Sep 17 00:00:00 2001 From: "helpcode.ai" <340500827+helpcodeai@users.noreply.github.com> Date: Mon, 19 Oct 2026 12:49:32 +0200 Subject: [PATCH 2/8] Serve workspace skills on demand with procedures and versions (#1148) Every applied skill was pasted into the initialize instructions, a skill could only hold a 2,000-character rule, and edits overwrote it. - skills_list / skill_get on /mcp/:serverId when the caller sees an applied skill, scoped to the server's organization and the caller's visible connectors; each skill is also offered as an MCP prompt. - Per-server skills_delivery: auto (full text up to 6,000 characters, else an index), inline or index. The migration pins servers already above the threshold to inline so nothing changes until an admin opts in. - Optional body (markdown, up to 20,000 characters) served by skill_get only; the Skills Finder can file one. - Edits keep the previous content in kg_skill_versions; skill_get and prompts count uses. Consolidate leaves skills with a procedure alone. - AI Skills page shows procedure, version, usage and history; the MCP server page gets the delivery setting. --- docs/knowledge-graph.md | 54 +++- .../20261019140000_skills_v2/migration.sql | 42 +++ packages/backend/prisma/schema.prisma | 32 +++ .../src/knowledge-graph/kg-resource.spec.ts | 4 +- .../knowledge-graph/kg-skill.service.spec.ts | 145 ++++++++++ .../src/knowledge-graph/kg-skill.service.ts | 272 ++++++++++++++++-- .../src/knowledge-graph/kg.controller.ts | 28 +- .../backend/src/knowledge-graph/kg.service.ts | 7 +- .../src/mcp-server/grant-visibility.spec.ts | 4 +- .../mcp-endpoint.controller.spec.ts | 57 +++- .../src/mcp-server/mcp-endpoint.controller.ts | 192 ++++++++++++- .../shared-endpoint.assistant.spec.ts | 2 +- .../shared-endpoint.controller.spec.ts | 2 +- .../src/mcp-servers/mcp-servers.controller.ts | 12 +- .../mcp-servers.instructions.spec.ts | 8 +- .../src/mcp-servers/mcp-servers.service.ts | 23 +- .../src/app/knowledge-graph/skills/page.tsx | 99 ++++++- .../frontend/src/app/mcp-server/[id]/page.tsx | 27 ++ packages/frontend/src/lib/api.ts | 24 +- 19 files changed, 964 insertions(+), 70 deletions(-) create mode 100644 packages/backend/prisma/migrations/20261019140000_skills_v2/migration.sql diff --git a/docs/knowledge-graph.md b/docs/knowledge-graph.md index 1135f7e22..019a399830 100644 --- a/docs/knowledge-graph.md +++ b/docs/knowledge-graph.md @@ -133,22 +133,48 @@ security event log as `LEARNING_CHANGED`. Settings belong to the workspace membership, so the same person can have different settings in two workspaces. Cloud and self-hosted behave the same. -### Skills are instructions, not extra tools - -> **Important:** applied skills do **not** add tool calls to your MCP surface. -> All applied skills for a server (its own server-scoped skills plus the -> connector-scoped ones for its connectors) are concatenated into a single -> `## Workspace skills` block inside the server's `instructions`. The agent reads -> them as guidance — the tool list is unchanged. +### How skills reach the AI client + +A server's applied skills (its own server-scoped skills plus the +connector-scoped ones of the connectors the caller can use) reach the client in +two ways: + +- **In the instructions** sent on connect, as a `## Workspace skills` block. + Each MCP server chooses how (MCP server page → *Workspace skills on connect*): + - **Automatic** (default): the full text while it fits in about 6,000 + characters, otherwise an index of ids, titles and *when to use*. + - **Always the full text.** + - **Always a list** (the index). +- **On demand**, with two built-in tools that appear on `/mcp/` when + the caller can see at least one applied skill: + - `skills_list(query?)` lists or searches the skills; + - `skill_get(skill)` returns one skill in full, by id or exact title. + + Both read the current version at call time and only return skills the caller + can see on that server. + +Each skill is also offered as an **MCP prompt**, so clients with a prompt +picker (slash commands) can start one by hand. + +A skill has a short **instruction** (up to 2,000 characters) and an optional +**procedure** (markdown, up to 20,000 characters) for the full steps of a +workflow. The procedure is never pasted into the instructions: the full-text +mode mentions it with its id, and `skill_get` returns it. The Agent Skills +Finder's `skills_save_to_workspace` can file a procedure too. + +The shared `/mcp` endpoint keeps its fixed toolset: its workspace guide +(`anythingmcp_get_workspace_guide`) returns the skills as text, procedures +included. + +**Versions and usage.** Editing a skill's title, *when to use*, instruction or +procedure keeps the previous content as a version (see *History* on the skill) +and increments its version number. Each `skill_get` or prompt use is counted, so +the Skills page shows how often a skill was opened and when. Consolidate with +AI leaves skills that have a procedure untouched. Because composition is dynamic, editing or deleting a skill takes effect on the -next request — there's no blob to clean up. - -> **Scaling note.** Applied skills are concatenated verbatim into the server's -> instruction block, so with many active skills that block grows and consumes -> context on every session. Keep the applied set tight: scope skills to the -> right server/connector, dismiss duplicates, and use **Consolidate with AI** to -> merge overlapping active skills into fewer rules. +next request (a stateful session keeps the instructions it started with; the +tools always read the current version). --- diff --git a/packages/backend/prisma/migrations/20261019140000_skills_v2/migration.sql b/packages/backend/prisma/migrations/20261019140000_skills_v2/migration.sql new file mode 100644 index 000000000..4f320fa3b --- /dev/null +++ b/packages/backend/prisma/migrations/20261019140000_skills_v2/migration.sql @@ -0,0 +1,42 @@ +-- Workspace skills v2: long procedures served on demand, versions, usage, +-- and a per-server choice of how skills reach the model on initialize. +ALTER TABLE "kg_skill_suggestions" ADD COLUMN "body" TEXT; +ALTER TABLE "kg_skill_suggestions" ADD COLUMN "version" INTEGER NOT NULL DEFAULT 1; +ALTER TABLE "kg_skill_suggestions" ADD COLUMN "use_count" INTEGER NOT NULL DEFAULT 0; +ALTER TABLE "kg_skill_suggestions" ADD COLUMN "last_used_at" TIMESTAMP(3); + +ALTER TABLE "mcp_server_configs" ADD COLUMN "skills_delivery" TEXT NOT NULL DEFAULT 'auto'; + +CREATE TABLE "kg_skill_versions" ( + "id" TEXT NOT NULL, + "skill_id" TEXT NOT NULL, + "organization_id" TEXT NOT NULL, + "version" INTEGER NOT NULL, + "title" TEXT NOT NULL, + "when_to_use" TEXT NOT NULL, + "instruction" TEXT NOT NULL, + "body" TEXT, + "edited_by_user_id" TEXT, + "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + + CONSTRAINT "kg_skill_versions_pkey" PRIMARY KEY ("id") +); + +CREATE UNIQUE INDEX "kg_skill_versions_skill_id_version_key" ON "kg_skill_versions"("skill_id", "version"); +CREATE INDEX "kg_skill_versions_organization_id_idx" ON "kg_skill_versions"("organization_id"); + +ALTER TABLE "kg_skill_versions" ADD CONSTRAINT "kg_skill_versions_skill_id_fkey" FOREIGN KEY ("skill_id") REFERENCES "kg_skill_suggestions"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- `auto` switches to an index above 6,000 characters of applied skills. Keep +-- today's behaviour (full text) on servers already above it, so nothing +-- changes until an admin picks a mode. Counts the server's own skills; the +-- 6000 must match SKILLS_INLINE_BUDGET in kg-skill.service.ts. +UPDATE "mcp_server_configs" AS s +SET "skills_delivery" = 'inline' +WHERE s."id" IN ( + SELECT k."mcp_server_id" + FROM "kg_skill_suggestions" AS k + WHERE k."status" = 'applied' AND k."mcp_server_id" IS NOT NULL + GROUP BY k."mcp_server_id" + HAVING SUM(length(k."title") + length(k."when_to_use") + length(k."instruction")) > 6000 +); diff --git a/packages/backend/prisma/schema.prisma b/packages/backend/prisma/schema.prisma index 4e24d5ff9..a5fb1c1da 100644 --- a/packages/backend/prisma/schema.prisma +++ b/packages/backend/prisma/schema.prisma @@ -621,6 +621,10 @@ model McpServerConfig { version String @default("1.0.0") isActive Boolean @default(true) @map("is_active") instructions String? @db.Text // custom instructions prepended to composed connector instructions + /// How applied skills reach the model on initialize: `auto` (full text + /// while small, else an index pointing to skill_get), `inline` (always the + /// full text) or `index` (always the index). + skillsDelivery String @default("auto") @map("skills_delivery") createdAt DateTime @default(now()) @map("created_at") updatedAt DateTime @updatedAt @map("updated_at") @@ -1379,6 +1383,15 @@ model KgSkillSuggestion { // consolidated) | finder (skills_save_to_workspace). Generation only // replaces its own pending `llm` rows. source String @default("manual") + // Optional long procedure (markdown). Never pasted into the instructions: + // served on demand by skill_get. + body String? @db.Text + // Incremented on every edit of title / whenToUse / instruction / body; the + // previous content is kept in kg_skill_versions. + version Int @default(1) + useCount Int @default(0) @map("use_count") + lastUsedAt DateTime? @map("last_used_at") + versions KgSkillVersion[] createdAt DateTime @default(now()) @map("created_at") updatedAt DateTime @updatedAt @map("updated_at") @@ -1387,6 +1400,25 @@ model KgSkillSuggestion { @@map("kg_skill_suggestions") } +// Earlier content of a workspace skill, written when an edit changes it. +model KgSkillVersion { + id String @id @default(cuid()) + skillId String @map("skill_id") + skill KgSkillSuggestion @relation(fields: [skillId], references: [id], onDelete: Cascade) + organizationId String @map("organization_id") + version Int + title String + whenToUse String @db.Text @map("when_to_use") + instruction String @db.Text + body String? @db.Text + editedByUserId String? @map("edited_by_user_id") + createdAt DateTime @default(now()) @map("created_at") + + @@unique([skillId, version]) + @@index([organizationId]) + @@map("kg_skill_versions") +} + // A submitted LLM batch job (cloud cron, cost-saver). Tracks the provider batch // id so a later cron run can retrieve results and apply them. `context` maps each // request's custom_id to what's needed to apply its result (org, type, ref map). diff --git a/packages/backend/src/knowledge-graph/kg-resource.spec.ts b/packages/backend/src/knowledge-graph/kg-resource.spec.ts index 64a51c568..27e167d75 100644 --- a/packages/backend/src/knowledge-graph/kg-resource.spec.ts +++ b/packages/backend/src/knowledge-graph/kg-resource.spec.ts @@ -59,7 +59,7 @@ describe('KgService.describeForResource', () => { const { svc } = make({ nodes: [node('n1', 'c1', 'Customer'), node('n2', 'c2', 'Order', { description: 'A sales order.' })], edges: [{ sourceNodeId: 'n2', targetNodeId: 'n1', kind: 'references', matchKey: 'customer_id', note: null }], - skills: [{ title: 'Monthly revenue', whenToUse: 'When asked for revenue per month.' }], + skills: [{ id: 'sk-rev', title: 'Monthly revenue', whenToUse: 'When asked for revenue per month.' }], }); const text = await svc.describeForResource(ORG, { connectorIds: ['c1', 'c2'], serverName: 'Sales' }); @@ -69,7 +69,7 @@ describe('KgService.describeForResource', () => { expect(text).toContain('`c1_get_customer`'); expect(text).toContain('`customer_number`'); expect(text).toContain('- Order (ERP) refers to Customer (CRM) via `customer_id`'); - expect(text).toContain('- **Monthly revenue**: When asked for revenue per month.'); + expect(text).toContain('- **Monthly revenue** (`sk-rev`): When asked for revenue per month.'); }); it('stays bounded on a large graph and says what it left out', async () => { diff --git a/packages/backend/src/knowledge-graph/kg-skill.service.spec.ts b/packages/backend/src/knowledge-graph/kg-skill.service.spec.ts index 3e5dc0921..c59fc99ef 100644 --- a/packages/backend/src/knowledge-graph/kg-skill.service.spec.ts +++ b/packages/backend/src/knowledge-graph/kg-skill.service.spec.ts @@ -281,3 +281,148 @@ describe('KgSkillService.activeSkillsText', () => { ); }); }); + +describe('KgSkillService delivery modes', () => { + const ORG = 'org-A'; + const row = (i: number, extra: Partial = {}) => ({ + id: `s${i}`, + title: `Skill ${i}`, + whenToUse: `when ${i}`, + instruction: `rule ${i}`, + body: null, + version: 1, + ...extra, + }); + const svcWith = (rows: any[]) => + new KgSkillService({ kgSkillSuggestion: { findMany: jest.fn().mockResolvedValue(rows) } } as any, {} as any); + + it('inline keeps the existing format for skills without a procedure', async () => { + const text = await svcWith([row(1)]).activeSkillsText(ORG, 'srv', [], 'inline'); + expect(text).toBe('## Workspace skills\n- Skill 1 (when: when 1): rule 1'); + }); + + it('inline references a procedure instead of pasting it', async () => { + const text = await svcWith([row(1, { body: 'STEP-BY-STEP' })]).activeSkillsText(ORG, 'srv', [], 'inline'); + expect(text).toContain('skill_get "s1"'); + expect(text).not.toContain('STEP-BY-STEP'); + }); + + it('full pastes the procedure (shared guide)', async () => { + const text = await svcWith([row(1, { body: 'STEP-BY-STEP' })]).activeSkillsText(ORG, 'srv', [], 'full'); + expect(text).toContain('STEP-BY-STEP'); + }); + + it('auto stays inline while small and switches to the index when large', async () => { + const small = await svcWith([row(1)]).activeSkillsText(ORG, 'srv', [], 'auto'); + expect(small).toContain('rule 1'); + + const big = Array.from({ length: 40 }, (_, i) => row(i, { instruction: 'x'.repeat(300) })); + const text = (await svcWith(big).activeSkillsText(ORG, 'srv', [], 'auto'))!; + expect(text).toContain('call `skill_get`'); + expect(text).toContain('- `s0` Skill 0: when 0'); + expect(text).not.toContain('x'.repeat(300)); + }); + + it('index lists up to its budget and points to skills_list for the rest', async () => { + const many = Array.from({ length: 200 }, (_, i) => row(i, { whenToUse: 'w'.repeat(150) })); + const text = (await svcWith(many).activeSkillsText(ORG, 'srv', [], 'index'))!; + expect(text.length).toBeLessThan(8_200); + expect(text).toMatch(/…and \d+ more: call `skills_list`/); + }); +}); + +describe('KgSkillService.getForCaller', () => { + it('reads only visible applied skills of the organization and counts the use', async () => { + const skill = { id: 's1', title: 'T', whenToUse: '', instruction: 'I', body: null, version: 2 }; + const findFirst = jest.fn().mockResolvedValue(skill); + const update = jest.fn().mockResolvedValue({}); + const svc = new KgSkillService({ kgSkillSuggestion: { findFirst, update } } as any, {} as any); + + expect(await svc.getForCaller('org-A', 'srv-1', ['c-1'], 's1')).toBe(skill); + const where = findFirst.mock.calls[0][0].where; + expect(where.AND[0]).toEqual({ + organizationId: 'org-A', + status: 'applied', + OR: [{ mcpServerId: { in: ['srv-1'] } }, { connectorId: { in: ['c-1'] } }], + }); + expect(where.AND[1]).toEqual({ OR: [{ id: 's1' }, { title: { equals: 's1', mode: 'insensitive' } }] }); + expect(update).toHaveBeenCalledWith({ + where: { id: 's1' }, + data: { useCount: { increment: 1 }, lastUsedAt: expect.any(Date) }, + }); + }); + + it('returns null without counting when the skill is not visible', async () => { + const update = jest.fn(); + const svc = new KgSkillService( + { kgSkillSuggestion: { findFirst: jest.fn().mockResolvedValue(null), update } } as any, + {} as any, + ); + expect(await svc.getForCaller('org-A', 'srv-1', [], 'other-org-skill')).toBeNull(); + expect(update).not.toHaveBeenCalled(); + }); +}); + +describe('KgSkillService.update versions', () => { + const current = { + id: 's1', + organizationId: 'org-A', + title: 'Old', + whenToUse: 'w', + instruction: 'old rule', + body: null, + version: 3, + }; + + function make() { + const tx = { + kgSkillVersion: { create: jest.fn() }, + kgSkillSuggestion: { update: jest.fn().mockResolvedValue({ id: 's1', version: 4 }) }, + }; + const prisma = { + kgSkillSuggestion: { + findUnique: jest.fn().mockResolvedValue(current), + update: jest.fn().mockResolvedValue({ id: 's1' }), + }, + $transaction: jest.fn(async (fn: any) => fn(tx)), + }; + return { svc: new KgSkillService(prisma as any, {} as any), prisma, tx }; + } + + it('keeps the previous content as a version and increments it on a content change', async () => { + const { svc, tx } = make(); + await svc.update('org-A', 's1', { instruction: 'new rule', body: 'steps' }, 'u-1'); + expect(tx.kgSkillVersion.create).toHaveBeenCalledWith({ + data: { + skillId: 's1', + organizationId: 'org-A', + version: 3, + title: 'Old', + whenToUse: 'w', + instruction: 'old rule', + body: null, + editedByUserId: 'u-1', + }, + }); + expect(tx.kgSkillSuggestion.update).toHaveBeenCalledWith({ + where: { id: 's1' }, + data: { instruction: 'new rule', body: 'steps', version: { increment: 1 } }, + }); + }); + + it('does not version a status-only change', async () => { + const { svc, prisma } = make(); + await svc.update('org-A', 's1', { status: 'dismissed' }); + expect(prisma.$transaction).not.toHaveBeenCalled(); + expect(prisma.kgSkillSuggestion.update).toHaveBeenCalledWith({ + where: { id: 's1' }, + data: { status: 'dismissed' }, + }); + }); + + it('refuses a skill of another organization', async () => { + const { svc, prisma } = make(); + prisma.kgSkillSuggestion.findUnique.mockResolvedValue({ ...current, organizationId: 'org-B' }); + await expect(svc.update('org-A', 's1', { title: 'x' })).rejects.toBeInstanceOf(NotFoundException); + }); +}); diff --git a/packages/backend/src/knowledge-graph/kg-skill.service.ts b/packages/backend/src/knowledge-graph/kg-skill.service.ts index 6430cc4ef..49e8fe95f 100644 --- a/packages/backend/src/knowledge-graph/kg-skill.service.ts +++ b/packages/backend/src/knowledge-graph/kg-skill.service.ts @@ -9,6 +9,87 @@ const MAX_INTENTS = 200; /** Where a skill came from; generation only ever replaces its own `llm` rows. */ export type SkillSource = 'manual' | 'llm' | 'finder'; +/** Longest procedure (`body`) a skill can carry. Served by skill_get only. */ +export const SKILL_BODY_MAX = 20_000; + +/** How applied skills reach the model on initialize (per MCP server). */ +export const SKILLS_DELIVERY_MODES = ['auto', 'inline', 'index'] as const; +export type SkillsDelivery = (typeof SKILLS_DELIVERY_MODES)[number]; + +/** + * `auto` keeps the full text up to this many characters, then switches to the + * index. The skills_v2 migration pins servers already above it to `inline`; + * keep both numbers equal. + */ +export const SKILLS_INLINE_BUDGET = 6_000; +/** The index stops listing entries past this size and points to skills_list. */ +const SKILLS_INDEX_BUDGET = 8_000; +const INDEX_WHEN_MAX = 200; + +/** Built-in MCP tool names (per-server endpoint). */ +export const SKILLS_LIST_TOOL = 'skills_list'; +export const SKILL_GET_TOOL = 'skill_get'; + +interface SkillRow { + id: string; + title: string; + whenToUse: string; + instruction: string; + body: string | null; + version: number; +} + +/** The skills a caller may see: the server's own and its visible connectors'. */ +function visibleWhere(organizationId: string, serverIds: string[], connectorIds: string[]) { + return { + // Re-stated rather than trusted from the ids, so a stray id can never + // pull another workspace's skills in. + organizationId, + status: 'applied', + OR: [ + { mcpServerId: { in: serverIds.length ? serverIds : ['__none__'] } }, + { connectorId: { in: connectorIds.length ? connectorIds : ['__none__'] } }, + ], + }; +} + +function oneLine(text: string, max: number): string { + const flat = text.replace(/\s+/g, ' ').trim(); + return flat.length > max ? `${flat.slice(0, max - 1)}…` : flat; +} + +function inlineText(rows: SkillRow[], withBodies: boolean): string { + const body = rows + .map((s) => { + const head = `- ${s.title}${s.whenToUse ? ` (when: ${s.whenToUse})` : ''}: ${s.instruction}`; + if (!s.body) return head; + return withBodies + ? `${head}\n\n${s.body}\n` + : `${head} (full procedure: ${SKILL_GET_TOOL} "${s.id}")`; + }) + .join('\n'); + return `## Workspace skills\n${body}`; +} + +function indexText(rows: SkillRow[]): string { + const head = + `## Workspace skills\n` + + `This workspace has ${rows.length} skill${rows.length === 1 ? '' : 's'}: its procedures and rules. ` + + `Before a task that matches one, call \`${SKILL_GET_TOOL}\` with its id and follow it. ` + + `\`${SKILLS_LIST_TOOL}\` searches them all.`; + const lines: string[] = []; + let size = head.length; + for (const s of rows) { + const line = `- \`${s.id}\` ${s.title}${s.whenToUse ? `: ${oneLine(s.whenToUse, INDEX_WHEN_MAX)}` : ''}`; + if (size + line.length + 1 > SKILLS_INDEX_BUDGET) break; + lines.push(line); + size += line.length + 1; + } + const rest = rows.length - lines.length; + if (rest > 0) lines.push(`- …and ${rest} more: call \`${SKILLS_LIST_TOOL}\`.`); + return `${head}\n${lines.join('\n')}`; +} + /** Auto-apply a generated skill only when at/above this confidence. */ export const SKILL_AUTO_APPLY_MIN = 0.9; @@ -320,9 +401,11 @@ export class KgSkillService { } } + // Skills with a procedure are left out: the merge works on the short + // rules only and would drop the procedure. const where = mcpServerId - ? { organizationId, status: 'applied', mcpServerId } - : { organizationId, status: 'applied', mcpServerId: null }; + ? { organizationId, status: 'applied', mcpServerId, body: null } + : { organizationId, status: 'applied', mcpServerId: null, body: null }; const current = await this.prisma.kgSkillSuggestion.findMany({ where, select: { @@ -391,32 +474,135 @@ export class KgSkillService { return { before: current.length, after, model: cfg.model, usage }; } - /** Active skills composed into an MCP server's instructions at serve time. */ + /** + * Applied skills as instruction text. `inline` is the full text (procedures + * referenced, not pasted), `index` lists titles for skill_get, `auto` picks + * inline while it fits SKILLS_INLINE_BUDGET, `full` pastes the procedures + * too (the shared /mcp guide, which has no skill_get). + */ async activeSkillsText( organizationId: string, serverId: string | string[], connectorIds: string[], + mode: SkillsDelivery | 'full' = 'inline', ): Promise { const serverIds = Array.isArray(serverId) ? serverId : [serverId]; - const skills = await this.prisma.kgSkillSuggestion.findMany({ + const rows: SkillRow[] = await this.prisma.kgSkillSuggestion.findMany({ + where: visibleWhere(organizationId, serverIds, connectorIds), + select: { id: true, title: true, whenToUse: true, instruction: true, body: true, version: true }, + orderBy: { confidence: 'desc' }, + }); + if (!rows.length) return null; + if (mode === 'full') return inlineText(rows, true); + if (mode === 'index') return indexText(rows); + const inline = inlineText(rows, false); + if (mode === 'inline' || inline.length <= SKILLS_INLINE_BUDGET) return inline; + return indexText(rows); + } + + /** Whether a caller sees at least one applied skill (offers the skill tools). */ + async hasVisible(organizationId: string, serverId: string, connectorIds: string[]): Promise { + const n = await this.prisma.kgSkillSuggestion.count({ + where: visibleWhere(organizationId, [serverId], connectorIds), + }); + return n > 0; + } + + /** skills_list: the caller's visible skills, optionally filtered. */ + async listForCaller( + organizationId: string, + serverId: string, + connectorIds: string[], + query?: string, + ) { + const q = (query ?? '').trim(); + const rows = await this.prisma.kgSkillSuggestion.findMany({ where: { - // Re-stated rather than trusted from the ids, so a stray id can never - // pull another workspace's skills into these instructions. - organizationId, - status: 'applied', - OR: [ - { mcpServerId: { in: serverIds.length ? serverIds : ['__none__'] } }, - { connectorId: { in: connectorIds.length ? connectorIds : ['__none__'] } }, + AND: [ + visibleWhere(organizationId, [serverId], connectorIds), + ...(q + ? [ + { + OR: [ + { title: { contains: q, mode: 'insensitive' as const } }, + { whenToUse: { contains: q, mode: 'insensitive' as const } }, + { instruction: { contains: q, mode: 'insensitive' as const } }, + ], + }, + ] + : []), ], }, - select: { title: true, whenToUse: true, instruction: true }, + select: { id: true, title: true, whenToUse: true, body: true, version: true }, orderBy: { confidence: 'desc' }, + take: 100, + }); + return rows.map((r) => ({ + id: r.id, + title: r.title, + whenToUse: r.whenToUse, + version: r.version, + hasProcedure: !!r.body, + })); + } + + /** + * skill_get: one visible skill by id or exact title (case-insensitive). + * Counts the use. Null when the caller cannot see it, the same answer as + * for a skill that does not exist. + */ + async getForCaller( + organizationId: string, + serverId: string, + connectorIds: string[], + ref: string, + ) { + const key = (ref ?? '').trim(); + if (!key) return null; + const skill = await this.prisma.kgSkillSuggestion.findFirst({ + where: { + AND: [ + visibleWhere(organizationId, [serverId], connectorIds), + { OR: [{ id: key }, { title: { equals: key, mode: 'insensitive' as const } }] }, + ], + }, + select: { + id: true, + title: true, + whenToUse: true, + instruction: true, + body: true, + version: true, + updatedAt: true, + }, + }); + if (!skill) return null; + // Best effort: a failed counter must never fail the read. + this.prisma.kgSkillSuggestion + .update({ + where: { id: skill.id }, + data: { useCount: { increment: 1 }, lastUsedAt: new Date() }, + }) + .catch((e: any) => this.logger.warn(`skill use count failed: ${e?.message ?? e}`)); + return skill; + } + + /** Earlier versions of one skill, newest first. */ + async versions(organizationId: string, id: string) { + await this.owned(organizationId, id); + return this.prisma.kgSkillVersion.findMany({ + where: { skillId: id, organizationId }, + orderBy: { version: 'desc' }, + select: { + version: true, + title: true, + whenToUse: true, + instruction: true, + body: true, + editedByUserId: true, + createdAt: true, + }, }); - if (!skills.length) return null; - const body = skills - .map((s) => `- ${s.title}${s.whenToUse ? ` (when: ${s.whenToUse})` : ''}: ${s.instruction}`) - .join('\n'); - return `## Workspace skills\n${body}`; } /** @@ -434,6 +620,7 @@ export class KgSkillService { mcpServerId?: string | null; status?: string; source?: SkillSource; + body?: string | null; }, ) { const title = (body.title || '').trim().slice(0, 160); @@ -479,6 +666,7 @@ export class KgSkillService { evidenceCount: 0, status, source: body.source === 'finder' ? 'finder' : 'manual', + body: normalizeBody(body.body), }, }); } @@ -499,20 +687,57 @@ export class KgSkillService { return this.prisma.kgSkillSuggestion.update({ where: { id }, data: { status: 'dismissed' } }); } + /** + * Edit a skill. A change to its content (title, when, instruction, body) + * keeps the previous content as a version and increments `version`; a + * status-only change does not. + */ async update( organizationId: string, id: string, - patch: { title?: string; whenToUse?: string; instruction?: string; status?: string }, + patch: { + title?: string; + whenToUse?: string; + instruction?: string; + body?: string | null; + status?: string; + }, + editedByUserId?: string, ) { - await this.owned(organizationId, id); + const current = await this.owned(organizationId, id); const data: Record = {}; if (typeof patch.title === 'string') data.title = patch.title.slice(0, 160); if (typeof patch.whenToUse === 'string') data.whenToUse = patch.whenToUse.slice(0, 1000); if (typeof patch.instruction === 'string') data.instruction = patch.instruction.slice(0, 2000); + if (patch.body !== undefined) data.body = normalizeBody(patch.body); if (patch.status && ['pending', 'applied', 'dismissed'].includes(patch.status)) { data.status = patch.status; } - return this.prisma.kgSkillSuggestion.update({ where: { id }, data }); + + const changed = (['title', 'whenToUse', 'instruction', 'body'] as const).some( + (k) => k in data && data[k] !== (current as any)[k], + ); + if (!changed) { + return this.prisma.kgSkillSuggestion.update({ where: { id }, data }); + } + return this.prisma.$transaction(async (tx) => { + await tx.kgSkillVersion.create({ + data: { + skillId: id, + organizationId, + version: current.version, + title: current.title, + whenToUse: current.whenToUse, + instruction: current.instruction, + body: current.body, + editedByUserId: editedByUserId ?? null, + }, + }); + return tx.kgSkillSuggestion.update({ + where: { id }, + data: { ...data, version: { increment: 1 } }, + }); + }); } async remove(organizationId: string, id: string) { @@ -521,3 +746,10 @@ export class KgSkillService { return { ok: true }; } } + +/** Trimmed, capped procedure; empty means none. */ +function normalizeBody(body: string | null | undefined): string | null { + if (typeof body !== 'string') return null; + const trimmed = body.trim(); + return trimmed ? trimmed.slice(0, SKILL_BODY_MAX) : null; +} diff --git a/packages/backend/src/knowledge-graph/kg.controller.ts b/packages/backend/src/knowledge-graph/kg.controller.ts index 7537305d6..da46f9a0d 100644 --- a/packages/backend/src/knowledge-graph/kg.controller.ts +++ b/packages/backend/src/knowledge-graph/kg.controller.ts @@ -21,10 +21,11 @@ import { IsIn, IsOptional, IsString, + MaxLength, } from 'class-validator'; import { Roles, RolesGuard } from '../auth/roles.guard'; import { KgService } from './kg.service'; -import { KgSkillService } from './kg-skill.service'; +import { KgSkillService, SKILL_BODY_MAX } from './kg-skill.service'; import { KgLearningService, LEARNING_BULK_MAX, @@ -44,10 +45,18 @@ class CreateSkillDto { @IsString() whenToUse?: string; - @ApiProperty({ description: 'The rule or procedure (stored up to 2000 characters).' }) + @ApiProperty({ description: 'The rule (stored up to 2000 characters).' }) @IsString() instruction: string; + @ApiPropertyOptional({ + description: `Optional full procedure in markdown (up to ${SKILL_BODY_MAX} characters). Served by skill_get, never pasted into the instructions.`, + }) + @IsOptional() + @IsString() + @MaxLength(SKILL_BODY_MAX) + body?: string | null; + @ApiPropertyOptional({ description: 'Scope the skill to one connector of this workspace.' }) @IsOptional() @IsString() @@ -80,6 +89,12 @@ class UpdateSkillDto { @IsString() instruction?: string; + @ApiPropertyOptional({ description: 'Full procedure in markdown; empty string or null removes it.' }) + @IsOptional() + @IsString() + @MaxLength(SKILL_BODY_MAX) + body?: string | null; + @ApiPropertyOptional({ enum: SKILL_STATUSES }) @IsOptional() @IsIn(SKILL_STATUSES) @@ -257,7 +272,14 @@ export class KgController { @Param('id') id: string, @Body() body: UpdateSkillDto, ) { - return this.skills.update(req.user.organizationId, id, body); + return this.skills.update(req.user.organizationId, id, body, req.user.sub); + } + + @Get('skills/:id/versions') + @Roles('ADMIN', 'EDITOR') + @ApiOperation({ summary: 'Earlier versions of a skill, newest first' }) + async skillVersions(@Req() req: any, @Param('id') id: string) { + return this.skills.versions(req.user.organizationId, id); } // ── Who contributes to skill learning ──────────────────────────────────── diff --git a/packages/backend/src/knowledge-graph/kg.service.ts b/packages/backend/src/knowledge-graph/kg.service.ts index 9a37590b4..6cf4eae18 100644 --- a/packages/backend/src/knowledge-graph/kg.service.ts +++ b/packages/backend/src/knowledge-graph/kg.service.ts @@ -690,8 +690,9 @@ export class KgService { if (skills.length) { out.push('', `## Workspace skills (${skills.length})`, ''); for (const sk of skills) { - out.push(`- **${sk.title}**${sk.whenToUse ? `: ${oneLine(sk.whenToUse)}` : ''}`); + out.push(`- **${sk.title}** (\`${sk.id}\`)${sk.whenToUse ? `: ${oneLine(sk.whenToUse)}` : ''}`); } + out.push('', 'Read a skill in full with `skill_get` and its id.'); } return out.join('\n'); } @@ -710,10 +711,10 @@ export class KgService { status: 'applied', ...(or.length ? { OR: or } : {}), }, - select: { title: true, whenToUse: true }, + select: { id: true, title: true, whenToUse: true }, take: 25, }); - return rows.map((r) => ({ title: r.title, whenToUse: r.whenToUse })); + return rows.map((r) => ({ id: r.id, title: r.title, whenToUse: r.whenToUse })); } async deleteEdge(organizationId: string, edgeId: string) { diff --git a/packages/backend/src/mcp-server/grant-visibility.spec.ts b/packages/backend/src/mcp-server/grant-visibility.spec.ts index d56ad7edd..c5c0fb609 100644 --- a/packages/backend/src/mcp-server/grant-visibility.spec.ts +++ b/packages/backend/src/mcp-server/grant-visibility.spec.ts @@ -54,7 +54,7 @@ function build( {} as any, {} as any, { resolve: jest.fn().mockResolvedValue(grant) } as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); return controller; } @@ -227,7 +227,7 @@ describe('grant-scoped visibility on the shared /mcp', () => { {} as any, {} as any, { resolve: jest.fn().mockResolvedValue(null) } as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); const listFor = async (user: any) => { diff --git a/packages/backend/src/mcp-server/mcp-endpoint.controller.spec.ts b/packages/backend/src/mcp-server/mcp-endpoint.controller.spec.ts index 12fbe59d8..7765d9987 100644 --- a/packages/backend/src/mcp-server/mcp-endpoint.controller.spec.ts +++ b/packages/backend/src/mcp-server/mcp-endpoint.controller.spec.ts @@ -70,7 +70,7 @@ describe('McpEndpointController — tenant isolation', () => { kgService as any, sessionManager as any, grants as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); }); @@ -335,7 +335,7 @@ describe('McpEndpointController — keys created for a server', () => { } as any, sessionManager, { resolve: jest.fn().mockResolvedValue(null) } as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); (controller as any).serveStateless = jest.fn().mockResolvedValue(undefined); }); @@ -451,7 +451,7 @@ describe('McpEndpointController — structuredContent', () => { { lookup: jest.fn(), isEnabled: jest.fn(), captureIntentEnabled: jest.fn() } as any, { get: jest.fn(), add: jest.fn(), remove: jest.fn() } as any, { resolve: jest.fn().mockResolvedValue(null) } as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); const entries = (controller as any).planToolSet({ @@ -524,7 +524,7 @@ describe('McpEndpointController — structuredContent', () => { { lookup: jest.fn(), isEnabled: jest.fn(), captureIntentEnabled: jest.fn() } as any, { get: jest.fn(), add: jest.fn(), remove: jest.fn() } as any, { resolve: jest.fn().mockResolvedValue(null) } as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); const entries = (controller as any).planToolSet({ serverTools: [TOOL], @@ -590,7 +590,7 @@ describe('McpEndpointController — knowledge graph scope', () => { kgService as any, {} as any, {} as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); return { controller: controller as any, kgService }; } @@ -957,7 +957,7 @@ describe('McpEndpointController — resources and instructions follow the tool p kgService, sessionManager as any, { resolve: jest.fn() } as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); built = undefined; // Keep the server the request built instead of serving it over HTTP. @@ -1064,6 +1064,51 @@ describe('McpEndpointController — resources and instructions follow the tool p await on.close(); }); + it('offers skills_list, skill_get and skill prompts only when the caller sees a skill', async () => { + const none = await connect(null); + const noneNames = (await none.listTools()).tools.map((t) => t.name); + expect(noneNames).not.toContain('skill_get'); + expect(noneNames).not.toContain('skills_list'); + await none.close(); + + const skill = { + id: 'sk-1', + title: 'Angebot erstellen', + whenToUse: 'When the user asks for a quote', + instruction: 'Always draft first.', + body: 'STEP 1: draft_quote', + version: 2, + }; + const kgSkills = (controller as any).kgSkills; + kgSkills.hasVisible = jest.fn().mockResolvedValue(true); + kgSkills.listForCaller = jest.fn().mockResolvedValue([ + { id: 'sk-1', title: skill.title, whenToUse: skill.whenToUse, version: 2, hasProcedure: true }, + ]); + kgSkills.getForCaller = jest.fn(async (_o: string, _s: string, _c: string[], ref: string) => + ref === 'sk-1' ? skill : null, + ); + + const client = await connect(['crm_get']); + const names = (await client.listTools()).tools.map((t) => t.name); + expect(names).toEqual(expect.arrayContaining(['skills_list', 'skill_get'])); + // Scoped to the server's organization and the caller's visible connectors. + expect(kgSkills.hasVisible).toHaveBeenCalledWith(ORG_A, 'srv-1', ['c-crm']); + + const got: any = await client.callTool({ name: 'skill_get', arguments: { skill: 'sk-1' } }); + expect(got.content[0].text).toContain('STEP 1: draft_quote'); + expect(got.content[0].text).toContain('version 2'); + expect(kgSkills.getForCaller).toHaveBeenCalledWith(ORG_A, 'srv-1', ['c-crm'], 'sk-1'); + + const missing: any = await client.callTool({ name: 'skill_get', arguments: { skill: 'nope' } }); + expect(missing.isError).toBe(true); + + const prompts = (await client.listPrompts()).prompts; + expect(prompts.map((p) => p.name)).toEqual(['angebot-erstellen']); + const prompt = await client.getPrompt({ name: 'angebot-erstellen' }); + expect((prompt.messages[0].content as any).text).toContain('STEP 1: draft_quote'); + await client.close(); + }); + it('a caller who sees everything keeps the full instructions', async () => { const client = await connect(null); const instructions = client.getInstructions() ?? ''; diff --git a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts index 6b06f5947..86fc66494 100644 --- a/packages/backend/src/mcp-server/mcp-endpoint.controller.ts +++ b/packages/backend/src/mcp-server/mcp-endpoint.controller.ts @@ -65,7 +65,12 @@ import { DynamicMcpTools } from './dynamic-mcp-tools'; import { RolesService } from '../roles/roles.service'; import { registerDemoTools } from './mcp-demo.tools'; import { KgService } from '../knowledge-graph/kg.service'; -import { KgSkillService } from '../knowledge-graph/kg-skill.service'; +import { + KgSkillService, + SKILL_BODY_MAX, + SKILL_GET_TOOL, + SKILLS_LIST_TOOL, +} from '../knowledge-graph/kg-skill.service'; import { outputSchemaToZodShape } from '../connectors/output-schema.util'; import { annotationsSignature, @@ -177,6 +182,9 @@ interface ToolSetParams { /** Optional text appended as a trailing content block to every successful * tool result (used by the public demo to say where the answer came from). */ resultFooter?: string; + /** Set when the caller can see at least one applied workspace skill: offers + * skills_list / skill_get, scoped to the server's organization. */ + skills?: { organizationId: string }; } interface InvocationContext { @@ -1107,12 +1115,21 @@ export class McpEndpointController { ? await this.kgService.isEnabled(invocationContext.organizationId) : false; + // Workspace skills belong to the server's organization and follow the + // caller's visible connectors, like the instructions above. + const skillsVisible = await this.kgSkills.hasVisible( + mcpServerConfig.organizationId, + serverId, + visibleConnectorIds, + ); + const params: ToolSetParams = { serverTools, allowedToolIds, captureIntent, kgEnabled, invocationContext, + skills: skillsVisible ? { organizationId: mcpServerConfig.organizationId } : undefined, }; // Register this server's tool surface onto the per-request McpServer. @@ -1120,6 +1137,14 @@ export class McpEndpointController { const handles = this.registerAll(mcpServer, entries, serverId); const signature = this.aggregateSig(entries); this.registerKgResource(mcpServer, params); + if (skillsVisible) { + await this.registerSkillPrompts( + mcpServer, + mcpServerConfig.organizationId, + serverId, + visibleConnectorIds, + ); + } this.registerServerResources( mcpServer, planServerResources({ @@ -1449,12 +1474,18 @@ export class McpEndpointController { ); } + const skillsVisible = await this.kgSkills.hasVisible( + serverOrganizationId, + serverId, + callerConnectorIds(serverTools, allowedToolIds, connectorIds), + ); const entries = this.planToolSet({ serverTools, allowedToolIds, captureIntent, kgEnabled, invocationContext, + skills: skillsVisible ? { organizationId: serverOrganizationId } : undefined, }); const signature = this.aggregateSig(entries); if (signature === session.signature) return; // nothing changed here @@ -1682,6 +1713,76 @@ export class McpEndpointController { }); } + // Workspace skills on demand. The instructions carry the skills as text + // or, when there are many or they hold long procedures, an index that + // points here. Both tools read at call time, so an edited skill is served + // fresh, and only skills the caller can see on this server. + if (params.skills && invocationContext.mcpServerId) { + const orgId = params.skills.organizationId; + const serverId = invocationContext.mcpServerId; + const scope = this.kgScopeConnectorIds(params); + if (!registeredNames.has(SKILLS_LIST_TOOL)) { + entries.push({ + name: SKILLS_LIST_TOOL, + sig: `${SKILLS_LIST_TOOL}:v1`, + register: (mcpServer: McpServer) => + mcpServer.registerTool( + SKILLS_LIST_TOOL, + { + description: + "Lists this workspace's skills: its procedures and rules for recurring tasks. " + + 'Returns id, title and when to use each. Pass a query to search titles, ' + + `"when to use" and rules. Then call ${SKILL_GET_TOOL} with the id of the one that fits.`, + inputSchema: { + query: z.string().max(200).optional().describe('Words to search for, e.g. "quote" or "invoice".'), + }, + annotations: { title: 'List workspace skills', readOnlyHint: true, openWorldHint: false }, + }, + async (args: { query?: string }) => { + const skills = await this.kgSkills.listForCaller(orgId, serverId, scope, args?.query); + return { + content: [{ type: 'text' as const, text: JSON.stringify({ skills }, null, 2) }], + }; + }, + ) as unknown as ToolHandle, + }); + } + if (!registeredNames.has(SKILL_GET_TOOL)) { + entries.push({ + name: SKILL_GET_TOOL, + sig: `${SKILL_GET_TOOL}:v1`, + register: (mcpServer: McpServer) => + mcpServer.registerTool( + SKILL_GET_TOOL, + { + description: + 'Returns one workspace skill in full: when to use it, its rule and, if it has one, ' + + 'the complete procedure. Call it before a task the skill covers and follow it.', + inputSchema: { + skill: z.string().min(1).max(200).describe('The skill id, or its exact title.'), + }, + annotations: { title: 'Get workspace skill', readOnlyHint: true, openWorldHint: false }, + }, + async (args: { skill: string }) => { + const found = await this.kgSkills.getForCaller(orgId, serverId, scope, args.skill); + if (!found) { + return { + isError: true, + content: [ + { + type: 'text' as const, + text: `No skill "${args.skill}" on this server. Call ${SKILLS_LIST_TOOL} to see the available ones.`, + }, + ], + }; + } + return { content: [{ type: 'text' as const, text: skillAsText(found) }] }; + }, + ) as unknown as ToolHandle, + }); + } + } + // Companion to the Agent Skills Finder connector: its tools find and read // a skill, this one files it as a *suggested* workspace skill. It lives // here rather than in the adapter because it writes to this workspace, @@ -1730,6 +1831,14 @@ export class McpEndpointController { .string() .max(80) .describe('SPDX id from skills_repo_info (e.g. "MIT", "Apache-2.0"), or "unknown".'), + procedure: z + .string() + .max(SKILL_BODY_MAX) + .optional() + .describe( + 'Optional: the full procedure in markdown (steps, templates, output format), in your own words. ' + + `Served on demand by ${SKILL_GET_TOOL}, never pasted into the instructions.`, + ), }, annotations: { title: 'Save skill to workspace', @@ -1745,6 +1854,7 @@ export class McpEndpointController { instruction: string; sourceUrl: string; license: string; + procedure?: string; }) => { const saved = await this.kgSkills.create(orgId, { title: args.title, @@ -1753,6 +1863,7 @@ export class McpEndpointController { mcpServerId: serverId, status: 'pending', source: 'finder', + body: args.procedure, }); const result = { saved: true, @@ -1842,6 +1953,53 @@ export class McpEndpointController { * kg_how_to_obtain one entity at a time. Same scope as that tool. The content * is built when the resource is read, so it is never staler than the graph. */ + /** + * Offers each skill the caller can see as an MCP prompt, so clients with a + * prompt picker (slash commands) can start one by hand. Built with the + * server like the resources; the text is read when the prompt is used. + */ + private async registerSkillPrompts( + mcpServer: McpServer, + organizationId: string, + serverId: string, + visibleConnectorIds: string[], + ): Promise { + const skills = await this.kgSkills.listForCaller(organizationId, serverId, visibleConnectorIds); + const used = new Set(); + for (const skill of skills) { + const name = uniquePromptName(skill.title, used); + try { + mcpServer.registerPrompt( + name, + { title: skill.title, description: skill.whenToUse || undefined }, + async () => { + const found = await this.kgSkills.getForCaller( + organizationId, + serverId, + visibleConnectorIds, + skill.id, + ); + return { + messages: [ + { + role: 'user' as const, + content: { + type: 'text' as const, + text: found + ? `Follow this workspace skill for my next request.\n\n${skillAsText(found)}` + : 'This skill is no longer available.', + }, + }, + ], + }; + }, + ); + } catch (err: any) { + this.logger.warn(`Skill prompt "${name}" not registered: ${err?.message ?? err}`); + } + } + } + private registerKgResource(mcpServer: McpServer, params: ToolSetParams): void { const { kgEnabled, invocationContext } = params; const orgId = invocationContext.organizationId; @@ -1910,3 +2068,35 @@ export class McpEndpointController { return stripEnvVarParams(schema, envVars); } } + +/** One skill as the text a model reads (skill_get, prompts). */ +function skillAsText(skill: { + id: string; + title: string; + whenToUse: string; + instruction: string; + body: string | null; + version: number; +}): string { + const parts = [`# ${skill.title}`, `id: ${skill.id} · version ${skill.version}`]; + if (skill.whenToUse) parts.push(`When to use: ${skill.whenToUse}`); + parts.push(`Rule: ${skill.instruction}`); + if (skill.body) parts.push(`## Procedure\n${skill.body}`); + return parts.join('\n\n'); +} + +/** A prompt name from a skill title: lower-case slug, unique on the server. */ +function uniquePromptName(title: string, used: Set): string { + const base = + title + .toLowerCase() + .normalize('NFKD') + .replace(/[\u0300-\u036f]/g, '') + .replace(/[^a-z0-9]+/g, '-') + .replace(/^-+|-+$/g, '') + .slice(0, 48) || 'skill'; + let name = base; + for (let n = 2; used.has(name); n++) name = `${base}-${n}`; + used.add(name); + return name; +} diff --git a/packages/backend/src/mcp-server/shared-endpoint.assistant.spec.ts b/packages/backend/src/mcp-server/shared-endpoint.assistant.spec.ts index 5803c1560..0b54c43fc 100644 --- a/packages/backend/src/mcp-server/shared-endpoint.assistant.spec.ts +++ b/packages/backend/src/mcp-server/shared-endpoint.assistant.spec.ts @@ -36,7 +36,7 @@ function build(setup: unknown) { resolve: jest.fn().mockResolvedValue(null), clientRedirectUris: jest.fn(async (id?: string) => (id ? (REDIRECTS[id] ?? []) : [])), } as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, { get: () => setup } as any, ); (controller as any).serveStateless = jest.fn( diff --git a/packages/backend/src/mcp-server/shared-endpoint.controller.spec.ts b/packages/backend/src/mcp-server/shared-endpoint.controller.spec.ts index 569c1fdad..4c4ecff06 100644 --- a/packages/backend/src/mcp-server/shared-endpoint.controller.spec.ts +++ b/packages/backend/src/mcp-server/shared-endpoint.controller.spec.ts @@ -98,7 +98,7 @@ function build( resolve: jest.fn().mockResolvedValue(opts.grant ?? null), clientRedirectUris: jest.fn(async (id?: string) => (id ? (REDIRECTS[id] ?? []) : [])), } as any, - { create: jest.fn() } as any, + { create: jest.fn(), hasVisible: jest.fn().mockResolvedValue(false) } as any, ); // Capture the per-request server instead of serving HTTP, then talk to it diff --git a/packages/backend/src/mcp-servers/mcp-servers.controller.ts b/packages/backend/src/mcp-servers/mcp-servers.controller.ts index 5a6712f26..6b12d4e42 100644 --- a/packages/backend/src/mcp-servers/mcp-servers.controller.ts +++ b/packages/backend/src/mcp-servers/mcp-servers.controller.ts @@ -15,7 +15,8 @@ import { import { ApiTags, ApiBearerAuth, ApiOperation, ApiQuery, ApiProperty, ApiPropertyOptional } from '@nestjs/swagger'; import { PaginationQueryDto } from '../common/pagination.dto'; import { AuthGuard } from '@nestjs/passport'; -import { IsString, IsOptional, IsBoolean, IsArray } from 'class-validator'; +import { IsString, IsOptional, IsBoolean, IsArray, IsIn } from 'class-validator'; +import { SKILLS_DELIVERY_MODES, type SkillsDelivery } from '../knowledge-graph/kg-skill.service'; import { McpServersService } from './mcp-servers.service'; import { LicenseGuardService } from '../license/license-guard.service'; import { PrismaService } from '../common/prisma.service'; @@ -73,6 +74,15 @@ class UpdateMcpServerDto { @IsOptional() @IsBoolean() isActive?: boolean; + + @ApiPropertyOptional({ + enum: SKILLS_DELIVERY_MODES, + description: + 'How applied skills reach AI clients on connect: auto (full text while small, else an index for skill_get), inline (always the full text), index (always the index).', + }) + @IsOptional() + @IsIn(SKILLS_DELIVERY_MODES as unknown as string[]) + skillsDelivery?: SkillsDelivery; } class AssignConnectorsDto { diff --git a/packages/backend/src/mcp-servers/mcp-servers.instructions.spec.ts b/packages/backend/src/mcp-servers/mcp-servers.instructions.spec.ts index b23ddf44e..aee0c66b7 100644 --- a/packages/backend/src/mcp-servers/mcp-servers.instructions.spec.ts +++ b/packages/backend/src/mcp-servers/mcp-servers.instructions.spec.ts @@ -46,7 +46,7 @@ describe('McpServersService — caller-scoped instructions and resources', () => expect(text).toContain('Use the CRM.'); expect(text).not.toContain('Ledger rules.'); expect(text).not.toContain('Fibu'); - expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', 'srv-1', ['c-crm']); + expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', 'srv-1', ['c-crm'], 'auto'); }); it('scopes the assignment query to the visible ids AND the server organization', async () => { @@ -64,7 +64,7 @@ describe('McpServersService — caller-scoped instructions and resources', () => const text = await service.getComposedInstructions('srv-1', []); expect(text).toBe('SERVER-GUIDANCE\n\n## Workspace skills\n- server-wide'); expect(prisma.mcpServerConnector.findMany).not.toHaveBeenCalled(); - expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', 'srv-1', []); + expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', 'srv-1', [], 'auto'); }); it('getVisibleContent returns the visible connectors with instructions and their stored rows', async () => { @@ -139,8 +139,8 @@ describe('McpServersService — shared guide skills per workspace', () => { const text = await service.getSharedGuide({ connectorIds: ['c-a', 'c-b'], serverIds: ['srv-a'] }); expect(kgSkills.activeSkillsText).toHaveBeenCalledTimes(2); - expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', ['srv-a'], ['c-a']); - expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-B', [], ['c-b']); + expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-A', ['srv-a'], ['c-a'], 'full'); + expect(kgSkills.activeSkillsText).toHaveBeenCalledWith('org-B', [], ['c-b'], 'full'); expect(text).toContain('- org-A'); expect(text).toContain('- org-B'); }); diff --git a/packages/backend/src/mcp-servers/mcp-servers.service.ts b/packages/backend/src/mcp-servers/mcp-servers.service.ts index eb21cbaee..338e89e3f 100644 --- a/packages/backend/src/mcp-servers/mcp-servers.service.ts +++ b/packages/backend/src/mcp-servers/mcp-servers.service.ts @@ -1,6 +1,10 @@ import { ConflictException, Injectable, Logger } from '@nestjs/common'; import { PrismaService } from '../common/prisma.service'; -import { KgSkillService } from '../knowledge-graph/kg-skill.service'; +import { + KgSkillService, + SKILLS_DELIVERY_MODES, + type SkillsDelivery, +} from '../knowledge-graph/kg-skill.service'; import { McpSessionManager } from './mcp-session.manager'; /** What the per-server endpoint may serve as text to one caller. */ @@ -163,7 +167,7 @@ export class McpServersService { ); } - async update(id: string, data: { name?: string; slug?: string; description?: string; instructions?: string; isActive?: boolean }) { + async update(id: string, data: { name?: string; slug?: string; description?: string; instructions?: string; isActive?: boolean; skillsDelivery?: string }) { return this.withSlugConflict(data.slug, () => this.prisma.mcpServerConfig.update({ where: { id }, @@ -390,10 +394,12 @@ export class McpServersService { // skills with its own organization and only its own server/connector ids. const orgIds = [...new Set(connectors.map((c) => c.organizationId))]; for (const orgId of orgIds) { + // No skill_get on the shared endpoint: procedures are returned in full. const skillsText = await this.kgSkills.activeSkillsText( orgId, servers.filter((s) => s.organizationId === orgId).map((s) => s.id), connectors.filter((c) => c.organizationId === orgId).map((c) => c.id), + 'full', ); if (skillsText) parts.push(skillsText); } @@ -481,7 +487,7 @@ export class McpServersService { serverId: string, visibleConnectorIds: string[], ): Promise<{ - server: { instructions: string | null; organizationId: string } | null; + server: { instructions: string | null; organizationId: string; skillsDelivery?: string } | null; serverConnectors: Array<{ connectorId: string; connector: { id: string; name: string; instructions: string | null }; @@ -489,7 +495,7 @@ export class McpServersService { }> { const server = await this.prisma.mcpServerConfig.findUnique({ where: { id: serverId }, - select: { instructions: true, organizationId: true }, + select: { instructions: true, organizationId: true, skillsDelivery: true }, }); const visible = [...new Set(visibleConnectorIds)]; if (!server || visible.length === 0) return { server, serverConnectors: [] }; @@ -511,7 +517,7 @@ export class McpServersService { private async composeInstructions( serverId: string, - server: { instructions: string | null; organizationId: string } | null, + server: { instructions: string | null; organizationId: string; skillsDelivery?: string } | null, serverConnectors: Array<{ connectorId: string; connector: { name: string; instructions: string | null }; @@ -537,6 +543,7 @@ export class McpServersService { server.organizationId, serverId, serverConnectors.map((sc) => sc.connectorId), + toDelivery(server.skillsDelivery), ) : null; if (skillsText) parts.push(skillsText); @@ -589,3 +596,9 @@ export class McpServersService { || 'server'; } } + +function toDelivery(value: string | undefined): SkillsDelivery { + return (SKILLS_DELIVERY_MODES as readonly string[]).includes(value ?? '') + ? (value as SkillsDelivery) + : 'auto'; +} diff --git a/packages/frontend/src/app/knowledge-graph/skills/page.tsx b/packages/frontend/src/app/knowledge-graph/skills/page.tsx index 80e039c6a..dacb13a10 100644 --- a/packages/frontend/src/app/knowledge-graph/skills/page.tsx +++ b/packages/frontend/src/app/knowledge-graph/skills/page.tsx @@ -8,6 +8,7 @@ import { mcpServers as mcpServersApi, connectors as connectorsApi, type KgSkill, + type KgSkillVersion, } from '@/lib/api'; import { AppShell } from '@/components/app-shell'; import { Button } from '@/components/ui/button'; @@ -212,9 +213,11 @@ export default function SkillsPage() { {/* The header already carries the way back to the graph, so this stays a description and nothing else. */}

- Rules inferred from the intents behind your tool calls, scoped to one connector or to a - whole MCP server. Active skills are composed - into that server's instructions automatically. + Rules and procedures for recurring tasks, scoped to one connector or to a whole MCP + server. Active skills reach that server's AI + clients on connect, as full text or as a list they open with{' '} + skill_get (set per server). Long procedures + are always opened on demand.

{status &&

{status}

} @@ -349,7 +352,23 @@ function SkillCard({ const [title, setTitle] = useState(s.title); const [whenToUse, setWhenToUse] = useState(s.whenToUse); const [instruction, setInstruction] = useState(s.instruction); + const [procedure, setProcedure] = useState(s.body ?? ''); const [busy, setBusy] = useState(false); + const [history, setHistory] = useState(null); + const [historyError, setHistoryError] = useState(''); + + const toggleHistory = async () => { + if (history) { + setHistory(null); + return; + } + setHistoryError(''); + try { + setHistory(await knowledgeGraph.skills.versions(token, s.id)); + } catch (e: any) { + setHistoryError(e.message || 'Could not load the history'); + } + }; const scope = s.mcpServer?.name ? `server: ${s.mcpServer.name}` @@ -397,6 +416,14 @@ function SkillCard({ className={cn(textareaCls, 'bg-[var(--surface-2)]')} placeholder="Instruction" /> +