diff --git a/src/mcpd/src/services/agent.service.ts b/src/mcpd/src/services/agent.service.ts index caadcf2..601d0dc 100644 --- a/src/mcpd/src/services/agent.service.ts +++ b/src/mcpd/src/services/agent.service.ts @@ -228,8 +228,18 @@ export class AgentService { const out: AgentView[] = []; for (const a of inputs) { const llm = await this.llms.getByName(a.llmName); + // v6: auto-create the project if it doesn't exist. Publishers + // commonly want their virtual agents pinned to a team-scoped + // project; pre-v6 they had to ask an admin to create the + // project first. ensureByName is idempotent — the second + // publisher landing on the same name reuses the existing row. + // Owner is the publishing user (first writer wins). const projectId = a.project !== undefined - ? (await this.projects.resolveAndGet(a.project)).id + ? (await this.projects.ensureByName( + a.project, + ownerId, + { auditNote: `Auto-created by virtual-agent registrar for '${a.name}'` }, + )).id : null; const existing = await this.repo.findByName(a.name); if (existing !== null) { diff --git a/src/mcpd/src/services/project.service.ts b/src/mcpd/src/services/project.service.ts index 38f49be..712d4f3 100644 --- a/src/mcpd/src/services/project.service.ts +++ b/src/mcpd/src/services/project.service.ts @@ -35,6 +35,41 @@ export class ProjectService { throw new NotFoundError(`Project not found: ${idOrName}`); } + /** + * v6: idempotent get-or-create by name. Used by virtual-agent + * registration so a publisher can declare `project: "my-team"` in + * their mcplocal config and have mcpd auto-create the project on + * first publish, with sensible defaults (description carries an + * audit trail of who created it; proxyModel + gated take their + * schema defaults). Subsequent registers from the same or other + * publishers reuse the existing project — only the publishing user + * who wins the create race owns the row. + * + * Caller passes `ownerId` so the project is owned by whoever first + * triggered the auto-create. Returns the resolved row whether it + * was found or just created. + */ + async ensureByName(name: string, ownerId: string, opts: { auditNote?: string } = {}): Promise { + const byName = await this.projectRepo.findByName(name); + if (byName !== null) return byName; + // Validate the name client-side to surface a clean error before + // we touch the DB. Same regex CreateProjectSchema enforces. + if (!/^[a-z0-9-]+$/.test(name) || name.length > 100) { + throw new Error( + `Cannot auto-create project '${name}' — name must be lowercase alphanumeric with hyphens (max 100 chars)`, + ); + } + const description = opts.auditNote ?? 'Auto-created by mcplocal virtual-agent registrar'; + return this.projectRepo.create({ + name, + description, + prompt: '', + ownerId, + proxyModel: '', + gated: true, + }); + } + async create(input: unknown, ownerId: string): Promise { const data = CreateProjectSchema.parse(input); diff --git a/src/mcpd/tests/virtual-agent-service.test.ts b/src/mcpd/tests/virtual-agent-service.test.ts index d5c8989..282f3e0 100644 --- a/src/mcpd/tests/virtual-agent-service.test.ts +++ b/src/mcpd/tests/virtual-agent-service.test.ts @@ -140,14 +140,40 @@ function mockLlms(): LlmService { } as unknown as LlmService; } -function mockProjects(): ProjectService { - return { - getById: vi.fn(async (id: string) => ({ id, name: 'mcpctl-dev' })), - resolveAndGet: vi.fn(async (idOrName: string) => ({ - id: idOrName === 'mcpctl-dev' ? 'proj-1' : 'proj-other', - name: idOrName, - })), - } as unknown as ProjectService; +function mockProjects(opts: { existingNames?: string[] } = {}): ProjectService & { _created: string[] } { + const existing = new Set(opts.existingNames ?? ['mcpctl-dev']); + const created: string[] = []; + // Reverse-lookup id → name so toView's getById finds the project we + // ensured/created (instead of always returning a hardcoded name). + const idToName = new Map(); + const remember = (name: string): { id: string; name: string } => { + const id = `proj-${name}`; + idToName.set(id, name); + return { id, name }; + }; + // Pre-seed the lookup so existing projects round-trip correctly. + for (const n of existing) remember(n); + const svc = { + _created: created, + getById: vi.fn(async (id: string) => ({ id, name: idToName.get(id) ?? 'mcpctl-dev' })), + resolveAndGet: vi.fn(async (idOrName: string) => { + if (!existing.has(idOrName)) { + const err = new Error(`Project not found: ${idOrName}`); + err.name = 'NotFoundError'; + throw err; + } + return remember(idOrName); + }), + // v6: ensureByName auto-creates when missing. + ensureByName: vi.fn(async (name: string, ownerId: string, _opts?: { auditNote?: string }) => { + if (existing.has(name)) return remember(name); + created.push(name); + existing.add(name); + const row = remember(name); + return { ...row, ownerId }; + }), + }; + return svc as unknown as ProjectService & { _created: string[] }; } describe('AgentService — virtual-agent lifecycle (v3 Stage 2)', () => { @@ -192,6 +218,37 @@ describe('AgentService — virtual-agent lifecycle (v3 Stage 2)', () => { expect(out[0]!.status).toBe('active'); }); + it('registerVirtualAgents auto-creates a missing project (v6 Stage 2)', async () => { + // Pre-v6 the publisher had to ask an admin to create the project + // first; otherwise registerVirtualAgents 404'd. v6: ensureByName + // creates with sensible defaults and the publishing user as owner. + const repo = mockAgentRepo(); + const projects = mockProjects({ existingNames: [] }); + const svc = new AgentService(repo, mockLlms(), projects); + const out = await svc.registerVirtualAgents( + 'sess-1', + [{ name: 'team-coder', llmName: 'vllm-local', project: 'platform' }], + 'owner-platform', + ); + expect(out).toHaveLength(1); + expect(out[0]!.project?.name).toBe('platform'); + expect(projects._created).toEqual(['platform']); + }); + + it('registerVirtualAgents reuses an existing project without re-creating it (v6 Stage 2)', async () => { + // ensureByName must be idempotent — second publisher landing on + // the same project name doesn't try to re-create (would 409). + const repo = mockAgentRepo(); + const projects = mockProjects({ existingNames: ['platform'] }); + const svc = new AgentService(repo, mockLlms(), projects); + await svc.registerVirtualAgents( + 'sess-2', + [{ name: 'team-reviewer', llmName: 'vllm-local', project: 'platform' }], + 'owner-other', + ); + expect(projects._created).toEqual([]); + }); + it('registerVirtualAgents refuses to overwrite a public agent (409)', async () => { const repo = mockAgentRepo([makeAgent({ name: 'reviewer', kind: 'public', providerSessionId: null })]); const svc = new AgentService(repo, mockLlms(), mockProjects()); diff --git a/src/mcplocal/src/http/config.ts b/src/mcplocal/src/http/config.ts index d06b9b6..b99ebb9 100644 --- a/src/mcplocal/src/http/config.ts +++ b/src/mcplocal/src/http/config.ts @@ -138,10 +138,30 @@ export interface AgentFileEntry { extras?: Record; } +/** + * v6: per-publisher namespacing config. When a `publisher.suffix` is set, + * mcplocal appends `-` to every published Llm/Agent name before + * sending the register payload. Two mcplocals with distinct suffixes can + * publish the same logical name (`vllm-local-qwen3` from a shared + * config template) without colliding on the cluster-wide unique-name + * constraint — they end up as `vllm-local-qwen3-alice` and + * `vllm-local-qwen3-bob`. Pair with an explicit `poolName` (v4) and they + * still appear as one logical pool. + * + * Set to `"auto"` to derive from the system hostname (sanitized to + * `[a-z0-9-]`); explicit string values override. When the field is + * absent or empty, no suffix is applied — this is fully backwards- + * compatible with pre-v6 configs. + */ +export interface PublisherConfig { + suffix?: string; +} + interface McpctlConfig { llm?: LlmFileConfig | LlmMultiFileConfig; agents?: AgentFileEntry[]; projects?: Record; + publisher?: PublisherConfig; } /** Cached config for the process lifetime (reloaded on SIGHUP if needed). */ @@ -225,6 +245,70 @@ export function loadProjectLlmOverride(projectName: string): ProjectLlmOverride * Load locally-declared agents from ~/.mcpctl/config.json (v3 virtual * agents). Returns empty array if no agents block is configured. */ +/** + * v6: resolve the per-publisher suffix from config (or env override). + * Returns the empty string when no suffix is configured (today's + * behavior — names pass through unchanged). Sanitizes both literal + * strings and the `"auto"` form to the same `[a-z0-9-]+` charset that + * mcpd's name validation accepts; non-conforming characters get + * replaced with `-` so a hostname like `Alice's-laptop.local` becomes + * `alice-s-laptop-local`. + * + * Resolution order: + * 1. Env `MCPCTL_PUBLISHER_SUFFIX` (operations override) + * 2. config.publisher.suffix === "auto" → os.hostname() + * 3. config.publisher.suffix === literal string + * 4. otherwise empty + */ +export function loadPublisherSuffix(env: Record = process.env): string { + const fromEnv = env['MCPCTL_PUBLISHER_SUFFIX']; + if (fromEnv !== undefined && fromEnv !== '') return sanitizeSuffix(fromEnv); + const config = loadFullConfig(); + const suffix = config.publisher?.suffix; + if (suffix === undefined || suffix === '') return ''; + if (suffix === 'auto') { + // eslint-disable-next-line @typescript-eslint/no-require-imports + const os = require('node:os') as typeof import('node:os'); + return sanitizeSuffix(os.hostname()); + } + return sanitizeSuffix(suffix); +} + +/** + * v6: apply the suffix to a name. The result must still be a valid + * mcpd resource name (`[a-z0-9-]{1,100}`); we leave the original name + * untouched when no suffix is set so legacy callers see no change. + * + * Convention: append, don't prefix. Operators reading + * `mcpctl get llm` typically scan by logical name first + * (`vllm-local-qwen3-…`) and the suffix is the disambiguator at the + * tail. The combined length is bounded — registrar refuses names + * longer than 100 chars (server-side validation matches), erroring + * loud rather than silently truncating. + */ +export function applyPublisherSuffix(name: string, suffix: string): string { + if (suffix === '') return name; + const combined = `${name}-${suffix}`; + if (combined.length > 100) { + throw new Error( + `Publisher-suffixed name '${combined}' exceeds the 100-char limit; shorten the base name or the suffix.`, + ); + } + return combined; +} + +function sanitizeSuffix(raw: string): string { + // Lowercase, replace anything outside [a-z0-9-] with '-', collapse + // runs of '-', strip leading/trailing '-'. End result is a stable + // identifier per host that round-trips unchanged on subsequent + // restarts of the same mcplocal. + return raw + .toLowerCase() + .replace(/[^a-z0-9-]+/g, '-') + .replace(/-+/g, '-') + .replace(/^-+|-+$/g, ''); +} + export function loadLocalAgents(): AgentFileEntry[] { const config = loadFullConfig(); return Array.isArray(config.agents) ? config.agents : []; diff --git a/src/mcplocal/src/main.ts b/src/mcplocal/src/main.ts index c4d901a..3dc842a 100644 --- a/src/mcplocal/src/main.ts +++ b/src/mcplocal/src/main.ts @@ -7,7 +7,7 @@ import { StdioProxyServer } from './server.js'; import { StdioUpstream } from './upstream/stdio.js'; import { HttpUpstream } from './upstream/http.js'; import { createHttpServer } from './http/server.js'; -import { loadHttpConfig, loadLlmProviders, loadLocalAgents } from './http/config.js'; +import { loadHttpConfig, loadLlmProviders, loadLocalAgents, loadPublisherSuffix, applyPublisherSuffix } from './http/config.js'; import type { HttpConfig, LlmProviderFileEntry, AgentFileEntry } from './http/config.js'; import { createProvidersFromConfig } from './llm-config.js'; import { createSecretStore } from '@mcpctl/shared'; @@ -204,6 +204,17 @@ async function maybeStartVirtualLlmRegistrar( const opted = llmEntries.filter((e) => e.publish === true); if (opted.length === 0 && localAgents.length === 0) return null; + // v6: per-publisher namespacing. Each user's mcplocal can append a + // suffix to all published names so two publishers sharing a config + // template (`vllm-local-qwen3`) don't collide on mcpd's cluster-wide + // unique-name constraint. Empty suffix = today's behavior. + const publisherSuffix = loadPublisherSuffix(); + // Map of local-provider-name → wire-side publish name. Used twice: + // once when building RegistrarPublishedProvider entries, again when + // rewriting agent.llm references so the agent rows get pinned to + // the suffixed wire name (otherwise they'd 404 on register). + const publishNameByLocal = new Map(); + const published: RegistrarPublishedProvider[] = []; for (const entry of opted) { const provider = providerRegistry.get(entry.name); @@ -211,6 +222,8 @@ async function maybeStartVirtualLlmRegistrar( process.stderr.write(`virtual-llm registrar: provider '${entry.name}' opted-in but not registered locally; skipping\n`); continue; } + const wireName = applyPublisherSuffix(entry.name, publisherSuffix); + publishNameByLocal.set(entry.name, wireName); const item: RegistrarPublishedProvider = { provider, type: entry.type, @@ -219,22 +232,24 @@ async function maybeStartVirtualLlmRegistrar( if (entry.tier !== undefined) item.tier = entry.tier; if (entry.wake !== undefined) item.wake = entry.wake; if (entry.poolName !== undefined) item.poolName = entry.poolName; + if (wireName !== provider.name) item.publishName = wireName; published.push(item); } // v3: forward locally-declared agents alongside the providers. We // only forward agents whose `llm` field points at a name we're // actually publishing (or pre-declared). Stale entries are dropped // with a warning rather than failing the whole registration. + // v6: agent's `llm` field also gets the publisher suffix applied + // (only when it matches a locally-published provider). When it + // doesn't match, the agent is presumed to be pinning a public Llm + // by name and the suffix is NOT applied. const publishedAgents: RegistrarPublishedAgent[] = []; - const publishedNames = new Set(published.map((p) => p.provider.name)); for (const a of localAgents) { - if (!publishedNames.has(a.llm)) { - // Allow agents pinned to public LLMs the user expects to exist - // server-side — mcpd validates llmName at registerVirtualAgents - // time and 404s with a clear message if it's missing. - // We don't drop these client-side; just note it. - } - const item: RegistrarPublishedAgent = { name: a.name, llmName: a.llm }; + // Pin to suffixed wire name when the agent's llm is one of ours; + // pass through otherwise (publisher means it for a public Llm). + const llmWireName = publishNameByLocal.get(a.llm) ?? a.llm; + const agentWireName = applyPublisherSuffix(a.name, publisherSuffix); + const item: RegistrarPublishedAgent = { name: agentWireName, llmName: llmWireName }; if (a.description !== undefined) item.description = a.description; if (a.systemPrompt !== undefined) item.systemPrompt = a.systemPrompt; if (a.project !== undefined) item.project = a.project; diff --git a/src/mcplocal/src/providers/registrar.ts b/src/mcplocal/src/providers/registrar.ts index aedc23e..5e2b17e 100644 --- a/src/mcplocal/src/providers/registrar.ts +++ b/src/mcplocal/src/providers/registrar.ts @@ -60,6 +60,18 @@ export interface RegistrarPublishedProvider { * Agents pinned to any pool member dispatch across all healthy members. */ poolName?: string; + /** + * v6: optional override for the wire-side name. When set, the row + * mcpd creates uses this name instead of `provider.name`. Used by + * the per-publisher namespacing path: each user's mcplocal can take + * a shared local config (`provider.name = "vllm-local-qwen3"`) and + * publish under a hostname-suffixed wire name + * (`vllm-local-qwen3-alice`) so two publishers don't collide on + * mcpd's cluster-wide name uniqueness. Inbound infer/wake tasks + * carry the wire name, so the registrar matches by + * `publishName ?? provider.name` everywhere. + */ + publishName?: string; } /** @@ -186,7 +198,10 @@ export class VirtualLlmRegistrar { if (!alive) initialStatus = 'hibernating'; } return { - name: p.provider.name, + // v6: when `publishName` is set, that's the cluster-wide unique + // name the row goes under. Defaults to the provider's local + // name (today's behavior — no mangling). + name: p.publishName ?? p.provider.name, type: p.type, model: p.model, ...(p.tier !== undefined ? { tier: p.tier } : {}), @@ -350,7 +365,10 @@ export class VirtualLlmRegistrar { * the heartbeat so mcpd's GC sweep doesn't decide we're stale mid-wake. */ private async handleWakeTask(task: { kind: 'wake'; taskId: string; llmName: string }): Promise { - const published = this.opts.publishedProviders.find((p) => p.provider.name === task.llmName); + // v6: match against the publish name (wire-side) when set, fall + // back to the local provider name. Inbound task frames carry the + // wire name mcpd knows the row by. + const published = this.opts.publishedProviders.find((p) => (p.publishName ?? p.provider.name) === task.llmName); if (published === undefined) { await this.postResult(task.taskId, { error: `provider '${task.llmName}' not registered locally` }); return; @@ -385,7 +403,10 @@ export class VirtualLlmRegistrar { } private async handleInferTask(task: InferTask): Promise { - const published = this.opts.publishedProviders.find((p) => p.provider.name === task.llmName); + // v6: match against the publish name (wire-side) when set, fall + // back to the local provider name. Inbound task frames carry the + // wire name mcpd knows the row by. + const published = this.opts.publishedProviders.find((p) => (p.publishName ?? p.provider.name) === task.llmName); if (published === undefined) { await this.postResult(task.taskId, { error: `provider '${task.llmName}' not registered locally` }); return; diff --git a/src/mcplocal/tests/publisher-suffix.test.ts b/src/mcplocal/tests/publisher-suffix.test.ts new file mode 100644 index 0000000..c65d1c1 --- /dev/null +++ b/src/mcplocal/tests/publisher-suffix.test.ts @@ -0,0 +1,61 @@ +/** + * v6 unit tests for the per-publisher namespacing helpers + * (loadPublisherSuffix + applyPublisherSuffix). The wiring through + * mcplocal/main.ts is exercised at the smoke level; these tests + * cover the pure logic so name-mangling regressions are caught fast. + */ +import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; +import { resetConfigCache, applyPublisherSuffix, loadPublisherSuffix } from '../src/http/config.js'; + +describe('applyPublisherSuffix', () => { + it('passes the name through unchanged when the suffix is empty', () => { + // Empty suffix is the legacy code path — pre-v6 behavior. + expect(applyPublisherSuffix('vllm-local-qwen3', '')).toBe('vllm-local-qwen3'); + }); + + it('appends - to a normal name', () => { + expect(applyPublisherSuffix('vllm-local-qwen3', 'alice')).toBe('vllm-local-qwen3-alice'); + }); + + it('throws when the combined length exceeds the 100-char limit', () => { + // Name validation on mcpd's side caps at 100; we'd rather error + // loud at enqueue time than have the register POST 422 later. + const longName = 'a'.repeat(95); + expect(() => applyPublisherSuffix(longName, 'alicebob')).toThrow(/100-char limit/); + }); + + it('lets a name + short suffix exactly hit 100', () => { + const name = 'a'.repeat(95); + expect(applyPublisherSuffix(name, 'four')).toHaveLength(100); + }); +}); + +describe('loadPublisherSuffix', () => { + beforeEach(() => { + resetConfigCache(); + vi.unstubAllEnvs(); + }); + + afterEach(() => { + vi.unstubAllEnvs(); + resetConfigCache(); + }); + + it('honors MCPCTL_PUBLISHER_SUFFIX env var (operations override)', () => { + // Env override takes precedence so an operator can flip the suffix + // for a one-off run without touching ~/.mcpctl/config.json. + expect(loadPublisherSuffix({ MCPCTL_PUBLISHER_SUFFIX: 'BoB.Lap-top' })).toBe('bob-lap-top'); + }); + + it('returns empty string when neither env nor config sets a suffix', () => { + expect(loadPublisherSuffix({})).toBe(''); + }); + + it('sanitizes uppercase + special chars + collapses runs', () => { + expect(loadPublisherSuffix({ MCPCTL_PUBLISHER_SUFFIX: "Alice's-MacBook.Pro!!" })).toBe('alice-s-macbook-pro'); + }); + + it('strips leading/trailing dashes after sanitization', () => { + expect(loadPublisherSuffix({ MCPCTL_PUBLISHER_SUFFIX: '___bob___' })).toBe('bob'); + }); +});