diff --git a/apps/e2e/demo-e2e-bundle-size/e2e/browser-bundle-size.e2e.spec.ts b/apps/e2e/demo-e2e-bundle-size/e2e/browser-bundle-size.e2e.spec.ts index af32de526..fc30a20cc 100644 --- a/apps/e2e/demo-e2e-bundle-size/e2e/browser-bundle-size.e2e.spec.ts +++ b/apps/e2e/demo-e2e-bundle-size/e2e/browser-bundle-size.e2e.spec.ts @@ -21,11 +21,11 @@ import { } from './helpers/browser-bundle'; const BUDGETS: Array<{ entry: string; label: string; maxBytes: number }> = [ - { entry: 'create-tool.ts', label: 'create() + one tool', maxBytes: 1_289_009 }, - { entry: 'create-webmcp-tool.ts', label: 'create() + WebMcpPlugin + one tool', maxBytes: 1_296_199 }, + { entry: 'create-tool.ts', label: 'create() + one tool', maxBytes: 1_290_698 }, + { entry: 'create-webmcp-tool.ts', label: 'create() + WebMcpPlugin + one tool', maxBytes: 1_297_891 }, ]; -const VITE_INITIAL_CHUNK_MAX_BYTES = 763_249; +const VITE_INITIAL_CHUNK_MAX_BYTES = 764_318; // What create() never needs: the parser behind @enclave-vm/ast (#885), the MCP client (#886) and server-only code const NEVER_BUNDLED: Record = { diff --git a/apps/e2e/demo-e2e-bundle-size/e2e/fixtures/sdk-root-exports.json b/apps/e2e/demo-e2e-bundle-size/e2e/fixtures/sdk-root-exports.json index c7cc5bbd9..d85e0cbf2 100644 --- a/apps/e2e/demo-e2e-bundle-size/e2e/fixtures/sdk-root-exports.json +++ b/apps/e2e/demo-e2e-bundle-size/e2e/fixtures/sdk-root-exports.json @@ -93,6 +93,7 @@ "DidHookOf": "function", "DistributedSemaphore": "class", "DuplicateAppIdError": "class", + "DuplicatePluginEntryError": "class", "DuplicatePluginError": "class", "Duty": "function", "DutyNameConflictError": "class", @@ -372,6 +373,7 @@ "PayloadTooLargeError": "class", "PersonNotSignedInError": "class", "Plugin": "function", + "PluginConfiguredTwiceError": "class", "PluginDependencyError": "class", "PluginEntry": "class", "PluginKind": "object", diff --git a/libs/observability/README.md b/libs/observability/README.md index 310e9875d..8beeb4b87 100644 --- a/libs/observability/README.md +++ b/libs/observability/README.md @@ -92,7 +92,7 @@ const shutdown = setupOTel({ serviceName: 'help-desk', exporter: 'otlp', endpoin The log sinks are `{ type: 'stdout', pretty? }` (one JSON object per line), `{ type: 'console' }` (a readable line), `{ type: 'callback', fn }`, `{ type: 'otlp', endpoint?, headers?, batchSize?, flushIntervalMs?, serviceName? }`, `{ type: 'pino', logger }` and `{ type: 'winston', logger }`. -`@FrontMcp({ observability })` registers `ObservabilityPlugin` for you. You can register `ObservabilityPlugin.init(options)` in `plugins` yourself, but `observability` also connects the `logging` sinks to the server's logger, which the plugin alone doesn't. +`@FrontMcp({ observability })` registers `ObservabilityPlugin` for you. You can register `ObservabilityPlugin.init(options)` in `plugins` yourself instead, but `observability` also connects the `logging` sinks to the server's logger, which the plugin alone doesn't. Use one or the other: a server that sets `observability` and also lists `ObservabilityPlugin` in `@FrontMcp({ plugins })` fails at startup with `PluginConfiguredTwiceError` (code `PLUGIN_CONFIGURED_TWICE`). ## Key exports diff --git a/libs/sdk/src/errors/__tests__/misconfiguration.spec.ts b/libs/sdk/src/errors/__tests__/misconfiguration.spec.ts index 0334c71b4..668f77ed2 100644 --- a/libs/sdk/src/errors/__tests__/misconfiguration.spec.ts +++ b/libs/sdk/src/errors/__tests__/misconfiguration.spec.ts @@ -1,6 +1,8 @@ import { + DuplicatePluginEntryError, DuplicatePluginError, JwtSecretRequiredError, + PluginConfiguredTwiceError, SessionSecretRequiredError, ToolAliasConflictError, } from '../index'; @@ -45,6 +47,18 @@ describe('misconfiguration classification (#646)', () => { expect(found?.remedy).toContain('Install it once'); }); + it('recognizes a plugin listed twice in one plugins list', () => { + const found = findMisconfiguration(new DuplicatePluginEntryError('ledger', '@FrontMcp({ plugins })', 1, 2)); + expect(found?.code).toBe('DUPLICATE_PLUGIN_ENTRY'); + expect(found?.remedy).toContain('List it once with all its options'); + }); + + it('recognizes a plugin a server option installs and the plugins list names too', () => { + const found = findMisconfiguration(new PluginConfiguredTwiceError('ObservabilityPlugin', 'observability')); + expect(found?.code).toBe('PLUGIN_CONFIGURED_TWICE'); + expect(found?.remedy).toContain('Configure the plugin in one place'); + }); + it('maps a ZodError to CONFIG_INVALID', () => { const err = new Error('bad config'); err.name = 'ZodError'; diff --git a/libs/sdk/src/errors/misconfiguration.ts b/libs/sdk/src/errors/misconfiguration.ts index d6a13604f..10abd24a7 100644 --- a/libs/sdk/src/errors/misconfiguration.ts +++ b/libs/sdk/src/errors/misconfiguration.ts @@ -36,6 +36,12 @@ export const MISCONFIGURATION_REMEDIES: Record = { DUPLICATE_PLUGIN: 'A plugin that can be installed once per server is installed twice (in one plugins list, or by the server and ' + 'an app, or by two apps), so the server refuses to start. Install it once; the server log names the plugin.', + DUPLICATE_PLUGIN_ENTRY: + 'One plugins list names a plugin twice, so the server refuses to start. List it once with all its options, ' + + 'or install the other configuration on another app; the server log names the plugin, the list and both entries.', + PLUGIN_CONFIGURED_TWICE: + 'A server option (such as `observability`) installs a plugin that the server plugins list names too, so the ' + + 'server refuses to start. Configure the plugin in one place; the server log names the option and the plugin.', }; /** diff --git a/libs/sdk/src/errors/plugin.errors.ts b/libs/sdk/src/errors/plugin.errors.ts index 6d3f5f661..aebed56c0 100644 --- a/libs/sdk/src/errors/plugin.errors.ts +++ b/libs/sdk/src/errors/plugin.errors.ts @@ -41,3 +41,29 @@ export class DuplicatePluginError extends PublicMcpError { ); } } + +/** Thrown at startup when one plugins list names a plugin twice, which would install only its last entry. */ +export class DuplicatePluginEntryError extends PublicMcpError { + constructor(pluginName: string, listName: string, firstEntry: number, secondEntry: number) { + super( + `Plugin "${pluginName}" is listed twice in ${listName} (entries ${firstEntry} and ${secondEntry}), and only ` + + 'one entry could be installed. List it once, with all its options in that entry, or install the other ' + + 'configuration on an @App of its own.', + 'DUPLICATE_PLUGIN_ENTRY', + 500, + ); + } +} + +/** Thrown at startup when a server option installs a plugin that the server's plugins list names too. */ +export class PluginConfiguredTwiceError extends PublicMcpError { + constructor(pluginClassName: string, serverOption: string) { + super( + `${pluginClassName} is configured twice: by @FrontMcp({ ${serverOption} }) and by an ${pluginClassName} entry ` + + `in @FrontMcp({ plugins }). Configure it in one place: move the entry's options to ${serverOption} and ` + + `remove the entry, or remove ${serverOption}.`, + 'PLUGIN_CONFIGURED_TWICE', + 500, + ); + } +} diff --git a/libs/sdk/src/plugin/__tests__/plugin.registry.duplicate-entry.spec.ts b/libs/sdk/src/plugin/__tests__/plugin.registry.duplicate-entry.spec.ts new file mode 100644 index 000000000..f116b8dc4 --- /dev/null +++ b/libs/sdk/src/plugin/__tests__/plugin.registry.duplicate-entry.spec.ts @@ -0,0 +1,162 @@ +/** + * One plugins list that names a plugin twice refuses to start. + * + * A plugins list keeps one entry per plugin token, so the second entry replaced the first and its + * options were dropped without a word. The same plugin in two lists (two apps, or the server and + * an app) is two installs, each with its own options, and keeps working. + */ +import 'reflect-metadata'; + +import { App, DynamicPlugin, LogLevel, Plugin, Tool, ToolContext, type ProviderType } from '../../common'; +import { DuplicatePluginEntryError, DuplicatePluginError } from '../../errors'; +import { FrontMcpInstance } from '../../front-mcp/front-mcp'; + +class LedgerOwner { + constructor(readonly name: string) {} +} + +interface LedgerOptions { + owner: string; +} + +@Plugin({ name: 'ledger' }) +class LedgerPlugin extends DynamicPlugin { + constructor(readonly options: LedgerOptions) { + super(); + } + + static override dynamicProviders(options: LedgerOptions): ProviderType[] { + return [{ name: 'ledger-owner', provide: LedgerOwner, useValue: new LedgerOwner(options.owner) }]; + } +} + +@Plugin({ name: 'audit' }) +class AuditPlugin {} + +@Plugin({ name: 'catalog', oncePerServer: 'One catalog holds every source.' }) +class CatalogPlugin extends DynamicPlugin { + constructor(readonly options: LedgerOptions) { + super(); + } +} + +@Tool({ name: 'billing_owner', inputSchema: {} }) +class BillingOwnerTool extends ToolContext { + async execute() { + return { owner: this.get(LedgerOwner).name }; + } +} + +@Tool({ name: 'support_owner', inputSchema: {} }) +class SupportOwnerTool extends ToolContext { + async execute() { + return { owner: this.get(LedgerOwner).name }; + } +} + +function startServer(config: Record) { + return FrontMcpInstance.createDirect({ + info: { name: 'duplicate-plugin-entry', version: '1.0.0' }, + logging: { level: LogLevel.Off }, + apps: [], + ...config, + } as never); +} + +@Plugin({ name: 'host', plugins: [LedgerPlugin.init({ owner: 'a' }), LedgerPlugin.init({ owner: 'b' })] }) +class HostPlugin {} + +function duplicateMessage(listName: string, firstEntry: number, secondEntry: number): string { + return `Plugin "ledger" is listed twice in ${listName} (entries ${firstEntry} and ${secondEntry}), and only one entry could be installed.`; +} + +describe('PluginRegistry — a plugin listed twice in one plugins list', () => { + it('refuses to start a server whose plugins list names the plugin class twice', async () => { + const started = startServer({ plugins: [LedgerPlugin, LedgerPlugin] }); + + await expect(started).rejects.toThrow(DuplicatePluginEntryError); + await expect(started).rejects.toThrow(duplicateMessage('@FrontMcp({ plugins })', 1, 2)); + }); + + it('refuses to start a server whose app lists two init() entries of the plugin, naming both entries', async () => { + @App({ + id: 'billing', + name: 'Billing', + plugins: [LedgerPlugin.init({ owner: 'a' }), AuditPlugin, LedgerPlugin.init({ owner: 'b' })], + }) + class BillingApp {} + + const started = startServer({ apps: [BillingApp] }); + + await expect(started).rejects.toThrow(DuplicatePluginEntryError); + await expect(started).rejects.toThrow(duplicateMessage('the plugins of app "billing"', 1, 3)); + }); + + it('refuses a list that names the plugin by its class and by init()', async () => { + await expect(startServer({ plugins: [LedgerPlugin.init({ owner: 'a' }), LedgerPlugin] })).rejects.toThrow( + duplicateMessage('@FrontMcp({ plugins })', 1, 2), + ); + }); + + it("names a plugin's nested plugins list", async () => { + await expect(startServer({ plugins: [HostPlugin] })).rejects.toThrow( + duplicateMessage('the plugins of plugin "host"', 1, 2), + ); + }); + + it('tells how to fix it, with the DUPLICATE_PLUGIN_ENTRY code', async () => { + const error = await startServer({ plugins: [LedgerPlugin, LedgerPlugin] }).catch((thrown: unknown) => thrown); + + expect(error).toMatchObject({ code: 'DUPLICATE_PLUGIN_ENTRY' }); + expect((error as Error).message).toContain( + 'List it once, with all its options in that entry, or install the other configuration on an @App of its own.', + ); + }); + + it('keeps DuplicatePluginError for a plugin installed once per server', async () => { + const started = startServer({ + plugins: [CatalogPlugin.init({ owner: 'a' }), CatalogPlugin.init({ owner: 'b' })], + }); + + await expect(started).rejects.toThrow(DuplicatePluginError); + await expect(started).rejects.not.toThrow(DuplicatePluginEntryError); + }); + + it('lets two apps install the plugin, each with its own options', async () => { + @App({ + id: 'billing', + name: 'Billing', + plugins: [LedgerPlugin.init({ owner: 'billing' })], + tools: [BillingOwnerTool], + }) + class BillingApp {} + @App({ + id: 'support', + name: 'Support', + plugins: [LedgerPlugin.init({ owner: 'support' })], + tools: [SupportOwnerTool], + }) + class SupportApp {} + + const server = await startServer({ apps: [BillingApp, SupportApp] }); + + expect((await server.callTool('billing_owner', {})).structuredContent).toEqual({ owner: 'billing' }); + expect((await server.callTool('support_owner', {})).structuredContent).toEqual({ owner: 'support' }); + await server.dispose(); + }); + + it('lets the server and an app each install the plugin', async () => { + @App({ + id: 'billing', + name: 'Billing', + plugins: [LedgerPlugin.init({ owner: 'billing' })], + tools: [BillingOwnerTool], + }) + class BillingApp {} + + const server = await startServer({ apps: [BillingApp], plugins: [LedgerPlugin.init({ owner: 'server' })] }); + + expect((await server.callTool('billing_owner', {})).structuredContent).toEqual({ owner: 'billing' }); + await server.dispose(); + }); +}); diff --git a/libs/sdk/src/plugin/__tests__/plugin.registry.once-per-server.spec.ts b/libs/sdk/src/plugin/__tests__/plugin.registry.once-per-server.spec.ts index 802d8cd5c..99528ce7b 100644 --- a/libs/sdk/src/plugin/__tests__/plugin.registry.once-per-server.spec.ts +++ b/libs/sdk/src/plugin/__tests__/plugin.registry.once-per-server.spec.ts @@ -1,14 +1,14 @@ /** * A plugin declaring `oncePerServer` refuses a second install in the same server. * - * A plugins list keeps one entry per plugin class, so listing a plugin twice dropped the first entry - * without a word; and two lists of one server (the server's and an app's, or two apps') installed two - * instances that registered the same providers, tools and skills over each other. + * Two lists of one server (the server's and an app's, or two apps') installed two instances that + * registered the same providers, tools and skills over each other. One list that names any plugin + * twice fails with DuplicatePluginEntryError instead; a oncePerServer plugin keeps its own error. */ import 'reflect-metadata'; import { App, DynamicPlugin, LogLevel, Plugin, Tool, ToolContext } from '../../common'; -import { DuplicatePluginError } from '../../errors'; +import { DuplicatePluginEntryError, DuplicatePluginError } from '../../errors'; import { FrontMcpInstance } from '../../front-mcp/front-mcp'; const REASON = 'One catalog can hold several sources.'; @@ -62,6 +62,31 @@ describe('PluginRegistry — a plugin installed once per server (oncePerServer)' await expect(started).rejects.toThrow(DUPLICATE_MESSAGE); }); + it('refuses two entries of it that share its class but give different inline names', async () => { + const started = startServer({ + plugins: [ + { ...CatalogPlugin.init({ source: 'a' }), name: 'catalog-a' }, + { ...CatalogPlugin.init({ source: 'b' }), name: 'catalog-b' }, + ], + }); + + await expect(started).rejects.toThrow(DuplicatePluginEntryError); + await expect(started).rejects.toThrow( + 'Plugin "catalog-b" is listed twice in @FrontMcp({ plugins }) (entries 1 and 2)', + ); + }); + + it('refuses a second entry that adds oncePerServer to a plugin whose first entry has none', async () => { + const started = startServer({ + plugins: [CounterPlugin.init({ source: 'a' }), { ...CounterPlugin.init({ source: 'b' }), oncePerServer: REASON }], + }); + + await expect(started).rejects.toThrow(DuplicatePluginEntryError); + await expect(started).rejects.toThrow( + 'Plugin "counter" is listed twice in @FrontMcp({ plugins }) (entries 1 and 2)', + ); + }); + it('refuses to start a server that installs it and whose app installs it too', async () => { @App({ name: 'billing', plugins: [CatalogPlugin.init({ source: 'b' })] }) class BillingApp {} @@ -110,12 +135,12 @@ describe('PluginRegistry — a plugin installed once per server (oncePerServer)' await Promise.all([first.dispose(), second.dispose()]); }); - it('still starts a server that lists twice a plugin without oncePerServer', async () => { - const server = await startServer({ + it('reports a plugin without oncePerServer listed twice as a duplicate entry, not as a second install', async () => { + const started = startServer({ plugins: [CounterPlugin.init({ source: 'a' }), CounterPlugin.init({ source: 'b' })], }); - expect((await server.listTools()).tools).toEqual([]); - await server.dispose(); + await expect(started).rejects.toThrow(DuplicatePluginEntryError); + await expect(started).rejects.not.toThrow(DuplicatePluginError); }); }); diff --git a/libs/sdk/src/plugin/plugin.registry.ts b/libs/sdk/src/plugin/plugin.registry.ts index 222f7711e..a271fcc21 100644 --- a/libs/sdk/src/plugin/plugin.registry.ts +++ b/libs/sdk/src/plugin/plugin.registry.ts @@ -24,6 +24,7 @@ import { initOptionsOf } from '../common/dynamic/dynamic.plugin'; import { collectDynamicProviders, collectDynamicTools, dedupePluginProviders } from '../common/dynamic/dynamic.utils'; import { installContextExtensions } from '../context/context-extension'; import { + DuplicatePluginEntryError, DuplicatePluginError, InvalidPluginScopeError, InvalidRegistryKindError, @@ -101,6 +102,43 @@ function isConfiguredByOptions(klass: Ctor): boolean { return typeof hooks.dynamicProviders === 'function' || typeof hooks.dynamicTools === 'function'; } +/** The plugins list a registry installs, as a duplicate-entry error names it. */ +function pluginListName(owner: EntryOwnerRef | undefined, hostPluginName: string | undefined): string { + if (hostPluginName !== undefined) return `the plugins of plugin "${hostPluginName}"`; + if (owner === undefined) return 'one plugins list'; + if (owner.kind === 'scope') return '@FrontMcp({ plugins })'; + return `the plugins of ${owner.kind} "${owner.id}"`; +} + +/** + * Refuses a list naming a plugin twice. A oncePerServer plugin's second entry under the same name gets + * DuplicatePluginError instead, when the server claims that name. + */ +function refuseDuplicateEntries(list: readonly PluginType[], listName: string): void { + const firstEntries = new Map(); + for (const [index, raw] of list.entries()) { + const { provide, metadata } = normalizePlugin(raw); + const firstEntry = firstEntries.get(provide); + if (firstEntry === undefined) { + firstEntries.set(provide, { + entryNumber: index + 1, + name: metadata.name, + oncePerServer: metadata.oncePerServer !== undefined, + }); + continue; + } + const sameOncePerServerPlugin = + firstEntry.oncePerServer && metadata.oncePerServer !== undefined && firstEntry.name === metadata.name; + if (sameOncePerServerPlugin) continue; + throw new DuplicatePluginEntryError( + metadata.name ?? tokenName(provide), + listName, + firstEntry.entryNumber, + index + 1, + ); + } +} + export default class PluginRegistry extends RegistryAbstract implements PluginRegistryInterface @@ -136,7 +174,10 @@ export default class PluginRegistry * - scope='server': hooks register to parentScope (if available) */ scopeInfo?: PluginScopeInfo, + /** The plugin whose nested `plugins` this registry installs, named in a duplicate-entry error. */ + hostPluginName?: string, ) { + refuseDuplicateEntries(list, pluginListName(owner, hostPluginName)); super('PluginRegistry', providers, list); this.scope = providers.getActiveScope(); this.scopeInfo = scopeInfo; @@ -243,7 +284,13 @@ export default class PluginRegistry this.owner?.kind === 'app' || this.owner?.kind === 'agent' || this.owner?.kind === 'scope' ? this.owner : pluginOwner; - const plugins = new PluginRegistry(providers, rec.metadata.plugins ?? [], nestedHookOwner, this.scopeInfo); + const plugins = new PluginRegistry( + providers, + rec.metadata.plugins ?? [], + nestedHookOwner, + this.scopeInfo, + rec.metadata.name, + ); await plugins.ready; const adapters = new AdapterRegistry(providers, rec.metadata.adapters ?? []); diff --git a/libs/sdk/src/scope/__tests__/observability-configured-twice.spec.ts b/libs/sdk/src/scope/__tests__/observability-configured-twice.spec.ts new file mode 100644 index 000000000..4bbdcedfd --- /dev/null +++ b/libs/sdk/src/scope/__tests__/observability-configured-twice.spec.ts @@ -0,0 +1,72 @@ +/** + * `@FrontMcp({ observability })` adds its own ObservabilityPlugin to the server's plugins list. A server that also + * lists ObservabilityPlugin there refuses to start with an error that names both places, not two list entries. + */ +import 'reflect-metadata'; + +import { DynamicPlugin, LogLevel, Plugin, Tool, ToolContext } from '../../common'; +import { DuplicatePluginEntryError, PluginConfiguredTwiceError } from '../../errors'; +import { FrontMcpInstance } from '../../front-mcp/front-mcp'; +import { registerOptionalModule } from '../optional-dependency.util'; + +interface ObservabilityOptions { + tracing?: boolean; + logging?: boolean; +} + +@Tool({ name: 'observability_probe', inputSchema: {} }) +class ObservabilityProbeTool extends ToolContext { + async execute() { + return 'installed'; + } +} + +@Plugin({ name: 'observability', tools: [ObservabilityProbeTool] }) +class ObservabilityPlugin extends DynamicPlugin { + constructor(readonly options: ObservabilityOptions) { + super(); + } +} + +const OFF: ObservabilityOptions = { tracing: false, logging: false }; + +function startServer(config: Record) { + return FrontMcpInstance.createDirect({ + info: { name: 'observability-configured-twice', version: '1.0.0' }, + logging: { level: LogLevel.Off }, + apps: [], + ...config, + } as never); +} + +beforeAll(() => { + registerOptionalModule('@frontmcp/observability', { ObservabilityPlugin }); +}); + +describe('ObservabilityPlugin configured by @FrontMcp({ observability }) and by a plugins entry', () => { + it('refuses to start, naming the option and the entry', async () => { + const started = startServer({ observability: OFF, plugins: [ObservabilityPlugin.init(OFF)] }); + + await expect(started).rejects.toThrow(PluginConfiguredTwiceError); + await expect(started).rejects.toThrow( + 'ObservabilityPlugin is configured twice: by @FrontMcp({ observability }) and by an ObservabilityPlugin entry ' + + "in @FrontMcp({ plugins }). Configure it in one place: move the entry's options to observability and " + + 'remove the entry, or remove observability.', + ); + await expect(started).rejects.not.toThrow(DuplicatePluginEntryError); + }); + + it('starts with the option alone, and installs its ObservabilityPlugin', async () => { + const server = await startServer({ observability: OFF }); + + expect((await server.listTools()).tools.map((tool) => tool.name)).toEqual(['observability_probe']); + await server.dispose(); + }); + + it('starts with the entry alone', async () => { + const server = await startServer({ plugins: [ObservabilityPlugin.init(OFF)] }); + + expect((await server.listTools()).tools.map((tool) => tool.name)).toEqual(['observability_probe']); + await server.dispose(); + }); +}); diff --git a/libs/sdk/src/scope/dev-events-observability.ts b/libs/sdk/src/scope/dev-events-observability.ts index a0046cf46..9a3efb5e3 100644 --- a/libs/sdk/src/scope/dev-events-observability.ts +++ b/libs/sdk/src/scope/dev-events-observability.ts @@ -12,9 +12,14 @@ export function devEventsObservabilityConfig( installedPlugins: Iterable, ): { tracing: false; logging: false } | undefined { const underFrontmcpDev = !!getEnv('FRONTMCP_DEV_EVENTS_FILE') || getEnv('FRONTMCP_DEV_EVENTS_IPC') === '1'; - if (!underFrontmcpDev) return undefined; - for (const plugin of installedPlugins) { - if (normalizePlugin(plugin).metadata.name === 'observability') return undefined; - } + if (!underFrontmcpDev || listsObservabilityPlugin(installedPlugins)) return undefined; return { tracing: false, logging: false }; } + +/** Whether a plugins list installs ObservabilityPlugin, from any copy of `@frontmcp/observability`. */ +export function listsObservabilityPlugin(plugins: Iterable): boolean { + for (const plugin of plugins) { + if (normalizePlugin(plugin).metadata.name === 'observability') return true; + } + return false; +} diff --git a/libs/sdk/src/scope/scope.instance.ts b/libs/sdk/src/scope/scope.instance.ts index db8bd9a36..9fee84d02 100644 --- a/libs/sdk/src/scope/scope.instance.ts +++ b/libs/sdk/src/scope/scope.instance.ts @@ -47,7 +47,7 @@ import { proxyTrustOf } from '../context/metadata.utils'; import type { ElicitationStore } from '../elicitation'; import type { PersistentQuestions } from '../elicitation/questions/question-scope.helper'; import { AuthConfigurationError, FlowExitedWithoutOutputError } from '../errors'; -import { UnenforcedMetadataError } from '../errors/plugin.errors'; +import { PluginConfiguredTwiceError, UnenforcedMetadataError } from '../errors/plugin.errors'; import FlowRegistry from '../flows/flow.registry'; import { assertAuthoritiesRules } from '../front-mcp/authorities-rules.check'; import { isEntryServer, isJobWorker } from '../front-mcp/detached-job.flag'; @@ -94,7 +94,7 @@ import { warnIfSessionModeIgnored } from '../transport/session-mode.check'; import type { TransportService } from '../transport/transport.registry'; import type WorkflowRegistry from '../workflow/workflow.registry'; import { guardStorageInBrowser, refuseServerStoresInBrowser, refuseStoresInBrowser } from './browser-stores'; -import { devEventsObservabilityConfig } from './dev-events-observability'; +import { devEventsObservabilityConfig, listsObservabilityPlugin } from './dev-events-observability'; import { registerServerEntries } from './server-entries.helper'; import { reportStartup } from './startup-report.helper'; @@ -681,6 +681,9 @@ export class Scope extends ScopeEntry { : undefined; const observabilityConfig = this.metadata.observability ?? impliedObservability; if (observabilityConfig) { + if (listsObservabilityPlugin(serverPlugins)) { + throw new PluginConfiguredTwiceError('ObservabilityPlugin', 'observability'); + } const { observabilityPlugins } = await import('./observability-scope.helper.js'); serverPlugins.push( ...observabilityPlugins({ diff --git a/libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md b/libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md index 0d49ae00f..5b9c4c335 100644 --- a/libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md +++ b/libs/skills/catalog/frontmcp-deployment/references/deploy-to-cloudflare.md @@ -196,7 +196,7 @@ npx wrangler secret put JWT_SECRET # openssl rand -hex 32 **When the server fails to start.** The Worker builds the server on its first request. A failed build is answered, never thrown to the platform, and the error's message is not echoed: -- A configuration fault (missing or weak secret, a startup check, a config the schema refuses): `500 {"error":"server_misconfigured","code":"…"}` — codes `SESSION_SECRET_REQUIRED`, `JWT_SECRET_REQUIRED`, `JWT_SECRET_INVALID`, `UNENFORCED_METADATA`, `AUTH_CONFIGURATION_ERROR`, `TOOL_ALIAS_CONFLICT`, `DUPLICATE_PLUGIN`, `CONFIG_INVALID`. +- A configuration fault (missing or weak secret, a startup check, a config the schema refuses): `500 {"error":"server_misconfigured","code":"…"}` — codes `SESSION_SECRET_REQUIRED`, `JWT_SECRET_REQUIRED`, `JWT_SECRET_INVALID`, `UNENFORCED_METADATA`, `AUTH_CONFIGURATION_ERROR`, `TOOL_ALIAS_CONFLICT`, `DUPLICATE_PLUGIN`, `DUPLICATE_PLUGIN_ENTRY`, `PLUGIN_CONFIGURED_TWICE`, `CONFIG_INVALID`. - Anything else (a remote that refused the connection, a package that failed to load): `503 {"error":"server_unavailable","code":"SERVER_START_FAILED"}` with `Retry-After`. The failure is kept until a retry delay passes (1 s, doubling up to 60 s); the first request after it builds again. The cause is logged once per attempt (`wrangler tail`). Same for `createEdgeMcp()`, `createFetchHandler()` on an edge isolate, and each session Durable Object. diff --git a/libs/skills/catalog/frontmcp-development/references/create-plugin.md b/libs/skills/catalog/frontmcp-development/references/create-plugin.md index 549de4855..93b01f5ef 100644 --- a/libs/skills/catalog/frontmcp-development/references/create-plugin.md +++ b/libs/skills/catalog/frontmcp-development/references/create-plugin.md @@ -565,7 +565,23 @@ A plugin whose tools, skills or providers are the server's, such as `@frontmcp/p export default class CatalogPlugin extends DynamicPlugin {} ``` -A server that installs it twice then fails at startup with `DuplicatePluginError` (code `DUPLICATE_PLUGIN`): `Plugin "catalog" can be installed once per server, and this server installs it twice.` followed by the reason. That covers the same `plugins` list naming it twice, the server and an app each installing it, and two apps of the server each installing it. An app with an endpoint of its own (`splitByApp`, or `standalone`) is a server of its own, so each such app may install one. Without `oncePerServer`, a plugin listed twice in one `plugins` list is installed once, from its last entry. +A server that installs it twice then fails at startup with `DuplicatePluginError` (code `DUPLICATE_PLUGIN`): `Plugin "catalog" can be installed once per server, and this server installs it twice.` followed by the reason. That covers the same `plugins` list naming it twice, the server and an app each installing it, and two apps of the server each installing it. An app with an endpoint of its own (`splitByApp`, or `standalone`) is a server of its own, so each such app may install one. + +### A plugin listed twice in one `plugins` list + +Any plugin, with or without `oncePerServer`, is listed once per `plugins` list. Two entries of one plugin in the same list (the class twice, two `init()` calls, or the class and an `init()`) fail at startup with `DuplicatePluginEntryError` (code `DUPLICATE_PLUGIN_ENTRY`), which names the plugin, the list (`@FrontMcp({ plugins })`, `the plugins of app ""`, `the plugins of agent ""` or `the plugins of plugin ""`) and the two entries by their place in it: `Plugin "cache" is listed twice in the plugins of app "billing" (entries 1 and 3), and only one entry could be installed.` Up to 1.11.0 the list kept only the last entry and dropped the other's options without a word. + +Put all the options in one entry. To run a plugin with two configurations, install each on its own `@App`: each app's list is installed on its own, so the same plugin in two apps' lists, or in the server's list and an app's, is two instances, each with its own options. A `oncePerServer` plugin still allows one install per server, and listing it twice in one list fails with its `DuplicatePluginError`. + +A server option that installs a plugin for you counts as an entry the user didn't write. `@FrontMcp({ observability })` adds `ObservabilityPlugin`; a server that sets it and also lists `ObservabilityPlugin` in `@FrontMcp({ plugins })` fails with `PluginConfiguredTwiceError` (code `PLUGIN_CONFIGURED_TWICE`), which names the option and the entry. Configure the plugin in one place. + +```typescript +@App({ name: 'Billing', plugins: [CachePlugin.init({ type: 'memory', defaultTTL: 60 })] }) +class BillingApp {} + +@App({ name: 'Reports', plugins: [CachePlugin.init({ type: 'memory', defaultTTL: 3600 })] }) +class ReportsApp {} +``` ## Step 5: Extend Metadata and Execution Context diff --git a/libs/skills/catalog/frontmcp-development/references/official-plugins.md b/libs/skills/catalog/frontmcp-development/references/official-plugins.md index cd0c8c168..2b60bdaa9 100644 --- a/libs/skills/catalog/frontmcp-development/references/official-plugins.md +++ b/libs/skills/catalog/frontmcp-development/references/official-plugins.md @@ -66,6 +66,8 @@ class MyApp {} class MyServer {} ``` +List each plugin once per `plugins` list. Two entries of one plugin in the same list (the class twice, or two `init()` calls) fail at startup with `DuplicatePluginEntryError` (code `DUPLICATE_PLUGIN_ENTRY`), naming the plugin, its list and both entries; up to 1.11.0 only the last entry was installed. Put all its options in one `init()`, or install each configuration on its own `@App` (see `create-plugin`). + --- ## 1. CodeCall Plugin (`@frontmcp/plugin-codecall`) diff --git a/libs/skills/catalog/frontmcp-observability/SKILL.md b/libs/skills/catalog/frontmcp-observability/SKILL.md index 528c8591c..f7bd7ac76 100644 --- a/libs/skills/catalog/frontmcp-observability/SKILL.md +++ b/libs/skills/catalog/frontmcp-observability/SKILL.md @@ -79,6 +79,8 @@ This enables auto-tracing for all SDK flows. Add structured logging: }) ``` +`observability` installs `ObservabilityPlugin` for you, so don't also list `ObservabilityPlugin.init()` in `@FrontMcp({ plugins })`. A server that does both fails at startup with `PluginConfiguredTwiceError` (code `PLUGIN_CONFIGURED_TWICE`): `ObservabilityPlugin is configured twice: by @FrontMcp({ observability }) and by an ObservabilityPlugin entry in @FrontMcp({ plugins }).` Move the entry's options to `observability` and remove the entry; only `observability` also connects the logging sinks to the server's logger. Up to 1.11.0 the server started anyway, and the entry's options could be dropped for the ones `observability` gives. + ## Step 3: Read the Relevant Reference Follow the scenario routing table above to find the right reference for your use case. @@ -143,15 +145,16 @@ Follow the scenario routing table above to find the right reference for your use ## Troubleshooting -| Problem | Cause | Solution | -| ------------------------------------------------ | ------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | -| `this.telemetry` is undefined in a tool | `observability` not enabled on the parent `@FrontMcp` config | Set `observability: true` (or a config object) in the `@FrontMcp` decorator; see `tracing-setup` | -| `Property 'telemetry' does not exist` (TS2339) | `@frontmcp/observability` is not part of the compilation | Add `import type {} from '@frontmcp/observability'` where you use it (or once in the server entry); see `telemetry-api` | -| Spans appear without `trace_id` in logs | Logger not connected to `StructuredLogTransport` | Use `this.logger`, not `console`; see `structured-logging` | -| OTLP exporter silently drops spans | Endpoint URL points at the UI, not the OTLP collector | Use the OTLP HTTP/gRPC ingest endpoint exposed by your vendor (Datadog, Coralogix, Logz, etc.); see `vendor-integrations` | -| Real session ID appears in span attributes | A custom span attribute writes `session.id` directly | Use the SDK-provided `mcp.session.id` (already hashed); never log the raw session token | -| Tests randomly fail with leftover spans | Exporter retained between tests | Reset the in-memory exporter in `afterEach`; see `testing-observability` | -| OTel auto-instrumentation double-traces requests | Both `setupOTel()` AND a vendor agent attached to the process | Pick one: either FrontMCP-managed OTel OR the vendor agent — not both | +| Problem | Cause | Solution | +| ------------------------------------------------ | ----------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------- | +| `this.telemetry` is undefined in a tool | `observability` not enabled on the parent `@FrontMcp` config | Set `observability: true` (or a config object) in the `@FrontMcp` decorator; see `tracing-setup` | +| `Property 'telemetry' does not exist` (TS2339) | `@frontmcp/observability` is not part of the compilation | Add `import type {} from '@frontmcp/observability'` where you use it (or once in the server entry); see `telemetry-api` | +| Spans appear without `trace_id` in logs | Logger not connected to `StructuredLogTransport` | Use `this.logger`, not `console`; see `structured-logging` | +| OTLP exporter silently drops spans | Endpoint URL points at the UI, not the OTLP collector | Use the OTLP HTTP/gRPC ingest endpoint exposed by your vendor (Datadog, Coralogix, Logz, etc.); see `vendor-integrations` | +| Real session ID appears in span attributes | A custom span attribute writes `session.id` directly | Use the SDK-provided `mcp.session.id` (already hashed); never log the raw session token | +| Tests randomly fail with leftover spans | Exporter retained between tests | Reset the in-memory exporter in `afterEach`; see `testing-observability` | +| OTel auto-instrumentation double-traces requests | Both `setupOTel()` AND a vendor agent attached to the process | Pick one: either FrontMCP-managed OTel OR the vendor agent — not both | +| Startup fails with `PLUGIN_CONFIGURED_TWICE` | `observability` is set and `ObservabilityPlugin` is listed in `plugins` | Configure it in one place: move the entry's options to `observability` and remove the entry | ## Examples