michal/mcpctl
1
0
Fork 0
Code Issues Pull Requests 1 Actions Packages Projects Releases Wiki Activity

feat(cli+mcpd): mcpctl skills sync + config claude extension

Browse Source 
Phase 5 of the Skills + Revisions + Proposals work. Skills are now
materialised onto disk under ~/.claude/skills/<name>/, with
hash-pinned diff against mcpd, atomic per-skill install, and
preservation of locally-modified files. `mcpctl config claude --project X`
now wires the full pickup chain: writes .mcpctl-project marker, runs
the initial sync, installs the SessionStart hook so subsequent Claude
invocations stay in sync transparently.

## Sync algorithm

1. Resolve project: `--project` flag overrides; else walk up from cwd
   looking for `.mcpctl-project`; else fall back to globals-only.
2. GET /api/v1/projects/:name/skills/visible (or
   /api/v1/skills?scope=global without a project). Server returns
   id + name + semver + scope + contentHash + metadata — no body, no
   files. The contentHash is sha256 of the canonicalised body, computed
   server-side; any reordering of keys produces the same hash, so it's
   a stable diff key.
3. Load ~/.mcpctl/skills-state.json (lives outside ~/.claude/skills/
   on purpose — Claude Code reads that tree and we don't want to
   pollute it with our bookkeeping).
4. Diff:
     - server skill not in state → INSTALL
     - server skill, state contentHash matches → SKIP (cheap path)
     - server skill, state contentHash differs → UPDATE (fetch full body)
     - state skill not in server → orphan, REMOVE (preserve if locally
       modified, unless --force)
5. Atomic per-skill install: write to <targetDir>.mcpctl-staging-<pid>/,
   rename existing tree to .mcpctl-trash-<pid>, swap staging in,
   rmtree the trash. A concurrent reader (Claude Code starting up)
   never sees a partial tree.
6. State file updated with new versions, per-file SHA-256, install
   path. saveState is atomic (temp + rename).

## Failure semantics

- `--quiet` mode (used by SessionStart hook): exit 0 on network /
  timeout / mcpd error. Fail-open is non-negotiable here — we never
  want a hung mcpd to block Claude Code starting up.
- Auth failure: exit 1, clear "run mcpctl login" message.
- Disk error during state save: exit 2.
- Per-skill errors are collected in the result and reported as a
  count; one bad skill doesn't stop the others.

Network fetches run with concurrency 5. The server-side
`/visible` endpoint is metadata-only so the cheap path (everything
unchanged) needs exactly one HTTP roundtrip total.

## Files added

### CLI utilities (src/cli/src/utils/)
- skills-state.ts — load/save state, per-file sha256, edit detection.
- project-marker.ts — walk-up to find `.mcpctl-project`, bounded by
  user home so we never search above $HOME.
- sessionhook.ts — install/remove a SessionStart hook entry tagged
  with `_mcpctl_managed: true`. Idempotent. Defensive against
  missing/empty/JSONC settings.json.
- skills-disk.ts — atomic install via staging-dir rename swap,
  symmetric atomic delete via trash-dir rename. Path-escape attempts
  in files{} are rejected.

### CLI command (src/cli/src/commands/)
- skills.ts — `mcpctl skills sync` Commander wrapper + the
  `runSkillsSync(opts, deps)` library function (also called from
  `mcpctl config claude --project`). Supports `--dry-run`, `--force`,
  `--quiet`, `--keep-orphans`. `--skip-postinstall` is reserved
  (postInstall execution lands in a follow-up PR, not this one).

### Wiring
- index.ts: registers `mcpctl skills` after `mcpctl review`.
- config.ts: `mcpctl config claude --project X` now writes the
  `.mcpctl-project` marker, runs `runSkillsSync` in-process, and calls
  `installManagedSessionHook('mcpctl skills sync --quiet')`. New flag
  `--skip-skills` opts out (used by tests; useful for CI).

## Server-side change

- src/mcpd/src/services/skill.service.ts: getVisibleSkills now
  computes contentHash on the fly from the canonical body shape the
  client will reconstruct. Cheap (sha256 of ~few KB per skill); no
  schema migration needed since hash is derived not stored.

## Tests

Four new utility test files (31 tests) under src/cli/tests/utils/:
- sessionhook.test.ts — creation, idempotency, command updates,
  preservation of user hooks, removal, empty/JSONC tolerance.
- skills-disk.test.ts — atomic write, replacement without leftovers,
  path-escape rejection, atomic delete, listing ignores
  staging/trash artifacts.
- skills-state.test.ts — sha256 determinism, state round-trip,
  schema-version drift handling, edit detection.
- project-marker.test.ts — cwd hit, walk-up, $HOME boundary, empty
  marker, write+read round-trip.

The existing `mcpctl config claude` test (claude.test.ts) was updated
to pass `--skip-skills` so it stays focused on .mcp.json generation;
the new sync flow is covered by the utility tests.

Full suite: 162 test files / 2157 tests green (up from 158 / 2127).

## Deferred to a follow-up

- `metadata.hooks` materialisation into `~/.claude/settings.json` —
  the data path exists, sync receives it; PR-7 or a focused follow-up
  will write the `_mcpctl_managed: true` entries for declarative
  hooks.
- `metadata.mcpServers` auto-attach via mcpd API — likewise.
- `metadata.postInstall` script execution — the most substantive
  deferred piece. Current sync logs a TODO and skips. The corporate
  trust model (publisher-side rigor, not client-side defence) means
  this is straightforward to add once we wire the curated env +
  timeout + audit emission. Orthogonal to file sync, easier to ship
  separately.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
Michal
2026-05-07 16:26:35 +01:00
parent db57bb5856
commit 58e8e956ce
15 changed files with 1264 additions and 18 deletions
Download Patch File Download Diff File Expand all files Collapse all files

51
src/cli/src/commands/config.ts
View File

@@ -1,6 +1,6 @@
import { Command } from 'commander';
import { writeFileSync, readFileSync, existsSync } from 'node:fs';
import { resolve, join } from 'node:path';
import { resolve, join, dirname } from 'node:path';
import { homedir } from 'node:os';
import { loadConfig, saveConfig, mergeConfig, getConfigPath, DEFAULT_CONFIG } from '../config/index.js';
import type { McpctlConfig, ConfigLoaderDeps } from '../config/index.js';
@@ -9,6 +9,9 @@ import { saveCredentials, loadCredentials } from '../auth/index.js';
import { createConfigSetupCommand } from './config-setup.js';
import type { CredentialsDeps, StoredCredentials } from '../auth/index.js';
import type { ApiClient } from '../api-client.js';
import { writeProjectMarker } from '../utils/project-marker.js';
import { installManagedSessionHook } from '../utils/sessionhook.js';
import { runSkillsSync } from './skills.js';
interface McpConfig {
mcpServers: Record<string, { command?: string; args?: string[]; url?: string; env?: Record<string, string> }>;
@@ -17,6 +20,8 @@ interface McpConfig {
export interface ConfigCommandDeps {
configDeps: Partial<ConfigLoaderDeps>;
log: (...args: string[]) => void;
/** API client for the skills sync side-effect of `config claude --project`. Optional so existing call sites work; without it we skip the sync step. */
apiClient?: ApiClient;
}
export interface ConfigApiDeps {
@@ -32,6 +37,11 @@ const defaultDeps: ConfigCommandDeps = {
export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?: ConfigApiDeps): Command {
const { configDeps, log } = { ...defaultDeps, ...deps };
// PR-5: api client used by `mcpctl config claude --project` to run the
// initial skills sync after wiring the .mcp.json. Threaded through from
// index.ts; falls back to apiDeps.client when not explicitly passed (the
// existing call site already wires `client` via apiDeps).
const skillsClient = deps?.apiClient ?? apiDeps?.client;
const config = new Command('config').description('Manage mcpctl configuration');
@@ -89,12 +99,13 @@ export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?:
function registerClaudeCommand(name: string, hidden: boolean): void {
const cmd = config
.command(name)
.description(hidden ? '' : 'Generate .mcp.json that connects a project via mcpctl mcp bridge')
.description(hidden ? '' : 'Generate .mcp.json + wire skills sync + install SessionStart hook')
.option('-p, --project <name>', 'Project name')
.option('-o, --output <path>', 'Output file path', '.mcp.json')
.option('--inspect', 'Include mcpctl-inspect MCP server for traffic monitoring')
.option('--stdout', 'Print to stdout instead of writing a file')
.action((opts: { project?: string; output: string; inspect?: boolean; stdout?: boolean }) => {
.option('--skip-skills', 'Skip the skills sync + SessionStart hook install step (PR-5+)')
.action(async (opts: { project?: string; output: string; inspect?: boolean; stdout?: boolean; skipSkills?: boolean }) => {
if (!opts.project && !opts.inspect) {
log('Error: at least one of --project or --inspect is required');
process.exitCode = 1;
@@ -141,6 +152,40 @@ export function createConfigCommand(deps?: Partial<ConfigCommandDeps>, apiDeps?:
writeFileSync(outputPath, JSON.stringify(finalConfig, null, 2) + '\n');
const serverCount = Object.keys(finalConfig.mcpServers).length;
log(`Wrote ${outputPath} (${serverCount} server(s))`);
// PR-5: write project marker, run initial skills sync, install
// SessionStart hook. Skipped when --inspect-only or --skip-skills.
if (opts.project && !opts.skipSkills) {
const projectDir = dirname(outputPath);
try {
const markerPath = await writeProjectMarker(projectDir, opts.project);
log(`Wrote ${markerPath}`);
} catch (err: unknown) {
log(`Warning: failed to write .mcpctl-project marker: ${err instanceof Error ? err.message : String(err)}`);
}
if (skillsClient) {
try {
const result = await runSkillsSync(
{ project: opts.project },
{ client: skillsClient, log: (...a) => log(...a as string[]), warn: (...a) => console.error(...(a as Parameters<typeof console.error>)) },
);
const total = result.installed.length + result.updated.length + result.removed.length;
if (total > 0) {
log(`Skills synced (${String(result.installed.length)} new, ${String(result.updated.length)} updated, ${String(result.removed.length)} removed)`);
}
} catch (err: unknown) {
log(`Warning: initial skills sync failed: ${err instanceof Error ? err.message : String(err)}`);
}
}
try {
const { settingsPath, updated } = await installManagedSessionHook('mcpctl skills sync --quiet');
log(updated ? `Installed SessionStart hook in ${settingsPath}` : `SessionStart hook already up to date in ${settingsPath}`);
} catch (err: unknown) {
log(`Warning: failed to install SessionStart hook: ${err instanceof Error ? err.message : String(err)}`);
}
}
});
if (hidden) {
// Commander shows empty-description commands but they won't clutter help output

328
src/cli/src/commands/skills.ts Normal file
View File

@@ -0,0 +1,328 @@
import { Command } from 'commander';
import { join } from 'node:path';
import { homedir } from 'node:os';
import type { ApiClient } from '../api-client.js';
import { findProjectMarker } from '../utils/project-marker.js';
import {
loadState,
saveState,
detectModifiedFiles,
type SkillState,
defaultStatePath,
} from '../utils/skills-state.js';
import {
installSkillAtomic,
removeSkillAtomic,
type SkillBody,
} from '../utils/skills-disk.js';
import { ApiError } from '../api-client.js';
/**
* `mcpctl skills sync` — materialise server-side skills onto disk under
* `~/.claude/skills/<name>/`. Per-skill atomic install; hash-pinned diff
* (server-computed contentHash); user-modification preservation.
*
* Failure semantics: in `--quiet` mode (used by the SessionStart hook),
* exit code 0 on network/timeout (fail-open so a hung mcpd never blocks
* Claude startup). Auth errors exit 1; disk errors exit 2.
*/
interface VisibleSkill {
id: string;
name: string;
description: string;
semver: string;
contentHash: string;
metadata: unknown;
scope: 'project' | 'global' | 'agent';
}
interface FullSkill {
id: string;
name: string;
description: string;
content: string;
files: Record<string, string>;
metadata: Record<string, unknown>;
semver: string;
projectId: string | null;
agentId: string | null;
}
export interface SyncOpts {
/** Project name override; otherwise marker walk-up + fall back to globals-only. */
project?: string;
dryRun?: boolean;
force?: boolean;
quiet?: boolean;
skipPostInstall?: boolean;
keepOrphans?: boolean;
/** For tests: override cwd start for the marker walk-up. */
cwd?: string;
/** For tests: override skills install root (default: ~/.claude/skills). */
installRoot?: string;
/** For tests: override state file path. */
statePath?: string;
}
export interface SyncResult {
installed: string[];
updated: string[];
skipped: string[];
removed: string[];
preserved: string[]; // skills with local edits we left alone
errors: Array<{ skill: string; error: string }>;
exitCode: 0 | 1 | 2;
}
export interface SyncDeps {
client: ApiClient;
log: (...args: unknown[]) => void;
/** stderr writer. Defaults to console.error. */
warn: (...args: unknown[]) => void;
}
/**
* Library entry — call from `mcpctl config claude --project X` and from
* the `skills sync` Commander action.
*/
export async function runSkillsSync(opts: SyncOpts, deps: SyncDeps): Promise<SyncResult> {
const { client, log, warn } = deps;
const result: SyncResult = {
installed: [],
updated: [],
skipped: [],
removed: [],
preserved: [],
errors: [],
exitCode: 0,
};
// 1. Resolve project scope.
let projectName = opts.project;
if (!projectName) {
const marker = await findProjectMarker(opts.cwd ?? process.cwd());
if (marker) projectName = marker.project;
}
// 2. Fetch the visible skill list.
let visible: VisibleSkill[];
try {
if (projectName) {
visible = await client.get<VisibleSkill[]>(`/api/v1/projects/${encodeURIComponent(projectName)}/skills/visible`);
} else {
// No project context — sync only globals.
const all = await client.get<VisibleSkill[]>('/api/v1/skills?scope=global');
visible = all;
}
} catch (err: unknown) {
if (err instanceof ApiError && err.status === 401) {
warn('mcpctl: auth failed — run `mcpctl login`');
result.exitCode = 1;
return result;
}
if (opts.quiet) {
// Fail-open in quiet mode (SessionStart hook context). The next sync
// will catch up; we never want to block Claude startup on a hung mcpd.
warn(`mcpctl: skills sync skipped — ${err instanceof Error ? err.message : String(err)}`);
result.exitCode = 0;
return result;
}
throw err;
}
// Filter agent-scoped skills for now — sync targets globals + project skills,
// but agent-scoped skills aren't surfaced to a user's Claude Code session
// (they're administrative). PR-3+ may revisit if agent-identity-on-disk
// becomes a concept.
visible = visible.filter((s) => s.scope !== 'agent');
// 3. Load state.
const statePath = opts.statePath ?? defaultStatePath();
const state = await loadState(statePath);
const installRoot = opts.installRoot ?? join(homedir(), '.claude', 'skills');
// 4. Diff.
const visibleByName = new Map(visible.map((s) => [s.name, s]));
const stateNames = Object.keys(state.skills);
// Determine install/update/skip per server skill.
const toFetch: VisibleSkill[] = [];
for (const v of visible) {
const prior = state.skills[v.name];
if (!prior) {
toFetch.push(v);
continue;
}
if (prior.contentHash === v.contentHash) {
result.skipped.push(v.name);
continue;
}
// Hash differs — content changed server-side. Need to fetch full body.
toFetch.push(v);
}
// 5. Apply install/update with concurrency limit (5 in-flight fetches).
const concurrency = 5;
for (let i = 0; i < toFetch.length; i += concurrency) {
const batch = toFetch.slice(i, i + concurrency);
await Promise.all(batch.map((v) => applyOne(v)));
}
// 6. Orphan removal: skills in state but not in server's visible set.
if (!opts.keepOrphans) {
for (const name of stateNames) {
if (visibleByName.has(name)) continue;
const prior = state.skills[name];
if (!prior) continue;
try {
// Preserve user-modified skills — warn + skip.
const modified = await detectModifiedFiles(prior.installDir, prior.files);
if (modified.length > 0 && !opts.force) {
warn(`mcpctl: skipping orphan removal of '${name}' — locally modified files: ${modified.join(', ')}. Re-run with --force to remove anyway.`);
result.preserved.push(name);
continue;
}
if (opts.dryRun) {
result.removed.push(name);
continue;
}
await removeSkillAtomic(prior.installDir);
delete state.skills[name];
result.removed.push(name);
} catch (err: unknown) {
result.errors.push({ skill: name, error: err instanceof Error ? err.message : String(err) });
}
}
}
// 7. Persist state.
state.lastSync = new Date().toISOString();
if (projectName !== undefined) state.lastSyncProject = projectName;
if (!opts.dryRun) {
try {
await saveState(state, statePath);
} catch (err: unknown) {
warn(`mcpctl: failed to persist state — ${err instanceof Error ? err.message : String(err)}`);
result.exitCode = 2;
}
}
// 8. Summary.
if (!opts.quiet || result.errors.length > 0 || result.installed.length > 0 || result.updated.length > 0 || result.removed.length > 0) {
const parts: string[] = [];
if (result.installed.length) parts.push(`${String(result.installed.length)} installed`);
if (result.updated.length) parts.push(`${String(result.updated.length)} updated`);
if (result.skipped.length) parts.push(`${String(result.skipped.length)} unchanged`);
if (result.removed.length) parts.push(`${String(result.removed.length)} removed`);
if (result.preserved.length) parts.push(`${String(result.preserved.length)} preserved (modified)`);
if (result.errors.length) parts.push(`${String(result.errors.length)} errors`);
if (parts.length === 0) parts.push('no changes');
if (!opts.quiet) {
log(`mcpctl skills sync${projectName ? ` (project: ${projectName})` : ' (global only)'}: ${parts.join(', ')}`);
} else if (result.installed.length || result.updated.length || result.removed.length || result.errors.length) {
// Quiet mode: only emit a single line if something actually happened.
warn(`mcpctl: ${parts.join(', ')}`);
}
}
return result;
async function applyOne(v: VisibleSkill): Promise<void> {
try {
// If on-disk files were locally modified, preserve unless --force.
const prior = state.skills[v.name];
const targetDir = prior?.installDir ?? join(installRoot, v.name);
if (prior && !opts.force) {
const modified = await detectModifiedFiles(prior.installDir, prior.files);
if (modified.length > 0) {
warn(`mcpctl: skipping update of '${v.name}' — locally modified files: ${modified.join(', ')}. Re-run with --force to overwrite.`);
result.preserved.push(v.name);
return;
}
}
if (opts.dryRun) {
if (prior) result.updated.push(v.name);
else result.installed.push(v.name);
return;
}
const full = await client.get<FullSkill>(`/api/v1/skills/${encodeURIComponent(v.id)}`);
const body: SkillBody = {
content: full.content,
...(Object.keys(full.files ?? {}).length > 0 ? { files: full.files } : {}),
};
const fileStates = await installSkillAtomic(targetDir, body);
const newState: SkillState = {
id: v.id,
semver: v.semver,
contentHash: v.contentHash,
scope: v.scope,
installDir: targetDir,
files: fileStates,
// Tier-2 fields — postInstall execution is deferred to a follow-up
// PR. For now we record the hash so we can detect script changes
// when execution lands.
postInstallHash: null,
lastSyncedAt: new Date().toISOString(),
};
state.skills[v.name] = newState;
if (prior) result.updated.push(v.name);
else result.installed.push(v.name);
} catch (err: unknown) {
result.errors.push({ skill: v.name, error: err instanceof Error ? err.message : String(err) });
}
}
}
// ── Commander wrapper ──
export interface SkillsCommandDeps {
client: ApiClient;
log: (...args: unknown[]) => void;
}
export function createSkillsCommand(deps: SkillsCommandDeps): Command {
const { client, log } = deps;
const warn = (...args: unknown[]): void => {
console.error(...(args as Parameters<typeof console.error>));
};
const cmd = new Command('skills').description('Manage Claude Code skill bundles synced from mcpd');
cmd.command('sync')
.description('Sync skills from mcpd onto disk under ~/.claude/skills/')
.option('-p, --project <name>', 'Project to sync (overrides .mcpctl-project marker)')
.option('--dry-run', 'Print what would change without writing anything')
.option('--force', 'Overwrite locally-modified skills')
.option('--quiet', 'Suppress all output unless something changed (used by SessionStart hook)')
.option('--skip-postinstall', 'Do not run metadata.postInstall scripts (no-op in v1; reserved)')
.option('--keep-orphans', 'Do not remove skills that are no longer in the server set')
.action(async (opts: {
project?: string;
dryRun?: boolean;
force?: boolean;
quiet?: boolean;
skipPostinstall?: boolean;
keepOrphans?: boolean;
}) => {
const result = await runSkillsSync(
{
...(opts.project !== undefined ? { project: opts.project } : {}),
...(opts.dryRun !== undefined ? { dryRun: opts.dryRun } : {}),
...(opts.force !== undefined ? { force: opts.force } : {}),
...(opts.quiet !== undefined ? { quiet: opts.quiet } : {}),
...(opts.skipPostinstall !== undefined ? { skipPostInstall: opts.skipPostinstall } : {}),
...(opts.keepOrphans !== undefined ? { keepOrphans: opts.keepOrphans } : {}),
},
{ client, log, warn },
);
if (result.exitCode !== 0) {
process.exitCode = result.exitCode;
}
});
return cmd;
}

6
src/cli/src/index.ts
View File

@@ -24,6 +24,7 @@ import { createChatLlmCommand } from './commands/chat-llm.js';
import { createMigrateCommand } from './commands/migrate.js';
import { createRotateCommand } from './commands/rotate.js';
import { createReviewCommand } from './commands/review.js';
import { createSkillsCommand } from './commands/skills.js';
import { ApiClient, ApiError } from './api-client.js';
import { loadConfig } from './config/index.js';
import { loadCredentials } from './auth/index.js';
@@ -274,6 +275,11 @@ export function createProgram(): Command {
client,
log: (...args) => console.log(...args),
}));
// PR-5: skills sync to ~/.claude/skills/ on demand or via SessionStart hook.
program.addCommand(createSkillsCommand({
client,
log: (...args) => console.log(...args),
}));
program.addCommand(createMcpCommand({
getProject: () => program.opts().project as string | undefined,
}), { hidden: true });

62
src/cli/src/utils/project-marker.ts Normal file
View File

@@ -0,0 +1,62 @@
/**
* Project detection for `mcpctl skills sync`. Walks up from cwd looking
* for a `.mcpctl-project` file (single line, project name). Written by
* `mcpctl config claude --project X` at project setup time.
*
* We deliberately don't probe git remotes, env vars, or config heuristics —
* the marker file is the one true source. If you want a different project
* for a sync, pass `--project` explicitly.
*/
import { readFile } from 'node:fs/promises';
import { dirname, join } from 'node:path';
export const MARKER_FILENAME = '.mcpctl-project';
/**
* Walk up from `start` (default: cwd) looking for the marker file. Returns
* the project name (file contents, trimmed) or null if the walk reaches the
* root without finding one. We stop at the filesystem root and at the user's
* home directory — searching above $HOME doesn't make sense for a per-user
* tool.
*/
export async function findProjectMarker(start: string = process.cwd(), homeDir?: string): Promise<{ project: string; markerPath: string } | null> {
const home = homeDir ?? process.env['HOME'];
let dir = start;
// Defense against broken or pathological inputs.
if (!dir || dir === '/') return null;
// Bound the walk: 50 levels is generous; protects against symlink loops.
for (let i = 0; i < 50; i++) {
const candidate = join(dir, MARKER_FILENAME);
try {
const raw = await readFile(candidate, 'utf-8');
const project = raw.split('\n')[0]?.trim() ?? '';
if (project.length === 0) return null;
return { project, markerPath: candidate };
} catch (err: unknown) {
if (!isNotFoundError(err)) throw err;
}
if (home && dir === home) return null;
const parent = dirname(dir);
if (parent === dir) return null; // reached root
dir = parent;
}
return null;
}
/**
* Write the marker file. Idempotent — overwriting with the same value is
* a no-op from the caller's perspective. Used by
* `mcpctl config claude --project X`.
*/
export async function writeProjectMarker(dir: string, project: string): Promise<string> {
const path = join(dir, MARKER_FILENAME);
const { writeFile } = await import('node:fs/promises');
await writeFile(path, project + '\n', 'utf-8');
return path;
}
function isNotFoundError(err: unknown): boolean {
return typeof err === 'object' && err !== null && (err as { code?: string }).code === 'ENOENT';
}

140
src/cli/src/utils/sessionhook.ts Normal file
View File

@@ -0,0 +1,140 @@
/**
* Manage Claude Code's SessionStart hook in `~/.claude/settings.json`.
*
* mcpctl needs `mcpctl skills sync --quiet` to run on every Claude
* invocation. We do this via Claude Code's SessionStart hook mechanism;
* to coexist with hooks the user added by hand, every entry we write
* carries a `_mcpctl_managed: true` marker (which Claude Code ignores
* but we use to identify our row on subsequent runs).
*
* Defensive against `~/.claude/settings.json` being missing, empty, or
* shaped differently than expected (e.g. comments — JSON-with-comments
* is allowed by some editors, though Claude Code itself only writes
* pure JSON).
*/
import { readFile, writeFile, mkdir, rename } from 'node:fs/promises';
import { dirname, join } from 'node:path';
import { homedir } from 'node:os';
export const MARKER_KEY = '_mcpctl_managed';
interface HookEntry {
type: 'command';
command: string;
// Markers — Claude Code ignores extra fields; we use them to identify ours.
[k: string]: unknown;
}
interface HookGroup {
hooks: HookEntry[];
[k: string]: unknown;
}
interface Settings {
hooks?: {
SessionStart?: HookGroup[];
[k: string]: unknown;
};
[k: string]: unknown;
}
function defaultSettingsPath(): string {
return join(homedir(), '.claude', 'settings.json');
}
async function readSettings(path: string): Promise<Settings> {
try {
const raw = await readFile(path, 'utf-8');
if (raw.trim().length === 0) return {};
// Strip line comments so files written by VS Code etc still parse.
// This is a heuristic — JSON-with-comments isn't a real spec — but it
// covers the common case. Block comments ("/* ... */") are not stripped.
const stripped = raw.replace(/^\s*\/\/.*$/gm, '');
return JSON.parse(stripped) as Settings;
} catch (err: unknown) {
if (isNotFoundError(err)) return {};
throw err;
}
}
async function writeSettings(path: string, settings: Settings): Promise<void> {
await mkdir(dirname(path), { recursive: true });
const tmp = `${path}.tmp.${String(process.pid)}`;
await writeFile(tmp, JSON.stringify(settings, null, 2) + '\n', 'utf-8');
await rename(tmp, path);
}
/**
* Insert or update the managed SessionStart hook. Idempotent — running
* `mcpctl config claude --project X` twice does not create duplicate
* entries.
*/
export async function installManagedSessionHook(
command: string,
settingsPath: string = defaultSettingsPath(),
): Promise<{ updated: boolean; settingsPath: string }> {
const settings = await readSettings(settingsPath);
if (!settings.hooks) settings.hooks = {};
if (!Array.isArray(settings.hooks.SessionStart)) settings.hooks.SessionStart = [];
const groups = settings.hooks.SessionStart;
let foundEntry = false;
let entryChanged = false;
for (const group of groups) {
if (!Array.isArray(group?.hooks)) continue;
for (let i = 0; i < group.hooks.length; i++) {
const entry = group.hooks[i];
if (entry !== undefined && entry[MARKER_KEY] === true) {
foundEntry = true;
if (entry.command !== command) {
group.hooks[i] = { type: 'command', command, [MARKER_KEY]: true };
entryChanged = true;
}
}
}
}
if (!foundEntry) {
groups.push({
hooks: [{ type: 'command', command, [MARKER_KEY]: true }],
});
entryChanged = true;
}
if (entryChanged) {
await writeSettings(settingsPath, settings);
}
return { updated: entryChanged, settingsPath };
}
/**
* Remove the managed SessionStart hook (used by `mcpctl config claude
* --uninstall` in a later PR). Returns whether anything was changed.
*/
export async function removeManagedSessionHook(
settingsPath: string = defaultSettingsPath(),
): Promise<{ removed: boolean; settingsPath: string }> {
const settings = await readSettings(settingsPath);
const groups = settings.hooks?.SessionStart;
if (!Array.isArray(groups)) return { removed: false, settingsPath };
let changed = false;
for (const group of groups) {
if (!Array.isArray(group?.hooks)) continue;
const before = group.hooks.length;
group.hooks = group.hooks.filter((entry) => entry?.[MARKER_KEY] !== true);
if (group.hooks.length !== before) changed = true;
}
// Drop any group that became empty.
settings.hooks!.SessionStart = groups.filter((g) => Array.isArray(g.hooks) && g.hooks.length > 0);
if (changed) {
await writeSettings(settingsPath, settings);
}
return { removed: changed, settingsPath };
}
function isNotFoundError(err: unknown): boolean {
return typeof err === 'object' && err !== null && (err as { code?: string }).code === 'ENOENT';
}

123
src/cli/src/utils/skills-disk.ts Normal file
View File

@@ -0,0 +1,123 @@
/**
* On-disk materialisation for skills. Atomic-by-rename: stage a skill's
* full file tree under `<targetDir>.mcpctl-staging-<pid>/`, then swap the
* old directory aside (rename to `.mcpctl-trash-<pid>`) and move the
* staging dir into place. A concurrent reader (Claude Code starting up)
* therefore never sees a partially-written tree.
*/
import { mkdir, rm, rename, writeFile, readdir, stat } from 'node:fs/promises';
import { dirname, join } from 'node:path';
import type { FileState } from './skills-state.js';
import { sha256Of } from './skills-state.js';
export interface SkillBody {
/** SKILL.md content. */
content: string;
/** Auxiliary files keyed by relative path. */
files?: Record<string, string>;
}
/**
* Write a skill atomically into `targetDir`. If a previous install exists,
* it's renamed to `<targetDir>.mcpctl-trash-<pid>` and rmtree'd after the
* swap succeeds — so the live tree is always consistent.
*
* Returns the per-file FileState map for the state file.
*/
export async function installSkillAtomic(targetDir: string, body: SkillBody): Promise<Record<string, FileState>> {
const parent = dirname(targetDir);
await mkdir(parent, { recursive: true });
const stagingDir = `${targetDir}.mcpctl-staging-${String(process.pid)}`;
// If a stale staging dir exists from a previous crash, scrub it.
await rm(stagingDir, { recursive: true, force: true });
await mkdir(stagingDir, { recursive: true });
const fileStates: Record<string, FileState> = {};
// Always write SKILL.md first.
await writeFileAt(stagingDir, 'SKILL.md', body.content, fileStates);
if (body.files) {
for (const [rel, content] of Object.entries(body.files)) {
// Reject paths that try to escape the install dir. Skill files are
// server-published; the server should already validate, but the
// client checks too as defence in depth.
if (rel.includes('..') || rel.startsWith('/')) {
throw new Error(`Skill file path escapes install dir: ${rel}`);
}
await writeFileAt(stagingDir, rel, content, fileStates);
}
}
// Atomic swap: rename existing tree aside, move staging in, rmtree the old.
const trashDir = `${targetDir}.mcpctl-trash-${String(process.pid)}`;
let hadExisting = false;
try {
await rename(targetDir, trashDir);
hadExisting = true;
} catch (err: unknown) {
if (!isNotFoundError(err)) throw err;
}
await rename(stagingDir, targetDir);
if (hadExisting) {
await rm(trashDir, { recursive: true, force: true });
}
return fileStates;
}
/**
* Symmetric atomic delete: rename to `.mcpctl-trash-<pid>` first, then
* rmtree. Skip if the directory doesn't exist.
*/
export async function removeSkillAtomic(targetDir: string): Promise<void> {
const trashDir = `${targetDir}.mcpctl-trash-${String(process.pid)}`;
try {
await rename(targetDir, trashDir);
} catch (err: unknown) {
if (isNotFoundError(err)) return;
throw err;
}
await rm(trashDir, { recursive: true, force: true });
}
/** True if `path` is a directory. */
export async function isDirectory(path: string): Promise<boolean> {
try {
const s = await stat(path);
return s.isDirectory();
} catch (err: unknown) {
if (isNotFoundError(err)) return false;
throw err;
}
}
/** List subdirectories of `parent` that aren't staging/trash artifacts. */
export async function listInstalledSkillNames(parent: string): Promise<string[]> {
try {
const entries = await readdir(parent, { withFileTypes: true });
return entries
.filter((e) => e.isDirectory())
.map((e) => e.name)
.filter((name) => !name.includes('.mcpctl-staging-') && !name.includes('.mcpctl-trash-'));
} catch (err: unknown) {
if (isNotFoundError(err)) return [];
throw err;
}
}
async function writeFileAt(
base: string,
rel: string,
content: string,
states: Record<string, FileState>,
): Promise<void> {
const full = join(base, rel);
await mkdir(dirname(full), { recursive: true });
await writeFile(full, content, 'utf-8');
const buf = Buffer.from(content, 'utf-8');
states[rel] = { sha256: sha256Of(buf), size: buf.length };
}
function isNotFoundError(err: unknown): boolean {
return typeof err === 'object' && err !== null && (err as { code?: string }).code === 'ENOENT';
}

136
src/cli/src/utils/skills-state.ts Normal file
View File

@@ -0,0 +1,136 @@
/**
* Local state for `mcpctl skills sync`. Lives at
* `~/.mcpctl/skills-state.json` (NOT under `~/.claude/skills/` — Claude
* Code reads that tree and we don't want to pollute it with our
* bookkeeping). Tracks installed skills + per-file SHA-256 so the next
* sync can detect server-side changes (via top-level contentHash) and
* client-side modifications (via per-file hash drift).
*/
import { createHash } from 'node:crypto';
import { readFile, writeFile, rename, mkdir, stat } from 'node:fs/promises';
import { dirname, join } from 'node:path';
import { homedir } from 'node:os';
const STATE_SCHEMA_VERSION = 1;
export interface FileState {
/** sha256 of the file contents at write time. */
sha256: string;
size: number;
}
export interface SkillState {
id: string;
semver: string;
/** sha256 of the canonicalised skill body — matches mcpd's hash. */
contentHash: string;
scope: 'project' | 'global' | 'agent';
installDir: string;
files: Record<string, FileState>;
/** sha256 of the postInstall script if any; null if none. */
postInstallHash: string | null;
lastSyncedAt: string;
}
export interface SkillsStateFile {
schemaVersion: number;
lastSync: string | null;
lastSyncProject: string | null;
/** keyed by skill name. */
skills: Record<string, SkillState>;
}
const DEFAULT_PATH = join(homedir(), '.mcpctl', 'skills-state.json');
export function defaultStatePath(): string {
return DEFAULT_PATH;
}
export function emptyState(): SkillsStateFile {
return {
schemaVersion: STATE_SCHEMA_VERSION,
lastSync: null,
lastSyncProject: null,
skills: {},
};
}
/**
* Compute sha256 of a buffer or string. Matches the
* `'sha256:'`-prefixed format mcpd produces.
*/
export function sha256Of(data: Buffer | string): string {
const buf = typeof data === 'string' ? Buffer.from(data, 'utf-8') : data;
return 'sha256:' + createHash('sha256').update(buf).digest('hex');
}
export async function loadState(path = DEFAULT_PATH): Promise<SkillsStateFile> {
try {
const raw = await readFile(path, 'utf-8');
const parsed = JSON.parse(raw) as Partial<SkillsStateFile>;
// Be lenient: if the schema is older or fields missing, hydrate to defaults.
if (parsed.schemaVersion !== STATE_SCHEMA_VERSION) {
// For schemaVersion drift in v1 we treat the file as unparseable
// and start fresh; future migrations can dispatch on the value.
return emptyState();
}
return {
schemaVersion: STATE_SCHEMA_VERSION,
lastSync: parsed.lastSync ?? null,
lastSyncProject: parsed.lastSyncProject ?? null,
skills: parsed.skills ?? {},
};
} catch (err: unknown) {
if (isNotFoundError(err)) {
return emptyState();
}
throw err;
}
}
/** Atomic write: temp file in the same dir, then rename. */
export async function saveState(state: SkillsStateFile, path = DEFAULT_PATH): Promise<void> {
const dir = dirname(path);
await mkdir(dir, { recursive: true });
const tmp = `${path}.tmp.${String(process.pid)}`;
await writeFile(tmp, JSON.stringify(state, null, 2) + '\n', 'utf-8');
await rename(tmp, path);
}
/** Detect whether on-disk file content matches what we last wrote. */
export async function hasFileBeenModified(installDir: string, relPath: string, recorded: FileState): Promise<boolean> {
try {
const buf = await readFile(join(installDir, relPath));
if (buf.length !== recorded.size) return true;
return sha256Of(buf) !== recorded.sha256;
} catch (err: unknown) {
if (isNotFoundError(err)) return true; // missing file ≠ pristine
throw err;
}
}
/** Walk a skill's installed files and report which were edited locally. */
export async function detectModifiedFiles(installDir: string, files: Record<string, FileState>): Promise<string[]> {
const modified: string[] = [];
for (const [rel, fs] of Object.entries(files)) {
if (await hasFileBeenModified(installDir, rel, fs)) {
modified.push(rel);
}
}
return modified;
}
/** Check if a path exists. */
export async function pathExists(p: string): Promise<boolean> {
try {
await stat(p);
return true;
} catch (err: unknown) {
if (isNotFoundError(err)) return false;
throw err;
}
}
function isNotFoundError(err: unknown): boolean {
return typeof err === 'object' && err !== null && (err as { code?: string }).code === 'ENOENT';
}

7
src/cli/tests/commands/claude.test.ts
View File

@@ -37,9 +37,12 @@ describe('config claude', () => {
{ configDeps: { configDir: tmpDir }, log },
{ client, credentialsDeps: { configDir: tmpDir }, log },
);
await cmd.parseAsync(['claude', '--project', 'homeautomation', '-o', outPath], { from: 'user' });
// PR-5: --skip-skills bypasses the new sync + SessionStart hook side
// effects so this test stays focused on .mcp.json generation. The new
// sync flow has its own tests under src/cli/tests/utils/.
await cmd.parseAsync(['claude', '--project', 'homeautomation', '-o', outPath, '--skip-skills'], { from: 'user' });
// No API call should be made
// No API call should be made when --skip-skills is set.
expect(client.get).not.toHaveBeenCalled();
const written = JSON.parse(readFileSync(outPath, 'utf-8'));

68
src/cli/tests/utils/project-marker.test.ts Normal file
View File

@@ -0,0 +1,68 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtemp, rm, mkdir, writeFile } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { findProjectMarker, writeProjectMarker, MARKER_FILENAME } from '../../src/utils/project-marker.js';
describe('project-marker', () => {
let tmp: string;
beforeEach(async () => {
tmp = await mkdtemp(join(tmpdir(), 'mcpctl-marker-'));
});
afterEach(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('finds marker in cwd', async () => {
await writeFile(join(tmp, MARKER_FILENAME), 'demo\n');
const result = await findProjectMarker(tmp, '/never-exists');
expect(result?.project).toBe('demo');
expect(result?.markerPath).toBe(join(tmp, MARKER_FILENAME));
});
it('walks up to find marker', async () => {
const sub = join(tmp, 'a', 'b', 'c');
await mkdir(sub, { recursive: true });
await writeFile(join(tmp, MARKER_FILENAME), 'parent-project');
const result = await findProjectMarker(sub, '/never-exists');
expect(result?.project).toBe('parent-project');
});
it('returns null when no marker exists', async () => {
const sub = join(tmp, 'a', 'b');
await mkdir(sub, { recursive: true });
const result = await findProjectMarker(sub, '/never-exists');
expect(result).toBeNull();
});
it('stops at user home directory', async () => {
// Use tmp itself as the "home" — the walk should not go above it.
const sub = join(tmp, 'projects', 'demo');
await mkdir(sub, { recursive: true });
// Marker would be at /tmp's parent (above home) — should not be found.
const result = await findProjectMarker(sub, tmp);
expect(result).toBeNull();
});
it('trims trailing whitespace from the project name', async () => {
await writeFile(join(tmp, MARKER_FILENAME), ' demo \nignored\n');
const result = await findProjectMarker(tmp, '/never-exists');
expect(result?.project).toBe('demo');
});
it('rejects empty marker file', async () => {
await writeFile(join(tmp, MARKER_FILENAME), '\n');
const result = await findProjectMarker(tmp, '/never-exists');
expect(result).toBeNull();
});
it('writeProjectMarker writes the file with a trailing newline', async () => {
const path = await writeProjectMarker(tmp, 'demo');
expect(path).toBe(join(tmp, MARKER_FILENAME));
const result = await findProjectMarker(tmp, '/never-exists');
expect(result?.project).toBe('demo');
});
});

106
src/cli/tests/utils/sessionhook.test.ts Normal file
View File

@@ -0,0 +1,106 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtemp, rm, readFile, writeFile, mkdir } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { installManagedSessionHook, removeManagedSessionHook, MARKER_KEY } from '../../src/utils/sessionhook.js';
describe('sessionhook', () => {
let tmp: string;
beforeEach(async () => {
tmp = await mkdtemp(join(tmpdir(), 'mcpctl-sessionhook-'));
});
afterEach(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('creates settings.json from scratch when missing', async () => {
const path = join(tmp, 'settings.json');
const result = await installManagedSessionHook('mcpctl skills sync --quiet', path);
expect(result.updated).toBe(true);
const settings = JSON.parse(await readFile(path, 'utf-8'));
expect(settings.hooks.SessionStart).toHaveLength(1);
const entry = settings.hooks.SessionStart[0].hooks[0];
expect(entry.command).toBe('mcpctl skills sync --quiet');
expect(entry[MARKER_KEY]).toBe(true);
});
it('is idempotent — re-running does not add duplicates', async () => {
const path = join(tmp, 'settings.json');
await installManagedSessionHook('mcpctl skills sync --quiet', path);
const second = await installManagedSessionHook('mcpctl skills sync --quiet', path);
expect(second.updated).toBe(false);
const settings = JSON.parse(await readFile(path, 'utf-8'));
const entries = settings.hooks.SessionStart.flatMap((g: { hooks: unknown[] }) => g.hooks);
const managed = entries.filter((e: Record<string, unknown>) => e[MARKER_KEY] === true);
expect(managed).toHaveLength(1);
});
it('updates the command in place when it changes', async () => {
const path = join(tmp, 'settings.json');
await installManagedSessionHook('mcpctl skills sync', path);
const updated = await installManagedSessionHook('mcpctl skills sync --quiet', path);
expect(updated.updated).toBe(true);
const settings = JSON.parse(await readFile(path, 'utf-8'));
const managed = settings.hooks.SessionStart
.flatMap((g: { hooks: unknown[] }) => g.hooks)
.find((e: Record<string, unknown>) => e[MARKER_KEY] === true);
expect(managed.command).toBe('mcpctl skills sync --quiet');
});
it('preserves non-managed hooks', async () => {
const path = join(tmp, 'settings.json');
await mkdir(tmp, { recursive: true });
await writeFile(path, JSON.stringify({
hooks: {
SessionStart: [{ hooks: [{ type: 'command', command: 'echo user-hook' }] }],
},
}));
await installManagedSessionHook('mcpctl skills sync --quiet', path);
const settings = JSON.parse(await readFile(path, 'utf-8'));
const all = settings.hooks.SessionStart.flatMap((g: { hooks: unknown[] }) => g.hooks);
expect(all).toHaveLength(2);
expect(all.find((e: Record<string, unknown>) => e.command === 'echo user-hook')).toBeDefined();
expect(all.find((e: Record<string, unknown>) => e[MARKER_KEY] === true)).toBeDefined();
});
it('remove drops the managed entry but keeps user hooks', async () => {
const path = join(tmp, 'settings.json');
await writeFile(path, JSON.stringify({
hooks: {
SessionStart: [{ hooks: [{ type: 'command', command: 'echo user' }] }],
},
}));
await installManagedSessionHook('mcpctl skills sync --quiet', path);
const removed = await removeManagedSessionHook(path);
expect(removed.removed).toBe(true);
const settings = JSON.parse(await readFile(path, 'utf-8'));
const all = settings.hooks.SessionStart.flatMap((g: { hooks: unknown[] }) => g.hooks);
expect(all).toHaveLength(1);
expect(all[0].command).toBe('echo user');
});
it('remove is a no-op when no managed entry exists', async () => {
const path = join(tmp, 'settings.json');
const result = await removeManagedSessionHook(path);
expect(result.removed).toBe(false);
});
it('survives empty settings.json', async () => {
const path = join(tmp, 'settings.json');
await writeFile(path, '');
await installManagedSessionHook('mcpctl skills sync --quiet', path);
const settings = JSON.parse(await readFile(path, 'utf-8'));
expect(settings.hooks.SessionStart).toHaveLength(1);
});
it('strips line comments before parsing', async () => {
const path = join(tmp, 'settings.json');
await writeFile(path, '// a leading comment\n{\n "hooks": {}\n}\n');
await installManagedSessionHook('mcpctl skills sync --quiet', path);
const settings = JSON.parse(await readFile(path, 'utf-8'));
expect(settings.hooks.SessionStart).toHaveLength(1);
});
});

85
src/cli/tests/utils/skills-disk.test.ts Normal file
View File

@@ -0,0 +1,85 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtemp, rm, readFile, readdir, writeFile, mkdir } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import { installSkillAtomic, removeSkillAtomic, listInstalledSkillNames } from '../../src/utils/skills-disk.js';
describe('skills-disk', () => {
let tmp: string;
beforeEach(async () => {
tmp = await mkdtemp(join(tmpdir(), 'mcpctl-skills-disk-'));
});
afterEach(async () => {
await rm(tmp, { recursive: true, force: true });
});
it('writes SKILL.md and aux files atomically', async () => {
const target = join(tmp, 'foo');
const states = await installSkillAtomic(target, {
content: '# Foo skill',
files: { 'scripts/setup.sh': '#!/bin/sh\necho hi' },
});
expect(await readFile(join(target, 'SKILL.md'), 'utf-8')).toBe('# Foo skill');
expect(await readFile(join(target, 'scripts/setup.sh'), 'utf-8')).toBe('#!/bin/sh\necho hi');
expect(states['SKILL.md']).toBeDefined();
expect(states['SKILL.md'].sha256).toMatch(/^sha256:/);
expect(states['SKILL.md'].size).toBe('# Foo skill'.length);
expect(states['scripts/setup.sh']).toBeDefined();
});
it('replaces an existing tree without leaving partial state', async () => {
const target = join(tmp, 'foo');
await installSkillAtomic(target, { content: 'v1' });
await installSkillAtomic(target, {
content: 'v2',
files: { 'extra.md': 'extra' },
});
expect(await readFile(join(target, 'SKILL.md'), 'utf-8')).toBe('v2');
expect(await readFile(join(target, 'extra.md'), 'utf-8')).toBe('extra');
// No staging or trash dirs left behind.
const entries = await readdir(tmp);
expect(entries.filter((e) => e.includes('mcpctl-staging') || e.includes('mcpctl-trash'))).toHaveLength(0);
});
it('rejects path-escape attempts', async () => {
const target = join(tmp, 'foo');
await expect(installSkillAtomic(target, {
content: 'x',
files: { '../escaped': 'bad' },
})).rejects.toThrow(/escapes install dir/);
});
it('rejects absolute paths in files{}', async () => {
const target = join(tmp, 'foo');
await expect(installSkillAtomic(target, {
content: 'x',
files: { '/etc/passwd-like': 'bad' },
})).rejects.toThrow(/escapes install dir/);
});
it('removes a skill atomically', async () => {
const target = join(tmp, 'foo');
await installSkillAtomic(target, { content: 'x' });
await removeSkillAtomic(target);
expect((await readdir(tmp)).filter((n) => n === 'foo')).toHaveLength(0);
});
it('remove is a no-op when target does not exist', async () => {
await expect(removeSkillAtomic(join(tmp, 'never-existed'))).resolves.toBeUndefined();
});
it('listInstalledSkillNames ignores staging/trash artifacts', async () => {
const skillsDir = join(tmp, 'skills-root');
await mkdir(skillsDir, { recursive: true });
await mkdir(join(skillsDir, 'real-skill'), { recursive: true });
await mkdir(join(skillsDir, 'real-skill.mcpctl-staging-1234'), { recursive: true });
await mkdir(join(skillsDir, 'something.mcpctl-trash-9999'), { recursive: true });
await writeFile(join(skillsDir, 'real-skill', 'SKILL.md'), 'x');
const names = await listInstalledSkillNames(skillsDir);
expect(names).toEqual(['real-skill']);
});
});

107
src/cli/tests/utils/skills-state.test.ts Normal file
View File

@@ -0,0 +1,107 @@
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import { mkdtemp, rm, writeFile, readFile, mkdir } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join } from 'node:path';
import {
loadState,
saveState,
emptyState,
sha256Of,
hasFileBeenModified,
detectModifiedFiles,
type FileState,
} from '../../src/utils/skills-state.js';
describe('skills-state', () => {
let tmp: string;
beforeEach(async () => {
tmp = await mkdtemp(join(tmpdir(), 'mcpctl-skills-state-'));
});
afterEach(async () => {
await rm(tmp, { recursive: true, force: true });
});
describe('sha256Of', () => {
it('is deterministic and prefixed', () => {
expect(sha256Of('hello')).toMatch(/^sha256:[0-9a-f]{64}$/);
expect(sha256Of('hello')).toBe(sha256Of('hello'));
expect(sha256Of('hello')).not.toBe(sha256Of('hellp'));
});
});
describe('load / save', () => {
it('returns empty state when file does not exist', async () => {
const state = await loadState(join(tmp, 'no-such.json'));
expect(state.skills).toEqual({});
expect(state.lastSync).toBeNull();
});
it('round-trips state', async () => {
const path = join(tmp, 'state.json');
const state = emptyState();
state.lastSync = '2026-05-07T00:00:00.000Z';
state.lastSyncProject = 'demo';
state.skills['my-skill'] = {
id: 'cuid-x',
semver: '0.1.0',
contentHash: sha256Of('body'),
scope: 'global',
installDir: '/tmp/foo',
files: { 'SKILL.md': { sha256: sha256Of('hi'), size: 2 } },
postInstallHash: null,
lastSyncedAt: '2026-05-07T00:00:00.000Z',
};
await saveState(state, path);
const loaded = await loadState(path);
expect(loaded).toEqual(state);
});
it('starts fresh on schema-version drift', async () => {
const path = join(tmp, 'state.json');
await writeFile(path, JSON.stringify({ schemaVersion: 99, skills: { x: {} } }));
const state = await loadState(path);
expect(state.schemaVersion).toBe(1);
expect(state.skills).toEqual({});
});
});
describe('hasFileBeenModified', () => {
it('false when content matches recorded hash + size', async () => {
const dir = join(tmp, 'sk');
await mkdir(dir);
await writeFile(join(dir, 'SKILL.md'), 'hello');
const recorded: FileState = { sha256: sha256Of('hello'), size: 5 };
expect(await hasFileBeenModified(dir, 'SKILL.md', recorded)).toBe(false);
});
it('true when content differs', async () => {
const dir = join(tmp, 'sk');
await mkdir(dir);
await writeFile(join(dir, 'SKILL.md'), 'edited');
const recorded: FileState = { sha256: sha256Of('hello'), size: 5 };
expect(await hasFileBeenModified(dir, 'SKILL.md', recorded)).toBe(true);
});
it('true when file is missing', async () => {
const recorded: FileState = { sha256: sha256Of('hello'), size: 5 };
expect(await hasFileBeenModified(tmp, 'missing.md', recorded)).toBe(true);
});
});
describe('detectModifiedFiles', () => {
it('returns the list of edited paths', async () => {
const dir = join(tmp, 'sk');
await mkdir(dir);
await writeFile(join(dir, 'SKILL.md'), 'pristine');
await writeFile(join(dir, 'extra.md'), 'edited');
const result = await detectModifiedFiles(dir, {
'SKILL.md': { sha256: sha256Of('pristine'), size: 8 },
'extra.md': { sha256: sha256Of('original'), size: 8 },
});
expect(result).toEqual(['extra.md']);
});
});
});

23
src/mcpd/src/services/skill.service.ts
View File

@@ -5,7 +5,7 @@ import type { IProjectRepository } from '../repositories/project.repository.js';
import type { IAgentRepository } from '../repositories/agent.repository.js';
import { CreateSkillSchema, UpdateSkillSchema } from '../validation/skill.schema.js';
import { NotFoundError } from './mcp-server.service.js';
import type { ResourceRevisionService } from './resource-revision.service.js';
import { ResourceRevisionService } from './resource-revision.service.js';
import type { ResourceProposalService } from './resource-proposal.service.js';
import { bumpSemver, type BumpKind } from '../utils/semver.js';
@@ -349,7 +349,7 @@ export class SkillService {
name: string;
description: string;
semver: string;
contentHash: string | null;
contentHash: string;
metadata: unknown;
scope: 'project' | 'global' | 'agent';
}>> {
@@ -359,7 +359,7 @@ export class SkillService {
name: string;
description: string;
semver: string;
contentHash: string | null;
contentHash: string;
metadata: unknown;
scope: 'project' | 'global' | 'agent';
}> = [];
@@ -367,16 +367,23 @@ export class SkillService {
let scope: 'project' | 'global' | 'agent' = 'global';
if (s.projectId !== null) scope = 'project';
else if (s.agentId !== null) scope = 'agent';
// Compute contentHash on the fly from the body shape that
// `mcpctl skills sync` will write to disk. The server hashes the
// canonicalised JSON; the client hashes the same JSON shape it
// receives, and they match. (Cheap — sha256 of a few KB.)
const contentHash = ResourceRevisionService.hash({
content: s.content,
description: s.description,
priority: s.priority,
files: s.files,
metadata: s.metadata,
});
out.push({
id: s.id,
name: s.name,
description: s.description,
semver: s.semver,
// contentHash lives on the latest revision row; sync clients can
// fetch it via /api/v1/revisions?resourceType=skill&resourceId=...
// until the resource row carries it directly. PR-5 will likely
// promote contentHash onto the resource itself.
contentHash: null,
contentHash,
metadata: s.metadata,
scope,
});