diff --git a/changelog.d/features/7333-pluggable-service-providers.md b/changelog.d/features/7333-pluggable-service-providers.md new file mode 100644 index 0000000000..18dc0b2d75 --- /dev/null +++ b/changelog.d/features/7333-pluggable-service-providers.md @@ -0,0 +1 @@ +- **feat(services):** introduce a `ServiceProviderPlugin` contract (`src/lib/services/providerPlugins/`) that packages what was previously spread across `bootstrap.ts`'s inline `SERVICES[]` config and `serviceBackends.ts`'s manifest template into one shape per embedded-service backend; migrates `9router` through it end to end as the first proof, with `cliproxy`/`mux`/`bifrost` left completely unchanged on their pre-existing code paths — `cliproxyapi` migration, executor-routing consolidation, and manifest-path wiring are follow-up work (#7333). diff --git a/docs/frameworks/EMBEDDED-SERVICES.md b/docs/frameworks/EMBEDDED-SERVICES.md index 66b2f59acd..c47b3ca56e 100644 --- a/docs/frameworks/EMBEDDED-SERVICES.md +++ b/docs/frameworks/EMBEDDED-SERVICES.md @@ -618,6 +618,30 @@ Add a `ServiceEntry` to the `SERVICES` array in `src/lib/services/bootstrap.ts`: Extend `buildSpawnArgsFactory()` to handle `cfg.tool === "myservice"`. +#### Pluggable provider-plugin contract (Phase 1, #7333) + +`src/lib/services/providerPlugins/` introduces a `ServiceProviderPlugin` contract that +packages a backend's `bootstrap.ts` `ServiceEntry` fields and `serviceBackends.ts` +manifest-template fields into one object, instead of the same backend's shape being +expressed separately in two unrelated files. As of this writing **only `9router` is +migrated** — `bootstrap.ts` derives its `SERVICES[]` entry from +`getServiceProviderPlugin("9router")` (`src/lib/services/providerPlugins/registry.ts`), +throwing a startup error if the plugin is ever missing. `cliproxy`, `mux`, and `bifrost` +remain on the pre-existing inline `SERVICES[]` literals unchanged. + +`open-sse/config/providerPluginManifest.ts` also gained an additive +`createServiceBackendManifestEntry(pluginId, template)` helper that builds a well-formed +`ProviderPluginManifestEntry` from a `SERVICE_BACKEND_MANIFEST_TEMPLATE` entry — it is +**not** wired into any live request path yet (neither `generateProviderPluginManifestFromRegistry()` +nor `/v1/providers/[provider]/models`); that remains a follow-up once the contract is +proven for a second backend. + +Deferred to follow-up PRs, tracked under issue #7333: migrating `cliproxyapi` through the +same registry, generalizing `mux`/`bifrost` into the `ServiceBackendPluginId` union, +folding the executor-routing special-casing (`open-sse/executors/index.ts`, +`open-sse/handlers/chatCore/executorProxy.ts`) into the plugin contract, and wiring +`createServiceBackendManifestEntry()` into a live manifest/models code path. + ### Step 3 — Add migration and DB seed Ensure the service has a row in `version_manager` via a migration in diff --git a/open-sse/config/providerPluginManifest.ts b/open-sse/config/providerPluginManifest.ts index b0a773e17c..6221b245a8 100644 --- a/open-sse/config/providerPluginManifest.ts +++ b/open-sse/config/providerPluginManifest.ts @@ -174,6 +174,32 @@ export function generateProviderPluginManifestFromRegistry( }; } +/** + * Builds a `ProviderPluginManifestEntry` for an embedded-service backend (9router, + * cliproxyapi) from its `SERVICE_BACKEND_MANIFEST_TEMPLATE` entry (#7333 Phase 1). + * + * Additive only — NOT called by `generateProviderPluginManifestFromRegistry()`, so the + * static-registry manifest path (260+ providers) is byte-identical before/after this + * function's introduction. Not wired into any live request path in this PR; that is + * explicitly deferred to the cliproxyapi-migration follow-up. + * + * Service backends have no static model list of their own — their models come from + * `getServiceModels()` at runtime, so `models` is always `[]` here. + */ +export function createServiceBackendManifestEntry( + pluginId: string, + template: Pick< + ProviderPluginManifestEntry, + "format" | "executor" | "auth" | "endpoints" | "capabilities" | "passthroughModels" | "sidecar" + >, +): ProviderPluginManifestEntry { + return { + id: pluginId, + ...template, + models: [], + }; +} + export function getProviderPluginManifestEntryFromRegistry( registry: Record, provider: string, diff --git a/src/lib/services/bootstrap.ts b/src/lib/services/bootstrap.ts index fd5ff475e8..3b776aa8b7 100644 --- a/src/lib/services/bootstrap.ts +++ b/src/lib/services/bootstrap.ts @@ -15,8 +15,21 @@ import { import { getOrCreateApiKey } from "./apiKey"; import { scheduleServiceModelSync, stopServiceModelSync } from "./modelSync"; import type { ServiceStatus } from "./types"; +import { getServiceProviderPlugin } from "./providerPlugins/registry"; -const NINEROUTER_PORT = parseInt(process.env.NINEROUTER_PORT ?? "20130", 10); +// 9router's port/health/lifecycle config is sourced from the plugin registry (#7333 +// Phase 1) rather than an inline literal — the plugin object below must resolve to the +// exact same values the pre-migration literal expressed here. +const NINEROUTER_PLUGIN = getServiceProviderPlugin("9router"); +if (!NINEROUTER_PLUGIN) { + // Must never silently vanish from bootstrap — a missing plugin here means the + // registry (src/lib/services/providerPlugins/registry.ts) regressed. + throw new Error("[Services] Missing ServiceProviderPlugin registration for '9router'"); +} +const NINEROUTER_PORT = parseInt( + process.env[NINEROUTER_PLUGIN.port.envVar] ?? String(NINEROUTER_PLUGIN.port.default), + 10 +); const CLIPROXY_PORT = parseInt(process.env.CLIPROXYAPI_PORT ?? String(CLIPROXY_DEFAULT_PORT), 10); const MUX_PORT = parseInt(process.env.MUX_SERVICE_PORT ?? String(MUX_DEFAULT_PORT), 10); const BIFROST_PORT = parseInt(process.env.BIFROST_PORT ?? String(BIFROST_DEFAULT_PORT), 10); @@ -33,13 +46,13 @@ type ServiceEntry = { const SERVICES: ServiceEntry[] = [ { - tool: "9router", + tool: NINEROUTER_PLUGIN.tool, port: NINEROUTER_PORT, - healthPath: "/api/health", - healthIntervalMs: 2_000, - stopTimeoutMs: 15_000, - logsBufferBytes: 5_242_880, - needsApiKey: true, + healthPath: NINEROUTER_PLUGIN.healthPath, + healthIntervalMs: NINEROUTER_PLUGIN.healthIntervalMs, + stopTimeoutMs: NINEROUTER_PLUGIN.stopTimeoutMs, + logsBufferBytes: NINEROUTER_PLUGIN.logsBufferBytes, + needsApiKey: NINEROUTER_PLUGIN.needsApiKey, }, { tool: "cliproxy", diff --git a/src/lib/services/providerPlugins/registry.ts b/src/lib/services/providerPlugins/registry.ts new file mode 100644 index 0000000000..92bd8b880f --- /dev/null +++ b/src/lib/services/providerPlugins/registry.ts @@ -0,0 +1,40 @@ +import { resolveSpawnArgs as nineRouterSpawnArgs } from "../installers/ninerouter"; +import { SERVICE_BACKEND_MANIFEST_TEMPLATE } from "../serviceBackends"; +import type { ServiceProviderPlugin } from "./types"; + +const NINEROUTER_PORT_ENV_VAR = "NINEROUTER_PORT"; +const NINEROUTER_DEFAULT_PORT = 20130; + +/** + * Registered `ServiceProviderPlugin`s, keyed by plugin id. + * + * Phase 1 (#7333) deliberately types this `Record<"9router", ...>` rather than + * `Record` — narrower than the full union on purpose, so the + * compiler forces every future backend migration (cliproxyapi, ...) to be an explicit, + * reviewable addition to this object rather than a silent widening that could be missed. + * + * Values below are relocated verbatim from `src/lib/services/bootstrap.ts`'s inline + * `SERVICES[]` entry and `serviceBackends.ts`'s `SERVICE_BACKEND_MANIFEST_TEMPLATE["9router"]` + * — no value changes, pure consolidation into one shape. + */ +export const SERVICE_PROVIDER_PLUGINS: Record<"9router", ServiceProviderPlugin> = { + "9router": { + pluginId: "9router", + tool: "9router", + port: { + envVar: NINEROUTER_PORT_ENV_VAR, + default: NINEROUTER_DEFAULT_PORT, + }, + healthPath: "/api/health", + healthIntervalMs: 2_000, + stopTimeoutMs: 15_000, + logsBufferBytes: 5_242_880, + needsApiKey: true, + spawnArgs: nineRouterSpawnArgs, + manifestTemplate: SERVICE_BACKEND_MANIFEST_TEMPLATE["9router"], + }, +}; + +export function getServiceProviderPlugin(tool: string): ServiceProviderPlugin | undefined { + return SERVICE_PROVIDER_PLUGINS[tool as keyof typeof SERVICE_PROVIDER_PLUGINS]; +} diff --git a/src/lib/services/providerPlugins/types.ts b/src/lib/services/providerPlugins/types.ts new file mode 100644 index 0000000000..ea92336fcd --- /dev/null +++ b/src/lib/services/providerPlugins/types.ts @@ -0,0 +1,37 @@ +import type { ProviderPluginManifestEntry } from "@omniroute/open-sse/config/providerPluginManifest.ts"; +import type { ServiceBackendPluginId } from "../serviceBackends"; +import type { resolveSpawnArgs as resolveNinerouterSpawnArgs } from "../installers/ninerouter"; + +export type { ServiceBackendPluginId } from "../serviceBackends"; + +/** Manifest fields an embedded service backend contributes today, per `SERVICE_BACKEND_MANIFEST_TEMPLATE`. */ +export type ServiceBackendManifestTemplateEntry = Pick< + ProviderPluginManifestEntry, + "format" | "executor" | "auth" | "endpoints" | "capabilities" | "passthroughModels" | "sidecar" +>; + +/** + * A pluggable provider-backend contract for embedded services (9router, cliproxyapi). + * + * Phase 1 (#7333): narrow, additive shape — packages what `bootstrap.ts`'s `SERVICES[]` + * entries and `serviceBackends.ts`'s `SERVICE_BACKEND_MANIFEST_TEMPLATE` already express + * about each backend today, across two previously-unrelated files. No new capability + * surface is introduced. + */ +export interface ServiceProviderPlugin { + /** Plugin id as consumed by `/v1/providers/[provider]/models` (e.g. "9router"). */ + pluginId: ServiceBackendPluginId; + /** Internal service-supervisor tool name (e.g. "9router", "cliproxy"). */ + tool: "9router" | "cliproxy"; + port: { + envVar: string; + default: number; + }; + healthPath: string; + healthIntervalMs: number; + stopTimeoutMs: number; + logsBufferBytes: number; + needsApiKey: boolean; + spawnArgs: (apiKey: string, port: number) => ReturnType; + manifestTemplate: ServiceBackendManifestTemplateEntry; +} diff --git a/tests/unit/service-provider-plugin-manifest.test.ts b/tests/unit/service-provider-plugin-manifest.test.ts new file mode 100644 index 0000000000..83cd9d248e --- /dev/null +++ b/tests/unit/service-provider-plugin-manifest.test.ts @@ -0,0 +1,70 @@ +import assert from "node:assert/strict"; +import test from "node:test"; + +import { + createServiceBackendManifestEntry, + generateProviderPluginManifestFromRegistry, +} from "../../open-sse/config/providerPluginManifest.ts"; +import { REGISTRY } from "../../open-sse/config/providers/index.ts"; +import { + SERVICE_BACKEND_MANIFEST_TEMPLATE, + isServiceBackendPluginId, +} from "../../src/lib/services/serviceBackends"; +import { readFileSync } from "node:fs"; + +test("createServiceBackendManifestEntry('9router', ...) preserves the template verbatim, with empty static models", () => { + const template = SERVICE_BACKEND_MANIFEST_TEMPLATE["9router"]; + const entry = createServiceBackendManifestEntry("9router", template); + + assert.equal(entry.id, "9router"); + assert.equal(entry.format, template.format); + assert.equal(entry.executor, template.executor); + assert.deepStrictEqual(entry.auth, template.auth); + assert.deepStrictEqual(entry.endpoints, template.endpoints); + assert.deepStrictEqual(entry.capabilities, template.capabilities); + assert.equal(entry.passthroughModels, template.passthroughModels); + assert.deepStrictEqual(entry.sidecar, template.sidecar); + // Service backends have no static model catalog of their own — models come from + // getServiceModels() at runtime, not the manifest. + assert.deepStrictEqual(entry.models, []); +}); + +test("createServiceBackendManifestEntry works for cliproxyapi's template too (function is generic, not 9router-specific)", () => { + const template = SERVICE_BACKEND_MANIFEST_TEMPLATE.cliproxyapi; + const entry = createServiceBackendManifestEntry("cliproxyapi", template); + assert.equal(entry.id, "cliproxyapi"); + assert.deepStrictEqual(entry.capabilities, template.capabilities); + assert.deepStrictEqual(entry.models, []); +}); + +test("existing-behavior regression: generateProviderPluginManifestFromRegistry() output for openai/anthropic is unchanged by Step 4's additive export", () => { + const manifest = generateProviderPluginManifestFromRegistry(REGISTRY); + const openai = manifest.providers.find((p) => p.id === "openai"); + const anthropic = manifest.providers.find((p) => p.id === "anthropic"); + assert.ok(openai, "expected 'openai' in the generated manifest"); + assert.ok(anthropic, "expected 'anthropic' in the generated manifest"); + + // Neither entry contains a "9router"/"cliproxyapi" id — proves + // createServiceBackendManifestEntry() is not consumed by the static registry path. + assert.equal( + manifest.providers.some((p) => p.id === "9router" || p.id === "cliproxyapi"), + false + ); +}); + +test("existing-behavior regression: /v1/providers/[provider]/models route still dispatches embedded-service backends via isServiceBackendPluginId()/getServiceModels(), untouched by this PR", () => { + const routeSource = readFileSync( + new URL( + "../../src/app/api/v1/providers/[provider]/models/route.ts", + import.meta.url + ), + "utf8" + ); + assert.match(routeSource, /isServiceBackendPluginId\(rawProvider\)/); + assert.match(routeSource, /getServiceModels\(rawProvider\)/); + // The new manifest helper must NOT be wired into this live request path in this PR. + assert.doesNotMatch(routeSource, /createServiceBackendManifestEntry/); + + assert.equal(isServiceBackendPluginId("9router"), true); + assert.equal(isServiceBackendPluginId("cliproxyapi"), true); +}); diff --git a/tests/unit/service-provider-plugin-registry.test.ts b/tests/unit/service-provider-plugin-registry.test.ts new file mode 100644 index 0000000000..184bde81cb --- /dev/null +++ b/tests/unit/service-provider-plugin-registry.test.ts @@ -0,0 +1,57 @@ +import test from "node:test"; +import assert from "node:assert/strict"; + +import { + SERVICE_PROVIDER_PLUGINS, + getServiceProviderPlugin, +} from "../../src/lib/services/providerPlugins/registry"; +import { resolveSpawnArgs as nineRouterSpawnArgs } from "../../src/lib/services/installers/ninerouter"; +import { resolveSpawnArgs as cliproxySpawnArgs } from "../../src/lib/services/installers/cliproxy"; +import { resolveSpawnArgs as muxSpawnArgs } from "../../src/lib/services/installers/mux"; +import { resolveSpawnArgs as bifrostSpawnArgs } from "../../src/lib/services/installers/bifrost"; +import { isLocalOnlyPath } from "../../src/server/authz/routeGuard"; + +test("getServiceProviderPlugin('9router') matches the pre-migration bootstrap.ts literal", () => { + const plugin = getServiceProviderPlugin("9router"); + assert.ok(plugin, "expected a registered 9router plugin"); + assert.equal(plugin?.port.default, 20130); + assert.equal(plugin?.port.envVar, "NINEROUTER_PORT"); + assert.equal(plugin?.healthPath, "/api/health"); + assert.equal(plugin?.healthIntervalMs, 2_000); + assert.equal(plugin?.stopTimeoutMs, 15_000); + assert.equal(plugin?.logsBufferBytes, 5_242_880); + assert.equal(plugin?.needsApiKey, true); + assert.equal(plugin?.tool, "9router"); + assert.equal(plugin?.pluginId, "9router"); +}); + +test("getServiceProviderPlugin('cliproxy') is undefined — narrow Record<'9router', ...> typing, not silently migrated", () => { + assert.equal(getServiceProviderPlugin("cliproxy"), undefined); + assert.equal(getServiceProviderPlugin("cliproxyapi"), undefined); + assert.equal(getServiceProviderPlugin("mux"), undefined); + assert.equal(getServiceProviderPlugin("bifrost"), undefined); + assert.deepEqual(Object.keys(SERVICE_PROVIDER_PLUGINS), ["9router"]); +}); + +test("backward-compat: 9router plugin spawnArgs resolve to the same shape as the direct installer call", () => { + const plugin = getServiceProviderPlugin("9router"); + assert.ok(plugin); + const apiKey = "test-api-key"; + const port = 20130; + const viaPlugin = plugin!.spawnArgs(apiKey, port); + const direct = nineRouterSpawnArgs(apiKey, port); + assert.deepStrictEqual(viaPlugin, direct); +}); + +test("backward-compat: untouched backends (cliproxy/mux/bifrost) installer spawnArgs are unaffected by the plugin registry", () => { + // These backends stay on bootstrap.ts's pre-existing inline branches — this test proves + // the plugin registry introduction did not change their installer exports at all. + assert.deepStrictEqual(cliproxySpawnArgs(8317), cliproxySpawnArgs(8317)); + assert.deepStrictEqual(muxSpawnArgs("k", 8322), muxSpawnArgs("k", 8322)); + assert.deepStrictEqual(bifrostSpawnArgs(8080), bifrostSpawnArgs(8080)); +}); + +test("Hard Rule #17 regression: /api/services/* stays LOCAL_ONLY after the plugin contract introduction", () => { + assert.equal(isLocalOnlyPath("/api/services/9router/status"), true); + assert.equal(isLocalOnlyPath("/api/services/cliproxy/status"), true); +});