diff --git a/README.en.md b/README.en.md index 0e4aaf7..6549348 100644 --- a/README.en.md +++ b/README.en.md @@ -131,7 +131,7 @@ The core boundary is deliberately small: providers return normalized data with p - **Market data:** Longbridge is the primary connector for US/HK/CN market data and brokerage portfolio access; Massive is available as a secondary US market-data provider. - **Agent runtime:** Pi runtime for configured LLM providers, with a deterministic local provider for development and offline golden paths. - **Desktop:** Electron with a macOS arm64 packaged build. The renderer, preload bridge, and main-process kernel are separated by context isolation and a whitelisted IPC surface. -- **Skills:** Vendored `SKILL.md` resources with references, enable/disable state, triggers, and capability requirements. +- **Skills:** Vendored `SKILL.md` resources with references, enable/disable state, triggers, and capability requirements, plus safe local user-skill installation from **Settings → Skills**. - **Agent evaluation:** Local or LangSmith-backed evaluation records traces, datasets, evaluators, experiments, and regression gates; engineering metrics can be linked to investment outcomes without claiming causation. ## Quick Start @@ -144,6 +144,11 @@ Download the latest macOS build from the [Releases page](https://github.com/hels 2. Connect Longbridge in **Settings → Connections** if you want live market data and portfolio access. 3. Select a symbol from the Watchlist and open **Deep Research**. +To extend the agent, open **Settings → Skills**, choose **Install from folder**, +and select a skill package containing `SKILL.md`. User skills are stored under +`~/.finagent/skills/`; bundled skills cannot be overwritten, and removing a +user skill moves its package to the system Trash. + Longbridge authentication can also be completed from the terminal: ```bash diff --git a/README.md b/README.md index 27033d3..2d8398e 100644 --- a/README.md +++ b/README.md @@ -131,7 +131,7 @@ Longbridge / Massive 提供商 - **行情数据：** Longbridge 是美股/港股/内地市场数据与券商投资组合访问的主要连接器；Massive 是美股市场的备选数据提供商。 - **Agent 运行时：** 为已配置的 LLM 提供商提供 Pi 运行时，另有用于开发与离线黄金路径的确定性本地提供商。 - **桌面端：** Electron，含 macOS arm64 打包构建。渲染进程、预加载桥与主进程内核通过上下文隔离与白名单化 IPC 接口面分离。 -- **技能：** 内置 `SKILL.md` 资源，含引用、启用/禁用状态、触发器与能力要求。 +- **技能：** 内置 `SKILL.md` 资源，含引用、启用/禁用状态、触发器与能力要求；也可在 **设置 → 技能** 中从本地文件夹安全安装用户 Skill。 - **Agent 评测：** 通过本地 Evaluation Backend 或 LangSmith 记录 trace、dataset、evaluator、experiment 与 regression gate；工程指标与投资结果保持可链接但不宣称因果关系。 ## 快速开始 @@ -144,6 +144,8 @@ Longbridge / Massive 提供商 2. 如需实时行情数据与投资组合访问，在 **设置 → 连接** 中连接 Longbridge。 3. 从自选清单中选择标的，打开**深度研究**。 +要扩展 Agent 能力，在 **设置 → 技能** 选择“从文件夹安装”，并选中一个包含 `SKILL.md` 的 Skill 目录。用户 Skill 保存在 `~/.finagent/skills/`；内置 Skill 不会被覆盖，删除用户 Skill 时会移入系统废纸篓。 + Longbridge 认证也可以在终端完成： ```bash diff --git a/README.zh-CN.md b/README.zh-CN.md index f0de697..50156e3 100644 --- a/README.zh-CN.md +++ b/README.zh-CN.md @@ -131,7 +131,7 @@ Longbridge / Massive 提供商 - **行情数据：** Longbridge 是美股/港股/内地市场数据与券商投资组合访问的主要连接器；Massive 是美股市场的备选数据提供商。 - **Agent 运行时：** 为已配置的 LLM 提供商提供 Pi 运行时，另有用于开发与离线黄金路径的确定性本地提供商。 - **桌面端：** Electron，含 macOS arm64 打包构建。渲染进程、预加载桥与主进程内核通过上下文隔离与白名单化 IPC 接口面分离。 -- **技能：** 内置 `SKILL.md` 资源，含引用、启用/禁用状态、触发器与能力要求。 +- **技能：** 内置 `SKILL.md` 资源，含引用、启用/禁用状态、触发器与能力要求；也可在 **设置 → 技能** 中从本地文件夹安全安装用户 Skill。 - **Agent 评测：** 通过本地 Evaluation Backend 或 LangSmith 记录 trace、dataset、evaluator、experiment 与 regression gate；工程指标与投资结果保持可链接但不宣称因果关系。 ## 快速开始 @@ -144,6 +144,8 @@ Longbridge / Massive 提供商 2. 如需实时行情数据与投资组合访问，在 **设置 → 连接** 中连接 Longbridge。 3. 从自选清单中选择标的，打开**深度研究**。 +要扩展 Agent 能力，在 **设置 → 技能** 选择“从文件夹安装”，并选中一个包含 `SKILL.md` 的 Skill 目录。用户 Skill 保存在 `~/.finagent/skills/`；内置 Skill 不会被覆盖，删除用户 Skill 时会移入系统废纸篓。 + Longbridge 认证也可以在终端完成： ```bash diff --git a/apps/electron/e2e/seed-locale.mjs b/apps/electron/e2e/seed-locale.mjs index af52fe5..ffb4ae9 100644 --- a/apps/electron/e2e/seed-locale.mjs +++ b/apps/electron/e2e/seed-locale.mjs @@ -15,3 +15,16 @@ export function seedLocale(userDataDir, locale = 'en-US') { console.warn(`seedLocale: could not write preference (${String(error)})`); } } + +/** Seed harnesses that test post-onboarding settings/workbench surfaces. */ +export function seedOnboardingCompleted(userDataDir) { + try { + writeFileSync( + join(userDataDir, 'onboarding.json'), + JSON.stringify({ completed: true }), + 'utf8' + ); + } catch (error) { + console.warn(`seedOnboardingCompleted: could not write state (${String(error)})`); + } +} diff --git a/apps/electron/e2e/skills-interactions.mjs b/apps/electron/e2e/skills-interactions.mjs index e1d1619..b27a218 100644 --- a/apps/electron/e2e/skills-interactions.mjs +++ b/apps/electron/e2e/skills-interactions.mjs @@ -11,7 +11,7 @@ // client; the real main-process handler cannot be forced to fail here. import { execSync, spawn } from 'node:child_process'; -import { seedLocale } from './seed-locale.mjs'; +import { seedLocale, seedOnboardingCompleted } from './seed-locale.mjs'; import { existsSync } from 'node:fs'; import { dirname, join } from 'node:path'; import { fileURLToPath } from 'node:url'; @@ -99,6 +99,7 @@ async function main() { execSync(`rm -rf "${userDataDir}"`); execSync(`mkdir -p "${userDataDir}"`); seedLocale(userDataDir, 'en-US'); + seedOnboardingCompleted(userDataDir); const electronProcess = spawn( electronBinary, [electronMain, `--remote-debugging-port=${CDP_PORT}`, '--no-sandbox'], @@ -134,6 +135,7 @@ async function main() { await page.getByRole('button', { name: 'Skills', exact: true }).click(); // Search box renders immediately (part of the loading surface). await page.locator('[data-testid="skills-search"]').waitFor({ timeout: 15_000 }); + await page.locator('[data-testid="skills-install-local"]').waitFor({ timeout: 15_000 }); // The list appears once skills load. await page.locator('[data-testid="skill-list"]').waitFor({ timeout: 15_000 }); await row.waitFor({ timeout: 15_000 }); @@ -181,6 +183,8 @@ async function main() { try { await row.click(); await drawer.waitFor({ timeout: 15_000 }); + const drawerText = (await drawer.textContent()) ?? ''; + if (!drawerText.includes('Bundled')) throw new Error('bundled source label missing'); pass('S4: detail drawer opens on row click'); } catch (error) { fail('S4: detail drawer opens on row click', error); diff --git a/apps/electron/src/main/index.ts b/apps/electron/src/main/index.ts index fca92c9..bddaada 100644 --- a/apps/electron/src/main/index.ts +++ b/apps/electron/src/main/index.ts @@ -1,4 +1,4 @@ -import { app, BrowserWindow, dialog, ipcMain, shell } from 'electron'; +import { app, BrowserWindow, dialog, ipcMain, shell, type OpenDialogOptions } from 'electron'; import { dirname, join } from 'path'; import { fileURLToPath } from 'url'; import { AgentKernelHost, toIpcResult } from './kernelHost.ts'; @@ -307,6 +307,27 @@ ipcMain.handle('skills:readResource', async (_event, skillId: unknown, relativeP toIpcResult(() => agentKernelHost.readSkillResource(skillId, relativePath)) ); +ipcMain.handle('skills:installLocal', async () => + toIpcResult(async () => { + const options: OpenDialogOptions = { + title: 'Install Folio skill', + properties: ['openDirectory'], + }; + const selection = mainWindow + ? await dialog.showOpenDialog(mainWindow, options) + : await dialog.showOpenDialog(options); + if (selection.canceled || selection.filePaths.length === 0) { + return { canceled: true as const }; + } + const installed = await agentKernelHost.installLocalSkillDirectory(selection.filePaths[0]); + return { canceled: false as const, ...installed }; + }) +); + +ipcMain.handle('skills:remove', async (_event, input: unknown) => + toIpcResult(() => agentKernelHost.removeUserSkill(input)) +); + // V7 Evaluation & observability (spec §61-68) ipcMain.handle('evaluation:getSettings', async () => toIpcResult(() => agentKernelHost.getEvaluationSettings()) diff --git a/apps/electron/src/main/kernelHost.ts b/apps/electron/src/main/kernelHost.ts index e9930b5..bc5e408 100644 --- a/apps/electron/src/main/kernelHost.ts +++ b/apps/electron/src/main/kernelHost.ts @@ -164,6 +164,7 @@ import { getLangSmithExtensionEntry, getRuntimeRoot, getSkillsDir, + getUserSkillsDir, listBundledPiExtensions, } from '@finagent/shared/resources'; import { @@ -288,7 +289,10 @@ export class AgentKernelHost { process.env.FINAGENT_PI_EXTENSION = getPiExtensionEntry(); this.credentials = new CredentialStore(join(app.getPath('userData'), 'credentials.json')); this.skillHub = new SkillHub({ - skillsDirectory: getSkillsDir(), + skillsDirectories: [ + { path: getSkillsDir(), source: 'bundled' }, + { path: getUserSkillsDir(), source: 'user' }, + ], stateFile: join(app.getPath('userData'), 'skills-state.json'), }); @@ -2039,7 +2043,7 @@ export class AgentKernelHost { // ------------------------------------------------------------------------- async listSkills() { - const metadataById = new Map(this.skillHub.listSkillMetadata().map((m) => [m.id, m])); + const metadataById = new Map(this.skillHub.listAllSkillMetadata().map((m) => [m.id, m])); return this.skillHub.listSkills().map((skill) => { const meta = metadataById.get(skill.id); return { @@ -2052,10 +2056,40 @@ export class AgentKernelHost { tier: meta?.tier, version: meta?.version, author: meta?.author, + source: meta?.source ?? 'bundled', }; }); } + async installLocalSkillDirectory(sourceDirectory: unknown) { + try { + return await this.skillHub.installSkillFromDirectory( + requireString(sourceDirectory, 'sourceDirectory') + ); + } catch (error) { + if (isCodeError(error)) throw error; + throw createCodeError( + 'SKILL_INSTALL_FAILED', + error instanceof Error ? error.message : 'Skill installation failed.' + ); + } + } + + async removeUserSkill(input: unknown): Promise { + const request = requireObject(input); + const skillId = requireString(request.skillId, 'skillId'); + try { + await shell.trashItem(this.skillHub.userSkillDirectory(skillId)); + await this.skillHub.loadSkills(); + } catch (error) { + if (isCodeError(error)) throw error; + throw createCodeError( + 'SKILL_REMOVE_FAILED', + error instanceof Error ? error.message : 'Skill removal failed.' + ); + } + } + async setSkillEnabled(input: unknown): Promise { const request = requireObject(input); await this.skillHub.setEnabled( @@ -2101,6 +2135,9 @@ export class AgentKernelHost { } const env: NodeJS.ProcessEnv = { FINAGENT_SKILLS_DIR: getSkillsDir(), + FINAGENT_SKILLS_DIRS: JSON.stringify( + this.skillHub.skillsDirectories.map((entry) => entry.path) + ), FINAGENT_PROVIDER_OVERRIDES: overrides.length > 0 ? JSON.stringify(overrides) : '', // V7: the Finagent extension enforces tool-output privacy from this flag // (spec §60) — always set so the level is unambiguous. diff --git a/apps/electron/src/preload/index.cjs b/apps/electron/src/preload/index.cjs index 5cc792a..faed7fe 100644 --- a/apps/electron/src/preload/index.cjs +++ b/apps/electron/src/preload/index.cjs @@ -2,26 +2,20 @@ var __defProp = Object.defineProperty; var __getOwnPropNames = Object.getOwnPropertyNames; var __getOwnPropDesc = Object.getOwnPropertyDescriptor; var __hasOwnProp = Object.prototype.hasOwnProperty; -function __accessProp(key) { - return this[key]; -} +var __moduleCache = /* @__PURE__ */ new WeakMap; var __toCommonJS = (from) => { - var entry = (__moduleCache ??= new WeakMap).get(from), desc; + var entry = __moduleCache.get(from), desc; if (entry) return entry; entry = __defProp({}, "__esModule", { value: true }); - if (from && typeof from === "object" || typeof from === "function") { - for (var key of __getOwnPropNames(from)) - if (!__hasOwnProp.call(entry, key)) - __defProp(entry, key, { - get: __accessProp.bind(from, key), - enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable - }); - } + if (from && typeof from === "object" || typeof from === "function") + __getOwnPropNames(from).map((key) => !__hasOwnProp.call(entry, key) && __defProp(entry, key, { + get: () => from[key], + enumerable: !(desc = __getOwnPropDesc(from, key)) || desc.enumerable + })); __moduleCache.set(from, entry); return entry; }; -var __moduleCache; // src/preload/index.ts var exports_preload = {}; @@ -129,7 +123,9 @@ var electronAPI = { setEnabled: (input) => import_electron.ipcRenderer.invoke("skills:setEnabled", input), listResources: (skillId) => import_electron.ipcRenderer.invoke("skills:listResources", skillId), readResource: (skillId, relativePath) => import_electron.ipcRenderer.invoke("skills:readResource", skillId, relativePath), - readiness: () => import_electron.ipcRenderer.invoke("skills:readiness") + readiness: () => import_electron.ipcRenderer.invoke("skills:readiness"), + installLocal: () => import_electron.ipcRenderer.invoke("skills:installLocal"), + remove: (input) => import_electron.ipcRenderer.invoke("skills:remove", input) }, about: { get: () => import_electron.ipcRenderer.invoke("app:about") diff --git a/apps/electron/src/preload/index.ts b/apps/electron/src/preload/index.ts index 7acffc2..b1bb28c 100644 --- a/apps/electron/src/preload/index.ts +++ b/apps/electron/src/preload/index.ts @@ -89,6 +89,8 @@ export interface ElectronAPI { listResources: (skillId: string) => Promise; readResource: (skillId: string, relativePath: string) => Promise; readiness: () => Promise; + installLocal: () => Promise; + remove: (input: { skillId: string }) => Promise; }; about: { get: () => Promise; @@ -283,6 +285,8 @@ const electronAPI: ElectronAPI = { readResource: (skillId: string, relativePath: string) => ipcRenderer.invoke('skills:readResource', skillId, relativePath), readiness: () => ipcRenderer.invoke('skills:readiness'), + installLocal: () => ipcRenderer.invoke('skills:installLocal'), + remove: (input: { skillId: string }) => ipcRenderer.invoke('skills:remove', input), }, about: { get: () => ipcRenderer.invoke('app:about'), diff --git a/apps/electron/src/renderer/finagentClient.ts b/apps/electron/src/renderer/finagentClient.ts index 1fe075e..14f3bb6 100644 --- a/apps/electron/src/renderer/finagentClient.ts +++ b/apps/electron/src/renderer/finagentClient.ts @@ -108,6 +108,8 @@ function createElectronClient(): FinagentClient { readResource: (skillId, relativePath) => ipcResult(window.electronAPI.skills.readResource(skillId, relativePath)), readiness: () => ipcResult(window.electronAPI.skills.readiness()), + installLocal: () => ipcResult(window.electronAPI.skills.installLocal()), + remove: (skillId) => ipcResult(window.electronAPI.skills.remove({ skillId })), }, about: { get: () => ipcResult(window.electronAPI.about.get()), diff --git a/bun.lock b/bun.lock index db80079..014e5fa 100644 --- a/bun.lock +++ b/bun.lock @@ -101,6 +101,9 @@ "packages/skill-hub": { "name": "@finagent/skill-hub", "version": "0.1.0", + "dependencies": { + "yaml": "^2.9.0", + }, "devDependencies": { "typescript": "^5.6.0", }, diff --git a/docs/coding-guide.md b/docs/coding-guide.md index ae6425e..155b8e2 100644 --- a/docs/coding-guide.md +++ b/docs/coding-guide.md @@ -21,9 +21,22 @@ When starting work on this project, Claude Code will automatically: ## Skill System Finance Agent uses a **Skill Hub** system for extensibility. Skills are loaded -from `SKILL.md` files (`//SKILL.md`) with frontmatter -(name/keywords) and can be enabled/disabled; the choice persists to -`skills-state.json`. Marketplace and editor features are out of scope for V1. +from `SKILL.md` packages in two ordered roots: + +1. bundled skills shipped under the runtime resources `skills/` directory; +2. user-installed skills under `~/.finagent/skills/`. + +Bundled skills are authoritative when ids collide. The Settings → Skills +screen can import a local package directory and remove user-installed packages; +bundled packages cannot be overwritten or removed. Enable/disable choices +persist to `skills-state.json`. + +Local imports require a `SKILL.md` with a lowercase, hyphenated id and non-empty +`name`/`description`. The installer rejects symbolic links, special files, +packages over 1,000 files or 50 MB, and existing ids. It copies into a staging +directory before an atomic rename, so failed imports do not leave partial +packages. Removal uses the operating system Trash. A remote community catalog +and URL installation remain future work. The current agent path runs on the persistent **Agent Kernel** in the Electron main process: diff --git a/packages/i18n/src/locales/en-US/settings.ts b/packages/i18n/src/locales/en-US/settings.ts index 92f42d5..48723a9 100644 --- a/packages/i18n/src/locales/en-US/settings.ts +++ b/packages/i18n/src/locales/en-US/settings.ts @@ -86,6 +86,9 @@ export const settings = { filterPartial: 'Partial', filterDisabled: 'Disabled', loading: 'Loading skills…', + installLocal: 'Install from folder', + installing: 'Installing…', + installSuccess: '{{name}} installed', retry: 'Retry', noneInstalled: 'No skills installed', noMatch: 'No skills match your search or filter', @@ -105,6 +108,12 @@ export const settings = { details: 'Details', version: 'Version', author: 'Author', + source: 'Source', + sourceBundled: 'Bundled', + sourceUser: 'User installed', + remove: 'Remove skill', + removing: 'Removing…', + removeDescription: 'Remove {{name}}? The package will be moved to the system Trash.', riskLevel: 'Risk level', tier: 'Tier', references: 'References', diff --git a/packages/i18n/src/locales/zh-CN/settings.ts b/packages/i18n/src/locales/zh-CN/settings.ts index 86a2994..b4e94e9 100644 --- a/packages/i18n/src/locales/zh-CN/settings.ts +++ b/packages/i18n/src/locales/zh-CN/settings.ts @@ -80,6 +80,9 @@ export const settings = { filterPartial: '部分', filterDisabled: '已禁用', loading: '正在加载技能…', + installLocal: '从文件夹安装', + installing: '正在安装…', + installSuccess: '已安装 {{name}}', retry: '重试', noneInstalled: '未安装技能', noMatch: '没有符合搜索或筛选条件的技能', @@ -99,6 +102,12 @@ export const settings = { details: '详情', version: '版本', author: '作者', + source: '来源', + sourceBundled: '内置', + sourceUser: '用户安装', + remove: '删除技能', + removing: '正在删除…', + removeDescription: '要删除 {{name}} 吗？技能包会被移入系统废纸篓。', riskLevel: '风险等级', tier: '层级', references: '参考资料', diff --git a/packages/pi-extension/src/tools/skillResources.test.ts b/packages/pi-extension/src/tools/skillResources.test.ts new file mode 100644 index 0000000..1a22a67 --- /dev/null +++ b/packages/pi-extension/src/tools/skillResources.test.ts @@ -0,0 +1,79 @@ +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { listSkillResourcesTool, readSkillResourceTool } from './skillResources.ts'; + +let root = ''; +let bundled = ''; +let user = ''; +const savedMultiple = process.env.FINAGENT_SKILLS_DIRS; +const savedSingle = process.env.FINAGENT_SKILLS_DIR; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'pi-skill-resources-')); + bundled = join(root, 'bundled'); + user = join(root, 'user'); + await Promise.all([mkdir(bundled, { recursive: true }), mkdir(user, { recursive: true })]); + process.env.FINAGENT_SKILLS_DIRS = JSON.stringify([bundled, user]); + delete process.env.FINAGENT_SKILLS_DIR; +}); + +afterEach(async () => { + if (savedMultiple === undefined) delete process.env.FINAGENT_SKILLS_DIRS; + else process.env.FINAGENT_SKILLS_DIRS = savedMultiple; + if (savedSingle === undefined) delete process.env.FINAGENT_SKILLS_DIR; + else process.env.FINAGENT_SKILLS_DIR = savedSingle; + await rm(root, { recursive: true, force: true }); +}); + +async function writeSkill(directory: string, id: string, contents: string): Promise { + const target = join(directory, id); + await mkdir(join(target, 'references'), { recursive: true }); + await writeFile(join(target, 'SKILL.md'), contents, 'utf8'); + await writeFile(join(target, 'references', 'guide.md'), `${contents} guide`, 'utf8'); +} + +describe('Pi skill resources with multiple roots', () => { + it('reads a user skill when it is absent from the bundled root', async () => { + await writeSkill(user, 'custom-skill', '# User skill'); + + const result = await readSkillResourceTool.execute( + 'call-1', + { skill: 'custom-skill', path: 'SKILL.md' }, + new AbortController().signal + ); + + expect(result.content[0]?.text).toBe('# User skill'); + }); + + it('keeps the first configured root authoritative on collisions', async () => { + await writeSkill(bundled, 'shared-skill', '# Bundled skill'); + await writeSkill(user, 'shared-skill', '# User skill'); + + const result = await readSkillResourceTool.execute( + 'call-2', + { skill: 'shared-skill', path: 'SKILL.md' }, + new AbortController().signal + ); + + expect(result.content[0]?.text).toBe('# Bundled skill'); + }); + + it('lists resources from a user skill and rejects invalid ids', async () => { + await writeSkill(user, 'custom-skill', '# User skill'); + + const result = await listSkillResourcesTool.execute( + 'call-3', + { skill: 'custom-skill' }, + new AbortController().signal + ); + + expect(result.content[0]?.text).toContain('references/guide.md'); + await expect(readSkillResourceTool.execute( + 'call-4', + { skill: '../escape', path: 'SKILL.md' }, + new AbortController().signal + )).rejects.toThrow('Invalid skill id'); + }); +}); diff --git a/packages/pi-extension/src/tools/skillResources.ts b/packages/pi-extension/src/tools/skillResources.ts index 64c2ccd..2d7b8b8 100644 --- a/packages/pi-extension/src/tools/skillResources.ts +++ b/packages/pi-extension/src/tools/skillResources.ts @@ -2,26 +2,57 @@ // // The agent receives a compact skill metadata index in its system prompt and // loads SKILL.md bodies / references on demand through these tools. Resources -// live in the directory named by FINAGENT_SKILLS_DIR (set by the Folio main -// process when spawning the runtime). +// live in the ordered directories named by FINAGENT_SKILLS_DIRS (set by the +// Folio main process when spawning the runtime). FINAGENT_SKILLS_DIR remains +// as a backward-compatible single-root fallback. import { Type } from '@sinclair/typebox'; import { readdir, readFile, realpath, stat } from 'node:fs/promises'; import { join, relative, resolve, sep } from 'node:path'; import { validateParams } from './validation.ts'; -function skillsRoot(): string | null { +function skillsRoots(): string[] { + const multiple = process.env.FINAGENT_SKILLS_DIRS; + if (multiple) { + try { + const parsed = JSON.parse(multiple) as unknown; + if (Array.isArray(parsed)) { + return Array.from(new Set( + parsed + .filter((entry): entry is string => typeof entry === 'string' && entry.length > 0) + .map((entry) => resolve(entry)) + )); + } + } catch { + // Fall through to the legacy single-root environment variable. + } + } const root = process.env.FINAGENT_SKILLS_DIR; - if (!root) return null; - return resolve(root); + return root ? [resolve(root)] : []; } -/** Resolve a skill resource path, refusing anything that escapes the root. */ -async function resolveSafePath(skillId: string, resourcePath: string): Promise { - const root = skillsRoot(); - if (!root) { +async function findSkillRoot(skillId: string): Promise { + if (!/^[a-z0-9](?:[a-z0-9-]{0,62}[a-z0-9])?$/.test(skillId)) { + throw new Error(`Invalid skill id: ${skillId}`); + } + const roots = skillsRoots(); + if (roots.length === 0) { throw new Error('Skill resources are not available in this runtime.'); } + for (const root of roots) { + try { + const candidate = await realpath(join(root, skillId)); + const candidateStat = await stat(candidate); + if (candidateStat.isDirectory()) return candidate; + } catch { + // Try the next configured root. + } + } + throw new Error(`Skill not found: ${skillId}`); +} + +/** Resolve a skill resource path, refusing anything that escapes the root. */ +async function resolveSafePath(skillId: string, resourcePath: string): Promise { if (typeof resourcePath !== 'string' || resourcePath.length === 0) { throw new Error('Resource path is required.'); } @@ -33,7 +64,7 @@ async function resolveSafePath(skillId: string, resourcePath: string): Promise candidate); const prefix = skillRoot.endsWith(sep) ? skillRoot : `${skillRoot}${sep}`; @@ -64,11 +95,7 @@ export const listSkillResourcesTool = { listSkillResourcesTool.parameters, rawParams ); - const root = skillsRoot(); - if (!root) { - throw new Error('Skill resources are not available in this runtime.'); - } - const skillRoot = await realpath(join(root, params.skill)); + const skillRoot = await findSkillRoot(params.skill); const out: string[] = []; const walk = async (directory: string) => { diff --git a/packages/shared/src/resources/index.ts b/packages/shared/src/resources/index.ts index f4a608f..c2fee16 100644 --- a/packages/shared/src/resources/index.ts +++ b/packages/shared/src/resources/index.ts @@ -2,6 +2,7 @@ export { isPackaged, getRuntimeRoot, getSkillsDir, + getUserSkillsDir, getPiExtensionEntry, getLangSmithExtensionEntry, listBundledPiExtensions, diff --git a/packages/shared/src/resources/resource-locator.test.ts b/packages/shared/src/resources/resource-locator.test.ts index 3614921..04b399b 100644 --- a/packages/shared/src/resources/resource-locator.test.ts +++ b/packages/shared/src/resources/resource-locator.test.ts @@ -1,10 +1,12 @@ import { afterEach, describe, expect, it } from 'bun:test'; +import { homedir } from 'node:os'; import { resolve } from 'node:path'; import { getPiCwd, getPiExtensionEntry, getRuntimeRoot, getSkillsDir, + getUserSkillsDir, isPackaged, } from './resource-locator.ts'; @@ -33,6 +35,11 @@ describe('ResourceLocator (dev mode)', () => { expect(getPiExtensionEntry()).toBe(resolve(getRuntimeRoot(), '.pi', 'extensions', 'finagent', 'index.ts')); }); + it('keeps user-installed skills outside bundled runtime resources', () => { + expect(getUserSkillsDir()).toBe(resolve(homedir(), '.finagent', 'skills')); + expect(getUserSkillsDir()).not.toBe(getSkillsDir()); + }); + it('does not report packaged without a real resourcesPath (non-Electron env)', () => { process.env[PACKAGED] = '1'; // No process.resourcesPath in a bare Node/Bun process -> still dev. diff --git a/packages/shared/src/resources/resource-locator.ts b/packages/shared/src/resources/resource-locator.ts index fc59f99..fcdcc4f 100644 --- a/packages/shared/src/resources/resource-locator.ts +++ b/packages/shared/src/resources/resource-locator.ts @@ -1,4 +1,5 @@ import { dirname, join, resolve } from 'node:path'; +import { homedir } from 'node:os'; import { fileURLToPath } from 'node:url'; /** @@ -53,6 +54,11 @@ export function getSkillsDir(): string { return join(getRuntimeRoot(), 'skills'); } +/** User-installed skills live outside the read-only packaged resources tree. */ +export function getUserSkillsDir(): string { + return join(homedir(), '.finagent', 'skills'); +} + /** * Entry file for the Pi extension. * diff --git a/packages/skill-hub/package.json b/packages/skill-hub/package.json index d59100a..c119218 100644 --- a/packages/skill-hub/package.json +++ b/packages/skill-hub/package.json @@ -7,7 +7,10 @@ "exports": { ".": "./src/index.ts" }, + "dependencies": { + "yaml": "^2.9.0" + }, "devDependencies": { "typescript": "^5.6.0" } -} \ No newline at end of file +} diff --git a/packages/skill-hub/src/index.ts b/packages/skill-hub/src/index.ts index c4a3f86..73fe509 100644 --- a/packages/skill-hub/src/index.ts +++ b/packages/skill-hub/src/index.ts @@ -10,18 +10,39 @@ // findSkillsByKeyword) is preserved; V2 adds metadata indexing, progressive // resource access with path safety, and keyword matching for the router. -import { mkdir, readFile, readdir, realpath, stat, writeFile } from 'node:fs/promises'; +import { + cp, + lstat, + mkdir, + mkdtemp, + readFile, + readdir, + realpath, + rename, + rm, + stat, + writeFile, +} from 'node:fs/promises'; import type { Dirent } from 'node:fs'; import { homedir } from 'node:os'; -import { join, relative, resolve, sep } from 'node:path'; +import { basename, join, relative, resolve, sep } from 'node:path'; import type { Skill } from '@finagent/core'; +import { parse as parseYaml } from 'yaml'; +export type SkillSource = 'bundled' | 'user'; + +export interface SkillDirectoryConfig { + path: string; + source: SkillSource; +} export interface SkillHubConfig { - /** Directory scanned for `/SKILL.md` skills. */ - skillsDirectory: string; + /** Legacy single directory scanned for `/SKILL.md` skills. */ + skillsDirectory?: string; + /** Ordered skill roots. Earlier roots win when ids collide. */ + skillsDirectories?: SkillDirectoryConfig[]; /** Persisted enable/disable state (JSON map of skillId → boolean). */ - stateFile: string; + stateFile?: string; } interface SkillState { @@ -34,6 +55,7 @@ export interface SkillMetadata { name: string; description: string; keywords: string[]; + source: SkillSource; license?: string; riskLevel?: string; requiresLogin?: boolean; @@ -62,25 +84,45 @@ interface ParsedSkillMarkdown { interface SkillDirectoryEntry { id: string; directory: string; + source: SkillSource; metadata: SkillMetadata; markdown: string; } +export interface SkillInstallResult { + skillId: string; + name: string; + source: 'user'; +} + +const MAX_SKILL_FILES = 1_000; +const MAX_SKILL_BYTES = 50 * 1024 * 1024; +const SKILL_ID_PATTERN = /^[a-z0-9](?:[a-z0-9-]{0,62}[a-z0-9])?$/; + export class SkillHub { private readonly entries = new Map(); private skills: Map = new Map(); - private readonly config: SkillHubConfig; + private readonly directories: SkillDirectoryConfig[]; + private readonly stateFile: string; constructor(config: Partial = {}) { - this.config = { - skillsDirectory: - config.skillsDirectory ?? join(homedir(), '.finagent', 'skills'), - stateFile: config.stateFile ?? join(homedir(), '.finagent', 'skills-state.json'), - }; + const defaultUserDirectory = join(homedir(), '.finagent', 'skills'); + this.directories = config.skillsDirectories?.map((entry) => ({ + path: resolve(entry.path), + source: entry.source, + })) ?? [{ + path: resolve(config.skillsDirectory ?? defaultUserDirectory), + source: config.skillsDirectory ? 'bundled' : 'user', + }]; + this.stateFile = config.stateFile ?? join(homedir(), '.finagent', 'skills-state.json'); } get skillsDirectory(): string { - return this.config.skillsDirectory; + return this.directories[0]?.path ?? join(homedir(), '.finagent', 'skills'); + } + + get skillsDirectories(): SkillDirectoryConfig[] { + return this.directories.map((entry) => ({ ...entry })); } /** Scan the skills directory and (re)load every `SKILL.md`. */ @@ -88,52 +130,60 @@ export class SkillHub { this.entries.clear(); this.skills.clear(); - const root = resolve(this.config.skillsDirectory); - let directories: Dirent[]; - try { - directories = await readdir(root, { withFileTypes: true }); - } catch { - return; // No skills directory yet — empty hub. - } - const state = await this.loadState(); - for (const dirent of directories) { - if (!dirent.isDirectory()) continue; - const directory = join(root, dirent.name); + for (const rootConfig of this.directories) { + let directories: Dirent[]; try { - const markdown = await readFile(join(directory, 'SKILL.md'), 'utf8'); - const parsed = parseSkillMarkdown(markdown); - const id = String(parsed.frontmatter.id ?? parsed.frontmatter.slug ?? dirent.name); - const name = String(parsed.frontmatter.name ?? dirent.name); - const description = toDescription(parsed.frontmatter); - const keywords = [ - ...splitKeywords(parsed.frontmatter.keywords), - ...extractTriggerKeywords(parsed.frontmatter.description), - ]; - const metadata = toMetadata(parsed.frontmatter); - metadata.id = id; - metadata.keywords = keywords; - if (!metadata.description) { - metadata.description = firstParagraph(parsed.body); - } - - this.entries.set(id, { id, directory, metadata, markdown }); - const enabled = state.enabled[id] ?? true; - this.skills.set(id, { - id, - name, - type: 'prompt', - trigger: { keywords }, - prompt: { system: parsed.body }, - metadata: { - enabled, - editable: false, - createdAt: 0, - updatedAt: 0, - }, - }); + directories = await readdir(rootConfig.path, { withFileTypes: true }); } catch { - // Missing/invalid SKILL.md — skip this directory. + continue; + } + for (const dirent of directories) { + if (!dirent.isDirectory()) continue; + const directory = join(rootConfig.path, dirent.name); + try { + const markdown = await readFile(join(directory, 'SKILL.md'), 'utf8'); + const parsed = parseSkillMarkdown(markdown); + const id = String(parsed.frontmatter.id ?? parsed.frontmatter.slug ?? dirent.name); + if (rootConfig.source === 'user' && !SKILL_ID_PATTERN.test(id)) continue; + if (this.entries.has(id)) continue; + const name = String(parsed.frontmatter.name ?? dirent.name); + const description = toDescription(parsed.frontmatter); + const keywords = [ + ...splitKeywords(parsed.frontmatter.keywords), + ...extractTriggerKeywords(parsed.frontmatter.description), + ]; + const metadata = toMetadata(parsed.frontmatter, rootConfig.source); + metadata.id = id; + metadata.keywords = keywords; + if (!metadata.description) { + metadata.description = firstParagraph(parsed.body); + } + + this.entries.set(id, { + id, + directory, + source: rootConfig.source, + metadata, + markdown, + }); + const enabled = state.enabled[id] ?? true; + this.skills.set(id, { + id, + name, + type: 'prompt', + trigger: { keywords }, + prompt: { system: parsed.body }, + metadata: { + enabled, + editable: false, + createdAt: 0, + updatedAt: 0, + }, + }); + } catch { + // Missing/invalid SKILL.md — skip this directory. + } } } } @@ -172,6 +222,75 @@ export class SkillHub { .map((entry) => entry.metadata); } + listAllSkillMetadata(): SkillMetadata[] { + return Array.from(this.entries.values()).map((entry) => entry.metadata); + } + + /** Validate and atomically install a local skill package into the user root. */ + async installSkillFromDirectory(sourceDirectory: string): Promise { + const userRoot = this.directories.find((entry) => entry.source === 'user')?.path; + if (!userRoot) { + throw new Error('User skill installation is not configured.'); + } + + const source = resolve(sourceDirectory); + const sourceStat = await lstat(source).catch(() => undefined); + if (!sourceStat?.isDirectory() || sourceStat.isSymbolicLink()) { + throw new Error('Selected skill package must be a real directory.'); + } + + const markdownPath = join(source, 'SKILL.md'); + const markdown = await readFile(markdownPath, 'utf8').catch(() => undefined); + if (!markdown) { + throw new Error('Selected directory does not contain SKILL.md.'); + } + const parsed = parseSkillMarkdown(markdown); + const id = String(parsed.frontmatter.id ?? parsed.frontmatter.slug ?? basename(source)); + const name = parsed.frontmatter.name; + if (!SKILL_ID_PATTERN.test(id)) { + throw new Error('Skill id must use 1-64 lowercase letters, digits, or hyphens.'); + } + if (typeof name !== 'string' || name.trim().length === 0) { + throw new Error('SKILL.md frontmatter must define a non-empty name.'); + } + if (toDescription(parsed.frontmatter).length === 0) { + throw new Error('SKILL.md frontmatter must define a non-empty description.'); + } + + await validatePackageTree(source); + await mkdir(userRoot, { recursive: true }); + const destination = join(userRoot, id); + if (this.entries.has(id) || await pathExists(destination)) { + throw new Error(`A skill with id "${id}" is already installed.`); + } + + const stagingRoot = await mkdtemp(join(userRoot, '.install-')); + const stagingSkill = join(stagingRoot, id); + try { + await cp(source, stagingSkill, { recursive: true, errorOnExist: true, force: false }); + await rename(stagingSkill, destination); + } finally { + await rm(stagingRoot, { recursive: true, force: true }); + } + + await this.loadSkills(); + const installed = this.entries.get(id); + if (!installed || installed.source !== 'user') { + await rm(destination, { recursive: true, force: true }); + await this.loadSkills(); + throw new Error('Installed skill could not be loaded.'); + } + + return { skillId: id, name: name.trim(), source: 'user' }; + } + + userSkillDirectory(skillId: string): string { + const entry = this.entries.get(skillId); + if (!entry) throw new Error(`Skill not found: ${skillId}`); + if (entry.source !== 'user') throw new Error('Bundled skills cannot be removed.'); + return entry.directory; + } + isEnabled(skillId: string): boolean { return this.skills.get(skillId)?.metadata.enabled ?? false; } @@ -315,7 +434,7 @@ export class SkillHub { private async loadState(): Promise { try { - const contents = await readFile(this.config.stateFile, 'utf8'); + const contents = await readFile(this.stateFile, 'utf8'); const parsed = JSON.parse(contents) as Partial; return { enabled: parsed.enabled ?? {} }; } catch { @@ -324,11 +443,45 @@ export class SkillHub { } private async saveState(state: SkillState): Promise { - await mkdir(resolve(this.config.stateFile, '..'), { recursive: true }); - await writeFile(this.config.stateFile, JSON.stringify(state, null, 2), 'utf8'); + await mkdir(resolve(this.stateFile, '..'), { recursive: true }); + await writeFile(this.stateFile, JSON.stringify(state, null, 2), 'utf8'); } } +async function pathExists(path: string): Promise { + return lstat(path).then(() => true, () => false); +} + +async function validatePackageTree(root: string): Promise { + let files = 0; + let bytes = 0; + const walk = async (directory: string): Promise => { + const entries = await readdir(directory, { withFileTypes: true }); + for (const entry of entries) { + const full = join(directory, entry.name); + if (entry.isSymbolicLink()) { + throw new Error(`Skill packages cannot contain symbolic links: ${relative(root, full)}`); + } + if (entry.isDirectory()) { + await walk(full); + continue; + } + if (!entry.isFile()) { + throw new Error(`Skill packages can contain only files and directories: ${relative(root, full)}`); + } + files += 1; + bytes += (await stat(full)).size; + if (files > MAX_SKILL_FILES) { + throw new Error(`Skill package exceeds the ${MAX_SKILL_FILES}-file limit.`); + } + if (bytes > MAX_SKILL_BYTES) { + throw new Error('Skill package exceeds the 50 MB size limit.'); + } + } + }; + await walk(root); +} + export const skillHub = new SkillHub(); // --------------------------------------------------------------------------- @@ -345,45 +498,9 @@ export function parseSkillMarkdown(contents: string): ParsedSkillMarkdown { } function parseFrontmatter(raw: string): Record { - const result: Record = {}; - const lines = raw.split(/\r?\n/); - for (let index = 0; index < lines.length; index += 1) { - const line = lines[index]; - const colon = line.indexOf(':'); - if (colon <= 0) continue; - const key = line.slice(0, colon).trim(); - const rawValue = line.slice(colon + 1).trim(); - if (!key || /^[#\s-]/.test(key)) continue; - - if (rawValue.length === 0 && index + 1 < lines.length && /^\s/.test(lines[index + 1] ?? '')) { - // Nested block: collect indented `key: value` lines until dedent. - const nested: Record = {}; - while (index + 1 < lines.length && /^\s/.test(lines[index + 1] ?? '')) { - index += 1; - const childLine = lines[index].trim(); - const childColon = childLine.indexOf(':'); - if (childColon <= 0) continue; - nested[childLine.slice(0, childColon).trim()] = parseScalar(childLine.slice(childColon + 1).trim()); - } - result[key] = nested; - continue; - } - result[key] = parseScalar(rawValue); - } - return result; -} - -function parseScalar(rawValue: string): unknown { - if (/^-?\d+(\.\d+)?$/.test(rawValue)) return Number(rawValue); - if (rawValue === 'true') return true; - if (rawValue === 'false') return false; - if ( - (rawValue.startsWith('"') && rawValue.endsWith('"')) || - (rawValue.startsWith("'") && rawValue.endsWith("'")) - ) { - return rawValue.slice(1, -1); - } - return rawValue; + const parsed: unknown = parseYaml(raw); + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return {}; + return parsed as Record; } function toDescription(frontmatter: Record): string { @@ -392,7 +509,7 @@ function toDescription(frontmatter: Record): string { return value.replace(/\s+/g, ' ').trim(); } -function toMetadata(frontmatter: Record): SkillMetadata { +function toMetadata(frontmatter: Record, source: SkillSource): SkillMetadata { const name = String(frontmatter.name ?? ''); const nested = (frontmatter.metadata ?? {}) as Record; return { @@ -400,6 +517,7 @@ function toMetadata(frontmatter: Record): SkillMetadata { name, description: toDescription(frontmatter), keywords: splitKeywords(frontmatter.keywords), + source, license: frontmatter.license === undefined ? undefined : String(frontmatter.license), riskLevel: nested.risk_level === undefined ? undefined : String(nested.risk_level), requiresLogin: nested.requires_login === undefined ? undefined : Boolean(nested.requires_login), diff --git a/packages/skill-hub/src/index.v2.test.ts b/packages/skill-hub/src/index.v2.test.ts index 627610f..232c6e3 100644 --- a/packages/skill-hub/src/index.v2.test.ts +++ b/packages/skill-hub/src/index.v2.test.ts @@ -23,7 +23,7 @@ async function writeFileAt(relativePath: string, contents: string) { const SKILL_MD = [ '---', 'name: market-data', - 'description: Quotes and K-lines. Triggers: "股价", "kline", "quote"', + 'description: \'Quotes and K-lines. Triggers: "股价", "kline", "quote"\'', 'license: MIT', 'metadata:', ' author: longbridge', @@ -44,6 +44,30 @@ describe('parseSkillMarkdown', () => { expect(parsed.body).toContain('# Market Data'); }); + it('parses literal and folded YAML block descriptions', () => { + const literal = parseSkillMarkdown([ + '---', + 'name: literal-description', + 'description: |', + ' First line.', + ' Triggers: "literal trigger"', + '---', + '# Literal', + ].join('\n')); + const folded = parseSkillMarkdown([ + '---', + 'name: folded-description', + 'description: >', + ' First line.', + ' Triggers: "folded trigger"', + '---', + '# Folded', + ].join('\n')); + + expect(literal.frontmatter.description).toBe('First line.\nTriggers: "literal trigger"\n'); + expect(folded.frontmatter.description).toBe('First line. Triggers: "folded trigger"\n'); + }); + it('treats files without frontmatter as body-only', () => { const parsed = parseSkillMarkdown('plain body'); expect(parsed.frontmatter).toEqual({}); @@ -115,11 +139,11 @@ describe('SkillHub V2', () => { it('matches skills by query tokens and scores relevance', async () => { await writeFileAt( 'longbridge-market-data/SKILL.md', - '---\nname: market data\ndescription: Real-time quotes and K-line charts for stocks.\n---\nbody' + "---\nname: market data\ndescription: 'Real-time quotes and K-line charts for stocks.'\n---\nbody" ); await writeFileAt( 'longbridge-technical/SKILL.md', - '---\nname: technical\ndescription: Technical analysis methodology: moving averages, RSI, MACD.\n---\nbody' + "---\nname: technical\ndescription: 'Technical analysis methodology: moving averages, RSI, MACD.'\n---\nbody" ); const hub = new SkillHub({ skillsDirectory: dir, stateFile: join(dir, 'state.json') }); await hub.loadSkills(); @@ -146,4 +170,33 @@ describe('SkillHub V2', () => { expect(metadata.license).toBe('MIT'); expect(metadata.defaultInstall).toBe(true); }); + + it('extracts trigger keywords from YAML block descriptions', async () => { + await writeFileAt('literal-description/SKILL.md', [ + '---', + 'name: Literal Description', + 'description: |', + ' Market data skill.', + ' Triggers: "股价", "quote"', + '---', + '# Literal Description', + ].join('\n')); + await writeFileAt('folded-description/SKILL.md', [ + '---', + 'name: Folded Description', + 'description: >', + ' Earnings skill.', + ' Triggers: "财报", "earnings"', + '---', + '# Folded Description', + ].join('\n')); + const hub = new SkillHub({ skillsDirectory: dir, stateFile: join(dir, 'state.json') }); + + await hub.loadSkills(); + + expect(hub.getSkill('literal-description')?.trigger.keywords).toEqual(['股价', 'quote']); + expect(hub.getSkill('folded-description')?.trigger.keywords).toEqual(['财报', 'earnings']); + expect(hub.listAllSkillMetadata().find((entry) => entry.id === 'folded-description')?.description) + .toBe('Earnings skill. Triggers: "财报", "earnings"'); + }); }); diff --git a/packages/skill-hub/src/installer.test.ts b/packages/skill-hub/src/installer.test.ts new file mode 100644 index 0000000..a4aa428 --- /dev/null +++ b/packages/skill-hub/src/installer.test.ts @@ -0,0 +1,124 @@ +import { mkdir, mkdtemp, readFile, readdir, rm, symlink, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { afterEach, beforeEach, describe, expect, it } from 'bun:test'; +import { SkillHub } from './index.ts'; + +let root = ''; +let bundled = ''; +let user = ''; +let source = ''; + +beforeEach(async () => { + root = await mkdtemp(join(tmpdir(), 'skill-installer-')); + bundled = join(root, 'bundled'); + user = join(root, 'user'); + source = join(root, 'source'); + await Promise.all([ + mkdir(bundled, { recursive: true }), + mkdir(user, { recursive: true }), + mkdir(source, { recursive: true }), + ]); +}); + +afterEach(async () => { + await rm(root, { recursive: true, force: true }); +}); + +async function writeSkill(directory: string, id: string, name = id): Promise { + const target = join(directory, id); + await mkdir(join(target, 'references'), { recursive: true }); + await writeFile(join(target, 'SKILL.md'), [ + '---', + `name: ${name}`, + `description: ${name} test skill`, + '---', + `# ${name}`, + ].join('\n'), 'utf8'); + await writeFile(join(target, 'references', 'guide.md'), '# Guide', 'utf8'); + return target; +} + +function createHub(): SkillHub { + return new SkillHub({ + skillsDirectories: [ + { path: bundled, source: 'bundled' }, + { path: user, source: 'user' }, + ], + stateFile: join(root, 'state.json'), + }); +} + +describe('SkillHub user packages', () => { + it('loads bundled and user roots while preserving bundled priority', async () => { + await writeSkill(bundled, 'shared', 'Bundled Shared'); + await writeSkill(user, 'shared', 'User Shared'); + await writeSkill(user, 'custom', 'Custom'); + const hub = createHub(); + + await hub.loadSkills(); + + expect(hub.getSkill('shared')?.name).toBe('Bundled Shared'); + expect(hub.getSkill('custom')?.name).toBe('Custom'); + expect(hub.listAllSkillMetadata().find((entry) => entry.id === 'shared')?.source).toBe('bundled'); + expect(hub.listAllSkillMetadata().find((entry) => entry.id === 'custom')?.source).toBe('user'); + }); + + it('atomically installs a valid package and reloads it', async () => { + const selected = await writeSkill(source, 'portfolio-notes', 'Portfolio Notes'); + const hub = createHub(); + await hub.loadSkills(); + + const result = await hub.installSkillFromDirectory(selected); + + expect(result).toMatchObject({ + skillId: 'portfolio-notes', + name: 'Portfolio Notes', + source: 'user', + }); + expect(await readFile(join(user, 'portfolio-notes', 'references', 'guide.md'), 'utf8')).toBe('# Guide'); + expect(hub.getSkill('portfolio-notes')?.name).toBe('Portfolio Notes'); + expect((await readdir(user)).some((entry) => entry.startsWith('.install-'))).toBe(false); + }); + + it('rejects packages that collide with a bundled skill', async () => { + await writeSkill(bundled, 'market-data', 'Market Data'); + const selected = await writeSkill(source, 'market-data', 'Replacement'); + const hub = createHub(); + await hub.loadSkills(); + + await expect(hub.installSkillFromDirectory(selected)).rejects.toThrow('already installed'); + expect(await readdir(user)).toEqual([]); + }); + + it('rejects symbolic links without leaving a partial install', async () => { + const selected = await writeSkill(source, 'unsafe-skill', 'Unsafe Skill'); + const outside = join(root, 'outside.txt'); + await writeFile(outside, 'secret', 'utf8'); + await symlink(outside, join(selected, 'references', 'leak.md')); + const hub = createHub(); + await hub.loadSkills(); + + await expect(hub.installSkillFromDirectory(selected)).rejects.toThrow('symbolic links'); + expect(await readdir(user)).toEqual([]); + }); + + it('allows removal lookup only for user-installed skills', async () => { + await writeSkill(bundled, 'built-in', 'Built In'); + await writeSkill(user, 'mine', 'Mine'); + const hub = createHub(); + await hub.loadSkills(); + + expect(hub.userSkillDirectory('mine')).toBe(join(user, 'mine')); + expect(() => hub.userSkillDirectory('built-in')).toThrow('Bundled skills cannot be removed'); + }); + + it('ignores manually placed user packages with unsafe ids', async () => { + await writeSkill(user, 'unsafe_id', 'Unsafe'); + const hub = createHub(); + + await hub.loadSkills(); + + expect(hub.getSkill('unsafe_id')).toBeUndefined(); + }); +}); diff --git a/packages/ui/src/atoms/sessionAtoms.test.ts b/packages/ui/src/atoms/sessionAtoms.test.ts index 82b77c3..cc74769 100644 --- a/packages/ui/src/atoms/sessionAtoms.test.ts +++ b/packages/ui/src/atoms/sessionAtoms.test.ts @@ -91,6 +91,8 @@ function makeClient(): FinagentClient { listResources: async () => ({ ok: false as const, error: { code: 'TEST', message: 'no-op' } }), readResource: async () => ({ ok: false as const, error: { code: 'TEST', message: 'no-op' } }), readiness: async () => ({ ok: false as const, error: { code: 'TEST', message: 'no-op' } }), + installLocal: async () => ({ ok: false as const, error: { code: 'TEST', message: 'no-op' } }), + remove: async () => ({ ok: false as const, error: { code: 'TEST', message: 'no-op' } }), }, }; } diff --git a/packages/ui/src/client.tsx b/packages/ui/src/client.tsx index 52baea9..91d33d8 100644 --- a/packages/ui/src/client.tsx +++ b/packages/ui/src/client.tsx @@ -98,6 +98,7 @@ export interface SkillListItem { version?: string; /** Parsed from SKILL.md frontmatter; absent until the main process maps it. */ author?: string; + source: 'bundled' | 'user'; } export interface SkillResourceItem { @@ -107,6 +108,13 @@ export interface SkillResourceItem { size?: number; } +export interface LocalSkillInstallResult { + canceled: boolean; + skillId?: string; + name?: string; + source?: 'user'; +} + /** Renderer-facing evaluation DTOs (spec §62-69). Never carries secrets. */ export interface EvaluationFeedbackItem { id: string; @@ -272,6 +280,8 @@ export interface FinagentClient { listResources: (skillId: string) => Promise>; readResource: (skillId: string, relativePath: string) => Promise>; readiness: () => Promise>; + installLocal: () => Promise>; + remove: (skillId: string) => Promise>; }; diagnostics?: { collect: () => Promise>; @@ -383,6 +393,8 @@ export const fallbackClient: FinagentClient = { listResources: missingClient('skills.listResources'), readResource: missingClient('skills.readResource'), readiness: missingClient('skills.readiness'), + installLocal: missingClient('skills.installLocal'), + remove: missingClient('skills.remove'), }, connections: { list: missingClient('connections.list'), diff --git a/packages/ui/src/components/settings/SkillDetailDrawer.tsx b/packages/ui/src/components/settings/SkillDetailDrawer.tsx index 35a2e1f..3142adf 100644 --- a/packages/ui/src/components/settings/SkillDetailDrawer.tsx +++ b/packages/ui/src/components/settings/SkillDetailDrawer.tsx @@ -1,6 +1,7 @@ import React, { useEffect, useRef, useState } from 'react'; import { createPortal } from 'react-dom'; import { useTranslation } from 'react-i18next'; +import { Trash2 } from 'lucide-react'; import type { SkillReadiness } from '@finagent/core'; import { useFinagentClient, type SkillListItem, type SkillResourceItem } from '../../client'; import { SkillReadinessBadge } from './SkillReadinessBadge'; @@ -14,6 +15,7 @@ export interface SkillDetailDrawerProps { togglingId: string | null; toggleError: SkillToggleError | null; onToggle: (skill: SkillListItem) => void; + onRemoved: (skillId: string) => void; } type AdvancedDoc = { status: 'loading' | 'error' | 'ready'; text?: string; error?: string }; @@ -39,6 +41,7 @@ export const SkillDetailDrawer: React.FC = ({ togglingId, toggleError, onToggle, + onRemoved, }) => { const { t } = useTranslation(); const client = useFinagentClient(); @@ -56,6 +59,8 @@ export const SkillDetailDrawer: React.FC = ({ const [advancedOpen, setAdvancedOpen] = useState(false); const [advancedDocs, setAdvancedDocs] = useState>({}); const loadedAdvancedRef = useRef(new Set()); + const [removing, setRemoving] = useState(false); + const [removeError, setRemoveError] = useState(null); // Esc closes; focus lands in the dialog on open. useEffect(() => { @@ -137,6 +142,20 @@ export const SkillDetailDrawer: React.FC = ({ if (next) void loadAdvanced(); }; + const removeSkill = async () => { + if (skill.source !== 'user') return; + if (!window.confirm(t('settings.skills.removeDescription', { name: skill.name }))) return; + setRemoving(true); + setRemoveError(null); + const result = await client.skills.remove(skill.id); + setRemoving(false); + if (!result.ok) { + setRemoveError(result.error.message); + return; + } + onRemoved(skill.id); + }; + const referenceResources = resources.filter((resource) => resource.kind === 'reference'); const drawerError = toggleError?.skillId === skill.id ? toggleError.message : null; const missingSet = new Set(readiness?.missing ?? []); @@ -265,9 +284,31 @@ export const SkillDetailDrawer: React.FC = ({
{t('settings.skills.tier')}
{skill.tier ?? '—'}
+
+
{t('settings.skills.source')}
+
+ {skill.source === 'user' ? t('settings.skills.sourceUser') : t('settings.skills.sourceBundled')} +
+
+ {skill.source === 'user' && ( +
+ + {removeError &&

{removeError}

} +
+ )} +
diff --git a/packages/ui/src/components/settings/SkillsView.test.tsx b/packages/ui/src/components/settings/SkillsView.test.tsx index 30a79ff..92f19f5 100644 --- a/packages/ui/src/components/settings/SkillsView.test.tsx +++ b/packages/ui/src/components/settings/SkillsView.test.tsx @@ -47,6 +47,7 @@ const FIXTURE: SkillListItem = { description: 'Market data via Longbridge CLI', riskLevel: 'read_only', tier: 'read', + source: 'bundled', }; function clientWith(overrides: Partial): FinagentClient { @@ -58,6 +59,8 @@ function clientWith(overrides: Partial): FinagentClien listResources: async () => ({ ok: true, data: [] }), readResource: async () => ({ ok: true, data: '# sample' }), readiness: async () => ({ ok: true, data: [] }), + installLocal: async () => ({ ok: true, data: { canceled: true } }), + remove: async () => ({ ok: true, data: undefined }), ...overrides, }, }; @@ -128,3 +131,83 @@ describe('SkillsView toggle', () => { expect(container.querySelector('[data-testid="skill-toggle-error-longbridge"]')).toBeNull(); }); }); + +describe('SkillsView local install', () => { + it('installs a selected package, reloads the list, and reports success', async () => { + let listCalls = 0; + const client = clientWith({ + list: async () => { + listCalls += 1; + return { ok: true, data: [FIXTURE] }; + }, + installLocal: async () => ({ + ok: true, + data: { canceled: false, skillId: 'custom-skill', name: 'Custom Skill', source: 'user' }, + }), + }); + const { container } = await renderSkillsView(client); + const install = container.querySelector('[data-testid="skills-install-local"]'); + expect(install).not.toBeNull(); + + await act(async () => { + install?.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + expect(listCalls).toBe(2); + expect(container.querySelector('[data-testid="skills-install-message"]')?.textContent).toContain('Custom Skill'); + }); + + it('shows installer failures without replacing the current list', async () => { + const client = clientWith({ + installLocal: async () => ({ + ok: false, + error: { code: 'SKILL_INSTALL_FAILED', message: 'SKILL.md is missing' }, + }), + }); + const { container } = await renderSkillsView(client); + + await act(async () => { + container.querySelector('[data-testid="skills-install-local"]') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + + const message = container.querySelector('[data-testid="skills-install-message"]'); + expect(message?.getAttribute('role')).toBe('alert'); + expect(message?.textContent).toContain('SKILL.md is missing'); + expect(container.querySelector('[data-testid="skill-row-longbridge"]')).not.toBeNull(); + }); + + it('removes a user skill only after confirmation and reloads the list', async () => { + let removeCalls = 0; + let listCalls = 0; + const userSkill: SkillListItem = { ...FIXTURE, id: 'custom-skill', name: 'Custom Skill', source: 'user' }; + const client = clientWith({ + list: async () => { + listCalls += 1; + return { ok: true, data: listCalls === 1 ? [userSkill] : [] }; + }, + remove: async () => { + removeCalls += 1; + return { ok: true, data: undefined }; + }, + }); + const { container } = await renderSkillsView(client); + + await act(async () => { + container.querySelector('[data-testid="skill-row-custom-skill"]') + ?.dispatchEvent(new MouseEvent('click', { bubbles: true })); + }); + let confirms = 0; + window.confirm = () => { + confirms += 1; + return true; + }; + const removeButton = document.querySelector('[data-testid="skill-remove"]'); + expect(removeButton).not.toBeNull(); + await act(async () => removeButton?.click()); + + expect(confirms).toBe(1); + expect(removeCalls).toBe(1); + expect(listCalls).toBe(2); + }); +}); diff --git a/packages/ui/src/components/settings/SkillsView.tsx b/packages/ui/src/components/settings/SkillsView.tsx index 1d44e2e..19a516e 100644 --- a/packages/ui/src/components/settings/SkillsView.tsx +++ b/packages/ui/src/components/settings/SkillsView.tsx @@ -1,6 +1,7 @@ import React, { useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { useAtom } from 'jotai'; import { useTranslation } from 'react-i18next'; +import { FolderPlus } from 'lucide-react'; import { useFinagentClient, type SkillListItem } from '../../client'; import { loadSkillReadiness, skillReadinessAtom } from '../../atoms/skillReadinessAtoms'; import { filterSkills, SKILL_STATUS_FILTERS, type SkillStatusFilter } from './skillFilters'; @@ -23,6 +24,8 @@ export const SkillsView: React.FC = () => { const [loading, setLoading] = useState(true); const [error, setError] = useState(null); const [skillReadiness, setSkillReadiness] = useAtom(skillReadinessAtom); + const [installing, setInstalling] = useState(false); + const [installMessage, setInstallMessage] = useState<{ tone: 'success' | 'error'; text: string } | null>(null); const [query, setQuery] = useState(''); const [status, setStatus] = useState('all'); @@ -70,6 +73,13 @@ export const SkillsView: React.FC = () => { triggerRef.current = null; }, []); + const handleRemoved = useCallback(async () => { + setOpenSkillId(null); + triggerRef.current = null; + await loadSkills(); + setSkillReadiness(await loadSkillReadiness()); + }, [loadSkills, setSkillReadiness]); + const handleToggle = useCallback( (skill: SkillListItem) => { void toggle(skill); @@ -77,30 +87,61 @@ export const SkillsView: React.FC = () => { [toggle] ); + const installLocalSkill = useCallback(async () => { + setInstalling(true); + setInstallMessage(null); + const result = await client.skills.installLocal(); + setInstalling(false); + if (!result.ok) { + setInstallMessage({ tone: 'error', text: result.error.message }); + return; + } + if (result.data.canceled) return; + await loadSkills(); + setSkillReadiness(await loadSkillReadiness()); + setInstallMessage({ + tone: 'success', + text: t('settings.skills.installSuccess', { name: result.data.name ?? result.data.skillId }), + }); + }, [client, loadSkills, setSkillReadiness, t]); + return (
-
-
+ + setQuery(event.target.value)} + placeholder={t('settings.skills.searchPlaceholder')} + aria-label={t('settings.skills.searchAria')} + data-testid="skills-search" + className="h-9 w-full rounded-[8px] border border-input bg-background pl-8 pr-3 text-[13px] text-foreground placeholder:text-foreground/40 transition-smooth hover:border-border-strong focus:border-accent focus:outline-none focus:ring-2 focus:ring-ring" + /> +
+
@@ -124,6 +165,15 @@ export const SkillsView: React.FC = () => { ); })}
+ {installMessage && ( +
+ {installMessage.text} +
+ )}
@@ -169,6 +219,7 @@ export const SkillsView: React.FC = () => { togglingId={togglingId} toggleError={toggleError} onToggle={handleToggle} + onRemoved={() => void handleRemoved()} /> )}
diff --git a/packages/ui/src/components/settings/skillFilters.test.ts b/packages/ui/src/components/settings/skillFilters.test.ts index 3f886ed..8b9a290 100644 --- a/packages/ui/src/components/settings/skillFilters.test.ts +++ b/packages/ui/src/components/settings/skillFilters.test.ts @@ -10,6 +10,7 @@ function skill(overrides: Partial = {}): SkillListItem { keywords: ['quote', 'kline'], enabled: true, description: 'Market data via Longbridge CLI', + source: 'bundled', ...overrides, }; }