From 170dc06496d5d3843ffcf1f03bfe479f9acb9d5e Mon Sep 17 00:00:00 2001 From: Michal Date: Sat, 8 Aug 2026 12:01:52 +0100 Subject: [PATCH] =?UTF-8?q?fix(cli):=20close=20third=20review=20=E2=80=94?= =?UTF-8?q?=20token=20collision,=20migration,=20ownership?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Round 2 fixed the first review but introduced regressions of its own, all of which only bite against state written by the previously installed build. `config prime-agent`: - Mint each credential under a unique `prime-agent-` name again. `McpToken` is unique on (name, projectId) and revoke is a soft delete, so round 2's fixed `prime-agent` name could only ever be minted once per project — and the revoke-first ordering destroyed the working credential before discovering the mint would fail. - Provision the credential BEFORE touching settings.json. Registering the new project unmounts the previously active one, so a failed mint must not be able to leave prime-agent with no working project at all. The command now aborts with settings.json untouched. - Retire only the token this auth.json actually held, once its replacement is stored. Sweeping every `prime-agent*` token for the project would revoke the credential another install (or a custom --output run) is using; anything else that looks orphaned is reported, not deleted. - Validate a pre-existing credential instead of trusting its presence: a revoked or expired token used to short-circuit provisioning and leave prime-agent broken while the command reported success. Matched by tokenPrefix against the project's active tokens, so the secret is never sent. Fails open when the API can't be consulted. - Actually write auth.json 0600. `writeFile`'s mode is ignored for an existing file and prime-agent creates auth.json itself at 0644, so chmod after writing. - Recognise the untagged mcpServers entries older CLIs wrote (canonical proxy URL + an `mcp:` mcpctl PAT in auth.json) so a switch unmounts them instead of leaving two gateways live. Hand-configured servers have no such credential and are still preserved. Same rule in the `/mcpctl` switcher's active-project lookup. - Add `--skip-marker`, and pass it from the `/mcpctl` switcher: the extension runs from whatever directory prime-agent was started in, and was silently re-scoping that repo's `.mcpctl-project`. `skills sync --agent prime-agent`: - Record ownership from the skill's own scope, not the syncing project's. Globals were being pinned to whichever project happened to sync them, after which every other project refused to update them forever. - Never adopt legacy, ownership-less state into the current scope. Round 2 did, which deleted the other project's skills on the first sync after upgrading. Such entries are attributed to the project that last wrote the state file, and left alone when that isn't the project syncing now. - Close the overwrite-guard bypass: a sync with no project, or a global landing on a project-owned name, could still clobber and re-own a tracked skill. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_017BMXdb2qZbPSh8Q7XpTyjB --- README.md | 22 +- completions/mcpctl.bash | 4 +- completions/mcpctl.fish | 2 + src/cli/src/commands/config.ts | 178 +++++++++++--- src/cli/src/commands/skills.ts | 62 +++-- src/cli/src/config/prime-agent-extension.ts | 2 +- src/cli/src/config/prime-agent.ts | 118 +++++++-- src/cli/tests/commands/prime-agent.test.ts | 227 +++++++++++++++++- .../tests/utils/prime-agent-skills.test.ts | 95 ++++++++ 9 files changed, 629 insertions(+), 81 deletions(-) diff --git a/README.md b/README.md index 8d781b9..28d99d1 100644 --- a/README.md +++ b/README.md @@ -125,14 +125,19 @@ mcpctl config prime-agent --project monitoring This: -1. Registers the proxy MCP gateway in `~/.prime/agent/settings.json` as +1. Provisions the project's bearer credential in `~/.prime/agent/auth.json` + (`mcp:monitoring`, written 0600) — either from `--token `, an existing + entry that is still active server-side, or a freshly minted project token. + This happens first: if no credential can be provisioned the command stops + here with a non-zero exit and leaves `settings.json` alone, so the project + you are currently on keeps working. +2. Registers the proxy MCP gateway in `~/.prime/agent/settings.json` as `mcpServers.monitoring = { "type": "http", "url": "https://mcp.ad.itaz.eu/projects/monitoring/mcp" }` - (merging with any existing servers and preserving all other settings). -2. Provisions the project's bearer credential in `~/.prime/agent/auth.json` - (`mcp:monitoring`) — either from `--token `, an existing entry, or an - auto-minted project token. + (merging with any existing servers and preserving all other settings), and + unmounts the previously active mcpctl project so exactly one is live. + Servers you configured by hand are never touched. 3. Writes a `.mcpctl-project` marker (only if none exists higher up, and never - from `$HOME`) so later syncs resolve the project. + from `$HOME`) so later syncs resolve the project. Skip with `--skip-marker`. 4. Syncs the project's skills into `~/.prime/agent/skills//` as markdown skills. The shared tree is ownership-tracked per project: it never deletes another project's skills or an untracked hand-authored skill. @@ -152,8 +157,13 @@ Skip individual steps as needed: mcpctl config prime-agent --project monitoring --token mcpctl_pat_xxx # provide token, don't mint mcpctl config prime-agent --project monitoring --skip-skills # don't sync skills mcpctl config prime-agent --project monitoring --skip-extension # don't install /mcpctl switcher +mcpctl config prime-agent --project monitoring --skip-marker # don't touch .mcpctl-project here ``` +The `/mcpctl` switcher runs with `--skip-extension --skip-marker`, so switching +projects from inside prime-agent never re-scopes whichever repository +prime-agent happened to be started in. + Preview the change without writing anything: ```bash diff --git a/completions/mcpctl.bash b/completions/mcpctl.bash index 0bbaf18..0e2be01 100644 --- a/completions/mcpctl.bash +++ b/completions/mcpctl.bash @@ -125,10 +125,10 @@ _mcpctl() { COMPREPLY=($(compgen -W "-p --project -o --output --inspect --stdout --skip-skills -h --help" -- "$cur")) ;; prime-agent) - COMPREPLY=($(compgen -W "-p --project -o --output --gateway-url --token --skip-skills --skip-extension --dry-run -h --help" -- "$cur")) + COMPREPLY=($(compgen -W "-p --project -o --output --gateway-url --token --skip-skills --skip-extension --skip-marker --dry-run -h --help" -- "$cur")) ;; prime-agent-generate) - COMPREPLY=($(compgen -W "-p --project -o --output --gateway-url --token --skip-skills --skip-extension --dry-run -h --help" -- "$cur")) + COMPREPLY=($(compgen -W "-p --project -o --output --gateway-url --token --skip-skills --skip-extension --skip-marker --dry-run -h --help" -- "$cur")) ;; setup) COMPREPLY=($(compgen -W "-h --help" -- "$cur")) diff --git a/completions/mcpctl.fish b/completions/mcpctl.fish index 31867cb..325719f 100644 --- a/completions/mcpctl.fish +++ b/completions/mcpctl.fish @@ -303,6 +303,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent" -l gateway-url complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent" -l token -d 'mcpctl project bearer token to store in auth.json (skips auto-minting)' -x complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent" -l skip-skills -d 'Skip the skills sync step' complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent" -l skip-extension -d 'Do not install the /mcpctl project-switcher extension' +complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent" -l skip-marker -d 'Do not write a .mcpctl-project marker in the current directory' complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent" -l dry-run -d 'Print what would change without writing or syncing' # config prime-agent-generate options @@ -312,6 +313,7 @@ complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent-generate" -l ga complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent-generate" -l token -d 'mcpctl project bearer token to store in auth.json (skips auto-minting)' -x complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent-generate" -l skip-skills -d 'Skip the skills sync step' complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent-generate" -l skip-extension -d 'Do not install the /mcpctl project-switcher extension' +complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent-generate" -l skip-marker -d 'Do not write a .mcpctl-project marker in the current directory' complete -c mcpctl -n "__mcpctl_subcmd_active config prime-agent-generate" -l dry-run -d 'Print what would change without writing or syncing' # config impersonate options diff --git a/src/cli/src/commands/config.ts b/src/cli/src/commands/config.ts index 864ed35..140803f 100644 --- a/src/cli/src/commands/config.ts +++ b/src/cli/src/commands/config.ts @@ -17,11 +17,21 @@ import { primeAgentSettingsPath, DEFAULT_MCPCTL_GATEWAY_URL, writePrimeAgentAuth, - hasPrimeAgentAuth, + readPrimeAgentAuthKey, + mcpTokenPrefixOf, + isMcpctlToken, } from '../config/prime-agent.js'; import { MCPCTL_SWITCH_EXTENSION, MCPCTL_SWITCH_EXTENSION_FILENAME } from '../config/prime-agent-extension.js'; import { runPrimeAgentSkillsSync } from '../utils/prime-agent-skills.js'; +/** + * Name (and name prefix) of the mcptokens `config prime-agent` mints. Each mint + * gets a unique `-` name because `McpToken` is unique on + * (name, projectId) and revoke is a soft delete — a fixed name could only ever + * be minted once per project. + */ +const PRIME_AGENT_TOKEN_PREFIX = 'prime-agent'; + interface McpConfig { mcpServers: Record }>; } @@ -215,6 +225,7 @@ export function createConfigCommand(deps?: Partial, apiDeps?: .option('--token ', 'mcpctl project bearer token to store in auth.json (skips auto-minting)') .option('--skip-skills', 'Skip the skills sync step') .option('--skip-extension', 'Do not install the /mcpctl project-switcher extension') + .option('--skip-marker', 'Do not write a .mcpctl-project marker in the current directory') .option('--dry-run', 'Print what would change without writing or syncing') .action(async (opts: { project?: string; @@ -223,6 +234,7 @@ export function createConfigCommand(deps?: Partial, apiDeps?: token?: string; skipSkills?: boolean; skipExtension?: boolean; + skipMarker?: boolean; dryRun?: boolean; }) => { if (opts.project === undefined || opts.project === '') { @@ -245,55 +257,49 @@ export function createConfigCommand(deps?: Partial, apiDeps?: authPath, mcpServers: { [opts.project]: { type: 'http', url } }, extension: opts.skipExtension === true ? '' : extPath, + marker: opts.skipMarker === true ? '' : join(process.cwd(), '.mcpctl-project'), }, - action: 'write settings.json + write auth.json credential + write .mcpctl-project marker + sync skills to ~/.prime/agent/skills/', + action: 'provision auth.json credential + write settings.json + write .mcpctl-project marker + sync skills to ~/.prime/agent/skills/', }, null, 2); log(dry); return; } - // 1. Register the proxy MCP gateway (merge; never destroy settings). - try { - const reg = await registerPrimeAgentMcp(opts.project, settingsPath, opts.gatewayUrl); - log(reg.created - ? `Created ${settingsPath} and registered '${reg.addedServer}' proxy MCP (${reg.url})` - : `Registered '${reg.addedServer}' proxy MCP in ${settingsPath} (${reg.url}; ${String(reg.totalServers)} server(s) total)`); - } catch (err: unknown) { - log(`Error: failed to write ${settingsPath}: ${err instanceof Error ? err.message : String(err)}`); - process.exitCode = 1; - return; - } - - // 2. Provision the bearer credential prime-agent needs for this project. + // 1. Provision the bearer credential prime-agent needs for this project. // mcpctl's stdio bridge supplied auth implicitly; over HTTP we must // store an mcp: token in auth.json. Use --token if given, - // keep an existing one, otherwise mint it via the API. A switch with - // no usable credential is a FAILURE (exit != 0) so the /mcpctl - // extension does not report success after leaving a project bare. + // keep a still-valid existing one, otherwise mint it via the API. + // + // This runs BEFORE settings.json is touched: registering the new + // project unmounts the previously active one, so a mint failure must + // not be able to leave prime-agent with no working project at all. + // A switch with no usable credential is a FAILURE (exit != 0) so the + // /mcpctl extension does not report success over a bare project. let provisioned = false; try { + // Whatever this auth.json held before we touched it — the only token + // this run is entitled to retire once it has a replacement. + const staleKey = await readPrimeAgentAuthKey(opts.project, authPath); if (opts.token !== undefined && opts.token !== '') { await writePrimeAgentAuth(opts.project, opts.token, authPath); log(`Stored bearer credential for '${opts.project}' (mcp:${opts.project}) in ${authPath}`); provisioned = true; - } else if (await hasPrimeAgentAuth(opts.project, authPath)) { + // Only when we actually replaced something: `--token` with a fresh + // auth.json must stay entirely offline, as documented. + if (staleKey !== null) { + await retireSupersededToken(opts.project, staleKey, opts.token); + } + } else if (await hasUsableCredential(opts.project, staleKey)) { log(`Bearer credential for '${opts.project}' already present in ${authPath}`); provisioned = true; } else if (skillsClient) { - // Revoke any prior active `prime-agent` token for this project - // first (tokens are immutable + shown once), so we never litter - // never-expiring tokens on repeated reprovisioning. - const list = await skillsClient - .get | unknown>(`/api/v1/mcptokens?projectName=${encodeURIComponent(opts.project)}`) - .catch(() => []); - const existing = Array.isArray(list) ? list : []; - for (const t of existing) { - if (t.name === 'prime-agent' && t.status === 'active') { - try { await skillsClient.post(`/api/v1/mcptokens/${t.id}/revoke`); } catch { /* best-effort */ } - } - } + // Mint under a fresh, unique name. `McpToken` is unique on + // (name, projectId) and revoke is a soft delete, so reusing a fixed + // name would collide with the revoked row forever. Retire the old + // tokens only *after* the replacement is safely on disk. + const stamp = `${Date.now().toString(36)}-${Math.floor(Math.random() * 1e6).toString(36)}`; const minted = await skillsClient.post<{ token?: string }>('/api/v1/mcptokens', { - name: 'prime-agent', + name: `${PRIME_AGENT_TOKEN_PREFIX}-${stamp}`, projectName: opts.project, ttl: 'never', description: `mcpctl proxy MCP credential for prime-agent (${new Date().toISOString()})`, @@ -302,6 +308,7 @@ export function createConfigCommand(deps?: Partial, apiDeps?: await writePrimeAgentAuth(opts.project, minted.token, authPath); log(`Minted + stored bearer credential for '${opts.project}' (mcp:${opts.project}) in ${authPath}`); provisioned = true; + await retireSupersededToken(opts.project, staleKey, minted.token); } else { log(`Error: no token returned minting for '${opts.project}'; pass --token to supply one`); } @@ -313,15 +320,37 @@ export function createConfigCommand(deps?: Partial, apiDeps?: } if (!provisioned) { process.exitCode = 1; + log(`Aborted: leaving ${settingsPath} unchanged so the currently active project keeps working`); + return; + } + + // 2. Register the proxy MCP gateway (merge; never destroy settings). + try { + const reg = await registerPrimeAgentMcp(opts.project, settingsPath, opts.gatewayUrl, { authPath }); + log(reg.created + ? `Created ${settingsPath} and registered '${reg.addedServer}' proxy MCP (${reg.url})` + : `Registered '${reg.addedServer}' proxy MCP in ${settingsPath} (${reg.url}; ${String(reg.totalServers)} server(s) total)`); + if (reg.removed.length > 0) { + log(`Unmounted previously active mcpctl project(s): ${reg.removed.join(', ')}`); + } + } catch (err: unknown) { + log(`Error: failed to write ${settingsPath}: ${err instanceof Error ? err.message : String(err)}`); + process.exitCode = 1; + return; } // 3. Write the .mcpctl-project marker so later `skills sync` calls can // resolve the project. An explicit -p is authoritative: it updates a // differing up-tree marker (so the scope doesn't silently revert on // the next sync), is a no-op when it already matches, and never - // scopes $HOME itself. + // scopes $HOME itself. `--skip-marker` opts out entirely: the + // /mcpctl switcher runs this command from whatever directory + // prime-agent happens to be started in, and must not silently + // re-scope an unrelated repo that Claude Code's own sync reads. try { - if (process.cwd() !== homedir()) { + if (opts.skipMarker === true) { + log('Skipped .mcpctl-project marker (--skip-marker)'); + } else if (process.cwd() !== homedir()) { const existing = await findProjectMarker(process.cwd(), homedir()); if (existing !== null && existing.project === opts.project) { log(`Already scoped by marker ${existing.markerPath} ('${existing.project}')`); @@ -375,6 +404,87 @@ export function createConfigCommand(deps?: Partial, apiDeps?: if (hidden) { void cmd; } + + /** + * Is the credential already in auth.json still usable? + * + * A key being *present* proves nothing — a revoked or expired token would + * short-circuit provisioning and leave prime-agent silently unable to reach + * the gateway while the command reported success. mcptokens are only ever + * shown once, so we compare the stored token's 16-char `tokenPrefix` + * against the project's *active* tokens instead of sending the secret. + * + * Fails open: no client, a non-mcpctl token (a user-supplied PAT of some + * other kind), or an unreachable API all mean "keep what's there" rather + * than minting a duplicate on every run. + */ + async function hasUsableCredential(project: string, key: string | null): Promise { + if (key === null) return false; + if (!skillsClient || !isMcpctlToken(key)) return true; + const tokens = await listProjectTokens(project); + if (tokens === null) return true; // can't check → don't churn credentials + const prefix = mcpTokenPrefixOf(key); + const live = tokens.some((t) => t.status === 'active' && t.tokenPrefix === prefix); + if (!live) { + log(`Stored credential for '${project}' is no longer active — minting a replacement`); + } + return live; + } + + /** + * Retire the token this auth.json used to hold, now that `keepToken` has + * replaced it on disk. + * + * Scoped to that one credential on purpose. Sweeping every `prime-agent` + * token for the project would revoke the one a *different* auth.json is + * using — another machine, or this machine when the run targeted a custom + * `--output`. Anything else that looks orphaned is reported, not deleted: + * an unnecessary token costs nothing, a revoked one costs a broken install. + * Best-effort throughout, and only ever called once the replacement is + * safely stored. + */ + async function retireSupersededToken(project: string, staleKey: string | null, keepToken: string): Promise { + if (!skillsClient) return; + const keepPrefix = mcpTokenPrefixOf(keepToken); + const stalePrefix = staleKey !== null && isMcpctlToken(staleKey) ? mcpTokenPrefixOf(staleKey) : null; + if (stalePrefix === keepPrefix) return; + const tokens = await listProjectTokens(project); + if (tokens === null) return; + + const orphans: string[] = []; + for (const t of tokens) { + if (typeof t.id !== 'string' || t.status !== 'active') continue; + if (t.tokenPrefix === keepPrefix) continue; + // Only ever consider tokens minted for this purpose. + const name = t.name ?? ''; + if (name !== PRIME_AGENT_TOKEN_PREFIX && !name.startsWith(`${PRIME_AGENT_TOKEN_PREFIX}-`)) continue; + if (t.tokenPrefix === stalePrefix) { + try { + await skillsClient.post(`/api/v1/mcptokens/${t.id}/revoke`); + log(`Revoked the superseded '${name}' token for '${project}'`); + } catch { /* best-effort */ } + } else { + orphans.push(name); + } + } + if (orphans.length > 0) { + log(`Note: '${project}' still has other prime-agent token(s): ${orphans.join(', ')}. ` + + `They may belong to another install; remove any you don't need with \`mcpctl delete mcptoken --project ${project}\`.`); + } + } + + interface ProjectToken { id?: string; name?: string; status?: string; tokenPrefix?: string } + + /** The project's tokens, or null when the API can't be consulted. */ + async function listProjectTokens(project: string): Promise { + if (!skillsClient) return null; + try { + const list = await skillsClient.get(`/api/v1/mcptokens?projectName=${encodeURIComponent(project)}`); + return Array.isArray(list) ? list as ProjectToken[] : null; + } catch { + return null; + } + } } registerClaudeCommand('claude', false); diff --git a/src/cli/src/commands/skills.ts b/src/cli/src/commands/skills.ts index f5ac265..f0f9770 100644 --- a/src/cli/src/commands/skills.ts +++ b/src/cli/src/commands/skills.ts @@ -192,14 +192,19 @@ export async function runSkillsSync(opts: SyncOpts, deps: SyncDeps): Promise (s.scope === 'global' ? null : (projectName ?? null)); const statePath = opts.statePath ?? (isPrimeAgent ? join(homeDir, '.mcpctl', 'skills-state-prime-agent.json') : defaultStatePath()); const state = await loadState(statePath); + // Which project last wrote this state file, captured before step 7 overwrites + // it. Skills tracked by a CLI that predates ownership recording carry no + // `project` field; this is the only evidence of who installed them. + const priorSyncProject = state.lastSyncProject; const installRoot = opts.installRoot ?? (isPrimeAgent ? join(homeDir, '.prime', 'agent', 'skills') : join(homeDir, '.claude', 'skills')); @@ -243,13 +248,7 @@ export async function runSkillsSync(opts: SyncOpts, deps: SyncDeps): Promise { try { // If on-disk files were locally modified, preserve unless --force. @@ -346,15 +366,17 @@ export async function runSkillsSync(opts: SyncOpts, deps: SyncDeps): Promise {\n exec(`mcpctl ${quoted}`, { timeout: 90_000, maxBuffer: 10 * 1024 * 1024 }, (err, stdout, stderr) => {\n if (err) reject(new Error((stderr || String(err)).trim() || String(err)));\n else resolve(stdout || '');\n });\n });\n}\n\nasync function listProjects(): Promise {\n const out = await mcpctl('get', 'projects', '-o', 'json');\n const parsed = JSON.parse(out || '[]') as Array<{ name?: string; description?: string }>;\n return parsed.filter((p) => p && typeof p.name === 'string').map((p) => ({\n name: p.name as string,\n description: p.description,\n }));\n}\n\n/**\n * The single *active* mcpctl project is the mcpServers entry that carries the\n * `mcpctlManaged: true` tag (written by `config prime-agent`). Untagged entries\n * (e.g. a hand-configured `sre`, websearch) are never treated as the active\n * mcpctl project, avoiding false short-circuits.\n */\nasync function activeProject(): Promise {\n try {\n const { readFile } = await import('node:fs/promises');\n const raw = await readFile(join(AGENT_DIR, 'settings.json'), 'utf-8');\n const settings = JSON.parse(raw) as { mcpServers?: Record> };\n if (!settings.mcpServers) return null;\n for (const name of Object.keys(settings.mcpServers)) {\n const entry = settings.mcpServers[name];\n if (entry && typeof entry === 'object' && entry['mcpctlManaged'] === true) return name;\n }\n return null;\n } catch {\n return null;\n }\n}\n\nexport default function mcpctlSwitch(pi: import('@earendil-works/pi-coding-agent').ExtensionAPI) {\n pi.registerCommand('mcpctl', {\n description: 'Switch the active mcpctl project (proxy MCP + skills) and reload',\n handler: async (_args, ctx) => {\n if (!ctx.hasUI) {\n ctx.ui.notify('/mcpctl needs an interactive session', 'error');\n return;\n }\n let projects: ProjectInfo[];\n try {\n projects = await listProjects();\n } catch (err) {\n ctx.ui.notify(`mcpctl: could not list projects — ${err instanceof Error ? err.message : String(err)}`, 'error');\n return;\n }\n if (projects.length === 0) {\n ctx.ui.notify('mcpctl: no projects found (is mcpctl logged in?)', 'info');\n return;\n }\n\n const active = await activeProject();\n const items = projects.map((p) => (p.description ? `${p.name} — ${p.description}` : p.name));\n\n const picked = await ctx.ui.select(\n active ? `Switch mcpctl project (current: ${active})` : 'Switch mcpctl project',\n items,\n );\n if (!picked) return;\n\n const name = picked.split(' — ')[0]?.trim();\n if (!name) return;\n if (name === active) {\n ctx.ui.notify(`Already on mcpctl project '${name}'`, 'info');\n return;\n }\n\n ctx.ui.notify(`Switching mcpctl project to '${name}'…`, 'info');\n try {\n // Mint the project token (if needed), write settings.json + auth.json,\n // and sync skills. --skip-extension stops re-installing this very file.\n await mcpctl('config', 'prime-agent', '--project', name, '--skip-extension');\n } catch (err) {\n ctx.ui.notify(`mcpctl: switch to '${name}' failed — ${err instanceof Error ? err.message : String(err)}`, 'error');\n return;\n }\n\n await ctx.reload();\n ctx.ui.notify(`Switched to mcpctl project '${name}'.`, 'success');\n },\n });\n}\n"; +export const MCPCTL_SWITCH_EXTENSION = "/**\n * Installed by `mcpctl config prime-agent` into ~/.prime/agent/extensions/.\n * Adds a `/mcpctl` slash command to switch the active mcpctl project (proxy\n * MCP + skills) from inside prime-agent, then reloads the session.\n *\n * It shells out to the `mcpctl` CLI (same binary that wrote the config) to\n * list projects and apply the switch, then asks the running TUI to reload so\n * the new project's MCP servers, credentials and skills take effect without an\n * app restart. Keeping the logic in the CLI means this UI shell stays in\n * lock-step with the machinery in the mcpctl repo.\n */\nimport { exec } from 'node:child_process';\nimport { homedir } from 'node:os';\nimport { join } from 'node:path';\n\nconst AGENT_DIR = join(homedir(), '.prime', 'agent');\n\ninterface ProjectInfo {\n name: string;\n description?: string;\n}\n\nfunction mcpctl(...args: string[]): Promise {\n const quoted = args.map((a) => `'${String(a).replace(/'/g, \"'\\\\''\")}'`).join(' ');\n return new Promise((resolve, reject) => {\n exec(`mcpctl ${quoted}`, { timeout: 90_000, maxBuffer: 10 * 1024 * 1024 }, (err, stdout, stderr) => {\n if (err) reject(new Error((stderr || String(err)).trim() || String(err)));\n else resolve(stdout || '');\n });\n });\n}\n\nasync function listProjects(): Promise {\n const out = await mcpctl('get', 'projects', '-o', 'json');\n const parsed = JSON.parse(out || '[]') as Array<{ name?: string; description?: string }>;\n return parsed.filter((p) => p && typeof p.name === 'string').map((p) => ({\n name: p.name as string,\n description: p.description,\n }));\n}\n\n/** Projects auth.json holds an mcpctl PAT for (`mcp:`). */\nasync function credentialedProjects(): Promise> {\n const out = new Set();\n try {\n const { readFile } = await import('node:fs/promises');\n const raw = await readFile(join(AGENT_DIR, 'auth.json'), 'utf-8');\n const parsed = JSON.parse(raw) as Record;\n for (const [k, v] of Object.entries(parsed)) {\n if (!k.startsWith('mcp:')) continue;\n const key = v?.key;\n if (typeof key === 'string' && key.startsWith('mcpctl_pat_')) out.add(k.slice(4));\n }\n } catch {\n // no auth.json (or unreadable) — nothing to adopt\n }\n return out;\n}\n\n/**\n * The single *active* mcpctl project. Entries this CLI wrote carry an\n * `mcpctlManaged: true` tag; entries written by an older CLI do not, so an\n * untagged entry also counts when its URL is the canonical\n * `/projects//mcp` proxy URL *and* auth.json holds an `mcp:` mcpctl\n * PAT. A hand-configured server has no such credential and is never mistaken\n * for the active project.\n */\nasync function activeProject(): Promise {\n try {\n const { readFile } = await import('node:fs/promises');\n const raw = await readFile(join(AGENT_DIR, 'settings.json'), 'utf-8');\n const settings = JSON.parse(raw) as { mcpServers?: Record> };\n if (!settings.mcpServers) return null;\n const names = Object.keys(settings.mcpServers);\n for (const name of names) {\n const entry = settings.mcpServers[name];\n if (entry && typeof entry === 'object' && entry['mcpctlManaged'] === true) return name;\n }\n const credentialed = await credentialedProjects();\n for (const name of names) {\n const entry = settings.mcpServers[name];\n const url = entry && typeof entry === 'object' ? entry['url'] : undefined;\n if (typeof url !== 'string' || !credentialed.has(name)) continue;\n if (url.replace(/\\/+$/, '').endsWith(`/projects/${encodeURIComponent(name)}/mcp`)) return name;\n }\n return null;\n } catch {\n return null;\n }\n}\n\nexport default function mcpctlSwitch(pi: import('@earendil-works/pi-coding-agent').ExtensionAPI) {\n pi.registerCommand('mcpctl', {\n description: 'Switch the active mcpctl project (proxy MCP + skills) and reload',\n handler: async (_args, ctx) => {\n if (!ctx.hasUI) {\n ctx.ui.notify('/mcpctl needs an interactive session', 'error');\n return;\n }\n let projects: ProjectInfo[];\n try {\n projects = await listProjects();\n } catch (err) {\n ctx.ui.notify(`mcpctl: could not list projects — ${err instanceof Error ? err.message : String(err)}`, 'error');\n return;\n }\n if (projects.length === 0) {\n ctx.ui.notify('mcpctl: no projects found (is mcpctl logged in?)', 'info');\n return;\n }\n\n const active = await activeProject();\n const items = projects.map((p) => (p.description ? `${p.name} — ${p.description}` : p.name));\n\n const picked = await ctx.ui.select(\n active ? `Switch mcpctl project (current: ${active})` : 'Switch mcpctl project',\n items,\n );\n if (!picked) return;\n\n const name = picked.split(' — ')[0]?.trim();\n if (!name) return;\n if (name === active) {\n ctx.ui.notify(`Already on mcpctl project '${name}'`, 'info');\n return;\n }\n\n ctx.ui.notify(`Switching mcpctl project to '${name}'…`, 'info');\n try {\n // Mint the project token (if needed), write settings.json + auth.json,\n // and sync skills. --skip-extension stops re-installing this very file;\n // --skip-marker stops us writing a .mcpctl-project into whatever\n // directory prime-agent was launched from, which would silently\n // re-scope that repo for Claude Code's own skills sync.\n await mcpctl('config', 'prime-agent', '--project', name, '--skip-extension', '--skip-marker');\n } catch (err) {\n ctx.ui.notify(`mcpctl: switch to '${name}' failed — ${err instanceof Error ? err.message : String(err)}`, 'error');\n return;\n }\n\n await ctx.reload();\n ctx.ui.notify(`Switched to mcpctl project '${name}'.`, 'success');\n },\n });\n}\n"; diff --git a/src/cli/src/config/prime-agent.ts b/src/cli/src/config/prime-agent.ts index 1889cb3..2e4c60e 100644 --- a/src/cli/src/config/prime-agent.ts +++ b/src/cli/src/config/prime-agent.ts @@ -17,13 +17,16 @@ * - A project's existing `mcpServers` entry is merged (user-added fields are * kept), never replaced wholesale. */ -import { readFile, writeFile, mkdir, stat } from 'node:fs/promises'; +import { readFile, writeFile, mkdir, stat, chmod } from 'node:fs/promises'; import { join, dirname } from 'node:path'; import { homedir } from 'node:os'; /** Base URL of the deployed mcpctl HTTP MCP gateway. */ export const DEFAULT_MCPCTL_GATEWAY_URL = 'https://mcp.ad.itaz.eu'; +/** Every mcpctl bearer token starts with this (see `@mcpctl/shared` generateToken). */ +const MCPCTL_TOKEN_PREFIX = 'mcpctl_pat_'; + /** Resolve the prime-agent settings.json path. */ export function primeAgentSettingsPath(homeDir: string = homedir()): string { return join(homeDir, '.prime', 'agent', 'settings.json'); @@ -72,6 +75,61 @@ export async function loadPrimeAgentSettings(path: string): Promise/mcp` + * proxy URL **and** auth.json holds an `mcp:` mcpctl PAT. Older CLIs + * wrote the entry + credential pair but no tag; without adopting them a + * project switch would leave two gateways mounted at once and the `/mcpctl` + * switcher would report no active project. + * + * A hand-configured server never has an mcpctl PAT stored under `mcp:`, + * so it is never adopted — that pairing is what makes the legacy match safe. + */ +export function isMcpctlManagedEntry( + name: string, + entry: unknown, + authKeys: ReadonlySet, +): boolean { + if (entry === null || typeof entry !== 'object') return false; + const rec = entry as Record; + if (rec['mcpctlManaged'] === true) return true; + const url = rec['url']; + if (typeof url !== 'string') return false; + // Host-agnostic: adopt regardless of which gateway the old entry pointed at. + const canonical = new RegExp(`/projects/${escapeRegExp(encodeURIComponent(name))}/mcp/*$`); + return canonical.test(url) && authKeys.has(name); +} + +function escapeRegExp(s: string): string { + return s.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); +} + +/** + * Names of the projects auth.json holds an mcpctl PAT for (`mcp:`). + * Used to recognise entries an older, tag-less CLI wrote. A missing or corrupt + * auth.json yields an empty set — adoption then simply doesn't happen. + */ +export async function primeAgentAuthProjects(authPath: string): Promise> { + let parsed: Record; + try { + parsed = await loadPrimeAgentAuth(authPath); + } catch { + return new Set(); + } + const out = new Set(); + for (const [k, v] of Object.entries(parsed)) { + if (!k.startsWith('mcp:')) continue; + const key = (v as { key?: unknown } | null)?.key; + if (typeof key === 'string' && key.startsWith(MCPCTL_TOKEN_PREFIX)) out.add(k.slice(4)); + } + return out; +} + export interface RegisterMcpResult { settingsPath: string; created: boolean; // true if the settings file did not previously exist @@ -93,6 +151,7 @@ export async function registerPrimeAgentMcp( project: string, settingsPath: string, gatewayUrl: string = DEFAULT_MCPCTL_GATEWAY_URL, + opts: { authPath?: string } = {}, ): Promise { const existed = await pathExists(settingsPath); const settings = await loadPrimeAgentSettings(settingsPath); @@ -110,13 +169,16 @@ export async function registerPrimeAgentMcp( // prime-agent loads every mcpServers entry, so only ONE mcpctl project should // be active at a time. Remove any *other* mcpctl-managed project entries we - // previously installed, but preserve untagged servers (e.g. a hand-configured + // previously installed — including the untagged ones older CLIs wrote (see + // isMcpctlManagedEntry) — but preserve hand-configured servers (a bespoke // `sre`, websearch, etc) so switching never nukes unrelated integrations. + const authKeys = opts.authPath !== undefined + ? await primeAgentAuthProjects(opts.authPath) + : new Set(); const removed: string[] = []; for (const k of Object.keys(settings.mcpServers)) { if (k === project) continue; - const entry = settings.mcpServers[k]; - if (entry && typeof entry === 'object' && (entry as Record)['mcpctlManaged'] === true) { + if (isMcpctlManagedEntry(k, settings.mcpServers[k], authKeys)) { delete settings.mcpServers[k]; removed.push(k); } @@ -134,22 +196,19 @@ export async function registerPrimeAgentMcp( * `~/.prime/agent/auth.json`, merging with any existing entries (the `itaz` * provider credential, other `mcp:*` servers, etc). * - * auth.json holds bearer tokens, so it is written 0600 (preserving an existing - * file's mode if present) — never the default umask. + * auth.json holds never-expiring bearer tokens, so it always ends up 0600 — + * never the default umask. `writeFile`'s `mode` only applies when the file is + * created, and prime-agent itself creates auth.json 0644, so we chmod after + * writing rather than trusting the open flags. */ export async function writePrimeAgentAuth(project: string, key: string, authPath: string): Promise { const current = await loadPrimeAgentAuth(authPath); current[`mcp:${project}`] = { type: 'api_key', key }; await mkdir(dirname(authPath), { recursive: true }); - // Preserve an existing 0600 mode; always 0600 on first creation. - let mode: number | undefined; + await writeFile(authPath, JSON.stringify(current, null, 2) + '\n', { mode: 0o600 }); try { - const s = await stat(authPath); - mode = s.mode; - } catch { - mode = 0o600; - } - await writeFile(authPath, JSON.stringify(current, null, 2) + '\n', { mode }); + await chmod(authPath, 0o600); + } catch { /* best-effort: a credential written is better than one refused */ } } /** @@ -176,11 +235,36 @@ async function loadPrimeAgentAuth(path: string): Promise } } -/** Does the project already have a credential in auth.json? Throws on corrupt JSON. */ -export async function hasPrimeAgentAuth(project: string, authPath: string): Promise { +/** + * The credential currently stored for `project`, or null if there is none. + * Throws on corrupt JSON (the caller must refuse to overwrite the file). + */ +export async function readPrimeAgentAuthKey(project: string, authPath: string): Promise { const parsed = await loadPrimeAgentAuth(authPath) as Record; const entry = parsed[`mcp:${project}`]; - return Boolean(entry && typeof entry === 'object' && typeof entry.key === 'string' && entry.key.length > 0); + if (entry && typeof entry === 'object' && typeof entry.key === 'string' && entry.key.length > 0) { + return entry.key; + } + return null; +} + +/** Does the project already have a credential in auth.json? Throws on corrupt JSON. */ +export async function hasPrimeAgentAuth(project: string, authPath: string): Promise { + return (await readPrimeAgentAuthKey(project, authPath)) !== null; +} + +/** + * The displayable prefix mcpd records for a raw token (`tokenPrefix` on + * McpToken): the first 16 characters. Lets us match a stored credential against + * the server's token list without ever sending the secret. + */ +export function mcpTokenPrefixOf(raw: string): string { + return raw.slice(0, 16); +} + +/** Is this string shaped like an mcpctl PAT (and therefore checkable server-side)? */ +export function isMcpctlToken(raw: string): boolean { + return raw.startsWith(MCPCTL_TOKEN_PREFIX); } async function pathExists(p: string): Promise { diff --git a/src/cli/tests/commands/prime-agent.test.ts b/src/cli/tests/commands/prime-agent.test.ts index ed2d993..6688d4e 100644 --- a/src/cli/tests/commands/prime-agent.test.ts +++ b/src/cli/tests/commands/prime-agent.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { writeFileSync, readFileSync, mkdtempSync, rmSync, existsSync, statSync } from 'node:fs'; +import { writeFileSync, readFileSync, mkdtempSync, rmSync, existsSync, statSync, chmodSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir, homedir } from 'node:os'; import { createConfigCommand } from '../../src/commands/config.js'; @@ -297,6 +297,231 @@ describe('config prime-agent', () => { expect(written.mcpServers['sre']).toBeDefined(); // untagged preserved }); + it('adopts an untagged entry an older CLI wrote, keeping hand-configured ones', async () => { + // Written by a CLI that predates `mcpctlManaged`: an untagged entry whose + // URL is canonical AND a matching mcp: PAT in auth.json. + const settingsPath = join(tmpDir, 'settings.json'); + writeFileSync(settingsPath, JSON.stringify({ + mcpServers: { + legacy: { type: 'http', url: `${DEFAULT_MCPCTL_GATEWAY_URL}/projects/legacy/mcp` }, + websearch: { type: 'http', url: 'https://search.example/mcp' }, // hand-configured + sre: { type: 'http', url: `${DEFAULT_MCPCTL_GATEWAY_URL}/projects/sre/mcp` }, // canonical URL, no credential + }, + })); + writeFileSync(join(tmpDir, 'auth.json'), JSON.stringify({ + itaz: { type: 'api_key', key: 'sk-provider' }, + 'mcp:legacy': { type: 'api_key', key: 'mcpctl_pat_legacytoken1234' }, + })); + + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'labctl', '-o', settingsPath, '--skip-skills', '--skip-extension', '--token', 'mcpctl_pat_x'], { from: 'user' }); + + const written = JSON.parse(readFileSync(settingsPath, 'utf-8')); + expect(written.mcpServers['legacy']).toBeUndefined(); // adopted + unmounted + expect(written.mcpServers['websearch']).toBeDefined(); // unrelated, preserved + expect(written.mcpServers['sre']).toBeDefined(); // no PAT → hand-set, preserved + expect(written.mcpServers['labctl'].mcpctlManaged).toBe(true); + }); + + it('mints each credential under a unique name (never a fixed one)', async () => { + const settingsPath = join(tmpDir, 'settings.json'); + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'p', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); + + const body = client.post.mock.calls.find((c) => c[0] === '/api/v1/mcptokens')?.[1] as { name: string }; + // A fixed name can only ever be minted once: McpToken is unique on + // (name, projectId) and revoke is a soft delete. + expect(body.name).not.toBe('prime-agent'); + expect(body.name).toMatch(/^prime-agent-[a-z0-9-]+$/); + }); + + it('revokes the token it replaced, only after the replacement is stored', async () => { + const settingsPath = join(tmpDir, 'settings.json'); + const authPath = join(tmpDir, 'auth.json'); + writeFileSync(authPath, JSON.stringify({ + 'mcp:p': { type: 'api_key', key: 'mcpctl_pat_oldtoken00000' }, + })); + const order: string[] = []; + const api = { + get: vi.fn(async (url: string) => { + order.push(`get ${url}`); + return [ + { id: 'tok-old', name: 'prime-agent-abc', status: 'active', tokenPrefix: 'mcpctl_pat_oldto' }, + { id: 'tok-other', name: 'ci-runner', status: 'active', tokenPrefix: 'mcpctl_pat_ci000' }, + ]; + }), + post: vi.fn(async (url: string) => { + order.push(`post ${url}`); + return {}; + }), + put: vi.fn(async () => ({})), + delete: vi.fn(async () => {}), + } as unknown as ApiClient; + + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client: api, credentialsDeps: { configDir: tmpDir }, log }, + ); + // Explicitly replace the stored credential. + await cmd.parseAsync(['prime-agent', '--project', 'p', '-o', settingsPath, '--skip-skills', '--skip-extension', '--token', 'mcpctl_pat_supplied00000'], { from: 'user' }); + + // The new credential landed on disk... + expect(JSON.parse(readFileSync(authPath, 'utf-8'))['mcp:p'].key).toBe('mcpctl_pat_supplied00000'); + // ...before the token it replaced was revoked — never the other way round. + const revokeAt = order.indexOf('post /api/v1/mcptokens/tok-old/revoke'); + expect(revokeAt).toBeGreaterThanOrEqual(0); + expect(statSync(authPath).mtimeMs).toBeGreaterThan(0); + // Tokens this auth.json never held are reported, never revoked. + expect(order).not.toContain('post /api/v1/mcptokens/tok-other/revoke'); + }); + + it('never revokes a token this auth.json did not hold', async () => { + // A run against a custom --output (or a second machine) must not touch the + // credential the real install is using. + const settingsPath = join(tmpDir, 'settings.json'); + const api = { + get: vi.fn(async () => [ + { id: 'tok-elsewhere', name: 'prime-agent-abc', status: 'active', tokenPrefix: 'mcpctl_pat_elsew' }, + ]), + post: vi.fn(async (url: string) => (url === '/api/v1/mcptokens' ? { token: 'mcpctl_pat_brandnew0000' } : {})), + put: vi.fn(async () => ({})), + delete: vi.fn(async () => {}), + } as unknown as ApiClient; + + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client: api, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'p', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); + + const revokes = api.post.mock.calls.filter((c) => String(c[0]).includes('/revoke')); + expect(revokes).toEqual([]); + // ...but the user is told about it rather than left guessing. + expect(output.join('\n')).toContain('prime-agent-abc'); + }); + + it('leaves settings.json untouched when the credential cannot be provisioned', async () => { + // The active project must keep working when a switch fails: registering the + // new project unmounts the old one, so it may not run before the mint. + const settingsPath = join(tmpDir, 'settings.json'); + const before = JSON.stringify({ + mcpServers: { + homeautomation: { type: 'http', url: `${DEFAULT_MCPCTL_GATEWAY_URL}/projects/homeautomation/mcp`, mcpctlManaged: true }, + }, + }); + writeFileSync(settingsPath, before); + const badClient = { ...client, get: vi.fn(async () => []), post: vi.fn(async () => ({})) } as unknown as ApiClient; + + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client: badClient, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'labctl', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); + + expect(process.exitCode).toBe(1); + expect(readFileSync(settingsPath, 'utf-8')).toBe(before); + }); + + it('re-mints when the stored credential is no longer active', async () => { + const settingsPath = join(tmpDir, 'settings.json'); + writeFileSync(join(tmpDir, 'auth.json'), JSON.stringify({ + 'mcp:p': { type: 'api_key', key: 'mcpctl_pat_revoked000000' }, + })); + const api = { + get: vi.fn(async () => [ + { id: 'tok-1', name: 'prime-agent-old', status: 'revoked', tokenPrefix: 'mcpctl_pat_revo' }, + ]), + post: vi.fn(async () => ({ token: 'mcpctl_pat_fresh0000000' })), + put: vi.fn(async () => ({})), + delete: vi.fn(async () => {}), + } as unknown as ApiClient; + + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client: api, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'p', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); + + const auth = JSON.parse(readFileSync(join(tmpDir, 'auth.json'), 'utf-8')); + expect(auth['mcp:p'].key).toBe('mcpctl_pat_fresh0000000'); + expect(process.exitCode).toBe(0); + }); + + it('keeps a stored credential that is still active', async () => { + const settingsPath = join(tmpDir, 'settings.json'); + writeFileSync(join(tmpDir, 'auth.json'), JSON.stringify({ + 'mcp:p': { type: 'api_key', key: 'mcpctl_pat_liveaaaaaaaa' }, + })); + const api = { + get: vi.fn(async () => [ + // mcpd records the first 16 chars of the raw token as tokenPrefix. + { id: 'tok-1', name: 'prime-agent-x', status: 'active', tokenPrefix: 'mcpctl_pat_livea' }, + ]), + post: vi.fn(async () => ({ token: 'should-not-be-minted' })), + put: vi.fn(async () => ({})), + delete: vi.fn(async () => {}), + } as unknown as ApiClient; + + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client: api, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'p', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); + + expect(api.post).not.toHaveBeenCalled(); + const auth = JSON.parse(readFileSync(join(tmpDir, 'auth.json'), 'utf-8')); + expect(auth['mcp:p'].key).toBe('mcpctl_pat_liveaaaaaaaa'); + }); + + it('tightens a pre-existing 0644 auth.json to 0600', async () => { + // prime-agent creates auth.json itself with the default umask; writeFile's + // `mode` is ignored for an existing file, so the write must chmod. + const settingsPath = join(tmpDir, 'settings.json'); + const authPath = join(tmpDir, 'auth.json'); + writeFileSync(authPath, JSON.stringify({ itaz: { type: 'api_key', key: 'sk-x' } }), { mode: 0o644 }); + chmodSync(authPath, 0o644); + + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'm', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); + + expect(statSync(authPath).mode & 0o777).toBe(0o600); + // The provider credential is still there. + expect(JSON.parse(readFileSync(authPath, 'utf-8')).itaz.key).toBe('sk-x'); + }); + + it('--skip-marker leaves the current directory alone', async () => { + // The /mcpctl switcher runs from whatever directory prime-agent started in. + const settingsPath = join(tmpDir, 'settings.json'); + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'sre', '-o', settingsPath, '--skip-skills', '--skip-extension', '--skip-marker', '--token', 'mcpctl_pat_x'], { from: 'user' }); + + expect(exceptionSafeRead(join(tmpDir, '.mcpctl-project'))).toBeNull(); + }); + + it('the installed switcher extension passes --skip-marker', async () => { + const settingsPath = join(tmpDir, 'settings.json'); + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'ha', '-o', settingsPath, '--skip-skills', '--token', 'mcpctl_pat_x'], { from: 'user' }); + + const ext = readFileSync(join(tmpDir, 'extensions', 'mcpctl-switch.ts'), 'utf-8'); + expect(ext).toContain("'--skip-extension', '--skip-marker'"); + }); + it('merges a re-configured project entry, preserving user-added fields', async () => { const settingsPath = join(tmpDir, 'settings.json'); writeFileSync(settingsPath, JSON.stringify({ diff --git a/src/cli/tests/utils/prime-agent-skills.test.ts b/src/cli/tests/utils/prime-agent-skills.test.ts index 1fcf24b..284c346 100644 --- a/src/cli/tests/utils/prime-agent-skills.test.ts +++ b/src/cli/tests/utils/prime-agent-skills.test.ts @@ -207,6 +207,101 @@ describe('runPrimeAgentSkillsSync', () => { expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toBe('# version-a\n'); }); + it('does not delete legacy, ownership-less state belonging to another project', async () => { + // State written by a CLI that predates the `project` field: the skill has + // no recorded owner and the file records projA as the last syncing project. + const legacyDir = join(installRoot, 'legacy-skill'); + mkdirSync(legacyDir, { recursive: true }); + writeFileSync(join(legacyDir, 'SKILL.md'), '# legacy\n', 'utf-8'); + writeFileSync(statePath, JSON.stringify({ + schemaVersion: 1, + lastSync: '2026-01-01T00:00:00.000Z', + lastSyncProject: 'projA', + skills: { + 'legacy-skill': { + id: 'l-1', semver: '1.0.0', contentHash: 'sha256:l', scope: 'project', + installDir: legacyDir, files: {}, postInstallHash: null, + lastSyncedAt: '2026-01-01T00:00:00.000Z', + // note: no `project` field + }, + }, + }), 'utf-8'); + + // First sync after upgrading, for a *different* project. + const client = mockClient({ visible: [], full: {} }); + const result = await runPrimeAgentSkillsSync({ project: 'projB', installRoot, statePath }, deps(client)); + + expect(result.removed).toEqual([]); + expect(existsSync(legacyDir)).toBe(true); + }); + + it('cleans up legacy state once the owning project syncs again', async () => { + const legacyDir = join(installRoot, 'legacy-skill'); + mkdirSync(legacyDir, { recursive: true }); + writeFileSync(join(legacyDir, 'SKILL.md'), '# legacy\n', 'utf-8'); + writeFileSync(statePath, JSON.stringify({ + schemaVersion: 1, + lastSync: '2026-01-01T00:00:00.000Z', + lastSyncProject: 'projA', + skills: { + 'legacy-skill': { + id: 'l-1', semver: '1.0.0', contentHash: 'sha256:l', scope: 'project', + installDir: legacyDir, files: {}, postInstallHash: null, + lastSyncedAt: '2026-01-01T00:00:00.000Z', + }, + }, + }), 'utf-8'); + + const client = mockClient({ visible: [], full: {} }); + const result = await runPrimeAgentSkillsSync({ project: 'projA', installRoot, statePath }, deps(client)); + + expect(result.removed).toContain('legacy-skill'); + expect(existsSync(legacyDir)).toBe(false); + }); + + it('keeps global skills updatable after switching projects', async () => { + // A global installed while projA was active must not be pinned to projA — + // globals are visible from every project. + const gv = (hash: string) => [ + { id: 'g-1', name: 'shared-global', description: 'd', semver: '1.0.0', contentHash: hash, metadata: {}, scope: 'global' }, + ]; + const gf = (hash: string, body: string) => ({ + 'g-1': { id: 'g-1', name: 'shared-global', description: 'd', semver: '1.0.0', contentHash: hash, content: body, files: {} }, + }); + + const clientA = mockClient({ visible: gv('sha256:v1'), full: gf('sha256:v1', '# v1\n') }); + await runPrimeAgentSkillsSync({ project: 'projA', installRoot, statePath }, deps(clientA)); + expect((await loadState(statePath)).skills['shared-global']?.project).toBeNull(); + + // Switch to projB; the global has been updated server-side. + const clientB = mockClient({ visible: gv('sha256:v2'), full: gf('sha256:v2', '# v2\n') }); + const resultB = await runPrimeAgentSkillsSync({ project: 'projB', installRoot, statePath }, deps(clientB)); + + expect(resultB.updated).toContain('shared-global'); + expect(resultB.preserved).toEqual([]); + expect(readFileSync(join(installRoot, 'shared-global', 'SKILL.md'), 'utf-8')).toBe('# v2\n'); + }); + + it('does not let a global-only sync clobber a project-owned skill', async () => { + const av = [ + { id: 'a-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:a', metadata: {}, scope: 'project' }, + ]; + const af = { 'a-1': { id: 'a-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:a', content: '# version-a\n', files: {} } }; + await runPrimeAgentSkillsSync({ project: 'projA', installRoot, statePath }, deps(mockClient({ visible: av, full: af }))); + + // A global of the same name shows up on a global-only sync. + const gv = [ + { id: 'g-9', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:g', metadata: {}, scope: 'global' }, + ]; + const gf = { 'g-9': { id: 'g-9', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:g', content: '# global\n', files: {} } }; + const empty = join(tmpDir, 'empty3'); + mkdirSync(empty, { recursive: true }); + const result = await runPrimeAgentSkillsSync({ cwd: empty, installRoot, statePath }, deps(mockClient({ visible: gv, full: gf }))); + + expect(result.preserved).toContain('x-skill'); + expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toBe('# version-a\n'); + }); + it('removes global orphans on a global-only sync', async () => { // First sync a global skill. const v = [