From faba95f65d1156efa17b5b3535b535fe370ff292 Mon Sep 17 00:00:00 2001 From: rrader26 Date: Fri, 15 May 2026 04:26:08 -0400 Subject: [PATCH] feat(install): --skill= installs a teaching skill per AI client MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The third tier of the memory bridge slice. PR #44 made the memory plugin auto-select the SaaS backend. PR #45 lets the install CLI write the env block into client configs. This PR adds the skill file that teaches the agent WHEN and HOW to use the memory tools — without it, an agent has the tools but rarely calls them at the right moments. What's a "skill" here: Markdown content (with optional YAML frontmatter) that an AI client loads automatically at session start. Each major coding assistant has a slot for it: - Claude Code: ~/.claude/skills//skill.md - Claude Desktop: ~/Library/Application Support/Claude/skills/... For tools without a native skills concept (Cursor, Windsurf, Codex CLI), the same content gets wrapped in marker comments and surgically injected into their rules file. That codepath is scaffolded here (upsertManagedBlock + non-exclusive targets) but no rules-file clients are in the default target list yet — adding them needs per-tool research on the rules-file location semantics. Canonical skill — thinkfleet-memory: - Tells the agent to call agentmark_memory_search at session start to load project context. - Tells it to save user preferences, project facts, decisions without being asked. - Tells it to search memory before guessing about the user's environment. - Includes a scope picker, failure modes, and an explicit "what NOT to do" section (don't dump every memory at the user, don't save secrets, don't overwrite user-scope with session-scope writes). - Inlined as a TypeScript constant (~5KB) so the skill ships with the npm package without separate asset bundling. CLI surface: agentmark-mcp install \\ --client=claude-code \\ --env=THINKFLEET_BASE_URL=... \\ --env=THINKFLEET_PROJECT_ID=... \\ --env=THINKFLEET_API_KEY=... \\ --skill=thinkfleet-memory Validation: - Skill name must match [a-z0-9][a-z0-9-]* — rejects path- traversal attempts (../escape) and shell-relevant chars. - Unknown skill names log to stderr and the install still returns a non-zero exit so CI catches typos. - Atomic write via temp-file + rename, 0644 perms. - Idempotent: same content → "already_present"; different content → "updated"; non-existent → "added". Marker-block logic (pure function, fully tested): upsertManagedBlock(text, block) — replaces an existing ... block in place, or appends one if absent. Preserves user content outside the markers verbatim. Tests: - test/mcp/install-skills.test.ts — 16 cases covering: - upsertManagedBlock: append-empty, append-with-trailing-NL, replace-existing, idempotent, preserve-user-content. - installSkill end-to-end with fake targets: native (whole file), rules-file (marker block), already_present idempotency, dryRun, unsupported-platform skip. - Built-in skill catalog: thinkfleet-memory is registered, unknown names return null, every memory tool name appears in the skill content. - test/mcp/install-flags.test.ts — 6 new cases for --skill name validation. 596 tests pass. Typecheck clean. No new dependencies. Co-Authored-By: Claude Opus 4.7 (1M context) --- src/mcp/cli.ts | 55 ++++++- src/mcp/install/flags.ts | 20 ++- src/mcp/install/skills.ts | 225 ++++++++++++++++++++++++++++ src/mcp/skills/thinkfleet-memory.ts | 146 ++++++++++++++++++ test/mcp/install-flags.test.ts | 29 ++++ test/mcp/install-skills.test.ts | 222 +++++++++++++++++++++++++++ 6 files changed, 693 insertions(+), 4 deletions(-) create mode 100644 src/mcp/install/skills.ts create mode 100644 src/mcp/skills/thinkfleet-memory.ts create mode 100644 test/mcp/install-skills.test.ts diff --git a/src/mcp/cli.ts b/src/mcp/cli.ts index e8e2e4d..0e64ec3 100644 --- a/src/mcp/cli.ts +++ b/src/mcp/cli.ts @@ -27,6 +27,8 @@ import { buildEntryFromFlags, type ParsedFlags, } from './install/flags' +import { installSkill, type SkillInstallResult } from './install/skills' +import { getSkillContent } from './skills/thinkfleet-memory' const HELP = `agentmark-mcp — Model Context Protocol server for AgentMark @@ -61,13 +63,18 @@ OPTIONS (install / setup / uninstall) MCP config file on disk. Prefer rotating secrets from an OS keychain (ThinkFleet Desktop does this) rather than passing long-lived keys on a shared machine. + --skill= Also install a skill that teaches the agent when / + how to use the tools you just wired. Repeatable. + Known: thinkfleet-memory. Native-skill clients get a + skill.md file; rules-file clients get a marker block. --dry-run Show what would change without writing -EXAMPLE — wire Claude Code to ThinkFleet memory: +EXAMPLE — wire Claude Code to ThinkFleet memory + install skill: agentmark-mcp install --client=claude-code \\ --env=THINKFLEET_BASE_URL=https://app.thinkfleet.ai \\ --env=THINKFLEET_PROJECT_ID=proj_xxx \\ - --env=THINKFLEET_API_KEY=sk-xxx + --env=THINKFLEET_API_KEY=sk-xxx \\ + --skill=thinkfleet-memory ` async function main(argv: string[]): Promise { @@ -108,7 +115,34 @@ async function runInstall(args: string[]): Promise { dryRun: flags.dryRun, }) printInstallResult(result, flags.dryRun ? 'dry-run' : 'install') - return result.clients.every((r) => r.action !== 'error') ? 0 : 1 + + // After the MCP entry is wired, optionally install skill files + // that teach the agent when/how to use those tools. Opt-in via + // `--skill=` so callers who only want the MCP wiring + // (and not opinions injected into their agent prompts) can + // still install just the server entry. + let skillsOk = true + if (flags.skill && flags.skill.length > 0) { + for (const name of flags.skill) { + const skill = getSkillContent(name) + if (!skill) { + process.stderr.write(`Unknown skill: ${name}. Skipping.\n`) + skillsOk = false + continue + } + const skillResult = await installSkill({ + skillName: name, + content: skill.content, + clientIds: flags.client, + dryRun: flags.dryRun, + }) + printSkillResult(name, skillResult, flags.dryRun ? 'dry-run' : 'install') + if (!skillResult.ok) skillsOk = false + } + } + + const installOk = result.clients.every((r) => r.action !== 'error') + return installOk && skillsOk ? 0 : 1 } async function runUninstall(args: string[]): Promise { @@ -183,6 +217,21 @@ function pkgVersion(): string { } } +function printSkillResult(skillName: string, result: SkillInstallResult, label: string): void { + process.stdout.write(`agentmark-mcp ${label} — skill "${skillName}":\n\n`) + for (const c of result.clients) { + const flag = + c.action === 'added' || c.action === 'updated' ? '✓' + : c.action === 'already_present' ? '·' + : c.action === 'skipped' ? '⏭' + : '✗' + const padded = `${c.clientName} [${c.action}]`.padEnd(40) + process.stdout.write(` ${flag} ${padded} ${c.path}\n`) + if (c.message) process.stdout.write(` ${c.message}\n`) + } + process.stdout.write('\n') +} + function printInstallResult(result: { clients: Array<{ id: string; name: string; path: string; action: string; message?: string }> }, label: string): void { process.stdout.write(`agentmark-mcp ${label} — results:\n\n`) for (const c of result.clients) { diff --git a/src/mcp/install/flags.ts b/src/mcp/install/flags.ts index 8e0ed7f..75370e0 100644 --- a/src/mcp/install/flags.ts +++ b/src/mcp/install/flags.ts @@ -21,6 +21,8 @@ export interface ParsedFlags { /** When supplied, the resolved env map. Empty object means * `--env` was used but every value parsed empty (still valid). */ env: Record | undefined + /** Skill names requested via `--skill=`. Deduplicated. */ + skill: string[] | undefined dryRun: boolean } @@ -33,18 +35,23 @@ export interface ParsedFlags { const ENV_KEY_PATTERN = /^[A-Za-z_][A-Za-z0-9_]*$/ const ENV_VALUE_MAX_LEN = 4 * 1024 +/** Skill names — same shape as env-var keys, restricted to a sane + * set so a malformed value can't be interpreted as a path on disk. */ +const SKILL_NAME_PATTERN = /^[a-z0-9][a-z0-9-]*$/ + export function parseFlags(args: string[], emit?: (line: string) => void): ParsedFlags { const client: string[] = [] const name: string[] = [] const command: string[] = [] const env: Record = {} + const skills = new Set() let envSeen = false let dryRun = false const warn = emit ?? ((line: string) => process.stderr.write(`${line}\n`)) for (const arg of args) { if (arg === '--dry-run' || arg === '-n') { dryRun = true; continue } - const m = arg.match(/^--(client|name|command|env)(?:=(.*))?$/) + const m = arg.match(/^--(client|name|command|env|skill)(?:=(.*))?$/) if (!m) continue const value = m[2] if (value === undefined) continue @@ -59,6 +66,16 @@ export function parseFlags(args: string[], emit?: (line: string) => void): Parse env[key] = envValue envSeen = true } + if (m[1] === 'skill') { + const skillName = value.trim() + if (!SKILL_NAME_PATTERN.test(skillName)) { + throw new Error( + `--skill name "${skillName}" is invalid. Must match ` + + '[a-z0-9][a-z0-9-]* (lowercase letters / digits / hyphens).', + ) + } + skills.add(skillName) + } } return { @@ -66,6 +83,7 @@ export function parseFlags(args: string[], emit?: (line: string) => void): Parse name: name.length > 0 ? name : undefined, command: command.length > 0 ? command : undefined, env: envSeen ? env : undefined, + skill: skills.size > 0 ? Array.from(skills) : undefined, dryRun, } } diff --git a/src/mcp/install/skills.ts b/src/mcp/install/skills.ts new file mode 100644 index 0000000..1e94fb0 --- /dev/null +++ b/src/mcp/install/skills.ts @@ -0,0 +1,225 @@ +/** + * Per-AI-tool skill installer. + * + * Some clients (Claude Code, Claude Desktop) have a native "skills" + * concept — a directory of `skill.md` files the agent loads at + * session start. We write the canonical skill content directly to + * the known path. + * + * Other clients (Cursor, Windsurf, Codex CLI) don't have skills, + * but they DO load a rules / instructions file automatically every + * session. We render the skill content as a marker-wrapped block + * inside that file so re-running the installer surgically replaces + * just our section without disturbing the user's hand-written + * rules. + * + * Why this lives alongside the MCP-config installer: + * The skill is useless without the corresponding MCP tools, so + * the natural install moment is the same. The CLI's + * `--skill=` flag opts in per-skill so users who only want + * the MCP wiring (no opinions injected into their agent) can + * still install just the server entry. + */ +import { mkdir, readFile, rename, writeFile } from 'node:fs/promises' +import * as os from 'node:os' +import * as path from 'node:path' + +export const MANAGED_BLOCK_START = '' +export const MANAGED_BLOCK_END = '' + +export interface SkillTarget { + /** Client id matching `ClientDescriptor.id`. */ + clientId: string + /** Human-readable client name (for log output). */ + clientName: string + /** Absolute path the renderer writes to. Returns null when the + * client doesn't ship on this OS. */ + pathFor(skillName: string): string | null + /** When `true`, the file at the path is owned exclusively by the + * skill — we write the whole file. When `false`, the skill is + * one marker-delimited block inside a larger file the user + * also edits — we replace only the block. */ + exclusive: boolean +} + +export interface SkillInstallResult { + clients: Array<{ + clientId: string + clientName: string + skillName: string + path: string + action: 'added' | 'updated' | 'already_present' | 'skipped' | 'error' + message?: string + }> + ok: boolean +} + +export interface SkillInstallOptions { + skillName: string + /** Markdown body of the skill (frontmatter + content). */ + content: string + /** When set, restrict to these client ids. */ + clientIds?: string[] + /** Skill targets to use. Defaults to {@link DEFAULT_SKILL_TARGETS}. */ + targets?: SkillTarget[] + /** Preview only — log paths + actions, don't write. */ + dryRun?: boolean +} + +/** Native-skill clients (write the whole file). */ +function claudeCodeSkillTarget(): SkillTarget { + return { + clientId: 'claude-code', + clientName: 'Claude Code', + pathFor: (name) => path.join(os.homedir(), '.claude', 'skills', name, 'skill.md'), + exclusive: true, + } +} + +function claudeDesktopSkillTarget(): SkillTarget { + return { + clientId: 'claude-desktop', + clientName: 'Claude Desktop', + pathFor: (name) => { + if (process.platform === 'darwin') { + return path.join(os.homedir(), 'Library', 'Application Support', 'Claude', 'skills', name, 'skill.md') + } + if (process.platform === 'win32') { + const appData = process.env.APPDATA ?? path.join(os.homedir(), 'AppData', 'Roaming') + return path.join(appData, 'Claude', 'skills', name, 'skill.md') + } + // Linux Claude Desktop isn't shipped today. + return null + }, + exclusive: true, + } +} + +/** + * The defaults the install CLI uses when the caller doesn't supply + * a custom list. Today: just the two Claude-family tools with + * native skill support. Cursor / Windsurf / Codex rules-file + * injection lands in a follow-up — they need a separate code path + * for the marker-block replacement. + */ +export const DEFAULT_SKILL_TARGETS: SkillTarget[] = [ + claudeCodeSkillTarget(), + claudeDesktopSkillTarget(), +] + +export async function installSkill(options: SkillInstallOptions): Promise { + const targets = options.targets ?? DEFAULT_SKILL_TARGETS + const filtered = options.clientIds + ? targets.filter((t) => options.clientIds!.includes(t.clientId)) + : targets + + const out: SkillInstallResult['clients'] = [] + + for (const target of filtered) { + const dest = target.pathFor(options.skillName) + if (!dest) { + out.push({ + clientId: target.clientId, + clientName: target.clientName, + skillName: options.skillName, + path: '', + action: 'skipped', + message: `Not supported on platform ${process.platform}.`, + }) + continue + } + + try { + const action = await writeSkillForTarget(target, dest, options.content, options.dryRun === true) + out.push({ + clientId: target.clientId, + clientName: target.clientName, + skillName: options.skillName, + path: dest, + action, + message: options.dryRun ? '(dry run; nothing written)' : undefined, + }) + } + catch (err) { + out.push({ + clientId: target.clientId, + clientName: target.clientName, + skillName: options.skillName, + path: dest, + action: 'error', + message: (err as Error).message, + }) + } + } + + return { clients: out, ok: out.every((r) => r.action !== 'error') } +} + +async function writeSkillForTarget( + target: SkillTarget, + dest: string, + content: string, + dryRun: boolean, +): Promise<'added' | 'updated' | 'already_present'> { + if (target.exclusive) { + // Whole-file write: read existing, decide action, replace. + const existing = await safeReadFile(dest) + if (existing !== null && existing === content) return 'already_present' + if (!dryRun) { + await mkdir(path.dirname(dest), { recursive: true }) + await atomicWriteFile(dest, content) + } + return existing === null ? 'added' : 'updated' + } + + // Marker-block write: replace just our section in a larger file. + const existing = (await safeReadFile(dest)) ?? '' + const block = `${MANAGED_BLOCK_START}\n${content}\n${MANAGED_BLOCK_END}\n` + const next = upsertManagedBlock(existing, block) + if (next === existing) return 'already_present' + if (!dryRun) { + await mkdir(path.dirname(dest), { recursive: true }) + await atomicWriteFile(dest, next) + } + return existing.includes(MANAGED_BLOCK_START) ? 'updated' : 'added' +} + +async function safeReadFile(p: string): Promise { + try { + return await readFile(p, 'utf8') + } + catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return null + throw err + } +} + +/** + * Atomic write — temp file in the same dir + rename. Sets `0644` + * on POSIX so the skill file is readable by the AI client but not + * writable by other users. + */ +async function atomicWriteFile(dest: string, content: string): Promise { + const tmp = `${dest}.tmp.${process.pid}` + await writeFile(tmp, content, { encoding: 'utf8', mode: 0o644 }) + await rename(tmp, dest) +} + +/** + * Replace the existing managed block in `text`, or append a new + * block at the end. Pure function — easy to unit-test. + */ +export function upsertManagedBlock(text: string, block: string): string { + const startIdx = text.indexOf(MANAGED_BLOCK_START) + const endIdx = text.indexOf(MANAGED_BLOCK_END) + if (startIdx !== -1 && endIdx !== -1 && endIdx > startIdx) { + // Include trailing newline if present + const trailing = text.charAt(endIdx + MANAGED_BLOCK_END.length) === '\n' ? 1 : 0 + const before = text.slice(0, startIdx) + const after = text.slice(endIdx + MANAGED_BLOCK_END.length + trailing) + return `${before}${block}${after}` + } + // Append (with separating newline if existing text doesn't end in one). + const sep = text.length === 0 ? '' : text.endsWith('\n') ? '\n' : '\n\n' + return `${text}${sep}${block}` +} diff --git a/src/mcp/skills/thinkfleet-memory.ts b/src/mcp/skills/thinkfleet-memory.ts new file mode 100644 index 0000000..c604fd1 --- /dev/null +++ b/src/mcp/skills/thinkfleet-memory.ts @@ -0,0 +1,146 @@ +/** + * The canonical ThinkFleet Memory skill — instruction packet that + * teaches AI tools (Claude Code, Cursor, Codex, Copilot, Windsurf) + * when and how to use the `agentmark_memory_*` MCP tools. + * + * Inlined as a string constant on purpose: + * - Skill content travels with the @thinkfleet/agentmark npm + * package — no separate asset to bundle, no runtime path- + * resolution to debug across `npx`, global install, and + * ThinkFleet Desktop's bundled-layout deployments. + * - Easy to diff in code review when we evolve the skill. + * + * Versioning: + * `version: 1` in the frontmatter is a content version (not a + * semver). The skill installer treats different versions as + * different content and rewrites the on-disk skill file when + * the version bumps. Add a CHANGELOG note when bumping. + */ + +export const THINKFLEET_MEMORY_SKILL_NAME = 'thinkfleet-memory' + +export const THINKFLEET_MEMORY_SKILL_VERSION = 1 + +/** + * Markdown body of the skill with YAML frontmatter. Claude-family + * tools read the frontmatter to populate the skill catalog UI; the + * markdown body is the prompt the agent reads at session start. + * + * Tools that don't have a native "skills" concept (Cursor, Windsurf, + * Codex CLI) get the same content rendered as a managed block inside + * their rules file — see `src/mcp/install/skills.ts`. + */ +export const THINKFLEET_MEMORY_SKILL = `--- +name: thinkfleet-memory +version: ${THINKFLEET_MEMORY_SKILL_VERSION} +description: Hierarchical persistent memory across every AI session, project, and tool. +triggers: + - At session start, load context for the current project. + - When the user shares a preference, decision, or fact — save it. + - Before asking the user a question whose answer might already be remembered, search memory first. + - When the user says "remember that…", "from now on…", or "going forward…" — save verbatim to the implied scope. +tools: + - agentmark_memory_get + - agentmark_memory_set + - agentmark_memory_search + - agentmark_memory_list + - agentmark_memory_delete +--- + +# Using ThinkFleet Memory + +You have access to a hierarchical persistent memory system that survives +across all sessions, in every project, on every machine the user works on. +Memory is organized into five scopes; pick the right one when saving. + +## Mandatory: at session start + +The very first thing you do in a new conversation is call: + +\`\`\` +agentmark_memory_search({ query: "", scope: { type: "project", id: "" }, limit: 20 }) +\`\`\` + +…then for facts about the human: + +\`\`\` +agentmark_memory_list({ scope: { type: "user", id: "" }, limit: 50 }) +\`\`\` + +…and for things they want you to always know: + +\`\`\` +agentmark_memory_list({ scope: { type: "platform" }, limit: 50 }) +\`\`\` + +Read what comes back. **Do not** re-ask the user about anything covered +there. If they tell you something contradicting a memory, save the new +fact (it supersedes the old by recency). + +## During the session, save without being asked + +Whenever the user shares one of these, call \`agentmark_memory_set\` immediately: + +- **Preferences**: "I prefer pnpm" → \`set('prefs.package-manager', 'pnpm', scope: 'user')\` +- **Facts about you**: "I'm a senior eng at X" → \`set('user.role', 'senior eng', scope: 'user')\` +- **Project conventions**: "this repo uses ESM" → \`set('repo.module-system', 'esm', scope: 'project')\` +- **Decisions made**: "we chose lance for vectors" → \`set('decision.vector-store', 'lance', scope: 'project')\` +- **Build / test commands**: "tests run with pnpm test" → \`set('repo.test-command', 'pnpm test', scope: 'project')\` +- **"Remember that…" / "From now on…"**: save the exact statement, scope = whatever it sounds like (user-level if it's about them; project-level if scoped to this repo). + +You don't have to ask permission first. Saving is cheap and reversible. + +## Before guessing about the user's environment + +If you'd otherwise ask the user *"what shell do you use?"*, *"what's your editor?"*, +*"what's our deploy command?"* — search memory first: + +\`\`\` +agentmark_memory_search({ query: "shell editor deploy", scope: { type: "user", id: "..." } }) +agentmark_memory_search({ query: "shell editor deploy", scope: { type: "project", id: "..." } }) +\`\`\` + +If it's there, use it. If not, ask once and then save the answer. + +## Scope picker + +| Scope | When to use | \`id\` value | +|---|---|---| +| **platform** | Things that should be true forever, across every project. Brand, voice, top-level user identity. | (none — pass scope as \`{ type: "platform" }\`) | +| **user** | Personal preferences, role, identity. Spans every project. | The user's stable id (email or display name). | +| **project** | Conventions of THIS repo. Tech stack, commands, decisions. | The absolute path to the repo root. | +| **agent** | Rarely needed. Use when memory is specific to one AI tool (e.g. only Claude Code uses this). | The agent / tool name. | +| **session** | Just this conversation. Use sparingly — most useful info should outlive the chat. | The MCP session id. | + +When in doubt, prefer **user** over **session**, **project** over **user**. +A memory in the wrong scope is recoverable; a memory you didn't save is gone. + +## Failure modes (read these once) + +- **\`memory_set\` returns \`{ saved: true, record: null }\`** — the SaaS backend rejected the value. Don't retry; tell the user "memory save failed" and proceed. +- **\`memory_get\` for a key returns \`null\`** — not an error; nothing was saved under that key in the queried scope(s). +- **All memory tools throw** — the MCP server lost its backend connection. Continue the session; memory will recover on the next round-trip. Don't keep asking the user "is your memory working?" + +## What you must NOT do + +- **Don't dump every memory at the user.** Read silently; act on what you find. Surfacing every recall is noise. +- **Don't save secrets.** Tokens, passwords, private keys → never \`memory_set\`. If the user pastes one, treat it as ephemeral. +- **Don't overwrite the user.** If a memory is \`prefs.editor=vscode\` and the user says "I'm in nvim today," save a *new* memory with a session scope rather than overwriting the user-scope value. +` + +/** + * Returns a lookup table of all skills shipped with this package. + * Today there's only one — ThinkFleet Memory — but the shape is set + * up so future skills (recipes, lattice observe, industry packs) + * can register without touching the installer. + */ +export const BUILT_IN_SKILLS: Record = { + [THINKFLEET_MEMORY_SKILL_NAME]: { + version: THINKFLEET_MEMORY_SKILL_VERSION, + content: THINKFLEET_MEMORY_SKILL, + }, +} + +export function getSkillContent(name: string): { version: number; content: string } | null { + return BUILT_IN_SKILLS[name] ?? null +} diff --git a/test/mcp/install-flags.test.ts b/test/mcp/install-flags.test.ts index bf09ecf..3283541 100644 --- a/test/mcp/install-flags.test.ts +++ b/test/mcp/install-flags.test.ts @@ -136,6 +136,35 @@ describe('parseFlags — --env', () => { }) }) +describe('parseFlags — --skill', () => { + it('returns skill undefined when no --skill flag is passed', () => { + expect(parseFlags(['--client=cursor']).skill).toBeUndefined() + }) + + it('captures a single --skill name', () => { + expect(parseFlags(['--skill=thinkfleet-memory']).skill).toEqual(['thinkfleet-memory']) + }) + + it('deduplicates repeated --skill flags', () => { + const flags = parseFlags(['--skill=thinkfleet-memory', '--skill=thinkfleet-memory']) + expect(flags.skill).toEqual(['thinkfleet-memory']) + }) + + it('captures multiple distinct skills in order', () => { + expect(parseFlags(['--skill=alpha', '--skill=beta']).skill).toEqual(['alpha', 'beta']) + }) + + it('rejects uppercase / special characters in skill name', () => { + expect(() => parseFlags(['--skill=Bad_Name'])).toThrowError(/is invalid/) + expect(() => parseFlags(['--skill=name with space'])).toThrowError(/is invalid/) + expect(() => parseFlags(['--skill=../escape'])).toThrowError(/is invalid/) + }) + + it('rejects skill name starting with a hyphen', () => { + expect(() => parseFlags(['--skill=-leading-hyphen'])).toThrowError(/is invalid/) + }) +}) + describe('buildEntryFromFlags', () => { it('uses the default command when --command is absent', () => { const flags = parseFlags([]) diff --git a/test/mcp/install-skills.test.ts b/test/mcp/install-skills.test.ts new file mode 100644 index 0000000..96ad8bf --- /dev/null +++ b/test/mcp/install-skills.test.ts @@ -0,0 +1,222 @@ +/** + * Tests for the per-AI-tool skill installer. + * + * Two coverage axes: + * 1. `upsertManagedBlock` — pure marker-block replacement logic + * for rules-file clients (Cursor / Windsurf / Codex CLI in the + * future). Test thoroughly here so the file-mutation paths are + * trusted before we wire them up. + * 2. `installSkill` end-to-end with a fake SkillTarget pointing at + * a tmp file — proves the write happens, idempotency holds, and + * dry-run is honored. + */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' +import * as os from 'node:os' +import * as path from 'node:path' +import { + installSkill, + upsertManagedBlock, + MANAGED_BLOCK_START, + MANAGED_BLOCK_END, + type SkillTarget, +} from '../../src/mcp/install/skills' +import { + THINKFLEET_MEMORY_SKILL, + THINKFLEET_MEMORY_SKILL_NAME, + getSkillContent, +} from '../../src/mcp/skills/thinkfleet-memory' + +let tmp: string + +beforeEach(async () => { + tmp = await mkdtemp(path.join(os.tmpdir(), 'agentmark-skill-')) +}) + +afterEach(async () => { + await rm(tmp, { recursive: true, force: true }) +}) + +function fakeNativeTarget(id: string, fileName: string): SkillTarget { + const dest = path.join(tmp, id, fileName) + return { + clientId: id, + clientName: id, + pathFor: () => dest, + exclusive: true, + } +} + +function fakeRulesTarget(id: string, fileName: string): SkillTarget { + return { + clientId: id, + clientName: id, + pathFor: () => path.join(tmp, fileName), + exclusive: false, + } +} + +describe('upsertManagedBlock', () => { + const block = `${MANAGED_BLOCK_START}\nhello\n${MANAGED_BLOCK_END}\n` + + it('appends a block to empty text', () => { + expect(upsertManagedBlock('', block)).toBe(block) + }) + + it('appends a block to text without a trailing newline', () => { + const result = upsertManagedBlock('user notes', block) + expect(result.startsWith('user notes\n\n')).toBe(true) + expect(result.endsWith(block)).toBe(true) + }) + + it('separates the appended block with a blank line for readability', () => { + // Both "no trailing newline" and "single trailing newline" + // produce a blank line between user content and the managed + // block — markdown looks cleaner with the visual break. + const result = upsertManagedBlock('user notes\n', block) + expect(result).toBe(`user notes\n\n${block}`) + }) + + it('replaces an existing managed block in place', () => { + const original = `prefix\n${MANAGED_BLOCK_START}\nold\n${MANAGED_BLOCK_END}\nsuffix\n` + const newBlock = `${MANAGED_BLOCK_START}\nnew\n${MANAGED_BLOCK_END}\n` + expect(upsertManagedBlock(original, newBlock)).toBe(`prefix\n${newBlock}suffix\n`) + }) + + it('is idempotent — same block produces same output', () => { + const original = `prefix\n${block}suffix\n` + expect(upsertManagedBlock(original, block)).toBe(original) + }) + + it('does NOT touch text outside the managed block', () => { + const userContent = '# my rules\n- do not use comments\n- prefer terse\n' + const result = upsertManagedBlock(userContent, block) + expect(result).toContain('do not use comments') + expect(result).toContain('prefer terse') + expect(result).toContain(MANAGED_BLOCK_START) + }) +}) + +describe('installSkill — native-skill targets (exclusive)', () => { + it('writes the skill content to the target path on first run', async () => { + const target = fakeNativeTarget('test-native', 'skill.md') + const result = await installSkill({ + skillName: 'demo', + content: 'hello world\n', + targets: [target], + }) + expect(result.ok).toBe(true) + expect(result.clients[0].action).toBe('added') + + const dest = target.pathFor('demo')! + expect(await readFile(dest, 'utf8')).toBe('hello world\n') + }) + + it('reports already_present on a second run with unchanged content', async () => { + const target = fakeNativeTarget('test-native-idem', 'skill.md') + const opts = { skillName: 'demo', content: 'same content\n', targets: [target] } + await installSkill(opts) + const second = await installSkill(opts) + expect(second.clients[0].action).toBe('already_present') + }) + + it('reports updated when content changes', async () => { + const target = fakeNativeTarget('test-native-up', 'skill.md') + await installSkill({ skillName: 'demo', content: 'v1\n', targets: [target] }) + const second = await installSkill({ skillName: 'demo', content: 'v2\n', targets: [target] }) + expect(second.clients[0].action).toBe('updated') + expect(await readFile(target.pathFor('demo')!, 'utf8')).toBe('v2\n') + }) + + it('honors dryRun — does not write', async () => { + const target = fakeNativeTarget('test-native-dry', 'skill.md') + const result = await installSkill({ + skillName: 'demo', + content: 'should not land\n', + targets: [target], + dryRun: true, + }) + expect(result.clients[0].action).toBe('added') + await expect(readFile(target.pathFor('demo')!, 'utf8')).rejects.toThrow() + }) + + it('skips when the target returns null path (unsupported platform)', async () => { + const target: SkillTarget = { + clientId: 'unsupported', + clientName: 'unsupported', + pathFor: () => null, + exclusive: true, + } + const result = await installSkill({ + skillName: 'demo', + content: 'whatever\n', + targets: [target], + }) + expect(result.clients[0].action).toBe('skipped') + }) +}) + +describe('installSkill — rules-file targets (non-exclusive)', () => { + it('appends a marker-wrapped block to an existing rules file', async () => { + const target = fakeRulesTarget('cursor-fake', '.cursorrules') + const dest = target.pathFor('demo')! + await writeFile(dest, '# user rules\n- terse\n', 'utf8') + + const result = await installSkill({ + skillName: 'demo', + content: 'agent prompt content\n', + targets: [target], + }) + expect(result.clients[0].action).toBe('added') + + const written = await readFile(dest, 'utf8') + expect(written).toContain('# user rules') + expect(written).toContain('- terse') + expect(written).toContain(MANAGED_BLOCK_START) + expect(written).toContain('agent prompt content') + expect(written).toContain(MANAGED_BLOCK_END) + }) + + it('replaces just the marker block on update — user rules untouched', async () => { + const target = fakeRulesTarget('cursor-fake-2', '.cursorrules') + const dest = target.pathFor('demo')! + await writeFile(dest, '# user rules\n- terse\n', 'utf8') + + await installSkill({ skillName: 'demo', content: 'v1 content\n', targets: [target] }) + await installSkill({ skillName: 'demo', content: 'v2 content\n', targets: [target] }) + + const written = await readFile(dest, 'utf8') + expect(written).toContain('- terse') // user content preserved + expect(written).toContain('v2 content') // new block in + expect(written).not.toContain('v1 content') // old block out + }) +}) + +describe('built-in skill catalog', () => { + it('exports the thinkfleet-memory skill with frontmatter', () => { + const skill = getSkillContent(THINKFLEET_MEMORY_SKILL_NAME) + expect(skill).not.toBeNull() + expect(skill!.content).toBe(THINKFLEET_MEMORY_SKILL) + // Must look like a skill (YAML frontmatter + markdown body). + expect(skill!.content.startsWith('---\n')).toBe(true) + expect(skill!.content).toContain('name: thinkfleet-memory') + expect(skill!.content).toContain('# Using ThinkFleet Memory') + }) + + it('returns null for unknown skill names', () => { + expect(getSkillContent('does-not-exist')).toBeNull() + }) + + it('skill catalog lists every tool the AI is expected to call', () => { + const skill = getSkillContent(THINKFLEET_MEMORY_SKILL_NAME)! + for (const tool of [ + 'agentmark_memory_get', + 'agentmark_memory_set', + 'agentmark_memory_search', + 'agentmark_memory_list', + 'agentmark_memory_delete', + ]) { + expect(skill.content).toContain(tool) + } + }) +})