From c346b937897c743cfb3c819641486611b24650fc Mon Sep 17 00:00:00 2001 From: Michal Date: Tue, 28 Apr 2026 15:54:06 +0100 Subject: [PATCH] feat(mcplocal): per-publisher namespacing for virtual Llms/Agents (v6 Stage 1) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two mcplocals sharing the same config template (`vllm-local-qwen3`) no longer collide on mcpd's cluster-wide unique-name constraint. Each publisher can append a suffix derived from hostname (or any other stable per-host identifier) so the wire-side names become distinct (`vllm-local-qwen3-alice`, `vllm-local-qwen3-bob`). Pair with an explicit `poolName` (v4) and the rows still appear as one logical pool — agents pinned to any member load-balance across both. Config (`~/.mcpctl/config.json`): { "publisher": { "suffix": "auto" } // → os.hostname() sanitized // or { "suffix": "alice" } for explicit override } Or via env: `MCPCTL_PUBLISHER_SUFFIX=alice` (operations override). Resolution order: env var → config.publisher.suffix → empty (legacy behavior, no mangling). Sanitization lowercases, replaces non-`[a-z0-9-]` runs with `-`, strips leading/trailing dashes — the result must satisfy mcpd's name validation, otherwise the register POST would 422. Wire shape: RegistrarPublishedProvider gets an optional `publishName` field. When set, the wire payload's `name` is `publishName` (suffixed); when not, today's `provider.name`. Inbound infer/wake task lookups match `publishName ?? provider.name` so the local registry stays addressable by its original name — SSE frames carrying the suffixed wire name still find their provider. Agents are forwarded with their own suffixed name AND a `llmName` rewritten through the same per-local→wire map so the agent rows pin to the suffixed Llm wire name (otherwise registerVirtualAgents would 404). Tests: 8 new tests covering applyPublisherSuffix (empty, normal, length limit, exact-100) and loadPublisherSuffix (env override, absent, sanitization, dash stripping). Existing registrar tests untouched — no suffix means no behavior change. --- src/mcplocal/src/http/config.ts | 84 +++++++++++++++++++++ src/mcplocal/src/main.ts | 33 +++++--- src/mcplocal/src/providers/registrar.ts | 27 ++++++- src/mcplocal/tests/publisher-suffix.test.ts | 61 +++++++++++++++ 4 files changed, 193 insertions(+), 12 deletions(-) create mode 100644 src/mcplocal/tests/publisher-suffix.test.ts 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'); + }); +});