From 081edc11e262b95000dbb1199475f52aafbcaa0b Mon Sep 17 00:00:00 2001 From: Etienne Rossignon Date: Sun, 25 Oct 2026 21:23:27 +0100 Subject: [PATCH 1/3] fix(mcp): unwrap i3X response envelopes in the REST client The client read bare keys (res.objects, res.values, ...) while node-i3x answers { success, result } and bulk { success, results: [...] }, so every tool returned empty lists without error. Unwrap both envelopes; a success:false envelope or a failed bulk item now throws with its responseDetail, and getObjectById returns null only for a 404 item. Also: strip a trailing /v1 from the base URL (was /v1/v1), send elementIds to /objects/related and read .object, keep the parent out of inspect's subComponents, read inspect values with maxDepth 2 (an Object's readings are its components), and take the advertised version from package.json. --- packages/mcp/src/bindings/rest-binding.ts | 156 ++++++++------ packages/mcp/src/cli.ts | 6 +- packages/mcp/src/server.ts | 7 +- packages/mcp/src/tools/inspect-asset.ts | 16 +- packages/mcp/tests/rest-binding.test.ts | 238 ++++++++++++++++++++++ 5 files changed, 354 insertions(+), 69 deletions(-) create mode 100644 packages/mcp/tests/rest-binding.test.ts diff --git a/packages/mcp/src/bindings/rest-binding.ts b/packages/mcp/src/bindings/rest-binding.ts index fce6775..c76374e 100644 --- a/packages/mcp/src/bindings/rest-binding.ts +++ b/packages/mcp/src/bindings/rest-binding.ts @@ -1,11 +1,14 @@ import type { + BulkResponse, CurrentValueResult, HistoricalValueResult, Namespace, ObjectInstanceResponse, ObjectType, + RelatedObjectResult, RelationshipType, ServerInfo, + SuccessResponse, } from '@node-i3x/core'; import { rankSuggestions } from '../utils/similarity.js'; import type { I3XClientBinding } from './types.js'; @@ -20,6 +23,8 @@ export class RestClientBinding implements I3XClientBinding { if (!cleanUrl.startsWith('http://') && !cleanUrl.startsWith('https://')) { cleanUrl = `http://${cleanUrl}`; } + // i3X base URLs are usually given with the /v1 suffix; every path below adds it. + cleanUrl = cleanUrl.replace(/\/v1$/i, ''); this.baseUrl = cleanUrl; this.requestTimeoutMs = timeoutMs; this.headers = { @@ -63,28 +68,69 @@ export class RestClientBinding implements I3XClientBinding { } } + /** Unwrap a `{ success, result }` envelope; a `success: false` envelope becomes an error. */ + private async get(path: string, options: RequestInit = {}): Promise { + const res = await this.request> & ErrorEnvelope>( + path, + options, + ); + if (res?.success !== true || !('result' in res)) { + throw new Error(`i3X request ${path} failed: ${describeFailure(res)}`); + } + return res.result as T; + } + + /** + * Unwrap a bulk `{ success, results: [{ elementId, success, result }] }` envelope, + * one entry per requested elementId, in request order. A failed item becomes an + * error naming its elementId; `notFoundAsNull` maps a 404 item to `null` instead. + */ + private async getBulk( + path: string, + body: unknown, + notFoundAsNull?: false, + ): Promise; + private async getBulk( + path: string, + body: unknown, + notFoundAsNull: true, + ): Promise>; + private async getBulk( + path: string, + body: unknown, + notFoundAsNull = false, + ): Promise> { + const res = await this.request> & ErrorEnvelope>(path, { + method: 'POST', + body: JSON.stringify(body), + }); + if (!Array.isArray(res?.results)) { + throw new Error(`i3X request ${path} failed: ${describeFailure(res)}`); + } + return res.results.map((item) => { + if (item.success) return (item.result ?? null) as T | null; + if (notFoundAsNull && item.responseDetail?.status === 404) return null; + throw new Error( + `i3X request ${path} failed for '${item.elementId}': ${describeFailure(item)}`, + ); + }); + } + async getInfo(): Promise { - return this.request('/v1/info'); + return this.get('/v1/info'); } async getNamespaces(): Promise { - const res = await this.request<{ namespaces: Namespace[] }>('/v1/namespaces'); - return res.namespaces || []; + return this.get('/v1/namespaces'); } async getObjectTypes(namespaceUri?: string): Promise { const query = namespaceUri ? `?namespaceUri=${encodeURIComponent(namespaceUri)}` : ''; - const res = await this.request<{ objectTypes: ObjectType[] }>( - `/v1/objecttypes${query}`, - ); - return res.objectTypes || []; + return this.get(`/v1/objecttypes${query}`); } async getRelationshipTypes(): Promise { - const res = await this.request<{ relationshipTypes: RelationshipType[] }>( - '/v1/relationshiptypes', - ); - return res.relationshipTypes || []; + return this.get('/v1/relationshiptypes'); } async getObjects(options?: { @@ -96,62 +142,43 @@ export class RestClientBinding implements I3XClientBinding { if (options?.typeElementId) params.set('typeElementId', options.typeElementId); const query = params.toString() ? `?${params.toString()}` : ''; - const res = await this.request<{ objects: ObjectInstanceResponse[] }>( - `/v1/objects${query}`, - ); - return res.objects || []; + return this.get(`/v1/objects${query}`); } async getObjectById(elementId: string): Promise { - try { - const res = await this.request<{ objects: ObjectInstanceResponse[] }>( - '/v1/objects/list', - { - method: 'POST', - body: JSON.stringify({ elementIds: [elementId] }), - }, - ); - if (res.objects && res.objects.length > 0) { - return res.objects[0] ?? null; - } - return null; - } catch { - return null; - } + const [object] = await this.getBulk( + '/v1/objects/list', + { elementIds: [elementId] }, + true, + ); + return object ?? null; } async getRelatedObjects( elementId: string, relationshipType?: string, ): Promise { - try { - const body: Record = { elementId }; - if (relationshipType) body.relationshipType = relationshipType; - - const res = await this.request<{ - relatedObjects: Array<{ targetObject: ObjectInstanceResponse }>; - }>('/v1/objects/related', { - method: 'POST', - body: JSON.stringify(body), - }); - return (res.relatedObjects || []).map((r) => r.targetObject); - } catch { - return []; - } + const body: Record = { elementIds: [elementId] }; + if (relationshipType) body.relationshipType = relationshipType; + + const [related] = await this.getBulk( + '/v1/objects/related', + body, + ); + // Servers may ignore relationshipType, so filter here as well. + return (related ?? []) + .filter((r) => !relationshipType || r.sourceRelationship === relationshipType) + .map((r) => r.object); } async getObjectValues( elementIds: string[], maxDepth = 1, ): Promise { - const res = await this.request<{ values: CurrentValueResult[] }>( - '/v1/objects/value', - { - method: 'POST', - body: JSON.stringify({ elementIds, maxDepth }), - }, - ); - return res.values || []; + return this.getBulk('/v1/objects/value', { + elementIds, + maxDepth, + }); } async getHistoricalValues( @@ -160,14 +187,12 @@ export class RestClientBinding implements I3XClientBinding { endTime: string, maxDepth = 1, ): Promise { - const res = await this.request<{ history: HistoricalValueResult[] }>( - '/v1/objects/history', - { - method: 'POST', - body: JSON.stringify({ elementIds, startTime, endTime, maxDepth }), - }, - ); - return res.history || []; + return this.getBulk('/v1/objects/history', { + elementIds, + startTime, + endTime, + maxDepth, + }); } async findSuggestions(unknownId: string): Promise { @@ -180,3 +205,14 @@ export class RestClientBinding implements I3XClientBinding { } } } + +interface ErrorEnvelope { + success?: boolean; + responseDetail?: { title?: string; status?: number; detail?: string } | null; +} + +function describeFailure(res: ErrorEnvelope | null | undefined): string { + const d = res?.responseDetail; + if (d) return [d.status, d.title, d.detail].filter(Boolean).join(' '); + return `unexpected response ${JSON.stringify(res)?.slice(0, 200)}`; +} diff --git a/packages/mcp/src/cli.ts b/packages/mcp/src/cli.ts index 5a06406..7acf788 100644 --- a/packages/mcp/src/cli.ts +++ b/packages/mcp/src/cli.ts @@ -1,4 +1,5 @@ #!/usr/bin/env node +import { createRequire } from 'node:module'; import { RestClientBinding } from './bindings/rest-binding.js'; import { NodeI3xMcpServer } from './server.js'; @@ -25,7 +26,10 @@ Environment Variables: } if (args.includes('--version') || args.includes('-v')) { - console.error('0.10.0'); + const { version } = createRequire(import.meta.url)('../package.json') as { + version: string; + }; + console.log(version); process.exit(0); } diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 117d915..457022a 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -1,3 +1,4 @@ +import { createRequire } from 'node:module'; import { Server } from '@modelcontextprotocol/sdk/server/index.js'; import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js'; import { @@ -18,6 +19,10 @@ import { handleInspectAsset, inspectAssetSchema } from './tools/inspect-asset.js import { handleMonitorEvents, monitorEventsSchema } from './tools/monitor-events.js'; import { handleQueryTelemetry, queryTelemetrySchema } from './tools/query-telemetry.js'; +const { version } = createRequire(import.meta.url)('../package.json') as { + version: string; +}; + export class NodeI3xMcpServer { private readonly server: Server; private binding: I3XClientBinding; @@ -28,7 +33,7 @@ export class NodeI3xMcpServer { this.server = new Server( { name: '@node-i3x/mcp', - version: '0.10.0', + version, }, { capabilities: { diff --git a/packages/mcp/src/tools/inspect-asset.ts b/packages/mcp/src/tools/inspect-asset.ts index 8a9d353..63aa086 100644 --- a/packages/mcp/src/tools/inspect-asset.ts +++ b/packages/mcp/src/tools/inspect-asset.ts @@ -47,8 +47,9 @@ export async function handleInspectAsset( includeSchema && asset.typeElementId ? binding.getObjectTypes() : Promise.resolve(null); + // maxDepth 2: an Object has no value of its own, its readings are its components. const valuesPromise = includeValues - ? binding.getObjectValues([elementId]) + ? binding.getObjectValues([elementId], 2) : Promise.resolve(null); const [relatedResult, schemaResult, valuesResult] = await Promise.allSettled([ @@ -58,12 +59,13 @@ export async function handleInspectAsset( ]); // Sub-components - if ( - relatedResult.status === 'fulfilled' && - Array.isArray(relatedResult.value) && - relatedResult.value.length > 0 - ) { - result.subComponents = relatedResult.value.map((r) => ({ + // Related objects include the parent (reverse relationship): it is not a sub-component. + const subComponents = + relatedResult.status === 'fulfilled' && Array.isArray(relatedResult.value) + ? relatedResult.value.filter((r) => r.elementId !== asset.parentId) + : []; + if (subComponents.length > 0) { + result.subComponents = subComponents.map((r) => ({ elementId: r.elementId, displayName: r.displayName || r.elementId, typeElementId: r.typeElementId, diff --git a/packages/mcp/tests/rest-binding.test.ts b/packages/mcp/tests/rest-binding.test.ts new file mode 100644 index 0000000..dbf1e1e --- /dev/null +++ b/packages/mcp/tests/rest-binding.test.ts @@ -0,0 +1,238 @@ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { RestClientBinding } from '../src/bindings/rest-binding.js'; +import { handleInspectAsset } from '../src/tools/inspect-asset.js'; + +// Payloads below mirror what a node-i3x server (packages/rest-server) answers. + +const machine = { + elementId: 'instance-1f7ad67cf3247a9d', + displayName: 'CoffeeMachineA', + typeElementId: 'CoffeeMachineType', + parentId: 'instance-plant', + isComposition: true, +}; +const boiler = { + elementId: 'instance-boiler', + displayName: 'Boiler', + parentId: machine.elementId, + isComposition: false, +}; +const plant = { elementId: 'instance-plant', displayName: 'Plant', isComposition: true }; + +type Route = (body: Record | undefined) => { + status?: number; + json: unknown; +}; + +let routes: Record; +let calls: Array<{ url: string; body?: Record }>; + +beforeEach(() => { + routes = {}; + calls = []; + vi.stubGlobal('fetch', async (input: string, init?: RequestInit) => { + const url = String(input); + const body = init?.body ? JSON.parse(String(init.body)) : undefined; + calls.push({ url, body }); + const path = new URL(url).pathname.replace(/^\/api\/i3x\/abc/, ''); + const route = routes[path]; + if (!route) return new Response('Not Found', { status: 404 }); + const { status = 200, json } = route(body); + return new Response(JSON.stringify(json), { status }); + }); +}); + +afterEach(() => { + vi.unstubAllGlobals(); +}); + +const elementIdsOf = (body?: Record) => + (body?.elementIds ?? []) as string[]; +const ok = (result: unknown) => () => ({ json: { success: true, result } }); +const bulk = (fn: (id: string) => unknown) => (body?: Record) => ({ + json: { + success: true, + results: elementIdsOf(body).map((elementId) => ({ + success: true, + elementId, + result: fn(elementId), + })), + }, +}); + +describe('RestClientBinding', () => { + describe('base URL', () => { + it.each([ + 'https://portal.example/api/i3x/abc', + 'https://portal.example/api/i3x/abc/', + 'https://portal.example/api/i3x/abc/v1', + 'https://portal.example/api/i3x/abc/v1/', + ])('accepts %s', async (baseUrl) => { + routes['/v1/objects'] = ok([plant]); + const objects = await new RestClientBinding(baseUrl).getObjects({ rootOnly: true }); + expect(objects).toEqual([plant]); + expect(calls[0]?.url).toBe( + 'https://portal.example/api/i3x/abc/v1/objects?root=true', + ); + }); + }); + + describe('success envelope', () => { + const client = () => new RestClientBinding('https://portal.example/api/i3x/abc'); + + it('unwraps result for objects, namespaces, types and info', async () => { + routes['/v1/objects'] = ok([plant, machine]); + routes['/v1/namespaces'] = ok([{ uri: 'urn:ns', displayName: 'NS' }]); + routes['/v1/objecttypes'] = ok([{ elementId: 'CoffeeMachineType' }]); + routes['/v1/relationshiptypes'] = ok([{ elementId: 'HasComponent' }]); + routes['/v1/info'] = ok({ serverName: 'node-i3x', specVersion: '1.0' }); + + const c = client(); + expect(await c.getObjects()).toEqual([plant, machine]); + expect(await c.getNamespaces()).toEqual([{ uri: 'urn:ns', displayName: 'NS' }]); + expect(await c.getObjectTypes()).toEqual([{ elementId: 'CoffeeMachineType' }]); + expect(await c.getRelationshipTypes()).toEqual([{ elementId: 'HasComponent' }]); + expect(await c.getInfo()).toMatchObject({ serverName: 'node-i3x' }); + }); + + it('unwraps bulk results for list, value and history', async () => { + routes['/v1/objects/list'] = bulk(() => machine); + routes['/v1/objects/value'] = bulk(() => ({ + isComposition: false, + value: 92.5, + quality: 'Good', + timestamp: '2026-10-25T10:00:00Z', + })); + routes['/v1/objects/history'] = bulk(() => ({ + isComposition: false, + values: [{ value: 1, quality: 'Good', timestamp: '2026-10-25T10:00:00Z' }], + })); + + const c = client(); + expect(await c.getObjectById(machine.elementId)).toEqual(machine); + expect(await c.getObjectValues([machine.elementId])).toEqual([ + { + isComposition: false, + value: 92.5, + quality: 'Good', + timestamp: '2026-10-25T10:00:00Z', + }, + ]); + const history = await c.getHistoricalValues( + [machine.elementId], + '2026-10-25T00:00:00Z', + '2026-10-25T12:00:00Z', + ); + expect(history[0]?.values).toHaveLength(1); + }); + + it('sends elementIds to /v1/objects/related and reads the related objects', async () => { + routes['/v1/objects/related'] = bulk(() => [ + { sourceRelationship: 'HasComponent', object: boiler }, + { sourceRelationship: 'IsComponentOf', object: plant }, + ]); + + const c = client(); + expect(await c.getRelatedObjects(machine.elementId)).toEqual([boiler, plant]); + expect(calls[0]?.body).toEqual({ elementIds: [machine.elementId] }); + expect(await c.getRelatedObjects(machine.elementId, 'HasComponent')).toEqual([ + boiler, + ]); + }); + + it('returns null from getObjectById for a 404 item', async () => { + routes['/v1/objects/list'] = (body) => ({ + json: { + success: false, + results: [ + { + success: false, + elementId: elementIdsOf(body)[0], + responseDetail: { title: 'Error', status: 404, detail: 'Not found' }, + }, + ], + }, + }); + expect(await client().getObjectById('nope')).toBeNull(); + }); + }); + + describe('error envelope', () => { + const client = () => new RestClientBinding('https://portal.example/api/i3x/abc'); + + it('throws the responseDetail of a success:false envelope instead of returning []', async () => { + routes['/v1/objects'] = () => ({ + json: { + success: false, + responseDetail: { title: 'Internal Server Error', status: 500, detail: 'boom' }, + }, + }); + await expect(client().getObjects()).rejects.toThrow( + /500 Internal Server Error boom/, + ); + }); + + it('throws on a body without the envelope instead of returning []', async () => { + routes['/v1/namespaces'] = () => ({ json: { namespaces: [] } }); + await expect(client().getNamespaces()).rejects.toThrow(/unexpected response/); + }); + + it('throws for a failed bulk item, naming its elementId', async () => { + routes['/v1/objects/value'] = () => ({ + json: { + success: false, + results: [ + { + success: false, + elementId: 'bad-id', + responseDetail: { title: 'Error', status: 404, detail: 'Not found' }, + }, + ], + }, + }); + await expect(client().getObjectValues(['bad-id'])).rejects.toThrow( + /'bad-id': 404 Error Not found/, + ); + }); + + it('getObjectById propagates a non-404 failure instead of reporting "not found"', async () => { + routes['/v1/objects/list'] = () => ({ + status: 401, + json: { + success: false, + responseDetail: { title: 'Unauthorized', status: 401, detail: 'bad token' }, + }, + }); + await expect(client().getObjectById(machine.elementId)).rejects.toThrow(/HTTP 401/); + }); + }); + + it('i3x_inspect_asset lists children, not the parent, as sub-components', async () => { + routes['/v1/objects/list'] = bulk(() => machine); + routes['/v1/objects/related'] = bulk(() => [ + { sourceRelationship: 'HasComponent', object: boiler }, + { sourceRelationship: 'IsComponentOf', object: plant }, + ]); + routes['/v1/objects/value'] = bulk(() => ({ + isComposition: true, + value: { Temperature: 92.5 }, + quality: 'Good', + timestamp: '2026-10-25T10:00:00Z', + })); + + const res = await handleInspectAsset( + new RestClientBinding('https://portal.example/api/i3x/abc/v1'), + { elementId: machine.elementId, includeValues: true, includeSchema: false }, + ); + expect(res).toMatchObject({ + elementId: machine.elementId, + displayName: 'CoffeeMachineA', + subComponents: [{ elementId: boiler.elementId }], + currentValues: { value: { Temperature: 92.5 } }, + }); + expect((res as { subComponents: unknown[] }).subComponents).toHaveLength(1); + expect(calls.find((c) => c.url.endsWith('/v1/objects/value'))?.body).toMatchObject({ + maxDepth: 2, + }); + }); +}); From 15844a9fbefa1e465c5fb2957e513b521416f040 Mon Sep 17 00:00:00 2001 From: Etienne Rossignon Date: Sun, 25 Oct 2026 21:39:58 +0100 Subject: [PATCH 2/3] fix(mcp): per-element results, maxDepth, honest monitor, real in-process routes Values/history now come back per element ({elementId, result|error}), so history reports keep their own elementId after a failure; stats are per series including components; GoodNoData is not bad; invalid or inverted times are errors. maxDepth (default 2) is exposed on inspect, telemetry and monitor, since an Object's readings are its components. Related results keep sourceRelationship; explore follows children by parentId. Suggestions match displayName and reuse the object list for 60 s. monitor_asset_events is a one-shot snapshot with quality alerts: it no longer claims a subscription. InProcessConnectorBinding now runs the real rest-server routes via inject() (createApp gains a logger option: Fastify logs to stdout, the MCP channel). Contract tests drive those routes; they fail on the previous sources. --- package-lock.json | 1 + packages/mcp/package.json | 1 + .../mcp/src/bindings/connector-binding.ts | 205 ++++-------------- packages/mcp/src/bindings/rest-binding.ts | 154 +++++++------ packages/mcp/src/bindings/types.ts | 28 ++- packages/mcp/src/server.ts | 23 +- packages/mcp/src/tools/explore-hierarchy.ts | 8 +- packages/mcp/src/tools/inspect-asset.ts | 68 +++--- packages/mcp/src/tools/monitor-events.ts | 49 +++-- packages/mcp/src/tools/query-telemetry.ts | 178 ++++++++------- packages/mcp/src/utils/similarity.ts | 25 +++ packages/mcp/tests/contract.test.ts | 189 ++++++++++++++++ packages/mcp/tests/mcp-server.test.ts | 179 +++++++++++++-- packages/mcp/tests/rest-binding.test.ts | 86 +++++++- packages/mcp/tsconfig.json | 3 +- packages/rest-server/src/app.ts | 12 +- packages/rest-server/src/index.ts | 2 +- 17 files changed, 812 insertions(+), 399 deletions(-) create mode 100644 packages/mcp/tests/contract.test.ts diff --git a/package-lock.json b/package-lock.json index 3bdd429..f977953 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7448,6 +7448,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.32.2", "@node-i3x/core": "0.11.2", + "@node-i3x/rest-server": "0.11.2", "zod": "^4.6.7" }, "bin": { diff --git a/packages/mcp/package.json b/packages/mcp/package.json index 943818a..22d82c5 100644 --- a/packages/mcp/package.json +++ b/packages/mcp/package.json @@ -57,6 +57,7 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.32.2", "@node-i3x/core": "0.11.2", + "@node-i3x/rest-server": "0.11.2", "zod": "^4.6.7" }, "devDependencies": { diff --git a/packages/mcp/src/bindings/connector-binding.ts b/packages/mcp/src/bindings/connector-binding.ts index ba8ed28..b9904da 100644 --- a/packages/mcp/src/bindings/connector-binding.ts +++ b/packages/mcp/src/bindings/connector-binding.ts @@ -1,169 +1,44 @@ -import type { - CurrentValueResult, - HistoricalValueResult, - I3xStack, - ModelNode, - Namespace, - ObjectInstanceResponse, - ObjectType, - RelationshipType, - ServerInfo, -} from '@node-i3x/core'; -import { rankSuggestions } from '../utils/similarity.js'; -import type { I3XClientBinding } from './types.js'; - -export class InProcessConnectorBinding implements I3XClientBinding { - constructor(private readonly stack: I3xStack) {} - - async getInfo(): Promise { - return { - specVersion: '1.0.0-beta', - serverVersion: '0.10.0', - serverName: 'node-i3x (In-Process Direct)', - capabilities: { - query: { history: true }, - update: { current: true, history: true }, - subscribe: { stream: true }, - }, - }; - } - - async getNamespaces(): Promise { - const model = await this.stack.modelService.getOrBuildModel(); - const uris = new Set(); - for (const node of model.nodesById.values()) { - if (node.namespaceUri) uris.add(node.namespaceUri); - } - return Array.from(uris).map((uri) => ({ - uri, - displayName: uri, - })); - } - - async getObjectTypes(namespaceUri?: string): Promise { - return this.stack.typeService.getObjectTypes(namespaceUri); - } - - async getRelationshipTypes(): Promise { - return [ - { - elementId: 'HasComponent', - displayName: 'Has Component', - namespaceUri: 'http://opcfoundation.org/UA/', - relationshipId: 'i=47', - reverseOf: 'ComponentOf', - }, - { - elementId: 'HasProperty', - displayName: 'Has Property', - namespaceUri: 'http://opcfoundation.org/UA/', - relationshipId: 'i=46', - reverseOf: 'PropertyOf', - }, - ]; - } - - async getObjects(options?: { - rootOnly?: boolean; - typeElementId?: string; - }): Promise { - const model = await this.stack.modelService.getOrBuildModel(); - let nodes: ModelNode[]; - - if (options?.rootOnly) { - nodes = model.rootIds - .map((id) => model.nodesById.get(id)) - .filter((n): n is ModelNode => n !== undefined && n.kind === 'instance'); - } else { - nodes = Array.from(model.nodesById.values()).filter( - (n): n is ModelNode => n.kind === 'instance' || n.kind === 'property', - ); - } - - if (options?.typeElementId) { - nodes = nodes.filter((n) => n.type === options.typeElementId); - } - - const rootSet = new Set(model.rootIds); - - return nodes.map((n) => ({ - elementId: n.id, - displayName: n.name, - typeElementId: n.type || 'BaseObjectType', - parentId: rootSet.has(n.id) ? null : undefined, - isComposition: n.kind === 'instance', - metadata: { - typeNamespaceUri: n.namespaceUri, - engUnit: n.engUnit || null, - sourceTypeId: n.sourceTypeId || null, - }, - })); - } - - async getObjectById(elementId: string): Promise { - const objects = await this.getObjects(); - return objects.find((o) => o.elementId === elementId) || null; - } - - async getRelatedObjects(elementId: string): Promise { - const model = await this.stack.modelService.getOrBuildModel(); - const node = model.nodesById.get(elementId); - if (!node?.children) return []; - - const children = node.children - .map((childId) => model.nodesById.get(childId)) - .filter((n): n is ModelNode => n !== undefined); - - return children.map((n) => ({ - elementId: n.id, - displayName: n.name, - typeElementId: n.type || 'BaseObjectType', - parentId: elementId, - isComposition: n.kind === 'instance', - metadata: { - typeNamespaceUri: n.namespaceUri, - engUnit: n.engUnit || null, - sourceTypeId: n.sourceTypeId || null, - }, - })); - } - - async getObjectValues( - elementIds: string[], - maxDepth = 1, - ): Promise { - const bulkResults = await this.stack.valueService.readValues(elementIds, maxDepth); - return bulkResults - .map((r) => r.result) - .filter((v): v is CurrentValueResult => v !== null && v !== undefined); +import type { RestServerDeps } from '@node-i3x/rest-server'; +import { RestClientBinding } from './rest-binding.js'; + +type InjectableApp = Awaited< + ReturnType +>; + +/** + * Binding to an i3X stack running in this process. + * + * Requests go through the real rest-server routes (Fastify `inject()`, no socket), + * so in-process answers are exactly what a remote node-i3x server would send: + * same parentId, roots, related objects, values and server info. + */ +export class InProcessConnectorBinding extends RestClientBinding { + constructor(deps: RestServerDeps) { + super('http://in-process', undefined, 15_000, injectFetch(deps)); } +} - async getHistoricalValues( - elementIds: string[], - startTime: string, - endTime: string, - maxDepth = 1, - ): Promise { - const start = startTime ? new Date(startTime) : null; - const end = endTime ? new Date(endTime) : null; - const bulkResults = await this.stack.historyService.readHistory( - elementIds, - start, - end, - maxDepth, +/** A fetch that answers from an in-process rest-server instead of the network. */ +export function injectFetch(deps: RestServerDeps): typeof fetch { + let app: Promise | undefined; + return async (input, init) => { + // Loaded lazily so the REST-only CLI never pulls in Fastify. No request logging: + // Fastify logs to stdout, which is the MCP stdio channel. + app ??= import('@node-i3x/rest-server').then(({ createApp }) => + createApp({ ...deps, requireAuth: false }, { logger: false }), ); - return bulkResults - .map((r) => r.result) - .filter((v): v is HistoricalValueResult => v !== null && v !== undefined); - } - - async findSuggestions(unknownId: string): Promise { - try { - const allObjects = await this.getObjects(); - const candidates = allObjects.map((o) => o.elementId); - return rankSuggestions(unknownId, candidates, 5, 0.4); - } catch { - return []; - } - } + const url = new URL(String(input)); + const res = await (await app).inject({ + method: (init?.method ?? 'GET') as 'GET' | 'POST', + url: `${url.pathname}${url.search}`, + headers: init?.headers as Record | undefined, + payload: typeof init?.body === 'string' ? init.body : undefined, + }); + return new Response(res.body, { + status: res.statusCode, + headers: { + 'content-type': String(res.headers['content-type'] ?? 'application/json'), + }, + }); + }; } diff --git a/packages/mcp/src/bindings/rest-binding.ts b/packages/mcp/src/bindings/rest-binding.ts index c76374e..6769401 100644 --- a/packages/mcp/src/bindings/rest-binding.ts +++ b/packages/mcp/src/bindings/rest-binding.ts @@ -1,5 +1,6 @@ import type { BulkResponse, + BulkResultItem, CurrentValueResult, HistoricalValueResult, Namespace, @@ -10,15 +11,29 @@ import type { ServerInfo, SuccessResponse, } from '@node-i3x/core'; -import { rankSuggestions } from '../utils/similarity.js'; -import type { I3XClientBinding } from './types.js'; +import { rankObjectSuggestions } from '../utils/similarity.js'; +import type { ElementResult, I3XClientBinding, Suggestion } from './types.js'; + +/** How long the full object list used for suggestions is reused. */ +const OBJECT_CACHE_TTL_MS = 60_000; export class RestClientBinding implements I3XClientBinding { private readonly baseUrl: string; private readonly headers: Record; private readonly requestTimeoutMs: number; + private readonly fetchImpl?: typeof fetch; + private objectCache?: { at: number; objects: Promise }; - constructor(baseUrl = 'http://localhost:3000', token?: string, timeoutMs = 15_000) { + /** + * @param fetchImpl - replaces the global fetch, e.g. to route requests into an + * in-process server (see InProcessConnectorBinding). + */ + constructor( + baseUrl = 'http://localhost:3000', + token?: string, + timeoutMs = 15_000, + fetchImpl?: typeof fetch, + ) { let cleanUrl = baseUrl.trim().replace(/\/+$/, ''); if (!cleanUrl.startsWith('http://') && !cleanUrl.startsWith('https://')) { cleanUrl = `http://${cleanUrl}`; @@ -27,6 +42,7 @@ export class RestClientBinding implements I3XClientBinding { cleanUrl = cleanUrl.replace(/\/v1$/i, ''); this.baseUrl = cleanUrl; this.requestTimeoutMs = timeoutMs; + this.fetchImpl = fetchImpl; this.headers = { 'Content-Type': 'application/json', Accept: 'application/json', @@ -41,7 +57,7 @@ export class RestClientBinding implements I3XClientBinding { const signal = options.signal || AbortSignal.timeout(this.requestTimeoutMs); try { - const response = await fetch(url, { + const response = await (this.fetchImpl ?? fetch)(url, { ...options, signal, headers: { @@ -69,11 +85,8 @@ export class RestClientBinding implements I3XClientBinding { } /** Unwrap a `{ success, result }` envelope; a `success: false` envelope becomes an error. */ - private async get(path: string, options: RequestInit = {}): Promise { - const res = await this.request> & ErrorEnvelope>( - path, - options, - ); + private async get(path: string): Promise { + const res = await this.request> & ErrorEnvelope>(path); if (res?.success !== true || !('result' in res)) { throw new Error(`i3X request ${path} failed: ${describeFailure(res)}`); } @@ -81,25 +94,11 @@ export class RestClientBinding implements I3XClientBinding { } /** - * Unwrap a bulk `{ success, results: [{ elementId, success, result }] }` envelope, - * one entry per requested elementId, in request order. A failed item becomes an - * error naming its elementId; `notFoundAsNull` maps a 404 item to `null` instead. + * POST a bulk request and return its `results` items, one per requested elementId. + * `success` is false on the envelope as soon as one item failed, so items are + * returned as they are; only a body without `results` is an error here. */ - private async getBulk( - path: string, - body: unknown, - notFoundAsNull?: false, - ): Promise; - private async getBulk( - path: string, - body: unknown, - notFoundAsNull: true, - ): Promise>; - private async getBulk( - path: string, - body: unknown, - notFoundAsNull = false, - ): Promise> { + private async postBulk(path: string, body: unknown): Promise[]> { const res = await this.request> & ErrorEnvelope>(path, { method: 'POST', body: JSON.stringify(body), @@ -107,15 +106,34 @@ export class RestClientBinding implements I3XClientBinding { if (!Array.isArray(res?.results)) { throw new Error(`i3X request ${path} failed: ${describeFailure(res)}`); } - return res.results.map((item) => { - if (item.success) return (item.result ?? null) as T | null; - if (notFoundAsNull && item.responseDetail?.status === 404) return null; - throw new Error( - `i3X request ${path} failed for '${item.elementId}': ${describeFailure(item)}`, - ); + return res.results; + } + + /** Bulk items as per-element results: a failed item keeps its elementId and error. */ + private async postBulkPerElement( + path: string, + elementIds: string[], + body: Record, + ): Promise[]> { + const items = await this.postBulk(path, { elementIds, ...body }); + return items.map((item, idx) => { + const elementId = item.elementId ?? elementIds[idx] ?? `element_${idx}`; + return item.success + ? { elementId, result: (item.result ?? undefined) as T | undefined } + : { elementId, error: describeFailure(item) }; }); } + /** The single item of a one-element bulk request. */ + private async postBulkOne( + path: string, + body: { elementIds: [string] } & Record, + ): Promise> { + const [item] = await this.postBulk(path, body); + if (!item) throw new Error(`i3X request ${path} returned no result`); + return item; + } + async getInfo(): Promise { return this.get('/v1/info'); } @@ -145,38 +163,46 @@ export class RestClientBinding implements I3XClientBinding { return this.get(`/v1/objects${query}`); } + /** `null` only when the server reports the element as not found (404). */ async getObjectById(elementId: string): Promise { - const [object] = await this.getBulk( - '/v1/objects/list', - { elementIds: [elementId] }, - true, + const path = '/v1/objects/list'; + const item = await this.postBulkOne(path, { + elementIds: [elementId], + }); + if (item.success) return item.result ?? null; + if (item.responseDetail?.status === 404) return null; + throw new Error( + `i3X request ${path} failed for '${elementId}': ${describeFailure(item)}`, ); - return object ?? null; } async getRelatedObjects( elementId: string, relationshipType?: string, - ): Promise { - const body: Record = { elementIds: [elementId] }; + ): Promise { + const path = '/v1/objects/related'; + const body: { elementIds: [string] } & Record = { + elementIds: [elementId], + }; if (relationshipType) body.relationshipType = relationshipType; - const [related] = await this.getBulk( - '/v1/objects/related', - body, - ); + const item = await this.postBulkOne(path, body); + if (!item.success) { + throw new Error( + `i3X request ${path} failed for '${elementId}': ${describeFailure(item)}`, + ); + } // Servers may ignore relationshipType, so filter here as well. - return (related ?? []) - .filter((r) => !relationshipType || r.sourceRelationship === relationshipType) - .map((r) => r.object); + return (item.result ?? []).filter( + (r) => !relationshipType || r.sourceRelationship === relationshipType, + ); } async getObjectValues( elementIds: string[], maxDepth = 1, - ): Promise { - return this.getBulk('/v1/objects/value', { - elementIds, + ): Promise[]> { + return this.postBulkPerElement('/v1/objects/value', elementIds, { maxDepth, }); } @@ -186,23 +212,25 @@ export class RestClientBinding implements I3XClientBinding { startTime: string, endTime: string, maxDepth = 1, - ): Promise { - return this.getBulk('/v1/objects/history', { + ): Promise[]> { + return this.postBulkPerElement( + '/v1/objects/history', elementIds, - startTime, - endTime, - maxDepth, - }); + { startTime, endTime, maxDepth }, + ); } - async findSuggestions(unknownId: string): Promise { - try { - const allObjects = await this.getObjects(); - const candidates = allObjects.map((o) => o.elementId); - return rankSuggestions(unknownId, candidates, 5, 0.4); - } catch { - return []; + /** Closest objects by elementId or displayName; a failure to list objects propagates. */ + async findSuggestions(query: string): Promise { + const now = Date.now(); + if (!this.objectCache || now - this.objectCache.at > OBJECT_CACHE_TTL_MS) { + const objects = this.getObjects(); + this.objectCache = { at: now, objects }; + objects.catch(() => { + if (this.objectCache?.objects === objects) this.objectCache = undefined; + }); } + return rankObjectSuggestions(query, await this.objectCache.objects); } } diff --git a/packages/mcp/src/bindings/types.ts b/packages/mcp/src/bindings/types.ts index 2353535..c1fe6e1 100644 --- a/packages/mcp/src/bindings/types.ts +++ b/packages/mcp/src/bindings/types.ts @@ -4,10 +4,24 @@ import type { Namespace, ObjectInstanceResponse, ObjectType, + RelatedObjectResult, RelationshipType, ServerInfo, } from '@node-i3x/core'; +/** Outcome of a bulk read for one requested elementId: its result, or why it failed. */ +export interface ElementResult { + elementId: string; + result?: T; + error?: string; +} + +/** A known object whose elementId or displayName is close to an unknown query. */ +export interface Suggestion { + elementId: string; + displayName?: string; +} + export interface AssetDiagnosticReport { elementId: string; displayName?: string; @@ -53,16 +67,22 @@ export interface I3XClientBinding { typeElementId?: string; }): Promise; getObjectById(elementId: string): Promise; + /** Related objects with their relationship (children, and the parent as a reverse one). */ getRelatedObjects( elementId: string, relationshipType?: string, - ): Promise; - getObjectValues(elementIds: string[], maxDepth?: number): Promise; + ): Promise; + /** One entry per requested elementId, in request order. */ + getObjectValues( + elementIds: string[], + maxDepth?: number, + ): Promise[]>; + /** One entry per requested elementId, in request order. */ getHistoricalValues( elementIds: string[], startTime: string, endTime: string, maxDepth?: number, - ): Promise; - findSuggestions(unknownId: string): Promise; + ): Promise[]>; + findSuggestions(query: string): Promise; } diff --git a/packages/mcp/src/server.ts b/packages/mcp/src/server.ts index 457022a..6d097fe 100644 --- a/packages/mcp/src/server.ts +++ b/packages/mcp/src/server.ts @@ -105,6 +105,11 @@ export class NodeI3xMcpServer { description: 'Whether to include the ObjectType JSON Schema definition (default: true)', }, + maxDepth: { + type: 'number', + description: + 'Value read depth through components: 1 = the element only, 2 = with its components (default), 0 = unlimited', + }, }, required: ['elementId'], }, @@ -155,7 +160,13 @@ export class NodeI3xMcpServer { }, endTime: { type: 'string', - description: 'ISO 8601 end time (defaults to now) for history mode', + description: + "ISO 8601 or relative duration (e.g. '-10m') end time (defaults to now) for history mode", + }, + maxDepth: { + type: 'number', + description: + 'Value read depth through components: 1 = the element only, 2 = with its components (default), 0 = unlimited', }, aggregateStats: { type: 'boolean', @@ -189,19 +200,19 @@ export class NodeI3xMcpServer { { name: 'i3x_monitor_asset_events', description: - 'Create an i3X subscription stream to monitor real-time VQT value changes or threshold alert conditions.', + 'One-shot health check of assets: current VQT values plus an alert list of every series whose quality is not Good. Does not subscribe; call again to re-check.', inputSchema: { type: 'object', properties: { elementIds: { type: 'array', items: { type: 'string' }, - description: 'Array of asset elementIds to monitor', + description: 'Array of asset elementIds to check', }, - condition: { - type: 'string', + maxDepth: { + type: 'number', description: - "Optional alert condition filter (e.g. 'temperature > 85 || quality != Good')", + 'Value read depth through components: 1 = the element only, 2 = with its components (default), 0 = unlimited', }, }, required: ['elementIds'], diff --git a/packages/mcp/src/tools/explore-hierarchy.ts b/packages/mcp/src/tools/explore-hierarchy.ts index db5435a..40cc8bb 100644 --- a/packages/mcp/src/tools/explore-hierarchy.ts +++ b/packages/mcp/src/tools/explore-hierarchy.ts @@ -51,8 +51,12 @@ export async function handleExploreHierarchy( visited.add(asset.elementId); if (currentDepth < maxDepth) { - const children = await binding.getRelatedObjects(asset.elementId); - const unvisitedChildren = children.filter((c) => !visited.has(c.elementId)); + // Follow children only: related objects also include the parent (reverse + // relationship), and parentId is what the spec ties to the hierarchy. + const related = await binding.getRelatedObjects(asset.elementId); + const unvisitedChildren = related + .map((r) => r.object) + .filter((c) => c.parentId === asset.elementId && !visited.has(c.elementId)); if (unvisitedChildren.length > 0) { node.children = await Promise.all( diff --git a/packages/mcp/src/tools/inspect-asset.ts b/packages/mcp/src/tools/inspect-asset.ts index 63aa086..eee089c 100644 --- a/packages/mcp/src/tools/inspect-asset.ts +++ b/packages/mcp/src/tools/inspect-asset.ts @@ -15,13 +15,22 @@ export const inspectAssetSchema = z.object({ .optional() .default(true) .describe('Whether to include the ObjectType JSON Schema definition'), + maxDepth: z + .number() + .int() + .min(0) + .optional() + .default(2) + .describe( + 'Value read depth through components: 1 = the asset only (an Object usually has no value of its own), 2 = with its components (default), 0 = unlimited', + ), }); export async function handleInspectAsset( binding: I3XClientBinding, args: z.infer, ) { - const { elementId, includeValues, includeSchema } = args; + const { elementId, includeValues, includeSchema, maxDepth } = args; const asset = await binding.getObjectById(elementId); if (!asset) { @@ -38,8 +47,10 @@ export async function handleInspectAsset( displayName: asset.displayName || asset.elementId, description: asset.metadata?.description || undefined, typeElementId: asset.typeElementId, + parentId: asset.parentId ?? null, isRoot: !asset.parentId, }; + const warnings: string[] = []; // Run sub-components, schema, and telemetry fetches concurrently for maximum speed & fault-tolerance const relatedPromise = binding.getRelatedObjects(elementId); @@ -47,9 +58,8 @@ export async function handleInspectAsset( includeSchema && asset.typeElementId ? binding.getObjectTypes() : Promise.resolve(null); - // maxDepth 2: an Object has no value of its own, its readings are its components. const valuesPromise = includeValues - ? binding.getObjectValues([elementId], 2) + ? binding.getObjectValues([elementId], maxDepth) : Promise.resolve(null); const [relatedResult, schemaResult, valuesResult] = await Promise.allSettled([ @@ -58,23 +68,26 @@ export async function handleInspectAsset( valuesPromise, ]); - // Sub-components - // Related objects include the parent (reverse relationship): it is not a sub-component. - const subComponents = - relatedResult.status === 'fulfilled' && Array.isArray(relatedResult.value) - ? relatedResult.value.filter((r) => r.elementId !== asset.parentId) - : []; - if (subComponents.length > 0) { - result.subComponents = subComponents.map((r) => ({ - elementId: r.elementId, - displayName: r.displayName || r.elementId, - typeElementId: r.typeElementId, - })); + // Sub-components: related objects also include the parent (a reverse relationship). + if (relatedResult.status === 'fulfilled') { + const subComponents = relatedResult.value.filter( + (r) => r.object.elementId !== asset.parentId, + ); + if (subComponents.length > 0) { + result.subComponents = subComponents.map((r) => ({ + elementId: r.object.elementId, + displayName: r.object.displayName || r.object.elementId, + typeElementId: r.object.typeElementId, + relationship: r.sourceRelationship, + })); + } + } else { + warnings.push(`Sub-components unavailable: ${errorMessage(relatedResult.reason)}`); } // Schema definition - if (schemaResult.status === 'fulfilled' && Array.isArray(schemaResult.value)) { - const targetType = schemaResult.value.find( + if (schemaResult.status === 'fulfilled') { + const targetType = schemaResult.value?.find( (t) => t.elementId === asset.typeElementId, ); if (targetType) { @@ -84,18 +97,23 @@ export async function handleInspectAsset( schema: targetType.schema, }; } + } else { + warnings.push(`Type schema unavailable: ${errorMessage(schemaResult.reason)}`); } // Current telemetry values - if ( - valuesResult.status === 'fulfilled' && - Array.isArray(valuesResult.value) && - valuesResult.value.length > 0 - ) { - result.currentValues = valuesResult.value[0]; - } else if (valuesResult.status === 'rejected') { - result.currentValuesWarning = 'Telemetry data temporarily unavailable'; + if (valuesResult.status === 'fulfilled') { + const entry = valuesResult.value?.[0]; + if (entry?.result) result.currentValues = entry.result; + else if (entry?.error) warnings.push(`Telemetry unavailable: ${entry.error}`); + } else { + warnings.push(`Telemetry unavailable: ${errorMessage(valuesResult.reason)}`); } + if (warnings.length > 0) result.warnings = warnings; return result; } + +function errorMessage(reason: unknown): string { + return reason instanceof Error ? reason.message : String(reason); +} diff --git a/packages/mcp/src/tools/monitor-events.ts b/packages/mcp/src/tools/monitor-events.ts index 33e0244..600f9fa 100644 --- a/packages/mcp/src/tools/monitor-events.ts +++ b/packages/mcp/src/tools/monitor-events.ts @@ -5,30 +5,53 @@ export const monitorEventsSchema = z.object({ elementIds: z .array(z.string()) .min(1) - .describe("Array of asset elementIds to monitor (e.g. ['Boiler_101', 'Pump_101'])"), - condition: z - .string() + .describe("Array of asset elementIds to check (e.g. ['Boiler_101', 'Pump_101'])"), + maxDepth: z + .number() + .int() + .min(0) .optional() + .default(2) .describe( - "Optional monitoring alert condition filter (e.g. 'temperature > 85 || quality != Good')", + 'Read depth through components: 1 = the element only, 2 = with its components (default), 0 = unlimited', ), }); +/** + * One-shot health snapshot: current values plus every series whose quality is not Good. + * It does not subscribe: MCP tool calls are request/response, so call it again to re-check. + */ export async function handleMonitorEvents( binding: I3XClientBinding, args: z.infer, ) { - const { elementIds, condition } = args; + const { elementIds, maxDepth } = args; + const snapshot = await binding.getObjectValues(elementIds, maxDepth); - // Snapshot current state first - const currentValues = await binding.getObjectValues(elementIds); + const alerts: Array<{ elementId: string; quality?: string; error?: string }> = []; + for (const { elementId, result, error } of snapshot) { + if (error) { + alerts.push({ elementId, error }); + continue; + } + if (result && result.value !== null && !isGood(result.quality)) { + alerts.push({ elementId, quality: result.quality }); + } + for (const [componentId, vqt] of Object.entries(result?.components ?? {})) { + if (!isGood(vqt.quality)) + alerts.push({ elementId: componentId, quality: vqt.quality }); + } + } return { - status: 'monitoring_active', - monitoredAssets: elementIds, - alertCondition: condition || 'All value or quality changes', - initialSnapshot: currentValues, - instructions: - 'Subscription stream registered. The MCP server will emit notification events on matching VQT telemetry updates or threshold breaches.', + status: 'snapshot', + checkedAt: new Date().toISOString(), + alerts, + snapshot, + note: 'Point-in-time read; no subscription is created. Call again to re-check.', }; } + +function isGood(quality: unknown): boolean { + return String(quality).toLowerCase().startsWith('good'); +} diff --git a/packages/mcp/src/tools/query-telemetry.ts b/packages/mcp/src/tools/query-telemetry.ts index bb5afc6..a4072ef 100644 --- a/packages/mcp/src/tools/query-telemetry.ts +++ b/packages/mcp/src/tools/query-telemetry.ts @@ -1,3 +1,4 @@ +import type { VQT } from '@node-i3x/core'; import { z } from 'zod'; import type { I3XClientBinding } from '../bindings/types.js'; @@ -17,12 +18,23 @@ export const queryTelemetrySchema = z.object({ .string() .optional() .describe( - "ISO 8601 start time or relative time string (e.g. '-2h', '-30m', '-1d') for history mode", + "ISO 8601 start time or relative time string (e.g. '-2h', '-30m', '-1d') for history mode (default: -1h)", ), endTime: z .string() .optional() - .describe('ISO 8601 end time (defaults to now) for history mode'), + .describe( + "ISO 8601 end time or relative time string (e.g. '-10m') for history mode (default: now)", + ), + maxDepth: z + .number() + .int() + .min(0) + .optional() + .default(2) + .describe( + 'Read depth through components: 1 = the element only, 2 = with its components (default), 0 = unlimited', + ), aggregateStats: z .boolean() .optional() @@ -32,58 +44,73 @@ export const queryTelemetrySchema = z.object({ ), }); -function parseStartTime(input?: string): string { - if (!input) { - const d = new Date(); - d.setHours(d.getHours() - 1); - return d.toISOString(); - } - - const match = input.match(/^-(\d+)([smhdw])$/i); +const RELATIVE_TIME = /^-(\d+)([smhdw])$/i; +const UNIT_MS: Record = { + s: 1_000, + m: 60_000, + h: 3_600_000, + d: 86_400_000, + w: 604_800_000, +}; + +/** ISO 8601 or relative ('-2h') time to ISO; an unparseable value is an error, not a default. */ +export function parseTime( + input: string | undefined, + fallback: Date, + name: string, +): string { + if (!input) return fallback.toISOString(); + + const match = input.match(RELATIVE_TIME); if (match) { - const amount = parseInt(match[1]!, 10); - const unit = match[2]!.toLowerCase(); - const d = new Date(); - - switch (unit) { - case 's': - d.setSeconds(d.getSeconds() - amount); - break; - case 'm': - d.setMinutes(d.getMinutes() - amount); - break; - case 'h': - d.setHours(d.getHours() - amount); - break; - case 'd': - d.setDate(d.getDate() - amount); - break; - case 'w': - d.setDate(d.getDate() - amount * 7); - break; - } - return d.toISOString(); + const amount = Number.parseInt(match[1]!, 10); + return new Date( + Date.now() - amount * UNIT_MS[match[2]!.toLowerCase()]!, + ).toISOString(); } - // Fallback to standard ISO Date parsing const parsed = new Date(input); if (Number.isNaN(parsed.getTime())) { - const defaultDate = new Date(); - defaultDate.setHours(defaultDate.getHours() - 1); - return defaultDate.toISOString(); + throw new Error( + `Invalid ${name} '${input}': expected ISO 8601 (e.g. '2026-10-25T10:00:00Z') or a relative time (e.g. '-2h').`, + ); } - return parsed.toISOString(); } +/** GoodNoData is a good status (no data in range), not a bad sample. */ +function isBadQuality(quality: unknown): boolean { + return quality != null && !String(quality).toLowerCase().startsWith('good'); +} + +function seriesStatistics(values: readonly VQT[]) { + const numericValues: number[] = []; + let badQualityCount = 0; + for (const v of values) { + if (isBadQuality(v.quality)) badQualityCount++; + if (typeof v.value === 'number' && !Number.isNaN(v.value)) + numericValues.push(v.value); + } + if (numericValues.length === 0) return { count: values.length, badQualityCount }; + + const sum = numericValues.reduce((a, b) => a + b, 0); + return { + min: Math.min(...numericValues), + max: Math.max(...numericValues), + mean: Math.round((sum / numericValues.length) * 100) / 100, + count: numericValues.length, + badQualityCount, + }; +} + export async function handleQueryTelemetry( binding: I3XClientBinding, args: z.infer, ) { - const { elementIds, mode, aggregateStats } = args; + const { elementIds, mode, aggregateStats, maxDepth } = args; if (mode === 'current') { - const values = await binding.getObjectValues(elementIds); + const values = await binding.getObjectValues(elementIds, maxDepth); return { mode: 'current', telemetry: values, @@ -91,13 +118,23 @@ export async function handleQueryTelemetry( } // History mode - const startTime = parseStartTime(args.startTime); - const endTimeDate = args.endTime ? new Date(args.endTime) : new Date(); - const endTime = Number.isNaN(endTimeDate.getTime()) - ? new Date().toISOString() - : endTimeDate.toISOString(); + const now = new Date(); + const startTime = parseTime( + args.startTime, + new Date(now.getTime() - UNIT_MS.h!), + 'startTime', + ); + const endTime = parseTime(args.endTime, now, 'endTime'); + if (startTime > endTime) { + throw new Error(`startTime ${startTime} is after endTime ${endTime}.`); + } - const history = await binding.getHistoricalValues(elementIds, startTime, endTime); + const history = await binding.getHistoricalValues( + elementIds, + startTime, + endTime, + maxDepth, + ); if (!aggregateStats) { return { @@ -107,47 +144,24 @@ export async function handleQueryTelemetry( }; } - // Calculate pre-computed statistical aggregates - const reports = history.map((record, idx) => { - const elementId = elementIds[idx] || `element_${idx}`; + // Statistics per series: the element's own values and each component separately, + // since mixing e.g. a temperature with a pressure is meaningless. + const reports = history.map(({ elementId, result, error }) => { + if (error || !result) return { elementId, error: error ?? 'No history returned' }; + const report: Record = { elementId, - timeRange: { startTime, endTime }, - dataPoints: record.values || [], + dataPoints: result.values ?? [], + statistics: seriesStatistics(result.values ?? []), }; - - const numericValues: number[] = []; - let badQualityCount = 0; - - for (const v of record.values || []) { - if (v.quality && String(v.quality).toUpperCase() !== 'GOOD') { - badQualityCount++; - } - if (typeof v.value === 'number' && !Number.isNaN(v.value)) { - numericValues.push(v.value); - } + if (result.components) { + report.components = Object.fromEntries( + Object.entries(result.components).map(([componentId, { values }]) => [ + componentId, + { dataPoints: values ?? [], statistics: seriesStatistics(values ?? []) }, + ]), + ); } - - if (numericValues.length > 0) { - const min = Math.min(...numericValues); - const max = Math.max(...numericValues); - const sum = numericValues.reduce((a, b) => a + b, 0); - const mean = Math.round((sum / numericValues.length) * 100) / 100; - - report.statistics = { - min, - max, - mean, - count: numericValues.length, - badQualityCount, - }; - } else { - report.statistics = { - count: (record.values || []).length, - badQualityCount, - }; - } - return report; }); diff --git a/packages/mcp/src/utils/similarity.ts b/packages/mcp/src/utils/similarity.ts index c1a6319..f16200a 100644 --- a/packages/mcp/src/utils/similarity.ts +++ b/packages/mcp/src/utils/similarity.ts @@ -56,3 +56,28 @@ export function rankSuggestions( .slice(0, topN) .map((item) => item.candidate); } + +/** + * Ranks objects against a query by the better of their elementId and displayName + * scores: elementIds are often opaque hashes, so names are what users type. + */ +export function rankObjectSuggestions( + query: string, + objects: ReadonlyArray<{ elementId: string; displayName?: string }>, + topN = 5, + minScore = 0.4, +): Array<{ elementId: string; displayName?: string }> { + return objects + .map(({ elementId, displayName }) => ({ + elementId, + displayName, + score: Math.max( + computeSimilarity(query, elementId), + displayName ? computeSimilarity(query, displayName) : 0, + ), + })) + .filter((item) => item.score >= minScore) + .sort((a, b) => b.score - a.score) + .slice(0, topN) + .map(({ elementId, displayName }) => ({ elementId, displayName })); +} diff --git a/packages/mcp/tests/contract.test.ts b/packages/mcp/tests/contract.test.ts new file mode 100644 index 0000000..efb292d --- /dev/null +++ b/packages/mcp/tests/contract.test.ts @@ -0,0 +1,189 @@ +// Contract tests: the MCP client against the real node-i3x rest-server routes, +// in-process through Fastify inject() (no port), instead of a hand-written mock. +import { createI3xStack, nullLogger } from '@node-i3x/core'; +import type { RestServerDeps } from '@node-i3x/rest-server'; +import { beforeAll, describe, expect, it } from 'vitest'; +import { MockDataSource } from '../../rest-server/tests/helpers/mock-data-source.js'; +import { + InProcessConnectorBinding, + injectFetch, +} from '../src/bindings/connector-binding.js'; +import { RestClientBinding } from '../src/bindings/rest-binding.js'; +import { handleExploreHierarchy } from '../src/tools/explore-hierarchy.js'; +import { handleInspectAsset } from '../src/tools/inspect-asset.js'; + +// Machine (root) ── Temperature, Reset, Motor ── Speed +function makeDeps(extra: Partial = {}): RestServerDeps { + const ds = new MockDataSource(); + ds.values['ns=2;s=Speed'] = 1500; + ds.extraNodes = [ + { + sourceNodeId: 'ns=2;s=Motor', + parentSourceNodeId: 'ns=2;s=Machine', + browseName: 'Motor', + nsuQualifiedName: 'nsu=http://example.com/:Motor', + displayName: 'Motor', + nodeClass: 'Object', + typeDefinition: null, + namespaceUri: 'http://example.com/', + eventNotifier: false, + }, + { + sourceNodeId: 'ns=2;s=Speed', + parentSourceNodeId: 'ns=2;s=Motor', + browseName: 'Speed', + nsuQualifiedName: 'nsu=http://example.com/:Speed', + displayName: 'Speed', + nodeClass: 'Variable', + typeDefinition: 'Double', + dataType: 'Double', + dataTypeName: 'Double', + namespaceUri: 'http://example.com/', + eventNotifier: false, + }, + ]; + return { + ...createI3xStack(ds, nullLogger), + dataSource: ds, + logger: nullLogger, + ...extra, + }; +} + +describe('MCP client against the real rest-server', () => { + let binding: InProcessConnectorBinding; + let machineId: string; + let motorId: string; + + beforeAll(async () => { + binding = new InProcessConnectorBinding(makeDeps()); + const all = await binding.getObjects(); + machineId = all.find((o) => o.displayName === 'Machine')!.elementId; + motorId = all.find((o) => o.displayName === 'Motor')!.elementId; + }); + + it('unwraps the envelope: roots have parentId null, children their parent', async () => { + const roots = await binding.getObjects({ rootOnly: true }); + expect(roots.map((o) => [o.displayName, o.parentId])).toEqual([['Machine', null]]); + const motor = await binding.getObjectById(motorId); + expect(motor?.parentId).toBe(machineId); + }); + + it('reports the real server info', async () => { + const info = await binding.getInfo(); + expect(info).toMatchObject({ + serverName: 'node-i3x', + capabilities: { update: { history: false } }, + }); + }); + + it('maps a 404 item to null and lists objects by displayName in suggestions', async () => { + expect(await binding.getObjectById('instance-does-not-exist')).toBeNull(); + const res = await handleInspectAsset(binding, { + elementId: 'Motr', + includeValues: false, + includeSchema: false, + maxDepth: 2, + }); + expect(res).toMatchObject({ + error: "Asset 'Motr' not found in i3X address space.", + suggestions: [{ elementId: motorId, displayName: 'Motor' }], + }); + }); + + it('keeps the relationship, and the parent out of sub-components and the tree', async () => { + const related = await binding.getRelatedObjects(motorId); + expect(related.map((r) => [r.sourceRelationship, r.object.displayName])).toEqual([ + ['HasComponent', 'Speed'], + ['IsComponentOf', 'Machine'], + ]); + + const inspected = await handleInspectAsset(binding, { + elementId: motorId, + includeValues: false, + includeSchema: false, + maxDepth: 2, + }); + expect(inspected).toMatchObject({ + parentId: machineId, + subComponents: [{ displayName: 'Speed', relationship: 'HasComponent' }], + }); + + const tree = await handleExploreHierarchy(binding, { maxDepth: 3 }); + const motor = tree.plantTopology?.[0]?.children?.find( + (c) => c.displayName === 'Motor', + ); + expect(motor?.children?.map((c) => c.displayName)).toEqual(['Speed']); + }); + + it('reads component values at maxDepth 2, none at maxDepth 1', async () => { + const [deep] = await binding.getObjectValues([motorId], 2); + expect(deep?.elementId).toBe(motorId); + expect(Object.values(deep?.result?.components ?? {})).toEqual([ + expect.objectContaining({ value: 1500, quality: 'Good' }), + ]); + const [shallow] = await binding.getObjectValues([motorId], 1); + expect(shallow?.result?.components ?? null).toBeNull(); + + const inspected = await handleInspectAsset(binding, { + elementId: motorId, + includeValues: true, + includeSchema: false, + maxDepth: 2, + }); + expect(inspected).toMatchObject({ currentValues: { isComposition: true } }); + }); + + it('pairs bulk value results with their elementId, failures included', async () => { + const res = await binding.getObjectValues(['instance-does-not-exist', motorId], 2); + expect(res.map((r) => [r.elementId, Boolean(r.result), Boolean(r.error)])).toEqual([ + ['instance-does-not-exist', false, true], + [motorId, true, false], + ]); + }); + + it.each(['http://i3x.example/api/i3x/abc', 'http://i3x.example/api/i3x/abc/v1'])( + 'a remote client reaches the routes with base URL %s', + async (baseUrl) => { + const deps = makeDeps(); + // The rest-server is mounted at the root; drop the portal-style prefix. + const inject = injectFetch(deps); + const viaPrefix: typeof fetch = (input, init) => + inject(String(input).replace('/api/i3x/abc', ''), init); + const client = new RestClientBinding(baseUrl, undefined, 15_000, viaPrefix); + const roots = await client.getObjects({ rootOnly: true }); + expect(roots.map((o) => o.displayName)).toEqual(['Machine']); + }, + ); + + it('surfaces an auth failure instead of "not found"', async () => { + // injectFetch disables requireAuth for in-process use, so build a remote-style + // client that sends no token to an app that requires one. + const { createApp } = await import('@node-i3x/rest-server'); + const app = await createApp(makeDeps({ apiKey: 'secret' }), { logger: false }); + const noToken: typeof fetch = async (input, init) => { + const url = new URL(String(input)); + const res = await app.inject({ + method: (init?.method ?? 'GET') as 'GET' | 'POST', + url: `${url.pathname}${url.search}`, + payload: typeof init?.body === 'string' ? init.body : undefined, + headers: { 'content-type': 'application/json' }, + }); + return new Response(res.body, { status: res.statusCode }); + }; + const client = new RestClientBinding( + 'http://i3x.example', + undefined, + 15_000, + noToken, + ); + await expect( + handleInspectAsset(client, { + elementId: 'whatever', + includeValues: true, + includeSchema: true, + maxDepth: 2, + }), + ).rejects.toThrow(/HTTP 401/); + }); +}); diff --git a/packages/mcp/tests/mcp-server.test.ts b/packages/mcp/tests/mcp-server.test.ts index 9ff018d..b17e6f1 100644 --- a/packages/mcp/tests/mcp-server.test.ts +++ b/packages/mcp/tests/mcp-server.test.ts @@ -79,9 +79,14 @@ class MockClientBinding implements I3XClientBinding { if (elementId === 'Boiler_101') { return [ { - elementId: 'Pump_101', - displayName: 'Feed Pump 101', - typeElementId: 'PumpType', + sourceRelationship: 'HasComponent', + object: { + elementId: 'Pump_101', + displayName: 'Feed Pump 101', + typeElementId: 'PumpType', + parentId: 'Boiler_101', + isComposition: false, + }, }, ]; } @@ -89,26 +94,36 @@ class MockClientBinding implements I3XClientBinding { } async getObjectValues(elementIds: string[]) { - return elementIds.map((id) => ({ - isComposition: true, - value: id === 'Boiler_101' ? { Temperature: 78.4, Pressure: 3.2 } : { Speed: 1500 }, - quality: 'Good' as const, - timestamp: new Date().toISOString(), + return elementIds.map((elementId) => ({ + elementId, + result: { + isComposition: true, + value: + elementId === 'Boiler_101' + ? { Temperature: 78.4, Pressure: 3.2 } + : { Speed: 1500 }, + quality: 'Good' as const, + timestamp: new Date().toISOString(), + }, })); } async getHistoricalValues(elementIds: string[]) { - return elementIds.map(() => ({ - isComposition: true, - values: [ - { timestamp: '2026-10-25T10:00:00Z', value: 75.0, quality: 'Good' as const }, - { timestamp: '2026-10-25T11:00:00Z', value: 85.5, quality: 'Good' as const }, - ], + return elementIds.map((elementId) => ({ + elementId, + result: { + isComposition: true, + values: [ + { timestamp: '2026-10-25T10:00:00Z', value: 75.0, quality: 'Good' as const }, + { timestamp: '2026-10-25T11:00:00Z', value: 85.5, quality: 'Good' as const }, + ], + }, })); } async findSuggestions(unknownId: string) { - if (unknownId.includes('Boiler')) return ['Boiler_101']; + if (unknownId.includes('Boiler')) + return [{ elementId: 'Boiler_101', displayName: 'Boiler 101' }]; return []; } } @@ -121,6 +136,7 @@ describe('@node-i3x/mcp High-Value Tools', () => { elementId: 'Boiler_101', includeValues: true, includeSchema: true, + maxDepth: 2, }); expect(res).toHaveProperty('elementId', 'Boiler_101'); @@ -135,10 +151,13 @@ describe('@node-i3x/mcp High-Value Tools', () => { elementId: 'Boiler_999', includeValues: true, includeSchema: true, + maxDepth: 2, }); expect(res).toHaveProperty('error'); - expect(res).toHaveProperty('suggestions', ['Boiler_101']); + expect(res).toHaveProperty('suggestions', [ + { elementId: 'Boiler_101', displayName: 'Boiler 101' }, + ]); }); it('i3x_explore_plant_hierarchy traverses ISA-95 topology tree', async () => { @@ -155,6 +174,7 @@ describe('@node-i3x/mcp High-Value Tools', () => { mode: 'history', startTime: '-2h', aggregateStats: true, + maxDepth: 2, }); expect(res.mode).toBe('history'); @@ -167,6 +187,85 @@ describe('@node-i3x/mcp High-Value Tools', () => { }); }); + it('i3x_query_telemetry pairs each report with its own elementId after a failure', async () => { + const binding = new MockClientBinding(); + binding.getHistoricalValues = async () => [ + { elementId: 'Missing', error: '404 Error Not found' }, + { + elementId: 'Boiler_101', + result: { + isComposition: true, + values: [], + components: { + Temp: { + values: [ + { + timestamp: '2026-10-25T10:00:00Z', + value: 10, + quality: 'Good' as const, + }, + { + timestamp: '2026-10-25T11:00:00Z', + value: 20, + quality: 'GoodNoData' as const, + }, + { + timestamp: '2026-10-25T12:00:00Z', + value: 'x', + quality: 'Bad' as const, + }, + ], + }, + }, + }, + }, + ]; + const res = await handleQueryTelemetry(binding, { + elementIds: ['Missing', 'Boiler_101'], + mode: 'history', + aggregateStats: true, + maxDepth: 2, + }); + + expect(res.telemetryReports?.[0]).toEqual({ + elementId: 'Missing', + error: '404 Error Not found', + }); + expect(res.telemetryReports?.[1]).toMatchObject({ + elementId: 'Boiler_101', + statistics: { count: 0, badQualityCount: 0 }, + components: { + Temp: { + statistics: { min: 10, max: 20, mean: 15, count: 2, badQualityCount: 1 }, + }, + }, + }); + }); + + it('i3x_query_telemetry rejects unparseable or inverted time ranges', async () => { + const base = { + elementIds: ['Boiler_101'], + mode: 'history' as const, + aggregateStats: true, + maxDepth: 2, + }; + await expect( + handleQueryTelemetry(mockBinding, { ...base, startTime: 'yesterday' }), + ).rejects.toThrow(/Invalid startTime 'yesterday'/); + await expect( + handleQueryTelemetry(mockBinding, { ...base, endTime: 'soon' }), + ).rejects.toThrow(/Invalid endTime 'soon'/); + await expect( + handleQueryTelemetry(mockBinding, { ...base, startTime: '-1h', endTime: '-2h' }), + ).rejects.toThrow(/is after endTime/); + const ok = await handleQueryTelemetry(mockBinding, { + ...base, + startTime: '-2h', + endTime: '-1h', + }); + expect(ok.mode).toBe('history'); + }); + it('i3x_discover_types_and_schemas lists ObjectTypes and schema field rules', async () => { const res = await handleDiscoverTypes(mockBinding, {}); @@ -175,14 +274,46 @@ describe('@node-i3x/mcp High-Value Tools', () => { expect(res.objectTypes[0].requiredProperties).toEqual(['Temperature', 'Pressure']); }); - it('i3x_monitor_asset_events sets up subscription stream snapshot', async () => { + it('i3x_monitor_asset_events returns a snapshot and claims no subscription', async () => { const res = await handleMonitorEvents(mockBinding, { elementIds: ['Boiler_101'], - condition: 'temperature > 80', + maxDepth: 2, }); - expect(res.status).toBe('monitoring_active'); - expect(res.monitoredAssets).toEqual(['Boiler_101']); + expect(res.status).toBe('snapshot'); + expect(res.alerts).toEqual([]); + expect(res.snapshot[0]?.elementId).toBe('Boiler_101'); + }); + + it('i3x_monitor_asset_events alerts on non-Good components and failed elements', async () => { + const binding = new MockClientBinding(); + binding.getObjectValues = async () => [ + { + elementId: 'Boiler_101', + result: { + isComposition: true, + value: null, + quality: 'GoodNoData' as const, + timestamp: '2026-10-25T10:00:00Z', + components: { + Temp: { + value: 99, + quality: 'Bad' as const, + timestamp: '2026-10-25T10:00:00Z', + }, + }, + }, + }, + { elementId: 'Nope', error: '404 Error Not found' }, + ]; + const res = await handleMonitorEvents(binding, { + elementIds: ['Boiler_101', 'Nope'], + maxDepth: 2, + }); + expect(res.alerts).toEqual([ + { elementId: 'Temp', quality: 'Bad' }, + { elementId: 'Nope', error: '404 Error Not found' }, + ]); }); it('similarity and ranking utility ranks typos higher than unrelated strings', async () => { @@ -208,14 +339,14 @@ describe('@node-i3x/mcp High-Value Tools', () => { elementId: 'Boiler_101', includeValues: true, includeSchema: true, + maxDepth: 2, }); expect(res).toHaveProperty('elementId', 'Boiler_101'); expect(res).toHaveProperty('displayName', 'Boiler 101'); - expect(res).toHaveProperty( - 'currentValuesWarning', - 'Telemetry data temporarily unavailable', - ); + expect(res).toHaveProperty('warnings', [ + 'Telemetry unavailable: Sensor gateway timeout', + ]); }); it('i3x_connect dynamically switches endpoint and verifies connection', async () => { diff --git a/packages/mcp/tests/rest-binding.test.ts b/packages/mcp/tests/rest-binding.test.ts index dbf1e1e..e79d11c 100644 --- a/packages/mcp/tests/rest-binding.test.ts +++ b/packages/mcp/tests/rest-binding.test.ts @@ -61,6 +61,20 @@ const bulk = (fn: (id: string) => unknown) => (body?: Record) = }); describe('RestClientBinding', () => { + it('suggests by displayName for opaque ids and caches the object list', async () => { + let listed = 0; + routes['/v1/objects'] = () => { + listed++; + return { json: { success: true, result: [plant, machine, boiler] } }; + }; + const c = new RestClientBinding('https://portal.example/api/i3x/abc'); + expect(await c.findSuggestions('CoffeeMachine')).toEqual([ + { elementId: machine.elementId, displayName: 'CoffeeMachineA' }, + ]); + await c.findSuggestions('Boilr'); + expect(listed).toBe(1); + }); + describe('base URL', () => { it.each([ 'https://portal.example/api/i3x/abc', @@ -112,10 +126,13 @@ describe('RestClientBinding', () => { expect(await c.getObjectById(machine.elementId)).toEqual(machine); expect(await c.getObjectValues([machine.elementId])).toEqual([ { - isComposition: false, - value: 92.5, - quality: 'Good', - timestamp: '2026-10-25T10:00:00Z', + elementId: machine.elementId, + result: { + isComposition: false, + value: 92.5, + quality: 'Good', + timestamp: '2026-10-25T10:00:00Z', + }, }, ]); const history = await c.getHistoricalValues( @@ -123,7 +140,8 @@ describe('RestClientBinding', () => { '2026-10-25T00:00:00Z', '2026-10-25T12:00:00Z', ); - expect(history[0]?.values).toHaveLength(1); + expect(history[0]?.elementId).toBe(machine.elementId); + expect(history[0]?.result?.values).toHaveLength(1); }); it('sends elementIds to /v1/objects/related and reads the related objects', async () => { @@ -133,10 +151,13 @@ describe('RestClientBinding', () => { ]); const c = client(); - expect(await c.getRelatedObjects(machine.elementId)).toEqual([boiler, plant]); + expect(await c.getRelatedObjects(machine.elementId)).toEqual([ + { sourceRelationship: 'HasComponent', object: boiler }, + { sourceRelationship: 'IsComponentOf', object: plant }, + ]); expect(calls[0]?.body).toEqual({ elementIds: [machine.elementId] }); expect(await c.getRelatedObjects(machine.elementId, 'HasComponent')).toEqual([ - boiler, + { sourceRelationship: 'HasComponent', object: boiler }, ]); }); @@ -177,8 +198,35 @@ describe('RestClientBinding', () => { await expect(client().getNamespaces()).rejects.toThrow(/unexpected response/); }); - it('throws for a failed bulk item, naming its elementId', async () => { + it('keeps a failed bulk item as a per-element error, next to the good ones', async () => { routes['/v1/objects/value'] = () => ({ + json: { + success: false, + results: [ + { + success: false, + elementId: 'bad-id', + responseDetail: { title: 'Error', status: 404, detail: 'Not found' }, + }, + { + success: true, + elementId: 'good-id', + result: { isComposition: false, value: 1, quality: 'Good', timestamp: 't' }, + }, + ], + }, + }); + expect(await client().getObjectValues(['bad-id', 'good-id'])).toEqual([ + { elementId: 'bad-id', error: '404 Error Not found' }, + { + elementId: 'good-id', + result: { isComposition: false, value: 1, quality: 'Good', timestamp: 't' }, + }, + ]); + }); + + it('getRelatedObjects throws for a failed item instead of returning []', async () => { + routes['/v1/objects/related'] = () => ({ json: { success: false, results: [ @@ -190,11 +238,22 @@ describe('RestClientBinding', () => { ], }, }); - await expect(client().getObjectValues(['bad-id'])).rejects.toThrow( + await expect(client().getRelatedObjects('bad-id')).rejects.toThrow( /'bad-id': 404 Error Not found/, ); }); + it('findSuggestions propagates a failure instead of reporting "no close matches"', async () => { + routes['/v1/objects'] = () => ({ + status: 401, + json: { + success: false, + responseDetail: { title: 'Unauthorized', status: 401, detail: 'bad token' }, + }, + }); + await expect(client().findSuggestions('CoffeeMachine')).rejects.toThrow(/HTTP 401/); + }); + it('getObjectById propagates a non-404 failure instead of reporting "not found"', async () => { routes['/v1/objects/list'] = () => ({ status: 401, @@ -222,12 +281,17 @@ describe('RestClientBinding', () => { const res = await handleInspectAsset( new RestClientBinding('https://portal.example/api/i3x/abc/v1'), - { elementId: machine.elementId, includeValues: true, includeSchema: false }, + { + elementId: machine.elementId, + includeValues: true, + includeSchema: false, + maxDepth: 2, + }, ); expect(res).toMatchObject({ elementId: machine.elementId, displayName: 'CoffeeMachineA', - subComponents: [{ elementId: boiler.elementId }], + subComponents: [{ elementId: boiler.elementId, relationship: 'HasComponent' }], currentValues: { value: { Temperature: 92.5 } }, }); expect((res as { subComponents: unknown[] }).subComponents).toHaveLength(1); diff --git a/packages/mcp/tsconfig.json b/packages/mcp/tsconfig.json index ffa5d3e..d42587a 100644 --- a/packages/mcp/tsconfig.json +++ b/packages/mcp/tsconfig.json @@ -6,6 +6,7 @@ }, "include": ["src/**/*"], "references": [ - { "path": "../core" } + { "path": "../core" }, + { "path": "../rest-server" } ] } diff --git a/packages/rest-server/src/app.ts b/packages/rest-server/src/app.ts index 0aeac06..db61040 100644 --- a/packages/rest-server/src/app.ts +++ b/packages/rest-server/src/app.ts @@ -56,8 +56,16 @@ export interface RestServerDeps { }; } -export async function createApp(deps: RestServerDeps): Promise { - const app = Fastify({ logger: true }); +export interface CreateAppOptions { + /** Fastify request logging (to stdout); off for in-process use over MCP stdio. Default true. */ + logger?: boolean; +} + +export async function createApp( + deps: RestServerDeps, + options: CreateAppOptions = {}, +): Promise { + const app = Fastify({ logger: options.logger ?? true }); // Make deps available to all routes via Fastify's decorate API. app.decorate('deps', deps); diff --git a/packages/rest-server/src/index.ts b/packages/rest-server/src/index.ts index 7af382a..6cc3ec5 100644 --- a/packages/rest-server/src/index.ts +++ b/packages/rest-server/src/index.ts @@ -1,2 +1,2 @@ -export type { RestServerDeps } from './app.js'; +export type { CreateAppOptions, RestServerDeps } from './app.js'; export { createApp } from './app.js'; From 657d869724a54b31b6282fa51663d6df4cd0bcdb Mon Sep 17 00:00:00 2001 From: Etienne Rossignon Date: Mon, 26 Oct 2026 07:42:00 +0100 Subject: [PATCH 3/3] fix(mcp): address PR review: monitor Bad/null, optional rest-server, shared children - monitor_asset_events alerts on an element read that is Bad with value null (how a failed read looks); the value guard skipped exactly those. - InProcessConnectorBinding moves to the mcp package's in-process subpath export, and the rest-server package becomes an optional peer dependency, so the REST-only CLI no longer installs Fastify and its plugins. - explore/inspect drop reverse relationships instead of filtering on parentId, which names a single parent: a shared component now shows under each parent. - A successful bulk item without result becomes a per-element error instead of an entry with neither result nor error. --- package-lock.json | 10 ++++- packages/mcp/package.json | 15 ++++++- packages/mcp/src/bindings/rest-binding.ts | 7 ++-- packages/mcp/src/in-process.ts | 3 ++ packages/mcp/src/index.ts | 1 - packages/mcp/src/tools/explore-hierarchy.ts | 8 ++-- packages/mcp/src/tools/inspect-asset.ts | 3 +- packages/mcp/src/tools/monitor-events.ts | 2 +- packages/mcp/src/utils/relationships.ts | 10 +++++ packages/mcp/tests/contract.test.ts | 5 +-- packages/mcp/tests/mcp-server.test.ts | 46 +++++++++++++++++++++ packages/mcp/tests/rest-binding.test.ts | 7 ++++ packages/mcp/tsup.config.ts | 2 +- 13 files changed, 103 insertions(+), 16 deletions(-) create mode 100644 packages/mcp/src/in-process.ts create mode 100644 packages/mcp/src/utils/relationships.ts diff --git a/package-lock.json b/package-lock.json index f977953..0b4d4f8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -7448,16 +7448,24 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.32.2", "@node-i3x/core": "0.11.2", - "@node-i3x/rest-server": "0.11.2", "zod": "^4.6.7" }, "bin": { "node-i3x-mcp": "dist/cli.js" }, "devDependencies": { + "@node-i3x/rest-server": "0.11.2", "@types/node": "^26.6.5", "tsup": "^9.0.0", "typescript": "6" + }, + "peerDependencies": { + "@node-i3x/rest-server": "0.11.2" + }, + "peerDependenciesMeta": { + "@node-i3x/rest-server": { + "optional": true + } } }, "packages/opcua-connector": { diff --git a/packages/mcp/package.json b/packages/mcp/package.json index 22d82c5..8ceb75e 100644 --- a/packages/mcp/package.json +++ b/packages/mcp/package.json @@ -37,6 +37,11 @@ "source": "./src/index.ts", "types": "./dist/index.d.ts", "import": "./dist/index.js" + }, + "./in-process": { + "source": "./src/in-process.ts", + "types": "./dist/in-process.d.ts", + "import": "./dist/in-process.js" } }, "publishConfig": { @@ -57,10 +62,18 @@ "dependencies": { "@modelcontextprotocol/sdk": "^1.32.2", "@node-i3x/core": "0.11.2", - "@node-i3x/rest-server": "0.11.2", "zod": "^4.6.7" }, + "peerDependencies": { + "@node-i3x/rest-server": "0.11.2" + }, + "peerDependenciesMeta": { + "@node-i3x/rest-server": { + "optional": true + } + }, "devDependencies": { + "@node-i3x/rest-server": "0.11.2", "@types/node": "^26.6.5", "tsup": "^9.0.0", "typescript": "6" diff --git a/packages/mcp/src/bindings/rest-binding.ts b/packages/mcp/src/bindings/rest-binding.ts index 6769401..f779712 100644 --- a/packages/mcp/src/bindings/rest-binding.ts +++ b/packages/mcp/src/bindings/rest-binding.ts @@ -118,9 +118,10 @@ export class RestClientBinding implements I3XClientBinding { const items = await this.postBulk(path, { elementIds, ...body }); return items.map((item, idx) => { const elementId = item.elementId ?? elementIds[idx] ?? `element_${idx}`; - return item.success - ? { elementId, result: (item.result ?? undefined) as T | undefined } - : { elementId, error: describeFailure(item) }; + if (!item.success) return { elementId, error: describeFailure(item) }; + // A success without a result would otherwise read as "no data" with no warning. + if (item.result == null) return { elementId, error: 'Server returned no result' }; + return { elementId, result: item.result as T }; }); } diff --git a/packages/mcp/src/in-process.ts b/packages/mcp/src/in-process.ts new file mode 100644 index 0000000..fb3e9a3 --- /dev/null +++ b/packages/mcp/src/in-process.ts @@ -0,0 +1,3 @@ +// @node-i3x/mcp/in-process: needs the optional peer dependency @node-i3x/rest-server. +// Kept out of the root export so the REST-only CLI and library never install Fastify. +export * from './bindings/connector-binding.js'; diff --git a/packages/mcp/src/index.ts b/packages/mcp/src/index.ts index 46557af..036ab93 100644 --- a/packages/mcp/src/index.ts +++ b/packages/mcp/src/index.ts @@ -1,4 +1,3 @@ -export * from './bindings/connector-binding.js'; export * from './bindings/rest-binding.js'; export * from './bindings/types.js'; export * from './server.js'; diff --git a/packages/mcp/src/tools/explore-hierarchy.ts b/packages/mcp/src/tools/explore-hierarchy.ts index 40cc8bb..b302107 100644 --- a/packages/mcp/src/tools/explore-hierarchy.ts +++ b/packages/mcp/src/tools/explore-hierarchy.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; import type { HierarchyNode, I3XClientBinding } from '../bindings/types.js'; +import { isParentRelationship } from '../utils/relationships.js'; export const exploreHierarchySchema = z.object({ rootElementId: z @@ -51,12 +52,13 @@ export async function handleExploreHierarchy( visited.add(asset.elementId); if (currentDepth < maxDepth) { - // Follow children only: related objects also include the parent (reverse - // relationship), and parentId is what the spec ties to the hierarchy. + // Follow children only: related objects also include the parent. Not filtered + // on parentId, which names a single parent: a shared component has several. const related = await binding.getRelatedObjects(asset.elementId); const unvisitedChildren = related + .filter((r) => !isParentRelationship(r.sourceRelationship)) .map((r) => r.object) - .filter((c) => c.parentId === asset.elementId && !visited.has(c.elementId)); + .filter((c) => !visited.has(c.elementId)); if (unvisitedChildren.length > 0) { node.children = await Promise.all( diff --git a/packages/mcp/src/tools/inspect-asset.ts b/packages/mcp/src/tools/inspect-asset.ts index eee089c..54a56b9 100644 --- a/packages/mcp/src/tools/inspect-asset.ts +++ b/packages/mcp/src/tools/inspect-asset.ts @@ -1,5 +1,6 @@ import { z } from 'zod'; import type { I3XClientBinding } from '../bindings/types.js'; +import { isParentRelationship } from '../utils/relationships.js'; export const inspectAssetSchema = z.object({ elementId: z @@ -71,7 +72,7 @@ export async function handleInspectAsset( // Sub-components: related objects also include the parent (a reverse relationship). if (relatedResult.status === 'fulfilled') { const subComponents = relatedResult.value.filter( - (r) => r.object.elementId !== asset.parentId, + (r) => !isParentRelationship(r.sourceRelationship), ); if (subComponents.length > 0) { result.subComponents = subComponents.map((r) => ({ diff --git a/packages/mcp/src/tools/monitor-events.ts b/packages/mcp/src/tools/monitor-events.ts index 600f9fa..c3771f2 100644 --- a/packages/mcp/src/tools/monitor-events.ts +++ b/packages/mcp/src/tools/monitor-events.ts @@ -34,7 +34,7 @@ export async function handleMonitorEvents( alerts.push({ elementId, error }); continue; } - if (result && result.value !== null && !isGood(result.quality)) { + if (result && !isGood(result.quality)) { alerts.push({ elementId, quality: result.quality }); } for (const [componentId, vqt] of Object.entries(result?.components ?? {})) { diff --git a/packages/mcp/src/utils/relationships.ts b/packages/mcp/src/utils/relationships.ts new file mode 100644 index 0000000..deda144 --- /dev/null +++ b/packages/mcp/src/utils/relationships.ts @@ -0,0 +1,10 @@ +/** + * Child-to-parent relationship names in `/objects/related` results: node-i3x's + * IsComponentOf and the i3X spec's ComponentOf / HasParent. Everything else points + * away from the source object. + */ +const PARENT_RELATIONSHIPS = new Set(['IsComponentOf', 'ComponentOf', 'HasParent']); + +export function isParentRelationship(sourceRelationship: string): boolean { + return PARENT_RELATIONSHIPS.has(sourceRelationship); +} diff --git a/packages/mcp/tests/contract.test.ts b/packages/mcp/tests/contract.test.ts index efb292d..6f3b812 100644 --- a/packages/mcp/tests/contract.test.ts +++ b/packages/mcp/tests/contract.test.ts @@ -4,11 +4,8 @@ import { createI3xStack, nullLogger } from '@node-i3x/core'; import type { RestServerDeps } from '@node-i3x/rest-server'; import { beforeAll, describe, expect, it } from 'vitest'; import { MockDataSource } from '../../rest-server/tests/helpers/mock-data-source.js'; -import { - InProcessConnectorBinding, - injectFetch, -} from '../src/bindings/connector-binding.js'; import { RestClientBinding } from '../src/bindings/rest-binding.js'; +import { InProcessConnectorBinding, injectFetch } from '../src/in-process.js'; import { handleExploreHierarchy } from '../src/tools/explore-hierarchy.js'; import { handleInspectAsset } from '../src/tools/inspect-asset.js'; diff --git a/packages/mcp/tests/mcp-server.test.ts b/packages/mcp/tests/mcp-server.test.ts index b17e6f1..d00a891 100644 --- a/packages/mcp/tests/mcp-server.test.ts +++ b/packages/mcp/tests/mcp-server.test.ts @@ -266,6 +266,52 @@ describe('@node-i3x/mcp High-Value Tools', () => { expect(ok.mode).toBe('history'); }); + it('i3x_monitor_asset_events alerts on a Bad element read with a null value', async () => { + const binding = new MockClientBinding(); + binding.getObjectValues = async () => [ + { + elementId: 'Controller_1', + result: { + isComposition: false, + value: null, + quality: 'Bad' as const, + timestamp: '2026-10-25T10:00:00Z', + }, + }, + ]; + const res = await handleMonitorEvents(binding, { + elementIds: ['Controller_1'], + maxDepth: 2, + }); + expect(res.alerts).toEqual([{ elementId: 'Controller_1', quality: 'Bad' }]); + }); + + it('i3x_explore_plant_hierarchy keeps a shared component under every parent', async () => { + const binding = new MockClientBinding(); + binding.getRelatedObjects = async (elementId: string) => + elementId === 'Boiler_101' + ? [ + { + // The server reports one parentId; this component is shared. + sourceRelationship: 'HasComponent', + object: { + elementId: 'Shared_Valve', + parentId: 'Boiler_202', + isComposition: false, + }, + }, + { + sourceRelationship: 'IsComponentOf', + object: { elementId: 'Plant', parentId: null, isComposition: true }, + }, + ] + : []; + const res = await handleExploreHierarchy(binding, { maxDepth: 2 }); + expect(res.plantTopology?.[0]?.children?.map((c) => c.elementId)).toEqual([ + 'Shared_Valve', + ]); + }); + it('i3x_discover_types_and_schemas lists ObjectTypes and schema field rules', async () => { const res = await handleDiscoverTypes(mockBinding, {}); diff --git a/packages/mcp/tests/rest-binding.test.ts b/packages/mcp/tests/rest-binding.test.ts index e79d11c..f062523 100644 --- a/packages/mcp/tests/rest-binding.test.ts +++ b/packages/mcp/tests/rest-binding.test.ts @@ -225,6 +225,13 @@ describe('RestClientBinding', () => { ]); }); + it('reports a successful item without result instead of silently dropping it', async () => { + routes['/v1/objects/value'] = bulk(() => null); + expect(await client().getObjectValues(['id-1'])).toEqual([ + { elementId: 'id-1', error: 'Server returned no result' }, + ]); + }); + it('getRelatedObjects throws for a failed item instead of returning []', async () => { routes['/v1/objects/related'] = () => ({ json: { diff --git a/packages/mcp/tsup.config.ts b/packages/mcp/tsup.config.ts index a9ff808..e98efef 100644 --- a/packages/mcp/tsup.config.ts +++ b/packages/mcp/tsup.config.ts @@ -1,7 +1,7 @@ import { defineConfig } from "tsup"; export default defineConfig({ - entry: ["src/index.ts", "src/cli.ts"], + entry: ["src/index.ts", "src/cli.ts", "src/in-process.ts"], format: ["esm"], dts: { compilerOptions: { composite: false },