diff --git a/apps/buddy/electron/preload/local-chat/skills.ts b/apps/buddy/electron/preload/local-chat/skills.ts index 0a0ac4b7..1926194a 100644 --- a/apps/buddy/electron/preload/local-chat/skills.ts +++ b/apps/buddy/electron/preload/local-chat/skills.ts @@ -4,7 +4,7 @@ import { LOCAL_CHAT_IPC_CHANNELS } from '../../shared/localChatApi' export function createSkillsApi(): Pick { return { skills: Object.freeze({ - list: spaceId => ipcRenderer.invoke(LOCAL_CHAT_IPC_CHANNELS.skillsList, { spaceId: spaceId ?? null }), + list: (spaceId, metadataOnly) => ipcRenderer.invoke(LOCAL_CHAT_IPC_CHANNELS.skillsList, { spaceId: spaceId ?? null, metadataOnly }), get: input => ipcRenderer.invoke(LOCAL_CHAT_IPC_CHANNELS.skillsGet, { ...input }), listFiles: input => ipcRenderer.invoke(LOCAL_CHAT_IPC_CHANNELS.skillsListFiles, { ...input }), readFile: input => ipcRenderer.invoke(LOCAL_CHAT_IPC_CHANNELS.skillsReadFile, { ...input }), diff --git a/apps/buddy/electron/shared/localChatApi.ts b/apps/buddy/electron/shared/localChatApi.ts index b7e3cf67..4d88cc21 100644 --- a/apps/buddy/electron/shared/localChatApi.ts +++ b/apps/buddy/electron/shared/localChatApi.ts @@ -317,7 +317,7 @@ export interface LocalChatApi { update: (input: LocalSpaceUpdateInput) => Promise } skills: { - list: (spaceId?: string | null) => Promise + list: (spaceId?: string | null, metadataOnly?: boolean) => Promise get: (input: { spaceId: string | null, id: string }) => Promise listFiles: (input: SkillDirectoryRequest) => Promise readFile: (input: SkillFileTarget) => Promise diff --git a/apps/buddy/service/src/BuddyService.ts b/apps/buddy/service/src/BuddyService.ts index caf3cfa3..53f09e4a 100644 --- a/apps/buddy/service/src/BuddyService.ts +++ b/apps/buddy/service/src/BuddyService.ts @@ -488,6 +488,7 @@ export async function startBuddyService( runInputs, runs, sessions: sessionBlueprints, + skills: skillService, }) const turnLauncher = new BuddyTurnLauncher({ lifecycle: runLifecycleService, diff --git a/apps/buddy/service/src/__tests__/buddyService.integration.spec.ts b/apps/buddy/service/src/__tests__/buddyService.integration.spec.ts index 79699211..10f53c87 100644 --- a/apps/buddy/service/src/__tests__/buddyService.integration.spec.ts +++ b/apps/buddy/service/src/__tests__/buddyService.integration.spec.ts @@ -139,7 +139,7 @@ describe('buddy runtime cross-subsystem contract', () => { resources: { skillReadRoots: [], skillReferences: [], - approvedSkillPaths: [], + approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'resources-1', diff --git a/apps/buddy/service/src/agent/context/__tests__/buddyComposerInputBoundary.spec.ts b/apps/buddy/service/src/agent/context/__tests__/buddyComposerInputBoundary.spec.ts index 31c41d13..768447f4 100644 --- a/apps/buddy/service/src/agent/context/__tests__/buddyComposerInputBoundary.spec.ts +++ b/apps/buddy/service/src/agent/context/__tests__/buddyComposerInputBoundary.spec.ts @@ -617,7 +617,7 @@ async function createFixture(options: { resources: { skillReadRoots: [], skillReferences: [], - approvedSkillPaths: [], + approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: DIRECTORY_CONTEXT, revision: 'offline-s0', diff --git a/apps/buddy/service/src/agent/execution/BuddyRunExecutionPlanner.ts b/apps/buddy/service/src/agent/execution/BuddyRunExecutionPlanner.ts index 542077b9..d0f55b5b 100644 --- a/apps/buddy/service/src/agent/execution/BuddyRunExecutionPlanner.ts +++ b/apps/buddy/service/src/agent/execution/BuddyRunExecutionPlanner.ts @@ -1,5 +1,6 @@ import type { AttachmentService } from '../../attachments/AttachmentService' import type { ProviderExecutionModelResolver } from '../../providers/ProviderExecutionModelResolver' +import type { SkillService } from '../../skills/SkillService' import type { CommandRequestRepository } from '../../storage/commandRequestRepository' import type { ConversationRepository } from '../../storage/conversationRepository' import type { RunInputRepository } from '../../storage/runInputRepository' @@ -25,6 +26,7 @@ export interface BuddyRunExecutionPlannerOptions { runInputs: Pick runs: Pick sessions: Pick + skills: Pick } export class BuddyRunExecutionPlanner { @@ -88,10 +90,13 @@ export class BuddyRunExecutionPlanner { const input = this.#options.runInputs.findByRunId(run.id) if (!input?.prompt.trim()) throw new BuddyAgentRunError('RUN_INPUT_NOT_FOUND') - for (const item of input.contextItems) { - if (item.kind !== 'skill' || !item.skill) - continue - const selected = item.skill + const selections = input.contextItems.flatMap(item => item.kind === 'skill' ? [item.skill ?? item.value] : []) + for (const selection of selections) { + if (typeof selection !== 'string' && !session.resources.skillReferences.some(skill => skill.id === selection.id && skill.name === selection.name)) + throw new SkillError('SKILL_CHANGED') + } + const selectedSkills = await this.#options.skills.materializeForSpace(conversation.spaceId, selections) + for (const { reference: selected } of selectedSkills) { if (!session.resources.skillReferences.some(skill => skill.id === selected.id && skill.name === selected.name && skill.revision === selected.revision)) throw new SkillError('SKILL_CHANGED') } diff --git a/apps/buddy/service/src/agent/execution/__tests__/BuddyAgentRunner.spec.ts b/apps/buddy/service/src/agent/execution/__tests__/BuddyAgentRunner.spec.ts index b38d5e2b..5ed806aa 100644 --- a/apps/buddy/service/src/agent/execution/__tests__/BuddyAgentRunner.spec.ts +++ b/apps/buddy/service/src/agent/execution/__tests__/BuddyAgentRunner.spec.ts @@ -1483,7 +1483,7 @@ function emptyResources() { return { skillReadRoots: [], skillReferences: [], - approvedSkillPaths: [], + approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'resources-1', diff --git a/apps/buddy/service/src/agent/execution/__tests__/BuddyTurnLauncher.spec.ts b/apps/buddy/service/src/agent/execution/__tests__/BuddyTurnLauncher.spec.ts index 1892d2a9..37438103 100644 --- a/apps/buddy/service/src/agent/execution/__tests__/BuddyTurnLauncher.spec.ts +++ b/apps/buddy/service/src/agent/execution/__tests__/BuddyTurnLauncher.spec.ts @@ -5,12 +5,13 @@ import type { BuddyTurnHandle, StartBuddyTurnInput, } from '../turnTypes' -import { mkdir, mkdtemp, realpath, rm, symlink } from 'node:fs/promises' +import { mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterEach, describe, expect, it, vi } from 'vitest' import { createRunEventLog } from '../../../events/createRunEventLog' import { RunLifecycleService } from '../../../runs/RunLifecycleService' +import { SkillService } from '../../../skills/SkillService' import { prepareTestTurnRequest } from '../../../storage/__tests__/composerDraftTestFixture' import { BuddyDataPaths } from '../../../storage/BuddyDataPaths' import { createCommandRequestRepository } from '../../../storage/commandRequestRepository' @@ -18,6 +19,7 @@ import { createConversationRepository } from '../../../storage/conversationRepos import { openBuddyDatabase } from '../../../storage/database' import { createRunInputRepository } from '../../../storage/runInputRepository' import { createRunRepository } from '../../../storage/runRepository' +import { createSkillRepository } from '../../../storage/skillRepository' import { createSpaceRepository } from '../../../storage/spaceRepository' import { BuddySessionBlueprintService } from '../../sessions/BuddySessionBlueprintService' import { BuddyRunExecutionPlanner } from '../BuddyRunExecutionPlanner' @@ -40,6 +42,30 @@ describe('buddyTurnLauncher', () => { expect(fixture.runs.findById('run-1')?.status).toBe('queued') }) + it.each(['legacy', 'current'] as const)('launches an unchanged %s Skill reference and rejects later package changes', async (format) => { + const fixture = await createFixture() + const directory = join(fixture.root, 'agent', 'skills', 'writer') + await mkdir(directory, { recursive: true }) + await writeFile(join(directory, 'SKILL.md'), '---\nname: writer\ndescription: Write text\n---\nWrite carefully.') + const resource = join(directory, 'reference.md') + await writeFile(resource, 'first version') + const [materialized] = await fixture.skills.materializeForSpace(null, ['writer']) + const selectedSkill = format === 'legacy' + ? { id: materialized!.reference.id, name: 'writer', revision: materialized!.reference.packageRevision! } + : materialized!.reference + fixture.prepareTurn({ spaceId: null, selectedSkill }) + + const plan = await fixture.planner.resolve('run-1') + expect(plan.kind).toBe('turn') + expect(plan.input.session.resources.skillReferences).toContainEqual(expect.objectContaining({ + id: selectedSkill.id, + revision: materialized!.reference.revision, + })) + await writeFile(resource, 'second version') + await expect(fixture.planner.resolve('run-1')).rejects.toMatchObject({ code: 'SKILL_CHANGED' }) + expect(fixture.runs.findById('run-1')?.status).toBe('queued') + }) + it('rebuilds the executable turn from persisted run facts', async () => { const fixture = await createFixture() fixture.prepareTurn({ spaceId: null }) @@ -177,19 +203,16 @@ async function createFixture(options: { modelInput?: readonly ('text' | 'image') database, }) const lifecycle = new RunLifecycleService({ eventLog, repository: runs }) + const skills = new SkillService({ + agentDirectory: join(root, 'agent'), + paths, + repository: createSkillRepository(database), + spaces, + }) const blueprints = new BuddySessionBlueprintService({ conversationGrants: { listActive: () => [] }, paths, - skills: { - loadForSpace: async () => ({ - diagnostics: [], - readRoots: [], - references: [], - paths: [], - revision: 'skills-revision-1', - skills: [], - }), - }, + skills, spaces, }) const planner = new BuddyRunExecutionPlanner({ @@ -200,6 +223,7 @@ async function createFixture(options: { modelInput?: readonly ('text' | 'image') runInputs: createRunInputRepository(database), runs, sessions: blueprints, + skills, }) return { createLauncher(overrides: { @@ -222,7 +246,8 @@ async function createFixture(options: { modelInput?: readonly ('text' | 'image') planner, resolveInputReferences, paths, - prepareTurn({ spaceId }: { spaceId: string | null }) { + skills, + prepareTurn({ spaceId, selectedSkill = options.selectedSkill }: { spaceId: string | null, selectedSkill?: SkillReference }) { prepareTestTurnRequest(database, { attachmentBindings: [], branchId: 'branch-1', @@ -239,7 +264,7 @@ async function createFixture(options: { modelInput?: readonly ('text' | 'image') runId: 'run-1', runInput: { attachmentIds: [], - contextItems: options.selectedSkill ? [{ kind: 'skill', value: options.selectedSkill.name, skill: options.selectedSkill }] : [], + contextItems: selectedSkill ? [{ kind: 'skill', value: selectedSkill.name, skill: selectedSkill }] : [], prompt: 'Persisted prompt', reasoning: 'high', serviceTier: 'priority', diff --git a/apps/buddy/service/src/agent/extensions/__tests__/inProcessExtensions.spec.ts b/apps/buddy/service/src/agent/extensions/__tests__/inProcessExtensions.spec.ts index 08b72f78..80bec210 100644 --- a/apps/buddy/service/src/agent/extensions/__tests__/inProcessExtensions.spec.ts +++ b/apps/buddy/service/src/agent/extensions/__tests__/inProcessExtensions.spec.ts @@ -67,7 +67,7 @@ describe('buddy in-process Pi extensions', () => { resources: { skillReadRoots: [], skillReferences: [], - approvedSkillPaths: [], + approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'resources-1', @@ -124,7 +124,7 @@ describe('buddy in-process Pi extensions', () => { resources: { skillReadRoots: [], skillReferences: [], - approvedSkillPaths: [], + approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'resources-1', @@ -216,7 +216,7 @@ describe('buddy in-process Pi extensions', () => { resources: { skillReadRoots: [], skillReferences: [], - approvedSkillPaths: [], + approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'resources-1', @@ -370,7 +370,7 @@ describe('buddy in-process Pi extensions', () => { resources: { skillReadRoots: [], skillReferences: [], - approvedSkillPaths: [], + approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'resources-1', diff --git a/apps/buddy/service/src/agent/extensions/discovery/__tests__/toolDiscoverySession.spec.ts b/apps/buddy/service/src/agent/extensions/discovery/__tests__/toolDiscoverySession.spec.ts index 3fdfeb70..3132f349 100644 --- a/apps/buddy/service/src/agent/extensions/discovery/__tests__/toolDiscoverySession.spec.ts +++ b/apps/buddy/service/src/agent/extensions/discovery/__tests__/toolDiscoverySession.spec.ts @@ -55,7 +55,7 @@ async function fixture() { model, modelRuntime: runtime, inProcessExtensions: [extension, discovery.extension], - resources: { skillReadRoots: [], skillReferences: [], approvedSkillPaths: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'empty' }, + resources: { skillReadRoots: [], skillReferences: [], approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'empty' }, } return { root, options, runtime, model } } diff --git a/apps/buddy/service/src/agent/resources/BuddySessionResources.ts b/apps/buddy/service/src/agent/resources/BuddySessionResources.ts index 080dcc93..aab10747 100644 --- a/apps/buddy/service/src/agent/resources/BuddySessionResources.ts +++ b/apps/buddy/service/src/agent/resources/BuddySessionResources.ts @@ -1,4 +1,4 @@ -import type { SkillReference } from '../../../../shared/skills/skillApi' +import type { LocalSkill, SkillReference } from '../../../../shared/skills/skillApi' import type { SkillService } from '../../skills/SkillService' import type { BoundedContextFilesResult } from './loadBoundedContextFiles' import { createHash } from 'node:crypto' @@ -6,7 +6,7 @@ import { createHash } from 'node:crypto' import { loadBoundedContextFiles } from './loadBoundedContextFiles' export interface BuddySessionResources { - approvedSkillPaths: readonly string[] + approvedSkills: readonly LocalSkill[] skillReadRoots: readonly string[] skillReferences: readonly SkillReference[] context: BoundedContextFilesResult @@ -55,7 +55,7 @@ export async function resolveBuddySessionResources( hash.update('\0') } return { - approvedSkillPaths: skills.paths, + approvedSkills: skills.skills, skillReadRoots: skills.readRoots, skillReferences: skills.references, context, diff --git a/apps/buddy/service/src/agent/resources/createBuddyResourceLoader.ts b/apps/buddy/service/src/agent/resources/createBuddyResourceLoader.ts index c77e1234..cf2d096d 100644 --- a/apps/buddy/service/src/agent/resources/createBuddyResourceLoader.ts +++ b/apps/buddy/service/src/agent/resources/createBuddyResourceLoader.ts @@ -1,9 +1,11 @@ import type { SettingsManager } from '@earendil-works/pi-coding-agent' import type { BuddyApprovalPolicy } from '../../../../shared/permissions/approvalPolicy' import type { BuddyExecutionProfile } from '../../../../shared/permissions/executionProfile' +import type { LocalSkill } from '../../../../shared/skills/skillApi' import type { BuddyInputReferenceV1 } from '../context/BuddyInputReference' import type { BuddyInProcessExtension } from '../extensions/BuddyInProcessExtension' import type { BoundedContextFile } from './loadBoundedContextFiles' +import { dirname } from 'node:path' import process from 'node:process' import { @@ -18,7 +20,7 @@ import { createBuddySystemPrompt } from './createBuddySystemPrompt' export interface CreateBuddyResourceLoaderOptions { getPendingInput?: () => BuddyInputReferenceV1 | null - approvedSkillPaths: readonly string[] + approvedSkills: readonly LocalSkill[] agentDir: string approvalPolicy: BuddyApprovalPolicy boundedContextFiles: readonly BoundedContextFile[] @@ -56,7 +58,7 @@ export async function createBuddyResourceLoader( const loader = new DefaultResourceLoader({ additionalExtensionPaths: [], additionalPromptTemplatePaths: [], - additionalSkillPaths: [...options.approvedSkillPaths], + additionalSkillPaths: [], additionalThemePaths: [], agentDir: options.agentDir, agentsFilesOverride: () => ({ agentsFiles: [...options.boundedContextFiles] }), @@ -67,6 +69,22 @@ export async function createBuddyResourceLoader( noExtensions: true, noPromptTemplates: true, noSkills: true, + skillsOverride: () => ({ + skills: options.approvedSkills.map(skill => ({ + name: skill.name, + description: skill.description, + filePath: skill.filePath, + baseDir: dirname(skill.filePath), + disableModelInvocation: skill.status === 'manual_only', + sourceInfo: { + path: skill.filePath, + source: 'lexora', + scope: skill.source === 'directory' ? 'project' : 'user', + origin: 'top-level', + }, + })), + diagnostics: [], + }), noThemes: true, settingsManager: options.settingsManager ?? createBuddySettingsManager(), systemPrompt, diff --git a/apps/buddy/service/src/agent/sessions/__tests__/BuddySessionBlueprintService.spec.ts b/apps/buddy/service/src/agent/sessions/__tests__/BuddySessionBlueprintService.spec.ts index 2b74a6df..ad012690 100644 --- a/apps/buddy/service/src/agent/sessions/__tests__/BuddySessionBlueprintService.spec.ts +++ b/apps/buddy/service/src/agent/sessions/__tests__/BuddySessionBlueprintService.spec.ts @@ -51,7 +51,7 @@ describe('buddySessionBlueprintService', () => { { canonicalRoot: spaceRoot, grantId: 'directory-1', kind: 'workspace' as const, root: spaceRoot }, { canonicalRoot: scratchRoot, grantId: 'space-1', kind: 'workspace' as const, root: scratchRoot }, ], - resources: { skillReadRoots: [], skillReferences: [], approvedSkillPaths: ['/skills/space/SKILL.md'] }, + resources: { skillReadRoots: [], skillReferences: [], approvedSkills: [] }, scratchRoot, space: { additionalDirectoryBindings: [], diff --git a/apps/buddy/service/src/agent/sessions/__tests__/cacheWarming.spec.ts b/apps/buddy/service/src/agent/sessions/__tests__/cacheWarming.spec.ts index 16a1d562..e1eee9fc 100644 --- a/apps/buddy/service/src/agent/sessions/__tests__/cacheWarming.spec.ts +++ b/apps/buddy/service/src/agent/sessions/__tests__/cacheWarming.spec.ts @@ -65,7 +65,7 @@ describe('buddy cache warming', () => { model, modelRuntime: runtime, thinkingLevel: 'off', - resources: { skillReadRoots: [], skillReferences: [], approvedSkillPaths: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'empty' }, + resources: { skillReadRoots: [], skillReferences: [], approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'empty' }, inProcessExtensions: [{ name: 'lexora-warm-fixture', factory(pi) { diff --git a/apps/buddy/service/src/agent/sessions/__tests__/createBuddySession.spec.ts b/apps/buddy/service/src/agent/sessions/__tests__/createBuddySession.spec.ts index 53d371d9..49e8ec1f 100644 --- a/apps/buddy/service/src/agent/sessions/__tests__/createBuddySession.spec.ts +++ b/apps/buddy/service/src/agent/sessions/__tests__/createBuddySession.spec.ts @@ -1,10 +1,15 @@ -import { mkdir, mkdtemp, realpath, rm, writeFile } from 'node:fs/promises' +import { mkdir, mkdtemp, realpath, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { InMemoryCredentialStore } from '@earendil-works/pi-ai' import { ModelRuntime, SessionManager } from '@earendil-works/pi-coding-agent' import { afterEach, describe, expect, it } from 'vitest' import { createImageGenerationCapability } from '../../../images/imageGenerationExtension' +import { SkillService } from '../../../skills/SkillService' +import { BuddyDataPaths } from '../../../storage/BuddyDataPaths' +import { openBuddyDatabase } from '../../../storage/database' +import { createSkillRepository } from '../../../storage/skillRepository' +import { createSpaceRepository } from '../../../storage/spaceRepository' import { createToolDiscoveryCapability } from '../../extensions/discovery/toolDiscoveryExtension' import { createBuddySession as createPreparedBuddySession } from '../createBuddySession' @@ -76,6 +81,21 @@ describe('createBuddySession', () => { const skillPath = join(directory, 'SKILL.md') await writeFile(skillPath, '---\nname: sample-workflow\ndescription: Inspect sample workflow inputs\n---\nBODY_ONLY_ON_DEMAND\nSee reference.md when needed.\n') await writeFile(join(directory, 'reference.md'), 'REFERENCE_ONLY_ON_DEMAND') + const database = openBuddyDatabase({ databasePath: ':memory:' }) + const skills = new SkillService({ + agentDirectory: join(root, 'agent'), + builtinSkillsDirectories: [join(root, 'skills')], + paths: new BuddyDataPaths(root), + repository: createSkillRepository(database), + spaces: createSpaceRepository(database), + }) + const approved = await skills.loadForSpace(null) + await skills.dispose() + database.close() + const outside = join(root, 'outside.md') + await writeFile(outside, '---\nname: sample-workflow\ndescription: OUTSIDE_METADATA\n---\nOUTSIDE_BODY') + await rm(skillPath) + await symlink(outside, skillPath) const modelRuntime = await ModelRuntime.create({ credentials: new InMemoryCredentialStore(), modelsPath: null, refreshOnCreate: false }) const model = modelRuntime.getModels()[0]! const result = await createBuddySession({ @@ -88,7 +108,7 @@ describe('createBuddySession', () => { ...createRuntimeOptions(), model, modelRuntime, - resources: { ...emptyResources(), skillReadRoots: [], skillReferences: [], approvedSkillPaths: [skillPath] }, + resources: { ...emptyResources(), skillReadRoots: [], skillReferences: [], approvedSkills: approved.skills }, }) try { expect(result.session.systemPrompt).toContain('') @@ -98,6 +118,7 @@ describe('createBuddySession', () => { expect(result.session.systemPrompt).toContain('Use the read tool to load a skill') expect(result.session.systemPrompt).not.toContain('BODY_ONLY_ON_DEMAND') expect(result.session.systemPrompt).not.toContain('REFERENCE_ONLY_ON_DEMAND') + expect(result.session.systemPrompt).not.toContain('OUTSIDE_METADATA') } finally { await result.shutdown('quit') @@ -238,7 +259,7 @@ function emptyResources() { return { skillReadRoots: [], skillReferences: [], - approvedSkillPaths: [], + approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'empty', diff --git a/apps/buddy/service/src/agent/sessions/createBuddySession.ts b/apps/buddy/service/src/agent/sessions/createBuddySession.ts index 8d9ac646..dbffa646 100644 --- a/apps/buddy/service/src/agent/sessions/createBuddySession.ts +++ b/apps/buddy/service/src/agent/sessions/createBuddySession.ts @@ -220,7 +220,7 @@ async function createConfiguredBuddySession( const settingsManager = createBuddySettingsManager() const resourceLoader = await createBuddyResourceLoader({ getPendingInput: options.getPendingInput, - approvedSkillPaths: [...options.resources.approvedSkillPaths], + approvedSkills: [...options.resources.approvedSkills], agentDir: runtime.agentDir, approvalPolicy: options.approvalPolicy, boundedContextFiles: context.agentsFiles, diff --git a/apps/buddy/service/src/agent/sessions/tree/__tests__/BuddyConversationTree.spec.ts b/apps/buddy/service/src/agent/sessions/tree/__tests__/BuddyConversationTree.spec.ts index 5ac56e64..09b01dfd 100644 --- a/apps/buddy/service/src/agent/sessions/tree/__tests__/BuddyConversationTree.spec.ts +++ b/apps/buddy/service/src/agent/sessions/tree/__tests__/BuddyConversationTree.spec.ts @@ -261,7 +261,7 @@ async function createFixture(recovery?: BuddySessionRecoveryService['create']) { }), }, }) - const sessionOptions = (branchId: string) => ({ agentDir: join(root, 'agent'), branchId, canonicalRoot: root, conversationId: 'conversation', conversationsDirectory: join(root, 'conversations'), cwd: root, approvalPolicy: 'policy' as const, executionProfile: 'workspace_write' as const, inProcessExtensions: [], model, modelRuntime, resources: { skillReadRoots: [], skillReferences: [], approvedSkillPaths: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'test' } }) + const sessionOptions = (branchId: string) => ({ agentDir: join(root, 'agent'), branchId, canonicalRoot: root, conversationId: 'conversation', conversationsDirectory: join(root, 'conversations'), cwd: root, approvalPolicy: 'policy' as const, executionProfile: 'workspace_write' as const, inProcessExtensions: [], model, modelRuntime, resources: { skillReadRoots: [], skillReferences: [], approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'test' } }) return { root, tree, diff --git a/apps/buddy/service/src/chat/ChatTurnService.ts b/apps/buddy/service/src/chat/ChatTurnService.ts index b06d3c41..e5681b61 100644 --- a/apps/buddy/service/src/chat/ChatTurnService.ts +++ b/apps/buddy/service/src/chat/ChatTurnService.ts @@ -298,7 +298,7 @@ export class ChatTurnService { async validatePreparedInput(input: PrepareTurnRequestInput, validateSkills = true): Promise { if (validateSkills) - await this.#validateSkillItems(input.spaceId, input.runInput.contextItems) + input.runInput.contextItems = await this.#validateSkillItems(input.spaceId, input.runInput.contextItems) await this.#options.inputValidation.validate({ conversationId: input.conversationId, branchId: input.branchId, @@ -617,7 +617,9 @@ export class ChatTurnService { async #validateSkillItems(spaceId: string | null, items: readonly RunInputRecord['contextItems'][number][]) { const selections = items.filter(item => item.kind === 'skill').map(item => item.skill ?? item.value) - await this.#options.skills.materializeForSpace(spaceId, selections) + const loaded = await this.#options.skills.materializeForSpace(spaceId, selections) + const references = new Map(loaded.map(skill => [skill.name, skill.reference])) + return items.map(item => item.kind === 'skill' ? { ...item, skill: references.get(item.value)! } : item) } #resolveConversationSpace(conversation: ConversationRecord): SpaceRecord | null { diff --git a/apps/buddy/service/src/connectors/mcp/__tests__/McpDiscovery.spec.ts b/apps/buddy/service/src/connectors/mcp/__tests__/McpDiscovery.spec.ts index bb24dfb5..babee22a 100644 --- a/apps/buddy/service/src/connectors/mcp/__tests__/McpDiscovery.spec.ts +++ b/apps/buddy/service/src/connectors/mcp/__tests__/McpDiscovery.spec.ts @@ -129,7 +129,7 @@ it('advertises enabled MCP capabilities, discovers Chinese queries, and calls th model, modelRuntime: runtime, inProcessExtensions: [mcp.extension, discovery.extension, policy], - resources: { skillReadRoots: [], skillReferences: [], approvedSkillPaths: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'empty' }, + resources: { skillReadRoots: [], skillReferences: [], approvedSkills: [], context: { agentsFiles: [], diagnostics: [] }, directoryContext: '', revision: 'empty' }, } const preview = await createIsolatedBuddyContextSnapshot(options) created = await createIsolatedBuddySession(options) diff --git a/apps/buddy/service/src/skills/SkillPackageCache.ts b/apps/buddy/service/src/skills/SkillPackageCache.ts index 77e5f1d8..265a8e51 100644 --- a/apps/buddy/service/src/skills/SkillPackageCache.ts +++ b/apps/buddy/service/src/skills/SkillPackageCache.ts @@ -3,7 +3,7 @@ import type { LoadedSkill } from './skillFiles' import { createHash } from 'node:crypto' import { lstat, readdir } from 'node:fs/promises' import { basename, dirname, join, relative } from 'node:path' -import { MAX_SKILL_BYTES, MAX_SKILL_FILES, MAX_SKILL_PACKAGE_BYTES, readSkill, requireSkillPath, SkillError } from './skillFiles' +import { MAX_SKILL_BYTES, MAX_SKILL_FILES, MAX_SKILL_PACKAGE_BYTES, readSkill, readSkillDocument, requireSkillPath, SkillError } from './skillFiles' export type ResolvedSkill = Omit @@ -16,6 +16,7 @@ const MAX_CACHED_PACKAGES = 256 export class SkillPackageCache { readonly #packages = new Map() + readonly #documents = new Map() readonly #pending = new Map>() async load(filePath: string, allowedRoot: string): Promise { @@ -31,8 +32,31 @@ export class SkillPackageCache { return loading } + async loadMetadata(filePath: string, allowedRoot: string): Promise { + const path = await requireSkillPath(allowedRoot, filePath) + const signature = fileSignature(path, await lstat(path, { bigint: true })) + const cached = this.#documents.get(path) + if (cached?.signature === signature) { + this.#documents.delete(path) + this.#documents.set(path, cached) + return cached.skill + } + this.#documents.delete(path) + const document = await readSkillDocument(path, allowedRoot) + if (!document.description) + throw new SkillError('SKILL_INVALID') + if (fileSignature(path, await lstat(path, { bigint: true })) !== signature) + throw new SkillError('SKILL_CHANGED') + const skill = { ...document, revision: document.referenceRevision } + this.#documents.set(path, { signature, skill }) + while (this.#documents.size > MAX_CACHED_PACKAGES) + this.#documents.delete(this.#documents.keys().next().value!) + return skill + } + clear() { this.#packages.clear() + this.#documents.clear() } async #load(path: string, allowedRoot: string): Promise { diff --git a/apps/buddy/service/src/skills/SkillService.ts b/apps/buddy/service/src/skills/SkillService.ts index bd014e85..dca09091 100644 --- a/apps/buddy/service/src/skills/SkillService.ts +++ b/apps/buddy/service/src/skills/SkillService.ts @@ -6,7 +6,7 @@ import type { SpaceRepository } from '../storage/spaceRepository' import type { LoadedSkill } from './skillFiles' import type { ResolvedSkill } from './SkillPackageCache' import { createHash, randomUUID } from 'node:crypto' -import { mkdir, readdir, rm } from 'node:fs/promises' +import { mkdir, readdir, realpath, rm } from 'node:fs/promises' import { basename, dirname, join, relative } from 'node:path' import { isSkillAvailable, skillsRpc } from '../../../shared/skills/skillApi' import { registerRuntimeRequest } from '../rpc/runtimeRequest' @@ -42,9 +42,16 @@ interface SkillServiceOptions { } interface Candidate { + allowedRoot: string | null entry: LocalSkill loaded: ResolvedSkill | null priority: number + referenceRevision: string +} + +interface ResolvedCatalog { + candidates: Candidate[] + catalog: LocalSkillCatalog } interface ImportPreview { @@ -59,8 +66,10 @@ export class SkillService { readonly #previews = new Map() readonly #inspector: SkillInspector readonly #packages = new SkillPackageCache() - readonly #resolutions = new Map>() + readonly #resolutions = new Map>() + readonly #resolved = new Map() #mutation: Promise = Promise.resolve() + #generation = 0 constructor(options: SkillServiceOptions) { this.#options = options @@ -83,19 +92,26 @@ export class SkillService { } } - async list(spaceId: string | null): Promise { - return (await this.#resolve(spaceId)).catalog + async list(spaceId: string | null, metadataOnly = false): Promise { + this.#invalidateResolutions(spaceId) + return (await this.#resolve(spaceId, metadataOnly)).catalog } async loadForSpace(spaceId: string | null): Promise { - const { catalog, candidates } = await this.#resolve(spaceId) + const { catalog, candidates } = await this.#resolve(spaceId, true) const effective = candidates.filter(candidate => isSkillAvailable(candidate.entry)) + const revision = createHash('sha256').update(JSON.stringify(effective.map(candidate => ({ + id: candidate.entry.id, + revision: candidate.referenceRevision, + status: candidate.entry.status, + enabled: candidate.entry.enabled, + })))).digest('hex') return { diagnostics: catalog.diagnostics, paths: effective.map(candidate => candidate.entry.filePath), readRoots: [...new Set(effective.map(candidate => dirname(candidate.entry.filePath)))], - references: effective.map(candidate => reference(candidate.entry)), - revision: catalog.revision, + references: effective.map(candidate => reference(candidate.entry, candidate.referenceRevision)), + revision, skills: effective.map(candidate => candidate.entry).sort((a, b) => a.name.localeCompare(b.name)), } } @@ -103,23 +119,43 @@ export class SkillService { async materializeForSpace(spaceId: string | null, selections: readonly (string | SkillReference)[]): Promise { if (!selections.length) return [] - const { candidates } = await this.#resolve(spaceId) + const { candidates } = await this.#resolve(spaceId, true) + const state = this.#resolutionKey(spaceId, true) const selected = new Map() for (const selection of selections) { const name = typeof selection === 'string' ? selection : selection.name const candidate = candidates.find(item => item.entry.name === name && isSkillAvailable(item.entry)) - if (!candidate?.loaded) + if (!candidate?.allowedRoot) throw new SkillError('SKILL_NOT_FOUND') - if (typeof selection !== 'string' && (selection.id !== candidate.entry.id || selection.revision !== candidate.entry.revision)) + let loaded: ResolvedSkill + try { + loaded = await this.#packages.load(candidate.entry.filePath, candidate.allowedRoot) + } + catch (error) { + this.#invalidateResolutions(spaceId) + throw error + } + if (loaded.referenceRevision !== candidate.referenceRevision) { + this.#invalidateResolutions(spaceId) throw new SkillError('SKILL_CHANGED') + } + if (typeof selection !== 'string' && ( + selection.id !== candidate.entry.id + || (selection.revision !== candidate.referenceRevision && selection.revision !== loaded.revision) + || (selection.packageRevision && selection.packageRevision !== loaded.revision) + )) { + throw new SkillError('SKILL_CHANGED') + } selected.set(name, { name, - body: candidate.loaded.body, + body: loaded.body, filePath: candidate.entry.filePath, - baseDirectory: candidate.loaded.baseDirectory, - reference: reference(candidate.entry), + baseDirectory: loaded.baseDirectory, + reference: reference(candidate.entry, candidate.referenceRevision, loaded.revision), }) } + if (this.#resolutionKey(spaceId, true) !== state) + throw new SkillError('SKILL_CHANGED') return [...selected.values()] } @@ -321,20 +357,37 @@ export class SkillService { async dispose() { await this.#mutation.catch(() => {}) await Promise.allSettled([...this.#resolutions.values()]) + this.#resolved.clear() this.#packages.clear() await Promise.all([...this.#previews.keys()].map(id => this.discard(id))) } - #resolve(spaceId: string | null) { + async #resolve(spaceId: string | null, lightweight = false): Promise { const space = this.#requireSpace(spaceId) const scope = JSON.stringify(space?.primaryDirectory ?? null) - const key = JSON.stringify([spaceId, scope, this.#options.repository.list()]) + const cacheId = this.#resolutionCacheId(spaceId, lightweight) + const state = this.#resolutionKey(spaceId, lightweight) + const generation = this.#generation + const key = JSON.stringify([state, generation]) + const cached = this.#resolved.get(cacheId) + if (cached?.key === key && await this.#isCatalogCurrent(cached.result)) { + if (this.#resolutionKey(spaceId, lightweight) !== state) + throw new SkillError('SKILL_CHANGED') + if (this.#generation === generation) + return cached.result + } + if (this.#generation !== generation) + return this.#resolve(spaceId, lightweight) const pending = this.#resolutions.get(key) if (pending) return pending - const resolving = this.#resolveCatalog(spaceId).then((result) => { + const resolving = this.#resolveCatalog(spaceId, lightweight).then((result) => { if (JSON.stringify(this.#requireSpace(spaceId)?.primaryDirectory ?? null) !== scope) throw new SkillError('SKILL_CHANGED') + if (lightweight && this.#resolutionKey(spaceId, lightweight) !== state) + throw new SkillError('SKILL_CHANGED') + if (lightweight && this.#generation === generation) + this.#resolved.set(cacheId, { key, result }) return result }).finally(() => { if (this.#resolutions.get(key) === resolving) @@ -344,7 +397,38 @@ export class SkillService { return resolving } - async #resolveCatalog(spaceId: string | null) { + #resolutionCacheId(spaceId: string | null, lightweight: boolean) { + return JSON.stringify([spaceId, lightweight]) + } + + #invalidateResolutions(spaceId: string | null) { + this.#generation++ + this.#resolved.delete(this.#resolutionCacheId(spaceId, false)) + this.#resolved.delete(this.#resolutionCacheId(spaceId, true)) + } + + async #isCatalogCurrent(result: ResolvedCatalog): Promise { + for (const candidate of result.candidates) { + if (!candidate.loaded || !candidate.allowedRoot) + continue + try { + if (await realpath(candidate.allowedRoot) !== candidate.allowedRoot) + return false + const document = await this.#packages.loadMetadata(candidate.entry.filePath, candidate.allowedRoot) + if (document.path !== candidate.loaded.path || document.referenceRevision !== candidate.referenceRevision) + return false + } + catch { return false } + } + return true + } + + #resolutionKey(spaceId: string | null, lightweight: boolean) { + const space = this.#requireSpace(spaceId) + return JSON.stringify([spaceId, JSON.stringify(space?.primaryDirectory ?? null), this.#options.repository.list(), lightweight]) + } + + async #resolveCatalog(spaceId: string | null, lightweight = false) { const space = this.#requireSpace(spaceId) const diagnostics: Array<{ code: 'SKILL_INVALID' | 'SKILL_NAME_COLLISION' | 'SKILL_PATH_OUTSIDE_SOURCE' | 'SKILL_SOURCE_UNREADABLE', message: string, path?: string }> = [] const candidates: Candidate[] = [] @@ -358,15 +442,18 @@ export class SkillService { : []), ] const discovered = new Map() + const discoveredRoots = new Map() for (const source of sources) { try { const root = await requireSkillPath(source.allowedRoot, source.root) for (const path of await discoverSkillFiles(root, source.kind !== 'application', path => diagnostics.push({ code: 'SKILL_PATH_OUTSIDE_SOURCE', message: 'Skill source is outside the allowed folder or cannot be read.', path }))) { try { - const loaded = await this.#packages.load(path, root) + const loaded = lightweight + ? await this.#packages.loadMetadata(path, root) + : await this.#packages.load(path, root) const id = skillIdentity(source.kind === 'application' ? `application:${loaded.name}` : `${source.kind}:${path}`) if (source.kind === 'directory') { - candidates.push({ loaded, priority: 1, entry: { + candidates.push({ allowedRoot: root, loaded, priority: 1, referenceRevision: loaded.referenceRevision, entry: { id, name: loaded.name, description: loaded.description, @@ -377,6 +464,7 @@ export class SkillService { status: loaded.manualOnly ? 'manual_only' : 'available', shadowedBy: null, revision: loaded.revision, + referenceRevision: loaded.referenceRevision, filePath: path, origin: null, canRemove: false, @@ -390,7 +478,7 @@ export class SkillService { if (!existing && records.some(record => record.name === loaded.name && record.managedBy === source.kind && record.spaceId === null)) continue const now = new Date().toISOString() - if (!existing || existing.path !== loaded.path || existing.revision !== loaded.revision) { + if (!lightweight && (!existing || existing.path !== loaded.path || existing.revision !== loaded.revision)) { this.#options.repository.save({ id, name: loaded.name, @@ -406,13 +494,40 @@ export class SkillService { }) } discovered.set(id, loaded) + discoveredRoots.set(id, root) + if (lightweight && !existing) { + candidates.push({ + allowedRoot: root, + loaded, + priority: source.kind === 'application' ? 3 : 4, + referenceRevision: loaded.referenceRevision, + entry: { + id, + name: loaded.name, + description: loaded.description, + source: 'global', + spaceId: null, + managedBy: source.kind, + enabled: true, + status: loaded.manualOnly ? 'manual_only' : 'available', + shadowedBy: null, + revision: loaded.revision, + referenceRevision: loaded.referenceRevision, + filePath: loaded.path, + origin: { kind: source.kind === 'application' ? 'application' : 'directory', location: root }, + canRemove: false, + canUpdate: false, + busy: false, + }, + }) + } } } catch { diagnostics.push({ code: 'SKILL_INVALID', message: 'Skill metadata or bundled resources are invalid.', path }) if (source.kind === 'directory') { const name = basename(path) === 'SKILL.md' ? basename(dirname(path)) : basename(path, '.md') - candidates.push({ loaded: null, priority: 1, entry: { + candidates.push({ allowedRoot: root, loaded: null, priority: 1, referenceRevision: 'invalid', entry: { id: skillIdentity(`directory:${path}`), name, description: '', @@ -423,6 +538,7 @@ export class SkillService { status: 'invalid', shadowedBy: null, revision: 'invalid', + referenceRevision: 'invalid', filePath: path, origin: null, canRemove: false, @@ -441,22 +557,32 @@ export class SkillService { } for (const record of this.#options.repository.list().filter(record => !record.spaceId || record.spaceId === spaceId)) { let loaded = discovered.get(record.id) ?? null + let allowedRoot = discoveredRoots.get(record.id) ?? null if (record.managedBy === 'user') { try { + allowedRoot = this.#options.paths.skillsDirectory(record.spaceId) await requireSkillPath(this.#options.paths.root, record.path) - loaded = await this.#packages.load(record.path, this.#options.paths.skillsDirectory(record.spaceId)) + loaded = lightweight + ? await this.#packages.loadMetadata(record.path, allowedRoot) + : await this.#packages.load(record.path, allowedRoot) if (loaded.name !== record.name) loaded = null } - catch { loaded = null } + catch { + loaded = null + allowedRoot = null + } } candidates.push({ + allowedRoot, loaded, priority: record.spaceId ? 2 : record.managedBy === 'external' ? 4 : 3, + referenceRevision: loaded?.referenceRevision ?? record.revision, entry: { id: record.id, name: record.name, description: loaded?.description ?? record.description, + referenceRevision: loaded?.referenceRevision ?? record.revision, source: record.spaceId ? 'space' : 'global', spaceId: record.spaceId, managedBy: record.managedBy, @@ -489,6 +615,7 @@ export class SkillService { } async #currentSkill(spaceId: string | null, id: string) { + this.#invalidateResolutions(spaceId) const { catalog } = await this.#resolve(spaceId) const skill = catalog.skills.find(skill => skill.id === id) if (!skill) @@ -540,8 +667,12 @@ export class SkillService { } } -function reference(skill: LocalSkill): SkillReference { - return { id: skill.id, name: skill.name, revision: skill.revision } +function reference( + skill: LocalSkill, + revision = skill.referenceRevision ?? skill.revision, + packageRevision?: string, +): SkillReference { + return { id: skill.id, name: skill.name, revision, ...(packageRevision ? { packageRevision } : {}) } } export function formatBuddySkillPrompt(skill: BuddyMaterializedSkill): string { @@ -551,7 +682,7 @@ export function formatBuddySkillPrompt(skill: BuddyMaterializedSkill): string { export function registerSkillServiceRpc(rpc: RuntimeRequestRegistrar, service: SkillService): () => void { const stops = [ - registerRuntimeRequest(rpc, skillsRpc.list, input => service.list(input.spaceId)), + registerRuntimeRequest(rpc, skillsRpc.list, input => service.list(input.spaceId, input.metadataOnly)), registerRuntimeRequest(rpc, skillsRpc.get, input => service.get(input.spaceId, input.id)), registerRuntimeRequest(rpc, skillsRpc.listFiles, input => service.listFiles(input)), registerRuntimeRequest(rpc, skillsRpc.readFile, input => service.readFile(input)), diff --git a/apps/buddy/service/src/skills/__tests__/SkillPackageCache.spec.ts b/apps/buddy/service/src/skills/__tests__/SkillPackageCache.spec.ts index 49468370..5a44abc6 100644 --- a/apps/buddy/service/src/skills/__tests__/SkillPackageCache.spec.ts +++ b/apps/buddy/service/src/skills/__tests__/SkillPackageCache.spec.ts @@ -15,6 +15,44 @@ afterEach(async () => { }) describe('skillPackageCache', () => { + it('loads skill metadata without traversing bundled resources', async () => { + const f = await fixture() + await rm(join(f.root, 'references'), { recursive: true }) + let nested = f.root + for (let depth = 0; depth < 18; depth++) { + nested = join(nested, `level-${depth}`) + await mkdir(nested) + } + await writeFile(join(nested, 'guide.md'), 'resource') + + const metadata = await f.cache.loadMetadata(f.path, f.root) + + expect(metadata.name).toBe('workflow') + expect(metadata.description).toBe('A local workflow') + expect(metadata.referenceRevision).toBeTruthy() + await expect(f.cache.load(f.path, f.root)).rejects.toMatchObject({ code: 'SKILL_TOO_LARGE' }) + }) + + it('reuses unchanged document bytes while detecting same-size edits and path escapes', async () => { + const f = await fixture() + const first = await f.cache.loadMetadata(f.path, f.root) + const read = vi.spyOn(boundedFile, 'readBoundedFile').mockRejectedValue(new Error('Unchanged content must not be read')) + expect(await f.cache.loadMetadata(f.path, f.root)).toEqual(first) + read.mockRestore() + const metadata = await stat(f.path) + await writeFile(f.path, first.content.replace('A local workflow', 'A newer workflow')) + await utimes(f.path, metadata.atime, metadata.mtime) + const changed = await f.cache.loadMetadata(f.path, f.root) + expect(changed.description).toBe('A newer workflow') + expect(changed.referenceRevision).not.toBe(first.referenceRevision) + const outside = await mkdtemp(join(tmpdir(), 'buddy-metadata-outside-')) + directories.push(outside) + await writeFile(join(outside, 'SKILL.md'), first.content) + await rm(f.path) + await symlink(join(outside, 'SKILL.md'), f.path) + await expect(f.cache.loadMetadata(f.path, f.root)).rejects.toMatchObject({ code: 'SKILL_INVALID' }) + }) + it('shares a complete revision across concurrent loads and reuses unchanged packages without reading content', async () => { const f = await fixture() const expected = await readSkill(f.path, f.root) diff --git a/apps/buddy/service/src/skills/__tests__/SkillService.spec.ts b/apps/buddy/service/src/skills/__tests__/SkillService.spec.ts index 651381ea..00a4b3e3 100644 --- a/apps/buddy/service/src/skills/__tests__/SkillService.spec.ts +++ b/apps/buddy/service/src/skills/__tests__/SkillService.spec.ts @@ -2,8 +2,9 @@ import type { DatabaseSync } from 'node:sqlite' import { mkdir, mkdtemp, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { afterEach, describe, expect, it } from 'vitest' +import { afterEach, describe, expect, it, vi } from 'vitest' +import * as boundedFile from '../../../../platform/filesystem/boundedFile' import { BuddyDataPaths } from '../../storage/BuddyDataPaths' import { openBuddyDatabase } from '../../storage/database' import { createSkillRepository } from '../../storage/skillRepository' @@ -15,6 +16,7 @@ const directories: string[] = [] const now = '2026-08-14T00:00:00.000Z' afterEach(async () => { + vi.restoreAllMocks() for (const database of databases.splice(0)) database.close() await Promise.all(directories.splice(0).map(path => rm(path, { force: true, recursive: true }))) @@ -48,7 +50,7 @@ describe('skillService', () => { expect(result.diagnostics).toContainEqual(expect.objectContaining({ code: 'SKILL_NAME_COLLISION' })) }) - it('changes the session resource revision when trusted skill content changes', async () => { + it('refreshes changed skill documents without accepting an older selection', async () => { const fixture = await createFixture() await writeSkill(fixture.global, 'mutable', 'first revision') @@ -57,7 +59,79 @@ describe('skillService', () => { const second = await fixture.service.loadForSpace(null) expect(first.paths).toEqual(second.paths) - expect(first.revision).not.toBe(second.revision) + expect(second.revision).not.toBe(first.revision) + await expect(fixture.service.materializeForSpace(null, first.references)).rejects.toMatchObject({ code: 'SKILL_CHANGED' }) + + const third = await fixture.service.loadForSpace(null) + expect(third.revision).not.toBe(first.revision) + expect((await fixture.service.materializeForSpace(null, third.references))[0]?.body).toBe('# mutable') + }) + + it.each(['document', 'source'] as const)('rejects a cached %s replaced by a symlink outside its source', async (target) => { + const fixture = await createFixture() + const root = join(fixture.trustedSpace, '.agents', 'skills') + await writeSkill(root, 'trusted', 'trusted metadata') + fixture.spaces.create(spaceInput('space-trusted', fixture.trustedSpace)) + expect((await fixture.service.loadForSpace('space-trusted')).skills).toHaveLength(1) + const outside = join(fixture.root, 'outside') + await writeSkill(outside, 'trusted', 'outside metadata') + const replaced = target === 'document' ? join(root, 'trusted', 'SKILL.md') : root + const replacement = target === 'document' ? join(outside, 'trusted', 'SKILL.md') : outside + await rm(replaced, { recursive: true }) + await symlink(replacement, replaced, target === 'source' ? 'junction' : 'file') + + const current = await fixture.service.loadForSpace('space-trusted') + expect(current.skills).toEqual([]) + expect(current.readRoots).toEqual([]) + expect(current.diagnostics.length).toBeGreaterThan(0) + }) + + it('keeps discovery and the picker lightweight after a full management inspection', async () => { + const fixture = await createFixture() + await writeSkill(fixture.global, 'large', 'large skill') + let nested = join(fixture.global, 'large') + for (let depth = 0; depth < 18; depth++) { + nested = join(nested, 'nested') + await mkdir(nested) + } + await writeFile(join(nested, 'reference.md'), 'resource') + const initial = await fixture.service.loadForSpace(null) + expect(initial.skills.map(skill => skill.name)).toEqual(['large']) + await fixture.service.list(null) + const picker = await fixture.service.list(null, true) + expect(picker.skills.map(skill => skill.name)).toEqual(['large']) + expect((await fixture.service.loadForSpace(null)).revision).toBe(initial.revision) + await writeSkill(fixture.global, 'new-skill', 'new skill') + expect((await fixture.service.list(null, true)).skills.map(skill => skill.name)).toEqual(['large', 'new-skill']) + }) + + it('rejects revocation while a selected package is being materialized', async () => { + const fixture = await createFixture() + const root = join(fixture.trustedSpace, '.agents', 'skills') + await writeSkill(root, 'trusted', 'trusted skill') + const resource = join(root, 'trusted', 'reference.md') + await writeFile(resource, 'reference') + fixture.spaces.create(spaceInput('space-trusted', fixture.trustedSpace)) + const initial = await fixture.service.loadForSpace('space-trusted') + const read = boundedFile.readBoundedFile + vi.spyOn(boundedFile, 'readBoundedFile').mockImplementation(async (...args) => { + const content = await read(...args) + if (args[1] === resource) { + fixture.spaces.update({ + id: 'space-trusted', + name: 'Trusted', + memoryScope: 'space_only', + primaryDirectory: null, + additionalDirectories: [], + updatedAt: now, + event: { id: 'revoke-during-load', eventType: 'space.config.updated', spaceId: 'space-trusted', createdAt: now, payload: {} }, + }) + } + return content + }) + + await expect(fixture.service.materializeForSpace('space-trusted', initial.references)).rejects.toMatchObject({ code: 'SKILL_CHANGED' }) + expect((await fixture.service.loadForSpace('space-trusted')).skills).toEqual([]) }) it('unloads revoked Space skills and rejects symlink escapes', async () => { @@ -84,18 +158,26 @@ describe('skillService', () => { expect((await fixture.service.list(null)).skills).toEqual([]) }) - it('rejects a queued reference immediately after a supporting resource changes', async () => { + it('does not make supporting resource changes part of the lightweight session revision', async () => { const fixture = await createFixture() await writeSkill(fixture.global, 'mutable', 'unchanged entry') const resource = join(fixture.global, 'mutable', 'guide.md') await writeFile(resource, 'version one') const first = await fixture.service.loadForSpace(null) + const listed = await fixture.service.list(null) + const listedSkill = listed.skills[0]! + const packageReference = { + id: listedSkill.id, + name: listedSkill.name, + revision: listedSkill.referenceRevision ?? listedSkill.revision, + packageRevision: listedSkill.revision, + } await writeFile(resource, 'version two') - await expect(fixture.service.materializeForSpace(null, first.references)).rejects.toMatchObject({ code: 'SKILL_CHANGED' }) + await expect(fixture.service.materializeForSpace(null, [packageReference])).rejects.toMatchObject({ code: 'SKILL_CHANGED' }) + expect((await fixture.service.materializeForSpace(null, first.references))[0]?.body).toBe('# mutable') const second = await fixture.service.loadForSpace(null) - expect(second.revision).not.toBe(first.revision) - expect((await fixture.service.materializeForSpace(null, second.references))[0]?.body).toBe('# mutable') + expect(second.revision).toBe(first.revision) }) it('rejects a pending resolution when the Space directory binding is cleared', async () => { diff --git a/apps/buddy/service/src/skills/__tests__/skillInstallation.spec.ts b/apps/buddy/service/src/skills/__tests__/skillInstallation.spec.ts index 6703d103..b3858647 100644 --- a/apps/buddy/service/src/skills/__tests__/skillInstallation.spec.ts +++ b/apps/buddy/service/src/skills/__tests__/skillInstallation.spec.ts @@ -157,6 +157,7 @@ describe('skill installation lifecycle', () => { expect((await f.service.list('space-a')).skills.find(skill => skill.id === global.id)?.status).toBe('shadowed') await f.service.setEnabled({ spaceId: 'space-a', id: local.id, revision: local.revision, enabled: true }) await writeFile(local.filePath, 'broken') + await f.service.list('space-a') expect((await f.service.loadForSpace('space-a')).skills).toEqual([]) expect((await f.service.list('space-a')).skills.find(skill => skill.id === local.id)?.status).toBe('invalid') }) @@ -216,8 +217,10 @@ describe('skill installation lifecycle', () => { expect((await f.service.materializeForSpace('space-a', [{ id: winner.id, name: winner.name, revision: winner.revision }]))[0]?.body).toBe('Directory instructions') await expect(f.service.materializeForSpace('space-a', [{ id: app.id, name: app.name, revision: app.revision }])).rejects.toMatchObject({ code: 'SKILL_CHANGED' }) await writeFile(join(skillRoot, 'SKILL.md'), 'broken') + await f.service.list('space-a') expect((await f.service.loadForSpace('space-a')).skills).toEqual([]) await rm(skillRoot, { recursive: true }) + await f.service.list('space-a') expect((await f.service.loadForSpace('space-a')).skills.map(skill => skill.id)).toEqual([local.id]) }) diff --git a/apps/buddy/service/src/skills/skillFiles.ts b/apps/buddy/service/src/skills/skillFiles.ts index 60ccf0b8..ba16863f 100644 --- a/apps/buddy/service/src/skills/skillFiles.ts +++ b/apps/buddy/service/src/skills/skillFiles.ts @@ -139,6 +139,7 @@ export async function readSkillDocument(filePath: string, allowedRoot = dirname( hasDeclaredName: typeof frontmatter.name === 'string' && !!frontmatter.name.trim(), description: typeof frontmatter.description === 'string' ? frontmatter.description.trim() : '', content: text, + referenceRevision: createHash('sha256').update(content).digest('hex'), body: body.trim(), metadata: Object.entries(frontmatter).map(([name, value]) => ({ name, value: typeof value === 'string' ? value : JSON.stringify(value, null, 2) ?? '' })), path, diff --git a/apps/buddy/shared/skills/skillApi.ts b/apps/buddy/shared/skills/skillApi.ts index 8da5e58d..56c96643 100644 --- a/apps/buddy/shared/skills/skillApi.ts +++ b/apps/buddy/shared/skills/skillApi.ts @@ -8,6 +8,7 @@ export const skillReferenceSchema = z.object({ id: idSchema, name: z.string().min(1), revision: z.string().min(1), + packageRevision: z.string().min(1).optional(), }).strict() export type SkillReference = z.infer @@ -31,6 +32,7 @@ export const skillSchema = z.object({ status: z.enum(['available', 'manual_only', 'disabled', 'shadowed', 'invalid']), shadowedBy: idSchema.nullable(), revision: z.string(), + referenceRevision: z.string().optional(), filePath: z.string(), origin: skillOriginSchema.nullable(), canUpdate: z.boolean(), @@ -81,7 +83,7 @@ const previewSchema = z.object({ export type SkillInstallPreview = DeepReadonly> export const skillsRequestSchemas = { - skillScope: scopeSchema, + skillScope: scopeSchema.extend({ metadataOnly: z.boolean().optional() }), target: targetSchema, file: fileTargetSchema, directory: directoryRequestSchema, diff --git a/apps/buddy/src/modules/tasks/state/composer/useComposerContextOptions.ts b/apps/buddy/src/modules/tasks/state/composer/useComposerContextOptions.ts index 7c8f9442..c6b9b4e3 100644 --- a/apps/buddy/src/modules/tasks/state/composer/useComposerContextOptions.ts +++ b/apps/buddy/src/modules/tasks/state/composer/useComposerContextOptions.ts @@ -32,7 +32,7 @@ export function useComposerContextOptions(options: ComposerContextOptions) { deepSearch, spaceId, } - const catalog = fileQuery === null ? await options.listSkills(spaceId) : null + const catalog = fileQuery === null ? await options.listSkills(spaceId, true) : null const sources = fileQuery === null ? null : await options.listSources(request) if (current !== scopeVersion) return { files: [], skills: [] } @@ -65,7 +65,11 @@ export function useComposerContextOptions(options: ComposerContextOptions) { path: null, value: skill.name, skillScope: skill.source, - skill: { id: skill.id, name: skill.name, revision: skill.revision }, + skill: { + id: skill.id, + name: skill.name, + revision: skill.referenceRevision ?? skill.revision, + }, })), } }