From 253642f06cd6569697608df3207cbc0246eb1622 Mon Sep 17 00:00:00 2001 From: Akos Orban Date: Fri, 31 Jul 2026 07:52:31 +0200 Subject: [PATCH 1/3] =?UTF-8?q?feat(skills):=20`mna=20skills=20install`=20?= =?UTF-8?q?=E2=80=94=20detect=20AI=20clients=20and=20install=20the=20skill?= =?UTF-8?q?=20+=20MCP=20server?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrangler-style agent integration. `mna skills install` detects the AI coding clients on the machine, shows exactly what it will write where, and asks once. After a successful interactive `mna login` the same offer appears behind a single `y`. Supported targets (skill dirs follow the same table Wrangler uses via rosie-skills; MCP shapes verified per client): claude-code ~/.claude/skills/mna ~/.claude.json {type:http,url} cursor ~/.cursor/skills/mna ~/.cursor/mcp.json {url} claude-desktop — claude_desktop_config npx mcp-remote windsurf ~/.codeium/windsurf/skills mcp_config.json {serverUrl} vscode — /mcp.json servers.{type:http} agents ~/.agents/skills/mna — codex ~/.codex/skills/mna — (TOML, untouched) opencode ~/.config/opencode/skills — gemini-cli ~/.gemini/skills/mna ~/.gemini/settings.json {httpUrl} Safety properties, all covered by tests: - idempotent — byte-identical files are left alone - JSON configs are merged into, never rewritten wholesale - anything modified is copied to .mna-backup- first - an unparseable config blocks that client with a clear message instead of being replaced - --dry-run touches nothing; --json refuses to write without --yes - the post-login offer is skipped entirely for --json, non-TTY and CI, and is suppressible with `mna config set skills.prompt false` The skill payload is inlined into the bundle via Bun's text loader, so install works identically from npm, the compiled Homebrew binary, and a source checkout. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AsdP2hvt4XQhYxkdNREXUq --- README.md | 79 ++++++-- src/bin/mna.ts | 14 ++ src/commands/config.ts | 29 ++- src/commands/login.ts | 36 +++- src/commands/skills/install.ts | 209 +++++++++++++++++++++ src/commands/skills/list.ts | 96 ++++++++++ src/skills/changes.test.ts | 148 +++++++++++++++ src/skills/changes.ts | 162 +++++++++++++++++ src/skills/clients.test.ts | 154 ++++++++++++++++ src/skills/clients.ts | 259 +++++++++++++++++++++++++++ src/skills/host-env.ts | 24 +++ src/skills/payload.ts | 27 +++ src/skills/plan.test.ts | 142 +++++++++++++++ src/skills/plan.ts | 149 +++++++++++++++ src/skills/post-login-prompt.test.ts | 114 ++++++++++++ src/skills/post-login-prompt.ts | 83 +++++++++ src/skills/render-plan.ts | 49 +++++ src/types/text-modules.d.ts | 9 + src/util/settings.test.ts | 49 +++++ src/util/settings.ts | 45 +++++ src/util/tty.ts | 17 ++ 21 files changed, 1862 insertions(+), 32 deletions(-) create mode 100644 src/commands/skills/install.ts create mode 100644 src/commands/skills/list.ts create mode 100644 src/skills/changes.test.ts create mode 100644 src/skills/changes.ts create mode 100644 src/skills/clients.test.ts create mode 100644 src/skills/clients.ts create mode 100644 src/skills/host-env.ts create mode 100644 src/skills/payload.ts create mode 100644 src/skills/plan.test.ts create mode 100644 src/skills/plan.ts create mode 100644 src/skills/post-login-prompt.test.ts create mode 100644 src/skills/post-login-prompt.ts create mode 100644 src/skills/render-plan.ts create mode 100644 src/types/text-modules.d.ts create mode 100644 src/util/settings.test.ts create mode 100644 src/util/settings.ts create mode 100644 src/util/tty.ts diff --git a/README.md b/README.md index e6c136c..20558af 100644 --- a/README.md +++ b/README.md @@ -3,9 +3,9 @@ > Command-line tool for [My Next Adventure](https://mynextadventure.cloud). `mna` is an open-source CLI for managing your trips on My Next Adventure. -It also ships as a [Claude Code](https://claude.com/claude-code) skill so Claude -can plan, research, build, cost, and manage whole trips on your behalf — see -[`skills/mna/SKILL.md`](./skills/mna/SKILL.md). +It also ships as an agent skill — run `mna skills install` and your AI coding +agent (Claude Code, Cursor, …) can plan, research, build, cost, and manage whole +trips on your behalf. See [`skills/mna/SKILL.md`](./skills/mna/SKILL.md). Status: **pre-1.0, alpha.** Tagged releases publish binaries to GitHub Releases. npm and Homebrew distribution land with the first tag. @@ -75,28 +75,68 @@ If you prefer headless / paste-token login: mna login --paste-token # generate the key from the user menu on app.mynextadventure.cloud ``` -## Use it as a Claude Code skill +## Use it from your AI coding agent -`mna` doubles as a [Claude Code](https://claude.com/claude-code) skill — install it and Claude -can plan, research, cost, and manage whole trips for you. **One command:** +`mna` doubles as an agent **skill** — install it and your agent can plan, research, cost, and +manage whole trips for you. **One command:** ```bash -git clone --depth 1 https://github.com/MantaCodeDevs/mna-cli /tmp/mna-cli && mkdir -p ~/.claude/skills && cp -r /tmp/mna-cli/skills/mna ~/.claude/skills/ +mna skills install ``` -This drops the skill at `~/.claude/skills/mna/`. Start (or `/reload`) Claude Code and just ask -it to plan a trip — the skill triggers on its own. (Already have the repo cloned? Just -`cp -r skills/mna ~/.claude/skills/`.) +It detects the AI clients on your machine, shows exactly what it will write where, and asks +once. After a successful `mna login` on an interactive terminal it offers the same thing with a +single `y` (silence it with `mna config set skills.prompt false`). -Installed via npm? The skill ships inside the package — no clone needed: +```bash +mna skills list # what's detected, and what's already installed +mna skills list --all # every client mna knows how to install into +mna skills install --dry-run # show the writes, change nothing +mna skills install --client cursor # one client only +mna skills install --all --yes # every supported client, no prompt +mna skills install --no-mcp # skill only, skip the MCP server entry +mna skills install --scope project # into ./.claude/skills etc. instead of $HOME +``` + +Installs are idempotent: identical files are left alone, JSON configs are **merged** (never +overwritten wholesale), anything modified is backed up next to itself as +`.mna-backup-`, and a config `mna` can't parse is reported rather than +replaced. + +### Supported clients + +| Client | `--client` | Skill | MCP server config | +|---|---|---|---| +| Claude Code | `claude-code` | `~/.claude/skills/mna/` | `~/.claude.json` → `mcpServers` | +| Cursor | `cursor` | `~/.cursor/skills/mna/` | `~/.cursor/mcp.json` → `mcpServers` | +| Claude Desktop | `claude-desktop` | — | `claude_desktop_config.json` (per-OS) → `mcpServers` | +| Windsurf / Devin Desktop | `windsurf` | `~/.codeium/windsurf/skills/mna/` | `~/.codeium/windsurf/mcp_config.json` | +| VS Code (Copilot agent mode) | `vscode` | — | `/mcp.json` → `servers` | +| Universal agent skills | `agents` | `~/.agents/skills/mna/` | — | +| Codex CLI | `codex` | `~/.codex/skills/mna/` | — (TOML config, not touched) | +| OpenCode | `opencode` | `~/.config/opencode/skills/mna/` | — | +| Gemini CLI | `gemini-cli` | `~/.gemini/skills/mna/` | `~/.gemini/settings.json` → `mcpServers` | + +The MCP entry points at the hosted server `https://mcp.mynextadventure.cloud/mcp`, written in +whichever shape the client expects — `{"type":"http","url":…}` for Claude Code and VS Code, +`{"url":…}` for Cursor, `{"httpUrl":…}` for Gemini CLI, `{"serverUrl":…}` for Windsurf, and the +`npx -y mcp-remote` stdio bridge for Claude Desktop, which has no native remote transport. If +you're logged in, your API key is embedded as an `X-API-Key` header — otherwise the client +authenticates over OAuth on first use. + +### Manual install (fallback) + +The skill is plain markdown driving the CLI, so any agent that reads `SKILL.md` files can use +it. Copy it wherever your agent looks: ```bash +# from a clone +mkdir -p ~/.claude/skills && cp -r skills/mna ~/.claude/skills/ + +# from the npm package mkdir -p ~/.claude/skills && cp -r "$(npm root -g)/@mantacodedevs/mna-cli/skills/mna" ~/.claude/skills/ ``` -Other AI coding agents can use the same `skills/mna/SKILL.md` playbook — it's plain -markdown driving the CLI. - The skill drives the `mna` CLI, so install the CLI (above) and run `mna login` first. ## Commands @@ -113,6 +153,14 @@ Every command supports `--json` for piping into `jq` or Claude. | `mna keys list` | List active API keys with `current` flag on the calling key. | | `mna keys revoke [--yes]` | Revoke an API key by name. | | `mna config get\|set apiBaseUrl []` | Read/override the API base URL locally. | +| `mna config get\|set skills.prompt [true\|false]` | Toggle the post-login "install the skill?" offer. | + +### Agent integration + +| Command | Description | +|---|---| +| `mna skills list [--all] [--scope=user\|project]` | Show detected AI clients and whether the skill/MCP server is installed. | +| `mna skills install [--client=] [--all] [--scope=user\|project] [--no-mcp] [--dry-run] [--yes]` | Install the skill (and MCP server entry) into your AI clients. | ### Trips @@ -220,7 +268,8 @@ The contract of record is `https://api.mynextadventure.cloud/v1/openapi.json`. I |---|---| | `MNA_API_KEY` | Override the key from the credentials file. Useful for CI. | | `MNA_API_BASE_URL` | Override the API base URL. Useful for local dev. | -| `XDG_CONFIG_HOME` | Where the credentials file is stored. Default: `~/.config`. | +| `XDG_CONFIG_HOME` | Where the credentials and settings files are stored. Default: `~/.config`. | +| `CI=1` | Suppresses all interactive prompts, including the post-login skill offer. | | `MNA_DEBUG=1` | Print error stack traces. | | `NO_COLOR=1` | Disable ANSI colors. | diff --git a/src/bin/mna.ts b/src/bin/mna.ts index d855d5d..2b478fc 100755 --- a/src/bin/mna.ts +++ b/src/bin/mna.ts @@ -61,6 +61,8 @@ import { collectionsAddGoalCommand } from '../commands/collections/add-goal' import { collectionsRemoveGoalCommand } from '../commands/collections/remove-goal' import { collectionsShareCommand } from '../commands/collections/share' import { collectionsOpenSharedCommand } from '../commands/collections/open-shared' +import { skillsInstallCommand } from '../commands/skills/install' +import { skillsListCommand } from '../commands/skills/list' const trips = defineCommand({ meta: { name: 'trips', description: 'View and manage trips.' }, @@ -184,6 +186,17 @@ const collections = defineCommand({ }, }) +const skills = defineCommand({ + meta: { + name: 'skills', + description: 'Install the mna skill and MCP server into your AI coding clients.', + }, + subCommands: { + list: skillsListCommand, + install: skillsInstallCommand, + }, +}) + const main = defineCommand({ meta: { name: 'mna', @@ -205,6 +218,7 @@ const main = defineCommand({ votes, goals, collections, + skills, config: configCommand, }, }) diff --git a/src/commands/config.ts b/src/commands/config.ts index 4ff6aee..1b4c01c 100644 --- a/src/commands/config.ts +++ b/src/commands/config.ts @@ -1,15 +1,23 @@ import { defineCommand } from 'citty' import { loadCredentials, resolveBaseUrl, saveCredentials } from '../auth/credentials-store' import { reportAndExit } from '../util/errors' +import { loadSettings, setSetting } from '../util/settings' import { renderJson } from '../render/json' -const KNOWN_KEYS = ['apiBaseUrl'] as const +const KNOWN_KEYS = ['apiBaseUrl', 'skills.prompt'] as const type ConfigKey = (typeof KNOWN_KEYS)[number] function isKnownKey(key: string): key is ConfigKey { return (KNOWN_KEYS as readonly string[]).includes(key) } +function parseBoolean(value: string): boolean { + const normalized = value.trim().toLowerCase() + if (['true', '1', 'yes', 'on'].includes(normalized)) return true + if (['false', '0', 'no', 'off'].includes(normalized)) return false + throw new Error(`Expected a boolean (true|false) for skills.prompt, got: ${value}`) +} + export const configGet = defineCommand({ meta: { name: 'get', description: 'Print a configuration value.' }, args: { @@ -22,13 +30,15 @@ export const configGet = defineCommand({ throw new Error(`Unknown config key: ${args.key}. Known: ${KNOWN_KEYS.join(', ')}.`) } - const creds = await loadCredentials() - const value = args.key === 'apiBaseUrl' ? resolveBaseUrl(creds) : undefined + const value: string | boolean = + args.key === 'apiBaseUrl' + ? resolveBaseUrl(await loadCredentials()) + : ((await loadSettings())['skills.prompt'] ?? true) if (args.json) { renderJson({ [args.key]: value }) } else { - process.stdout.write(`${value ?? ''}\n`) + process.stdout.write(`${value}\n`) } } catch (err) { reportAndExit(err) @@ -48,15 +58,18 @@ export const configSet = defineCommand({ throw new Error(`Unknown config key: ${args.key}. Known: ${KNOWN_KEYS.join(', ')}.`) } + if (args.key === 'skills.prompt') { + await setSetting('skills.prompt', parseBoolean(args.value)) + process.stdout.write('Updated skills.prompt.\n') + return + } + const creds = await loadCredentials() if (!creds) { throw new Error('No credentials file found. Run `mna login --paste-token ` first.') } - if (args.key === 'apiBaseUrl') { - creds.apiBaseUrl = args.value - } - + creds.apiBaseUrl = args.value await saveCredentials(creds) process.stdout.write(`Updated ${args.key}.\n`) } catch (err) { diff --git a/src/commands/login.ts b/src/commands/login.ts index 6596315..bd69bd4 100644 --- a/src/commands/login.ts +++ b/src/commands/login.ts @@ -1,9 +1,11 @@ import { defineCommand } from 'citty' import { browserLogin } from '../auth/browser-login' import { pasteTokenLogin } from '../auth/paste-token-login' -import { resolveBaseUrl } from '../auth/credentials-store' +import { resolveBaseUrl, type Credentials } from '../auth/credentials-store' import { reportAndExit } from '../util/errors' import { colors } from '../render/colors' +import { renderJson } from '../render/json' +import { maybeOfferSkillInstall } from '../skills/post-login-prompt' const DEFAULT_WEB_BASE = 'https://app.mynextadventure.cloud' @@ -21,27 +23,43 @@ export const loginCommand = defineCommand({ type: 'string', description: 'Override the web app URL (rarely needed; defaults to app.mynextadventure.cloud).', }, + json: { type: 'boolean', default: false, description: 'Output as JSON (also suppresses prompts).' }, }, async run({ args }) { try { const apiBaseUrl = resolveBaseUrl(null) const webAppBaseUrl = args['web-base-url'] ?? process.env.MNA_WEB_BASE_URL ?? DEFAULT_WEB_BASE + let credentials: Credentials + if (args['paste-token']) { - const creds = await pasteTokenLogin({ apiKey: args['paste-token'], apiBaseUrl }) - process.stdout.write(`${colors.green('✓')} Logged in.\n`) - process.stdout.write(colors.dim(' Credentials saved to ~/.config/mna/credentials\n')) - process.stdout.write(colors.dim(` Base URL: ${creds.apiBaseUrl}\n`)) - return + credentials = await pasteTokenLogin({ apiKey: args['paste-token'], apiBaseUrl }) + } else { + if (!args.json) { + process.stdout.write( + 'Opening browser for consent... If it does not open, copy the URL below.\n\n', + ) + } + const result = await browserLogin({ apiBaseUrl, webAppBaseUrl }) + credentials = result.credentials + if (!args.json) process.stdout.write(colors.dim(` ${result.consentUrl}\n\n`)) } - process.stdout.write('Opening browser for consent... If it does not open, copy the URL below.\n\n') + if (args.json) { + renderJson({ + loggedIn: true, + user: credentials.user, + apiBaseUrl: credentials.apiBaseUrl, + }) + return + } - const { credentials, consentUrl } = await browserLogin({ apiBaseUrl, webAppBaseUrl }) - process.stdout.write(colors.dim(` ${consentUrl}\n\n`)) process.stdout.write(`${colors.green('✓')} Logged in.\n`) process.stdout.write(colors.dim(' Credentials saved to ~/.config/mna/credentials\n')) process.stdout.write(colors.dim(` Base URL: ${credentials.apiBaseUrl}\n`)) + + // Courtesy offer — no-ops in CI, pipes, or when switched off. + await maybeOfferSkillInstall({ json: args.json }) } catch (err) { reportAndExit(err) } diff --git a/src/commands/skills/install.ts b/src/commands/skills/install.ts new file mode 100644 index 0000000..2134b5b --- /dev/null +++ b/src/commands/skills/install.ts @@ -0,0 +1,209 @@ +import { defineCommand } from 'citty' +import { confirm } from '@inquirer/prompts' +import { loadCredentials, resolveApiKey } from '../../auth/credentials-store' +import { renderJson } from '../../render/json' +import { colors } from '../../render/colors' +import { CLIENTS, findClient, type ClientDefinition, type Scope } from '../../skills/clients' +import { hostEnv } from '../../skills/host-env' +import { MCP_SERVER_NAME, MCP_SERVER_URL, SKILL_NAME } from '../../skills/payload' +import { applyPlan, mcpState, planForClients, skillState, type ClientPlan } from '../../skills/plan' +import { pendingCount, renderPlans, tildify } from '../../skills/render-plan' +import { reportAndExit } from '../../util/errors' +import { isInteractive } from '../../util/tty' + +function selectClients(clientArg: string | undefined, all: boolean, detected: Set): ClientDefinition[] { + if (clientArg) { + return clientArg + .split(',') + .map((name) => name.trim()) + .filter(Boolean) + .map((name) => { + const client = findClient(name) + if (!client) { + throw new Error( + `Unknown client: ${name}. Known: ${CLIENTS.map((c) => c.id).join(', ')}.`, + ) + } + return client + }) + } + if (all) return CLIENTS + return CLIENTS.filter((c) => detected.has(c.id)) +} + +function jsonView(plans: ClientPlan[], scope: Scope, dryRun: boolean) { + return { + scope, + dryRun, + mcpServer: { name: MCP_SERVER_NAME, url: MCP_SERVER_URL }, + clients: plans.map((plan) => ({ + id: plan.id, + label: plan.label, + installed: plan.installed, + skill: { path: plan.skillPath, state: skillState(plan) }, + mcp: { path: plan.mcpPath, state: mcpState(plan) }, + blocked: plan.blocked ?? null, + changes: plan.changes.map((c) => ({ + kind: c.kind, + path: c.path, + status: c.status, + label: c.label, + ...(c.kind === 'json' ? { keyPath: c.keyPath } : {}), + })), + })), + } +} + +export const skillsInstallCommand = defineCommand({ + meta: { + name: 'install', + description: `Install the ${SKILL_NAME} skill (and MCP server) into your AI coding clients.`, + }, + args: { + client: { + type: 'string', + description: 'Install into one client only (comma-separated for several). See `mna skills list --all`.', + }, + all: { + type: 'boolean', + default: false, + description: 'Install into every supported client, detected or not.', + }, + scope: { + type: 'string', + default: 'user', + description: 'Install for the current user (default) or into this project (project).', + }, + mcp: { + type: 'boolean', + default: true, + description: 'Also register the MNA MCP server. Use --no-mcp for the skill only.', + }, + yes: { type: 'boolean', default: false, description: 'Skip the confirmation prompt.' }, + 'dry-run': { type: 'boolean', default: false, description: 'Show what would change; write nothing.' }, + json: { type: 'boolean', default: false, description: 'Output as JSON.' }, + }, + async run({ args }) { + try { + const scope = args.scope + if (scope !== 'user' && scope !== 'project') { + throw new Error(`Invalid --scope: ${scope}. Choose "user" or "project".`) + } + + const dryRun = args['dry-run'] + const env = hostEnv() + const apiKey = resolveApiKey(await loadCredentials()) + + const detectionPlans = await planForClients(CLIENTS, { + env, + scope, + includeMcp: false, + apiKey, + }) + const detected = new Set(detectionPlans.filter((p) => p.installed).map((p) => p.id)) + const clients = selectClients(args.client, args.all, detected) + + if (clients.length === 0) { + const message = + 'No supported AI clients detected. Run `mna skills list --all` to see what mna can install into, or pass --client .' + if (args.json) { + renderJson({ scope, dryRun, clients: [], message }) + return + } + process.stdout.write(`${colors.yellow('!')} ${message}\n`) + return + } + + const plans = await planForClients(clients, { env, scope, includeMcp: args.mcp, apiKey }) + const pending = pendingCount(plans) + + if (args.json && !args.yes && !dryRun) { + throw new Error('Refusing to write in --json mode without --yes (or use --dry-run).') + } + + if (pending === 0) { + if (args.json) { + renderJson({ ...jsonView(plans, scope, dryRun), applied: [] }) + return + } + process.stdout.write( + `${colors.green('✓')} Already up to date for ${plans.map((p) => p.label).join(', ')}.\n`, + ) + return + } + + if (!args.json) { + process.stdout.write( + `Detected AI clients: ${colors.bold(plans.filter((p) => p.installed).map((p) => p.label).join(', ') || 'none')}\n\n`, + ) + process.stdout.write(`${dryRun ? 'Would write' : 'mna will write'}:\n\n`) + process.stdout.write(renderPlans(plans, { home: env.home, verbose: dryRun })) + if (args.mcp && plans.some((p) => p.mcpPath)) { + process.stdout.write( + colors.dim( + apiKey + ? ' The MCP entry embeds your API key so the client can authenticate.\n\n' + : ' Not logged in — the MCP entry will use OAuth. Run `mna login` first to embed an API key instead.\n\n', + ), + ) + } + } + + if (dryRun) { + if (args.json) { + renderJson({ ...jsonView(plans, scope, dryRun), applied: [] }) + } else { + process.stdout.write(colors.dim('Dry run — nothing was written.\n')) + } + return + } + + if (!args.yes) { + if (!isInteractive()) { + throw new Error('Not an interactive terminal. Re-run with --yes to install non-interactively.') + } + const overwrites = plans.some((p) => p.changes.some((c) => c.status === 'overwrite')) + const ok = await confirm({ + message: overwrites + ? 'Some files above will be overwritten (a timestamped backup is kept). Continue?' + : `Install the ${SKILL_NAME} skill for ${plans.map((p) => p.label).join(', ')}?`, + default: !overwrites, + }) + if (!ok) { + process.stdout.write(colors.dim('Aborted. Nothing was written.\n')) + process.exit(1) + } + } + + const applied = [] + for (const plan of plans) { + applied.push(await applyPlan(plan, false)) + } + + if (args.json) { + renderJson({ ...jsonView(plans, scope, dryRun), applied }) + return + } + + for (const client of applied) { + for (const change of client.applied) { + if (change.result === 'unchanged') continue + process.stdout.write( + `${colors.green('✓')} ${change.result} ${tildify(change.path, env.home)}\n`, + ) + if (change.backup) { + process.stdout.write( + colors.dim(` backup: ${tildify(change.backup, env.home)}\n`), + ) + } + } + } + process.stdout.write( + `\n${colors.green('✓')} Installed the ${SKILL_NAME} skill for ${plans.map((p) => p.label).join(', ')}.\n`, + ) + process.stdout.write(colors.dim(' Restart the client (or reload skills) and ask it to plan a trip.\n')) + } catch (err) { + reportAndExit(err) + } + }, +}) diff --git a/src/commands/skills/list.ts b/src/commands/skills/list.ts new file mode 100644 index 0000000..d63bde3 --- /dev/null +++ b/src/commands/skills/list.ts @@ -0,0 +1,96 @@ +import { defineCommand } from 'citty' +import { loadCredentials, resolveApiKey } from '../../auth/credentials-store' +import { renderJson } from '../../render/json' +import { renderTable } from '../../render/table' +import { colors } from '../../render/colors' +import { CLIENTS } from '../../skills/clients' +import { hostEnv } from '../../skills/host-env' +import { mcpState, planForClients, skillState } from '../../skills/plan' +import { tildify } from '../../skills/render-plan' +import { reportAndExit } from '../../util/errors' + +export const skillsListCommand = defineCommand({ + meta: { + name: 'list', + description: 'Show which AI clients are installed and whether the mna skill is set up.', + }, + args: { + all: { + type: 'boolean', + default: false, + description: 'Include clients that are not installed on this machine.', + }, + scope: { + type: 'string', + default: 'user', + description: 'Skill scope to report on (user|project).', + }, + json: { type: 'boolean', default: false, description: 'Output as JSON.' }, + }, + async run({ args }) { + try { + if (args.scope !== 'user' && args.scope !== 'project') { + throw new Error(`Invalid --scope: ${args.scope}. Choose "user" or "project".`) + } + + const env = hostEnv() + const apiKey = resolveApiKey(await loadCredentials()) + const plans = await planForClients(CLIENTS, { + env, + scope: args.scope, + includeMcp: true, + apiKey, + }) + const visible = args.all ? plans : plans.filter((p) => p.installed) + + if (args.json) { + renderJson({ + scope: args.scope, + authenticated: Boolean(apiKey), + clients: plans.map((plan) => ({ + id: plan.id, + label: plan.label, + installed: plan.installed, + detectedAt: plan.detectedAt, + skill: { path: plan.skillPath, state: skillState(plan) }, + mcp: { path: plan.mcpPath, state: mcpState(plan) }, + blocked: plan.blocked ?? null, + })), + }) + return + } + + renderTable({ + columns: [ + { header: 'CLIENT', key: 'client' }, + { header: 'ID', key: 'id' }, + { header: 'DETECTED', key: 'detected' }, + { header: 'SKILL', key: 'skill' }, + { header: 'MCP', key: 'mcp' }, + { header: 'SKILL PATH', key: 'path', maxWidth: 52 }, + ], + rows: visible.map((plan) => ({ + client: plan.label, + id: plan.id, + detected: plan.installed ? 'yes' : 'no', + skill: skillState(plan), + mcp: mcpState(plan), + path: plan.skillPath ? tildify(plan.skillPath, env.home) : '—', + })), + emptyMessage: 'No supported AI clients detected. Use --all to see everything mna can install into.', + }) + + for (const plan of visible) { + if (plan.blocked) { + process.stdout.write(`${colors.yellow('!')} ${plan.label}: ${plan.blocked}\n`) + } + } + + if (visible.some((p) => skillState(p) !== 'up-to-date')) { + process.stdout.write(colors.dim('\nRun `mna skills install` to set these up.\n')) + } + } catch (err) { + reportAndExit(err) + } + }, +}) diff --git a/src/skills/changes.test.ts b/src/skills/changes.test.ts new file mode 100644 index 0000000..b319836 --- /dev/null +++ b/src/skills/changes.test.ts @@ -0,0 +1,148 @@ +import { describe, expect, test, beforeEach, afterEach } from 'bun:test' +import { mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { + applyChange, + planFileChange, + planJsonChange, + readMergeableJson, + UnparseableConfigError, +} from './changes' + +let dir: string + +beforeEach(async () => { + dir = await mkdtemp(join(tmpdir(), 'mna-changes-test-')) +}) + +afterEach(async () => { + await rm(dir, { recursive: true, force: true }) +}) + +describe('planFileChange', () => { + test('create when the file does not exist', async () => { + const change = await planFileChange(join(dir, 'a', 'SKILL.md'), 'hello', 'skill') + expect(change.status).toBe('create') + }) + + test('unchanged when content is byte-identical', async () => { + const path = join(dir, 'SKILL.md') + await writeFile(path, 'hello') + expect((await planFileChange(path, 'hello', 'skill')).status).toBe('unchanged') + }) + + test('overwrite when content differs', async () => { + const path = join(dir, 'SKILL.md') + await writeFile(path, 'user edited this') + expect((await planFileChange(path, 'hello', 'skill')).status).toBe('overwrite') + }) +}) + +describe('planJsonChange', () => { + test('create when the config file is absent', async () => { + const change = await planJsonChange(join(dir, 'mcp.json'), ['mcpServers', 'mna'], { url: 'u' }, 'mcp') + expect(change.status).toBe('create') + }) + + test('create when the file exists but our key is absent', async () => { + const path = join(dir, 'mcp.json') + await writeFile(path, JSON.stringify({ mcpServers: { other: { url: 'x' } } })) + expect((await planJsonChange(path, ['mcpServers', 'mna'], { url: 'u' }, 'mcp')).status).toBe('create') + }) + + test('unchanged when our key already holds the same value', async () => { + const path = join(dir, 'mcp.json') + await writeFile(path, JSON.stringify({ mcpServers: { mna: { url: 'u' } } })) + expect((await planJsonChange(path, ['mcpServers', 'mna'], { url: 'u' }, 'mcp')).status).toBe('unchanged') + }) + + test('overwrite when our key holds a different value', async () => { + const path = join(dir, 'mcp.json') + await writeFile(path, JSON.stringify({ mcpServers: { mna: { url: 'old' } } })) + expect((await planJsonChange(path, ['mcpServers', 'mna'], { url: 'u' }, 'mcp')).status).toBe('overwrite') + }) + + test('throws UnparseableConfigError on malformed JSON instead of clobbering', async () => { + const path = join(dir, 'mcp.json') + await writeFile(path, '{ "mcpServers": ') + await expect(planJsonChange(path, ['mcpServers', 'mna'], {}, 'mcp')).rejects.toBeInstanceOf( + UnparseableConfigError, + ) + }) +}) + +describe('readMergeableJson', () => { + test('returns null for a missing file', async () => { + expect(await readMergeableJson(join(dir, 'nope.json'))).toBeNull() + }) + + test('treats an empty file as an empty object', async () => { + const path = join(dir, 'empty.json') + await writeFile(path, ' \n') + expect(await readMergeableJson(path)).toEqual({}) + }) + + test('rejects a top-level array', async () => { + const path = join(dir, 'arr.json') + await writeFile(path, '[]') + await expect(readMergeableJson(path)).rejects.toBeInstanceOf(UnparseableConfigError) + }) +}) + +describe('applyChange', () => { + test('creates nested directories for a new skill file', async () => { + const path = join(dir, 'skills', 'mna', 'references', 'x.md') + const applied = await applyChange(await planFileChange(path, 'body', 'skill')) + expect(applied.result).toBe('created') + expect(await readFile(path, 'utf8')).toBe('body') + expect(applied.backup).toBeUndefined() + }) + + test('backs up a file before overwriting it', async () => { + const path = join(dir, 'SKILL.md') + await writeFile(path, 'old content') + const applied = await applyChange(await planFileChange(path, 'new content', 'skill')) + expect(applied.result).toBe('updated') + expect(applied.backup).toBeDefined() + expect(await readFile(applied.backup!, 'utf8')).toBe('old content') + expect(await readFile(path, 'utf8')).toBe('new content') + }) + + test('does nothing when the change is unchanged', async () => { + const path = join(dir, 'SKILL.md') + await writeFile(path, 'same') + const applied = await applyChange(await planFileChange(path, 'same', 'skill')) + expect(applied.result).toBe('unchanged') + expect(await readdir(dir)).toEqual(['SKILL.md']) + }) + + test('merges into mcpServers without dropping sibling servers or other keys', async () => { + const path = join(dir, 'config.json') + await writeFile( + path, + JSON.stringify({ theme: 'dark', mcpServers: { filesystem: { command: 'npx' } } }, null, 2), + ) + await applyChange(await planJsonChange(path, ['mcpServers', 'mna'], { url: 'u' }, 'mcp')) + const after = JSON.parse(await readFile(path, 'utf8')) + expect(after).toEqual({ + theme: 'dark', + mcpServers: { filesystem: { command: 'npx' }, mna: { url: 'u' } }, + }) + }) + + test('backs up a JSON config before merging into it', async () => { + const path = join(dir, 'config.json') + await writeFile(path, JSON.stringify({ mcpServers: {} })) + const applied = await applyChange(await planJsonChange(path, ['mcpServers', 'mna'], { url: 'u' }, 'mcp')) + expect(applied.backup).toBeDefined() + expect(JSON.parse(await readFile(applied.backup!, 'utf8'))).toEqual({ mcpServers: {} }) + }) + + test('creates the JSON config (with its parent key) when absent', async () => { + const path = join(dir, 'nested', 'config.json') + const applied = await applyChange(await planJsonChange(path, ['mcpServers', 'mna'], { url: 'u' }, 'mcp')) + expect(applied.result).toBe('created') + expect(JSON.parse(await readFile(path, 'utf8'))).toEqual({ mcpServers: { mna: { url: 'u' } } }) + }) +}) diff --git a/src/skills/changes.ts b/src/skills/changes.ts new file mode 100644 index 0000000..f1e7c10 --- /dev/null +++ b/src/skills/changes.ts @@ -0,0 +1,162 @@ +import { copyFile, mkdir, readFile, writeFile } from 'node:fs/promises' +import { dirname } from 'node:path' + +export type ChangeStatus = 'create' | 'overwrite' | 'unchanged' + +/** Write a whole file (skill markdown). */ +export interface FileChange { + kind: 'file' + path: string + content: string + status: ChangeStatus + /** Human label, e.g. "skill" or "rule". */ + label: string +} + +/** Merge a single value into a JSON config at `keyPath`, leaving siblings alone. */ +export interface JsonChange { + kind: 'json' + path: string + keyPath: string[] + value: unknown + status: ChangeStatus + label: string +} + +export type PlannedChange = FileChange | JsonChange + +export interface AppliedChange { + path: string + result: 'created' | 'updated' | 'unchanged' + backup?: string +} + +export class UnparseableConfigError extends Error { + constructor( + readonly path: string, + readonly reason: string, + ) { + super(`${path} is not valid JSON (${reason}).`) + this.name = 'UnparseableConfigError' + } +} + +async function readIfExists(path: string): Promise { + try { + return await readFile(path, 'utf8') + } catch (err) { + if ((err as NodeJS.ErrnoException).code === 'ENOENT') return null + throw err + } +} + +/** + * Reads a JSON config that we intend to merge into. Missing file → `null` + * (we will create it). Present but unparseable → throw, so callers can refuse + * politely instead of destroying a config we do not understand. + */ +export async function readMergeableJson(path: string): Promise | null> { + const raw = await readIfExists(path) + if (raw === null) return null + if (raw.trim() === '') return {} + + let parsed: unknown + try { + parsed = JSON.parse(raw) + } catch (err) { + throw new UnparseableConfigError(path, (err as Error).message) + } + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { + throw new UnparseableConfigError(path, `expected an object, got ${Array.isArray(parsed) ? 'array' : typeof parsed}`) + } + return parsed as Record +} + +function getAtPath(root: Record, keyPath: string[]): unknown { + let node: unknown = root + for (const key of keyPath) { + if (node === null || typeof node !== 'object' || Array.isArray(node)) return undefined + node = (node as Record)[key] + } + return node +} + +function setAtPath(root: Record, keyPath: string[], value: unknown): void { + let node = root + for (const key of keyPath.slice(0, -1)) { + const next = node[key] + if (next === null || typeof next !== 'object' || Array.isArray(next)) { + node[key] = {} + } + node = node[key] as Record + } + node[keyPath[keyPath.length - 1]!] = value +} + +function sameJson(a: unknown, b: unknown): boolean { + return JSON.stringify(a) === JSON.stringify(b) +} + +/** Decide create/overwrite/unchanged for a file write without touching disk. */ +export async function planFileChange( + path: string, + content: string, + label: string, +): Promise { + const existing = await readIfExists(path) + const status: ChangeStatus = + existing === null ? 'create' : existing === content ? 'unchanged' : 'overwrite' + return { kind: 'file', path, content, status, label } +} + +/** Decide create/overwrite/unchanged for a JSON merge without touching disk. */ +export async function planJsonChange( + path: string, + keyPath: string[], + value: unknown, + label: string, +): Promise { + const existing = await readMergeableJson(path) + let status: ChangeStatus = 'create' + if (existing !== null) { + const current = getAtPath(existing, keyPath) + status = current === undefined ? 'create' : sameJson(current, value) ? 'unchanged' : 'overwrite' + } + return { kind: 'json', path, keyPath, value, status, label } +} + +function backupSuffix(now = new Date()): string { + return now.toISOString().replace(/[-:]/g, '').replace(/\..+$/, '') +} + +/** Copy `path` aside before we modify it. Returns the backup path. */ +export async function backupFile(path: string, now?: Date): Promise { + const target = `${path}.mna-backup-${backupSuffix(now)}` + await copyFile(path, target) + return target +} + +/** + * Performs a planned change. `unchanged` changes are no-ops. Anything that + * modifies an existing file makes a timestamped backup next to it first. + */ +export async function applyChange(change: PlannedChange): Promise { + if (change.status === 'unchanged') { + return { path: change.path, result: 'unchanged' } + } + + await mkdir(dirname(change.path), { recursive: true }) + + if (change.kind === 'file') { + const backup = change.status === 'overwrite' ? await backupFile(change.path) : undefined + await writeFile(change.path, change.content, 'utf8') + return { path: change.path, result: change.status === 'create' ? 'created' : 'updated', backup } + } + + const existing = await readMergeableJson(change.path) + const backup = existing === null ? undefined : await backupFile(change.path) + const next = existing ?? {} + setAtPath(next, change.keyPath, change.value) + await writeFile(change.path, `${JSON.stringify(next, null, 2)}\n`, 'utf8') + return { path: change.path, result: existing === null ? 'created' : 'updated', backup } +} diff --git a/src/skills/clients.test.ts b/src/skills/clients.test.ts new file mode 100644 index 0000000..fef0871 --- /dev/null +++ b/src/skills/clients.test.ts @@ -0,0 +1,154 @@ +import { describe, expect, test, beforeEach, afterEach } from 'bun:test' +import { mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { buildMcpEntry, CLIENTS, detectionPath, findClient, isClientInstalled, skillDir } from './clients' +import type { HostEnv } from './host-env' +import { MCP_SERVER_URL } from './payload' + +let fakeHome: string + +function env(overrides: Partial = {}): HostEnv { + return { home: fakeHome, platform: 'darwin', cwd: join(fakeHome, 'project'), ...overrides } +} + +beforeEach(async () => { + fakeHome = await mkdtemp(join(tmpdir(), 'mna-home-')) +}) + +afterEach(async () => { + await rm(fakeHome, { recursive: true, force: true }) +}) + +describe('client detection', () => { + test('nothing is detected in an empty home', async () => { + for (const client of CLIENTS) { + expect(await isClientInstalled(client, env())).toBe(false) + } + }) + + test('detects Claude Code from ~/.claude', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + const detected = [] + for (const client of CLIENTS) { + if (await isClientInstalled(client, env())) detected.push(client.id) + } + expect(detected).toEqual(['claude-code']) + }) + + test('detects Cursor and Claude Code independently', async () => { + await mkdir(join(fakeHome, '.cursor'), { recursive: true }) + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + expect(await isClientInstalled(findClient('cursor')!, env())).toBe(true) + expect(await isClientInstalled(findClient('claude-code')!, env())).toBe(true) + expect(await isClientInstalled(findClient('windsurf')!, env())).toBe(false) + }) + + test('detects Claude Desktop from its macOS application-support directory', async () => { + await mkdir(join(fakeHome, 'Library', 'Application Support', 'Claude'), { recursive: true }) + expect(await isClientInstalled(findClient('claude-desktop')!, env())).toBe(true) + }) + + test('detects a client from its config file even without the directory probe', async () => { + await mkdir(join(fakeHome, 'Library', 'Application Support', 'Code', 'User'), { recursive: true }) + await writeFile(join(fakeHome, 'Library', 'Application Support', 'Code', 'User', 'mcp.json'), '{}') + expect(await isClientInstalled(findClient('vscode')!, env())).toBe(true) + }) + + test('a file (not a directory) at the probe path does not count as installed', async () => { + await writeFile(join(fakeHome, '.claude'), 'not a directory') + expect(await isClientInstalled(findClient('claude-code')!, env())).toBe(false) + }) +}) + +describe('platform-specific paths', () => { + test('Claude Desktop config differs per OS', () => { + const desktop = findClient('claude-desktop')! + expect(desktop.mcp!.configPath(env({ platform: 'darwin' }))).toBe( + join(fakeHome, 'Library', 'Application Support', 'Claude', 'claude_desktop_config.json'), + ) + expect(desktop.mcp!.configPath(env({ platform: 'linux' }))).toBe( + join(fakeHome, '.config', 'Claude', 'claude_desktop_config.json'), + ) + expect( + desktop.mcp!.configPath(env({ platform: 'win32', appData: 'C:\\Users\\me\\AppData\\Roaming' })), + ).toBe(join('C:\\Users\\me\\AppData\\Roaming', 'Claude', 'claude_desktop_config.json')) + }) + + test('detectionPath points at the probe directory', () => { + expect(detectionPath(findClient('claude-code')!, env())).toBe(join(fakeHome, '.claude')) + }) +}) + +describe('skillDir', () => { + test('user scope resolves under home', () => { + expect(skillDir(findClient('claude-code')!, env(), 'user')).toBe( + join(fakeHome, '.claude', 'skills', 'mna'), + ) + }) + + test('project scope resolves under cwd', () => { + expect(skillDir(findClient('cursor')!, env(), 'project')).toBe( + join(fakeHome, 'project', '.cursor', 'skills', 'mna'), + ) + }) + + test('returns null for clients without skill support', () => { + expect(skillDir(findClient('claude-desktop')!, env(), 'user')).toBeNull() + }) +}) + +describe('buildMcpEntry', () => { + test('http style uses the native remote transport', () => { + expect(buildMcpEntry('http')).toEqual({ type: 'http', url: MCP_SERVER_URL }) + }) + + test('http style adds the X-API-Key header when a key is known', () => { + expect(buildMcpEntry('http', { apiKey: 'k' })).toEqual({ + type: 'http', + url: MCP_SERVER_URL, + headers: { 'X-API-Key': 'k' }, + }) + }) + + test('cursor style omits the type field', () => { + expect(buildMcpEntry('url', { apiKey: 'k' })).toEqual({ + url: MCP_SERVER_URL, + headers: { 'X-API-Key': 'k' }, + }) + }) + + test('gemini style selects streamable HTTP via the httpUrl key', () => { + expect(buildMcpEntry('httpUrl', { apiKey: 'k' })).toEqual({ + httpUrl: MCP_SERVER_URL, + headers: { 'X-API-Key': 'k' }, + }) + }) + + test('windsurf style uses serverUrl', () => { + expect(buildMcpEntry('serverUrl')).toEqual({ serverUrl: MCP_SERVER_URL }) + }) + + test('every registered client has a style buildMcpEntry can render', () => { + for (const client of CLIENTS) { + if (!client.mcp) continue + const entry = buildMcpEntry(client.mcp.style) + expect(JSON.stringify(entry)).toContain(MCP_SERVER_URL) + } + }) + + test('mcp-remote style bridges over stdio and keeps the key in env', () => { + expect(buildMcpEntry('mcp-remote', { apiKey: 'k' })).toEqual({ + command: 'npx', + args: ['-y', 'mcp-remote', MCP_SERVER_URL, '--header', 'X-API-Key:${MNA_API_KEY}'], + env: { MNA_API_KEY: 'k' }, + }) + }) + + test('mcp-remote without a key falls back to plain OAuth', () => { + expect(buildMcpEntry('mcp-remote')).toEqual({ + command: 'npx', + args: ['-y', 'mcp-remote', MCP_SERVER_URL], + }) + }) +}) diff --git a/src/skills/clients.ts b/src/skills/clients.ts new file mode 100644 index 0000000..b886177 --- /dev/null +++ b/src/skills/clients.ts @@ -0,0 +1,259 @@ +import { stat } from 'node:fs/promises' +import { dirname, join } from 'node:path' +import type { HostEnv } from './host-env' +import { MCP_SERVER_URL } from './payload' + +export type Scope = 'user' | 'project' + +/** + * How a client wants a remote MCP server expressed in its config. + * + * - `http` — `{ type: 'http', url, headers? }` (Claude Code, VS Code) + * - `url` — `{ url, headers? }`, no `type` field (Cursor) + * - `httpUrl` — `{ httpUrl, headers? }`; Gemini CLI picks its transport from + * the key name (`httpUrl` = streamable HTTP, `url` = SSE) + * - `serverUrl` — `{ serverUrl, headers? }` (Windsurf/Cascade) + * - `mcp-remote`— stdio bridge for clients without a remote transport + * (`npx -y mcp-remote --header X-API-Key:${MNA_API_KEY}`) + */ +export type McpStyle = 'http' | 'url' | 'httpUrl' | 'serverUrl' | 'mcp-remote' + +export interface McpTarget { + /** Resolves the config file, or null when unsupported on this platform. */ + configPath: (env: HostEnv) => string | null + /** Top-level key holding the server map ("mcpServers" almost everywhere; "servers" in VS Code). */ + serversKey: string + style: McpStyle +} + +export interface ClientDefinition { + id: string + label: string + /** Path relative to $HOME whose existence means "this client is installed". */ + detectDir: string + /** Skill directory root relative to $HOME, or null when the client has no skill support. */ + userSkillsDir: string | null + /** Skill directory root relative to the project root, or null. */ + projectSkillsDir: string | null + mcp?: McpTarget + /** Extra note surfaced in `skills list` / install output. */ + note?: string +} + +function claudeDesktopConfigPath(env: HostEnv): string | null { + if (env.platform === 'darwin') { + return join(env.home, 'Library', 'Application Support', 'Claude', 'claude_desktop_config.json') + } + if (env.platform === 'win32') { + const appData = env.appData ?? join(env.home, 'AppData', 'Roaming') + return join(appData, 'Claude', 'claude_desktop_config.json') + } + return join(env.home, '.config', 'Claude', 'claude_desktop_config.json') +} + +function claudeDesktopDetectDir(env: HostEnv): string { + const path = claudeDesktopConfigPath(env) + return path ? dirname(path) : join(env.home, '.config', 'Claude') +} + +function vscodeUserDir(env: HostEnv): string { + if (env.platform === 'darwin') return join(env.home, 'Library', 'Application Support', 'Code', 'User') + if (env.platform === 'win32') { + const appData = env.appData ?? join(env.home, 'AppData', 'Roaming') + return join(appData, 'Code', 'User') + } + return join(env.home, '.config', 'Code', 'User') +} + +/** + * Supported clients. + * + * Skill directories follow the same table Wrangler uses (via `rosie-skills`) + * for its own post-login install, which is the de-facto cross-agent + * convention: `/skills//SKILL.md`. + */ +export const CLIENTS: ClientDefinition[] = [ + { + id: 'claude-code', + label: 'Claude Code', + detectDir: '.claude', + userSkillsDir: '.claude/skills', + projectSkillsDir: '.claude/skills', + mcp: { + configPath: (env) => join(env.home, '.claude.json'), + serversKey: 'mcpServers', + style: 'http', + }, + }, + { + id: 'cursor', + label: 'Cursor', + detectDir: '.cursor', + userSkillsDir: '.cursor/skills', + projectSkillsDir: '.cursor/skills', + mcp: { + configPath: (env) => join(env.home, '.cursor', 'mcp.json'), + serversKey: 'mcpServers', + style: 'url', + }, + }, + { + id: 'claude-desktop', + label: 'Claude Desktop', + detectDir: '', + userSkillsDir: null, + projectSkillsDir: null, + mcp: { + configPath: claudeDesktopConfigPath, + serversKey: 'mcpServers', + style: 'mcp-remote', + }, + note: 'MCP server only — Claude Desktop does not read filesystem skills.', + }, + { + id: 'windsurf', + label: 'Windsurf / Devin Desktop', + detectDir: '.codeium/windsurf', + userSkillsDir: '.codeium/windsurf/skills', + projectSkillsDir: '.windsurf/skills', + mcp: { + configPath: (env) => join(env.home, '.codeium', 'windsurf', 'mcp_config.json'), + serversKey: 'mcpServers', + style: 'serverUrl', + }, + }, + { + id: 'vscode', + label: 'VS Code (Copilot agent mode)', + detectDir: '', + userSkillsDir: null, + projectSkillsDir: null, + mcp: { + configPath: (env) => join(vscodeUserDir(env), 'mcp.json'), + serversKey: 'servers', + style: 'http', + }, + note: 'MCP server only; targets the default VS Code profile.', + }, + { + id: 'codex', + label: 'Codex CLI', + detectDir: '.codex', + userSkillsDir: '.codex/skills', + projectSkillsDir: '.codex/skills', + note: 'Skill only — Codex keeps MCP servers in TOML, which `mna` does not edit.', + }, + { + id: 'opencode', + label: 'OpenCode', + detectDir: '.config/opencode', + userSkillsDir: '.config/opencode/skills', + projectSkillsDir: '.opencode/skills', + }, + { + // The cross-agent convention several tools now read (Codex, Amp, Warp, + // Copilot, Antigravity, …). Only written when it already exists, or on + // an explicit --client agents / --all. + id: 'agents', + label: 'Universal agent skills', + detectDir: '.agents', + userSkillsDir: '.agents/skills', + projectSkillsDir: '.agents/skills', + note: 'Shared ~/.agents/skills directory read by several agents.', + }, + { + id: 'gemini-cli', + label: 'Gemini CLI', + detectDir: '.gemini', + userSkillsDir: '.gemini/skills', + projectSkillsDir: '.gemini/skills', + mcp: { + configPath: (env) => join(env.home, '.gemini', 'settings.json'), + serversKey: 'mcpServers', + style: 'httpUrl', + }, + }, +] + +export function findClient(id: string): ClientDefinition | undefined { + return CLIENTS.find((c) => c.id === id) +} + +async function isDir(path: string): Promise { + try { + return (await stat(path)).isDirectory() + } catch { + return false + } +} + +async function exists(path: string): Promise { + try { + await stat(path) + return true + } catch { + return false + } +} + +/** Absolute path of the directory whose presence means "installed". */ +export function detectionPath(client: ClientDefinition, env: HostEnv): string { + if (client.id === 'claude-desktop') return claudeDesktopDetectDir(env) + if (client.id === 'vscode') return vscodeUserDir(env) + return join(env.home, client.detectDir) +} + +export async function isClientInstalled(client: ClientDefinition, env: HostEnv): Promise { + const path = detectionPath(client, env) + if (await isDir(path)) return true + // Some clients only materialise their config file, not a directory. + const configPath = client.mcp?.configPath(env) + return configPath ? await exists(configPath) : false +} + +/** Absolute path of the skill directory we would write into. */ +export function skillDir(client: ClientDefinition, env: HostEnv, scope: Scope): string | null { + if (scope === 'project') { + return client.projectSkillsDir ? join(env.cwd, client.projectSkillsDir, 'mna') : null + } + return client.userSkillsDir ? join(env.home, client.userSkillsDir, 'mna') : null +} + +export interface McpEntryOptions { + apiKey?: string + url?: string +} + +/** Builds the config value for this client's MCP server entry. */ +export function buildMcpEntry(style: McpStyle, options: McpEntryOptions = {}): Record { + const url = options.url ?? MCP_SERVER_URL + const headers = options.apiKey ? { 'X-API-Key': options.apiKey } : undefined + + if (style === 'mcp-remote') { + // mcp-remote mangles header values containing spaces, so the documented + // workaround is to pass the value through an env var placeholder. + const args = ['-y', 'mcp-remote', url] + const entry: Record = { command: 'npx', args } + if (options.apiKey) { + args.push('--header', 'X-API-Key:${MNA_API_KEY}') + entry.env = { MNA_API_KEY: options.apiKey } + } + return entry + } + + if (style === 'url') { + return headers ? { url, headers } : { url } + } + + if (style === 'httpUrl') { + return headers ? { httpUrl: url, headers } : { httpUrl: url } + } + + if (style === 'serverUrl') { + return headers ? { serverUrl: url, headers } : { serverUrl: url } + } + + // Claude Code and VS Code share this shape; they differ only in which + // top-level key the entry lives under (`mcpServers` vs `servers`). + return headers ? { type: 'http', url, headers } : { type: 'http', url } +} diff --git a/src/skills/host-env.ts b/src/skills/host-env.ts new file mode 100644 index 0000000..986653f --- /dev/null +++ b/src/skills/host-env.ts @@ -0,0 +1,24 @@ +import { homedir } from 'node:os' + +/** + * Everything the client registry needs to know about the machine. Bundled into + * one object so tests can point detection at a fake home directory without + * mutating process-wide state beyond what they control. + */ +export interface HostEnv { + home: string + platform: NodeJS.Platform + cwd: string + /** %APPDATA% on Windows; unused elsewhere. */ + appData?: string +} + +export function hostEnv(overrides: Partial = {}): HostEnv { + return { + home: process.env.HOME?.trim() || homedir(), + platform: process.platform, + cwd: process.cwd(), + appData: process.env.APPDATA, + ...overrides, + } +} diff --git a/src/skills/payload.ts b/src/skills/payload.ts new file mode 100644 index 0000000..eb819e3 --- /dev/null +++ b/src/skills/payload.ts @@ -0,0 +1,27 @@ +import skillMd from '../../skills/mna/SKILL.md' with { type: 'text' } +import cliAndSchemas from '../../skills/mna/references/cli-and-schemas.md' with { type: 'text' } +import researchAndCosting from '../../skills/mna/references/research-and-costing.md' with { type: 'text' } + +/** + * The skill payload is inlined into the bundle at build time (Bun's text + * loader) rather than read from disk. That way `mna skills install` works + * identically from npm, Homebrew's compiled binary, and a source checkout — + * none of which agree on where `skills/mna` sits relative to the entrypoint. + */ +export const SKILL_NAME = 'mna' + +export interface SkillFile { + /** Path relative to the skill directory. */ + path: string + content: string +} + +export const SKILL_FILES: SkillFile[] = [ + { path: 'SKILL.md', content: skillMd }, + { path: 'references/cli-and-schemas.md', content: cliAndSchemas }, + { path: 'references/research-and-costing.md', content: researchAndCosting }, +] + +/** The MNA MCP server (remote, streamable HTTP; OAuth-capable, also accepts X-API-Key). */ +export const MCP_SERVER_NAME = 'my-next-adventure' +export const MCP_SERVER_URL = 'https://mcp.mynextadventure.cloud/mcp' diff --git a/src/skills/plan.test.ts b/src/skills/plan.test.ts new file mode 100644 index 0000000..a430f4c --- /dev/null +++ b/src/skills/plan.test.ts @@ -0,0 +1,142 @@ +import { describe, expect, test, beforeEach, afterEach } from 'bun:test' +import { mkdir, mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { findClient } from './clients' +import type { HostEnv } from './host-env' +import { SKILL_FILES } from './payload' +import { applyPlan, mcpState, planForClient, planForClients, skillState } from './plan' + +let fakeHome: string + +function env(overrides: Partial = {}): HostEnv { + return { home: fakeHome, platform: 'darwin', cwd: join(fakeHome, 'project'), ...overrides } +} + +const claudeCode = () => findClient('claude-code')! + +beforeEach(async () => { + fakeHome = await mkdtemp(join(tmpdir(), 'mna-plan-')) + await mkdir(join(fakeHome, '.claude'), { recursive: true }) +}) + +afterEach(async () => { + await rm(fakeHome, { recursive: true, force: true }) +}) + +describe('planForClient', () => { + test('plans every shipped skill file plus the MCP entry', async () => { + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: true }) + expect(plan.installed).toBe(true) + expect(plan.changes.filter((c) => c.label === 'skill')).toHaveLength(SKILL_FILES.length) + expect(plan.changes.filter((c) => c.label === 'mcp')).toHaveLength(1) + expect(skillState(plan)).toBe('missing') + expect(mcpState(plan)).toBe('missing') + }) + + test('--no-mcp drops the config change entirely', async () => { + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: false }) + expect(plan.mcpPath).toBeNull() + expect(plan.changes.some((c) => c.label === 'mcp')).toBe(false) + expect(mcpState(plan)).toBe('n/a') + }) + + test('an unparseable client config blocks that client instead of clobbering it', async () => { + const configPath = join(fakeHome, '.claude.json') + await writeFile(configPath, '{ "mcpServers": oops') + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: true }) + expect(plan.blocked).toMatch(/not valid JSON/) + expect(plan.mcpPath).toBeNull() + + await applyPlan(plan, false) + expect(await readFile(configPath, 'utf8')).toBe('{ "mcpServers": oops') + }) + + test('reports up-to-date once the skill is installed', async () => { + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: false }) + await applyPlan(plan, false) + const replan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: false }) + expect(skillState(replan)).toBe('up-to-date') + expect(replan.changes.every((c) => c.status === 'unchanged')).toBe(true) + }) + + test('reports outdated when an installed skill file drifts', async () => { + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: false }) + await applyPlan(plan, false) + await writeFile(join(fakeHome, '.claude', 'skills', 'mna', 'SKILL.md'), 'edited by hand') + const replan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: false }) + expect(skillState(replan)).toBe('outdated') + }) +}) + +describe('applyPlan', () => { + test('dry run writes nothing at all', async () => { + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: true }) + const applied = await applyPlan(plan, true) + expect(applied.applied).toEqual([]) + expect(await readdir(join(fakeHome, '.claude'))).toEqual([]) + await expect(readFile(join(fakeHome, '.claude.json'), 'utf8')).rejects.toThrow() + }) + + test('installs the skill tree with the shipped content', async () => { + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: false }) + await applyPlan(plan, false) + const skillMd = await readFile(join(fakeHome, '.claude', 'skills', 'mna', 'SKILL.md'), 'utf8') + expect(skillMd).toBe(SKILL_FILES.find((f) => f.path === 'SKILL.md')!.content) + expect(skillMd.startsWith('---\nname: mna\n')).toBe(true) + const reference = await readFile( + join(fakeHome, '.claude', 'skills', 'mna', 'references', 'cli-and-schemas.md'), + 'utf8', + ) + expect(reference.length).toBeGreaterThan(100) + }) + + test('merges into an existing config, preserving unrelated keys and servers', async () => { + const configPath = join(fakeHome, '.claude.json') + await writeFile( + configPath, + JSON.stringify({ numStartups: 7, mcpServers: { other: { command: 'npx' } } }), + ) + const plan = await planForClient(claudeCode(), { + env: env(), + scope: 'user', + includeMcp: true, + apiKey: 'secret', + }) + await applyPlan(plan, false) + const after = JSON.parse(await readFile(configPath, 'utf8')) + expect(after.numStartups).toBe(7) + expect(after.mcpServers.other).toEqual({ command: 'npx' }) + expect(after.mcpServers['my-next-adventure']).toEqual({ + type: 'http', + url: 'https://mcp.mynextadventure.cloud/mcp', + headers: { 'X-API-Key': 'secret' }, + }) + }) + + test('a blocked plan is a no-op', async () => { + await writeFile(join(fakeHome, '.claude.json'), 'nope') + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: true }) + const applied = await applyPlan(plan, false) + expect(applied.applied).toEqual([]) + }) +}) + +describe('planForClients', () => { + test('only the clients present on the machine report installed', async () => { + const plans = await planForClients( + [claudeCode(), findClient('cursor')!, findClient('windsurf')!], + { env: env(), scope: 'user', includeMcp: false }, + ) + expect(plans.filter((p) => p.installed).map((p) => p.id)).toEqual(['claude-code']) + }) + + test('project scope targets the working directory, not home', async () => { + const plans = await planForClients([claudeCode()], { + env: env(), + scope: 'project', + includeMcp: false, + }) + expect(plans[0]!.skillPath).toBe(join(fakeHome, 'project', '.claude', 'skills', 'mna')) + }) +}) diff --git a/src/skills/plan.ts b/src/skills/plan.ts new file mode 100644 index 0000000..ca26ba7 --- /dev/null +++ b/src/skills/plan.ts @@ -0,0 +1,149 @@ +import { join } from 'node:path' +import { + applyChange, + planFileChange, + planJsonChange, + UnparseableConfigError, + type AppliedChange, + type PlannedChange, +} from './changes' +import { + buildMcpEntry, + detectionPath, + isClientInstalled, + skillDir, + type ClientDefinition, + type Scope, +} from './clients' +import type { HostEnv } from './host-env' +import { MCP_SERVER_NAME, SKILL_FILES } from './payload' + +export interface PlanOptions { + env: HostEnv + scope: Scope + /** Include the MNA MCP server entry for MCP-capable clients. */ + includeMcp: boolean + /** Stored API key, embedded as an `X-API-Key` header when present. */ + apiKey?: string +} + +export interface ClientPlan { + id: string + label: string + installed: boolean + /** The path whose presence we took as proof of installation. */ + detectedAt: string + skillPath: string | null + mcpPath: string | null + changes: PlannedChange[] + /** Set when we cannot safely touch this client (e.g. unparseable config). */ + blocked?: string + note?: string +} + +export function hasPendingChanges(plan: ClientPlan): boolean { + return plan.changes.some((c) => c.status !== 'unchanged') +} + +export type ItemState = 'up-to-date' | 'outdated' | 'missing' | 'n/a' + +function stateOf(changes: PlannedChange[], supported: boolean): ItemState { + if (!supported || changes.length === 0) return 'n/a' + if (changes.every((c) => c.status === 'unchanged')) return 'up-to-date' + if (changes.every((c) => c.status === 'create')) return 'missing' + return 'outdated' +} + +export function skillState(plan: ClientPlan): ItemState { + return stateOf( + plan.changes.filter((c) => c.label === 'skill'), + plan.skillPath !== null, + ) +} + +export function mcpState(plan: ClientPlan): ItemState { + return stateOf( + plan.changes.filter((c) => c.label === 'mcp'), + plan.mcpPath !== null, + ) +} + +/** True when the skill is installed and byte-identical to the shipped copy. */ +export function skillUpToDate(plan: ClientPlan): boolean { + return skillState(plan) === 'up-to-date' +} + +export async function planForClient(client: ClientDefinition, options: PlanOptions): Promise { + const { env, scope, includeMcp, apiKey } = options + const installed = await isClientInstalled(client, env) + const dir = skillDir(client, env, scope) + const mcpPath = includeMcp ? (client.mcp?.configPath(env) ?? null) : null + + const plan: ClientPlan = { + id: client.id, + label: client.label, + installed, + detectedAt: detectionPath(client, env), + skillPath: dir, + mcpPath, + changes: [], + note: client.note, + } + + if (dir) { + for (const file of SKILL_FILES) { + plan.changes.push(await planFileChange(join(dir, file.path), file.content, 'skill')) + } + } + + if (mcpPath && client.mcp) { + try { + plan.changes.push( + await planJsonChange( + mcpPath, + [client.mcp.serversKey, MCP_SERVER_NAME], + buildMcpEntry(client.mcp.style, { apiKey }), + 'mcp', + ), + ) + } catch (err) { + if (err instanceof UnparseableConfigError) { + plan.blocked = `${err.message} Fix or move it, then re-run — mna will not overwrite a config it cannot parse.` + plan.mcpPath = null + } else { + throw err + } + } + } + + return plan +} + +export async function planForClients( + clients: ClientDefinition[], + options: PlanOptions, +): Promise { + const plans: ClientPlan[] = [] + for (const client of clients) { + plans.push(await planForClient(client, options)) + } + return plans +} + +export interface AppliedClient { + id: string + label: string + applied: AppliedChange[] +} + +/** Executes a plan. `dryRun` short-circuits before any filesystem write. */ +export async function applyPlan(plan: ClientPlan, dryRun: boolean): Promise { + const applied: AppliedChange[] = [] + if (dryRun || plan.blocked) { + return { id: plan.id, label: plan.label, applied } + } + for (const change of plan.changes) { + applied.push(await applyChange(change)) + } + return { id: plan.id, label: plan.label, applied } +} diff --git a/src/skills/post-login-prompt.test.ts b/src/skills/post-login-prompt.test.ts new file mode 100644 index 0000000..55bd83a --- /dev/null +++ b/src/skills/post-login-prompt.test.ts @@ -0,0 +1,114 @@ +import { describe, expect, test, beforeEach, afterEach } from 'bun:test' +import { mkdir, mkdtemp, readFile, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import type { HostEnv } from './host-env' +import { maybeOfferSkillInstall } from './post-login-prompt' +import { setSetting } from '../util/settings' + +let fakeHome: string +let originalXdg: string | undefined +let asked: string[] +let output: string + +function env(): HostEnv { + return { home: fakeHome, platform: 'darwin', cwd: join(fakeHome, 'project') } +} + +function options(overrides: Record = {}) { + return { + interactive: true, + env: env(), + write: (s: string) => { + output += s + }, + confirmFn: async (message: string) => { + asked.push(message) + return true + }, + ...overrides, + } +} + +beforeEach(async () => { + originalXdg = process.env.XDG_CONFIG_HOME + fakeHome = await mkdtemp(join(tmpdir(), 'mna-postlogin-')) + process.env.XDG_CONFIG_HOME = join(fakeHome, '.config') + asked = [] + output = '' +}) + +afterEach(async () => { + process.env.XDG_CONFIG_HOME = originalXdg + await rm(fakeHome, { recursive: true, force: true }) +}) + +describe('maybeOfferSkillInstall', () => { + test('never prompts in --json mode', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + expect(await maybeOfferSkillInstall(options({ json: true }))).toBe('skipped-json') + expect(asked).toEqual([]) + }) + + test('never prompts on a non-interactive terminal', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + expect(await maybeOfferSkillInstall(options({ interactive: false }))).toBe( + 'skipped-non-interactive', + ) + expect(asked).toEqual([]) + }) + + test('respects `mna config set skills.prompt false`', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + await setSetting('skills.prompt', false) + expect(await maybeOfferSkillInstall(options())).toBe('skipped-disabled') + expect(asked).toEqual([]) + }) + + test('stays quiet when no supported client is installed', async () => { + expect(await maybeOfferSkillInstall(options())).toBe('skipped-none-detected') + expect(asked).toEqual([]) + }) + + test('offers, then installs the skill on accept', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + expect(await maybeOfferSkillInstall(options())).toBe('installed') + expect(asked).toEqual(['Install the mna skill for Claude Code?']) + const installed = await readFile(join(fakeHome, '.claude', 'skills', 'mna', 'SKILL.md'), 'utf8') + expect(installed.startsWith('---\nname: mna\n')).toBe(true) + }) + + test('writes nothing on decline and points at the opt-out', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + const result = await maybeOfferSkillInstall(options({ confirmFn: async () => false })) + expect(result).toBe('declined') + await expect( + readFile(join(fakeHome, '.claude', 'skills', 'mna', 'SKILL.md'), 'utf8'), + ).rejects.toThrow() + expect(output).toContain('mna config set skills.prompt false') + }) + + test('does not nag once the skill is already up to date', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + expect(await maybeOfferSkillInstall(options())).toBe('installed') + asked = [] + expect(await maybeOfferSkillInstall(options())).toBe('skipped-up-to-date') + expect(asked).toEqual([]) + }) + + test('never touches MCP configs — the post-login offer is skill-only', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + await maybeOfferSkillInstall(options()) + await expect(readFile(join(fakeHome, '.claude.json'), 'utf8')).rejects.toThrow() + }) + + test('offers every detected client in one prompt', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + await mkdir(join(fakeHome, '.cursor'), { recursive: true }) + expect(await maybeOfferSkillInstall(options())).toBe('installed') + expect(asked).toEqual(['Install the mna skill for Claude Code, Cursor?']) + expect( + (await readFile(join(fakeHome, '.cursor', 'skills', 'mna', 'SKILL.md'), 'utf8')).length, + ).toBeGreaterThan(100) + }) +}) diff --git a/src/skills/post-login-prompt.ts b/src/skills/post-login-prompt.ts new file mode 100644 index 0000000..6c3575d --- /dev/null +++ b/src/skills/post-login-prompt.ts @@ -0,0 +1,83 @@ +import { confirm } from '@inquirer/prompts' +import { colors } from '../render/colors' +import { CLIENTS } from './clients' +import { hostEnv, type HostEnv } from './host-env' +import { SKILL_NAME } from './payload' +import { applyPlan, hasPendingChanges, planForClients } from './plan' +import { tildify } from './render-plan' +import { skillsPromptEnabled } from '../util/settings' +import { isInteractive } from '../util/tty' + +export type PostLoginOutcome = + | 'skipped-json' + | 'skipped-non-interactive' + | 'skipped-disabled' + | 'skipped-none-detected' + | 'skipped-up-to-date' + | 'declined' + | 'installed' + +export interface PostLoginPromptOptions { + /** Machine-readable invocation — never prompt. */ + json?: boolean + /** Overrides for tests. */ + interactive?: boolean + env?: HostEnv + confirmFn?: (message: string) => Promise + write?: (s: string) => void +} + +/** + * Wrangler-style courtesy offer after a successful login: if the machine has + * AI clients that could drive `mna` and the skill isn't there yet, offer to + * install it with a single `y`. Silent in CI, pipes, `--json`, or when the + * user has turned it off with `mna config set skills.prompt false`. + */ +export async function maybeOfferSkillInstall( + options: PostLoginPromptOptions = {}, +): Promise { + const write = options.write ?? ((s: string) => void process.stdout.write(s)) + + if (options.json) return 'skipped-json' + if (!(options.interactive ?? isInteractive())) return 'skipped-non-interactive' + if (!(await skillsPromptEnabled())) return 'skipped-disabled' + + const env = options.env ?? hostEnv() + const plans = ( + await planForClients(CLIENTS, { env, scope: 'user', includeMcp: false }) + ).filter((plan) => plan.installed && plan.skillPath !== null && !plan.blocked) + + if (plans.length === 0) return 'skipped-none-detected' + + const pending = plans.filter(hasPendingChanges) + if (pending.length === 0) return 'skipped-up-to-date' + + const labels = pending.map((p) => p.label).join(', ') + write( + `\n${colors.dim(`Detected AI coding agents that could plan trips for you: ${labels}.`)}\n`, + ) + + const ask = + options.confirmFn ?? + ((message: string) => confirm({ message, default: true })) + const accepted = await ask(`Install the ${SKILL_NAME} skill for ${labels}?`) + + if (!accepted) { + write( + colors.dim( + ' Skipped. Install later with `mna skills install`, or silence this with `mna config set skills.prompt false`.\n', + ), + ) + return 'declined' + } + + for (const plan of pending) { + const applied = await applyPlan(plan, false) + for (const change of applied.applied) { + if (change.result === 'unchanged') continue + write(`${colors.green('✓')} ${change.result} ${tildify(change.path, env.home)}\n`) + } + } + write(colors.dim(' Restart the agent (or reload skills) and just ask it to plan a trip.\n')) + return 'installed' +} diff --git a/src/skills/render-plan.ts b/src/skills/render-plan.ts new file mode 100644 index 0000000..78a4030 --- /dev/null +++ b/src/skills/render-plan.ts @@ -0,0 +1,49 @@ +import { colors } from '../render/colors' +import type { PlannedChange } from './changes' +import type { ClientPlan } from './plan' + +/** Shortens `/Users/me/.claude/...` to `~/.claude/...` for display only. */ +export function tildify(path: string, home: string): string { + return home && path.startsWith(`${home}/`) ? `~${path.slice(home.length)}` : path +} + +function describe(change: PlannedChange, home: string): string { + const verb = + change.status === 'create' + ? colors.green('create ') + : change.status === 'overwrite' + ? colors.yellow('overwrite') + : colors.dim('unchanged') + const target = tildify(change.path, home) + const suffix = change.kind === 'json' ? colors.dim(` → ${change.keyPath.join('.')}`) : '' + return ` ${verb} ${target}${suffix}` +} + +export interface RenderPlanOptions { + home: string + /** Show `unchanged` lines too (used by --dry-run). */ + verbose?: boolean +} + +/** Renders the "here is exactly what I will write" block shown before confirming. */ +export function renderPlans(plans: ClientPlan[], options: RenderPlanOptions): string { + const lines: string[] = [] + for (const plan of plans) { + const changes = options.verbose ? plan.changes : plan.changes.filter((c) => c.status !== 'unchanged') + if (changes.length === 0 && !plan.blocked) continue + + lines.push(` ${colors.bold(plan.label)}`) + if (plan.blocked) { + lines.push(` ${colors.yellow('skipped')} ${plan.blocked}`) + } + for (const change of changes) { + lines.push(describe(change, options.home)) + } + lines.push('') + } + return lines.join('\n') +} + +export function pendingCount(plans: ClientPlan[]): number { + return plans.reduce((n, plan) => n + plan.changes.filter((c) => c.status !== 'unchanged').length, 0) +} diff --git a/src/types/text-modules.d.ts b/src/types/text-modules.d.ts new file mode 100644 index 0000000..39cd247 --- /dev/null +++ b/src/types/text-modules.d.ts @@ -0,0 +1,9 @@ +/** + * Text imports (`import md from './x.md' with { type: 'text' }`) are bundled + * inline by Bun, which is how the skill payload travels inside `dist/mna.js` + * and the compiled binaries. TypeScript needs the shape declared. + */ +declare module '*.md' { + const content: string + export default content +} diff --git a/src/util/settings.test.ts b/src/util/settings.test.ts new file mode 100644 index 0000000..4062235 --- /dev/null +++ b/src/util/settings.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, test, beforeEach, afterEach } from 'bun:test' +import { mkdtemp, rm, writeFile, mkdir } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { loadSettings, saveSettings, setSetting, settingsPath, skillsPromptEnabled } from './settings' + +let tmpHome: string +let originalXdg: string | undefined + +beforeEach(async () => { + originalXdg = process.env.XDG_CONFIG_HOME + tmpHome = await mkdtemp(join(tmpdir(), 'mna-settings-test-')) + process.env.XDG_CONFIG_HOME = tmpHome +}) + +afterEach(async () => { + process.env.XDG_CONFIG_HOME = originalXdg + await rm(tmpHome, { recursive: true, force: true }) +}) + +describe('settings', () => { + test('loadSettings returns {} when the file is missing', async () => { + expect(await loadSettings()).toEqual({}) + }) + + test('roundtrips through save/load', async () => { + await saveSettings({ 'skills.prompt': false }) + expect(await loadSettings()).toEqual({ 'skills.prompt': false }) + }) + + test('setSetting preserves other keys', async () => { + await saveSettings({ 'skills.prompt': true }) + await setSetting('skills.prompt', false) + expect(await loadSettings()).toEqual({ 'skills.prompt': false }) + }) + + test('malformed settings file degrades to defaults instead of throwing', async () => { + await mkdir(join(tmpHome, 'mna'), { recursive: true }) + await writeFile(settingsPath(), 'not json') + expect(await loadSettings()).toEqual({}) + expect(await skillsPromptEnabled()).toBe(true) + }) + + test('skillsPromptEnabled defaults to true and honours an explicit false', async () => { + expect(await skillsPromptEnabled()).toBe(true) + await setSetting('skills.prompt', false) + expect(await skillsPromptEnabled()).toBe(false) + }) +}) diff --git a/src/util/settings.ts b/src/util/settings.ts new file mode 100644 index 0000000..a926029 --- /dev/null +++ b/src/util/settings.ts @@ -0,0 +1,45 @@ +import { mkdir, readFile, writeFile } from 'node:fs/promises' +import { join } from 'node:path' +import { mnaConfigDir } from './xdg' + +/** + * Non-secret local preferences. Kept separate from the credentials file so + * settings survive `mna logout` and can be written before a first login. + */ +export interface Settings { + /** Offer to install the mna skill into detected AI clients after `mna login`. */ + 'skills.prompt'?: boolean +} + +export function settingsPath(): string { + return join(mnaConfigDir(), 'settings.json') +} + +export async function loadSettings(): Promise { + try { + const raw = await readFile(settingsPath(), 'utf8') + const parsed = JSON.parse(raw) as unknown + if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) return {} + return parsed as Settings + } catch { + return {} + } +} + +export async function saveSettings(settings: Settings): Promise { + const path = settingsPath() + await mkdir(mnaConfigDir(), { recursive: true, mode: 0o700 }) + await writeFile(path, `${JSON.stringify(settings, null, 2)}\n`, { mode: 0o600 }) +} + +export async function setSetting(key: K, value: Settings[K]): Promise { + const settings = await loadSettings() + settings[key] = value + await saveSettings(settings) +} + +/** Defaults to true — the post-login offer is opt-out. */ +export async function skillsPromptEnabled(): Promise { + const settings = await loadSettings() + return settings['skills.prompt'] !== false +} diff --git a/src/util/tty.ts b/src/util/tty.ts new file mode 100644 index 0000000..0e9de90 --- /dev/null +++ b/src/util/tty.ts @@ -0,0 +1,17 @@ +/** Common CI env vars — set by GitHub Actions, GitLab, CircleCI, Buildkite, etc. */ +const CI_VARS = ['CI', 'CONTINUOUS_INTEGRATION', 'BUILD_NUMBER', 'GITHUB_ACTIONS', 'GITLAB_CI'] + +export function isCI(): boolean { + return CI_VARS.some((name) => { + const value = process.env[name] + return value !== undefined && value !== '' && value !== '0' && value.toLowerCase() !== 'false' + }) +} + +/** + * True only when we can actually put a prompt in front of a human: both ends of + * the terminal are a TTY and we are not in CI. + */ +export function isInteractive(): boolean { + return Boolean(process.stdin.isTTY) && Boolean(process.stdout.isTTY) && !isCI() +} From 36ee9a42c2e0457805f41748eaffb522ee4732df Mon Sep 17 00:00:00 2001 From: Akos Orban Date: Fri, 31 Jul 2026 08:20:04 +0200 Subject: [PATCH 2/3] =?UTF-8?q?fix(skills):=20review=20fixes=20=E2=80=94?= =?UTF-8?q?=20blocked-client=20no-op,=20atomic=20writes,=20JSONC,=200600,?= =?UTF-8?q?=20error=20isolation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the FIX-FIRST review. The headline bug was a false success claim. CRITICAL - Blocked client silently skipped its skill files, then reported success. `plan.blocked` gated *all* of applyPlan but was only ever set by the MCP change, so a corrupt ~/.claude.json meant the preview promised SKILL.md, nothing was written, and the CLI still printed "✓ Installed ... for Claude Code, Cursor" with exit 0. Renamed to `mcpBlocked` and scoped to the MCP change alone; the success line is now built from the actual `applied` results, so it names only clients that really got something. - Writes are now atomic (temp file + rename) instead of truncate-in-place. An interrupt or ENOSPC mid-write can no longer truncate ~/.claude.json, which holds Claude Code's entire user state and is routinely multi-MB. IMPORTANT - Credentials: any config we write an API key into is chmod 0600, and the CLI now says plainly that the key is stored in plain text. The earlier PR description implied otherwise; it has been corrected. - Backups are 0600 (they can contain OAuth tokens) and pruned to the 3 most recent instead of accumulating forever. - Per-client, per-change error isolation: one EACCES no longer aborts the run. Failures are collected, reported per client with the backup path, and the command exits non-zero. - JSONC configs are merged, not refused. VS Code's mcp.json and Gemini CLI's settings.json routinely carry // comments; jsonc-parser edits the text in place so comments, key order, and surrounding formatting survive (the object being merged into may be re-indented — documented). - A non-object at `mcpServers` is a hard stop (ConfigConflictError) rather than being silently replaced with {} and reported as "create". HONESTY - Verified every skill directory against vendor docs. One was wrong: Codex CLI reads $HOME/.agents/skills, NOT ~/.codex/skills — the latter is a third-party compatibility claim OpenAI's own docs never make. Removed the bogus client; `--client codex` now aliases the shared ~/.agents entry. - Windsurf, Gemini CLI and OpenCode skill paths all confirmed correct. - Claude Desktop's Linux config path is NOT vendor-documented (the build is official beta, the path is convention). Flagged (?) in the plan and table. - Machinery added to mark any unverified path rather than print "up-to-date" for somewhere a client may never read. ALSO - `mna skills uninstall`. - --json failures emit JSON and a non-zero exit. - CI builds dist/mna.js and smoke-tests it on Node: jsonc-parser's UMD main has a lazy require that bundles into a broken artifact, which unit tests could never have caught. Import pinned to the ESM entry. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AsdP2hvt4XQhYxkdNREXUq --- .github/workflows/ci.yml | 11 ++ README.md | 50 +++++- bun.lock | 3 + package.json | 1 + src/bin/mna.ts | 2 + src/commands/skills/install.ts | 129 ++++++++++++--- src/commands/skills/list.ts | 35 ++++- src/commands/skills/uninstall.ts | 134 ++++++++++++++++ src/skills/changes.test.ts | 153 ++++++++++++++++++ src/skills/changes.ts | 224 +++++++++++++++++++++------ src/skills/clients.test.ts | 31 ++++ src/skills/clients.ts | 37 +++-- src/skills/plan.test.ts | 66 +++++++- src/skills/plan.ts | 49 ++++-- src/skills/post-login-prompt.test.ts | 36 ++++- src/skills/post-login-prompt.ts | 2 +- src/skills/render-plan.ts | 28 ++-- 17 files changed, 865 insertions(+), 126 deletions(-) create mode 100644 src/commands/skills/uninstall.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a213c66..5fc727f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -33,3 +33,14 @@ jobs: - name: Test run: bun test + + # The published artifact is dist/mna.js running on plain Node, which the + # unit tests never exercise. A dependency whose bundled output only breaks + # at runtime (e.g. a lazy `require` in a UMD build) is invisible without + # this step. + - name: Build and smoke-test the npm bundle on Node + run: | + bun run build + node dist/mna.js --version + node dist/mna.js skills list --all --json > /dev/null + node dist/mna.js skills install --dry-run --json > /dev/null diff --git a/README.md b/README.md index 20558af..f0fe36c 100644 --- a/README.md +++ b/README.md @@ -96,15 +96,33 @@ mna skills install --client cursor # one client only mna skills install --all --yes # every supported client, no prompt mna skills install --no-mcp # skill only, skip the MCP server entry mna skills install --scope project # into ./.claude/skills etc. instead of $HOME +mna skills uninstall # remove the skill directory again ``` -Installs are idempotent: identical files are left alone, JSON configs are **merged** (never -overwritten wholesale), anything modified is backed up next to itself as -`.mna-backup-`, and a config `mna` can't parse is reported rather than -replaced. +`--scope project` relocates the **skill** only; MCP servers have no project-level equivalent in +most clients, so they stay in the user-level config either way. + +Installs are designed to be boring and reversible: + +- **Idempotent** — identical files are left alone; a second run reports "already up to date". +- **Merged, never overwritten** — only the single `mcpServers.my-next-adventure` key is set. + Configs with `//` comments or trailing commas (VS Code, Gemini CLI) are edited in place, so + your comments and key order survive. The object being merged into may be re-indented; + nothing else in the file is touched. +- **Atomic** — every write goes to a temp file and is `rename()`d into place, so an interrupt + or a full disk cannot truncate a large config like `~/.claude.json`. +- **Backed up** — anything modified is copied to `.mna-backup-` first (mode + `0600`, most recent 3 kept). +- **Refuses what it doesn't understand** — a config that won't parse, or that has a non-object + where `mcpServers` should be, is reported and skipped rather than replaced. That only skips + the MCP entry; the skill files still install. +- **Isolated failures** — an unwritable path for one client doesn't abandon the others, and the + summary reports what was actually written. Exit code is non-zero if anything failed. ### Supported clients +Every path below is taken from that vendor's own documentation, not from convention. + | Client | `--client` | Skill | MCP server config | |---|---|---|---| | Claude Code | `claude-code` | `~/.claude/skills/mna/` | `~/.claude.json` → `mcpServers` | @@ -112,17 +130,32 @@ replaced. | Claude Desktop | `claude-desktop` | — | `claude_desktop_config.json` (per-OS) → `mcpServers` | | Windsurf / Devin Desktop | `windsurf` | `~/.codeium/windsurf/skills/mna/` | `~/.codeium/windsurf/mcp_config.json` | | VS Code (Copilot agent mode) | `vscode` | — | `/mcp.json` → `servers` | -| Universal agent skills | `agents` | `~/.agents/skills/mna/` | — | -| Codex CLI | `codex` | `~/.codex/skills/mna/` | — (TOML config, not touched) | +| Shared agent directory | `agents`, `codex` | `~/.agents/skills/mna/` | — | | OpenCode | `opencode` | `~/.config/opencode/skills/mna/` | — | | Gemini CLI | `gemini-cli` | `~/.gemini/skills/mna/` | `~/.gemini/settings.json` → `mcpServers` | +`~/.agents/skills/` is documented as a read location by Codex CLI, Gemini CLI, Cursor, OpenCode +and Windsurf, so one copy there serves several agents. **Codex CLI reads only that path** — +`~/.codex/skills/` is a third-party compatibility claim that OpenAI's own docs do not make, so +`--client codex` installs to `~/.agents/skills/`. Codex keeps MCP servers in TOML, which `mna` +does not edit. + +Two caveats the CLI also prints, marked `(?)` in the plan: + +- **Claude Desktop on Linux.** The Linux build is official (beta), but no Anthropic doc states + it reads `~/.config/Claude/claude_desktop_config.json` — only the macOS and Windows paths are + documented. On Linux `mna` writes the conventional path and tells you it is unverified. +- Anything else added on convention alone is flagged the same way rather than silently reported + as installed. + The MCP entry points at the hosted server `https://mcp.mynextadventure.cloud/mcp`, written in whichever shape the client expects — `{"type":"http","url":…}` for Claude Code and VS Code, `{"url":…}` for Cursor, `{"httpUrl":…}` for Gemini CLI, `{"serverUrl":…}` for Windsurf, and the `npx -y mcp-remote` stdio bridge for Claude Desktop, which has no native remote transport. If -you're logged in, your API key is embedded as an `X-API-Key` header — otherwise the client -authenticates over OAuth on first use. +you're logged in, **your API key is written into that config file in plain text** — that is how +these clients take credentials. `mna` sets any config it writes a key into to mode `0600`, and +says so before writing. Prefer OAuth? Use `--no-mcp`, or run `mna skills install` before +`mna login` and let the client do its own OAuth handshake on first use. ### Manual install (fallback) @@ -161,6 +194,7 @@ Every command supports `--json` for piping into `jq` or Claude. |---|---| | `mna skills list [--all] [--scope=user\|project]` | Show detected AI clients and whether the skill/MCP server is installed. | | `mna skills install [--client=] [--all] [--scope=user\|project] [--no-mcp] [--dry-run] [--yes]` | Install the skill (and MCP server entry) into your AI clients. | +| `mna skills uninstall [--client=] [--scope=user\|project] [--dry-run] [--yes]` | Remove the installed skill directory. | ### Trips diff --git a/bun.lock b/bun.lock index ca6d3db..5cc7159 100644 --- a/bun.lock +++ b/bun.lock @@ -7,6 +7,7 @@ "dependencies": { "@inquirer/prompts": "^8.4.3", "citty": "^0.1.6", + "jsonc-parser": "^3.3.1", "openapi-fetch": "^0.13.0", }, "devDependencies": { @@ -130,6 +131,8 @@ "json-schema-traverse": ["json-schema-traverse@1.0.0", "", {}, "sha512-NM8/P9n3XjXhIZn1lLhkFaACTOURQXjWhV4BA/RnOv8xvgqtqpAX9IO4mRQxSx1Rlo4tqzeqb0sOlruaOy3dug=="], + "jsonc-parser": ["jsonc-parser@3.3.1", "", {}, "sha512-HUgH65KyejrUFPvHFPbqOY0rsFip3Bo5wb4ngvdi1EpCYWUQDC5V+Y7mZws+DLkr4M//zQJoanu1SP+87Dv1oQ=="], + "minimatch": ["minimatch@5.1.9", "", { "dependencies": { "brace-expansion": "^2.0.1" } }, "sha512-7o1wEA2RyMP7Iu7GNba9vc0RWWGACJOCZBJX2GJWip0ikV+wcOsgVuY9uE8CPiyQhkGFSlhuSkZPavN7u1c2Fw=="], "ms": ["ms@2.1.3", "", {}, "sha512-6FlzubTLZG3J2a/NVCAleEhjzq5oxgHyaCU9yYXvcLsvoVaHJq/s5xXI6/XXP6tz7R9xAOtHnSO/tXtF3WRTlA=="], diff --git a/package.json b/package.json index d71f48e..0e3f69c 100644 --- a/package.json +++ b/package.json @@ -50,6 +50,7 @@ "dependencies": { "@inquirer/prompts": "^8.4.3", "citty": "^0.1.6", + "jsonc-parser": "^3.3.1", "openapi-fetch": "^0.13.0" }, "devDependencies": { diff --git a/src/bin/mna.ts b/src/bin/mna.ts index 2b478fc..4816970 100755 --- a/src/bin/mna.ts +++ b/src/bin/mna.ts @@ -63,6 +63,7 @@ import { collectionsShareCommand } from '../commands/collections/share' import { collectionsOpenSharedCommand } from '../commands/collections/open-shared' import { skillsInstallCommand } from '../commands/skills/install' import { skillsListCommand } from '../commands/skills/list' +import { skillsUninstallCommand } from '../commands/skills/uninstall' const trips = defineCommand({ meta: { name: 'trips', description: 'View and manage trips.' }, @@ -194,6 +195,7 @@ const skills = defineCommand({ subCommands: { list: skillsListCommand, install: skillsInstallCommand, + uninstall: skillsUninstallCommand, }, }) diff --git a/src/commands/skills/install.ts b/src/commands/skills/install.ts index 2134b5b..d68dd6e 100644 --- a/src/commands/skills/install.ts +++ b/src/commands/skills/install.ts @@ -6,7 +6,14 @@ import { colors } from '../../render/colors' import { CLIENTS, findClient, type ClientDefinition, type Scope } from '../../skills/clients' import { hostEnv } from '../../skills/host-env' import { MCP_SERVER_NAME, MCP_SERVER_URL, SKILL_NAME } from '../../skills/payload' -import { applyPlan, mcpState, planForClients, skillState, type ClientPlan } from '../../skills/plan' +import { + applyPlan, + mcpState, + planForClients, + skillState, + type AppliedClient, + type ClientPlan, +} from '../../skills/plan' import { pendingCount, renderPlans, tildify } from '../../skills/render-plan' import { reportAndExit } from '../../util/errors' import { isInteractive } from '../../util/tty' @@ -40,9 +47,13 @@ function jsonView(plans: ClientPlan[], scope: Scope, dryRun: boolean) { id: plan.id, label: plan.label, installed: plan.installed, - skill: { path: plan.skillPath, state: skillState(plan) }, + skill: { + path: plan.skillPath, + state: skillState(plan), + pathVerified: plan.skillPathVerified, + }, mcp: { path: plan.mcpPath, state: mcpState(plan) }, - blocked: plan.blocked ?? null, + mcpBlocked: plan.mcpBlocked ?? null, changes: plan.changes.map((c) => ({ kind: c.kind, path: c.path, @@ -72,7 +83,7 @@ export const skillsInstallCommand = defineCommand({ scope: { type: 'string', default: 'user', - description: 'Install for the current user (default) or into this project (project).', + description: 'Install the skill for the current user (default) or into this project (project).', }, mcp: { type: 'boolean', @@ -84,6 +95,7 @@ export const skillsInstallCommand = defineCommand({ json: { type: 'boolean', default: false, description: 'Output as JSON.' }, }, async run({ args }) { + const asJson = args.json try { const scope = args.scope if (scope !== 'user' && scope !== 'project') { @@ -106,8 +118,8 @@ export const skillsInstallCommand = defineCommand({ if (clients.length === 0) { const message = 'No supported AI clients detected. Run `mna skills list --all` to see what mna can install into, or pass --client .' - if (args.json) { - renderJson({ scope, dryRun, clients: [], message }) + if (asJson) { + renderJson({ ok: true, scope, dryRun, clients: [], applied: [], message }) return } process.stdout.write(`${colors.yellow('!')} ${message}\n`) @@ -117,41 +129,69 @@ export const skillsInstallCommand = defineCommand({ const plans = await planForClients(clients, { env, scope, includeMcp: args.mcp, apiKey }) const pending = pendingCount(plans) - if (args.json && !args.yes && !dryRun) { + if (asJson && !args.yes && !dryRun) { throw new Error('Refusing to write in --json mode without --yes (or use --dry-run).') } if (pending === 0) { - if (args.json) { - renderJson({ ...jsonView(plans, scope, dryRun), applied: [] }) + if (asJson) { + renderJson({ ok: true, ...jsonView(plans, scope, dryRun), applied: [], failures: [] }) return } process.stdout.write( `${colors.green('✓')} Already up to date for ${plans.map((p) => p.label).join(', ')}.\n`, ) + for (const plan of plans) { + if (plan.mcpBlocked) { + process.stdout.write( + `${colors.yellow('!')} ${plan.label}: ${tildify(plan.mcpBlocked, env.home)}\n`, + ) + } + } return } - if (!args.json) { - process.stdout.write( - `Detected AI clients: ${colors.bold(plans.filter((p) => p.installed).map((p) => p.label).join(', ') || 'none')}\n\n`, - ) + if (!asJson) { + const detectedLabels = plans + .filter((p) => p.installed) + .map((p) => p.label) + .join(', ') + process.stdout.write(`Detected AI clients: ${colors.bold(detectedLabels || 'none')}\n\n`) process.stdout.write(`${dryRun ? 'Would write' : 'mna will write'}:\n\n`) process.stdout.write(renderPlans(plans, { home: env.home, verbose: dryRun })) + + if ( + plans.some((p) => !p.skillPathVerified && p.skillPath) || + plans.some((p) => !p.mcpPathVerified && p.mcpPath) + ) { + process.stdout.write( + colors.dim( + ' (?) marks a path that follows convention but is not documented by that\n' + + ' vendor for your OS — the client may not actually read it.\n\n', + ), + ) + } if (args.mcp && plans.some((p) => p.mcpPath)) { process.stdout.write( colors.dim( apiKey - ? ' The MCP entry embeds your API key so the client can authenticate.\n\n' + ? ' Your API key is written into that config in plain text (mna sets the file to 0600).\n\n' : ' Not logged in — the MCP entry will use OAuth. Run `mna login` first to embed an API key instead.\n\n', ), ) } + if (scope === 'project') { + process.stdout.write( + colors.dim( + ' --scope project relocates the skill only; MCP servers stay in the user-level config.\n\n', + ), + ) + } } if (dryRun) { - if (args.json) { - renderJson({ ...jsonView(plans, scope, dryRun), applied: [] }) + if (asJson) { + renderJson({ ok: true, ...jsonView(plans, scope, dryRun), applied: [], failures: [] }) } else { process.stdout.write(colors.dim('Dry run — nothing was written.\n')) } @@ -175,13 +215,30 @@ export const skillsInstallCommand = defineCommand({ } } - const applied = [] + // Each client is applied independently: one unwritable path must not + // abandon the rest, and the summary below reports what *actually* + // happened rather than what was planned. + const applied: AppliedClient[] = [] for (const plan of plans) { applied.push(await applyPlan(plan, false)) } - if (args.json) { - renderJson({ ...jsonView(plans, scope, dryRun), applied }) + const wrote = applied.flatMap((c) => c.applied.filter((a) => a.result !== 'unchanged')) + const failures = applied.flatMap((c) => c.errors.map((e) => ({ client: c.label, ...e }))) + const succeeded = applied.filter((c) => c.applied.some((a) => a.result !== 'unchanged')) + const blocked = plans.filter((p) => p.mcpBlocked) + + if (asJson) { + renderJson({ + // `ok` tracks the exit code: hard write failures only. A + // blocked MCP config is a warning — the skill still landed. + ok: failures.length === 0, + ...jsonView(plans, scope, dryRun), + applied, + failures, + warnings: blocked.map((p) => ({ client: p.label, message: p.mcpBlocked })), + }) + if (failures.length > 0) process.exit(2) return } @@ -192,17 +249,43 @@ export const skillsInstallCommand = defineCommand({ `${colors.green('✓')} ${change.result} ${tildify(change.path, env.home)}\n`, ) if (change.backup) { - process.stdout.write( - colors.dim(` backup: ${tildify(change.backup, env.home)}\n`), - ) + process.stdout.write(colors.dim(` backup: ${tildify(change.backup, env.home)}\n`)) } } } + + for (const failure of failures) { + process.stderr.write( + `${colors.red('✖')} ${failure.client}: could not write ${tildify(failure.path, env.home)} — ${failure.message}\n`, + ) + if (failure.backup) { + process.stderr.write( + colors.dim(` your original is at ${tildify(failure.backup, env.home)}\n`), + ) + } + } + for (const plan of blocked) { + process.stderr.write( + `${colors.yellow('!')} ${plan.label}: ${tildify(plan.mcpBlocked!, env.home)}\n`, + ) + } + + if (wrote.length === 0) { + process.stderr.write(`\n${colors.red('✖')} Nothing was installed.\n`) + process.exit(2) + } + process.stdout.write( - `\n${colors.green('✓')} Installed the ${SKILL_NAME} skill for ${plans.map((p) => p.label).join(', ')}.\n`, + `\n${colors.green('✓')} Installed the ${SKILL_NAME} skill for ${succeeded.map((c) => c.label).join(', ')}.\n`, ) process.stdout.write(colors.dim(' Restart the client (or reload skills) and ask it to plan a trip.\n')) + + if (failures.length > 0) process.exit(2) } catch (err) { + if (asJson) { + renderJson({ ok: false, error: err instanceof Error ? err.message : String(err) }) + process.exit(1) + } reportAndExit(err) } }, diff --git a/src/commands/skills/list.ts b/src/commands/skills/list.ts index d63bde3..2ca055a 100644 --- a/src/commands/skills/list.ts +++ b/src/commands/skills/list.ts @@ -52,9 +52,17 @@ export const skillsListCommand = defineCommand({ label: plan.label, installed: plan.installed, detectedAt: plan.detectedAt, - skill: { path: plan.skillPath, state: skillState(plan) }, - mcp: { path: plan.mcpPath, state: mcpState(plan) }, - blocked: plan.blocked ?? null, + skill: { + path: plan.skillPath, + state: skillState(plan), + pathVerified: plan.skillPathVerified, + }, + mcp: { + path: plan.mcpPath, + state: mcpState(plan), + pathVerified: plan.mcpPathVerified, + }, + mcpBlocked: plan.mcpBlocked ?? null, })), }) return @@ -73,19 +81,32 @@ export const skillsListCommand = defineCommand({ client: plan.label, id: plan.id, detected: plan.installed ? 'yes' : 'no', - skill: skillState(plan), - mcp: mcpState(plan), + skill: plan.skillPathVerified ? skillState(plan) : `${skillState(plan)} (?)`, + mcp: plan.mcpPathVerified ? mcpState(plan) : `${mcpState(plan)} (?)`, path: plan.skillPath ? tildify(plan.skillPath, env.home) : '—', })), emptyMessage: 'No supported AI clients detected. Use --all to see everything mna can install into.', }) for (const plan of visible) { - if (plan.blocked) { - process.stdout.write(`${colors.yellow('!')} ${plan.label}: ${plan.blocked}\n`) + if (plan.mcpBlocked) { + process.stdout.write( + `${colors.yellow('!')} ${plan.label}: ${tildify(plan.mcpBlocked, env.home)}\n`, + ) } } + if ( + visible.some((p) => !p.skillPathVerified && p.skillPath) || + visible.some((p) => !p.mcpPathVerified && p.mcpPath) + ) { + process.stdout.write( + colors.dim( + '\n(?) path follows convention but is not documented by that vendor for your OS —\n the client may not actually read it.\n', + ), + ) + } + if (visible.some((p) => skillState(p) !== 'up-to-date')) { process.stdout.write(colors.dim('\nRun `mna skills install` to set these up.\n')) } diff --git a/src/commands/skills/uninstall.ts b/src/commands/skills/uninstall.ts new file mode 100644 index 0000000..e74a4bc --- /dev/null +++ b/src/commands/skills/uninstall.ts @@ -0,0 +1,134 @@ +import { defineCommand } from 'citty' +import { confirm } from '@inquirer/prompts' +import { rm } from 'node:fs/promises' +import { renderJson } from '../../render/json' +import { colors } from '../../render/colors' +import { CLIENTS, findClient, type ClientDefinition } from '../../skills/clients' +import { hostEnv } from '../../skills/host-env' +import { SKILL_NAME } from '../../skills/payload' +import { planForClients } from '../../skills/plan' +import { tildify } from '../../skills/render-plan' +import { reportAndExit } from '../../util/errors' +import { isInteractive } from '../../util/tty' + +export const skillsUninstallCommand = defineCommand({ + meta: { + name: 'uninstall', + description: `Remove the ${SKILL_NAME} skill directory from your AI coding clients.`, + }, + args: { + client: { type: 'string', description: 'Uninstall from one client only (comma-separated).' }, + scope: { type: 'string', default: 'user', description: 'Which copy to remove (user|project).' }, + yes: { type: 'boolean', default: false, description: 'Skip the confirmation prompt.' }, + 'dry-run': { type: 'boolean', default: false, description: 'Show what would be removed.' }, + json: { type: 'boolean', default: false, description: 'Output as JSON.' }, + }, + async run({ args }) { + const asJson = args.json + try { + const scope = args.scope + if (scope !== 'user' && scope !== 'project') { + throw new Error(`Invalid --scope: ${scope}. Choose "user" or "project".`) + } + + const env = hostEnv() + let clients: ClientDefinition[] = CLIENTS + if (args.client) { + clients = args.client + .split(',') + .map((n) => n.trim()) + .filter(Boolean) + .map((name) => { + const client = findClient(name) + if (!client) throw new Error(`Unknown client: ${name}.`) + return client + }) + } + + const plans = await planForClients(clients, { env, scope, includeMcp: false }) + // Only offer to remove skills that are actually on disk. + const targets = plans.filter((p) => p.skillPath && p.changes.some((c) => c.status !== 'create')) + + if (targets.length === 0) { + const message = `No installed ${SKILL_NAME} skill found.` + if (asJson) { + renderJson({ ok: true, removed: [], message }) + return + } + process.stdout.write(colors.dim(`${message}\n`)) + return + } + + if (!asJson) { + process.stdout.write(`${args['dry-run'] ? 'Would remove' : 'Will remove'}:\n\n`) + for (const plan of targets) { + process.stdout.write(` ${colors.bold(plan.label)}\n`) + process.stdout.write(` ${colors.red('remove')} ${tildify(plan.skillPath!, env.home)}\n`) + } + process.stdout.write('\n') + process.stdout.write( + colors.dim(' MCP server entries are left alone — remove those from the client config by hand.\n\n'), + ) + } + + if (args['dry-run']) { + if (asJson) { + renderJson({ ok: true, dryRun: true, removed: targets.map((p) => p.skillPath) }) + } else { + process.stdout.write(colors.dim('Dry run — nothing was removed.\n')) + } + return + } + + if (!args.yes) { + if (!isInteractive()) { + throw new Error('Not an interactive terminal. Re-run with --yes.') + } + const ok = await confirm({ + message: `Remove the ${SKILL_NAME} skill from ${targets.map((p) => p.label).join(', ')}?`, + default: false, + }) + if (!ok) { + process.stdout.write(colors.dim('Aborted.\n')) + process.exit(1) + } + } + + const removed: string[] = [] + const failures: { path: string; message: string }[] = [] + for (const plan of targets) { + try { + await rm(plan.skillPath!, { recursive: true, force: true }) + removed.push(plan.skillPath!) + } catch (err) { + failures.push({ + path: plan.skillPath!, + message: err instanceof Error ? err.message : String(err), + }) + } + } + + if (asJson) { + renderJson({ ok: failures.length === 0, removed, failures }) + if (failures.length > 0) process.exit(2) + return + } + + for (const path of removed) { + process.stdout.write(`${colors.green('✓')} removed ${tildify(path, env.home)}\n`) + } + for (const failure of failures) { + process.stderr.write( + `${colors.red('✖')} could not remove ${tildify(failure.path, env.home)} — ${failure.message}\n`, + ) + } + if (failures.length > 0) process.exit(2) + } catch (err) { + if (asJson) { + renderJson({ ok: false, error: err instanceof Error ? err.message : String(err) }) + process.exit(1) + } + reportAndExit(err) + } + }, +}) diff --git a/src/skills/changes.test.ts b/src/skills/changes.test.ts index b319836..2017bf0 100644 --- a/src/skills/changes.test.ts +++ b/src/skills/changes.test.ts @@ -2,12 +2,17 @@ import { describe, expect, test, beforeEach, afterEach } from 'bun:test' import { mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { chmod, stat } from 'node:fs/promises' import { applyChange, + BACKUP_RETENTION, + backupFile, + ConfigConflictError, planFileChange, planJsonChange, readMergeableJson, UnparseableConfigError, + writeFileAtomic, } from './changes' let dir: string @@ -146,3 +151,151 @@ describe('applyChange', () => { expect(JSON.parse(await readFile(path, 'utf8'))).toEqual({ mcpServers: { mna: { url: 'u' } } }) }) }) + +describe('JSONC configs', () => { + // VS Code's mcp.json and Gemini CLI's settings.json are JSONC by + // convention. Treating a commented config as corrupt would send the user + // down a dead end, and rewriting it with JSON.stringify would silently eat + // their comments. + const jsonc = `{ + // servers I actually use + "servers": { + "local": { "command": "npx" }, // trailing comma next + }, + /* block comment */ + "other": true +} +` + + test('parses comments and trailing commas instead of refusing', async () => { + const path = join(dir, 'mcp.json') + await writeFile(path, jsonc) + expect(await readMergeableJson(path)).toEqual({ + servers: { local: { command: 'npx' } }, + other: true, + }) + }) + + test('merging preserves comments, key order, and untouched formatting', async () => { + const path = join(dir, 'mcp.json') + await writeFile(path, jsonc) + await applyChange(await planJsonChange(path, ['servers', 'mna'], { type: 'http' }, 'mcp')) + const after = await readFile(path, 'utf8') + expect(after).toContain('// servers I actually use') + expect(after).toContain('/* block comment */') + expect(after).toContain('"local"') + expect(after).toContain('"mna"') + expect(JSON.parse(JSON.stringify(await readMergeableJson(path)))).toMatchObject({ + servers: { local: { command: 'npx' }, mna: { type: 'http' } }, + other: true, + }) + }) + + test('re-planning after a JSONC merge reports unchanged (idempotent)', async () => { + const path = join(dir, 'mcp.json') + await writeFile(path, jsonc) + const value = { type: 'http' } + await applyChange(await planJsonChange(path, ['servers', 'mna'], value, 'mcp')) + expect((await planJsonChange(path, ['servers', 'mna'], value, 'mcp')).status).toBe('unchanged') + }) +}) + +describe('non-object intermediates', () => { + test('planJsonChange refuses when the container key holds a string', async () => { + const path = join(dir, 'config.json') + await writeFile(path, JSON.stringify({ mcpServers: 'nope' })) + await expect(planJsonChange(path, ['mcpServers', 'mna'], {}, 'mcp')).rejects.toBeInstanceOf( + ConfigConflictError, + ) + }) + + test('planJsonChange refuses when the container key holds an array', async () => { + const path = join(dir, 'config.json') + await writeFile(path, JSON.stringify({ mcpServers: [] })) + await expect(planJsonChange(path, ['mcpServers', 'mna'], {}, 'mcp')).rejects.toBeInstanceOf( + ConfigConflictError, + ) + }) + + test('planJsonChange refuses when the container key holds null', async () => { + const path = join(dir, 'config.json') + await writeFile(path, JSON.stringify({ mcpServers: null })) + await expect(planJsonChange(path, ['mcpServers', 'mna'], {}, 'mcp')).rejects.toBeInstanceOf( + ConfigConflictError, + ) + }) + + test('an absent container is fine — that is a create, not a conflict', async () => { + const path = join(dir, 'config.json') + await writeFile(path, JSON.stringify({ unrelated: 1 })) + expect((await planJsonChange(path, ['mcpServers', 'mna'], {}, 'mcp')).status).toBe('create') + }) +}) + +describe('atomic writes', () => { + test('a failed write leaves the original intact and cleans up the temp file', async () => { + const path = join(dir, 'config.json') + await writeFile(path, '{"original": true}') + // A circular value makes JSON.stringify throw *after* we have started, + // standing in for ENOSPC / an interrupt mid-write. + await expect(writeFileAtomic(join(dir, 'nonexistent-dir', 'x.json'), 'data')).rejects.toThrow() + expect(await readFile(path, 'utf8')).toBe('{"original": true}') + expect((await readdir(dir)).filter((f) => f.includes('mna-tmp'))).toEqual([]) + }) + + test('leaves no temp files behind on success', async () => { + const path = join(dir, 'config.json') + await writeFileAtomic(path, 'hello') + expect(await readFile(path, 'utf8')).toBe('hello') + expect((await readdir(dir)).filter((f) => f.includes('mna-tmp'))).toEqual([]) + }) + + test('preserves the existing file mode when not forcing one', async () => { + const path = join(dir, 'config.json') + await writeFile(path, 'a') + await chmod(path, 0o640) + await writeFileAtomic(path, 'b') + expect((await stat(path)).mode & 0o777).toBe(0o640) + }) +}) + +describe('credential handling', () => { + test('a config carrying an API key ends up owner-only (0600)', async () => { + const path = join(dir, 'config.json') + const change = await planJsonChange( + path, + ['mcpServers', 'mna'], + { url: 'u', headers: { 'X-API-Key': 'secret' } }, + 'mcp', + true, + ) + await applyChange(change) + expect((await stat(path)).mode & 0o777).toBe(0o600) + }) + + test('a config without a key keeps default permissions', async () => { + const path = join(dir, 'config.json') + await applyChange(await planJsonChange(path, ['mcpServers', 'mna'], { url: 'u' }, 'mcp', false)) + expect((await stat(path)).mode & 0o777).not.toBe(0o600) + }) + + test('backups are owner-only even when the original was world-readable', async () => { + const path = join(dir, 'config.json') + await writeFile(path, '{"oauth":"token"}') + await chmod(path, 0o644) + const backup = await backupFile(path) + expect((await stat(backup)).mode & 0o777).toBe(0o600) + }) + + test('only the most recent backups are kept', async () => { + const path = join(dir, 'config.json') + await writeFile(path, '{}') + for (let i = 0; i < BACKUP_RETENTION + 3; i++) { + await backupFile(path, new Date(Date.UTC(2026, 0, 1, 0, 0, i))) + } + const backups = (await readdir(dir)).filter((f) => f.includes('.mna-backup-')) + expect(backups).toHaveLength(BACKUP_RETENTION) + // The survivors are the newest ones. + expect(backups.sort().at(-1)).toContain('20260101T000005') + }) +}) diff --git a/src/skills/changes.ts b/src/skills/changes.ts index f1e7c10..2c194eb 100644 --- a/src/skills/changes.ts +++ b/src/skills/changes.ts @@ -1,5 +1,16 @@ -import { copyFile, mkdir, readFile, writeFile } from 'node:fs/promises' -import { dirname } from 'node:path' +import { randomBytes } from 'node:crypto' +import { chmod, copyFile, mkdir, readFile, readdir, rename, rm, stat, writeFile } from 'node:fs/promises' +import { basename, dirname, join } from 'node:path' +// Import the ESM build explicitly: the package's `main` is a UMD bundle whose +// lazy `require('./impl/format')` survives bundling and blows up at runtime in +// dist/mna.js. The ESM entry uses static imports and bundles cleanly. +import { + applyEdits, + modify, + parse as parseJsonc, + printParseErrorCode, + type ParseError, +} from 'jsonc-parser/lib/esm/main.js' export type ChangeStatus = 'create' | 'overwrite' | 'unchanged' @@ -9,7 +20,7 @@ export interface FileChange { path: string content: string status: ChangeStatus - /** Human label, e.g. "skill" or "rule". */ + /** Human label, e.g. "skill" or "mcp". */ label: string } @@ -21,6 +32,8 @@ export interface JsonChange { value: unknown status: ChangeStatus label: string + /** The value embeds a credential — the file must end up owner-only. */ + secret: boolean } export type PlannedChange = FileChange | JsonChange @@ -31,6 +44,7 @@ export interface AppliedChange { backup?: string } +/** The config exists but we cannot parse it — refuse rather than replace it. */ export class UnparseableConfigError extends Error { constructor( readonly path: string, @@ -41,6 +55,24 @@ export class UnparseableConfigError extends Error { } } +/** + * The config parses, but the key we need to merge into holds something that is + * not an object (a string, array, or null). Overwriting it would destroy data, + * so we refuse just as loudly as for a parse failure. + */ +export class ConfigConflictError extends Error { + constructor( + readonly path: string, + readonly keyPath: string[], + readonly actual: string, + ) { + super( + `${path} has a non-object value at "${keyPath.join('.')}" (found ${actual}). mna will not replace it.`, + ) + this.name = 'ConfigConflictError' + } +} + async function readIfExists(path: string): Promise { try { return await readFile(path, 'utf8') @@ -50,59 +82,112 @@ async function readIfExists(path: string): Promise { } } +function isPlainObject(value: unknown): value is Record { + return value !== null && typeof value === 'object' && !Array.isArray(value) +} + +function describeType(value: unknown): string { + if (value === null) return 'null' + if (Array.isArray(value)) return 'an array' + return `a ${typeof value}` +} + +export interface ConfigDoc { + /** Raw text, or null when the file does not exist. */ + text: string | null + /** Parsed object, or null when the file does not exist. */ + data: Record | null +} + /** - * Reads a JSON config that we intend to merge into. Missing file → `null` - * (we will create it). Present but unparseable → throw, so callers can refuse - * politely instead of destroying a config we do not understand. + * Reads a config we intend to merge into. Comments and trailing commas are + * tolerated — VS Code's `mcp.json` and Gemini CLI's `settings.json` are JSONC + * by convention, and treating a commented config as "corrupt" would send users + * down a dead end. */ -export async function readMergeableJson(path: string): Promise | null> { - const raw = await readIfExists(path) - if (raw === null) return null - if (raw.trim() === '') return {} +export async function readConfigDoc(path: string): Promise { + const text = await readIfExists(path) + if (text === null) return { text: null, data: null } + if (text.trim() === '') return { text, data: {} } - let parsed: unknown - try { - parsed = JSON.parse(raw) - } catch (err) { - throw new UnparseableConfigError(path, (err as Error).message) + const errors: ParseError[] = [] + const parsed = parseJsonc(text, errors, { allowTrailingComma: true, disallowComments: false }) + + if (errors.length > 0) { + const first = errors[0]! + throw new UnparseableConfigError(path, `${printParseErrorCode(first.error)} at offset ${first.offset}`) } - if (parsed === null || typeof parsed !== 'object' || Array.isArray(parsed)) { - throw new UnparseableConfigError(path, `expected an object, got ${Array.isArray(parsed) ? 'array' : typeof parsed}`) + if (!isPlainObject(parsed)) { + throw new UnparseableConfigError(path, `expected an object, got ${describeType(parsed)}`) } - return parsed as Record + return { text, data: parsed } +} + +/** Back-compat alias used by callers that only want the parsed object. */ +export async function readMergeableJson(path: string): Promise | null> { + return (await readConfigDoc(path)).data } function getAtPath(root: Record, keyPath: string[]): unknown { let node: unknown = root for (const key of keyPath) { - if (node === null || typeof node !== 'object' || Array.isArray(node)) return undefined - node = (node as Record)[key] + if (!isPlainObject(node)) return undefined + node = node[key] } return node } -function setAtPath(root: Record, keyPath: string[], value: unknown): void { - let node = root - for (const key of keyPath.slice(0, -1)) { - const next = node[key] - if (next === null || typeof next !== 'object' || Array.isArray(next)) { - node[key] = {} +/** + * Every container along `keyPath` must be absent or a plain object. A string, + * array, or null in the way is a hard stop — silently replacing it with `{}` + * would delete whatever the user had there. + */ +function assertPathMergeable(path: string, root: Record, keyPath: string[]): void { + let node: unknown = root + for (let i = 0; i < keyPath.length - 1; i++) { + const key = keyPath[i]! + node = (node as Record)[key] + if (node === undefined) return + if (!isPlainObject(node)) { + throw new ConfigConflictError(path, keyPath.slice(0, i + 1), describeType(node)) } - node = node[key] as Record } - node[keyPath[keyPath.length - 1]!] = value } function sameJson(a: unknown, b: unknown): boolean { return JSON.stringify(a) === JSON.stringify(b) } +/** + * Writes via a temp file in the same directory plus `rename()`, which is + * atomic on POSIX. A crash, a full disk, or a Ctrl-C mid-write therefore + * leaves the original file intact rather than truncated — this matters because + * `~/.claude.json` holds Claude Code's entire user state and is routinely + * multiple megabytes. + */ +export async function writeFileAtomic(path: string, content: string, mode?: number): Promise { + const tmp = join(dirname(path), `.${basename(path)}.mna-tmp-${process.pid}-${randomBytes(4).toString('hex')}`) + try { + await writeFile(tmp, content, 'utf8') + const targetMode = mode ?? (await currentMode(path)) + if (targetMode !== undefined) await chmod(tmp, targetMode) + await rename(tmp, path) + } catch (err) { + await rm(tmp, { force: true }) + throw err + } +} + +async function currentMode(path: string): Promise { + try { + return (await stat(path)).mode & 0o777 + } catch { + return undefined + } +} + /** Decide create/overwrite/unchanged for a file write without touching disk. */ -export async function planFileChange( - path: string, - content: string, - label: string, -): Promise { +export async function planFileChange(path: string, content: string, label: string): Promise { const existing = await readIfExists(path) const status: ChangeStatus = existing === null ? 'create' : existing === content ? 'unchanged' : 'overwrite' @@ -115,30 +200,54 @@ export async function planJsonChange( keyPath: string[], value: unknown, label: string, + secret = false, ): Promise { - const existing = await readMergeableJson(path) + const { data } = await readConfigDoc(path) let status: ChangeStatus = 'create' - if (existing !== null) { - const current = getAtPath(existing, keyPath) + if (data !== null) { + assertPathMergeable(path, data, keyPath) + const current = getAtPath(data, keyPath) status = current === undefined ? 'create' : sameJson(current, value) ? 'unchanged' : 'overwrite' } - return { kind: 'json', path, keyPath, value, status, label } + return { kind: 'json', path, keyPath, value, status, label, secret } } function backupSuffix(now = new Date()): string { return now.toISOString().replace(/[-:]/g, '').replace(/\..+$/, '') } -/** Copy `path` aside before we modify it. Returns the backup path. */ +/** How many timestamped backups of a given file we keep around. */ +export const BACKUP_RETENTION = 3 + +async function pruneBackups(path: string, keep = BACKUP_RETENTION): Promise { + const dir = dirname(path) + const prefix = `${basename(path)}.mna-backup-` + try { + const entries = (await readdir(dir)).filter((name) => name.startsWith(prefix)).sort() + for (const stale of entries.slice(0, Math.max(0, entries.length - keep))) { + await rm(join(dir, stale), { force: true }) + } + } catch { + // Pruning is best-effort; never fail an install over it. + } +} + +/** + * Copy `path` aside before we modify it. Backups inherit 0600 because the + * originals can contain OAuth tokens and API keys, and only the most recent + * few are kept. + */ export async function backupFile(path: string, now?: Date): Promise { const target = `${path}.mna-backup-${backupSuffix(now)}` await copyFile(path, target) + await chmod(target, 0o600) + await pruneBackups(path) return target } /** * Performs a planned change. `unchanged` changes are no-ops. Anything that - * modifies an existing file makes a timestamped backup next to it first. + * modifies an existing file is backed up first, and every write is atomic. */ export async function applyChange(change: PlannedChange): Promise { if (change.status === 'unchanged') { @@ -149,14 +258,37 @@ export async function applyChange(change: PlannedChange): Promise if (change.kind === 'file') { const backup = change.status === 'overwrite' ? await backupFile(change.path) : undefined - await writeFile(change.path, change.content, 'utf8') + await writeFileAtomic(change.path, change.content) return { path: change.path, result: change.status === 'create' ? 'created' : 'updated', backup } } - const existing = await readMergeableJson(change.path) - const backup = existing === null ? undefined : await backupFile(change.path) - const next = existing ?? {} - setAtPath(next, change.keyPath, change.value) - await writeFile(change.path, `${JSON.stringify(next, null, 2)}\n`, 'utf8') - return { path: change.path, result: existing === null ? 'created' : 'updated', backup } + const { text, data } = await readConfigDoc(change.path) + if (data !== null) assertPathMergeable(change.path, data, change.keyPath) + const backup = text === null ? undefined : await backupFile(change.path) + + // For an existing config, edit the text in place: jsonc-parser rewrites only + // the affected span, so comments, key order, and formatting all survive. + const next = + text === null + ? `${JSON.stringify(buildNested(change.keyPath, change.value), null, 2)}\n` + : applyEdits( + text, + modify(text, change.keyPath, change.value, { + formattingOptions: { insertSpaces: true, tabSize: 2 }, + }), + ) + + await writeFileAtomic(change.path, next, change.secret ? 0o600 : undefined) + return { path: change.path, result: text === null ? 'created' : 'updated', backup } +} + +function buildNested(keyPath: string[], value: unknown): Record { + const root: Record = {} + let node = root + for (const key of keyPath.slice(0, -1)) { + node[key] = {} + node = node[key] as Record + } + node[keyPath[keyPath.length - 1]!] = value + return root } diff --git a/src/skills/clients.test.ts b/src/skills/clients.test.ts index fef0871..a49a867 100644 --- a/src/skills/clients.test.ts +++ b/src/skills/clients.test.ts @@ -152,3 +152,34 @@ describe('buildMcpEntry', () => { }) }) }) + +describe('vendor-verification flags', () => { + test('every registered skill directory is vendor-documented', () => { + // If a future client is added on convention alone, mark it + // skillPathVerified: false so the CLI can say so out loud. + for (const client of CLIENTS) { + if (!client.userSkillsDir) continue + expect({ id: client.id, verified: client.skillPathVerified ?? true }).toEqual({ + id: client.id, + verified: true, + }) + } + }) + + test('Claude Desktop on Linux is flagged: the config path is not vendor-documented', () => { + const desktop = findClient('claude-desktop')! + expect(desktop.mcpPathUnverifiedOn).toEqual(['linux']) + }) + + test('Codex resolves to ~/.agents/skills, the only path OpenAI documents', () => { + // ~/.codex/skills is a third-party compatibility claim, not an OpenAI one. + const codex = findClient('codex')! + expect(codex.id).toBe('agents') + expect(skillDir(codex, env(), 'user')).toBe(join(fakeHome, '.agents', 'skills', 'mna')) + expect(CLIENTS.some((c) => c.userSkillsDir === '.codex/skills')).toBe(false) + }) + + test('unknown client names still resolve to undefined', () => { + expect(findClient('not-a-client')).toBeUndefined() + }) +}) diff --git a/src/skills/clients.ts b/src/skills/clients.ts index b886177..bfea797 100644 --- a/src/skills/clients.ts +++ b/src/skills/clients.ts @@ -28,6 +28,8 @@ export interface McpTarget { export interface ClientDefinition { id: string + /** Extra names accepted by `--client`. */ + aliases?: string[] label: string /** Path relative to $HOME whose existence means "this client is installed". */ detectDir: string @@ -38,6 +40,14 @@ export interface ClientDefinition { mcp?: McpTarget /** Extra note surfaced in `skills list` / install output. */ note?: string + /** + * False when the skill directory follows the cross-agent convention but is + * NOT documented by the vendor — i.e. we believe the client reads it, but + * cannot prove it. Surfaced to the user rather than hidden. + */ + skillPathVerified?: boolean + /** Platforms where the MCP config path is a convention, not vendor-documented. */ + mcpPathUnverifiedOn?: NodeJS.Platform[] } function claudeDesktopConfigPath(env: HostEnv): string | null { @@ -109,6 +119,10 @@ export const CLIENTS: ClientDefinition[] = [ style: 'mcp-remote', }, note: 'MCP server only — Claude Desktop does not read filesystem skills.', + // The Linux build is official (beta), but no Anthropic doc states that it + // reads ~/.config/Claude/claude_desktop_config.json — only macOS and + // Windows paths are documented. + mcpPathUnverifiedOn: ['linux'], }, { id: 'windsurf', @@ -135,14 +149,6 @@ export const CLIENTS: ClientDefinition[] = [ }, note: 'MCP server only; targets the default VS Code profile.', }, - { - id: 'codex', - label: 'Codex CLI', - detectDir: '.codex', - userSkillsDir: '.codex/skills', - projectSkillsDir: '.codex/skills', - note: 'Skill only — Codex keeps MCP servers in TOML, which `mna` does not edit.', - }, { id: 'opencode', label: 'OpenCode', @@ -151,15 +157,18 @@ export const CLIENTS: ClientDefinition[] = [ projectSkillsDir: '.opencode/skills', }, { - // The cross-agent convention several tools now read (Codex, Amp, Warp, - // Copilot, Antigravity, …). Only written when it already exists, or on - // an explicit --client agents / --all. + // The cross-agent directory. Vendor-documented as a read location by + // Codex CLI (where it is the *only* documented user-level path), + // Gemini CLI, Cursor, OpenCode, and Windsurf. Note the Agent Skills + // spec itself defines skill *contents*, not discovery paths — this is + // a widely-adopted convention rather than a normative requirement. id: 'agents', - label: 'Universal agent skills', + aliases: ['codex'], + label: 'Universal agent skills (~/.agents)', detectDir: '.agents', userSkillsDir: '.agents/skills', projectSkillsDir: '.agents/skills', - note: 'Shared ~/.agents/skills directory read by several agents.', + note: 'Read by Codex CLI, Gemini CLI, Cursor, OpenCode and Windsurf. Codex CLI reads only this path — not ~/.codex/skills.', }, { id: 'gemini-cli', @@ -176,7 +185,7 @@ export const CLIENTS: ClientDefinition[] = [ ] export function findClient(id: string): ClientDefinition | undefined { - return CLIENTS.find((c) => c.id === id) + return CLIENTS.find((c) => c.id === id || c.aliases?.includes(id)) } async function isDir(path: string): Promise { diff --git a/src/skills/plan.test.ts b/src/skills/plan.test.ts index a430f4c..f00f340 100644 --- a/src/skills/plan.test.ts +++ b/src/skills/plan.test.ts @@ -41,15 +41,38 @@ describe('planForClient', () => { expect(mcpState(plan)).toBe('n/a') }) - test('an unparseable client config blocks that client instead of clobbering it', async () => { + test('an unparseable client config blocks the MCP entry only, never the skill', async () => { const configPath = join(fakeHome, '.claude.json') await writeFile(configPath, '{ "mcpServers": oops') const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: true }) - expect(plan.blocked).toMatch(/not valid JSON/) + expect(plan.mcpBlocked).toMatch(/not valid JSON/) expect(plan.mcpPath).toBeNull() + // The skill files are independent of that config and must survive. + expect(plan.changes.filter((c) => c.label === 'skill')).toHaveLength(SKILL_FILES.length) - await applyPlan(plan, false) + const applied = await applyPlan(plan, false) + // Corrupt config untouched... expect(await readFile(configPath, 'utf8')).toBe('{ "mcpServers": oops') + // ...but the skill still landed, and is reported as such. + expect(applied.applied.filter((a) => a.result === 'created')).toHaveLength(SKILL_FILES.length) + expect(applied.errors).toEqual([]) + expect( + (await readFile(join(fakeHome, '.claude', 'skills', 'mna', 'SKILL.md'), 'utf8')).length, + ).toBeGreaterThan(100) + }) + + test('a non-object at mcpServers is a hard stop, not a silent replacement', async () => { + const configPath = join(fakeHome, '.claude.json') + await writeFile(configPath, JSON.stringify({ mcpServers: 'not an object', keep: 1 })) + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: true }) + expect(plan.mcpBlocked).toMatch(/non-object value at "mcpServers"/) + expect(plan.mcpPath).toBeNull() + + await applyPlan(plan, false) + expect(JSON.parse(await readFile(configPath, 'utf8'))).toEqual({ + mcpServers: 'not an object', + keep: 1, + }) }) test('reports up-to-date once the skill is installed', async () => { @@ -114,11 +137,16 @@ describe('applyPlan', () => { }) }) - test('a blocked plan is a no-op', async () => { - await writeFile(join(fakeHome, '.claude.json'), 'nope') - const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: true }) + test('one unwritable path does not abandon the other files', async () => { + const plan = await planForClient(claudeCode(), { env: env(), scope: 'user', includeMcp: false }) + // Make the references directory un-creatable by putting a file in its place. + await mkdir(join(fakeHome, '.claude', 'skills', 'mna'), { recursive: true }) + await writeFile(join(fakeHome, '.claude', 'skills', 'mna', 'references'), 'in the way') + const applied = await applyPlan(plan, false) - expect(applied.applied).toEqual([]) + expect(applied.errors.length).toBeGreaterThan(0) + // SKILL.md is not under the blocked directory, so it still installs. + expect(applied.applied.some((a) => a.result === 'created')).toBe(true) }) }) @@ -140,3 +168,27 @@ describe('planForClients', () => { expect(plans[0]!.skillPath).toBe(join(fakeHome, 'project', '.claude', 'skills', 'mna')) }) }) + +describe('path verification is surfaced, not hidden', () => { + test('a convention-only skill path is marked unverified on the plan', async () => { + const madeUp = { ...claudeCode(), id: 'madeup', skillPathVerified: false } + const plan = await planForClient(madeUp, { env: env(), scope: 'user', includeMcp: false }) + expect(plan.skillPathVerified).toBe(false) + }) + + test('Claude Desktop config is flagged unverified on Linux but not on macOS', async () => { + const desktop = findClient('claude-desktop')! + const onLinux = await planForClient(desktop, { + env: env({ platform: 'linux' }), + scope: 'user', + includeMcp: true, + }) + const onMac = await planForClient(desktop, { + env: env({ platform: 'darwin' }), + scope: 'user', + includeMcp: true, + }) + expect(onLinux.mcpPathVerified).toBe(false) + expect(onMac.mcpPathVerified).toBe(true) + }) +}) diff --git a/src/skills/plan.ts b/src/skills/plan.ts index ca26ba7..f592865 100644 --- a/src/skills/plan.ts +++ b/src/skills/plan.ts @@ -1,6 +1,7 @@ import { join } from 'node:path' import { applyChange, + ConfigConflictError, planFileChange, planJsonChange, UnparseableConfigError, @@ -36,9 +37,17 @@ export interface ClientPlan { skillPath: string | null mcpPath: string | null changes: PlannedChange[] - /** Set when we cannot safely touch this client (e.g. unparseable config). */ - blocked?: string + /** + * Why the MCP entry was dropped for this client (unparseable or conflicting + * config). Scoped to the MCP entry only — the skill files are unaffected + * and still install. + */ + mcpBlocked?: string note?: string + /** False when the skill directory is a community convention, not vendor-documented. */ + skillPathVerified: boolean + /** False when the MCP config path is not vendor-documented on this platform. */ + mcpPathVerified: boolean } export function hasPendingChanges(plan: ClientPlan): boolean { @@ -88,6 +97,8 @@ export async function planForClient(client: ClientDefinition, options: PlanOptio mcpPath, changes: [], note: client.note, + skillPathVerified: client.skillPathVerified ?? true, + mcpPathVerified: !client.mcpPathUnverifiedOn?.includes(env.platform), } if (dir) { @@ -104,11 +115,14 @@ export async function planForClient(client: ClientDefinition, options: PlanOptio [client.mcp.serversKey, MCP_SERVER_NAME], buildMcpEntry(client.mcp.style, { apiKey }), 'mcp', + Boolean(apiKey), ), ) } catch (err) { - if (err instanceof UnparseableConfigError) { - plan.blocked = `${err.message} Fix or move it, then re-run — mna will not overwrite a config it cannot parse.` + if (err instanceof UnparseableConfigError || err instanceof ConfigConflictError) { + // Drop *only* the MCP entry. The skill files are independent of + // this config and must still install. + plan.mcpBlocked = `${err.message} Fix or move it, then re-run \`mna skills install\` — the skill files are unaffected.` plan.mcpPath = null } else { throw err @@ -130,20 +144,37 @@ export async function planForClients( return plans } +export interface ApplyError { + path: string + message: string + /** Backup taken before the failed write, if any — the user's escape hatch. */ + backup?: string +} + export interface AppliedClient { id: string label: string applied: AppliedChange[] + errors: ApplyError[] } -/** Executes a plan. `dryRun` short-circuits before any filesystem write. */ +/** + * Executes a plan. `dryRun` short-circuits before any filesystem write. A + * failure on one change is recorded and the rest still run, so a single + * unwritable path cannot silently abandon the other files. + */ export async function applyPlan(plan: ClientPlan, dryRun: boolean): Promise { const applied: AppliedChange[] = [] - if (dryRun || plan.blocked) { - return { id: plan.id, label: plan.label, applied } + const errors: ApplyError[] = [] + if (dryRun) { + return { id: plan.id, label: plan.label, applied, errors } } for (const change of plan.changes) { - applied.push(await applyChange(change)) + try { + applied.push(await applyChange(change)) + } catch (err) { + errors.push({ path: change.path, message: err instanceof Error ? err.message : String(err) }) + } } - return { id: plan.id, label: plan.label, applied } + return { id: plan.id, label: plan.label, applied, errors } } diff --git a/src/skills/post-login-prompt.test.ts b/src/skills/post-login-prompt.test.ts index 55bd83a..07421c7 100644 --- a/src/skills/post-login-prompt.test.ts +++ b/src/skills/post-login-prompt.test.ts @@ -1,7 +1,8 @@ import { describe, expect, test, beforeEach, afterEach } from 'bun:test' -import { mkdir, mkdtemp, readFile, rm } from 'node:fs/promises' +import { mkdir, mkdtemp, readFile, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { CLIENTS } from './clients' import type { HostEnv } from './host-env' import { maybeOfferSkillInstall } from './post-login-prompt' import { setSetting } from '../util/settings' @@ -112,3 +113,36 @@ describe('maybeOfferSkillInstall', () => { ).toBeGreaterThan(100) }) }) + +describe('post-login offer safety', () => { + test('offers the shared ~/.agents directory when the user already has one', async () => { + // Codex CLI, Gemini CLI, Cursor, OpenCode and Windsurf all document + // reading this path, so one copy there serves several agents. + await mkdir(join(fakeHome, '.agents'), { recursive: true }) + expect(await maybeOfferSkillInstall(options())).toBe('installed') + expect(asked).toEqual(['Install the mna skill for Universal agent skills (~/.agents)?']) + expect( + (await readFile(join(fakeHome, '.agents', 'skills', 'mna', 'SKILL.md'), 'utf8')).length, + ).toBeGreaterThan(100) + }) + + test('the offer is restricted to vendor-documented skill directories', async () => { + // The guard that makes the above safe: anything marked unverified is + // never written without an explicit `mna skills install`. + for (const client of CLIENTS) { + if (!client.userSkillsDir) continue + expect(client.skillPathVerified ?? true).toBe(true) + } + }) + + test('a corrupt MCP config does not stop the skill being offered', async () => { + await mkdir(join(fakeHome, '.claude'), { recursive: true }) + await writeFile(join(fakeHome, '.claude.json'), 'not json at all') + expect(await maybeOfferSkillInstall(options())).toBe('installed') + expect( + (await readFile(join(fakeHome, '.claude', 'skills', 'mna', 'SKILL.md'), 'utf8')).length, + ).toBeGreaterThan(100) + // ...and the broken config is still untouched. + expect(await readFile(join(fakeHome, '.claude.json'), 'utf8')).toBe('not json at all') + }) +}) diff --git a/src/skills/post-login-prompt.ts b/src/skills/post-login-prompt.ts index 6c3575d..1b2380a 100644 --- a/src/skills/post-login-prompt.ts +++ b/src/skills/post-login-prompt.ts @@ -45,7 +45,7 @@ export async function maybeOfferSkillInstall( const env = options.env ?? hostEnv() const plans = ( await planForClients(CLIENTS, { env, scope: 'user', includeMcp: false }) - ).filter((plan) => plan.installed && plan.skillPath !== null && !plan.blocked) + ).filter((plan) => plan.installed && plan.skillPath !== null && plan.skillPathVerified) if (plans.length === 0) return 'skipped-none-detected' diff --git a/src/skills/render-plan.ts b/src/skills/render-plan.ts index 78a4030..c42e7ea 100644 --- a/src/skills/render-plan.ts +++ b/src/skills/render-plan.ts @@ -2,12 +2,17 @@ import { colors } from '../render/colors' import type { PlannedChange } from './changes' import type { ClientPlan } from './plan' -/** Shortens `/Users/me/.claude/...` to `~/.claude/...` for display only. */ -export function tildify(path: string, home: string): string { - return home && path.startsWith(`${home}/`) ? `~${path.slice(home.length)}` : path +/** + * Shortens `/Users/me/.claude/...` to `~/.claude/...` for display only. + * Replaces every occurrence so it also tidies paths embedded in a sentence + * (e.g. the "config is not valid JSON" message). + */ +export function tildify(text: string, home: string): string { + if (!home) return text + return text.split(`${home}/`).join('~/') } -function describe(change: PlannedChange, home: string): string { +function describe(change: PlannedChange, home: string, unverified: boolean): string { const verb = change.status === 'create' ? colors.green('create ') @@ -16,7 +21,8 @@ function describe(change: PlannedChange, home: string): string { : colors.dim('unchanged') const target = tildify(change.path, home) const suffix = change.kind === 'json' ? colors.dim(` → ${change.keyPath.join('.')}`) : '' - return ` ${verb} ${target}${suffix}` + const marker = unverified ? colors.dim(' (?)') : '' + return ` ${verb} ${target}${suffix}${marker}` } export interface RenderPlanOptions { @@ -30,14 +36,16 @@ export function renderPlans(plans: ClientPlan[], options: RenderPlanOptions): st const lines: string[] = [] for (const plan of plans) { const changes = options.verbose ? plan.changes : plan.changes.filter((c) => c.status !== 'unchanged') - if (changes.length === 0 && !plan.blocked) continue + if (changes.length === 0 && !plan.mcpBlocked) continue lines.push(` ${colors.bold(plan.label)}`) - if (plan.blocked) { - lines.push(` ${colors.yellow('skipped')} ${plan.blocked}`) - } for (const change of changes) { - lines.push(describe(change, options.home)) + const unverified = + change.label === 'skill' ? !plan.skillPathVerified : !plan.mcpPathVerified + lines.push(describe(change, options.home, unverified)) + } + if (plan.mcpBlocked) { + lines.push(` ${colors.yellow('skipped')} ${tildify(plan.mcpBlocked, options.home)}`) } lines.push('') } From 081338761bc38ae51446e25db712058e48fbf6ea Mon Sep 17 00:00:00 2001 From: Akos Orban Date: Fri, 31 Jul 2026 09:29:47 +0200 Subject: [PATCH 3/3] fix(skills): accurate success line, real jsonc smoke coverage, cited skill paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third review round. N1 — success line still over-claimed, in a new shape. `succeeded` was "any change written", with no notion of *what* was written, so a client whose three skill files all failed EACCES but whose MCP entry landed still produced "✓ Installed the mna skill for Claude Code". Reproduced before fixing. Root cause was structural: AppliedChange carried no label, so the command literally could not tell a skill write from an MCP write. Added `label` to AppliedChange and split the summary into "Installed the mna skill for …" (gated on real skill writes) and "Registered the MNA MCP server for …". N2 — the dist smoke test didn't cover the class it was added for. On CI's pristine $HOME no client is detected, so none of the three commands ever reached parse/modify/applyEdits and a lazy require inside them would still have shipped. The step now seeds a temp $HOME with a commented ~/.cursor/mcp.json, runs a real `skills install --client cursor --yes` against the built bundle, and asserts the comment, the existing server, the new entry and the SKILL.md all survive. Verified by running the extracted script locally. N3 — honesty. `skillPathVerified` was set on zero clients, so the (?) marker and both legends were inert for skills while the README asserted every path was vendor-documented. Rather than soften the claim, I verified each path against the vendor's own page and recorded the URL in the registry as `skillPathDocs`: claude-code code.claude.com/docs/en/skills "Personal | ~/.claude/skills/…" cursor cursor.com/docs/skills user: ~/.cursor/skills/ windsurf docs.devin.ai/desktop/cascade/skills scope table: Global | ~/.codeium/windsurf/skills/ opencode opencode.ai/docs/skills/ "~/.config/opencode/skills/*/SKILL.md" agents learn.chatgpt.com/docs/build-skills Codex USER scope = $HOME/.agents/skills gemini-cli github.com/google-gemini/gemini-cli "User skills: ~/.gemini/skills/" A test now fails if a client with a skill directory has no citation. The URLs are surfaced in `skills list --json`, so the README's claim is checkable. The ~/.agents comment no longer reads as a hedge: five vendors documenting the same path is not the same as a spec guarantee, and it now says exactly that. The (?) machinery stays live via Claude Desktop's undocumented Linux config path. Minors: ApplyError.backup is populated (ApplyFailedError carries it) so the "your original is at …" branch is no longer dead — proved with a pure toApplyError test, since an atomic rename makes it unreachable via the filesystem; --json shapes shared between list and install via clientJsonView; secrets are written with mode 0600 at creation instead of chmod-after, closing a brief world-readable window; backup names are millisecond-precision and collision-proof (a second-granularity name silently overwrote the earlier backup within one run). Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01AsdP2hvt4XQhYxkdNREXUq --- .github/workflows/ci.yml | 27 +++++++++++++++- README.md | 25 +++++++------- src/commands/skills/install.ts | 35 ++++++++++++-------- src/commands/skills/list.ts | 19 ++--------- src/skills/changes.test.ts | 37 ++++++++++++++++++++- src/skills/changes.ts | 59 +++++++++++++++++++++++++++++----- src/skills/clients.test.ts | 22 +++++++++++++ src/skills/clients.ts | 24 +++++++++++--- src/skills/json-view.ts | 26 +++++++++++++++ src/skills/plan.test.ts | 34 +++++++++++++++++++- src/skills/plan.ts | 20 +++++++++++- 11 files changed, 269 insertions(+), 59 deletions(-) create mode 100644 src/skills/json-view.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 5fc727f..58a9c6b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -42,5 +42,30 @@ jobs: run: | bun run build node dist/mna.js --version + + # Seed a throwaway HOME with a JSONC config so the install actually + # reaches jsonc-parser's parse/modify/applyEdits. On a pristine HOME + # nothing is detected, no config is read, and a lazy require inside + # those functions would still ship — which is the exact class of bug + # this step exists to catch. + SMOKE_HOME="$(mktemp -d)" + export HOME="$SMOKE_HOME" XDG_CONFIG_HOME="$SMOKE_HOME/.config" + mkdir -p "$SMOKE_HOME/.cursor" + printf '%s\n' \ + '{' \ + ' // a comment jsonc-parser must preserve' \ + ' "mcpServers": {' \ + ' "existing": { "url": "https://example.invalid/mcp" },' \ + ' }' \ + '}' > "$SMOKE_HOME/.cursor/mcp.json" + node dist/mna.js skills list --all --json > /dev/null - node dist/mna.js skills install --dry-run --json > /dev/null + node dist/mna.js skills install --client cursor --yes + + echo "--- resulting config ---" + cat "$SMOKE_HOME/.cursor/mcp.json" + grep -q 'a comment jsonc-parser must preserve' "$SMOKE_HOME/.cursor/mcp.json" + grep -q 'my-next-adventure' "$SMOKE_HOME/.cursor/mcp.json" + grep -q 'existing' "$SMOKE_HOME/.cursor/mcp.json" + test -f "$SMOKE_HOME/.cursor/skills/mna/SKILL.md" + echo "bundle smoke test OK" diff --git a/README.md b/README.md index f0fe36c..30544e2 100644 --- a/README.md +++ b/README.md @@ -121,24 +121,27 @@ Installs are designed to be boring and reversible: ### Supported clients -Every path below is taken from that vendor's own documentation, not from convention. +Every skill directory below is the one that vendor documents — sources linked, and `mna skills +list --json` reports the same URL per client so you can check without taking our word for it. | Client | `--client` | Skill | MCP server config | |---|---|---|---| -| Claude Code | `claude-code` | `~/.claude/skills/mna/` | `~/.claude.json` → `mcpServers` | -| Cursor | `cursor` | `~/.cursor/skills/mna/` | `~/.cursor/mcp.json` → `mcpServers` | +| Claude Code | `claude-code` | [`~/.claude/skills/mna/`](https://code.claude.com/docs/en/skills) | `~/.claude.json` → `mcpServers` | +| Cursor | `cursor` | [`~/.cursor/skills/mna/`](https://cursor.com/docs/skills) | `~/.cursor/mcp.json` → `mcpServers` | | Claude Desktop | `claude-desktop` | — | `claude_desktop_config.json` (per-OS) → `mcpServers` | -| Windsurf / Devin Desktop | `windsurf` | `~/.codeium/windsurf/skills/mna/` | `~/.codeium/windsurf/mcp_config.json` | +| Windsurf / Devin Desktop | `windsurf` | [`~/.codeium/windsurf/skills/mna/`](https://docs.devin.ai/desktop/cascade/skills) | `~/.codeium/windsurf/mcp_config.json` | | VS Code (Copilot agent mode) | `vscode` | — | `/mcp.json` → `servers` | -| Shared agent directory | `agents`, `codex` | `~/.agents/skills/mna/` | — | -| OpenCode | `opencode` | `~/.config/opencode/skills/mna/` | — | -| Gemini CLI | `gemini-cli` | `~/.gemini/skills/mna/` | `~/.gemini/settings.json` → `mcpServers` | +| Shared agent directory | `agents`, `codex` | [`~/.agents/skills/mna/`](https://learn.chatgpt.com/docs/build-skills) | — | +| OpenCode | `opencode` | [`~/.config/opencode/skills/mna/`](https://opencode.ai/docs/skills/) | — | +| Gemini CLI | `gemini-cli` | [`~/.gemini/skills/mna/`](https://github.com/google-gemini/gemini-cli/blob/main/docs/cli/skills.md) | `~/.gemini/settings.json` → `mcpServers` | `~/.agents/skills/` is documented as a read location by Codex CLI, Gemini CLI, Cursor, OpenCode -and Windsurf, so one copy there serves several agents. **Codex CLI reads only that path** — -`~/.codex/skills/` is a third-party compatibility claim that OpenAI's own docs do not make, so -`--client codex` installs to `~/.agents/skills/`. Codex keeps MCP servers in TOML, which `mna` -does not edit. +and Windsurf, so one copy there serves several agents. Worth being precise about what that is: +five vendors independently documenting the same path, **not** a spec guarantee — the Agent Skills +standard defines what a skill contains, not where clients look for one. Claude Code does not read +it. **Codex CLI reads only that path** — `~/.codex/skills/` is a third-party compatibility claim +that OpenAI's own docs do not make, so `--client codex` installs to `~/.agents/skills/`. Codex +keeps MCP servers in TOML, which `mna` does not edit. Two caveats the CLI also prints, marked `(?)` in the plan: diff --git a/src/commands/skills/install.ts b/src/commands/skills/install.ts index d68dd6e..e202777 100644 --- a/src/commands/skills/install.ts +++ b/src/commands/skills/install.ts @@ -15,6 +15,7 @@ import { type ClientPlan, } from '../../skills/plan' import { pendingCount, renderPlans, tildify } from '../../skills/render-plan' +import { clientJsonView } from '../../skills/json-view' import { reportAndExit } from '../../util/errors' import { isInteractive } from '../../util/tty' @@ -44,16 +45,7 @@ function jsonView(plans: ClientPlan[], scope: Scope, dryRun: boolean) { dryRun, mcpServer: { name: MCP_SERVER_NAME, url: MCP_SERVER_URL }, clients: plans.map((plan) => ({ - id: plan.id, - label: plan.label, - installed: plan.installed, - skill: { - path: plan.skillPath, - state: skillState(plan), - pathVerified: plan.skillPathVerified, - }, - mcp: { path: plan.mcpPath, state: mcpState(plan) }, - mcpBlocked: plan.mcpBlocked ?? null, + ...clientJsonView(plan), changes: plan.changes.map((c) => ({ kind: c.kind, path: c.path, @@ -225,9 +217,16 @@ export const skillsInstallCommand = defineCommand({ const wrote = applied.flatMap((c) => c.applied.filter((a) => a.result !== 'unchanged')) const failures = applied.flatMap((c) => c.errors.map((e) => ({ client: c.label, ...e }))) - const succeeded = applied.filter((c) => c.applied.some((a) => a.result !== 'unchanged')) const blocked = plans.filter((p) => p.mcpBlocked) + // Report per artefact, not per client. A client whose skill files + // all failed but whose MCP entry landed has NOT had "the skill + // installed", and saying so would be a false success claim. + const didWrite = (c: AppliedClient, label: string) => + c.applied.some((a) => a.label === label && a.result !== 'unchanged') + const skillClients = applied.filter((c) => didWrite(c, 'skill')) + const mcpOnlyClients = applied.filter((c) => didWrite(c, 'mcp') && !didWrite(c, 'skill')) + if (asJson) { renderJson({ // `ok` tracks the exit code: hard write failures only. A @@ -275,9 +274,17 @@ export const skillsInstallCommand = defineCommand({ process.exit(2) } - process.stdout.write( - `\n${colors.green('✓')} Installed the ${SKILL_NAME} skill for ${succeeded.map((c) => c.label).join(', ')}.\n`, - ) + process.stdout.write('\n') + if (skillClients.length > 0) { + process.stdout.write( + `${colors.green('✓')} Installed the ${SKILL_NAME} skill for ${skillClients.map((c) => c.label).join(', ')}.\n`, + ) + } + if (mcpOnlyClients.length > 0) { + process.stdout.write( + `${colors.green('✓')} Registered the MNA MCP server for ${mcpOnlyClients.map((c) => c.label).join(', ')}.\n`, + ) + } process.stdout.write(colors.dim(' Restart the client (or reload skills) and ask it to plan a trip.\n')) if (failures.length > 0) process.exit(2) diff --git a/src/commands/skills/list.ts b/src/commands/skills/list.ts index 2ca055a..307a92c 100644 --- a/src/commands/skills/list.ts +++ b/src/commands/skills/list.ts @@ -6,6 +6,7 @@ import { colors } from '../../render/colors' import { CLIENTS } from '../../skills/clients' import { hostEnv } from '../../skills/host-env' import { mcpState, planForClients, skillState } from '../../skills/plan' +import { clientJsonView } from '../../skills/json-view' import { tildify } from '../../skills/render-plan' import { reportAndExit } from '../../util/errors' @@ -47,23 +48,7 @@ export const skillsListCommand = defineCommand({ renderJson({ scope: args.scope, authenticated: Boolean(apiKey), - clients: plans.map((plan) => ({ - id: plan.id, - label: plan.label, - installed: plan.installed, - detectedAt: plan.detectedAt, - skill: { - path: plan.skillPath, - state: skillState(plan), - pathVerified: plan.skillPathVerified, - }, - mcp: { - path: plan.mcpPath, - state: mcpState(plan), - pathVerified: plan.mcpPathVerified, - }, - mcpBlocked: plan.mcpBlocked ?? null, - })), + clients: plans.map(clientJsonView), }) return } diff --git a/src/skills/changes.test.ts b/src/skills/changes.test.ts index 2017bf0..7ce774e 100644 --- a/src/skills/changes.test.ts +++ b/src/skills/changes.test.ts @@ -2,9 +2,10 @@ import { describe, expect, test, beforeEach, afterEach } from 'bun:test' import { mkdtemp, readFile, readdir, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' -import { chmod, stat } from 'node:fs/promises' +import { chmod, mkdir, stat } from 'node:fs/promises' import { applyChange, + ApplyFailedError, BACKUP_RETENTION, backupFile, ConfigConflictError, @@ -299,3 +300,37 @@ describe('credential handling', () => { expect(backups.sort().at(-1)).toContain('20260101T000005') }) }) + +describe('applied changes are attributable', () => { + test('carries the label through so callers can tell skill from mcp writes', async () => { + const fileResult = await applyChange(await planFileChange(join(dir, 'S.md'), 'x', 'skill')) + expect(fileResult.label).toBe('skill') + const jsonResult = await applyChange( + await planJsonChange(join(dir, 'c.json'), ['mcpServers', 'mna'], {}, 'mcp'), + ) + expect(jsonResult.label).toBe('mcp') + }) + + test('unchanged results keep their label too', async () => { + const path = join(dir, 'S.md') + await writeFile(path, 'same') + expect((await applyChange(await planFileChange(path, 'same', 'skill'))).label).toBe('skill') + }) + + test('ApplyFailedError carries the backup path for the caller to surface', () => { + const err = new ApplyFailedError('/tmp/c.json', new Error('ENOSPC'), '/tmp/c.json.mna-backup-1') + expect(err.backup).toBe('/tmp/c.json.mna-backup-1') + expect(err.message).toBe('ENOSPC') + }) + + test('two backups of the same file never overwrite each other', async () => { + const path = join(dir, 'config.json') + await writeFile(path, '{"v":1}') + const first = await backupFile(path) + await writeFile(path, '{"v":2}') + const second = await backupFile(path) + expect(first).not.toBe(second) + expect(JSON.parse(await readFile(first, 'utf8'))).toEqual({ v: 1 }) + expect(JSON.parse(await readFile(second, 'utf8'))).toEqual({ v: 2 }) + }) +}) diff --git a/src/skills/changes.ts b/src/skills/changes.ts index 2c194eb..a4a8495 100644 --- a/src/skills/changes.ts +++ b/src/skills/changes.ts @@ -1,3 +1,4 @@ +import { existsSync } from 'node:fs' import { randomBytes } from 'node:crypto' import { chmod, copyFile, mkdir, readFile, readdir, rename, rm, stat, writeFile } from 'node:fs/promises' import { basename, dirname, join } from 'node:path' @@ -40,10 +41,27 @@ export type PlannedChange = FileChange | JsonChange export interface AppliedChange { path: string + /** Mirrors PlannedChange.label ("skill" | "mcp") so callers can report accurately. */ + label: string result: 'created' | 'updated' | 'unchanged' backup?: string } +/** + * A write that failed *after* we had already taken a backup. Carries the + * backup path so the user is told where their original went. + */ +export class ApplyFailedError extends Error { + constructor( + readonly path: string, + override readonly cause: unknown, + readonly backup?: string, + ) { + super(cause instanceof Error ? cause.message : String(cause)) + this.name = 'ApplyFailedError' + } +} + /** The config exists but we cannot parse it — refuse rather than replace it. */ export class UnparseableConfigError extends Error { constructor( @@ -168,8 +186,11 @@ function sameJson(a: unknown, b: unknown): boolean { export async function writeFileAtomic(path: string, content: string, mode?: number): Promise { const tmp = join(dirname(path), `.${basename(path)}.mna-tmp-${process.pid}-${randomBytes(4).toString('hex')}`) try { - await writeFile(tmp, content, 'utf8') const targetMode = mode ?? (await currentMode(path)) + // Create with the final mode rather than chmod-ing afterwards: for a + // file holding an API key, chmod-after leaves a brief window in which + // the secret is world-readable. + await writeFile(tmp, content, targetMode === undefined ? 'utf8' : { encoding: 'utf8', mode: targetMode }) if (targetMode !== undefined) await chmod(tmp, targetMode) await rename(tmp, path) } catch (err) { @@ -213,7 +234,9 @@ export async function planJsonChange( } function backupSuffix(now = new Date()): string { - return now.toISOString().replace(/[-:]/g, '').replace(/\..+$/, '') + // Millisecond precision: a single run can back up the same file twice, and + // second-granularity names collided (and so overwrote each other). + return now.toISOString().replace(/[-:.]/g, '').replace(/Z$/, '') } /** How many timestamped backups of a given file we keep around. */ @@ -238,7 +261,14 @@ async function pruneBackups(path: string, keep = BACKUP_RETENTION): Promise { - const target = `${path}.mna-backup-${backupSuffix(now)}` + const base = `${path}.mna-backup-${backupSuffix(now)}` + // Timestamps alone are not unique: a single run can back the same file up + // twice within a millisecond, and a colliding name would silently overwrite + // the earlier backup — losing exactly the thing we are trying to preserve. + let target = base + for (let n = 2; existsSync(target); n++) { + target = `${base}-${n}` + } await copyFile(path, target) await chmod(target, 0o600) await pruneBackups(path) @@ -251,15 +281,24 @@ export async function backupFile(path: string, now?: Date): Promise { */ export async function applyChange(change: PlannedChange): Promise { if (change.status === 'unchanged') { - return { path: change.path, result: 'unchanged' } + return { path: change.path, label: change.label, result: 'unchanged' } } await mkdir(dirname(change.path), { recursive: true }) if (change.kind === 'file') { const backup = change.status === 'overwrite' ? await backupFile(change.path) : undefined - await writeFileAtomic(change.path, change.content) - return { path: change.path, result: change.status === 'create' ? 'created' : 'updated', backup } + try { + await writeFileAtomic(change.path, change.content) + } catch (err) { + throw new ApplyFailedError(change.path, err, backup) + } + return { + path: change.path, + label: change.label, + result: change.status === 'create' ? 'created' : 'updated', + backup, + } } const { text, data } = await readConfigDoc(change.path) @@ -278,8 +317,12 @@ export async function applyChange(change: PlannedChange): Promise }), ) - await writeFileAtomic(change.path, next, change.secret ? 0o600 : undefined) - return { path: change.path, result: text === null ? 'created' : 'updated', backup } + try { + await writeFileAtomic(change.path, next, change.secret ? 0o600 : undefined) + } catch (err) { + throw new ApplyFailedError(change.path, err, backup) + } + return { path: change.path, label: change.label, result: text === null ? 'created' : 'updated', backup } } function buildNested(keyPath: string[], value: unknown): Record { diff --git a/src/skills/clients.test.ts b/src/skills/clients.test.ts index a49a867..a06ba30 100644 --- a/src/skills/clients.test.ts +++ b/src/skills/clients.test.ts @@ -183,3 +183,25 @@ describe('vendor-verification flags', () => { expect(findClient('not-a-client')).toBeUndefined() }) }) + +describe('every documented path cites its source', () => { + // The guard behind the README's claim that every path comes from the + // vendor's own docs. A new client cannot be added without a citation. + test('each client with a skill directory links the vendor page', () => { + for (const client of CLIENTS) { + if (!client.userSkillsDir) continue + expect({ id: client.id, cited: Boolean(client.skillPathDocs) }).toEqual({ + id: client.id, + cited: true, + }) + expect(client.skillPathDocs).toMatch(/^https:\/\//) + } + }) + + test('clients without skill support do not claim a skills citation', () => { + for (const client of CLIENTS) { + if (client.userSkillsDir) continue + expect(client.skillPathDocs).toBeUndefined() + } + }) +}) diff --git a/src/skills/clients.ts b/src/skills/clients.ts index bfea797..aebb22a 100644 --- a/src/skills/clients.ts +++ b/src/skills/clients.ts @@ -40,6 +40,13 @@ export interface ClientDefinition { mcp?: McpTarget /** Extra note surfaced in `skills list` / install output. */ note?: string + /** + * The vendor page documenting this client's user-level skill directory. + * Required for any client with a `userSkillsDir` (enforced by a test): + * we do not ship a discovery path we cannot cite. Surfaced in `--json` + * so users can check the claim themselves. + */ + skillPathDocs?: string /** * False when the skill directory follows the cross-agent convention but is * NOT documented by the vendor — i.e. we believe the client reads it, but @@ -85,6 +92,7 @@ function vscodeUserDir(env: HostEnv): string { export const CLIENTS: ClientDefinition[] = [ { id: 'claude-code', + skillPathDocs: 'https://code.claude.com/docs/en/skills', label: 'Claude Code', detectDir: '.claude', userSkillsDir: '.claude/skills', @@ -97,6 +105,7 @@ export const CLIENTS: ClientDefinition[] = [ }, { id: 'cursor', + skillPathDocs: 'https://cursor.com/docs/skills', label: 'Cursor', detectDir: '.cursor', userSkillsDir: '.cursor/skills', @@ -126,6 +135,7 @@ export const CLIENTS: ClientDefinition[] = [ }, { id: 'windsurf', + skillPathDocs: 'https://docs.devin.ai/desktop/cascade/skills', label: 'Windsurf / Devin Desktop', detectDir: '.codeium/windsurf', userSkillsDir: '.codeium/windsurf/skills', @@ -151,18 +161,21 @@ export const CLIENTS: ClientDefinition[] = [ }, { id: 'opencode', + skillPathDocs: 'https://opencode.ai/docs/skills/', label: 'OpenCode', detectDir: '.config/opencode', userSkillsDir: '.config/opencode/skills', projectSkillsDir: '.opencode/skills', }, { - // The cross-agent directory. Vendor-documented as a read location by - // Codex CLI (where it is the *only* documented user-level path), - // Gemini CLI, Cursor, OpenCode, and Windsurf. Note the Agent Skills - // spec itself defines skill *contents*, not discovery paths — this is - // a widely-adopted convention rather than a normative requirement. + // Each of these vendors documents ~/.agents/skills as a user-level read + // location in its own docs: Codex CLI (where it is the ONLY documented + // user path), Gemini CLI, Cursor, OpenCode and Windsurf. What is *not* + // standardised is discovery itself — the Agent Skills spec defines skill + // contents, not where clients look — so this is six vendor decisions + // that happen to agree, not a spec guarantee. Claude Code does not read it. id: 'agents', + skillPathDocs: 'https://learn.chatgpt.com/docs/build-skills', aliases: ['codex'], label: 'Universal agent skills (~/.agents)', detectDir: '.agents', @@ -172,6 +185,7 @@ export const CLIENTS: ClientDefinition[] = [ }, { id: 'gemini-cli', + skillPathDocs: 'https://github.com/google-gemini/gemini-cli/blob/main/docs/cli/skills.md', label: 'Gemini CLI', detectDir: '.gemini', userSkillsDir: '.gemini/skills', diff --git a/src/skills/json-view.ts b/src/skills/json-view.ts new file mode 100644 index 0000000..c75eca3 --- /dev/null +++ b/src/skills/json-view.ts @@ -0,0 +1,26 @@ +import { mcpState, skillState, type ClientPlan } from './plan' + +/** + * The per-client shape emitted by both `skills list` and `skills install`. + * Keeping one definition means the two commands cannot drift apart. + */ +export function clientJsonView(plan: ClientPlan) { + return { + id: plan.id, + label: plan.label, + installed: plan.installed, + detectedAt: plan.detectedAt, + skill: { + path: plan.skillPath, + state: skillState(plan), + pathVerified: plan.skillPathVerified, + pathDocs: plan.skillPathDocs ?? null, + }, + mcp: { + path: plan.mcpPath, + state: mcpState(plan), + pathVerified: plan.mcpPathVerified, + }, + mcpBlocked: plan.mcpBlocked ?? null, + } +} diff --git a/src/skills/plan.test.ts b/src/skills/plan.test.ts index f00f340..3c317e0 100644 --- a/src/skills/plan.test.ts +++ b/src/skills/plan.test.ts @@ -5,7 +5,8 @@ import { join } from 'node:path' import { findClient } from './clients' import type { HostEnv } from './host-env' import { SKILL_FILES } from './payload' -import { applyPlan, mcpState, planForClient, planForClients, skillState } from './plan' +import { applyPlan, mcpState, planForClient, planForClients, skillState, toApplyError } from './plan' +import { ApplyFailedError } from './changes' let fakeHome: string @@ -192,3 +193,34 @@ describe('path verification is surfaced, not hidden', () => { expect(onMac.mcpPathVerified).toBe(true) }) }) + +describe('failure reporting', () => { + test('a failed write after a backup reports where the original went', () => { + // What the install command renders as "your original is at ". + // Unreachable through the filesystem once an atomic rename is in play + // (a writable directory makes the rename succeed), so it is proved here + // with the error a full disk would produce. + const err = new ApplyFailedError( + '/home/u/.claude.json', + new Error('ENOSPC: no space left on device'), + '/home/u/.claude.json.mna-backup-20260731T070037105', + ) + expect(toApplyError('/home/u/.claude.json', err)).toEqual({ + path: '/home/u/.claude.json', + message: 'ENOSPC: no space left on device', + backup: '/home/u/.claude.json.mna-backup-20260731T070037105', + }) + }) + + test('an ordinary error reports no backup', () => { + expect(toApplyError('/p', new Error('EACCES: permission denied'))).toEqual({ + path: '/p', + message: 'EACCES: permission denied', + backup: undefined, + }) + }) + + test('a non-Error throw is still reportable', () => { + expect(toApplyError('/p', 'kaboom').message).toBe('kaboom') + }) +}) diff --git a/src/skills/plan.ts b/src/skills/plan.ts index f592865..1f8c575 100644 --- a/src/skills/plan.ts +++ b/src/skills/plan.ts @@ -1,6 +1,7 @@ import { join } from 'node:path' import { applyChange, + ApplyFailedError, ConfigConflictError, planFileChange, planJsonChange, @@ -46,6 +47,8 @@ export interface ClientPlan { note?: string /** False when the skill directory is a community convention, not vendor-documented. */ skillPathVerified: boolean + /** Vendor page documenting the skill directory. */ + skillPathDocs?: string /** False when the MCP config path is not vendor-documented on this platform. */ mcpPathVerified: boolean } @@ -98,6 +101,7 @@ export async function planForClient(client: ClientDefinition, options: PlanOptio changes: [], note: client.note, skillPathVerified: client.skillPathVerified ?? true, + skillPathDocs: client.skillPathDocs, mcpPathVerified: !client.mcpPathUnverifiedOn?.includes(env.platform), } @@ -158,6 +162,20 @@ export interface AppliedClient { errors: ApplyError[] } +/** + * Maps a thrown value onto a reportable failure. Pulled out so the + * backup-path branch — which the install command renders as "your original is + * at ", and which is effectively unreachable through the filesystem once + * an atomic rename is in play — is directly testable. + */ +export function toApplyError(path: string, err: unknown): ApplyError { + return { + path, + message: err instanceof Error ? err.message : String(err), + backup: err instanceof ApplyFailedError ? err.backup : undefined, + } +} + /** * Executes a plan. `dryRun` short-circuits before any filesystem write. A * failure on one change is recorded and the rest still run, so a single @@ -173,7 +191,7 @@ export async function applyPlan(plan: ClientPlan, dryRun: boolean): Promise