From fa7055ac5e860d6b852f83156653a12da8b09f5b Mon Sep 17 00:00:00 2001 From: Michal Date: Sat, 8 Aug 2026 11:28:15 +0100 Subject: [PATCH] fix(cli): close second review on prime-agent sync + switcher (auth, ownership, switching) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the second round of `config prime-agent` review (10 findings). auth.json (config/prime-agent.ts) — the settings.json data-loss fix had a twin: - loadPrimeAgentAuth now fails loudly on corrupt JSON instead of swallow-and- rewrite, so one syntax error can no longer destroy the provider API key and every other project's credential. hasPrimeAgentAuth shares that guarantee. - writePrimeAgentAuth writes 0600 (preserving an existing file's mode) instead of the default umask — bearer tokens are no longer world-readable on first creation. state ownership (commands/skills.ts) — the ownership model edge cases: - orphan-removal guard now normalises a canonical scope (project name, or null for globals; legacy undefined adopted to current scope) instead of comparing null against undefined, so global-only syncs and pre-PR state can no longer leave stale skills on disk forever. - a same-named skill *tracked* to a different project is preserved (with a warning) rather than silently overwritten in the shared flat tree. - mcpServers auto-attach is gated behind !isPrimeAgent with an explicit warning (prime-agent's HTTP gateway must not mutate shared mcpd project attachments); this also makes the earlier dropped-attach concern explicit rather than silent. single-active project + switcher (config/prime-agent.ts, prime-agent-extension.ts): - registerPrimeAgentMcp tags the project's entry mcpctlManaged:true and removes other mcpctl-managed entries, so prime-agent has one *active* mcpctl project while preserving untagged servers (hand-configured sre, websearch, etc). - the /mcpctl extension now reads that tag as the single source of truth for the active project, fixing the false short-circuit / no-op switch. config.ts command: - an explicit -p now updates a differing up-tree .mcpctl-project marker (scope no longer silently reverts on the next sync), no-ops when it matches, and still never scopes $HOME. - a project left with no usable credential now exits non-zero (the /mcpctl extension checks child exit status, so it no longer reports a successful switch after provisioning failed). - skills sync is treated as best-effort: settings+auth determine switch success, so a skills error no longer falsely fails the switch. - token minting now revokes prior active `prime-agent` tokens before creating a fresh one (no more never-expiring token litter / lost-credential duplication). Tests (544 green): corrupt auth.json refusal, 0600 mode, mint-failure exit code, single-active dedup preserving untagged sre, cross-project overwrite preservation, and global-orphan removal on global-only sync. --- src/cli/src/commands/config.ts | 62 +++++++++++----- src/cli/src/commands/skills.ts | 38 +++++++++- src/cli/src/config/prime-agent-extension.ts | 2 +- src/cli/src/config/prime-agent.ts | 72 ++++++++++++++----- src/cli/tests/commands/prime-agent.test.ts | 65 ++++++++++++++++- .../tests/utils/prime-agent-skills.test.ts | 42 +++++++++++ 6 files changed, 238 insertions(+), 43 deletions(-) diff --git a/src/cli/src/commands/config.ts b/src/cli/src/commands/config.ts index 586d888..864ed35 100644 --- a/src/cli/src/commands/config.ts +++ b/src/cli/src/commands/config.ts @@ -267,17 +267,33 @@ export function createConfigCommand(deps?: Partial, apiDeps?: // 2. 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. + // 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. + let provisioned = false; try { 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)) { log(`Bearer credential for '${opts.project}' already present in ${authPath}`); + provisioned = true; } else if (skillsClient) { - const tokenName = `prime-agent-${Date.now()}-${Math.floor(Math.random() * 1e6).toString(36)}`; + // 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 */ } + } + } const minted = await skillsClient.post<{ token?: string }>('/api/v1/mcptokens', { - name: tokenName, + name: 'prime-agent', projectName: opts.project, ttl: 'never', description: `mcpctl proxy MCP credential for prime-agent (${new Date().toISOString()})`, @@ -285,26 +301,36 @@ export function createConfigCommand(deps?: Partial, apiDeps?: if (typeof minted?.token === 'string' && minted.token.length > 0) { await writePrimeAgentAuth(opts.project, minted.token, authPath); log(`Minted + stored bearer credential for '${opts.project}' (mcp:${opts.project}) in ${authPath}`); + provisioned = true; } else { - log(`Warning: no token returned minting for '${opts.project}'; pass --token to supply one`); + log(`Error: no token returned minting for '${opts.project}'; pass --token to supply one`); } } else { - log('Warning: no API client available to mint a project token — pass --token to provision auth.json'); + log('Error: no API client available to mint a project token — pass --token to provision auth.json'); } } catch (err: unknown) { - log(`Warning: could not provision bearer credential for '${opts.project}': ${err instanceof Error ? err.message : String(err)}`); + log(`Error: could not provision bearer credential for '${opts.project}': ${err instanceof Error ? err.message : String(err)}`); + } + if (!provisioned) { + process.exitCode = 1; } // 3. Write the .mcpctl-project marker so later `skills sync` calls can - // resolve the project. Never clobber an existing marker found by - // walk-up, and never scope $HOME itself. + // 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. try { - const existing = await findProjectMarker(process.cwd(), homedir()); - if (existing !== null) { - log(`Project already scoped by existing marker ${existing.markerPath} ('${existing.project}'); not overwriting`); - } else if (process.cwd() !== homedir()) { - const markerPath = await writeProjectMarker(process.cwd(), opts.project); - log(`Wrote ${markerPath}`); + 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}')`); + } else { + const markerPath = await writeProjectMarker(process.cwd(), opts.project); + log(existing !== null + ? `Updated project marker ${markerPath} ('${existing.project}' → '${opts.project}')` + : `Wrote ${markerPath}`); + } } else { log('Skipped .mcpctl-project marker (running from $HOME)'); } @@ -313,6 +339,9 @@ export function createConfigCommand(deps?: Partial, apiDeps?: } // 4. Sync skills into prime-agent's skills tree (skippable). + // Best-effort: settings + auth (steps 1–2) determine whether the + // switch succeeded. A skills error is reported but must not flip the + // /mcpctl switch to "failed" when MCP access is already provisioned. if (opts.skipSkills !== true) { if (skillsClient) { try { @@ -324,13 +353,8 @@ export function createConfigCommand(deps?: Partial, apiDeps?: if (total > 0 || result.errors.length > 0) { log(`Prime-agent skills synced (${String(result.installed.length)} new, ${String(result.updated.length)} updated, ${String(result.removed.length)} removed, ${String(result.errors.length)} errors)`); } - if (result.exitCode !== 0) { - process.exitCode = result.exitCode; - log(`Warning: prime-agent skills sync exited with code ${String(result.exitCode)}`); - } } catch (err: unknown) { log(`Warning: prime-agent skills sync failed: ${err instanceof Error ? err.message : String(err)}`); - process.exitCode = 1; } } else { log('Warning: no API client available; skipping skills sync (run `mcpctl skills sync --agent prime-agent` separately)'); diff --git a/src/cli/src/commands/skills.ts b/src/cli/src/commands/skills.ts index 48afdf1..f5ac265 100644 --- a/src/cli/src/commands/skills.ts +++ b/src/cli/src/commands/skills.ts @@ -192,6 +192,10 @@ export async function runSkillsSync(opts: SyncOpts, deps: SyncDeps): Promise 0 && projectName) { + if (isPrimeAgent) { + // prime-agent talks to the gateway over HTTP; auto-attaching a + // skill's declared server deps would mutate the *shared* mcpd project + // attachments (and a /mcpctl switch would re-trigger it). Deliberately + // never attach for prime-agent, but say so instead of being silent. + if (mcpServerDeps.length > 0) { + warn(`mcpctl: skill '${v.name}' declares mcpServers but prime-agent sync does not attach project servers; skipping attach`); + } + } else if (mcpServerDeps.length > 0 && projectName) { try { const att = await attachSkillMcpServers(client, projectName, mcpServerDeps, warn); for (const srv of att.attached) { @@ -470,7 +502,7 @@ 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\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 url = settings.mcpServers[name]?.url ?? '';\n const m = url.match(/\\/projects\\/([^/]+)\\/mcp$/);\n if (m && m[1] === name) 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/**\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"; diff --git a/src/cli/src/config/prime-agent.ts b/src/cli/src/config/prime-agent.ts index 5632323..1889cb3 100644 --- a/src/cli/src/config/prime-agent.ts +++ b/src/cli/src/config/prime-agent.ts @@ -79,6 +79,8 @@ export interface RegisterMcpResult { newServer: boolean; // true if the project's MCP entry was not already present url: string; totalServers: number; + /** Previously-managed mcpctl project entries removed so `addedServer` is the sole active one. */ + removed: string[]; } /** @@ -102,51 +104,83 @@ export async function registerPrimeAgentMcp( const url = projectMcpUrl(project, gatewayUrl); const existing = settings.mcpServers[project]; const newServer = existing === undefined; - // Merge: keep any user-added fields on the project's entry (e.g. headers). - settings.mcpServers[project] = { ...(existing ?? {}), type: 'http', url }; + // Merge: keep any user-added fields on the project's entry (e.g. headers), + // and tag it so the /mcpctl switcher can find the single *active* project. + settings.mcpServers[project] = { ...(existing ?? {}), type: 'http', url, mcpctlManaged: true }; + + // 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 + // `sre`, websearch, etc) so switching never nukes unrelated integrations. + 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) { + delete settings.mcpServers[k]; + removed.push(k); + } + } const totalServers = Object.keys(settings.mcpServers).length; await mkdir(dirname(settingsPath), { recursive: true }); await writeFile(settingsPath, JSON.stringify(settings, null, 2) + '\n', 'utf-8'); - return { settingsPath, created: !existed, addedServer: project, newServer, url, totalServers }; + return { settingsPath, created: !existed, addedServer: project, newServer, url, totalServers, removed }; } /** * Ensure `mcp:` carries `{ type: "api_key", key }` in * `~/.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. */ 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 }); - // auth.json is 0600 normally; preserve an existing mode if present. - await writeFile(authPath, JSON.stringify(current, null, 2) + '\n', 'utf-8'); + // Preserve an existing 0600 mode; always 0600 on first creation. + let mode: number | undefined; + try { + const s = await stat(authPath); + mode = s.mode; + } catch { + mode = 0o600; + } + await writeFile(authPath, JSON.stringify(current, null, 2) + '\n', { mode }); } -/** Load auth.json; missing/corrupt (non-JSON) treated as a fresh file. */ +/** + * Load auth.json. + * - Missing/empty → `{}` (a brand-new file about to be created). + * - Corrupt JSON → throws, so the caller refuses to overwrite it (the same + * guarantee as settings.json — one syntax error must not destroy every + * credential, including the provider API key). + */ async function loadPrimeAgentAuth(path: string): Promise> { + let raw: string; + try { + raw = await readFile(path, 'utf-8'); + } catch (err: unknown) { + if ((err as { code?: string }).code === 'ENOENT') return {}; + throw new Error(`failed to read ${path}: ${err instanceof Error ? err.message : String(err)}`); + } + if (raw.trim().length === 0) return {}; try { - const raw = await readFile(path, 'utf-8'); - if (raw.trim().length === 0) return {}; const parsed = JSON.parse(raw) as Record; return typeof parsed === 'object' && parsed !== null ? parsed : {}; - } catch { - return {}; + } catch (err: unknown) { + throw new Error(`auth file ${path} is not valid JSON — refusing to overwrite it. Fix it and re-run (${err instanceof Error ? err.message : String(err)})`); } } -/** Does the project already have a credential in auth.json? */ +/** Does the project already have a credential in auth.json? Throws on corrupt JSON. */ export async function hasPrimeAgentAuth(project: string, authPath: string): Promise { - try { - const raw = await readFile(authPath, 'utf-8'); - const parsed = JSON.parse(raw) as Record; - const entry = parsed?.[`mcp:${project}`]; - return Boolean(entry && typeof entry === 'object' && typeof entry.key === 'string' && entry.key.length > 0); - } catch { - return false; - } + 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); } 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 1f9c7d5..ed2d993 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 } from 'node:fs'; +import { writeFileSync, readFileSync, mkdtempSync, rmSync, existsSync, statSync } from 'node:fs'; import { join } from 'node:path'; import { tmpdir, homedir } from 'node:os'; import { createConfigCommand } from '../../src/commands/config.js'; @@ -61,6 +61,7 @@ describe('config prime-agent', () => { expect(written.mcpServers['homeautomation']).toEqual({ type: 'http', url: `${DEFAULT_MCPCTL_GATEWAY_URL}/projects/homeautomation/mcp`, + mcpctlManaged: true, }); expect(output.join('\n')).toContain('homeautomation'); }); @@ -86,6 +87,7 @@ describe('config prime-agent', () => { expect(written.mcpServers['proj-1']).toEqual({ type: 'http', url: `${DEFAULT_MCPCTL_GATEWAY_URL}/projects/proj-1/mcp`, + mcpctlManaged: true, }); }); @@ -216,6 +218,45 @@ describe('config prime-agent', () => { expect(exceptionSafeRead(join(homedir(), '.mcpctl-project'))).toBeNull(); }); + it('refuses to overwrite a corrupt auth.json (and does not mint over it)', async () => { + const settingsPath = join(tmpDir, 'settings.json'); + writeFileSync(join(tmpDir, 'auth.json'), '{ not valid json'); + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'x', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); + + expect(output.join('\n')).toContain('refusing to overwrite'); + expect(readFileSync(join(tmpDir, 'auth.json'), 'utf-8')).toBe('{ not valid json'); + expect(process.exitCode).toBe(1); + }); + + it('exits non-zero when a credential cannot be provisioned', async () => { + // mockClient post returns { token: ... } by default; override to no token. + const badClient = { ...client, post: vi.fn(async () => ({})) } as typeof client; + const settingsPath = join(tmpDir, 'settings.json'); + const cmd = createConfigCommand( + { configDeps: { configDir: tmpDir }, log }, + { client: badClient, credentialsDeps: { configDir: tmpDir }, log }, + ); + await cmd.parseAsync(['prime-agent', '--project', 'x', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); + expect(process.exitCode).toBe(1); + // body of provisioning error surfaced + expect(output.join('\n')).toContain('no token returned'); + }); + + it('writes auth.json with mode 0600', 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', 'm', '-o', settingsPath, '--skip-skills', '--skip-extension'], { from: 'user' }); // mint path, mock post returns token + const mode = statSync(join(tmpDir, 'auth.json')).mode & 0o777; + expect(mode).toBe(0o600); + }); + it('refuses to overwrite a corrupt settings.json', async () => { const settingsPath = join(tmpDir, 'settings.json'); writeFileSync(settingsPath, '{ this is not valid json !!!'); @@ -235,6 +276,27 @@ describe('config prime-agent', () => { expect(readFileSync(settingsPath, 'utf-8')).toBe('{ this is not valid json !!!'); }); + it('keeps a single active mcpctl project, preserving untagged servers (sre)', async () => { + const settingsPath = join(tmpDir, 'settings.json'); + writeFileSync(settingsPath, JSON.stringify({ + mcpServers: { + sre: { type: 'http', url: `${DEFAULT_MCPCTL_GATEWAY_URL}/projects/sre/mcp` }, // untagged, hand-set + homeautomation: { type: 'http', url: `${DEFAULT_MCPCTL_GATEWAY_URL}/projects/homeautomation/mcp`, mcpctlManaged: true }, + }, + })); + // Active project is homeautomation (tagged). Switch to labctl. + 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['labctl'].mcpctlManaged).toBe(true); // new active + expect(written.mcpServers['homeautomation']).toBeUndefined(); // old managed removed + expect(written.mcpServers['sre']).toBeDefined(); // untagged preserved + }); + it('merges a re-configured project entry, preserving user-added fields', async () => { const settingsPath = join(tmpDir, 'settings.json'); writeFileSync(settingsPath, JSON.stringify({ @@ -253,6 +315,7 @@ describe('config prime-agent', () => { type: 'http', url: `${DEFAULT_MCPCTL_GATEWAY_URL}/projects/ha/mcp`, headers: { Authorization: 'Bearer u' }, // user-added field preserved + mcpctlManaged: true, }); }); }); diff --git a/src/cli/tests/utils/prime-agent-skills.test.ts b/src/cli/tests/utils/prime-agent-skills.test.ts index a6b56e5..1fcf24b 100644 --- a/src/cli/tests/utils/prime-agent-skills.test.ts +++ b/src/cli/tests/utils/prime-agent-skills.test.ts @@ -183,4 +183,46 @@ describe('runPrimeAgentSkillsSync', () => { expect(result2.removed).toContain('old-skill'); expect(existsSync(join(installRoot, 'old-skill'))).toBe(false); }); + + it('does not overwrite a same-named skill owned by a different project', async () => { + // Project A installs skill X. + 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: {} } }; + const clientA = mockClient({ visible: av, full: af }); + await runPrimeAgentSkillsSync({ project: 'projA', installRoot, statePath }, deps(clientA)); + expect(readFileSync(join(installRoot, 'x-skill', 'SKILL.md'), 'utf-8')).toBe('# version-a\n'); + + // Project B also has a skill named X with different content. + const bv = [ + { id: 'b-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:b', metadata: {}, scope: 'project' }, + ]; + const bf = { 'b-1': { id: 'b-1', name: 'x-skill', description: 'd', semver: '1.0.0', contentHash: 'sha256:b', content: '# version-b\n', files: {} } }; + const clientB = mockClient({ visible: bv, full: bf }); + const resultB = await runPrimeAgentSkillsSync({ project: 'projB', installRoot, statePath }, deps(clientB)); + + expect(resultB.preserved).toContain('x-skill'); + // A's version is untouched (not clobbered by B's). + 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 = [ + { id: 'g-1', name: 'gone-global', description: 'd', semver: '1.0.0', contentHash: 'sha256:g', metadata: {}, scope: 'global' }, + ]; + const f = { 'g-1': { id: 'g-1', name: 'gone-global', description: 'd', semver: '1.0.0', contentHash: 'sha256:g', content: '# g\n', files: {} } }; + const client1 = mockClient({ visible: v, full: f }); + const empty = join(tmpDir, 'empty2'); + mkdirSync(empty, { recursive: true }); + await runPrimeAgentSkillsSync({ cwd: empty, installRoot, statePath }, deps(client1)); + expect(existsSync(join(installRoot, 'gone-global'))).toBe(true); + + // Next global-only sync: the global is gone from the visible set. + const client2 = mockClient({ visible: [], full: {} }); + const result2 = await runPrimeAgentSkillsSync({ cwd: empty, installRoot, statePath }, deps(client2)); + expect(result2.removed).toContain('gone-global'); + expect(existsSync(join(installRoot, 'gone-global'))).toBe(false); + }); });